stas / jsonapi.rb

Lightweight, simple and maintained JSON:API support for your next Ruby HTTP API.
MIT License
262 stars 58 forks source link

filtering by (ransack) scopes does not work #42

Open fluxsaas opened 3 years ago

fluxsaas commented 3 years ago

Expected Behavior

filtering by scopes should work with jsonapi.rb and ransack, see: https://github.com/activerecord-hackery/ransack#using-scopesclass-methods

Actual Behavior

it seems not to work properly, only filtering by allowed attributes works.

i created a branch and added failing tests:

https://github.com/stas/jsonapi.rb/compare/master...easyPEP:feature/ransack-scopes

i would like to start working on a solution to make it work. do you have any specific recommendations?

fluxsaas commented 3 years ago

first draft:

see: https://github.com/stas/jsonapi.rb/compare/master...easyPEP:feature/ransack-scopes

what i like:

what i don't like:

to resolve that we could:

we can remove logic from the gem (allowed_fields, allowed_scopes) e.g. from jsonapi_filter_params and rely on the ransack Authorization on the Model level with ransortable_attributes and ransackable_scopes ransackable_associations, ...

ressource = User.last
allowed_scopes = ['created_before']
ressource.class.send(:define_singleton_method, 'ransackable_scopes') do
  allowed_scopes
end
User.ransackable_scopes

e.g.:

jsonapi_filter(User.all, jsonapi_allowed_attributes, jsonapi_allowed_scopes, options)
...
def jsonapi_allowed_attributes
   User.ransackable_attributes(current_user)
end

def jsonapi_allowed_scopes
   User.ransackable_scopes(current_user)
end
stas commented 3 years ago

Hi there @fluxsaas and thanks for the issue :bow:

I'm a bit confused on what's not working tbh :see_no_evil: If you add the Ransack scope name to the list of allowed filterables it should work just fine. I don't see why these should be separated, please correct me if I'm wrong.

I've been using ransackers for a while with no changes btw :)

fluxsaas commented 3 years ago

mh, maybe i have an error in my config

i created a new branch with only the failing specs:

https://github.com/stas/jsonapi.rb/commit/c78f73fa8e215b84aa0190e1b786687c39670954

if i run the tests i get some failing specs:

  1) UsersController GET /users with users returns users filtered by scope ensures ransack scopes are working properly
     Failure/Error: expect(ransack.result.to_sql).to eq(expected_sql)

       expected: "SELECT \"users\".* FROM \"users\" WHERE (created_at < '2013-02-01')"
            got: "SELECT \"users\".* FROM \"users\""

       (compared using ==)
     # ./spec/filtering_spec.rb:113:in `block (5 levels) in <top (required)>'

  2) UsersController GET /users with users returns users filtered by scope should return only
     Failure/Error: expect(response_json['data'].size).to eq(1)

       expected: 1
            got: 3

       (compared using ==)
     # ./spec/filtering_spec.rb:118:in `block (5 levels) in <top (required)>'

Finished in 0.33584 seconds (files took 2.03 seconds to load)
29 examples, 2 failures
fluxsaas commented 3 years ago

update

replacing created_before with created_before_gt does not seem to fix it

https://github.com/stas/jsonapi.rb/compare/master...easyPEP:feature/ransack-scopes-failing-test?expand=1

update 2:

i enabled the github actions on my repository:

https://github.com/easyPEP/jsonapi.rb/runs/2076179007?check_suite_focus=true

stas commented 3 years ago

@fluxsaas apologies for the late reply.

I took a look at the PR you shared and I can see how that can be useful. Still, I'd like us to use the allowed_fields list to pass the scopes where/when needed. If you find some time to prepare a PR, I'd be happy to merge it! :bow:

barberj commented 1 year ago

I confirm that ransack_scopes also doesn't seem to be working with my jsonapi.rb endpoint.