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

[HOLD for payment 2022-02-08] Improve UX of Setting a password on account sign up with expired or malformed token #3189

Closed
anthony-hull opened this issue May 27, 2021 · 62 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Weekly KSv2

Comments

@anthony-hull
Copy link
Contributor

anthony-hull commented May 27, 2021

Expected Result:

When a user tries to set their password during the new user flow and uses an expired or malformed magic token.
The form to be replaced by the following copy:
This set password link is invalid or has expired. A new one is waiting for you in your email inbox!

Actual Result:

Error was as follows:
image
The user is able to keep re-submitting the form from an unrecoverable and invalid state.

Action Performed:

trigger password reset
make sure you're logged out
navigate to here: http://expensify.cash/setpassword/[USERID]/[RESETCODE]
make sure you have credentials.login key set to your email address in local storage
submit a valid password

Platform:

Where is this issue occurring?

Web

Upwork job link- https://www.upwork.com/jobs/~01ee9af3d9febfb893

View all open jobs on Upwork

@anthony-hull anthony-hull added the AutoAssignerTriage Auto assign issues for triage to an available triage team member label May 27, 2021
@MelvinBot
Copy link

Triggered auto assignment to @lschurr (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

@MelvinBot MelvinBot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label May 27, 2021
@anthony-hull
Copy link
Contributor Author

This looks like it has been changed to something more useful in the backend:
image
Possibly still some call to action to resend a code for the user would be good?

@lschurr
Copy link
Contributor

lschurr commented Jun 7, 2021

Hm, when I tried to reproduce this, I got a privacy error:

image

I'm going to add the eng label to have someone else take a look.

@lschurr lschurr added Engineering Improvement Item broken or needs improvement. Weekly KSv2 labels Jun 7, 2021
@MelvinBot
Copy link

Triggered auto assignment to @stitesExpensify (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@lschurr lschurr removed their assignment Jun 7, 2021
@stitesExpensify
Copy link
Contributor

Hi @anthony-hull, can you clarify your solution please?

@anthony-hull
Copy link
Contributor Author

anthony-hull commented Jun 8, 2021

I think there should be some way of the user requesting a new set password email link. So maybe replace the whole form with a request new link button?

Otherwise the user has no way of actioning the error message and they are stuck on the form unable to progress.

Or you could automatically send a new link when the API notices an expired set user token was used and the form could inform the user of the email and to check their mailbox

@anthony-hull
Copy link
Contributor Author

It looks like the API now re-sends en email when you use an expired token. But I've been using the reset password UX rather than set password for the a new user for most of my testing.

@stitesExpensify
Copy link
Contributor

I think it makes sense to replace the form with a new link button in this case. Are you still interested in this @anthony-hull ?

@stitesExpensify
Copy link
Contributor

Bump @anthony-hull 😄

@MelvinBot MelvinBot removed the Overdue label Oct 6, 2021
@anthony-hull
Copy link
Contributor Author

anthony-hull commented Oct 7, 2021

I've just checked this flow and the API now just resends a new email if if gets a bad magic code. The button wouldn't be needed.

But informing the user that a new email has been sent would be an improvement I think.
Also locking the form, or replacing it with the message would be a good idea. As trying again would just result in the same error and a second email being sent out

@stitesExpensify
Copy link
Contributor

Cool, I'm in favor of you making that solution as those seem like great improvements to me! If you want to update the OP I'll add the external label and we can get you working on it! We are still currently in a merge hold though so I'm not sure when it will get merged/paid out FYI

@anthony-hull
Copy link
Contributor Author

ok :)

@anthony-hull anthony-hull changed the title Setting a password on account sign up with expired token gives wrong error message Improve UX of Setting a password on account sign up with expired or malformed token Oct 12, 2021
@anthony-hull
Copy link
Contributor Author

@stitesExpensify Hey Brandon I've updated the body and title, let me know what you think

@stitesExpensify
Copy link
Contributor

Looks good! I updated the copy a little bit and we should be good to go

@anthony-hull
Copy link
Contributor Author

I like the tweak. Does this need an external label now?

@stitesExpensify stitesExpensify added the External Added to denote the issue can be worked on by a contributor label Oct 19, 2021
@mallenexpensify
Copy link
Contributor

Apologies for the label/assigning spam, wanted to check to see who it got assigned to. @kadiealexander and @SofiedeVreese , I'm around through the holidays so I'll keep tabs on this.

@kadiealexander
Copy link
Contributor

You're a champion Matt, thank you!!

@mallenexpensify mallenexpensify added Weekly KSv2 and removed Daily KSv2 labels Dec 28, 2021
@MelvinBot MelvinBot removed the Overdue label Dec 28, 2021
@mallenexpensify
Copy link
Contributor

@anthony-hull do you have an update on progress? We recently updated our CONTRIBUTING.md to include details on expected timeliness of working on issue .

Daily updates on weekdays are highly recommended. If you know you won’t be able to provide updates for > 1 week, please comment on the PR or issue how long you plan to be out so that we may plan accordingly. We understand everyone needs a little vacation here and there. Any issue that doesn't receive an update for 1 full week may be considered abandoned and the original contract terminated.

Just wanna make sure the PR will be submitted soon.

@MelvinBot MelvinBot removed the Overdue label Jan 5, 2022
@anthony-hull
Copy link
Contributor Author

thanks!
I'm working on this tonight and tomorrow, I updated the linked PR earlier this evening. I hope to have it ready for review tomorrow

@anthony-hull
Copy link
Contributor Author

I haven't managed to finish the PR today.
I don't foresee any blockers for getting it finished tomorrow.
Currently I'm working through the edge cases listed in my tests.

@mallenexpensify
Copy link
Contributor

Thanks for the update @anthony-hull !

@anthony-hull
Copy link
Contributor Author

anthony-hull commented Jan 7, 2022

I encountered extra complexity and ran out of working time today. I will carry on working on this tomorrow.
I also have an outstanding issue with another issue that this PR will also close which needs to be answered. I can carry on with the rest of the functionality without this input however

@anthony-hull
Copy link
Contributor Author

anthony-hull commented Jan 9, 2022

I resolved the above issue. But I have a new outstanding question in the PR. Most of the logic is done.

@anthony-hull
Copy link
Contributor Author

still pending input on question

@anthony-hull
Copy link
Contributor Author

I'm unblocked. Will finish tomorrow!

@mallenexpensify
Copy link
Contributor

Appreciate the updates @anthony-hull !

@anthony-hull
Copy link
Contributor Author

The PR linked is ready for review, I have requested for such. Just missing some translations and response on required testing documentation!
Hopefully get this one merged soon!

@anthony-hull
Copy link
Contributor Author

Waiting on PR review, answer to a question and a backend bug to be confirmed reproduced by the reviewer and fixed

@mallenexpensify
Copy link
Contributor

looks like PR is on staging! #6587

@botify botify added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Feb 1, 2022
@botify botify changed the title Improve UX of Setting a password on account sign up with expired or malformed token [HOLD for payment 2022-02-08] Improve UX of Setting a password on account sign up with expired or malformed token Feb 1, 2022
@botify
Copy link

botify commented Feb 1, 2022

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.33-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2022-02-08. 🎊

@mallenexpensify
Copy link
Contributor

mallenexpensify commented Feb 8, 2022

Thanks @anthony-hull for the help and persistence with this issue. Paid in Upwork, $500 - $250ea for reporting and the fix (link from hax stating the job was initially created for both)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Weekly KSv2
Projects
None yet
Development

No branches or pull requests

10 participants