[$250] [Search v2.1] - (you) disappears after selecting own user name

[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.22-5
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to staging.new.expensify.com/search/filters
2. Click From or To
3. Select your user name

Expected Result:

(you) will not disappear after selecting own user name

Actual Result:

(you) disappears after selecting own user name

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

Bug6576827_1724140883869.bandicam_2024-08-20_15-59-12-380.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01bc33a0aabc7f4ab9
• Upwork Job ID: 1826026505817504872
• Last Price Increase: 2024-08-27
• Automatic offers:
• FitseTLT | Contributor | 103781419

Issue Owner

Current Issue Owner: @FitseTLT

[melvin-bot]

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

@twisterdotcom 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 #wave-collect - Release 1

[etCoderDysto]

Edited by proposal-police: This proposal was edited at 2024-08-20 13:39:32 UTC.

Proposal

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

(you) disappears after selecting own user name

What is the root cause of that problem?

When the user selects their account, In getParticipantsOption, we assign assign displayName which is the users name without (you) to text

App/src/libs/OptionsListUtils.ts

Lines 447 to 457 in d519981

	function getParticipantsOption(participant: ReportUtils.OptionData | Participant, personalDetails: OnyxEntry<PersonalDetailsList>): Participant {
	const detail = getPersonalDetailsForAccountIDs([participant.accountID ?? -1], personalDetails)[participant.accountID ?? -1];
	// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
	const login = detail?.login || participant.login || '';
	const displayName = PersonalDetailsUtils.getDisplayNameOrDefault(detail, LocalePhoneNumber.formatPhoneNumber(login));
	return {
	keyForList: String(detail?.accountID),
	login,
	accountID: detail?.accountID ?? -1,
	text: displayName,

And the text prop will be used to render the users display name in InviteMemberListItem component

getParticipantsOption is called here by formatSectionsFromSearchTerm

App/src/libs/OptionsListUtils.ts

Line 2378 in d519981

return isPolicyExpenseChat ? getPolicyExpenseReportOption(participant) : getParticipantsOption(participant, personalDetails);

And formatSectionsFromSearchTerm is called here to fetch section

App/src/components/Search/SearchFiltersParticipantsSelector.tsx

Lines 98 to 99 in d519981

	const formattedResults = OptionsListUtils.formatSectionsFromSearchTerm(
	cleanSearchTerm,

Then the section will be used by SectionList

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

Since participant.text has the display name that includes (you) we should assign participant.text to text prop and fallback to displayName if participant.text is undefiend

text: participant?.text ?? displayName,

What alternative solutions did you explore? (Optional)

[FitseTLT]

Edited by proposal-police: This proposal was edited at 2024-08-20 14:33:24 UTC.

Proposal

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

Search filters - (you) disappears after selecting own user name

What is the root cause of that problem?

We are properly setting (you) to current user here

App/src/components/Search/SearchFiltersParticipantsSelector.tsx

Lines 111 to 113 in a32da40

	if (chatOptions.currentUserOption && !isCurrentUserSelected) {
	const formattedName = ReportUtils.getDisplayNameForParticipant(chatOptions.currentUserOption.accountID, false, true, true, personalDetails);
	newSections.push({

but only doing it when current user is not selected

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

We should do the same update of text we did here for selected current user too
So change this to

   const currentUserSelected = formattedResults.section.data.find((option) => option.accountID === chatOptions.currentUserOption?.accountID);
        newSections.push(formattedResults.section);

        if (chatOptions.currentUserOption) {
            const formattedName = ReportUtils.getDisplayNameForParticipant(chatOptions.currentUserOption.accountID, false, true, true, personalDetails);
            if (!currentUserSelected) {
                newSections.push({
                    title: '',
                    data: [{...chatOptions.currentUserOption, text: formattedName}],
                    shouldShow: true,
                });
            } else {
                currentUserSelected.text = formattedName;
            }
        }

What alternative solutions did you explore? (Optional)

[bernhardoj]

Proposal

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

The (you) text disappears from the current user option after selecting it.

What is the root cause of that problem?

When the current user is not selected yet, we format the name using getDisplayNameForParticipant and pass shouldAddCurrentUserPostfix as true which adds the (you) suffix.

App/src/components/Search/SearchFiltersParticipantsSelector.tsx

Lines 111 to 115 in c6d81c8

	if (chatOptions.currentUserOption && !isCurrentUserSelected) {
	const formattedName = ReportUtils.getDisplayNameForParticipant(chatOptions.currentUserOption.accountID, false, true, true, personalDetails);
	newSections.push({
	title: '',
	data: [{...chatOptions.currentUserOption, text: formattedName}],

When the user is selected, the selected options is formatted with OptionsListUtils.formatSectionsFromSearchTerm > getParticipantsOption.

App/src/components/Search/SearchFiltersParticipantsSelector.tsx

Lines 98 to 100 in c6d81c8

	const formattedResults = OptionsListUtils.formatSectionsFromSearchTerm(
	cleanSearchTerm,
	selectedOptions,

App/src/libs/OptionsListUtils.ts

Line 2378 in c6d81c8

return isPolicyExpenseChat ? getPolicyExpenseReportOption(participant) : getParticipantsOption(participant, personalDetails);

In getParticipantsOption, the text is from PersonalDetailsUtils.getDisplayNameOrDefault whcih doesn't contains the (you) suffix because we don't pass shouldAddCurrentUserPostfix to it.

App/src/libs/OptionsListUtils.ts

Line 451 in c6d81c8

const displayName = PersonalDetailsUtils.getDisplayNameOrDefault(detail, LocalePhoneNumber.formatPhoneNumber(login));

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

Pass as true for shouldAddCurrentUserPostfix to getDisplayNameOrDefault.

const displayName = PersonalDetailsUtils.getDisplayNameOrDefault(detail, LocalePhoneNumber.formatPhoneNumber(login), undefined, shouldAddCurrentUserPostfix);

This will require us to pass down the value as true from SearchFiltersParticipantsSelector > formatSectionsFromSearchTerm > getParticipantsOption.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

@twisterdotcom, @abdulrahuman5196 Eep! 4 days overdue now. Issues have feelings too...

[abdulrahuman5196]

Hi, came back from weekend. Will check in the morning.

[melvin-bot]

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

[abdulrahuman5196]

@FitseTLT 's proposal here #47712 (comment) looks good and works well. It aims to fix the issue in the SearchFiltersParticipantsSelector similar to existing approach of showing (you).

🎀 👀 🎀
C+ Reviewed

[melvin-bot]

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

[bernhardoj]

@abdulrahuman5196 any opinion with my proposal?

I think the selected proposal doesn't follow a similar approach. The approach is mutating the item returned from the array below.

App/src/components/Search/SearchFiltersParticipantsSelector.tsx

Line 107 in a32da40

const isCurrentUserSelected = selectedOptions.find((option) => option.accountID === chatOptions.currentUserOption?.accountID);

The current approach is, selected options are formatted by OptionsListUtils.formatSectionsFromSearchTerm and then pushed to the list. If the current user isn't selected, we push the current user with (you) suffix to the list. If the user is selected, then it will be handled by OptionsListUtils.formatSectionsFromSearchTerm, just like the other selected items.

Also, the selected proposal will cause the current user to always be at the top even though there are selected items.

[FitseTLT]

Also, the selected proposal will cause the current user to always be at the top even though there are selected items.

@abdulrahuman5196 It's because I moved down the newSections.push(formattedResults.section); below the if block I have updated and moved it back. It works fine now.
We alrdeay have a code that concatenates (you)

App/src/components/Search/SearchFiltersParticipantsSelector.tsx

Line 115 in 8c69c58

data: [{...chatOptions.currentUserOption, text: formattedName}],

the only problem we forgot to do the same for selected list so my approach is indeed using the same as existing approach.

2024-08-29.20-47-03.mp4

[abdulrahuman5196]

@bernhardoj

I think the selected proposal doesn't follow a similar approach. The approach is mutating the item returned from the array below.

I noticed that during approval, but its more like a PR code fix.
for the issue you mentioned, we can fix that during PR since we don't consider the proposal code to be the expect PR code.

any opinion with my proposal?
Pass as true for shouldAddCurrentUserPostfix to getDisplayNameOrDefault

I was not aligned in passing shouldAddCurrentUserPostfix as true in the common helper method usecase. Since the default value of shouldAddCurrentUserPostfix in getDisplayNameOrDefault is false and I have also noticed we are not passing it as true in multiple common places.

[bernhardoj]

I was not aligned in passing shouldAddCurrentUserPostfix as true in the common helper method usecase. Since the default value of shouldAddCurrentUserPostfix in getDisplayNameOrDefault is false and I have also noticed we are not passing it as true in multiple common places.

I don't see why this is a problem since we only pass it as true in the filter page just like we did with the non-selected current user section.

I think the selected proposal doesn't follow a similar approach. The approach is mutating the item returned from the array below.

I noticed that during approval, but its more like a PR code fix.

I believe the root issue is that we are not formatting it inside OptionsListUtils.formatSectionsFromSearchTerm (assuming you agree with this), but the selected proposal explanation is revolving around mutating the array and lacks the explanation that we already did some formatting inside OptionsListUtils.formatSectionsFromSearchTerm which is why I posted my proposal because I think the approach is very different.

^ just an explaination why I posted my proposal 😄, i'll accept any decision

[melvin-bot]

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

[twisterdotcom]

We have an assignee @MelvinBot

[melvin-bot]

@twisterdotcom @Gonals @FitseTLT @abdulrahuman5196 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!

[FitseTLT]

PR will be up tomorrow

[FitseTLT]

bump for a review @abdulrahuman5196

[FitseTLT]

A PR was raised and under review but another PR fixed it so I had to close it.

[luacmartins]

It seems like we can close this issue then?

[FitseTLT]

@twisterdotcom PR was raised and was on review; I think Payment is due here.

[abdulrahuman5196]

@twisterdotcom PR was raised and was on review; I think Payment is due here.

Yeah. Same is mentioned in C+ doc as well

[twisterdotcom]

Aha, I see.

If a contributor has been hired for a job and we decide to close the job before it is successfully completed, full payment is due for the hired contributor. If the C+ only reviewed proposals and not a PR, they are eligible for 50%. If they started reviewing a PR it's 100%. One caveat here might be if the hired contributor wasn't working on their PR with urgency.

Okay, cool, @abdulrahuman5196 was actively reviewing, so let's do both $250.

Payment Summary:

• @FitseTLT paid $250 via Upwork here (Contributor)
• @abdulrahuman5196 paid $250 via Newdot requests (C+)

[JmillsExpensify]

$250 approved for @abdulrahuman5196