Skip to content

[refactor] fcm 리팩토링#1191

Merged
lepitaaar merged 11 commits intodevelop/befrom
refactor/#1190-fcm-code-refactoring-MOA-640
Feb 16, 2026
Merged

[refactor] fcm 리팩토링#1191
lepitaaar merged 11 commits intodevelop/befrom
refactor/#1190-fcm-code-refactoring-MOA-640

Conversation

@lepitaaar
Copy link
Contributor

@lepitaaar lepitaaar commented Feb 13, 2026

#️⃣연관된 이슈

#1190, MOA-640

📝작업 내용

기존 FCM 전송 구조는 도메인/엔티티 계층과 Firebase SDK가 직접 결합되어 있어 테스트가 어려웠고, 알림 전송 정책 제어도 불명확했습니다.
이번 PR은 결합도 축소 + 테스트 가능성 강화 + 전송 정책 명시화를 목표로 리팩토링했습니다.

기존 코드의 한계

  • Club 엔티티에서 Firebase SDK를 직접 호출해 도메인 모델이 외부 인프라에 의존
  • 상태 계산 로직과 알림 메시지 생성/전송 로직이 한 클래스에 혼재
  • D-Day 알림과 상태 변경 알림에서 payload 조립 로직 중복
  • “알림을 보낼지”를 요청 단에서 제어할 수 없음
  • 전송 여부 관련 회귀를 검증하기 어려움

주요 변경 사항

  1. FCM 전송 경계 분리
  • PushNotificationPort 도입
  • FirebasePushNotificationAdapter로 Firebase SDK 호출 캡슐화
  • PushPayload 기반으로 전송 데이터 표준화
  1. 도메인/계산 책임 정리
  • 엔티티의 직접 푸시 전송 책임 제거
  • RecruitmentStateCalculator는 상태 계산 책임만 수행
  • 상태 알림 메시지 생성은 RecruitmentStateNotificationBuilder로 분리
  1. 중복 제거
  • ClubNotificationPayloadFactory 도입으로 topic/data/path/action 조립 로직 공통화
  • D-Day/상태 알림 빌더가 공통 팩토리 사용
  1. 알림 전송 정책 명시화
  • ClubRecruitmentInfoUpdateRequestsendNotification 필드 추가
  • sendNotification 미지정(null) 시 기본값 false 적용
  • 상태가 변경되어도 sendNotification=false면 전송하지 않도록 처리
  1. 테스트 보강
  • 포트/어댑터 테스트 추가
  • 상태 계산/상태 알림 빌더/공통 페이로드 팩토리 테스트 추가
  • 서비스 레벨에서 sendNotification=true/false/null 분기 테스트 추가
  • PushPayload는 record 동등성(equals) 중심으로 검증

기존 대비 개선점

항목 기존 리팩토링 이후
계층 분리 엔티티/도메인이 Firebase SDK 직접 의존 포트/어댑터 경계로 인프라 의존 분리
책임 분리 계산/메시지 생성/전송이 섞임 계산기/빌더/전송 포트로 역할 분리
중복 알림 payload 조립 중복 공통 팩토리로 일원화
정책 제어 요청에서 전송 여부 제어 불가 sendNotification으로 명시적 제어
테스트 용이성 전송 여부 검증 어려움 전송 호출 여부/정책 분기 단위 테스트 가능

중점적으로 리뷰받고 싶은 부분

sendNotification 기본값을 false로 둔 정책(미지정 시 미전송)이 요구사항에 맞는지 확인 부탁드립니다.

논의하고 싶은 부분

현재는 “개발자(관리자) 모집정보 수정 API”에도 동일하게 sendNotification 정책을 적용했습니다.
관리자 수정 시에는 별도 기본 정책(예: 항상 false 또는 항상 true)이 필요한지 논의하고 싶습니다.

🫡 참고사항

  • 관련 테스트 실행 완료: ./gradlew unitTest

