Skip to content

[AWAITING CHECKLIST] [HOLD for payment 2024-12-12] [$250] Search-Order of the words is reversed when the query is submitted #52170

Closed

Description

@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: v9.0.58-2
Reproducible in staging?: Y
Reproducible in production?: Y
Issue reported by: Applause Internal Team

Action Performed:

  1. Navigate to the Search page.
  2. Click on the search bar icon to open the search field.
  3. Enter any search query and press the Enter key to submit the query.

Expected Result:

The order of the words remains without changes when the query is submitted.

Actual Result:

The order of the words is reversed when the query is submitted.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6656086_1730861572949!51314-Web

Bug6656086_1730861572948.51314-Web.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021854577953623256419
  • Upwork Job ID: 1854577953623256419
  • Last Price Increase: 2024-11-21
  • Automatic offers:
    • Tony-MK | Contributor | 105099831
Issue OwnerCurrent Issue Owner: @garrettmknight

Activity

  1. added
    BugSomething is broken. Auto assigns a BugZero manager.
    on Nov 7, 2024
  2. melvin-bot commented on Nov 7, 2024

    @melvin-bot

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

  3. added
    ExternalAdded to denote the issue can be worked on by a contributor
    on Nov 7, 2024
  4. changed the title [-]Search-Order of the words is reversed when the query is submitted[/-] [+][$250] Search-Order of the words is reversed when the query is submitted[/+] on Nov 7, 2024
  5. melvin-bot commented on Nov 7, 2024

    @melvin-bot
  6. added
    Help WantedApply this label when an issue is open to proposals by contributors
    on Nov 7, 2024
  7. melvin-bot commented on Nov 7, 2024

    @melvin-bot

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

  8. Tony-MK commented on Nov 7, 2024

    @Tony-MK
    Contributor

    Edited by proposal-police: This proposal was edited at 2024-11-13 10:36:52 UTC.

    Proposal

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

    Search-Order of the words is reversed when the query is submitted.

    What is the root cause of that problem?

    The SearchPage uses the SearchQueryUtils.buildSearchQueryJSON function to build a query.

    const {q} = route.params;
    const queryJSON = useMemo(() => SearchQueryUtils.buildSearchQueryJSON(q), [q]);

    However, we end up sorting the query.flatFilters which are the keywords by alphabetical order when calling the getQueryHashes function.

    This is because a consistent hash function is needed.

    result.flatFilters = flatFilters;
    const {primaryHash, recentSearchHash} = getQueryHashes(result);
    result.hash = primaryHash;

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

    Instead of sorting the original query.flatFilters array inplace, we should create a new array where we sort the query.flatFilters.

    Also, instead of using forEach we can map the query.flatFilters to a function so the original array is not mutated and orderedQuery parsed.

    Optionally, I believe it is unnecessary to have two code snippets where we modify the query.flatFilters, seen below, to make orderedQuery so let's join the two code snippets into one.

    query.flatFilters.forEach((filter) => {
    filter.filters.sort((a, b) => localeCompare(a.value.toString(), b.value.toString()));
    });

    query.flatFilters
    .map((filter) => buildFilterValuesString(filter.key, filter.filters))
    .sort()
    .forEach((filterString) => (orderedQuery += ` ${filterString}`));

  9. nkdengineer commented on Nov 8, 2024

    @nkdengineer
    Contributor

    Proposal

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

    The order of the words is reversed when the query is submitted.

    What is the root cause of that problem?

    Here is the function that we use to get the hash query. We have two sort functions here

    function getQueryHashes(query: SearchQueryJSON): {primaryHash: number; recentSearchHash: number} {

    The first one is used to sort all values in a filters of a filter

    query.flatFilters.forEach((filter) => {
    filter.filters.sort((a, b) => localeCompare(a.value.toString(), b.value.toString()));
    });

    The second one is used to sort all filters by key

    query.flatFilters
    .map((filter) => buildFilterValuesString(filter.key, filter.filters))
    .sort()
    .forEach((filterString) => (orderedQuery += ` ${filterString}`));

    For the case of this issue Test and G are in the same filter with the key being keyword then it's sorted by the first function

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

    I think for the keyword key filter, we should keep the order of all values. We can skip the sort function here if the key is keyword

    query.flatFilters.forEach((filter) => {
        if (filter.key === 'keyword') {
            return;
        }
        filter.filters.sort((a, b) => localeCompare(a.value.toString(), b.value.toString()));
    });
    

    query.flatFilters.forEach((filter) => {
    filter.filters.sort((a, b) => localeCompare(a.value.toString(), b.value.toString()));
    });

    What alternative solutions did you explore? (Optional)

    If we want to keep the order of all filters and all values of the filter we can remove these lines and the sort function here

  10. melvin-bot commented on Nov 11, 2024

    @melvin-bot

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

  11. 45 remaining items

  12. garrettmknight commented on Dec 17, 2024

    @garrettmknight
    Contributor

    @rushatgabhane please complete the checklist when you get a chance before submitting for payment.

  13. garrettmknight commented on Dec 21, 2024

    @garrettmknight
    Contributor

    @rushatgabhane can you complete the checklist when you get a chance?

  14. garrettmknight commented on Dec 30, 2024

    @garrettmknight
    Contributor

    Still awaiting the checklist

  15. JmillsExpensify commented on Dec 30, 2024

    @JmillsExpensify
    Contributor

    Confirming the payment summary:

  16. garrettmknight commented on Jan 7, 2025

    @garrettmknight
    Contributor

    Still awaiting checklist @rushatgabhane

  17. rushatgabhane commented on Jan 9, 2025

    @rushatgabhane
    Member

    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/50883/files#r1908818356

    • [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: Not critical but we can add test for it.

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

    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:

    Regression Test Proposal

    Precondition:

    Test:

    1. Go to the search page.
    2. Click on the search bar.
    3. Type in random letters or words preferably in reverse alphabetical order. E.g.: dog cat bird apple.
    4. Press enter to search the keywords.
    5. Check if the order of the keywords in the search bar is unchanged and exactly as you typed in Step 3.

    Do we agree 👍 or 👎

  18. garrettmknight commented on Jan 10, 2025

    @garrettmknight
    Contributor

    $250 approved for @rushatgabhane

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionBugSomething is broken. Auto assigns a BugZero manager.ExternalAdded to denote the issue can be worked on by a contributorWeeklyKSv2

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions