-
Notifications
You must be signed in to change notification settings - Fork 0
[feat/#20] kakao api #25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds Kakao social login: dependency and repo config, OAuth intent handler in AndroidManifest, Retrofit service + DTOs, AuthRepository and DI bindings, Compose UI and navigation for login, Kakao login manager, LoginViewModel and LoginState, and Kakao SDK initialization in MainActivity. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as KaKaoLoginScreen
participant KLM as KakaoLoginManager
participant SDK as Kakao SDK
participant VM as LoginViewModel
participant Repo as AuthRepositoryImpl
participant Service as AuthService
participant Backend
User->>UI: Tap "카카오 로그인"
UI->>KLM: startKakaoLogin(context, callback)
KLM->>SDK: check KakaoTalk availability
alt KakaoTalk available
KLM->>SDK: login via KakaoTalk
else
KLM->>SDK: login via KakaoAccount
end
SDK-->>KLM: accessToken / error
KLM-->>UI: onResult(accessToken, error)
UI->>VM: loginWithKakaoToken(accessToken)
VM->>Repo: loginWithSocialToken("kakao", accessToken)
Repo->>Service: POST /auth/login (SocialLoginRequest)
Service->>Backend: request
Backend-->>Service: Response<SocialLoginResponse>
Service-->>Repo: response
Repo-->>VM: Result(success|failure)
VM-->>UI: update loginState (Success / Error)
UI->>User: navigate or show error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
♻️ Duplicate comments (1)
app/src/main/java/com/hsLink/hslink/presentation/main/MainActivity.kt (1)
38-39: Move hardcoded API key to BuildConfig.This hardcoded key should be moved to BuildConfig as mentioned in the
AndroidManifest.xmlreview.
🧹 Nitpick comments (5)
app/src/main/java/com/hsLink/hslink/presentation/login/state/LoginState.kt (1)
5-6: Consider usingdata objectfor better debugging.Modern Kotlin (1.9+) supports
data object, which provides bettertoString()output and is now preferred over plainobjectfor sealed class members.Apply this diff:
- object Idle : LoginState() // 대기중 (아무것도 안 한 상태) - object Loading : LoginState() // 로딩중 (로그인 진행중) + data object Idle : LoginState() // 대기중 (아무것도 안 한 상태) + data object Loading : LoginState() // 로딩중 (로그인 진행중)app/build.gradle.kts (1)
91-92: ---Use version catalog for Kakao SDK dependency and update to the latest version.
The Kakao SDK version is hardcoded while other dependencies use the version catalog (
libs.*). Additionally, version 2.20.1 is outdated; the latest version is 2.22.0. Update to the latest version and add the dependency to your version catalog for consistency and easier maintenance.Add to your
libs.versions.toml:[versions] kakao-sdk = "2.22.0" [libraries] kakao-sdk-user = { module = "com.kakao.sdk:v2-user", version.ref = "kakao-sdk" }Then update
app/build.gradle.kts:- // KaKao - implementation("com.kakao.sdk:v2-user:2.20.1") + // Kakao + implementation(libs.kakao.sdk.user)app/src/main/java/com/hsLink/hslink/presentation/main/MainActivity.kt (2)
41-68: Guard debug logging with BuildConfig.DEBUG.The key hash logging is useful for development but should not run in production builds. Wrap this code in a debug check.
Apply this diff:
+ if (BuildConfig.DEBUG) { // 키 해시 확인 (API 레벨에 따라 다른 방법 사용) try { val packageInfo = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) { packageManager.getPackageInfo(packageName, PackageManager.GET_SIGNING_CERTIFICATES) } else { @Suppress("DEPRECATION") packageManager.getPackageInfo(packageName, PackageManager.GET_SIGNATURES) } val signatures = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) { packageInfo.signingInfo?.apkContentsSigners } else { @Suppress("DEPRECATION") packageInfo.signatures } signatures?.let { sigs -> for (signature in sigs) { val md = MessageDigest.getInstance("SHA") md.update(signature.toByteArray()) val keyHash = Base64.encodeToString(md.digest(), Base64.NO_WRAP) Log.d("KeyHash", "KeyHash: $keyHash") println("KeyHash: $keyHash") } } } catch (e: Exception) { Log.e("KeyHash", "Error getting key hash", e) } + }
132-134: Add TODO comment for placeholder implementation.The
checkLoginStatus()function currently always returnsfalse, which means users will always see the login screen. While this aligns with the current PR scope, adding a TODO comment would help track this incomplete feature.Apply this diff:
private suspend fun checkLoginStatus(): Boolean { + // TODO: Implement actual login status check (e.g., check for stored auth token) return false }app/src/main/java/com/hsLink/hslink/data/dto/request/SocialLoginRequest.kt (1)
5-9: Add a custom toString() to mask the sensitive accessToken field.The current code doesn't directly log the request object, but since
SocialLoginRequestis a Kotlin data class, its auto-generatedtoString()method exposes all fields including the sensitiveaccessToken. If this object is ever logged (via Retrofit logging interceptors, debugging tools, or future code changes), the token would be exposed. Add a customtoString()implementation that masks the token value:@Serializable data class SocialLoginRequest( val provider: String, val accessToken: String ) { override fun toString(): String = "SocialLoginRequest(provider=$provider, accessToken=***)" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
app/src/main/res/drawable/img_login_logo.pngis excluded by!**/*.png
📒 Files selected for processing (20)
app/build.gradle.kts(1 hunks)app/src/main/AndroidManifest.xml(1 hunks)app/src/main/java/com/hsLink/hslink/core/navigation/AppRoute.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/di/NetworkModule.kt(2 hunks)app/src/main/java/com/hsLink/hslink/data/di/RepositoryModule.kt(2 hunks)app/src/main/java/com/hsLink/hslink/data/dto/request/SocialLoginRequest.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/dto/response/SocialLoginResponse.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/repositoryimpl/AuthRepositoryImpl.kt(1 hunks)app/src/main/java/com/hsLink/hslink/data/service/login/AuthService.kt(1 hunks)app/src/main/java/com/hsLink/hslink/domain/repository/AuthRepository.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/login/component/KakaoLoginButton.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/login/navigation/LoginNavigation.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/login/screen/KaKaoLoginScreen.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/login/state/KakaoLoginManager.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/login/state/LoginState.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/login/viewmodel/LoginViewModel.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/main/MainActivity.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/main/MainNavHost.kt(2 hunks)app/src/main/res/drawable/ic_kakao_icon.xml(1 hunks)settings.gradle.kts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
app/src/main/java/com/hsLink/hslink/presentation/login/navigation/LoginNavigation.kt (2)
app/src/main/java/com/hsLink/hslink/presentation/main/MainNavigator.kt (1)
navigate(36-54)app/src/main/java/com/hsLink/hslink/presentation/login/screen/KaKaoLoginScreen.kt (1)
KaKaoLoginScreen(41-161)
app/src/main/java/com/hsLink/hslink/presentation/login/screen/KaKaoLoginScreen.kt (1)
app/src/main/java/com/hsLink/hslink/presentation/login/component/KakaoLoginButton.kt (1)
KakaoLoginButton(23-59)
app/src/main/java/com/hsLink/hslink/presentation/main/MainActivity.kt (3)
app/src/main/java/com/hsLink/hslink/core/designsystem/theme/Theme.kt (1)
HsLinkTheme(37-57)app/src/main/java/com/hsLink/hslink/presentation/login/screen/KaKaoLoginScreen.kt (1)
KaKaoLoginScreen(41-161)app/src/main/java/com/hsLink/hslink/presentation/main/MainScreen.kt (1)
MainScreen(12-35)
app/src/main/java/com/hsLink/hslink/presentation/main/MainNavHost.kt (1)
app/src/main/java/com/hsLink/hslink/presentation/login/navigation/LoginNavigation.kt (1)
loginNavGraph(16-28)
🔇 Additional comments (11)
settings.gradle.kts (1)
19-19: LGTM!The Kakao Maven repository is correctly configured to resolve Kakao SDK dependencies.
app/src/main/res/drawable/ic_kakao_icon.xml (1)
1-10: LGTM!The Kakao icon vector drawable is properly defined and ready for use in the login UI.
app/src/main/java/com/hsLink/hslink/core/navigation/AppRoute.kt (1)
1-9: LGTM!The serializable route definitions follow Compose Navigation best practices for type-safe navigation.
app/src/main/java/com/hsLink/hslink/data/dto/response/SocialLoginResponse.kt (1)
1-11: LGTM!The DTO is properly structured with immutable properties and correct serialization annotations.
app/src/main/java/com/hsLink/hslink/data/di/NetworkModule.kt (1)
68-72: LGTM!The AuthService provider follows the established pattern and is properly configured with Dagger/Hilt annotations.
app/src/main/java/com/hsLink/hslink/presentation/main/MainNavHost.kt (1)
30-34: Temporary routing aligns with PR scope.Both
onNavigateToMainandonNavigateToOnboardingcurrently route tonavigateToHome(), which aligns with the PR description noting that onboarding navigation is not yet implemented.app/src/main/java/com/hsLink/hslink/data/service/login/AuthService.kt (1)
9-12: LGTM!The Retrofit service interface follows best practices with proper suspend function usage and Response wrapper for error handling.
app/src/main/java/com/hsLink/hslink/presentation/login/state/KakaoLoginManager.kt (2)
10-30: LGTM! Solid fallback strategy.The login flow correctly checks for KakaoTalk availability and falls back to KakaoAccount when needed, providing a robust user experience.
33-40: LGTM!The helper function cleanly wraps the KakaoAccount login flow.
app/src/main/java/com/hsLink/hslink/presentation/main/MainActivity.kt (1)
79-83: LGTM!The serializable route objects follow Jetpack Compose Navigation's type-safe approach correctly.
app/src/main/java/com/hsLink/hslink/data/di/RepositoryModule.kt (1)
29-33: LGTM!The repository binding follows Hilt best practices and is consistent with the existing module structure. The
@Singletonscope is appropriate for a repository.
| <activity | ||
| android:name="com.kakao.sdk.auth.AuthCodeHandlerActivity" | ||
| android:exported="true"> | ||
| <intent-filter> | ||
| <action android:name="android.intent.action.VIEW" /> | ||
| <category android:name="android.intent.category.DEFAULT" /> | ||
| <category android:name="android.intent.category.BROWSABLE" /> | ||
| <!-- 실제 네이티브 앱 키 사용 --> | ||
| <data android:host="oauth" android:scheme="kakaoa0bbd28c9384baa131a731d1b914307c" /> | ||
| </intent-filter> | ||
| </activity> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move hardcoded API key to BuildConfig.
The Kakao native app key a0bbd28c9384baa131a731d1b914307c is hardcoded in the manifest (line 39) and also in MainActivity.kt (line 39). Hardcoding API keys in source-controlled files exposes them in version control history and decompiled APKs.
Move the key to local.properties or use BuildConfig:
In build.gradle.kts:
android {
defaultConfig {
// Read from local.properties
val properties = Properties()
properties.load(project.rootProject.file("local.properties").inputStream())
val kakaoAppKey = properties.getProperty("kakao.app.key", "")
manifestPlaceholders["kakaoAppKey"] = kakaoAppKey
buildConfigField("String", "KAKAO_APP_KEY", "\"$kakaoAppKey\"")
}
}In AndroidManifest.xml:
- <data android:host="oauth" android:scheme="kakaoa0bbd28c9384baa131a731d1b914307c" />
+ <data android:host="oauth" android:scheme="kakao${kakaoAppKey}" />In MainActivity.kt:
- KakaoSdk.init(this, "a0bbd28c9384baa131a731d1b914307c")
+ KakaoSdk.init(this, BuildConfig.KAKAO_APP_KEY)In local.properties (not committed):
kakao.app.key=a0bbd28c9384baa131a731d1b914307c🤖 Prompt for AI Agents
In app/src/main/AndroidManifest.xml around lines 31 to 41 the Kakao native app
key is hardcoded in the activity data scheme; move this secret to a build-time
property and use BuildConfig/manifestPlaceholder instead. Add code in
build.gradle.kts to read kakao.app.key from local.properties, set
manifestPlaceholders["kakaoAppKey"] and a
buildConfigField("String","KAKAO_APP_KEY",...) so the key is injected at build
time; update AndroidManifest.xml to reference the placeholder for the scheme
(use the manifest placeholder variable instead of the literal key); update
MainActivity.kt to read the key from BuildConfig.KAKAO_APP_KEY (not a hardcoded
string); add the key to local.properties (do NOT commit local.properties) and
ensure any existing hardcoded occurrences are removed or replaced.
| @@ -0,0 +1,10 @@ | |||
| package com.hsLink.hslink.domain.repository | |||
|
|
|||
| import com.hsLink.hslink.data.dto.response.SocialLoginResponse | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Domain layer depends on data layer DTO.
The domain repository interface directly references SocialLoginResponse from the data layer, violating Clean Architecture principles. The domain layer should define its own models (e.g., a domain AuthResult or User entity), and the data layer implementation should map between DTOs and domain models.
While this approach works functionally and is common in smaller projects, it couples the domain to data layer implementation details and makes testing/mocking more difficult.
Consider introducing a domain model:
// In domain layer
data class AuthResult(
val accessToken: String,
val refreshToken: String,
val isNewUser: Boolean,
val needsOnboarding: Boolean
)Then update the repository interface:
-import com.hsLink.hslink.data.dto.response.SocialLoginResponse
+import com.hsLink.hslink.domain.model.AuthResult
interface AuthRepository {
suspend fun loginWithSocialToken(
provider: String,
accessToken: String
- ): Result<SocialLoginResponse>
+ ): Result<AuthResult>
}The implementation would map SocialLoginResponse to AuthResult.
🤖 Prompt for AI Agents
In app/src/main/java/com/hsLink/hslink/domain/repository/AuthRepository.kt
around line 3, the domain repository references the data-layer DTO
SocialLoginResponse; create a pure domain model (e.g., AuthResult) in the domain
layer with only domain fields (accessToken, refreshToken, isNewUser,
needsOnboarding), update the AuthRepository interface to return that domain
model instead of SocialLoginResponse, and update the data-layer implementation
to map from SocialLoginResponse to AuthResult before returning.
| import android.R.attr.enabled | ||
| import androidx.compose.foundation.Image | ||
| import androidx.compose.foundation.R |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused imports.
Lines 3 and 5 appear to import unused symbols. The enabled parameter is a Kotlin Boolean and doesn't require android.R.attr.enabled, and androidx.compose.foundation.R doesn't seem to be used in this file.
Apply this diff to remove the unused imports:
-import android.R.attr.enabled
import androidx.compose.foundation.Image
-import androidx.compose.foundation.R
import androidx.compose.foundation.layout.RowCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
app/src/main/java/com/hsLink/hslink/presentation/login/component/KakaoLoginButton.kt
around lines 3 to 5, remove the unused imports android.R.attr.enabled and
androidx.compose.foundation.R; the enabled parameter is a Kotlin Boolean and
doesn't require android.R.attr.enabled, and androidx.compose.foundation.R is not
referenced in this file — simply delete those import lines so only required
imports remain.
| fun KakaoLoginButton( | ||
| onClick: () -> Unit, | ||
| modifier: Modifier = Modifier, | ||
| enabled: Boolean = true | ||
| ) { | ||
| Button( | ||
| onClick = onClick, | ||
| enabled = enabled, | ||
| colors = ButtonDefaults.buttonColors( | ||
| containerColor = Color(0xFFFEE500), // 카카오 노란색 | ||
| contentColor = Color.Black // 텍스트 및 아이콘 색상 | ||
| ), | ||
| shape = RoundedCornerShape(6.dp), // 모서리 둥글기 | ||
| modifier = Modifier | ||
| .fillMaxWidth(0.85f) // 가로 폭 (조정 가능) | ||
| .height(54.dp) // 높이 (Figma 기준) | ||
| ) { | ||
| Row( | ||
| verticalAlignment = Alignment.CenterVertically | ||
| ) { | ||
| // 말풍선 아이콘 (SVG 또는 PNG 리소스) | ||
| Image( | ||
| painter = painterResource(id = com.hsLink.hslink.R.drawable.ic_kakao_icon), | ||
| contentDescription = "카카오 로고", | ||
| modifier = Modifier | ||
| .size(20.dp) | ||
| .padding(end = 8.dp) | ||
| ) | ||
| Text( | ||
| text = "카카오 로그인", | ||
| color = Color(0xFF191919), // 카카오 가이드 기준 검정 85% | ||
| fontSize = 16.sp | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apply the modifier parameter.
The modifier parameter passed to the function is not used. Line 37 creates a new Modifier instead of chaining the provided parameter, preventing external customization of the button's layout.
Apply this diff to use the modifier parameter:
shape = RoundedCornerShape(6.dp),
- modifier = Modifier
+ modifier = modifier
.fillMaxWidth(0.85f)
.height(54.dp)🤖 Prompt for AI Agents
In
app/src/main/java/com/hsLink/hslink/presentation/login/component/KakaoLoginButton.kt
around lines 24 to 58, the passed-in modifier parameter is ignored because the
Button uses a fresh Modifier chain (Modifier.fillMaxWidth(...).height(...))
instead of applying the provided modifier; update the Button's modifier to start
from the function parameter (modifier) and then chain the layout modifiers
(e.g., modifier.fillMaxWidth(0.85f).height(54.dp)) so external callers can
customize the composable.
| LaunchedEffect(loginState) { | ||
| // 지역 변수로 만들어서 스마트 캐스팅 가능하게 만들기 | ||
| val currentState = loginState | ||
| when (currentState) { | ||
| is LoginState.Success -> { | ||
| if (currentState.needsOnboarding) { | ||
| onNavigateToOnboarding() | ||
| } else { | ||
| onNavigateToHome() | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reset success state after triggering navigation.
loginState stays Success after navigation, so if the user returns (back press, configuration change, etc.) the LaunchedEffect fires again and immediately re-navigates. Call viewModel.resetLoginState() once the navigation completes to prevent loops.
if (currentState.needsOnboarding) {
onNavigateToOnboarding()
} else {
onNavigateToHome()
}
+ viewModel.resetLoginState()🤖 Prompt for AI Agents
In
app/src/main/java/com/hsLink/hslink/presentation/login/screen/KaKaoLoginScreen.kt
around lines 53 to 63, the LaunchedEffect handling LoginState.Success navigates
but leaves loginState as Success, causing immediate re-navigation when the
screen is revisited; after calling onNavigateToOnboarding() or
onNavigateToHome(), invoke viewModel.resetLoginState() to clear the success
state so the effect won't fire again — place the resetLoginState() call
immediately after each navigation branch inside the LaunchedEffect (or after the
when block) so it runs once the navigation completes.
| Box( | ||
| modifier = modifier | ||
| .fillMaxSize() | ||
| .background( | ||
| brush = Brush.verticalGradient( | ||
| colors = listOf( | ||
| HsLinkTheme.colors.SkyBlue400, | ||
| Color.White | ||
| ), | ||
| startY = 0f, | ||
| endY = Float.POSITIVE_INFINITY | ||
| ) | ||
| ) | ||
| ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apply scaffold padding to avoid overlapping system bars.
The caller supplies paddingValues, but the screen ignores them, so the content can render under the status/navigation bars. Apply the provided padding before fillMaxSize().
- Box(
- modifier = modifier
- .fillMaxSize()
+ Box(
+ modifier = modifier
+ .padding(paddingValues)
+ .fillMaxSize()📝 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.
| Box( | |
| modifier = modifier | |
| .fillMaxSize() | |
| .background( | |
| brush = Brush.verticalGradient( | |
| colors = listOf( | |
| HsLinkTheme.colors.SkyBlue400, | |
| Color.White | |
| ), | |
| startY = 0f, | |
| endY = Float.POSITIVE_INFINITY | |
| ) | |
| ) | |
| ) { | |
| Box( | |
| modifier = modifier | |
| .padding(paddingValues) | |
| .fillMaxSize() | |
| .background( | |
| brush = Brush.verticalGradient( | |
| colors = listOf( | |
| HsLinkTheme.colors.SkyBlue400, | |
| Color.White | |
| ), | |
| startY = 0f, | |
| endY = Float.POSITIVE_INFINITY | |
| ) | |
| ) | |
| ) { |
🤖 Prompt for AI Agents
In
app/src/main/java/com/hsLink/hslink/presentation/login/screen/KaKaoLoginScreen.kt
around lines 69 to 82, the Box ignores the incoming paddingValues and can render
under system bars; apply the provided paddingValues to the modifier before
calling fillMaxSize() so the scaffold insets are respected — update the modifier
chain to include .padding(paddingValues) (or
.padding(paddingValues.calculateTopPadding(), ...) if needed) prior to
.fillMaxSize() so the content is shifted inside system bars.
| Spacer(modifier = Modifier.padding(115.dp)) | ||
|
|
||
| KakaoLoginButton( | ||
| onClick = { | ||
| KakaoLoginManager.startKakaoLogin(context) { accessToken, error -> | ||
| if (error != null) { | ||
| // TODO: 에러 처리 개선 | ||
| println("카카오 로그인 실패: ${error.message}") | ||
| } else if (accessToken != null) { | ||
| // 서버에 토큰 전송 | ||
| viewModel.loginWithKakaoToken(accessToken) | ||
| } | ||
| } | ||
| }, | ||
| enabled = loginState !is LoginState.Loading | ||
| ) | ||
|
|
||
| // 로그인 상태에 따른 UI - 여기도 지역 변수 사용 | ||
| val currentLoginState = loginState | ||
| when (currentLoginState) { | ||
| is LoginState.Loading -> { | ||
| Spacer(modifier = Modifier.padding(16.dp)) | ||
| CircularProgressIndicator( | ||
| color = HsLinkTheme.colors.DeepBlue600 | ||
| ) | ||
| Text( | ||
| text = "로그인 중...", | ||
| style = HsLinkTheme.typography.body_14Normal, | ||
| color = HsLinkTheme.colors.DeepBlue600, | ||
| modifier = Modifier.padding(top = 8.dp) | ||
| ) | ||
| } | ||
|
|
||
| is LoginState.Error -> { | ||
| Spacer(modifier = Modifier.padding(16.dp)) | ||
| Text( | ||
| text = currentLoginState.message, | ||
| color = Color.Red, | ||
| style = HsLinkTheme.typography.body_14Normal | ||
| ) | ||
| } | ||
|
|
||
| is LoginState.Success -> { | ||
| Spacer(modifier = Modifier.padding(16.dp)) | ||
| Text( | ||
| text = "로그인 성공!", | ||
| color = Color.Green, | ||
| style = HsLinkTheme.typography.body_14Normal | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spacer with only padding renders no gap.
Spacer needs an explicit size; Modifier.padding(...) alone keeps it at 0×0, so the intended vertical spacing never appears. Replace the padding-only spacers with Modifier.height(...) (add import androidx.compose.foundation.layout.height).
- Spacer(modifier = Modifier.padding(115.dp))
+ Spacer(modifier = Modifier.height(115.dp))
@@
- Spacer(modifier = Modifier.padding(16.dp))
+ Spacer(modifier = Modifier.height(16.dp))
@@
- Spacer(modifier = Modifier.padding(16.dp))
+ Spacer(modifier = Modifier.height(16.dp))
@@
- Spacer(modifier = Modifier.padding(16.dp))
+ Spacer(modifier = Modifier.height(16.dp))🤖 Prompt for AI Agents
In
app/src/main/java/com/hsLink/hslink/presentation/login/screen/KaKaoLoginScreen.kt
around lines 104 to 152, the Spacer calls use Modifier.padding(...) which yields
a 0×0 spacer so no vertical gap is produced; replace those Spacer(modifier =
Modifier.padding(X.dp)) usages with Spacer(modifier = Modifier.height(X.dp)) and
add the import androidx.compose.foundation.layout.height at the top of the file
so the intended vertical spacing appears.
| KakaoLoginManager.startKakaoLogin(context) { accessToken, error -> | ||
| if (error != null) { | ||
| // TODO: 에러 처리 개선 | ||
| println("카카오 로그인 실패: ${error.message}") | ||
| } else if (accessToken != null) { | ||
| // 서버에 토큰 전송 | ||
| viewModel.loginWithKakaoToken(accessToken) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surface Kakao client errors to the state.
When the Kakao SDK returns an error, the UI only logs it, leaving the user without feedback. Publish a LoginState.Error through the view model so the existing error UI can react.
if (error != null) {
- // TODO: 에러 처리 개선
- println("카카오 로그인 실패: ${error.message}")
+ viewModel.handleKakaoLoginError(
+ error.message ?: "카카오 로그인에 실패했습니다."
+ )
} else if (accessToken != null) {Kotlin (add to LoginViewModel as shown in its review comment)
fun handleKakaoLoginError(message: String) {
_loginState.value = LoginState.Error(message)
}🤖 Prompt for AI Agents
In
app/src/main/java/com/hsLink/hslink/presentation/login/screen/KaKaoLoginScreen.kt
around lines 108 to 116, the Kakao SDK error is only printed to the console and
not surfaced to the UI; replace the println with a call to the view model error
handler so the UI can react: when error != null call
viewModel.handleKakaoLoginError(error.message ?: "카카오 로그인 실패") (ensure
LoginViewModel has the handleKakaoLoginError function that sets _loginState to
LoginState.Error), and keep the else branch unchanged for successful accessToken
handling.
| fun resetLoginState() { | ||
| _loginState.value = LoginState.Idle | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expose a helper to publish client-side login errors.
To support surfacing Kakao SDK failures, add a small helper that emits LoginState.Error with the received message.
fun resetLoginState() {
_loginState.value = LoginState.Idle
}
+
+ fun handleKakaoLoginError(message: String) {
+ _loginState.value = LoginState.Error(message)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun resetLoginState() { | |
| _loginState.value = LoginState.Idle | |
| } | |
| fun resetLoginState() { | |
| _loginState.value = LoginState.Idle | |
| } | |
| fun handleKakaoLoginError(message: String) { | |
| _loginState.value = LoginState.Error(message) | |
| } |
🤖 Prompt for AI Agents
In
app/src/main/java/com/hsLink/hslink/presentation/login/viewmodel/LoginViewModel.kt
around lines 44 to 46, add a public helper function (e.g.,
publishClientError(message: String)) that sets the internal _loginState to
LoginState.Error with the provided message so callers (like Kakao SDK handlers)
can surface client-side login errors; implement it to post/assign the value on
the appropriate thread/dispatcher (use _loginState.value =
LoginState.Error(message) for main-thread or _loginState.postValue(...) if using
LiveData off-main), keep it simple and consistent with existing state updates,
and ensure the function is exposed from the ViewModel API for external callers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/java/com/hsLink/hslink/data/di/NetworkModule.kt (1)
68-72: Consider using expression body syntax for consistency.The implementation is correct, but for consistency with other service providers in this file (
providePostServiceandprovideCommunityPostService), consider using expression body syntax.Apply this diff for consistency:
- @Provides - @Singleton - fun provideAuthService(retrofit: Retrofit): AuthService { - return retrofit.create(AuthService::class.java) - } + @Provides + @Singleton + fun provideAuthService(retrofit: Retrofit): AuthService = + retrofit.create(AuthService::class.java)Operational note: If you later need auth-specific behavior (e.g., token interceptors, custom timeout for auth endpoints), consider creating a dedicated
OkHttpClientforAuthServiceinstead of reusing the base client.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/java/com/hsLink/hslink/data/di/NetworkModule.kt(2 hunks)app/src/main/java/com/hsLink/hslink/data/di/RepositoryModule.kt(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-test
🔇 Additional comments (2)
app/src/main/java/com/hsLink/hslink/data/di/NetworkModule.kt (1)
6-6: LGTM!The import is correctly added to support the new
AuthServiceprovider method.app/src/main/java/com/hsLink/hslink/data/di/RepositoryModule.kt (1)
32-32: Verify the singleton scope decision.The
@Singletonannotation is applied only toAuthRepository, while other repository bindings (DummyRepository,PostRepository,CommunityRepository) don't have explicit scope annotations.For an authentication repository that maintains login state, singleton scope makes sense. However, please verify whether:
- This is intentional (auth state management requires a single instance), or
- Other repositories should also be singletons for consistency
| fun bindAuthRepository( | ||
| authRepositoryImpl: AuthRepositoryImpl | ||
| ): AuthRepository |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix method naming to match existing pattern.
The method name should be bindsAuthRepository (with 's') to maintain consistency with the other binding methods in this module: bindsDummyRepository, bindsPostRepository, and bindsCommunityPostRepository.
Apply this diff to fix the naming:
- fun bindAuthRepository(
+ fun bindsAuthRepository(
authRepositoryImpl: AuthRepositoryImpl
): AuthRepository📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun bindAuthRepository( | |
| authRepositoryImpl: AuthRepositoryImpl | |
| ): AuthRepository | |
| fun bindsAuthRepository( | |
| authRepositoryImpl: AuthRepositoryImpl | |
| ): AuthRepository |
🤖 Prompt for AI Agents
In app/src/main/java/com/hsLink/hslink/data/di/RepositoryModule.kt around lines
33 to 35, the binder method is named bindAuthRepository but should follow the
module's naming convention and be renamed to bindsAuthRepository; update the
function name from bindAuthRepository to bindsAuthRepository (including any
references/usages if present) so it matches bindsDummyRepository,
bindsPostRepository, and bindsCommunityPostRepository and keeps consistency
across the DI module.
| <category android:name="android.intent.category.DEFAULT" /> | ||
| <category android:name="android.intent.category.BROWSABLE" /> | ||
| <!-- 실제 네이티브 앱 키 사용 --> | ||
| <data android:host="oauth" android:scheme="kakaoa0bbd28c9384baa131a731d1b914307c" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
님아.
ISSUE
❗ WORK DESCRIPTION
📸 SCREENSHOT
2025-11-11.11.40.00.mov
📢 TO REVIEWERS
Summary by CodeRabbit
New Features
Chores