feat: 스터디 수료 쿠폰을 스터디 당 하나만 가지도록 변경 #851
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/gdschongik/gdsc/domain/common/vo/Money.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/coupon/application/CouponService.java(2 hunks)src/main/java/com/gdschongik/gdsc/domain/coupon/dao/CouponRepository.java(1 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 (3)
src/main/java/com/gdschongik/gdsc/domain/coupon/dao/CouponRepository.java (1)
10-10: 메서드 구현이 적절합니다!Spring Data JPA 명명 규칙을 잘 따르고 있으며, Optional 반환 타입을 사용하여 null 안전성을 보장합니다.
src/main/java/com/gdschongik/gdsc/domain/common/vo/Money.java (1)
21-21: 상수 추출이 잘 되었습니다!PR #847에서 논의된 하드코딩 문제를 적절히 해결했습니다. Money 값 객체에 상수를 정의함으로써 코드의 유지보수성이 향상되었습니다.
src/main/java/com/gdschongik/gdsc/domain/coupon/application/CouponService.java (1)
124-130: findOrCreate 메서드 구현이 깔끔합니다!기존 쿠폰을 찾거나 새로 생성하는 로직이 잘 구현되어 있습니다. Money.FIVE_THOUSAND 상수를 사용하여 하드코딩을 제거한 점도 좋습니다.
|
|
||
| String couponName = couponNameUtil.generateStudyCompletionCouponName(study); | ||
| // TODO: 요청할 때마다 새로운 쿠폰 생성되는 문제 수정: 스터디마다 하나의 쿠폰만 존재하도록 쿠폰 타입 및 참조 식별자 추가 | ||
| Coupon coupon = Coupon.createAutomatic(couponName, Money.from(5000L), CouponType.STUDY_COMPLETION, study); | ||
| couponRepository.save(coupon); | ||
| Coupon coupon = findOrCreate(CouponType.STUDY_COMPLETION, study); |
There was a problem hiding this comment.
💡 Codebase verification
🛠️ Refactor suggestion
스터디 히스토리 유효성 검사 추가가 필요합니다
현재 구현에서는 다음과 같은 잠재적인 문제가 있습니다:
- studyHistories가 비어있는 경우 IndexOutOfBoundsException 발생 가능
- 서로 다른 스터디의 히스토리가 포함된 경우 잘못된 쿠폰이 발급될 수 있음
리뷰 코멘트에서 제안된 검증 로직 추가를 권장드립니다:
if (studyHistories.isEmpty()) {
throw new CustomException(STUDY_HISTORY_NOT_FOUND);
}
boolean allSameStudy = studyHistories.stream()
.map(StudyHistory::getStudy)
.distinct()
.count() == 1;
if (!allSameStudy) {
throw new CustomException(INVALID_STUDY_HISTORIES);
}추가로 INVALID_STUDY_HISTORIES 에러 코드도 정의가 필요합니다.
🔗 Analysis chain
studyHistories의 유효성 검사가 필요합니다.
다음 사항들을 확인해주세요:
- studyHistories가 비어있는 경우에 대한 처리
- 모든 studyHistory가 동일한 study에 속하는지 검증
아래는 검증을 위한 코드 제안입니다:
+ if (studyHistories.isEmpty()) {
+ throw new CustomException(STUDY_HISTORY_NOT_FOUND);
+ }
+
+ boolean allSameStudy = studyHistories.stream()
+ .map(StudyHistory::getStudy)
+ .distinct()
+ .count() == 1;
+ if (!allSameStudy) {
+ throw new CustomException(INVALID_STUDY_HISTORIES);
+ }
+
Study study = studyHistories.get(0).getStudy();🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Get the complete implementation of CouponService
ast-grep --pattern 'class CouponService {
$$$
}'
# Search for any existing validation logic related to studyHistories
rg "studyHistories" -A 5 -B 5
# Check for custom exception definitions
rg "STUDY_HISTORY_NOT_FOUND|INVALID_STUDY_HISTORIES" -A 2 -B 2
Length of output: 26389
| @@ -123,4 +120,12 @@ public void createAndIssueCouponByStudyHistories(List<Long> studyHistoryIds) { | |||
| "[CouponService] 스터디 수료 쿠폰 발급: issuedCouponIds={}", | |||
| issuedCoupons.stream().map(IssuedCoupon::getId).toList()); | |||
| } | |||
|
|
|||
| private Coupon findOrCreate(CouponType couponType, Study study) { | |||
There was a problem hiding this comment.
저번에 findOrCreate 계열 따닥 이슈 있었는데, 이것도 unique constraint 있어야 방지할 수 있지 않을까 싶네요
There was a problem hiding this comment.
오 그렇겠네요
쿠폰에 unique constraint 걸어두겠습니다~
| @@ -123,4 +120,12 @@ public void createAndIssueCouponByStudyHistories(List<Long> studyHistoryIds) { | |||
| "[CouponService] 스터디 수료 쿠폰 발급: issuedCouponIds={}", | |||
| issuedCoupons.stream().map(IssuedCoupon::getId).toList()); | |||
| } | |||
|
|
|||
| private Coupon findOrCreate(CouponType couponType, Study study) { | |||
There was a problem hiding this comment.
그리고 submitAssignment 에서도 findOrCreate 가 있는데, 여기서는 create 분기에서는 repository.save를 호출하지 않고 그냥 정적 팩토리로 생성된 객체를 리턴하고, 바깥에서 save를 호출하는 식으로 하고 있네요.
해당 로직에서는 find 분기로 가져온 과제제출이력을 수정할 니즈가 있어서, 바깥에서도 save를 항상 호출 가능한 상황이었지만...
여기서는 순수하게 조회 or 생성이기 때문에, 안에서 save를 호출하는 것이 맞을 것 같네요.
그래도 AssignmentHistory의 findOrCreate 내부에서 save를 호출하도록 투두 하나 추가해주면 좋을 것 같아요
There was a problem hiding this comment.
네네 todo 추가하고 issue도 만들어둘게요
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/com/gdschongik/gdsc/domain/coupon/domain/Coupon.java (1)
29-29: 애플리케이션 레벨의 유효성 검사 추가를 고려해보세요.데이터베이스 제약 조건과 함께, 서비스 레이어에서도 중복 쿠폰 생성 시도를 미리 확인하는 것이 좋습니다. 이를 통해:
- 불필요한 데이터베이스 작업을 줄일 수 있습니다
- 사용자 친화적인 에러 메시지를 제공할 수 있습니다
- 동시성 문제를 더 우아하게 처리할 수 있습니다
서비스 레이어에 유효성 검사 로직을 추가하는 예시 코드를 제공해드릴까요?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/gdschongik/gdsc/domain/coupon/domain/Coupon.java(1 hunks)src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/gdschongik/gdsc/domain/study/application/StudentStudyHistoryService.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-test
🔇 Additional comments (2)
src/main/java/com/gdschongik/gdsc/domain/coupon/domain/Coupon.java (2)
20-21: 필요한 임포트문이 잘 추가되었습니다.데이터베이스 제약 조건을 위한 필수 어노테이션들이 정확하게 임포트되었습니다.
29-29: 스터디당 하나의 쿠폰만 허용하도록 제약 조건이 잘 구현되었습니다.
coupon_type과study_id컬럼의 조합에 대한 유니크 제약 조건 추가로 각 스터디 당 동일한 타입의 쿠폰이 중복 생성되는 것을 방지할 수 있습니다.다음 스크립트를 실행하여 데이터베이스 스키마가 올바르게 적용되었는지 확인하시기 바랍니다:
✅ Verification successful
스터디당 동일한 쿠폰 타입의 중복 생성 방지가 올바르게 구현되었습니다.
@Table어노테이션의uniqueConstraints설정이coupon_type과study_id컬럼에 대해 올바르게 정의되어 있습니다.- Study 엔티티와의 관계 매핑이
@ManyToOne으로 적절하게 구현되어 있어, 데이터베이스 레벨에서 제약 조건이 정상적으로 적용됩니다.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: 데이터베이스 스키마에서 유니크 제약 조건이 올바르게 적용되었는지 확인 # JPA 엔티티에서 유니크 제약 조건 검색 rg -l "UniqueConstraint.*coupon_type.*study_id" . # 마이그레이션 파일에서 유니크 제약 조건 검색 fd -e sql . | xargs rg "UNIQUE.*coupon_type.*study_id"Length of output: 177
Script:
#!/bin/bash # Check complete Coupon entity structure and Study relationship ast-grep --pattern 'class Coupon { $$$ }' src/main/java/com/gdschongik/gdsc/domain/coupon/domain/Coupon.java # Look for Study entity and its relationship fd Study.javaLength of output: 193
Script:
#!/bin/bash # Get complete content of both entity files cat src/main/java/com/gdschongik/gdsc/domain/coupon/domain/Coupon.java cat src/main/java/com/gdschongik/gdsc/domain/study/domain/Study.java # Check for any other files that might reference these entities rg -l "Coupon|Study" --type javaLength of output: 21020
🌱 관련 이슈
📌 작업 내용 및 특이사항
기존에는 바로 create하고 save했다면, findOrCreate해서 save 하도록 수정했습니다.
📝 참고사항
📚 기타
Summary by CodeRabbit
Summary by CodeRabbit
새로운 기능
개선 사항
기타
FIVE_THOUSAND추가