Require Ransack v4

[javierjulio]

As part of a major ActiveAdmin release, we'll now require Ransack v4 which in turn requires users to declare an allowlist of attributes and associations on their models.

Closes #7809

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.02 ⚠️

Comparison is base (c1fe7a4) 99.11% compared to head (6c14325) 99.09%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8009      +/-   ##
==========================================
- Coverage   99.11%   99.09%   -0.02%     
==========================================
  Files         197      197              
  Lines        4945     4949       +4     
==========================================
+ Hits         4901     4904       +3     
- Misses         44       45       +1     

Impacted Files	Coverage Δ
...active_admin/orm/active_record/comments/comment.rb	100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[Fire-Dragon-DoL]

Should activeadmin website documentation be updated too? It seems like it will "naturally " become public API.

Notice that activeadmin could build this list out of filter and provide an API to add more/remove

[javierjulio]

What would be updated in our documentation? That is a public API in Ransack v4. It is expected that users update their models with an allowlist. It's not something that ActiveAdmin configures.

[Fire-Dragon-DoL]

What would be updated in our documentation? That is a public API in Ransack v4. It is expected that users update their models with an allowlist. It's not something that ActiveAdmin configures.

Oh it's at the model level, sorry did not realize that. Thank you.

I would mention in the filter documentation at least a reference to "you must allow attributes you want to enable filter for" and link to the ransack documentation relevant section

[javierjulio]

@Fire-Dragon-DoL yes, I think a simple mention wouldn't hurt. It will be obvious to a user once running the app since they will get a Ransack warning displayed with all the details on what to change.

Will you create a PR to update the docs? I would accept a PR that adds a line mentioning that models now require an allowlist for Ransack with a link to the specific Ransack PR.

[Fire-Dragon-DoL]

@Fire-Dragon-DoL yes, I think a simple mention wouldn't hurt. It will be obvious to a user once running the app since they will get a Ransack warning displayed with all the details on what to change.

Will you create a PR to update the docs? I would accept a PR that adds a line mentioning that models now require an allowlist for Ransack with a link to the specific Ransack PR.

Yes! I would be happy to do that

[Fire-Dragon-DoL]

@Fire-Dragon-DoL yes, I think a simple mention wouldn't hurt. It will be obvious to a user once running the app since they will get a Ransack warning displayed with all the details on what to change.

Will you create a PR to update the docs? I would accept a PR that adds a line mentioning that models now require an allowlist for Ransack with a link to the specific Ransack PR.

Wrote the PR:
#8043

Please let me know if you'd like me to work a bit more on the language, I tried to be terse, but there are a lot of concepts to talk about the allowlist (activeadmin, ransack, model, authorization, attributes)

[Fire-Dragon-DoL]

Proceeded to upgrade my own version to activeadmin 3 today and was looking for documentation on the topic. Very happy to discover it was there, just in time to remember I wrote it 🤦‍♂️