feat: 알림 단계 설정과 자동 알림 발송 구현#77
Merged
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 34 minutes and 47 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough사용자 알림 선호도를 enum으로 확장하고 알림 엔터티/응답에 level·child 컨텍스트를 추가했습니다. 저장소·서비스·컨트롤러와 회원가입/뉴스레터 통합을 업데이트하고 NotificationScheduler를 추가해 마감·체크리스트·주간 알림을 자동 생성합니다. 변경사항알림 설정 단계 및 자동 알림 생성
Sequence DiagramsequenceDiagram
participant Scheduler as NotificationScheduler
participant CalendarRepo as CalendarEventRepository
participant ChecklistRepo as ChecklistRepository
participant NewsletterRepo as NewsletterRepository
participant ChildRepo as ChildRepository
participant NotificationSvc as NotificationService
Scheduler->>CalendarRepo: findByStartAtGreaterThanEqualAndStartAtLessThan(range)
Scheduler->>ChecklistRepo: findByTypeAndCompletedFalseAndTargetDate(...)
CalendarRepo-->>Scheduler: events
ChecklistRepo-->>Scheduler: checklists
Scheduler->>NewsletterRepo: findById(...) (for newsletterId)
Scheduler->>ChildRepo: findFirstByUserIdAndNameAndDeletedAtIsNull(userId,name)
Scheduler->>NotificationSvc: createNotification(NotificationCreateCommand)
NotificationSvc-->>NotificationSvc: resolveChildName / build Notification entity
NotificationSvc-->>Scheduler: confirmation / exception logged
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/gachi/be/domain/notification/service/NotificationService.java (1)
160-164:⚠️ Potential issue | 🟠 Major | ⚡ Quick win알림 level null 허용으로 저장/발송 경로가 깨질 수 있습니다.
createNotification에서command.level()을 검증하지 않고 그대로 빌더에 주입하고 있습니다. 호출부에서 level을 누락하면 DB 제약 위반 또는 이후 단계 필터링 시 런타임 오류로 이어질 수 있으니, null 방어(기본값 지정 또는 예외)가 필요합니다.🔧 제안 수정
if (command == null || command.type() == null + || command.level() == null || !StringUtils.hasText(command.title()) || !StringUtils.hasText(command.body())) { throw new BusinessException(ErrorCode.INVALID_INPUT_VALUE); } @@ .type(command.type()) - .level(command.level()) + .level(command.level())Also applies to: 178-178
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/gachi/be/domain/notification/service/NotificationService.java` around lines 160 - 164, The createNotification path in NotificationService currently skips validating command.level() and passes it straight into Notification.builder(), which can cause DB constraint or runtime filter errors if callers omit level; update the validation in the createNotification method to check command.level() (or provide a sensible default like NotificationLevel.DEFAULT/INFO) and throw BusinessException(ErrorCode.INVALID_INPUT_VALUE) when null (or document/set the default), and apply the same null-defense to the other affected site noted in the review (the similar check around the update path at the other reported location) so both places either validate or default command.level() before building/persisting the Notification.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/com/gachi/be/domain/auth/service/impl/AuthServiceImpl.java`:
- Around line 141-143: The signup flow in AuthServiceImpl currently forwards
request.notificationPreference() directly to the entity builder (via
.notificationPreference(request.notificationPreference())), relying on the
entity to default missing values; change the signup path to enforce the spec by
mapping a null/absent request.notificationPreference() to the default IMPORTANT
before calling .notificationPreference(...). Locate the builder call around
.languageCode(...)/.notificationPreference(...)/.emailVerifiedAt(now) and
replace the direct use of request.notificationPreference() with a computed value
(e.g., notificationPreference != null ? notificationPreference : IMPORTANT) so
the signup endpoint explicitly guarantees the default.
In
`@src/main/java/com/gachi/be/domain/checklist/repository/ChecklistRepository.java`:
- Line 29: The current repository method countByUserIdAndCompletedFalse
incorrectly includes TODO-type items in its tally; change the query surface to
only count checklist items with ChecklistType.CHECKLIST (e.g., add or replace
with a method such as countByUserIdAndCompletedFalseAndType or
countByUserIdAndCompletedFalseAndTypeEquals checking ChecklistType.CHECKLIST) so
the "incomplete but completable" count excludes ChecklistType.TODO; update
usages of countByUserIdAndCompletedFalse to call the new method.
In
`@src/main/java/com/gachi/be/domain/newsletter/pipeline/NewsletterPipelineService.java`:
- Around line 189-192: The createAnalysisCompletedNotification call must be
isolated from the markCompleted (REQUIRES_NEW) transaction so that failures in
notification do not roll back the completed-state save; modify the block where
n.complete(...), Newsletter saved = newsletterRepository.save(n) and
createAnalysisCompletedNotification(saved) are called to first persist the
completed state (newsletterRepository.save in the method that invokes n.complete
/ markCompleted) and then invoke createAnalysisCompletedNotification(saved) in a
separate try/catch or separate transactional/non-transactional method so any
exception is caught and logged without propagating; apply the same change to the
similar sequence around the code referenced in the 221-239 region so
notification failures are logged/isolated rather than causing the REQUIRES_NEW
transaction to rollback.
In
`@src/main/java/com/gachi/be/domain/notification/repository/NotificationRepository.java`:
- Line 22: 현재 JPQL 조건 "AND (:childId IS NULL OR n.childId = :childId OR
n.childName = :childName)"는 childId가 주어져도 같은 이름의 다른 자녀 알림을 포함하므로, childName 기반
조회는 childId가 비어 있는 레거시 데이터에만 허용되도록 수정하세요: 교체할 조건은 "AND ( ( :childId IS NULL AND
(n.childId IS NULL OR n.childName = :childName) ) OR n.childId = :childId )"처럼
작성해 NotificationRepository의 해당 JPQL/SQL 쿼리(현재 n.childId, n.childName을 사용하는 WHERE
절)를 업데이트하고 동일한 변경을 파일의 다른 두 위치(언급된 28-29의 유사 조건)에도 적용하세요.
In
`@src/main/java/com/gachi/be/domain/notification/service/NotificationScheduler.java`:
- Around line 65-84: The loop in NotificationScheduler that processes each
CalendarEvent (using resolveChildId, payload(), putIfPresent and
notificationService.createNotification) must not let one event's exception stop
the whole batch; wrap the per-event processing (everything inside the for
(CalendarEvent event : events) { ... } block) in a try-catch, catch Exception,
log the error with contextual data (event.getId(), event.getUserId(), targetDate
or childName) and continue to the next event. Apply the same per-item try-catch
pattern to the other similar blocks (the loops that call
notificationService.createNotification at lines referenced) so each failure is
isolated and does not abort the scheduler run.
---
Outside diff comments:
In
`@src/main/java/com/gachi/be/domain/notification/service/NotificationService.java`:
- Around line 160-164: The createNotification path in NotificationService
currently skips validating command.level() and passes it straight into
Notification.builder(), which can cause DB constraint or runtime filter errors
if callers omit level; update the validation in the createNotification method to
check command.level() (or provide a sensible default like
NotificationLevel.DEFAULT/INFO) and throw
BusinessException(ErrorCode.INVALID_INPUT_VALUE) when null (or document/set the
default), and apply the same null-defense to the other affected site noted in
the review (the similar check around the update path at the other reported
location) so both places either validate or default command.level() before
building/persisting the Notification.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9604ca94-b8d7-4042-ab8b-123ccb8b4534
📒 Files selected for processing (26)
src/main/java/com/gachi/be/GachiBeApplication.javasrc/main/java/com/gachi/be/domain/auth/dto/request/SignupRequest.javasrc/main/java/com/gachi/be/domain/auth/service/impl/AuthServiceImpl.javasrc/main/java/com/gachi/be/domain/calendar/repository/CalendarEventRepository.javasrc/main/java/com/gachi/be/domain/checklist/repository/ChecklistRepository.javasrc/main/java/com/gachi/be/domain/child/repository/ChildRepository.javasrc/main/java/com/gachi/be/domain/newsletter/pipeline/NewsletterPipelineService.javasrc/main/java/com/gachi/be/domain/newsletter/repository/NewsletterRepository.javasrc/main/java/com/gachi/be/domain/notification/api/controller/NotificationController.javasrc/main/java/com/gachi/be/domain/notification/dto/response/NotificationResponse.javasrc/main/java/com/gachi/be/domain/notification/entity/Notification.javasrc/main/java/com/gachi/be/domain/notification/entity/enums/NotificationLevel.javasrc/main/java/com/gachi/be/domain/notification/entity/enums/NotificationType.javasrc/main/java/com/gachi/be/domain/notification/repository/NotificationRepository.javasrc/main/java/com/gachi/be/domain/notification/service/NotificationCreateCommand.javasrc/main/java/com/gachi/be/domain/notification/service/NotificationPushDispatcher.javasrc/main/java/com/gachi/be/domain/notification/service/NotificationScheduler.javasrc/main/java/com/gachi/be/domain/notification/service/NotificationService.javasrc/main/java/com/gachi/be/domain/user/api/controller/UserController.javasrc/main/java/com/gachi/be/domain/user/dto/request/ChangeNotificationRequest.javasrc/main/java/com/gachi/be/domain/user/dto/response/UserMeResponse.javasrc/main/java/com/gachi/be/domain/user/entity/User.javasrc/main/java/com/gachi/be/domain/user/entity/enums/NotificationPreference.javasrc/main/java/com/gachi/be/domain/user/repository/UserRepository.javasrc/main/java/com/gachi/be/global/code/SuccessCode.javasrc/main/resources/db/migration/V16__notification_preference_and_context.sql
Contributor
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/main/java/com/gachi/be/domain/newsletter/pipeline/NewsletterPipelineService.java (1)
191-199:⚠️ Potential issue | 🟠 Major | ⚡ Quick win알림 생성 실패 격리가 아직 트랜잭션 레벨에서 불완전합니다.
Line 191-193의
try/catch는 예외 전파만 막고, JPA/트랜잭션 상태가 rollback-only로 표시되는 케이스는 막지 못할 수 있습니다.markCompleted커밋 이후에 알림을 실행하도록 분리하는 편이 안전합니다.🔧 제안 수정
+import org.springframework.transaction.support.TransactionSynchronization; +import org.springframework.transaction.support.TransactionSynchronizationManager; ... Newsletter saved = newsletterRepository.save(n); - try { - createAnalysisCompletedNotification(saved); - } catch (Exception ex) { - log.warn( - "[Pipeline] 분석 완료 알림 생성 실패. newsletterId={}, error={}", - saved.getId(), - ex.getMessage(), - ex); - } + TransactionSynchronizationManager.registerSynchronization( + new TransactionSynchronization() { + `@Override` + public void afterCommit() { + try { + createAnalysisCompletedNotification(saved); + } catch (Exception ex) { + log.warn( + "[Pipeline] 분석 완료 알림 생성 실패. newsletterId={}, error={}", + saved.getId(), + ex.getMessage(), + ex); + } + } + });#!/bin/bash set -euo pipefail echo "[1] NotificationService 트랜잭션 경계 확인" fd -i 'NotificationService*.java' src/main/java rg -n --type=java -C3 '`@Transactional`|createNotification\s*\(' src/main/java/com/gachi/be/domain/notification echo "[2] 호출부(NewsletterPipelineService) 경계 확인" rg -n --type=java -C3 'markCompleted\(|createAnalysisCompletedNotification\(' src/main/java/com/gachi/be/domain/newsletter/pipeline/NewsletterPipelineService.java예상 결과:
createNotification(...)이 기본/REQUIRED 트랜잭션이면, 내부 예외가 rollback-only를 유발할 수 있는지 점검 필요.- 위 조건이면
afterCommit(또는 별도 빈의 REQUIRES_NEW)로 완전 분리하는 것이 안전합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/gachi/be/domain/newsletter/pipeline/NewsletterPipelineService.java` around lines 191 - 199, The current try/catch around createAnalysisCompletedNotification only prevents exception propagation but doesn’t guarantee the notification runs after the markCompleted transaction commits; move the notification out of the transactional commit path so it executes after commit: after calling markCompleted(saved) (or wherever the entity is persisted), register a post-commit action (e.g., TransactionSynchronizationManager.registerSynchronization with afterCommit) or invoke a separate NotificationService method annotated with `@Transactional`(propagation = REQUIRES_NEW) / or publish an event and handle it in a post-commit listener; ensure you call createAnalysisCompletedNotification (or the NotificationService.createNotification) from that post-commit context and remove the in-transaction try/catch in NewsletterPipelineService so failures in notification won’t mark the main transaction rollback-only.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In
`@src/main/java/com/gachi/be/domain/newsletter/pipeline/NewsletterPipelineService.java`:
- Around line 191-199: The current try/catch around
createAnalysisCompletedNotification only prevents exception propagation but
doesn’t guarantee the notification runs after the markCompleted transaction
commits; move the notification out of the transactional commit path so it
executes after commit: after calling markCompleted(saved) (or wherever the
entity is persisted), register a post-commit action (e.g.,
TransactionSynchronizationManager.registerSynchronization with afterCommit) or
invoke a separate NotificationService method annotated with
`@Transactional`(propagation = REQUIRES_NEW) / or publish an event and handle it
in a post-commit listener; ensure you call createAnalysisCompletedNotification
(or the NotificationService.createNotification) from that post-commit context
and remove the in-transaction try/catch in NewsletterPipelineService so failures
in notification won’t mark the main transaction rollback-only.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c608c8e3-ea21-4e77-b96e-8f64a5643320
📒 Files selected for processing (5)
src/main/java/com/gachi/be/domain/auth/service/impl/AuthServiceImpl.javasrc/main/java/com/gachi/be/domain/checklist/repository/ChecklistRepository.javasrc/main/java/com/gachi/be/domain/newsletter/pipeline/NewsletterPipelineService.javasrc/main/java/com/gachi/be/domain/notification/repository/NotificationRepository.javasrc/main/java/com/gachi/be/domain/notification/service/NotificationScheduler.java
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/gachi/be/domain/newsletter/pipeline/NewsletterPipelineService.java`:
- Around line 193-194: markCompleted() (also markProcessing and
markFailedWithSnapshot) is being called directly from runPipeline() in the same
class so the `@Transactional`(propagation = Propagation.REQUIRES_NEW) boundary is
bypassed and the TransactionSynchronization afterCommit registered in
scheduleAnalysisCompletedNotification (lines ~223-235) fires with the outer
runPipeline transaction instead of a new commit; to fix, extract
markCompleted/markProcessing/markFailedWithSnapshot (and their calls to
scheduleAnalysisCompletedNotification) into a separate `@Service` bean (e.g.,
NewsletterPipelineStatusService), inject that bean into
NewsletterPipelineService and call the methods via the injected proxy so
REQUIRES_NEW takes effect and the afterCommit callbacks trigger on the new
transaction commit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2bcb4818-0d61-4e79-a005-b3170467b25c
📒 Files selected for processing (1)
src/main/java/com/gachi/be/domain/newsletter/pipeline/NewsletterPipelineService.java
Hminkyung
approved these changes
May 27, 2026
Collaborator
Hminkyung
left a comment
Hminkyung
left a comment
There was a problem hiding this comment.
확인했습니다아ㅏㅏㅏ!! 고생하셨습니당 :)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 작업 요약
URGENT_ONLY,IMPORTANT,ALL,OFF로 확장level,childId,childName을 추가하고,childId기준 필터 조회를 지원🌿 브랜치 정보
feat/#76-notification-settings-autodevelop(기본)✅ 체크리스트
feat/refac/hotfix/chore/design/bugfix)feat/fix/refactor/docs/style/chore)🧪 테스트 결과
Summary by CodeRabbit