Skip to content
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

Merged
merged 4 commits into from
Aug 6, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package com.gdschongik.gdsc.domain.study.domain;

import com.gdschongik.gdsc.domain.common.model.BaseEntity;
import com.gdschongik.gdsc.domain.member.domain.Member;
import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.EnumType;
import jakarta.persistence.Enumerated;
import jakarta.persistence.FetchType;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
import jakarta.persistence.JoinColumn;
import jakarta.persistence.ManyToOne;
import lombok.AccessLevel;
import lombok.Builder;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Getter
@Entity
@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class AssignmentHistory extends BaseEntity {

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
@Column(name = "assignment_history_id")
private Long id;

@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "study_detail_id")
private StudyDetail studyDetail;

@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "member_id")
private Member member;

@Column(columnDefinition = "TEXT")
private String submissionLink;
Copy link
Member

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;
Copy link
Member

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 타입 사용하시는게 좋을듯 합니다

Copy link
Member Author

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는 별도 이슈에서 수정할게요~


@Enumerated(EnumType.STRING)
private AssignmentSubmissionStatus status;

@Builder(access = AccessLevel.PRIVATE)
private AssignmentHistory(
StudyDetail studyDetail,
Member member,
String submissionLink,
String commitHash,
long length,
AssignmentSubmissionStatus status) {
this.studyDetail = studyDetail;
this.member = member;
this.submissionLink = submissionLink;
this.commitHash = commitHash;
this.length = length;
this.status = status;
}

public static AssignmentHistory create(
StudyDetail studyDetail,
Member member,
String submissionLink,
String commitHash,
long length,
AssignmentSubmissionStatus status) {
return AssignmentHistory.builder()
.studyDetail(studyDetail)
.member(member)
.submissionLink(submissionLink)
.commitHash(commitHash)
.length(length)
.status(status)
.build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.gdschongik.gdsc.domain.study.domain;

import lombok.Getter;
import lombok.RequiredArgsConstructor;

@Getter
@RequiredArgsConstructor
public enum AssignmentSubmissionStatus {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

개인적인 생각인데 성공 / 실패 여부를 나타내는 필드와, 실패 사유를 나타내는 필드를 분리하는 건 어떨까요?
서비스가 고도화된다면 실패 케이스도 여러 가지가 생길 것 같은데, 대부분의 관심사는 성공했는지 실패했는지일 것이고
사용자한테 사유 알려줄 때만 실패 세부 케이스가 필요해질 것 같아서 이게 더 괜찮지 않나 싶네요

토스페이먼츠 연동하면서 API 스펙을 좀 봤는데, 여기도 결제에 대해서 결제 성공 / 실패 여부에 대한 enum 값 외에도
실패에 대한 사유를 다루는 별도 필드를 두는 경우가 있더라고요. 이쪽도 실패 케이스가 복잡해지면서 이렇게 구현한 것 같았습니다

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

확실히 분리하는게 나중에 쓰기도 편하겠네요
수정했습니다~

NOT_SUBMITTED("미제출"),
WORD_COUNT_INSUFFICIENT("글자수 부족"),
LOCATION_UNIDENTIFIABLE("위치 확인불가"),
SUBMISSION_SUCCESS("제출 성공");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
SUBMISSION_SUCCESS("제출 성공");
SUCCESS("제출 성공");


private final String value;
}