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

Update ransack to 4.x #2224

Merged
merged 4 commits into from
Jun 27, 2023
Merged

Conversation

jrochkind
Copy link
Contributor

We use ransack to avoid some boilerplate in our admin dashboards, around sorting and filtering on Collection and Work lists.

Ransack 4.0 has a backwards breaking change where you NEED to list all attributes you want ransack to operate on in allowlist methods in the model (Work or Collection).

It's not much well-documented, but see activerecord-hackery/ransack#1400

This is a security improvement, there were some potential holes there. (Mostly not applicable to use, because our own uses of ransack are all behind admin logins, but still better to be a tight ship -- and stay on the latest dependencies generally).

I really don't like having to add methods to the MODELS. This would ideally have been controller-level. But that's not what ransack chose. In general, ransack seems to be kind of barely maintained (see lack of docs for this change, for instance)... I'd love to move away from it. We actually barely use it, the main convenient thing it does for us involves those SORT headers, where you can click on any header to sort ASC or DESC. Without ransack it'd be a lot of boilerplate. Perhaps we should extract the parts that just do that into it's own gem or something.

But meanwhile, we get everything working for ransack 4.0.

Unfortunately, if we MISSED an attribute in the allowlist -- the default behavior is just for it to no-op not include it in the search/sort. But i've tested everything manually pretty thoroughly on admin Work and Collection, which seems via a sourcecode search to be only place we use ransack. I've tried turning on the ransack configuration to RAISE -- only in dev and test -- instead of silently ignoring. But that does not seem to actually work for sorting, our main use case. activerecord-hackery/ransack#1427

Commits:

  • update ransack gem to 4.0
  • allowlist Work attributes for ransack 4.x
  • allowlist Collection attributes for ransack 4.x
  • try to have ransack error in dev/test when we missed an allowlist item, in cases where it can do so

This PR is built on top of #2223 and that needs to be merged first.

# For ransack use, we need to list all attributes we want to use ransack to SEARCH or SORT by.
#
# We really probably oughta stop using ransack, I hate having this in the model.
def self.ransackable_attributes(auth_obj=nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

Noted!

@jrochkind jrochkind marked this pull request as ready for review June 27, 2023 15:11
@jrochkind jrochkind merged commit 1d16453 into update_rubygem_dependencies Jun 27, 2023
@jrochkind jrochkind mentioned this pull request Jul 3, 2023
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.

2 participants