Skip to content

재촉하기 API 에러 해결#257

Merged
kelly6bf merged 2 commits intodevfrom
feature/#256-reminder_api_error_fix
Feb 4, 2026
Merged

재촉하기 API 에러 해결#257
kelly6bf merged 2 commits intodevfrom
feature/#256-reminder_api_error_fix

Conversation

@kelly6bf
Copy link

@kelly6bf kelly6bf commented Feb 4, 2026

😉 연관 이슈

Resolves #256

🧑‍💻 수행 작업

  • 잘못 설정된 트랜잭션 read-only 설정 수정
  • 분기 처리 로직에서 조기 종료가 되지 않는 잘못된 로직 수정

📢 참고 사항

X

Summary by CodeRabbit

  • 버그 수정

    • TODO 리마인더 처리 로직 개선으로 알림 시스템 안정성 향상
  • 최적화

    • 개발 및 로컬 환경에서 알림 시스템 지원 범위 확대

@kelly6bf kelly6bf self-assigned this Feb 4, 2026
@kelly6bf kelly6bf added the deploy 해당 라벨을 붙이면 배포 작업을 진행합니다. (붙이지 않으면 배포 작업이 진행되지 않고 머지만 됩니다.) label Feb 4, 2026
@coderabbitai
Copy link

coderabbitai bot commented Feb 4, 2026

📝 Walkthrough

Walkthrough

Spring 프로필 활성화 조건을 확대하고, 리마인더 서비스 메서드에 트랜잭션 처리 및 제어 흐름을 추가하는 변경입니다. 총 2개 파일에서 3줄이 수정되었습니다.

Changes

Cohort / File(s) Summary
Spring 프로필 설정
src/main/java/site/dogether/notification/sender/performance_test/PerformanceTestNotificationSender.java
@Profile 어노테이션을 "pt"에서 {"pt", "dev", "local"}로 변경하여 빈 활성화 조건 확대
트랜잭션 리마인더 서비스
src/main/java/site/dogether/reminder/service/TodoActivityReminderService.java
sendReminder 메서드에 @Transactional 어노테이션 추가 및 TODO_CERTIFICATION 처리 후 조기 반환 로직 도입

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed 제목이 PR의 주요 변경사항(재촉하기 API 에러 해결)을 명확하게 요약하고 있으며, 간결하고 구체적입니다.
Description check ✅ Passed PR 설명이 템플릿의 모든 필수 섹션(연관 이슈, 수행 작업, 참고 사항)을 포함하고 있으며, 각 섹션이 적절하게 작성되어 있습니다.
Linked Issues check ✅ Passed PR의 코드 변경사항들(트랜잭션 설정 수정, 분기 처리 로직 개선)이 이슈 #256의 재촉하기 API 에러 해결 목표를 충족합니다.
Out of Scope Changes check ✅ Passed 모든 변경사항이 이슈 #256의 범위 내(트랜잭션 설정 수정, 분기 처리 로직 수정)에 있으며, 범위 외 변경사항은 없습니다.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/#256-reminder_api_error_fix

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@kelly6bf kelly6bf merged commit 9c9dc38 into dev Feb 4, 2026
7 of 8 checks passed
@kelly6bf kelly6bf deleted the feature/#256-reminder_api_error_fix branch February 4, 2026 07:55
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
`@src/main/java/site/dogether/notification/sender/performance_test/PerformanceTestNotificationSender.java`:
- Around line 10-12: PerformanceTestNotificationSender is marked
`@Profile`({"pt","dev","local"}) and `@Primary` which causes it to override
production senders like FcmNotificationSender in dev/local; restrict the bean to
only the performance-test profile or remove primary selection: either change the
`@Profile` on PerformanceTestNotificationSender to `@Profile`("pt") so it only loads
in the pt profile, or keep the broader `@Profile` but remove `@Primary` so
FcmNotificationSender remains the default; update the annotation on the
PerformanceTestNotificationSender class accordingly and ensure no other bean is
accidentally marked `@Primary`.
🧹 Nitpick comments (1)
src/main/java/site/dogether/reminder/service/TodoActivityReminderService.java (1)

38-46: 명시적 분기로 향후 enum 확장 시 오동작 방지

