Open
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note
|
| Cohort / File(s) | Summary |
|---|---|
Event & Domain Models SurveyCompletedEvent.java, PushAlimEvent.java, PushProperty.java |
새로운 PushAlimEvent 인터페이스와 SurveyCompletedEvent 레코드를 정의하고, JPA 엔티티로 PushProperty를 추가하여 푸시 템플릿 속성을 저장합니다. |
Push Value Objects PushTemplate*.java, PushTemplateAddVO.java, PushTemplateModifyVO.java |
템플릿 추가/수정 요청과 템플릿 컨텍스트를 위한 VO 클래스들을 도입합니다. |
Survey Completion Flow ResponseCommandService.java |
설문 완료 시 ApplicationEventPublisher를 주입하고 SurveyCompletedEvent를 발행하는 로직을 추가합니다. |
Toss Push Integration TossApiClient.java, TossPushPort.java, PushTemplateSendRequest.java, PushResultResponse.java |
Toss API 클라이언트에 푸시 송신 기능을 구현하고, SSLContext를 통한 보안 통신을 지원합니다. |
Push Template Management PushTemplateAddRequest.java, PushTemplateModifyRequest.java, TossErrorCode.java |
Toss 푸시 관련 DTO와 에러 코드(TOSS_PUSH_SEND_ERROR, TOSS_PUSH_NOT_FOUND)를 정의합니다. |
Push Application Layer PushUseCase.java, PushFacade.java |
푸시 템플릿 관리 및 발송을 담당하는 use case와 facade를 구현합니다. PushFacade는 템플릿 병합, SSL 초기화, 비동기 전송을 처리합니다. |
Push Persistence PushPropertyRepository.java, PushPropertyRepositoryImpl.java, PushPropertyJpaRepository.java |
푸시 속성 데이터 접근 계층을 정의하고 구현합니다. QueryDSL과 배치 삽입을 활용합니다. |
Event Handling & Async PushEventListener.java, AsyncConfig.java |
비동기 이벤트 리스너를 추가하고 "pushAlimExecutor" 스레드 풀(core=4, max=8)을 정의합니다. |
Discord Alert Infrastructure AlertNotifier.java, DiscordAlertNotifier.java, NoOpAlertNotifier.java, DiscordAlarmAsyncFacade.java, DiscordAlarmService.java, PushAlimAlert.java |
AlertNotifier 인터페이스에 sendPushAlimAsync 메서드를 추가하고, 모든 구현체에서 구현합니다. Discord 알림 서비스에 푸시 알림 전송 로직을 추가합니다. |
Code Cleanup SurveyRepositoryImpl.java |
getSurveyDetailDataById 메서드에서 디버그 System.out.println 문을 제거합니다. |
Sequence Diagram(s)
sequenceDiagram
actor User as User
participant Survey as ResponseCommandService
participant Publisher as ApplicationEventPublisher
participant Listener as PushEventListener
participant Facade as PushFacade
participant Toss as TossApiClient
participant Discord as DiscordAlarmService
User->>Survey: 설문 완료
Survey->>Survey: 상태를 CLOSED로 설정
Survey->>Publisher: SurveyCompletedEvent 발행
Publisher-->>Listener: 이벤트 전달 (AFTER_COMMIT)
Listener->>Listener: PushCommand 생성
Listener->>Facade: fillTemplateAndSendPush(command)
Facade->>Facade: 템플릿 컨텍스트 조회
Facade->>Facade: 기본값과 제공된 값 병합
Facade->>Facade: PushTemplateSendRequest 구성
Facade->>Toss: sendPush(SSLContext, request)
Toss->>Toss: 푸시 전송
Toss-->>Facade: PushResultResponse
alt 성공
Facade->>Discord: alertNotifier.sendPushAlimAsync()
Discord->>Discord: 성공 메시지 포맷
else 실패
Discord->>Discord: 실패 메시지 포맷
end
Discord-->>User: Discord 알림
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~50 minutes
이 변경은 여러 계층(도메인, 애플리케이션, 인프라, 영속성)에 걸친 광범위한 기능 추가로, 복잡한 로직(특히 PushFacade의 템플릿 병합과 TossApiClient의 푸시 송신), 다양한 DTO/VO 도입, 이벤트 기반 흐름의 이해가 필요합니다.
Possibly related PRs
- ✨ OMF-60 설문 응답자가 목표인원을 넘지 않도록 카운트 기능 추가 #82: ResponseCommandService에서 설문 완료 로직을 수정하여 Redis 정리 작업을 처리 — 현재 PR은 이를 기반으로 SurveyCompletedEvent 발행 기능을 추가합니다.
- ✨ OMF-100 백오피스 구축 #103: SurveyRepositoryImpl의 getSurveyDetailDataById 메서드와 관련된 변경 — 현재 PR은 디버그 출력을 제거하는 정리 작업을 포함합니다.
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 6.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | PR 제목이 설문 마감 시 사용자에게 푸시알림을 전송하는 주요 변경사항을 명확하게 반영하고 있습니다. |
| Description check | ✅ Passed | PR 설명이 관련 이슈, 작업 완료 항목, 기술적 상세사항을 포함하고 있어 템플릿의 필수 섹션을 충족합니다. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feat/OMF-218
Tip
Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/OneQ/OnSurvey/domain/participation/model/event/SurveyCompletedEvent.java`:
- Around line 7-25: SurveyCompletedEvent's eventContext can be null and
getPushContext() may throw NPE; update the record to enforce null-safety by
normalizing or validating eventContext at construction (e.g., in the compact
constructor for SurveyCompletedEvent) so it is never null (use
Objects.requireNonNull(eventContext, ...) or default to Collections.emptyMap()),
and keep getPushContext() returning the non-null map; reference
SurveyCompletedEvent, the compact constructor, and getPushContext() when making
the change.
In
`@src/main/java/OneQ/OnSurvey/domain/participation/service/response/ResponseCommandService.java`:
- Line 78: The event is being published with an empty context (Map.of()),
causing missing template data and runtime TOSS_PUSH_NOT_FOUND errors; in
ResponseCommandService where eventPublisher.publishEvent(new
SurveyCompletedEvent(creator, Map.of())) is called, construct and pass a
populated context map containing the dynamic fields needed by the push template
(e.g., "surveyTitle", "completedCount", "surveyId", and any "creatorName" or
locale info) by extracting values from the current survey/response objects and
handling nulls/formatting (e.g., convert counts to strings); replace the
Map.of() with that context map so SurveyCompletedEvent receives the required
data for push_property template resolution.
In `@src/main/java/OneQ/OnSurvey/global/common/config/AsyncConfig.java`:
- Around line 25-34: The ThreadPoolTaskExecutor beans pushAlimExecutor and
discordAlarmExecutor in AsyncConfig are missing a rejection policy and graceful
shutdown settings; update both factory methods to call
exec.setRejectedExecutionHandler(new
java.util.concurrent.ThreadPoolExecutor.CallerRunsPolicy()),
exec.setWaitForTasksToCompleteOnShutdown(true), and
exec.setAwaitTerminationSeconds(<reasonable timeout, e.g., 30>) on the
ThreadPoolTaskExecutor before exec.initialize() so tasks are run on caller when
the queue is full and in-flight tasks are allowed to complete on application
shutdown.
In `@src/main/java/OneQ/OnSurvey/global/infra/discord/DiscordAlarmService.java`:
- Around line 121-131: The sendPushAlimAsync method lacks input validation: add
a null check for the PushAlimAlert parameter and validate its failedCount() to
prevent negative values from being treated as valid; specifically, in
DiscordAlarmService.sendPushAlimAsync, early-return with a logged warning if
alert is null, and treat failedCount() < 0 as invalid (log and return) while
handling failedCount() == 0 as the “no failures” path instead of treating
negatives as successes—adjust the conditional around failedCount() (and any
title/description assignment) to use >= 0/== 0/ > 0 checks as appropriate and
reference PushAlimAlert.failedCount() when making these decisions.
In `@src/main/java/OneQ/OnSurvey/global/infra/toss/client/TossApiClient.java`:
- Around line 392-405: The failure branch in TossApiClient currently uses
successRoot (defined as root.path("success")) which skews alert metrics; update
the logic so isSuccess(root) true paths use successRoot for sent/failed counts,
but the failure branch does not read successRoot — instead extract counts from
the error/failure structure (e.g., root.path("error") or root.path("fail") as
provided by the API) or default them to 0, and then pass those values into
alertNotifier.sendPushAlimAsync new PushAlimAlert(...) so PushAlimAlert receives
correct sentPushCount and failCount on errors; ensure the change is applied
where successRoot, isSuccess(root), alertNotifier.sendPushAlimAsync and
PushAlimAlert are referenced.
- Around line 394-395: In TossApiClient (the error-parsing block where you read
error fields into variables `code` and `msg`), change the mapping for `msg` to
read `error.reason` instead of `error.errorCode`; specifically update the code
that sets the `msg` variable so it uses
root.path("error").path("reason").asText("unknown") so subsequent logging uses
the actual error message rather than the error code (leave the `code` extraction
from `error.errorType` unchanged).
In
`@src/main/java/OneQ/OnSurvey/global/infra/toss/common/dto/push/PushResultResponse.java`:
- Around line 8-10: The factory method PushResultResponse.of(...) uses parameter
name sentPushContentId which doesn't match the actual field sentPushContent and
the JSON string passed from TossApiClient; rename the parameter in of(Long
sentPushCount, String sentPushContentId) to sentPushContent (or vice versa
rename the field) so names reflect the same data, and update the constructor
invocation inside of(...) to pass the renamed parameter to the
PushResultResponse(...) constructor (ensure PushResultResponse.of, its parameter
list, and any callers use the new name consistently).
In
`@src/main/java/OneQ/OnSurvey/global/infra/toss/common/dto/push/PushTemplateAddRequest.java`:
- Around line 8-23: The PushTemplateAddRequest record accepts external input but
lacks validation; add bean validation annotations to the record
components—annotate name and code with `@NotBlank` (and optionally `@Size`(max=...))
and annotate defaultContext with `@NotNull` and `@NotEmpty` plus `@Valid` to validate
its values; also constrain the map values (List<String>) with
`@NotNull/`@Size(min=1) (or a custom validator) to ensure each entry has the
expected elements and prevent empty lists or nulls, and ensure the controller
parameter is validated (e.g., `@Valid` on the request parameter) so invalid
requests are rejected before downstream logic in PushTemplateAddRequest, name,
code, defaultContext.
In
`@src/main/java/OneQ/OnSurvey/global/infra/toss/common/dto/push/PushTemplateModifyRequest.java`:
- Around line 8-20: The PushTemplateModifyRequest record's defaultContext
currently lacks validation so invalid entries (null, empty lists, lists with
size != 2, empty keys, or non-string elements) can propagate; add a canonical
constructor in PushTemplateModifyRequest that validates defaultContext: ensure
the map itself is non-null, each key is non-blank, and each value is a non-null
List with exactly 2 elements (elements may be null but must be strings if
present), otherwise throw IllegalArgumentException with a clear message; perform
any normalization you need (e.g., convert empty strings to null or trim keys)
inside that constructor so downstream code consuming PushTemplateModifyRequest
can rely on well-formed defaultContext.
In `@src/main/java/OneQ/OnSurvey/global/push/adapter/in/PushEventListener.java`:
- Around line 21-34: The async event handler
PushEventListener.handlePushAlimEvent currently calls
pushUseCase.fillTemplateAndSendPush without local error handling, so exceptions
thrown in the `@Async` thread are lost; wrap the call in a try-catch that catches
Exception, logs the failure with contextual info (event.getTargetUserKey(),
event.getPushTemplateName(), templateCtx) and the exception via the class
logger, and optionally emit a failure metric or publish a compensating event so
failures are observable and traceable instead of being silently swallowed.
In
`@src/main/java/OneQ/OnSurvey/global/push/adapter/out/persistence/PushPropertyRepositoryImpl.java`:
- Around line 48-58: In PushPropertyRepositoryImpl the code incorrectly passes
the List returned by vo.addTemplateList() into
jpaQueryFactory.insert(...).values(...), causing a type mismatch; change this to
iterate the List and perform an insert per NewPushTemplate (use
vo.addTemplateList().forEach(...)) calling
jpaQueryFactory.insert(pushProperty).columns(...).values(template.name(),
template.code(), template.contextKey(), template.contextValue(),
template.description()).execute() for each item (or use a proper batch insert
API if available), and double-check that contextValue is intentionally mapped to
pushProperty.defaultValue as noted in the review.
In `@src/main/java/OneQ/OnSurvey/global/push/application/PushFacade.java`:
- Around line 103-107: When iterating vo.modifyTemplateList(), the lookup
pushPropertyMap.get(modify.contextKey()) can return null and calling
PushProperty.updateContext(...) causes an NPE; change the loop in PushFacade to
validate the lookup result and throw a domain/validation exception (e.g.,
InvalidPushContextException) when property is null instead of calling
updateContext on null, or perform pre-validation of vo.modifyTemplateList()
entries before the forEach; reference the symbols vo.modifyTemplateList(),
modify.contextKey(), pushPropertyMap, and PushProperty.updateContext() when
locating where to add the null check and domain exception.
- Around line 42-45: init() currently calls
tossPushPort.createSSLContext(publicCrt, privateKey) without validating inputs;
add explicit fail-fast checks for publicCrt and privateKey (e.g., null/empty)
before calling createSSLContext and throw a clear, descriptive exception (or
IllegalStateException) indicating which SSL setting is missing so application
startup fails with an actionable message; reference init(), sslContext,
tossPushPort.createSSLContext(...), publicCrt, and privateKey.
In `@src/main/java/OneQ/OnSurvey/global/push/domain/entity/PushProperty.java`:
- Around line 49-52: The PushProperty class currently has a private all-args
constructor (via `@AllArgsConstructor`(access = AccessLevel.PRIVATE)) so callers
cannot instantiate the entity; add a public static factory method (e.g., public
static PushProperty of(...) or create(...)) inside the PushProperty class that
accepts the necessary fields and returns a new PushProperty using the existing
private constructor, or alternatively add a public `@Builder`; ensure the
factory/builder covers the required fields used elsewhere (defaultValue,
description, etc.) so external code can create instances and still use methods
like updateContext.
- Around line 41-47: The PushProperty entity's contextKey field is currently
nullable despite being part of a unique constraint—update the contextKey
declaration in PushProperty to include nullable = false in its `@Column` so nulls
are disallowed; additionally add an explicit `@Column` annotation to the
description field (e.g., `@Column`(name = "description", columnDefinition = "TEXT"
or preferred settings) on the description property) so the column is explicitly
defined by JPA. Ensure you modify the fields named contextKey and description in
the PushProperty class accordingly.
In `@src/main/java/OneQ/OnSurvey/global/push/domain/vo/PushTemplateAddVO.java`:
- Around line 22-23: The conversion in PushTemplateAddVO assumes
pushTemplateAddRequest.defaultContext() is non-null and that each entry's value
list has at least two elements; add defensive validation in the
PushTemplateAddVO conversion logic (where defaultContext() is read) to check for
null and ensure each Map.Entry value list has the required minimum size, and if
validation fails throw a clear IllegalArgumentException (or return a
well-defined default) with a message referencing the offending key; update the
same checks for the other conversion at lines ~38-39 that mirror this logic so
both paths validate input before accessing list indices.
In `@src/main/java/OneQ/OnSurvey/global/push/domain/vo/PushTemplateModifyVO.java`:
- Around line 21-26: The stream mapping in PushTemplateModifyVO that builds new
ModifyPushTemplate from pushTemplateModifyRequest.defaultContext() assumes each
map value exists and has at least two elements, causing
NPE/IndexOutOfBoundsException; update the mapping to first filter or validate
entries where entry.getValue() != null && entry.getValue().size() >= 2 (or
supply safe defaults) before calling get(0)/get(1), and/or log/throw a clear
validation exception for malformed/defaultContext entries so ModifyPushTemplate
is only constructed from safe, well-formed data.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c7f04450-7b99-42c6-8bf3-2e80d5a970f8
📒 Files selected for processing (28)
src/main/java/OneQ/OnSurvey/domain/participation/model/event/SurveyCompletedEvent.javasrc/main/java/OneQ/OnSurvey/domain/participation/service/response/ResponseCommandService.javasrc/main/java/OneQ/OnSurvey/domain/survey/repository/SurveyRepositoryImpl.javasrc/main/java/OneQ/OnSurvey/global/common/config/AsyncConfig.javasrc/main/java/OneQ/OnSurvey/global/common/event/pushAlim/PushAlimEvent.javasrc/main/java/OneQ/OnSurvey/global/infra/discord/DiscordAlarmAsyncFacade.javasrc/main/java/OneQ/OnSurvey/global/infra/discord/DiscordAlarmService.javasrc/main/java/OneQ/OnSurvey/global/infra/discord/notifier/AlertNotifier.javasrc/main/java/OneQ/OnSurvey/global/infra/discord/notifier/DiscordAlertNotifier.javasrc/main/java/OneQ/OnSurvey/global/infra/discord/notifier/NoOpAlertNotifier.javasrc/main/java/OneQ/OnSurvey/global/infra/discord/notifier/dto/PushAlimAlert.javasrc/main/java/OneQ/OnSurvey/global/infra/toss/client/TossApiClient.javasrc/main/java/OneQ/OnSurvey/global/infra/toss/common/dto/push/PushResultResponse.javasrc/main/java/OneQ/OnSurvey/global/infra/toss/common/dto/push/PushTemplateAddRequest.javasrc/main/java/OneQ/OnSurvey/global/infra/toss/common/dto/push/PushTemplateModifyRequest.javasrc/main/java/OneQ/OnSurvey/global/infra/toss/common/dto/push/PushTemplateSendRequest.javasrc/main/java/OneQ/OnSurvey/global/infra/toss/common/exception/TossErrorCode.javasrc/main/java/OneQ/OnSurvey/global/push/adapter/in/PushEventListener.javasrc/main/java/OneQ/OnSurvey/global/push/adapter/out/persistence/PushPropertyJpaRepository.javasrc/main/java/OneQ/OnSurvey/global/push/adapter/out/persistence/PushPropertyRepositoryImpl.javasrc/main/java/OneQ/OnSurvey/global/push/application/PushFacade.javasrc/main/java/OneQ/OnSurvey/global/push/application/port/in/PushUseCase.javasrc/main/java/OneQ/OnSurvey/global/push/application/port/out/PushPropertyRepository.javasrc/main/java/OneQ/OnSurvey/global/push/application/port/out/TossPushPort.javasrc/main/java/OneQ/OnSurvey/global/push/domain/entity/PushProperty.javasrc/main/java/OneQ/OnSurvey/global/push/domain/vo/PushTemplateAddVO.javasrc/main/java/OneQ/OnSurvey/global/push/domain/vo/PushTemplateModifyVO.javasrc/main/java/OneQ/OnSurvey/global/push/domain/vo/PushTemplateVO.java
💤 Files with no reviewable changes (1)
- src/main/java/OneQ/OnSurvey/domain/survey/repository/SurveyRepositoryImpl.java
src/main/java/OneQ/OnSurvey/domain/participation/model/event/SurveyCompletedEvent.java
Show resolved
Hide resolved
src/main/java/OneQ/OnSurvey/domain/participation/service/response/ResponseCommandService.java
Show resolved
Hide resolved
src/main/java/OneQ/OnSurvey/domain/participation/service/response/ResponseCommandService.java
Show resolved
Hide resolved
src/main/java/OneQ/OnSurvey/global/infra/discord/DiscordAlarmService.java
Show resolved
Hide resolved
src/main/java/OneQ/OnSurvey/global/push/domain/vo/PushTemplateAddVO.java
Show resolved
Hide resolved
src/main/java/OneQ/OnSurvey/global/push/domain/vo/PushTemplateModifyVO.java
Show resolved
Hide resolved
KJaeKwan
added a commit
that referenced
this pull request
Mar 5, 2026
KJaeKwan
added a commit
that referenced
this pull request
Mar 5, 2026
KJaeKwan
added a commit
that referenced
this pull request
Mar 5, 2026
KJaeKwan
added a commit
that referenced
this pull request
Mar 5, 2026
KJaeKwan
added a commit
that referenced
this pull request
Mar 5, 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.
✨ Related Issue
📌 Task Details
PushEventListener가PushAlimEvent를 상속받는 이벤트 구현체를 구독하고, 해당 이벤트를 받아 알림을 전송code는 토스에서 승인된 캠페인(스마트 발송)의 템플릿 코드context-key와default-value로 존재한다. 만일 이벤트 객체에 템플릿을 채울 context가 추가로 존재하면context-key에 대응되는 기본값이 오버라이드된다.💬 Review Requirements (Optional)
새로운 템플릿 추가 및 기존 템플릿 수정은 백오피스에 구현할 예정
Summary by CodeRabbit
릴리스 노트