Bugfix/redefine associations on inherited serializers (#1848)

* replace reflection collection type with hash to prevent duplicated associations in some cases

* include tests

* Fix robucup offenses

* Improve test

* Remove usless requirement

* improve tests

* remove custom_options option from Post and InheritedPost serializer

* Improve tests

* update changelog

* update changelog

Author: dariush-y

CHANGELOG.md

@@ -16,6 +16,7 @@ Misc:
 
 Fixes:
 - [#1814](https://github.com/rails-api/active_model_serializers/pull/1814) Ensuring read_multi works with fragment cache
+- [#1848](https://github.com/rails-api/active_model_serializers/pull/1848) Redefine associations on inherited serializers. (@EhsanYousefi)
 
 Misc:
 - [#1808](https://github.com/rails-api/active_model_serializers/pull/1808) Adds documentation for `fields` option. (@luizkowalski)

lib/active_model/serializer/associations.rb

@@ -13,7 +13,7 @@ module Associations
       included do
         with_options instance_writer: false, instance_reader: true do |serializer|
           serializer.class_attribute :_reflections
-          self._reflections ||= []
+          self._reflections ||= {}
         end
 
         extend ActiveSupport::Autoload
@@ -74,7 +74,8 @@ def has_one(name, options = {}, &block) # rubocop:disable Style/PredicateName
         # @api private
         #
         def associate(reflection)
-          self._reflections << reflection
+          key = reflection.options[:key]
+          key ? self._reflections[key] = reflection : self._reflections[reflection.name] = reflection
         end
       end
 
@@ -86,7 +87,7 @@ def associations(include_directive = ActiveModelSerializers.default_include_dire
         return unless object
 
         Enumerator.new do |y|
-          self.class._reflections.each do |reflection|
+          self.class._reflections.values.each do |reflection|
             next if reflection.excluded?(self)
             key = reflection.options.fetch(:key, reflection.name)
             next unless include_directive.key?(key)

test/serializers/association_macros_test.rb

@@ -11,7 +11,7 @@ class AssociationsTestSerializer < Serializer
       end
 
       def before_setup
-        @reflections = AssociationsTestSerializer._reflections
+        @reflections = AssociationsTestSerializer._reflections.values
       end
 
       def test_has_one_defines_reflection

test/serializers/associations_test.rb

@@ -1,5 +1,4 @@
 require 'test_helper'
-
 module ActiveModel
   class Serializer
     class AssociationsTest < ActiveSupport::TestCase
@@ -104,13 +103,13 @@ def test_associations_inheritance_with_new_association
         end
 
         assert(
-          PostSerializer._reflections.all? do |reflection|
-            inherited_klass._reflections.include?(reflection)
+          PostSerializer._reflections.values.all? do |reflection|
+            inherited_klass._reflections.values.include?(reflection)
           end
         )
 
         assert(
-          inherited_klass._reflections.any? do |reflection|
+          inherited_klass._reflections.values.any? do |reflection|
             reflection.name == :top_comments
           end
         )
@@ -290,6 +289,61 @@ def test_illegal_conditional_associations
           assert_match(/:if should be a Symbol, String or Proc/, exception.message)
         end
       end
+
+      class InheritedSerializerTest < ActiveSupport::TestCase
+        InheritedPostSerializer = Class.new(PostSerializer) do
+          belongs_to :author, polymorphic: true
+          has_many :comments, key: :reviews
+        end
+
+        InheritedAuthorSerializer = Class.new(AuthorSerializer) do
+          has_many :roles, polymorphic: true
+          has_one :bio, polymorphic: true
+        end
+
+        def setup
+          @author = Author.new(name: 'Steve K.')
+          @post = Post.new(title: 'New Post', body: 'Body')
+          @post_serializer = PostSerializer.new(@post)
+          @author_serializer = AuthorSerializer.new(@author)
+          @inherited_post_serializer = InheritedPostSerializer.new(@post)
+          @inherited_author_serializer = InheritedAuthorSerializer.new(@author)
+          @author_associations = @author_serializer.associations.to_a
+          @inherited_author_associations = @inherited_author_serializer.associations.to_a
+          @post_associations = @post_serializer.associations.to_a
+          @inherited_post_associations = @inherited_post_serializer.associations.to_a
+        end
+
+        test 'an author serializer must have [posts,roles,bio] associations' do
+          expected = [:posts, :roles, :bio].sort
+          result = @author_serializer.associations.map(&:name).sort
+          assert_equal(result, expected)
+        end
+
+        test 'a post serializer must have [author,comments,blog] associations' do
+          expected = [:author, :comments, :blog].sort
+          result = @post_serializer.associations.map(&:name).sort
+          assert_equal(result, expected)
+        end
+
+        test 'a serializer inheriting from another serializer can redefine has_many and has_one associations' do
+          expected = [:roles, :bio].sort
+          result = (@inherited_author_associations - @author_associations).map(&:name).sort
+          assert_equal(result, expected)
+        end
+
+        test 'a serializer inheriting from another serializer can redefine belongs_to associations' do
+          expected = [:author, :comments, :blog].sort
+          result = (@inherited_post_associations - @post_associations).map(&:name).sort
+          assert_equal(result, expected)
+        end
+
+        test 'a serializer inheriting from another serializer can have an additional association with the same name but with different key' do
+          expected = [:author, :comments, :blog, :reviews].sort
+          result = @inherited_post_serializer.associations.map { |a| a.options.fetch(:key, a.name) }.sort
+          assert_equal(result, expected)
+        end
+      end
     end
   end
 end