-
Notifications
You must be signed in to change notification settings - Fork 0
merge develop to main #79
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
Conversation
…reatedAt [Feature] SCRUM-75 comment 반환 시 작성일 함께 반환 (#76)
Summary of ChangesHello @jiminnimij, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 develop 브랜치의 변경 사항을 main 브랜치로 병합하는 것으로, 주로 댓글(Comment) 데이터 처리 방식에 대한 리팩토링을 포함합니다. 기존에 댓글의 특정 필드(예: 내용)만 다루던 방식에서 벗어나, Comment 엔티티 객체 자체를 전달하고 활용하도록 코드를 개선하여 데이터의 무결성을 높이고 더 많은 댓글 속성에 접근할 수 있도록 했습니다. 이는 향후 기능 확장을 용이하게 할 것입니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
안녕하세요. 코드 변경 사항을 검토했습니다.
이번 PR은 댓글(답변) 관련 로직을 수정하여, 기존에 댓글 내용(String)만 다루던 부분에서 Comment 객체 전체를 사용하도록 변경하는 것으로 보입니다. 이로 인해 ExhibitionPreview DTO에 답변 생성 시간이 추가되는 등 기능이 확장되었습니다.
몇 가지 개선점을 제안합니다.
CommentRepository의 쿼리 메소드가Post에 댓글이 여러 개일 경우NonUniqueResultException을 발생시킬 수 있는 심각한 위험이 있습니다. 이를 해결하기 위한 수정을 제안했습니다.ExhibitionPreview의of팩토리 메소드에 중복 코드가 있어 리팩토링을 제안했습니다.CommentValidator의 메소드 이름이 변경된 기능과 일치하지 않아 명확한 이름으로 변경할 것을 제안했습니다.
자세한 내용은 각 파일의 인라인 댓글을 확인해주세요.
| @Query(""" | ||
| SELECT c.content | ||
| SELECT c | ||
| FROM Comment c | ||
| WHERE c.post = :post | ||
| """) | ||
| Optional<String> findContentByPost(Post post); | ||
| Optional<Comment> findContentByPost(Post post); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 쿼리는 Post에 여러 개의 Comment가 있을 경우 여러 엔티티를 반환할 수 있습니다. 하지만, 메소드의 반환 타입이 Optional<Comment>이므로, 2개 이상의 댓글이 조회될 경우 NonUniqueResultException이 발생하여 런타임 오류로 이어질 수 있습니다. 이는 심각한 버그를 유발할 수 있습니다.
또한, 메소드 이름인 findContentByPost는 이제 전체 Comment 객체를 반환하므로 이름과 실제 동작이 일치하지 않습니다.
이 문제를 해결하고 코드의 의도를 명확히 하기 위해, Post에 대한 첫 번째 댓글만 가져오도록 하거나, 모든 댓글을 리스트로 받아 처리하는 것을 권장합니다. Spring Data JPA의 파생 쿼리 메소드를 사용하면 더 간결하게 표현할 수 있습니다.
예를 들어, 첫 번째 댓글만 필요하다면 다음과 같이 변경할 수 있습니다:
// @Query 어노테이션을 제거하고 메소드 시그니처를 변경합니다.
Optional<Comment> findFirstByPost(Post post);만약 가장 최근 댓글이 필요하다면 다음과 같이 할 수 있습니다:
Optional<Comment> findFirstByPostOrderByCreatedAtDesc(Post post);이러한 변경은 이 메소드를 호출하는 CommentValidator 등의 다른 부분에서도 수정이 필요합니다.
| private final CommentRepository commentRepository; | ||
|
|
||
| public String validateCommentContent(Post post) { | ||
| public Comment validateCommentContent(Post post) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
메소드의 반환 타입이 String에서 Comment 객체로 변경되면서, validateCommentContent라는 이름은 더 이상 메소드의 동작을 정확하게 반영하지 않습니다. 이제 이 메소드는 댓글 내용을 검증하는 것이 아니라, Post에 해당하는 Comment를 찾아 반환하고, 없으면 예외를 던집니다.
코드의 가독성과 명확성을 높이기 위해, 메소드의 실제 동작을 더 잘 나타내는 이름으로 변경하는 것이 좋습니다. 예를 들어 findCommentByPostOrThrow와 같은 이름이 더 적절해 보입니다.
| public Comment validateCommentContent(Post post) { | |
| public Comment findCommentByPostOrThrow(Post post) { |
| public static ExhibitionPreview of(Post post, Comment comment, boolean isWriter) { | ||
| if (comment == null) { | ||
| return ExhibitionPreview.builder() | ||
| .postId(post.getPostId()) | ||
| .writer(new PostWriterFormatting(post.getDisplayNo())) | ||
| .createdAt(new DateFormatting(post.getCreatedAt())) | ||
| .content(post.getContent()) | ||
| .isWriter(isWriter) | ||
| .hasAnswer(post.isHasAnswer()) | ||
| .answer(null) | ||
| .answerCreatedAt(null) | ||
| .build(); | ||
| } | ||
| return ExhibitionPreview.builder() | ||
| .postId(post.getPostId()) | ||
| .writer(new PostWriterFormatting(post.getDisplayNo())) | ||
| .createdAt(new DateFormatting(post.getCreatedAt())) | ||
| .content(post.getContent()) | ||
| .isWriter(isWriter) | ||
| .hasAnswer(post.isHasAnswer()) | ||
| .answer(answer) | ||
| .answer(comment.getContent()) | ||
| .answerCreatedAt(new DateFormatting(comment.getCreatedAt())) | ||
| .build(); | ||
|
|
||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 of 메소드는 comment가 null인 경우와 아닌 경우에 대해 ExhibitionPreview 객체를 생성하는 로직이 중복됩니다. builder를 한 번만 생성하고 조건에 따라 answer와 answerCreatedAt 필드를 설정하도록 리팩토링하면 코드가 더 간결해지고 유지보수하기 좋아집니다.
public static ExhibitionPreview of(Post post, Comment comment, boolean isWriter) {
ExhibitionPreview.ExhibitionPreviewBuilder builder = ExhibitionPreview.builder()
.postId(post.getPostId())
.writer(new PostWriterFormatting(post.getDisplayNo()))
.createdAt(new DateFormatting(post.getCreatedAt()))
.content(post.getContent())
.isWriter(isWriter)
.hasAnswer(post.isHasAnswer());
if (comment != null) {
builder.answer(comment.getContent())
.answerCreatedAt(new DateFormatting(comment.getCreatedAt()));
}
return builder.build();
}
#️⃣ 연관된 이슈
#️⃣ 작업 내용
#️⃣ 테스트 결과
#️⃣ 변경 사항 체크리스트
#️⃣ 스크린샷 (선택)
#️⃣ 리뷰 요구사항 (선택)
📎 참고 자료 (선택)