(CM-936) Content design error message tweaks

app/controllers/concerns/date_range_validator.rb

@@ -1,5 +1,6 @@
 class DateRangeValidator
   SUBSCHEMA_BLOCK_TYPE = "date_range".freeze
+  include ApplicationHelper
 
   def initialize(edition, object_params)
     @edition = edition
@@ -52,13 +53,17 @@ def translate_datetime_error(error, field_name)
       I18n.t(
         "activerecord.errors.models.edition.attributes.#{field_name}.blank",
         attribute: attribute,
-        default: I18n.t("activerecord.errors.models.edition.blank", attribute: attribute),
+        default: I18n.t("activerecord.errors.models.edition.blank",
+                        attribute: attribute,
+                        attribute_with_indefinite_article: add_indefinite_article(attribute)),
       )
     when :date_invalid
       I18n.t(
         "activerecord.errors.models.edition.attributes.#{field_name}.invalid",
         attribute: attribute,
-        default: I18n.t("activerecord.errors.models.edition.invalid", attribute: attribute),
+        default: I18n.t("activerecord.errors.models.edition.invalid",
+                        attribute: attribute,
+                        attribute_with_indefinite_article: add_indefinite_article(attribute)),
       )
     end
   end

app/models/concerns/schema/field/translations.rb

@@ -1,6 +1,8 @@
 class Schema::Field
   module Translations
     extend ActiveSupport::Concern
+    include ApplicationHelper
+
     included do
       def label
         I18n.t(translation_lookup_path("labels"), default: default_translation_value)
@@ -14,6 +16,24 @@ def hint
         I18n.t(translation_lookup_path("hints"), default: nil)
       end
 
+      def error_message(error_type, **args)
+        path = [
+          "activerecord.errors.models",
+          translation_lookup_path("attributes"),
+          error_type,
+        ].join(".")
+        # If the translated label is a hash, this means this field has nested fields, so we use the `title` as the attribute instead
+        attribute = label.is_a?(String) ? label.downcase : title.downcase
+
+        default = I18n.t(
+          "activerecord.errors.models.edition.#{error_type}",
+          attribute:,
+          attribute_with_indefinite_article: add_indefinite_article(attribute),
+          **args,
+        )
+        I18n.t(path, default:, **args)
+      end
+
     private
 
       def default_translation_value

app/models/document.rb

@@ -1,4 +1,6 @@
 class Document < ApplicationRecord
+  include ApplicationHelper
+
   include Scopes::SearchableByKeyword
   include Scopes::SearchableByLeadOrganisation
   include Scopes::SearchableByUpdatedDate
@@ -63,6 +65,15 @@ def schema
     @schema ||= Schema.find_by_block_type(block_type)
   end
 
+  def title_name
+    attribute_key = "activerecord.attributes.edition/document.title.#{block_type || 'default'}"
+    I18n.t(attribute_key, default: I18n.t("activerecord.attributes.edition/document.title.default"))
+  end
+
+  def title_name_with_indefinite_article
+    add_indefinite_article(title_name.downcase)
+  end
+
 private
 
   def embed_code_prefix
@@ -71,7 +82,7 @@ def embed_code_prefix
 
   def sluggable_string_contains_alphanumeric_chars
     if sluggable_string !~ /[a-z0-9]+/i
-      errors.add(:title, I18n.t("document.index.errors.title.missing_valid_chars"))
+      errors.add(:title, I18n.t("activerecord.errors.models.document.attributes.title.missing_valid_chars", attribute: title_name))
     end
   end
 end

app/models/edition.rb

@@ -1,5 +1,10 @@
 class Edition < ApplicationRecord
-  validates :title, presence: true
+  validates :title, presence: {
+    message: lambda do |edition, _|
+      I18n.t("activerecord.errors.models.edition.blank",
+             attribute_with_indefinite_article: edition.document.title_name_with_indefinite_article)
+    end,
+  }
   validates :change_note, presence: true, if: :major_change?, on: :change_note
   validates :major_change, inclusion: [true, false], on: :change_note
 

app/public/models/schema.rb

@@ -66,6 +66,20 @@ def field(name)
     fields.find(proc { raise "Field '#{name}' not found" }) { |f| f.name == name }
   end
 
+  def all_fields
+    collected = fields.dup
+
+    subschemas.each do |subschema|
+      collected.concat(subschema.all_fields)
+    end
+
+    fields.each do |field|
+      collected.concat(field.all_nested_fields)
+    end
+
+    collected
+  end
+
   def subschema(name)
     subschemas.find { |s| s.id == name }
   end

app/public/models/schema/field.rb

@@ -49,6 +49,18 @@ def nested_fields
       end
     end
 
