From 003e6cde9657a0debf51406ecf6edb4f9b92d70f Mon Sep 17 00:00:00 2001 From: Johannes Fleck Date: Tue, 27 Oct 2026 15:35:46 +0100 Subject: [PATCH 1/7] refactor: extract the bookmark card into a component (#277) --- .../components/dashboard/BookmarkCard.svelte | 89 +++++++++++++++++++ .../dashboard/DashboardBookmarks.svelte | 80 ++--------------- src/lib/dashboard/types.ts | 7 +- 3 files changed, 102 insertions(+), 74 deletions(-) create mode 100644 src/lib/components/dashboard/BookmarkCard.svelte diff --git a/src/lib/components/dashboard/BookmarkCard.svelte b/src/lib/components/dashboard/BookmarkCard.svelte new file mode 100644 index 00000000..5303f639 --- /dev/null +++ b/src/lib/components/dashboard/BookmarkCard.svelte @@ -0,0 +1,89 @@ + + +
+ + +
+
+ + {bookmark.name} + +
+
+ + {product.name} + + {#if bookmark.environment} + + {bookmark.environment} + + {/if} +
+
+ + {bookmarkHostname(bookmark.url)} + +
+
+ + +
diff --git a/src/lib/components/dashboard/DashboardBookmarks.svelte b/src/lib/components/dashboard/DashboardBookmarks.svelte index eb7288ec..b9f1e202 100644 --- a/src/lib/components/dashboard/DashboardBookmarks.svelte +++ b/src/lib/components/dashboard/DashboardBookmarks.svelte @@ -2,15 +2,11 @@ import { SvelteSet } from 'svelte/reactivity'; import * as m from '$lib/paraglide/messages.js'; import IconAdd from 'virtual:icons/material-symbols/add'; - import IconEdit from 'virtual:icons/material-symbols/edit'; - import IconStar from 'virtual:icons/material-symbols/star'; - import IconStarOutline from 'virtual:icons/material-symbols/star-outline'; import AddBookmarkModal from '$lib/components/dashboard/AddBookmarkModal.svelte'; + import BookmarkCard from '$lib/components/dashboard/BookmarkCard.svelte'; import { updateBookmark } from '$lib/dashboard/bookmarks-api.js'; import { addToast } from '$lib/stores/toast.svelte.js'; - import { getProduct } from '$lib/dashboard/products'; - import ProductIcon from '$lib/components/dashboard/ProductIcon.svelte'; - import { bookmarkHostname, type Bookmark } from '$lib/dashboard/types'; + import type { Bookmark } from '$lib/dashboard/types'; let { bookmarks @@ -83,72 +79,12 @@ {:else if bookmarks.length > 0}
{#each bookmarks as bookmark (bookmark.id)} - {@const product = getProduct(bookmark.productId)} -
- - -
-
- - {bookmark.name} - -
-
- - {product.name} - - {#if bookmark.environment} - - {bookmark.environment} - - {/if} -
-
- - {bookmarkHostname(bookmark.url)} - -
-
- - -
+ togglePin(bookmark)} + onEdit={() => openEditBookmark(bookmark)} + /> {/each}
{:else} diff --git a/src/lib/dashboard/types.ts b/src/lib/dashboard/types.ts index 60c9f85f..3941ead6 100644 --- a/src/lib/dashboard/types.ts +++ b/src/lib/dashboard/types.ts @@ -18,8 +18,11 @@ export const bookmarkFormSchema = z.object({ pinned: z.boolean() }); -/** A stored bookmark as returned by the API; `createdAt` is an ISO timestamp. */ -export type Bookmark = z.infer & { id: string; createdAt: string }; +/** The fields every bookmark is displayed with; the id keys it in lists. */ +export type BookmarkLink = z.infer & { id: string }; + +/** A user's stored bookmark as returned by the API; `createdAt` is an ISO timestamp. */ +export type Bookmark = BookmarkLink & { createdAt: string }; /** * Body of a bookmark create request. Stored bookmarks keep any product id, so a From a5a55a87508bb40a9670282e89c927d4a70c094b Mon Sep 17 00:00:00 2001 From: Johannes Fleck Date: Tue, 27 Oct 2026 15:40:03 +0100 Subject: [PATCH 2/7] feat: show global bookmarks from STACKABLE_COCKPIT_GLOBAL_BOOKMARKS (#277) Administrators can configure read-only bookmarks for every user as a JSON array, also via the Helm chart (features.bookmarks.global). They are shown before the user's own bookmarks on the dashboard and in the sidebar, also without a signed-in user. Invalid configuration stops the server at startup. --- .env.example | 3 + .env.test | 2 + README.md | 6 ++ deploy/helm/cockpit/README.md | 1 + deploy/helm/cockpit/templates/deployment.yaml | 6 ++ deploy/helm/cockpit/values.yaml | 10 +++ e2e/bookmarks.spec.ts | 1 - e2e/global-bookmarks.spec.ts | 64 +++++++++++++++ src/hooks.server.ts | 2 + .../components/dashboard/BookmarkCard.svelte | 57 +++++++------- .../dashboard/DashboardBookmarks.svelte | 32 +++++--- .../DashboardBookmarks.svelte.spec.ts | 57 +++++++++++++- .../components/layout/sidebar/Sidebar.svelte | 9 ++- src/lib/dashboard/types.ts | 3 + .../server/dashboard/global-bookmarks.test.ts | 78 +++++++++++++++++++ src/lib/server/dashboard/global-bookmarks.ts | 57 ++++++++++++++ src/routes/(app)/+layout.server.ts | 8 +- src/routes/(app)/+layout.svelte | 2 +- src/routes/(app)/+page.svelte | 12 ++- src/routes/(app)/layout.server.test.ts | 34 +++++++- src/routes/(app)/page.svelte.spec.ts | 37 +++++++-- 21 files changed, 423 insertions(+), 58 deletions(-) create mode 100644 e2e/global-bookmarks.spec.ts create mode 100644 src/lib/server/dashboard/global-bookmarks.test.ts create mode 100644 src/lib/server/dashboard/global-bookmarks.ts diff --git a/.env.example b/.env.example index 2bd6f7f4..5520e989 100644 --- a/.env.example +++ b/.env.example @@ -56,6 +56,9 @@ DATABASE_PASSWORD=change-me # STACKABLE_COCKPIT_COMPLETION_ENABLED=false # Disable SQL editor code completion (default: true) # STACKABLE_COCKPIT_STORAGE_BROWSER_ENABLED=true # Enable S3/HDFS file browser (default: false) # PUBLIC_STACKABLE_COCKPIT_STORAGE_TRANSFER_ENABLED=true # Enables cut, copy, paste, rename and drag-and-drop move in the storage browser; also enforced by the transfer API (default: false) +# Bookmarks shown to every user (JSON array; productId, name and http(s) url required, environment and pinned optional). +# Invalid JSON or entries stop the server at startup. +# STACKABLE_COCKPIT_GLOBAL_BOOKMARKS='[{"productId":"superset","name":"Dashboards","url":"https://superset.example.com"}]' # Storage browser limits and settings. Invalid values stop the server at startup. # STACKABLE_COCKPIT_TEXT_PREVIEW_BYTES=262144 # Max bytes fetched for text, TSV and JSON previews; editable files are fetched completely (default: 262144) diff --git a/.env.test b/.env.test index 23c4b8ae..3c74a4f6 100644 --- a/.env.test +++ b/.env.test @@ -17,6 +17,8 @@ PUBLIC_STACKABLE_COCKPIT_DEFAULT_PAGE_SIZE=25 PUBLIC_STACKABLE_COCKPIT_MAX_RECENT_FILES=15 PUBLIC_STACKABLE_COCKPIT_UPLOAD_CONCURRENCY=3 PUBLIC_STACKABLE_COCKPIT_STORAGE_TRANSFER_ENABLED=true +# Shown on every page, so names must not clash with other tests' locators. +STACKABLE_COCKPIT_GLOBAL_BOOKMARKS='[{"productId":"opensearch","name":"Cluster Logs","environment":"prod","url":"https://logs.example.com","pinned":true},{"productId":"custom","name":"Team Wiki","url":"https://wiki.example.com"}]' # One object at a time, so the cancel E2E test can stop a move mid-way. STACKABLE_COCKPIT_STORAGE_TRANSFER_CONCURRENCY=1 GARAGE_ADMIN_URL=http://localhost:30902 diff --git a/README.md b/README.md index 1e2b6c16..d4f7dda8 100644 --- a/README.md +++ b/README.md @@ -124,6 +124,12 @@ Invalid values (e.g. `abc` for an integer) stop the server at startup with an er | ------------------------------------------------ | --------- | ------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------- | | `STACKABLE_COCKPIT_STORAGE_TRANSFER_CONCURRENCY` | `integer` | `8` | Objects copied at once within one copy, move or rename request. Copies run server-side in the storage backend (S3 CopyObject); no data streams through the app. | `STACKABLE_COCKPIT_STORAGE_TRANSFER_CONCURRENCY=4` | +### Dashboard Bookmarks + +| Variable | Type | Default | Description | Example | +| ------------------------------------ | ------------ | ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------- | +| `STACKABLE_COCKPIT_GLOBAL_BOOKMARKS` | `JSON` array | none | Bookmarks shown to every user, before their own, on the dashboard and in the sidebar. Users cannot change them. Each entry needs `productId` (e.g. `trino`, `superset`, or `custom`; unknown ids show as a custom link), `name` (up to 40 characters) and an HTTP(S) `url`; `environment` and `pinned` are optional. Invalid JSON or entries stop the server at startup. | `STACKABLE_COCKPIT_GLOBAL_BOOKMARKS='[{"productId":"superset","name":"Dashboards","url":"https://superset.example.com"}]'` | + ### Request Body Size | Variable | Type | Default | Description | Example | diff --git a/deploy/helm/cockpit/README.md b/deploy/helm/cockpit/README.md index 245972ac..c0a5f106 100644 --- a/deploy/helm/cockpit/README.md +++ b/deploy/helm/cockpit/README.md @@ -122,6 +122,7 @@ All feature parameters are optional. Omit them to use the application defaults. | `features.trino.completion.enabled` | Enable SQL code completion. | `true` (application default) | | `features.storage.enabled` | Show the S3/HDFS file browser and enable routes under `/storage`. | `false` (application default) | | `features.storage.transfer` | Show Cut, Copy, Paste and Rename and allow drag-and-drop move. The transfer API rejects copies, moves and renames while this is disabled. | `false` (application default) | +| `features.bookmarks.global` | Bookmarks shown to every user, before their own; users cannot change them. A list of entries with `productId` (e.g. `trino`, `superset` or `custom`; unknown ids show as a custom link), `name` (up to 40 characters) and an HTTP(S) `url`, plus optional `environment` and `pinned`. Invalid entries stop the application at startup. | none | ### Storage Browser Parameters diff --git a/deploy/helm/cockpit/templates/deployment.yaml b/deploy/helm/cockpit/templates/deployment.yaml index 60ed19bf..8878cfa7 100644 --- a/deploy/helm/cockpit/templates/deployment.yaml +++ b/deploy/helm/cockpit/templates/deployment.yaml @@ -140,6 +140,12 @@ spec: - name: STACKABLE_COCKPIT_DOWNLOAD_HISTORY_RETENTION_DAYS value: {{ include "cockpit.envValue" .Values.storage.downloadHistoryRetentionDays }} {{- end }} + {{- with (index $features "bookmarks") }} + {{- with .global }} + - name: STACKABLE_COCKPIT_GLOBAL_BOOKMARKS + value: {{ toJson . | quote }} + {{- end }} + {{- end }} {{- if .Values.trino.queryTtl }} - name: STACKABLE_COCKPIT_QUERY_TTL value: {{ .Values.trino.queryTtl | int64 |quote }} diff --git a/deploy/helm/cockpit/values.yaml b/deploy/helm/cockpit/values.yaml index 0e810288..28b23d54 100644 --- a/deploy/helm/cockpit/values.yaml +++ b/deploy/helm/cockpit/values.yaml @@ -114,6 +114,16 @@ terminationGracePeriodSeconds: 30 # # Show Cut, Copy, Paste and Rename and allow drag-and-drop move. The transfer API rejects # # copies, moves and renames while disabled. Default: false (application default). # transfer: false +# bookmarks: +# # Bookmarks shown to every user, before their own. Users cannot change them. +# # productId (e.g. trino, superset or custom; unknown ids show as a custom link), +# # name (up to 40 characters) and an http(s) url are required; environment and +# # pinned are optional. Invalid entries stop the application at startup. Default: none. +# global: +# - productId: superset +# name: Dashboards +# url: https://superset.example.com +# environment: prod logging: level: "" diff --git a/e2e/bookmarks.spec.ts b/e2e/bookmarks.spec.ts index 61260a00..a0ea1727 100644 --- a/e2e/bookmarks.spec.ts +++ b/e2e/bookmarks.spec.ts @@ -147,6 +147,5 @@ test.describe('Dashboard bookmarks', () => { await expect(confirmDialog).toBeHidden(); await expect(editDialog).toBeHidden(); await expect(page.getByLabel('Bookmarks').getByText('Dashboards')).not.toBeVisible(); - await expect(page.getByText(/no bookmarks yet/i)).toBeVisible(); }); }); diff --git a/e2e/global-bookmarks.spec.ts b/e2e/global-bookmarks.spec.ts new file mode 100644 index 00000000..1776c441 --- /dev/null +++ b/e2e/global-bookmarks.spec.ts @@ -0,0 +1,64 @@ +import type { Page } from '@playwright/test'; +import { test, expect } from './support/fixtures.js'; +import { waitForHydration } from './support/helpers'; +import { resetBookmarks } from './support/bookmarks.js'; + +// The global bookmarks come from STACKABLE_COCKPIT_GLOBAL_BOOKMARKS in .env.test. +test.describe('Global bookmarks', () => { + test.use({ locale: 'en-US' }); + + const section = (page: Page) => page.getByLabel('Bookmarks'); + const sidebarNav = (page: Page) => page.getByRole('navigation', { name: 'Main navigation' }); + + test.beforeEach(async ({ page }) => { + await resetBookmarks(page, [ + { productId: 'superset', name: 'Dashboards', url: 'https://superset.example.com' } + ]); + }); + + test.afterEach(async ({ page }) => { + await resetBookmarks(page); + }); + + test('shows global bookmarks before the user’s own, read-only', async ({ page }) => { + await page.goto('/'); + await waitForHydration(page); + + // Only the title links: the image links are hidden from assistive technology. + await expect(section(page).getByRole('link')).toHaveText([ + 'Cluster Logs', + 'Team Wiki', + 'Dashboards' + ]); + await expect(section(page).getByText('3 links')).toBeVisible(); + await expect(section(page).getByText('prod', { exact: true })).toBeVisible(); + + for (const name of ['Cluster Logs', 'Team Wiki']) { + await expect(section(page).getByRole('button', { name })).toHaveCount(0); + } + await expect( + section(page).getByRole('button', { name: 'Edit bookmark Dashboards' }) + ).toBeVisible(); + }); + + test('lists global bookmarks in the sidebar by their configured pin', async ({ + page, + viewport + }) => { + test.skip((viewport?.width ?? 1280) < 1024, 'Sidebar is off-canvas on small screens'); + await page.addInitScript(() => localStorage.removeItem('sidebar_tools_open')); + await page.goto('/'); + await waitForHydration(page); + + await expect(sidebarNav(page).getByRole('link', { name: 'Cluster Logs' })).toHaveAttribute( + 'href', + 'https://logs.example.com' + ); + await expect(sidebarNav(page).getByRole('link', { name: 'Team Wiki' })).toBeVisible(); + + // Collapsing Tools hides the unpinned global bookmark but keeps the pinned one. + await page.getByRole('button', { name: 'Tools', exact: true }).click(); + await expect(sidebarNav(page).getByRole('link', { name: 'Team Wiki' })).not.toBeVisible(); + await expect(sidebarNav(page).getByRole('link', { name: 'Cluster Logs' })).toBeVisible(); + }); +}); diff --git a/src/hooks.server.ts b/src/hooks.server.ts index 6df0fe73..1d826708 100644 --- a/src/hooks.server.ts +++ b/src/hooks.server.ts @@ -16,6 +16,7 @@ import '$lib/server/storage/config.js'; import { storageEncryptionKey } from '$lib/server/storage/encryption-key.js'; import { notConnected } from '$lib/server/storage/not-connected.js'; import { runMigrations } from '$lib/server/migrate.js'; +import { globalBookmarks } from '$lib/server/dashboard/global-bookmarks.js'; // Allow self-signed TLS certificates in development (e.g. local Trino with self-signed certs). if (dev) { @@ -125,6 +126,7 @@ export const handle = sequence( * detected immediately rather than on the first request that uses the key. */ export const init: ServerInit = async () => { + globalBookmarks(); // throws immediately if the env var is invalid if (!(await runMigrations())) { throw new Error('Database migrations failed'); } diff --git a/src/lib/components/dashboard/BookmarkCard.svelte b/src/lib/components/dashboard/BookmarkCard.svelte index 5303f639..7f4bc90c 100644 --- a/src/lib/components/dashboard/BookmarkCard.svelte +++ b/src/lib/components/dashboard/BookmarkCard.svelte @@ -15,8 +15,9 @@ }: { bookmark: BookmarkLink; pinPending?: boolean; - onTogglePin: () => void; - onEdit: () => void; + /** Without handlers the card is read-only (global bookmarks). */ + onTogglePin?: () => void; + onEdit?: () => void; } = $props(); const product = $derived(getProduct(bookmark.productId)); @@ -62,28 +63,32 @@ - - + {#if onTogglePin} + + {/if} + {#if onEdit} + + {/if} diff --git a/src/lib/components/dashboard/DashboardBookmarks.svelte b/src/lib/components/dashboard/DashboardBookmarks.svelte index b9f1e202..340dc1e5 100644 --- a/src/lib/components/dashboard/DashboardBookmarks.svelte +++ b/src/lib/components/dashboard/DashboardBookmarks.svelte @@ -6,19 +6,27 @@ import BookmarkCard from '$lib/components/dashboard/BookmarkCard.svelte'; import { updateBookmark } from '$lib/dashboard/bookmarks-api.js'; import { addToast } from '$lib/stores/toast.svelte.js'; - import type { Bookmark } from '$lib/dashboard/types'; + import type { Bookmark, GlobalBookmark } from '$lib/dashboard/types'; let { - bookmarks + bookmarks, + globalBookmarks = [], + loadFailed = false }: { - /** Null when loading them failed: the section says so instead of listing them. */ + /** The user's bookmarks; null when unavailable, which leaves only the global ones. */ bookmarks: Bookmark[] | null; + /** Read-only bookmarks for everyone, shown before the user's. */ + globalBookmarks?: GlobalBookmark[]; + /** Loading the user's bookmarks failed: the section says so. */ + loadFailed?: boolean; } = $props(); let addModalOpen = $state(false); let editingBookmark: Bookmark | null = $state(null); const pinning = new SvelteSet(); + const count = $derived(globalBookmarks.length + (bookmarks?.length ?? 0)); + function openEditBookmark(bookmark: Bookmark) { editingBookmark = bookmark; addModalOpen = true; @@ -56,9 +64,9 @@ > {m.bookmark_section_title()} - {#if bookmarks?.length} + {#if count > 0} - {m.bookmark_count_links({ count: bookmarks.length })} + {m.bookmark_count_links({ count })} {/if} @@ -74,11 +82,15 @@ {/if} - {#if !bookmarks} -
{m.bookmark_load_error()}
- {:else if bookmarks.length > 0} + {#if loadFailed} +
{m.bookmark_load_error()}
+ {/if} + {#if count > 0}
- {#each bookmarks as bookmark (bookmark.id)} + {#each globalBookmarks as bookmark (bookmark.id)} + + {/each} + {#each bookmarks ?? [] as bookmark (bookmark.id)} {/each}
- {:else} + {:else if !loadFailed}

{m.dashboard_bookmarks_empty()}

diff --git a/src/lib/components/dashboard/DashboardBookmarks.svelte.spec.ts b/src/lib/components/dashboard/DashboardBookmarks.svelte.spec.ts index fb4cf75e..da29fddf 100644 --- a/src/lib/components/dashboard/DashboardBookmarks.svelte.spec.ts +++ b/src/lib/components/dashboard/DashboardBookmarks.svelte.spec.ts @@ -1,7 +1,7 @@ import { page } from 'vitest/browser'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { render } from 'vitest-browser-svelte'; -import type { Bookmark } from '$lib/dashboard/types'; +import type { Bookmark, GlobalBookmark } from '$lib/dashboard/types'; import DashboardBookmarks from './DashboardBookmarks.svelte'; const { updateBookmark, addToast } = vi.hoisted(() => ({ @@ -27,6 +27,15 @@ const bookmark: Bookmark = { createdAt: '2026-01-01T00:00:00.000Z' }; +const globalBookmark: GlobalBookmark = { + id: 'global-0', + productId: 'custom', + name: 'Team Wiki', + environment: '', + url: 'https://wiki.example.com', + pinned: true +}; + describe('DashboardBookmarks', () => { beforeEach(() => vi.clearAllMocks()); @@ -41,7 +50,7 @@ describe('DashboardBookmarks', () => { }); it('shows a message instead of the list, without adding, when loading failed', async () => { - render(DashboardBookmarks, { bookmarks: null }); + render(DashboardBookmarks, { bookmarks: null, loadFailed: true }); await expect .element(page.getByRole('heading', { level: 3, name: 'Bookmarks' })) @@ -55,6 +64,19 @@ describe('DashboardBookmarks', () => { await expect.element(page.getByText(/no bookmarks yet/i)).not.toBeInTheDocument(); }); + it('still shows the global bookmarks when loading the user’s failed', async () => { + render(DashboardBookmarks, { + bookmarks: null, + globalBookmarks: [globalBookmark], + loadFailed: true + }); + + await expect + .element(page.getByText('Bookmarks could not be loaded. Please reload the page later.')) + .toBeVisible(); + await expect.element(page.getByRole('link', { name: 'Team Wiki' })).toBeVisible(); + }); + it('shows an empty state when there are no bookmarks', async () => { render(DashboardBookmarks, { bookmarks: [] }); @@ -108,6 +130,37 @@ describe('DashboardBookmarks', () => { } }); + it('shows global bookmarks first, read-only, and counts them', async () => { + render(DashboardBookmarks, { bookmarks: [bookmark], globalBookmarks: [globalBookmark] }); + + const titles = page + .getByRole('link') + .elements() + .map((link) => link.textContent?.trim()); + expect(titles).toEqual(['Team Wiki', 'Shared Dashboard']); + await expect.element(page.getByText('2 links')).toBeVisible(); + await expect + .element(page.getByRole('button', { name: 'Edit bookmark Team Wiki' })) + .not.toBeInTheDocument(); + await expect + .element(page.getByRole('button', { name: /pin bookmark Team Wiki/i })) + .not.toBeInTheDocument(); + await expect + .element(page.getByRole('button', { name: 'Edit bookmark Shared Dashboard' })) + .toBeVisible(); + }); + + it('shows only the global bookmarks, without adding, when the user’s are unavailable', async () => { + render(DashboardBookmarks, { bookmarks: null, globalBookmarks: [globalBookmark] }); + + await expect.element(page.getByRole('link', { name: 'Team Wiki' })).toBeVisible(); + await expect + .element(page.getByRole('button', { name: /add bookmark/i })) + .not.toBeInTheDocument(); + await expect.element(page.getByText(/bookmarks are added by hand/i)).not.toBeInTheDocument(); + await expect.element(page.getByText(/no bookmarks yet/i)).not.toBeInTheDocument(); + }); + it('opens the add dialog', async () => { render(DashboardBookmarks, { bookmarks: [] }); diff --git a/src/lib/components/layout/sidebar/Sidebar.svelte b/src/lib/components/layout/sidebar/Sidebar.svelte index 25a68e79..ead78e8d 100644 --- a/src/lib/components/layout/sidebar/Sidebar.svelte +++ b/src/lib/components/layout/sidebar/Sidebar.svelte @@ -12,7 +12,7 @@ import { getPlatformSection, getToolsSection } from './nav-items.js'; import { getProduct } from '$lib/dashboard/products'; import ProductIcon from '$lib/components/dashboard/ProductIcon.svelte'; - import type { Bookmark } from '$lib/dashboard/types'; + import type { BookmarkLink } from '$lib/dashboard/types'; import stackableLogo from '$lib/logos/stackable.svg?enhanced'; import TooltipTrigger from '$lib/components/TooltipTrigger.svelte'; @@ -25,7 +25,8 @@ collapsed?: boolean; mobileOpen?: boolean; storageBrowserEnabled?: boolean; - bookmarks?: Bookmark[]; + /** Global bookmarks first, then the user's; both are plain links here. */ + bookmarks?: BookmarkLink[]; } = $props(); const platformSection = $derived(getPlatformSection()); @@ -154,12 +155,12 @@ {/snippet} -{#snippet bookmarkIcon(bookmark: Bookmark)} +{#snippet bookmarkIcon(bookmark: BookmarkLink)} {@const product = getProduct(bookmark.productId)} {/snippet} -{#snippet bookmarkItem(bookmark: Bookmark)} +{#snippet bookmarkItem(bookmark: BookmarkLink)}
  • & { id: string }; +/** A bookmark configured for everyone by the administrator; read-only. */ +export type GlobalBookmark = BookmarkLink; + /** A user's stored bookmark as returned by the API; `createdAt` is an ISO timestamp. */ export type Bookmark = BookmarkLink & { createdAt: string }; diff --git a/src/lib/server/dashboard/global-bookmarks.test.ts b/src/lib/server/dashboard/global-bookmarks.test.ts new file mode 100644 index 00000000..e9826eff --- /dev/null +++ b/src/lib/server/dashboard/global-bookmarks.test.ts @@ -0,0 +1,78 @@ +import { describe, it, expect, vi } from 'vitest'; + +vi.mock('$lib/server/logging', () => import('$lib/test-utils/mock-logger.js')); + +import { parseGlobalBookmarks } from './global-bookmarks.js'; + +const json = (value: unknown) => JSON.stringify(value); + +describe('parseGlobalBookmarks', () => { + it('has no global bookmarks when the variable is unset or blank', () => { + expect(parseGlobalBookmarks(undefined)).toEqual([]); + expect(parseGlobalBookmarks(' ')).toEqual([]); + }); + + it('parses bookmarks, numbers their ids and defaults the optional fields', () => { + const bookmarks = parseGlobalBookmarks( + json([ + { + productId: 'superset', + name: 'Dashboards', + environment: 'prod', + url: 'https://superset.example.com', + pinned: true + }, + { productId: 'custom', name: 'Team Wiki', url: 'https://wiki.example.com' } + ]) + ); + + expect(bookmarks).toEqual([ + { + id: 'global-0', + productId: 'superset', + name: 'Dashboards', + environment: 'prod', + url: 'https://superset.example.com', + pinned: true + }, + { + id: 'global-1', + productId: 'custom', + name: 'Team Wiki', + environment: '', + url: 'https://wiki.example.com', + pinned: false + } + ]); + }); + + it('rejects invalid JSON', () => { + expect(() => parseGlobalBookmarks('[{')).toThrow(/not valid JSON/); + }); + + it('rejects anything but an array', () => { + expect(() => parseGlobalBookmarks(json({ name: 'Wiki' }))).toThrow(/is invalid/); + }); + + it.each([ + ['a non-HTTP URL', { url: 'javascript:alert(1)' }, 'url'], + ['a missing name', { name: undefined }, 'name'], + ['a name above the length limit', { name: 'x'.repeat(41) }, 'name'], + ['a misspelt key', { pined: true }, 'pined'] + ])('rejects %s and names the entry and field', (_, override, field) => { + const entry = { productId: 'custom', name: 'Wiki', url: 'https://wiki.example.com' }; + + const parse = () => parseGlobalBookmarks(json([entry, { ...entry, ...override }])); + + expect(parse).toThrow('[1]'); + expect(parse).toThrow(field); + }); + + it('does not repeat the configured values in the error', () => { + const url = 'ftp://user:secret@files.example.com'; + + expect(() => parseGlobalBookmarks(json([{ productId: 'custom', name: 'Files', url }]))).toThrow( + expect.objectContaining({ message: expect.not.stringContaining('secret') }) + ); + }); +}); diff --git a/src/lib/server/dashboard/global-bookmarks.ts b/src/lib/server/dashboard/global-bookmarks.ts new file mode 100644 index 00000000..9a4f612f --- /dev/null +++ b/src/lib/server/dashboard/global-bookmarks.ts @@ -0,0 +1,57 @@ +import { z } from 'zod'; +import { env } from '$env/dynamic/private'; +import { logger } from '$lib/server/logging'; +import { bookmarkFormSchema, type GlobalBookmark } from '$lib/dashboard/types'; + +const log = logger.child({ module: 'global-bookmarks' }); + +// Strict, so a misspelt key such as "pined" fails instead of being ignored. An +// unknown productId is accepted and shown as a custom link, as for user bookmarks. +const globalBookmarksSchema = z.array( + z.strictObject({ + ...bookmarkFormSchema.shape, + environment: bookmarkFormSchema.shape.environment.default(''), + pinned: bookmarkFormSchema.shape.pinned.default(false) + }) +); + +/** + * Parse `STACKABLE_COCKPIT_GLOBAL_BOOKMARKS`: a JSON array of bookmarks shown + * to every user. Throws on invalid JSON or entries; the error names the failing + * entries and fields but not their values, as URLs can contain credentials. + */ +export function parseGlobalBookmarks(value: string | undefined): GlobalBookmark[] { + if (!value?.trim()) return []; + let json: unknown; + try { + json = JSON.parse(value); + } catch { + throw new Error('STACKABLE_COCKPIT_GLOBAL_BOOKMARKS is not valid JSON'); + } + const parsed = globalBookmarksSchema.safeParse(json); + if (!parsed.success) { + throw new Error( + `STACKABLE_COCKPIT_GLOBAL_BOOKMARKS is invalid:\n${z.prettifyError(parsed.error)}` + ); + } + return parsed.data.map((bookmark, index) => ({ id: `global-${index}`, ...bookmark })); +} + +let cached: GlobalBookmark[] | undefined; + +/** + * The configured global bookmarks. Parsed on first call and cached; called at + * server start so an invalid configuration stops the server. A function rather + * than a module-level constant so tests can import this file without the env var. + */ +export function globalBookmarks(): GlobalBookmark[] { + if (cached) return cached; + try { + cached = parseGlobalBookmarks(env.STACKABLE_COCKPIT_GLOBAL_BOOKMARKS); + } catch (err) { + log.error({ err }, 'Invalid global bookmarks configuration'); + throw err; + } + log.info({ count: cached.length }, 'Loaded global bookmarks'); + return cached; +} diff --git a/src/routes/(app)/+layout.server.ts b/src/routes/(app)/+layout.server.ts index 2c13f544..dd3329e7 100644 --- a/src/routes/(app)/+layout.server.ts +++ b/src/routes/(app)/+layout.server.ts @@ -1,12 +1,18 @@ import type { LayoutServerLoad } from './$types'; import { storageBrowserEnabled } from '$lib/server/feature-flags.js'; import { listBookmarks } from '$lib/server/dashboard/bookmarks-db.js'; +import { globalBookmarks } from '$lib/server/dashboard/global-bookmarks.js'; import { dashboardBookmarksTotal } from '$lib/server/metrics.js'; import { BOOKMARKS_DEPENDENCY, type Bookmark } from '$lib/dashboard/types'; export const load: LayoutServerLoad = async ({ locals, depends }) => { depends(BOOKMARKS_DEPENDENCY); - return { user: locals.user, storageBrowserEnabled, ...(await loadBookmarks(locals)) }; + return { + user: locals.user, + storageBrowserEnabled, + globalBookmarks: globalBookmarks(), + ...(await loadBookmarks(locals)) + }; }; /** diff --git a/src/routes/(app)/+layout.svelte b/src/routes/(app)/+layout.svelte index 5aa94105..ff81f039 100644 --- a/src/routes/(app)/+layout.svelte +++ b/src/routes/(app)/+layout.svelte @@ -78,7 +78,7 @@ bind:collapsed={sidebarCollapsed} bind:mobileOpen storageBrowserEnabled={data.storageBrowserEnabled} - bookmarks={data.bookmarks ?? []} + bookmarks={[...data.globalBookmarks, ...(data.bookmarks ?? [])]} />
    diff --git a/src/routes/(app)/+page.svelte b/src/routes/(app)/+page.svelte index 99b18442..d459a9ba 100644 --- a/src/routes/(app)/+page.svelte +++ b/src/routes/(app)/+page.svelte @@ -8,9 +8,13 @@

    {m.dashboard_welcome()}

    - - {#if data.bookmarks || data.bookmarksFailed} - + + {#if data.bookmarks || data.bookmarksFailed || data.globalBookmarks.length > 0} + {/if}
    diff --git a/src/routes/(app)/layout.server.test.ts b/src/routes/(app)/layout.server.test.ts index 885c95ee..3591be61 100644 --- a/src/routes/(app)/layout.server.test.ts +++ b/src/routes/(app)/layout.server.test.ts @@ -4,9 +4,23 @@ const { listBookmarks } = vi.hoisted(() => ({ listBookmarks: vi.fn() })); vi.mock('$lib/server/dashboard/bookmarks-db.js', () => ({ listBookmarks })); vi.mock('$lib/server/feature-flags.js', () => ({ storageBrowserEnabled: false })); +vi.mock('$lib/server/dashboard/global-bookmarks.js', () => ({ + globalBookmarks: () => [GLOBAL_BOOKMARK] +})); import { load } from './+layout.server.js'; +const { GLOBAL_BOOKMARK } = vi.hoisted(() => ({ + GLOBAL_BOOKMARK: { + id: 'global-0', + productId: 'custom', + name: 'Team Wiki', + environment: '', + url: 'https://wiki.example.com', + pinned: true + } +})); + const BOOKMARK = { id: '00000000-0000-0000-0000-000000000001', productId: 'trino', @@ -33,15 +47,23 @@ describe('(app) layout load', () => { const data = await load(event); - expect(data).toMatchObject({ bookmarks: [BOOKMARK], bookmarksFailed: false }); + expect(data).toMatchObject({ + bookmarks: [BOOKMARK], + bookmarksFailed: false, + globalBookmarks: [GLOBAL_BOOKMARK] + }); expect(listBookmarks).toHaveBeenCalledWith('user-1'); expect(event.depends).toHaveBeenCalledWith('app:bookmarks'); }); - it('has no bookmarks without a signed-in user', async () => { + it('has only the global bookmarks without a signed-in user', async () => { const data = await load(mockEvent(undefined)); - expect(data).toMatchObject({ bookmarks: null, bookmarksFailed: false }); + expect(data).toMatchObject({ + bookmarks: null, + bookmarksFailed: false, + globalBookmarks: [GLOBAL_BOOKMARK] + }); expect(listBookmarks).not.toHaveBeenCalled(); }); @@ -51,7 +73,11 @@ describe('(app) layout load', () => { const data = await load(event); - expect(data).toMatchObject({ bookmarks: null, bookmarksFailed: true }); + expect(data).toMatchObject({ + bookmarks: null, + bookmarksFailed: true, + globalBookmarks: [GLOBAL_BOOKMARK] + }); expect(event.locals.logger.error).toHaveBeenCalled(); }); }); diff --git a/src/routes/(app)/page.svelte.spec.ts b/src/routes/(app)/page.svelte.spec.ts index 2f1ce1a8..e3c27a0a 100644 --- a/src/routes/(app)/page.svelte.spec.ts +++ b/src/routes/(app)/page.svelte.spec.ts @@ -1,12 +1,30 @@ import { page } from 'vitest/browser'; import { describe, expect, it } from 'vitest'; import { render } from 'vitest-browser-svelte'; -import type { Bookmark } from '$lib/dashboard/types'; +import type { Bookmark, GlobalBookmark } from '$lib/dashboard/types'; import Page from './+page.svelte'; -const renderPage = (bookmarks: Bookmark[] | null = [], bookmarksFailed = false) => +const globalBookmark: GlobalBookmark = { + id: 'global-0', + productId: 'custom', + name: 'Team Wiki', + environment: '', + url: 'https://wiki.example.com', + pinned: false +}; + +const renderPage = ( + bookmarks: Bookmark[] | null = [], + { globalBookmarks = [] as GlobalBookmark[], bookmarksFailed = false } = {} +) => render(Page, { - data: { user: undefined, storageBrowserEnabled: false, bookmarks, bookmarksFailed } + data: { + user: undefined, + storageBrowserEnabled: false, + bookmarks, + bookmarksFailed, + globalBookmarks + } } as unknown as Parameters>[1]); describe('/(app)/+page.svelte', () => { @@ -23,7 +41,7 @@ describe('/(app)/+page.svelte', () => { }); it('says so when loading the bookmarks failed', async () => { - renderPage(null, true); + renderPage(null, { bookmarksFailed: true }); await expect .element(page.getByText('Bookmarks could not be loaded. Please reload the page later.')) @@ -33,7 +51,16 @@ describe('/(app)/+page.svelte', () => { .not.toBeInTheDocument(); }); - it('hides everything bookmark-related without a signed-in user', async () => { + it('shows the global bookmarks when the user’s are unavailable', async () => { + renderPage(null, { globalBookmarks: [globalBookmark] }); + + await expect.element(page.getByRole('link', { name: 'Team Wiki' })).toBeVisible(); + await expect + .element(page.getByRole('button', { name: /add bookmark/i })) + .not.toBeInTheDocument(); + }); + + it('hides everything bookmark-related without a user or global bookmarks', async () => { renderPage(null); await expect.element(page.getByRole('heading', { level: 2 })).toBeInTheDocument(); From 379432fe1ff1a2a4ee1f80703ee77613e68ebf40 Mon Sep 17 00:00:00 2001 From: Johannes Fleck Date: Tue, 27 Oct 2026 16:47:46 +0100 Subject: [PATCH 3/7] fix: reject a JSON string for global bookmarks in the Helm chart (#277) features.bookmarks.global must be a YAML list. A JSON string, e.g. copied from the README, was encoded again and stopped the application at startup; Helm now fails at render time with a clear message instead. --- deploy/helm/cockpit/README.md | 2 +- deploy/helm/cockpit/templates/deployment.yaml | 3 +++ deploy/helm/cockpit/values.yaml | 3 ++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/deploy/helm/cockpit/README.md b/deploy/helm/cockpit/README.md index c0a5f106..2524cc90 100644 --- a/deploy/helm/cockpit/README.md +++ b/deploy/helm/cockpit/README.md @@ -122,7 +122,7 @@ All feature parameters are optional. Omit them to use the application defaults. | `features.trino.completion.enabled` | Enable SQL code completion. | `true` (application default) | | `features.storage.enabled` | Show the S3/HDFS file browser and enable routes under `/storage`. | `false` (application default) | | `features.storage.transfer` | Show Cut, Copy, Paste and Rename and allow drag-and-drop move. The transfer API rejects copies, moves and renames while this is disabled. | `false` (application default) | -| `features.bookmarks.global` | Bookmarks shown to every user, before their own; users cannot change them. A list of entries with `productId` (e.g. `trino`, `superset` or `custom`; unknown ids show as a custom link), `name` (up to 40 characters) and an HTTP(S) `url`, plus optional `environment` and `pinned`. Invalid entries stop the application at startup. | none | +| `features.bookmarks.global` | Bookmarks shown to every user, before their own; users cannot change them. A list of entries with `productId` (e.g. `trino`, `superset` or `custom`; unknown ids show as a custom link), `name` (up to 40 characters) and an HTTP(S) `url`, plus optional `environment` and `pinned`. Must be a YAML list, not a JSON string. Invalid entries stop the application at startup. | none | ### Storage Browser Parameters diff --git a/deploy/helm/cockpit/templates/deployment.yaml b/deploy/helm/cockpit/templates/deployment.yaml index 8878cfa7..9eed44bf 100644 --- a/deploy/helm/cockpit/templates/deployment.yaml +++ b/deploy/helm/cockpit/templates/deployment.yaml @@ -142,6 +142,9 @@ spec: {{- end }} {{- with (index $features "bookmarks") }} {{- with .global }} + {{- if not (kindIs "slice" .) }} + {{- fail "features.bookmarks.global must be a YAML list of bookmarks; a JSON string is not accepted" }} + {{- end }} - name: STACKABLE_COCKPIT_GLOBAL_BOOKMARKS value: {{ toJson . | quote }} {{- end }} diff --git a/deploy/helm/cockpit/values.yaml b/deploy/helm/cockpit/values.yaml index 28b23d54..116ee8f9 100644 --- a/deploy/helm/cockpit/values.yaml +++ b/deploy/helm/cockpit/values.yaml @@ -118,7 +118,8 @@ terminationGracePeriodSeconds: 30 # # Bookmarks shown to every user, before their own. Users cannot change them. # # productId (e.g. trino, superset or custom; unknown ids show as a custom link), # # name (up to 40 characters) and an http(s) url are required; environment and -# # pinned are optional. Invalid entries stop the application at startup. Default: none. +# # pinned are optional. Must be a YAML list, not a JSON string. Invalid entries stop +# # the application at startup. Default: none. # global: # - productId: superset # name: Dashboards From f673d8b08bafbfa25729e778cd1df94427ba9adc Mon Sep 17 00:00:00 2001 From: Johannes Fleck Date: Tue, 27 Oct 2026 16:47:53 +0100 Subject: [PATCH 4/7] refactor: define the bookmark display order once (#277) The sidebar and the dashboard both list global bookmarks first via bookmarksInDisplayOrder. The GlobalBookmark alias is gone in favour of BookmarkLink. --- .../dashboard/DashboardBookmarks.svelte | 30 +++++++++++-------- .../DashboardBookmarks.svelte.spec.ts | 4 +-- src/lib/dashboard/types.test.ts | 26 +++++++++++++++- src/lib/dashboard/types.ts | 14 +++++++-- src/lib/server/dashboard/global-bookmarks.ts | 8 ++--- src/routes/(app)/+layout.svelte | 3 +- src/routes/(app)/page.svelte.spec.ts | 6 ++-- 7 files changed, 64 insertions(+), 27 deletions(-) diff --git a/src/lib/components/dashboard/DashboardBookmarks.svelte b/src/lib/components/dashboard/DashboardBookmarks.svelte index 340dc1e5..e0d880d6 100644 --- a/src/lib/components/dashboard/DashboardBookmarks.svelte +++ b/src/lib/components/dashboard/DashboardBookmarks.svelte @@ -6,7 +6,7 @@ import BookmarkCard from '$lib/components/dashboard/BookmarkCard.svelte'; import { updateBookmark } from '$lib/dashboard/bookmarks-api.js'; import { addToast } from '$lib/stores/toast.svelte.js'; - import type { Bookmark, GlobalBookmark } from '$lib/dashboard/types'; + import { bookmarksInDisplayOrder, type Bookmark, type BookmarkLink } from '$lib/dashboard/types'; let { bookmarks, @@ -16,7 +16,7 @@ /** The user's bookmarks; null when unavailable, which leaves only the global ones. */ bookmarks: Bookmark[] | null; /** Read-only bookmarks for everyone, shown before the user's. */ - globalBookmarks?: GlobalBookmark[]; + globalBookmarks?: BookmarkLink[]; /** Loading the user's bookmarks failed: the section says so. */ loadFailed?: boolean; } = $props(); @@ -25,7 +25,8 @@ let editingBookmark: Bookmark | null = $state(null); const pinning = new SvelteSet(); - const count = $derived(globalBookmarks.length + (bookmarks?.length ?? 0)); + const allBookmarks = $derived(bookmarksInDisplayOrder(globalBookmarks, bookmarks)); + const count = $derived(allBookmarks.length); function openEditBookmark(bookmark: Bookmark) { editingBookmark = bookmark; @@ -87,16 +88,19 @@ {/if} {#if count > 0}
    - {#each globalBookmarks as bookmark (bookmark.id)} - - {/each} - {#each bookmarks ?? [] as bookmark (bookmark.id)} - togglePin(bookmark)} - onEdit={() => openEditBookmark(bookmark)} - /> + {#each allBookmarks as bookmark (bookmark.id)} + + {@const own = bookmarks?.find((b) => b.id === bookmark.id)} + {#if own} + togglePin(own)} + onEdit={() => openEditBookmark(own)} + /> + {:else} + + {/if} {/each}
    {:else if !loadFailed} diff --git a/src/lib/components/dashboard/DashboardBookmarks.svelte.spec.ts b/src/lib/components/dashboard/DashboardBookmarks.svelte.spec.ts index da29fddf..bc9cba8b 100644 --- a/src/lib/components/dashboard/DashboardBookmarks.svelte.spec.ts +++ b/src/lib/components/dashboard/DashboardBookmarks.svelte.spec.ts @@ -1,7 +1,7 @@ import { page } from 'vitest/browser'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { render } from 'vitest-browser-svelte'; -import type { Bookmark, GlobalBookmark } from '$lib/dashboard/types'; +import type { Bookmark, BookmarkLink } from '$lib/dashboard/types'; import DashboardBookmarks from './DashboardBookmarks.svelte'; const { updateBookmark, addToast } = vi.hoisted(() => ({ @@ -27,7 +27,7 @@ const bookmark: Bookmark = { createdAt: '2026-01-01T00:00:00.000Z' }; -const globalBookmark: GlobalBookmark = { +const globalBookmark: BookmarkLink = { id: 'global-0', productId: 'custom', name: 'Team Wiki', diff --git a/src/lib/dashboard/types.test.ts b/src/lib/dashboard/types.test.ts index 645e1c30..471d9091 100644 --- a/src/lib/dashboard/types.test.ts +++ b/src/lib/dashboard/types.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { bookmarkFormSchema, bookmarkHostname } from './types'; +import { bookmarkFormSchema, bookmarkHostname, bookmarksInDisplayOrder } from './types'; const urlSchema = bookmarkFormSchema.shape.url; @@ -37,3 +37,27 @@ describe('bookmarkHostname', () => { expect(bookmarkHostname('superset.example')).toBe('superset.example'); }); }); + +describe('bookmarksInDisplayOrder', () => { + const link = (id: string) => ({ + id, + productId: 'custom', + name: id, + environment: '', + url: 'https://example.com', + pinned: false + }); + + it('lists global bookmarks before the user’s own', () => { + const own = { ...link('own'), createdAt: '2026-10-27T10:00:00.000Z' }; + + expect(bookmarksInDisplayOrder([link('global')], [own]).map((b) => b.id)).toEqual([ + 'global', + 'own' + ]); + }); + + it('lists only the global bookmarks when the user’s are unavailable', () => { + expect(bookmarksInDisplayOrder([link('global')], null).map((b) => b.id)).toEqual(['global']); + }); +}); diff --git a/src/lib/dashboard/types.ts b/src/lib/dashboard/types.ts index bc5a70f5..62171b16 100644 --- a/src/lib/dashboard/types.ts +++ b/src/lib/dashboard/types.ts @@ -21,9 +21,6 @@ export const bookmarkFormSchema = z.object({ /** The fields every bookmark is displayed with; the id keys it in lists. */ export type BookmarkLink = z.infer & { id: string }; -/** A bookmark configured for everyone by the administrator; read-only. */ -export type GlobalBookmark = BookmarkLink; - /** A user's stored bookmark as returned by the API; `createdAt` is an ISO timestamp. */ export type Bookmark = BookmarkLink & { createdAt: string }; @@ -42,6 +39,17 @@ export const bookmarkUpdateSchema = bookmarkCreateSchema export type BookmarkUpdate = z.infer; +/** + * All bookmarks in the order the dashboard and the sidebar list them: the + * global ones first, then the user's own. + */ +export function bookmarksInDisplayOrder( + globalBookmarks: BookmarkLink[], + bookmarks: Bookmark[] | null +): BookmarkLink[] { + return [...globalBookmarks, ...(bookmarks ?? [])]; +} + /** Hostname of a bookmark URL, or the input unchanged if it does not parse. */ export function bookmarkHostname(url: string): string { try { diff --git a/src/lib/server/dashboard/global-bookmarks.ts b/src/lib/server/dashboard/global-bookmarks.ts index 9a4f612f..1e3809dd 100644 --- a/src/lib/server/dashboard/global-bookmarks.ts +++ b/src/lib/server/dashboard/global-bookmarks.ts @@ -1,7 +1,7 @@ import { z } from 'zod'; import { env } from '$env/dynamic/private'; import { logger } from '$lib/server/logging'; -import { bookmarkFormSchema, type GlobalBookmark } from '$lib/dashboard/types'; +import { bookmarkFormSchema, type BookmarkLink } from '$lib/dashboard/types'; const log = logger.child({ module: 'global-bookmarks' }); @@ -20,7 +20,7 @@ const globalBookmarksSchema = z.array( * to every user. Throws on invalid JSON or entries; the error names the failing * entries and fields but not their values, as URLs can contain credentials. */ -export function parseGlobalBookmarks(value: string | undefined): GlobalBookmark[] { +export function parseGlobalBookmarks(value: string | undefined): BookmarkLink[] { if (!value?.trim()) return []; let json: unknown; try { @@ -37,14 +37,14 @@ export function parseGlobalBookmarks(value: string | undefined): GlobalBookmark[ return parsed.data.map((bookmark, index) => ({ id: `global-${index}`, ...bookmark })); } -let cached: GlobalBookmark[] | undefined; +let cached: BookmarkLink[] | undefined; /** * The configured global bookmarks. Parsed on first call and cached; called at * server start so an invalid configuration stops the server. A function rather * than a module-level constant so tests can import this file without the env var. */ -export function globalBookmarks(): GlobalBookmark[] { +export function globalBookmarks(): BookmarkLink[] { if (cached) return cached; try { cached = parseGlobalBookmarks(env.STACKABLE_COCKPIT_GLOBAL_BOOKMARKS); diff --git a/src/routes/(app)/+layout.svelte b/src/routes/(app)/+layout.svelte index ff81f039..7e13224b 100644 --- a/src/routes/(app)/+layout.svelte +++ b/src/routes/(app)/+layout.svelte @@ -6,6 +6,7 @@ import Header from '$lib/components/layout/header/Header.svelte'; import ToastHost from '$lib/components/ToastHost.svelte'; import SessionExpiredModal from '$lib/components/layout/SessionExpiredModal.svelte'; + import { bookmarksInDisplayOrder } from '$lib/dashboard/types'; let { children, data } = $props(); @@ -78,7 +79,7 @@ bind:collapsed={sidebarCollapsed} bind:mobileOpen storageBrowserEnabled={data.storageBrowserEnabled} - bookmarks={[...data.globalBookmarks, ...(data.bookmarks ?? [])]} + bookmarks={bookmarksInDisplayOrder(data.globalBookmarks, data.bookmarks)} />
    diff --git a/src/routes/(app)/page.svelte.spec.ts b/src/routes/(app)/page.svelte.spec.ts index e3c27a0a..a18519e7 100644 --- a/src/routes/(app)/page.svelte.spec.ts +++ b/src/routes/(app)/page.svelte.spec.ts @@ -1,10 +1,10 @@ import { page } from 'vitest/browser'; import { describe, expect, it } from 'vitest'; import { render } from 'vitest-browser-svelte'; -import type { Bookmark, GlobalBookmark } from '$lib/dashboard/types'; +import type { Bookmark, BookmarkLink } from '$lib/dashboard/types'; import Page from './+page.svelte'; -const globalBookmark: GlobalBookmark = { +const globalBookmark: BookmarkLink = { id: 'global-0', productId: 'custom', name: 'Team Wiki', @@ -15,7 +15,7 @@ const globalBookmark: GlobalBookmark = { const renderPage = ( bookmarks: Bookmark[] | null = [], - { globalBookmarks = [] as GlobalBookmark[], bookmarksFailed = false } = {} + { globalBookmarks = [] as BookmarkLink[], bookmarksFailed = false } = {} ) => render(Page, { data: { From c92752ccdac666dd5d96e77293c5b394e5faa63c Mon Sep 17 00:00:00 2001 From: Johannes Fleck Date: Tue, 27 Oct 2026 16:47:53 +0100 Subject: [PATCH 5/7] test: trim the global bookmarks E2E test to the wiring (#277) --- e2e/global-bookmarks.spec.ts | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/e2e/global-bookmarks.spec.ts b/e2e/global-bookmarks.spec.ts index 1776c441..18265d25 100644 --- a/e2e/global-bookmarks.spec.ts +++ b/e2e/global-bookmarks.spec.ts @@ -20,7 +20,9 @@ test.describe('Global bookmarks', () => { await resetBookmarks(page); }); - test('shows global bookmarks before the user’s own, read-only', async ({ page }) => { + // Card rendering and the read-only cards are covered by the DashboardBookmarks + // component tests; this checks the wiring from the env var to the page. + test('shows the configured bookmarks before the user’s own', async ({ page }) => { await page.goto('/'); await waitForHydration(page); @@ -30,15 +32,6 @@ test.describe('Global bookmarks', () => { 'Team Wiki', 'Dashboards' ]); - await expect(section(page).getByText('3 links')).toBeVisible(); - await expect(section(page).getByText('prod', { exact: true })).toBeVisible(); - - for (const name of ['Cluster Logs', 'Team Wiki']) { - await expect(section(page).getByRole('button', { name })).toHaveCount(0); - } - await expect( - section(page).getByRole('button', { name: 'Edit bookmark Dashboards' }) - ).toBeVisible(); }); test('lists global bookmarks in the sidebar by their configured pin', async ({ From 565a5aba5277cef59bcf3f8dc3472085536ee427 Mon Sep 17 00:00:00 2001 From: Johannes Fleck Date: Thu, 29 Oct 2026 14:59:32 +0100 Subject: [PATCH 6/7] refactor: move global bookmarks to the dashboard config (#277) Global bookmarks are configuration, not a feature toggle. They are parsed with parseEnv like the storage config, via a new envJson helper, and the module is imported for its side effect at startup. In the Helm chart, features.bookmarks.global moves to bookmarks.global, as features only holds toggles. --- deploy/helm/cockpit/README.md | 7 ++- deploy/helm/cockpit/templates/deployment.yaml | 6 +- deploy/helm/cockpit/values.yaml | 24 ++++---- src/hooks.server.ts | 7 +-- src/lib/env-schema.test.ts | 30 +++++++++- src/lib/env-schema.ts | 23 ++++++++ ...lobal-bookmarks.test.ts => config.test.ts} | 17 ++++-- src/lib/server/dashboard/config.ts | 42 ++++++++++++++ src/lib/server/dashboard/global-bookmarks.ts | 57 ------------------- src/routes/(app)/+layout.server.ts | 4 +- src/routes/(app)/layout.server.test.ts | 4 +- 11 files changed, 133 insertions(+), 88 deletions(-) rename src/lib/server/dashboard/{global-bookmarks.test.ts => config.test.ts} (81%) create mode 100644 src/lib/server/dashboard/config.ts delete mode 100644 src/lib/server/dashboard/global-bookmarks.ts diff --git a/deploy/helm/cockpit/README.md b/deploy/helm/cockpit/README.md index 2524cc90..b5cfab6b 100644 --- a/deploy/helm/cockpit/README.md +++ b/deploy/helm/cockpit/README.md @@ -122,7 +122,6 @@ All feature parameters are optional. Omit them to use the application defaults. | `features.trino.completion.enabled` | Enable SQL code completion. | `true` (application default) | | `features.storage.enabled` | Show the S3/HDFS file browser and enable routes under `/storage`. | `false` (application default) | | `features.storage.transfer` | Show Cut, Copy, Paste and Rename and allow drag-and-drop move. The transfer API rejects copies, moves and renames while this is disabled. | `false` (application default) | -| `features.bookmarks.global` | Bookmarks shown to every user, before their own; users cannot change them. A list of entries with `productId` (e.g. `trino`, `superset` or `custom`; unknown ids show as a custom link), `name` (up to 40 characters) and an HTTP(S) `url`, plus optional `environment` and `pinned`. Must be a YAML list, not a JSON string. Invalid entries stop the application at startup. | none | ### Storage Browser Parameters @@ -146,6 +145,12 @@ Limits and tuning for the file browser. Empty values use the application default | `storage.parquetPreviewDisallowedCompressionTypes` | Comma-separated Parquet compression codecs that are not previewed. A `-no_offset` suffix (e.g. `gzip-no_offset`) only blocks files without an offset index. | `""` (application default `gzip-no_offset`) | | `storage.downloadHistoryRetentionDays` | Days a user's download history is kept. Integer from 1 to 3650. | `""` (application default `30`) | +### Bookmark Parameters + +| Parameter | Description | Default | +| --- | --- | --- | +| `bookmarks.global` | Bookmarks shown to every user, before their own; users cannot change them. A list of entries with `productId` (e.g. `trino`, `superset` or `custom`; unknown ids show as a custom link), `name` (up to 40 characters) and an HTTP(S) `url`, plus optional `environment` and `pinned`. Must be a YAML list, not a JSON string. Invalid entries stop the application at startup. | `[]` | + ### Authentication Parameters OIDC is the only supported auth mechanism. Disable for local testing. diff --git a/deploy/helm/cockpit/templates/deployment.yaml b/deploy/helm/cockpit/templates/deployment.yaml index 9eed44bf..7cad21ca 100644 --- a/deploy/helm/cockpit/templates/deployment.yaml +++ b/deploy/helm/cockpit/templates/deployment.yaml @@ -140,15 +140,13 @@ spec: - name: STACKABLE_COCKPIT_DOWNLOAD_HISTORY_RETENTION_DAYS value: {{ include "cockpit.envValue" .Values.storage.downloadHistoryRetentionDays }} {{- end }} - {{- with (index $features "bookmarks") }} - {{- with .global }} + {{- with .Values.bookmarks.global }} {{- if not (kindIs "slice" .) }} - {{- fail "features.bookmarks.global must be a YAML list of bookmarks; a JSON string is not accepted" }} + {{- fail "bookmarks.global must be a YAML list of bookmarks; a JSON string is not accepted" }} {{- end }} - name: STACKABLE_COCKPIT_GLOBAL_BOOKMARKS value: {{ toJson . | quote }} {{- end }} - {{- end }} {{- if .Values.trino.queryTtl }} - name: STACKABLE_COCKPIT_QUERY_TTL value: {{ .Values.trino.queryTtl | int64 |quote }} diff --git a/deploy/helm/cockpit/values.yaml b/deploy/helm/cockpit/values.yaml index 116ee8f9..e1fcc9dd 100644 --- a/deploy/helm/cockpit/values.yaml +++ b/deploy/helm/cockpit/values.yaml @@ -114,17 +114,6 @@ terminationGracePeriodSeconds: 30 # # Show Cut, Copy, Paste and Rename and allow drag-and-drop move. The transfer API rejects # # copies, moves and renames while disabled. Default: false (application default). # transfer: false -# bookmarks: -# # Bookmarks shown to every user, before their own. Users cannot change them. -# # productId (e.g. trino, superset or custom; unknown ids show as a custom link), -# # name (up to 40 characters) and an http(s) url are required; environment and -# # pinned are optional. Must be a YAML list, not a JSON string. Invalid entries stop -# # the application at startup. Default: none. -# global: -# - productId: superset -# name: Dashboards -# url: https://superset.example.com -# environment: prod logging: level: "" @@ -200,6 +189,19 @@ database: # The Secret Operator mounts ca.crt into /stackable/database-tls. secretClass: "" +# Dashboard bookmarks. +bookmarks: + # Bookmarks shown to every user, before their own. Users cannot change them. + # productId (e.g. trino, superset or custom; unknown ids show as a custom link), + # name (up to 40 characters) and an http(s) url are required; environment and + # pinned are optional. Must be a YAML list, not a JSON string. Invalid entries stop + # the application at startup. + global: [] + # - productId: superset + # name: Dashboards + # url: https://superset.example.com + # environment: prod + # Storage browser settings that are not feature flags. Empty or omitted values use the application # defaults; invalid values stop the application at startup. storage: diff --git a/src/hooks.server.ts b/src/hooks.server.ts index 1d826708..90eacd04 100644 --- a/src/hooks.server.ts +++ b/src/hooks.server.ts @@ -10,13 +10,13 @@ import { requestLogger, logger } from '$lib/server/logging'; import { getConnectionFromHeader } from '$lib/server/storage/connection.js'; import { STORAGE_CONNECTION_ID_HEADER } from '$lib/storage/connection-id-header.js'; import { storageBrowserEnabled } from '$lib/server/feature-flags.js'; -// Imported for its side effect: parses the storage and PUBLIC_ client config at -// startup, so invalid values stop the server instead of failing the first request. +// Imported for their side effect: parse the storage, dashboard and PUBLIC_ client +// config at startup, so invalid values stop the server instead of failing the first request. import '$lib/server/storage/config.js'; +import '$lib/server/dashboard/config.js'; import { storageEncryptionKey } from '$lib/server/storage/encryption-key.js'; import { notConnected } from '$lib/server/storage/not-connected.js'; import { runMigrations } from '$lib/server/migrate.js'; -import { globalBookmarks } from '$lib/server/dashboard/global-bookmarks.js'; // Allow self-signed TLS certificates in development (e.g. local Trino with self-signed certs). if (dev) { @@ -126,7 +126,6 @@ export const handle = sequence( * detected immediately rather than on the first request that uses the key. */ export const init: ServerInit = async () => { - globalBookmarks(); // throws immediately if the env var is invalid if (!(await runMigrations())) { throw new Error('Database migrations failed'); } diff --git a/src/lib/env-schema.test.ts b/src/lib/env-schema.test.ts index df99abe0..f083fb41 100644 --- a/src/lib/env-schema.test.ts +++ b/src/lib/env-schema.test.ts @@ -1,6 +1,13 @@ import { describe, it, expect } from 'vitest'; import { z } from 'zod'; -import { envBoolean, envInteger, envList, envOptionalInteger, parseEnv } from './env-schema.js'; +import { + envBoolean, + envInteger, + envJson, + envList, + envOptionalInteger, + parseEnv +} from './env-schema.js'; describe('envBoolean', () => { const schema = envBoolean(false); @@ -52,6 +59,27 @@ describe('envList', () => { }); }); +describe('envJson', () => { + const schema = envJson(z.array(z.number()), []); + + it('parses and validates the JSON', () => { + expect(schema.parse('[1, 2]')).toEqual([1, 2]); + expect(schema.safeParse('["a"]').success).toBe(false); + }); + + it('uses the default when unset or blank', () => { + expect(schema.parse(undefined)).toEqual([]); + expect(schema.parse(' ')).toEqual([]); + }); + + it('rejects invalid JSON without repeating the value', () => { + const result = schema.safeParse('[secret'); + expect(result.error?.issues).toEqual([ + expect.objectContaining({ message: 'must be valid JSON' }) + ]); + }); +}); + describe('parseEnv', () => { const schema = z.object({ A: envInteger(1), B: envBoolean(true) }); diff --git a/src/lib/env-schema.ts b/src/lib/env-schema.ts index 4a40f54d..623c4dc9 100644 --- a/src/lib/env-schema.ts +++ b/src/lib/env-schema.ts @@ -56,6 +56,29 @@ export function envList(defaultValue: string[], separator: RegExp | string = ',' ); } +/** JSON validated with `schema`, or `defaultValue` when unset. */ +export function envJson( + schema: T, + defaultValue: Exclude, undefined> +) { + return z.preprocess( + blankAsUnset, + z + .string() + .transform((value, ctx): unknown => { + try { + return JSON.parse(value); + } catch { + // No parser message: it can quote the value, which may hold credentials. + ctx.addIssue({ code: 'custom', message: 'must be valid JSON' }); + return z.NEVER; + } + }) + .pipe(schema) + .default(defaultValue) + ); +} + /** * Parses `env` with `schema`. Throws one error that names every invalid * variable, so a misconfigured deployment fails at startup with all problems diff --git a/src/lib/server/dashboard/global-bookmarks.test.ts b/src/lib/server/dashboard/config.test.ts similarity index 81% rename from src/lib/server/dashboard/global-bookmarks.test.ts rename to src/lib/server/dashboard/config.test.ts index e9826eff..22ef2617 100644 --- a/src/lib/server/dashboard/global-bookmarks.test.ts +++ b/src/lib/server/dashboard/config.test.ts @@ -2,11 +2,14 @@ import { describe, it, expect, vi } from 'vitest'; vi.mock('$lib/server/logging', () => import('$lib/test-utils/mock-logger.js')); -import { parseGlobalBookmarks } from './global-bookmarks.js'; +import { parseDashboardConfig } from './config.js'; + +const parseGlobalBookmarks = (value: string | undefined) => + parseDashboardConfig({ STACKABLE_COCKPIT_GLOBAL_BOOKMARKS: value }).globalBookmarks; const json = (value: unknown) => JSON.stringify(value); -describe('parseGlobalBookmarks', () => { +describe('parseDashboardConfig', () => { it('has no global bookmarks when the variable is unset or blank', () => { expect(parseGlobalBookmarks(undefined)).toEqual([]); expect(parseGlobalBookmarks(' ')).toEqual([]); @@ -47,11 +50,15 @@ describe('parseGlobalBookmarks', () => { }); it('rejects invalid JSON', () => { - expect(() => parseGlobalBookmarks('[{')).toThrow(/not valid JSON/); + expect(() => parseGlobalBookmarks('[{')).toThrow( + 'STACKABLE_COCKPIT_GLOBAL_BOOKMARKS: must be valid JSON' + ); }); it('rejects anything but an array', () => { - expect(() => parseGlobalBookmarks(json({ name: 'Wiki' }))).toThrow(/is invalid/); + expect(() => parseGlobalBookmarks(json({ name: 'Wiki' }))).toThrow( + /STACKABLE_COCKPIT_GLOBAL_BOOKMARKS: .*array/ + ); }); it.each([ @@ -64,7 +71,7 @@ describe('parseGlobalBookmarks', () => { const parse = () => parseGlobalBookmarks(json([entry, { ...entry, ...override }])); - expect(parse).toThrow('[1]'); + expect(parse).toThrow('STACKABLE_COCKPIT_GLOBAL_BOOKMARKS.1'); expect(parse).toThrow(field); }); diff --git a/src/lib/server/dashboard/config.ts b/src/lib/server/dashboard/config.ts new file mode 100644 index 00000000..0e9dbfde --- /dev/null +++ b/src/lib/server/dashboard/config.ts @@ -0,0 +1,42 @@ +// Dashboard settings: configuration, not feature toggles (those are in +// `$lib/server/feature-flags`). Parsed once at module init. + +import { z } from 'zod'; +import { env } from '$env/dynamic/private'; +import { envJson, parseEnv } from '$lib/env-schema.js'; +import { logger } from '$lib/server/logging'; +import { bookmarkFormSchema, type BookmarkLink } from '$lib/dashboard/types'; + +const log = logger.child({ module: 'dashboard-config' }); + +// Strict, so a misspelt key such as "pined" fails instead of being ignored. An +// unknown productId is accepted and shown as a custom link, as for user bookmarks. +const globalBookmarkSchema = z.strictObject({ + ...bookmarkFormSchema.shape, + environment: bookmarkFormSchema.shape.environment.default(''), + pinned: bookmarkFormSchema.shape.pinned.default(false) +}); + +const schema = z.object({ + STACKABLE_COCKPIT_GLOBAL_BOOKMARKS: envJson(z.array(globalBookmarkSchema), []) +}); + +/** + * Throws on invalid configuration. The error names the failing entries and + * fields but not their values, as URLs can contain credentials. + */ +export function parseDashboardConfig(env: Record) { + const config = parseEnv(schema, env); + const globalBookmarks: BookmarkLink[] = config.STACKABLE_COCKPIT_GLOBAL_BOOKMARKS.map( + (bookmark, index) => ({ id: `global-${index}`, ...bookmark }) + ); + return { globalBookmarks }; +} + +const config = parseDashboardConfig(env); +log.info({ count: config.globalBookmarks.length }, 'Loaded global bookmarks'); + +/** Bookmarks shown to every user, before their own, on the dashboard and in the + * sidebar. Users cannot change them. + * Controlled by `STACKABLE_COCKPIT_GLOBAL_BOOKMARKS` (JSON array). Default: none. */ +export const globalBookmarks = config.globalBookmarks; diff --git a/src/lib/server/dashboard/global-bookmarks.ts b/src/lib/server/dashboard/global-bookmarks.ts deleted file mode 100644 index 1e3809dd..00000000 --- a/src/lib/server/dashboard/global-bookmarks.ts +++ /dev/null @@ -1,57 +0,0 @@ -import { z } from 'zod'; -import { env } from '$env/dynamic/private'; -import { logger } from '$lib/server/logging'; -import { bookmarkFormSchema, type BookmarkLink } from '$lib/dashboard/types'; - -const log = logger.child({ module: 'global-bookmarks' }); - -// Strict, so a misspelt key such as "pined" fails instead of being ignored. An -// unknown productId is accepted and shown as a custom link, as for user bookmarks. -const globalBookmarksSchema = z.array( - z.strictObject({ - ...bookmarkFormSchema.shape, - environment: bookmarkFormSchema.shape.environment.default(''), - pinned: bookmarkFormSchema.shape.pinned.default(false) - }) -); - -/** - * Parse `STACKABLE_COCKPIT_GLOBAL_BOOKMARKS`: a JSON array of bookmarks shown - * to every user. Throws on invalid JSON or entries; the error names the failing - * entries and fields but not their values, as URLs can contain credentials. - */ -export function parseGlobalBookmarks(value: string | undefined): BookmarkLink[] { - if (!value?.trim()) return []; - let json: unknown; - try { - json = JSON.parse(value); - } catch { - throw new Error('STACKABLE_COCKPIT_GLOBAL_BOOKMARKS is not valid JSON'); - } - const parsed = globalBookmarksSchema.safeParse(json); - if (!parsed.success) { - throw new Error( - `STACKABLE_COCKPIT_GLOBAL_BOOKMARKS is invalid:\n${z.prettifyError(parsed.error)}` - ); - } - return parsed.data.map((bookmark, index) => ({ id: `global-${index}`, ...bookmark })); -} - -let cached: BookmarkLink[] | undefined; - -/** - * The configured global bookmarks. Parsed on first call and cached; called at - * server start so an invalid configuration stops the server. A function rather - * than a module-level constant so tests can import this file without the env var. - */ -export function globalBookmarks(): BookmarkLink[] { - if (cached) return cached; - try { - cached = parseGlobalBookmarks(env.STACKABLE_COCKPIT_GLOBAL_BOOKMARKS); - } catch (err) { - log.error({ err }, 'Invalid global bookmarks configuration'); - throw err; - } - log.info({ count: cached.length }, 'Loaded global bookmarks'); - return cached; -} diff --git a/src/routes/(app)/+layout.server.ts b/src/routes/(app)/+layout.server.ts index dd3329e7..3e47a053 100644 --- a/src/routes/(app)/+layout.server.ts +++ b/src/routes/(app)/+layout.server.ts @@ -1,7 +1,7 @@ import type { LayoutServerLoad } from './$types'; import { storageBrowserEnabled } from '$lib/server/feature-flags.js'; import { listBookmarks } from '$lib/server/dashboard/bookmarks-db.js'; -import { globalBookmarks } from '$lib/server/dashboard/global-bookmarks.js'; +import { globalBookmarks } from '$lib/server/dashboard/config.js'; import { dashboardBookmarksTotal } from '$lib/server/metrics.js'; import { BOOKMARKS_DEPENDENCY, type Bookmark } from '$lib/dashboard/types'; @@ -10,7 +10,7 @@ export const load: LayoutServerLoad = async ({ locals, depends }) => { return { user: locals.user, storageBrowserEnabled, - globalBookmarks: globalBookmarks(), + globalBookmarks, ...(await loadBookmarks(locals)) }; }; diff --git a/src/routes/(app)/layout.server.test.ts b/src/routes/(app)/layout.server.test.ts index 3591be61..b6dfe0e9 100644 --- a/src/routes/(app)/layout.server.test.ts +++ b/src/routes/(app)/layout.server.test.ts @@ -4,9 +4,7 @@ const { listBookmarks } = vi.hoisted(() => ({ listBookmarks: vi.fn() })); vi.mock('$lib/server/dashboard/bookmarks-db.js', () => ({ listBookmarks })); vi.mock('$lib/server/feature-flags.js', () => ({ storageBrowserEnabled: false })); -vi.mock('$lib/server/dashboard/global-bookmarks.js', () => ({ - globalBookmarks: () => [GLOBAL_BOOKMARK] -})); +vi.mock('$lib/server/dashboard/config.js', () => ({ globalBookmarks: [GLOBAL_BOOKMARK] })); import { load } from './+layout.server.js'; From 63ff78982fc64ef323a2c271748cfecbb668f98b Mon Sep 17 00:00:00 2001 From: Johannes Fleck Date: Thu, 29 Oct 2026 15:38:59 +0100 Subject: [PATCH 7/7] test: check which sidebar list each bookmark lands in (#277) The pinned global bookmark from .env.test always shows Favourites, so the test passed even if the user's own pins stopped reaching the sidebar. --- e2e/sidebar-bookmarks.spec.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/e2e/sidebar-bookmarks.spec.ts b/e2e/sidebar-bookmarks.spec.ts index e361554e..873ab168 100644 --- a/e2e/sidebar-bookmarks.spec.ts +++ b/e2e/sidebar-bookmarks.spec.ts @@ -37,11 +37,18 @@ test.describe('Sidebar bookmarks', () => { await page.goto('/'); await waitForHydration(page); + // The pinned global bookmark from .env.test always shows Favourites, so check + // which list each of the user's bookmarks lands in. + const listWith = (name: string) => + sidebarNav(page) + .getByRole('list') + .filter({ has: page.getByRole('link', { name }) }); await expect(page.getByText('Favourites')).toBeVisible(); - await expect(sidebarNav(page).getByRole('link', { name: 'Pipelines' })).toBeVisible(); + await expect(listWith('Pipelines')).toBeVisible(); + await expect(listWith('Pipelines').getByRole('link', { name: 'Dashboards' })).toHaveCount(0); await expect(page.getByRole('button', { name: 'Tools', exact: true })).toBeVisible(); - await expect(sidebarNav(page).getByRole('link', { name: 'Dashboards' })).toBeVisible(); - await expect(sidebarNav(page).getByRole('link', { name: 'Trino' })).toBeVisible(); + await expect(listWith('Trino').getByRole('link', { name: 'Dashboards' })).toBeVisible(); + await expect(listWith('Trino').getByRole('link', { name: 'Pipelines' })).toHaveCount(0); }); test('opens each bookmark in a new tab', async ({ page }) => {