Skip to content

Address internal testing issues#139

Merged
RyanCheung555 merged 5 commits intomainfrom
ryan/bug-fixing
Apr 15, 2026
Merged

Address internal testing issues#139
RyanCheung555 merged 5 commits intomainfrom
ryan/bug-fixing

Conversation

@RyanCheung555
Copy link
Copy Markdown
Contributor

@RyanCheung555 RyanCheung555 commented Apr 15, 2026

Overview

  • Update compiler versions to meet new requirements
  • Fix small bugs/Add small UX enhancements

Changes Made

  • Force portrait only for better UX
  • Hide stale route requests
  • Update calculating distance placeholder to not be text-heavy
  • Fixed search bar UI on different sized devices
  • Add functionality to marker pins (takes user to route)

Test Coverage

  • Medium and Small phone to test different experiences

Screenshots (delete if not applicable)

Marker Functionality Demo
marker_demo.webm

Summary by CodeRabbit

  • New Features

    • Map markers are now clickable to start route planning.
  • Bug Fixes

    • Improved distance placeholder text.
    • Search bar horizontal spacing adjusted for cleaner layout.
    • Empty/cleared and idle states now show explicit messages (e.g., “No … available”) and avoid spinners when nothing is loading.
  • Chores

    • Updated Android SDK target to 36 and bumped app version to 2.0.
  • Behavior

    • App orientation locked to portrait.

@RyanCheung555 RyanCheung555 changed the title Addressing internal testing issues Address internal testing issues Apr 15, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 15, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6179ce8a-40dc-401c-b2b7-3aa4aa41c9b7

📥 Commits

Reviewing files that changed from the base of the PR and between 0aed98c and c387782.

📒 Files selected for processing (7)
  • app/src/main/java/com/cornellappdev/transit/models/RouteRepository.kt
  • app/src/main/java/com/cornellappdev/transit/networking/ApiResponse.kt
  • app/src/main/java/com/cornellappdev/transit/ui/components/LoadingLocationItems.kt
  • app/src/main/java/com/cornellappdev/transit/ui/components/home/EcosystemBottomSheetContent.kt
  • app/src/main/java/com/cornellappdev/transit/ui/screens/HomeScreen.kt
  • app/src/main/java/com/cornellappdev/transit/ui/screens/RouteScreen.kt
  • app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (2)
  • app/src/main/java/com/cornellappdev/transit/models/RouteRepository.kt
  • app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt

📝 Walkthrough

Walkthrough

Raised Android/Gradle toolchain and app versions; added an idle ApiResponse state and UI handling for it; added RouteRepository.clearLastRoute and made RouteViewModel explicitly clear/cancel when coordinates are missing; wired map marker clicks to a new onPlaceClick → beginRouteOptions → navigation flow; minor UI padding and manifest orientation/property changes.

Changes

Cohort / File(s) Summary
Build & Versioning
app/build.gradle.kts, build.gradle.kts, gradle/wrapper/gradle-wrapper.properties
Bumped compileSdk/targetSdk to 36; versionCode 9→10 and versionName 1.0→2.0; enabled ECOSYSTEM_FLAG for build variants; AGP 8.1.4→8.13.2; Gradle wrapper 8.9→8.13.
Manifest Configuration
app/src/main/AndroidManifest.xml
Set android:screenOrientation="portrait" on MainActivity; added android.window.PROPERTY_COMPAT_ALLOW_RESTRICTED_RESIZABILITY application property.
API Response & State Handling
app/src/main/java/com/cornellappdev/transit/networking/ApiResponse.kt, app/src/main/java/com/cornellappdev/transit/ui/components/LoadingLocationItems.kt, app/src/main/java/com/cornellappdev/transit/ui/components/home/EcosystemBottomSheetContent.kt, app/src/main/java/com/cornellappdev/transit/ui/screens/RouteScreen.kt
Added ApiResponse.Idle and updated several UI components and lists to handle the idle/cleared state (render informational items or no-ops where appropriate).
Route logic
app/src/main/java/com/cornellappdev/transit/models/RouteRepository.kt, app/src/main/java/com/cornellappdev/transit/ui/viewmodels/RouteViewModel.kt
Added RouteRepository.clearLastRoute(); RouteViewModel.getLatestOptions() now treats start/end as nullable, cancels in-flight job and clears last route when coordinates are missing, otherwise proceeds to fetch route.
Home UI / Interaction
app/src/main/java/com/cornellappdev/transit/ui/components/home/AddFavoritesSearchSheet.kt, app/src/main/java/com/cornellappdev/transit/ui/components/home/HomeScreenMarkers.kt, app/src/main/java/com/cornellappdev/transit/ui/screens/HomeScreen.kt, app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt
Added horizontal padding (16.dp) to search bars and exposed modifier param for HomeScreenSearchBar. HomeScreenMarkers now accepts onPlaceClick: (Place) -> Unit and wires marker taps to that callback; HomeScreen uses it to call homeViewModel.beginRouteOptions() and navigate. Updated distance placeholder text.

Sequence Diagram(s)