Summary by CodeRabbit

  • New Features

    • 모집 상태 변경 시 푸시 알림 발송 동작 추가
    • 모집 마감 D-day(1일, 3일, 7일) 알림 기능 추가
    • 모집 정보 업데이트 시 알림 발송 여부를 선택할 수 있는 옵션 추가
  • Tests

    • 알림 발송 관련 단위 테스트 대거 추가(상태 변경, D-day, 전송 성공/예외 등)

@lepitaaar lepitaaar self-assigned this Feb 13, 2026
@lepitaaar lepitaaar added 🔨 Refactor 코드 리팩토링 💾 BE Backend labels Feb 13, 2026
@vercel
Copy link

vercel bot commented Feb 13, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
moadong Ready Ready Preview, Comment Feb 16, 2026 6:33am

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 13, 2026

Warning

.coderabbit.yaml has a parsing error

The CodeRabbit configuration file in this repository has a parsing error and default settings were used instead. Please fix the error(s) in the configuration file. You can initialize chat with CodeRabbit to get help with the configuration file.

💥 Parsing errors (1)
Validation error: Invalid regex pattern for base branch. Received: "**" at "reviews.auto_review.base_branches[0]"
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Walkthrough

Club 엔티티의 직접 FCM 전송 메서드(sendPushNotification)를 제거하고, PushNotificationPort/PushPayload 기반 추상화를 도입했습니다. 모집 상태 계산은 void→boolean으로 변경되어 상태 변경 시 조건부 알림 흐름(빌더 → 포트 → 어댑터)을 트리거하도록 수정되었습니다.

Changes

Cohort / File(s) Summary
FCM 포트 및 모델
backend/src/main/java/moadong/fcm/port/PushNotificationPort.java, backend/src/main/java/moadong/fcm/model/PushPayload.java
푸시 전송 추상화 인터페이스(PushNotificationPort) 및 전송 페이로드 레코드(PushPayload) 추가
FCM 어댑터
backend/src/main/java/moadong/fcm/adapter/FirebasePushNotificationAdapter.java
PushNotificationPort 구현체 추가 — PushPayload를 Firebase Message로 변환해 전송하고 성공/실패를 boolean으로 반환
엔티티/DTO 변경
backend/src/main/java/moadong/club/entity/Club.java, backend/src/main/java/moadong/club/payload/request/ClubRecruitmentInfoUpdateRequest.java
Club.sendPushNotification(Message) 제거; ClubRecruitmentInfoUpdateRequestsendNotification 필드 및 shouldSendNotification() 추가
알림 팩토리/빌더
backend/src/main/java/moadong/club/util/ClubNotificationPayloadFactory.java, backend/src/main/java/moadong/club/util/RecruitmentStateNotificationBuilder.java, backend/src/main/java/moadong/club/util/RecruitmentDdayNotificationBuilder.java
Message 기반 생성 로직을 PushPayload + payloadFactory로 리팩토링; 알림 본문 생성 책임 분리
상태 계산기 변경
backend/src/main/java/moadong/club/util/RecruitmentStateCalculator.java
calculate 반환형 void→boolean(상태 변경 여부 반환)으로 변경; 기존의 알림 생성/전송 관련 메서드(및 FCM 의존성) 제거
서비스/노티파이어 변경
backend/src/main/java/moadong/club/service/ClubProfileService.java, .../RecruitmentDdayNotifier.java, .../RecruitmentStateChecker.java
PushNotificationPort 주입으로 club.sendPushNotification(...) 호출 제거; 상태 변경 시 RecruitmentStateNotificationBuilderPushPayload 생성 후 포트로 전송
테스트 및 픽스처
backend/src/test/.../ClubProfileServiceDateTest.java, .../RecruitmentDdayNotifierTest.java, .../RecruitmentStateCheckerTest.java, .../ClubNotificationPayloadFactoryTest.java, .../RecruitmentStateCalculatorTest.java, .../RecruitmentStateNotificationBuilderTest.java, .../FirebasePushNotificationAdapterTest.java, .../fixture/ClubRequestFixture.java
포트/빌더 모킹 추가, sendNotification 플래그 관련 테스트 추가/수정, 새 컴포넌트(Firebase adapter, payload factory, builders)에 대한 단위 테스트 추가

