Allow options on multiple attributes at once

[nicklandgrebe]

Purpose

Add options to Serializer.attributes so we can apply conditions to multiple attributes at once.

Changes

Check if last item in attrs passed into Serializer.attributes has condition class == Hash, and if so, pop it off attrs, then pass it into attribute(attr, options) every time attrs is iterated over.

Before, only attribute(attr) would be called every time attrs is iterated over.

Caveats

Related GitHub issues

#1403 (comment)

Additional helpful information

[nicklandgrebe]

Heyo, I added a test for the lines changed. Apologies for not doing this in the first place! Didn't see a case till now.

[groyoh]

I guess something like would be better:

+          options = attrs.last.class == Hash ? attrs.pop : {}
+          options = options.except(:key)

At least, we prevent ourselves from having unintended behavior.

[remear]

Note that the CHANGELOG has changed in master recently with the release of 0.10.0. This PR needs to be updated to move the entry to the new master section. The commits also need to be squashed.

[zhen6939]

Hey, @nicklandgrebe.
I am using your post in my code but it has a problem.
If there are multiple attributes called, duplicated attributes would be ignored in false conditions, for example:

class CandidateSerializer < ActiveModel::Serializer
  def self.attributes(*attrs)
    binding.pry
    options = attrs.last.class == Hash ? attrs.pop : {}
    options = options.except(:key)

    attrs = attrs.first if attrs.first.class == Array

    attrs.each do |attr|
      attribute(attr, options)
    end
  end

  attributes %I(university resume paper_test_score), if: :in_paper_test?
  attributes %I(university resume homework), if: :in_homework?

  def in_paper_test
    true
  end

  def in_home_work
    false
  end
end

It returns

...
:attributes=>{:name=>"zhangsan", :cellphone=>"10000000001", :email=>"[email protected]", :gender=>false, :"paper_test_score"=>100},
...

Attributes :university and :resume are ignored since they have false condition in second attributes called. In other words, they are overwritten in attribute(attr, options) .

I think the problem is caused at

def attribute(attr, options = {}, &block)
  key = options.fetch(:key, attr)
  _attributes_data[key] = Attribute.new(attr, options, block)
end

Only one attribute instance for one key so that the following duplicated attributes would replace the previous one.

You did a greate job, and let's make it perfect.

[groyoh]

@zhen639 attributes are not really meant to be (re)defined multiple times (at least that meither tested or implemented AFAIK). I would suggest:

class CandidateSerializer < ActiveModel::Serializer
  attribute :paper_test_score, if: :in_paper_test?
  attribute :homework, if: :in_homework?
  attributes %I(university resume), if: :in_paper_test_and_homework?

  def in_paper_test_and_homework?
    in_paper_test? && in_homework?
  end

  def in_paper_test?
    true
  end

  def in_home_work?
    false
  end
end

[bf4]

good call. I wonder if this could be supported via activesupport with_options and no code changes?

[bf4]

attributes are not really meant to be (re)defined multiple times

This is definitely a scenario ams does not protect against

[beauby]

@bf4 I think it (AMS) should (protect against attributes redefinition).

[adin-ttc]

Anything happening with this? i have a workaround but it would be nice :)

[bf4]

basic workaround is if you are

attributes :university, :resume, :paper_test_score, if: :in_paper_test?

then you can

attribute :university, if: :in_paper_test?
attribute :resume, if: :in_paper_test?
attribute :paper_test_score, if: :in_paper_test?

other work around is to use composition, inheritance, or options...

[bf4]

PR needs to be resubmitted against 0-10-stable.

[nicklandgrebe]

Resubmitted as #2148 against 0-10-stable. This totally fell off my radar, thanks for the push!