Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search - Own email address is displayed twice on /search/filters page #47537

Closed
3 of 6 tasks
IuliiaHerets opened this issue Aug 16, 2024 · 14 comments
Closed
3 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering Reviewing Has a PR in review Weekly KSv2

Comments

@IuliiaHerets
Copy link

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: 9.0.21-0
Reproducible in staging?: Y
Reproducible in production?: N (Can't check - new feature)
Issue was found when executing this PR: #46962
Email or phone of affected tester (no customers): gocemate+a980@gmail.com
Issue reported by: Applause Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to /search/filters
  3. Click on From filter and check email list
  4. Click on To filter and check email list

Expected Result:

Self email address should be displayed once

Actual Result:

Self email address is displayed twice, both From and To filters

Workaround:

Unknown

Platforms:

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6573342_1723757431190.Recording__3735.mp4

image (233)

View all open jobs on GitHub

@IuliiaHerets IuliiaHerets added DeployBlockerCash This issue or pull request should block deployment Bug Something is broken. Auto assigns a BugZero manager. labels Aug 16, 2024
Copy link

melvin-bot bot commented Aug 16, 2024

Triggered auto assignment to @stephanieelliott (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 melvin-bot bot added the Daily KSv2 label Aug 16, 2024
Copy link

melvin-bot bot commented Aug 16, 2024

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

Copy link
Contributor

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

@IuliiaHerets
Copy link
Author

We think that this bug might be related to #vip-vsb

@bernhardoj
Copy link
Contributor

bernhardoj commented Aug 16, 2024

Edited by proposal-police: This proposal was edited at 2024-08-16 04:30:10 UTC.

Proposal

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

There are 2 current user options in the search From and To filters.

What is the root cause of that problem?

In the search filter From/To, we put the current user option at the top of the list and then the recent reports.

if (chatOptions.currentUserOption && !isCurrentUserSelected) {
newSections.push({
title: '',
data: [chatOptions.currentUserOption],
shouldShow: true,
});
}
newSections.push({
title: '',
data: chatOptions.recentReports,
shouldShow: chatOptions.recentReports.length > 0,
});

However, the recent reports also contains a self DM, so the list contains 2 current user option. That's because we include self DM when getting the options (the last param).

return OptionsListUtils.getFilteredOptions(
options.reports,
options.personalDetails,
undefined,
'',
selectedOptions,
CONST.EXPENSIFY_EMAILS,
false,
true,
false,
{},
[],
false,
{},
[],
true,
false,
false,
0,
undefined,
true,
);

This is a new search filter feature, so not a regression.

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

We shouldn't include the self DM into the recent report because we already put the current user at the top of the list. We can set it to false or just remove the param.

@techievivek
Copy link
Contributor

@bernhardoj can you please link the GH that caused this regression?

@techievivek
Copy link
Contributor

This seems to be coming from here #46962

@techievivek
Copy link
Contributor

I will just inform on the PR and it can be fixed as a followup, and nothing significant to be a blocker.

@techievivek techievivek added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Aug 16, 2024
@luacmartins
Copy link
Contributor

I agree this is not a blocker. @Kicu @rayane-djouah since we worked on this PR together.

@Kicu
Copy link
Contributor

Kicu commented Aug 19, 2024

hey I did the filters and I will take a look today at this issue and try to come up with a fix. Please assign me

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Aug 19, 2024
@Kicu
Copy link
Contributor

Kicu commented Aug 19, 2024

PR with a simple fix is ready.

@bernhardoj your suggestion was actually correct and I ended up doing exactly the same thing you suggested - so switching the argument-flag for including selfDM. Feels the cleanest

@rayane-djouah
Copy link
Contributor

PR on staging

@rayane-djouah
Copy link
Contributor

We can close this one. No payment is due.

@luacmartins
Copy link
Contributor

Nice! Thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

7 participants