[HOLD for payment 2025-01-02] [$250] Search - Search loads infinitely when search query contains type:chat and category query

[IuliiaHerets]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 9.0.65-1
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): applausetester+kh0811007@applause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Click search router icon.
3. Enter the following query: type:chat in: category:car
4. Hit Enter.

Expected Result:

Search will not load infinitely.

Actual Result:

Search loads infinitely.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6671890_1732196532024.20241121_212559.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021859790661845309370
• Upwork Job ID: 1859790661845309370
• Last Price Increase: 2024-12-06
• Automatic offers:
• daledah | Contributor | 105274097

Issue Owner

Current Issue Owner: @Christinadobrzyn

[melvin-bot]

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

[daledah]

Edited by proposal-police: This proposal was edited at 2024-11-21T21:22:56Z.

Proposal

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

Search loads infinitely.

What is the root cause of that problem?

The API Search with the above query returns error

When Search API fails, we just update the loading to false

App/src/libs/actions/Search.ts

Line 45 in 4c44827

isLoading: false,

that make the condition shouldShowLoadingState is still false

App/src/components/Search/index.tsx

Lines 190 to 195 in 4c44827

	const isDataLoaded =
	searchResults?.data !== undefined && searchResults?.search?.type === type && Array.isArray(status)
	? searchResults?.search?.status === status.join(',')
	: searchResults?.search?.status === status;
	const shouldShowLoadingState = !isOffline && !isDataLoaded;

so we can see the loading state

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

Solution 1: Add failure data within error in

App/src/libs/actions/Search.ts

Line 51 in 4c44827

return {optimisticData, finallyData};

then in Search.tsx, detect if error is in searchResults, we will show the error message

Solution 2: Add failure data within data: [], status, and type

    const failureData = [{
            onyxMethod: Onyx.METHOD.MERGE,
            key: `${ONYXKEYS.COLLECTION.SNAPSHOT}${hash}`,
            value: {
                data: [],
                search: {
                    status,
                    type
                }
            },
    }]

then the empty state will be shown

What alternative solutions did you explore? (Optional)

NA

[Christinadobrzyn]

I think this might have to do with how type:chat in:<any chat> category:car is being read? Here's what is showing under keyword, I think this is supposed to be 'chat'?

[melvin-bot]

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

[melvin-bot]

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

[Anaslancer]

Proposal

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

Search - Search loads infinitely when search query contains type:chat and category query

What is the root cause of that problem?

The Search api returns jsonCode: 402 and empty onyxData.
then

App/src/components/Search/index.tsx

Lines 190 to 195 in 5f483dc

	const isDataLoaded =
	searchResults?.data !== undefined && searchResults?.search?.type === type && Array.isArray(status)
	? searchResults?.search?.status === status.join(',')
	: searchResults?.search?.status === status;
	const shouldShowLoadingState = !isOffline && !isDataLoaded;

shouldShowLoadingState is true because searchResults?.search?.status is undefined.
According to this below logic, we are showing the infinite loading component.

App/src/components/Search/index.tsx

Lines 274 to 281 in 5f483dc

	if (shouldShowLoadingState) {
	return (
	<SearchRowSkeleton
	shouldAnimate
	containerStyle={shouldUseNarrowLayout && styles.searchListContentContainerStyles}
	/>
	);
	}

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

We have to add the failureData in getOnyxLoadingData function.

    const failureData: OnyxUpdate[] = [
        {
            onyxMethod: Onyx.METHOD.MERGE,
            key: `${ONYXKEYS.COLLECTION.SNAPSHOT}${hash}`,
            value: {
                search: {
                    ...(status && { status }),
                },
                data: undefined,
            },
        }
    ];

What alternative solutions did you explore? (Optional)

Or the backend should send the correct result in Search api.

Contributor details

Your Expensify account email: anasup1995@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~01aff093c9a804b145

[melvin-bot]

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

[huult]

Edited by proposal-police: This proposal was edited at 2024-11-29 04:40:23 UTC.

Proposal

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

Search loads infinitely when search query contains type:chat and category query

What is the root cause of that problem?

This issue occurred because we sent incorrect parameters to the Search API. On the backend, there is parameter validation that returns a 402 error code.

For example, if type === chat, we cannot set the key as category, expenseType, or currency,... because they are used for expense types. This is validated on the backend, or there is logic that does not handle this case, resulting in an error.

• 

So, the backend response onyxData is empty, causing the condition to hide the loading indicator to fail. As a result, the loading indicator remains visible when onyxData is empty, as shown in the response log above.

App/src/components/Search/index.tsx

Lines 194 to 199 in 754a11c

	const isDataLoaded =
	searchResults?.data !== undefined && searchResults?.search?.type === type && Array.isArray(status)
	? searchResults?.search?.status === status.join(',')
	: searchResults?.search?.status === status;
	const shouldShowLoadingState = !isOffline && !isDataLoaded;

For example, the backend returns a 402 error:
type:chat status:all in:mountain tag:abcd
type:chat status:all in:mountain expenseType:abcd
type:chat status:all in:mountain currency:abcd

Because this query is only correct for type:expense

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

To resolve this issue, we should check if type === chat, and in that case, don't accept category, expenseType, or currency ... as filter keys. Instead, convert them to quoted values, or if we want to change anything, we can discuss it in the pull request.. Something like this:

