[FEAT/#6] 카카오 로그인 구현#22
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthrough카카오 로그인 통합을 위해 저장소와 의존성을 추가하고, 각 앱 모듈에서 카카오 키를 빌드 설정과 매니페스트에 주입했습니다. 앱 시작 시 Kakao SDK를 초기화하도록 변경했고, 로그인 매니저·뷰모델·화면을 연결해 로그인 결과를 처리합니다. 홈 이동에는 백스택 정리 옵션이 적용됩니다. Changes카카오 로그인 통합
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant LoginScreen
participant LoginViewModel
participant KakaoLoginManager
participant KakaoSDK
LoginScreen->>LoginViewModel: onLoginClick(context)
LoginViewModel->>KakaoLoginManager: login(context)
KakaoLoginManager->>KakaoSDK: loginWithKakaoTalk / loginWithKakaoAccount
KakaoSDK-->>KakaoLoginManager: OAuthToken 또는 오류
KakaoLoginManager-->>LoginViewModel: onResult(Result)
LoginViewModel-->>LoginScreen: ShowToast / NavigateToHome Effect
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 4
🧹 Nitpick comments (3)
presentation/auth/src/main/java/com/ssing/presentation/auth/LoginScreen.kt (2)
57-57: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value하드코딩된 문자열 사용.
Text("로그인"),Text("임시 로그인 버튼"),Text("카카오 연결 해제")모두 리터럴 문자열입니다.stringResource()를 사용해 리소스로 분리하는 것이 좋습니다. As per path instructions, "하드코딩 색상·문자열 회피, MaterialTheme/stringResource 사용".Also applies to: 64-64, 80-80
🤖 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 `@presentation/auth/src/main/java/com/ssing/presentation/auth/LoginScreen.kt` at line 57, The LoginScreen composables are using hardcoded Korean text literals, which should be moved to string resources. Update the Text calls in LoginScreen to use stringResource() instead of inline strings for the login title, temporary login button label, and Kakao disconnect label, and add or reuse the corresponding resource entries so the UI text is centralized and localizable.Source: Path instructions
67-81: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUI 레이어에서 SDK 직접 호출 (ViewModel 우회).
"카카오 연결 해제" 버튼이
UserApiClient.instance.unlink를 Composable에서 직접 호출합니다.onKakaoClick처럼 ViewModel을 통해 처리하도록 위임하면 계층 분리가 유지되고 테스트도 쉬워집니다.[TEST]주석과 PR 설명상 임시 버튼임은 인지하고 있으나, 정식 배포 전 제거 또는 ViewModel 경유로 정리하는 것을 권장합니다.🤖 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 `@presentation/auth/src/main/java/com/ssing/presentation/auth/LoginScreen.kt` around lines 67 - 81, “카카오 연결 해제” 버튼이 Composable인 LoginScreen에서 UserApiClient.instance.unlink를 직접 호출하는 UI/SDK 결합 문제입니다. LoginScreen의 버튼 onClick 로직을 제거하고, onKakaoClick처럼 ViewModel의 이벤트/메서드로 위임한 뒤 ViewModel에서 unlink를 처리하도록 옮기세요. 임시 [TEST] 버튼이라면 정식 배포 전에 삭제하거나, 최소한 UI는 상태/이벤트 전달만 하도록 분리해 계층 경계를 유지하세요.presentation/auth/src/main/java/com/ssing/presentation/auth/LoginViewModel.kt (1)
18-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value불필요한 이중
viewModelScope.launch중첩.
kakaoLoginManager.login()은 suspend 함수가 아니라 콜백 등록 후 즉시 반환되므로, 바깥쪽launch는 실질적인 비동기 대기 없이 곧바로 끝납니다. 실제 효과 전송은 콜백 내부의 두 번째launch에서 일어나므로 바깥launch는 제거해도 동작이 동일하며 가독성이 좋아집니다.♻️ 제안 리팩터
fun onLoginClick(context: Context) { - viewModelScope.launch { - kakaoLoginManager.login(context) { result -> - viewModelScope.launch { - result.onSuccess { token -> - sendEffect(LoginContract.Effect.ShowToast("로그인 되었습니다.")) - sendEffect(LoginContract.Effect.NavigateToHome) - - // TODO: 서버 연결 시 token.accessToken 전달 - }.onFailure { error -> - sendEffect(LoginContract.Effect.ShowToast("로그인에 실패했습니다.")) - - // TODO: 실패 처리 - } - } - } + kakaoLoginManager.login(context) { result -> + viewModelScope.launch { + result.onSuccess { token -> + sendEffect(LoginContract.Effect.ShowToast("로그인 되었습니다.")) + sendEffect(LoginContract.Effect.NavigateToHome) + + // TODO: 서버 연결 시 token.accessToken 전달 + }.onFailure { error -> + sendEffect(LoginContract.Effect.ShowToast("로그인에 실패했습니다.")) + + // TODO: 실패 처리 + } + } } }🤖 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 `@presentation/auth/src/main/java/com/ssing/presentation/auth/LoginViewModel.kt` around lines 18 - 34, `LoginViewModel.onLoginClick` has an unnecessary outer `viewModelScope.launch` because `kakaoLoginManager.login()` only registers a callback and returns immediately, while the actual work happens in the inner launch inside the callback. Remove the outer coroutine wrapper and keep the effect handling in the callback path so `onLoginClick`, `kakaoLoginManager.login`, and the `result.onSuccess`/`onFailure` branches stay functionally the same but with simpler flow.
🤖 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/consumer/build.gradle.kts`:
- Around line 8-10: `properties` 로딩 로직이 `local.properties` 부재나
`KAKAO_NATIVE_APP_KEY` 누락을 검증하지 않아 빌드가 조용히 실패할 수 있습니다. `Properties().apply {
load(...) }`를 사용하는 부분에서 파일 존재 여부와 키 존재 여부를 명시적으로 확인하고, 없으면 즉시 의미 있는 예외로 실패하도록
처리하세요. `build.gradle.kts`의 이 로직은 `app/consumer`와 동일 패턴이 있는 `app/instructor`에도
적용해 중복된 검증을 함께 정리하세요.
In `@app/instructor/build.gradle.kts`:
- Around line 9-11: `properties` 초기화에서 local.properties가 없거나
KAKAO_NATIVE_APP_KEY가 누락된 경우를 안전하게 처리하도록 `app/instructor/build.gradle.kts`의
properties 로딩 로직을 보강하세요.
`load(rootProject.file("local.properties").inputStream())`를 수행하기 전에 파일 존재를 확인하고,
키 조회 시에는 `Properties`에서 값을 검증해 null/빈 값이면 명확히 실패하도록 `properties`,
`KAKAO_NATIVE_APP_KEY`, 그리고 해당 값을 사용하는 구성 로직을 함께 수정하세요.
In `@gradle/libs.versions.toml`:
- Line 27: Update the Kakao SDK dependency version to the latest release by
changing the kakao entry in the versions catalog from the current value to
2.24.0. Keep the change limited to the version declaration so any references
that consume kakao through the catalog pick up the new release automatically.
In
`@presentation/auth/src/main/java/com/ssing/presentation/auth/KakaoLoginManager.kt`:
- Around line 30-41: The Kakao account login failure branch in
KakaoLoginManager.loginWithKakaoAccount is logging the wrong error variable.
Update the Timber.e call inside the loginWithKakaoAccount callback to use error2
instead of the outer-scope error, so the logged failure matches the actual Kakao
account login result.
---
Nitpick comments:
In `@presentation/auth/src/main/java/com/ssing/presentation/auth/LoginScreen.kt`:
- Line 57: The LoginScreen composables are using hardcoded Korean text literals,
which should be moved to string resources. Update the Text calls in LoginScreen
to use stringResource() instead of inline strings for the login title, temporary
login button label, and Kakao disconnect label, and add or reuse the
corresponding resource entries so the UI text is centralized and localizable.
- Around line 67-81: “카카오 연결 해제” 버튼이 Composable인 LoginScreen에서
UserApiClient.instance.unlink를 직접 호출하는 UI/SDK 결합 문제입니다. LoginScreen의 버튼 onClick
로직을 제거하고, onKakaoClick처럼 ViewModel의 이벤트/메서드로 위임한 뒤 ViewModel에서 unlink를 처리하도록
옮기세요. 임시 [TEST] 버튼이라면 정식 배포 전에 삭제하거나, 최소한 UI는 상태/이벤트 전달만 하도록 분리해 계층 경계를 유지하세요.
In
`@presentation/auth/src/main/java/com/ssing/presentation/auth/LoginViewModel.kt`:
- Around line 18-34: `LoginViewModel.onLoginClick` has an unnecessary outer
`viewModelScope.launch` because `kakaoLoginManager.login()` only registers a
callback and returns immediately, while the actual work happens in the inner
launch inside the callback. Remove the outer coroutine wrapper and keep the
effect handling in the callback path so `onLoginClick`,
`kakaoLoginManager.login`, and the `result.onSuccess`/`onFailure` branches stay
functionally the same but with simpler flow.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1745a25a-9b60-47de-a0d6-6d5f5cd1760c
📒 Files selected for processing (14)
app/consumer/build.gradle.ktsapp/consumer/src/main/AndroidManifest.xmlapp/consumer/src/main/java/com/ssing/consumer/ConsumerMainNavHost.ktapp/consumer/src/main/java/com/ssing/consumer/SsingConsumerApp.ktapp/instructor/build.gradle.ktsapp/instructor/src/main/AndroidManifest.xmlapp/instructor/src/main/java/com/ssing/instructor/InstructorMainNavHost.ktapp/instructor/src/main/java/com/ssing/instructor/SsingInstructorApp.ktgradle/libs.versions.tomlpresentation/auth/build.gradle.ktspresentation/auth/src/main/java/com/ssing/presentation/auth/KakaoLoginManager.ktpresentation/auth/src/main/java/com/ssing/presentation/auth/LoginScreen.ktpresentation/auth/src/main/java/com/ssing/presentation/auth/LoginViewModel.ktsettings.gradle.kts
| when (effect) { | ||
| is LoginContract.Effect.NavigateToHome -> navigateToHome() | ||
| is LoginContract.Effect.ShowToast -> {} | ||
| is LoginContract.Effect.ShowToast -> Toast.makeText(context, effect.message, Toast.LENGTH_SHORT).show() |
There was a problem hiding this comment.
p2 : Context 확장함수 있어요! 사용하면 더 편합니당
| is LoginContract.Effect.ShowToast -> Toast.makeText(context, effect.message, Toast.LENGTH_SHORT).show() | |
| is LoginContract.Effect.ShowToast -> context.toast(effect.message) |
There was a problem hiding this comment.
와 코드가 엄청 쭐었다!! 이런게 있다니 호오
바로 반영해씁니다! 🫶🫶
| ) { | ||
|
|
||
| fun onLoginClick() { | ||
| fun onLoginClick(context: Context) { |
There was a problem hiding this comment.
p2 : 이 메소드명은 kakaoLogin()이 되어야하지 않을까용?
| ) { | ||
|
|
||
| fun onLoginClick() { | ||
| fun onLoginClick(context: Context) { |
| navigateToHome = { navController.navigate( | ||
| ConsumerHome, | ||
| navController.clearBackStackNavOptions(), | ||
| ) }, |
There was a problem hiding this comment.
p2 : 자동정렬 쓰고 잇나욤~? 윈도우 기준 ctrl + alt + l 단축키
그리고 파라미터 둘 이상일 때는 named로 써줍시다!
| navigateToHome = { navController.navigate( | |
| ConsumerHome, | |
| navController.clearBackStackNavOptions(), | |
| ) }, | |
| navigateToHome = { | |
| navController.navigate( | |
| route = ConsumerHome, | |
| navOptions = navController.clearBackStackNavOptions(), | |
| ) | |
| }, |
| navigateToHome = { navController.navigate( | ||
| InstructorHome, | ||
| navController.clearBackStackNavOptions(), | ||
| ) }, |
There was a problem hiding this comment.
p2 : 여기도 정렬이랑 named 파라미터 신경 써주세용!!
|
|
||
| @EntryPoint | ||
| @InstallIn(ActivityComponent::class) | ||
| interface KakaoLoginEntryPoint { |
There was a problem hiding this comment.
p1 : 이거 미사용 되고 있네요!! Screen에서 Activity로 열어줘야할 거예요 이것도 예시코드에 있으니까 참고!!
| versionName = "1.0" | ||
|
|
||
| val kakaoNativeAppKey = properties.getProperty("KAKAO_NATIVE_APP_KEY").orEmpty() | ||
| require(kakaoNativeAppKey.isNotBlank()) { |
There was a problem hiding this comment.
이거 코드래빗 말 듣고 local properties가 없을 경우 처리한건데 CI에 local properties가 없어서 통과가 안된다요,... 이거 어떻게 해야할까요 ,..🥹🥹
There was a problem hiding this comment.
엇 이거 깃헙에서 빌드할 때 secrets에서 local.properties 만들어주는거 아닌가..?? 유빈언니가 넣어줘야하남..???
There was a problem hiding this comment.
아아 다시 봤는데 local.properties는 있는데 안에 네이티브키가 안 들어있어서 그런 거 같은디... 유빈언니한테 한 번 물어볼께용
joyrii
left a comment
joyrii
left a comment
There was a problem hiding this comment.
어려웠을 것 같은데 넘 고생 많았어요 ㅠㅠ 일단 나라면 이렇게 못했을 것 같다... 짱짱!!
| Timber.i("🍫 카카오 계정 로그인 성공") | ||
| onResult(Result.success(token2)) | ||
| } else { | ||
| Timber.e(error2, "🍫 카카오계정 로그인 실패") |
There was a problem hiding this comment.
p3: 카카오라 초콜릿 ...🍫🍫? 귀여워서 바닥 부셨어요
| if (error is ClientError && error.reason == ClientErrorCause.Cancelled) { | ||
| Timber.d("🍫 카카오톡 로그인 취소") | ||
| return@loginWithKakaoTalk | ||
| } |
There was a problem hiding this comment.
p3: 로그인 취소가 되었을 때에는 result 반환을 안 해도 괜찮은지 궁금해요!
| private fun initKakaoSdk() { | ||
| KakaoSdk.init(this, BuildConfig.KAKAO_NATIVE_APP_KEY) | ||
| } |
There was a problem hiding this comment.
p3: 앱이 두 개라 따로 설정해줘야 하는군요... 어렵따 엄청 헷갈렸겠는데!!
oilbeaneda
left a comment
oilbeaneda
left a comment
There was a problem hiding this comment.
예리미 너무너무 고생많앗다 너무 잘하고 너무 멋져 !!!!!!! 늘 많이 배우고 열심히 하는 모습 저두 많이 배워요 ❤️
| if (error is ClientError && error.reason == ClientErrorCause.Cancelled) { | ||
| Timber.d("🍫 카카오톡 로그인 취소") | ||
| return@loginWithKakaoTalk |
There was a problem hiding this comment.
| if (error is ClientError && error.reason == ClientErrorCause.Cancelled) { | |
| Timber.d("🍫 카카오톡 로그인 취소") | |
| return@loginWithKakaoTalk | |
| if (error is ClientError && error.reason == ClientErrorCause.Cancelled) { | |
| Timber.d("🍫 카카오톡 로그인 취소") | |
| onResult(Result.failure(error)) | |
| return@loginWithKakaoTalk | |
| } |
p1: 사용자가 로그인을 취소할 경우 onResult 콜백을 호출하지 않고 return 하고 있어서 취소 시에도 실패 결과를 넘겨주면 좋을 것 같아요 ㅎㅎ
There was a problem hiding this comment.
엇 단순 사용자 취소면 오류없이 조용히 넘어가는 편이 좋지 않을까요??
There was a problem hiding this comment.
호오 그르네요 취소했을 때는 실패한게 아닌데 실패라고 뜨면 안되겟네!!!!!!1 취소했을 때도 onResult 불려서 사용자 취소라고 구분해주면 좋을 것 같다고 생각했는데 여기서는 의미 없띠니 쓰루하장 ㅎㅎ
| import dagger.hilt.EntryPoint | ||
| import dagger.hilt.InstallIn | ||
| import dagger.hilt.android.components.ActivityComponent | ||
| import jakarta.inject.Inject |
There was a problem hiding this comment.
p1:
| import jakarta.inject.Inject | |
| import javax.inject |
로 써주세용
이유는 javax. 는 오랫동안 사용되어 온 자바 표준 DI 패키지인데 ilt 공식 문서나 예제 코드에서도 아직은 javax.inject를 기본으로 사용하는 경우가 훨씬 많아요 글구 다른파일들은 다 javax 임! ㅎㅎ
| import jakarta.inject.Inject | ||
| import timber.log.Timber | ||
|
|
||
| class KakaoLoginManager @Inject constructor() { |
There was a problem hiding this comment.
p2:internal붙여주는 거 어때요 ㅎㅎ
|
|
||
| @EntryPoint | ||
| @InstallIn(ActivityComponent::class) | ||
| interface KakaoLoginEntryPoint { |
| viewModelScope.launch { | ||
| sendEffect(LoginContract.Effect.NavigateToHome) | ||
| kakaoLoginManager.login(context) { result -> | ||
| viewModelScope.launch { | ||
| result.onSuccess { token -> | ||
| sendEffect(LoginContract.Effect.ShowToast("로그인 되었습니다.")) | ||
| sendEffect(LoginContract.Effect.NavigateToHome) |
There was a problem hiding this comment.
p2:
viewModelScope.launch { kakaoLoginManager.login(context) { result -> result.onSuccess { token -> sendEffect(LoginContract.Effect.ShowToast("로그인 되었습니다.")) sendEffect(LoginContract.Effect.NavigateToHome)
이렇게 한번에 적어줄 수 있을 것 같은데 viewModelScope.launch 를 두 번 감싼 이유가 있을까용?
| internal class LoginViewModel @Inject constructor( | ||
| private val kakaoLoginManager: KakaoLoginManager, | ||
| ) : | ||
| BaseViewModel<LoginContract.State, LoginContract.Effect>( |
There was a problem hiding this comment.
p1: BaseViewModel 내부에 sendEffect가 이미 viewModelScope를 내장하고 있습니다.
LoginViewModel에서 중복로 viewModelScope.launch를 감싸지 않도록 수정해보자요 ㅎㅎ
4 participants
Related issue 🛠
Work Description ✏️
임시 로그인 버튼누르면 로그인 가능!Screenshot 📸
KakaoTalk_20260702_184831313.mp4
Uncompleted Tasks 😅
To Reviewers 📢
로그인 기능을 구현하긴 했는데... 지금 단계에서 더 구현해야되는데 안 된 부분 있으면 말해주세요 🥹
그리고 로컬 프로퍼티는 노션에 올려두겠습니다!!
LOCAL_PROPERTIES vs. local.properties
로컬 프로퍼티 작성할 때 어떤 표기 선호하시나용...?! (지금은 어퍼케이스임)
Summary by CodeRabbit
요약