fix: 도서 검색 DTO 변경사항에 따른 LastPage 로직 수정 및 빈 ISBN 대응 로직 추가#135
Merged
seoyoon513 merged 4 commits intodevelopfrom Aug 10, 2025
Merged
fix: 도서 검색 DTO 변경사항에 따른 LastPage 로직 수정 및 빈 ISBN 대응 로직 추가#135seoyoon513 merged 4 commits intodevelopfrom
seoyoon513 merged 4 commits intodevelopfrom
Conversation
- LazyColumn item key를 title-isbn13 조합으로 변경 - BookItem 클릭 시 isbn이 비어 있으면 토스트 메세지로 안내
Walkthrough도서 검색 API의 DTO 구조가 변경되어, 기존의 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BookSearchPresenter
participant Repository
participant Network
participant UI
User->>BookSearchPresenter: 검색 요청
BookSearchPresenter->>Repository: 도서 목록 요청
Repository->>Network: API 호출
Network-->>Repository: BookSearchResponse(lastPage, books[link,...])
Repository-->>BookSearchPresenter: BookSearchModel(lastPage, books[key, link,...])
BookSearchPresenter-->>UI: UI 상태 갱신
User->>UI: 도서 항목 클릭
UI->>BookSearchPresenter: OnBookClick 이벤트
BookSearchPresenter-->>UI: (isbn 없으면) 토스트, (있으면) 등록 시트 표시
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ 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 comments)
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: 4
🧹 Nitpick comments (4)
build-logic/src/main/kotlin/com/ninecraft/booket/convention/ApplicationConstants.kt (1)
9-9: VERSION_CODE 증가 OK. 배포 정책과 동기화 확인 권장배포 대상이라면 VERSION_NAME, 릴리즈 노트(체인지로그), 스토어 메타데이터 자동화와의 정합성도 함께 확인해 주세요.
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt (1)
81-88: lastPage 플래그 사용으로 전환한 점 적절합니다페이지 크기 비교 대신 서버의 lastPage를 신뢰하도록 바뀐 점 좋습니다. 추가로 초기 검색(startIndex == START_INDEX)에서도 lastPage가 true라면 footerState를 End로 맞춰 두면 UI 일관성이 더 좋습니다.
예:
if (startIndex == START_INDEX) { uiState = UiState.Success footerState = if (isLastPage) FooterState.End else FooterState.Idle } else { footerState = if (isLastPage) FooterState.End else FooterState.Idle }core/network/src/main/kotlin/com/ninecraft/booket/core/network/response/BookSearchResponse.kt (1)
44-45: BookSummary.link도 기본값 지정으로 디코딩 안정성 확보초기/구버전 응답에서 link가 누락될 가능성을 고려해 기본값을 지정하면 안정적입니다.
- @SerialName("link") - val link: String, + @SerialName("link") + val link: String = "",실제 서버 응답 스키마에 link가 항상 존재하는지 확인 부탁드립니다.
core/model/src/main/kotlin/com/ninecraft/booket/core/model/BookSearchModel.kt (1)
27-27: BookSummaryModel.link 추가: 용도 명확화 및 빈 값 처리 확인
- 필드 추가 방향성 맞습니다. UI/네비게이션에서 상대/절대 URL 표준화 여부만 확인해 주세요.
- 링크가 빈 문자열일 수 있으므로 클릭 가드/비활성화 처리 유무 점검 권장합니다. (ISBN 빈 값 처리와 동일 맥락)
권장 주석 추가 예시:
- val link: String = "", + // 도서 상세 페이지 링크(절대 URL). 없거나 접근 불가하면 빈 문자열 유지. + val link: String = "",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
build-logic/src/main/kotlin/com/ninecraft/booket/convention/ApplicationConstants.kt(1 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/mapper/ResponseToModel.kt(2 hunks)core/model/src/main/kotlin/com/ninecraft/booket/core/model/BookSearchModel.kt(2 hunks)core/network/src/main/kotlin/com/ninecraft/booket/core/network/response/BookSearchResponse.kt(2 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt(2 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUi.kt(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#46
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/component/InfiniteLazyColumn.kt:83-95
Timestamp: 2025-07-14T00:46:03.843Z
Learning: seoyoon513과 팀은 한국어 주석을 선호하며, 한국어 주석을 영어로 번역하라는 제안을 하지 않아야 함
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (3)
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUi.kt (1)
224-226: BookSearchUi key 생성 로직 검증 필요UI에서 item key로
state.books[index].key를 사용합니다:count = state.books.size, key = { index -> state.books[index].key },이
key가 mapper에서$title-$isbn13형태로 생성된다면, ISBN이 빈 경우 여러 도서가 동일한 key를 가져 충돌 위험이 있습니다. 아래를 확인해 주세요.
- BookSearchPresenter.kt (또는 관련 mapper)
- UI 모델에
key를 할당하는 부분:"$title-$isbn13"패턴과 ISBN 공백 시 fallback 처리 여부- ISBN이 빈 도서가 2개 이상인 검색 결과에서 스크롤/리컴포지션 시 아이템 깜빡임·스킵 현상 테스트
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/mapper/ResponseToModel.kt (2)
64-66: lastPage 매핑 반영 LGTM서버의 페이지 종료 플래그를 모델로 그대로 전달하는 변경이 적절합니다.
76-79: ISBN이 비어있을 때 link를 Fallback으로 사용해 Key 충돌 방지 제안ISBN이 없는 항목이 여러 개일 경우
$title-형태의 중복 key가 발생할 수 있습니다. link 값을 활용해 고유성을 확보하는 것을 권장합니다.대상 파일 및 위치:
- core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/mapper/ResponseToModel.kt (76–79행)
제안 Diff:
- key = "$title-$isbn13", + key = if (isbn13.isNotBlank()) "$title-$isbn13" else "$title-$link",변경 후, 동일 제목·빈 ISBN 도서가 여러 개 노출되는 검색 결과에서 Compose 리스트의 key 충돌이 재현되지 않는지 확인해 주세요.
| val query: String = "", | ||
| val searchCategoryId: Int = 0, | ||
| val searchCategoryName: String = "", | ||
| val lastPage: Boolean = false, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
마지막 페이지 플래그 추가 LGTM — 네이밍(nit)과 하위호환 확인 요청
- 서버 DTO 변경 반영 OK.
- Kotlin 관례상 Boolean은
isLastPage네이밍도 고려 가능하나, API와의 정합성이 더 중요하다면 현 상태 유지도 무방합니다. (nit) - 구버전/필드 미존재 응답 대비로, 매퍼에서
lastPage누락 시 fallback 계산(예:books.size < itemsPerPage)을 두었는지 확인 부탁드립니다. - Presenter/UI가 개수 기반 판별 로직에서
lastPage기반으로 전환되었는지 잔존 로직 점검 권장.
다음 스크립트로 매핑 및 사용처를 빠르게 확인할 수 있습니다:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== lastPage 매핑 위치 탐색 =="
fd -p "ResponseToModel.kt" | xargs -I{} rg -n "BookSearchModel\\(" "{}" -n -A 10 || true
echo
echo "== lastPage 사용처 탐색 =="
rg -n "lastPage" -A 3 || true
echo
echo "== (레거시) 개수 기반 마지막 페이지 판별 잔존 여부 =="
rg -n "books\\.size\\s*[<!=]" -A 2 || trueLength of output: 2808
마지막 페이지 플래그 누락 시 안전 대체 로직 추가 필요
- core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/mapper/ResponseToModel.kt 에서
lastPage를 단순 매핑만 하고 있어, JSON 응답에 해당 필드가 없으면 파싱 오류가 발생할 수 있습니다.
→lastPage를 nullable로 변경하거나, 매퍼에서 누락 시books.size < itemsPerPage기반으로 fallback 처리 로직을 추가해야 합니다. - feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt (82행) 에서
isLastPage = result.lastPage로 전환되어 있으며, 개수(size) 기반 판별 로직은 더 이상 남아 있지 않습니다.
예시 수정안 (ResponseToModel.kt):
fun BookSearchResponse.toModel(): BookSearchModel = BookSearchModel(
version = version,
title = title,
pubDate = pubDate,
totalResults = totalResults,
startIndex = startIndex,
itemsPerPage = itemsPerPage,
query = query,
searchCategoryId = searchCategoryId,
searchCategoryName = searchCategoryName,
- lastPage = lastPage,
+ lastPage = lastPage ?: (books.size < itemsPerPage),
books = books.map { it.toModel() },
)위와 같이 매퍼 레벨에서 하위 호환과 필드 미존재 시에도 정상 동작을 보장하도록 수정해주세요.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In core/model/src/main/kotlin/com/ninecraft/booket/core/model/BookSearchModel.kt
at line 16, the lastPage property is currently non-nullable and defaults to
false, which can cause parsing errors if the JSON response lacks this field. To
fix this, make lastPage nullable or update the mapper in
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/mapper/ResponseToModel.kt
to provide a fallback by checking if books.size is less than itemsPerPage when
lastPage is missing. This ensures backward compatibility and prevents errors
when the field is absent.
| val coverImageUrl: String = "", | ||
| val link: String = "", | ||
| val userBookStatus: String = "", | ||
| val key: String = "", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
key의 고유성/안정성 보장 필요(빈 ISBN 시 충돌 가능성)
- UI 무한 스크롤의 item key는 “유일”하고 “재조합에도 불변”해야 합니다.
- 현재 매퍼에서
title + isbn13조합으로 생성한다면(요약에 따르면),isbn13이 빈 경우 동명서·동저자 도서에서 충돌 가능성이 있습니다. - 제안: 매퍼에서 아래 우선순위로 생성
isbn13이 존재하면isbn13- 없으면
link(상세 URL) - 그마저 없으면
title|author|publisher|coverImageUrl조합 문자열(또는 해시)
권장 주석 추가 예시:
- val key: String = "",
+ // UI 리스트 key(고유/불변). isbn13 우선, 없으면 link, 그마저 없으면 대표 특성 조합으로 생성.
+ val key: String = "",매퍼 예시(외부 파일 참고용):
// core/data/impl/.../ResponseToModel.kt
fun BookSummary.toModel(): BookSummaryModel = BookSummaryModel(
isbn13 = isbn13.orEmpty(),
title = title.orEmpty(),
author = author.orEmpty(),
publisher = publisher.orEmpty(),
coverImageUrl = coverImageUrl.orEmpty(),
link = link.orEmpty(),
userBookStatus = userBookStatus.orEmpty(),
key = when {
!isbn13.isNullOrBlank() -> isbn13
!link.isNullOrBlank() -> link
else -> listOf(title, author, publisher, coverImageUrl).joinToString("|") { it.orEmpty() }
}
)빠른 확인 스크립트:
필요하시면 테스트 케이스(빈 ISBN/중복 title 시 key 충돌 방지)도 함께 작성해 드리겠습니다.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== key 생성 로직 확인(ResponseToModel) =="
fd -p "ResponseToModel.kt" | xargs -I{} rg -n "key\\s*=" "{}" -n -A 3 -B 3 || true
echo
echo "== Compose 리스트에서 key 사용 확인 =="
rg -n "key\\s*=\\s*\\{\\s*it\\.key\\s*\\}" -A 1 -B 1 || trueLength of output: 471
UI 리스트 key 고유성/불변성 보장 필요: key 생성 로직 및 Compose key 지정 검토
현재 확인 결과
- ResponseToModel.kt(75–79행)에서
로직을 사용하고 있어 isbn13이 빈 문자열일 경우
key = "$title-$isbn13",
"$title-"형태가 되어 동명·동저자 도서 간 충돌 가능성이 있습니다. - Compose 리스트(items) 호출부에서는
key = { it.key }를 지정하지 않아 기본 인덱스 기반 key 사용 중으로 보입니다.
◆ 제안 사항
- Mapper(key) 생성 로직 개선
fun BookSummary.toModel(): BookSummaryModel = BookSummaryModel(
// …기존 필드…
- key = "$title-$isbn13",
+ key = when {
+ !isbn13.isNullOrBlank() -> isbn13
+ !link.isNullOrBlank() -> link
+ else -> listOf(title, author, publisher, coverImageUrl)
+ .joinToString("|") { it.orEmpty() }
+ }
)- BookSearchModel.kt에 주석 추가
- val key: String = "",
+ // UI 리스트 key(고유/불변): isbn13 우선, 없으면 link, 그마저 없으면 주요 특성 결합
+ val key: String = "",- Compose
LazyColumn등에서 key 파라미터 지정
- items(bookList) { item -> … }
+ items(bookList, key = { it.key }) { item -> … }테스트 케이스(빈 ISBN·동명 도서 충돌 방지) 추가 작성도 권장드립니다.
🤖 Prompt for AI Agents
In core/model/src/main/kotlin/com/ninecraft/booket/core/model/BookSearchModel.kt
at line 29, the current key generation logic uses "$title-$isbn13" which can
cause key collisions if isbn13 is empty, resulting in non-unique keys for books
with the same title. To fix this, improve the key generation logic to ensure
uniqueness and immutability, for example by including a fallback or a unique
identifier when isbn13 is empty. Add a comment in BookSearchModel.kt explaining
the key's purpose and uniqueness requirements. Also, update the Compose
LazyColumn or similar UI components to explicitly specify the key parameter
using the improved key property to avoid default index-based keys. Finally, add
test cases to cover scenarios with empty ISBNs and duplicate titles to prevent
collisions.
Comment on lines
+26
to
+27
| @SerialName("lastPage") | ||
| val lastPage: Boolean, |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
lastPage 추가 시 백워드 호환성 고려(기본값 지정 권장)
kotlinx.serialization 사용 시, 구버전 서버 응답에 lastPage가 없으면 디코딩 실패할 수 있습니다. 기본값을 두면 안전합니다.
서버/클라이언트 배포 타이밍이 완전히 동기화되어 있다면 스킵 가능하지만, 그렇지 않다면 아래처럼 기본값을 권장합니다.
- @SerialName("lastPage")
- val lastPage: Boolean,
+ @SerialName("lastPage")
+ val lastPage: Boolean = false,lastPage 및 link 필드에 기본값 추가(백워드 호환성 확보)
kotlinx.serialization 사용 시, 서버 응답에 해당 키가 없으면 MissingFieldException이 발생합니다. 서버·클라이언트 배포 시점이 완전히 동기화되지 않을 경우를 대비해 다음과 같이 기본값을 지정하세요:
- 대상 파일:
core/network/src/main/kotlin/com/ninecraft/booket/core/network/response/BookSearchResponse.kt - 적용 위치:
@SerialName("lastPage")및@SerialName("link")선언부
@@ core/network/src/main/kotlin/com/ninecraft/booket/core/network/response/BookSearchResponse.kt
- @SerialName("lastPage")
- val lastPage: Boolean,
+ @SerialName("lastPage")
+ val lastPage: Boolean = false,
- @SerialName("link")
- val link: String,
+ @SerialName("link")
+ val link: String = "",이렇게 설정하면 구버전 서버 응답에서도 안전하게 디코딩할 수 있습니다.
🤖 Prompt for AI Agents
In
core/network/src/main/kotlin/com/ninecraft/booket/core/network/response/BookSearchResponse.kt
around lines 26 to 27, add default values to the lastPage and link fields to
prevent MissingFieldException during deserialization when the server response
lacks these keys. Modify the declarations by assigning appropriate default
values (e.g., false for lastPage and an empty string or null for link) to ensure
backward compatibility with older server versions.
Comment on lines
186
to
+192
| selectedBookIsbn = event.isbn13 | ||
| isBookRegisterBottomSheetVisible = true | ||
|
|
||
| if (selectedBookIsbn.isEmpty()) { | ||
| sideEffect = BookSearchSideEffect.ShowToast("isbn이 없는 도서는 등록할 수 없습니다") | ||
| } else { | ||
| isBookRegisterBottomSheetVisible = true | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
빈 ISBN 처리 시 상태 오염 방지 및 견고성 개선(isEmpty→isBlank, 할당 위치 이동)
빈 ISBN이면 하단시트를 열지 않으므로 selectedBookIsbn을 미리 세팅하지 않는 편이 상태 일관성에 유리합니다. 공백 포함 케이스도 방지하려면 isBlank 사용을 권장합니다.
- selectedBookIsbn = event.isbn13
-
- if (selectedBookIsbn.isEmpty()) {
+ if (event.isbn13.isBlank()) {
sideEffect = BookSearchSideEffect.ShowToast("isbn이 없는 도서는 등록할 수 없습니다")
} else {
+ selectedBookIsbn = event.isbn13
isBookRegisterBottomSheetVisible = true
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| selectedBookIsbn = event.isbn13 | |
| isBookRegisterBottomSheetVisible = true | |
| if (selectedBookIsbn.isEmpty()) { | |
| sideEffect = BookSearchSideEffect.ShowToast("isbn이 없는 도서는 등록할 수 없습니다") | |
| } else { | |
| isBookRegisterBottomSheetVisible = true | |
| } | |
| if (event.isbn13.isBlank()) { | |
| sideEffect = BookSearchSideEffect.ShowToast("isbn이 없는 도서는 등록할 수 없습니다") | |
| } else { | |
| selectedBookIsbn = event.isbn13 | |
| isBookRegisterBottomSheetVisible = true | |
| } |
🤖 Prompt for AI Agents
In
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt
around lines 186 to 192, avoid setting selectedBookIsbn before validating it
because if the ISBN is blank, the bottom sheet should not open and the state
could become inconsistent. Move the assignment of selectedBookIsbn inside the
else block after confirming the ISBN is not blank, and replace the isEmpty check
with isBlank to handle cases with whitespace-only strings.
easyhooon
approved these changes
Aug 10, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 관련 이슈
📙 작업 설명
🧪 테스트 내역
💬 추가 설명 or 리뷰 포인트
Summary by CodeRabbit
신규 기능
버그 수정
개선 사항
기타