cerebris / jsonapi-resources

A resource-focused Rails library for developing JSON:API compliant servers.
http://jsonapi-resources.com
MIT License
2.32k stars 529 forks source link

Filter on relationship route? #1065

Open kevintraver opened 7 years ago

kevintraver commented 7 years ago

Is it possible to filter on a relationship route?

class PostResource < JSONAPI::Resource
  has_many :comments

  def records_for_comments(options = {})
    records_for(:comments).public
  end
end
class CommentResource < JSONAPI::Resource
  has_one :post

  filter :active,
    apply: -> (records, value, options) {
      if value
        records.active
      else
        records.inactive
    end
  }
end

This calls the filter:

/comments?filter[active]=1

This does not seem to call the filter:

/posts/14/comments?filter[active]=1

reachire-smendola commented 3 years ago

I believe it's worse than that. What I'm finding is that the filter is being applied to posts, rather than comments. Assuming Posts has active and inactive scopes, then: The end result of the above example would be that the comments on post 14 would all be included if that post is active, and would all be excluded if the post is not active.

In my specific data model, I have

class UserResource {
      has_many :user_focuses
}

and

class UserFocusResource < BaseResource
      filter :active,
             default: 'true',
             apply: lambda { |records, values, options|
               Rails.logger.debug(records.inspect)
               if values == ['true']
                 records.active
               elsif values == ['false']
                 records.inactive
               elsif values == []
                 records
               else
                 raise JSONAPI::Exceptions::InvalidFilterValue.new('active', values.join(','))
               end
             }
}

The request: /api/v1/users-resources?filter[active]=true correctly logs: #<ActiveRecord::Relation [#<**UserFocus** id: "46403c2c-b1b6-497e-91f5-4ac9f09e355d"... and correctly filters the result based on the active/inactive state of the user-focuses.

But /api/v1/users/_xxx_/users-resources?filter[active]=true logs: #<ActiveRecord::Relation [#<User id: "97033f49-2e70-44... which shows the filter is being applied to Users, not UserFocuses.

The end result is that if the User is active, all the user-focuses are included. If the User is inactive, then none of the user-focuses are included. The active/inactive status of the user-focuses themselves is not considered.

This must be a bug, right?

kwstannard commented 11 months ago

We are seeing this too

johnnymo87 commented 6 months ago

This problem only exists on version 0.10.x. I'm currently monkey-patching in a bug fix like so.

config/initializers/jsonapi_resources.rb

# 0.10.x has a bug where lamdba filters are broken on related resources because
# the `records` ActiveRecord relation are not the type of the related resource,
# but rather that of the primary resource, with the related resource joined in.
# This is fixed in 0.11.x.
#
# For example, in `GET /product-lines/:id/recipes`, the primary resource is the
# product line, and the related resource is the recipe. And so the `records`
# are not recipe records but product line records, with recipe records joined
# in.
#
# For more, see
# https://github.com/cerebris/jsonapi-resources/issues/1065.
#
# As a fix, when we detect that we're encountering the bug, we pass a fresh
# `_model_class.all` ActiveRecord relation of the related record into the
# lambda filter. And the we `merge` the result of the lambda filter with the
# `records` of the primary resource, such that the overall ActiveRecord thunk
# is something along the lines of
# `ProductLine.where(id: 1).joins(:recipes).merge(Recipe.where(id: 2))`.
module JSONAPI
  class ActiveRelationResource
    class << self
      alias _original_apply_filter apply_filter

      def apply_filter(records, filter, value, options = {})
        i_am_a_lambda_filter = _allowed_filters.dig(filter.to_sym, :apply)
        return _original_apply_filter(records, filter, value, options) unless i_am_a_lambda_filter

        i_am_a_related_resource = !records.is_a?(_model_class.none.class)
        return _original_apply_filter(records, filter, value, options) unless i_am_a_related_resource

        related_records = _original_apply_filter(_model_class.all, filter, value, options)
        records.merge(related_records)
      end
    end
  end
end