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

[Refactor/#157-comment-repository] 차단 회원의 자식 댓글 조회 막기 #158

Merged
merged 10 commits into from
Feb 9, 2025

Conversation

yeahjinjeong
Copy link
Collaborator

@yeahjinjeong yeahjinjeong commented Dec 29, 2024

📒 개요

차단 회원의 자식 댓글 조회 막기

📍 Issue 번호

🛠️ 작업사항

  • 쿼리
  • 레포 테스트
  • element 수정
  • 서비스 수정
  • 컨트롤러 테스트

🧰 추가 논의사항

  • 자식 댓글을 원래 element에서 페이징 처리를 해줬는데 쿼리로 받아오니까 서비스에서 해줘야할지 ...?
  • 네이티브 쿼리에 최초 조회 건수 하드코딩 해놓음

@yeahjinjeong yeahjinjeong self-assigned this Dec 29, 2024
@yeahjinjeong yeahjinjeong added the 🛠️ enhancement New feature or request label Dec 29, 2024
memberRepository.save(POST_OWNER);
memberRepository.save(PARENT_COMMENTER);
memberRepository.save(CHILD_COMMENTER);
Member loginMember = memberRepository.save(LOGIN_MEMBER);
Copy link
Member

Choose a reason for hiding this comment

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

memberRepository.save(POST_OWNER);
memberRepository.save(PARENT_COMMENTER);
memberRepository.save(CHILD_COMMENTER);
Member loginMember = memberRepository.save(LOGIN_MEMBER);

이 부분 모든 곳에서 겹치니 따로 beforeeach 같은걸로 묶어도 될 것 같아요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아 이게 변수에 저장해서 써야할 것 들이 있어서 beforeeach에서 묶고 find조회해서 쓰는 게 나을까요?

Copy link
Member

Choose a reason for hiding this comment

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

class BlockServiceTest extends ServiceTest {

    // ...

    private Member owner;
    private Member taggedMember1;
    private Member taggedMember2;
    private Member loginMember;

    @BeforeEach
    void setUp() {
        owner = memberRepository.save(OWNER);
        taggedMember1 = memberRepository.save(TAGGED_MEMBER1);
        taggedMember2 = memberRepository.save(TAGGED_MEMBER2);
        loginMember = memberRepository.save(LOGIN_MEMBER);
    }

전 이런식으로 하고 있는데 어떤가요?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이 방식으로 수정했습니다~

memberRepository.deleteAll();
tagRepository.deleteAll();
blockRepository.deleteAll();
}
Copy link
Member

Choose a reason for hiding this comment

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

이거 상단에 @transactional 붙여줘서 필요없지 않나요?

Copy link
Member

Choose a reason for hiding this comment

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

getComments()가 지금 걱정되는데 여기도 부모 댓글 + 그 각각의 자식댓글 조회인데,,
여기서 그 각각의 자식 댓글을 조회할 때 차단 로직도 잘 적용되나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아 getComments()에도 추가해야 할 것 같아요

@Query(value = "SELECT filtered.* " +
"FROM ( " +
" SELECT c.*, " +
" ROW_NUMBER() OVER (PARTITION BY c.parent_comment_id ORDER BY c.created_date ASC) AS row_num " +
Copy link
Member

Choose a reason for hiding this comment

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

우왁,, 첨보는거,,👍👍

@yeahjinjeong yeahjinjeong changed the title [Test/#157-comment-repository] 차단 회원의 자식 댓글 조회 막기 [Refactor/#157-comment-repository] 차단 회원의 자식 댓글 조회 막기 Jan 6, 2025
@yeahjinjeong yeahjinjeong requested a review from 5jisoo January 6, 2025 17:33
Copy link
Member

@5jisoo 5jisoo left a comment

Choose a reason for hiding this comment

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

테스트가

  1. 자식 댓글 조회 - 정상 테스트
  2. 자식 댓글 조회 - 자식 댓글 작성자에게 차단됐을때
  3. 부모 댓글 조회 - 정상 테스트
  4. 부모 댓글 조회 - 부모 댓글 작성자를 차단했을 때
  5. 자식 댓글 조회 - 정상 테스트
  6. 자식 댓글 조회 - 자식 댓글 작성자를 차단했을 때

이렇게같은데
자식 / 부모 댓글 정렬.. 한번만 맞춰주시면서 @DisplayName에 설명 조금만 추가해도 좋을 것 같아요 (조금.. 헷갈림,,ㅎㅎ) -> (아니면 메소드 순서에 다른 뜻이 있었나요?)

@@ -20,12 +22,25 @@ public class CommentElements {
private String loginMemberProfileImage;

public CommentElements(
final Member loginMember, final Page<Comment> parentCommentPage
final Member loginMember, final Page<Comment> parentCommentPage, final List<Comment> childComments
Copy link
Member

Choose a reason for hiding this comment

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

다른 코드와의 통일성을 위해,, 엔터 한번씩 쳐도 좋을 것 같아요ㅎㅎ

final Page<Comment> commentList = commentRepository.findParentCommentByPost(post, loginMember, pageable);
return new CommentElements(loginMember, commentList);
final Page<Comment> parentCommentList = commentRepository.findParentCommentByPost(post, loginMember, pageable);
final List<Comment> childCommentList = commentRepository.findChildCommentByParentComments(parentCommentList.stream().map(Comment::getId).toList(), loginMember.getId());
Copy link
Member

Choose a reason for hiding this comment

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

요기두용 intellij 로 켜서 보니까 너무 긴 것 같기두..?!

for (Comment parentComment : parentCommentPage.getContent()) {
List<Comment> childCommentList = new ArrayList<>();
for (Comment childComment : childComments) {
if (Objects.equals(parentComment.getId(), childComment.getParentComment().getId())) {
Copy link
Member

Choose a reason for hiding this comment

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

오 그냥 equals()랑 뭐가 다른지 찾아보니까 이건 npe가 방지되는군여

}
}
parentCommentList.add(new ParentCommentElement(parentComment, childCommentList));
}
Copy link
Member

Choose a reason for hiding this comment

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

굿 이렇게 조립하는게 제일 깔끔할 것 같아요 어차피 페이징 덕에 큰 데이터를 끌고 오는 것도 아니라서 👍

" and ?2 not in (select b.blocked_id from block b where b.blocker_id = c.member_id) " +
") child_comments " +
"where child_comments.row_num <= 30", nativeQuery = true)
List<Comment> findChildCommentByParentComments(List<Long> parentCommentIds, Long loginMemberId);
Copy link
Member

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.

이거 다른 코드들이랑 통일성 때메 했는데 ㅋㅋㅋ ㅜㅜ """이게 편해요 ..

Copy link
Member

Choose a reason for hiding this comment

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

아 ㅋㅋㅋㅋㅋㅋ 다른 코드 쓸때 """ < 이걸 몰랐어서 안썼던 거에요ㅎ;; 그냥 써도 될 것 같아요..허허

@yeahjinjeong
Copy link
Collaborator Author

테스트가

  1. 자식 댓글 조회 - 정상 테스트
  2. 자식 댓글 조회 - 자식 댓글 작성자에게 차단됐을때
  3. 부모 댓글 조회 - 정상 테스트
  4. 부모 댓글 조회 - 부모 댓글 작성자를 차단했을 때
  5. 자식 댓글 조회 - 정상 테스트
  6. 자식 댓글 조회 - 자식 댓글 작성자를 차단했을 때

이렇게같은데 자식 / 부모 댓글 정렬.. 한번만 맞춰주시면서 @DisplayName에 설명 조금만 추가해도 좋을 것 같아요 (조금.. 헷갈림,,ㅎㅎ) -> (아니면 메소드 순서에 다른 뜻이 있었나요?)

API 순서대로 했던 거 같아요 위에는 getComments() 밑에는 답글 조회 ..! 설명 추가하겠습니다

@dudrhy12
Copy link
Contributor

dudrhy12 commented Feb 9, 2025

sonarqubecloud issue 중에서 고칠만한건 없으셨나요?!

Copy link

github-actions bot commented Feb 9, 2025

Overall Project 62.24% 🍏
File Coverage
CommentController.java 100% 🍏
CommentRepository.java 58.33% 🍏
Constant.java 0% 🍏

Copy link

sonarqubecloud bot commented Feb 9, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
37.5% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@yeahjinjeong yeahjinjeong merged commit 5749daa into develop Feb 9, 2025
1 of 2 checks passed
@yeahjinjeong yeahjinjeong deleted the test/#157-comment-repository branch February 15, 2025 02:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants