feat: 로그인-로그인 구현(#27)#34
Open
dada4679 wants to merge 9 commits into
Open
Conversation
4 tasks
📝 WalkthroughWalkthrough카카오 SDK 로그인과 서버 인증을 연결하고, 토큰 저장·재발급 및 자동 로그인을 구현했습니다. 약관 조회·동의 상태 관리와 랜딩부터 로그인·약관 화면까지의 내비게이션 흐름도 추가했습니다. Changes카카오 인증 및 로그인 흐름
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (4 passed)
✨ 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 |
Gemini AI 코드리뷰안녕하세요, 시니어 Android 개발자입니다. 이번 PR은 카카오 로그인, 약관 동의, 그리고 토큰 관리를 위한 네트워크 설정을 포함하는 중요한 기능 개발이네요. 전반적으로 코드 품질이 좋고, 아키텍처 원칙을 잘 따르려고 노력한 점이 보입니다. 특히 Orbit MVI 패턴과 Jetpack Compose의 상태 관리를 잘 적용하려 한 부분이 인상 깊습니다. 몇 가지 개선할 점과 질문이 있어 코멘트 남깁니다. 1. Kotlin 코드 리뷰
2. Jetpack Compose 코드 리뷰
3. Repository/DataSource 레이어
4. ViewModel
5. FCM/SSE/실시간 통신 관련 코드
전반적인 의견 및 추가 제안
PR 작성자에게 묻는 질문:
결론적으로, 이번 PR은 매우 잘 작성되었습니다. 위에서 언급한 몇 가지 개선 사항(특히 수고 많으셨습니다! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/login/impl/src/main/java/kr/co/call/impl/screen/AgreementScreen.kt (1)
164-170: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win약관을 받기 전에는 다음 단계로 진행하지 못하게 하세요.
terms가 비어 있으면isRequiredChecked가 true가 되어 로딩/조회 실패 상태에서도submitAgreements()를 실행할 수 있습니다.uiState.terms.isNotEmpty()를 함께 확인하고, 제출 측에서도 필수 약관을 검증하세요. 제공된AgreementUiState정의에서 빈 컬렉션의all이 true가 되는 것을 확인했습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/login/impl/src/main/java/kr/co/call/impl/screen/AgreementScreen.kt` around lines 164 - 170, Update the NextButton enabled condition to require both uiState.isRequiredChecked and uiState.terms.isNotEmpty(), preventing progression without loaded terms. Also add the same non-empty terms validation in the submitAgreements() flow so submission remains blocked even if the UI state is bypassed.
🧹 Nitpick comments (4)
core/domain/src/main/java/kr/co/call/domain/model/login/LoginToken.kt (1)
7-10: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win토큰을 보유한 모든
data class의 기본toString()을 차단하세요.기본
toString()이 토큰 원문을 출력하므로 객체가 로그나 예외 메시지에 포함될 때 인증 정보가 유출될 수 있습니다.
core/domain/src/main/java/kr/co/call/domain/model/login/LoginToken.kt#L7-L10: 토큰을 마스킹한toString()을 구현하세요.core/network/src/main/java/kr/co/call/network/dto/login/LoginRequestDto.kt#L7-L10: access token을 마스킹하거나 DTO 로깅을 금지하세요.core/network/src/main/java/kr/co/call/network/dto/login/LoginResponseDto.kt#L7-L20:LoginResponseDto와LoginTokenResult모두 토큰을 마스킹하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/domain/src/main/java/kr/co/call/domain/model/login/LoginToken.kt` around lines 7 - 10, Override the default toString() for every token-bearing data class so raw credentials are never exposed. In core/domain/src/main/java/kr/co/call/domain/model/login/LoginToken.kt:7-10, mask both tokens; in core/network/src/main/java/kr/co/call/network/dto/login/LoginRequestDto.kt:7-10, mask the access token or prevent DTO logging; and in core/network/src/main/java/kr/co/call/network/dto/login/LoginResponseDto.kt:7-20, mask tokens in both LoginResponseDto and LoginTokenResult.settings.gradle.kts (1)
24-26: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winKakao 저장소에 콘텐츠 필터를 추가하세요.
현재 저장소가 모든 group에 대해 활성화되어 있어, 향후 의존성 추가 시 불필요한 저장소 탐색 및 dependency-confusion 위험이 커집니다.
com.kakao.sdk만 허용하도록 제한하세요.제안
maven { - url=uri("https://devrepo.kakao.com/nexus/content/groups/public/") + url = uri("https://devrepo.kakao.com/nexus/content/groups/public/") + content { + includeGroup("com.kakao.sdk") + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@settings.gradle.kts` around lines 24 - 26, Restrict the Kakao Maven repository declaration to the com.kakao.sdk group by adding an exclusive content filter around the repository in the dependency repositories configuration. Keep the existing repository URL unchanged and ensure other dependency groups continue using the appropriate repositories.core/network/src/main/java/kr/co/call/network/interceptor/TokenAuthenticator.kt (1)
20-28: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
TokenAuthenticator에@Singleton스코프를 명시하는 것을 권장합니다.
refreshLock은 앱 전체에서 동시 토큰 재발급을 막기 위한 목적인데, 클래스에 스코프가 지정되지 않아 Dagger가 다른 주입 지점에서 별도 인스턴스를 생성하면 락이 인스턴스별로 분리되어 동기화가 무력화될 수 있습니다. 현재는NetworkModule의 한 곳에서만 주입되어 문제가 드러나지 않지만, 명시적으로@Singleton을 붙여 의도를 고정하는 것이 안전합니다.♻️ 제안하는 수정
+@Singleton class TokenAuthenticator `@Inject` constructor ( private val tokenDataStore: TokenDataStore, private val tokenReissueApi: TokenReissueApi, ): Authenticator {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/network/src/main/java/kr/co/call/network/interceptor/TokenAuthenticator.kt` around lines 20 - 28, Mark TokenAuthenticator with the `@Singleton` scope so all injection sites share the same refreshLock instance and token reissue synchronization remains app-wide. Keep the existing constructor injection and Authenticator implementation unchanged.core/network/src/main/java/kr/co/call/network/di/NetworkModule.kt (1)
61-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winbaseUrl 문자열이 두 곳에 중복 하드코딩되어 있습니다.
provideRetrofit와provideReissueRetrofit에 동일한 baseUrl이 중복 작성되어 있어, 서버 주소 변경 시 한쪽을 누락할 위험이 있습니다. 상수로 추출해 공유하는 것을 권장합니다.♻️ 제안하는 수정
+object NetworkConfig { + const val BASE_URL = "https://api.lovecall.example.com/api/v1/" +} + fun provideRetrofit(...): Retrofit { return Retrofit.Builder() - .baseUrl("https://api.lovecall.example.com/api/v1/") + .baseUrl(NetworkConfig.BASE_URL)Also applies to: 90-90
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/network/src/main/java/kr/co/call/network/di/NetworkModule.kt` at line 61, Extract the duplicated API base URL used by provideRetrofit and provideReissueRetrofit into a shared constant, then reference that constant in both Retrofit builders so future server address changes require updating only one value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/kr/co/call/callfromai/AppScreen.kt`:
- Around line 112-114: Update the LandingSideEffect.NavigateToHome callback in
AppScreen to navigate saved-token users to HomeNavKey instead of
AgreementNavKey. If agreement consent requires separate handling, split it into
a distinct side effect rather than reusing NavigateToHome.
In
`@core/data/src/main/java/kr/co/call/data/repositoryImpl/AgreementRepositoryImpl.kt`:
- Around line 13-17: Update AgreementRepositoryImpl.getTerms so all
agreementApi.getTerms failures, including HTTP, IO, and serialization
exceptions, are caught and converted to the project’s established domain error
or Result type; also map unsuccessful responses to that same domain-level
representation while preserving the response failure details. Do not let
Retrofit or transport exceptions or IllegalStateException escape the repository
boundary.
- Line 18: 성공 응답의 약관 변환 로직에서 response.result.orEmpty()를 사용하지 말고 result 누락을 필수
검증하도록 수정하세요. result가 null이면 빈 목록으로 진행하지 말고 기존 도메인 에러 처리 경로로 반환하며, 값이 존재할 때만 현재
map 변환을 수행하세요.
In
`@core/data/src/main/java/kr/co/call/data/repositoryImpl/LoginRepositoryImpl.kt`:
- Around line 35-46: Update LoginRepositoryImpl’s token handling to validate
both result.accessToken and result.refreshToken with isNotBlank() before
tokenDataStore.saveTokens. Treat null, empty, or whitespace-only token values as
a domain-level error, and persist tokens only when both values are valid.
- Around line 23-33: Update LoginRepositoryImpl around loginApi.login so both
Retrofit/IO exceptions and unsuccessful responses are mapped to the project’s
domain error contract rather than propagated directly. Replace the generic
IllegalStateException(response.message) path and wrap DataSource failures using
the existing domain error type or mapper, preserving successful login behavior.
In `@core/datastore/src/main/java/kr/co/call/datastore/TokenDataStore.kt`:
- Around line 18-58: Update TokenDataStore so ACCESS_TOKEN and REFRESH_TOKEN are
encrypted at rest instead of stored as plaintext Preferences values. Apply the
same secure storage mechanism consistently in getAccessToken, getRefreshToken,
saveTokens, and clearTokens, using androidx.security:security-crypto or the
project’s established encrypted serializer/secret-box utility while preserving
the existing nullable token behavior and APIs.
- Around line 22-29: Update getAccessToken and getRefreshToken in TokenDataStore
so dataStore.data reads catch IOException, including FileNotFoundException, and
fall back to emptyPreferences(). Preserve the existing token lookup behavior
while ensuring storage read failures return null instead of propagating. Apply
the same emptyPreferences fallback consistently with the AuthInterceptor and
TokenAuthenticator runBlocking flows.
In `@core/network/src/main/java/kr/co/call/network/di/NetworkModule.kt`:
- Around line 36-51: Update the logging configuration in provideOkHttpClient and
provideReissueOkHttpClient so BODY-level HttpLoggingInterceptor logging is not
enabled in release or other non-debug builds, preventing Authorization headers
and token request bodies from being logged. Preserve appropriate debug-only
logging behavior while ensuring reissue requests never expose raw accessToken or
refreshToken outside debug builds.
In
`@core/network/src/main/java/kr/co/call/network/interceptor/AuthInterceptor.kt`:
- Around line 42-50: Update String.isAuthFreePath and AUTH_FREE_ENDPOINTS to
compare normalized exact paths instead of using endsWith. Normalize the request
path and configured endpoints consistently, accounting for the base path and
trailing slashes, then return true only for an exact endpoint match so paths
such as /other/auth/kakao remain authenticated.
- Around line 35-39: Update the HttpLoggingInterceptor configuration in
provideOkHttpClient() to redact both Authorization and Cookie headers before
logging, and restrict Level.BODY logging to DEBUG builds only. Apply the same
redaction and DEBUG-only BODY configuration to the token-refresh client so
headers added by AuthInterceptor are never exposed.
- Around line 25-28: Update AuthInterceptor so intercept does not call
tokenDataStore.getAccessToken() through runBlocking or read DataStore
synchronously; instead, use a thread-safe in-memory access-token cache, and
ensure token persistence or refresh flows update that cache whenever the token
changes.
In `@feature/login/impl/src/main/java/kr/co/call/impl/entry/LoginEntryBuilder.kt`:
- Around line 71-92: Update the login failure flow in LoginEntryBuilder so Kakao
login onFailure dispatches the error to a LoginViewModel intent instead of only
logging it. Handle the resulting LoginSideEffect.ShowError in the screen with a
user-visible message such as a Snackbar, while retaining logging only as
supplemental diagnostics.
In `@feature/login/impl/src/main/java/kr/co/call/impl/screen/AgreementScreen.kt`:
- Around line 45-52: PR 설명에 변경된 화면의 시각 검증 자료를 첨부하세요.
feature/login/impl/src/main/java/kr/co/call/impl/screen/AgreementScreen.kt
45-52의 약관 목록·선택 상태 화면,
feature/login/impl/src/main/java/kr/co/call/impl/screen/LandingScreen.kt 80-91의
랜딩 화면, feature/login/impl/src/main/java/kr/co/call/impl/screen/LoginScreen.kt
113-125의 카카오 로그인 화면을 각각 스크린샷 또는 화면 녹화로 제공하세요.
In
`@feature/login/impl/src/main/java/kr/co/call/impl/viewmodel/LandingViewModel.kt`:
- Around line 6-18: ViewModel들이 데이터 저장소와 비즈니스 로직을 직접 의존하지 않고 도메인 UseCase 경계를
사용하도록 변경하세요.
feature/login/impl/src/main/java/kr/co/call/impl/viewmodel/LandingViewModel.kt:6-18의
LandingViewModel은 TokenDataStore 주입을 CheckAutoLoginUseCase로 교체하세요.
feature/login/impl/src/main/java/kr/co/call/impl/viewmodel/LoginViewModel.kt:5-18의
LoginViewModel은 서버 로그인과 토큰 저장을 담당하는 도메인 UseCase를 주입하세요.
feature/login/impl/src/main/java/kr/co/call/impl/viewmodel/AgreementViewModel.kt:5-15의
AgreementViewModel은 약관 조회와 동의 제출을 각각 도메인 UseCase에 위임하세요.
In
`@feature/login/impl/src/main/java/kr/co/call/impl/viewmodel/LoginViewModel.kt`:
- Around line 62-68: Update LoginViewModel.kt lines 62-68 so ShowError is
rendered to the user through the existing Snackbar or Dialog mechanism. In
AgreementViewModel.kt lines 36-42, display terms-loading failures and expose a
retry event; in lines 70-74, display submission failures and allow the user to
retry. Ensure all three ShowError paths provide actionable UI feedback instead
of only logging with Timber.e.
In
`@feature/login/impl/src/main/java/kr/co/call/impl/viewmodel/state/AgreementUiState.kt`:
- Around line 14-17: AgreementUiState의 isRequiredChecked에서 terms가 비어 있지 않은 경우에만
필수 동의 완료로 판단하도록 수정하세요. isAllChecked와 동일한 비어 있지 않은 목록 조건을 추가하고, 약관 로드 전에는 false를
유지하면서 약관이 로드된 뒤의 기존 필수 항목 검사 동작은 보존하세요.
---
Outside diff comments:
In `@feature/login/impl/src/main/java/kr/co/call/impl/screen/AgreementScreen.kt`:
- Around line 164-170: Update the NextButton enabled condition to require both
uiState.isRequiredChecked and uiState.terms.isNotEmpty(), preventing progression
without loaded terms. Also add the same non-empty terms validation in the
submitAgreements() flow so submission remains blocked even if the UI state is
bypassed.
---
Nitpick comments:
In `@core/domain/src/main/java/kr/co/call/domain/model/login/LoginToken.kt`:
- Around line 7-10: Override the default toString() for every token-bearing data
class so raw credentials are never exposed. In
core/domain/src/main/java/kr/co/call/domain/model/login/LoginToken.kt:7-10, mask
both tokens; in
core/network/src/main/java/kr/co/call/network/dto/login/LoginRequestDto.kt:7-10,
mask the access token or prevent DTO logging; and in
core/network/src/main/java/kr/co/call/network/dto/login/LoginResponseDto.kt:7-20,
mask tokens in both LoginResponseDto and LoginTokenResult.
In `@core/network/src/main/java/kr/co/call/network/di/NetworkModule.kt`:
- Line 61: Extract the duplicated API base URL used by provideRetrofit and
provideReissueRetrofit into a shared constant, then reference that constant in
both Retrofit builders so future server address changes require updating only
one value.
In
`@core/network/src/main/java/kr/co/call/network/interceptor/TokenAuthenticator.kt`:
- Around line 20-28: Mark TokenAuthenticator with the `@Singleton` scope so all
injection sites share the same refreshLock instance and token reissue
synchronization remains app-wide. Keep the existing constructor injection and
Authenticator implementation unchanged.
In `@settings.gradle.kts`:
- Around line 24-26: Restrict the Kakao Maven repository declaration to the
com.kakao.sdk group by adding an exclusive content filter around the repository
in the dependency repositories configuration. Keep the existing repository URL
unchanged and ensure other dependency groups continue using the appropriate
repositories.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 973e64e7-f808-40f9-8f7d-0382dd6c51df
📒 Files selected for processing (44)
app/build.gradle.ktsapp/src/main/AndroidManifest.xmlapp/src/main/java/kr/co/call/callfromai/App.ktapp/src/main/java/kr/co/call/callfromai/AppScreen.ktcore/data/src/main/java/kr/co/call/data/di/RepositoryModule.ktcore/data/src/main/java/kr/co/call/data/repositoryImpl/AgreementRepositoryImpl.ktcore/data/src/main/java/kr/co/call/data/repositoryImpl/LoginRepositoryImpl.ktcore/datastore/build.gradle.ktscore/datastore/src/main/java/kr/co/call/datastore/TokenDataStore.ktcore/datastore/src/main/java/kr/co/call/datastore/di/DataStoreModule.ktcore/domain/src/main/java/kr/co/call/domain/model/login/AgreementTerm.ktcore/domain/src/main/java/kr/co/call/domain/model/login/LoginToken.ktcore/domain/src/main/java/kr/co/call/domain/repository/AgreementRepository.ktcore/domain/src/main/java/kr/co/call/domain/repository/LoginRepository.ktcore/network/build.gradle.ktscore/network/src/main/java/kr/co/call/network/api/AgreementApi.ktcore/network/src/main/java/kr/co/call/network/api/LoginApi.ktcore/network/src/main/java/kr/co/call/network/api/TokenReissueApi.ktcore/network/src/main/java/kr/co/call/network/di/NetworkModule.ktcore/network/src/main/java/kr/co/call/network/dto/login/LoginRequestDto.ktcore/network/src/main/java/kr/co/call/network/dto/login/LoginResponseDto.ktcore/network/src/main/java/kr/co/call/network/dto/login/TermsDto.ktcore/network/src/main/java/kr/co/call/network/dto/login/TokenReissueDto.ktcore/network/src/main/java/kr/co/call/network/interceptor/AuthInterceptor.ktcore/network/src/main/java/kr/co/call/network/interceptor/TokenAuthenticator.ktfeature/login/api/src/main/java/kr/co/call/api/LoginRoute.ktfeature/login/impl/build.gradle.ktsfeature/login/impl/src/main/java/kr/co/call/impl/auth/KakaoLoginManager.ktfeature/login/impl/src/main/java/kr/co/call/impl/entry/LoginEntryBuilder.ktfeature/login/impl/src/main/java/kr/co/call/impl/screen/AgreementScreen.ktfeature/login/impl/src/main/java/kr/co/call/impl/screen/LandingScreen.ktfeature/login/impl/src/main/java/kr/co/call/impl/screen/LoginScreen.ktfeature/login/impl/src/main/java/kr/co/call/impl/viewmodel/AgreementSideEffect.ktfeature/login/impl/src/main/java/kr/co/call/impl/viewmodel/AgreementUiState.ktfeature/login/impl/src/main/java/kr/co/call/impl/viewmodel/AgreementViewModel.ktfeature/login/impl/src/main/java/kr/co/call/impl/viewmodel/LandingSideEffect.ktfeature/login/impl/src/main/java/kr/co/call/impl/viewmodel/LandingViewModel.ktfeature/login/impl/src/main/java/kr/co/call/impl/viewmodel/LoginSideEffect.ktfeature/login/impl/src/main/java/kr/co/call/impl/viewmodel/LoginViewModel.ktfeature/login/impl/src/main/java/kr/co/call/impl/viewmodel/state/AgreementUiState.ktfeature/login/impl/src/main/java/kr/co/call/impl/viewmodel/state/LandingUiState.ktfeature/login/impl/src/main/java/kr/co/call/impl/viewmodel/state/LoginState.ktgradle/libs.versions.tomlsettings.gradle.kts
💤 Files with no reviewable changes (1)
- feature/login/impl/src/main/java/kr/co/call/impl/viewmodel/AgreementUiState.kt
Comment on lines
+71
to
+92
| loginViewModel.collectSideEffect { sideEffect -> | ||
| when (sideEffect) { | ||
| LoginSideEffect.NavigateToNext -> { | ||
| navigateToAgreement() | ||
| } | ||
|
|
||
| is LoginSideEffect.ShowError -> { | ||
| Timber.e(sideEffect.message) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| LoginScreen( | ||
| onKakaoLoginClick={ | ||
| kakaoLoginManager.login( | ||
| context=context, | ||
| onSuccess=loginViewModel::loginWithKakao, | ||
| onFailure={error -> | ||
| Timber.e(error,"카카오 로그인 실패") | ||
| }, | ||
| onCancel={Timber.d("카카오 로그인 취소")} | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
로그인 실패를 사용자에게 표시하세요.
카카오 로그인 실패와 ShowError가 모두 로그로만 처리되어 사용자는 실패 원인을 알 수 없습니다. 실패 콜백을 ViewModel intent로 전달하고, 화면에서 표시 가능한 side effect(예: Snackbar)로 처리하세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@feature/login/impl/src/main/java/kr/co/call/impl/entry/LoginEntryBuilder.kt`
around lines 71 - 92, Update the login failure flow in LoginEntryBuilder so
Kakao login onFailure dispatches the error to a LoginViewModel intent instead of
only logging it. Handle the resulting LoginSideEffect.ShowError in the screen
with a user-visible message such as a Snackbar, while retaining logging only as
supplemental diagnostics.
Comment on lines
45
to
52
| fun AgreementScreen( | ||
| modifier: Modifier, | ||
| uiState: AgreementUiState, | ||
| onNextClick:()->Unit, | ||
| onAgreementViewClick:(AgreementType)->Unit, | ||
| onAgreementToggle: (AgreementType)->Unit, | ||
| onAgreementViewClick:(AgreementTerm)->Unit, | ||
| onAgreementToggle: (Long)->Unit, | ||
| onAllAgreementsCheckedChange: (Boolean) -> Unit | ||
| ) { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
변경된 화면의 시각 검증 자료를 PR에 첨부해 주세요.
제공된 PR 설명에는 스크린샷 또는 화면 녹화가 없습니다.
feature/login/impl/src/main/java/kr/co/call/impl/screen/AgreementScreen.kt#L45-L52: 약관 목록·선택 상태가 보이는 화면을 첨부하세요.feature/login/impl/src/main/java/kr/co/call/impl/screen/LandingScreen.kt#L80-L91: 랜딩 화면을 첨부하세요.feature/login/impl/src/main/java/kr/co/call/impl/screen/LoginScreen.kt#L113-L125: 카카오 로그인 화면을 첨부하세요.
As per coding guidelines, "*Screen.kt 변경 시 PR 설명에 스크린샷 또는 화면 녹화가 첨부되었는지 확인"해야 합니다.
📍 Affects 3 files
feature/login/impl/src/main/java/kr/co/call/impl/screen/AgreementScreen.kt#L45-L52(this comment)feature/login/impl/src/main/java/kr/co/call/impl/screen/LandingScreen.kt#L80-L91feature/login/impl/src/main/java/kr/co/call/impl/screen/LoginScreen.kt#L113-L125
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@feature/login/impl/src/main/java/kr/co/call/impl/screen/AgreementScreen.kt`
around lines 45 - 52, PR 설명에 변경된 화면의 시각 검증 자료를 첨부하세요.
feature/login/impl/src/main/java/kr/co/call/impl/screen/AgreementScreen.kt
45-52의 약관 목록·선택 상태 화면,
feature/login/impl/src/main/java/kr/co/call/impl/screen/LandingScreen.kt 80-91의
랜딩 화면, feature/login/impl/src/main/java/kr/co/call/impl/screen/LoginScreen.kt
113-125의 카카오 로그인 화면을 각각 스크린샷 또는 화면 녹화로 제공하세요.
Source: Coding guidelines
There was a problem hiding this comment.
@codebidoof 넵! 이 환경에서는 앱을 실행해 실제 화면을 캡처할 수는 없어서, PR 작성자분께 아래 화면 스크린샷(또는 하나의 화면 녹화)을 PR 설명에 첨부해 달라고 요청드린 상태입니다.
- 랜딩 화면
- 카카오 로그인 화면
- 약관 목록 및 필수/선택 동의 상태가 보이는 화면
첨부되면 시각적으로도 확인하기 훨씬 좋겠습니다 🐰
Comment on lines
+62
to
+68
| // 서버 또는 카카오 로그인 오류 메시지를 화면에 전달 | ||
| postSideEffect( | ||
| LoginSideEffect.ShowError( | ||
| message = error.message | ||
| ?: "로그인 실패", | ||
| ), | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
오류 SideEffect를 사용자에게 표시하세요.
제공된 LoginEntryBuilder 소비 코드에서 세 ShowError 경로는 모두 Timber.e만 호출합니다. 로그인 실패는 아무 안내 없이 남고, 약관 조회 실패는 빈 화면에서 재시도 수단도 없습니다.
feature/login/impl/src/main/java/kr/co/call/impl/viewmodel/LoginViewModel.kt#L62-L68: 로그인 화면에서ShowError를 Snackbar 또는 Dialog로 표시하세요.feature/login/impl/src/main/java/kr/co/call/impl/viewmodel/AgreementViewModel.kt#L36-L42: 약관 조회 실패를 표시하고 재시도 이벤트를 제공하세요.feature/login/impl/src/main/java/kr/co/call/impl/viewmodel/AgreementViewModel.kt#L70-L74: 약관 제출 실패를 화면에 표시해 사용자가 재시도할 수 있게 하세요.
📍 Affects 2 files
feature/login/impl/src/main/java/kr/co/call/impl/viewmodel/LoginViewModel.kt#L62-L68(this comment)feature/login/impl/src/main/java/kr/co/call/impl/viewmodel/AgreementViewModel.kt#L36-L42feature/login/impl/src/main/java/kr/co/call/impl/viewmodel/AgreementViewModel.kt#L70-L74
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@feature/login/impl/src/main/java/kr/co/call/impl/viewmodel/LoginViewModel.kt`
around lines 62 - 68, Update LoginViewModel.kt lines 62-68 so ShowError is
rendered to the user through the existing Snackbar or Dialog mechanism. In
AgreementViewModel.kt lines 36-42, display terms-loading failures and expose a
retry event; in lines 70-74, display submission failures and allow the user to
retry. Ensure all three ShowError paths provide actionable UI feedback instead
of only logging with Timber.e.
Gemini AI 코드리뷰안녕하세요, 시니어 Android 개발자로서 해당 Pull Request를 리뷰하겠습니다. 전반적으로 카카오 로그인, 토큰 관리, 약관 동의 플로우를 잘 구현한 PR로 보입니다. 특히 네트워크 계층에서 인증 및 토큰 재발급 로직을 꼼꼼하게 처리한 점이 인상 깊습니다. 아래는 각 항목별 상세 리뷰 및 몇 가지 개선 제안입니다. 1.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/datastore/src/main/java/kr/co/call/datastore/TokenDataStore.kt`:
- Around line 165-169: Update Preferences.toStoredTokens so both accessToken and
refreshToken are normalized to null when their stored values are blank, ensuring
getAccessToken() and getCachedAccessToken() do not treat whitespace-only tokens
as valid.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 69422401-26fd-4a1b-aec8-5eeb886425fd
📒 Files selected for processing (15)
app/src/main/java/kr/co/call/callfromai/AppScreen.ktcore/data/src/main/java/kr/co/call/data/repositoryImpl/AgreementRepositoryImpl.ktcore/data/src/main/java/kr/co/call/data/repositoryImpl/LoginRepositoryImpl.ktcore/datastore/build.gradle.ktscore/datastore/src/main/java/kr/co/call/datastore/TokenDataStore.ktcore/datastore/src/main/java/kr/co/call/datastore/di/DataStoreModule.ktcore/domain/src/main/java/kr/co/call/domain/repository/AgreementRepository.ktcore/domain/src/main/java/kr/co/call/domain/repository/LoginRepository.ktcore/network/src/main/java/kr/co/call/network/di/NetworkModule.ktcore/network/src/main/java/kr/co/call/network/interceptor/AuthInterceptor.ktcore/network/src/main/java/kr/co/call/network/interceptor/TokenAuthenticator.ktfeature/login/impl/src/main/java/kr/co/call/impl/entry/LoginEntryBuilder.ktfeature/login/impl/src/main/java/kr/co/call/impl/viewmodel/AgreementViewModel.ktfeature/login/impl/src/main/java/kr/co/call/impl/viewmodel/LoginViewModel.ktfeature/login/impl/src/main/java/kr/co/call/impl/viewmodel/state/AgreementUiState.kt
🚧 Files skipped from review as they are similar to previous changes (9)
- core/datastore/build.gradle.kts
- core/domain/src/main/java/kr/co/call/domain/repository/LoginRepository.kt
- feature/login/impl/src/main/java/kr/co/call/impl/viewmodel/state/AgreementUiState.kt
- core/datastore/src/main/java/kr/co/call/datastore/di/DataStoreModule.kt
- app/src/main/java/kr/co/call/callfromai/AppScreen.kt
- core/network/src/main/java/kr/co/call/network/interceptor/TokenAuthenticator.kt
- feature/login/impl/src/main/java/kr/co/call/impl/viewmodel/AgreementViewModel.kt
- feature/login/impl/src/main/java/kr/co/call/impl/entry/LoginEntryBuilder.kt
- core/network/src/main/java/kr/co/call/network/di/NetworkModule.kt
Gemini AI 코드리뷰안녕하세요, 시니어 Android 개발자입니다. PR을 꼼꼼히 리뷰해 보았습니다. 이번 PR은 카카오 로그인 및 약관 동의 기능을 구현하며, Hilt DI, Retrofit 네트워크 처리, DataStore 기반 토큰 관리 등 중요한 아키텍처 요소들을 매우 안정적으로 통합하고 있습니다. 전반적으로 코드 품질이 높고, 여러 상황에 대한 예외 처리와 견고성이 돋보입니다. PR 제목: 카카오 로그인 및 약관 동의 기능 구현 ✅ 전반적인 리뷰 요약
🛠️ 상세 코드 리뷰1. Kotlin 코드 리뷰
2. Jetpack Compose 코드
3. Repository/DataSource 레이어
4. ViewModel
5. FCM/SSE/실시간 통신 관련 코드
💡 최종 결론 및 권장사항이 PR은 매우 높은 품질의 코드를 포함하고 있으며, 핵심적인 기능과 아키텍처 원칙을 훌륭하게 준수하고 있습니다. 특히 토큰 관리 및 네트워크 처리 부분은 많은 고려와 노력이 들어간 것으로 보입니다. 두 가지 권장사항을 반영하면 더욱 완벽해질 것입니다:
이러한 개선사항들을 적용하면 더욱 유지보수하기 쉽고, 확장성 있는 코드가 될 것입니다. 수고 많으셨습니다! |
JiwonLee42
reviewed
Jul 26, 2026
| val content: String, | ||
| val isRequired: Boolean, | ||
| ) | ||
|
|
Collaborator
There was a problem hiding this comment.
이거 나중에 아원이가 공통 응답 data class 만들면 수정하면 될듯합니당
JiwonLee42
reviewed
Jul 26, 2026
| data class AgreementUiState( | ||
| val terms: List<AgreementTerm> = emptyList(), | ||
| val checkedTermIds: Set<Long> = emptySet(), | ||
| val isLoading: Boolean = false, |
Collaborator
There was a problem hiding this comment.
val status: LoadStatus = LoadStatus.Idle 요런식으로 수정하는게 좋을 것 같아용
다른 UiState도 마찬가지로 하면 좋을 것 같습니다!!
codebidoof
reviewed
Jul 26, 2026
codebidoof
left a comment
codebidoof
left a comment
Collaborator
There was a problem hiding this comment.
고생했어!! LandingViewModel 쪽은 수정이 필요할 듯 해잉
| ), | ||
| ) | ||
| }, | ||
| navigateAfterAgreement={ |
Collaborator
There was a problem hiding this comment.
오 파라미터 이름 직관적이라 너무 조아요
|
|
||
| val kakaoNativeAppKey = | ||
| localProperties.getProperty("KAKAO_NATIVE_APP_KEY") | ||
| ?: error("local.properties에 KAKAO_NATIVE_APP_KEY가 없음") |
Collaborator
There was a problem hiding this comment.
val kakaoNativeAppKey =
localProperties.getProperty("KAKAO_NATIVE_APP_KEY")
?: throw GradleException(
"""
local.properties에 KAKAO_NATIVE_APP_KEY가 없습니다.
local.properties에 다음을 추가해주세요.
KAKAO_NATIVE_APP_KEY=xxxxxxxxxxxxxxxx
""".trimIndent()
)이런 식으로 error 말구 GradleException을 발생시키는 게 더 조을 것 가타용
| interface AgreementRepository { | ||
| suspend fun getTerms(): Result<List<AgreementTerm>> | ||
| suspend fun agreeTerms( | ||
| agreements: Map<Long, Boolean> |
Collaborator
There was a problem hiding this comment.
그냥 인자 2개 넣는 거로 해도 될 것 같은데? Map 이 오히려 코드 복잡도를 늘리는 느낌이야
| okHttpClient: OkHttpClient, | ||
| ): Retrofit{ | ||
| return Retrofit.Builder() | ||
| .baseUrl("https://api.lovecall.example.com/api/v1/") |
Collaborator
There was a problem hiding this comment.
아원이가 세팅한 뒤에는 이거 BuildConfig에서 불러오는 식으로 수정이 필요할 것 같아용 보안 문제때문에
| import com.kakao.sdk.user.UserApiClient | ||
| import timber.log.Timber | ||
|
|
||
| class KakaoLoginManager { |
Comment on lines
45
to
52
| fun AgreementScreen( | ||
| modifier: Modifier, | ||
| uiState: AgreementUiState, | ||
| onNextClick:()->Unit, | ||
| onAgreementViewClick:(AgreementType)->Unit, | ||
| onAgreementToggle: (AgreementType)->Unit, | ||
| onAgreementViewClick:(AgreementTerm)->Unit, | ||
| onAgreementToggle: (Long)->Unit, | ||
| onAllAgreementsCheckedChange: (Boolean) -> Unit | ||
| ) { |
Comment on lines
+5
to
+18
| data class AgreementUiState( | ||
| val terms: List<AgreementTerm> = emptyList(), | ||
| val checkedTermIds: Set<Long> = emptySet(), | ||
| val isLoading: Boolean = false, | ||
| ){ | ||
| val isAllChecked: Boolean | ||
| get() = terms.isNotEmpty() && | ||
| terms.all { it.termId in checkedTermIds } | ||
|
|
||
| val isRequiredChecked: Boolean | ||
| get() = terms.isNotEmpty() && terms | ||
| .filter { it.isRequired } | ||
| .all { it.termId in checkedTermIds } | ||
| } No newline at end of file |
Collaborator
There was a problem hiding this comment.
주석 남겨주시면 리뷰할 때 훨씬 편할 것 같아용
Comment on lines
+7
to
+9
| data class LandingUiState( | ||
| val isCheckingAutoLogin:Boolean=true, | ||
| ) No newline at end of file |
Collaborator
There was a problem hiding this comment.
필드가 하나뿐인데 data class 로 정의하신 이유가 있을까용?
Comment on lines
+16
to
+50
| @HiltViewModel | ||
| class LandingViewModel @Inject constructor( | ||
| private val tokenDataStore: TokenDataStore, | ||
| ): ViewModel(), | ||
| ContainerHost<LandingUiState, LandingSideEffect> { | ||
| override val container= container <LandingUiState,LandingSideEffect>( | ||
| initialState=LandingUiState(), | ||
| ) | ||
| init { | ||
| // 랜딩 화면이 생성되면 자동 로그인 확인을 바로 시작 | ||
| checkAutoLoginAfterSplash() | ||
| } | ||
| /** | ||
| * 스플래시 화면 노출 후 저장된 Access Token을 확인한다. | ||
| * 현재는 토큰 존재 여부를 기준으로 자동 로그인 화면을 결정한다. | ||
| */ | ||
| private fun checkAutoLoginAfterSplash()=intent{ | ||
| delay(SPLASH_DURATION_MILLIS) | ||
|
|
||
| val accessToken = tokenDataStore.getAccessToken() | ||
| reduce{ | ||
| state.copy( | ||
| isCheckingAutoLogin = false, | ||
| ) | ||
| } | ||
| if (accessToken.isNullOrBlank()){ | ||
| postSideEffect(LandingSideEffect.NavigateToLogin) | ||
| }else{ | ||
| postSideEffect(LandingSideEffect.NavigateToHome) | ||
| } | ||
| } | ||
| private companion object { | ||
| const val SPLASH_DURATION_MILLIS=3_000L | ||
| } | ||
| } No newline at end of file |
Collaborator
There was a problem hiding this comment.
현재 3초 뒤에 토큰 읽기 작업을 시작하는 방식으로 로직이 짜여져 있는데, SPLASH_DURATION_MILLIS는 스플래시 화면(랜딩스크린)을 띄우는 최소 시간만을 정의하는 식으로 사용하고 자동 로그인 체크는 화면 진입과 동시에 수행하는 방식이 더 적절할 것 같아!! 이후 체크가 완료되면 남은 시간만큼만 대기한 뒤 화면을 전환하면 될 것 같앙 예를 들면 이런 느낌으로?
private fun checkAutoLoginAfterSplash() = intent {
val splashStartTime = System.currentTimeMillis()
// 토큰 조회는 바로 시작
val accessToken = tokenDataStore.getAccessToken()
// 스플래시 최소 노출 시간 보장
val elapsedTime = System.currentTimeMillis() - splashStartTime
val remainingTime = SPLASH_DURATION_MILLIS - elapsedTime
if (remainingTime > 0) {
delay(remainingTime)
}
reduce {
state.copy(
isCheckingAutoLogin = false,
)
}
if (accessToken.isNullOrBlank()) {
postSideEffect(LandingSideEffect.NavigateToLogin)
} else {
postSideEffect(LandingSideEffect.NavigateToHome)
}
}
private companion object {
const val SPLASH_DURATION_MILLIS = 3_000L
}
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
3 participants
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.
📄 작업 내용 요약
-카카오 소셜 로그인 연동
-토큰 재발급 로직 구현
-서버 인증 토큰 관리
📎 Issue 번호
✅ 작업 목록
📝 기타 참고사항
Summary by CodeRabbit