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] mWeb - Create expense from distansce "use current location" get a big delay about 20s #55137

Closed
1 of 8 tasks
IuliiaHerets opened this issue Jan 12, 2025 · 22 comments
Closed
1 of 8 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Jan 12, 2025

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: v9.0.84-1
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5442592&group_by=cases:section_id&group_order=asc&group_id=296081
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team
Device used: Pixel 7 Pro /Android 15/Chrome
App Component: Other

Action Performed:

  1. Open the URL https://staging.new.expensify.com/
  2. Log in with Gmail account
  3. Click on the FAB button > Submit expense
  4. Start a distance expense
  5. Select "use current location"

Expected Result:

The app should let the user use their current location right away

Actual Result:

When create a distance expense, there is a big delay of about 20 seconds before the app can load the current location

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6712233_1736654452821.BigDelay.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021879307964726263223
  • Upwork Job ID: 1879307964726263223
  • Last Price Increase: 2025-02-04
Issue OwnerCurrent Issue Owner: @
@IuliiaHerets IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jan 12, 2025
Copy link

melvin-bot bot commented Jan 12, 2025

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

@truph01
Copy link
Contributor

truph01 commented Jan 13, 2025

Proposal

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

  • When create a distance expense, there is a big delay of about 20 seconds before the app can load the current location

What is the root cause of that problem?

  • When the user presses "Use current location," the function at this line is called:

setIsFetchingCurrentLocation(true);

This updates the isFetchingCurrentLocation state to true, which triggers the display of a loading indicator::

{isFetchingCurrentLocation && <FullScreenLoadingIndicator />}

  • Once the getCurrentPosition() function completes—whether it succeeds or fails—the isFetchingCurrentLocation state is set to false, causing the loading indicator to disappear. This ensures the user interface reflects the completion of the location fetch process..

  • In this issue, it is observed that navigator.geolocation.getCurrentPosition() can sometimes take an unusually long time to resolve, as reported by the OP, it may take over 20 seconds to resolve in some cases, leading to an extended display of the loading indicator. This creates a poor user experience, as users may perceive the application as unresponsive.

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

  • From the user's perspective, even if the current location is eventually retrieved after 20 seconds, no one is likely to wait that long—they will perceive it as a bug. To address this, we should implement a timeout mechanism for getCurrentPosition() to ensure the process does not exceed a reasonable time limit (e.g., 10 seconds). If the timeout is reached, a fallback behavior, such as displaying an error message or providing an option for manual location entry, should be triggered.

  • We already have this logic in place, but the timeout value is set too high (30,000 ms). We need to update it to a more appropriate value, such as 10,000 ms (can be decided later):

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

What alternative solutions did you explore? (Optional)

@mallenexpensify mallenexpensify added the Needs Reproduction Reproducible steps needed label Jan 13, 2025
@MelvinBot
Copy link

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

@mallenexpensify
Copy link
Contributor

Unable to reproduce on Android with new gmail account.
@truph01 are you able to reproduce? If so, can you share a vid? Thx

@truph01
Copy link
Contributor

truph01 commented Jan 14, 2025

@mallenexpensify The ability to reproduce this bug depends on the specific location and environment, we already mentioned this in here: QA probably won't be able to simulate a poor GPS connection. While I couldn't replicate the ~20-second delay mentioned by the OP, I was able to observe a ~10-second delay from my side, which is still a significant delay time:

Screen.Recording.2025-01-14.at.09.31.56.mov

@dominictb
Copy link
Contributor

dominictb commented Jan 14, 2025

@mallenexpensify I can reproduce this issue with ~25 seconds delay after clearing browser's caches for Expensify site:

video_2025-01-14_14-20-54.mp4

I can take over this issue.

@mallenexpensify mallenexpensify added the External Added to denote the issue can be worked on by a contributor label Jan 14, 2025
@melvin-bot melvin-bot bot changed the title mWeb - Create expense from distansce "use current location" get a big delay about 20s [$250] mWeb - Create expense from distansce "use current location" get a big delay about 20s Jan 14, 2025
Copy link

melvin-bot bot commented Jan 14, 2025

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

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

melvin-bot bot commented Jan 14, 2025

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

@mallenexpensify
Copy link
Contributor

Thx @truph01 and @dominictb .
Assigned @dominictb as C+, please review proposals above.

@dominictb
Copy link
Contributor

@truph01 Set the timeout to another value does not sound good to me. Can you research the @react-native-community/geolocation library to see if there're any other ways to reduce the GPS query time?

@truph01
Copy link
Contributor

truph01 commented Jan 17, 2025

@dominictb

Can you research the @react-native-community/geolocation library to see if there're any other ways to reduce the GPS query time?

On the web, we use the getCurrentPosition method to retrieve the current location:

navigator.geolocation.getCurrentPosition(success, error, options);

The documentation outlines three options that can help optimize response time:

  • maximumAge: Must be set to 0 as per the comment here:

maximumAge: 0, // No cache, always get fresh location info

From the analysis above, from my side, the only actionable optimization is to reduce the timeout value. What do you think?

@melvin-bot melvin-bot bot added the Overdue label Jan 20, 2025
Copy link

melvin-bot bot commented Jan 21, 2025

@mallenexpensify, @dominictb Huh... This is 4 days overdue. Who can take care of this?

@dominictb
Copy link
Contributor

@truph01 Hmm I'm still not super satisfied with that. Let's investigate further to make sure there's no other ways at all.

@melvin-bot melvin-bot bot removed the Overdue label Jan 21, 2025
Copy link

melvin-bot bot commented Jan 21, 2025

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

@melvin-bot melvin-bot bot added the Overdue label Jan 23, 2025
Copy link

melvin-bot bot commented Jan 26, 2025

@mallenexpensify @dominictb this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

Copy link

melvin-bot bot commented Jan 27, 2025

@mallenexpensify, @dominictb Huh... This is 4 days overdue. Who can take care of this?

Copy link

melvin-bot bot commented Jan 28, 2025

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

@mallenexpensify
Copy link
Contributor

Thinking more about this, I'm leaning towards closing this unless we have more/better data on the bug

simulate a poor GPS connection.

ie. if I have NO GPS connection then I'll never get current location. If I have good-enough GPS, I'll get it soon-enough.
If we try to fix bugs within the margins of never and good-enough, it's a slippery slope of what the expected behaviour really is.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jan 29, 2025
Copy link

melvin-bot bot commented Feb 3, 2025

@mallenexpensify, @dominictb Huh... This is 4 days overdue. Who can take care of this?

Copy link

melvin-bot bot commented Feb 4, 2025

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

@mallenexpensify
Copy link
Contributor

Closing for now. Feel free to add comments if you disagree or have other thoughts/ideas, I'll get a notification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
Status: Done
Development

No branches or pull requests

6 participants