[$250] Search - Duplicate 'Concierge' are displayed on Search page

[m-natarajan]

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: v1.4.67-0
Reproducible in staging?: y
Reproducible in production?: n
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: Applause internal team
Slack conversation:

Action Performed:

1. Sign up with a new account
2. Finish the onboarding flow
3. Click on 'Search' icon

Expected Result:

There shouldn't be duplicate Concierge result

Actual Result:

There is duplicate Concierge result

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

Bug6463560_1714203662919.Screen_Recording_2024-04-27_at_10.33.17_in_the_morning.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01472b58e48bd6509a
• Upwork Job ID: 1784315238702837760
• Last Price Increase: 2024-05-04

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @cristipaval (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

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

[m-natarajan]

@cristipaval 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.

[anmurali]

I can reproduce this but it resolves in a few seconds and then subsequent searches only pull up one result. So I don't think this needs to be an hourly issue..

[melvin-bot]

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

[melvin-bot]

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

[skyweb331]

Proposal

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

Exclude duplicated concierge options (and now DMs too) in the search results.

What is the root cause of that problem?

Those two concierge results are both from [email protected].
Search options are built from reports and personalDetails mostly, and this concierge appeared on both cases, one as report and one as personal detail.

When creating options, we fetch participants for each report. and this logic is changed in #40271 to the following.

App/src/libs/OptionsListUtils.ts

Line 1459 in 84c4535

const accountIDs = isSelfDM ? [currentUserAccountID ?? 0] : Object.keys(report.participants ?? {}).map(Number);

Current concierge report has 2 participants, one is [email protected] and the other one is me.
This logic change affects assigning login for search option in createOption function

App/src/libs/OptionsListUtils.ts

Lines 734 to 738 in 84c4535

	if (!hasMultipleParticipants) {
	result.login = personalDetail?.login;
	result.accountID = Number(personalDetail?.accountID);
	result.phoneNumber = personalDetail?.phoneNumber;
	}

Concierge report has 2 participants, so it passes this if cases, thus, search option from concierge report has undefined for login property.
Due to undefined for login property, when generating options from reports, [email protected] will not be pushed into optionsToExclude here.

App/src/libs/OptionsListUtils.ts

Lines 1799 to 1801 in 84c4535

	if (reportOption.login) {
	optionsToExclude.push({login: reportOption.login});
	}

Thus, [email protected] is included from personalDetails.

Then, why visiting concierge report and opening search does not show duplicated result?

It's because, #40271 has missed applying correct logics here.

App/src/libs/OptionsListUtils.ts

Lines 1490 to 1498 in e138a59

	function createOptionFromReport(report: Report, personalDetails: OnyxEntry<PersonalDetailsList>) {
	const isSelfDM = ReportUtils.isSelfDM(report);
	const accountIDs = isSelfDM ? [currentUserAccountID ?? 0] : report.participantAccountIDs ?? [];
	return {
	item: report,
	...createOption(accountIDs, personalDetails, report, {}),
	};
	}

Search options are updated when report changes, and above function is used in this case.
As you can see, it is using old logic, so search option for concierge report will have [email protected] for login property, thus, [email protected] is included in optionsToExclude for personalDetails.

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

1. Apply Fix group chat isn't shows in search page after split #40271 logic when report changes too.
2. Duplicated results are all from DMs, so add DM logins to optionsToExclude list, to ignore it from personalDetails.

This is already done on my side and checked it fully functioned.
skyweb331@16fb637

What alternative solutions did you explore? (Optional)

n/a

[rayane-djouah]

@skyweb331, as this bug was considered a deploy blocker, can you identify the offending PR from #41122 (comment)?

[skyweb331]

Two concierge results appear on search result because of #37519.
According to this PR, when generating search options, accountIds are from report.participants (before this, report.visibleChatMemberAccountIDs) and concierge report has only one participants (it's concierge) for visibleChatMemberAccountIDs and two participants ([email protected] and current user ) for participants
Due to this, Concierge report is appeared and we see two concierges.

Current production excludes the concierge report from reports and includes [email protected] from personalDetails, but I think, concierge report should be included and [email protected] should be excluded from personalDetails, so getting participants from report.participants is correct and [email protected] should be excluded from personalDetails

And this way, we can keep #37519 working as intended

[rayane-djouah]

@skyweb331, Thanks for the proposal, I reverted #40271 and confirmed that it's the offending PR.
However, the root cause of your proposal seems unclear to me.

Can you clarify the following:

• Why currently there are two concierge options the first time when the concierge report is unread and then when we open the report the duplicate option disappears?

two participants ([email protected] and current user ) for participants, Due to this, Concierge report is appeared and we see two concierges.

• Can you elaborate on how this is causing duplicate options?

Current production excludes the concierge report from reports and includes [email protected] from personalDetails, but I think, concierge report should be included and [email protected] should be excluded from personalDetails, so getting participants from report.participants is correct and [email protected] should be excluded from personalDetails

When reverting the PR I see that the report option is included and the personalDetails option is not included. I think that this is expected behaviour.

Also, your suggested solution works, but I don't think we should exclude the Concierge from personalDetails as this may introduce a regression where the concierge personalDetails option will not be included when there is no Concierge report available for some reason. we should instead exclude a personalDetails option only if a corresponding report is available.

[skyweb331]

@rayane-djouah Did you revert the PR??? I see that PR is now on Production and I can see the same result on Production now.

[rayane-djouah]

@skyweb331 I mean that I reverted the PR locally for testing

[skyweb331]

Can we discuss this on slack?

[skyweb331]

@rayane-djouah
I found why concierge duplication disappears when concierge report is visited on production.

#40271 has missed one change. Following function should use report.participants according to the PR but it doesn't.

App/src/libs/OptionsListUtils.ts

Lines 1490 to 1498 in e138a59

	function createOptionFromReport(report: Report, personalDetails: OnyxEntry<PersonalDetailsList>) {
	const isSelfDM = ReportUtils.isSelfDM(report);
	const accountIDs = isSelfDM ? [currentUserAccountID ?? 0] : report.participantAccountIDs ?? [];
	return {
	item: report,
	...createOption(accountIDs, personalDetails, report, {}),
	};
	}

As I mentioned before, search options are created at the first search window is opened, and search options are updated when reports and personal details are updated. And #40271 missed applying the correct logic when a report is changed, so it looks like it works correct once you visit the concierge report. But this is completely incorrect.

We should use report.participants when report is changed too, to make #40271 works correctly for every cases, including initialization and report changes.

In this case, problem is not limited to only concierge report. I checked that current production shows duplicated search results for every DMs ( two participants, one is me ).

Recording.2024-04-30.144058.mp4

So my solution is to filter DMs participants from personalDetails and my code is here. This way, we can keep #40271 working correctly and remove the duplications.
skyweb331@16fb637

[rayane-djouah]

@skyweb331 According to contributing guidelines when you want to update an existing proposal, please go back and edit your original proposal, then post a new comment to the issue to alert everyone that it has been updated.

The proposal lacks a clear and easily understood root cause; please focus on finding an explanation for why the personal details option is not excluded when there is a corresponding report and try to debug the flow to find the root cause. This will help us avoid writing a workaround.
I find filtering DM participants from personal details a workaround because these options were getting excluded before, and now they aren't. Therefore, there is an existing logic to filter them, and we should find out why it's not doing so.

Please update your original proposal and tag me again when it's ready for review.

[skyweb331]

@rayane-djouah Updated proposal

[rayane-djouah]

@skyweb331, the root cause is making sense to me now.
For the solution, can we change the logic here instead:

App/src/libs/OptionsListUtils.ts

Lines 734 to 738 in 84c4535

	if (!hasMultipleParticipants) {
	result.login = personalDetail?.login;
	result.accountID = Number(personalDetail?.accountID);
	result.phoneNumber = personalDetail?.phoneNumber;
	}

[rayane-djouah]

The bug is fixed by #41232 and it's no longer reproducible on the latest main:

cc @anmurali

[skyweb331]

:) He fixed his bug... Took long time to discussion, no result for me

[melvin-bot]

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