Merge pull request #9096 from alphagov/tidy-up-bulk-republishing

Tidy up bulk republishing

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

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

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|

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

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

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

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

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

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

test/unit/app/models/corporate_information_page_test.rb

@@ -449,7 +449,7 @@ def self.should_be_invalid_without(type, attribute_name)
     corp_page.update_in_search_index
   end
 
-  test "re-indexes the organisation after the 'About Us' CIP is saved" do
+  test "re-indexes the organisation after the 'About us' CIP is saved" do
     org = create(:organisation, govuk_status: "live")
     corp_page = create(
       :corporate_information_page,
@@ -462,7 +462,7 @@ def self.should_be_invalid_without(type, attribute_name)
     corp_page.save!
   end
 
-  test "re-indexes the worldwide organisation after the 'About Us' CIP is saved" do
+  test "re-indexes the worldwide organisation after the 'About us' CIP is saved" do
     worldwide_org = create(:worldwide_organisation)
     about_us = create(
       :corporate_information_page,
@@ -495,7 +495,7 @@ def self.should_be_invalid_without(type, attribute_name)
     other_page.save!
   end
 
-  test "republishes the 'About Us' CIP if another CIP is saved" do
+  test "republishes the 'About us' CIP if another CIP is saved" do
     org = create(:organisation, govuk_status: "live")
     about_us = create(
       :corporate_information_page,
@@ -512,7 +512,7 @@ def self.should_be_invalid_without(type, attribute_name)
     other_page.touch
   end
 
-  test "does not republish the 'About Us' CIP if it is saved" do
+  test "does not republish the 'About us' CIP if it is saved" do
     org = create(:organisation, govuk_status: "live")
     about_us = create(
       :corporate_information_page,

test/unit/app/models/editionable_worldwide_organisation_test.rb

@@ -385,7 +385,7 @@ class EditionableWorldwideOrganisationTest < ActiveSupport::TestCase
     assert_equal :a_home_page_list_of_offices, world_organisation.__send__(:home_page_offices_list)
   end
 
-  test "#corporate_information_page_types does not return `About Us` pages" do
+  test "#corporate_information_page_types does not return `About us` pages" do
     organisation = build(:editionable_worldwide_organisation)
 
     assert_not_includes organisation.corporate_information_page_types.map(&:slug), "about"

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
 
@@ -39,13 +50,6 @@ class BulkRepublisherTest < ActiveSupport::TestCase
 
   describe "#republish_all_documents_with_pre_publication_editions" do
     test "queues all documents with pre-publication editions for republishing" do
-      document_without_pre_publication_edition = create(:document, editions: [build(:published_edition)])
-
-      PublishingApiDocumentRepublishingWorker
-        .expects(:perform_async_in_queue)
-        .with("bulk_republishing", document_without_pre_publication_edition.id, true)
-        .never
-
       queue_sequence = sequence("queue")
 
       2.times do
@@ -59,5 +63,16 @@ class BulkRepublisherTest < ActiveSupport::TestCase
 
       BulkRepublisher.new.republish_all_documents_with_pre_publication_editions
     end
+
+    test "doesn't queue documents without pre-publication editions for republishing" do
+      document = create(:document, editions: [build(:published_edition)])
+
+      PublishingApiDocumentRepublishingWorker
+        .expects(:perform_async_in_queue)
+        .with("bulk_republishing", document.id, true)
+        .never
+
+      BulkRepublisher.new.republish_all_documents_with_pre_publication_editions
+    end
   end
 end