From 019f9ebde82259e3357ed98b2969475beb19716e Mon Sep 17 00:00:00 2001 From: Peter Yates Date: Sat, 12 Jun 2021 14:43:59 +0100 Subject: [PATCH] Implement custom sorting of error messages Currently this depends on there being a #error_order method being present on the object/model associated with the builder. I went for this approach so that with objects that are represented by several forms through an app, there's no duplication in the views. It may be worth reconsidering this, but it feels broadly sensible. The way ordering works is the #error_order method should return an array of attribute names, eg: [:name, :email, :age] and when building the list of errors the error summary promotes these to the start. Any errors on attributes that aren't covered by #error_order will appear after those that are, in the order they were defined in the object or model. If there is no #error_order method on the object, don't try to re-order and just go with what Rails gives us, which is the order they were defined. --- .../elements/error_summary.rb | 22 +++++- .../builder/error_summary_spec.rb | 76 +++++++++++++++++++ spec/support/examples.rb | 41 ++++++++++ spec/support/utility.rb | 4 + 4 files changed, 142 insertions(+), 1 deletion(-) diff --git a/lib/govuk_design_system_formbuilder/elements/error_summary.rb b/lib/govuk_design_system_formbuilder/elements/error_summary.rb index ce5b75de..3d7da9c0 100644 --- a/lib/govuk_design_system_formbuilder/elements/error_summary.rb +++ b/lib/govuk_design_system_formbuilder/elements/error_summary.rb @@ -35,11 +35,31 @@ def summary end def list - @builder.object.errors.messages.map do |attribute, messages| + error_messages.map do |attribute, messages| list_item(attribute, messages.first) end end + def error_messages + messages = @builder.object.errors.messages + + if reorder_errors? + return messages.sort_by.with_index(1) do |(attr, _val), i| + error_order.index(attr) || (i + messages.size) + end + end + + @builder.object.errors.messages + end + + def reorder_errors? + @builder.object.respond_to?(:error_order) && @builder.object.error_order.present? + end + + def error_order + @builder.object.error_order + end + def list_item(attribute, message) tag.li(link_to(message, same_page_link(field_id(attribute)), data: { turbolinks: false })) end diff --git a/spec/govuk_design_system_formbuilder/builder/error_summary_spec.rb b/spec/govuk_design_system_formbuilder/builder/error_summary_spec.rb index 04c21762..21ab22d5 100644 --- a/spec/govuk_design_system_formbuilder/builder/error_summary_spec.rb +++ b/spec/govuk_design_system_formbuilder/builder/error_summary_spec.rb @@ -292,6 +292,82 @@ end end end + + describe "custom sort order" do + let(:actual_order) do + parsed_subject + .css('li > a') + .map { |element| element['href'] } + .map { |href| href.match(%r[#{object_name}-(?.*)-field-error])[:attribute_name] } + .map { |attribute| dashes_to_underscores(attribute) } + end + + context "by default" do + # the object here is Person, defined in spec/support/examples.rb + # + # the validation order is: name, favourite colour, projects, cv + # + # name is present on the object + specify "errors are displayed in the order they're defined in the model" do + expect(object.name).to be_present + + expect(actual_order).to eql(%w(favourite_colour projects cv)) + end + end + + describe "overriding" do + let(:overridden_order) { object.error_order.map(&:to_s) } + + context "when the object has no overridden ordering" do + let(:object) { OrderedErrors.new } + let(:expected_order) { %w(a b c d e) } + + # there's no error_order method on the object, ensure nothing blows up + specify "the error messages are displayed in the order they were defined in the model" do + expect(actual_order).to eql(expected_order) + end + end + + context "when all attributes are named in the ordering" do + let(:object) { OrderedErrorsWithCustomOrder.new } + + # the default validation order is (:a, :b, :c, :d, :e) + # + # the overridden order is (:e, :d, :c, :b, :a) + specify "the error messages are displayed in the overridden order" do + expect(actual_order).to eql(overridden_order) + end + end + + context "when there are attributes with errors that aren't named in the ordering" do + let(:object) { OrderedErrorsWithCustomOrderAndExtraAttributes.new } + + # the default validation order is (:a, :b, :c, :d, :e) + # + # the overridden order is (:e, :d, :c, :b, :a) + # + # the extra attributes (:g, :h, :i) validation order is (:i, :h, :g) + specify "the errors for attributes with overridden ordering are first" do + expect(actual_order).to start_with(overridden_order) + end + + specify "the errors for extra attributes appear last, in the order they were defined in the model" do + expect(actual_order).to end_with(%w(i h g)) + end + end + + context "when the ordering specifies attributes that aren't present on the object" do + let(:object) { OrderedErrorsWithCustomOrderAndInvalidAttributes.new } + let(:expected_order) { %w(a b c d e) } + + # there's no error_order method, ensure it doesn't blow up. it shouldn't + # because #index will return nil + specify "the error messages are displayed in the order they were defined in the model" do + expect(actual_order).to eql(expected_order) + end + end + end + end end end diff --git a/spec/support/examples.rb b/spec/support/examples.rb index 8d89ee76..9453b27a 100644 --- a/spec/support/examples.rb +++ b/spec/support/examples.rb @@ -33,6 +33,8 @@ def initialize(_args = nil) class Person < Being include ActiveModel::Model + attr_accessor :error_order + validates :name, presence: { message: 'Enter a name' }, length: { minimum: 2, message: 'Name should be longer than 1' } @@ -108,3 +110,42 @@ def initialize(code:, name:) end WrongDate = Struct.new(:d, :m, :y) + +class OrderedErrors + include ActiveModel::Model + include ActiveModel::Attributes + + attribute :a, :string + attribute :b, :string + attribute :c, :string + attribute :d, :string + attribute :e, :string + + validates :a, presence: true, length: { minimum: 3 } + validates :b, presence: true, length: { minimum: 3 } + validates :c, presence: true, length: { minimum: 3 } + validates :d, presence: true, length: { minimum: 3 } + validates :e, presence: true, length: { minimum: 3 } +end + +class OrderedErrorsWithCustomOrder < OrderedErrors + def error_order + %i(a b c d e).reverse + end +end + +class OrderedErrorsWithCustomOrderAndInvalidAttributes < OrderedErrors + def error_order + %i(x y z) + end +end + +class OrderedErrorsWithCustomOrderAndExtraAttributes < OrderedErrorsWithCustomOrder + attribute :g, :string + attribute :h, :string + attribute :i, :string + + validates :i, presence: true, length: { minimum: 3 } + validates :h, presence: true, length: { minimum: 3 } + validates :g, presence: true, length: { minimum: 3 } +end diff --git a/spec/support/utility.rb b/spec/support/utility.rb index e6cfd13d..c7c7a853 100644 --- a/spec/support/utility.rb +++ b/spec/support/utility.rb @@ -14,6 +14,10 @@ def underscores_to_dashes(val) val.to_s.tr('_', '-') end +def dashes_to_underscores(val) + val.to_s.tr('-', '_') +end + def rails_version ENV.fetch('RAILS_VERSION') { '6.1.1' } end