[feat] #51 - TODO 상세 조회#60
Conversation
WalkthroughTodo 상세조회 기능이 추가됨. ChangesTODO 상세조회 기능
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
| boolean completed | ||
| ) { | ||
| public static SubtaskResponse from(Subtask subtask) { | ||
| Objects.requireNonNull(subtask, "subtask must not be null"); |
There was a problem hiding this comment.
P5) 다른 에러 메시지들이 모두 한글로 작성되어 있어서, 해당 메시지도 한글로 통일해도 좋을 것 같습니다!
| boolean completed | ||
| ) { | ||
| public static SubtaskResponse from(Subtask subtask) { | ||
| Objects.requireNonNull(subtask, "subtask must not be null"); |
There was a problem hiding this comment.
정확히 어떤 상황을 대비하기 위한 로직인지 궁금합니다!! 찾아보니 의도적으로 잘못된 상태를 빠르게 발견하기 위한 방어 코드로 사용하는 경우가 있는 것 같은데 혹시 테스트 코드나 예상하지 못한 데이터 유입을 검증하기 위한 목적으로 추가하신 걸까용
There was a problem hiding this comment.
테스트용은 아닙니다. 런타임 계약(contract)을 문서화 + 강제하는 방어 코드입니다. 예상치 못한 데이터 유입을 조기에 드러내기 위한 방어 목적이 맞습니다!
aneykrap
left a comment
aneykrap
left a comment
There was a problem hiding this comment.
코멘트 달아놓은 부분 잘못 이해한거일수도 있는데 한번 확인부탁드려용!!!!
반복은 어려워....
| .orElseThrow(() -> new CustomException(TodoErrorCode.TODO_NOT_FOUND)); | ||
|
|
||
| TodoInstance instance = todoInstanceRepository | ||
| .findByTodo_IdAndDate(todo.getId(), todo.getStartDate()) |
There was a problem hiding this comment.
p2) 현재 상세 조회 요청에서는 조회 기준 날짜를 전달받지 않고 todo.getStartDate()로 TodoInstance를 조회하고 있는 것 같아용
반복 todo를 시작일 이후 날짜에서 선택한 경우에도 시작일 인스턴스의 완료 여부와 타이머 상태가 반환될 수 있지 않을까 합니당....
선택한 날짜의 상태를 보여주는거니깐 조회 날짜를 함께 전달받아야 하지 않을까용(둥글동글)
There was a problem hiding this comment.
네 그렇네요..... 시작 기준 날짜를 기준으로만 조회를 진행해서 해당하는 상세 조회의 특성이 나타나지 않을 것 같습니다. 이부분 수정하겠습니다!
| } | ||
|
|
||
| public TodoDetailResponse getTodoDetail(Long userId, Long todoId) { | ||
| Todo todo = todoRepository.findByIdAndUser_Id(todoId, userId) |
There was a problem hiding this comment.
대부분의 경우 todo Id로 다른 사람의 todo 에 접근하지 못하도록 userId도 같애 넘긴다고 하더라고요!
| public record RepeatResponse( | ||
| String type, | ||
| List<Weekday> weekdays, | ||
| Integer dayOfMonth |
There was a problem hiding this comment.
반복 관련 응답 구조를 묶어서 응답구조가 명확하게 보이는것 같아용!!!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/Timo/Timo/domain/todo/controller/TodoController.java (1)
46-59: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
date누락도 400으로 처리해야 합니다
@RequestParam String date가 빠지면MissingServletRequestParameterException이GlobalExceptionHandler의Exception.class로 떨어져 500이 됩니다.MissingServletRequestParameterException을 400으로 매핑하거나,required = false로 두고 서비스에서INVALID_REQUEST로 통일하세요.🤖 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/controller/TodoController.java` around lines 46 - 59, The getTodoDetail endpoint in TodoController currently lets a missing date request parameter fall through to a 500 via the generic Exception handler. Update the handling around getTodoDetail and GlobalExceptionHandler so MissingServletRequestParameterException is mapped to a 400 response, or make the date parameter optional and validate it explicitly before calling todoService.getTodoDetail, returning INVALID_REQUEST for missing values.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.
Outside diff comments:
In `@src/main/java/com/Timo/Timo/domain/todo/controller/TodoController.java`:
- Around line 46-59: The getTodoDetail endpoint in TodoController currently lets
a missing date request parameter fall through to a 500 via the generic Exception
handler. Update the handling around getTodoDetail and GlobalExceptionHandler so
MissingServletRequestParameterException is mapped to a 400 response, or make the
date parameter optional and validate it explicitly before calling
todoService.getTodoDetail, returning INVALID_REQUEST for missing values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e32c0293-8d95-491f-842a-6f69cfbae72a
⛔ Files ignored due to path filters (1)
src/main/java/com/Timo/Timo/domain/todo/docs/TodoControllerDocs.javais excluded by!**/docs/**
📒 Files selected for processing (4)
src/main/java/com/Timo/Timo/domain/todo/controller/TodoController.javasrc/main/java/com/Timo/Timo/domain/todo/dto/response/TodoDetailResponse.javasrc/main/java/com/Timo/Timo/domain/todo/exception/TodoErrorCode.javasrc/main/java/com/Timo/Timo/domain/todo/service/TodoService.java
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/Timo/Timo/domain/todo/exception/TodoErrorCode.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/Timo/Timo/domain/todo/dto/response/TodoDetailResponse.java
laura-jung
force-pushed
the
feat/#51-todoDetail
branch
from
3a1df95 to
8bf1c00
Compare
github-actions
Bot
added
✨ feat
slack-approval-notified
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/java/com/Timo/Timo/domain/todo/exception/TodoErrorCode.java (1)
16-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value메시지 끝 마침표 불일치
TODO_NOT_FOUND메시지는 마침표(.)가 없고,TODO_NOT_FOUND_ON_DATE메시지는 마침표가 있습니다. 메시지 포맷 일관성을 맞추는 것이 좋습니다.🔧 마침표 통일 제안
- TODO_NOT_FOUND(HttpStatus.NOT_FOUND, "TODO_404", "존재하지 않는 투두입니다"), + TODO_NOT_FOUND(HttpStatus.NOT_FOUND, "TODO_404", "존재하지 않는 투두입니다."),🤖 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/exception/TodoErrorCode.java` at line 16, TODO_NOT_FOUND 메시지의 문장 끝 표기를 TODO_NOT_FOUND_ON_DATE와 일관되게 맞춰주세요. TodoErrorCode enum의 TODO_NOT_FOUND 상수 메시지 문자열을 수정해 다른 에러 메시지들과 같은 마침표 종료 규칙을 따르도록 정리하면 됩니다.
🤖 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/exception/TodoErrorCode.java`:
- Around line 16-17: `TodoErrorCode`에서 `TODO_NOT_FOUND`와
`TODO_NOT_FOUND_ON_DATE`가 같은 code 값을 공유하고 있어 클라이언트가 구분할 수 없습니다. `TodoErrorCode`
enum의 각 상수를 확인해 `TODO_NOT_FOUND_ON_DATE`에 `TODO_404_DATE`처럼 별도 식별 코드 값을 부여하고, 관련
에러 매핑이나 응답 생성 로직이 이 새로운 코드를 그대로 사용하도록 맞춰주세요.
- Around line 16-20: Normalize the indentation in TodoErrorCode so the enum
constants use the same leading whitespace as the surrounding entries; the new
TODO_NOT_FOUND, TODO_NOT_FOUND_ON_DATE, MAX_COUNT_EXCEEDED,
IS_COMPLETED_REQUIRED, and TIMER_RUNNING lines should match the existing enum
formatting in this class. Keep the TodoErrorCode enum declaration consistent
throughout by adjusting the indentation only, without changing the enum values
or messages.
---
Nitpick comments:
In `@src/main/java/com/Timo/Timo/domain/todo/exception/TodoErrorCode.java`:
- Line 16: TODO_NOT_FOUND 메시지의 문장 끝 표기를 TODO_NOT_FOUND_ON_DATE와 일관되게 맞춰주세요.
TodoErrorCode enum의 TODO_NOT_FOUND 상수 메시지 문자열을 수정해 다른 에러 메시지들과 같은 마침표 종료 규칙을
따르도록 정리하면 됩니다.
🪄 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: d6817cdc-721c-490d-9b27-cc8288973d58
⛔ Files ignored due to path filters (1)
src/main/java/com/Timo/Timo/domain/todo/docs/TodoControllerDocs.javais excluded by!**/docs/**
📒 Files selected for processing (5)
src/main/java/com/Timo/Timo/domain/todo/controller/TodoController.javasrc/main/java/com/Timo/Timo/domain/todo/dto/response/TodoDetailResponse.javasrc/main/java/com/Timo/Timo/domain/todo/exception/TodoErrorCode.javasrc/main/java/com/Timo/Timo/domain/todo/exception/TodoSuccessCode.javasrc/main/java/com/Timo/Timo/domain/todo/service/TodoService.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/Timo/Timo/domain/todo/controller/TodoController.java
- src/main/java/com/Timo/Timo/domain/todo/service/TodoService.java
| TODO_NOT_FOUND(HttpStatus.NOT_FOUND, "TODO_404", "존재하지 않는 투두입니다"), | ||
| TODO_NOT_FOUND_ON_DATE(HttpStatus.NOT_FOUND, "TODO_404", "해당 날짜에 존재하지 않는 투두입니다."), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
TODO_NOT_FOUND와 TODO_NOT_FOUND_ON_DATE가 동일한 에러 코드 "TODO_404"를 공유
두 에러 상황이 서로 다른 의미(존재하지 않는 투두 vs 해당 날짜에 없는 투두)임에도 동일한 code 값을 사용하고 있습니다. 클라이언트가 에러 코드만으로 두 상황을 구분할 수 없어, TODO_NOT_FOUND_ON_DATE에 별도 코드(예: "TODO_404_1" 또는 "TODO_404_DATE")를 부여하는 것을 고려해 보세요.
🤖 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/exception/TodoErrorCode.java` around
lines 16 - 17, `TodoErrorCode`에서 `TODO_NOT_FOUND`와 `TODO_NOT_FOUND_ON_DATE`가 같은
code 값을 공유하고 있어 클라이언트가 구분할 수 없습니다. `TodoErrorCode` enum의 각 상수를 확인해
`TODO_NOT_FOUND_ON_DATE`에 `TODO_404_DATE`처럼 별도 식별 코드 값을 부여하고, 관련 에러 매핑이나 응답 생성
로직이 이 새로운 코드를 그대로 사용하도록 맞춰주세요.
| TODO_NOT_FOUND(HttpStatus.NOT_FOUND, "TODO_404", "존재하지 않는 투두입니다"), | ||
| TODO_NOT_FOUND_ON_DATE(HttpStatus.NOT_FOUND, "TODO_404", "해당 날짜에 존재하지 않는 투두입니다."), | ||
| MAX_COUNT_EXCEEDED(HttpStatus.CONFLICT, "TODO_409", "해당 날짜의 투두가 최대 개수(20개)를 초과했습니다."), | ||
| IS_COMPLETED_REQUIRED(HttpStatus.BAD_REQUEST, "COMMON_400", "isCompleted는 필수입니다."), | ||
| TIMER_RUNNING(HttpStatus.CONFLICT, "TODO_409", "타이머가 실행 중인 TODO는 변경할 수 없습니다. 타이머를 먼저 종료해주세요."); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
들여쓰기 불일치
변경된 라인 16-20은 tab + 공백 2칸으로 들여쓰기되어 있지만, 위쪽 라인 14-15는 tab 1칸만 사용하고 있습니다. 포맷팅 일관성을 위해 동일한 들여쓰기를 사용해 주세요.
🔧 들여쓰기 수정 제안
INVALID_REQUEST(HttpStatus.BAD_REQUEST, "TODO_400", "필수 필드가 누락되었거나 형식이 올바르지 않습니다."),
INVALID_TITLE(HttpStatus.BAD_REQUEST, "TODO_400", "투두명은 한국어 20자/영어 30자를 초과할 수 없습니다."),
- TODO_NOT_FOUND(HttpStatus.NOT_FOUND, "TODO_404", "존재하지 않는 투두입니다"),
- TODO_NOT_FOUND_ON_DATE(HttpStatus.NOT_FOUND, "TODO_404", "해당 날짜에 존재하지 않는 투두입니다."),
- MAX_COUNT_EXCEEDED(HttpStatus.CONFLICT, "TODO_409", "해당 날짜의 투두가 최대 개수(20개)를 초과했습니다."),
- IS_COMPLETED_REQUIRED(HttpStatus.BAD_REQUEST, "COMMON_400", "isCompleted는 필수입니다."),
- TIMER_RUNNING(HttpStatus.CONFLICT, "TODO_409", "타이머가 실행 중인 TODO는 변경할 수 없습니다. 타이머를 먼저 종료해주세요.");
+ TODO_NOT_FOUND(HttpStatus.NOT_FOUND, "TODO_404", "존재하지 않는 투두입니다"),
+ TODO_NOT_FOUND_ON_DATE(HttpStatus.NOT_FOUND, "TODO_404", "해당 날짜에 존재하지 않는 투두입니다."),
+ MAX_COUNT_EXCEEDED(HttpStatus.CONFLICT, "TODO_409", "해당 날짜의 투두가 최대 개수(20개)를 초과했습니다."),
+ IS_COMPLETED_REQUIRED(HttpStatus.BAD_REQUEST, "COMMON_400", "isCompleted는 필수입니다."),
+ TIMER_RUNNING(HttpStatus.CONFLICT, "TODO_409", "타이머가 실행 중인 TODO는 변경할 수 없습니다. 타이머를 먼저 종료해주세요.");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| TODO_NOT_FOUND(HttpStatus.NOT_FOUND, "TODO_404", "존재하지 않는 투두입니다"), | |
| TODO_NOT_FOUND_ON_DATE(HttpStatus.NOT_FOUND, "TODO_404", "해당 날짜에 존재하지 않는 투두입니다."), | |
| MAX_COUNT_EXCEEDED(HttpStatus.CONFLICT, "TODO_409", "해당 날짜의 투두가 최대 개수(20개)를 초과했습니다."), | |
| IS_COMPLETED_REQUIRED(HttpStatus.BAD_REQUEST, "COMMON_400", "isCompleted는 필수입니다."), | |
| TIMER_RUNNING(HttpStatus.CONFLICT, "TODO_409", "타이머가 실행 중인 TODO는 변경할 수 없습니다. 타이머를 먼저 종료해주세요."); | |
| INVALID_REQUEST(HttpStatus.BAD_REQUEST, "TODO_400", "필수 필드가 누락되었거나 형식이 올바르지 않습니다."), | |
| INVALID_TITLE(HttpStatus.BAD_REQUEST, "TODO_400", "투두명은 한국어 20자/영어 30자를 초과할 수 없습니다."), | |
| TODO_NOT_FOUND(HttpStatus.NOT_FOUND, "TODO_404", "존재하지 않는 투두입니다"), | |
| TODO_NOT_FOUND_ON_DATE(HttpStatus.NOT_FOUND, "TODO_404", "해당 날짜에 존재하지 않는 투두입니다."), | |
| MAX_COUNT_EXCEEDED(HttpStatus.CONFLICT, "TODO_409", "해당 날짜의 투두가 최대 개수(20개)를 초과했습니다."), | |
| IS_COMPLETED_REQUIRED(HttpStatus.BAD_REQUEST, "COMMON_400", "isCompleted는 필수입니다."), | |
| TIMER_RUNNING(HttpStatus.CONFLICT, "TODO_409", "타이머가 실행 중인 TODO는 변경할 수 없습니다. 타이머를 먼저 종료해주세요."); |
🤖 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/exception/TodoErrorCode.java` around
lines 16 - 20, Normalize the indentation in TodoErrorCode so the enum constants
use the same leading whitespace as the surrounding entries; the new
TODO_NOT_FOUND, TODO_NOT_FOUND_ON_DATE, MAX_COUNT_EXCEEDED,
IS_COMPLETED_REQUIRED, and TIMER_RUNNING lines should match the existing enum
formatting in this class. Keep the TodoErrorCode enum declaration consistent
throughout by adjusting the indentation only, without changing the enum values
or messages.
3 participants
관련 이슈 🛠
작업 내용 요약 ✏️
todoId로 접근시 조회되는 todo 상세 조회 api를 설계하였습니다.
주요 변경 사항 🛠️
스크린샷 📷
Summary by CodeRabbit