[HOLD for payment 2021-09-15] Language getting automatically set to English while signing in

[thesahindia]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Your language should be set to Spanish
2. Go to signin page
3. Enter any email address or phone number
4. Try to go back to the signin page by clicking on the link below
5. Language will be set to English

Expected Result:

Language should not change

Actual Result:

Workaround:

No, Spanish Users might find it hard to navigate.

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number:
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:

View all open jobs on Upwork

[MelvinBot]

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

[thesahindia]

Proposal

We need to preserve the locale inside restartSignin function

I can do this by connecting to Onyx for current locale and use this block of code inside restartSignin function

const preferredLocale = currentPreferredLocale;
Onyx.clear().then(() => {
if (preferredLocale) {
Onyx.set(ONYXKEYS.NVP_PREFERRED_LOCALE, preferredLocale);
}
});

[MelvinBot]

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

[MelvinBot]

Triggered auto assignment to @arielgreen (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[deetergp]

Definitely something a external contributor can do, and @thesahindia's proposal sounds good.

[arielgreen]

https://www.upwork.com/jobs/~01f7d2db5bd805d8ca

[MelvinBot]

Triggered auto assignment to @tgolen (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[arielgreen]

@tgolen can you review @thesahindia's proposal?

@thesahindia please also be sure to apply to the Upwork job here.

[thesahindia]

@arielgreen Submitted the proposal 😄

[tgolen]

I would like to request some changes to your proposal to increase the quality of the code.

I would suggest refactoring this code slightly so that there is only one method in the entire codebase in actions/Session.js which calls Onyx.clear() and it would be a place to handle all of the "need to keep this after clearing Onyx". We are currently only doing this properly in redirectToSignIn() by saving the local and the active clients key.

[thesahindia]

@tgolen
Hi, I want to know if it'd be a bad idea to just use redirectToSignIn instead of restartSignin since redirectToSignIn is handling this situation in a better way.

App/src/libs/actions/SignInRedirect.js

Lines 28 to 34 in 69ee2fe

	/**
	* Clears the Onyx store and redirects to the sign in page.
	* Normally this method would live in Session.js, but that would cause a circular dependency with Network.js.
	*
	* @param {String} [errorMessage] error message to be displayed on the sign in page
	*/
	function redirectToSignIn(errorMessage) {

Here it says that If we move this function to Session.js then it can cause circular dependency. So, maybe moving the logic to Session.js is not a good idea. Let me know your thoughts.

[tgolen]

Yeah, I'd be fine with getting rid of restartSignin completely. Good suggestion! I understand about the circular dependency. It is OK to stay in its own file like that.

With that then, I think it's a complete proposal and I give the 🟢 to be hired for this @arielgreen.

[arielgreen]

@thesahindia just sent over an offer! I also raised the price to include the bonus you'll receive for proposing AND fixing this.

[thesahindia]

Thanks @arielgreen, I have accepted the offer!

[thesahindia]

@tgolen
I have one last query about the use of currentURL inside src/libs/actions/SignInRedirect.js .
We are using it here only.

App/src/libs/actions/SignInRedirect.js

Lines 41 to 43 in e5957e1

	if (!currentURL) {
	return;
	}

And then there's no use case of it.
However, on the login screen the currentURL will not be set to any value and will fail this condition.
We can fix this by giving it a default value of "/" or we can just remove currentURL from the file as I don't see any other use case of it.

Let me know what should be done here. Thanks!

Correction: Using a default value will not fix it as it's being set to the value of null .

[tgolen]

It's entirely possible that currentURL can be removed completely. It was added in this PR to fix this GH. I think we need to be a little bit careful here because exitTo is still used in the VALIDATE_CODE_URL but it's used as a query parameter now as opposed to be part of the path. However, the VALIDATE_CODE_URL isn't used to construct a "new expensify" URL, it's used to construct a URL pointing to expensify.com.

So, I still think we are pretty safe to remove currentURL from this file, but you might want to double-check my investigation.

[tgolen]

In fact, I really don't like that VALIDATE_CODE_URL lives in ROUTES.js since it isn't related to a route for New Expensify. Since it's only referenced once, I would just move the logic from ROUTES.js into App.js where it was being referenced. Would you mind cleaning that up?

[thesahindia]

Sure, openSignedInLink will look like this after the clean-up.

function openSignedInLink(url = '') {
     API.GetAccountValidateCode().then((response) => {
     const exitToURL = url ? `exitTo=${url}` : '';
     const validateCodeUrl = `v/${currentUserAccountID}/${response.validateCode}${exitToURL}`;
     Linking.openURL(CONFIG.EXPENSIFY.URL_EXPENSIFY_COM + validateCodeUrl);
     });
}

[parasharrajat]

Sure, openSignedInLink will look like this after the clean-up.

function openSignedInLink(url = '') {
     API.GetAccountValidateCode().then((response) => {
     const exitToURL = url ? `exitTo=${url}` : '';
     const validateCodeUrl = `v/${currentUserAccountID}/${response.validateCode}${exitToURL}`;
     Linking.openURL(CONFIG.EXPENSIFY.URL_EXPENSIFY_COM + validateCodeUrl);
     });
}

Beware, This might be not working in Safari Mobile. Linking.openURL does not work here in the Async way. Please test it.

[thesahindia]

Oh, if this might break something then I think it's better to just leave it as it is?

[tgolen]

I don't think we need to worry about Linking.openURL() in this issue. It was already there previously, so nothing is breaking worse than it was already broken before. @parasharrajat Is there an open issue yet for fixing openURL() for mobile safari? If not, let's get one created.

[thesahindia]

Found this issue while testing the code.

For some reason, there's a delay in Android (but Web looks fine)

Android :

androidBug.mp4

Web :

video-2021-08-24_00.28.07.mp4

Not sure but it could be related to this issue #4513

[parasharrajat]

It is not related to #4513

[arielgreen]

@thesahindia can you please provide an update here?

[thesahindia]

The PR is up. Just waiting for a reply here
Edit: Merged!

[botify]

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

[botify]

🚀 Deployed to staging by @tgolen in version: 1.0.93-2 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

[botify]

🚀 Deployed to production by @yuwenmemon in version: 1.0.94-1 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

[tgolen]

It's OK melvin. It's not overdue

[arielgreen]

Paid.