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

[$125] [Search v2.3] - Default search name shows the report/user ID instead of report/user name #49216

Open
6 tasks done
IuliiaHerets opened this issue Sep 14, 2024 · 19 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Sep 14, 2024

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.35-0
Reproducible in staging?: Y
Reproducible in production?: N
Email or phone of affected tester (no customers): applausetester+kh010901@applause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to Search > Chats.
  3. Click Filters.
  4. Click In.
  5. Select a report > Save.
  6. Click Save search.

Expected Result:

Default search name will show the report name.

Actual Result:

Default search name shows the report ID instead of report name.
Ths same goes for user ID when "From" filter is saved.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6603220_1726299165075!Screenshot_2024-09-14_at_15 31 20
Bug6603195_1726295110542.bandicam_2024-09-14_09-18-53-000.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021835502366052877051
  • Upwork Job ID: 1835502366052877051
  • Last Price Increase: 2024-09-16
Issue OwnerCurrent Issue Owner: @jayeshmangwani
@IuliiaHerets IuliiaHerets added DeployBlockerCash This issue or pull request should block deployment Bug Something is broken. Auto assigns a BugZero manager. labels Sep 14, 2024
Copy link

melvin-bot bot commented Sep 14, 2024

Triggered auto assignment to @sakluger (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 Sep 14, 2024
Copy link

melvin-bot bot commented Sep 14, 2024

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

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Sep 14, 2024
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.

@ishpaul777
Copy link
Contributor

ishpaul777 commented Sep 14, 2024

Edited by proposal-police: This proposal was edited at 2023-10-20T13:45:00Z.

Proposal

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

Saved search - Default search name shows the report/user ID instead of report/user name

What is the root cause of that problem?

From: #48566
we are directly showing the query name which has report/user ID instead of report/user name here:

title: item.name,

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

  1. first we need to check if the query name === query, that means query is not renamed by user and still has the default name
  2. if the query name === query, then we need to use same logic in SearchUtils.getSearchHeaderTitle as we are using for search header title, here queryJSON will be build with SearchUtils.buildSearchQueryJSON for the item
            let title = item.name;

            if (title === item.query) {
                const jsonQuery = SearchUtils.buildSearchQueryJSON(item.query);
                title = jsonQuery ? SearchUtils.getSearchHeaderTitle(jsonQuery, personalDetails, cardList, reports, taxRates) : item.query;
            }

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

@jaydamani
Copy link
Contributor

Proposal

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

default search name shows ID instead of name

What is the root cause of that problem?

when saving a search query, we use the raw query as name

const saveSearchName = name ?? queryJSON?.inputQuery ?? '';

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

Use getSearchHeaderTitle to get a more user friendly string. so the update code will be

    const saveSearchName = name ?? SearchUtils.getSearchHeaderTitle(queryJSON, personalDetails, cardList, reports, taxRates) ?? '';

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

@luacmartins luacmartins assigned lakchote and luacmartins and unassigned thienlnam Sep 15, 2024
@luacmartins
Copy link
Contributor

@lakchote will address this in a follow up since it came from his PR

@luacmartins luacmartins added the External Added to denote the issue can be worked on by a contributor label Sep 16, 2024
@melvin-bot melvin-bot bot changed the title Saved search - Default search name shows the report/user ID instead of report/user name [$250] Saved search - Default search name shows the report/user ID instead of report/user name Sep 16, 2024
Copy link

melvin-bot bot commented Sep 16, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 16, 2024
@luacmartins luacmartins changed the title [$250] Saved search - Default search name shows the report/user ID instead of report/user name [$125] Saved search - Default search name shows the report/user ID instead of report/user name Sep 16, 2024
Copy link

melvin-bot bot commented Sep 16, 2024

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

Copy link

melvin-bot bot commented Sep 16, 2024

Upwork job price has been updated to $125

@luacmartins
Copy link
Contributor

@jaydamani are you available to work on a fix for this?

@jaydamani
Copy link
Contributor

@luacmartins yes, I can work on it now.

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Hourly KSv2 labels Sep 16, 2024
@luacmartins
Copy link
Contributor

I already put up a PR with a slightly different solution since we shouldn't update the actual saved search name (we wanna be able to dynamically get those if the display name changes, etc)

@jaydamani
Copy link
Contributor

@luacmartins That PR could break the rename functionality. If you rename a search, it would still show the query instead of the name.

My changes only affects when name provided by user is undefined
const saveSearchName = name ?? SearchUtils.getSearchHeaderTitle(queryJSON, personalDetails, cardList, reports, taxRates) ?? '';

Also, I think your PR partially implements proposal by @ishpaul777
Fully implementing it should fix the rename issue.

@luacmartins
Copy link
Contributor

Ah yes, I noticed that as well. I updated the PR and I agree that in the end the solution was quite similar to @ishpaul777's solution. I'll make sure that we compensate them for the solution.

@luacmartins
Copy link
Contributor

Fixed on staging

@luacmartins luacmartins removed the DeployBlockerCash This issue or pull request should block deployment label Sep 16, 2024
@jayeshmangwani jayeshmangwani removed their assignment Sep 16, 2024
@jayeshmangwani
Copy link
Contributor

I haven't reviewed the PR; Unassigning myself from the issue

@luacmartins luacmartins changed the title [$125] Saved search - Default search name shows the report/user ID instead of report/user name [$125] [Search v2.3] - Default search name shows the report/user ID instead of report/user name Sep 19, 2024
@luacmartins
Copy link
Contributor

This is done

@ishpaul777
Copy link
Contributor

we need to sort payment #49216 (comment) 😅 can we reopen @luacmartins

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 26, 2024
@luacmartins luacmartins reopened this Sep 26, 2024
@luacmartins
Copy link
Contributor

@sakluger could you please help pay $50 to @ishpaul777 since we used their solution? Thank you!

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 External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
Status: Done
Development

No branches or pull requests

8 participants