[$250] Chat - For removed user system message, reply in thread page header doesn't shown mentions

[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.79-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: 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
Component: Chat Report View
Attachments
Drag a file

Action Performed:

1. Go to https://staging.new.expensify.com/home
2. Open a room chat
3. Invite a user and tap "invite them" in the whisper
4. Open reply in thread for the invited system message
5. Note header is shown invited with mentions
6. Go to members page tapping header
7. Remove the member
8. Go to room chat
9. Open reply in thread page for removed user system message

Expected Result:

For removed user system message, reply in thread page header must show mentions

Actual Result:

For removed user system message, reply in thread page header doesn't shown mentions

Workaround:

Unknown

Platforms:

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

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6702967_1735305558950.Screenrecorder-2024-12-27-18-39-29-74_compress_1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021873792561565329294
• Upwork Job ID: 1873792561565329294
• Last Price Increase: 2024-12-30

Issue Owner

Current Issue Owner: @dukenv0307

[melvin-bot]

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

[FitseTLT]

Edited by proposal-police: This proposal was edited at 2024-12-27 14:10:53 UTC.

Proposal

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

Chat - For removed user system message, reply in thread page header doesn't shown mentions

What is the root cause of that problem?

We are not specifically handling member change parent actions in getReportName just like we did for context menu action to parse the mentions

App/src/pages/home/report/ContextMenu/ContextMenuActions.tsx

Lines 412 to 414 in 12e0941

	const logMessage = ReportActionsUtils.getMemberChangeMessageFragment(reportAction).html ?? '';
	setClipboardMessage(logMessage);
	} else if (reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.UPDATE_NAME) {

so it is falling back to the report action message parsed without the mentions

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

We should handle it specifically here to align how they are displayed in ReportActionItemMessage below here or above here

if (ReportActionsUtils.isMemberChangeAction(parentReportAction)) {
        const logMessage = ReportActionsUtils.getMemberChangeMessageFragment(parentReportAction).html ?? '';
        return Parser.htmlToText(logMessage);
    }

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

We can add a test for getReportName to assert it returns the correct report name for chat threads with this type of parent report action

What alternative solutions did you explore? (Optional)

If needed we can also make the LHN text consistent but it looks like we are intentionally displaying removed 1 user kind of text by handling them specifically here

[truph01]

Proposal

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

• For removed user system message, reply in thread page header doesn't shown mentions

What is the root cause of that problem?

• Assume the HTML for the "removed user system message" is:

<muted-text>removed <mention-user accountID=18866836></mention-user></muted-text>

• When retrieving the report name to display in the header, the function parseReportActionHtmlToText is used.

• Within this function, the incorrect mentionUserRegex is applied:

App/src/libs/ReportUtils.ts

Line 3913 in 12e0941

const mentionUserRegex = /<mention-user accountID="(\d+)" *\/>/gi;

• The existing regex only matches HTML like:

<muted-text>removed <mention-user accountID=18866836 /></muted-text>

but does not match the assumed HTML format.

• As a result, the output is only removed. A similar issue occurs with the invited action.

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

• We can convert the html from <any-tag></any-tag> to <any-tag /> and use that value instead. So in here, add:

    const transformedHtml = html.replace(/<mention-user accountID="?(\d+)"?\s*><\/mention-user>/gi, '<mention-user accountID="$1" />');

then use it in here and here instead of raw html.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

• Add test cases to ensure the updated regex correctly matches both formats of the component: <any-tag></any-tag> and <any-tag />.

What alternative solutions did you explore? (Optional)

• We can update the mention user regex in:

App/src/libs/ReportUtils.ts

Line 3913 in 12e0941

const mentionUserRegex = /<mention-user accountID="(\d+)" *\/>/gi;

to make sure to match both <any-tag></any-tag> and <any-tag />:

const mentionUserRegex = /<mention-user accountID=(\d+|"(\d+)")(?: *\/>|>(?:.*?)<\/mention-user>)/gi;

• The similar should be applied to this in expensify-common.

[mkzie2]

Edited by proposal-police: This proposal was edited at 2024-12-31 05:17:07 UTC.

Proposal

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

For removed user system message, reply in thread page header doesn't shown mentions

What is the root cause of that problem?

We've already handled the invite/remove message for parent report action of a thread message here

App/src/libs/ReportUtils.ts

Lines 4072 to 4074 in 12e0941

	if (isAdminRoom(report) || isUserCreatedPolicyRoom(report)) {
	return getAdminRoomInvitedParticipants(parentReportAction, reportActionMessage);
	}

But this function always returns early because we're using the || condition here. This condition is always true because if an action is isPolicyChangeLogAction, it will not be isRoomChangeLogAction and vice versa.

App/src/libs/ReportUtils.ts

Lines 3835 to 3837 in 12e0941

	if (!ReportActionsUtils.isPolicyChangeLogAction(parentReportAction) || !ReportActionsUtils.isRoomChangeLogAction(parentReportAction)) {
	return parentReportActionMessage || Localize.translateLocal('parentReportAction.deletedMessage');
	}

getAdminRoomInvitedParticipants always returns parentReportActionMessage and we get it in parseReportActionHtmlToText with mentioned regex here that doesn't cover the mention like <muted-text>removed <mention-user accountID=...></mention-user></muted-text> (this the the regex of invite/remove room member system message).

App/src/libs/ReportUtils.ts

Line 3886 in dae99d1

function parseReportActionHtmlToText(reportAction: OnyxEntry<ReportAction>, reportID: string, childReportID?: string): string {

So the thread header only shows removed

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

1. We should update this condition correctly by using && instead of ||. For the bug of parseReportActionHtmlToText function, I think it should be handled in a separate issue since it's not the RCA of this issue.

if (!ReportActionsUtils.isPolicyChangeLogAction(parentReportAction) ?? !ReportActionsUtils.isRoomChangeLogAction(parentReportAction)) { 
   return parentReportActionMessage || Localize.translateLocal('parentReportAction.deletedMessage'); 
} 

App/src/libs/ReportUtils.ts

Lines 3835 to 3837 in 12e0941

	if (!ReportActionsUtils.isPolicyChangeLogAction(parentReportAction) || !ReportActionsUtils.isRoomChangeLogAction(parentReportAction)) {
	return parentReportActionMessage || Localize.translateLocal('parentReportAction.deletedMessage');
	}

2. The reason is for the action, we use getEffectiveDisplayName function that will prioritize the login of the user

App/src/libs/ReportActionsUtils.ts

Line 1186 in 8921546

const handleText = PersonalDetailsUtils.getEffectiveDisplayName(personalDetail) ?? Localize.translateLocal('common.hidden');

and for the header, we use getDisplayNameForParticipant function that will prioritize the display name.

App/src/libs/ReportUtils.ts

Line 3843 in 8921546

const name = getDisplayNameForParticipant(id);

That is inconsistent we also need to fix it by using the same function getEffectiveDisplayName or getDisplayNameForParticipant.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

We need to add a test case for getAdminRoomInvitedParticipants function and verify that it returns the correct message for isPolicyChangeLogAction, isRoomChangeLogAction and other actions

What alternative solutions did you explore? (Optional)

NA

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[melvin-bot]

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

[melvin-bot]

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

[dukenv0307]

@mkzie2's proposal look promissing

But this function always returns early because we're using the || condition here. This condition is always true because if an action is isPolicyChangeLogAction, it will not be isRoomChangeLogAction and vice versa.

But I see the inconsistency in the header (displayName) and action (email) for users who have displayName. Can you check it?

[mkzie2]

But I see the inconsistency in the header (displayName) and action (email) for users who have displayName. Can you check it?

@dukenv0307 The reason is for the action, we use getEffectiveDisplayName function that will prioritize the login of the user

App/src/libs/ReportActionsUtils.ts

Line 1186 in 8921546

const handleText = PersonalDetailsUtils.getEffectiveDisplayName(personalDetail) ?? Localize.translateLocal('common.hidden');

and for the header, we use getDisplayNameForParticipant function that will prioritize the display name.

App/src/libs/ReportUtils.ts

Line 3843 in 8921546

const name = getDisplayNameForParticipant(id);

We should only use one of them for both places. Updated proposal.