[LNK-22] Leenk 생일 축하 알림 구현 #73
Conversation
- 생일 알림 수신 설정이 활성화된 유저 조회 메서드 추가
- BirthdayAnnouncementContent 클래스 추가 - NotificationType enum에 BIRTHDAY_ANNOUNCEMENT 상수 추가 - 매퍼 추가 - 생일 알림 usecase 구현(본인 제외)
Walkthrough생일 알림 시스템을 추가합니다. 생일 공지·축하·편지용 알림 타입과 콘텐츠 엔티티를 도입하고, 일일 스케줄러와 알림 발송 유스케이스/매퍼를 구현하며 사용자 설정에 생일 알림 옵션을 추가합니다. BirthdayLetter 저장 흐름에서 편지 알림 발송 호출도 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor Scheduler as 스케줄러 (자정)
participant BirthdayNotifyScheduler
participant BirthdayNotificationUsecase
participant UserService as 사용자 서비스
participant BirthdayNotificationMapper
participant NotificationRepo as 알림 저장소
participant SqsPublisher as SQS 발행자
participant BirthdayLetterUsecase
Scheduler->>BirthdayNotifyScheduler: sendBirthdayNotifications()
BirthdayNotifyScheduler->>BirthdayNotificationUsecase: announceUserBirthday(today)
BirthdayNotificationUsecase->>UserService: 오늘 생일 사용자 조회
BirthdayNotificationUsecase->>UserService: 알림 수신 대상 조회 (isBirthdayNotify=true)
loop 수신자 × 생일 사용자
alt 본인 아님
BirthdayNotificationUsecase->>BirthdayNotificationMapper: toBirthdayAnnouncementNotification()
BirthdayNotificationMapper-->>BirthdayNotificationUsecase: Notification
BirthdayNotificationUsecase->>NotificationRepo: save(Notification)
alt 수신자 FCM 토큰 존재
BirthdayNotificationUsecase->>SqsPublisher: publish 메시지
end
end
end
BirthdayNotifyScheduler->>BirthdayNotificationUsecase: celebrateBirthday(today)
BirthdayNotificationUsecase->>UserService: 오늘 생일 사용자 조회
loop 생일 사용자
alt 사용자 알림 활성화
BirthdayNotificationUsecase->>BirthdayNotificationMapper: toBirthdayCelebrateNotification()
BirthdayNotificationMapper-->>BirthdayNotificationUsecase: Notification
BirthdayNotificationUsecase->>NotificationRepo: save(Notification)
alt FCM 토큰 존재
BirthdayNotificationUsecase->>SqsPublisher: publish 메시지
end
end
end
BirthdayLetterUsecase->>BirthdayNotificationUsecase: saveBirthdayLetterNotification(birthdayLetter)
BirthdayNotificationUsecase->>BirthdayNotificationMapper: toBirthdayLetterNotification()
BirthdayNotificationMapper-->>BirthdayNotificationUsecase: Notification
BirthdayNotificationUsecase->>NotificationRepo: save(Notification)
BirthdayNotificationUsecase->>UserService: 수신자 설정 확인
alt 알림 활성화 및 FCM 토큰 존재
BirthdayNotificationUsecase->>SqsPublisher: publish 메시지
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
src/main/java/leets/leenk/domain/notification/domain/service/scheduler/BirthdayNotifyScheduler.java (1)
15-15: 타임존 설정 일관성 개선 제안KST 상수를 정의했지만
@Scheduled어노테이션에는 문자열로 다시 지정하고 있습니다. 일관성을 위해 상수를 재사용하는 것을 고려해보세요.- private static final ZoneId KST = ZoneId.of("Asia/Seoul"); + private static final String ZONE_SEOUL = "Asia/Seoul"; + private static final ZoneId KST = ZoneId.of(ZONE_SEOUL); - @Scheduled(cron = "0 0 0 * * *", zone = "Asia/Seoul") + @Scheduled(cron = "0 0 0 * * *", zone = ZONE_SEOUL)Also applies to: 19-19
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
src/main/java/leets/leenk/domain/birthday/application/usecase/BirthdayLetterUseCase.java(3 hunks)src/main/java/leets/leenk/domain/media/application/exception/MediaFailedUpdateException.java~(0 hunks)src/main/java/leets/leenk/domain/notification/application/mapper/BirthdayNotificationMapper.java(1 hunks)src/main/java/leets/leenk/domain/notification/application/usecase/BirthdayNotificationUsecase.java(1 hunks)src/main/java/leets/leenk/domain/notification/domain/entity/NotificationContent.java(2 hunks)src/main/java/leets/leenk/domain/notification/domain/entity/birthdayContent/BirthdayAnnouncementContent.java(1 hunks)src/main/java/leets/leenk/domain/notification/domain/entity/birthdayContent/BirthdayCelebrateContent.java(1 hunks)src/main/java/leets/leenk/domain/notification/domain/entity/birthdayContent/BirthdayLetterContent.java(1 hunks)src/main/java/leets/leenk/domain/notification/domain/entity/enums/NotificationType.java(1 hunks)src/main/java/leets/leenk/domain/notification/domain/service/scheduler/BirthdayNotifyScheduler.java(1 hunks)src/main/java/leets/leenk/domain/user/application/dto/request/NotificationSettingUpdateRequest.java(1 hunks)src/main/java/leets/leenk/domain/user/domain/entity/UserSetting.java(2 hunks)src/main/java/leets/leenk/domain/user/domain/repository/UserRepository.java(1 hunks)src/main/java/leets/leenk/domain/user/domain/repository/UserSettingRepository.java(1 hunks)src/main/java/leets/leenk/domain/user/domain/service/user/UserGetService.java(2 hunks)src/main/java/leets/leenk/domain/user/domain/service/usersetting/UserSettingGetService.java(1 hunks)src/main/java/leets/leenk/domain/user/domain/service/usersetting/UserSettingUpdateService.java(1 hunks)src/main/java/leets/leenk/global/sqs/application/mapper/AwsSqsManager.java(1 hunks)src/main/java/leets/leenk/global/sqs/application/mapper/SqsMessageEventMapper.java(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/leets/leenk/domain/media/application/exception/MediaFailedUpdateException.java~
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/leets/leenk/domain/notification/application/usecase/BirthdayNotificationUsecase.java (2)
src/main/java/leets/leenk/domain/birthday/application/usecase/BirthdayLetterUseCase.java (1)
Service(24-77)src/main/java/leets/leenk/domain/notification/domain/service/scheduler/BirthdayNotifyScheduler.java (1)
Service(11-27)
src/main/java/leets/leenk/domain/notification/domain/service/scheduler/BirthdayNotifyScheduler.java (1)
src/main/java/leets/leenk/domain/notification/application/usecase/BirthdayNotificationUsecase.java (1)
Service(19-104)
src/main/java/leets/leenk/domain/notification/application/mapper/BirthdayNotificationMapper.java (1)
src/main/java/leets/leenk/global/sqs/application/mapper/SqsMessageEventMapper.java (1)
Component(14-106)
🔇 Additional comments (15)
src/main/java/leets/leenk/domain/user/application/dto/request/NotificationSettingUpdateRequest.java (1)
16-19: LGTM!생일 알림 설정 필드가 기존 패턴과 일관되게 추가되었습니다. Swagger 문서화도 적절합니다.
src/main/java/leets/leenk/domain/notification/domain/entity/NotificationContent.java (2)
5-7: LGTM!생일 관련 컨텐츠 클래스 임포트가 적절히 추가되었습니다.
34-37: LGTM!세 가지 생일 알림 타입이 기존 패턴과 일관되게 JsonSubTypes 매핑에 추가되었습니다.
src/main/java/leets/leenk/domain/user/domain/service/usersetting/UserSettingUpdateService.java (1)
27-29: LGTM!생일 알림 설정 업데이트 로직이 기존 패턴과 일관되게 구현되었습니다.
src/main/java/leets/leenk/global/sqs/application/mapper/AwsSqsManager.java (1)
40-45: null 처리 전략 검증 필요String null 값을 공백
" "으로 처리하는 이유를 확인해 주세요. 일반적으로 빈 문자열""이 더 적절할 수 있습니다.다음 사항들을 검증해 주세요:
- SQS 메시지 속성에서 공백
" "이 빈 문자열""보다 선호되는 이유- title, fcmToken, path 등의 속성이 null일 때 공백으로 처리하는 것이 의도된 동작인지 확인
src/main/java/leets/leenk/domain/notification/domain/entity/birthdayContent/BirthdayCelebrateContent.java (1)
1-13: LGTM!생일 축하 알림 컨텐츠 엔티티가 기존 패턴과 일관되게 구현되었습니다. Lombok 어노테이션과 상속 구조가 적절합니다.
src/main/java/leets/leenk/domain/user/domain/entity/UserSetting.java (2)
34-35: LGTM! 생일 알림 설정 필드가 올바르게 추가되었습니다.다른 알림 설정 필드들과 동일한 패턴을 따르고 있으며, 기본값이 true로 설정되어 있어 사용자 경험이 좋습니다.
59-61: LGTM! 업데이트 메서드가 올바르게 추가되었습니다.다른 설정 업데이트 메서드들과 일관된 패턴을 유지하고 있습니다.
src/main/java/leets/leenk/domain/notification/domain/entity/birthdayContent/BirthdayLetterContent.java (1)
1-14: LGTM! 생일 편지 콘텐츠 클래스가 올바르게 구현되었습니다.다른 콘텐츠 클래스들과 일관된 구조를 따르고 있으며, 필요한 정보(발신자 이름, 편지 ID)를 적절히 포함하고 있습니다.
src/main/java/leets/leenk/domain/notification/domain/service/scheduler/BirthdayNotifyScheduler.java (1)
20-26: LGTM! 스케줄러 로직이 올바르게 구현되었습니다.매일 자정(KST)에 생일 알림을 전송하는 로직이 정확하며, 두 가지 유형의 알림(공지 및 축하)을 순차적으로 처리하는 것이 적절합니다.
src/main/java/leets/leenk/domain/notification/domain/entity/enums/NotificationType.java (1)
27-30: LGTM! 생일 알림 타입들이 올바르게 추가되었습니다.세 가지 생일 알림 타입(공지, 축하, 편지 수신)이 적절하게 정의되었으며,
{name}플레이스홀더를 사용하여 동적으로 이름을 치환할 수 있도록 설계되었습니다.src/main/java/leets/leenk/domain/notification/domain/entity/birthdayContent/BirthdayAnnouncementContent.java (1)
1-13: LGTM! 생일 공지 콘텐츠 클래스가 올바르게 구현되었습니다.다른 생일 콘텐츠 클래스들과 일관된 구조를 따르고 있으며, 생일자 이름을 저장하는 간단하고 명확한 설계입니다.
src/main/java/leets/leenk/domain/birthday/application/usecase/BirthdayLetterUseCase.java (1)
12-12: LGTM! 생일 편지 알림 통합이 올바르게 구현되었습니다.생일 편지 저장 후 알림을 전송하는 흐름이 적절하며, 의존성 주입도 올바르게 설정되었습니다.
Also applies to: 32-32, 49-50
src/main/java/leets/leenk/domain/user/domain/repository/UserSettingRepository.java (1)
26-28: LGTM! 생일 알림 사용자 조회 쿼리가 올바르게 구현되었습니다.활성 사용자(
leaveDate IS NULL) 중 생일 알림을 활성화한 사용자를 조회하는 쿼리가 정확합니다. 다른 알림과 달리 특정 사용자를 제외하지 않는 것이 생일 공지 알림의 특성에 맞습니다.src/main/java/leets/leenk/domain/notification/application/mapper/BirthdayNotificationMapper.java (1)
14-38: 컨텐츠 템플릿 적용이 명확합니다Line 14~38에서 생일 알림 세 가지 타입을 깔끔하게 빌더로 구성해 가독성과 재사용성이 좋습니다.
src/main/java/leets/leenk/domain/user/domain/repository/UserRepository.java
Outdated
Show resolved
Hide resolved
src/main/java/leets/leenk/domain/user/domain/service/user/UserGetService.java
Outdated
Show resolved
Hide resolved
src/main/java/leets/leenk/domain/user/domain/service/usersetting/UserSettingGetService.java
Show resolved
Hide resolved
src/main/java/leets/leenk/global/sqs/application/mapper/AwsSqsManager.java
Show resolved
Hide resolved
| public SqsMessageEvent toBirthdaySqsMessageEvent(Notification notification, String fcmToken, User user) { | ||
| return SqsMessageEvent.builder() | ||
| .title(notification.getContent().getTitle()) | ||
| .content(notification.getContent().getBody().replace("{name}", | ||
| "[" + user.getName() + "]")) | ||
| .fcmToken(fcmToken) | ||
| .path(notification.getNotificationType().getPath()) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
id 파라미터 누락으로 인한 알림 추적 불가 문제
다른 모든 매핑 메서드들(toSqsMessageEvent, fromNotificationWithTag, fromFeedFirstReaction 등)은 id 파라미터를 받아 SqsMessageEvent에 설정하지만, 이 메서드만 id 파라미터가 없습니다. 이로 인해 생일 알림을 클릭했을 때 올바른 화면으로 이동하지 못할 수 있습니다.
다음과 같이 수정하세요:
- public SqsMessageEvent toBirthdaySqsMessageEvent(Notification notification, String fcmToken, User user) {
+ public SqsMessageEvent toBirthdaySqsMessageEvent(Notification notification, String fcmToken, User user, Long id) {
return SqsMessageEvent.builder()
.title(notification.getContent().getTitle())
.content(notification.getContent().getBody().replace("{name}",
"[" + user.getName() + "]"))
.fcmToken(fcmToken)
.path(notification.getNotificationType().getPath())
+ .id(id)
.build();
}그리고 이 메서드를 호출하는 BirthdayNotificationUsecase의 모든 호출 부분도 id 파라미터를 추가해야 합니다.
다음 스크립트로 호출 부분을 확인하세요:
🏁 Script executed:
#!/bin/bash
# Description: Find all calls to toBirthdaySqsMessageEvent to verify id parameter needs to be added
rg -n "toBirthdaySqsMessageEvent" --type java -A 5Length of output: 3560
🏁 Script executed:
#!/bin/bash
# Find SqsMessageEvent class definition and check for id field
find . -name "SqsMessageEvent.java" -type f | head -5Length of output: 75
🏁 Script executed:
#!/bin/bash
# Find all methods in SqsMessageEventMapper
rg -n "public SqsMessageEvent" --type java -A 2Length of output: 3298
🏁 Script executed:
#!/bin/bash
# Get context around call sites (lines before and after)
rg -n "toBirthdaySqsMessageEvent" --type java -B 10 | grep -E "(toBirthdaySqsMessageEvent|notification|\.id|\.getId|\.getNotification)"Length of output: 5920
🏁 Script executed:
#!/bin/bash
# Find Notification class and check for id field
find . -name "Notification.java" -type f -path "*/domain/*" | head -3Length of output: 79
🏁 Script executed:
#!/bin/bash
# Get full context around call sites to see what id should be
rg -n "toBirthdaySqsMessageEvent" --type java -B 15 -A 10 src/main/java/leets/leenk/domain/notification/application/usecase/BirthdayNotificationUsecase.javaLength of output: 2585
🏁 Script executed:
#!/bin/bash
# Check if Notification entity has getId method
rg -n "getId\(\)|private.*id|public.*id" --type java src/main/java/leets/leenk/domain/notification/domain/entity/Notification.java | head -10Length of output: 91
🏁 Script executed:
#!/bin/bash
# Check User class id field
rg -n "private.*id|public.*getId" --type java -A 1 src/main/java/leets/leenk/domain/user/domain/entity/User.java | head -20Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check BirthdayLetter class id field
find . -name "BirthdayLetter.java" -type f -path "*/domain/*" -exec rg -n "private.*id|public.*getId" --type java -A 1 {} \; | head -20Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check SqsMessageEvent class definition to see what id type it expects
cat src/main/java/leets/leenk/global/sqs/application/dto/SqsMessageEvent.javaLength of output: 296
id 파라미터 누락으로 인한 알림 추적 불가 문제
다른 모든 매핑 메서드들(toSqsMessageEvent, fromNotificationWithTag, fromFeedFirstReaction 등)은 Long id 파라미터를 받아 SqsMessageEvent에 설정하지만, 이 메서드만 id 파라미터가 없습니다. 이로 인해 생일 알림을 클릭했을 때 올바른 화면으로 이동하지 못할 수 있습니다.
BirthdayNotificationUsecase의 세 곳 호출 부분(52, 75, 94줄)에서 다음과 같이 수정하세요:
- 52줄:
receiver.getId()추가 - 75줄:
birthdayUser.getId()추가 - 94줄:
birthdayLetter.getSender().getId()추가
그리고 매퍼 메서드 서명을 다음과 같이 변경:
- public SqsMessageEvent toBirthdaySqsMessageEvent(Notification notification, String fcmToken, User user) {
+ public SqsMessageEvent toBirthdaySqsMessageEvent(Notification notification, String fcmToken, User user, Long id) {
return SqsMessageEvent.builder()
.title(notification.getContent().getTitle())
.content(notification.getContent().getBody().replace("{name}",
"[" + user.getName() + "]"))
.fcmToken(fcmToken)
.path(notification.getNotificationType().getPath())
+ .id(id)
.build();
}📝 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.
| public SqsMessageEvent toBirthdaySqsMessageEvent(Notification notification, String fcmToken, User user) { | |
| return SqsMessageEvent.builder() | |
| .title(notification.getContent().getTitle()) | |
| .content(notification.getContent().getBody().replace("{name}", | |
| "[" + user.getName() + "]")) | |
| .fcmToken(fcmToken) | |
| .path(notification.getNotificationType().getPath()) | |
| .build(); | |
| } | |
| public SqsMessageEvent toBirthdaySqsMessageEvent(Notification notification, String fcmToken, User user, Long id) { | |
| return SqsMessageEvent.builder() | |
| .title(notification.getContent().getTitle()) | |
| .content(notification.getContent().getBody().replace("{name}", | |
| "[" + user.getName() + "]")) | |
| .fcmToken(fcmToken) | |
| .path(notification.getNotificationType().getPath()) | |
| .id(id) | |
| .build(); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/leets/leenk/global/sqs/application/mapper/SqsMessageEventMapper.java
around lines 97 to 105, the toBirthdaySqsMessageEvent method is missing the Long
id parameter so the resulting SqsMessageEvent cannot carry the target id for
navigation; change the method signature to accept a Long id and set it on the
SqsMessageEvent builder, and then update the three call sites in
BirthdayNotificationUsecase: at line 52 pass receiver.getId(), at line 75 pass
birthdayUser.getId(), and at line 94 pass birthdayLetter.getSender().getId().
There was a problem hiding this comment.
@jj0526, 알겠습니다! 생일 푸시 알림에는 id를 전달하지 않는 것이 의도된 설계군요. 현재 구현이 요구사항에 맞게 되어 있네요. 확인해주셔서 감사합니다! 🐰
ヽ(・∀・)ノ
✏️ Learnings added
Learnt from: jj0526
Repo: Leets-Makers/Leenk-BE PR: 73
File: src/main/java/leets/leenk/global/sqs/application/mapper/SqsMessageEventMapper.java:97-105
Timestamp: 2025-11-10T05:47:55.301Z
Learning: 생일 푸시 알림(`toBirthdaySqsMessageEvent`)에는 다른 알림 매핑 메서드들과 달리 `id` 파라미터를 포함하지 않는다. 이는 의도된 설계이며, 생일 알림은 특정 상세 페이지로 이동할 필요가 없기 때문이다.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/leets/leenk/domain/notification/domain/entity/birthdayContent/BirthdayCelebrateContent.java (1)
8-14: BirthdayAnnouncementContent와 구조가 동일합니다.이 클래스는 BirthdayAnnouncementContent와 정확히 동일한 필드 구조를 가지고 있습니다 (birthdayUserId, birthdayUserName). 현재는 알림 타입별로 타입 안전성을 제공하는 장점이 있지만, 향후 유지보수를 고려하여 공통 추상 클래스나 공통 필드를 가진 베이스 클래스를 도입하는 것을 검토해볼 수 있습니다.
현재 구조도 충분히 작동하며, 향후 각 알림 타입이 서로 다른 필드를 필요로 할 경우를 대비한 확장 가능한 설계입니다.
src/main/java/leets/leenk/domain/notification/application/mapper/BirthdayNotificationMapper.java (1)
14-39: 매퍼 메서드의 chained 호출이 위험합니다. @nonnull 어노테이션으로 계약을 명시해주세요.
toBirthdayLetterNotification메서드의 34번 라인(birthdayLetter.getReceiver().getId())과 43번 라인(birthdayLetter.getSender().getName())에서 lazy-loaded 관계를 chained로 접근합니다. 데이터베이스 제약조건(nullable=false,optional=false)만으로는 JPA lazy loading 실패 시 NPE를 방지할 수 없습니다.또한
toBirthdayAnnouncementNotification,toBirthdayCelebrateNotification메서드의 User 파라미터와toBirthdayLetterNotification메서드의 BirthdayLetter 파라미터에 명시적인 null 검증이 없습니다.대안: Spring의
@org.springframework.lang.NonNull또는 Jakarta의@jakarta.annotation.Nonnull어노테이션을 메서드 파라미터에 추가하여 계약을 명시하고, IDE/정적 분석 도구의 지원을 받으세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/leets/leenk/domain/notification/application/mapper/BirthdayNotificationMapper.java(1 hunks)src/main/java/leets/leenk/domain/notification/domain/entity/birthdayContent/BirthdayAnnouncementContent.java(1 hunks)src/main/java/leets/leenk/domain/notification/domain/entity/birthdayContent/BirthdayCelebrateContent.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/leets/leenk/domain/notification/domain/entity/birthdayContent/BirthdayAnnouncementContent.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/leets/leenk/domain/notification/domain/entity/birthdayContent/BirthdayCelebrateContent.java (1)
src/main/java/leets/leenk/domain/notification/domain/entity/birthdayContent/BirthdayAnnouncementContent.java (1)
SuperBuilder(8-14)
src/main/java/leets/leenk/domain/notification/application/mapper/BirthdayNotificationMapper.java (1)
src/main/java/leets/leenk/global/sqs/application/mapper/SqsMessageEventMapper.java (1)
Component(14-106)
🔇 Additional comments (1)
src/main/java/leets/leenk/domain/notification/application/mapper/BirthdayNotificationMapper.java (1)
41-66: 검증 완료 - NotificationType enum에 올바른 플레이스홀더 포함 확인됨NotificationType enum의 BIRTHDAY 관련 항목들이 모두
{name}플레이스홀더를 포함하고 있습니다:
BIRTHDAY_ANNOUNCEMENT: "오늘은 {name}의 생일이야\n축하해주러 가볼까?"BIRTHDAY_CELEBRATE: "생일 축하해, 멋쟁이 {name}!"BIRTHDAY_LETTER: "{name}에게 생일 편지를 받았어!"BirthdayNotificationMapper의 content 빌더 메서드들이 올바르게 구현되어 있으며, SqsMessageEventMapper에서 이 템플릿의 {name} 플레이스홀더를 실제 사용자 이름으로 치환할 수 있도록 설계되어 있습니다.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/java/leets/leenk/domain/notification/application/usecase/BirthdayNotificationUsecase.java (1)
45-63: 중첩 루프로 인한 성능 고려사항생일자와 알림 수신자를 이중 반복하여 O(n×m) 복잡도가 발생합니다. 사용자 수가 많아지면 알림 생성/발송에 시간이 오래 걸릴 수 있습니다. 현재는 스케줄러에서 호출되므로 당장 문제가 되지 않지만, 향후 사용자 규모 증가 시 배치 처리 또는 비동기 작업 분할을 고려해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/leets/leenk/domain/notification/application/usecase/BirthdayNotificationUsecase.java(1 hunks)src/main/java/leets/leenk/domain/user/domain/repository/UserSettingRepository.java(1 hunks)src/main/java/leets/leenk/domain/user/domain/service/user/UserGetService.java(1 hunks)src/main/java/leets/leenk/global/sqs/application/mapper/AwsSqsManager.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/leets/leenk/domain/user/domain/service/user/UserGetService.java
- src/main/java/leets/leenk/domain/user/domain/repository/UserSettingRepository.java
- src/main/java/leets/leenk/global/sqs/application/mapper/AwsSqsManager.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/leets/leenk/domain/notification/application/usecase/BirthdayNotificationUsecase.java (2)
src/main/java/leets/leenk/domain/birthday/application/usecase/BirthdayLetterUseCase.java (1)
Service(24-77)src/main/java/leets/leenk/domain/notification/domain/service/scheduler/BirthdayNotifyScheduler.java (1)
Service(11-27)
...in/java/leets/leenk/domain/notification/application/usecase/BirthdayNotificationUsecase.java
Show resolved
Hide resolved
...in/java/leets/leenk/domain/notification/application/usecase/BirthdayNotificationUsecase.java
Show resolved
Hide resolved
soo0711
left a comment
soo0711
left a comment
There was a problem hiding this comment.
PR에 써진 작업 내용 모두 잘 구현되어있는 것 같습니다 !! 수고하셨습니다 👍
| UserSetting userSetting; | ||
| try{ | ||
| userSetting = userSettingGetService.findByUser(birthdayUser); | ||
| } catch (Exception e){ | ||
| continue; | ||
| } | ||
| if(userSetting == null || !userSetting.isBirthdayNotify()) continue; |
There was a problem hiding this comment.
여기 코드가 saveBirthdayLetterNotification(BirthdayLetter birthdayLetter)에서도 반복되는 부분인데 별도의 private 메서드로 추출하면 어떨까요?
There was a problem hiding this comment.
좋습니다 별도 메소드로 분리했습니다~!
| UserSetting userSetting; | ||
| try{ | ||
| userSetting = userSettingGetService.findByUser(birthdayUser); | ||
| } catch (Exception e){ | ||
| return; | ||
| } | ||
| if(userSetting == null || !userSetting.isBirthdayNotify()) return; |
| private MessageAttributeValue convertToAttributeValue(Long value) { | ||
| return MessageAttributeValue.builder() | ||
| .dataType("Number") | ||
| .stringValue(value.toString()) |
There was a problem hiding this comment.
toString()에서 삼항연산자를 사용하게 된 이유가 무엇인가요?
There was a problem hiding this comment.
value가 null인 경우 NPE가 발생하여 추가했습니다!
| private static final ZoneId KST = ZoneId.of("Asia/Seoul"); | ||
|
|
||
| private final BirthdayNotificationUsecase birthdayNotificationUsecase; | ||
|
|
||
| @Scheduled(cron = "0 0 0 * * *", zone = "Asia/Seoul") |
There was a problem hiding this comment.
지역 설정에 대한 부분이 존재하는데 이 부분은 application.yml 파일에서 한번에 관리해도 좋을 것 같다는 생각이 듭니다.
제 개인적 의견이니 한번 고려만해주시고 시현님 뜻대로 하는 것이 좋아보입니다!!
There was a problem hiding this comment.
지역(시간대) 관련 설정을 application.yml에서 통합 관리하는 방향이 하드코딩보다 훨씬 명확해 보여 수정하여 반영했습니다~!
hyxklee
left a comment
hyxklee
left a comment
There was a problem hiding this comment.
큰 문제는 없어 보이네용
다만,, 전체적인 도메인 구조의 리팩토링은 필요해보입니다
한 번 설계부터 다시 해봅시다!
There was a problem hiding this comment.
해당 파일은 사용하지 않는 파일이고 MediaUpdateFailedException.java에서 해당 기능을 쓰는것으로 보여서 삭제했습니다
| Boolean newReactionNotify, | ||
|
|
||
| @Schema(description = "생일 알림 여부 수정", example = "false") | ||
| Boolean birthdayNotify |
There was a problem hiding this comment.
생일 알림 여부 별도 설정하기로 했었나요?? 왜 피그마에 화면이 없는 거 같지
@nabbang6 프론트 담당자분께서도 해당 사항 인지하고 계실까욤?
There was a problem hiding this comment.
생일 알림 여부도 회의시간에 추가하기로 했었습니다!
| import org.springframework.data.domain.Slice; | ||
| import org.springframework.stereotype.Service; | ||
|
|
||
| import java.time.LocalDate; |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/main/java/leets/leenk/domain/birthday/domain/service/scheduler/BirthdayNotifyScheduler.java (1)
16-22: 타임존 명시 및 에러 핸들링 추가를 권장합니다.현재 구현에서 다음 개선이 필요합니다:
타임존 명시:
LocalDate.now()는 시스템 기본 타임존을 사용하므로 환경에 따라 다른 결과가 나올 수 있습니다.LocalDate.now(ZoneId.of("Asia/Seoul"))처럼 명시적으로 타임존을 지정하는 것이 안전합니다.에러 핸들링: 두 메서드가 순차 실행되므로
announceUserBirthday에서 예외 발생 시celebrateBirthday가 실행되지 않습니다. 각 메서드를 try-catch로 감싸서 하나가 실패해도 다른 하나는 실행되도록 보장해야 합니다.로깅 부재: 스케줄러 실행 시작/종료, 처리된 사용자 수 등의 로깅이 없어 모니터링이 어렵습니다.
다음과 같이 개선할 수 있습니다:
+import java.time.ZoneId; +import lombok.extern.slf4j.Slf4j; + +@Slf4j @Service @RequiredArgsConstructor public class BirthdayNotifyScheduler { private final BirthdayNotificationUsecase birthdayNotificationUsecase; @Scheduled(cron = "0 0 0 * * *") public void sendBirthdayNotifications() { - LocalDate today = LocalDate.now(); - birthdayNotificationUsecase.announceUserBirthday(today); - - birthdayNotificationUsecase.celebrateBirthday(today); - + LocalDate today = LocalDate.now(ZoneId.of("Asia/Seoul")); + log.info("생일 알림 스케줄러 시작 - 날짜: {}", today); + + try { + birthdayNotificationUsecase.announceUserBirthday(today); + } catch (Exception e) { + log.error("생일 공지 알림 전송 실패", e); + } + + try { + birthdayNotificationUsecase.celebrateBirthday(today); + } catch (Exception e) { + log.error("생일 축하 알림 전송 실패", e); + } + + log.info("생일 알림 스케줄러 완료"); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/leets/leenk/domain/birthday/domain/service/scheduler/BirthdayNotifyScheduler.java(1 hunks)src/main/java/leets/leenk/domain/notification/application/usecase/BirthdayNotificationUsecase.java(1 hunks)src/main/resources/application.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: jj0526
Repo: Leets-Makers/Leenk-BE PR: 73
File: src/main/java/leets/leenk/global/sqs/application/mapper/SqsMessageEventMapper.java:97-105
Timestamp: 2025-11-10T05:47:55.325Z
Learning: 생일 푸시 알림(`toBirthdaySqsMessageEvent`)에는 다른 알림 매핑 메서드들과 달리 `id` 파라미터를 포함하지 않는다. 이는 의도된 설계이며, 생일 알림은 특정 상세 페이지로 이동할 필요가 없기 때문이다.
📚 Learning: 2025-11-10T05:47:55.325Z
Learnt from: jj0526
Repo: Leets-Makers/Leenk-BE PR: 73
File: src/main/java/leets/leenk/global/sqs/application/mapper/SqsMessageEventMapper.java:97-105
Timestamp: 2025-11-10T05:47:55.325Z
Learning: 생일 푸시 알림(`toBirthdaySqsMessageEvent`)에는 다른 알림 매핑 메서드들과 달리 `id` 파라미터를 포함하지 않는다. 이는 의도된 설계이며, 생일 알림은 특정 상세 페이지로 이동할 필요가 없기 때문이다.
Applied to files:
src/main/java/leets/leenk/domain/notification/application/usecase/BirthdayNotificationUsecase.java
📚 Learning: 2025-08-15T06:25:40.452Z
Learnt from: jj0526
Repo: Leets-Makers/Leenk-BE PR: 55
File: src/main/java/leets/leenk/domain/leenk/domain/service/LeenkStatusBatchService.java:32-33
Timestamp: 2025-08-15T06:25:40.452Z
Learning: User jj0526 prefers to avoid individual existence checks in batch notification processing due to performance concerns. They favor scheduler-based approaches (30-minute intervals) that inherently minimize duplicate risks over runtime duplicate prevention checks.
Applied to files:
src/main/java/leets/leenk/domain/notification/application/usecase/BirthdayNotificationUsecase.java
🧬 Code graph analysis (1)
src/main/java/leets/leenk/domain/notification/application/usecase/BirthdayNotificationUsecase.java (1)
src/main/java/leets/leenk/domain/birthday/application/usecase/BirthdayLetterUseCase.java (1)
Service(24-77)
🔇 Additional comments (2)
src/main/resources/application.yml (1)
6-7: 생일 알림 스케줄링을 위한 적절한 타임존 설정입니다.Jackson의 타임존 설정을 Asia/Seoul로 추가하여 모든 Date/LocalDateTime 직렬화 시 일관된 타임존을 사용하게 됩니다. 이는 자정(00:00)에 실행되는 생일 알림 스케줄러와 알림 콘텐츠의 시간 표현에 중요합니다.
스케줄러 구현 코드에서 이 타임존 설정을 활용하여 정확하게 Asia/Seoul 기준 자정을 계산하는지 확인해 주세요. 예를 들어,
BirthdayNotifyScheduler가ZoneId.of("Asia/Seoul")을 명시적으로 사용하거나, Spring 기본 설정에 의존하는지 검증하면 좋습니다.src/main/java/leets/leenk/domain/notification/application/usecase/BirthdayNotificationUsecase.java (1)
91-110: 구조가 일관되고 명확합니다.헬퍼 메서드를 사용하여 사용자 설정을 확인한 후 알림을 저장하는 흐름이 명확하며, 이전 리뷰에서 지적된 문제가 잘 해결되었습니다.
다만
isBirthdayNotificationEnabled헬퍼 메서드의 예외 처리 문제(별도 코멘트 참조)가 해결되면 더욱 안정적일 것입니다.
...in/java/leets/leenk/domain/notification/application/usecase/BirthdayNotificationUsecase.java
Show resolved
Hide resolved
...in/java/leets/leenk/domain/notification/application/usecase/BirthdayNotificationUsecase.java
Show resolved
Hide resolved
| private boolean isBirthdayNotificationEnabled(User birthdayUser) { | ||
| try { | ||
| UserSetting userSetting = userSettingGetService.findByUser(birthdayUser); | ||
| return userSetting != null && userSetting.isBirthdayNotify(); | ||
| } catch (Exception e) { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
너무 광범위한 예외 처리로 실제 오류가 숨겨질 수 있습니다.
현재 구현의 문제점:
-
Line 116:
Exception을 잡는 것은 너무 광범위하여NullPointerException, 데이터베이스 연결 오류 등 실제 심각한 문제를 감출 수 있습니다. -
무조건 false 반환: 모든 예외를 "알림 비활성화"와 동일하게 처리하므로, 실제 시스템 오류와 사용자 설정을 구분할 수 없습니다.
-
로깅 부재: 예외가 발생해도 기록되지 않아 문제 추적이 어렵습니다.
다음과 같이 개선하세요:
private boolean isBirthdayNotificationEnabled(User birthdayUser) {
try {
UserSetting userSetting = userSettingGetService.findByUser(birthdayUser);
return userSetting != null && userSetting.isBirthdayNotify();
- } catch (Exception e) {
+ } catch (RuntimeException e) {
+ log.error("사용자 설정 조회 실패 - userId: {}", birthdayUser.getId(), e);
return false;
}
}또는 userSettingGetService.findByUser()가 던지는 구체적인 예외 타입이 있다면 그것을 잡는 것이 더 좋습니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/leets/leenk/domain/notification/application/usecase/BirthdayNotificationUsecase.java
around lines 112-119, replace the broad try-catch that catches Exception and
unconditionally returns false: instead catch only the specific expected
exceptions from userSettingGetService.findByUser (e.g., a UserNotFoundException
or the service's declared checked exceptions) and handle those by returning
false when appropriate; for unexpected exceptions (e.g., DataAccessException,
NullPointerException) do not swallow them — log the error with the class logger
including the exception stack trace and either rethrow or wrap in a runtime
exception so system errors are visible; ensure normal flow still returns
userSetting != null && userSetting.isBirthdayNotify().
Related issue 🛠
작업 내용 💻
[Bob]에게 생일 편지를 받았어!와 같이 실제 유저 이름으로 치환됩니다추가)
스크린샷 📷
{ "code": 1300, "message": "알림 조회에 성공했습니다.", "data": { "notificationResponses": [ { "id": "690f53a4aacbd9cac330c145", "userId": 1, "notificationType": "BIRTHDAY_LETTER", "path": "birthday", "isRead": false, "content": { "title": "Leenk", "body": "{name}에게 생일 편지를 받았어!", "senderName": "Bob", "birthdayLetterId": 1 }, "updateDate": "2025-11-08T23:28:52.26" }, { "id": "69076a343d0fc74ca83ad1ea", "userId": 1, "notificationType": "BIRTHDAY_CELEBRATE", "path": "birthday", "isRead": false, "content": { "title": "Leenk", "body": "생일 축하해, 멋쟁이 {name}!", "birthdayUserId": 1, "birthdayUserName": "Alice" }, "updateDate": "2025-11-02T23:27:00.521" }, { "id": "69076a343d0fc74ca83ad1de", "userId": 1, "notificationType": "BIRTHDAY_ANNOUNCEMENT", "path": "birthday", "isRead": false, "content": { "title": "Leenk", "body": "오늘은 {name}의 생일이야\n축하해주러 가볼까?", "birthdayUserId": 2, "birthdayUserName": "지아" }, "updateDate": "2025-11-02T23:27:00.176" },같이 얘기해보고 싶은 내용이 있다면 작성 📢
Summary by CodeRabbit
새로운 기능
설정
개선사항