설정 페이지 구현 #78
설정 페이지 구현 #78
Conversation
📝 WalkthroughWalkthrough설정 기능 모듈(feature/settings)이 신규 추가되었습니다. 사용자 정보 조회, 로그아웃, 계정 탈퇴 기능을 포함하며, 이에 따른 네트워크 서비스(UserService, 로그아웃/계정탈퇴 엔드포인트), 저장소 계층(UserRepository, AuthRepository 확장), 도메인 모델(User), 그리고 설정 화면 UI(SettingsScreen, SettingsAccountScreen, SettingsAboutScreen)가 구현되었습니다. 네비게이션 라우트, DI 모듈, 리소스(문자열, 벡터 드로어블)도 함께 추가되었으며, 메인 화면에서 설정으로의 네비게이션 경로가 연결되었습니다. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@core/design-system/src/main/res/drawable/ic_info.xml`:
- Around line 1-20: The vector drawable hardcodes color "#171717" on the path
elements (look for the path tags with android:strokeColor and android:fillColor
in ic_info.xml), which breaks dark mode; replace those hardcoded colors by
referencing theme/colors (e.g., use android:strokeColor and android:fillColor
that reference a color resource like `@color/xyz` or a theme attribute like
?attr/colorOnSurface) or remove per-path colors and set
android:tint="?attr/colorOnSurface" on the root <vector> so the icon follows the
app theme at runtime; ensure all three <path> entries are updated and that any
runtime Compose usages still apply tint consistently.
In `@core/design-system/src/main/res/drawable/ic_profile_small.xml`:
- Around line 1-20: The vector drawable ic_profile_small currently hardcodes
strokeColor="#171717", which breaks dark mode; replace the hardcoded color in
both <path> elements (the attributes using strokeColor) with a color resource
reference (e.g., `@color/icon_stroke`) and add corresponding entries in
values/colors.xml and values-night/colors.xml (e.g., icon_stroke_light / night
value) so the drawable follows theme; do the same replacement pattern for other
icons that use strokeColor to centralize icon coloring.
In `@data/src/main/java/com/twix/data/repository/DefaultAuthRepository.kt`:
- Around line 28-38: The logout and withdrawAccount implementations call
tokenProvider.clear() inside safeApiCall, which can mask token-clear failures as
API errors; move tokenProvider.clear() out of the safeApiCall so the API call
(service.logout()/service.withdrawAccount()) is executed inside safeApiCall and,
on success, clear the token afterwards; for withdrawAccount ensure token removal
happens unconditionally (e.g., in a finally block or after handling the
AppResult) so local tokens are cleared even if tokenProvider.clear() itself
throws, and adjust callers to observe API result separately from local
token-clearing.
In
`@feature/settings/src/main/java/com/twix/settings/about/SettingsAboutScreen.kt`:
- Around line 63-71: The Image in SettingsAboutScreen (the back button Image
composable) currently hardcodes contentDescription="back"; replace this with a
localized string via stringResource (e.g., stringResource(R.string.cd_back) with
Korean text like "뒤로 가기"), mirroring the fix to SettingsMenuCard if present, and
add or reuse a shared string resource (e.g., cd_back) to ensure consistent,
accessible descriptions across screens; update the Image call to use
stringResource(...) instead of the hardcoded literal.
In
`@feature/settings/src/main/java/com/twix/settings/account/SettingsAccountScreen.kt`:
- Around line 116-152: CommonDialog의 confirm/dismiss 역할이 반전되어 있어 의미와 동작을 맞추십시오:
현재 confirmText/ onConfirm는 취소 동작이고 dismissText/ onDismiss가
탈퇴(onWithdrawAccount)를 실행하므로, confirmText를
stringResource(R.string.action_withdraw_account)로 하고 onConfirm에서
onWithdrawAccount() 호출 후 showWithdrawDialog = false로 닫도록 변경하고, dismissText를
stringResource(R.string.word_cancel)로 하여 onDismiss에서는 단순히 showWithdrawDialog =
false만 수행하도록 수정(또는 만약 반전이 의도된 UX라면 CommonDialog 호출부에 해당 의도를 명확히 설명하는 주석을 추가).
Ensure to update onDismissRequest consistently; references: CommonDialog,
confirmText, dismissText, onConfirm, onDismiss, onDismissRequest,
onWithdrawAccount.
In
`@feature/settings/src/main/java/com/twix/settings/component/SettingsMenuCard.kt`:
- Around line 56-64: The Image's contentDescription is hardcoded as "menu"
making icons indistinguishable to screen readers; update the Image in
SettingsMenuCard (the resId?.let block) to use the passed-in title (or a
localized string based on title) as the contentDescription and ensure
null-safety—e.g., set contentDescription = title ?: "" or derive a proper
localized description—so each menu item's icon conveys its specific context to
accessibility tools.
In
`@feature/settings/src/main/java/com/twix/settings/navigation/SettingsNavGraph.kt`:
- Around line 64-71: The current navigateToLogin block uses
popUpTo(NavRoutes.SettingsRoute.route) which only clears the Settings stack and
leaves the authenticated MainGraph (home) in back stack; change the popUpTo
target in the navController.navigate call to popUpTo(NavRoutes.MainGraph.route)
{ inclusive = true } (or to NavRoutes.LoginGraph.route if you intend to remove
the entire login graph) so that navController.navigate in navigateToLogin clears
the authenticated stack (refer to navController.navigate,
NavRoutes.SettingsRoute, NavRoutes.MainGraph, and NavRoutes.LoginRoute in
SettingsNavGraph.kt).
In `@feature/settings/src/main/java/com/twix/settings/SettingsViewModel.kt`:
- Around line 33-35: The SettingsIntent.SetNickName intent is never dispatched
by the UI so its handler setNickName in SettingsViewModel is effectively dead;
either add a TODO comment indicating a planned nickname edit UI (e.g., above
SettingsIntent.SetNickName or the setNickName method) if you will implement
nickname editing later, or remove the unused intent and its handler
(SettingsIntent.SetNickName and setNickName) to follow YAGNI—also ensure
fetchUserInfo remains the sole source of nickname updates if you remove the
intent.
🧹 Nitpick comments (12)
domain/src/main/java/com/twix/domain/model/enums/SettingsMenuAction.kt (1)
1-6:SettingsMenuAction이 도메인 계층에 위치하는 것이 적절한지 검토가 필요합니다.이 enum은 설정 화면의 메뉴 탐색 액션(
Account,Information)을 정의하고 있어, 비즈니스 로직보다는 UI/프레젠테이션 관심사에 가까워 보입니다. 도메인 모듈은 Android 및 프레임워크 의존성 없이 순수 비즈니스 로직만 담아야 하는데, 설정 메뉴의 네비게이션 액션이 도메인 모델로 적합한지 고민해보면 좋겠습니다.
feature/settings모듈 내부로 이동하는 것이 도메인 계층의 순수성을 유지하는 데 도움이 될 수 있습니다. 혹시 다른 모듈에서도 이 enum을 참조해야 하는 이유가 있다면 현재 위치도 합리적일 수 있으니, 의도를 공유해주시면 좋겠습니다.As per coding guidelines, "Domain 모듈 리뷰 가이드 - 비즈니스 로직이 명확히 표현되어 있는가?"
feature/settings/build.gradle.kts (2)
7-13:inputStream()이 명시적으로 닫히지 않아 리소스 누수 가능성이 있습니다.
load(localPropertiesFile.inputStream())에서 생성된InputStream이close()없이 사용되고 있습니다. Gradle 빌드 스크립트에서 실질적인 문제가 되는 경우는 드물지만, 리소스 관리 모범 사례로use블록을 사용하는 것이 좋습니다.♻️ 개선 제안
val localProperties = Properties().apply { val localPropertiesFile = rootProject.file("local.properties") if (localPropertiesFile.exists()) { - load(localPropertiesFile.inputStream()) + localPropertiesFile.inputStream().use { load(it) } } }
22-31:buildTypes.all대신defaultConfig에서buildConfigField를 설정하는 것이 더 간결합니다.모든 빌드 타입에 동일한 값을 적용하고 있으므로,
defaultConfig블록에서 한 번만 선언하면 의도가 더 명확해집니다.buildTypes.all을 사용하면 나중에 빌드 타입별로 다른 값을 설정하려는 의도로 오해될 수 있습니다.♻️ 개선 제안
android { namespace = "com.twix.settings" buildFeatures { buildConfig = true } val privacyPolicyUrl = localProperties.getProperty("privacy_policy_url") ?: providers.gradleProperty("privacy_policy_url").orNull ?: "https://incongruous-sweatshirt-b32.notion.site/Keepliuv-3024eb2e10638051824ef9ac7f9a522f" - buildTypes { - all { - buildConfigField("String", "PRIVACY_POLICY_URL", "\"$privacyPolicyUrl\"") - } - } + defaultConfig { + buildConfigField("String", "PRIVACY_POLICY_URL", "\"$privacyPolicyUrl\"") + } }core/design-system/src/main/res/drawable/ic_warning.xml (1)
1-26: 다중 색상을 사용하는 경고 아이콘으로, 리소스 구조는 적절합니다.이 아이콘은 빨간색(
#FF6363), 흰색, 회색 등 여러 색상을 조합한 일러스트 스타일 아이콘이므로, 단순 tint 적용이 어렵습니다. 다만, 다크 모드에서#171717스트로크가 어두운 배경과 구분되지 않을 수 있으므로, 다크 모드용 대체 리소스(drawable-night/ic_warning.xml)를 제공하는 것을 고려해 보시면 좋겠습니다.feature/settings/src/main/java/com/twix/settings/SettingsIntent.kt (1)
5-13: Intent가 사용자 액션 단위로 잘 정의되어 있습니다.
Logout과WithdrawAccount는 명확한 사용자 액션입니다. 한 가지 토론 포인트로,SetNickName은 사용자가 직접 트리거하는 액션인가요, 아니면 서버에서 사용자 정보를 조회한 후 상태를 설정하는 용도인가요?만약 후자라면, MVI에서 Intent는 일반적으로 "사용자의 의도"를 나타내므로, ViewModel 내부에서 직접 state를 업데이트하는 방식이 더 적절할 수 있습니다. 물론 현재 방식도 동작에는 문제가 없으므로 팀 컨벤션에 따라 결정하시면 됩니다.
As per coding guidelines, "Intent가 사용자 액션 단위로 명확히 정의되어 있는가?" —
SetNickName의 성격을 명확히 하면 좋겠습니다.feature/settings/src/main/java/com/twix/settings/component/SettingsMenuCard.kt (1)
25-79: 코딩 가이드라인: Preview Composable이 제공되지 않았습니다.
SettingsMenuFrame과SettingsMenuItem모두 재사용 가능한 공용 컴포넌트인데,@Preview함수가 없으면 디자인 확인과 향후 유지보수 시 불편합니다. 간단한 Preview를 추가하면 개발 효율이 높아집니다.As per coding guidelines,
feature/**파일에서는 "Preview Composable이 제공되는가?"를 확인해야 합니다.feature/settings/src/main/java/com/twix/settings/about/SettingsAboutScreen.kt (1)
90-99:context.getAppVersion()이 recomposition마다 호출될 수 있습니다.
getAppVersion()은PackageManager를 통해 버전을 조회하는 작업이므로,remember로 감싸 불필요한 재호출을 방지하는 것이 좋습니다.⚡ 개선 제안
SettingsAboutScreen내부에서:+ val appVersion = remember { context.getAppVersion() }그리고 사용처에서:
- text = context.getAppVersion(), + text = appVersion,domain/src/main/java/com/twix/domain/repository/AuthRepository.kt (1)
6-14:login()과 새 메서드 간의 반환 타입 불일치가 있습니다.기존
login()은Unit을 직접 반환하는 반면, 새로 추가된logout()과withdrawAccount()는AppResult<Unit>을 반환합니다. 에러 처리 전략이 메서드마다 다르면 호출부에서 혼란이 생길 수 있습니다.현재 PR 범위에서
login()을 변경하는 것은 과할 수 있지만, 향후login()도AppResult로 통일하는 것이 좋겠습니다. 이렇게 하면 Repository 계층의 에러 처리 패턴이 일관되어 유지보수가 쉬워집니다.core/navigation/src/main/java/com/twix/navigation/NavRoutes.kt (1)
57-67: Settings 라우트가 잘 정의되었습니다.그래프와 하위 라우트 구조가 명확합니다. 한 가지 참고 사항으로, 기존 라우트들은 플랫한 네이밍(
"login","main","goal_editor/{id}")을 사용하는 반면, Settings 하위 라우트는 경로 스타일("settings/account","settings/about")을 사용하고 있습니다.기능상 문제는 없지만, 프로젝트 전체 네이밍 컨벤션 통일 관점에서
"settings_account","settings_about"같은 플랫 네이밍도 고려해볼 수 있습니다. 팀 내 선호도에 따라 결정하시면 됩니다.feature/settings/src/main/java/com/twix/settings/SettingsViewModel.kt (1)
26-31:fetchUserInfo()실패 시 사용자에게 피드백이 없습니다.
onError콜백이 없어서 API 호출 실패 시 닉네임과 이메일이 빈 문자열로 남게 됩니다.BaseViewModel.handleError가 로깅은 처리하지만, 사용자 입장에서는 정보가 비어 보이는 상태가 됩니다.로딩/에러 상태를
SettingsUiState에 추가하거나, 최소한 토스트로 실패를 알려주는 것은 어떨까요?feature/settings/src/main/java/com/twix/settings/SettingsScreen.kt (1)
52-109:@PreviewComposable이 제공되지 않았습니다.Coding Guidelines의 Compose UI 항목에서 "Preview Composable이 제공되는가?"를 확인하도록 요구하고 있습니다.
SettingsScreen과ProfileInfo에 대해@Preview함수를 추가하면 UI 개발/검증 시 편의성이 높아집니다.As per coding guidelines, "Preview Composable이 제공되는가?"
feature/settings/src/main/java/com/twix/settings/account/SettingsAccountScreen.kt (1)
66-153:SettingsAccountScreen에도@PreviewComposable이 없습니다.SettingsScreen.kt와 마찬가지로, UI 검증을 위해 Preview 함수를 추가하는 것이 좋겠습니다.
As per coding guidelines, "Preview Composable이 제공되는가?"
| override suspend fun logout(): AppResult<Unit> = | ||
| safeApiCall { | ||
| service.logout() | ||
| tokenProvider.clear() | ||
| } | ||
|
|
||
| override suspend fun withdrawAccount() = | ||
| safeApiCall { | ||
| service.withdrawAccount() | ||
| tokenProvider.clear() | ||
| } |
There was a problem hiding this comment.
tokenProvider.clear()가 safeApiCall 내부에 있어 데이터 정합성 문제가 발생할 수 있습니다.
현재 구조에서는 service.logout()이 성공한 후 tokenProvider.clear()에서 예외가 발생하면, 서버에서는 이미 로그아웃 처리되었지만 로컬 토큰은 남아있는 불일치 상태가 됩니다. safeApiCall이 해당 예외를 AppResult.Error로 감싸서 반환하므로, 호출부는 로그아웃이 실패한 것으로 판단하게 됩니다.
토큰 클리어는 API 호출 성공 후 별도로 수행하는 것이 더 안전합니다.
🛠️ 개선 제안
override suspend fun logout(): AppResult<Unit> =
- safeApiCall {
- service.logout()
- tokenProvider.clear()
+ safeApiCall { service.logout() }
+ .also { result ->
+ if (result is AppResult.Success) {
+ tokenProvider.clear()
+ }
}
override suspend fun withdrawAccount() =
- safeApiCall {
- service.withdrawAccount()
- tokenProvider.clear()
+ safeApiCall { service.withdrawAccount() }
+ .also { result ->
+ if (result is AppResult.Success) {
+ tokenProvider.clear()
+ }
}withdrawAccount()의 경우에는 서버에서 계정이 삭제되었으므로, 토큰 클리어 실패 시에도 로컬 토큰을 반드시 제거해야 할 수 있습니다. 이 경우 finally 블록이나 무조건 클리어를 고려해보세요.
🤖 Prompt for AI Agents
In `@data/src/main/java/com/twix/data/repository/DefaultAuthRepository.kt` around
lines 28 - 38, The logout and withdrawAccount implementations call
tokenProvider.clear() inside safeApiCall, which can mask token-clear failures as
API errors; move tokenProvider.clear() out of the safeApiCall so the API call
(service.logout()/service.withdrawAccount()) is executed inside safeApiCall and,
on success, clear the token afterwards; for withdrawAccount ensure token removal
happens unconditionally (e.g., in a finally block or after handling the
AppResult) so local tokens are cleared even if tokenProvider.clear() itself
throws, and adjust callers to observe API result separately from local
token-clearing.
feature/settings/src/main/java/com/twix/settings/about/SettingsAboutScreen.kt
Show resolved
Hide resolved
feature/settings/src/main/java/com/twix/settings/account/SettingsAccountScreen.kt
Show resolved
Hide resolved
feature/settings/src/main/java/com/twix/settings/component/SettingsMenuCard.kt
Show resolved
Hide resolved
feature/settings/src/main/java/com/twix/settings/navigation/SettingsNavGraph.kt
Show resolved
Hide resolved
1 participant
이슈 번호
작업내용
결과물
2026-02-10.3.13.30.mov
리뷰어에게 추가로 요구하는 사항 (선택)
일단 심사에 필요한 것들 위주로 구현했습니다!