-
Notifications
You must be signed in to change notification settings - Fork 0
[Fix] 리뷰 V2에서 리뷰 요약과 리뷰 리스트가 불일치 하는 상황 개선 #311
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
Summary of ChangesHello @HI-JIN2, 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은 리뷰 시스템의 일관성과 정확성을 향상시키는 데 중점을 둡니다. 특히, 리뷰 요약과 실제 리뷰 목록 간의 데이터 불일치 문제를 해결하고, 개별 리뷰의 평점이 올바르게 표시되도록 수정합니다. 이를 통해 사용자에게 더 정확하고 신뢰할 수 있는 리뷰 정보를 제공합니다. Highlights
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
안녕하세요. 리뷰 V2의 요약과 리스트 불일치 문제를 해결하고, 개별 리뷰 평점이 정확히 표시되도록 수정한 점이 인상 깊습니다. 전반적으로 문제 상황을 잘 해결하셨습니다. 코드의 안정성과 가독성을 높이기 위해 몇 가지 제안 사항을 남깁니다. 잠재적인 NullPointerException 수정, 로직 일관성 확보, 그리고 코드 가독성 개선에 대한 내용이 포함되어 있습니다. 검토 부탁드립니다.
| Integer rating = (review.getRating() != null) | ||
| ? review.getRating() | ||
| : review.getRatings().getMainRating(); |
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.
review.getRating()이 null일 때 review.getRatings().getMainRating()을 호출하는데, review.getRatings()가 null일 수 있어 NullPointerException이 발생할 위험이 있습니다. MealReviewResponse에서처럼 getRatings()에 대한 null 체크를 추가해야 합니다.
| Integer rating = (review.getRating() != null) | |
| ? review.getRating() | |
| : review.getRatings().getMainRating(); | |
| Integer rating = (review.getRating() != null) | |
| ? review.getRating() | |
| : (review.getRatings() != null ? review.getRatings().getMainRating() : null); |
| menuNames = Collections.singletonList( | ||
| new MenuIdNameLikeDto(review.getMenu().getId(), | ||
| review.getMenu().getName(), | ||
| false) | ||
| ); |
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.
메뉴 단일 리뷰(review.getMenu() != null)를 처리하는 로직에서 isLike 값이 false로 하드코딩되어 있습니다. 이로 인해 사용자가 메뉴에 '좋아요'를 표시했더라도 항상 false로 표시될 수 있습니다. review.getMenuLikes()를 확인하여 실제 '좋아요' 상태를 반영하도록 수정해야 합니다.
| menuNames = Collections.singletonList( | |
| new MenuIdNameLikeDto(review.getMenu().getId(), | |
| review.getMenu().getName(), | |
| false) | |
| ); | |
| boolean isLiked = review.getMenuLikes().stream() | |
| .anyMatch(like -> like.getMenu().getId().equals(review.getMenu().getId()) && like.getIsLike()); | |
| menuNames = Collections.singletonList( | |
| new MenuIdNameLikeDto(review.getMenu().getId(), | |
| review.getMenu().getName(), | |
| isLiked) | |
| ); |
| public static MealReviewResponse from(Review review, | ||
| Long userId, | ||
| List<ValidMenuForViewResponse.MenuDto> validMenus,Double rating) { | ||
| List<ValidMenuForViewResponse.MenuDto> validMenus, Integer rating) { |
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.
from 메서드의 rating 파라미터가 현재 사용되지 않고 있습니다. 메서드 내부에서 resolvedRating 변수를 통해 평점을 다시 계산하고 있으므로, 이 파라미터를 제거하여 코드를 더 명확하게 만드는 것이 좋겠습니다. 메서드 시그니처를 변경한 후에는 이 메서드를 호출하는 ReviewServiceV2.findMealReviewList 부분도 함께 수정해야 합니다.
| public static MealReviewResponse from(Review review, | |
| Long userId, | |
| List<ValidMenuForViewResponse.MenuDto> validMenus,Double rating) { | |
| List<ValidMenuForViewResponse.MenuDto> validMenus, Integer rating) { | |
| public static MealReviewResponse from(Review review, | |
| Long userId, | |
| List<ValidMenuForViewResponse.MenuDto> validMenus) { |
| .map(review -> MealReviewResponse.from(review, | ||
| userId, validMenus,mealRatingService.getMainRatingAverage(review.getMeal().getId()))) | ||
| userId, validMenus, review.getRating())) |
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.
MealReviewResponse.from 메서드에 전달되는 네 번째 인자(review.getRating())는 메서드 내부에서 사용되지 않습니다. 해당 파라미터를 제거하는 것을 고려해보세요. 이렇게 하면 코드가 더 깔끔해지고 의도를 명확하게 전달할 수 있습니다. 이 변경을 위해서는 MealReviewResponse.from 메서드의 시그니처도 함께 수정해야 합니다.
| .map(review -> MealReviewResponse.from(review, | |
| userId, validMenus,mealRatingService.getMainRatingAverage(review.getMeal().getId()))) | |
| userId, validMenus, review.getRating())) | |
| .map(review -> MealReviewResponse.from(review, | |
| userId, validMenus)) |
| .map(review -> { | ||
| Integer main = (review.getRatings() != null) ? review.getRatings() | ||
| .getMainRating() : null; | ||
| return (main != null) ? main : review.getRating(); | ||
| }) |
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.
평점을 가져오는 로직이 DTO 클래스(MealReviewResponse, ReviewDetail)의 로직과 일관되지 않습니다. DTO에서는 review.getRating()을 우선적으로 사용하지만, 여기서는 review.getRatings().getMainRating()을 우선적으로 사용하고 있습니다. 코드의 일관성을 위해 DTO와 동일한 로직으로 수정하는 것을 권장합니다.
| .map(review -> { | |
| Integer main = (review.getRatings() != null) ? review.getRatings() | |
| .getMainRating() : null; | |
| return (main != null) ? main : review.getRating(); | |
| }) | |
| .map(review -> { | |
| if (review.getRating() != null) { | |
| return review.getRating(); | |
| } | |
| return review.getRatings() != null ? review.getRatings().getMainRating() : null; | |
| }) |
| .average() | ||
| .stream() | ||
| .boxed() | ||
| .findFirst() | ||
| .orElse(null); |
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.
OptionalDouble을 Double로 변환하는 과정이 다소 복잡합니다. .stream().boxed().findFirst().orElse(null) 대신 isPresent()와 getAsDouble()를 사용하여 더 간결하고 명확하게 표현할 수 있습니다.
예시:
OptionalDouble averageOpt = ... .average();
Double mainRatingAverage = averageOpt.isPresent() ? averageOpt.getAsDouble() : null;이 방식은 코드 가독성을 높여줍니다. 전체 평균 계산 로직을 리팩토링할 때 참고하시면 좋겠습니다.
| RatingAverages averageRating = RatingAverages.builder() | ||
| .mainRating(mainRatingAverage) | ||
| .build(); | ||
| ReviewRatingCount ratingCountMap = ReviewRatingCount.from(combinedReviews); |
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.
JjungminLee
left a comment
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.
감사합니다! 진님이 써주신 설명 글 덕분에 로직이 이해가 갔어요
대신 해주셔서 정말 감사합니다! 면목이 없어요ㅠㅠ
맛,양에 대한 rating 필드를 지우지 않았던 것은 ios분들의 작업속도가 어디까지인지 몰라서 지우지 않았습니다. 이전에 제가 한번 필드 지웠다가 ios 잇슈앱이 흰화면 나왔던 일이 있었어서요..!
| .filter(menu -> !MenuFilterUtil.isExcludedFromReview( | ||
| menu.getName())) | ||
| // .filter(menu -> !MenuFilterUtil.isExcludedFromReview( | ||
| // menu.getName())) |
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.
아주 사소하긴 한데요! 주석은 안쓰면 지우는게 좋아보입니다!
#️⃣ Issue Number
#309
📝 요약(Summary)
문제 상황 1 리뷰 요약과 리뷰 리스트가 서로 맞지 않음
dev db meal id 4305 + 4305 속 menu들의 리뷰
논제: 이전에 작성한 제외되는 메뉴에 대한 리뷰(배추김치, 요구르트)를 버릴것이냐?
아니면 작성은 새로 못하게 하되, 이전에 작성한 애들까지는 포함해서 보여줄 것이냐
-> 본인 생각은 후자가 맞다고 생각함. 이미 작성된 리뷰이기 때문에
4305번 밀에 대한 리뷰는 1개
쌀밥 리뷰 1개
배추김치 4개
요구르트 5개
리뷰의 개수니까 다 더하면 11개가 맞음
문제상황 2 리뷰 리스트속 리뷰 item의 rating이 Int가 아님
전: 리뷰 리스트 속 모든 리뷰의 rating이 그 개별 리뷰에 대한 rating이 아니라
전체 리뷰들의 rating 평균으로 되어 있었음후:
개별 리뷰의 rating으로 수정함💬 공유사항 to 리뷰어
review table에 mainRating 과 rating은 사실상 같은 값으로 치는게 맞다 생각합니다.
구버전 menu에 대한 리뷰는 3종 별점이 있지만, 현버전은 meal에 대한 리뷰로 rating만 기재합니다
그런데 또 변동메뉴식당이 아닌 스낵코너의 경우에는 여전히 menu에 대한 리뷰를 작성해야합니다.
고로, 두 필드를 합치고 네이밍을 rating으로 바꾸는 방향이 좋지 않을까 생각합니다.
(+ 추후, 더는 쓰이지 않을 맛,양에 대한 rating 필드를 지우는게 좋지 않을까요?)
(현재 NPE를 방지하기 위해서 rating, 없으면 mainRating을 불러오도록 처리하였습니다)
✅ PR Checklist
PR이 다음 요구 사항을 충족하는지 확인하세요.