feat: 도서 상세 API 변경 사항 반영 및 V2 suffix 제거 #253
Open
Conversation
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
WalkthroughReadingRecord 관련 모델·네트워크·매퍼·UI가 V2 규격(대표 감정 primaryEmotion, detailEmotions, nullable pageNumber)로 통합·갱신되었고, 감정 분석 유틸·여러 컴포저블 안정성 덤프 항목이 제거/재분류되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI (Presenter/Composables)
participant Repo as RecordRepository
participant Service as ReedService
participant Network as Network (HTTP)
participant Mapper as ResponseToModel
UI->>Repo: 요청 (post/get/edit) {userBookId / readingRecordId, payload}
Repo->>Service: 호출 (api/v2/reading-records/...)
Service->>Network: HTTP request
Network-->>Service: HTTP response (ReadingRecord / ReadingRecordV2 shape)
Service-->>Repo: 응답 DTO
Repo->>Mapper: response.toModel()
Mapper-->>Repo: ReadingRecordModel (primaryEmotion, detailEmotions, pageNumber?)
Repo-->>UI: Result<ReadingRecordModel>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
대표 감정을 representativeEmotion으로 내려주는 관계로, 네이티브 코드내에서 대표 감정을 선출하는 로직 제거
각 Emotion 네이밍 warmth, joy, sadness, insight, other로 통일(리소스 네이밍 수정)
사용하지 않는 dto 클래스 제거
누락된 model 모듈 @stable -> @immutable 추가 적용
Contributor
Author
@seoyoon513 이거 생각해보니, 화면내에 모든 기록들은 같은 도서 정보를 가지고 있으므로 도서 상세화면 진입시 받아온 도서 정보를 매핑해서 문제 해결할 수 있었습니다. 해당 커밋의 방법처럼 fallback 처리만 적용하면 문제 없이 정상 동작하여 작업 마무리 했습니다! 0575085 |
Contributor
|
@easyhooon /seed/stats API에서 기타 감정 개수가 내려오지 않는 것 같은데 확인 된 부분일까요? {
"categories": [
{
"name": "따뜻함",
"count": 0
},
{
"name": "즐거움",
"count": 2
},
{
"name": "슬픔",
"count": 2
},
{
"name": "깨달음",
"count": 0
}
]
} |
seoyoon513
reviewed
Jan 24, 2026
| @Serializable | ||
| data class ReadingRecordsResponse( | ||
| @SerialName("representativeEmotion") | ||
| val representativeEmotion: PrimaryEmotion, |
Contributor
There was a problem hiding this comment.
기록 없이 책만 등록했을 때, 해당 필드 null로 내려와서 null 체크 필요할 것 같습니다~!
Contributor
Author
There was a problem hiding this comment.
아 그렇겠네요. 기록이 없을때가 있군요
책만 등록하고 기록을 안 할 경우, 대표 감정은 null로 내려옴
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/mapper/ResponseToModel.kt`:
- Around line 277-281: Category.toEmotionModel() currently returns null when
EmotionCode.fromDisplayName(name) fails, which causes mapNotNull to drop that
category and lose stats; change this to fall back to a default/OTHER EmotionCode
(e.g., EmotionCode.OTHER) instead of returning null and construct the
EmotionModel with that fallback so unknown names are aggregated under OTHER;
update references in EmotionModel and tests if any to expect OTHER for unknown
display names.
In
`@feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailUi.kt`:
- Around line 286-290: state.representativeEmotion is being force-unwrapped with
!! when calling CollectedSeeds which can crash if null; change the call site in
BookDetailUi (inside the if (state.hasEmotionData()) block) to handle null
safely: either skip rendering CollectedSeeds when representativeEmotion == null,
pass a safe default (e.g., an Emotion.empty or neutral value) via
representativeEmotion ?: defaultEmotion, or update CollectedSeeds to accept a
nullable representativeEmotion and handle it internally; ensure you touch the
CollectedSeeds call (seedsStats = state.seedsStats, representativeEmotion =
state.representativeEmotion, isStatsExpanded = state.isStatsExpanded) and
corresponding CollectedSeeds signature/usage so no !! is used.
Comment on lines
277
to
281
| internal fun Category.toEmotionModel(): EmotionModel? { | ||
| val emotion = Emotion.fromDisplayName(name) ?: return null | ||
| return EmotionModel( | ||
| name = emotion, | ||
| code = EmotionCode.fromDisplayName(name) ?: return null, | ||
| count = count, | ||
| ) |
There was a problem hiding this comment.
알 수 없는 감정명이 들어오면 통계가 누락될 수 있습니다.
fromDisplayName 실패 시 null을 반환해 mapNotNull에서 제거되므로 새로운 감정(또는 “기타”)이 통째로 빠질 수 있습니다. 최소한 OTHER로 폴백하는 편이 안전합니다.
✅ 폴백 처리 예시
- return EmotionModel(
- code = EmotionCode.fromDisplayName(name) ?: return null,
- count = count,
- )
+ val code = EmotionCode.fromDisplayName(name) ?: EmotionCode.OTHER
+ return EmotionModel(
+ code = code,
+ count = count,
+ )🤖 Prompt for AI Agents
In
`@core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/mapper/ResponseToModel.kt`
around lines 277 - 281, Category.toEmotionModel() currently returns null when
EmotionCode.fromDisplayName(name) fails, which causes mapNotNull to drop that
category and lose stats; change this to fall back to a default/OTHER EmotionCode
(e.g., EmotionCode.OTHER) instead of returning null and construct the
EmotionModel with that fallback so unknown names are aggregated under OTHER;
update references in EmotionModel and tests if any to expect OTHER for unknown
display names.
Comment on lines
286
to
290
| if (state.hasEmotionData()) { | ||
| CollectedSeeds( | ||
| seedsStats = state.seedsStats, | ||
| representativeEmotion = state.representativeEmotion!!, | ||
| isStatsExpanded = state.isStatsExpanded, |
There was a problem hiding this comment.
대표 감정 null 가능성에 대비한 안전 처리 필요
representativeEmotion이 nullable인데 !!로 강제 언래핑하고 있습니다. 서버가 null을 내려주거나 hasEmotionData()가 대표 감정의 null 여부를 보장하지 않으면 런타임 크래시가 발생할 수 있습니다. 안전하게 가드하거나 CollectedSeeds가 nullable을 수용하도록 조정하는 편이 안전합니다.
🛠️ 안전 가드 예시
- if (state.hasEmotionData()) {
- CollectedSeeds(
- seedsStats = state.seedsStats,
- representativeEmotion = state.representativeEmotion!!,
- isStatsExpanded = state.isStatsExpanded,
- onToggleClick = { ... },
- )
- } else {
- Spacer(modifier = Modifier.height(ReedTheme.spacing.spacing10))
- }
+ if (state.hasEmotionData() && state.representativeEmotion != null) {
+ CollectedSeeds(
+ seedsStats = state.seedsStats,
+ representativeEmotion = state.representativeEmotion,
+ isStatsExpanded = state.isStatsExpanded,
+ onToggleClick = { ... },
+ )
+ } else {
+ Spacer(modifier = Modifier.height(ReedTheme.spacing.spacing10))
+ }📝 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
| if (state.hasEmotionData()) { | |
| CollectedSeeds( | |
| seedsStats = state.seedsStats, | |
| representativeEmotion = state.representativeEmotion!!, | |
| isStatsExpanded = state.isStatsExpanded, | |
| if (state.hasEmotionData() && state.representativeEmotion != null) { | |
| CollectedSeeds( | |
| seedsStats = state.seedsStats, | |
| representativeEmotion = state.representativeEmotion, | |
| isStatsExpanded = state.isStatsExpanded, |
🤖 Prompt for AI Agents
In
`@feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailUi.kt`
around lines 286 - 290, state.representativeEmotion is being force-unwrapped
with !! when calling CollectedSeeds which can crash if null; change the call
site in BookDetailUi (inside the if (state.hasEmotionData()) block) to handle
null safely: either skip rendering CollectedSeeds when representativeEmotion ==
null, pass a safe default (e.g., an Emotion.empty or neutral value) via
representativeEmotion ?: defaultEmotion, or update CollectedSeeds to accept a
nullable representativeEmotion and handle it internally; ensure you touch the
CollectedSeeds call (seedsStats = state.seedsStats, representativeEmotion =
state.representativeEmotion, isStatsExpanded = state.isStatsExpanded) and
corresponding CollectedSeeds signature/usage so no !! is used.
Contributor
Author
There was a problem hiding this comment.
state.hasEmotionData() 감정이 있다는건 대표 감정이 있다는 의미라 !! 붙혀도 괜찮아
There was a problem hiding this comment.
@easyhooon, 알겠습니다! hasEmotionData()가 representativeEmotion의 null 여부를 이미 보장하고 있다면 !! 사용이 안전하네요. 도메인 로직에 대한 설명 감사합니다. 👍
✏️ Learnings added
Learnt from: easyhooon
Repo: YAPP-Github/Reed-Android PR: 253
File: feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/BookDetailUi.kt:286-290
Timestamp: 2026-01-25T07:21:02.190Z
Learning: In BookDetailUi.kt (Reed-Android project), the `hasEmotionData()` function guarantees that `state.representativeEmotion` is not null when it returns true, so force-unwrapping with `!!` is safe within the `if (state.hasEmotionData())` block.
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.
🧠 Learnings used
Learnt from: easyhooon
Repo: YAPP-Github/Reed-Android PR: 174
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt:128-133
Timestamp: 2025-08-28T12:25:54.058Z
Learning: In BookSearchPresenter.kt, when a guest user tries to register a book and is redirected to login, the bottom sheet (isBookRegisterBottomSheetVisible) and selection state (selectedBookIsbn, selectedBookStatus) are intentionally kept open/preserved so that when the user returns from login, they can continue from where they left off without losing context.
Learnt from: easyhooon
Repo: YAPP-Github/Reed-Android PR: 88
File: feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/book/component/RecordItem.kt:29-37
Timestamp: 2025-07-31T23:17:40.054Z
Learning: Reed-Android 프로젝트에서는 API가 준비되지 않은 상황에서 UI를 먼저 구현하고, API 연동 시점에 하드코딩된 데이터를 실제 데이터로 교체하는 개발 방식을 사용한다. RecordItem 컴포넌트의 emotionTags 매개변수도 API 연동 시점에 `text = emotionTags.joinToString(separator = "·") { "#$it" }`로 적용될 예정이다.
seoyoon513
approved these changes
Jan 26, 2026
Contributor
seoyoon513
left a comment
seoyoon513
left a comment
There was a problem hiding this comment.
@easyhooon 씨앗 통계 API 이슈 확인 되면 머지 진행하시죠~!! 고생하셨습니다bb
Contributor
Author
@seoyoon513 씨앗통계 API endpoint v2로 업데이트 되어 기타 감정까지 잘 내려오는 것 확인했습니다~ {"categories":[{"name":"따뜻함","count":0},{"name":"즐거움","count":0},{"name":"슬픔","count":0},{"name":"깨달음","count":0},{"name":"기타","count":2}]} |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
🔗 관련 이슈
📙 작업 설명
@Stable->@Immutable로 어노테이션 적용 변경🧪 테스트 내역 (선택)
📸 스크린샷 또는 시연 영상 (선택)
💬 추가 설명 or 리뷰 포인트 (선택)
@Stable->@Immutable로 변경했습니다. 이전에 개발시엔 이 두 어노테이션의 차이를 잘 몰라서,@Immutable사용이 조심스러워 사용하지 않았으나, 이제는 사용해도 된다고 판단하였습니다.Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.