[$250] Group chat - On deleting parent message,text "Deleted message" is not shown

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 9.0.21
Reproducible in staging?: Y
Reproducible in production?: Y
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. Launch app
2. Tap on a group chat
3. Send a message
4. Long press the sent message and select reply in thread
5. Send a message
6. Tap header and note header title is shown under avatar
7. Navigate back and delete the parent message
8. Tap header and note under avatar parent message is shown and not "deleted message " text as in header title

Expected Result:

In group chat, on deleting parent message, the text " Deleted message " must be shown under avatar

Actual Result:

In group chat, on deleting parent message, the text " Deleted message " is not shown under avatar

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

Bug6573760_1723805266501.Screenrecorder-2024-08-16-16-08-50-188_compress_1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0129be1229dfa60701
• Upwork Job ID: 1825843284584468948
• Last Price Increase: 2024-08-20
• Automatic offers:
• ahmedGaber93 | Reviewer | 103689325

Issue Owner

Current Issue Owner: @ahmedGaber93

[melvin-bot]

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

[lanitochka17]

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

[lanitochka17]

@bfitzexpensify 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

[FitseTLT]

Edited by proposal-police: This proposal was edited at 2024-08-16 12:36:08 UTC.

Proposal

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

On deleting parent message,text "Deleted message" is not shown

What is the root cause of that problem?

We are using getGroupChatName to get the report name here

App/src/pages/ReportDetailsPage.tsx

Lines 244 to 245 in d4d5a25

	const reportName = ReportUtils.isDeprecatedGroupDM(report) || isGroupChat ? ReportUtils.getGroupChatName(undefined, false, report) : ReportUtils.getReportName(report);

Because isGroupChat will be true for chat threads opened inside group chats

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

isGroupChat is not enough and we also need to check that it is not chat thread

    const reportName = ReportUtils.isDeprecatedGroupDM(report) || (isGroupChat && !isChatThread) ? ReportUtils.getGroupChatName(undefined, false, report) : ReportUtils.getReportName(report);

or

    const reportName = ReportUtils.isDeprecatedGroupDM(report) || (isGroupChat && !isThread) ? ReportUtils.getGroupChatName(undefined, false, report) : ReportUtils.getReportName(report);

same fix should be applied here too if we are not going to update isGroupChat as suggested in my alternative approach (which fix the problem from the root cause)

App/src/components/LHNOptionsList/OptionRowLHN.tsx

Line 134 in 578006f

const fullTitle = isGroupChat ? ReportUtils.getGroupChatName(undefined, false, report) : optionItem.text;

What alternative solutions did you explore? (Optional)

As chat threads with group chat as their parent report also have chatType as group; everywhere we are using isGroupChat in the app, we are wrongly including these chat threads. so the current code to determine isGroupChat is not correct and the one we should fix it from the root cause and avoid similar problems in the whole code base

App/src/libs/ReportUtils.ts

