Merge pull request #9798 from alphagov/content-modelling/754-update-e…

…rror-messaging-across-the-journey-in-design-and-code

(754) Update error messaging across the journey

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

@@ -19,4 +19,47 @@ 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)
   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"
+      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?
+    end
+  end
+
+  def validate_scheduled_publication_params
+    error_base = "activerecord.errors.models.content_block_manager/content_block/edition.attributes.scheduled_publication"
+    if scheduled_publication_params.values.all?(&:blank?)
+      @content_block_edition.errors.add(:scheduled_publication, t("#{error_base}.blank"))
+    elsif scheduled_publication_time_params.all?(&:blank?)
+      @content_block_edition.errors.add(:scheduled_publication, t("#{error_base}.time.blank"))
+    elsif scheduled_publication_date_params.all?(&:blank?)
+      @content_block_edition.errors.add(:scheduled_publication, t("#{error_base}.date.blank"))
+    elsif scheduled_publication_params.values.any?(&:blank?)
+      @content_block_edition.errors.add(:scheduled_publication, t("#{error_base}.invalid_date"))
+    end
+
+    raise ActiveRecord::RecordInvalid, @content_block_edition if @content_block_edition.errors.any?
+  end
+
+  def scheduled_publication_time_params
+    [
+      scheduled_publication_params["scheduled_publication(4i)"],
+      scheduled_publication_params["scheduled_publication(5i)"],
+    ]
+  end
+
+  def scheduled_publication_date_params
+    [
+      scheduled_publication_params["scheduled_publication(1i)"],
+      scheduled_publication_params["scheduled_publication(2i)"],
+      scheduled_publication_params["scheduled_publication(3i)"],
+    ]
+  end
 end

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

@@ -51,17 +51,6 @@ def prepend_views
     prepend_view_path Rails.root.join("lib/engines/content_block_manager/app/views")
   end
 
-  def validate_scheduled_edition
-    if params[:schedule_publishing].blank?
-      @content_block_edition.errors.add(:schedule_publishing, t("errors.messages.blank"))
-      raise ActiveRecord::RecordInvalid, @content_block_edition
-    elsif params[:schedule_publishing] == "schedule"
-      @content_block_edition.assign_attributes(scheduled_publication_params)
-      @content_block_edition.assign_attributes(state: "scheduled")
-      raise ActiveRecord::RecordInvalid unless @content_block_edition.valid?
-    end
-  end
-
   def review_update_url
     schedule_publishing = params[:schedule_publishing]
     scheduled_at = scheduled_publication_params.to_h

lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/documents_controller.rb

@@ -48,7 +48,7 @@ def new_document_options_redirect
     if params[:block_type].present?
       redirect_to content_block_manager.new_content_block_manager_content_block_edition_path(block_type: params.require(:block_type))
     else
-      redirect_to content_block_manager.new_content_block_manager_content_block_document_path, flash: { error: "You must select a block type" }
+      redirect_to content_block_manager.new_content_block_manager_content_block_document_path, flash: { error: I18n.t("activerecord.errors.models.content_block_manager/content_block/document.attributes.block_type.blank") }
     end
   end
 

lib/engines/content_block_manager/app/forms/content_block_manager/content_block/edition_form.rb

@@ -4,7 +4,7 @@
 class ContentBlockManager::ContentBlock::EditionForm
   include ContentBlockManager::Engine.routes.url_helpers
 
-  attr_reader :content_block_edition, :schema
+  attr_reader :schema
 
   def self.for(content_block_edition:, schema:)
     content_block_edition.document&.latest_edition_id ? Update.new(content_block_edition:, schema:) : Create.new(content_block_edition:, schema:)
@@ -15,6 +15,11 @@ def initialize(content_block_edition:, schema:)
     @schema = schema
   end
 
+  def content_block_edition
+    @content_block_edition.errors.delete("document.sluggable_string")
+    @content_block_edition
+  end
+
   def attributes
     @schema.fields.each_with_object({}) do |field, hash|
       hash[field] = nil

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

@@ -1,15 +1,15 @@
 module ContentBlockManager
   module ContentBlock
     class Edition < ApplicationRecord
+      validates :title, presence: true
+
       include Documentable
       include HasAuditTrail
       include HasAuthors
-      include HasLeadOrganisation
       include ValidatesDetails
+      include HasLeadOrganisation
       include Workflow
 
-      validates :title, presence: true
-
       scope :current_versions, lambda {
         joins(
           "LEFT JOIN content_block_documents document ON document.latest_edition_id = content_block_editions.id",

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

@@ -22,9 +22,7 @@ def add_blank_errors(error)
 
   def add_format_errors(error)
     key = error["data_pointer"].delete_prefix("/")
-    format = error["schema"]["format"]
-    message = format.present? ? "is an invalid #{format.humanize}" : "is invalid"
-    edition.errors.add("details_#{key}", :invalid, message:)
+    edition.errors.add("details_#{key}", :invalid)
   end
 
   def validate_with_schema(edition)

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

@@ -4,9 +4,9 @@ class ContentBlockManager::ScheduledPublicationValidator < ActiveModel::Validato
   def validate(edition)
     if edition.state == "scheduled"
       if edition.scheduled_publication.blank?
-        edition.errors.add("scheduled_publication", :blank, message: "date and time cannot be blank")
+        edition.errors.add("scheduled_publication", :blank)
       elsif edition.scheduled_publication < Time.zone.now
-        edition.errors.add("scheduled_publication", :future_date, message: "date and time must be in the future")
+        edition.errors.add("scheduled_publication", :future_date)
       end
     end
   end

lib/engines/content_block_manager/app/views/content_block_manager/content_block/shared/_form.html.erb

@@ -1,21 +1,22 @@
-<% content_for :error_summary, render(Admin::ErrorSummaryComponent.new(object: @form.content_block_edition)) %>
+<% parent_class = "content_block_manager_content_block_edition" %>
+
+<% content_for :error_summary, render(Admin::ErrorSummaryComponent.new(object: @form.content_block_edition, parent_class:)) %>
 
 <%= form_with(
       url: @form.url,
       method: :post,
       model: [content_block_manager, @form.content_block_edition]) do |f| %>
   <%= hidden_field_tag "content_block/edition[document_attributes][block_type]",
-                       @form.schema.block_type,
-                       id: "content_block_manager/content_block_edition_document_block_type" %>
+                       @form.schema.block_type %>
 
   <%= render "govuk_publishing_components/components/input", {
     label: {
       text: "Title",
     },
     name: "content_block/edition[title]",
-    id: "content_block_manager/content_block/edition_title",
+    id: "#{parent_class}_title",
     value: @form.content_block_edition&.title,
-    error_items: errors_for(@form.content_block_edition.errors, "edition.title".to_sym),
+    error_items: errors_for(@form.content_block_edition.errors, "title".to_sym),
   } %>
 
   <% @form.attributes.each do |field, _value| %>
@@ -24,14 +25,14 @@
         text: field.humanize,
       },
       name: "content_block/edition[details[#{field}]]",
-      id: "content_block_manager/content_block/edition_details_#{field}",
+      id: "#{parent_class}_details_#{field}",
       value: @form.content_block_edition.details&.fetch(field, nil),
       error_items: errors_for(@form.content_block_edition.errors, "details_#{field}".to_sym),
     } %>
   <% end %>
 
   <%= render "components/select_with_search", {
-    id: "content_block/edition_lead_organisation",
+    id: "#{parent_class}_lead_organisation",
     name: "content_block/edition[organisation_id]",
     label:  "Lead organisation",
     include_blank: true,
@@ -50,7 +51,7 @@
       text: "Instructions to publishers (optional)",
     },
     name: "content_block/edition[instructions_to_publishers]",
-    id: "content_block/edition_instructions_to_publishers",
+    id: "#{parent_class}_instructions_to_publishers",
     value: @form.content_block_edition&.instructions_to_publishers,
     error_items: errors_for(@form.content_block_edition.errors, "instructions_to_publishers".to_sym),
   } %>

lib/engines/content_block_manager/config/locales/en.yml

@@ -1,5 +1,26 @@
 en:
   activerecord:
+    errors:
+      models:
+        content_block_manager/content_block/document:
+          attributes:
+            block_type:
+              blank: Select a content block
+        content_block_manager/content_block/edition:
+          format: "%{message}"
+          invalid: "Invalid %{attribute}"
+          blank: "%{attribute} cannot be blank"
+          attributes:
+            schedule_publishing:
+              blank: "Select publish date"
+            scheduled_publication:
+              blank: "Scheduled publication date and time cannot be blank"
+              invalid_date: "Invalid scheduled publication date and time"
+              future_date: "Scheduled publication date and time must be in the future"
+              date:
+                blank: "Scheduled publication date cannot be blank"
+              time:
+                blank: "Scheduled publication time cannot be blank"
     attributes:
       content_block_manager/content_block/edition/document:
         title: Title
@@ -27,4 +48,4 @@ en:
         detail: You can now view the updated content block.
     review_page:
       errors:
-        confirm: Confirm details are correct
+        confirm: Tick box to confirm details are correct

lib/engines/content_block_manager/features/create_object.feature

@@ -51,7 +51,7 @@ Feature: Create a content object
     When I complete the form with the following fields:
       | title            | email_address   | department | organisation |
       | my email address | xxxxx           | Somewhere  | Ministry of Example |
-    Then I should see a message that the "email_address" field is an invalid "email"
+    Then I should see a message that the field is an invalid "Email address"
 
   Scenario: GDS editor sees validation errors for unconfirmed answers
     When I visit the Content Block Manager home page

lib/engines/content_block_manager/features/edit_object.feature

@@ -92,7 +92,7 @@ Feature: Edit a content object
     When I complete the form with the following fields:
     | title            | email_address   | organisation |
     | my email address | xxxxx           | Ministry of Example |
-    Then I should see a message that the "email_address" field is an invalid "email"
+    Then I should see a message that the field is an invalid "Email address"
 
   Scenario: GDS editor sees validation errors for unconfirmed answers
     When I visit the Content Block Manager home page

lib/engines/content_block_manager/features/reschedule_object.feature

@@ -45,4 +45,4 @@ Feature: Schedule a content object
     When I click to view the content block
     And I click to edit the schedule
     And I save and continue
-    Then I see the error message "Schedule publishing cannot be blank"
+    Then I should see an error message telling me that schedule publishing cannot be blank

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

@@ -115,12 +115,12 @@
 
   fill_in "Title", with: @title if @title.present?
 
-  select @organisation, from: "content_block/edition_lead_organisation" if @organisation.present?
+  select @organisation, from: "content_block_manager_content_block_edition_lead_organisation" if @organisation.present?
 
   fill_in "Instructions to publishers", with: @instructions_to_publishers if @instructions_to_publishers.present?
 
   fields.keys.each do |k|
-    fill_in "content_block_manager/content_block/edition_details_#{k}", with: @details[k]
+    fill_in "content_block_manager_content_block_edition_details_#{k}", with: @details[k]
   end
 
   click_save_and_continue
@@ -447,19 +447,19 @@ def should_show_edit_form_for_email_address_content_block(document_title, email_
 end
 
 Then("I see the errors informing me the date is invalid") do
-  assert_text "Scheduled publication is not in the correct format", minimum: 2
+  assert_text I18n.t("activerecord.errors.models.content_block_manager/content_block/edition.attributes.scheduled_publication.time.blank"), minimum: 2
 end
 
-Then("I see the error message {string}") do |text|
-  assert_text text, minimum: 2
+Then("I should see an error message telling me that schedule publishing cannot be blank") do
+  assert_text I18n.t("activerecord.errors.models.content_block_manager/content_block/edition.attributes.schedule_publishing.blank"), minimum: 2
 end
 
 Then("I see the errors informing me the date must be in the future") do
-  assert_text "Scheduled publication date and time must be in the future", minimum: 2
+  assert_text I18n.t("activerecord.errors.models.content_block_manager/content_block/edition.attributes.scheduled_publication.future_date"), minimum: 2
 end
 
-Then("I should see a message that the {string} field is an invalid {string}") do |field_name, format|
-  assert_text "#{ContentBlockManager::ContentBlock::Edition.human_attribute_name("details_#{field_name}")} is an invalid #{format.titleize}"
+Then("I should see a message that the field is an invalid {string}") do |format|
+  assert_text I18n.t("activerecord.errors.models.content_block_manager/content_block/edition.invalid", attribute: format)
 end
 
 Then("I should see a message that I need to confirm the details are correct") do
@@ -568,7 +568,7 @@ def should_show_edit_form_for_email_address_content_block(document_title, email_
 end
 
 Then(/^I should see an error prompting me to choose an object type$/) do
-  assert_text "You must select a block type"
+  assert_text I18n.t("activerecord.errors.models.content_block_manager/content_block/document.attributes.block_type.blank")
 end
 
 Then(/^I am shown where the changes will take place$/) do
@@ -775,7 +775,7 @@ def visit_edit_page
 def change_details
   fill_in "Title", with: "Changed title"
   fill_in "Email address", with: "[email protected]"
-  select "Ministry of Example", from: "content_block/edition_lead_organisation"
+  select "Ministry of Example", from: "content_block_manager_content_block_edition_lead_organisation"
   fill_in "Instructions to publishers", with: "new context information"
   click_save_and_continue
 end

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

@@ -114,7 +114,7 @@ class ContentBlockManager::ContentBlock::DocumentsTest < ActionDispatch::Integra
       follow_redirect!
 
       assert_equal new_content_block_manager_content_block_document_path, path
-      assert_equal "You must select a block type", flash[:error]
+      assert_equal I18n.t("activerecord.errors.models.content_block_manager/content_block/document.attributes.block_type.blank"), flash[:error]
     end
 
     it "redirects when the block type is specified" do

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

@@ -139,7 +139,7 @@ class ContentBlockManager::ContentBlock::WorkflowTest < ActionDispatch::Integrat
             put content_block_manager.content_block_manager_content_block_workflow_path(id: edition.id, step:)
 
             assert_template "content_block_manager/content_block/editions/workflow/schedule_publishing"
-            assert_match(/Schedule publishing cannot be blank/, response.body)
+            assert_match(/#{I18n.t('activerecord.errors.models.content_block_manager/content_block/edition.attributes.schedule_publishing.blank')}/, response.body)
           end
         end
       end