-
Notifications
You must be signed in to change notification settings - Fork 10
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
Bump ransack from 3.2.1 to 4.0.0 #3235
Conversation
fa560da
to
d2cc28d
Compare
end | ||
|
||
def self.ransackable_associations(_auth_object = nil) | ||
['user'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have an association here as we use user_name
which allows sorting on the User association name attribute
@@ -3,7 +3,7 @@ | |||
<tr> | |||
<th><%= sort_link(@search, :message, t('.messsage')) %></th> | |||
<th><%= sort_link(@search, :user_name, t('.creator')) %></th> | |||
<th><%= sort_link(@search, :posted_at, t('.posted_at')) %></th> | |||
<th><%= sort_link(@search, :created_at, t('.posted_at')) %></th> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Believe this was a bug? We have no column on Announcements for posted_at
? I assume this should be created_at instead.
This basically wasn't doing anything before. Now its properly sorting by created at:
app | Announcement Load (2.1ms) SELECT "announcements".* FROM "announcements" WHERE "announcements"."removed_at" IS NOT NULL ORDER BY "announcements"."created_at" ASC LIMIT $1 OFFSET $2 [["LIMIT", 10], ["OFFSET", 0]]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Context
The hardest part of this v4.0.0 upgrade was they forced you to require explict allowlisting of attributes and associations in your models for Ransack for added security. The PR for this was here: activerecord-hackery/ransack#1400
Basically we just have explicitly provide what columns and associations you can use in Ransack for your model like so:
Note: Ransack is only used in our admin dashboard and on 3 screens as of today. The Batch Ingest, User and Announcement index admin screens. All these are working with these changes (even found a bug which is now fixed!). So this upgrade is relatively safe.
Bumps ransack from 3.2.1 to 4.0.0.
Release notes
Sourced from ransack's releases.
... (truncated)
Changelog
Sourced from ransack's changelog.
Commits
dbffa8b
Release 4.0.0 (#1402)2922e33
Require explict allowlisting of attributes and associations (#1400)784e600
Bump http-cache-semantics from 4.1.0 to 4.1.1 in /docs (#1401)fc4b83d
Add support for default predicates (#1384)d658628
Merge pull request #1398 from activerecord-hackery/bump-deps14a17a8
Upgrade recursive-readdir to 2.2.3ab6fa2d
Upgrade serve-handler to 6.1.5555d558
Prevent changing host through params (#1391)cb712fd
Bump ua-parser-js from 0.7.31 to 0.7.33 in /docs (#1397)5778eb7
Bump json5 from 2.2.1 to 2.2.3 in /docs (#1390)