Open
Conversation
Member
|
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThis change adds date-based filtering functionality to the game schedule. The AdvancedFilterBottomSheet now accepts an initial date filter parameter, HomeScreen connects filter callbacks to the ViewModel, and HomeViewModel implements date filtering logic with state management for selected date filters. Changes
Sequence DiagramsequenceDiagram
participant User
participant HomeScreen
participant AdvancedFilterBottomSheet
participant HomeViewModel
participant HomeUiState
User->>HomeScreen: Tap Advanced Filter Button
HomeScreen->>AdvancedFilterBottomSheet: Show with initialDate=selectedDateFilter
User->>AdvancedFilterBottomSheet: Select Date Filter
AdvancedFilterBottomSheet->>HomeScreen: onApply(dateFilter)
HomeScreen->>HomeViewModel: onDateFilterApplied(dateFilter)
HomeViewModel->>HomeUiState: Update selectedDateFilter
HomeViewModel->>HomeViewModel: Re-filter games by date
HomeViewModel->>HomeScreen: Emit updated loadedState
HomeScreen->>User: Display filtered games
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/src/main/java/com/cornellappdev/score/viewmodel/HomeViewModel.kt (1)
30-36: Inconsistent null handling across date filter cases.Line 31 for
TODAYrelies onnull == LocalDate.now()evaluating tofalse, while other cases explicitly check for null. This works but is inconsistent.🧹 Suggested fix for consistency
val dateMatch = when (selectedDateFilter) { - DateFilter.TODAY -> game.date == LocalDate.now() + DateFilter.TODAY -> game.date != null && game.date == LocalDate.now() DateFilter.WITHIN_7_DAYS -> game.date != null && !game.date.isAfter(LocalDate.now().plusDays(7)) DateFilter.WITHIN_A_MONTH -> game.date != null && !game.date.isAfter(LocalDate.now().plusDays(30)) DateFilter.OVER_A_MONTH -> game.date != null && game.date.isAfter(LocalDate.now().plusDays(30)) null -> true }🤖 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/HomeViewModel.kt` around lines 30 - 36, The date filter when-block in HomeViewModel (the val dateMatch assignment) treats DateFilter.TODAY differently by not checking game.date for null; change the TODAY branch to explicitly guard against null like the other branches (e.g., DateFilter.TODAY -> game.date != null && game.date == LocalDate.now()) so every case consistently checks game.date before accessing it.app/src/main/java/com/cornellappdev/score/components/AdvancedFiltersBottomSheet.kt (1)
23-23: Minor: Duplicate@OptInannotation.The
ExperimentalMaterial3Api::classis listed twice in the same annotation.🧹 Suggested fix
-@OptIn(ExperimentalMaterial3Api::class, ExperimentalMaterial3Api::class) +@OptIn(ExperimentalMaterial3Api::class)🤖 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/components/AdvancedFiltersBottomSheet.kt` at line 23, The `@OptIn` annotation in AdvancedFiltersBottomSheet is duplicated (ExperimentalMaterial3Api::class appears twice); remove the redundant entry so the annotation only lists ExperimentalMaterial3Api::class once (i.e., keep a single `@OptIn`(ExperimentalMaterial3Api::class)) to clean up the duplicate import/annotation above the AdvancedFiltersBottomSheet declaration.
🤖 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/screen/HomeScreen.kt`:
- Around line 95-101: The AdvancedFilterBottomSheet onApply lambda currently
receives price, location, date but only forwards date via
homeViewModel.onDateFilterApplied, silently ignoring price/location; either
disable the price/location UI in AdvancedFilterBottomSheet until implemented, or
add a TODO and a proper forwarding stub: update the onApply handler in
HomeScreen (where onApply = { price, location, date -> ... }) to either (A) call
a new ViewModel method homeViewModel.onFiltersApplied(price, location, date)
(create a stub onFiltersApplied in HomeViewModel to accept price and location
and wire it later), or (B) add a clear TODO comment in the onApply block and
disable the price/location sections in AdvancedFilterBottomSheet so selections
are not available to users; also ensure onReset still calls
homeViewModel.onFiltersReset and initialDate uses uiState.selectedDateFilter.
---
Nitpick comments:
In
`@app/src/main/java/com/cornellappdev/score/components/AdvancedFiltersBottomSheet.kt`:
- Line 23: The `@OptIn` annotation in AdvancedFiltersBottomSheet is duplicated
(ExperimentalMaterial3Api::class appears twice); remove the redundant entry so
the annotation only lists ExperimentalMaterial3Api::class once (i.e., keep a
single `@OptIn`(ExperimentalMaterial3Api::class)) to clean up the duplicate
import/annotation above the AdvancedFiltersBottomSheet declaration.
In `@app/src/main/java/com/cornellappdev/score/viewmodel/HomeViewModel.kt`:
- Around line 30-36: The date filter when-block in HomeViewModel (the val
dateMatch assignment) treats DateFilter.TODAY differently by not checking
game.date for null; change the TODAY branch to explicitly guard against null
like the other branches (e.g., DateFilter.TODAY -> game.date != null &&
game.date == LocalDate.now()) so every case consistently checks game.date before
accessing it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 66ea7440-b6c6-4f3c-a150-3c77459e7ced
📒 Files selected for processing (3)
app/src/main/java/com/cornellappdev/score/components/AdvancedFiltersBottomSheet.ktapp/src/main/java/com/cornellappdev/score/screen/HomeScreen.ktapp/src/main/java/com/cornellappdev/score/viewmodel/HomeViewModel.kt
Comment on lines
95
to
+101
| onApply = { price, location, date -> | ||
| // TODO: forward to ViewModel | ||
| homeViewModel.onDateFilterApplied(date) | ||
| }, | ||
| onReset = { | ||
| // TODO: reset filters in ViewModel | ||
| } | ||
| homeViewModel.onFiltersReset() | ||
| }, | ||
| initialDate = uiState.selectedDateFilter |
There was a problem hiding this comment.
Price and location filter parameters are silently ignored.
The onApply callback receives price, location, and date, but only date is forwarded to the ViewModel. If the UI allows selecting price/location filters, users may be confused when those selections have no effect.
Consider either:
- Disabling price/location sections in
AdvancedFilterBottomSheetuntil implemented, or - Adding a TODO comment to track the remaining filter implementations.
Would you like me to open an issue to track implementing the price and location filters?
🤖 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/HomeScreen.kt` around lines
95 - 101, The AdvancedFilterBottomSheet onApply lambda currently receives price,
location, date but only forwards date via homeViewModel.onDateFilterApplied,
silently ignoring price/location; either disable the price/location UI in
AdvancedFilterBottomSheet until implemented, or add a TODO and a proper
forwarding stub: update the onApply handler in HomeScreen (where onApply = {
price, location, date -> ... }) to either (A) call a new ViewModel method
homeViewModel.onFiltersApplied(price, location, date) (create a stub
onFiltersApplied in HomeViewModel to accept price and location and wire it
later), or (B) add a clear TODO comment in the onApply block and disable the
price/location sections in AdvancedFilterBottomSheet so selections are not
available to users; also ensure onReset still calls homeViewModel.onFiltersReset
and initialDate uses uiState.selectedDateFilter.
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
2 participants
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.
Advanced filtering for date
Summary by CodeRabbit
Release Notes