Skip to content

Commit

Permalink
Refactor ActiveModel::ErrorSerializer (#98)
Browse files Browse the repository at this point in the history 
* CI Matrix: Drop Rails 6, Ruby 2.7

Test Ruby 3.3 and Rails 6.1 instead

* Refactor error spec

The previous specs were harder to read, and they relied on the ordering
of the errors, which is unnecessary.

* Add spec for non-interpolated error messages

* Simplify ActiveModel::ErrorSerializer

We're only running CI for Rails 6.1+ now, and we can now rely on a
stable API for ActiveModel::Errors.
  • Loading branch information
@mamhoff
mamhoff authored Jun 23, 2024
1 parent d2f94b5 commit 52b4128
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 120 deletions.
28 changes: 4 additions & 24 deletions lib/jsonapi/active_model_error_serializer.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,35 +12,15 @@ class ActiveModelErrorSerializer < ErrorSerializer
end

attribute :code do |object|
_, error_hash = object
code = error_hash[:error] unless error_hash[:error].is_a?(Hash)
code ||= error_hash[:message] || :invalid
# `parameterize` separator arguments are different on Rails 4 vs 5...
code.to_s.delete("''").parameterize.tr('-', '_')
object.type.to_s.delete("''").parameterize.tr('-', '_')
end

attribute :detail do |object, params|
error_key, error_hash = object
errors_object = params[:model].errors

# Rails 4 provides just the message.
if error_hash[:error].present? && error_hash[:error].is_a?(Hash)
message = errors_object.generate_message(
error_key, nil, error_hash[:error]
)
elsif error_hash[:error].present?
message = errors_object.generate_message(
error_key, error_hash[:error], error_hash
)
else
message = error_hash[:message]
end

errors_object.full_message(error_key, message)
attribute :detail do |object, _params|
object.full_message
end

attribute :source do |object, params|
error_key, _ = object
error_key = object.attribute
model_serializer = params[:model_serializer]
attrs = (model_serializer.attributes_to_serialize || {}).keys
rels = (model_serializer.relationships_to_serialize || {}).keys
Expand Down
28 changes: 4 additions & 24 deletions lib/jsonapi/rails.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ def self.add_errors_renderer!
JSONAPI::ErrorSerializer.new(resource, options)
) unless resource.is_a?(ActiveModel::Errors)

errors = []
model = resource.instance_variable_get(:@base)

if respond_to?(:jsonapi_serializer_class, true)
Expand All @@ -55,31 +54,12 @@ def self.add_errors_renderer!
model_serializer = JSONAPI::Rails.serializer_class(model, false)
end

details = {}
if ::Rails.gem_version >= Gem::Version.new('6.1')
resource.each do |error|
attr = error.attribute
details[attr] ||= []
details[attr] << error.detail.merge(message: error.message)
end
elsif resource.respond_to?(:details)
details = resource.details
else
details = resource.messages
end

details.each do |error_key, error_hashes|
error_hashes.each do |error_hash|
# Rails 4 provides just the message.
error_hash = { message: error_hash } unless error_hash.is_a?(Hash)

errors << [ error_key, error_hash ]
end
end

JSONAPI::Rails.serializer_to_json(
JSONAPI::ActiveModelErrorSerializer.new(
errors, params: { model: model, model_serializer: model_serializer }
resource.errors, params: {
model: model,
model_serializer: model_serializer
}
)
)
end
Expand Down
6 changes: 6 additions & 0 deletions spec/dummy.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ def self.ransackable_associations(auth_object = nil)
end

class Note < ActiveRecord::Base
validate :title_cannot_contain_slurs
validates_format_of :title, without: /BAD_TITLE/
validates_numericality_of :quantity, less_than: 100, if: :quantity?
belongs_to :user, required: true
Expand All @@ -52,6 +53,11 @@ def self.ransackable_associations(auth_object = nil)
def self.ransackable_attributes(auth_object = nil)
%w(created_at id quantity title updated_at user_id)
end

private
def title_cannot_contain_slurs
errors.add(:base, 'Title has slurs') if title.to_s.include?('SLURS')
end
end

class CustomNoteSerializer
Expand Down
165 changes: 93 additions & 72 deletions spec/errors_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,15 @@
it do
expect(response).to have_http_status(:unprocessable_entity)
expect(response_json['errors'].size).to eq(1)

expect(response_json['errors'].first.keys)
.to contain_exactly('status', 'source', 'title', 'detail', 'code')

expect(response_json['errors'][0]['status']).to eq('422')
expect(response_json['errors'][0]['title'])
.to eq(Rack::Utils::HTTP_STATUS_CODES[422])
expect(response_json['errors'][0]['source']).to eq('pointer' => '')
expect(response_json['errors'][0]['detail']).to be_nil
expect(response_json['errors'][0]['code']).to be_nil
expect(response_json['errors']).to contain_exactly(
{
'status' => '422',
'source' => { 'pointer' => '' },
'title' => 'Unprocessable Entity',
'detail' => nil,
'code' => nil
}
)
end
end

Expand All @@ -55,19 +54,20 @@
it do
expect(response).to have_http_status(:unprocessable_entity)
expect(response_json['errors'].size).to eq(1)
expect(response_json['errors'][0]['status']).to eq('422')
expect(response_json['errors'][0]['code']).to include('blank')
expect(response_json['errors'][0]['title'])
.to eq(Rack::Utils::HTTP_STATUS_CODES[422])
expect(response_json['errors'][0]['source'])
.to eq('pointer' => '/data/relationships/user')
if Rails.gem_version >= Gem::Version.new('6.1')
expect(response_json['errors'][0]['detail'])
.to eq('User must exist')
expected_detail = if Rails.gem_version >= Gem::Version.new('6.1')
'User must exist'
else
expect(response_json['errors'][0]['detail'])
.to eq('User can\'t be blank')
'User can\'t be blank'
end
expect(response_json['errors']).to contain_exactly(
{
'status' => '422',
'source' => { 'pointer' => '/data/relationships/user' },
'title' => 'Unprocessable Entity',
'detail' => expected_detail,
'code' => 'blank'
}
)
end

context 'required by validations' do
Expand All @@ -81,45 +81,51 @@
it do
expect(response).to have_http_status(:unprocessable_entity)
expect(response_json['errors'].size).to eq(3)
expect(response_json['errors'][0]['status']).to eq('422')
expect(response_json['errors'][0]['code']).to include('invalid')
expect(response_json['errors'][0]['title'])
.to eq(Rack::Utils::HTTP_STATUS_CODES[422])
expect(response_json['errors'][0]['source'])
.to eq('pointer' => '/data/attributes/title')
expect(response_json['errors'][0]['detail'])
.to eq('Title is invalid')

expect(response_json['errors'][1]['status']).to eq('422')

if Rails::VERSION::MAJOR >= 5
expect(response_json['errors'][1]['code']).to eq('invalid')
else
expect(response_json['errors'][1]['code']).to eq('has_typos')
end

expect(response_json['errors'][1]['title'])
.to eq(Rack::Utils::HTTP_STATUS_CODES[422])
expect(response_json['errors'][1]['source'])
.to eq('pointer' => '/data/attributes/title')
expect(response_json['errors'][1]['detail'])
.to eq('Title has typos')

expect(response_json['errors'][2]['status']).to eq('422')

if Rails::VERSION::MAJOR >= 5
expect(response_json['errors'][2]['code']).to eq('less_than')
else
expect(response_json['errors'][2]['code'])
.to eq('must_be_less_than_100')
end

expect(response_json['errors'][2]['title'])
.to eq(Rack::Utils::HTTP_STATUS_CODES[422])
expect(response_json['errors'][2]['source'])
.to eq('pointer' => '/data/attributes/quantity')
expect(response_json['errors'][2]['detail'])
.to eq('Quantity must be less than 100')
expect(response_json['errors']).to contain_exactly(
{
'status' => '422',
'source' => { 'pointer' => '/data/attributes/title' },
'title' => 'Unprocessable Entity',
'detail' => 'Title is invalid',
'code' => 'invalid'
},
{
'status' => '422',
'source' => { 'pointer' => '/data/attributes/title' },
'title' => 'Unprocessable Entity',
'detail' => 'Title has typos',
'code' => 'invalid'
},
{
'status' => '422',
'source' => { 'pointer' => '/data/attributes/quantity' },
'title' => 'Unprocessable Entity',
'detail' => 'Quantity must be less than 100',
'code' => 'less_than'
}
)
end
end

context 'validations with non-interpolated messages' do
let(:params) do
payload = note_params.dup
payload[:data][:attributes][:title] = 'SLURS ARE GREAT'
payload
end

it do
expect(response).to have_http_status(:unprocessable_entity)
expect(response_json['errors'].size).to eq(1)
expect(response_json['errors']).to contain_exactly(
{
'status' => '422',
'source' => { 'pointer' => '' },
'title' => 'Unprocessable Entity',
'detail' => 'Title has slurs',
'code' => 'title_has_slurs'
}
)
end
end

Expand All @@ -134,8 +140,15 @@

it do
expect(response).to have_http_status(:unprocessable_entity)
expect(response_json['errors'][0]['source'])
.to eq('pointer' => '/data/attributes/title')
expect(response_json['errors']).to contain_exactly(
{
'status' => '422',
'source' => { 'pointer' => '/data/attributes/title' },
'title' => 'Unprocessable Entity',
'detail' => nil,
'code' => nil
}
)
end
end
end
Expand All @@ -147,11 +160,15 @@
it do
expect(response).to have_http_status(:not_found)
expect(response_json['errors'].size).to eq(1)
expect(response_json['errors'][0]['status']).to eq('404')
expect(response_json['errors'][0]['title'])
.to eq(Rack::Utils::HTTP_STATUS_CODES[404])
expect(response_json['errors'][0]['source']).to be_nil
expect(response_json['errors'][0]['detail']).to be_nil
expect(response_json['errors']).to contain_exactly(
{
'status' => '404',
'source' => nil,
'title' => 'Not Found',
'detail' => nil,
'code' => nil
}
)
end
end

Expand All @@ -162,11 +179,15 @@
it do
expect(response).to have_http_status(:internal_server_error)
expect(response_json['errors'].size).to eq(1)
expect(response_json['errors'][0]['status']).to eq('500')
expect(response_json['errors'][0]['title'])
.to eq(Rack::Utils::HTTP_STATUS_CODES[500])
expect(response_json['errors'][0]['source']).to be_nil
expect(response_json['errors'][0]['detail']).to be_nil
expect(response_json['errors']).to contain_exactly(
{
'status' => '500',
'source' => nil,
'title' => 'Internal Server Error',
'detail' => nil,
'code' => nil
}
)
end
end
end
Expand Down

0 comments on commit 52b4128

Please sign in to comment.