: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.

[saneshark]

I'd be 👍 but I really do think it's not good practice. It would help people keep things DRY by using the same serializer, but I think that's what inheritance and concerns and such are better suited to do with unique serializer classes.

@kurko I think to your point, it comes down to do we want ActiveModelSerializer to have the flexibility that jbuilder has or do we want to help reinforce some best practices around writing APIs* if it is the former then I am 👍 but if it is the latter (which I prefer), then I'm 👎* By making it more difficult to remove it using filter vs :if it might help reinforce the idea that you shouldn't be doing this type of thing.

[iRet]

+1 for filter
I'm currently using to trim unwanted atributes this code (0.8.3) and passing arguments to searializer through scope:

class BaseSerializer < ActiveModel::Serializer
  def attributes
    return super.extract!(*scope[:only]) if scope.try(:[], :only)
    return super.except(*scope[:except]) if scope.try(:[], :except)
    super
  end
end

That's might be not the best idea to override scope. But the question is how to filter attributes in 0.10.x?

[joaomdmoura]

@iRet and everyone involved.
I know this is not exactly what we discussed but might by a cleaner and good option to filter attributes. This fields options it already used by json-api adapter, I've fixed it and implemented it to on json adapter.
Check it out and 👍 it if it would work for you. Btw it's for 0.10.x version.

[iRet]

@joaomdmoura actually this is not exactly what I'm looking for. It may be useful especially in combination with ability to define default_serializer_options which seems non functional anymore?
However second handy scenario is to have presets of fields depending on action, requires ability of custom processing from serializer side. In particular render light set of data for index action and detailed for show actions. I'm not fully sure it is correct way from architecturall point of view, to render different fieldsets dependeing on parameter received from scope, but it was so handy and clean.
I think attributes method to was a great customization tool.

[joaomdmoura]

@iRet sorry I had forgot the link. It does exactly what you said. (or not?)

[iRet]

@joaomdmoura thanks! not exactly but better than nothing.

[lxcodes]

@joaomdmoura Is this currently the only way to filter in 0.10.x? Seems odd when I have 30-40 attributes in a few of my models and I need to only remove 1 or 2 based on scope; end up with a 3 line string of fields in my controller as well as multiple render statements depending on which to remove.

Is AMS open to having an except option to optionally just remove fields as well or is it possible to reimplement filter? Apologies if I'm missing a simple option.

[iRet]

@al3x-edge +1 for except!
Lack of this feature and missing customization makes 0.10 unusable for my and I belive for many projects running on 0.8 – 0.9.
I'm staying on 0.8 just because of better features set.

@joaomdmoura can we do something with this before release?

[joaomdmoura]

Hey @iRet and @al3x-edge, I understand the case here, and indeed it makes sense.
We'll find a solution to this before the release, for sure. I'm inclined to implement filter back and move away from if and except.
Because of that I'm closing this Issue, opening a new one to bring filter back