From 7746cf3608986bc3826faa744c7ddd8cdb56ea70 Mon Sep 17 00:00:00 2001 From: Nicolas Meienberger Date: Thu, 12 Feb 2026 18:30:21 +0100 Subject: [PATCH] refactor: pr feedbacks --- app/client/hooks/use-server-events.ts | 6 ++-- .../components/schedule-mirrors-config.tsx | 2 +- app/client/modules/backups/routes/backups.tsx | 21 +++++++++++-- .../routes/repository-details.tsx | 7 ----- .../__tests__/backups.execution.test.ts | 31 +++++++++++++++++++ .../modules/backups/backups.execution.ts | 7 ++--- app/server/modules/backups/backups.queries.ts | 12 +++++-- 7 files changed, 67 insertions(+), 19 deletions(-) diff --git a/app/client/hooks/use-server-events.ts b/app/client/hooks/use-server-events.ts index 17bc34eb..28eefd16 100644 --- a/app/client/hooks/use-server-events.ts +++ b/app/client/hooks/use-server-events.ts @@ -1,4 +1,4 @@ -import { useEffect, useRef } from "react"; +import { useCallback, useEffect, useRef } from "react"; import { useQueryClient } from "@tanstack/react-query"; import type { BackupCompletedEventDto, @@ -205,7 +205,7 @@ export function useServerEvents() { }; }, [queryClient]); - const addEventListener = (event: ServerEventType, handler: EventHandler) => { + const addEventListener = useCallback((event: ServerEventType, handler: EventHandler) => { if (!handlersRef.current.has(event)) { handlersRef.current.set(event, new Set()); } @@ -214,7 +214,7 @@ export function useServerEvents() { return () => { handlersRef.current.get(event)?.delete(handler); }; - }; + }, []); return { addEventListener }; } diff --git a/app/client/modules/backups/components/schedule-mirrors-config.tsx b/app/client/modules/backups/components/schedule-mirrors-config.tsx index 855c0574..3fa4e940 100644 --- a/app/client/modules/backups/components/schedule-mirrors-config.tsx +++ b/app/client/modules/backups/components/schedule-mirrors-config.tsx @@ -356,7 +356,7 @@ export const ScheduleMirrorsConfig = ({ scheduleId, primaryRepositoryId, reposit {getLabel(assignment)} diff --git a/app/client/modules/backups/routes/backups.tsx b/app/client/modules/backups/routes/backups.tsx index a30bf683..6524a85c 100644 --- a/app/client/modules/backups/routes/backups.tsx +++ b/app/client/modules/backups/routes/backups.tsx @@ -49,8 +49,25 @@ export function BackupsPage() { if (over && active.id !== over.id) { setLocalItems((currentItems) => { const baseItems = currentItems ?? schedules?.map((s) => s.id) ?? []; - const oldIndex = baseItems.indexOf(active.id as number); - const newIndex = baseItems.indexOf(over.id as number); + const activeId = active.id as number; + const overId = over.id as number; + let oldIndex = baseItems.indexOf(activeId); + let newIndex = baseItems.indexOf(overId); + + if (oldIndex === -1 || newIndex === -1) { + const freshItems = schedules?.map((s) => s.id) ?? []; + oldIndex = freshItems.indexOf(activeId); + newIndex = freshItems.indexOf(overId); + + if (oldIndex === -1 || newIndex === -1) { + return currentItems; + } + + const newItems = arrayMove(freshItems, oldIndex, newIndex); + reorderMutation.mutate({ body: { scheduleIds: newItems } }); + return newItems; + } + const newItems = arrayMove(baseItems, oldIndex, newIndex); reorderMutation.mutate({ body: { scheduleIds: newItems } }); diff --git a/app/client/modules/repositories/routes/repository-details.tsx b/app/client/modules/repositories/routes/repository-details.tsx index b6074623..3460db62 100644 --- a/app/client/modules/repositories/routes/repository-details.tsx +++ b/app/client/modules/repositories/routes/repository-details.tsx @@ -6,13 +6,6 @@ import { RepositoryInfoTabContent } from "../tabs/info"; import { RepositorySnapshotsTabContent } from "../tabs/snapshots"; import { useNavigate, useSearch } from "@tanstack/react-router"; -export const handle = { - breadcrumb: (match: Route.MetaArgs) => [ - { label: "Repositories", href: "/repositories" }, - { label: match.loaderData?.name || match.params.id }, - ], -}; - export default function RepositoryDetailsPage({ repositoryId }: { repositoryId: string }) { const navigate = useNavigate(); const { tab } = useSearch({ from: "/(dashboard)/repositories/$repositoryId" }); diff --git a/app/server/modules/backups/__tests__/backups.execution.test.ts b/app/server/modules/backups/__tests__/backups.execution.test.ts index e24397bc..cc585238 100644 --- a/app/server/modules/backups/__tests__/backups.execution.test.ts +++ b/app/server/modules/backups/__tests__/backups.execution.test.ts @@ -296,6 +296,37 @@ describe("mirror operations", () => { expect(updatedMirror?.lastCopyAt).not.toBeNull(); }); + test("should finalize mirror status when mirror settings are updated during copy", async () => { + // arrange + const volume = await createTestVolume(); + const sourceRepository = await createTestRepository(); + const mirrorRepository = await createTestRepository(); + const schedule = await createTestBackupSchedule({ + volumeId: volume.id, + repositoryId: sourceRepository.id, + }); + + const originalMirror = await createTestBackupScheduleMirror(schedule.id, mirrorRepository.id); + + resticCopyMock.mockImplementationOnce(async () => { + await backupsService.updateMirrors(schedule.id, { + mirrors: [{ repositoryId: mirrorRepository.id, enabled: true }], + }); + return { success: true, output: "" }; + }); + + // act + await backupsExecutionService.copyToMirrors(schedule.id, sourceRepository, null); + + // assert + const mirrors = await backupsService.getMirrors(schedule.id); + expect(mirrors).toHaveLength(1); + expect(mirrors[0]?.id).not.toBe(originalMirror.id); + expect(mirrors[0]?.lastCopyStatus).toBe("success"); + expect(mirrors[0]?.lastCopyError).toBeNull(); + expect(mirrors[0]?.lastCopyAt).not.toBeNull(); + }); + test("should update mirror status on failure", async () => { // arrange const volume = await createTestVolume(); diff --git a/app/server/modules/backups/backups.execution.ts b/app/server/modules/backups/backups.execution.ts index 61ddeec3..066c1c4a 100644 --- a/app/server/modules/backups/backups.execution.ts +++ b/app/server/modules/backups/backups.execution.ts @@ -367,7 +367,6 @@ const copyToSingleMirror = async ( schedule: BackupSchedule, sourceRepository: Repository, mirror: { - id: number; repositoryId: string; repository: Repository; }, @@ -384,7 +383,7 @@ const copyToSingleMirror = async ( repositoryName: mirror.repository.name, }); - await mirrorQueries.updateStatus(mirror.id, { + await mirrorQueries.updateStatus(scheduleId, mirror.repositoryId, { lastCopyStatus: "in_progress", lastCopyError: null, }); @@ -408,7 +407,7 @@ const copyToSingleMirror = async ( }); } - await mirrorQueries.updateStatus(mirror.id, { + await mirrorQueries.updateStatus(scheduleId, mirror.repositoryId, { lastCopyAt: Date.now(), lastCopyStatus: "success", lastCopyError: null, @@ -427,7 +426,7 @@ const copyToSingleMirror = async ( const errorMessage = toMessage(error); logger.error(`[Background] Failed to copy to mirror repository ${mirror.repository.name}: ${errorMessage}`); - await mirrorQueries.updateStatus(mirror.id, { + await mirrorQueries.updateStatus(scheduleId, mirror.repositoryId, { lastCopyAt: Date.now(), lastCopyStatus: "error", lastCopyError: errorMessage, diff --git a/app/server/modules/backups/backups.queries.ts b/app/server/modules/backups/backups.queries.ts index 0b782ce9..5f64664b 100644 --- a/app/server/modules/backups/backups.queries.ts +++ b/app/server/modules/backups/backups.queries.ts @@ -59,8 +59,16 @@ export const mirrorQueries = { }); }, - updateStatus: async (mirrorId: number, status: MirrorStatusUpdate) => { - return db.update(backupScheduleMirrorsTable).set(status).where(eq(backupScheduleMirrorsTable.id, mirrorId)); + updateStatus: async (scheduleId: number, repositoryId: string, status: MirrorStatusUpdate) => { + return db + .update(backupScheduleMirrorsTable) + .set(status) + .where( + and( + eq(backupScheduleMirrorsTable.scheduleId, scheduleId), + eq(backupScheduleMirrorsTable.repositoryId, repositoryId), + ), + ); }, };