Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.36k stars 2.79k forks source link

[$250] Task – Hmm… page appears for a moment when open task in public room as anon user #45564

Closed izarutskaya closed 2 months ago

izarutskaya commented 2 months ago

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: v9.0.8-1 Reproducible in staging?: Y Reproducible in production?: N If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4729138 Email or phone of affected tester (no customers): applausetester+jpcategory_1@applause.expensifail.com Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: Applause-Internal team

Action Performed:

  1. Go to https://staging.new.expensify.com/
  2. Log out
  3. Navigate via link of public room with a task (my link: https://staging.new.expensify.com/r/1353086947735283)
  4. Open task details

Expected Result:

Task detail opens without Hmm… page

Actual Result:

Hmm… page appears for a moment then task detail opens

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

https://github.com/user-attachments/assets/c630e778-bd45-403d-a3c3-d52b44999bb2

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0146cdf4505cc0858f
  • Upwork Job ID: 1813528846949260907
  • Last Price Increase: 2024-07-17
Issue OwnerCurrent Issue Owner: @eVoloshchak
melvin-bot[bot] commented 2 months ago

Triggered auto assignment to @twisterdotcom (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.

melvin-bot[bot] commented 2 months ago

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

github-actions[bot] commented 2 months ago

:wave: 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.
izarutskaya commented 2 months ago

We think this issue might be related to the #vip-vsb.

izarutskaya commented 2 months ago

Production

https://github.com/user-attachments/assets/5fafa43e-c5ad-42ba-b013-b4339c6f748d

twisterdotcom commented 2 months ago

vip-vsb is on hold at the moment, so I don't think we can call this a deploy blocker or hourly.

melvin-bot[bot] commented 2 months ago

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

melvin-bot[bot] commented 2 months ago

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

dominictb commented 2 months ago

Proposal

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

Hmm… page appears for a moment then task detail opens

What is the root cause of that problem?

We have the logic to trigger Onyx.connect in

https://github.com/Expensify/react-native-onyx/blob/c53826bbfe31e423fad0c33b0cb9ab097b88f128/lib/useOnyx.ts#L188

Then set isFirstConnectionRef.current = false and shouldGetCachedValueRef.current = true; in here

When isFirstConnectionRef=false and shouldGetCachedValueRef=true, the new value is get from cache

https://github.com/Expensify/react-native-onyx/blob/c53826bbfe31e423fad0c33b0cb9ab097b88f128/lib/useOnyx.ts#L135

at that time, it's undefined

-> reportMetadata is undefined

-> The skeleton is shown

https://github.com/Expensify/App/blob/43d464315cfc72b1191083203ad9cd879cdf2cf1/src/pages/home/ReportScreen.tsx#L405

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

We already said that

If `newValueRef.current` is `undefined` it means that the cache doesn't have a value for that key yet.
If `newValueRef.current` is `null` or any other value it means that the cache does have a value for that key

So we should use the initialValue if we're trying to get value from cache, but it's undefined

add the following code below this logic

if(!isFirstConnectionRef.current && newValueRef.current === undefined){
                newValueRef.current = options?.initialValue
            }

What alternative solutions did you explore? (Optional)

kaushiktd commented 2 months ago

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

Hmm… page appears for a moment then task detail opens

What is the root cause of that problem?

The "Hmm…" page appears due to incorrect or incomplete conditions being evaluated for rendering the task detail. Specifically, the current implementation does not adequately handle cases where a valid report ID exists, causing the system to default to the "Not Found" page.

The original condition for showing the "Not Found" page is maintained but is evaluated only if the valid report ID check does not succeed. https://github.com/Expensify/App/blob/43d464315cfc72b1191083203ad9cd879cdf2cf1/src/pages/home/ReportScreen.tsx#L404

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

Valid Report ID Check Added: Introduced a preliminary check to validate if the currentReportIDFormRoute is a valid report ID using ReportUtils.isValidReportIDFromPath. This ensures that valid report IDs are recognized correctly.

if (currentReportIDFormRoute && ReportUtils.isValidReportIDFromPath(currentReportIDFormRoute)) {
    return false;
}

video:- screen-capture.webm

twisterdotcom commented 2 months ago

How are these proposals @eVoloshchak?

eVoloshchak commented 2 months ago

This bug was reproducible a couple of days ago, but I can't reproduce it anymore with the latest main

@dominictb, @kaushiktd, could you please double check this is still reproducible for you?

dominictb commented 2 months ago

Hmmm, I can't reproduce this bug

kaushiktd commented 2 months ago

@eVoloshchak Yesterday morning, the code I pulled was still reproducing the error. However, the new code no longer reproduces it.

twisterdotcom commented 2 months ago

Okay, let's close! Thanks everyone for the input anyway.