Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.34k stars 2.77k forks source link

[$125] Saved search - Default search name shows the report/user ID instead of report/user name #49216

Open IuliiaHerets opened 3 days ago

IuliiaHerets commented 3 days ago

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:

Screenshots/Videos

Bug6603220_1726299165075!Screenshot_2024-09-14_at_15 31 20

https://github.com/user-attachments/assets/d150f998-9d1b-4c9b-a994-14d355155147

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
melvin-bot[bot] commented 3 days ago

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[bot] commented 3 days ago

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

github-actions[bot] commented 3 days ago

:wave: 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 commented 3 days ago

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: https://github.com/Expensify/App/pull/48566 we are directly showing the query name which has report/user ID instead of report/user name here:

https://github.com/Expensify/App/blob/2dc08e931bb58a72ab4d09e74165fe9be0c002a7/src/pages/Search/SearchTypeMenu.tsx#L103

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 commented 2 days ago

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

https://github.com/Expensify/App/blob/0c618acc8b5a0b77eb27dfa049a18e3135c51c52/src/libs/actions/Search.ts#L55

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 commented 2 days ago

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

melvin-bot[bot] commented 2 days ago

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

melvin-bot[bot] commented 2 days ago

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

melvin-bot[bot] commented 2 days ago

Upwork job price has been updated to $125

luacmartins commented 2 days ago

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

jaydamani commented 2 days ago

@luacmartins yes, I can work on it now.

luacmartins commented 2 days ago

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 commented 2 days ago

@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 commented 2 days ago

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 commented 2 days ago

Fixed on staging

jayeshmangwani commented 2 days ago

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