From 37f5fe2fb29d8a83f4bccbdc0fad8e7684c764b9 Mon Sep 17 00:00:00 2001 From: Harriet H-W Date: Tue, 3 Dec 2024 17:42:48 +0000 Subject: [PATCH 1/2] show errors if confirm not checked on review page Because this is not an active record error we have to have some custom behaviour here. Currently the review page is only shown for the create/new journey, so have checked we are on the 'new' step here. --- .../editions/workflow_controller.rb | 11 ++++++-- .../editions/workflow/review.html.erb | 28 ++++++++++++++++++- .../features/create_object.feature | 15 ++++++++++ .../content_block_manager_steps.rb | 8 ++++++ .../content_block/workflow_test.rb | 14 +++++++++- 5 files changed, 72 insertions(+), 4 deletions(-) diff --git a/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/workflow_controller.rb b/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/workflow_controller.rb index bac845ca42a..f080dbe3461 100644 --- a/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/workflow_controller.rb +++ b/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/workflow_controller.rb @@ -135,8 +135,15 @@ def schedule_or_publish render "content_block_manager/content_block/editions/workflow/schedule_publishing" end + REVIEW_ERROR = Data.define(:attribute, :full_message) def publish - new_edition = ContentBlockManager::PublishEditionService.new.call(@content_block_edition) - redirect_to content_block_manager.content_block_manager_content_block_workflow_path(id: new_edition.id, step: :confirmation) + if params[:step] == NEW_BLOCK_STEPS[:review] && params[:is_confirmed].blank? + @confirm_error_copy = "Confirm details are correct" + @error_summary_errors = [{ text: @confirm_error_copy, href: "#is_confirmed-0" }] + render "content_block_manager/content_block/editions/workflow/review" + else + new_edition = ContentBlockManager::PublishEditionService.new.call(@content_block_edition) + redirect_to content_block_manager.content_block_manager_content_block_workflow_path(id: new_edition.id, step: :confirmation) + end end end diff --git a/lib/engines/content_block_manager/app/views/content_block_manager/content_block/editions/workflow/review.html.erb b/lib/engines/content_block_manager/app/views/content_block_manager/content_block/editions/workflow/review.html.erb index 6ea2ee319af..29712d75023 100644 --- a/lib/engines/content_block_manager/app/views/content_block_manager/content_block/editions/workflow/review.html.erb +++ b/lib/engines/content_block_manager/app/views/content_block_manager/content_block/editions/workflow/review.html.erb @@ -6,6 +6,13 @@ } %> <% end %> +<% if @error_summary_errors %> + <%= render "govuk_publishing_components/components/error_summary", { + title: "There is a problem", + items: @error_summary_errors, + } %> +<% end %> +
<%= render( @@ -16,9 +23,28 @@
+<%= form_with(url: content_block_manager.content_block_manager_content_block_workflow_path(@content_block_edition, step: ContentBlockManager::ContentBlock::Editions::WorkflowController::NEW_BLOCK_STEPS[:review]), method: :put) do %> +
+
+ <%= render "govuk_publishing_components/components/checkboxes", { + name: "is_confirmed", + id: "is_confirmed", + heading: "Confirm", + visually_hide_heading: true, + no_hint_text: true, + error: @confirm_error_copy, + items: [ + { + label: "By creating this content block you are confirming that, to the best of your knowledge, the details you are providing are correct.", + value: true, + }, + ], + } %> +
+
+
- <%= form_with(url: content_block_manager.content_block_manager_content_block_workflow_path(@content_block_edition, step: ContentBlockManager::ContentBlock::Editions::WorkflowController::NEW_BLOCK_STEPS[:review]), method: :put) do %>
<%= render "govuk_publishing_components/components/button", { diff --git a/lib/engines/content_block_manager/features/create_object.feature b/lib/engines/content_block_manager/features/create_object.feature index 2d9cb82e02e..f160d18968a 100644 --- a/lib/engines/content_block_manager/features/create_object.feature +++ b/lib/engines/content_block_manager/features/create_object.feature @@ -24,6 +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 Then the edition should have been created successfully And I should be taken to the confirmation page for a new block @@ -52,6 +53,19 @@ Feature: Create a content object | my email address | xxxxx | Somewhere | Ministry of Example | Then I should see a message that the "email_address" field is an invalid "email" + Scenario: GDS editor sees validation errors for unconfirmed answers + When I visit the Content Block Manager home page + And I click to create an object + Then I should see all the schemas listed + When I click on the "email_address" schema + Then I should see a form for the schema + When I complete the form with the following fields: + | 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 + 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 When I visit the Content Block Manager home page And I click to create an object @@ -86,6 +100,7 @@ Feature: Create a content object | title | | 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 should be taken to the confirmation page for a new block diff --git a/lib/engines/content_block_manager/features/step_definitions/content_block_manager_steps.rb b/lib/engines/content_block_manager/features/step_definitions/content_block_manager_steps.rb index 081bf5f141e..373df8831ba 100644 --- a/lib/engines/content_block_manager/features/step_definitions/content_block_manager_steps.rb +++ b/lib/engines/content_block_manager/features/step_definitions/content_block_manager_steps.rb @@ -429,6 +429,10 @@ def should_show_edit_form_for_email_address_content_block(document_title, email_ assert_text "#{ContentBlockManager::ContentBlock::Edition.human_attribute_name("details_#{field_name}")} is an invalid #{format.titleize}" end +Then("I should see a message that I need to confirm the details are correct") do + assert_text "Confirm details are correct", minimum: 2 +end + Then("I should see a permissions error") do assert_text "Permissions error" end @@ -456,6 +460,10 @@ def should_show_edit_form_for_email_address_content_block(document_title, email_ assert_text "Review email address" end +Then("I confirm my answers are correct") do + check "By creating this content block you are confirming that, to the best of your knowledge, the details you are providing are correct." +end + Then("I accept and publish") do click_on "Accept and publish" end diff --git a/lib/engines/content_block_manager/test/integration/content_block/workflow_test.rb b/lib/engines/content_block_manager/test/integration/content_block/workflow_test.rb index 1e9025e6638..1d9b5c39ec1 100644 --- a/lib/engines/content_block_manager/test/integration/content_block/workflow_test.rb +++ b/lib/engines/content_block_manager/test/integration/content_block/workflow_test.rb @@ -44,11 +44,23 @@ class ContentBlockManager::ContentBlock::WorkflowTest < ActionDispatch::Integrat describe "#update" do it "posts the new edition to the Publishing API and marks edition as published" do assert_edition_is_published do - put content_block_manager.content_block_manager_content_block_workflow_path(id: edition.id, step:) + put content_block_manager.content_block_manager_content_block_workflow_path(id: edition.id, step:, is_confirmed: true) end end end end + + describe "when the edition details have not been confirmed" do + let(:step) { ContentBlockManager::ContentBlock::Editions::WorkflowController::NEW_BLOCK_STEPS[:review] } + + describe "#update" do + it "returns to the review page" do + put content_block_manager.content_block_manager_content_block_workflow_path(id: edition.id, step:) + + assert_template "content_block_manager/content_block/editions/workflow/review" + end + end + end end describe "when updating an existing content block" do From 1252dd291384d0604e4a31017b3344dc5c708ef3 Mon Sep 17 00:00:00 2001 From: Harriet H-W Date: Thu, 5 Dec 2024 11:24:29 +0000 Subject: [PATCH 2/2] remove date string formatting on schedule feature spec this was causing tests to be flakey because sometimes it would format with the space padded before time, sometimes without. https://gds.slack.com/archives/C0776B04EJU/p1733396809325189 --- .../features/step_definitions/content_block_manager_steps.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/engines/content_block_manager/features/step_definitions/content_block_manager_steps.rb b/lib/engines/content_block_manager/features/step_definitions/content_block_manager_steps.rb index 373df8831ba..3da073fb356 100644 --- a/lib/engines/content_block_manager/features/step_definitions/content_block_manager_steps.rb +++ b/lib/engines/content_block_manager/features/step_definitions/content_block_manager_steps.rb @@ -191,7 +191,7 @@ def has_support_button end When("I should be taken to the scheduled confirmation page") do - assert_text "Email address scheduled to publish on #{@future_date.strftime('%e %B %Y%l:%M%P').strip}" + assert_text "Email address scheduled to publish on" assert_text "You can now view the updated schedule of the content block." expect(page).to have_link(