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

[Bug] Fix nasty error when clicking on partner approval request email as partner #3686

Closed
2 tasks
cielf opened this issue Jun 25, 2023 · 18 comments · Fixed by #3974
Closed
2 tasks

[Bug] Fix nasty error when clicking on partner approval request email as partner #3686

cielf opened this issue Jun 25, 2023 · 18 comments · Fixed by #3974

Comments

@cielf
Copy link
Collaborator

cielf commented Jun 25, 2023

Summary

Fix this: If you are signed in as a partner, and you click on the link in the partner approval request email, it throws an unhandled exception

Why?

This (or at least something very similar to it) has happened in production to several different users in the last month. I know, it doesn't sound like standard behaviour, but let's eliminate this cause.

Details

Handle the error, showing a message like "You must be logged in as the essentials bank's organization administrator to approve partner applications"

Recreation

On your local:

  • Sign in as org_admin1@example.org
  • Invite a new partner. (create a partner, then invite them)
  • Logout -
  • Accept the invitation (should pop up in a new tab).
  • Go into "Edit My Organization" (for clarity, you're the partner now), and click "Submit for Approval".
  • Without logging out, click on the "Review This Organization" link in the email (that will have popped up in a new tab)

Image

This will give you an error like this:

Image

Other things to consider

This probably also happens if you are logged in as an org_user. (That's a more likely scenario.)

Major hint!

If you add the role id (for the org_admin) to the link in the email, then you can change the controller to validate that role id and switch to it before continuing.

Criteria for Completion

  • The error is handled in a smooth and friendly way
  • Tests demonstrating/supporting the fix.
@zeeshan-haidar
Copy link
Contributor

I can work on this

@dorner
Copy link
Collaborator

dorner commented Jul 6, 2023

Hey @zeeshan-haidar - did you want this one or #3707 ?

@zeeshan-haidar
Copy link
Contributor

@dorner I actually want both, I have already started working on #3707, I just don't wanna sit idle once I finish that #3707

@dorner
Copy link
Collaborator

dorner commented Jul 6, 2023

All righty - good luck!

@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2023

This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.

@github-actions github-actions bot added the stale label Aug 6, 2023
@zeeshan-haidar
Copy link
Contributor

zeeshan-haidar commented Aug 6, 2023

This issue is in progress, already created PR

@github-actions github-actions bot removed the stale label Aug 7, 2023
@dorner
Copy link
Collaborator

dorner commented Aug 10, 2023

guh not sure why the bot is marking it as stale when the PR had changes 😦

@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2023

This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.

@github-actions
Copy link
Contributor

Automatically unassigned after 7 days of inactivity.

@thebkbuffalo
Copy link
Contributor

I can pick this ticket up.

@dorner
Copy link
Collaborator

dorner commented Nov 10, 2023

Go for it!

@github-actions github-actions bot removed the Help Wanted Groomed + open to all! label Nov 10, 2023
@thebkbuffalo
Copy link
Contributor

@dorner quick question! I'm new to this app and not sure what is the correct page to be displayed. Can you add some info on that? Thank you!

@github-actions github-actions bot removed the stale label Nov 11, 2023
@dorner
Copy link
Collaborator

dorner commented Nov 11, 2023

@thebkbuffalo I'd take a look at the original (now apparently abandoned) PR as a place to start looking: #3759

@thebkbuffalo
Copy link
Contributor

@dorner - question for you. I'm working out how data is associated, and have sorted out whats breaking here, but not sure the right way to fix it. These new Partners getting created don't have an associated Organization via roles (they only have Partner roles, not org roles) which is what's breaking links created on the _lte_sidebar.html.erb page. However, I'm not entirely sure these newly created Partners should have an associated Organization or not. Any light you can shed on these roles and what should be associated or not would be helpful. Thank you!

@dorner
Copy link
Collaborator

dorner commented Nov 29, 2023

Partners all are associated with an organization (it's a 1-to-many, not many-to-many relationship). Partners themselves don't have roles, only partner users. So it would be partner user -> partner (via role) -> organization.

@thebkbuffalo
Copy link
Contributor

Sorry I mistyped in my last comment. I understand the association between Partners and Organizations. The question I was trying to get to was more about the association between Users and Organizations. The users created during the new partner creation process don't have any associated roles that correspond to organizations. So when the user clicks the Review Organization link, what's breaking is calling current_user.organization in the _lte_sidebar shared view because the User doesn't have any associated Organization via Roles, or more specifically Organization_Role.

So my real question is, should that association from User to Organization via Roles exist for these new Users whose only role is as a Partner? If so, then during the Partner creation process that Role needs to be created as well for the User. If not, may have to modify how we get that Organization data in the sidebar view.

Thank you again for the help!

@thebkbuffalo
Copy link
Contributor

created a PR fixing this bug.
#3965

@thebkbuffalo
Copy link
Contributor

thebkbuffalo commented Dec 14, 2023

Created a new PR with correct fix.

#3974

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment