[hotfix] #191 - 반복 투두의 하위 태스크 완료·메모를 날짜별로 분리#193
Conversation
|
Warning Review limit reached
Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Walkthrough반복 Todo의 서브태스크 완료 상태를 날짜별 ChangesTodoInstance 기반 상태 관리
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant TodoController
participant TodoService
participant SubtaskCompletionRepository
Client->>TodoController: 날짜와 서브태스크 상태 전송
TodoController->>TodoService: 날짜 포함 상태 변경 호출
TodoService->>SubtaskCompletionRepository: 완료 레코드 조회 또는 저장
TodoService->>SubtaskCompletionRepository: 완료 상태 갱신
TodoService-->>TodoController: 상태 변경 응답
TodoController-->>Client: 응답 반환
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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: 1
🧹 Nitpick comments (3)
db/migration/2026-07-16__drop_subtasks_completed.sql (1)
5-7: 🧹 Nitpick | 🔵 Trivial | 💤 Low value무중단 배포(Zero-downtime deployment)를 위한 운영 환경 고려 사항
컬럼을 삭제하는 DDL 구문은 현재 이 테이블과 컬럼을 참조하고 있는 구버전 애플리케이션 인스턴스에서 SQL 오류를 발생시킬 수 있습니다. 시스템이 무중단 배포 환경으로 운영 중이거나 향후 이를 계획하고 있다면, 애플리케이션 코드에서 해당 컬럼에 대한 참조를 완전히 제거한 버전을 성공적으로 배포한 후, 다음 배포 주기나 별도의 마이그레이션 단계에서 컬럼을 안전하게 삭제하는 것을 권장합니다.
🤖 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 `@db/migration/2026-07-16__drop_subtasks_completed.sql` around lines 5 - 7, Defer the DROP COLUMN operation in migration 2026-07-16__drop_subtasks_completed.sql until after all application instances no longer reference subtasks.completed. Remove or postpone the ALTER TABLE statement so zero-downtime deployments retain compatibility with older application versions, then perform the column deletion in a subsequent migration or deployment phase.src/main/java/com/Timo/Timo/domain/home/service/HomeTodoReader.java (1)
89-102: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win데이터베이스 레벨에서 완료된 하위 태스크만 필터링하도록 조회 성능 개선
현재
findByTodoInstance_IdIn을 통해 모든SubtaskCompletion레코드를 데이터베이스에서 조회한 후, Java의Stream.filter를 사용해 메모리 상에서completed == true인 항목을 걸러내고 있습니다. 데이터가 늘어날 경우 완료되지 않은 불필요한 레코드까지 메모리에 로드하게 되어 메모리와 네트워크 I/O 성능 저하가 발생할 수 있습니다.Repository에
completed == true조건이 포함된 쿼리 메서드를 추가하여 데이터베이스 레벨에서 즉시 필터링하도록 개선하는 것을 권장합니다. (이 방식을 적용할 경우 동일하게 필터링 로직을 수행 중인TodoService.resolveCompletedSubtaskIds등의 다른 로직에도 함께 적용할 수 있습니다.)As per path instructions, 쿼리 성능과 N+1 문제가 없는지 확인해야 합니다.
♻️ 수정 제안
SubtaskCompletionRepository에 새로운 쿼리 메서드를 추가합니다:List<SubtaskCompletion> findByTodoInstance_IdInAndCompletedTrue(Collection<Long> todoInstanceIds);이후
HomeTodoReader의 로직을 다음과 같이 수정합니다:- return subtaskCompletionRepository.findByTodoInstance_IdIn(instanceIds).stream() - .filter(SubtaskCompletion::isCompleted) + return subtaskCompletionRepository.findByTodoInstance_IdInAndCompletedTrue(instanceIds).stream() .collect(Collectors.groupingBy( completion -> completion.getTodoInstance().getId(), Collectors.mapping(completion -> completion.getSubtask().getId(), Collectors.toSet()) ));🤖 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 `@src/main/java/com/Timo/Timo/domain/home/service/HomeTodoReader.java` around lines 89 - 102, Update SubtaskCompletionRepository with a findByTodoInstance_IdInAndCompletedTrue(Collection<Long> todoInstanceIds) query method, then change HomeTodoReader.loadCompletedSubtaskIds to use it and remove the in-memory SubtaskCompletion::isCompleted filter. Preserve the existing empty-input handling and grouping behavior, and verify the repository query fetches the required associations without introducing N+1 queries.Source: Path instructions
src/main/java/com/Timo/Timo/domain/todo/service/TodoService.java (1)
159-169: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win불필요한 데이터베이스 UPDATE 쿼리를 방지하도록 초기화 로직을 개선해 주세요.
현재 로직은 완료 레코드가 존재하지 않을 때 항상
false로 데이터베이스에 저장(INSERT)한 직후updateCompleted를 호출하여 상태를 덮어씁니다. 이로 인해 트랜잭션 종료 시 의도치 않은UPDATE쿼리가 추가로 발생할 수 있습니다.요청받은 상태(
request.isCompleted())로 엔티티를 바로 초기화하고, 마지막에 한 번만save를 호출하여 영속성 컨텍스트를 효율적으로 활용하도록 리팩토링하는 것을 권장합니다.♻️ 제안하는 수정안
requireOccurrence(todo, date); TodoInstance instance = todoInstanceReorderer.materializeInstance(userId, todo, date); SubtaskCompletion completion = subtaskCompletionRepository .findByTodoInstance_IdAndSubtask_Id(instance.getId(), subtaskId) - .orElseGet(() -> subtaskCompletionRepository.save( - SubtaskCompletion.of(instance, subtask, false))); + .orElseGet(() -> SubtaskCompletion.of(instance, subtask, request.isCompleted())); + completion.updateCompleted(request.isCompleted()); + subtaskCompletionRepository.save(completion); return new SubtaskStatusChangeResponse(subtaskId, request.isCompleted());🤖 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 `@src/main/java/com/Timo/Timo/domain/todo/service/TodoService.java` around lines 159 - 169, Update the completion initialization in the subtask status change method around subtaskCompletionRepository and SubtaskCompletion.of so new records are created with request.isCompleted() instead of false. Remove the unconditional completion.updateCompleted call, then invoke subtaskCompletionRepository.save exactly once after resolving or creating the completion, preserving the existing response behavior.Source: Path instructions
🤖 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
`@src/main/java/com/Timo/Timo/domain/todo/repository/SubtaskCompletionRepository.java`:
- Around line 20-22: Update the deleteByTodoId bulk JPQL query to avoid the
implicit association traversal through sc.todoInstance.todo.id. Use a subquery
that explicitly selects matching TodoInstance IDs for the given todoId, then
delete SubtaskCompletion records whose todoInstance ID is in that result.
---
Nitpick comments:
In `@db/migration/2026-07-16__drop_subtasks_completed.sql`:
- Around line 5-7: Defer the DROP COLUMN operation in migration
2026-07-16__drop_subtasks_completed.sql until after all application instances no
longer reference subtasks.completed. Remove or postpone the ALTER TABLE
statement so zero-downtime deployments retain compatibility with older
application versions, then perform the column deletion in a subsequent migration
or deployment phase.
In `@src/main/java/com/Timo/Timo/domain/home/service/HomeTodoReader.java`:
- Around line 89-102: Update SubtaskCompletionRepository with a
findByTodoInstance_IdInAndCompletedTrue(Collection<Long> todoInstanceIds) query
method, then change HomeTodoReader.loadCompletedSubtaskIds to use it and remove
the in-memory SubtaskCompletion::isCompleted filter. Preserve the existing
empty-input handling and grouping behavior, and verify the repository query
fetches the required associations without introducing N+1 queries.
In `@src/main/java/com/Timo/Timo/domain/todo/service/TodoService.java`:
- Around line 159-169: Update the completion initialization in the subtask
status change method around subtaskCompletionRepository and SubtaskCompletion.of
so new records are created with request.isCompleted() instead of false. Remove
the unconditional completion.updateCompleted call, then invoke
subtaskCompletionRepository.save exactly once after resolving or creating the
completion, preserving the existing response behavior.
🪄 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.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e16e2498-0b28-40c0-bdb4-43a802fd817f
⛔ Files ignored due to path filters (2)
src/main/java/com/Timo/Timo/domain/timer/docs/TimerStartControllerDocs.javais excluded by!**/docs/**src/main/java/com/Timo/Timo/domain/todo/docs/TodoControllerDocs.javais excluded by!**/docs/**
📒 Files selected for processing (17)
db/migration/2026-07-16__drop_subtasks_completed.sqlsrc/main/java/com/Timo/Timo/domain/focus/service/FocusService.javasrc/main/java/com/Timo/Timo/domain/home/dto/response/HomeResponse.javasrc/main/java/com/Timo/Timo/domain/home/mapper/HomeTodoMapper.javasrc/main/java/com/Timo/Timo/domain/home/service/HomeTodoReader.javasrc/main/java/com/Timo/Timo/domain/todo/controller/TodoController.javasrc/main/java/com/Timo/Timo/domain/todo/dto/request/TodoMemoUpdateRequest.javasrc/main/java/com/Timo/Timo/domain/todo/dto/request/TodoSubtaskUpdateRequest.javasrc/main/java/com/Timo/Timo/domain/todo/dto/request/TodoUpdateRequest.javasrc/main/java/com/Timo/Timo/domain/todo/dto/response/SubtaskStatusChangeResponse.javasrc/main/java/com/Timo/Timo/domain/todo/dto/response/TodoDetailResponse.javasrc/main/java/com/Timo/Timo/domain/todo/entity/Subtask.javasrc/main/java/com/Timo/Timo/domain/todo/entity/SubtaskCompletion.javasrc/main/java/com/Timo/Timo/domain/todo/entity/Todo.javasrc/main/java/com/Timo/Timo/domain/todo/entity/TodoInstance.javasrc/main/java/com/Timo/Timo/domain/todo/repository/SubtaskCompletionRepository.javasrc/main/java/com/Timo/Timo/domain/todo/service/TodoService.java
💤 Files with no reviewable changes (2)
- src/main/java/com/Timo/Timo/domain/todo/dto/response/SubtaskStatusChangeResponse.java
- src/main/java/com/Timo/Timo/domain/todo/dto/request/TodoUpdateRequest.java
1 participant
관련 이슈 🛠
작업 내용 요약 ✏️
반복 투두는 여러 날짜가 같은 todoId(규칙)를 공유하는데, 하위 태스크 완료 여부와 메모가
규칙 레벨에 저장되어 모든 날짜가 상태를 공유하는 문제가 있었습니다.
상위 투두 완료는 이미 날짜별(TodoInstance)로 분리되어 있어 하위와 불일치가 발생했습니다.
주요 변경 사항 🛠️
SubtaskCompletion엔티티 신규(todo_instance, subtask, completed)+ unique(todo_instance_id, subtask_id)Subtask(규칙)에서 공유, 완료 여부만 인스턴스별 저장date추가 → 해당 날짜 인스턴스의 완료만 upsertSubtask.completed및 관련 필드/DTO 제거테스트 결과 📄
매일 투두 생성하여 일정 투두만 하위태스크 완료 여부 변경진행하였습니다.
스크린샷 📷
리뷰 요구사항 📢
바쁘니까 머지할게요
Summary by CodeRabbit
새로운 기능
개선