function buildSearchQueryString(queryJSON?: SearchQueryJSON) {
    ...
// src/libs/SearchQueryUtils.ts#L313
    for (const filter of filters) {
+        const includeQuotes =
+            queryParts.some((item) => item === 'type:chat') &&
+            (filter.key === FILTER_KEYS.CATEGORY ||
+                filter.key === FILTER_KEYS.EXPENSE_TYPE ||
+                filter.key === FILTER_KEYS.TAG ||
+                filter.key === FILTER_KEYS.TAX_RATE ||
+                filter.key === FILTER_KEYS.DESCRIPTION ||
+                filter.key === FILTER_KEYS.MERCHANT ||
+                filter.key === FILTER_KEYS.CARD_ID ||
+                filter.key === FILTER_KEYS.CURRENCY);

        const filterValueString = buildFilterValuesString(filter.key, filter.filters);

+        if (includeQuotes) {
+            queryParts.push(`"${filterValueString}"`.replace(/\s+/g, ''));
+        } else {
            queryParts.push(filterValueString);
+        }
    }

    return queryParts.join(' ');
}

Note: if other type (trip or ...) same with that case, then we able reuse this solution.

Test branch

POC

Screen.Recording.2024-11-25.at.15.43.11.mov

What alternative solutions did you explore? (Optional)

To resolve this issue, we need to do two things:

1. Double-check the JSON query to match the backend's requirements.
2. Handle the case where onyxData is an empty array.

App/src/components/Search/index.tsx

Line 308 in 9933888

//src/components/Search/index.tsx#L308
+    if (searchResults === undefined && !isOffline) {
+        setShouldShowStatusBarLoading(false);

+        return (
+            <View style={[shouldUseNarrowLayout ? styles.searchListContentContainerStyles : styles.mt3, styles.flex1]}>
+                <EmptySearchView type={type} />
+            </View>
+        );
+    }

POC

[melvin-bot]

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

[Christinadobrzyn]

@alitoshmatov can you review these proposals? Thanks!

[alitoshmatov]

that make the condition shouldShowLoadingState is still false

@daledah I assume you meant isDataLoaded is still false, your RCA is correct. Can you elaborate how you want to show error. I like the idea of adding failureData to handle any errors in api.

[alitoshmatov]

@Anaslancer Thank you for your proposal, your solution is the same as @daledah 's.

[alitoshmatov]

@huult Thank you for detailed proposal, your RCA is correct. But I don't think your solution is future-proof, your solution does solve this issue, but when backend responds with another error the same thing happens again.

[huult]

@alitoshmatov Thank you for your feedback.

This issue occurred because the app sent a query with incorrect syntax to the backend, which the backend could not process, resulting in an error.

Therefore, I think we should validate the query to ensure the correct syntax is sent to the backend. With this approach, we can perform the search; if not, we can handle the error by hiding the loading indicator and showing a 'not found' message. However, this might introduce a new issue where the category is sent, but the search result is 'not found'.

But I don't think your solution is future-proof,

With this concern, I suggest we confirm with the internal team to get information about the filter key that the backend can support.

For example:

type = expense support key: category, tag...
type = chat support key: from, to...
type = trip support key: from, to...

to we can create the correct query syntax.

@alitoshmatov If you agree with this, we can discuss it in detail and define the next steps

[huult]

Proposal updated

• Add alternative solutions

[melvin-bot]

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

[daledah]

@alitoshmatov I think we can show the error at the end of search page same as what we did for form. We can get some thoughts from design team. What do you think?

[melvin-bot]

@Christinadobrzyn, @alitoshmatov Eep! 4 days overdue now. Issues have feelings too...

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[alitoshmatov]

Issue not reproducible during KI retests. (First week)

Issue is still reproducable with type:chat status:unread in:Mountain category:car query

[Christinadobrzyn]

hi @alitoshmatov and @daledah just checking on the PR for this. Can you provide an update?

[alitoshmatov]

PR in progress, waiting @daledah to apply changes to text copies based on slack conversation here

[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.78-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: show empty search view when search API return error #54007

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

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

• @alitoshmatov requires payment through NewDot Manual Requests
• @daledah requires payment automatic offer (Contributor)

[melvin-bot]

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

[Christinadobrzyn]

Payment day

Contributor: @daledah paid $250 via Upwork
Contributor+: @alitoshmatov owed $250 via NewDot

@alitoshmatov do we need a regression test for this?

[alitoshmatov]

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: https://github.com/Expensify/App/pull/52185/files#r1904459243

• [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: No discussion

• [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.

• [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/457802

Regression Test Proposal

Precondition:

Test:

1. Go to search page
2. Enter type:chat status:unread in:Mountain category:car into search input
3. Click search
4. Make sure infinite loading does not appear
5. Make sure empty page is displayed

Do we agree 👍 or 👎

[alitoshmatov]

@Christinadobrzyn Completed the checklist, requested payment in ND

[Christinadobrzyn]

Thanks @alitoshmatov! Regression test here

Payment summary here - #52907 (comment)

Please make sure to reach out in ND for payment @alitoshmatov!

Going to close this as complete

[JmillsExpensify]

$250 approved for @alitoshmatov