[Refactor] 1.2.x 나머지 수정사항 진행 (알림 제외)#132
Conversation
…factor/#131_ver_1.2.x
Walkthrough알림 카드 컴포넌트를 새로운 패키지로 이동하고 관련 import를 갱신했으며(빨간 점 레이아웃·시간 표기 조정), AlarmViewModel의 더미 항목 하나를 제거했습니다. 구독 바 모서리 반경과 검색 도서 상세 헤더/간격이 조정되고 피드 미리보가 추가되었습니다. 앱 매니페스트의 android:allowBackup가 false로 변경되었고, AuthStateManager/AuthInterceptor/MainActivity 연동으로 토큰 만료 이벤트 기반 전역 로그인 내비게이션이 도입되었습니다. 또한 계정 탈퇴 API 및 DeleteAccountViewModel/화면이 추가되어 계정 삭제 흐름이 구현되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as 앱(UI)
participant Main as MainActivity
participant Interceptor as AuthInterceptor
participant TokenMgr as TokenManager
participant Server as 서버
participant AuthState as AuthStateManager
Note over Client,Server: 요청 시점 (인증 헤더 주입 및 401 처리)
Client->>Server: HTTP Request (via Interceptor)
Interceptor->>Client: 요청 검사: Authorization header가 있으면 주입 생략
Interceptor->>TokenMgr: token/tempToken 조회
Interceptor->>Server: Request with Authorization
Server-->>Interceptor: 401 응답
Interceptor->>TokenMgr: 토큰 삭제/초기화
Interceptor->>AuthState: triggerTokenExpired()
Interceptor-->>Client: 401 반환
Note over AuthState,Main: 앱 레벨 반응 (토큰 만료 이벤트)
AuthState->>Main: tokenExpiredEvent emit
Main->>Main: collectLatest -> navController.popUpTo(0, inclusive=true) -> navigate(Login)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (1 passed, 2 warnings, 2 inconclusive)❌ Failed checks (2 warnings, 2 inconclusive)
✅ Passed checks (1 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ 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 |
![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)
app/src/main/java/com/texthip/thip/ui/feed/component/MySubscribelistBar.kt (1)
136-142: 빈 구독바 모서리 12dp 상향 LGTM — 디자인 토큰으로 추출 고려UI 의도에 부합합니다. 동일 반경(12dp)을 여러 곳에서 사용한다면 Theme(예: shapes) 또는 상수로 추출해 일관성/유지보수성을 높이는 것을 권장합니다.
app/src/main/java/com/texthip/thip/ui/common/alarmpage/component/CardAlarm.kt (1)
114-129: 빨간 점(읽음 표시) 고정 공간 확보 OK — 레이아웃 단순화 소폭 제안6dp 고정 공간 확보로 수직 정렬이 안정적입니다. 미세 제안: Column+Box 중첩 대신 Box 하나에
.size(6.dp)후if (!isRead)일 때만 내부 점을align(Alignment.Center)로 배치하면 계층이 줄어듭니다. 기능 변화는 없습니다.app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageReactionScreen.kt (1)
26-26: CardAlarm 새 경로 import 반영 LGTM — 필터 비교 문자열의 로컬라이즈 제안import 업데이트는 적절합니다. 추가로, 필터 로직에서
"좋아요","댓글"하드코딩 대신 stringResource 기반 비교로 현지화 안정성을 확보하는 것을 권장합니다.예시:
val likeLabel = stringResource(R.string.likes) val commentLabel = stringResource(R.string.comments) val filtered = reactions.filter { item -> val showAll = (isLikesSelected && isCommentsSelected) || (!isLikesSelected && !isCommentsSelected) showAll || (isLikesSelected && item.type == likeLabel) || (isCommentsSelected && item.type == commentLabel) }app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookDetailScreen.kt (1)
632-661: Preview용 UiState 구성 간소화 제안
emptyList()로 생성 후copy로 다시 설정하는 과정을 한 번에 작성하면 가독성이 좋아집니다. 기능 변화 없음.- val mockUiState = BookDetailUiState( - bookDetail = mockBookDetail, - relatedFeeds = emptyList(), // feedItems로 변환되므로 빈 리스트 - isLoadingFeeds = false, - isLoadingMore = false, - currentSort = "like" - ).copy( - // feedItems를 직접 설정하기 위해 relatedFeeds를 임시로 설정 - relatedFeeds = mockFeedItems.map { feedItem -> - com.texthip.thip.data.model.feed.response.RelatedFeedItem( - feedId = feedItem.id.toInt(), - creatorId = feedItem.id.toInt(), - creatorNickname = feedItem.userName, - creatorProfileImageUrl = feedItem.userProfileImage, - aliasName = feedItem.userRole, - aliasColor = "#FF6B9D", - postDate = feedItem.timeAgo, - isbn = mockBookDetail.isbn, - bookTitle = feedItem.bookTitle, - bookAuthor = feedItem.authName, - contentBody = feedItem.content, - contentUrls = feedItem.imageUrls, - likeCount = feedItem.likeCount, - commentCount = feedItem.commentCount, - isSaved = feedItem.isSaved, - isLiked = feedItem.isLiked, - isWriter = false - ) - } - ) + val mockUiState = BookDetailUiState( + bookDetail = mockBookDetail, + relatedFeeds = mockFeedItems.map { feedItem -> + com.texthip.thip.data.model.feed.response.RelatedFeedItem( + feedId = feedItem.id.toInt(), + creatorId = feedItem.id.toInt(), + creatorNickname = feedItem.userName, + creatorProfileImageUrl = feedItem.userProfileImage, + aliasName = feedItem.userRole, + aliasColor = "#FF6B9D", + postDate = feedItem.timeAgo, + isbn = mockBookDetail.isbn, + bookTitle = feedItem.bookTitle, + bookAuthor = feedItem.authName, + contentBody = feedItem.content, + contentUrls = feedItem.imageUrls, + likeCount = feedItem.likeCount, + commentCount = feedItem.commentCount, + isSaved = feedItem.isSaved, + isLiked = feedItem.isLiked, + isWriter = false + ) + }, + isLoadingFeeds = false, + isLoadingMore = false, + currentSort = "like" + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- 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/ui/common/alarmpage/component/CardAlarm.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/common/alarmpage/screen/AlarmScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmViewModel.kt(0 hunks)app/src/main/java/com/texthip/thip/ui/feed/component/MySubscribelistBar.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageReactionScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookDetailScreen.kt(4 hunks)
💤 Files with no reviewable changes (1)
- app/src/main/java/com/texthip/thip/ui/common/alarmpage/viewmodel/AlarmViewModel.kt
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookDetailScreen.kt (1)
app/src/main/java/com/texthip/thip/ui/theme/Theme.kt (1)
ThipTheme(41-69)
🔇 Additional comments (6)
app/src/main/java/com/texthip/thip/ui/common/alarmpage/component/CardAlarm.kt (3)
171-207: 프리뷰의 timeAgo 현지화 적용 LGTM프리뷰에서
"12시간 전"과 같이 완성 문자열을 사용하도록 정리된 점 좋습니다.
1-1: 잔여 레거시 참조 없음 확인됨 — 리뷰 승인
전체app/src코드에서com.texthip.thip.ui.common.cards.CardAlarm경로를 참조하는 import가 발견되지 않았습니다.
131-138: timeAgo 직접 표기 전환: 호출부 포맷 완성 여부 확인
AlarmScreen과MypageReactionScreen호출부에서 전달되는timeAgo가"12시간 전"등 완성된 로컬라이즈드 문자열인지 점검해주세요.- 일부 Preview(PreviewNotificationCards, AlarmScreenPreview)에서는 이미
"12시간 전"형태지만, 도메인 모델이나 서버 응답에서 숫자만 넘어올 가능성이 있으니, 필요 시CardAlarm내부로 포맷팅 로직(예:Duration/Date기반 API) 회귀를 검토하세요.app/src/main/java/com/texthip/thip/ui/common/alarmpage/screen/AlarmScreen.kt (1)
26-26: 아래 스크립트로 레거시 import 및 사용처를 재검증해 주세요.#!/bin/bash # legacy CardAlarm import 경로 검색 rg -n -uu -g '*.kt' 'com\.texthip\.thip\.ui\.common\.cards\.CardAlarm' # 패키지명 변경 전후 카드 컴포넌트 참조 전체 검색 rg -n -uu -g '*.kt' 'cards\.CardAlarm' # CardAlarm 컴포넌트 사용 위치 검색 (timeAgo 전달 부분 포함) rg -n -uu -g '*.kt' 'CardAlarm\('app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookDetailScreen.kt (2)
400-406: 피드 섹션 전환 그라데이션 높이 40dp 상향 LGTM전환부 가독성이 개선됩니다. 기존 상단/하단 여백과의 합산 간격만 디자인과 재확인하면 충분합니다.
460-466: 첫 피드 아이템 상단 간격 0dp 조정 LGTM상단 전환 그라데이션(40dp)과의 중복 여백 제거로 레이아웃이 깔끔해졌습니다.
| import com.texthip.thip.ui.common.modal.InfoPopup | ||
| import com.texthip.thip.ui.common.topappbar.GradationTopAppBar | ||
| import com.texthip.thip.ui.mypage.component.SavedFeedCard | ||
| import com.texthip.thip.ui.mypage.mock.FeedItem |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Preview 전용 mock 타입 import — 릴리즈 번들 포함 여부 확인
ui.mypage.mock.FeedItem은 프리뷰에서만 사용됩니다. 해당 패키지가 main 소스셋에 포함되어 릴리즈 번들에 실리지 않는지 확인 부탁드립니다. 가능하면 debug/preview 소스셋으로 이동하거나 프리뷰 파일 분리를 고려해 주세요.
검증 스크립트(릴리즈용 gradle 소스셋 경로에 mock 포함 여부 확인):
🏁 Script executed:
#!/bin/bash
# mock 경로 위치 파악
fd -a 'FeedItem.kt' | sed -n '1,120p'Length of output: 80
FeedItem.kt를 main 소스셋에서 debug/preview 전용 소스셋으로 이동 필요
현재 app/src/main/java/com/texthip/thip/ui/mypage/mock/FeedItem.kt가 main 소스셋에 위치해 있어 릴리즈 번들에 포함될 위험이 확인되었습니다. 해당 mock 타입은 debug 또는 preview 전용 소스셋으로 옮기거나, preview 전용 파일로 분리해 주세요.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/src/main/AndroidManifest.xml (2)
9-12: allowBackup=false일 때 dataExtractionRules/fullBackupContent는 무효 — 혼란 방지 정리 제안allowBackup을 false로 두면 Auto Backup/Device-to-Device 전송이 모두 비활성화되어 dataExtractionRules와 fullBackupContent가 사실상 사용되지 않습니다. 유지하면 오해 소지가 있으니 둘 중 하나로 정리하는 것을 권장합니다.
- 옵션 A(현 상태 유지, 정리): 백업 완전 차단을 유지하고 관련 속성 제거
- android:dataExtractionRules="@xml/data_extraction_rules" - android:fullBackupContent="@xml/backup_rules"
- 옵션 B(미세 제어): 백업은 허용하되 토큰만 제외
- android:allowBackup="false" + android:allowBackup="true"그리고 @xml/data_extraction_rules에서 토큰 저장소만 제외되었는지 확인/보완 필요(예: SharedPreferences/DB 항목 exclude). 원하시면 규칙 예시 초안 제공 가능합니다.
9-9: 사용자 이사(백업/기기전송) UX 영향 점검 필요allowBackup=false로 인해 Google 클라우드 백업과 Android 12+ 기기 간 데이터 전송이 모두 막힙니다. 의도한 정책인지 확인해 주세요. 간단 점검:
- 로그인 → 앱 삭제 → 재설치 후 로그인 상태가 복원되지 않아야 함.
- Android 12+ 기기 전송 시 앱 데이터가 이사되지 않아야 함.
필요 시 시나리오별 동작 표 정리 도와드릴게요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/src/main/AndroidManifest.xml(1 hunks)
🔇 Additional comments (1)
app/src/main/AndroidManifest.xml (1)
9-9: 보안 강화를 위한 백업 차단 적용, 의도와 부합android:allowBackup="false" 설정으로 앱 데이터(특히 토큰) 자동 백업/복원이 차단됩니다. "재설치 시 토큰이 복원되지 않도록" 하는 요구와 일치합니다.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/texthip/thip/utils/auth/AuthInterceptor.kt (1)
17-20: 조기 반환으로 401 처리 누락 가능요청에 이미 Authorization 헤더가 있으면 즉시 반환되어 이후 401 처리 로직이 실행되지 않습니다. 헤더 주입만 건너뛰고, 응답은 동일하게 검사하도록 재구성하세요.
- if (original.header("Authorization") != null) { - return chain.proceed(original) - } + val hasAuthHeader = original.header("Authorization") != null
🧹 Nitpick comments (5)
app/src/main/java/com/texthip/thip/data/manager/AuthStateManager.kt (2)
10-15: 이벤트 드랍 가능성 — 버퍼 오버플로 정책과 tryEmit 결과 처리 권장burst 상황에서 이벤트가 조용히 드랍될 수 있습니다. 드랍 정책을 명시하고 실패 시 로깅 정도는 해두는 편이 안전합니다.
+import kotlinx.coroutines.channels.BufferOverflow @@ - private val _tokenExpiredEvent = MutableSharedFlow<Unit>(extraBufferCapacity = 1) + private val _tokenExpiredEvent = MutableSharedFlow<Unit>( + replay = 0, + extraBufferCapacity = 1, + onBufferOverflow = BufferOverflow.DROP_OLDEST + ) @@ - fun triggerTokenExpired() { - _tokenExpiredEvent.tryEmit(Unit) - } + fun triggerTokenExpired() { + val emitted = _tokenExpiredEvent.tryEmit(Unit) + if (!emitted) { + // TODO: 필요 시 로깅/메트릭 추가 + } + }
9-15: 확장성 고려: Auth 이벤트를 sealed class로 일반화향후 갱신 실패/강제 로그아웃 등 다양한 인증 이벤트가 생길 수 있으니,
SharedFlow<AuthEvent>로 일반화하면 재사용성이 좋아집니다. 당장은 필수는 아닙니다.app/src/main/java/com/texthip/thip/utils/auth/AuthInterceptor.kt (1)
21-23: runBlocking 중복 호출 최소화두 번 호출하던
runBlocking을 한 번으로 합쳐 컨텍스트 전환 비용을 줄였습니다(위 diff에 반영).app/src/main/java/com/texthip/thip/MainActivity.kt (2)
44-49: 전체 백스택 정리 시 popUpTo(0) 사용은 불안정 — 시작 목적지 기준으로 정리 권장라이브러리 버전에 따라
0사용이 취약할 수 있습니다. 시작 목적지 기준으로 안전하게 비우는 패턴으로 교체하세요. 중복 내비 방지를 위해launchSingleTop도 권장합니다.+import androidx.navigation.NavGraph.Companion.findStartDestination @@ - LaunchedEffect(Unit) { - authStateManager.tokenExpiredEvent.collectLatest { - navController.navigate(CommonRoutes.Login) { - popUpTo(0) { inclusive = true } - } - } - } + LaunchedEffect(Unit) { + authStateManager.tokenExpiredEvent.collectLatest { + navController.navigate(CommonRoutes.Login) { + popUpTo(navController.graph.findStartDestination().id) { inclusive = true } + launchSingleTop = true + } + } + }
44-49: 수명주기 인지 수집 고려(선택)백그라운드 전환 시 과수집을 막고 싶다면
repeatOnLifecycle(STARTED)로 감싸는 방식을 검토하세요.val lifecycleOwner = androidx.compose.ui.platform.LocalLifecycleOwner.current LaunchedEffect(authStateManager, lifecycleOwner) { androidx.lifecycle.repeatOnLifecycle(androidx.lifecycle.Lifecycle.State.STARTED) { authStateManager.tokenExpiredEvent.collectLatest { /* navigate */ } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- 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/MainActivity.kt(1 hunks)app/src/main/java/com/texthip/thip/data/manager/AuthStateManager.kt(1 hunks)app/src/main/java/com/texthip/thip/utils/auth/AuthInterceptor.kt(2 hunks)
| val token = runBlocking { tokenManager.getTokenOnce() } | ||
| // 2. 정식 토큰이 없으면, 임시 토큰을 확인합니다. | ||
| val tempToken = runBlocking { tokenManager.getTempTokenOnce() } | ||
|
|
||
| // 보낼 토큰을 결정합니다 (정식 토큰 우선). | ||
| val tokenToSend = token ?: tempToken | ||
|
|
||
| val newRequest = original.newBuilder().apply { | ||
| tokenToSend?.let { addHeader("Authorization", "Bearer $it") } | ||
| }.build() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
토큰 없이 보낸 요청의 401까지 로그인 강제됨 — 조건 추가 필요
현재는 토큰을 실지 않았더라도 401이면 토큰 삭제+로그인 이동을 트리거합니다. 공개/비인증 API 401에도 반응하게 되어 UX가 흔들릴 수 있습니다. “이 인터셉터가 실제로 Authorization을 붙인 경우에만” 처리하도록 조건을 강화하세요.
- val token = runBlocking { tokenManager.getTokenOnce() }
- val tempToken = runBlocking { tokenManager.getTempTokenOnce() }
+ val (token, tempToken) = runBlocking {
+ tokenManager.getTokenOnce() to tokenManager.getTempTokenOnce()
+ }
@@
- val newRequest = original.newBuilder().apply {
- tokenToSend?.let { addHeader("Authorization", "Bearer $it") }
- }.build()
+ val requestToProceed = if (hasAuthHeader) {
+ original
+ } else {
+ original.newBuilder().apply {
+ tokenToSend?.let { addHeader("Authorization", "Bearer $it") }
+ }.build()
+ }
@@
- val response = chain.proceed(newRequest)
+ val response = chain.proceed(requestToProceed)
@@
- if (response.code == 401) {
+ // 이 인터셉터가 Authorization을 실제로 붙였을 때만 처리
+ if (response.code == 401 && !hasAuthHeader && tokenToSend != null) {
runBlocking {
tokenManager.clearTokens()
authStateManager.triggerTokenExpired()
}
}추가 메모:
- 가능하면 “우리 API 도메인에만” 위 처리를 적용하도록 베이스 URL 기준 가드(Host 체크)를 넣으면 오탐 401을 더 줄일 수 있습니다.
Also applies to: 30-41
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/utils/auth/AuthInterceptor.kt around lines
21-28 (and similarly at 30-41), the interceptor currently treats any 401 as a
signal to delete tokens and navigate to login even when no Authorization header
was attached; change the logic so the 401-handling only runs if the interceptor
actually added an Authorization header (i.e., tokenToSend != null /
Authorization present on the request) and the request is to our API host (guard
by checking request.url.host against the app's API base host); implement these
two guards before clearing tokens and forcing login on 401.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (11)
app/src/main/java/com/texthip/thip/data/service/UserService.kt (1)
18-18: DELETE 응답 스키마(204 여부) 확인 필요—역직렬화 실패 방지서버가 204 No Content를 반환하면
BaseResponse<Unit>역직렬화가 실패합니다. BE가 항상 래핑된 JSON(BaseResponse)으로 응답하는지 확인 부탁드립니다. 204 가능성이 있다면Response<Unit>로 받고 isSuccessful만 체크하는 형태가 안전합니다.참고 diff(필요 시 적용):
- @DELETE("users") - suspend fun deleteAccount(): BaseResponse<Unit> + @DELETE("users") + suspend fun deleteAccount(): retrofit2.Response<Unit>(추가 import:
import retrofit2.Response)또한 엔드포인트가
DELETE /users가 맞는지(예:/users/me) 사양 확인 부탁드립니다.Also applies to: 78-80
app/src/main/java/com/texthip/thip/data/repository/UserRepository.kt (1)
141-145: 반환 타입을 Result로 단순화 + 에러 매핑 일관화
Unit?는 불필요합니다.Result<Unit>로 축소하면 호출부 의도가 명확해집니다. 또한updateProfile만 별도 예외 파싱을 하고 있는데, 삭제도 동일 규칙을 적용해 일관성을 맞추는 것을 권장합니다.간단 타입 정리 diff:
- suspend fun deleteAccount(): Result<Unit?> = runCatching { + suspend fun deleteAccount(): Result<Unit> = runCatching { userService.deleteAccount() .handleBaseResponse() .getOrThrow() }만약 Service를
Response<Unit>로 바꾸면 여기서는check(response.isSuccessful)형태로 대응해 주세요.app/src/main/java/com/texthip/thip/ui/navigator/navigations/MyPageNavigation.kt (1)
62-64: 로그인 화면 이동 시 백스택 및 세션 정리 보장
onNavigateToLogin에서 인증 백스택을 완전히 비우고(예:popUpTo(0)또는 루트까지inclusive=true) 토큰/세션 삭제가 선행되도록 해주세요. 삭제 직후 뒤로가기로 보호 화면에 재진입되는 것을 방지해야 합니다.app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageLeavethipScreen.kt (2)
61-66: 내비게이션/에러 토스트는 단발성 이벤트로 처리 권장
State(Boolean/String?)는 프로세스 복원/재구성 시 재실행 위험이 있습니다.SharedFlow<UiEvent>등으로 one-shot 이벤트로 전환하면 안전합니다.예시:
// VM sealed interface UiEvent { data object DeleteCompleted: UiEvent; data class Error(val msg:String): UiEvent } val uiEvents = MutableSharedFlow<UiEvent>() // UI LaunchedEffect(Unit) { viewModel.uiEvents.collect { e -> when(e){ is UiEvent.DeleteCompleted -> onNavigateToLogin() is UiEvent.Error -> Toast.makeText(context, e.msg, Toast.LENGTH_SHORT).show() } } }Also applies to: 69-73
207-213: Preview는 hiltViewModel() 기본값 때문에 실패 가능프리뷰 전용 오버로드(상태 주입형)나 Fake VM을 받아서 사용하세요.
예시 프리뷰 전용 컴포저블:
@Composable fun DeleteAccountScreenPreviewable( uiState: DeleteAccountUiState = DeleteAccountUiState(), onNavigateBack: () -> Unit = {}, onNavigateToLogin: () -> Unit = {}, onConfirmDelete: () -> Unit = {} ) { /* UI만 구성 */ }프리뷰에서는
DeleteAccountScreenPreviewable(...)사용.app/src/main/java/com/texthip/thip/ui/mypage/viewmodel/DeleteAccountViewModel.kt (6)
3-3: ViewModel에 UI Context 전달 대신 @ApplicationContext 주입 권장Activity Context 전달은 불필요하며 회수 전까지의 참조 위험이 있습니다. Application Context를 주입해 사용해주세요.
import android.content.Context +import dagger.hilt.android.qualifiers.ApplicationContext @@ class DeleteAccountViewModel @Inject constructor( private val userRepository: UserRepository, - private val tokenManager: TokenManager + private val tokenManager: TokenManager, + @ApplicationContext private val appContext: Context, ) : ViewModel() { @@ - fun deleteAccount(context: Context) { + fun deleteAccount() { @@ - GoogleSignIn.getClient(context, gso).signOut() + GoogleSignIn.getClient(appContext, gso).signOut()Also applies to: 25-29, 34-35, 55-56
54-56: 계정 삭제 시 Google은 signOut보다 revokeAccess가 더 적합
signOut()은 로컬 연결 해제만 수행합니다. 계정 삭제 후엔 동의 철회까지 포함하는revokeAccess()가 더 일관됩니다.- val gso = GoogleSignInOptions.Builder(GoogleSignInOptions.DEFAULT_SIGN_IN).build() - GoogleSignIn.getClient(context, gso).signOut() + val gso = GoogleSignInOptions.Builder(GoogleSignInOptions.DEFAULT_SIGN_IN).build() + GoogleSignIn.getClient(context, gso).revokeAccess()
31-33: 공개 상태 스트림 타입 명시로 인터페이스 안정성 강화
uiState에 명시적 타입을 부여하면 외부 사용처와 리팩터링 안정성이 올라갑니다.import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow @@ - val uiState = _uiState.asStateFlow() + val uiState: StateFlow<DeleteAccountUiState> = _uiState.asStateFlow()Also applies to: 13-15
47-49: 로그 TAG 상수화 및 일관 사용하드코딩된 TAG 문자열을 상수로 통일하면 검색/필터링이 용이합니다.
- Log.e("DeleteAccountViewModel", "카카오 연결 끊기 실패", error) + Log.e(TAG, "카카오 연결 끊기 실패", error) @@ - Log.d("DeleteAccountViewModel", "카카오 연결 끊기 성공") + Log.d(TAG, "카카오 연결 끊기 성공") @@ - Log.e("DeleteAccountViewModel", "회원탈퇴 실패", exception) + Log.e(TAG, "회원탈퇴 실패", exception)클래스 내부에 추가:
private companion object { const val TAG = "DeleteAccountViewModel" }Also applies to: 60-60
61-66: 사용자 노출 에러 메시지 매핑
exception.message직접 노출은 조악하거나 영어일 수 있습니다. 네트워크/서버 코드별 메시지 맵핑 또는 공통 에러 파이프라인을 통해 일관된 한국어 문구를 노출하세요.원하시면 공통
ErrorMapper초안과 유닛 테스트 템플릿을 드리겠습니다.
25-33: 책임 분리: 외부 연동 해제 로직을 별도 UseCase/Manager로 이동ViewModel이 SDK 세부사항(Kakao/Google)을 직접 아는 구조입니다. 테스트 용이성과 변경 격리를 위해
AccountDeletionUseCase또는AuthProviderManager로 위임하고, ViewModel은 상태 전이만 담당하도록 정리하면 좋습니다.원하시면 인터페이스 설계(포트/어댑터)와 주입 예시를 제공하겠습니다.
Also applies to: 34-69
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- 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/data/repository/UserRepository.kt(1 hunks)app/src/main/java/com/texthip/thip/data/service/UserService.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageLeavethipScreen.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/mypage/viewmodel/DeleteAccountViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/navigations/MyPageNavigation.kt(1 hunks)
🔇 Additional comments (1)
app/src/main/java/com/texthip/thip/ui/mypage/viewmodel/DeleteAccountViewModel.kt (1)
12-16: I/O 작업 메인 스레드 실행 방지 확인
- ViewModelScope.launch는 기본적으로 Dispatchers.Main에서 실행됩니다 (이전 학습 참조). userRepository.deleteAccount()와 tokenManager.clearTokens()가 내부에서 Dispatchers.IO로 전환되는지 검증하세요. 전환 로직이 없다면 withContext(Dispatchers.IO)로 래핑하거나 Repository 레이어에서 IO 디스패처 사용을 보장해야 합니다.
| onConfirm = { | ||
| isDialogVisible = false | ||
| // TODO: 회원탈퇴 로직 | ||
| viewModel.deleteAccount(context) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
ViewModel에 Android Context 전달 지양
viewModel.deleteAccount(context)는 누수/테스트성 저하 위험이 있습니다. UI 의존은 Composable로, 앱 컨텍스트가 필요하면 @ApplicationContext로 주입하세요.
권장 diff(호출부):
- viewModel.deleteAccount(context)
+ viewModel.deleteAccount()ViewModel 예시(참고):
class DeleteAccountViewModel @Inject constructor(
@ApplicationContext private val appContext: Context,
private val repo: UserRepository,
private val tokenManager: TokenManager
) : ViewModel() {
fun deleteAccount() { /* ... */ }
}🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageLeavethipScreen.kt
around line 188, remove the direct pass of an Android Context into the ViewModel
(viewModel.deleteAccount(context)); instead update the ViewModel to receive
@ApplicationContext-injected Context via its constructor (or otherwise use
application-scoped dependencies) and expose a parameterless deleteAccount()
method; change the call in this composable to viewModel.deleteAccount(); move
any context-dependent work (e.g., file/SharedPreferences/Toast) into the
ViewModel using the injected app context and adjust DI/Hilt bindings to provide
@ApplicationContext to the ViewModel.
| // 로딩 중일 때 전체 화면에 로딩 인디케이터 표시 | ||
| if (uiState.isLoading) { | ||
| Box( | ||
| modifier = Modifier.fillMaxSize(), | ||
| contentAlignment = Alignment.Center | ||
| ) { | ||
| CircularProgressIndicator() | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
로딩 오버레이가 Column 자식으로는 실제 오버레이/터치 차단이 되지 않습니다
현재 구조에서는 하위에 쌓여 터치가 막히지 않을 수 있습니다. 삭제 중 중복 탭/중복 호출을 방지하려면 모달 다이얼로그로 띄워주세요.
권장 diff:
- if (uiState.isLoading) {
- Box(
- modifier = Modifier.fillMaxSize(),
- contentAlignment = Alignment.Center
- ) {
- CircularProgressIndicator()
- }
- }
+ if (uiState.isLoading) {
+ Dialog(
+ onDismissRequest = {},
+ properties = androidx.compose.ui.window.DialogProperties(
+ dismissOnBackPress = false,
+ dismissOnClickOutside = false
+ )
+ ) {
+ Box(
+ modifier = Modifier
+ .fillMaxWidth()
+ .padding(24.dp),
+ contentAlignment = Alignment.Center
+ ) {
+ CircularProgressIndicator()
+ }
+ }
+ }(필요 시 import: import androidx.compose.ui.window.DialogProperties)
📝 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.
| // 로딩 중일 때 전체 화면에 로딩 인디케이터 표시 | |
| if (uiState.isLoading) { | |
| Box( | |
| modifier = Modifier.fillMaxSize(), | |
| contentAlignment = Alignment.Center | |
| ) { | |
| CircularProgressIndicator() | |
| } | |
| } | |
| // 로딩 중일 때 전체 화면에 로딩 인디케이터 표시 | |
| if (uiState.isLoading) { | |
| Dialog( | |
| onDismissRequest = {}, | |
| properties = DialogProperties( | |
| dismissOnBackPress = false, | |
| dismissOnClickOutside = false | |
| ) | |
| ) { | |
| Box( | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .padding(24.dp), | |
| contentAlignment = Alignment.Center | |
| ) { | |
| CircularProgressIndicator() | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/ui/mypage/screen/MypageLeavethipScreen.kt
around lines 195-203, the current loading indicator is a Box inside the Column
so it may not block touches to underlying content; replace this in-column
overlay with a modal Dialog (using androidx.compose.ui.window.Dialog and
DialogProperties) so the loading state presents a full-screen modal that
prevents interaction and duplicate taps/calls; create a Dialog with an empty
onDismissRequest (or disable dismiss) and place a centered
CircularProgressIndicator inside, and add the necessary import for
DialogProperties.
| fun deleteAccount(context: Context) { | ||
| viewModelScope.launch { | ||
| _uiState.update { it.copy(isLoading = true, errorMessage = null) } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
중복 탭으로 인한 이중 요청 방지 가드 추가 권장
연속 탭 시 deleteAccount()가 중복 실행될 수 있습니다. 간단 가드로 서버 중복 호출과 UI 깜빡임을 막아주세요.
- fun deleteAccount(context: Context) {
+ fun deleteAccount(context: Context) {
viewModelScope.launch {
- _uiState.update { it.copy(isLoading = true, errorMessage = null) }
+ if (_uiState.value.isLoading) return@launch
+ _uiState.update { it.copy(isLoading = true, errorMessage = null) }📝 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.
| fun deleteAccount(context: Context) { | |
| viewModelScope.launch { | |
| _uiState.update { it.copy(isLoading = true, errorMessage = null) } | |
| fun deleteAccount(context: Context) { | |
| viewModelScope.launch { | |
| // Guard against duplicate taps: if already loading, skip | |
| if (_uiState.value.isLoading) return@launch | |
| _uiState.update { it.copy(isLoading = true, errorMessage = null) } | |
| // …rest of delete logic… | |
| } | |
| } |
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/mypage/viewmodel/DeleteAccountViewModel.kt
around lines 34-37, add a simple re-entrancy guard so consecutive taps won’t
trigger duplicate deleteAccount() calls: check the current _uiState (or an
isLoading flag) at the start and return immediately if a delete is already in
progress, set the flag to true before initiating the network call, and ensure
you clear the flag (set isLoading = false) in a finally block after the call
completes or fails.
| // 2. 서버 요청 성공 후 토큰 삭제 | ||
| tokenManager.clearTokens() | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
로컬 데이터(캐시/DB) 정리 여부 확인
토큰만 삭제되고 Room/캐시/파일 등이 남아있다면 재설치/재로그인 시 잔여 데이터가 보일 수 있습니다. 계정 삭제 플로우에 로컬 정리(use-case 또는 repo 메서드) 포함 여부 확인 바랍니다.
검증 스크립트:
원하시면 userRepository.clearLocalData() 형태의 안전한 롤백 가능한 구현을 제안드릴게요.
🏁 Script executed:
#!/bin/bash
# 로컬 정리 후보 탐색
rg -nP "(clear(All)?Tables|deleteAll|wipe|clearCache|clearUser.*Data)" -g "app/**"
# TokenManager 외 추가 정리 유틸 존재 여부
rg -n "clearTokens|logout" -g "app/**"Length of output: 838
로컬 데이터(캐시/DB) 정리 로직 추가 필요
DeleteAccountViewModel에서 tokenManager.clearTokens()만 호출되고 있어 Room DB, 파일, 기타 캐시가 그대로 남아 있습니다. 서버 계정 삭제 성공 후 DeleteAccountUseCase 또는 UserRepository에 clearLocalData() 같은 로컬 정리 메서드를 구현·호출해주세요.
| UserApiClient.instance.unlink { error -> | ||
| if (error != null) { | ||
| Log.e("DeleteAccountViewModel", "카카오 연결 끊기 실패", error) | ||
| } else { | ||
| Log.d("DeleteAccountViewModel", "카카오 연결 끊기 성공") | ||
| } | ||
| } | ||
|
|
||
| // 4. 구글 SDK에서 로그아웃 | ||
| val gso = GoogleSignInOptions.Builder(GoogleSignInOptions.DEFAULT_SIGN_IN).build() | ||
| GoogleSignIn.getClient(context, gso).signOut() | ||
|
|
||
| _uiState.update { it.copy(isLoading = false, isDeleteCompleted = true) } |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
외부 SDK(Kakao/Google) 해제 비동기 완료 전에 완료 플래그 세팅됨
unlink() 콜백과 signOut()(Task) 완료를 기다리지 않고 isDeleteCompleted = true로 전환합니다. 실제 연결 해제/토큰 철회가 끝나기 전에 네비게이션이 일어나 잔존 세션/동의가 남을 수 있습니다. 두 작업 완료 후 완료 플래그를 갱신하거나 실패는 무시하되 완료 시점을 동기화하는 것을 권장합니다.
예시(코루틴 기반, 선택): Kakao 콜백을 suspendCancellableCoroutine으로 감싸고, Google은 revokeAccess().await()(kotlinx-coroutines-play-services 필요)로 대기한 뒤 모두 끝나면 완료 플래그를 세팅합니다.
// helper
private suspend fun unlinkKakao(): Result<Unit> = suspendCancellableCoroutine { cont ->
UserApiClient.instance.unlink { error ->
if (error != null) cont.resume(Result.failure(error), null)
else cont.resume(Result.success(Unit), null)
}
}
// usage (성공 분기 내)
val gso = GoogleSignInOptions.Builder(GoogleSignInOptions.DEFAULT_SIGN_IN).build()
val googleClient = GoogleSignIn.getClient(context, gso)
runCatching {
unlinkKakao().getOrThrow()
googleClient.revokeAccess().await()
}.onFailure { Log.w(TAG, "외부 연동 해제 일부 실패", it) }
_uiState.update { it.copy(isLoading = false, isDeleteCompleted = true) }라이브러리 존재 여부 확인:
🏁 Script executed:
#!/bin/bash
rg -n "kotlinx-coroutines-play-services" -g "**/build.gradle*"Length of output: 0
🏁 Script executed:
#!/bin/bash
rg -n "kotlinx-coroutines-play-services" -g "**/*.gradle*"
rg -n "org.jetbrains.kotlinx:kotlinx-coroutines-play-services" -g "**/*.gradle*"Length of output: 0
비동기 작업 완료 후에만 isDeleteCompleted 설정
현재 Kakao unlink 콜백과 Google signOut/revokeAccess 완료를 기다리지 않고 바로 _uiState.update { it.copy(isLoading = false, isDeleteCompleted = true) }가 호출됩니다. 실제 토큰 철회/연결 해제 완료 시점까지 플래그 설정을 지연하거나, 실패 시 로그만 남기고 완료 시점을 동기화하세요.
또한, 예시에서 사용한 kotlinx-coroutines-play-services 의존성이 build.gradle에 포함되어 있지 않습니다. 해당 라이브러리를 추가하거나(예: implementation "org.jetbrains.kotlinx:kotlinx-coroutines-play-services:<버전>"), Google API 호출에 addOnCompleteListener 내에서 _uiState.update를 호출하는 방식으로 대체 구현해주세요.
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/mypage/viewmodel/DeleteAccountViewModel.kt
around lines 45 to 57, the code sets _uiState.isDeleteCompleted immediately
instead of after asynchronous Kakao unlink and Google signOut/revoke complete;
change the flow to wait for both async operations to finish (either by using the
Kakao unlink callback and Google Task.addOnCompleteListener or by adding
kotlinx-coroutines-play-services and using Task.await()) and only call
_uiState.update { it.copy(isLoading = false, isDeleteCompleted = true) } after
both have succeeded (or after both have completed while logging failures); if
you choose the coroutine approach add implementation
"org.jetbrains.kotlinx:kotlinx-coroutines-play-services:<version>" to
build.gradle and convert the Google signOut Task to await(), otherwise call
_uiState.update inside the Google addOnCompleteListener and in the Kakao
callback and coordinate completion (e.g., count down both completions) so the
state update happens once both are done.
2 participants
➕ 이슈 링크
🔎 작업 내용
📸 스크린샷
😢 해결하지 못한 과제
[] TASK
📢 리뷰어들에게
Summary by CodeRabbit
UI 변경
데이터
리팩터
새 기능 / 보안
잡무