-
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
Feature/세미나 지각 & 결석 벌점 처리 구현 #307
The head ref may contain hidden characters: "feature/\uC138\uBBF8\uB098-\uC9C0\uAC01-\uACB0\uC11D-\uBC8C\uC810\uCC98\uB9AC"
Conversation
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.
테스트도 너무 잘 짜주셨고 로직도 훨씬 간단해진 것 같네요~
개인적으론 LATENESS가 언제 되는지 로직을 찾느라 좀 해맸습니다 ㅎㅎ;;
스프링 스케줄러가 제때 잘 동작하는 지도 테스트 하여야 할까요? 해야 한다면 어떤 방법으로 테스트하는 게 좋은 지 궁금합니다.
제 생각엔 스프링 스케줄러 테스트는 딱히 필요 없을 것 같아요~ LocalDate.now()
를 사용할 때 실제로 현재 시간을 가져오는지 테스트 하지 않는 것 처럼 말이죵.
스케줄러가 잘 동작할지 걱정되는건 이해합니다 ㅎㅎ 이럴 때 있는게 dev 서버니까, dev 서버에 한 번 올려서 세미나 만들고 출석안하고 기다려보시죵 출석 하지 않은 회원들이 금요일 자정 지나서 결석으로 바뀌는지 dev 서버에서 보시면 될 것 같습니다.
+ 하나 궁금한게 있는데 회원별 SeminarAttendace
는 세미나가 생성될 때 생기는걸까요? 그럼 세미나가 생긴 후에 가입한 회원은 그 세미나에서 출석을 할 수 없는건지..?
private static final long SEMINAR_ABSENCE_MERIT_TYPE_ID = 2; | ||
private static final long SEMINAR_DUAL_LATENESS_MERIT_TYPE_ID = 3; |
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.
상벌점 종류가 추가될 때마다 이렇게 DB에 하드하게 넣어야 하는걸까요?
만약 그렇다면, id numbering은 어떻게 하죠? 겹치는게 있을수도, 빠뜨린게 있을 수도 있지 않을까요?
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.
세미나 결석
과 지각 2회
상벌점 사유는 세미나 출석 구현할 때 필요하기도 해서, 기본 사유로 등록해두는 게 좋을 것 같다고 판단했습니다.! 그래서 하드하게 넣는 건 이거 두개만 해당할 것 같아요! 나머지는 상벌점 추가 api로...
checkAttemptNumberLimit(key); | ||
|
||
validAttendanceCode(seminar, request, key); | ||
validAttendanceCode(seminar, attendanceCode, key); | ||
checkDuplicateAttendance(seminarAttendance); |
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.
check
나 valid
로 prefix를 통일하면 좋을 것 같습니다~
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.
check로 통일하겠습니다 ㅎㅎ
@@ -86,4 +111,15 @@ public void changeStatus(long seminarId, long memberId, SeminarAttendanceStatusR | |||
|
|||
seminarAttendance.changeStatus(request.excuse(), request.statusType()); | |||
} | |||
|
|||
@Scheduled(cron = "0 0 0 ? * 5", zone = "Asia/Seoul") // 매주 금요일 자정에 실행 |
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.
세미나가 항상 금요일에 한다는 보장은 없으니 그냥 매일 자정에 cron이 돌도록 해도 좋을 것 같아요~ 만약 세미나가 없다면 어차피 find 할 때 안찾아질테니까요 ㅎㅎ
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때도 같은 방식) 이렇게 하면 문제가
|
세미나가 생성될 때만 하지 말고, 회원이 추가되거나 활동인원으로 바뀔 때 (이 기회에 |
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.
수고많으셨습니다~ 저도 옵저버 패턴 한번 살펴봐야겠어요!!
이 문제에 대한 해결방법이 있는건가... 하면서 고민 진짜 많이 했는데
옵저버 패턴을 연구해서 적용해보도록 하겠습니다 👍 감사합니다! |
아 생각해보니 자정에 스케줄러 돌릴 때 처리해도 되긴 하겠네요 ㅋㅋㅋ 차선책으로 두겠습니당! |
남은 부분 처리는 #239 에서 처리하겠습니다~! |
# Conflicts: # src/main/java/com/keeper/homepage/domain/member/entity/Member.java
🔥 Related Issue
📝 Description
⭐️ Review Request
지각 2회도 결석과 구분되어야 할 벌점 사유인 것 같아
merit_type
에 기본으로 데이터 넣도록 하겠습니다! -> 기획자님께 전달하기 ⭐️