sequenceDiagram
    participant Marker as "Map Marker"
    participant Home as "HomeScreen / UI"
    participant HomeVM as "HomeViewModel"
    participant Nav as "NavController"
    participant RouteVM as "RouteViewModel"
    participant Repo as "RouteRepository"
    Marker->>Home: user taps marker
    Home->>HomeVM: onPlaceClick(place)
    HomeVM->>RouteVM: beginRouteOptions(place)
    HomeVM->>Nav: navigateSingleTop(\"route\")
    RouteVM->>RouteVM: compute start/end coords (nullable)
    alt coords missing
        RouteVM->>RouteVM: cancel fetchRouteJob
        RouteVM->>Repo: clearLastRoute()
        RouteVM-->>HomeVM: return (no fetch)
    else coords present
        RouteVM->>Repo: getRoute(start,end,arriveBy)
        Repo-->>RouteVM: route response (Pending/Success/Error)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • AndrewCheung360

Poem

🐰
Gradle hops up, SDKs in tow,
Markers point where routes may go,
Idle states now rest in peace,
Cleared routes let new journeys cease,
Padded search and portrait glow.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title is vague and generic, using a non-descriptive phrase that doesn't convey specific information about the changeset's main improvements. Consider a more specific title such as 'Update compiler versions and fix UX issues' or 'Upgrade build config and enhance marker interactivity' to better reflect the primary changes.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description covers most required sections with relevant details about changes, testing, and includes supporting media, though some sections like Next Steps and Related PRs are appropriately omitted.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ryan/bug-fixing

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
app/build.gradle.kts (1)

45-53: Consolidate ECOSYSTEM_FLAG to defaultConfig to eliminate duplication across build types.

ECOSYSTEM_FLAG=true is currently set in three separate build types (ecosystem, debug, release), enabling ecosystem-related init-time fetches and UI in RouteRepository.kt (line 78), GymRepository.kt (line 72), EateryRepository.kt (line 29), and HomeScreen.kt (line 307). Moving the flag to defaultConfig avoids per-build-type drift and simplifies maintenance.

♻️ Suggested consolidation
 android {
     defaultConfig {
         applicationId = "com.cornellappdev.transit"
         minSdk = 26
         targetSdk = 36
         versionCode = 10
         versionName = "2.0"
+        buildConfigField("boolean", "ECOSYSTEM_FLAG", "true")
     }
     buildTypes {
         create("ecosystem") {
             initWith(getByName("debug"))
             isDebuggable = true
-            buildConfigField("boolean", "ECOSYSTEM_FLAG", "true")
             signingConfig = signingConfigs.getByName("debug")
         }
         debug {
-            buildConfigField("boolean", "ECOSYSTEM_FLAG", "true")
         }
         release {
-            buildConfigField("boolean", "ECOSYSTEM_FLAG", "true")
             signingConfig = signingConfigs.getByName("debug")
         }
     }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/build.gradle.kts` around lines 45 - 53, Move the ECOSYSTEM_FLAG build
config into the module's defaultConfig and remove the duplicated
buildConfigField("boolean", "ECOSYSTEM_FLAG", "true") entries from each build
type; specifically, add buildConfigField("boolean", "ECOSYSTEM_FLAG", "true")
inside the existing defaultConfig block and delete the same calls from the
ecosystem, debug and release buildType blocks so RouteRepository, GymRepository,
EateryRepository and HomeScreen (which read BuildConfig.ECOSYSTEM_FLAG) use the
single consolidated value.
🤖 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/models/RouteRepository.kt`:
- Around line 117-119: In clearLastRoute(), stop emitting ApiResponse.Pending
(which RouteScreen treats as isRefreshing) and instead emit a non-loading
cleared state via _lastRouteFlow; e.g., emit ApiResponse.Idle or
ApiResponse.Empty (add this new sealed-case to ApiResponse if it doesn't exist)
so RouteScreen won’t show a refresh spinner—update clearLastRoute() to set
_lastRouteFlow.value = ApiResponse.Idle (or ApiResponse.Empty) and ensure any
consumers handle the new state.

---

Nitpick comments:
In `@app/build.gradle.kts`:
- Around line 45-53: Move the ECOSYSTEM_FLAG build config into the module's
defaultConfig and remove the duplicated buildConfigField("boolean",
"ECOSYSTEM_FLAG", "true") entries from each build type; specifically, add
buildConfigField("boolean", "ECOSYSTEM_FLAG", "true") inside the existing
defaultConfig block and delete the same calls from the ecosystem, debug and
release buildType blocks so RouteRepository, GymRepository, EateryRepository and
HomeScreen (which read BuildConfig.ECOSYSTEM_FLAG) use the single consolidated
value.
🪄 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: e8705081-e404-4b4c-aefa-891a1347b617

📥 Commits

Reviewing files that changed from the base of the PR and between da48247 and 0aed98c.

📒 Files selected for processing (11)
  • app/build.gradle.kts
  • app/release/app-release.aab
  • app/src/main/AndroidManifest.xml
  • app/src/main/java/com/cornellappdev/transit/models/RouteRepository.kt
  • app/src/main/java/com/cornellappdev/transit/ui/components/home/AddFavoritesSearchSheet.kt
  • app/src/main/java/com/cornellappdev/transit/ui/components/home/HomeScreenMarkers.kt
  • app/src/main/java/com/cornellappdev/transit/ui/screens/HomeScreen.kt
  • app/src/main/java/com/cornellappdev/transit/ui/viewmodels/HomeViewModel.kt
  • app/src/main/java/com/cornellappdev/transit/ui/viewmodels/RouteViewModel.kt
  • build.gradle.kts
  • gradle/wrapper/gradle-wrapper.properties

Comment thread app/src/main/java/com/cornellappdev/transit/ui/screens/HomeScreen.kt Outdated
Comment thread app/src/main/java/com/cornellappdev/transit/ui/screens/HomeScreen.kt Outdated
Copy link
Copy Markdown
Member

@AndrewCheung360 AndrewCheung360 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM, just take a look at the coderabbit comments if they make snese.

@RyanCheung555 RyanCheung555 merged commit 11de6d2 into main Apr 15, 2026
1 check passed
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