utafrali commented on code in PR #3770:
URL:
https://github.com/apache/rocketmq-dashboard/pull/3770#discussion_r3941991316
##########
web/src/pages/ops/notificationDeliveries.tsx:
##########
@@ -58,12 +59,37 @@ const NotificationDeliveriesPage = () => {
const retryingIdsInFlight = useRef(new Set<number>());
const retryingVisibleInFlight = useRef(false);
const [refreshNonce, setRefreshNonce] = useState(0);
+ const [exporting, setExporting] = useState(false);
const refresh = () => {
setLoading(true);
setRefreshNonce((current) => current + 1);
};
+ const handleExport = async () => {
+ setExporting(true);
+ try {
+ const csv = await exportAlertDeliveries({
+ channel,
+ status,
+ instanceId,
+ });
+ const blob = new Blob([csv], { type: 'text/csv;charset=utf-8' });
+ const url = URL.createObjectURL(blob);
+ const anchor = document.createElement('a');
+ anchor.href = url;
Review Comment:
The anchor element is never appended to the DOM before `.click()` is called.
Firefox requires the anchor to be in the document tree for programmatic
downloads to fire — the download silently does nothing in Firefox. Fix:
```typescript
document.body.appendChild(anchor);
anchor.click();
document.body.removeChild(anchor);
URL.revokeObjectURL(url);
```
##########
server/src/main/java/org/apache/rocketmq/studio/persistence/mapper/RmqAlertNotificationOutboxMapper.java:
##########
@@ -66,6 +66,23 @@ int renewClaim(@Param("id") Long id, @Param("claimToken")
String claimToken,
List<NotificationDeliveryPageVO> findPage(@Param("channel") String
channel, @Param("status") String status,
@Param("instanceId") String instanceId, @Param("limit") int limit,
@Param("offset") long offset);
+ @Select("<script>"
+ + "SELECT o.id, o.alert_id AS alertId, o.channel, o.status,
o.attempt_count AS attemptCount, "
+ + "o.next_attempt_at AS nextAttemptAt, o.last_error AS lastError,
o.delivered_at AS deliveredAt, "
Review Comment:
`message_content` is selected here but
`NotificationOutboxService.exportDeliveries` never calls
`row.getMessageContent()` — the PR description even explicitly says full
message content is intentionally excluded. This column holds a full alert
payload; fetching it across up to 10 000 rows wastes I/O and heap. Remove it
from the SELECT list in `findExportPage`.
##########
server/src/main/java/org/apache/rocketmq/studio/ops/alert/NotificationOutboxService.java:
##########
@@ -63,6 +64,9 @@
public class NotificationOutboxService {
private static final int MAX_ATTEMPTS = 5;
private static final int BATCH_SIZE = 20;
Review Comment:
Every column in the CSV header is camelCase (`deliveryId`, `alertId`,
`createdAt`, ...) except `attempts`. The VO field is `attemptCount` and the DB
column is `attempt_count`. Using `attempts` is the odd one out and will confuse
any tooling that tries to match header names to field names. Rename it to
`attemptCount` to be consistent with the rest of the header.
##########
server/src/main/java/org/apache/rocketmq/studio/ops/alert/SystemAlertController.java:
##########
@@ -87,6 +87,14 @@ public Result<PageResult<NotificationDeliveryPageVO>>
listDeliveriesPage(
return Result.ok(notificationOutboxService.listDeliveries(channel,
status, instanceId, page, pageSize));
}
Review Comment:
Returning `Result<String>` serializes the CSV as a JSON string value. Every
embedded quote must be escaped, doubling parts of the payload, and the frontend
has to parse JSON only to re-encode the data as a Blob. A proper download
endpoint should return bytes with the right headers:
```java
@GetMapping(value = "/deliveries/export", produces =
"text/csv;charset=UTF-8")
public ResponseEntity<byte[]> exportDeliveries(
@RequestParam(required = false) String channel,
@RequestParam(required = false) String status,
@RequestParam(required = false) String instanceId) {
byte[] csv = notificationOutboxService
.exportDeliveries(channel, status, instanceId)
.getBytes(StandardCharsets.UTF_8);
return ResponseEntity.ok()
.header(HttpHeaders.CONTENT_DISPOSITION,
"attachment; filename=\"notification-deliveries.csv\"")
.body(csv);
}
```
This also makes the endpoint work correctly from curl or any non-browser
client without post-processing.
--
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]
