Repository navigation
[Feat/#101] 누락된 푸시 알림 구현 - #104
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThese changes implement missing push notification features including scheduled reminders for incomplete todos and habits, weekly growth confirmations, and notification flows triggered when habit periods expire or parents view reward information. Changes
Sequence Diagram(s)sequenceDiagram
participant Scheduler as HabitScheduler
participant Repo as HabitRepository
participant NotifSvc as NotificationService
participant FamilyRel as FamilyRelationService
Scheduler->>Repo: findExpiredHabits(now)
Repo-->>Scheduler: List<Habit> with users
loop For each expired habit
alt Child user detected
Scheduler->>NotifSvc: sendToUser(childId, childMsg)
NotifSvc-->>Scheduler: ✓
Scheduler->>FamilyRel: getConnectedMembers(childId)
FamilyRel-->>Scheduler: List of members
alt Parent members exist
Scheduler->>NotifSvc: sendToUsers(parentIds, parentMsg)
NotifSvc-->>Scheduler: ✓
end
end
end
Scheduler->>Repo: updateExpiredHabitsStatus(now)
Repo-->>Scheduler: Update complete
sequenceDiagram
participant Service as HabitService
participant Repo as HabitRepository
participant TxnSync as TransactionSynchronization
participant NotifSvc as NotificationService
Service->>Repo: Query habits by reward status
Repo-->>Service: Habit list
Service->>Service: Compute connected children IDs
alt Children exist
Service->>TxnSync: Register afterCommit callback
TxnSync-->>Service: Registered
Note over Service: Method returns
Note over TxnSync: After transaction commits
TxnSync->>NotifSvc: sendToUsers(childIds, "newReward")
NotifSvc-->>TxnSync: ✓
end
sequenceDiagram
participant Scheduler as PushNotificationScheduler
participant UserSvc as UserService
participant HabitRepo as HabitRepository
participant TodoRepo as TodoRepository
participant NotifSvc as NotificationService
Scheduler->>UserSvc: findAllActiveUsers()
UserSvc-->>Scheduler: List<User>
Scheduler->>Scheduler: Extract user IDs
par Todo Reminder at 13:00
Scheduler->>TodoRepo: findIncompleteByDate(users, today)
TodoRepo-->>Scheduler: Incomplete todos
alt Non-empty result
Scheduler->>NotifSvc: sendToUsers(userIds, todoMsg)
NotifSvc-->>Scheduler: ✓
end
and Habit Reminder at 12:00
Scheduler->>HabitRepo: findIncompleteHabits(users)
HabitRepo-->>Scheduler: Incomplete habits
alt Non-empty result
Scheduler->>NotifSvc: sendToUsers(userIds, habitMsg)
NotifSvc-->>Scheduler: ✓
end
and Weekly Growth at Sunday 20:00
Scheduler->>Scheduler: Partition users (CHILD/PARENT)
alt Children non-empty
Scheduler->>NotifSvc: sendToUsers(childIds, childGrowthMsg)
end
alt Parents non-empty
Scheduler->>NotifSvc: sendToUsers(parentIds, parentGrowthMsg)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 1
🧹 Nitpick comments (1)
src/main/java/com/swyp/server/domain/habit/service/HabitScheduler.java (1)
36-59: Consider batching family relation lookups to avoid N+1 queries.The current implementation calls
familyRelationService.getConnectedMembers()for each expired CHILD habit (line 47-48). If many habits expire simultaneously, this results in N additional database queries.Consider collecting all child user IDs first, then fetching family relations in a single batch query, to improve performance under load.
♻️ Sketch of batched approach
- expiredHabits.stream() - .filter(h -> h.getUser().getUserType() == UserType.CHILD) - .forEach( - h -> { - notificationService.sendToUser( - h.getUser().getId(), - "해봄", - "보상을 받을 수 있어요. 지금 바로 확인해 볼까요?", - Map.of()); - - List<Long> parentIds = - familyRelationService - .getConnectedMembers(h.getUser().getId()) - .stream() - .filter(m -> m.getUserType() == UserType.PARENT) - .map(User::getId) - .toList(); - if (!parentIds.isEmpty()) { - notificationService.sendToUsers( - parentIds, "해봄", "자녀가 습관을 완료했어요. 보상을 줄 시간이에요!", Map.of()); - } - }); + List<Long> childUserIds = expiredHabits.stream() + .filter(h -> h.getUser().getUserType() == UserType.CHILD) + .map(h -> h.getUser().getId()) + .distinct() + .toList(); + + // Send to children + notificationService.sendToUsers( + childUserIds, "해봄", "보상을 받을 수 있어요. 지금 바로 확인해 볼까요?", Map.of()); + + // Batch fetch parent relations and notify (requires new batch method in FamilyRelationService) + // Or iterate if batch not available: + Set<Long> parentIds = new HashSet<>(); + for (Long childId : childUserIds) { + familyRelationService.getConnectedMembers(childId).stream() + .filter(m -> m.getUserType() == UserType.PARENT) + .map(User::getId) + .forEach(parentIds::add); + } + if (!parentIds.isEmpty()) { + notificationService.sendToUsers( + new ArrayList<>(parentIds), "해봄", "자녀가 습관을 완료했어요. 보상을 줄 시간이에요!", Map.of()); + }Note: The sketch above also deduplicates child notifications (one per child vs one per habit) and parent notifications. Verify whether sending one notification per child or per habit is the intended behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/swyp/server/domain/habit/service/HabitScheduler.java` around lines 36 - 59, The expiredHabits stream in HabitScheduler currently calls familyRelationService.getConnectedMembers() per habit causing N+1 queries; instead collect all child user IDs from expiredHabits (filtering by UserType.CHILD), call familyRelationService.getConnectedMembers(...) once with the set/list of child IDs to fetch relations in batch, map those results to parent ID lists keyed by child ID, then iterate the expired children to send notifications using notificationService.sendToUser/sendToUsers while deduplicating parentIds per child (and optionally deduplicating across children if you intend one parent notification), and finally call habitRepository.updateExpiredHabitsStatus(now) as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/swyp/server/domain/habit/service/HabitService.java`:
- Around line 110-126: The notification block in HabitService.getHabitRewards is
incorrectly sending "new reward" notifications on every read and also
re-computes childIds; remove the
TransactionSynchronization.registerSynchronization(...) block (the afterCommit()
notification call that uses notificationService.sendToUsers) from the
getHabitRewards/read flow so viewing rewards no longer triggers notifications,
and eliminate the redundant childIds computation in that method; if
notifications are needed when a reward is actually created, add the
TransactionSynchronization/notificationService.sendToUsers logic to the write
path (e.g., createHabit or the reward-creation method) instead, referencing the
same notificationService and childIds resolution there.
---
Nitpick comments:
In `@src/main/java/com/swyp/server/domain/habit/service/HabitScheduler.java`:
- Around line 36-59: The expiredHabits stream in HabitScheduler currently calls
familyRelationService.getConnectedMembers() per habit causing N+1 queries;
instead collect all child user IDs from expiredHabits (filtering by
UserType.CHILD), call familyRelationService.getConnectedMembers(...) once with
the set/list of child IDs to fetch relations in batch, map those results to
parent ID lists keyed by child ID, then iterate the expired children to send
notifications using notificationService.sendToUser/sendToUsers while
deduplicating parentIds per child (and optionally deduplicating across children
if you intend one parent notification), and finally call
habitRepository.updateExpiredHabitsStatus(now) as before.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 26551dd4-b966-4e2a-b623-8c89912efb25
📒 Files selected for processing (4)
src/main/java/com/swyp/server/domain/habit/repository/HabitRepository.javasrc/main/java/com/swyp/server/domain/habit/service/HabitScheduler.javasrc/main/java/com/swyp/server/domain/habit/service/HabitService.javasrc/main/java/com/swyp/server/global/notification/PushNotificationScheduler.java
guzzang
approved these changes
Apr 5, 2026
| } | ||
| }); | ||
|
|
||
| habitRepository.updateExpiredHabitsStatus(now); |
Contributor
There was a problem hiding this comment.
P3:
로직 자체는 문제 없는 거 같은데 00시에 알림 보내도 괜찮을까요...??
Collaborator
Author
There was a problem hiding this comment.
저도 00시 알림이 부자연스러워 보여서 걱정이 되었는데, 일단 기획서 기준으로는 만료 즉시 알림이 맞아서 스케줄러 실행 시간인 00시에 보내도록 구현했습니다..! PM분께 추가 확인 해보고 추후 수정해볼게요!
5 tasks done
5 tasks done
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.
📌 관련 이슈
✨ 변경 사항
📚 리뷰어 참고 사항
✅ 체크리스트
Summary by CodeRabbit