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

Optionally allow overriding the policy class for permitted params #825

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Numie
Copy link

@Numie Numie commented Aug 18, 2024

Fixes #742

To do

  • I have read the contributing guidelines.
  • I have added relevant tests.
  • I have adjusted relevant documentation.
  • I have made sure the individual commits are meaningful.
  • I have added relevant lines to the CHANGELOG.

PS: Thank you for contributing to Pundit ❤️

@Numie Numie force-pushed the numie/permitted-attributes-policy-class branch from 6757dda to 6ea9e3f Compare August 18, 2024 15:00
@Burgestrand Burgestrand added the simmering undecided but generally optimistic label Aug 29, 2024
@Numie Numie force-pushed the numie/permitted-attributes-policy-class branch from 6ea9e3f to 73bf078 Compare August 30, 2024 03:51
@Burgestrand
Copy link
Member

Burgestrand commented Aug 30, 2024

Hi, thanks for the pull request!

I adjusted the PR description to use the PR template we've got going on. This change would need a CHANGELOG entry.

On another note, I feel the scope of this PR is somewhat inflated by the moving of the permitted_attributes implementation into Pundit::Context, but I'm not sure why that was done. I think that can be skipped entirely, to keep the change small.

I also notice that pundit_params_for(record) also looks up a policy, and I wonder if that policy shouldn't be the same policy if you pass in policy_class. That would necessitate a change there too.

I'll ponder the whole Pundit::Context and permitted parameters relationship in the meanwhile, too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
simmering undecided but generally optimistic waiting for response
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add policy_class parameter to permitted_attributes function
2 participants