[feat] #210 포즈 상세 좌우 스와이프 지원#215
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
Changes포즈 상세 좌우 스와이프 및 페이징 지원
사진 상세 즐겨찾기 동기화 다중 항목 처리 확장
Sequence Diagram(s)sequenceDiagram
participant 사용자
participant PoseScreen
participant PoseViewModel
participant PoseDetailScreen
participant PoseDetailViewModel
participant PoseRepository
사용자->>PoseScreen: 포즈 아이템 클릭 (pose, index)
PoseScreen->>PoseViewModel: ClickPoseItem(pose, index)
PoseViewModel->>PoseScreen: NavigateToPoseDetail(PoseNavKey.PoseDetail)
Note over PoseScreen: poses = 페이징 스냅샷, hasNext = append 상태
PoseScreen->>PoseDetailScreen: navigate(PoseNavKey.PoseDetail)
PoseDetailScreen->>PoseDetailViewModel: create(key) → 초기 poses/index/북마크 맵 구성
PoseDetailViewModel-->>PoseDetailScreen: uiState(poses, currentPage)
사용자->>PoseDetailScreen: 좌우 스와이프
PoseDetailScreen->>PoseDetailViewModel: PageChanged(page)
PoseDetailViewModel->>PoseDetailViewModel: preloadIfNeeded(currentIndex)
PoseDetailViewModel->>PoseRepository: getPosesPage(nextPage) 또는 getBookmarkedPosesPage(nextPage)
PoseRepository-->>PoseDetailViewModel: Result~PosePage~(poses, hasNext)
PoseDetailViewModel-->>PoseDetailScreen: uiState 업데이트(poses 누적)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@feature/pose/api/src/main/java/com/neki/android/feature/pose/api/PoseNavKey.kt`:
- Around line 19-27: The PoseDetail data class in PoseNavKey contains a full
poses list that gets serialized during navigation, causing
TransactionTooLargeException risks when handling large datasets. Remove the
poses property from the PoseDetail class and instead implement a shared cache
mechanism (such as a repository or in-memory cache) to store pose snapshots
keyed by poseId. Update PoseDetailViewModel initialization to retrieve poses
from the cache rather than from the navigation arguments, keeping only the
essential context parameters in PoseDetail (poseId, initialIndex, hasNext,
headCount, sortOrder, isBookmarkOnly) for the navigation payload.
In
`@feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/detail/PoseDetailViewModel.kt`:
- Around line 34-35: The nextPage calculation in PoseDetailViewModel using floor
division (key.poses.size / PAGE_SIZE) can cause re-requesting already-loaded
pages if the initial snapshot size is not a multiple of PAGE_SIZE, and the
append logic around line 181 lacks duplicate removal, causing duplicates to
accumulate in the list. Fix the nextPage calculation to use ceiling division to
correctly calculate the starting page for pagination, and add duplicate removal
logic when appending new poses to ensure only new items are added to the list.
In
`@feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/main/component/PoseListContent.kt`:
- Around line 102-107: The aspectRatio calculation in the PoseListContent
component only validates that width and height are not null, but does not check
if they are valid positive values before performing the division. Add an
additional validation to ensure that both pose.width and pose.height are greater
than 0 in the hasSize condition, so that when creating the mutableFloatStateOf
for aspectRatio, you only perform the division if height is a valid positive
number. Otherwise, default to 0f. Apply this same fix to both occurrences of
aspectRatio calculation (the first one around lines 102-107 and the second one
around lines 125-129).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a07240f4-7492-48bc-aa50-eb074f9895f9
📒 Files selected for processing (21)
core/data-api/src/main/java/com/neki/android/core/dataapi/repository/PoseRepository.ktcore/data/src/main/java/com/neki/android/core/data/paging/BookmarkPosePagingSource.ktcore/data/src/main/java/com/neki/android/core/data/paging/PosePagingSource.ktcore/data/src/main/java/com/neki/android/core/data/remote/model/response/PoseResponse.ktcore/data/src/main/java/com/neki/android/core/data/repository/impl/PoseRepositoryImpl.ktcore/model/src/main/java/com/neki/android/core/model/Pose.ktcore/model/src/main/java/com/neki/android/core/model/PosePage.ktfeature/pose/api/src/main/java/com/neki/android/feature/pose/api/PoseNavKey.ktfeature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/detail/PoseDetailContract.ktfeature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/detail/PoseDetailScreen.ktfeature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/detail/PoseDetailViewModel.ktfeature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/detail/component/PoseDetailImageItem.ktfeature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/main/PoseContract.ktfeature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/main/PoseScreen.ktfeature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/main/PoseViewModel.ktfeature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/main/component/PoseListContent.ktfeature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/navigation/PoseEntryProvider.ktfeature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/random/RandomPoseContract.ktfeature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/random/RandomPoseScreen.ktfeature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/random/RandomPoseViewModel.ktfeature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/random/component/RandomPoseFloatingBar.kt
💤 Files with no reviewable changes (4)
- feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/random/RandomPoseViewModel.kt
- feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/random/RandomPoseContract.kt
- feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/random/component/RandomPoseFloatingBar.kt
- feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/random/RandomPoseScreen.kt
| data class PoseDetail( | ||
| val poseId: Long, | ||
| val poses: List<Pose> = emptyList(), | ||
| val initialIndex: Int = 0, | ||
| val hasNext: Boolean = false, | ||
| val headCount: PeopleCount? = null, | ||
| val sortOrder: SortOrder = SortOrder.DESC, | ||
| val isBookmarkOnly: Boolean = false, | ||
| ) : PoseNavKey |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# PoseDetail 키 생성 지점 확인
rg -n --type=kotlin -C3 'PoseNavKey\.PoseDetail\('
# 목록 스냅샷/리스트가 payload로 주입되는 경로 확인
rg -n --type=kotlin -C5 'poses\s*=|itemSnapshotList|snapshot' \
feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/main/PoseScreen.kt \
feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/main/PoseViewModel.kt
# 전체 키 객체를 navigate로 전달하는 경로 확인
rg -n --type=kotlin -C3 \
'fun MainNavigator\.navigateToPoseDetail\(poseDetail: PoseNavKey\.PoseDetail\)|navigate\(poseDetail\)'Repository: Team-Neki/Team-Neki-Android
Length of output: 6460
🏁 Script executed:
# Get full PoseDetail class definition
cat -n feature/pose/api/src/main/java/com/neki/android/feature/pose/api/PoseNavKey.kt | head -50Repository: Team-Neki/Team-Neki-Android
Length of output: 1633
🏁 Script executed:
# Check how poses field is used in the destination screen (PoseDetailScreen)
rg -n --type=kotlin 'poseDetail\.poses|navKey\.poses' \
feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/detail/Repository: Team-Neki/Team-Neki-Android
Length of output: 53
🏁 Script executed:
# Look for any size checks or serialization guards
rg -n --type=kotlin 'TransactionTooLarge|Parcelable|Bundle' feature/pose/Repository: Team-Neki/Team-Neki-Android
Length of output: 53
🏁 Script executed:
# Verify if poses is actually needed during navigation or if it's just passed through
rg -n --type=kotlin 'poses\.size|poses\.isEmpty|poses\[' \
feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/detail/ \
feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/main/Repository: Team-Neki/Team-Neki-Android
Length of output: 1323
🏁 Script executed:
# Check PoseDetailViewModel initialization to see where it gets the poses from the nav key
rg -n --type=kotlin -B5 -A10 'class PoseDetailViewModel|constructor.*key.*PoseNavKey' \
feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/detail/PoseDetailViewModel.ktRepository: Team-Neki/Team-Neki-Android
Length of output: 833
🏁 Script executed:
# Look for the initialState or reduce logic to see if poses from key are used in state
rg -n --type=kotlin -B3 -A10 'reduce|initialState|copy.*poses' \
feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/detail/PoseDetailViewModel.kt | head -80Repository: Team-Neki/Team-Neki-Android
Length of output: 3599
🏁 Script executed:
# Check if there's any data loading from DB/API in PoseDetailViewModel
rg -n --type=kotlin 'getPoseDetail|fetchPose|observePose' \
feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/detail/Repository: Team-Neki/Team-Neki-Android
Length of output: 403
PoseNavKey에 전체 poses 리스트를 싣는 방식은 네비게이션 실패를 유발할 수 있습니다.
Line 21의 poses: List<Pose>를 직렬화하여 Line 74에서 posePagingItems.itemSnapshotList.items로 채운 후 Line 43에서 전달하면, 로드된 아이템이 많은 상태에서 payload가 커져 TransactionTooLargeException 또는 상태 복원 실패 리스크가 큽니다.
PoseDetailViewModel에서 초기화 시(Line 45) 이 poses 리스트를 직접 사용하며, Line 47의 bookmark 매핑과 Line 34의 페이지네이션 계산도 이에 의존하므로, 구조적 개선이 필요합니다. 네비게이션 키에는 최소 컨텍스트(poseId, initialIndex, 필터/정렬, hasNext)만 두고, 목록 스냅샷은 공유 메모리 캐시(키 기반)로 분리하는 것이 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@feature/pose/api/src/main/java/com/neki/android/feature/pose/api/PoseNavKey.kt`
around lines 19 - 27, The PoseDetail data class in PoseNavKey contains a full
poses list that gets serialized during navigation, causing
TransactionTooLargeException risks when handling large datasets. Remove the
poses property from the PoseDetail class and instead implement a shared cache
mechanism (such as a repository or in-memory cache) to store pose snapshots
keyed by poseId. Update PoseDetailViewModel initialization to retrieve poses
from the cache rather than from the navigation arguments, keeping only the
essential context parameters in PoseDetail (poseId, initialIndex, hasNext,
headCount, sortOrder, isBookmarkOnly) for the navigation payload.
| private var nextPage: Int = key.poses.size / PAGE_SIZE | ||
| private var isLoadingMore: Boolean = false |
There was a problem hiding this comment.
다음 페이지 계산/append 로직 때문에 중복 포즈가 누적될 수 있습니다.
Line 34의 key.poses.size / PAGE_SIZE는 내림 계산이라 초기 스냅샷이 페이지 크기의 배수가 아니면 이미 포함된 페이지를 재요청할 수 있고, Line 181에서 중복 제거 없이 append되어 목록이 중복됩니다.
수정 예시
- private var nextPage: Int = key.poses.size / PAGE_SIZE
+ private var nextPage: Int = if (key.poses.isEmpty()) 0 else (key.poses.size + PAGE_SIZE - 1) / PAGE_SIZE
...
- .onSuccess { page ->
- reduce {
- copy(
- poses = poses + page.poses,
- committedBookmarks = committedBookmarks + page.poses.associate { it.id to it.isBookmarked },
- )
- }
+ .onSuccess { page ->
+ reduce {
+ val existingIds = poses.asSequence().map { it.id }.toSet()
+ val appended = page.poses.filterNot { it.id in existingIds }
+ copy(
+ poses = poses + appended,
+ committedBookmarks = committedBookmarks + appended.associate { it.id to it.isBookmarked },
+ )
+ }
hasNext = page.hasNext
nextPage++
}Also applies to: 179-187
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/detail/PoseDetailViewModel.kt`
around lines 34 - 35, The nextPage calculation in PoseDetailViewModel using
floor division (key.poses.size / PAGE_SIZE) can cause re-requesting
already-loaded pages if the initial snapshot size is not a multiple of
PAGE_SIZE, and the append logic around line 181 lacks duplicate removal, causing
duplicates to accumulate in the list. Fix the nextPage calculation to use
ceiling division to correctly calculate the starting page for pagination, and
add duplicate removal logic when appending new poses to ensure only new items
are added to the list.
| val hasSize = pose.width != null && pose.height != null | ||
| var aspectRatio by if (hasSize) { | ||
| remember { mutableFloatStateOf(pose.width!! / pose.height!!.toFloat()) } | ||
| } else { | ||
| rememberSaveable { mutableFloatStateOf(0f) } | ||
| } |
There was a problem hiding this comment.
서버 width/height가 0 이하일 때 aspectRatio 계산이 깨질 수 있습니다.
Line 104는 null만 확인하고 나눗셈을 수행합니다. height <= 0 데이터가 오면 비정상 ratio가 만들어져 Modifier.aspectRatio(...)에서 런타임 오류를 유발할 수 있습니다.
수정 예시
- val hasSize = pose.width != null && pose.height != null
- var aspectRatio by if (hasSize) {
- remember { mutableFloatStateOf(pose.width!! / pose.height!!.toFloat()) }
+ val hasValidSize = (pose.width ?: 0) > 0 && (pose.height ?: 0) > 0
+ var aspectRatio by if (hasValidSize) {
+ remember(pose.width, pose.height) { mutableFloatStateOf(pose.width!!.toFloat() / pose.height!!.toFloat()) }
} else {
rememberSaveable { mutableFloatStateOf(0f) }
}
...
- if (!hasSize) {
+ if (!hasValidSize) {
val size = state.painter.intrinsicSize
if (size.width > 0f && size.height > 0f) {
aspectRatio = size.width / size.height
}
}Also applies to: 125-129
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/main/component/PoseListContent.kt`
around lines 102 - 107, The aspectRatio calculation in the PoseListContent
component only validates that width and height are not null, but does not check
if they are valid positive values before performing the division. Add an
additional validation to ensure that both pose.width and pose.height are greater
than 0 in the hasSize condition, so that when creating the mutableFloatStateOf
for aspectRatio, you only perform the division if height is a valid positive
number. Otherwise, default to 0f. Apply this same fix to both occurrences of
aspectRatio calculation (the first one around lines 102-107 and the second one
around lines 125-129).
Ojongseok
left a comment
There was a problem hiding this comment.
고생하셨습니다! 전체적으로 좋아보입니닷
한 가지 제안하고 싶은 부분은 포즈 - 포즈 상세의 스와이프 로직을 아카이빙 - 사진 상세의 스와이프 로직과 동일하게 가져가는게 어떨까요?
예를 들어, 좋아요/북마크 상태가 PhotoDetailViewModel에서는 committedFavorites: MutableMap ViewModel에서 관리되고 있고, PoseDetailViewModel에서는 committedBookmarks: Map의 PoseDetailState에서 관리되고 있습니다!
각 뷰모델의 onCleared() 함수 내에서 현재 사진의 commited 상태만 보는지, 전체 리스트의 상태를 순회하는지의 차이도 있구요!
🔗 관련 이슈
📙 작업 설명
🧪 테스트 내역 (선택)
검증 명령:
📸 스크린샷 또는 시연 영상 (선택)
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선사항
UI 변경