Use new eatery backend #137
Merged
Merged
Conversation
…etter error messages for empty vs backend failure
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughConvert route fetching to a suspending call with cancellation-aware job management; restructure Eatery JSON model and previews; update networking (endpoint path, DateTime parsing, reduced timeouts); thread LazyListState per ecosystem category and adjust UI empty/error states and content mappings. Changes
Sequence Diagram(s)sequenceDiagram
participant VM as ViewModel
participant Repo as RouteRepository
participant API as NetworkApi
participant Backend as Backend
VM->>VM: cancel existing fetchRouteJob (if any)
VM->>Repo: launch new coroutine -> fetchRoute(params)
Repo->>Repo: set _lastRouteFlow = ApiResponse.Pending
Repo->>API: getRoute(RouteRequest)
API->>Backend: HTTP request ("/routes" or similar)
Backend-->>API: response
API-->>Repo: routeResponse
Repo-->>Repo: publish ApiResponse.Success(routeResponse.unwrap())
Note over Repo: CancellationException is rethrown to avoid setting Error for cancelled requests
Repo-->>VM: (observer/Flow) updated _lastRouteFlow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (5)
app/src/main/java/com/cornellappdev/transit/util/ContentConstants.kt (1)
16-25: Consider removing commented-out code after migration is stable.The commented-out dining room entries create code clutter. Once the new eatery backend is confirmed stable, these should be removed. If they need to be retained for reference, consider moving them to version control history or documentation instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/transit/util/ContentConstants.kt` around lines 16 - 25, Remove the commented-out dining room entries in ContentConstants.kt (the commented put(...) lines) to reduce clutter now that the new eatery backend is stable; if you want to keep them for reference, extract those lines into a docs/ or migration_notes.md file or rely on VCS history instead of leaving commented code in the ContentConstants object.app/src/main/java/com/cornellappdev/transit/models/ecosystem/Eatery.kt (2)
35-36: Nullable type with non-null default can cause inconsistent behavior.
List<String>? = emptyList()means the field can still becomenullif the JSON explicitly containsnull. Consumers must still handle bothnulland empty list cases. Consider either:
- Use
List<String> = emptyList()if null should always be treated as empty- Use
List<String>? = nulland let callers useorEmpty()consistently♻️ Suggested fix for consistency
- `@Json`(name = "paymentMethods") var paymentMethods: List<String>? = emptyList(), - `@Json`(name = "eateryTypes") var eateryTypes: List<String>? = emptyList(), + `@Json`(name = "paymentMethods") var paymentMethods: List<String> = emptyList(), + `@Json`(name = "eateryTypes") var eateryTypes: List<String> = emptyList(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/transit/models/ecosystem/Eatery.kt` around lines 35 - 36, The nullable properties paymentMethods and eateryTypes in Eatery.kt are declared as List<String>? = emptyList(), which allows JSON null to produce a real null and causes inconsistent handling; change their types to non-nullable with empty defaults by making them List<String> = emptyList() (update the declarations for paymentMethods and eateryTypes) and adjust any callers that relied on null to treat them as empty (or remove unnecessary null checks) so the model consistently provides an empty list instead of null.
74-76: Complex boolean condition is hard to read.The expression
dailyHours[dayOfWeek]?.none { it.contains(timeString) } != falseis non-intuitive. Consider extracting to a named variable or helper function for clarity.♻️ Suggested readability improvement
+ val isNewTimeSlot = dailyHours[dayOfWeek]?.none { it.contains(timeString) } ?: true - if (dayOfWeek != null && dailyHours[dayOfWeek]?.none { it.contains(timeString) } != false) { + if (dayOfWeek != null && isNewTimeSlot) { dailyHours.computeIfAbsent(dayOfWeek) { mutableListOf() }.add(timeString) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/transit/models/ecosystem/Eatery.kt` around lines 74 - 76, Extract the complex boolean into a named variable or small helper to improve readability: replace the inline condition in the if (dayOfWeek != null && dailyHours[dayOfWeek]?.none { it.contains(timeString) } != false) with either a local val (e.g., val isTimeAbsentForDay = dailyHours[dayOfWeek]?.none { it.contains(timeString) } != false && dayOfWeek != null) and then use if (isTimeAbsentForDay) { ... } or add a private helper in the Eatery class such as fun isTimeAbsent(dayOfWeek: String?, timeString: String): Boolean that encapsulates dailyHours[dayOfWeek]?.none { it.contains(timeString) } != false and null handling, then call if (isTimeAbsent(dayOfWeek, timeString)) { dailyHours.computeIfAbsent(dayOfWeek) { mutableListOf() }.add(timeString) }.app/src/main/java/com/cornellappdev/transit/networking/MoshiAdapters.kt (1)
38-48: Timezone handling mismatch between serialization and deserialization.
toJsonformats with a hardcoded'Z'suffix (implying UTC), butfromJsonconverts toLocalDateTimeusingZoneId.systemDefault(). This can cause time drift depending on device timezone. If the backend sends UTC timestamps, consider using a fixedZoneId.of("UTC")for consistency, or useZonedDateTime/OffsetDateTimeto preserve timezone info.Also, returning
LocalDateTime.MINon parse failure silently masks bad data. Consider logging a warning when falling back toLocalDateTime.MINso parsing issues are discoverable during development/debugging.♻️ Suggested improvement
`@FromJson` fun fromJson(dateTime: String): LocalDateTime { return try { val instant = Instant.parse(dateTime) - LocalDateTime.ofInstant(instant, ZoneId.systemDefault()) + LocalDateTime.ofInstant(instant, ZoneId.of("UTC")) } catch (_: Exception) { try { LocalDateTime.parse(dateTime) } catch (_: Exception) { + Log.w("DateTimeAdapter", "Failed to parse datetime: $dateTime") LocalDateTime.MIN } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/transit/networking/MoshiAdapters.kt` around lines 38 - 48, fromJson currently parses instants into LocalDateTime using ZoneId.systemDefault(), which mismatches the toJson that appends 'Z' (UTC) and can cause timezone drift; change fromJson in MoshiAdapters.kt to convert Instant.parse(dateTime) using ZoneId.of("UTC") (or parse into OffsetDateTime/ ZonedDateTime and convert explicitly) so serialization/deserialization use the same UTC reference, and add a warning log when falling back to LocalDateTime.MIN on parse failure (replace the silent catch that returns LocalDateTime.MIN in fromJson with a log call before returning) to make parse errors discoverable; reference functions: fromJson, toJson, and the LocalDateTime.MIN fallback.app/src/main/java/com/cornellappdev/transit/models/RouteRepository.kt (1)
193-195: Consider logging the swallowed exception.The caught exception is discarded, which can make debugging backend failures difficult. Consider adding logging before setting the error state.
📝 Proposed fix to add logging
- } catch (e: Exception) { - _lastRouteFlow.value = ApiResponse.Error + } catch (e: Exception) { + Log.e("RouteRepository", "Failed to fetch route", e) + _lastRouteFlow.value = ApiResponse.Error }You'll need to add
import android.util.Logat the top of the file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/transit/models/RouteRepository.kt` around lines 193 - 195, The catch block in RouteRepository (where _lastRouteFlow is set to ApiResponse.Error) currently swallows exceptions; update the catch in the function containing _lastRouteFlow (in RouteRepository.kt) to log the exception before setting _lastRouteFlow.value = ApiResponse.Error (use android.util.Log with a clear tag, e.g., "RouteRepository" and include e or e.message), and add the import android.util.Log at the top of the file so the exception details are recorded for debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/java/com/cornellappdev/transit/models/ecosystem/Eatery.kt`:
- Around line 35-36: The nullable properties paymentMethods and eateryTypes in
Eatery.kt are declared as List<String>? = emptyList(), which allows JSON null to
produce a real null and causes inconsistent handling; change their types to
non-nullable with empty defaults by making them List<String> = emptyList()
(update the declarations for paymentMethods and eateryTypes) and adjust any
callers that relied on null to treat them as empty (or remove unnecessary null
checks) so the model consistently provides an empty list instead of null.
- Around line 74-76: Extract the complex boolean into a named variable or small
helper to improve readability: replace the inline condition in the if (dayOfWeek
!= null && dailyHours[dayOfWeek]?.none { it.contains(timeString) } != false)
with either a local val (e.g., val isTimeAbsentForDay =
dailyHours[dayOfWeek]?.none { it.contains(timeString) } != false && dayOfWeek !=
null) and then use if (isTimeAbsentForDay) { ... } or add a private helper in
the Eatery class such as fun isTimeAbsent(dayOfWeek: String?, timeString:
String): Boolean that encapsulates dailyHours[dayOfWeek]?.none {
it.contains(timeString) } != false and null handling, then call if
(isTimeAbsent(dayOfWeek, timeString)) { dailyHours.computeIfAbsent(dayOfWeek) {
mutableListOf() }.add(timeString) }.
In `@app/src/main/java/com/cornellappdev/transit/models/RouteRepository.kt`:
- Around line 193-195: The catch block in RouteRepository (where _lastRouteFlow
is set to ApiResponse.Error) currently swallows exceptions; update the catch in
the function containing _lastRouteFlow (in RouteRepository.kt) to log the
exception before setting _lastRouteFlow.value = ApiResponse.Error (use
android.util.Log with a clear tag, e.g., "RouteRepository" and include e or
e.message), and add the import android.util.Log at the top of the file so the
exception details are recorded for debugging.
In `@app/src/main/java/com/cornellappdev/transit/networking/MoshiAdapters.kt`:
- Around line 38-48: fromJson currently parses instants into LocalDateTime using
ZoneId.systemDefault(), which mismatches the toJson that appends 'Z' (UTC) and
can cause timezone drift; change fromJson in MoshiAdapters.kt to convert
Instant.parse(dateTime) using ZoneId.of("UTC") (or parse into OffsetDateTime/
ZonedDateTime and convert explicitly) so serialization/deserialization use the
same UTC reference, and add a warning log when falling back to LocalDateTime.MIN
on parse failure (replace the silent catch that returns LocalDateTime.MIN in
fromJson with a log call before returning) to make parse errors discoverable;
reference functions: fromJson, toJson, and the LocalDateTime.MIN fallback.
In `@app/src/main/java/com/cornellappdev/transit/util/ContentConstants.kt`:
- Around line 16-25: Remove the commented-out dining room entries in
ContentConstants.kt (the commented put(...) lines) to reduce clutter now that
the new eatery backend is stable; if you want to keep them for reference,
extract those lines into a docs/ or migration_notes.md file or rely on VCS
history instead of leaving commented code in the ContentConstants object.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 52e53e7c-edbe-4585-a1b3-2a0370503156
📒 Files selected for processing (12)
app/src/main/java/com/cornellappdev/transit/models/RouteRepository.ktapp/src/main/java/com/cornellappdev/transit/models/ecosystem/Eatery.ktapp/src/main/java/com/cornellappdev/transit/networking/EateryNetworkApi.ktapp/src/main/java/com/cornellappdev/transit/networking/MoshiAdapters.ktapp/src/main/java/com/cornellappdev/transit/networking/NetworkModule.ktapp/src/main/java/com/cornellappdev/transit/ui/components/home/DetailedPlaceSheetContent.ktapp/src/main/java/com/cornellappdev/transit/ui/components/home/EateryDetailsContent.ktapp/src/main/java/com/cornellappdev/transit/ui/components/home/EcosystemBottomSheetContent.ktapp/src/main/java/com/cornellappdev/transit/ui/screens/HomeScreen.ktapp/src/main/java/com/cornellappdev/transit/ui/screens/RouteScreen.ktapp/src/main/java/com/cornellappdev/transit/ui/viewmodels/RouteViewModel.ktapp/src/main/java/com/cornellappdev/transit/util/ContentConstants.kt
There was a problem hiding this comment.
Pull request overview
Updates the Android client to work with the new eatery backend payload/endpoint and improves route UX by distinguishing backend failures from “no routes found,” while also reducing perceived latency by canceling stale route requests and shortening network timeouts.
Changes:
- Switch eatery API path to
/eateries/, updateEatery/Eventmodels + Moshi date parsing to match new backend fields. - Improve Route screen messaging for backend failures vs. legitimately empty route results, and cancel in-flight route requests when new ones start.
- Preserve bottom-sheet scroll per ecosystem filter tab and reduce OkHttp timeouts from 60s to 30s.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/java/com/cornellappdev/transit/util/ContentConstants.kt | Updates/curates hardcoded “About” mappings used as fallback content. |
| app/src/main/java/com/cornellappdev/transit/ui/viewmodels/RouteViewModel.kt | Cancels previous fetch job before launching a new route request. |
| app/src/main/java/com/cornellappdev/transit/ui/screens/RouteScreen.kt | Adds distinct empty-results UI vs backend error UI copy. |
| app/src/main/java/com/cornellappdev/transit/ui/screens/HomeScreen.kt | Maintains separate LazyListState per ecosystem filter and scrolls to top on filter change. |
| app/src/main/java/com/cornellappdev/transit/ui/components/home/EcosystemBottomSheetContent.kt | Threads LazyListState through to keep tab scroll position stable; updates previews. |
| app/src/main/java/com/cornellappdev/transit/ui/components/home/EateryDetailsContent.kt | Adds about-text fallback logic and adjusts header subtitle. |
| app/src/main/java/com/cornellappdev/transit/ui/components/home/DetailedPlaceSheetContent.kt | Updates previews to match new Eatery constructor fields. |
| app/src/main/java/com/cornellappdev/transit/networking/NetworkModule.kt | Reduces OkHttp read/connect timeouts to 30 seconds. |
| app/src/main/java/com/cornellappdev/transit/networking/MoshiAdapters.kt | Updates DateTimeAdapter to parse string timestamps. |
| app/src/main/java/com/cornellappdev/transit/networking/EateryNetworkApi.kt | Points eatery fetch to /eateries/. |
| app/src/main/java/com/cornellappdev/transit/models/ecosystem/Eatery.kt | Migrates eatery JSON fields to new schema; updates Event timestamp field names. |
| app/src/main/java/com/cornellappdev/transit/models/RouteRepository.kt | Makes fetchRoute suspend + cancellation-aware so stale requests can be canceled cleanly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…composed when accessed
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/src/main/java/com/cornellappdev/transit/models/ecosystem/Eatery.kt (2)
20-20: Consider using a typed model forannouncementsinstead ofList<Any>.Line 20 uses
List<Any>, which weakens type safety. While the field is currently unused in the codebase, defining a structuredAnnouncementtype would improve maintainability if this field is used in the future.Suggested approach
`@JsonClass`(generateAdapter = true) data class Announcement( `@Json`(name = "title") val title: String? = null, `@Json`(name = "body") val body: String? = null )Then update the field:
`@Json`(name = "announcements") var announcements: List<Announcement> = emptyList()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/transit/models/ecosystem/Eatery.kt` at line 20, The announcements field in Eatery (var announcements: List<Any>) is untyped; create a typed model Announcement (e.g., data class Announcement with nullable title and body and `@Json/`@JsonClass annotations) and change the Eatery property to var announcements: List<Announcement> = emptyList(); update imports and Moshi/adapter annotations as needed so serialization still works (refer to the Eatery class and the new Announcement type when making changes).
100-101: DateTimeAdapter already handles timezone conversion—consider Instant for explicitness, but not necessary for this use case.The
DateTimeAdaptercorrectly parses API timestamps asInstantbefore converting toLocalDateTimewithZoneId.systemDefault(). For operating hours display (the only usage of Event timestamps),LocalDateTimeis semantically appropriate since eatery hours are inherently local times independent of UTC offset. UsingInstantwould slightly clarify intent that the original data is UTC-based, but the current implementation functions correctly and this refactoring is optional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/transit/models/ecosystem/Eatery.kt` around lines 100 - 101, The current Event timestamp fields startTime and endTime in Eatery.kt can remain LocalDateTime because DateTimeAdapter already converts Instants to system-local LocalDateTime and eatery hours are local by nature; leave `@Json`(name = "startTimestamp") val startTime: LocalDateTime? and `@Json`(name = "endTimestamp") val endTime: LocalDateTime? unchanged. If you opt to make the UTC-ness explicit, change the types to Instant and update DateTimeAdapter and any usages (e.g., UI formatting and operating-hours logic) to convert Instant -> LocalDateTime at presentation time, ensuring code paths referencing startTime/endTime are adjusted to call Instant.atZone(ZoneId.systemDefault()).toLocalDateTime() where needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/java/com/cornellappdev/transit/models/ecosystem/Eatery.kt`:
- Line 20: The announcements field in Eatery (var announcements: List<Any>) is
untyped; create a typed model Announcement (e.g., data class Announcement with
nullable title and body and `@Json/`@JsonClass annotations) and change the Eatery
property to var announcements: List<Announcement> = emptyList(); update imports
and Moshi/adapter annotations as needed so serialization still works (refer to
the Eatery class and the new Announcement type when making changes).
- Around line 100-101: The current Event timestamp fields startTime and endTime
in Eatery.kt can remain LocalDateTime because DateTimeAdapter already converts
Instants to system-local LocalDateTime and eatery hours are local by nature;
leave `@Json`(name = "startTimestamp") val startTime: LocalDateTime? and
`@Json`(name = "endTimestamp") val endTime: LocalDateTime? unchanged. If you opt
to make the UTC-ness explicit, change the types to Instant and update
DateTimeAdapter and any usages (e.g., UI formatting and operating-hours logic)
to convert Instant -> LocalDateTime at presentation time, ensuring code paths
referencing startTime/endTime are adjusted to call
Instant.atZone(ZoneId.systemDefault()).toLocalDateTime() where needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 81f23ef5-abea-44fc-b5b1-1f6251435344
📒 Files selected for processing (3)
app/src/main/java/com/cornellappdev/transit/models/ecosystem/Eatery.ktapp/src/main/java/com/cornellappdev/transit/ui/components/home/GymDetailsContent.ktapp/src/main/java/com/cornellappdev/transit/util/ContentConstants.kt
✅ Files skipped from review due to trivial changes (1)
- app/src/main/java/com/cornellappdev/transit/util/ContentConstants.kt
| package com.cornellappdev.transit.util | ||
|
|
||
| private val ABOUT_CONTENT = mapOf( | ||
| "Terrace Restaurant" to "The Terrace often features five to six made-to-order options, such as burritos, pho, gyros, and more throughout the day.", |
Member
There was a problem hiding this comment.
Nit: theoretically, these long string sentences should be in some sort of resource string xml but it should be fine
AndrewCheung360
approved these changes
Mar 22, 2026
Member
AndrewCheung360
left a comment
AndrewCheung360
left a comment
There was a problem hiding this comment.
LGTM as long as you tested on your end
This was referenced Apr 23, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Changes Made
Test Coverage
Summary by CodeRabbit
New Features
Bug Fixes
Refactor