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

[BSVR]-119 내 시야기록 조회 API 구현 #36

Merged
merged 6 commits into from
Jul 17, 2024
Merged

Conversation

pminsung12
Copy link
Collaborator

@pminsung12 pminsung12 commented Jul 17, 2024

📌 개요 (필수)

  • 내 시야기록 조회 API 구현했어요.

🔨 작업 사항 (필수)

  • 필터링 가능한 레포지토리 및 서비스 함수 구현
  • request dto와 controller 구현

⚡️ 관심 리뷰 (선택)

  • 아래 코멘트에서 확인해주세요.

🌱 연관 내용 (선택)

  • 준원이 PR이 머지되서 이제 유저 정보가 토큰 형태로 들어오면 userId 추출하는 리팩토링 필요!

💻 실행 화면 (필수)

  • 필터링 없이 전체 조회

image

  • 연도 필터링 조회

image

  • 아이템 없는 경우 -> 200 ok 지만 reviews 배열이 비어있는 형태로 반환

image

  • 테스트 결과

image

@pminsung12 pminsung12 added the ✨ Feature 기능 개발 label Jul 17, 2024
@pminsung12 pminsung12 self-assigned this Jul 17, 2024
Copy link

github-actions bot commented Jul 17, 2024

Test Results

39 tests  +1   39 ✅ +1   0s ⏱️ ±0s
14 suites ±0    0 💤 ±0 
14 files   ±0    0 ❌ ±0 

Results for commit 38d249a. ± Comparison against base commit a674d24.

♻️ This comment has been updated with latest results.

Comment on lines +36 to +43
public ReviewListResult findMyReviews(
Long userId, int offset, int limit, Integer year, Integer month) {
List<Review> reviews = reviewRepository.findByUserId(userId, offset, limit, year, month);
Long totalCount = reviewRepository.countByUserId(userId, year, month);
List<KeywordCount> topKeywords = Collections.emptyList(); // 사용자 리뷰에 대한 탑 키워드 모음 필요 없음

return new ReviewListResult(reviews, topKeywords, totalCount, offset, limit);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  • usecase 반환형인 ReviewListResult에 topkeyword부분은 블록으로 조회할 때는 필요하고, 내 리뷰만 조회(지금 만든 API)할 때는 필요하지 않아.
  • 이 경우에 레코드를 따로 만들어줄지(형태가 거의 똑같음) 그냥 emptyList를 반환(null로 두면 npe가 생김)해서 response 넘겨줄지 고민인데 일단 후자로 했어.
  • 혹시 개선 아이디어 있으면 코멘트 남겨주라

Copy link
Collaborator

Choose a reason for hiding this comment

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

나는 레코드 따로 만들어주는게 좋다고 생각해!
dto는 서로 아무런 연관 없관도 없고, 변동에도 취약하니 재사용 하기 보단 독립적으로 가져가는게 좋을 것 같아.
또 이 메서드만 봤을 때, topKeywords는 매번 emptyList인데 왜 필드가 있는건지 의문이 들기도 하고!
지금은 이대로 진행해도, 나중엔 리팩토링하면 좋을 듯 하다 👀

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

확인~! 중간발표 이후에 리팩토링할게!

Comment on lines +1 to +14
package org.depromeet.spot.application.review.dto.request;

import jakarta.validation.constraints.Max;
import jakarta.validation.constraints.Min;
import jakarta.validation.constraints.PositiveOrZero;

import io.swagger.v3.oas.annotations.Parameter;

public record MyReviewRequest(
@Parameter(description = "유저 ID") Long userId,
@PositiveOrZero @Parameter(description = "시작 위치 (기본값: 0)") Integer offset,
@Min(1) @Max(50) @Parameter(description = "조회할 리뷰 수 (기본값: 10, 최대: 50)") Integer limit,
@Min(1000) @Max(9999) @Parameter(description = "년도 (4자리 숫자)") Integer year,
@Min(1) @Max(12) @Parameter(description = "월 (1-12)") Integer month) {}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

request dto 필드로 useid 넣어놨는데 이건 토큰 검증방식 적용되면 토큰에서 받아오는걸로~

Comment on lines 50 to 52
System.out.printf(
"userId: %d, offset: %d, limit: %d, year: %d, month: %d\n",
userId, offset, limit, year, month);
Copy link
Collaborator

Choose a reason for hiding this comment

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

요거 지워주세용! 디버깅에 필요한 값이면 @slf4j 등을 활용해서 로그로 찍어줘~

Copy link
Collaborator

@EunjiShin EunjiShin left a comment

Choose a reason for hiding this comment

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

LGTM! 고생했오

@pminsung12 pminsung12 merged commit d92b939 into main Jul 17, 2024
4 checks passed
@pminsung12 pminsung12 deleted the feat/BSVR-119 branch July 17, 2024 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants