rails-api / active_model_serializers

ActiveModel::Serializer implementation and Rails hooks
MIT License
5.33k stars 1.39k forks source link

Optional attributes / Passing arbitrary params to serializer #599

Closed kenn closed 8 years ago

kenn commented 10 years ago

(from #552) I just realized that AMS is in flux. But I'd throw in my issue (based on 0.9), anyway.

We have a master-detail view where the master shows a list of conversations with the last message included, and the detail displays one conversation with all messages.

One way to do it is:

class Conversation
  has_many :messages
  has_one :last_message
  attr_accessor :show_details
end

class ConversationSerializer < ActiveModel::Serializer
  attributes :last_message, :messages

  def attributes
    if object.show_details
      super.except(:last_message)
    else
      super.except(:messages)
    end
  end
end

# controller
@conversation.show_details = true
render json: @conversation

But this feels wrong on two fronts:

For those reason, I would suggest that we enable to pass arbitrary parameters to the serializer, from controller.

class ConversationSerializer < ActiveModel::Serializer
  params :show_details

  def attributes
    if @show_details
      super.except(:last_message)
    else
      super.except(:messages)
    end
  end
end

# controller
render json: @conversation, serializer_params: { show_details: true }

and pass down the params to each item, when the target was an array.

That way, we don't have to alter the model and separation can be done nicely. What do you think?

kenn commented 10 years ago

This feature will ensure more flexibility - like pagination in embedded objects.

class ConversationSerializer < ActiveModel::Serializer
  params :show_details, :page, :per_page

  def attributes
    if @show_details
      super.merge(messages: object.messages.page(@page).per(@per_page))
    else
      super.merge(last_message: object.last_message)
    end
  end
end

# controller
render json: @conversation, serializer_params: { show_details: true, page: params[:page], per_page: params[:per_page] }
tchak commented 10 years ago

in case of full/detail view I would suggest using two serializers with a common shared parent if needed and then do :

render json: @conversation, serializer: ConversationDetailSerializer

Do you have any other use cases?

kenn commented 10 years ago

@tchak I don't like that approach (as mentioned in #552), because it requires at least 3 classes to achieve DRY-ness just to handle one optional attribute. It quickly gets out of control when you have more optional behaviors.

Let's assume that you have a user class, which has limited accessibility for the public profile, and expose more data for your friends. The class also has "following" and "followed" flag, which represents relationship, but those relationship attirbutes won't be included if it was called from yourself, or you're not logged in. When it's called from yourself, the class includes "email_confirmed", to display alert if the email hasn't been confirmed yet.

class UserSerializer < ActiveModel::Serializer
  attributes :id, :username, :created_at, ...

  params :authorization, :include_details

  def attributes
    base_attributes = @include_details ? super.merge(details: object.details) : super

    case @authorization
    when :public
      base_attributes
    when :myself
      base_attributes.merge(email_confirmed: object.email_confirmed?)
    when :loggedin
      base_attributes.merge(following: scope.following?(object), followed: scope.followed?(object))
    when :friends
      base_attributes.merge(following: scope.following?(object), followed: scope.followed?(object), friends_only_data: object.friends_only_data)
    end
  end
end

# controller
render json: @user, serializer_params: { include_details: true, authorization: :friends }

With this fairly simple example, you already need 9 classes if you go with the inheritance way. We need a way to manage combinatorial explosion.

kenn commented 10 years ago

Yet another example, a picture model using paperclip, that automatically choose the best thumbnail for a given numeric size.

class Picture < ActiveRecord::Base
  has_attached_file :data, { tiny: '32x32^', small: '48x48^', ... }
end

class PictureSerializer < ActiveModel::Serializer
  attributes :url, :content_type
  params :size

  def url
    object.data.url(thumbnail_key)
  end

  def thumbnail_key
    case @size
    when 0..32
      :tiny
    when 33..48
      :small
    ...
    else
      :original
    end
  end
end

# controller
render json: @pictures, serializer_params: { size: 100 }

One nice thing about this design is that controller's params can be handled by strong_parameters or rails_param, then mass-assigned.

render json: @pictures, serializer_params: params.permit(:size)
kenn commented 10 years ago

Another way of doing it might be accepting a serializer instance rather than a class:

render json: @pictures, serializer: PictureSerializer.new(params.permit(:size))
dilizarov commented 9 years ago

Just noticed this, and think this addition is needed for flexibility. I love this gem, but it would be helpful to easily bring in data that is indirectly related to the model. For example, imagine showing a user a list of his or her clubs, and showing the number of users in each club. It is easy to get the number of users for each club like such: UserClub.group_by(:club_id).count(:user_id), but then if you want to serialize a Club with the attribute :number_of_users, it isn't easy.

The following is a solution, but would lead to N + 1 Queries. Ultimately, one would like to be able to pass in the value from the outside.

class ClubSerializer < ActiveModel::Serializer
  attributes :name, :creator_id, :number_of_users

  def number_of_users
    UserClub.group_by(:club_id).count(:user_id)[object.id]
  end
end

Also, one could include this information in the meta of the JSON, but that doesn't feel right either.

jbhatab commented 9 years ago

I have a scenario where I need params as well. I need to get the amount of weight a user lost based on a date range. I can't think of any other way to do this without passing in params.

dilizarov commented 9 years ago

@jbhatab, I found a solution, which I feel is a tad hacky, but works.

If you reference my post, right above yours, you see that I need the number_of_users per club. One way to do it is the following:

class Club < ActiveRecord::Base
  attr_accessor :number_of_users

  #Whatever else you would have in your model
end

Then, before you try to serialize it, you would take your object, club, and simply give it the value it needs based on calculations you did elsewhere. So, you'd do something like club.number_of_users = 59.

Once you have that, in your serializer, do something like:

class ClubSerializer < ActiveModel::Serializer
  attributes :name, :creator_id

  def attributes
    data = super
    data[:number_of_users] = object.number_of_users unless object.number_of_users.nil?
    data
  end
end
kenn commented 9 years ago

@dilizarov @jbhatab It is demonstrated in my original post. :smile: Still, we need a way to separate view concern / serializer concern from models.

dilizarov commented 9 years ago

@kenn Oh, I didn't notice that you already showed that method. I think we're all in agreement though. Params would be awesome.

jbhatab commented 9 years ago

@dilizarov @kenn Thanks for the temp fix. The documentation on this issue is all over the place. The github has closed issues that seem like it fixed it, open issues that say it's a problem, and stack overflow posts that say it's answered but it's not. Just letting you guys know.

What is the guy in this post referencing? Isn't this "params"? Because it doesn't work.

http://stackoverflow.com/questions/23347587/how-to-pass-parameters-to-activemodel-serializer

jbhatab commented 9 years ago

Also can't I just do the following if I'm using attr_accessor instead of the attributes super thing.

attributes :number_of_users

That should get the correct value and put it in the serialized data correctly.

kenn commented 9 years ago

Looks like a patch has been merged to master. https://github.com/rails-api/active_model_serializers/pull/679 I would prefer something shorter than serialization_options[:show_details], though.

I'll try to cook up a pull request if we like params macro and ivar assignment better as shown in my comments above. WDYT? @steveklabnik

dilizarov commented 9 years ago

@jbhatab Sure, but for API requests that don't need the number_of_users, you'll have the :number_of_users attribute be nil for every club. My method opts out of that.

jbhatab commented 9 years ago

ok I see. And I tried the whole serialization_options and that didn't work. Just letting you know. I was using version .90

jbhatab commented 9 years ago

For some reason the master branch says I'm on version .90, but I just switched explicitly to the '0-9-stable' branch and serialization_options works now. Just wanted to let you guys know. Here's the line in my gem file,

gem 'active_model_serializers', git: 'https://github.com/rails-api/active_model_serializers.git', branch: '0-9-stable'
steveklabnik commented 9 years ago

It's probably because you haven't updated in a while. Master used to be 0.9.0.

jbhatab commented 9 years ago

I only started this project 2 weeks ago. I was using master, but serialization_options doesnt work on master. What version is master?

kriskhaira commented 9 years ago

@jbhatab I had the same problem; and ended up downgrading to 0.8.x and used @options instead. http://stackoverflow.com/questions/23347587/how-to-pass-parameters-to-activemodel-serializer

I had another issue with 0.9.x anyway with infinite loops. @steveklabnik even called 0.8.x a "dead end" here because 0.10.x will be based on 0.9.x: https://github.com/rails-api/active_model_serializers/issues/670#issuecomment-59813913

manuelmeurer commented 9 years ago

@kriskhaira He's calling 0.9 a dead end, not 0.8.

0.9 is sorta its own thing, a dead end.

steveklabnik commented 9 years ago

This is correct. 0.10 is based on 0.8, not 0.9.

kriskhaira commented 9 years ago

@steveklabnik @manuelmeurer Yeah you're right

toobulkeh commented 9 years ago

:+1:

jszwedko commented 9 years ago

Is there a recommended way to do this with 0.10? I see https://github.com/rails-api/active_model_serializers/pull/679 was merged in after this issue was opened, but since 0.10 is based on 0.8 it is not in master.

jszwedko commented 9 years ago

Never mind, found it under instance_options :smile:

bf4 commented 9 years ago

@jszwedko do you have an example of what you did? This issue has great discussion in it..

jszwedko commented 9 years ago

@bf4

Sure thing, I did something like:

PostController

...
  def show 
    render json: @post, user_id: 1234
  end
...

PostSerializer

...
  def my_comments
    Comments.where(user_id: instance_options[:user_id], post_id: object.id)
  end
...
bf4 commented 9 years ago

ah, @jszwedko awesome, very clever use of all options going to the serializer unless the adapter eats uses them

davidwparker commented 9 years ago

Thanks @jszwedko - exactly what I needed!

victormier commented 8 years ago

Any solution for 0.10? instance_options are empty when I do this:

render json: @post, user_id: 1234
bf4 commented 8 years ago

@jszwedko I think everyone would :100: if you were to write a documentation PR for this... :smile: or maybe even with tests :)

