-
Notifications
You must be signed in to change notification settings - Fork 0
[Feat] 챌린지 라운드 연장 알림 구현 #189
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
Conversation
- 알림 발생의 상세 원인(Context) 식별을 위해 ResourceType에 ROUND를 추가 - NotificationType 엔티티의 typeName 필드 타입을 String에서 Enum으로 변경 - 알림 타입 미존재 시 사용할 NOTIFICATION_TYPE_NOT_FOUND 에러 코드를 추가
- NotificationTypeRepository에서 Enum 타입을 통해 알림 설정을 조회 - RoundRecordRepository에 알림 대상 유저와 알림 설정 정보를 한 번에 가져오는 Fetch Join 쿼리
- ChallengeExtensionEvent를 통한 이벤트 기반 구조를 적용 - NotificationEventListener에서 비동기(@async)로 알림 데이터를 생성하고 NotificationDelivery를 저장 - 기획 이미지에 명시된 챌린지별 맞춤 제목과 줄바꿈이 포함된 메시지 멘트를 반영 - 앱 내 알림 목록 노출 요구사항에 따라 수신 설정 여부와 관계없이 모든 참여자에게 데이터를 생성
- 알림 설정을 꺼둔 사용자도 DB(알림 목록)에는 정상적으로 저장되는지 확인하는 검증 로직 - 동일 라운드에 대해 하루 두 번 이상 알림이 생성되지 않는지 멱등성 보장 여부를 테스트
- NotificationEventListener에서 알림 생성 시 챌린지의 imageKey를 전달받아 저장하도록 로직을 수정 - NotificationEventListenerTest에서 챌린지의 이미지 키가 알림 이벤트에 정확히 매핑되어 저장되는지 검증하는 테스트 케이스를 업데이트
- notification_event 테이블에 image_key 컬럼을 추가 - context_type 및 target_type Enum에 'ROUND' 값을 추가 - notification_type의 type_name 컬럼을 VARCHAR에서 ENUM 타입으로 변경
📝 WalkthroughWalkthrough챌린지 라운드 종료 3일 전 알림 워크플로우가 추가되었습니다: 스케줄러가 대상 라운드를 찾고 이벤트를 발행하면 트랜잭션 커밋 후 비동기 리스너가 중복 검사, 알림 타입 조회, NotificationEvent(이미지키 포함) 및 NotificationDelivery 생성/저장 작업을 수행합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Scheduler as NotificationScheduler
participant Publisher as ApplicationEventPublisher
participant Listener as NotificationEventListener
participant RoundRepo as RoundRepository
participant NotifEventRepo as NotificationEventRepository
participant NotifTypeRepo as NotificationTypeRepository
participant RoundRecordRepo as RoundRecordRepository
participant DB as Database
Scheduler->>RoundRepo: find rounds where endDate = today + CHALLENGER_DECISION_DAYS
RoundRepo-->>Scheduler: rounds
loop for each round
Scheduler->>Publisher: publish(ChallengeExtensionEvent(roundId))
end
Publisher->>Listener: handleChallengeExtensionEvent(event) [`@Async`, AFTER_COMMIT, REQUIRES_NEW]
Listener->>NotifEventRepo: existsByContextTypeAndContextIdAndCreatedAtAfter(ROUND, roundId, cutoff)
alt not exists
NotifEventRepo-->>Listener: false
Listener->>RoundRepo: findByIdWithChallenge(roundId)
RoundRepo-->>Listener: Round + Challenge(+imageKey)
Listener->>NotifTypeRepo: findByTypeName(CHALLENGE_EXTENSION)
NotifTypeRepo-->>Listener: NotificationType
Listener->>NotifEventRepo: save(NotificationEvent{context=ROUND, contextId, imageKey, message})
NotifEventRepo-->>DB: INSERT notification_event
Listener->>RoundRecordRepo: findAllByRoundWithUserAndSetting(round, status)
RoundRecordRepo-->>Listener: RoundRecord list (with user & setting)
Listener->>DB: INSERT notification_delivery (per recipient)
DB-->>Listener: OK
else exists
NotifEventRepo-->>Listener: true
Note right of Listener: idempotent → exit
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
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 |
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.
Actionable comments posted: 3
🧹 Nitpick comments (8)
src/main/java/com/hrr/backend/domain/round/repository/RoundRecordRepository.java (1)
55-64: 하드코딩된 문자열 대신 Enum 상수 사용을 권장합니다.쿼리에서
'JOINED'문자열을 직접 사용하고 있습니다.UserChallenge.status필드가 Enum 타입일 가능성이 높으므로, 문자열 대신 Enum 상수를 참조하는 것이 타입 안전성과 유지보수성 측면에서 더 좋습니다.개선 방법:
- JPQL에서 Enum을 직접 참조:
"AND uc.status = com.hrr.backend.domain.user.entity.UserChallengeStatus.JOINED"- 또는 파라미터로 전달하여 바인딩
관련 자료:
JOIN FETCH를 활용한 N+1 문제 예방은 훌륭한 접근입니다! 👍
🔎 Enum 사용 예시
@Query("SELECT rr FROM RoundRecord rr " + "JOIN FETCH rr.userChallenge uc " + "JOIN FETCH uc.user u " + "JOIN FETCH u.notificationSetting " + "WHERE rr.round = :round " + - "AND uc.status = 'JOINED'") + "AND uc.status = com.hrr.backend.domain.user.entity.UserChallengeStatus.JOINED") List<RoundRecord> findAllByRoundWithUserAndSetting(@Param("round") Round round);src/main/java/com/hrr/backend/domain/notification/repository/NotificationTypeRepository.java (1)
1-14: 표준적인 Spring Data JPA Repository 구현입니다!
NotificationTypeNameenum을 사용한 타입 안전한 조회 메서드가 잘 구현되었습니다.참고사항 (선택):
인터페이스에@Repository어노테이션은 기술적으로 불필요합니다.JpaRepository를 상속하는 인터페이스는 Spring Data JPA가 자동으로 감지하고 빈으로 등록합니다. 하지만 명시적으로 표기하는 것이 팀 컨벤션이라면 유지하셔도 무방합니다.관련 자료:
src/main/java/com/hrr/backend/global/scheduler/NotificationScheduler.java (1)
23-32: 스케줄러에 예외 처리를 추가하는 것을 권장합니다.현재 스케줄러 메서드에 예외 처리가 없어서, 특정 라운드 처리 중 예외가 발생하면 나머지 라운드의 알림이 전송되지 않을 수 있습니다. 또한 예외가 전파되어 스케줄러 자체가 중단될 수 있습니다.
권장사항:
- 각 라운드 처리를 try-catch로 감싸서 개별 실패가 전체 배치에 영향을 주지 않도록 처리
- 로깅을 추가하여 실패한 케이스를 추적
- 필요시 재시도 로직이나 Dead Letter Queue 고려
🔎 제안하는 예외 처리 구조
+import lombok.extern.slf4j.Slf4j; + +@Slf4j @Component @RequiredArgsConstructor public class NotificationScheduler { private final RoundRepository roundRepository; private final ApplicationEventPublisher eventPublisher; @Transactional(readOnly = true) @Scheduled(cron = "0 0 9 * * *") public void scheduleChallengeExtensionNotifications() { LocalDate targetDate = LocalDate.now().plusDays(Challenge.CHALLENGER_DECISION_DAYS); List<Round> targetRounds = roundRepository.findAllByEndDate(targetDate); + int successCount = 0; + int failCount = 0; + for (Round round : targetRounds) { - eventPublisher.publishEvent(new ChallengeExtensionEvent(round.getId())); + try { + eventPublisher.publishEvent(new ChallengeExtensionEvent(round.getId())); + successCount++; + } catch (Exception e) { + log.error("Failed to publish ChallengeExtensionEvent for round: {}", round.getId(), e); + failCount++; + } } + + log.info("Challenge extension notification scheduling completed. Success: {}, Failed: {}", + successCount, failCount); } }src/test/java/com/hrr/backend/domain/notification/listener/NotificationEventListenerTest.java (2)
45-105: 테스트 커버리지가 잘 구성되어 있습니다! 👍성공 케이스와 멱등성 검증이 잘 작성되어 있습니다. 다만, 몇 가지 엣지 케이스 테스트 추가를 고려해보세요:
Round를 찾지 못했을 때GlobalException(ROUND_NOT_FOUND)발생 검증NotificationType을 찾지 못했을 때GlobalException(NOTIFICATION_TYPE_NOT_FOUND)발생 검증- 참여자가 없는 경우 (
records가 비어있을 때) 정상 처리 검증이러한 테스트를 추가하면 예외 경로에 대한 신뢰도가 높아집니다.
98-101:argThat검증 시 null 안전성 고려 필요
savedEvent.getImageKey()가null일 경우NullPointerException이 발생할 수 있습니다. 테스트의 안정성을 위해 null 체크를 추가하는 것이 좋습니다.🔎 제안하는 수정
verify(eventRepository).save(argThat(savedEvent -> - savedEvent.getImageKey().equals(testImageKey) && - savedEvent.getTitle().contains("테스트 챌린지") + testImageKey.equals(savedEvent.getImageKey()) && + savedEvent.getTitle() != null && + savedEvent.getTitle().contains("테스트 챌린지") ));src/main/java/com/hrr/backend/domain/notification/listener/NotificationEventListener.java (2)
47-53: 비동기 컨텍스트에서 예외 처리 보강 권장
@Async메서드에서 발생하는 예외는 호출자에게 전파되지 않습니다.GlobalException이 발생하면 로그 없이 실패할 수 있으므로, try-catch로 감싸서 로깅하거나AsyncUncaughtExceptionHandler를 구성하는 것을 권장합니다.🔎 예외 로깅 추가 예시
@Async("getAsyncExecutor") @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) @Transactional(propagation = Propagation.REQUIRES_NEW) public void handleChallengeExtensionEvent(ChallengeExtensionEvent event) { try { Long roundId = event.getRoundId(); // ... existing logic ... } catch (Exception e) { log.error("챌린지 연장 알림 처리 실패: RoundId={}", event.getRoundId(), e); throw e; // 필요 시 재시도 로직 고려 } }Spring 공식 문서에서 AsyncUncaughtExceptionHandler 설정 방법을 참고하세요.
41-45: 명시적 ZoneId 사용으로 타임존 안정성 강화좋은 소식은
HrrBackendApiApplication에서@PostConstruct를 통해TimeZone.setDefault(TimeZone.getTimeZone("Asia/Seoul"))로 타임존을 명시적으로 설정했다는 점입니다. 따라서 현재 코드는 의도대로 동작할 것입니다.다만, 더 견고한 코드로 개선하기 위해
LocalDate.now()대신 명시적으로ZoneId를 지정하는 것을 추천합니다:LocalDateTime.now(ZoneId.of("Asia/Seoul")).atStartOfDay()또는 애플리케이션 상수로 ZoneId를 관리하면 서버 타임존 설정 변경이나 실수로 인한 오류를 사전에 방지할 수 있습니다. 이는 Java 공식 문서(java.time.ZoneId)에서 권장하는 방식입니다.
src/main/resources/db/migration/V2.24__update_notification_entities_and_types.sql (1)
1-38: 프로시저 패턴 사용은 좋지만 롤백 전략 부재Flyway에서 DDL 변경은 기본적으로 롤백되지 않습니다. 문제 발생 시 수동 롤백 스크립트를 준비해두는 것이 좋습니다. 특히 ENUM 변경은 되돌리기 어려울 수 있습니다.
🔎 롤백 스크립트 예시 (별도 파일로 관리)
-- V2.24_ROLLBACK__update_notification_entities_and_types.sql (수동 실행용) -- image_key 컬럼 제거 ALTER TABLE notification_event DROP COLUMN image_key; -- ENUM 원복 (이전 값으로) -- ALTER TABLE notification_event MODIFY COLUMN context_type ENUM('BADGE', 'CHALLENGE', 'COMMENT', 'USER', 'VERIFICATION');
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/main/java/com/hrr/backend/domain/notification/entity/NotificationEvent.javasrc/main/java/com/hrr/backend/domain/notification/entity/NotificationType.javasrc/main/java/com/hrr/backend/domain/notification/entity/enums/NotificationTypeName.javasrc/main/java/com/hrr/backend/domain/notification/entity/enums/ResourceType.javasrc/main/java/com/hrr/backend/domain/notification/event/ChallengeExtensionEvent.javasrc/main/java/com/hrr/backend/domain/notification/listener/NotificationEventListener.javasrc/main/java/com/hrr/backend/domain/notification/repository/NotificationEventRepository.javasrc/main/java/com/hrr/backend/domain/notification/repository/NotificationTypeRepository.javasrc/main/java/com/hrr/backend/domain/round/repository/RoundRecordRepository.javasrc/main/java/com/hrr/backend/global/response/ErrorCode.javasrc/main/java/com/hrr/backend/global/scheduler/NotificationScheduler.javasrc/main/resources/db/migration/V2.24__update_notification_entities_and_types.sqlsrc/test/java/com/hrr/backend/domain/notification/listener/NotificationEventListenerTest.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/hrr/backend/domain/notification/listener/NotificationEventListener.java (1)
src/main/java/com/hrr/backend/global/scheduler/NotificationScheduler.java (1)
Component(16-33)
🔇 Additional comments (13)
src/main/java/com/hrr/backend/domain/round/repository/RoundRecordRepository.java (1)
3-3: 임포트 추가 확인 완료!새로운 메서드의 파라미터 타입으로
Round를 사용하기 위한 임포트 추가가 적절합니다.src/main/java/com/hrr/backend/domain/notification/entity/enums/ResourceType.java (1)
8-9: 간결하고 명확한 Enum 확장입니다!라운드 관련 알림 컨텍스트를 지원하기 위한
ROUND상수 추가가 적절합니다. 네이밍도 기존 컨벤션과 일관성이 있습니다.src/main/java/com/hrr/backend/domain/notification/entity/enums/NotificationTypeName.java (1)
1-5: 확장 가능한 깔끔한 Enum 설계입니다!현재는
CHALLENGE_EXTENSION하나만 정의되어 있지만, 향후 다른 알림 타입을 추가하기 용이한 구조입니다. 한글 주석도 코드 이해에 도움이 됩니다.참고:
Enum 상수가 늘어나면 각 타입별 메타데이터(예: 기본 메시지 템플릿, 우선순위)를 enum에 포함하는 것도 고려해볼 수 있습니다.src/main/java/com/hrr/backend/domain/notification/event/ChallengeExtensionEvent.java (1)
6-10: 깔끔한 이벤트 클래스 구조입니다!불변성을 잘 유지하고 있으며, 이벤트 기반 아키텍처에 적합한 단순한 구조입니다.
src/main/java/com/hrr/backend/domain/notification/entity/NotificationType.java (1)
24-26: 타입 안정성을 높이는 좋은 리팩토링입니다!
String에서Enum으로 변경하여 컴파일 타임에 타입 검증이 가능해졌습니다.EnumType.STRING사용으로 DB 컬럼 순서 변경에도 안전합니다.src/main/java/com/hrr/backend/global/response/ErrorCode.java (1)
155-155: 적절한 에러 코드 추가입니다.다른 알림 관련 에러 코드들과 일관된 패턴을 따르고 있으며, 넘버링도 순차적으로 잘 관리되고 있습니다.
src/main/java/com/hrr/backend/domain/notification/repository/NotificationEventRepository.java (1)
10-15: 멱등성 체크를 위한 적절한 메서드 추가입니다!Spring Data JPA의 메서드 네이밍 컨벤션을 정확히 따르고 있으며, 메서드명만으로도 의도가 명확하게 전달됩니다. 중복 알림 생성을 방지하는 중요한 역할을 수행합니다.
src/main/java/com/hrr/backend/global/scheduler/NotificationScheduler.java (1)
26-26: ✓Challenge.CHALLENGER_DECISION_DAYS상수가 Challenge 엔티티에 정의되어 있습니다.확인 결과, 상수가 Challenge 클래스의 33번 라인에
public static final int CHALLENGER_DECISION_DAYS = 3;로 명확하게 정의되어 있습니다. NotificationScheduler에서의 참조는 유효하며 문제가 없습니다.src/test/java/com/hrr/backend/domain/notification/listener/NotificationEventListenerTest.java (1)
107-124: 멱등성 테스트 LGTM! 🎯중복 알림 생성 방지 로직이 정확히 검증되고 있습니다.
eq()매처와never()검증을 적절히 활용했네요.src/main/java/com/hrr/backend/domain/notification/listener/NotificationEventListener.java (3)
35-38: 비동기 이벤트 리스너 구성 LGTM! ✨
@Async+@TransactionalEventListener(AFTER_COMMIT)+@Transactional(REQUIRES_NEW)조합이 올바르게 구성되어 있습니다. 스케줄러의 읽기 전용 트랜잭션 커밋 후 새 트랜잭션에서 비동기로 처리되므로 트랜잭션 격리가 잘 되어 있네요.
55-66: NotificationEvent 생성 로직 LGTM! 🎉
imageKey필드가 올바르게 포함되어 있고, 챌린지 제목을 포함한 메시지 포맷팅이 잘 되어 있습니다.contextType을ROUND로,targetType을CHALLENGE로 구분한 설계가 명확합니다.
68-84: 수신자 처리 및 일괄 저장 LGTM! 👏모든 참여자에게
NotificationDelivery를 생성하는 비즈니스 요구사항이 잘 구현되어 있습니다.saveAll()을 사용한 배치 저장도 효율적입니다.한 가지 고려사항: 참여자가 매우 많은 경우(예: 수천 명), 단일 트랜잭션에서 모든 Delivery를 저장하면 메모리 및 DB 락 이슈가 발생할 수 있습니다. 현재 챌린지 규모에서 문제가 없다면 괜찮지만, 향후 확장 시 배치 청크 처리를 고려해보세요.
src/main/resources/db/migration/V2.24__update_notification_entities_and_types.sql (1)
19-23: ENUM 변경 시 기존 데이터 호환성 확인 필요
context_type과target_typeENUM에'ROUND'를 추가하는 것은 좋습니다. 다만, 기존 데이터에 새 ENUM에 없는 값이 있다면 마이그레이션이 실패합니다.마이그레이션 전에 기존 데이터를 확인하세요:
SELECT DISTINCT context_type FROM notification_event; SELECT DISTINCT target_type FROM notification_event;
| @Column(name = "image_key", length = 255) | ||
| private String imageKey; |
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.
🧩 Analysis chain
🏁 Script executed:
# imageKey 관련 사용처 찾기
rg -n --type=java 'imageKey|image_key|setImageKey|getImageKey' -g '!**/test/**' -A 2 -B 2Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 15297
🏁 Script executed:
# NotificationEvent.java 전체 구조 확인
fd -n "NotificationEvent.java" --type f -x wc -l {}Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 308
🏁 Script executed:
# 이미지 키가 어떻게 생성/할당되는지 확인
rg -n --type=java 'new.*[Ss]3|imageKey\s*=' -A 2 -B 2 | head -50Repository: Hrr-HabbitRoutineResult/hrr-server
Length of output: 511
다른 엔티티와의 스키마 일관성을 유지하세요.
imageKey 필드에 length = 255 제약이 설정되어 있으나, 동일한 목적의 다른 엔티티(Challenge, UserMission, RandomMission)에는 길이 제약이 없습니다. 코드베이스의 S3 키 예시("uploads/uuid_image.jpg", "challenges/uuid_image.jpg")를 보면 255자는 충분하지만, 스키마 일관성 측면에서 개선이 필요합니다.
권장사항:
- 일관성을 위해
length = 255제약을 제거하거나, 모든imageKey필드에 동일한 제약을 적용하세요. - 필드의 용도를 명확히 하기 위해 Javadoc을 추가하면 유지보수성이 향상됩니다:
/**
* 알림에 표시될 이미지의 S3 객체 키
* API 응답 시 {@link S3UrlUtil#toFullUrl(String)}로 전체 URL로 변환됨
*/
@Column(name = "image_key")
private String imageKey;관련 참고자료: AWS S3 객체 키 명명 규칙
🤖 Prompt for AI Agents
In
src/main/java/com/hrr/backend/domain/notification/entity/NotificationEvent.java
around lines 63-64, the imageKey column has length = 255 which is inconsistent
with other entities (Challenge, UserMission, RandomMission); update for schema
consistency by either removing the length attribute here or adding length = 255
to the other imageKey fields across those entities (choose one consistent
approach for the codebase), and add a Javadoc above the field describing its
purpose, e.g., "알림에 표시될 이미지의 S3 객체 키; API 응답 시 S3UrlUtil.toFullUrl(String)로 전체
URL로 변환됨".
src/main/java/com/hrr/backend/global/scheduler/NotificationScheduler.java
Outdated
Show resolved
Hide resolved
| -- 3. notification_type 테이블: 데이터 보정 및 타입 변경 | ||
| UPDATE notification_type SET type_name = 'CHALLENGE_EXTENSION' WHERE type_name IS NOT NULL; | ||
|
|
||
| ALTER TABLE notification_type | ||
| MODIFY COLUMN type_name ENUM('CHALLENGE_EXTENSION') NOT NULL; |
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.
이 마이그레이션은 두 가지 잠재적 문제가 있습니다:
-
기존 데이터 덮어쓰기: Line 26에서 기존
type_name값을 모두'CHALLENGE_EXTENSION'으로 변경합니다. 만약 다른 타입의 알림이 이미 존재한다면 데이터가 손실됩니다. -
확장성 제한: Line 29에서 ENUM을
('CHALLENGE_EXTENSION')만 허용하도록 설정합니다. 향후 새로운 알림 타입(예:BADGE_EARNED,COMMENT_REPLY등)을 추가하려면 또 다른 마이그레이션이 필요합니다.
🔎 확장 가능한 ENUM 설계 제안
-- 3. notification_type 테이블: 데이터 보정 및 타입 변경
-UPDATE notification_type SET type_name = 'CHALLENGE_EXTENSION' WHERE type_name IS NOT NULL;
+-- 기존 데이터가 없는 경우에만 실행하거나, 기존 값 매핑 로직 추가
+UPDATE notification_type SET type_name = 'CHALLENGE_EXTENSION' WHERE type_name IS NULL OR type_name = '';
ALTER TABLE notification_type
- MODIFY COLUMN type_name ENUM('CHALLENGE_EXTENSION') NOT NULL;
+ MODIFY COLUMN type_name ENUM('CHALLENGE_EXTENSION', 'BADGE_EARNED', 'COMMENT_REPLY', 'VERIFICATION_APPROVED') NOT NULL;프로덕션 환경에 기존 알림 데이터가 있는지 확인하고, 향후 알림 타입 확장 계획이 있다면 ENUM 값을 미리 추가하는 것을 권장합니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| -- 3. notification_type 테이블: 데이터 보정 및 타입 변경 | |
| UPDATE notification_type SET type_name = 'CHALLENGE_EXTENSION' WHERE type_name IS NOT NULL; | |
| ALTER TABLE notification_type | |
| MODIFY COLUMN type_name ENUM('CHALLENGE_EXTENSION') NOT NULL; | |
| -- 3. notification_type 테이블: 데이터 보정 및 타입 변경 | |
| -- 기존 데이터가 없는 경우에만 실행하거나, 기존 값 매핑 로직 추가 | |
| UPDATE notification_type SET type_name = 'CHALLENGE_EXTENSION' WHERE type_name IS NULL OR type_name = ''; | |
| ALTER TABLE notification_type | |
| MODIFY COLUMN type_name ENUM('CHALLENGE_EXTENSION', 'BADGE_EARNED', 'COMMENT_REPLY', 'VERIFICATION_APPROVED') NOT NULL; |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/com/hrr/backend/global/scheduler/NotificationScheduler.java (1)
27-31: 로깅 추가를 고려하세요 (선택사항).운영 환경에서 스케줄러의 실행 상태를 모니터링하기 위해 로깅을 추가하는 것을 권장합니다. 예를 들어:
- 스케줄러 시작 시 대상 날짜와 발견된 라운드 수
- 각 라운드에 대한 이벤트 발행 완료
- 예외 발생 시 에러 로그
🔎 로깅 추가 예시
+import lombok.extern.slf4j.Slf4j; + +@Slf4j @Component @RequiredArgsConstructor public class NotificationScheduler { @Transactional(readOnly = true) @Scheduled(cron = "0 0 9 * * *") public void scheduleChallengeExtensionNotifications() { LocalDate targetDate = LocalDate.now().plusDays(Challenge.CHALLENGER_DECISION_DAYS); + log.info("챌린지 연장 알림 스케줄러 시작 - 대상 날짜: {}", targetDate); + List<Round> targetRounds = roundRepository.findAllByEndDate(targetDate); + log.info("알림 대상 라운드 {}개 발견", targetRounds.size()); for (Round round : targetRounds) { eventPublisher.publishEvent(new ChallengeExtensionEvent(round.getId())); + log.debug("라운드 ID {}에 대한 연장 알림 이벤트 발행 완료", round.getId()); } + + log.info("챌린지 연장 알림 스케줄러 완료 - 총 {}개 이벤트 발행", targetRounds.size()); } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/hrr/backend/global/scheduler/NotificationScheduler.java
🔇 Additional comments (2)
src/main/java/com/hrr/backend/global/scheduler/NotificationScheduler.java (2)
23-25: 스케줄링 설정이 명확하고 적절합니다!매일 오전 9시 실행되는 cron 표현식과
@Transactional(readOnly = true)설정이 읽기 전용 작업에 적합합니다. 메서드명도 의도를 잘 표현하고 있네요.
27-31: 이미 올바르게 구현되어 있습니다! 👍검증 결과,
NotificationEventListener가 정확하게@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)를 사용하고 있으며,@Transactional(propagation = Propagation.REQUIRES_NEW)로 새로운 트랜잭션에서 안전하게 처리되고 있습니다.추가로 우수한 구현 사항들도 확인됩니다:
- 멱등성 체크 (41-45줄):
eventRepository.existsByContextTypeAndContextIdAndCreatedAtAfter()로 중복 알림 방지- 비동기 처리 (35줄):
@Async로 성능 최적화- 로깅 구현 (83줄): 운영 모니터링을 위한
log.info()포함- 에러 처리:
GlobalException으로 데이터 일관성 보장트랜잭션 커밋 후 이벤트 처리로 롤백 문제도 완벽하게 해결되었습니다. 사려 깊은 구현입니다.
- 스케줄러 시작 및 계산된 대상 날짜 기록 - 조회된 대상 라운드 수 및 데이터 부재 시 상태 기록 - 각 라운드별 이벤트 발행 성공/실패 여부 개별 기록
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/test/java/com/hrr/backend/domain/notification/listener/NotificationEventListenerTest.java (1)
102-108: 검증 로직 개선을 고려해보세요현재
argThat의 람다 표현식에서getImageKey(),getTitle()의 null 체크가 누락되어 있어, 구현이 변경되면 테스트가 NPE로 실패할 수 있습니다. 또한 Line 108의 타입 캐스팅은 다소 어색합니다.🔎 더 안전한 검증 방식
- verify(eventRepository).save(argThat(savedEvent -> - savedEvent.getImageKey().equals(testImageKey) && - savedEvent.getTitle().contains("테스트 챌린지") - )); - - // 중요: 설정과 관계없이 참여자 전원(2명)에게 알림 데이터가 저장되어야 함 - verify(notificationRepository).saveAll(argThat(deliveries -> ((List<?>)deliveries).size() == 2)); + verify(eventRepository).save(argThat(savedEvent -> + testImageKey.equals(savedEvent.getImageKey()) && + savedEvent.getTitle() != null && + savedEvent.getTitle().contains("테스트 챌린지") + )); + + // 중요: 설정과 관계없이 참여자 전원(2명)에게 알림 데이터가 저장되어야 함 + ArgumentCaptor<List> captor = ArgumentCaptor.forClass(List.class); + verify(notificationRepository).saveAll(captor.capture()); + assertThat(captor.getValue()).hasSize(2);또는 Hamcrest matcher를 활용하면 더 명확합니다:
verify(notificationRepository).saveAll(argThat(deliveries -> deliveries instanceof List && ((List<?>)deliveries).size() == 2 ));
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/hrr/backend/domain/notification/listener/NotificationEventListener.javasrc/main/java/com/hrr/backend/domain/round/repository/RoundRecordRepository.javasrc/main/java/com/hrr/backend/global/scheduler/NotificationScheduler.javasrc/test/java/com/hrr/backend/domain/notification/listener/NotificationEventListenerTest.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/hrr/backend/global/scheduler/NotificationScheduler.java
- src/main/java/com/hrr/backend/domain/notification/listener/NotificationEventListener.java
🔇 Additional comments (2)
src/main/java/com/hrr/backend/domain/round/repository/RoundRecordRepository.java (1)
56-68: 효율적인 쿼리 설계입니다! 👍JOIN FETCH를 활용해 N+1 문제를 방지하고, Enum 파라미터로 타입 안정성까지 확보했네요. 알림 대상자와 설정 정보를 한 번에 조회하는 목적에 딱 맞는 구현입니다.
src/test/java/com/hrr/backend/domain/notification/listener/NotificationEventListenerTest.java (1)
111-128: 멱등성 검증이 깔끔합니다! 🎯이미 생성된 알림에 대해 중복 처리를 방지하는 로직을 명확하게 검증하고 있습니다.
never()검증으로 부수 효과가 없음을 확인하는 것도 좋습니다.
yc3697
left a comment
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.
확인했습니다! 연장 알림 저장까지만 되고 아직 fcm 전송은 안된거죠..?
넵 알림은 1차 런칭까지는 안 하는 걸로 생각했다고 해서 우선 무조건 필요로 하는 연장 알림만 전송 후 알림 목록에서 볼 수 있도록 했습니다! |
#️⃣ 연관된 이슈
✨ 작업 내용 (Summary)
챌린지 종료 3일 전 챌린저들에게 라운드 연장 여부를 안내하는 알림 생성 로직 구현
✅ 변경 사항 체크리스트
🧪 테스트 결과
📸 스크린샷
💬 리뷰 요구사항
📎 참고 자료
Summary by CodeRabbit
새로운 기능
개선사항
테스트
데이터베이스
오류 처리
✏️ Tip: You can customize this high-level summary in your review settings.