[Feature] 온보딩 api, 회원탈퇴 연결 #208
Hidden character warning
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthrough온보딩 검색 기능에 다중 장르 선택과 커서 기반 페이지네이션을 추가하고, 계정 탈퇴 API를 POST 방식으로 개선합니다. 무한 스크롤로 더 많은 콘텐츠를 로드할 수 있습니다. Changes온보딩 검색 및 탈퇴 기능
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/src/main/java/com/flint/presentation/onboarding/OnboardingContentScreen.kt (1)
109-113: ⚡ Quick win무한 스크롤 트리거 조건 개선 필요
현재
LaunchedEffect의 key가gridState.canScrollForward만 사용되어,isLoadingMore가false로 변경되어도 effect가 재실행되지 않습니다. 사용자가 이미 하단에 도달한 상태에서 추가 데이터 로드가 완료되면, 다음 페이지 로드를 위해 스크롤을 위로 올렸다가 다시 내려야 합니다.
isLoadingMore와nextCursor변화에도 반응하도록 key를 확장하는 것을 권장합니다.♻️ 권장 수정안
- LaunchedEffect(gridState.canScrollForward) { + LaunchedEffect( + gridState.canScrollForward, + contentUiState.isLoadingMore, + contentUiState.nextCursor, + ) { if (!gridState.canScrollForward && contentUiState.nextCursor != null && !contentUiState.isLoadingMore) { onLoadMore() } }🤖 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 `@app/src/main/java/com/flint/presentation/onboarding/OnboardingContentScreen.kt` around lines 109 - 113, The LaunchedEffect currently only keys off gridState.canScrollForward so it won't re-run when contentUiState.isLoadingMore or contentUiState.nextCursor change; update the LaunchedEffect key to include gridState.canScrollForward, contentUiState.isLoadingMore, and contentUiState.nextCursor (use their primitive/value properties) and keep the same body that calls onLoadMore() when !gridState.canScrollForward && contentUiState.nextCursor != null && !contentUiState.isLoadingMore so the effect retriggers when loading finishes or nextCursor changes.app/src/main/java/com/flint/presentation/onboarding/OnboardingViewModel.kt (1)
196-228: 💤 Low value페이지네이션 구현 확인 - 중복 콘텐츠 방지
병합 로직에서 중복 콘텐츠를 필터링하지 않고 있습니다. 커서 기반 페이지네이션에서 일반적으로 중복이 발생하지 않지만, 백엔드 데이터 변경이나 네트워크 재시도 시 동일 콘텐츠가 포함될 수 있습니다. 이 경우
LazyVerticalGrid에서 동일한key가 발생해 경고가 출력됩니다.현재 구현이 문제를 일으킬 가능성은 낮으나, 방어적으로 중복 제거를 추가하는 것을 고려해 주세요.
♻️ 선택적 수정안
.onSuccess { result -> - val merged = (currentItems + result.contents).toImmutableList() + val existingIds = currentItems.map { it.id }.toSet() + val newItems = result.contents.filterNot { it.id in existingIds } + val merged = (currentItems + newItems).toImmutableList() _contentUiState.update { it.copy(🤖 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 `@app/src/main/java/com/flint/presentation/onboarding/OnboardingViewModel.kt` around lines 196 - 228, In loadMoreContents, the merge of currentItems + result.contents can introduce duplicate items causing duplicate keys in LazyVerticalGrid; update the merge before emitting to _contentUiState so duplicates are removed (e.g., use distinctBy on the content identifier field present in the items) — locate the merge in loadMoreContents where merged is created and replace it with a de-duplicated list (retain original ordering or prefer new items as needed), then emit UiState.Success with that deduplicated collection and existing nextCursor/isLoadingMore updates; ensure you reference the same item id property used across your UI keying logic and keep using searchRepository.getSearchContentList and OnboardingContentUiState.GENRES as-is.
🤖 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 `@app/src/main/java/com/flint/domain/repository/AuthRepository.kt`:
- Around line 42-45: The withdraw function currently uses a hardcoded default
agreedTermsIds = listOf("10") which can send incorrect consent data; remove the
default so suspend fun withdraw(agreedTermsIds: List<String>): Result<Unit>
requires callers to pass the selected IDs, and construct the WithdrawRequestDto
with that parameter as before (WithdrawRequestDto(agreedTermsIds =
agreedTermsIds)) and update all call sites that relied on the default to supply
the actual selected list; keep the suspendRunCatching and api.withdraw(…) usage
intact.
---
Nitpick comments:
In
`@app/src/main/java/com/flint/presentation/onboarding/OnboardingContentScreen.kt`:
- Around line 109-113: The LaunchedEffect currently only keys off
gridState.canScrollForward so it won't re-run when contentUiState.isLoadingMore
or contentUiState.nextCursor change; update the LaunchedEffect key to include
gridState.canScrollForward, contentUiState.isLoadingMore, and
contentUiState.nextCursor (use their primitive/value properties) and keep the
same body that calls onLoadMore() when !gridState.canScrollForward &&
contentUiState.nextCursor != null && !contentUiState.isLoadingMore so the effect
retriggers when loading finishes or nextCursor changes.
In `@app/src/main/java/com/flint/presentation/onboarding/OnboardingViewModel.kt`:
- Around line 196-228: In loadMoreContents, the merge of currentItems +
result.contents can introduce duplicate items causing duplicate keys in
LazyVerticalGrid; update the merge before emitting to _contentUiState so
duplicates are removed (e.g., use distinctBy on the content identifier field
present in the items) — locate the merge in loadMoreContents where merged is
created and replace it with a de-duplicated list (retain original ordering or
prefer new items as needed), then emit UiState.Success with that deduplicated
collection and existing nextCursor/isLoadingMore updates; ensure you reference
the same item id property used across your UI keying logic and keep using
searchRepository.getSearchContentList and OnboardingContentUiState.GENRES as-is.
🪄 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: e33d7206-2b78-4e9e-a483-389cafccd3bb
📒 Files selected for processing (10)
app/src/main/java/com/flint/data/api/AuthApi.ktapp/src/main/java/com/flint/data/api/SearchApi.ktapp/src/main/java/com/flint/data/dto/auth/request/WithdrawRequestDto.ktapp/src/main/java/com/flint/domain/mapper/search/SearchContentMapper.ktapp/src/main/java/com/flint/domain/model/search/SearchContentItemModel.ktapp/src/main/java/com/flint/domain/repository/AuthRepository.ktapp/src/main/java/com/flint/domain/repository/SearchRepository.ktapp/src/main/java/com/flint/presentation/onboarding/OnboardingContentScreen.ktapp/src/main/java/com/flint/presentation/onboarding/OnboardingUiState.ktapp/src/main/java/com/flint/presentation/onboarding/OnboardingViewModel.kt
| // :TODO 일단 10으로 고정해둠 수정예정 | ||
| suspend fun withdraw(agreedTermsIds: List<String> = listOf("10")): Result<Unit> = | ||
| suspendRunCatching { | ||
| api.withdraw() | ||
| api.withdraw(WithdrawRequestDto(agreedTermsIds = agreedTermsIds)) |
There was a problem hiding this comment.
탈퇴 약관 ID 기본값을 저장소 API에 고정하지 마세요.
Line 43의 listOf("10") 때문에 호출부가 값을 빠뜨리면 항상 특정 약관 ID로 탈퇴 요청이 전송됩니다. 약관 ID가 서버 설정과 달라지는 순간 탈퇴가 실패하거나 잘못된 동의 이력이 남을 수 있어서, 여기서는 기본값을 두지 말고 현재 화면에서 선택된 값을 반드시 넘기도록 강제하는 편이 안전합니다.
변경 예시
- // :TODO 일단 10으로 고정해둠 수정예정
- suspend fun withdraw(agreedTermsIds: List<String> = listOf("10")): Result<Unit> =
+ suspend fun withdraw(agreedTermsIds: List<String>): Result<Unit> =
suspendRunCatching {
api.withdraw(WithdrawRequestDto(agreedTermsIds = agreedTermsIds))
preferencesManager.clearAll()
}🤖 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 `@app/src/main/java/com/flint/domain/repository/AuthRepository.kt` around lines
42 - 45, The withdraw function currently uses a hardcoded default agreedTermsIds
= listOf("10") which can send incorrect consent data; remove the default so
suspend fun withdraw(agreedTermsIds: List<String>): Result<Unit> requires
callers to pass the selected IDs, and construct the WithdrawRequestDto with that
parameter as before (WithdrawRequestDto(agreedTermsIds = agreedTermsIds)) and
update all call sites that relied on the default to supply the actual selected
list; keep the suspendRunCatching and api.withdraw(…) usage intact.
There was a problem hiding this comment.
위에 TODO로 남겨둠 수정예정입니다
There was a problem hiding this comment.
@ckals413, TODO 주석으로 남겨두신 것 확인했습니다. 나중에 수정하실 때 잊지 않도록 GitHub 이슈로 트래킹해 드릴까요? 원하시면 말씀해 주세요.
🧠 Learnings used
Learnt from: nahy-512
Repo: imflint/Flint-Android PR: 135
File: app/src/main/java/com/flint/domain/repository/AuthRepository.kt:1-22
Timestamp: 2026-01-19T00:48:44.889Z
Learning: Architect the Android app following Google's recommended Android App Architecture for this repo: place the data layer innermost, the domain layer can depend on the data layer, and avoid circular dependencies with other layers. For Kotlin files under app/src/main/java, ensure accompanying architecture aligns with this guidance (e.g., domain interfaces depending on data layer implementations or abstractions, not the other way around).
Learnt from: giovannijunseokim
Repo: imflint/Flint-Android PR: 153
File: app/src/main/java/com/flint/presentation/collectiondetail/CollectionDetailViewModel.kt:98-105
Timestamp: 2026-01-21T08:37:42.767Z
Learning: Guideline: For APIs like toggleContentBookmark that do not return a bookmarkCount, perform optimistic updates by updating the isBookmarked state on the client and compute bookmarkCount locally if needed. Do not rely on the server for the count; ensure the server response only conveys the boolean bookmarked state and synchronize this state accordingly. This applies to Kotlin Android ViewModels and related UI state management across files that handle similar bookmark toggle endpoints.
0556fa2 to
7bf14e9
Compare
📮 관련 이슈
FLT_20_온보딩 콘텐츠 검색 api연결
📌 작업 내용
📸 스크린샷
😅 미구현
🫛 To. 리뷰어
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선사항