PUT /panels/ carries a version and refuses a stale one with 409

The Panels dialog re-reads the list on 409. A panels.json without a
version reads as 0.

backend/fluksio/api/routes/panels.py

@@ -269,6 +269,8 @@ class PanelsPublic(BaseModel):
     """
 
     panels: list[PanelDef] = Field(default_factory=list)
+    #: What a save of this list has to send back.
+    version: int = 0
     #: Whatever this instance was told it is reachable at. The same setting
     #: the password-reset links are built from, so an instance that has it
     #: wrong has it wrong in both places.
@@ -277,7 +279,9 @@ class PanelsPublic(BaseModel):
 
 def _public(config: PanelsConfig) -> PanelsPublic:
     return PanelsPublic(
-        panels=config.panels, frontend_host=settings.FRONTEND_HOST.rstrip("/")
+        panels=config.panels,
+        version=config.version,
+        frontend_host=settings.FRONTEND_HOST.rstrip("/"),
     )
 
 
@@ -298,6 +302,11 @@ async def save_panels(body: PanelsConfig) -> Any:
     A panel that disappears here takes its credential with it, so this is also
     how a device is unpaired along with its assignment. Re-pairing one screen
     and keeping the assignment is ``/{panel_id}/unpair`` below.
+
+    The body carries the ``version`` it was based on, and a save someone else
+    has moved past is refused with 409, as a dashboard's is. Two clients each
+    writing back the whole list would otherwise drop each other's panels, and
+    a dropped panel revokes its screen at once.
     """
     seen = set()
     for panel in body.panels:
@@ -314,9 +323,19 @@ async def save_panels(body: PanelsConfig) -> Any:
         # saving an arrangement — and reading it in a separate step from
         # writing it is exactly how an unpair in between was undone.
         with edit_lock:
-            stored = {p.id: p.nonce for p in read_config().panels}
+            stored = read_config()
+            if body.version != stored.version:
+                raise HTTPException(
+                    status_code=409,
+                    detail={
+                        "message": "Someone else saved the panels first",
+                        "current_version": stored.version,
+                    },
+                )
+            nonces = {p.id: p.nonce for p in stored.panels}
             for panel in body.panels:
-                panel.nonce = stored.get(panel.id, 0)
+                panel.nonce = nonces.get(panel.id, 0)
+            body.version = stored.version + 1
             write_config(body)
 
     await run_in_threadpool(_save)

backend/fluksio/flow/panels.py

@@ -96,6 +96,9 @@ class PanelsConfig(BaseModel):
     """Every panel this instance knows about."""
 
     panels: list[PanelDef] = Field(default_factory=list)
+    #: Bumped on every save; a save based on an older one is refused, as a
+    #: dashboard's is. A file written before there was one reads as 0.
+    version: int = 0
 
 
 #: Held across a read-modify-write of the panels file.

backend/tests/api/routes/test_panels.py

@@ -11,7 +11,11 @@ DASHBOARDS = f"{settings.API_V1_STR}/dashboards"
 
 
 def _panels(client: TestClient, headers: dict[str, str], config: dict) -> None:
-    response = client.put(f"{PREFIX}/", headers=headers, json=config)
+    """Replace the panels, based on whatever is stored now."""
+    version = client.get(f"{PREFIX}/", headers=headers).json()["version"]
+    response = client.put(
+        f"{PREFIX}/", headers=headers, json={**config, "version": version}
+    )
     assert response.status_code == 200, response.text
 
 
@@ -130,6 +134,39 @@ def test_duplicate_panel_is_refused(
     assert response.status_code == 422
 
 
+def test_a_stale_save_is_refused(
+    client: TestClient, superuser_token_headers: dict[str, str]
+) -> None:
+    """Two clients writing back the whole list must not drop each other's panels."""
+    read = client.get(f"{PREFIX}/", headers=superuser_token_headers).json()
+    base = read["version"]
+
+    saved = client.put(
+        f"{PREFIX}/",
+        headers=superuser_token_headers,
+        json={"panels": [*read["panels"], {"id": "mine"}], "version": base},
+    )
+    assert saved.status_code == 200, saved.text
+    assert saved.json()["version"] == base + 1
+
+    stale = client.put(
+        f"{PREFIX}/",
+        headers=superuser_token_headers,
+        json={"panels": [*read["panels"], {"id": "theirs"}], "version": base},
+    )
+    assert stale.status_code == 409
+    assert stale.json()["detail"]["current_version"] == base + 1
+
+    ids = [
+        p["id"]
+        for p in client.get(f"{PREFIX}/", headers=superuser_token_headers).json()[
+            "panels"
+        ]
+    ]
+    assert "mine" in ids
+    assert "theirs" not in ids
+
+
 def test_polling_needs_the_secret(client: TestClient) -> None:
     started = client.post(f"{PREFIX}/pair").json()
     assert len(started["code"]) == 6

docs/code/api.md

@@ -177,7 +177,7 @@ stored once. Node code uses `save_artifact` and `load_artifact` instead.
 | `POST` | `/dashboards/from-flow/{flow}` | create a batch flow's results dashboard |
 | `PUT` | `/dashboards/{name}` | save a draft |
 | `POST` | `/dashboards/{name}/publish`, `/dashboards/{name}/discard`, `/dashboards/{name}/rename` | publish, discard, or rename |
-| `GET`, `PUT` | `/panels/` | read or update panel assignments |
+| `GET`, `PUT` | `/panels/` | read or replace panel assignments; send the current `version` |
 | `POST` | `/panels/pair` | start device pairing |
 | `GET` | `/panels/pair/{code}` | poll pairing with its `secret` |
 | `POST` | `/panels/{id}/pair` | approve the pairing |

frontend/src/client/schemas.gen.ts

@@ -2933,6 +2933,11 @@ export const PanelsConfigSchema = {
             },
             type: 'array',
             title: 'Panels'
+        },
+        version: {
+            type: 'integer',
+            title: 'Version',
+            default: 0
         }
     },
     type: 'object',
