Pass fields down from constructor (MERGED)

[vasilakisfil]

Passes down the fields option to the serializer from the constructor unless it's overridden.

[beauby]

Could you expand on the reasons you think this is something we should have?
@bf4 I think we talked about docs describing which options belong to what object/method. I just can't remember whether we ended up doing it or not.

[vasilakisfil]

Well decide, either this #1286 is a bug or this #1288 should be reopened and specify in the Readme what goes where. At the moment default Rails render passes everything to everywhere.

[beauby]

@vasilakisfil As @bf4 explained, this is by design, the options are then split and adapter options are directed towards the adapter.

[vasilakisfil]

Wait something you haven't understood obviously. Let's take it step by step.
Are fields param supposed to be on ActiveModel::SerializableResource constructor or on serializable_hash method ? Both is not an answer :)

[beauby]

@vasilakisfil First option: https://github.com/rails-api/active_model_serializers/blob/master/lib/action_controller/serialization.rb#L28

[vasilakisfil]

Ok then we need to fix 2 things:

1. Fields is not passed down from constructor to serializer. This pull request fixes it and this Constructor does not pass down the options to serializer #1286 shows that there is a bug.
2. We need to separate what is passed where in the default Rails render method, here: https://github.com/rails-api/active_model_serializers/blob/master/lib/action_controller/serialization.rb#L50-L51 The fact that it passes everything to everywhere is why fields seems to work but it doesn't.

[beauby]

1. fields is an adapter-level option. It is passed to the adapter.
2. I agree that the situation is not the clearest. @bf4 thoughts?

[vasilakisfil]

About (1) if it's an adapter option, why #1286 fails ?

[bf4]

do you mean options[:fields] ||= instance_options[:fields]?

[vasilakisfil]

yeah that's better I will fix it!

[bf4]

@vasilakisfil let's finish this?

[beauby]

The actual bug about 1) is that the fields option gets passed down to associations whereas it should not be.

[vasilakisfil]

No that's another bug. See #1286 tests for the bug I am telling you.

[beauby]

You are right, this should really be querying instance_options instead of options.

[vasilakisfil]

resource_object_for is also called for the associations so that won't fix the bug that fields is being applied to the associations too (tested it locally and it fails).

[beauby]

There are 2 bugs:

1. The fields option gets passed down to associations,
2. The fields option is looked for in options (which are really the serializable_hash options) instead of instance_options which are the adapter options.

[NullVoxPopuli]

So, @beauby, is #2 partially addressed by this PR?

[bf4]

@beauby @NullVoxPopuli @vasilakisfil I just merged this but on review #1287 (comment) did I misunderstand the conversation? (It's late and I'm tired, so delegating thoughts)