SCRUM-243 SCRUM-244 SCRUM-245 feature: kakao login and server login integration#14
Merged
gdaegeun539 merged 59 commits intoproject-lyrics:developfrom Apr 18, 2026
Conversation
- add kakao sdk repository, add login sdk - add kakao sdk r8 rules - add application class and sdk init code - set kakao api key loader - add login sdk redirection activity setting
- add login, logout, unlink
- own feelin server jwt token model - common oauth token model - to unify support kakao, google oauth token
Add androidx.datastore.preferences library to enable secure token storage with Flow-based reactive data access. - Add datastore version 1.1.1 to libs.versions.toml - Add datastore-preferences dependency to app module
Add fromString() companion method to convert stored string values back to OAuthProvider enum for DataStore retrieval. - Support case-insensitive string matching - Return null for invalid provider names
Implement DataStore-based local data source to persist authentication tokens and OAuth information. Storage strategy: - Backend JWT tokens (access, refresh, userId) - OAuth provider and tokens - Flow-based reactive data access - Singleton pattern for app-wide consistency Multi-module ready: Can be moved to :core:data module
Implement singleton manager to handle authentication state and token management across the entire app lifecycle. Responsibilities: - Memory caching via StateFlow for reactive UI - Delegate persistence to AuthLocalDataSource - Synchronize in-memory state with storage - Manage both Backend JWT and OAuth tokens Features: - Auto-load tokens on app start - Reactive login state (isLoggedIn) - OAuth provider tracking - Token update and clear operations Multi-module ready: Can be moved to :core:data module
Implement OkHttp interceptor to automatically attach JWT access token to all API requests. Features: - Read token from AuthManager StateFlow - Add Authorization: Bearer header - Skip if no token available - Singleton pattern for OkHttp client Includes TokenAuthenticator stub for future 401 handling and automatic token refresh implementation. Multi-module ready: Can be moved to :core:network module
Update AuthRepository to use AuthManager for centralized token management and reactive login state. Changes: - Remove EncryptedSharedPreferences dependency - Integrate AuthManager for token persistence - Expose isLoggedIn and userId as StateFlow - Store OAuth provider and tokens on login - Clear all tokens on logout via AuthManager - Update AuthToken model to use Long userId Backend API compatibility: - Match response structure with userId as Long - Temporary dummy token until backend integration TODO: Implement actual backend API calls for login/logout
Configure Hilt DI module for authentication components to ensure singleton instances across the app. Provides: - AuthLocalDataSource @singleton - AuthManager @singleton Dependencies are automatically injected via constructor injection pattern following Hilt best practices. Multi-module ready: Can be moved to :core:data module
- serializer to kotlinx.serialization
- include DeviceId Header
Member
Author
Member
Author
|
서버 주소를 Dev가 아닌 Prod로 설정해 테스트하면 가입되지 않은 계정에 대해 회원가입 화면으로 이동합니다. |
- change url to prod server because of dev server's error
Extract the HttpException error mapping logic into a specialized private method to improve readability of the login process. - Replace onFailure block with mapAuthenticationFailure method - Add @Suppress("ReturnCount") to deleteAccount with a TODO comment
gdaegeun539
commented
Mar 23, 2026
| ReturnCount: | ||
| active: true | ||
| max: 2 | ||
| max: 3 |
Member
Author
There was a problem hiding this comment.
이번 PR을 장기 유기하게 했던 부분이라 고심끝에 반환 최대 카운트를 3으로 올렸습니다.
이 부분은 현정님의 의견도 듣고 싶습니다.
gdaegeun539
commented
Mar 24, 2026
| } | ||
|
|
||
| private fun loginWithKakaoTalk(context: Context, viewModel: LoginViewModel) { | ||
| UserApiClient.instance.loginWithKakaoTalk(context) { token, error -> |
Member
Author
There was a problem hiding this comment.
카카오 sdk에서 로그인 화면을 띄우는 부분이 액티비티 컨텍스트를 요구해서 loginWithXXX만 로그인 컴포저블에 위치했어요.
이 부분을 예전에 말씀드렸는지 기억이 안 나서 코멘트로 다시 적어봐요.
1 task
hyunjung-choi
approved these changes
Apr 14, 2026
Member
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactors social login to remove Activity from data layer: initializes Kakao SDK in Application, moves UI interactions to Compose, introduces token-based backend auth flow, centralized AuthManager with DataStore persistence, network wiring (OkHttp/Retrofit, interceptors, token refresh), new DTOs/domain models, and ViewModel/Compose login wiring. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant LoginScreen
participant KakaoSDK as "Kakao SDK"
participant LoginViewModel
participant AuthRepository
participant AuthRemoteDataSource
participant Backend
participant AuthManager
participant AuthLocalDataSource
User->>LoginScreen: Tap Kakao Login
LoginScreen->>KakaoSDK: Start login (uses Activity/Context)
KakaoSDK-->>LoginScreen: Callback with OAuthToken
LoginScreen->>LoginViewModel: login(provider, OAuthToken)
LoginViewModel->>AuthRepository: authenticateWithBackend(provider, token)
AuthRepository->>AuthRemoteDataSource: signIn(provider, token, deviceId)
AuthRemoteDataSource->>Backend: POST /auth/sign-in
Backend-->>AuthRemoteDataSource: AuthToken(response)
AuthRemoteDataSource-->>AuthRepository: Result<AuthToken>
AuthRepository->>AuthManager: saveAllToken(access, refresh, userId, oauth...)
AuthManager->>AuthLocalDataSource: persist tokens via DataStore
AuthLocalDataSource->>AuthLocalDataSource: write preferences
AuthManager-->>LoginViewModel: state flows updated (isLoggedIn)
LoginViewModel-->>LoginScreen: loginUiState = Success
LoginScreen->>User: Navigate to Main
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 19
🧹 Nitpick comments (13)
agents.md (1)
36-36: Add language identifiers to fenced code blocks.The markdown linter is flagging missing language specifiers on fenced code blocks. Adding language identifiers improves documentation rendering and suppresses linter warnings.
📝 Suggested fix - add language identifiers
For commit message examples, consider adding
textorgitcommitas the language identifier:Lines 36-40:
-``` +```text feat: Add Kakao login featureApply similar changes to the other fenced code blocks at lines 46, 59, 69, 76, 101, 118, 132, 142, and 156.
Also applies to: 46-46, 59-59, 69-69, 76-76, 101-101, 118-118, 132-132, 142-142, 156-156
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@agents.md` at line 36, Add missing language identifiers to the fenced code blocks in agents.md so the markdown linter stops flagging them; specifically update each triple-backtick block (``` ) at the mentioned locations to include an appropriate language token (e.g., use "text" or "gitcommit" for commit message examples) for the blocks at lines 36, 46, 59, 69, 76, 101, 118, 132, 142, and 156.app/src/main/java/com/lyrics/feelin/core/data/util/ResponseExtension.kt (1)
25-33: Consider catching serialization exceptions.
safeApiCallonly catchesIOException. If Retrofit's converter (e.g., kotlinx.serialization, Gson) throws during deserialization, the exception will propagate uncaught. Depending on your error handling strategy, you may want to catch these as well.Option to catch broader exceptions
suspend fun <T> safeApiCall( apiCall: suspend () -> Response<T> ): Result<T> { return try { apiCall().toResult() } catch (e: IOException) { Result.failure(e) + } catch (e: Exception) { + // Catch serialization errors and other unexpected exceptions + Result.failure(e) } }Alternatively, if you prefer explicit handling, catch
kotlinx.serialization.SerializationExceptionspecifically.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/lyrics/feelin/core/data/util/ResponseExtension.kt` around lines 25 - 33, The safeApiCall function currently only catches IOException so serialization errors from Retrofit converters can escape; update safeApiCall (and/or the toResult conversion path) to also catch deserialization failures by adding a catch block for kotlinx.serialization.SerializationException (or Gson's JsonSyntaxException/JsonParseException if using Gson), and optionally include a broader catch(Exception) fallback to convert those exceptions into Result.failure with useful context; reference the suspend fun <T> safeApiCall(...) and the toResult() call to locate where to add these additional catches.app/src/main/java/com/lyrics/feelin/presentation/view/component/profile/ProfileComponent.kt (1)
15-16: Consider usingremember(type)to react to parameter changes.If
typechanges during recomposition,profileDatawon't update becauserememberwithout keys only computes once. Also,mutableStateOfis unnecessary here since the value isn't mutated.Suggested simplification
`@Composable` fun ProfileComponent(type: ProfileType, size: Int, modifier: Modifier = Modifier) { - val profileData = remember { mutableStateOf(ProfileComponentData.fromProfileType(type)) } + val profileData = remember(type) { ProfileComponentData.fromProfileType(type) } Image( - painter = painterResource(profileData.value.enableAsset), + painter = painterResource(profileData.enableAsset), contentDescription = "User Profile Image", modifier = modifier.size(size.dp), ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/lyrics/feelin/presentation/view/component/profile/ProfileComponent.kt` around lines 15 - 16, The ProfileComponent currently uses remember { mutableStateOf(ProfileComponentData.fromProfileType(type)) } which won’t update when the type parameter changes and unnecessarily wraps the value in mutableStateOf; change it to remember(type) { ProfileComponentData.fromProfileType(type) } (or compute directly without remember if not needed) so ProfileComponentData.fromProfileType(type) is recomputed when type changes and remove the mutableStateOf wrapper; update references to use the plain remembered value instead of a State object.app/src/main/java/com/lyrics/feelin/core/data/datasource/sdk/KakaoAuthDataSource.kt (1)
13-41: Consider addinginvokeOnCancellationfor cleanup.When using
suspendCancellableCoroutine, if the coroutine is cancelled mid-flight, the Kakao SDK callback will still execute and attempt to resume a cancelled continuation (which is safely ignored). However, it's good practice to addinvokeOnCancellationfor clarity and potential cleanup.Example pattern
suspend fun logout(): Result<Unit> = suspendCancellableCoroutine { continuation -> UserApiClient.instance.logout { error -> if (continuation.isActive) { // handle result... } } continuation.invokeOnCancellation { Log.d(TAG, "logout: Coroutine cancelled") } }Also applies to: 44-73
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/lyrics/feelin/core/data/datasource/sdk/KakaoAuthDataSource.kt` around lines 13 - 41, The logout suspend function using suspendCancellableCoroutine should register continuation.invokeOnCancellation to perform cleanup and avoid handling results after cancellation; update the UserApiClient.instance.logout callback (in the suspendCancellableCoroutine block inside logout and similarly in the other coroutine blocks at lines 44-73) to check continuation.isActive before calling continuation.resume/Result and add continuation.invokeOnCancellation { /* log and/or cancel SDK request if possible */ } so cancelled coroutines don't attempt to process SDK callbacks or leave requests dangling.app/src/main/java/com/lyrics/feelin/core/data/di/DataModule.kt (1)
9-18: KDoc is misleading and doesn't match the actual provided dependencies.The documentation states it provides
AuthLocalDataSourceandAuthManager, but the module only providesKakaoAuthDataSource. Update the KDoc to accurately reflect the module's contents.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/lyrics/feelin/core/data/di/DataModule.kt` around lines 9 - 18, The KDoc for DataModule is inaccurate: it mentions providing AuthLocalDataSource and AuthManager but the module actually provides KakaoAuthDataSource; update the KDoc above the DataModule declaration to list the real provided singleton(s) (e.g., KakaoAuthDataSource) and remove or correct references to AuthLocalDataSource and AuthManager so the comments accurately reflect the module's contents and intent.app/src/main/java/com/lyrics/feelin/core/data/datasource/sdk/util/KakakoAuthSdkExtension.kt (1)
1-1: Typo in filename: "Kakako" should be "Kakao".Rename the file from
KakakoAuthSdkExtension.kttoKakaoAuthSdkExtension.ktfor consistency with other Kakao-related naming in the codebase.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/lyrics/feelin/core/data/datasource/sdk/util/KakakoAuthSdkExtension.kt` at line 1, Rename the source file named "KakakoAuthSdkExtension.kt" to "KakaoAuthSdkExtension.kt" and update any internal references to the old filename; ensure the Kotlin file still declares package com.lyrics.feelin.core.data.datasource.sdk.util and that any imports, tests, or usages referencing KakakoAuthSdkExtension are updated to the corrected class/file name "KakaoAuthSdkExtension" to maintain consistency.app/src/main/java/com/lyrics/feelin/core/domain/model/OAuthProvider.kt (1)
11-13: HardenfromStringwith input normalization.
fromStringis case-insensitive, but" kakao "still fails. Trimming first makes parsing more resilient.Proposed tweak
fun fromString(value: String): OAuthProvider? { - return entries.find { it.name.equals(value, ignoreCase = true) } + val normalized = value.trim() + return entries.find { it.name.equals(normalized, ignoreCase = true) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/lyrics/feelin/core/domain/model/OAuthProvider.kt` around lines 11 - 13, The fromString function in OAuthProvider should normalize input (trim whitespace and handle null/empty) before matching; update OAuthProvider.fromString to first guard against null/blank inputs, call value.trim() (and optionally toLowerCase()) and then use entries.find { it.name.equals(normalizedValue, ignoreCase = true) } so inputs like " kakao " resolve correctly.app/src/main/java/com/lyrics/feelin/core/data/datasource/remote/dto/SignInRequestDto.kt (1)
7-9: StabilizeauthProviderwire values explicitly.Line 9 currently serializes a domain enum directly. That couples API payload values to enum constant names and makes future renames risky. Prefer a DTO-specific wire type (or string) with explicit serialized values.
Proposed direction
`@Serializable` data class SignInRequestDto( val socialAccessToken: String, - val authProvider: OAuthProvider, + val authProvider: String, )Then map in datasource/repository with an explicit contract-safe value (for example, fixed literals agreed with backend).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/lyrics/feelin/core/data/datasource/remote/dto/SignInRequestDto.kt` around lines 7 - 9, The DTO currently exposes authProvider: OAuthProvider which serializes enum names directly; change SignInRequestDto to use a stable wire type (e.g., String or a DTO-specific enum) and map domain OAuthProvider -> that wire value in the datasource/repository layer; update the code paths that construct SignInRequestDto to pass the agreed-upon literal/wire value for each OAuthProvider and avoid serializing the domain enum directly so renames won’t break the API contract.app/detekt.yml (1)
150-159: ScopeTooManyFunctionsexclusion more narrowly.Excluding
**/datasource/**disables function-count checks across the whole data-source layer. Prefer targetted exclusions (specific files/classes) to avoid hiding future complexity growth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/detekt.yml` around lines 150 - 159, The current detekt excludes entry removes TooManyFunctions checks for the entire "**/datasource/**" package; change it to narrowly exclude only the specific files or classes that legitimately exceed function limits (e.g., the concrete DataSource implementations) instead of the whole directory. Update the excludes array to list explicit filenames or class-specific patterns (replace "**/datasource/**" with targeted patterns like "**/datasource/MyConcreteDataSource.kt" or class-name regexes) or configure the TooManyFunctions rule to ignore only specified classes via its rule-specific excludes; keep the rest of the datasource package under the default TooManyFunctions checks.app/src/main/java/com/lyrics/feelin/core/data/interceptor/AppVersionInterceptor.kt (1)
19-22: Fix the misleading inline comment for header type.Line 19 says Authorization header, but this block adds
App-Version.🧹 Proposed fix
- // Authorization 헤더 추가 + // App-Version 헤더 추가🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/lyrics/feelin/core/data/interceptor/AppVersionInterceptor.kt` around lines 19 - 22, The inline comment above the request builder in AppVersionInterceptor.kt is misleading ("Authorization 헤더 추가") but the code adds an "App-Version" header; update that comment to accurately reflect the header being added (e.g., "App-Version 헤더 추가") or remove it, locating the change near the authenticatedRequest assignment that calls originalRequest.newBuilder().header("App-Version", appVersion).build().app/src/main/java/com/lyrics/feelin/core/data/datasource/remote/AuthApiService.kt (1)
19-29: Prefer DTOs at the Retrofit boundary instead of domain models.
AuthApiServiceis currently usingAuthTokenandSignUpDatafromcore.domainas HTTP payloads. That couples backend serialization directly to domain evolution and makes future API-only changes ripple through higher layers. Dedicated request/response DTOs here would keep the boundary cleaner and easier to change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/lyrics/feelin/core/data/datasource/remote/AuthApiService.kt` around lines 19 - 29, AuthApiService is using domain models (AuthToken, SignUpData) at the Retrofit boundary; create dedicated DTO classes and swap them into the API signatures to decouple serialization. Add request/response DTOs (e.g., AuthTokenDto, SignInRequestDto, SignUpRequestDto) and change signIn to accept SignInRequestDto and return Response<AuthTokenDto>, signUp to accept SignUpRequestDto, and reIssueToken to accept RefreshTokenRequestDto/return Response<AuthTokenDto>; update signOut to use ServerStatusResponseDto if already a DTO. After changing the method signatures in AuthApiService, add mapping functions between domain models and DTOs (e.g., toDomain()/toDto()) in the DTO classes and update callers to perform conversion at the data layer only.app/src/main/java/com/lyrics/feelin/core/data/repository/AuthRepository.kt (1)
186-189: PrefergetOrThrow()overgetOrNull()!!for clarity.While safe after
onFailurereturns,getOrNull()!!obscures the intent. UsinggetOrThrow()makes the "guaranteed success" assumption explicit.♻️ Suggested refactor
- val token = tokenResult.getOrNull()!! + val token = tokenResult.getOrThrow()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/lyrics/feelin/core/data/repository/AuthRepository.kt` around lines 186 - 189, Replace the unsafe getOrNull()!! usage on tokenResult with tokenResult.getOrThrow() to make the "guaranteed success" assumption explicit; locate the code in AuthRepository where tokenResult is read (the call that currently does tokenResult.getOrNull()!!) and pass the resulting token into authManager.saveServerToken as before, but obtained via getOrThrow() to avoid the forced non-null assertion.app/src/main/java/com/lyrics/feelin/presentation/view/login/LoginViewModel.kt (1)
66-70: Extract magic error code to a named constant.The hardcoded
"02000"error code should be a constant for maintainability and discoverability.♻️ Suggested refactor
+private const val ERROR_CODE_SIGNUP_REQUIRED = "02000" + `@HiltViewModel` class LoginViewModel `@Inject` constructor(- if (it.description.errorCode == "02000") { + if (it.description.errorCode == ERROR_CODE_SIGNUP_REQUIRED) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/lyrics/feelin/presentation/view/login/LoginViewModel.kt` around lines 66 - 70, Extract the hardcoded error code "02000" into a named constant and use that constant in the LoginViewModel check; specifically, add a descriptive constant (e.g., SIGN_UP_REQUIRED_ERROR_CODE) near the LoginViewModel class or companion object, replace the literal in the FeelinServerException branch (the condition it.description.errorCode == "02000") with the constant, and ensure any tests or other references use the same constant for consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@agents.md`:
- Line 1: The file is misnamed: rename the markdown file from agents.md to
AGENTS.md to match the documentation convention and the references used in other
docs; update any references in claude.md and GEMINI.md (or other files that
import/mention `@AGENTS.md`) to point to the new uppercase filename so all links
and imports consistently use AGENTS.md.
In `@app/build.gradle.kts`:
- Around line 35-39: Normalize the Kakao key retrieval so missing property
doesn't become the literal "null": read the value from localProperties
(localProperties.getProperty("kakao.native.app.key.dev") or
localProperties["kakao.native.app.key.dev"]) into a single non-null variable
(use .orEmpty() or fail-fast with a clear exception) and then reuse that
variable for both buildConfigField(...) and
manifestPlaceholders["kakaoNativeAppKey"] instead of calling
localProperties[...] twice.
In `@app/proguard-rules.pro`:
- Line 31: Update the misspelled comment "refrofit2 (with r8 full mode) - Kakao
SDK" by changing "refrofit2" to "retrofit2" so the comment reads "retrofit2
(with r8 full mode) - Kakao SDK"; locate the comment string in
proguard-rules.pro and correct the typo to improve searchability and clarity.
In
`@app/src/main/java/com/lyrics/feelin/core/data/datasource/sdk/GoogleAuthDataSource.kt`:
- Around line 5-7: Fix the typo in the TODO comment inside the
GoogleAuthDataSource class: change "Not Yet Implementated" to "Not Yet
Implemented" (or better "Not yet implemented") so the TODO reads correctly;
update the comment inside the GoogleAuthDataSource class constructor to use the
corrected spelling.
In
`@app/src/main/java/com/lyrics/feelin/core/data/datasource/sdk/KakaoAuthDataSource.kt`:
- Around line 75-77: The log TAG constant in the companion object is
inconsistent: change the value of TAG (currently "KakaoAuthDataService") to
match the class name "KakaoAuthDataSource" so log filtering is correct; update
the companion object’s private const val TAG to "KakaoAuthDataSource" wherever
it’s referenced (e.g., in KakaoAuthDataSource) to keep naming consistent.
- Around line 11-12: The class-level `@Singleton` on KakaoAuthDataSource is
ineffective because instances are created via the
DataModule.provideKakaoAuthDataSource() provider; remove the `@Singleton`
annotation from the KakaoAuthDataSource class and add `@Singleton` to the
`@Provides` method (provideKakaoAuthDataSource) in DataModule so Hilt will scope
the provided instance as a singleton.
In `@app/src/main/java/com/lyrics/feelin/core/data/di/DataModule.kt`:
- Around line 23-24: The provider method provideKakaoAuthDataSource currently
creates a new KakaoAuthDataSource on every injection; annotate the method with
`@Singleton` so the DI graph uses a single instance in the SingletonComponent
scope—update the provideKakaoAuthDataSource function to include the `@Singleton`
annotation while keeping the `@Provides` annotation to ensure KakaoAuthDataSource
is provided as a singleton.
In `@app/src/main/java/com/lyrics/feelin/core/data/di/NetworkModule.kt`:
- Around line 45-50: The Retrofit builder's baseUrl currently uses plaintext
"http://api.feelinapp.com/" in NetworkModule (inside the Retrofit.Builder()
call); change that value to use "https://api.feelinapp.com/" so auth-related
endpoints (sign-in, refresh, delete) use TLS, and ensure any localhost/local-dev
exceptions are gated behind an explicit dev-flavor or host check (not the
default). Update the baseUrl string in the Retrofit.Builder() invocation and
keep cleartext only for explicit local-dev configuration paths.
- Around line 30-37: The HTTP logging is set to BODY unconditionally causing
sensitive data (Authorization headers and token refresh bodies) to be logged;
change the OkHttp setup in the OkHttpClient.Builder so HttpLoggingInterceptor is
only added for debug builds (guard its creation with a build flag) and configure
the interceptor to redact the "Authorization" header before adding it; keep the
existing interceptors (appVersionInterceptor, authInterceptor) and authenticator
(tokenAuthenticator) but replace unconditional
.addInterceptor(httpLoggingInterceptor) with a conditional add that only inserts
a redacting, debug-only HttpLoggingInterceptor.
In
`@app/src/main/java/com/lyrics/feelin/core/data/interceptor/AppVersionInterceptor.kt`:
- Around line 16-18: In AppVersionInterceptor
(app/src/main/java/.../AppVersionInterceptor.kt) remove the temporary hardcoded
branch so that the app uses the real build version: replace the current val
appVersion = if (false) BuildConfig.VERSION_NAME else "1.0.3" with logic that
returns BuildConfig.VERSION_NAME (i.e., eliminate the if(false) and hardcoded
"1.0.3"); ensure the appVersion variable and any code that reads it continue to
compile and send BuildConfig.VERSION_NAME in requests, and add a comment or TODO
only if you need to track discussion with the server team.
In
`@app/src/main/java/com/lyrics/feelin/core/data/interceptor/AuthInterceptor.kt`:
- Around line 70-74: The authenticate logic in AuthInterceptor currently uses
response.request.header("Authorization") == null which is inverted and does not
stop repeated refresh attempts; replace that check with a prior-response based
guard used by OkHttp: inspect response.priorResponse (or walk priorResponse
chain) to detect if a 401 retry has already occurred and return null to stop
further refresh attempts in authenticate; update the authenticate method in
AuthInterceptor to bail out when response.priorResponse is non-null (or when
prior 401 retries >= 1) before attempting token refresh.
In `@app/src/main/java/com/lyrics/feelin/core/data/manager/AuthManager.kt`:
- Around line 61-96: The init block currently launches loadTokensFromStorage()
asynchronously which can leave accessToken/refreshToken null when network
requests (via AuthInterceptor) start; change initialization to a
suspending-ready pattern by adding a CompletableDeferred<Unit> (e.g.,
initializationComplete), complete it at the end of loadTokensFromStorage()
(launched on managerScope), and expose a suspend function awaitInitialization()
that calls initializationComplete.await(); then ensure callers (app
startup/MainActivity/FeelinApplication) call awaitInitialization() before making
auth-dependent requests so tokens are guaranteed loaded for AuthInterceptor.
In `@app/src/main/java/com/lyrics/feelin/core/data/repository/AuthRepository.kt`:
- Around line 56-60: In AuthRepository where you call
authRemoteDataSource.signIn (the block passing provider, oAuthToken, deviceId),
remove the hardcoded deviceId "android-develop-test-202603200009" and replace it
with a runtime value fetched from the device or persisted ID: e.g., obtain
Settings.Secure.ANDROID_ID or a locally generated UUID (persisted in
SharedPreferences/Datastore) via a helper (DeviceIdProvider / DeviceIdUtil) and
pass that value into signIn so each device has a unique identifier for push,
analytics, and sessions.
In `@app/src/main/java/com/lyrics/feelin/FeelinApplication.kt`:
- Line 12: Add a guard in FeelinApplication before calling KakaoSdk.init to
validate BuildConfig.KAKAO_NATIVE_APP_KEY is non-null and not blank; if it is
missing or empty, throw an IllegalStateException (or log an error and abort app
startup) with a clear message indicating KAKAO_NATIVE_APP_KEY is not configured,
then only call KakaoSdk.init(this, BuildConfig.KAKAO_NATIVE_APP_KEY) when
validation passes so the app fails fast on misconfiguration.
In `@app/src/main/java/com/lyrics/feelin/presentation/view/login/LoginScreen.kt`:
- Around line 243-246: The Google login button is active but
googleLogin(context, viewModel) is a no-op; either implement the flow or disable
the button UI until ready: update the LoginScreen where the Google button is
rendered to set it disabled/unclickable (or show a "Coming soon" state) and
remove/replace the onClick that calls googleLogin, or make googleLogin return a
user-facing placeholder (e.g., show a toast/dialog) and keep the button
disabled; reference the googleLogin function and the LoginScreen's Google button
handler so reviewers can find and change the click behavior and visual enabled
state consistently.
- Around line 61-67: The backend error handling in LoginScreen.kt force-unwraps
the nullable description on LoginUiState.Error (title!!) when checking for
LoginErrorType.BACKEND_SERVER, which can crash if description is null; change
the logic around LoginUiState.Error (and the local variable title) to safely
handle a null description by providing a fallback string (e.g., "서버 오류" or a
generic message) before passing it to FeelinModalDialog so the dialog always
receives a non-null title.
- Line 55: The preview fails because LoginScreen's default parameter uses
hiltViewModel<LoginViewModel>() which requires Hilt; refactor by extracting a
stateless composable (e.g., LoginContent) that takes the necessary state and
event callbacks, have LoginScreen(route) obtain the LoginViewModel and call
LoginContent, and update LoginScreenPreview to call LoginContent with a mocked
state and no-op callbacks (or alternatively create a fake LoginViewModel
instance and pass it explicitly into LoginScreenPreview); ensure references to
LoginScreen, LoginContent (new), LoginScreenPreview, and LoginViewModel are
updated so previews no longer trigger hiltViewModel().
In `@app/src/main/java/com/lyrics/feelin/util/HttpExceptionExtension.kt`:
- Around line 10-22: The current try/catch in HttpExceptionExtension.kt only
catches NullPointerException while Json.decodeFromString<ServerErrorDto> can
throw SerializationException and response()!!.errorBody()!!.string() can throw
IOException; update the catch to handle SerializationException and IOException
(and keep a broad fallback like Exception if desired), and eliminate the use of
"!!" by using safe-calls and elvis operators to obtain the error string (e.g.,
response()?.errorBody()?.string() ?: fallbackJson) before passing it to
Json.decodeFromString, logging the caught exception via Log.e("ServerErrorDto",
"...", e) and returning the fallback ServerErrorDto when parsing/IO fails.
In `@app/src/main/res/xml/network_security_config.xml`:
- Around line 2-7: The network-security-config currently enables
cleartextTrafficPermitted on the domain-config containing api.feelinapp.com and
dev.feelinapp.com; change this so production never allows cleartext by setting
cleartextTrafficPermitted="false" for the production config and move any HTTP
allowance into a debug-only config (or a separate domain-config limited to
10.0.2.2 / emulator IPs) that is selected via build variants; update the
manifest/application networkSecurityConfig reference to point to the debug
config for debug builds and keep the production config (network-security-config)
with cleartextTrafficPermitted="false" and only HTTPS domains listed.
---
Nitpick comments:
In `@agents.md`:
- Line 36: Add missing language identifiers to the fenced code blocks in
agents.md so the markdown linter stops flagging them; specifically update each
triple-backtick block (``` ) at the mentioned locations to include an
appropriate language token (e.g., use "text" or "gitcommit" for commit message
examples) for the blocks at lines 36, 46, 59, 69, 76, 101, 118, 132, 142, and
156.
In `@app/detekt.yml`:
- Around line 150-159: The current detekt excludes entry removes
TooManyFunctions checks for the entire "**/datasource/**" package; change it to
narrowly exclude only the specific files or classes that legitimately exceed
function limits (e.g., the concrete DataSource implementations) instead of the
whole directory. Update the excludes array to list explicit filenames or
class-specific patterns (replace "**/datasource/**" with targeted patterns like
"**/datasource/MyConcreteDataSource.kt" or class-name regexes) or configure the
TooManyFunctions rule to ignore only specified classes via its rule-specific
excludes; keep the rest of the datasource package under the default
TooManyFunctions checks.
In
`@app/src/main/java/com/lyrics/feelin/core/data/datasource/remote/AuthApiService.kt`:
- Around line 19-29: AuthApiService is using domain models (AuthToken,
SignUpData) at the Retrofit boundary; create dedicated DTO classes and swap them
into the API signatures to decouple serialization. Add request/response DTOs
(e.g., AuthTokenDto, SignInRequestDto, SignUpRequestDto) and change signIn to
accept SignInRequestDto and return Response<AuthTokenDto>, signUp to accept
SignUpRequestDto, and reIssueToken to accept RefreshTokenRequestDto/return
Response<AuthTokenDto>; update signOut to use ServerStatusResponseDto if already
a DTO. After changing the method signatures in AuthApiService, add mapping
functions between domain models and DTOs (e.g., toDomain()/toDto()) in the DTO
classes and update callers to perform conversion at the data layer only.
In
`@app/src/main/java/com/lyrics/feelin/core/data/datasource/remote/dto/SignInRequestDto.kt`:
- Around line 7-9: The DTO currently exposes authProvider: OAuthProvider which
serializes enum names directly; change SignInRequestDto to use a stable wire
type (e.g., String or a DTO-specific enum) and map domain OAuthProvider -> that
wire value in the datasource/repository layer; update the code paths that
construct SignInRequestDto to pass the agreed-upon literal/wire value for each
OAuthProvider and avoid serializing the domain enum directly so renames won’t
break the API contract.
In
`@app/src/main/java/com/lyrics/feelin/core/data/datasource/sdk/KakaoAuthDataSource.kt`:
- Around line 13-41: The logout suspend function using
suspendCancellableCoroutine should register continuation.invokeOnCancellation to
perform cleanup and avoid handling results after cancellation; update the
UserApiClient.instance.logout callback (in the suspendCancellableCoroutine block
inside logout and similarly in the other coroutine blocks at lines 44-73) to
check continuation.isActive before calling continuation.resume/Result and add
continuation.invokeOnCancellation { /* log and/or cancel SDK request if possible
*/ } so cancelled coroutines don't attempt to process SDK callbacks or leave
requests dangling.
In
`@app/src/main/java/com/lyrics/feelin/core/data/datasource/sdk/util/KakakoAuthSdkExtension.kt`:
- Line 1: Rename the source file named "KakakoAuthSdkExtension.kt" to
"KakaoAuthSdkExtension.kt" and update any internal references to the old
filename; ensure the Kotlin file still declares package
com.lyrics.feelin.core.data.datasource.sdk.util and that any imports, tests, or
usages referencing KakakoAuthSdkExtension are updated to the corrected
class/file name "KakaoAuthSdkExtension" to maintain consistency.
In `@app/src/main/java/com/lyrics/feelin/core/data/di/DataModule.kt`:
- Around line 9-18: The KDoc for DataModule is inaccurate: it mentions providing
AuthLocalDataSource and AuthManager but the module actually provides
KakaoAuthDataSource; update the KDoc above the DataModule declaration to list
the real provided singleton(s) (e.g., KakaoAuthDataSource) and remove or correct
references to AuthLocalDataSource and AuthManager so the comments accurately
reflect the module's contents and intent.
In
`@app/src/main/java/com/lyrics/feelin/core/data/interceptor/AppVersionInterceptor.kt`:
- Around line 19-22: The inline comment above the request builder in
AppVersionInterceptor.kt is misleading ("Authorization 헤더 추가") but the code adds
an "App-Version" header; update that comment to accurately reflect the header
being added (e.g., "App-Version 헤더 추가") or remove it, locating the change near
the authenticatedRequest assignment that calls
originalRequest.newBuilder().header("App-Version", appVersion).build().
In `@app/src/main/java/com/lyrics/feelin/core/data/repository/AuthRepository.kt`:
- Around line 186-189: Replace the unsafe getOrNull()!! usage on tokenResult
with tokenResult.getOrThrow() to make the "guaranteed success" assumption
explicit; locate the code in AuthRepository where tokenResult is read (the call
that currently does tokenResult.getOrNull()!!) and pass the resulting token into
authManager.saveServerToken as before, but obtained via getOrThrow() to avoid
the forced non-null assertion.
In `@app/src/main/java/com/lyrics/feelin/core/data/util/ResponseExtension.kt`:
- Around line 25-33: The safeApiCall function currently only catches IOException
so serialization errors from Retrofit converters can escape; update safeApiCall
(and/or the toResult conversion path) to also catch deserialization failures by
adding a catch block for kotlinx.serialization.SerializationException (or Gson's
JsonSyntaxException/JsonParseException if using Gson), and optionally include a
broader catch(Exception) fallback to convert those exceptions into
Result.failure with useful context; reference the suspend fun <T>
safeApiCall(...) and the toResult() call to locate where to add these additional
catches.
In `@app/src/main/java/com/lyrics/feelin/core/domain/model/OAuthProvider.kt`:
- Around line 11-13: The fromString function in OAuthProvider should normalize
input (trim whitespace and handle null/empty) before matching; update
OAuthProvider.fromString to first guard against null/blank inputs, call
value.trim() (and optionally toLowerCase()) and then use entries.find {
it.name.equals(normalizedValue, ignoreCase = true) } so inputs like " kakao "
resolve correctly.
In
`@app/src/main/java/com/lyrics/feelin/presentation/view/component/profile/ProfileComponent.kt`:
- Around line 15-16: The ProfileComponent currently uses remember {
mutableStateOf(ProfileComponentData.fromProfileType(type)) } which won’t update
when the type parameter changes and unnecessarily wraps the value in
mutableStateOf; change it to remember(type) {
ProfileComponentData.fromProfileType(type) } (or compute directly without
remember if not needed) so ProfileComponentData.fromProfileType(type) is
recomputed when type changes and remove the mutableStateOf wrapper; update
references to use the plain remembered value instead of a State object.
In
`@app/src/main/java/com/lyrics/feelin/presentation/view/login/LoginViewModel.kt`:
- Around line 66-70: Extract the hardcoded error code "02000" into a named
constant and use that constant in the LoginViewModel check; specifically, add a
descriptive constant (e.g., SIGN_UP_REQUIRED_ERROR_CODE) near the LoginViewModel
class or companion object, replace the literal in the FeelinServerException
branch (the condition it.description.errorCode == "02000") with the constant,
and ensure any tests or other references use the same constant for consistency.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7640ee2f-2474-4a3e-abb6-7206399cc5aa
📒 Files selected for processing (46)
SOCIAL_LOGIN_REFACTOR.mdagents.mdapp/build.gradle.ktsapp/detekt.ymlapp/proguard-rules.proapp/src/main/AndroidManifest.xmlapp/src/main/java/com/lyrics/feelin/FeelinApplication.ktapp/src/main/java/com/lyrics/feelin/core/data/datasource/local/AuthLocalDataSource.ktapp/src/main/java/com/lyrics/feelin/core/data/datasource/remote/AuthApiService.ktapp/src/main/java/com/lyrics/feelin/core/data/datasource/remote/AuthRemoteDataSource.ktapp/src/main/java/com/lyrics/feelin/core/data/datasource/remote/dto/RefreshTokenRequestDto.ktapp/src/main/java/com/lyrics/feelin/core/data/datasource/remote/dto/ServerErrorDto.ktapp/src/main/java/com/lyrics/feelin/core/data/datasource/remote/dto/ServerStatusResponseDto.ktapp/src/main/java/com/lyrics/feelin/core/data/datasource/remote/dto/SignInRequestDto.ktapp/src/main/java/com/lyrics/feelin/core/data/datasource/remote/dto/exception/FeelinServerException.ktapp/src/main/java/com/lyrics/feelin/core/data/datasource/sdk/GoogleAuthDataSource.ktapp/src/main/java/com/lyrics/feelin/core/data/datasource/sdk/KakaoAuthDataSource.ktapp/src/main/java/com/lyrics/feelin/core/data/datasource/sdk/util/KakakoAuthSdkExtension.ktapp/src/main/java/com/lyrics/feelin/core/data/di/DataModule.ktapp/src/main/java/com/lyrics/feelin/core/data/di/NetworkModule.ktapp/src/main/java/com/lyrics/feelin/core/data/interceptor/AppVersionInterceptor.ktapp/src/main/java/com/lyrics/feelin/core/data/interceptor/AuthInterceptor.ktapp/src/main/java/com/lyrics/feelin/core/data/manager/AuthManager.ktapp/src/main/java/com/lyrics/feelin/core/data/repository/AuthRepository.ktapp/src/main/java/com/lyrics/feelin/core/data/util/ResponseExtension.ktapp/src/main/java/com/lyrics/feelin/core/domain/model/AuthToken.ktapp/src/main/java/com/lyrics/feelin/core/domain/model/Gender.ktapp/src/main/java/com/lyrics/feelin/core/domain/model/OAuthProvider.ktapp/src/main/java/com/lyrics/feelin/core/domain/model/OAuthToken.ktapp/src/main/java/com/lyrics/feelin/core/domain/model/ProfileType.ktapp/src/main/java/com/lyrics/feelin/core/domain/model/SignUpData.ktapp/src/main/java/com/lyrics/feelin/core/domain/model/TermAgreementStatus.ktapp/src/main/java/com/lyrics/feelin/navigation/FeelinNavHost.ktapp/src/main/java/com/lyrics/feelin/presentation/view/component/note/NoteComponentData.ktapp/src/main/java/com/lyrics/feelin/presentation/view/component/profile/ProfileComponent.ktapp/src/main/java/com/lyrics/feelin/presentation/view/component/profile/ProfileComponentData.ktapp/src/main/java/com/lyrics/feelin/presentation/view/login/LoginScreen.ktapp/src/main/java/com/lyrics/feelin/presentation/view/login/LoginViewModel.ktapp/src/main/java/com/lyrics/feelin/presentation/view/mypage/MyPageScreen.ktapp/src/main/java/com/lyrics/feelin/presentation/view/mypage/MyPageScreenState.ktapp/src/main/java/com/lyrics/feelin/presentation/view/mypage/MyPageViewModel.ktapp/src/main/java/com/lyrics/feelin/util/HttpExceptionExtension.ktapp/src/main/res/xml/network_security_config.xmlbuild.gradle.ktsgradle/libs.versions.tomlsettings.gradle.kts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Around line 36-40: The fenced code blocks containing commit/message examples
(e.g., the blocks starting with the commit list "feat: Add Kakao login feature
..." and the blocks showing "Subject: Summary of changes", checklists like "✅
feat: Add user authentication", and the generic message examples like "fixed
stuff\nupdated files...") are missing language identifiers and trigger
markdownlint MD040; update each fenced code block by adding an explicit language
identifier (use "text") after the opening triple-backticks for those blocks and
any other similar fenced blocks flagged in this doc so they render and
lint-cleanly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4a465c3a-2fc1-439b-93d9-28b459414e15
📒 Files selected for processing (4)
AGENTS.mdCLAUDE.mdapp/proguard-rules.proapp/src/main/java/com/lyrics/feelin/core/data/datasource/sdk/KakaoAuthDataSource.kt
✅ Files skipped from review due to trivial changes (1)
- app/proguard-rules.pro
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/lyrics/feelin/core/data/datasource/sdk/KakaoAuthDataSource.kt
Stop retrying token refresh when a request has already been retried once after a 401 response.
fix ci failure when local.properties can't exist
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/android-ci.yml:
- Around line 12-14: Move KAKAO_NATIVE_APP_KEY_DEV out of the workflow-level env
and add it only to the build step that runs the Gradle assemble (the step that
executes ./gradlew assembleDebug or is named "Build"/"assembleDebug"); remove it
from the global env block so the detekt/static-analysis step no longer receives
the secret. Ensure the build step's env includes KAKAO_NATIVE_APP_KEY_DEV: ${{
secrets.KAKAO_NATIVE_APP_KEY_DEV }} so only the assembleDebug build phase has
access.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 28fb541f-daa9-446a-9e3f-667368ed68b7
📒 Files selected for processing (2)
.github/workflows/android-ci.ymlapp/build.gradle.kts
Resolve missing Kakao key failures by reading the CI environment variable before falling back to local.properties. Use a dummy value in PR CI so fork-based build checks can run without exposing or requiring real secrets.
- add error description's null-fallback
Wait for auth token restoration before interceptor reads cached JWTs. This prevents cold-start requests from being sent without Authorization headers.
Member
Author
|
@hyunjung-choi 코드 부분부분의 문제는 수정했으므로 머지하겠습니다. |
Collaborator
|
@gdaegeun539 고생하셨습니다 ! 🙇♀️ 해당 PR은 추후에도 계속 살펴보도록 할게요 . . ! |
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
2 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.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce?
What is the current behavior?
What is the new behavior (if this is a feature change)?
LoginScreen에 격리해 구현합니다.Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
Yes
'What is the new behavior' 섹션에 서술된 것 처럼 라이브러리 추가 및 초기 설정이 많습니다.
Hilt를 사용하기 위한 기본 설정 및 스코프를 설정합니다.
SCRUM-243 init: kakao login sdk set #12 과 같이 레포지토리와 DataSource는
@Singleton스코프를 유지합니다.몇 개의 함수가 detekt의 반환 개수 기준을 통과하지 못해 CI가 실패하는데, 이 기준은 한 번 논의했으면 합니다.
Other information:
PR 양이 많은데, 양해 부탁드립니다. 억지로 작업량을 줄인 #12 처럼 일부분만을 올리는 것 보다 완전히 작동하는 코드를 올리는 게 맞다고 보아, 외부 SDK 설정-HTTP 통신-코드 연동을 위한 Hilt 설정을 모두 진행한 PR로 다시 올립니다.
지금은 소셜 토큰으로 로그인을 진행할 때 우리 서버에 계정이 없어 02000 에러를 받는 상황까지만 구현됐고, 이후 사항은 회원가입 온보딩에 연동해야 해요.
혹시 예외처리를 완전히 진행하는 걸 원하시거나, 다이얼로그까지 띄우는 것을 원하신다면 말씀해 주세요. 이 PR에 기능이 이미 많아서 통합하는 건 어려울 거 같은데, 진행을 원하시면 다른 티켓을 끊어서 작업할게요.-> 2026.03. 수정: 예외로 인한 다이얼로그 표시 및 회원가입/홈 내비게이션 연동까지 진행됐습니다.
Summary by CodeRabbit
New Features
Improvements
Documentation