Merge pull request #9832 from alphagov/content-modelling/829-change-b…

…utton-copy-on-review-screen

(829) Change button copy on review screen

lib/engines/content_block_manager/app/controllers/concerns/can_schedule_or_publish.rb

@@ -1,11 +1,15 @@
 module CanScheduleOrPublish
   extend ActiveSupport::Concern
 
+  def self.included(base)
+    base.helper_method :is_scheduling?
+  end
+
   def schedule_or_publish
     @schema = ContentBlockManager::ContentBlock::Schema.find_by_block_type(@content_block_edition.document.block_type)
 
-    if params[:schedule_publishing] == "schedule"
-      ContentBlockManager::ScheduleEditionService.new(@schema).call(@content_block_edition, scheduled_publication_params)
+    if is_scheduling?
+      ContentBlockManager::ScheduleEditionService.new(@schema).call(@content_block_edition)
     else
       publish and return
     end
@@ -21,15 +25,22 @@ def publish
   end
 
   def validate_scheduled_edition
-    if params[:schedule_publishing].blank?
-      @content_block_edition.errors.add(:schedule_publishing, t("activerecord.errors.models.content_block_manager/content_block/edition.attributes.schedule_publishing.blank"))
-      raise ActiveRecord::RecordInvalid, @content_block_edition
-    elsif params[:schedule_publishing] == "schedule"
+    case params[:schedule_publishing]
+    when "schedule"
       validate_scheduled_publication_params
 
-      @content_block_edition.assign_attributes(scheduled_publication_params)
-      @content_block_edition.assign_attributes(state: "scheduled")
-      raise ActiveRecord::RecordInvalid, @content_block_edition unless @content_block_edition.valid?
+      @content_block_edition.update!(scheduled_publication_params)
+      if @content_block_edition.valid?(:scheduling)
+        @content_block_edition.save!
+      else
+        raise ActiveRecord::RecordInvalid, @content_block_edition
+      end
+    when "now"
+      @content_block_edition.update!(scheduled_publication: nil, state: "draft")
+      ContentBlockManager::SchedulePublishingWorker.dequeue(@content_block_edition)
+    else
+      @content_block_edition.errors.add(:schedule_publishing, t("activerecord.errors.models.content_block_manager/content_block/edition.attributes.schedule_publishing.blank"))
+      raise ActiveRecord::RecordInvalid, @content_block_edition
     end
   end
 
@@ -62,4 +73,15 @@ def scheduled_publication_date_params
       scheduled_publication_params["scheduled_publication(3i)"],
     ]
   end
+
+  def review_update_url
+    content_block_manager.content_block_manager_content_block_workflow_path(
+      @content_block_edition,
+      step: ContentBlockManager::ContentBlock::Editions::WorkflowController::UPDATE_BLOCK_STEPS[:review_update],
+    )
+  end
+
+  def is_scheduling?
+    @content_block_edition.scheduled_publication.present?
+  end
 end

lib/engines/content_block_manager/app/controllers/content_block_manager/base_controller.rb

@@ -50,16 +50,4 @@ def support_url
   def prepend_views
     prepend_view_path Rails.root.join("lib/engines/content_block_manager/app/views")
   end
-
-  def review_update_url
-    schedule_publishing = params[:schedule_publishing]
-    scheduled_at = scheduled_publication_params.to_h
-
-    content_block_manager.content_block_manager_content_block_workflow_path(
-      @content_block_edition,
-      step: ContentBlockManager::ContentBlock::Editions::WorkflowController::UPDATE_BLOCK_STEPS[:review_update],
-      schedule_publishing:,
-      scheduled_at:,
-    )
-  end
 end

lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/documents/schedule_controller.rb

@@ -8,12 +8,15 @@ def edit
 
   def update
     document = ContentBlockManager::ContentBlock::Document.find(params[:document_id])
-    @content_block_edition = document.latest_edition
-    validate_scheduled_edition
+    previous_edition = document.latest_edition
+    @content_block_edition = previous_edition.dup
+    @content_block_edition.state = "draft"
+    @content_block_edition.organisation = previous_edition.lead_organisation
+    @content_block_edition.creator = current_user
 
