Skip to content

Commit

Permalink
Implement custom sorting of error messages
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
peteryates committed Jun 12, 2021
1 parent f873492 commit 019f9eb
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 1 deletion.
22 changes: 21 additions & 1 deletion lib/govuk_design_system_formbuilder/elements/error_summary.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
76 changes: 76 additions & 0 deletions spec/govuk_design_system_formbuilder/builder/error_summary_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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}-(?<attribute_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

Expand Down
41 changes: 41 additions & 0 deletions spec/support/examples.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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' }
Expand Down Expand Up @@ -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
4 changes: 4 additions & 0 deletions spec/support/utility.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 019f9eb

Please sign in to comment.