[REFACTOR] 카테고리, 포폴, 상품, 예약, 리뷰, 위시 도메인 Annotation 재정비 #251
[REFACTOR] 카테고리, 포폴, 상품, 예약, 리뷰, 위시 도메인 Annotation 재정비 #251
Conversation
📝 WalkthroughWalkthroughAPI 인터페이스로 HTTP 매핑 및 일부 파라미터/검증 어노테이션을 이동하고, 컨트롤러 구현에서는 해당 매핑/바인딩 어노테이션들을 제거하거나 컨트롤러 레벨 검증( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
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/org/sopt/snappinserver/api/v1/review/controller/ReviewApi.java (1)
40-41:⚠️ Potential issue | 🟡 Minor
@PathVariable파라미터에@Schema대신@Parameter사용을 권장합니다.팀 컨벤션에 따르면
PathVariable,RequestParam,RequestHeader등 오퍼레이션 파라미터에는@Parameter를, RequestBody DTO 필드에는@Schema를 사용해야 합니다. 현재reviewId는 PathVariable이므로@Schema대신@Parameter가 적합합니다.@Parameter는 이미 4번 라인에 import되어 있어 별도 추가 없이 바로 사용 가능합니다.💡 수정 제안
- `@Schema`(description = "리뷰 ID") - `@PathVariable` `@NotNull` Long reviewId + `@Parameter`(description = "리뷰 ID") + `@PathVariable` `@NotNull` Long reviewIdBased on learnings: "annotate controller method parameters (e.g., RequestParam, PathVariable, RequestHeader) with
Parameter, and annotate the fields of RequestBody DTOs withSchema."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/sopt/snappinserver/api/v1/review/controller/ReviewApi.java` around lines 40 - 41, The method parameter reviewId in ReviewApi is a PathVariable annotated with `@Schema` but per team convention operation parameters should use `@Parameter`; replace the `@Schema` annotation on the `@PathVariable` Long reviewId with `@Parameter` (no import needed since `@Parameter` is already imported) so the PathVariable parameter follows the convention for operation parameters.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@src/main/java/org/sopt/snappinserver/api/v1/review/controller/ReviewApi.java`:
- Around line 40-41: The method parameter reviewId in ReviewApi is a
PathVariable annotated with `@Schema` but per team convention operation parameters
should use `@Parameter`; replace the `@Schema` annotation on the `@PathVariable` Long
reviewId with `@Parameter` (no import needed since `@Parameter` is already imported)
so the PathVariable parameter follows the convention for operation parameters.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/org/sopt/snappinserver/api/v1/review/controller/ReviewApi.java`:
- Around line 39-40: The PathVariable parameter reviewId in ReviewApi should
validate that it is a positive DB identifier and provide an example for Swagger:
add the javax.validation.constraints.@Positive annotation alongside `@NotNull` on
the reviewId parameter and augment the `@Parameter`(...) declaration to include an
example (e.g., example = "1") so the controller method signature (the reviewId
parameter in ReviewApi) both enforces >0 at the API layer and improves OpenAPI
docs.
| @Parameter(description = "리뷰 ID") | ||
| @PathVariable @NotNull Long reviewId |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
@PathVariable 파라미터에 @Positive 추가를 고려해보세요.
@Schema → @Parameter 변경은 PathVariable 파라미터 문서화 컨벤션(PR #250 학습 내용)에 잘 맞습니다.
다만, reviewId는 DB 식별자이므로 @NotNull과 함께 @Positive를 추가하면 음수나 0이 입력되는 경우를 API 계층에서 차단할 수 있습니다. 프로젝트 내 다른 PathVariable 패턴(PR #80)에서도 @NotNull, @Positive를 함께 사용하는 것이 확인됩니다.
추가로, @Parameter(description = "리뷰 ID")에 example 속성을 함께 작성하면 Swagger 문서 품질이 향상됩니다.
✨ 제안 수정 사항
- `@Parameter`(description = "리뷰 ID")
- `@PathVariable` `@NotNull` Long reviewId
+ `@Parameter`(description = "리뷰 ID", example = "1", required = true)
+ `@PathVariable` `@NotNull` `@Positive` Long reviewIdBased on learnings: "define API-facing interfaces for controllers that declare PathVariable, NotNull, Positive, etc." (PR #80) and "leverage description, required, and example attributes where appropriate" (PR #250).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/org/sopt/snappinserver/api/v1/review/controller/ReviewApi.java`
around lines 39 - 40, The PathVariable parameter reviewId in ReviewApi should
validate that it is a positive DB identifier and provide an example for Swagger:
add the javax.validation.constraints.@Positive annotation alongside `@NotNull` on
the reviewId parameter and augment the `@Parameter`(...) declaration to include an
example (e.g., example = "1") so the controller method signature (the reviewId
parameter in ReviewApi) both enforces >0 at the API layer and improves OpenAPI
docs.
2 participants
👀 Summary
🖇️ Tasks
🔍 To Reviewer
#241 에서 정리한 코드 컨벤션을 바탕으로 어노테이션을 정리했으며, 기능 상의 변경사항은 없습니다.