Repository navigation
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 50 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
22d0521 to
2adbdb3
Compare
| private final EventRepository eventRepository; | ||
| private final OutboxWriter outboxWriter; | ||
|
|
||
| @Scheduled(cron = "0 0 10 * * *", zone = "Asia/Seoul") |
There was a problem hiding this comment.
cron으로 고정 시각 표현한 점 이해했습니다.
| @EventListener | ||
| public void on(EventStartingTomorrow event) { | ||
| notificationDispatcher.dispatch( | ||
| NotificationType.EVENT_STARTING_TOMORROW, List.of(event.eventTitle()), event.eventId(), event.applicantIds()); |
There was a problem hiding this comment.
event 모듈에서는 전체 member를 참조할 수 없기 때문에, member 모듈에서 신청자 ID를 받고 전체에서 제외하는 형식으로 알림 발송하는 점 이해했습니다!
| List<LockerPeriod> periods = lockerRepository.findPublishedPeriodsApplyEndingBetween( | ||
| tomorrow.atStartOfDay(), tomorrow.plusDays(1).atStartOfDay()); | ||
| for (LockerPeriod period : periods) { | ||
| if (now.isBefore(period.getApplyStartAt())) { |
There was a problem hiding this comment.
지금은 사물함 신청이 다 찬 상태에서도 마감임박 알림이 가는 것으로 보이는데, 가능하다면 이 조건에 정원 마감 관련 조건도 추가해서 이미 정원이 다 찬 상태에서는 알림이 안 가도록 하는 것은 어떨까요?
There was a problem hiding this comment.
정원 마감을 확인하려면 모든 사물함이 다 마감되었는지를 확인해야 하기 때문에 이 정도는 정책상 어쩔 수 없는 부분으로 넘기는 게 맞을 거 같습니다!
| @Scheduled(cron = "0 0 10 * * *", zone = "Asia/Seoul") | ||
| @Transactional | ||
| public void recordDailyNotifications() { | ||
| LocalDateTime now = LocalDateTime.now(DateUtil.KST); |
There was a problem hiding this comment.
#112 PR에서 JVM 기본 시간대를 서울로 고정했으니 LocalDateTime.now(DateUtil.KST)는 LocalDateTime.now()와 같은 값이라, 그냥 now()로 써도 될 것 같습니다. 그리고 현재 저장소 전체에서 이 부분이 정해지지 않고 코드마다 다르게 되어있어서, 컨벤션 규약으로 정해서 어느 쪽을 쓸지 팀 규칙으로 정해두면 좋을 것 같습니다.
There was a problem hiding this comment.
이건 이미 많은 곳에서 이렇게 쓰이고 있어서 추후에 이슈를 새로 파서 진행하는 게 좋을 거 같아요. 그래서 일부러 따로 언급하지 않고 있었는데 앞으로는 다들 이거 주의해서 코드 검토해주세요~
There was a problem hiding this comment.
넵 확인했습니다 이번 열린피드백 페이지에서도 수정해서 커밋 올리겠습니다!
| public void on(RentalOverdue event) { | ||
| notificationDispatcher.dispatch( | ||
| NotificationType.RENTAL_OVERDUE, List.of(event.itemName()), event.rentalHistoryId(), List.of(event.memberId())); | ||
| } |
There was a problem hiding this comment.
applicatIds가 dispatch에서는 받을 사람으로 쓰이고 dispatchToAll에서는 뺄 사람으로 쓰이는것으로 이해했습니다!!
| outboxWriter.record(new EventStartingTomorrow(event.getId(), event.getTitle(), applicantIds)); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
구조상 event 모듈에서 member를 볼 수 없어서 스케줄러가 신청자 ID만 싣고 넘기는 구조 이해했습니다.
2adbdb3 to
98412f5
Compare
#️⃣연관된 이슈
🎯 해결하려는 문제가 무엇인가요?
정해진 시각에 가는 예약 알림 6종을 보냅니다. 모든 시각은 KST입니다.
calculateRecruitStatus == OPEN)return_deadline)이 물품마다 달라 고정 시각 cron 하나로는 보낼 수 없어 이렇게 했습니다.❓ 왜 해결해야 하나요?
#120까지는 사용자 행동 직후 알림만 있고, 시간이 지나서 가야 하는 알림(전날 안내, 마감 임박, 반납 기한)은 보내는 곳이 없었습니다.
⭐ 어떻게 해결했나요?
common.event):EventStartingTomorrow,EventApplyClosingTomorrow,LockerApplyClosingTomorrow,LockerUsageEndingSoon,RentalDueSoon,RentalOverdue. 신청자 기준 알림은 신청자 ID 목록을 싣고, "미신청자" 알림은 신청자 ID를 제외 목록으로 실어 member 쪽이 "활성 회원 − 신청자"를 계산합니다(event 모듈은 회원 전체를 모름).service.impl, package-private): 자기 데이터로 대상을 고르고OutboxWriter로 기록만 합니다. 메서드에@Transactional(아웃박스 기록은 트랜잭션 필수), cron에zone = "Asia/Seoul".EventNotificationScheduler(event 모듈)LockerNotificationScheduler(event 모듈)RentalNotificationScheduler(welfare 모듈). 반납 기한은 기존RentalRecord.dueAt()(물품 반납 정책 기준)으로 계산합니다. 반납 정책이 없는 물품은 기한을 계산할 수 없어 빠집니다.EventRepository: 시작·마감 일시 구간으로 게시 행사 조회, 유효 신청자 IDLockerRepository·LockerApplicationRepository: 마감 일시 구간·이용 종료일로 게시 회차 조회, 회차 신청자 IDRentalHistoryRepository.findAllByStatus,RentalHistoryService.getAllReturnRequiredRentals()(기존 회원별 조회와 같은 짝짓기 로직)NotificationEventListener에 6종 메서드를 추가했습니다.architecture.md6-2절에 예약 이벤트 스케줄러 위치·규칙을 추가했습니다.EventServiceImplTest,LockerServiceImplTest,LockerApplicationServiceImplTest의 가짜 저장소가 포트를 직접 구현해서, 새 메서드에 기존 방식대로UnsupportedOperationException스텁을 추가했습니다.🧩 이 PR의 한계 & 트레이드오프
MON-FRI). 금요일 10시 이후에 연체된 대여는 월요일 10시에 첫 알림이 갑니다. 공휴일은 따로 거르지 않아 평일 공휴일에는 보냅니다.apply_end_at을 확인하지 않아, "내일 마감" 알림 뒤에도 신청이 막히지 않습니다(기존 동작, 이번 범위 아님).RENTAL상태 이력 전체와 물품을 읽습니다.rental_status인덱스가 없지만 대여 건수가 적어 두지 않았습니다.ModularityTests·DomainImplAccessTests까지 확인했습니다. 로컬에 DB가 없어 새 JPQL·파생 쿼리와 스케줄 실행은 dev에서 확인해야 합니다(대상 행의 날짜를 내일로 바꾼 뒤 10시 실행 확인, 또는 일시적으로 cron을 바꿔 확인).⛓️ 기존 기능에 미치는 영향
outbox_eventsINSERT가 생깁니다.RentalHistoryService에 조회 메서드가 하나 늘었습니다.🔀 Edge Case & 실패 시나리오
📋 검토한 대안과 선택 이유
💬 리뷰 포인트
floorToCheckInterval)