-    @url = review_update_url
+    validate_scheduled_edition
 
-    render "content_block_manager/content_block/editions/workflow/review"
+    redirect_to content_block_manager.content_block_manager_content_block_workflow_path(@content_block_edition, step: :review_update)
   rescue ActiveRecord::RecordInvalid
     render "content_block_manager/content_block/documents/schedule/edit"
   end

lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/workflow_controller.rb

@@ -30,6 +30,8 @@ def show
       schedule_publishing
     when NEW_BLOCK_STEPS[:review]
       review
+    when UPDATE_BLOCK_STEPS[:review_update]
+      review_update
     when SHARED_STEPS[:confirmation]
       confirmation
     end
@@ -44,7 +46,7 @@ def update
     when UPDATE_BLOCK_STEPS[:review_links]
       redirect_to content_block_manager.content_block_manager_content_block_workflow_path(id: @content_block_edition.id, step: :schedule_publishing)
     when UPDATE_BLOCK_STEPS[:schedule_publishing]
-      review_update
+      validate_schedule
     when UPDATE_BLOCK_STEPS[:review_update]
       validate_review_page("review_update")
     when NEW_BLOCK_STEPS[:review]
@@ -84,11 +86,19 @@ def review_url
     )
   end
 
-  def review_update
+  def validate_schedule
     @content_block_edition = ContentBlockManager::ContentBlock::Edition.find(params[:id])
 
     validate_scheduled_edition
 
+    redirect_to content_block_manager.content_block_manager_content_block_workflow_path(@content_block_edition, step: :review_update)
+  rescue ActiveRecord::RecordInvalid
+    render "content_block_manager/content_block/editions/workflow/schedule_publishing"
+  end
+
+  def review_update
+    @content_block_edition = ContentBlockManager::ContentBlock::Edition.find(params[:id])
+
     @url = review_update_url
 
     render :review

lib/engines/content_block_manager/app/models/content_block_manager/content_block/edition/workflow.rb

@@ -14,7 +14,7 @@ def valid_state?(state)
 
       date_attributes :scheduled_publication
 
-      validates_with ContentBlockManager::ScheduledPublicationValidator
+      validates_with ContentBlockManager::ScheduledPublicationValidator, if: -> { validation_context == :scheduling || state == "scheduled" }
 
       state_machine auto_scopes: true do
         state :draft

lib/engines/content_block_manager/app/services/content_block_manager/concerns/dequeueable.rb

@@ -11,10 +11,6 @@ def dequeue_all_previously_queued_editions(content_block_edition)
           edition.supersede!
         end
       end
-
-      def dequeue_current_edition_if_previously_scheduled(content_block_edition)
-        ContentBlockManager::SchedulePublishingWorker.dequeue(content_block_edition) if content_block_edition.scheduled?
-      end
     end
   end
 end

lib/engines/content_block_manager/app/services/content_block_manager/publish_edition_service.rb

@@ -29,7 +29,6 @@ def publish_with_rollback(content_block_edition)
           ],
         },
       )
-      dequeue_current_edition_if_previously_scheduled(content_block_edition)
       dequeue_all_previously_queued_editions(content_block_edition)
       publish_publishing_api_edition(content_id:)
       update_content_block_document_with_latest_edition(content_block_edition)

lib/engines/content_block_manager/app/services/content_block_manager/schedule_edition_service.rb

@@ -6,9 +6,8 @@ def initialize(schema)
       @schema = schema
     end
 
-    def call(edition, scheduled_publication_params)
+    def call(edition)
       schedule_with_rollback do
-        edition.update!(scheduled_publication_params)
         edition.update_document_reference_to_latest_edition!
         edition
       end
@@ -24,7 +23,6 @@ def schedule_with_rollback
       ActiveRecord::Base.transaction do
         content_block_edition = yield
 
-        dequeue_current_edition_if_previously_scheduled(content_block_edition)
         content_block_edition.schedule! unless content_block_edition.scheduled?
 
         dequeue_all_previously_queued_editions(content_block_edition)

lib/engines/content_block_manager/app/validators/content_block_manager/scheduled_publication_validator.rb

