feat: admin 쿠폰 메모 추가 api - #166 #170
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
📝 WalkthroughSummary by CodeRabbit릴리스 노트
Walkthrough관리자용 쿠폰 메모 일괄 업데이트 API(PATCH /api/admin/coupons/memos), 요청 DTO(CouponMemoUpdateRequest), 서비스 로직(AdminCouponService.updateCouponMemos), CouponRetriever 조회 메서드 리팩토링, CouponEntity.updateMemo 및 신규 에러 코드 2개를 추가했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin Client
participant Controller as AdminCouponController
participant Service as AdminCouponService
participant Retriever as CouponRetriever
participant DB as Database
Admin->>Controller: PATCH /api/admin/coupons/memos\n(CouponMemoUpdateRequest)
activate Controller
Controller->>Service: updateCouponMemos(couponMemos)
activate Service
Service->>Service: validCouponDuplicated(couponIds)
alt 중복 ID 발견
Service-->>Controller: AdminApiException (CONFLICT_DUPLICATE_COUPON_ID)
Controller-->>Admin: 409 Conflict
else 중복 없음
Service->>Retriever: findAllCouponEntitiesByIds(couponIds)
activate Retriever
Retriever->>DB: SELECT * FROM coupons WHERE id IN (...)
DB-->>Retriever: CouponEntity list
Retriever-->>Service: entities
deactivate Retriever
Service->>Service: validNotFoundCoupon(entities, requestedIds)
alt 일부 누락
Service-->>Controller: AdminApiException (NOT_FOUND_COUPON)
Controller-->>Admin: 404 Not Found
else 모두 존재
loop 각 엔티티
Service->>Service: entity.updateMemo(memo)
end
Service->>DB: commit
Service-->>Controller: 성공 응답
Controller-->>Admin: 200 OK (BaseResponse)
end
end
deactivate Service
deactivate Controller
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 분
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/permitseoul/permitserver/domain/coupon/core/component/CouponRetriever.java (1)
33-33: 불필요한 세미콜론을 제거하세요.닫는 중괄호 뒤에 세미콜론이 불필요합니다.
if(!couponRepository.existsByCouponCodeAndUsedFalse(couponCode)) { throw new CouponConflictException(); - }; + } }
♻️ Duplicate comments (1)
src/main/java/com/permitseoul/permitserver/domain/admin/coupon/api/controller/AdminCouponController.java (1)
4-44: 쿠폰 메모 일괄 수정 API 시그니처와 사용 패턴이 기존 컨벤션과 잘 맞습니다.
PATCH /api/admin/coupons/memos라는 URL/HTTP 메서드 선택이 “메모 일괄 수정”이라는 책임과 잘 매칭됩니다.CouponMemoUpdateRequest를 그대로 받고, 서비스에는couponMemoUpdateRequest.coupons()만 전달하는 형태라 Controller 레이어가 불필요하게 비대해지지 않은 점도 좋습니다.- 응답도 기존 Admin API들과 동일하게
ApiResponseUtil.success(SuccessCode.OK)를 사용하여 일관성이 유지됩니다.DTO 쪽 Bean Validation 보완(내부 record에 대한 검증 전파,
couponId타입 등)은 DTO 파일 코멘트에서 이미 언급한 내용이라, Controller 쪽 구현 자체는 그대로 가셔도 될 것 같습니다.
🧹 Nitpick comments (3)
src/main/java/com/permitseoul/permitserver/domain/coupon/core/domain/entity/CouponEntity.java (1)
58-60: 메모 업데이트 메서드는 무난하며, 메모 null 허용 여부만 명확히 하면 좋겠습니다.엔티티 내부에서 메모를 변경하는 전용 메서드를 둔 점은 좋습니다. 다만
memo컬럼이 nullable 설정이 없고, Admin 요청 DTO에서는memo에@NotNull을 걸어 두어서,
- 메모를 null로 초기화(삭제) 하는 요구가 실제로 있는지,
- 없다면 컬럼을
nullable = false로 두고 DTO 쪽도@NotBlank등으로 더 엄격히 가져갈지정도만 팀 내에서 합의해 두면 이후 일관성 유지에 도움이 될 것 같습니다.
src/main/java/com/permitseoul/permitserver/global/response/code/ErrorCode.java (1)
89-90: 쿠폰 전용 에러코드 추가는 적절하며, 메시지 표현만 약간 통일하면 더 좋겠습니다.
40428 NOT_FOUND_COUPON,40907 CONFLICT_DUPLICATE_COUPON_ID모두 코드/HTTP Status 설계는 기존 패턴과 잘 맞습니다.- 다만 기존 메시지들이 대부분 한글 + 구체 리소스를 쓰는 편(
쿠폰코드,티켓, …)이라,
coupon을 찾을 수 없습니다.→쿠폰을 찾을 수 없습니다.중복된 쿠폰아이디 요청입니다.→중복된 쿠폰 ID 요청입니다.혹은중복된 쿠폰 식별자 요청입니다.
정도로 통일해 두면 나중에 로그/문서에서 읽기 더 수월할 것 같습니다.기능적으로는 그대로 사용해도 문제 없어 보입니다.
Also applies to: 110-111
src/main/java/com/permitseoul/permitserver/domain/admin/coupon/api/service/AdminCouponService.java (1)
29-83: 쿠폰 메모 일괄 업데이트 트랜잭션 플로우는 적절하며, 검증/가독성 측면에서 약간의 리팩터 여지가 있습니다.좋은 점:
issueCoupons에@Transactional을 붙여 발행 로직이 트랜잭션으로 묶인 점,updateCouponMemos에서
- 입력 ID 중복 검증 →
- ID→메모 Map 구성 →
- ID 리스트로 한 번에 조회 →
- 존재 여부 검증 후 일괄 업데이트
로 이어지는 플로우 자체는 깔끔하고, 업데이트도 한 트랜잭션 내에서 처리되어 일관성 측면에서 괜찮습니다.
선택적으로 고려해볼 수 있는 부분은 다음 정도입니다.
존재 검증 로직의 표현 방식
- 현재
validNotFoundCoupon은couponEntities.size() != couponMemoMap.size()만 비교하는데, 의도는 충분히 드러나지만,- 나중에 어떤
couponId가 누락됐는지 로깅/에러 메시지에 포함하고 싶다면
- 요청 ID 집합과 실제 조회된 ID 집합의 차집합을 구해 남기는 구조로 확장하기가 더 쉽습니다.
- 지금 수준에서도 기능적으로 문제는 없고, 필요 시에만 확장하면 될 듯합니다.
validNotFoundCoupon시그니처 단순화
- 현재는
Map<Long, String>과List<CouponEntity>를 모두 받지만, 실제로는 size만 사용하므로
validNotFoundCoupon(int requestSize, int foundSize)정도로 단순화하면 읽기에 조금 더 직관적일 수 있습니다.- 큰 차이는 아니므로 취향/팀 컨벤션에 따라 유지하셔도 무방합니다.
기능적 버그는 보이지 않고, 위 내용은 전부 선택적인 리팩터 수준입니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/permitseoul/permitserver/domain/admin/coupon/api/controller/AdminCouponController.java(2 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/coupon/api/dto/request/CouponMemoUpdateRequest.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/coupon/api/service/AdminCouponService.java(2 hunks)src/main/java/com/permitseoul/permitserver/domain/coupon/core/component/CouponRetriever.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/coupon/core/domain/entity/CouponEntity.java(1 hunks)src/main/java/com/permitseoul/permitserver/global/response/code/ErrorCode.java(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: sjk4618
Repo: PERMIT-SEOUL/permit-server PR: 48
File: src/main/java/com/permitseoul/permitserver/domain/guest/core/domain/entity/GuestEntity.java:31-37
Timestamp: 2025-07-15T09:37:32.765Z
Learning: sjk4618 prefers to implement factory methods or public constructors for entities when they are actually needed, rather than creating them proactively.
🧬 Code graph analysis (2)
src/main/java/com/permitseoul/permitserver/domain/admin/coupon/api/controller/AdminCouponController.java (1)
src/main/java/com/permitseoul/permitserver/global/response/ApiResponseUtil.java (1)
ApiResponseUtil(7-29)
src/main/java/com/permitseoul/permitserver/domain/admin/coupon/api/service/AdminCouponService.java (1)
src/main/java/com/permitseoul/permitserver/domain/admin/base/api/exception/AdminApiException.java (1)
RequiredArgsConstructor(8-13)
🔇 Additional comments (1)
src/main/java/com/permitseoul/permitserver/domain/coupon/core/component/CouponRetriever.java (1)
20-21: 검증 결과: 제안하신 내용은 불필요합니다. 기존 코드에 이미 검증 로직이 존재합니다.
AdminCouponService의validNotFoundCoupon메서드(79-83줄)가 요청한 모든 쿠폰 ID의 존재 여부를 검증합니다:private void validNotFoundCoupon(final Map<Long, String> couponMemoMap, final List<CouponEntity> couponEntities) { if (couponEntities.size() != couponMemoMap.size()) { throw new AdminApiException(ErrorCode.NOT_FOUND_COUPON); } }
- 요청 ID 개수(couponMemoMap.size())와 조회된 엔티티 개수(couponEntities.size())를 비교
- 불일치 시 즉시
AdminApiException발생- 존재하지 않는 ID로 인한 부분 업데이트는 발생하지 않음
따라서
CouponRetriever.findAllCouponEntitiesByIds에 추가 검증을 넣을 필요가 없습니다.Likely an incorrect or invalid review comment.
...om/permitseoul/permitserver/domain/admin/coupon/api/dto/request/CouponMemoUpdateRequest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/com/permitseoul/permitserver/domain/admin/coupon/api/dto/request/CouponMemoUpdateRequest.java (1)
11-11:@NotEmpty메시지가 불완전합니다.
@NotEmpty어노테이션은 null뿐만 아니라 빈 리스트도 검증하지만, 현재 메시지는 "null입니다"만 언급하고 있습니다. 사용자에게 정확한 오류 원인을 전달하기 위해 메시지를 수정해 주세요.다음과 같이 수정하는 것을 권장합니다:
- @NotEmpty(message = "coupon List가 null입니다.") + @NotEmpty(message = "coupon List가 null이거나 비어 있습니다.")
🧹 Nitpick comments (1)
src/main/java/com/permitseoul/permitserver/domain/admin/coupon/api/dto/request/CouponMemoUpdateRequest.java (1)
16-17: primitivelong대신Long래퍼 타입 사용을 권장합니다.현재
@Min(value = 1)을 사용한 방식도 동작하지만(JSON에서 필드가 누락되면 0으로 바인딩되어 검증 실패),Long래퍼 타입을 사용하면 더 명시적이고 관용적입니다.다음과 같이 변경할 수 있습니다:
public record CouponMemoProp( - @Min(value = 1, message = "couponId는 1 이상이어야 합니다.") - long couponId, + @NotNull(message = "couponId가 null입니다.") + @Min(value = 1, message = "couponId는 1 이상이어야 합니다.") + Long couponId, @NotNull(message = "couponMemo가 null입니다.") String memo ) { }또는
@Positive어노테이션을 사용할 수도 있습니다:public record CouponMemoProp( - @Min(value = 1, message = "couponId는 1 이상이어야 합니다.") - long couponId, + @NotNull(message = "couponId가 null입니다.") + @Positive(message = "couponId는 양수여야 합니다.") + Long couponId, @NotNull(message = "couponMemo가 null입니다.") String memo ) { }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/permitseoul/permitserver/domain/admin/coupon/api/dto/request/CouponMemoUpdateRequest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: sjk4618
Repo: PERMIT-SEOUL/permit-server PR: 48
File: src/main/java/com/permitseoul/permitserver/domain/guest/core/domain/entity/GuestEntity.java:31-37
Timestamp: 2025-07-15T09:37:32.765Z
Learning: sjk4618 prefers to implement factory methods or public constructors for entities when they are actually needed, rather than creating them proactively.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🔥Pull requests
⛳️ 작업한 브랜치
👷 작업한 내용
🚨 참고 사항