Sequence Diagram

sequenceDiagram
    participant Client
    participant Service as ClubProfileService
    participant Calculator as RecruitmentStateCalculator
    participant Builder as RecruitmentStateNotificationBuilder
    participant Port as PushNotificationPort
    participant Adapter as FirebasePushNotificationAdapter
    participant Firebase as Firebase Cloud Messaging

    Client->>Service: updateClubRecruitmentInfo(request)
    Service->>Calculator: calculate(club, start, end)
    Calculator-->>Service: boolean changed

    alt changed && request.shouldSendNotification()
        Service->>Builder: build(club, status)
        Builder-->>Service: PushPayload
        Service->>Port: send(PushPayload)
        Port->>Adapter: send(PushPayload)
        Adapter->>Firebase: send(message)
        Firebase-->>Adapter: messageId
        Adapter-->>Port: true
        Port-->>Service: true
    else no change or no notification
        Service-->>Client: no notification
    end

    Service->>Service: save club & commit
    Service-->>Client: updated club
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

✅ Test

Suggested reviewers

  • Zepelown
  • seongwon030
🚥 Pre-merge checks | ✅ 4 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection ⚠️ Warning ⚠️ Unable to check for merge conflicts: Invalid branch name format
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title "[refactor] fcm 리팩토링" directly reflects the main objective of refactoring FCM-related code structure and dependencies.
Linked Issues check ✅ Passed All code changes align with MOA-640 objectives: PushNotificationPort 및 FirebasePushNotificationAdapter 도입으로 Firebase 캡슐화 [✓], PushPayload 표준화 [✓], 알림 빌더 및 팩토리 분리 [✓], sendNotification 정책 명시화 [✓], 단위 테스트 추가 [✓].
Out of Scope Changes check ✅ Passed All changes are directly related to FCM 리팩토링: 책임 분리(Club 엔티티에서 pushNotification 메서드 제거), 포트/어댑터 도입, 빌더/팩토리 추가, 테스트 작성 모두 리팩토링 범위 내.

✏️ 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 refactor/#1190-fcm-code-refactoring-MOA-640
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch refactor/#1190-fcm-code-refactoring-MOA-640
  • Create stacked PR with resolved conflicts
  • Post resolved changes as copyable diffs in a comment

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.

@lepitaaar lepitaaar requested a review from Zepelown February 13, 2026 06:36
@github-actions
Copy link

github-actions bot commented Feb 13, 2026

Test Results

86 tests   86 ✅  16s ⏱️
19 suites   0 💤
19 files     0 ❌

Results for commit 37b6765.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@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: 5

🤖 Fix all issues with AI agents
In `@backend/src/main/java/moadong/club/service/ClubProfileService.java`:
- Around line 60-67: The notification is being sent before the entity is
persisted: move the pushNotificationPort.send(...) call to after the successful
persistence (after clubRepository.save(...) completes) so notifications reflect
committed state; locate the code using pushNotificationPort.send and
recruitmentStateNotificationBuilder.build and ensure clubRepository.save (or the
method that persists the Club) runs and returns without exception before
invoking send, or alternatively emit a domain event after save and handle
notification dispatch in an event listener to guarantee eventual consistency.

In `@backend/src/main/java/moadong/club/service/RecruitmentDdayNotifier.java`:
- Around line 57-61: The code in RecruitmentDdayNotifier is ignoring the boolean
return from pushNotificationPort.send(), causing sentCount to increment even on
failures; update the block that calls
pushNotificationPort.send(notificationBuilder.build(club, daysLeft)) to capture
its boolean result, only increment sentCount when the result is true, and add a
log.warn or log.error when it returns false (include club.getId(),
club.getName(), and daysLeft in the failure log) so failures are visible for the
scheduler.

