[FEAT] 일정 및 준비/실행 항목 알람 구현 - #228
Merged
Merged
Conversation
|
Warning Review limit reached
Next review available in: 29 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe PR adds Firebase Cloud Messaging, alarm consent and state management, alarm REST endpoints, cursor-based alarm queries, reminder scheduling, Redis delayed delivery, recurring reminder rescheduling, and completion-based reminder cancellation. ChangesAlarm and Reminder Delivery
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 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: 19
🧹 Nitpick comments (13)
src/main/java/com/tryna/domain/reminder/service/AlarmReminderScheduleService.java (1)
81-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog the original cause before you map it to a 500 error code.
Both
tryblocks catch everyExceptionand replace it withAlarmErrorCode.F101_EVENT_ALARM_500orAlarmErrorCode.F102_ACTIONITEM_ALARM_500. The stack trace is discarded. A constraint violation, a Redis timeout, and a mapping error all become the same opaque response, which makes production diagnosis hard.Also note that the two blocks are structurally identical. Extract a shared helper if you touch this area again.
♻️ Proposed change
} catch (BusinessException e) { throw e; } catch (Exception e) { + log.error("일정 리마인더 생성에 실패했습니다. userId={}, eventId={}", userId, eventId, e); throw new BusinessException(AlarmErrorCode.F101_EVENT_ALARM_500); }Add
@Slf4jto the class.Also applies to: 122-139
🤖 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/tryna/domain/reminder/service/AlarmReminderScheduleService.java` around lines 81 - 98, Add Lombok `@Slf4j` to AlarmReminderScheduleService and log the caught original exception, including its stack trace, immediately before converting it to F101_EVENT_ALARM_500 or F102_ACTIONITEM_ALARM_500 in both try blocks. Preserve BusinessException propagation and the existing mapped error responses; optionally use a shared helper for the identical logging-and-mapping logic.src/main/java/com/tryna/domain/reminder/repository/AlarmDelayedQueueRepository.java (1)
31-37: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a parameterized script type instead of the raw
List.
RedisScript<List>forces the unchecked cast at Line 53. Declare the script asRedisScript<List<String>>with an explicit type reference, or keep the raw form and document why. The current form compiles with a raw-type warning.Also note the
unpack(dueItems)call. Lua limits the argument count ofunpack. The current caller passesBATCH_SIZE = 50, so this is safe today. If any caller raiseslimitto several thousand, the script fails. Consider clampinglimitinsidepopDue.🤖 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/tryna/domain/reminder/repository/AlarmDelayedQueueRepository.java` around lines 31 - 37, Update POP_DUE_SCRIPT to use a parameterized RedisScript<List<String>> with an explicit type reference, and adjust popDue to remove the resulting unchecked conversion. In popDue, clamp the requested limit to a safe maximum before passing it to the Lua script so unpack(dueItems) cannot exceed Lua’s argument limit, while preserving normal BATCH_SIZE behavior.src/main/java/com/tryna/domain/alarm/util/AlarmCursorCodec.java (1)
29-29: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSpecify the charset explicitly.
String.getBytes()uses the platform default charset. The decoder on line 42 lets Jackson detect the encoding. The current payload holds only ASCII, so the values match today, but the pairing breaks if the payload ever carries non-ASCII text. UseStandardCharsets.UTF_8on both sides.♻️ Proposed change
- return Base64.getUrlEncoder().withoutPadding().encodeToString(json.getBytes()); + return Base64.getUrlEncoder().withoutPadding() + .encodeToString(json.getBytes(StandardCharsets.UTF_8));Add the import:
import java.nio.charset.StandardCharsets;🤖 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/tryna/domain/alarm/util/AlarmCursorCodec.java` at line 29, Update the encoding and decoding logic in AlarmCursorCodec to use StandardCharsets.UTF_8 explicitly instead of the platform-default charset, adding the necessary import and applying it consistently to both String-to-byte and byte-to-String conversions.src/main/resources/application.yaml (1)
20-21: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider requiring the credentials in production profiles.
The empty default lets the application start without Firebase credentials.
FcmPushServicethen logs a warning and drops every push. Alarm delivery fails silently in that state. If you want local runs to stay easy, keep the empty default for the local profile and require the value in the production profile, or add a startup health indicator that reports the disabled state.🤖 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/resources/application.yaml` around lines 20 - 21, Update the Firebase credentials configuration used by FcmPushService so production requires FIREBASE_CREDENTIALS_BASE64 instead of defaulting to empty, while preserving an empty default for the local profile if needed. Ensure production startup fails or clearly reports the missing credential rather than allowing pushes to be silently dropped.src/main/java/com/tryna/domain/alarm/service/AlarmQueryService.java (2)
76-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the original failure information.
The generic
catchblock discards the exception. No log entry and no cause remain, so a productionF100_ALARMLIST_500gives no way to find the root cause. Log the exception and attach it as the cause.♻️ Proposed change
} catch (BusinessException e) { throw e; } catch (Exception e) { + log.error("알람 목록 조회에 실패했습니다. userId={}", userId, e); throw new BusinessException(AlarmErrorCode.F100_ALARMLIST_500); }Add
@Slf4jto the class for the logger. Confirm thatBusinessExceptioncan carry a cause; if it cannot, the log entry alone is enough.🤖 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/tryna/domain/alarm/service/AlarmQueryService.java` around lines 76 - 80, Update the generic catch block in AlarmQueryService to log the original exception with the class logger, and preserve it as the cause when constructing BusinessException if that constructor supports causes. Add Lombok’s `@Slf4j` annotation to AlarmQueryService; if BusinessException cannot accept a cause, retain the exception details in the log while keeping the existing error code.
83-86: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReconsider
F100_ALARMLIST_409for the single-alarm path.
validateActiveUserruns for bothgetAlarmandgetMyAlarms. A deleted user callingGET /api/v1/alarms/my/{reminderId}receives the alarm-list error code and its message about the list query. Use a shared user-state error code, or add a dedicated code for the detail endpoint, so the response matches the called endpoint.The local variable
useris also unused; the call can stand alone.🤖 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/tryna/domain/alarm/service/AlarmQueryService.java` around lines 83 - 86, The validateActiveUser method currently uses the alarm-list-specific F100_ALARMLIST_409 code for both detail and list requests; replace it with a shared user-state code or a dedicated detail-endpoint code, and remove the unused user local by invoking the repository lookup directly while preserving the exception behavior.src/main/java/com/tryna/domain/reminder/repository/RemindersRepository.java (1)
20-36: 🚀 Performance & Scalability | 🔵 TrivialConfirm the supporting index for the keyset query.
The keyset predicate and the
ORDER BY r.updatedAt DESC, r.reminderId DESCneed a composite index on(user_id, updated_at DESC, reminder_id DESC). Without it, every alarm-list request sorts the user's full reminder set. The existence checks on lines 39-47 and the collection queries on lines 50-63 likewise need indexes ontarget_event_idplusreminder_statusand ontarget_action_item_idplusreminder_status.#!/bin/bash # Show the reminders table definition and every index declared for it. fd -e sql . src/main/resources/db/migration --exec rg -n -i -C6 'CREATE TABLE[[:space:]]+(IF NOT EXISTS[[:space:]]+)?reminders\b|CREATE[[:space:]]+(UNIQUE[[:space:]]+)?INDEX[^;]*reminders' # Show index declarations on the JPA entity. fd -g 'Reminders.java' src/main/java --exec rg -n -C3 '`@Table`|`@Index`|`@Column`'🤖 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/tryna/domain/reminder/repository/RemindersRepository.java` around lines 20 - 36, Update the reminders database schema or the Reminders entity indexing configuration to add composite indexes for (user_id, updated_at DESC, reminder_id DESC), (target_event_id, reminder_status), and (target_action_item_id, reminder_status), reusing the project’s existing migration or `@Table/`@Index conventions.src/main/java/com/tryna/global/exception/GlobalExceptionHandler.java (1)
70-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider replacing the URI chain with a declarative map.
Each new feature adds another branch to this chain, so the handler now knows about every endpoint. A
Mapof method plus path pattern toErrorCode, or an@ExceptionHandlerin a controller advice scoped to the alarm package, keeps the routing knowledge next to the feature.Also note that line 72 uses
startsWith, so any future path with the/api/v1/alarms/myprefix inheritsF100_ALARMLIST_400_3. If you only want the list and detail endpoints, use anAntPathMatcherpattern such as/api/v1/alarms/myand/api/v1/alarms/my/*.🤖 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/tryna/global/exception/GlobalExceptionHandler.java` around lines 70 - 73, Refactor the endpoint-specific URI chain in GlobalExceptionHandler into a declarative method-and-path mapping, or move alarm-specific handling into scoped controller advice. Preserve the existing error codes, and replace the broad startsWith matching for /api/v1/alarms/my with exact matching for the list endpoint plus a single-segment detail pattern so unrelated prefixed paths do not inherit F100_ALARMLIST_400_3.src/main/java/com/tryna/domain/alarm/controller/docs/AlarmControllerDocs.java (1)
119-131: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a distinct path for alarm correction.
correctEventAlarmsdocumentsPATCH /api/v1/alarms/{eventId}, andgetAlarmdocumentsGET /api/v1/alarms/{reminderId}. The same single-segment template carries two different identifier types. Clients and future maintainers can confuse aneventIdwith areminderId. A path such as/api/v1/alarms/correct/event/{eventId}matches the existing/remind/event/{eventId}style and removes the ambiguity.🤖 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/tryna/domain/alarm/controller/docs/AlarmControllerDocs.java` around lines 119 - 131, Update the route mapping for correctEventAlarms to use a distinct event-correction path such as /api/v1/alarms/correct/event/{eventId}, matching the existing /remind/event/{eventId} convention, while keeping getAlarm’s reminderId path unchanged.src/main/java/com/tryna/domain/alarm/controller/AlarmController.java (1)
45-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider centralizing the authentication guard.
All eight endpoints repeat the same
userId == nullcheck. Extract it into a private helper, or resolve it once through an argument resolver or@PreAuthorize. This removes eight copies of the same branch and prevents a future endpoint from omitting the check.♻️ Example helper
+ private Long requireUserId(Long userId) { + if (userId == null) { + throw new BusinessException(AuthErrorCode.AUTH_401); + } + return userId; + }🤖 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/tryna/domain/alarm/controller/AlarmController.java` around lines 45 - 50, Centralize the repeated authentication validation used by the AlarmController endpoints: extract the userId == null check and AUTH_401 exception into a private helper, then reuse it from agreeAlarmTerm and the other authenticated endpoint methods. Preserve the existing authenticated behavior and exception type while removing duplicated guard branches.src/main/java/com/tryna/domain/alarm/service/AlarmTermService.java (1)
36-42: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse distinct error codes for the two failure causes.
Line 38 and line 41 both throw
F100_ALARM_TERM_400. Line 38 means the ALARM term row does not exist, which is a server-side configuration problem. Line 41 means the user already agreed, which is a client conflict. Clients cannot tell the two apart, and a missing term row is reported as a client error. Add a 409-style code for the duplicate agreement and a 500-style code for the missing term, consistent withF100_PUSH_TOKEN_409andF100_PUSH_TOKEN_500inAlarmPushTokenService.🤖 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/tryna/domain/alarm/service/AlarmTermService.java` around lines 36 - 42, Use distinct error codes in AlarmTermService: replace the missing ALARM term exception in the findLatestTermsByTypes flow with a 500-style code, and replace the existing-agreement exception in the userAgreedTermsRepository check with a 409-style code. Add or reuse AlarmErrorCode entries consistent with F100_PUSH_TOKEN_500 and F100_PUSH_TOKEN_409.src/main/java/com/tryna/domain/alarm/service/AlarmPushTokenService.java (2)
21-23: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRemove
@Transactionalfrom this method.This method touches only Redis and Firebase. Neither participates in the JPA transaction, so the annotation grants no atomicity or rollback. It does hold a database connection for the whole method, including the blocking FCM dry-run send inside
fcmPushService.validateToken. Under a slow Firebase response, connections are held for no benefit.♻️ Proposed change
- `@Transactional` public void registerPushToken(Long userId, String fcmPushToken) {Also confirm that the Firebase SDK in use is configured with a send timeout. Without one, a slow Firebase response occupies the request thread.
🤖 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/tryna/domain/alarm/service/AlarmPushTokenService.java` around lines 21 - 23, Remove `@Transactional` from registerPushToken in AlarmPushTokenService so the Redis/Firebase-only flow does not hold a JPA connection; also verify that the configured Firebase SDK client has an explicit send timeout and add or configure one if missing.
36-37: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDecouple the push-token TTL from the refresh-token lifetime.
The TTL comes from
jwtTokenProvider.getRefreshExpirationSeconds(). The lifetime of an FCM push token and the lifetime of a JWT refresh token are unrelated concerns. A change to the refresh-token policy silently changes push retention. Introduce a dedicated configuration property for the push-token TTL.🤖 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/tryna/domain/alarm/service/AlarmPushTokenService.java` around lines 36 - 37, Update AlarmPushTokenService to derive the FCM token Duration from a dedicated push-token TTL configuration property rather than jwtTokenProvider.getRefreshExpirationSeconds(). Inject or reuse the established configuration mechanism, and pass that value to fcmTokenRedisRepository.add while leaving refresh-token expiration handling unchanged.
🤖 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 @.github/workflows/deploy.yml:
- Line 110: Remove FIREBASE_CREDENTIALS_BASE64 from the task definition’s
environment-variables and store it in AWS Secrets Manager or SSM Parameter Store
instead. Configure the ECS container secrets entry with valueFrom referencing
that secret, and grant access only to the task execution role.
In `@src/main/java/com/tryna/domain/alarm/controller/AlarmController.java`:
- Around line 159-161: Update the success message returned by the action-item
endpoint in AlarmController so it identifies a 준비/실행 항목 reminder alarm rather
than a 일정 reminder, and accurately states that the reminder was scheduled rather
than sent. Leave the existing response code and response payload unchanged.
- Around line 142-144: Update the response code in AlarmController’s success
response from F101_EVEMT_ALARM_200 to F101_EVENT_ALARM_200, matching the
AlarmErrorCode token and preserving the existing response message and payload.
In `@src/main/java/com/tryna/domain/alarm/dto/AlarmDetailResponse.java`:
- Around line 20-33: Update AlarmDetailResponse.from to read both targetEvent
and targetActionItem associations into local variables and null-check them
before accessing their IDs, rather than inferring the association solely from
TargetType. Preserve null IDs for missing associations so inconsistent reminders
do not throw during AlarmQueryService.getMyAlarms mapping, and remove the
TargetType import if it is no longer used.
In `@src/main/java/com/tryna/domain/alarm/service/AlarmPushTokenService.java`:
- Around line 32-34: Update the registration method containing the
F100_PUSH_TOKEN_409 guard to allow rotated FCM tokens: inspect
FcmTokenRedisRepository’s available write operations and replace the user’s
stored token or add the new token for multi-device support. Do not reject merely
because any token exists; only retain a 409 when the identical token is already
stored if that product rule is required.
In `@src/main/java/com/tryna/domain/alarm/service/AlarmQueryService.java`:
- Around line 64-70: Update the cursor pagination flow in AlarmQueryService and
its matching query logic to use an immutable ordering key, preferably createdAt
plus reminderId, instead of updatedAt. Encode the same immutable fields in
AlarmCursorCodec.Cursor and ensure the repository ordering and cursor predicates
use that identical keyset order.
In `@src/main/java/com/tryna/domain/alarm/service/FcmPushService.java`:
- Around line 31-34: Update
src/main/java/com/tryna/domain/alarm/service/FcmPushService.java:31-34 to return
UNAVAILABLE when FirebaseApp.getApps() is empty, unless a real token-format
validation is performed; update FcmPushService.send at
src/main/java/com/tryna/domain/alarm/service/FcmPushService.java:58-80 to return
a delivery outcome that lets the dispatcher retry transient failures and remove
UNREGISTERED tokens; update src/main/resources/application.yaml:20-21 to require
FIREBASE_CREDENTIALS_BASE64 in production or expose Firebase’s disabled state
through a health indicator.
- Around line 31-34: Update the Firebase-uninitialized branch in FcmPushService
to return TokenValidationResult.UNAVAILABLE instead of VALID, ensuring unchecked
non-blank tokens are not stored as usable tokens and callers can return
F100_PUSH_TOKEN_500. Only retain VALID if this branch first performs the
required token length and character-set validation.
- Around line 40-42: Configure finite Firebase connect and read timeouts during
FirebaseApp initialization, using the existing FirebaseOptions setup rather than
changing FcmPushService.send validation flow. Ensure
FirebaseMessaging.send(message, true) uses those bounded timeout values so
stalled FCM requests cannot block the token-validation request indefinitely.
In `@src/main/java/com/tryna/domain/event/util/EventRecurrenceCalculator.java`:
- Around line 55-77: Update nextMonthly and nextYearly to derive the target
month or year from current without retaining current’s clamped day, then apply
event.getRecurrenceDayOfMonth() to that target using the month’s valid-day
clamp. Preserve the existing behavior when recurrenceDayOfMonth is null and
ensure configured dates such as 31 or February 29 recover their canonical day in
later occurrences.
In
`@src/main/java/com/tryna/domain/reminder/repository/AlarmDelayedQueueRepository.java`:
- Around line 49-65: Add a re-enqueue method to AlarmDelayedQueueRepository
alongside popDue that schedules a reminder ID back into QUEUE_KEY with a short
backoff score, then update AlarmReminderDispatcher.dispatchDueReminders to
invoke it whenever dispatchOne fails. Preserve the existing SCHEDULED state and
ensure the failed reminder is available for a later poll instead of being
permanently removed.
In
`@src/main/java/com/tryna/domain/reminder/service/AlarmReminderDispatcher.java`:
- Around line 164-168: Update AlarmReminderDispatcher.java at lines 164-168 and
188-192: in both reschedule paths, repeatedly advance the occurrence with
EventRecurrenceCalculator.nextOccurrenceDateAfter until
computeEventReminderTimeForOccurrence or
computeActionItemReminderTimeForOccurrence returns a future time, enforcing a
maximum number of advances; schedule only that future time, and call markSent()
in the action-item path when no future occurrence is found.
- Around line 110-123: Update sendPush to handle FCM delivery per token without
allowing one failure to abort the entire reminder dispatch: catch and log
exceptions for individual fcmPushService.send calls, continue processing
remaining tokens, and return success only when at least one token is delivered
while preserving the existing false result for an empty token set. If
FcmPushService exposes sendEachForMulticast, prefer its per-token result
handling to avoid sequential network calls and still report partial success.
- Around line 46-78: Move the transactional per-reminder logic from
AlarmReminderDispatcher.dispatchOne into a separate Spring-managed
AlarmReminderDispatchExecutor bean, including its required collaborators, and
invoke that bean from dispatchDueReminders instead of self-invoking dispatchOne.
Preserve the existing dispatch behavior while ensuring each item runs through
the transactional proxy; add a test verifying the reminder status is persisted
after one dispatch cycle.
In
`@src/main/java/com/tryna/domain/reminder/service/AlarmReminderScheduleService.java`:
- Around line 251-255: Update buildEventAlarmBody to ensure the concatenated
date-time and description never exceed the 255-character alarm_body column
limit. Truncate the final trimmed body to 255 characters while preserving the
existing formatting and null-description handling.
- Around line 160-175: Update correctEventAlarms to validate recomputed
schedules before rescheduling: compute the event schedule once outside its
reminder loop, and process each event or action-item reminder only when
isFuture(scheduledAt) returns true, preventing null or past queue entries. Add
the isFuture helper using a non-null check and comparison with the current time.
For reminders that fail validation, confirm and implement the intended
behavior—skip them or cancel via markCanceled() and
alarmDelayedQueueRepository.cancel(...).
- Around line 307-318: Update deriveActionItemOccurrenceDate in
AlarmReminderScheduleService to handle recurring TIMED_ACTION items with null
offsetDays by deriving a valid occurrence relative to the recurring reminder
rather than reusing the stored occurrenceDate. Add the smallest appropriate
migration or runtime guard, while preserving existing behavior for non-recurring
items and timed actions with a defined offset.
In `@src/main/java/com/tryna/global/exception/AlarmErrorCode.java`:
- Around line 85-101: Correct the code string values in the three
F101_EVENT_ALARM_400_1, F101_EVENT_ALARM_400_2, and F101_EVENT_ALARM_400_3 enum
entries from “EVEMT” to “EVENT” so they match the enum names and public API
contract. If “EVEMT” is specification-mandated, retain it and document that
spelling as intentional instead.
- Around line 139-155: Update the messages for F102_ACTIONITEM_ALARM_403,
F102_ACTIONITEM_ALARM_409, and F102_ACTIONITEM_ALARM_500 in AlarmErrorCode so
they refer to the relevant action item (준비/실행 항목) instead of 일정, while
preserving each message’s permission, duplicate-alarm, and push-send error
meaning.
---
Nitpick comments:
In `@src/main/java/com/tryna/domain/alarm/controller/AlarmController.java`:
- Around line 45-50: Centralize the repeated authentication validation used by
the AlarmController endpoints: extract the userId == null check and AUTH_401
exception into a private helper, then reuse it from agreeAlarmTerm and the other
authenticated endpoint methods. Preserve the existing authenticated behavior and
exception type while removing duplicated guard branches.
In
`@src/main/java/com/tryna/domain/alarm/controller/docs/AlarmControllerDocs.java`:
- Around line 119-131: Update the route mapping for correctEventAlarms to use a
distinct event-correction path such as /api/v1/alarms/correct/event/{eventId},
matching the existing /remind/event/{eventId} convention, while keeping
getAlarm’s reminderId path unchanged.
In `@src/main/java/com/tryna/domain/alarm/service/AlarmPushTokenService.java`:
- Around line 21-23: Remove `@Transactional` from registerPushToken in
AlarmPushTokenService so the Redis/Firebase-only flow does not hold a JPA
connection; also verify that the configured Firebase SDK client has an explicit
send timeout and add or configure one if missing.
- Around line 36-37: Update AlarmPushTokenService to derive the FCM token
Duration from a dedicated push-token TTL configuration property rather than
jwtTokenProvider.getRefreshExpirationSeconds(). Inject or reuse the established
configuration mechanism, and pass that value to fcmTokenRedisRepository.add
while leaving refresh-token expiration handling unchanged.
In `@src/main/java/com/tryna/domain/alarm/service/AlarmQueryService.java`:
- Around line 76-80: Update the generic catch block in AlarmQueryService to log
the original exception with the class logger, and preserve it as the cause when
constructing BusinessException if that constructor supports causes. Add Lombok’s
`@Slf4j` annotation to AlarmQueryService; if BusinessException cannot accept a
cause, retain the exception details in the log while keeping the existing error
code.
- Around line 83-86: The validateActiveUser method currently uses the
alarm-list-specific F100_ALARMLIST_409 code for both detail and list requests;
replace it with a shared user-state code or a dedicated detail-endpoint code,
and remove the unused user local by invoking the repository lookup directly
while preserving the exception behavior.
In `@src/main/java/com/tryna/domain/alarm/service/AlarmTermService.java`:
- Around line 36-42: Use distinct error codes in AlarmTermService: replace the
missing ALARM term exception in the findLatestTermsByTypes flow with a 500-style
code, and replace the existing-agreement exception in the
userAgreedTermsRepository check with a 409-style code. Add or reuse
AlarmErrorCode entries consistent with F100_PUSH_TOKEN_500 and
F100_PUSH_TOKEN_409.
In `@src/main/java/com/tryna/domain/alarm/util/AlarmCursorCodec.java`:
- Line 29: Update the encoding and decoding logic in AlarmCursorCodec to use
StandardCharsets.UTF_8 explicitly instead of the platform-default charset,
adding the necessary import and applying it consistently to both String-to-byte
and byte-to-String conversions.
In
`@src/main/java/com/tryna/domain/reminder/repository/AlarmDelayedQueueRepository.java`:
- Around line 31-37: Update POP_DUE_SCRIPT to use a parameterized
RedisScript<List<String>> with an explicit type reference, and adjust popDue to
remove the resulting unchecked conversion. In popDue, clamp the requested limit
to a safe maximum before passing it to the Lua script so unpack(dueItems) cannot
exceed Lua’s argument limit, while preserving normal BATCH_SIZE behavior.
In `@src/main/java/com/tryna/domain/reminder/repository/RemindersRepository.java`:
- Around line 20-36: Update the reminders database schema or the Reminders
entity indexing configuration to add composite indexes for (user_id, updated_at
DESC, reminder_id DESC), (target_event_id, reminder_status), and
(target_action_item_id, reminder_status), reusing the project’s existing
migration or `@Table/`@Index conventions.
In
`@src/main/java/com/tryna/domain/reminder/service/AlarmReminderScheduleService.java`:
- Around line 81-98: Add Lombok `@Slf4j` to AlarmReminderScheduleService and log
the caught original exception, including its stack trace, immediately before
converting it to F101_EVENT_ALARM_500 or F102_ACTIONITEM_ALARM_500 in both try
blocks. Preserve BusinessException propagation and the existing mapped error
responses; optionally use a shared helper for the identical logging-and-mapping
logic.
In `@src/main/java/com/tryna/global/exception/GlobalExceptionHandler.java`:
- Around line 70-73: Refactor the endpoint-specific URI chain in
GlobalExceptionHandler into a declarative method-and-path mapping, or move
alarm-specific handling into scoped controller advice. Preserve the existing
error codes, and replace the broad startsWith matching for /api/v1/alarms/my
with exact matching for the list endpoint plus a single-segment detail pattern
so unrelated prefixed paths do not inherit F100_ALARMLIST_400_3.
In `@src/main/resources/application.yaml`:
- Around line 20-21: Update the Firebase credentials configuration used by
FcmPushService so production requires FIREBASE_CREDENTIALS_BASE64 instead of
defaulting to empty, while preserving an empty default for the local profile if
needed. Ensure production startup fails or clearly reports the missing
credential rather than allowing pushes to be silently dropped.
🪄 Autofix
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 Plus
Run ID: b23e92ee-0aa9-4727-9317-5bd6d4a81c3a
📒 Files selected for processing (33)
.github/workflows/deploy.ymlbuild.gradlesrc/main/java/com/tryna/TrynaApplication.javasrc/main/java/com/tryna/domain/action/service/ActionItemService.javasrc/main/java/com/tryna/domain/alarm/controller/AlarmController.javasrc/main/java/com/tryna/domain/alarm/controller/docs/AlarmControllerDocs.javasrc/main/java/com/tryna/domain/alarm/dto/ActionItemReminderResponse.javasrc/main/java/com/tryna/domain/alarm/dto/AlarmCorrectionResponse.javasrc/main/java/com/tryna/domain/alarm/dto/AlarmDetailResponse.javasrc/main/java/com/tryna/domain/alarm/dto/AlarmListResponse.javasrc/main/java/com/tryna/domain/alarm/dto/AlarmPushTokenRequest.javasrc/main/java/com/tryna/domain/alarm/dto/AlarmStateResponse.javasrc/main/java/com/tryna/domain/alarm/dto/EventReminderResponse.javasrc/main/java/com/tryna/domain/alarm/service/AlarmPushTokenService.javasrc/main/java/com/tryna/domain/alarm/service/AlarmQueryService.javasrc/main/java/com/tryna/domain/alarm/service/AlarmStateService.javasrc/main/java/com/tryna/domain/alarm/service/AlarmTermService.javasrc/main/java/com/tryna/domain/alarm/service/FcmPushService.javasrc/main/java/com/tryna/domain/alarm/util/AlarmCursorCodec.javasrc/main/java/com/tryna/domain/event/util/EventRecurrenceCalculator.javasrc/main/java/com/tryna/domain/reminder/entity/Reminders.javasrc/main/java/com/tryna/domain/reminder/repository/AlarmDelayedQueueRepository.javasrc/main/java/com/tryna/domain/reminder/repository/RemindersRepository.javasrc/main/java/com/tryna/domain/reminder/service/AlarmReminderDispatcher.javasrc/main/java/com/tryna/domain/reminder/service/AlarmReminderScheduleService.javasrc/main/java/com/tryna/domain/term/enums/TermType.javasrc/main/java/com/tryna/domain/term/repository/UserAgreedTermsRepository.javasrc/main/java/com/tryna/domain/user/entity/Users.javasrc/main/java/com/tryna/global/config/FirebaseConfig.javasrc/main/java/com/tryna/global/exception/AlarmErrorCode.javasrc/main/java/com/tryna/global/exception/GlobalExceptionHandler.javasrc/main/resources/application.yamlsrc/main/resources/db/migration/V18__add_alarm_term_and_state.sql
There was a problem hiding this comment.
Actionable comments posted: 2
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/tryna/domain/reminder/service/AlarmReminderScheduleService.java (1)
74-93: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winMap reminder uniqueness conflicts to 409.
The unique indexes already prevent concurrent duplicate inserts for the same target, schedule time, and delivery channel. Map
DataIntegrityViolationExceptiontoF101_EVENT_ALARM_409orF102_ACTIONITEM_ALARM_409instead of returning a 500 response.🤖 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/tryna/domain/reminder/service/AlarmReminderScheduleService.java` around lines 74 - 93, Update the exception handling in AlarmReminderScheduleService around the remindersRepository.save calls to catch DataIntegrityViolationException caused by reminder uniqueness constraints and throw F101_EVENT_ALARM_409 for event reminders or F102_ACTIONITEM_ALARM_409 for action-item reminders. Preserve existing handling for unrelated exceptions.
🤖 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/tryna/domain/reminder/service/AlarmReminderScheduleService.java`:
- Line 63: Update AlarmReminderScheduleService validation calls to use dedicated
400 error codes for disabled alarms: replace the codes passed at the alarm-state
checks near validateAlarmStateEnabled with new distinct disabled-state codes,
while preserving F101_EVENT_ALARM_403 and F102_ACTIONITEM_ALARM_403 for
ownership-denial checks.
In `@src/main/java/com/tryna/global/config/SecurityConfig.java`:
- Around line 45-46: Require authentication for alarm consent and FCM token
registration: remove both alarm paths from allowUris in SecurityConfig.java;
update the corresponding AlarmController methods to use `@AuthenticationPrincipal`
Long userId with a null check; and update both declarations in
AlarmControllerDocs.java to include `@SecurityRequirement`(name = "bearerAuth")
plus the hidden authenticated-user parameter. Apply changes in
SecurityConfig.java lines 45-46 and AlarmControllerDocs.java lines 33-45; the
controller implementation requires the related AlarmController methods.
---
Outside diff comments:
In
`@src/main/java/com/tryna/domain/reminder/service/AlarmReminderScheduleService.java`:
- Around line 74-93: Update the exception handling in
AlarmReminderScheduleService around the remindersRepository.save calls to catch
DataIntegrityViolationException caused by reminder uniqueness constraints and
throw F101_EVENT_ALARM_409 for event reminders or F102_ACTIONITEM_ALARM_409 for
action-item reminders. Preserve existing handling for unrelated exceptions.
🪄 Autofix
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 Plus
Run ID: 30e848d3-3756-44fa-9066-fb54d0a0137a
📒 Files selected for processing (6)
src/main/java/com/tryna/domain/alarm/controller/AlarmController.javasrc/main/java/com/tryna/domain/alarm/controller/docs/AlarmControllerDocs.javasrc/main/java/com/tryna/domain/alarm/service/AlarmQueryService.javasrc/main/java/com/tryna/domain/reminder/repository/RemindersRepository.javasrc/main/java/com/tryna/domain/reminder/service/AlarmReminderScheduleService.javasrc/main/java/com/tryna/global/config/SecurityConfig.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/tryna/domain/alarm/service/AlarmQueryService.java
- src/main/java/com/tryna/domain/alarm/controller/AlarmController.java
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/tryna/domain/alarm/service/FcmPushService.java`:
- Around line 93-105: Update the multicast send flow in FcmPushService to
partition tokens into batches of at most 500 before calling
sendEachForMulticast. Send each batch with the existing notification and data
payload, then merge each BatchResponse’s per-token outcomes in original token
order so token/outcome alignment is preserved. Apply failure handling per batch
without marking unrelated successful batches as transient failures.
- Around line 176-180: Update mapDeliveryResult to stop treating every
INVALID_ARGUMENT as UNREGISTERED: map it to a separate permanent
delivery-failure result, while preserving UNREGISTERED for UNREGISTERED and
SENDER_ID_MISMATCH. Only classify INVALID_ARGUMENT as UNREGISTERED when the
payload is explicitly known to be valid, and ensure
AlarmReminderDispatchExecutor retains tokens for payload failures.
In
`@src/main/java/com/tryna/domain/reminder/repository/AlarmDelayedQueueRepository.java`:
- Around line 51-53: Change ReminderRepository.reEnqueue to track retry attempts
and return false once the configured maximum is reached, marking the reminder as
failed instead of scheduling it again; otherwise preserve the one-minute backoff
and return true. Update AlarmReminderDispatchExecutor.sendPush and
AlarmReminderDispatcher.dispatchDueReminders to mark the reminder failed when
reEnqueue returns false, and clear the retry counter after a successful send.
In
`@src/main/java/com/tryna/domain/reminder/service/AlarmReminderDispatchExecutor.java`:
- Around line 167-177: The reschedule paths reuse stale alarm-body text from the
previous occurrence. Expose the body-builder methods already used by
AlarmReminderScheduleService, then in AlarmReminderDispatchExecutor update both
the event path at
src/main/java/com/tryna/domain/reminder/service/AlarmReminderDispatchExecutor.java:167-177
and the action-item path at :188-198 to build the body for nextScheduledAt and
pass it to reminder.reschedule instead of reminder.getAlarmBody().
🪄 Autofix
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 Plus
Run ID: 5a34b3e4-6e63-47ce-9034-43618c8a88db
📒 Files selected for processing (11)
src/main/java/com/tryna/domain/alarm/controller/AlarmController.javasrc/main/java/com/tryna/domain/alarm/dto/AlarmDetailResponse.javasrc/main/java/com/tryna/domain/alarm/service/AlarmPushTokenService.javasrc/main/java/com/tryna/domain/alarm/service/FcmPushService.javasrc/main/java/com/tryna/domain/reminder/repository/AlarmDelayedQueueRepository.javasrc/main/java/com/tryna/domain/reminder/service/AlarmReminderDispatchExecutor.javasrc/main/java/com/tryna/domain/reminder/service/AlarmReminderDispatcher.javasrc/main/java/com/tryna/domain/reminder/service/AlarmReminderScheduleService.javasrc/main/java/com/tryna/global/config/FirebaseConfig.javasrc/main/resources/application-prod.yamlsrc/main/resources/application.yaml
🚧 Files skipped from review as they are similar to previous changes (5)
- src/main/resources/application.yaml
- src/main/java/com/tryna/domain/alarm/service/AlarmPushTokenService.java
- src/main/java/com/tryna/domain/alarm/controller/AlarmController.java
- src/main/java/com/tryna/domain/alarm/dto/AlarmDetailResponse.java
- src/main/java/com/tryna/domain/reminder/service/AlarmReminderScheduleService.java
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/tryna/domain/reminder/service/AlarmReminderDispatchExecutor.java`:
- Around line 118-121: Remove the complete FCM token from the PERMANENT_FAILURE
warning in AlarmReminderDispatchExecutor, logging only reminder.getReminderId()
or an approved non-reversible token fingerprint. Apply the same redaction to the
exception log near the referenced branch, ensuring no application log includes
the raw outcome.token().
- Around line 192-193: Update AlarmReminderDispatchExecutor so the event
reminder path at
src/main/java/com/tryna/domain/reminder/service/AlarmReminderDispatchExecutor.java:192-193
records the Redis scheduling change in an outbox within the same transaction as
reminder.reschedule, then relays it to Redis only after commit; apply the same
outbox flow to the action-item reminder path at
src/main/java/com/tryna/domain/reminder/service/AlarmReminderDispatchExecutor.java:213-215.
🪄 Autofix
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 Plus
Run ID: da69502c-852c-493c-aef6-b8cf20a39400
📒 Files selected for processing (4)
src/main/java/com/tryna/domain/alarm/service/FcmPushService.javasrc/main/java/com/tryna/domain/reminder/repository/AlarmDelayedQueueRepository.javasrc/main/java/com/tryna/domain/reminder/service/AlarmReminderDispatchExecutor.javasrc/main/java/com/tryna/domain/reminder/service/AlarmReminderDispatcher.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/tryna/domain/reminder/service/AlarmReminderDispatcher.java
This was referenced Aug 11, 2026
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
1 participant
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.
🔗 이슈 번호
#️⃣ 기능 설명
알람 기능을 구현했습니다. ALARM 약관 동의·FCM 토큰 등록·알람 on/off 토글·일정/준비·실행 항목 리마인드 예약·일정 수정 시 알람 보정·완료 시 알람 취소·알람 조회까지, 명세서 기준 API와 Redis 기반 지연 발송 파이프라인을 포함합니다.
📌 작업 내용
DB
V18:terms에ALARM타입 추가,users.alarm_state컬럼 추가TermType.ALARMenum 및Users.alarmState필드 반영FirebaseConfig,FIREBASE_CREDENTIALS_BASE64환경변수)firebase-admin의존성 추가, deploy workflow에 Firebase secret 반영AlarmDelayedQueueRepository) — Redisson 없이StringRedisTemplate사용@EnableScheduling및AlarmReminderDispatcher(15초 주기 FCM 발송 컨슈머)API — F100 (알람 설정/조회)
POST /api/v1/alarms/term— ALARM 약관 동의 및alarm_state활성화POST /api/v1/alarms/push-token— FCM 토큰 검증 후 Redis 저장PATCH /api/v1/alarms/state— 알람 접근 권한 토글 (활성화 시 약관 동의 필수)GET /api/v1/alarms/my— 내 알람 목록 커서 페이징 (updated_at DESC, size 1~100)GET /api/v1/alarms/{reminderId}— 알람 단건 조회PATCH /api/v1/alarms/{eventId}— 일정 수정(C107) 후 알람 발송 시각·제목·본문 보정API — F101 / F102 (리마인드 예약)
POST /api/v1/alarms/remind/event/{eventId}— 일정 리마인드 알람 생성POST /api/v1/alarms/remind/action-item/{actionItemId}— 준비/실행 항목 리마인드 알람 생성UNTIMED_PREP: 부모 일정 시작 2시간 전TIMED_ACTION:displayTime또는 표시일 07:00EventRecurrenceCalculator)도메인 로직
Reminders엔티티 factory/reschedule/상태 전이 메서드 추가AlarmReminderScheduleService— 생성·보정·취소 및 발송 시각 계산ActionItemService.updateActionItemStatus—COMPLETED처리 시 예약 알람 취소AlarmReminderDispatcher— 발송 직전completed_at/ soft delete /alarm_state재검증AlarmErrorCode— F100/F101/F102 관련 에러 코드 정의AlarmControllerDocs)✅ 체크리스트
📸 스크린샷 (선택)
변경 사항
변경 목적
호환성 및 주요 영향
users테이블에alarm_state컬럼을 추가했습니다.terms.term_type에ALARM값을 추가했습니다.FIREBASE_CREDENTIALS_BASE64설정이 필요합니다.테스트