[UPLUS-150] 청구서 데이터 예약 알림 메시지 형식 수정#15
Conversation
Summary of ChangesHello @starboxxxx, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 청구서 알림 데이터 구조와 처리 로직을 리팩토링합니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이번 PR은 청구서 데이터 기반의 알림 메시지 형식을 기존 InvoiceAggregateRecord에서 새로운 InvoiceNotificationEvent로 리팩토링하는 작업을 포함하고 있습니다. 전반적으로 메시지 구조가 더 명확해지고 Kafka 이벤트로 사용하기에 적합해진 점이 좋습니다.
다만, 코드 내에 하드코딩된 값(매직 넘버, 매직 스트링)들이 여러 곳에서 발견되었습니다. 이들을 상수나 enum으로 추출하여 코드의 가독성과 유지보수성을 높이는 방향으로 개선하는 것을 제안합니다. 또한, 일부 Java 네이밍 컨벤션에 맞지 않는 부분에 대한 개선 의견도 포함했습니다. 자세한 내용은 각 파일의 리뷰 코멘트를 참고해 주세요.
| private String mapItemName(String type) { | ||
| return switch (type) { | ||
| case "PLAN" -> "기본 요금"; | ||
| case "VAS" -> "부가서비스"; | ||
| case "DISCOUNT" -> "할인"; | ||
| case "MICRO" -> "소액 결제"; | ||
| default -> "기타"; | ||
| }; | ||
| } |
There was a problem hiding this comment.
mapItemName 메소드와 buildEvent 메소드 내에서 "PLAN", "VAS" 등 invoiceType을 나타내는 문자열이 하드코딩되어 사용되고 있습니다. (e.g., L72, L108-L111)
이러한 방식은 오타에 취약하며, 새로운 타입이 추가될 때 여러 곳을 수정해야 하는 불편함이 있습니다.
InvoiceItemType과 같은 enum을 정의하여 타입 안정성을 높이고 유지보수성을 개선하는 것을 강력히 권장합니다. enum을 사용하면 관련 로직을 한 곳에서 관리할 수 있습니다.
개선 예시:
public enum InvoiceItemType {
PLAN("기본 요금"),
VAS("부가서비스"),
DISCOUNT("할인"),
MICRO("소액 결제"),
ETC("기타");
private final String koreanName;
InvoiceItemType(String koreanName) {
this.koreanName = koreanName;
}
public String getKoreanName() {
return koreanName;
}
public static InvoiceItemType fromString(String text) {
if (text != null) {
for (InvoiceItemType b : InvoiceItemType.values()) {
if (text.equalsIgnoreCase(b.name())) {
return b;
}
}
}
return ETC;
}
}
// InvoiceSendProcessor.java
private InvoiceNotificationEvent buildEvent() {
String planName = currentItems.stream()
.filter(i -> InvoiceItemType.PLAN.name().equals(i.invoiceType())) // enum 사용
.map(InvoiceItemRecord::invoiceName)
.findFirst()
.orElse(null);
// ...
}
private String mapItemName(String type) {
return InvoiceItemType.fromString(type).getKoreanName();
}| UUID.randomUUID(), | ||
| item.id(), | ||
| 101L, | ||
| 2L, |
There was a problem hiding this comment.
templateGroupId로 사용되는 2L은 의미를 알기 어려운 매직 넘버입니다. 코드의 가독성과 유지보수성을 높이기 위해, 이 값의 의미를 명확히 나타내는 상수로 정의하는 것을 고려해 보세요. 예를 들어, NotificationTemplateConstants와 같은 별도의 상수 관리 클래스에 USAGE_NOTIFICATION_TEMPLATE_GROUP_ID와 같이 정의하여 사용할 수 있습니다.
|
|
||
| return new InvoiceNotificationEvent( | ||
| UUID.randomUUID(), | ||
| 1L, // templateGroupId 고정 |
There was a problem hiding this comment.
templateGroupId로 사용되는 1L은 매직 넘버입니다. 이 값의 의미를 명확히 나타내는 상수로 정의하여 가독성과 유지보수성을 향상시키는 것이 좋습니다. 예를 들어, INVOICE_NOTIFICATION_TEMPLATE_GROUP_ID와 같은 이름의 상수를 사용할 수 있습니다. NotificationSendProcessor에서 발견된 매직 넘버와 함께 NotificationTemplateConstants와 같은 클래스에서 상수를 중앙 관리하면 더욱 체계적인 코드가 될 것입니다.
| 1L, // templateGroupId 고정 | ||
| new InvoiceNotificationEvent.SubscriptionInfo( | ||
| currentInvoice.subId(), | ||
| currentInvoice.phone_enc(), |
There was a problem hiding this comment.
사용하시는 currentInvoice 객체의 phone_enc()와 email_enc() 메소드 호출은 InvoiceSendRecord의 필드명이 Java 네이밍 컨벤션(camelCase)을 따르지 않고 있음을 보여줍니다. 코드의 일관성과 가독성을 위해 InvoiceSendRecord의 phone_enc, email_enc 필드를 phoneEnc, emailEnc와 같이 camelCase로 리팩토링하는 것을 고려해 보세요. 이 변경은 InvoiceSendRecord를 생성하는 부분(예: DB 쿼리)에도 영향을 줄 수 있으므로 관련 코드도 함께 수정해야 합니다.
| currentInvoice.createdAt().format(DATE_FORMAT), | ||
| currentInvoice.dueDate().format(DATE_FORMAT), | ||
| currentInvoice.totalPrice(), | ||
| "PAID", |
There was a problem hiding this comment.
paymentStatus에 하드코딩된 문자열 "PAID"가 사용되었습니다. 이러한 값은 오타에 취약하고, 나중에 상태가 추가/변경될 때 관리가 어렵습니다. PaymentStatus와 같은 enum을 정의하거나, 최소한 상수로 분리하여 관리하는 것을 권장합니다.
개선 예시:
public enum PaymentStatus {
PAID, UNPAID, FAILED;
}
// ...
new InvoiceNotificationEvent.Variables(
// ...
PaymentStatus.PAID.name(),
items
);
SonarQube Quality Summary (Community)❌ Quality Gate FAILED Branch: Issues
Measures
🔗 Dashboard: https://sonarqube.swthewhite.store/dashboard?id=batch-core&branch=fix/UPLUS-150 Generated automatically by GitHub Actions. |
|
👋 PR 리마인드 알림입니다 이 PR이 몇 시간 동안 업데이트되지 않았어요.
바쁘실 수 있다는 점 이해합니다 🙏 |
1 participant
🎫 지라 티켓
UPLUS-150
✅ 작업 사항
청구서 데이터 예약 알림 메시지 형식 수정
📋 체크리스트
⌨ 기타