+    def all_nested_fields
+      return [] if nested_fields.nil?
+
+      collected = nested_fields.dup
+
+      nested_fields.each do |field|
+        collected.concat(field.nested_fields) if field.nested_fields.present?
+      end
+
+      collected
+    end
+
     def nested_field(nested_field_name)
       raise(ArgumentError, "Provide the name of a nested field") if nested_field_name.blank?
 

app/validators/details_validator.rb

@@ -26,18 +26,18 @@ def add_blank_errors(error)
       key = key_with_optional_prefix(error, k)
       edition.errors.add(
         "details_#{key}",
-        translate_error("blank", k),
+        error_message_for_key(key, "blank"),
       )
     end
   end
 
   def add_format_minimum_errors(error, block_type)
-    attribute, key = get_attribute_and_key_from_error(error)
+    key = key_with_optional_prefix(error, nil)
     minimum_date = resolve_format_minimum_date(error["schema"]["formatMinimum"], block_type)
 
     edition.errors.add(
       "details_#{key}",
-      translate_error("minimum", attribute, minimum_date: minimum_date),
+      error_message_for_key(key, "minimum", minimum_date: minimum_date),
     )
   end
 
@@ -53,19 +53,14 @@ def resolve_format_minimum_date(format_minimum, block_type)
   end
 
   def add_format_errors(error)
-    attribute, key = get_attribute_and_key_from_error(error)
+    key = key_with_optional_prefix(error, nil)
+
     edition.errors.add(
       "details_#{key}",
-      translate_error("invalid", attribute),
+      error_message_for_key(key, "invalid"),
     )
   end
 
-  def get_attribute_and_key_from_error(error)
-    data_pointer = error["data_pointer"].delete_prefix("/")
-    field_items = data_pointer.split("/")
-    [field_items.last, key_with_optional_prefix(error, nil)]
-  end
-
   def validate_with_schema(edition)
     # Fetch the details and remove any blank fields (JSONSchema classes an empty string as valid,
     # unless a specific format has been specified)
@@ -91,16 +86,6 @@ def key_with_optional_prefix(error, key)
     end
   end
 
-  def translate_error(type, attribute, **args)
-    default = "activerecord.errors.models.edition.#{type}".to_sym
-    I18n.t(
-      "activerecord.errors.models.edition.attributes.#{attribute}.#{type}",
-      attribute: attribute.humanize,
-      default: [default],
-      **args,
-    )
-  end
-
 private
 
   def block_type_blank?
@@ -131,4 +116,15 @@ def compact_nested(object)
     object.each { |o| compact_nested(o) }
     object
   end
+
+  def error_message_for_key(key, type, **args)
+    field = field_for_key(key)
+    field&.error_message(type, **args)
+  end
+
+  def field_for_key(key)
+    # Remove the index from the key if present, e.g. "details_1_title" becomes "details_title"
+    id_attribute = "edition_details_#{key.gsub(/_([0-9]+)_/, '_')}"
+    edition.schema.all_fields.find { |f| f.id_attribute == id_attribute }
+  end
 end

config/locales/en.yml

@@ -1,22 +1,61 @@
 en:
+  errors:
+    format: "%{message}"
   activerecord:
     errors:
       title: There is a problem
       models:
         document:
           attributes:
+            title:
+              missing_valid_chars: "%{attribute} must include at least one letter or number"
             block_type:
               blank: Select a content block
               contact_methods:
                 blank: Select a contact method
         edition:
           title:
             not_unique: "A content block with the same title already exists. If you continue, this block will not have a unique title."
-          format: "%{message}"
-          invalid: "Invalid %{attribute}"
-          blank: "%{attribute} cannot be blank"
+          invalid: "Enter %{attribute_with_indefinite_article} in the correct format"
+          blank: "Enter %{attribute_with_indefinite_article}"
           minimum: "%{attribute} must be after %{minimum_date}"
           attributes:
+            contact:
+              contact_links:
+                url:
+                  blank: "Enter a URL"
+                  invalid: "Enter a URL in the correct format"
+              telephones:
+                telephone_numbers:
+                  blank: "Enter a telephone number"
+                  label:
+                    blank: "Enter a telephone label"
+            time_period:
+              date_range:
+                start:
+                  blank: "Enter a start date"
+                  invalid: "Enter a real date"
+                end:
+                  blank: "Enter an end date"
+                  invalid: "Enter a real date"
+                  minimum: "The end date must be after the start date"
+            tax:
+              things_taxed:
+                type:
+                  blank: "Select type"
+                rates:
+                  blank: "Add at least one rate"
+                  name:
+                    blank: "Enter a rate name"
+                  value:
+                    blank: "Enter a rate value"
+                  bands:
+                    name:
+                      blank: "Enter a band name"
+            change_note:
+              blank: "Enter a change note"
+            lead_organisation_id:
+              blank: "Select a lead organisation"
             schedule_publishing:
               blank: "Select publish date"
             call_charges_info_url:
