Skip to content

[fix] #197 - 반복 투두 완료 처리 시 타이머 검사를 날짜 단위로 변경 - #200

Merged
laura-jung merged 1 commit into
developfrom
refactor/#197-todoRefactor
Sep 3, 2026
Merged

[fix] #197 - 반복 투두 완료 처리 시 타이머 검사를 날짜 단위로 변경#200
laura-jung merged 1 commit into
developfrom
refactor/#197-todoRefactor

Conversation

@laura-jung

@laura-jung laura-jung commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

관련 이슈 🛠

작업 내용 요약 ✏️

반복 투두의 완료 처리 시, 다른 날짜의 타이머가 실행 중이면 해당 규칙의 모든 날짜 완료가 막히던 문제를 수정했습니다. Todo(규칙)와 TodoInstance(날짜별 실체)가 결합된 지점 중, 완료 가드가 날짜를 무시하고 규칙 단위로 타이머를 검사하던 것이 원인이었습니다.

주요 변경 사항 🛠️

  • 완료 가드 날짜 스코프화: TodoService.changeCompletion에서 hasActiveTimer(todoId)hasActiveTimerOn(todoId, date)로 변경하여, 해당 날짜에 실행 중인 타이머만 완료를 차단하도록 수정
  • TimerService 메서드 추가: 규칙+날짜 단위로 활성 타이머를 확인하는 hasActiveTimerOn(todoId, date) 추가
  • Repository 조회 추가: existsByTodo_IdAndTargetDateAndStatusIn(todoId, targetDate, statuses) 추가

트러블 슈팅 ⚽️

  • 문제: 유저당 활성 타이머는 1개만 허용되는데, 완료 가드가 todoId(규칙)만으로 활성 타이머를 검사 → 9/2 타이머 실행 중 9/3 완료가 불가능했습니다.
  • 원인: 타이머는 targetDate로 날짜가 구분되지만, 완료 검사 로직만 날짜를 전달하지 않고 규칙 전체를 기준으로 판단했습니다.
  • 안전성 확인: TimerService.startTimer에서 targetDate가 항상 non-null(resolvedDate)로 저장되므로, targetDate 기준 조회에서 활성 타이머가 누락되지 않음을 확인했습니다.
  • 범위 한정: updateTodo(스케줄·소요시간 변경), deleteTodo(규칙 전체 삭제)는 전 날짜에 영향을 주는 작업이므로 기존의 규칙 단위 hasActiveTimer 검사를 그대로 유지했습니다.

테스트 결과 📄

compileJava 통과. Swagger 수동 검증:

스크린샷 📷

테스트는 '매일 반복 일정' 생성 -> 해당 반복 일정에 해당하는 타이머 하나 실행 -> 다른 반복 일정 완료 처리 진행
스크린샷 2026-09-02 오전 3 28 54
스크린샷 2026-09-02 오전 3 32 29
스크린샷 2026-09-02 오전 3 33 22

리뷰 요구사항 📢

  • TODO를 전체적으로 리팩토링을 해볼까 했는데 화면설계서가 수정될 예정이라고 해서 화면설계서 나오면 좀더 본격적으로 고칠 예정입니다

Summary by CodeRabbit

  • 기능 개선

    • 할 일 완료 상태를 변경할 때, 해당 할 일과 날짜에 실행 중이거나 일시 정지된 타이머가 있는지 확인합니다.
    • 특정 날짜의 활성 타이머 존재 여부를 확인할 수 있습니다.
    • 타이머 상태가 실행 중 또는 일시 정지인 기록을 기준으로 정확하게 판단합니다.
  • 동작 개선

    • 타이머 기록과 할 일 완료 상태 간의 날짜 기준 검증이 강화되었습니다.
    • 기존의 현재 활성 타이머 확인 기능은 그대로 유지됩니다.

다른 날짜의 타이머가 실행 중일 때 해당 규칙의 모든 날짜 완료가 막히던
문제를 수정. hasActiveTimer(todoId) 대신 날짜를 함께 확인하는
hasActiveTimerOn(todoId, date)으로 완료 가드를 변경했다.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@laura-jung laura-jung self-assigned this Sep 1, 2026
@laura-jung laura-jung linked an issue Sep 1, 2026 that may be closed by this pull request
1 task
@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 84b96c65-80de-4241-8386-6ff3af23a8fa

