Skip to content

Conversation

@david-parkk
Copy link
Contributor

@david-parkk david-parkk commented Dec 16, 2025

#️⃣ 연관된 이슈

closes #358

📝작업 내용

  • Notification 추가

작업 상세 내용

상세 내용을 입력해주세요.

💬리뷰 요구사항

리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요.

Summary by CodeRabbit

새로운 기능

  • 푸시 알림에 제목과 본문 내용을 함께 전송할 수 있도록 개선되었습니다. 사용자 대상 알림과 주제 기반 알림 모두에서 더 풍부한 알림 정보를 제공할 수 있습니다.

✏️ Tip: You can customize this high-level summary in your review settings.

david-parkk and others added 30 commits December 1, 2025 07:29
- FCM 설정
- DB import
@coderabbitai
Copy link

coderabbitai bot commented Dec 16, 2025

워크스루

FCM 알림 전송 기능에서 Notification 페이로드를 추가하여 사용자 및 토픽 메시지 전송 메서드를 개선했습니다. sendToUserssendToTopic 메서드에 알림 제목과 본문을 포함하는 Notification 객체를 MulticastMessage 및 Message에 통합했습니다.

변경사항

코호트 / 파일 변경 요약
FCM 알림 페이로드 추가
src/main/java/ku_rum/backend/domain/alarm/application/FcmService.java
com.google.firebase.messaging.Notification import 추가. sendToUsers 메서드에서 MulticastMessage에 요청의 제목과 본문으로 Notification 페이로드 추가. sendToTopic 메서드에서 Message에 Notification 페이로드 추가.

추정 코드 리뷰 노력

🎯 2 (Simple) | ⏱️ ~8분

  • Notification 페이로드가 요청 데이터와 올바르게 매핑되는지 확인
  • 두 메서드 모두 일관된 방식으로 Notification 객체를 구성하는지 검증
  • FCM API 호환성 및 예외 처리 상태 확인

관련 가능성 있는 PR

  • [Fix] FCM Gradle + Function #361: FcmService.java의 FCM 전송 로직을 수정하며, 해당 PR은 sendMulticast 사용을 sendEachForMulticast로 업데이트하여 직접적으로 관련됨
  • [Fix] FCM 로깅 #359: 동일한 ku_rum.backend.domain.alarm.application.FcmService 클래스를 수정하며, SLF4J 로깅 및 향상된 예외 로깅을 sendToUsers/sendToTopic 메서드에 추가함

제안 리뷰어

  • kmw10693

🐰 알림이 날개를 달아,
제목과 본문이 함께 춤을 춘다네!
FCM의 숲을 거쳐 사용자에게 도착하고,
오류의 안개는 걷혀간다.
작은 수정이 큰 기쁨을 안겨주도다! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ❓ Inconclusive 설명이 템플릿 구조를 따르고 있으나, '작업 상세 내용' 섹션이 템플릿 플레이스홀더 그대로 작성되어 있습니다. '작업 상세 내용' 섹션에 구체적인 구현 세부사항을 추가하세요. 예: 어떤 부분에서 Notification 페이로드를 추가했는지, sendToUsers/sendToTopic 메서드의 변경사항 등을 설명해주세요.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed 제목이 변경 사항의 주요 내용인 알림 기능 추가를 명확하게 요약하고 있습니다.
Linked Issues check ✅ Passed 이슈 #358는 FCM 알림 전송 오류를 보고하고 있으며, PR 변경사항이 Notification 페이로드를 추가하여 해당 문제를 해결하려는 시도로 보입니다.
Out of Scope Changes check ✅ Passed 모든 변경사항이 FcmService의 Notification 기능 추가에 집중되어 있으며, 이슈 #358의 목표 범위 내에 있습니다.
✨ 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 fix/#358

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.

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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9986371 and 7960d2c.

📒 Files selected for processing (1)
  • src/main/java/ku_rum/backend/domain/alarm/application/FcmService.java (3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (2)
src/main/java/ku_rum/backend/domain/alarm/application/FcmService.java (2)

10-10: LGTM: 필요한 import 추가

Notification 클래스를 사용하기 위한 import가 올바르게 추가되었습니다.


75-83: 토픽 메시지에도 알림 페이로드 일관성있게 추가

Message 빌더에 notification 페이로드가 sendToUsers와 동일한 패턴으로 추가되어 일관성이 유지됩니다. 토픽 기반 알림도 이제 시스템 알림으로 자동 표시됩니다.

Comment on lines +47 to 58

MulticastMessage message = MulticastMessage.builder()
.addAllTokens(tokens)
.setNotification(
Notification.builder()
.setTitle(request.title())
.setBody(request.body())
.build()
)
.putData("title", request.title())
.putData("body", request.body())
.build();
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

fd FcmService.java -type f

Repository: KU-rum/backend

Length of output: 228


🏁 Script executed:

find . -name "FcmService.java" -type f | head -5

Repository: KU-rum/backend

Length of output: 129


🏁 Script executed:

rg "issue.*358|error.*1404" -i --max-count 20

Repository: KU-rum/backend

Length of output: 213


🏁 Script executed:

cat -n src/main/java/ku_rum/backend/domain/alarm/application/FcmService.java

Repository: KU-rum/backend

Length of output: 6172


🏁 Script executed:

rg "FCM_SEND_ERROR|1404" -B 5 -A 5 src/main/java/ku_rum/backend/global/support/status/BaseExceptionResponseStatus.java

Repository: KU-rum/backend

Length of output: 734


🏁 Script executed:

git log --oneline -20 -- src/main/java/ku_rum/backend/domain/alarm/application/FcmService.java

Repository: KU-rum/backend

Length of output: 139


FCM 알림 페이로드 추가로 시스템 알림 자동 표시 개선

MulticastMessage에 notification 페이로드가 올바르게 추가되었습니다. Notification.builder().setTitle().setBody().build() 패턴은 Firebase Admin SDK의 공식 권장 방식이며, 이를 통해 시스템 알림이 자동으로 표시됩니다. Data 페이로드와 notification 페이로드를 동시에 사용하는 것은 유효한 패턴입니다.

sendToTopic() 메서드(75-80행)도 동일한 패턴으로 일관성 있게 구현되어 있습니다.

🤖 Prompt for AI Agents
In src/main/java/ku_rum/backend/domain/alarm/application/FcmService.java around
lines 47 to 58, ensure the MulticastMessage includes a Notification payload so
system notifications are shown automatically; replace the current
MulticastMessage construction with one that uses
Notification.builder().setTitle(request.title()).setBody(request.body()).build(),
adds all tokens via addAllTokens(tokens), and also includes the same data keys
(putData("title", request.title()) and putData("body", request.body())) so it
matches the sendToTopic implementation and provides both notification and data
payloads.

@github-actions
Copy link

Test Results

 37 files   37 suites   10s ⏱️
150 tests 150 ✅ 0 💤 0 ❌
151 runs  151 ✅ 0 💤 0 ❌

Results for commit 7960d2c.

@david-parkk david-parkk merged commit dab4b4c into develop Dec 16, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FCM 알림 오류

3 participants