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

feat: 쿠폰 도메인에 메타데이터 필드 추가 #847

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

Sangwook02
Copy link
Member

@Sangwook02 Sangwook02 commented Jan 22, 2025

🌱 관련 이슈

📌 작업 내용 및 특이사항

  • Coupon에 쿠폰 타입(CouponType), 발급 방식(IssuanceMethodType), 스터디 필드를 추가했습니다.
  • CouponTypeIssuanceMethodType은 enum 처리했고 CouponType 종류는 기획팀과 논의하여 우선 ADMIN, STUDY_COMPLETION만 만들어 뒀습니다.
  • CouponType이 스터디 관련인 경우, 어떤 study에서 발급된건지 저장하기 위한 목적으로 study 필드도 있습니다.
    ADMIN인 경우에는 이 필드가 null로 채워져서 정규화의 필요성에 대해 코어타임에 논의했지만 쿠폰의 수가 많지 않아 단일테이블을 유지하기로 했습니다.

📝 참고사항

📚 기타

Summary by CodeRabbit

릴리즈 노트

  • 새로운 기능

    • 쿠폰 생성 및 발급 프로세스에 쿠폰 유형 및 발급 방식 추가
    • 스터디 완료와 연계된 쿠폰 발급 기능 도입
  • 개선 사항

    • 쿠폰 생성 시 더 상세한 메타데이터 지원
    • 쿠폰 유형(관리자, 스터디 수료) 구분 가능
    • 자동 및 수동 쿠폰 발급 방식 지원
  • 기술적 변경

    • 쿠폰 도메인 모델 확장
    • 쿠폰 생성 및 발급 로직 개선

@Sangwook02 Sangwook02 self-assigned this Jan 22, 2025
@Sangwook02 Sangwook02 requested a review from a team as a code owner January 22, 2025 06:15
Copy link

coderabbitai bot commented Jan 22, 2025

Warning

Rate limit exceeded

@Sangwook02 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 22 minutes and 10 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 5fad801 and b0069cb.

📒 Files selected for processing (1)
  • src/main/java/com/gdschongik/gdsc/domain/coupon/dto/request/CouponCreateRequest.java (1 hunks)

Walkthrough

이 풀 리퀘스트는 쿠폰 도메인의 구조를 확장하고 메타데이터를 추가하는 변경 사항을 포함하고 있습니다. Coupon 클래스에 새로운 필드인 couponType, issuanceMethodType, study를 도입하여 쿠폰의 세부 정보를 더욱 상세하게 관리할 수 있게 되었습니다. 이와 함께 관련된 서비스, DTO, 테스트 클래스들도 이러한 변경 사항에 맞춰 업데이트되었습니다.

Changes

파일 변경 요약
CouponService.java 쿠폰 생성 및 발급 로직 업데이트, StudyRepository 추가
Coupon.java 새로운 필드 추가: couponType, issuanceMethodType, study
CouponType.java 새로운 열거형 추가: ADMIN, STUDY_COMPLETION
IssuanceMethodType.java 새로운 열거형 추가: AUTOMATIC, MANUAL
CouponCreateRequest.java 새로운 필드 추가: couponType, studyId
테스트 클래스들 새로운 쿠폰 생성 파라미터에 맞춰 테스트 케이스 업데이트

Assessment against linked issues

