[$250] [HOLD for payment 2024-06-13] Handle showing selfDM as the first item in the list when users search for their own email or display name

[techievivek]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Problem:

We recently merged a backend PR that returns selfDM reports in search results when users search for their email or display name. However, the backend response does not guarantee that the selfDM report will appear as the first result. This can make it difficult for users to quickly locate their selfDM in the search results even when they are returned by the backend.

Solution:

Update the frontend logic to ensure that the selfDM report appears first in the search results in UI when users specifically search for their own email or display name. Implement post-processing logic to reorder the search results, ensuring the selfDM report is at the top of the list. This will make it easier for users to find their selfDM when searching for their own email or display name.

Issue Owner

Current Issue Owner: @

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01401c18bd7e84e155
• Upwork Job ID: 1801202714715048228
• Last Price Increase: 2024-06-13

[melvin-bot]

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

[melvin-bot]

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

[dragnoir]

Proposal

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

Handle showing selfDM as the first item in the list when users search for their own email or display name

What is the root cause of that problem?

New feature

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

On ChatFinderPage we have the filteredOptions that turns the new list when we search.

App/src/pages/ChatFinderPage/index.tsx

Line 102 in 525ad6f

const newOptions = OptionsListUtils.filterOptions(searchOptions, debouncedSearchValue, betas);

To implement the feature where items with isSelfDM: true in newOptions.recentReports are moved to the top of the list, you can use JavaScript's sort method within your useMemo hook. Here's how we can modify the function to achieve this:

const filteredOptions = useMemo(() => {
    if (debouncedSearchValue.trim() === '') {
        return {
            recentReports: [],
            personalDetails: [],
            userToInvite: null,
            headerMessage: '',
        };
    }

    const newOptions = OptionsListUtils.filterOptions(searchOptions, debouncedSearchValue, betas);
    console.log('newOptions: ', newOptions);

    // Sort recentReports to move items with isSelfDM: true to the top
    const sortedRecentReports = newOptions.recentReports.sort((a, b) => {
        if (a.isSelfDM && !b.isSelfDM) return -1;
        if (!a.isSelfDM && b.isSelfDM) return 1;
        return 0;
    });

    const header = OptionsListUtils.getHeaderMessage(sortedRecentReports.length + Number(!!newOptions.userToInvite) > 0, false, debouncedSearchValue);
    return {
        recentReports: sortedRecentReports,
        personalDetails: newOptions.personalDetails,
        userToInvite: newOptions.userToInvite,
        headerMessage: header,
    };
}, [debouncedSearchValue, searchOptions, betas]);

Explanation:

1. Check if debouncedSearchValue is empty: If it is, return the initial empty state.
2. Filter Options: Use OptionsListUtils.filterOptions to get the newOptions.
3. Sort recentReports: Use the sort method to reorder newOptions.recentReports, moving items with isSelfDM: true to the top.
4. Return Sorted Options: Return the sorted recentReports along with other newOptions properties.

This ensures that any item with the attribute isSelfDM: true appears at the top of the recentReports array in the returned object.

POC video:

my name: Rachid L and my email is [email protected]

20240527_095213.mp4

We can also integrate this feature inside OptionsListUtils.filterOptions

[nkdengineer]

Proposal

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

What is the root cause of that problem?

In here, we're ordering the report by lastVisibleActionCreated and archived report will be remain at the bottom

App/src/libs/OptionsListUtils.ts

