App icon indicating copy to clipboard operation
App copied to clipboard

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

Open lanitochka17 opened this issue 1 year ago β€’ 27 comments

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
  • [x] MacOS: Chrome / Safari
  • [x] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/ebaf0fe8-4f29-4cd9-b509-c10f696baf62

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: @RachCHopkins

lanitochka17 avatar Oct 24 '24 14:10 lanitochka17

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.

melvin-bot[bot] avatar Oct 24 '24 14:10 melvin-bot[bot]

@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 avatar Oct 24 '24 14:10 lanitochka17

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

lanitochka17 avatar Oct 24 '24 14:10 lanitochka17

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. https://github.com/Expensify/App/blob/81e877a3e75e51fc19a14e43e4632d3889949584/src/components/Search/SearchPageHeader.tsx#L338-L351

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

Krishna2323 avatar Oct 24 '24 14:10 Krishna2323

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)

shahinyan11 avatar Oct 24 '24 14:10 shahinyan11

Will triage tomorrow.

RachCHopkins avatar Oct 29 '24 05:10 RachCHopkins

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

RachCHopkins avatar Oct 30 '24 02:10 RachCHopkins

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

melvin-bot[bot] avatar Oct 30 '24 02:10 melvin-bot[bot]

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

melvin-bot[bot] avatar Oct 30 '24 02:10 melvin-bot[bot]

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

melvin-bot[bot] avatar Nov 04 '24 18:11 melvin-bot[bot]

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

RachCHopkins avatar Nov 05 '24 05:11 RachCHopkins

πŸ“£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πŸ’Έ

melvin-bot[bot] avatar Nov 06 '24 16:11 melvin-bot[bot]

@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!

eVoloshchak avatar Nov 10 '24 22:11 eVoloshchak

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

melvin-bot[bot] avatar Nov 10 '24 22:11 melvin-bot[bot]

@Krishna2323's proposal LGTM.

lakchote avatar Nov 11 '24 09:11 lakchote

πŸ“£ @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 πŸ“–

melvin-bot[bot] avatar Nov 11 '24 09:11 melvin-bot[bot]

PR will be up today.

Krishna2323 avatar Nov 12 '24 10:11 Krishna2323

@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?

https://github.com/user-attachments/assets/36e7b200-3f5a-4c88-8146-2ff2c152f2a9

Krishna2323 avatar Nov 12 '24 22:11 Krishna2323

@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?

lakchote avatar Nov 13 '24 09:11 lakchote

I think this issue is similar to https://github.com/Expensify/App/issues/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

luacmartins avatar Nov 13 '24 19:11 luacmartins

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).

Kicu avatar Nov 14 '24 08:11 Kicu

@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?

RachCHopkins avatar Dec 02 '24 06:12 RachCHopkins

@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

lakchote avatar Dec 02 '24 07:12 lakchote

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

melvin-bot[bot] avatar Dec 02 '24 21:12 melvin-bot[bot]

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

  • https://github.com/Expensify/App/pull/52374

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

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

  • @eVoloshchak requires payment through NewDot Manual Requests
  • @Krishna2323 requires payment automatic offer (Contributor)

melvin-bot[bot] avatar Dec 02 '24 21:12 melvin-bot[bot]

@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]

melvin-bot[bot] avatar Dec 02 '24 21:12 melvin-bot[bot]

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

RachCHopkins avatar Dec 10 '24 01:12 RachCHopkins

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

melvin-bot[bot] avatar Dec 13 '24 09:12 melvin-bot[bot]

@eVoloshchak friendly bump!

lakchote avatar Dec 13 '24 09:12 lakchote

  • The PR that introduced the bug has been identified. Link to the PR: there isn't a PR that has introduced this bug, this is a functionality that was missed initially
  • 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: N/A
  • 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: Don't think an additional discussion is needed

Regression Test Proposal

  1. Go to Search
  2. Click Filters
  3. Apply any filter and click Save
  4. Click View results
  5. Open the workspace switcher and select a workspace
  6. Click on the search input field
  7. Type any query and hit Enter
  8. Verify that the active workspace is not changed in the workspace switcher

Do we agree πŸ‘ or πŸ‘Ž

eVoloshchak avatar Dec 16 '24 21:12 eVoloshchak