-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: 과제 히스토리 테이블 추가 #570
feat: 과제 히스토리 테이블 추가 #570
Conversation
Walkthrough새로 도입된 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 Configuration File (
|
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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/study/domain/AssignmentHistory.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentSubmissionStatus.java (1 hunks)
Additional comments not posted (5)
src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentSubmissionStatus.java (2)
1-5
: 패키지 선언 및 임포트가 적절합니다.패키지 선언과 임포트는 표준적이며 해당 컨텍스트에 적합합니다.
6-14
: 열거형 선언 및 상수가 적절합니다.
AssignmentSubmissionStatus
열거형은 네 개의 상수를 정의하고 있으며, 각 상수에 연결된 문자열 값은 과제 제출 컨텍스트에서 명확하고 의미가 있습니다.src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentHistory.java (3)
1-19
: 패키지 선언 및 임포트가 적절합니다.패키지 선언과 임포트는 표준적이며 해당 컨텍스트에 적합합니다.
20-47
: 클래스 선언, 애노테이션 및 필드가 적절합니다.
AssignmentHistory
클래스는 JPA와 Lombok 애노테이션으로 잘 구조화되어 있으며, 필드는 과제 이력에 필요한 모든 정보를 포함하고 있습니다.
48-79
: 생성자 및 빌더 메서드가 적절합니다.빌더 패턴이 올바르게 구현되었으며, 정적 팩토리 메서드는 가독성과 사용성을 높입니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
private Member member; | ||
|
||
@Column(columnDefinition = "TEXT") | ||
private String submissionLink; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
사실 링크는 별도로 관리하지 않아도 되긴 하겠지만...
이미지 기능과 마찬가지로 매번 url을 빌딩해서 리턴하려면 번거로움이 있을 것 같으니
제출할 때 그냥 문자열로 만들어서 저장해버리면 괜찮긴 하겠네요
|
||
private String commitHash; | ||
|
||
private long length; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
content_length가 더 적절할 것 같아요. 그냥 length라고 하면 좀 모호한 것 같습니다
그리고 nullable한 값이라면 wrapper 타입 사용하시는게 좋을듯 합니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
과제를 제출하는 시점에 AssignmentHistory가 생성된다고 생각해서 length가 nullable하지 않다고 생각했어요
근데 수강신청 하는 시점에 생성한다면 nullable한 값이 될 것 같고, 과제 제출 시점보단 수강신청 시점에 생성하는게 적절할 것 같네요!
수정하고, 수강 신청 api는 별도 이슈에서 수정할게요~
NOT_SUBMITTED("미제출"), | ||
WORD_COUNT_INSUFFICIENT("글자수 부족"), | ||
LOCATION_UNIDENTIFIABLE("위치 확인불가"), | ||
SUBMISSION_SUCCESS("제출 성공"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SUBMISSION_SUCCESS("제출 성공"); | |
SUCCESS("제출 성공"); |
|
||
@Getter | ||
@RequiredArgsConstructor | ||
public enum AssignmentSubmissionStatus { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
개인적인 생각인데 성공 / 실패 여부를 나타내는 필드와, 실패 사유를 나타내는 필드를 분리하는 건 어떨까요?
서비스가 고도화된다면 실패 케이스도 여러 가지가 생길 것 같은데, 대부분의 관심사는 성공했는지 실패했는지일 것이고
사용자한테 사유 알려줄 때만 실패 세부 케이스가 필요해질 것 같아서 이게 더 괜찮지 않나 싶네요
토스페이먼츠 연동하면서 API 스펙을 좀 봤는데, 여기도 결제에 대해서 결제 성공 / 실패 여부에 대한 enum 값 외에도
실패에 대한 사유를 다루는 별도 필드를 두는 경우가 있더라고요. 이쪽도 실패 케이스가 복잡해지면서 이렇게 구현한 것 같았습니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확실히 분리하는게 나중에 쓰기도 편하겠네요
수정했습니다~
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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/study/domain/AssignmentHistory.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentSubmissionStatus.java (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentHistory.java
- src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentSubmissionStatus.java
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentHistory.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentSubmissionStatus.java (1 hunks)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/SubmissionFailureType.java (1 hunks)
Files skipped from review due to trivial changes (1)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/SubmissionFailureType.java
Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentSubmissionStatus.java
Additional comments not posted (6)
src/main/java/com/gdschongik/gdsc/domain/study/domain/AssignmentHistory.java (6)
1-14
: 패키지 및 임포트 확인 완료패키지 및 임포트가 적절하게 설정되었습니다.
15-23
: 클래스 어노테이션 및 정의 확인 완료Lombok 어노테이션과 엔티티 정의가 적절하게 설정되었습니다.
25-36
: ID 및 관계 설정 확인 완료ID 생성 전략과
StudyDetail
및Member
와의 관계가 적절하게 정의되었습니다.
38-49
: 컬럼 및 열거형 확인 완료컬럼 정의와 열거형 사용이 적절하게 설정되었습니다.
51-65
: 빌더 및 생성자 확인 완료빌더와 생성자가 적절하게 정의되고 사용되었습니다.
67-77
: 정적 팩토리 메서드 확인 완료정적 팩토리 메서드가 적절하게 정의되고 사용되었습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
🌱 관련 이슈
📌 작업 내용 및 특이사항
📝 참고사항
📚 기타
Summary by CodeRabbit
Summary by CodeRabbit
새로운 기능
AssignmentHistory
클래스가 추가되었습니다.AssignmentSubmissionStatus
열거형이 도입되었습니다.SubmissionFailureType
열거형이 새로 추가되었습니다.버그 수정