:if option for attribute method

[Le6ow5k1]

I think it would be nice to add an :if option for attribute method, so attribute will appear only if result of calling lambda specified at :if key is true.

class ActivitySerializer < ActiveModel::Serializer
  attribute :complete_till, if: -> { object.type == 'Task' }
end

[joaomdmoura]

@Le6ow5k1 It isn't the first time I hear this proposition.
I'm not sure how I feel about it once that I usually prefer using a method to overwrite the attribute.
It would also imply in some refactoring on fragment cache.
Is there other benefit, apart of syntax?

[Le6ow5k1]

@joaomdmoura didn't understand how it can be done using method to overwrite attribute. The only way to solve this problem that I found is to use filter method to filter out unwanted keys. But this syntax is probably better.

[joaomdmoura]

@Le6ow5k1 this would be an example.

class ActivitySerializer < ActiveModel::Serializer
  attribute :complete_till

  def complete_till
    if object.type == 'Task'
      # Logic here
    end
  end
end

[Le6ow5k1]

@joaomdmoura but this gives me { "complete_till": null } if object.type == 'Task'. Instead I want to eliminate this key from response, so for your example response would be {} if object.type == 'Task'.

My current approach:

class ActivitySerializer < ActiveModel::Serializer
  attribute :complete_till

  def type
    # logic to determine type
  end

  def filter(keys)
    case type
    when 'Task'
      keys
    else
      keys - [:complete_till]
    end
  end
end

[saneshark]

Yea, while I like the :if syntax for readability, I think it could be argued that this approach violates some of the principles of SOLID. I'm not sure a serializer should ever conditionally remove an attribute, instead, that attribute should be NULL. If you don't want the attribute there at all, then perhaps a different serializer is what you want, as any attributes that exist in the serializer are meant to be behaviors that that class respond to.

While the same could be attributed to an association, in that context I think it's OK, particularly if you're working with a collection and need to avoid N+1 in the event that object.association is not loaded.

[ankit8898]

@Le6ow5k1 Does your filter method gets called.. i tried the same approach with master but filter is not called ?

[Le6ow5k1]

@ankit8898 Yes, but I'm using 0.9.3 version.

[Le6ow5k1]

So I guess for 0.10.x version there's no solution for such problem. Would like to hear from repository owners how they feel about my proposal.

[joaomdmoura]

@Le6ow5k1 indeed, as @saneshark wrote, in the actual architecture using 2 different serializer might be what fits you better.

... @Le6ow5k1 okay cc/ @kurko

[kurko]

1. I'd say that we definitely need filter implemented.
2. as for :if, it's not something I think I'd personally use. To implement that, I think we should get a lot more people backing this up.