현재 enum DailyTodoActivityReminderTypeTODO_CERTIFICATIONTODO_CERTIFICATION_REVIEW 두 값만 가지고 있어 기존 코드는 정상 작동합니다. 그러나 else 절이 모든 non-TODO_CERTIFICATION 값을 암묵적으로 리뷰 리마인더로 처리하므로, 향후 enum에 새로운 값이 추가되면 예상치 못한 동작이 발생할 수 있습니다.

switch 문으로 각 케이스를 명시적으로 처리하고 default에서 예외를 던지면 새로운 enum 값 추가 시 컴파일 타임에 처리를 강제할 수 있어 더 안전합니다.

명시적 분기 예시
-    if (reminderType == TODO_CERTIFICATION) {
-        sendTodoCertificationReminder(requester, todoId);
-        return;
-    }
-    sendTodoCertificationReviewReminder(requester, todoId);
+    switch (reminderType) {
+        case TODO_CERTIFICATION:
+            sendTodoCertificationReminder(requester, todoId);
+            break;
+        case TODO_CERTIFICATION_REVIEW:
+            sendTodoCertificationReviewReminder(requester, todoId);
+            break;
+        default:
+            throw new IllegalArgumentException("지원하지 않는 reminderType: " + reminderType);
+    }

Comment on lines 10 to 12
@Slf4j
@Profile("pt")
@Profile({"pt", "dev", "local"})
@Primary
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# Find all classes implementing NotificationSender
echo "=== NotificationSender implementations ==="
rg -n --type=java 'implements NotificationSender' -A 3 src/main/java/site/dogether/notification/sender

echo ""
echo "=== `@Profile` annotations in sender package ==="
rg -n --type=java '@Profile' -B 2 -A 1 src/main/java/site/dogether/notification/sender

echo ""
echo "=== `@Primary` annotations in sender package ==="
rg -n --type=java '@Primary' -B 3 -A 1 src/main/java/site/dogether/notification/sender

echo ""
echo "=== Directory structure of sender package ==="
find src/main/java/site/dogether/notification/sender -type f -name "*.java" | head -20

Repository: dogether-project/dogether-backend

Length of output: 2914


dev/local 환경에서 성능 테스트용 Sender가 실제 Sender를 덮어쓰고 있습니다

현재 설정으로는 @Profile({"pt", "dev", "local"}) + @Primary 조합이 dev/local 환경에서 PerformanceTestNotificationSender를 기본 선택 빈으로 만듭니다. FcmNotificationSender(프로필 제한 없음)가 존재해도 @Primary로 인해 성능 테스트용 Sender가 우선되며, 520ms 지연이 모든 알림 요청에 적용됩니다.

성능 테스트 목적이라면 이 빈은 "pt" 프로필에만 활성화되어야 합니다. 의도한 동작이 아니라면 다음 중 하나를 적용해주세요:

옵션 1: 성능 테스트 프로필에만 제한
-@Profile({"pt", "dev", "local"})
+@Profile("pt")
옵션 2: `@Primary` 제거
-@Primary
📝 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.

Suggested change
@Slf4j
@Profile("pt")
@Profile({"pt", "dev", "local"})
@Primary
`@Slf4j`
`@Profile`("pt")
`@Primary`
Suggested change
@Slf4j
@Profile("pt")
@Profile({"pt", "dev", "local"})
@Primary
`@Slf4j`
`@Profile`({"pt", "dev", "local"})
🤖 Prompt for AI Agents
In
`@src/main/java/site/dogether/notification/sender/performance_test/PerformanceTestNotificationSender.java`
around lines 10 - 12, PerformanceTestNotificationSender is marked
`@Profile`({"pt","dev","local"}) and `@Primary` which causes it to override
production senders like FcmNotificationSender in dev/local; restrict the bean to
only the performance-test profile or remove primary selection: either change the
`@Profile` on PerformanceTestNotificationSender to `@Profile`("pt") so it only loads
in the pt profile, or keep the broader `@Profile` but remove `@Primary` so
FcmNotificationSender remains the default; update the annotation on the
PerformanceTestNotificationSender class accordingly and ensure no other bean is
accidentally marked `@Primary`.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deploy 해당 라벨을 붙이면 배포 작업을 진행합니다. (붙이지 않으면 배포 작업이 진행되지 않고 머지만 됩니다.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BE] 재촉하기 API 에러 해결

1 participant