Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HOLD for payment 2025-01-18] [$250] Troubleshoot - Use staging server option is disabled upon logging in and after clearing cache #54620

Closed
1 of 8 tasks
izarutskaya opened this issue Dec 27, 2024 · 28 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Weekly KSv2

Comments

@izarutskaya
Copy link

izarutskaya commented Dec 27, 2024

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


Version Number: 9.0.79-0
Reproducible in staging?: Yes
Reproducible in production?: Unable to check as it testing preference
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: No, reproducible on hybrid only
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/cases/view/2761712
Email or phone of affected tester (no customers): N/A
Issue reported by: Applause Internal Team
Device used: iPhone 15 iOS 18.1.1
App Component: User Settings

Action Performed:

  1. Open staging Hybrid app and log in
  2. Navigate to Settings/ Troubleshoot
  3. Scroll down to the Use staging server option and verify if it is enabled
  4. Enable the option manually
  5. In the Troubleshoot tap Clear cache and restart, then Reset and refresh
  6. Check the Use staging server option toggle position

Expected Result:

Use staging server option is enabled automatically upon log in and remains enabled after clearing cache

Actual Result:

Use staging server option is disabled upon logging in and after clearing cache

Workaround:

Unknown

Platforms:

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6701433_1734979627116.ScreenRecording_12-23-2024_10-27-31_1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021872694579402665672
  • Upwork Job ID: 1872694579402665672
  • Last Price Increase: 2024-12-27
  • Automatic offers:
    • allgandalf | Reviewer | 105510487
    • ikevin127 | Contributor | 105510488
Issue OwnerCurrent Issue Owner: @anmurali
@izarutskaya izarutskaya added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 27, 2024
Copy link

melvin-bot bot commented Dec 27, 2024

