-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: 혜택 카테고리, 상점 조회 API 작성 #887
Conversation
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.
수고하셨습니다. 간단한거 몇가지만 체크해주세요.
(저도 전에 1000줄 pr날렸다가 팀원들과 멘토님께 피드백 받고, 구조 리팩토링도 따로 pr 나누려고 하는데 한번 고민해보시길)
src/main/java/in/koreatech/koin/domain/benefit/dto/BenefitCategoryResponse.java
Outdated
Show resolved
Hide resolved
src/main/java/in/koreatech/koin/domain/benefit/dto/BenefitCategoryResponse.java
Outdated
Show resolved
Hide resolved
src/main/java/in/koreatech/koin/domain/benefit/controller/BenefitApi.java
Outdated
Show resolved
Hide resolved
… feature/880-get-benefit-category
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.
파일체인지 보고 엄두도 안났는데 패키지 변경이었네요 👍
|
||
private final ShopBenefitService shopBenefitService; | ||
|
||
@GetMapping("/benefit/categories") |
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.
/benefit
공통이라 뺴도 될것 같아요
public record BenefitCategoryResponse( | ||
@Schema(description = "혜택 카테고리 리스트", requiredMode = NOT_REQUIRED) | ||
List<InnerBenefitResponse> benefits | ||
|
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.
개행 발견
… feature/880-get-benefit-category
🔥 연관 이슈
🚀 작업 내용
💬 리뷰 중점사항
이번 PR머지시키고 상점 조회에서 query dsl활용해서 성능 최적화 한번 시도해보려고 합니다.