📥 Commits

Reviewing files that changed from the base of the PR and between b6ccdc9 and e8d629d.

📒 Files selected for processing (3)
  • src/main/java/com/Timo/Timo/domain/timer/repository/TimerRecordRepository.java
  • src/main/java/com/Timo/Timo/domain/timer/service/TimerService.java
  • src/main/java/com/Timo/Timo/domain/todo/service/TodoService.java

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

활성 타이머 조회에 목표 날짜 조건이 추가되었습니다. TimerService는 특정 todo와 날짜의 RUNNING 또는 PAUSED 기록을 확인합니다. TodoService는 완료 처리 시 날짜별 검사를 사용합니다.

Changes

날짜별 타이머 검사

Layer / File(s) Summary
날짜별 활성 타이머 조회
src/main/java/com/Timo/Timo/domain/timer/repository/TimerRecordRepository.java, src/main/java/com/Timo/Timo/domain/timer/service/TimerService.java
저장소에 todo ID, 목표 날짜, 상태 목록을 사용하는 존재 여부 조회 메서드를 추가했습니다. TimerService는 해당 메서드로 날짜별 활성 타이머를 확인합니다.
할 일 완료 처리 연동
src/main/java/com/Timo/Timo/domain/todo/service/TodoService.java
changeCompletion의 활성 타이머 검사를 todo와 날짜 기준으로 변경했습니다.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to e8d62

This PR limits completion blocking to timers running on the same date while preserving broader guards for rule-wide changes; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: aneykrap, jy000n

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning PR은 반복 투두 완료 시 활성 타이머 검사 범위를 날짜 단위로 변경합니다. 그러나 연결된 이슈 #197의 주요 요구사항인 todo 및 todoInstance 엔티티 리팩토링을 수행한 근거가 없습니다. todo 및 todoInstance 엔티티 리팩토링을 구현하거나, 현재 변경 사항에 맞는 이슈를 연결해 주세요. 이슈 #197의 범위를 현재 동작 변경으로 명확히 갱신하는 방법도 사용할 수 있습니다.
Out of Scope Changes check ⚠️ Warning 타이머 Repository와 TimerService에 날짜별 활성 타이머 조회 기능을 추가하고 완료 처리 로직을 변경했습니다. 이 변경은 연결된 이슈 #197의 todo 및 todoInstance 엔티티 리팩토링 범위와 직접 일치하지 않습니다. 타이머 검사 동작 변경을 별도 이슈로 분리하거나, 연결된 이슈에 해당 요구사항을 추가해 범위를 명확히 하세요. 엔티티 리팩토링과 무관한 변경은 별도 PR로 분리하세요.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PR 제목은 반복 투두 완료 처리의 타이머 검사를 규칙 단위에서 날짜 단위로 변경하는 주요 변경 사항을 정확하게 설명합니다.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/#197-todoRefactor

Comment @coderabbitai help to get the list of available commands.

@Jy000n Jy000n left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오옹 PR 잘 읽었숩니당 코드도 적절하게 잘 바꿔주신 것 같네용👍


boolean existsByTodo_IdAndStatusIn(Long todoId, List<TimerStatus> statuses);

boolean existsByTodo_IdAndTargetDateAndStatusIn(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저 ".. Todo_Id .."에서 _가 뭘 의미하는 거였죠,, 합숙하면서 들었었는데 까먹었습니다..😥

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아하 todo의 id를 말합니다. 보통 sql문으로 쓸때는 todo.id로 불러올텐데 메서드명으로 쓸 때는 저렇게 Todo_Id로 표기하는 것으로 알고 있습니다

@aneykrap aneykrap left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 잘 읽었습니당
hasActiveTimerOn(todoId, date)로 해서 이제 해당날짜에 실행중인 타이머만 완료처리가 가능해지겠어용!! 고생하셨습니다!

@github-actions github-actions Bot added the slack-approval-notified Slack 승인 완료 알림 중복 방지용 라벨 label Sep 2, 2026
@laura-jung
laura-jung merged commit 99218bf into develop Sep 3, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🍀 윤아 🛠️ fix slack-approval-notified Slack 승인 완료 알림 중복 방지용 라벨

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[refactor] todo 엔티티 리팩토링

3 participants