Restore trip-change notification tap into the directions focus (#1939)#1940
Merged
Merged
Conversation
PR #1872 embedded directions into the home map and retired the standalone TripPlanScreen, whose maybeRestoreFromIntent was the only consumer of the trip-plan monitor's "your trip changed" notification intent. With that gone, tapping the alert opened generic HOME instead of the changed trip: the intent still carried the request bundle (copyIntoBundleSimple), the updated itineraries (OTPConstants.ITINERARIES, JSON), and INTENT_SOURCE=NOTIFICATION, but nothing in HOME read them. Re-add the consumer in HomeActivity's launch-intent side-effect path, driving the Activity-scoped VMs instead of the retired screen's: guard on INTENT_SOURCE=NOTIFICATION, JSON-decode the itineraries, rehydrate the request via TripRequestBuilder.initFromBundleSimple, then enterDirections() + TripPlanViewModel.restoreFrom(...) to inject the precomputed itineraries as PlanResult.Success without re-planning. The existing directions render path (results sheet + map draw) reacts with no further changes; the producer side and restoreFrom were already intact. No re-restore guard is needed: the launch-intent channel submits each real intent exactly once (cold onCreate seed, warm onNewIntent), so a config change doesn't re-fire it. params=null (monitor not re-armed from the restore) matches pre-#1872 behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesNotification trip restoration
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Notification
participant HomeActivity
participant TripRequestBuilder
participant TripPlanViewModel
Notification->>HomeActivity: Launch with request and itinerary extras
HomeActivity->>TripRequestBuilder: Rebuild trip request
HomeActivity->>HomeActivity: Decode updated itineraries
HomeActivity->>TripPlanViewModel: Restore directions state
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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 & why
Fixes #1939.
PR #1872 embedded directions into the home map and retired the standalone
TripPlanScreen, whosemaybeRestoreFromIntentwas the only consumer of the trip-plan monitor's "your trip changed" notification intent. With that screen gone, tapping the alert opened generic HOME instead of the changed trip — a regression relative to pre-#1872 behavior.The producer side was left intact and staged for this follow-up:
TripPlanMonitorService.notifyChangestill builds aHomeActivityintent carrying the simplified request bundle (copyIntoBundleSimple), the updated itineraries (OTPConstants.ITINERARIES, JSON), andOTPConstants.INTENT_SOURCE = Source.NOTIFICATION. Nothing in HOME read those extras. This PR re-adds the missing consumer.Change
One file —
HomeActivity.kt. A newmaybeRestoreDirectionsFromIntent(intent)in the launch-intent side-effect path (applyLaunchIntentSideEffects, which runs once per real intent, cold and warm, lifecycle-gated to STARTED):INTENT_SOURCE == Source.NOTIFICATION(viaIntentCompat.getSerializableExtra, avoiding the deprecatedgetSerializableso the strict warnings-as-errors gate stays green).ITINERARIESwithString.toTripItineraries(); bails if empty.TripRequestBuilder.initFromBundleSimple.homeViewModel.enterDirections()thentripPlanViewModel.restoreFrom(...), injecting the precomputed itineraries asPlanResult.Success— no re-plan.The existing directions render path (directions results sheet → map draw) reacts to the
Directionsfocus +PlanResult.Successwith no further changes.restoreFromwas already present but had no callers; this wires it up. Mirrors the deletedmaybeRestoreFromIntent, and follows the same two-call shape as the recent long-press "directions from/to here" feature (#1938).Notes
launchIntentschannel submits each real intent exactly once (coldonCreateseed gated on a nullsavedInstanceState, warmonNewIntent), so a config change doesn't re-fire the restore; the activity-scopedTripPlanViewModelkeeps the results across rotation.params = null(trip-update monitor not re-armed from the restore) matches pre-Trip planner: map-overlay layout (top-sheet form + bottom-sheet directions) #1872 behavior — out of scope here.@Suppress("UnwrappedClockValue")+ rationale for the "default to device now when the intent carries no time" fallback.Verification
./gradlew :onebusaway-android:compileObaGoogleDebugKotlin -PwarningsAsErrors=true.INTENT_SOURCE+ decodableITINERARIESgate).🤖 Generated with Claude Code
Summary by CodeRabbit