목표 달성 여부 설명
쿠폰 식별을 위한 메타데이터 필드 추가 [#839]

Possibly related PRs

Suggested labels

✨ feature, ♻️ refactor

Suggested reviewers

  • kckc0608

Poem

🐰 쿠폰의 세계에 새 바람이 불어와
메타데이터로 더 풍성해진 하루
타입과 연결, 이제는 더 명확해
토끼의 눈빛처럼 반짝이는 쿠폰이여
코드의 꽃이 피어나네! 🌸


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🔭 Outside diff range comments (2)
src/test/java/com/gdschongik/gdsc/domain/coupon/domain/CouponTest.java (1)

Line range hint 22-38: 새로 추가된 필드들에 대한 테스트 케이스가 부족합니다.

다음과 같은 테스트 케이스들을 추가하는 것이 좋겠습니다:

  1. STUDY_COMPLETION 타입일 때 study가 null인 경우
  2. ADMIN 타입일 때 study가 존재하는 경우
  3. 각 IssuanceMethodType에 대한 케이스

예시 테스트 코드:

@Test
void STUDY_COMPLETION_타입일때_스터디가_NULL이면_실패한다() {
    // when & then
    assertThatThrownBy(() -> 
        Coupon.create(COUPON_NAME, Money.from(ONE), STUDY_COMPLETION, AUTOMATIC, null))
        .isInstanceOf(CustomException.class)
        .hasMessageContaining(STUDY_REQUIRED_FOR_STUDY_COMPLETION.getMessage());
}

@Test
void ADMIN_타입일때_스터디가_존재하면_실패한다() {
    // given
    Study study = Study.create("테스트 스터디");

    // when & then
    assertThatThrownBy(() -> 
        Coupon.create(COUPON_NAME, Money.from(ONE), ADMIN, AUTOMATIC, study))
        .isInstanceOf(CustomException.class)
        .hasMessageContaining(STUDY_NOT_ALLOWED_FOR_ADMIN.getMessage());
}
src/test/java/com/gdschongik/gdsc/domain/coupon/application/CouponServiceTest.java (1)

Line range hint 31-195: 쿠폰 메타데이터 필드에 대한 테스트 케이스 보강 필요

현재 테스트는 ADMIN 타입의 쿠폰만 테스트하고 있습니다. 다음과 같은 테스트 케이스를 추가해야 합니다:

  • STUDY_COMPLETION 타입 쿠폰 생성 테스트
  • 스터디 연관 관계가 있는 쿠폰 생성 테스트
  • 잘못된 스터디 ID로 쿠폰 생성 시도 시 실패 테스트
🧹 Nitpick comments (8)
src/test/java/com/gdschongik/gdsc/domain/order/domain/OrderValidatorTest.java (1)

Line range hint 4-447: 쿠폰 타입에 대한 테스트 케이스 추가 필요

현재 테스트는 쿠폰의 기본적인 유효성 검사만 다루고 있습니다. 새로 추가된 CouponType과 관련된 다음과 같은 테스트 케이스들을 추가하는 것이 좋겠습니다:

  1. ADMIN 타입 쿠폰 사용 시나리오
  2. STUDY_COMPLETION 타입 쿠폰 사용 시나리오
  3. 잘못된 쿠폰 타입 사용 시나리오

예시 테스트 케이스:

@Test
void STUDY_COMPLETION_쿠폰은_스터디가_null이_아닐때만_사용_가능하다() {
    // given
    Member currentMember = createAssociateMember(1L);
    RecruitmentRound recruitmentRound = createRecruitmentRound(
            LocalDateTime.now().minusDays(1),
            LocalDateTime.now().plusDays(1),
            2024,
            SemesterType.FIRST,
            MONEY_20000_WON);
    Membership membership = createMembership(currentMember, recruitmentRound);
    
    IssuedCoupon issuedCoupon = fixtureHelper.createAndIssue(
            MONEY_5000_WON, 
            currentMember, 
            CouponType.STUDY_COMPLETION,
            null); // study가 null이면 안됨
    
    MoneyInfo moneyInfo = MoneyInfo.of(MONEY_20000_WON, MONEY_5000_WON, MONEY_15000_WON);
    
    // when & then
    assertThatThrownBy(() -> orderValidator.validatePendingOrderCreate(
            membership, issuedCoupon, moneyInfo, currentMember))
            .isInstanceOf(CustomException.class)
            .hasMessage(COUPON_STUDY_REQUIRED.getMessage());
}
src/main/java/com/gdschongik/gdsc/domain/coupon/domain/IssuanceMethodType.java (2)

6-8: 클래스 수준 문서화 추가를 제안드립니다.

각 발급 방식의 의미와 사용 사례를 설명하는 Javadoc을 추가하면 더 명확할 것 같습니다.

다음과 같이 문서화를 추가해보세요:

+/**
+ * 쿠폰 발급 방식을 정의하는 열거형
+ * <p>
+ * AUTOMATIC: 시스템에 의해 자동으로 발급되는 쿠폰 (예: 스터디 완료 시)
+ * MANUAL: 관리자에 의해 수동으로 발급되는 쿠폰
+ */
 @Getter
 @AllArgsConstructor
 public enum IssuanceMethodType {

9-13: API 지원을 위한 유틸리티 메서드 추가를 제안드립니다.

PR 목표에 명시된 대로 프론트엔드에서 사용 가능한 발급 방식을 조회하기 위한 API가 필요합니다. 이를 지원하기 위한 유틸리티 메서드를 추가하면 좋을 것 같습니다.

다음과 같은 메서드 추가를 고려해보세요:

     private final String value;
+
+    /**
+     * 모든 발급 방식과 해당 설명을 Map으로 반환합니다.
+     * 
+     * @return 발급 방식 enum과 설명을 담은 Map
+     */
+    public static Map<String, String> getAvailableTypes() {
+        return Arrays.stream(values())
+            .collect(Collectors.toMap(
+                IssuanceMethodType::name,
+                IssuanceMethodType::getValue
+            ));
+    }
 }
src/main/java/com/gdschongik/gdsc/domain/coupon/domain/CouponType.java (1)

8-13: 각 enum 값에 대한 문서화가 필요합니다.

각 쿠폰 타입의 용도와 사용 사례를 명확히 하기 위해 Javadoc 주석을 추가하는 것이 좋겠습니다.

 public enum CouponType {
+    /** 관리자가 직접 발급하는 쿠폰 */
     ADMIN("어드민"),
+    /** 스터디 완료 시 자동으로 발급되는 쿠폰 */
     STUDY_COMPLETION("스터디 수료");

     private final String value;
 }
src/main/java/com/gdschongik/gdsc/domain/coupon/domain/Coupon.java (1)

46-48: 성능 최적화를 위한 인덱스 추가가 필요합니다.

study_id 컬럼에 대한 인덱스를 추가하여 쿠폰 조회 성능을 개선하는 것이 좋겠습니다.

 @ManyToOne(fetch = FetchType.LAZY)
-@JoinColumn(name = "study_id")
+@JoinColumn(name = "study_id", index = @Index(name = "idx_coupon_study_id"))
 private Study study;
src/test/java/com/gdschongik/gdsc/domain/coupon/domain/IssuedCouponTest.java (1)

26-26: 메타데이터 필드에 대한 테스트 케이스 추가 필요

새로 추가된 CouponTypeIssuanceMethodType 필드에 대한 테스트 케이스가 부족합니다. 다음과 같은 테스트 케이스를 추가하는 것이 좋습니다:

  • 스터디 완료 쿠폰 타입(STUDY_COMPLETION)에 대한 테스트
  • 스터디 연관 관계가 있는 경우와 없는 경우에 대한 테스트

Also applies to: 41-41, 56-56, 75-75, 89-89, 103-103

src/test/java/com/gdschongik/gdsc/helper/FixtureHelper.java (1)

87-89: 테스트 헬퍼 메서드의 유연성 개선 필요

createAndIssue 메서드가 IssuanceMethodType.AUTOMATIC을 하드코딩하고 있습니다. 다양한 발급 방식을 테스트할 수 있도록 메서드를 개선하는 것이 좋습니다.

다음과 같이 수정을 제안합니다:

-    public IssuedCoupon createAndIssue(Money money, Member member, CouponType couponType, Study study) {
-        Coupon coupon = Coupon.create(COUPON_NAME, money, couponType, IssuanceMethodType.AUTOMATIC, study);
+    public IssuedCoupon createAndIssue(Money money, Member member, CouponType couponType, IssuanceMethodType issuanceMethodType, Study study) {
+        Coupon coupon = Coupon.create(COUPON_NAME, money, couponType, issuanceMethodType, study);
         return IssuedCoupon.create(coupon, member);
    }
src/test/java/com/gdschongik/gdsc/helper/IntegrationTest.java (1)

225-229: 테스트 헬퍼 메서드가 적절히 수정되었습니다만, 문서화가 필요합니다.

메서드의 파라미터가 늘어났으므로, 각 파라미터의 역할과 제약사항을 설명하는 JavaDoc 주석이 필요합니다.

다음과 같이 JavaDoc을 추가하는 것을 제안합니다:

+    /**
+     * 쿠폰을 생성하고 발급하는 테스트 헬퍼 메서드
+     * @param money 쿠폰 금액
+     * @param member 발급 대상 회원
+     * @param couponType 쿠폰 타입 (ADMIN 또는 STUDY_COMPLETION)
+     * @param study 스터디 (couponType이 STUDY_COMPLETION인 경우에만 필요)
+     * @return 발급된 쿠폰
+     */
     protected IssuedCoupon createAndIssue(Money money, Member member, CouponType couponType, Study study) {
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0160cd8 and 8ceeb8d.

📒 Files selected for processing (12)
  • src/main/java/com/gdschongik/gdsc/domain/coupon/application/CouponService.java (4 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/coupon/domain/Coupon.java (2 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/coupon/domain/CouponType.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/coupon/domain/IssuanceMethodType.java (1 hunks)
  • src/main/java/com/gdschongik/gdsc/domain/coupon/dto/request/CouponCreateRequest.java (1 hunks)
  • src/test/java/com/gdschongik/gdsc/domain/coupon/application/CouponServiceTest.java (10 hunks)
  • src/test/java/com/gdschongik/gdsc/domain/coupon/domain/CouponTest.java (3 hunks)
  • src/test/java/com/gdschongik/gdsc/domain/coupon/domain/IssuedCouponTest.java (7 hunks)
  • src/test/java/com/gdschongik/gdsc/domain/order/application/OrderServiceTest.java (14 hunks)
  • src/test/java/com/gdschongik/gdsc/domain/order/domain/OrderValidatorTest.java (2 hunks)
  • src/test/java/com/gdschongik/gdsc/helper/FixtureHelper.java (3 hunks)
  • src/test/java/com/gdschongik/gdsc/helper/IntegrationTest.java (3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: build-test
🔇 Additional comments (3)
src/test/java/com/gdschongik/gdsc/domain/order/domain/OrderValidatorTest.java (1)

8-8: 메서드 시그니처 변경이 올바르게 반영되었습니다.

CouponType 임포트와 createAndIssue 메서드의 파라미터 추가가 적절하게 구현되었습니다.

Also applies to: 49-49

src/main/java/com/gdschongik/gdsc/domain/coupon/domain/IssuanceMethodType.java (1)

1-5: 패키지 구조와 임포트가 잘 구성되어 있습니다!

도메인 주도 설계(DDD) 원칙에 따라 패키지가 잘 구성되어 있으며, 필요한 Lombok 어노테이션만 임포트하여 깔끔합니다.

src/test/java/com/gdschongik/gdsc/helper/IntegrationTest.java (1)

17-18: 새로운 enum 타입들이 올바르게 임포트되었습니다.

쿠폰 도메인의 메타데이터 필드 추가에 필요한 CouponTypeIssuanceMethodType enum이 적절하게 임포트되었습니다.

Comment on lines 10 to 14
public record CouponCreateRequest(
@NotBlank String name,
@Positive BigDecimal discountAmount,
CouponType couponType,
@Nullable @Schema(description = "스터디 관련 쿠폰이 아니라면 null을 가집니다.") Long studyId) {}
Copy link

@coderabbitai coderabbitai bot Jan 22, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

couponType 필드에 대한 유효성 검증이 누락되었습니다.

couponType 필드가 null이 되지 않도록 @NotNull 어노테이션을 추가해야 합니다. 또한 studyId가 필요한 상황에서 누락되는 것을 방지하기 위한 커스텀 밸리데이션도 고려해보세요.

 public record CouponCreateRequest(
         @NotBlank String name,
         @Positive BigDecimal discountAmount,
-        CouponType couponType,
+        @NotNull CouponType couponType,
         @Nullable @Schema(description = "스터디 관련 쿠폰이 아니라면 null을 가집니다.") Long studyId) {}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public record CouponCreateRequest(
@NotBlank String name,
@Positive BigDecimal discountAmount,
CouponType couponType,
@Nullable @Schema(description = "스터디 관련 쿠폰이 아니라면 null을 가집니다.") Long studyId) {}
public record CouponCreateRequest(
@NotBlank String name,
@Positive BigDecimal discountAmount,
@NotNull CouponType couponType,
@Nullable @Schema(description = "스터디 관련 쿠폰이 아니라면 null을 가집니다.") Long studyId) {}

Copy link
Member Author

Choose a reason for hiding this comment

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

반영 완료했습니다~

Copy link

Choose a reason for hiding this comment

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

🔗 Analysis chain

@Sangwook02 확인했습니다. 변경사항이 잘 반영되었네요! 👍

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if @NotNull annotation was added to couponType field
rg "@NotNull.*CouponType couponType" src/main/java/com/gdschongik/gdsc/domain/coupon/dto/request/CouponCreateRequest.java

Length of output: 199


If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!

Comment on lines +40 to +48
@Enumerated(EnumType.STRING)
private CouponType couponType;

@Enumerated(EnumType.STRING)
private IssuanceMethodType issuanceMethodType;

@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "study_id")
private Study study;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

도메인 무결성 검증이 필요합니다.

새로 추가된 필드들에 대한 도메인 규칙 검증이 누락되었습니다. CouponTypeStudy 사이의 관계에 대한 검증이 특히 중요합니다.

다음과 같은 검증 메소드를 추가해주세요:

private static void validateStudyRequirement(CouponType couponType, Study study) {
    if (couponType == CouponType.STUDY_COMPLETION && study == null) {
        throw new CustomException(STUDY_REQUIRED_FOR_STUDY_COMPLETION);
    }
    if (couponType == CouponType.ADMIN && study != null) {
        throw new CustomException(STUDY_NOT_ALLOWED_FOR_ADMIN);
    }
}

Comment on lines +64 to +77
public static Coupon create(
String name,
Money discountAmount,
CouponType couponType,
IssuanceMethodType issuanceMethodType,
Study study) {
validateDiscountAmountPositive(discountAmount);
return Coupon.builder().name(name).discountAmount(discountAmount).build();
return Coupon.builder()
.name(name)
.discountAmount(discountAmount)
.couponType(couponType)
.issuanceMethodType(issuanceMethodType)
.study(study)
.build();
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

도메인 검증 로직이 create 메소드에서 누락되었습니다.

create 메소드에서 새로 추가된 필드들에 대한 검증을 수행해야 합니다.

 public static Coupon create(
         String name,
         Money discountAmount,
         CouponType couponType,
         IssuanceMethodType issuanceMethodType,
         Study study) {
     validateDiscountAmountPositive(discountAmount);
+    validateStudyRequirement(couponType, study);
     return Coupon.builder()
             .name(name)
             .discountAmount(discountAmount)
             .couponType(couponType)
             .issuanceMethodType(issuanceMethodType)
             .study(study)
             .build();
 }

Committable suggestion skipped: line range outside the PR's diff.

// TODO: 요청할 때마다 새로운 쿠폰 생성되는 문제 수정: 스터디마다 하나의 쿠폰만 존재하도록 쿠폰 타입 및 참조 식별자 추가
Coupon coupon = Coupon.create(couponName, Money.from(5000L));
Coupon coupon = Coupon.create(couponName, Money.from(5000L), CouponType.STUDY_COMPLETION, AUTOMATIC, study);
Copy link

@coderabbitai coderabbitai bot Jan 22, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

하드코딩된 할인 금액 값 개선 필요

스터디 완료 쿠폰의 할인 금액이 5000원으로 하드코딩되어 있습니다. 이 값은 설정 파일이나 상수로 관리되어야 합니다.

Copy link
Member

Choose a reason for hiding this comment

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

저도 이 의견에 동의합니다. 할인액은 상수로 관리하면 좋을 것 같아요

Copy link

Choose a reason for hiding this comment

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

@kckc0608 감사합니다. 이 피드백을 반영하여 다음 PR에서 할인액을 상수로 관리하도록 하겠습니다.


✏️ Learnings added
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:26:59.977Z
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.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Comment on lines +117 to +119
String couponName = couponNameUtil.generateStudyCompletionCouponName(study);
// TODO: 요청할 때마다 새로운 쿠폰 생성되는 문제 수정: 스터디마다 하나의 쿠폰만 존재하도록 쿠폰 타입 및 참조 식별자 추가
Coupon coupon = Coupon.create(couponName, Money.from(5000L));
Coupon coupon = Coupon.create(couponName, Money.from(5000L), CouponType.STUDY_COMPLETION, AUTOMATIC, study);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

스터디별 중복 쿠폰 생성 문제 해결 필요

TODO 주석에서 언급된 것처럼, 현재 구현은 요청할 때마다 새로운 쿠폰이 생성되는 문제가 있습니다.

다음과 같은 해결 방안을 제안합니다:

  1. 스터디 ID와 쿠폰 타입을 복합 유니크 키로 설정
  2. 쿠폰 생성 전에 기존 쿠폰 존재 여부를 확인하는 로직 추가
+        Optional<Coupon> existingCoupon = couponRepository.findByStudyAndCouponType(study, CouponType.STUDY_COMPLETION);
+        Coupon coupon = existingCoupon.orElseGet(() -> {
             String couponName = couponNameUtil.generateStudyCompletionCouponName(study);
-            Coupon coupon = Coupon.create(couponName, Money.from(5000L), CouponType.STUDY_COMPLETION, AUTOMATIC, study);
-            couponRepository.save(coupon);
+            Coupon newCoupon = Coupon.create(couponName, Money.from(5000L), CouponType.STUDY_COMPLETION, AUTOMATIC, study);
+            return couponRepository.save(newCoupon);
+        });

Committable suggestion skipped: line range outside the PR's diff.

@@ -83,7 +84,7 @@ class 임시주문_생성할때 {

Membership membership = createMembership(member, recruitmentRound);

IssuedCoupon issuedCoupon = createAndIssue(MONEY_5000_WON, member);
IssuedCoupon issuedCoupon = createAndIssue(MONEY_5000_WON, member, ADMIN, null);
Copy link

@coderabbitai coderabbitai bot Jan 22, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

테스트 케이스들이 새로운 쿠폰 메타데이터를 반영하고 있으나, 테스트 커버리지 개선이 필요합니다.

모든 테스트 케이스에서 ADMIN 타입의 쿠폰만 테스트하고 있습니다. STUDY_COMPLETION 타입에 대한 테스트도 필요합니다.

다음과 같은 테스트 케이스 추가를 제안합니다:

  1. 스터디 완료 쿠폰으로 주문 생성
  2. 스터디 없이 STUDY_COMPLETION 타입 쿠폰 생성 시 실패 케이스
  3. 쿠폰 타입별 할인 정책 검증

예시 테스트 코드:

@Test
void 스터디완료쿠폰으로_주문생성_성공() {
    // given
    Member member = createMember();
    Study study = createStudy(member, START_TO_END_PERIOD, START_TO_END_PERIOD);
    IssuedCoupon issuedCoupon = createAndIssue(MONEY_5000_WON, member, STUDY_COMPLETION, study);
    
    // when & then
    // ... 주문 생성 및 검증 로직
}

Also applies to: 122-122, 163-163, 199-199, 233-233, 288-288, 368-368, 410-410, 455-455, 503-503, 548-548, 583-583, 616-616

Copy link
Member Author

Choose a reason for hiding this comment

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

스터디 관련 test는 #842 에서 추가

Copy link

Choose a reason for hiding this comment

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

@Sangwook02 알겠습니다. 스터디 관련 테스트 케이스들을 #842 이슈에서 다루시는 것으로 하겠습니다.

🔗 Analysis chain

@Sangwook02 알겠습니다. 스터디 관련 테스트 케이스들을 #842 이슈에서 다루시는 것으로 하겠습니다.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if issue #842 exists
gh issue view 842

Length of output: 187


If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!

Copy link
Member

Choose a reason for hiding this comment

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

이거 이런 스크립트는 굳이 언급할 필요 없을듯 합니다
코드래빗이 나중에 계속해서 반복되는 내용 리뷰할 때 학습 필요한 부분 있으면 그때 알려주면 되는 거라서요
불필요한 내용 learning에 추가하면 이상한 소리하더라고요

Copy link

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

Copy link

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

Copy link
Member

@uwoobeat uwoobeat left a comment

Choose a reason for hiding this comment

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

API 요청을 통해 생성된 쿠폰은 항상 수동 쿠폰이고 / 이벤트 등을 통해 트리거링되어 생성된 쿠폰은 항상 자동 쿠폰이기 때문에 정팩메 상에서 구분이 필요하지 않을까... 싶네요

@@ -83,7 +84,7 @@ class 임시주문_생성할때 {

Membership membership = createMembership(member, recruitmentRound);

IssuedCoupon issuedCoupon = createAndIssue(MONEY_5000_WON, member);
IssuedCoupon issuedCoupon = createAndIssue(MONEY_5000_WON, member, ADMIN, null);
Copy link
Member

Choose a reason for hiding this comment

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

이거 이런 스크립트는 굳이 언급할 필요 없을듯 합니다
코드래빗이 나중에 계속해서 반복되는 내용 리뷰할 때 학습 필요한 부분 있으면 그때 알려주면 되는 거라서요
불필요한 내용 learning에 추가하면 이상한 소리하더라고요


@Getter
@AllArgsConstructor
public enum IssuanceMethodType {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public enum IssuanceMethodType {
public enum IssuanceType {

MethodType 이 의미 상 같은 표현으로 보입니다

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

✨ 쿠폰 식별을 위한 메타데이터 필드 추가
3 participants