Skip to content

Commit 52b4128

Browse files
mamhoffmamhoff
authored
Refactor ActiveModel::ErrorSerializer (#98)
* 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.
1 parent d2f94b5 commit 52b4128

4 files changed

Lines changed: 107 additions & 120 deletions

File tree

‎lib/jsonapi/active_model_error_serializer.rb‎

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -12,35 +12,15 @@ class ActiveModelErrorSerializer < ErrorSerializer
1212
end
1313

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

22-
attribute :detail do |object, params|
23-
error_key, error_hash = object
24-
errors_object = params[:model].errors
25-
26-
# Rails 4 provides just the message.
27-
if error_hash[:error].present? && error_hash[:error].is_a?(Hash)
28-
message = errors_object.generate_message(
29-
error_key, nil, error_hash[:error]
30-
)
31-
elsif error_hash[:error].present?
32-
message = errors_object.generate_message(
33-
error_key, error_hash[:error], error_hash
34-
)
35-
else
36-
message = error_hash[:message]
37-
end
38-
39-
errors_object.full_message(error_key, message)
18+
attribute :detail do |object, _params|
19+
object.full_message
4020
end
4121

4222
attribute :source do |object, params|
43-
error_key, _ = object
23+
error_key = object.attribute
4424
model_serializer = params[:model_serializer]
4525
attrs = (model_serializer.attributes_to_serialize || {}).keys
4626
rels = (model_serializer.relationships_to_serialize || {}).keys

‎lib/jsonapi/rails.rb‎

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ def self.add_errors_renderer!
4646
JSONAPI::ErrorSerializer.new(resource, options)
4747
) unless resource.is_a?(ActiveModel::Errors)
4848

49-
errors = []
5049
model = resource.instance_variable_get(:@base)
5150

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

58-
details = {}
59-
if ::Rails.gem_version >= Gem::Version.new('6.1')
60-
resource.each do |error|
61-
attr = error.attribute
62-
details[attr] ||= []
63-
details[attr] << error.detail.merge(message: error.message)
64-
end
65-
elsif resource.respond_to?(:details)
66-
details = resource.details
67-
else
68-
details = resource.messages
69-
end
70-
71-
details.each do |error_key, error_hashes|
72-
error_hashes.each do |error_hash|
73-
# Rails 4 provides just the message.
74-
error_hash = { message: error_hash } unless error_hash.is_a?(Hash)
75-
76-
errors << [ error_key, error_hash ]
77-
end
78-
end
79-
8057
JSONAPI::Rails.serializer_to_json(
8158
JSONAPI::ActiveModelErrorSerializer.new(
82-
errors, params: { model: model, model_serializer: model_serializer }
59+
resource.errors, params: {
60+
model: model,
61+
model_serializer: model_serializer
62+
}
8363
)
8464
)
8565
end

‎spec/dummy.rb‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ def self.ransackable_associations(auth_object = nil)
4141
end
4242

4343
class Note < ActiveRecord::Base
44+
validate :title_cannot_contain_slurs
4445
validates_format_of :title, without: /BAD_TITLE/
4546
validates_numericality_of :quantity, less_than: 100, if: :quantity?
4647
belongs_to :user, required: true
@@ -52,6 +53,11 @@ def self.ransackable_associations(auth_object = nil)
5253
def self.ransackable_attributes(auth_object = nil)
5354
%w(created_at id quantity title updated_at user_id)
5455
end
56+
57+
private
58+
def title_cannot_contain_slurs
59+
errors.add(:base, 'Title has slurs') if title.to_s.include?('SLURS')
60+
end
5561
end
5662

5763
class CustomNoteSerializer

‎spec/errors_spec.rb‎

