Skip to content

Use active_admin_authorization as Ransack auth_object option#8143

Merged
mgrunberg merged 2 commits into
activeadmin:masterfrom
mgrunberg:send-auth-object-to-ransack
Dec 1, 2023
Merged

Use active_admin_authorization as Ransack auth_object option#8143
mgrunberg merged 2 commits into
activeadmin:masterfrom
mgrunberg:send-auth-object-to-ransack

Conversation

@mgrunberg

Copy link
Copy Markdown
Contributor

fixes #8108

The issue suggest to use active_admin_authorization as auth_object. It make sense to me.

I'm not implementing the idea of a default implementation of ransackable attributes. That doesn't seems easy to me. I would appreciate a PR or a battle tested example for that.

@codecov

codecov Bot commented Nov 29, 2023

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f26e4e8) 99.04% compared to head (638b126) 99.04%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #8143   +/-   ##
=======================================
  Coverage   99.04%   99.04%           
=======================================
  Files         182      182           
  Lines        4722     4722           
=======================================
  Hits         4677     4677           
  Misses         45       45           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mgrunberg mgrunberg force-pushed the send-auth-object-to-ransack branch from 5c0fc02 to 6137289 Compare November 29, 2023 15:50
@mgrunberg mgrunberg marked this pull request as ready for review November 29, 2023 15:50
@mgrunberg mgrunberg force-pushed the send-auth-object-to-ransack branch 2 times, most recently from 9b02bd4 to 26d0d0a Compare November 29, 2023 16:52
Comment thread CHANGELOG.md
fixes activeadmin#8108

The issue suggest to use `active_admin_authorization` as auth_object. It
make sense to me.

I'm not implementing the idea of a default implementation of ransackable
attributes. That doesn't seems easy to me. I would appreciate a PR or a
battle tested example for that.
@mgrunberg mgrunberg force-pushed the send-auth-object-to-ransack branch from 26d0d0a to d928873 Compare November 29, 2023 20:48
@mgrunberg mgrunberg removed the request for review from tagliala November 29, 2023 20:49
@javierjulio javierjulio changed the title Send auth_object option to ransack Use active_admin_authorization as Ransack auth_object option Dec 1, 2023

@javierjulio javierjulio left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@mgrunberg mgrunberg merged commit 9204840 into activeadmin:master Dec 1, 2023
@mgrunberg mgrunberg deleted the send-auth-object-to-ransack branch December 1, 2023 13:32
@baldarn

baldarn commented May 15, 2024

Copy link
Copy Markdown

Sorry, is this going to be released anytime soon?

I saw that was merged, but still in 3.2.1 is not released

@javierjulio

Copy link
Copy Markdown
Member

@baldarn it is already released in v4. This change was seen as a breaking change so we won't release on v3.

@qdegraeve

Copy link
Copy Markdown

@javierjulio sorry to reopen the discussion but v4 is not released as a stable version yet and is a huge change.
I don't really see why this is considered as a breaking change as the change goes from nil to a very specific auth_object that can only be activeadmin related.

Would it be possible to reconsider releasing it on v3 ?

@baldarn

baldarn commented Jun 19, 2024

Copy link
Copy Markdown

@javierjulio sorry to reopen the discussion but v4 is not released as a stable version yet and is a huge change. I don't really see why this is considered as a breaking change as the change goes from nil to a very specific auth_object that can only be activeadmin related.

Would it be possible to reconsider releasing it on v3 ?

we added a simple patch in our app to get this feature.

just monkey patch

lib/active_admin/resource_controller/data_access.rb

and you are good to go!

@javierjulio

Copy link
Copy Markdown
Member

@qdegraeve those that worked on the feature view it as a breaking change and I trust that so no sorry it will not be released in v3. I don't see much concern either since it's a one line change in a two line method that hasn't been modified since it's original implementation and Metasearch-to-Ransack migration over a decade ago. As @baldarn mentioned it is simple and safe to just monkey patch if you need the behavior now on v3. Thank you.

@qdegraeve

Copy link
Copy Markdown

Thanks for your reply, I'll add the patch as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ransack never receives an auth object

4 participants