Conversation
amjiao
left a comment
amjiao
left a comment
There was a problem hiding this comment.
just two quick things to address, otherwise looks good!
| @@ -220,16 +230,26 @@ fun GameDetailsContent( | |||
| } | |||
|
|
|||
| Spacer(modifier = Modifier.weight(1f)) | |||
There was a problem hiding this comment.
the spacing is slightly off here
design
There was a problem hiding this comment.
I couldnt find the correct measurement, but this looks kind of similar to me
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 31 minutes and 58 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis change extends the game details feature by adding ticket purchasing and calendar integration. It retrieves ticket links via GraphQL, propagates the data through models and mappers, and implements UI buttons for buying tickets via external intent and adding games to the device calendar. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
app/src/main/res/drawable/ticket.xml (1)
10-10: Avoid hardcoded white stroke color for theme safety.
#ffffffat Line 10 and Line 16 can fail on light backgrounds and reduces theme adaptability. Prefer a theme attribute (or tint at usage site) so the icon follows light/dark modes consistently.Suggested change
- android:strokeColor="#ffffff" + android:strokeColor="?attr/colorOnSurface" ... - android:strokeColor="#ffffff" + android:strokeColor="?attr/colorOnSurface"Also applies to: 16-16
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/res/drawable/ticket.xml` at line 10, Replace the hardcoded stroke color in ticket.xml (the android:strokeColor attributes currently set to "#ffffff") with a theme attribute so the drawable follows light/dark themes; update the android:strokeColor entries to reference an appropriate theme color attribute (for example ?attr/colorOnBackground or ?attr/colorOnSurface / a custom attr like ?attr/ticketStrokeColor) and ensure the theme defines that attribute (or provide a default in styles) so the icon adapts to light/dark modes instead of using the literal "#ffffff".
🤖 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/score/model/Game.kt`:
- Line 304: The current assignment ticketUrl = ticketUrl ?: "" forces missing
URLs to an empty string; update the mapping in Game.kt so
DetailsCardData.ticketUrl remains nullable by preserving nulls and converting
blank strings to null instead (e.g., set ticketUrl = ticketUrl?.takeIf {
it.isNotBlank() } or simply leave ticketUrl as-is). Locate the assignment to
ticketUrl in the Game -> DetailsCardData mapping and replace the forced
empty-string normalization with a nullable-preserving normalization.
In `@app/src/main/java/com/cornellappdev/score/model/GameByIdQueryMappers.kt`:
- Line 28: The color alpha is being set using 0.4f * 255 which yields an
out-of-range value for Color.copy(alpha = …) (it expects 0.0–1.0); update the
call in GameByIdQueryMappers (the expression with
parseColor(this.color).copy(...)) to use a normalized alpha value like 0.4f
instead of 0.4f * 255, and apply the same change to all similar usages in
ScoreRepository.kt where Color.copy(alpha) is passed a scaled value.
In `@app/src/main/java/com/cornellappdev/score/screen/GameDetailsScreen.kt`:
- Around line 232-241: In GameDetailsScreen where ButtonPrimary is rendered for
"Buy Tickets", validate gameCard.ticketUrl is non-blank, parse it into a Uri and
ensure the scheme is "http" or "https", then build the
Intent(Intent.ACTION_VIEW, uri) and call
intent.resolveActivity(context.packageManager) to verify an activity can handle
it; only render the ButtonPrimary when both the URL/URI scheme validation and
resolveActivity check pass, and when clicked call startActivity(intent) knowing
it is safe. Reference: GameDetailsScreen, ButtonPrimary, gameCard.ticketUrl,
Intent, Uri, context.packageManager, resolveActivity.
In `@app/src/main/java/com/cornellappdev/score/viewmodel/GameDetailsViewModel.kt`:
- Around line 72-79: The calendar Intent creation and launch (the Intent
assigned to variable intent and the call context.startActivity(intent)) must be
hardened: before calling startActivity, check whether
intent.resolveActivity(context.packageManager) != null to ensure a calendar app
exists and wrap the launch in a try/catch for ActivityNotFoundException; also if
the provided context is not an Activity (context !is Activity) add
Intent.FLAG_ACTIVITY_NEW_TASK to the intent so it can be started from
non-Activity contexts. Ensure these checks/wrapping are applied around the
existing intent and the context.startActivity(intent) call.
---
Nitpick comments:
In `@app/src/main/res/drawable/ticket.xml`:
- Line 10: Replace the hardcoded stroke color in ticket.xml (the
android:strokeColor attributes currently set to "#ffffff") with a theme
attribute so the drawable follows light/dark themes; update the
android:strokeColor entries to reference an appropriate theme color attribute
(for example ?attr/colorOnBackground or ?attr/colorOnSurface / a custom attr
like ?attr/ticketStrokeColor) and ensure the theme defines that attribute (or
provide a default in styles) so the icon adapts to light/dark modes instead of
using the literal "#ffffff".
🪄 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: 6c8c9f92-f989-4c7f-aa2a-6a9114b83471
📒 Files selected for processing (6)
app/src/main/graphql/GameById.graphqlapp/src/main/java/com/cornellappdev/score/model/Game.ktapp/src/main/java/com/cornellappdev/score/model/GameByIdQueryMappers.ktapp/src/main/java/com/cornellappdev/score/screen/GameDetailsScreen.ktapp/src/main/java/com/cornellappdev/score/viewmodel/GameDetailsViewModel.ktapp/src/main/res/drawable/ticket.xml
| oppScore = convertScores(scoreBreakdown?.getOrNull(1), sport).second | ||
| ?: parsedScores?.second ?: 0 | ||
| ?: parsedScores?.second ?: 0, | ||
| ticketUrl = ticketUrl ?: "" |
There was a problem hiding this comment.
Preserve missing ticket URLs as absent.
DetailsCardData.ticketUrl is nullable, but this converts a missing URL into "", making downstream null checks ineffective. Keep it null or normalize blank values to null.
🐛 Proposed fix
- ticketUrl = ticketUrl ?: ""
+ ticketUrl = ticketUrl?.takeIf { it.isNotBlank() }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ticketUrl = ticketUrl ?: "" | |
| ticketUrl = ticketUrl?.takeIf { it.isNotBlank() } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/cornellappdev/score/model/Game.kt` at line 304, The
current assignment ticketUrl = ticketUrl ?: "" forces missing URLs to an empty
string; update the mapping in Game.kt so DetailsCardData.ticketUrl remains
nullable by preserving nulls and converting blank strings to null instead (e.g.,
set ticketUrl = ticketUrl?.takeIf { it.isNotBlank() } or simply leave ticketUrl
as-is). Locate the assignment to ticketUrl in the Game -> DetailsCardData
mapping and replace the forced empty-string normalization with a
nullable-preserving normalization.
| return GameDetailsTeam( | ||
| id = this.id, | ||
| color = parseColor(this.color).copy(alpha = 0.4f*255), | ||
| color = parseColor(this.color).copy(alpha = 0.4f * 255), |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the valid range for the alpha parameter of androidx.compose.ui.graphics.Color.copy in Jetpack Compose?
💡 Result:
The valid range for the alpha parameter of androidx.compose.ui.graphics.Color.copy in Jetpack Compose is [0.0, 1.0] as a Float. This is confirmed by multiple official sources including the Android Developer documentation and Compose UI Graphics API references, which state that the alpha component is always in the range [0..1] for the Color class. The copy function takes alpha: Float = this.alpha, where this.alpha returns a value in [0..1]. In the Color constructor source code (for sRGB, the default), alpha is explicitly clamped using fastCoerceIn(0.0f, 1.0f) before encoding. While the copy function itself does not show explicit clamping in the signature, values outside [0,1] are handled consistently with the Color representation, which normalizes alpha to [0,1]. The internal encoding uses a 10-bit integer [0..1023] mapped from the float [0..1].
Citations:
- 1: https://composables.com/jetpack-compose/androidx.compose.ui/ui-graphics/classes/Color/api
- 2: https://developer.android.com/reference/kotlin/androidx/compose/ui/graphics/Color
- 3: https://composables.com/docs/androidx.compose.ui/ui-graphics/classes/Color
- 4: https://developer.android.com/reference/kotlin/androidx/compose/ui/graphics/Color?authuser=1
- 5: https://github.com/androidx/androidx/blob/androidx-main/compose/ui/ui-graphics/src/commonMain/kotlin/androidx/compose/ui/graphics/Color.kt
🏁 Script executed:
# First, find the file
fd -i GameByIdQueryMappers.ktRepository: cuappdev/score-android
Length of output: 137
🏁 Script executed:
# Examine the file around line 28
sed -n '20,35p' app/src/main/java/com/cornellappdev/score/model/GameByIdQueryMappers.ktRepository: cuappdev/score-android
Length of output: 528
🏁 Script executed:
# Search for similar patterns of alpha calculations with * 255
rg "copy\(alpha\s*=.*\*\s*255" app/src/main/java/Repository: cuappdev/score-android
Length of output: 543
🏁 Script executed:
# Get more context from ScoreRepository.kt to see these other instances
rg -B 2 -A 2 "copy\(alpha\s*=.*\*\s*255" app/src/main/java/com/cornellappdev/score/model/ScoreRepository.ktRepository: cuappdev/score-android
Length of output: 818
Fix alpha value in Color.copy() to use normalized [0.0, 1.0] range.
Color.copy(alpha = ...) expects a value in [0.0, 1.0]; 0.4f * 255 produces 102f, which is outside the valid range and causes incorrect color rendering. Use 0.4f directly.
Note: Similar issues exist in ScoreRepository.kt at multiple locations.
🎨 Proposed fix
- color = parseColor(this.color).copy(alpha = 0.4f * 255),
+ color = parseColor(this.color).copy(alpha = 0.4f),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| color = parseColor(this.color).copy(alpha = 0.4f * 255), | |
| color = parseColor(this.color).copy(alpha = 0.4f), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/cornellappdev/score/model/GameByIdQueryMappers.kt` at
line 28, The color alpha is being set using 0.4f * 255 which yields an
out-of-range value for Color.copy(alpha = …) (it expects 0.0–1.0); update the
call in GameByIdQueryMappers (the expression with
parseColor(this.color).copy(...)) to use a normalized alpha value like 0.4f
instead of 0.4f * 255, and apply the same change to all similar usages in
ScoreRepository.kt where Color.copy(alpha) is passed a scaled value.
| Row() { | ||
| ButtonPrimary( | ||
| "Buy Tickets", | ||
| painterResource(R.drawable.ticket), | ||
| onClick = { | ||
| gameCard.ticketUrl?.let { url -> | ||
| val intent = Intent(Intent.ACTION_VIEW, Uri.parse(url)) | ||
| context.startActivity(intent) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Android ACTION_VIEW Intent startActivity ActivityNotFoundException resolveActivity documentation
💡 Result:
When launching an Android ACTION_VIEW Intent via startActivity, an ActivityNotFoundException is thrown if no activity is available to handle the implicit intent. This commonly occurs with ACTION_VIEW for viewing URIs (e.g., http, geo, content) when no suitable app like a browser, gallery, or maps is installed or accessible. Official documentation recommends two primary strategies to handle this: 1. Check resolveActivity first: Query the PackageManager to verify if any activity can handle the intent before calling startActivity. If resolveActivity returns non-null, proceed safely. Example (Kotlin): val intent = Intent(Intent.ACTION_VIEW, uri) if (intent.resolveActivity(packageManager) != null) { startActivity(intent) } else { // Handle gracefully, e.g., show message or fallback } Example (Java): Intent intent = new Intent(Intent.ACTION_VIEW, uri); if (intent.resolveActivity(getPackageManager) != null) { startActivity(intent); } else { // Handle gracefully } 2. Wrap in try-catch: Catch ActivityNotFoundException as a fallback. Example (Kotlin): try { startActivity(intent) } catch (e: ActivityNotFoundException) { // Define what your app should do if no activity can handle the intent. } Example (Java): Similar structure. Best practice combines both: Use resolveActivity to proactively avoid the exception, and try-catch for defense-in-depth. For Android 11+, declare in AndroidManifest.xml for package visibility if resolveActivity returns unexpected nulls for common intents like https VIEW. This applies specifically to implicit intents like ACTION_VIEW, which rely on intent resolution via action, data (URI scheme/type), and categories against installed apps' manifest filters.
Citations:
- 1: https://developer.android.com/reference/android/content/ActivityNotFoundException
- 2: https://developer.android.com/reference/android/content/Intent
- 3: https://developer.android.com/training/basics/intents/sending
- 4: https://android-dot-devsite-v2-prod.appspot.com/guide/components/intents-common
- 5: https://developer.android.com/guide/components/intents-common
🏁 Script executed:
find . -name "GameDetailsScreen.kt" -type fRepository: cuappdev/score-android
Length of output: 137
🏁 Script executed:
cat -n app/src/main/java/com/cornellappdev/score/screen/GameDetailsScreen.kt | sed -n '220,250p'Repository: cuappdev/score-android
Length of output: 1660
🏁 Script executed:
# Check if there are any other URL validation patterns in the codebase for reference
rg "resolveActivity|ActivityNotFoundException" --type kotlin -A 3 -B 3Repository: cuappdev/score-android
Length of output: 48
🏁 Script executed:
# Search for gameCard definition and ticketUrl property
rg "ticketUrl" --type kotlin -B 2 -A 2Repository: cuappdev/score-android
Length of output: 3435
🏁 Script executed:
# Check the gameCard data class definition
rg "data class.*GameCard|ticketUrl.*:" --type kotlin -A 5Repository: cuappdev/score-android
Length of output: 1890
🏁 Script executed:
# Check imports in GameDetailsScreen to see if packageManager is accessible
head -50 app/src/main/java/com/cornellappdev/score/screen/GameDetailsScreen.ktRepository: cuappdev/score-android
Length of output: 2385
🏁 Script executed:
# Check how context is obtained in GameDetailsScreen
rg "LocalContext" app/src/main/java/com/cornellappdev/score/screen/GameDetailsScreen.kt -B 5 -A 5Repository: cuappdev/score-android
Length of output: 1459
🏁 Script executed:
# Check if packageManager is used elsewhere in Compose code
rg "packageManager|resolveActivity" --type kotlinRepository: cuappdev/score-android
Length of output: 48
🏁 Script executed:
# Check the full composable function signature to see context parameter
sed -n '150,250p' app/src/main/java/com/cornellappdev/score/screen/GameDetailsScreen.kt | head -100Repository: cuappdev/score-android
Length of output: 4594
Validate and safely launch ticket URLs.
The current implementation can crash if no app handles the intent or if the URL is blank/malformed. Hide the button when the URL is invalid and check that an activity can handle the intent before launching.
The fix should:
- Validate the URL is non-blank
- Parse and validate the URI scheme is http or https
- Use
resolveActivity(context.packageManager)before callingstartActivity()to preventActivityNotFoundException - Only show the button if the URL is valid and an activity can handle it
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/cornellappdev/score/screen/GameDetailsScreen.kt` around
lines 232 - 241, In GameDetailsScreen where ButtonPrimary is rendered for "Buy
Tickets", validate gameCard.ticketUrl is non-blank, parse it into a Uri and
ensure the scheme is "http" or "https", then build the
Intent(Intent.ACTION_VIEW, uri) and call
intent.resolveActivity(context.packageManager) to verify an activity can handle
it; only render the ButtonPrimary when both the URL/URI scheme validation and
resolveActivity check pass, and when clicked call startActivity(intent) knowing
it is safe. Reference: GameDetailsScreen, ButtonPrimary, gameCard.ticketUrl,
Intent, Uri, context.packageManager, resolveActivity.
| val intent = Intent(Intent.ACTION_INSERT, CalendarContract.Events.CONTENT_URI).apply { | ||
| putExtra(CalendarContract.EXTRA_EVENT_BEGIN_TIME, startMillis) | ||
| putExtra(CalendarContract.EXTRA_EVENT_END_TIME, endMillis) | ||
| putExtra(CalendarContract.Events.TITLE, gameCard.title) | ||
| putExtra(CalendarContract.Events.EVENT_LOCATION, gameCard.locationString) | ||
| putExtra(CalendarContract.Events.DESCRIPTION, "${gameCard.sport} - ${gameCard.gender}") | ||
| } | ||
| context.startActivity(intent) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Android Context.startActivity FLAG_ACTIVITY_NEW_TASK ActivityNotFoundException implicit intent documentation
💡 Result:
When calling Context.startActivity from a non-Activity context (like ApplicationContext or Service) with an implicit intent, you must add Intent.FLAG_ACTIVITY_NEW_TASK to the intent flags. Otherwise, Android throws AndroidRuntimeException: "Calling startActivity from outside of an Activity context requires the FLAG_ACTIVITY_NEW_TASK flag. Is this really what you want?" This check occurs in ContextImpl.startActivity before intent resolution. This requirement is separate from ActivityNotFoundException, which is thrown later if no app can handle the implicit intent (no matching intent-filter with CATEGORY_DEFAULT). Official docs recommend checking intent.resolveActivity(getPackageManager) != null before startActivity to avoid ActivityNotFoundException. Use intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) for non-activity contexts. For implicit intents, always verify resolvability first. Example: Intent intent = new Intent(Intent.ACTION_VIEW, Uri.parse("https://example.com")); intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK); if (intent.resolveActivity(context.getPackageManager) != null) { context.startActivity(intent); } else { // Handle no app available }
Citations:
- 1: https://stackoverflow.com/questions/3918517/calling-startactivity-from-outside-of-an-activity-context
- 2: https://stackoverflow.com/questions/64898438/startactivity-not-working-even-after-adding-intent-flag-activity-new-task
- 3: https://stackoverflow.com/questions/54842282/calling-startactivity-outside-of-an-activity-context-requires-flag-activity-ne
- 4: https://developer.android.com/reference/android/content/ActivityNotFoundException
- 5: https://developer.android.com/reference/android/content/Intent
- 6: https://developer.android.com/guide/components/intents-common
- 7: https://android.googlesource.com/platform/frameworks/base/+/HEAD/core/java/android/app/ContextImpl.java
- 8: https://github.com/android/platform_frameworks_base/blob/master/core/java/android/app/ContextImpl.java
- 9: https://developer.android.com/training/basics/intents/sending
- 10: https://developer.android.com/guide/components/intents-filters
🏁 Script executed:
grep -A 25 "fun addGameToCalendar" app/src/main/java/com/cornellappdev/score/viewmodel/GameDetailsViewModel.ktRepository: cuappdev/score-android
Length of output: 1374
🏁 Script executed:
rg "addGameToCalendar" app/src/main/java/com/cornellappdev/score/viewmodel/GameDetailsViewModel.kt -B 3 -A 3Repository: cuappdev/score-android
Length of output: 266
🏁 Script executed:
rg "addGameToCalendar" app/src/main/java/com/cornellappdev/score/ -B 2 -A 2Repository: cuappdev/score-android
Length of output: 1231
🏁 Script executed:
rg -A 5 "onAddToCalendar = {" app/src/main/java/com/cornellappdev/score/screen/GameDetailsScreen.ktRepository: cuappdev/score-android
Length of output: 167
🏁 Script executed:
rg "onAddToCalendar" app/src/main/java/com/cornellappdev/score/screen/GameDetailsScreen.kt -A 5Repository: cuappdev/score-android
Length of output: 614
Harden the calendar intent launch.
This accepts any Context and launches an implicit intent directly. A non-Activity context requires FLAG_ACTIVITY_NEW_TASK and will crash at runtime. Similarly, a device without a calendar app will crash on ActivityNotFoundException.
Proposed fix
val intent = Intent(Intent.ACTION_INSERT, CalendarContract.Events.CONTENT_URI).apply {
putExtra(CalendarContract.EXTRA_EVENT_BEGIN_TIME, startMillis)
putExtra(CalendarContract.EXTRA_EVENT_END_TIME, endMillis)
putExtra(CalendarContract.Events.TITLE, gameCard.title)
putExtra(CalendarContract.Events.EVENT_LOCATION, gameCard.locationString)
putExtra(CalendarContract.Events.DESCRIPTION, "${gameCard.sport} - ${gameCard.gender}")
}
+ if (context !is android.app.Activity) {
+ intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
+ }
+
+ try {
+ context.startActivity(intent)
+ } catch (e: android.content.ActivityNotFoundException) {
+ Log.w("Calendar", "No calendar app available to handle insert intent", e)
+ }
- context.startActivity(intent)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/cornellappdev/score/viewmodel/GameDetailsViewModel.kt`
around lines 72 - 79, The calendar Intent creation and launch (the Intent
assigned to variable intent and the call context.startActivity(intent)) must be
hardened: before calling startActivity, check whether
intent.resolveActivity(context.packageManager) != null to ensure a calendar app
exists and wrap the launch in a try/catch for ActivityNotFoundException; also if
the provided context is not an Activity (context !is Activity) add
Intent.FLAG_ACTIVITY_NEW_TASK to the intent so it can be started from
non-Activity contexts. Ensure these checks/wrapping are applied around the
existing intent and the context.startActivity(intent) call.
2 participants
Add to Calendar
Ticketing
Summary by CodeRabbit