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

[HOLD for payment 2024-12-09] [$250] Search - Workspace switcher resets to Expensify after manually searching via search input #51416

Open
2 of 8 tasks
lanitochka17 opened this issue Oct 24, 2024 · 29 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Overdue

Comments

@lanitochka17
Copy link

lanitochka17 commented Oct 24, 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.53-0
Reproducible in staging?: Y
Reproducible in production?: Y
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: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to Search
  3. Click Filters
  4. Apply any filter and click Save
  5. Click View results
  6. Open workspace switcher and select a workspace
  7. Click on the search input field
  8. Type any query and hit Enter

Expected Result:

The workspace switcher will not change after manually typing on the search field and hitting Enter

Actual Result:

The workspace switcher changes to Expensify after manually typing on the search field and hitting Enter

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

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

Screenshots/Videos

Add any screenshot/video evidence
Bug6644138_1729758750803.20241024_162621.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021851453987704756817
  • Upwork Job ID: 1851453987704756817
  • Last Price Increase: 2024-11-06
  • Automatic offers:
    • Krishna2323 | Contributor | 104839473
Issue OwnerCurrent Issue Owner: @eVoloshchak
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 24, 2024
Copy link

melvin-bot bot commented Oct 24, 2024

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

@lanitochka17
Copy link
Author

@RachCHopkins FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

@lanitochka17
Copy link
Author

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

@Krishna2323
Copy link
Contributor

Krishna2323 commented Oct 24, 2024

Edited by proposal-police: This proposal was edited at 2024-10-24 14:37:30 UTC.

Proposal


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

Search - Workspace switcher resets to Expensify after manually searching via search input

What is the root cause of that problem?

  • The policyID isn't included in the input.
  • The policyID is not set when we are passing the standardizedQuery to buildSearchQueryString.
    const onSubmit = () => {
    if (!inputValue) {
    return;
    }
    const inputQueryJSON = SearchQueryUtils.buildSearchQueryJSON(inputValue);
    if (inputQueryJSON) {
    const standardizedQuery = SearchQueryUtils.standardizeQueryJSON(inputQueryJSON, cardList, taxRates);
    const query = SearchQueryUtils.buildSearchQueryString(standardizedQuery);
    SearchActions.clearAllFilters();
    Navigation.navigate(ROUTES.SEARCH_CENTRAL_PANE.getRoute({query}));
    } else {
    Log.alert(`${CONST.ERROR.ENSURE_BUGBOT} user query failed to parse`, inputValue, false);
    }
    };

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


  • We can the value of activeWorkspaceID and if it is available, we can set the policyID. Or instead of activeWorkspaceID, we can check for queryJSON.policyID
    const onSubmit = () => {
        if (!inputValue) {
            return;
        }
        const inputQueryJSON = SearchQueryUtils.buildSearchQueryJSON(inputValue);
        if (inputQueryJSON) {
            const standardizedQuery = SearchQueryUtils.standardizeQueryJSON(inputQueryJSON, cardList, taxRates);
            if (activeWorkspaceID) {
                standardizedQuery.policyID = activeWorkspaceID;
            }
            const query = SearchQueryUtils.buildSearchQueryString(standardizedQuery);
            SearchActions.clearAllFilters();
            Navigation.navigate(ROUTES.SEARCH_CENTRAL_PANE.getRoute({query}));
        } else {
            Log.alert(`${CONST.ERROR.ENSURE_BUGBOT} user query failed to parse`, inputValue, false);
        }
    };
  • We should also look for the same bug in other places.

What alternative solutions did you explore? (Optional)

Result

@shahinyan11
Copy link

shahinyan11 commented Oct 24, 2024

Edited by proposal-police: This proposal was edited at 2024-10-24 14:57:34 UTC.

Proposal

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

Search - Workspace switcher resets to Expensify after manually searching via search input

What is the root cause of that problem?

Here we build search query from inputValue which does not contain policyId

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

I don't know if there might be other parameters in the URL that might also be missed like policyId, so I'm providing a solution below to not miss any parameter.

Update inputQueryJSON parameter to {...queryJSON, ...inputQueryJSON}.

What alternative solutions did you explore? (Optional)

@melvin-bot melvin-bot bot added the Overdue label Oct 28, 2024
@RachCHopkins
Copy link
Contributor

Will triage tomorrow.

@melvin-bot melvin-bot bot removed the Overdue label Oct 29, 2024
@RachCHopkins
Copy link
Contributor

Easy to Repro. I did not know the workspace switcher was a thing!

