[HOLD for payment 2025-02-04] [$250] Removing one value from Search filter clears the search

[luacmartins]

Coming from here, we're clearing the search when we remove one of the values from a filter, e.g. removing category:Advertising from category:Advertising tag:Accounting.

Reproduction steps:

Go to Reports page
Search for category:"Meals and Entertainment",Advertising
Click on search header and edit query to remove Advertising
Expected Result:

Search query is now type:expense status:all category:"Meals and Entertainment"
Actual Result:

Search query is cleared

Extra info:
Based on @jaydamani's investigation, submitSearch gets called twice for some reason and on second time the search query is blank so filter resets

Screen.Recording.2025-01-17.at.19.33.04.mp4

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021880343282436872367
• Upwork Job ID: 1880343282436872367
• Last Price Increase: 2025-01-17
• Automatic offers:
• ishakakkad | Contributor | 105774673

Issue Owner

Current Issue Owner: @kadiealexander

[melvin-bot]

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

[luacmartins]

cc @jaydamani

[melvin-bot]

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

[melvin-bot]

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

[QichenZhu]

Proposal

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

Editing search conditions clears the search.

What is the root cause of that problem?

1. The selection list calls submitSearch() because it automatically selects the first item when pressing Enter.
   

   App/src/components/Search/SearchPageHeaderInput.tsx

   Line 173 in 91b84ef

   submitSearch(item.searchQuery);

2. submitSearch() clears the text input.

App/src/components/Search/SearchPageHeaderInput.tsx

Line 148 in 91b84ef

setTextInputValue('');

3. The search input calls submitSearch() again, and the input is empty at that time.

App/src/components/Search/SearchPageHeaderInput.tsx

Lines 262 to 264 in 91b84ef

	onSubmit={() => {
	submitSearch(textInputValue);
	}}

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

1. In the search popup this issue doesn't happen because it checks if the an item has focus before submitting the search.

App/src/components/Search/SearchRouter/SearchRouter.tsx

