-
Notifications
You must be signed in to change notification settings - Fork 0
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
[FU-346] 날짜별 스케줄 API 개발 #90
Conversation
- 기존: 시간대만 비교 - 수정: 날짜가 동일한 경우 start time, end time 비교
scheduleId를 추가로 반환
- 날짜별 스케줄 수정 시, 기존 로직에서는 동일한 스케줄과 시간대가 중복되는 경우를 처리하지 못하였음. 이를 보완하는 오버로딩 함수 정의하였음 - 날짜별 스케줄 조회 시 현시점 이후의 데이터만 조회해옴. 등록 및 수정 시에도 현시점 이후의 데이터만 등록할 수 있도록 검증함
테스트 코드 작성 시 LocalDateTime.now()를 모킹하기 위해 Clock을 빈으로 등록하였음
고생했어!! 코드는 아직 보는 중인데 조회 관련해서 몇 가지 궁금한 게 있어 🧐
|
|
응응 리스트로 보고 관리하기엔 좀 어려울 것 같아서 캘린더 기반으로 퍼블리싱 중이었어! 월별 조회랑 일별 조회 분리한다면 월별 조회에서는 날짜별 특정 상태 스케줄의 유무만 받아도 괜찮고, 아니면 월별 조회에서 데이터 전부 다 받아서 일별 조회는 따로 안 만드는 방법도 있을 것 같다! 검증 관련해서 든 생각은
|
혹시 날짜별 스케줄이나 확정 일정쪽 디자인 된 거 있으면 피그마 공유해줄 수 있어? 월별조회, 일별조회 이런것도 보면서 생각하면 좀 더 쉬울 것 같은데 지금은 어떻게 API를 수정해야 할지 잘 감이 안오네 😵💫
|
디자인 슬랙에 올려 놓을게!! |
기존: 현시점 이후의 스케줄만 조회 가능 변경: 과거 데이터를 포함한 월별 스케줄 조회 가능
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.
꼼꼼하게 잘 작성해줘서 코멘트는 하나밖에 없어!👍 테스트코드도 깔끔해서 다음 테스트코드 작성할 때 참고해서 작성해볼게😊
private void validateScheduleOverlap(Member member, DailyScheduleRequest request, Long scheduleId) { | ||
List<DailySchedule> overlappingSchedules = dailyScheduleRepository.findOverlappingSchedules(member, | ||
request.getDate(), request.getStartTime(), request.getEndTime()); | ||
|
||
if (overlappingSchedules.size() == 1 && overlappingSchedules.get(0).getId().equals(scheduleId)) { | ||
return; | ||
} |
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.
날짜별 스케줄을 업데이트할 때 기존 일정의 시작 시간이 새로운 일정의 종료 시간보다 빠르고, 기존 일정의 종료 시간이 새로운 일정의 시작 시간보다 늦은 경우에만 업데이트 가능하도록 하는 것 같은데 어떤 기준인건지 궁금해..!
같은 날짜에서 시작 시간과 끝나는 시간은 어떻게 업데이트해도 괜찮다고 생각돼서!
추가로 날짜별 스케줄을 생성하거나 업데이트할 때 startTime, endTime이 올바른지 검증해주는 로직도 추가하면 좋을 것 같아!
- startTime과 endTime 같은 경우
- startTime이 endTime보다 더 늦은 경우
이렇게 두 경우가 있을 것 같아!
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.
날짜별 스케줄을 업데이트할 때 기존 일정의 시작 시간이 새로운 일정의 종료 시간보다 빠르고, 기존 일정의 종료 시간이 새로운 일정의 시작 시간보다 늦은 경우에만 업데이트 가능하도록 하는 것 같은데 어떤 기준인건지 궁금해..!
특정 날짜에 날짜별스케줄을 두 개 이상 등록하려고 할 때 기존에 등록되어 있던 시간대와 겹치지 않게 검증하려고 의도한거였어! 예를 들면 1/7일 10:00-13:00까지 예약중지
가 등록되어 있는 상태에서 1/7일 10:00-11:00에 예약오픈
을 겹치게 등록할 수 없도록 하려는..!
근데 생각해보니까 예약확정
의 경우에는 내부예약건인 경우 날짜별스케줄에 자동 연동되게끔 하는 기능을 추가할텐데, 이 경우에 자동연동 된 예약 스케줄이 기존에 날짜별스케줄에 등록되어 있는 시간대와 겹칠 수 있고, 이게 장기적으로는 지금 코드에서 검증하는 내용을 어렵게 만들 수도 있을 것 같네! 🫨
대안으로는 이 두가지 정도가 생각나는데.. 어떤 것 같아? 😣
- 위 상황이 발생하면 내부예약건을 날짜별 스케줄에 자동연동 시키지 않고 클라이언트 측에 예외메시지로 상황을 알린다.
- 특정 날짜에 대해 날짜별스케줄을 여러개 등록할 때 시간대가 겹쳐도 상관없게끔 구현한다.
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.
추가로 날짜별 스케줄을 생성하거나 업데이트할 때 startTime, endTime이 올바른지 검증해주는 로직도 추가하면 좋을 것 같아!
startTime과 endTime 같은 경우
startTime이 endTime보다 더 늦은 경우
이렇게 두 경우가 있을 것 같아!
이 두개 다 생각 못했는데 고마워 🤩 검증에 추가할게!!
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.
'예약 확정 일정' 테이블이 있었고 이후에 '날짜별 스케줄' 테이블이 추가되면서 '날짜별 스케줄' 테이블에는 예약 확정 일정에 대한 정보가 없다보니 겹치는 부분을 제어하기가 쉽지 않은 것 같네..!
조금 복잡도가 올라갈 것 같긴 한데 (1)날짜별 스케줄을 등록할 때
현재 예약 확정 상태인 '입금대기'와 '촬영대기' 상태의 촬영 시작시간과 종료시간을 고려해서 등록하면 겹치는 걸 방지할 수 있을 것 같고 반대로 (2)예약을 받고 촬영 시작시간과 종료시간을 선택할 때
날짜별 스케줄과 겹치지는 않는지 검증하는 로직을 사용하면 겹치는 걸 방지할 수 있고..!
(1),(2)의 상황에 날짜별 스케줄과 예약 확정 일정이 충돌할 지점으로 보여서 위 방법을 생각해봤는데 꽤 복잡도가 높아질 것 같아서 구현하기에 괜찮으면 고려해봐도 좋을 것 같아서 코멘트 남겨놔!
너가 말해준 방법에 대한 의견은⬇️
- 이 방법은 적용시키면 '예약 확정 일정'은 메인페이지나 '예약 일정 오픈' 페이지에서 캘린더형태로도 확인할 수 있고 작가가 직접 등록한 '예약 중지' 혹은 '추가 오픈'에 대한 부분은 날짜별 스케줄에서 구분지어 확인할 수 있어서 더 좋을 것 같아! 결국 날짜별 스케줄에서 예약 확정일정을 제외하는 방법인거지?!
- 위에서 내가 얘기한 방법이 어렵다면 작가가 직접 등록한 '예약 중지', '추가 오픈' 상태에 한에서는 겹치지 않게 제어하고 '예약 확정'에 대해서만 겹칠 수 있도록 하는 건 어때? (예약 중지 상태랑 추가 오픈이 겹치는건 사진작가가 추후에 이해하기 어려울 수 있을 것 같아서..! 그에 비해 예약 확정은 촬영건이니까 충분히 고려한 후일 것 같아서 괜찮아보이고!
읽어보고 의견 남겨주면 좀 더 고민해볼게!
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.
-
너가 말해준 것 중에 '예약 확정'에 대해서는 다른 날짜별 스케줄(예약 중지, 추가 오픈)과 겹치는걸 허용하게 하는 것도 괜찮을 것 같다!
지금 코드보다 좀 더 복잡해지긴 하겠지만 크게 어려운 내용은 아니라서 부담은 없어! 다만 이 방식을 선택하면고객측에서 예약 가능한 시간대를 조회하는 기능
개발할 때 좀더 신경 써줘야 하고, "예약 중지와 추가 오픈 시간은 겹치게 등록할 수 없습니다" 등의 안내문구를 고객한테 노출해야 할 필요는 생길 수 있겠다! -
아니면 날짜별 스케줄 간에 시간대가 겹치는건 아예 불가능하게 막아버리고, 상담중->입금대기 단계에서 예약 확정일정을 날짜별 스케줄에 자동연동 시킬 때 기존 날짜별 스케줄과 충돌하면 "해당 시간대에 이미 등록된 스케줄이 있어서 자동연동이 불가합니다. 직접 등록/수정해주세요." 등의 안내문구를 주는 방법이 있을 것 같아!
어떤걸 선택하냐에 따라서 구현방식이 조금씩 바뀔거같아서 한번 더 의견주면 반영해볼게 😀
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.
(1번 관련해서만 추가로!)
나도 이건 시작 시간 / 종료 시간으로 계산하기보다 말해 준 방식대로 구현되면 좋을 것 같아! 배열로 받으면 프론트에서도 띄우기가 가장 수월할 것 같고, 이 방법을 적용한다면 (좀 더 좋은 로직이 없을지 좀 고민은 되지만) 디폴트 배열에 우선순위가 낮은 순으로 false / true를 조정해서 반환하는 식으로 구현할 수 있겠다 🧐
30분 단위일 때 타임블록이 48개,, 가 되면 화면에 그리기엔 너무 많을 것 같긴 한데, 예약이 가능한 시간대만 타임블록 표시하기 / 처음으로 예약 가능한 시간 ~ 마지막으로 예약 가능한 시간 사이만 타임블록을 표시하고, 이 사이에서 예약 불가능한 타임블록은 블락 처리하기 같은 식으로 보완하면 더 좋을 듯!!
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.
코멘트 다 확인했어! 👍
나도 유진이가 이야기해준 부분에 동의하고 '위계를 둬서 같은 레벨끼리 검증한다' 라는 개념 좋은 것 같아!
근데 스케줄 테이블은 한번 더 정리가 필요할 것 같아!
테이블 명 | 설명 |
---|---|
기본 스케줄 | 고객이 예약 일정을 조회할 때 타임슬롯의 단위와 시작시간, 종료시간 기준을 잡는데 사용됨 |
날짜별 스케줄 | 고객이 예약 일정을 조회할 때 기본 스케줄에 포함되지 않은 예외사항을 추가로 반영하는데 사용됨. 추가오픈, 예약중지, 예약확정 타입으로 저장되며 예약중지 와 예약확정 의 역할은 같지만(고객이 해당 시간대 예약불가) 사진작가의 이해를 돕기 위해서 타입이 구분되는 것 뿐이다. |
예약 확정 일정 | 사진작가가 캘린더 기반으로 촬영 일정을 관리하는데 사용됨. 고객과는 관련 없는 테이블 (사진작가가 캘린더에 일정을 아무리 추가 등록해도 고객이 예약가능한 시간대를 조회할 때 영향을 미치지 않음) |
이 중에서 '예약 확정' 개념이 좀 헷갈릴 수 있을 거 같은데, 날짜별 스케줄을 조회할 때 '추가 오픈', '예약 중지' 건은 날짜별 스케줄 테이블
에서 가져오고, '예약 확정'건은 예약 확정 일정 테이블
에서 가져오는게 아니야! 이렇게 하면 테이블의 용도는 분리되어서 좋지만, 오히려 비즈니스 로직에서는 두 테이블 간 의존성이 높아지겠더라구.. 그래서 아래처럼 구현해야 할 것 같았어!
- 내부예약건의 경우 상담중->입금대기로 넘어갈 때 '날짜별 스케줄'에 자동연동 여부를 선택할 수 있다.
- 만약 자동연동을 시키면 해당 확정일정은
날짜별 스케줄 테이블
과확정일정 테이블
에 중복으로 저장된다. - 자동연동을 선택하지 않으면 해당 확정일정은
확정일정 테이블
에만 저장된다.
(=고객은 여전히 이 시간대에 예약을 신청할 수 있다)
- 만약 자동연동을 시키면 해당 확정일정은
- 외부예약건을 등록하고 싶을 경우 사진작가는 '캘린더 보기 페이지'와 '날짜별 스케줄 페이지'에 각각 등록해야 한다.
- 캘린더 보기 페이지에 등록할 경우
확정일정 테이블
에만 반영되고, 사진작가가 단순히 캘린더로 자신의 촬영일정을 관리하는 용도로만 사용된다. - 날짜별 스케줄 페이지에 등록할 경우
날짜별 스케줄 테이블
에만 반영되고, 고객이 이 시간대에 예약을 체결할 수 없도록 막는 용도로만 사용된다.
- 캘린더 보기 페이지에 등록할 경우
😭 최최종 정리
- '날짜별 스케줄'에서
추가오픈
<->예약중지
간의 충돌이 없어야 한다. (이번 PR에 추가 반영할게) - '날짜별 스케줄'에서
예약확정
<->추가오픈
예약중지
간에는 겹치는 시간대가 있어도 괜찮다. - 상담중->입금대기 단계에서 스케줄 자동연동을 선택할 때, '예약 확정 테이블'내에서 충돌이 발생하지 않도록 검증한다. '날짜별 스케줄 테이블'내에서는 별도의 충돌 여부를 검증하지 않는다.
- 고객이 예약가능한 날짜를 계산할 때 다음의 순서를 따른다.
(1) 기본 스케줄 테이블에서 슬롯 단위/시작시간/종료시간을 가져온다.
(2) 날짜별 스케줄 테이블에서 추가로 예약가능한 시간(추가오픈), 예외적으로 예약불가능한 시간(예약중지,예약확정)을 가져와서 시본스케줄의 시간대를 덮어 씌운다.
추가 논의 필요한 부분
날짜별 스케줄을 등록할 때, 기본 스케줄 단위에 맞춰야 하는가?
(ex: 1시간 단위로 기본스케줄이 설정되면 날짜별 스케줄도 1시간 단위로만 등록하게끔 해야 하는가?)
시간 단위를 맞출 경우 | 시간 단위를 맞추지 않을 경우 | |
---|---|---|
장점 | 고객이 예약가능한 시간대 조회할때 가장 깔끔함 | 사진작가측 자유도가 높아짐 |
단점 | 사진작가측 자유도가 많이 떨어짐. (ex:기본스케줄 단위는 1시간인데, 사진작가는 30분만 예약을 중지하고 싶다면..?) |
고객이 예약가능한 시간대 조회하는 기능 개발 시 복잡도가 높아짐 (ex: 기본스케줄은 1시간 단위인데, 날짜별 스케줄이 30분 단위로 등록되어 있는 경우 슬롯 불균형이 생김) |
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.
- 확정 일정 테이블
- 변경 가능한 상황: 작가의 내부 예약 일정 관리, 외부 예약 확정 건 추가 등록
- 조회되는 상황: 작가의 업무 캘린더
- 날짜별 스케줄 테이블
- 변경 가능한 상황: 작가의 날짜별 스케줄 수정, 내부 예약 일정 관리시 연동되도록 선택했을 경우
- 조회되는 상황: 작가의 날짜별 스케줄 확인, 고객의 예약 일정 조회
이렇게인 건가?! 전에 슬랙에서도 얘기해 준 것 같은데 또 헷갈렸네… 😂 정리해 준 내용에 동의하고 추가로 적어 준 내용 관해서는 나는 시간 단위를 맞추게 강제해도 될 것 같아! 1시간 단위를 사용한다면 날짜별 스케줄에서는 무조건 1시간 단위로 만들어지는 타임블록을 추가로 끄거나 켤 수만 있는 느낌으로? 🤔
지금 1시간 단위 설정은 무조건 정시에 시작해서 정시에 끝나게 하고 있다 보니 (9:30 ~ 10:30의 스케줄 설정 불가) 조금 한계가 있는 것 같긴 한데 🥹 네이버 예약을 기준으로 생각했을 때는 비슷하게 동작하고 있는 것 같아서 괜찮을 것 같다고 생각했어! 시스템 상에서는 무조건 설정한 시간 단위에 맞춰야 하지만, 실제 현실에서는 조금 유연하게 하는 식으로 적용될 수 있지 않을까…! 지금까지 본 사례에서는 촬영을 그렇게 빡빡하게 잡지 않았는데 또 규모가 큰 경우에는 이런 수요가 있으려나? 하는 생각이 좀 들긴 한다
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.
시간 단위를 맞추지 않으면 운영시간 단위를 설정한 의미가 퇴색될 것 같은..!? 시간을 사진작가가 모두 커스텀하기보단 기준을 주고 거기서 세세한 부분은 고객과 개인적으로 조율하는게 좋아보여! 네이버예약에서 그런 사례도 많이 봤었고🤔 (이외에 유리가 정리해준 부분에 대해서는 모두 동의해!)
'예약 확정'에 대한 개념은 잘 이해했어! ERD에서 날짜별 테이블에 (예약확정) 이렇게 표시되어 있다보니 예약 확정 일정 테이블에서 참조되는걸로 생각했었네😅
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.
유진이가 이해한거 맞아! 깔끔하게 정리해줬당
테이블명 때문에 그런지 더 헷갈리네😂
둘 다 의견 고마워! 그럼 기본 스케줄 단위와 맞추는걸로..!
시작시간이 종료시간과 같거나, 종료시간이 시작시간보다 더 앞인 경우 예외 발생
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.
의견 주고 받느라 고생했어! 다음 티켓에서 남은 수정사항들 잘 부탁해!😊
체크리스트
작업 내역
추가 작업이 필요한 내역
(ex 기본스케줄 단위가 30분이면, 날짜별 스케줄도 30분 단위로 설정할 수 있도록)
고민한 내용
날짜별 스케줄 등록/수정 요청 시, 요청 스케줄 날짜가 현재보다 과거의 시점이 아닌지 검증하기 위해
LocalDateTime.now()
를 사용해 현재 시점을 가져오게 됩니다. 서비스 로직 자체에서는 별 문제가 되지 않지만 테스트 코드를 작성할 때는LocalDateTime.now()
로 인해 테스트 코드 작성 의도와 다르게 테스트가 실패하는 경우를 경험하게 되었습니다.예를 들어, 날짜별 스케줄을 등록하려는 스케줄이 1/5 10:00~11:00이고, 테스트 코드를 돌리는 현재시점은 1/5 09:00 라고 가정해보면.
09:00에 테스트코드를 돌렸을땐 정상적으로 날짜별 스케줄이 등록되겠지만 시간이 지나 1/5 11:00에 다시 테스트코드를 돌려보면 과거 시점의 날짜별 스케줄을 등록하려는 요청으로 간주해
DAILY_SCHEDULE_IN_PAST
에러가 발생하게 됩니다.이러한 문제를 해결하기 위해 테스트코드에서 LocalDateTime을 모킹하려고 했지만, now()는 static 메서드였고 다른 방법을 찾아야 했습니다💆♀️ 이때 LocalDateTime.now()는 사실 LocalDateTime.now(clock)과 같은 형태로 구현되어 있는데, 파라미터로 받는 이 Clock 객체를 활용하면 관련 문제를 해결할 수 있음을 알게 되었습니다!
그래서 Clock을 빈으로 등록한 뒤 서비스 레이어에서 주입받고, 테스트코드에서는 이 Clock 객체를 모킹하여 테스트 시 고정된 Clock 인스턴스를 주입해 시간 의존성을 제거할 수 있도록 했습니다. 관련된 내용은 테스트 코드에서 더 확인할 수 있습니다
_리뷰 요청사항
1차 코드리뷰 이후 추가 변경사항
START_TIME_AFTER_END_TIME
예외를 반환함