Conversation
Walkthrough책 관련 API 엔드포인트·DTO·Repository/Service 메서드와 뷰모델을 추가·개편하고, 검색 UI를 MVVM(디바운스)·무한스크롤로 전환했으며 네비게이션 경로와 일부 UI(책 등록 네비게이션, 장르 칩 등)를 확장/수정했습니다. (≤50단어) Changes
Sequence Diagram(s)sequenceDiagram
participant UI as SearchBookScreen
participant VM as SearchBookViewModel
participant Repo as BookRepository
participant Svc as BookService
UI->>VM: updateSearchQuery(query)
VM-->>VM: debounce(1s) → performSearch
VM->>Repo: searchBooks(keyword, page)
Repo->>Svc: GET /books?keyword=&page=&isFinalized=
Svc-->>Repo: BaseResponse<BookSearchResponse>
Repo-->>VM: Result<BookSearchResponse?>
VM-->>UI: uiState 업데이트 (결과/로딩/페이징)
sequenceDiagram
participant UI as SearchBookDetailScreen
participant VM as BookDetailViewModel
participant Repo as BookRepository
participant Svc as BookService
UI->>VM: loadBookDetail(isbn)
VM->>Repo: getBookDetail(isbn)
Repo->>Svc: GET /books/{isbn}
Svc-->>Repo: BaseResponse<BookDetailResponse>
Repo-->>VM: Result<BookDetailResponse?>
VM-->>UI: uiState.bookDetail 갱신
UI->>VM: saveBook(isbn, type)
VM->>Repo: saveBook(isbn, BookSaveRequest(type))
Repo->>Svc: POST /books/{isbn}/saved
Svc-->>Repo: BaseResponse<BookSaveResponse>
Repo-->>VM: Result<BookSaveResponse?>
VM-->>UI: uiState.bookDetail.isSaved 업데이트
sequenceDiagram
participant UI as SearchBookGroupScreen
participant VM as SearchBookGroupViewModel
participant Repo as BookRepository
participant Svc as BookService
UI->>VM: loadRecruitingRooms(isbn)
VM->>Repo: getRecruitingRooms(isbn, cursor=null)
Repo->>Svc: GET /books/{isbn}/recruiting-rooms
Svc-->>Repo: BaseResponse<RecruitingRoomsResponse>
Repo-->>VM: Result<RecruitingRoomsResponse?>
VM-->>UI: uiState(목록/total/nextCursor)
UI->>VM: loadMoreRooms()
VM->>Repo: getRecruitingRooms(isbn, cursor)
Repo-->>VM: 결과 append
VM-->>UI: 목록 append, 로딩 상태 갱신
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
✨ 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: 26
🔭 Outside diff range comments (1)
app/src/main/java/com/texthip/thip/ui/common/buttons/GenreChipButton.kt (1)
61-69: 접근성: 닫기 아이콘 터치 타깃(48dp) 부족 + contentDescription 누락
- 현재
Icon의 클릭 영역이 사실상 20dp 수준으로, Material 권장 최소 터치 타깃(48dp)에 미달합니다.- 상호작용 가능한 요소의
contentDescription이null입니다. 스크린리더에서 “레이블 없는 버튼”으로 읽힐 수 있습니다.
IconButton또는 래퍼 박스로 히트 영역을 키우고, 아이콘 크기는 20dp로 유지하세요. 또한 접근성용 문자열을 리소스로 제공하세요.- Icon( - painter = painterResource(R.drawable.ic_x_20), - contentDescription = null, - tint = Color.Unspecified, - modifier = Modifier - .clickable { - onCloseClick() - } - .size(20.dp) - ) + Box( + modifier = Modifier + .size(40.dp) // 권장: 40~48dp + .clickable(role = Role.Button) { onCloseClick() }, + contentAlignment = Alignment.Center + ) { + Icon( + painter = painterResource(R.drawable.ic_x_20), + contentDescription = stringResource(R.string.a11y_close), + tint = Color.Unspecified, + modifier = Modifier.size(20.dp) + ) + }추가 import:
import androidx.compose.ui.res.stringResource import androidx.compose.ui.semantics.Rolestrings.xml에
a11y_close(예: "닫기")를 추가해 주세요. 참고로 중첩 clickable(칩 전체 + 닫기)은 자식(닫기)이 우선 소비하므로, 닫기 터치 시 외부 onClick은 호출되지 않습니다(의도대로 동작).
🧹 Nitpick comments (28)
app/src/main/java/com/texthip/thip/ui/common/buttons/GenreChipButton.kt (1)
84-84: 프리뷰의 하드코딩 문자열을 stringResource로 되돌려 주세요이전 PR에서 합의된 문자열 리소스 사용 선호사항에 따라, Preview에서도 리소스를 쓰는 편이 좋습니다.
예시(기존 값이
R.string.essay였던 점을 감안한 복구안):- text = "책", + text = stringResource(R.string.essay),필요 시
R.string.book등 도메인에 맞는 리소스를 신설해도 됩니다. 사용 시 아래 import를 추가하세요:import androidx.compose.ui.res.stringResourceapp/src/main/java/com/texthip/thip/data/model/book/response/BookSaveResponse.kt (1)
5-9: 역직렬화 안전성 향상: 기본값을 추가해 주세요서버 필드 누락 시 kotlinx.serialization에서 예외가 발생할 수 있습니다. 기본값을 부여해 방어적으로 처리하는 것을 권장합니다.
@Serializable data class BookSaveResponse( - val isbn: String, - val isSaved: Boolean + val isbn: String = "", + val isSaved: Boolean = false )추가로, 서버 응답 키가 스네이크 케이스라면(예: is_saved), @SerialName로 매핑이 필요한지 확인 부탁드립니다. 서버 스키마와 필드명 일치 여부도 함께 점검 부탁드립니다.
app/src/main/java/com/texthip/thip/data/model/book/response/RecentSearchResponse.kt (1)
6-14: 기본값 추가로 역직렬화 안정성 강화서버가 특정 필드를 누락할 때를 대비해 기본값을 두는 것을 권장합니다. 리스트는 emptyList(), 문자열은 빈 문자열이 안전합니다.
@Serializable data class RecentSearchResponse( - val recentSearchList: List<RecentSearchItem> + val recentSearchList: List<RecentSearchItem> = emptyList() ) @Serializable data class RecentSearchItem( - val recentSearchId: Int, - val searchTerm: String + val recentSearchId: Int, + val searchTerm: String = "" )또한 recentSearchId가 서버에서 Long(또는 숫자 범위가 큰 타입)일 수 있습니다. 서버 스키마 타입(Int vs Long) 정합성 확인을 부탁드립니다.
app/src/main/java/com/texthip/thip/ui/search/component/SearchBookFilteredResult.kt (2)
69-73: 문구/리소스 일관성 제안stringResource(R.string.group_searched_room_size)는 "그룹" 도메인 문자열로 보입니다. "책 검색 결과" 전용 문자열 리소스를 분리해 도메인 일관성을 유지하는 것을 권장합니다.
103-110: Divider 컴포넌트 사용으로 가독성/일관성 개선수동 Spacer+background 대신 Material3 Divider 사용을 권장합니다. 동일 두께/색상 지정 가능합니다.
- if (index < bookList.size - 1) { - Spacer( - modifier = Modifier - .fillMaxWidth() - .height(1.dp) - .background(colors.DarkGrey02) - ) - } + if (index < bookList.size - 1) { + androidx.compose.material3.Divider( + color = colors.DarkGrey02, + thickness = 1.dp + ) + }추가로 androidx.compose.material3.Divider 임포트가 필요합니다.
app/src/main/java/com/texthip/thip/ui/navigator/extensions/CommonNavigationExtensions.kt (1)
39-42: 중복 네비게이션 방지: launchSingleTop 설정 제안빠른 연타/중복 클릭 시 동일 화면이 스택에 중복 쌓일 수 있습니다. launchSingleTop 옵션으로 방지하는 것을 권장합니다.
아래처럼 수정 제안드립니다.
fun NavHostController.navigateToRegisterBook() { - navigate(CommonRoutes.RegisterBook) + navigate(CommonRoutes.RegisterBook) { + launchSingleTop = true + } }app/src/main/java/com/texthip/thip/ui/navigator/routes/CommonRoutes.kt (1)
5-12: 간단 KDoc 추가로 의도 명확화 제안Route의 용도(진입 경로/딥링크 여부 등)를 KDoc으로 남겨두면 유지보수 시 도움이 됩니다.
예시:
- RegisterBook: “검색 → 책 등록 요청 화면으로 이동”
- Alarm: “알림 목록 화면”
app/src/main/java/com/texthip/thip/data/model/book/response/BookDetailResponse.kt (1)
6-16: 역직렬화 탄력성 확보(기본값/nullable) 제안백엔드 필드 누락 시 크래시를 방지하려면 기본값을 두거나 nullable로 선언하는 것을 권장합니다. 팀 규약에 따라 채택을 검토해 주세요.
예시(기본값 부여):
@Serializable data class BookDetailResponse( - val title: String, - val imageUrl: String, - val authorName: String, - val publisher: String, - val isbn: String, - val description: String, - val recruitingRoomCount: Int, - val readCount: Int, - val isSaved: Boolean + val title: String = "", + val imageUrl: String = "", + val authorName: String = "", + val publisher: String = "", + val isbn: String = "", + val description: String = "", + val recruitingRoomCount: Int = 0, + val readCount: Int = 0, + val isSaved: Boolean = false )app/src/main/java/com/texthip/thip/data/model/book/request/BookSaveRequest.kt (1)
6-8: 불명확한 필드명 명확화 제안(@SerialName 유지)Boolean 필드명이 type이면 의미가 모호합니다. 도메인 의미에 맞게 save/shouldSave 등으로 이름을 바꾸고, 서버 키를 유지하려면 @SerialName("type")을 사용하세요.
적용 예:
package com.texthip.thip.data.model.book.request import kotlinx.serialization.Serializable +import kotlinx.serialization.SerialName @Serializable data class BookSaveRequest( - val type: Boolean + @SerialName("type") + val save: Boolean )주의: 저장소/서비스/VM의 파라미터명도 함께 변경해야 합니다.
app/src/main/java/com/texthip/thip/ui/navigator/routes/SearchRoutes.kt (1)
7-11: 타입드 네비게이션 라우트 추가는 적절함. R8/Proguard 환경 안정성을 위해 @keep 추가 권장
- @serializable를 라우트 서브타입(data class)들에 붙인 점은 Jetpack Navigation Typed API 사용에 적합합니다.
- 릴리즈 빌드에서 난독화가 켜져 있을 경우, 라우트 타입의 FQCN/메타데이터가 바뀌어 경로 직렬화가 실패하는 이슈를 방지하려면 @keep를 함께 부여하는 것을 권장합니다.
적용 diff(선택):
- @Serializable + @Keep + @Serializable data class BookDetail(val isbn: String) : SearchRoutes() - @Serializable + @Keep + @Serializable data class BookGroup(val isbn: String) : SearchRoutes()파일 상단 import 추가(선택):
import androidx.annotation.Keepapp/src/main/java/com/texthip/thip/ui/common/screen/RegisterBookScreen.kt (1)
47-48: Spacer는 padding 대신 height/width 사용 권장의도한 간격 확보에는 padding보다 height/width가 더 명시적입니다.
- Spacer(modifier = Modifier.padding(top = 8.dp)) + Spacer(modifier = Modifier.height(8.dp))app/src/main/java/com/texthip/thip/ui/navigator/extensions/SearchNavigationExtensions.kt (1)
17-19: 그룹 화면 네비게이션에도 launchSingleTop 적용 권장fun NavHostController.navigateToBookGroup(isbn: String) { - navigate(SearchRoutes.BookGroup(isbn = isbn)) + navigate( + SearchRoutes.BookGroup(isbn = isbn), + navOptions { launchSingleTop = true } + ) }app/src/main/java/com/texthip/thip/ui/navigator/navigations/SearchNavigation.kt (1)
41-47: TODO 남김: 우측 버튼/피드 작성 네비게이션 연결우측 버튼/피드 작성으로의 이동이 미구현 상태입니다. 릴리즈 범위에 포함된다면 이 TODO는 남기지 말고 라우트/인텐트 연결을 완료해 주세요. 필요 시 네비게이션 익스텐션(navigateToWriteFeed 등) 추가를 도와드릴 수 있습니다.
app/src/main/java/com/texthip/thip/ui/search/component/SearchActiveField.kt (2)
70-76: 구분선은 Spacer 대신 Divider 사용 권장의도상 구분선이므로 Material3 Divider가 더 적합합니다.
변경 예시(동일 위치 적용):
- Spacer( - modifier = Modifier - .fillMaxWidth() - .height(1.dp) - .background(colors.DarkGrey02) - ) + Divider( + modifier = Modifier.fillMaxWidth(), + thickness = 1.dp, + color = colors.DarkGrey02 + )추가 import:
import androidx.compose.material3.Divider
80-95: isLoading 의미 명확화(페이지네이션 로딩 여부 구분)하단 인디케이터를 제어하는 플래그가 초기 로딩과 혼재될 경우 UX가 어색해질 수 있습니다. 이 컴포넌트의 매개변수명을 isLoadingMore로 변경하거나, 초기 로딩은 상위에서 별도 로더로 처리하고 여기서는 페이지네이션 로딩만 표현하는 것이 명확합니다.
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookUiState.kt (1)
37-37: canLoadMore 조건에 결과 존재 여부 포함 고려초기 상태에서 hasMorePages 기본값이 true인 경우, 빈 결과에도 canLoadMore가 true가 될 수 있습니다. 무한 스크롤 조기 트리거를 방지하려면 hasResults를 포함시키는 것을 고려해 주세요.
예시:
- val canLoadMore: Boolean get() = hasMorePages && !isSearching && !isLoadingMore + val canLoadMore: Boolean get() = hasMorePages && !isSearching && !isLoadingMore && hasResultsapp/src/main/java/com/texthip/thip/ui/search/viewmodel/BookDetailViewModel.kt (1)
22-41: StateFlow 업데이트는 update 확장 함수 사용으로 일관성/안전성 강화중복 읽기-쓰기 패턴 대신 MutableStateFlow.update를 사용하면 가독성과 원자성이 좋아집니다.
적용 예시(부분):
import kotlinx.coroutines.flow.update // 로딩 시작 _uiState.update { it.copy(isLoading = true, error = null) } // 성공 _uiState.update { it.copy(bookDetail = bookDetail, isLoading = false, error = null) } // 실패 _uiState.update { it.copy(isLoading = false, error = exception.message ?: "책 정보를 불러오는데 실패했습니다.") }Also applies to: 43-66
app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookGroupScreen.kt (2)
59-71: UI 매핑 로직 분리 제안UI에서 도메인→UI 모델 매핑을 직접 수행하면 테스트/재사용성이 떨어집니다. ViewModel 또는 별도 Mapper에 위임하는 것을 권장합니다.
114-125: 사용자 노출 에러 메시지 정제 필요exception.message를 그대로 노출하면 기술적인 메시지가 사용자에게 보여질 수 있습니다. 공통 에러 문구(stringResource)로 매핑하거나, 디버그 빌드에서만 상세 메시지를 노출하는 분기를 권장합니다.
app/src/main/java/com/texthip/thip/data/repository/BookRepository.kt (1)
43-47: 하드코딩된 기본값 대신 상수 사용 권장
type = "BOOK"기본값이 하드코딩되어 있습니다. 유지보수성을 위해 상수로 정의하는 것이 좋습니다.파일 상단에 companion object를 추가하여 상수를 정의하세요:
@Singleton class BookRepository @Inject constructor( private val bookService: BookService ) { + companion object { + private const val DEFAULT_SEARCH_TYPE = "BOOK" + } /** 저장된 책 또는 모임 책 목록 조회 */그리고 메서드를 수정하세요:
-suspend fun getRecentSearches(type: String = "BOOK"): Result<RecentSearchResponse?> = runCatching { +suspend fun getRecentSearches(type: String = DEFAULT_SEARCH_TYPE): Result<RecentSearchResponse?> = runCatching {app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookGroupViewModel.kt (1)
26-40: 불필요한 viewModelScope.launch 래핑
loadRecruitingRooms메서드에서viewModelScope.launch를 사용하고 있지만, 내부에서 suspend 함수를 호출하지 않고 다시loadRooms를 호출하므로 불필요한 래핑입니다.fun loadRecruitingRooms(isbn: String) { currentIsbn = isbn - viewModelScope.launch { - _uiState.value = _uiState.value.copy( - isLoading = true, - error = null, - recruitingRooms = emptyList(), - nextCursor = null, - hasMore = true, - totalCount = 0 - ) - + _uiState.value = _uiState.value.copy( + isLoading = true, + error = null, + recruitingRooms = emptyList(), + nextCursor = null, + hasMore = true, + totalCount = 0 + ) + + viewModelScope.launch { loadRooms(isbn, null) } }app/src/main/java/com/texthip/thip/data/service/BookService.kt (1)
28-32: 동일한 엔드포인트 경로 사용으로 인한 혼동 가능성
getBooks와searchBooks모두@GET("books")엔드포인트를 사용합니다. 쿼리 파라미터로 구분되지만, 명확성을 위해 주석을 추가하는 것이 좋습니다./** 책 검색 */ @GET("books") suspend fun searchBooks( + // Note: Same endpoint as getBooks, differentiated by query params @Query("keyword") keyword: String, @Query("page") page: Int = 1 ): BaseResponse<BookSearchResponse>app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt (1)
41-41: 라이브 검색 딜레이를 상수로 정의1000ms 딜레이가 하드코딩되어 있습니다. 유지보수성과 가독성을 위해 상수로 정의하는 것이 좋습니다.
클래스 상단에 companion object를 추가하세요:
@HiltViewModel class SearchBookViewModel @Inject constructor( private val bookRepository: BookRepository ) : ViewModel() { + + companion object { + private const val LIVE_SEARCH_DELAY_MS = 1000L + } private val _uiState = MutableStateFlow(SearchBookUiState())그리고 딜레이를 사용하는 부분을 수정하세요:
- delay(1000) // Live search에 딜레이 추가 + delay(LIVE_SEARCH_DELAY_MS) // Live search에 딜레이 추가app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookScreen.kt (2)
51-63: DisposableEffect의 키 파라미터 누락
DisposableEffect에lifecycleOwner를 키로 전달했지만, 실제로는 변경되지 않는 값입니다. Unit을 사용하는 것이 더 명확합니다.- DisposableEffect(lifecycleOwner) { + DisposableEffect(Unit) { val observer = LifecycleEventObserver { _, event -> if (event == Lifecycle.Event.ON_RESUME) { viewModel.refreshData() } } lifecycleOwner.lifecycle.addObserver(observer) onDispose { lifecycleOwner.lifecycle.removeObserver(observer) } }
180-180: 날짜 포맷 하드코딩날짜 포맷이 하드코딩되어 있습니다. 일관성을 위해 상수로 정의하거나 리소스로 관리하는 것이 좋습니다.
파일 상단에 상수를 정의하세요:
private const val POPULAR_BOOK_DATE_FORMAT = "MM.dd"그리고 사용 부분을 수정하세요:
-popularBookDate = SimpleDateFormat("MM.dd", Locale.getDefault()).format(Date()), +popularBookDate = SimpleDateFormat(POPULAR_BOOK_DATE_FORMAT, Locale.getDefault()).format(Date()),app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookDetailScreen.kt (3)
145-152: 알림바 표시 타이밍: LaunchedEffect 키 조정 필요
LaunchedEffect(Unit)는 이 컴포저블이 새로 컴포즈될 때만 1회 수행됩니다. 다른 ISBN으로 전환 시 재실행을 의도했다면 키를bookDetail?.isbn으로 두는 편이 안전합니다.적용 제안(diff):
- LaunchedEffect(Unit) { + LaunchedEffect(bookDetail?.isbn) { if (!isLoading && error == null && bookDetail != null) { isAlarmVisible = true delay(5000) isAlarmVisible = false } }
340-345: 북마크 토글: 낙관적 업데이트 실패 시 롤백/오류 처리 필요클릭 시 즉시
isBookmarked를 토글하고, 상위에서viewModel.saveBook(isbn, newState)를 호출하지만, 실패 시 롤백/피드백 처리(스낵바, 토스트 등)가 없습니다. 네트워크 실패에 대비한 오류 처리와 상태 롤백을 고려해 주세요.권장 패턴:
- 클릭 시 로컬 상태를 토글한 뒤 ViewModel에 요청
- 실패 시 로컬 상태를 원복하고 사용자에게 알림
- 진행 중 중복 클릭 방지(디바운스/로딩 상태 표시)
ViewModel의 반환(성공/실패 신호) 인터페이스를 알려주시면 구체적인 코드 제안을 드리겠습니다.Also applies to: 113-116
405-409: FilterButton 절대 오프셋(462.dp) 의존
top = 462.dp는 상단 이미지 높이(420.dp)와 추가 여백을 상수로 가정하는 것으로, 다양한 화면/폰트/상태에서 어긋날 수 있습니다. 해당 위치 고정이 필요하다면, 기준이 되는 레이아웃(예: 상단 이미지 Box) 내부에 배치해align과 상대 padding으로 해결하거나, 측정 결과에 기반한 배치를 고려해 주세요.원칙:
- 가능한 상수 오프셋 대신 부모 레이아웃의 스코프 정렬(BoxScope.align) + margin 사용
- 필요 시
SubcomposeLayout/onGloballyPositioned로 동적 측정 후 배치
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
app/src/main/java/com/texthip/thip/data/model/book/request/BookSaveRequest.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/response/BookDetailResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/response/BookSaveResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/response/BookSearchResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/response/MostSearchedBooksResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/response/RecentSearchResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/response/RecruitingRoomsResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/repository/BookRepository.kt(2 hunks)app/src/main/java/com/texthip/thip/data/service/BookService.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/common/buttons/GenreChipButton.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/common/screen/RegisterBookScreen.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/navigator/extensions/CommonNavigationExtensions.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/navigator/extensions/SearchNavigationExtensions.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/navigations/CommonNavigation.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/navigator/navigations/SearchNavigation.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/routes/CommonRoutes.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/routes/SearchRoutes.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/search/component/SearchActiveField.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/search/component/SearchBookFilteredResult.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/search/mock/BookData.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookDetailScreen.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookGroupScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookScreen.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/search/viewmodel/BookDetailViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookGroupViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookUiState.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-01T07:19:10.981Z
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#30
File: app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoomSmall.kt:108-112
Timestamp: 2025-07-01T07:19:10.981Z
Learning: The user rbqks529 prefers to extract hardcoded UI strings to StringResource files (strings.xml) rather than keeping them inline in Compose components, which is a good practice for maintainability and localization in Android development.
Applied to files:
app/src/main/java/com/texthip/thip/ui/common/buttons/GenreChipButton.kt
🧬 Code Graph Analysis (7)
app/src/main/java/com/texthip/thip/ui/navigator/navigations/CommonNavigation.kt (1)
app/src/main/java/com/texthip/thip/ui/common/screen/RegisterBookScreen.kt (1)
RegisterBookScreen(21-61)
app/src/main/java/com/texthip/thip/ui/search/component/SearchBookFilteredResult.kt (2)
app/src/main/java/com/texthip/thip/ui/search/component/SearchEmptyResult.kt (1)
SearchEmptyResult(21-55)app/src/main/java/com/texthip/thip/ui/common/cards/CardBookList.kt (1)
CardBookList(30-89)
app/src/main/java/com/texthip/thip/ui/search/component/SearchActiveField.kt (1)
app/src/main/java/com/texthip/thip/ui/common/cards/CardBookList.kt (1)
CardBookList(30-89)
app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookGroupScreen.kt (3)
app/src/main/java/com/texthip/thip/ui/common/topappbar/DefaultTopAppBar.kt (1)
DefaultTopAppBar(22-65)app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoom.kt (1)
CardItemRoom(35-170)app/src/main/java/com/texthip/thip/ui/theme/Theme.kt (1)
ThipTheme(41-69)
app/src/main/java/com/texthip/thip/ui/navigator/navigations/SearchNavigation.kt (3)
app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookScreen.kt (1)
SearchBookScreen(41-118)app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookDetailScreen.kt (1)
SearchBookDetailScreen(55-120)app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookGroupScreen.kt (1)
SearchBookGroupScreen(45-87)
app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookDetailScreen.kt (6)
app/src/main/java/com/texthip/thip/ui/common/topappbar/GradationTopAppBar.kt (1)
GradationTopAppBar(24-72)app/src/main/java/com/texthip/thip/ui/common/topappbar/DefaultTopAppBar.kt (1)
DefaultTopAppBar(22-65)app/src/main/java/com/texthip/thip/ui/common/buttons/ActionMediumButton.kt (1)
ActionMediumButton(31-80)app/src/main/java/com/texthip/thip/ui/common/buttons/FilterButton.kt (1)
FilterButton(37-128)app/src/main/java/com/texthip/thip/ui/common/modal/InfoPopup.kt (1)
InfoPopup(31-103)app/src/main/java/com/texthip/thip/ui/theme/Theme.kt (1)
ThipTheme(41-69)
app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookScreen.kt (5)
app/src/main/java/com/texthip/thip/ui/search/component/SearchRecentBook.kt (1)
SearchRecentBook(30-138)app/src/main/java/com/texthip/thip/ui/search/component/SearchActiveField.kt (1)
SearchActiveField(29-97)app/src/main/java/com/texthip/thip/ui/search/component/SearchEmptyResult.kt (1)
SearchEmptyResult(21-55)app/src/main/java/com/texthip/thip/ui/search/component/SearchBookFilteredResult.kt (1)
SearchBookFilteredResult(36-133)app/src/main/java/com/texthip/thip/ui/theme/Theme.kt (1)
ThipTheme(41-69)
🪛 detekt (1.23.8)
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt
[warning] 191-191: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 211-211: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 227-227: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🔇 Additional comments (19)
app/src/main/java/com/texthip/thip/ui/common/buttons/GenreChipButton.kt (1)
11-11: height import 추가 적절
.height(40.dp)사용에 맞춰 import가 추가되었습니다. 문제 없습니다.app/src/main/java/com/texthip/thip/ui/search/mock/BookData.kt (1)
9-10: 데이터 클래스 변경의 파급 영향 확인 필요(동등성/구조분해/copy)isbn 필드 추가로 equals/hashCode, componentN, copy 시그니처가 변경되었습니다. 구조분해나 copy를 사용하는 호출부가 있다면 기대 동작이 유지되는지 확인해 주세요. 기본값이 있어 컴파일 호환성은 유지되지만, 런타임 로직 영향 가능성이 있습니다.
app/src/main/java/com/texthip/thip/data/model/book/response/BookSearchResponse.kt (1)
6-23: 페이징 메타데이터 타입/기준 검증 요청
- totalElements가 대형 데이터셋에서 Int 범위를 넘어갈 수 있습니다. Long 사용 필요 여부를 서버 스키마로 확인해 주세요.
- page가 0-기반인지 1-기반인지(서버 기준) 확인 후 ViewModel/Repository 페이징 로직과 일치시켜 주세요. 불일치 시 오프셋 오류가 발생합니다.
app/src/main/java/com/texthip/thip/ui/navigator/extensions/CommonNavigationExtensions.kt (1)
5-5: 올바른 의존성 추가 확인RegisterBook 경로 사용을 위한 CommonRoutes import 추가는 적절합니다.
app/src/main/java/com/texthip/thip/ui/navigator/routes/CommonRoutes.kt (1)
11-11: 새로운 Route 추가 적합Typed Navigation(제네릭 composable)과 직렬화 전략에 맞춰 data object RegisterBook 추가가 잘 되어 있습니다.
app/src/main/java/com/texthip/thip/ui/navigator/navigations/CommonNavigation.kt (2)
11-11: 필요한 컴포저블 import 적절RegisterBookScreen import 추가는 변경 내용과 일관됩니다.
31-36: LGTM: 신규 화면 라우팅 연결 적절Typed Navigation을 사용한 composable<CommonRoutes.RegisterBook> 등록과 onLeftClick에 navigateBack 전달이 일관적입니다.
app/src/main/java/com/texthip/thip/data/model/book/request/BookSaveRequest.kt (1)
6-8: 서버 계약 재확인 필요: type=true/false 의미true가 “저장”인지 “해제”인지, 혹은 토글을 의미하는지 명확히 해주세요. 클라이언트-서버 간 오해로 반대 동작이 발생할 수 있습니다. 명확해지면 이름(예: save/unsave 혹은 action)을 확정하시길 권장합니다.
검증 항목:
- API 문서에서 요청 본문 예시와 의미 확인
- 서버 응답(BookSaveResponse.isSaved)와의 일관성 확인
app/src/main/java/com/texthip/thip/ui/common/screen/RegisterBookScreen.kt (2)
22-25: API 확장 LGTM: onLeftClick 기본 인자 추가로 하위 호환성 유지
- 기본값을 둔 콜백 파라미터 추가로 기존 호출부 영향 없이 네비게이션 뒤로가기 등을 유연하게 연결할 수 있게 되었습니다.
34-35: TopAppBar 콜백 연결 적절함
- DefaultTopAppBar에 외부 콜백을 그대로 전달하는 배선이 명확하고 간결합니다.
app/src/main/java/com/texthip/thip/ui/navigator/navigations/SearchNavigation.kt (2)
19-26: 콜백 기반 네비게이션 분리, 좋습니다SearchBookScreen에서 NavHostController 의존을 제거하고 콜백으로 네비게이션을 위임한 구조가 깔끔합니다. 테스트 용이성과 재사용성이 좋아졌습니다.
6-15: Typed Routes 및 네비게이션 익스텐션 확인 완료
SearchRoutes와GroupRoutes서브클래스에 모두@Serializable가 선언되어 있으며,
익스텐션 함수navigateToBookDetail(isbn: String),navigateToBookGroup(isbn: String),navigateToGroupRecruit(roomId: Int)의 파라미터 타입이 각 라우트 데이터 클래스 필드와 정확히 일치함을 검증했습니다.
런타임 파싱/네비게이션 오류는 발생하지 않습니다.app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookUiState.kt (1)
35-40: UI 상태 모델 구성 적절합니다파생 프로퍼티(hasResults, canLoadMore, showEmptyState, showInitialScreen)로 화면 분기를 명확히 표현한 점 좋습니다. 페이징/검색 상태의 상호 배제 로직도 적절합니다.
app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookGroupScreen.kt (1)
55-57: 중복 호출 없음 확인:SearchBookGroupViewModelinit 블록에loadRecruitingRooms미호출
SearchBookGroupViewModel클래스에는init { … }블록이 존재하지 않으며, ViewModel 초기화 시점에loadRecruitingRooms를 호출하는 코드가 없습니다.
따라서 화면의 LaunchedEffect(isbn)에서만 API 요청이 발생하며, 중복 호출 우려는 없습니다.Likely an incorrect or invalid review comment.
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookGroupViewModel.kt (1)
57-57: 페이지네이션 시 빈 리스트 대신 기존 데이터 유지초기 로드가 아닌 경우(
cursor != null) 기존 룸 리스트를 가져오는 로직이 있지만, 더 명확하게 표현할 수 있습니다.app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt (1)
177-195: 로딩 실패 시 조용한 처리에 대한 검토 필요인기 책 로딩이 실패해도 사용자에게 알리지 않습니다. 중요한 기능이라면 에러 상태를 UI에 반영하는 것을 고려해보세요.
인기 책 로딩이 실제로 중요한 기능인지 확인이 필요합니다. 만약 중요한 기능이라면 에러 처리를 추가하는 것이 좋습니다.
app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookDetailScreen.kt (3)
198-211: Coil AsyncImage 사용과 폴백/에러 플레이스홀더 설정 좋습니다실제 커버 URL 로딩, 블러 효과, fallback/error 처리까지 깔끔하게 구성되어 안정적인 사용자 경험을 제공합니다.
154-160: 북마크 상태 동기화 LaunchedEffect 적절서버 상태 변화 시
bookDetail.isSaved를 소스로 하여 로컬isBookmarked를 동기화하는 접근은 일관성을 보장합니다.
55-65: 화면 진입 시 데이터 로드 플로우 구성 적절
isbn키로LaunchedEffect에서 단발 로드를 트리거하고,uiState에 따라 로딩/에러/성공을 분기하는 구조가 명확합니다. 프리뷰에서는 Content만 사용하여 Hilt 종속성을 우회하는 점도 좋습니다.Also applies to: 68-71, 102-119
| data class BookDetailResponse( | ||
| val title: String, | ||
| val imageUrl: String, | ||
| val authorName: String, | ||
| val publisher: String, | ||
| val isbn: String, | ||
| val description: String, | ||
| val recruitingRoomCount: Int, | ||
| val readCount: Int, | ||
| val isSaved: Boolean | ||
| ) No newline at end of file |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
API 필드 네이밍 규칙 검증 필요 (@SerialName 필요 여부)
서버 응답이 snake_case(예: image_url)라면 @SerialName 또는 글로벌 네이밍 전략 설정이 필요합니다. 현재는 camelCase로 선언되어 있어 직렬화 설정에 따라 역직렬화 실패가 발생할 수 있습니다.
다음 스크립트로 코드베이스에서 전역 네이밍 전략 혹은 @SerialName 사용 여부를 확인해 주세요. 예상:
- namingStrategy 설정이 있다면 @SerialName 없이도 동작.
- 없다면 각 필드에 @SerialName 지정 필요.
🏁 Script executed:
#!/bin/bash
# 전역 Json 설정에서 namingStrategy / ignoreUnknownKeys 등을 사용하는지 탐색
rg -n "Json\s*\{[^\}]*\}" -A 5
# kotlinx.serialization @SerialName 사용 여부 확인
rg -n "@SerialName\("
# Retrofit/Ktor 등에서 kotlinx.serialization converter 설정 파일 탐색
fd -t f -a | rg -n "ConverterFactory|kotlinx\.serialization|Json\(.*\)"Length of output: 11023
BookDetailResponse에 @SerialName 어노테이션 추가 필요
프로젝트 전반에서 전역 네이밍 전략(Json { namingStrategy }) 설정이 발견되지 않으며, 기존의 모든 Request/Response 모델에서는 각 필드마다 @SerialName을 명시하고 있습니다. 따라서 BookDetailResponse에서도 일관성을 유지하기 위해 각 프로퍼티에 @SerialName을 추가해야 합니다.
파일:
• app/src/main/java/com/texthip/thip/data/model/book/response/BookDetailResponse.kt
수정 대상 필드(추가할 어노테이션):
- title
- imageUrl
- authorName
- publisher
- isbn
- description
- recruitingRoomCount
- readCount
- isSaved
예시 diff:
data class BookDetailResponse(
- val title: String,
- val imageUrl: String,
- val authorName: String,
- val publisher: String,
- val isbn: String,
- val description: String,
- val recruitingRoomCount: Int,
- val readCount: Int,
- val isSaved: Boolean
+ @SerialName("title") val title: String,
+ @SerialName("imageUrl") val imageUrl: String,
+ @SerialName("authorName") val authorName: String,
+ @SerialName("publisher") val publisher: String,
+ @SerialName("isbn") val isbn: String,
+ @SerialName("description") val description: String,
+ @SerialName("recruitingRoomCount") val recruitingRoomCount: Int,
+ @SerialName("readCount") val readCount: Int,
+ @SerialName("isSaved") val isSaved: Boolean
)📝 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.
| data class BookDetailResponse( | |
| val title: String, | |
| val imageUrl: String, | |
| val authorName: String, | |
| val publisher: String, | |
| val isbn: String, | |
| val description: String, | |
| val recruitingRoomCount: Int, | |
| val readCount: Int, | |
| val isSaved: Boolean | |
| ) | |
| data class BookDetailResponse( | |
| @SerialName("title") val title: String, | |
| @SerialName("imageUrl") val imageUrl: String, | |
| @SerialName("authorName") val authorName: String, | |
| @SerialName("publisher") val publisher: String, | |
| @SerialName("isbn") val isbn: String, | |
| @SerialName("description") val description: String, | |
| @SerialName("recruitingRoomCount") val recruitingRoomCount: Int, | |
| @SerialName("readCount") val readCount: Int, | |
| @SerialName("isSaved") val isSaved: Boolean | |
| ) |
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/data/model/book/response/BookDetailResponse.kt
around lines 6 to 16, add @SerialName annotations to every property to match the
expected JSON field names (use snake_case for multiword names). Specifically
annotate title, imageUrl, authorName, publisher, isbn, description,
recruitingRoomCount, readCount and isSaved with @SerialName("title"),
@SerialName("image_url"), @SerialName("author_name"), @SerialName("publisher"),
@SerialName("isbn"), @SerialName("description"),
@SerialName("recruiting_room_count"), @SerialName("read_count"), and
@SerialName("is_saved") respectively so the data class fields map correctly to
the JSON payload.
app/src/main/java/com/texthip/thip/data/model/book/response/BookSearchResponse.kt
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/data/model/book/response/MostSearchedBooksResponse.kt
Show resolved
Hide resolved
| data class PopularBookItem( | ||
| val rank: Int, | ||
| val title: String, | ||
| val imageUrl: String, | ||
| val isbn: String | ||
| ) No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
아이템 필드에도 기본값 추가 권장 + 타입/네이밍 일관성 점검
- 문자열/정수 필드에 기본값을 지정하면 필드 누락 시 복구가 가능합니다.
- 네이밍은 PopularBookItem 대신 MostSearchedBookItem이 응답명과 더 일관될 수 있습니다(선택).
data class PopularBookItem(
- val rank: Int,
- val title: String,
- val imageUrl: String,
- val isbn: String
+ val rank: Int = 0,
+ val title: String = "",
+ val imageUrl: String = "",
+ val isbn: String = ""
)주의: 서버가 snake_case(예: image_url, isbn13)를 사용한다면 @SerialName으로 매핑해야 합니다(검증 후 적용).
📝 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.
| data class PopularBookItem( | |
| val rank: Int, | |
| val title: String, | |
| val imageUrl: String, | |
| val isbn: String | |
| ) | |
| data class PopularBookItem( | |
| val rank: Int = 0, | |
| val title: String = "", | |
| val imageUrl: String = "", | |
| val isbn: String = "" | |
| ) |
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/data/model/book/response/MostSearchedBooksResponse.kt
around lines 11-16, the data class fields should have safe defaults and
naming/type consistency: add default values for rank (e.g., 0) and strings
(e.g., empty ""), consider renaming PopularBookItem to MostSearchedBookItem to
match the response name, and if the server uses snake_case (image_url, isbn13)
add @SerialName annotations mapping each JSON name to the Kotlin property names;
update imports for kotlinx.serialization if needed and ensure types (Int/String)
match the API contract.
| data class RecruitingRoomsResponse( | ||
| val recruitingRoomList: List<RecruitingRoomItem>, | ||
| val totalRoomCount: Int, | ||
| val nextCursor: String?, | ||
| val isLast: Boolean | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
페이징 응답 내구성 강화를 위한 기본값 지정 권장
- 페이징 메타 필드가 누락되면 크래시 위험이 있습니다. 기본값을 지정해 방어적으로 처리하는 것을 권장합니다.
data class RecruitingRoomsResponse(
- val recruitingRoomList: List<RecruitingRoomItem>,
- val totalRoomCount: Int,
- val nextCursor: String?,
- val isLast: Boolean
+ val recruitingRoomList: List<RecruitingRoomItem> = emptyList(),
+ val totalRoomCount: Int = 0,
+ val nextCursor: String? = null,
+ val isLast: Boolean = false
)🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/data/model/book/response/RecruitingRoomsResponse.kt
around lines 6 to 11, the paging meta fields (totalRoomCount, nextCursor,
isLast) are non-nullable without defaults which can lead to crashes if omitted;
make the response defensive by providing sensible default values (e.g.,
totalRoomCount = 0, nextCursor = null is already nullable but ensure default,
isLast = true/false as appropriate) in the data class declaration so callers
deserializing partial responses won't crash and the model remains stable.
| bookRepository.getBookDetail(isbn) | ||
| .onSuccess { bookDetail -> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Repository 호출의 Dispatcher 확인 권장
viewModelScope는 Main을 기본으로 사용하므로, Repository 내부에서 IO Dispatcher로 전환하지 않으면 메인 스레드 작업이 될 수 있습니다.
아래 스크립트로 BookRepository 구현에서 IO 컨텍스트 사용 여부를 확인해 주세요:
Also applies to: 47-48
🏁 Script executed:
#!/bin/bash
# BookRepository 구현 검색
rg -n "class\s+BookRepository|interface\s+BookRepository" -A 200
# suspend 구현부에서 withContext(Dispatchers.IO) 또는 적절한 dispatcher 사용 여부 확인
rg -n "withContext\s*\(\s*Dispatchers\.IO\s*\)" -SLength of output: 6468
Repository 메서드에 IO Dispatcher 적용 필요
ViewModelScope는 기본적으로 Main 디스패처에서 실행되므로, Repository 내부의 suspend 함수들을 명시적으로 IO 디스패처로 전환하지 않으면 네트워크 호출 이후 후속 처리도 메인 스레드에서 수행될 수 있습니다.
대상 위치:
- app/src/main/java/com/texthip/thip/data/repository/BookRepository.kt
suspend fun getBookDetail(isbn: String)(57–61줄)- 이와 유사한 다른 suspend 메서드들 (
getBooks,searchBooks등)
권장 조치:
-suspend fun getBookDetail(isbn: String): Result<BookDetailResponse?> = runCatching {
- bookService.getBookDetail(isbn)
- .handleBaseResponse()
- .getOrThrow()
-}
+suspend fun getBookDetail(isbn: String): Result<BookDetailResponse?> =
+ withContext(Dispatchers.IO) {
+ runCatching {
+ bookService.getBookDetail(isbn)
+ .handleBaseResponse()
+ .getOrThrow()
+ }
+ }이와 같이 네트워크/API 호출을 withContext(Dispatchers.IO)로 래핑하여 메인 스레드 블로킹을 방지해주세요.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/data/repository/BookRepository.kt around
lines 57–61 (and apply similarly to other suspend methods like getBooks and
searchBooks), the repository suspend functions perform network/API work but are
not explicitly switched to the IO dispatcher; wrap the network and IO-bound
parts of these methods with withContext(Dispatchers.IO) so the heavy work runs
on Dispatchers.IO and only lightweight results are returned to the caller,
ensuring you import kotlinx.coroutines.Dispatchers and
kotlinx.coroutines.withContext and keep the public suspend signature unchanged.
There was a problem hiding this comment.
해당 로직은 다 viewModel scope내에서 실행되는데 상관없는거 아니야?
There was a problem hiding this comment.
@rbqks529 좋은 질문이네요! 하지만 여전히 IO Dispatcher가 필요합니다.
ViewModelScope는 기본적으로 Dispatchers.Main을 사용합니다. 즉:
- suspend 함수라고 해서 자동으로 적절한 디스패처로 전환되지 않습니다
- Repository의 네트워크 호출이 명시적으로
withContext(Dispatchers.IO)로 래핑되지 않으면, 메인 스레드에서 네트워크 I/O가 발생할 수 있습니다 - 이는 ANR(Application Not Responsive)을 유발할 수 있습니다
올바른 패턴:
// ViewModel (Main dispatcher)
viewModelScope.launch { // <- Dispatchers.Main
val result = repository.getBookDetail(isbn) // <- 여기서 IO로 전환되어야 함
}
// Repository
suspend fun getBookDetail(isbn: String) = withContext(Dispatchers.IO) {
// 네트워크 호출은 IO 스레드에서
runCatching { bookService.getBookDetail(isbn).handleBaseResponse().getOrThrow() }
}ViewModelScope 내에서 실행된다고 해서 네트워크 작업이 자동으로 백그라운드 스레드로 이동하지는 않습니다. Repository 계층에서 명시적으로 디스패처를 전환해주는 것이 안전하고 권장되는 패턴입니다.
٩(◕‿◕)۶
✏️ Learnings added
Learnt from: rbqks529
PR: THIP-TextHip/THIP-Android#79
File: app/src/main/java/com/texthip/thip/ui/search/viewmodel/BookDetailViewModel.kt:26-27
Timestamp: 2025-08-13T05:42:13.567Z
Learning: ViewModelScope는 기본적으로 Dispatchers.Main을 사용하므로, Repository의 suspend 함수에서 네트워크 호출 시 명시적으로 withContext(Dispatchers.IO)로 전환해야 메인 스레드 블로킹을 방지할 수 있다. suspend 함수라고 해서 자동으로 적절한 디스패처가 사용되지는 않는다.
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.
| .onSuccess { saveResponse -> | ||
| saveResponse?.let { | ||
| // 책 상세 정보의 isSaved 상태 업데이트 | ||
| val updatedBookDetail = _uiState.value.bookDetail?.copy(isSaved = it.isSaved) | ||
| _uiState.value = _uiState.value.copy( | ||
| bookDetail = updatedBookDetail, | ||
| isSaving = false, | ||
| error = null | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
saveBook 성공 응답 null 시 isSaving 플래그가 해제되지 않습니다
saveResponse가 null이면 let 블록이 실행되지 않아 isSaving이 true로 고정되는 버그가 발생합니다. 성공 경로에서도 항상 isSaving을 false로 내려주도록 수정해 주세요.
수정 예시:
- bookRepository.saveBook(isbn, type)
- .onSuccess { saveResponse ->
- saveResponse?.let {
- // 책 상세 정보의 isSaved 상태 업데이트
- val updatedBookDetail = _uiState.value.bookDetail?.copy(isSaved = it.isSaved)
- _uiState.value = _uiState.value.copy(
- bookDetail = updatedBookDetail,
- isSaving = false,
- error = null
- )
- }
- }
+ bookRepository.saveBook(isbn, type)
+ .onSuccess { saveResponse ->
+ val updatedBookDetail = saveResponse?.let {
+ _uiState.value.bookDetail?.copy(isSaved = it.isSaved)
+ } ?: _uiState.value.bookDetail
+ _uiState.value = _uiState.value.copy(
+ bookDetail = updatedBookDetail,
+ isSaving = false,
+ error = 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.
| .onSuccess { saveResponse -> | |
| saveResponse?.let { | |
| // 책 상세 정보의 isSaved 상태 업데이트 | |
| val updatedBookDetail = _uiState.value.bookDetail?.copy(isSaved = it.isSaved) | |
| _uiState.value = _uiState.value.copy( | |
| bookDetail = updatedBookDetail, | |
| isSaving = false, | |
| error = null | |
| ) | |
| } | |
| } | |
| bookRepository.saveBook(isbn, type) | |
| .onSuccess { saveResponse -> | |
| val updatedBookDetail = saveResponse?.let { | |
| _uiState.value.bookDetail?.copy(isSaved = it.isSaved) | |
| } ?: _uiState.value.bookDetail | |
| _uiState.value = _uiState.value.copy( | |
| bookDetail = updatedBookDetail, | |
| isSaving = false, | |
| error = null | |
| ) | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/ui/search/viewmodel/BookDetailViewModel.kt
around lines 48-58, the onSuccess block only updates UI state inside
saveResponse?.let, so when saveResponse is null the isSaving flag remains true;
modify the success handling to always set isSaving = false (and clear error)
even if saveResponse is null — either move the _uiState update outside the let
or add an else branch that sets _uiState.value = _uiState.value.copy(isSaving =
false, error = null) and, if saveResponse is non-null, also update
bookDetail.isSaved accordingly.
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
app/src/main/java/com/texthip/thip/ui/common/buttons/GenreChipButton.kt (1)
62-71: 닫기 아이콘 접근성 및 터치 타깃 강화
- 인터랙티브 아이콘에 contentDescription = null은 접근성 이슈입니다.
- 역할(Role.Button) 부여로 스크린리더 힌트를 개선하세요.
- 크기 지정은 clickable 이전에 두는 편이 직관적입니다. 최소 터치 타깃(권장 48dp)은 디자인 제약이 있으면 24dp 이상으로妥協하는 것도 한 방법입니다.
권장 변경:
Icon( painter = painterResource(R.drawable.ic_x_20), - contentDescription = null, + contentDescription = stringResource(R.string.cd_clear_chip), tint = Color.Unspecified, modifier = Modifier - .clickable { - onCloseClick() - } - .size(20.dp) + .size(20.dp) + .clickable(role = Role.Button) { onCloseClick() } )추가 import:
import androidx.compose.ui.res.stringResource import androidx.compose.ui.semantics.Role문자열 리소스가 없다면 strings.xml에 추가가 필요합니다:
- name: cd_clear_chip
- value(ko): "칩 삭제" 또는 "닫기"
필요하시다면 리소스 추가 PR도 도와드릴게요.
♻️ Duplicate comments (14)
app/src/main/java/com/texthip/thip/ui/common/buttons/GenreChipButton.kt (1)
38-49: 클릭 범위/리플 클리핑/모양(Shape) 불일치 지속 — modifier 순서 정리 및 shape 통일 권장
- clickable이 padding보다 앞에 있어 패딩 영역이 히트 테스트/리플 범위에서 제외됩니다.
- background의 RoundedCornerShape(12.dp) vs border의 RoundedCornerShape(20.dp) 불일치가 여전히 남아있습니다.
- clip은 이미 20.dp로 잘 적용되어 있으니, background/border도 20.dp로 통일하고 clickable을 마지막으로 이동하세요. 접근성을 위해 Role.Button도 부여하세요.
아래와 같이 정리하면 모양/리플/접근성이 모두 개선됩니다.
.height(40.dp) .clip(RoundedCornerShape(20.dp)) - .background(color = Color.Transparent, shape = RoundedCornerShape(12.dp)) - .clickable { - onClick() - } + .background(color = Color.Transparent, shape = RoundedCornerShape(20.dp)) .border( width = 1.dp, color = colors.Grey02, shape = RoundedCornerShape(20.dp) ) - .padding(top = 8.dp, bottom = 8.dp, end = 8.dp, start = 12.dp), + .padding(start = 12.dp, top = 8.dp, end = 8.dp, bottom = 8.dp) + .clickable(role = Role.Button) { onClick() },추가 import:
import androidx.compose.ui.semantics.Roleapp/src/main/java/com/texthip/thip/ui/navigator/navigations/SearchNavigation.kt (2)
31-38: ISBN 빈 문자열 가드 추가 + 뒤로가기는 navigateUp으로 통일하세요딥링크/라우트 파싱 이슈로 isbn이 빈 문자열일 수 있습니다. 빈값이면 즉시 navigateUp으로 빠지는 가드가 필요합니다. 또한 뒤로가기는 popBackStack보다 navigateUp을 권장합니다. (이전 코멘트와 동일 제안)
적용 예시:
val route = backStackEntry.toRoute<SearchRoutes.BookDetail>() val isbn = route.isbn + if (isbn.isBlank()) { + navController.navigateUp() + return@composable + } SearchBookDetailScreen( isbn = isbn, onLeftClick = { - navController.popBackStack() + navController.navigateUp() },
51-59: BookGroup 라우트도 ISBN 가드 + navigateUp으로 통일BookDetail과 동일하게 빈 ISBN 가드와 navigateUp 사용을 권장합니다. (이전 코멘트와 동일 제안)
적용 예시:
val route = backStackEntry.toRoute<SearchRoutes.BookGroup>() val isbn = route.isbn + if (isbn.isBlank()) { + navController.navigateUp() + return@composable + } SearchBookGroupScreen( isbn = isbn, onLeftClick = { - navController.popBackStack() + navController.navigateUp() },app/src/main/java/com/texthip/thip/data/model/book/response/RecruitingRoomsResponse.kt (3)
7-12: 페이징 응답에 기본값 추가한 점 좋습니다부분 응답/스키마 변화에 대한 내구성이 확보되었습니다.
15-22: 아이템 기본값 지정 반영 잘 됐습니다null/missing 필드로 인한 디코딩 실패 가능성을 낮춥니다. roomId를 Int로 유지한 점도 현재 사용처와 일치합니다.
11-11: RecruitingRoomsResponse의 isLast 기본값을 false로 변경해주세요필드 누락 시 기본값이 true일 경우
hasMore = !isLast로직이 즉시 false가 되어 무한 스크롤이 조기 종료될 위험이 있습니다. 방어적 기본값은 false가 더 안전합니다.수정 위치:
- app/src/main/java/com/texthip/thip/data/model/book/response/RecruitingRoomsResponse.kt:11
추천 diff:
- @SerialName("isLast") val isLast: Boolean = true + @SerialName("isLast") val isLast: Boolean = false이후 API 응답에서
isLast가 누락되어도 계속 페이지를 로드하도록 동작이 보장됩니다.app/src/main/java/com/texthip/thip/data/model/book/response/BookSearchResponse.kt (2)
18-24: imageUrl nullable 및 기본값 부여 적절실서비스에서 이미지 누락을 안전하게 처리합니다. UI(CardBookList)가 String?를 수용하므로 호환됩니다.
9-14: BookSearchResponse 페이징 메타 기본값 수정 필요
• Service/Repository는 1-based 페이지를 사용하지만, 데이터 클래스 기본값이 page = 0으로 설정돼 있어 필드 누락 시 오프바이원 위험이 있습니다.
• first/last 기본값을 true로 두면 JSON 필드가 누락될 때 페이징 로직이 곧바로 ‘첫/마지막 페이지’로 잘못 해석될 수 있습니다. 안전한 기본값은 모두 false입니다.다음과 같이 조정을 권장합니다.
--- a/app/src/main/java/com/texthip/thip/data/model/book/response/BookSearchResponse.kt +++ b/app/src/main/java/com/texthip/thip/data/model/book/response/BookSearchResponse.kt @@ - @SerialName("page") val page: Int = 0, + @SerialName("page") val page: Int = 1, @SerialName("size") val size: Int = 0, @SerialName("totalElements") val totalElements: Int = 0, @SerialName("totalPages") val totalPages: Int = 0, - @SerialName("last") val last: Boolean = true, - @SerialName("first") val first: Boolean = true + @SerialName("last") val last: Boolean = false, + @SerialName("first") val first: Boolean = false이 변경은 서버에서 페이징 필드를 정상적으로 내려줄 경우 동작에 영향이 없으며, JSON 필드 누락 시에도 안전한 기본 동작을 보장합니다.
app/src/main/java/com/texthip/thip/ui/search/component/SearchActiveField.kt (2)
46-47: 빈 목록 가드 추가로 조기 트리거 문제 해결됨totalItemsCount > 0 조건으로 초기 onLoadMore 오발동을 방지했습니다.
60-78: LazyColumn 항목에 안정적인 key 지정데이터 갱신 시 스크롤/애니메이션 상태 보존을 위해 고유 키를 지정하세요. isbn이 존재/고유하다면 이를 사용하세요.
- itemsIndexed(bookList) { index, book -> + itemsIndexed(bookList, key = { _, book -> book.isbn }) { index, book -> Column { CardBookList( modifier = Modifier.clickable { onBookClick(book) }, title = book.title, author = book.author, publisher = book.publisher, imageUrl = book.imageUrl ) if (index < bookList.size - 1) { Spacer( modifier = Modifier .fillMaxWidth() .height(1.dp) .background(colors.DarkGrey02) ) } } }isbn이 비어있을 수 있다면 index를 조합하는 등 고유성 확보가 필요합니다.
app/src/main/java/com/texthip/thip/ui/search/component/SearchBookFilteredResult.kt (3)
83-90: LazyColumn 아이템에 안정적인 key 부여로 스크롤/상태 보존 개선현재 itemsIndexed에 key가 없어 스크롤 포지션/내부 상태 보존이 흔들릴 수 있습니다. isbn을 우선 키로, 빈 경우 결정적 대체 키를 사용하세요.
- LazyColumn( + LazyColumn( state = listState, verticalArrangement = Arrangement.spacedBy(12.dp) ) { - itemsIndexed(bookList) { index, book -> + itemsIndexed( + items = bookList, + key = { _, book -> book.isbn.ifBlank { "${book.title}-${book.author}-${book.publisher}" } } + ) { index, book -> Column { CardBookList( modifier = Modifier.clickable { onBookClick(book) },
36-44: 빈 결과 화면의 onRequestBook 콜백 주입이 여전히 누락되어 있습니다 — 시그니처에 추가하세요이 컴포저블에서 빈 결과 상태를 다시 노출할 계획이라면 onRequestBook 콜백을 시그니처에 포함하고 하위 UI로 전달할 수 있어야 합니다.
적용 예시:
fun SearchBookFilteredResult( resultCount: Int, bookList: List<BookData>, isLoading: Boolean = false, hasMore: Boolean = true, onLoadMore: () -> Unit = {}, + onRequestBook: () -> Unit = {}, onBookClick: (BookData) -> Unit = {} ) {
83-123: SearchBookFilteredResult에 Empty-state 복원 및 onRequestBook 파라미터 추가필요 변경 사항
SearchBookFilteredResult시그니처에onRequestBook: () -> Unit = {}추가bookList.isEmpty() && !isLoading조건일 때SearchEmptyResult노출로 빈 결과 UX 복원SearchBookScreen.kt에서SearchBookFilteredResult호출부에onRequestBook = onRequestBook전달수정 예시
--- a/app/src/main/java/com/texthip/thip/ui/search/component/SearchBookFilteredResult.kt +++ b/app/src/main/java/com/texthip/thip/ui/search/component/SearchBookFilteredResult.kt @@ -fun SearchBookFilteredResult( - resultCount: Int, - bookList: List<BookData>, - isLoading: Boolean = false, - hasMore: Boolean = true, - onLoadMore: () -> Unit = {}, - onBookClick: (BookData) -> Unit = {} -) { +fun SearchBookFilteredResult( + resultCount: Int, + bookList: List<BookData>, + isLoading: Boolean = false, + hasMore: Boolean = true, + onLoadMore: () -> Unit = {}, + onBookClick: (BookData) -> Unit = {}, + onRequestBook: () -> Unit = {} +) { val listState = rememberLazyListState() … - LazyColumn( + if (bookList.isEmpty() && !isLoading) { + SearchEmptyResult( + mainText = stringResource(R.string.book_no_search_result1), + subText = stringResource(R.string.book_no_search_result2), + onRequestBook = onRequestBook + ) + } else { + LazyColumn( state = listState, verticalArrangement = Arrangement.spacedBy(12.dp) ) { @@ } + } }--- a/app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookScreen.kt +++ b/app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookScreen.kt @@ SearchMode.CompleteSearch -> { - SearchBookFilteredResult( - resultCount = totalElements, - bookList = searchResults, - isLoading = isSearching || isLoadingMore, - hasMore = canLoadMore, - onLoadMore = onLoadMore, - onBookClick = onBookClick - ) + SearchBookFilteredResult( + resultCount = totalElements, + bookList = searchResults, + isLoading = isSearching || isLoadingMore, + hasMore = canLoadMore, + onLoadMore = onLoadMore, + onBookClick = onBookClick, + onRequestBook = onRequestBook + )검증 포인트
- 검색 결과 0건 & 로딩 상태 아님일 때
SearchEmptyResult노출 여부onRequestBook호출 시 네비게이션/요청 흐름 정상 동작 확인app/src/main/java/com/texthip/thip/data/repository/BookRepository.kt (1)
29-33: 페이지네이션 파라미터(page) 검증 누락 — 음수/0 방지 가드 추가/** 책 검색 */ suspend fun searchBooks(keyword: String, page: Int = 1, isFinalized: Boolean = false): Result<BookSearchResponse?> = runCatching { + require(page > 0) { "Page must be positive" } bookService.searchBooks(keyword, page, isFinalized) .handleBaseResponse() .getOrThrow() }
🧹 Nitpick comments (20)
app/src/main/java/com/texthip/thip/ui/common/buttons/GenreChipButton.kt (1)
86-86: 프리뷰의 하드코드 텍스트 OK미리보기 용도로 "책" 하드코드 사용은 무방합니다. 실제 UI에서는 stringResource 사용을 유지해 로컬라이제이션 일관성을 확보하세요.
app/src/main/java/com/texthip/thip/ui/navigator/navigations/SearchNavigation.kt (2)
39-47: TODO 정리 필요: 우측 버튼/피드 작성 동작사용자 액션이 무시되지 않도록 최소한의 피드백(예: 스낵바)이나 TODO에 이슈 키(예: THIP2025-xxx)를 명시해 추적 가능하게 해주세요. 필요 시 네비게이션 확장 추가 구현 도와드릴 수 있습니다.
63-70: 람다 파라미터 섀도잉 해소로 가독성 개선외부 스코프의 isbn과 동일한 이름으로 파라미터를 선언해 섀도잉이 발생합니다. 혼동 방지를 위해 명확한 이름으로 변경을 권장합니다.
- onCreateRoomClick = { isbn, title, imageUrl, author -> + onCreateRoomClick = { bookIsbn, bookTitle, bookImageUrl, bookAuthor -> navController.navigateToGroupMakeRoomWithBook( - isbn = isbn, - title = title, - imageUrl = imageUrl, - author = author + isbn = bookIsbn, + title = bookTitle, + imageUrl = bookImageUrl, + author = bookAuthor ) }app/src/main/java/com/texthip/thip/ui/group/myroom/screen/GroupMyScreen.kt (2)
74-80: 무한 스크롤 조건식: 작은 리스트에서 즉시 연쇄 로드될 수 있어 임계값 보정 권장현재 totalItems가 1~2처럼 작은 경우
totalItems - 3이 음수가 되어 스크롤 없이도 즉시 로드가 트리거됩니다. 의도된 자동 프리페치가 아니라면 임계 인덱스를 0 이상으로 클램핑하는 것이 안전합니다. 또한 remember 키에listState를 포함해 파생 상태 객체 재생성을 보다 명시적으로 제어하는 편이 좋습니다.의도대로 작은 목록에서 자동 추가 로드를 원하시는지 확인 부탁드립니다. 원치 않으시면 아래 패치를 적용하세요.
- val shouldLoadMore by remember(uiState.canLoadMore, uiState.isLoadingMore) { - derivedStateOf { - val lastVisibleIndex = listState.layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: 0 - val totalItems = listState.layoutInfo.totalItemsCount - uiState.canLoadMore && !uiState.isLoadingMore && totalItems > 0 && lastVisibleIndex >= totalItems - 3 - } - } + val shouldLoadMore by remember(listState, uiState.canLoadMore, uiState.isLoadingMore) { + derivedStateOf { + val lastVisibleIndex = listState.layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: -1 + val totalItems = listState.layoutInfo.totalItemsCount + val preloadThreshold = 3 + val triggerIndex = ((totalItems - 1) - preloadThreshold).coerceAtLeast(0) + uiState.canLoadMore && !uiState.isLoadingMore && totalItems > 0 && lastVisibleIndex >= triggerIndex + } + }
82-86: LaunchedEffect 트리거 안정화: snapshotFlow로 중복 트리거/깜빡임 방지
LaunchedEffect(shouldLoadMore)패턴은 상태 변화 타이밍에 따라 드물게 중복 호출 가능성이 있습니다.snapshotFlow { shouldLoadMore }에distinctUntilChanged().filter { it }를 붙여 수집하면 안전합니다.- LaunchedEffect(shouldLoadMore) { - if (shouldLoadMore) { - onLoadMore() - } - } + LaunchedEffect(listState, uiState.canLoadMore, uiState.isLoadingMore) { + snapshotFlow { shouldLoadMore } + .distinctUntilChanged() + .filter { it } + .collect { onLoadMore() } + }아래 임포트를 파일 상단에 추가해 주세요:
import androidx.compose.runtime.snapshotFlow import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.filterapp/src/main/java/com/texthip/thip/ui/group/done/screen/GroupDoneScreen.kt (2)
64-70: 무한 스크롤 조건식: 작은 리스트에서 즉시 로드 방지 및 remember 키 최적화
totalItems - 3가 음수가 될 수 있어 초기 소량 아이템에서 즉시 로드가 발생합니다. 의도된 동작이 아니라면 트리거 인덱스를 0 이상으로 보정하세요. 또한listState를 remember 키에 포함하는 것을 권장합니다.작은 리스트에서 자동 로드를 원하시는지 확인 부탁드립니다. 자동 로드를 원치 않으면 아래와 같이 보정하십시오.
- val shouldLoadMore by remember(uiState.canLoadMore, uiState.isLoadingMore) { - derivedStateOf { - val lastVisibleIndex = listState.layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: 0 - val totalItems = listState.layoutInfo.totalItemsCount - uiState.canLoadMore && !uiState.isLoadingMore && totalItems > 0 && lastVisibleIndex >= totalItems - 3 - } - } + val shouldLoadMore by remember(listState, uiState.canLoadMore, uiState.isLoadingMore) { + derivedStateOf { + val lastVisibleIndex = listState.layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: -1 + val totalItems = listState.layoutInfo.totalItemsCount + val preloadThreshold = 3 + val triggerIndex = ((totalItems - 1) - preloadThreshold).coerceAtLeast(0) + uiState.canLoadMore && !uiState.isLoadingMore && totalItems > 0 && lastVisibleIndex >= triggerIndex + } + }
72-76: snapshotFlow 수집으로 무한 스크롤 트리거 안정화
LaunchedEffect(shouldLoadMore)대신snapshotFlow { shouldLoadMore }를 수집하면 중복 호출을 줄이고 안정적으로 트리거할 수 있습니다.- LaunchedEffect(shouldLoadMore) { - if (shouldLoadMore) { - onLoadMore() - } - } + LaunchedEffect(listState, uiState.canLoadMore, uiState.isLoadingMore) { + snapshotFlow { shouldLoadMore } + .distinctUntilChanged() + .filter { it } + .collect { onLoadMore() } + }아래 임포트를 파일 상단에 추가해 주세요:
import androidx.compose.runtime.snapshotFlow import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.filterapp/src/main/java/com/texthip/thip/ui/navigator/extensions/GroupNavigationExtensions.kt (1)
19-32: 네비게이션 헬퍼 추가 LGTM + 간단 KDoc 제안함수 의도가 명확하므로 간단한 KDoc을 추가하면 호출부에서 목적 파악이 더 쉬워집니다. 문자열에 슬래시/특수문자가 포함될 수 있으므로(특히 title, imageUrl) 직렬화/인코딩을 라우팅 계층에서 안전하게 처리하는지 점검해 주세요.
KDoc 추가 예시(diff):
-// 책 정보가 미리 선택된 모임방 만들기 화면으로 이동 +/** + * 책 데이터가 미리 채워진 모임방 만들기 화면으로 이동합니다. + * @param isbn 선택된 책 ISBN (필수) + * @param title 책 제목 + * @param imageUrl 책 표지 이미지 URL + * @param author 저자명 + */ // 책 정보가 미리 선택된 모임방 만들기 화면으로 이동 fun NavHostController.navigateToGroupMakeRoomWithBook(app/src/main/java/com/texthip/thip/ui/navigator/navigations/GroupNavigation.kt (1)
103-112: LaunchedEffect 키 단순화로 불필요한 재실행 리스크 축소 제안route가 data class라 equals 기준으로는 안전하지만, 최초 1회만 수행되는 것이 의도라면 Unit 키로 고정하는 편이 사이드이펙트 재실행을 더 명확히 방지합니다.
적용 예시:
- // 책 정보를 ViewModel에 미리 설정 - LaunchedEffect(route) { + // 책 정보를 ViewModel에 미리 설정 (진입 시 1회) + LaunchedEffect(Unit) { viewModel.setPreselectedBook( isbn = route.isbn, title = route.title, imageUrl = route.imageUrl, author = route.author ) }app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupSelectBook.kt (2)
40-41: isBookPreselected 파라미터 추가 적절합니다.
- 기본값 false로 기존 호출부 호환성 유지가 좋습니다.
- 이름도 의도(외부에서 선선택 여부를 제어)와 일치합니다.
해당 파라미터의 의도를 문서화하면 재사용성이 좋아집니다. 함수 직전에 KDoc을 추가하는 것을 권장합니다.
예시(파일 외부 변경 코드):
/** * 선택된 도서 UI. * * isBookPreselected: * - true: 외부(네비게이션 등)에서 미리 선택된 상태로 진입. '변경' 액션을 노출하지 않음. * - false: 사용자가 자유롭게 도서를 변경 가능. */
121-127: '변경' 버튼 조건부 렌더링 처리 적절합니다.
- 선선택(true) 시 변경 버튼을 숨기는 정책이 명확합니다.
다만 사용자 입장에서 버튼이 갑자기 사라져 혼란을 줄 수 있으니, 선택 영역 우측에 아주 미묘한 가이드(예: "다른 화면에서 선택됨")를 보조 텍스트로 표시하는 것도 고려해볼 만합니다. 또는 버튼을 숨기지 않고 disabled 처리하여 이유를 onDisabledClick으로 안내하는 UX도 대안입니다.
disabled 처리 예시:
- if (!isBookPreselected) { - OptionChipButton( - text = stringResource(R.string.change), - onClick = onChangeBookClick, - isSelected = false - ) - } + OptionChipButton( + text = stringResource(R.string.change), + onClick = onChangeBookClick, + isSelected = false, + enabled = !isBookPreselected, + onDisabledClick = { /* TODO: 스낵바 등으로 안내: '선택된 책은 변경할 수 없습니다' */ } + )app/src/main/java/com/texthip/thip/data/model/book/response/BookListResponse.kt (1)
14-18: DTO 간 필드 네이밍 일관성 검토 권장(title vs bookTitle)검색 응답은 title, 저장 목록은 bookTitle을 사용합니다. 도메인 모델로 매핑 시 통일된 속성명 사용을 권장합니다(예: Domain.Book.title). DTO는 유지하고, 매퍼에서 정규화하면 좋습니다.
app/src/main/java/com/texthip/thip/data/model/book/response/BookSaveResponse.kt (1)
6-10: 응답 의미를 명시하는 KDoc 추가 제안응답의 시맨틱(요청 반영 후 상태인지 등)을 KDoc으로 명시하면 가독성이 좋아집니다.
적용 예시:
@Serializable +/** + * 책 저장(북마크) 상태 변경 API 응답. + * - isSaved: 요청 적용 이후 서버가 판단한 최종 저장 상태 + */ data class BookSaveResponse( @SerialName("isbn") val isbn: String = "", @SerialName("isSaved") val isSaved: Boolean = false )app/src/main/java/com/texthip/thip/data/model/book/response/RecruitingRoomsResponse.kt (1)
21-21: deadlineEndDate 포맷 명시 및 파싱 책임 분리DTO에는 String 유지하되, 예상 포맷(예: ISO-8601 yyyy-MM-dd 또는 Instant)을 KDoc에 명시하고 도메인/뷰모델 매핑에서 kotlinx-datetime으로 파싱할 것을 권장합니다.
app/src/main/java/com/texthip/thip/ui/search/component/SearchActiveField.kt (1)
40-41: derivedStateOf 기억 범위 간결화(미세 최적화)remember 키를 제거해 불필요한 재생성을 줄일 수 있습니다. derivedStateOf가 내부 읽기 상태를 추적합니다.
- val shouldLoadMore by remember(hasMore, isLoading) { + val shouldLoadMore by remember { derivedStateOf {app/src/main/java/com/texthip/thip/ui/search/component/SearchBookFilteredResult.kt (1)
48-56: 무한 스크롤 트리거: remember 키와 off-by-one 가드 보완
- remember 키에 listState를 포함하지 않아 의도를 읽기 어렵습니다. 포함해도 성능 영향은 미미합니다.
- 마지막 가시 아이템 인덱스의 기본값을 -1로 두는 것이 안전합니다(0 아이템 시 0으로 보정).
- val shouldLoadMore by remember(hasMore, isLoading) { + val shouldLoadMore by remember(listState, hasMore, isLoading) { derivedStateOf { val layoutInfo = listState.layoutInfo val totalItemsCount = layoutInfo.totalItemsCount - val lastVisibleItemIndex = (layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: 0) + 1 + val lastVisibleItemIndex = (layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: -1) + 1 - hasMore && !isLoading && totalItemsCount > 0 && lastVisibleItemIndex >= totalItemsCount - 3 + totalItemsCount > 0 && + hasMore && + !isLoading && + lastVisibleItemIndex >= totalItemsCount - 3 } }app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookGroupViewModel.kt (2)
56-65: canLoadMore를 재사용해 조건식을 단일화하면 가독성과 일관성이 좋아집니다중복된 조건 체크를 uiState.canLoadMore로 통일하세요. 추후 조건 변경에도 한 곳만 수정하면 됩니다.
- val currentState = _uiState.value - if (currentState.hasMore && !currentState.isLoading && !currentState.isLoadingMore && currentIsbn.isNotBlank()) { + val currentState = _uiState.value + if (currentState.canLoadMore && currentIsbn.isNotBlank()) { loadMoreJob?.cancel() loadMoreJob = viewModelScope.launch { _uiState.value = _uiState.value.copy(isLoadingMore = true) - loadRooms(currentIsbn, currentState.nextCursor) + loadRooms(currentIsbn, currentState.nextCursor) } }
27-38: 빈 ISBN에 대한 빠른 가드 추가 제안UI에서 방어하더라도 ViewModel 레벨에서 공백 ISBN을 빠르게 무시하면 불필요한 API 호출을 더 확실히 차단할 수 있습니다.
적용 예:
- Line 27 바로 아래에 early-return 추가:
- if (isbn.isBlank()) return
app/src/main/java/com/texthip/thip/data/repository/BookRepository.kt (2)
56-61: 입력 유효성 보강: isbn 공백 방지 가드 권장API 전단에서 즉시 실패시키면 서버 불필요 호출을 줄이고 오류 원인을 명확히 할 수 있습니다.
/** 책 상세 조회 */ suspend fun getBookDetail(isbn: String): Result<BookDetailResponse?> = runCatching { + require(isbn.isNotBlank()) { "ISBN must not be blank" } bookService.getBookDetail(isbn) .handleBaseResponse() .getOrThrow() } /** 책 저장/저장취소 */ suspend fun saveBook(isbn: String, type: Boolean): Result<BookSaveResponse?> = runCatching { + require(isbn.isNotBlank()) { "ISBN must not be blank" } bookService.saveBook(isbn, BookSaveRequest(type)) .handleBaseResponse() .getOrThrow() } /** 모집중인 방 조회 */ suspend fun getRecruitingRooms(isbn: String, cursor: String? = null): Result<RecruitingRoomsResponse?> = runCatching { + require(isbn.isNotBlank()) { "ISBN must not be blank" } bookService.getRecruitingRooms(isbn, cursor) .handleBaseResponse() .getOrThrow() }Also applies to: 63-68, 70-75
63-68: saveBook의 매개변수명(type: Boolean)이 의미를 드러내지 않습니다API payload가 BookSaveRequest(type)인 것은 알지만, 호출부 관점에서는 type이 true/false가 무엇을 의미하는지 불명확합니다. isSaved 또는 shouldSave처럼 의미를 드러내는 이름으로 바꾸길 권장합니다. 바이너리 액션이면 enum(저장/해제) 사용도 고려하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
app/src/main/java/com/texthip/thip/data/model/book/request/BookSaveRequest.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/response/BookDetailResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/response/BookListResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/response/BookSaveResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/response/BookSearchResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/response/MostSearchedBooksResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/response/RecentSearchResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/response/RecruitingRoomsResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/repository/BookRepository.kt(2 hunks)app/src/main/java/com/texthip/thip/data/service/BookService.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/common/buttons/GenreChipButton.kt(5 hunks)app/src/main/java/com/texthip/thip/ui/group/done/screen/GroupDoneScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupSelectBook.kt(3 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomUiState.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/screen/GroupMyScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/extensions/GroupNavigationExtensions.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/navigations/GroupNavigation.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/navigations/SearchNavigation.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/routes/GroupRoutes.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/search/component/SearchActiveField.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/search/component/SearchBookFilteredResult.kt(3 hunks)app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookGroupScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookGroupViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- app/src/main/java/com/texthip/thip/data/model/book/response/BookDetailResponse.kt
- app/src/main/java/com/texthip/thip/data/model/book/request/BookSaveRequest.kt
- app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookGroupScreen.kt
- app/src/main/java/com/texthip/thip/data/model/book/response/MostSearchedBooksResponse.kt
- app/src/main/java/com/texthip/thip/data/model/book/response/RecentSearchResponse.kt
- app/src/main/java/com/texthip/thip/data/service/BookService.kt
- app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt
🧰 Additional context used
🧬 Code Graph Analysis (6)
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (4)
app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (1)
updateState(33-35)app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt (1)
updateState(27-29)app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt (1)
updateState(26-28)app/src/main/java/com/texthip/thip/ui/group/room/viewmodel/GroupRoomRecruitViewModel.kt (1)
updateState(29-31)
app/src/main/java/com/texthip/thip/ui/navigator/navigations/GroupNavigation.kt (1)
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt (1)
GroupMakeRoomScreen(49-92)
app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupSelectBook.kt (1)
app/src/main/java/com/texthip/thip/ui/common/buttons/OptionChipButton.kt (1)
OptionChipButton(30-92)
app/src/main/java/com/texthip/thip/ui/search/component/SearchActiveField.kt (1)
app/src/main/java/com/texthip/thip/ui/common/cards/CardBookList.kt (1)
CardBookList(30-89)
app/src/main/java/com/texthip/thip/ui/search/component/SearchBookFilteredResult.kt (2)
app/src/main/java/com/texthip/thip/ui/common/cards/CardBookList.kt (1)
CardBookList(30-89)app/src/main/java/com/texthip/thip/ui/theme/Theme.kt (1)
ThipTheme(41-69)
app/src/main/java/com/texthip/thip/ui/navigator/navigations/SearchNavigation.kt (3)
app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookScreen.kt (1)
SearchBookScreen(41-118)app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookDetailScreen.kt (1)
SearchBookDetailScreen(55-120)app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookGroupScreen.kt (1)
SearchBookGroupScreen(45-91)
🔇 Additional comments (11)
app/src/main/java/com/texthip/thip/ui/common/buttons/GenreChipButton.kt (2)
11-11: LGTM: height import 추가 적절height(40.dp) 사용에 맞춘 import 추가는 적절합니다.
20-20: LGTM: clip import 추가 적절리플 클리핑을 위해 clip 사용은 필수이며, import 추가 OK입니다.
app/src/main/java/com/texthip/thip/ui/navigator/navigations/SearchNavigation.kt (3)
6-16: 타입드 라우트(toRoute) + 네비게이션 확장 구성 적절합니다타입드 라우팅 사용과 목적별 확장 함수로 라우트를 모듈화한 방향성 좋습니다.
20-27: SearchBookScreen 콜백 분리 및 전환 연결 👍UI에서 네비게이션을 콜백으로 분리해 MVVM 경계가 명확합니다. 상세/등록 화면 전환도 자연스럽습니다.
60-62: 확장 함수 시그니처 일치 확인 완료
navigateToGroupRecruit(roomId)호출부의roomId: Int와
확장 함수 정의(fun NavHostController.navigateToGroupRecruit(roomId: Int))가 일치함을 확인했습니다.
런타임 타입 불일치 이슈가 없으므로 추가 조치가 필요 없습니다.app/src/main/java/com/texthip/thip/ui/navigator/routes/GroupRoutes.kt (1)
10-17: 새 라우트 MakeRoomWithBook 도입 적절사전선택된 책 정보를 안전하게 전달하기 위한 @serializable data class 추가 방향이 명확하고, 다른 라우트와의 일관성도 좋습니다.
app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt (1)
140-142: GroupSelectBook에 사전선택 상태 전달 LGTM프롭스로 isBookPreselected를 명시적으로 전달해 UI 표현을 분기하는 흐름이 자연스럽습니다. ViewModel 측에서 수동 선택 시 플래그를 false로 리셋하도록만 보완되면 UX가 안정적으로 유지될 것입니다.
app/src/main/java/com/texthip/thip/ui/navigator/navigations/GroupNavigation.kt (2)
98-123: 타입 세이프 라우트 추가 및 사전 주입 흐름 연계가 명확합니다.
- MakeRoomWithBook 경로에서 toRoute로 파라미터 복원 → ViewModel에 선반영 → 화면 그리기 흐름이 일관적입니다.
- onNavigateBack / onGroupCreated 모두 navigateBack으로 통일된 것도 UX 측면에서 깔끔합니다.
100-101: MakeRoomWithBook 라우트의 @serializable 적용 확인됨GroupRoutes.kt에서 MakeRoomWithBook 클래스에 @serializable 어노테이션이 선언되어 있어 Typed route 사용 시 직렬화/역직렬화가 정상적으로 보장됩니다.
- app/src/main/java/com/texthip/thip/ui/navigator/routes/GroupRoutes.kt (라인 10–16):
@Serializable data class MakeRoomWithBook(...)- 네비게이션 확장 함수
navigateToGroupMakeRoomWithBook도 정의되어 있어 경로 전달에 문제가 없습니다.app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupSelectBook.kt (1)
163-175: GroupSelectBook의 isBookPreselected 전달 확인됨
GroupMakeRoomScreen.kt(라인 137~142)에서isBookPreselected = uiState.isBookPreselected로 의도대로 전달되고 있습니다.프리뷰 추가도 좋습니다.
isBookPreselected = true시나리오를 별도로 검증할 수 있어 컴포넌트 회귀 테스트에 유용합니다.app/src/main/java/com/texthip/thip/data/model/book/response/BookListResponse.kt (1)
9-10: 디폴트 빈 목록 적용은 좋습니다서버 필드 누락 시 크래시를 방지합니다. 페이징/목록 화면에서도 안전하게 동작하겠습니다.
| val genres: List<Genre> = emptyList(), | ||
| val isBookPreselected: Boolean = false |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
isBookPreselected 플래그 추가: 의미/리셋 시점 명확화 필요
플래그 자체는 합리적이지만, 사용자 상호작용(예: 바텀시트에서 책을 다시 선택) 시 false로 되돌리지 않으면 UI 상태가 드리프트할 수 있습니다. 현재 selectBook()이 플래그를 내리지 않아, 최초 사전선택 이후에도 변경 버튼이 계속 숨겨지는 등의 UX 문제가 발생할 수 있습니다. ViewModel에서 수동 선택 시 반드시 false로 리셋되도록 보완해 주세요. 또한 사전선택 상태가 유지되어야 하는 정확한 조건(처음 진입 직후까지만인지 등)을 주석으로 명시해 두는 것을 권장합니다.
🏁 Script executed:
#!/bin/bash
set -e
# Locate the implementation of selectBook in the codebase
rg -n "fun selectBook" -A20 -B2 -g "*.kt"Length of output: 2803
isBookPreselected 리셋 로직 보강 필요
selectBook(book: BookData) 호출 시 isBookPreselected를 false로 되돌리지 않아, 기존에 사전선택된 상태가 계속 유지될 수 있습니다. 이로 인해 “변경” 버튼이 계속 숨겨지는 등 UX 이슈가 발생할 수 있습니다.
- 수정 파일
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt - 변경 위치
fun selectBook(book: BookData)내부의updateState블록 - 권장 추가 사항
•isBookPreselected가true로 유지되어야 하는 정확한 조건(예: 최초 진입 직후 한정)을 KDoc 또는 주석으로 명시
예시 diff:
fun selectBook(book: BookData) {
- updateState { it.copy(selectedBook = book) }
+ updateState {
+ it.copy(
+ selectedBook = book,
+ isBookPreselected = false
+ )
+ }
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt
around the updateState block inside fun selectBook(book: BookData), reset
isBookPreselected to false when a user explicitly selects a book so the
preselected flag does not persist across manual selections; update the state
mutation to set isBookPreselected = false (unless you have a documented
exception), and add a KDoc or inline comment above the property or ViewModel
constructor clarifying the exact condition(s) when isBookPreselected should
remain true (for example: only true immediately after initial screen entry until
the first explicit selection).
| fun selectBook(book: BookData) { | ||
| updateState { it.copy(selectedBook = book) } | ||
| } |
There was a problem hiding this comment.
사용자 수동 선택 시 isBookPreselected 리셋 누락 → UI 드리프트 가능성
사전선택 후 사용자가 바텀시트에서 책을 다시 고르면 isBookPreselected를 false로 내려야 하나, 현재 selectBook()이 플래그를 변경하지 않습니다. 이로 인해 GroupSelectBook에서 “변경” 버튼이 숨겨진 상태가 지속되는 등 UX 회귀가 발생할 수 있습니다. 아래와 같이 수정해 주세요.
적용 제안(diff):
fun selectBook(book: BookData) {
- updateState { it.copy(selectedBook = book) }
+ // 사용자 수동 선택 시 사전선택 상태 해제
+ updateState { it.copy(selectedBook = book, isBookPreselected = false) }
}
fun setPreselectedBook(isbn: String, title: String, imageUrl: String, author: String) {
- val preselectedBook = BookData(
- title = title,
- imageUrl = imageUrl,
- author = author,
- isbn = isbn
- )
- updateState {
- it.copy(
- selectedBook = preselectedBook,
- isBookPreselected = true
- )
- }
+ // 방어적 체크(선택): 빈 ISBN은 무시하거나 에러 처리
+ if (isbn.isBlank()) return
+ val preselectedBook = BookData(
+ title = title,
+ imageUrl = imageUrl,
+ author = author,
+ isbn = isbn
+ )
+ updateState {
+ it.copy(
+ selectedBook = preselectedBook,
+ isBookPreselected = true
+ )
+ }
}추가 제안:
- setPreselectedBook 내부에서 selectBook(preselectedBook)을 호출한 다음 isBookPreselected만 true로 덮어써서 로직을 단일화하는 것도 유지보수성에 유리합니다.
Also applies to: 58-71
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt
around lines 54-56 (and also review lines 58-71), selectBook(book: BookData)
currently updates selectedBook but does not reset isBookPreselected, which can
leave the UI in a preselected state; change selectBook to update the state with
selectedBook = book and isBookPreselected = false so manual user selection
clears the preselected flag. Optionally refactor setPreselectedBook to call
selectBook(preselectedBook) and then set isBookPreselected = true to centralize
selection logic and avoid duplication.
| // 책 정보와 모집중인 방 정보 동시 로드 | ||
| loadBookDetail(isbn) | ||
| loadRooms(isbn, null) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
"동시 로드" 주석과 달리 실제로는 순차 실행됩니다 — 진짜 병렬 실행으로 변경 필요
loadBookDetail와 loadRooms가 같은 코루틴에서 순차로 실행되어 초기 응답 지연이 커질 수 있습니다. 자식 코루틴 2개를 동시에 실행하세요.
- // 책 정보와 모집중인 방 정보 동시 로드
- loadBookDetail(isbn)
- loadRooms(isbn, null)
+ // 책 정보와 모집중인 방 정보 동시 로드
+ launch { loadBookDetail(isbn) }
+ launch { loadRooms(isbn, null) }🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookGroupViewModel.kt
around lines 40-44, the two calls loadBookDetail(isbn) and loadRooms(isbn, null)
are currently invoked sequentially despite the "동시 로드" comment; change them to
run concurrently by creating two child coroutines in a coroutineScope (or
supervisorScope) and start both work items (e.g., with async/launch) then wait
for both to complete (await/join) so results load in parallel while preserving
structured concurrency and proper error handling.
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookGroupViewModel.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookGroupViewModel.kt (2)
81-88: 과거 코멘트에서 제기된 null 응답 시 hasMore=false 설정 이슈가 해결되었습니다null 응답 분기에서 hasMore=false, 로딩 플래그 해제, 초기 요청인 경우에만 에러 노출 로직이 반영되어 있습니다. 이전 피드백이 잘 반영되었습니다.
40-43: "동시 로드" 주석과 달리 실제로는 순차 실행됨 — 진짜 병렬 실행으로 교정 필요loadBookDetail과 loadRooms가 같은 코루틴에서 순차 호출되어 초기 응답 지연이 커질 수 있습니다. coroutineScope 내에서 두 작업을 자식 코루틴으로 병렬 실행하세요.
viewModelScope.launch { _uiState.value = _uiState.value.copy( isLoading = true, error = null, recruitingRooms = emptyList(), nextCursor = null, hasMore = true, totalCount = 0, bookDetail = null ) - // 책 정보와 모집중인 방 정보 동시 로드 - loadBookDetail(isbn) - loadRooms(isbn, null) + // 책 정보와 모집중인 방 정보 동시 로드 (실제 병렬 실행) + kotlinx.coroutines.coroutineScope { + launch { loadBookDetail(isbn) } + launch { loadRooms(isbn, null) } + } }app/src/main/java/com/texthip/thip/data/repository/BookRepository.kt (1)
29-35: 페이지 파라미터 유효성 검증 추가 필요음수/0 페이지가 API로 전달되지 않도록 사전에 방지하세요. 조기 실패가 디버깅과 안정성에 유리합니다.
- suspend fun searchBooks(keyword: String, page: Int = 1, isFinalized: Boolean = false): Result<BookSearchResponse?> = runCatching { - bookService.searchBooks(keyword, page, isFinalized) + suspend fun searchBooks(keyword: String, page: Int = 1, isFinalized: Boolean = false): Result<BookSearchResponse?> = runCatching { + require(page >= 1) { "page must be >= 1" } + bookService.searchBooks(keyword.trim(), page, isFinalized) .handleBaseResponse() .getOrThrow() }
🧹 Nitpick comments (12)
app/src/main/java/com/texthip/thip/data/repository/GroupRepository.kt (3)
68-70: 널 안정성 향상은 적절하지만 예외 타입을 도메인에 맞게 명확화 권장NoSuchElementException 대신 IllegalStateException 또는 도메인 전용 예외로 의미를 분명히 하면 상위 레이어에서 처리하기 용이합니다.
- .getOrThrow() - ?: throw NoSuchElementException("모집중인 모임방 정보를 찾을 수 없습니다.") + .getOrThrow() + ?: throw IllegalStateException("모집중인 모임방 정보를 찾을 수 없습니다.")
74-79: 생성 응답 널 처리: 동일하게 예외 타입 정렬 제안메시지는 좋습니다. 예외 타입을 IllegalStateException으로 통일하면 핸들링 일관성이 올라갑니다.
- .getOrThrow() - ?: throw NoSuchElementException("모임방 생성 응답을 받을 수 없습니다.") + .getOrThrow() + ?: throw IllegalStateException("모임방 생성 응답을 받을 수 없습니다.")
84-89: 참여/취소 응답 널 처리: 예외 통일동일 맥락으로 예외 타입 통일을 권장합니다.
- .getOrThrow() - ?: throw NoSuchElementException("모임방 참여/취소 응답을 받을 수 없습니다.") + .getOrThrow() + ?: throw IllegalStateException("모임방 참여/취소 응답을 받을 수 없습니다.")app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookUiState.kt (2)
30-33: unused UI 이벤트 필드 정리 또는 실제 사용처 연결 필요
showToast,toastMessage가 사용되지 않습니다. 유지할 계획이면 ViewModel에서 설정/소비 로직을 연결하고, 아니면 제거하여 상태를 간결하게 유지하세요.제거 시 제안 패치:
- val error: String? = null, - val showToast: Boolean = false, - val toastMessage: String = "" + val error: String? = null
36-37: 빈 상태 표시 기준 재검토 권장 (라이브 검색 중 빈 결과 노출 방지)현 로직은 라이브 검색 완료 시에도 빈 결과를 즉시 노출합니다. UX 의도에 따라 "완료 검색"에서만 빈 상태를 보여주는 게 자연스러울 수 있습니다.
예시 수정:
- val showEmptyState: Boolean get() = searchQuery.isNotBlank() && searchResults.isEmpty() && !isSearching + val showEmptyState: Boolean get() = isCompleteSearching && searchQuery.isNotBlank() && searchResults.isEmpty() && !isSearchingapp/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt (5)
87-95: 검색 시작 시 결과 목록 즉시 초기화로 인한 UI 플리커 가능성라이브 검색 시에는 기존 결과를 유지한 채 로딩 인디케이터만 갱신하는 편이 플리커가 적습니다(완료 검색은 초기화 유지 가능).
조건부 초기화로 변경 제안:
updateState { it.copy( isSearching = true, error = null, - searchResults = emptyList(), + searchResults = if (isLiveSearch) it.searchResults else emptyList(), currentPage = 1 ) }
38-43: 동일 쿼리 반복 입력에 대한 불필요한 디바운스/네트워크 호출 방지이전 쿼리와 동일하면 조기 반환하여 불필요한 debounce와 호출을 피하세요.
fun updateSearchQuery(query: String) { - updateState { it.copy(searchQuery = query) } + if (query == uiState.value.searchQuery) return + updateState { it.copy(searchQuery = query) }
50-53: 라이브 검색 디바운스 1000ms는 다소 길게 느껴질 수 있음일반적으로 300~500ms가 UX 측면에서 자연스럽습니다. 제품 요구에 따라 조정 고려 바랍니다.
191-201: 최근 검색어 Map 키 정규화(대소문자/공백)로 일관성 확보키워드 대소문자/여분 공백 차이로 삭제 매칭이 실패할 수 있습니다. 삽입/조회 모두 동일 규칙으로 정규화하세요.
삽입 시:
- recentSearchResponse.recentSearchList.forEach { item -> - recentSearchMap[item.searchTerm] = item - } + recentSearchResponse.recentSearchList.forEach { item -> + recentSearchMap[item.searchTerm.trim().lowercase()] = item + }삭제 시:
- recentSearchMap[keyword]?.let { recentSearchItem -> + recentSearchMap[keyword.trim().lowercase()]?.let { recentSearchItem -> deleteRecentSearch(recentSearchItem.recentSearchId) }Also applies to: 226-230
232-249: 초기화 시 totalElements 리셋 누락 및 searchQuery 중복 세팅
totalElements를 0으로 초기화하지 않아 후속 UI 로직이 혼동될 수 있습니다.updateSearchQuery("")경로에서 이미 빈 문자열로 세팅된searchQuery를 다시 빈 값으로 덮어씁니다(무해하지만 불필요).updateState { it.copy( - searchQuery = "", + // 호출자가 이미 searchQuery를 비운 경우 중복 세팅 불필요 isInitial = true, isLiveSearching = false, isCompleteSearching = false, searchResults = emptyList(), currentPage = 1, + totalElements = 0, hasMorePages = true, isSearching = false, isLoadingMore = false, error = null ) }app/src/main/java/com/texthip/thip/data/repository/BookRepository.kt (2)
21-27: null 응답 처리 정책 일관성
getBooks는 null을emptyList()로 치환하지만, 다른 메서드들은 null을 그대로 반환합니다. UI 단에서의 처리 복잡도를 줄이기 위해, 일관된 정책(예: null은 실패로 전환 혹은 null → 빈 값 치환)을 정하고 적용하는 것을 권장합니다.
65-69: saveBook 파라미터 명확성 개선 권장
type: Boolean은 의미가 모호합니다. 예: 저장(true)/해제(false)라면isSaved또는save같은 의미 있는 이름으로 변경하는 것이 가독성에 좋습니다. API 요청 모델BookSaveRequest필드명도 함께 정합성을 맞추세요.예시:
- suspend fun saveBook(isbn: String, type: Boolean): Result<BookSaveResponse?> = runCatching { - bookService.saveBook(isbn, BookSaveRequest(type)) + suspend fun saveBook(isbn: String, isSaved: Boolean): Result<BookSaveResponse?> = runCatching { + bookService.saveBook(isbn, BookSaveRequest(isSaved)) }주의: 호출부와
BookSaveRequest필드명도 함께 수정 필요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/src/main/java/com/texthip/thip/data/repository/BookRepository.kt(2 hunks)app/src/main/java/com/texthip/thip/data/repository/GroupRepository.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookScreen.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookGroupViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookUiState.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookScreen.kt
🧰 Additional context used
🧬 Code Graph Analysis (4)
app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt (5)
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt (1)
updateState(34-36)app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt (1)
updateState(27-29)app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (1)
updateState(33-35)app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (1)
updateState(37-39)app/src/main/java/com/texthip/thip/ui/group/room/viewmodel/GroupRoomRecruitViewModel.kt (1)
updateState(29-31)
app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt (5)
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt (1)
updateState(34-36)app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt (1)
updateState(26-28)app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (1)
updateState(33-35)app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (1)
updateState(37-39)app/src/main/java/com/texthip/thip/ui/group/room/viewmodel/GroupRoomRecruitViewModel.kt (1)
updateState(29-31)
app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (5)
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt (1)
updateState(34-36)app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt (1)
updateState(27-29)app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt (1)
updateState(26-28)app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (1)
updateState(37-39)app/src/main/java/com/texthip/thip/ui/group/room/viewmodel/GroupRoomRecruitViewModel.kt (1)
updateState(29-31)
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt (5)
app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt (2)
loadInitialData(35-38)updateState(27-29)app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (2)
loadInitialData(41-45)updateState(33-35)app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt (1)
updateState(26-28)app/src/main/java/com/texthip/thip/data/repository/BookRepository.kt (1)
deleteRecentSearch(51-55)app/src/main/java/com/texthip/thip/data/service/BookService.kt (1)
deleteRecentSearch(46-49)
🔇 Additional comments (3)
app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt (1)
81-85: null 응답 처리 추가: 무한 로드 차단에 효과적입니다null 응답에서 hasMore=false, isLastPage=true로 명시한 처리로 추가 로드 트리거가 적절히 종료됩니다. UI 상태와 내부 플래그가 일관되게 유지되는 점도 좋습니다.
app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (1)
92-95: null 응답 시 hasMoreMyGroups = false 처리: 페이징 종료 플래그가 명확해졌습니다null 응답 분기에서 hasMoreMyGroups를 false로 내려 무한 로드 트리거를 방지하는 처리가 적절합니다.
app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt (1)
63-67: null 응답 처리 반영: 추가 로드 종료가 명확합니다null 응답 시 hasMore=false 및 isLastPage=true 설정으로 무한 스크롤 루프를 잘 차단합니다. 이후 finally 블록에서 로딩 플래그를 해제하는 흐름과도 일관적입니다.
| fun loadRecruitingRooms(isbn: String) { | ||
| currentIsbn = isbn | ||
| viewModelScope.launch { | ||
| _uiState.value = _uiState.value.copy( | ||
| isLoading = true, | ||
| error = null, | ||
| recruitingRooms = emptyList(), |
There was a problem hiding this comment.
신규 검색 시작 시 이전 로드-모어 작업 취소 누락 및 데이터 섞임 위험
새 ISBN으로 초기 로드를 시작할 때 기존 loadMoreJob이 동작 중이면 이전 ISBN의 페이지가 뒤늦게 현재 상태에 합쳐질 수 있습니다. 초기 로드 진입 시 loadMoreJob을 취소하고, 응답 반영 전에 ISBN 일치 여부를 검증하세요.
fun loadRecruitingRooms(isbn: String) {
- currentIsbn = isbn
+ // 이전 로드-모어 작업이 남아있다면 취소하여 데이터 섞임 방지
+ loadMoreJob?.cancel()
+ currentIsbn = isbn또한 응답 반영 시 현재 ISBN과 요청 ISBN이 일치하는지 확인하세요:
private suspend fun loadRooms(isbn: String, cursor: String?) {
bookRepository.getRecruitingRooms(isbn, cursor)
.onSuccess { response ->
+ // 다른 ISBN으로 전환되었다면 이 응답은 무시
+ if (isbn != currentIsbn) return@onSuccess
response?.let { recruitingRoomsResponse ->
val currentRooms = if (cursor == null) emptyList() else _uiState.value.recruitingRooms
_uiState.value = _uiState.value.copy(
recruitingRooms = currentRooms + recruitingRoomsResponse.recruitingRoomList,
totalCount = recruitingRoomsResponse.totalRoomCount,
nextCursor = recruitingRoomsResponse.nextCursor,
hasMore = !recruitingRoomsResponse.isLast,
isLoading = false,
isLoadingMore = false,
error = null
)
} ?: run {
_uiState.value = _uiState.value.copy(
isLoading = false,
isLoadingMore = false,
hasMore = false, // null 응답 시 더 이상 로드할 수 없음을 명시
error = if (cursor == null) "모집중인 방 정보를 찾을 수 없습니다." else 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.
| fun loadRecruitingRooms(isbn: String) { | |
| currentIsbn = isbn | |
| viewModelScope.launch { | |
| _uiState.value = _uiState.value.copy( | |
| isLoading = true, | |
| error = null, | |
| recruitingRooms = emptyList(), | |
| // In SearchBookGroupViewModel.kt | |
| fun loadRecruitingRooms(isbn: String) { | |
| // 이전 로드-모어 작업이 남아있다면 취소하여 데이터 섞임 방지 | |
| loadMoreJob?.cancel() | |
| currentIsbn = isbn | |
| viewModelScope.launch { | |
| _uiState.value = _uiState.value.copy( | |
| isLoading = true, | |
| error = null, | |
| recruitingRooms = emptyList(), | |
| // … other fields if any | |
| ) | |
| // … the rest of your initial load logic | |
| } | |
| } | |
| private suspend fun loadRooms(isbn: String, cursor: String?) { | |
| bookRepository.getRecruitingRooms(isbn, cursor) | |
| .onSuccess { response -> | |
| // 다른 ISBN으로 전환되었다면 이 응답은 무시 | |
| if (isbn != currentIsbn) return@onSuccess | |
| response?.let { recruitingRoomsResponse -> | |
| val currentRooms = if (cursor == null) emptyList() else _uiState.value.recruitingRooms | |
| _uiState.value = _uiState.value.copy( | |
| recruitingRooms = currentRooms + recruitingRoomsResponse.recruitingRoomList, | |
| totalCount = recruitingRoomsResponse.totalRoomCount, | |
| nextCursor = recruitingRoomsResponse.nextCursor, | |
| hasMore = !recruitingRoomsResponse.isLast, | |
| isLoading = false, | |
| isLoadingMore = false, | |
| error = null | |
| ) | |
| } ?: run { | |
| _uiState.value = _uiState.value.copy( | |
| isLoading = false, | |
| isLoadingMore = false, | |
| hasMore = false, | |
| error = if (cursor == null) "모집중인 방 정보를 찾을 수 없습니다." else null | |
| ) | |
| } | |
| } | |
| // … error handling etc. | |
| } |
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookGroupViewModel.kt
around lines 27-33, when starting loadRecruitingRooms for a new ISBN you must
cancel any ongoing loadMoreJob to prevent responses from previous ISBNs being
merged; call loadMoreJob?.cancel() (and optionally join/cancelAndJoin) before
launching the new request, clear or replace the job reference, and set
currentIsbn = isbn before the network call; after receiving the response verify
that currentIsbn == isbn before applying the result to _uiState (skip or ignore
the response if they differ) and ensure loadMoreJob is cleared when it completes
or is cancelled.
| val isInitial: Boolean = true, | ||
| val isLiveSearching: Boolean = false, | ||
| val isCompleteSearching: Boolean = false, | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
다중 boolean 상태 플래그는 불일치/모순 상태를 초래하기 쉬움 → 단일 모드로 수렴 권장
isInitial, isLiveSearching, isCompleteSearching이 동시에 true/false가 될 수 있는 여지가 있고, 실제로 VM에서 완료 시 해제되지 않아 상태 누수가 발생합니다. 단일 enum(또는 sealed class) 기반 모드로 단순화하는 것을 권장합니다.
예시:
enum class SearchMode { Initial, Live, Complete }
data class SearchBookUiState(
val searchMode: SearchMode = SearchMode.Initial,
// ...
) {
val showInitialScreen get() = searchMode == SearchMode.Initial && searchQuery.isBlank()
// ...
}추가로 ViewModel의 모드 전환 지점에서 searchMode만 일관되게 업데이트하도록 리팩터링하면 isAnySearching/표시 로직도 명확해집니다. 원하시면 관련 수정 패치도 함께 드리겠습니다.
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookUiState.kt
around lines 11 to 14, multiple Boolean flags (isInitial, isLiveSearching,
isCompleteSearching) can become inconsistent and cause state leakage; replace
them with a single enum or sealed-mode property (e.g., SearchMode { Initial,
Live, Complete }) on the UI state, update derived getters (showInitialScreen,
isAnySearching, etc.) to compute from that mode and existing fields, and
refactor ViewModel state transitions so only the mode property is set when
changing search state to ensure consistency.
| val canLoadMore: Boolean get() = hasMorePages && !isSearching && !isLoadingMore | ||
| val showEmptyState: Boolean get() = searchQuery.isNotBlank() && searchResults.isEmpty() && !isSearching | ||
| val showInitialScreen: Boolean get() = isInitial && searchQuery.isBlank() | ||
| val isAnySearching: Boolean get() = isLiveSearching || isCompleteSearching |
There was a problem hiding this comment.
isAnySearching 계산 로직 버그: 실제 로딩 여부 대신 모드 플래그에 의존
검색 완료 후에도 isLiveSearching/isCompleteSearching가 false로 내려가지 않아, UI가 영구히 "검색 중"으로 오인할 수 있습니다. 실제 진행 중 여부는 isSearching/isLoadingMore로 계산하는 것이 안전합니다.
아래처럼 수정을 제안합니다:
- val isAnySearching: Boolean get() = isLiveSearching || isCompleteSearching
+ val isAnySearching: Boolean get() = isSearching || isLoadingMore🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookUiState.kt
around line 38, the isAnySearching getter currently returns mode flags
(isLiveSearching || isCompleteSearching); change it to reflect actual loading
state by returning isSearching || isLoadingMore instead so UI reflects real
progress rather than mode flags, and remove dependence on
isLiveSearching/isCompleteSearching in this getter.
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookUiState.kt (1)
10-14: 검색 상태 다중 Boolean 플래그 → 단일 모드로 수렴 권장다중 플래그는 상호 모순/누수 상태를 초래하기 쉽습니다. 단일 enum/모드로 단순화하세요. 이전 리뷰에서도 동일 제안을 드렸습니다.
예시 패치:
- // 상태 관리 단순화 - boolean 필드 사용 - val isInitial: Boolean = true, - val isLiveSearching: Boolean = false, - val isCompleteSearching: Boolean = false, + // 단일 모드로 상태 정의 + val searchMode: SearchMode = SearchMode.Initial,파일 외 추가 코드(상단 등 적절한 위치):
enum class SearchMode { Initial, Live, Complete }ViewModel에서는 isInitial/isLiveSearching/isCompleteSearching 대신 searchMode만 일관 업데이트하도록 리팩터링하는 것을 권장합니다.
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt (1)
101-109: 성공 경로에서도 모드 플래그 해제 누락 → UI 일관성 문제성공 시에도 isLiveSearching/isCompleteSearching가 false로 내려가지 않습니다. 이전 리뷰에서도 동일 이슈를 지적했습니다.
updateState { it.copy( searchResults = searchResponse.searchResult, currentPage = searchResponse.page, totalElements = searchResponse.totalElements, hasMorePages = !searchResponse.last, isSearching = false, - error = null + error = null, + isLiveSearching = false, + isCompleteSearching = false ) }
🧹 Nitpick comments (5)
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookUiState.kt (2)
27-27: 초기 hasMorePages 기본값 false로 시작 권장초기 화면에서 불필요한 “더 불러오기” 가능 상태를 피하려면 기본값을 false로 두는 편이 안전합니다. 실제 검색 응답으로 갱신하면 됩니다.
- val hasMorePages: Boolean = true, + val hasMorePages: Boolean = false,
36-36: 빈 상태 표시 조건에 isLoadingMore 제외 필요페이지 추가 로딩 중(set isLoadingMore)에는 빈 상태 UI가 깜빡일 수 있습니다. 로딩 중에는 빈 상태를 숨기세요.
- val showEmptyState: Boolean get() = searchQuery.isNotBlank() && searchResults.isEmpty() && !isSearching + val showEmptyState: Boolean get() = + searchQuery.isNotBlank() && searchResults.isEmpty() && !isSearching && !isLoadingMoreapp/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt (3)
87-95: 라이브 검색 시 결과 리스트를 즉시 비우지 않도록 조건부 유지라이브 검색 중 매 입력마다 리스트를 비우면 깜빡임이 큽니다. 완전 검색에서만 초기화하고, 라이브 검색은 기존 목록을 유지한 채 로딩 표시만 갱신하는 접근을 권장합니다.
updateState { - it.copy( - isSearching = true, - error = null, - searchResults = emptyList(), - currentPage = 1 - ) + it.copy( + isSearching = true, + error = null, + searchResults = if (isLiveSearch) it.searchResults else emptyList(), + currentPage = 1 + ) }
51-51: 매직 넘버 딜레이 상수화테스트/튜닝을 쉽게 하려면 딜레이 값을 상수로 추출하세요.
- delay(1000) // Live search에 딜레이 추가 + delay(LIVE_SEARCH_DEBOUNCE_MS) // Live search에 딜레이 추가파일 하단 등 적절한 위치:
companion object { private const val LIVE_SEARCH_DEBOUNCE_MS = 1000L }
229-234: 키워드 기반 삭제 시 공백·대소문자 차이 보정사용자 입력의 선/후행 공백으로 Map 조회 실패 가능성이 있습니다. trim() 적용을 권장합니다. 필요 시 대소문자 무시 처리를 함께 고려하세요.
- fun deleteRecentSearchByKeyword(keyword: String) { - recentSearchMap[keyword]?.let { recentSearchItem -> + fun deleteRecentSearchByKeyword(keyword: String) { + recentSearchMap[keyword.trim()]?.let { recentSearchItem -> deleteRecentSearch(recentSearchItem.recentSearchId) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookGroupViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookUiState.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookGroupViewModel.kt
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt (5)
app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt (2)
loadInitialData(35-38)updateState(27-29)app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (2)
loadInitialData(41-45)updateState(33-35)app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt (1)
updateState(26-28)app/src/main/java/com/texthip/thip/data/repository/BookRepository.kt (1)
deleteRecentSearch(51-55)app/src/main/java/com/texthip/thip/data/service/BookService.kt (1)
deleteRecentSearch(46-49)
🔇 Additional comments (1)
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt (1)
16-69: isLiveSearching/isCompleteSearching 플래그 변경 영향 검증 필요Live 검색(실시간)과 Complete 검색(완료) 시점에
isLiveSearching/isCompleteSearching을false로 초기화하는 본 PR 변경이, UI 분기 로직과 충돌 없는지 확인해주세요. 변경 후에도 의도한 화면(실시간 검색 결과, 완료 검색 결과)이 정상 출력되는지 아래 위치를 중심으로 점검 바랍니다.• SearchBookViewModel.performSearch 내부
– 에러 및 성공 처리(updateState) 시 플래그 초기화 코드 확인
• onSearchButtonClick/ updateSearchQuery 호출부 (app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt:43–49, 63–69)
• clearSearchResults 호출부 (…/SearchBookViewModel.kt:240–246)
• SearchBookScreenContent 분기 (app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookScreen.kt:175–185, 200–202)위 지점에서 플래그 값 변화가 화면 분기 로직과 일치하는지 확인 후, 필요 시 성공 경로에도
isCompleteSearching = true상태 유지 또는 UI 분기 로직 수정이 필요합니다.
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt
Show resolved
Hide resolved
| searchJob?.cancel() | ||
| if (query.isNotBlank()) { | ||
| updateState { | ||
| it.copy( | ||
| isInitial = false, | ||
| isLiveSearching = true, | ||
| isCompleteSearching = false | ||
| ) | ||
| } | ||
| searchJob = viewModelScope.launch { | ||
| delay(1000) // Live search에 딜레이 추가 | ||
| performSearch(query, isLiveSearch = true) | ||
| } | ||
| } else { | ||
| clearSearchResults() | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
새 검색 시작 시 기존 로드모어 작업도 취소 필요
새 쿼리 검색을 시작하면서 loadMoreJob을 취소하지 않으면, 이전 쿼리의 페이지 요청이 뒤늦게 도착해 상태 오염(다른 쿼리 결과가 합쳐짐) 가능성이 있습니다.
searchJob?.cancel()
+ loadMoreJob?.cancel()
if (query.isNotBlank()) {
updateState {
it.copy(
isInitial = false,
isLiveSearching = true,
isCompleteSearching = false
)
}
searchJob = viewModelScope.launch {
- delay(1000) // Live search에 딜레이 추가
+ delay(LIVE_SEARCH_DEBOUNCE_MS) // Live search에 딜레이 추가
performSearch(query, isLiveSearch = true)
}
} else {
clearSearchResults()
}추가로 상수화(파일 밖 보강):
companion object {
private const val LIVE_SEARCH_DEBOUNCE_MS = 1000L
}
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt
Show resolved
Hide resolved
| bookRepository.searchBooks(query, 1, isFinalized = !isLiveSearch) | ||
| .onSuccess { response -> | ||
| response?.let { searchResponse -> | ||
| updateState { | ||
| it.copy( | ||
| searchResults = searchResponse.searchResult, | ||
| currentPage = searchResponse.page, | ||
| totalElements = searchResponse.totalElements, | ||
| hasMorePages = !searchResponse.last, | ||
| isSearching = false, | ||
| error = null | ||
| ) | ||
| } | ||
| } ?: run { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
검색 성공 응답의 스테일 가드 추가 (쿼리 변경 경쟁 상태 방지)
요청-응답 간에 사용자가 쿼리를 바꾸면 이전 응답이 뒤늦게 도착해 상태를 덮어쓸 위험이 있습니다. 응답 적용 전 현재 쿼리 일치 여부를 확인하세요.
- bookRepository.searchBooks(query, 1, isFinalized = !isLiveSearch)
+ val expectedQuery = query
+ bookRepository.searchBooks(query, 1, isFinalized = !isLiveSearch)
.onSuccess { response ->
+ // 스테일 응답 가드
+ if (uiState.value.searchQuery != expectedQuery) return@onSuccess
response?.let { searchResponse ->
updateState {📝 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.
| bookRepository.searchBooks(query, 1, isFinalized = !isLiveSearch) | |
| .onSuccess { response -> | |
| response?.let { searchResponse -> | |
| updateState { | |
| it.copy( | |
| searchResults = searchResponse.searchResult, | |
| currentPage = searchResponse.page, | |
| totalElements = searchResponse.totalElements, | |
| hasMorePages = !searchResponse.last, | |
| isSearching = false, | |
| error = null | |
| ) | |
| } | |
| } ?: run { | |
| // Capture the query at request time to guard against stale responses | |
| val expectedQuery = query | |
| bookRepository.searchBooks(query, 1, isFinalized = !isLiveSearch) | |
| .onSuccess { response -> | |
| // 스테일 응답 가드: 쿼리가 변경되었으면 무시 | |
| if (uiState.value.searchQuery != expectedQuery) return@onSuccess | |
| response?.let { searchResponse -> | |
| updateState { | |
| it.copy( | |
| searchResults = searchResponse.searchResult, | |
| currentPage = searchResponse.page, | |
| totalElements = searchResponse.totalElements, | |
| hasMorePages = !searchResponse.last, | |
| isSearching = false, | |
| error = null | |
| ) | |
| } | |
| } ?: run { | |
| // ... null-response handling | |
| } | |
| } | |
| .onFailure { throwable -> | |
| // ... failure handling | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt
around lines 97 to 110, the success handler unconditionally applies the response
which can overwrite newer search results if the user changed the query before
the response arrived; add a stale-response guard by capturing the query used for
this request (or passing it into the callback) and before calling updateState
verify it still matches the current ViewModel query (or expected request id);
only update state when they match, otherwise ignore the response.
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt
Show resolved
Hide resolved
| private suspend fun performLoadMore() { | ||
| val currentState = uiState.value | ||
| val nextPage = currentState.currentPage + 1 | ||
|
|
||
| updateState { it.copy(isLoadingMore = true) } | ||
|
|
||
| bookRepository.searchBooks(currentState.searchQuery, nextPage, isFinalized = true) | ||
| .onSuccess { response -> | ||
| response?.let { searchResponse -> | ||
| updateState { | ||
| it.copy( | ||
| searchResults = it.searchResults + searchResponse.searchResult, | ||
| currentPage = searchResponse.page, | ||
| totalElements = searchResponse.totalElements, | ||
| hasMorePages = !searchResponse.last, | ||
| isLoadingMore = false, | ||
| error = null | ||
| ) | ||
| } | ||
| } ?: run { | ||
| updateState { | ||
| it.copy( | ||
| isLoadingMore = false, | ||
| hasMorePages = false, // null 응답 시 더 이상 페이지가 없음을 명시 | ||
| error = "추가 결과를 불러올 수 없습니다." | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| .onFailure { throwable -> | ||
| updateState { | ||
| it.copy( | ||
| isLoadingMore = false, | ||
| error = throwable.message ?: "추가 결과를 불러오는 중 오류가 발생했습니다." | ||
| ) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
로드 모어 응답에도 스테일 가드 필요 + 방어적 처리
로드 모어 중 쿼리 변경/새 검색 시작 시 이전 로드모어 응답이 합쳐지는 문제를 방지하세요.
private suspend fun performLoadMore() {
val currentState = uiState.value
val nextPage = currentState.currentPage + 1
+ val expectedQuery = currentState.searchQuery
updateState { it.copy(isLoadingMore = true) }
bookRepository.searchBooks(currentState.searchQuery, nextPage, isFinalized = true)
.onSuccess { response ->
+ // 스테일 응답 가드
+ if (uiState.value.searchQuery != expectedQuery) return@onSuccess
response?.let { searchResponse ->
updateState {
it.copy(
searchResults = it.searchResults + searchResponse.searchResult,
currentPage = searchResponse.page,
totalElements = searchResponse.totalElements,
hasMorePages = !searchResponse.last,
isLoadingMore = false,
error = null
)
}
} ?: run {
updateState {
it.copy(
isLoadingMore = false,
hasMorePages = false, // null 응답 시 더 이상 페이지가 없음을 명시
error = "추가 결과를 불러올 수 없습니다."
)
}
}
}
.onFailure { throwable ->
updateState {
it.copy(
isLoadingMore = false,
error = throwable.message ?: "추가 결과를 불러오는 중 오류가 발생했습니다."
)
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private suspend fun performLoadMore() { | |
| val currentState = uiState.value | |
| val nextPage = currentState.currentPage + 1 | |
| updateState { it.copy(isLoadingMore = true) } | |
| bookRepository.searchBooks(currentState.searchQuery, nextPage, isFinalized = true) | |
| .onSuccess { response -> | |
| response?.let { searchResponse -> | |
| updateState { | |
| it.copy( | |
| searchResults = it.searchResults + searchResponse.searchResult, | |
| currentPage = searchResponse.page, | |
| totalElements = searchResponse.totalElements, | |
| hasMorePages = !searchResponse.last, | |
| isLoadingMore = false, | |
| error = null | |
| ) | |
| } | |
| } ?: run { | |
| updateState { | |
| it.copy( | |
| isLoadingMore = false, | |
| hasMorePages = false, // null 응답 시 더 이상 페이지가 없음을 명시 | |
| error = "추가 결과를 불러올 수 없습니다." | |
| ) | |
| } | |
| } | |
| } | |
| .onFailure { throwable -> | |
| updateState { | |
| it.copy( | |
| isLoadingMore = false, | |
| error = throwable.message ?: "추가 결과를 불러오는 중 오류가 발생했습니다." | |
| ) | |
| } | |
| } | |
| } | |
| private suspend fun performLoadMore() { | |
| val currentState = uiState.value | |
| val nextPage = currentState.currentPage + 1 | |
| val expectedQuery = currentState.searchQuery | |
| updateState { it.copy(isLoadingMore = true) } | |
| bookRepository.searchBooks(currentState.searchQuery, nextPage, isFinalized = true) | |
| .onSuccess { response -> | |
| // 스테일 응답 가드 | |
| if (uiState.value.searchQuery != expectedQuery) return@onSuccess | |
| response?.let { searchResponse -> | |
| updateState { | |
| it.copy( | |
| searchResults = it.searchResults + searchResponse.searchResult, | |
| currentPage = searchResponse.page, | |
| totalElements = searchResponse.totalElements, | |
| hasMorePages = !searchResponse.last, | |
| isLoadingMore = false, | |
| error = null | |
| ) | |
| } | |
| } ?: run { | |
| updateState { | |
| it.copy( | |
| isLoadingMore = false, | |
| hasMorePages = false, // null 응답 시 더 이상 페이지가 없음을 명시 | |
| error = "추가 결과를 불러올 수 없습니다." | |
| ) | |
| } | |
| } | |
| } | |
| .onFailure { throwable -> | |
| updateState { | |
| it.copy( | |
| isLoadingMore = false, | |
| error = throwable.message ?: "추가 결과를 불러오는 중 오류가 발생했습니다." | |
| ) | |
| } | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt (5)
35-37: StateFlow 원자적 업데이트 적용: 경쟁 상태 방지에 적절합니다
MutableStateFlow.update로 상태 갱신을 원자적으로 처리한 점 좋습니다. 동시 코루틴 업데이트에서 덮어쓰기 리스크를 줄입니다.
41-43: 새 검색 전 in-flight 작업 정리(Cancel) 반영: OK
searchJob과loadMoreJob모두를 취소하고 새 검색을 시작하도록 정리된 점 👍. 과거 쿼리/페이지 요청이 뒤늦게 도착해 상태를 오염시키는 문제를 줄입니다.Also applies to: 65-66
115-125: null 응답 시 hasMorePages=false 명시: OK초기 검색/로드모어의 null 응답 분기에서
hasMorePages = false를 명시한 점 적절합니다. 잘못된 “더 불러오기” 상태를 방지합니다.Also applies to: 159-166
101-114: 검색 성공 후 모드 플래그 미해제 + 스테일 응답 가드 부재
- 성공 분기에서
isLiveSearching/isCompleteSearching를false로 되돌리지 않아 UI가 “검색 중”으로 남을 수 있습니다.- 요청-응답 사이에 사용자가 쿼리를 변경하면 이전 응답이 뒤늦게 도착해 상태를 덮어쓸 수 있습니다. 쿼리 일치 여부로 스테일 응답을 필터링하세요.
아래와 같이 수정 제안합니다:
private suspend fun performSearch(query: String, isLiveSearch: Boolean) { updateState { it.copy( isSearching = true, error = null, searchResults = emptyList(), currentPage = 1 ) } - bookRepository.searchBooks(query, 1, isFinalized = !isLiveSearch) + val expectedQuery = query.trim() + bookRepository.searchBooks(expectedQuery, 1, isFinalized = !isLiveSearch) .onSuccess { response -> + // 스테일 응답 가드: 현재 쿼리와 요청 당시 쿼리가 다르면 무시 + if (uiState.value.searchQuery.trim() != expectedQuery) return@onSuccess response?.let { searchResponse -> updateState { it.copy( searchResults = searchResponse.searchResult, currentPage = searchResponse.page, totalElements = searchResponse.totalElements, hasMorePages = !searchResponse.last, isSearching = false, - error = null + error = null, + isLiveSearching = false, + isCompleteSearching = false ) } } ?: run { updateState { it.copy( searchResults = emptyList(), isSearching = false, isLiveSearching = false, isCompleteSearching = false, hasMorePages = false, error = if (isLiveSearch) null else "검색 결과를 불러올 수 없습니다." ) } } } .onFailure { throwable -> updateState { it.copy( searchResults = emptyList(), isSearching = false, isLiveSearching = false, isCompleteSearching = false, error = if (isLiveSearch) null else (throwable.message ?: "검색 중 오류가 발생했습니다.") ) } } }Also applies to: 127-137
140-151: 로드 모어 응답도 스테일 가드 필요로드 모어 중 새 검색이 시작되면 이전 페이지 응답이 합쳐질 수 있습니다. 쿼리 일치 여부로 필터링하세요. 또한 로드모어 요청 쿼리도 trim하여 일관성 유지가 좋습니다.
private suspend fun performLoadMore() { val currentState = uiState.value val nextPage = currentState.currentPage + 1 + val expectedQuery = currentState.searchQuery.trim() updateState { it.copy(isLoadingMore = true) } - bookRepository.searchBooks(currentState.searchQuery, nextPage, isFinalized = true) + bookRepository.searchBooks(expectedQuery, nextPage, isFinalized = true) .onSuccess { response -> + // 스테일 응답 가드 + if (uiState.value.searchQuery.trim() != expectedQuery) return@onSuccess response?.let { searchResponse -> updateState { it.copy( searchResults = it.searchResults + searchResponse.searchResult, currentPage = searchResponse.page, totalElements = searchResponse.totalElements, hasMorePages = !searchResponse.last, isLoadingMore = false, error = null ) } } ?: run { updateState { it.copy( isLoadingMore = false, hasMorePages = false, // null 응답 시 더 이상 페이지가 없음을 명시 error = "추가 결과를 불러올 수 없습니다." ) } } } .onFailure { throwable -> updateState { it.copy( isLoadingMore = false, error = throwable.message ?: "추가 결과를 불러오는 중 오류가 발생했습니다." ) } } }Also applies to: 146-168
🧹 Nitpick comments (3)
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt (3)
53-53: 딜레이 매직넘버 상수화
delay(1000)을 의미 있는 상수로 치환해 가독성과 유지보수성을 높이세요.- delay(1000) // Live search에 딜레이 추가 + delay(LIVE_SEARCH_DEBOUNCE_MS) // Live search에 딜레이 추가클래스 내부에 상수 추가:
companion object { private const val LIVE_SEARCH_DEBOUNCE_MS = 1000L }
245-256: clearSearchResults 시 hasMorePages 초기값 재고초기화 시
hasMorePages = true는 의미적으로 혼동을 줄 수 있습니다. 비어있는 결과에서는false가 더 자연스럽습니다(실제 검색이 시작되면 서버 응답으로 덮어씌워집니다).currentPage = 1, - hasMorePages = true, + hasMorePages = false, isSearching = false, isLoadingMore = false,
194-196: 조용한 실패 처리에 최소한의 로깅 추가 권장UI에는 에러를 노출하지 않더라도, 디버깅을 위해
Timber.d/w등으로 한 줄 로그는 남겨두는 편이 운영/분석에 유리합니다.예)
.onFailure { t -> Timber.w(t, "Failed to load most searched books") }Also applies to: 216-218, 228-230
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt (5)
app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt (2)
loadInitialData(35-38)updateState(27-29)app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (2)
loadInitialData(41-45)updateState(33-35)app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt (1)
updateState(26-28)app/src/main/java/com/texthip/thip/data/repository/BookRepository.kt (1)
deleteRecentSearch(51-55)app/src/main/java/com/texthip/thip/data/service/BookService.kt (1)
deleteRecentSearch(46-49)
| <application | ||
| android:name=".ThipApplication" | ||
| android:allowBackup="true" | ||
| android:networkSecurityConfig="@xml/network_security_config" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
networkSecurityConfig 연결 자체는 OK. 다만 현재 설정(IP 기반)은 적용되지 않을 수 있으며, 빌드타입 분리 필요.
- 현
@xml/network_security_config에서 IP를 사용하는 domain-config는 매칭되지 않아 cleartext 허용 효과가 없을 가능성이 큽니다. - 디버그/릴리스 분리 권장:
- debug: 전역 cleartext 허용(또는 개발 도메인만 허용)
- release: cleartext 전면 금지 및 HTTPS 사용
검증을 위해 코드베이스에서 실제 사용 중인 엔드포인트를 확인해주세요.
다음 스크립트로 http 스킴 및 해당 IP 사용 여부를 검색할 수 있습니다:
릴리스 안전성 강화를 위해(선택):
- main Manifest에서는 해당 속성을 제거하고(src/debug/AndroidManifest.xml에만 추가) 릴리스에선 HTTPS만 허용하도록 유지하는 것을 권장합니다.
릴리스에서 속성을 제거하는 변경 예시:
- android:networkSecurityConfig="@xml/network_security_config"🏁 Script executed:
#!/bin/bash
# 루트에서 실행. 코드 내 http/해당 IP 레퍼런스 검색
rg -n -S -A 2 $'http://|https://|3\\.37\\.87\\.117'Length of output: 54199
AndroidManifest 네트워크 보안 설정 빌드타입별 분리 권장
현 설정(IP 기반 domain-config)이 실제 엔드포인트와 매칭되지 않아 cleartext 허용 효과가 없으며, 코드베이스 전체에 http:// 스킴·해당 IP 참조도 발견되지 않았습니다. 빌드타입별로 설정을 분리해 명확히 관리하세요.
- app/src/main/res/xml/network_security_config.xml
<domain-config>에3.37.87.117만 명시 → Android의<domain>은 도메인 네임만 처리하므로 IP 주소 매칭 불가
- 코드베이스 전체에서
http://스킴·3.37.87.117직접 호출 없음- 실제 API 엔드포인트가 외부 설정(BuildConfig, 플레버, CI/CD 변수 등)으로 주입된다면 해당 경로도 확인 필요
조치 제안
app/src/main/AndroidManifest.xml에서android:networkSecurityConfig="@xml/network_security_config"제거src/debug/AndroidManifest.xml에만 추가
app/src/main/res/xml/network_security_config.xml수정- IP 대신 개발용 도메인(예:
dev.example.com)을<domain>에 명시하거나 <base-config cleartextTrafficPermitted="true"/>로 전역 cleartext 허용- 릴리스용은
<base-config cleartextTrafficPermitted="false"/>유지
- IP 대신 개발용 도메인(예:
- 릴리스 빌드에서는 HTTPS 전용 사용 강제
- 필요 시
<application android:usesCleartextTraffic="false">추가
- 필요 시
| <domain-config cleartextTrafficPermitted="true"> | ||
| <domain includeSubdomains="true">3.37.87.117</domain> | ||
| </domain-config> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
IP 주소에 대한 domain-config는 매칭되지 않음 — 설정이 효과가 없습니다.
<domain>는 호스트명 전용입니다. IP(3.37.87.117)는 domain 매칭 대상이 아니며,includeSubdomains도 IP에는 의미가 없습니다.- 결과적으로 Android 9(API 28)+에서 여전히 “CLEARTEXT communication to 3.37.87.117 not permitted by network security policy”가 발생할 수 있습니다.
대안:
- 옵션 A(임시·디버그 전용): 전역 cleartext 허용(base-config)로 전환하고, 이 파일을 debug 빌드에만 포함.
- 옵션 B(권장): 서버에 DNS 호스트명을 발급받아 HTTPS로 전환. 그 후 호스트명 기반 domain-config만 허용.
옵션 A 적용 예시(현재 파일 내 변경; 반드시 debug 빌드로 분리 권장):
<network-security-config>
- <domain-config cleartextTrafficPermitted="true">
- <domain includeSubdomains="true">3.37.87.117</domain>
- </domain-config>
+ <!-- Debug-only: 전역 cleartext 허용. Release에서는 금지해야 합니다. -->
+ <base-config cleartextTrafficPermitted="true" />
</network-security-config>옵션 B 적용 예시(정식 도메인 사용 시):
<network-security-config>
- <domain-config cleartextTrafficPermitted="true">
- <domain includeSubdomains="true">3.37.87.117</domain>
- </domain-config>
+ <domain-config cleartextTrafficPermitted="true">
+ <!-- 예: api.example.com -->
+ <domain includeSubdomains="false">YOUR_HOSTNAME</domain>
+ </domain-config>
</network-security-config>📝 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.
| <domain-config cleartextTrafficPermitted="true"> | |
| <domain includeSubdomains="true">3.37.87.117</domain> | |
| </domain-config> | |
| <network-security-config> | |
| <!-- Debug-only: 전역 cleartext 허용. Release에서는 금지해야 합니다. --> | |
| <base-config cleartextTrafficPermitted="true" /> | |
| </network-security-config> |
| <domain-config cleartextTrafficPermitted="true"> | |
| <domain includeSubdomains="true">3.37.87.117</domain> | |
| </domain-config> | |
| <network-security-config> | |
| <domain-config cleartextTrafficPermitted="true"> | |
| <!-- 예: api.example.com --> | |
| <domain includeSubdomains="false">YOUR_HOSTNAME</domain> | |
| </domain-config> | |
| </network-security-config> |
🤖 Prompt for AI Agents
In app/src/main/res/xml/network_security_config.xml around lines 3 to 5, the
domain-config uses a raw IP (3.37.87.117) which Android's <domain> only matches
against hostnames, so the cleartext rule is ineffective and causes CLEARTEXT
errors; fix by either A) for debug only: change to a base-config that permits
cleartext globally and ensure this file is included only in debug builds (move
to src/debug/res/xml or use manifest / buildType configuration), or B)
preferably: obtain and use a DNS hostname with TLS and replace the IP entry with
a <domain> hostname entry (remove includeSubdomains on IP), then switch to HTTPS
and restrict cleartext in the config; implement one of these two changes and
remove the IP-based domain-config.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
app/src/main/java/com/texthip/thip/data/manager/Genre.kt (2)
12-12: 중복 인자 제거 제안: 기본값과 동일한 값 명시 불필요현재 networkApiCategory가 apiCategory와 동일하므로 기본값에 맡기면 중복을 줄일 수 있습니다.
적용 diff:
- SCIENCE_IT("science_it", "과학·IT", "과학·IT"), + SCIENCE_IT("science_it", "과학·IT"),
12-12: 서버 카테고리 구분자(“/”→“·”) 변경에 따른 API 호환성 검토 필요
다음 사항을 확인하고 백엔드가 새 구분자(“·”)를 정상 처리하는지 검증해 주세요.
- 기존 표기 “과학/IT” 사용 위치
• app/src/main/res/values/strings.xml:375 (api_genre_science_it→ “과학/IT”)- 신규 표기 “과학·IT” 사용 위치
• app/src/main/res/values/strings.xml:97 (science_it→ “과학·IT”)
• FeedWriteScreen.kt:68
• GenreChipRow.kt:54
• Genre.kt:12 (networkApiCategory기본값으로 “과학·IT” 반환)- API 호출 시 전달되는 쿼리 파라미터
• GroupService.getRooms(@query("category")…): 동적으로 “과학·IT” 전송백엔드가 여전히 “과학/IT”를 기대한다면 필터링·검색 로직이 깨질 수 있습니다.
필요 시 다음 중 하나를 적용하세요:
- 네트워크 매퍼에서 “과학·IT”↔“과학/IT” 상호 변환 레이어 추가
- strings.xml 리소스 통합(기존 “과학/IT” → “과학·IT” 일괄 교체)
- 백엔드 API 스펙 업데이트(“·” 허용)
app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupMainCard.kt (1)
93-97: 긴 제목 말줄임(Ellipsis) 처리 제안제목이 1줄 초과 시 UI 깨짐을 방지하기 위해 Ellipsis 처리를 권장합니다.
아래처럼 overflow 옵션을 추가해 주세요:
- Text( - text = data.roomTitle, - style = typography.smalltitle_sb600_s18_h24, - color = colors.Black, - maxLines = 1 - ) + Text( + text = data.roomTitle, + style = typography.smalltitle_sb600_s18_h24, + color = colors.Black, + maxLines = 1, + overflow = androidx.compose.ui.text.style.TextOverflow.Ellipsis + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/src/main/java/com/texthip/thip/data/manager/Genre.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/group/response/JoinedRoomListResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupMainCard.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupPager.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt(1 hunks)
🔇 Additional comments (3)
app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupPager.kt (1)
146-146: 프리뷰 데이터의 필드명 변경 반영이 정확합니다bookTitle → roomTitle 리네임이 프리뷰에 일관되게 반영되어 빌드 안정성 측면에서 좋습니다.
Also applies to: 153-153, 160-160, 177-177
app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt (1)
196-196: 프리뷰의 JoinedRoomResponse 필드명 변경 반영 OKPreviewGroupScreen 내 roomTitle 적용이 일관적입니다. 관련 UI 컴포넌트와 모델 변경과도 정합성이 맞습니다.
Also applies to: 203-203, 210-210
app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupMainCard.kt (1)
174-174: 프리뷰 데이터 리네임 반영 확인PreviewMyGroupMainCard의 roomTitle 반영이 적절합니다.
| @SerialName("roomId") val roomId: Int, | ||
| @SerialName("bookImageUrl") val bookImageUrl: String?, | ||
| @SerialName("bookTitle") val bookTitle: String, | ||
| @SerialName("roomTitle") val roomTitle: String, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
서버 호환성(역직렬화) 위험: bookTitle → roomTitle 변경 시 구버전 응답과 불일치 가능
서버가 아직 "bookTitle"을 내려주는 경우 앱 크래시(누락 필드) 또는 역직렬화 실패가 발생할 수 있습니다. 구·신 규격을 모두 수용하도록 fallback을 추가하는 것을 권장합니다. 또한 안전성을 위해 기본값을 지정하면 런타임 오류를 줄일 수 있습니다.
권장 변경안:
import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable
+import kotlinx.serialization.json.JsonNames
@Serializable
data class JoinedRoomResponse(
@SerialName("roomId") val roomId: Int,
@SerialName("bookImageUrl") val bookImageUrl: String?,
- @SerialName("roomTitle") val roomTitle: String,
+ @SerialName("roomTitle")
+ @JsonNames("bookTitle")
+ val roomTitle: String = "",
@SerialName("memberCount") val memberCount: Int,
@SerialName("userPercentage") val userPercentage: Int
)검증 스크립트(레포 내부에서 실행):
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) 코드베이스에서 남아있는 'bookTitle' 사용 위치 확인:"
rg -n --hidden --glob '!.git' --glob '!**/build/**' '\bbookTitle\b' -A 2
echo
echo "2) kotlinx-serialization 버전 확인(JsonNames 지원 여부 점검):"
rg -n --hidden --glob '!.git' --glob '!**/build/**' -e 'kotlinx-serialization' -e 'serialization' -e 'kotlinx.*serialization.*json' -A 2Length of output: 24376
group 응답 모델 roomTitle 필드에 구버전 bookTitle 하위 호환 처리 필요
JoinedRoomListResponse.kt에서 서버가 여전히 bookTitle을 내려줄 경우 역직렬화 오류가 발생할 수 있으므로, 다음과 같이 fallback 및 기본값을 설정하세요.
• 파일:
app/src/main/java/com/texthip/thip/data/model/group/response/JoinedRoomListResponse.kt
• 수정 예시(diff):
import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable
+import kotlinx.serialization.json.JsonNames
@Serializable
data class JoinedRoomListResponse(
@SerialName("roomId") val roomId: Int,
@SerialName("bookImageUrl") val bookImageUrl: String?,
- @SerialName("roomTitle") val roomTitle: String,
+ @SerialName("roomTitle")
+ @JsonNames("bookTitle")
+ val roomTitle: String = "",
@SerialName("memberCount") val memberCount: Int,
@SerialName("userPercentage") val userPercentage: Int
)• 추가 확인 사항
– gradle/libs.versions.toml에서 kotlinx-serialization-json 버전이 1.3.0 이상인지 확인하여 @JsonNames 지원 여부를 보장하세요.
– 다른 그룹/룸 관련 응답 모델에도 동일한 필드명이 변경된 곳이 있는지 검토하고, 필요 시 동일한 패턴으로 하위 호환 처리를 적용하세요.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/src/main/java/com/texthip/thip/ui/group/room/screen/GroupRoomRecruitScreen.kt (1)
166-166: 성능/확장성 관점에서 LazyColumn 고려현재는 Column + verticalScroll이라 모든 자식이 한 번에 컴포즈됩니다. 추천 모임방 섹션 등 콘텐츠가 더 늘어나는 경우, LazyColumn으로 전환하면 초기 컴포지션/메모리 사용을 줄일 수 있습니다. 당장은 필수는 아니나 스크린이 확장될 계획이라면 고려해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/texthip/thip/ui/group/room/screen/GroupRoomRecruitScreen.kt(3 hunks)
🔇 Additional comments (2)
app/src/main/java/com/texthip/thip/ui/group/room/screen/GroupRoomRecruitScreen.kt (2)
17-19: 세로 스크롤 도입을 위한 import 추가 적절
rememberScrollState,verticalScroll추가가 목적에 부합합니다. 불필요한 import도 없습니다.
105-105: 하단 고정 버튼(50dp)과 스크롤 콘텐츠 겹침 방지작은 화면에서 마지막 아이템이 버튼 뒤로 가려지지 않도록, 현재 바텀 패딩(20dp)을 버튼 노출 여부에 따라 동적으로 확장해주세요.
- 대상 파일
- app/src/main/java/com/texthip/thip/ui/group/room/screen/GroupRoomRecruitScreen.kt
- 변경 위치: 약 105행 (scrollState 선언부)
- 유사 로직 적용 위치: 162–167행
@@ val scrollState = rememberScrollState() + // 하단 고정 버튼(50.dp)과의 겹침 방지를 위해 여유 패딩 적용 + val contentBottomPadding = if (uiState.currentButtonType != null) 70.dp else 20.dp @@ Column( Modifier .fillMaxSize() - .padding(start = 20.dp, end = 20.dp, bottom = 20.dp) + .padding(start = 20.dp, end = 20.dp, bottom = contentBottomPadding) .verticalScroll(scrollState) ) {작은 화면(5~5.5인치, 폰트 스케일 1.2x 이상)에서 스크롤을 최하단까지 내렸을 때 추천 모임방 카드가 버튼 뒤에 가려지지 않는지 확인 부탁드립니다.
rbqks529
force-pushed
the
feat/#67_API_Book
branch
from
88d9ad6 to
7042833
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (4)
app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt (3)
52-54: reset 호출과 로드모어가 동시에 실행될 수 있는 경쟁 상태 방지 필요현재 reset 분기에서는 isLoadingMore를 확인하지 않아, 로드모어 진행 중에 새로고침이 겹쳐 실행될 수 있습니다. 리스트/커서 상태가 뒤엉킬 수 있으므로 동시 실행을 차단하세요.
다음과 같이 게이트를 강화하는 것을 제안합니다:
if (reset) { - if (isInitialLoading) return + if (isInitialLoading || isLoadingMore) return isInitialLoading = true } else {
55-58: GroupDoneViewModel: 로딩 시작 시 UI 상태도 갱신 필요UI 상태(
uiState.isLoadingMore)가 갱신되지 않아 무한스크롤 게이트가 해제되지 않고 로딩 인디케이터가 표시되지 않습니다. 로드 모어를 시작할 때 내부 플래그와 함께 UI 상태도 true로 반영해야 합니다.
- 파일:
app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt
위치: lines 55–58제안 diff:
} else { if (isLoadingMore || isLastPage) return isLoadingMore = true + // UI에도 로딩 중으로 표시되도록 상태 업데이트 + updateState { it.copy(isLoadingMore = true) } }
6-6: 실제 RoomType 모델 정의 및 mock 의존 제거 필요검증 결과:
RoomType은 오직app/src/main/java/com/texthip/thip/ui/group/myroom/mock/RoomType.kt에서만 정의되어 있습니다.- 아래 파일들이 모두
com.texthip.thip.ui.group.myroom.mock.RoomType에 의존 중입니다:
- app/src/main/java/com/texthip/thip/ui/navigator/navigations/GroupNavigation.kt:17
- app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt:6
- app/src/main/java/com/texthip/thip/ui/group/done/mock/MyRoomCardData.kt:3
- app/src/main/java/com/texthip/thip/ui/group/myroom/screen/GroupMyScreen.kt:34
- app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt:6
- app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyUiState.kt:4
실제 도메인/데이터 계층의
RoomType타입이 아직 정의되어 있지 않으므로:
- 도메인 또는 데이터 모듈에
RoomType(enum/sealed/data class) 모델을 새로 정의하세요.- UI 및 내비게이션 코드에서
com.texthip.thip.ui.group.myroom.mock.RoomType대신 새 정의한 타입으로 임포트 경로를 교체해주세요.이후 PR에 반영된 변경 사항을 다시 리뷰하겠습니다.
app/src/main/java/com/texthip/thip/ui/common/screen/RegisterBookScreen.kt (1)
47-48: Spacer에 padding만 주면 실제 간격이 생기지 않습니다Spacer는 크기가 0이므로 padding만으로는 시각적 여백이 생기지 않습니다. height(8.dp)로 변경하세요.
- Spacer(modifier = Modifier.padding(top = 8.dp)) + Spacer(modifier = Modifier.height(8.dp))
♻️ Duplicate comments (15)
app/src/main/java/com/texthip/thip/ui/search/component/SearchBookFilteredResult.kt (4)
87-88: LazyColumn 아이템에 안정적인 key를 부여하세요키 없이 itemsIndexed를 사용하면 상태 보존/성능에 불리합니다. isbn을 우선 키로 사용하고 빈 경우 대체 키를 구성하세요.
- itemsIndexed(bookList) { index, book -> + itemsIndexed( + items = bookList, + key = { _, book -> book.isbn.ifBlank { "${book.title}-${book.author}-${book.publisher}" } } + ) { index, book ->
38-44: 빈 결과 화면 콜백(onRequestBook) 누락 — 공개 API에 추가 필요빈 결과 상태에서 “책 요청” 네비게이션/동작을 트리거할 콜백이 컴포저블 API에 없습니다. onRequestBook을 파라미터로 받도록 확장하세요.
fun SearchBookFilteredResult( resultCount: Int, bookList: List<BookData>, isLoading: Boolean = false, hasMore: Boolean = true, onLoadMore: () -> Unit = {}, + onRequestBook: () -> Unit = {}, onBookClick: (BookData) -> Unit = {} ) {원하시면, 전체 호출부/네비게이션 연결까지 변경 PR 패치를 생성해드릴 수 있습니다.
83-123: 빈 결과 화면 UI가 사라졌습니다 — 사용자 가이드 제공 및 콜백 연결 필요bookList가 비었고 isLoading이 아닐 때 아무 UI도 나타나지 않습니다. 기존 UX(빈 화면 + “책 요청” 버튼)를 복구하고 onRequestBook을 전달하세요.
- LazyColumn( - state = listState, - verticalArrangement = Arrangement.spacedBy(12.dp) - ) { + if (bookList.isEmpty() && !isLoading) { + SearchEmptyResult( + mainText = stringResource(R.string.book_no_search_result1), + subText = stringResource(R.string.book_no_search_result2), + onRequestBook = onRequestBook + ) + } else { + LazyColumn( + state = listState, + verticalArrangement = Arrangement.spacedBy(12.dp) + ) { itemsIndexed(bookList) { index, book -> Column { CardBookList( modifier = Modifier.clickable { onBookClick(book) }, title = book.title, author = book.author, publisher = book.publisher, imageUrl = book.imageUrl ) if (index < bookList.size - 1) { Spacer( modifier = Modifier .fillMaxWidth() .height(1.dp) .background(colors.DarkGrey02) ) } } } // 로딩 인디케이터 if (isLoading) { item { Box( modifier = Modifier .fillMaxWidth() .padding(16.dp), contentAlignment = Alignment.Center ) { CircularProgressIndicator( modifier = Modifier.size(24.dp), color = colors.White ) } } } - } + } + }주의: SearchEmptyResult가 다른 패키지에 있다면 import 추가가 필요합니다.
48-56: 무한 스크롤 트리거: remember 키에 listState 추가 및 경계 계산 보강 필요스크롤 변화에 확실히 반응하도록 listState를 remember 키에 포함하세요. 또한 가시 아이템이 없는 초기 레이아웃 타이밍을 안전하게 처리하도록 경계 계산을 보강하는 편이 좋습니다.
아래처럼 수정 제안드립니다:
- val shouldLoadMore by remember(hasMore, isLoading) { + val shouldLoadMore by remember(listState, hasMore, isLoading) { derivedStateOf { val layoutInfo = listState.layoutInfo val totalItemsCount = layoutInfo.totalItemsCount - val lastVisibleItemIndex = (layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: 0) + 1 - - hasMore && !isLoading && totalItemsCount > 0 && lastVisibleItemIndex >= totalItemsCount - 3 + val lastVisibleItemIndex = (layoutInfo.visibleItemsInfo.lastOrNull()?.index ?: -1) + 1 + totalItemsCount > 0 && + hasMore && + !isLoading && + lastVisibleItemIndex >= (totalItemsCount - 3).coerceAtLeast(0) } }app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookGroupScreen.kt (1)
219-229: LazyColumn 아이템에 key 지정으로 안정성/성능 개선재구성 시 상태 보존과 성능을 위해 고유 키를 지정하세요. 이전 리뷰와 동일 제안입니다.
- items(recruitingList) { item -> + items(recruitingList, key = { it.id }) { item ->app/src/main/java/com/texthip/thip/ui/navigator/navigations/SearchNavigation.kt (4)
36-38: 뒤로 가기는 popBackStack 대신 navigateUp 사용 권장상위 그래프/액티비티 백스택과의 일관성이 좋아집니다. 이전 리뷰 재반복.
- onLeftClick = { - navController.popBackStack() - }, + onLeftClick = { + navController.navigateUp() + },
57-59: BookGroup 뒤로 가기도 navigateUp으로 통일popBackStack → navigateUp 교체 제안(일관성).
- onLeftClick = { - navController.popBackStack() - }, + onLeftClick = { + navController.navigateUp() + },
32-35: ISBN 빈값 가드 추가 필요딥링크/라우트 파싱 이슈로 빈 문자열일 수 있으니 즉시 navigateUp으로 빠지는 방어 로직을 넣으세요. 이전 리뷰와 동일 맥락입니다.
- val route = backStackEntry.toRoute<SearchRoutes.BookDetail>() - val isbn = route.isbn + val route = backStackEntry.toRoute<SearchRoutes.BookDetail>() + val isbn = route.isbn + if (isbn.isBlank()) { + navController.navigateUp() + return@composable + }
52-55: BookGroup 라우트도 ISBN 빈값 가드 필요빈 ISBN이면 화면 렌더를 건너뛰고 즉시 뒤로 가도록 처리하세요.
- val route = backStackEntry.toRoute<SearchRoutes.BookGroup>() - val isbn = route.isbn + val route = backStackEntry.toRoute<SearchRoutes.BookGroup>() + val isbn = route.isbn + if (isbn.isBlank()) { + navController.navigateUp() + return@composable + }app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookGroupViewModel.kt (2)
41-44: “동시 로드” 주석과 다르게 실제로는 순차 실행 — 진짜 병렬 실행으로 수정초기 체감 속도에 영향이 큽니다. 두 작업을 자식 코루틴으로 병렬 실행하세요. (과거 리뷰 반복 제안)
- // 책 정보와 모집중인 방 정보 동시 로드 - loadBookDetail(isbn) - loadRooms(isbn, null) + // 책 정보와 모집중인 방 정보 동시 로드 (병렬) + launch { loadBookDetail(isbn) } + launch { loadRooms(isbn, null) }
66-87: ISBN 전환 레이스 가드 추가(모집방 페이징/초기 로드)요청 당시 ISBN과 현재 ISBN이 다르면 응답을 무시해야 데이터 섞임을 방지할 수 있습니다.
- bookRepository.getRecruitingRooms(isbn, cursor) + bookRepository.getRecruitingRooms(isbn, cursor) .onSuccess { response -> + if (isbn != currentIsbn) return@onSuccess response?.let { recruitingRoomsResponse -> val currentRooms = if (cursor == null) emptyList() else _uiState.value.recruitingRooms _uiState.value = _uiState.value.copy( recruitingRooms = currentRooms + recruitingRoomsResponse.recruitingRoomList, totalCount = recruitingRoomsResponse.totalRoomCount, nextCursor = recruitingRoomsResponse.nextCursor, hasMore = !recruitingRoomsResponse.isLast, isLoading = false, isLoadingMore = false, error = null ) } ?: run { _uiState.value = _uiState.value.copy( isLoading = false, isLoadingMore = false, hasMore = false, // null 응답 시 더 이상 로드할 수 없음을 명시 error = if (cursor == null) "모집중인 방 정보를 찾을 수 없습니다." else null ) } }app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookScreen.kt (1)
79-87: 인기 책 데이터 매핑 시 빈 문자열 사용
popularBooks매핑에서author와publisher를 빈 문자열로 설정하고 있습니다. 이는 데이터 일관성 문제를 야기할 수 있습니다.app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt (3)
101-138: 검색 완료 시 모드 플래그 미해제 문제
performSearch의 성공 경로(lines 104-113)에서isLiveSearching과isCompleteSearching플래그를 false로 설정하지 않아서 UI가 계속 "검색 중" 상태로 남아있을 수 있습니다.성공 경로에서도 검색 모드 플래그를 해제해야 합니다:
updateState { it.copy( searchResults = searchResponse.searchResult, currentPage = searchResponse.page, totalElements = searchResponse.totalElements, hasMorePages = !searchResponse.last, isSearching = false, - error = null + error = null, + isLiveSearching = false, + isCompleteSearching = false ) }
101-102: 검색 요청의 스테일 응답 방지 필요빠른 연속 검색 시 이전 요청의 응답이 늦게 도착하여 최신 검색 결과를 덮어쓸 수 있습니다.
요청 시점의 쿼리를 캡처하고 응답 시점에 검증하여 스테일 응답을 무시하도록 개선하세요:
private suspend fun performSearch(query: String, isLiveSearch: Boolean) { + val expectedQuery = query updateState { it.copy( isSearching = true, error = null, searchResults = emptyList(), currentPage = 1 ) } bookRepository.searchBooks(query, 1, isFinalized = !isLiveSearch) .onSuccess { response -> + // 스테일 응답 가드 + if (uiState.value.searchQuery != expectedQuery) return@onSuccess response?.let { searchResponse ->
146-177: 로드 모어에도 스테일 응답 방지 필요검색과 마찬가지로 로드 모어 작업 중에도 사용자가 새 검색을 시작하면 이전 페이지 요청의 결과가 합쳐질 수 있습니다.
로드 모어에도 동일한 스테일 가드를 적용하세요:
private suspend fun performLoadMore() { val currentState = uiState.value val nextPage = currentState.currentPage + 1 + val expectedQuery = currentState.searchQuery updateState { it.copy(isLoadingMore = true) } bookRepository.searchBooks(currentState.searchQuery, nextPage, isFinalized = true) .onSuccess { response -> + // 스테일 응답 가드 + if (uiState.value.searchQuery != expectedQuery) return@onSuccess response?.let { searchResponse ->
🧹 Nitpick comments (11)
app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt (6)
71-77: isLoadingMore=false 설정이 성공/finally에 중복되어 있습니다성공 분기와 finally 모두에서 isLoadingMore=false를 설정합니다. 한 곳(finally)으로 모아 단일 종료 지점을 유지하면 상태 전이가 단순해지고 실수를 줄일 수 있습니다.
다음과 같이 정리하는 것을 권장합니다:
- updateState { - it.copy( - expiredRooms = currentList + response.roomList, - error = null, - isLoadingMore = false, - hasMore = !response.isLast - ) - } + updateState { + it.copy( + expiredRooms = currentList + response.roomList, + error = null, + hasMore = !response.isLast + ) + } ... - updateState { it.copy(hasMore = false, isLoadingMore = false) } + updateState { it.copy(hasMore = false) }Also applies to: 83-83
61-66: 초기 로드 시 이전 에러 상태를 함께 초기화 권장reset 로드에서 이전 오류 메시지가 남아 있을 수 있습니다. 초기화 시 error=null로 함께 정리해 주면 UX가 깔끔합니다.
다음 변경 제안:
- updateState { it.copy(isLoading = true, expiredRooms = emptyList(), hasMore = true) } + updateState { it.copy(isLoading = true, expiredRooms = emptyList(), hasMore = true, error = null) }
40-46: getUserName 실패 케이스도 처리해 주세요현재 onFailure가 없어 실패 시 침묵합니다. 오류 상태를 표출하거나 기본값을 적용하는 편이 안전합니다.
예시:
repository.getUserName() .onSuccess { name -> updateState { it.copy(userName = name) } } + .onFailure { e -> + // 필요 시 Sentry/로그 추가 가능 + updateState { it.copy(error = e.message ?: "사용자 이름을 불러오지 못했습니다.") } + }
8-12: MutableStateFlow.update 확장 함수를 사용해 일관성/원자성 확보다른 VM(SearchBook/GroupMy 등)과 동일하게 update 확장 함수를 사용하면 가독성과 일관성이 좋아집니다.
다음 import를 추가하세요:
import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch
27-29: updateState 구현을 update 확장 함수로 단순화동일 스레드라 해도 확장 함수 사용이 간결하고 안전합니다.
- private fun updateState(update: (GroupDoneUiState) -> GroupDoneUiState) { - _uiState.value = update(_uiState.value) - } + private fun updateState(update: (GroupDoneUiState) -> GroupDoneUiState) { + _uiState.update(update) + }
22-24: hasMore(UI)와 isLastPage(내부) 이중 관리로 불일치 위험동일 개념을 두 변수가 담당해 드리프트 가능성이 있습니다. 한쪽만 소스 오브 트루스로 삼는 구조(예: 내부 isLastPage 제거, UI의 hasMore만 관리)로 단순화하는 것을 권장합니다.
app/src/main/java/com/texthip/thip/ui/search/component/SearchBookFilteredResult.kt (2)
58-62: onLoadMore 트리거를 snapshotFlow로 관찰하는 방안 제안현 구현도 동작하나, 스크롤 변경에 따른 트리거 중복 및 타이밍 문제를 줄이려면 snapshotFlow + distinctUntilChanged로 관찰/호출하는 방식이 더 견고합니다.
예시:
LaunchedEffect(listState, hasMore, isLoading) { snapshotFlow { val info = listState.layoutInfo val total = info.totalItemsCount val last = (info.visibleItemsInfo.lastOrNull()?.index ?: -1) + 1 total > 0 && hasMore && !isLoading && last >= (total - 3).coerceAtLeast(0) } .distinctUntilChanged() .filter { it } .collect { onLoadMore() } }
83-86: 간격 중복 가능성: spacedBy(12.dp) + 항목 내 구분선(1dp)리스트 레벨 간격(12dp)과 아이템 내부 구분선이 같이 적용되어 시각적으로 기대보다 간격이 커질 수 있습니다. 하나로 통일하는 것을 권장합니다.
대안:
- 리스트 간격 유지, 구분선 제거
- 리스트 간격 제거, 아이템 사이에 구분선만 유지
app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookGroupScreen.kt (2)
197-211: 무한 스크롤 임계치 보정 및 중복 트리거 완화 제안
- size < 3인 경우
size - 3이 음수가 되어 첫 렌더 직후 즉시 onLoadMore가 호출될 수 있습니다. 안전하게 max(0, size - 3)로 보정하세요.- snapshotFlow 수집 값에 distinctUntilChanged를 추가하면 같은 인덱스 반복 알림으로 인한 중복 호출을 줄일 수 있습니다.
- LaunchedEffect(listState, canLoadMore, isLoadingMore) { - snapshotFlow { listState.layoutInfo.visibleItemsInfo.lastOrNull()?.index } - .collect { lastVisibleIndex -> - if (lastVisibleIndex != null && - recruitingList.isNotEmpty() && - !isLoadingMore && - lastVisibleIndex >= recruitingList.size - 3 && - canLoadMore) { - onLoadMore() - } - } - } + LaunchedEffect(listState, canLoadMore, isLoadingMore) { + snapshotFlow { listState.layoutInfo.visibleItemsInfo.lastOrNull()?.index } + .distinctUntilChanged() + .collect { lastVisibleIndex -> + val threshold = max(0, recruitingList.size - 3) + if (lastVisibleIndex != null && + recruitingList.isNotEmpty() && + !isLoadingMore && + lastVisibleIndex >= threshold && + canLoadMore) { + onLoadMore() + } + } + }추가로 파일 상단에 다음 import가 필요합니다(이미 있다면 생략):
import kotlin.math.max import kotlinx.coroutines.flow.distinctUntilChanged
251-269: 책 정보 미로딩 시 ‘방 만들기’ 버튼 비활성화 권장bookTitle·imageUrl·author가 아직 로딩되지 않았을 때 빈 값으로 네비게이션되면 UX가 어색합니다. 최소한 title 기준으로 활성화 제어하세요.
- Button( + Button( + enabled = bookTitle.isNotBlank(), colors = ButtonDefaults.buttonColors( containerColor = colors.Purple ),app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookGroupViewModel.kt (1)
28-31: loadMoreJob 취소는 cancelAndJoin으로 완결성 있게 대기(선택)초기 로드 진입 전에 기존 loadMoreJob이 완전히 종료될 때까지 대기하면 레이스 가능성을 더 줄일 수 있습니다. suspend 함수가 아니므로 viewModelScope.launch 블록 안으로 이동해 적용하세요.
- fun loadRecruitingRooms(isbn: String) { - loadMoreJob?.cancel() // 신규 검색 시 이전 로드 작업 취소 - currentIsbn = isbn - viewModelScope.launch { + fun loadRecruitingRooms(isbn: String) { + currentIsbn = isbn + viewModelScope.launch { + // 신규 검색 시작 전 진행 중인 loadMore 작업을 완전히 종료 + loadMoreJob?.cancelAndJoin() _uiState.value = _uiState.value.copy( isLoading = true, error = null, recruitingRooms = emptyList(), nextCursor = null, hasMore = true, totalCount = 0, bookDetail = null )추가 import:
import kotlinx.coroutines.cancelAndJoin
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (49)
app/src/main/AndroidManifest.xml(1 hunks)app/src/main/java/com/texthip/thip/data/manager/Genre.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/request/BookSaveRequest.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/response/BookDetailResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/response/BookListResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/response/BookSaveResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/response/BookSearchResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/response/MostSearchedBooksResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/response/RecentSearchResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/book/response/RecruitingRoomsResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/model/group/response/JoinedRoomListResponse.kt(1 hunks)app/src/main/java/com/texthip/thip/data/repository/BookRepository.kt(2 hunks)app/src/main/java/com/texthip/thip/data/repository/GroupRepository.kt(1 hunks)app/src/main/java/com/texthip/thip/data/service/BookService.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/common/buttons/GenreChipButton.kt(5 hunks)app/src/main/java/com/texthip/thip/ui/common/screen/RegisterBookScreen.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/group/done/screen/GroupDoneScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupSelectBook.kt(3 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomUiState.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupMainCard.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupPager.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/screen/GroupMyScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/room/screen/GroupRoomRecruitScreen.kt(3 hunks)app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/extensions/CommonNavigationExtensions.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/navigator/extensions/GroupNavigationExtensions.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/extensions/SearchNavigationExtensions.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/navigations/CommonNavigation.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/navigator/navigations/GroupNavigation.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/navigations/SearchNavigation.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/routes/CommonRoutes.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/routes/GroupRoutes.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/navigator/routes/SearchRoutes.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/search/component/SearchActiveField.kt(2 hunks)app/src/main/java/com/texthip/thip/ui/search/component/SearchBookFilteredResult.kt(3 hunks)app/src/main/java/com/texthip/thip/ui/search/mock/BookData.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookDetailScreen.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookGroupScreen.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookScreen.kt(4 hunks)app/src/main/java/com/texthip/thip/ui/search/viewmodel/BookDetailViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookGroupViewModel.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookUiState.kt(1 hunks)app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt(1 hunks)app/src/main/res/xml/network_security_config.xml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/src/main/java/com/texthip/thip/data/manager/Genre.kt
🚧 Files skipped from review as they are similar to previous changes (39)
- app/src/main/java/com/texthip/thip/ui/group/makeroom/screen/GroupMakeRoomScreen.kt
- app/src/main/java/com/texthip/thip/ui/group/room/screen/GroupRoomRecruitScreen.kt
- app/src/main/java/com/texthip/thip/ui/navigator/routes/GroupRoutes.kt
- app/src/main/java/com/texthip/thip/data/model/book/response/BookSaveResponse.kt
- app/src/main/res/xml/network_security_config.xml
- app/src/main/java/com/texthip/thip/ui/navigator/extensions/CommonNavigationExtensions.kt
- app/src/main/java/com/texthip/thip/data/model/book/response/BookSearchResponse.kt
- app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupMainCard.kt
- app/src/main/java/com/texthip/thip/ui/group/myroom/screen/GroupMyScreen.kt
- app/src/main/java/com/texthip/thip/data/model/group/response/JoinedRoomListResponse.kt
- app/src/main/java/com/texthip/thip/data/model/book/response/BookListResponse.kt
- app/src/main/java/com/texthip/thip/data/model/book/response/RecruitingRoomsResponse.kt
- app/src/main/java/com/texthip/thip/ui/search/component/SearchActiveField.kt
- app/src/main/java/com/texthip/thip/data/model/book/response/BookDetailResponse.kt
- app/src/main/java/com/texthip/thip/ui/navigator/extensions/GroupNavigationExtensions.kt
- app/src/main/AndroidManifest.xml
- app/src/main/java/com/texthip/thip/data/model/book/response/MostSearchedBooksResponse.kt
- app/src/main/java/com/texthip/thip/ui/group/done/screen/GroupDoneScreen.kt
- app/src/main/java/com/texthip/thip/ui/navigator/navigations/CommonNavigation.kt
- app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomUiState.kt
- app/src/main/java/com/texthip/thip/ui/search/mock/BookData.kt
- app/src/main/java/com/texthip/thip/ui/navigator/routes/SearchRoutes.kt
- app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt
- app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt
- app/src/main/java/com/texthip/thip/data/model/book/request/BookSaveRequest.kt
- app/src/main/java/com/texthip/thip/ui/navigator/extensions/SearchNavigationExtensions.kt
- app/src/main/java/com/texthip/thip/data/repository/GroupRepository.kt
- app/src/main/java/com/texthip/thip/data/service/BookService.kt
- app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookUiState.kt
- app/src/main/java/com/texthip/thip/ui/group/screen/GroupScreen.kt
- app/src/main/java/com/texthip/thip/ui/group/myroom/component/GroupPager.kt
- app/src/main/java/com/texthip/thip/ui/navigator/navigations/GroupNavigation.kt
- app/src/main/java/com/texthip/thip/ui/search/viewmodel/BookDetailViewModel.kt
- app/src/main/java/com/texthip/thip/ui/common/buttons/GenreChipButton.kt
- app/src/main/java/com/texthip/thip/ui/group/makeroom/component/GroupSelectBook.kt
- app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookDetailScreen.kt
- app/src/main/java/com/texthip/thip/data/model/book/response/RecentSearchResponse.kt
- app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt
- app/src/main/java/com/texthip/thip/data/repository/BookRepository.kt
🧰 Additional context used
🧬 Code Graph Analysis (6)
app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt (5)
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt (1)
updateState(35-37)app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt (1)
updateState(26-28)app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (1)
updateState(33-35)app/src/main/java/com/texthip/thip/ui/group/makeroom/viewmodel/GroupMakeRoomViewModel.kt (1)
updateState(37-39)app/src/main/java/com/texthip/thip/ui/group/room/viewmodel/GroupRoomRecruitViewModel.kt (1)
updateState(29-31)
app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookGroupScreen.kt (3)
app/src/main/java/com/texthip/thip/ui/common/topappbar/DefaultTopAppBar.kt (1)
DefaultTopAppBar(22-65)app/src/main/java/com/texthip/thip/ui/common/cards/CardItemRoom.kt (1)
CardItemRoom(35-170)app/src/main/java/com/texthip/thip/ui/theme/Theme.kt (1)
ThipTheme(41-69)
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt (5)
app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt (2)
loadInitialData(35-38)updateState(27-29)app/src/main/java/com/texthip/thip/ui/group/viewmodel/GroupViewModel.kt (2)
loadInitialData(41-45)updateState(33-35)app/src/main/java/com/texthip/thip/ui/group/myroom/viewmodel/GroupMyViewModel.kt (1)
updateState(26-28)app/src/main/java/com/texthip/thip/data/repository/BookRepository.kt (1)
deleteRecentSearch(51-55)app/src/main/java/com/texthip/thip/data/service/BookService.kt (1)
deleteRecentSearch(46-49)
app/src/main/java/com/texthip/thip/ui/search/component/SearchBookFilteredResult.kt (2)
app/src/main/java/com/texthip/thip/ui/common/cards/CardBookList.kt (1)
CardBookList(30-89)app/src/main/java/com/texthip/thip/ui/theme/Theme.kt (1)
ThipTheme(41-69)
app/src/main/java/com/texthip/thip/ui/navigator/navigations/SearchNavigation.kt (3)
app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookScreen.kt (1)
SearchBookScreen(40-116)app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookDetailScreen.kt (1)
SearchBookDetailScreen(55-120)app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookGroupScreen.kt (1)
SearchBookGroupScreen(45-91)
app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookScreen.kt (5)
app/src/main/java/com/texthip/thip/ui/search/component/SearchRecentBook.kt (1)
SearchRecentBook(30-138)app/src/main/java/com/texthip/thip/ui/search/component/SearchActiveField.kt (1)
SearchActiveField(29-97)app/src/main/java/com/texthip/thip/ui/search/component/SearchEmptyResult.kt (1)
SearchEmptyResult(21-55)app/src/main/java/com/texthip/thip/ui/search/component/SearchBookFilteredResult.kt (1)
SearchBookFilteredResult(36-125)app/src/main/java/com/texthip/thip/ui/theme/Theme.kt (1)
ThipTheme(41-69)
🔇 Additional comments (19)
app/src/main/java/com/texthip/thip/ui/group/done/viewmodel/GroupDoneViewModel.kt (1)
81-84: null 응답 시 페이징 종료 처리 추가한 점 좋습니다null 응답을 명시적 종료(hasMore=false, isLastPage=true)로 해석해 중복 로드와 불필요한 네트워크 호출을 차단합니다. 일관된 상태 전이가 보장됩니다.
app/src/main/java/com/texthip/thip/ui/search/component/SearchBookFilteredResult.kt (2)
70-73: 문구/리소스 확인 필요: group_searched_room_size 사용책 검색 결과인데 “group_searched_room_size” 문자열 리소스를 사용하고 있습니다. 책 전용 리소스가 있다면 교체하세요. 없다면 신규 리소스 추가를 권장합니다.
108-121: 로딩 인디케이터를 LazyColumn footer로 추가한 구성은 적절합니다하단에 item으로 배치해 스크롤 문맥을 해치지 않으며, isLoading 조건부로 깔끔하게 제어됩니다.
app/src/main/java/com/texthip/thip/ui/common/screen/RegisterBookScreen.kt (2)
22-25: API 확장 LGTM — 기본값으로 역호환성 유지onLeftClick 파라미터 추가가 적절하고, 기본값으로 기존 호출부와의 호환성도 유지됩니다.
34-35: TopAppBar 콜백 연동 적절DefaultTopAppBar에 onLeftClick을 전달하도록 변경한 부분 문제 없습니다.
app/src/main/java/com/texthip/thip/ui/navigator/routes/CommonRoutes.kt (1)
10-11: RegisterBook 라우트 추가 LGTM직렬화 가능한 sealed object로 정의가 일관적이며, 기존 Alarm과의 호환성에도 문제 없습니다.
app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookGroupScreen.kt (1)
59-71: 도메인 → UI 매핑 간결하고 명확daysLeft 계산 및 GroupCardItemRoomData로의 매핑이 깔끔합니다. 이후 LazyColumn 렌더링과의 인터페이스도 잘 맞습니다.
app/src/main/java/com/texthip/thip/ui/search/screen/SearchBookScreen.kt (6)
43-46: 함수 시그니처 변경으로 MVVM 패턴 적용함수 시그니처가 기존의 단순한 데이터 전달 방식에서 ViewModel을 활용한 MVVM 패턴으로 개선되었습니다.
hiltViewModel()을 기본값으로 하면서도 테스트용 ViewModel 주입이 가능하도록 설계되어 있어 좋습니다.
50-62: 생명주기 인식 데이터 새로고침
DisposableEffect와LifecycleEventObserver를 사용하여 화면이 다시 활성화될 때 데이터를 자동으로 새로고침하는 로직이 잘 구현되어 있습니다. 메모리 누수 방지를 위한 observer 정리도 적절합니다.
95-116: ViewModel과 UI 간의 데이터 바인딩ViewModel의 상태를 UI 컴포넌트로 적절하게 전달하는 매핑 로직이 잘 구현되어 있습니다. 각 콜백 함수들이 ViewModel의 메서드와 올바르게 연결되어 있어 MVVM 패턴을 효과적으로 활용하고 있습니다.
118-141: UI 상태별 렌더링 로직 분리
SearchBookScreenContent로 UI 렌더링 로직을 분리하여 코드의 가독성과 테스트 가능성을 높였습니다. 다양한 UI 상태(isInitial,isLiveSearching,isCompleteSearching)를 명확하게 구분하여 처리하는 것이 좋습니다.
175-183: 초기 상태에서 날짜 포맷팅인기 책 목록의 날짜를
SimpleDateFormat으로 "MM.dd" 형식으로 포맷팅하고 있습니다. 현재 날짜를 사용하는 것이 맞는지 확인이 필요합니다.실제 인기 책 데이터의 날짜를 사용해야 하는지, 아니면 현재 날짜를 사용하는 것이 올바른 요구사항인지 확인해주세요.
224-273: Preview 데이터 및 함수 추가다양한 UI 상태를 테스트할 수 있는 Preview 함수들과 Mock 데이터가 잘 구성되어 있습니다. 초기 상태, 라이브 검색, 완료 검색, 빈 상태 등 모든 주요 시나리오를 커버하고 있어 개발 및 디자인 검증에 도움이 됩니다.
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookViewModel.kt (6)
17-30: ViewModel 구조 및 의존성 주입
@HiltViewModel과 생성자 주입을 통한 의존성 관리가 적절하게 구현되어 있습니다.recentSearchMap을 통한 키워드 기반 빠른 검색 최적화도 좋은 접근입니다.
35-37: StateFlow 업데이트 방식 개선
MutableStateFlow.update를 사용하여 원자적 상태 업데이트를 구현한 것이 좋습니다. 이전 리뷰에서 지적된 동시 업데이트 안전성 문제가 해결되었습니다.
39-59: 디바운스 검색 로직1초 디바운스와 함께 라이브 검색을 구현한 것이 좋습니다. 이전 작업을 취소하는 로직도 적절하게 구현되어 있어 불필요한 네트워크 요청을 방지합니다.
61-79: 검색 버튼 클릭 처리완전 검색으로 전환 시 기존 작업들을 취소하고 최근 검색어를 업데이트하는 로직이 잘 구현되어 있습니다. 이전 리뷰에서 지적된
loadMoreJob취소 누락도 해결되었습니다.
234-239: 키워드 기반 최근 검색어 삭제 최적화Map 기반의 빠른 키워드 검색을 통한 최근 검색어 삭제 로직이 효율적으로 구현되어 있습니다. UI에서 키워드만으로 삭제할 수 있어 사용성이 좋습니다.
264-268: 리소스 정리
onCleared에서 진행 중인 코루틴 작업들을 취소하여 메모리 누수를 방지하는 것이 적절합니다.
| import com.texthip.thip.R | ||
| import com.texthip.thip.ui.search.mock.BookData | ||
| import com.texthip.thip.ui.common.cards.CardBookList | ||
| import com.texthip.thip.ui.search.mock.BookData |
There was a problem hiding this comment.
🛠️ Refactor suggestion
컴포저블 공개 API가 mock 모델에 의존하고 있습니다
SearchBookFilteredResult가 ui.search.mock.BookData에 직접 의존하면 테스트/모듈성/교체 용이성이 저하됩니다. UI 전용 모델(예: ui.model.BookListItemUiModel)로 공개 API를 분리하고, ViewModel에서 API/엔터티 → UI 모델 매핑을 수행하세요.
| bookRepository.getBookDetail(isbn) | ||
| .onSuccess { bookDetail -> | ||
| _uiState.value = _uiState.value.copy(bookDetail = bookDetail) | ||
| } | ||
| .onFailure { } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
ISBN 전환 레이스 가드 추가(책 상세)
이전 요청의 늦은 응답이 현재 화면을 덮어쓰지 않도록 현재 ISBN과 요청 ISBN을 비교해 반영을 가드하세요.
- bookRepository.getBookDetail(isbn)
+ bookRepository.getBookDetail(isbn)
.onSuccess { bookDetail ->
- _uiState.value = _uiState.value.copy(bookDetail = bookDetail)
+ if (isbn != currentIsbn) return@onSuccess
+ _uiState.value = _uiState.value.copy(bookDetail = bookDetail)
}
.onFailure { }📝 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.
| bookRepository.getBookDetail(isbn) | |
| .onSuccess { bookDetail -> | |
| _uiState.value = _uiState.value.copy(bookDetail = bookDetail) | |
| } | |
| .onFailure { } | |
| } | |
| bookRepository.getBookDetail(isbn) | |
| .onSuccess { bookDetail -> | |
| // 다른 ISBN으로 전환되었다면 이 응답은 무시 | |
| if (isbn != currentIsbn) return@onSuccess | |
| _uiState.value = _uiState.value.copy(bookDetail = bookDetail) | |
| } | |
| .onFailure { } | |
| } |
🤖 Prompt for AI Agents
In
app/src/main/java/com/texthip/thip/ui/search/viewmodel/SearchBookGroupViewModel.kt
around lines 48 to 53, the success handler unconditionally updates _uiState with
the returned bookDetail which can overwrite a newer detail when ISBNs change;
guard the update by comparing the ISBN used for the request with the current UI
state's ISBN and only set _uiState.value.copy(bookDetail = bookDetail) if they
match (i.e., if the requestIsbn == _uiState.value.currentIsbn or the existing
bookDetail?.isbn), otherwise ignore the result.
Nico1eKim
left a comment
Nico1eKim
left a comment
There was a problem hiding this comment.
수고하셨습니다 ~ 이번에도 수정된 코드가 짱많네요 히익 !
|
|
||
| /** 저장된 책 또는 모임 책 목록 조회 */ | ||
| suspend fun getBooks(type: String) = runCatching { | ||
| suspend fun getBooks(type: String): Result<List<BookSavedResponse>> = runCatching { |
There was a problem hiding this comment.
여기에서 BookSavedResponse를 List로 감싸기보다는 애초에 List로 감싸진걸 BookSavedResponse로 만드는건 어떤가요?
| .getOrThrow()!! | ||
| .type | ||
| .getOrThrow() | ||
| ?: throw NoSuchElementException("모임방 참여/취소 응답을 받을 수 없습니다.") |
There was a problem hiding this comment.
이거 .handleBaseResponse().getOrThrow()이렇게 수정부탁드립니둥 ~
| CircularProgressIndicator( | ||
| modifier = Modifier.size(24.dp), | ||
| color = colors.White | ||
| ) |
There was a problem hiding this comment.
이거 저희 한번 다같이 로딩중일 때 ui 통일하면 좋을듯요? 일단 디자이너님들한테 부탁은 해놓은 상태입니다
| Box( | ||
| modifier = Modifier | ||
| .padding(start = 12.dp) | ||
| .size(44.dp) | ||
| .border( | ||
| width = 1.dp, | ||
| color = colors.Grey02, | ||
| shape = RoundedCornerShape(12.dp) | ||
| ) | ||
| .background( | ||
| color = Color.Transparent, | ||
| shape = RoundedCornerShape(12.dp) | ||
| ) | ||
| .clickable { | ||
| val newBookmarkState = !isBookmarked | ||
| isBookmarked = newBookmarkState | ||
| onBookmarkClick(bookDetail.isbn, newBookmarkState) | ||
| }, | ||
| contentAlignment = Alignment.Center, | ||
| ) { | ||
| Icon( | ||
| painter = painterResource( | ||
| if (isBookmarked) | ||
| R.drawable.ic_save_filled | ||
| else | ||
| R.drawable.ic_save | ||
| ), | ||
| contentDescription = null, | ||
| tint = Color.Unspecified, | ||
| modifier = Modifier.size(24.dp) | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
hoxy 이거 북마크를 위한 코드인가요? 그렇다면 ActionMediumButton에 북마크 true로 하시면 됩니다
| DisposableEffect(lifecycleOwner) { | ||
| val observer = LifecycleEventObserver { _, event -> | ||
| if (event == Lifecycle.Event.ON_RESUME) { | ||
| viewModel.refreshData() |
| } | ||
| } | ||
| .onFailure { | ||
| // 인기 책 로딩 실패는 조용히 처리 |
2 participants
➕ 이슈 링크
🔎 작업 내용
📸 스크린샷
😢 해결하지 못한 과제
책 검색 로직이 수~목 중에 수정된다고 합니다
해당 책으로 모집중인 모임방 화면에서 모임방 생성 화면으로 연결하는 로직은 구현 중에 있습니다.
📢 리뷰어들에게
Summary by CodeRabbit
신기능
내비게이션
UI/UX
기타