feat: 과제 제출이력 V2 도메인 구현 #937
Merged
Conversation
This file contains 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
📝 WalkthroughWalkthrough새로운 Java 클래스 Changes
Sequence Diagram(s)sequenceDiagram
participant S as Service
participant AH as AssignmentHistoryV2
S->>AH: create(StudySessionV2, Member)
AH-->>S: AssignmentHistoryV2 Instance
alt Successful Submission
S->>AH: success(submissionLink, commitHash, contentLength, committedAt)
AH-->>S: Update success state
else Failed Submission
S->>AH: fail(submissionFailureType)
AH-->>S: Throws CustomException (if invalid type)
end
Possibly related PRs
Suggested labels
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Pro Plan! Simply use the command ✨ Finishing Touches
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
|
|
/gemini review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/AssignmentHistoryV2.java (2)
88-90: 메서드 이름과 로직의 명확성을 개선해보세요.현재
isSubmitted()는submissionFailureType이NOT_SUBMITTED가 아닌지 확인합니다. 부정문을 사용한 로직은 혼란을 줄 수 있으므로, 더 직관적인 방식으로 변경하는 것이 좋습니다.다음과 같이 수정해보세요:
public boolean isSubmitted() { - return submissionFailureType != NOT_SUBMITTED; + return submissionFailureType == NONE || + (submissionFailureType != NOT_SUBMITTED && submissionFailureType != null); }또는 더 명확한 메서드 이름을 사용할 수도 있습니다:
-public boolean isSubmitted() { +public boolean hasSubmissionAttempt() { return submissionFailureType != NOT_SUBMITTED; }
107-117: 실패 유형 검증이 잘 구현되어 있습니다.유효하지 않은 실패 유형에 대한 검증이 포함되어 있는 것은 좋습니다. 다만, 검증 로직을 더 명확하게 할 수 있습니다.
다음과 같이 열거형에 유효성 검사 메서드를 추가하거나 별도의 유틸리티 메서드를 만들어서 사용할 수 있습니다:
public void fail(SubmissionFailureType submissionFailureType) { - if (submissionFailureType == NOT_SUBMITTED || submissionFailureType == NONE) { + if (!isValidFailureType(submissionFailureType)) { throw new CustomException(ASSIGNMENT_INVALID_FAILURE_TYPE); } this.submissionLink = null; this.commitHash = null; this.contentLength = null; this.committedAt = null; this.submissionStatus = FAILURE; this.submissionFailureType = submissionFailureType; } +private boolean isValidFailureType(SubmissionFailureType type) { + return type != NOT_SUBMITTED && type != NONE; +}
📜 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/AssignmentHistoryV2.java(1 hunks)
🔇 Additional comments (6)
src/main/java/com/gdschongik/gdsc/domain/studyv2/domain/AssignmentHistoryV2.java (6)
29-35: 엔티티 구조 잘 설계되었습니다.테이블 명과 유니크 제약조건이 적절하게 설정되어 있습니다.
uniqueConstraints를 사용하여 동일한 스터디 세션에 대한 멤버별 제출 중복을 방지하는 것은 좋은 설계입니다.
42-46: 열거형 타입 사용이 적절합니다.
EnumType.STRING을 사용하여 데이터베이스에 문자열로 저장하는 방식은 유지보수성을 높여주며, 코드 이해도를 높이는 좋은 방법입니다.
57-63: 지연 로딩 사용이 적절합니다.
FetchType.LAZY를 사용하여 성능 최적화를 고려한 점이 좋습니다. 다만, N+1 문제가 발생할 수 있으니 조회 시 필요한 경우 fetch join을 사용해주세요.
65-75: 생성자 접근 제한이 잘 되어 있습니다.빌더 패턴을 사용하여 객체 생성을 제한하고, 생성자를
private으로 선언한 것은 객체 생성을 통제하는 좋은 방법입니다.
77-84: 팩토리 메서드 패턴이 잘 적용되었습니다.정적 팩토리 메서드를 통해 객체 생성 과정을 캡슐화하고, 기본값을 설정하는 접근 방식이 좋습니다.
98-105: 성공 처리 메서드의 구현이 적절합니다.제출 성공 시 모든 필드가 적절하게 업데이트되고 있습니다.
submissionFailureType을NONE으로 설정하는 것도 좋은 접근입니다.
Comment on lines
+35
to
+118
| public class AssignmentHistoryV2 { | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| @Column(name = "assignment_history_v2_id") | ||
| private Long id; | ||
|
|
||
| @Enumerated(EnumType.STRING) | ||
| private AssignmentSubmissionStatus submissionStatus; | ||
|
|
||
| @Enumerated(EnumType.STRING) | ||
| private SubmissionFailureType submissionFailureType; | ||
|
|
||
| private Integer contentLength; | ||
|
|
||
| @Column(columnDefinition = "TEXT") | ||
| private String submissionLink; | ||
|
|
||
| private String commitHash; | ||
|
|
||
| private LocalDateTime committedAt; | ||
|
|
||
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "study_session_v2_id") | ||
| private StudySessionV2 studySession; | ||
|
|
||
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "member_id") | ||
| private Member member; | ||
|
|
||
| @Builder(access = AccessLevel.PRIVATE) | ||
| private AssignmentHistoryV2( | ||
| AssignmentSubmissionStatus submissionStatus, | ||
| SubmissionFailureType submissionFailureType, | ||
| StudySessionV2 studySession, | ||
| Member member) { | ||
| this.submissionStatus = submissionStatus; | ||
| this.submissionFailureType = submissionFailureType; | ||
| this.studySession = studySession; | ||
| this.member = member; | ||
| } | ||
|
|
||
| public static AssignmentHistoryV2 create(StudySessionV2 studySession, Member member) { | ||
| return AssignmentHistoryV2.builder() | ||
| .submissionStatus(FAILURE) | ||
| .submissionFailureType(NOT_SUBMITTED) | ||
| .studySession(studySession) | ||
| .member(member) | ||
| .build(); | ||
| } | ||
|
|
||
| // 데이터 조회 로직 | ||
|
|
||
| public boolean isSubmitted() { | ||
| return submissionFailureType != NOT_SUBMITTED; | ||
| } | ||
|
|
||
| public boolean isSucceeded() { | ||
| return submissionStatus == SUCCESS; | ||
| } | ||
|
|
||
| // 데이터 변경 로직 | ||
|
|
||
| public void success(String submissionLink, String commitHash, Integer contentLength, LocalDateTime committedAt) { | ||
| this.submissionLink = submissionLink; | ||
| this.commitHash = commitHash; | ||
| this.contentLength = contentLength; | ||
| this.committedAt = committedAt; | ||
| this.submissionStatus = SUCCESS; | ||
| this.submissionFailureType = NONE; | ||
| } | ||
|
|
||
| public void fail(SubmissionFailureType submissionFailureType) { | ||
| if (submissionFailureType == NOT_SUBMITTED || submissionFailureType == NONE) { | ||
| throw new CustomException(ASSIGNMENT_INVALID_FAILURE_TYPE); | ||
| } | ||
| this.submissionLink = null; | ||
| this.commitHash = null; | ||
| this.contentLength = null; | ||
| this.committedAt = null; | ||
| this.submissionStatus = FAILURE; | ||
| this.submissionFailureType = submissionFailureType; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
널 체크와 파라미터 유효성 검증을 추가해보세요.
success 및 fail 메서드에서 파라미터의 널 체크나 유효성 검증이 부족합니다. 외부에서 호출될 수 있는 메서드이므로 더 강건한 유효성 검증이 필요합니다.
다음과 같이 수정해볼 수 있습니다:
public void success(String submissionLink, String commitHash, Integer contentLength, LocalDateTime committedAt) {
+ // 필수 파라미터 널 체크
+ if (submissionLink == null || commitHash == null || committedAt == null) {
+ throw new CustomException(INVALID_INPUT_VALUE);
+ }
this.submissionLink = submissionLink;
this.commitHash = commitHash;
this.contentLength = contentLength;
this.committedAt = committedAt;
this.submissionStatus = SUCCESS;
this.submissionFailureType = NONE;
}또한 commitHash의 형식 검증 등 추가적인 유효성 검증도 고려해볼 수 있습니다.
📝 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.
Suggested change
| public class AssignmentHistoryV2 { | |
| @Id | |
| @GeneratedValue(strategy = GenerationType.IDENTITY) | |
| @Column(name = "assignment_history_v2_id") | |
| private Long id; | |
| @Enumerated(EnumType.STRING) | |
| private AssignmentSubmissionStatus submissionStatus; | |
| @Enumerated(EnumType.STRING) | |
| private SubmissionFailureType submissionFailureType; | |
| private Integer contentLength; | |
| @Column(columnDefinition = "TEXT") | |
| private String submissionLink; | |
| private String commitHash; | |
| private LocalDateTime committedAt; | |
| @ManyToOne(fetch = FetchType.LAZY) | |
| @JoinColumn(name = "study_session_v2_id") | |
| private StudySessionV2 studySession; | |
| @ManyToOne(fetch = FetchType.LAZY) | |
| @JoinColumn(name = "member_id") | |
| private Member member; | |
| @Builder(access = AccessLevel.PRIVATE) | |
| private AssignmentHistoryV2( | |
| AssignmentSubmissionStatus submissionStatus, | |
| SubmissionFailureType submissionFailureType, | |
| StudySessionV2 studySession, | |
| Member member) { | |
| this.submissionStatus = submissionStatus; | |
| this.submissionFailureType = submissionFailureType; | |
| this.studySession = studySession; | |
| this.member = member; | |
| } | |
| public static AssignmentHistoryV2 create(StudySessionV2 studySession, Member member) { | |
| return AssignmentHistoryV2.builder() | |
| .submissionStatus(FAILURE) | |
| .submissionFailureType(NOT_SUBMITTED) | |
| .studySession(studySession) | |
| .member(member) | |
| .build(); | |
| } | |
| // 데이터 조회 로직 | |
| public boolean isSubmitted() { | |
| return submissionFailureType != NOT_SUBMITTED; | |
| } | |
| public boolean isSucceeded() { | |
| return submissionStatus == SUCCESS; | |
| } | |
| // 데이터 변경 로직 | |
| public void success(String submissionLink, String commitHash, Integer contentLength, LocalDateTime committedAt) { | |
| this.submissionLink = submissionLink; | |
| this.commitHash = commitHash; | |
| this.contentLength = contentLength; | |
| this.committedAt = committedAt; | |
| this.submissionStatus = SUCCESS; | |
| this.submissionFailureType = NONE; | |
| } | |
| public void fail(SubmissionFailureType submissionFailureType) { | |
| if (submissionFailureType == NOT_SUBMITTED || submissionFailureType == NONE) { | |
| throw new CustomException(ASSIGNMENT_INVALID_FAILURE_TYPE); | |
| } | |
| this.submissionLink = null; | |
| this.commitHash = null; | |
| this.contentLength = null; | |
| this.committedAt = null; | |
| this.submissionStatus = FAILURE; | |
| this.submissionFailureType = submissionFailureType; | |
| } | |
| } | |
| public class AssignmentHistoryV2 { | |
| @Id | |
| @GeneratedValue(strategy = GenerationType.IDENTITY) | |
| @Column(name = "assignment_history_v2_id") | |
| private Long id; | |
| @Enumerated(EnumType.STRING) | |
| private AssignmentSubmissionStatus submissionStatus; | |
| @Enumerated(EnumType.STRING) | |
| private SubmissionFailureType submissionFailureType; | |
| private Integer contentLength; | |
| @Column(columnDefinition = "TEXT") | |
| private String submissionLink; | |
| private String commitHash; | |
| private LocalDateTime committedAt; | |
| @ManyToOne(fetch = FetchType.LAZY) | |
| @JoinColumn(name = "study_session_v2_id") | |
| private StudySessionV2 studySession; | |
| @ManyToOne(fetch = FetchType.LAZY) | |
| @JoinColumn(name = "member_id") | |
| private Member member; | |
| @Builder(access = AccessLevel.PRIVATE) | |
| private AssignmentHistoryV2( | |
| AssignmentSubmissionStatus submissionStatus, | |
| SubmissionFailureType submissionFailureType, | |
| StudySessionV2 studySession, | |
| Member member) { | |
| this.submissionStatus = submissionStatus; | |
| this.submissionFailureType = submissionFailureType; | |
| this.studySession = studySession; | |
| this.member = member; | |
| } | |
| public static AssignmentHistoryV2 create(StudySessionV2 studySession, Member member) { | |
| return AssignmentHistoryV2.builder() | |
| .submissionStatus(FAILURE) | |
| .submissionFailureType(NOT_SUBMITTED) | |
| .studySession(studySession) | |
| .member(member) | |
| .build(); | |
| } | |
| // 데이터 조회 로직 | |
| public boolean isSubmitted() { | |
| return submissionFailureType != NOT_SUBMITTED; | |
| } | |
| public boolean isSucceeded() { | |
| return submissionStatus == SUCCESS; | |
| } | |
| // 데이터 변경 로직 | |
| public void success(String submissionLink, String commitHash, Integer contentLength, LocalDateTime committedAt) { | |
| // 필수 파라미터 널 체크 | |
| if (submissionLink == null || commitHash == null || committedAt == null) { | |
| throw new CustomException(INVALID_INPUT_VALUE); | |
| } | |
| this.submissionLink = submissionLink; | |
| this.commitHash = commitHash; | |
| this.contentLength = contentLength; | |
| this.committedAt = committedAt; | |
| this.submissionStatus = SUCCESS; | |
| this.submissionFailureType = NONE; | |
| } | |
| public void fail(SubmissionFailureType submissionFailureType) { | |
| if (submissionFailureType == NOT_SUBMITTED || submissionFailureType == NONE) { | |
| throw new CustomException(ASSIGNMENT_INVALID_FAILURE_TYPE); | |
| } | |
| this.submissionLink = null; | |
| this.commitHash = null; | |
| this.contentLength = null; | |
| this.committedAt = null; | |
| this.submissionStatus = FAILURE; | |
| this.submissionFailureType = submissionFailureType; | |
| } | |
| } |
Comment on lines
+1
to
+118
| import jakarta.persistence.JoinColumn; | ||
| import jakarta.persistence.ManyToOne; | ||
| import jakarta.persistence.Table; | ||
| import jakarta.persistence.UniqueConstraint; | ||
| import java.time.LocalDateTime; | ||
| import lombok.AccessLevel; | ||
| import lombok.Builder; | ||
| import lombok.Getter; | ||
| import lombok.NoArgsConstructor; | ||
|
|
||
| @Getter | ||
| @Entity | ||
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| @Table( | ||
| name = "assignment_history_v2", | ||
| uniqueConstraints = {@UniqueConstraint(columnNames = {"study_session_v2_id", "member_id"})}) | ||
| public class AssignmentHistoryV2 { | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| @Column(name = "assignment_history_v2_id") | ||
| private Long id; | ||
|
|
||
| @Enumerated(EnumType.STRING) | ||
| private AssignmentSubmissionStatus submissionStatus; | ||
|
|
||
| @Enumerated(EnumType.STRING) | ||
| private SubmissionFailureType submissionFailureType; | ||
|
|
||
| private Integer contentLength; | ||
|
|
||
| @Column(columnDefinition = "TEXT") | ||
| private String submissionLink; | ||
|
|
||
| private String commitHash; | ||
|
|
||
| private LocalDateTime committedAt; | ||
|
|
||
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "study_session_v2_id") | ||
| private StudySessionV2 studySession; | ||
|
|
||
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "member_id") | ||
| private Member member; | ||
|
|
||
| @Builder(access = AccessLevel.PRIVATE) | ||
| private AssignmentHistoryV2( | ||
| AssignmentSubmissionStatus submissionStatus, | ||
| SubmissionFailureType submissionFailureType, | ||
| StudySessionV2 studySession, | ||
| Member member) { | ||
| this.submissionStatus = submissionStatus; | ||
| this.submissionFailureType = submissionFailureType; | ||
| this.studySession = studySession; | ||
| this.member = member; | ||
| } | ||
|
|
||
| public static AssignmentHistoryV2 create(StudySessionV2 studySession, Member member) { | ||
| return AssignmentHistoryV2.builder() | ||
| .submissionStatus(FAILURE) | ||
| .submissionFailureType(NOT_SUBMITTED) | ||
| .studySession(studySession) | ||
| .member(member) | ||
| .build(); | ||
| } | ||
|
|
||
| // 데이터 조회 로직 | ||
|
|
||
| public boolean isSubmitted() { | ||
| return submissionFailureType != NOT_SUBMITTED; | ||
| } | ||
|
|
||
| public boolean isSucceeded() { | ||
| return submissionStatus == SUCCESS; | ||
| } | ||
|
|
||
| // 데이터 변경 로직 | ||
|
|
||
| public void success(String submissionLink, String commitHash, Integer contentLength, LocalDateTime committedAt) { | ||
| this.submissionLink = submissionLink; | ||
| this.commitHash = commitHash; | ||
| this.contentLength = contentLength; | ||
| this.committedAt = committedAt; | ||
| this.submissionStatus = SUCCESS; | ||
| this.submissionFailureType = NONE; | ||
| } | ||
|
|
||
| public void fail(SubmissionFailureType submissionFailureType) { | ||
| if (submissionFailureType == NOT_SUBMITTED || submissionFailureType == NONE) { | ||
| throw new CustomException(ASSIGNMENT_INVALID_FAILURE_TYPE); | ||
| } | ||
| this.submissionLink = null; | ||
| this.commitHash = null; | ||
| this.contentLength = null; | ||
| this.committedAt = null; | ||
| this.submissionStatus = FAILURE; | ||
| this.submissionFailureType = submissionFailureType; | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
단위 테스트 구현이 필요합니다.
새로 추가된 도메인 클래스에 대한 단위 테스트가 필요합니다. 특히 isSubmitted(), isSucceeded(), success(), fail() 메서드에 대한 테스트가 중요합니다.
다음과 같은 테스트 케이스를 고려해보세요:
create()호출 시 기본 상태 확인success()호출 후 상태 변경 확인fail()호출 시 유효하지 않은 실패 유형에 대한 예외 발생 확인fail()호출 후 상태 변경 확인isSubmitted()및isSucceeded()가 각 상태에 맞게 정확한 값을 반환하는지 확인
🏁 Script executed:
#!/bin/bash
# 테스트 클래스 파일 존재 여부 확인
fd "AssignmentHistoryV2Test.java" src/testLength of output: 42
단위 테스트 추가 구현 필요
현재 새로 추가된 AssignmentHistoryV2 도메인 클래스에 대한 단위 테스트가 존재하지 않는 것으로 확인되었습니다. 따라 아래의 테스트 케이스들을 포함하는 단위 테스트를 구현해 주세요:
create()호출 시, 기본 상태가 올바르게 설정되었는지 검증success()호출 후,submissionLink,commitHash,contentLength,committedAt,submissionStatus,submissionFailureType등 필드의 변경이 올바른지 확인fail()호출 시, 잘못된 실패 유형(NOT_SUBMITTED또는NONE) 입력 시CustomException발생 여부 검증fail()호출 후, 필드 초기화 및 상태 변경이 올바른지 확인isSubmitted()와isSucceeded()메서드가 각각의 상태에 따라 정확한 값을 반환하는지 검증
해당 단위 테스트 파일은 src/test/AssignmentHistoryV2Test.java 경로 하에 추가해 주시기 바랍니다.
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The code introduces a new domain entity, AssignmentHistoryV2, to manage assignment submission history. The implementation appears well-structured and includes necessary fields and methods for tracking submission status, failure types, and submission details. However, there are a few areas where improvements can be made to enhance code clarity and maintainability.
Summary of Findings
- Redundant Imports: The code imports
AssignmentSubmissionStatusandSubmissionFailureTypetwice, once directly and once via static imports. This redundancy should be removed for clarity. - Naming Consistency: Consider renaming
isSucceededtoisSuccessfor better consistency with theSUCCESSenum value and general readability. - Exception Handling: The
failmethod throws aCustomExceptionfor invalid failure types. Ensure that this exception is appropriately handled in the calling code to prevent unexpected application behavior.
Assessment
The code introduces a new domain entity, AssignmentHistoryV2, to manage assignment submission history. The implementation appears well-structured and includes necessary fields and methods for tracking submission status, failure types, and submission details. However, there are a few areas where improvements can be made to enhance code clarity and maintainability. I recommend addressing these comments before merging, and also recommend that others review and approve this code before merging.
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.
🌱 관련 이슈
📌 작업 내용 및 특이사항
📝 참고사항
📚 기타
Summary by CodeRabbit