feat: iOS와 로그인 플로우 통일(Guest Mode 구현)#174
Conversation
LibrarySearch 화면과 분리를 위한 도서 검색화면 prefix Book 추가
…rs로 변경 buttonColors -> textButtonColors
Walkthrough게스트 모드 및 로그인 리다이렉트 통합. UserState 추가와 AuthRepository API 확장, 게스트 전용 도서 검색 엔드포인트/모델/매퍼/인터셉터 추가, 여러 화면(Home/Library/Search/Settings 등)에 게스트 분기 및 로그인 유도 UI/사이드이펙트 적용, 스크린 타입들의 서명 변경(예: Login/Terms data class, BookSearchScreen) 포함. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Home as HomePresenter
participant Auth as AuthRepository
participant UI as HomeUi
User->>Home: Open Home
Home->>Auth: collect userState
alt userState == Guest
Home->>UI: state.isGuestMode = true
UI-->>User: Guest-only UI (등록/안내)
else LoggedIn
Home->>UI: state.isGuestMode = false
Home->>Home: load home content (getHome)
UI-->>User: Home content
end
sequenceDiagram
autonumber
actor User
participant Search as BookSearchPresenter
participant Auth as AuthRepository
participant Repo as BookRepository
participant Net as ReedService
participant Nav as Navigator
User->>Search: Search(query)
Search->>Auth: read userState
alt Guest
Search->>Repo: searchBookAsGuest(query,start)
Repo->>Net: GET /api/v1/books/guest/search
Net-->>Repo: GuestBookSearchResponse
else LoggedIn
Search->>Repo: searchBook(query,start)
Repo->>Net: GET /api/v1/books/search (auth)
Net-->>Repo: BookSearchResponse
end
Repo-->>Search: BookSearchModel
Search-->>User: Results
User->>Search: Try register book
alt Guest
Search-->>User: Toast(login_required)
Search->>Nav: redirectToLogin()
else LoggedIn
Search->>Repo: upsertBook(...)
Repo-->>Search: Result
Search-->>User: Success/Failure toast
end
sequenceDiagram
autonumber
actor User
participant Any as AnyPresenter
participant Nav as Navigator
Any->>Nav: redirectToLogin()
Nav->>Nav: peek() -> currentScreen
Nav->>Nav: delayedGoTo(LoginScreen(returnToScreen=currentScreen))
Note right of Nav: returns previous screen to caller
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
feature/edit/src/main/kotlin/com/ninecraft/booket/feature/edit/record/RecordEditPresenter.kt (1)
39-42: Analytics 상수 충돌: 화면 이름과 저장 이벤트가 동일 키 사용
RECORD_EDIT와RECORD_EDIT_SAVE가 모두"record_edit_save"로 동일합니다. 화면 뷰와 이벤트 키를 구분해야 분석이 가능합니다.- private const val RECORD_EDIT = "record_edit_save" + private const val RECORD_EDIT = "record_edit" private const val RECORD_EDIT_SAVE = "record_edit_save"feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt (1)
47-61: Loading 중복 호출 가능성 차단 필요
uiState가Loading인 경우에도 네트워크 호출이 재시작될 수 있습니다(빠른 재시도/재진입). 진입부 가드 추가를 권장합니다.다음 diff로 재호출을 방지하세요:
fun loadHomeContent() { - scope.launch { - if (uiState is UiState.Idle || uiState is UiState.Error) { - uiState = UiState.Loading - } + if (uiState is UiState.Loading) return + scope.launch { + if (uiState is UiState.Idle || uiState is UiState.Error) { + uiState = UiState.Loading + }feature/login/src/main/kotlin/com/ninecraft/booket/feature/termsagreement/TermsAgreementPresenter.kt (1)
41-41: TermsAgreementUiState에 sideEffect 필드 추가 및 Presenter에서 전달
- Presenter에서 설정한
sideEffect를 UI로 내보내도록TermsAgreementPresenter.kt에서 상태 반환부 수정:return TermsAgreementUiState( isAllAgreed = isAllAgreed, agreedTerms = agreedTerms, - eventSink = ::handleEvent, + sideEffect = sideEffect, + eventSink = ::handleEvent, )TermsAgreementUiState.kt데이터 클래스에val sideEffect: TermsAgreementSideEffect?프로퍼티 추가- UI 소비 후
sideEffect = null로 리셋하는 로직도 구현 필요feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.kt (1)
177-183: 게스트 모드에서 라이브러리 초기 로딩이 401을 유발합니다 + 상태 변화에 재구동되지 않습니다.PR 목표상 라이브러리는 비로그인 접근 불가입니다. 현재 RememberedEffect(Unit)으로 게스트 상태에서도 API를 호출하고, 로그인으로 상태가 바뀌어도 재호출되지 않습니다. userState를 key로 사용하고, 게스트일 때는 호출을 건너뛰세요.
[Suggest fix]- RememberedEffect(Unit) { - filterLibraryBooks( - status = currentFilter.getApiValue(), - page = START_INDEX, - size = PAGE_SIZE, - ) - } + RememberedEffect(userState) { + if (userState is UserState.LoggedIn) { + filterLibraryBooks( + status = currentFilter.getApiValue(), + page = START_INDEX, + size = PAGE_SIZE, + ) + } else { + // 게스트 상태: 목록/푸터 초기화로 불필요한 에러 UI 방지 + uiState = UiState.Idle + footerState = FooterState.Idle + books = persistentListOf() + currentPage = START_INDEX + isLastPage = false + } + }feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt (1)
113-122: 401 처리 누락 및 오류 메시지 일관성 — handleException 사용으로 통일 권장검색 실패 분기에서 401→로그인 이동이 누락되어 있고, 메시지도 직접 파싱합니다.
handleException을 재사용해 로그인 요구/오류 메시지를 일관되게 처리하세요.- .onFailure { exception -> - Logger.d(exception) - analyticsHelper.logEvent(ERROR_SEARCH_LOADING) - val errorMessage = exception.message ?: "알 수 없는 오류가 발생했습니다." - if (startIndex == START_INDEX) { - uiState = UiState.Error(exception) - } else { - footerState = FooterState.Error(errorMessage) - } - } + .onFailure { exception -> + analyticsHelper.logEvent(ERROR_SEARCH_LOADING) + handleException( + exception = exception, + onError = { message -> + Logger.d(exception) + if (startIndex == START_INDEX) { + uiState = UiState.Error(exception) + } else { + footerState = FooterState.Error(message) + } + }, + onLoginRequired = { + navigator.redirectToLogin() + }, + ) + }
🧹 Nitpick comments (33)
core/network/src/main/kotlin/com/ninecraft/booket/core/network/TokenInterceptor.kt (1)
19-35: contains 기반 경로 매칭은 우발적 우회 위험 — encodedPath 기준 정합 매칭 제안현재
url.contains(...)는 쿼리스트링/부분 경로 포함 시 우발적 매칭 가능성이 있습니다.request.url.encodedPath로 정확히 일치(또는 startsWith/Regex 앵커) 검사로 변경을 권장합니다.예시 리팩터:
- private val noAuthEndpoints = setOf( - "api/v1/auth/signin", - "api/v1/auth/refresh", - "api/v1/books/guest/search", - ) + private val noAuthPaths = setOf( + "/api/v1/auth/signin", + "/api/v1/auth/refresh", + "/api/v1/books/guest/search", + ) override fun intercept(chain: Interceptor.Chain): Response { val request = chain.request() - val url = request.url.toString() - - val isNoAuthEndpoint = noAuthEndpoints.any { url.contains(it) } + val path = request.url.encodedPath + val isNoAuthEndpoint = path in noAuthPathscore/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/button/ButtonColorStyle.kt (1)
11-21: TEXT 컬러 스타일 추가 LGTM — KAKAO pressed 동일 정책 반영 확인설계 의도에 부합합니다. 과거 학습에 따르면 KAKAO는 pressed 시 동일 색상을 유지해야 하며, 현재 구현이 이를 준수합니다. 다만 STROKE의 pressed/idle 색이 동일한데, 의도라면 주석으로 명시해 주세요. 의도가 아니라면 pressed 시 미세 변화가 있도록 색을 분기하는 것도 고려해 주세요.
Also applies to: 29-37, 43-46
core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/button/ReedTextButton.kt (1)
3-11: FlowRow 안정화 여부 확인 — 필요 시 @OptIn 추가프로젝트의 Compose 버전에 따라
FlowRow가 여전히 실험 API일 수 있습니다. 미리보기 함수에만@OptIn(ExperimentalLayoutApi::class)를 적용하면 안전합니다.import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.ExperimentalLayoutApi import androidx.compose.foundation.layout.FlowRow ... -@ComponentPreview -@Composable +@ComponentPreview +@Composable +@OptIn(ExperimentalLayoutApi::class) private fun ReedTextButtonPreview() {Also applies to: 76-83
feature/edit/src/main/kotlin/com/ninecraft/booket/feature/edit/record/RecordEditPresenter.kt (1)
51-56: 페이지 유효성: 음수/0/비정상 입력 허용 가능성현재는
MAX_PAGE초과만 체크합니다. 1 이상MAX_PAGE이하만 허용하도록 범위를 명시하세요. 빈 입력일 때는 에러가 뜨지 않도록 고려했습니다.- val isPageError by remember { - derivedStateOf { - val page = recordPageState.text.toString().toIntOrNull() ?: 0 - page > MAX_PAGE - } - } + val isPageError by remember { + derivedStateOf { + val text = recordPageState.text.toString() + val page = text.toIntOrNull() + text.isNotEmpty() && (page == null || page !in 1..MAX_PAGE) + } + }core/designsystem/src/main/res/values/strings.xml (1)
6-6: LGTM: 공통 '로그인 필요' 메시지 추가게스트 가드 공통 메시지로 적절합니다. 톤앤매너 통일이 필요하면 문장부호/경어체는 디자인 가이드에 맞춰 일괄 조정하면 됩니다.
feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.kt (2)
85-90: 페이지 유효성 범위 보강 제안
MAX_PAGE초과만 체크 중입니다. 1..MAX_PAGE 범위를 사용하면 음수/0/비정상 입력 방지가 됩니다(빈 값은 에러 미노출).- val isPageError by remember { - derivedStateOf { - val page = recordPageState.text.toString().toIntOrNull() ?: 0 - page > MAX_PAGE - } - } + val isPageError by remember { + derivedStateOf { + val text = recordPageState.text.toString() + val page = text.toIntOrNull() + text.isNotEmpty() && (page == null || page !in 1..MAX_PAGE) + } + }
45-53: 중복 상수(DRY): MAX_PAGE 값의 중복 정의 제거 권장
RecordEditPresenter와 동일한MAX_PAGE = 4032가 반복됩니다. 공용 모듈(예: core/constants)로 승격해 단일 소스로 관리하세요.feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/library/LibrarySearchPresenter.kt (1)
35-43: 일관성 제안: Presenter에도 screen 주입 고려다른 Presenter들처럼
@Assisted screen: LibrarySearchScreen을 주입하면 상태 복원이나 향후 파라미터화가 쉬워집니다.feature/search/src/main/res/values/strings.xml (1)
21-21: 용어 표기 통일: ISBN 대문자 표기 권장국제 표준 약어는 대문자로 표기하는 것이 일반적입니다.
- <string name="error_book_no_isbn">isbn이 없는 도서는 등록할 수 없습니다</string> + <string name="error_book_no_isbn">ISBN이 없는 도서는 등록할 수 없습니다</string>feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/extensions/Navigator.kt (1)
19-23: redirectToLogin 네비게이션 전략 일관성 개선 제안
resetRoot: Boolean = false플래그를 추가해 상황에 따라resetRoot(LoginScreen(current))또는delayedGoTo(LoginScreen(current))을 선택하도록 개선.- LoginScreen은
data class LoginScreen(val returnToScreen: Screen? = null)으로returnToScreen이 이미Screen?타입임.feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt (1)
59-60: LoginScreen 생성자에 returnToScreen 인자 명시 적용
스플래시 경로의navigator.resetRoot(LoginScreen())를 아래와 같이 수정해 주세요. 레거시 호출(resetRoot(LoginScreen))은 코드베이스에서 발견되지 않았습니다.- navigator.resetRoot(LoginScreen()) + navigator.resetRoot(LoginScreen(returnToScreen = HomeScreen))feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUiState.kt (1)
16-16: isGuestMode 추가 적합 — 성능/안정성 위해 UiState에 @immutable 부여 권장Compose 최적화를 위해
SettingsUiState자체에@Immutable을 부여하면 리컴포지션 비용을 낮출 수 있습니다. (이미Immutableimport됨)-import androidx.compose.runtime.Immutable +import androidx.compose.runtime.Immutable @@ -data class SettingsUiState( +@Immutable +data class SettingsUiState( val isLoading: Boolean = false, @@ - val isGuestMode: Boolean = false, + val isGuestMode: Boolean = false,또한 토스트 등 문자열 리소스 일관성을 위해
SettingsSideEffect.ShowToast.message를UiText로 전환하는 것도 고려해 주세요.feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/HandleBookSearchSideEffect.kt (1)
11-12: 사이드이펙트 처리 로직에 early-return 패턴 적용 제안
RememberedEffect(state.sideEffect)내부에서val effect = state.sideEffect ?: return@RememberedEffect로 바로 분기하면 중복 접근·분기문이 줄어들어 가독성이 향상됩니다.–
HandleBookSearchSideEffects호출부(BookSearchUi.kt:54)에서eventSink가 정상 전달되고 있음을 확인했습니다.feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUi.kt (1)
153-160: 로그인 항목에도 회전 화살표 아이콘을 추가해 리스트 일관성 유지Privacy/Terms/OSS 항목과 동일한 네비게이션 affordance를 주는 편이 UX 일관성에 좋습니다.
다음과 같이 action을 추가하세요:
SettingItem( title = stringResource(R.string.login), onItemClick = { state.eventSink(SettingsUiEvent.OnLoginClick) }, + action = { + Icon( + imageVector = ImageVector.vectorResource(id = designR.drawable.ic_chevron_right), + contentDescription = "Right Chevron Icon", + tint = Color.Unspecified, + ) + }, )feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginUiState.kt (1)
19-22: Toast 메시지도 UiText로 일원화 권장다른 모듈(Library 등)과 일관성을 위해 String → UiText로 변경하세요.
- data class ShowToast( - val message: String, - private val key: String = UUID.randomUUID().toString(), - ) : LoginSideEffect + data class ShowToast( + val message: UiText, + private val key: String = UUID.randomUUID().toString(), + ) : LoginSideEffect추가로 Handle 쪽에서 asString(context) 변환을 적용해야 합니다.
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultBookRepository.kt (1)
20-31: 중복 로직(게스트/일반 검색) 경량화 제안두 메서드 모두 동일한 후처리(최근 검색 추가, toModel, Result 래핑)를 수행합니다. 공통 헬퍼로 축약하면 유지보수성이 좋아집니다.
다음과 같이 private 헬퍼를 추가하고 두 메서드를 단순화할 수 있습니다(클래스 내부에 추가).
// 클래스 내부에 추가 private suspend fun <R> searchBooks( query: String, start: Int, requester: suspend () -> R, mapper: (R) -> BookSearchModel, ) = runSuspendCatching { val result = mapper(requester()) bookRecentSearchDataSource.addRecentSearch(query) result }그리고 메서드들을 아래처럼 변경:
-override suspend fun searchBookAsGuest( - query: String, - start: Int, -) = runSuspendCatching { - val result = service.searchBookAsGuest( - query = query, - start = start, - ).toModel() - - bookRecentSearchDataSource.addRecentSearch(query) - result -} +override suspend fun searchBookAsGuest(query: String, start: Int) = + searchBooks( + query = query, + start = start, + requester = { service.searchBookAsGuest(query = query, start = start) }, + mapper = { it.toModel() }, + )-override suspend fun searchBook( - query: String, - start: Int, -) = runSuspendCatching { - val result = service.searchBook( - query = query, - start = start, - ).toModel() - - bookRecentSearchDataSource.addRecentSearch(query) - result -} +override suspend fun searchBook(query: String, start: Int) = + searchBooks( + query = query, + start = start, + requester = { service.searchBook(query = query, start = start) }, + mapper = { it.toModel() }, + )Also applies to: 33-44
core/network/src/main/kotlin/com/ninecraft/booket/core/network/service/ReedService.kt (1)
54-66: 게스트 검색 API 화이트리스트 등록 확인
TokenInterceptor의 noAuthEndpoints에 "api/v1/books/guest/search"가 포함된 것을 확인했습니다.
- searchBookAsGuest 및 searchBook 기본 파라미터 값이 중복됩니다: QueryType, SearchTarget, Sort, CoverSize 등을 상수/enum으로 중앙화해 오타·불일치 리스크를 줄이기를 권장합니다.
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt (1)
168-169: 게스트 모드 Error 분기에서 가이드 UI 노출: UX 적합. 중복 UI 추출 권장
- 게스트 모드에서 에러 화면 대신 가이드/등록 CTA 노출은 합리적입니다.
- 다만 Success와 Error(게스트) 분기에 동일한 헤더+EmptyBookCard 구간이 중복됩니다. 작은 컴포저블로 추출해 유지보수성을 높이세요.
예시:
@Composable private fun ReadingNowEmpty(onRegister: () -> Unit, modifier: Modifier = Modifier) { Column(modifier = modifier.verticalScroll(rememberScrollState())) { Spacer(Modifier.height(ReedTheme.spacing.spacing6)) Text( text = stringResource(R.string.home_content_label_reading_now), modifier = Modifier.padding(start = ReedTheme.spacing.spacing5), color = ReedTheme.colors.contentSecondary, style = ReedTheme.typography.headline2Medium, ) Spacer(Modifier.height(ReedTheme.spacing.spacing3)) EmptyBookCard( onBookRegisterClick = onRegister, modifier = Modifier.padding(horizontal = ReedTheme.spacing.spacing5), ) Spacer(Modifier.height(ReedTheme.spacing.spacing6)) } }Also applies to: 173-201
feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginUi.kt (3)
50-58: 닫기 상단바는 Scaffold의 topBar로 올리는 것이 적합현재 상단바가 콘텐츠 내부에 있어 인셋/스크롤/오버레이 동작이 일관적이지 않을 수 있습니다. Scaffold의 topBar 슬롯으로 이동을 권장합니다.
- ReedScaffold( - modifier = modifier.fillMaxSize(), - ) { innerPadding -> + ReedScaffold( + modifier = modifier.fillMaxSize(), + topBar = { + if (state.returnToScreen != null) { + ReedCloseTopAppBar( + onClose = { state.eventSink(LoginUiEvent.OnCloseButtonClick) }, + ) + } + }, + ) { innerPadding -> ... - Column { - if (state.returnToScreen != null) { - ReedCloseTopAppBar( - onClose = { - state.eventSink(LoginUiEvent.OnCloseButtonClick) - }, - ) - } + Column {Also applies to: 63-69
93-114: 로딩 중 중복 클릭 방지: 로그인 버튼 비활성화 필요isLoading 동안 카카오 로그인을 비활성화하면 중복 이벤트를 더 확실히 차단할 수 있습니다.
ReedButton( onClick = { state.eventSink(LoginUiEvent.OnKakaoLoginButtonClick) }, sizeStyle = largeButtonStyle, colorStyle = ReedButtonColorStyle.KAKAO, modifier = Modifier .fillMaxWidth() .padding( start = ReedTheme.spacing.spacing5, end = ReedTheme.spacing.spacing5, ), + enabled = !state.isLoading, text = stringResource(id = R.string.kakao_login),
116-124: 게스트 로그인 버튼도 로딩 중 비활성화 권장동일하게 로딩 중 클릭을 막아 UI 일관성을 유지하세요.
if (state.returnToScreen == null) { ReedTextButton( onClick = { state.eventSink(LoginUiEvent.OnGuestLoginButtonClick) }, text = stringResource(R.string.guest_login), sizeStyle = smallButtonStyle, colorStyle = ReedButtonColorStyle.TEXT, + enabled = !state.isLoading, ) }feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt (1)
95-97: 로그인/로그아웃 전환 시 홈 데이터 자동 갱신최초 1회 로딩만 있어 사용자 상태 전환(게스트↔로그인) 시 홈 카드가 갱신되지 않을 수 있습니다.
userState를 키로 한 효과를 추가하세요.RememberedEffect(true) { loadHomeContent() } + +// 사용자 상태 변화에 맞춰 재로딩 +LaunchedEffect(userState) { + loadHomeContent() +}추가로 필요한 import:
import androidx.compose.runtime.LaunchedEffectfeature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/Screens.kt (1)
19-19: returnToScreen 타입을 ReedScreen?으로 한정현재
Screen?은 외부 구현체도 허용해 직렬화 보장이 약합니다. 우리 앱 스크린으로 한정해 안전성을 높이는 것이 좋습니다.- data class LoginScreen(val returnToScreen: Screen? = null) : ReedScreen(name = ScreenNames.LOGIN) + data class LoginScreen(val returnToScreen: ReedScreen? = null) : ReedScreen(name = ScreenNames.LOGIN) ... - data class TermsAgreementScreen(val returnToScreen: Screen? = null) : ReedScreen(name = ScreenNames.TERMS_AGREEMENT) + data class TermsAgreementScreen(val returnToScreen: ReedScreen? = null) : ReedScreen(name = ScreenNames.TERMS_AGREEMENT)Also applies to: 28-28
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/mapper/ResponseToModel.kt (1)
71-82: 로그인 검색 매핑에도 동일한 키 일관성 적용 권장게스트와 동일한 기준을 사용해 키 생성 규칙을 통일하세요.
internal fun BookSummary.toModel(): BookSummaryModel { - return BookSummaryModel( - isbn13 = isbn13, - title = title.decodeHtmlEntities(), - author = author, - publisher = publisher, - coverImageUrl = coverImageUrl, - link = link, - userBookStatus = userBookStatus, - key = "$title-$isbn13", - ) + val decodedTitle = title.decodeHtmlEntities() + return BookSummaryModel( + isbn13 = isbn13, + title = decodedTitle, + author = author, + publisher = publisher, + coverImageUrl = coverImageUrl, + link = link, + userBookStatus = userBookStatus, + key = "$decodedTitle-$isbn13", + ) }feature/login/src/main/kotlin/com/ninecraft/booket/feature/termsagreement/TermsAgreementPresenter.kt (1)
78-83: popUntil 사용 시 백스택 미존재 케이스 처리
returnToScreen이 백스택에 없으면popUntil로 전체가 비워질 수 있습니다. 결정론적 복귀를 위해resetRoot(returnToScreen)사용을 권장합니다.- if (screen.returnToScreen != null) { - navigator.popUntil { it == screen.returnToScreen } - } else { - navigator.resetRoot(HomeScreen) - } + screen.returnToScreen?.let { target -> + navigator.resetRoot(target) + } ?: run { + navigator.resetRoot(HomeScreen) + }core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultAuthRepository.kt (1)
50-54: 불필요한 동일 상태 방출 최소화(distinctUntilChanged) 권장토큰이 갱신되어도 상태가 동일(예: 로그인 상태 유지)이면 재방출을 억제해 불필요한 리컴포지션을 줄일 수 있습니다.
import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.distinctUntilChanged ... override val autoLoginState = tokenDataSource.accessToken - .map { accessToken -> - if (accessToken.isBlank()) AutoLoginState.NOT_LOGGED_IN else AutoLoginState.LOGGED_IN - } + .map { accessToken -> + if (accessToken.isBlank()) AutoLoginState.NOT_LOGGED_IN else AutoLoginState.LOGGED_IN + } + .distinctUntilChanged() override val userState = tokenDataSource.accessToken - .map { accessToken -> - if (accessToken.isBlank()) UserState.Guest else UserState.LoggedIn - } + .map { accessToken -> + if (accessToken.isBlank()) UserState.Guest else UserState.LoggedIn + } + .distinctUntilChanged()Also applies to: 55-59
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/UiText.kt (2)
9-9: Compose 안정성 힌트를 위해 @immutable 부착 권장.Ui state로 자주 왕복되는 타입이라 안정성 힌트를 주면 불필요한 재구성을 줄이는 데 도움이 됩니다.
[Suggest fix]+@Immutable sealed class UiText {추가 import:
import androidx.compose.runtime.Immutable
8-8: 외부 링크 주석 대신 KDoc으로 의도 기록을 권장합니다.유튜브 링크는 사라질 수 있어 유지보수에 취약합니다. 간단한 KDoc으로 의도/배경을 남겨주세요.
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt (1)
65-66: LoginScreen()로의 resetRoot 호출이 중복됩니다.헬퍼로 추출해 중복 제거하면 가독성↑, 실수↓.
예: private fun navigateToLoginRoot() = navigator.resetRoot(LoginScreen())Also applies to: 77-78, 95-96, 107-108
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUi.kt (1)
74-77: modifier 전달 누락으로 상위에서 넘긴 제약이 소실됩니다.상위에서 받은 modifier를 그대로 넘기는 편이 안전합니다.
[Suggest fix]- BookSearchContent( - state = state, - modifier = Modifier, - ) + BookSearchContent( + state = state, + modifier = modifier, + )feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginPresenter.kt (1)
45-55: 중첩 coroutine 제거로 흐름 단순화 제안.navigateAfterLogin 내부에서 scope.launch를 다시 여는 중첩 구조입니다. 로그인 처리 코루틴 안에서 순차적으로 프로필 조회까지 수행하면 구조가 단순해집니다.
[Suggest fix]- fun navigateAfterLogin() { - scope.launch { - userRepository.getUserProfile() - .onSuccess { userProfile -> - if (userProfile.termsAgreed) { - if (screen.returnToScreen == null) { - navigator.resetRoot(HomeScreen) - } else { - navigator.popUntil { it == screen.returnToScreen } - } - } else { - navigator.resetRoot(TermsAgreementScreen()) - } - }.onFailure { exception -> - exception.message?.let { Logger.e(it) } - sideEffect = exception.message?.let { - LoginSideEffect.ShowToast(it) - } - } - } - } + suspend fun navigateAfterLogin() { + userRepository.getUserProfile() + .onSuccess { userProfile -> + if (userProfile.termsAgreed) { + if (screen.returnToScreen == null) { + navigator.resetRoot(HomeScreen) + } else { + navigator.popUntil { it == screen.returnToScreen } + } + } else { + navigator.resetRoot(TermsAgreementScreen()) + } + }.onFailure { exception -> + exception.message?.let { Logger.e(it) } + sideEffect = exception.message?.let { LoginSideEffect.ShowToast(it) } + } + }is LoginUiEvent.Login -> { scope.launch { try { isLoading = true - authRepository.login(event.accessToken) - .onSuccess { - navigateAfterLogin() - }.onFailure { exception -> + val loginResult = authRepository.login(event.accessToken) + if (loginResult.isSuccess) { + navigateAfterLogin() + } else { + val exception = loginResult.exceptionOrNull() exception.message?.let { Logger.e(it) } analyticsHelper.logEvent(EVENT_ERROR_LOGIN) - sideEffect = exception.message?.let { - LoginSideEffect.ShowToast(it) - } - } + sideEffect = exception?.message?.let { LoginSideEffect.ShowToast(it) } + } } finally { isLoading = false } } }Also applies to: 80-98
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt (2)
116-116: 하드코딩 문자열의 지역화 누락문자열(“알 수 없는 오류가 발생했습니다.”)은 리소스로 분리하거나
UiText기반으로 통일하는 것이 좋습니다. 현재FooterState.Error가String만 받는다면, 후속 PR에서UiText지원을 고려해 주세요.Also applies to: 120-120
160-161: 로그인 이동 방식 일관화 제안여기서는
resetRoot(LoginScreen()), 위(게스트 업서트)에서는redirectToLogin()을 사용합니다. 하나로 통일해 관리하세요(확장함수 권장).- navigator.resetRoot(LoginScreen()) + navigator.redirectToLogin()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (47)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/UiText.kt(1 hunks)core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/AuthRepository.kt(2 hunks)core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/BookRepository.kt(1 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/mapper/ResponseToModel.kt(2 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultAuthRepository.kt(2 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultBookRepository.kt(1 hunks)core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/button/ButtonColorStyle.kt(2 hunks)core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/button/ReedTextButton.kt(1 hunks)core/designsystem/src/main/res/values/strings.xml(1 hunks)core/model/src/main/kotlin/com/ninecraft/booket/core/model/UserState.kt(1 hunks)core/network/src/main/kotlin/com/ninecraft/booket/core/network/TokenInterceptor.kt(1 hunks)core/network/src/main/kotlin/com/ninecraft/booket/core/network/response/GuestBookSearchResponse.kt(1 hunks)core/network/src/main/kotlin/com/ninecraft/booket/core/network/service/ReedService.kt(2 hunks)feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailPresenter.kt(4 hunks)feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/record/RecordDetailPresenter.kt(2 hunks)feature/edit/src/main/kotlin/com/ninecraft/booket/feature/edit/record/RecordEditPresenter.kt(1 hunks)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt(5 hunks)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt(1 hunks)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUiState.kt(1 hunks)feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/HandleLibrarySideEffects.kt(1 hunks)feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.kt(7 hunks)feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryUi.kt(2 hunks)feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryUiState.kt(4 hunks)feature/library/src/main/res/values/strings.xml(1 hunks)feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginPresenter.kt(5 hunks)feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginUi.kt(3 hunks)feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginUiState.kt(2 hunks)feature/login/src/main/kotlin/com/ninecraft/booket/feature/termsagreement/TermsAgreementPresenter.kt(5 hunks)feature/login/src/main/res/values/strings.xml(1 hunks)feature/onboarding/src/main/kotlin/com/ninecraft/booket/feature/onboarding/OnboardingPresenter.kt(1 hunks)feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.kt(1 hunks)feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/ScreenNames.kt(1 hunks)feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/Screens.kt(1 hunks)feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/component/MainBottomBar.kt(0 hunks)feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/extensions/Navigator.kt(2 hunks)feature/search/build.gradle.kts(1 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt(11 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUi.kt(5 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUiState.kt(3 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/HandleBookSearchSideEffect.kt(1 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/library/LibrarySearchPresenter.kt(1 hunks)feature/search/src/main/res/values/strings.xml(1 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt(8 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUi.kt(1 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUiState.kt(2 hunks)feature/settings/src/main/res/values/strings.xml(1 hunks)feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt(2 hunks)
💤 Files with no reviewable changes (1)
- feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/component/MainBottomBar.kt
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-07-16T16:11:27.044Z
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#52
File: feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/screens/DelegateNavigator.kt:0-0
Timestamp: 2025-07-16T16:11:27.044Z
Learning: In the Reed-Android project's DelegateNavigator implementation, the pop(), peek(), and peekBackStack() methods should always use childNavigator without branching logic, as they operate on the currently active navigation stack. Only goTo() and resetRoot() methods need to route between childNavigator and rootNavigator based on screen type using isMainTabScreen() function.
Applied to files:
feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/extensions/Navigator.kt
📚 Learning: 2025-07-16T16:11:27.044Z
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#52
File: feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/screens/DelegateNavigator.kt:0-0
Timestamp: 2025-07-16T16:11:27.044Z
Learning: In the Reed-Android project's DelegateNavigator implementation, the pop(), peek(), and peekBackStack() methods should always use childNavigator without branching logic, as they operate on the currently active navigation stack. Only goTo() and resetRoot() methods need to route between childNavigator and rootNavigator based on screen type.
Applied to files:
feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/extensions/Navigator.kt
📚 Learning: 2025-07-31T23:30:37.547Z
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#88
File: feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/record/RecordDetailPresenter.kt:18-47
Timestamp: 2025-07-31T23:30:37.547Z
Learning: In Circuit architecture, presenters receive the Screen object directly as a constructor parameter (e.g., Assisted private val screen: RecordDetailScreen), and screen parameters are accessed through this screen object (e.g., screen.recordId). Screen parameters should not be added as separate constructor parameters.
Applied to files:
feature/login/src/main/kotlin/com/ninecraft/booket/feature/termsagreement/TermsAgreementPresenter.ktfeature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginPresenter.kt
📚 Learning: 2025-07-31T23:30:37.547Z
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#88
File: feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/record/RecordDetailPresenter.kt:18-47
Timestamp: 2025-07-31T23:30:37.547Z
Learning: In Circuit architecture, screen parameters are not manually passed to presenter constructors. The CircuitInject annotation automatically handles the injection of the entire Screen object, making screen parameters accessible without explicitly adding them to the constructor or factory methods.
Applied to files:
feature/login/src/main/kotlin/com/ninecraft/booket/feature/termsagreement/TermsAgreementPresenter.ktfeature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginPresenter.kt
📚 Learning: 2025-07-08T12:33:01.863Z
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#32
File: core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/button/ButtonColorStyle.kt:10-16
Timestamp: 2025-07-08T12:33:01.863Z
Learning: Reed Android 프로젝트에서 KAKAO 버튼 스타일은 디자이너가 pressed 상태 색상을 별도로 정의하지 않았기 때문에 pressed 상태에서도 동일한 Kakao 색상을 사용한다.
Applied to files:
core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/button/ButtonColorStyle.kt
🧬 Code graph analysis (8)
feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryUi.kt (2)
core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/button/ReedButton.kt (1)
ReedButton(34-114)core/ui/src/main/kotlin/com/ninecraft/booket/core/ui/component/ReedErrorUi.kt (1)
ReedErrorUi(22-48)
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultBookRepository.kt (1)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/RunSuspendCatching.kt (1)
runSuspendCatching(16-30)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUi.kt (1)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/component/SettingItem.kt (1)
SettingItem(15-54)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt (2)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/component/BookCard.kt (1)
EmptyBookCard(208-267)core/ui/src/main/kotlin/com/ninecraft/booket/core/ui/component/ReedErrorUi.kt (1)
ReedErrorUi(22-48)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUi.kt (1)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/HandleBookSearchSideEffect.kt (1)
HandleBookSearchSideEffects(8-28)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/HandleBookSearchSideEffect.kt (1)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/osslicenses/OssLicensesUiState.kt (1)
eventSink(6-8)
feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginUi.kt (3)
core/ui/src/main/kotlin/com/ninecraft/booket/core/ui/component/ReedTopAppBar.kt (1)
ReedCloseTopAppBar(103-118)core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/button/ReedButton.kt (1)
ReedButton(34-114)core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/button/ReedTextButton.kt (1)
ReedTextButton(25-74)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt (1)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/HandleException.kt (1)
handleException(19-46)
⏰ 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: ci-build
🔇 Additional comments (47)
feature/login/src/main/res/values/strings.xml (1)
8-8: 게스트 진입 라벨 추가 적절"회원가입 없이 둘러보기" 문구 명확하고 플로우 의도에 부합합니다. 다른 모듈과의 키 충돌도 없습니다.
core/network/src/main/kotlin/com/ninecraft/booket/core/network/TokenInterceptor.kt (1)
13-17: noAuthEndpoints 경로 일치 확인
서비스 정의(ReedService.kt:55)의@GET("api/v1/books/guest/search")와 정확히 일치합니다.core/model/src/main/kotlin/com/ninecraft/booket/core/model/UserState.kt (1)
3-6: 프로젝트가 Kotlin 버전 2.2.0을 사용 중이므로data object(Kotlin 1.9+) 기능 사용이 가능합니다.feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUiState.kt (1)
20-26: HomeUiState 호출부에 위치 기반 인수가 없어, isGuestMode 순서 변경은 불필요합니다core/network/src/main/kotlin/com/ninecraft/booket/core/network/response/GuestBookSearchResponse.kt (1)
6-30: 게스트 검색 응답 모델 정의 적절함서버 스키마에 충실한 직렬화 모델로 보이며, 필수 필드를 명확히 지정한 점 좋습니다. 매핑/도메인 변환과의 정합성만 유지해 주세요.
feature/onboarding/src/main/kotlin/com/ninecraft/booket/feature/onboarding/OnboardingPresenter.kt (1)
40-41: LoginScreen 인스턴스 인자화 전환 LGTM새 Screen 모델(LoginScreen())에 맞춘 변경으로 보이며 기능적 문제 없어 보입니다.
feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/record/RecordDetailPresenter.kt (1)
72-74: LoginScreen 인자 지원 및 복귀 로직 확인 필요
- LoginScreen이
screen파라미터를 받도록 생성자가 수정되었는지, 해당 인자를 바탕으로 로그인 후 RecordDetail로 복귀하는 로직이 구현되었는지 직접 검증 필요feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/HandleLibrarySideEffects.kt (1)
18-19: UiText 처리 일원화 적용 LGTM
UiText.asString(context)사용으로 메시지 처리 일관성과 지역화 대응이 개선되었습니다.core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/BookRepository.kt (1)
15-18: BookRepository 구현체 및 호출부 수동 점검 필요
스크립트 실행 결과 인터페이스를 구현한 클래스가 발견되지 않았습니다. Fake/Stub/Prod 구현체와 관련 테스트, 호출부에서searchBookAsGuest오버라이드 및 사용 여부를 수동으로 확인해 주세요.feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/ScreenNames.kt (1)
8-8: Deprecated alias 추가 불필요
코드베이스(.kt)에서ScreenNames.SEARCH및 이전 라우트 문자열(search_book_input) 사용처가 발견되지 않아 호환성 유지를 위한 deprecated alias 추가가 필요 없습니다.feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUiState.kt (1)
44-45: OnLoginClick 이벤트 정상 처리 확인
Presenter(SettingsPresenter.kt:198-201)에서navigator.redirectToLogin()으로 핸들링되고, UI(SettingsUi.kt:157)에서state.eventSink(SettingsUiEvent.OnLoginClick)으로 바인딩되어 있습니다.feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryUiState.kt (3)
30-31: 게스트 모드 상태 필드 추가 — 적절합니다UI 분기와 프리뷰 기본값 모두 안전합니다.
37-41: Toast 메시지의 UiText 전환과 키 추가 — 방향성 좋습니다UiText 일원화 및 랜덤 키로 재방출 보장 패턴이 적절합니다.
56-57: OnLoginClick Presenter 처리 경로 확인
LibraryPresenter에서 OnLoginClick 이벤트가navigator.redirectToLogin()호출로 로그인 화면으로 리다이렉트되도록 이미 연결되었습니다. 필요 시returnToScreen=LibraryScreen파라미터 추가 여부만 검토해 주세요.feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUi.kt (2)
153-174: 로그인/로그아웃 분기 자체는 타당게스트/로그인 상태별로 노출 항목을 분리한 접근은 요구사항과 부합합니다.
153-160: OnLoginClick 네비게이션 검증 완료
SettingsPresenter의OnLoginClick처리부에서navigator.redirectToLogin()이 호출되며, 해당 확장함수가 내부에서 현재 화면(peek())을LoginScreen(currentScreen)에 전달해returnToScreen을 지정하도록 구현되어 있습니다.feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryUi.kt (2)
21-24: 단순 import 추가UI 컴포넌트 사용을 위한 정상적인 import입니다.
171-177: 로그인 버튼 동작 — 적절SECONDARY 스타일과 중복 클릭 컷팅 기본값을 쓰는 구성은 안전합니다.
feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginUiState.kt (2)
11-11: returnToScreen 도입 — 요구사항 부합로그인 후 복귀 지점을 명시할 수 있어 플로우가 단순해집니다.
29-31: 게스트/닫기 이벤트 추가 — 적절UI에서의 게스트 진입 및 닫기 액션 매핑에 필요합니다.
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultBookRepository.kt (1)
20-31: 게스트 검색 도입: 기존 흐름과 일관성 확보(LGTM)
- runSuspendCatching 활용, toModel 매핑, 최근 검색 저장 흐름이 기존 searchBook과 동일합니다. 목적 달성 측면에서 적절합니다.
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUiState.kt (3)
35-36: 게스트 모드 상태 추가: UI/플로우 분기 기반 확보(LGTM)
- isGuestMode 플래그로 프레젠터/화면 분기 제어가 가능해졌습니다. PR 목적과 일치합니다.
51-51: InitSideEffect 처리 경로 정상 확인됨
HandleBookSearchSideEffect.kt에서 InitSideEffect 이벤트를 발행하고, BookSearchPresenter.kt의is BookSearchUiEvent.InitSideEffect -> sideEffect = null로직을 통해 사이드이펙트가 사라짐을 확인했습니다.
44-48: 해당 기능의 ShowToast 호출부 UiText 마이그레이션 완료
feature/search/book 내 모든 BookSearchSideEffect.ShowToast 호출이 UiText.DirectString/StringResource 로 적용돼 있어 추가 조치 불필요합니다.feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginUi.kt (1)
50-58: insets 처리 변경(LGTM)systemBarsPadding 제거 후 Scaffold의 innerPadding 사용으로 일관된 인셋 처리가 가능해졌습니다.
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt (3)
33-38: DI 변경 및 사용자 상태 도입 LGTM
AuthRepository주입과collectAsRetainedState(initial = UserState.Guest)로 게스트 모드 파생 상태를 만드는 접근은 적절합니다.isGuestMode계산도 명확합니다.Also applies to: 43-43
69-71: 게스트 분기 UX 확인책 등록 진입은 검색 화면으로 이동하도록 변경되었는데, 이 후 실제 “등록” 액션에서 게스트에게 로그인 유도 팝업이 뜨는지(BookSearch 플로우) 최종 확인 부탁드립니다.
103-108: 파생 UI 상태 계산 LGTM
isGuestMode = userState is UserState.Guest계산은 명확하고 UI 분기 용도로 적합합니다.feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/Screens.kt (1)
22-22: 잔존 참조 없음 확인
검색 결과SearchScreen및ScreenNames.SEARCH참조가 코드베이스에서 발견되지 않았습니다.core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/mapper/ResponseToModel.kt (1)
84-98: 게스트 검색 매핑 LGTM게스트 검색 응답을 도메인 모델로 일관되게 변환하고 있어 재사용성 좋습니다.
feature/login/src/main/kotlin/com/ninecraft/booket/feature/termsagreement/TermsAgreementPresenter.kt (1)
31-36: Circuit 규약 준수 LGTMPresenter가
@Assisted screen: TermsAgreementScreen을 직접 받도록 수정된 점이 Circuit 패턴과 일치합니다(이전 러닝과도 부합).Also applies to: 95-96
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultAuthRepository.kt (1)
60-63: 현재 상태 조회 구현 LGTM동일 로직을 동기 조회로 노출하는 부분은 단순명료합니다.
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsPresenter.kt (3)
49-49: userState 보존 수집 및 isGuestMode 파생 처리, 방향성 좋습니다.
198-202: 게스트 → 로그인 진입을 redirectToLogin()으로 처리한 점 좋습니다.현재 스크린을 returnToScreen으로 넘기는 확장 메서드 패턴과 일관적입니다.
68-72: SettingsSideEffect.ShowToast는 아직 String을 받습니다. UiText로 감쌀 필요가 없으며, 해당 제안은 적용되지 않습니다.Likely an incorrect or invalid review comment.
feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.kt (1)
118-126: 게스트의 ‘라이브러리 검색’ 가드 + 토스트/리다이렉트 처리 좋습니다.UX 의도(로그인 유도)와 일치합니다.
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUi.kt (1)
48-58: BookSearch로 리네이밍 및 사이드이펙트 배선(HandleBookSearchSideEffects) 적절합니다.feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginPresenter.kt (3)
28-33: Circuit 규약에 맞춘 Screen 주입 전환(Learnings와 일치) 좋습니다.Assisted screen: LoginScreen, Factory 시그니처 정합성 모두 OK.
100-106: 게스트 로그인 → HomeScreen로 이동 로직 적절합니다.게스트 세션 유지(토큰 비보유) 가정하에 다른 화면의 userState 파생 흐름과도 일관적입니다.
50-55: popUntil 비교 기준 확인 완료:navigator.redirectToLogin()이navigator.peek()로 현재 스크린 인스턴스를returnToScreen으로 전달하므로popUntil { it == screen.returnToScreen }비교가 동일 인스턴스를 기준으로 올바르게 동작합니다.feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt (7)
13-13: 임포트/리소스 alias 구성 적절게스트/로그인 분기와 UiText, ImpressionEffect, designR alias 도입으로 충돌 없이 명확합니다. 유지해 주세요.
Also applies to: 15-15, 19-19, 23-26, 33-33, 42-42
47-47: Auth 상태 수집/노출 방식 적절 — 초기값 Guest 처리 OK
collectAsRetainedState(initial = UserState.Guest)로 안정적입니다. 하위 UI가isGuestMode를 신뢰한다는 가정하에 문제 없어 보입니다.Also applies to: 63-63, 273-273
153-154: 서버/예외 메시지를 DirectString으로 전달 — 합리적서버/네트워크 오류 메시지는 지역화 범위를 벗어나므로
UiText.DirectString선택이 타당합니다.
169-171: InitSideEffect로 사이드이펙트 초기화 — 좋습니다중복 토스트/이펙트 재생 방지에 유효합니다.
220-221: ISBN 누락 토스트의 리소스화 — 좋습니다하드코딩 제거 및 다국어 대비가 되어 있습니다.
33-33: ImpressionEffect로 화면 노출 로깅 추가 — 적절화면 트래킹 포인트가 명확합니다. 다른 검색 관련 이벤트 네이밍(
SEARCH_BOOK_NO_RESULT)도 현재 규칙에 맞게 일관되어 보입니다.Also applies to: 258-261
279-279: @CircuitInject 스크린 매핑 교정 — OK
BookSearchScreen으로 일관화되어 DI/네비 체인이 명료합니다.
| vararg val args: Any, | ||
| ) : UiText() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
null 포맷 인자 허용을 위해 args 타입을 Any?로 변경하세요.
nullable 인자를 넘겨야 하는 리소스 포맷팅(use-case 꽤 있습니다)에서 현재 시그니처(vararg Any)는 null 전달이 불가능합니다.
[Suggest fix]
- vararg val args: Any,
+ vararg val args: Any?,📝 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.
| vararg val args: Any, | |
| ) : UiText() | |
| vararg val args: Any?, | |
| ) : UiText() |
🤖 Prompt for AI Agents
In core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/UiText.kt
around lines 14 to 15, the vararg parameter is currently declared as Any which
disallows passing null format arguments; change the signature to use vararg val
args: Any? so callers can pass nullable values, and update any related call
sites or overloads if necessary to accept and propagate nulls safely (e.g.,
ensure formatting logic handles null entries without throwing).
| import com.ninecraft.booket.core.model.AutoLoginState | ||
| import com.ninecraft.booket.core.model.UserState |
There was a problem hiding this comment.
중복 상태 API 노출(autoLoginState vs userState) — 정리 필요
새 UserState로 전환하면서 기존 autoLoginState가 남아 API가 중복/혼선을 일으킵니다. 제거하거나 @deprecated로 마이그레이션 경로를 명확히 하세요.
권장 수정(완전 제거):
-import com.ninecraft.booket.core.model.AutoLoginState
import com.ninecraft.booket.core.model.UserState
@@
- val autoLoginState: Flow<AutoLoginState>
-
val userState: Flow<UserState>
suspend fun getCurrentUserState(): UserState대안: 유지가 필요하다면
@Deprecated("Use userState instead", ReplaceWith("userState"))
val autoLoginState: Flow<AutoLoginState>Also applies to: 14-19
🤖 Prompt for AI Agents
In
core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/AuthRepository.kt
around lines 3-4 and also 14-19, the old autoLoginState API is left alongside
the new userState causing duplicate/confusing state exposure; either remove
autoLoginState entirely or mark it deprecated to provide a clear migration path.
If removing: delete the autoLoginState property and any references, keeping only
userState. If retaining for compatibility: annotate the autoLoginState property
with @Deprecated("Use userState instead", ReplaceWith("userState")) and ensure
its implementation delegates to userState so behavior stays consistent; update
imports and usages accordingly.
| val userState: Flow<UserState> | ||
|
|
||
| suspend fun getCurrentUserState(): UserState | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
구현/호출부 전반 검증 요청
DefaultAuthRepository 구현과 호출부가 전부 userState/getCurrentUserState로 이행되었는지 확인해 주세요.
🏁 Script executed:
#!/bin/bash
# 남은 사용처 확인
rg -nC2 '\bautoLoginState\b'
# 구현 존재 확인
rg -nC3 'class\s+DefaultAuthRepository\b|override\s+val\s+userState|override\s+suspend\s+fun\s+getCurrentUserState'Length of output: 6579
SplashPresenter에서 authRepository.autoLoginState 사용을 모두 제거하고 userState/getCurrentUserState로 교체하세요.
– 파일: feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt (라인 48, 80, 130)
🤖 Prompt for AI Agents
In
core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/AuthRepository.kt
around lines 16-19 and in
feature/splash/src/main/kotlin/com/ninecraft/booket/splash/SplashPresenter.kt at
lines 48, 80, 130, replace all uses of authRepository.autoLoginState with the
new API: use authRepository.userState (Flow<UserState>) when you need an
observable stream (collect it or convert to a StateFlow/LiveData in the
presenter scope), and call suspend fun authRepository.getCurrentUserState() from
a coroutine when you need the current snapshot. Update the three call sites to
either collect userState with lifecycle/coroutine scope (e.g.,
presenterScope.launch { authRepository.userState.collect { ... } }) or call
getCurrentUserState() inside a coroutine and handle the returned UserState, and
remove any references/imports to autoLoginState.
There was a problem hiding this comment.
자동 로그인 여부와, 로그인 여부는 다름
There was a problem hiding this comment.
@easyhooon 맞습니다. 자동 로그인 설정과 현재 로그인 상태는 별개의 개념이네요.
autoLoginState는 사용자가 자동 로그인을 설정했는지의 여부를 나타내고, userState는 현재 사용자가 게스트인지 로그인된 상태인지를 나타내는 것으로 이해됩니다. 두 상태는 독립적으로 관리되어야 하므로 이전 제안을 철회하겠습니다.
✏️ Learnings added
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#174
File: core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/AuthRepository.kt:16-19
Timestamp: 2025-08-28T12:26:12.459Z
Learning: In AuthRepository, autoLoginState and userState serve different purposes and should not be conflated:
- autoLoginState: tracks whether automatic login is enabled (user preference/setting)
- userState: tracks current authentication state (Guest vs LoggedIn)
These are orthogonal concepts that need to be maintained separately.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| internal fun GuestBookSummary.toModel(): BookSummaryModel { | ||
| return BookSummaryModel( | ||
| isbn13 = isbn13, | ||
| title = title.decodeHtmlEntities(), | ||
| author = author, | ||
| publisher = publisher, | ||
| coverImageUrl = coverImageUrl, | ||
| link = link, | ||
| userBookStatus = "BEFORE_REGISTRATION", | ||
| key = "$title-$isbn13", | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
키 계산 시 디코딩 불일치로 동일 도서 중복 키 가능성
title을 디코딩하여 표시하면서 key는 원본 title로 생성되어, HTML 엔티티 포함 타이틀에서 키 불일치가 발생할 수 있습니다. 디코딩된 타이틀 기반으로 키를 생성해 로그인/게스트 간 키 일관성을 확보하세요.
internal fun GuestBookSummary.toModel(): BookSummaryModel {
- return BookSummaryModel(
- isbn13 = isbn13,
- title = title.decodeHtmlEntities(),
- author = author,
- publisher = publisher,
- coverImageUrl = coverImageUrl,
- link = link,
- userBookStatus = "BEFORE_REGISTRATION",
- key = "$title-$isbn13",
- )
+ val decodedTitle = title.decodeHtmlEntities()
+ return BookSummaryModel(
+ isbn13 = isbn13,
+ title = decodedTitle,
+ author = author,
+ publisher = publisher,
+ coverImageUrl = coverImageUrl,
+ link = link,
+ userBookStatus = "BEFORE_REGISTRATION",
+ key = "$decodedTitle-$isbn13",
+ )
}또한 상수 문자열 "BEFORE_REGISTRATION"는 상수로 추출해 오타를 방지하세요(예: private const val BEFORE_REGISTRATION = "BEFORE_REGISTRATION").
📝 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.
| internal fun GuestBookSummary.toModel(): BookSummaryModel { | |
| return BookSummaryModel( | |
| isbn13 = isbn13, | |
| title = title.decodeHtmlEntities(), | |
| author = author, | |
| publisher = publisher, | |
| coverImageUrl = coverImageUrl, | |
| link = link, | |
| userBookStatus = "BEFORE_REGISTRATION", | |
| key = "$title-$isbn13", | |
| ) | |
| } | |
| internal fun GuestBookSummary.toModel(): BookSummaryModel { | |
| val decodedTitle = title.decodeHtmlEntities() | |
| return BookSummaryModel( | |
| isbn13 = isbn13, | |
| title = decodedTitle, | |
| author = author, | |
| publisher = publisher, | |
| coverImageUrl = coverImageUrl, | |
| link = link, | |
| userBookStatus = "BEFORE_REGISTRATION", | |
| key = "$decodedTitle-$isbn13", | |
| ) | |
| } |
🤖 Prompt for AI Agents
In
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/mapper/ResponseToModel.kt
around lines 100 to 111, the model uses decoded title for display but builds the
key from the raw title and hardcodes the status string; change key generation to
use the decoded title (so key = "$decodedTitle-$isbn13") to avoid duplicate keys
for titles with HTML entities, and extract the status string into a private
const val BEFORE_REGISTRATION = "BEFORE_REGISTRATION" (then reference
BEFORE_REGISTRATION for userBookStatus) to prevent typos and centralize the
constant.
| override suspend fun searchBookAsGuest( | ||
| query: String, | ||
| start: Int, | ||
| ) = runSuspendCatching { | ||
| val result = service.searchBookAsGuest( | ||
| query = query, | ||
| start = start, | ||
| ).toModel() | ||
|
|
||
| bookRecentSearchDataSource.addRecentSearch(query) | ||
| result | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
runSuspendCatching가 suspend 블록을 지원하는지 확인 필요
현재 블록 내부에서 Retrofit suspend API를 호출합니다. runSuspendCatching가 suspend 블록 시그니처(suspend () -> T)를 지원하지 않으면 컴파일/런타임 이슈가 발생합니다. 아래 스크립트로 선언을 확인해 주세요. 필요한 경우 runSuspendCatching를 suspend inline으로 변경해야 합니다.
필요 시 변경 예시(참고: 선언부 파일에서 적용):
-@OptIn(ExperimentalContracts::class)
-@Suppress("WRONG_INVOCATION_KIND", "TooGenericExceptionCaught")
-inline fun <T> runSuspendCatching(block: () -> T): Result<T> {
+@OptIn(ExperimentalContracts::class)
+@Suppress("TooGenericExceptionCaught")
+suspend inline fun <T> runSuspendCatching(
+ crossinline block: suspend () -> T
+): Result<T> {
contract { callsInPlace(block, InvocationKind.EXACTLY_ONCE) }
return try {
- Result.success(block())
+ Result.success(block())
} catch (rethrown: CancellationException) {
throw rethrown
} catch (exception: Throwable) {
Result.failure(exception)
}
}🏁 Script executed:
#!/bin/bash
# runSuspendCatching 선언부가 suspend 블록을 받는지 확인
rg -nP --glob 'core/common/**' 'runSuspendCatching\s*\(' -C3
# 기대: 선언부에 'suspend' 키워드와 'block: suspend \(\) ->' 형태가 포함되어야 함Length of output: 1058
runSuspendCatching에 suspend 블록 시그니처 지원 추가 필요
현재 선언( core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/RunSuspendCatching.kt )이 non-suspend 블록만 받도록 되어 있어, Retrofit의 suspend API 호출 시 컴파일 오류가 발생합니다. 선언부를 아래와 같이 수정하세요:
- @OptIn(ExperimentalContracts::class)
- @Suppress("WRONG_INVOCATION_KIND", "TooGenericExceptionCaught")
- inline fun <T> runSuspendCatching(block: () -> T): Result<T> {
+ @OptIn(ExperimentalContracts::class)
+ @Suppress("TooGenericExceptionCaught")
+ suspend inline fun <T> runSuspendCatching(
+ crossinline block: suspend () -> T
+ ): Result<T> {
contract { callsInPlace(block, InvocationKind.EXACTLY_ONCE) }
return try {
Result.success(block())🤖 Prompt for AI Agents
In
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultBookRepository.kt
around lines 20 to 31, the call to runSuspendCatching currently fails because
the utility only accepts non‑suspend blocks; update the declaration in
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/RunSuspendCatching.kt
so it accepts a suspend lambda (e.g., change the block parameter type to suspend
() -> T and make the wrapper function suspend as needed), preserving error
handling semantics and return typing so Retrofit suspend calls compile.
| if (userState is UserState.Guest) { | ||
| repository.searchBookAsGuest(query = query, start = startIndex) | ||
| } else { | ||
| repository.searchBook(query = query, start = startIndex) | ||
| } | ||
| .onSuccess { result -> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
동시 검색 요청 경쟁 상태 가능 — 이전 잡 취소 추가 권장
빠른 스크롤/재검색 시 이전 검색 코루틴이 살아 있어 목록/풋터 상태가 뒤섞일 수 있습니다. 마지막 검색만 유효하도록 취소 토큰을 관리해 주세요.
다음과 같이 검색 잡을 보관/취소하세요:
+import kotlinx.coroutines.Job
...
override fun present(): BookSearchUiState {
val scope = rememberCoroutineScope()
+ var searchJob by rememberRetained { mutableStateOf<Job?>(null) }
...
- fun searchBooks(query: String, startIndex: Int = START_INDEX) {
- scope.launch {
+ fun searchBooks(query: String, startIndex: Int = START_INDEX) {
+ searchJob?.cancel()
+ searchJob = scope.launch {
if (startIndex == START_INDEX) {
uiState = UiState.Loading
} else {
footerState = FooterState.Loading
}
...
}
}📝 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.
| if (userState is UserState.Guest) { | |
| repository.searchBookAsGuest(query = query, start = startIndex) | |
| } else { | |
| repository.searchBook(query = query, start = startIndex) | |
| } | |
| .onSuccess { result -> | |
| ++ b/feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt | |
| @@ | |
| import kotlinx.coroutines.Job | |
| ... | |
| override fun present(): BookSearchUiState { | |
| val scope = rememberCoroutineScope() | |
| var searchJob by rememberRetained { mutableStateOf<Job?>(null) } | |
| fun searchBooks(query: String, startIndex: Int = START_INDEX) { | |
| // cancel any in-flight search so only the latest result is applied | |
| searchJob?.cancel() | |
| searchJob = scope.launch { | |
| if (startIndex == START_INDEX) { | |
| uiState = UiState.Loading | |
| } else { | |
| footerState = FooterState.Loading | |
| } | |
| val call = if (userState is UserState.Guest) { | |
| repository.searchBookAsGuest(query = query, start = startIndex) | |
| } else { | |
| repository.searchBook(query = query, start = startIndex) | |
| } | |
| call | |
| .onSuccess { result -> | |
| // handle success… | |
| } | |
| .onFailure { error -> | |
| // handle error… | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt
around lines 87 to 92, concurrent search coroutines can race and interleave
results; keep a reference to the active search Job (e.g., a private var
searchJob: Job?), cancel it before starting a new search, then assign the newly
launched coroutine to that variable so only the latest search runs; ensure
cancellation occurs on the same CoroutineScope used to launch the job and handle
cancellation exceptions if needed.
| if (userState is UserState.Guest) { | ||
| sideEffect = BookSearchSideEffect.ShowToast(UiText.StringResource(designR.string.login_required)) | ||
| navigator.redirectToLogin() | ||
| return@launch | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
게스트 업서트 시 바텀시트 닫기 누락 및 내비 흐름 일관화
게스트가 등록 버튼을 누르면 로그인으로 전환하지만 바텀시트/선택 상태는 유지됩니다. 닫고 상태를 초기화한 뒤 이동하세요.
if (userState is UserState.Guest) {
sideEffect = BookSearchSideEffect.ShowToast(UiText.StringResource(designR.string.login_required))
- navigator.redirectToLogin()
+ isBookRegisterBottomSheetVisible = false
+ selectedBookIsbn = ""
+ selectedBookStatus = null
+ navigator.redirectToLogin()
return@launch
}📝 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.
| if (userState is UserState.Guest) { | |
| sideEffect = BookSearchSideEffect.ShowToast(UiText.StringResource(designR.string.login_required)) | |
| navigator.redirectToLogin() | |
| return@launch | |
| } | |
| if (userState is UserState.Guest) { | |
| sideEffect = BookSearchSideEffect.ShowToast(UiText.StringResource(designR.string.login_required)) | |
| isBookRegisterBottomSheetVisible = false | |
| selectedBookIsbn = "" | |
| selectedBookStatus = null | |
| navigator.redirectToLogin() | |
| return@launch | |
| } |
🤖 Prompt for AI Agents
In
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt
around lines 128-133, the guest path shows a toast and navigates to login but
does not close the bottom sheet or clear selection state; before calling
navigator.redirectToLogin() emit/dispatch a side effect to close the bottom
sheet (e.g., BookSearchSideEffect.CloseBottomSheet or equivalent) and reset any
selection-related state (clear selectedBook/selectedItem flags or update uiState
to initial selection values), then call navigator.redirectToLogin() and return.
There was a problem hiding this comment.
의도한거임 돌아왔을때 그대로 유지되어있으라고
There was a problem hiding this comment.
@easyhooon 이해했습니다! 로그인 후 돌아왔을 때 사용자가 이전 상태를 그대로 유지할 수 있도록 하는 UX 설계군요. 맥락을 보존하는 좋은 접근 방식입니다.
✏️ Learnings added
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#174
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt:128-133
Timestamp: 2025-08-28T12:25:54.013Z
Learning: In BookSearchPresenter.kt, when a guest user tries to register a book and is redirected to login, the bottom sheet (isBookRegisterBottomSheetVisible) and selection state (selectedBookIsbn, selectedBookStatus) are intentionally kept open/preserved so that when the user returns from login, they can continue from where they left off without losing context.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| navigator.resetRoot(LoginScreen()) | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
로그인 후 라이브러리 검색 화면으로 복귀 보장
이 화면은 screen 주입이 없으므로, 싱글톤 LibrarySearchScreen을 returnToScreen으로 넘겨 복귀성을 확보하세요.
- navigator.resetRoot(LoginScreen())
+ navigator.resetRoot(LoginScreen(returnToScreen = LibrarySearchScreen))📝 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.
| navigator.resetRoot(LoginScreen()) | |
| }, | |
| navigator.resetRoot(LoginScreen(returnToScreen = LibrarySearchScreen)) |
🤖 Prompt for AI Agents
In
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/library/LibrarySearchPresenter.kt
around lines 103-104, the navigation to the login screen calls
navigator.resetRoot(LoginScreen()) without supplying a return target; update the
navigation so the singleton LibrarySearchScreen instance is provided as the
returnToScreen (e.g., pass LibrarySearchScreen singleton into the
LoginScreen/navigation call) so that after login the app reliably returns to the
library search screen.
| <string name="settings_optional_update_title">최신 버전이 출시되었습니다.</string> | ||
| <string name="settings_optional_update_message">최적의 사용 환경을 위해 업데이트해주세요.</string> | ||
| <string name="settings_optional_update_button_text">업데이트하기</string> | ||
| <string name="login">로그인</string> |
There was a problem hiding this comment.
리소스 키 충돌: "login" 중복 정의(모듈 간 값 불일치)
feature/library 모듈에도 같은 키 이름("login")이 있으며 값이 "로그인 하기"로 상이합니다. 앱 머지 시 예기치 않은 문자열이 노출될 수 있습니다. 모듈 프리픽스 부여로 충돌 제거를 권장합니다(예: settings_login).
권장 변경안:
- <string name="login">로그인</string>
+ <string name="settings_login">로그인</string>또한 해당 참조 코드(예: SettingsUi.kt)의 R.string.login → R.string.settings_login로 업데이트가 필요합니다.
🤖 Prompt for AI Agents
In feature/settings/src/main/res/values/strings.xml around line 21, the string
resource key "login" conflicts with the same key in feature/library; rename the
resource to a module-prefixed key (e.g., change key to "settings_login") and
keep the value "로그인" (or adjust if needed), then update all code references
(e.g., in SettingsUi.kt) from R.string.login to R.string.settings_login so the
module uses the new namespaced resource and avoids merge collisions.
There was a problem hiding this comment.
🧩 Analysis chain
@easyhooon Perfect fix! 👍
You correctly resolved the resource conflict by:
- Removing the duplicate
"login"resource from settings module - Updating the code reference in
SettingsUi.ktto use the existingR.string.settings_login
This eliminates the potential merge conflict between modules while maintaining the same functionality. The solution is clean and follows proper resource naming conventions.
/| /|
( :v: )
|(_)|
| |
🏁 Script executed:
#!/bin/bash
# Check the specific commit to see what changes were made
git show 74f4d915850de0c4a732af8d5081a74a786dc135
Length of output: 7214
@easyhooon Perfect! 👍
You've addressed the resource conflict issue correctly by:
- Resource conflict fix: Renamed
"login"to"settings_login"in the settings module - Code reference update: Updated
SettingsUi.ktto useR.string.settings_login - Bonus UI improvement: Refactored
LoginUi.ktto eliminate code duplication by moving theReedButtonoutside the conditional branches and simplifying the spacing logic
The resource naming now follows proper module prefixing conventions, eliminating the potential merge conflict while maintaining functionality. The UI refactoring also makes the code cleaner and more maintainable.
Great work implementing all the feedback! 🎉
/| /|
( :v: )
|(_)|
| |
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginUi.kt (1)
50-58: ReedScaffold의 topBar 슬롯으로 CloseTopAppBar 이동
content 내부에 TopAppBar를 두고 innerPadding을 적용하면 상단 인셋/레이아웃 계산이 어긋날 수 있습니다. ReedScaffold의topBar파라미터로ReedCloseTopAppBar를 옮기고, content 블록 내의 기존 TopAppBar 코드를 제거하세요.
🧹 Nitpick comments (5)
core/model/src/main/kotlin/com/ninecraft/booket/core/model/BookSearchModel.kt (1)
34-36: 문자열 기반 상태 비교는 타입 세이프티에 취약합니다.네트워크/매퍼에서 오타나 신규 값 추가 시 런타임 버그로 이어집니다. 최소한 enum/inline value class로 모델 계층을 타입 세이프하게 만드는 것을 권장합니다.
예시(선언만, 점진 도입 가능):
enum class BookRegistrationStatus { BEFORE_REGISTRATION, AFTER_REGISTRATION } data class BookSummaryModel( // ... val registrationStatus: BookRegistrationStatus = BookRegistrationStatus.BEFORE_REGISTRATION, ) { val isRegistered: Boolean get() = registrationStatus == BookRegistrationStatus.AFTER_REGISTRATION }또는 현재 구조 유지 시 mapper에서 허용되지 않은 값은 반드시 BEFORE_REGISTRATION로 강제 매핑해 주세요(unknown-safe).
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUiState.kt (1)
51-51: 이벤트 명 확장성/의미 명료화 제안: InitSideEffect → ConsumeSideEffect.의도는 “초기화”가 아니라 “소비/정리”로 보입니다. 동일 의미의 사용처가 여럿이라면 용어 정합성을 위해 리네이밍을 고려해 주세요.
feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginUi.kt (3)
63-69: 닫기 아이콘 접근성/현지화 개선 제안
ReedCloseTopAppBar내부에서 endIcon의 description이 "Close"로 하드코딩되어 있습니다(코어 컴포넌트 참조). 현지화/스크린리더 대응을 위해 문자열 리소스로 교체하는 것을 권장합니다.코어 컴포넌트 예시 변경안:
--- core/ui/.../ReedTopAppBar.kt +import androidx.compose.ui.res.stringResource @@ ReedTopAppBar( @@ - endIconRes = R.drawable.ic_close, - endIconDescription = "Close", + endIconRes = R.drawable.ic_close, + endIconDescription = stringResource(R.string.cd_close), endIconOnClick = onClose, )strings.xml:
<string name="cd_close">닫기</string>
77-81: 미세 dp 값 정규화(디자인 토큰/정수 dp 권장)
67.14.dp는 실제 렌더링 이점이 없고 디바이스 스케일에 따라 애매하게 보일 수 있습니다. 디자인 토큰 또는 정수 dp로 반올림하세요.- modifier = Modifier.height(67.14.dp), + modifier = Modifier.height(68.dp),
159-169: 프리뷰 케이스 보강 제안TopBar 노출 상태(returnToScreen != null)도 별도 Preview로 추가하면 QA/디자인 확인이 빨라집니다.
원하시면 Preview 스텁(Screen 모형)까지 포함한 샘플을 드리겠습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
core/model/src/main/kotlin/com/ninecraft/booket/core/model/BookSearchModel.kt(1 hunks)feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginUi.kt(3 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUi.kt(6 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUiState.kt(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginUi.kt (3)
core/ui/src/main/kotlin/com/ninecraft/booket/core/ui/component/ReedTopAppBar.kt (1)
ReedCloseTopAppBar(103-118)core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/button/ReedButton.kt (1)
ReedButton(34-114)core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/button/ReedTextButton.kt (1)
ReedTextButton(25-74)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUi.kt (1)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/HandleBookSearchSideEffect.kt (1)
HandleBookSearchSideEffects(8-28)
⏰ 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: ci-build
🔇 Additional comments (7)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUiState.kt (1)
45-47: ShowToast 메시지 UiText 이행 완료 확인
BookSearch 및 Library 모듈의 모든 ShowToast 호출에서 원시 문자열 없이 UiText(StringResource/DirectString)만 사용 중입니다.feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUi.kt (4)
48-50: 화면 DI 및 컴포저블 리네이밍 일관성 확보, 좋습니다.BookSearchScreen과 BookSearchUi로 명확해져 가독성이 개선되었습니다.
54-57: 사이드이펙트 소비 루프의 재진입/중복 실행 안전성 확인 필요.RememberedEffect 내부에서 sideEffect != null 시 즉시 InitSideEffect를 발행하는 패턴은 상태 동기화 타이밍에 따라 재발행이 일어날 수 있습니다(특히 동일 토스트 연속 발생 시).
- Presenter에서 InitSideEffect(혹은 Clear/Consume)가 idempotent하게 처리되는지 확인해 주세요.
- 가능하면 동일 키 재발행 방지를 위해 key를 메시지/타임스탬프 기반으로 관리하거나, consume 이후에만 새로운 sideEffect를 발행하도록 보장해 주세요.
84-87: Content 분리로 테스트/미리보기 용이성 향상, OK.
225-226: enabled = !isRegistered — 게스트 요구사항과의 정합성 재검토 요청.PR 목적상 “게스트도 검색 결과 접근 가능, 등록 시에만 로그인 유도”라면, 아이템 자체 클릭/상호작용은 게스트에서도 가능해야 합니다. enabled가 아이템 전체 비활성화를 의미한다면 게스트 UX를 방해할 수 있습니다.
옵션:
- 게스트는 항상 아이템 클릭 가능, 등록 CTA만 게이트:
- enabled = !state.books[index].isRegistered, + enabled = state.isGuestMode || !state.books[index].isRegistered,
- 또는 BookItem 내부에서 register 버튼만 state.isGuestMode로 분기하고, 아이템 루트는 항상 enabled.
위 결정은 Presenter의 로직(게스트일 때 등록 시 로그인 다이얼로그로 전환)과 반드시 일치해야 합니다.
feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginUi.kt (2)
27-33: ReedTextButton/CloseTopAppBar 도입을 위한 import 추가 적절새 컴포넌트 도입 대비된 import 구성이 명확합니다.
93-99: 게스트 진입 가드 조건 확인 요청
returnToScreen != null일 때 게스트 버튼을 숨기는 정책이 요구사항과 일치하는지 확인 필요합니다(예: 검색→도서 등록 진입 시 로그인 필수인지, 최초 진입/리다이렉트 케이스별 기대 동작).Also applies to: 124-129
| val isRegistered: Boolean | ||
| get() = userBookStatus != BEFORE_REGISTRATION |
There was a problem hiding this comment.
기본값으로 인해 isRegistered가 true로 잘못 계산됩니다.
현재 userBookStatus의 기본값이 빈 문자열("")이라 isRegistered가 기본적으로 true가 됩니다. 기본 상태가 미등록임을 의도했다면 userBookStatus의 기본값을 BEFORE_REGISTRATION으로 지정해야 합니다.
다음과 같이 기본값을 조정해 주세요(선택 영역 밖 변경이므로 참고용 스니펫):
data class BookSummaryModel(
// ...
- val userBookStatus: String = "",
+ val userBookStatus: String = BEFORE_REGISTRATION,
// ...
)🤖 Prompt for AI Agents
In core/model/src/main/kotlin/com/ninecraft/booket/core/model/BookSearchModel.kt
around lines 31-32, isRegistered is computed from userBookStatus which currently
defaults to an empty string causing isRegistered to be true by default; change
the default value of userBookStatus in the data class (e.g., BookSummaryModel)
to BEFORE_REGISTRATION instead of "" so that isRegistered evaluates correctly,
and update any constructors/tests/instantiations that rely on the old default to
pass the new default value if needed.
| Column( | ||
| modifier = Modifier.fillMaxWidth(), | ||
| horizontalAlignment = Alignment.CenterHorizontally, | ||
| ) { | ||
| if (state.returnToScreen == null) { | ||
| ReedButton( | ||
| onClick = { | ||
| state.eventSink(LoginUiEvent.OnKakaoLoginButtonClick) | ||
| }, | ||
| sizeStyle = largeButtonStyle, | ||
| colorStyle = ReedButtonColorStyle.KAKAO, | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .padding( | ||
| start = ReedTheme.spacing.spacing5, | ||
| end = ReedTheme.spacing.spacing5, | ||
| ), | ||
| text = stringResource(id = R.string.kakao_login), | ||
| leadingIcon = { | ||
| Icon( | ||
| imageVector = ImageVector.vectorResource(id = R.drawable.ic_kakao), | ||
| contentDescription = "Kakao Icon", | ||
| tint = Color.Unspecified, | ||
| ) | ||
| }, | ||
| ) | ||
| Spacer(modifier = Modifier.height(ReedTheme.spacing.spacing2)) | ||
| ReedTextButton( | ||
| onClick = { | ||
| state.eventSink(LoginUiEvent.OnGuestLoginButtonClick) | ||
| }, | ||
| text = stringResource(R.string.guest_login), | ||
| sizeStyle = smallButtonStyle, | ||
| colorStyle = ReedButtonColorStyle.TEXT, | ||
| ) | ||
| }, | ||
| ) | ||
| } else { | ||
| ReedButton( | ||
| onClick = { | ||
| state.eventSink(LoginUiEvent.OnKakaoLoginButtonClick) | ||
| }, | ||
| sizeStyle = largeButtonStyle, | ||
| colorStyle = ReedButtonColorStyle.KAKAO, | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .padding( | ||
| start = ReedTheme.spacing.spacing5, | ||
| end = ReedTheme.spacing.spacing5, | ||
| ), | ||
| text = stringResource(id = R.string.kakao_login), | ||
| leadingIcon = { | ||
| Icon( | ||
| imageVector = ImageVector.vectorResource(id = R.drawable.ic_kakao), | ||
| contentDescription = "Kakao Icon", | ||
| tint = Color.Unspecified, | ||
| ) | ||
| }, | ||
| ) | ||
| Spacer(modifier = Modifier.height(ReedTheme.spacing.spacing8)) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
중복 제거 + 아이콘 contentDescription 정리(null) + 간격 계산 통합
- 동일한 카카오 로그인 버튼이 if/else에 중복됨 → 단일화.
- 버튼 텍스트가 있으므로 아이콘의 contentDescription은 null로 두어 중복 낭독을 피하는 것이 접근성 측면에서 일반적.
- 게스트 버튼 유무에 따라 Spacer 높이만 분기.
- Column(
+ val requiresLogin = state.returnToScreen != null
+ Column(
modifier = Modifier.fillMaxWidth(),
horizontalAlignment = Alignment.CenterHorizontally,
) {
- if (state.returnToScreen == null) {
- ReedButton(
- onClick = {
- state.eventSink(LoginUiEvent.OnKakaoLoginButtonClick)
- },
- sizeStyle = largeButtonStyle,
- colorStyle = ReedButtonColorStyle.KAKAO,
- modifier = Modifier
- .fillMaxWidth()
- .padding(
- start = ReedTheme.spacing.spacing5,
- end = ReedTheme.spacing.spacing5,
- ),
- text = stringResource(id = R.string.kakao_login),
- leadingIcon = {
- Icon(
- imageVector = ImageVector.vectorResource(id = R.drawable.ic_kakao),
- contentDescription = "Kakao Icon",
- tint = Color.Unspecified,
- )
- },
- )
- Spacer(modifier = Modifier.height(ReedTheme.spacing.spacing2))
- ReedTextButton(
- onClick = {
- state.eventSink(LoginUiEvent.OnGuestLoginButtonClick)
- },
- text = stringResource(R.string.guest_login),
- sizeStyle = smallButtonStyle,
- colorStyle = ReedButtonColorStyle.TEXT,
- )
- } else {
- ReedButton(
- onClick = {
- state.eventSink(LoginUiEvent.OnKakaoLoginButtonClick)
- },
- sizeStyle = largeButtonStyle,
- colorStyle = ReedButtonColorStyle.KAKAO,
- modifier = Modifier
- .fillMaxWidth()
- .padding(
- start = ReedTheme.spacing.spacing5,
- end = ReedTheme.spacing.spacing5,
- ),
- text = stringResource(id = R.string.kakao_login),
- leadingIcon = {
- Icon(
- imageVector = ImageVector.vectorResource(id = R.drawable.ic_kakao),
- contentDescription = "Kakao Icon",
- tint = Color.Unspecified,
- )
- },
- )
- Spacer(modifier = Modifier.height(ReedTheme.spacing.spacing8))
- }
+ ReedButton(
+ onClick = {
+ state.eventSink(LoginUiEvent.OnKakaoLoginButtonClick)
+ },
+ sizeStyle = largeButtonStyle,
+ colorStyle = ReedButtonColorStyle.KAKAO,
+ modifier = Modifier
+ .fillMaxWidth()
+ .padding(
+ start = ReedTheme.spacing.spacing5,
+ end = ReedTheme.spacing.spacing5,
+ ),
+ text = stringResource(id = R.string.kakao_login),
+ leadingIcon = {
+ Icon(
+ imageVector = ImageVector.vectorResource(id = R.drawable.ic_kakao),
+ contentDescription = null,
+ tint = Color.Unspecified,
+ )
+ },
+ )
+ Spacer(
+ modifier = Modifier.height(
+ if (requiresLogin) ReedTheme.spacing.spacing8 else ReedTheme.spacing.spacing2,
+ ),
+ )
+ if (!requiresLogin) {
+ ReedTextButton(
+ onClick = {
+ state.eventSink(LoginUiEvent.OnGuestLoginButtonClick)
+ },
+ text = stringResource(R.string.guest_login),
+ sizeStyle = smallButtonStyle,
+ colorStyle = ReedButtonColorStyle.TEXT,
+ )
+ }
}참고: 상단 로고 contentDescription = "Reed Logo"도 향후 stringResource 또는 null(장식용)로 정리 고려.
📝 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.
| Column( | |
| modifier = Modifier.fillMaxWidth(), | |
| horizontalAlignment = Alignment.CenterHorizontally, | |
| ) { | |
| if (state.returnToScreen == null) { | |
| ReedButton( | |
| onClick = { | |
| state.eventSink(LoginUiEvent.OnKakaoLoginButtonClick) | |
| }, | |
| sizeStyle = largeButtonStyle, | |
| colorStyle = ReedButtonColorStyle.KAKAO, | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .padding( | |
| start = ReedTheme.spacing.spacing5, | |
| end = ReedTheme.spacing.spacing5, | |
| ), | |
| text = stringResource(id = R.string.kakao_login), | |
| leadingIcon = { | |
| Icon( | |
| imageVector = ImageVector.vectorResource(id = R.drawable.ic_kakao), | |
| contentDescription = "Kakao Icon", | |
| tint = Color.Unspecified, | |
| ) | |
| }, | |
| ) | |
| Spacer(modifier = Modifier.height(ReedTheme.spacing.spacing2)) | |
| ReedTextButton( | |
| onClick = { | |
| state.eventSink(LoginUiEvent.OnGuestLoginButtonClick) | |
| }, | |
| text = stringResource(R.string.guest_login), | |
| sizeStyle = smallButtonStyle, | |
| colorStyle = ReedButtonColorStyle.TEXT, | |
| ) | |
| }, | |
| ) | |
| } else { | |
| ReedButton( | |
| onClick = { | |
| state.eventSink(LoginUiEvent.OnKakaoLoginButtonClick) | |
| }, | |
| sizeStyle = largeButtonStyle, | |
| colorStyle = ReedButtonColorStyle.KAKAO, | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .padding( | |
| start = ReedTheme.spacing.spacing5, | |
| end = ReedTheme.spacing.spacing5, | |
| ), | |
| text = stringResource(id = R.string.kakao_login), | |
| leadingIcon = { | |
| Icon( | |
| imageVector = ImageVector.vectorResource(id = R.drawable.ic_kakao), | |
| contentDescription = "Kakao Icon", | |
| tint = Color.Unspecified, | |
| ) | |
| }, | |
| ) | |
| Spacer(modifier = Modifier.height(ReedTheme.spacing.spacing8)) | |
| } | |
| } | |
| val requiresLogin = state.returnToScreen != null | |
| Column( | |
| modifier = Modifier.fillMaxWidth(), | |
| horizontalAlignment = Alignment.CenterHorizontally, | |
| ) { | |
| ReedButton( | |
| onClick = { | |
| state.eventSink(LoginUiEvent.OnKakaoLoginButtonClick) | |
| }, | |
| sizeStyle = largeButtonStyle, | |
| colorStyle = ReedButtonColorStyle.KAKAO, | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .padding( | |
| start = ReedTheme.spacing.spacing5, | |
| end = ReedTheme.spacing.spacing5, | |
| ), | |
| text = stringResource(id = R.string.kakao_login), | |
| leadingIcon = { | |
| Icon( | |
| imageVector = ImageVector.vectorResource(id = R.drawable.ic_kakao), | |
| contentDescription = null, | |
| tint = Color.Unspecified, | |
| ) | |
| }, | |
| ) | |
| Spacer( | |
| modifier = Modifier.height( | |
| if (requiresLogin) ReedTheme.spacing.spacing8 else ReedTheme.spacing.spacing2, | |
| ), | |
| ) | |
| if (!requiresLogin) { | |
| ReedTextButton( | |
| onClick = { | |
| state.eventSink(LoginUiEvent.OnGuestLoginButtonClick) | |
| }, | |
| text = stringResource(R.string.guest_login), | |
| sizeStyle = smallButtonStyle, | |
| colorStyle = ReedButtonColorStyle.TEXT, | |
| ) | |
| } | |
| } |
🤖 Prompt for AI Agents
In feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginUi.kt
around lines 89 to 148, the Kakao login button is duplicated inside the if/else,
the icon contentDescription should be null (decorative) to avoid redundant
screen-reader output, and only the Spacer height needs to vary based on presence
of the guest button; refactor by extracting a single ReedButton declaration for
Kakao outside the branch, set the Icon's contentDescription = null, and
conditionally render only the Spacer with two different heights (small when
guest button present, larger when absent), keeping the ReedTextButton for guest
inside the branch.
| val isBookRegisterBottomSheetVisible: Boolean = false, | ||
| val selectedBookStatus: BookStatus? = null, | ||
| val isBookRegisterSuccessBottomSheetVisible: Boolean = false, | ||
| val isGuestMode: Boolean = false, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
게스트 모드 플래그 추가는 적절합니다. 게이트가 UI/Presenter 전 구간에서 일관되는지 점검하세요.
등록 바텀시트 표시, 등록 버튼 클릭, 상세 진입 등에서 isGuestMode 분기가 누락되지 않았는지 확인 필요.
아래 스크립트로 관련 분기와 이벤트 핸들링 위치를 빠르게 점검해 주세요:
🏁 Script executed:
#!/bin/bash
# 게스트 분기/등록 관련 이벤트 사용처 점검
rg -nP -C3 '\bisGuestMode\b|guest|OnBookRegisterButtonClick|OnBookRegisterBottomSheetDismiss|OnBookClick' --type=ktLength of output: 144
🏁 Script executed:
#!/bin/bash
# 게스트 모드 분기 및 등록 이벤트 사용처 재점검 (kt 확장자 기준)
rg -nP -C3 '\b(isGuestMode|guest|OnBookRegisterButtonClick|OnBookRegisterBottomSheetDismiss|OnBookClick)\b' -g '*.kt'Length of output: 26157
BookSearch UI/Presenter에 isGuestMode 분기 누락
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUi.kt (약 220–224행):
state.isGuestMode에 따라 BookItem 클릭·등록 버튼·BottomSheet 표시 여부를 제어하는 분기 로직 추가 필요
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt (약 273–276행):
isGuestMode를 기반으로 OnBookClick 및 OnBookRegisterButtonClick 이벤트 처리 검증 로직 추가 필요
🤖 Prompt for AI Agents
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUiState.kt
line 35: UI and presenter currently miss branching for isGuestMode; update the
BookSearchUi (around lines ~220–224) to check state.isGuestMode and disable/hide
BookItem click, register button and BottomSheet when true, and update
BookSearchPresenter (around lines ~273–276) to guard OnBookClick and
OnBookRegisterButtonClick handlers by isGuestMode (no-op or show guest-only
prompt) so guest users cannot trigger register/click actions; ensure consistent
state checks and any user feedback UI path for guest mode.
| exception = state.uiState.exception, | ||
| onRetryClick = { state.eventSink(HomeUiEvent.OnRetryClick) }, | ||
| ) | ||
| if (state.isGuestMode) { |
There was a problem hiding this comment.
Error시 체크보다는 HomePresenter의 RememberedEffect에서 분기하는 건 어떨까 싶어요. 게스트 모드일때는 굳이 home API를 찌를 필요가 없다고 생각합니다!
There was a problem hiding this comment.
e7ece1c
아예 API 호출을 막을 생각을 못했네여 굿 입니다. 반영완료
| exception = state.uiState.exception, | ||
| onRetryClick = { state.eventSink(LibraryUiEvent.OnRetryClick) }, | ||
| ) | ||
| if (state.isGuestMode) { |
There was a problem hiding this comment.
여기도 마찬가지로 Error일때 판단하기 보단 API 찌르는 시점에서 판단하는게 더 좋지 않을까 생각해요!
|
|
||
| @Parcelize | ||
| data object TermsAgreementScreen : ReedScreen(name = ScreenNames.TERMS_AGREEMENT) | ||
| data class TermsAgreementScreen(val returnToScreen: Screen? = null) : ReedScreen(name = ScreenNames.TERMS_AGREEMENT) |
There was a problem hiding this comment.
Screen 타입을 파라미터로 받아서 popUntil 해주는게 너무 좋네요 👍👍
| import androidx.compose.ui.res.stringResource | ||
|
|
||
| // https://www.youtube.com/watch?v=mB1Lej0aDus | ||
| sealed class UiText { |
There was a problem hiding this comment.
present에 String을 그대로 넣는 부분이 찝찝했었는데 (토끼한테 매번 잡히는 것도 그렇구) 너무 좋네요 👍👍
| val isRegistered: Boolean | ||
| get() = userBookStatus != BEFORE_REGISTRATION | ||
|
|
||
| companion object { |
|
|
||
| if (startIndex == START_INDEX) { | ||
| uiState = UiState.Success | ||
| analyticsHelper.logEvent(SEARCH_BOOK_RESULT) |
There was a problem hiding this comment.
도서 검색 결과가 없을 때 로그 이벤트 수집하는 이유가 있을까요?
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.kt (3)
133-141: 게스트 모드에서 필터 클릭 시 API 호출 발생 — 조기 반환으로 차단 필요UI는 게스트 프롬프트만 보여주므로, 불필요한 네트워크 호출을 막아야 합니다.
is LibraryUiEvent.OnFilterClick -> { + if (userState is UserState.Guest) { + return + } if (currentFilter == event.filterOption) { return }
151-155: 게스트 모드에서의 추가 로드 방지게스트 상태에서 OnLoadMore가 들어와도 호출되지 않도록 가드하세요. 방어적 프로그래밍 차원에서 안전합니다.
is LibraryUiEvent.OnLoadMore -> { - if (footerState !is FooterState.Loading && !isLastPage) { + if (userState is UserState.Guest) return + if (footerState !is FooterState.Loading && !isLastPage) { filterLibraryBooks(status = currentFilter.getApiValue(), page = currentPage + 1, size = PAGE_SIZE) } }
157-159: 게스트 모드에서의 재시도 방지Retry 역시 동일하게 차단하세요.
is LibraryUiEvent.OnRetryClick -> { - filterLibraryBooks(status = currentFilter.getApiValue(), page = currentPage, size = PAGE_SIZE) + if (userState is UserState.Guest) return + filterLibraryBooks(status = currentFilter.getApiValue(), page = currentPage, size = PAGE_SIZE) }feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt (1)
47-61: 중복 로드 방지 가드 추가현재 상태가 Loading/Success여도 추가 호출이 가능해(특히 빠른 상태 변동 시) 중복 요청이 발생할 수 있습니다. 간단한 early-return을 추가하세요.
fun loadHomeContent() { scope.launch { + if (uiState is UiState.Loading || uiState is UiState.Success) return@launch if (uiState is UiState.Idle || uiState is UiState.Error) { uiState = UiState.Loading } bookRepository.getHome() .onSuccess { result -> uiState = UiState.Success recentBooks = result.recentBooks.toPersistentList() }.onFailure { exception -> uiState = UiState.Error(exception) } } }
♻️ Duplicate comments (3)
feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryUi.kt (2)
106-135: Column 내부 fillMaxSize로 인한 오버플로우 — weight(1f)로 남은 공간만 차지하도록 수정 필요FilterChipGroup 위에 형제가 있는 상태에서 Box에 fillMaxSize를 주면 레이아웃 충돌/오버플로우가 납니다. 이전 코멘트와 동일 이슈입니다. weight(1f)로 남은 영역만 사용하세요.
아래처럼 변경(+ import 추가 필요: androidx.compose.foundation.layout.weight):
+import androidx.compose.foundation.layout.weight @@ - Box( - modifier = Modifier.fillMaxSize(), + Box( + modifier = Modifier + .fillMaxWidth() + .weight(1f), contentAlignment = Alignment.Center, ) {
180-186: Error UI도 weight 컨테이너로 감싸기ReedErrorUi 내부가 fillMaxSize이므로, Column 내에서 그대로 쓰면 충돌합니다. 호출부에서 weight(1f) 박스로 감싸 주세요.
- ReedErrorUi( - exception = state.uiState.exception, - onRetryClick = { state.eventSink(LibraryUiEvent.OnRetryClick) }, - ) + Box( + modifier = Modifier + .fillMaxWidth() + .weight(1f), + contentAlignment = Alignment.Center, + ) { + ReedErrorUi( + exception = state.uiState.exception, + onRetryClick = { state.eventSink(LibraryUiEvent.OnRetryClick) }, + ) + }feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt (1)
95-99: 게스트 모드에서 Home API 호출 회피 구현 확인 — 이전 피드백 반영userState 기반 RememberedEffect 분기로 게스트일 때 호출을 막았습니다. 이전 리뷰(“게스트 모드일때는 굳이 home API를 찌를 필요가 없다”) 취지에 부합합니다.
🧹 Nitpick comments (3)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt (2)
104-111: 헤더/간격 UI 중복 제거로 DRY 유지게스트/비게스트 두 경로에서 동일한 타이틀(Text)과 상단/하단 Spacer 패턴이 반복됩니다. 작은 컴포저블로 추출해 중복을 제거하면 향후 스타일 변경에 안전합니다.
// 파일 하단에 추가 @Composable private fun ReadingNowHeader() { Spacer(modifier = Modifier.height(ReedTheme.spacing.spacing6)) Text( text = stringResource(R.string.home_content_label_reading_now), modifier = Modifier.padding(start = ReedTheme.spacing.spacing5), color = ReedTheme.colors.contentSecondary, style = ReedTheme.typography.headline2Medium, ) Spacer(modifier = Modifier.height(ReedTheme.spacing.spacing3)) }- Spacer(modifier = Modifier.height(ReedTheme.spacing.spacing6)) - Text( - text = stringResource(R.string.home_content_label_reading_now), - modifier = Modifier.padding(start = ReedTheme.spacing.spacing5), - color = ReedTheme.colors.contentSecondary, - style = ReedTheme.typography.headline2Medium, - ) - Spacer(modifier = Modifier.height(ReedTheme.spacing.spacing3)) + ReadingNowHeader()Also applies to: 134-141
175-191: 페이지 인디케이터에 접근성/테스트 태그 부여 제안스크린리더/테스트 자동화 지원을 위해 각 점에 semantics와 testTag를 추가하세요.
- Box( - modifier = Modifier - .size(12.dp) - .padding(3.dp) - .clip(CircleShape) - .background(color), - ) + Box( + modifier = Modifier + .size(12.dp) + .padding(3.dp) + .clip(CircleShape) + .background(color) + .semantics { + contentDescription = + if (pagerState.currentPage == iteration) + "current_page_${iteration + 1}" + else "page_${iteration + 1}" + } + .testTag("home_pager_dot_${iteration + 1}"), + )참고:
testTag사용 시import androidx.compose.ui.platform.testTag가 필요합니다.feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt (1)
95-99: 로그아웃 전환 시 화면 상태 초기화 제안게스트로 전환되면 기존 데이터가 메모리에 남습니다(현재는 UI에서만 숨김). 전환 시 상태를 초기화하면 메모리/일관성 측면에서 안전합니다.
- RememberedEffect(userState) { - if (userState !is UserState.Guest) { - loadHomeContent() - } - } + RememberedEffect(userState) { + if (userState !is UserState.Guest) { + loadHomeContent() + } else { + uiState = UiState.Idle + recentBooks = persistentListOf() + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt(6 hunks)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt(2 hunks)feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.kt(7 hunks)feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryUi.kt(2 hunks)feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginUi.kt(3 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUi.kt(1 hunks)feature/settings/src/main/res/values/strings.xml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- feature/settings/src/main/res/values/strings.xml
- feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUi.kt
- feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/LoginUi.kt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-28T12:25:54.013Z
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#174
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt:128-133
Timestamp: 2025-08-28T12:25:54.013Z
Learning: In BookSearchPresenter.kt, when a guest user tries to register a book and is redirected to login, the bottom sheet (isBookRegisterBottomSheetVisible) and selection state (selectedBookIsbn, selectedBookStatus) are intentionally kept open/preserved so that when the user returns from login, they can continue from where they left off without losing context.
Applied to files:
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.ktfeature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryUi.ktfeature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.kt
🧬 Code graph analysis (3)
feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryUi.kt (2)
core/designsystem/src/main/kotlin/com/ninecraft/booket/core/designsystem/component/button/ReedButton.kt (1)
ReedButton(34-114)core/ui/src/main/kotlin/com/ninecraft/booket/core/ui/component/ReedErrorUi.kt (1)
ReedErrorUi(22-48)
feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.kt (2)
core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/BookRepository.kt (1)
filterLibraryBooks(34-38)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultBookRepository.kt (1)
filterLibraryBooks(58-60)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt (3)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/component/BookCard.kt (2)
EmptyBookCard(208-267)BookCard(49-206)core/ui/src/main/kotlin/com/ninecraft/booket/core/ui/component/ReedLoadingIndicator.kt (1)
ReedLoadingIndicator(13-25)core/ui/src/main/kotlin/com/ninecraft/booket/core/ui/component/ReedErrorUi.kt (1)
ReedErrorUi(22-48)
⏰ 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: ci-build
🔇 Additional comments (8)
feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryUi.kt (1)
21-23: 디자인 시스템 버튼 도입 적절게스트 모드 CTA에 ReedButton/ColorStyle/Size 사용 선택 적절합니다.
feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.kt (1)
118-127: 게스트 분기 + 로그인 리다이렉트 처리 적절토스트 후 redirectToLogin 호출 흐름 명확합니다. iOS와의 플로우 통일 목적에 부합합니다.
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt (4)
112-119: 게스트 경로의 등록 버튼 이벤트 연결 LGTM게스트에서 등록 버튼을 로그인 유도 플로우로 연결하는 이벤트 바인딩이 명확합니다.
151-173: Pager와 이벤트 바인딩 구조 적절pageCount, item 접근, 디테일/기록 이동 이벤트 바인딩 모두 자연스럽습니다.
123-126: 로딩 상태 처리 LGTM공용 로딩 인디케이터 사용이 일관적입니다.
197-202: 게스트 모드에서 오류 UI 숨김 — 의도에 부합로그인 사용자 경로에서만 오류/재시도 UI를 노출하는 방향이 게스트 모드 목적과 일치합니다.
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt (2)
106-110: isGuestMode 파생 상태 노출 LGTMPresenter에서 userState 파생값으로 UI에 명시 전달하는 구조가 명확합니다.
69-71: BookSearchScreen 로그인 리다이렉트 후 컨텍스트 유지 검증 완료
BookSearchPresenter의rememberRetained상태(isBookRegisterBottomSheetVisible,selectedBookIsbn,selectedBookStatus)가navigator.redirectToLogin→popUntil흐름에서 초기화되지 않고 유지됩니다.
| InfinityLazyColumn( | ||
| modifier = Modifier.fillMaxSize(), | ||
| loadMore = { | ||
| state.eventSink(LibraryUiEvent.OnLoadMore) | ||
| }, | ||
| ) { | ||
| items(state.books) { | ||
| LibraryBookItem( | ||
| book = it, | ||
| onBookClick = { | ||
| state.eventSink(LibraryUiEvent.OnBookClick(it.userBookId, it.isbn13)) | ||
| }, | ||
| ) | ||
| Box( | ||
| modifier = modifier | ||
| .fillMaxWidth() | ||
| .height(1.dp) | ||
| .background(ReedTheme.colors.borderPrimary), | ||
| ) | ||
| } | ||
| item { | ||
| LoadStateFooter( | ||
| footerState = state.footerState, | ||
| onRetryClick = { state.eventSink(LibraryUiEvent.OnLoadMore) }, | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
리스트 컨테이너도 fillMaxSize → weight(1f)로 교체
InfinityLazyColumn이 Column 자식이므로 동일한 오버플로우 리스크가 있습니다. 남은 공간만 사용하도록 조정하세요.
- InfinityLazyColumn(
- modifier = Modifier.fillMaxSize(),
+ InfinityLazyColumn(
+ modifier = Modifier
+ .fillMaxWidth()
+ .weight(1f),
loadMore = {
state.eventSink(LibraryUiEvent.OnLoadMore)
},
) {📝 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.
| InfinityLazyColumn( | |
| modifier = Modifier.fillMaxSize(), | |
| loadMore = { | |
| state.eventSink(LibraryUiEvent.OnLoadMore) | |
| }, | |
| ) { | |
| items(state.books) { | |
| LibraryBookItem( | |
| book = it, | |
| onBookClick = { | |
| state.eventSink(LibraryUiEvent.OnBookClick(it.userBookId, it.isbn13)) | |
| }, | |
| ) | |
| Box( | |
| modifier = modifier | |
| .fillMaxWidth() | |
| .height(1.dp) | |
| .background(ReedTheme.colors.borderPrimary), | |
| ) | |
| } | |
| item { | |
| LoadStateFooter( | |
| footerState = state.footerState, | |
| onRetryClick = { state.eventSink(LibraryUiEvent.OnLoadMore) }, | |
| ) | |
| } | |
| } | |
| InfinityLazyColumn( | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .weight(1f), | |
| loadMore = { | |
| state.eventSink(LibraryUiEvent.OnLoadMore) | |
| }, | |
| ) { | |
| items(state.books) { | |
| LibraryBookItem( | |
| book = it, | |
| onBookClick = { | |
| state.eventSink(LibraryUiEvent.OnBookClick(it.userBookId, it.isbn13)) | |
| }, | |
| ) | |
| Box( | |
| modifier = modifier | |
| .fillMaxWidth() | |
| .height(1.dp) | |
| .background(ReedTheme.colors.borderPrimary), | |
| ) | |
| } | |
| item { | |
| LoadStateFooter( | |
| footerState = state.footerState, | |
| onRetryClick = { state.eventSink(LibraryUiEvent.OnLoadMore) }, | |
| ) | |
| } | |
| } |
🤖 Prompt for AI Agents
feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryUi.kt
around lines 150-176: the InfinityLazyColumn is using Modifier.fillMaxSize()
while being a child of a Column, which can cause overflow; replace
Modifier.fillMaxSize() with Modifier.weight(1f) (ensure you import
androidx.compose.foundation.layout.weight and that InfinityLazyColumn is called
from a ColumnScope), removing the fillMaxSize usage so the list only takes
remaining space.
seoyoon513
left a comment
seoyoon513
left a comment
There was a problem hiding this comment.
고생하셨습니다~ 약관동의 부분만 확인되면 될 것 같네요!
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.kt (1)
133-140: 게스트 모드에서 API 호출 발생 가능 — 이벤트 가드 추가 필요게스트일 때 OnFilterClick/OnLoadMore/OnRetryClick가 서버 호출을 트리거할 수 있습니다. 불필요한 에러/토스트 폭탄을 막기 위해 가드를 넣어주세요.
아래처럼 조정 권장:
is LibraryUiEvent.OnFilterClick -> { if (currentFilter == event.filterOption) { return } + if (userState is UserState.Guest) { + sideEffect = LibrarySideEffect.ShowToast(UiText.StringResource(designR.string.login_required)) + return + } currentFilter = event.filterOption filterLibraryBooks(status = currentFilter.getApiValue(), page = START_INDEX, size = PAGE_SIZE) } is LibraryUiEvent.OnLoadMore -> { - if (footerState !is FooterState.Loading && !isLastPage) { + if (userState is UserState.Guest) return + if (footerState !is FooterState.Loading && uiState !is UiState.Loading && !isLastPage) { filterLibraryBooks(status = currentFilter.getApiValue(), page = currentPage + 1, size = PAGE_SIZE) } } is LibraryUiEvent.OnRetryClick -> { - filterLibraryBooks(status = currentFilter.getApiValue(), page = currentPage, size = PAGE_SIZE) + if (userState is UserState.Guest) return + filterLibraryBooks(status = currentFilter.getApiValue(), page = currentPage, size = PAGE_SIZE) }Also applies to: 151-155, 157-159
♻️ Duplicate comments (2)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt (1)
97-118: 게스트 분기 UI 추가 LGTM + 중복 체크 제거 반영 확인게스트 전용 스크롤 컬럼과 EmptyBookCard 와이어링 적절합니다. 이전 커밋에서 지적된 중복 isGuestMode 분기 제거도 확인했습니다.
feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.kt (1)
177-185: 로그인/로그아웃 전환 시 상태 초기화 누락 — RememberedEffect에 reset 로직 추가이전 코멘트에서 지적된 내용이 일부만 반영되었습니다. Guest 전환 시 리스트/페이징을 초기화하지 않으면 비로그인 상태에서 기존 데이터가 남습니다. 로그인 전환 시에도 페이징 초기화 후 첫 페이지를 로드하세요.
-RememberedEffect(userState) { - if (userState !is UserState.Guest) { - filterLibraryBooks( - status = currentFilter.getApiValue(), - page = START_INDEX, - size = PAGE_SIZE, - ) - } -} +RememberedEffect(userState) { + // 공통 초기화 + currentPage = START_INDEX + isLastPage = false + + if (userState !is UserState.Guest) { + // 로그인 직후, 기존 목록/푸터 초기화 후 첫 페이지 로드 + books = persistentListOf() + footerState = FooterState.Idle + filterLibraryBooks( + status = currentFilter.getApiValue(), + page = START_INDEX, + size = PAGE_SIZE, + ) + } else { + // 게스트 전환 시 화면 상태 정리 + uiState = UiState.Idle + footerState = FooterState.Idle + books = persistentListOf() + } +}
🧹 Nitpick comments (6)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt (6)
92-96: 최상위 컨테이너에 전달된 modifier 적용 누락Box에 로컬 Modifier를 고정 사용하면 상위에서 전달한 testTag/패딩/클립 등이 무시됩니다. 전달된 modifier를 최상위 컨테이너에 적용하세요.
- Box( - modifier = Modifier + Box( + modifier = modifier .fillMaxSize() .background(ReedTheme.colors.baseSecondary), ) {
120-125: Idle 상태에서 빈 화면 — Loading 표시 권장초기 진입 시 Idle이 잠깐 유지되면 공백 화면이 보일 수 있습니다. Idle도 로딩으로 처리해 UX 공백을 줄이세요.
- is UiState.Idle -> {} + is UiState.Idle -> { + ReedLoadingIndicator() + }
126-139: 섹션 헤더/스크롤 구조 중복 최소화 제안게스트/비게스트에서 동일한 헤더(Text)와 스크롤 구성이 중복됩니다. 작은 컴포저블로 추출해 재사용하세요.
아래와 같이 파일 하단에 추가:
@Composable private fun ReadingNowHeader() { Spacer(modifier = Modifier.height(ReedTheme.spacing.spacing6)) Text( text = stringResource(R.string.home_content_label_reading_now), modifier = Modifier.padding(start = ReedTheme.spacing.spacing5), color = ReedTheme.colors.contentSecondary, style = ReedTheme.typography.headline2Medium, ) Spacer(modifier = Modifier.height(ReedTheme.spacing.spacing3)) }그리고 두 분기에서 해당 블록을
ReadingNowHeader()로 교체.
149-171: 인덱스 반복 접근 감소로 가독성/안전성 개선
state.recentBooks[page]반복 인덱싱 대신 지역 변수로 담아 사용하면 가독성과 NPE 방어(변경 중) 측면에서 유리합니다.- HorizontalPager( + HorizontalPager( state = pagerState, modifier = Modifier.fillMaxWidth(), contentPadding = PaddingValues(horizontal = ReedTheme.spacing.spacing5), pageSpacing = ReedTheme.spacing.spacing5, ) { page -> - BookCard( - recentBookInfo = state.recentBooks[page], + val book = state.recentBooks[page] + BookCard( + recentBookInfo = book, onBookDetailClick = { state.eventSink( HomeUiEvent.OnBookDetailClick( - state.recentBooks[page].userBookId, - state.recentBooks[page].isbn13, + book.userBookId, + book.isbn13, ), ) }, onRecordButtonClick = { - state.eventSink(HomeUiEvent.OnRecordButtonClick(state.recentBooks[page].userBookId)) + state.eventSink(HomeUiEvent.OnRecordButtonClick(book.userBookId)) }, ) }
173-189: 페이저 인디케이터 전환 애니메이션 제안점 색상 전환에 애니메이션을 주면 전환 감이 더 자연스럽습니다.
필요 import:
import androidx.compose.animation.core.animateFloatAsState
import androidx.compose.animation.animateColorAsState- repeat(pagerState.pageCount) { iteration -> - val color = - if (pagerState.currentPage == iteration) ReedTheme.colors.bgPrimary else ReedTheme.colors.bgSecondaryPressed + repeat(pagerState.pageCount) { iteration -> + val target = if (pagerState.currentPage == iteration) ReedTheme.colors.bgPrimary else ReedTheme.colors.bgSecondaryPressed + val color by animateColorAsState(targetValue = target, label = "pagerDotColor") Box( modifier = Modifier .size(12.dp) .padding(3.dp) .clip(CircleShape) .background(color), ) }
206-217: 게스트/데이터 케이스별 프리뷰 추가 제안디자인/QA 확인 편의를 위해 Guest, Empty, Pager(여러 권) 프리뷰를 각각 제공하면 좋습니다.
예시(추가 코드):
@DevicePreview @Composable private fun HomeGuestPreview() { ReedTheme { HomeUi( state = HomeUiState( isGuestMode = true, uiState = UiState.Idle, eventSink = {}, ), ) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt(1 hunks)feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.kt(7 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#174
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt:128-133
Timestamp: 2025-08-28T12:25:54.013Z
Learning: In BookSearchPresenter.kt, when a guest user tries to register a book and is redirected to login, the bottom sheet (isBookRegisterBottomSheetVisible) and selection state (selectedBookIsbn, selectedBookStatus) are intentionally kept open/preserved so that when the user returns from login, they can continue from where they left off without losing context.
📚 Learning: 2025-08-28T12:25:54.013Z
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#174
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt:128-133
Timestamp: 2025-08-28T12:25:54.013Z
Learning: In BookSearchPresenter.kt, when a guest user tries to register a book and is redirected to login, the bottom sheet (isBookRegisterBottomSheetVisible) and selection state (selectedBookIsbn, selectedBookStatus) are intentionally kept open/preserved so that when the user returns from login, they can continue from where they left off without losing context.
Applied to files:
feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.ktfeature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt
🧬 Code graph analysis (2)
feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.kt (2)
core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/BookRepository.kt (1)
filterLibraryBooks(34-38)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultBookRepository.kt (1)
filterLibraryBooks(58-60)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt (3)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/component/BookCard.kt (2)
EmptyBookCard(208-267)BookCard(49-206)core/ui/src/main/kotlin/com/ninecraft/booket/core/ui/component/ReedLoadingIndicator.kt (1)
ReedLoadingIndicator(13-25)core/ui/src/main/kotlin/com/ninecraft/booket/core/ui/component/ReedErrorUi.kt (1)
ReedErrorUi(22-48)
⏰ 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: ci-build
🔇 Additional comments (8)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt (3)
141-148: 최근 도서 없음 처리 LGTMEmpty 상태에서 등록 버튼을 홈 이벤트로 연결한 점 좋습니다. 게스트/비게스트 케이스 간 UX 일관성 유지됩니다.
195-200: 에러 UI 연결 LGTM예외 메시지/재시도 액션 연결 적절합니다. 게스트 분기에서 에러 UI를 노출하지 않는 점도 의도와 일치합니다.
97-118: HomePresenter에 게스트 모드 API 호출 차단 여부 확인
스크립트 실행 결과 HomePresenter.kt에서 isGuestMode 분기나 홈/최근 도서 API 호출 흔적이 발견되지 않았습니다. 게스트 모드 시 실제로 API 호출이 전혀 발생하지 않는지 다시 검토해주세요.feature/library/src/main/kotlin/com/ninecraft/booket/feature/library/LibraryPresenter.kt (5)
10-11: Auth/UiText 도입 및 DI 시그니처 변경 LGTMAuthRepository 주입, collectAsRetainedState, designR alias, redirectToLogin 확장 사용 모두 일관적입니다.
Also applies to: 14-14, 20-20, 24-24, 36-36, 40-41
73-73: 레포지토리 교체 반영 OKbookRepository로의 전환 및 호출 시그니처 일치합니다.
118-126: 로그인 유도 + 토스트 처리 LGTM게스트의 검색 진입을 토스트와 함께 로그인으로 유도하는 UX는 PR 목표와 부합합니다.
197-197: isGuestMode 파생 상태 전달 LGTMUI 분기를 단순화하는 좋은 추가입니다.
118-126: redirectToLogin은 suspend 함수로 coroutine 범위 내에서 호출이 필요합니다
navigator.redirectToLogin()는suspend fun Navigator.redirectToLogin()이므로scope.launch { … }없이 직접 호출하면 컴파일 오류가 발생합니다. 제안된 변경은 불필요합니다.Likely an incorrect or invalid review comment.
versionName: 1.1.2 -> 1.2.0 versionCode: 6 -> 7
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt (1)
109-118: 에러 처리 일관화 — handleException으로 메시지/네트워크 케이스 통합현재는 exception.message에 의존해 사용자 메시지가 불안정합니다. 공통 헬퍼로 통일해 UX와 로깅을 개선하세요.
- Logger.d(exception) - analyticsHelper.logEvent(ERROR_SEARCH_LOADING) - val errorMessage = exception.message ?: "알 수 없는 오류가 발생했습니다." - if (startIndex == START_INDEX) { - uiState = UiState.Error(exception) - } else { - footerState = FooterState.Error(errorMessage) - } + Logger.d(exception) + analyticsHelper.logEvent(ERROR_SEARCH_LOADING) + handleException( + exception = exception, + onError = { message -> + if (startIndex == START_INDEX) { + uiState = UiState.Error(Exception(message)) + } else { + footerState = FooterState.Error(message) + } + }, + )
♻️ Duplicate comments (1)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt (1)
78-91: 동시 검색 경쟁 상태 — 이전 검색 Job 취소 및 결과 중복 제거 필요빠른 재검색/스크롤 시 이전 코루틴과 결과가 뒤섞일 수 있습니다. 마지막 검색만 유효하도록 이전 Job을 취소하고, 페이지 병합 시 ISBN 기준으로 중복 제거하세요.
아래와 같이 수정 제안합니다.
@@ - fun searchBooks(query: String, startIndex: Int = START_INDEX) { - scope.launch { + fun searchBooks(query: String, startIndex: Int = START_INDEX) { + searchJob?.cancel() + searchJob = scope.launch { @@ - } else { - (books + result.books).toPersistentList() - } + } else { + (books + result.books) + .distinctBy { it.isbn13 } + .toPersistentList() + }아래 보일러플레이트를 함께 추가하세요(선택 라인 외 추가 코드):
// imports import kotlinx.coroutines.Job // after line 61 (scope 선언 직후) var searchJob by rememberRetained { mutableStateOf<Job?>(null) }Also applies to: 92-98, 198-203
🧹 Nitpick comments (2)
gradle/libs.versions.toml (1)
6-7: 버전 상승(1.1.2 → 1.2.0, code 6 → 7) 적절합니다.게스트 모드 등 기능 추가에 맞춘 마이너 버전 업으로 보이며, versionCode도 증가했습니다. 릴리스 태깅/CHANGELOG 갱신 및 스토어 노트 업데이트만 잊지 마세요.
필요하시면 릴리스 노트 템플릿 초안을 만들어 드릴게요.
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt (1)
86-90: 선택: 인증 만료(401) 시 게스트 검색으로 폴백해 끊김 없는 검색 유지로그인 사용자가 토큰 만료 시 검색이 실패로 끝납니다. 401이면 게스트 엔드포인트로 자동 재시도하면 UX가 매끄럽습니다.
예시:
// imports import retrofit2.HttpException // onFailure 블록 상단 if (exception is HttpException && exception.code() == 401) { repository.searchBookAsGuest(query = query, start = startIndex) .onSuccess { /* 기존 성공 처리 재사용 */ } .onFailure { inner -> handleException(inner, onError = { msg -> if (startIndex == START_INDEX) uiState = UiState.Error(Exception(msg)) else footerState = FooterState.Error(msg) }) } return@launch }401 발생 가능 여부와 API 제약을 확인해 주세요.
Also applies to: 109-118
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt(11 hunks)gradle/libs.versions.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#174
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt:128-133
Timestamp: 2025-08-28T12:25:54.013Z
Learning: In BookSearchPresenter.kt, when a guest user tries to register a book and is redirected to login, the bottom sheet (isBookRegisterBottomSheetVisible) and selection state (selectedBookIsbn, selectedBookStatus) are intentionally kept open/preserved so that when the user returns from login, they can continue from where they left off without losing context.
📚 Learning: 2025-08-28T12:25:54.013Z
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#174
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt:128-133
Timestamp: 2025-08-28T12:25:54.013Z
Learning: In BookSearchPresenter.kt, when a guest user tries to register a book and is redirected to login, the bottom sheet (isBookRegisterBottomSheetVisible) and selection state (selectedBookIsbn, selectedBookStatus) are intentionally kept open/preserved so that when the user returns from login, they can continue from where they left off without losing context.
Applied to files:
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt
🧬 Code graph analysis (1)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt (1)
core/common/src/main/kotlin/com/ninecraft/booket/core/common/utils/HandleException.kt (1)
handleException(19-46)
⏰ 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: ci-build
🔇 Additional comments (7)
gradle/libs.versions.toml (1)
6-7: 카탈로그 기반 버전 적용 검증 결과
- core/data/impl/build.gradle.kts의 defaultConfig에서
buildConfigField("String", "APP_VERSION", "\"${libs.versions.versionName.get()}\"")로 버전명을 카탈로그에서 불러오고 있습니다.- 그러나
libs.versions.versionCode.get()참조가 전혀 발견되지 않습니다. 모든 모듈의 Gradle 설정(build.gradle/build.gradle.kts)에
versionCode = libs.versions.versionCode.get().toInt()형태로 버전코드를 적용했는지 누락 여부를 확인해 주세요.- AndroidManifest.xml에는 하드코딩된
android:versionName/android:versionCode속성이 없습니다.feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt (6)
124-128: 게스트 업서트 경로에서 상태 보존 의도 확인 — 그대로 OK게스트 → 로그인 복귀 시 바텀시트/선택 상태 유지 설계가 학습된 의도와 일치합니다. UX 관점에서 적절합니다.
215-216: ISBN 누락 에러 처리의 현지화 적용 확인하드코딩 제거 및 UiText.StringResource 사용 적절합니다.
254-256: 화면 노출 로깅 위치 적절ImpressionEffect로 1회성 화면 노출 로깅 구현이 적절합니다.
269-269: UI 분기 상태 제공 적합isGuestMode를 UI 상태로 노출하는 접근이 명확합니다.
275-275: @CircuitInject 스크린 매핑 변경 LGTMBookSearchScreen 기준으로의 DI 연결이 현재 구조와 일치합니다.
199-203: 페이지네이션 로직 검증 완료:start는 1-based 페이지 번호이며, 다음 페이지 요청 시currentStartIndex + 1이 올바릅니다.
| navigator.resetRoot(LoginScreen()) | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
로그인 리다이렉트 방식 일관화 권장
여기서는 resetRoot를, 위 게스트 분기에서는 redirectToLogin()을 사용합니다. 동일한 확장 함수를 사용해 내비게이션 일관성을 유지하세요.
- navigator.resetRoot(LoginScreen())
+ navigator.redirectToLogin()📝 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.
| navigator.resetRoot(LoginScreen()) | |
| }, | |
| navigator.redirectToLogin() | |
| }, |
🤖 Prompt for AI Agents
In
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt
around lines 156-157, the code uses navigator.resetRoot(LoginScreen()) while the
guest branch above uses navigator.redirectToLogin(); replace the resetRoot call
with the same extension function (navigator.redirectToLogin()) to keep
navigation behavior consistent, and remove or adjust any now-unused imports or
parameters so the project builds.
2 participants
🔗 관련 이슈
📙 작업 설명
🧪 테스트 내역 (선택)
📸 스크린샷 또는 시연 영상 (선택)
BookSearch.mp4
LibrarySetting.mp4
영상 찍다가 리다이렉트로 로그인화면 진입시, 하단에 버튼 패딩 넣는거 누락한거 확인해서 추가했습니다
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
신기능
개선사항