Merge pull request #1232 from beauby/move-root-to-adapter

Extract attributes filtering from serializer into adapter.

CHANGELOG.md

@@ -24,6 +24,7 @@ Fixes:
 - [#1214](https://github.com/rails-api/active_model_serializers/pull/1214) retrieve the key from the reflection options when building associations (@NullVoxPopuli, @hut8)
 
 Misc:
+- [#1232](https://github.com/rails-api/active_model_serializers/pull/1232) fields option no longer handled at serializer level (@beauby)
 - [#1178](https://github.com/rails-api/active_model_serializers/pull/1178) env CAPTURE_STDERR=false lets devs see hard failures (@bf4)
 - [#1177](https://github.com/rails-api/active_model_serializers/pull/1177) Remove Adapter autoloads in favor of require (@bf4)
 - [#1117](https://github.com/rails-api/active_model_serializers/pull/1117) FlattenJson adapter no longer inherits Json adapter, renamed to Attributes (@bf4)

lib/active_model/serializer.rb

@@ -144,13 +144,8 @@ module ActiveModel
       root || object.class.model_name.to_s.underscore
     end
 
-    def attributes(options = {})
-      attributes =
-        if options[:fields]
-          self.class._attributes & options[:fields]
-        else
-          self.class._attributes.dup
-        end
+    def attributes
+      attributes = self.class._attributes.dup
 
       attributes.each_with_object({}) do |name, hash|
         unless self.class._fragmented

lib/active_model/serializer/adapter/attributes.rb

@@ -57,7 +57,10 @@ module ActiveModel
 
         def resource_object_for(options)
           cache_check(serializer) do
-            serializer.attributes(options)
+            attributes = serializer.attributes
+            attributes.slice!(*options[:fields]) if options[:fields]
+
+            attributes
           end
         end
       end

lib/active_model/serializer/adapter/json_api.rb

@@ -47,7 +47,7 @@ module ActiveModel
 
           fields = options.delete(:fields)
           if fields
-            @fieldset = ActiveModel::Serializer::Fieldset.new(fields, serializer.json_key)
+            @fieldset = ActiveModel::Serializer::Fieldset.new(fields)
           else
             @fieldset = options[:fieldset]
           end
@@ -60,7 +60,7 @@ module ActiveModel
             if serializer.respond_to?(:each)
               serializable_hash_for_collection(options)
             else
-              serializable_hash_for_single_resource(options)
+              serializable_hash_for_single_resource
             end
 
           ApiObjects::JsonApi.add!(hash)
@@ -99,8 +99,8 @@ module ActiveModel
           hash
         end
 
-        def serializable_hash_for_single_resource(options)
-          primary_data = primary_data_for(serializer, options)
+        def serializable_hash_for_single_resource
+          primary_data = primary_data_for(serializer)
           relationships = relationships_for(serializer)
           included = included_resources(@include_tree)
           hash = { data: primary_data }
@@ -134,22 +134,22 @@ module ActiveModel
           { id: id.to_s, type: type }
         end
 
-        def resource_object_for(serializer, options = {})
-          options[:fields] = fieldset && fieldset.fields_for(serializer)
-
+        def resource_object_for(serializer)
           cache_check(serializer) do
-            result = resource_identifier_for(serializer)
-            attributes = serializer.attributes(options).except(:id)
-            result[:attributes] = attributes if attributes.any?
-            result
+            resource_object = resource_identifier_for(serializer)
+            requested_fields = fieldset && fieldset.fields_for(resource_object[:type])
+            attributes = serializer.attributes.except(:id)
+            attributes.slice!(*requested_fields) if requested_fields
+            resource_object[:attributes] = attributes if attributes.any?
+            resource_object
           end
         end
 
-        def primary_data_for(serializer, options)
+        def primary_data_for(serializer)
           if serializer.respond_to?(:each)
-            serializer.map { |s| resource_object_for(s, options) }
+            serializer.map { |s| resource_object_for(s) }
           else
-            resource_object_for(serializer, options)
+            resource_object_for(serializer)
           end
         end
 
@@ -187,7 +187,7 @@ module ActiveModel
           else
             return unless serializer && serializer.object
 
-            primary_data = primary_data_for(serializer, instance_options)
+            primary_data = primary_data_for(serializer)
             relationships = relationships_for(serializer)
             primary_data[:relationships] = relationships if relationships.any?
 

lib/active_model/serializer/fieldset.rb

@@ -1,8 +1,7 @@
 module ActiveModel
   class Serializer
     class Fieldset
-      def initialize(fields, root = nil)
-        @root       = root
+      def initialize(fields)
         @raw_fields = fields
       end
 
@@ -10,27 +9,19 @@ module ActiveModel
         @fields ||= parsed_fields
       end
 
-      def fields_for(serializer)
-        key = serializer.json_key
-        fields[key.to_sym] || fields[key.pluralize.to_sym]
+      def fields_for(type)
+        fields[type.singularize.to_sym] || fields[type.pluralize.to_sym]
       end
 
       private
 
       ActiveModelSerializers.silence_warnings do
-        attr_reader :raw_fields, :root
+        attr_reader :raw_fields
       end
 
       def parsed_fields
         if raw_fields.is_a?(Hash)
           raw_fields.inject({}) { |h, (k, v)| h[k.to_sym] = v.map(&:to_sym); h }
-        elsif raw_fields.is_a?(Array)
-          if root.nil?
-            raise ArgumentError, 'The root argument must be specified if the fields argument is an array.'.freeze
-          end
-          hash = {}
-          hash[root.to_sym] = raw_fields.map(&:to_sym)
-          hash
         else
           {}
         end

test/adapter/json_api/collection_test.rb

@@ -58,8 +58,10 @@ module ActiveModel
           end
 
           def test_limiting_fields
-            @adapter = ActiveModel::Serializer::Adapter::JsonApi.new(@serializer, fields: ['title'])
-
+            actual = ActiveModel::SerializableResource.new(
+              [@first_post, @second_post], adapter: :json_api,
+                                           fields: { posts: ['title'] })
+                     .serializable_hash
             expected = [
               {
                 id: '1',
@@ -86,7 +88,7 @@ module ActiveModel
                 }
               }
             ]
-            assert_equal(expected, @adapter.serializable_hash[:data])
+            assert_equal(expected, actual[:data])
           end
         end
       end

test/serializers/attributes_test.rb

@@ -18,11 +18,6 @@ module ActiveModel
           @profile_serializer.class._attributes)
       end
 
-      def test_attributes_with_fields_option
-        assert_equal({ name: 'Name 1' },
-          @profile_serializer.attributes(fields: [:name]))
-      end
-
       def test_attributes_inheritance_definition
         assert_equal([:id, :body], @serializer_klass._attributes)
       end

test/serializers/fieldset_test.rb

@@ -4,22 +4,11 @@ module ActiveModel
   class Serializer
     class FieldsetTest < Minitest::Test
       def test_fieldset_with_hash
-        fieldset = ActiveModel::Serializer::Fieldset.new({ 'post' => %w(id title), 'coment' => ['body'] })
+        fieldset = ActiveModel::Serializer::Fieldset.new('post' => %w(id title), 'comment' => ['body'])
+        expected = { :post => [:id, :title], :comment => [:body] }
 
-        assert_equal(
-          { :post => [:id, :title], :coment => [:body] },
-          fieldset.fields
-        )
-      end
-
-      def test_fieldset_with_array_of_fields_and_root_name
-        fieldset = ActiveModel::Serializer::Fieldset.new(['title'], 'post')
-
-        assert_equal(
-          { :post => [:title] },
-          fieldset.fields
-        )
+        assert_equal(expected, fieldset.fields)
       end
     end
   end
-end
+end