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: 사용자 마이페이지 리뷰 조회 기능 구현 #433

Merged
merged 4 commits into from
Aug 14, 2023

Conversation

wugawuga
Copy link
Collaborator

Issue

✨ 구현한 기능

  • 사용자 마이페이지 접속 시 해당 사용자 리뷰 조회 기능 추가입니다.

📢 논의하고 싶은 내용

  • x

🎸 기타

  • x

⏰ 일정

  • 추정 시간 : 2
  • 걸린 시간 : 3

@github-actions
Copy link

github-actions bot commented Aug 13, 2023

Unit Test Results

146 tests   146 ✔️  12s ⏱️
  82 suites      0 💤
  82 files        0

Results for commit 90d6c66.

♻️ This comment has been updated with latest results.

Copy link
Member

@70825 70825 left a comment

Choose a reason for hiding this comment

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

코드 깔끔하게 잘 작성했네요 👍
테스트 코드에서 수정할만한 부분 코멘트 남겼습니다~

Comment on lines 226 to 232
final var actualReviews = response.jsonPath().getList("reviews", MemberReviewDto.class);
final var expectedReviews = Collections.emptyList();

STATUS_CODE를_검증한다(response, 정상_처리);
페이지를_검증한다(response, page);
assertThat(actualReviews).usingRecursiveComparison()
.isEqualTo(expectedReviews);
Copy link
Member

Choose a reason for hiding this comment

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

actualReviews, expectedReviews를 검증하는건 STATUS_CODE를_검증한다, 페이지를_검증한다처럼 따로 메서드로 분리하는건 어떤가요??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

따로 분리가 되어 있습니다. 하지만 emptyList 같은경우 List 로 메소드를 함께 쓸 수 없다는 단점이 있습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

private <T> void 사용자_꿀조합_목록_조회_결과를_검증한다(final ExtractableResponse<Response> response, final List<T> recipes, final PageDto page) {
    페이지를_검증한다(response, page);
    사용자_꿀조합_목록을_검증한다(response, recipes);
}

private <T> void 사용자_꿀조합_목록을_검증한다(final ExtractableResponse<Response> response, final List<T> expected) {

    final var actual = response.jsonPath().getList("recipes", MemberRecipeDto.class);

    assertThat(actual).usingRecursiveComparison().isEqualTo(expected);
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이런식으로 구현하면 다같이 쓸 수 있습니다!! @70825 은 어떻게 생각하시나요?

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.

아 제가 못봤었네요 😭
사용자_꿀조합_목록을_검증한다 이 부분만 이렇게 바꿔도 될 것 같습니다!
T 사용하는건 생각도 못했녜요

Copy link
Collaborator

Choose a reason for hiding this comment

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

제네릭 멋져요...👍

Comment on lines +246 to +247
RESPONSE_CODE와_MESSAGE를_검증한다(response, LOGIN_MEMBER_NOT_FOUND.getCode(),
LOGIN_MEMBER_NOT_FOUND.getMessage());
Copy link
Member

Choose a reason for hiding this comment

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

getCode(), getMessage()는 내부 메서드에서 처리하는게 핵심 테스트 로직 파악할 때 더 깔끔할 것 같은데 어떤가요??

Copy link
Collaborator Author

@wugawuga wugawuga Aug 13, 2023

Choose a reason for hiding this comment

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

동의합니다!!

하지만 request 검증이 실패할 경우는 따로 errorcode 가 있지는 않습니다. @NotNull(message= "~~~") 이런식으로 예외 메세지가 담겨있습니다. 그래서 Code 를 따로 분리하려면 각 ErrorCode enum 에 맞는 메소드가 다 필요하게 됩니다!!
그래서 code, message 를 받는 식으로 구현해보았습니다!!

Copy link
Member

Choose a reason for hiding this comment

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

저는 ErrorCode가 최상위 클래스인줄 알고 적었는데 서로 다른 것이었군요
그러면 우가 코드가 맞는 것 같습니다!

Comment on lines 564 to 571
final var expectedReviews = List.of(review3_1, review2_1, review1_1);
final var expectedReviewDtos = expectedReviews.stream()
.map(MemberReviewDto::toDto)
.collect(Collectors.toList());
final var expectedPage = new SortingReviewsPageDto(3L, 1L, true, true, 0L, 10L);

assertThat(result.getReviews()).usingRecursiveComparison().isEqualTo(expectedReviewDtos);
assertThat(result.getPage()).usingRecursiveComparison().isEqualTo(expectedPage);
Copy link
Member

Choose a reason for hiding this comment

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

actualReviews, expectedReviews를 검증하는건 STATUS_CODE를_검증한다, 페이지를_검증한다처럼 따로 메서드로 분리하는건 어떤가요??

여기두요!

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

@70825 70825 left a comment

Choose a reason for hiding this comment

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

우가 최고 👍

Comment on lines 226 to 232
final var actualReviews = response.jsonPath().getList("reviews", MemberReviewDto.class);
final var expectedReviews = Collections.emptyList();

STATUS_CODE를_검증한다(response, 정상_처리);
페이지를_검증한다(response, page);
assertThat(actualReviews).usingRecursiveComparison()
.isEqualTo(expectedReviews);
Copy link
Member

Choose a reason for hiding this comment

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

아 제가 못봤었네요 😭
사용자_꿀조합_목록을_검증한다 이 부분만 이렇게 바꿔도 될 것 같습니다!
T 사용하는건 생각도 못했녜요

Comment on lines +246 to +247
RESPONSE_CODE와_MESSAGE를_검증한다(response, LOGIN_MEMBER_NOT_FOUND.getCode(),
LOGIN_MEMBER_NOT_FOUND.getMessage());
Copy link
Member

Choose a reason for hiding this comment

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

저는 ErrorCode가 최상위 클래스인줄 알고 적었는데 서로 다른 것이었군요
그러면 우가 코드가 맞는 것 같습니다!

Copy link
Collaborator

@Go-Jaecheol Go-Jaecheol left a comment

Choose a reason for hiding this comment

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

프로덕션 코드는 완벽한 것 같고, 저도 테스트 코드에서 개행 관련 코멘트 하나 남겼는데 그것만 확인해주세용 👍

Copy link
Collaborator

@hanueleee hanueleee left a comment

Choose a reason for hiding this comment

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

뚝딱뚝딱 우가 👍

Comment on lines 226 to 232
final var actualReviews = response.jsonPath().getList("reviews", MemberReviewDto.class);
final var expectedReviews = Collections.emptyList();

STATUS_CODE를_검증한다(response, 정상_처리);
페이지를_검증한다(response, page);
assertThat(actualReviews).usingRecursiveComparison()
.isEqualTo(expectedReviews);
Copy link
Collaborator

Choose a reason for hiding this comment

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

제네릭 멋져요...👍

@wugawuga wugawuga merged commit 9cd5f02 into develop Aug 14, 2023
3 checks passed
@wugawuga wugawuga deleted the feat/issue-418 branch August 14, 2023 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BE] 마이페이지 리뷰 조회
4 participants