cickes commented 8 years ago

I too receive undefined local variable or method 'instance_options' when trying render json: @post, user_id: 1234 with v0.10.0. Can anyone help to explain or point to the correct resource?

beauby commented 8 years ago

@cickes instance_options is only available in the master branch, not in the current RC. In the current RC, you have to use options instead.

dilizarov commented 8 years ago

While instance_options is a method that I think works great, I'd like to say that I really enjoy the attr_accessor way of doing things as well. For example, in a feed of posts, I need to mark if a user has liked each post.

After fetching the feed's posts, I then do something like current_user.mark_liked_posts!(@posts) which for each Post would mark the attr_accessor :liked as false or true. I suppose you could pass in another hash of post ids all with false/true values and access that via instance_options... But I wouldn't look away from the attr_accessor approach just because it feels hacky. It's just Ruby :)

cickes commented 8 years ago

Thanks @beauby. Changing from instance_options to options worked.

kenn commented 8 years ago

@dilizarov your case is one of the "legit" use cases of attr_accessor, as liked sounds like a model concern. But you clearly don't want show_details option in the model, do you? :)

I consider serializer as one of the presenter / decorator patterns, and the view (serializer) concerns should be self-contained.

And considering how often we use them, repetitive @instance_options[:show_details] is an eyesore and I prefer having @show_details or params.show_details instead.

bf4 commented 8 years ago

Closing as I've extracted @jszwedko solution to #1506 to make it easier for new contributors. Also possibly relevant is serialization scope https://github.com/rails-api/active_model_serializers/pull/1252

Malachi-Holden commented 4 years ago

I tried @options, @instance_options, and serialization_options, no luck on any of them. I'm using v 0.10.6. @options and @instance_options are always nil, and serialization_options gives me a undefined local variable or method `serialization_options' Has this changed again? What's the latest term?

Malachi-Holden commented 4 years ago

I also tried instance_options (without the @) and it still didn't work. Undefined method or variable