Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.ninecraft.booket.core.data.api.repository

import com.ninecraft.booket.core.model.AutoLoginState
import com.ninecraft.booket.core.model.LoginMethod
import com.ninecraft.booket.core.model.UserState
import kotlinx.coroutines.flow.Flow

Expand All @@ -16,4 +17,10 @@ interface AuthRepository {
val userState: Flow<UserState>

suspend fun getCurrentUserState(): UserState

val recentLoginMethod: Flow<LoginMethod>

suspend fun setRecentLoginMethod(loginMethod: LoginMethod)

suspend fun clearRecentLoginMethod()
Comment on lines +21 to +25
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find and examine the logout implementation in DefaultAuthRepository
rg -A 10 "override suspend fun logout" --type kotlin

Repository: YAPP-Github/Reed-Android

Length of output: 1459


logout() 메서드에서 clearRecentLoginMethod() 호출 누락

logout() 메서드에서 clearRecentLoginMethod()를 호출하지 않고 있습니다. withdraw() 메서드에서는 호출되고 있으나, logout()에서는 누락되어 있습니다. 사용자가 로그아웃 후 다시 로그인 화면에 접근할 때 이전 로그인 방식이 표시되는 불일치한 동작이 발생합니다. logout() 메서드에도 clearRecentLoginMethod()를 추가하여 withdraw()와 일관성 있게 수정해주세요.

🤖 Prompt for AI Agents
In
core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/AuthRepository.kt
around lines 21 to 25, the recentLoginMethod flow and clearRecentLoginMethod()
are defined but the logout() implementation does not call
clearRecentLoginMethod(), causing the previous login method to remain visible
after logout; update the logout() method to invoke suspend fun
clearRecentLoginMethod() (same as withdraw()) so the stored recent login method
is cleared on logout, and ensure any call is awaited and errors are handled
consistently with withdraw().

}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package com.ninecraft.booket.core.data.impl.repository

