From 08fa7bfefbe19711be2c29b9be1d5f960bee1911 Mon Sep 17 00:00:00 2001 From: Ynda Jas Date: Wed, 17 Apr 2024 17:11:23 +0100 Subject: [PATCH 01/11] Add organisations to republishing tasks index This adds the task to republish an organisation by slug to the index of republishing tasks. This is not yet linked up to next page This also adds a bit more structure to the page: - two new `h2`s separate the specific page republishing tasks from other republishing tasks that require a slug - the main preamble is moved into a details component to reduce the text wall-i-ness of the page --- app/views/admin/republishing/index.html.erb | 68 ++++++++++++++++--- .../admin/republishing_controller_test.rb | 7 +- 2 files changed, 64 insertions(+), 11 deletions(-) diff --git a/app/views/admin/republishing/index.html.erb b/app/views/admin/republishing/index.html.erb index de816ed0065..cf84cf85d08 100644 --- a/app/views/admin/republishing/index.html.erb +++ b/app/views/admin/republishing/index.html.erb @@ -4,18 +4,33 @@
-

- Sometimes it may be necessary to republish content to the Publishing API. This will refresh the content on the website. -

+
+ + + Guidance on republishing content + + +
+

+ Sometimes it may be necessary to republish content to the Publishing API. This will refresh the content on the website. +

-

- For example, if we make an update to govspeak and a publishing application pre-renders that content prior to its submission to Publishing API, that would require us to re-render and save new HTML for content. -

+

+ For example, if we make an update to govspeak and a publishing application pre-renders that content prior to its submission to Publishing API, that would require us to re-render and save new HTML for content. +

+ +

+ The following actions will allow you to schedule the republishing of content that was originally published in this application. + Any linked editions will also be republished through dependency resolution. + Try to pick the republishing task most focused to the scope of what you need to republish to avoid unnecessary server load. +

+
+
+ +

Individual pages

- The following actions will allow you to schedule the republishing of content that was originally published in this application. - Any linked editions will also be republished through dependency resolution. - Try to pick the republishing task most focused to the scope of what you need to republish to avoid unnecessary server load. + You can republish a selection of individual pages using the links below. If the page you wish to republish is not listed below, you may be able to use the 'Document' link in the next section.

<%= render "govuk_publishing_components/components/table", { @@ -42,5 +57,40 @@ ] end, } %> + +

Other individual content

+ +

+ You can republish certain types of other indidivual content using the following actions. +

+ +

+ You'll need to provide the slug for the specific content you want to republish. Instructions for how to find this are provided on each page. +

