-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Feature] 이동봉사자 봉사 후기 등록 API 구현 #44
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
가독성 좋은 코드 👍 수고하셨습니다!
@RequestPart(name = "files", required = false) List<MultipartFile> files) { | ||
reviewService.createReview(loginUser.getUsername(), postId, request, files); | ||
return ResponseEntity.noContent().build(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uri는 일관성을 유지해야 할 것 같아요!
근황에 대해서도 /volunteers/dogStatus/{postId} 와 같은 형식으로 되어야 할 것 같다고 생각됩니다!
/volunteers/reviews/{postId}는 간결하다는 장점이 있고, 계층 구조가 명확하지 않다는 단점이 있습니다!
/volunteers/posts/{postId}/reviews는 계층 구조가 명확하다는 장점이 있고, 간결하지 않다는 단점이 있습니다!
계층 구조를 생각했을 때 depth가 공고 -> 후기 라면 /volunteers/posts/{postId}/reviews이 직관적으로 알 수 있다고 생각되는데! 개발자들끼리 공유하는 주소이기 때문에 직관적이면 좋지 않을까!? 하고 저는 생각했습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
생각해 보니 동의합니다! 이렇게 직관적인 계층 구조를 사용해 본 적이 없었는데, 서비스 특성 상 계층 구조를 명확히 해서 리소스 간의 관계를 나타내는 게 좋을 것 같네요! 이해하기도 편하고~
import jakarta.validation.constraints.NotBlank; | ||
|
||
public record ReviewCreateRequest(@NotBlank(message = "후기 내용은 필수 입력 값입니다.") | ||
String content) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ Size가 문자열, 리스트, 컬렉션의 크기 제한에 쓴다고 나와 있어서 전에 닉네임 글자 수 제한에 사용했는데 잘 동작했습니다! 지금 이동봉사자 자체 회원가입 API에서 검증이 되는 것을 볼 수 있는데 한 번 확인해 주실 수 있나요 ~~!
그리고 DB, UI 상에서 막혀 있더라도 Validation은 추가해 주는 게 좋다고 생각합니다! 이중 검증!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이중 검증 좋습니다!
문자열에 제대로 validation을 걸어주지 않았던 걸까요, 리뷰 보고 반영해 보니 넘 잘 돌아가네요!~!
다음부턴 더 꼼꼼히 찾아봐야겠어요!
감사합니다👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
리뷰 반영하는 속도 짱 . .
💡 연관된 이슈
close #41
📝 작업 내용
💬 리뷰 요구 사항
/volunteers/reviews/{postId}
로 설정했습니다./volunteers/reviews
뒤에 postId, reviewId 두 개가 각각 오더라도 POST, GET으로 구분하면 되지 않을까 하는데 다른 의견 있으시다면 듣고 싶어요.