From 4b0c230d76deff599ad203d59159158f60c15e79 Mon Sep 17 00:00:00 2001 From: Mark Havekes Date: Tue, 4 Jun 2019 10:27:02 +0200 Subject: [PATCH 1/6] add failing test and suggestion --- .../adapter/json_api/relationship.rb | 1 + test/adapter/json_api/relationship_test.rb | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/lib/active_model_serializers/adapter/json_api/relationship.rb b/lib/active_model_serializers/adapter/json_api/relationship.rb index 2879e3eb..f26dfbd4 100644 --- a/lib/active_model_serializers/adapter/json_api/relationship.rb +++ b/lib/active_model_serializers/adapter/json_api/relationship.rb @@ -55,6 +55,7 @@ module ActiveModelSerializers else association.reflection.type.to_s end + # TODO: probably return nil if association.object is nil? ResourceIdentifier.for_type_with_id(type, id, serializable_resource_options) else # TODO(BF): Process relationship without evaluating lazy_association diff --git a/test/adapter/json_api/relationship_test.rb b/test/adapter/json_api/relationship_test.rb index 35069537..9a21f0a7 100644 --- a/test/adapter/json_api/relationship_test.rb +++ b/test/adapter/json_api/relationship_test.rb @@ -35,6 +35,20 @@ module ActiveModelSerializers assert_equal(expected, actual) end + def test_relationship_with_nil_model_and_belongs_to_id_on_self + ActiveModelSerializers.config.jsonapi_use_foreign_key_on_belongs_to_relationship = true + + expected = { data: nil } + + model_attributes = { blog: nil } + relationship_name = :blog + model = new_model(model_attributes) + actual = build_serializer_and_serialize_relationship(model, relationship_name) do + belongs_to :blog + end + assert_equal(expected, actual) + end + def test_relationship_with_data_array expected = { data: [ From 28a172e66a4473fe49cd66e7574ca6c01b4e5362 Mon Sep 17 00:00:00 2001 From: Mark Havekes Date: Fri, 7 Jun 2019 12:17:18 +0200 Subject: [PATCH 2/6] return nil if id is nil --- lib/active_model_serializers/adapter/json_api/relationship.rb | 1 - .../adapter/json_api/resource_identifier.rb | 2 +- test/adapter/json_api/relationship_test.rb | 4 ++++ test/adapter/json_api/type_test.rb | 3 +-- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/active_model_serializers/adapter/json_api/relationship.rb b/lib/active_model_serializers/adapter/json_api/relationship.rb index f26dfbd4..2879e3eb 100644 --- a/lib/active_model_serializers/adapter/json_api/relationship.rb +++ b/lib/active_model_serializers/adapter/json_api/relationship.rb @@ -55,7 +55,6 @@ module ActiveModelSerializers else association.reflection.type.to_s end - # TODO: probably return nil if association.object is nil? ResourceIdentifier.for_type_with_id(type, id, serializable_resource_options) else # TODO(BF): Process relationship without evaluating lazy_association diff --git a/lib/active_model_serializers/adapter/json_api/resource_identifier.rb b/lib/active_model_serializers/adapter/json_api/resource_identifier.rb index 8aee0a7d..e321914e 100644 --- a/lib/active_model_serializers/adapter/json_api/resource_identifier.rb +++ b/lib/active_model_serializers/adapter/json_api/resource_identifier.rb @@ -13,7 +13,7 @@ module ActiveModelSerializers type = inflect_type(type) type = type_for(:no_class_needed, type, options) if id.blank? - { type: type } + nil else { id: id.to_s, type: type } end diff --git a/test/adapter/json_api/relationship_test.rb b/test/adapter/json_api/relationship_test.rb index 9a21f0a7..38a67fe2 100644 --- a/test/adapter/json_api/relationship_test.rb +++ b/test/adapter/json_api/relationship_test.rb @@ -36,6 +36,7 @@ module ActiveModelSerializers end def test_relationship_with_nil_model_and_belongs_to_id_on_self + original_config = ActiveModelSerializers.config.jsonapi_use_foreign_key_on_belongs_to_relationship ActiveModelSerializers.config.jsonapi_use_foreign_key_on_belongs_to_relationship = true expected = { data: nil } @@ -46,7 +47,10 @@ module ActiveModelSerializers actual = build_serializer_and_serialize_relationship(model, relationship_name) do belongs_to :blog end + assert_equal(expected, actual) + ensure + ActiveModelSerializers.config.jsonapi_use_foreign_key_on_belongs_to_relationship = original_config end def test_relationship_with_data_array diff --git a/test/adapter/json_api/type_test.rb b/test/adapter/json_api/type_test.rb index 8a7aaa28..56f7efcb 100644 --- a/test/adapter/json_api/type_test.rb +++ b/test/adapter/json_api/type_test.rb @@ -147,8 +147,7 @@ module ActiveModelSerializers def test_for_type_with_id_given_blank_id id = '' actual = ResourceIdentifier.for_type_with_id('admin_user', id, {}) - expected = { type: 'admin-users' } - assert_equal actual, expected + assert_nil actual end def test_for_type_with_id_inflected From 58f4f98e41200662f5f899b96dfbd84d7aa003ef Mon Sep 17 00:00:00 2001 From: Mark Havekes Date: Tue, 11 Jun 2019 23:23:41 +0200 Subject: [PATCH 3/6] lock jsonapi renderer to < 0.2.1 --- active_model_serializers.gemspec | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/active_model_serializers.gemspec b/active_model_serializers.gemspec index a29ef9dc..5e8c030a 100644 --- a/active_model_serializers.gemspec +++ b/active_model_serializers.gemspec @@ -42,7 +42,8 @@ Gem::Specification.new do |spec| spec.add_development_dependency 'minitest', ['~> 5.0', '< 5.11'] # 'thread_safe' - spec.add_runtime_dependency 'jsonapi-renderer', ['>= 0.1.1.beta1', '< 0.3'] + # this is locked to < 0.2.1 untill `jsonapi-rb/jsonapi-renderer#38` gets merged and released + spec.add_runtime_dependency 'jsonapi-renderer', ['>= 0.1.1.beta1', '< 0.2.1'] spec.add_runtime_dependency 'case_transform', '>= 0.2' spec.add_development_dependency 'activerecord', rails_versions From cee0ad4494f62748246ed375ac5718c860c86fdd Mon Sep 17 00:00:00 2001 From: Mark Havekes Date: Thu, 13 Jun 2019 10:52:44 +0200 Subject: [PATCH 4/6] update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff86d345..5fa42f9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ Fixes: - Fix Rails 6.0 deprication warnings - update test fixture schema to use `timestamps` instead of `timestamp` - [#2223](https://github.com/rails-api/active_model_serializers/pull/2223) Support Fieldset in Attributes/JSON adapters documented in [docs/general/fields.md](https://github.com/rails-api/active_model_serializers/blob/0-10-stable/docs/general/fields.md) that worked partially before (@bf4) +- [#2337](https://github.com/rails-api/active_model_serializers/pull/2337) fix incorrect belongs_to serialization when foreign_key on object and belongs_to is blank (@InteNs) + - Fixes incorrect json-api generation when `jsonapi_use_foreign_key_on_belongs_to_relationship` is `true` and the relationship is blank Misc: From 4022337966a13ef78a5bee0347cd5a3d295c8af1 Mon Sep 17 00:00:00 2001 From: Mark Havekes Date: Thu, 13 Jun 2019 15:49:55 +0200 Subject: [PATCH 5/6] Revert "lock jsonapi renderer to < 0.2.1" This reverts commit 58f4f98e41200662f5f899b96dfbd84d7aa003ef. --- active_model_serializers.gemspec | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/active_model_serializers.gemspec b/active_model_serializers.gemspec index 5e8c030a..a29ef9dc 100644 --- a/active_model_serializers.gemspec +++ b/active_model_serializers.gemspec @@ -42,8 +42,7 @@ Gem::Specification.new do |spec| spec.add_development_dependency 'minitest', ['~> 5.0', '< 5.11'] # 'thread_safe' - # this is locked to < 0.2.1 untill `jsonapi-rb/jsonapi-renderer#38` gets merged and released - spec.add_runtime_dependency 'jsonapi-renderer', ['>= 0.1.1.beta1', '< 0.2.1'] + spec.add_runtime_dependency 'jsonapi-renderer', ['>= 0.1.1.beta1', '< 0.3'] spec.add_runtime_dependency 'case_transform', '>= 0.2' spec.add_development_dependency 'activerecord', rails_versions From 22849c0399bce1c5481e829f5386d15a163eb05d Mon Sep 17 00:00:00 2001 From: Mark Havekes Date: Fri, 14 Jun 2019 09:52:16 +0200 Subject: [PATCH 6/6] fix include in failing test --- test/action_controller/json_api/linked_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/action_controller/json_api/linked_test.rb b/test/action_controller/json_api/linked_test.rb index 3e38e112..69d8974d 100644 --- a/test/action_controller/json_api/linked_test.rb +++ b/test/action_controller/json_api/linked_test.rb @@ -82,7 +82,7 @@ module ActionController def render_collection_with_include setup_post - render json: [@post], adapter: :json_api, include: 'author, comments' + render json: [@post], adapter: :json_api, include: 'author,comments' end end