Triggered auto assignment to @carlosmiceli (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

Copy link

melvin-bot bot commented Dec 27, 2024

Triggered auto assignment to @anmurali (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

Copy link

melvin-bot bot commented Dec 27, 2024

💬 A slack conversation has been started in #expensify-open-source

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Dec 27, 2024
Copy link
Contributor

👋 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.

@carlosmiceli carlosmiceli added External Added to denote the issue can be worked on by a contributor and removed DeployBlockerCash This issue or pull request should block deployment labels Dec 27, 2024
@melvin-bot melvin-bot bot changed the title Troubleshoot - Use staging server option is disabled upon logging in and after clearing cache [$250] Troubleshoot - Use staging server option is disabled upon logging in and after clearing cache Dec 27, 2024
Copy link

melvin-bot bot commented Dec 27, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 27, 2024
Copy link

melvin-bot bot commented Dec 27, 2024

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

@carlosmiceli
Copy link
Contributor

Removing DB since it's a staging issue, but I'm not 100% sure it can be solved by contributors. Adding the label so we get confirmation.

@carlosmiceli carlosmiceli added Daily KSv2 and removed Hourly KSv2 labels Dec 27, 2024
@ikevin127
Copy link
Contributor

Edited by proposal-police: This proposal was edited at 2024-12-28 00:22:34 UTC.

Proposal

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

Use staging server option is disabled upon logging in and after clearing cache.

What is the root cause of that problem?

Note

The issue is present on all platform, it's not specific to HybridApp.

This happens because shouldUseStagingServer boolean is stored in ONYXKEYS.USER key (see reference) and when Clear cache and restart is executed, clearOnyxAndResetApp is called here which clears onyx storage except for KEYS_TO_PRESERVE which doesn't include ONYXKEYS.USER where shouldUseStagingServer boolean is stored.

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

Preserve ONYXKEYS.USER object key shouldUseStagingServer and its value such that Use Staging Server option remains toggled ON after Clear cache and restart is executed in case it was ON before.

One way to execute this is to only preserve shouldUseStagingServer from ONYXKEYS.USER:

  • In App.ts where clearOnyxAndResetApp is located, extract and preserve shouldUseStagingServer similar to what was done here with preservedUserSession, and just like here, preserve shouldUseStagingServer which we would write like this:
// top of `App.ts` under `preservedUserSession` logic:
let preservedShouldUseStagingServer: boolean | undefined;
Onyx.connect({
    key: ONYXKEYS.USER,
    callback: (value) => {
        preservedShouldUseStagingServer = value?.shouldUseStagingServer;
    },
});

// above Onyx.clear(KEYS_TO_PRESERVE) block:
const shouldUseStagingServer = preservedShouldUseStagingServer; // <- we do this because `let preservedShouldUseStagingServer` will be lost once Onyx is cleared, so we need to store it in a constant before clearing (below)

// within the `clearOnyxAndResetApp` function, inside the Onyx.clear(KEYS_TO_PRESERVE) block:
if (shouldUseStagingServer) {
    Onyx.set(ONYXKEYS.USER, {shouldUseStagingServer});
}

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

We can test that after clearOnyxAndResetApp was called, ONYXKEYS.USER object with key shouldUseStagingServer is preserved.

What alternative solutions did you explore? (Optional)

An alternative way to execute this would be to preserve the whole ONYXKEYS.USER object:

  • which can contain multiple key / values that we might not want to preserve:
{
    isUsingExpensifyCard: false,
    isFromPublicDomain: true,
    hasAccessibleDomainPolicies: false,
    isGuide: false,
    isSubscribedToNewsletter: true,
    shouldUseStagingServer: true, // <- this one is of interest to us for this issue
    validated: false,
}

to do this we would simply pass ONYXKEYS.USER here to KEYS_TO_PRESERVE.

Result video (before / after)

MacOS: Chrome
Before After
before.mov
after.mov

@allgandalf
Copy link
Contributor

@carlosmiceli first things first, this was not a deploy blocker:

Screenshot 2024-12-31 at 2 17 22 AM

QA marked it as one because we don't show these options in production.

Now, coming to the proposal, @ikevin127 's RCA is indeed correct. As for the solution is concerned, the USER onyx object must be cleared on purpose, so i agree with their main solution to preserve the shouldUseStagingServer key. Let's go with their proposal here

🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented Dec 30, 2024

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

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 30, 2024
Copy link

melvin-bot bot commented Dec 30, 2024

📣 @allgandalf 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

Copy link

melvin-bot bot commented Dec 30, 2024

📣 @ikevin127 🎉 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 📖

@melvin-bot melvin-bot bot added the Weekly KSv2 label Dec 30, 2024
@ikevin127
Copy link
Contributor

PR #54673 is now ready for review! 🚀

@ikevin127
Copy link
Contributor

♻️ PR is and was ready for review for 3 days, still awaiting @allgandalf's review.

@allgandalf
Copy link
Contributor

♻️ PR is on staging

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jan 11, 2025
@melvin-bot melvin-bot bot changed the title [$250] Troubleshoot - Use staging server option is disabled upon logging in and after clearing cache [HOLD for payment 2025-01-18] [$250] Troubleshoot - Use staging server option is disabled upon logging in and after clearing cache Jan 11, 2025
Copy link

melvin-bot bot commented Jan 11, 2025

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

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jan 11, 2025
Copy link

melvin-bot bot commented Jan 11, 2025

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

If no regressions arise, payment will be issued on 2025-01-18. 🎊

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

Copy link

melvin-bot bot commented Jan 11, 2025

@ikevin127 / @allgandalf @anmurali @ikevin127 / @allgandalf The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

@allgandalf
Copy link
Contributor

@carlosmiceli we never considered the case to store the value of the toggle in Onyx, so this should be treated as a new feature, If you agree with this can you remove the bug label and add new feature label, Then this issue would only require a regression test rather than the checklist, or do you disagree with the ^?

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jan 17, 2025
@carlosmiceli carlosmiceli added NewFeature Something to build that is a new item. and removed Bug Something is broken. Auto assigns a BugZero manager. labels Jan 17, 2025
Copy link

melvin-bot bot commented Jan 17, 2025

Current assignee @anmurali is eligible for the NewFeature assigner, not assigning anyone new.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Jan 17, 2025
Copy link

melvin-bot bot commented Jan 17, 2025

⚠️ It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time ⚠️

Copy link

melvin-bot bot commented Jan 17, 2025

Triggered auto assignment to Design team member for new feature review - @dubielzyk-expensify (NewFeature)

@carlosmiceli
Copy link
Contributor

@allgandalf yeah, that makes sense! Just switched the labels 👍

@ikevin127
Copy link
Contributor

Not sure who's supposed to handle payments here since this was changed to New Feature and is now labeled as Weekly🤔

cc @carlosmiceli @anmurali for visibility regarding payments

@dubielzyk-expensify
Copy link
Contributor

Need any design eyes on this?

@allgandalf
Copy link
Contributor

no thanks for asking @dubielzyk-expensify , only regression test and payment pending

@allgandalf
Copy link
Contributor

Regression Test Proposal

  1. Open app and log in.
  2. Navigate to Settings > Troubleshoot.
  3. Scroll down to the Use Staging Server option and enabled it.
  4. In the Troubleshoot tap Clear cache and restart, then confirm with Reset and refresh.

Verify that: Use Staging Server option toggle is persisted to ON (Enabled) if it was enabled before.

Do we agree 👍 or 👎

@anmurali
Copy link

Paid and added the test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Weekly KSv2
Projects
None yet
Development

No branches or pull requests

6 participants