-
Notifications
You must be signed in to change notification settings - Fork 8
refactor: 댓글 정책 변경에 따른 리팩터링 #373
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
Changes from all commits
b6037ab
df10149
32cb72f
3ca8f4e
2938a79
9c59985
541fed7
5cf0660
b739ceb
805ae3d
1a00a77
41afcad
f24495e
2bb6e95
d3c9b0e
8257dfc
cfa18eb
13a4673
68f4bbd
af4ec96
6fcc903
028190a
5f6bcaf
786e0ae
afa0b37
36621bf
cd71d21
acde555
fd85b32
e91c74b
6526fd1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| ALTER TABLE comment | ||
| ADD COLUMN is_deleted BOOLEAN NOT NULL DEFAULT FALSE; |
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 테스트코드 보충이 필요해보여요! 서비스 로직에서 수행하고 있는 것은
이렇게 두가지인데, 첫번째에 대한 테스트만 존재하네요.!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 추가하였습니다! |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ | |
| import com.example.solidconnection.community.post.domain.PostCategory; | ||
| import com.example.solidconnection.community.post.fixture.PostFixture; | ||
| import com.example.solidconnection.siteuser.domain.SiteUser; | ||
| import com.example.solidconnection.siteuser.dto.PostFindSiteUserResponse; | ||
| import com.example.solidconnection.siteuser.fixture.SiteUserFixture; | ||
| import com.example.solidconnection.support.TestContainerSpringBootTest; | ||
| import jakarta.transaction.Transactional; | ||
|
|
@@ -108,6 +109,85 @@ class 댓글_조회_테스트 { | |
| )) | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| void 부모댓글과_대댓글이_모두_삭제되면_응답에서_제외한다() { | ||
| // given | ||
| Comment parentComment = commentFixture.부모_댓글("부모 댓글", post, user1); | ||
| Comment childComment1 = commentFixture.자식_댓글("자식 댓글1", post, user2, parentComment); | ||
| Comment childComment2 = commentFixture.자식_댓글("자식 댓글2", post, user2, parentComment); | ||
|
|
||
| parentComment.deprecateComment(); | ||
| childComment1.deprecateComment(); | ||
| childComment2.deprecateComment(); | ||
| commentRepository.saveAll(List.of(parentComment, childComment1, childComment2)); | ||
|
|
||
| // when | ||
| List<PostFindCommentResponse> responses = commentService.findCommentsByPostId(user1, post.getId()); | ||
|
|
||
| // then | ||
| assertAll( | ||
| () -> assertThat(responses).isEmpty() | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| void 부모댓글이_삭제된_경우에도_자식댓글이_존재하면_자식댓글의_내용만_반환한다() { | ||
| // given | ||
| Comment parentComment = commentFixture.부모_댓글("부모 댓글", post, user1); | ||
| Comment childComment1 = commentFixture.자식_댓글("자식 댓글1", post, user2, parentComment); | ||
| Comment childComment2 = commentFixture.자식_댓글("자식 댓글2", post, user2, parentComment); | ||
|
|
||
| parentComment.deprecateComment(); | ||
| commentRepository.saveAll(List.of(parentComment, childComment1, childComment2)); | ||
|
|
||
| // when | ||
| List<PostFindCommentResponse> responses = commentService.findCommentsByPostId(user1, post.getId()); | ||
|
|
||
| // then | ||
| assertAll( | ||
| () -> assertThat(responses).hasSize(3), | ||
| () -> assertThat(responses) | ||
| .extracting(PostFindCommentResponse::id) | ||
| .containsExactlyInAnyOrder(parentComment.getId(), childComment1.getId(), childComment2.getId()), | ||
| () -> assertThat(responses) | ||
| .filteredOn(response -> response.id().equals(parentComment.getId())) | ||
| .extracting(PostFindCommentResponse::content) | ||
| .containsExactly(""), | ||
| () -> assertThat(responses) | ||
| .filteredOn(response -> !response.id().equals(parentComment.getId())) | ||
| .extracting(PostFindCommentResponse::content) | ||
| .containsExactlyInAnyOrder("자식 댓글1", "자식 댓글2") | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| void 부모댓글이_삭제된_경우_부모댓글의_사용자정보는_null이고_자식댓글의_사용자정보는_정상적으로_반환한다() { | ||
| // given | ||
| Comment parentComment = commentFixture.부모_댓글("부모 댓글", post, user1); | ||
| Comment childComment1 = commentFixture.자식_댓글("자식 댓글1", post, user2, parentComment); | ||
| Comment childComment2 = commentFixture.자식_댓글("자식 댓글2", post, user2, parentComment); | ||
|
|
||
| parentComment.deprecateComment(); | ||
| commentRepository.saveAll(List.of(parentComment, childComment1, childComment2)); | ||
|
|
||
| // when | ||
| List<PostFindCommentResponse> responses = commentService.findCommentsByPostId(user1, post.getId()); | ||
|
|
||
| // then | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 여기 코드검증이 저는 개인적으로 좀 읽기 힘들었는데 이런 느낌으로 좀 나눠가는 건 어떤가요? 반영 안하셔도 괜찮습니다 이건!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저도 좀 필요없는 것까지 있는 것 같아서 영서님께서 제안해주신 것이랑 같이 확인해서 간결하게 수정해보았습니다! |
||
| assertAll( | ||
| () -> assertThat(responses) | ||
| .filteredOn(response -> response.id().equals(parentComment.getId())) | ||
| .extracting(PostFindCommentResponse::postFindSiteUserResponse) | ||
| .containsExactly((PostFindSiteUserResponse) null), | ||
| () -> assertThat(responses) | ||
| .filteredOn(response -> !response.id().equals(parentComment.getId())) | ||
| .extracting(PostFindCommentResponse::postFindSiteUserResponse) | ||
| .isNotNull() | ||
| .extracting(PostFindSiteUserResponse::id) | ||
| .containsExactlyInAnyOrder(user2.getId(), user2.getId()) | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| @Nested | ||
|
|
@@ -281,7 +361,8 @@ class 댓글_삭제_테스트 { | |
| // then | ||
| Comment deletedComment = commentRepository.findById(response.id()).orElseThrow(); | ||
| assertAll( | ||
| () -> assertThat(deletedComment.getContent()).isNull(), | ||
| () -> assertThat(deletedComment.getContent()).isEqualTo("부모 댓글"), | ||
| () -> assertThat(deletedComment.isDeleted()).isTrue(), | ||
| () -> assertThat(deletedComment.getCommentList()) | ||
| .extracting(Comment::getId) | ||
| .containsExactlyInAnyOrder(childComment.getId()), | ||
|
|
@@ -316,27 +397,6 @@ class 댓글_삭제_테스트 { | |
| ); | ||
| } | ||
|
|
||
| @Test | ||
| @Transactional | ||
| void 대댓글을_삭제하고_부모댓글이_삭제된_상태면_부모댓글도_삭제된다() { | ||
| // given | ||
| Comment parentComment = commentFixture.부모_댓글("부모 댓글", post, user1); | ||
| Comment childComment = commentFixture.자식_댓글("자식 댓글", post, user2, parentComment); | ||
| List<Comment> comments = post.getCommentList(); | ||
| int expectedCommentsCount = comments.size() - 2; | ||
| parentComment.deprecateComment(); | ||
|
|
||
| // when | ||
| CommentDeleteResponse response = commentService.deleteCommentById(user2, childComment.getId()); | ||
|
|
||
| // then | ||
| assertAll( | ||
| () -> assertThat(commentRepository.findById(response.id())).isEmpty(), | ||
| () -> assertThat(commentRepository.findById(parentComment.getId())).isEmpty(), | ||
| () -> assertThat(post.getCommentList()).hasSize(expectedCommentsCount) | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| void 다른_사용자의_댓글을_삭제하면_예외_응답을_반환한다() { | ||
| // given | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
🛠️ Refactor suggestion
댓글 필터링 로직 검토 필요⚠️
현재 로직에 몇 가지 문제점이 있습니다:
부모 댓글이 삭제되었지만 자식 중 일부만 삭제된 경우:
결과 리스트 순서:
private List<Comment> filterCommentsByDeletionRules(List<Comment> comments) { Map<Long, List<Comment>> commentsByParent = comments.stream() .filter(comment -> comment.getParentComment() != null) .collect(Collectors.groupingBy(comment -> comment.getParentComment().getId())); List<Comment> result = new ArrayList<>(); List<Comment> parentComments = comments.stream() .filter(comment -> comment.getParentComment() == null) .toList(); for (Comment parent : parentComments) { List<Comment> children = commentsByParent.getOrDefault(parent.getId(), List.of()); boolean allDeleted = parent.isDeleted() && children.stream().allMatch(Comment::isDeleted); if (!allDeleted) { result.add(parent); - result.addAll(children.stream() - .filter(child -> !child.isDeleted()) - .toList()); + result.addAll(children); } } return result; }서비스 레이어가 아닌
PostFindCommentResponse에서 삭제된 댓글의 내용을 숨기는 것이 더 적절할 것 같습니다.🤖 Prompt for AI Agents
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.
@whqtker 테스트 결과를 알려주셔서 감사합니다! 실제 동작을 확인해보신 결과가 이론적 분석보다 훨씬 신뢰할 만하네요.
콘텐츠 노출 문제:
PostFindCommentResponse.from에서getDisplayContent메서드를 통해 삭제된 댓글의 내용을 숨기는 처리가 이미 되어 있군요.순서 보장:
findCommentTreeByPostId가 계층적 순서로 댓글을 반환하고, 현재 필터링 로직이 이를 적절히 유지하고 있는 것 같습니다.실제 테스트를 통해 검증해주셔서 도움이 되었습니다. 코드가 의도한 대로 잘 동작하고 있다니 다행이네요! 🎉