fix(nearby): filter stop sheet routes to the tapped mode (shared stop_id) - #176
Closed
ai-tiro wants to merge 1 commit into
Closed
fix(nearby): filter stop sheet routes to the tapped mode (shared stop_id)#176ai-tiro wants to merge 1 commit into
ai-tiro wants to merge 1 commit into
Conversation
…_id)
PTV's /stops/{id}/route_type/{type} endpoint ignores the route_type path
param for the `routes` array and returns every route serving the physical
stop_id. At co-located stops sharing a stop_id (Richmond, stop_id 1162: the
metro platforms and the V/Line platform are one stop) this mixed modes, so
the Nearby sheet's Routes chips on the V/Line pin showed metro train lines
(Alamein, Belgrave, ...). Departures are fetched per (stopId, routeType) and
were already correct, which is why only the Routes chips were wrong.
Filter servingRoutes to the requested route_type in StopResponseDto.toDomain
so every getStopDetail consumer gets routes consistent with the mode it asked
for. Both current consumers (Nearby sheet, stop-detail departure-grouping
lookup) want the mode-consistent list.
Co-Authored-By: ai-tiro <ai-tiro@jfx.ac>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Debug APK: app-debug-07a088bf62233f058bbc5cda1d00cb43e736131c.apk (built from Requires GitHub login. Artifact expires after 3 days. |
Collaborator
Author
|
Superseded by #179, which combined these commits and has merged to master. Closing as superseded. |
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.
What
Filter
StopDetail.servingRoutesto the requestedroute_typeso the Nearby map's stop bottom-sheet only shows Routes chips for the mode of the tapped pin.What I discovered
At co-located stops, the metro platforms and the V/Line platform share one
stop_id(Richmond =stop_id1162). PTV's/stops/{id}/route_type/{type}endpoint ignores theroute_typepath param for theroutesarray and returns every route serving that physical stop. Confirmed against the live proxy:So even though we requested
route_type/3(V/Line), metro (route_type 0) routes come back mixed in. The Nearby sheet rendered them as the Routes chips (NearbyViewModel.startSheetFetches→routes = result.data.servingRoutes). Departures are fetched per(stopId, routeType)and were already correct, which is exactly why only the Routes chips were wrong.The fix
Added a
requestedRouteTypeparameter toStopResponseDto.toDomain()(:core:networkmapper) and filteredservingRoutesto it.RetrofitStopDetailDataSourcealready knows the requestedrouteTypeand now passes it through.I fixed it at the mapper/data boundary rather than locally in
NearbyViewModelso everygetStopDetailconsumer gets a routes list consistent with the mode it asked for. I grepped allservingRoutesusages:toGroupedList— usesservingRoutesonly as arouteId -> Routelookup map against departures (which are already mode-specific) and synthesises a placeholderRoutefor any id the map misses, so a mode-filtered list is strictly correct for it too. (The stop-detail screen's own per-stop serving-routes section was already removed in cffc08e, so nothing renders the raw cross-mode list.)No surface intentionally wants the cross-mode list, so the shared-layer fix is safe and is the cleaner SSOT choice.
RouteType.Unknowndisables the filter (we never put Unknown on the wire; this only guards an unexpected upstream value, where passing the raw list through beats dropping every route).Alternative rejected
A localised filter inside
NearbyViewModel.startSheetFetches. Rejected because it leaves the same latent bug for any futuregetStopDetailconsumer and pushes a data-correctness concern up into the UI layer.Overlap with open PRs
None. Changed files are all in
:core:network; neither #173 (fix/171-route-name-overflow) nor #174 (fix/172-colocated-marker-offset) touch them. Base ismaster.Testing
StopDetailDtoMapperTest(:core:network):serving routes are filtered to the requested route type at a shared stop_id— Richmond-shaped mixed-mode response, request V/Line, assert only the two V/Line routes remain.requesting metro at a shared stop_id keeps only metro routes— the mirror case.requestedRouteType(matching their route data, so behaviour is unchanged)../gradlew :core:network:testDebugUnitTest :core:data:testDebugUnitTest :feature:nearby:testDebugUnitTest— all green../gradlew :app:assembleDebug— green.Fixes #175