-
Notifications
You must be signed in to change notification settings - Fork 7
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
좋아요 swagger 문서화 제공 #675
좋아요 swagger 문서화 제공 #675
Conversation
backend/src/main/java/codezap/template/dto/response/FindAllTemplateItemResponse.java
Outdated
Show resolved
Hide resolved
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.
안녕하세요 짱수
문서화 수고하셨어요 👍
코멘트 남겨보았으니 확인 부탁드려요!
같이 얘기 편하게 나눠봅시당 🔥
@@ -0,0 +1,22 @@ | |||
package codezap.template.controller; |
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.
⚡️ Like 라는 도메인을 별도로 분리해보면 어떨까요?
지금 우리 서비스는 API가 있냐를 기준으로, 패키지를 분리가 되어있는 것 같아요.
카테고리, 태그도 template 하위가 아닌 하나의 독립된 도메인으로 보고 있는 것 처럼요!
또 개인적으로 분리를 했을 때의 장점이 더 많다고 생각해요
분리를 하게 되면 Like 관련 로직을 독립적으로 관리할 수 있고, 복잡한 Template가 더 복잡해지는 것을 막을 수도 있겠네요..!
또 현재 서비스에서 큰 장점은 아니지만 댓글처럼 다른 엔티티에 대해서도 좋아요 기능을 확장하기 수월하는 등 확장 가능성이 더 큰 것으로 알고 있어요 🤔
고민해보시고 다른 의견이나 질문이 있다면 편하게 요청주세요!
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.
오, 단순히 좋아요는 템플릿에 의존적이니까... 라고 생각하고 하나의 패키지에 두었는데요!
API 이야기도, 확장에 관련된 의견도 동의합니다!
패키지를 분리해 둘게요!
throw new NotImplementedException(); | ||
} | ||
|
||
@PostMapping("dislike/{templateId}") |
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.
좋아요
라는 리소스를 취소한다는 API이군요!
⚡️ POST는 서버 입장에서 새로운 리소스를 생성한다는 의미로 받아들여지기 때문에, 좋아요를 삭제하는 API는 DELETE는 어떨까 싶네요!
혹시 만약 우리 서비스가 커져서, 템플릿 언따봉(ㅋㅋ) 기능이 생긴다면 어떻게 API를 구현할 수 있을까요?
함께 생각해보면 도움이 될 것 같아요 🔥
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.
언따봉 ㅋㅋ
너무 이해되었습니다~!!
몰리 체고~~
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.
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.
고생 많았어요 짱수~~👍
몰리 말처럼 도메인 분리가 더 좋다고 생각해 RC 남겨요~
@@ -23,13 +23,17 @@ public record FindAllTemplateItemResponse( | |||
List<FindTagResponse> tags, | |||
@Schema(description = "썸네일") | |||
FindThumbnailResponse thumbnail, | |||
@Schema(description = "좋아요 수", example = "134") | |||
Long likeCount, | |||
@Schema(description = "조회 회원의 좋아요 여부", example = "true") |
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.
LikeController 명세에서는 멤버
라고 했는데 여기서는 회원
이라고 칭하고 있어요.
하나로 통일하는 건 어떨까요?
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.
오, 문서를 작성하면서는 이런 부분이 잘 안보이는군요 ㅠㅠ
바~로 수정해 두겠습니닷!
@Operation(summary = "좋아요", description = "템플릿을 좋아요 합니다.") | ||
@ApiResponse(responseCode = "200", description = "좋아요 성공") | ||
@ApiErrorResponse(status = HttpStatus.BAD_REQUEST, instance = "/like/1", errorCases = { | ||
@ErrorCase(description = "템플릿 Id 에 해당하는 템플릿이 존재하지 않는 경우", exampleMessage = "식별자 1에 해당하는 템플릿이 존재하지 않습니다.") |
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.
둘 중 무엇이든 ID
와 식별자
중 하나로 통일하는 건 어떨까요?
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.
흠 이부분은 "식별자 XX 에 해당하는 YY 가 존재하지 않습니다." 가 공통적으로 사용되는 에러 메시지 인 것 같아서 차용하였는데요.
그렇다고 템플릿 식별자에 해당하는 템플릿이 존재하지 않는 경우 라고 적으면 지금보다 잘 이해가 될까 라는 생각이 들기도 해요.
zeus~ 는 어떻게 생각하시나요?!
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.
이미 해당 표현이 다른 곳에서도 사용되고 있어서 고민이 좀 되겠는걸요 ㅎㅎ...
개인적으로는 모든 곳에서 ID로 통일하면 좋겠어요~!
@kyum-q |
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.
굿 ! 짱 수 정 말 최 고 에 요
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.
피드백이 많이 달려서 추가할만한 건 없는 것 같네요.
고생하셨습니다!!
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 #668
📍주요 변경 사항
🎸기타
@vi-wolhwa
코드 읽을만하면 미리 문서 확인 후 변경이 필요한 부분을 공유해 주시면 더 좋아요~~