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

fix: Can't re-send shop manager invite to existing non-activated user #5178

Merged

Conversation

loan-laux
Copy link
Collaborator

Resolves #5176
Impact: major
Type: bugfix

Issue

After sending a shop manager invite to a non-registered e-mail, a new user is created for them with a token to be used to set their password. An e-mail invite is then sent with a URL containing this token.

However, if you want to re-send an invitation to this same person, the accounts/inviteShopMember will see an existing account for this e-mail, not checking whether it was activated or not. This means that no new "set password" token will be sent, and the user will be promoted to shop manager (which it already was), as if it was an already active account.

Solution

In the accounts/inviteShopMember logic, if an account already exists for the e-mail address but isn't activated yet, generate a new token and send a new invite.

Also, add some confirmation dialogs in case an e-mail already exists, whether it's activated or not.

Breaking changes

None.

Testing

  1. In the Accounts page of the operator, invite an e-mail address that's already registered and activated.
  2. See a dialog asking you if you want to promote this existing user to shop manager.
  3. Invite an e-mail that's never been used for an account before.
  4. Receive invite on that e-mail. Delete it "by mistake".
  5. Re-invite this same e-mail again in an attempt to send a new invite.
  6. See a dialog asking you if you want to send a new invitation. Click yes.
  7. Receive a new invitation with a new token.
  8. Click on the button in the e-mail, see the "set password" form.

Signed-off-by: Loan Laux <loan@outgrow.io>
Signed-off-by: Loan Laux <loan@outgrow.io>
…en and send new invite

Signed-off-by: Loan Laux <loan@outgrow.io>
Copy link
Contributor

@aldeed aldeed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment in two places, but otherwise I think this can be merged. Thanks @loan-laux !

@aldeed
Copy link
Contributor

aldeed commented Jun 24, 2019

Ping @loan-laux

@loan-laux
Copy link
Collaborator Author

Sorry @aldeed, I was on vacation and just came back today. I'll implement the requested changes today.

Signed-off-by: Loan Laux <loan@outgrow.io>
@loan-laux loan-laux force-pushed the outgrow-fix-admin-invite-flow branch from 798c5e5 to 71697cb Compare June 26, 2019 09:30
@loan-laux
Copy link
Collaborator Author

@aldeed Let me know if this fix is what you had in mind.

Copy link
Member

@willopez willopez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some small modifications needed.

Signed-off-by: Loan Laux <loan@outgrow.io>
Signed-off-by: Loan Laux <loan@outgrow.io>
@loan-laux
Copy link
Collaborator Author

Good catch @willopez, thanks!

Copy link
Member

@willopez willopez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@willopez
Copy link
Member

willopez commented Jul 1, 2019

integration tests failing due to this known issue: #5246.

I have verified all tests pass locally.

@willopez
Copy link
Member

willopez commented Jul 1, 2019

@loan-laux thank you for your contribution 👍

@willopez willopez merged commit be645d3 into reactioncommerce:develop Jul 1, 2019
@jeffcorpuz jeffcorpuz mentioned this pull request Jul 2, 2019
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.

3 participants