[$250] [HOLD for payment 2024-04-05] Workspace - Not here page shows up when opening message in thread in deleted workspace

[lanitochka17]

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: 1.4.51-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to workspace chat
3. Send a message
4. Delete the workspace
5. Return to the workspace chat
6. Hover over the message in Step 3 > Reply in thread

Expected Result:

The message will be opened in archived thread

Actual Result:

Not here page shows up when opening message in thread in deleted workspace

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

Add any screenshot/video evidence

Bug6411510_1710274628913.20240313_041445.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0129fa34eb79923fe5
• Upwork Job ID: 1777369739173396480
• Last Price Increase: 2024-04-08

[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 @blimpich (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[lanitochka17]

We think that this bug might be related to #vip-vsp
CC @quinthar

[lanitochka17]

@blimpich FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[lanitochka17]

Production:

Recording.1396.mp4

[allgandalf]

Proposal

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

What is the root cause of that problem?

Offending PR #35566

The double assertion !! actually leads to true and we get shouldShowNotFoundPage to true:

App/src/pages/home/ReportScreen.tsx

Line 454 in 1533507

(!!reportIDFromParams && !ReportUtils.isValidReportIDFromPath(reportIDFromParams)),

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

App/src/pages/home/ReportScreen.tsx

Line 442 in 1533507

const reportIDFromParams = route.params.reportID;

Set reportIDFromParams to '' if we don't have reportID

Then remove !!reportIDFromParams && this will give correct value to shouldShowNotFoundPage

What alternative solutions did you explore? (Optional)

N/A

[blimpich]

@GandalfGwaihir thank you for identifying the PR that caused this regression! However we generally regard deploy blockers to be the responsibility of who created them. So I would expect @VickyStash and @aimane-chnaif to handle this, as part of them completing #25216.

Demoting this since its not a particularly bad bug, and is a more niche scenario.

[blimpich]

Hmmm, for some reason GitHub won't let me assign @VickyStash to this issue. Either way, @VickyStash @aimane-chnaif can you handle this issue as a follow up to the typescript conversion?

[allgandalf]

No worries, @VickyStash can use my proposal to triage the issue faster :) thanks

[VickyStash]

@blimpich Sure, on my way to prepare follow-up PR!

[VickyStash]

Btw, the suggested proposal didn't work for me, I still see the same message.
And for me, it looks like the problem is in the first part of the shouldShowNotFoundPage variable condition, cause that's what turning to true for me.
I'm continuing to investigate...

[nkdengineer]

Proposal

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

• WS switcher resets to Expensify when creating a room under the same WS

What is the root cause of that problem?

• When replying in chat, in here we call navigateToAndOpenChildReport
• In that function, there is logic that navigate user to thread:
  

  App/src/libs/actions/Report.ts

  Lines 787 to 788 in 91d7eb2

  	Navigation.navigateWithSwitchPolicyID({route: ROUTES.HOME});
  	Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(report.reportID));

• There is the case that the report data is not stored successfully before navigating to it, so that leads to more than one OpenReport API is called. The 2nd call will set the report as null, that leads to the bug.

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

• We need to make sure the report in here is always stored successfully before calling Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(newChat.reportID)).
• We can do it by creating a isReadyPromise in ReportUtils like the one below:

let waitingReportID: string = ''
let resolveIsReadyPromise: (args?: unknown[]) => void;
let isReadyPromise =(reportID: string)=> new Promise((resolve) => {
    waitingReportID = reportID
    resolveIsReadyPromise = resolve;
});

function checkRequiredData() {
    const targetReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${waitingReportID}`] ?? {};
    if(targetReport?.reportID){
        resolveIsReadyPromise();
    }
   
}
Onyx.connect({
    key: ONYXKEYS.COLLECTION.REPORT,
    waitForCollectionCallback: true,
    callback: (value) => {
        allReports = value
        checkRequiredData()
    },
});

• Then update:
  

  App/src/libs/actions/Report.ts

  Line 841 in 91d7eb2

  Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(newChat.reportID));

  
  to:

        ReportUtils.isReadyPromise(newChat.reportID).then(()=>{
            Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(newChat.reportID));
        })

What alternative solutions did you explore? (Optional)

• We should consider other similar cases that leads to the bug, for example [HOLD for payment 2024-03-22] [$500] - MEDIUM - Workspace switcher - WS switcher resets to Expensify when creating a room under the same WS #37534

[allgandalf]

@VickyStash , i removed !!reportIDFromParams and it worked as expected for me, the value of !!reportIDFromParams is always set as true :)

[VickyStash]

@GandalfGwaihir I've tried twice with your solution, and I still see the same problem 😅 But overall this part of the code was added in this PR, and it initially had this check for reportIDFromParams, so I haven't changed the logic.

[aimane-chnaif]

@GandalfGwaihir I don't think that line is the root cause.
reportIDFromParams && !ReportUtils.isValidReportIDFromPath(reportIDFromParams) was original code.
When it's empty string, returns '' so it shouldn't affect shouldShowNotFoundPage logic even after changing it to boolean

[allgandalf]

What i mean to say is if we only have !ReportUtils.isValidReportIDFromPath(reportIDFromParams as a condition check, then this works fine and we are able to see threads in deleted messages :) so ig the problem would lie in the way we do !!reportIDFromParams

[aimane-chnaif]

@GandalfGwaihir I don't get what you mean. Please explain with example.
Old code: reportIDFromParams && !ReportUtils.isValidReportIDFromPath(reportIDFromParams)
New code: !!reportIDFromParams && !ReportUtils.isValidReportIDFromPath(reportIDFromParams)
Your solution: !ReportUtils.isValidReportIDFromPath(reportIDFromParams)

If this is the offending line, why reverting to old code doesn't work?

[VickyStash]

@GandalfGwaihir Do you exactly follow the flow described in the ticket? You mention deleted messages, but in the test steps, it says to remove the workspace

[aimane-chnaif]

@GandalfGwaihir Based on your comment, this bug should have been existed for a while, not a deploy blocker.

[allgandalf]

@aimane-chnaif , this is not reproducible in production though :)

simplescreenrecorder-2024-03-13_19.13.47.mp4

[melvin-bot]

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

[melvin-bot]

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

[miljakljajic]

@aimane-chnaif I am having issues hiring you for the job - can you please apply to it in upwork? I sent you an invite to the posting but I couldn't hire you:

[miljakljajic]

@aimane-chnaif is OOO, he'll accept when he comes back.

[blimpich]

not overdue

[miljakljajic]

still waiting on aimane

[miljakljajic]

Going to close this out and we can re-open when Aimane comes back. I have also DMed them in slack.

[aimane-chnaif]

@miljakljajic sorry I am back. Can you please reopen and sort payment on upwork? Thanks