Copilot commented on code in PR #847:
URL: 
https://github.com/apache/rocketmq-dashboard/pull/847#discussion_r3702980685


##########
server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertRuleController.java:
##########
@@ -59,4 +60,11 @@ public Result<Void> deleteRule(@Valid @RequestBody 
DeleteAlertRuleDTO request) {
         alertService.deleteRule(request.getId());
         return Result.ok();
     }
+
+    private AlertRuleVO requireAlertRule(AlertRuleVO rule) {
+        if (rule == null) {
+            throw new BusinessException(400, "Alert rule request is required");
+        }
+        return rule;
+    }

Review Comment:
   The error code/message for a null alert rule request is duplicated across 
controller and service. To avoid drift (e.g., message changes in one place but 
not the other), centralize these into a shared constant (e.g., in 
`AlertService`, a dedicated `AlertErrors` constants class, or an enum) and 
reference it from both layers.



##########
server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertService.java:
##########
@@ -67,14 +67,20 @@ public String exportPrometheusRulesYaml() {
 
 
     public AlertRuleVO createRule(AlertRuleVO rule) {
+        if (rule == null) {
+            throw new BusinessException(400, "Alert rule request is required");
+        }

Review Comment:
   `400` is a magic number here. Consider using a named constant (project-wide 
or local) or `HttpStatus.BAD_REQUEST.value()` to make intent clearer and reduce 
the chance of inconsistent status codes across the codebase.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to