Skip to content

LG-8232 Update Event: IDV forget password email confirmed#7756

Merged
olatifflexion merged 2 commits intomainfrom
olatif/LG8232-ar011-password-reset-email
Feb 6, 2023
Merged

LG-8232 Update Event: IDV forget password email confirmed#7756
olatifflexion merged 2 commits intomainfrom
olatif/LG8232-ar011-password-reset-email

Conversation

@olatifflexion
Copy link
Contributor

changelog: Internal, Attempts API, Passing request_id in the forget password email link

changelog: Internal, Attempts API, Passing request_id in the email link
<center>
<%= link_to t('user_mailer.reset_password_instructions.link_text'),
edit_user_password_url(reset_password_token: @token, locale: @locale),
edit_user_password_url(reset_password_token: @token, locale: @locale, request_id: @request_id),
Copy link
Contributor

Choose a reason for hiding this comment

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

can you provide some background on what linking the request_id does here? does it keep some session around that we need for attempts API?

If so, can we add some sort of acceptance spec that maybe clicks one of these emails and sees the event linked correctly? or something that helps explain why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! I added this (some other emails also pass request_id in URL and they are logging events) to keep some SP sessions.
Sure let me check I will try to add some tests.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

currently, events get logged when the user clicks the reset password link from the email within the same browser session, but if a user closes the browser and then clicks the reset password link from the email our system does not log the event for that reason I added request_id in the links.

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for adding the specs!

@olatifflexion olatifflexion merged commit b280412 into main Feb 6, 2023
@olatifflexion olatifflexion deleted the olatif/LG8232-ar011-password-reset-email branch February 6, 2023 16:55
@aduth aduth mentioned this pull request Feb 9, 2023
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.

2 participants