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

Evaluate ransackable_scopes before attributes when building the query #759

Merged
merged 2 commits into from
Mar 15, 2022

Conversation

faragorn
Copy link
Contributor

@faragorn faragorn commented Feb 3, 2017

This paragraph in README.md shows that Ransack can properly evaluate this scope:

 scope :salary_gt, ->(amount) { where('salary > ?', amount) }

if the scope is properly whitelisted:

  def self.ransackable_scopes(auth_object = nil)
    %i(salary_gt)
  end

However, it is not the case because attribute_method?(salary_gt) will evaluate to true even if the attribute is not whitelisted in ransackable/ransortable_attributes.

I assumed that scopes were introduced to easily override/customize #{attribute}_#{predicate} pair without breaking the naming convention.
In that case this PR should make ransack work as per the README mentioned above.

Test cases will follow, but I would appreciate initial Yes/No before putting more time in to it.

@scarroll32 scarroll32 mentioned this pull request Feb 24, 2021
13 tasks
@scarroll32
Copy link
Member

@faragorn it's been a while, but this looks very useful.

@faragorn
Copy link
Contributor Author

faragorn commented Jan 5, 2022

@scarroll32 I will finish it hopefully this month

@deivid-rodriguez
Copy link
Contributor

This one also makes sense to me!

@scarroll32
Copy link
Member

@faragorn this is a candidate for the 3.0.0 Release if you can finish it. 🥇

@faragorn
Copy link
Contributor Author

@scarroll32 Added a spec, and rebased, let me know if looks good and further steps. Cheers!

@scarroll32
Copy link
Member

Thank you for your contribution @faragorn, it will be pushed to RubyGems as part of 3.0.0, and available as a ref to master until then.

@scarroll32 scarroll32 merged commit bbd4b14 into activerecord-hackery:master Mar 15, 2022
@faragorn faragorn deleted the scopes-first branch March 15, 2022 17:11
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.

3 participants