@@ -2,12 +2,10 @@ class ContentBlockManager::ScheduledPublicationValidator < ActiveModel::Validato
   attr_reader :edition
 
   def validate(edition)
-    if edition.state == "scheduled"
-      if edition.scheduled_publication.blank?
-        edition.errors.add("scheduled_publication", :blank)
-      elsif edition.scheduled_publication < Time.zone.now
-        edition.errors.add("scheduled_publication", :future_date)
-      end
+    if edition.scheduled_publication.blank?
+      edition.errors.add("scheduled_publication", :blank)
+    elsif edition.scheduled_publication < Time.zone.now
+      edition.errors.add("scheduled_publication", :future_date)
     end
   end
 end

lib/engines/content_block_manager/app/views/content_block_manager/content_block/editions/workflow/review.html.erb

@@ -48,9 +48,8 @@
       <div class="govuk-button-group govuk-!-margin-bottom-6">
         <div>
           <%= render "govuk_publishing_components/components/button", {
-            text: "Confirm",
+            text: is_scheduling? ? "Schedule" : "Publish",
             name: "confirm",
-            value: "Confirm",
             type: "submit",
           } %>
       <% end %>

lib/engines/content_block_manager/app/views/content_block_manager/content_block/editions/workflow/schedule_publishing.html.erb

@@ -4,11 +4,11 @@
       params:,
       context:,
       back_link: content_block_manager.content_block_manager_content_block_workflow_path(
-        edition_id: @content_block_edition.id,
+        @content_block_edition,
         step: ContentBlockManager::ContentBlock::Editions::WorkflowController::UPDATE_BLOCK_STEPS[:review_links],
       ),
       form_url: content_block_manager.content_block_manager_content_block_workflow_path(
-        edition_id: @content_block_edition.id,
+        @content_block_edition,
         step: ContentBlockManager::ContentBlock::Editions::WorkflowController::UPDATE_BLOCK_STEPS[:schedule_publishing],
       ),
       is_rescheduling: false,

lib/engines/content_block_manager/features/create_object.feature

@@ -24,8 +24,7 @@ Feature: Create a content object
       | title            | email_address   | department | organisation        | instructions_to_publishers |
       | my email address | foo@example.com | Somewhere  | Ministry of Example | this is important  |
     Then I am asked to review my answers
-    And I confirm my answers are correct
-    When I click confirm
+    And I review and confirm my answers are correct
     Then the edition should have been created successfully
     And I should be taken to the confirmation page for a new block
 
@@ -63,7 +62,7 @@ Feature: Create a content object
       | title            | email_address   | department | organisation |
       | my email address | foo@example.com           | Somewhere  | Ministry of Example |
     Then I am asked to review my answers
-    When I click confirm
+    When I click publish without confirming my details
     Then I should see a message that I need to confirm the details are correct
 
   Scenario: GDS editor does not see error when not providing instructions to publishers
@@ -101,8 +100,7 @@ Feature: Create a content object
       | my email address 2 |
     Then I am asked to review my answers
     And I confirm my answers are correct
-    When I click confirm
-    Then the edition should have been created successfully
+    And I review and confirm my answers are correct
     And I should be taken to the confirmation page for a new block
 
   Scenario: Draft documents are not listed

lib/engines/content_block_manager/features/edit_object.feature

@@ -104,7 +104,7 @@ Feature: Edit a content object
     And I choose to publish the change now
     When I save and continue
     Then I am asked to review my answers
-    When I click confirm
+    When I click publish without confirming my details
     Then I should see a message that I need to confirm the details are correct
 
   @enable-sidekiq-test-mode

lib/engines/content_block_manager/features/reschedule_object.feature

@@ -58,3 +58,16 @@ Feature: Schedule a content object
     And I click the cancel link
     Then I should be taken back to the document page
     And no draft Content Block Edition has been created
+
+  @disable-sidekiq-test-mode
+  Scenario: GDS editor cancels the rescheduling of an object on the confirmation page
+    When I am updating a content block
+    Then I am asked when I want to publish the change
+    And I schedule the change for 7 days in the future
+    When I review and confirm my answers are correct
+    When I click to view the content block
+    And I click to edit the schedule
+    And I schedule the change for 5 days in the future
+    And I click cancel
+    Then I am taken back to Content Block Manager home page
+    And no draft Content Block Edition has been created

