[HOLD for payment 2023-12-15] [$500]Thread - Undefined displayed in header offline when opening left thread

[izarutskaya]

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. Open the app
2. Navigate to a conversation
3. Start a thread and send some messages
4. Disable internet connection
5. Tap the three dots and leave the thread
6. Open the thread again and rejoin

Expected Result:

The parent message is shown in the header or "hmm, it's not there" screen

Actual Result:

"Undefined (archived)" is displayed in the header and there is no option to rejoin

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.79-2

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

Notes/Photos/Videos: Any additional supporting documentation

Bug6227234_1696588130168.video_2023-10-06_06-27-43.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause-Internal Team

Slack conversation: @

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e4c2c4838dc79392
• Upwork Job ID: 1710244942675664896
• Last Price Increase: 2023-10-27
• Automatic offers:
• tienifr | Contributor | 27059051

Issue Owner

Current Issue Owner: @peterdbarkerUK

[melvin-bot]

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

[melvin-bot]

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

[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 - @ArekChr (External)

[izarutskaya]

On web and mweb there is no header at all, just the "U" avatar.
On Prod, it shows not here page. No infinite loading

Screen_Recording_20231006_150714_New.Expensify.mp4

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

[graylewis]

Proposal

Please re-state the problem that we are trying to solve in this issue.
When offline and attempting to rejoin a thread, the app displays "undefined (archived)" as the title of the chat on iOS and Android native. Optimally, there would be a message displayed here that says something like "You are offline" instead.

What is the root cause of that problem?
In the getReportName function, archived rooms are concatenated with "(archived)" without any check to see if the name of the room is defined. Since the only data available through onyx is the stateNum and the statusNum (this is the optimistic data flow), the name of the room is undefined and you end up with "undefined (archived)"

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

1. Add a check to see if formattedName is defined on line 1832 of ReportUtils.js. This is best practice and will prevent similar future potential errors. (This will ensure that the skeletons are shown rather than an error)
2. Since a 404 error doesn't really make sense (the thread exists after all) I would recommend either showing a "you are offline" placeholder or persisting the parent name of the thread to disk when the thread is deleted. If it's important that all left threads are not persisted to disk, then I'd recommend the "you are offline" placeholder.

What alternative solutions did you explore? (Optional)
NA

[tienifr]

This issue comes from this PR

Proposal

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

When trying to rejoin a thread in offline mode, it shows infinite loading and the the "U" avatar.

While currently in prod it will show not here page and no infinite loading.

What is the root cause of that problem?

In this line, we default the isLoadingReportActions to true before the reportMetadata is loaded. This means that in offline mode, if we access a report that we didn't open before, isLoadingReportActions will stay as true and the loading screen will show forever, because the Not Found page will not show in this case (see logic here).

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

We need to add an additional check to here, if the isLoadingReportActions or isOptimisticDelete is true but we're offline, the shouldShowNotFoundPage will still be true. This will make sure we show not found page correctly and the user is never stuck on the loading screen if the report was not loaded when offline.

const shouldShowNotFoundPage = useMemo(
        () => (!firstRenderRef.current && !report.reportID && (isOffline || (!reportMetadata.isLoadingReportActions && !isOptimisticDelete)) && !isLoading && !userLeavingStatus) || shouldHideReport,
        [report, reportMetadata, isLoading, shouldHideReport, isOptimisticDelete, userLeavingStatus, isOffline],
    );

This will make the behavior similar to current production app.

What alternative solutions did you explore? (Optional)

If the user is offline and the report is not loaded, an alternative is to show a message like Please go online to access this report. When the user goes online, if the report exists, it will load normally, if not, it will show not found page as usual.

[tienifr]

@mountiny FYI I have a proposal for this deploy blocker above, and can quickly raise the PR if assigned 👍.

cc @ArekChr

[melvin-bot]

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

[mountiny]

@tienifr Please go ahead!

[bernhardoj]

Isn't it correct to show the skeletons? When we reopen a left thread:

1. We don't have the report data yet,
2. We optimistically set isLoadingReportActions to true.

I can see the skeleton on prod too, so it SHOULDN'T be a blocker. Looking at the video here, looks like the app is on an old version because the header menu should be a three-dot icon now. It also shouldn't be related to this PR.

I think the real issue here is that the header shows a U avatar and an undefined chat title.

I will explain the root cause briefly (maybe?) here:
When we leave a thread, it will optimistically archive the report

App/src/libs/actions/Report.js

Lines 1913 to 1928 in 98f3b43

	API.write(
	'LeaveRoom',
	{
	reportID,
	},
	{
	optimisticData: [
	{
	onyxMethod: Onyx.METHOD.SET,
	key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
	value: {
	stateNum: CONST.REPORT.STATE_NUM.SUBMITTED,
	statusNum: CONST.REPORT.STATUS.CLOSED,
	},
	},
	],

We get the chat title from ReportUtils.getReportName. If it's an archived room, it will append the chat title (formattedName) with (archived)

App/src/libs/ReportUtils.js

Lines 1806 to 1808 in 98f3b43

	if (isArchivedRoom(report)) {
	formattedName += ` (${Localize.translateLocal('common.archived')})`;
	}

In our case, the left thread is an archived room. However, formattedName doesn't have an initial value. The logic above expects formattedName is always available before reaching the above if block.

App/src/libs/ReportUtils.js

Line 1769 in 98f3b43

let formattedName;

So, we get undefined (archived).

For the avatar, we get it from ReportUtils.getIcons. If it's an archived room, it will get the icon from getWorkspaceIcon.

App/src/libs/ReportUtils.js

Lines 1109 to 1112 in 98f3b43

	if (isAdminRoom(report) || isAnnounceRoom(report) || isChatRoom(report) || isArchivedRoom(report)) {
	const workspaceIcon = getWorkspaceIcon(report, policy);
	return [workspaceIcon];
	}

But because it's not a workspace, we get an "Unavailable workspace" icon.

Both avatar and title will be shown if title is not empty.

App/src/pages/home/HeaderView.js

Lines 215 to 244 in 98f3b43

	{Boolean(props.report && title) && (
	<View style={[styles.flex1, styles.flexRow, styles.alignItemsCenter, styles.justifyContentBetween]}>
	<PressableWithoutFeedback
	onPress={() => ReportUtils.navigateToDetailsPage(props.report)}
	style={[styles.flexRow, styles.alignItemsCenter, styles.flex1]}
	disabled={shouldDisableDetailPage}
	accessibilityLabel={title}
	accessibilityRole={CONST.ACCESSIBILITY_ROLE.BUTTON}
	>
	{shouldShowSubscript ? (
	<SubscriptAvatar
	mainAvatar={icons[0]}
	secondaryAvatar={icons[1]}
	size={defaultSubscriptSize}
	/>
	) : (
	<MultipleAvatars
	icons={icons}
	shouldShowTooltip={!isChatRoom || isChatThread}
	/>
	)}
	<View style={[styles.flex1, styles.flexColumn]}>
	<DisplayNames
	fullTitle={title}
	displayNamesWithTooltips={displayNamesWithTooltips}
	tooltipEnabled
	numberOfLines={1}
	textStyles={[styles.headerText, styles.pre]}
	shouldUseFullTitle={isChatRoom || isPolicyExpenseChat || isChatThread || isTaskReport}
	/>

So, I think the real solution here is to have an empty title instead of undefined (archived) to prevent showing both the wrong avatar and title by making sure formattedName is not empty before appending.

App/src/libs/ReportUtils.js

Lines 1806 to 1808 in 98f3b43

	if (isArchivedRoom(report)) {
	formattedName += ` (${Localize.translateLocal('common.archived')})`;
	}

With the above finding, I can also confirm this issue happened with the policy room.

If you agree with the above finding, then I will create a formal proposal.

[melvin-bot]

@shawnborton, @nkuoch, @peterdbarkerUK, @ArekChr, @mollfpr, @tienifr Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[peterdbarkerUK]

I'm OoO sick, will take care of payouts upon return (likely tomorrow). @ArekChr @mollfpr - could you check the regression list?

[mollfpr]

@peterdbarkerUK I hope you feel better soon! I'll do the manual request in NewDot for payment.

---

The PR that introduced the bug has been identified. Link to the PR:
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:

I couldn't find the PR causing the bug, there's a lot of commit to track it down.

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:

The regression step should be enough.

Determine if we should create a regression test for this bug.
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.

1. Open the App and open a chat
2. Start a thread and send some messages
3. Go offline
4. Click the three dots button on the thread header and select Leave
5. Open the thread from the main chat again
6. Verify skeleton shows in thread body, and title shows in the thread header

[peterdbarkerUK]

Thanks @mollfpr - $500 for the C+ Review, just a heads up that @JmillsExpensify is OoO until next week to action the payment.

@tienifr please accept the offer when you get a chance!

[melvin-bot]

@shawnborton, @nkuoch, @peterdbarkerUK, @ArekChr, @mollfpr, @tienifr Whoops! This issue is 2 days overdue. Let's get this updated quick!

[JmillsExpensify]

$500 payment approved for @mollfpr based on comment above.

[melvin-bot]

@shawnborton, @nkuoch, @peterdbarkerUK, @ArekChr, @mollfpr, @tienifr Eep! 4 days overdue now. Issues have feelings too...

[peterdbarkerUK]

@tienifr I can action the final payout once you've accepted the offer!

[tienifr]

@tienifr please accept the offer when you get a chance!

@peterdbarkerUK sorry, it seems that this offer has expired, would you mind recreating the offer?

Thanks!

[peterdbarkerUK]

No problem - here it is!

[melvin-bot]

@shawnborton, @nkuoch, @peterdbarkerUK, @ArekChr, @mollfpr, @tienifr Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[peterdbarkerUK]

@tienifr $500 paid, thanks!

[peterdbarkerUK]

Oop - will create the TR update in the morning

[melvin-bot]

@shawnborton, @nkuoch, @peterdbarkerUK, @ArekChr, @mollfpr, @tienifr Huh... This is 4 days overdue. Who can take care of this?

[peterdbarkerUK]

Done!