Lines changed: 93 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,15 @@
3232
it do
3333
expect(response).to have_http_status(:unprocessable_entity)
3434
expect(response_json['errors'].size).to eq(1)
35-
36-
expect(response_json['errors'].first.keys)
37-
.to contain_exactly('status', 'source', 'title', 'detail', 'code')
38-
39-
expect(response_json['errors'][0]['status']).to eq('422')
40-
expect(response_json['errors'][0]['title'])
41-
.to eq(Rack::Utils::HTTP_STATUS_CODES[422])
42-
expect(response_json['errors'][0]['source']).to eq('pointer' => '')
43-
expect(response_json['errors'][0]['detail']).to be_nil
44-
expect(response_json['errors'][0]['code']).to be_nil
35+
expect(response_json['errors']).to contain_exactly(
36+
{
37+
'status' => '422',
38+
'source' => { 'pointer' => '' },
39+
'title' => 'Unprocessable Entity',
40+
'detail' => nil,
41+
'code' => nil
42+
}
43+
)
4544
end
4645
end
4746

@@ -55,19 +54,20 @@
5554
it do
5655
expect(response).to have_http_status(:unprocessable_entity)
5756
expect(response_json['errors'].size).to eq(1)
58-
expect(response_json['errors'][0]['status']).to eq('422')
59-
expect(response_json['errors'][0]['code']).to include('blank')
60-
expect(response_json['errors'][0]['title'])
61-
.to eq(Rack::Utils::HTTP_STATUS_CODES[422])
62-
expect(response_json['errors'][0]['source'])
63-
.to eq('pointer' => '/data/relationships/user')
64-
if Rails.gem_version >= Gem::Version.new('6.1')
65-
expect(response_json['errors'][0]['detail'])
66-
.to eq('User must exist')
57+
expected_detail = if Rails.gem_version >= Gem::Version.new('6.1')
58+
'User must exist'
6759
else
68-
expect(response_json['errors'][0]['detail'])
69-
.to eq('User can\'t be blank')
60+
'User can\'t be blank'
7061
end
62+
expect(response_json['errors']).to contain_exactly(
63+
{
64+
'status' => '422',
65+
'source' => { 'pointer' => '/data/relationships/user' },
66+
'title' => 'Unprocessable Entity',
67+
'detail' => expected_detail,
68+
'code' => 'blank'
69+
}
70+
)
7171
end
7272

7373
context 'required by validations' do
@@ -81,45 +81,51 @@
8181
it do
8282
expect(response).to have_http_status(:unprocessable_entity)
8383
expect(response_json['errors'].size).to eq(3)
84-
expect(response_json['errors'][0]['status']).to eq('422')
85-
expect(response_json['errors'][0]['code']).to include('invalid')
86-
expect(response_json['errors'][0]['title'])
87-
.to eq(Rack::Utils::HTTP_STATUS_CODES[422])
88-
expect(response_json['errors'][0]['source'])
89-
.to eq('pointer' => '/data/attributes/title')
90-
expect(response_json['errors'][0]['detail'])
91-
.to eq('Title is invalid')
92-
93-
expect(response_json['errors'][1]['status']).to eq('422')
94-
95-
if Rails::VERSION::MAJOR >= 5
96-
expect(response_json['errors'][1]['code']).to eq('invalid')
97-
else
98-
expect(response_json['errors'][1]['code']).to eq('has_typos')
99-
end
100-
101-
expect(response_json['errors'][1]['title'])
102-
.to eq(Rack::Utils::HTTP_STATUS_CODES[422])
103-
expect(response_json['errors'][1]['source'])
104-
.to eq('pointer' => '/data/attributes/title')
105-
expect(response_json['errors'][1]['detail'])
106-
.to eq('Title has typos')
107-
108-
expect(response_json['errors'][2]['status']).to eq('422')
109-
110-
if Rails::VERSION::MAJOR >= 5
111-
expect(response_json['errors'][2]['code']).to eq('less_than')
112-
else
113-
expect(response_json['errors'][2]['code'])
114-
.to eq('must_be_less_than_100')
115-
end
116-
117-
expect(response_json['errors'][2]['title'])
118-
.to eq(Rack::Utils::HTTP_STATUS_CODES[422])
119-
expect(response_json['errors'][2]['source'])
120-
.to eq('pointer' => '/data/attributes/quantity')
121-
expect(response_json['errors'][2]['detail'])
122-
.to eq('Quantity must be less than 100')
84+
expect(response_json['errors']).to contain_exactly(
85+
{
86+
'status' => '422',
87+
'source' => { 'pointer' => '/data/attributes/title' },
88+
'title' => 'Unprocessable Entity',
89+
'detail' => 'Title is invalid',
90+
'code' => 'invalid'
91+
},
92+
{
93+
'status' => '422',
94+
'source' => { 'pointer' => '/data/attributes/title' },
95+
'title' => 'Unprocessable Entity',
96+
'detail' => 'Title has typos',
97+
'code' => 'invalid'
98+
},
99+
{
100+
'status' => '422',
101+
'source' => { 'pointer' => '/data/attributes/quantity' },
102+
'title' => 'Unprocessable Entity',
103+
'detail' => 'Quantity must be less than 100',
104+
'code' => 'less_than'
105+
}
106+
)
107+
end
108+
end
109+
110+
context 'validations with non-interpolated messages' do
111+
let(:params) do
112+
payload = note_params.dup
113+
payload[:data][:attributes][:title] = 'SLURS ARE GREAT'
114+
payload
115+
end
116+
117+
it do
118+
expect(response).to have_http_status(:unprocessable_entity)
119+
expect(response_json['errors'].size).to eq(1)
120+
expect(response_json['errors']).to contain_exactly(
121+
{
122+
'status' => '422',
123+
'source' => { 'pointer' => '' },
124+
'title' => 'Unprocessable Entity',
125+
'detail' => 'Title has slurs',
126+
'code' => 'title_has_slurs'
127+
}
128+
)
123129
end
124130
end
125131

