Deep-Linking (PARTIAL)#145
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds deep-link launch UI for eatery and gym details with Play Store fallback and package visibility declarations; refactors HomeViewModel to pass explicit user location into distance sorting and introduces IntentUtils.openDeepLink for launching external apps. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Sheet as DetailedPlaceSheetContent
participant Detail as Eatery/GymDetailsContent
participant Intent as IntentUtils
participant PM as PackageManager
participant External as External App
participant Store as Play Store
User->>Sheet: Open place details
Sheet->>Detail: Render with onDeepLinkClick callback
User->>Detail: Tap "view menu"/"view gym"
Detail->>Intent: openDeepLink(packageName)
Intent->>PM: getLaunchIntentForPackage()
alt App installed
PM-->>Intent: launch intent
Intent->>External: startActivity(launchIntent)
External-->>User: app opens
else App not installed
PM-->>Intent: null
Intent->>Store: ACTION_VIEW market://details?id=...
alt Market app handles intent
Store-->>User: Play Store opens
else
Intent->>Store: ACTION_VIEW https://play.google.com/...
Store-->>User: Browser opens Play Store page
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt (2)
127-129: Stray blank lines.Lines 128–129 are empty leftovers from the refactor; consider removing them.
🤖 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/ui/viewmodels/HomeViewModel.kt` around lines 127 - 129, The stray blank lines after the userLocation assignment (val userLocation = location?.let { LatLng(it.latitude, it.longitude) }) in HomeViewModel.kt should be removed to clean up the refactor; locate the declaration of userLocation in the HomeViewModel class/constructor and delete the extra empty lines so the surrounding code is compact and properly formatted.
56-78: Sort recomputes on every location emission.
libraryCardsFlow(andstaticPlacesFlow) now re-sort the entire list every timecurrentLocationemits, which can be quite frequent for an active GPS feed. Consider applying.distinctUntilChanged()on the location source (or a coarse-grained version of it) before combining, so the work only happens when distance ordering can meaningfully change.🤖 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/ui/viewmodels/HomeViewModel.kt` around lines 56 - 78, The combine using routeRepository.libraryFlow and locationRepository.currentLocation currently re-sorts on every location emission (causing excessive work); fix it by debouncing/coarsening the location stream before combining—apply distinctUntilChanged (or map the LatLng to a coarse-grained bucket/rounded coordinates) to locationRepository.currentLocation and use that new flow in the combine that produces libraryCardsFlow/staticPlacesFlow so numericalDistanceToPlace and the sorting only run when location meaningfully changes; update references in the combine block that calls numericalDistanceToPlace and toLibraryCardUiState accordingly.app/src/main/java/com/cornellappdev/transit/ui/components/home/DetailedPlaceSheetContent.kt (1)
105-134: Extract hardcoded package names to constants.
"com.cornellappdev.android.eatery"and"com.cornellappdev.uplift"are duplicated as string literals here and must stay in sync with the<queries>entries inAndroidManifest.xml. Consider exposing them asconst val(e.g., inIntentUtils) so there is a single source of truth.♻️ Suggested change
+// In IntentUtils.kt +object IntentUtils { + const val EATERY_PACKAGE = "com.cornellappdev.android.eatery" + const val UPLIFT_PACKAGE = "com.cornellappdev.uplift" + ... +}- context.openDeepLink("com.cornellappdev.android.eatery") + context.openDeepLink(IntentUtils.EATERY_PACKAGE) ... - context.openDeepLink("com.cornellappdev.uplift") + context.openDeepLink(IntentUtils.UPLIFT_PACKAGE)🤖 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/ui/components/home/DetailedPlaceSheetContent.kt` around lines 105 - 134, Two hardcoded deep-link package name string literals ("com.cornellappdev.android.eatery" and "com.cornellappdev.uplift") are duplicated; extract them into single const vals (e.g., add const val EATERY_PACKAGE and UPLIFT_PACKAGE in an existing IntentUtils or similar utility object) and replace the literals used in context.openDeepLink calls in DetailedPlaceSheetContent (references: context.openDeepLink, LibraryDetailsContent/GymDetailsContent invocation sites) to use those constants so the package names remain a single source of truth and stay in sync with AndroidManifest <queries>.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/cornellappdev/transit/util/IntentUtils.kt`:
- Line 20: The Play Store fallback URL contains a stray closing brace after the
package name in the startActivity(Intent(Intent.ACTION_VIEW, "...".toUri()))
call; remove the extra '}' so the interpolated string is
"https://play.google.com/store/apps/details?id=$packageName" (i.e., change the
URI passed to Intent.ACTION_VIEW to use id=$packageName without the trailing
brace) to restore a valid Play Store URL.
- Around line 14-23: The else-branch in IntentUtils.kt can crash and swallows
exceptions: wrap the outer fallback startActivity call in a try/catch for
ActivityNotFoundException, log the caught exception (instead of ignoring it),
and ensure both Intents (the market Intent created in this block and the HTTPS
fallback Intent) have FLAG_ACTIVITY_NEW_TASK added so the helper is safe to call
with an Application Context; update the catch to log via your logger/context
(e.g., processLogger or Log) and avoid rethrowing so failures are diagnosable
and the app won’t crash when no resolver exists.
---
Nitpick comments:
In
`@app/src/main/java/com/cornellappdev/transit/ui/components/home/DetailedPlaceSheetContent.kt`:
- Around line 105-134: Two hardcoded deep-link package name string literals
("com.cornellappdev.android.eatery" and "com.cornellappdev.uplift") are
duplicated; extract them into single const vals (e.g., add const val
EATERY_PACKAGE and UPLIFT_PACKAGE in an existing IntentUtils or similar utility
object) and replace the literals used in context.openDeepLink calls in
DetailedPlaceSheetContent (references: context.openDeepLink,
LibraryDetailsContent/GymDetailsContent invocation sites) to use those constants
so the package names remain a single source of truth and stay in sync with
AndroidManifest <queries>.
In `@app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt`:
- Around line 127-129: The stray blank lines after the userLocation assignment
(val userLocation = location?.let { LatLng(it.latitude, it.longitude) }) in
HomeViewModel.kt should be removed to clean up the refactor; locate the
declaration of userLocation in the HomeViewModel class/constructor and delete
the extra empty lines so the surrounding code is compact and properly formatted.
- Around line 56-78: The combine using routeRepository.libraryFlow and
locationRepository.currentLocation currently re-sorts on every location emission
(causing excessive work); fix it by debouncing/coarsening the location stream
before combining—apply distinctUntilChanged (or map the LatLng to a
coarse-grained bucket/rounded coordinates) to locationRepository.currentLocation
and use that new flow in the combine that produces
libraryCardsFlow/staticPlacesFlow so numericalDistanceToPlace and the sorting
only run when location meaningfully changes; update references in the combine
block that calls numericalDistanceToPlace and toLibraryCardUiState accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 35269a18-7493-41e5-982a-237693823bcb
📒 Files selected for processing (7)
app/src/main/AndroidManifest.xmlapp/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/GymDetailsContent.ktapp/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.ktapp/src/main/java/com/cornellappdev/transit/util/IntentUtils.ktuplift-android
| style = Style.heading2, | ||
| color = TransitBlue | ||
| color = TransitBlue, | ||
| modifier = Modifier.clickable( |
There was a problem hiding this comment.
Can make this a TextButton instead of clickable text
| style = Style.heading2, | ||
| color = TransitBlue | ||
| color = TransitBlue, | ||
| modifier = Modifier.clickable( |
There was a problem hiding this comment.
Can make this a TextButton instead of clickable text
|
What is the uplift-android folder for? |
Sorry, one of my git commands caused a weird error and I didn't know why or what happened. I'm pretty sure that's what it was because it was talking about some uplift-anroid thing. Will delete |
| fun numericalDistanceToPlace(latitude: Double?, longitude: Double?): Double { | ||
| val currentLocationSnapshot = currentLocation.value | ||
| return if (currentLocationSnapshot != null && latitude != null && longitude != null) { | ||
| fun numericalDistanceToPlace( |
There was a problem hiding this comment.
Should probably be private if only used in this vm. Can also potentially be moved into one of the util files to keep this vm leaner
Merge branch 'main' into abby/deep-linking
Overview
Added Linking Between Navi & Eatery/Uplift
Changes Made
Test Coverage
Next Steps (delete if not applicable)
Related PRs or Issues (delete if not applicable)
Screenshots (delete if not applicable)
Clicking On Eatery Link (Eatery Installed on Phone)
Screen_recording_20260426_202257.webm
Clicking On Uplift Link (Uplift Installed on Phone)
Screen_recording_20260426_202354.webm
Clicking On Eatery Link (Eatery NOT Installed on Phone)
Screen_recording_20260426_202556.webm
Clicking On Uplit Link (Uplift NOT Installed on Phone)
Screen_recording_20260426_202710.webm
Summary by CodeRabbit
New Features
Improvements
Chores