@@ -2949,6 +2954,11 @@ export const PanelsPublicSchema = {
             type: 'array',
             title: 'Panels'
         },
+        version: {
+            type: 'integer',
+            title: 'Version',
+            default: 0
+        },
         frontend_host: {
             type: 'string',
             title: 'Frontend Host',

frontend/src/client/sdk.gen.ts

@@ -1946,6 +1946,11 @@ export class PanelsService {
      * A panel that disappears here takes its credential with it, so this is also
      * how a device is unpaired along with its assignment. Re-pairing one screen
      * and keeping the assignment is ``/{panel_id}/unpair`` below.
+     *
+     * The body carries the ``version`` it was based on, and a save someone else
+     * has moved past is refused with 409, as a dashboard's is. Two clients each
+     * writing back the whole list would otherwise drop each other's panels, and
+     * a dropped panel revokes its screen at once.
      * @param data The data for the request.
      * @param data.requestBody
      * @returns PanelsPublic Successful Response

frontend/src/client/types.gen.ts

@@ -994,6 +994,7 @@ export type direction = 'in' | 'out';
  */
 export type PanelsConfig = {
     panels?: Array<PanelDef>;
+    version?: number;
 };
 
 /**
@@ -1009,6 +1010,7 @@ export type PanelsConfig = {
  */
 export type PanelsPublic = {
     panels?: Array<PanelDef>;
+    version?: number;
     frontend_host?: string;
 };
 

frontend/src/components/Dashboard/PanelsDialog.tsx

@@ -107,9 +107,17 @@ export function PanelsDialog() {
   const host = config?.frontend_host ?? ""
 
   const write = (next: PanelsConfig) =>
-    save.mutate(next, {
-      onError: (error) => handleError.call(showErrorToast, error as ApiError),
-    })
+    save.mutate(
+      { ...next, version: config?.version },
+      {
+        onError: (error) =>
+          (error as ApiError).status === 409
+            ? showErrorToast(
+                "Someone else changed the panels first. Their version is shown now, so make the change again.",
+              )
+            : handleError.call(showErrorToast, error as ApiError),
+      },
+    )
 
   const replace = (id: string, panel: PanelDef) =>
     write({ panels: panels.map((p) => (p.id === id ? panel : p)) })

frontend/src/components/Dashboard/queries.ts

@@ -1,6 +1,7 @@
 import { useMutation, useQueryClient } from "@tanstack/react-query"
 
 import {
+  ApiError,
   type DashboardDef_Input,
   DashboardsService,
   MessagesService,
@@ -55,7 +56,13 @@ export const panelQueryOptions = (id: string) => ({
   queryFn: () => PanelsService.readPanel({ panelId: id }),
 })
 
-/** Replace the panels. Superuser-only on the server. */
+/**
+ * Replace the panels, carrying the version they were read at. Superuser-only
+ * on the server.
+ *
+ * A 409 means someone else saved first, so the list is read again and the
+ * next change is based on theirs.
+ */
 export function useSavePanels() {
   const queryClient = useQueryClient()
   return useMutation({
@@ -65,6 +72,10 @@ export function useSavePanels() {
       queryClient.setQueryData(panelKeys.all, saved)
       queryClient.invalidateQueries({ queryKey: panelKeys.all })
     },
+    onError: (error) => {
+      if (error instanceof ApiError && error.status === 409)
+        queryClient.invalidateQueries({ queryKey: panelKeys.all })
+    },
   })
 }
 

frontend/tests/panel.spec.ts

@@ -107,6 +107,7 @@ test.beforeAll(async ({ browser }) => {
         ...existing,
         { id: flowName, title: "Hall", dashboards: [first, second] },
       ],
+      version: config.version,
     },
   })
   await page.close()
@@ -116,7 +117,8 @@ test.afterAll(async ({ browser }) => {
   // Restoring the list is also what removes this spec's own panel.
   if (panels) {
     const page = await apiPage(browser)
-    await api(page, "/panels/", { method: "PUT", data: { panels } })
+    const { version } = await (await api(page, "/panels/")).json()
+    await api(page, "/panels/", { method: "PUT", data: { panels, version } })
     await page.close()
   }
   await deleteAll(browser, [
@@ -240,6 +242,7 @@ async function setTouch(page: Page, touch: boolean) {
       panels: (config.panels ?? []).map((panel: PanelDef) =>
         panel.id === flowName ? { ...panel, touch } : panel,
       ),
+      version: config.version,
     },
   })
 }