Open
Conversation
📝 WalkthroughWalkthroughSetting 모듈을 제거하고 MyPage 모듈로 교체했습니다. 로그아웃 플로우를 추가하고 MainActivityViewModel에서 앱 재개 시 로그인 상태를 갱신하도록 변경했습니다. Resume 모듈의 여러 함수를 suspend로 변경하고 데이트 유효성 검사를 추가했습니다. Changes
Sequence DiagramsequenceDiagram
participant User
participant MyPageScreen
participant MyPageViewModel
participant LogoutUseCase
participant LoginRepository
participant KakaoLoginManager
participant AuthEventBus
participant MainActivity
User->>MyPageScreen: 로그아웃 버튼 클릭
MyPageScreen->>MyPageScreen: 로그아웃 확인 다이얼로그 표시
User->>MyPageScreen: 확인 버튼 클릭
MyPageScreen->>MyPageViewModel: setEvent(LogoutClicked)
MyPageViewModel->>MyPageViewModel: isLoading = true
MyPageViewModel->>LogoutUseCase: invoke()
LogoutUseCase->>LoginRepository: logOut()
LoginRepository->>LoginRepository: loginService.logout()
LoginRepository->>LoginRepository: tokenManager.deleteToken()
par 병렬 실행
LogoutUseCase-->>MyPageViewModel: AppResult.Success(Unit)
MyPageViewModel->>KakaoLoginManager: kakaoLogout()
KakaoLoginManager->>KakaoLoginManager: UserApiClient.logout()
KakaoLoginManager-->>MyPageViewModel: Result.Success(Unit)
end
MyPageViewModel->>AuthEventBus: emit(LoggedOut)
AuthEventBus-->>MainActivity: LoggedOut 이벤트 수신
MainActivity->>MainActivity: isLoggedIn = false
MyPageViewModel->>MyPageViewModel: isLoading = false
MyPageViewModel-->>MyPageScreen: 로그아웃 완료
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50분 분석 근거:
주요 검토 항목:
Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
feature/download/impl/src/main/java/com/nonggle/feature/download/impl/DownloadScreen.kt (1)
1-1:⚠️ Potential issue | 🟡 Minor패키지 선언이 실제 파일 위치와 일치하지 않음
DownloadScreen.kt는
feature/download/impl/src/main/java/com/nonggle/feature/download/impl/디렉토리에 위치하지만, 패키지는com.nonggle.feature.download.impl.navigation으로 선언되어 있습니다. 실제로 navigation 서브패키지 디렉토리에는 DownLoadEntryProvider.kt만 있으므로, DownloadScreen.kt의 패키지를com.nonggle.feature.download.impl으로 수정하거나 파일을 navigation 디렉토리로 이동해야 합니다. 현재 상태는 패키지 구조를 혼란스럽게 만들 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/download/impl/src/main/java/com/nonggle/feature/download/impl/DownloadScreen.kt` at line 1, 파일 DownloadScreen.kt의 패키지 선언이 실제 디렉토리와 맞지 않습니다; DownloadScreen.kt (참조되는 심볼: DownloadScreen) 파일을 현재 디렉토리 구조에 맞게 수정하세요 — 방법1: 파일 내 패키지 선언을 com.nonggle.feature.download.impl 으로 변경하거나, 방법2: 현재 패키지 com.nonggle.feature.download.impl.navigation을 유지하려면 파일을 navigation 서브디렉토리로 이동(참고: DownLoadEntryProvider.kt는 navigation에 있음)하여 패키지와 파일 경로가 일치하도록 만드세요.feature/resume/impl/src/main/java/com/nonggle/impl/step1/ResumeStep1ViewModel.kt (1)
145-158:⚠️ Potential issue | 🟡 Minor공백만 입력해도 자격증 태그가 추가됩니다.
Line 147에서
trim()한 값을 바로 리스트에 넣고 있어서" "만 입력한 경우에도 빈 항목이 저장됩니다. ViewModel에서 trim 결과가 blank면 즉시 return하고, 버튼 활성화 조건도 같은 규칙으로 맞추는 게 안전합니다.수정 예시
private suspend fun addCertification() { - val newCertificationList = - currentState.info.certificationList + CertificationTag(certificationTitle = currentState.certificationInput.trim()) + val value = currentState.certificationInput.trim() + if (value.isEmpty()) return + val newCertificationList = + currentState.info.certificationList + CertificationTag(certificationTitle = value) updateState { copy( info = info.copy(certificationList = newCertificationList), certificationInput = "" )As per coding guidelines,
**/*.kt: 상태 관리와 단방향 데이터 흐름을 우선적으로 리뷰해줘.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/resume/impl/src/main/java/com/nonggle/impl/step1/ResumeStep1ViewModel.kt` around lines 145 - 158, The addCertification() function currently uses currentState.certificationInput.trim() without validating it, allowing whitespace-only entries; update addCertification() to compute a trimmed value from currentState.certificationInput, return immediately if that trimmed value is blank, and only then append a new CertificationTag(using the trimmed string) and call resumeStore.update (mapping certificationTitle from the new list). Also ensure the UI button enablement logic uses the same trimmed-blank check on certificationInput so the add action is disabled for whitespace-only input.feature/resume/impl/src/main/java/com/nonggle/impl/step3/ResumeStep3ViewModel.kt (1)
42-53:⚠️ Potential issue | 🟠 Major빈 성격 태그가 draft에만 저장됩니다.
Line 45에서 빈 입력이면 UI 갱신만 건너뛰고, Line 51의
resumeStore.update는 계속 실행됩니다. 결과적으로 화면에는 없는데 draft에는 빈 문자열이 남습니다. blank 체크를 함수 초반으로 올려 state/store를 같은 조건으로 갱신하세요.수정 예시
private suspend fun addPersonality() { - val newList = currentState.personalityList + PersonalityTag(personality = currentState.personality?.trim() ?: "") + val value = currentState.personality?.trim().orEmpty() + if (value.isEmpty()) return + val newList = currentState.personalityList + PersonalityTag(personality = value) updateState { - if ((personality ?: "").trim().isEmpty()) return@updateState this copy( personalityList = newList, personality = "" ) }As per coding guidelines,
**/*.kt: 상태 관리와 단방향 데이터 흐름을 우선적으로 리뷰해줘.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/resume/impl/src/main/java/com/nonggle/impl/step3/ResumeStep3ViewModel.kt` around lines 42 - 53, addPersonality 함수에서 현재 입력이 빈 문자열인 경우 UI 갱신은 건너뛰지만 resumeStore.update는 계속 실행되어 draft에 빈 태그가 저장되는 버그가 있습니다; 해결하려면 addPersonality 초반에 val trimmed = currentState.personality?.trim() ?: "" 같은 변수를 만들어 trimmed.isEmpty()면 즉시 반환하도록 하고, 그 이후에만 newList를 만들고 updateState(...) 및 resumeStore.update { it.copy(personalityList = newList.map { it.personality }) }를 호출해 UI(state)와 store가 동일한 조건으로 업데이트되게 하세요 (참조: addPersonality, currentState, personalityList, personality, updateState, resumeStore.update).feature/resume/impl/src/test/java/com/nonggle/impl/step2/ResumeStep2ViewModelTest.kt (1)
90-90:⚠️ Potential issue | 🟠 Major저장소에 저장되는
totalCareer타입이String이므로 테스트가 실패합니다
ResumeWritingModel.totalCareer는String타입이지만, 테스트에서는Period객체와 비교하고 있습니다. ViewModel의saveTempResume()에서getPeriodFormatter(totalCareer)로 포맷된 문자열을 저장하기 때문에,storedDraft.totalCareer는 "1년 2개월 3일" 형태의 String입니다.라인 90, 133-134, 148, 150 등에서
Period객체와 직접 비교하면 항상 실패합니다.ResumeStep2State.totalCareer(Period 타입)와ResumeWritingModel.totalCareer(String 타입) 간의 타입 일관성을 맞춰야 합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/resume/impl/src/test/java/com/nonggle/impl/step2/ResumeStep2ViewModelTest.kt` at line 90, The test is comparing a Period to a stored String; update the assertions to compare like-for-like by formatting the expected Period into the same String that saveTempResume() stores: call getPeriodFormatter(expectedPeriod) (or otherwise format expectedPeriod to "1년 2개월 3일") and assertEquals(formattedExpected, storedDraft.totalCareer); similarly update other assertions that compare ResumeStep2State.totalCareer (Period) vs ResumeWritingModel.totalCareer (String) so they either format the Period before comparison or parse the stored String into a Period to match the state's type, referencing ResumeWritingModel.totalCareer, ResumeStep2State.totalCareer, saveTempResume(), and getPeriodFormatter().
🧹 Nitpick comments (22)
feature/mypage/impl/src/androidTest/java/com/nonggle/mypage/impl/ExampleInstrumentedTest.kt (1)
17-23: 패키지 이름 검증이 정확히 업데이트됨테스트의 예상 패키지 이름이
com.nonggle.mypage.impl.test로 올바르게 변경되었습니다.다만 이 파일은 Android Studio가 자동 생성하는 보일러플레이트 테스트로, 실제 기능 검증 가치는 없습니다. 향후 MyPage 모듈의 실제 기능(로그아웃 플로우, UI 컴포넌트 등)을 검증하는 테스트로 교체하거나 삭제를 고려할 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/mypage/impl/src/androidTest/java/com/nonggle/mypage/impl/ExampleInstrumentedTest.kt` around lines 17 - 23, The ExampleInstrumentedTest boilerplate (class ExampleInstrumentedTest, method useAppContext) only asserts the package name and provides no real value; either replace it with meaningful instrumented tests for MyPage behaviors (e.g., logout flow, UI components) by implementing new test methods that exercise those flows, or remove the file entirely if you don't want empty boilerplate tests in the module; update or delete ExampleInstrumentedTest accordingly and ensure any test annotations/imports are adjusted to avoid unused warnings.core/designsystem/src/main/java/com/nonggle/designsystem/component/Navigation.kt (1)
50-62: LGTM!
NonggleNavigationBar의 UI 개선이 적절합니다:
Column래핑으로 인한 recomposition 오버헤드는 무시할 수준입니다.- 하드코딩 색상 대신 테마 기반 색상 사용이 유지보수에 좋습니다.
HorizontalDivider로 시각적 구분이 명확해졌습니다.향후 확장성을 고려해 divider 표시 여부를 파라미터로 제어할 수 있도록 하면 더 유연해질 수 있습니다:
♻️ Optional: divider 표시 제어 파라미터 추가
`@Composable` fun NonggleNavigationBar( modifier: Modifier = Modifier, + showDivider: Boolean = true, content: `@Composable` RowScope.() -> Unit, ) { Column { - HorizontalDivider( - thickness = 1.dp, - color = NonggleTheme.colorScheme.g_line - ) + if (showDivider) { + HorizontalDivider( + thickness = 1.dp, + color = NonggleTheme.colorScheme.g_line + ) + } NavigationBar(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/designsystem/src/main/java/com/nonggle/designsystem/component/Navigation.kt` around lines 50 - 62, Add an optional parameter (e.g., showDivider: Boolean = true) to the NonggleNavigationBar composable (or the function that renders the Column/NavigatonBar block) and use it to conditionally render the HorizontalDivider: when showDivider is true include the HorizontalDivider with the existing thickness/color, otherwise omit it; update all call sites of NonggleNavigationBar (or the constructor/provider) to pass the new parameter where specific behavior is required so default behavior remains unchanged.feature/download/impl/src/main/java/com/nonggle/feature/download/impl/DownloadScreen.kt (3)
237-237: Composable 함수 네이밍 컨벤션
deleteDialog역시 PascalCase(DeleteDialog)로 변경해야 합니다.-fun deleteDialog( +fun DeleteDialog(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/download/impl/src/main/java/com/nonggle/feature/download/impl/DownloadScreen.kt` at line 237, Rename the composable function deleteDialog to PascalCase DeleteDialog to follow composable naming conventions: update the function declaration (deleteDialog -> DeleteDialog), refactor all call sites and imports/usages that reference deleteDialog to use DeleteDialog, and run a compile to ensure any lambda parameters or previews referring to the old name are updated as well.
87-94: 불필요한 Boolean 비교 제거
isLoading과isError가 nullable Boolean인 경우== true비교가 필요하지만, non-null Boolean이라면 직접 사용하는 것이 더 깔끔합니다. 타입 확인이 필요합니다.♻️ non-null Boolean인 경우 권장 수정
- if (uiState.isLoading == true) { + if (uiState.isLoading) { ... - } else if (uiState.isError == true) { + } else if (uiState.isError) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/download/impl/src/main/java/com/nonggle/feature/download/impl/DownloadScreen.kt` around lines 87 - 94, The conditional checks in DownloadScreen are using unnecessary "== true" (uiState.isLoading == true and uiState.isError == true); confirm that uiState.isLoading and uiState.isError are non-null Booleans and replace those comparisons with direct boolean checks (if (uiState.isLoading) and if (uiState.isError)) in the composable to simplify the code and improve readability, keeping the existing Box/CircularProgressIndicator and error branch intact.
130-130: Composable 함수 네이밍 컨벤션
resumeItem은 Composable 함수이므로 PascalCase(ResumeItem)를 사용해야 합니다. Compose 컨벤션에서 Composable 함수는 명사형 PascalCase를 권장합니다.-fun resumeItem( +fun ResumeItem(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/download/impl/src/main/java/com/nonggle/feature/download/impl/DownloadScreen.kt` at line 130, The Composable function resumeItem should be renamed to PascalCase ResumeItem; update the function declaration (resumeItem -> ResumeItem) and any `@Composable` annotation present, then update all call sites and imports/usages to use ResumeItem instead of resumeItem to follow Compose naming conventions and keep the function as a noun-style Composable.core/designsystem/src/main/java/com/nonggle/designsystem/component/TopAppBar.kt (1)
63-66: 파라미터 순서 개선 권장Compose 컨벤션상 필수 파라미터(
appBarTitle)가 선택적 파라미터(modifier)보다 앞에 위치하는 것이 일반적입니다. 현재 순서도 동작하지만, API 일관성을 위해 순서 변경을 고려해 보세요.♻️ 권장 수정
fun NonggleMainTopAppBar( - modifier: Modifier = Modifier, appBarTitle: String, + modifier: Modifier = Modifier, ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/designsystem/src/main/java/com/nonggle/designsystem/component/TopAppBar.kt` around lines 63 - 66, Reorder the parameters of the composable function NonggleMainTopAppBar so the required appBarTitle appears before the optional modifier: change the signature to take appBarTitle first and Modifier = Modifier second; update all call sites that relied on positional arguments (or prefer named args) to match the new parameter order to preserve API consistency with Compose conventions while keeping modifier optional with its default.app/src/main/res/values/strings.xml (1)
5-5: 리소스 키 이름과 값 불일치
nav_item_setting이라는 키 이름이 "마이페이지" 값과 맞지 않습니다. 향후 유지보수를 위해nav_item_mypage로 변경하는 것이 좋습니다. 단, 이 변경은 코드베이스 전반에 영향을 줄 수 있으므로 별도 작업으로 진행해도 됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/res/values/strings.xml` at line 5, Rename the string resource key nav_item_setting to nav_item_mypage and update all references to it across the codebase (e.g., usages in layouts, menu XML, and Kotlin/Java code that call R.string.nav_item_setting or getString(R.string.nav_item_setting)); ensure the XML value remains "마이페이지", update any proguard/resource references if present, and run a project-wide search/replace to avoid missed references and build errors.app/src/main/java/com/nonggle/nonggleresume/ui/NonggleApp.kt (1)
48-55: 오프라인 Toast 구현 - 개선 가능현재 구현은 동작하지만,
stringResource가 매 recomposition마다 호출됩니다. 성능 영향은 미미하지만,LaunchedEffect내부에서 context를 통해 직접 가져오면 불필요한 호출을 줄일 수 있습니다.♻️ 개선 제안 (선택)
val context = LocalContext.current -val offlineToastMessage = stringResource(R.string.offline_Message) LaunchedEffect(isOffline) { if (isOffline) { - Toast.makeText(context, offlineToastMessage, Toast.LENGTH_LONG).show() + Toast.makeText(context, context.getString(R.string.offline_Message), Toast.LENGTH_LONG).show() } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/nonggle/nonggleresume/ui/NonggleApp.kt` around lines 48 - 55, The current code calls stringResource(...) on every recomposition; instead, inside the LaunchedEffect observing isOffline use the LocalContext.current (context) to fetch the string once (e.g., context.getString(R.string.offline_Message)) or use remember to cache it, then call Toast.makeText(context, message, Toast.LENGTH_LONG).show(); update references in NonggleApp.kt around LocalContext.current, LaunchedEffect(isOffline), offlineToastMessage and Toast.makeText to use the locally retrieved/cached message to avoid repeated stringResource calls.feature/mypage/impl/build.gradle.kts (1)
11-28: 의존성 정렬 및 그룹화 권장가독성과 유지보수성을 위해 의존성을 카테고리별로 그룹화하는 것을 권장합니다: AndroidX/testing → external libraries → project modules.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/mypage/impl/build.gradle.kts` around lines 11 - 28, The dependencies block is ungrouped; reorder and group entries in the dependencies { ... } block so AndroidX/testing dependencies (implementation(libs.androidx.core.ktx), implementation(libs.androidx.appcompat), testImplementation(libs.junit), androidTestImplementation(...)) come first, then external libraries (implementation(libs.coil.compose), implementation(libs.coil.network.okhttp)), and finally project modules (implementation(project(":core:designsystem")), implementation(project(":core:ui")), implementation(project(":core:domain")), implementation(project(":core:common")), implementation(project(":feature:mypage:api")), implementation(project(":feature:login:impl")), implementation(project(":core:model"))), adding a blank line between groups for readability and optionally sorting alphabetically within each group.feature/mypage/api/build.gradle.kts (1)
11-13:navigation3.runtime을api로 변경 권장
MyPageNavKey가NavKey인터페이스를 구현하고 public으로 노출되므로, 이 모듈을 의존하는 다른 모듈에서도NavKey타입에 접근할 수 있어야 합니다.implementation대신api를 사용하면 transitive dependency로 노출됩니다.♻️ 수정 제안
dependencies { - implementation(libs.navigation3.runtime) + api(libs.navigation3.runtime) }As per coding guidelines: "api / implementation 선택"을 빌드 성능과 모듈화 관점에서 리뷰.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/mypage/api/build.gradle.kts` around lines 11 - 13, The dependency on navigation3.runtime is declared with implementation which prevents transitive exposure of NavKey; since MyPageNavKey implements the public NavKey interface and consumers of this module need that type, change the dependency declaration for libs.navigation3.runtime from implementation to api in build.gradle.kts so NavKey is available transitively to dependent modules (locate the dependencies block containing implementation(libs.navigation3.runtime) and replace it with an api declaration).app/src/main/java/com/nonggle/nonggleresume/MainActivity.kt (1)
102-105:systemBarsPadding()과statusBarsPadding()중복 적용
systemBarsPadding()은 이미 status bar와 navigation bar 패딩을 모두 포함합니다.statusBarsPadding()을 추가로 적용하면 status bar 영역에 대한 패딩이 두 번 적용되어 불필요한 여백이 생깁니다.♻️ 수정 제안
Surface( modifier = Modifier .fillMaxSize() - .systemBarsPadding() - .statusBarsPadding(), + .systemBarsPadding(), color = NonggleTheme.colorScheme.white )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/nonggle/nonggleresume/MainActivity.kt` around lines 102 - 105, The modifier chain currently applies both systemBarsPadding() and statusBarsPadding() (Modifier.fillMaxSize().systemBarsPadding().statusBarsPadding()), causing double top padding; remove the redundant statusBarsPadding() call and keep systemBarsPadding() so the system bars (status + navigation) are padded exactly once.feature/mypage/impl/src/main/java/com/nonggle/mypage/impl/navigation/MyPageEntryProvider.kt (1)
9-12: 미사용 파라미터navigator
navigator파라미터가 선언되었지만 사용되지 않고 있습니다. 현재 필요 없다면 제거하거나, 향후 화면 이동 기능에 사용될 예정이라면 TODO 주석을 추가하는 것이 좋습니다.♻️ 미사용 파라미터 제거
-fun EntryProviderScope<NavKey>.myPageEntryProvider(navigator: Navigator) { +fun EntryProviderScope<NavKey>.myPageEntryProvider() { entry<MyPageNavKey> { MyPageScreen() } }Based on learnings: "Login feature의 api/impl 분리 전략: api 모듈에 NavKey만 배치, impl 모듈에 LoginEntryProvider와 LoginScreen 등 실구현 배치."에 따라 구조는 적절합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/mypage/impl/src/main/java/com/nonggle/mypage/impl/navigation/MyPageEntryProvider.kt` around lines 9 - 12, The function EntryProviderScope<NavKey>.myPageEntryProvider declares an unused parameter navigator; remove the navigator parameter from the function signature (and any callers) if navigation isn't needed, or keep it and add a clear TODO comment inside myPageEntryProvider indicating future use for screen navigation; locate the function by name myPageEntryProvider and the related symbols MyPageNavKey and MyPageScreen when making the change.app/src/main/java/com/nonggle/nonggleresume/navigation/TopLevelNavItem.kt (1)
30-34: 리소스 네이밍 불일치
MYPAGE항목이NonggleIcons.settingSelected/Unselected와R.string.nav_item_setting을 참조하고 있습니다. 기능적으로는 문제없지만, 나중에 유지보수 시 혼란을 줄 수 있습니다.아이콘과 문자열 리소스 이름을
mypage로 변경하는 것을 고려해 보세요:
NonggleIcons.mypageSelected/mypageUnselectedR.string.nav_item_mypage🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/nonggle/nonggleresume/navigation/TopLevelNavItem.kt` around lines 30 - 34, Rename the icon and string resources for the MYPAGE nav item to match the feature name and update references: change NonggleIcons.settingSelected/settingUnselected to NonggleIcons.mypageSelected/mypageUnselected and change R.string.nav_item_setting to R.string.nav_item_mypage, then update the TopLevelNavItem constant MYPAGE to use these new symbols and update the actual resource definitions (icons object entries and strings.xml key) and any other usages that reference the old names.feature/login/impl/src/main/java/com/nonggle/impl/KakaoLoginManager.kt (1)
56-67:suspendCancellableCoroutine사용을 권장합니다.현재
suspendCoroutine은 취소를 지원하지 않습니다. 코루틴이 취소되어도 Kakao SDK 콜백은 계속 실행됩니다. 로그아웃의 경우 치명적이지 않지만, 코루틴 취소 시 리소스 정리를 위해suspendCancellableCoroutine을 사용하는 것이 더 안전합니다.♻️ 취소 가능한 코루틴으로 변경
+import kotlin.coroutines.cancellation.CancellationException +import kotlinx.coroutines.suspendCancellableCoroutine suspend fun kakaoLogout(): Result<Unit> { return runCatching { - suspendCoroutine { continuation -> + suspendCancellableCoroutine { continuation -> UserApiClient.instance.logout { error -> + if (!continuation.isActive) return@logout when { error != null -> continuation.resumeWithException(error) else -> continuation.resume(Unit) } } } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/login/impl/src/main/java/com/nonggle/impl/KakaoLoginManager.kt` around lines 56 - 67, Replace suspendCoroutine with suspendCancellableCoroutine in kakaoLogout and adapt the callback handling so the coroutine cancellation is respected: inside suspendCancellableCoroutine call UserApiClient.instance.logout { error -> ... } and before calling continuation.resume or continuation.resumeWithException verify the CancellableContinuation is not cancelled (or skip resuming if cancelled); also register continuation.invokeOnCancellation { /* cleanup if possible */ } to handle cancellation (e.g., clear any SDK listeners or no-op if SDK has no cancel API). This ensures kakaoLogout (function kakaoLogout and the UserApiClient.instance.logout callback) supports coroutine cancellation and avoids resuming a cancelled continuation.feature/mypage/impl/src/main/java/com/nonggle/mypage/impl/MyPageScreen.kt (1)
77-78: Composable 함수 네이밍 컨벤션 위반:logoutDialog→LogoutDialogCompose의 네이밍 컨벤션에 따르면,
@Composable함수는 PascalCase를 사용해야 합니다. 이는 Compose 컴파일러의 최적화와 코드 가독성에 도움이 됩니다.✏️ 제안된 수정
`@Composable` -fun logoutDialog( +fun LogoutDialog( onDismiss: () -> Unit = {}, onConfirm: () -> Unit = {}, ) {호출부도 함께 수정:
if(showLogoutDialog) { - logoutDialog( + LogoutDialog( onDismiss = { showLogoutDialog = false },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/mypage/impl/src/main/java/com/nonggle/mypage/impl/MyPageScreen.kt` around lines 77 - 78, Rename the `@Composable` function logoutDialog to PascalCase as LogoutDialog and update every call site that references logoutDialog to use LogoutDialog instead; specifically modify the function declaration named logoutDialog and any usages (e.g., invocations inside MyPageScreen or other composables) so the symbol is consistently renamed to LogoutDialog to follow Compose naming conventions.feature/resume/impl/src/main/java/com/nonggle/impl/main/ResumeMainScreen.kt (1)
44-50:LaunchedEffectkey를Unit으로 변경 권장다른 Screen들과 동일하게, effect 수집을 위한
LaunchedEffect의 key는Unit이 적절합니다.제안된 수정
- LaunchedEffect(viewModel.effect) { + LaunchedEffect(Unit) { viewModel.effect.collect { effect -> when(effect) { is ResumeMainEffect.NavigateToComplete -> navigateToComplete() } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/resume/impl/src/main/java/com/nonggle/impl/main/ResumeMainScreen.kt` around lines 44 - 50, LaunchedEffect currently uses viewModel.effect as its key; change it to use Unit instead so the effect collector runs once like other screens: wrap the existing viewModel.effect.collect { ... } block inside LaunchedEffect(Unit) while keeping the same collection logic (collecting from viewModel.effect and handling ResumeMainEffect.NavigateToComplete by calling navigateToComplete()) so the unique symbols to update are LaunchedEffect, viewModel.effect, ResumeMainEffect.NavigateToComplete, and navigateToComplete.feature/resume/impl/src/main/java/com/nonggle/impl/step2/ResumeStep2ViewModel.kt (2)
114-158:saveTempResume과deleteTempResume중복 로직 통합 권장두 함수의 로직이 거의 동일합니다. 하나의 private 함수로 통합하면 유지보수성이 향상됩니다.
제안된 리팩토링
+ private suspend fun updateDraftCareerList(newCareerList: List<CareerFormData>) { + val totalCareer = sumCareerPeriod(newCareerList) + resumeStore.update { + it.copy( + careerList = newCareerList.map { item -> + ResumeWritingModel.Career( + careerStartDate = item.careerStartDate ?: LocalDate.now(), + careerEndDate = item.careerEndDate ?: LocalDate.now(), + careerPeriod = getPeriodFormatter( + period = Period.between(item.careerStartDate, item.careerEndDate) + ), + careerDescription = item.careerDescription, + careerDetail = item.careerDetail + ) + }, + totalCareer = getPeriodFormatter(totalCareer) + ) + } + } private suspend fun addCareerItem(data: CareerFormData) { val newCareerList = currentState.careerList + data updateState { copy( careerList = this.careerList + data, careerFormData = CareerFormData(), totalCareer = sumCareerPeriod(newCareerList) ) } - saveTempResume(newCareerList) + updateDraftCareerList(newCareerList) } private suspend fun deleteCareerItem(id: String) { val newCareerList = currentState.careerList.filter { it.id != id } updateState { copy( totalCareer = sumCareerPeriod(newCareerList), careerList = newCareerList ) } - deleteTempResume(newCareerList) + updateDraftCareerList(newCareerList) } - - private suspend fun saveTempResume(newCareerList: List<CareerFormData>) { ... } - - private suspend fun deleteTempResume(newCareerList: List<CareerFormData>) { ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/resume/impl/src/main/java/com/nonggle/impl/step2/ResumeStep2ViewModel.kt` around lines 114 - 158, Both saveTempResume and deleteTempResume contain identical logic; consolidate them into a single private helper (e.g., updateTempResume) that accepts newCareerList: List<CareerFormData>, computes totalCareer via sumCareerPeriod(newCareerList), maps each CareerFormData to ResumeWritingModel.Career (using careerStartDate ?: LocalDate.now(), careerEndDate ?: LocalDate.now(), careerPeriod = getPeriodFormatter(period = Period.between(it.careerStartDate, it.careerEndDate)), careerDescription, careerDetail) and calls resumeStore.update { it.copy(careerList = mappedList, totalCareer = getPeriodFormatter(totalCareer)) }; replace calls to saveTempResume and deleteTempResume with this new helper and remove the duplicated methods.
8-8: 사용되지 않는 import 제거
ResumeStep1Effect가 import되어 있지만 이 파일에서 사용되지 않습니다.제안된 수정
-import com.nonggle.resume.impl.step1.ResumeStep1Effect🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/resume/impl/src/main/java/com/nonggle/impl/step2/ResumeStep2ViewModel.kt` at line 8, Remove the unused import of ResumeStep1Effect from ResumeStep2ViewModel: locate the import statement "import com.nonggle.resume.impl.step1.ResumeStep1Effect" in the ResumeStep2ViewModel file and delete it so the file no longer contains an unused import; run a quick compile or linter to confirm no references to ResumeStep1Effect remain in the class.feature/resume/impl/src/main/java/com/nonggle/impl/step1/ResumeStep1Screen.kt (1)
82-93:LaunchedEffectkey를Unit으로 변경 권장Step2Screen과 동일하게,
viewModel.effect를 key로 사용하는 대신Unit을 사용하여 effect 수집을 한 번만 설정하는 것이 적절합니다.제안된 수정
- LaunchedEffect(viewModel.effect) { + LaunchedEffect(Unit) { viewModel.effect.collect { effect -> when (effect) { is ResumeStep1Effect.SendImageVolumeOverFlowMessage -> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/resume/impl/src/main/java/com/nonggle/impl/step1/ResumeStep1Screen.kt` around lines 82 - 93, The LaunchedEffect is currently keyed on viewModel.effect which can re-subscribe unexpectedly; change the key to Unit so the collection is set up once: in ResumeStep1Screen replace LaunchedEffect(viewModel.effect) with LaunchedEffect(Unit) and keep the existing viewModel.effect.collect { effect -> when (effect) { is ResumeStep1Effect.SendImageVolumeOverFlowMessage -> ...; is ResumeStep1Effect.SendBirthDateNotValidMessage -> ... } } so the Toast handling for ResumeStep1Effect remains unchanged but the collector is only launched once.feature/resume/impl/src/main/java/com/nonggle/impl/main/ResumeMainViewModel.kt (1)
36-50:validateResume간소화 가능현재 구현은 명확하지만,
all()함수를 사용하면 더 간결하게 작성할 수 있습니다.선택적 리팩토링 제안
private fun validateResume(resume: ResumeWritingModel): Boolean { - val isNameValid = resume.userName.isNotEmpty() - val isBirthDateValid = resume.birthDate.isNotEmpty() - val isGenderValid = resume.gender.isNotEmpty() - val isIntroduceDetailValid = resume.introduceDetail.isNotEmpty() - val isPersonalityListValid = resume.personalityList.isNotEmpty() - val isIntroduceValid = resume.introduce.isNotEmpty() - - return isNameValid && - isBirthDateValid && - isGenderValid && - isIntroduceValid && - isIntroduceDetailValid && - isPersonalityListValid + return listOf( + resume.userName, + resume.birthDate, + resume.gender, + resume.introduce, + resume.introduceDetail + ).all { it.isNotEmpty() } && resume.personalityList.isNotEmpty() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/resume/impl/src/main/java/com/nonggle/impl/main/ResumeMainViewModel.kt` around lines 36 - 50, The validateResume function can be simplified by replacing the multiple boolean locals with a single concise check using Kotlin's all(): build a collection of the string properties to validate (resume.userName, resume.birthDate, resume.gender, resume.introduce, resume.introduceDetail, resume.personalityList) and return collection.all { it.isNotEmpty() } inside validateResume; this keeps the same semantics but reduces verbosity while still referencing the existing validateResume and ResumeWritingModel properties.feature/resume/impl/src/main/java/com/nonggle/impl/step2/ResumeStep2Screen.kt (1)
52-60:LaunchedEffectkey를Unit으로 변경 권장
viewModel.effect를 key로 사용하면 ViewModel 인스턴스가 변경될 때마다 LaunchedEffect가 재시작됩니다. effect Flow 수집은 한 번만 설정하면 되므로Unit을 key로 사용하는 것이 적절합니다.제안된 수정
- LaunchedEffect(viewModel.effect) { + LaunchedEffect(Unit) { viewModel.effect.collect { effect -> when(effect) { is ResumeStep2Effect.SendStartCareerNotValidMessage -> { Toast.makeText(context, startCareerDateNotValidMessage, Toast.LENGTH_SHORT).show() } } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/resume/impl/src/main/java/com/nonggle/impl/step2/ResumeStep2Screen.kt` around lines 52 - 60, In ResumeStep2Screen.kt update the LaunchedEffect usage so it doesn't restart when the ViewModel instance changes: replace LaunchedEffect(viewModel.effect) that wraps viewModel.effect.collect { ... } with a LaunchedEffect keyed to Unit (e.g., LaunchedEffect(Unit)) so the flow collection is established once; keep the inner collect block (handling ResumeStep2Effect.SendStartCareerNotValidMessage and showing the Toast) unchanged and ensure you're collecting from viewModel.effect inside that Unit-keyed LaunchedEffect.feature/resume/impl/src/main/res/values/strings.xml (1)
12-12: String 리소스 네이밍 일관성 확인 필요
resume1Screen_birthdate_notValid와resume2Screen_startCareerDate_NotValid에서notValidvsNotValid케이싱이 다릅니다. 일관성을 위해 동일한 패턴을 사용하는 것이 좋습니다.Also applies to: 43-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/resume/impl/src/main/res/values/strings.xml` at line 12, The two string resource names use inconsistent casing: resume1Screen_birthdate_notValid (lowercase "notValid") vs resume2Screen_startCareerDate_NotValid (capital "NotValid"); rename resume2Screen_startCareerDate_NotValid to resume2Screen_startCareerDate_notValid (or vice versa if you prefer the other pattern) and update all usages and translations accordingly (search for both keys including the occurrence mentioned at lines 43-43) so the naming pattern for the "notValid" suffix is consistent across the project.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/nonggle/nonggleresume/MainActivityViewModel.kt`:
- Around line 55-61: The refreshLoginState function currently unconditionally
sets _uiState to Loading which causes a flash on every resume; change
refreshLoginState (and callers) to accept a showLoading: Boolean = false and
only set _uiState = MainActivityUiState.Loading when showLoading is true or when
current _uiState is not already Success, then proceed to read
loginRepository.isLoggedIn().first() and set _uiState =
MainActivityUiState.Success; also ensure the potentially blocking
loginRepository.isLoggedIn().first() runs off the Main thread by either making
the repository switch dispatchers internally or wrapping the call in
withContext(Dispatchers.IO) inside the coroutine to avoid main-thread I/O.
In
`@core/data/src/main/java/com/nonggle/data/repositoryimpl/LoginRepositoryImpl.kt`:
- Around line 44-52: The current logOut in LoginRepositoryImpl always calls
tokenManager.deleteToken() regardless of loginService.logout() result; change
logOut so it first calls val response = loginService.logout(), then only call
tokenManager.deleteToken() when response is AppResult.Success (i.e., on
successful server logout) and return AppResult.Success(Unit); for the
AppResult.Error branch, return the error without deleting the token (or if you
intentionally want to always delete, add a clear comment and change the return
to wrap the logout error with a marker that the token was deleted); key symbols:
LoginRepositoryImpl.logOut, loginService.logout(), tokenManager.deleteToken(),
MainActivityViewModel.SessionExpired.
In `@feature/mypage/impl/build.gradle.kts`:
- Line 26: The build is directly depending on feature:login:impl which breaks
the api/impl separation; replace the
implementation(project(":feature:login:impl")) dependency in build.gradle.kts
with a dependency on the login API (e.g.,
implementation(project(":feature:login:api"))) or, if you need domain-level
types, depend on core:domain instead and access LoginRepository/LogoutUseCase
through that API/domain layer so only API modules are referenced from the mypage
impl.
In
`@feature/mypage/impl/src/main/java/com/nonggle/mypage/impl/MyPageViewModel.kt`:
- Around line 35-40: The code currently emits
authEventBus.emit(AuthEvent.LoggedOut) unconditionally even when apiResult is
AppResult.Error or kakaoResult.isFailure; change the logic in the logout flow
(surrounding apiResult, kakaoResult, postEffect(MyPageEffect.LogoutFailed),
authEventBus.emit(AuthEvent.LoggedOut), updateState { copy(isLoading = false) })
so that you only call authEventBus.emit(AuthEvent.LoggedOut) when both apiResult
is not AppResult.Error and kakaoResult.isFailure is false (i.e., logout
succeeded); keep calling postEffect(MyPageEffect.LogoutFailed) on failure and
always ensure updateState { copy(isLoading = false) } runs after handling
success or failure.
- Line 8: MyPageViewModel currently imports and uses KakaoLoginManager from
feature/login/impl, which breaks module boundaries; remove the direct import and
instead depend on an abstraction: either (A) add a KakaoLoginManager interface
(or IKakaoLogout) to feature/login/api and have feature/login/impl implement it,
then inject that interface into MyPageViewModel (constructor or DI) in place of
the concrete type, or (B) extend the existing LogoutUseCase to include Kakao
logout behavior and have MyPageViewModel call LogoutUseCase only; update
feature/login/impl to implement the new api interface or extended use case and
update MyPageViewModel to use the API-facing symbol (the new interface or
LogoutUseCase) rather than KakaoLoginManager.
In
`@feature/resume/impl/src/main/java/com/nonggle/impl/step1/ResumeStep1ViewModel.kt`:
- Around line 23-25: 현재 ResumeStep1ViewModel에서 SelectImage 이벤트를 매번 새
코루틴(viewModelScope.launch)으로 처리해 이미지 메타 조회가 지연되면 이전 선택이 나중 선택으로 덮어써질 수 있으니,
SelectImage 처리를 전용 cancellable Job으로 변경하거나 이벤트 디스패치를 동기화하고 메타 조회 부분만 취소 가능한
코루틴으로 분리하세요; 구체적으로 ResumeStep1ViewModel의 SelectImage 분기에서 기존에 보관한
imageSelectionJob을 취소하고 새 Job으로 setProfileImageUri와 이후 이미지 메타 조회(현재 메타 조회를 수행하는
함수/블록)를 실행하거나, 이벤트 핸들러는 즉시 setProfileImageUri만 수행하고 메타 조회를 별도의 cancellable 함수로
옮겨 imageSelectionJob으로 관리해 중복/역전이 발생하지 않도록 보장하세요.
---
Outside diff comments:
In
`@feature/download/impl/src/main/java/com/nonggle/feature/download/impl/DownloadScreen.kt`:
- Line 1: 파일 DownloadScreen.kt의 패키지 선언이 실제 디렉토리와 맞지 않습니다; DownloadScreen.kt
(참조되는 심볼: DownloadScreen) 파일을 현재 디렉토리 구조에 맞게 수정하세요 — 방법1: 파일 내 패키지 선언을
com.nonggle.feature.download.impl 으로 변경하거나, 방법2: 현재 패키지
com.nonggle.feature.download.impl.navigation을 유지하려면 파일을 navigation 서브디렉토리로
이동(참고: DownLoadEntryProvider.kt는 navigation에 있음)하여 패키지와 파일 경로가 일치하도록 만드세요.
In
`@feature/resume/impl/src/main/java/com/nonggle/impl/step1/ResumeStep1ViewModel.kt`:
- Around line 145-158: The addCertification() function currently uses
currentState.certificationInput.trim() without validating it, allowing
whitespace-only entries; update addCertification() to compute a trimmed value
from currentState.certificationInput, return immediately if that trimmed value
is blank, and only then append a new CertificationTag(using the trimmed string)
and call resumeStore.update (mapping certificationTitle from the new list). Also
ensure the UI button enablement logic uses the same trimmed-blank check on
certificationInput so the add action is disabled for whitespace-only input.
In
`@feature/resume/impl/src/main/java/com/nonggle/impl/step3/ResumeStep3ViewModel.kt`:
- Around line 42-53: addPersonality 함수에서 현재 입력이 빈 문자열인 경우 UI 갱신은 건너뛰지만
resumeStore.update는 계속 실행되어 draft에 빈 태그가 저장되는 버그가 있습니다; 해결하려면 addPersonality 초반에
val trimmed = currentState.personality?.trim() ?: "" 같은 변수를 만들어
trimmed.isEmpty()면 즉시 반환하도록 하고, 그 이후에만 newList를 만들고 updateState(...) 및
resumeStore.update { it.copy(personalityList = newList.map { it.personality })
}를 호출해 UI(state)와 store가 동일한 조건으로 업데이트되게 하세요 (참조: addPersonality, currentState,
personalityList, personality, updateState, resumeStore.update).
In
`@feature/resume/impl/src/test/java/com/nonggle/impl/step2/ResumeStep2ViewModelTest.kt`:
- Line 90: The test is comparing a Period to a stored String; update the
assertions to compare like-for-like by formatting the expected Period into the
same String that saveTempResume() stores: call
getPeriodFormatter(expectedPeriod) (or otherwise format expectedPeriod to "1년
2개월 3일") and assertEquals(formattedExpected, storedDraft.totalCareer); similarly
update other assertions that compare ResumeStep2State.totalCareer (Period) vs
ResumeWritingModel.totalCareer (String) so they either format the Period before
comparison or parse the stored String into a Period to match the state's type,
referencing ResumeWritingModel.totalCareer, ResumeStep2State.totalCareer,
saveTempResume(), and getPeriodFormatter().
---
Nitpick comments:
In `@app/src/main/java/com/nonggle/nonggleresume/MainActivity.kt`:
- Around line 102-105: The modifier chain currently applies both
systemBarsPadding() and statusBarsPadding()
(Modifier.fillMaxSize().systemBarsPadding().statusBarsPadding()), causing double
top padding; remove the redundant statusBarsPadding() call and keep
systemBarsPadding() so the system bars (status + navigation) are padded exactly
once.
In `@app/src/main/java/com/nonggle/nonggleresume/navigation/TopLevelNavItem.kt`:
- Around line 30-34: Rename the icon and string resources for the MYPAGE nav
item to match the feature name and update references: change
NonggleIcons.settingSelected/settingUnselected to
NonggleIcons.mypageSelected/mypageUnselected and change
R.string.nav_item_setting to R.string.nav_item_mypage, then update the
TopLevelNavItem constant MYPAGE to use these new symbols and update the actual
resource definitions (icons object entries and strings.xml key) and any other
usages that reference the old names.
In `@app/src/main/java/com/nonggle/nonggleresume/ui/NonggleApp.kt`:
- Around line 48-55: The current code calls stringResource(...) on every
recomposition; instead, inside the LaunchedEffect observing isOffline use the
LocalContext.current (context) to fetch the string once (e.g.,
context.getString(R.string.offline_Message)) or use remember to cache it, then
call Toast.makeText(context, message, Toast.LENGTH_LONG).show(); update
references in NonggleApp.kt around LocalContext.current,
LaunchedEffect(isOffline), offlineToastMessage and Toast.makeText to use the
locally retrieved/cached message to avoid repeated stringResource calls.
In `@app/src/main/res/values/strings.xml`:
- Line 5: Rename the string resource key nav_item_setting to nav_item_mypage and
update all references to it across the codebase (e.g., usages in layouts, menu
XML, and Kotlin/Java code that call R.string.nav_item_setting or
getString(R.string.nav_item_setting)); ensure the XML value remains "마이페이지",
update any proguard/resource references if present, and run a project-wide
search/replace to avoid missed references and build errors.
In
`@core/designsystem/src/main/java/com/nonggle/designsystem/component/Navigation.kt`:
- Around line 50-62: Add an optional parameter (e.g., showDivider: Boolean =
true) to the NonggleNavigationBar composable (or the function that renders the
Column/NavigatonBar block) and use it to conditionally render the
HorizontalDivider: when showDivider is true include the HorizontalDivider with
the existing thickness/color, otherwise omit it; update all call sites of
NonggleNavigationBar (or the constructor/provider) to pass the new parameter
where specific behavior is required so default behavior remains unchanged.
In
`@core/designsystem/src/main/java/com/nonggle/designsystem/component/TopAppBar.kt`:
- Around line 63-66: Reorder the parameters of the composable function
NonggleMainTopAppBar so the required appBarTitle appears before the optional
modifier: change the signature to take appBarTitle first and Modifier = Modifier
second; update all call sites that relied on positional arguments (or prefer
named args) to match the new parameter order to preserve API consistency with
Compose conventions while keeping modifier optional with its default.
In
`@feature/download/impl/src/main/java/com/nonggle/feature/download/impl/DownloadScreen.kt`:
- Line 237: Rename the composable function deleteDialog to PascalCase
DeleteDialog to follow composable naming conventions: update the function
declaration (deleteDialog -> DeleteDialog), refactor all call sites and
imports/usages that reference deleteDialog to use DeleteDialog, and run a
compile to ensure any lambda parameters or previews referring to the old name
are updated as well.
- Around line 87-94: The conditional checks in DownloadScreen are using
unnecessary "== true" (uiState.isLoading == true and uiState.isError == true);
confirm that uiState.isLoading and uiState.isError are non-null Booleans and
replace those comparisons with direct boolean checks (if (uiState.isLoading) and
if (uiState.isError)) in the composable to simplify the code and improve
readability, keeping the existing Box/CircularProgressIndicator and error branch
intact.
- Line 130: The Composable function resumeItem should be renamed to PascalCase
ResumeItem; update the function declaration (resumeItem -> ResumeItem) and any
`@Composable` annotation present, then update all call sites and imports/usages to
use ResumeItem instead of resumeItem to follow Compose naming conventions and
keep the function as a noun-style Composable.
In `@feature/login/impl/src/main/java/com/nonggle/impl/KakaoLoginManager.kt`:
- Around line 56-67: Replace suspendCoroutine with suspendCancellableCoroutine
in kakaoLogout and adapt the callback handling so the coroutine cancellation is
respected: inside suspendCancellableCoroutine call UserApiClient.instance.logout
{ error -> ... } and before calling continuation.resume or
continuation.resumeWithException verify the CancellableContinuation is not
cancelled (or skip resuming if cancelled); also register
continuation.invokeOnCancellation { /* cleanup if possible */ } to handle
cancellation (e.g., clear any SDK listeners or no-op if SDK has no cancel API).
This ensures kakaoLogout (function kakaoLogout and the
UserApiClient.instance.logout callback) supports coroutine cancellation and
avoids resuming a cancelled continuation.
In `@feature/mypage/api/build.gradle.kts`:
- Around line 11-13: The dependency on navigation3.runtime is declared with
implementation which prevents transitive exposure of NavKey; since MyPageNavKey
implements the public NavKey interface and consumers of this module need that
type, change the dependency declaration for libs.navigation3.runtime from
implementation to api in build.gradle.kts so NavKey is available transitively to
dependent modules (locate the dependencies block containing
implementation(libs.navigation3.runtime) and replace it with an api
declaration).
In `@feature/mypage/impl/build.gradle.kts`:
- Around line 11-28: The dependencies block is ungrouped; reorder and group
entries in the dependencies { ... } block so AndroidX/testing dependencies
(implementation(libs.androidx.core.ktx),
implementation(libs.androidx.appcompat), testImplementation(libs.junit),
androidTestImplementation(...)) come first, then external libraries
(implementation(libs.coil.compose), implementation(libs.coil.network.okhttp)),
and finally project modules (implementation(project(":core:designsystem")),
implementation(project(":core:ui")), implementation(project(":core:domain")),
implementation(project(":core:common")),
implementation(project(":feature:mypage:api")),
implementation(project(":feature:login:impl")),
implementation(project(":core:model"))), adding a blank line between groups for
readability and optionally sorting alphabetically within each group.
In
`@feature/mypage/impl/src/androidTest/java/com/nonggle/mypage/impl/ExampleInstrumentedTest.kt`:
- Around line 17-23: The ExampleInstrumentedTest boilerplate (class
ExampleInstrumentedTest, method useAppContext) only asserts the package name and
provides no real value; either replace it with meaningful instrumented tests for
MyPage behaviors (e.g., logout flow, UI components) by implementing new test
methods that exercise those flows, or remove the file entirely if you don't want
empty boilerplate tests in the module; update or delete ExampleInstrumentedTest
accordingly and ensure any test annotations/imports are adjusted to avoid unused
warnings.
In `@feature/mypage/impl/src/main/java/com/nonggle/mypage/impl/MyPageScreen.kt`:
- Around line 77-78: Rename the `@Composable` function logoutDialog to PascalCase
as LogoutDialog and update every call site that references logoutDialog to use
LogoutDialog instead; specifically modify the function declaration named
logoutDialog and any usages (e.g., invocations inside MyPageScreen or other
composables) so the symbol is consistently renamed to LogoutDialog to follow
Compose naming conventions.
In
`@feature/mypage/impl/src/main/java/com/nonggle/mypage/impl/navigation/MyPageEntryProvider.kt`:
- Around line 9-12: The function EntryProviderScope<NavKey>.myPageEntryProvider
declares an unused parameter navigator; remove the navigator parameter from the
function signature (and any callers) if navigation isn't needed, or keep it and
add a clear TODO comment inside myPageEntryProvider indicating future use for
screen navigation; locate the function by name myPageEntryProvider and the
related symbols MyPageNavKey and MyPageScreen when making the change.
In `@feature/resume/impl/src/main/java/com/nonggle/impl/main/ResumeMainScreen.kt`:
- Around line 44-50: LaunchedEffect currently uses viewModel.effect as its key;
change it to use Unit instead so the effect collector runs once like other
screens: wrap the existing viewModel.effect.collect { ... } block inside
LaunchedEffect(Unit) while keeping the same collection logic (collecting from
viewModel.effect and handling ResumeMainEffect.NavigateToComplete by calling
navigateToComplete()) so the unique symbols to update are LaunchedEffect,
viewModel.effect, ResumeMainEffect.NavigateToComplete, and navigateToComplete.
In
`@feature/resume/impl/src/main/java/com/nonggle/impl/main/ResumeMainViewModel.kt`:
- Around line 36-50: The validateResume function can be simplified by replacing
the multiple boolean locals with a single concise check using Kotlin's all():
build a collection of the string properties to validate (resume.userName,
resume.birthDate, resume.gender, resume.introduce, resume.introduceDetail,
resume.personalityList) and return collection.all { it.isNotEmpty() } inside
validateResume; this keeps the same semantics but reduces verbosity while still
referencing the existing validateResume and ResumeWritingModel properties.
In
`@feature/resume/impl/src/main/java/com/nonggle/impl/step1/ResumeStep1Screen.kt`:
- Around line 82-93: The LaunchedEffect is currently keyed on viewModel.effect
which can re-subscribe unexpectedly; change the key to Unit so the collection is
set up once: in ResumeStep1Screen replace LaunchedEffect(viewModel.effect) with
LaunchedEffect(Unit) and keep the existing viewModel.effect.collect { effect ->
when (effect) { is ResumeStep1Effect.SendImageVolumeOverFlowMessage -> ...; is
ResumeStep1Effect.SendBirthDateNotValidMessage -> ... } } so the Toast handling
for ResumeStep1Effect remains unchanged but the collector is only launched once.
In
`@feature/resume/impl/src/main/java/com/nonggle/impl/step2/ResumeStep2Screen.kt`:
- Around line 52-60: In ResumeStep2Screen.kt update the LaunchedEffect usage so
it doesn't restart when the ViewModel instance changes: replace
LaunchedEffect(viewModel.effect) that wraps viewModel.effect.collect { ... }
with a LaunchedEffect keyed to Unit (e.g., LaunchedEffect(Unit)) so the flow
collection is established once; keep the inner collect block (handling
ResumeStep2Effect.SendStartCareerNotValidMessage and showing the Toast)
unchanged and ensure you're collecting from viewModel.effect inside that
Unit-keyed LaunchedEffect.
In
`@feature/resume/impl/src/main/java/com/nonggle/impl/step2/ResumeStep2ViewModel.kt`:
- Around line 114-158: Both saveTempResume and deleteTempResume contain
identical logic; consolidate them into a single private helper (e.g.,
updateTempResume) that accepts newCareerList: List<CareerFormData>, computes
totalCareer via sumCareerPeriod(newCareerList), maps each CareerFormData to
ResumeWritingModel.Career (using careerStartDate ?: LocalDate.now(),
careerEndDate ?: LocalDate.now(), careerPeriod = getPeriodFormatter(period =
Period.between(it.careerStartDate, it.careerEndDate)), careerDescription,
careerDetail) and calls resumeStore.update { it.copy(careerList = mappedList,
totalCareer = getPeriodFormatter(totalCareer)) }; replace calls to
saveTempResume and deleteTempResume with this new helper and remove the
duplicated methods.
- Line 8: Remove the unused import of ResumeStep1Effect from
ResumeStep2ViewModel: locate the import statement "import
com.nonggle.resume.impl.step1.ResumeStep1Effect" in the ResumeStep2ViewModel
file and delete it so the file no longer contains an unused import; run a quick
compile or linter to confirm no references to ResumeStep1Effect remain in the
class.
In `@feature/resume/impl/src/main/res/values/strings.xml`:
- Line 12: The two string resource names use inconsistent casing:
resume1Screen_birthdate_notValid (lowercase "notValid") vs
resume2Screen_startCareerDate_NotValid (capital "NotValid"); rename
resume2Screen_startCareerDate_NotValid to resume2Screen_startCareerDate_notValid
(or vice versa if you prefer the other pattern) and update all usages and
translations accordingly (search for both keys including the occurrence
mentioned at lines 43-43) so the naming pattern for the "notValid" suffix is
consistent across the project.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6609a0a5-e824-4e55-94ca-8ad076472fc0
📒 Files selected for processing (73)
app/build.gradle.ktsapp/src/main/java/com/nonggle/nonggleresume/MainActivity.ktapp/src/main/java/com/nonggle/nonggleresume/MainActivityViewModel.ktapp/src/main/java/com/nonggle/nonggleresume/navigation/TopLevelNavItem.ktapp/src/main/java/com/nonggle/nonggleresume/ui/NonggleApp.ktapp/src/main/java/com/nonggle/nonggleresume/ui/NonggleAppState.ktapp/src/main/res/drawable/ic_launcher_background.xmlapp/src/main/res/drawable/ic_launcher_foreground.xmlapp/src/main/res/mipmap-anydpi-v26/ic_launcher.xmlapp/src/main/res/mipmap-anydpi-v26/ic_launcher_round.xmlapp/src/main/res/mipmap-hdpi/ic_launcher.webpapp/src/main/res/mipmap-hdpi/ic_launcher_round.webpapp/src/main/res/mipmap-mdpi/ic_launcher.webpapp/src/main/res/mipmap-mdpi/ic_launcher_round.webpapp/src/main/res/mipmap-xhdpi/ic_launcher.webpapp/src/main/res/mipmap-xhdpi/ic_launcher_round.webpapp/src/main/res/mipmap-xxhdpi/ic_launcher.webpapp/src/main/res/mipmap-xxhdpi/ic_launcher_round.webpapp/src/main/res/mipmap-xxxhdpi/ic_launcher.webpapp/src/main/res/mipmap-xxxhdpi/ic_launcher_round.webpapp/src/main/res/values/strings.xmlapp/src/test/java/com/nonggle/nonggleresume/MainActivityViewModelTest.ktcore/common/src/main/java/com/nonggle/common/result/AuthEventBus.ktcore/data/src/main/java/com/nonggle/data/repositoryimpl/LoginRepositoryImpl.ktcore/data/src/main/java/com/nonggle/data/repositoryimpl/ResumeDraftStore.ktcore/designsystem/src/main/java/com/nonggle/designsystem/component/Button.ktcore/designsystem/src/main/java/com/nonggle/designsystem/component/Navigation.ktcore/designsystem/src/main/java/com/nonggle/designsystem/component/TopAppBar.ktcore/domain/src/main/java/com/nonggle/domain/repository/ResumeDraftStoreInterface.ktcore/domain/src/main/java/com/nonggle/domain/usecase/LogoutUseCase.ktcore/network/src/main/java/com/nonggle/network/service/LoginService.ktfeature/download/impl/src/main/java/com/nonggle/feature/download/impl/DownloadScreen.ktfeature/download/impl/src/main/res/values/strings.xmlfeature/home/impl/src/main/java/com/nonggle/feature/home/impl/HomeScreen.ktfeature/home/impl/src/main/res/values/strings.xmlfeature/login/impl/src/main/java/com/nonggle/impl/KakaoLoginManager.ktfeature/mypage/api/.gitignorefeature/mypage/api/build.gradle.ktsfeature/mypage/api/consumer-rules.profeature/mypage/api/proguard-rules.profeature/mypage/api/src/main/AndroidManifest.xmlfeature/mypage/api/src/main/java/com/nonggle/mypage/api/MyPageNavKey.ktfeature/mypage/impl/.gitignorefeature/mypage/impl/build.gradle.ktsfeature/mypage/impl/consumer-rules.profeature/mypage/impl/proguard-rules.profeature/mypage/impl/src/androidTest/java/com/nonggle/mypage/impl/ExampleInstrumentedTest.ktfeature/mypage/impl/src/main/AndroidManifest.xmlfeature/mypage/impl/src/main/java/com/nonggle/mypage/impl/MyPageContract.ktfeature/mypage/impl/src/main/java/com/nonggle/mypage/impl/MyPageScreen.ktfeature/mypage/impl/src/main/java/com/nonggle/mypage/impl/MyPageViewModel.ktfeature/mypage/impl/src/main/java/com/nonggle/mypage/impl/navigation/MyPageEntryProvider.ktfeature/mypage/impl/src/main/res/values/strings.xmlfeature/mypage/impl/src/test/java/com/nonggle/mypage/impl/ExampleUnitTest.ktfeature/resume/impl/src/main/java/com/nonggle/impl/component/ResumeStep1Component.ktfeature/resume/impl/src/main/java/com/nonggle/impl/main/ResumeMainContract.ktfeature/resume/impl/src/main/java/com/nonggle/impl/main/ResumeMainScreen.ktfeature/resume/impl/src/main/java/com/nonggle/impl/main/ResumeMainViewModel.ktfeature/resume/impl/src/main/java/com/nonggle/impl/step1/ResumeStep1Contract.ktfeature/resume/impl/src/main/java/com/nonggle/impl/step1/ResumeStep1Screen.ktfeature/resume/impl/src/main/java/com/nonggle/impl/step1/ResumeStep1ViewModel.ktfeature/resume/impl/src/main/java/com/nonggle/impl/step2/ResumeStep2Contract.ktfeature/resume/impl/src/main/java/com/nonggle/impl/step2/ResumeStep2Screen.ktfeature/resume/impl/src/main/java/com/nonggle/impl/step2/ResumeStep2ViewModel.ktfeature/resume/impl/src/main/java/com/nonggle/impl/step3/ResumeStep3Screen.ktfeature/resume/impl/src/main/java/com/nonggle/impl/step3/ResumeStep3ViewModel.ktfeature/resume/impl/src/main/res/values/strings.xmlfeature/resume/impl/src/test/java/com/nonggle/impl/step2/ResumeStep2ViewModelTest.ktfeature/setting/build.gradle.ktsfeature/setting/src/main/java/com/nonggle/setting/SettingScreen.ktfeature/setting/src/main/java/com/nonggle/setting/navigation/SettingEntryProvider.ktfeature/setting/src/main/res/values/strings.xmlsettings.gradle.kts
💤 Files with no reviewable changes (5)
- app/src/main/java/com/nonggle/nonggleresume/ui/NonggleAppState.kt
- feature/setting/src/main/res/values/strings.xml
- feature/setting/src/main/java/com/nonggle/setting/navigation/SettingEntryProvider.kt
- feature/setting/build.gradle.kts
- feature/setting/src/main/java/com/nonggle/setting/SettingScreen.kt
Comment on lines
+55
to
+61
| private fun refreshLoginState() { | ||
| viewModelScope.launch { | ||
| _uiState.value = MainActivityUiState.Loading | ||
| _isLoggedIn.value = loginRepository.isLoggedIn().first() | ||
| _uiState.value = MainActivityUiState.Success | ||
| } | ||
| } |
There was a problem hiding this comment.
onResume()마다 Loading 상태로 전환되어 UX 저하 가능성이 있습니다.
앱이 백그라운드에서 복귀할 때마다 _uiState가 Loading으로 설정되면 스플래시 화면이 잠깐 다시 표시될 수 있습니다. 초기 로딩 시에만 Loading 상태를 사용하거나, 이미 Success 상태인 경우 Loading으로 전환하지 않는 로직이 필요합니다.
또한, loginRepository.isLoggedIn().first()가 DataStore 등 I/O를 수반한다면 Main dispatcher에서 실행될 수 있으므로, repository 내부에서 적절한 dispatcher 전환이 이루어지는지 확인이 필요합니다.
💡 제안된 수정
private fun refreshLoginState() {
viewModelScope.launch {
- _uiState.value = MainActivityUiState.Loading
+ // 초기 로딩 시에만 Loading 표시
+ if (_uiState.value == MainActivityUiState.Loading) {
+ // 이미 Loading 상태면 유지
+ }
_isLoggedIn.value = loginRepository.isLoggedIn().first()
_uiState.value = MainActivityUiState.Success
}
}또는 초기화와 resume 로직을 분리:
private fun refreshLoginState(showLoading: Boolean = false) {
viewModelScope.launch {
if (showLoading) {
_uiState.value = MainActivityUiState.Loading
}
_isLoggedIn.value = loginRepository.isLoggedIn().first()
_uiState.value = MainActivityUiState.Success
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/nonggle/nonggleresume/MainActivityViewModel.kt` around
lines 55 - 61, The refreshLoginState function currently unconditionally sets
_uiState to Loading which causes a flash on every resume; change
refreshLoginState (and callers) to accept a showLoading: Boolean = false and
only set _uiState = MainActivityUiState.Loading when showLoading is true or when
current _uiState is not already Success, then proceed to read
loginRepository.isLoggedIn().first() and set _uiState =
MainActivityUiState.Success; also ensure the potentially blocking
loginRepository.isLoggedIn().first() runs off the Main thread by either making
the repository switch dispatchers internally or wrapping the call in
withContext(Dispatchers.IO) inside the coroutine to avoid main-thread I/O.
Comment on lines
44
to
52
| override suspend fun logOut(): AppResult<Unit> { | ||
| /// TODO: 로그아웃 구현 | ||
| return AppResult.Success(Unit) | ||
| val response = loginService.logout() | ||
| tokenManager.deleteToken() | ||
|
|
||
| return when(response) { | ||
| is AppResult.Success -> AppResult.Success(Unit) | ||
| is AppResult.Error -> response | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: 로그아웃 관련 다른 구현이나 주석을 확인하여 의도된 동작 파악
rg -n -C3 "logOut|logout|deleteToken" --type=ktRepository: Nonggle/Android
Length of output: 85
🏁 Script executed:
#!/bin/bash
# Kotlin 파일에서 logout, logOut, deleteToken 패턴 검색
rg -n "logOut|logout|deleteToken" --type-list | head -5Repository: Nonggle/Android
Length of output: 448
🏁 Script executed:
#!/bin/bash
# .kt 확장자 파일에서 logout 관련 코드 검색
fd -e kt | head -20Repository: Nonggle/Android
Length of output: 1480
🏁 Script executed:
#!/bin/bash
# LoginRepository 관련 파일 찾기
fd -e kt "LoginRepository"Repository: Nonggle/Android
Length of output: 212
🏁 Script executed:
#!/bin/bash
# core/data 디렉토리의 구조와 파일 확인
find core/data -name "*.kt" -type f | grep -i loginRepository: Nonggle/Android
Length of output: 276
🏁 Script executed:
#!/bin/bash
# LoginRepositoryImpl.kt 파일 내용 확인
cat core/data/src/main/java/com/nonggle/data/repositoryimpl/LoginRepositoryImpl.ktRepository: Nonggle/Android
Length of output: 1868
🏁 Script executed:
#!/bin/bash
# LoginRepository 인터페이스 확인
cat core/domain/src/main/java/com/nonggle/domain/repository/LoginRepository.ktRepository: Nonggle/Android
Length of output: 401
🏁 Script executed:
#!/bin/bash
# 로그아웃 관련 테스트 케이스 검색
find . -name "*Test.kt" -o -name "*Spec.kt" | xargs grep -l "logOut\|logout" 2>/dev/nullRepository: Nonggle/Android
Length of output: 133
🏁 Script executed:
#!/bin/bash
# 로그아웃 호출 코드 찾기
rg "logOut\(\)" -A3 -B3Repository: Nonggle/Android
Length of output: 5368
🏁 Script executed:
#!/bin/bash
# TokenManager의 deleteToken 구현 확인
fd -e kt -path "*TokenManager*"Repository: Nonggle/Android
Length of output: 227
🏁 Script executed:
#!/bin/bash
# MainActivityViewModel 전체 코드 확인
cat app/src/main/java/com/nonggle/nonggleresume/MainActivityViewModel.ktRepository: Nonggle/Android
Length of output: 2247
🏁 Script executed:
#!/bin/bash
# TokenManager 파일 찾기
find . -name "*TokenManager*" -type fRepository: Nonggle/Android
Length of output: 180
🏁 Script executed:
#!/bin/bash
# AppResult 타입 확인
rg "class AppResult|sealed class AppResult" -A5Repository: Nonggle/Android
Length of output: 41
🏁 Script executed:
#!/bin/bash
# TokenManager 인터페이스 확인
cat core/auth/src/main/java/com/nonggle/auth/TokenManager.ktRepository: Nonggle/Android
Length of output: 460
🏁 Script executed:
#!/bin/bash
# TinkTokenManager 구현 확인
cat core/auth/src/main/java/com/nonggle/auth/TinkTokenManager.ktRepository: Nonggle/Android
Length of output: 2456
🏁 Script executed:
#!/bin/bash
# AppResult 정의 찾기
find . -name "*.kt" -type f | xargs grep -l "sealed class AppResult\|class AppResult" | head -5Repository: Nonggle/Android
Length of output: 41
서버 로그아웃 실패 시에도 토큰 삭제 - 설계 의도는 명확하나, 에러 처리 개선 필요
현재 구현은 loginService.logout() 결과와 무관하게 항상 tokenManager.deleteToken()을 호출합니다. 코드 분석 결과 이는 의도된 동작입니다:
MainActivityViewModel에서SessionExpired이벤트 발생 시logOut()반환값을 무시하고 즉시 로컬 상태를 업데이트- 세션 만료 시나리오에서는 토큰이 이미 서버에서 무효화되었으므로 로컬만 정리하면 됨
다만 개선이 필요한 부분:
- 에러 상황의 혼동성: 서버 로그아웃이 실패(
AppResult.Error)해도 토큰은 이미 삭제됨. 호출자가 재시도하려 해도 토큰이 없어서 불가능 - 비대칭 패턴:
login()은 성공 시에만 저장하지만,logOut()은 실패해도 삭제 - 의도가 코드에서 드러나지 않음
개선 방안:
deleteToken()호출을 성공 여부 확인 후로 이동하거나- 에러 무시 의도를 명확히 문서화하고, 에러 발생 시에도 토큰이 이미 삭제된 상태임을 처리
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@core/data/src/main/java/com/nonggle/data/repositoryimpl/LoginRepositoryImpl.kt`
around lines 44 - 52, The current logOut in LoginRepositoryImpl always calls
tokenManager.deleteToken() regardless of loginService.logout() result; change
logOut so it first calls val response = loginService.logout(), then only call
tokenManager.deleteToken() when response is AppResult.Success (i.e., on
successful server logout) and return AppResult.Success(Unit); for the
AppResult.Error branch, return the error without deleting the token (or if you
intentionally want to always delete, add a clear comment and change the return
to wrap the logout error with a marker that the token was deleted); key symbols:
LoginRepositoryImpl.logOut, loginService.logout(), tokenManager.deleteToken(),
MainActivityViewModel.SessionExpired.
| implementation(project(":core:domain")) | ||
| implementation(project(":core:common")) | ||
| implementation(project(":feature:mypage:api")) | ||
| implementation(project(":feature:login:impl")) |
There was a problem hiding this comment.
impl 모듈 간 직접 의존성 - 아키텍처 위반
feature:login:impl에 대한 직접 의존은 api/impl 분리 원칙을 위반합니다. impl 모듈은 다른 feature의 api 모듈에만 의존해야 합니다.
만약 로그인 관련 기능이 필요하다면 feature:login:api를 통해 접근하거나, 공통 도메인 레이어(core:domain)를 활용해야 합니다.
🔧 수정 제안
- implementation(project(":feature:login:impl"))
+ implementation(project(":feature:login:api"))로그인 기능이 필요한 경우 LoginRepository 또는 LogoutUseCase와 같은 도메인 레이어를 통해 접근하는 것이 바람직합니다.
Based on learnings: "Login feature의 api/impl 분리 전략: api 모듈에 NavKey만 배치, impl 모듈에 실구현 배치."
📝 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.
Suggested change
| implementation(project(":feature:login:impl")) | |
| implementation(project(":feature:login:api")) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@feature/mypage/impl/build.gradle.kts` at line 26, The build is directly
depending on feature:login:impl which breaks the api/impl separation; replace
the implementation(project(":feature:login:impl")) dependency in
build.gradle.kts with a dependency on the login API (e.g.,
implementation(project(":feature:login:api"))) or, if you need domain-level
types, depend on core:domain instead and access LoginRepository/LogoutUseCase
through that API/domain layer so only API modules are referenced from the mypage
impl.
| import com.nonggle.common.result.AuthEventBus | ||
| import com.nonggle.model.AppResult | ||
| import com.nonggle.domain.usecase.LogoutUseCase | ||
| import com.nonggle.impl.KakaoLoginManager |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check module dependencies and KakaoLoginManager usage
# Check if mypage module depends on login/impl in build.gradle
fd -t f "build.gradle" feature/mypage --exec cat {} \; | grep -i "login"
# Check KakaoLoginManager location
fd -t f "KakaoLoginManager.kt" --exec echo "Found: {}"Repository: Nonggle/Android
Length of output: 189
🏁 Script executed:
# Check login/api module contents and exports
fd -t f "\.kt$" feature/login/api --exec basename {} \; | sort | uniq
# Examine MyPageViewModel file
cat -n feature/mypage/impl/src/main/java/com/nonggle/mypage/impl/MyPageViewModel.kt
# Check for any abstraction/interface related to KakaoLoginManager
rg "KakaoLoginManager|KakaoLogin" feature/login/api -t kt -A 3Repository: Nonggle/Android
Length of output: 1806
🏁 Script executed:
# Correct the rg command syntax and check login/api
fd -t f "\.kt$" feature/login/api
# Check LogoutUseCase implementation
fd -t f "LogoutUseCase.kt" --exec cat -n {} \;
# Search for any authentication abstraction in login/api
rg "interface|class" feature/login/api -g "*.kt" -B 2 -A 5Repository: Nonggle/Android
Length of output: 559
🏁 Script executed:
# List all files in login/api module
fd -t f "\.kt$" feature/login/api -x echo {}
# List all files in login/impl module
fd -t f "\.kt$" feature/login/impl -x echo {}
# Search for KakaoLoginManager interface or abstraction
rg "KakaoLoginManager|KakaoLogout" feature/login --type kotlinRepository: Nonggle/Android
Length of output: 976
🏁 Script executed:
# Check KakaoLoginManager implementation
cat -n feature/login/impl/src/main/java/com/nonggle/impl/KakaoLoginManager.kt
# Check LoginContract for abstractions
cat -n feature/login/impl/src/main/java/com/nonggle/impl/LoginContract.kt
# Check LoginNavKey to understand what api module exports
cat -n feature/login/api/src/main/java/com/nonggle/api/LoginNavKey.ktRepository: Nonggle/Android
Length of output: 4020
모듈 경계 위반: feature/login/impl 모듈을 직접 의존합니다.
KakaoLoginManager를 feature/login/impl에서 직접 import하여 사용 중입니다. mypage/impl 모듈이 login/impl에 직접 의존하면 feature 모듈 간 격리 원칙을 위반하고 추후 순환 의존 위험을 만듭니다.
로그아웃 기능을 다음 중 하나로 추상화해야 합니다:
login/api모듈에KakaoLoginManager인터페이스를 정의하고 mypage가 이를 의존하도록 변경- 또는
LogoutUseCase를 확장하여 Kakao 로그아웃도 포함하도록 구조화
이는 기존 api/impl 분리 전략과 일관성을 유지하는 방향입니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@feature/mypage/impl/src/main/java/com/nonggle/mypage/impl/MyPageViewModel.kt`
at line 8, MyPageViewModel currently imports and uses KakaoLoginManager from
feature/login/impl, which breaks module boundaries; remove the direct import and
instead depend on an abstraction: either (A) add a KakaoLoginManager interface
(or IKakaoLogout) to feature/login/api and have feature/login/impl implement it,
then inject that interface into MyPageViewModel (constructor or DI) in place of
the concrete type, or (B) extend the existing LogoutUseCase to include Kakao
logout behavior and have MyPageViewModel call LogoutUseCase only; update
feature/login/impl to implement the new api interface or extended use case and
update MyPageViewModel to use the API-facing symbol (the new interface or
LogoutUseCase) rather than KakaoLoginManager.
Comment on lines
+35
to
+40
| if (apiResult is AppResult.Error || kakaoResult.isFailure) { | ||
| postEffect(MyPageEffect.LogoutFailed) | ||
| } | ||
|
|
||
| authEventBus.emit(AuthEvent.LoggedOut) | ||
| updateState { copy(isLoading = false) } |
There was a problem hiding this comment.
로그아웃 실패 시에도 AuthEvent.LoggedOut이 무조건 emit됩니다.
apiResult가 Error이거나 kakaoResult가 실패해도 Line 39에서 LoggedOut 이벤트가 발행됩니다. 이는 서버/카카오 로그아웃이 실패했음에도 앱 전체에 로그아웃 상태를 전파하게 되어 상태 불일치가 발생할 수 있습니다.
실패 시에는 LoggedOut emit을 건너뛰고 사용자에게 재시도 기회를 주는 것이 적절합니다.
🐛 제안된 수정
if (apiResult is AppResult.Error || kakaoResult.isFailure) {
postEffect(MyPageEffect.LogoutFailed)
+ updateState { copy(isLoading = false) }
+ return@launch
}
authEventBus.emit(AuthEvent.LoggedOut)
updateState { copy(isLoading = false) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@feature/mypage/impl/src/main/java/com/nonggle/mypage/impl/MyPageViewModel.kt`
around lines 35 - 40, The code currently emits
authEventBus.emit(AuthEvent.LoggedOut) unconditionally even when apiResult is
AppResult.Error or kakaoResult.isFailure; change the logic in the logout flow
(surrounding apiResult, kakaoResult, postEffect(MyPageEffect.LogoutFailed),
authEventBus.emit(AuthEvent.LoggedOut), updateState { copy(isLoading = false) })
so that you only call authEventBus.emit(AuthEvent.LoggedOut) when both apiResult
is not AppResult.Error and kakaoResult.isFailure is false (i.e., logout
succeeded); keep calling postEffect(MyPageEffect.LogoutFailed) on failure and
always ensure updateState { copy(isLoading = false) } runs after handling
success or failure.
Comment on lines
+23
to
+25
| viewModelScope.launch { | ||
| when (event) { | ||
| is ResumeStep1Event.SelectImage -> setProfileImageUri(event.imageUri) |
There was a problem hiding this comment.
이미지 선택 결과가 역전될 수 있습니다.
Line 23에서 SelectImage를 새 코루틴으로 분리한 뒤 Line 48에서 이미지 메타 조회를 수행해서, 먼저 고른 이미지의 처리가 늦게 끝나면 마지막 선택을 덮어쓸 수 있습니다. SelectImage는 이전 작업을 취소하는 전용 Job으로 처리하거나, 이벤트 dispatch는 동기적으로 두고 필요한 분기만 launch 하는 편이 안전합니다.
As per coding guidelines, **/*.kt: Coroutine / Flow 사용의 안정성 (dispatcher, cancellation, leak)을 우선적으로 리뷰해줘.
Also applies to: 45-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/resume/impl/src/main/java/com/nonggle/impl/step1/ResumeStep1ViewModel.kt`
around lines 23 - 25, 현재 ResumeStep1ViewModel에서 SelectImage 이벤트를 매번 새
코루틴(viewModelScope.launch)으로 처리해 이미지 메타 조회가 지연되면 이전 선택이 나중 선택으로 덮어써질 수 있으니,
SelectImage 처리를 전용 cancellable Job으로 변경하거나 이벤트 디스패치를 동기화하고 메타 조회 부분만 취소 가능한
코루틴으로 분리하세요; 구체적으로 ResumeStep1ViewModel의 SelectImage 분기에서 기존에 보관한
imageSelectionJob을 취소하고 새 Job으로 setProfileImageUri와 이후 이미지 메타 조회(현재 메타 조회를 수행하는
함수/블록)를 실행하거나, 이벤트 핸들러는 즉시 setProfileImageUri만 수행하고 메타 조회를 별도의 cancellable 함수로
옮겨 imageSelectionJob으로 관리해 중복/역전이 발생하지 않도록 보장하세요.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항
테스트