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

대댓글의 순서가 정렬되지 않는 버그 해결 #360

Merged
merged 4 commits into from
Jun 4, 2024
Merged

Conversation

mingmingmon
Copy link
Collaborator

Summary

#359

대댓글의 순서가 오래된 순으로 정렬되어 보이도록 수정했습니다.

Tasks

대댓글 정렬 순서 어노테이션으로 명시

Screenshot

image

image

@mingmingmon mingmingmon added the 🐞 Bug 버그 제보 및 수정 label Jun 3, 2024
@mingmingmon mingmingmon requested a review from limehee June 3, 2024 16:27
@mingmingmon mingmingmon self-assigned this Jun 3, 2024
@limehee limehee linked an issue Jun 3, 2024 that may be closed by this pull request
@limehee
Copy link
Collaborator

limehee commented Jun 3, 2024

JPA에서 @OrderBy라는 어노테이션을 제공하는 지는 몰랐네요.
다만, 다음와 같은 문제가 우려됩니다.

  1. 해당 어노테이션에 기록된 설명(주석)에 따르면, @OrderBy 어노테이션을 사용할 경우, 해당 컬렉션을 조회할 때 항상 지정한 기준으로 정렬하여 결과를 반환합니다.
    위 말은 JPA를 통해 조회를 할 경우 무조건 지정한 방식으로의 정렬을 추가한다고 해석됩니다. 현재 페이지네이션에서 정렬 기준을 파라미터로 받아 동적으로 처리할 수 있도록 되어있는데, 해당 어노테이션의 영향을 받아 상황에 따라 원치 않는 결과가 반환될까 우려가 됩니다.

  2. 현재 수정사항은 1의 이유로 최근 작업한 페이지네이션 개선의 의도를 훼손한다는 생각이 듭니다. CommentServicetoCommentResponseDto() 메소드에서 부모와 연관된 자식 댓글을 불러오고 있는데, 해당 메소드를 개선하여 정렬을 처리하는 편이 더 낫다고 판단됩니다.

해당 작업은 버그 수정에 해당하지만 치명적인 버그는 아니기에, 추가 개선을 기다리겠습니다.

@mingmingmon
Copy link
Collaborator Author

mingmingmon commented Jun 4, 2024

피드백 감사합니다! getAllComments() 내부에서 댓글에 대한 자식 댓글을 불러오고, 자식 댓글들이 sortChildrenComments()를 거치는 것으로 해결했습니다.

현재 개선된 페이지네이션은 댓글에 대한 정렬 기준이고, 자식 댓글에 대한 커스텀 정렬은 내부적으로 sortChildrenComments()로 처리하게 되었습니다. 즉, 사용자로부터 입력받은 페이지네이션의 정렬 기준은 댓글에만 적용되고 있습니다.

따라서 짚어주신 페이지네이션 의도 훼손 가능성에 대해서 고민해본 결과 다음과 같은 결론을 내렸습니다.

  1. 댓글 자체는 커스텀 정렬이 가능합니다.
  2. 자식 댓글에 대한 커스텀 정렬이 필요하다면, 페이지네이션의 정렬을 받는 경우에 sortBysortDirection을 자식 댓글에 대해서도 한 세트씩 만들어야합니다.
  3. 개인적인 의견으로 getAllComments() 의 경우는 프론트분들과 잠깐 이야기 나눈 뒤로 고민을 많이 했는데요. createdAt을 기준으로 오래된 순서대로 댓글과 대댓글을 정렬하는게 좋은 것 같습니다. 따라서 현재 다른 칼럼들로도 댓글을 정렬을 해놓을 수 있게 했는데, 댓글 전체 조회에서 사용자가 무작위로 댓글 순서를 바꾸는게 과연 좋은가? 라고 생각이 들었습니다. 게시판의 댓글은 누가 언제 어떤 댓글을 달았는지가 중요한 것 같고, 따라서 생성된 순서대로 보이는게 의미있는 것 같아 생각했습니다.
  4. 반면 내 댓글 조회하기 의 경우는 좋아요나, 게시글 id 등 추가적인 칼럼들로 사용자가 원하는 정렬을 할 수 있는 것이 맞다고 생각합니다.

3은 제 개인적인 의견이기에, 댓글과 자식댓글에도 커스텀 정렬이 시스템에 꼭 필요하다면 관련해서 추가 작업을 다른 이슈에서 진행하겠습니다!

사진은 댓글의 커스텀 페이지네이션을 다시 확인해본 결과입니다.
image

@mingmingmon
Copy link
Collaborator Author

추가로 페이지네이션에서 외래키 기준 정렬할 때 sortBy를 칼럼명이 아닌, domain에 선언한 필드명으로 전달해야 함을 확인해서, 괜찮으시면 해당 이슈에서 같이 수정하고자 합니다!

@limehee
Copy link
Collaborator

limehee commented Jun 4, 2024

추가로 페이지네이션에서 외래키 기준 정렬할 때 sortBy를 칼럼명이 아닌, domain에 선언한 필드명으로 전달해야 함을 확인해서, 괜찮으시면 해당 이슈에서 같이 수정하고자 합니다!

해당 부분은 이번 버그 수정과 연관성이 낮다고 판단되어 별도의 이슈를 작성하는게 낫다고 판단됩니다.
따로 작업하는 편이 작업 내용 추적에도 더 좋아보여요.

@limehee
Copy link
Collaborator

limehee commented Jun 4, 2024

정렬 관련 버그에 대해서는 수정이 올바르게 되었다고 판단되어 develop 브랜치에 병합하겠습니다.

@limehee limehee merged commit 9ae97ff into develop Jun 4, 2024
1 check failed
@limehee limehee deleted the bug/#359 branch June 4, 2024 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 Bug 버그 제보 및 수정
Projects
None yet
Development

Successfully merging this pull request may close these issues.

대댓글의 순서가 정렬되지 않는 버그
2 participants