[HOLD for payment 2024-12-09] [$250] Search - Workspace switcher resets to Expensify after manually searching via search input
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:
- Go to staging.new.expensify.com
- Go to Search
- Click Filters
- Apply any filter and click Save
- Click View results
- Open workspace switcher and select a workspace
- Click on the search input field
- 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
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 Owner
Current Issue Owner: @RachCHopkins
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.
@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
We think that this bug might be related to #wave-control
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
standardizedQuerytobuildSearchQueryString. 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
activeWorkspaceIDand if it is available, we can set thepolicyID. Or instead ofactiveWorkspaceID, we can check forqueryJSON.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
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)
Will triage tomorrow.
Easy to Repro. I did not know the workspace switcher was a thing!
Job added to Upwork: https://www.upwork.com/jobs/~021851453987704756817
Triggered auto assignment to Contributor-plus team member for initial proposal review - @eVoloshchak (External)
@eVoloshchak, @RachCHopkins Eep! 4 days overdue now. Issues have feelings too...
@eVoloshchak how do you feel about the proposals so far?
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
@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!
Triggered auto assignment to @lakchote, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
@Krishna2323's proposal LGTM.
π£ @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 π
PR will be up today.
@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 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?
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
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).
@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 @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
Reviewing label has been removed, please complete the "BugZero Checklist".
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)
@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]
@eVoloshchak can you please do the checklist so that I can pay people? Thanks!
@eVoloshchak, @lakchote, @RachCHopkins, @Krishna2323 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
@eVoloshchak friendly bump!
- 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
- Go to Search
- Click Filters
- Apply any filter and click Save
- Click View results
- Open the workspace switcher and select a workspace
- Click on the search input field
- Type any query and hit Enter
- Verify that the active workspace is not changed in the workspace switcher
Do we agree π or π