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 v2.3] - Educational tooltip appears repeatedly after relogin #49204

Closed
2 of 6 tasks
IuliiaHerets opened this issue Sep 14, 2024 · 22 comments
Closed
2 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

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/A - new feature, doesn't exist in prod
Issue was found when executing this PR: #48566
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.
  3. Click Filters.
  4. Add a few filters.
  5. Click Save search.
  6. Repeat Step 3 to 5 a few times to create more saved searches.
  7. Go to Troubleshoot > Clear cache and restart > Reset and refresh (or log out and log in again).
  8. Go to Search.

Expected Result:

Educational tooltip will not appear again as it has appeared previously.

Actual Result:

Educational tooltip appears repeatedly for each saved search after relogin.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6603089_1726279707918.20240914_100413.mp4

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @lakchote
@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
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.

Copy link

melvin-bot bot commented Sep 14, 2024

Triggered auto assignment to @puneetlath (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 @puneetlath (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

@melvin-bot melvin-bot bot removed the Hourly KSv2 label Sep 14, 2024
@IuliiaHerets
Copy link
Author

We think that this bug might be related to #wave-control

@gijoe0295
Copy link
Contributor

gijoe0295 commented Sep 14, 2024

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

Proposal

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

  1. Tooltips show for every saved search items
  2. Tooltips reappear after log in again

What is the root cause of that problem?

  1. We show tooltip for every items. As long as shouldRenderTooltip is true, it's true for every items:

shouldRenderTooltip: !shouldHideSavedSearchRenameTooltip,

  1. NVP_SHOULD_HIDE_SAVED_SEARCH_RENAME_TOOLTIP is a local value and gets clear out once signing out.

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

  1. Add index param to createSavedSearchMenuItem and only show tooltip for the first item:

return Object.entries(savedSearches).map(([key, item]) => createSavedSearchMenuItem(item as SaveSearchItem, key, shouldUseNarrowLayout));

const createSavedSearchMenuItem = useCallback(

shouldRenderTooltip: index === 0 && !shouldHideSavedSearchRenameTooltip
  1. Add an API endpoint to modify NVP_SHOULD_HIDE_SAVED_SEARCH_RENAME_TOOLTIP just like we did here:

function dismissTrackTrainingModal() {

@luacmartins
Copy link
Contributor

This is known and NAB for now. We're addressing it as a follow up.

@lakchote
Copy link
Contributor

lakchote commented Sep 17, 2024

Also to be solved:

The tooltip should not show over a dialog.

image
image

Steps:

  1. Clicked on an expense on the search result
  2. Clicked on the receipt thumbnail
  3. The tooltip is still showing above the dialog

Slack discussion

@luacmartins luacmartins changed the title Saved search - Educational tooltip appears repeatedly after relogin [Search v2.3] - Educational tooltip appears repeatedly after relogin Sep 19, 2024
@luacmartins luacmartins moved this from Done to Polish in [#whatsnext] #expense Sep 19, 2024
@melvin-bot melvin-bot bot added the Overdue label Sep 19, 2024
@luacmartins
Copy link
Contributor

Draft PRs up

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 Overdue labels Sep 20, 2024
@lakchote
Copy link
Contributor

Even though it was not external and we weren't looking for proposals, @gijoe0295's solution helped partially resolve the issue (we needed a backend PR too).

As such I'd say we can compensate him $50 for his solution.

cc @puneetlath since you were originally assigned as a BZ member

@puneetlath
Copy link
Contributor

Ok sounds good. @gijoe0295 offer is here: https://www.upwork.com/nx/wm/offer/104071644. Please ping me on this issue when you've accepted.

@luacmartins luacmartins moved this from Polish to Done in [#whatsnext] #expense Sep 26, 2024
@paultsimura
Copy link
Contributor

Deployed to production today. Payment is due on 2024-10-04.

@gijoe0295
Copy link
Contributor

@puneetlath Accepted the offer.

@puneetlath
Copy link
Contributor

Paid. Thanks everyone!

@paultsimura
Copy link
Contributor

@puneetlath I believe, I am eligible to a C+ payment for the PR review here 🤔

@puneetlath
Copy link
Contributor

Ah apologies, I thought only @gijoe0295 needed compensation from this comment. Can you link the PR @paultsimura?

@puneetlath puneetlath reopened this Oct 6, 2024
@paultsimura
Copy link
Contributor

Sure @puneetlath, here it is: #49473

@puneetlath
Copy link
Contributor

Ok cool, I've sent you an offer here: https://www.upwork.com/nx/wm/offer/104292932

Can you complete the checklist as well?

@paultsimura
Copy link
Contributor

Thanks, I've accepted the offer. Will fill the checklist by tomorrow👍

@paultsimura
Copy link
Contributor

  • The PR that introduced the bug has been identified. Link to the PR: [Search v2.3] [App] Implement Saved Search feature #48566
  • 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: https://github.com/Expensify/App/pull/48566/files#r1790066475
  • 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
  • Determine if we should create a regression test for this bug: Yes
  • 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. Go to Search
  2. Click Filters
  3. Add a few filters
  4. Click "Save search"
  5. Verify the educational tooltip "You can rename..." is shown
  6. Save another search
  7. Verify the educational tooltip is not shown again
  8. Logout and Login
  9. Navigate to the Search page
  10. Verify the saved searches are present and the educational tooltip is not shown

Do we agree 👍 or 👎

@paultsimura
Copy link
Contributor

@puneetlath the checklist is ready: #49204 (comment)

@VictoriaExpensify
Copy link
Contributor

Payment Summary:
Contributor+: @paultsimura paid $250 via Upwork

@puneetlath
Copy link
Contributor

Regression test issue is here: https://github.com/Expensify/Expensify/issues/434820

Thanks for handling the payment @VictoriaExpensify. Closing!

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
Status: Done
Development

No branches or pull requests

7 participants