[Hold for payment 2024-05-30][$250] Deleted message disappear for a moment when transistioning from offline to online

[kavimuru]

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.57-3
Reproducible in staging?: y
Reproducible in production?: y
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
Expensify/Expensify Issue URL:
Issue reported by: @roryabraham
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1711665611722399?thread_ts=1711571474.268739&cid=C01GTK53T8Q

Action Performed:

1. Go offline
2. Send a message
3. Reply to the message
4. Delete the message you sent in (2)
5. Come back online

Expected Result:

While waiting for the network request to process deleted message should appear strikethrough and then deleted message should appear

Actual Result:

While network request from offline to online processes, the message deleted from step 2 will disappear rather than showing [Deleted Message].

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

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017b4b082efdd47324
• Upwork Job ID: 1795973516571975680
• Last Price Increase: 2024-05-30

[melvin-bot]

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

[melvin-bot]

Current assignee @roryabraham is eligible for the DeployBlockerCash assigner, not assigning anyone new.

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

[allgandalf]

The issue mentions reproducible in both so NAB right ? @roryabraham @kavimuru

[shubham1206agra]

@roryabraham Please assign me as C+ here as I was C+ on the previous issue too.

[yuwenmemon]

Yeah if this is reproducible in both this is not a blocker.

[sonialiap]

Since this is not a blocker, moving to Daily

[FitseTLT]

Proposal

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

Deleted message disappear for a moment when transistioning from offline to online

What is the root cause of that problem?

In here

App/src/libs/actions/Report.ts

Lines 1315 to 1316 in 99cc0b6

	isDeletedParentAction
	? [WRITE_COMMANDS.UPDATE_COMMENT]

We haven't included ADD_COMMENT when the report action is isThreadParentMessage so as we created the report action offline (ADD_COMMENT) the isDeletedReportAction set as true by DELETE_COMMENT of the parent message is getting reset when going online and the isDeletedReportAction is being reset to false and as the message.html is empty it will disappear and get not displayed

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

We need to add ADD_COMMENT for parent messages too here as they are conflicting requests

App/src/libs/actions/Report.ts

Lines 1315 to 1316 in 99cc0b6

	isDeletedParentAction
	? [WRITE_COMMANDS.UPDATE_COMMENT]

isDeletedParentAction
                        ? [WRITE_COMMANDS.ADD_COMMENT, WRITE_COMMANDS.UPDATE_COMMENT]

What alternative solutions did you explore? (Optional)

[sonialiap]

@shubham1206agra what do you think of the above proposal?

[roryabraham]

putting this on HOLD for #39432, as there's a good likelihood this will be fixed over there

[bernhardoj]

Proposal

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

Deleted thread parent message disappears when go online instead of showing [Deleted message]

What is the root cause of that problem?

In ReportActionsList, we are filtering out any action that has a pending action of DELETE.

App/src/pages/home/report/ReportActionsList.tsx

Lines 188 to 194 in b0a50cb

	const sortedVisibleReportActions = useMemo(
	() =>
	sortedReportActions.filter(
	(reportAction) =>
	(isOffline || reportAction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || reportAction.errors) &&
	ReportActionsUtils.shouldReportActionBeVisible(reportAction, reportAction.reportActionID),
	),

It's not happening while offline because we include isOffline condition on the filter logic. The filter was first introduced in #31670 and the filtered report actions were previously used only in this useEffect.

App/src/pages/home/report/ReportActionsList.tsx

Lines 218 to 229 in b0a50cb

	useEffect(() => {
	if (
	scrollingVerticalOffset.current < AUTOSCROLL_TO_TOP_THRESHOLD &&
	previousLastIndex.current !== lastActionIndex &&
	reportActionSize.current > sortedVisibleReportActions.length &&
	hasNewestReportAction
	) {
	reportScrollManager.scrollToBottom();
	}
	previousLastIndex.current = lastActionIndex;
	reportActionSize.current = sortedVisibleReportActions.length;
	}, [lastActionIndex, sortedVisibleReportActions, reportScrollManager, hasNewestReportAction, linkedReportActionID]);

But after the comment linking, we add ReportActionsUtils.shouldReportActionBeVisible condition and use it to render the list items too. We add ReportActionsUtils.shouldReportActionBeVisible because the report actions from the report screen include all actions.

App/src/pages/home/ReportScreen.tsx

Line 707 in b0a50cb

selector: (allReportActions: OnyxEntry<OnyxTypes.ReportActions>) => ReportActionsUtils.getSortedReportActionsForDisplay(allReportActions, true),

App/src/libs/ReportActionsUtils.ts

Lines 609 to 619 in b0a50cb

	function getSortedReportActionsForDisplay(reportActions: ReportActions | null | ReportAction[], shouldIncludeInvisibleActions = false): ReportAction[] {
	let filteredReportActions: ReportAction[] = [];
	if (!reportActions) {
	return [];
	}
	if (shouldIncludeInvisibleActions) {
	filteredReportActions = Object.values(reportActions);
	} else {
	filteredReportActions = Object.entries(reportActions)
	.filter(([key, reportAction]) => shouldReportActionBeVisible(reportAction, key))

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

First of all, whether we use sortedVisibleReportActions to render the list or not, the first filter condition is not complete
(isOffline || reportAction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || reportAction.errors)

because we don't consider the case for deleted parent action (isDeletedParentAction).

So, my suggestion is to include ReportActionsUtils.isDeletedParentAction(reportAction) condition.

-(isOffline || reportAction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || reportAction.errors)
+(isOffline || ReportActionsUtils.isDeletedParentAction(reportAction) || reportAction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || reportAction.errors)

This is enough, but if we see the above condition, it's similar to shouldReportActionBeVisible.

App/src/libs/ReportActionsUtils.ts

Lines 500 to 502 in b0a50cb

	const isDeleted = isDeletedAction(reportAction);
	const isPending = !!reportAction.pendingAction;
	return !isDeleted || isPending || isDeletedParentAction(reportAction) || isReversedTransaction(reportAction);

The main difference is, shouldReportActionBeVisible includes all action with pending action of DELETE. I'm thinking of updating shouldReportActionBeVisible to include pending action of DELETE only while offline so we don't need to use the first condition, but I'm not sure because shouldReportActionBeVisible is being used in many places and I don't know what regression would wait us.

[shubham1206agra]

@bernhardoj's proposal looks good.

🎀 👀 🎀 C+ Reviewed

[melvin-bot]

Triggered auto assignment to @iwiznia, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[roryabraham]

next step - waiting for @bernhardoj to open a PR

[bernhardoj]

Sorry for the delay, PR is ready for review

cc: @shubham1206agra

[melvin-bot]

This issue has not been updated in over 15 days. @sonialiap, @roryabraham, @bernhardoj, @shubham1206agra eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[roryabraham]

PR was deployed to prod a week ago. Ready to pay.

[melvin-bot]

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

[melvin-bot]

Current assignee @shubham1206agra is eligible for the External assigner, not assigning anyone new.

[sonialiap]

Payment summary:

• @bernhardoj $500 - paid ✔️
• @shubham1206agra $500 - paid ✔️

[shubham1206agra]

@sonialiap I think the issue was created before the April 6 deadline. The bounty should be $500 here.

[sonialiap]

@shubham1206agra updated :)

[shubham1206agra]

@sonialiap Accepted

[sonialiap]

paid ✔️