feat: 스터디 히스토리 V2 도메인 구현 #876
Conversation
Walkthrough이번 PR은 새로운 StudyHistoryV2 클래스가 추가된 변경사항을 포함합니다. 해당 클래스는 BaseEntity를 상속하며 JPA 엔티티로 study_history_v2 테이블과 매핑됩니다. 클래스는 id, status(StudyHistoryStatus), repositoryLink, Member 및 StudyV2 엔티티와의 연관 관계 필드를 포함합니다. 또한, 생성, repositoryLink 업데이트, 완료 처리, 완료 취소 및 완료 여부 확인 메서드를 제공하며, 엔티티 제거 전 StudyApplyCanceledEvent 이벤트를 등록합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant SH as StudyHistoryV2
participant E as Event System
C->>SH: create(member, study)
SH-->>C: 인스턴스 반환
C->>SH: updateRepositoryLink(link)
C->>SH: complete()
SH->>E: StudyCompletion 이벤트 등록
C->>SH: withdrawCompletion()
SH->>E: CompletionWithdrawn 이벤트 등록
C->>SH: Entity 제거
SH->>E: StudyApplyCanceled 이벤트 등록
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/StudyHistoryV2.java(1 hunks)
🔇 Additional comments (2)
src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/StudyHistoryV2.java (2)
32-34: 유니크 제약 조건에 대한 예외 처리 추가 필요member_id와 study_v2_id의 유니크 제약 조건 위반 시 발생하는 예외에 대한 적절한 처리가 필요합니다. 사용자에게 의미 있는 에러 메시지를 전달하기 위해 예외 처리 로직을 추가하는 것이 좋습니다.
48-54: 연관 관계의 cascade 설정 검토 필요
@ManyToOne관계에서 cascade 옵션이 명시되어 있지 않습니다. StudyV2나 Member 엔티티의 생명주기가 StudyHistoryV2에 영향을 받아야 하는지 검토가 필요합니다.
| public static StudyHistoryV2 create(Member student, StudyV2 study) { | ||
| return StudyHistoryV2.builder().student(student).study(study).build(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
파라미터 유효성 검증 추가 필요
create 메소드에서 student와 study 파라미터의 null 체크가 누락되어 있습니다. 잠재적인 NullPointerException을 방지하기 위해 검증 로직을 추가하는 것이 좋습니다.
다음과 같이 수정을 제안합니다:
public static StudyHistoryV2 create(Member student, StudyV2 study) {
+ if (student == null || study == null) {
+ throw new IllegalArgumentException("student와 study는 null이 될 수 없습니다.");
+ }
return StudyHistoryV2.builder().student(student).study(study).build();
}📝 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.
| public static StudyHistoryV2 create(Member student, StudyV2 study) { | |
| return StudyHistoryV2.builder().student(student).study(study).build(); | |
| } | |
| public static StudyHistoryV2 create(Member student, StudyV2 study) { | |
| if (student == null || study == null) { | |
| throw new IllegalArgumentException("student와 study는 null이 될 수 없습니다."); | |
| } | |
| return StudyHistoryV2.builder().student(student).study(study).build(); | |
| } |
| public void withdrawCompletion() { | ||
| studyHistoryStatus = NONE; | ||
| registerEvent(new StudyHistoryCompletionWithdrawnEvent(this.id)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
상태 변경 검증 로직 추가 필요
withdrawCompletion 메소드에서 현재 상태가 COMPLETED가 아닌 경우에 대한 처리가 누락되어 있습니다. 잘못된 상태에서의 철회를 방지하기 위한 검증이 필요합니다.
다음과 같이 수정을 제안합니다:
public void withdrawCompletion() {
+ if (!isCompleted()) {
+ throw new IllegalStateException("완료되지 않은 스터디는 철회할 수 없습니다.");
+ }
studyHistoryStatus = NONE;
registerEvent(new StudyHistoryCompletionWithdrawnEvent(this.id));
}📝 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.
| public void withdrawCompletion() { | |
| studyHistoryStatus = NONE; | |
| registerEvent(new StudyHistoryCompletionWithdrawnEvent(this.id)); | |
| } | |
| public void withdrawCompletion() { | |
| if (!isCompleted()) { | |
| throw new IllegalStateException("완료되지 않은 스터디는 철회할 수 없습니다."); | |
| } | |
| studyHistoryStatus = NONE; | |
| registerEvent(new StudyHistoryCompletionWithdrawnEvent(this.id)); | |
| } |
| public void complete() { | ||
| studyHistoryStatus = COMPLETED; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
상태 변경 검증 로직 추가 필요
complete 메소드에서 현재 상태가 이미 COMPLETED인 경우에 대한 처리가 누락되어 있습니다. 중복 완료 처리를 방지하기 위한 검증이 필요합니다.
다음과 같이 수정을 제안합니다:
public void complete() {
+ if (isCompleted()) {
+ throw new IllegalStateException("이미 완료된 스터디입니다.");
+ }
studyHistoryStatus = COMPLETED;
}📝 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.
| public void complete() { | |
| studyHistoryStatus = COMPLETED; | |
| } | |
| public void complete() { | |
| if (isCompleted()) { | |
| throw new IllegalStateException("이미 완료된 스터디입니다."); | |
| } | |
| studyHistoryStatus = COMPLETED; | |
| } |
|
|
||
| @Builder(access = AccessLevel.PRIVATE) | ||
| private StudyHistoryV2(Member student, StudyV2 study) { | ||
| this.studyHistoryStatus = NONE; |
There was a problem hiding this comment.
빌더에서는 전달된 값을 그대로 할당
정팩메에서 디폴트 값을 지정하도록 수정해야 합니다
|
|
||
| @Comment("수료 상태") | ||
| @Enumerated(EnumType.STRING) | ||
| private StudyHistoryStatus studyHistoryStatus; |
There was a problem hiding this comment.
| private StudyHistoryStatus studyHistoryStatus; | |
| private StudyHistoryStatus status; |
There was a problem hiding this comment.
수정하는 김에 #850 이것도 반영했어요
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistory.java(4 hunks)
🔇 Additional comments (2)
src/main/java/com/gdschongik/gdsc/domain/study/domain/StudyHistory.java (2)
52-57: 생성자 구현이 적절합니다!빌더 패턴과 private 접근 제어자를 사용하여 객체 생성을 적절히 제한하고 있습니다.
59-61: 팩토리 메서드 구현이 적절합니다!정적 팩토리 메서드를 통해 초기 상태를
NONE으로 설정하는 것이 적절합니다.
| private String repositoryLink; | ||
|
|
||
| @Comment("수료 상태") | ||
| @Enumerated(EnumType.STRING) | ||
| private StudyHistoryStatus studyHistoryStatus; | ||
|
|
||
| private String repositoryLink; | ||
| private StudyHistoryStatus status; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
repositoryLink 필드에 JPA 어노테이션 추가 필요
데이터베이스 매핑을 명시적으로 하기 위해 다음과 같은 개선이 필요합니다:
- private String repositoryLink;
+ @Column(name = "repository_link")
+ @Comment("레포지토리 링크")
+ private String repositoryLink;📝 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 String repositoryLink; | |
| @Comment("수료 상태") | |
| @Enumerated(EnumType.STRING) | |
| private StudyHistoryStatus studyHistoryStatus; | |
| private String repositoryLink; | |
| private StudyHistoryStatus status; | |
| @Column(name = "repository_link") | |
| @Comment("레포지토리 링크") | |
| private String repositoryLink; | |
| @Comment("수료 상태") | |
| @Enumerated(EnumType.STRING) | |
| private StudyHistoryStatus status; |
| public void complete() { | ||
| studyHistoryStatus = COMPLETED; | ||
| status = COMPLETED; | ||
| } | ||
|
|
||
| /** | ||
| * 스터디 수료 철회 | ||
| */ | ||
| public void withdrawCompletion() { | ||
| studyHistoryStatus = NONE; | ||
| status = NONE; | ||
| registerEvent(new StudyHistoryCompletionWithdrawnEvent(this.id)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
상태 변경 시 유효성 검사 및 이벤트 처리 개선 필요
상태 변경 메서드에 다음과 같은 개선이 필요합니다:
- 상태 변경 전 유효성 검사
complete()메서드에도 이벤트 등록 추가
public void complete() {
+ if (status == COMPLETED) {
+ throw new IllegalStateException("이미 수료 완료된 스터디입니다.");
+ }
status = COMPLETED;
+ registerEvent(new StudyHistoryCompletedEvent(this.id));
}
public void withdrawCompletion() {
+ if (status != COMPLETED) {
+ throw new IllegalStateException("수료 상태가 아닌 스터디는 철회할 수 없습니다.");
+ }
status = NONE;
registerEvent(new StudyHistoryCompletionWithdrawnEvent(this.id));
}
|
|
||
| @PreRemove | ||
| private void preRemove() { | ||
| registerEvent(new StudyApplyCanceledEvent(this.study.getId(), this.student.getId())); |
There was a problem hiding this comment.
아 리뷰 반영을 V1에 반영함..
수정하고 머지할게요
|
그리고 기존 엔티티 순서는 그대로 냅둬도 될듯요 |
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
🌱 관련 이슈
📌 작업 내용 및 특이사항
📝 참고사항
📚 기타
Summary by CodeRabbit