[HOLD for payment 2024-11-26] [$250] Search - Unread message isnot shown in bold in search preview

[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.53-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/runs/view/26723&group_by=cases:section_id&group_order=asc&group_id=229066
Issue reported by: Applause - Internal Team

Action Performed:

1. Send a message from another account to this account and dont open it
2. Open search
3. Observe that the unread message is listed in recent preview but is not shown in bold

Expected Result:

Unread message should be shown in bold in chat preview

Actual Result:

Message is not shown in bold

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

Bug6644086_1729753637530.scrnli_5eMHEcX7CXtnS1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021851597452146176383
• Upwork Job ID: 1851597452146176383
• Last Price Increase: 2024-11-06
• Automatic offers:
• rojiphil | Reviewer | 104868161

Issue Owner

Current Issue Owner: @muttmuure

[melvin-bot]

Triggered auto assignment to @muttmuure (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]

@muttmuure 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-10-24 14:52:05 UTC.

Proposal

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

Search - Unread message isnot shown in bold in search preview

What is the root cause of that problem?

When we build the option we are setting isBold here

App/src/libs/OptionsListUtils.ts

Line 1954 in f46bce0

reportOption.isBold = shouldBoldTitleByDefault || shouldUseBoldText(reportOption);

but inside shouldUseBoldText we are trying to get notification preference from the option but it doesn't exist (the report data is included in reportOption.item) so it will set isBold false

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

This function is used in two cases inside getOption as mentioned above and here for LHN case but in LHN case option.item is undefined and the notification preference data is available in optionItem
so here

App/src/libs/OptionsListUtils.ts

Lines 2552 to 2553 in f46bce0

	function shouldUseBoldText(report: ReportUtils.OptionData): boolean {
	const notificationPreference = ReportUtils.getReportNotificationPreference(report);

for getReportNotificationPreference we should pass report.item (this will apply for getOptions case) and as a backup we will pass report (this will apply for LHN case)

function shouldUseBoldText(report: ReportUtils.OptionData | SearchOption): boolean {
    const notificationPreference = ReportUtils.getReportNotificationPreference(report.item ?? report);
    return report.isUnread === true && notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.MUTE && notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN;
}

What alternative solutions did you explore? (Optional)

[bernhardoj]

Edited by proposal-police: This proposal was edited at 2024-10-24 14:40:40 UTC.

Proposal

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

Unread report in search router modal isn't shown in bold text.

What is the root cause of that problem?

We already have a logic to decide whether to bold the text/title or not.

App/src/libs/OptionsListUtils.ts

Line 1963 in 642fd97

reportOption.isBold = shouldBoldTitleByDefault || shouldUseBoldText(reportOption);

Because search page wants to bold based on the unread status, shouldBoldTitleByDefault is false, so we rely on shouldUseBoldText which checks if the report is unread and the notification preference is not MUTE or HIDDEN.

App/src/libs/OptionsListUtils.ts

Lines 2626 to 2629 in 642fd97

	function shouldUseBoldText(report: ReportUtils.OptionData): boolean {
	const notificationPreference = ReportUtils.getReportNotificationPreference(report);
	return report.isUnread === true && notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.MUTE && notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN;
	}

shouldUseBoldText checks the notification preference from the report participants and defaults to HIDDEN if undefined. In our case, it's undefined.

App/src/libs/ReportUtils.ts

Lines 1227 to 1232 in 642fd97

	function getReportNotificationPreference(report: OnyxEntry<Report>, shouldDefaltToHidden = true): ValueOf<typeof CONST.REPORT.NOTIFICATION_PREFERENCE> {
	if (!shouldDefaltToHidden) {
	return report?.participants?.[currentUserAccountID ?? -1]?.notificationPreference ?? getDefaultNotificationPreferenceForReport(report);
	}
	return report?.participants?.[currentUserAccountID ?? -1]?.notificationPreference ?? CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN;
	}

That's because the report option doesn't have participants data and only participantsList.

App/src/libs/OptionsListUtils.ts

Lines 679 to 710 in 642fd97

	const result: ReportUtils.OptionData = {
	text: undefined,
	alternateText: undefined,
	pendingAction: undefined,
	allReportErrors: undefined,
	brickRoadIndicator: null,
	icons: undefined,
	tooltipText: null,
	ownerAccountID: undefined,
	subtitle: undefined,
	participantsList: undefined,
	accountID: 0,
	login: undefined,
	reportID: '',
	phoneNumber: undefined,
	hasDraftComment: false,
	keyForList: undefined,
	isDefaultRoom: false,
	isPinned: false,
	isWaitingOnBankAccount: false,
	iouReportID: undefined,
	isIOUReportOwner: null,
	iouReportAmount: 0,
	isChatRoom: false,
	shouldShowSubscript: false,
	isPolicyExpenseChat: false,
	isOwnPolicyExpenseChat: false,
	isExpenseReport: false,
	policyID: undefined,
	isOptimisticPersonalDetail: false,
	lastMessageText: '',
	};

participantsList contains the personal detail data of each participant.

App/src/libs/OptionsListUtils.ts

Lines 712 to 718 in 642fd97

	const personalDetailMap = getPersonalDetailsForAccountIDs(accountIDs, personalDetails);
	const personalDetailList = Object.values(personalDetailMap).filter((details): details is PersonalDetails => !!details);
	const personalDetail = personalDetailList.at(0);
	let hasMultipleParticipants = personalDetailList.length > 1;
	let subtitle;
	let reportName;
	result.participantsList = personalDetailList;

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

We can add participants to the report option data.

result.participants = report.participants;

And we can also consider replacing participantsList with participants to avoid confusion. To get the personal detail data, we can get it from PERSONAL_DETAILS_LIST onyx, keyed with the accountID from participants.

OR

Inside shouldUseBoldText, we can get the notification preference from report.notificationPreference because we already build it here.

App/src/libs/OptionsListUtils.ts

Line 745 in 642fd97

result.notificationPreference = ReportUtils.getReportNotificationPreference(report);

const notificationPreference = report.notificationPreference ?? ReportUtils.getReportNotificationPreference(report);

but I prefer the 1st solution and remove the notificationPreference when creating the option (createOption) so we will only have 1 source of truth, that is by using ReportUtils.getReportNotificationPreference(report).

[FitseTLT]

Only added a little bit of detailed explanation of the solution.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

@rojiphil, @muttmuure Huh... This is 4 days overdue. Who can take care of this?

[rojiphil]

Reviewing today

[rojiphil]

Thanks for your proposals.
@FitseTLT proposal is to use either report.item or report.
And @bernhardoj proposal is to use a single source of truth which I think is a better approach.

The purpose of optionData was to have only as much report data as needed. So, adding participants does not seem a good idea especially since we have already captured notificationPreference as the single source of truth. And, if needed, we can call ReportUtils.getReportNotificationPreference as a fallback.

@bernhardoj proposal with 2nd solution LGTM.
🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[srikarparsi]

Hey @luacmartins, is it currently expected for previews in search to be bolded when unread? I think it should be as well but I just wasn't sure if that is slated for later?

[luacmartins]

Yes, we should show them the same way as displayed in the LHN (bold in this case)

[melvin-bot]

@rojiphil, @muttmuure, @srikarparsi 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[melvin-bot]

📣 @rojiphil 🎉 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

[bernhardoj]

PR is ready

cc: @rojiphil

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

• Fix unread search results doesn't show the title in bold #52443

If no regressions arise, payment will be issued on 2024-11-26. 🎊

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

• @rojiphil requires payment automatic offer (Reviewer)
• @bernhardoj requires payment through NewDot Manual Requests

[melvin-bot]

@rojiphil @muttmuure @rojiphil The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[muttmuure]

$250 - @bernhardoj C+
$250 - @rojiphil C

[bernhardoj]

Requested in ND.

[rojiphil]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production
• 2b. Reported on staging (deploy blocker)
• 2c. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] 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:

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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: Not Required. The existing checklist is good enough to capture such issues.

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

Regression Test Proposal

Precondition:

The user must have unread chats

Test:

1. Open search from the search icon at the top
2. Verify the unread chats in the search list has bold title

Do we agree 👍 or 👎

[rojiphil]

$250 - @rojiphil

@muttmuure Completed checklist and accepted the offer too. Thanks

[JmillsExpensify]

$250 approved for @bernhardoj

[muttmuure]

Everyone is paid