Bringing Filter to 0.10.x

[joaomdmoura]

tl;dr

Older versions of AMS had a filterfeature that older users are used to. We would like to bring this to 0.10.x.

---

Older versions of AMS had a filterfeature that older users are used to.
We would like to bring this to 0.10.x mainly for two reasons:

1. There is no easy way to filter attributes on 0.10.x so far
2. It will help ppl to update from 0.8 and 0.9

The idea is to provide a method named filter, which should return an array used to determine what attributes and associations should be included in the output. This is typically used to customize output based on current_user. For example:

class PostSerializer < ActiveModel::Serializer
  attributes :id, :title, :body, :author

  def filter(keys)
    if scope.admin?
      keys
    else
      keys - [:author]
    end
  end
end

---

related to issues:

• filter method called multiple times. #459

related to PRs:

• Fixes filter method being called multiple times.  #460
• add if/unless options to attribute #1266

[bf4]

This is also a feature of the JSON API

• http://jsonapi.org/format/#fetching-filtering
• http://jsonapi.org/recommendations/#filtering

[joaomdmoura]

But in specific to JSON-API I think the fields implementation is covering it.

[bf4]

Not sure I understand. Fields is different.

• http://jsonapi.org/format/#fetching-sparse-fieldsets
• http://jsonapi.org/examples/

[joaomdmoura]

Yup, you're right I messed up.
It seems it's not necessary but it's a recommendation that we should aim for.

It seems to me that some cases are not so useful, taking into considerations the rails context we usually have, like:
GET /comments?filter[post]=1

But as I said, is part of the convention and we should aim for that. 😄
Would be awesome to have filter back working out of the box with the filtering patter on json-api.

[BenMorganIO]

I was just working on this, this was my implementation:

class Api::BaseController
  protected

  def index(collection = [])
    render json: collection.where(filter_params).paginate(page_params), include: params[:include]
  end

  private

  def filter_params
    params.fetch(:filter, {}).permit(filter_attributes << :id).transform_values { |v| v.split(',') }
  end

  def filter_attributes
    serializer = "Spree::#{controller_name.titlecase.singularize}Serializer"
    serializer.constantize._attributes
  end

  def page_params
    params.fetch(:page, {}).permit(:number, :size)
  end
end

class Api::PricesController
  def index
    super Price.all
  end

  def show
    super Price.find(params[:id])
  end
end

Its based on the controller and I think the JSON API's definition of a filter is to identify objects that have an id of 1,2,3,..,n+1 or what if you have a prices endpoint and you want to filter by currency? Use cases:

/api/prices?filter[currency]=CAD,USD
/api/countries?filter[iso3]=CAD,USA&include=states

The filtering should be more controller based since we're focusing more on content coming from a URI and deciphering what is and isn't secure to accept. The fields is more of trying to focus on minimizing the JSON payload; especially over making a whitelist that doesn't include certain relationships (like not wanting 50,000 ids back over some requests, but not others).

[beauby]

Considering what the spec says:

JSON API is agnostic about the strategies supported by a server. The filter query parameter can be used as the basis for any number of filtering strategies.

I think it is not AMS's responsibility to handle this parameter.

[beauby]

However, the 0.8.0 filter feature (which is unrelated to the JSONAPI filter) remains a useful tool that I believe should be available in 0.10.0.

[BenMorganIO]

@beauby considering how easy it is to do on ones own, maybe make another gem that just adds more conventional sugar.

[BenMorganIO]

The 0.8.0 "filter" feature might need to be renamed to "fields". I think that's how the JSON API spec uses the terms.

[beauby]

Thing is, I'm not even sure what AMS should do with the filter query param if it were to do anything. Surely, filtering a collection before rendering should be done at the DB level.

[beauby]

@BenMorganIO The 0.8.0 filter is not really the same as the JSONAPI fields, as the former allows for the dev to pick which attributes/relationships will be serialized depending on certain conditions (like "is the current user an admin?"), whereas the latter allows the client to specify which fields it wishes to receive.

[BenMorganIO]

There's an include helper, so I think there's room for an argument on why the filter and fields params should be a part of the AMS 0.10.

However, I would opt for AMS to be have a second gem that adds that sugar to ActionController.

[BenMorganIO]

@beauby I like how the filter can do that based on the current users role. I'm 👍 for that. I'd rather opt for something like a public_attributes, user_attributes, admin_attributes. I feel like that would would be a much more intuitive API.

(And maybe adding some dynamic helpers for permissions as well. Like if you have Vendor/Buyer users or Designer/Client users.)

[beauby]

Again, include is intended to be usable by the dev to specify which relationships they want to include in the response. As a commodity, AMS supports the JSONAPI format for the include query parameter, but it's not limited to that scenario.

I'm all up for having JSONAPI-related helpers somewhere (I think that'd be great). I just don't think they all belong within AMS.

[BenMorganIO]

@beauby I'm all 👍 for a JSONAPI-related helper gem for AMS. I'd love to put my JSON API filters params as a helper somewhere.

[vasilakisfil]

I am totally against having the filtering logic inside the AMS classes. However there should definitely be an option for filtering attributes (JSONAPI fields param) in a serializer, but outside the AMS class. This was possible in the 0.9.x with the only: option. Maybe relevant to #1141

It's not a good idea to try to make AMS an all around tool. AMS responsibility is to serialize. We should create other gems for filtering (JSONAPI filter param) and including associations (JSONAPI include param) in the db level. Basically AMS 0.10.x should ONLY deal with serializing the objects and not dealing with the incoming params. For the incoming params we should built other gems.

Personally I have created 2 gems that are relevant to the JSONAPI filter params which I could make them JSONAPI-compliant if you think it's worth the effort:
https://github.com/kollegorna/active_hash_relation
https://github.com/kollegorna/mongoid_hash_query

These gems are supposed to be run before the serializers.

Another reason that we shouldn't make AMS an all around tool is because they should be compatible with any db (sql, nosql, redis etc) even with POROs.

[vasilakisfil]

I think I can provide you a gem that filters attributes (JSONAPI fields param) of a resource in the db level for ActiveRecord and Mongoid using AR's select(:attribute1,..) and Mongoid's only(:attribute1,..) respectively. I would create that anyway next week so why not coordinating with you..

[BenMorganIO]

So, Spree's V2 API actually can filter attributes. What I want to do is focus it on self and support associations later on. See: https://github.com/wildcardlabs/solidus_json_api/blob/master/app/controllers/concerns/spree/api/v2/renderable.rb#L19

You can see it in action here: http://solidusapi.wildcardlabs.com/#filtering

However, what I want it to be able to do is:

# current
curl "https://example.com/api/articles?filter[id]=2,3,4"

# hope
curl "https://example.com/api/articles?filter[self.id]=2,3,4"
curl "https://example.com/api/articles?filter[comments.id]=2,3,4"

[bf4]

@joaomdmoura Can be closed to due merge of #1403 ?

Other related issues/prs:

• Using include_<association>? in 0.10.x not working? #857
• Include option for associations in serializers #1333
• Add :except as an option to Adapter::Attributes #1538

[joaomdmoura]

Yup! #1403 seems to solve it in a better and more elegant way 😁 I'm closing it!