feat: 수료 철회시 쿠폰 회수 이벤트 구현 #857
Conversation
Walkthrough이 풀 리퀘스트는 스터디 수료 철회 시 발급된 쿠폰을 회수하는 새로운 기능을 추가합니다. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/main/java/com/gdschongik/gdsc/domain/coupon/application/CouponEventHandler.java (1)
Line range hint
15-15: 도메인 이벤트 처리 방향성 검토가 필요합니다.TODO 주석에서 언급된 것처럼, 도메인 이벤트 처리 방향이 일관되지 않은 것으로 보입니다. 아키텍처 일관성을 위해 이벤트 처리 방향을 통일하는 것이 좋겠습니다.
src/main/java/com/gdschongik/gdsc/domain/coupon/dao/IssuedCouponCustomRepositoryImpl.java (2)
41-52: 메서드에 JavaDoc 문서화가 필요합니다.비즈니스 로직의 명확한 이해를 위해 다음 내용을 포함한 문서화를 추가해주세요:
- 메서드의 목적
- 각 매개변수의 역할
- 반환값의 의미
- 쿠폰 회수 로직에서의 역할
예시:
/** * 취소되지 않은(revoked=false) 발급된 쿠폰을 조회합니다. * * @param couponType 조회할 쿠폰의 유형 * @param member 쿠폰을 발급받은 회원 * @param study 쿠폰이 발급된 스터디 * @return 조건에 맞는 발급된 쿠폰 (Optional) */
41-52: 매개변수 유효성 검사 추가를 고려해주세요.null 매개변수로 인한 잠재적인 문제를 방지하기 위해 다음과 같은 검증 로직 추가를 제안드립니다:
@Override public Optional<IssuedCoupon> findNonRevokedIssuedCouponByCouponTypeAndMemberAndStudy( CouponType couponType, Member member, Study study) { Objects.requireNonNull(couponType, "쿠폰 타입은 null일 수 없습니다"); Objects.requireNonNull(member, "회원은 null일 수 없습니다"); Objects.requireNonNull(study, "스터디는 null일 수 없습니다"); return Optional.ofNullable(queryFactory .selectFrom(issuedCoupon) // ... 나머지 쿼리 로직 ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/gdschongik/gdsc/domain/coupon/application/CouponEventHandler.java(2 hunks)src/main/java/com/gdschongik/gdsc/domain/coupon/application/CouponService.java(3 hunks)src/main/java/com/gdschongik/gdsc/domain/coupon/dao/IssuedCouponCustomRepository.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/coupon/dao/IssuedCouponCustomRepositoryImpl.java(2 hunks)src/main/java/com/gdschongik/gdsc/domain/coupon/dao/IssuedCouponQueryMethod.java(2 hunks)
🧰 Additional context used
📓 Learnings (1)
src/main/java/com/gdschongik/gdsc/domain/coupon/application/CouponService.java (1)
Learnt from: kckc0608
PR: GDSC-Hongik/gdsc-server#847
File: src/main/java/com/gdschongik/gdsc/domain/coupon/application/CouponService.java:115-115
Timestamp: 2025-01-22T12:27:00.185Z
Learning: Discount amounts in the coupon domain should be managed as constants rather than hardcoded values, especially for specific coupon types like study completion coupons.
🔇 Additional comments (5)
src/main/java/com/gdschongik/gdsc/domain/coupon/dao/IssuedCouponCustomRepository.java (1)
16-17: 메서드 시그니처가 명확하고 적절합니다!Optional 반환 타입을 사용하여 결과가 없을 수 있는 상황을 잘 처리했습니다.
src/main/java/com/gdschongik/gdsc/domain/coupon/application/CouponEventHandler.java (1)
25-29: 이벤트 핸들러 구현이 깔끔합니다!기존 패턴을 일관되게 따르고 있으며, BEFORE_COMMIT 페이즈를 적절히 사용하고 있습니다.
src/main/java/com/gdschongik/gdsc/domain/coupon/dao/IssuedCouponQueryMethod.java (1)
20-22: 쿼리 메서드 구현이 일관성 있고 안전합니다!null 체크를 통한 안전한 처리와 기존 쿼리 메서드들과의 일관된 패턴을 잘 유지하고 있습니다.
src/main/java/com/gdschongik/gdsc/domain/coupon/application/CouponService.java (1)
133-148: 쿠폰 회수 메서드 구현이 견고합니다!
- 트랜잭션 처리가 적절합니다
- 예외 처리가 명확합니다
- 로깅이 충분합니다
src/main/java/com/gdschongik/gdsc/domain/coupon/dao/IssuedCouponCustomRepositoryImpl.java (1)
6-14: 새로운 import 문이 적절히 추가되었습니다!새로운 기능 구현에 필요한 클래스들이 명확하게 import 되었습니다.
| @@ -109,7 +110,7 @@ public void createAndIssueCouponByStudyHistories(List<Long> studyHistoryIds) { | |||
| List<Member> students = memberRepository.findAllById(studentIds); | |||
| Study study = studyHistories.get(0).getStudy(); | |||
|
|
|||
| Coupon coupon = findOrCreate(CouponType.STUDY_COMPLETION, study); | |||
| Coupon coupon = findOrCreate(STUDY_COMPLETION, study); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
상수로 관리되어야 할 할인 금액이 하드코딩되어 있습니다.
이전 피드백에 따르면, 특정 쿠폰 타입의 할인 금액은 상수로 관리되어야 합니다. Money.FIVE_THOUSAND을 쿠폰 타입별 상수로 분리하는 것이 좋겠습니다.
+ private static final Money STUDY_COMPLETION_DISCOUNT_AMOUNT = Money.FIVE_THOUSAND;
private Coupon findOrCreate(CouponType couponType, Study study) {
return couponRepository.findByCouponTypeAndStudy(couponType, study).orElseGet(() -> {
String couponName = couponNameUtil.generateStudyCompletionCouponName(study);
- Coupon coupon = Coupon.createAutomatic(couponName, Money.FIVE_THOUSAND, couponType, study);
+ Coupon coupon = Coupon.createAutomatic(couponName, STUDY_COMPLETION_DISCOUNT_AMOUNT, couponType, study);
return couponRepository.save(coupon);
});
}Committable suggestion skipped: line range outside the PR's diff.
| import org.springframework.data.domain.Page; | ||
| import org.springframework.data.domain.Pageable; | ||
|
|
||
| public interface IssuedCouponCustomRepository { | ||
|
|
||
| Page<IssuedCoupon> findAllIssuedCoupons(IssuedCouponQueryOption queryOption, Pageable pageable); | ||
|
|
||
| Optional<IssuedCoupon> findNonRevokedIssuedCouponByCouponTypeAndMemberAndStudy( |
There was a problem hiding this comment.
| Optional<IssuedCoupon> findNonRevokedIssuedCouponByCouponTypeAndMemberAndStudy( | |
| Optional<IssuedCoupon> findUnrevokedIssuedStudyCoupon( |
spring data jpa 사용하는게 아니라서, byCouponTypeAnd... 는 안붙여줘도 파라미터 단에서 충분히 파악 가능한 컨텍스트일 것 같네요.
There was a problem hiding this comment.
안 그래도 이 부분 고민하긴 했는데 다른 querydsl 쿼리들을 저런식으로 네이밍한 것 같아서 일단 이렇게 해뒀습니다.
일단 이 부분은 제안 내용으로 수정하고 이외 쿼리들은 여유있을때 별도 이슈에서 처리하겠습니다~
| return Optional.ofNullable(queryFactory | ||
| .selectFrom(issuedCoupon) | ||
| .leftJoin(issuedCoupon.coupon, coupon) | ||
| .where(hasRevoked(false) |
There was a problem hiding this comment.
fk 잡혀있는 eqMember -> eqStudy를 먼저 걸고, 그 다음에 eqType과 hasRevoked 순서로 where 조건 거는 게 더 나을 것 같네요.
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/com/gdschongik/gdsc/domain/coupon/dao/IssuedCouponCustomRepositoryImpl.java (2)
46-49: 쿼리 성능 최적화를 위한 where 조건 순서 개선이 필요합니다.FK 관계와 인덱스 활용을 고려하여 where 조건의 순서를 다음과 같이 변경하는 것이 좋을 것 같습니다:
- .where(eqMember(member) - .and(coupon.study.eq(study)) - .and(hasRevoked(false)) - .and(coupon.couponType.eq(couponType))) + .where(coupon.study.eq(study) + .and(eqMember(member)) + .and(coupon.couponType.eq(couponType)) + .and(hasRevoked(false)))변경 이유:
- FK 관계가 있는 study 조건을 먼저 적용하여 초기 결과셋을 최소화
- member와 couponType으로 추가 필터링
- 마지막으로 revoked 상태 확인
43-45: leftJoin 사용이 필요한지 검토가 필요합니다.현재 쿼리에서는 coupon 엔티티의 필드들을 where 절에서만 사용하고 있습니다. 이 경우 leftJoin 대신 innerJoin을 사용하는 것이 더 적절할 것 같습니다.
- .leftJoin(issuedCoupon.coupon, coupon) + .join(issuedCoupon.coupon, coupon)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/gdschongik/gdsc/domain/coupon/application/CouponService.java(3 hunks)src/main/java/com/gdschongik/gdsc/domain/coupon/dao/IssuedCouponCustomRepository.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/coupon/dao/IssuedCouponCustomRepositoryImpl.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/gdschongik/gdsc/domain/coupon/application/CouponService.java
🔇 Additional comments (1)
src/main/java/com/gdschongik/gdsc/domain/coupon/dao/IssuedCouponCustomRepository.java (1)
16-16: 메서드 구현이 적절해 보입니다.Optional 반환 타입의 사용과 메서드 시그니처가 명확하며, 이전 리뷰에서 논의된 네이밍 컨벤션이 잘 반영되었습니다.
Sangwook02
deleted the
feature/841-revoke-coupon-on-study-completion-withdrawal
branch
🌱 관련 이슈
📌 작업 내용 및 특이사항
📝 참고사항
📚 기타
Summary by CodeRabbit
새로운 기능
버그 수정
리팩토링