찌르기 쿨타임 처리 수정#171
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough포크(찌르기) 쿨다운을 목표(goalId) 단독 기준에서 목표와 날짜(targetDate) 조합 기준으로 변경했습니다. Room 데이터베이스 버전을 2로 올리고 Estimated code review effort: 4 (Complex) | ~50 minutes 변경 사항
Sequence Diagram(s)sequenceDiagram
participant PhotologDetailViewModel
participant PokeGoalUseCase
participant PokeRepository
participant PokeHistoryDao
PhotologDetailViewModel->>PokeGoalUseCase: invoke(goalId, targetDate)
PokeGoalUseCase->>PokeRepository: findPokeHistory(goalId, targetDate)
PokeRepository->>PokeHistoryDao: findPokeHistoryEntity(goalId, targetDate)
PokeHistoryDao-->>PokeRepository: PokeHistoryEntity? / pokedAt
PokeRepository-->>PokeGoalUseCase: pokedAt or null
alt 쿨다운 중
PokeGoalUseCase-->>PhotologDetailViewModel: OnCooldown(remainingMs)
PhotologDetailViewModel->>PhotologDetailViewModel: startPokeCooldown(remainingMs)
else 쿨다운 없음
PokeGoalUseCase->>PokeRepository: savePokeHistory(goalId, targetDate, now)
PokeGoalUseCase-->>PhotologDetailViewModel: Success
PhotologDetailViewModel->>PhotologDetailViewModel: startPokeCooldown(COOLDOWN_MS)
end
주요 이슈: 명시적으로 연결된 이슈 없음 시: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
domain/src/main/java/com/twix/domain/usecase/PokeGoalUseCase.kt (1)
31-35:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win시스템 시간 변경 시 쿨타임이 설정값(3시간)을 초과할 수 있습니다.
현재는
pokedAt가currentTime보다 미래면elapsedMs가 음수가 되어remainingMs가COOLDOWN_MS보다 커질 수 있습니다. 사용자 입장에서는 쿨타임이 비정상적으로 길어집니다.remainingMs를0..COOLDOWN_MS범위로 고정해 주세요.수정 예시
- val elapsedMs = currentTime - pokedAt - val remainingMs = COOLDOWN_MS - elapsedMs - - return remainingMs.coerceAtLeast(0L) + val elapsedMs = (currentTime - pokedAt).coerceAtLeast(0L) + val remainingMs = COOLDOWN_MS - elapsedMs + + return remainingMs.coerceIn(0L, COOLDOWN_MS)🤖 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 `@domain/src/main/java/com/twix/domain/usecase/PokeGoalUseCase.kt` around lines 31 - 35, The current remainingMs calculation can exceed COOLDOWN_MS if pokedAt is in the future; modify the return in PokeGoalUseCase (where currentTime, elapsedMs and remainingMs are computed) to clamp remainingMs into the 0..COOLDOWN_MS range—e.g. replace remainingMs.coerceAtLeast(0L) with remainingMs.coerceIn(0L, COOLDOWN_MS) so the returned cooldown is never negative or greater than the configured COOLDOWN_MS.
🧹 Nitpick comments (1)
feature/photolog/detail/src/test/java/com/twix/photolog/detail/PhotologDetailViewModelTest.kt (1)
67-88: ⚡ Quick winSad Path 테스트가 빠져 있어 회귀를 놓칠 수 있습니다.
현재 케이스는 쿨타임 정상 흐름 검증은 좋지만,
pokeGoalResult = AppResult.Error(...)상황에서isPoking복구와 실패 후 상태를 검증하지 않아 회귀 여지가 남습니다.
에러 경로 테스트 1건을 추가해dispatch(PhotologDetailIntent.Poke)이후isPoking == false및 쿨타임 상태/사이드이펙트 기대값을 함께 확인해보실까요?As per coding guidelines, "Happy / Sad Path가 모두 커버되는가?" 항목에 따라 실패 경로 검증을 포함하는 것이 좋습니다.
🤖 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/photolog/detail/src/test/java/com/twix/photolog/detail/PhotologDetailViewModelTest.kt` around lines 67 - 88, Add a sad-path unit test that mirrors the existing cooldown test but configures FakePokeRepository to return AppResult.Error for the poke call, then dispatch PhotologDetailIntent.Poke and advance the test dispatcher (runCurrent/runTest); assert that viewModel.uiState.value.isPoking is false after the failure, verify pokeCooldownRemaining and isPokeDisabled reflect the expected post-error state, and check pokeRepository.pokeGoalCallCount (and any error-handling side-effects) to ensure the failure path is covered; use the same helpers (createViewModel, FakePokeRepository, runTest/StandardTestDispatcher, PhotologDetailIntent.Poke) so the new test parallels the happy-path case.Source: Coding guidelines
🤖 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/photolog/detail/src/main/java/com/twix/photolog/detail/PhotologDetailViewModel.kt`:
- Around line 175-179: The pokeToPartner() coroutine sets isPoking = true then
calls pokeGoalUseCase.invoke(...) but if that call throws the ViewModel can
remain stuck; wrap the invoke + handlePokeResult call in a try/catch inside
pokeToPartner(), on exception call handlePokeError(exception) and ensure
isPoking is cleared (reduce { copy(isPoking = false) }) in both success and
error paths; rethrow CancellationException (if exception is
CancellationException) so cancellations propagate normally.
---
Outside diff comments:
In `@domain/src/main/java/com/twix/domain/usecase/PokeGoalUseCase.kt`:
- Around line 31-35: The current remainingMs calculation can exceed COOLDOWN_MS
if pokedAt is in the future; modify the return in PokeGoalUseCase (where
currentTime, elapsedMs and remainingMs are computed) to clamp remainingMs into
the 0..COOLDOWN_MS range—e.g. replace remainingMs.coerceAtLeast(0L) with
remainingMs.coerceIn(0L, COOLDOWN_MS) so the returned cooldown is never negative
or greater than the configured COOLDOWN_MS.
---
Nitpick comments:
In
`@feature/photolog/detail/src/test/java/com/twix/photolog/detail/PhotologDetailViewModelTest.kt`:
- Around line 67-88: Add a sad-path unit test that mirrors the existing cooldown
test but configures FakePokeRepository to return AppResult.Error for the poke
call, then dispatch PhotologDetailIntent.Poke and advance the test dispatcher
(runCurrent/runTest); assert that viewModel.uiState.value.isPoking is false
after the failure, verify pokeCooldownRemaining and isPokeDisabled reflect the
expected post-error state, and check pokeRepository.pokeGoalCallCount (and any
error-handling side-effects) to ensure the failure path is covered; use the same
helpers (createViewModel, FakePokeRepository, runTest/StandardTestDispatcher,
PhotologDetailIntent.Poke) so the new test parallels the happy-path case.
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 668257ad-95ce-406a-a7cd-339c9c5f44fa
📒 Files selected for processing (15)
.github/PULL_REQUEST_TEMPLATE.mdcore/database/src/main/java/com/twix/database/TwixDatabase.ktcore/database/src/main/java/com/twix/database/di/DatabaseModule.ktcore/database/src/main/java/com/twix/database/poke/PokeHistoryDao.ktcore/database/src/main/java/com/twix/database/poke/PokeHistoryEntity.ktdata/src/main/java/com/twix/data/repository/DefaultPokeRepository.ktdomain/src/main/java/com/twix/domain/repository/PokeRepository.ktdomain/src/main/java/com/twix/domain/usecase/PokeGoalUseCase.ktdomain/src/test/java/com/twix/domain/fake/FakePokeRepository.ktdomain/src/test/java/com/twix/domain/usecase/PokeGoalUseCaseTest.ktfeature/main/src/main/java/com/twix/home/HomeViewModel.ktfeature/photolog/detail/src/main/java/com/twix/photolog/detail/PhotologDetailScreen.ktfeature/photolog/detail/src/main/java/com/twix/photolog/detail/PhotologDetailViewModel.ktfeature/photolog/detail/src/main/java/com/twix/photolog/detail/component/PhotologCardContent.ktfeature/photolog/detail/src/test/java/com/twix/photolog/detail/PhotologDetailViewModelTest.kt
💤 Files with no reviewable changes (2)
- .github/PULL_REQUEST_TEMPLATE.md
- feature/photolog/detail/src/main/java/com/twix/photolog/detail/PhotologDetailScreen.kt
| onClickUpload | ||
| } else { | ||
| { if (!isPokeDisabled) onPoke() } | ||
| { if (!uiState.isPoking) onPoke() } |
There was a problem hiding this comment.
PhotologDetailUiState에 선언되어 있는 isPokeDisabled를 사용하는 건 어떨까요? 지금 isPoking만 바라보고 있으면 쿨다운 상황에서도 클릭 이벤트 자체는 발생할 것 같습니다.
| private fun pokeToPartner() { | ||
| viewModelScope.launch { | ||
| startPokeLoading() | ||
| handlePokeResult(pokeGoalUseCase.invoke(argGoalId)) | ||
| reduce { copy(isPoking = true) } | ||
| handlePokeResult(pokeGoalUseCase.invoke(argGoalId, argTargetDate.toString())) |
There was a problem hiding this comment.
메서드 초반에 isPoking이나 isPokeDisabled 활용해서 가드 로직을 추가하면 좋을 거 같아요
이슈 번호
#170
작업내용
goalId단독에서goalId + targetDate복합키로 변경해, 같은 목표라도 날짜별로 독립적인 쿨타임을 갖도록 수정poke_history스키마 변경에 따라 DB version을 2로 올리고MIGRATION_1_2를 추가poke_history는 날짜 정보가 없는 로컬 쿨타임 캐시라 drop/recreate 처리결과물
123.mp4
리뷰어에게 추가로 요구하는 사항 (선택)