Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

filtering by (ransack) scopes does not work #42

Open
fluxsaas opened this issue Mar 10, 2021 · 6 comments
Open

filtering by (ransack) scopes does not work #42

fluxsaas opened this issue Mar 10, 2021 · 6 comments
Milestone

Comments

@fluxsaas
Copy link
Contributor

fluxsaas commented Mar 10, 2021

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:

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
Copy link
Contributor Author

fluxsaas commented Mar 10, 2021

first draft:

see: master...easyPEP:feature/ransack-scopes

what i like:

  • it should keep it backward compatible.

what i don't like:

  • we have to manage ransack (model.ransackable_scopes) and jsonapi.rb (allowed_scopes) scopes

to resolve that we could:

  • move responsibility for ransortable_attributes and ransackable_scopes back to the ransack gem. this adds some complexity for first time users and is properly not backwards compatible but might make it a little clearer.

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

  • dynamicly add the method ransackable_scopes on the model class
ressource = User.last
allowed_scopes = ['created_before']
ressource.class.send(:define_singleton_method, 'ransackable_scopes') do
  allowed_scopes
end
User.ransackable_scopes
  • hint in the docs how we can pass in the attributes and 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
Copy link
Owner

stas commented Mar 10, 2021

Hi there @fluxsaas and thanks for the issue 🙇

I'm a bit confused on what's not working tbh 🙈
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
Copy link
Contributor Author

mh, maybe i have an error in my config

i created a new branch with only the failing specs:

c78f73f

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
Copy link
Contributor Author

fluxsaas commented Mar 10, 2021

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
Copy link
Owner

stas commented Apr 3, 2021

@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! 🙇

@mamhoff mamhoff mentioned this issue Jun 24, 2021
2 tasks
@stas stas added this to the v1.8 milestone Sep 1, 2021
@barberj
Copy link

barberj commented Feb 15, 2023

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

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 a pull request may close this issue.

3 participants