@@ -134,8 +140,15 @@
134140

135141
it do
136142
expect(response).to have_http_status(:unprocessable_entity)
137-
expect(response_json['errors'][0]['source'])
138-
.to eq('pointer' => '/data/attributes/title')
143+
expect(response_json['errors']).to contain_exactly(
144+
{
145+
'status' => '422',
146+
'source' => { 'pointer' => '/data/attributes/title' },
147+
'title' => 'Unprocessable Entity',
148+
'detail' => nil,
149+
'code' => nil
150+
}
151+
)
139152
end
140153
end
141154
end
@@ -147,11 +160,15 @@
147160
it do
148161
expect(response).to have_http_status(:not_found)
149162
expect(response_json['errors'].size).to eq(1)
150-
expect(response_json['errors'][0]['status']).to eq('404')
151-
expect(response_json['errors'][0]['title'])
152-
.to eq(Rack::Utils::HTTP_STATUS_CODES[404])
153-
expect(response_json['errors'][0]['source']).to be_nil
154-
expect(response_json['errors'][0]['detail']).to be_nil
163+
expect(response_json['errors']).to contain_exactly(
164+
{
165+
'status' => '404',
166+
'source' => nil,
167+
'title' => 'Not Found',
168+
'detail' => nil,
169+
'code' => nil
170+
}
171+
)
155172
end
156173
end
157174

@@ -162,11 +179,15 @@
162179
it do
163180
expect(response).to have_http_status(:internal_server_error)
164181
expect(response_json['errors'].size).to eq(1)
165-
expect(response_json['errors'][0]['status']).to eq('500')
166-
expect(response_json['errors'][0]['title'])
167-
.to eq(Rack::Utils::HTTP_STATUS_CODES[500])
168-
expect(response_json['errors'][0]['source']).to be_nil
169-
expect(response_json['errors'][0]['detail']).to be_nil
182+
expect(response_json['errors']).to contain_exactly(
183+
{
184+
'status' => '500',
185+
'source' => nil,
186+
'title' => 'Internal Server Error',
187+
'detail' => nil,
188+
'code' => nil
189+
}
190+
)
170191
end
171192
end
172193
end

0 commit comments

Comments
 (0)