In `@backend/src/main/java/moadong/club/util/RecruitmentStateCalculator.java`:
- Around line 14-19: The method calculate in RecruitmentStateCalculator should
treat a null previous status as a special case: change the final return to
explicitly check for null so it only reports a change when oldStatus is non-null
and different (i.e., return oldStatus != null && oldStatus != newStatus); locate
the previous status via
club.getClubRecruitmentInformation().getClubRecruitmentStatus() and ensure
club.updateRecruitmentStatus(newStatus) behavior remains, and add/adjust the
RecruitmentStateCalculatorTest to cover the null-oldStatus scenario if needed.

In
`@backend/src/main/java/moadong/club/util/RecruitmentStateNotificationBuilder.java`:
- Around line 24-33: The build method in RecruitmentStateNotificationBuilder can
NPE when ClubRecruitmentInformation.getRecruitmentEnd() or getRecruitmentStart()
is null for OPEN/UPCOMING states; update build(Club, ClubRecruitmentStatus) to
defensively check the relevant dates retrieved via
club.getClubRecruitmentInformation().getRecruitmentEnd() and
getRecruitmentStart(), and if null use a safe fallback message (e.g., a generic
prompt like "모집 기간이 곧 공지됩니다. 확인해주세요!") instead of formatting the date, then call
payloadFactory.create(club, body) as before; ensure you only format when the
date is non-null to avoid NullPointerException.

In
`@backend/src/main/java/moadong/fcm/adapter/FirebasePushNotificationAdapter.java`:
- Around line 20-37: In FirebasePushNotificationAdapter.send, the
Message.builder() chain (creating the Message from payload) is outside the
try-catch so NPEs during message construction are not caught; move the entire
message creation into the try block (i.e., wrap Message.builder() ... .build()
together with firebaseMessaging.send(message) inside the same try) so any
exceptions from Message.builder() or payload fields are handled by the existing
catch in send; retain the existing log/error behavior in the catch.
🧹 Nitpick comments (7)
backend/src/test/java/moadong/fixture/ClubRequestFixture.java (1)

40-66: 주석 처리된 코드 정리를 고려해 주세요.

TODO 주석과 함께 주석 처리된 createValidRequest()createCustomRequest() 메서드가 남아 있습니다. 현재 PR의 범위 밖이긴 하지만, 장기적으로 사용하지 않는 코드는 제거하고 필요 시 git history에서 복원하는 것이 좋습니다.

backend/src/main/java/moadong/club/service/RecruitmentStateChecker.java (1)

42-52: 알림 전송과 저장 순서에 따른 중복 전송 가능성.

pushNotificationPort.send()clubRepository.save() 이전에 호출됩니다. save 실패 시 상태가 미반영되어 다음 스케줄링에서 동일 알림이 재전송될 수 있습니다. 또한 send()boolean 반환값이 여기서도 무시되고 있습니다.

현재 규모에서는 심각한 문제는 아니지만, 향후 고려할 사항으로 남겨둡니다.

backend/src/test/java/moadong/club/service/RecruitmentStateCheckerTest.java (2)

140-160: build() 호출 시 상태값 검증이 누락되어 있습니다.

Line 154에서 any()를 사용하여 build의 status 인자를 검증하지 않고 있습니다. 실제 RecruitmentStateCheckercalculate() 이후 변경된 상태를 build()에 전달하므로, 정확한 상태값(예: ClubRecruitmentStatus.OPEN)을 검증해야 테스트의 의미가 명확해집니다.

🛠️ 제안
-        when(recruitmentStateNotificationBuilder.build(eq(club), any())).thenReturn(payload);
+        when(info.getClubRecruitmentStatus())
+                .thenReturn(ClubRecruitmentStatus.CLOSED)  // before calculate
+                .thenReturn(ClubRecruitmentStatus.OPEN);    // after calculate
+        when(recruitmentStateNotificationBuilder.build(eq(club), eq(ClubRecruitmentStatus.OPEN))).thenReturn(payload);

63-101: 기존 테스트에서 알림 미전송 검증 추가를 권장합니다.

calculate()가 기본적으로 false를 반환하므로 알림이 전송되지 않아야 하지만, 이를 명시적으로 검증하지 않고 있습니다. verify(pushNotificationPort, never()).send(any())를 추가하면 의도가 더 명확해집니다.