lib/engines/content_block_manager/features/step_definitions/content_block_manager_steps.rb

@@ -306,6 +306,10 @@ def has_support_button
   assert_equal current_path, content_block_manager.content_block_manager_root_path
 end
 
+Then("I am taken back to the view page of the content block") do
+  assert_equal current_path, content_block_manager.content_block_manager_content_block_document_path(@content_block.document)
+end
+
 And("no draft Content Block Edition has been created") do
   assert_equal 0, ContentBlockManager::ContentBlock::Edition.where(state: "draft").count
 end
@@ -509,7 +513,11 @@ def should_show_edit_form_for_email_address_content_block(document_title, email_
 When("I review and confirm my answers are correct") do
   assert_text "Review email address"
   check "By creating this content block you are confirming that, to the best of your knowledge, the details you are providing are correct."
-  click_on "Confirm"
+  click_on @is_scheduled ? "Schedule" : "Publish"
+end
+
+When("I click publish without confirming my details") do
+  click_on "Publish"
 end
 
 When(/^dependent content exists for a content block$/) do
@@ -660,6 +668,7 @@ def should_show_edit_form_for_email_address_content_block(document_title, email_
 end
 
 Then(/^I choose to publish the change now$/) do
+  @is_scheduled = false
   choose "Publish the edit now"
 end
 
@@ -697,6 +706,7 @@ def should_show_edit_form_for_email_address_content_block(document_title, email_
 And(/^I schedule the change for (\d+) days in the future$/) do |number_of_days|
   choose "Schedule the edit for the future"
   @future_date = number_of_days.days.since(Time.zone.now)
+  @is_scheduled = true
   fill_in_date_and_time_field(@future_date)
 
   click_on "Save and continue"
@@ -749,6 +759,7 @@ def should_show_edit_form_for_email_address_content_block(document_title, email_
 end
 
 And("the block is scheduled and published") do
+  @is_scheduled = true
   create(:scheduled_publishing_robot)
   near_future_date = 1.minute.from_now
   fill_in_date_and_time_field(near_future_date)

lib/engines/content_block_manager/test/integration/content_block/workflow_test.rb

@@ -96,7 +96,7 @@ class ContentBlockManager::ContentBlock::WorkflowTest < ActionDispatch::Integrat
 
       describe "#update" do
         describe "when choosing to publish immediately" do
-          it "shows the review page" do
+          it "redirects to the review step" do
             scheduled_at = {
               "scheduled_publication(1i)": "",
               "scheduled_publication(2i)": "",
@@ -111,12 +111,12 @@ class ContentBlockManager::ContentBlock::WorkflowTest < ActionDispatch::Integrat
                   scheduled_at:,
                 }
 
-            assert_template "content_block_manager/content_block/editions/workflow/review"
+            assert_redirected_to content_block_manager_content_block_workflow_path(id: edition.id, step: :review_update)
           end
         end
 
         describe "when scheduling publication" do
-          it "shows the review page" do
+          it "redirects to the review step" do
             scheduled_at = {
               "scheduled_publication(1i)": "2024",
               "scheduled_publication(2i)": "01",
@@ -130,7 +130,7 @@ class ContentBlockManager::ContentBlock::WorkflowTest < ActionDispatch::Integrat
               scheduled_at:,
             }
 
-            assert_template "content_block_manager/content_block/editions/workflow/review"
+            assert_redirected_to content_block_manager_content_block_workflow_path(id: edition.id, step: :review_update)
           end
         end
 

lib/engines/content_block_manager/test/unit/app/controllers/concerns/can_schedule_or_publish_test.rb

@@ -1,6 +1,13 @@
 require "test_helper"
 
 class EditionFormTestClass
+  class << self
+    def helper_method(method)
+      @helper_methods ||= []
+      @helper_methods << method
+    end
+  end
+
   include CanScheduleOrPublish
   include I18n::Base