+ + <%= render "govuk_publishing_components/components/table", { + head: [ + { + text: "Content type", + }, + { + text: "Action", + }, + ], + rows: [ + [ + { + text: "Organisation", + }, + { + text: link_to(sanitize("Republish #{tag.span('an organisation', class: 'govuk-visually-hidden')}"), + '#', + id: "republish-organisation", + class: "govuk-link", + ), + }, + ], + ], + } %>
diff --git a/test/functional/admin/republishing_controller_test.rb b/test/functional/admin/republishing_controller_test.rb index ef76f2d6c97..842fafe1cf9 100644 --- a/test/functional/admin/republishing_controller_test.rb +++ b/test/functional/admin/republishing_controller_test.rb @@ -11,8 +11,11 @@ class Admin::RepublishingControllerTest < ActionController::TestCase view_test "GDS Admin users should be able to acess the GET :index and see links to republishable content" do get :index - assert_select ".govuk-table__cell:nth-child(1) a[href='https://www.test.gov.uk/government/history/past-prime-ministers']", text: "Past Prime Ministers" - assert_select ".govuk-table__cell:nth-child(2) a[href='/government/admin/republishing/page/past-prime-ministers/confirm']", text: "Republish 'Past Prime Ministers' page" + assert_select ".govuk-table:nth-of-type(1) .govuk-table__cell:nth-child(1) a[href='https://www.test.gov.uk/government/history/past-prime-ministers']", text: "Past Prime Ministers" + assert_select ".govuk-table:nth-of-type(1) .govuk-table__cell:nth-child(2) a[href='/government/admin/republishing/page/past-prime-ministers/confirm']", text: "Republish 'Past Prime Ministers' page" + + assert_select ".govuk-table:nth-of-type(2) .govuk-table__cell:nth-child(2) a[href='#']", text: "Republish an organisation" + assert_response :ok end From 4c0a9b1b0fdd2389128ea2203e46a8ea8c080530 Mon Sep 17 00:00:00 2001 From: Ynda Jas Date: Thu, 18 Apr 2024 19:11:23 +0100 Subject: [PATCH 02/11] Add page for finding an organisation to republish This is not yet hooked up with a search action so the form doesn't do anything in this commit --- .../admin/republishing_controller.rb | 2 ++ .../republishing/find_organisation.html.erb | 24 +++++++++++++++++++ app/views/admin/republishing/index.html.erb | 2 +- config/routes.rb | 3 +++ .../admin/republishing_controller_test.rb | 15 +++++++++++- 5 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 app/views/admin/republishing/find_organisation.html.erb diff --git a/app/controllers/admin/republishing_controller.rb b/app/controllers/admin/republishing_controller.rb index 7aeaafa6be6..ade1d745a6b 100644 --- a/app/controllers/admin/republishing_controller.rb +++ b/app/controllers/admin/republishing_controller.rb @@ -24,6 +24,8 @@ def republish_page redirect_to(admin_republishing_index_path) end + def find_organisation; end + private def enforce_permissions! diff --git a/app/views/admin/republishing/find_organisation.html.erb b/app/views/admin/republishing/find_organisation.html.erb new file mode 100644 index 00000000000..6e6ebbaafc2 --- /dev/null +++ b/app/views/admin/republishing/find_organisation.html.erb @@ -0,0 +1,24 @@ +<% content_for :page_title, "Republish an organisation" %> +<% content_for :title, "Which organisation would you like to republish?" %> +<% content_for :title_margin_bottom, 6 %> + +
+
+ <%= form_with(url: '#', method: :post, data: { + module: "prevent-multiple-form-submissions", + }) do %> + + <%= render "govuk_publishing_components/components/input", { + label: { + text: "Enter the slug for the organisation", + }, + hint: "You can get the slug from the last part of the public URL - everything after '/government/organisations/'.", + name: "organisation_slug", + } %> + + <%= render "govuk_publishing_components/components/button", { + text: "Continue", + } %> + <% end %> +
+
diff --git a/app/views/admin/republishing/index.html.erb b/app/views/admin/republishing/index.html.erb index cf84cf85d08..869cf62fee3 100644 --- a/app/views/admin/republishing/index.html.erb +++ b/app/views/admin/republishing/index.html.erb @@ -84,7 +84,7 @@ }, { text: link_to(sanitize("Republish #{tag.span('an organisation', class: 'govuk-visually-hidden')}"), - '#', + admin_republishing_organisation_find_path, id: "republish-organisation", class: "govuk-link", ), diff --git a/config/routes.rb b/config/routes.rb index 1d7dc1a699c..9bb8944cd3f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -28,6 +28,9 @@ def redirect(path, options = { prefix: Whitehall.router_prefix }) get "/:page_slug/confirm" => "republishing#confirm_page", as: :republishing_page_confirm post "/:page_slug/republish" => "republishing#republish_page", as: :republishing_page_republish end + scope :organisation do + get "/find" => "republishing#find_organisation", as: :republishing_organisation_find + end end resources :documents, only: [] do diff --git a/test/functional/admin/republishing_controller_test.rb b/test/functional/admin/republishing_controller_test.rb index 842fafe1cf9..1e9c2f10114 100644 --- a/test/functional/admin/republishing_controller_test.rb +++ b/test/functional/admin/republishing_controller_test.rb @@ -14,7 +14,7 @@ class Admin::RepublishingControllerTest < ActionController::TestCase assert_select ".govuk-table:nth-of-type(1) .govuk-table__cell:nth-child(1) a[href='https://www.test.gov.uk/government/history/past-prime-ministers']", text: "Past Prime Ministers" assert_select ".govuk-table:nth-of-type(1) .govuk-table__cell:nth-child(2) a[href='/government/admin/republishing/page/past-prime-ministers/confirm']", text: "Republish 'Past Prime Ministers' page" - assert_select ".govuk-table:nth-of-type(2) .govuk-table__cell:nth-child(2) a[href='#']", text: "Republish an organisation" + assert_select ".govuk-table:nth-of-type(2) .govuk-table__cell:nth-child(2) a[href='/government/admin/republishing/organisation/find']", text: "Republish an organisation" assert_response :ok end @@ -67,4 +67,17 @@ class Admin::RepublishingControllerTest < ActionController::TestCase post :republish_page, params: { page_slug: "past-prime-ministers" } assert_response :forbidden end + + view_test "GDS Admin users should be able to GET :find_organisation" do + get :find_organisation + + assert_response :ok + end + + test "Non-GDS Admin users should not be able to GET :find_organisation" do + login_as :writer + + get :find_organisation + assert_response :forbidden + end end From c87915156d4391ccddf38f0cb3c2324edc695dfc Mon Sep 17 00:00:00 2001 From: Ynda Jas Date: Fri, 19 Apr 2024 14:17:33 +0100 Subject: [PATCH 03/11] Tidy up republishing controller test descriptions --- .../functional/admin/republishing_controller_test.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/functional/admin/republishing_controller_test.rb b/test/functional/admin/republishing_controller_test.rb index 1e9c2f10114..4452ca5aeb2 100644 --- a/test/functional/admin/republishing_controller_test.rb +++ b/test/functional/admin/republishing_controller_test.rb @@ -8,7 +8,7 @@ class Admin::RepublishingControllerTest < ActionController::TestCase should_be_an_admin_controller - view_test "GDS Admin users should be able to acess the GET :index and see links to republishable content" do + view_test "GDS Admin users should be able to GET :index and see links to republishable content" do get :index assert_select ".govuk-table:nth-of-type(1) .govuk-table__cell:nth-child(1) a[href='https://www.test.gov.uk/government/history/past-prime-ministers']", text: "Past Prime Ministers" @@ -19,14 +19,14 @@ class Admin::RepublishingControllerTest < ActionController::TestCase assert_response :ok end - test "Non-GDS Admin users should not be able to access the GET :index" do + test "Non-GDS Admin users should not be able to GET :index" do login_as :writer get :index assert_response :forbidden end - test "GDS Admin users should be able to access GET :confirm_page with a republishable page slug" do + test "GDS Admin users should be able to GET :confirm_page with a republishable page slug" do get :confirm_page, params: { page_slug: "past-prime-ministers" } assert_response :ok end @@ -36,14 +36,14 @@ class Admin::RepublishingControllerTest < ActionController::TestCase assert_response :not_found end - test "Non-GDS Admin users should not be able to access GET :confirm_page" do + test "Non-GDS Admin users should not be able to GET :confirm_page" do login_as :writer get :confirm_page, params: { page_slug: "past-prime-ministers" } assert_response :forbidden end - test "GDS Admin users should be able to access POST :republish_page with a republishable page slug" do + test "GDS Admin users should be able to POST :republish_page with a republishable page slug" do PresentPageToPublishingApiWorker.expects(:perform_async).with("PublishingApi::HistoricalAccountsIndexPresenter").once post :republish_page, params: { page_slug: "past-prime-ministers" } @@ -59,7 +59,7 @@ class Admin::RepublishingControllerTest < ActionController::TestCase assert_response :not_found end - test "Non-GDS Admin users should not be able to access POST :republish_page" do + test "Non-GDS Admin users should not be able to POST :republish_page" do PresentPageToPublishingApiWorker.expects(:perform_async).with("PublishingApi::HistoricalAccountsIndexPresenter").never login_as :writer From 11d624c6892004319aff5258b79d8057276a1bd5 Mon Sep 17 00:00:00 2001 From: Ynda Jas Date: Thu, 18 Apr 2024 19:12:56 +0100 Subject: [PATCH 04/11] Search for an organisation to republish The search will currently redirect to the main admin page when the organisation is found. This will be updated to the confirmation page once added --- .../admin/republishing_controller.rb | 11 +++++++++ .../republishing/find_organisation.html.erb | 2 +- config/routes.rb | 1 + .../admin/republishing_controller_test.rb | 24 +++++++++++++++++++ 4 files changed, 37 insertions(+), 1 deletion(-) diff --git a/app/controllers/admin/republishing_controller.rb b/app/controllers/admin/republishing_controller.rb index ade1d745a6b..d3152369125 100644 --- a/app/controllers/admin/republishing_controller.rb +++ b/app/controllers/admin/republishing_controller.rb @@ -26,6 +26,17 @@ def republish_page def find_organisation; end + def search_organisation + organisation = Organisation.find_by(slug: params[:organisation_slug]) + + unless organisation + flash[:alert] = "Organisation with slug '#{params[:organisation_slug]}' not found" + return redirect_to(admin_republishing_organisation_find_path) + end + + redirect_to("#") + end + private def enforce_permissions! diff --git a/app/views/admin/republishing/find_organisation.html.erb b/app/views/admin/republishing/find_organisation.html.erb index 6e6ebbaafc2..6a87eb550ce 100644 --- a/app/views/admin/republishing/find_organisation.html.erb +++ b/app/views/admin/republishing/find_organisation.html.erb @@ -4,7 +4,7 @@
- <%= form_with(url: '#', method: :post, data: { + <%= form_with(url: admin_republishing_organisation_search_path, method: :post, data: { module: "prevent-multiple-form-submissions", }) do %> diff --git a/config/routes.rb b/config/routes.rb index 9bb8944cd3f..aef487d9d41 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -30,6 +30,7 @@ def redirect(path, options = { prefix: Whitehall.router_prefix }) end scope :organisation do get "/find" => "republishing#find_organisation", as: :republishing_organisation_find + post "/search" => "republishing#search_organisation", as: :republishing_organisation_search end end diff --git a/test/functional/admin/republishing_controller_test.rb b/test/functional/admin/republishing_controller_test.rb index 4452ca5aeb2..c746894ef27 100644 --- a/test/functional/admin/republishing_controller_test.rb +++ b/test/functional/admin/republishing_controller_test.rb @@ -80,4 +80,28 @@ class Admin::RepublishingControllerTest < ActionController::TestCase get :find_organisation assert_response :forbidden end + + test "GDS Admin users should be able to POST :search_organisation with an existing organisation slug" do + create(:organisation, slug: "an-existing-organisation") + + post :search_organisation, params: { organisation_slug: "an-existing-organisation" } + + assert_redirected_to "#" + end + + test "GDS Admin users should be redirected back to :find_organisation when trying to POST :search_organisation with a nonexistent organisation slug" do + get :search_organisation, params: { organisation_slug: "not-an-existing-organisation" } + + assert_redirected_to admin_republishing_organisation_find_path + assert_equal "Organisation with slug 'not-an-existing-organisation' not found", flash[:alert] + end + + test "Non-GDS Admin users should not be able to POST :search_organisation" do + create(:organisation, slug: "an-existing-organisation") + + login_as :writer + + post :search_organisation, params: { organisation_slug: "an-existing-organisation" } + assert_response :forbidden + end end From 9631ef1b3f916701671d8d38dc4bb615b31b5817 Mon Sep 17 00:00:00 2001 From: Ynda Jas Date: Thu, 18 Apr 2024 19:22:11 +0100 Subject: [PATCH 05/11] Confirm organisation to republish This is not yet hooked up with a republish action so the form doesn't do anything in this commit --- .../admin/republishing_controller.rb | 13 ++++++++--- .../confirm_organisation.html.erb | 18 +++++++++++++++ config/routes.rb | 1 + .../admin/republishing_controller_test.rb | 23 ++++++++++++++++++- 4 files changed, 51 insertions(+), 4 deletions(-) create mode 100644 app/views/admin/republishing/confirm_organisation.html.erb diff --git a/app/controllers/admin/republishing_controller.rb b/app/controllers/admin/republishing_controller.rb index d3152369125..8c5bb038ced 100644 --- a/app/controllers/admin/republishing_controller.rb +++ b/app/controllers/admin/republishing_controller.rb @@ -27,14 +27,21 @@ def republish_page def find_organisation; end def search_organisation - organisation = Organisation.find_by(slug: params[:organisation_slug]) + @organisation = Organisation.find_by(slug: params[:organisation_slug]) - unless organisation + unless @organisation flash[:alert] = "Organisation with slug '#{params[:organisation_slug]}' not found" return redirect_to(admin_republishing_organisation_find_path) end - redirect_to("#") + redirect_to(admin_republishing_organisation_confirm_path(params[:organisation_slug])) + end + + def confirm_organisation + unless @organisation&.slug == params[:organisation_slug] + @organisation = Organisation.find_by(slug: params[:organisation_slug]) + render "admin/errors/not_found", status: :not_found unless @organisation + end end private diff --git a/app/views/admin/republishing/confirm_organisation.html.erb b/app/views/admin/republishing/confirm_organisation.html.erb new file mode 100644 index 00000000000..bec6106cc87 --- /dev/null +++ b/app/views/admin/republishing/confirm_organisation.html.erb @@ -0,0 +1,18 @@ +<% content_for :page_title, "Republish '#{@organisation.name}'" %> +<% content_for :title, "Are you sure you want to republish '#{@organisation.name}'?" %> +<% content_for :title_margin_bottom, 6 %> + +
+
+

+ This will schedule the <%= link_to @organisation.name, @organisation.public_url, { class: "govuk-link" } %> organisation to be republished. +

+ <%= form_with(url: "#", method: :post, data: { + module: "prevent-multiple-form-submissions", + }) do + render("govuk_publishing_components/components/button", { + text: "Confirm republishing", + }) + end %> +
+
diff --git a/config/routes.rb b/config/routes.rb index aef487d9d41..f365d06cbe0 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -31,6 +31,7 @@ def redirect(path, options = { prefix: Whitehall.router_prefix }) scope :organisation do get "/find" => "republishing#find_organisation", as: :republishing_organisation_find post "/search" => "republishing#search_organisation", as: :republishing_organisation_search + get "/:organisation_slug/confirm" => "republishing#confirm_organisation", as: :republishing_organisation_confirm end end diff --git a/test/functional/admin/republishing_controller_test.rb b/test/functional/admin/republishing_controller_test.rb index c746894ef27..bbdfa695bf7 100644 --- a/test/functional/admin/republishing_controller_test.rb +++ b/test/functional/admin/republishing_controller_test.rb @@ -86,7 +86,7 @@ class Admin::RepublishingControllerTest < ActionController::TestCase post :search_organisation, params: { organisation_slug: "an-existing-organisation" } - assert_redirected_to "#" + assert_redirected_to admin_republishing_organisation_confirm_path("an-existing-organisation") end test "GDS Admin users should be redirected back to :find_organisation when trying to POST :search_organisation with a nonexistent organisation slug" do @@ -104,4 +104,25 @@ class Admin::RepublishingControllerTest < ActionController::TestCase post :search_organisation, params: { organisation_slug: "an-existing-organisation" } assert_response :forbidden end + + test "GDS Admin users should be able to GET :confirm_organisation with an existing organisation slug" do + create(:organisation, slug: "an-existing-organisation") + + get :confirm_organisation, params: { organisation_slug: "an-existing-organisation" } + assert_response :ok + end + + test "GDS Admin users should see a 404 page when trying to GET :confirm_organisation with a nonexistent organisation slug" do + get :confirm_organisation, params: { organisation_slug: "not-an-existing-organisation" } + assert_response :not_found + end + + test "Non-GDS Admin users should not be able to GET :confirm_organisation" do + create(:organisation, slug: "an-existing-organisation") + + login_as :writer + + get :confirm_organisation, params: { organisation_slug: "an-existing-organisation" } + assert_response :forbidden + end end From 4b5e6fec17dc1ba81671f21c0089a9bf4ea1e383 Mon Sep 17 00:00:00 2001 From: Ynda Jas Date: Thu, 18 Apr 2024 19:23:11 +0100 Subject: [PATCH 06/11] Republish an organisation --- .../admin/republishing_controller.rb | 11 +++++++ .../confirm_organisation.html.erb | 2 +- config/routes.rb | 1 + .../admin/republishing_controller_test.rb | 29 +++++++++++++++++++ 4 files changed, 42 insertions(+), 1 deletion(-) diff --git a/app/controllers/admin/republishing_controller.rb b/app/controllers/admin/republishing_controller.rb index 8c5bb038ced..fedf7292ace 100644 --- a/app/controllers/admin/republishing_controller.rb +++ b/app/controllers/admin/republishing_controller.rb @@ -44,6 +44,17 @@ def confirm_organisation end end + def republish_organisation + unless @organisation&.slug == params[:organisation_slug] + @organisation = Organisation.find_by(slug: params[:organisation_slug]) + return render "admin/errors/not_found", status: :not_found unless @organisation + end + + @organisation.publish_to_publishing_api + flash[:notice] = "The '#{@organisation.name}' organisation has been scheduled for republishing" + redirect_to(admin_republishing_index_path) + end + private def enforce_permissions! diff --git a/app/views/admin/republishing/confirm_organisation.html.erb b/app/views/admin/republishing/confirm_organisation.html.erb index bec6106cc87..26bc9d2cb51 100644 --- a/app/views/admin/republishing/confirm_organisation.html.erb +++ b/app/views/admin/republishing/confirm_organisation.html.erb @@ -7,7 +7,7 @@

This will schedule the <%= link_to @organisation.name, @organisation.public_url, { class: "govuk-link" } %> organisation to be republished.

- <%= form_with(url: "#", method: :post, data: { + <%= form_with(url: admin_republishing_organisation_republish_path(@organisation.slug), method: :post, data: { module: "prevent-multiple-form-submissions", }) do render("govuk_publishing_components/components/button", { diff --git a/config/routes.rb b/config/routes.rb index f365d06cbe0..58eeaf29937 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -32,6 +32,7 @@ def redirect(path, options = { prefix: Whitehall.router_prefix }) get "/find" => "republishing#find_organisation", as: :republishing_organisation_find post "/search" => "republishing#search_organisation", as: :republishing_organisation_search get "/:organisation_slug/confirm" => "republishing#confirm_organisation", as: :republishing_organisation_confirm + post "/:organisation_slug/republish" => "republishing#republish_organisation", as: :republishing_organisation_republish end end diff --git a/test/functional/admin/republishing_controller_test.rb b/test/functional/admin/republishing_controller_test.rb index bbdfa695bf7..cc577bbc44f 100644 --- a/test/functional/admin/republishing_controller_test.rb +++ b/test/functional/admin/republishing_controller_test.rb @@ -125,4 +125,33 @@ class Admin::RepublishingControllerTest < ActionController::TestCase get :confirm_organisation, params: { organisation_slug: "an-existing-organisation" } assert_response :forbidden end + + test "GDS Admin users should be able to POST :republish_organisation with an existing organisation slug" do + create(:organisation, slug: "an-existing-organisation", name: "An Existing Organisation") + + Organisation.any_instance.expects(:publish_to_publishing_api).once + + post :republish_organisation, params: { organisation_slug: "an-existing-organisation" } + + assert_redirected_to admin_republishing_index_path + assert_equal "The 'An Existing Organisation' organisation has been scheduled for republishing", flash[:notice] + end + + test "GDS Admin users should see a 404 page when trying to POST :republish_organisation with a nonexistent organisation slug" do + Organisation.any_instance.expects(:publish_to_publishing_api).never + + get :republish_organisation, params: { organisation_slug: "not-an-existing-organisation" } + assert_response :not_found + end + + test "Non-GDS Admin users should not be able to POST :republish_organisation" do + create(:organisation, slug: "an-existing-organisation", name: "An Existing Organisation") + + Organisation.any_instance.expects(:publish_to_publishing_api).never + + login_as :writer + + post :republish_organisation, params: { organisation_slug: "an-existing-organisation" } + assert_response :forbidden + end end From 98b261efcb7c94fcddbaa4b4536f4b285b11477c Mon Sep 17 00:00:00 2001 From: Ynda Jas Date: Fri, 19 Apr 2024 15:35:39 +0100 Subject: [PATCH 07/11] Improve flash notice wording This is a little more grammatically conventional --- app/controllers/admin/republishing_controller.rb | 2 +- .../republishing_published_documents_steps.rb.rb | 2 +- test/functional/admin/republishing_controller_test.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/admin/republishing_controller.rb b/app/controllers/admin/republishing_controller.rb index fedf7292ace..7a8a15ca96c 100644 --- a/app/controllers/admin/republishing_controller.rb +++ b/app/controllers/admin/republishing_controller.rb @@ -20,7 +20,7 @@ def republish_page return render "admin/errors/not_found", status: :not_found unless page_to_republish PresentPageToPublishingApiWorker.perform_async(page_to_republish[:presenter]) - flash[:notice] = "'#{page_to_republish[:title]}' page has been scheduled for republishing" + flash[:notice] = "The '#{page_to_republish[:title]}' page has been scheduled for republishing" redirect_to(admin_republishing_index_path) end diff --git a/features/step_definitions/republishing_published_documents_steps.rb.rb b/features/step_definitions/republishing_published_documents_steps.rb.rb index 9df82a6ab55..ea9a4692329 100644 --- a/features/step_definitions/republishing_published_documents_steps.rb.rb +++ b/features/step_definitions/republishing_published_documents_steps.rb.rb @@ -9,5 +9,5 @@ end Then(/^I can see the "Past prime ministers" page has been scheduled for republishing/) do - expect(page).to have_selector(".gem-c-success-alert", text: "'Past Prime Ministers' page has been scheduled for republishing") + expect(page).to have_selector(".gem-c-success-alert", text: "The 'Past Prime Ministers' page has been scheduled for republishing") end diff --git a/test/functional/admin/republishing_controller_test.rb b/test/functional/admin/republishing_controller_test.rb index cc577bbc44f..559f63b28f0 100644 --- a/test/functional/admin/republishing_controller_test.rb +++ b/test/functional/admin/republishing_controller_test.rb @@ -49,7 +49,7 @@ class Admin::RepublishingControllerTest < ActionController::TestCase post :republish_page, params: { page_slug: "past-prime-ministers" } assert_redirected_to admin_republishing_index_path - assert_equal "'Past Prime Ministers' page has been scheduled for republishing", flash[:notice] + assert_equal "The 'Past Prime Ministers' page has been scheduled for republishing", flash[:notice] end test "GDS Admin users should see a 404 page when trying to POST :republish_page with an unregistered page slug" do From be4145f7fd6faf742167e4da21f428e82eee63eb Mon Sep 17 00:00:00 2001 From: Ynda Jas Date: Fri, 19 Apr 2024 15:31:11 +0100 Subject: [PATCH 08/11] Add feature test for republishing an organisation The prime minister role needs to exist for the republishing index page to render properly, so I've included a specific `Given` condition related to that --- features/republishing-documents.feature | 6 ++++++ ...publishing_published_documents_steps.rb.rb | 20 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/features/republishing-documents.feature b/features/republishing-documents.feature index 3ccba7ca17f..b347bf32bd3 100644 --- a/features/republishing-documents.feature +++ b/features/republishing-documents.feature @@ -11,3 +11,9 @@ Feature: Republishing published documents And the "Past prime ministers" page can be republished When I request a republish of the "Past prime ministers" page Then I can see the "Past prime ministers" page has been scheduled for republishing + + Scenario: Republish an organisation + Given a published organisation "An Existing Organisation" exists + And the "An Existing Organisation" organisation can be republished + When I request a republish of the "An Existing Organisation" organisation + Then I can see the "An Existing Organisation" organisation has been scheduled for republishing diff --git a/features/step_definitions/republishing_published_documents_steps.rb.rb b/features/step_definitions/republishing_published_documents_steps.rb.rb index ea9a4692329..74c76fa51f5 100644 --- a/features/step_definitions/republishing_published_documents_steps.rb.rb +++ b/features/step_definitions/republishing_published_documents_steps.rb.rb @@ -11,3 +11,23 @@ Then(/^I can see the "Past prime ministers" page has been scheduled for republishing/) do expect(page).to have_selector(".gem-c-success-alert", text: "The 'Past Prime Ministers' page has been scheduled for republishing") end + +Given(/^a published organisation "An Existing Organisation" exists$/) do + create(:organisation, name: "An Existing Organisation", slug: "an-existing-organisation") +end + +Given(/^the "An Existing Organisation" organisation can be republished$/) do + create(:ministerial_role, name: "Prime Minister", cabinet_member: true) +end + +When(/^I request a republish of the "An Existing Organisation" organisation$/) do + visit admin_republishing_index_path + find("#republish-organisation").click + fill_in "Enter the slug for the organisation", with: "an-existing-organisation" + click_button("Continue") + click_button("Confirm republishing") +end + +Then(/^I can see the "An Existing Organisation" organisation has been scheduled for republishing/) do + expect(page).to have_selector(".gem-c-success-alert", text: "The 'An Existing Organisation' organisation has been scheduled for republishing") +end From 8101afc5dc1f04a1efd5aa337f8e259def5dddcb Mon Sep 17 00:00:00 2001 From: Ynda Jas Date: Fri, 19 Apr 2024 19:42:13 +0100 Subject: [PATCH 09/11] Rename republishing feature test files These now match the language used elsewhere, with 'content' as the generic term for all the republishing entities: pages, documents, organisations, roles, and people --- ...epublishing-documents.feature => republishing-content.feature} | 0 ...hed_documents_steps.rb.rb => republishing_content_steps.rb.rb} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename features/{republishing-documents.feature => republishing-content.feature} (100%) rename features/step_definitions/{republishing_published_documents_steps.rb.rb => republishing_content_steps.rb.rb} (100%) diff --git a/features/republishing-documents.feature b/features/republishing-content.feature similarity index 100% rename from features/republishing-documents.feature rename to features/republishing-content.feature diff --git a/features/step_definitions/republishing_published_documents_steps.rb.rb b/features/step_definitions/republishing_content_steps.rb.rb similarity index 100% rename from features/step_definitions/republishing_published_documents_steps.rb.rb rename to features/step_definitions/republishing_content_steps.rb.rb From 3f8efabcc63ecb1b42f450bc5bde31c988e94aef Mon Sep 17 00:00:00 2001 From: Ynda Jas Date: Mon, 22 Apr 2024 13:58:33 +0100 Subject: [PATCH 10/11] Add UI for republishing other individual pages We already added the UI for republishing the "Past Prime Ministers" page. Since we wrote/refactored this in an DRY-ly extendible way, adding in other individual presenter-based pages is relatively straightforward This adds republishing flows for the remaining individual pages that have dedicated Rake tasks, with a minimal refactor of the `republishing_pages` method to keep it DRY In most cases, the `base_path` presenter method was already public, but a few needed bringing in line, since the controller needs to access this I've included a feature test for each page since each is presented to the user as a discrete action. They all use the same slightly abstracted steps under the hood --- .../admin/republishing_controller.rb | 26 +++++++---- .../embassies_index_presenter.rb | 4 +- .../ministers_index_presenter.rb | 8 ++-- features/republishing-content.feature | 46 +++++++++++++++++-- .../republishing_content_steps.rb.rb | 27 +++++++++-- 5 files changed, 87 insertions(+), 24 deletions(-) diff --git a/app/controllers/admin/republishing_controller.rb b/app/controllers/admin/republishing_controller.rb index 7a8a15ca96c..c0318b4b9d4 100644 --- a/app/controllers/admin/republishing_controller.rb +++ b/app/controllers/admin/republishing_controller.rb @@ -62,13 +62,23 @@ def enforce_permissions! end def republishable_pages - historical_accounts_index_presenter = PublishingApi::HistoricalAccountsIndexPresenter.new - - [{ - title: historical_accounts_index_presenter.content[:title], - public_path: historical_accounts_index_presenter.base_path, - slug: historical_accounts_index_presenter.base_path.split("/").last, - presenter: "PublishingApi::HistoricalAccountsIndexPresenter", - }] + [ + "PublishingApi::HistoricalAccountsIndexPresenter", + "PublishingApi::HowGovernmentWorksPresenter", + "PublishingApi::OperationalFieldsIndexPresenter", + "PublishingApi::MinistersIndexPresenter", + "PublishingApi::EmbassiesIndexPresenter", + "PublishingApi::WorldIndexPresenter", + "PublishingApi::OrganisationsIndexPresenter", + ].map do |presenter_class_string| + presenter_instance = presenter_class_string.constantize.new + + { + title: presenter_instance.content[:title], + public_path: presenter_instance.base_path, + slug: presenter_instance.base_path.split("/").last, + presenter: presenter_class_string, + } + end end end diff --git a/app/presenters/publishing_api/embassies_index_presenter.rb b/app/presenters/publishing_api/embassies_index_presenter.rb index 5d898b752bf..4fcaf0c4a0e 100644 --- a/app/presenters/publishing_api/embassies_index_presenter.rb +++ b/app/presenters/publishing_api/embassies_index_presenter.rb @@ -29,12 +29,12 @@ def links { parent: [WORLD_INDEX_CONTENT_ID] } end - private - def base_path "/world/embassies" end + private + def details { world_locations: world_locations.map do |embassy| diff --git a/app/presenters/publishing_api/ministers_index_presenter.rb b/app/presenters/publishing_api/ministers_index_presenter.rb index c6e2c9774f0..d7ef4744aca 100644 --- a/app/presenters/publishing_api/ministers_index_presenter.rb +++ b/app/presenters/publishing_api/ministers_index_presenter.rb @@ -46,6 +46,10 @@ def links } end + def base_path + "/government/ministers" + end + private def details @@ -60,10 +64,6 @@ def details end end - def base_path - "/government/ministers" - end - def reshuffle_in_progress? SitewideSetting.find_by(key: :minister_reshuffle_mode)&.on || false end diff --git a/features/republishing-content.feature b/features/republishing-content.feature index b347bf32bd3..7fe7f832c59 100644 --- a/features/republishing-content.feature +++ b/features/republishing-content.feature @@ -6,11 +6,47 @@ Feature: Republishing published documents Background: Given I am a GDS admin - Scenario: Republish the "Past prime ministers" page - Given a published publication "Past prime ministers" exists - And the "Past prime ministers" page can be republished - When I request a republish of the "Past prime ministers" page - Then I can see the "Past prime ministers" page has been scheduled for republishing + Scenario: Republish the "Past Prime Ministers" page + Given a published publication "Past Prime Ministers" exists + And the "Past Prime Ministers" page can be republished + When I request a republish of the "Past Prime Ministers" page + Then I can see the "Past Prime Ministers" page has been scheduled for republishing + + Scenario: Republish the "How government works" page + Given a published publication "How government works" exists + And the "How government works" page can be republished + When I request a republish of the "How government works" page + Then I can see the "How government works" page has been scheduled for republishing + + Scenario: Republish the "Fields of operation" page + Given a published publication "Fields of operation" exists + And the "Fields of operation" page can be republished + When I request a republish of the "Fields of operation" page + Then I can see the "Fields of operation" page has been scheduled for republishing + + Scenario: Republish the "Ministers" page + Given a published publication "Ministers" exists + And the "Ministers" page can be republished + When I request a republish of the "Ministers" page + Then I can see the "Ministers" page has been scheduled for republishing + + Scenario: Republish the "Find a British embassy, high commission or consulate" page + Given a published publication "Find a British embassy, high commission or consulate" exists + And the "Find a British embassy, high commission or consulate" page can be republished + When I request a republish of the "Find a British embassy, high commission or consulate" page + Then I can see the "Find a British embassy, high commission or consulate" page has been scheduled for republishing + + Scenario: Republish the "Help and services around the world" page + Given a published publication "Help and services around the world" exists + And the "Help and services around the world" page can be republished + When I request a republish of the "Help and services around the world" page + Then I can see the "Help and services around the world" page has been scheduled for republishing + + Scenario: Republish the "Departments, agencies and public bodies" page + Given a published publication "Departments, agencies and public bodies" exists + And the "Departments, agencies and public bodies" page can be republished + When I request a republish of the "Departments, agencies and public bodies" page + Then I can see the "Departments, agencies and public bodies" page has been scheduled for republishing Scenario: Republish an organisation Given a published organisation "An Existing Organisation" exists diff --git a/features/step_definitions/republishing_content_steps.rb.rb b/features/step_definitions/republishing_content_steps.rb.rb index 74c76fa51f5..0912cd86f1b 100644 --- a/features/step_definitions/republishing_content_steps.rb.rb +++ b/features/step_definitions/republishing_content_steps.rb.rb @@ -1,15 +1,15 @@ -Given(/^the "Past prime ministers" page can be republished$/) do +Given(/^the "([^"]*)" page can be republished$/) do |_page_title| create(:ministerial_role, name: "Prime Minister", cabinet_member: true) end -When(/^I request a republish of the "Past prime ministers" page$/) do +When(/^I request a republish of the "([^"]*)" page$/) do |page_title| visit admin_republishing_index_path - find("#republish-past-prime-ministers").click + find(republishing_link_id_from_page_title(page_title)).click click_button("Confirm republishing") end -Then(/^I can see the "Past prime ministers" page has been scheduled for republishing/) do - expect(page).to have_selector(".gem-c-success-alert", text: "The 'Past Prime Ministers' page has been scheduled for republishing") +Then(/^I can see the "([^"]*)" page has been scheduled for republishing/) do |page_title| + expect(page).to have_selector(".gem-c-success-alert", text: "The '#{page_title}' page has been scheduled for republishing") end Given(/^a published organisation "An Existing Organisation" exists$/) do @@ -31,3 +31,20 @@ Then(/^I can see the "An Existing Organisation" organisation has been scheduled for republishing/) do expect(page).to have_selector(".gem-c-success-alert", text: "The 'An Existing Organisation' organisation has been scheduled for republishing") end + +def republishing_link_id_from_page_title(page_title) + link_id = "#republish-" + + link_id += case page_title + when "Find a British embassy, high commission or consulate" + "embassies" + when "Help and services around the world" + "world" + when "Departments, agencies and public bodies" + "organisations" + else + page_title.downcase.gsub(" ", "-") + end + + link_id +end From 6194d1e4042a8f4b8883112641f75d6b6e95665c Mon Sep 17 00:00:00 2001 From: Ynda Jas Date: Mon, 22 Apr 2024 13:59:24 +0100 Subject: [PATCH 11/11] Improve screen reader messaging This improves the grammar of the screen reader message for the page republishing action links --- app/views/admin/republishing/index.html.erb | 2 +- test/functional/admin/republishing_controller_test.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/admin/republishing/index.html.erb b/app/views/admin/republishing/index.html.erb index 869cf62fee3..387c4239411 100644 --- a/app/views/admin/republishing/index.html.erb +++ b/app/views/admin/republishing/index.html.erb @@ -48,7 +48,7 @@ text: link_to(page[:title], Plek.website_root + page[:public_path], class:"govuk-link"), }, { - text: link_to(sanitize("Republish #{tag.span('\'' + page[:title] + '\' page', class: 'govuk-visually-hidden')}"), + text: link_to(sanitize("Republish #{tag.span('the \'' + page[:title] + '\' page', class: 'govuk-visually-hidden')}"), admin_republishing_page_confirm_path(page[:slug]), id: "republish-" + page[:slug], class: "govuk-link", diff --git a/test/functional/admin/republishing_controller_test.rb b/test/functional/admin/republishing_controller_test.rb index 559f63b28f0..0cf2262bb6c 100644 --- a/test/functional/admin/republishing_controller_test.rb +++ b/test/functional/admin/republishing_controller_test.rb @@ -12,7 +12,7 @@ class Admin::RepublishingControllerTest < ActionController::TestCase get :index assert_select ".govuk-table:nth-of-type(1) .govuk-table__cell:nth-child(1) a[href='https://www.test.gov.uk/government/history/past-prime-ministers']", text: "Past Prime Ministers" - assert_select ".govuk-table:nth-of-type(1) .govuk-table__cell:nth-child(2) a[href='/government/admin/republishing/page/past-prime-ministers/confirm']", text: "Republish 'Past Prime Ministers' page" + assert_select ".govuk-table:nth-of-type(1) .govuk-table__cell:nth-child(2) a[href='/government/admin/republishing/page/past-prime-ministers/confirm']", text: "Republish the 'Past Prime Ministers' page" assert_select ".govuk-table:nth-of-type(2) .govuk-table__cell:nth-child(2) a[href='/government/admin/republishing/organisation/find']", text: "Republish an organisation"