Fix nasty error when clicking on partner approval request email as partner #3759
Conversation
…o 3686-fix-nasty-error-on-partner-approval
| def validate_user | ||
| return unless current_user.has_role?(Role::PARTNER, current_partner) | ||
|
|
||
| redirect_to partner_user_root_path, error: "You must be logged in as the essentials bank's organization administrator to approve partner applications!" |
There was a problem hiding this comment.
These 2 things don't seem to line up... the first line is checking if you are a partner user, but the text seems to indicate you need to be an org admin? I think this should be checking if you're an org admin if I understand the bug correctly.
There was a problem hiding this comment.
@dorner yes I tried putting check for org_admin but lots of test were failing where user was org_user, which I guess org_user also have access to this method, so I put check for both org_Admin and org_user
but I was wondering what if there is any other user too who has access to this
so instead I put check to block only partner, since so far what we know about this bug it's occurring when partner click on "Review this organization" link and try to access this method, we might need to update error message to include org_user also but I am not sure about the permission rules, wdyt?
There was a problem hiding this comment.
My understanding is that only the org admin can approve the requests. If that's the case, then if the current user is not signed in as an org_admin, we should be kicking them out.
There was a problem hiding this comment.
You know, I would have thought so, too -- but the "Approve Partner" button appears when I am signed in as user_1@example.com.
There was a problem hiding this comment.
@dorner, @zeeshan-haidar What's more, I was able to approve a partner at the user level. The org_admin must be pretty much to limit user management (and probably audits and maybe adjustments)-- this is managing the relationship with an existing partner -- when we "approve" a partner, what we are saying is "this partner is allowed to submit requests".
There was a problem hiding this comment.
@dorner, @zeeshan-haidar However I would still prefer to have it based on a 'we allow these roles to do this' basis, rather than a 'we block these people from doing this' -- I think it's better from a hygiene p.o.v -- If we, for instance, later put in a role for a kind of user with narrow access (for instance, a warehouse worker who would only be dealing with packing scheduled distributions), we're less likely to accidentally give them too many permissions.
…o 3686-fix-nasty-error-on-partner-approval
…o 3686-fix-nasty-error-on-partner-approval
…o 3686-fix-nasty-error-on-partner-approval
…o 3686-fix-nasty-error-on-partner-approval
|
@dorner If I'm reading the history correctly, this PR should be closed now? |
|
Yep I think so! |
Resolves #3686
Description
In-case current user role is partner then he will be redirected to partner's dashboard with error message
Type of change
How Has This Been Tested?
Screenshots