[HOLD for payment 2024-06-28] [$250] Search - Top result shows admin's email address

[izarutskaya]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 1.4.76-0
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/4574481&group_by=cases:section_id&group_order=asc&group_id=229067
Email or phone of affected tester (no customers): gocemate+a91@gmail.com
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

Preconditions:

1. Prepare an account that has a secondary login assigned

Steps:

1. Log in with the account that has the conversation with the user that has 2 logins
2. Click on FAB
3. Click on New Chat
4. Search for the Secondary login of the account
5. Verify you're redirected to the conversation with the primary login email
6. Go to Search and check results list

Expected Result:

Primary login should be on the top of the results list

Actual Result:

Top result shows admin's email address instead of last chat interacted with

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

Bug6493115_1716828834899.Recording__3050.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01ed8948bcc189ed41
• Upwork Job ID: 1798110219659124736
• Last Price Increase: 2024-06-11
• Automatic offers:
• bernhardoj | Contributor | 102706171

Issue Owner

Current Issue Owner: @kevinksullivan

[melvin-bot]

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

[Nodebrute]

Proposal

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

Top result shows admin's email address

What is the root cause of that problem?

In recent reports, reports with secondary logins have also been added. We should filter-out reports based on preexistingReportID

App/src/libs/OptionsListUtils.ts

Lines 1899 to 1905 in e5a1366

	if (
	!includeThreads &&
	(!!reportOption.login || reportOption.reportID) &&
	optionsToExclude.some((option) => option.login === reportOption.login || option.reportID === reportOption.reportID)
	) {
	continue;
	}

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

We should check if the report has preexistingReportID then we should not add that report in recent reports. We can do something like this

   if(reportOption?.item?.preexistingReportID){
                continue;
            }

App/src/libs/OptionsListUtils.ts

Lines 1899 to 1905 in e5a1366

	if (
	!includeThreads &&
	(!!reportOption.login || reportOption.reportID) &&
	optionsToExclude.some((option) => option.login === reportOption.login || option.reportID === reportOption.reportID)
	) {
	continue;
	}

We can utilize the same check in other places where this issue persists.

What alternative solutions did you explore? (Optional)

[bernhardoj]

Proposal

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

Search result shows an item with an empty title but with the login/email of the current user.

What is the root cause of that problem?

When we start a chat with a secondary login, BE will return a preExistingReportID. The app will then navigate to the existing report. However, the secondary login report is never removed, so it's shown at the top of the search result. Why does the subtitle show the current user login?

The subtitle/alternate text is built based on the first report participant (if the previous checks of the logic aren't satisfied)

App/src/libs/OptionsListUtils.ts

Line 1878 in 36931e7

reportOption.alternateText = getAlternateText(reportOption, {showChatPreviewLine, forcePolicyNamePreview});

App/src/libs/OptionsListUtils.ts

Line 575 in 36931e7

: LocalePhoneNumber.formatPhoneNumber(option.participantsList && option.participantsList.length > 0 ? option.participantsList[0].login ?? '' : '');

Normally, a DM contains the current user and the user that we chat with as the participants, but the secondary login report only contains the current user. We already filter out current users from DM participants,

App/src/libs/OptionsListUtils.ts

Lines 1508 to 1511 in 36931e7

	const isOneOnOneChat = ReportUtils.isOneOnOneChat(report);
	const accountIDs = Object.keys(report.participants ?? {})
	.map(Number)
	.filter((accountID) => accountID !== currentUserAccountID || !isOneOnOneChat);

but the isOneOnOneChat returns false because it expects the report to have 1 participant besides the current user.

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

We should clear the secondary login report when a pre-existing report is found. We did this before but broken by #41338.

We should clear it in the callback

App/src/libs/actions/Report.ts

Lines 1266 to 1272 in 36931e7

	let callback = () => {};
	// Only re-route them if they are still looking at the optimistically created report
	if (Navigation.getActiveRoute().includes(`/r/${report.reportID}`)) {
	callback = () => {
	Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(report.preexistingReportID ?? ''), CONST.NAVIGATION.TYPE.UP);
	};
	}

We can do it like this:

let callback = () => {
    Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, null);
};

if (Navigation.getActiveRoute().includes(`/r/${report.reportID}`)) {
    const currCallback = callback;
    callback = () => {
        currCallback();
        Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(report.preexistingReportID ?? ''), CONST.NAVIGATION.TYPE.UP);
    };
}

[melvin-bot]

@kevinksullivan Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

@kevinksullivan 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[kevinksullivan]

I think this is a low priority VSB bug.

[melvin-bot]

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

[melvin-bot]

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

[aimane-chnaif]

reviewing

[melvin-bot]

@kevinksullivan, @aimane-chnaif Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

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

[aimane-chnaif]

updating shortly

[melvin-bot]

@kevinksullivan @aimane-chnaif this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[aimane-chnaif]

@bernhardoj thanks for the detailed explanation. Please make sure that your solution doesn't cause regression of #38983. Also, I wonder if we don't need to hide the report which was already optimistically created before fix.

[bernhardoj]

Please make sure that your solution doesn't cause regression of #38983

I have tested it before and it won't cause a regression of #38983

Also, I wonder if we don't need to hide the report which was already optimistically created before fix.

I don't think it's worth to do that. They just need to relogin to remove the report.

[aimane-chnaif]

@bernhardoj's proposal looks good to me.
🎀👀🎀 C+ reviewed

[melvin-bot]

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

[dangrous]

@bernhardoj's proposal makes sense! Assigning.

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

• Clear the optimistic secondary login report when there is a preexisting report #43633

If no regressions arise, payment will be issued on 2024-06-28. 🎊

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

• @bernhardoj requires payment automatic offer (Contributor)
• @aimane-chnaif requires payment through NewDot Manual Requests

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

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

[dangrous]

Looks like we're ready for payment here! @aimane-chnaif can you complete the checklist when you have a moment please? Thanks!

[aimane-chnaif]

• The PR that introduced the bug has been identified. Link to the PR: https://github.com/Expensify/App/pull/413381
• 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/41338/files#r1661083631
• 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

This bug was caught during regression test - https://expensify.testrail.io/index.php?/tests/view/4574481&group_by=cases:section_id&group_order=asc&group_id=229067

[melvin-bot]

@dangrous, @kevinksullivan, @bernhardoj, @aimane-chnaif Whoops! This issue is 2 days overdue. Let's get this updated quick!

[kevinksullivan]

Payment summary:

• @aimane-chnaif $250 (NewDot)
• @bernhardoj $250 (Newdot)

[bernhardoj]

@kevinksullivan hi, I'm still in the process of setting up the NewDot payment. Should we reopen this issue in the meantime?

[bernhardoj]

Requested in ND.

[JmillsExpensify]

$250 approved for @bernhardoj

[aimane-chnaif]

I am still using upwork. Can you please reopen and sort payment? Thanks
cc: @dangrous as @kevinksullivan is no longer available (re-add Bug label please)

[dangrous]

Reopening, will get another BZ in here!

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

[dangrous]

@slafortune please see #42692 (comment) - thank you!