[Network] Language Picker on Login Page call the API unnecessarily & does not behave correctly

[parasharrajat]

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

---

ISSUES

1. Language Picker calls the API which is unnecessary and It blocks the network queue.
2. Its theme does not match other inputs in the app. No border, etc.
3. We have to select the two times to make language change take effect.

3. We have to select the language two times to switch it.

Action Performed:

1. Open the login page & sign in with some account.
2. Sign out of the app.
3. Switch a language.
4. Check Console logs for seeing the blocked network queue.

Expected Result:

1. No API calls should be made instead language should be set on Onyx only.
2. The picker theme should match with the theme of other Pickers.
3. It should work in the first change.

Actual Result:

1. It blocks the queue and makes an unnecessary backend request.
2. Styling does not match the app theme.
3. We have to select the language two times to activate it.

Workaround:

None. UI issue

Platform:

Where is this issue occurring?

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

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

login-picker.mp4

Expensify/Expensify Issue URL:

View all open jobs on Upwork

view this job on Upwork

Proposal

1. Correct the Dropdown theme so that it shows the outline when opened.
2. Block the API call when Language is selected. To do this we can add a prop offline to the LocalPicker which can then pass to

   App/src/libs/actions/App.js

   Line 38 in 8b10c1c

   function setLocale(locale) {

   to block the API call when offline is true.
3. Or, based on the suggestion from Peter, we can simply check for the currentUserAccountID before making the API call as this is only present when the user is logined.

[MelvinBot]

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

[MelvinBot]

@cdraeger11 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[parasharrajat]

We have to select the two times to make language change take effect

This seems another issue with Onyx. ONYXKEYS.NVP_PREFERRED_LOCALE is being reset to en from es for the first attempt after logout.

I tested the withLocalize HOC that if it's getting correct preferredLocale but its not. LocalePicker only run the setLocale one time with es next time it is skipped due to the locale !== preferredLocale. But we get en from the Onyx as preferredLocale. I don't know much of Onyx so asking help from cc: @kidroca. may be its happening to other keys as well.

[kidroca]

I'll take a look and post back

[kidroca]

I've found the problem while debugging

It can be traced to the 20th second of this video where I follow the steps to reproduce with the deubgger

Eexpensify_Locale_Picker_On_Login_Screen_Debugging.mp4

• clue: 1 Between the 20 and 25 seconds you can see a call to Onyx.clear being made
• clue: 2 At 1:20 a Onyx.set call that sets the es locale again is observed - it originates from redirectToSignIn

---

Short version: A network request to save the locale should not be made, because there is no logged in user.
In such cases (missing authToken) the internal logic redirects back to login, which resets storage and applies some defaults - see below

---

The problem comes from the network request that happens as it involves addDefaultValuesToParameters

1. Network request to set locale

App/src/libs/actions/App.js

Lines 38 to 41 in 8b10c1c

	function setLocale(locale) {
	API.PreferredLocale_Update({name: 'preferredLocale', value: locale});
	Onyx.merge(ONYXKEYS.NVP_PREFERRED_LOCALE, locale);
	}

2. addDefaultValuesToParameters calls redirectToSignIn

App/src/libs/API.js

Lines 51 to 60 in 8b10c1c

	function addDefaultValuesToParameters(command, parameters) {
	const finalParameters = {...parameters};
	if (isAuthTokenRequired(command) && !parameters.authToken) {
	// If we end up here with no authToken it means we are trying to make an API request before we are signed in.
	// In this case, we should cancel the current request by pausing the queue and clearing the remaining requests.
	if (!authToken) {
	redirectToSignIn();
	console.debug('A request was made without an authToken', {command, parameters});

There's no apiToken and so redirectToSignIn is called

3. redirectToSignIn clears storage and resets locale

App/src/libs/actions/SignInRedirect.js

Lines 46 to 53 in 8b10c1c

	const preferredLocale = currentPreferredLocale;
	// Clearing storage discards the authToken. This causes a redirect to the SignIn screen
	Onyx.clear()
	.then(() => {
	if (preferredLocale) {
	Onyx.set(ONYXKEYS.NVP_PREFERRED_LOCALE, preferredLocale);
	}

Here currentPreferredLocale is the locale before the update, because connections aren't notified with the new value yet
When the connections are notified with the updated value this function already runs and preferredLocale is set
And this later causes the bug where the locale does not get updated

[parasharrajat]

Oh. Great. Network requests will definitely be removed. Good to know that it fixes the issue.

[kidroca]

Regarding the proposal, you don't have to go through this "Block the API call when Language is selected. To do this we can add a prop offline to the LocalPicker which can then pass to"
You don't need to pass a prop to the LocalePicker and then pass it to setLocale as parameter

setLocale already has access to the session here

App/src/libs/actions/App.js

Lines 13 to 19 in 8b10c1c

	let currentUserAccountID;
	Onyx.connect({
	key: ONYXKEYS.SESSION,
	callback: (val) => {
	currentUserAccountID = lodashGet(val, 'accountID', '');
	},
	});

So the same information can be extracted like

let currentUserAccountID;
let isLoggedIn = false;
Onyx.connect({
    key: ONYXKEYS.SESSION,
    callback: (val) => {
        currentUserAccountID = lodashGet(val, 'accountID', '');
        isLoggedIn = Boolean(lodashGet(val, 'authToken'));
    },
});

And then used in setLocale

function setLocale(locale) {
  if (isLoggedIn) {
      API.PreferredLocale_Update({name: 'preferredLocale', value: locale});
  }

  Onyx.merge(ONYXKEYS.NVP_PREFERRED_LOCALE, locale);
}

[kidroca]

Or the request can be skipped simply because there would not be a currentUserAccountID

function setLocale(locale) {
  if (currentUserAccountID) {
      API.PreferredLocale_Update({name: 'preferredLocale', value: locale});
  }

  Onyx.merge(ONYXKEYS.NVP_PREFERRED_LOCALE, locale);
}

[MelvinBot]

@cdraeger11 Still overdue 6 days?! Let's take care of this!

[MelvinBot]

@cdraeger11 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

[marcaaron]

Seems like there are some clear ideas here. Gonna move this one along.

[MelvinBot]

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

[jboniface]

job posted here

[MelvinBot]

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

[parasharrajat]

What do you think @Beamanator?

[Beamanator]

@parasharrajat would you mind re-posting exactly what the latest proposal is? :D Looks like it went through a few reviews so I just want to be clear what the latest plan is

[parasharrajat]

Sure.

1. Correct the Dropdown theme so that it matches other dropdowns in the app.
2. Block the API call when Language is selected. based on the suggestion we can just put a check in the setLocale for currentUserAccountID where If this is undefined don't make the API call.
   

   App/src/libs/actions/App.js

   Line 38 in 8b10c1c

   function setLocale(locale) {

3. Fix the language switching so that it works on the first attempt. It looks like blocking API call will fix it too. Thanks to Kidroca for help.

[Beamanator]

Thanks for confirming and re-posing @parasharrajat - The job is yours! Please submit a PR when you have a chance 👍

[botify]

🚀 Deployed to staging by @Beamanator in version: 1.0.88-3 🚀

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

[botify]

🚀 Deployed to production by @roryabraham in version: 1.0.90-2 🚀

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

[jboniface]

it looks like this should have been closed, right? or is there something still pending on this issue?

[parasharrajat]

@jboniface
Payment 🤑 .

---

Lol, what! I was never hired for this job. 🦀

[jboniface]

oh whoops, accept it and I'll pay you. I was expecting the new process where we show the "hold for payment" message in the title but this must have deployed before that

[jboniface]

ok i paid it (note: i put it up for 500 because you both reported and solved it).