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 - Deleted search doesn´t disappear from secondary device despite refreshing page #53939

Open
4 of 8 tasks
vincdargento opened this issue Dec 11, 2024 · 19 comments
Open
4 of 8 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Internal Requires API changes or must be handled by Expensify staff Weekly KSv2

Comments

@vincdargento
Copy link

vincdargento commented Dec 11, 2024

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.74-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5328850&group_by=cases:section_id&group_order=asc&group_id=296775
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

Prerequisite: Account has to be opened in two devices or environments at the same time.

  1. Open the Expensify app.
  2. Tap on "Search" on the bottom of the screen.
  3. Tap on the filters icon.
  4. Apply any filter.
  5. Tap on "Save Search"
  6. Switch to second device and verify that the search was created too.
  7. On main device, tap on dropdown menu.
  8. Tap on the three dots icon on the saved search and delete it.
  9. Switch to second device again and refresh the page.

Expected Result:

Search deleted from main device, should disappear from every device in which the account is logged.

Actual Result:

Deleted saved search, doesn´t disappear from secondary device despite refreshing the page, and the user can interact with it normally.

Workaround:

Unknown

Platforms:

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

bug.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021866860154375173814
  • Upwork Job ID: 1866860154375173814
  • Last Price Increase: 2024-12-18
Issue OwnerCurrent Issue Owner: @rojiphil
@vincdargento vincdargento added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 11, 2024
Copy link

melvin-bot bot commented Dec 11, 2024

Triggered auto assignment to @bfitzexpensify (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.

@bfitzexpensify bfitzexpensify added the External Added to denote the issue can be worked on by a contributor label Dec 11, 2024
Copy link

melvin-bot bot commented Dec 11, 2024

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

@melvin-bot melvin-bot bot changed the title Search - Deleted search doesn´t disappear from secondary device despite refreshing page [$250] Search - Deleted search doesn´t disappear from secondary device despite refreshing page Dec 11, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 11, 2024
Copy link

melvin-bot bot commented Dec 11, 2024

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

@bfitzexpensify bfitzexpensify moved this to Bugs and Follow Up Issues in [#whatsnext] #expense Dec 11, 2024
@FitseTLT
Copy link
Contributor

Already being handled in #53687 Let's close it

@github-project-automation github-project-automation bot moved this from Bugs and Follow Up Issues to Done in [#whatsnext] #expense Dec 11, 2024
@rojiphil
Copy link
Contributor

Already being handled in #53687 Let's close it

@FitseTLT Can you please explain how this issue is been handled in the mentioned issue?

@rojiphil
Copy link
Contributor

@bfitzexpensify I think this issue is different. Can you please reopen this issue until this comment is resolved?

@FitseTLT
Copy link
Contributor

The RCA is the same @rojiphil we re-request search action when the transaction length increase but in both cases the expense is deleted which means transaction length decreases hence we don't request search here

const newTransactionAdded = transactionsLength && typeof previousTransactionsLength === 'number' && transactionsLength > previousTransactionsLength;
const newReportActionAdded = reportActionsLength && typeof prevReportActionsLength === 'number' && reportActionsLength > prevReportActionsLength;
// Check if a new transaction or report action was added
if ((!isChat && !!newTransactionAdded) || (isChat && !!newReportActionAdded)) {
// Set the flag indicating the search is triggered by the hook
triggeredByHookRef.current = true;
// Trigger the search
SearchActions.search({queryJSON, offset});

@bfitzexpensify please close it they are the same issues

@rojiphil
Copy link
Contributor

The RCA is the same @rojiphil we re-request search action when the transaction length increase but in both cases the expense is deleted which means transaction length decreases hence we don't request search here

@FitseTLT Wondering how the RCA can be the same when this issue is not about deleting an expense. Rather, it is about deleting the saved search. Are they not different?

@FitseTLT
Copy link
Contributor

Wow very sorry @rojiphil 🙏 It's totally different issue. Sorry for the confusion here.

@MrMuzyk
Copy link
Contributor

MrMuzyk commented Dec 16, 2024

I am Michał from Callstack - expert contributor group. I’d like to work on this job.

@melvin-bot melvin-bot bot added the Overdue label Dec 16, 2024
@MrMuzyk
Copy link
Contributor

MrMuzyk commented Dec 16, 2024

Proposal

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

When deleting last saved search it's not being removed correctly on parallel device

What is the root cause of that problem?

This issue only happens for the last saved search that user tries to remove. If there are any saved searches left after delete everything works fine.

I believe the issues lays on the BE side. When request is processed on the "main device", BE sends a pusher update to sync all the connected clients for the user that made the request. However that pusher event is sent with an empty array [] instead of an empty object {} which causes fail during update inside Onyx on the "secondary device".

Screenshot 2024-12-16 at 15 53 29

To confirm that, I've logged the value that this particular Onyx.set is receiving - was []. I've also added a condition to modify passed value in that one scenario. When swapped to empty object {} everything executed without any issues and saved search was deleted on both accounts simultaneously

Modification to Onyx.set

function set(key, value) {
    console.log(key, value, 'onyxSet')
    if (key === 'nvp_savedSearches') {
        value = {};
    }

Screenshot 2024-12-16 at 15 59 18

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

I believe the issue lies on the BE side. When DeleteSavedSearch command that removes last saved search is processed by BE, pusher update that is being send should clear the nvp_savedSearches via empty object {} instead of array []

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

N/A

What alternative solutions did you explore? (Optional)

N/A

Copy link

melvin-bot bot commented Dec 17, 2024

@rojiphil, @bfitzexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

@rojiphil
Copy link
Contributor

I believe the issue lies on the BE side.

@MrMuzyk Thanks for the proposal. I agree with the analysis.
Let's bring in the internal engineer to weigh in here.
🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented Dec 18, 2024

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

Copy link

melvin-bot bot commented Dec 18, 2024

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

@carlosmiceli
Copy link
Contributor

The analysis seems correct. I'll take over this one.

@carlosmiceli carlosmiceli added Internal Requires API changes or must be handled by Expensify staff Weekly KSv2 and removed External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Daily KSv2 labels Dec 18, 2024
@rojiphil
Copy link
Contributor

Unassigning myself as there is nothing to be done from FE side. Will stay subscribed though. Thanks.

@rojiphil rojiphil removed their assignment Dec 19, 2024
Copy link

melvin-bot bot commented Dec 25, 2024

@carlosmiceli @bfitzexpensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@carlosmiceli carlosmiceli changed the title [$250] Search - Deleted search doesn´t disappear from secondary device despite refreshing page Search - Deleted search doesn´t disappear from secondary device despite refreshing page Dec 26, 2024
@melvin-bot melvin-bot bot added the Overdue label Dec 27, 2024
@carlosmiceli
Copy link
Contributor

I have quite a bit on my plate so will have to unassign myself, maybe @luacmartins can fix this one quickly since he has a lot of context on Search.

@melvin-bot melvin-bot bot removed the Overdue label Dec 28, 2024
@carlosmiceli carlosmiceli removed their assignment Dec 28, 2024
@carlosmiceli carlosmiceli added Weekly KSv2 and removed Weekly KSv2 labels Dec 28, 2024
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. Internal Requires API changes or must be handled by Expensify staff Weekly KSv2
Projects
Status: Done
Development

No branches or pull requests

6 participants