From ca588c706edc9863a90470bc2dc36025fa810ab5 Mon Sep 17 00:00:00 2001 From: Ynda Jas Date: Wed, 8 May 2024 17:32:41 +0100 Subject: [PATCH 1/3] Tidy up republishing messaging - Clarify which republishing tasks are scheduled vs performed immediately (between page loads) - Make structure of messages more consistent --- .../admin/republishing_controller.rb | 10 +++++----- .../republishing/confirm_document.html.erb | 2 +- .../republishing/confirm_organisation.html.erb | 2 +- .../admin/republishing/confirm_person.html.erb | 2 +- .../admin/republishing/confirm_role.html.erb | 4 ++-- app/views/admin/republishing/index.html.erb | 4 ++-- features/republishing-content.feature | 8 ++++---- .../republishing_content_steps.rb.rb | 18 +++++++++--------- .../admin/republishing_controller_test.rb | 10 +++++----- 9 files changed, 30 insertions(+), 30 deletions(-) diff --git a/app/controllers/admin/republishing_controller.rb b/app/controllers/admin/republishing_controller.rb index 318246a183e..056270e2cef 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] = "The '#{page_to_republish[:title]}' page has been scheduled for republishing" + flash[:notice] = "The page '#{page_to_republish[:title]}' has been scheduled for republishing" redirect_to(admin_republishing_index_path) end @@ -51,7 +51,7 @@ def republish_organisation end @organisation.publish_to_publishing_api - flash[:notice] = "The '#{@organisation.name}' organisation has been scheduled for republishing" + flash[:notice] = "The organisation '#{@organisation.name}' has been republished" redirect_to(admin_republishing_index_path) end @@ -82,7 +82,7 @@ def republish_person end @person.publish_to_publishing_api - flash[:notice] = "The '#{@person.name}' person has been scheduled for republishing" + flash[:notice] = "The person '#{@person.name}' has been republished" redirect_to(admin_republishing_index_path) end @@ -113,7 +113,7 @@ def republish_role end @role.publish_to_publishing_api - flash[:notice] = "The '#{@role.name}' role has been scheduled for republishing" + flash[:notice] = "The role '#{@role.name}' has been republished" redirect_to(admin_republishing_index_path) end @@ -144,7 +144,7 @@ def republish_document end PublishingApiDocumentRepublishingWorker.new.perform(@document.id) - flash[:notice] = "Editions for the document with slug '#{@document.slug}' have been scheduled for republishing" + flash[:notice] = "Editions for the document with slug '#{@document.slug}' have been republished" redirect_to(admin_republishing_index_path) end diff --git a/app/views/admin/republishing/confirm_document.html.erb b/app/views/admin/republishing/confirm_document.html.erb index fbd9fdaee84..fdc006f1a85 100644 --- a/app/views/admin/republishing/confirm_document.html.erb +++ b/app/views/admin/republishing/confirm_document.html.erb @@ -5,7 +5,7 @@

- This will schedule the following editions to be republished. + This will republish the following editions.