@@ -64,9 +103,6 @@ en:
           invalid: "Lead Organisation is not valid"
         request:
           invalid: Invalid request
-        title:
-          missing_valid_chars: "must contain at least one letter or number"
-
   domain_event:
     title:
       edition:

features/step_definitions/embedded_object_steps.rb

@@ -129,14 +129,16 @@
   schema = @schema.subschema(object_type.pluralize)
   required_fields = schema.body["required"]
   required_fields.each do |required_field|
-    assert_text "#{Edition.human_attribute_name("details_#{required_field}")} cannot be blank", minimum: 2
+    field = schema.field(required_field)
+    assert_text field.error_message("blank"), minimum: 2
   end
 end
 
 Then("I should see errors for the required nested {string} fields") do |nested_object_name|
   subschema = @schema.subschema(@object_type.pluralize)
   required_fields(subschema, nested_object_name).each do |required_field|
-    expect(page).to have_text("#{Edition.human_attribute_name("details_#{required_field}")} cannot be blank")
+    field = subschema.field(nested_object_name.pluralize).nested_field(required_field)
+    expect(page).to have_text(field.error_message("blank"))
   end
 end
 

features/step_definitions/error_message_steps.rb

@@ -11,13 +11,13 @@
 end
 
 Then("I should see errors for the required fields") do
-  assert_text "Title cannot be blank", minimum: 2
+  assert_text "Enter a title", minimum: 2
 
   required_fields = @schema.fields.select(&:is_required?).map(&:name)
   required_fields.each do |required_field|
     assert_text "#{Edition.human_attribute_name("details_#{required_field}")} cannot be blank", minimum: 2
   end
-  assert_text "Lead organisation cannot be blank", minimum: 2
+  assert_text "Select a lead organisation", minimum: 2
 end
 
 Then("I should see a message that the filter dates are invalid") do

spec/components/shared/error_summary_component_spec.rb

@@ -126,7 +126,8 @@ class ErrorSummaryTestObject
   include ActiveModel::Model
   attr_accessor :title, :date
 
-  validates :title, :date, presence: true
+  validates :title, presence: { message: "Title can't be blank" }
+  validates :date, presence: { message: "Date can't be blank" }
   validate :date_is_a_date
 
   def initialize(title, date)
@@ -135,6 +136,6 @@ def initialize(title, date)
   end
 
   def date_is_a_date
-    errors.add(:date, :invalid) unless date.is_a?(Date)
+    errors.add(:date, "Date is invalid") unless date.is_a?(Date)
   end
 end

spec/requests/workflow_spec.rb

@@ -333,7 +333,7 @@ def self.it_shows_the_correct_context
               }
 
           expect(response).to render_template("editions/workflow/edit_draft")
-          expect(page).to have_text(I18n.t("activerecord.errors.models.edition.blank", attribute: "Title"))
+          expect(page).to have_text(I18n.t("activerecord.errors.models.edition.blank", attribute_with_indefinite_article: "a title"))
         end
       end
     end
@@ -509,7 +509,7 @@ def self.it_shows_the_correct_context
                 },
               }
 
-          expect(page).to have_text(I18n.t("activerecord.errors.models.edition.blank", attribute: "Change note"))
+          expect(page).to have_text(I18n.t("activerecord.errors.models.edition.attributes.change_note.blank"))
         end
 
         it "shows an error if major_change is blank" do

spec/unit/app/models/document_spec.rb

@@ -198,11 +198,20 @@
       expect(document.content_id_alias).to eq("this-is-a-title")
     end
 
-    it "ensures the sluggable string must contain some letters or numbers" do
-      document = build(:document, sluggable_string: "💯💯💯")
-
-      expect(document.valid?).to be(false)
-      expect(document.errors[:title]).to include(I18n.t("document.index.errors.title.missing_valid_chars"))
+    {
+      pension: "Title",
+      contact: "Contact name",
+      tax: "Tax name",
+      time_period: "Time period name",
+    }.each do |block_type, attribute_name|
+      context "when the edition is a #{block_type}" do
+        it "ensures the sluggable string must contain some letters or numbers" do
+          document = build(:document, block_type, sluggable_string: "💯💯💯")
+
+          expect(document.valid?).to be(false)
+          expect(document.errors[:title]).to include(I18n.t("activerecord.errors.models.document.attributes.title.missing_valid_chars", attribute: attribute_name))
+        end
+      end
     end
   end