Skip to content
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.

Sign up for GitHub

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

refactor: 댓글 삭제 로직 변경 #694

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

refactor: 댓글 삭제 로직 변경 #694

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f939d51
refactor: 댓글 삭제 로직 변경
joelonsw Nov 9, 2021
3418885
refactor(User): comments cascade 옵션 변경
joelonsw Nov 9, 2021
da07907
refactor(Feed): orphanRemoval 코드 삭제
joelonsw Nov 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions backend/src/main/java/com/wooteco/nolto/admin/application/AdminService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,7 @@ public void deleteComment(User adminUser, Long commentId) {
if (findComment.isParentComment()) {
applicationEventPublisher.publishEvent(new NotificationCommentDeleteEvent(findComment));
}
User author = findComment.getAuthor();
author.deleteComment(findComment);
commentRepository.delete(findComment);
}

private Comment getComment(Long commentId) {
Expand Down
2 changes: 1 addition & 1 deletion backend/src/main/java/com/wooteco/nolto/feed/application/CommentService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public void deleteComment(User user, Long commentId) {
if (findComment.isParentComment()) {
applicationEventPublisher.publishEvent(new NotificationCommentDeleteEvent(findComment));
}
user.deleteComment(findComment);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User.deleteComment() 해당 메서드는 이제 사용하는 곳이 없으니 제거되면 좋겠네요 !! ㅎㅎ
(전반적으로 User 안에 쓰이지 않고 있는 메서드는 제거되어도 좋을 것 같아요 ㅎㅎㅎ)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

옷 이거 제거된 거 일거에용!! ㅎㅎ

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아아 User 클래스 안에는 남아있귈랭!! ㅎㅎ

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

남아있는 메서드 말씀하시는 거 였군여~~ 제삼다 ㅎㅎ

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반영했습니다~

commentRepository.delete(findComment);
}

public CommentResponse createReply(User user, Long feedId, Long commentId, CommentRequest request) {
Expand Down
4 changes: 2 additions & 2 deletions backend/src/main/java/com/wooteco/nolto/feed/domain/Comment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ public class Comment extends BaseEntity {
@JoinColumn(name = "parent_id")
private Comment parentComment;

@OneToMany(mappedBy = "comment", cascade = CascadeType.REMOVE, orphanRemoval = true)
@OneToMany(mappedBy = "comment", cascade = CascadeType.REMOVE)
private List<CommentLike> likes = new ArrayList<>();

@OneToMany(mappedBy = "parentComment", cascade = CascadeType.REMOVE, orphanRemoval = true)
@OneToMany(mappedBy = "parentComment", cascade = CascadeType.REMOVE)
private List<Comment> replies = new ArrayList<>();

public Comment(String content, boolean helper) {
Expand Down
4 changes: 2 additions & 2 deletions backend/src/main/java/com/wooteco/nolto/feed/domain/Feed.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ public class Feed extends BaseEntity {
@OneToMany(mappedBy = "feed", cascade = CascadeType.REMOVE)
private List<Like> likes = new ArrayList<>();

@OneToMany(mappedBy = "feed", cascade = CascadeType.ALL, orphanRemoval = true)
@OneToMany(mappedBy = "feed", cascade = CascadeType.ALL)
private List<FeedTech> feedTechs = new ArrayList<>();

@OneToMany(mappedBy = "feed", cascade = CascadeType.REMOVE, orphanRemoval = true)
@OneToMany(mappedBy = "feed", cascade = CascadeType.REMOVE)
private List<Comment> comments = new ArrayList<>();

@Builder
Expand Down
8 changes: 2 additions & 6 deletions backend/src/main/java/com/wooteco/nolto/user/domain/User.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ public class User extends BaseEntity {
@OneToMany(mappedBy = "user", cascade = CascadeType.ALL)
private List<Like> likes = new ArrayList<>();

@OneToMany(mappedBy = "author", cascade = CascadeType.ALL, orphanRemoval = true)
@OneToMany(mappedBy = "author", cascade = CascadeType.REMOVE)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 이제 자식과의 관계를 끊어서 삭제하는 로직이 없으니 현재로써는 orphanRemoval 설정이 불필요하군요 !!!

서비스의 역할에서 보면 영속화 된 데이터를 지운다라는 측면에서 봐야할거같아요.
영속화된 데이터를 지우는데 orphanRemoval 설정에 의존적인 느낌이라 좋지 않아 보이네요

코멘트를 읽다가 찰리의 이 말도 꽤 인상 깊었습니다 ㅎㅎ
음 이번에 조엘 pr로 삭제 로직을 부모 객체와의 연관관계를 통해서가 아닌 대상을 직접 삭제하기로 하고
여기서 불필요한 orphanRemoval 설정을 제거해주었다면,
Feed에 있는 orphanRemoval 설정도 제거해줘서 통일성을 가져가도 좋을 것 같다는 생각이 드네요 !!
(보니까 실제로 해당 옵션이 아무런 작용도 하지 않고 있었네요 😅)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요것도 반영했습니다~

private List<Comment> comments = new ArrayList<>();

@OneToMany(mappedBy = "user", cascade = CascadeType.REMOVE, orphanRemoval = true)
@OneToMany(mappedBy = "user", cascade = CascadeType.REMOVE)
private List<CommentLike> commentLikes = new ArrayList<>();

public User(String socialId, SocialType socialType, String nickName, String imageUrl) {
Expand Down Expand Up @@ -153,10 +153,6 @@ public void addComment(Comment comment) {
this.comments.add(comment);
}

public void deleteComment(Comment comment) {
this.comments.remove(comment);
}

public void validateAdmin() {
if (!this.isAdmin()) {
throw new UnauthorizedException(ErrorType.ADMIN_ONLY);
Expand Down
18 changes: 10 additions & 8 deletions backend/src/test/java/com/wooteco/nolto/admin/application/AdminServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,23 +249,25 @@ void findAllComments() {
void deleteComment() {
//given
Feed 피드1 = 진행중_단계의_피드_생성("피드1", "피드1").writtenBy(조엘);
feedRepository.saveAndFlush(피드1);
feedRepository.save(피드1);
Feed 피드2 = 진행중_단계의_피드_생성("피드2", "피드2").writtenBy(조엘);
feedRepository.saveAndFlush(피드2);
feedRepository.save(피드2);
Feed 피드3 = 진행중_단계의_피드_생성("피드3", "피드3").writtenBy(조엘);
feedRepository.saveAndFlush(피드3);
feedRepository.save(피드3);

final Comment 피드1_댓글1 = new Comment("피드1_댓글1", false).writtenBy(찰리, 피드1);
commentRepository.saveAndFlush(피드1_댓글1);
commentRepository.save(피드1_댓글1);
final Comment 피드1_댓글2 = new Comment("피드1_댓글2", false).writtenBy(찰리, 피드1);
commentRepository.saveAndFlush(피드1_댓글2);
commentRepository.save(피드1_댓글2);
final Comment 피드1_댓글3 = new Comment("피드1_댓글3", false).writtenBy(찰리, 피드1);
commentRepository.saveAndFlush(피드1_댓글3);
commentRepository.save(피드1_댓글3);

final Comment 피드2_댓글1 = new Comment("피드2_댓글1", false).writtenBy(찰리, 피드2);
commentRepository.saveAndFlush(피드2_댓글1);
commentRepository.save(피드2_댓글1);
final Comment 피드2_댓글2 = new Comment("피드2_댓글2", false).writtenBy(찰리, 피드2);
commentRepository.saveAndFlush(피드2_댓글2);
commentRepository.save(피드2_댓글2);
em.flush();
em.clear();

//when
adminService.deleteComment(User.ADMIN_USER, 피드1_댓글1.getId());
Expand Down
2 changes: 2 additions & 0 deletions backend/src/test/java/com/wooteco/nolto/feed/application/CommentServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ void deleteReply() {
assertThat(구글_유저.getComments().size()).isOne();
assertThat(아마찌.getComments().size()).isOne();
em.flush();
em.clear();

// when
Comment 아마찌_대댓글 = commentService.findEntityById(아마찌_대댓글_생성_응답.getId());
Expand Down Expand Up @@ -333,6 +334,7 @@ void deleteCommentExistReply() {
commentLikeService.addCommentLike(포모_댓글_생성_응답.getId(), 구글_유저);
commentLikeService.addCommentLike(아마찌_대댓글_생성_응답.getId(), 아마찌);
em.flush();
em.clear();

// when
Comment 포모_댓글 = commentService.findEntityById(포모_댓글_생성_응답.getId());
Expand Down