refactor: pr feedbacks

app/client/modules/backups/components/backup-progress-card.tsx

@@ -5,7 +5,7 @@ import { Progress } from "~/client/components/ui/progress";
 import { useServerEvents } from "~/client/hooks/use-server-events";
 import type { BackupProgressEventDto } from "~/schemas/events-dto";
 import { formatDuration } from "~/utils/utils";
-import { formatBytes } from "../../../../utils/format-bytes";
+import { formatBytes } from "~/utils/format-bytes";
 
 type Props = {
 	scheduleId: number;
@@ -71,7 +71,9 @@ export const BackupProgressCard = ({ scheduleId }: Props) => {
 					<p className="font-medium">
 						{progress ? (
 							<>
-								<ByteSize bytes={progress.bytes_done} base={1024} /> / <ByteSize bytes={progress.total_bytes} base={1024} />
+								<ByteSize bytes={progress.bytes_done} base={1024} />
+								&nbsp;/&nbsp;
+								<ByteSize bytes={progress.total_bytes} base={1024} />
 							</>
 						) : (
 							"—"

app/server/modules/backups/backups.execution.ts

@@ -132,11 +132,6 @@ const runBackupOperation = async (ctx: BackupContext, signal: AbortSignal) => {
 	}
 };
 
-const buildBackupSummary = (result: ResticBackupOutputDto | null | undefined) => {
-	if (!result) return undefined;
-	return result;
-};
-
 const finalizeSuccessfulBackup = async (
 	ctx: BackupContext,
 	scheduleId: number,
@@ -144,7 +139,6 @@ const finalizeSuccessfulBackup = async (
 	result: ResticBackupOutputDto | null,
 ) => {
 	const finalStatus = exitCode === 0 ? "success" : "warning";
-	const summary = buildBackupSummary(result);
 
 	if (ctx.schedule.retentionPolicy) {
 		void runForget(scheduleId).catch((error) => {
@@ -183,7 +177,7 @@ const finalizeSuccessfulBackup = async (
 		volumeName: ctx.volume.name,
 		repositoryName: ctx.repository.name,
 		status: finalStatus,
-		summary,
+		summary: result ?? undefined,
 	});
 
 	notificationsService
@@ -191,7 +185,7 @@ const finalizeSuccessfulBackup = async (
 			volumeName: ctx.volume.name,
 			repositoryName: ctx.repository.name,
 			scheduleName: ctx.schedule.name,
-			summary,
+			summary: result ?? undefined,
 		})
 		.catch((error) => {
 			logger.error(`Failed to send backup success notification: ${toMessage(error)}`);

app/server/modules/notifications/notifications.service.ts

@@ -328,7 +328,7 @@ const formatBytesText = (bytes: number) => {
 	return unit ? `${text} ${unit}` : text;
 };
 
-const buildBackupSummaryLines = (summary?: ResticBackupRunSummaryDto) => {
+const buildBackupNotificationLines = (summary?: ResticBackupRunSummaryDto) => {
 	if (!summary) return [];
 
 	const safeNumber = (value: number | undefined) => (typeof value === "number" && Number.isFinite(value) ? value : 0);
@@ -336,6 +336,28 @@ const buildBackupSummaryLines = (summary?: ResticBackupRunSummaryDto) => {
 	const safeBytesText = (value: number | undefined) => formatBytesText(safeNumber(value));
 	const safeDurationText = (value: number | undefined) =>
 		typeof value === "number" && Number.isFinite(value) ? formatDuration(Math.round(value)) : "N/A";
+
+	const hasDetailedStats = summary.files_new || summary.files_changed || summary.dirs_new || summary.data_blobs;
+
+	if (!hasDetailedStats) {
+		const lines: (string | null)[] = [];
+
+		if (summary.total_duration) {
+			lines.push(`Duration: ${Math.round(summary.total_duration)}s`);
+		}
+		if (summary.total_files_processed !== undefined) {
+			lines.push(`Files: ${summary.total_files_processed.toLocaleString()}`);
+		}
+		if (summary.total_bytes_processed !== undefined) {
+			lines.push(`Size: ${safeBytesText(summary.total_bytes_processed)}`);
+		}
+		if (summary.snapshot_id) {
+			lines.push(`Snapshot: ${summary.snapshot_id}`);
+		}
+
+		return lines.filter((line): line is string => Boolean(line));
+	}
+
 	const snapshotText = summary.snapshot_id ?? "N/A";
 
 	const lines = [
@@ -357,7 +379,7 @@ const buildBackupSummaryLines = (summary?: ResticBackupRunSummaryDto) => {
 		`- Snapshot: ${snapshotText}`,
 	];
 
-	return lines.filter((line): line is string => Boolean(line));
+	return lines.filter(Boolean);
 };
 
 const sendBackupNotification = async (
@@ -368,10 +390,6 @@ const sendBackupNotification = async (
 		repositoryName: string;
 		scheduleName?: string;
 		error?: string;
-		duration?: number;
-		filesProcessed?: number;
-		bytesProcessed?: string;
-		snapshotId?: string;
 		summary?: ResticBackupRunSummaryDto;
 	},
 ) => {
@@ -415,11 +433,7 @@ const sendBackupNotification = async (
 				const decryptedConfig = await decryptSensitiveFields(assignment.destination.config);
 				const shoutrrrUrl = buildShoutrrrUrl(decryptedConfig);
 
-				const result = await sendNotification({
-					shoutrrrUrl,
-					title,
-					body,
-				});
+				const result = await sendNotification({ shoutrrrUrl, title, body });
 
 				if (result.success) {
 					logger.info(
@@ -448,21 +462,11 @@ function buildNotificationMessage(
 		repositoryName: string;
 		scheduleName?: string;
 		error?: string;
-		duration?: number;
-		filesProcessed?: number;
-		bytesProcessed?: string;
-		snapshotId?: string;
 		summary?: ResticBackupRunSummaryDto;
 	},
 ) {
 	const backupName = context.scheduleName ?? "backup";
-	const derivedDuration =
-		context.duration ?? (context.summary?.total_duration ? context.summary.total_duration * 1000 : undefined);
-	const derivedFilesProcessed = context.filesProcessed ?? context.summary?.total_files_processed;
-	const derivedBytesProcessed =
-		context.bytesProcessed ?? (context.summary ? formatBytesText(context.summary.total_bytes_processed) : undefined);
-	const derivedSnapshotId = context.snapshotId ?? context.summary?.snapshot_id;
-	const summaryLines = buildBackupSummaryLines(context.summary);
+	const notificationLines = buildBackupNotificationLines(context.summary);
 
 	switch (event) {
 		case "start":
@@ -477,40 +481,34 @@ function buildNotificationMessage(
 					.join("\n"),
 			};
 
-		case "success":
+		case "success": {
+			const bodyLines = [
+				`Volume: ${context.volumeName}`,
+				`Repository: ${context.repositoryName}`,
+				context.scheduleName ? `Schedule: ${context.scheduleName}` : null,
+				...notificationLines,
+			];
+
 			return {
 				title: `Zerobyte ${backupName} completed successfully`,
-				body: [
-					`Volume: ${context.volumeName}`,
-					`Repository: ${context.repositoryName}`,
-					context.scheduleName ? `Schedule: ${context.scheduleName}` : null,
-					derivedDuration ? `Duration: ${Math.round(derivedDuration / 1000)}s` : null,
-					derivedFilesProcessed !== undefined ? `Files: ${derivedFilesProcessed.toLocaleString()}` : null,
-					derivedBytesProcessed ? `Size: ${derivedBytesProcessed}` : null,
-					derivedSnapshotId ? `Snapshot: ${derivedSnapshotId}` : null,
-					...summaryLines,
-				]
-					.filter(Boolean)
-					.join("\n"),
+				body: bodyLines.filter(Boolean).join("\n"),
 			};
+		}
+
+		case "warning": {
+			const bodyLines = [
+				`Volume: ${context.volumeName}`,
+				`Repository: ${context.repositoryName}`,
+				context.scheduleName ? `Schedule: ${context.scheduleName}` : null,
+				context.error ? `Warning: ${context.error}` : null,
+				...notificationLines,
+			];
 
-		case "warning":
 			return {
 				title: `Zerobyte ${backupName} completed with warnings`,
-				body: [
-					`Volume: ${context.volumeName}`,
-					`Repository: ${context.repositoryName}`,
-					context.scheduleName ? `Schedule: ${context.scheduleName}` : null,
-					derivedDuration ? `Duration: ${Math.round(derivedDuration / 1000)}s` : null,
-					derivedFilesProcessed !== undefined ? `Files: ${derivedFilesProcessed.toLocaleString()}` : null,
-					derivedBytesProcessed ? `Size: ${derivedBytesProcessed}` : null,
-					derivedSnapshotId ? `Snapshot: ${derivedSnapshotId}` : null,
-					context.error ? `Warning: ${context.error}` : null,
-					...summaryLines,
-				]
-					.filter(Boolean)
-					.join("\n"),
+				body: bodyLines.filter(Boolean).join("\n"),
 			};
+		}
 
 		case "failure":
 			return {

app/server/modules/repositories/repositories.controller.ts

@@ -49,6 +49,7 @@ import { getRcloneRemoteInfo, listRcloneRemotes } from "../../utils/rclone";
 import { requireAuth, requireOrgAdmin } from "../auth/auth.middleware";
 import { toMessage } from "~/server/utils/errors";
 import { requireDevPanel } from "../auth/dev-panel.middleware";
+import { getSnapshotDuration } from "../../utils/snapshots";
 
 export const repositoriesController = new Hono()
 	.use(requireAuth)
@@ -102,18 +103,14 @@ export const repositoriesController = new Hono()
 		const snapshots = res.map((snapshot) => {
 			const { summary } = snapshot;
 
-			let duration = 0;
-			if (summary) {
-				const { backup_start, backup_end } = summary;
-				duration = new Date(backup_end).getTime() - new Date(backup_start).getTime();
-			}
+			const duration = getSnapshotDuration(summary);
 
 			return {
 				short_id: snapshot.short_id,
 				duration,
 				paths: snapshot.paths,
 				tags: snapshot.tags ?? [],
-				size: summary?.total_bytes_processed || 0,
+				size: summary?.total_bytes_processed ?? 0,
 				time: new Date(snapshot.time).getTime(),
 				retentionCategories: retentionCategories.get(snapshot.short_id) ?? [],
 				summary: summary,
@@ -132,11 +129,7 @@ export const repositoriesController = new Hono()
 		const { id, snapshotId } = c.req.param();
 		const snapshot = await repositoriesService.getSnapshotDetails(id, snapshotId);
 
-		let duration = 0;
-		if (snapshot.summary) {
-			const { backup_start, backup_end } = snapshot.summary;
-			duration = new Date(backup_end).getTime() - new Date(backup_start).getTime();
-		}
+		const duration = getSnapshotDuration(snapshot.summary);
 
 		const response = {
 			short_id: snapshot.short_id,
@@ -144,7 +137,7 @@ export const repositoriesController = new Hono()
 			time: new Date(snapshot.time).getTime(),
 			paths: snapshot.paths,
 			hostname: snapshot.hostname,
-			size: snapshot.summary?.total_bytes_processed || 0,
+			size: snapshot.summary?.total_bytes_processed ?? 0,
 			tags: snapshot.tags ?? [],
 			retentionCategories: [],
 			summary: snapshot.summary,

app/server/utils/snapshots.ts

@@ -0,0 +1,4 @@
+export function getSnapshotDuration(summary?: { backup_start: string; backup_end: string }): number {
+	if (!summary) return 0;
+	return new Date(summary.backup_end).getTime() - new Date(summary.backup_start).getTime();
+}