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

[Fix] 댓글 개수 조회 차단된 유저의 댓글 및 답글 제외 #304

Merged
merged 4 commits into from
May 1, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ public List<ReplyResponse> getReplies(Long parentId, CursorPageable<Long> cPage)
}

public InsightCommentCountResponse getCommentCount(Long insightId) {
return commentAssembler.toInsightCommentCountResponse(commentDomainService.countByInsightId(insightId));
blockFilterUtil.filterInsightWriter(insightId);
Copy link
Collaborator

Choose a reason for hiding this comment

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

checkBlocking으로 메소드 명 수정하는건 어때요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

바꾸면 checkBlockingInsightWriter 말씀하신게 맞을까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

아뇨 그냥 간단하게 checkBlocking ㅎ

이유는 이게 필터링 역할 같지는 않아서요 -> 제가 생각한 필터링 역할은 리스트에서 항목을 걸러내는 것을 생각했어요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

차단 관련된 기능이 여러개(insight 작성자, user의 id, 응답에서 제거) 있어서 저거 하나만 checkBlocking로 바꾸기는 어려울 것 같아요.
필터링 키워드를 바꾼다면, 위에서 말씀드린 checkBlockingInsightWriter처럼 적용 대상에 대한 정보가 필요해요.

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.

수정했어요. 한번 확인 부탁드려요 ㅎㅎ

Long userId = SecurityUtil.getUserId();
return commentAssembler.toInsightCommentCountResponse(commentDomainService.countByInsightId(insightId, userId));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public InsightStatisticsResponse getStatistics(Long insightId) {
insightQueryDomainService.validateWriter(SecurityUtil.getUserId(), insightId);
Long viewCount = insightQueryDomainService.getViewCount(insightId);
Long reactionCount = reactionDomainService.getCurrentReactionAggregation(insightId).getAllReactionCount();
Long commentCount = commentDomainService.countByInsightId(insightId);
Long commentCount = commentDomainService.countByInsightId(insightId, SecurityUtil.getUserId());
Long bookmarkCount = bookmarkQueryDomainService.countBookmarkByInsightId(insightId);
Long shareCount = 0L; // FIXME 공유 카운팅 추가 시 변경 필요
return insightAssembler.toInsightStatisticsResponse(viewCount, reactionCount, commentCount,bookmarkCount, shareCount);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Repository;

import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Optional;
Expand Down Expand Up @@ -152,4 +153,31 @@ private JPQLQuery<Long> findFirstReplyIds(List<Comment> parents) {
.groupBy(comment.parent.id)
.where(comment.parent.in(parents));
}

public List<Long> findIdsByUserIds(Long insightId, Collection<Long> blockedUserIds) {
return queryFactory
.select(comment.id)
.from(comment)
.where(comment.insight.id.eq(insightId))
.where(comment.writer.id.in(blockedUserIds))
.fetch();
}

public Long countByParentIds(List<Long> blockedUserCommentIds) {
return queryFactory
.select(comment.count())
.from(comment)
.where(comment.parent.id.in(blockedUserCommentIds))
.fetchFirst();
}

public Long countRepliesByUserIds(Long insightId, Collection<Long> blockedUserIds) {
return queryFactory
.select(comment.count())
.from(comment)
.where(comment.insight.id.eq(insightId))
.where(comment.parent.id.isNotNull())
.where(comment.writer.id.in(blockedUserIds))
.fetchFirst();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return queryFactory
.select(comment.id)
.from(comment)
.where(comment.insight.id.eq(insightId))
.where(comment.writer.id.in(blockedUserIds))
.fetch();
}
public Long countByParentIds(List<Long> blockedUserCommentIds) {
return queryFactory
.select(comment.count())
.from(comment)
.where(comment.parent.id.in(blockedUserCommentIds))
.fetchFirst();
}
public Long countRepliesByUserIds(Long insightId, Collection<Long> blockedUserIds) {
return queryFactory
.select(comment.count())
.from(comment)
.where(comment.insight.id.eq(insightId))
.where(comment.parent.id.isNotNull())
.where(comment.writer.id.in(blockedUserIds))
.fetchFirst();
return queryFactory.select(comment.id)
.from(comment)
.where(comment.insight.id.eq(insightId))
.where(comment.writer.id.in(blockedUserIds))
.fetch();
}
public Long countByParentIds(List<Long> blockedUserCommentIds) {
return queryFactory.select(comment.count())
.from(comment)
.where(comment.parent.id.in(blockedUserCommentIds))
.fetchFirst();
}
public Long countRepliesByUserIds(Long insightId, Collection<Long> blockedUserIds) {
return queryFactory.select(comment.count())
.from(comment)
.where(comment.insight.id.eq(insightId))
.where(comment.parent.id.isNotNull())
.where(comment.writer.id.in(blockedUserIds))
.fetchFirst();

Copy link
Collaborator Author

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

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.

네네. 맞추는게 좋아보여요. 우선 저 파일은 전부 수정할게요 ㅎㅎ

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@
import ccc.keewedomain.persistence.repository.utils.CursorPageable;
import ccc.keewedomain.service.insight.query.InsightQueryDomainService;
import ccc.keewedomain.service.user.UserDomainService;
import ccc.keewedomain.service.user.query.ProfileQueryDomainService;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;

import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;

@Service
@RequiredArgsConstructor
Expand All @@ -28,6 +30,7 @@ public class CommentDomainService {
private final CommentQueryRepository commentQueryRepository;
private final InsightQueryDomainService insightQueryDomainService;
private final UserDomainService userDomainService;
private final ProfileQueryDomainService profileQueryDomainService;

public Comment create(CommentCreateDto dto) {
Insight insight = insightQueryDomainService.getByIdOrElseThrow(dto.getInsightId());
Expand Down Expand Up @@ -68,8 +71,20 @@ private void validateHasNoParent(Comment comment) {
}
}

public Long countByInsightId(Long insightId) {
return commentQueryRepository.countByInsightId(insightId);
public Long countByInsightId(Long insightId, Long userId) {
Long commentCount = commentQueryRepository.countByInsightId(insightId);
Set<Long> blockedUserIds = profileQueryDomainService.findBlockedUserIds(userId);
if(blockedUserIds.isEmpty()) {
return commentCount;
}
// 1. 차단한 유저의 댓글 제외 - 차단한 유저들이 작성한 댓글의 id 조회
List<Long> blockedUserCommentIds = commentQueryRepository.findIdsByUserIds(insightId, blockedUserIds);
// 2. 차단한 유저의 댓글에 달린 답글 제외 - 1에서 조회한 id가 parent인 댓글 개수 조회
Long replyOnBlockedCommentCount = commentQueryRepository.countByParentIds(blockedUserCommentIds);
// 3. 차단한 유저의 답글 수 조회 - 1, 2에서 중복으로 제거되는 개수 보완 목적
Long dupBlockedReplyCount = commentQueryRepository.countRepliesByUserIds(insightId, blockedUserIds);
// 4. 전체 개수에서 빼기
return commentCount - blockedUserCommentIds.size() - replyOnBlockedCommentCount + dupBlockedReplyCount;
Comment on lines +74 to +87
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

Comment를 사용해서 필터링 했으면 더 간단했을거같네요 ㅋㅋ
성능을 생각하셔서 쿼리에 많이 의존적인거같은데, 복잡한 요구사항이 들어올수록 뭔가 경직돼있음을 느끼셨을거같아요.

고생하셨어요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

맞는 말이네요 ㅎㅎ
댓글 조회도 pagination으로 조회하는데 여기서 전체를 조회하기에는 부담스럽더라구요 ㅠ

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

}

public Map<Long, Long> getReplyNumbers(List<Comment> parents) {
Expand Down