refactor: 리쿠르팅 생성 API 수정 #452
Conversation
|
Caution Review failedThe pull request is closed. Walkthrough이번 변경 사항은 리쿠르팅 생성하기 API를 개선하기 위해 다양한 수정과 추가가 이루어졌습니다. 주요 변경 사항으로는 새로운 유효성 검사 로직 추가, 기존 메서드의 로직 수정, 새로운 클래스와 메서드 도입 등이 포함됩니다. 또한, 테스트 코드가 리팩토링되어 더 나은 검증을 지원합니다. Changes
Assessment against linked issues
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? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- src/main/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentService.java (4 hunks)
- src/main/java/com/gdschongik/gdsc/domain/recruitment/dao/RecruitmentRepository.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/recruitment/dto/request/RecruitmentCreateRequest.java (1 hunks)
- src/test/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentServiceTest.java (1 hunks)
Files skipped from review due to trivial changes (1)
- src/main/java/com/gdschongik/gdsc/domain/recruitment/dto/request/RecruitmentCreateRequest.java
Additional comments not posted (11)
src/main/java/com/gdschongik/gdsc/domain/recruitment/dao/RecruitmentRepository.java (1)
10-10: 확인 필요: 주석이 해제된 메소드
existsByAcademicYearAndSemesterType메소드가 주석에서 해제되었습니다. 이 메소드가 제대로 작동하고 있는지 확인하세요.src/test/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentServiceTest.java (4)
7-12: 새로운 import 확인새로운 import가 추가되었습니다. 필요한 import인지 확인하세요.
21-33: 테스트 추가: 학기 시작일과 종료일의 연도가 입력된 학년도와 다를 경우학기 시작일과 종료일의 연도가 입력된 학년도와 다를 경우 실패하는 테스트가 추가되었습니다. 이 테스트는 새로운 검증 로직을 잘 반영하고 있습니다.
35-45: 테스트 추가: 학기 시작일과 종료일의 학기가 입력된 학기와 다를 경우학기 시작일과 종료일의 학기가 입력된 학기와 다를 경우 실패하는 테스트가 추가되었습니다. 이 테스트는 새로운 검증 로직을 잘 반영하고 있습니다.
47-62: 테스트 추가: 학년도와 학기가 모두 중복되는 리쿠르팅일 경우학년도와 학기가 모두 중복되는 리쿠르팅일 경우 실패하는 테스트가 추가되었습니다. 이 테스트는 새로운 검증 로직을 잘 반영하고 있습니다.
src/main/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentService.java (6)
8-19: 새로운 import 확인새로운 import가 추가되었습니다. 필요한 import인지 확인하세요.
36-51: 메소드 수정: createRecruitment
createRecruitment메소드가 수정되어 추가적인 검증과 엔티티 생성 로직이 포함되었습니다. 이 메소드는 새로운 요구사항을 잘 반영하고 있습니다.
61-65: 검증 로직 추가: validateRecruitmentOverlap
validateRecruitmentOverlap메소드에 중복 리쿠르팅 검증 로직이 추가되었습니다. 이 로직은 중복된 리쿠르팅을 방지합니다.
82-90: 검증 로직 수정: validatePeriodMatchesAcademicYear
validatePeriodMatchesAcademicYear메소드의 검증 로직이 수정되었습니다. 학기 시작일과 종료일의 연도가 입력된 학년도와 맞지 않으면 예외를 던집니다.
92-101: 검증 로직 수정: validatePeriodMatchesSemesterType
validatePeriodMatchesSemesterType메소드의 검증 로직이 수정되었습니다. 학기 시작일과 종료일의 학기가 입력된 학기와 맞지 않으면 예외를 던집니다.
103-123: 메소드 추가: getSemesterTypeByStartDateOrEndDate
getSemesterTypeByStartDateOrEndDate메소드가 추가되었습니다. 이 메소드는 날짜를 기반으로 학기 타입을 반환합니다. 로직이 올바른지 확인하세요.
| validatePeriodMatchesAcademicYear( | ||
| request.semesterStartDate(), request.semesterEndDate(), request.academicYear()); | ||
| validatePeriodMatchesSemesterType( | ||
| request.semesterStartDate(), request.semesterEndDate(), request.semesterType()); | ||
| validateRecruitmentOverlap(request.academicYear(), request.semesterType()); |
There was a problem hiding this comment.
이 세개는 도메인 메소드로 존재해도 상관없지않을까요?
There was a problem hiding this comment.
우리 레포에서 입력 값 검증은 대체로 서비스에서 처리하고 있어서 이 부분도 여기에 있는게 맞을 것 같습니다!
There was a problem hiding this comment.
움 사실 전 그 기준이 모호해요
저는 이전에 모든 도메인으로 들어가는 값 검증을 서비스에서 진행했거든요
하지만 여기선 도메인에서 진행하는 검증로직이 있고, 서비스에서 진행하는 로직이 있는것 같아서요...
혹시 이부분에대해서 어떠한 기준으로 진행하는지 동기화를 위해 조금만 설명해주실 수 잇을까요?
There was a problem hiding this comment.
음 이것도 이야기하자면 긴데, 결론부터 말하자면 이렇습니다
도메인 계층에서 수행되는 검증로직
- 도메인 객체 내부에 있는 상태만으로 처리할 수 있는 검증 로직인 경우 도메인 내부에서 처리합니다.
- 가령 준회원 승급 도메인 로직에 사용되는
advanceAssociate를 들 수 있습니다. 해당 메서드는 준회원으로 승급하기 전에 준회원 승급조건이 모두 충족되었는지 검증하는데요, 준회원 승급조건에 해당하는AssociateRequirement의 경우 멤버 도메인 내부 VO로 관리되고 있기 떄문에, 내부 상태로 검증될 수 있는 경우입니다.
응용 계층에서 수행되는 검증로직 (-> 도메인 서비스에서 수행되는 검증로직)
- 도메인 내부에 있는 상태만으로 처리할 수 없는 경우가 있습니다. 이 경우 응용 레이어에서 처리될 수도 있습니다.
- 가령
이번 학기에 리쿠르팅은 단 하나만 존재해야 한다같은 uniqueness에 대한 도메인 규칙의 경우 내부 상태만으로 처리할 수 없겠죠. 이 경우 응용 계층에서 validation을 수행합니다. - 단 이렇게 하면 도메인 규칙이 응용 레이어로 노출됩니다. 이걸 막기 위해서 도메인 서비스를 사용합니다 (
OrderValidator를 참고해주세요). 아직은 응용 레이어에 도메인 로직 노출된 경우가 대다수지만, 점진적으로 바꿔나가야겠죠,,,
정리
위 로직의 경우 제가 팔로업을 잘 못하던 시기에 작성된 코드라 바로 이해하기엔 어렵지만... validateRecruitmentOverlap의 경우 내부 상태만으로 검증할 수 없는, 응용 계층에서 검증되어야 하는 사례라고 할 수 있습니다. 왜냐면 A라는 리쿠르팅과 B라는 리쿠르팅은 서로 겹치지 말아야 한다 라는 도메인 규칙에 대하여, aRecruitment.validateOverlap(bRecruitment) 같이 어느 한쪽에다가 다른 한쪽이 겹치는지 검증하게 하는 건 조금 어색하니까요~ 이런 경우에 도메인 서비스를 사용하는 겁니다.
예전에 블로그 til에 허접하게 몇 자 적어둔게 있는데 참고하시면 좋을 것 같아 올려봅니다
https://www.uwoobeat.dev/til-27
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- src/main/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentService.java (4 hunks)
- src/main/java/com/gdschongik/gdsc/domain/recruitment/domain/RecruitmentValidator.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/recruitment/dto/request/RecruitmentCreateRequest.java (1 hunks)
- src/test/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentServiceTest.java (1 hunks)
- src/test/java/com/gdschongik/gdsc/domain/recruitment/domain/RecruitmentValidatorTest.java (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentService.java
- src/test/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentServiceTest.java
Additional comments not posted (3)
src/main/java/com/gdschongik/gdsc/domain/recruitment/dto/request/RecruitmentCreateRequest.java (1)
Line range hint
1-14:
변경된 필드명 확인필드
periodStartDate와periodEndDate가 각각semesterStartDate와semesterEndDate로 변경되었습니다. 변경 사항이 PR 목표와 일치하며 문제가 없습니다.src/main/java/com/gdschongik/gdsc/domain/recruitment/domain/RecruitmentValidator.java (1)
1-24: 새로운 클래스와 메서드 추가 확인새로운 클래스
RecruitmentValidator와 메서드validateRecruitmentCreate가 추가되었습니다. 이 메서드는 학년도와 학기 유형이 같은 리쿠르팅이 이미 존재하는지 검증합니다. 변경 사항이 PR 목표와 일치하며 문제가 없습니다.src/test/java/com/gdschongik/gdsc/domain/recruitment/domain/RecruitmentValidatorTest.java (1)
1-40: 새로운 테스트 클래스와 메서드 추가 확인새로운 테스트 클래스
RecruitmentValidatorTest와 중첩 클래스리쿠르팅_생성시가 추가되었습니다. 테스트 메서드는 학년도와 학기 유형이 중복되는 리쿠르팅이 존재할 때 예외를 발생시키는지 검증합니다. 변경 사항이 PR 목표와 일치하며 문제가 없습니다.
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/main/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentService.java (4 hunks)
- src/test/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentServiceTest.java (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentService.java
- src/test/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentServiceTest.java
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
| } | ||
| } | ||
|
|
||
| private LocalDateTime getSemesterStartDate(Integer academicYear, SemesterType semesterType) { |
There was a problem hiding this comment.
학기 시작일이랑 종료일은 학년도랑 학기랑 맞는지 검증하는 로직이 너무 복잡해질것 같기도 하고
시작일은 3/1과 9/1로 고정, 종료일은 다음 학기 시작일의 전날로 하면 서버에서 직접 값을 넣는 편이 간단할 것 같아서 이렇게 했어요.
어드민에서 결정할 수 있게 해야할 이유가 있을까요?
There was a problem hiding this comment.
아직 학기 시작일 / 종료일을 어떻게 사용할지 정책이 애매한 상황에서 서버에서 특정 정책을 강요하면 문제가 생길 것 같아서요.
애매한 부분은 최대한 허용해놓고 추후에 정책 생기면 반영하는 게 좋지 않을까 싶습니다
가령 준회원 강등 -> 학기 종료일 이후 즉시 실행이라는 정책이 생기면, 어드민에서 해당 값을 컨트롤하고 싶을 수 있는데, 이때 다음 학기 시작일 전날이라는 정책이 걸려있으면 문제가 생길 것 같아요
There was a problem hiding this comment.
네 그럼 일단은 특별한 정책이 없으니 Future로만 검증하도록 하고 Request에 포함하는걸로 하겠습니다
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- src/main/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentService.java (4 hunks)
- src/main/java/com/gdschongik/gdsc/domain/recruitment/domain/RecruitmentValidator.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/recruitment/dto/request/RecruitmentCreateRequest.java (1 hunks)
- src/test/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentServiceTest.java (1 hunks)
- src/test/java/com/gdschongik/gdsc/domain/recruitment/domain/RecruitmentValidatorTest.java (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- src/main/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentService.java
- src/main/java/com/gdschongik/gdsc/domain/recruitment/domain/RecruitmentValidator.java
- src/main/java/com/gdschongik/gdsc/domain/recruitment/dto/request/RecruitmentCreateRequest.java
- src/test/java/com/gdschongik/gdsc/domain/recruitment/domain/RecruitmentValidatorTest.java
Additional comments not posted (4)
src/test/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentServiceTest.java (4)
4-4: 테스트 대상 클래스 및 메서드의 imports 추가 확인필요한 상수와 클래스들이 import 되었는지 확인합니다.
8-12: 테스트 대상 클래스 및 메서드의 imports 추가 확인필요한 상수와 클래스들이 import 되었는지 확인합니다.
20-21: DI를 통한 의존성 주입 확인AdminRecruitmentService와 RecruitmentRepository가 DI를 통해 주입되었습니다.
23-38: 테스트 성공 케이스 확인리쿠르팅 생성시 성공하는 테스트 케이스가 추가되었습니다.
그러나, 삭제된 테스트 메서드들이 다른 곳에서 여전히 필요할 수 있으니 확인이 필요합니다.
Verification successful
테스트 성공 케이스 확인
리쿠르팅 생성시 성공하는 테스트 케이스가 추가되었습니다.
삭제된 테스트 메서드들이 다른 곳에서 여전히 필요할 수 있으니 확인이 필요합니다.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if the removed test methods exist elsewhere in the codebase. # Test: Search for the removed test methods. Expect: No occurrences in the codebase. rg --type java '모집회차_수정시' rg --type java '모집_시작일이_지났다면_수정_실패한다' rg --type java '기간이_중복되는_RecruitmentRound가_있다면_실패한다' rg --type java '차수가_중복되는_RecruitmentRound가_있다면_실패한다'Length of output: 1333
Script:
#!/bin/bash # Description: Verify if the removed test methods are defined elsewhere in the codebase. # Test: Search for definitions of the removed test methods. Expect: No occurrences in the codebase. rg --type java 'void 모집회차_수정시' rg --type java 'void 모집_시작일이_지났다면_수정_실패한다' rg --type java 'void 기간이_중복되는_RecruitmentRound가_있다면_실패한다' rg --type java 'void 차수가_중복되는_RecruitmentRound가_있다면_실패한다'Length of output: 644
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/main/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentService.java (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/gdschongik/gdsc/domain/recruitment/application/AdminRecruitmentService.java
| @Test | ||
| void 학년도_학기가_모두_중복되는_리쿠르팅이라면_실패한다() { | ||
| // when & then | ||
| assertThatThrownBy(() -> recruitmentValidator.validateRecruitmentCreate(true)) |
There was a problem hiding this comment.
given 절에서 어떤 값을 넘겨주는지를 명시적으로 표현하는게 좋을듯 합니다
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
🌱 관련 이슈
📌 작업 내용 및 특이사항
기존 Recruitment와 다른 점이 있다면, 모집기간 대신에 학기기간을 가지게 됩니다.따라서 [학기 시작일과 종료일]이 [학년도와 학기]에 맞는지를 검증합니다.
해당 학년도와 학기로 생성된 리쿠르팅이 이미 있는지를 검증합니다.📝 참고사항
📚 기타
Summary by CodeRabbit
새로운 기능
버그 수정
테스트