-
Notifications
You must be signed in to change notification settings - Fork 633
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
add hook for clear pundit context #830
base: main
Are you sure you want to change the base?
Conversation
Thank you for the PR! I like the implementation. Clear, well-written, and tests. Nice! I have some hesitations that I need to ponder:
Before this would be merged, assuming it will be merged, I think some changes should be made:
|
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.
@Burgestrand Thank you for your thoughtful feedback! I appreciate your comments and am glad the implementation resonates well with you. Regarding the concerns: Against: Expands public API: You’re absolutely right :) For: Future-proofing: Agreed! One of the key advantages of adding this feature is precisely the long-term value it provides for those who rely on Pundit. It gives them confidence that their systems will remain stable and maintainable as the library evolves. Now, for the specific changes:
Thanks again for the detailed review! I’ll proceed with the suggested changes and address the concerns you’ve raised. |
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.
Thank you for making the requested changes. I have some more 😁!
I've thought about this some more, and while I haven't yet finally decided I'm leaning even more into merging this.
0d25175
to
a80c98b
Compare
Fix #811
As far as I can see, the caches are not being cleared after a
UserContext
change. Therefore, policies are being performed using the old UserContext. I would like to implement a simple solution to this issue. With this solution, caches can be cleared when deemed necessary.To do
PS: Thank you for contributing to Pundit ❤️