Conversation
Walkthrough관리자용 학사일정 테스트 알림 기능이 추가되었습니다. 새 API 엔드포인트, 요청 DTO/명령/이벤트 타입, 포트·서비스 확장, 이벤트 발행·리스너, Firebase 전송 경로 및 응답 코드가 포함됩니다. Changes
Sequence DiagramsequenceDiagram
participant Admin as Admin Client
participant API as AdminCommandApiV2
participant UseCase as AdminCommandUseCase
participant Service as AdminCommandService
participant EventPort as AdminEventPort
participant Adapter as AdminFirebaseMessageAdapter
participant EventBus as Spring EventBus
participant Listener as MessageAdminEventListener
participant FirebaseSvc as FirebaseNotificationService
participant FCM as Firebase Cloud Messaging
Admin->>API: POST /api/v2/admin/academic/dev (title, body)
API->>API: request.toCommand()
API->>UseCase: createAcademicTestNotification(command)
UseCase->>Service: createAcademicTestNotification(command)
Service->>EventPort: sendAcademicTestNotification(title, body)
EventPort->>Adapter: sendAcademicTestNotification(title, body)
Adapter->>EventBus: publish AcademicTestNotificationEvent
EventBus->>Listener: `@EventListener` (async) handle event
Listener->>FirebaseSvc: sendAcademicTestNotification(command)
FirebaseSvc->>FCM: build Message (title, body, messageType=academic) and send
FCM-->>FirebaseSvc: send result
FirebaseSvc-->>Listener: result
Listener-->>API: processing complete
API-->>Admin: 200 OK (ADMIN_TEST_ACADEMIC_CREATE_SUCCESS)
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
jiyun921
added
🐛 Bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
src/main/java/com/kustacks/kuring/calendar/application/service/AcademicEventNotificationService.java (1)
93-103: FCM data payload에messageType추가는 의도에 잘 맞습니다.실제 학사일정 알림에
"messageType": "academic"을 넣는 방향은 클라이언트 측 분류용으로 적절해 보입니다. 다만 이후 다른 FCM 발송 지점에서도 동일한 key/value를 사용할 가능성이 크다면, 상수나 enum으로 추출해 두면 오타 방지와 변경 용이성 측면에서 조금 더 안전할 것 같습니다.src/main/java/com/kustacks/kuring/admin/application/port/out/AdminEventPort.java (1)
5-18: 학사 테스트 알림용 이벤트 메서드 추가는 적절하지만, 예외 처리 정책은 한 번 정리해 볼 만합니다.
sendTestNotificationByAdmin은FirebaseMessageSendException을 던지는 반면,sendAcademicTestNotification은 예외를 전파하지 않는 형태라 두 테스트용 경로의 오류 처리 방식이 달라졌습니다.
학사 테스트 알림 실패도 상위 계층에서 감지/로그가 필요하다면, 동일하게 예외를 던지거나(throws 추가) 결과 객체를 반환하는 식으로 정책을 맞춰 두는 것도 고려해 볼 수 있습니다.src/main/java/com/kustacks/kuring/message/application/port/in/dto/AcademicTestNotificationCommand.java (1)
3-7: 입력 유효성 검증이 없습니다.
title과body필드에@NotBlank등의 유효성 검증 어노테이션이 없습니다. 빈 문자열이나 null 값이 전달될 경우 FCM 전송 시 문제가 발생할 수 있습니다.+import jakarta.validation.constraints.NotBlank; + public record AcademicTestNotificationCommand( - String title, - String body + @NotBlank String title, + @NotBlank String body ) { }src/main/java/com/kustacks/kuring/admin/adapter/in/web/dto/AcademicTestNotificationRequest.java (1)
5-11: Request DTO에 유효성 검증 어노테이션 추가를 권장합니다.API 요청 DTO에
@NotBlank등의 유효성 검증 어노테이션이 없어, 빈 값이 들어올 경우 컨트롤러 레벨에서 검증 없이 서비스까지 전달됩니다.+import jakarta.validation.constraints.NotBlank; + public record AcademicTestNotificationRequest( - String title, - String body + @NotBlank String title, + @NotBlank String body ) { public AcademicTestNotificationCommand toCommand() { return new AcademicTestNotificationCommand(title, body); } }src/main/java/com/kustacks/kuring/message/application/service/FirebaseNotificationService.java (1)
67-87: 메시지 생성 로직의 중복을 고려해 볼 수 있습니다.기능적으로 올바르게 구현되어 있습니다. 다만
createMessageFromCommand와 유사한 메시지 빌딩 패턴이 인라인으로 작성되어 있어, 향후 유지보수를 위해 별도 메서드로 추출하는 것을 고려해 볼 수 있습니다.또한
addDevSuffix를 사용하여 항상 dev 접미사를 추가하는데, 이는 테스트 알림 용도로 의도된 것으로 보입니다.src/main/java/com/kustacks/kuring/admin/adapter/in/web/AdminCommandApiV2.java (1)
80-89: 테스트 학사일정 알림 엔드포인트가 올바르게 구현되었습니다.구현이 기존 테스트 알림 엔드포인트(
createTestNotice)와 일관된 패턴을 따르고 있으며, ROOT 권한으로 적절히 보호되고 있습니다.선택적 개선사항:
요청 본문에
@Valid어노테이션을 추가하고AcademicTestNotificationRequest의 필드에 검증 제약(예:@NotBlank,@NotNull)을 추가하여 입력 값 검증을 강화하는 것을 고려해보세요. 다만 이는 파일 내 다른 엔드포인트들과의 일관성을 고려하여 결정하시기 바랍니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/main/java/com/kustacks/kuring/admin/adapter/in/web/AdminCommandApiV2.java(3 hunks)src/main/java/com/kustacks/kuring/admin/adapter/in/web/dto/AcademicTestNotificationRequest.java(1 hunks)src/main/java/com/kustacks/kuring/admin/adapter/out/event/AdminFirebaseMessageAdapter.java(2 hunks)src/main/java/com/kustacks/kuring/admin/application/port/in/AdminCommandUseCase.java(2 hunks)src/main/java/com/kustacks/kuring/admin/application/port/out/AdminEventPort.java(1 hunks)src/main/java/com/kustacks/kuring/admin/application/service/AdminCommandService.java(2 hunks)src/main/java/com/kustacks/kuring/calendar/application/service/AcademicEventNotificationService.java(1 hunks)src/main/java/com/kustacks/kuring/common/dto/ResponseCodeAndMessages.java(2 hunks)src/main/java/com/kustacks/kuring/message/adapter/in/event/MessageAdminEventListener.java(2 hunks)src/main/java/com/kustacks/kuring/message/adapter/in/event/dto/AcademicTestNotificationEvent.java(1 hunks)src/main/java/com/kustacks/kuring/message/application/port/in/FirebaseWithAdminUseCase.java(1 hunks)src/main/java/com/kustacks/kuring/message/application/port/in/dto/AcademicTestNotificationCommand.java(1 hunks)src/main/java/com/kustacks/kuring/message/application/service/FirebaseNotificationService.java(5 hunks)src/test/java/com/kustacks/kuring/acceptance/AdminAcceptanceTest.java(2 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 and analyze
🔇 Additional comments (10)
src/main/java/com/kustacks/kuring/common/dto/ResponseCodeAndMessages.java (1)
26-57: 테스트 학사일정 알림용 응답 코드 추가가 일관성 있게 잘 정의되었습니다.다른 Admin 응답 상수들과 형식(HTTP 200, 한국어 메시지)이 동일하고, 인수 테스트에서 검증하는 메시지 문자열과도 일치해 보여서 사용 측에서 혼동 없이 사용할 수 있을 것 같습니다.
src/main/java/com/kustacks/kuring/message/adapter/in/event/dto/AcademicTestNotificationEvent.java (1)
1-12: 이벤트 DTO와 커맨드 간 매핑 구조가 단순·명확합니다.
record로 이벤트를 정의하고toCommand()에서 그대로 위임하는 방식이라 가독성이 좋습니다. 추후 title/body 외 필드가 추가될 경우AcademicTestNotificationCommand와 이 메서드를 함께 업데이트해 주기만 하면 될 것 같습니다.src/test/java/com/kustacks/kuring/acceptance/AdminAcceptanceTest.java (1)
3-36: 테스트 학사일정 알림 인수 테스트가 흐름을 잘 검증하고 있습니다.루트 관리자 로그인 →
/api/v2/admin/academic/dev호출 → 상태코드,code,message,data까지 검증하는 시나리오가 기존 공지/알림 인수 테스트들과 스타일도 일관되고, 새 응답 코드("테스트 학사일정 알림 생성에 성공하였습니다")와 정확히 맞물려 있어 회귀 방지에 도움이 될 것 같습니다.Also applies to: 154-179
src/main/java/com/kustacks/kuring/message/application/port/in/FirebaseWithAdminUseCase.java (1)
3-13: Admin용 Firebase 유즈케이스에 학사 테스트 알림용 메서드가 자연스럽게 확장되었습니다.일반 공지, 테스트 공지에 이어 학사 테스트 알림도 별도 커맨드 타입으로 분리된 설계가 명확합니다. 이 인터페이스를 구현하는 클래스들(FirebaseNotificationService 등)에 신규 메서드 구현이 모두 추가되어 있는지만 한 번 더 확인해 두면 좋겠습니다.
src/main/java/com/kustacks/kuring/admin/application/port/in/AdminCommandUseCase.java (1)
3-24: Admin 유즈케이스에 학사 테스트 알림 생성이 명시적으로 추가되어 역할이 명확합니다.공지(실제/테스트), 알림 스케줄링 등 기존 기능 옆에 학사 테스트 알림 생성이 별도 메서드로 정의되어 있어서, 상위(Controller)에서 의도를 드러내며 호출하기 좋습니다. 구현체와 API 레이어에서 이 메서드를 일관되게 사용하고 있으니 현재 형태로 충분해 보입니다.
src/main/java/com/kustacks/kuring/admin/application/service/AdminCommandService.java (1)
15-16: 학사 테스트 알림 생성 서비스 구현이 포트 위임 패턴과 잘 맞습니다.
createAcademicTestNotification이AcademicTestNotificationCommand에서 title/body만 추출해AdminEventPort로 위임하는 구조는 기존 테스트 공지/실제 공지 흐름과도 자연스럽게 일관됩니다.
추가 검증 로직(예: 비밀번호 확인)이 필요 없다면 현재 구현으로 충분해 보입니다.Also applies to: 86-92
src/main/java/com/kustacks/kuring/message/adapter/in/event/MessageAdminEventListener.java (1)
42-48: 기존 패턴과 일관성 있는 구현입니다.
sendTestNotificationEvent와 동일한@Async+@EventListener패턴을 따르고 있어 일관성이 유지됩니다.src/main/java/com/kustacks/kuring/admin/adapter/out/event/AdminFirebaseMessageAdapter.java (1)
42-45: 기존 이벤트 발행 패턴을 따르는 구현입니다.
sendNotificationByAdmin메서드와 동일한Events.raise()패턴을 사용하여 일관성을 유지하고 있습니다.src/main/java/com/kustacks/kuring/message/application/service/FirebaseNotificationService.java (1)
155-158: FCM 메시지 분류를 위한 messageType 필드 추가 - LGTM!PR 제목에 명시된 대로
messageType필드가 admin, notice, academic 메시지에 각각 추가되어 클라이언트 측에서 메시지 유형을 분류할 수 있게 되었습니다.src/main/java/com/kustacks/kuring/admin/adapter/in/web/AdminCommandApiV2.java (1)
4-4: LGTM!새로운 엔드포인트에 필요한 임포트가 올바르게 추가되었습니다.
Also applies to: 42-42
zbqmgldjfh
left a comment
zbqmgldjfh
left a comment
There was a problem hiding this comment.
한부분만 리뷰 남겨두었습니다.
잘못됬다가 아니라! 같의 논의해보 는 느낌으로 남겨두었습니다!
장단점? 이랄까?
| } | ||
|
|
||
| @Override | ||
| public void sendAcademicTestNotification(AcademicTestNotificationCommand command) { |
There was a problem hiding this comment.
근데, 이걸 test용도로 만들어다는거는? 향후 Prod 목적으로도 동일한 메서드가 추가되는건가요? 단일 메서드를 두고 환경(머신 자체의 linux 환경값에)에 따라 다르게 전달하는건 어떤가요?
There was a problem hiding this comment.
학사일정 알림이 원래는 스케줄러에서 자동으로 전송돼서 prod용으로 수동 전송하는 api를 따로 추가안할 것 같습니다! 이건 test용도로 수동으로 알림 전송해봐서 확인해보려고 만들었습니다!
There was a problem hiding this comment.
엇 근데 지금 공지 알림 전송은 test용이랑 prod용 메서드가 따로 존재하네요! 단일 메서드로 하면 코드가 깔끔해질 것 같긴 한데 실수로 prod로 알림이 전송되는 일이 있을 수도 있어서.. 어떤게 더 좋을까여??
There was a problem hiding this comment.
메서드에서 test를 명시해주신 지금 방식이 더 주의를 할 수 있어서 장점이 충분한것 같습니다!.
지금 방식으로 가시죠~
| .setTopic(serverProperties.ifDevThenAddSuffix(ACADEMIC_EVENT_TOPIC)) | ||
| .putData("title", title) | ||
| .putData("body", body) | ||
| .putData("messageType", "academic") |
There was a problem hiding this comment.
소나큐브 이슈에도 비슷한 내용이 있는데 enum화 해두는게 어떤지요??
There was a problem hiding this comment.
넵 enum화 하는게 훨씬 좋을 것 같아요!!
| var response = RestAssured | ||
| .given().log().all() | ||
| .header("Authorization", "Bearer " + accessToken) | ||
| .contentType(MediaType.APPLICATION_JSON_VALUE) | ||
| .accept(MediaType.APPLICATION_JSON_VALUE) | ||
| .body(new AcademicTestNotificationRequest("테스트-수강신청 일정", "오늘은 수강신청 일정이 있어요")) | ||
| .when().post("/api/v2/admin/academic/dev") | ||
| .then().log().all() | ||
| .extract(); |
There was a problem hiding this comment.
다른 곳 처럼 이 부분도 Step 객체에서 관리하도록 하는게 좋을거 같아요~
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/test/java/com/kustacks/kuring/acceptance/AdminStep.java (1)
142-155: 테스트 헬퍼 메서드가 올바르게 구현되었습니다.RestAssured를 사용한 API 호출 구조가 기존 메서드들과 일관성 있게 작성되었으며, 헤더 설정과 로깅이 적절합니다.
선택적 개선 사항: 일부 다른 API 호출 메서드들(예:
피드백_조회_확인,금칙어_로드_응답_확인등)처럼 응답 검증을 위한 별도의 assertion 헬퍼 메서드를 추가하면 테스트 코드의 일관성과 재사용성이 향상될 수 있습니다.🔎 선택적 개선안: assertion 헬퍼 메서드 추가
public static void 테스트_학사일정_알림_발송_응답_확인(ExtractableResponse<Response> response) { assertAll( () -> assertThat(response.statusCode()).isEqualTo(HttpStatus.OK.value()), () -> assertThat(response.jsonPath().getInt("code")).isEqualTo(200), () -> assertThat(response.jsonPath().getString("message")).isEqualTo("테스트 학사일정 알림 생성에 성공하였습니다"), () -> assertThat(response.jsonPath().getString("data")).isNull() ); }src/main/java/com/kustacks/kuring/message/application/service/FirebaseNotificationService.java (1)
70-90: 데이터 맵 생성을 하나로 통합하는 것을 고려해보세요.현재
Map.of()로 title/body를 추가한 후putData()로 messageType을 별도로 추가하고 있습니다. 이를 하나의Map.of()호출로 통합하면 코드가 더 간결해집니다.🔎 제안하는 리팩토링
Message newMessage = Message.builder() .setNotification(Notification.builder() .setTitle(command.title()) .setBody(command.body()) .build()) .putAllData(Map.of( "title", command.title(), - "body", command.body() + "body", command.body(), + "messageType", ACADEMIC.getValue() )) - .putData("messageType", ACADEMIC.getValue()) .setTopic(serverProperties.addDevSuffix(ACADEMIC_EVENT_TOPIC)) .build();
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/kustacks/kuring/calendar/application/service/AcademicEventNotificationService.javasrc/main/java/com/kustacks/kuring/message/application/service/FirebaseNotificationService.javasrc/main/java/com/kustacks/kuring/message/domain/MessageType.javasrc/test/java/com/kustacks/kuring/acceptance/AdminAcceptanceTest.javasrc/test/java/com/kustacks/kuring/acceptance/AdminStep.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/kustacks/kuring/calendar/application/service/AcademicEventNotificationService.java
- src/test/java/com/kustacks/kuring/acceptance/AdminAcceptanceTest.java
⏰ 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 and analyze
🔇 Additional comments (3)
src/test/java/com/kustacks/kuring/acceptance/AdminStep.java (1)
3-3: LGTM!새로운 테스트 헬퍼 메서드에 필요한 import가 올바르게 추가되었습니다.
src/main/java/com/kustacks/kuring/message/domain/MessageType.java (1)
1-15: enum 구현이 깔끔합니다.MessageType enum이 세 가지 알림 타입(notice, admin, academic)을 명확하게 정의하고 있으며, Lombok을 활용한 구현이 적절합니다. 타입 안전성을 제공하여 문자열 오타 위험을 방지합니다.
src/main/java/com/kustacks/kuring/message/application/service/FirebaseNotificationService.java (1)
158-158: messageType 필드 추가가 일관성 있게 적용되었습니다.Admin 및 Notice 메시지에 messageType 필드가 올바르게 추가되어, 클라이언트가 알림 타입을 구분할 수 있게 되었습니다. MessageType enum을 활용하여 타입 안전성도 확보했습니다.
Also applies to: 171-171
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/com/kustacks/kuring/message/application/service/FirebaseNotificationService.java (1)
70-90: 테스트 알림 전송 로직이 올바르게 구현되었습니다.
addDevSuffix()를 사용하여 항상 개발 환경 토픽으로만 전송되도록 보장한 점이 좋습니다. 실수로 프로덕션에 알림이 전송되는 것을 방지할 수 있습니다.선택사항: 일관성을 위한 헬퍼 메서드 추출 고려
현재 Message 생성 로직이 메서드 내부에 인라인으로 구현되어 있는데,
createMessageFromCommand나createMessageFromDto처럼 별도 헬퍼 메서드로 추출하면 코드 구조의 일관성이 높아집니다. 다만 테스트 전용 메서드이므로 현재 구현도 충분히 수용 가능합니다.+ private Message createAcademicTestMessage(AcademicTestNotificationCommand command) { + return Message.builder() + .setNotification(Notification.builder() + .setTitle(command.title()) + .setBody(command.body()) + .build()) + .putAllData(Map.of( + "title", command.title(), + "body", command.body(), + "messageType", ACADEMIC.getValue() + )) + .setTopic(serverProperties.addDevSuffix(ACADEMIC_EVENT_TOPIC)) + .build(); + } + @Override public void sendAcademicTestNotification(AcademicTestNotificationCommand command) { try { - Message newMessage = Message.builder() - .setNotification(Notification.builder() - .setTitle(command.title()) - .setBody(command.body()) - .build()) - .putAllData(Map.of( - "title", command.title(), - "body", command.body(), - "messageType", ACADEMIC.getValue() - )) - .setTopic(serverProperties.addDevSuffix(ACADEMIC_EVENT_TOPIC)) - .build(); - + Message newMessage = createAcademicTestMessage(command); firebaseMessagingPort.send(newMessage); } catch (FirebaseMessagingException exception) { throw new FirebaseMessageSendException(); } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/kustacks/kuring/message/application/service/FirebaseNotificationService.java
⏰ 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 and analyze
🔇 Additional comments (2)
src/main/java/com/kustacks/kuring/message/application/service/FirebaseNotificationService.java (2)
12-12: LGTM! 필요한 import가 적절히 추가되었습니다.새로운 학사일정 테스트 알림 기능에 필요한 커맨드, 토픽, 메시지 타입 상수들이 올바르게 import 되었습니다.
Also applies to: 26-26, 28-30
158-158: LGTM! 메시지 타입 분류 필드가 적절히 추가되었습니다.기존 관리자 알림과 공지 알림 전송 경로에
messageType필드를 추가하여 클라이언트가 알림 유형을 식별할 수 있도록 개선되었습니다.putAllData()이후putData()로 messageType을 추가하는 패턴이 일관되게 적용되었습니다.Also applies to: 171-171
|
|
|
||
| @Getter | ||
| @RequiredArgsConstructor | ||
| public enum MessageType { |
3 participants
#️⃣ 이슈
#321
📌 요약
🛠️ 상세
FCM payload에 messageType 필드 추가 (academic, admin, notice 총 3종류)
-> 클라이언트가 알림 유형을 정확히 식별할 수 있도록 개선
테스트 학사일정 알림 전송 API(POST /api/v2/admin/academic/dev) 구현
-> title, body 기반으로 academic 테스트 알림 발송
인수 테스트 추가
-> 테스트 학사일정 알림 생성 시 HTTP 200, code 200, message “테스트 학사일정 알림 생성에 성공하였습니다”, data = null을 검증하도록 구현
-> FakeFirebaseAdapter를 통해 테스트 환경에서 실제 FCM 대신 안전하게 검증 수행
💬 기타
-> 테스트 API는 어떤 환경에서도 반드시 테스트 앱으로만 전송되어야 하므로 addDevSuffix()를 사용해 항상 .dev 토픽으로 전송되도록 수정하여 해결
Summary by CodeRabbit
릴리스 노트
New Features
Enhancements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.