From 54a56e4896f8ffdd1bd27d21ceee8f08bf07a2e5 Mon Sep 17 00:00:00 2001 From: Martin Gregoire Date: Wed, 27 Aug 2025 14:50:30 +0200 Subject: [PATCH 1/2] chore: fix error serialization for string errors When adding custom validation errors, Rails allows both Symbols and Strings: ``` class Foo < ActiveRecord::Base validate :add_symbol_error validate :add_string_error def add_symbol_error errors.add(:title, :too_short) end def add_string_error errors.add(:title, 'is too short') end end ``` The Symbol error will later be translated by Rails using the i18n logic, when calling `foo.errors.full_messages` or similar methods. The String error will stay as it is. see https://api.rubyonrails.org/v7.0.8.7/classes/ActiveModel/Errors.html#method-i-add This commit introduces support for these String errors in the error serialization class. Before this change, the error response for the validation that was added in the specs looked like this: ```ruby [ { "status"=>"422", "source"=>{"pointer"=>"/data/attributes/title"}, "title"=>"Unprocessable Entity", "detail"=> "Title translation missing: en.activerecord.errors.models.note.attributes.title.is not so nice", "code"=>"is_not_so_nice" } ] ``` Please note the "translation missing" part in the `detail`, and the `code` that is a parameterized version of the String error. After this change, the error response looks like this: ```ruby [ { "status"=>"422", "source"=>{"pointer"=>"/data/attributes/title"}, "title"=>"Unprocessable Entity", "detail"=>"Title is not so nice", "code"=>"invalid" } ] ``` The `detail` is now the String error as passed in by Rails, and the `code` is a generic `"invalid"` one. --- lib/jsonapi/active_model_error_serializer.rb | 16 +++++++++++---- spec/dummy.rb | 15 ++++++++++++++ spec/errors_spec.rb | 21 ++++++++++++++++++++ 3 files changed, 48 insertions(+), 4 deletions(-) diff --git a/lib/jsonapi/active_model_error_serializer.rb b/lib/jsonapi/active_model_error_serializer.rb index 59a3e3c..3968142 100644 --- a/lib/jsonapi/active_model_error_serializer.rb +++ b/lib/jsonapi/active_model_error_serializer.rb @@ -13,7 +13,9 @@ class ActiveModelErrorSerializer < ErrorSerializer attribute :code do |object| _, error_hash = object - code = error_hash[:error] unless error_hash[:error].is_a?(Hash) + unless error_hash[:error].is_a?(Hash) || error_hash[:error].is_a?(String) + code = error_hash[:error] + end code ||= error_hash[:message] || :invalid # `parameterize` separator arguments are different on Rails 4 vs 5... code.to_s.delete("''").parameterize.tr('-', '_') @@ -29,9 +31,15 @@ class ActiveModelErrorSerializer < ErrorSerializer error_key, nil, error_hash[:error] ) elsif error_hash[:error].present? - message = errors_object.generate_message( - error_key, error_hash[:error], error_hash - ) + # if the error was added as a string, we do not need to generate an + # error message, but use the error as is + if error_hash[:error].is_a?(String) + message = error_hash[:error] + else + message = errors_object.generate_message( + error_key, error_hash[:error], error_hash + ) + end else message = error_hash[:message] end diff --git a/spec/dummy.rb b/spec/dummy.rb index d273f81..eca55f0 100644 --- a/spec/dummy.rb +++ b/spec/dummy.rb @@ -43,6 +43,21 @@ class Note < ActiveRecord::Base validates_format_of :title, without: /BAD_TITLE/ validates_numericality_of :quantity, less_than: 100, if: :quantity? belongs_to :user, required: true + + validate :nice_title + + # this validation is added to test that a validation error added as a string + # (instead of a symbol) works as well + # + # see https://api.rubyonrails.org/v7.0.8.7/classes/ActiveModel/Errors.html#method-i-add + # + # > If `type` is a string, it will be used as error message. + # + def nice_title + return unless title == 'NOT_SO_NICE_TITLE' + + errors.add(:title, 'is not so nice') + end end class CustomNoteSerializer diff --git a/spec/errors_spec.rb b/spec/errors_spec.rb index 38cf978..517071b 100644 --- a/spec/errors_spec.rb +++ b/spec/errors_spec.rb @@ -119,6 +119,27 @@ end end + context 'with a validation error type that is a string' do + let(:params) do + payload = note_params.dup + payload[:data][:attributes][:title] = 'NOT_SO_NICE_TITLE' + payload + end + + it do + expect(response).to have_http_status(:unprocessable_entity) + expect(response_json['errors'].size).to eq(2) + 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 not so nice') + end + end + context 'as a param attribute' do let(:params) do payload = note_params.dup From 8f3b8d51ee881801a8ac761f15f3013d176be95f Mon Sep 17 00:00:00 2001 From: Martin Gregoire Date: Wed, 27 Aug 2025 15:00:10 +0200 Subject: [PATCH 2/2] chore: rubocop see https://github.com/easyPEP/jsonapi.rb/actions/runs/17267231602/job/49002149588?pr=2 ``` Running RuboCop... RuboCop failed! Inspecting 18 files ................C. Offenses: spec/pagination_spec.rb:61:15: C: [Correctable] Rails/CompactBlank: Use compact_blank instead. }.reject { |_k, _v| _v.blank? } ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ spec/pagination_spec.rb:160:15: C: [Correctable] Rails/CompactBlank: Use compact_blank instead. }.reject { |_k, _v| _v.blank? } ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 18 files inspected, 2 offenses detected, 2 offenses autocorrectable ``` --- spec/pagination_spec.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/spec/pagination_spec.rb b/spec/pagination_spec.rb index ec1c3c4..bdc5fcc 100644 --- a/spec/pagination_spec.rb +++ b/spec/pagination_spec.rb @@ -53,6 +53,8 @@ context 'on page 2 out of 3' do let(:as_list) { } + # .compact_blank is not available in our Rails version + # rubocop:disable Rails/CompactBlank let(:params) do { page: { number: 2, size: 1 }, @@ -60,6 +62,7 @@ as_list: as_list }.reject { |_k, _v| _v.blank? } end + # rubocop:enable Rails/CompactBlank context 'on an array of resources' do let(:as_list) { true } @@ -153,12 +156,15 @@ context 'on paging beyond the last page' do let(:as_list) { } + # .compact_blank is not available in our Rails version + # rubocop:disable Rails/CompactBlank let(:params) do { page: { number: 5, size: 1 }, as_list: as_list }.reject { |_k, _v| _v.blank? } end + # rubocop:enable Rails/CompactBlank context 'on an array of resources' do let(:as_list) { true }