@RachCHopkins RachCHopkins added the External Added to denote the issue can be worked on by a contributor label Oct 30, 2024
@melvin-bot melvin-bot bot changed the title Search - Workspace switcher resets to Expensify after manually searching via search input [$250] Search - Workspace switcher resets to Expensify after manually searching via search input Oct 30, 2024
Copy link

melvin-bot bot commented Oct 30, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 30, 2024
Copy link

melvin-bot bot commented Oct 30, 2024

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

Copy link

melvin-bot bot commented Nov 4, 2024

@eVoloshchak, @RachCHopkins Eep! 4 days overdue now. Issues have feelings too...

@melvin-bot melvin-bot bot added the Overdue label Nov 4, 2024
@RachCHopkins
Copy link
Contributor

@eVoloshchak how do you feel about the proposals so far?

@garrettmknight garrettmknight moved this to External Bugs and Follow Up Issues in [#whatsnext] #expense Nov 5, 2024
Copy link

melvin-bot bot commented Nov 6, 2024

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

@eVoloshchak
Copy link
Contributor

@Krishna2323's proposal looks good to me!

We should also look for the same bug in other places.

The same bug occurs with a simple search (without selecting a category), this does seem to resolve this issue for any search query

🎀👀🎀 C+ reviewed!

Copy link

melvin-bot bot commented Nov 10, 2024

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

@lakchote
Copy link
Contributor

@Krishna2323's proposal LGTM.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 11, 2024
Copy link

melvin-bot bot commented Nov 11, 2024

📣 @Krishna2323 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@Krishna2323
Copy link
Contributor

PR will be up today.

@Krishna2323
Copy link
Contributor

Krishna2323 commented Nov 12, 2024

@lakchote @eVoloshchak, I’m unsure what to do with the search router. Currently, it removes the active workspace when we search. Should we update it to maintain the workspace selection during a search, or is it expected behavior to remove the selection to show all results?

Monosnap.screencast.2024-11-13.04-07-07.mp4

@lakchote
Copy link
Contributor

@Krishna2323 in your test you switch to another workspace, then you search for something and select a result corresponding to yet another workspace.

In my opinion, we should switch to the workspace of the search's result selected in the search router.

What's your opinion @luacmartins?

@luacmartins
Copy link
Contributor

I think this issue is similar to #49637, where we want to keep the workspace switcher and not clear it when the user makes a search either typed in the router or via the UI filters. cc @Kicu

@Kicu
Copy link
Contributor

Kicu commented Nov 14, 2024

I agree, in fact this feels like a regression. I swear that I seem to remember working on this, and the scenario shown in the video from @Krishna2323 used to work 🤔

I believe when submitting the search query that the user typed in, we should check if there is a workspace selected, and if yes then add policyID to final query.
The mechanism that correctly sets workspace switcher when policyID is part of search query, is already in the code and works (I verified on stage).

@RachCHopkins
Copy link
Contributor

@lakchote @luacmartins @eVoloshchak forgive my ignorance here - what is the next step for this issue? We need to make it work how @Kicu describes above?

@lakchote
Copy link
Contributor

lakchote commented Dec 2, 2024

@lakchote @luacmartins @eVoloshchak forgive my ignorance here - what is the next step for this issue? We need to make it work how @Kicu describes above?

Yes, we'll make it work like @Kicu described above

@Krishna2323 raised a PR that was merged to fix the issue

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Dec 2, 2024
@melvin-bot melvin-bot bot changed the title [$250] Search - Workspace switcher resets to Expensify after manually searching via search input [HOLD for payment 2024-12-09] [$250] Search - Workspace switcher resets to Expensify after manually searching via search input Dec 2, 2024
Copy link

melvin-bot bot commented Dec 2, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Dec 2, 2024
Copy link

melvin-bot bot commented Dec 2, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.69-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-12-09. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Dec 2, 2024

@eVoloshchak @RachCHopkins @eVoloshchak The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

@garrettmknight garrettmknight moved this from Bugs and Follow Up Issues to Hold for Payment in [#whatsnext] #expense Dec 3, 2024
@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Dec 9, 2024
@RachCHopkins
Copy link
Contributor

@eVoloshchak can you please do the checklist so that I can pay people? Thanks!

@melvin-bot melvin-bot bot added the Overdue label Dec 12, 2024
Copy link

melvin-bot bot commented Dec 13, 2024

@eVoloshchak, @lakchote, @RachCHopkins, @Krishna2323 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@lakchote
Copy link
Contributor

@eVoloshchak friendly bump!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Overdue
Projects
Status: Hold for Payment
Development

No branches or pull requests

8 participants