Lines 1140 to 1141 in d4d5a25

	function isGroupChat(report: OnyxEntry<Report> | Partial<Report>): boolean {
	return getChatType(report) === CONST.REPORT.CHAT_TYPE.GROUP;

so should be changed to

    return getChatType(report) === CONST.REPORT.CHAT_TYPE.GROUP && !isThread(report);

[bernhardoj]

Proposal

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

If a thread is created on a group chat and the thread is deleted with a message, the report detail page shows the thread name instead of [Deleted message]

What is the root cause of that problem?

If the report is a group chat, then we use getGroupChatName to get the name. A thread always inherits its parent chat type, so a thread in a group chat is a group chat.

App/src/pages/ReportDetailsPage.tsx

Line 244 in 578006f

const reportName = ReportUtils.isDeprecatedGroupDM(report) || isGroupChat ? ReportUtils.getGroupChatName(undefined, false, report) : ReportUtils.getReportName(report);

getGroupChatName itself returns the reportName of a report if found and a thread reportName is the thread parent message.

App/src/libs/ReportUtils.ts

Lines 2140 to 2144 in 578006f

	function getGroupChatName(participants?: SelectedParticipant[], shouldApplyLimit = false, report?: OnyxEntry<Report>): string | undefined {
	// If we have a report always try to get the name from the report.
	if (report?.reportName) {
	return report.reportName;
	}

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

Remove the group chat check and just use ReportUtils.getReportName. ReportUtils.getReportName already handles the group chat case.

App/src/libs/ReportUtils.ts

Lines 3736 to 3738 in 578006f

	if (isGroupChat(report)) {
	return getGroupChatName(undefined, true, report) ?? '';
	}

We need to fix in OptionRowLHN too by removing the group chat check.

App/src/components/LHNOptionsList/OptionRowLHN.tsx

Line 134 in 578006f

const fullTitle = isGroupChat ? ReportUtils.getGroupChatName(undefined, false, report) : optionItem.text;

[FitseTLT]

Updated to add a better alternative approach

[melvin-bot]

@bfitzexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

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

[melvin-bot]

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

[ahmedGaber93]

@bernhardoj In details page getGroupChatName use shouldApplyLimit with false value but getReportName use it with true value

App/src/pages/ReportDetailsPage.tsx

Lines 244 to 245 in d4d5a25

	const reportName = ReportUtils.isDeprecatedGroupDM(report) || isGroupChat ? ReportUtils.getGroupChatName(undefined, false, report) : ReportUtils.getReportName(report);

App/src/libs/ReportUtils.ts

Lines 3736 to 3738 in 578006f

	if (isGroupChat(report)) {
	return getGroupChatName(undefined, true, report) ?? '';
	}

I know this is may achieve the desired consistency in the other issue, but can you please add more context about this change affect? FYI, they have been added in this PR #40134

[bernhardoj]

Beside the limit, the change will simply make it consistent with other parts of the app where we prioritize other naming case before the group chat, such as the thread.

App/src/libs/ReportUtils.ts

Lines 3667 to 3733 in ad7c1d5

	if (parentReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.SUBMITTED) {
	return getIOUSubmittedMessage(parentReportAction);
	}
	if (parentReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.FORWARDED) {
	return getIOUForwardedMessage(parentReportAction);
	}
	if (parentReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.APPROVED) {
	return getIOUApprovedMessage(parentReportAction);
	}
	if (isChatThread(report)) {
	if (!isEmptyObject(parentReportAction) && ReportActionsUtils.isTransactionThread(parentReportAction)) {
	formattedName = getTransactionReportName(parentReportAction);
	if (isArchivedRoom(report, getReportNameValuePairs(report?.reportID))) {
	formattedName += ` (${Localize.translateLocal('common.archived')})`;
	}
	return formatReportLastMessageText(formattedName);
	}
	if (!isEmptyObject(parentReportAction) && ReportActionsUtils.isOldDotReportAction(parentReportAction)) {
	return ReportActionsUtils.getMessageOfOldDotReportAction(parentReportAction);
	}
	if (parentReportActionMessage?.isDeletedParentAction) {
	return Localize.translateLocal('parentReportAction.deletedMessage');
	}
	const isAttachment = ReportActionsUtils.isReportActionAttachment(!isEmptyObject(parentReportAction) ? parentReportAction : undefined);
	const reportActionMessage = getReportActionMessage(parentReportAction, report?.parentReportID, report?.reportID ?? '').replace(/(\n+|\r\n|\n|\r)/gm, ' ');
	if (isAttachment && reportActionMessage) {
	return `[${Localize.translateLocal('common.attachment')}]`;
	}
	if (
	parentReportActionMessage?.moderationDecision?.decision === CONST.MODERATION.MODERATOR_DECISION_PENDING_HIDE ||
	parentReportActionMessage?.moderationDecision?.decision === CONST.MODERATION.MODERATOR_DECISION_HIDDEN ||
	parentReportActionMessage?.moderationDecision?.decision === CONST.MODERATION.MODERATOR_DECISION_PENDING_REMOVE
	) {
	return Localize.translateLocal('parentReportAction.hiddenMessage');
	}
	if (isAdminRoom(report) || isUserCreatedPolicyRoom(report)) {
	return getAdminRoomInvitedParticipants(parentReportAction, reportActionMessage);
	}
	if (reportActionMessage && isArchivedRoom(report, getReportNameValuePairs(report?.reportID))) {
	return `${reportActionMessage} (${Localize.translateLocal('common.archived')})`;
	}
	if (!isEmptyObject(parentReportAction) && ReportActionsUtils.isModifiedExpenseAction(parentReportAction)) {
	return ModifiedExpenseMessage.getForReportAction(report?.reportID, parentReportAction);
	}
	if (isTripRoom(report)) {
	return report?.reportName ?? '';
	}
	return reportActionMessage;
	}
	if (isClosedExpenseReportWithNoExpenses(report)) {
	return Localize.translateLocal('parentReportAction.deletedReport');
	}
	if (isTaskReport(report) && isCanceledTaskReport(report, parentReportAction)) {
	return Localize.translateLocal('parentReportAction.deletedTask');
	}
	if (isGroupChat(report)) {
	return getGroupChatName(undefined, true, report) ?? '';
	}

Btw, if we look at the PR changes for ReportDetailsPage, we can see that previously we passed shouldApplyLimit as true.

Then, there is a discussion in the PR whether it's safe to pass shouldApplyLimit as true in ReportSettingsPage where we previously pass it as false. Then, this commit comes as the result of the discussion which changes shouldApplyLimit as false in ReportDetailsPage instead of ReportSettingsPage.

So, I think it's an overlook when changing the code.

[nkdengineer]

Proposal

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

In group chat, on deleting parent message, the text " Deleted message " is not shown under avatar

What is the root cause of that problem?

The thread of a group chat also has a chat type group then in this case we get the wrong report name of the thread by calling getGroupChatName

App/src/pages/ReportDetailsPage.tsx

Line 250 in 50de692

const reportName = ReportUtils.isDeprecatedGroupDM(report) || isGroupChat ? ReportUtils.getGroupChatName(undefined, false, report) : ReportUtils.getReportName(report);

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

We should only get the report name by calling getGroupChatName if the chat has a chat type group and not is a thread. We already have isRootGroupChat function that covers this case

const reportName = ReportUtils.isRootGroupChat(report) ? ReportUtils.getGroupChatName(undefined, false, report) : ReportUtils.getReportName(report); 

App/src/pages/ReportDetailsPage.tsx

Line 250 in 50de692

const reportName = ReportUtils.isDeprecatedGroupDM(report) || isGroupChat ? ReportUtils.getGroupChatName(undefined, false, report) : ReportUtils.getReportName(report);

We also need to fix in LHN here

const isGroupChat = ReportUtils.isRootGroupChat(report);

App/src/components/LHNOptionsList/OptionRowLHN.tsx

Line 132 in 50de692

const isGroupChat = ReportUtils.isGroupChat(optionItem) || ReportUtils.isDeprecatedGroupDM(optionItem);

What alternative solutions did you explore? (Optional)

[ahmedGaber93]

@bernhardoj's proposal LGTM!

getReprtName has the two cases isChatThread and isGroupChat that needed to fix this issue, also getReprtName used on the header here so using it on details page can fix the consistency issue #47803

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 @ahmedGaber93 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[ahmedGaber93]

Not overdue, the issue has just been assigned and we are waiting PR.

[bernhardoj]

PR is ready

cc: @ahmedGaber93

[bfitzexpensify]

I am heading out of office until September 21st, so assigning a buddy to watch over this in my absence.

Current status: PR in review

[melvin-bot]

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

[miljakljajic]

@bfitzexpensify my leave starts tomorrow so I will leave this with you

[ahmedGaber93]

Regression Test Proposal

1. Open a group chat
2. Send a message and reply in a thread
3. Delete the thread parent message
4. Open the details page
5. Verify the chat name is [Deleted message]
6. Verify the thread title in LHN is [Deleted message]

Do we agree 👍 or 👎

[ahmedGaber93]

@bfitzexpensify Bump for payment, The production deploy automation failed, but based on this deploy checklist #48954 This is issue was due for payment 2024-09-18.

[ahmedGaber93]

@bfitzexpensify Bump ^

[bfitzexpensify]

Payment summary:

@ahmedGaber93 due $250 for C+ work - offer sent via Upwork
@bernhardoj due $250 for contributor work via manual request

[bernhardoj]

Requested in ND.

[JmillsExpensify]

$250 approved for @bernhardoj

[ahmedGaber93]

@bfitzexpensify Offer accepted.

[bfitzexpensify]

Payment complete.