Remove custom serializer from association options

[stevenharman]

As of SHA: c8cfe94, we now pass the options hash for one serializer down the associations. However, that also means we're now passing along the :serializer and :each_serializer options specified on a has_one or has_many association. This change will delete those values from the options hash when initializing the respective Association object.

[spastorino]

Can you execute reject on options when assigning to @options instead of deleting ?

[stevenharman]

@spastorino Yes, one option would be to reject both the :serializer and :each_serializer keys when initializing a Serializer (

active_model_serializers/lib/active_model/serializer.rb

Line 110 in c8cfe94

@options = options.reject{|k,v| [:root, :meta_key, :meta].include?(k) }

). In fact, that was my first approach upon discovering this bug.

I then changed to the current approach of deleting the :serializer and :each_serializer keys in the Association and ArraySerializer, respectively, as that felt like a more natural place; removing the keys where they are used requires less cognitive load than stripping them out a few classes away.

I'm happy to make the change, but first wanted to give my reasoning.

[spastorino]

I think I'd move both things to Serializer and put that in a constant RESERVED_OPTIONS or something like that. Given that I'm not an english native speaker you will probably come up with a better name for it ;)

[stevenharman]

Out of curiosity, is there a reason you prefer to reject rather than delete them? (I presume to avoid mutating the hash).

A second, more design-oriented, question: Why do you prefer to do the rejection in Serializer as opposed to doing it the spots where they are used (Association and ArraySerializer, specifically)?

EDIT: I'm not trying to be difficult, just trying to get a feel for the style and conventions of this code base so that future contributions might be simpler.

[spastorino]

I think it's better for now if we only pass scope to associations.

[stevenharman]

Well, that's OK I suppose. But I believe the tests were quite useful to prevent this kind of regression in the future. Would you accept and merge a Pull Request that just added the tests from this PR?