Conversation
- 기존 챌린지 인증 관련 DTO 필드, 네이밍 수정 및 불필요한 코드 제거 - 관리자용 인증 상태 업데이트, 참여자, 챌린지 제목 등 신규 DTO 추가 - 개인/팀 챌린지 인증 DTO 생성 메서드 리팩토링 - 인증 상태 enum 적용 및 관련 필드 개선
- CertificationStatus enum 추가 및 기존 Boolean 필드 대체 - 인증 상태 관리에 따른 승인/거절/수정 메서드 리팩토링 - 상태 기반의 예외 처리 및 상태 확인 메서드 추가
- 개인/팀 챌린지 인증 검색을 위한 복합 조건 및 페이징 쿼리 로직 추가 - 챌린지 그룹 코드 및 참여자 memberKey 조회 기능 구현 - 인증 상태 기반 필터링 로직 리팩토링
- 개인/팀 챌린지 제목, 그룹 코드, 참여자 조회 메서드 구현 - 관리자용 복합 조건 인증 검색 및 필터링 로직 추가 - 인증 상태 업데이트 로직 및 내부 메서드 리팩토링
- 개인/팀 챌린지 제목, 그룹 코드, 참여자 조회 엔드포인트 구현 - 복합 조건 기반 인증 검색 및 페이징 로직 추가 - 인증 상태 업데이트 메서드 구현 및 메시지 추가 - 관련 Swagger 문서 인터페이스 정의 및 주석 보완
WalkthroughA comprehensive administrative API layer for challenge certifications was introduced, including new REST controllers, DTOs, service logic, repository queries, and documentation. The certification approval logic was refactored from a boolean flag to a status enum, with corresponding changes across entities, DTOs, and repositories. Extensive filtering, pagination, and status update features for admin use were added. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin User
participant Controller as AdminChallengeCertificationController
participant Service as ChallengeCertificationService
participant Repo as CertificationRepository
Admin->>Controller: PATCH /api/admin/certifications/{id} (status update)
Controller->>Service: updateCertificationStatus(certificationId, status)
Service->>Repo: findById(certificationId)
Repo-->>Service: Certification entity
Service->>Service: validate status & update entity (approve/reject)
Service->>Repo: save changes
Service-->>Controller: void
Controller-->>Admin: ApiTemplate<Void> (success)
sequenceDiagram
participant Admin as Admin User
participant Controller as AdminChallengeCertificationController
participant Service as ChallengeCertificationService
participant Repo as CertificationRepository
Admin->>Controller: GET /api/admin/personal-certifications (with filters)
Controller->>Service: getPersonalCertificationsWithFilters(searchRequest)
Service->>Repo: findPersonalCertificationsWithFilters(searchRequest)
Repo-->>Service: CursorTemplate with DTOs
Service-->>Controller: CursorTemplate with DTOs
Controller-->>Admin: ApiTemplate<CursorTemplate<...>>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (14)
🚧 Files skipped from review as they are similar to previous changes (7)
🧰 Additional context used🧬 Code Graph Analysis (1)src/main/java/com/example/green/domain/challengecert/repository/TeamChallengeCertificationRepositoryImpl.java (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (30)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (12)
src/main/java/com/example/green/domain/challengecert/enums/CertificationStatus.java (1)
6-8: Consider more generic status naming for broader applicability.The enum structure is well-designed, but
PAIDmight be too specific for a general certification status. ConsiderAPPROVEDinstead to make it more universally applicable across different certification contexts.Also, note that the test file expects a
REQUESTEDstatus (line 111 in the test), but this enum only definesPENDING,PAID, andREJECTED.public enum CertificationStatus { - PENDING, PAID, REJECTED + PENDING, APPROVED, REJECTEDsrc/main/java/com/example/green/domain/challengecert/dto/AdminParticipantMemberKeyResponseDto.java (3)
6-13: Consider adding API documentation and simplifying the factory method.The record structure is well-defined and appropriate. However, consider these improvements:
- Add Swagger annotations for API documentation consistency with other DTOs
- The static factory method
ofis redundant since records provide constructors automaticallyApply this diff to add API documentation:
+import io.swagger.v3.oas.annotations.media.Schema; + /** * 관리자 챌린지 참여자 memberKey 응답 DTO */ +@Schema(description = "관리자 챌린지 참여자 memberKey 응답") public record AdminParticipantMemberKeyResponseDto( + @Schema(description = "회원 키", example = "google_3421") String memberKey, + @Schema(description = "닉네임", example = "사용자닉네임") String nickname ) { - public static AdminParticipantMemberKeyResponseDto of(String memberKey, String nickname) { - return new AdminParticipantMemberKeyResponseDto(memberKey, nickname); - } }Alternatively, if you prefer to keep the factory method for consistency with other DTOs, the current implementation is acceptable.
10-12: Remove redundant factory method.The static factory method
ofis unnecessary since records automatically provide a constructor with the same signature. This adds unnecessary code without any additional value.- public static AdminParticipantMemberKeyResponseDto of(String memberKey, String nickname) { - return new AdminParticipantMemberKeyResponseDto(memberKey, nickname); - }
6-9: Add field documentation and validation.Consider adding Swagger annotations and validation to improve API documentation and ensure data integrity.
public record AdminParticipantMemberKeyResponseDto( + @Schema(description = "참여자 회원 키", example = "google_3421") + @NotBlank String memberKey, + @Schema(description = "참여자 닉네임", example = "홍길동") + @NotBlank String nickname ) {src/main/java/com/example/green/domain/challengecert/dto/AdminChallengeTitleResponseDto.java (2)
9-21: Add API documentation for consistency with admin endpoints.The record structure and factory method are well-implemented and follow established patterns in the codebase. However, consider adding Swagger annotations for API documentation consistency.
Apply this diff to add API documentation:
+import io.swagger.v3.oas.annotations.media.Schema; + /** * 관리자 챌린지 제목 응답 DTO */ +@Schema(description = "관리자 챌린지 제목 응답") public record AdminChallengeTitleResponseDto( + @Schema(description = "챌린지 ID", example = "1") Long challengeId, + @Schema(description = "챌린지 명", example = "7일 운동 챌린지") String challengeName, + @Schema(description = "챌린지 타입") ChallengeType challengeType ) {
9-13: Add Swagger documentation for better API specs.Consider adding field documentation to improve the generated API documentation.
public record AdminChallengeTitleResponseDto( + @Schema(description = "챌린지 ID", example = "1") Long challengeId, + @Schema(description = "챌린지 이름", example = "환경보호 챌린지") String challengeName, + @Schema(description = "챌린지 타입") ChallengeType challengeType ) {src/main/java/com/example/green/domain/challengecert/repository/PersonalChallengeCertificationRepositoryImpl.java (1)
113-127: Consider fetch join performance implicationsMultiple fetch joins can lead to Cartesian product issues. Review if all fetch joins are necessary, particularly the member fetch join on line 117.
If member data is already loaded through the participation relationship, you might optimize by removing redundant fetch joins:
List<PersonalChallengeCertification> certifications = queryFactory .selectFrom(personalChallengeCertification) .join(personalChallengeCertification.participation, personalChallengeParticipation).fetchJoin() .join(personalChallengeParticipation.personalChallenge, personalChallenge).fetchJoin() - .join(personalChallengeParticipation.member, member).fetchJoin() + .join(personalChallengeParticipation.member, member)src/main/java/com/example/green/domain/challengecert/dto/ChallengeCertificationListResponseDto.java (1)
40-74: Consider reducing code duplication between factory methodsBoth factory methods have identical implementation logic. Consider extracting a common method that accepts the base type to eliminate duplication.
You could reduce duplication by leveraging the common base class:
+ private static ChallengeCertificationListResponseDto fromBaseCertification( + BaseChallengeCertification certification) { + return new ChallengeCertificationListResponseDto( + certification.getId(), + certification.getMember().getId(), + certification.getMember().getProfile().getNickname(), + certification.getMember().getEmail(), + certification.getCertificationImageUrl(), + certification.getCertificationReview(), + certification.getCertifiedDate(), + certification.getStatus() + ); + } + public static ChallengeCertificationListResponseDto fromPersonalChallengeCertification( PersonalChallengeCertification certification) { - - return new ChallengeCertificationListResponseDto( - certification.getId(), - certification.getMember().getId(), - certification.getMember().getProfile().getNickname(), - certification.getMember().getEmail(), - certification.getCertificationImageUrl(), - certification.getCertificationReview(), - certification.getCertifiedDate(), - certification.getStatus() - ); + return fromBaseCertification(certification); } public static ChallengeCertificationListResponseDto fromTeamChallengeCertification( TeamChallengeCertification certification) { - - return new ChallengeCertificationListResponseDto( - certification.getId(), - certification.getMember().getId(), - certification.getMember().getProfile().getNickname(), - certification.getMember().getEmail(), - certification.getCertificationImageUrl(), - certification.getCertificationReview(), - certification.getCertifiedDate(), - certification.getStatus() - ); + return fromBaseCertification(certification); }src/main/java/com/example/green/domain/challengecert/entity/BaseChallengeCertification.java (2)
61-61: Remove redundant status initializationThe
statusfield is initialized twice - once at declaration (line 61) and again in the constructor (line 76). Since the field is already initialized at declaration, the constructor assignment is redundant.protected BaseChallengeCertification( Member member, String certificationImageUrl, String certificationReview, LocalDateTime certifiedAt, LocalDate certifiedDate ) { this.member = member; this.certificationImageUrl = certificationImageUrl; this.certificationReview = certificationReview; this.certifiedAt = certifiedAt; this.certifiedDate = certifiedDate; - this.status = CertificationStatus.PENDING; }Also applies to: 76-76
115-124: Consider consolidating permission check methodsThe
canApprove()andcanReject()methods have identical implementations. If the business logic is truly the same for both operations, consider consolidating them or documenting why they're separate.If approval and rejection will always have the same preconditions:
+ /** + * 상태 변경 가능 여부 확인 + */ + public boolean canChangeStatus() { + return this.status == CertificationStatus.PENDING; + } + /** * 승인 가능 여부 확인 */ public boolean canApprove() { - return this.status == CertificationStatus.PENDING; + return canChangeStatus(); } /** * 거절 가능 여부 확인 */ public boolean canReject() { - return this.status == CertificationStatus.PENDING; + return canChangeStatus(); }This makes it clear that both operations have the same precondition and simplifies future maintenance if the logic needs to change.
src/main/java/com/example/green/domain/challengecert/service/ChallengeCertificationService.java (1)
315-336: Consider improving the certification lookup logicThe current implementation searches both repositories sequentially. Consider using a more efficient approach or at least documenting why both lookups are necessary.
You could use a more functional approach:
@Transactional public void updateCertificationStatus(Long certificationId, String status) { - // 먼저 개인 챌린지 인증에서 찾아보기 - PersonalChallengeCertification personalCertification = personalChallengeCertificationRepository - .findById(certificationId) - .orElse(null); - - if (personalCertification != null) { - updateCertificationStatusInternal(personalCertification, status); - return; - } - - // 팀 챌린지 인증에서 찾아보기 - TeamChallengeCertification teamCertification = teamChallengeCertificationRepository - .findById(certificationId) - .orElseThrow(() -> new ChallengeCertException(ChallengeCertExceptionMessage.CERTIFICATION_NOT_FOUND)); - - updateCertificationStatusInternal(teamCertification, status); + var certification = personalChallengeCertificationRepository.findById(certificationId) + .<BaseChallengeCertification>map(cert -> cert) + .or(() -> teamChallengeCertificationRepository.findById(certificationId) + .map(cert -> cert)) + .orElseThrow(() -> new ChallengeCertException(ChallengeCertExceptionMessage.CERTIFICATION_NOT_FOUND)); + + updateCertificationStatusInternal(certification, status); }src/main/java/com/example/green/domain/challengecert/dto/AdminTeamCertificationSearchRequestDto.java (1)
14-28: Consider adding validation annotations.For better data integrity, consider adding validation annotations, especially for cursor and challengeId fields.
@Schema(description = "챌린지 ID (선택사항, null이면 전체 조회)", example = "1") +@Positive Long challengeId, @Schema(description = "그룹 코드 (선택사항, null이면 전체 그룹)", example = "T-20250109-143523-C8NQ") String groupCode, @Schema(description = "인증 상태 리스트 (선택사항, null이면 전체 상태)", example = "[\"PENDING\", \"PAID\", \"REJECTED\"]") +@Valid List<CertificationStatus> statuses, @Schema(description = "커서 (페이징용, 마지막 인증 ID)") +@Positive Long cursor,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/main/java/com/example/green/domain/challengecert/controller/AdminChallengeCertificationController.java(1 hunks)src/main/java/com/example/green/domain/challengecert/controller/docs/AdminChallengeCertificationControllerDocs.java(1 hunks)src/main/java/com/example/green/domain/challengecert/controller/message/ChallengeCertificationResponseMessage.java(1 hunks)src/main/java/com/example/green/domain/challengecert/dto/AdminCertificationStatusUpdateRequestDto.java(1 hunks)src/main/java/com/example/green/domain/challengecert/dto/AdminChallengeTitleResponseDto.java(1 hunks)src/main/java/com/example/green/domain/challengecert/dto/AdminGroupCodeResponseDto.java(1 hunks)src/main/java/com/example/green/domain/challengecert/dto/AdminParticipantMemberKeyResponseDto.java(1 hunks)src/main/java/com/example/green/domain/challengecert/dto/AdminPersonalCertificationSearchRequestDto.java(1 hunks)src/main/java/com/example/green/domain/challengecert/dto/AdminTeamCertificationSearchRequestDto.java(1 hunks)src/main/java/com/example/green/domain/challengecert/dto/ChallengeCertificationDetailResponseDto.java(1 hunks)src/main/java/com/example/green/domain/challengecert/dto/ChallengeCertificationListResponseDto.java(2 hunks)src/main/java/com/example/green/domain/challengecert/entity/BaseChallengeCertification.java(4 hunks)src/main/java/com/example/green/domain/challengecert/enums/CertificationStatus.java(1 hunks)src/main/java/com/example/green/domain/challengecert/exception/ChallengeCertExceptionMessage.java(1 hunks)src/main/java/com/example/green/domain/challengecert/repository/PersonalChallengeCertificationRepositoryCustom.java(2 hunks)src/main/java/com/example/green/domain/challengecert/repository/PersonalChallengeCertificationRepositoryImpl.java(2 hunks)src/main/java/com/example/green/domain/challengecert/repository/TeamChallengeCertificationRepositoryCustom.java(2 hunks)src/main/java/com/example/green/domain/challengecert/repository/TeamChallengeCertificationRepositoryImpl.java(2 hunks)src/main/java/com/example/green/domain/challengecert/service/ChallengeCertificationService.java(4 hunks)src/test/java/com/example/green/domain/challengecert/repository/PersonalChallengeCertificationRepositoryImplTest.java(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
src/main/java/com/example/green/domain/challengecert/dto/AdminParticipantMemberKeyResponseDto.java (1)
src/main/java/com/example/green/domain/challenge/controller/dto/admin/AdminChallengeParticipantListResponseDto.java (1)
AdminChallengeParticipantListResponseDto(9-39)
src/main/java/com/example/green/domain/challengecert/entity/BaseChallengeCertification.java (1)
src/main/java/com/example/green/domain/challengecert/exception/ChallengeCertException.java (1)
ChallengeCertException(6-11)
src/main/java/com/example/green/domain/challengecert/dto/AdminChallengeTitleResponseDto.java (1)
src/main/java/com/example/green/domain/challenge/controller/dto/admin/AdminChallengeDetailResponseDto.java (1)
AdminChallengeDetailResponseDto(13-91)
src/main/java/com/example/green/domain/challengecert/repository/TeamChallengeCertificationRepositoryImpl.java (1)
src/main/java/com/example/green/domain/challengecert/repository/PersonalChallengeCertificationRepositoryImpl.java (1)
Repository(26-171)
src/main/java/com/example/green/domain/challengecert/repository/PersonalChallengeCertificationRepositoryImpl.java (1)
src/main/java/com/example/green/domain/challengecert/repository/TeamChallengeCertificationRepositoryImpl.java (1)
Repository(28-173)
🪛 GitHub Actions: PR Check
src/test/java/com/example/green/domain/challengecert/repository/PersonalChallengeCertificationRepositoryImplTest.java
[error] 104-177: Compilation error: cannot find symbol methods certificationId(), challengeId(), challengeTitle(), status() with variable REQUESTED in CertificationStatus in ChallengeCertificationListResponseDto.
🔇 Additional comments (26)
src/main/java/com/example/green/domain/challengecert/dto/AdminCertificationStatusUpdateRequestDto.java (1)
10-14: LGTM! Well-designed DTO with proper validation.The record structure is clean and appropriate for this use case. The
@NotNullvalidation ensures data integrity, and the documentation clearly explains the purpose.src/main/java/com/example/green/domain/challengecert/exception/ChallengeCertExceptionMessage.java (2)
13-13: Good refactoring to support the new status system.Renaming from
CERTIFICATION_ALREADY_APPROVEDtoCERTIFICATION_ALREADY_PROCESSEDis more appropriate with the new enum-based status system where certifications can be either approved (PAID) or rejected.
18-18: Well-added validation support for the new status system.The new
INVALID_CERTIFICATION_STATUSexception message provides proper validation feedback for the admin status update functionality.src/main/java/com/example/green/domain/challengecert/dto/AdminGroupCodeResponseDto.java (1)
6-14: LGTM! Clean and well-structured DTO.The record design is appropriate, and the static factory method
of()provides a convenient way to create instances. The field names are clear and self-documenting.src/main/java/com/example/green/domain/challengecert/controller/message/ChallengeCertificationResponseMessage.java (2)
14-23: LGTM! Well-organized admin response messages.The new admin-specific response message constants are well-structured with:
- Consistent
ADMIN_prefix naming pattern- Clear, descriptive Korean messages
- Good organization with comment separation
- Complete coverage of admin operations
The implementation follows established patterns and aligns perfectly with the new admin API functionality.
16-23: LGTM! Comprehensive admin response messages.The new admin response messages are well-organized, descriptive, and cover all the administrative functionality mentioned in the PR objectives. The separation with a comment is a good organizational practice.
src/main/java/com/example/green/domain/challengecert/dto/AdminTeamCertificationSearchRequestDto.java (3)
13-55: LGTM! Well-designed search request DTO with comprehensive filtering.The record is excellently structured with:
- Appropriate filtering fields: challengeId, groupCode, statuses for comprehensive search criteria
- Smart size handling: Compact constructor enforcing backend-controlled page size of 10
- Comprehensive helper methods: Clear boolean checks for filtering logic with proper null/empty handling
- Excellent documentation: Well-annotated Swagger schemas with meaningful examples
The
isAllGroups()method properly handles both null and whitespace-only strings, which is a thoughtful implementation detail.
30-33: LGTM! Good use of compact constructor.Using the compact constructor to enforce a fixed page size is a clean approach that prevents clients from overriding the backend's pagination constraints.
35-54: LGTM! Well-designed helper methods.The helper methods provide clear, readable ways to check filter conditions. The
isAllGroups()method appropriately handles both null and empty string cases.src/main/java/com/example/green/domain/challengecert/repository/TeamChallengeCertificationRepositoryCustom.java (3)
40-54: LGTM! Well-designed repository interface extension for admin operations.The new repository methods are excellently designed with:
- Clear method names: Explicitly indicate admin functionality and purpose
- Appropriate signatures: Proper use of admin DTOs and return types
- Comprehensive documentation: Detailed JavaDoc following established patterns
- Consistent design: Aligns with existing repository method patterns
Both methods properly support the admin API requirements for group code retrieval and filtered team certification queries with cursor-based pagination.
40-45: LGTM! Well-documented admin method.The method signature and documentation clearly describe the purpose and usage for retrieving group codes for admin dropdowns.
47-54: LGTM! Comprehensive filtering method design.The method signature appropriately uses the search DTO for complex filtering criteria and maintains consistency with cursor-based pagination patterns used elsewhere in the codebase.
src/main/java/com/example/green/domain/challengecert/repository/PersonalChallengeCertificationRepositoryCustom.java (1)
41-54: Well-designed admin query methodsThe new repository methods follow good design patterns with clear responsibilities:
findParticipantMemberKeysByChallengeIdprovides efficient participant lookupfindPersonalCertificationsWithFilterssupports complex filtering with paginationThe JavaDoc documentation clearly describes the purpose and parameters.
src/main/java/com/example/green/domain/challengecert/dto/AdminPersonalCertificationSearchRequestDto.java (1)
45-47: Consistent trimming in helper methodThe
isAllMembers()method trims the memberKey for the empty check, which is good. Ensure this matches the actual filtering logic in the repository implementation.src/main/java/com/example/green/domain/challengecert/dto/ChallengeCertificationDetailResponseDto.java (2)
10-20: Clean refactoring to Java recordExcellent refactoring from a builder-based class to a Java record. The shift from challenge-centric to member-centric fields aligns well with administrative needs, and the
CertificationStatusenum provides better type safety than the previous boolean field.
22-52: Null safety confirmed for Member.profileThe
profilefield is an @Embedded value object with a protected no-args constructor and is always initialized in both cases:
- When creating a new Member (via
Member.create), the constructor setsthis.profileusing the builder.- When loading from the database, JPA instantiates the embeddable via its no-args constructor before populating its fields.
Therefore calls to
member.getProfile().getNickname()cannot result in a null-pointer exception. No changes needed here.src/main/java/com/example/green/domain/challengecert/repository/PersonalChallengeCertificationRepositoryImpl.java (2)
95-107: Efficient participant lookup queryGood use of QueryDSL projections for efficient DTO mapping. The
distinct()ensures unique participants are returned.
149-170: Well-structured query condition buildersThe helper methods follow QueryDSL best practices:
- Returning null for empty conditions (ignored by QueryDSL)
- Proper string trimming for memberKey
- Clear separation of concerns
src/main/java/com/example/green/domain/challengecert/repository/TeamChallengeCertificationRepositoryImpl.java (3)
97-107: Clean group code query implementationGood use of projections to efficiently map team challenge group data to DTOs.
151-172: Consistent condition builders across repositoriesThe condition helper methods maintain consistency with the personal challenge repository implementation, which is excellent for maintainability.
113-128: Performance risk: deep fetch-join in findTeamCertificationsWithFiltersThis single query traverses six joins (five with
fetchJoin), which can easily blow up into Cartesian products and slow down large datasets. I didn’t find any existing batch-fetch, query-hint, or entity-graph configurations in the codebase, so consider one of the following optimizations:• Introduce a
@BatchSizeor XML<batch-size>on the related associations.
• Apply@EntityGraphon the repository method to load only the needed paths.
• Use@QueryHints({ @QueryHint(name = "org.hibernate.fetchSize", value = "…") })to tune fetch sizes.
• Break this into two simpler queries—first load certification IDs by pagination, then batch-fetch the associations.
• Switch to a projection (DTO) query if only read-only data is needed.Please verify which approach best fits your use case and add the corresponding configuration.
src/main/java/com/example/green/domain/challengecert/dto/ChallengeCertificationListResponseDto.java (1)
12-38: Well-structured DTO with improved type safetyThe refactoring from a Boolean
approvedfield to theCertificationStatusenum improves type safety and provides more expressive status representations. The addition of member information fields enhances the DTO's utility for administrative views.src/main/java/com/example/green/domain/challengecert/entity/BaseChallengeCertification.java (1)
82-97: Well-implemented status transition methodsThe
approve()andreject()methods properly enforce state transitions by only allowing changes from PENDING status. The use of a genericCERTIFICATION_ALREADY_PROCESSEDexception for both cases is appropriate.src/main/java/com/example/green/domain/challengecert/controller/docs/AdminChallengeCertificationControllerDocs.java (1)
23-200: Excellent API documentationThe documentation is comprehensive and well-structured, with:
- Clear operation descriptions and usage flows
- Detailed parameter explanations with examples
- Helpful cross-references between related endpoints
- Well-explained pagination patterns
This level of documentation will significantly improve the developer experience for API consumers.
src/main/java/com/example/green/domain/challengecert/controller/AdminChallengeCertificationController.java (1)
106-113: Enum-to-String Conversion VerifiedThe
CertificationStatus.name()values (PENDING,PAID,REJECTED) exactly match thecaselabels inChallengeCertificationService.updateCertificationStatusInternal, so using.name()here is safe and requires no changes.src/main/java/com/example/green/domain/challengecert/dto/AdminChallengeTitleResponseDto.java (1)
14-20: LGTM! Good use of factory method.The
fromfactory method provides semantic clarity for entity-to-DTO conversion and is appropriate here, unlike simple parameter passing.
...a/com/example/green/domain/challengecert/dto/AdminPersonalCertificationSearchRequestDto.java
Outdated
Show resolved
Hide resolved
| private void updateCertificationStatusInternal(Object certification, String status) { | ||
| switch (status.toUpperCase()) { | ||
| case "PAID" -> { | ||
| if (certification instanceof PersonalChallengeCertification personal) { | ||
| personal.approve(); | ||
| } else if (certification instanceof TeamChallengeCertification team) { | ||
| team.approve(); | ||
| } | ||
| } | ||
| case "REJECTED" -> { | ||
| if (certification instanceof PersonalChallengeCertification personal) { | ||
| personal.reject(); | ||
| } else if (certification instanceof TeamChallengeCertification team) { | ||
| team.reject(); | ||
| } | ||
| } | ||
| default -> throw new ChallengeCertException(ChallengeCertExceptionMessage.INVALID_CERTIFICATION_STATUS); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety in status update logic
Using Object type and repeated instanceof checks reduces type safety and creates code duplication.
Since both PersonalChallengeCertification and TeamChallengeCertification extend BaseChallengeCertification which has the approve() and reject() methods, use the base type:
-private void updateCertificationStatusInternal(Object certification, String status) {
+private void updateCertificationStatusInternal(BaseChallengeCertification certification, String status) {
switch (status.toUpperCase()) {
- case "PAID" -> {
- if (certification instanceof PersonalChallengeCertification personal) {
- personal.approve();
- } else if (certification instanceof TeamChallengeCertification team) {
- team.approve();
- }
- }
- case "REJECTED" -> {
- if (certification instanceof PersonalChallengeCertification personal) {
- personal.reject();
- } else if (certification instanceof TeamChallengeCertification team) {
- team.reject();
- }
- }
+ case "PAID" -> certification.approve();
+ case "REJECTED" -> certification.reject();
default -> throw new ChallengeCertException(ChallengeCertExceptionMessage.INVALID_CERTIFICATION_STATUS);
}
}This eliminates the type checks and code duplication while maintaining the same functionality.
📝 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.
| private void updateCertificationStatusInternal(Object certification, String status) { | |
| switch (status.toUpperCase()) { | |
| case "PAID" -> { | |
| if (certification instanceof PersonalChallengeCertification personal) { | |
| personal.approve(); | |
| } else if (certification instanceof TeamChallengeCertification team) { | |
| team.approve(); | |
| } | |
| } | |
| case "REJECTED" -> { | |
| if (certification instanceof PersonalChallengeCertification personal) { | |
| personal.reject(); | |
| } else if (certification instanceof TeamChallengeCertification team) { | |
| team.reject(); | |
| } | |
| } | |
| default -> throw new ChallengeCertException(ChallengeCertExceptionMessage.INVALID_CERTIFICATION_STATUS); | |
| } | |
| } | |
| private void updateCertificationStatusInternal(BaseChallengeCertification certification, String status) { | |
| switch (status.toUpperCase()) { | |
| case "PAID" -> certification.approve(); | |
| case "REJECTED" -> certification.reject(); | |
| default -> throw new ChallengeCertException(ChallengeCertExceptionMessage.INVALID_CERTIFICATION_STATUS); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/example/green/domain/challengecert/service/ChallengeCertificationService.java
around lines 341 to 359, the method updateCertificationStatusInternal uses
Object type and multiple instanceof checks causing code duplication and reduced
type safety. Change the method parameter type from Object to
BaseChallengeCertification, then call approve() or reject() directly on this
base type without any instanceof checks. This simplifies the code, removes
duplication, and leverages polymorphism since both
PersonalChallengeCertification and TeamChallengeCertification inherit from
BaseChallengeCertification.
.../green/domain/challengecert/repository/PersonalChallengeCertificationRepositoryImplTest.java
Outdated
Show resolved
Hide resolved
- 페이지 크기 필드 제거 및 백엔드 고정값 명시 - DTO 주석에 페이지 크기 고정 여부 설명 추가
- `pageSize` 파라미터 도입으로 인증 검색 메서드 리팩토링
- 기존 Boolean 필드를 CertificationStatus enum으로 대체 - 테스트 DTO와 메서드의 필드 네이밍 변경 및 상태 기반 검증 로직 추가
Unit Test Coverage Report
|
1 participant
#️⃣ 연관된 이슈
closed #153
📝 작업 내용
관리자가 챌린지 인증을 효율적으로 관리할 수 있는 API를 구현했습니다.
개인/팀 챌린지 인증 목록을 복합 조건으로 조회하고, 인증 상태를 관리할 수 있는 기능을 제공합니다.
🔵 개인 챌린지 인증 관리
🟡 팀 챌린지 인증 관리
공통 기능
💬 리뷰 요구사항(선택)
📌 PR 진행 시 참고사항
P1: 꼭 반영해 주세요 (Request Changes) – 이슈나 취약점 관련P2: 반영을 고려해 주세요 (Comment) – 개선 의견P3: 단순 제안 (Chore)Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Refactor