Lines 1782 to 1788 in 525ad6f

	const orderedReportOptions = lodashSortBy(filteredReportOptions, (option) => {
	const report = option.item;
	if (option.isArchivedRoom) {
	return CONST.DATE.UNIX_EPOCH;
	}
	return report?.lastVisibleActionCreated;

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

We should order the selfDM at the top of the report list by return new Date().toString() if the report is selfDM. So this report will be always at the top of the list

const orderedReportOptions = lodashSortBy(filteredReportOptions, (option) => {
    const report = option.item;
    if (option.isArchivedRoom) {
        return CONST.DATE.UNIX_EPOCH;
    }
    if (option.isSelfDM) {
        return new Date().toString();
    }

    return report?.lastVisibleActionCreated;
});

App/src/libs/OptionsListUtils.ts

Lines 1782 to 1788 in 525ad6f

	const orderedReportOptions = lodashSortBy(filteredReportOptions, (option) => {
	const report = option.item;
	if (option.isArchivedRoom) {
	return CONST.DATE.UNIX_EPOCH;
	}
	return report?.lastVisibleActionCreated;

What alternative solutions did you explore? (Optional)

NA

[bernhardoj]

Proposal

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

We want to show self DM as the first result when the user searches for their own login/display name.

What is the root cause of that problem?

A feature request.

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

We currently able to search from 2 pages, ChatFinderPage and NewChatPage. They have different logic to sort.

For ChatFinderPage, we use this logic to sort the results.

App/src/libs/OptionsListUtils.ts

Lines 1561 to 1581 in 525ad6f

	function orderOptions(options: ReportUtils.OptionData[], searchValue: string | undefined) {
	return lodashOrderBy(
	options,
	[
	(option) => {
	if (!!option.isChatRoom || option.isArchivedRoom) {
	return 3;
	}
	if (!option.login) {
	return 2;
	}
	if (option.login.toLowerCase() !== searchValue?.toLowerCase()) {
	return 1;
	}
	// When option.login is an exact match with the search value, returning 0 puts it at the top of the option list
	return 0;
	},
	],
	['asc'],
	);

It's executed only when we type something in the search field. To put the self-DM at the top of the list, we can return 0 if the option is self DM.

if (option.isSelfDM) {
    return 0;
}

For NewChatPage, the result is sorted here.

App/src/libs/OptionsListUtils.ts

Lines 1782 to 1789 in 525ad6f

	const orderedReportOptions = lodashSortBy(filteredReportOptions, (option) => {
	const report = option.item;
	if (option.isArchivedRoom) {
	return CONST.DATE.UNIX_EPOCH;
	}
	return report?.lastVisibleActionCreated;
	});

To put the self-DM at the top of the list and only when we search for our self, we can add this code:

if (searchValue) {
    return [option.isSelfDM, report?.lastVisibleActionCreated];
}

This will sort it first by isSelfDM.

[getusha]

@bernhardoj's proposal looks good to me. the solution is straight forward and works perfectly.
🎀 👀 🎀 C+ Reviewed

[melvin-bot]

Current assignee @techievivek is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[bernhardoj]

PR is ready

cc: @getusha

[Julesssss]

Hey all, it sounds like the PR didn't fully fix the issue. @kbecciv could you please point to the specific case that is failing?

[bernhardoj]

Looking at the video, looks like the search request doesn't return the self DM. I can repro it in one of my accounts where the self DM is not loaded yet and when I try to search, SearchForReports always returns with an error.

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

• Put self DM at the top of search result #42704

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

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

• @bernhardoj requires payment (Needs manual offer from BZ)
• @getusha requires payment (Needs manual offer from BZ)

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

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

[jliexpensify]

Bump @getusha for the checklist

[jliexpensify]

Can I confirm there's no regressions or issues that affect payment @techievivek and @Julesssss (based off this comment)?

Payment Summary:

• C: @bernhardoj $250
• C+: @getusha $250

https://www.upwork.com/jobs/~01401c18bd7e84e155

[melvin-bot]

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

[melvin-bot]

Current assignee @getusha is eligible for the External assigner, not assigning anyone new.

[jliexpensify]

Re-applied label as no Upworks job was initially created

[jliexpensify]

@getusha I don't know who you are - what's your Upworks profile? Please add your full name (as per Upworks) in your GH profile, thanks!

[getusha]

@jliexpensify could we hold on payments for a few days please? i am working on moving payments to ND. thanks

[jliexpensify]

No worries, I'll just spin up a new Summary.

Can I also confirm there's no regressions or issues that affect payment @techievivek and @Julesssss (based off this comment)?

UPDATED Payment Summary:

• C: @bernhardoj $250
• C+: @getusha $250 (ND payments)

https://www.upwork.com/jobs/~01401c18bd7e84e155

[jliexpensify]

Hi, sorry another bump for @techievivek and @Julesssss"

Can I also confirm there's no regressions or issues that affect payment @techievivek and @Julesssss (based off this #42627 (comment))?

[Julesssss]

Yeah I don't believe there are any regressions. I think it's fine to pay out

[jliexpensify]

Paid @bernhardoj, thanks for your patience! @getusha please refer to this summary for ND payments.

[jliexpensify]

@getusha also, does a checklist need to be completed here?

[getusha]

[@getusha] The PR that introduced the bug has been identified. Link to the PR: N/a it is a feature request
[@getusha] 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: N/a
[@getusha] 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
[@getusha] Determine if we should create a regression test for this bug. Yes
[@getusha] 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

1. Open search page
2. Search for the account you currently signed in with (email or name)
3. Verify that self DM appears at the top of the result

Do we agree 👍 or 👎

[JmillsExpensify]

$250 approved for @getusha