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

Rework authorization policies #447

Merged
merged 2 commits into from May 26, 2022
Merged

Rework authorization policies #447

merged 2 commits into from May 26, 2022

Conversation

joemasilotti
Copy link
Owner

@joemasilotti joemasilotti commented May 26, 2022

This PR migrates authorization from Pundit to Action Policy.

The original goal was to pass along error messages and redirection URLs for ColdMessageController. But I'm not there yet – this PR only migrates the underlying system in place. (And adds a missing authorization!)

While reviewing this PR I realized that my original expectation is a little flawed. A lot of the recent refactors, like the service layer one, stem from ColdMessageController having what looks like a lot of logic.

Migrating to Action Policy would allow us to pass error messages and "context" when an authorization fails. I was planning on using that to pass the redirection URL and a custom error message.

In hindsight, I think that's putting a little too much responsibility on the policy. For now I'm going to keep these changes that refactor some of the policies and use the view helper from Pundit.

Pull request checklist

  • My code contains tests covering the code I modified
  • I linted and tested the project with bin/check
  • I added significant changes and product updates to the changelog

* Use policy().method? helper in views for conditional logic
* Keep consolidated MessagePolicy and ConversationPolicy instead of
  MessagingPolicy
@joemasilotti joemasilotti changed the title Migrate from Pundit to Action Policy Rework authorization policies May 26, 2022
@joemasilotti joemasilotti marked this pull request as ready for review May 26, 2022 18:42
@joemasilotti joemasilotti merged commit e21ebcc into main May 26, 2022
5 checks passed
@joemasilotti joemasilotti deleted the action-policy branch May 26, 2022 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant