๐ :: (#958) ํธ์ ์๋ฆผ ๋นํ์ฑํ ์ ๊ณต์ง ํญ์ด ๋น์ด ๋ณด์ ๋ฌธ์ ํด๊ฒฐ#959
Hidden character warning
Conversation
|
Warning Review limit reached
Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review detailsโ๏ธ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ๐ Files selected for processing (1)
๐ WalkthroughWalkthroughNotification and notice data are now fetched separately, represented by distinct UI models, and rendered through keyed lists. Notice detail callbacks accept a single ID, while notification navigation is forwarded directly to the screen callback. ChangesNotification and notice flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant NotificationScreen
participant NotificationViewModel
participant NotificationRepository
participant NoticeRepository
User->>NotificationScreen: Open notifications
NotificationScreen->>NotificationViewModel: Observe notification state
NotificationViewModel->>NotificationRepository: Fetch notifications
NotificationRepository-->>NotificationViewModel: Notification data
NotificationViewModel->>NoticeRepository: Fetch notices
NoticeRepository-->>NotificationViewModel: Notice data
NotificationViewModel-->>NotificationScreen: NotificationUi and NoticeUi lists
User->>NotificationScreen: Select notification or notice
NotificationScreen-->>User: Invoke detail callback with item ID
Possibly related issues
๐ฅ Pre-merge checks | โ 5โ Passed checks (5 passed)
โจ 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 |
ashxom
changed the title
![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 (4)
feature/src/main/kotlin/team/aliens/dms/android/feature/notification/ui/NotificationScreen.kt (3)
183-200: ๐ Maintainability & Code Quality | ๐ต Trivial | โก Quick winSame naming concern as the
onNotificationDetailClickcallback above.This parameter here is purely notice-detail navigation; see consolidated comment.
๐ค Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/src/main/kotlin/team/aliens/dms/android/feature/notification/ui/NotificationScreen.kt` around lines 183 - 200, Rename the NoticeItems callback parameter onNotificationDetailClick to reflect that it performs notice-detail navigation, matching the naming convention requested for the corresponding callback above, and update the argument passed to NoticeItem accordingly.
1-1: ๐ Maintainability & Code Quality | ๐ต Trivial | โก Quick winRename
onNotificationDetailClick/onNavigateNotificationDetailClickto reflect notice-only usage.Across this call chain (
NotificationโNotificationScreenโNoticeItemsโNoticeItem), the callback is exclusively driven by notice clicks (notice.id) and is never used for point-notification navigation, which goes through the separateonNotificationClick/onNavigatePointHistorypath. Keeping the "Notification" name invites confusion about which flow it belongs to, especially now that the PR explicitly splits notification vs. notice data.
feature/src/main/kotlin/team/aliens/dms/android/feature/notification/ui/NotificationScreen.kt#L98-115: rename theNotificationcomposable'sonNavigateNotificationDetailClickparameter (and its forwarding at Line 109) toonNavigateNoticeDetailClick.feature/src/main/kotlin/team/aliens/dms/android/feature/notification/ui/NotificationScreen.kt#L183-200: renameNoticeItems'sonNotificationDetailClickparameter toonNoticeDetailClick.feature/src/main/kotlin/team/aliens/dms/android/feature/notification/ui/component/NoticeItem.kt#L18-33: renameNoticeItem'sonNotificationDetailClickparameter toonNoticeDetailClick.โป๏ธ Proposed rename (illustrative, apply consistently across all three sites)
- onNotificationDetailClick: (UUID) -> Unit, + onNoticeDetailClick: (UUID) -> Unit,๐ค Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/src/main/kotlin/team/aliens/dms/android/feature/notification/ui/NotificationScreen.kt` at line 1, Rename the notice-detail callback consistently across the Notification composable, NoticeItems, and NoticeItem: use onNavigateNoticeDetailClick for Notification and onNoticeDetailClick for NoticeItems and NoticeItem, including parameter declarations, forwarding calls, and invocation sites. Keep the separate onNotificationClick/onNavigatePointHistory flow unchanged.
98-115: ๐ Maintainability & Code Quality | ๐ต Trivial | โก Quick winCallback name no longer matches its purpose.
onNotificationDetailClick/onNavigateNotificationDetailClickis only ever wired to notice-item clicks (seeNoticeItemsat Line 155-158 andNoticeItem.kt), never to point-notification clicks. Naming it after "notification" is misleading now that the two flows are split. See consolidated comment for full scope.๐ค Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/src/main/kotlin/team/aliens/dms/android/feature/notification/ui/NotificationScreen.kt` around lines 98 - 115, The notification detail callback naming is misleading because it handles notice-item clicks rather than point-notification clicks. Rename the callback parameter and corresponding `onNavigateNotificationDetailClick` references in `NotificationScreen` and its `NoticeItems`/`NoticeItem` wiring to use notice-item terminology consistently, without changing callback behavior.feature/src/main/kotlin/team/aliens/dms/android/feature/notification/ui/component/NoticeItem.kt (1)
18-33: ๐ Maintainability & Code Quality | ๐ต Trivial | โก Quick winParameter name doesn't reflect that this is notice-only.
onNotificationDetailClickhere always receives a notice ID (notice.id); see consolidated comment covering the full call chain.๐ค Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/src/main/kotlin/team/aliens/dms/android/feature/notification/ui/component/NoticeItem.kt` around lines 18 - 33, Rename the NoticeItem callback parameter onNotificationDetailClick to a notice-specific name, and update its invocation and all callers through the notification UI call chain to use the new name while preserving the existing notice.id argument and behavior.
๐ค Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@feature/src/main/kotlin/team/aliens/dms/android/feature/notification/viewmodel/NotificationViewModel.kt`:
- Around line 31-54: Remove the NotificationTopic.POINT filter from
fetchNotifications so notifications of every supported topic, including
study-room, outing, and daybreak-study types, are mapped into notificationsUi
and displayed in the Notification tab. Keep the existing mapping and
success/failure handling unchanged.
---
Nitpick comments:
In
`@feature/src/main/kotlin/team/aliens/dms/android/feature/notification/ui/component/NoticeItem.kt`:
- Around line 18-33: Rename the NoticeItem callback parameter
onNotificationDetailClick to a notice-specific name, and update its invocation
and all callers through the notification UI call chain to use the new name while
preserving the existing notice.id argument and behavior.
In
`@feature/src/main/kotlin/team/aliens/dms/android/feature/notification/ui/NotificationScreen.kt`:
- Around line 183-200: Rename the NoticeItems callback parameter
onNotificationDetailClick to reflect that it performs notice-detail navigation,
matching the naming convention requested for the corresponding callback above,
and update the argument passed to NoticeItem accordingly.
- Line 1: Rename the notice-detail callback consistently across the Notification
composable, NoticeItems, and NoticeItem: use onNavigateNoticeDetailClick for
Notification and onNoticeDetailClick for NoticeItems and NoticeItem, including
parameter declarations, forwarding calls, and invocation sites. Keep the
separate onNotificationClick/onNavigatePointHistory flow unchanged.
- Around line 98-115: The notification detail callback naming is misleading
because it handles notice-item clicks rather than point-notification clicks.
Rename the callback parameter and corresponding
`onNavigateNotificationDetailClick` references in `NotificationScreen` and its
`NoticeItems`/`NoticeItem` wiring to use notice-item terminology consistently,
without changing callback behavior.
๐ช 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: a90ee955-ec2d-4d32-bc21-7fbd55de3cfb
๐ Files selected for processing (3)
feature/src/main/kotlin/team/aliens/dms/android/feature/notification/ui/NotificationScreen.ktfeature/src/main/kotlin/team/aliens/dms/android/feature/notification/ui/component/NoticeItem.ktfeature/src/main/kotlin/team/aliens/dms/android/feature/notification/viewmodel/NotificationViewModel.kt
1 participant
๊ฐ์
GET/notifications์ ์์กดํ๋ ๊ณต์ง ํญ์ GET/notices ๊ธฐ๋ฐ์ผ๋ก ๋ถ๋ฆฌํด์ ํธ์ ์๋ฆผ ๋นํ์ฑํ ์ฌ๋ถ์ ๊ด๊ณ์์ด ๊ณต์ง ๋ชฉ๋ก์ด ์ ์ ๋ ธ์ถ๋๋๋ก ์์ ํ์ต๋๋ค.
์์ ์ฌํญ
์ถ๊ฐ ๋ก ํ ๋ง
Summary by CodeRabbit
New Features
Bug Fixes