[FEAT] 알림 센터 API 구현 #141
[FEAT] 알림 센터 API 구현 #141
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough알림 API/모델·리포지토리·서비스와 FCM 처리 로직 및 매니페스트 설정이 추가되고, 알림 화면이 MVVM(AlarmViewModel/AlarmUiState)으로 전환되며 인텐트 기반 알림 처리 및 알림 기반 내비게이션 흐름이 도입되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant AlarmScreen
participant AlarmVM as AlarmViewModel
participant Repo as NotificationRepository
participant Svc as NotificationService
participant API as Backend
User->>AlarmScreen: 진입 / 풀투리프레시 / 필터 변경 / 스크롤 끝
AlarmScreen->>AlarmVM: loadNotifications / loadMore / changeNotificationType
AlarmVM->>Repo: getNotifications(type, cursor)
Repo->>Svc: GET /notifications?cursor=&type=
Svc->>API: HTTP GET
API-->>Svc: BaseResponse<NotificationListResponse>
Svc-->>Repo: BaseResponse
Repo-->>AlarmVM: Result<NotificationListResponse?>
AlarmVM->>AlarmVM: 상태 병합(nextCursor/isLast, 로딩 플래그)
AlarmVM-->>AlarmScreen: uiState (StateFlow)
AlarmScreen-->>User: UI 업데이트
sequenceDiagram
autonumber
participant FCM as MyFirebaseMessagingService
participant Repo as NotificationRepository
participant Main as MainActivity
participant Nav as NavController
FCM->>Repo: onNotificationReceived() (emit refresh)
FCM->>Main: PendingIntent (사용자가 탭)
Main->>Repo: checkNotification(notificationId) via MainScreen flow
Repo-->>Main: NotificationCheckResponse
Main->>Nav: navigateFromNotification(response)
Nav-->>App: 대상 화면으로 이동
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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.
Pull Request Overview
This PR implements the notification center API functionality with real API integration replacing mock data, and updates the top bar notification icon to reflect unread notification status.
- Replaces mock notification data with real API integration for the alarm/notification center
- Implements dynamic notification icon status based on unread notifications in top bar
- Adds pagination and pull-to-refresh functionality for notifications
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| CommonNavigation.kt | Removes mock data dependencies from alarm screen navigation |
| GroupScreen.kt | Integrates AlarmViewModel to show unread notification indicator in top bar |
| FeedScreen.kt | Integrates AlarmViewModel to show unread notification indicator in top bar |
| AlarmViewModel.kt | Complete rewrite to use real API with pagination and state management |
| AlarmUiState.kt | New data class for managing alarm screen state |
| AlarmScreen.kt | Major refactor to use real API data with pull-to-refresh and infinite scroll |
| NotificationType.kt | New enum for notification type filtering |
| AlarmFilterRow.kt | Adds selection state support for filter buttons |
| NotificationService.kt | Adds API endpoint for fetching notifications |
| NotificationRepository.kt | Implements notification data fetching logic |
| NotificationListResponse.kt | New data models for notification API responses |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This utility function lacks documentation. Consider adding a KDoc comment explaining its purpose: removing prefix brackets from notification titles (e.g., '[피드]' → '').
| /** | |
| * Removes a bracketed prefix from the beginning of a notification title. | |
| * For example, "[피드] 내 글을 좋아합니다." becomes "내 글을 좋아합니다." | |
| * | |
| * @param title The notification title string, possibly with a bracketed prefix. | |
| * @return The title string without the bracketed prefix. | |
| */ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (16)
app/src/main/java/com/texthip/thip/ui/common/alarmpage/mock/NotificationType.kt (1)
3-7: mock 패키지 위치 재검토 + 서버 쿼리 값 매핑 일원화 제안
- enum이 실제 API 필터에 쓰이고 있어 mock 패키지보다는 실제 모듈(예: ui/common/alarmpage/model 또는 domain/data 계층)로 이동하는 편이 적절합니다.
- FEED_AND_ROOM의 서버 쿼리값이 "feedAndRoom"인지, 혹은 type 미전달(null)로 전체 조회인지 확인 필요합니다. 현재 value를 그대로 넘기면 서버 기대와 어긋날 수 있습니다. 매핑 함수를 중앙집중화해 오용을 방지하세요.
예시(매핑 함수):
// 위치 예: common/domain fun NotificationType.asQueryParam(): String? = when (this) { NotificationType.FEED_AND_ROOM -> null // 전체 조회가 null이라면 NotificationType.FEED -> "feed" NotificationType.ROOM -> "room" }app/src/main/java/com/texthip/thip/ui/common/alarmpage/component/AlarmFilterRow.kt (1)
24-35: Index 안전성 확보 및 방어적 접근selectedStates가 길이 2 미만일 때 IndexOutOfBoundsException 위험이 있습니다. getOrElse로 방어하거나, 초입에 require로 길이를 보장하세요.
적용 예(해당 줄 교체):
- isSelected = selectedStates[0], + isSelected = selectedStates.getOrElse(0) { false }, ... - isSelected = selectedStates[1], + isSelected = selectedStates.getOrElse(1) { false },추가 권장(함수 초반):
require(selectedStates.size >= 2) { "selectedStates must have size >= 2" }app/src/main/java/com/texthip/thip/data/repository/NotificationRepository.kt (1)
65-73: 파라미터 순서 매핑 OK + enum 기반 API 오버로드 제안
- (확인) Repository 시그니처(type, cursor) → Service 호출(getNotifications(cursor, type)) 매핑이 올바릅니다.
- 문자열 남용 방지를 위해 NotificationType 기반 오버로드를 추가하고 내부에서 매핑해 주는 편이 안전합니다.
예시:
suspend fun getNotifications( type: NotificationType = NotificationType.FEED_AND_ROOM, cursor: String? = null ): Result<NotificationListResponse?> { val typeQuery = type.asQueryParam() // FEED_AND_ROOM -> null 등 return getNotifications(type = typeQuery, cursor = cursor) }app/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmUiState.kt (1)
6-15: UI–Data 결합 완화 및 편의 프로퍼티 추가 제안
- UI 상태가 data 레이어의 DTO(NotificationResponse)에 직접 의존합니다. 중간 UI/Domain 모델로 매핑하면 변경 전파를 줄일 수 있습니다.
- 호출부 중복을 줄이기 위해 읽지 않은 알림 여부를 UI 상태에 파생 프로퍼티로 제공하는 것도 좋습니다.
예시:
val hasUnreadNotifications: Boolean get() = notifications.any { !it.isChecked }app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt (1)
58-60: AlarmViewModel 수명주기 수집 개선 + 공유 스코프 검토
- collectAsState 대신 collectAsStateWithLifecycle 사용을 권장합니다(수명주기 안전).
- Feed/Group 등 여러 탭에서 동일한 알림 뱃지를 일관되게 보여주려면 AlarmViewModel을 상위 NavGraph 스코프로 공유하거나, 리포지토리의 공유 Flow를 구독하도록 구성하는 것이 좋습니다. (현재 각 화면에서 hiltViewModel()로 가져오면 서로 다른 인스턴스일 수 있음)
적용 예(해당 라인 교체):
- val uiState by viewModel.uiState.collectAsState() - val alarmUiState by alarmViewModel.uiState.collectAsState() + val uiState by viewModel.uiState.collectAsStateWithLifecycle() + val alarmUiState by alarmViewModel.uiState.collectAsStateWithLifecycle()공유 스코프 예시:
// 상위 그래프 route 식별자 사용 val parentEntry = remember(navController) { navController.getBackStackEntry("root_graph") } val alarmViewModel: AlarmViewModel = hiltViewModel(parentEntry)Also applies to: 66-71, 169-173
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (1)
87-91: 수명주기 안전 수집 + ViewModel 공유 스코프 검토
- collectAsStateWithLifecycle 사용 권장(두 ViewModel 모두).
- 하단 탭 간 동일한 알림 상태가 필요하면 AlarmViewModel을 상위 NavGraph 스코프로 공유하세요.
적용 예:
- val feedUiState by feedViewModel.uiState.collectAsState() - val alarmUiState by alarmViewModel.uiState.collectAsState() + val feedUiState by feedViewModel.uiState.collectAsStateWithLifecycle() + val alarmUiState by alarmViewModel.uiState.collectAsStateWithLifecycle()프리뷰/호출부는 이미 hasUnreadNotifications 반영이 되어 있어 변경 영향은 최소입니다.
Also applies to: 258-261, 294-296, 336-338, 665-666
app/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmViewModel.kt (5)
30-32: 초기 로딩 이중 호출 가능성 (init + Screen의 LaunchedEffect).ViewModel의 init에서 reset 로드를 수행하고, 화면(AlarmScreen)의 LaunchedEffect에서도 즉시 refresh를 호출합니다. 현재 isLoadingData가 reset 요청을 허용하는 구조라 초기 진입 시 두 번 호출될 수 있습니다. 화면 쪽 초기 호출 제거를 권합니다(동일 PR 내 AlarmScreen.kt의 Line 48-51 제안 참고).
34-37: 로딩 중복 방지 가드 단순화.reset 여부와 관계없이 이미 로딩 중이면 바로 리턴하는 것이 안전합니다. 초기 중복 호출, 빠른 연속 스크롤에서의 중복 fetch를 줄일 수 있습니다.
fun loadNotifications(reset: Boolean = false) { - if (isLoadingData && !reset) return - if (isLastPage && !reset) return + if (isLoadingData) return + if (!reset && isLastPage) return
38-90: 예외 전파 시 에러 상태 누락 방지 (catch 추가).repository가 Result로 실패를 반환하지 않고 예외를 던질 경우, 현재 구조에서는 onFailure가 실행되지 않아 UI error가 설정되지 않습니다. catch 블록을 추가해 예외를 상태에 반영하세요.
viewModelScope.launch { - try { + try { isLoadingData = true ... - } finally { + } catch (e: Exception) { + updateState { it.copy(error = e.message ?: "알 수 없는 오류") } + } finally { isLoadingData = false updateState { it.copy(isLoading = false, isLoadingMore = false) } } }
42-54: 풀-리프레시 UX 개선: 성공 시 교체, 실패 시 기존 리스트 보존.reset 분기에서 목록을 즉시 비우면 실패 시 화면이 공백이 됩니다. 로딩 스피너만 켠 상태로 기존 목록을 유지하고, 성공 시에만 리스트를 교체하도록 바꾸는 것을 권장합니다.
- 1단계: reset 시 리스트 즉시 비우기 제거
- 2단계: onSuccess에서 reset이면 교체, 아니면 append
if (reset) { updateState { it.copy( isLoading = true, - notifications = emptyList(), hasMore = true ) } nextCursor = null isLastPage = false } else { updateState { it.copy(isLoadingMore = true) } } @@ - val currentList = - if (reset) emptyList() else uiState.value.notifications + val currentList = uiState.value.notifications + val newList = + if (reset) response.notifications + else currentList + response.notifications updateState { it.copy( - notifications = currentList + response.notifications, + notifications = newList, error = null, hasMore = !response.isLast ) }Also applies to: 66-77
6-7: NotificationType가 mock 패키지에 위치.프로덕션 코드에서 사용하는 enum은 mock 패키지에서 분리하여 공용/도메인 위치로 이동하는 편이 좋습니다(의존 방향 명확화, 테스트 더블과 분리).
Also applies to: 100-104
app/src/main/java/com/texthip/thip/ui/common/alarmpage/screen/AlarmScreen.kt (5)
48-51: 초기 데이터 중복 로드 제거 (ViewModel init과 중복).초기 합류 시 ViewModel.init에서 이미 reset 로딩을 수행합니다. 아래 LaunchedEffect는 동일 요청을 한 번 더 발생시킬 수 있으므로 제거를 권장합니다.
- LaunchedEffect(key1 = Unit) { - viewModel.refreshData() - }
46-47: 수명 주기 인지 컬렉션으로 전환.collectAsState 대신 collectAsStateWithLifecycle 사용을 권장합니다(백스택/비가시 상태에서 불필요한 수집 방지).
- val uiState by viewModel.uiState.collectAsState() + val uiState by viewModel.uiState.collectAsStateWithLifecycle()추가 import:
import androidx.lifecycle.compose.collectAsStateWithLifecycle
153-165: 하단 로딩 인디케이터 추가 권장.무한 스크롤 시 isLoadingMore=true일 때 하단에 로딩 인디케이터를 넣어 피드백을 주는 것이 UX에 유리합니다.
) { items(uiState.notifications, key = { it.notificationId }) { notification -> CardAlarm( badgeText = notification.notificationType, title = removeBracketPrefix(notification.title), message = notification.content, timeAgo = notification.postDate, isRead = notification.isChecked, onClick = { // TODO: 알림 읽음 처리 } ) } + if (uiState.isLoadingMore) { + item { + Column( + modifier = Modifier + .fillMaxSize() + .padding(vertical = 8.dp), + horizontalAlignment = Alignment.CenterHorizontally + ) { + androidx.compose.material3.CircularProgressIndicator() + } + } + } }
155-159: 배지 텍스트 i18n 매핑 권장.server에서 내려오는 notificationType 문자열을 그대로 노출하기보다, 타입 → stringResource 매핑으로 현지화/일관성 유지가 좋습니다.
70-71: 스크롤 상태 보존(옵션).회전/프로세스 재생성에 대비해 LazyListState를 rememberSaveable + Saver로 보존하는 것을 고려해 보세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
app/src/main/java/com/texthip/thip/data/model/notification/response/NotificationListResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/repository/NotificationRepository.kt(2 hunks)app/src/main/java/com/texthip/thip/data/service/NotificationService.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/common/alarmpage/component/AlarmFilterRow.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/common/alarmpage/mock/NotificationType.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/common/alarmpage/screen/AlarmScreen.kt(3 hunks)app/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmUiState.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt(6 hunks)app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/navigator/navigations/CommonNavigation.kt(0 hunks)
💤 Files with no reviewable changes (1)
- app/src/main/java/com/texthip/thip/ui/navigator/navigations/CommonNavigation.kt
🧰 Additional context used
🧬 Code graph analysis (3)
app/src/main/java/com/texthip/thip/ui/common/alarmpage/component/AlarmFilterRow.kt (1)
app/src/main/java/com/texthip/thip/ui/common/buttons/OptionChipButton.kt (1)
OptionChipButton(30-93)
app/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmViewModel.kt (1)
app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (1)
updateState(30-32)
app/src/main/java/com/texthip/thip/ui/common/alarmpage/screen/AlarmScreen.kt (2)
app/src/main/java/com/texthip/thip/ui/common/alarmpage/component/AlarmFilterRow.kt (1)
AlarmFilterRow(16-37)app/src/main/java/com/texthip/thip/ui/common/alarmpage/component/CardAlarm.kt (1)
CardAlarm(39-155)
🔇 Additional comments (4)
app/src/main/java/com/texthip/thip/data/service/NotificationService.kt (1)
37-41: type 파라미터의 의미 확인(전체 조회 전략)type를 null로 보내면 전체 조회가 되는지, 아니면 "feedAndRoom"과 같은 문자열을 요구하는지 서버 스펙 확인이 필요합니다. FEED_AND_ROOM 처리 방식에 따라 ViewModel/Repository 매핑을 맞춰주세요.
app/src/main/java/com/texthip/thip/data/model/notification/response/NotificationListResponse.kt (1)
6-21: 직렬화 컨버터: kotlinx.serialization 사용 확인 — notificationId 타입 검증 필요
- app/src/main/java/com/texthip/thip/data/di/NetworkModule.kt에서 Retrofit에 json.asConverterFactory(...)로 kotlinx.serialization 컨버터가 등록되어 있어 @Serializable/@SerialName 애노테이션이 유효합니다.
- NotificationListResponse.kt의 notificationId는 현재 Int입니다. 서버 API 스펙에서 ID 범위(정수 크기)를 확인하고, 서버가 64비트(Long) 값을 반환할 가능성이 있으면 Int → Long 변경을 적용하세요. (파일: app/src/main/java/com/texthip/thip/data/model/notification/response/NotificationListResponse.kt)
app/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmViewModel.kt (1)
63-64: 네트워크 디스패처 확인 필요.viewModelScope는 기본 Main입니다. repository.getNotifications 내부에서 IO 디스패처 또는 Retrofit/OkHttp 백그라운드 스레드에서 실행되는지 확인해 주세요. 그렇지 않다면 여기서 withContext(Dispatchers.IO)로 감싸는 것을 권장합니다.
app/src/main/java/com/texthip/thip/ui/common/alarmpage/screen/AlarmScreen.kt (1)
184-186: LGTM – 제목 전처리 유틸 깔끔합니다.대괄호 접두 제거 정규식이 의도에 부합합니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/src/main/java/com/texthip/thip/data/model/notification/response/NotificationCheckResponse.kt (1)
8-11:params필드를 옵셔널하게 처리하는 편이 안전합니다실제 응답에서
params가 누락되거나null로 내려오면 Kotlin Serialization이MissingFieldException을 던져 화면 진입 자체가 막힐 수 있습니다. 기본값을 두거나 nullable 처리로 방어 코드를 넣어두는 걸 권장합니다.가능한 수정 예시는 다음과 같습니다.
- @SerialName("params") val params: Map<String, JsonElement> + @SerialName("params") val params: Map<String, JsonElement> = emptyMap()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
app/src/main/java/com/texthip/thip/data/model/notification/request/NotificationCheckRequest.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/notification/response/NotificationCheckResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt
🔇 Additional comments (1)
app/src/main/java/com/texthip/thip/data/model/notification/request/NotificationCheckRequest.kt (1)
6-9: 요청 모델 정의 깔끔합니다백엔드 스펙과 직렬화 키가 일치하고, 불필요한 필드도 없어 유지 관리하기 좋습니다.
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (23)
app/src/main/java/com/texthip/thip/service/MyFirebaseMessagingService.kt (3)
86-91: requestCode Int 변환 오버플로우 가능 — 안전한 변환으로 변경Long→Int 캐스팅은 오버플로우로 음수가 될 수 있습니다. 모듈러 변환으로 충돌을 줄이세요.
- System.currentTimeMillis().toInt(), // 고유한 requestCode 사용 + (System.currentTimeMillis() % Int.MAX_VALUE).toInt(), // 안전한 고유 requestCode
106-110: notificationId도 Int 오버플로우 가드 적용동일한 안전 변환을 notificationId에도 적용해 충돌/음수 발생을 방지하세요.
- val notificationId = - dataPayload["notificationId"]?.toIntOrNull() ?: System.currentTimeMillis().toInt() + val notificationId = + dataPayload["notificationId"]?.toIntOrNull() + ?: (System.currentTimeMillis() % Int.MAX_VALUE).toInt()
40-47: Direct Boot 모드에서 저장소 접근 가드 필요Manifest에 directBootAware=true가 설정되어 있으면 사용자가 잠금 해제 전에도 서비스가 실행될 수 있습니다. Repository가 사용자 보호 스토리지/자격증명에 의존하면 예외/오동작 위험이 있으므로 UserUnlocked 가드를 권장합니다.
- CoroutineScope(Dispatchers.IO).launch { - try { - notificationRepository.onNotificationReceived() - } catch (e: Exception) { - Log.e(TAG, "Failed to trigger notification refresh", e) - } - } + CoroutineScope(Dispatchers.IO).launch { + try { + val um = getSystemService(android.os.UserManager::class.java) + if (um?.isUserUnlocked == true) { + notificationRepository.onNotificationReceived() + } else { + Log.w(TAG, "User locked; skip repository refresh in direct-boot mode") + } + } catch (e: Exception) { + Log.e(TAG, "Failed to trigger notification refresh", e) + } + }Repository가 direct-boot 안전한지 확인 부탁드립니다(EncryptedSharedPreferences/DataStore 사용 여부 등).
app/src/main/AndroidManifest.xml (2)
62-66: directBootAware=true에 따른 제약 검토 필요사용자 잠금 전 실행 시 Repository/DI가 사용자 보호 스토리지에 접근하면 실패합니다. 필요성이 낮다면 제거, 필요하다면 코드에서 UserUnlocked 가드(서비스 코멘트 참고)를 유지하세요.
66-68: 서비스 intent-filter priority는 효과 미미 — 제거 고려priority 속성은 주로 브로드캐스트 리시버에 의미 있습니다. MESSAGING_EVENT 서비스에는 불필요하므로 제거를 권장합니다.
- <intent-filter android:priority="1000"> + <intent-filter>app/src/main/java/com/texthip/thip/MainActivity.kt (2)
81-104: BG 알림 extras 키 확인 필요 — 키 상수 정규화 권장"gcm.notification.data.notificationId" 키는 실제 FCM에서 전달되지 않을 가능성이 큽니다. 일반적으로 data payload의 키 그대로("notificationId" 등) 또는 gcm.notification.* 접두는 notification payload 필드에만 사용됩니다. 실제 수신 extras를 기준으로 키를 정규화해 주세요.
제안:
- 공통 키 상수 정의(예: "notification_id", "notificationId") 후 우선순위 매핑.
- 실 단말에서 extras 키 로그를 기반으로 매칭 보정.
필요 시 제가 키 정규화 유틸을 추가해 드릴 수 있습니다.
116-128: Intent extras 정리 로직 단순화 — replaceExtras 호출 제거removeExtra로 이미 정리되며, replaceExtras(intent.extras)는 실질 효과가 없습니다. 제거하세요.
- // Intent 플래그도 정리 - intent.replaceExtras(intent.extras)app/src/main/java/com/texthip/thip/ui/navigator/extensions/NotificationNavigationExtensions.kt (2)
50-56: 미사용 변수 제거: postType
postType를 읽어오지만 사용하지 않습니다. 제거하여 혼동을 줄이세요.아래 diff를 적용하세요:
- val postId = params["postId"]?.toStringOrNull()?.toIntOrNull() - val postType = params["postType"]?.toStringOrNull() + val postId = params["postId"]?.toStringOrNull()?.toIntOrNull()
21-27: 파라미터 부족 시 폴백 처리필수 파라미터가 없을 때 현재는 no-op입니다. 간단한 폴백(예: 탭 루트로 이동) 또는 누락 키를 포함한 로그를 남기면 디버깅과 UX가 좋아집니다.
예:
ROOM_POST_DETAIL에서roomId/page중 하나라도 없을 경우GroupRoutes.Room또는 알림 센터로 복귀하도록 처리.Also applies to: 58-61, 63-66
app/src/main/java/com/texthip/thip/data/model/notification/response/NotificationCheckResponse.kt (1)
13-29: 장기적으로는 라우트별 파라미터 타입 안전성 강화 고려
Map<String, JsonElement>는 유연하지만 타입 안정성이 약합니다. 필요 시sealed class(route별 payload)로 확장해 역직렬화 레이어에서 컴파일타임 검증을 강화할 수 있습니다.app/src/main/java/com/texthip/thip/ui/group/note/viewmodel/GroupNoteViewModel.kt (1)
134-140: 불필요한 async/awaitAll 제거 (의미 없는 동시성 래핑)
loadPosts/loadBookPageInfo가 내부에서 각각launch를 사용하므로 여기서async/awaitAll은 효과가 없습니다. 간단히 호출만 하거나, 두 함수 자체를suspend로 바꿔coroutineScope에서 병렬화하세요.권장 최소 수정안:
- private fun refreshAllData() { - viewModelScope.launch { - val postsJob = async { loadPosts(isRefresh = true) } - val bookPageJob = async { loadBookPageInfo() } - awaitAll(postsJob, bookPageJob) - } - } + private fun refreshAllData() { + viewModelScope.launch { + // 두 작업 모두 내부에서 자체 코루틴을 시작하므로 추가 동기화 불필요 + loadPosts(isRefresh = true) + loadBookPageInfo() + } + }app/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmUiState.kt (2)
3-5: NotificationType를 mock 패키지에서 가져오는 의존성 제거 권장실서비스 코드에서 mock 패키지 의존은 유지보수에 불리합니다. NotificationType을 공용 도메인/모델 계층으로 이동하고 여기서 참조하세요.
Also applies to: 8-9
14-16: hasUnreadNotifications의 연산 비용 최소화대용량 리스트에서 매 recomposition마다 any 연산이 반복됩니다. ViewModel에서 미리 계산해 상태로 보유하거나 derivedState로 최소화하는 것을 고려하세요.
app/src/main/java/com/texthip/thip/MainScreen.kt (2)
29-33: Composable에서 EntryPoint로 Repository 직접 주입은 지양이 로직은 테스트/구조화에 불리합니다. ViewModel를 통해 주입/호출하거나, Activity/Navigation 진입지점에서 처리하여 MainScreen에는 이벤트만 전달하는 구조로 정리하는 것을 권장합니다.
59-59: 변수 이름 충돌(섀도잉) 정리파라미터(notificationData)와 let 블록 변수명이 동일해 가독성이 떨어집니다. 변수명 변경 권장.
- data?.let { notificationData -> + data?.let { nd -> - if (notificationData.fromNotification && notificationData.notificationId != null) { + if (nd.fromNotification && nd.notificationId != null) { - Log.d("MainScreen", "Processing notification: ${notificationData.notificationId}") + Log.d("MainScreen", "Processing notification: ${nd.notificationId}") - notificationData.notificationId.toInt() + nd.notificationId.toInt() - Log.e("MainScreen", "Invalid notification ID format: ${notificationData.notificationId}", e) + Log.e("MainScreen", "Invalid notification ID format: ${nd.notificationId}", e) - val result = notificationRepository.checkNotification(notificationId) + val result = notificationRepository.checkNotification(notificationId) - Log.d("MainScreen", "Notification check successful, navigating to: ${response.route}") + Log.d("MainScreen", "Notification check successful, navigating to: ${response.route}") - processedNotificationId = notificationData.notificationId + processedNotificationId = nd.notificationId - Log.w("MainScreen", "Notification check returned null response") + Log.w("MainScreen", "Notification check returned null response") - Log.e("MainScreen", "Failed to check notification: ${notificationData.notificationId}", exception) + Log.e("MainScreen", "Failed to check notification: ${nd.notificationId}", exception) - Log.e("MainScreen", "Unexpected error processing notification: ${notificationData.notificationId}", e) + Log.e("MainScreen", "Unexpected error processing notification: ${nd.notificationId}", e)app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (1)
87-91: 수명주기 인지 수집으로 변경 권장Flow 수집은 collectAsStateWithLifecycle 사용을 권장합니다. 불필요한 재수집과 백그라운드 리소스 소모를 줄일 수 있습니다.
- val feedUiState by feedViewModel.uiState.collectAsState() - val alarmUiState by alarmViewModel.uiState.collectAsState() + val feedUiState by feedViewModel.uiState.collectAsStateWithLifecycle() + val alarmUiState by alarmViewModel.uiState.collectAsStateWithLifecycle()추가로 다음 import가 필요합니다:
import androidx.lifecycle.compose.collectAsStateWithLifecycleapp/src/main/java/com/texthip/thip/ui/common/alarmpage/screen/AlarmScreen.kt (2)
114-117: PullToRefresh의 isRefreshing 상태 분리 권장초기 로딩/페이지 로딩과 당겨서 새로고침(isRefreshing)을 구분하지 않으면 UX 혼선이 생깁니다. uiState에 isRefreshing(Boolean)을 별도로 두고 연결하세요.
- PullToRefreshBox( - isRefreshing = uiState.isLoading, + PullToRefreshBox( + isRefreshing = uiState.isRefreshing, // 새 필드 도입 권장 onRefresh = onRefresh,
96-103: 필터 토글 로직 단순화BooleanArray 대신 현재 타입만으로 토글을 계산하면 간결합니다.
- val selectedStates = remember(uiState.currentNotificationType) { - when (uiState.currentNotificationType) { - NotificationType.FEED -> booleanArrayOf(true, false) - NotificationType.ROOM -> booleanArrayOf(false, true) - else -> booleanArrayOf(false, false) // FEED_AND_ROOM - } - } + val selectedStates = when (uiState.currentNotificationType) { + NotificationType.FEED -> booleanArrayOf(true, false) + NotificationType.ROOM -> booleanArrayOf(false, true) + else -> booleanArrayOf(false, false) + } ... - onToggle = { idx -> - val newNotificationType = when { - // 피드 버튼을 눌렀을 때 - idx == 0 -> { - if (selectedStates[0]) { - // 이미 선택된 상태면 전체로 변경 - NotificationType.FEED_AND_ROOM - } else { - // 선택되지 않은 상태면 피드만 - NotificationType.FEED - } - } - // 모임 버튼을 눌렀을 때 - idx == 1 -> { - if (selectedStates[1]) { - NotificationType.FEED_AND_ROOM - } else { - NotificationType.ROOM - } - } - else -> NotificationType.FEED_AND_ROOM - } - onChangeNotificationType(newNotificationType) - } + onToggle = { idx -> + val t = uiState.currentNotificationType + val next = when (idx) { + 0 -> if (t == NotificationType.FEED) NotificationType.FEED_AND_ROOM else NotificationType.FEED + 1 -> if (t == NotificationType.ROOM) NotificationType.FEED_AND_ROOM else NotificationType.ROOM + else -> NotificationType.FEED_AND_ROOM + } + onChangeNotificationType(next) + }Also applies to: 124-151
app/src/main/java/com/texthip/thip/ui/group/note/screen/GroupNoteScreen.kt (1)
260-304: 지정 포스트 스크롤 로직의 신뢰성 개선고정 오프셋(+1/+2)과 임의 delay는 취약합니다. snapshotFlow로 아이템 배치 완료를 관측 후 스크롤하세요.
- LaunchedEffect(uiState.scrollToPostId, uiState.posts, uiState.isLoading) { - val scrollToPostId = uiState.scrollToPostId - if (scrollToPostId != null && uiState.posts.isNotEmpty() && !uiState.isLoading) { - val targetIndex = uiState.posts.indexOfFirst { it.postId == scrollToPostId } - if (targetIndex != -1) { - val targetPost = uiState.posts[targetIndex] - val actualIndex = if (uiState.selectedTabIndex == 0) { targetIndex + 2 } else { targetIndex + 1 } - kotlinx.coroutines.delay(100) - try { - listState.animateScrollToItem(actualIndex) - if (openComments) { - kotlinx.coroutines.delay(200) - selectedPostForComment = targetPost - isCommentBottomSheetVisible = true - } - } catch (e: Exception) { - listState.scrollToItem(actualIndex) - if (openComments) { - kotlinx.coroutines.delay(200) - selectedPostForComment = targetPost - isCommentBottomSheetVisible = true - } - } - onEvent(GroupNoteEvent.ClearScrollTarget) - } - } - } + LaunchedEffect(uiState.scrollToPostId, uiState.posts, uiState.isLoading) { + val targetId = uiState.scrollToPostId ?: return@LaunchedEffect + if (uiState.isLoading || uiState.posts.isEmpty()) return@LaunchedEffect + val baseIndex = uiState.posts.indexOfFirst { it.postId == targetId } + if (baseIndex == -1) return@LaunchedEffect + val headerOffset = if (uiState.selectedTabIndex == 0) 2 else 1 + val actualIndex = baseIndex + headerOffset + // 리스트가 충분히 렌더링될 때까지 대기 + kotlinx.coroutines.flow.snapshotFlow { listState.layoutInfo.totalItemsCount } + .first { it > actualIndex } + listState.animateScrollToItem(actualIndex) + if (openComments) { + kotlinx.coroutines.delay(200) + selectedPostForComment = uiState.posts[baseIndex] + isCommentBottomSheetVisible = true + } + onEvent(GroupNoteEvent.ClearScrollTarget) + }필요 시 import:
import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.snapshotFlowapp/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmViewModel.kt (3)
27-29: 동시 업데이트 안전성: MutableStateFlow.update 사용 권장CAS 보장되는
MutableStateFlow.update로 교체해 잠재적 경쟁 상태를 줄이세요.적용 diff:
- private fun updateState(update: (AlarmUiState) -> AlarmUiState) { - _uiState.value = update(_uiState.value) - } + private fun updateState(update: (AlarmUiState) -> AlarmUiState) { + _uiState.update(update) + }추가 import:
import kotlinx.coroutines.flow.update
81-90: 중복 항목 제거(de-dup)로 페이징 안정성 개선페이지 중복 또는 빠른 새로고침에서 동일 알림이 중복될 수 있습니다.
distinctBy(notificationId)적용을 권장합니다. 상단 diff에 포함했습니다.
122-135: 이중 읽음 처리 가능성(무해) — 유지 또는 한 곳으로 단순화
checkNotification성공 시 로컬에서 읽음 처리하고, 동시에 Repository의 업데이트 플로우도 수집하므로 중복 반영될 수 있으나 무해합니다. 필요 시 한 쪽만 반영하도록 단순화 가능.app/src/main/java/com/texthip/thip/data/repository/NotificationRepository.kt (1)
75-83: 네트워크 호출 Dispatcher 명시 권장(IO)Retrofit가 내부적으로 스레드를 전환하더라도 호출 전후 처리로 Main이 묶일 수 있습니다.
withContext(Dispatchers.IO)래핑을 권장합니다. 전 함수에 일관 적용 추천.예시 diff(getNotifications에 적용):
- ): Result<NotificationListResponse?> { - return runCatching { - val response = notificationService.getNotifications(cursor, type) - response.handleBaseResponse().getOrNull() - } - } + ): Result<NotificationListResponse?> = withContext(Dispatchers.IO) { + runCatching { + val response = notificationService.getNotifications(cursor, type) + response.handleBaseResponse().getOrNull() + } + }추가 import:
import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (20)
app/src/main/AndroidManifest.xml(2 hunks)app/src/main/java/com/texthip/thip/MainActivity.kt(5 hunks)app/src/main/java/com/texthip/thip/MainScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/notification/request/NotificationCheckRequest.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/notification/response/NotificationCheckResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/repository/NotificationRepository.kt(5 hunks)app/src/main/java/com/texthip/thip/data/service/NotificationService.kt(2 hunks)app/src/main/java/com/texthip/thip/service/MyFirebaseMessagingService.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/common/alarmpage/screen/AlarmScreen.kt(3 hunks)app/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmUiState.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt(9 hunks)app/src/main/java/com/texthip/thip/ui/group/note/screen/GroupNoteScreen.kt(6 hunks)app/src/main/java/com/texthip/thip/ui/group/note/viewmodel/GroupNoteViewModel.kt(5 hunks)app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt(5 hunks)app/src/main/java/com/texthip/thip/ui/navigator/extensions/GroupNavigationExtensions.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/navigator/extensions/NotificationNavigationExtensions.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/navigations/CommonNavigation.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/navigator/navigations/GroupNavigation.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/navigator/routes/GroupRoutes.kt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
app/src/main/java/com/texthip/thip/ui/navigator/navigations/GroupNavigation.kt (1)
app/src/main/java/com/texthip/thip/ui/group/note/screen/GroupNoteScreen.kt (1)
GroupNoteScreen(75-192)
app/src/main/java/com/texthip/thip/ui/common/alarmpage/screen/AlarmScreen.kt (2)
app/src/main/java/com/texthip/thip/ui/common/alarmpage/component/AlarmFilterRow.kt (1)
AlarmFilterRow(16-37)app/src/main/java/com/texthip/thip/ui/common/alarmpage/component/CardAlarm.kt (1)
CardAlarm(39-155)
app/src/main/java/com/texthip/thip/ui/group/note/screen/GroupNoteScreen.kt (1)
app/src/main/java/com/texthip/thip/ui/group/note/viewmodel/GroupNoteViewModel.kt (1)
onEvent(142-189)
app/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmViewModel.kt (1)
app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (1)
updateState(30-32)
🔇 Additional comments (22)
app/src/main/java/com/texthip/thip/MainActivity.kt (2)
51-56: 초기 Intent 처리 위치 적절 — LGTMsetContent 이전에 알림 Intent를 처리해 초기 진입 동작이 안정적입니다.
107-113: notificationData 소비 후 초기화 플로우 확인notificationData를 MainScreen에서 소비 후 null로 리셋하지 않으면 재조합 시 중복 처리 가능성이 있습니다. 소비-리셋 프로토콜을 점검해 주세요.
app/src/main/java/com/texthip/thip/ui/navigator/navigations/CommonNavigation.kt (1)
19-23: 알림 기반 내비게이션 연결 LGTMAlarmScreen에서 navigateFromNotification으로 위임하는 구조가 명확합니다.
app/src/main/java/com/texthip/thip/ui/navigator/extensions/GroupNavigationExtensions.kt (2)
26-33: 가독성 개선 포매팅 LGTM
90-108: GroupRoutes.Note 호출부 파라미터 매핑 일치 확인됨app/src/main/java/com/texthip/thip/service/MyFirebaseMessagingService.kt (1)
112-126: minSdk가 29로 설정되어 있어 API 26 미만 기기를 지원하지 않으므로 별도 버전 가드 불필요합니다.Likely an incorrect or invalid review comment.
app/src/main/java/com/texthip/thip/ui/navigator/routes/GroupRoutes.kt (1)
43-50: 경로 데이터(Serializable) 필드 추가/순서 변경의 호환성 확인Typed Navigation과 직렬화 기반 라우팅 사용 중에 필드 추가/순서 변경이 딥링크/백스택 복원에 영향이 없는지 확인 필요합니다. 내부 전달만 사용 중이면 문제없지만, 문자열 경로 직렬화나 저장 복원과 섞일 경우 주의가 필요합니다.
혹시 기존 딥링크나 저장된 back stack state를 읽는 경로가 있으면 알려주세요. 필요 시 마이그레이션 가이드를 덧붙이겠습니다.
app/src/main/java/com/texthip/thip/ui/navigator/navigations/GroupNavigation.kt (2)
340-362: GroupNote 경로 확장 연동 LGTM
openComments,postId전달과GroupNoteScreen매핑이 일관됩니다. 초기 스크롤/댓글 열기 시나리오에 적합합니다.
144-150:deadlineDatenull 체크 로직 검증 필요
app/src/main/java/com/texthip/thip/data/model/rooms/response/JoinedRoomListResponse.kt에서
@SerialName("deadlineDate") val deadlineDate: String? = null으로 선언됨- null이 실제로 “시작 전”인지 “시작 후”인지 API 스펙/도메인 관점에서 확인 필요
app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt (2)
69-74: 알림 뱃지 연동 LGTM
AlarmViewModel의hasUnreadNotifications를 TopAppBar에 그대로 반영하는 흐름이 명확합니다.Also applies to: 187-192
144-150: 중복 코멘트: deadlineDate 기반 분기 검증해당 분기 로직에 대한 검증 요청은 GroupNavigation.kt에 남긴 코멘트를 참고해주세요.
app/src/main/java/com/texthip/thip/data/model/notification/response/NotificationCheckResponse.kt (1)
7-11: 응답 모델 정의 LGTM라우트와 동적 파라미터 맵 설계가 알림 라우팅 목적에 적합합니다.
app/src/main/java/com/texthip/thip/ui/group/note/viewmodel/GroupNoteViewModel.kt (1)
105-110: 초기 스크롤 타깃 설정 LGTM
initialPostId를 UI 상태로 노출 후 이벤트로 클리어하는 패턴이 명확합니다.app/src/main/java/com/texthip/thip/data/service/NotificationService.kt (1)
39-49: 신규 알림 API 추가 LGTM목록/체크 엔드포인트 시그니처가 요청/응답 모델과 일치합니다. 선택적 쿼리도 적절합니다.
서버 스펙에
type허용값이 명시돼 있다면(예: ALL/UNREAD/READ), enum/상수화로 오입력 방지를 고려해 주세요.app/src/main/java/com/texthip/thip/data/model/notification/request/NotificationCheckRequest.kt (1)
7-9: notificationId 타입 범위 및 일치성 검토
NotificationCheckRequest, NotificationListResponse 등 모델에서 notificationId를 Int로 선언해 32-bit 범위를 초과하는 값이 넘어올 경우 오버플로우가 발생할 수 있습니다. 서버 스펙에서 notificationId 타입(Int vs Long)을 확인하고, 필요 시 Long으로 변경한 뒤 repository, UI 변환 로직 등 전반을 함께 업데이트하세요.app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (1)
259-266: 알림 뱃지 연동 LGTMhasUnreadNotifications을 TopAppBar로 전달하는 연결이 명확합니다.
Also applies to: 336-341
app/src/main/java/com/texthip/thip/ui/common/alarmpage/screen/AlarmScreen.kt (2)
154-173: 리스트/아이템 구성 LGTM키 안정성 확보 및 클릭→체크 API 연동 흐름이 명확합니다.
192-194: 유틸 함수 KDoc 추가 제안(이전 코멘트와 중복)removeBracketPrefix에 간단한 KDoc을 추가해 의도를 명확히 해주세요.
app/src/main/java/com/texthip/thip/ui/group/note/screen/GroupNoteScreen.kt (1)
176-187: 내 프로필 분기 네비게이션 LGTM현재 사용자와 비교해 내/타인 프로필 네비게이션을 분기하는 로직이 명확합니다.
app/src/main/java/com/texthip/thip/MainScreen.kt (1)
78-93: checkNotification은 suspend 함수로 선언되어 비동기 호출이 보장되므로 UI 스레드 블로킹 없이 안전합니다app/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmViewModel.kt (1)
35-47: 이벤트 수집 자체는 적절함 (Repository 설정 주의)수집 로직은 적절합니다. 다만 Repository의 SharedFlow가 기본 설정이면 emit 시 수집자 부재로 인해 suspend될 수 있습니다. 아래 Repository 코멘트의 버퍼/tryEmit 수정 권장안을 참고하세요.
app/src/main/java/com/texthip/thip/data/repository/NotificationRepository.kt (1)
80-81: 서비스 파라미터 순서 확인 완료 서비스 인터페이스getNotifications(cursor, type)시그니처와 호출부(notificationService.getNotifications(cursor, type))가 일치합니다.
| <!-- FCM 기본 설정 --> | ||
| <meta-data | ||
| android:name="com.google.firebase.messaging.default_notification_icon" | ||
| android:resource="@mipmap/ic_launcher" /> | ||
|
|
||
| <!-- FCM 기본 클릭 액션 설정 --> | ||
| <meta-data | ||
| android:name="com.google.firebase.messaging.default_notification_channel_id" | ||
| android:value="thip_notifications" /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
FCM 기본 알림 아이콘은 단색 drawable로 지정
mipmap/ic_launcher 대신 단색 drawable을 사용해야 합니다. 상태바/티커 표시 품질을 위해 교체해 주세요.
<meta-data
android:name="com.google.firebase.messaging.default_notification_icon"
- android:resource="@mipmap/ic_launcher" />
+ android:resource="@drawable/ic_notification_small" />ic_notification_small 리소스가 없다면 생성이 필요합니다.
🤖 Prompt for AI Agents
app/src/main/AndroidManifest.xml lines 25-33: the FCM default notification icon
currently points to mipmap/ic_launcher which is not a single-color drawable;
replace the meta-data android:resource to reference a single-color drawable
(e.g., @drawable/ic_notification_small) and if ic_notification_small does not
exist, add a drawable resource (vector or shape) with a single solid foreground
color and transparent background suitable for status bar rendering, then update
resource name in the manifest to that drawable.
app/src/main/java/com/texthip/thip/data/repository/NotificationRepository.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/data/repository/NotificationRepository.kt
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/data/repository/NotificationRepository.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/service/MyFirebaseMessagingService.kt
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/service/MyFirebaseMessagingService.kt
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmViewModel.kt
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmViewModel.kt
Show resolved
Hide resolved
| // FeedViewModel을 통해 현재 사용자 정보 가져오기 | ||
| val feedViewModel: FeedViewModel = hiltViewModel() | ||
| val feedUiState by feedViewModel.uiState.collectAsStateWithLifecycle() | ||
| val currentUserId = feedUiState.myFeedInfo?.creatorId | ||
|
|
||
| // 내 피드 정보가 없으면 로드 | ||
| LaunchedEffect(Unit) { | ||
| if (feedUiState.myFeedInfo == null) { | ||
| feedViewModel.onTabSelected(1) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
현재 사용자 ID 취득을 위해 FeedViewModel 의존/호출은 과도함
그룹 노트 화면에서 피드 뷰모델을 생성하고 onTabSelected(1)로 네트워크 로딩을 유발합니다. 성능/결합도 측면에서 부적절합니다. Auth/UserRepository 등에서 currentUserId를 제공받거나 상위에서 주입하는 방식으로 교체하세요.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageNotificationEditScreen.kt (2)
58-76: 권한 거부 시 UX 보완(설정 이동) + 날짜 포맷 중복 제거 제안
- 거부 시 “설정으로 이동” 액션(Snackbar/ToastWithAction 등) 제공을 권장합니다. “다시 묻지 않기” 케이스에 대비해 앱 알림 설정 화면으로 이동 Intent를 제공하면 UX가 개선됩니다.
- 날짜 포맷 생성 코드가 중복됩니다. 헬퍼로 추출해 재사용하세요.
중복 제거 예시(해당 범위 내 최소 수정):
- val dateFormat = SimpleDateFormat("yyyy년 M월 d일 H시 m분", Locale.KOREAN) - toastDateTime = dateFormat.format(Date()) + toastDateTime = formatNowKoreanDatetime() ... - val dateFormat = SimpleDateFormat("yyyy년 M월 d일 H시 m분", Locale.KOREAN) - toastDateTime = dateFormat.format(Date()) + toastDateTime = formatNowKoreanDatetime()헬퍼 추가(파일 하단 등 적절한 위치):
private fun formatNowKoreanDatetime(): String = SimpleDateFormat("yyyy년 M월 d일 H시 m분", Locale.KOREAN).format(Date())설정 이동 액션(권장 예시):
// 권한 거부 시: val intent = Intent(Settings.ACTION_APP_NOTIFICATION_SETTINGS).apply { putExtra(Settings.EXTRA_APP_PACKAGE, context.packageName) } context.startActivity(intent) // 또는 UI에서 안내 후 사용자가 탭하면 이동
90-108: 권한 요청 중 토글 재탭 방지와 상태 동기화
- 권한 다이얼로그 표시 동안 토글을 비활성화하거나 로딩 상태를 노출해 중복 탭/깜빡임을 방지하세요(optimistic UI vs. 실제 상태 간 불일치 최소화).
- 권한 필요 시 즉시 토글 상태를 변경하지 않고 결과에 따라 반영하는 현재 전략은 안전합니다. 다만 “요청 중” 플래그를 두어 UI 힌트를 주는 것을 권장합니다.
간단 예시:
var requesting by rememberSaveable { mutableStateOf(false) } // 요청 직전 requesting = true notificationPermissionLauncher.launch(Manifest.permission.POST_NOTIFICATIONS) // onResult 내부 requesting = falseToggleSwitchButton에서 requesting 시 비활성화 처리(컴포넌트가 지원한다면) 또는 별도 로딩 표시를 추가하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
app/src/main/java/com/texthip/thip/MainScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/data/manager/FcmTokenManager.kt(0 hunks)app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageNotificationEditScreen.kt(6 hunks)
💤 Files with no reviewable changes (1)
- app/src/main/java/com/texthip/thip/data/manager/FcmTokenManager.kt
🔇 Additional comments (5)
app/src/main/java/com/texthip/thip/MainScreen.kt (1)
50-57: LaunchedEffect 키에 fromNotification을 포함해야 합니다동일 ID에서
fromNotification값만 false → true로 바뀌면 현재 키 구성으로는 효과가 재실행되지 않아 FCM 라우팅이 영구적으로 누락됩니다. 지난 코멘트와 같은 이슈이니 반드시 키를 확장해 주세요.- LaunchedEffect(notificationData?.notificationId) { + LaunchedEffect(notificationData?.notificationId, notificationData?.fromNotification) {app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageNotificationEditScreen.kt (4)
3-4: 권한 요청 런처 도입 LGTMPOST_NOTIFICATIONS 요청 플로우를 위한 Activity Result API 추가 적절합니다.
26-26: LocalContext import 적절권한 유틸 호출 맥락을 위해 필요한 의존성입니다.
53-53: LocalContext.current 사용 적절Composable 수명과 동일한 안전한 접근입니다.
42-42: NotificationPermissionUtils 분기 로직 정상 확인
API 33 미만에서는shouldRequestNotificationPermission이 false를 반환하며, 이미 권한 보유 시에도 요청하지 않습니다.
| message = when (message) { | ||
| "push_on" -> stringResource(R.string.push_on) | ||
| "push_off" -> stringResource(R.string.push_off) | ||
| "permission_denied" -> "알림 권한이 필요합니다. 설정에서 권한을 허용해주세요." | ||
| else -> stringResource(R.string.push_off) | ||
| }, |
There was a problem hiding this comment.
하드코딩된 한글 문자열을 리소스로 이동하세요
"permission_denied" 분기에서 문자열이 하드코딩되어 i18n이 깨집니다. strings.xml에 추가 후 사용하세요.
예시:
- "permission_denied" -> "알림 권한이 필요합니다. 설정에서 권한을 허용해주세요."
+ "permission_denied" -> stringResource(R.string.notification_permission_denied)추가로, "push_on"/"push_off"/"permission_denied"와 같은 매직 스트링 대신
- sealed class/enum 또는
- @stringres 기반 이벤트
로 타입 안정성을 높이는 것을 권장합니다.
📝 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.
| message = when (message) { | |
| "push_on" -> stringResource(R.string.push_on) | |
| "push_off" -> stringResource(R.string.push_off) | |
| "permission_denied" -> "알림 권한이 필요합니다. 설정에서 권한을 허용해주세요." | |
| else -> stringResource(R.string.push_off) | |
| }, | |
| message = when (message) { | |
| "push_on" -> stringResource(R.string.push_on) | |
| "push_off" -> stringResource(R.string.push_off) | |
| "permission_denied" -> stringResource(R.string.notification_permission_denied) | |
| else -> stringResource(R.string.push_off) | |
| }, |
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageNotificationEditScreen.kt
around lines 139 to 144, the branch for "permission_denied" contains a hardcoded
Korean string; move this string into strings.xml (e.g., add <string
name="permission_denied">알림 권한이 필요합니다. 설정에서 권한을 허용해주세요.</string>) and replace
the hardcoded literal with stringResource(R.string.permission_denied). Also
refactor the message discriminator to avoid magic strings by introducing a
sealed class or enum (or use @StringRes-based identifiers) for the notification
states ("push_on", "push_off", "permission_denied") and update call sites to use
those typed values so the when-expression becomes type-safe and uses string
resources consistently.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageNotificationEditScreen.kt (1)
90-108: ON/OFF 분기 중복 로직 축약 및 권한 거부 UX 보강 제안
- 동일한 토스트/날짜 세팅 반복을 축약하세요(위 코멘트와 동일 패턴).
- 권한 거부 시 설정 화면 이동 액션(선택사항) 제공을 고려하세요.
본 구간 내 변경 예:
- if (NotificationPermissionUtils.shouldRequestNotificationPermission(context)) { - // 권한이 필요하면 권한 요청 - notificationPermissionLauncher.launch(android.Manifest.permission.POST_NOTIFICATIONS) - } else { - // 권한이 이미 있거나 필요없으면 바로 설정 변경 - viewModel.onNotificationToggle(enabled) - toastMessage = "push_on" - val dateFormat = SimpleDateFormat("yyyy년 M월 d일 H시 m분", Locale.KOREAN) - toastDateTime = dateFormat.format(Date()) - } + if (NotificationPermissionUtils.shouldRequestNotificationPermission(context)) { + notificationPermissionLauncher.launch(android.Manifest.permission.POST_NOTIFICATIONS) + } else { + viewModel.onNotificationToggle(true) + toastResId = R.string.push_on + toastDateTime = nowKoreanDate() + } } else { - // 알림을 끄는 경우는 권한 체크 없이 바로 설정 변경 - viewModel.onNotificationToggle(enabled) - toastMessage = "push_off" - val dateFormat = SimpleDateFormat("yyyy년 M월 d일 H시 m분", Locale.KOREAN) - toastDateTime = dateFormat.format(Date()) + viewModel.onNotificationToggle(false) + toastResId = R.string.push_off + toastDateTime = nowKoreanDate() }선택적 UX 개선(설정 화면 이동):
// 권한 거부 브랜치에서 val intent = android.content.Intent(android.provider.Settings.ACTION_APP_NOTIFICATION_SETTINGS).apply { putExtra(android.provider.Settings.EXTRA_APP_PACKAGE, context.packageName) } // 적절한 UI(다이얼로그/스낵바 액션 등)에서 호출 context.startActivity(intent)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
app/src/main/java/com/texthip/thip/MainScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/data/repository/NotificationRepository.kt(5 hunks)app/src/main/java/com/texthip/thip/service/MyFirebaseMessagingService.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageNotificationEditScreen.kt(6 hunks)app/src/main/res/values/strings.xml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmViewModel.kt (1)
app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (1)
updateState(30-32)
🔇 Additional comments (6)
app/src/main/java/com/texthip/thip/service/MyFirebaseMessagingService.kt (2)
56-57: 로그 문구가 여전히 상태와 맞지 않습니다.실제로 포그라운드 여부를 판별하지 않는데도 “App is in foreground…”라고 로그를 남깁니다. 오해를 줄이도록 문구를 중립적으로 바꾸거나, 진짜 포그라운드 체크를 추가해 주세요.
- Log.d(TAG, "App is in foreground, showing custom notification: title=$title, body=$body") + Log.d(TAG, "Handling FCM message: title=$title, body=$body, data=$dataPayload")
94-102: 알림 small icon은 아직도 adaptive 아이콘을 사용하고 있습니다.상태바 small icon은 단색/투명이어야 하는데
R.mipmap.ic_launcher(컬러 adaptive 아이콘)를 그대로 쓰면 흰 박스나 색상 깨짐이 발생합니다. 단색 드로어블로 교체하고, 대응 리소스 및 Manifest 기본 아이콘도 함께 정리해 주세요.- .setSmallIcon(R.mipmap.ic_launcher) + .setSmallIcon(R.drawable.ic_notification_small)(위 변경과 함께
res/drawable/ic_notification_small.xml등 단색 아이콘 리소스를 추가하고, Manifest 내 default notification icon도 동일 리소스로 업데이트해야 합니다.)app/src/main/res/values/strings.xml (1)
130-130: 문구 리소스 추가 LGTM알림 권한 거부 시 노출할 안내 문구가 리소스로 중앙화되어 i18n/재사용에 적합합니다.
app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageNotificationEditScreen.kt (3)
3-4: 필요한 import 추가 적절권한 요청/컨텍스트/퍼미션 유틸 도입이 목적에 부합합니다.
Also applies to: 26-26, 42-42
58-76: 매직 스트링 제거하고 @stringres로 직접 상태를 들고 가세요 (+ 날짜 포맷 중복 제거)이전 코멘트와 동일 맥락입니다. "push_on/push_off/permission_denied" 토큰 대신 @stringres(Int)로 상태를 저장하면 when 매핑/오타 리스크를 없애고 코드가 단순해집니다. 날짜 포맷 생성도 헬퍼로 1회 정의하세요.
적용 예 (본 구간 내 변경):
- if (isGranted) { - // 권한이 허용되면 알림 활성화 - viewModel.onNotificationToggle(true) - toastMessage = "push_on" - val dateFormat = SimpleDateFormat("yyyy년 M월 d일 H시 m분", Locale.KOREAN) - toastDateTime = dateFormat.format(Date()) - } else { - // 권한이 거부되면 토스트 메시지 표시 - toastMessage = "permission_denied" - val dateFormat = SimpleDateFormat("yyyy년 M월 d일 H시 m분", Locale.KOREAN) - toastDateTime = dateFormat.format(Date()) - } + if (isGranted) { + viewModel.onNotificationToggle(true) + toastResId = R.string.push_on + toastDateTime = nowKoreanDate() + } else { + toastResId = R.string.notification_permission_required + toastDateTime = nowKoreanDate() + }본 구간 외 보조 변경(참고):
// 상태 정의 var toastResId by rememberSaveable { mutableStateOf<Int?>(null) } var toastDateTime by rememberSaveable { mutableStateOf("") } // 날짜 포맷 헬퍼(파일 상단 또는 하단에 배치) private fun nowKoreanDate(): String = java.text.SimpleDateFormat("yyyy년 M월 d일 H시 m분", java.util.Locale.KOREAN) .format(java.util.Date())
139-144: when 매핑 제거하고 @stringres 상태 직접 사용 + 안전한 기본값 처리토스트 메시지 매핑 로직을 제거하고 @stringres를 직접 사용하면 단순해집니다. 기본값을 강제로 push_off로 처리하는 것도 지양하세요.
- ToastWithDate( - message = when (message) { - "push_on" -> stringResource(R.string.push_on) - "push_off" -> stringResource(R.string.push_off) - "permission_denied" -> stringResource(R.string.notification_permission_required) - else -> stringResource(R.string.push_off) - }, - date = toastDateTime, - modifier = Modifier.fillMaxWidth() - ) + // 전제: toastMessage -> toastResId(@StringRes Int?)로 변경 + toastResId?.let { resId -> + ToastWithDate( + message = stringResource(resId), + date = toastDateTime, + modifier = Modifier.fillMaxWidth() + ) + }보조 변경(본 블록 바깥):
- AnimatedVisibility.visible 및 LaunchedEffect 키를 toastResId 기준으로 변경.
- 이름 일관성: strings.xml 키는 permission_required인데 토큰은 permission_denied였습니다. @stringres로 전환하면 이 불일치 문제도 자연스럽게 해소됩니다.
app/src/main/java/com/texthip/thip/data/repository/NotificationRepository.kt
Show resolved
Hide resolved
| var processedNotificationId by remember { mutableStateOf<String?>(null) } | ||
|
|
||
| // 푸시 알림에서 온 경우 알림 읽기 API 호출 및 네비게이션 | ||
| LaunchedEffect(notificationData?.notificationId, notificationData?.fromNotification) { | ||
| val data = notificationData | ||
|
|
||
| // 중복 처리 방지: 이미 처리한 알림이면 스킵 | ||
| if (data?.notificationId == processedNotificationId) { | ||
| Log.d("MainScreen", "Notification already processed: ${data?.notificationId}") | ||
| return@LaunchedEffect | ||
| } | ||
|
|
||
| data?.let { notificationData -> | ||
| if (notificationData.fromNotification && notificationData.notificationId != null) { | ||
| Log.d("MainScreen", "Processing notification: ${notificationData.notificationId}") | ||
|
|
||
| try { | ||
| val entryPoint = EntryPointAccessors.fromApplication( | ||
| context.applicationContext, | ||
| MainScreenEntryPoint::class.java | ||
| ) | ||
| val notificationRepository = entryPoint.notificationRepository() | ||
|
|
||
| // 알림 ID를 Int로 변환 시도 | ||
| val notificationId = try { | ||
| notificationData.notificationId.toInt() | ||
| } catch (e: NumberFormatException) { | ||
| Log.e("MainScreen", "Invalid notification ID format: ${notificationData.notificationId}", e) | ||
| return@LaunchedEffect | ||
| } | ||
|
|
||
| val result = notificationRepository.checkNotification(notificationId) | ||
|
|
||
| result.onSuccess { response -> | ||
| if (response != null) { | ||
| Log.d("MainScreen", "Notification check successful, navigating to: ${response.route}") | ||
| navController.navigateFromNotification(response) | ||
| // 알림 상태 강제 새로고침 트리거 | ||
| notificationRepository.onNotificationReceived() | ||
| // 처리 완료 표시 | ||
| processedNotificationId = notificationData.notificationId | ||
| } else { |
There was a problem hiding this comment.
동일 알림 재실행이 막히는 상태 관리 버그 수정 필요
processedNotificationId를 한 번 설정하면 동일 ID의 알림은 같은 컴포지션 생애 동안 다시 처리되지 않습니다. 앱이 살아 있는 동안 사용자가 같은 푸시 알림을 다시 탭하거나 시스템이 동일한 ID로 재전달할 때 네비게이션이 아예 이뤄지지 않는 치명적인 UX 문제가 생깁니다. 알림 재진입이 자연스럽게 가능하도록, 알림 처리가 끝난 뒤에는 processedNotificationId를 초기화하거나(예: notificationData == null인 경우), 이벤트 키를 고유하게 만들어 매번 새 알림으로 인식하도록 로직을 손봐 주세요. 최소한 동일 ID를 다시 처리할 수 있는 탈출구가 필요합니다.
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/MainScreen.kt around lines 47–88,
processedNotificationId is left set after first handling which prevents
re-processing the same notification ID for the lifetime of the composition;
update the state so the same ID can be handled again by either clearing
processedNotificationId after processing (e.g., set it back to null immediately
after navigation/notificationRepository.onNotificationReceived()) or by
resetting it when notificationData becomes null (handle that case in the
LaunchedEffect) — alternatively generate a unique event key (e.g., combine
notificationId with a timestamp or incrementing counter) when setting
processedNotificationId so repeated taps with the same ID are treated as
distinct events.
app/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmViewModel.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt (2)
69-71: collectAsState → collectAsStateWithLifecycle 사용 권장수명주기 인식 수집으로 불필요한 수집/재구성을 줄일 수 있습니다.
적용 diff:
- val uiState by viewModel.uiState.collectAsState() - val alarmUiState by alarmViewModel.uiState.collectAsState() + val uiState by viewModel.uiState.collectAsStateWithLifecycle() + val alarmUiState by alarmViewModel.uiState.collectAsStateWithLifecycle()추가 import:
import androidx.lifecycle.compose.collectAsStateWithLifecycle
148-154: 탐색 분기 조건 검증 필요: deadlineDate == null 의미 확인deadlineDate가 null이면 “시작 후”로 간주하는 것이 서버 계약과 일치하는지 확인해 주세요. 빈 문자열/비정상 포맷/시간 경과 케이스 처리도 필요할 수 있습니다. 가능하면 명시적 플래그(예: isStarted)를 사용하는 편이 안전합니다.
원하시면 서버 스키마에 맞춘 안전한 분기 헬퍼를 제안하겠습니다.
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (1)
89-91: collectAsState → collectAsStateWithLifecycle 사용 권장수명주기 인식 수집으로 백그라운드 수집을 방지할 수 있습니다.
적용 diff:
- val feedUiState by feedViewModel.uiState.collectAsState() - val alarmUiState by alarmViewModel.uiState.collectAsState() + val feedUiState by feedViewModel.uiState.collectAsStateWithLifecycle() + val alarmUiState by alarmViewModel.uiState.collectAsStateWithLifecycle()추가 import:
import androidx.lifecycle.compose.collectAsStateWithLifecycleapp/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmViewModel.kt (1)
95-101: 페이지 중복 방지: distinctBy 추가 권장페이지네이션에서 중복 항목 방지를 위해 ID 기준 dedup을 권장합니다.
적용 diff:
- it.copy( - notifications = currentList + response.notifications, + it.copy( + notifications = (currentList + response.notifications) + .distinctBy { n -> n.notificationId }, error = null, hasMore = !response.isLast )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
app/src/main/java/com/texthip/thip/MainScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/data/repository/NotificationRepository.kt(5 hunks)app/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt(9 hunks)app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmViewModel.kt (1)
app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (1)
updateState(30-32)
🔇 Additional comments (19)
app/src/main/java/com/texthip/thip/MainScreen.kt (1)
47-94: 같은 알림을 다시 열 수 없는 상태입니다.
processedNotificationId가 한 번 세팅되면 초기화되지 않아서 동일한notificationId가 다시 전달되면 곧바로 return 되어 네비게이션이 발생하지 않습니다. 동일한 푸시를 재탭하거나 시스템이 같은 ID로 재전달할 때 알림 이동이 막히는 심각한 UX 저하가 생깁니다. 처리 중에만 가드를 유지하고 완료 후에는 다시 열 수 있도록 ID를 초기화해 주세요.- if (data?.notificationId == processedNotificationId) { + if (data?.fromNotification == true && + data.notificationId != null && + data.notificationId == processedNotificationId + ) { return@LaunchedEffect } data?.let { notificationData -> if (notificationData.fromNotification && notificationData.notificationId != null) { - try { + processedNotificationId = notificationData.notificationId + try { val entryPoint = EntryPointAccessors.fromApplication( context.applicationContext, MainScreenEntryPoint::class.java ) val notificationRepository = entryPoint.notificationRepository() @@ - val result = notificationRepository.checkNotification(notificationId) - - result.onSuccess { response -> + val result = notificationRepository.checkNotification(notificationId) + + result.onSuccess { response -> if (response != null) { navController.navigateFromNotification(response) notificationRepository.onNotificationReceived() - processedNotificationId = notificationData.notificationId } else { Log.w("MainScreen", "Notification check returned null response") } }.onFailure { exception -> Log.e("MainScreen", "Failed to check notification: ${notificationData.notificationId}", exception) } } catch (e: Exception) { Log.e("MainScreen", "Unexpected error processing notification: ${notificationData.notificationId}", e) + } finally { + processedNotificationId = null } } } }app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt (4)
65-68: 초기 진입 시 알림 데이터 동기화 호출 적절화면 재진입 초기화와 함께 알림도 새로고침하는 선택이 타당합니다.
72-75: 상단바 뱃지 상태 연결 좋습니다알림 읽지 않음 여부를 GroupContent에 주입하는 구조가 명확합니다.
83-86: 풀투리프레시 시 그룹/알림 동시 새로고침 OK사용자 기대와 일치합니다.
190-196: 상단바 알림 뱃지 연동 LGTMLogoTopAppBar에 hasUnreadNotifications 연동이 올바릅니다.
app/src/main/java/com/texthip/thip/data/repository/NotificationRepository.kt (4)
27-33: SharedFlow 버퍼링/오버플로 설정 적절백프레셔 문제를 예방하는 구성(replay=0, extraBufferCapacity, DROP_OLDEST)과 공개형 SharedFlow 노출 패턴이 적절합니다.
Also applies to: 34-40
84-92: getNotifications 파라미터 순서 확인 필요Service 시그니처(type, cursor) ↔ Repository 호출(cursor, type) 불일치 가능성이 있습니다. 현재 구현이 정확한지 확인해 주세요.
검증 스크립트:
#!/bin/bash # NotificationService 시그니처 및 호출부 검증 rg -n -C3 --type=kt '\binterface\s+NotificationService\b' rg -n -C2 --type=kt '\bfun\s+getNotifications\s*\(' rg -n -C2 --type=kt '\bgetNotifications\s*\('설명: 첫 두 명령으로 실제 함수 정의의 파라미터 순서를 확인하고, 마지막 명령으로 호출부들이 동일한 순서를 따르는지 확인합니다. 불일치 시 컴파일은 되더라도 잘못된 응답이 내려올 수 있습니다.
100-103: 읽음 성공 이벤트 비차단 전파 👍tryEmit 사용으로 수집자 부재 시에도 호출부를 막지 않습니다.
108-110: 푸시 수신 이벤트 비차단 전파 👍서비스/백그라운드 컨텍스트에서 안전한 방식입니다.
app/src/main/java/com/texthip/thip/ui/feed/screen/FeedScreen.kt (6)
158-160: 변경 사항 영향 없음형 변환/기본값 처리만 보이며 동작 영향은 없어 보입니다.
223-229: 바텀탭 재선택 시 알림 동기화 호출 적절피드 새로고침과 함께 알림 상태를 맞추는 흐름이 일관됩니다.
262-263: 상단바 알림 뱃지 상태 전달 OK알림 상태를 단방향으로 FeedContent에 주입하는 구조가 좋습니다.
289-293: 풀투리프레시에서 알림 동시 새로고침 적절UX 기대에 부합합니다.
340-345: LogoTopAppBar 알림 뱃지 연동 LGTM뷰 계층에 종속성 누수 없이 상태 주입이 잘 되어 있습니다.
670-670: 프리뷰 hasUnreadNotifications 기본값 명시 👍프리뷰에서 false로 명시하여 의도 파악이 쉽습니다.
app/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmViewModel.kt (4)
16-19: Hilt 주입 구성 적절Repository 주입과 @hiltviewmodel 사용이 올바릅니다.
36-49: Flow 구독으로 로컬 상태 동기화 OK읽음/새로고침 이벤트를 구독해 상태를 갱신하는 접근이 적절합니다.
112-118: loadJob 식별 체크로 상태 리셋 경합 방지 👍Job 일치 확인 후 플래그 리셋하는 방식이 안전합니다.
138-146: check → 읽음 반영 → 내비게이션 흐름 적절UI 반영 순서와 콜백 설계가 명확합니다.
rbqks529
left a comment
rbqks529
left a comment
There was a problem hiding this comment.
-
앱을 시작했을 때 FCM 토큰을 서버에 전송하는 로직이 있고, 푸시 알림은 앱이 화면에 있을때 커스텀 알림을 표시하고, 백그라운드에서는 시스템에서 알림을 표시
-
두 알림 모두 Intent extras로 notificationId를 설정하고 MainScreen으로 이동해서 최종 네비게이션이 처리됨
There was a problem hiding this comment.
클로드의 도움을 받아서 백그라운드 푸시알림일 때 인텐트로 notificationId를 넘기도록 수정했습니다.
There was a problem hiding this comment.
MainActivity에서 전달받은 알림 데이터를 네비게이션으로 연결하는 로직을 구현했습니다
| return@LaunchedEffect | ||
| } | ||
|
|
||
| val result = notificationRepository.checkNotification(notificationId) |
There was a problem hiding this comment.
일단 권한이 없어도 전송을 해야됨
2 participants
➕ 이슈 링크
🔎 작업 내용
📸 스크린샷
알림 센터 조회
https://github.com/user-attachments/assets/55db3423-eede-41c0-97a6-711749868bec
알림 센터 아이콘
😢 해결하지 못한 과제
📢 리뷰어들에게
Summary by CodeRabbit