[Hold, Testing][$500] Theme - Theme is always reset to "Use Device Settings" after relogin

[izarutskaya]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: v1.4.13-4
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause-Internal Team
Slack conversation: @

Action Performed:

Precondition: App theme is set to Light mode.

1. Launch New Expensify app.
2. Log out.
3. Log in to the same account.
4. Go to Settings > Preferences > Theme.

Expected Result:

The theme preference before logout is preserved.

Actual Result:

The theme is reset to "Use Device Settings" after relogin.

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6314462_1702641664249.RPReplay_Final1702620718.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0146344d6185cdc37b
• Upwork Job ID: 1735649568056549376
• Last Price Increase: 2024-01-05

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0146344d6185cdc37b

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

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

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

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

[ikevin127]

Proposal

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

The theme is reset to "Use Device Settings" after relogin.

What is the root cause of that problem?

The theme onyx data is cleared when logout is performed.

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

We can pass successData to LogOut where we set the ONYXKEYS.PREFERRED_THEME to the value of the theme.

Changed code:

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

+    let preferredTheme: ValueOf<typeof CONST.THEME> | null = null;
+    Onyx.connect({
+        key: ONYXKEYS.PREFERRED_THEME,
+        callback: (val) => (preferredTheme = val),
+    });

/**
 * Clears the Onyx store and redirects user to the sign in page
 */
function signOut() {
    ...
+    const successData = [
+        {
+            onyxMethod: Onyx.METHOD.SET,
+            key: ONYXKEYS.PREFERRED_THEME,
+            value: preferredTheme,
+        },
+    ];

+    API.write('LogOut', params, {successData});
    ...

What alternative solutions did you explore? (Optional)

App/src/libs/actions/SignInRedirect.ts

Line 15 in 121ca59

function clearStorageAndRedirect(errorMessage?: string) {

Preserve theme value in clearStorageAndRedirect function by adding ONYXKEYS.PREFERRED_THEME to keysToPreserve array.

Both solutions fulfil the expected result.

Videos

MacOS: Chrome / Safari

Screen.Recording.2023-12-15.at.16.32.24.mov

[shubham1206agra]

Proposal

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

Theme - Theme is always reset to "Use Device Settings" after relogin

What is the root cause of that problem?

We are not preserving ONYXKEYS.PREFERRED_THEME on logout.

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

We should preserve the ONYXKEYS.PREFERRED_THEME key in clearStorageAndRedirect function.

Line

App/src/libs/actions/SignInRedirect.ts

Lines 19 to 22 in 121ca59

	const keysToPreserve: OnyxKey[] = [];
	keysToPreserve.push(ONYXKEYS.NVP_PREFERRED_LOCALE);
	keysToPreserve.push(ONYXKEYS.ACTIVE_CLIENTS);
	keysToPreserve.push(ONYXKEYS.DEVICE_ID);

[Julesssss]

This shouldn't block deploy, removing the label

[Charan-hs]

Proposal

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

Theme is always reset to "Use Device Settings" after relogin

What is the root cause of that problem?

When the User Logs Out we are Flushing Most of Values Stored in Onyx Store.
and we are currently not storing the theme value in either the Onyx store or the BE.

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

1. We can store Theme Value along with User ID so that when User A sets the theme as "Dark", and when User B Login It should be a conflict.
2. We can Store in locally or In BE. Before Flushing we can make it as persit or when singing out we push theme value to BE.

What alternative solutions did you explore? (Optional)

[danieldoglas]

@situchan please review the proposals

[danieldoglas]

Also, I think it makes sense to assign @grgia to this issue since she's the person with the most knowledge on our side

[grgia]

I am fixing this internally in https://github.com/Expensify/Web-Expensify/pull/40078

[melvin-bot]

Current assignee @situchan is eligible for the Internal assigner, not assigning anyone new.

[grgia]

Although @situchan maybe we should go with one of these proposals on App side. Do you have thoughts on this?

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

@michaelhaxhiu, @grgia, @situchan Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

@michaelhaxhiu, @grgia, @situchan 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[melvin-bot]

@michaelhaxhiu @grgia @situchan this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

@michaelhaxhiu, @grgia, @situchan 10 days overdue. Is anyone even seeing these? Hello?

[melvin-bot]

@michaelhaxhiu, @grgia, @situchan 10 days overdue. I'm getting more depressed than Marvin.

[melvin-bot]

@michaelhaxhiu, @grgia, @situchan 12 days overdue now... This issue's end is nigh!

[situchan]

Let me check possibility to fix in frontend

[grgia]

Otherwise I believe this will be fixed by https://github.com/Expensify/Web-Expensify/pull/40078 (internal)

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

@michaelhaxhiu @grgia @situchan this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[grgia]

We've merged internal auth PR, will retest after changes are deployed