Conversation
[Feat] ALARM 연동
|
✅ PR 검증 성공 브랜치: 모든 검증을 통과했습니다. 리뷰를 요청해주세요. |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces an alarm/notification system to the SCM service. The changes add event-driven notification functionality using Kafka to send alerts to users when specific purchase-related actions occur (requisition approval/rejection, purchase order approval/rejection/delivery).
- Added alarm event data models including
AlarmEvent,AlarmSentEvent,StatusEvent, and supporting enum types - Integrated alarm notification sending at key business events in purchase requisition and purchase order workflows
- Updated Kafka infrastructure to support new alarm topics and event handling
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| TargetType.java | New enum defining alarm recipient types (employee, department, customer, supplier) |
| SourceType.java | New enum defining alarm source systems with conversion and validation utilities |
| LinkType.java | New enum defining types of linked documents for alarm references |
| AlarmType.java | New enum defining alarm categories by department/module |
| StatusEvent.java | New event class for tracking alarm processing status with failure details |
| AlarmSentEvent.java | New event class for alarm delivery notifications from alarm server to clients |
| AlarmEvent.java | New event class for alarm requests from services to alarm server |
| QuotationServiceImpl.java | Added TODO comment for quotation confirmation alarm |
| MesServiceImpl.java | Added empty line (whitespace change) |
| PurchaseRequisitionServiceImpl.java | Integrated alarm sending for requisition approval and rejection |
| PurchaseOrderServiceImpl.java | Integrated alarm sending for order approval, rejection, delivery start, and delivery completion |
| SalesOrderServiceImpl.java | Added TODO comment for sales order shipment completion alarm |
| KafkaProducerServiceImpl.java | Updated to use new alarm topic and improved code formatting |
| KafkaProducerService.java | Updated import structure and formatting |
| AlarmStatusEventListener.java | New Kafka listener for handling alarm status events |
| KafkaTopicConfig.java | Added new alarm-related Kafka topics |
| .husky/pre-commit | Removed husky pre-commit hook |
| .husky/commit-msg | Removed husky commit message hook |
Comments suppressed due to low confidence (1)
src/main/java/org/ever/_4ever_be_scm/scm/mm/service/impl/PurchaseOrderServiceImpl.java:1
- Duplicate UUID generation on lines 512 and 516. Both
eventIdandalarmIdare being assigned the same UUID value, but callingUuidCreator.getTimeOrderedEpoch().toString()twice may generate different values. If they should be the same, generate once and reuse; if different, this is likely unintended.
package org.ever._4ever_be_scm.scm.mm.service.impl;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| AlarmEvent alarmEventForCreate = AlarmEvent.builder() | ||
| .eventId(UuidCreator.getTimeOrderedEpoch().toString()) | ||
| .eventType(AlarmEvent.class.getName()) | ||
| .timestamp(LocalDateTime.now()) | ||
| .source(SourceType.SCM.name()) | ||
| .alarmId(UuidCreator.getTimeOrderedEpoch().toString()) |
There was a problem hiding this comment.
Duplicate UUID generation on lines 323 and 327. Both eventId and alarmId are being assigned the same UUID value, but calling UuidCreator.getTimeOrderedEpoch().toString() twice may generate different values. If they should be the same, generate once and reuse; if different, this is likely unintended. This pattern repeats throughout the file.
| AlarmEvent alarmEventForCreate = AlarmEvent.builder() | |
| .eventId(UuidCreator.getTimeOrderedEpoch().toString()) | |
| .eventType(AlarmEvent.class.getName()) | |
| .timestamp(LocalDateTime.now()) | |
| .source(SourceType.SCM.name()) | |
| .alarmId(UuidCreator.getTimeOrderedEpoch().toString()) | |
| String uuid = UuidCreator.getTimeOrderedEpoch().toString(); | |
| AlarmEvent alarmEventForCreate = AlarmEvent.builder() | |
| .eventId(uuid) | |
| .eventType(AlarmEvent.class.getName()) | |
| .timestamp(LocalDateTime.now()) | |
| .source(SourceType.SCM.name()) | |
| .alarmId(uuid) |
| AlarmEvent alarmEventForCreate = AlarmEvent.builder() | ||
| .eventId(UuidCreator.getTimeOrderedEpoch().toString()) | ||
| .eventType(AlarmEvent.class.getName()) | ||
| .timestamp(LocalDateTime.now()) | ||
| .source(SourceType.SCM.name()) | ||
| .alarmId(UuidCreator.getTimeOrderedEpoch().toString()) |
There was a problem hiding this comment.
Duplicate UUID generation on lines 386 and 390. Both eventId and alarmId are being assigned the same UUID value, but calling UuidCreator.getTimeOrderedEpoch().toString() twice may generate different values. If they should be the same, generate once and reuse; if different, this is likely unintended.
| AlarmEvent alarmEventForCreate = AlarmEvent.builder() | |
| .eventId(UuidCreator.getTimeOrderedEpoch().toString()) | |
| .eventType(AlarmEvent.class.getName()) | |
| .timestamp(LocalDateTime.now()) | |
| .source(SourceType.SCM.name()) | |
| .alarmId(UuidCreator.getTimeOrderedEpoch().toString()) | |
| String alarmEventId = UuidCreator.getTimeOrderedEpoch().toString(); | |
| AlarmEvent alarmEventForCreate = AlarmEvent.builder() | |
| .eventId(alarmEventId) | |
| .eventType(AlarmEvent.class.getName()) | |
| .timestamp(LocalDateTime.now()) | |
| .source(SourceType.SCM.name()) | |
| .alarmId(alarmEventId) |
| AlarmEvent alarmEventForCreate = AlarmEvent.builder() | ||
| .eventId(UuidCreator.getTimeOrderedEpoch().toString()) | ||
| .eventType(AlarmEvent.class.getName()) | ||
| .timestamp(LocalDateTime.now()) | ||
| .source(SourceType.SCM.name()) | ||
| .alarmId(UuidCreator.getTimeOrderedEpoch().toString()) |
There was a problem hiding this comment.
Duplicate UUID generation on lines 395 and 399. Both eventId and alarmId are being assigned the same UUID value, but calling UuidCreator.getTimeOrderedEpoch().toString() twice may generate different values. If they should be the same, generate once and reuse; if different, this is likely unintended. This pattern repeats in multiple places in this file.
| AlarmEvent alarmEventForCreate = AlarmEvent.builder() | |
| .eventId(UuidCreator.getTimeOrderedEpoch().toString()) | |
| .eventType(AlarmEvent.class.getName()) | |
| .timestamp(LocalDateTime.now()) | |
| .source(SourceType.SCM.name()) | |
| .alarmId(UuidCreator.getTimeOrderedEpoch().toString()) | |
| String eventAlarmUuid = UuidCreator.getTimeOrderedEpoch().toString(); | |
| AlarmEvent alarmEventForCreate = AlarmEvent.builder() | |
| .eventId(eventAlarmUuid) | |
| .eventType(AlarmEvent.class.getName()) | |
| .timestamp(LocalDateTime.now()) | |
| .source(SourceType.SCM.name()) | |
| .alarmId(eventAlarmUuid) |
| .targetType(TargetType.EMPLOYEE) | ||
| .title("발주서 승인") | ||
| .message("해당 발주서가 승인되었습니다. 발주서 번호 = " + productOrder.getProductOrderCode()) | ||
| .linkId(productOrder.getProductOrderCode()) |
There was a problem hiding this comment.
Inconsistent linkId field usage. Line 405 uses productOrder.getProductOrderCode() while other alarm events in this file (lines 523, 590, 710) also use the same. However, in PurchaseRequisitionServiceImpl, the linkId is set to productRequest.getId() (lines 333, 396). The linkId should consistently use either the ID or the code, not mix both approaches.
| .linkId(productOrder.getProductOrderCode()) | |
| .linkId(productOrder.getId()) |
| AlarmEvent alarmEventForCreate = AlarmEvent.builder() | ||
| .eventId(UuidCreator.getTimeOrderedEpoch().toString()) | ||
| .eventType(AlarmEvent.class.getName()) | ||
| .timestamp(LocalDateTime.now()) | ||
| .source(SourceType.SCM.name()) | ||
| .alarmId(UuidCreator.getTimeOrderedEpoch().toString()) |
There was a problem hiding this comment.
Duplicate UUID generation on lines 580 and 584. Both eventId and alarmId are being assigned the same UUID value, but calling UuidCreator.getTimeOrderedEpoch().toString() twice may generate different values. If they should be the same, generate once and reuse; if different, this is likely unintended.
| AlarmEvent alarmEventForCreate = AlarmEvent.builder() | |
| .eventId(UuidCreator.getTimeOrderedEpoch().toString()) | |
| .eventType(AlarmEvent.class.getName()) | |
| .timestamp(LocalDateTime.now()) | |
| .source(SourceType.SCM.name()) | |
| .alarmId(UuidCreator.getTimeOrderedEpoch().toString()) | |
| String alarmEventId = UuidCreator.getTimeOrderedEpoch().toString(); | |
| AlarmEvent alarmEventForCreate = AlarmEvent.builder() | |
| .eventId(alarmEventId) | |
| .eventType(AlarmEvent.class.getName()) | |
| .timestamp(LocalDateTime.now()) | |
| .source(SourceType.SCM.name()) | |
| .alarmId(alarmEventId) |
| AlarmEvent alarmEventForCreate = AlarmEvent.builder() | ||
| .eventId(UuidCreator.getTimeOrderedEpoch().toString()) | ||
| .eventType(AlarmEvent.class.getName()) | ||
| .timestamp(LocalDateTime.now()) | ||
| .source(SourceType.SCM.name()) | ||
| .alarmId(UuidCreator.getTimeOrderedEpoch().toString()) |
There was a problem hiding this comment.
Duplicate UUID generation on lines 700 and 704. Both eventId and alarmId are being assigned the same UUID value, but calling UuidCreator.getTimeOrderedEpoch().toString() twice may generate different values. If they should be the same, generate once and reuse; if different, this is likely unintended.
| AlarmEvent alarmEventForCreate = AlarmEvent.builder() | |
| .eventId(UuidCreator.getTimeOrderedEpoch().toString()) | |
| .eventType(AlarmEvent.class.getName()) | |
| .timestamp(LocalDateTime.now()) | |
| .source(SourceType.SCM.name()) | |
| .alarmId(UuidCreator.getTimeOrderedEpoch().toString()) | |
| String alarmEventId = UuidCreator.getTimeOrderedEpoch().toString(); | |
| AlarmEvent alarmEventForCreate = AlarmEvent.builder() | |
| .eventId(alarmEventId) | |
| .eventType(AlarmEvent.class.getName()) | |
| .timestamp(LocalDateTime.now()) | |
| .source(SourceType.SCM.name()) | |
| .alarmId(alarmEventId) |
| kafkaProducerService.sendAlarmEvent(alarmEventForCreate) | ||
| .whenComplete((result, ex) -> { | ||
| if (ex != null) { | ||
| log.error("[ALARM] 알림 요청 전송 실패 - alarmId: {}, targetId: {}, error: {}", | ||
| alarmEventForCreate.getAlarmId(), targetId, ex.getMessage(), ex); | ||
| } else if (result != null) { | ||
| log.info("[ALARM] 알림 요청 전송 성공 - topic: {}, partition: {}, offset: {}", | ||
| result.getRecordMetadata().topic(), | ||
| result.getRecordMetadata().partition(), | ||
| result.getRecordMetadata().offset()); | ||
| } else { | ||
| log.warn("[ALARM] 알림 요청 전송 결과가 null 입니다 - alarmId: {}, targetId: {}", | ||
| alarmEventForCreate.getAlarmId(), targetId); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Duplicated alarm event logging code. This exact logging pattern for handling alarm event results is repeated multiple times across PurchaseRequisitionServiceImpl and PurchaseOrderServiceImpl. Consider extracting this into a reusable method or utility to reduce code duplication and improve maintainability.
| // public static final String ALARM_SENT_TOPIC = "alarm-sent"; // 알림 발송 | ||
| // public static final String ALARM_SENT_STATUS_TOPIC = "alarm-sent-status"; // 알림 발송 상태 | ||
|
|
||
| @Bean |
There was a problem hiding this comment.
Missing Kafka topic bean definitions. While ALARM_REQUEST_TOPIC and ALARM_REQUEST_STATUS_TOPIC constants are defined, there are no corresponding @Bean methods to actually create these topics (unlike other topics in this file which have bean definitions). This will cause the topics not to be auto-created by Spring Kafka.
| @Bean | |
| @Bean | |
| public NewTopic alarmRequestTopic() { | |
| return TopicBuilder.name(ALARM_REQUEST_TOPIC) | |
| .partitions(3) | |
| .replicas(1) | |
| .build(); | |
| } | |
| @Bean | |
| public NewTopic alarmRequestStatusTopic() { | |
| return TopicBuilder.name(ALARM_REQUEST_STATUS_TOPIC) | |
| .partitions(3) | |
| .replicas(1) | |
| .build(); | |
| } | |
| @Bean |
| import lombok.Getter; | ||
| import lombok.RequiredArgsConstructor; | ||
|
|
||
| @Getter | ||
| @RequiredArgsConstructor |
There was a problem hiding this comment.
Unnecessary Lombok annotations on enum. The @RequiredArgsConstructor annotation is unnecessary for enums as they cannot have constructors in the traditional sense. Additionally, @Getter is redundant since enum values are already accessible. These annotations have no effect and should be removed.
| import lombok.Getter; | |
| import lombok.RequiredArgsConstructor; | |
| @Getter | |
| @RequiredArgsConstructor |
| import org.ever._4ever_be_scm.infrastructure.kafka.event.BaseEvent; | ||
|
|
||
| @Getter | ||
| @SuperBuilder |
There was a problem hiding this comment.
Default toString(): FailureInfo inherits toString() from Object, and so is not suitable for printing.
요약
주요 변경 사항
관련 이슈
테스트/검증
확인 사항
스크린샷/로그(선택)