Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$250] [HOLD for payment 2024-06-13] HIGH: [API Reliability] Add cancel function when you select an option in search and navigate away from the optionList #42384

Closed
6 tasks
muttmuure opened this issue May 20, 2024 · 20 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Internal Requires API changes or must be handled by Expensify staff

Comments

@muttmuure
Copy link
Contributor

muttmuure commented May 20, 2024

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


What performance issue do we need to solve?

e.g. memory consumption, storage read/write times, React native bridge concerns, inefficient React component rendering, etc.

Currently, when you use search in New Expensify, we show results progressively and put the "best result" at the top. However a query for the full optionList always finishes after you've started it, even if you navigate away from the page that is performing the command.

What is the impact of this on end-users?

List specific user experiences that will be improved by solving this problem e.g. app boot time, time to for some interaction to complete, etc.

When you have navigated to the page you actually want, that page is a little slow because there is an expensive operation still running in the background.

List any benchmarks that show the severity of the issue

Please also provide exact steps taken to collect metrics above if any so we can independently verify the results.

Proposed solution (if any)

Please list out the steps you think we should take to solve this issue.

Create a new "cancel" API command that ceases the operation of querying search when you navigate to the report you've selected from the optionList.

List any benchmarks after implementing the changes to show impacts of the proposed solution (if any)

Note: These should be the same as the benchmarks collected before any changes.

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Version Number:
Reproducible in staging?:
Reproducible in production?:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:
Issue reported by:
Slack conversation:

View all open jobs on Upwork

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0164d3f1e9ba4d7d59
  • Upwork Job ID: 1801734919193659319
  • Last Price Increase: 2024-06-14
Issue OwnerCurrent Issue Owner: @
Issue OwnerCurrent Issue Owner: @VictoriaExpensify
Copy link

melvin-bot bot commented May 20, 2024

Auto-assigning issues to engineers is no longer supported. If you think this issue should receive engineering attention, please raise it in #whatsnext.

@adhorodyski
Copy link
Contributor

Isn't the need more on cancelling the frontend request, like with an https://developer.mozilla.org/en-US/docs/Web/API/AbortController?

@muttmuure
Copy link
Contributor Author

Ah, yeah probably. Apologies as I didn't fully understand this one, you might be right

@muttmuure muttmuure changed the title HIGH: [API Reliability] Add cancel API command function when you select an option in search HIGH: [API Reliability] Add cancel function when you select an option in search and navigate away from the optionList May 20, 2024
@muttmuure
Copy link
Contributor Author

Would you be interested in taking it on?

@adhorodyski
Copy link
Contributor

sure, let's do it - you can assign this to me

@adhorodyski
Copy link
Contributor

adhorodyski commented May 21, 2024

@muttmuure I won't be available in the next few days, sharing my findings from today. I'm looping in the Callstack team to take over in our morning :)


Research

Each API.read command like searchForReports can be manually aborted when having a canCancel data parameter, but this relies on a single AbortController instance for all the requests. We could manually trigger cancellation here this way:

const selectReport = (option: OptionData) => {
        console.log('Navigating to a report', option);
        if (!option) {
            return;
        }

+      HttpUtils.cancelPendingRequests();

        if (option.reportID) {
            setSearchValue('');
            Navigation.dismissModal(option.reportID);
        } else {
            Report.navigateToAndOpenReport(option.login ? [option.login] : []);
        }
    };

...but it'd cancel everything that is currently running, potentially not only our Search queries (which we have multiple when typing even slightly slower).


Options

  1. Test the above solution on a build, it may be that it's totally safe to run a regular full cancellation there.
  2. Add per-query cancellation, which will require using a separate abort controller for this API command we can call .abort() on when selecting a report.

@rinej
Copy link
Contributor

rinej commented May 22, 2024

@muttmuure hello I am Tomek from Callstack, I will take it over from Adam

@muttmuure
Copy link
Contributor Author

Thanks!

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels May 24, 2024
@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jun 6, 2024
@melvin-bot melvin-bot bot changed the title HIGH: [API Reliability] Add cancel function when you select an option in search and navigate away from the optionList [HOLD for payment 2024-06-13] HIGH: [API Reliability] Add cancel function when you select an option in search and navigate away from the optionList Jun 6, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jun 6, 2024
Copy link

melvin-bot bot commented Jun 6, 2024

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

Copy link

melvin-bot bot commented Jun 6, 2024

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

If no regressions arise, payment will be issued on 2024-06-13. 🎊

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jun 12, 2024
Copy link

melvin-bot bot commented Jun 13, 2024

Issue is ready for payment but no BZ is assigned. @VictoriaExpensify you are the lucky winner! Please verify the payment summary looks correct and complete the checklist. Thanks!

Copy link

melvin-bot bot commented Jun 13, 2024

Payment Summary

Upwork Job

BugZero Checklist (@VictoriaExpensify)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@mallenexpensify mallenexpensify added the Internal Requires API changes or must be handled by Expensify staff label Jun 14, 2024
@melvin-bot melvin-bot bot added the Overdue label Jun 14, 2024
@mountiny mountiny added the External Added to denote the issue can be worked on by a contributor label Jun 14, 2024
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2024-06-13] HIGH: [API Reliability] Add cancel function when you select an option in search and navigate away from the optionList [$250] [HOLD for payment 2024-06-13] HIGH: [API Reliability] Add cancel function when you select an option in search and navigate away from the optionList Jun 14, 2024
Copy link

melvin-bot bot commented Jun 14, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 14, 2024
Copy link

melvin-bot bot commented Jun 14, 2024

Current assignee @shubham1206agra is eligible for the External assigner, not assigning anyone new.

@mountiny mountiny removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 14, 2024
@mountiny
Copy link
Contributor

$250 to @shubham1206agra for their review

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jun 14, 2024
Copy link

melvin-bot bot commented Jun 18, 2024

@rinej, @mountiny, @adhorodyski, @VictoriaExpensify, @shubham1206agra Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@kadiealexander
Copy link
Contributor

kadiealexander commented Jun 19, 2024

@shubham1206agra I have sent you an Upwork contract on behalf of @VictoriaExpensify, could you please let us know here when it's been accepted and we'll issue payment? Thanks!

@shubham1206agra
Copy link
Contributor

@kadiealexander Accepted

@VictoriaExpensify
Copy link
Contributor

Thanks @shubham1206agra ! I'm just having a few Upwork issues but I'll get this sorted and get you paid ASAP

@melvin-bot melvin-bot bot removed the Overdue label Jun 19, 2024
@kadiealexander
Copy link
Contributor

I've paid out @shubham1206agra while Vic sorts her access.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Internal Requires API changes or must be handled by Expensify staff
Projects
None yet
Development

No branches or pull requests

8 participants