backend/src/test/java/moadong/club/service/ClubProfileServiceDateTest.java (1)

87-103: 테스트 간 중복 setup을 헬퍼로 추출하면 가독성이 좋아집니다.

3개의 새 테스트에서 ClubRecruitmentInfoUpdateRequest 생성, UserFixture.createUserDetails, Club 생성, clubRepository stubbing이 반복됩니다. 테스트 가독성과 유지보수를 위해 공통 부분을 private 헬퍼 메서드로 추출하는 것을 고려해 보세요.

backend/src/main/java/moadong/club/service/ClubProfileService.java (1)

55-67: 알림 전송 로직이 두 메서드에서 중복됩니다.

updateClubRecruitmentInfo(Line 55-67)와 updateClubRecruitmentInfoByClubId(Line 113-125)의 알림 전송 블록이 동일합니다. private 메서드로 추출하면 향후 알림 로직 변경 시 한 곳만 수정하면 됩니다.

♻️ 알림 전송 로직 추출 제안
+    private void sendNotificationIfNeeded(Club club, ClubRecruitmentInfoUpdateRequest request) {
+        boolean changed = recruitmentStateCalculator.calculate(
+                club,
+                club.getClubRecruitmentInformation().getRecruitmentStart(),
+                club.getClubRecruitmentInformation().getRecruitmentEnd()
+        );
+        if (changed && request.shouldSendNotification()) {
+            pushNotificationPort.send(
+                    recruitmentStateNotificationBuilder.build(
+                            club,
+                            club.getClubRecruitmentInformation().getClubRecruitmentStatus()
+                    )
+            );
+        }
+    }

두 메서드에서 중복 블록을 sendNotificationIfNeeded(club, request)로 대체할 수 있습니다.

Also applies to: 113-125

backend/src/main/java/moadong/club/util/RecruitmentStateCalculator.java (1)

36-40: Line 36의 조건이 중복됩니다 (기존 코드).

Line 29에서 now.isBefore(recruitmentStartDate)true인 경우를 이미 처리하고 반환하므로, Line 36에 도달한 시점에서 !now.isBefore(recruitmentStartDate)는 항상 true입니다. 간소화하면 가독성이 향상됩니다.

