[HOLD for payment 2024-08-02] [$250] mWeb - Room - System message for room description with markdown is shown as html code

[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.2
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4672130
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to https://staging.new.expensify.com/home
2. Tap on a room chat
3. Tap header-- room description
4. Enter 🦢💫😁🤡
5. Save the description
6. In room conversation page, note the system message of room description saved

Expected Result:

System message for room description with markdown must not be shown as html code

Actual Result:

System message for room description with markdown is shown as html code

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

Bug6524679_1719360869830.Screenrecorder-2024-06-26-05-37-06-269_compress_1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01c48ce1e75fd4d291
• Upwork Job ID: 1814416039583443889
• Last Price Increase: 2024-07-19
• Automatic offers:
• allgandalf | Contributor | 103199145

Issue Owner

Current Issue Owner: @miljakljajic

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

[lanitochka17]

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

[lanitochka17]

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

[bernhardoj]

Caused by #44300

[melvin-bot]

@miljakljajic Huh... This is 4 days overdue. Who can take care of this?

[miljakljajic]

@roryabraham @yuwenmemon is this a regression from #44300?

[melvin-bot]

@miljakljajic Eep! 4 days overdue now. Issues have feelings too...

[lanitochka17]

Issue is still reproducible on the latest build 9.0.9-1

Screenrecorder-2024-07-18-22-29-22-847.mp4

[bernhardoj]

Because it's not fixed yet, I posted my proposal.

Proposal

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

The system message of the room description update is shown with an HTML tag.

What is the root cause of that problem?

If the action is an update room description, we use ReportActionItemBasicMessage.

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

Lines 652 to 655 in 9c5297e

	} else if (action.actionName === CONST.REPORT.ACTIONS.TYPE.ROOM_CHANGE_LOG.UPDATE_ROOM_DESCRIPTION) {
	const message = `${translate('roomChangeLog.updateRoomDescription')} ${(originalMessage as OriginalMessageChangeLog)?.description}`;
	children = <ReportActionItemBasicMessage message={message} />;
	} else if (ReportActionsUtils.isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.DISMISSED_VIOLATION)) {

ReportActionItemBasicMessage only renders it as plain text. This happens after #44300 which will also make the message translatable. Previously, we didn't check for the update room description action and let it be rendered by ReportActionItemMessage which uses TextCommentFragment that will render the HTML correctly. The fragment is constructed from the action message as shown below.

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

Lines 45 to 51 in ffea2ab

	const actionMessage = action.previousMessage ?? action.message;
	let fragments: Message[] = [];
	if (Array.isArray(actionMessage)) {
	fragments = actionMessage.filter((item): item is Message => !!item);
	} else {
	fragments = actionMessage ? [actionMessage] : [];
	}

App/src/pages/home/report/comment/TextCommentFragment.tsx

Lines 55 to 80 in 9c5297e

	if (!shouldRenderAsText(html, text ?? '') && !(containsOnlyEmojis && styleAsDeleted)) {
	const editedTag = fragment?.isEdited ? `<edited ${styleAsDeleted ? 'deleted' : ''} ${containsOnlyEmojis ? 'islarge' : ''}></edited>` : '';
	const htmlWithDeletedTag = styleAsDeleted ? `<del>${html}</del>` : html;
	const htmlContent = containsOnlyEmojis ? Str.replaceAll(htmlWithDeletedTag, '<emoji>', '<emoji islarge>') : htmlWithDeletedTag;
	let htmlWithTag = editedTag ? `${htmlContent}${editedTag}` : htmlContent;
	if (styleAsMuted) {
	htmlWithTag = `<muted-text>${htmlWithTag}<muted-text>`;
	}
	return (
	<RenderCommentHTML
	source={source}
	html={htmlWithTag}
	/>
	);
	}
	const message = isEmpty(iouMessage) ? text : iouMessage;
	return (
	<Text style={[containsOnlyEmojis && styles.onlyEmojisText, styles.ltr, style]}>
	<ZeroWidthView
	text={text}
	displayAsGroup={displayAsGroup}

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

We need to use ReportActionItemMessage back by removing this code

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

Lines 652 to 654 in 9c5297e

	} else if (action.actionName === CONST.REPORT.ACTIONS.TYPE.ROOM_CHANGE_LOG.UPDATE_ROOM_DESCRIPTION) {
	const message = `${translate('roomChangeLog.updateRoomDescription')} ${(originalMessage as OriginalMessageChangeLog)?.description}`;
	children = <ReportActionItemBasicMessage message={message} />;

and to keep make it translatable, in ReportActionItemMessage, we need to construct a custom fragments array for the update room description action.

let fragments: Message[] = [];
if (action.actionName === CONST.REPORT.ACTIONS.TYPE.ROOM_CHANGE_LOG.UPDATE_ROOM_DESCRIPTION) {
    const message = `${translate('roomChangeLog.updateRoomDescription')} ${ReportActionsUtils.getOriginalMessage(action)?.description}`;
    fragments = [{text: message, html: `<muted-text>${message}</muted-text>`, type: 'COMMENT'}];
} else {
    if (Array.isArray(actionMessage)) {
        fragments = actionMessage.filter((item): item is Message => !!item);
    } else {
        fragments = actionMessage ? [actionMessage] : [];
    }
}

Last, we need to use Parser.htmlToText for the description here

App/src/libs/SidebarUtils.ts

Line 429 in ffea2ab

result.alternateText = `${lastActorDisplayName} ${Localize.translate(preferredLocale, 'roomChangeLog.updateRoomDescription')} ${lastActionOriginalMessage?.description}`.trim();

[melvin-bot]

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

[melvin-bot]

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

[bernhardoj]

PR is ready

[allgandalf]

I was assigned to the linked PR as C+, does that require a C+ review @roryabraham @miljakljajic ?

[roryabraham]

Didn't mean to unassign the original C+, but now that you're assigned to review the PR you can keep it. Thanks in advance for your review @allgandalf

[melvin-bot]

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

[allgandalf]

Thanks, I will complete the checklist over the weekend 👍

[roryabraham]

PR is merged

[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 9.0.12-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix room update description system message shows as html tag #45818

If no regressions arise, payment will be issued on 2024-08-02. 🎊

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

• @bernhardoj requires payment through NewDot Manual Requests
• @allgandalf requires payment automatic offer (Contributor)

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

• [@allgandalf] The PR that introduced the bug has been identified. Link to the PR:
• [@allgandalf] 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:
• [@allgandalf] 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:
• [@allgandalf] Determine if we should create a regression test for this bug.
• [@allgandalf] 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.
• [@miljakljajic] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[allgandalf]

• The PR that introduced the bug has been identified. Link to the PR: fixes the "Room chat LHN preview displays "removed 0 user" after editing room description " #44300

• 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: https://github.com/Expensify/App/pull/44300/files#r1694545132

• 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: N/A

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

Regression Test Proposal

• Open a room or create a new one
• Update the room description to contain an emoji/markdown
• Verify the update room description system message is rendered as markdown
• Go back to LHN
• Verify the room last message shows the message as plain text

Do we agree 👍 or 👎

[miljakljajic]

@bernhardoj is owed 250 USD for their work reviewing this issue

[miljakljajic]

@allgandalf paid you in upwork!

[bernhardoj]

Requested in ND.

[JmillsExpensify]

$250 approved for @bernhardoj