From 1e636fdbe2d8b8c1e6b3a86fd115fda51cffb7b3 Mon Sep 17 00:00:00 2001 From: Developer Date: Mon, 6 Jul 2026 12:11:47 +0000 Subject: [PATCH] Follow-ups: reference reorder, detach service_id, named-dashboard widgets Three reusable-widget follow-up fixes: 1. Reference sort_order independently reorderable. Reordering a referenced widget now updates the widget_references.sort_order (per- dashboard), not the shared widget instance sort_order. New backend update_widget_reference method + PUT /api/widgets/references/{id} endpoint. Frontend moveInstance checks _ref_id to choose the right mutation (updateRef for references, saveWidget for owned). 2. Detach preserves service_id. detach_widget_reference now copies the original widget's service_id into the clone, so service-bound widgets (Grafana chart, Jellyfin activity) continue to render after detach. 3. Named dashboards support widget references. NamedDashboardPage fetches useWidgetReferences('named:') and renders them via WidgetInstanceCard alongside pinned links. 'Edit widgets' button opens WidgetConfigDialog with dashboardScope='named:'. Also: removed useMemo on combinedWidgets in WidgetConfigDialog to fix a react-hooks/preserve-manual-memoization lint error (the React Compiler ESLint plugin couldn't verify the spread+sort memoization). 283 backend tests pass (+1 update_reference test); 128 frontend tests pass; ruff clean; 0 lint errors. --- .../routers/widgets.py | 13 ++ .../services/settings_store.py | 29 ++++- backend/tests/test_widgets.py | 122 ++++++++++++++---- frontend/src/api/widgets.ts | 10 ++ .../src/components/WidgetConfigDialog.tsx | 65 ++++++---- .../__tests__/WidgetConfigDialog.test.tsx | 1 + frontend/src/hooks/useWidgets.ts | 17 +++ frontend/src/pages/NamedDashboardPage.tsx | 57 ++++++-- .../__tests__/NamedDashboardPage.test.tsx | 50 ++++++- 9 files changed, 294 insertions(+), 70 deletions(-) diff --git a/backend/src/media_library_viewer_api/routers/widgets.py b/backend/src/media_library_viewer_api/routers/widgets.py index c128341..a4a0ccf 100644 --- a/backend/src/media_library_viewer_api/routers/widgets.py +++ b/backend/src/media_library_viewer_api/routers/widgets.py @@ -269,6 +269,19 @@ def delete_reference( return {"status": "deleted"} +@router.put("/references/{reference_id}") +def update_reference( + reference_id: str, + sort_order: int, + store: SettingsStore = Depends(get_settings_store), +) -> dict[str, Any]: + """Update a widget reference's sort_order (per-dashboard reordering).""" + try: + return store.update_widget_reference(reference_id, sort_order) + except ValueError as exc: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(exc)) from exc + + @router.post("/references/{reference_id}/detach") def detach_reference( reference_id: str, diff --git a/backend/src/media_library_viewer_api/services/settings_store.py b/backend/src/media_library_viewer_api/services/settings_store.py index c7021d5..c6cac9c 100644 --- a/backend/src/media_library_viewer_api/services/settings_store.py +++ b/backend/src/media_library_viewer_api/services/settings_store.py @@ -1570,6 +1570,30 @@ class SettingsStore: with self.connect() as conn: conn.execute("DELETE FROM widget_references WHERE id = ?", (reference_id,)) + def update_widget_reference(self, reference_id: str, sort_order: int) -> dict[str, Any]: + """Update only the sort_order on a widget reference (per-dashboard reordering).""" + self.init_schema() + with self.connect() as conn: + row = conn.execute( + "SELECT * FROM widget_references WHERE id = ?", + (reference_id,), + ).fetchone() + if not row: + raise ValueError(f"Reference {reference_id} not found") + conn.execute( + "UPDATE widget_references SET sort_order = ? WHERE id = ?", + (sort_order, reference_id), + ) + widget = self.get_widget(row["widget_id"]) + return { + "id": row["id"], + "dashboard_scope": row["dashboard_scope"], + "widget_id": row["widget_id"], + "sort_order": sort_order, + "created_at": int(row["created_at"]), + "widget": widget, + } + def detach_widget_reference(self, reference_id: str, dashboard_scope: str) -> dict[str, Any]: """Clone the referenced widget into a new standalone instance owned by the scope.""" self.init_schema() @@ -1583,10 +1607,11 @@ class SettingsStore: source = self.get_widget(row["widget_id"]) if not source: raise ValueError(f"Source widget {row['widget_id']} not found") - # Clone: new widget with service_id=NULL (dashboard scope), same config/kind/title. + # Clone: copy the widget verbatim including service_id (so service-bound + # widgets keep working), only the id/created_at change. cloned = self.upsert_widget( { - "service_id": None, + "service_id": source.get("service_id"), "widget_kind": source["widget_kind"], "title": source["title"], "config": source["config"], diff --git a/backend/tests/test_widgets.py b/backend/tests/test_widgets.py index 9b249ed..f82ed0b 100644 --- a/backend/tests/test_widgets.py +++ b/backend/tests/test_widgets.py @@ -730,6 +730,7 @@ async def test_jellyfin_activity_shows_all_sessions(): # Widget references (live-link widgets across dashboards) # --------------------------------------------------------------------------- + @pytest.fixture def widget_ref_client(monkeypatch): """TestClient with an isolated SettingsStore + encryption key.""" @@ -770,21 +771,26 @@ def test_widget_reference_lifecycle(widget_ref_client): }, ) service = store.list_services("grafana")[0] - widget = store.upsert_widget({ - "service_id": service["id"], - "widget_kind": "chart", - "title": "CPU IOWait", - "config": {"query": "rate(cpu[5m])", "datasource_uid": "prometheus"}, - "enabled": True, - "sort_order": 0, - }) + widget = store.upsert_widget( + { + "service_id": service["id"], + "widget_kind": "chart", + "title": "CPU IOWait", + "config": {"query": "rate(cpu[5m])", "datasource_uid": "prometheus"}, + "enabled": True, + "sort_order": 0, + } + ) # Reference it on "main" dashboard. - resp = client.post("/api/widgets/references", json={ - "dashboard_scope": "main", - "widget_id": widget["id"], - "sort_order": 5, - }) + resp = client.post( + "/api/widgets/references", + json={ + "dashboard_scope": "main", + "widget_id": widget["id"], + "sort_order": 5, + }, + ) assert resp.status_code == 201 ref = resp.json() assert ref["dashboard_scope"] == "main" @@ -823,20 +829,25 @@ def test_widget_reference_detach(widget_ref_client): }, ) service = store.list_services("grafana")[0] - widget = store.upsert_widget({ - "service_id": service["id"], - "widget_kind": "chart", - "title": "Memory", - "config": {"query": "mem", "datasource_uid": "prometheus"}, - "enabled": True, - "sort_order": 0, - }) + widget = store.upsert_widget( + { + "service_id": service["id"], + "widget_kind": "chart", + "title": "Memory", + "config": {"query": "mem", "datasource_uid": "prometheus"}, + "enabled": True, + "sort_order": 0, + } + ) # Reference on "main". - resp = client.post("/api/widgets/references", json={ - "dashboard_scope": "main", - "widget_id": widget["id"], - }) + resp = client.post( + "/api/widgets/references", + json={ + "dashboard_scope": "main", + "widget_id": widget["id"], + }, + ) ref_id = resp.json()["id"] # Detach. @@ -845,7 +856,7 @@ def test_widget_reference_detach(widget_ref_client): cloned = resp.json() assert cloned["title"] == "Memory" assert cloned["widget_kind"] == "chart" - assert cloned["service_id"] is None # dashboard-scoped clone + assert cloned["service_id"] == service["id"] # Fix 2: preserves service binding assert cloned["config"]["query"] == "mem" assert cloned["id"] != widget["id"] # new independent widget @@ -854,3 +865,62 @@ def test_widget_reference_detach(widget_ref_client): assert len(refs) == 0 # Original still exists. assert store.get_widget(widget["id"]) is not None + + +def test_widget_reference_update_sort_order(widget_ref_client): + """PUT /references/{id} updates only the reference's sort_order (Fix 1).""" + client, store = widget_ref_client + + widget_a = store.upsert_widget( + { + "service_id": None, + "widget_kind": "static", + "title": "A", + "config": {"text": "a"}, + "enabled": True, + "sort_order": 0, + } + ) + widget_b = store.upsert_widget( + { + "service_id": None, + "widget_kind": "static", + "title": "B", + "config": {"text": "b"}, + "enabled": True, + "sort_order": 1, + } + ) + + # Two references on the same dashboard scope. + resp = client.post( + "/api/widgets/references", + json={ + "dashboard_scope": "named:test", + "widget_id": widget_a["id"], + "sort_order": 0, + }, + ) + ref_a = resp.json() + resp = client.post( + "/api/widgets/references", + json={ + "dashboard_scope": "named:test", + "widget_id": widget_b["id"], + "sort_order": 1, + }, + ) + ref_b = resp.json() + + # Swap sort orders via PUT (per-dashboard reorder). + resp = client.put(f"/api/widgets/references/{ref_a['id']}?sort_order=1") + assert resp.status_code == 200 + assert resp.json()["sort_order"] == 1 + + resp = client.put(f"/api/widgets/references/{ref_b['id']}?sort_order=0") + assert resp.status_code == 200 + assert resp.json()["sort_order"] == 0 + + # Widget instances themselves are unchanged. + assert store.get_widget(widget_a["id"])["sort_order"] == 0 + assert store.get_widget(widget_b["id"])["sort_order"] == 1 diff --git a/frontend/src/api/widgets.ts b/frontend/src/api/widgets.ts index 07fb09f..52a3d31 100644 --- a/frontend/src/api/widgets.ts +++ b/frontend/src/api/widgets.ts @@ -87,3 +87,13 @@ export async function detachWidgetReference( ): Promise { return post(`/api/widgets/references/${referenceId}/detach`); } + +export async function updateWidgetReference( + referenceId: string, + sortOrder: number, +): Promise { + return put( + `/api/widgets/references/${referenceId}?sort_order=${sortOrder}`, + {}, + ); +} diff --git a/frontend/src/components/WidgetConfigDialog.tsx b/frontend/src/components/WidgetConfigDialog.tsx index 6e847a1..52da72d 100644 --- a/frontend/src/components/WidgetConfigDialog.tsx +++ b/frontend/src/components/WidgetConfigDialog.tsx @@ -34,6 +34,7 @@ import { useDeleteWidgetReference, useDetachWidgetReference, useSaveWidgetInstance, + useUpdateWidgetReference, useWidgetInstances, useWidgetReferences, } from "../hooks/useWidgets"; @@ -212,20 +213,13 @@ export function WidgetConfigDialog({ const createRef = useCreateWidgetReference(); const deleteRef = useDeleteWidgetReference(); const detachRef = useDetachWidgetReference(); + const updateRef = useUpdateWidgetReference(); const { data: allWidgets = [] } = useWidgetInstances(); const [showExisting, setShowExisting] = useState(false); const [existingSearch, setExistingSearch] = useState(""); const [draft, setDraft] = useState(null); - const sortedInstances = useMemo( - () => - [...instances].sort( - (a, b) => a.sort_order - b.sort_order || a.created_at - b.created_at, - ), - [instances], - ); - function startAddBuiltIn(kind: string) { const binding = BUILTIN_WIDGETS[kind]; setDraft({ @@ -297,13 +291,29 @@ export function WidgetConfigDialog({ async function moveInstance(index: number, direction: -1 | 1) { const targetIndex = index + direction; - if (targetIndex < 0 || targetIndex >= sortedInstances.length) return; - const a = sortedInstances[index]; - const b = sortedInstances[targetIndex]; - // Sequential (not Promise.all) to avoid a race where the first mutation's - // cache invalidation refetches before the second completes, reverting the swap. - await saveWidget.mutateAsync({ ...a, sort_order: b.sort_order }); - await saveWidget.mutateAsync({ ...b, sort_order: a.sort_order }); + if (targetIndex < 0 || targetIndex >= combinedWidgets.length) return; + const a = combinedWidgets[index]; + const b = combinedWidgets[targetIndex]; + const aRefId = (a as { _ref_id?: string })._ref_id; + const bRefId = (b as { _ref_id?: string })._ref_id; + // References use their own sort_order on the widget_references row; + // owned widgets use the widget instance's sort_order. + if (aRefId) { + await updateRef.mutateAsync({ + referenceId: aRefId, + sortOrder: b.sort_order, + }); + } else { + await saveWidget.mutateAsync({ ...a, sort_order: b.sort_order }); + } + if (bRefId) { + await updateRef.mutateAsync({ + referenceId: bRefId, + sortOrder: a.sort_order, + }); + } else { + await saveWidget.mutateAsync({ ...b, sort_order: a.sort_order }); + } } async function removeInstance(instance: WidgetInstance) { @@ -311,20 +321,19 @@ export function WidgetConfigDialog({ } // Build a combined view of owned widgets + references for display. + const owned = [...instances].sort( + (a, b) => a.sort_order - b.sort_order || a.created_at - b.created_at, + ); + const refs = references.map((r) => ({ + ...r.widget, + _ref_id: r.id, + _is_reference: true as const, + })); + const combinedWidgets = [...owned, ...refs].sort( + (a, b) => a.sort_order - b.sort_order || a.created_at - b.created_at, + ); + const referencedWidgetIds = new Set(references.map((r) => r.widget_id)); - const combinedWidgets = useMemo(() => { - const owned = [...instances].sort( - (a, b) => a.sort_order - b.sort_order || a.created_at - b.created_at, - ); - const refs = references.map((r) => ({ - ...r.widget, - _ref_id: r.id, - _is_reference: true as const, - })); - return [...owned, ...refs].sort( - (a, b) => a.sort_order - b.sort_order || a.created_at - b.created_at, - ); - }, [instances, references]); // Available widgets for the "Add existing" picker: all widgets not already // on this dashboard (owned or referenced). diff --git a/frontend/src/components/__tests__/WidgetConfigDialog.test.tsx b/frontend/src/components/__tests__/WidgetConfigDialog.test.tsx index 2482069..d6a19ce 100644 --- a/frontend/src/components/__tests__/WidgetConfigDialog.test.tsx +++ b/frontend/src/components/__tests__/WidgetConfigDialog.test.tsx @@ -24,6 +24,7 @@ vi.mock("../../hooks/useWidgets", () => ({ useCreateWidgetReference: () => ({ mutateAsync: vi.fn() }), useDeleteWidgetReference: () => ({ mutateAsync: vi.fn() }), useDetachWidgetReference: () => ({ mutateAsync: vi.fn() }), + useUpdateWidgetReference: () => ({ mutateAsync: vi.fn() }), })); vi.mock("../../hooks/useServices", () => ({ diff --git a/frontend/src/hooks/useWidgets.ts b/frontend/src/hooks/useWidgets.ts index c259f97..3a94f80 100644 --- a/frontend/src/hooks/useWidgets.ts +++ b/frontend/src/hooks/useWidgets.ts @@ -10,6 +10,7 @@ import { fetchWidgetInstances, fetchWidgetReferences, updateWidgetInstance, + updateWidgetReference, } from "../api/widgets"; import type { WidgetInstanceInput } from "../types"; @@ -101,3 +102,19 @@ export function useDetachWidgetReference() { }, }); } + +export function useUpdateWidgetReference() { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: ({ + referenceId, + sortOrder, + }: { + referenceId: string; + sortOrder: number; + }) => updateWidgetReference(referenceId, sortOrder), + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: ["widgets", "references"] }); + }, + }); +} diff --git a/frontend/src/pages/NamedDashboardPage.tsx b/frontend/src/pages/NamedDashboardPage.tsx index 9c24ba9..ad3b28d 100644 --- a/frontend/src/pages/NamedDashboardPage.tsx +++ b/frontend/src/pages/NamedDashboardPage.tsx @@ -1,10 +1,14 @@ -import { useMemo } from "react"; +import { useMemo, useState } from "react"; import { useParams } from "react-router-dom"; -import { Boxes } from "lucide-react"; +import { Boxes, Settings2 } from "lucide-react"; import { Alert, AlertDescription } from "@/components/ui/alert"; +import { Button } from "@/components/ui/button"; import { Skeleton } from "@/components/ui/skeleton"; import { useDashboardBySlug } from "../hooks/useDashboards"; +import { useWidgetReferences } from "../hooks/useWidgets"; import { PinnedServiceLink } from "../components/PinnedServiceLink"; +import { WidgetInstanceCard } from "../components/WidgetInstance"; +import { WidgetConfigDialog } from "../components/WidgetConfigDialog"; /** * Payload model for named dashboards (design choice: inline items, not widget @@ -42,12 +46,24 @@ function parseItems(payload: Record): DashboardItem[] { export function NamedDashboardPage() { const { slug = "" } = useParams<{ slug: string }>(); const { data: dashboard, isLoading, isError } = useDashboardBySlug(slug); + const dashboardScope = `named:${slug}`; + const { data: widgetRefs = [] } = useWidgetReferences(dashboardScope); + const [configOpen, setConfigOpen] = useState(false); const items = useMemo( () => parseItems(dashboard?.payload ?? {}), [dashboard?.payload], ); + const visibleWidgets = useMemo( + () => + widgetRefs + .filter((r) => r.widget.enabled) + .map((r) => r.widget) + .sort((a, b) => a.sort_order - b.sort_order), + [widgetRefs], + ); + if (isLoading) { return ; } @@ -64,17 +80,36 @@ export function NamedDashboardPage() { return (
-
+

{dashboard.label}

+
- {items.length === 0 ? ( + + {visibleWidgets.length > 0 ? ( +
+ {visibleWidgets.map((widget) => ( + + ))} +
+ ) : null} + + {items.length === 0 && visibleWidgets.length === 0 ? ( - This dashboard has no shortcuts yet. Add pinned service links from - the dashboard management panel on the Services page. + This dashboard is empty. Add widgets via "Edit widgets" or pinned + service links from the dashboard management panel on the Services + page. - ) : ( + ) : items.length > 0 ? (
{items.map((item, index) => ( ))}
- )} + ) : null} + + setConfigOpen(false)} + dashboardScope={dashboardScope} + />
); } diff --git a/frontend/src/pages/__tests__/NamedDashboardPage.test.tsx b/frontend/src/pages/__tests__/NamedDashboardPage.test.tsx index 2943fce..b1a9f16 100644 --- a/frontend/src/pages/__tests__/NamedDashboardPage.test.tsx +++ b/frontend/src/pages/__tests__/NamedDashboardPage.test.tsx @@ -1,21 +1,39 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { render, screen } from "@testing-library/react"; import { MemoryRouter, Route, Routes } from "react-router-dom"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { NamedDashboardPage } from "../NamedDashboardPage"; vi.mock("../../hooks/useDashboards", () => ({ useDashboardBySlug: vi.fn(() => ({ data: undefined, isLoading: true })), })); +vi.mock("../../hooks/useWidgets", () => ({ + useWidgetReferences: () => ({ data: [] }), +})); + +vi.mock("../../components/WidgetConfigDialog", () => ({ + WidgetConfigDialog: () =>
, +})); + +vi.mock("../../components/WidgetInstance", () => ({ + WidgetInstanceCard: () =>
, +})); + import { useDashboardBySlug } from "../../hooks/useDashboards"; function renderPage(slug: string) { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); return render( - - - } /> - - , + + + + } /> + + + , ); } @@ -88,6 +106,26 @@ describe("NamedDashboardPage", () => { } as never); renderPage("empty"); expect(screen.getByText("Empty")).toBeInTheDocument(); - expect(screen.getByText(/no shortcuts yet/i)).toBeInTheDocument(); + expect(screen.getByText(/This dashboard is empty/i)).toBeInTheDocument(); + }); + + it("renders an edit-widgets button", () => { + vi.mocked(useDashboardBySlug).mockReturnValue({ + data: { + id: "d1", + label: "Storage", + slug: "storage", + sort_order: 0, + payload: { items: [] }, + created_at: 1, + updated_at: 1, + }, + isLoading: false, + isError: false, + } as never); + renderPage("storage"); + expect( + screen.getByRole("button", { name: /Edit widgets/i }), + ).toBeInTheDocument(); }); });