[HOLD for payment 2024-02-26] [$500] Add support for redirecting user to a target URL(via exitTo query param) when they click on the magic link code

[techievivek]

Part of #34166

Please go through the description of the above issue carefully before you post any questions.

We aim to enhance user experience by allowing first-time users to directly access our product feature without a need to manually go through the login process. This enhancement involves embedding a magic code in the offline activity email sent to users.

Magic Link Format: https://new.expensify.com/v//?exitTo=valid_target_path_in_newDot
https://new.expensify.com/v/123/000000?exitTo=r/12345

Functional Requirements:

1. If the user is not logged in, the magic code should trigger the sign-in process automatically and redirect the user to the target URL specified in the exitTo parameter of the link once the sign-in logic is successfully executed.

2. If the user is already logged in, the magic code sign-in should be skipped, and the user should be directly redirected to the exitTo URL.

Resources:
You will probably need to play with ValidateLoginPage logic
Website file: https://github.com/Expensify/App/blob/main/src/pages/ValidateLoginPage/index.website.tsx
Other platform: https://github.com/Expensify/App/blob/main/src/pages/ValidateLoginPage/index.tsx

Let me know if anything is unclear or if you need help with anything. Thanks.

CC @MitchExpensify

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a6d9657733b55abf
• Upwork Job ID: 1757021871380135936
• Last Price Increase: 2024-02-12
• Automatic offers:
• rayane-djouah | Contributor | 0

[melvin-bot]

Triggered auto assignment to @bfitzexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01a6d9657733b55abf

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @allroundexperts (External)

[abzokhattab]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Add support for redirecting user to a target URL(via exitTo query param) when they click on the magic link code

What is the root cause of that problem?

new feature

What changes do you think we should make in order to solve the problem?

we need to modify the follwoing useEffect to check if the exitTo is present then it will navigate to it instead of the goBack

App/src/pages/ValidateLoginPage/index.tsx

Lines 16 to 28 in 484116c

	useEffect(() => {
	// Wait till navigation becomes available
	Navigation.isNavigationReady().then(() => {
	if (session?.authToken) {
	// If already signed in, do not show the validate code if not on web,
	// because we don't want to block the user with the interstitial page.
	Navigation.goBack();
	} else {
	Session.signInWithValidateCodeAndNavigate(Number(accountID), validateCode);
	}
	});
	// eslint-disable-next-line react-hooks/exhaustive-deps
	}, []);

additionaly we need to add another param exitTo in the signInWithValidateCodeAndNavigate function

App/src/libs/actions/Session/index.ts

Lines 507 to 510 in 207385a

	function signInWithValidateCodeAndNavigate(accountID: number, validateCode: string, twoFactorAuthCode = '') {
	signInWithValidateCode(accountID, validateCode, twoFactorAuthCode);
	Navigation.navigate(ROUTES.HOME);
	}

and use it in the navigation:

function signInWithValidateCodeAndNavigate(accountID: number, validateCode: string, twoFactorAuthCode = '', exitTo = ROUTES.HOME) {
    signInWithValidateCode(accountID, validateCode, twoFactorAuthCode);
    Navigation.navigate(exitTo);
}

[rayane-djouah]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Add support for redirecting user to a target URL(via exitTo query param) when they click on the magic link code

What is the root cause of that problem?

New feature

What changes do you think we should make in order to solve the problem?

we should add the new exitTo param here:

App/src/libs/Navigation/types.ts

Lines 443 to 446 in 72ffe93

	[SCREENS.VALIDATE_LOGIN]: {
	accountID: string;
	validateCode: string;
	};

and here:

App/src/pages/ValidateLoginPage/index.website.tsx

Lines 16 to 18 in 72ffe93

	route: {
	params: {accountID, validateCode},
	},

and here:

App/src/pages/ValidateLoginPage/index.tsx

Line 12 in 72ffe93

params: {accountID, validateCode},

we should check if an exitTo param is provided, in this case, we should navigate to it. here, and here.
for native, we should replace here signInWithValidateCodeAndNavigate with signInWithValidateCode and check if an exitTo param is provided, in this case, we should navigate to it, here, and here.

here is a draft PR with code changes for testing : #36390

here is the result recordings:

If the user is not logged in (and initiated the login process on the same browser)

1.Recording.2024-02-13.015036.mp4

If the user is not logged in (and initiated the login process in another browser)

3.Recording.2024-02-13.015229.mp4

If the user is already logged in

2.Recording.2024-02-13.015620.mp4

What alternative solutions did you explore? (Optional)

N/A

[techievivek]

I was already playing with changes similar to above, can you please add a branch for testing in your proposal, thanks.

[techievivek]

Cc @rayane-djouah @abzokhattab

[rayane-djouah]

Proposal

Updated

[techievivek]

Great, thanks for the screen recordings. It seems to behave the way we expected. @allroundexperts, can you please review the above proposals? This is a time-sensitive one, thanks. 🙇

[allroundexperts]

Both @rayane-djouah and @abzokhattab proposed the solution correctly. @abzokhattab was the first one to propose, but @rayane-djouah created a draft PR and shared the working screen recordings as well. Normally, the first one to propose gets the issue but since we need to move fast here, and the draft PR is already up, let's go with @rayane-djouah's proposal.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Current assignee @techievivek is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[melvin-bot]

📣 @rayane-djouah 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[rayane-djouah]

Thank you! I'm working on author checklist, will raise the PR shortly!

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

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

• Add support for redirecting user to a target URL when they click on the magic link code #36390

If no regressions arise, payment will be issued on 2024-02-26. 🎊

For reference, here are some details about the assignees on this issue:

• @allroundexperts requires payment through NewDot Manual Requests
• @rayane-djouah requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@allroundexperts] The PR that introduced the bug has been identified. Link to the PR:
• [@allroundexperts] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@allroundexperts] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@allroundexperts] Determine if we should create a regression test for this bug.
• [@allroundexperts] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@bfitzexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[bfitzexpensify]

Switching this to Daily so I'm on top of it for payment

[techievivek]

@rayane-djouah Another improvement that we can make here is that the magic code page appears for a moment. QA team shared this video.

306718766-3c0f57f0-a683-4849-a2c3-4ea77b84d505.mp4

Can you look into it and see if we can improve this.

[MitchExpensify]

Reminder set to pay

[bfitzexpensify]

@techievivek just checking in on your comment here - do you consider this a regression from the solution used in this issue?

[techievivek]

No, we can't classify it as a regression because it doesn't cause any general breakage. It would simply contribute to a smoother transition.

[bfitzexpensify]

Gotcha, thanks for clarifying.

Payment summary:

C+: $500 to be paid to @allroundexperts through NewDot Manual Requests
Contributor: $500 to be paid to @rayane-djouah via Upwork - complete ✅

@allroundexperts, a reminder to complete the BZ checklist when you get a chance - thanks!

[bfitzexpensify]

Bump on the BZ checklist @allroundexperts - thank you!

[allroundexperts]

@bfitzexpensify This is a new feature and checklist isn't needed here.

[bfitzexpensify]

Cool - closing this out. Thanks everyone!

[JmillsExpensify]

$500 approved for @allroundexperts based on summary.