Feature/KD-49 GraduationUser 승인 API, 나의 정보 조회 API 구현#293
Feature/KD-49 GraduationUser 승인 API, 나의 정보 조회 API 구현#293JangYeongHu merged 17 commits intodevelopfrom
Conversation
JangYeongHu
had a problem deploying
to
coverage
GitHub Actions
Failure
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough이 변경사항은 졸업 대상자 관리 시스템에 배치 승인 기능을 추가하고, 증서/논문 승인 기능을 구현하며, API 계층을 리팩토링하고, 엑셀 생성 로직을 개선합니다. 또한 여러 도메인 서비스에 쿼리 서비스와 예외 처리를 확대합니다. Changes
Estimated code review effort🎯 4 (복잡함) | ⏱️ ~60분 특별 주의 사항:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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: 11
🧹 Nitpick comments (19)
aics-domain/src/main/java/kgu/developers/domain/thesis/domain/ThesisRepository.java (1)
10-10: Optional 반환 타입을 개선하세요.
Optional<Boolean>은 세 가지 상태(empty, true, false)를 가질 수 있어 혼란을 야기할 수 있습니다. 차라리findByIdAndDeletedAtIsNull로 전체 엔티티를 조회한 후isApproved()를 호출하거나, 존재하지 않을 경우 예외를 던지고Boolean을 반환하는 방식이 더 명확합니다.다음과 같이 리팩토링을 고려하세요:
- Optional<Boolean> findApprovalByIdAndDeletedAtIsNull(Long id);이 메서드를 제거하고 호출부에서 다음과 같이 사용:
Thesis thesis = thesisRepository.findByIdAndDeletedAtIsNull(id) .orElseThrow(() -> new ThesisNotFoundException()); return thesis.isApproved();aics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/response/GraduationUserSummaryResponse.java (1)
47-57: DateTimeFormatter를 static 상수로 추출 고려
DateTimeFormatter는 스레드 안전하므로 메서드 호출 시마다 생성하는 대신 static 상수로 추출하면 성능을 개선할 수 있습니다.@Builder public record GraduationUserSummaryResponse( // ... fields ... ) { + private static final DateTimeFormatter DATE_FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd"); + public static GraduationUserSummaryResponse of(GraduationUser graduationUser, GraduationUserStatusResponse status) { - DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd"); - return GraduationUserSummaryResponse.builder() .id(graduationUser.getId()) .studentId(graduationUser.getUserId()) .name(graduationUser.getName()) - .graduationDate(graduationUser.getGraduationDate().format(formatter)) + .graduationDate(graduationUser.getGraduationDate().format(DATE_FORMATTER)) .graduationType(graduationUser.getGraduationType() != null ? graduationUser.getGraduationType().getDescription() : "미정") .status(status) .build(); } }aics-domain/src/main/java/kgu/developers/domain/thesis/application/command/ThesisCommandService.java (2)
15-17: @transactional 어노테이션 누락
CertificateCommandService와 달리 이 서비스에는@Transactional어노테이션이 없습니다.approve()메서드에서 조회와 저장이 함께 이루어지므로 트랜잭션 관리가 필요합니다.@Service +@Transactional @RequiredArgsConstructor public class ThesisCommandService {
32-40: 들여쓰기 불일치기존 코드는 탭을 사용하지만, 새로 추가된
approve()메서드는 스페이스를 사용합니다. 코드 스타일 일관성을 위해 탭으로 통일해주세요.aics-domain/src/main/java/kgu/developers/domain/graduationUser/application/command/GraduationUserCommandService.java (1)
54-60: switch 문에 default 케이스 고려현재
ThesisTypeenum에 대해 exhaustive switch를 사용하고 있습니다. 향후 새로운 타입이 추가될 경우를 대비하여 명시적인 처리를 고려해주세요.public void updateThesis(GraduationUser graduationUser, Long thesisId, ThesisType thesisType) { switch (thesisType) { case MID_THESIS -> graduationUser.updateMidThesisId(thesisId); case FINAL_THESIS -> graduationUser.updateFinalThesisId(thesisId); + default -> throw new IllegalArgumentException("Unsupported thesis type: " + thesisType); } graduationUserRepository.save(graduationUser); }aics-domain/src/main/java/kgu/developers/domain/certificate/application/command/CertificateCommandService.java (1)
34-42: 로직은 정확하나 들여쓰기 불일치
approve()메서드의 로직은 올바릅니다. 다만, 기존 코드는 탭을 사용하지만 이 메서드는 스페이스를 사용하고 있어 일관성이 없습니다.aics-domain/src/main/java/kgu/developers/domain/certificate/application/query/CertificateQueryService.java (1)
8-16: @transactional(readOnly = true) 추가 권장Query 서비스에는
@Transactional(readOnly = true)를 추가하면 읽기 전용 트랜잭션 최적화와 명시적인 의도 표현에 도움이 됩니다.UserQueryService에서도 이 패턴을 사용하고 있습니다.@Service +@Transactional(readOnly = true) @RequiredArgsConstructor public class CertificateQueryService {필요한 import 추가:
import org.springframework.transaction.annotation.Transactional;aics-domain/src/main/java/kgu/developers/domain/certificate/infrastructure/repository/CertificateRepositoryImpl.java (1)
10-30: 메서드 이름과 실제 JPA 호출 이름을 맞춰 두는 편이 더 명확합니다.
CertificateRepository.findApprovalByIdAndDeletedAtIsNull이 내부에서jpaCertificateRepository.findApprovalById(id)를 호출하고 있어, 삭제 플래그 필터링이 어디서 적용되는지 코드만 보고는 바로 이해하기 어렵습니다.가능하다면:
- JPA 레포 메서드 이름도
findApprovalByIdAndDeletedAtIsNull로 맞추거나,- 혹은 이 구현체 쪽에서
deletedAt IS NULL조건이 포함된 메서드를 호출한다는 점이 드러나게 네이밍/구조를 조금 정리해 두면, 이후 유지보수 시 혼동을 줄일 수 있을 것 같습니다.aics-domain/src/main/java/kgu/developers/domain/thesis/infrastructure/repository/JpaThesisRepository.java (1)
3-15: JPA 레포 반환 타입과 엔티티 타입의 불일치가 런타임 혼선을 줄 수 있습니다.현재
JpaThesisRepository는JpaRepository<ThesisJpaEntity, Long>를 상속하면서, 파생 쿼리 메서드findByIdAndDeletedAtIsNull의 반환 타입을Optional<Thesis>(도메인 모델)로 두고 있습니다. Spring Data JPA에서는 보통:
- 엔티티 레포지토리(
JpaRepository<ThesisJpaEntity, Long>)는ThesisJpaEntity또는 그 프로젝션을 반환하고,- 도메인 모델(
Thesis)로의 매핑은 별도 구현체(예:ThesisRepositoryImpl)에서 담당하는 패턴을 많이 사용합니다.현재처럼 도메인 클래스
Thesis를 직접 반환 타입으로 두면,
- 별도의 프로젝션/DTO 매핑 설정 없이 정상 동작하는지,
- 혹은 도메인/인프라 계층 경계를 흐리는 것은 아닌지
를 한 번 더 점검해 보는 것이 좋겠습니다. 가능하다면 이 메서드는Optional<ThesisJpaEntity>를 반환하게 두고, 도메인 변환은 상위 레포지토리 구현에서 처리하는 쪽이 안정적입니다.aics-api/src/main/java/kgu/developers/api/graduationUser/application/GraduationUserFacade.java (3)
21-26:userQueryService.getMyId()가 중복 호출됩니다.22번 줄과 24번 줄에서 동일한 메서드를 두 번 호출하고 있습니다. 이미
userId변수에 저장된 값을 재사용하세요.public void updateGraduationType(GraduationType type) { String userId = userQueryService.getMyId(); GraduationUser graduationUser = graduationUserQueryService.getByUserId(userId); - graduationUser.validateAccessPermission(userQueryService.getMyId()); + graduationUser.validateAccessPermission(userId); graduationUserCommandService.updateGraduationType(graduationUser,type); }
28-33:userQueryService.getMyId()가 중복 호출됩니다.위와 동일하게 29번 줄과 31번 줄에서 중복 호출이 발생합니다.
public void updateGraduationUserEmail(String email) { String userId = userQueryService.getMyId(); GraduationUser graduationUser = graduationUserQueryService.getByUserId(userId); - graduationUser.validateAccessPermission(userQueryService.getMyId()); + graduationUser.validateAccessPermission(userId); graduationUserCommandService.updateGraduationUserEmail(graduationUser,email); }
35-39: 읽기 전용 메서드에 대한 트랜잭션 최적화를 고려하세요.
getMyGraduationUser()는 데이터를 조회만 하지만, 클래스 레벨의@Transactional이 적용되어 쓰기 트랜잭션으로 실행됩니다.ThesisFacade와CertificateFacade처럼 클래스 레벨에@Transactional(readOnly = true)를 적용하고, 쓰기 메서드에만@Transactional을 오버라이드하는 패턴을 고려하세요.aics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/response/GraduationUserStatusResponse.java (1)
5-51: 졸업 상태 표현용 sealed interface/record 구조가 명확합니다
Certificate/Thesis(및Middle,Final)로 케이스를 분리한 설계와 Swagger 메타데이터 구성이 직관적이고, 클라이언트 입장에서도 사용하기 좋아 보입니다.다만
type을String이 아닌GraduationType같은 enum으로 두면 서버 쪽에서 오타나 잘못된 값이 들어갈 여지를 더 줄일 수 있습니다.
API 스펙상 문자열로 노출해야 한다면, 현재 구현도 충분히 괜찮고 선택 사항으로만 고려해 보시면 될 것 같습니다.aics-domain/src/testFixtures/java/mock/repository/FakeThesisRepository.java (1)
14-15: 테스트용 FakeThesisRepository에서 동기화 컬렉션/AtomicLong은 단순 List/long으로도 충분해 보입니다단일 스레드에서 동작하는 테스트용 fake 레포지토리라면
Collections.synchronizedList와AtomicLong까지 사용할 필요는 없어 보입니다. 다른Fake*Repository들과 통일성을 맞추는 측면에서도, 아래처럼 단순한 구조를 고려해 볼 수 있습니다.- private final List<Thesis> data = Collections.synchronizedList(new ArrayList<>()); - private final AtomicLong sequence = new AtomicLong(1); + private final List<Thesis> data = new ArrayList<>(); + private long sequence = 1L; ... - .id(sequence.getAndIncrement()) + .id(sequence++)테스트 코드에서 동시성 시나리오를 다루지 않는 한, 이렇게 단순화해도 기능에는 영향이 없고 가독성은 조금 더 좋아집니다.
Based on learnings, ...aics-admin/src/main/java/kgu/developers/admin/graduationUser/application/GraduationUserAdminFacade.java (3)
85-104: 하드코딩된 문자열 대신 enum 값 사용을 권장합니다.Line 103에서
"CERTIFICATE"문자열이 하드코딩되어 있습니다.GraduationTypeenum과의 일관성을 위해 enum 값을 사용하는 것이 좋습니다.- return new GraduationUserStatusResponse.Certificate("CERTIFICATE", submitted, approval); + return new GraduationUserStatusResponse.Certificate(GraduationType.CERTIFICATE.name(), submitted, approval);
106-127: 변수 네이밍이 camelCase 규칙을 위반하고 있습니다.
midThesisapproval과finalThesisapproval변수명이 camelCase 규칙을 따르지 않습니다. 또한 Line 126의"THESIS"문자열도 enum 값으로 대체하는 것이 좋습니다.private GraduationUserStatusResponse.Thesis buildThesisStatus(Long middleThesisId, Long finalThesisId) { boolean midThesisSubmitted = middleThesisId != null; - boolean midThesisapproval = false; + boolean midThesisApproval = false; if(midThesisSubmitted) { - midThesisapproval = thesisQueryService.isApproved(middleThesisId); + midThesisApproval = thesisQueryService.isApproved(middleThesisId); } GraduationUserStatusResponse.Thesis.Middle midStatus = - new GraduationUserStatusResponse.Thesis.Middle(midThesisSubmitted,midThesisapproval); + new GraduationUserStatusResponse.Thesis.Middle(midThesisSubmitted, midThesisApproval); boolean finalThesisSubmitted = finalThesisId != null; - boolean finalThesisapproval = false; + boolean finalThesisApproval = false; if(finalThesisSubmitted) { - finalThesisapproval = thesisQueryService.isApproved(finalThesisId); + finalThesisApproval = thesisQueryService.isApproved(finalThesisId); } GraduationUserStatusResponse.Thesis.Final finalStatus = - new GraduationUserStatusResponse.Thesis.Final(finalThesisSubmitted,finalThesisapproval); + new GraduationUserStatusResponse.Thesis.Final(finalThesisSubmitted, finalThesisApproval); - return new GraduationUserStatusResponse.Thesis("THESIS", midStatus, finalStatus); + return new GraduationUserStatusResponse.Thesis(GraduationType.THESIS.name(), midStatus, finalStatus); }
163-194:graduationType이 null인 경우의 처리가 누락되어 있습니다.현재 로직에서
graduationType이CERTIFICATE나THESIS가 아닌 경우(예: null) 해당 사용자는 어떠한 처리 없이 무시됩니다. 이는 의도된 동작일 수 있으나, 명시적으로 처리하거나 로깅을 추가하는 것이 좋습니다.또한 변수명
midThesisapproved,finalThesisapproved도 camelCase 규칙을 따라야 합니다.@Transactional public GraduationUserBatchApproveResponse approveGraduationUsers(GraduationUserBatchApproveRequest request) { List<GraduationUser> users = request.ids().stream() .map(graduationUserQueryService::getById) .toList(); List<Long> approvedUserIds = new ArrayList<>(); for(GraduationUser user: users) { + if(user.getGraduationType() == null) { + continue; + } + if(user.getGraduationType() == GraduationType.CERTIFICATE) { if(user.getCertificateId() == null) continue; boolean approved = certificateCommandService.approve(user.getCertificateId()); if(approved) approvedUserIds.add(user.getId()); } else if(user.getGraduationType() == GraduationType.THESIS) { - boolean midThesisapproved = false; - boolean finalThesisapproved = false; + boolean midThesisApproved = false; + boolean finalThesisApproved = false; if(user.getMidThesisId() != null) { - midThesisapproved = thesisCommandService.approve(user.getMidThesisId()); + midThesisApproved = thesisCommandService.approve(user.getMidThesisId()); } if(user.getFinalThesisId() != null) { - finalThesisapproved = thesisCommandService.approve(user.getFinalThesisId()); + finalThesisApproved = thesisCommandService.approve(user.getFinalThesisId()); } - if(midThesisapproved || finalThesisapproved) + if(midThesisApproved || finalThesisApproved) approvedUserIds.add(user.getId()); } } return GraduationUserBatchApproveResponse.from(approvedUserIds); }aics-admin/src/testFixtures/java/graduationUser/application/GraduationUserAdminFacadeTest.java (2)
187-204: 새로 추가된approveGraduationUsers메서드에 대한 테스트가 누락되었습니다.
GraduationUserAdminFacade에 새로 추가된 배치 승인 기능(approveGraduationUsers)에 대한 테스트 케이스가 필요합니다. CERTIFICATE와 THESIS 타입 각각에 대한 승인 로직을 검증하는 테스트를 추가하는 것을 권장합니다.테스트 케이스 작성을 도와드릴까요?
142-159:getGraduationUsersByNameAndGraduationType테스트에서 status 필드 검증이 누락되었습니다.
GraduationUserSummaryResponse에 새로 추가된status필드가 올바르게 반환되는지 검증하는 assertion이 없습니다.certificateId,midThesisId,finalThesisId가 설정된 테스트 데이터를 추가하고, status 필드의submitted와approved값을 검증하는 것을 권장합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (51)
aics-admin/src/main/java/kgu/developers/admin/graduationUser/application/GraduationUserAdminFacade.java(4 hunks)aics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/GraduationUserAdminController.java(2 hunks)aics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/GraduationUserAdminControllerImpl.java(3 hunks)aics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/request/GraduationUserBatchApproveRequest.java(1 hunks)aics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/request/GraduationUserCreateRequest.java(0 hunks)aics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/response/GraduationUserBatchApproveResponse.java(1 hunks)aics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/response/GraduationUserStatusResponse.java(1 hunks)aics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/response/GraduationUserSummaryPageResponse.java(1 hunks)aics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/response/GraduationUserSummaryResponse.java(2 hunks)aics-admin/src/testFixtures/java/graduationUser/application/GraduationUserAdminFacadeTest.java(2 hunks)aics-api/src/main/java/kgu/developers/api/certificate/application/CertificateFacade.java(1 hunks)aics-api/src/main/java/kgu/developers/api/certificate/presentation/CertificateControllerImpl.java(2 hunks)aics-api/src/main/java/kgu/developers/api/graduationUser/application/GraduationUserFacade.java(2 hunks)aics-api/src/main/java/kgu/developers/api/graduationUser/presentation/GraduationUserController.java(2 hunks)aics-api/src/main/java/kgu/developers/api/graduationUser/presentation/GraduationUserControllerImpl.java(2 hunks)aics-api/src/main/java/kgu/developers/api/graduationUser/presentation/request/GraduationTypeUpdateRequest.java(0 hunks)aics-api/src/main/java/kgu/developers/api/graduationUser/presentation/response/GraduationUserStatus.java(1 hunks)aics-api/src/main/java/kgu/developers/api/graduationUser/presentation/response/MyGraduationUserResponse.java(1 hunks)aics-api/src/main/java/kgu/developers/api/thesis/application/ThesisFacade.java(1 hunks)aics-api/src/main/java/kgu/developers/api/thesis/presentation/ThesisControllerImpl.java(2 hunks)aics-api/src/main/java/kgu/developers/api/thesis/presentation/request/ThesisSubmitRequest.java(1 hunks)aics-api/src/testFixtures/java/graduationUser/application/GraduatoinUserFacadeTest.java(3 hunks)aics-domain/src/main/java/kgu/developers/domain/certificate/application/command/CertificateCommandService.java(3 hunks)aics-domain/src/main/java/kgu/developers/domain/certificate/application/query/CertificateQueryService.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/certificate/domain/Certificate.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/certificate/domain/CertificateRepository.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/certificate/exception/CertificateDomainExceptionCode.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/certificate/exception/CertificateNotFoundException.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/certificate/infrastructure/repository/CertificateRepositoryImpl.java(2 hunks)aics-domain/src/main/java/kgu/developers/domain/certificate/infrastructure/repository/JpaCertificateRepository.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/graduationUser/application/command/GraduationUserCommandService.java(3 hunks)aics-domain/src/main/java/kgu/developers/domain/graduationUser/application/query/GraduationUserQueryService.java(3 hunks)aics-domain/src/main/java/kgu/developers/domain/graduationUser/domain/GraduationType.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/graduationUser/domain/GraduationUser.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/graduationUser/domain/GraduationUserExcel.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/graduationUser/infrastructure/entity/GraduationUserJpaEntity.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/graduationUser/infrastructure/excel/GraduationUserExcelColumn.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/graduationUser/infrastructure/excel/GraduationUserExcelImpl.java(2 hunks)aics-domain/src/main/java/kgu/developers/domain/graduationUser/infrastructure/excel/GraduationUserExcelRow.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/thesis/application/command/ThesisCommandService.java(2 hunks)aics-domain/src/main/java/kgu/developers/domain/thesis/application/query/ThesisQueryService.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/thesis/domain/Thesis.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/thesis/domain/ThesisRepository.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/thesis/domain/ThesisType.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/thesis/exception/ThesisDomainExceptionCode.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/thesis/exception/ThesisNotFoundException.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/thesis/infrastructure/repository/JpaThesisRepository.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/thesis/infrastructure/repository/ThesisRepositoryImpl.java(2 hunks)aics-domain/src/main/java/kgu/developers/domain/user/application/query/UserQueryService.java(1 hunks)aics-domain/src/testFixtures/java/mock/repository/FakeCertificateRepository.java(1 hunks)aics-domain/src/testFixtures/java/mock/repository/FakeThesisRepository.java(1 hunks)
💤 Files with no reviewable changes (2)
- aics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/request/GraduationUserCreateRequest.java
- aics-api/src/main/java/kgu/developers/api/graduationUser/presentation/request/GraduationTypeUpdateRequest.java
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-01-07T02:26:40.629Z
Learnt from: LeeShinHaeng
Repo: kgu-developers/aics-server PR: 166
File: aics-domain/src/main/java/kgu/developers/domain/professor/domain/Professor.java:0-0
Timestamp: 2025-01-07T02:26:40.629Z
Learning: In the Professor entity, the image field should use VARCHAR(255) to accommodate URLs, and officeLoc field should use VARCHAR(20) for storing office location information. Additionally, image URLs should be validated using the pattern "^https?://.*$".
Applied to files:
aics-domain/src/main/java/kgu/developers/domain/graduationUser/infrastructure/entity/GraduationUserJpaEntity.java
📚 Learning: 2024-12-03T05:04:36.193Z
Learnt from: LeeShinHaeng
Repo: kgu-developers/aics-server PR: 124
File: aics-domain/src/testFixtures/java/mock/FakeLabRepository.java:28-37
Timestamp: 2024-12-03T05:04:36.193Z
Learning: `aics-domain/src/testFixtures/java/mock/FakeLabRepository.java`의 `FakeLabRepository` 클래스에서는 현재 동시성 처리가 필요하지 않으며, 다른 `FakeRepository`들과의 통일성을 위해 동기화를 추가하지 않습니다.
Applied to files:
aics-domain/src/testFixtures/java/mock/repository/FakeThesisRepository.javaaics-domain/src/testFixtures/java/mock/repository/FakeCertificateRepository.java
📚 Learning: 2024-12-03T07:09:18.535Z
Learnt from: minjo-on
Repo: kgu-developers/aics-server PR: 125
File: aics-domain/src/testFixtures/java/mock/FakeCommentRepository.java:41-45
Timestamp: 2024-12-03T07:09:18.535Z
Learning: `aics-domain/src/testFixtures/java/mock/FakeCommentRepository.java`의 `findById` 메서드에서는 데이터가 확장될 가능성이 없고 성능 차이가 크지 않으므로 Map을 사용하지 않고 List를 사용하는 것이 바람직합니다.
Applied to files:
aics-domain/src/testFixtures/java/mock/repository/FakeThesisRepository.javaaics-domain/src/testFixtures/java/mock/repository/FakeCertificateRepository.java
📚 Learning: 2024-12-03T05:01:55.233Z
Learnt from: LeeShinHaeng
Repo: kgu-developers/aics-server PR: 124
File: aics-domain/src/testFixtures/java/mock/FakeLabRepository.java:13-57
Timestamp: 2024-12-03T05:01:55.233Z
Learning: `LabServiceTest.java`에서 Java의 `BeforeEach` 애너테이션을 사용하여 각 테스트 전에 `FakeLabRepository`를 초기화하므로, `FakeLabRepository.java`에 `clear()` 메서드를 추가할 필요가 없다.
Applied to files:
aics-domain/src/testFixtures/java/mock/repository/FakeThesisRepository.javaaics-admin/src/testFixtures/java/graduationUser/application/GraduationUserAdminFacadeTest.javaaics-domain/src/testFixtures/java/mock/repository/FakeCertificateRepository.java
🧬 Code graph analysis (9)
aics-api/src/main/java/kgu/developers/api/certificate/application/CertificateFacade.java (2)
aics-api/src/main/java/kgu/developers/api/graduationUser/application/GraduationUserFacade.java (1)
Component(13-40)aics-api/src/main/java/kgu/developers/api/thesis/application/ThesisFacade.java (1)
Component(14-32)
aics-domain/src/main/java/kgu/developers/domain/thesis/application/command/ThesisCommandService.java (1)
aics-domain/src/main/java/kgu/developers/domain/thesis/exception/ThesisNotFoundException.java (1)
ThesisNotFoundException(7-11)
aics-domain/src/main/java/kgu/developers/domain/certificate/application/query/CertificateQueryService.java (3)
aics-domain/src/main/java/kgu/developers/domain/certificate/exception/CertificateNotFoundException.java (1)
CertificateNotFoundException(7-11)aics-domain/src/main/java/kgu/developers/domain/certificate/application/command/CertificateCommandService.java (1)
Service(16-43)aics-domain/src/main/java/kgu/developers/domain/thesis/application/query/ThesisQueryService.java (1)
Service(8-17)
aics-api/src/main/java/kgu/developers/api/thesis/application/ThesisFacade.java (2)
aics-api/src/main/java/kgu/developers/api/graduationUser/application/GraduationUserFacade.java (1)
Component(13-40)aics-api/src/main/java/kgu/developers/api/certificate/application/CertificateFacade.java (1)
Component(13-31)
aics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/response/GraduationUserBatchApproveResponse.java (1)
aics-domain/src/main/java/kgu/developers/domain/certificate/domain/Certificate.java (1)
Builder(11-61)
aics-domain/src/main/java/kgu/developers/domain/graduationUser/application/query/GraduationUserQueryService.java (1)
aics-domain/src/main/java/kgu/developers/domain/graduationUser/exception/GraduationUserNotFoundException.java (1)
GraduationUserNotFoundException(7-11)
aics-domain/src/main/java/kgu/developers/domain/thesis/application/query/ThesisQueryService.java (4)
aics-domain/src/main/java/kgu/developers/domain/thesis/exception/ThesisNotFoundException.java (1)
ThesisNotFoundException(7-11)aics-domain/src/main/java/kgu/developers/domain/graduationUser/application/query/GraduationUserQueryService.java (1)
Service(19-112)aics-domain/src/main/java/kgu/developers/domain/certificate/application/query/CertificateQueryService.java (1)
Service(8-17)aics-domain/src/main/java/kgu/developers/domain/thesis/application/command/ThesisCommandService.java (1)
Service(15-41)
aics-admin/src/testFixtures/java/graduationUser/application/GraduationUserAdminFacadeTest.java (6)
aics-domain/src/testFixtures/java/mock/repository/FakeCertificateRepository.java (1)
FakeCertificateRepository(13-47)aics-domain/src/testFixtures/java/mock/repository/FakeFileRepository.java (1)
FakeFileRepository(10-51)aics-domain/src/testFixtures/java/mock/repository/FakeGraduationUserRepository.java (1)
FakeGraduationUserRepository(18-100)aics-domain/src/testFixtures/java/mock/repository/FakeScheduleRepository.java (1)
FakeScheduleRepository(14-63)aics-domain/src/testFixtures/java/mock/repository/FakeThesisRepository.java (1)
FakeThesisRepository(13-46)aics-domain/src/testFixtures/java/mock/repository/FakeUserRepository.java (1)
FakeUserRepository(18-79)
aics-domain/src/main/java/kgu/developers/domain/graduationUser/application/command/GraduationUserCommandService.java (1)
aics-domain/src/main/java/kgu/developers/domain/user/exception/UserNotFoundException.java (1)
UserNotFoundException(7-11)
🔇 Additional comments (36)
aics-domain/src/main/java/kgu/developers/domain/graduationUser/infrastructure/entity/GraduationUserJpaEntity.java (1)
42-43: advisorProfessor 컬럼 nullable 전환에 따른 도메인/응답 레벨 처리 확인 요청DB 제약을 NOT NULL → NULL 허용으로 완화한 방향은 자연스러워 보입니다. 다만 이제
advisorProfessor가 null일 수 있으므로,
- 졸업 대상자 단건/페이지 조회, 상태 조회 API, 엑셀 Export 등에서 advisor 교수 미지정(null) 상태를 어떻게 표현/처리할지(예: 빈 문자열 vs
nullvs “미지정”)를 모두 일관되게 맞췄는지,- 도메인 모델
GraduationUser생성/수정 시 advisorProfessor를 필수로 가정하는 검증이 남아 있지 않은지,한 번 전체적으로 점검해 주시면 좋겠습니다. 변경 자체는 문제 없어 보입니다.
aics-domain/src/main/java/kgu/developers/domain/certificate/exception/CertificateDomainExceptionCode.java (1)
10-22: LGTM!표준적인 예외 코드 패턴을 올바르게 구현했습니다.
aics-domain/src/main/java/kgu/developers/domain/graduationUser/domain/GraduationUser.java (1)
61-71: LGTM!새로운 업데이트 메서드들이 기존 패턴과 일관되게 구현되었습니다.
aics-api/src/main/java/kgu/developers/api/certificate/presentation/CertificateControllerImpl.java (1)
5-34: LGTM!파사드 패턴을 적용하여 여러 서비스 간의 조율을 개선했습니다. 좋은 아키텍처 개선입니다.
aics-domain/src/main/java/kgu/developers/domain/thesis/infrastructure/repository/ThesisRepositoryImpl.java (1)
22-30: LGTM!JPA 레포지토리로의 단순 위임 구현이 올바릅니다.
aics-domain/src/main/java/kgu/developers/domain/thesis/domain/Thesis.java (1)
51-57: LGTM!승인 상태 조회 및 변경 메서드가 도메인 모델에 적절하게 캡슐화되었습니다.
aics-domain/src/main/java/kgu/developers/domain/certificate/infrastructure/repository/JpaCertificateRepository.java (1)
13-14: LGTM!스칼라 프로젝션을 위한
@Query사용이 적절합니다. soft-delete 필터링과 함께 승인 상태만 조회하는 효율적인 쿼리입니다.aics-domain/src/main/java/kgu/developers/domain/graduationUser/domain/GraduationType.java (1)
9-9: Breaking Change: THESIS 설명 값 변경 확인 필요
THESIS의 description이 "논문"에서 "보고서"로 변경되었습니다. 이 값은 API 응답 및 Excel 내보내기에서 사용자에게 표시되므로, 기존 클라이언트나 사용자에게 영향을 줄 수 있습니다.이 변경이 의도된 것인지 확인해 주세요.
aics-domain/src/main/java/kgu/developers/domain/certificate/domain/Certificate.java (1)
53-60: LGTM!도메인 모델에
isApproved()와approve()메서드를 추가한 것이 적절합니다.ThesisCommandService의 관련 코드 스니펫에서 확인할 수 있듯이, 멱등성 체크는 서비스 계층에서 수행되므로 도메인 모델은 단순하게 유지됩니다.aics-domain/src/main/java/kgu/developers/domain/thesis/application/query/ThesisQueryService.java (1)
8-16: LGTM!
CertificateQueryService와 일관된 패턴으로 구현되었습니다. Repository를 통한 승인 상태 조회와ThesisNotFoundException예외 처리가 적절합니다.aics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/response/GraduationUserSummaryResponse.java (1)
30-45: 상태 필드 및 Schema 문서화가 잘 되었습니다.
GraduationUserStatusResponse를 통해 졸업 요건 상태를 표현하고, 상세한 JSON 예시와 함께 Schema 어노테이션을 제공한 것이 API 문서화에 도움이 됩니다.aics-domain/src/main/java/kgu/developers/domain/graduationUser/application/command/GraduationUserCommandService.java (1)
21-32: LGTM!
validateId메서드가 중복 체크와 사용자 존재 여부를 모두 검증하도록 개선되었습니다. 유효성 검증 로직이 잘 통합되어 있습니다.aics-domain/src/main/java/kgu/developers/domain/certificate/domain/CertificateRepository.java (1)
5-11: LGTM!soft-delete 패턴을 따르는 조회 메서드들이 적절하게 추가되었습니다.
ThesisRepository와 일관된 인터페이스 설계입니다.aics-domain/src/main/java/kgu/developers/domain/user/application/query/UserQueryService.java (1)
41-53: LGTM!
getMyId()메서드를 분리하여 재사용성을 높인 좋은 리팩토링입니다.me()메서드가getMyId()에 위임하도록 변경되어 코드 중복이 제거되었습니다.aics-domain/src/main/java/kgu/developers/domain/graduationUser/domain/GraduationUserExcel.java (1)
3-8: Excel 생성 시 Row 전용 타입 사용 전환 잘 되어 있습니다.
GraduationUser대신GraduationUserExcelRow를 받도록 인터페이스를 바꾼 방향이 명확하고, 메서드 시그니처도 일관성 있게 정리되었습니다. 별다른 문제 없어 보입니다.aics-domain/src/main/java/kgu/developers/domain/certificate/exception/CertificateNotFoundException.java (1)
1-11: 도메인 전용 예외 정의가 적절합니다.
CertificateNotFoundException이 공통CustomException+ 도메인 에러코드를 잘 활용하고 있어, 인증서 조회 실패 상황 표현에 적합해 보입니다.aics-domain/src/main/java/kgu/developers/domain/thesis/domain/ThesisType.java (1)
1-14: Thesis 단계 Enum 정의가 명확합니다.
MID_THESIS/FINAL_THESIS와 한글 설명 필드를 분리한 구조가 직관적이고, Lombok 조합도 문제 없어 보입니다. 이후 타입 안전한 분기 처리에도 잘 사용할 수 있겠습니다.aics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/GraduationUserAdminController.java (1)
12-17: 졸업 대상자 일괄 승인 API 시그니처가 기존 컨벤션과 잘 맞습니다.
GraduationUserBatchApproveRequest/GraduationUserBatchApproveResponse사용,@Valid @RequestBody+ Swagger 메타데이터 구성,
이 모두 다른 배치 API들과 일관되고, 추후 클라이언트 연동에도 무리가 없어 보입니다.Also applies to: 140-150
aics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/response/GraduationUserBatchApproveResponse.java (1)
1-20: 배치 승인 응답 DTO 구조가 단순하고 사용하기 좋습니다.
record+@Builder조합과from(List<Long>)팩토리로 응답 생성이 명확하게 표현되어 있고, Swagger@Schema메타도 적절합니다. 현재 요구사항 기준으로 충분해 보입니다.aics-api/src/main/java/kgu/developers/api/thesis/presentation/ThesisControllerImpl.java (1)
5-6: Facade로 의존성 정리와 파라미터 확장이 잘 반영되었습니다.컨트롤러가
ThesisFacade에만 의존하도록 정리하고,submitThesis호출 시scheduleId와 함께request.thesisType()까지 전달하도록 변경한 부분이 이번 도메인 변경 의도와 잘 맞습니다. API·도메인 사이 경계가 더 깔끔해진 것 같습니다.Also applies to: 22-23, 30-33
aics-domain/src/main/java/kgu/developers/domain/thesis/exception/ThesisDomainExceptionCode.java (1)
10-22: LGTM!프로젝트의 다른 도메인 예외 코드(예: CertificateDomainExceptionCode)와 동일한 패턴을 따르고 있으며, 구현이 정확합니다.
aics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/request/GraduationUserBatchApproveRequest.java (1)
9-14: LGTM!Jakarta Bean Validation 제약조건이 적절하게 적용되어 있고, 리스트 요소에 대한
@Positive타입 어노테이션도 올바르게 사용되었습니다.aics-domain/src/main/java/kgu/developers/domain/thesis/exception/ThesisNotFoundException.java (1)
7-11: LGTM!
GraduationUserNotFoundException와 동일한 패턴을 따르며 구현이 올바릅니다.aics-domain/src/main/java/kgu/developers/domain/graduationUser/application/query/GraduationUserQueryService.java (2)
51-77: LGTM!
determineStage메서드가 졸업 유형에 따른 단계를 명확하게 결정하며, null 체크가 적절히 수행되고 있습니다.
79-101: LGTM!
determineStatus메서드가 논문/자격증 승인 상태를 올바르게 조회하며, Optional 처리가 적절합니다.aics-api/src/main/java/kgu/developers/api/graduationUser/presentation/response/MyGraduationUserResponse.java (1)
28-43: 승인(Approval) 상태 확인 로직이 누락된 것 같습니다.PR 목표에 따르면 제출 상태와 승인 상태를 모두 반환해야 합니다. 현재 로직은 제출 여부(ID null 체크)만 확인하고, 승인 상태는 확인하지 않습니다.
GRADUATION_REQUIREMENTS_MET을 반환하기 전에 승인 상태도 확인해야 하는지 검토가 필요합니다.aics-api/src/main/java/kgu/developers/api/thesis/application/ThesisFacade.java (1)
14-31: LGTM!CertificateFacade와 일관된 구조입니다.논문 제출 워크플로우를 적절히 조율하며,
ThesisType매개변수를 통해 중간/최종 논문을 구분하는 확장성 있는 설계입니다.aics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/response/GraduationUserSummaryPageResponse.java (1)
13-43: LGTM! 도메인 객체 의존성 제거가 잘 이루어졌습니다.팩토리 메서드가
GraduationUser대신GraduationUserSummaryResponse를 직접 받도록 변경되어, 프레젠테이션 레이어에서 도메인 객체에 대한 의존성이 제거되었습니다. Swagger 예시도 새로운 상태 구조를 잘 반영하고 있습니다.aics-api/src/testFixtures/java/graduationUser/application/GraduatoinUserFacadeTest.java (2)
37-42: GraduationUserCommandService 생성자 변경 대응이 적절합니다
FakeUserRepository를 함께 주입하도록 테스트 초기화를 수정한 부분이 실제GraduationUserCommandService생성자 시그니처와 잘 맞습니다. 테스트 격리도 유지되고 있어서 추가 수정 필요 없어 보입니다.
72-78: ID 제거 후 현재 로그인 사용자 기반 Facade API 사용이 잘 반영되었습니다
updateGraduationType,updateGraduationUserEmail을 더 이상 ID 없이 호출하도록 변경하고, 검증은 여전히 ID=1L 엔티티를 직접 조회해서 수행하는 방식이라,
- Facade 내부에서 SecurityContext의 현재 사용자 → GraduationUser 매핑이 기대대로 동작하는지 테스트하고
- 기존 테스트의 검증 로직도 그대로 재사용할 수 있어서 구조가 깔끔합니다.
현재 형태 그대로 유지해도 될 것 같습니다.
Also applies to: 87-92
aics-admin/src/main/java/kgu/developers/admin/graduationUser/presentation/GraduationUserAdminControllerImpl.java (1)
97-104: 배치 삭제·승인 엔드포인트가 요청 DTO 기반으로 잘 정리되었습니다
deleteGraduationUsers가@Valid @RequestBody GraduationUserBatchDeleteRequest를 받도록 바뀌고,approveGraduationUsers가GraduationUserBatchApproveRequest를 본문으로 받는 구조라 확장성과 가독성이 좋아졌습니다.컨트롤러는 Facade로 깔끔하게 위임하고 있고, HTTP 상태코드도 200/201/204 조합이 일관적이라 현재 구현은 그대로 가져가셔도 되겠습니다.
Also applies to: 106-113
aics-domain/src/main/java/kgu/developers/domain/graduationUser/infrastructure/excel/GraduationUserExcelColumn.java (1)
11-18: GraduationUserExcelRow 기반 컬럼 정의 및 상태 컬럼 추가가 일관됩니다Excel 컬럼이
GraduationUserExcelRow의 프로퍼티 레퍼런스를 사용하도록 정리되고,"단계","상태"컬럼(CURRENT_STAGE,APPROVAL_STATUS)이 추가된 부분이 Excel 출력 요구사항(현재 단계/승인 상태 노출)에 잘 맞습니다.
valueExtractor제네릭 타입을Function<GraduationUserExcelRow, Object>로 바꾼 것도GraduationUserExcelImpl쪽 변경과 정합성이 있어서 현재 구현으로 충분해 보입니다.Also applies to: 21-23
aics-api/src/main/java/kgu/developers/api/graduationUser/presentation/GraduationUserControllerImpl.java (1)
26-31: 현재 로그인 사용자 기준 엔드포인트로의 전환과 /my 추가가 자연스럽습니다
/graduation-type,GraduationTypeUpdateRequest,GraduationUserEmailUpdateRequest만을 받아서 Facade에 위임하는 구조로 정리된 것이 현재 사용자 기반 도메인 로직과 잘 맞습니다.
GET /my에서MyGraduationUserResponse를 반환하는 부분도 단순·명확하고, 사용자 역할에 대한@PreAuthorize("hasRole('ROLE_USER')")와도 잘 어울립니다.다만 이전에
/{graduationUserId}/...경로를 사용하던 클라이언트가 있다면, 새 경로로 마이그레이션이 모두 끝났는지 한 번 더 점검해 보시면 좋겠습니다.Also applies to: 34-39, 41-46
aics-domain/src/main/java/kgu/developers/domain/graduationUser/infrastructure/excel/GraduationUserExcelImpl.java (1)
26-37: GraduationUserExcelRow 기반 generate 로직으로의 전환이 깔끔합니다
generate가List<GraduationUserExcelRow>를 받도록 바뀌고,populateDataRows도 동일 타입을 사용하면서GraduationUserExcelColumn의valueExtractor를 적용하는 구조라, 도메인 → Excel 행 모델 분리가 명확해졌습니다.헤더 스타일을 제거하고 단순히 텍스트·컬럼 폭만 설정하도록 한 것도 기능 요구사항 관점에서 문제 없고,
SXSSFWorkbook/ByteArrayOutputStream에 대한 try-with-resources 및 예외 래핑(GraduationUserExcelGenerationFailed)도 잘 유지되고 있습니다.Also applies to: 42-51, 53-65
aics-admin/src/main/java/kgu/developers/admin/graduationUser/application/GraduationUserAdminFacade.java (1)
36-45: 클래스 레벨 트랜잭션 설정 및 의존성 주입이 적절합니다.
@Transactional(readOnly = true)를 클래스 레벨에 적용하여 기본적으로 읽기 전용으로 설정하고, 변경이 필요한 메서드에만@Transactional을 명시적으로 추가한 것은 좋은 패턴입니다.aics-admin/src/testFixtures/java/graduationUser/application/GraduationUserAdminFacadeTest.java (1)
50-92: 테스트 설정이 적절하게 업데이트되었습니다.새로 추가된 서비스 의존성들(
ThesisCommandService,ThesisQueryService,CertificateCommandService,CertificateQueryService)이 올바르게 주입되었습니다.
JangYeongHu
changed the title
JangYeongHu
had a problem deploying
to
coverage
GitHub Actions
Failure
JangYeongHu
had a problem deploying
to
coverage
GitHub Actions
Failure
JangYeongHu
had a problem deploying
to
coverage
GitHub Actions
Failure
Test Coverage Report
Files
|
JangYeongHu
had a problem deploying
to
coverage
GitHub Actions
Failure
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## develop #293 +/- ##
=============================================
- Coverage 90.20% 87.50% -2.71%
- Complexity 53 65 +12
=============================================
Files 20 22 +2
Lines 194 256 +62
Branches 2 15 +13
=============================================
+ Hits 175 224 +49
- Misses 18 19 +1
- Partials 1 13 +12
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
dkdltm221
left a comment
dkdltm221
left a comment
There was a problem hiding this comment.
코드 리뷰하면서 배웠습니다!
따로 문제는 없어보이네요 고생하셨어요 👍
3 participants
Summary
GraduationUser에 대한 일괄 승인 API, 졸업 대상자 자신에 대한 현재 제출 상태 확인 API를 구현하였습니다.
Tasks