♻️ 조건 간소화 제안
-        if (!now.isBefore(recruitmentStartDate) && now.isBefore(recruitmentEndDate)) {
+        if (now.isBefore(recruitmentEndDate)) {

Comment on lines 57 to 61
if (NOTIFICATION_DAYS.contains(daysLeft)) {
log.info("D-Day 알림 전송 - clubId: {}, clubName: {}, D-{}", club.getId(), club.getName(), daysLeft);
club.sendPushNotification(notificationBuilder.build(club, daysLeft));
pushNotificationPort.send(notificationBuilder.build(club, daysLeft));
sentCount++;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

send() 반환값이 무시되고 있습니다.

pushNotificationPort.send()boolean을 반환하지만 결과를 확인하지 않습니다. 전송 실패 시에도 sentCount가 증가하여 로그의 전송 건수가 부정확해집니다. 스케줄러 특성상 재시도가 없으므로 최소한 실패 로깅이 필요합니다.

🛠️ 제안
             if (NOTIFICATION_DAYS.contains(daysLeft)) {
                 log.info("D-Day 알림 전송 - clubId: {}, clubName: {}, D-{}", club.getId(), club.getName(), daysLeft);
-                pushNotificationPort.send(notificationBuilder.build(club, daysLeft));
-                sentCount++;
+                boolean sent = pushNotificationPort.send(notificationBuilder.build(club, daysLeft));
+                if (sent) {
+                    sentCount++;
+                } else {
+                    log.warn("D-Day 알림 전송 실패 - clubId: {}, clubName: {}", club.getId(), club.getName());
+                }
             }
🤖 Prompt for AI Agents
In `@backend/src/main/java/moadong/club/service/RecruitmentDdayNotifier.java`
around lines 57 - 61, The code in RecruitmentDdayNotifier is ignoring the
boolean return from pushNotificationPort.send(), causing sentCount to increment
even on failures; update the block that calls
pushNotificationPort.send(notificationBuilder.build(club, daysLeft)) to capture
its boolean result, only increment sentCount when the result is true, and add a
log.warn or log.error when it returns false (include club.getId(),
club.getName(), and daysLeft in the failure log) so failures are visible for the
scheduler.

Comment on lines +14 to +19
public boolean calculate(Club club, ZonedDateTime recruitmentStartDate, ZonedDateTime recruitmentEndDate) {
ClubRecruitmentStatus oldStatus = club.getClubRecruitmentInformation().getClubRecruitmentStatus();
ClubRecruitmentStatus newStatus = calculateRecruitmentStatus(recruitmentStartDate, recruitmentEndDate);
club.updateRecruitmentStatus(newStatus);

if (oldStatus == newStatus)
return;

Message message = buildRecruitmentMessage(club, newStatus);
club.sendPushNotification(message);
return oldStatus != newStatus;
Copy link
Contributor

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:

#!/bin/bash
# Find ClubRecruitmentInformation entity
echo "=== Searching for ClubRecruitmentInformation entity ==="
fd "ClubRecruitmentInformation.java" -type f --exec cat {} \;

echo ""
echo "=== Searching for Club entity ==="
fd "Club.java" -type f | head -5 | xargs -I {} sh -c 'echo "File: {}"; cat "{}"'

Repository: Moadong/moadong

Length of output: 508


🏁 Script executed:

#!/bin/bash
# Find ClubRecruitmentInformation entity
echo "=== Searching for ClubRecruitmentInformation.java ==="
find backend -name "ClubRecruitmentInformation.java" -exec cat {} \;

echo ""
echo "=== Searching for Club.java ==="
find backend -name "Club.java" -exec cat {} \;

echo ""
echo "=== Searching for clubRecruitmentStatus field references ==="
rg -n "clubRecruitmentStatus" backend --type java -C2

Repository: Moadong/moadong

Length of output: 14004


🏁 Script executed:

#!/bin/bash
echo "=== RecruitmentStateCalculatorTest content ==="
cat backend/src/test/java/moadong/club/util/RecruitmentStateCalculatorTest.java

Repository: Moadong/moadong

Length of output: 2501


oldStatus가 null일 수 있으므로 엣지 케이스 처리 필요

ClubRecruitmentInformationclubRecruitmentStatus 필드는 @NotNull 검증 어노테이션만 있고, 생성자에서 기본값이 설정되지 않습니다. Club의 생성자들이 ClubRecruitmentInformation.builder().build()로 초기화하므로, clubRecruitmentStatus는 null로 남을 수 있습니다.

현재 테스트(RecruitmentStateCalculatorTest)는 명시적으로 상태를 설정한 경우만 검증하므로, oldStatus가 null인 시나리오를 놓치고 있습니다. 이 경우 oldStatus != newStatus는 항상 true를 반환하여 최초 상태 설정 시에도 "변경됨"으로 간주됩니다.

다음 중 하나를 수행하세요:

  1. 명시적 null 확인 추가: oldStatus != null && oldStatus != newStatus (의도가 초기 상태만 제외하려는 경우)
  2. null 케이스에 대한 단위 테스트 추가 (의도가 null도 변경으로 처리하려는 경우)
  3. 동작 의도를 문서화
🤖 Prompt for AI Agents
In `@backend/src/main/java/moadong/club/util/RecruitmentStateCalculator.java`
around lines 14 - 19, The method calculate in RecruitmentStateCalculator should
treat a null previous status as a special case: change the final return to
explicitly check for null so it only reports a change when oldStatus is non-null
and different (i.e., return oldStatus != null && oldStatus != newStatus); locate
the previous status via
club.getClubRecruitmentInformation().getClubRecruitmentStatus() and ensure
club.updateRecruitmentStatus(newStatus) behavior remains, and add/adjust the
RecruitmentStateCalculatorTest to cover the null-oldStatus scenario if needed.

Comment on lines 20 to 37
public boolean send(PushPayload payload) {
Message message = Message.builder()
.setNotification(Notification.builder()
.setTitle(payload.title())
.setBody(payload.body())
.build())
.putAllData(payload.data())
.setTopic(payload.topic())
.build();

try {
String messageId = firebaseMessaging.send(message);
log.info("FCM send success - topic: {}, messageId: {}", payload.topic(), messageId);
return true;
} catch (Exception e) {
log.error("FCM send failed - topic: {}, error: {}", payload.topic(), e.getMessage());
return false;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Message 생성이 try-catch 블록 밖에 있어 예외가 누락될 수 있습니다.

Message.builder() 체인(Line 21-28)에서 payload의 필드가 null이면 NullPointerException이 발생하지만, try-catchfirebaseMessaging.send() 호출만 감싸고 있습니다. Message 생성도 try 블록 안으로 이동하는 것이 안전합니다.

🐛 try-catch 범위 확장 제안
     `@Override`
     public boolean send(PushPayload payload) {
+        try {
         Message message = Message.builder()
                 .setNotification(Notification.builder()
                         .setTitle(payload.title())
                         .setBody(payload.body())
                         .build())
                 .putAllData(payload.data())
                 .setTopic(payload.topic())
                 .build();
 
-        try {
             String messageId = firebaseMessaging.send(message);
             log.info("FCM send success - topic: {}, messageId: {}", payload.topic(), messageId);
             return true;
         } catch (Exception e) {
             log.error("FCM send failed - topic: {}, error: {}", payload.topic(), e.getMessage());
             return false;
         }
     }
🤖 Prompt for AI Agents
In
`@backend/src/main/java/moadong/fcm/adapter/FirebasePushNotificationAdapter.java`
around lines 20 - 37, In FirebasePushNotificationAdapter.send, the
Message.builder() chain (creating the Message from payload) is outside the
try-catch so NPEs during message construction are not caught; move the entire
message creation into the try block (i.e., wrap Message.builder() ... .build()
together with firebaseMessaging.send(message) inside the same try) so any
exceptions from Message.builder() or payload fields are handled by the existing
catch in send; retain the existing log/error behavior in the catch.

Copy link
Contributor

@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.

🧹 Nitpick comments (2)
backend/src/main/java/moadong/club/service/ClubProfileService.java (1)

50-72: 두 메서드 간 알림 전송 로직 중복

updateClubRecruitmentInfoupdateClubRecruitmentInfoByClubId의 상태 계산 → 저장 → 조건부 알림 전송 흐름이 거의 동일합니다. 공통 private 메서드로 추출하면 향후 유지보수가 수월해집니다.

Also applies to: 107-130

backend/src/test/java/moadong/club/service/ClubProfileServiceDateTest.java (1)

128-150: changed=false일 때 알림 미전송 케이스 테스트 누락

현재 changed=true && sendNotification=true만 검증하고 있습니다. changed=false && sendNotification=true일 때 알림이 전송되지 않는지 명시적으로 검증하는 테스트를 추가하면 상태 변경 조건 분기의 커버리지가 완성됩니다.

💡 테스트 추가 예시
`@DisplayName`("모집 상태 변경이 없으면 sendNotification=true여도 알림을 보내지 않는다")
`@Test`
void 모집글_수정시_상태변경_없으면_알림을_보내지_않는다() {
    ClubRecruitmentInfoUpdateRequest request = new ClubRecruitmentInfoUpdateRequest(
            Instant.now(),
            Instant.now().plusSeconds(3600),
            "테스트 대상",
            "https://fake-url.com",
            true
    );
    CustomUserDetails customUserDetails = UserFixture.createUserDetails("test");
    Club club = new Club();
    when(clubRepository.findClubByUserId(any())).thenReturn(Optional.of(club));
    when(recruitmentStateCalculator.calculate(any(), any(), any())).thenReturn(false);

    clubProfileService.updateClubRecruitmentInfo(request, customUserDetails);

    verify(pushNotificationPort, never()).send(any());
}

@lepitaaar lepitaaar merged commit ee55ce5 into develop/be Feb 16, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

💾 BE Backend 🔨 Refactor 코드 리팩토링

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants