Feature/#83 댓글 대댓글 crud api 구현 #97

java-saeng · 2023-07-25T15:10:31Z

#️⃣연관된 이슈

댓글 및 대댓글 CRUD API 구현 #83

📝작업 내용

댓글 관련 CRUD API 를 구현해습니다.

예상 소요 시간 및 실제 소요 시간

예상 시간 : 수요일 오후 6시
실제 소요 시간 : 수요일 오전 12시

💬리뷰 요구사항

코드 잘 읽히시는지, 제대로 작성된 것 같은지 확인 부탁드립니다.

#83

…터리 메서드 만듦 #83

…023-emmsale into Feature/#83-댓글_대댓글_CRUD_API_구현

#83

- 테스트에서 createdAt, updatedAt을 등록하지 못하기 때문에 null 제약조건 - createdAt, updatedAt 을 nullable 하는 것으로 수정 #83

#83

hong-sile

전체적으로 통일성이 있어서 확인하긴 편했습니다.

몇몇 커멘트 남겼으니 확인해주세요.

hong-sile · 2023-07-26T00:59:26Z

backend/emm-sale/src/main/java/com/emmsale/comment/application/CommentCommandService.java

+      final Event savedEvent
+  ) {
+    final Comment savedComment = commentRepository.save(
+        new Comment(savedEvent, null, member, commentAddRequest.getContent())


요건 취향이긴 한데, 한번 정적팩터리 메서드를 정의해서
RootComment를 만들 때랑 ChildComment 만들 때랑 다른 메서드를 사용하게 하는 것은 어떨까요?

현장 리뷰 완료요~~

hong-sile · 2023-07-26T01:02:25Z

backend/emm-sale/src/main/java/com/emmsale/comment/api/CommentApi.java

+    commentCommandService.delete(commentId, member);
+  }
+
+  @PatchMapping("/comments/{comment-id}")


comment 내용을 완전히 덮어씌우는 거라면
Put 은 어떤가요?

현장 리뷰 완료요~~

hong-sile · 2023-07-26T04:28:13Z

backend/emm-sale/src/test/java/com/emmsale/comment/application/CommentCommandServiceTest.java

+  @DisplayName("delete() : 댓글을 삭제할 때, 로그인 한 본인의 댓글이 아니면 CAN_NOT_DELETE_COMMENT 가 발생합니다.")
+  void test_delete_canNotDeleteComment() throws Exception {
+    //given
+    final Member 댓글_작성자 = memberRepository.findById(1L).get();


댓글 작성자를 모든 테스트에서 쓴다면 필드 변수로 빼보는 건 어떨까요?

현장 리뷰 완료요~~

hong-sile · 2023-07-26T04:31:48Z

backend/emm-sale/src/test/java/com/emmsale/comment/domain/CommentRepositoryTest.java

+        new Comment(event2, null, member, "부모댓글1")
+    );
+
+    final long eventId = 2L;


나중에 event 관련된 테스트가 생기면 auto_increment 때문에 터질 것 같아요.
event의 getId로 eventId를 세팅하면 어떨까요?

현장 리뷰 완료요~~

hyeonjerry

수고하셨습니다!
크게 수정할 점은 보이지 않습니다.

hyeonjerry · 2023-07-26T02:34:06Z

backend/emm-sale/src/main/java/com/emmsale/comment/application/CommentCommandService.java

+@Service
+@RequiredArgsConstructor
+@Transactional
+public class CommentCommandService {


혹시 Command가 나타내는게 Query 이외의 모든 동작인가요? 지금은 Query와 Command밖에 없어서 쉽게 예측이 되지만 더 늘어난다면 Command에 어떤 동작이 있는지 예측하기 어려울 것 같아요.
혹시 CommentWriteService, CommentDeleteService 이런 식으로 나누는건 어떠신가요?

현장 리뷰 완료요~~

hyeonjerry · 2023-07-26T02:43:22Z

backend/emm-sale/src/main/java/com/emmsale/comment/api/CommentApi.java

+
+  @DeleteMapping("/comments/{comment-id}")
+  @ResponseStatus(HttpStatus.NO_CONTENT)
+  public void delete(@PathVariable("comment-id") final Long commentId, final Member member) {


이건 개인적으로 궁금해서 여쭤보는건데 혹시 comment-id로 받으시는 이유가 있으신가요?
commentId로 받으면 PathVariable에 name을 생략할 수 있는데 굳이 comment-id로 받아서 name을 명시한 이유가 있는지 궁금합니다.

현장 리뷰 완료요~~

hyeonjerry · 2023-07-26T04:09:40Z

backend/emm-sale/src/main/java/com/emmsale/comment/application/dto/CommentModifyRequest.java

+
+  private final String content;
+
+  private CommentModifyRequest() {


혹시 이게 왜 필요한지 알 수 있을까요?
없으면 테스트가 터지는데 다른 Request DTO들은 없어도 되는데 이 친구만 특별하게 있어야 하는 이유가 궁금합니다!

현장 리뷰 완료요~~

hyeonjerry · 2023-07-26T04:11:58Z

backend/emm-sale/src/main/java/com/emmsale/comment/application/dto/CommentResponse.java

+@Getter
+public class CommentResponse {
+
+  private static final String DELETE_COMMENT_CONTENT = "삭제된 댓글입니다.";


이 상수가 사용되고 있는 곳을 찾을 수 없는데 혹시 어디서 사용하는 건가요?
isDeleted를 통해 true일 경우 이 값으로 대체하는 로직이 빠진걸까요?

hyeonjerry · 2023-07-26T04:24:32Z

backend/emm-sale/src/main/java/com/emmsale/comment/exception/CommentExceptionType.java

+      "해당 댓글은 존재하지 않습니다."
+  ),
+
+  CAN_NOT_DELETE_COMMENT(


이건 개인적인 의견인데 갑자기 생각이 나서 코멘트 달아둡니다.
혹시 NOT_FOUND처럼 FORBIDDEN으로 http 상태 메시지로 통일하는건 어떠신가요?

현장 리뷰 완료요~~

hyeonjerry · 2023-07-26T04:31:32Z

backend/emm-sale/src/main/java/com/emmsale/comment/exception/CommentExceptionType.java

+  ),
+
+  CAN_NOT_MODIFY_DELETED_COMMENT(
+      HttpStatus.FORBIDDEN,


이건 인가의 문제가 아니라 클라이언트의 잘못된 요청이므로 BAD_REQUEST가 더 좋을 것 같은데 우르는 어떻게 생각하시나요?

현장 리뷰 완료요~~

hyeonjerry · 2023-07-26T04:33:58Z

backend/emm-sale/src/test/java/com/emmsale/member/domain/MemberActivityRepositoryTest.java

@@ -36,6 +38,8 @@ void findAllByMemberId() throws Exception {
        .map(MemberActivity::getId)
        .collect(toUnmodifiableList());

+    System.out.println("memberCareerIds.size() = " + memberCareerIds.size());


혹시 콘솔 출력을 추가하신 이유가 있으신가요?

현장 리뷰 완료요~~

amaran-th

수고하셨습니다!
코멘트 몇 개 남겼으니 확인해주시면 감사하겠습니다~

amaran-th · 2023-07-26T03:59:37Z

backend/emm-sale/src/main/java/com/emmsale/comment/exception/CommentExceptionType.java

+  CAN_NOT_MODIFY_COMMENT(
+      HttpStatus.FORBIDDEN,
+      "댓글 작성자 외에는 수정할 수 없습니다."
+  ),


개인적으로 CAN_NOT_DELETE_COMMENT, CAN_NOT_MODIFY_COMMENT보다는 INVALID_AUTHORITY가 원인을 파악하기에 좀 더 직관적일 것 같다는 생각이 듭니다!

amaran-th · 2023-07-26T04:03:19Z

backend/emm-sale/src/main/java/com/emmsale/comment/application/CommentCommandService.java

+    final Comment comment = commentRepository.findById(commentId)
+        .orElseThrow(() -> new CommentException(NOT_FOUND_COMMENT));
+
+    validateSameWriter(loginMember, comment, CAN_NOT_DELETE_COMMENT);


혹시 CAN_NOT_DELETE_COMMENT 예외 타입을 validateSameWriter() 메서드에서 직접 사용하지 않고 매개변수로 넘겨주신 이유가 있나요??
조금 불필요한 작업으로 느껴져서요

amaran-th · 2023-07-26T04:28:20Z

backend/emm-sale/src/main/java/com/emmsale/comment/application/dto/CommentResponse.java

+@Getter
+public class CommentResponse {
+
+  private static final String DELETE_COMMENT_CONTENT = "삭제된 댓글입니다.";


이제 사용되고 있지 않은 상수인 것 같으니 지워줘도 될 것 같습니다!

#83

- 2L -> event2.getId()

- parentId가 nullable 하기 때문에 이에 따라서 optional을 사용하여 리팩터링 - 정적 팩터리 메서드 사용으로 생성자 private 로 변경 후 테스트 수정 #83

#83

java-saeng · 2023-07-27T03:46:43Z

현장 리뷰 완료

java-saeng added 17 commits July 24, 2023 21:09

feat : Comment 생성 API 구현

629a086

#83

docs : Comment http 문서 작성

c05f146

#83

refactor : CommentResponse에서 Comment를 파라미터로 받아서 Response 로 만들어주는 정적 팩…

96eb475

…터리 메서드 만듦 #83

Merge branch 'backend-main' of https://github.com/woowacourse-teams/2…

328e140

…023-emmsale into Feature/#83-댓글_대댓글_CRUD_API_구현

refactor : 이전 커밋 pull 하면서 EventRepository 패키지 변경으로 인한 import 변경

164ce3e

#83

refactor : 정적 팩토리 추가로 인해 private 기본 생성자 추가 및 테스트를 위한 모든 파라미터를 가진 생성자 추가

3fcd4b5

#83

feat : 행사에 존재하는 댓글 모두 조회하는 API 구현

9f878ed

#83

fix : @DataJpaTest 관련 테스트가 모두 실패하는 것을 data.sql 을 매 테스트 메서드마다 실행하는 것으로 해결

4816a5a

#83

feat : 문서화 테스트

dcd52a4

#83

feat : http 테스트 추가

3a0042d

#83

fix : AuditingEntityListener 때문에 테스트에서 문제 해결

29b9788

- 테스트에서 createdAt, updatedAt을 등록하지 못하기 때문에 null 제약조건 - createdAt, updatedAt 을 nullable 하는 것으로 수정 #83

feat : Comment 삭제 API 구현

fce3a79

#83

feat : Comment 삭제 API 문서 추가

68c52ed

#83

feat : Comment 삭제 http 테스트 추가

d090c86

#83

feat : 댓글 수정 API 구현

6a316eb

#83

feat : 댓글 수정 API rest docs

4596168

#83

feat : 댓글 수정 API http 테스트 추가

ee38f8a

#83

java-saeng added Backend 백엔드 관련 이슈 기능 추가 새로운 기능 추가 및 기존 기능 변경 labels Jul 25, 2023

java-saeng requested review from hong-sile, hyeonjerry and amaran-th July 25, 2023 15:10

java-saeng self-assigned this Jul 25, 2023

java-saeng changed the base branch from main to backend-main July 25, 2023 15:10

hong-sile requested changes Jul 26, 2023

View reviewed changes

hyeonjerry approved these changes Jul 26, 2023

View reviewed changes

amaran-th requested changes Jul 26, 2023

View reviewed changes

java-saeng added 3 commits July 26, 2023 14:30

feat : 댓글 정보 반환 시 댓글 작성자의 이름, imageUrl, 아이디 반환

df00603

#83

fix : conflict 해결 및 pull backend-main

1684186

fix : conflict 해결 및 pull backend-main

e6a96e5

java-saeng added 5 commits July 26, 2023 22:14

test : 모든 테스트에서 사용하는 변수 인스턴스 필드로 빼기

e8f03fa

test : auto increment 로 인해 테스트 깨질 수 있는 것을 예방하기 위해 생성된 이벤트의 아이디를 사용

404a370

- 2L -> event2.getId()

refactor : FORBIDDEN일 경우 exceptionType 변경

98f0b0d

- 2L -> event2.getId()

refactor : 부모, 자식 Comment 생성 시 정적 팩터리 메서드 사용

c29ed29

- parentId가 nullable 하기 때문에 이에 따라서 optional을 사용하여 리팩터링 - 정적 팩터리 메서드 사용으로 생성자 private 로 변경 후 테스트 수정 #83

refactor : Comment에서 Parent 가 null일 수 있으므로 getter에 Optional 추가

cb1fd05

#83

java-saeng requested a review from hong-sile July 27, 2023 03:46

hong-sile approved these changes Jul 27, 2023

View reviewed changes

amaran-th approved these changes Jul 27, 2023

View reviewed changes

java-saeng merged commit 1d40df8 into backend-main Jul 27, 2023
1 check passed

hyeonjerry deleted the Feature/#83-댓글_대댓글_CRUD_API_구현 branch July 31, 2023 04:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/#83 댓글 대댓글 crud api 구현 #97

Feature/#83 댓글 대댓글 crud api 구현 #97

java-saeng commented Jul 25, 2023

hong-sile left a comment

hong-sile Jul 26, 2023

java-saeng Jul 26, 2023

hong-sile Jul 26, 2023

java-saeng Jul 26, 2023

hong-sile Jul 26, 2023

java-saeng Jul 26, 2023

hong-sile Jul 26, 2023

java-saeng Jul 26, 2023

hyeonjerry left a comment

hyeonjerry Jul 26, 2023

java-saeng Jul 26, 2023

hyeonjerry Jul 26, 2023

java-saeng Jul 26, 2023

hyeonjerry Jul 26, 2023

java-saeng Jul 26, 2023

hyeonjerry Jul 26, 2023

hyeonjerry Jul 26, 2023

java-saeng Jul 26, 2023

hyeonjerry Jul 26, 2023

java-saeng Jul 26, 2023

hyeonjerry Jul 26, 2023

java-saeng Jul 26, 2023

amaran-th left a comment

amaran-th Jul 26, 2023

amaran-th Jul 26, 2023

amaran-th Jul 26, 2023

java-saeng commented Jul 27, 2023


		private final String content;

		private CommentModifyRequest() {

Feature/#83 댓글 대댓글 crud api 구현 #97

Feature/#83 댓글 대댓글 crud api 구현 #97

Conversation

java-saeng commented Jul 25, 2023

#️⃣연관된 이슈

📝작업 내용

예상 소요 시간 및 실제 소요 시간

💬리뷰 요구사항

hong-sile left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hyeonjerry left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amaran-th left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

java-saeng commented Jul 27, 2023