import com.ninecraft.booket.core.common.utils.runSuspendCatching
import com.ninecraft.booket.core.data.api.repository.AuthRepository
import com.ninecraft.booket.core.datastore.api.datasource.LoginMethodDataSource
import com.ninecraft.booket.core.datastore.api.datasource.TokenDataSource
import com.ninecraft.booket.core.model.AutoLoginState
import com.ninecraft.booket.core.model.LoginMethod
import com.ninecraft.booket.core.model.UserState
import com.ninecraft.booket.core.network.request.LoginRequest
import com.ninecraft.booket.core.network.service.ReedService
Expand All @@ -19,6 +21,7 @@ private const val KAKAO_PROVIDER_TYPE = "KAKAO"
class DefaultAuthRepository(
private val service: ReedService,
private val tokenDataSource: TokenDataSource,
private val loginMethodDataSource: LoginMethodDataSource,
) : AuthRepository {
override suspend fun login(accessToken: String) = runSuspendCatching {
val response = service.login(
Expand All @@ -38,6 +41,7 @@ class DefaultAuthRepository(
override suspend fun withdraw() = runSuspendCatching {
service.withdraw()
clearTokens()
clearRecentLoginMethod()
}

private suspend fun saveTokens(accessToken: String, refreshToken: String) {
Expand All @@ -51,6 +55,10 @@ class DefaultAuthRepository(
tokenDataSource.clearTokens()
}

override suspend fun clearRecentLoginMethod() {
loginMethodDataSource.clearRecentLoginMethod()
}

override val autoLoginState = tokenDataSource.accessToken
.map { accessToken ->
if (accessToken.isBlank()) AutoLoginState.NOT_LOGGED_IN else AutoLoginState.LOGGED_IN
Expand All @@ -65,4 +73,10 @@ class DefaultAuthRepository(
val accessToken = tokenDataSource.getAccessToken()
return if (accessToken.isBlank()) UserState.Guest else UserState.LoggedIn
}

override val recentLoginMethod = loginMethodDataSource.recentLoginMethod

override suspend fun setRecentLoginMethod(loginMethod: LoginMethod) {
loginMethodDataSource.setRecentLoginMethod(loginMethod)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package com.ninecraft.booket.core.datastore.api.datasource

import com.ninecraft.booket.core.model.LoginMethod
import kotlinx.coroutines.flow.Flow

interface LoginMethodDataSource {
val recentLoginMethod: Flow<LoginMethod>
suspend fun setRecentLoginMethod(loginMethod: LoginMethod)
suspend fun clearRecentLoginMethod()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package com.ninecraft.booket.core.datastore.impl.datasource

import androidx.datastore.core.DataStore
import androidx.datastore.preferences.core.Preferences
import androidx.datastore.preferences.core.edit
import androidx.datastore.preferences.core.stringPreferencesKey
import com.ninecraft.booket.core.datastore.api.datasource.LoginMethodDataSource
import com.ninecraft.booket.core.datastore.impl.di.LoginMethodDataStore
import com.ninecraft.booket.core.datastore.impl.util.handleIOException
import com.ninecraft.booket.core.di.DataScope
import com.ninecraft.booket.core.model.LoginMethod
import dev.zacsweers.metro.Inject
import dev.zacsweers.metro.SingleIn
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.map

@SingleIn(DataScope::class)
@Inject
class DefaultLoginMethodDataSource(
@LoginMethodDataStore private val dataStore: DataStore<Preferences>,
) : LoginMethodDataSource {
override val recentLoginMethod: Flow<LoginMethod> = dataStore.data
.handleIOException()
.map { prefs ->
val method = prefs[RECENT_LOGIN_METHOD]
when (method) {
"KAKAO" -> LoginMethod.KAKAO
"GOOGLE" -> LoginMethod.GOOGLE
else -> LoginMethod.NONE
}
}

override suspend fun setRecentLoginMethod(loginMethod: LoginMethod) {
dataStore.edit { prefs ->
prefs[RECENT_LOGIN_METHOD] = loginMethod.name
}
}

override suspend fun clearRecentLoginMethod() {
dataStore.edit { prefs ->
prefs.remove(RECENT_LOGIN_METHOD)
}
}

companion object {
private val RECENT_LOGIN_METHOD = stringPreferencesKey("RECENT_LOGIN_METHOD")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ import androidx.datastore.preferences.core.Preferences
import androidx.datastore.preferences.preferencesDataStore
import com.ninecraft.booket.core.datastore.api.datasource.BookRecentSearchDataSource
import com.ninecraft.booket.core.datastore.api.datasource.LibraryRecentSearchDataSource
import com.ninecraft.booket.core.datastore.api.datasource.LoginMethodDataSource
import com.ninecraft.booket.core.datastore.api.datasource.NotificationDataSource
import com.ninecraft.booket.core.datastore.api.datasource.OnboardingDataSource
import com.ninecraft.booket.core.datastore.api.datasource.TokenDataSource
import com.ninecraft.booket.core.datastore.impl.datasource.DefaultBookRecentSearchDataSource
import com.ninecraft.booket.core.datastore.impl.datasource.DefaultLibraryRecentSearchDataSource
import com.ninecraft.booket.core.datastore.impl.datasource.DefaultLoginMethodDataSource
import com.ninecraft.booket.core.datastore.impl.datasource.DefaultNotificationDataSource
import com.ninecraft.booket.core.datastore.impl.datasource.DefaultOnboardingDataSource
import com.ninecraft.booket.core.datastore.impl.datasource.DefaultTokenDataSource
Expand All @@ -25,12 +27,14 @@ private const val BOOK_RECENT_SEARCH_DATASTORE_NAME = "BOOK_RECENT_SEARCH_DATAST
private const val LIBRARY_RECENT_SEARCH_DATASTORE_NAME = "LIBRARY_RECENT_SEARCH_DATASTORE"
private const val ONBOARDING_DATASTORE_NAME = "ONBOARDING_DATASTORE"
private const val NOTIFICATION_DATASTORE_NAME = "NOTIFICATION_DATASTORE"
private const val LOGIN_METHOD_DATASTORE_NAME = "LOGIN_METHOD_DATASTORE"

private val Context.tokenDataStore by preferencesDataStore(name = TOKEN_DATASTORE_NAME)
private val Context.bookRecentSearchDataStore by preferencesDataStore(name = BOOK_RECENT_SEARCH_DATASTORE_NAME)
private val Context.libraryRecentSearchDataStore by preferencesDataStore(name = LIBRARY_RECENT_SEARCH_DATASTORE_NAME)
private val Context.onboardingDataStore by preferencesDataStore(name = ONBOARDING_DATASTORE_NAME)
private val Context.notificationDataStore by preferencesDataStore(name = NOTIFICATION_DATASTORE_NAME)
private val Context.loginMethodDataStore by preferencesDataStore(name = LOGIN_METHOD_DATASTORE_NAME)

@ContributesTo(DataScope::class)
interface DataStoreGraph {
Expand Down Expand Up @@ -65,6 +69,12 @@ interface DataStoreGraph {
@ApplicationContext context: Context,
): DataStore<Preferences> = context.notificationDataStore

@LoginMethodDataStore
@Provides
fun provideLoginMethodDataStore(
@ApplicationContext context: Context,
): DataStore<Preferences> = context.loginMethodDataStore

@Binds
val DefaultTokenDataSource.bind: TokenDataSource

Expand All @@ -79,4 +89,7 @@ interface DataStoreGraph {

@Binds
val DefaultNotificationDataSource.bind: NotificationDataSource

@Binds
val DefaultLoginMethodDataSource.bind: LoginMethodDataSource
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,7 @@ annotation class OnboardingDataStore
@Qualifier
@Retention(AnnotationRetention.BINARY)
annotation class NotificationDataStore

@Qualifier
@Retention(AnnotationRetention.BINARY)
annotation class LoginMethodDataStore
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.ninecraft.booket.core.model

enum class LoginMethod {
NONE,
KAKAO,
GOOGLE,
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.ninecraft.booket.feature.login

import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.rememberCoroutineScope
Expand All @@ -10,6 +11,7 @@ import com.ninecraft.booket.core.common.constants.ErrorScope
import com.ninecraft.booket.core.common.event.postErrorDialog
import com.ninecraft.booket.core.data.api.repository.AuthRepository
import com.ninecraft.booket.core.data.api.repository.UserRepository
import com.ninecraft.booket.core.model.LoginMethod
import com.ninecraft.booket.feature.screens.HomeScreen
import com.ninecraft.booket.feature.screens.LoginScreen
import com.ninecraft.booket.feature.screens.TermsAgreementScreen
Expand Down Expand Up @@ -50,6 +52,15 @@ class LoginPresenter(
val scope = rememberCoroutineScope()
var isLoading by rememberRetained { mutableStateOf(false) }
var sideEffect by rememberRetained { mutableStateOf<LoginSideEffect?>(null) }
var showLoginTooltip by rememberRetained { mutableStateOf(false) }
var recentLoginMethod by rememberRetained { mutableStateOf(LoginMethod.NONE) }

LaunchedEffect(Unit) {
authRepository.recentLoginMethod.collect { method ->
recentLoginMethod = method
showLoginTooltip = method != LoginMethod.NONE
}
}
Comment on lines +58 to +63
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

툴팁 해제 후 다시 표시되는 잠재적 문제가 있습니다.

LaunchedEffect에서 collect를 사용하여 recentLoginMethod를 수신하고 있는데, 사용자가 툴팁을 해제(showLoginTooltip = false)한 후에도 Flow에서 동일한 값이 다시 emit되면 툴팁이 다시 표시될 수 있습니다.

해제 상태를 별도로 추적하거나, collectLatest와 함께 해제 플래그를 관리하는 방식을 고려해 주세요.

🔎 제안하는 수정 방안
+        var hasUserDismissedTooltip by rememberRetained { mutableStateOf(false) }
+
         LaunchedEffect(Unit) {
             authRepository.recentLoginMethod.collect { method ->
                 recentLoginMethod = method
-                showLoginTooltip = method != LoginMethod.NONE
+                showLoginTooltip = method != LoginMethod.NONE && !hasUserDismissedTooltip
             }
         }

그리고 OnDismissLoginTooltip 핸들러에서:

                 is LoginUiEvent.OnDismissLoginTooltip -> {
                     showLoginTooltip = false
+                    hasUserDismissedTooltip = true
                 }
📝 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.

Suggested change
LaunchedEffect(Unit) {
authRepository.recentLoginMethod.collect { method ->
recentLoginMethod = method
showLoginTooltip = method != LoginMethod.NONE
}
}
var hasUserDismissedTooltip by rememberRetained { mutableStateOf(false) }
LaunchedEffect(Unit) {
authRepository.recentLoginMethod.collect { method ->
recentLoginMethod = method
showLoginTooltip = method != LoginMethod.NONE && !hasUserDismissedTooltip
}
}
🤖 Prompt for AI Agents
In
feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginPresenter.kt
around lines 58 to 63, the LaunchedEffect collects recentLoginMethod and
unconditionally sets showLoginTooltip when a value is emitted, which allows the
tooltip to reappear if the Flow re-emits the same/non-NONE method after the user
dismissed it; fix by adding a dismissed flag (e.g., loginTooltipDismissed) that
the OnDismissLoginTooltip handler sets to true, and in the collector only set
showLoginTooltip = true when method != LoginMethod.NONE AND
loginTooltipDismissed is false (or use collectLatest and check the dismissed
flag before showing), ensuring the dismiss action prevents future emissions from
re-showing the tooltip.


fun navigateAfterLogin() {
scope.launch {
Expand Down Expand Up @@ -97,6 +108,7 @@ class LoginPresenter(
isLoading = true
authRepository.login(event.accessToken)
.onSuccess {
authRepository.setRecentLoginMethod(LoginMethod.KAKAO)
userRepository.syncFcmToken()
navigateAfterLogin()
}.onFailure { exception ->
Expand All @@ -120,6 +132,10 @@ class LoginPresenter(
is LoginUiEvent.OnCloseButtonClick -> {
navigator.pop()
}

is LoginUiEvent.OnDismissLoginTooltip -> {
showLoginTooltip = false
}
}
}

Expand All @@ -131,6 +147,8 @@ class LoginPresenter(
isLoading = isLoading,
returnToScreen = screen.returnToScreen,
sideEffect = sideEffect,
showLoginTooltip = showLoginTooltip,
recentLoginMethod = recentLoginMethod,
eventSink = ::handleEvent,
)
}
Expand Down
Loading
Loading