-
Notifications
You must be signed in to change notification settings - Fork 300
4단계 - 수강신청(요구사항 변경) #786
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
base: melodist
Are you sure you want to change the base?
4단계 - 수강신청(요구사항 변경) #786
Changes from all commits
cce9c3b
88944c8
b503c81
06fbcbb
e870d78
334d1d4
f41196c
84ec263
34fbb91
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
# 4단계 - 수강신청(요구사항 변경) | ||
## 3단계 피드백 | ||
### DTO 사용 | ||
- DTO를 사용하지 않는다고 하더라도 도메인 내부의 값을 알기 위한 getter는 필수불가결 | ||
- 그러나 현재 요구사항에서 Session 전체 값을 update할 필요는 없음 | ||
- SessionDto를 Session으로 변환하는 과정에서 다른 Repository에 의존하는 것에는 문제가 있음 | ||
- UserService에서 List<NsUser> 반환 | ||
- imageRepository 의존성 제거 → imageRepository에서 SessionThumbnail 반환 | ||
- sessionEnrollmentRepository 의존성 제거 -> 단순 CRUD 기능의 계층을 늘릴 필요는 없음 | ||
- Service 간 의존성을 반드시 제거할 필요는 없음 (단, 순환 참조는 발생하면 안됨) | ||
### Repository | ||
- 도메인 객체에 ID는 필요 없는 값 | ||
- 하지만 이것을 분리하여 얻는 장점은 크지 않음 | ||
- 참고: [https://mincanit.tistory.com/74](https://mincanit.tistory.com/74) | ||
## 수강 신청 변경 | ||
### 수강 신청 상태 추가 | ||
- 기존 EnrollmentManager에서는 List<NsUser>를 사용하여 수강신청을 관리 | ||
- 수강 신청 상태 - 사용자를 묶는 객체 추가 | ||
- EnrollmentManager는 수강 신청 상태와 관계 없이 List<NsUser> 도 필요 | ||
- List<NsUser> 대신 Map<NsUser, EnrollmentStatus>를 갖도록 변경하자 | ||
- 이러면 별도의 Enrollment 객체는 필요하지 않음 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
# 4단계 - 수강신청(요구사항 변경) | ||
## 핵심 학습 목표 | ||
- DB 테이블이 변경될 때도 스트랭글러 패턴을 적용해 점진적인 리팩터링을 연습한다. | ||
- [스트랭글러(교살자) 패턴 - 마틴 파울러](https://martinfowler.com/bliki/StranglerFigApplication.html) | ||
- [스트랭글러 무화과 패턴](https://docs.microsoft.com/ko-kr/azure/architecture/patterns/strangler-fig) | ||
## 변경된 기능 요구사항 | ||
### 강의 수강신청은 강의 상태가 모집중일 때만 가능하다. | ||
- 강의가 진행 중인 상태에서도 수강신청이 가능해야 한다. | ||
- 강의 진행 상태(준비중, 진행중, 종료)와 모집 상태(비모집중, 모집중)로 상태 값을 분리해야 한다. | ||
### 강의는 강의 커버 이미지 정보를 가진다. | ||
- 강의는 하나 이상의 커버 이미지를 가질 수 있다. | ||
### 강사가 승인하지 않아도 수강 신청하는 모든 사람이 수강 가능하다. | ||
- 우아한테크코스(무료), 우아한테크캠프 Pro(유료)와 같이 선발된 인원만 수강 가능해야 한다. | ||
- 강사는 수강신청한 사람 중 선발된 인원에 대해서만 수강 승인이 가능해야 한다. | ||
- 강사는 수강신청한 사람 중 선발되지 않은 사람은 수강을 취소할 수 있어야 한다. | ||
## 프로그래밍 요구사항 | ||
- 리팩터링할 때 컴파일 에러와 기존의 단위 테스트의 실패를 최소화하면서 점진적인 리팩터링이 가능하도록 한다. | ||
- DB 테이블에 데이터가 존재한다는 가정하에 리팩터링해야 한다. | ||
- 즉, 기존에 쌓인 데이터를 제거하지 않은 상태로 리팩터링 해야 한다. |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,7 +1,7 @@ | ||||||
package nextstep.courses.domain.session; | ||||||
|
||||||
import lombok.Getter; | ||||||
import nextstep.courses.domain.session.enrollment.Enrollment; | ||||||
import nextstep.courses.domain.session.enrollment.Enrollments; | ||||||
import nextstep.courses.domain.session.info.SessionInfo; | ||||||
import nextstep.payments.domain.Payment; | ||||||
import nextstep.users.domain.NsUser; | ||||||
|
@@ -10,12 +10,12 @@ | |||||
public class Session { | ||||||
private final SessionId id; | ||||||
private final SessionInfo info; | ||||||
private final Enrollment enrollment; | ||||||
private final Enrollments enrollments; | ||||||
|
||||||
public Session(SessionId id, SessionInfo info, Enrollment enrollment) { | ||||||
public Session(SessionId id, SessionInfo info, Enrollments enrollments) { | ||||||
this.id = id; | ||||||
this.info = info; | ||||||
this.enrollment = enrollment; | ||||||
this.enrollments = enrollments; | ||||||
} | ||||||
|
||||||
public void enroll(NsUser user, Payment payment) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Enrollments을 Session의 인스턴스 변수로 구현하지 말고 Session의 정보들을 활용해 Enrollments을 생성하도록 구현하는 것은 어떨까? |
||||||
|
@@ -24,7 +24,11 @@ public void enroll(NsUser user, Payment payment) { | |||||
info.validatePayment(payment); | ||||||
} | ||||||
|
||||||
enrollment.enroll(user); | ||||||
enrollments.enroll(user); | ||||||
} | ||||||
|
||||||
public void approve(NsUser user) { | ||||||
enrollments.approve(user); | ||||||
} | ||||||
|
||||||
public boolean isPaid() { | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
package nextstep.courses.domain.session; | ||
|
||
import lombok.Getter; | ||
|
||
@Getter | ||
public enum SessionProgressStatus { | ||
PREPARING("준비중"), | ||
IN_PROGRESS("진행중"), | ||
CLOSED("종료"); | ||
|
||
private final String description; | ||
|
||
SessionProgressStatus(String description) { | ||
this.description = description; | ||
} | ||
|
||
public boolean isInProgress() { | ||
return this == IN_PROGRESS; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,9 +1,54 @@ | ||||||||||||||
package nextstep.courses.domain.session.enrollment; | ||||||||||||||
|
||||||||||||||
import nextstep.courses.domain.session.SessionStatus; | ||||||||||||||
import nextstep.users.domain.NsUser; | ||||||||||||||
|
||||||||||||||
public interface Enrollment { | ||||||||||||||
void enroll(NsUser user); | ||||||||||||||
SessionStatus getStatus(); | ||||||||||||||
public class Enrollment { | ||||||||||||||
private final NsUser user; | ||||||||||||||
private EnrollmentStatus enrollmentStatus; | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||||||||||||||
|
||||||||||||||
public Enrollment(NsUser user, EnrollmentStatus enrollmentStatus) { | ||||||||||||||
this.user = user; | ||||||||||||||
this.enrollmentStatus = enrollmentStatus; | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
public Enrollment(NsUser user) { | ||||||||||||||
this(user, EnrollmentStatus.PENDING_APPROVAL); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
public NsUser getUser() { | ||||||||||||||
return user; | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
public EnrollmentStatus getEnrollmentStatus() { | ||||||||||||||
return enrollmentStatus; | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
public void approve() { | ||||||||||||||
if (enrollmentStatus == EnrollmentStatus.PENDING_APPROVAL) { | ||||||||||||||
enrollmentStatus = EnrollmentStatus.ENROLLED; | ||||||||||||||
} else { | ||||||||||||||
throw new IllegalStateException("승인 대기 상태의 수강신청만 승인할 수 있습니다."); | ||||||||||||||
} | ||||||||||||||
Comment on lines
+27
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
이 로직 구현을 EnrollmentStatus에 구현하는 것은 어떨까? |
||||||||||||||
} | ||||||||||||||
|
||||||||||||||
public void cancel() { | ||||||||||||||
if (enrollmentStatus == EnrollmentStatus.PENDING_APPROVAL || | ||||||||||||||
enrollmentStatus == EnrollmentStatus.ENROLLED) { | ||||||||||||||
enrollmentStatus = EnrollmentStatus.CANCELLED; | ||||||||||||||
} else { | ||||||||||||||
throw new IllegalStateException("승인 대기 또는 수강신청 상태만 취소할 수 있습니다."); | ||||||||||||||
} | ||||||||||||||
Comment on lines
+35
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 앞의 피드백과 같음 |
||||||||||||||
} | ||||||||||||||
|
||||||||||||||
public boolean isPendingApproval() { | ||||||||||||||
return enrollmentStatus == EnrollmentStatus.PENDING_APPROVAL; | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
public boolean isEnrolled() { | ||||||||||||||
return enrollmentStatus == EnrollmentStatus.ENROLLED; | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
public boolean isCancelled() { | ||||||||||||||
return enrollmentStatus == EnrollmentStatus.CANCELLED; | ||||||||||||||
} | ||||||||||||||
Comment on lines
+43
to
+53
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 3개의 메서드를 모두 EnrollmentStatus에 구현하는 것은 어떨까? |
||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,31 +1,106 @@ | ||
package nextstep.courses.domain.session.enrollment; | ||
|
||
import lombok.Getter; | ||
import nextstep.courses.domain.session.SessionStatus; | ||
import nextstep.courses.domain.session.SessionProgressStatus; | ||
import nextstep.courses.domain.session.SessionRecruitmentStatus; | ||
import nextstep.users.domain.NsUser; | ||
|
||
import java.util.ArrayList; | ||
import java.util.HashMap; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.stream.Collectors; | ||
|
||
@Getter | ||
public class EnrollmentManager { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FreeEnrollments와 PaidEnrollments의 중복을 EnrollmentManager을 통해 해결하는 것으로 보여진다. |
||
private final List<NsUser> enrolledUsers; | ||
private final SessionStatus status; | ||
private final Map<NsUser, EnrollmentStatus> enrollmentStatuses; | ||
@Getter | ||
private final SessionProgressStatus progressStatus; | ||
@Getter | ||
private final SessionRecruitmentStatus recruitmentStatus; | ||
|
||
public EnrollmentManager(List<NsUser> enrolledUsers, SessionStatus status) { | ||
this.enrolledUsers = enrolledUsers; | ||
this.status = status; | ||
public EnrollmentManager(SessionProgressStatus progressStatus, SessionRecruitmentStatus recruitmentStatus) { | ||
this.enrollmentStatuses = new HashMap<>(); | ||
this.progressStatus = progressStatus; | ||
this.recruitmentStatus = recruitmentStatus; | ||
} | ||
|
||
public EnrollmentManager() { | ||
this(SessionProgressStatus.PREPARING, SessionRecruitmentStatus.RECRUITING); | ||
} | ||
|
||
public void addEnrollment(NsUser user) { | ||
enrollmentStatuses.put(user, EnrollmentStatus.PENDING_APPROVAL); | ||
} | ||
|
||
public void approveEnrollment(NsUser user) { | ||
if (!enrollmentStatuses.containsKey(user)) { | ||
throw new IllegalArgumentException("해당 사용자의 수강신청 정보가 없습니다."); | ||
} | ||
|
||
EnrollmentStatus currentStatus = enrollmentStatuses.get(user); | ||
if (currentStatus != EnrollmentStatus.PENDING_APPROVAL) { | ||
throw new IllegalStateException("승인 대기 상태의 수강신청만 승인할 수 있습니다."); | ||
} | ||
|
||
enrollmentStatuses.put(user, EnrollmentStatus.ENROLLED); | ||
} | ||
|
||
public void cancelEnrollment(NsUser user) { | ||
if (!enrollmentStatuses.containsKey(user)) { | ||
throw new IllegalArgumentException("해당 사용자의 수강신청 정보가 없습니다."); | ||
} | ||
|
||
EnrollmentStatus currentStatus = enrollmentStatuses.get(user); | ||
if (currentStatus != EnrollmentStatus.PENDING_APPROVAL && currentStatus != EnrollmentStatus.ENROLLED) { | ||
throw new IllegalStateException("승인 대기 또는 수강신청 상태만 취소할 수 있습니다."); | ||
} | ||
|
||
enrollmentStatuses.put(user, EnrollmentStatus.CANCELLED); | ||
} | ||
|
||
public EnrollmentStatus getEnrollmentStatus(NsUser user) { | ||
return enrollmentStatuses.getOrDefault(user, null); | ||
} | ||
|
||
public List<NsUser> getEnrolledUsers() { | ||
return enrollmentStatuses.entrySet().stream() | ||
.filter(entry -> entry.getValue() == EnrollmentStatus.ENROLLED) | ||
.map(Map.Entry::getKey) | ||
.collect(Collectors.toList()); | ||
} | ||
|
||
public List<NsUser> getPendingApprovalUsers() { | ||
return enrollmentStatuses.entrySet().stream() | ||
.filter(entry -> entry.getValue() == EnrollmentStatus.PENDING_APPROVAL) | ||
.map(Map.Entry::getKey) | ||
.collect(Collectors.toList()); | ||
} | ||
|
||
public List<NsUser> getCancelledUsers() { | ||
return enrollmentStatuses.entrySet().stream() | ||
.filter(entry -> entry.getValue() == EnrollmentStatus.CANCELLED) | ||
.map(Map.Entry::getKey) | ||
.collect(Collectors.toList()); | ||
} | ||
|
||
public List<Enrollment> getAllEnrollments() { | ||
List<Enrollment> enrollments = new ArrayList<>(); | ||
for (Map.Entry<NsUser, EnrollmentStatus> entry : enrollmentStatuses.entrySet()) { | ||
enrollments.add(new Enrollment(entry.getKey(), entry.getValue())); | ||
} | ||
return enrollments; | ||
} | ||
|
||
public void enroll(NsUser user) { | ||
validateEnrollment(user); | ||
enrolledUsers.add(user); | ||
addEnrollment(user); | ||
} | ||
|
||
private void validateEnrollment(NsUser user) { | ||
if (user == null) { | ||
throw new IllegalArgumentException("수강 신청할 사용자가 없습니다."); | ||
} | ||
if (!status.isRecruiting()) { | ||
if (!recruitmentStatus.isRecruiting()) { | ||
throw new IllegalStateException("수강 신청이 불가능합니다."); | ||
} | ||
if (hasEnrolledUser(user)) { | ||
|
@@ -34,6 +109,7 @@ private void validateEnrollment(NsUser user) { | |
} | ||
|
||
private boolean hasEnrolledUser(NsUser user) { | ||
return enrolledUsers.contains(user); | ||
return enrollmentStatuses.containsKey(user); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
package nextstep.courses.domain.session.enrollment; | ||
|
||
public enum EnrollmentStatus { | ||
PENDING_APPROVAL("승인대기"), | ||
ENROLLED("수강신청"), | ||
WAITING("대기"), | ||
CANCELLED("취소"); | ||
|
||
private final String description; | ||
|
||
EnrollmentStatus(String description) { | ||
this.description = description; | ||
} | ||
|
||
public String getDescription() { | ||
return description; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
package nextstep.courses.domain.session.enrollment; | ||
|
||
import nextstep.courses.domain.session.SessionProgressStatus; | ||
import nextstep.courses.domain.session.SessionRecruitmentStatus; | ||
import nextstep.users.domain.NsUser; | ||
|
||
import java.util.List; | ||
|
||
public interface Enrollments { | ||
void enroll(NsUser user); | ||
void approve(NsUser user); | ||
void cancel(NsUser user); | ||
SessionProgressStatus getProgressStatus(); | ||
SessionRecruitmentStatus getRecruitmentStatus(); | ||
List<NsUser> getEnrolledUsers(); | ||
List<NsUser> getPendingApprovalUsers(); | ||
EnrollmentStatus getEnrollmentStatus(NsUser user); | ||
} |
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.
todo list 작성 후 구현 👍