Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.55k stars 2.89k forks source link

[$250] Search - Inconsistency between No tag and No category filters #51828

Open lanitochka17 opened 1 week ago

lanitochka17 commented 1 week ago

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.56-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): applausetester+ak1031@applause.expensifail.com Issue reported by: Applause - Internal Team

Action Performed:

Peconditions: existing account with workspace created, tags are enabled for the workspace and the following expenses are created:

Expected Result:

Only expenses matching category selected in filters and without tags should be displayed

Actual Result:

All expenses without tags are displayed, category filter is omitted

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/84878378-831e-42b5-ad1f-d4e56eadf76b

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021853546282413139923
  • Upwork Job ID: 1853546282413139923
  • Last Price Increase: 2024-11-04
Issue OwnerCurrent Issue Owner: @jayeshmangwani
melvin-bot[bot] commented 1 week ago

Triggered auto assignment to @slafortune (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 commented 1 week ago

@slafortune 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 commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

@slafortune Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

slafortune commented 1 week ago

Either way - you can be left with options that don't pertain if you filter by tags then categories. While it doesn't have a category, it also doesn't have the proper tag to be included in the filter -

image
melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

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

klajdipaja commented 1 week ago

This is a BE issue. The query that we send seems to me like it has the right data in it. This is what we send to BE for a search wiht no tag but Advertising category type:expense status:all category:Advertising tag:none :

{
    "type": "expense",
    "status": "all",
    "sortBy": "date",
    "sortOrder": "desc",
    "filters": {
        "operator": "and",
        "left": {
            "operator": "eq",
            "left": "category",
            "right": "Advertising"
        },
        "right": {
            "operator": "eq",
            "left": "tag",
            "right": "none"
        }
    },
    "inputQuery": "sortBy:date sortOrder:desc type:expense status:all category:Advertising tag:none",
    "hash": 113990055,
    "offset": 0
}

Maybe we need to provide a different value instead of none when No Tag or No Category is selected?

ijmalik commented 5 days ago

Proposal

Please re-state the problem that we are trying to solve in this issue. In Search, there is an inconsistency in the behavior when selecting "No Tag" or "No Category" filters.

What is the root cause of that problem?

1- https://github.com/Expensify/App/blob/ad0f834b805b763fb5885d2003ca2bdd6e33ff64/src/pages/Search/SearchAdvancedFiltersPage/SearchFiltersCategoryPage.tsx#L20-L24 Initially, searchAdvancedFiltersForm returns "none" for the category when the user selects "No Category."

2- https://github.com/Expensify/App/blob/afa7b33107747d0242acfd01d032d5339be390d9/src/libs/actions/Search.ts#L139-L141 The issue then arises due to the way jsonQuery is structured when passed to the SEARCH API. For example, the current jsonQuery looks like this:

{
    "type": "expense",
    "status": "all",
    "sortBy": "date",
    "sortOrder": "desc",
    "filters": {
        "operator": "and",
        "left": {
            "operator": "eq",
            "left": "category",
            "right": "Fees"
        },
        "right": {
            "operator": "eq",
            "left": "tag",
            "right": "none"
        }
    },
    "inputQuery": "sortBy:date sortOrder:desc type:expense status:all category:Fees tag:none",
    "hash": 46304472,
    "recentSearchHash": 2055726720,
    "offset": 0
}

The backend does not interpret "none" consistently, whether it appears in filters or inputQuery. Additionally, filter order affects the results.

Correct Results occur, for example, when "No Tag" is selected before "Fees" in the category:

type:expense status:all tag:none category:Fees
type:expense status:all category:none tag:"First Tag"
type:expense status:all tag:none merchant:m10

Incorrect Results are returned, for example, when "Fees" is selected before "No Tag":

type:expense status:all category:Fees tag:none
type:expense status:all tag:"First Tag" category:none
type:expense status:all merchant:m10 tag:none

This is the root cause of the inconsistent behavior with "No Tag" and "No Category" filters.

What changes do you think we should make in order to solve the problem? https://github.com/Expensify/App/blob/afa7b33107747d0242acfd01d032d5339be390d9/src/libs/actions/Search.ts#L139 Replace with

const jsonQuery = JSON.stringify(removeNoneFromTagAndCategory(queryWithOffset));

and add the following function to replace "none" with empty string

function removeNoneFromTagAndCategory(queryObject) {
    if (queryObject.filters?.right) {
        const { left, right } = queryObject.filters.right;
        if ((left === "tag" || left === "category") && right === "none") {
            queryObject.filters.right.right = '';
        }
    }

    if (queryObject.inputQuery) {
        queryObject.inputQuery = queryObject.inputQuery
            .replace("tag:none", "tag:")
            .replace("category:none", "category:");
    }
    return queryObject;
}

What alternative solutions did you explore? (Optional) Rather than replacing "none" with an empty string, we could reorder the jsonQuery properly before the API call to ensure consistent results. For example:

type:expense status:all tag:none category:Fees
type:expense status:all category:none tag:"First Tag"
type:expense status:all tag:none merchant:m10

Screenshot: 51828-0Expenses

Videos: Before Change: Correct Results

https://github.com/user-attachments/assets/2020a3cb-1cea-4aa9-ba99-7da481a86472

Before Change: Incorrect Results

https://github.com/user-attachments/assets/20018427-6ea4-48a3-aad4-ad52ef0a1b2c

After Change: All Results are Correct

https://github.com/user-attachments/assets/76785e1b-652a-49e5-b072-5ac7860204c3

jayeshmangwani commented 4 days ago

@ijmalik Issue appears to be coming from the backend, so it seems internal for now. could you please confirm if the issue is from the FE or BE?

ijmalik commented 4 days ago

@jayeshmangwani Hi, This is a front-end (FE) issue, and my proposal above provides the resolution.https://github.com/Expensify/App/issues/51828#issuecomment-2461892145

On Fri, Nov 8, 2024, 11:25 AM Jayesh Mangwani @.***> wrote:

@ijmalik https://github.com/ijmalik Issue appears to be coming from the backend https://github.com/Expensify/App/issues/51828#issuecomment-2456815267, so it seems internal for now. could you please confirm if the issue is from the FE or BE?

— Reply to this email directly, view it on GitHub https://github.com/Expensify/App/issues/51828#issuecomment-2463875779, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQ6GVN7BPUVH6XHQCXNXITZ7RKN3AVCNFSM6AAAAABQ65FZHOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINRTHA3TKNZXHE . You are receiving this because you were mentioned.Message ID: @.***>

melvin-bot[bot] commented 1 day ago

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