Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. Walkthrough통계 서비스가 규칙 발생 데이터를 기준으로 일별·월별 통계를 계산하도록 변경되었습니다. 기존 통계 전용 저장소 쿼리와 프로젝션은 제거되었습니다. 미래 날짜의 규칙 발생도 통계 계산에 포함됩니다. Changes통계 발생 집계 전환
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to No confirmed merge-blocking risk remains in the statistics occurrence transition. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
|
|
@coderabbitai full review |
|
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Jy000n
left a comment
Jy000n
left a comment
There was a problem hiding this comment.
고생하셨습니다아!! 반복 일정에 맞게 통계도 잘 바뀐 것 같네용
코리를 달던 중 궁금한 점이 생겼는데요,, 그러면 화목에 반복 일정이 있는 투두라고 쳤을 때, 1주차는 화목 투두를 완료한 후에, 해당 반복 투두 일정을 그 이후부터 월요일로 변경하고 싶으면 어떻게 되는걸까요..?? 변경하게 된다면 현재 코드로서는 기존에 1주차(화목)에 완료했던 투두 기록이 사라지는 것 같아서 관련하여 리뷰 같이 남겨보았으니 참고해주시면 감사하겠습니다 :)
| private DailyOccurrence summarizeDate(List<Todo> rules, Map<InstanceKey, TodoInstance> instancesByKey, LocalDate date) { | ||
| int totalCount = 0; | ||
| int completedCount = 0; | ||
|
|
||
| for (Todo rule : rules) { | ||
| if (!todoDateCalculator.occursOn(rule, date)) { | ||
| continue; | ||
| } | ||
| totalCount++; | ||
| TodoInstance instance = instancesByKey.get(new InstanceKey(rule.getId(), date)); | ||
| if (instance != null && instance.isCompleted()) { | ||
| completedCount++; | ||
| } | ||
| } |
There was a problem hiding this comment.
[P3] 반복 규칙 변경 시 기존 완료 기록이 통계에서 누락될 수 있는 것 같습니다! summarizeDate는 occursOn이 false면 바로 continue해서 이미 존재하는 TodoInstance도 무시하게 돼서 그런 것 같습니다!
ex) 화/목 반복 투두를 1주차에서 완료 처리한 뒤 그 이후 반복 요일을 월요일로 바꾸면 통계(summary/calendar)에서는 해당 완료 기록이 사라집니다.
laura-jung
left a comment
laura-jung
left a comment
There was a problem hiding this comment.
계획된 일정과 다른 날짜에 실행된 경우를 집계하는게 좀 많이 복잡하네용.....
코멘트 남겨두었으니 확인부탁드립니다!!
수고하셨습니다!
| Map<Long, String> tagNamesById = findTagNames(instances); | ||
| List<DailyTodoResponse> todos = instances.stream() | ||
| .map(instance -> toDailyTodoResponse(instance, actualSecondsByTodoId, tagNamesById)) | ||
| List<Todo> occurringTodos = statisticsOccurrenceCalculator.findOccurringRules(userId, date); |
There was a problem hiding this comment.
[P1] 예정일과 실행일이 다른 타이머 기록이 일별 상세 목록에서 누락될 수 있는 로직인 것 같습니다.
예를 들어 9/10에 예정된 투두를 9/11에 30분 실행한 뒤 /statistics/daily?date=2026-09-11을 조회하면, 타이머 쿼리는 실제 기록 시각 기준으로 30분을 가져옵니다. 하지만 todos는 findOccurringRules()로 9/11에 예정된 규칙만 조회하므로 해당 투두가 제외될 수도 있을 것 같아요
예정된 투두 목록에 해당 날짜의 타이머 기록에 등장한 todoId도 합쳐서 반환하는 방식도 있어야 할 것 같습니다!
| int completedCount = 0; | ||
|
|
||
| for (Todo rule : rules) { | ||
| if (!todoDateCalculator.occursOn(rule, date)) { |
There was a problem hiding this comment.
[p3] 이부분에서 원래의 반복 계획을 먼저 확인하는것 같네요! 이렇게 되면 반복 계획일과 실행된 날짜가 맞지 않을경우 제외될 수 있을 것 같습니다!
예를 들어 ‘매주 월요일 독서’ 투두를 화요일에 실행할 때, 타이머 시작 요청에서 targetDate를 생략하면 오늘인 화요일을 사용합니다. 이 경우 화요일 TodoInstance가 생성되고, 타이머 종료 시 ‘화요일 독서 완료’로 저장될 수 있습니다.
하지만 현재 계산기는 ‘화요일은 월요일 반복 규칙의 발생일이 아니다’라는 이유로 바로 제외합니다. 따라서 실제로 화요일 완료 기록이 저장돼 있어도 완료 개수에는 반영되지 않습니다.
3 participants
관련 이슈 🛠
작업 내용 요약 ✏️
/api/v1/statistics/summary,/calendar,/daily) API가 실제 TodoInstance row(완료 토글·타이머 시작 등으로 사용자가 건드릴 때만 생성됨)만 보고 집계하던 방식을 반복 투두의 발생 규칙(target_date) 기준으로 집계하도록 수정주요 변경 사항 🛠
StatisticsOccurrenceCalculator추가:TodoRepository.findRulesInRange+TodoDateCalculator.occursOn+TodoInstanceRepository.findByTodoIdsAndDateRange를 통해반복 규칙상 발생하는 날짜는 인스턴스 유무와 무관하게 집계하고 완료 여부만 실제 인스턴스가 있을 때 덧입히도록 계산
StatisticsService.getCalendar/getSummary/getDaily가 위 계산로직을 사용하도록 변경TodoRepository의 통계 전용 쿼리 2개(findDailyCompletionStats,findMonthlySummaryStats)와 프로젝션 인터페이스 2개(TodoDailyCompletionStats,TodoMonthlySummaryStats) 삭제트러블 슈팅 ⚽️
TodoRepository의 쿼리는 이미TodoInstance.date/completed기준으로 바뀌어 있었지만StatisticsService가 옛 시그니처(Todo.createdAt기반)를 그대로호출하고 있어 컴파일 자체가 깨진 상태였음 — 우선 바로잡음
totalTodoCount가 실제 발생 9회 중 완료 처리한 3회로만 집계됨.getDaily의 응답 정렬 순서가 기존TodoInstance.sortOrder(사용자가 수동 정렬한 순서) 대신Todo규칙 등록순(createdAt asc)으로 바뀜 — 통계 조회 화면이라 UI 정렬과는 무관하다고 판단하고 진행함테스트 결과 📄
./gradlew compileJava,./gradlew test전체 통과-
summary: 9월 8회 발생 중 2회만 완료 처리 →totalTodoCount: 8,completedTodoCount: 2,activeDayCount: 8로 정상 반영 (수정 전에는2/2/2로 누락됐었음)-
calendar: 완료한 날짜는 100%, 발생했지만 미완료인 날짜는 0%로 정상 표시(날짜 자체가 누락되지 않음)-
daily: 아직 오지 않은/한 번도 안 건드린 발생일도 투두 목록에 정상적으로 포함됨스크린샷 📷
리뷰 요구사항 📢
뭔가 복잡한 느낌과 빼먹은 기분이 드네여... 티모버들이 한번 더 확인해주시면 감사하겠습니당
📎 참고 자료 (선택)
없음
Summary by CodeRabbit