[HOLD for payment 2023-10-10] [HOLD for payment 2023-10-09] Chat - Deeplinks - Skeleton is shown instead "You don’t have access" page

[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!

---

Action Performed:

1. Log in to NewDot
2. On the URL change the conversation ID to any combination of 1-4 numbers. E.g https://staging.new.expensify.com/r/1234

Expected Result:

Verify You see a You don't have access to this chat message in the conversation history

Actual Result:

Skeleton is shown instead You don’t have access to this chat message. Hmm page was shown 1 sec and then Skeleton appeared

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.75-2

Reproducible in staging?: Yes

Reproducible in production?: No

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

Notes/Photos/Videos: Any additional supporting documentation

Bug6219055_1695993441154.RPReplay_Final1695942665.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

[melvin-bot]

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

[OSBotify]

👋 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 @johnmlee101 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[lanitochka17]

Issue reproduced on build 1.3.75.3 iPhone 14 Pro/ 17.0.2

RPReplay_Final1696009484.MP4

[situchan]

Confirmed this regression came from #26772
cc: @ospfranco @mountiny

[fedirjh]

I couldn’t find the root cause within the linked PR #26772 , it may be caused by Onyx's latest updates.

When Onyx returns null value for a given report (not found report), the default value is passed to the ReportScreen component.

The line causing this bug is :

App/src/pages/home/ReportScreen.js

Line 307 in 07e0a5a

() => (!_.isEmpty(report) && !isDefaultReport && !report.reportID && !isOptimisticDelete && !report.isLoadingReportActions && !isLoading) || shouldHideReport,

isDefaultReport is set to true when the report equals the default value and this made the condition for displaying the not found view fails.

I couldn’t understand the purpose of this function and why it was used, it was implemented in this PR #26602 . Removing it will fix this issue.

App/src/pages/home/ReportScreen.js

Lines 113 to 118 in 07e0a5a

	* Function to check weather the report available in props is default
	*
	* @param {Object} report
	* @returns {Boolean}
	*/
	const checkDefaultReport = (report) => report === defaultProps.report;

cc @b4s36t4 could please help debug this issue?

[b4s36t4]

@fedirjh I will check and update here soon.

[b4s36t4]

@fedirjh that function was written because when I access a thread that I'm not part (previously I have) for a just a moment the prop is having default report.

It's a check to figure out to show loading screen as the report is still loading. Last time I checked for a invalid reportid we're having a different value with report. Maybe with the linked PR we're having it as the default value.

If removing that check work, I highly suggest you should the behavior for leaving a thread and joining a thread. If you don't see any issues then that's fine else we might have to figure out something.

[fedirjh]

@b4s36t4 It seems to be working fine with removing that condition, the old issue is not reproducible as well,

What is the expected behavior offline for the other issue?

CleanShot.2023-09-29.at.23.03.38.mp4

[b4s36t4]

@fedirjh You need to access the report fast means like maybe do a network simulation if it works fine then we can remove.

[b4s36t4]

If I access a thread I don't have access even in offline we should show loader view.

[fedirjh]

You need to access the report fast means like maybe do a network simulation if it works fine then we can remove.

With a slow network, it is still reproducible.

[mountiny]

Taking this on to manage the fix

[mountiny]

fixed

[melvin-bot]

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

[melvin-bot]

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

• [CP Staging] Fixes not found page not being shown properly when not having access to report #28584

If no regressions arise, payment will be issued on 2023-10-09. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

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

• @fedirjh requires payment

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@fedirjh] The PR that introduced the bug has been identified. Link to the PR:
• [@fedirjh] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@fedirjh] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@fedirjh] Determine if we should create a regression test for this bug.
• [@fedirjh] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@sonialiap] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

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

• [CP Staging] Fixes not found page not being shown properly when not having access to report #28584

If no regressions arise, payment will be issued on 2023-10-10. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

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

• @fedirjh requires payment

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@fedirjh] The PR that introduced the bug has been identified. Link to the PR:
• [@fedirjh] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@fedirjh] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@fedirjh] Determine if we should create a regression test for this bug.
• [@fedirjh] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@sonialiap] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.