From 86f7201dec3c063cdfb9dd35821ba18f38eeef70 Mon Sep 17 00:00:00 2001 From: Ynda Jas Date: Thu, 30 May 2024 19:05:57 +0100 Subject: [PATCH] Rename about us pages bulk content type This is specific to published about us pages, per the definition of `about_us` from `HasCorporateInformationPages` A regular migration is used here to ensure that the data is updated at the same time as the enum changing, and to allow the change to be made automatically during the deployment process --- app/helpers/admin/republishing_helper.rb | 12 ++++++------ app/models/republishing_event.rb | 2 +- app/services/bulk_republisher.rb | 2 +- ...sation_about_us_pages_bulk_content_type.rb | 19 +++++++++++++++++++ db/schema.rb | 2 +- features/bulk-republishing-content.feature | 8 ++++---- .../bulk_republishing_content_steps.rb | 10 +++++----- .../bulk_republishing_controller_test.rb | 18 +++++++++--------- .../admin/republishing_controller_test.rb | 2 +- .../app/services/bulk_republisher_test.rb | 17 ++++++++++++++--- 10 files changed, 61 insertions(+), 31 deletions(-) create mode 100644 db/migrate/20240530154757_rename_all_organisation_about_us_pages_bulk_content_type.rb diff --git a/app/helpers/admin/republishing_helper.rb b/app/helpers/admin/republishing_helper.rb index f0a3076cb2b..6715436d889 100644 --- a/app/helpers/admin/republishing_helper.rb +++ b/app/helpers/admin/republishing_helper.rb @@ -17,12 +17,12 @@ def bulk_content_type_metadata confirmation_path: admin_bulk_republishing_all_confirm_path("all-documents-with-pre-publication-editions"), republish_method: -> { BulkRepublisher.new.republish_all_documents_with_pre_publication_editions }, }, - all_organisation_about_us_pages: { - id: "all-organisation-about-us-pages", - name: "all organisation 'About Us' pages", - republishing_path: admin_bulk_republishing_all_republish_path("all-organisation-about-us-pages"), - confirmation_path: admin_bulk_republishing_all_confirm_path("all-organisation-about-us-pages"), - republish_method: -> { BulkRepublisher.new.republish_all_organisation_about_us_pages }, + all_published_organisation_about_us_pages: { + id: "all-published-organisation-about-us-pages", + name: "all published organisation 'About us' pages", + republishing_path: admin_bulk_republishing_all_republish_path("all-published-organisation-about-us-pages"), + confirmation_path: admin_bulk_republishing_all_confirm_path("all-published-organisation-about-us-pages"), + republish_method: -> { BulkRepublisher.new.republish_all_published_organisation_about_us_pages }, }, } end diff --git a/app/models/republishing_event.rb b/app/models/republishing_event.rb index adfd148a8be..5adf36bf08d 100644 --- a/app/models/republishing_event.rb +++ b/app/models/republishing_event.rb @@ -10,7 +10,7 @@ class RepublishingEvent < ApplicationRecord enum :bulk_content_type, %i[ all_documents - all_organisation_about_us_pages all_documents_with_pre_publication_editions + all_published_organisation_about_us_pages ] end diff --git a/app/services/bulk_republisher.rb b/app/services/bulk_republisher.rb index a85a26109ac..4a045ce7b1c 100644 --- a/app/services/bulk_republisher.rb +++ b/app/services/bulk_republisher.rb @@ -1,5 +1,5 @@ class BulkRepublisher - def republish_all_organisation_about_us_pages + def republish_all_published_organisation_about_us_pages document_ids = Organisation.all.map(&:about_us).compact.pluck(:document_id) document_ids.each do |document_id| diff --git a/db/migrate/20240530154757_rename_all_organisation_about_us_pages_bulk_content_type.rb b/db/migrate/20240530154757_rename_all_organisation_about_us_pages_bulk_content_type.rb new file mode 100644 index 00000000000..419c1d49b00 --- /dev/null +++ b/db/migrate/20240530154757_rename_all_organisation_about_us_pages_bulk_content_type.rb @@ -0,0 +1,19 @@ +class RenameAllOrganisationAboutUsPagesBulkContentType < ActiveRecord::Migration[7.1] + def up + RepublishingEvent + .where(bulk_content_type: :all_organisation_about_us_pages) + .update_all( + bulk_content_type: :all_published_organisation_about_us_pages, + action: "All published organisation 'About us' pages have been queued for republishing", + ) + end + + def down + RepublishingEvent + .where(bulk_content_type: :all_published_organisation_about_us_pages) + .update_all( + bulk_content_type: :all_organisation_about_us_pages, + action: "All published organisation 'About Us' pages have been queued for republishing", + ) + end +end diff --git a/db/schema.rb b/db/schema.rb index 6ad156dff1c..92739744917 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_05_29_103123) do +ActiveRecord::Schema[7.1].define(version: 2024_05_30_154757) do create_table "assets", charset: "utf8mb3", force: :cascade do |t| t.string "asset_manager_id", null: false t.string "variant", null: false diff --git a/features/bulk-republishing-content.feature b/features/bulk-republishing-content.feature index 0a8a1f0f0f9..1e956b75555 100644 --- a/features/bulk-republishing-content.feature +++ b/features/bulk-republishing-content.feature @@ -16,7 +16,7 @@ Feature: Bulk republishing content When I request a bulk republishing of all documents with pre-publication editions Then I can see that all documents with pre-publication editions have been queued for republishing - Scenario: Republish all Organisation "About us" pages - Given Organisation "About us" pages exist - When I request a bulk republishing of the Organisation "About us" pages - Then I can see the Organisation "About us" pages have been queued for republishing + Scenario: Republish all published Organisation "About us" pages + Given Published organisation "About us" pages exist + When I request a bulk republishing of all published organisation "About us" pages + Then I can see all published organisation "About us" pages have been queued for republishing diff --git a/features/step_definitions/bulk_republishing_content_steps.rb b/features/step_definitions/bulk_republishing_content_steps.rb index df12ce881fb..67a78d30838 100644 --- a/features/step_definitions/bulk_republishing_content_steps.rb +++ b/features/step_definitions/bulk_republishing_content_steps.rb @@ -28,17 +28,17 @@ expect(page).to have_selector(".gem-c-success-alert", text: "All documents with pre-publication editions have been queued for republishing") end -Given(/^Organisation "About us" pages exist$/) do +Given(/^Published organisation "About us" pages exist$/) do 2.times { create(:about_corporate_information_page) } end -When(/^I request a bulk republishing of the Organisation "About us" pages$/) do +When(/^I request a bulk republishing of all published organisation "About us" pages$/) do visit admin_republishing_index_path - find("#all-organisation-about-us-pages").click + find("#all-published-organisation-about-us-pages").click fill_in "What is the reason for republishing?", with: "It needs republishing" click_button("Confirm republishing") end -Then(/^I can see the Organisation "About us" pages have been queued for republishing$/) do - expect(page).to have_selector(".gem-c-success-alert", text: "All organisation 'About us' pages have been queued for republishing") +Then(/^I can see all published organisation "About us" pages have been queued for republishing$/) do + expect(page).to have_selector(".gem-c-success-alert", text: "All published organisation 'About us' pages have been queued for republishing") end diff --git a/test/functional/admin/bulk_republishing_controller_test.rb b/test/functional/admin/bulk_republishing_controller_test.rb index a215809b5ac..bba3a7ad38d 100644 --- a/test/functional/admin/bulk_republishing_controller_test.rb +++ b/test/functional/admin/bulk_republishing_controller_test.rb @@ -8,7 +8,7 @@ class Admin::BulkRepublishingControllerTest < ActionController::TestCase should_be_an_admin_controller test "GDS Admin users should be able to GET :confirm_all with a valid parameterless bulk content type" do - get :confirm_all, params: { bulk_content_type: "all-organisation-about-us-pages" } + get :confirm_all, params: { bulk_content_type: "all-published-organisation-about-us-pages" } assert_response :ok end @@ -20,30 +20,30 @@ class Admin::BulkRepublishingControllerTest < ActionController::TestCase test "Non-GDS Admin users should not be able to GET :confirm_all" do login_as :writer - get :confirm_all, params: { bulk_content_type: "all-organisation-about-us-pages" } + get :confirm_all, params: { bulk_content_type: "all-published-organisation-about-us-pages" } assert_response :forbidden end test "GDS Admin users should be able to POST :republish_all with a valid bulk content type and a reason, creating a RepublishingEvent for the current user" do - BulkRepublisher.any_instance.expects(:republish_all_organisation_about_us_pages).once + BulkRepublisher.any_instance.expects(:republish_all_published_organisation_about_us_pages).once - post :republish_all, params: { bulk_content_type: "all-organisation-about-us-pages", reason: "this needs republishing" } + post :republish_all, params: { bulk_content_type: "all-published-organisation-about-us-pages", reason: "this needs republishing" } newly_created_event = RepublishingEvent.last assert_equal newly_created_event.user, current_user assert_equal newly_created_event.reason, "this needs republishing" - assert_equal newly_created_event.action, "All organisation 'About us' pages have been queued for republishing" + assert_equal newly_created_event.action, "All published organisation 'About us' pages have been queued for republishing" assert_equal newly_created_event.bulk, true - assert_equal newly_created_event.bulk_content_type, "all_organisation_about_us_pages" + assert_equal newly_created_event.bulk_content_type, "all_published_organisation_about_us_pages" assert_redirected_to admin_republishing_index_path - assert_equal "All organisation 'About us' pages have been queued for republishing", flash[:notice] + assert_equal "All published organisation 'About us' pages have been queued for republishing", flash[:notice] end test "GDS Admin users should encounter an error on POST :republish_all without a `reason` and be sent back to the confirm_all page" do BulkRepublisher.expects(:new).never - post :republish_all, params: { bulk_content_type: "all-organisation-about-us-pages", reason: "" } + post :republish_all, params: { bulk_content_type: "all-published-organisation-about-us-pages", reason: "" } assert_equal ["Reason can't be blank"], assigns(:republishing_event).errors.full_messages assert_template "confirm_all" @@ -61,7 +61,7 @@ class Admin::BulkRepublishingControllerTest < ActionController::TestCase login_as :writer - post :republish_all, params: { bulk_content_type: "all-organisation-about-us-pages", reason: "this needs republishing" } + post :republish_all, params: { bulk_content_type: "all-published-organisation-about-us-pages", reason: "this needs republishing" } assert_response :forbidden end end diff --git a/test/functional/admin/republishing_controller_test.rb b/test/functional/admin/republishing_controller_test.rb index 4a574a4c949..5e40d80fdf9 100644 --- a/test/functional/admin/republishing_controller_test.rb +++ b/test/functional/admin/republishing_controller_test.rb @@ -21,7 +21,7 @@ class Admin::RepublishingControllerTest < ActionController::TestCase assert_select ".govuk-table:nth-of-type(3) .govuk-table__body .govuk-table__row:nth-child(1) .govuk-table__cell:nth-child(2) a[href='/government/admin/republishing/bulk/all-documents/confirm']", text: "Republish all documents" assert_select ".govuk-table:nth-of-type(3) .govuk-table__body .govuk-table__row:nth-child(2) .govuk-table__cell:nth-child(2) a[href='/government/admin/republishing/bulk/all-documents-with-pre-publication-editions/confirm']", text: "Republish all documents with pre-publication editions" - assert_select ".govuk-table:nth-of-type(3) .govuk-table__body .govuk-table__row:nth-child(3) .govuk-table__cell:nth-child(2) a[href='/government/admin/republishing/bulk/all-organisation-about-us-pages/confirm']", text: "Republish all organisation 'About Us' pages" + assert_select ".govuk-table:nth-of-type(3) .govuk-table__body .govuk-table__row:nth-child(3) .govuk-table__cell:nth-child(2) a[href='/government/admin/republishing/bulk/all-published-organisation-about-us-pages/confirm']", text: "Republish all published organisation 'About us' pages" assert_response :ok end diff --git a/test/unit/app/services/bulk_republisher_test.rb b/test/unit/app/services/bulk_republisher_test.rb index ebb6fbcde7c..fca5cfd0a75 100644 --- a/test/unit/app/services/bulk_republisher_test.rb +++ b/test/unit/app/services/bulk_republisher_test.rb @@ -3,8 +3,8 @@ class BulkRepublisherTest < ActiveSupport::TestCase extend Minitest::Spec::DSL - describe "#republish_all_organisation_about_us_pages" do - test "queues Organisation 'About us' pages for republishing" do + describe "#republish_all_published_organisation_about_us_pages" do + test "queues all published organisation 'About us' pages for republishing" do queue_sequence = sequence("queue") 2.times do @@ -16,7 +16,18 @@ class BulkRepublisherTest < ActiveSupport::TestCase .in_sequence(queue_sequence) end - BulkRepublisher.new.republish_all_organisation_about_us_pages + BulkRepublisher.new.republish_all_published_organisation_about_us_pages + end + + test "doesn't queue draft organisation 'About us' pages for republishing" do + about_us_page = create(:draft_about_corporate_information_page) + + PublishingApiDocumentRepublishingWorker + .expects(:perform_async_in_queue) + .with("bulk_republishing", about_us_page.document_id, true) + .never + + BulkRepublisher.new.republish_all_published_organisation_about_us_pages end end