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

Allow filtering by scope #54

Closed
wants to merge 2 commits into from

Conversation

mamhoff
Copy link
Contributor

@mamhoff mamhoff commented Jun 24, 2021

What is the current behavior?

jsonapi.rb will filter out filtering terms that refer to a scope defined on the searchable model.

I've build on a fantastic failing spec by @fluxsaas - thank you!

What is the new behavior?

This allows the user of jsonapi.rb to specify scopes that are marked as ransackable on the resource being queried.

Scopes do not have a predicate, and this just removes the predicate check. No API changes, really.

Related PRs and issues

Should fix #42

Checklist

Please make sure the following requirements are complete:

  • Tests for the changes have been added (for bug fixes / features)
  • [-] Docs have been reviewed and added / updated if needed (for bug fixes /
    features)
  • All automated checks pass (CI/CD)

@fluxsaas
Copy link
Contributor

awesome!

i actually put some more work into it, but wasn't able to do a proper PR:

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

i added a allowed_scopes array option. but yours seems a little more elegant.

Scopes are special in that they don't have a predicate. This removes the
check for the predicate, and thus makes scopes as filtering options work
with JSONAPI.rb.
@mamhoff mamhoff force-pushed the mamhoff/ransackable_scopes branch from 00aee90 to e9dc2e4 Compare June 24, 2021 17:40
@stas stas mentioned this pull request Aug 4, 2021
3 tasks
@stas
Copy link
Owner

stas commented Aug 4, 2021

@mamhoff @fluxsaas first of all, apologies for being super late on reviewing this one.

I carried on the work that you've started and put it all together in the #57

Main reason why I moved it into an options flag, is because this is by far not a common thing to have and it requires extra steps (configure the scopes, add it to ransack, etc.). I also wanted to make sure that the options are explicit, as @fluxsaas pointed, since leaving filterables without any validation for predicates in a public API can lead to various misuses.

I'll be closing this and merging #57 if that's ok. Please continue the conversation in #57

@stas stas closed this Aug 4, 2021
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.

filtering by (ransack) scopes does not work
3 participants