Enable filtering of fields in nested assocation

[south37]

Purpose

Filtering nested association's fields.

Changes

Consider the case in which User, Profile, UserSerializer, ProfileSerializer are defined as below.

class User < ApplicationRecord
  has_one :profile
end

class Profile < ApplicationRecord
  belongs_to :user
end

class UserSerializer < ActiveModel::Serializer
  attributes :id, :registered

  has_one :profile
end

class ProfileSerializer < ActiveModel::Serializer
  attributes :id, :location, :birthday
end

user is defined as below.

[2] pry(main)> user = User.new(id: 1, registered: true)
=> #<User:0x00007fcaab93df78 id: 1, registered: true>

[3] pry(main)> user.profile = Profile.new(id: 2, location: 'united_states', birthday: Date.today)
=> #<Profile:0x00007fcaac567b78 id: 2, location: "united_states", birthday: Mon, 27 Aug 2018>

We compare the result of ActiveModelSerializers::SerializableResource.new(user, adapter: :json, fields: [:registered], include: { profile: { only: [:birthday] } }).serializable_hash.

Before

We cannot filter fields in nested associations, so all fields of profile are returned.

[8] pry(main)> ActiveModelSerializers::SerializableResource.new(user, adapter: :json, fields: [:registered], include: { profile: { only: [:birthday] } }).serializable_hash

Rendered UserSerializer with ActiveModelSerializers::Adapter::Json (0.26ms)
=> {:user=>
  {:registered=>true, :profile=>{:id=>2, :location=>"united_states", :birthday=>Mon, 27 Aug 2018}}}

After

By applying this PR, we can filter fields in nested associations by using only keyword as below.

[3] pry(main)> ActiveModelSerializers::SerializableResource.new(user, adapter: :json, fields: [:registered], include: { profile: { only: [:birthday] } }).serializable_hash

Rendered UserSerializer with ActiveModelSerializers::Adapter::Json (4.44ms)
=> {:user=>{:registered=>true, :profile=>{:birthday=>Mon, 27 Aug 2018}}}

Caveats

Nothing

Related GitHub issues

#1895

Additional helpful information

We use this patch in production.

[bf4]

Have you looked at #2223 ?

[south37]

No, I didn't know #2223 .
It seems to be what I want ! 😄
Do you have any plan to merge it ?

[bf4]

@south37 I need someone to test it against their app as I don't have a live app to run against at this time.

[johan-smits]

@bf4 no one did the testing?

[wasifhossain]

Hi @johan-smits, would you mind writing a solid test to support this PR?

[bf4]

@wasifhossain take a look at #2223 It has a test in it and tries a little harder to unify the adapters by using the 'fieldset' object

[wasifhossain]

@bf4 after reviewing #2223, I think we can close this PR in favor of that

[johan-smits]

@wasifhossain I will try, I'm not familiar with the codebase but will make some time available to add one.

[wasifhossain]

@johan-smits we would appreciate your time and effort. But the fact that #2223 already addressed the issue in a robust way that this PR is trying to achieve, we should better wait for that PR to be merged soon. I think @bf4 can confirm the case.

Thanks.

[johan-smits]

@wasifhossain Thats sounds good, will wait for the PR to merge, will this also become soon in a gem release?

[wasifhossain]

Hi @johan-smits, can you please try the changes on #2223 for the purpose and share your valuable feedback with us in that thread?

Receiving positive feedback would help us merge that change with confidence :)

[johan-smits]

@wasifhossain I'll continue my feedback on the #2223 issue

[wasifhossain]

closing this as #2223 is merged