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.

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

#248 Article에 대한 Comment의 종속성 제거하기 #252

Merged
merged 13 commits into from
Nov 5, 2020

Conversation

icyMojito
Copy link
Collaborator

Resolved #248

+AnalysisController와 Service에 대한 리팩토링, 그에 따른 테스트도 변경하여 추가하였습니다!

icyMojito and others added 12 commits November 4, 2020 17:02
- Comment 인수테스트 변경
- 조회에 대한 Controller 로직 구현 및 테스트 추가
- CommentRepository에 findAllByArticleId 로직 구현
- CommentService에 getComments 로직 구현
- CommentServiceTest에 GET 요청에 대한 테스트 작성
- CommentRepositoryTest에, CommentRepository의 findAllByArticleId에 대한 테스트를 추가하였음
- Article과 Comment의 연관관계를 단방향으로 재설계
- ArticleResponse에서 CommentResponse 분리
- Article 관련 코드나 테스트에서 Comment 관련 로직 제거
- CommentDocumentation에 GET 요청에 대한 문서화 코드를 작성
- ArticleDocumentation의 포맷 일부 수정
- 게시물을 표시할 때, 댓글의 개수가 필요함. 그래서 Article과 Comment의 양방향 관계를 살리고, ArticleResponse에 댓글의 개수를 담아주도록 로직 추가함
- Comments.vue에서 comment에 대한 GET 요청을 보내도록 로직을 수정함
- Comments.vue에서 props 대신 vuex를 활용하도록 로직 수정함. 또한, props로 article의 정보를 보내는 로직 모두 삭제
- ArticleResponse 변경에 따라 Article 문서화 테스트도 수정
- CommentService에 countComments 추가
- AnalysisController의 구조를 리팩토링함, DTO 생성 등을 Service에 위임함
- AnalysisService를 리팩토링함. 일부 메서드를 private로 변경함
- AnalysisController에 대한 테스트 코드를 구현
- AnalysisDocumentation에 AnalysisController의 변경사항을 반영
Copy link
Collaborator

@hwanghe159 hwanghe159 left a comment

Choose a reason for hiding this comment

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

양방향 -> 단방향으로의 전환에다가
props -> vuex으로의 전환 둘다 성공한 쿨라임, 카프카 너무 고생많았습니다!!!
코드보면서 너무 시원했네요 ㅋㅋㅋ 바로 어푸르브 하렉요!


ArticlesAnalysisResponse articlesAnalysisResponse = new ArticlesAnalysisResponse(articleEmotionsCount,
mostEmotionId);
ArticlesAnalysisResponse articlesAnalysisResponse = analysisService.getArticlesAnalysis(member);
Copy link
Collaborator

Choose a reason for hiding this comment

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

음 깔끔 그자체 (만족)

Comment on lines -41 to -42
@OneToMany(mappedBy = "article")
private List<Comment> comments = new ArrayList<>();
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

@hotheadfactory hotheadfactory left a comment

Choose a reason for hiding this comment

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

안녕하세요 카프카! (+쿨라임^^)
코드가 정말 깔끔해졌네요~~!!
Approve 드릴게요!

@@ -51,13 +51,13 @@
</div>
</div>
<div
v-if="article.isCommentAllowed && article.comments.length > 0"
v-if="article.isCommentAllowed && article.commentsSize > 0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

기존에는 피드에서도 댓글까지 전부 로드해 오는 방식이라 비효율적이었는데, 이렇게 바뀌니 효율성이 증가하는 효과도 있겠군요!

@@ -56,4 +68,8 @@ public void deleteComment(Member member, Long commentId) throws IllegalAccessExc
comment.delete();
commentRepository.save(comment);
}

public Long countComments(Long articleId) {
return commentRepository.countCommentsByArticleId(articleId);
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.

저는 지워진 댓글이라도 표시해주는게 사용자 입장에서 합리적으로 보일 것이라 생각했어요!!
만약 댓글을 달고 삭제를 했는데, 그 개수가 표시가 안된다면 사용자가 "삭제되어서 개수가 없구나" 보다는 "오류가 났나?" 라고 오해할 수도 있을듯해요.
더군다나 새벽 서비스에서는 삭제된 댓글도 "삭제된 댓글입니다" 라고 나오니까요 😂

Copy link
Collaborator

@chomily chomily left a comment

Choose a reason for hiding this comment

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

Article과 Comment를 분리해주셨군요 💯 👍
성능도 개선되고 코드도 좋아졌네요!
생각보다 할 일이 많았던 것 같은데, 고맙습니다!ㅎㅎㅎ

Comment on lines +78 to +85
async [FETCH_COMMENTS]({ commit }, articleId) {
return CommentService.get(articleId)
.then(({ data }) => {
commit(SET_COMMENTS, data);
return data;
})
.catch(error => commit(CATCH_ERROR, error));
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

분리 굿굿굿 👍

@chomily chomily merged commit b9b5180 into develop Nov 5, 2020
@chomily chomily deleted the refactor/248-separate-comment-from-article branch November 5, 2020 09:24
@include42 include42 mentioned this pull request Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants