Bookmarks in DB - UI

e2e/sidebar-bookmarks.spec.ts

@@ -1,11 +1,11 @@
 import type { Page } from '@playwright/test';
 import { test, expect } from './support/fixtures.js';
 import { waitForHydration } from './support/helpers';
+import { resetBookmarks } from './support/bookmarks.js';
 
 test.describe('Sidebar bookmarks', () => {
   test.use({ locale: 'en-US' });
 
-  const BOOKMARK_ID = 'b6d6c7b6-8f24-4c4a-9c64-4f3b7c19e6a1';
   const BOOKMARK_URL = 'https://superset.example.com';
 
   // Bookmarks are also shown on the dashboard, so scope link lookups to the sidebar.
@@ -17,36 +17,20 @@ test.describe('Sidebar bookmarks', () => {
   );
 
   test.beforeEach(async ({ page }) => {
-    await page.addInitScript(
-      ({ bookmarkId, bookmarkUrl }) => {
-        localStorage.removeItem('dashboard_bookmarks');
-        localStorage.removeItem('sidebar_tools_open');
-        localStorage.setItem(
-          'dashboard_bookmarks',
-          JSON.stringify([
-            {
-              id: bookmarkId,
-              productId: 'superset',
-              name: 'Dashboards',
-              environment: '',
-              url: bookmarkUrl,
-              pinned: false,
-              createdAt: new Date().toISOString()
-            },
-            {
-              id: 'e7d8c7b6-9f35-4d4b-8d75-5f4c8d2fa7b2',
-              productId: 'airflow',
-              name: 'Pipelines',
-              environment: '',
-              url: 'https://airflow.example.com',
-              pinned: true,
-              createdAt: new Date().toISOString()
-            }
-          ])
-        );
-      },
-      { bookmarkId: BOOKMARK_ID, bookmarkUrl: BOOKMARK_URL }
-    );
+    await page.addInitScript(() => localStorage.removeItem('sidebar_tools_open'));
+    await resetBookmarks(page, [
+      { productId: 'superset', name: 'Dashboards', url: BOOKMARK_URL },
+      {
+        productId: 'airflow',
+        name: 'Pipelines',
+        url: 'https://airflow.example.com',
+        pinned: true
+      }
+    ]);
+  });
+
+  test.afterEach(async ({ page }) => {
+    await resetBookmarks(page);
   });
 
   test('shows pinned bookmarks in Favourites and the rest in Tools', async ({ page }) => {
@@ -81,28 +65,26 @@ test.describe('Sidebar bookmarks', () => {
     await expect(trinoLink.locator('..').getByRole('link')).toHaveCount(1);
   });
 
-  test('ignores invalid bookmarks stored in localStorage', async ({ page }) => {
-    await page.addInitScript(() => {
-      localStorage.setItem(
-        'dashboard_bookmarks',
-        JSON.stringify([
-          {
-            id: 'invalid-bookmark',
-            productId: 'superset',
-            name: 'Unsafe link',
-            environment: '',
-            url: 'javascript:alert(1)',
-            pinned: false,
-            createdAt: new Date().toISOString()
-          }
-        ])
-      );
-    });
-
+  test('follows pin changes made on the dashboard without a page reload', async ({ page }) => {
     await page.goto('/');
     await waitForHydration(page);
+    // Lost on a full page load, so it proves the sidebar updated in place.
+    await page.evaluate(() => ((window as unknown as { noReload: boolean }).noReload = true));
 
-    await expect(page.getByRole('link', { name: 'Unsafe link' })).not.toBeVisible();
+    // With Tools collapsed, only pinned bookmarks remain visible in the sidebar.
+    await page.getByRole('button', { name: 'Tools', exact: true }).click();
+    const dashboardsLink = sidebarNav(page).getByRole('link', { name: 'Dashboards' });
+    await expect(dashboardsLink).not.toBeVisible();
+
+    await page.getByRole('button', { name: 'Pin bookmark Dashboards' }).click();
+    await expect(dashboardsLink).toBeVisible();
+
+    await page.getByRole('button', { name: 'Unpin bookmark Dashboards' }).click();
+    await expect(dashboardsLink).not.toBeVisible();
+
+    expect(await page.evaluate(() => (window as unknown as { noReload?: boolean }).noReload)).toBe(
+      true
+    );
   });
 
   test('tools section is collapsible', async ({ page }) => {

e2e/support/bookmarks.ts

@@ -0,0 +1,30 @@
+import { expect, type Page } from '@playwright/test';
+
+interface BookmarkSeed {
+  productId: string;
+  name: string;
+  url: string;
+  environment?: string;
+  pinned?: boolean;
+}
+
+/**
+ * Replace the signed-in worker user's bookmarks through the API. Bookmarks are
+ * stored per user and each worker reuses one user for all of its tests, so
+ * bookmark tests reset them before and after: leftovers would show up in the
+ * sidebar of every later test in the worker (e.g. a "Dashboards" bookmark next
+ * to the "Dashboard" nav link).
+ */
+export async function resetBookmarks(page: Page, seeds: BookmarkSeed[] = []): Promise<void> {
+  const listed = await page.request.get('/api/bookmarks');
+  expect(listed.status()).toBe(200);
+  for (const { id } of (await listed.json()) as { id: string }[]) {
+    expect((await page.request.delete(`/api/bookmarks/${id}`)).status()).toBe(204);
+  }
+  for (const seed of seeds) {
+    const created = await page.request.post('/api/bookmarks', {
+      data: { environment: '', pinned: false, ...seed }
+    });
+    expect(created.status()).toBe(201);
+  }
+}

messages/de.json

@@ -508,6 +508,12 @@
   "bookmark_delete_confirm_message": "Möchten Sie \"{name}\" wirklich löschen? Dies kann nicht rückgängig gemacht werden.",
   "bookmark_delete": "Löschen",
   "bookmark_cancel": "Abbrechen",
+  "bookmark_save_error": "Das Lesezeichen konnte nicht gespeichert werden. Bitte versuchen Sie es erneut.",
+  "bookmark_delete_error": "Das Lesezeichen konnte nicht gelöscht werden. Bitte versuchen Sie es erneut.",
+  "bookmark_pin_error": "Die Anheftung von „{name}“ konnte nicht geändert werden. Bitte versuchen Sie es erneut.",
+  "bookmark_load_error": "Die Lesezeichen konnten nicht geladen werden. Bitte laden Sie die Seite später neu.",
+  "bookmark_limit_error": "Sie können bis zu {limit} Lesezeichen anlegen. Löschen Sie eines, um ein weiteres hinzuzufügen.",
+  "bookmark_saving": "Wird gespeichert…",
   "bookmark_product_custom": "Eigener Link",
   "bookmark_default_name_trino": "SQL-Editor",
   "bookmark_default_name_superset": "Dashboards",

messages/en.json

@@ -508,6 +508,12 @@
   "bookmark_delete_confirm_message": "Are you sure you want to delete \"{name}\"? This cannot be undone.",
   "bookmark_delete": "Delete",
   "bookmark_cancel": "Cancel",
+  "bookmark_save_error": "The bookmark could not be saved. Please try again.",
+  "bookmark_delete_error": "The bookmark could not be deleted. Please try again.",
+  "bookmark_pin_error": "Could not update the pin for \"{name}\". Please try again.",
+  "bookmark_load_error": "Bookmarks could not be loaded. Please reload the page later.",
+  "bookmark_limit_error": "You can have up to {limit} bookmarks. Delete one to add another.",
+  "bookmark_saving": "Saving…",
   "bookmark_product_custom": "Custom Link",
   "bookmark_default_name_trino": "SQL Editor",
   "bookmark_default_name_superset": "Dashboards",

src/lib/components/dashboard/AddBookmarkModal.svelte

@@ -3,10 +3,16 @@
   import Modal from '$lib/components/Modal.svelte';
   import { getProduct, PRODUCTS, type Product } from '$lib/dashboard/products';
   import ProductIcon from './ProductIcon.svelte';
-  import { addBookmark, removeBookmark, updateBookmark } from '$lib/dashboard/bookmarks.svelte.js';
   import {
+    addBookmark,
+    BookmarkRequestError,
+    removeBookmark,
+    updateBookmark
+  } from '$lib/dashboard/bookmarks-api.js';
+  import {
+    BOOKMARK_LIMIT,
     BOOKMARK_MAX_LENGTH,
-    bookmarkFormSchema,
+    bookmarkCreateSchema,
     bookmarkHostname,
     type Bookmark
   } from '$lib/dashboard/types';
@@ -31,6 +37,8 @@
   let wasOpen = false;
   let nameError = $state(false);
   let urlError = $state(false);
+  let saving = $state(false);
+  let requestError = $state<string | null>(null);
 
   let uid = $props.id();
 
@@ -45,6 +53,7 @@
     pinned = target?.pinned ?? false;
     nameError = false;
     urlError = false;
+    requestError = null;
   }
 
   // Only `open` is tracked; the bookmark is read once per open.
@@ -72,63 +81,92 @@
   }
 
   function isValidUrl(value: string): boolean {
-    return bookmarkFormSchema.shape.url.safeParse(value.trim()).success;
+    return bookmarkCreateSchema.shape.url.safeParse(value.trim()).success;
   }
 
   function handleUrlBlur() {
     if (url.trim()) urlError = !isValidUrl(url);
   }
 
-  function handleSubmit(event: SubmitEvent) {
+  /**
+   * Runs a bookmark request; closes the dialog on success, otherwise shows
+   * `errorMessage`, or the limit when the server rejects another bookmark.
+   */
+  async function save(request: () => Promise<void>, errorMessage: string) {
+    saving = true;
+    requestError = null;
+    try {
+      await request();
+      open = false;
+    } catch (err) {
+      requestError =
+        err instanceof BookmarkRequestError && err.status === 409
+          ? m.bookmark_limit_error({ limit: BOOKMARK_LIMIT })
+          : errorMessage;
+    } finally {
+      saving = false;
+    }
+  }
+
+  async function handleSubmit(event: SubmitEvent) {
     event.preventDefault();
+    if (saving) return;
     const values = {
       productId: selectedProduct.id,
       name: name.trim(),
       environment: environment.trim(),
       url: url.trim(),
       pinned
     };
-    const parsed = bookmarkFormSchema.safeParse(values);
+    const parsed = bookmarkCreateSchema.safeParse(values);
     if (!parsed.success) {
       nameError = !values.name;
       urlError = !isValidUrl(values.url);
       document.getElementById(nameError ? `${uid}-name` : `${uid}-url`)?.focus();
       return;
     }
 
-    if (bookmark) {
-      updateBookmark({ ...bookmark, ...parsed.data });
-    } else {
-      addBookmark({ createdAt: new Date().toISOString(), ...parsed.data });
-    }
-    open = false;
+    const target = bookmark;
+    await save(
+      () => (target ? updateBookmark(target.id, parsed.data) : addBookmark(parsed.data)),
+      m.bookmark_save_error()
+    );
+  }
+
+  function close() {
+    if (!saving) open = false;
   }
 
   // The confirmation opens on top of the edit dialog, so cancelling (or Escape)
   // returns to the form with the user's unsaved edits intact.
   function handleDeleteClick() {
+    if (saving) return;
     confirmDeleteOpen = true;
   }
 
   function handleDeleteCancel() {
     confirmDeleteOpen = false;
   }
 
-  function handleDeleteConfirm() {
-    if (!bookmark) return;
-    removeBookmark(bookmark.id);
+  async function handleDeleteConfirm() {
+    const target = bookmark;
+    if (!target || saving) return;
+    // Back to the edit dialog, which shows the error if the delete fails.
     confirmDeleteOpen = false;
-    open = false;
+    await save(() => removeBookmark(target.id), m.bookmark_delete_error());
   }
 </script>
 
-<Modal bind:open aria-labelledby="{uid}-title">
+<!-- Not closable while a request runs, so its result cannot land on a reopened form. -->
+<Modal bind:open dismissible={!saving} aria-labelledby="{uid}-title">
   <div class="modal-box max-w-2xl">
     <button
       type="button"
       class="btn btn-sm btn-circle btn-ghost absolute top-2 right-2"
+      class:btn-disabled={saving}
       aria-label={m.action_close()}
-      onclick={() => (open = false)}
+      aria-disabled={saving}
+      onclick={close}
     >
       <IconClose class="size-5" aria-hidden="true" />
     </button>
@@ -267,16 +305,42 @@
         </div>
       </div>
 
+      {#if requestError}
+        <div role="alert" class="alert alert-error mt-6 text-sm">{requestError}</div>
+      {/if}
+      <!-- The spinner is decorative; this announces the save to screen readers. -->
+      <p role="status" class="sr-only">{saving ? m.bookmark_saving() : ''}</p>
+
       <div class="modal-action">
         {#if isEditing}
-          <button type="button" class="btn btn-error mr-auto" onclick={handleDeleteClick}>
+          <button
+            type="button"
+            class="btn btn-error mr-auto"
+            class:btn-disabled={saving}
+            aria-disabled={saving}
+            onclick={handleDeleteClick}
+          >
             {m.bookmark_delete()}
           </button>
         {/if}
-        <button type="button" class="btn btn-ghost" onclick={() => (open = false)}>
+        <button
+          type="button"
+          class="btn btn-ghost"
+          class:btn-disabled={saving}
+          aria-disabled={saving}
+          onclick={close}
+        >
           {m.bookmark_cancel()}
         </button>
-        <button type="submit" class="btn btn-primary">
+        <button
+          type="submit"
+          class="btn btn-primary"
+          class:btn-disabled={saving}
+          aria-disabled={saving}
+        >
+          {#if saving}
+            <span class="loading loading-spinner loading-sm" aria-hidden="true"></span>
+          {/if}
           {isEditing ? m.bookmark_save_changes() : m.bookmark_add_title()}
         </button>
       </div>