<%= render "govuk_publishing_components/components/table", { diff --git a/app/views/admin/republishing/confirm_organisation.html.erb b/app/views/admin/republishing/confirm_organisation.html.erb index 26bc9d2cb51..40936c5e5cd 100644 --- a/app/views/admin/republishing/confirm_organisation.html.erb +++ b/app/views/admin/republishing/confirm_organisation.html.erb @@ -5,7 +5,7 @@

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

<%= form_with(url: admin_republishing_organisation_republish_path(@organisation.slug), method: :post, data: { module: "prevent-multiple-form-submissions", diff --git a/app/views/admin/republishing/confirm_person.html.erb b/app/views/admin/republishing/confirm_person.html.erb index b75a31d8e46..4e38452f601 100644 --- a/app/views/admin/republishing/confirm_person.html.erb +++ b/app/views/admin/republishing/confirm_person.html.erb @@ -5,7 +5,7 @@

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

<%= form_with(url: admin_republishing_person_republish_path(@person.slug), method: :post, data: { module: "prevent-multiple-form-submissions", diff --git a/app/views/admin/republishing/confirm_role.html.erb b/app/views/admin/republishing/confirm_role.html.erb index afa384c280e..1f9378ea4ea 100644 --- a/app/views/admin/republishing/confirm_role.html.erb +++ b/app/views/admin/republishing/confirm_role.html.erb @@ -6,9 +6,9 @@

<% if @role.public_url %> - This will schedule the <%= link_to @role.name, @role.public_url, { class: "govuk-link" } %> role to be republished. + This will republish the role <%= link_to @role.name, @role.public_url, { class: "govuk-link" } %>. <% else %> - This will schedule the '<%= @role.name %>' role to be republished. + This will republish the role '<%= @role.name %>'. <% end %>

<%= form_with(url: admin_republishing_role_republish_path(@role.slug), method: :post, data: { diff --git a/app/views/admin/republishing/index.html.erb b/app/views/admin/republishing/index.html.erb index 21ceef8bf90..6053e1345ee 100644 --- a/app/views/admin/republishing/index.html.erb +++ b/app/views/admin/republishing/index.html.erb @@ -20,7 +20,7 @@

- The following actions will allow you to schedule the republishing of content that was originally published in this application. + The following actions will allow you to republish 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.

@@ -30,7 +30,7 @@

Individual pages

- 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. + You can schedule a selection of individual pages for republishing 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", { diff --git a/features/republishing-content.feature b/features/republishing-content.feature index 3ea794b4b23..5c0a8489b52 100644 --- a/features/republishing-content.feature +++ b/features/republishing-content.feature @@ -52,22 +52,22 @@ Feature: Republishing published documents 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 + Then I can see the "An Existing Organisation" organisation has been republished Scenario: Republish a person Given a published person "Existing Person" exists And the "Existing Person" person can be republished When I request a republish of the "Existing Person" person - Then I can see the "Existing Person" person has been scheduled for republishing + Then I can see the "Existing Person" person has been republished Scenario: Republish a role Given a published role "An Existing Role" exists And the "An Existing Role" role can be republished When I request a republish of the "An Existing Role" role - Then I can see the "An Existing Role" role has been scheduled for republishing + Then I can see the "An Existing Role" role has been republished Scenario: Republish a document Given a document with slug "an-existing-document" exists And the "an-existing-document" document's editions can be republished When I request a republish of the "an-existing-document" document's editions - Then I can see the "an-existing-document" document's editions have been scheduled for republishing + Then I can see the "an-existing-document" document's editions have been republished diff --git a/features/step_definitions/republishing_content_steps.rb.rb b/features/step_definitions/republishing_content_steps.rb.rb index 317d93ecb58..22cd959528c 100644 --- a/features/step_definitions/republishing_content_steps.rb.rb +++ b/features/step_definitions/republishing_content_steps.rb.rb @@ -9,7 +9,7 @@ end 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") + expect(page).to have_selector(".gem-c-success-alert", text: "The page '#{page_title}' has been scheduled for republishing") end Given(/^a published organisation "An Existing Organisation" exists$/) do @@ -28,8 +28,8 @@ 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") +Then(/^I can see the "An Existing Organisation" organisation has been republished/) do + expect(page).to have_selector(".gem-c-success-alert", text: "The organisation 'An Existing Organisation' has been republished") end Given(/^a published person "Existing Person" exists$/) do @@ -48,8 +48,8 @@ click_button("Confirm republishing") end -Then(/^I can see the "Existing Person" person has been scheduled for republishing/) do - expect(page).to have_selector(".gem-c-success-alert", text: "The 'Existing Person' person has been scheduled for republishing") +Then(/^I can see the "Existing Person" person has been republished/) do + expect(page).to have_selector(".gem-c-success-alert", text: "The person 'Existing Person' has been republished") end Given(/^a published role "An Existing Role" exists$/) do @@ -68,8 +68,8 @@ click_button("Confirm republishing") end -Then(/^I can see the "An Existing Role" role has been scheduled for republishing/) do - expect(page).to have_selector(".gem-c-success-alert", text: "The 'An Existing Role' role has been scheduled for republishing") +Then(/^I can see the "An Existing Role" role has been republished/) do + expect(page).to have_selector(".gem-c-success-alert", text: "The role 'An Existing Role' has been republished") end Given(/^a document with slug "an-existing-document" exists$/) do @@ -89,8 +89,8 @@ click_button("Confirm republishing") end -Then(/^I can see the "an-existing-document" document's editions have been scheduled for republishing/) do - expect(page).to have_selector(".gem-c-success-alert", text: "Editions for the document with slug 'an-existing-document' have been scheduled for republishing") +Then(/^I can see the "an-existing-document" document's editions have been republished/) do + expect(page).to have_selector(".gem-c-success-alert", text: "Editions for the document with slug 'an-existing-document' have been republished") end def republishing_link_id_from_page_title(page_title) diff --git a/test/functional/admin/republishing_controller_test.rb b/test/functional/admin/republishing_controller_test.rb index 83bb07c4087..ba31d087133 100644 --- a/test/functional/admin/republishing_controller_test.rb +++ b/test/functional/admin/republishing_controller_test.rb @@ -52,7 +52,7 @@ class Admin::RepublishingControllerTest < ActionController::TestCase post :republish_page, params: { page_slug: "past-prime-ministers" } assert_redirected_to admin_republishing_index_path - assert_equal "The 'Past Prime Ministers' page has been scheduled for republishing", flash[:notice] + assert_equal "The page 'Past Prime Ministers' 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 @@ -137,7 +137,7 @@ class Admin::RepublishingControllerTest < ActionController::TestCase 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] + assert_equal "The organisation 'An Existing Organisation' has been republished", flash[:notice] end test "GDS Admin users should see a 404 page when trying to POST :republish_organisation with a nonexistent organisation slug" do @@ -224,7 +224,7 @@ class Admin::RepublishingControllerTest < ActionController::TestCase post :republish_person, params: { person_slug: "existing-person" } assert_redirected_to admin_republishing_index_path - assert_equal "The 'Existing Person' person has been scheduled for republishing", flash[:notice] + assert_equal "The person 'Existing Person' has been republished", flash[:notice] end test "GDS Admin users should see a 404 page when trying to POST :republish_person with a nonexistent person slug" do @@ -311,7 +311,7 @@ class Admin::RepublishingControllerTest < ActionController::TestCase post :republish_role, params: { role_slug: "an-existing-role" } assert_redirected_to admin_republishing_index_path - assert_equal "The 'An Existing Role' role has been scheduled for republishing", flash[:notice] + assert_equal "The role 'An Existing Role' has been republished", flash[:notice] end test "GDS Admin users should see a 404 page when trying to POST :republish_role with a nonexistent role slug" do @@ -398,7 +398,7 @@ class Admin::RepublishingControllerTest < ActionController::TestCase post :republish_document, params: { document_slug: "an-existing-document" } assert_redirected_to admin_republishing_index_path - assert_equal "Editions for the document with slug 'an-existing-document' have been scheduled for republishing", flash[:notice] + assert_equal "Editions for the document with slug 'an-existing-document' have been republished", flash[:notice] end test "GDS Admin users should see a 404 page when trying to POST :republish_document with a nonexistent document slug" do From c87ec5cb9aee6d0b8a6eb11249158a9f1fb28d3a Mon Sep 17 00:00:00 2001 From: Ynda Jas Date: Wed, 8 May 2024 17:37:04 +0100 Subject: [PATCH 2/3] Make helper method private This isn't used outside this module --- app/helpers/admin/url_helper.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/helpers/admin/url_helper.rb b/app/helpers/admin/url_helper.rb index 016ac91ba4c..501104f95a6 100644 --- a/app/helpers/admin/url_helper.rb +++ b/app/helpers/admin/url_helper.rb @@ -74,6 +74,8 @@ def admin_header_link(name, path, path_matcher = nil, options = {}) end end +private + def active_link_class(path_matcher) request.path.match?(path_matcher) ? "active" : "" end From 6cee7c0964a368ae4f42d3ea8821106541233b89 Mon Sep 17 00:00:00 2001 From: Ynda Jas Date: Wed, 8 May 2024 17:42:18 +0100 Subject: [PATCH 3/3] Add "Republish content" to "More" page This is only visible to users with GDS admin permissions --- app/helpers/admin/url_helper.rb | 6 ++++++ app/views/admin/more/index.html.erb | 1 + test/functional/admin/more_controller_test.rb | 20 +++++++++++++++++++ 3 files changed, 27 insertions(+) diff --git a/app/helpers/admin/url_helper.rb b/app/helpers/admin/url_helper.rb index 501104f95a6..9cd8124de38 100644 --- a/app/helpers/admin/url_helper.rb +++ b/app/helpers/admin/url_helper.rb @@ -74,6 +74,12 @@ def admin_header_link(name, path, path_matcher = nil, options = {}) end end + def admin_republish_content_link + if can?(:administer, :republish_content) + admin_link "Republish content", admin_republishing_index_path + end + end + private def active_link_class(path_matcher) diff --git a/app/views/admin/more/index.html.erb b/app/views/admin/more/index.html.erb index c8e45f5b84c..35940dca812 100644 --- a/app/views/admin/more/index.html.erb +++ b/app/views/admin/more/index.html.erb @@ -19,6 +19,7 @@ admin_topical_events_link, admin_world_location_news_link, admin_worldwide_organisations_link, + admin_republish_content_link, ], } %>
diff --git a/test/functional/admin/more_controller_test.rb b/test/functional/admin/more_controller_test.rb index 9ebb1d48c1a..ba0e9d22056 100644 --- a/test/functional/admin/more_controller_test.rb +++ b/test/functional/admin/more_controller_test.rb @@ -84,4 +84,24 @@ class Admin::MoreControllerTest < ActionController::TestCase assert_select ".govuk-list" assert_select "a.govuk-link", text: "Emergency banner" end + + view_test "GET #index does not render Republish content option when the user is not a GDS Admin." do + organisation = create(:organisation, name: "cabinet-minister") + login_as(:writer, organisation) + + get :index + + assert_select ".govuk-list" + refute_select "a.govuk-link", text: "Republish content" + end + + view_test "GET #index renders Republish content option when the user is a GDS Admin." do + organisation = create(:organisation, name: "government-digital-service") + login_as(:gds_admin, organisation) + + get :index + + assert_select ".govuk-list" + assert_select "a.govuk-link", text: "Republish content" + end end