Conversation
# Conflicts: # data/study/src/main/java/com/umcspot/spot/study/datasource/StudyDataSource.kt # data/study/src/main/java/com/umcspot/spot/study/datasourceimpl/StudyDataSourceImpl.kt # data/study/src/main/java/com/umcspot/spot/study/mapper/StudyMapper.kt # data/study/src/main/java/com/umcspot/spot/study/repositoryimpl/StudyRepositoryImpl.kt # data/study/src/main/java/com/umcspot/spot/study/service/StudyService.kt # domain/study/src/main/java/com/umcspot/spot/study/repository/StudyRepository.kt # feature/main/src/main/java/com/umcspot/spot/main/MainNavHost.kt # feature/main/src/main/java/com/umcspot/spot/main/MainScreen.kt # feature/study/src/main/java/com/umcspot/spot/study/detail/StudyDetailScreen.kt # feature/study/src/main/java/com/umcspot/spot/study/detail/StudyDetailViewModel.kt # feature/study/src/main/java/com/umcspot/spot/study/detail/model/StudyDetailState.kt # feature/study/src/main/java/com/umcspot/spot/study/detail/navigation/StudyDetailNavigation.kt
Closed
3 tasks
Walkthrough스터디 게시판(보드) 기능을 추가하는 변경사항입니다. 게시글 생성, 조회, 목록 조회, 좋아요/핀 토글, 댓글 작성 등의 API 및 UI를 포함하는 전체 스택 구현입니다. 네트워크, 데이터, 도메인, UI 레이어 전반에 걸쳐 확장되었습니다. Changes
Sequence DiagramsequenceDiagram
actor User
participant Screen as StudyBoardPostScreen
participant ViewModel as StudyDetailViewModel
participant Repo as StudyRepository
participant Service as StudyService
participant Server as API Server
User->>Screen: 게시글 작성 (제목, 내용, 비공개 설정)
Screen->>ViewModel: submitPostFromEditor(studyId)
ViewModel->>Repo: postBoard(studyId, BoardCreateModel)
Repo->>Service: postBoard(studyId, BoardCreateRequestDto)
Service->>Server: POST /studies/{studyId}/board/posts
Server-->>Service: BoardCreateResponseDto {postId}
Service-->>Repo: BaseResponse<BoardCreateResponseDto>
Repo-->>ViewModel: Result<Long>
ViewModel->>ViewModel: postState 업데이트<br/>(에디터 초기화)
ViewModel-->>Screen: BoardPostSuccess SideEffect
Screen->>User: 게시글 생성 완료, 뒤로 이동
User->>+Screen as DetailScreen: 게시글 상세 열기
Screen->>ViewModel: fetchStudyPostDetail(studyId, postId)
ViewModel->>Repo: getStudyPostDetail(studyId, postId)
Repo->>Service: getStudyPostDetail(studyId, postId)
Service->>Server: GET /studies/{studyId}/board/posts/{postId}
Server-->>Service: StudyPostDetailResponseDto
Service-->>Repo: BaseResponse<StudyPostDetailResponseDto>
Repo->>Repo: toDomain() 변환
Repo-->>ViewModel: Result<StudyPostDetailResult>
ViewModel->>ViewModel: postDetailState 업데이트
ViewModel-->>Screen: UiState.Success<PostData>
Screen-->>User: 게시글, 댓글 표시
User->>Screen: 좋아요 버튼 클릭
Screen->>ViewModel: toggleStudyPostDetailLike()
ViewModel->>ViewModel: 낙관적 UI 업데이트<br/>(isLiked, likeCount)
ViewModel->>Repo: studyPostLike(studyId, postId)
Repo->>Service: studyPostLike(studyId, postId)
Service->>Server: POST /studies/{studyId}/board/posts/{postId}/like
Server-->>Service: NullResultResponse {success}
Service-->>Repo: 응답
Repo-->>ViewModel: Result<Unit>
ViewModel->>ViewModel: 실패 시 롤백<br/>또는 상태 확정
User->>Screen: 댓글 입력 & 전송
Screen->>ViewModel: sendStudyPostComment(studyId, postId, content)
ViewModel->>Repo: createStudyPostComment(studyId, postId, content)
Repo->>Service: createStudyPostComment(studyId, postId, StudyPostCommentRequestDto)
Service->>Server: POST /studies/{studyId}/board/posts/{postId}/comments
Server-->>Service: SendStudyPostCommentResponseDto {commentId}
Service-->>Repo: BaseResponse<SendStudyPostCommentResponseDto>
Repo->>Repo: Comment를 CommentResult로 변환
Repo-->>ViewModel: Result<Unit>
ViewModel->>ViewModel: commentData 리스트 업데이트
ViewModel-->>Screen: postDetailState 갱신
Screen-->>User: 댓글 목록에 표시
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly Related PRs
Suggested Labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/study/src/main/java/com/umcspot/spot/study/detail/StudyDetailViewModel.kt (1)
467-477:⚠️ Potential issue | 🔴 Critical메모아 반응 토글 진입점이 여기서 끝납니다.
여기서는 현재 선택 상태만 계산하고 종료합니다. 그래서 3-인자
toggleMemoirReaction()경로로 들어오면 UI 갱신도, 아래 4-인자 오버로드 호출도 일어나지 않아 반응 버튼이 사실상 동작하지 않습니다.💡 제안 수정
fun toggleMemoirReaction(studyId: Long, memoirId: Long, reactionType: String) { val memoir = _uiState.value.memoirState.memoirs.find { it.memoirId == memoirId } ?: return val isCurrentlySelected = when (reactionType) { "FIRE" -> memoir.reactions.isFired "HEART" -> memoir.reactions.isHearted "STAR" -> memoir.reactions.isStarred "SMILE" -> memoir.reactions.isSmiled else -> return } + val nextSelected = !isCurrentlySelected + updateMemoirReactionUIState(memoirId, reactionType, nextSelected) + toggleMemoirReaction(studyId, memoirId, reactionType, isCurrentlySelected) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/study/src/main/java/com/umcspot/spot/study/detail/StudyDetailViewModel.kt` around lines 467 - 477, The toggleMemoirReaction(studyId: Long, memoirId: Long, reactionType: String) function computes isCurrentlySelected but then returns early; update it to continue by dispatching the actual toggle flow: compute the new desired state and call the overloaded toggleMemoirReaction(studyId, memoirId, reactionType, isCurrentlySelected) (or the existing method that performs UI update + backend call) so the UI and network actions execute. Locate the function toggleMemoirReaction(studyId, memoirId, reactionType: String) and ensure it forwards to the 4-argument overload (or invokes the same logic used in that overload) instead of exiting after computing isCurrentlySelected.
🧹 Nitpick comments (3)
feature/study/src/main/java/com/umcspot/spot/study/detail/model/StudyPostContentState.kt (1)
16-19: 댓글 데이터도UiState로 감싸는 구조가 더 안전합니다.지금 구조에서는
빈 댓글과로딩 실패를 동일하게 표현하게 되어 UI/재시도 처리 분기가 어려워집니다.구조 개선 예시
data class StudyPostContentState( val postData: UiState<PostData> = UiState.Empty, - val commentData : ImmutableList<CommentData> = persistentListOf() + val commentData: UiState<ImmutableList<CommentData>> = UiState.Empty )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/study/src/main/java/com/umcspot/spot/study/detail/model/StudyPostContentState.kt` around lines 16 - 19, Wrap the comment list in UiState to distinguish empty vs error/loading by changing StudyPostContentState's commentData from ImmutableList<CommentData> = persistentListOf() to a UiState<ImmutableList<CommentData>> (e.g., UiState.Empty by default); update any consumers of StudyPostContentState.commentData (views, reducers, mappers) to handle UiState states (Loading/Success/Error) instead of treating an empty list as failure and adjust places that construct comment data to emit UiState.Success(persistentListOf(...)) or UiState.Error(...) as appropriate.feature/study/src/main/java/com/umcspot/spot/study/detail/component/post/SwipeRevealRow.kt (1)
1-91: 전체 주석 처리된 컴포넌트 파일은 머지 전 정리해 주세요.현재 상태는 실행 코드가 전혀 없어 기능 기여 없이 코드베이스 복잡도만 증가합니다. 실제 사용 예정이면 구현을 활성화하고, 미완료라면 파일 제거 후 별도 작업으로 분리하는 편이 좋습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/study/src/main/java/com/umcspot/spot/study/detail/component/post/SwipeRevealRow.kt` around lines 1 - 91, This file contains a fully commented-out composable (SwipeRevealRow and RevealState) which adds no runtime value; either re-enable the component by uncommenting and ensuring all imports and references (RevealState enum, SwipeRevealRow composable, functions like rememberAnchoredDraggableState, LaunchedEffect blocks, snapshotFlow usage, and modifiers offset/anchoredDraggable) compile and are exercised by a calling screen, or delete the file and move the in-progress implementation to a feature branch/issue for later work; pick one approach and apply it consistently so the codebase no longer contains a dead, commented-out component.data/study/src/main/java/com/umcspot/spot/study/dto/response/StudyPostDetailResponseDto.kt (1)
32-39: 상세 DTO의stats타입을 목록 DTO에서 분리하는 편이 안전합니다.지금은
StudyPostDetailResponseDto가StudyPostsResponseDto.kt에 있는 top-levelStats에 직접 묶여 있습니다. 한쪽 응답 스키마가 바뀌면 다른 endpoint 파싱도 같이 깨질 수 있어서, 공용 DTO로 명시 분리하거나 이 파일 전용 타입으로 두는 편이 유지보수에 더 안전합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/study/src/main/java/com/umcspot/spot/study/dto/response/StudyPostDetailResponseDto.kt` around lines 32 - 39, StudyPostDetailResponseDto currently reuses the top-level Stats type defined for StudyPostsResponseDto which couples two response schemas; to fix, introduce a dedicated Stats type for this file (e.g., DetailStats) or move a shared Stats into a common DTO file and update StudyPostDetailResponseDto to reference the new type; update the import/usages where StudyPostDetailResponseDto.stats is declared and ensure serialization annotations (`@SerialName`("stats")) remain correct for the new type name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/post/PostItem.kt`:
- Around line 104-106: PostItem의 ClickSurface에서 고정된 Modifier.fillMaxWidth()로 호출해
호출자가 전달한 modifier(padding, semantics, testTag 등)를 덮어쓰고 있습니다; ClickSurface 호출부를
수정해 전달받은 modifier를 보존하도록 변경하세요 (예: replace Modifier.fillMaxWidth() with
modifier.then(Modifier.fillMaxWidth()) or modifier.fillMaxWidth()) so the
passed-in modifier is applied before adding fillMaxWidth; locate the
ClickSurface call in PostItem.kt (the line with ClickSurface(onClick = {
onClick(item) }, modifier = ...)) and use the passed modifier variable instead
of the static Modifier.
In `@core/designsystem/src/main/res/drawable/ic_pin.xml`:
- Around line 6-8: The path in ic_pin.xml currently hardcodes
android:fillColor="#C5C5CD"; replace that hardcoded hex with a theme-aware color
reference (either a color resource or a theme attribute) so the icon respects
dark mode and branding. Concretely, update the path's android:fillColor to
reference a color resource (e.g. `@color/design_system_icon`) or a theme attribute
(e.g. ?attr/colorOnBackground or ?attr/colorControlNormal) and ensure the
referenced color is defined in your colors.xml or themes.xml for light/dark
variants; modify the <path> element with the new reference instead of the
literal hex.
In
`@data/study/src/main/java/com/umcspot/spot/study/dto/response/BoardCreateResponseDto.kt`:
- Around line 7-9: The BoardCreateResponseDto currently declares postId as
String which risks runtime parsing errors later; change the postId property in
BoardCreateResponseDto from String to Long (keep `@SerialName`("postId")) so the
DTO carries a numeric ID type end-to-end and update any callers/serializers that
construct or consume BoardCreateResponseDto to pass/expect a Long instead of
relying on toLong parsing.
In
`@data/study/src/main/java/com/umcspot/spot/study/dto/response/SendStudyPostCommentResponseDto.kt`:
- Around line 7-10: The DTO SendStudyPostCommentResponseDto currently declares
commentId as String which can cause type mismatch later; change the property
type to a numeric ID (preferably Long) and update its serialization mapping if
needed so commentId: Long replaces commentId: String in the data class, ensuring
downstream code expecting numeric IDs won't need conversions.
In
`@data/study/src/main/java/com/umcspot/spot/study/repositoryimpl/StudyRepositoryImpl.kt`:
- Around line 480-482: The code maps response.result.toDomainList() without
checking response.isSuccess, so failures get turned into mapping/NPEs instead of
surfacing server error messages; update the blocks that call
studyDataSource.getStudyPostsList (and the similar call at the other location)
to first check response.isSuccess and on true return
response.result.toDomainList(), otherwise propagate a meaningful exception or
return a failed Result containing response.message (or response.error) so the
original server failure is preserved for logging/handling.
In
`@feature/study/src/main/java/com/umcspot/spot/study/detail/screen/StudyBoardPostScreen.kt`:
- Around line 54-61: Replace the current LaunchedEffect(Unit) block so
resetPostEditor() runs only once on first composition and side-effect collection
runs in a separate effect: use a rememberSaveable mutableStateOf flag (e.g.,
initialized) to guard a one-time call to viewModel.resetPostEditor(), set the
flag true after calling it, then move the viewModel.sideEffect.collect { ... }
into its own LaunchedEffect that only handles side effects (collects and calls
onBackClick() when effect is StudyDetailSideEffect.BoardPostSuccess). Add
imports for androidx.compose.runtime.rememberSaveable and
androidx.compose.runtime.mutableStateOf as suggested.
In
`@feature/study/src/main/java/com/umcspot/spot/study/detail/screen/StudyPostContentScreen.kt`:
- Around line 251-255: The click handler currently hides showReportRequestDialog
and immediately sets showAcceptRequestDialog = true before the report result;
change this so the UI only shows the success dialog after observing the report
success result from onReportPost (e.g., via the ViewModel's LiveData/StateFlow
or the callback returned by onReportPost) — remove the direct assignment of
showAcceptRequestDialog = true from the onClick lambda (the block around onClick
that references showReportRequestDialog, showAcceptRequestDialog, and
onReportPost) and instead set showAcceptRequestDialog = true only when the
report success state is emitted/confirmed; apply the same change to the similar
block at lines 258-266.
- Around line 236-240: The current onClick handler calls onDeletePost(...) and
then immediately onDeleteClick(), which can close the screen even if deletion
fails; change the flow so onDeleteClick() is invoked only after a confirmed
successful delete. Update onDeletePost (or add a new variant) to return a
success boolean or accept a completion callback (e.g., onDeletePost(studyId,
post.postId, onResult: (success)->Unit)) and in the onClick set
showDeleteRequestDialog = false only when the result indicates success, calling
onDeleteClick() from that success branch; keep showDeleteRequestDialog behavior
unchanged for failure so the user remains on screen.
- Around line 302-309: The Image used as the clickable menu icon in
StudyPostContentScreen (the Image with painterResource(R.drawable.meetball) and
modifier.clickable { menuExpanded = true }) lacks an accessibility label; update
its contentDescription from null to a descriptive string (preferably a string
resource like "Open menu" or "메뉴 열기") so screen readers announce its purpose
when the Image with .clickable is focused; ensure you update the Image call in
StudyPostContentScreen.kt and use stringResource(...) for localization where
applicable.
In
`@feature/study/src/main/java/com/umcspot/spot/study/detail/StudyDetailScreen.kt`:
- Around line 105-124: DisposableEffect is currently capturing lifecycleOwner
and uiState.plannerState.selectedDate directly which can cause stale reads and
leaked observers; change the DisposableEffect keys to include lifecycleOwner
(and studyId and selectedTab) e.g. DisposableEffect(lifecycleOwner, studyId,
selectedTab) and wrap uiState.plannerState.selectedDate with
rememberUpdatedState to get a fresh value inside the LifecycleEventObserver
(e.g. val currentSelectedDate =
rememberUpdatedState(uiState.plannerState.selectedDate)) and use
currentSelectedDate.value when calling viewModel.fetchMonthlySchedules, leaving
the rest of the LifecycleEventObserver and add/remove observer logic intact.
In
`@feature/study/src/main/java/com/umcspot/spot/study/detail/StudyDetailViewModel.kt`:
- Around line 683-708: togglePostLike sends duplicate requests when tapped
quickly because it lacks the in-flight guard used by
toggleStudyPostDetailLike/inFlightDetailLikes; add a similar inFlightPosts
(e.g., a MutableSet<Long>) and check at the start of togglePostLike to
early-return if postId is present, add postId to the set before calling
studyRepository.studyPostLike or studyPostUnLike, and remove it in a finally
block so concurrent taps are ignored while a request is in-flight; ensure you
only update _uiState inside result.onSuccess (as now) and that the in-flight set
is referenced (and cleared) even on failure to prevent permanent blocking.
- Around line 526-556: fetchStudyBoardPosts can send duplicate requests for the
same cursor when multiple scroll events occur; add an in-flight guard keyed by
the cursor (or a single boolean for list-loading) to prevent launching another
request for the same nextCursor, and also perform cursor-based deduplication
when merging results. Concretely: in StudyDetailViewModel introduce a mutableSet
or map like inFlightCursors (or a single isPostListLoading flag), check/insert
the cursor (use null as initial) before calling
studyRepository.getStudyPostsList, return early if already in-flight, and remove
the cursor from the set in both onSuccess and onFailure; additionally when
updating state in the onSuccess block (fetchStudyBoardPosts -> _uiState.update),
merge posts by skipping items whose unique id already exists in
state.postState.studyPosts (or ignore the response if nextCursor equals the
current state.nextCursor) to ensure duplicate pages are not appended.
- Around line 710-727: The fetchStudyPostDetail flow updates
currentDetailStudyId/currentDetailPostId but doesn’t verify them when the async
callback returns; guard the success and failure handlers inside
fetchStudyPostDetail by comparing the returned context to the
currentDetailStudyId and currentDetailPostId before calling _uiState.update or
emitError. Specifically, inside the onSuccess and onFailure blocks for
studyRepository.getStudyPostDetail in fetchStudyPostDetail, check that the
stored currentDetailStudyId and currentDetailPostId still match the studyId and
postId parameters (or the identifiers from the returned detail) and only then
update _uiState or call emitError to avoid stale responses overwriting the UI.
---
Outside diff comments:
In
`@feature/study/src/main/java/com/umcspot/spot/study/detail/StudyDetailViewModel.kt`:
- Around line 467-477: The toggleMemoirReaction(studyId: Long, memoirId: Long,
reactionType: String) function computes isCurrentlySelected but then returns
early; update it to continue by dispatching the actual toggle flow: compute the
new desired state and call the overloaded toggleMemoirReaction(studyId,
memoirId, reactionType, isCurrentlySelected) (or the existing method that
performs UI update + backend call) so the UI and network actions execute. Locate
the function toggleMemoirReaction(studyId, memoirId, reactionType: String) and
ensure it forwards to the 4-argument overload (or invokes the same logic used in
that overload) instead of exiting after computing isCurrentlySelected.
---
Nitpick comments:
In
`@data/study/src/main/java/com/umcspot/spot/study/dto/response/StudyPostDetailResponseDto.kt`:
- Around line 32-39: StudyPostDetailResponseDto currently reuses the top-level
Stats type defined for StudyPostsResponseDto which couples two response schemas;
to fix, introduce a dedicated Stats type for this file (e.g., DetailStats) or
move a shared Stats into a common DTO file and update StudyPostDetailResponseDto
to reference the new type; update the import/usages where
StudyPostDetailResponseDto.stats is declared and ensure serialization
annotations (`@SerialName`("stats")) remain correct for the new type name.
In
`@feature/study/src/main/java/com/umcspot/spot/study/detail/component/post/SwipeRevealRow.kt`:
- Around line 1-91: This file contains a fully commented-out composable
(SwipeRevealRow and RevealState) which adds no runtime value; either re-enable
the component by uncommenting and ensuring all imports and references
(RevealState enum, SwipeRevealRow composable, functions like
rememberAnchoredDraggableState, LaunchedEffect blocks, snapshotFlow usage, and
modifiers offset/anchoredDraggable) compile and are exercised by a calling
screen, or delete the file and move the in-progress implementation to a feature
branch/issue for later work; pick one approach and apply it consistently so the
codebase no longer contains a dead, commented-out component.
In
`@feature/study/src/main/java/com/umcspot/spot/study/detail/model/StudyPostContentState.kt`:
- Around line 16-19: Wrap the comment list in UiState to distinguish empty vs
error/loading by changing StudyPostContentState's commentData from
ImmutableList<CommentData> = persistentListOf() to a
UiState<ImmutableList<CommentData>> (e.g., UiState.Empty by default); update any
consumers of StudyPostContentState.commentData (views, reducers, mappers) to
handle UiState states (Loading/Success/Error) instead of treating an empty list
as failure and adjust places that construct comment data to emit
UiState.Success(persistentListOf(...)) or UiState.Error(...) as appropriate.
🪄 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: CHILL
Plan: Pro
Run ID: 5189d7ce-5711-4ee7-aedf-cf10e212c506
📒 Files selected for processing (34)
core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/post/CountsView.ktcore/designsystem/src/main/java/com/umcspot/spot/designsystem/component/post/PostItem.ktcore/designsystem/src/main/res/drawable/ic_pin.xmlcore/designsystem/src/main/res/drawable/ic_unpin.xmlcore/network/src/main/java/com/umcspot/spot/network/model/BaseResponse.ktdata/study/src/main/java/com/umcspot/spot/study/datasource/StudyDataSource.ktdata/study/src/main/java/com/umcspot/spot/study/datasourceimpl/StudyDataSourceImpl.ktdata/study/src/main/java/com/umcspot/spot/study/dto/request/BoardCreateRequestDto.ktdata/study/src/main/java/com/umcspot/spot/study/dto/request/StudyPostCommentRequestDto.ktdata/study/src/main/java/com/umcspot/spot/study/dto/response/BoardCreateResponseDto.ktdata/study/src/main/java/com/umcspot/spot/study/dto/response/SendStudyPostCommentResponseDto.ktdata/study/src/main/java/com/umcspot/spot/study/dto/response/StudyPostDetailResponseDto.ktdata/study/src/main/java/com/umcspot/spot/study/dto/response/StudyPostsResponseDto.ktdata/study/src/main/java/com/umcspot/spot/study/mapper/StudyMapper.ktdata/study/src/main/java/com/umcspot/spot/study/repositoryimpl/StudyRepositoryImpl.ktdata/study/src/main/java/com/umcspot/spot/study/service/StudyService.ktdomain/study/src/main/java/com/umcspot/spot/study/model/BoardCreateModel.ktdomain/study/src/main/java/com/umcspot/spot/study/model/StudyPostsDetailResult.ktdomain/study/src/main/java/com/umcspot/spot/study/model/StudyPostsResultList.ktdomain/study/src/main/java/com/umcspot/spot/study/repository/StudyRepository.ktfeature/home/src/main/java/com/umcspot/spot/home/HomeScreen.ktfeature/home/src/main/java/com/umcspot/spot/home/navigation/HomeNavigation.ktfeature/main/src/main/java/com/umcspot/spot/main/MainNavHost.ktfeature/main/src/main/java/com/umcspot/spot/main/MainNavigator.ktfeature/main/src/main/java/com/umcspot/spot/main/MainScreen.ktfeature/study/src/main/java/com/umcspot/spot/study/detail/StudyDetailScreen.ktfeature/study/src/main/java/com/umcspot/spot/study/detail/StudyDetailViewModel.ktfeature/study/src/main/java/com/umcspot/spot/study/detail/component/post/SwipeRevealRow.ktfeature/study/src/main/java/com/umcspot/spot/study/detail/model/StudyDetailState.ktfeature/study/src/main/java/com/umcspot/spot/study/detail/model/StudyPostContentState.ktfeature/study/src/main/java/com/umcspot/spot/study/detail/navigation/StudyDetailNavigation.ktfeature/study/src/main/java/com/umcspot/spot/study/detail/screen/StudyBoardPostScreen.ktfeature/study/src/main/java/com/umcspot/spot/study/detail/screen/StudyDetailBoardScreen.ktfeature/study/src/main/java/com/umcspot/spot/study/detail/screen/StudyPostContentScreen.kt
👮 Files not reviewed due to content moderation or server errors (8)
- core/designsystem/src/main/res/drawable/ic_unpin.xml
- data/study/src/main/java/com/umcspot/spot/study/dto/request/StudyPostCommentRequestDto.kt
- feature/home/src/main/java/com/umcspot/spot/home/navigation/HomeNavigation.kt
- feature/main/src/main/java/com/umcspot/spot/main/MainNavigator.kt
- core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/post/CountsView.kt
- feature/main/src/main/java/com/umcspot/spot/main/MainNavHost.kt
- feature/main/src/main/java/com/umcspot/spot/main/MainScreen.kt
- feature/study/src/main/java/com/umcspot/spot/study/detail/screen/StudyDetailBoardScreen.kt
Comment on lines
+104
to
+106
| ClickSurface( | ||
| onClick = { onClick(item) }, | ||
| modifier = Modifier.fillMaxWidth() |
There was a problem hiding this comment.
새 오버로드에서 전달받은 modifier가 무시됩니다.
Line 106에서 Modifier.fillMaxWidth()를 고정해서 호출부가 넘긴 padding, semantics, testTag 같은 수정이 모두 사라집니다. 기존 API 계약을 유지하려면 전달받은 modifier를 이어서 써야 합니다.
🔧 Proposed fix
ClickSurface(
onClick = { onClick(item) },
- modifier = Modifier.fillMaxWidth()
+ modifier = modifier.fillMaxWidth()
) {📝 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
| ClickSurface( | |
| onClick = { onClick(item) }, | |
| modifier = Modifier.fillMaxWidth() | |
| ClickSurface( | |
| onClick = { onClick(item) }, | |
| modifier = modifier.fillMaxWidth() | |
| ) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/post/PostItem.kt`
around lines 104 - 106, PostItem의 ClickSurface에서 고정된 Modifier.fillMaxWidth()로
호출해 호출자가 전달한 modifier(padding, semantics, testTag 등)를 덮어쓰고 있습니다; ClickSurface
호출부를 수정해 전달받은 modifier를 보존하도록 변경하세요 (예: replace Modifier.fillMaxWidth() with
modifier.then(Modifier.fillMaxWidth()) or modifier.fillMaxWidth()) so the
passed-in modifier is applied before adding fillMaxWidth; locate the
ClickSurface call in PostItem.kt (the line with ClickSurface(onClick = {
onClick(item) }, modifier = ...)) and use the passed modifier variable instead
of the static Modifier.
Comment on lines
+6
to
+8
| <path | ||
| android:pathData="M7.227,0.41C7.071,0.253 6.879,0.136 6.668,0.068C6.457,-0 6.232,-0.017 6.013,0.018C5.794,0.054 5.587,0.141 5.408,0.272C5.23,0.404 5.085,0.576 4.985,0.774L3.774,3.195C3.659,3.425 3.464,3.605 3.226,3.7L0.713,4.705C0.605,4.748 0.51,4.817 0.436,4.906C0.361,4.995 0.31,5.101 0.287,5.215C0.264,5.328 0.269,5.446 0.302,5.557C0.336,5.668 0.396,5.768 0.478,5.85L2.305,7.678L0,9.982V10.478H0.495L2.8,8.172L4.627,9.999C4.709,10.081 4.81,10.142 4.921,10.175C5.032,10.208 5.149,10.214 5.263,10.19C5.376,10.167 5.482,10.116 5.571,10.042C5.66,9.968 5.729,9.872 5.772,9.765L6.777,7.252C6.873,7.013 7.053,6.818 7.283,6.703L9.703,5.492C9.902,5.393 10.074,5.248 10.205,5.069C10.337,4.89 10.424,4.683 10.459,4.464C10.495,4.245 10.478,4.021 10.41,3.81C10.342,3.599 10.224,3.407 10.067,3.25L7.227,0.41Z" | ||
| android:fillColor="#C5C5CD"/> |
There was a problem hiding this comment.
아이콘 색상 하드코딩은 테마 일관성을 깨뜨릴 수 있습니다.
fillColor를 고정 hex 대신 디자인시스템 색상 리소스(또는 theme attr)로 연결해 두는 편이 다크모드/브랜딩 변경에 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/designsystem/src/main/res/drawable/ic_pin.xml` around lines 6 - 8, The
path in ic_pin.xml currently hardcodes android:fillColor="#C5C5CD"; replace that
hardcoded hex with a theme-aware color reference (either a color resource or a
theme attribute) so the icon respects dark mode and branding. Concretely, update
the path's android:fillColor to reference a color resource (e.g.
`@color/design_system_icon`) or a theme attribute (e.g. ?attr/colorOnBackground or
?attr/colorControlNormal) and ensure the referenced color is defined in your
colors.xml or themes.xml for light/dark variants; modify the <path> element with
the new reference instead of the literal hex.
Comment on lines
+7
to
+9
| data class BoardCreateResponseDto( | ||
| @SerialName("postId") val postId: String | ||
| ) No newline at end of file |
There was a problem hiding this comment.
postId를 String으로 두면 ID 파싱 실패 경로가 생깁니다.
생성 결과 ID는 이후 화면 라우팅/조회에서 숫자 ID로 쓰일 가능성이 높아, 문자열 파싱(toLong)에 의존하면 런타임 예외 위험이 생깁니다. DTO에서 타입을 일관되게 맞추는 편이 안전합니다.
수정 제안
data class BoardCreateResponseDto(
- `@SerialName`("postId") val postId: String
+ `@SerialName`("postId") val postId: Long
)📝 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
| data class BoardCreateResponseDto( | |
| @SerialName("postId") val postId: String | |
| ) | |
| data class BoardCreateResponseDto( | |
| `@SerialName`("postId") val postId: Long | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@data/study/src/main/java/com/umcspot/spot/study/dto/response/BoardCreateResponseDto.kt`
around lines 7 - 9, The BoardCreateResponseDto currently declares postId as
String which risks runtime parsing errors later; change the postId property in
BoardCreateResponseDto from String to Long (keep `@SerialName`("postId")) so the
DTO carries a numeric ID type end-to-end and update any callers/serializers that
construct or consume BoardCreateResponseDto to pass/expect a Long instead of
relying on toLong parsing.
Comment on lines
+7
to
+10
| data class SendStudyPostCommentResponseDto( | ||
| @SerialName("commentId") | ||
| val commentId: String | ||
| ) |
There was a problem hiding this comment.
commentId도 숫자 ID 체계와 타입을 맞추는 게 안전합니다.
댓글 ID가 문자열이면 후속 로직에서 숫자 변환 실패가 발생할 수 있습니다. DTO 단계에서 타입 정합성을 맞춰 두는 것을 권장합니다.
수정 제안
data class SendStudyPostCommentResponseDto(
`@SerialName`("commentId")
- val commentId: String
+ val commentId: Long
)📝 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
| data class SendStudyPostCommentResponseDto( | |
| @SerialName("commentId") | |
| val commentId: String | |
| ) | |
| data class SendStudyPostCommentResponseDto( | |
| `@SerialName`("commentId") | |
| val commentId: Long | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@data/study/src/main/java/com/umcspot/spot/study/dto/response/SendStudyPostCommentResponseDto.kt`
around lines 7 - 10, The DTO SendStudyPostCommentResponseDto currently declares
commentId as String which can cause type mismatch later; change the property
type to a numeric ID (preferably Long) and update its serialization mapping if
needed so commentId: Long replaces commentId: String in the data class, ensuring
downstream code expecting numeric IDs won't need conversions.
Comment on lines
+480
to
+482
| val response = studyDataSource.getStudyPostsList(studyId, cursor, size) | ||
| response.result.toDomainList() | ||
| }.onFailure { |
There was a problem hiding this comment.
응답 성공 여부 검증이 누락되어 실패 원인이 왜곡됩니다.
두 메서드 모두 response.isSuccess 확인 없이 response.result를 바로 매핑합니다. 실패 응답일 때 서버 메시지 대신 매핑/NPE 예외로 바뀌어 디버깅과 에러 처리 품질이 떨어집니다.
수정 제안
override suspend fun getStudyPostsList(
studyId : Long,
cursor: Long?,
size: Int
) : Result<StudyPostsResultList> =
runCatching {
val response = studyDataSource.getStudyPostsList(studyId, cursor, size)
+ if (!response.isSuccess) {
+ throw Exception(response.message.ifBlank { "게시글 목록 조회에 실패했습니다." })
+ }
response.result.toDomainList()
}.onFailure {
Log.e("StudyRepository", "getStudyPostsList failed", it)
}
@@
override suspend fun getStudyPostDetail(
studyId: Long,
postId: Long
): Result<StudyPostDetailResult> =
runCatching {
val response = studyDataSource.getStudyPostDetail(studyId, postId)
+ if (!response.isSuccess) {
+ throw Exception(response.message.ifBlank { "게시글 상세 조회에 실패했습니다." })
+ }
response.result.toDomain()
}.onFailure {
Log.e("StudyRepository", "getStudyPostDetail failed", it)
}Also applies to: 491-493
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@data/study/src/main/java/com/umcspot/spot/study/repositoryimpl/StudyRepositoryImpl.kt`
around lines 480 - 482, The code maps response.result.toDomainList() without
checking response.isSuccess, so failures get turned into mapping/NPEs instead of
surfacing server error messages; update the blocks that call
studyDataSource.getStudyPostsList (and the similar call at the other location)
to first check response.isSuccess and on true return
response.result.toDomainList(), otherwise propagate a meaningful exception or
return a failed Result containing response.message (or response.error) so the
original server failure is preserved for logging/handling.
Comment on lines
+302
to
+309
| Image( | ||
| painter = painterResource(R.drawable.meetball), | ||
| contentDescription = null, | ||
| modifier = Modifier | ||
| .align(Alignment.Center) | ||
| .size(screenWidthDp(14.dp)) | ||
| .clickable { menuExpanded = true } | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the file in the repository
find . -name "StudyPostContentScreen.kt" -type fRepository: SPOTeam/Android-V2
Length of output: 154
🏁 Script executed:
# Read the file around lines 302-309
sed -n '295,315p' ./feature/study/src/main/java/com/umcspot/spot/study/detail/screen/StudyPostContentScreen.ktRepository: SPOTeam/Android-V2
Length of output: 893
🏁 Script executed:
# Check for similar patterns in the codebase to see if there are other instances
rg "contentDescription\s*=\s*null" --type kotlin | head -20Repository: SPOTeam/Android-V2
Length of output: 2890
🏁 Script executed:
# Check the context around the Image to confirm it's interactive
sed -n '300,320p' ./feature/study/src/main/java/com/umcspot/spot/study/detail/screen/StudyPostContentScreen.ktRepository: SPOTeam/Android-V2
Length of output: 834
클릭 가능한 메뉴 아이콘에 접근성 라벨이 없습니다.
contentDescription = null인 상태에서 .clickable 수정자가 적용된 Image는 스크린리더에서 목적을 알 수 없습니다. 메뉴 버튼의 역할을 설명하는 contentDescription을 추가해 주세요.
수정 제안
Image(
painter = painterResource(R.drawable.meetball),
- contentDescription = null,
+ contentDescription = "게시글 더보기 메뉴",
modifier = Modifier
.align(Alignment.Center)
.size(screenWidthDp(14.dp))
.clickable { menuExpanded = 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
| Image( | |
| painter = painterResource(R.drawable.meetball), | |
| contentDescription = null, | |
| modifier = Modifier | |
| .align(Alignment.Center) | |
| .size(screenWidthDp(14.dp)) | |
| .clickable { menuExpanded = true } | |
| ) | |
| Image( | |
| painter = painterResource(R.drawable.meetball), | |
| contentDescription = "게시글 더보기 메뉴", | |
| modifier = Modifier | |
| .align(Alignment.Center) | |
| .size(screenWidthDp(14.dp)) | |
| .clickable { menuExpanded = true } | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/study/src/main/java/com/umcspot/spot/study/detail/screen/StudyPostContentScreen.kt`
around lines 302 - 309, The Image used as the clickable menu icon in
StudyPostContentScreen (the Image with painterResource(R.drawable.meetball) and
modifier.clickable { menuExpanded = true }) lacks an accessibility label; update
its contentDescription from null to a descriptive string (preferably a string
resource like "Open menu" or "메뉴 열기") so screen readers announce its purpose
when the Image with .clickable is focused; ensure you update the Image call in
StudyPostContentScreen.kt and use stringResource(...) for localization where
applicable.
Comment on lines
+105
to
+124
| val lifecycleOwner = LocalLifecycleOwner.current | ||
| DisposableEffect(studyId, selectedTab) { | ||
| val observer = LifecycleEventObserver { _, event -> | ||
| if (event != Lifecycle.Event.ON_RESUME) return@LifecycleEventObserver | ||
|
|
||
| when (selectedTab) { | ||
| StudyDetailTab.HOME -> viewModel.fetchStudyHomeDetail(studyId) | ||
| StudyDetailTab.PLANNER -> { | ||
| val date = uiState.plannerState.selectedDate | ||
| viewModel.fetchMonthlySchedules(studyId, date.year, date.monthValue) | ||
| } | ||
| StudyDetailTab.BOARD -> viewModel.fetchStudyBoardPosts(studyId, refresh = true) | ||
| StudyDetailTab.MEMOIR -> viewModel.fetchAllMemoirs(studyId) | ||
| } | ||
| } | ||
|
|
||
| val lifecycle = lifecycleOwner.lifecycle | ||
| lifecycle.addObserver(observer) | ||
| onDispose { lifecycle.removeObserver(observer) } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f StudyDetailScreen.ktRepository: SPOTeam/Android-V2
Length of output: 140
🏁 Script executed:
cat -n feature/study/src/main/java/com/umcspot/spot/study/detail/StudyDetailScreen.kt | sed -n '100,130p'Repository: SPOTeam/Android-V2
Length of output: 1492
🌐 Web query:
Compose DisposableEffect LifecycleEventObserver stale state best practices
💡 Result:
Best practices for using DisposableEffect with LifecycleEventObserver in Jetpack Compose, particularly avoiding stale state: 1. Use DisposableEffect to register LifecycleEventObserver and ensure cleanup in onDispose. Official example from Android docs for analytics on lifecycle events: @Composable fun HomeScreen( lifecycleOwner: LifecycleOwner = LocalLifecycleOwner.current, onStart: -> Unit, onStop: -> Unit ) { val currentOnStart by rememberUpdatedState(onStart) val currentOnStop by rememberUpdatedState(onStop) DisposableEffect(lifecycleOwner) { val observer = LifecycleEventObserver { _, event -> when { event == Lifecycle.Event.ON_START -> currentOnStart event == Lifecycle.Event.ON_STOP -> currentOnStop } } lifecycleOwner.lifecycle.addObserver(observer) onDispose { lifecycleOwner.lifecycle.removeObserver(observer) } } // Home screen content } Key practices to avoid stale state (stale closures): - Wrap callbacks in rememberUpdatedState(newValue): Returns a State that always reads the latest value during execution, even if the effect was created with an older snapshot. Essential when lambdas might change between recompositions but effect key (like lifecycleOwner) doesn't. - Pass dependencies as keys (e.g., DisposableEffect(lifecycleOwner)): Restarts effect if lifecycleOwner changes, disposing old observer first. - Always include meaningful onDispose cleanup; empty onDispose is poor practice. Prefer higher-level APIs when possible (Lifecycle 2.7.0+): - LifecycleResumeEffect(key1) { /* on RESUME / onPauseOrDispose { / cleanup / } } - LifecycleStartEffect(key1) { / on START / onStopOrDispose { / cleanup / } } - LifecycleEventEffect(Lifecycle.Event.ON_START) { / one-shot, no paired cleanup */ } These automatically handle observer registration/removal, reducing boilerplate and error risk vs. manual DisposableEffect. Avoid: - Capturing changing state/lambdas directly in observer: Causes stale closures as effect persists across recompositions. - Unstable keys causing excessive restarts. - State updates inside DisposableEffect (use LaunchedEffect for async/state changes). - Long-running sync work (non-suspendable only). For async lifecycle-aware work, combine with repeatOnLifecycle in LaunchedEffect. This ensures resource safety, correct lifecycle sync, and fresh state access.
Citations:
- 1: https://developer.android.com/develop/ui/compose/side-effects
- 2: https://developer.android.com/topic/libraries/architecture/compose
- 3: https://developer.android.com/reference/kotlin/androidx/compose/runtime/rememberUpdatedState.composable
- 4: https://developer.android.com/reference/kotlin/androidx/lifecycle/compose/LifecycleResumeEffect.composable
ON_RESUME 옵저버가 오래된 상태를 캡처합니다.
이 DisposableEffect는 selectedTab만 key로 두고 lifecycleOwner와 uiState.plannerState.selectedDate를 직접 캡처합니다. 플래너 날짜를 바꾼 뒤 resume하면 이전 날짜로 조회하거나, lifecycleOwner가 변경되었을 때 예전 lifecycle에 observer가 남을 수 있습니다.
DisposableEffect(lifecycleOwner, studyId, selectedTab)로 키를 추가하고 selectedDate는 rememberUpdatedState로 감싸서 항상 최신 값을 읽도록 수정하세요.
🔧 제안된 수정
val lifecycleOwner = LocalLifecycleOwner.current
+ val currentSelectedDate by rememberUpdatedState(uiState.plannerState.selectedDate)
- DisposableEffect(studyId, selectedTab) {
+ DisposableEffect(lifecycleOwner, studyId, selectedTab) {
val observer = LifecycleEventObserver { _, event ->
if (event != Lifecycle.Event.ON_RESUME) return@LifecycleEventObserver
when (selectedTab) {
StudyDetailTab.HOME -> viewModel.fetchStudyHomeDetail(studyId)
StudyDetailTab.PLANNER -> {
- val date = uiState.plannerState.selectedDate
+ val date = currentSelectedDate
viewModel.fetchMonthlySchedules(studyId, date.year, date.monthValue)
}
StudyDetailTab.BOARD -> viewModel.fetchStudyBoardPosts(studyId, refresh = true)
StudyDetailTab.MEMOIR -> viewModel.fetchAllMemoirs(studyId)
}
}
val lifecycle = lifecycleOwner.lifecycle
lifecycle.addObserver(observer)
onDispose { lifecycle.removeObserver(observer) }
}📝 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
| val lifecycleOwner = LocalLifecycleOwner.current | |
| DisposableEffect(studyId, selectedTab) { | |
| val observer = LifecycleEventObserver { _, event -> | |
| if (event != Lifecycle.Event.ON_RESUME) return@LifecycleEventObserver | |
| when (selectedTab) { | |
| StudyDetailTab.HOME -> viewModel.fetchStudyHomeDetail(studyId) | |
| StudyDetailTab.PLANNER -> { | |
| val date = uiState.plannerState.selectedDate | |
| viewModel.fetchMonthlySchedules(studyId, date.year, date.monthValue) | |
| } | |
| StudyDetailTab.BOARD -> viewModel.fetchStudyBoardPosts(studyId, refresh = true) | |
| StudyDetailTab.MEMOIR -> viewModel.fetchAllMemoirs(studyId) | |
| } | |
| } | |
| val lifecycle = lifecycleOwner.lifecycle | |
| lifecycle.addObserver(observer) | |
| onDispose { lifecycle.removeObserver(observer) } | |
| } | |
| val lifecycleOwner = LocalLifecycleOwner.current | |
| val currentSelectedDate by rememberUpdatedState(uiState.plannerState.selectedDate) | |
| DisposableEffect(lifecycleOwner, studyId, selectedTab) { | |
| val observer = LifecycleEventObserver { _, event -> | |
| if (event != Lifecycle.Event.ON_RESUME) return@LifecycleEventObserver | |
| when (selectedTab) { | |
| StudyDetailTab.HOME -> viewModel.fetchStudyHomeDetail(studyId) | |
| StudyDetailTab.PLANNER -> { | |
| val date = currentSelectedDate | |
| viewModel.fetchMonthlySchedules(studyId, date.year, date.monthValue) | |
| } | |
| StudyDetailTab.BOARD -> viewModel.fetchStudyBoardPosts(studyId, refresh = true) | |
| StudyDetailTab.MEMOIR -> viewModel.fetchAllMemoirs(studyId) | |
| } | |
| } | |
| val lifecycle = lifecycleOwner.lifecycle | |
| lifecycle.addObserver(observer) | |
| onDispose { lifecycle.removeObserver(observer) } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/study/src/main/java/com/umcspot/spot/study/detail/StudyDetailScreen.kt`
around lines 105 - 124, DisposableEffect is currently capturing lifecycleOwner
and uiState.plannerState.selectedDate directly which can cause stale reads and
leaked observers; change the DisposableEffect keys to include lifecycleOwner
(and studyId and selectedTab) e.g. DisposableEffect(lifecycleOwner, studyId,
selectedTab) and wrap uiState.plannerState.selectedDate with
rememberUpdatedState to get a fresh value inside the LifecycleEventObserver
(e.g. val currentSelectedDate =
rememberUpdatedState(uiState.plannerState.selectedDate)) and use
currentSelectedDate.value when calling viewModel.fetchMonthlySchedules, leaving
the rest of the LifecycleEventObserver and add/remove observer logic intact.
Comment on lines
+526
to
+556
| fun fetchStudyBoardPosts(studyId: Long, refresh: Boolean = false) { | ||
| viewModelScope.launch { | ||
| val currentState = _uiState.value.postState | ||
| val cursor = if (refresh) null else currentState.nextCursor | ||
|
|
||
| if (!refresh && !currentState.hasNext && currentState.studyPosts.isNotEmpty()) return@launch | ||
|
|
||
| _uiState.update { it.copy(isLoading = true) } | ||
|
|
||
| studyRepository.getStudyPostsList( | ||
| studyId = studyId, | ||
| cursor = cursor, | ||
| size = 20 | ||
| ).onSuccess { posts -> | ||
| _uiState.update { state -> | ||
| val baseList = if (refresh || cursor == null) emptyList() else state.postState.studyPosts | ||
| state.copy( | ||
| postState = state.postState.copy( | ||
| studyPosts = (baseList + posts.studyPostsList).toPersistentList(), | ||
| hasNext = posts.hasNext, | ||
| nextCursor = posts.nextCursor | ||
| ), | ||
| isLoading = false | ||
| ) | ||
| } | ||
| }.onFailure { | ||
| _uiState.update { state -> state.copy(isLoading = false) } | ||
| emitError(it) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
페이지 요청 중복을 막지 않아 같은 게시글 페이지가 두 번 붙을 수 있습니다.
무한 스크롤 이벤트가 연속으로 들어오면 같은 cursor로 요청이 둘 이상 날아가고, 응답이 모두 성공했을 때 동일 페이지가 studyPosts에 중복 append될 수 있습니다. 목록 로딩 전용 in-flight 가드나 cursor 기준 중복 방지가 필요합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/study/src/main/java/com/umcspot/spot/study/detail/StudyDetailViewModel.kt`
around lines 526 - 556, fetchStudyBoardPosts can send duplicate requests for the
same cursor when multiple scroll events occur; add an in-flight guard keyed by
the cursor (or a single boolean for list-loading) to prevent launching another
request for the same nextCursor, and also perform cursor-based deduplication
when merging results. Concretely: in StudyDetailViewModel introduce a mutableSet
or map like inFlightCursors (or a single isPostListLoading flag), check/insert
the cursor (use null as initial) before calling
studyRepository.getStudyPostsList, return early if already in-flight, and remove
the cursor from the set in both onSuccess and onFailure; additionally when
updating state in the onSuccess block (fetchStudyBoardPosts -> _uiState.update),
merge posts by skipping items whose unique id already exists in
state.postState.studyPosts (or ignore the response if nextCursor equals the
current state.nextCursor) to ensure duplicate pages are not appended.
Comment on lines
+683
to
+708
| fun togglePostLike(studyId: Long, postId: Long, isCurrentlyLiked: Boolean) { | ||
| viewModelScope.launch { | ||
| val result = if (isCurrentlyLiked) { | ||
| studyRepository.studyPostUnLike(studyId, postId) | ||
| } else { | ||
| studyRepository.studyPostLike(studyId, postId) | ||
| } | ||
|
|
||
| result.onSuccess { | ||
| _uiState.update { state -> | ||
| val updatedPosts = state.postState.studyPosts | ||
| .map { post -> | ||
| if (post.postId == postId) { | ||
| post.copy( | ||
| isLiked = !isCurrentlyLiked, | ||
| likeCount = if (!isCurrentlyLiked) post.likeCount + 1 else post.likeCount - 1 | ||
| ) | ||
| } else post | ||
| } | ||
| .toPersistentList() | ||
|
|
||
| state.copy(postState = state.postState.copy(studyPosts = updatedPosts)) | ||
| } | ||
| }.onFailure { emitError(it) } | ||
| } | ||
| } |
There was a problem hiding this comment.
목록 좋아요는 연속 탭 시 카운트가 쉽게 틀어집니다.
상세용 toggleStudyPostDetailLike()은 inFlightDetailLikes로 중복 요청을 막는데, 목록용은 같은 보호가 없습니다. 빠르게 두 번 탭하면 같은 isCurrentlyLiked 값으로 요청이 중복 전송되고 likeCount가 2번 증감할 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/study/src/main/java/com/umcspot/spot/study/detail/StudyDetailViewModel.kt`
around lines 683 - 708, togglePostLike sends duplicate requests when tapped
quickly because it lacks the in-flight guard used by
toggleStudyPostDetailLike/inFlightDetailLikes; add a similar inFlightPosts
(e.g., a MutableSet<Long>) and check at the start of togglePostLike to
early-return if postId is present, add postId to the set before calling
studyRepository.studyPostLike or studyPostUnLike, and remove it in a finally
block so concurrent taps are ignored while a request is in-flight; ensure you
only update _uiState inside result.onSuccess (as now) and that the in-flight set
is referenced (and cleared) even on failure to prevent permanent blocking.
Comment on lines
+710
to
+727
| fun fetchStudyPostDetail(studyId: Long, postId: Long) { | ||
| currentDetailStudyId = studyId | ||
| currentDetailPostId = postId | ||
| _uiState.update { it.copy(postDetailState = UiState.Loading) } | ||
|
|
||
| viewModelScope.launch { | ||
| studyRepository.getStudyPostDetail(studyId, postId) | ||
| .onSuccess { detail -> | ||
| _uiState.update { it.copy(postDetailState = UiState.Success(detail)) } | ||
| } | ||
| .onFailure { t -> | ||
| _uiState.update { | ||
| it.copy(postDetailState = UiState.Failure(t.message ?: "게시글을 불러오지 못했습니다.")) | ||
| } | ||
| emitError(t) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
이전 상세 응답이 늦게 도착하면 현재 화면을 덮어쓸 수 있습니다.
currentDetailStudyId/currentDetailPostId를 저장하지만 성공/실패 콜백에서 응답 대상 검증을 하지 않습니다. A 게시글에서 B 게시글로 바로 이동했을 때 A 응답이 늦게 오면 postDetailState가 B 화면 위에 잘못 반영됩니다.
💡 제안 수정
viewModelScope.launch {
studyRepository.getStudyPostDetail(studyId, postId)
.onSuccess { detail ->
+ if (currentDetailStudyId != studyId || currentDetailPostId != postId) return@onSuccess
_uiState.update { it.copy(postDetailState = UiState.Success(detail)) }
}
.onFailure { t ->
+ if (currentDetailStudyId != studyId || currentDetailPostId != postId) return@onFailure
_uiState.update {
it.copy(postDetailState = UiState.Failure(t.message ?: "게시글을 불러오지 못했습니다."))
}
emitError(t)
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/study/src/main/java/com/umcspot/spot/study/detail/StudyDetailViewModel.kt`
around lines 710 - 727, The fetchStudyPostDetail flow updates
currentDetailStudyId/currentDetailPostId but doesn’t verify them when the async
callback returns; guard the success and failure handlers inside
fetchStudyPostDetail by comparing the returned context to the
currentDetailStudyId and currentDetailPostId before calling _uiState.update or
emitError. Specifically, inside the onSuccess and onFailure blocks for
studyRepository.getStudyPostDetail in fetchStudyPostDetail, check that the
stored currentDetailStudyId and currentDetailPostId still match the studyId and
postId parameters (or the identifiers from the returned detail) and only then
update _uiState or call emitError to avoid stale responses overwriting the UI.
This was referenced Apr 13, 2026
Merged
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.
Related issue 🛠
Work Description 📝
Screenshot 📸
Uncompleted Tasks 😅
PR Point 📌
트러블 슈팅 💥
Summary by CodeRabbit
새로운 기능