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

fix(StopDetailsPage): Distinct errorKeys from API requests on NearbyTransitPage #641

Merged
merged 5 commits into from
Jan 14, 2025

Conversation

KaylaBrady
Copy link
Collaborator

@KaylaBrady KaylaBrady commented Jan 10, 2025

Summary

Ticket: 🤖 | Unfiltered Stop Details | Display error banners

What is this PR for?

Solves part 2 of #638.

getSchedule was always using the same errorKey, so a race condition where the nearby transit getSchedule request finishes after the stopDetails request resulted the stopDetails error banner disappearing.

Now, getSchedule and getGlobalData take an errorKey param so the error can be recorded in a context-dependent way.

Additionally, while debugging I noticed that checkPredictionsStale was not being called periodically. This adds that functionality.

Note:
I created a separate ticket to appropriately clear the error banner when navigating between nearby transit & stop details. This is an issue on iOS as well, and is therefore out of scope of this android parity task.

iOS

  • If you added any user-facing strings on iOS, are they included in Localizable.xcstrings?
    • Add temporary machine translations, marked "Needs Review"

android

  • All user-facing strings added to strings resource
  • Expensive calculations are run in withContext(Dispatchers.Default) where possible

Testing

What testing have you done?

  • Added unit tests
  • Ran locally with hardcoded error for getSchedule for place-pktrm. Confirmed error banner displayed as expected
  • Added logs for checkPredictionStale, confirmed it was called multiple times. Not exactly every 5 seconds, so something may be going on there that warrants further investigation, but it was being called on at least some interval.

@KaylaBrady KaylaBrady requested a review from a team as a code owner January 10, 2025 20:50
@KaylaBrady KaylaBrady requested a review from EmmaSimon January 10, 2025 20:50
@KaylaBrady KaylaBrady changed the title Kb stop details errors fix(StopDetailsPage): Distinct errorKeys from API requests on NearbyTransitPage Jan 10, 2025
@KaylaBrady KaylaBrady changed the base branch from main to kb-schedule-vm-only January 10, 2025 21:02
@KaylaBrady KaylaBrady force-pushed the kb-stop-details-errors branch from b437f8c to ec12599 Compare January 10, 2025 21:02
Base automatically changed from kb-schedule-vm-only to main January 14, 2025 14:31
@KaylaBrady KaylaBrady force-pushed the kb-stop-details-errors branch from db4e985 to 22d569a Compare January 14, 2025 14:35
@KaylaBrady KaylaBrady merged commit 0c9883c into main Jan 14, 2025
7 checks passed
@KaylaBrady KaylaBrady deleted the kb-stop-details-errors branch January 14, 2025 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants