-
Notifications
You must be signed in to change notification settings - Fork 3
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
[Fix/#335] 종료된 공연 예매 불가능 하도록 수정 및 로그인 여부에 따른 공연 등록 페이지 접근 로직 수정 #339
Conversation
PR 작성하느라 고생 많았어요!! 라벨 잘 지정되었는지 확인 한 번 해 주기 🫶 |
Quality Gate passedIssues Measures |
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.
최고띠예... 의견이 궁금한 부분 남겨두긴 했는데 크게 수정이 필요한 부분은 없어 보여서 어푸할게요~
if (data) { | ||
const nowDate = new Date(); | ||
const lastPerformanceDate = new Date( | ||
data.scheduleList[data?.scheduleList.length - 1].performanceDate | ||
); | ||
if (nowDate > lastPerformanceDate) { | ||
openAlert({ | ||
title: "종료된 공연입니다.", | ||
okText: "확인", | ||
okCallback: () => { | ||
navigate("/main"); | ||
}, | ||
}); | ||
} | ||
} | ||
}, [data]); |
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.
p5) 준혁 오빠 코드리뷰에는 서버에서 dueDate를 받는 게 좋을 것 같다고 달았는데... 생각해보니까 간단한 계산이라 클라에서 해도 되나? 싶은 생각이 들어서요!
그때그때 다르겠지만 일반적으로 이렇게 서버에서 dueDate를 가지고 있긴 하지만 넘겨주지 않을 때, 어차피 서버에 가지고 있는 값이긴 하니 dueDate도 함께 달라고 요청하는 게 좋은 방식인지 그냥 위 코드처럼 공연 날짜만 받아서 계산하는 게 좋은 방식인지 궁금함미다............. 아무나 의견 plz
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.
p5) 제 생각은 dueDate를 서버에서 받게 된다면, 어차피 회차수에 따라서 yyyy.mm.dd 하나만 받거나 yyyy.mm.dd ~ yyyy.mm.dd 와 같은 형식 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.
앗 제가 언급한 dueDate는 날짜를 받는 게 아니라 공연까지 얼마나 남았느냐에 대한 정보라서 당일이라면 0, 공연 전이라면 1 이상, 공인 이후면 -로 넘어오는 값이라 클라에서 구분할 필요는 없을 것 같아요! 여러 날짜동안 진행되는 정보의 경우에도 서버에서 알맞게 dueDate를 처리해 주고 있는 거로 알고 있습니다
const nowDate = new Date(); | ||
const lastPerformanceDate = new Date( | ||
data?.scheduleList[data?.scheduleList.length - 1]?.performanceDate | ||
); | ||
// 현재 시간이 마지막 공연 시간보다 크면 예매 버튼 비활성화 | ||
const isBookDisabled = nowDate > lastPerformanceDate; | ||
|
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.
p5) 이 친구도 위에 리뷰 남긴 것처럼 서버에서 받으면 dueDate < 0 하면 더 간단할 것 같아요!! 이건 그냥 의견이구 뭐가 좋은지는...... 고민 필요..........
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.
p5) 확실히 dueDate 받으면 클라 측은 많이 간단해지긴 하겠네요! 굿
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 ~!
const nowDate = new Date(); | ||
const lastPerformanceDate = new Date( | ||
data?.scheduleList[data?.scheduleList.length - 1]?.performanceDate | ||
); | ||
// 현재 시간이 마지막 공연 시간보다 크면 예매 버튼 비활성화 | ||
const isBookDisabled = nowDate > lastPerformanceDate; | ||
|
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.
p5) 확실히 dueDate 받으면 클라 측은 많이 간단해지긴 하겠네요! 굿
const nowDate = new Date(); | ||
const lastPerformanceDate = new Date( | ||
data.scheduleList[data?.scheduleList.length - 1].performanceDate | ||
); |
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.
p5) 역시 직접 계산해버리셨군요! 저도 그리 어렵지 않은 작업이기도 하고, 빠르게 변경할 수 있는거라 직접 구현했는데.. 👍
📌 관련 이슈번호
🎟️ PR 유형
어떤 변경 사항이 있나요?
✅ Key Changes