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

[BE] feat: 내가 받은 리뷰 보기 기능 구현 #109

Merged
merged 24 commits into from
Jul 25, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
f522fd7
refactor: contains 작동을 위한 EqualsAndHashcode 추가
nayonsoso Jul 25, 2024
85ddfd2
fix: lazyInitialization 해결
nayonsoso Jul 25, 2024
a0ab043
feat: 질문 레포지토리 생성
nayonsoso Jul 25, 2024
b08db88
feat: 내가 받은 리뷰 응답 생성
nayonsoso Jul 25, 2024
4f18dda
refactor: 리뷰 항목과 질문의 연관관계 변경 및 답변 최대 글자수 DB에 반영
nayonsoso Jul 25, 2024
65a7917
refactor: 리뷰에 리뷰그룹 초기화 부분 추가
nayonsoso Jul 25, 2024
a8f2528
feat: 내가 받은 리뷰 조회 기능 구현
nayonsoso Jul 25, 2024
022910c
feat: 받은 리뷰가 없을 때의 응답 추가
nayonsoso Jul 25, 2024
453d1f4
refactor: dto 설명 추가
nayonsoso Jul 25, 2024
92ea554
refactor: dto 설명 수정
nayonsoso Jul 25, 2024
f2e1a0d
refactor: 인자 형식 수정, 개행 수정
nayonsoso Jul 25, 2024
bf00a66
refactor: transactional 어노테이션 추가
nayonsoso Jul 25, 2024
aa87cbd
refactor: 내가 받은 리뷰 조회할 때Page객체 말고 List로 받아오도록 수정
nayonsoso Jul 25, 2024
4a491fd
refactor: 미리보기 만드는 기능 도메인 안으로 이동
nayonsoso Jul 25, 2024
6fcbc6d
test: 테스트 코드 개선
nayonsoso Jul 25, 2024
010db8e
refactor: 마지막으로 본 리뷰ID가 없는 로직에 대해 수정
nayonsoso Jul 25, 2024
3a229bb
docs: 스웨거 데코레이션 적용
nayonsoso Jul 25, 2024
412523e
refactor: lastReviewId가 null 이어도 가장 최신 리뷰를 찾을 수 있도록 수정
nayonsoso Jul 25, 2024
1499f37
Merge remote-tracking branch 'refs/remotes/origin/develop' into 107-g…
donghoony Jul 25, 2024
bdc847b
Merge remote-tracking branch 'refs/remotes/origin/develop' into 107-g…
donghoony Jul 25, 2024
af8cf09
Merge remote-tracking branch 'origin/107-get-my-received-review' into…
donghoony Jul 25, 2024
1fb6465
refactor: eqaulsAndHashCode 재정의
nayonsoso Jul 25, 2024
c8e3cbe
refactor: eqaulsAndHashCode 재재정의
nayonsoso Jul 25, 2024
065f752
refactor: API Docs 반영
donghoony Jul 25, 2024
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 @@ -3,6 +3,7 @@
import jakarta.persistence.CollectionTable;
import jakarta.persistence.ElementCollection;
import jakarta.persistence.Embeddable;
import jakarta.persistence.FetchType;
import jakarta.persistence.JoinColumn;
import java.util.Collections;
import java.util.List;
Expand All @@ -21,7 +22,7 @@ public class Keywords {

private static final int MAX_KEYWORD_COUNT = 5;

@ElementCollection
@ElementCollection(fetch = FetchType.EAGER)
@CollectionTable(name = "review_keyword", joinColumns = @JoinColumn(name = "review_id"))
private Set<Long> keywordIds;

Expand Down
2 changes: 2 additions & 0 deletions backend/src/main/java/reviewme/member/domain/GithubId.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
import jakarta.persistence.Column;
import jakarta.persistence.Embeddable;
import lombok.AccessLevel;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Embeddable
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@EqualsAndHashCode(of = "id")
@Getter
public class GithubId {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
import reviewme.review.dto.request.CreateReviewRequest;
import reviewme.review.dto.response.ReceivedReviewsResponse;
import reviewme.review.dto.response.ReviewDetailResponse;
import reviewme.review.service.ReviewService;

Expand All @@ -32,4 +33,12 @@ public ResponseEntity<ReviewDetailResponse> findReview(@PathVariable long id,
ReviewDetailResponse response = reviewService.findReview(id, memberId);
return ResponseEntity.ok(response);
}

@GetMapping("/reviews")
public ResponseEntity<ReceivedReviewsResponse> findMyReceivedReview(@RequestParam long memberId,
@RequestParam(defaultValue = "9999") long lastReviewId,
@RequestParam(defaultValue = "10") int size) {
ReceivedReviewsResponse myReceivedReview = reviewService.findMyReceivedReview(memberId, lastReviewId, size);
return ResponseEntity.ok(myReceivedReview);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

우리 디스코드에 공유한 API 문서에서는 요청 URL을 이렇게 받더라고요.
/reviews?revieweeId=?&lastReviewId=?&memberId=? (memberId 로 로그인했다고 가정)
그런데 지금은 '내가 받은 리뷰 조회' 이니까 revieweeId 랑 memberId가 같은 역할을 하는 중복 데이터라 생각해서 일단은 memberId 를 통해서만 조회하도록 했습니다. revieweeId는 무시하도록!

우선은 그냥 단순히 특정 회원의 리뷰만 조회한다고 생각하면 될까요?
(누구든지 memberId를 넣으면 다른 사람의 리뷰를 조회 가능!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

그춍.. 사실 목표한 기능은 '내가 받은 리뷰 보기'이지만
로그인 기능이 없고, memberId 로 로그인한다고 가정하고 있는 상황이니깐요😓

Copy link
Contributor

Choose a reason for hiding this comment

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

다음 리팩터링 사항 논의 때 고려해보죠~

1 change: 1 addition & 0 deletions backend/src/main/java/reviewme/review/domain/Review.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ public Review(Member reviewer, Member reviewee, ReviewerGroup reviewerGroup,
}
this.reviewer = reviewer;
this.reviewee = reviewee;
this.reviewerGroup = reviewerGroup;
this.keywords = new Keywords(keywords);
this.createdAt = createdAt;
reviewerGroup.addReview(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import jakarta.persistence.Id;
import jakarta.persistence.JoinColumn;
import jakarta.persistence.ManyToOne;
import jakarta.persistence.OneToOne;
import jakarta.persistence.Table;
import lombok.AccessLevel;
import lombok.Getter;
Expand All @@ -31,11 +30,11 @@ public class ReviewContent {
@JoinColumn(name = "review_id", nullable = false)
private Review review;

@OneToOne
@ManyToOne
Copy link
Contributor

Choose a reason for hiding this comment

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

요구사항대로 잘 되었네요 👍🏻

@JoinColumn(name = "question_id", nullable = false)
private Question question;

@Column(name = "answer", nullable = false)
@Column(name = "answer", nullable = false, length = MAX_ANSWER_LENGTH)
Copy link
Contributor

Choose a reason for hiding this comment

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

DB에 글자수 조건 반영 좋네요!
(이 코드 관련한 건 아니지만 새삼 다른 곳에도 했나 생각해봤는데 안되어있는 것 같아서, 이 부분도 맞춰줘야하겠다 싶어요. 아래 예시)

@Entity
public class ReviewerGroup {

    @Column(name = "description", nullable = false)
    private String description;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

헉스 그런데 제가 하면 혹시라도 conflict 날까봐 리팩터링 목록에 넣어두겠습니다!

#112

private String answer;

public ReviewContent(Review review, Question question, String answer) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package reviewme.review.dto.response;

import io.swagger.v3.oas.annotations.media.Schema;

Copy link
Contributor

Choose a reason for hiding this comment

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

위쪽에 어떤 응답인지 @Schema 달아주세요~!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

헉 속성에만 추가하고, dto 자체에는 안달아줬네요 😱
추가하고 푸쉬했습니다!

@Schema(description = "선택된 키워드 응답")
public record ReceivedReviewKeywordsResponse(

@Schema(description = "키워드 아이디")
Copy link
Contributor

Choose a reason for hiding this comment

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

다른 곳에서는 아이디를 "ID"로 표기했던 것 같은데, "아이디" 또는 "ID"로 통일해야 할 것 같아요.

Copy link
Contributor Author

@nayonsoso nayonsoso Jul 25, 2024

Choose a reason for hiding this comment

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

image
스크린샷 2024-07-25 오후 3 46 25

웁스 그렇네요!! 이슈 만들었습니다
#112

long id,

@Schema(description = "키워드 내용")
String content
) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package reviewme.review.dto.response;

import io.swagger.v3.oas.annotations.media.Schema;
import java.time.LocalDate;
import java.util.List;

@Schema(description = "리뷰 내용 응답")
public record ReceivedReviewResponse(

@Schema(description = "리뷰 아이디")
long id,

@Schema(description = "공개 여부")
boolean isPublic,

@Schema(description = "리뷰 작성일")
LocalDate createdAt,

@Schema(description = "응답 내용 미리보기")
String contentPreview,

@Schema(description = "리뷰어 그룹 정보")
ReceivedReviewReviewerGroupResponse reviewerGroup,

@Schema(description = "키워드")
List<ReceivedReviewKeywordsResponse> keywords
) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package reviewme.review.dto.response;

import io.swagger.v3.oas.annotations.media.Schema;

@Schema(description = "리뷰어 그룹 정보 응답")
Copy link
Contributor

Choose a reason for hiding this comment

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

리뷰어 그룹 정보를 내보내는 DTO가 많기에 "리뷰 목록의 리뷰어 그룹 정보 응답"으로 컨텍스트를 추가해주는 건 어떨까요?
(사실 Swagger에서 이 정보를 어느정도 중요도있게 받아들이는지 잘 감이 안와서 한편으로는 제가 제안한 컨텍스트 추가가 과한가? 싶기도 합니다. 편하게 의견 알려주세요!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

스웨거에서 볼 때 JSON 안에서 감싸져서 보여질 것이기 때문에 별도의 맥락 추가는 없어도 괜찮을 것 같아요 ㅎㅎ(귀찮은 것 아님)

public record ReceivedReviewReviewerGroupResponse(

@Schema(description = "리뷰어 그룹 아이디")
long id,

@Schema(description = "리뷰어 그룹 이름")
String name,

@Schema(description = "리뷰어 그룹 썸네일 이미지 URL")
String thumbnailUrl
) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package reviewme.review.dto.response;

import io.swagger.v3.oas.annotations.media.Schema;
import java.util.List;

@Schema(description = "내가 받은 리뷰 응답")
public record ReceivedReviewsResponse(

@Schema(description = "응답 개수")
long size,

@Schema(description = "마지막 리뷰 아이디")
long lastReviewId,

@Schema(description = "받은 리뷰 목록")
List<ReceivedReviewResponse> reviews
) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package reviewme.review.repository;

import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.stereotype.Repository;
import reviewme.review.domain.Question;

@Repository
public interface QuestionRepository extends JpaRepository<Question, Long> {
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package reviewme.review.repository;

import java.util.List;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;
import org.springframework.stereotype.Repository;
import reviewme.member.domain.Member;
import reviewme.member.domain.ReviewerGroup;
Expand All @@ -15,4 +17,15 @@ public interface ReviewRepository extends JpaRepository<Review, Long> {
default Review getReviewById(Long id) {
return findById(id).orElseThrow(ReviewNotFoundException::new);
}

@Query("""
SELECT r
FROM Review r
WHERE r.reviewee.id = :revieweeId
AND r.id < :lastViewedReviewId
ORDER BY r.createdAt DESC
LIMIT :size
"""
)
List<Review> findAllByRevieweeBeforeLastViewedReviewId(long revieweeId, long lastViewedReviewId, int size);
Copy link
Contributor

Choose a reason for hiding this comment

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

  • 메서드를 읽기 좋게 바꾸어도 좋겠습니다. Query 사용하고 있으니 더더욱 의미를 내포하는 게 중요하다고 생각해요
  • default 메서드 가장 아래로 내려주세요~

Copy link
Contributor Author

@nayonsoso nayonsoso Jul 25, 2024

Choose a reason for hiding this comment

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

음~ 쿼리를 썼으니 JPA가 제시하는 이름 형식을 따르지 않아도 된다는 말씀이군요!
사실 저도 그게 맞다고는 생각했는데, 뭔가 통일감을 위해서 장황하게 작성한거긴 하거든요 ㅎㅎ
그렇게 말씀해주시니 더 알아보기 쉽게 보겠습니다!

}
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,13 @@
import reviewme.member.domain.Member;
import reviewme.member.domain.ReviewerGroup;
import reviewme.member.repository.MemberRepository;
import reviewme.member.repository.ReviewerGroupRepository;
import reviewme.review.domain.Review;
import reviewme.review.domain.ReviewContent;
import reviewme.review.dto.request.CreateReviewRequest;
import reviewme.review.dto.response.ReceivedReviewKeywordsResponse;
import reviewme.review.dto.response.ReceivedReviewResponse;
import reviewme.review.dto.response.ReceivedReviewReviewerGroupResponse;
import reviewme.review.dto.response.ReceivedReviewsResponse;
import reviewme.review.dto.response.ReviewDetailResponse;
import reviewme.review.dto.response.ReviewDetailReviewContentResponse;
import reviewme.review.dto.response.ReviewDetailReviewerGroupResponse;
Expand All @@ -25,9 +28,9 @@
@RequiredArgsConstructor
public class ReviewService {

public static final int REVIEW_CONTENT_PREIVEW_MAX_LENGHT = 150;
Copy link
Contributor

Choose a reason for hiding this comment

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

상수 필드 띄워주세요!

nayonsoso marked this conversation as resolved.
Show resolved Hide resolved
private final ReviewRepository reviewRepository;
private final MemberRepository memberRepository;
private final ReviewerGroupRepository reviewerGroupRepository;
private final ReviewContentRepository reviewContentRepository;
private final KeywordRepository keywordRepository;

Expand All @@ -54,7 +57,8 @@ public ReviewDetailResponse findReview(long reviewId, long memberId) {
reviewerGroup.getDescription(),
reviewerGroup.getThumbnailUrl()
);
List<ReviewDetailReviewContentResponse> reviewContentResponses = reviewContents.stream()
List<ReviewDetailReviewContentResponse> reviewContentResponses = reviewContents
.stream()
.map(content -> new ReviewDetailReviewContentResponse(
content.getQuestion(),
content.getAnswer()
Expand All @@ -75,4 +79,50 @@ public ReviewDetailResponse findReview(long reviewId, long memberId) {
keywordContents
);
}

@Transactional(readOnly = true)
public ReceivedReviewsResponse findMyReceivedReview(long memberId, long lastReviewId, int size) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@Transactional(readonly = true) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

!

List<Review> reviews = reviewRepository.findAllByRevieweeBeforeLastViewedReviewId(
Copy link
Contributor

Choose a reason for hiding this comment

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

이제 개행 안해도 되겠네요~

memberId, lastReviewId, size);

int totalSize = reviews.size();
if (totalSize == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

이 부분에 대한 분기가 필요한가요? 어차피 마지막 값이 나오지 않는다면 더이상 요청을 하지 않을 것이라고 생각했어요.
분기가 필요하다면, size()로 꺼내오지 말고 isEmpty() 메서드를 쓰는 게 좋겠습니다.

Copy link
Contributor Author

@nayonsoso nayonsoso Jul 25, 2024

Choose a reason for hiding this comment

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

분기는 필요할 것 같은게요!

  1. 어떠한 리뷰도 작성되지 않은 상태에서 리뷰가 있는 것과 동일한 로직과 실행되려면
        return new ReceivedReviewsResponse(
                reviews.size(),
                reviews.get(totalSize - 1).getId(),  // ❗️❗️ 이부분에서 IndexOutOfBound 발생
                reviews.stream()
                        .map(review -> new ReceivedReviewResponse(
                                review.getId(),
                                review.isPublic(),
                                review.getCreatedAt().toLocalDate(),
                                getReviewContentPreview(review),
                                new ReceivedReviewReviewerGroupResponse(
                                        review.getReviewerGroup().getId(),
                                        review.getReviewerGroup().getGroupName(),
                                        review.getReviewerGroup().getThumbnailUrl()
                                ),
                                getKeywordResponse(review)))
                        .toList());

입니다.

ieEmpty() 부분은 적용하겠습니다!

return new ReceivedReviewsResponse(0, 0, List.of());
}

return new ReceivedReviewsResponse(
reviews.size(),
reviews.get(totalSize - 1).getId(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Java 21 절실하네요 🤔

reviews.stream()
Copy link
Contributor

Choose a reason for hiding this comment

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

인덴트가 많아지니 따로 변수로 뽑아도 좋겠네요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

음 new XXXResponse 들이 다 map 안에서 사용되는 것들이라 분리한다면 함수로 분리해줘야 할 것 같네요!


일단 private 함수로 만들어주긴 했는데, 서비스 코드에서 dto 를 만들기 위해 필요한 함수가 많아져서 dto 안에 도메인 -> dto 함수를 만드는 것도 고려해볼 수 있는 시점 같아요

.map(review -> new ReceivedReviewResponse(
review.getId(),
review.isPublic(),
review.getCreatedAt().toLocalDate(),
getReviewContentPreview(review),
Copy link
Contributor

Choose a reason for hiding this comment

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

(개인적인 의견)

  • 점점 답변에 책임이 늘고 있습니다. 길이 검증도 해야하고, 이제는 미리보기까지 보여줘야 하네요.
  • 값 객체로 승격시켜서 private 메서드를 활용하지 않고, review.getPreview()로 가져오면 어떨까요?

Copy link
Contributor

Choose a reason for hiding this comment

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

@donghoony 👍👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

따로 값 객체를 만들기보다,
ReviewContoent 도메인에서 미리보기를 받아오게 수정했습니다!

new ReceivedReviewReviewerGroupResponse(
review.getReviewerGroup().getId(),
review.getReviewerGroup().getGroupName(),
review.getReviewerGroup().getThumbnailUrl()
),
getKeywordResponse(review)))
.toList());
}

private String getReviewContentPreview(Review review) {
Copy link
Contributor

Choose a reason for hiding this comment

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

이건 ReviewContent의 책임으로 있어도 괜찮을 것 같은데 어떤가용?

Copy link
Contributor

Choose a reason for hiding this comment

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

  • 추가) 아루의 의견대로 ansewer 승격 후 책임 가지게 하는 것도 좋을 것 같아요. 작업이 복잡하면 추후 이슈로 등록해서 리팩토링 하죠~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

옮겼습니다! 날카로우시네요 ㅎ0ㅎ

String firstContentAnswer = reviewContentRepository.findAllByReviewId(review.getId())
.get(0)
.getAnswer();
return firstContentAnswer.substring(0, REVIEW_CONTENT_PREIVEW_MAX_LENGHT);
}

private List<ReceivedReviewKeywordsResponse> getKeywordResponse(Review review) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ReceivedReviewKeywordsResponse 생성을 메서드로 분리한 김에 ReceivedReviewResponseReceivedReviewReviewerGroupResponse 생성도 분리하면 어떨까요?

return review.getKeywords().getKeywordIds()
.stream()
.map(keywordRepository::getKeywordById)
.map(keyword -> new ReceivedReviewKeywordsResponse(
keyword.getId(),
keyword.getContent()
))
.toList();
}
}
2 changes: 2 additions & 0 deletions backend/src/test/java/reviewme/fixture/MemberFixture.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ public enum MemberFixture {

회원_산초("산초", 1L),
회원_아루("아루", 2L),
회원_커비("커비", 3L),
회원_테드("테드", 4L),
;

private final String name;
Expand Down
Loading