Require a partner for the partner dashboard #4063
Conversation
| @@ -93,6 +93,15 @@ def authorize_admin | |||
| current_user.has_role?(Role::ORG_ADMIN, current_organization) | |||
| end | |||
|
|
|||
| def require_partner | |||
There was a problem hiding this comment.
Why is this in ApplicationController if it's only used in the partner DashboardController?
There was a problem hiding this comment.
Future proofing to make it look like the others -- authorize_user, current_role, etc. It looked like those so I put it with those -- wamme to move it?
There was a problem hiding this comment.
Hmm... I think Partner::BaseController is a fine place to put it. It's still fairly centralized and makes it more obvious that it's only relevant to pages in that namespace.
| @@ -4,6 +4,8 @@ class DashboardsController < BaseController | |||
|
|
|||
| protect_from_forgery with: :exception | |||
|
|
|||
| before_action :require_partner | |||
There was a problem hiding this comment.
Shouldn't this be in the partner base controller? I feel like every page under there needs a current partner, no?
There was a problem hiding this comment.
oh, I didn't realize BaseController is Partners::BaseController. I'll move it!
| @@ -93,6 +93,15 @@ def authorize_admin | |||
| current_user.has_role?(Role::ORG_ADMIN, current_organization) | |||
| end | |||
|
|
|||
| def require_partner | |||
There was a problem hiding this comment.
Hmm... I think Partner::BaseController is a fine place to put it. It's still fairly centralized and makes it more obvious that it's only relevant to pages in that namespace.
|
@awwaiid is this PR still relevant? |
|
I'll brush it off and see what we should do |
|
lol... the changes looked fine but a lot of tests are failing. 😁 |
|
Hot dang! |
|
@awwaiid: Your PR |
No description provided.