Lines 295 to 301 in 91b84ef

	onSubmit={() => {
	const focusedOption = listRef.current?.getFocusedOption();
	if (!focusedOption) {
	submitSearch(textInputValue);
	return;
	}

We could copy the same logic to SearchRouterHeaderInput.

2. Don't automatically highlight the first item when the input is empty, as this indicates the user intends to clear the search.

App/src/components/Search/SearchPageHeaderInput.tsx

Lines 127 to 128 in 91b84ef

	if (updatedUserQuery || textInputValue.length > 0) {
	listRef.current?.updateAndScrollToFocusedIndex(0);

Change the above conditions as below:

if (updatedUserQuery && textInputValue.length > 0) {

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

Spy on Navigation.navigate() and expect it to be called once with the correct parameters after editing the search conditions and pressing Enter.

What alternative solutions did you explore? (Optional)

[github-actions]

⚠️ @QichenZhu Thanks for your proposal. Please update it to follow the proposal template, as proposals are only reviewed if they follow that format (note the mandatory sections).

[HRedz]

Proposal

Please restate the problem

Removing filters from an existing search query and resubmitting it causes the entire query to be cleared instead of just the filters you removed.

What is the root cause of the problem

In the SearchPageHeaderInput.tsx submitSearch has the following conditional block which will execute when a query is modified, wiping the entire query.

App/src/components/Search/SearchPageHeaderInput.tsx

Lines 146 to 151 in dd0de18

	if (updatedQuery !== originalInputQuery) {
	clearAllFilters();
	setTextInputValue('');
	setAutocompleteQueryValue('');
	setIsAutocompleteListVisible(false);
	}

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

We could comment out these 2 lines

App/src/components/Search/SearchPageHeaderInput.tsx

Lines 148 to 149 in dd0de18

	setTextInputValue('');
	setAutocompleteQueryValue('');

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

Substituting a value in the query (e.g., removing “Advertising”)

• Action: Start with a full query like "category:"Meals and Entertainment",Advertising", remove “Advertising,” and submit.
• Expected: The text input updates to "category:"Meals and Entertainment"" and does not get cleared entirely.

Submitting a brand-new query

• Action: Replace the existing query with a completely different search term or field (e.g., "category:"Travel"" or "keyword:"Laptop"") and submit.
• Expected: The app navigates/updates the route appropriately, and the query remains in the search bar.

Re-submitting the same query

• Action: Type (or autocomplete) a query ("category:"Meals and Entertainment""), submit it once, then immediately submit the exact same query again.
• Expected: The second submission should not cause unexpected clears if the query is truly unchanged.

What alternative solutions did you explore? (Optional)

We could add more conditions to the if statement to prevent it from executing while you are editing an existing query, by tracking the state of the query (writing a new one or editing an existing one).

[melvin-bot]

📣 @HRedz! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[HRedz]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01929686b8d6175591

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[s77rt]

@QichenZhu Thanks for the proposal. Your RCA is correct. I think removing onSubmit makes sense here but can you confirm how the initial onListItemPress call is executed?

[s77rt]

@HRedz Thanks for the proposal. Your RCA is not correct as the bug does not occur just be choosing another query e.g. clicking on the search item.

Screen.Recording.2025-01-18.at.5.56.22.PM.mov

[ishakakkad]

🚨 Edited by proposal-police: This proposal was edited at 2025-01-19 19:47:34 UTC.

Proposal

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

Removing one value from Search filter clears the search

What is the root cause of that problem?

When we remove the value from filter and press enter, two event fire onListItemPress event on selection list from here and another onSubmit event for the input field from here

1. When event fire for selection list it will call submitSearch()
   

   App/src/components/Search/SearchPageHeaderInput.tsx

   Line 173 in dc7bd0e

   submitSearch(item.searchQuery);

2. And submitSearch clear out text input value.
   

   App/src/components/Search/SearchPageHeaderInput.tsx

   Line 148 in dc7bd0e

   setTextInputValue('');

3. And when onSubmit event for the input field it will call same submitSearch() method and input value is empty this time.
   

   App/src/components/Search/SearchPageHeaderInput.tsx

   Lines 262 to 264 in dc7bd0e

   	onSubmit={() => {
   	submitSearch(textInputValue);
   	}}

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

To fix this issue we need to make sure submitSearch() will call only once. But we can not remove the onSubmit method from here .

If we remove this method it will not allow you reset the filter. Because when we clear input field and press enter it will take first item from the selection list always.

So to fix this issue we need to make sure whenever input field does not have any value and press enter we should run this block. And when input has value we should run this block. For this following changes are required.

1. We need to remove focus from the selection list when we clear the input. For this we need to make following changes here

onSearchQueryChange={(userQuery) => {
    onSearchQueryChange(userQuery);
    if (!userQuery) {
        listRef.current?.updateAndScrollToFocusedIndex(-1);
    }
}}

2. We should call submit button on input if any item is not focused from the selection list. For this we need to make changes here

onSubmit={() => {
  const focusedOption = listRef.current?.getFocusedOption(); 
  if (!focusedOption) { 
      submitSearch(textInputValue);
  }
}}

3. When user clear the input field, and after that user select the previously selected item and press enter. For this we should set the input text for the previous value. For this there can be two solutions.

• Solution 1: Following changes needed here.

} else if (item.searchItemType === CONST.SEARCH.SEARCH_ROUTER_ITEM_TYPE.SEARCH) {
    submitSearch(item.searchQuery);
    setTextInputValue(item.text || '');
    setAutocompleteQueryValue(item.text || '');
}

OR

• Solution 2: Change the route param with timestamp, it will force react to rerender the route. We need to change here

Navigation.navigate(ROUTES.SEARCH_CENTRAL_PANE.getRoute({query: updatedQuery}));
Navigation.setParams({timeStamp: new Date().getTime()});

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

NA UI bug

What alternative solutions did you explore? (Optional)

[s77rt]

@ishakakkad Thanks for the proposal. Your RCA is correct however the solution won't always work as expected, e.g. if you clear the input, you can't select the first item, you will have to use arrow key down then up.

[ishakakkad]

@s77rt thank you for your suggestion. You are right, previous solution does not work in all the scenarios. I have updated my proposal. Can you please recheck?

[QichenZhu]

@s77rt Thanks for your review!

how the initial onListItemPress call is executed?

The selection list selects the focused option when pressing Enter, even if pressed within a text input.

App/src/components/SelectionList/BaseSelectionList.tsx

Lines 766 to 768 in 155818e

	/** Selects row when pressing Enter */
	useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ENTER, selectFocusedOption, {
	captureOnInputs: true,

selectFocusedOption calls selectRow, which calls onSelectRow. onSelectRow is bound to onListItemPress, which calls submitSearch.

App/src/components/SelectionList/BaseSelectionList.tsx

Line 428 in 155818e

selectRow(focusedOption);

App/src/components/SelectionList/BaseSelectionList.tsx

Line 383 in 155818e

onSelectRow(item);

App/src/components/Search/SearchRouter/SearchRouterList.tsx

Line 473 in 91b84ef

onSelectRow={onListItemPress}

App/src/components/Search/SearchPageHeaderInput.tsx

Line 278 in 91b84ef

searchQueryItem={searchQueryItem}

App/src/components/Search/SearchPageHeaderInput.tsx

Line 173 in 91b84ef

submitSearch(item.searchQuery);

[QichenZhu]

Proposal

Updated to support clearing search conditions

[github-actions]

⚠️ @QichenZhu Thanks for your proposal. Please update it to follow the proposal template, as proposals are only reviewed if they follow that format (note the mandatory sections).

[s77rt]

@ishakakkad Thanks for the update. Removing focus on clearing the text input is something that is supposed to be in place already but due to the outdated textInputValue, it's not working correctly. We can probably omit that extra condition.

App/src/components/Search/SearchPageHeaderInput.tsx

Lines 127 to 131 in 9c54d12

	if (updatedUserQuery || textInputValue.length > 0) {
	listRef.current?.updateAndScrollToFocusedIndex(0);
	} else {
	listRef.current?.updateAndScrollToFocusedIndex(-1);
	}

As for the other problem (re-selecting previous query), we should use queryText. This can be done in an else block here

App/src/components/Search/SearchPageHeaderInput.tsx

Lines 146 to 151 in 9c54d12

	if (updatedQuery !== originalInputQuery) {
	clearAllFilters();
	setTextInputValue('');
	setAutocompleteQueryValue('');
	setIsAutocompleteListVisible(false);
	}

Overall the proposal looks good to me.

🎀 👀 🎀 C+ reviewed
Link to proposal

[melvin-bot]

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

[QichenZhu]

@s77rt Thanks for the feedback!

[ishakakkad]

PR will be created by tomorrow.

[parasharrajat]

@ishakakkad Given that you are a new contributor, you are only allowed to work on a single issue at a time. I noticed that you are assigned to 3 issues. Please refrain yourself from posting proposals until those are completed. Please read our contributing guidelines.

[ishakakkad]

Thank you @parasharrajat I will take care.

[luacmartins]

@ishakakkad let's prioritize this one since it's affecting several users

[ishakakkad]

@luacmartins I am already working on this, will create PR soon.

[ishakakkad]

@s77rt PR is ready to review.

[melvin-bot]

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

[melvin-bot]

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

• Removing one value from Search filter clears the search #55636

If no regressions arise, payment will be issued on 2025-02-04. 🎊

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

• @s77rt requires payment through NewDot Manual Requests
• @ishakakkad requires payment automatic offer (Contributor)

[melvin-bot]

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

[s77rt]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
• 2b. Reported on staging (eg. found during regression or PR testing)
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] 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: Use autocomplete on Search Results Header #52568 (review)

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

  Bug requires regression test: Yes

Regression Test Proposal Template

• [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue: https://github.com/Expensify/Expensify/issues/467396

Regression Test Proposal

Precondition:

• Have submitted an expense with Category: Advertising and Merchant: Google
• Have submitted an expense with Category: Advertising and Merchant: Facebook

Test:

1. Go to Reports > Expenses
2. Search for type:expense status:all category:Advertising merchant:Google
3. Verify that only advertising expenses with Google merchant are displayed
4. Change search query to: type:expense status:all category:Advertising
5. Verify that only advertising expenses (with any merchant) are displayed

Do we agree 👍 or 👎

[kadiealexander]

Payments due:

• Contributor: @ishakakkad paid $250 via Upwork (automatic offer)
• Reviewer: @@s77rt owed $250 via Manual Requests

Upwork job

[JmillsExpensify]

$250 approved for @s77rt