Skip to content

Implement Included Resource Filtering#1183

Merged
lgebhardt merged 3 commits intoJSONAPI-Resources:release-0-9from
hatchloyalty:pr-0-9-3-filter-included-resources
Nov 7, 2018
Merged

Implement Included Resource Filtering#1183
lgebhardt merged 3 commits intoJSONAPI-Resources:release-0-9from
hatchloyalty:pr-0-9-3-filter-included-resources

Conversation

@carlthuringer
Copy link
Copy Markdown

@carlthuringer carlthuringer commented Sep 19, 2018

Inspirational Credit and original code: @beniutek

Closes #890
Closes #844
Closes #896
Closes #314

After much thrashing and some time out in the wilderness with our forked version, I came back here to finish the job, also noticing some bizarre behavior stemming from the original implementation.

Bizarreness:
The loop in the serializer which goes through included resources and adds them to the included resources cache is taking advantage of active record association cacheing such that if any modification of the scope happened during the loading of the primary resource data, then the included resources would also follow suit. However, this is broken if certain things are done to the association scope in the records_for(relation) public method, which may be overridden in the resource. When such happens, the touched-scope in the cache is lost and the records are retrieved anew. Under those conditions, the blasé attitude of the serializer towards the RequestParser's findings re: filtering results in the queries being re-run without filter, thus you may get more included records than you bargained for if you've tripped over the ActiveRecord Association cache.

@willisplummer
Copy link
Copy Markdown

Just running into this - would be awesome to get it in!

@lgebhardt lgebhardt merged commit a925eff into JSONAPI-Resources:release-0-9 Nov 7, 2018
@lgebhardt
Copy link
Copy Markdown
Contributor

@carlthuringer Thanks!

@carlthuringer carlthuringer deleted the pr-0-9-3-filter-included-resources branch November 7, 2018 15:32
@kdelvare
Copy link
Copy Markdown

kdelvare commented Dec 1, 2018

How does one add that kind of filter to the allowed filters?

@lgebhardt
Copy link
Copy Markdown
Contributor

@kdelvare You can add the filter as filter 'tags.name' to filter to posts that have tags matching a particular name.

Comment thread lib/jsonapi/resource.rb
related_directives = include_directives.include_directives.fetch(:include_related)
related_directives.reduce(records) do |memo, (relationship_name, config)|
relationship = _relationship(relationship_name)
next memo unless relationship && relationship.is_a?(JSONAPI::Relationship::ToMany)
Copy link
Copy Markdown

@josephbridgwaterrowe josephbridgwaterrowe Jan 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@carlthuringer does this mean that the filtering won't work on a relationship declared as a has_one?

I've been trying to filter with /option_date_defaults?filter[exchange_commodity.code]=XC01, but the filter is not added to the where statement of the query.

These are my resources:

# /app/resource/api/v1/option_date_default_resource.rb
# frozen_string_literal: true

module Api
  module V1
    class OptionDateDefaultResource < JSONAPI::Resource # :nodoc:
      attributes :due_date_month, :exchange_commodity_code, :option_date_month

      filter 'due_date_month'
      filter 'exchange_commodity.code'

      has_one :exchange_commodity

      def exchange_commodity_code
        @model.exchange_commodity.code
      end
    end
  end
end

# /app/resource/api/v1/exchange_commodity_resource.rb
# frozen_string_literal: true

module Api
  module V1
    class ExchangeCommodityResource < JSONAPI::Resource # :nodoc:
      attributes :code, :exchange_code, :exchange_name, :name, :uuid

      filter :code

      has_one :exchange
      has_many :option_date_defaults
    end
  end
end

Any clarification or help you could provide would be appreciated.

Thanks.

In the meantime I'm going to use the approach here from #890

@codeguru42
Copy link
Copy Markdown

I couldn't find any references in the documentation that shows how to use this. I finally figured it out, but it would be great to include examples in the docs for users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants