mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-24 00:00:46 +00:00
feat(frontend): keep ?workspace= off workspace-agnostic pages
Instance-level pages (/workers, /service_logs, /instance_groups, /concurrency_groups) don't depend on the active workspace, so seeding ?workspace= there is noise. Add a workspaceAgnosticRoute predicate: on these routes the layout strips the param for a clean URL, but first adopts any explicit ?workspace= into the store so a shared deep link still propagates the workspace to subsequent scoped pages. State is preserved by the store + storage, not the URL, so navigation across these pages keeps the workspace. Verified headless: agnostic pages carry no param, a deep-linked param is adopted (storage updated) then stripped, and scoped pages still seed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
import { describe, it, expect } from 'vitest'
|
||||
import { workspaceParamAllowed } from './workspaceParam'
|
||||
import { workspaceParamAllowed, workspaceAgnosticRoute } from './workspaceParam'
|
||||
|
||||
describe('workspaceParamAllowed', () => {
|
||||
it('allows regular workspace-scoped app routes', () => {
|
||||
@@ -36,3 +36,33 @@ describe('workspaceParamAllowed', () => {
|
||||
expect(workspaceParamAllowed('/scripts/get/f/team/oauth-helper')).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('workspaceAgnosticRoute', () => {
|
||||
it('flags instance-level routes as workspace-agnostic', () => {
|
||||
expect(workspaceAgnosticRoute('/workers')).toBe(true)
|
||||
expect(workspaceAgnosticRoute('/service_logs')).toBe(true)
|
||||
expect(workspaceAgnosticRoute('/instance_groups')).toBe(true)
|
||||
expect(workspaceAgnosticRoute('/concurrency_groups')).toBe(true)
|
||||
expect(workspaceAgnosticRoute('/workers/sub/path')).toBe(true)
|
||||
})
|
||||
|
||||
it('treats workspace-scoped routes as not agnostic', () => {
|
||||
expect(workspaceAgnosticRoute('/')).toBe(false)
|
||||
expect(workspaceAgnosticRoute('/runs')).toBe(false)
|
||||
expect(workspaceAgnosticRoute('/audit_logs')).toBe(false)
|
||||
expect(workspaceAgnosticRoute('/variables')).toBe(false)
|
||||
expect(workspaceAgnosticRoute('/scripts/get/u/me/s')).toBe(false)
|
||||
})
|
||||
|
||||
it('does not match on a mid-path or prefix-collision segment', () => {
|
||||
expect(workspaceAgnosticRoute('/workers_extra')).toBe(false)
|
||||
expect(workspaceAgnosticRoute('/apps/get/u/me/workers')).toBe(false)
|
||||
})
|
||||
|
||||
it('agnostic routes are still within the allowed (non-hard-excluded) set', () => {
|
||||
// They must pass workspaceParamAllowed so the param can be read/adopted
|
||||
// before being stripped.
|
||||
expect(workspaceParamAllowed('/workers')).toBe(true)
|
||||
expect(workspaceParamAllowed('/service_logs')).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -11,10 +11,32 @@
|
||||
const WORKSPACE_PARAM_EXCLUDED_PREFIXES = ['/user/', '/oauth/']
|
||||
|
||||
/**
|
||||
* Whether the `?workspace=` query param should be kept in sync with the active
|
||||
* workspace store on the given route. Drives both directions of the sync in the
|
||||
* logged layout: URL → store on navigation, and store → URL via replaceState.
|
||||
* Whether the `?workspace=` query param may appear at all on the given route.
|
||||
* Hard exclusion: drives both directions of the sync in the logged layout — when
|
||||
* false the param is never read into the store nor written to the URL.
|
||||
*/
|
||||
export function workspaceParamAllowed(pathname: string): boolean {
|
||||
return !WORKSPACE_PARAM_EXCLUDED_PREFIXES.some((prefix) => pathname.startsWith(prefix))
|
||||
}
|
||||
|
||||
// Routes whose content does not depend on the active workspace (instance-level
|
||||
// pages). The param is meaningless here, so the URL is kept clean: an explicit
|
||||
// `?workspace=` is still adopted into the store — so a shared link propagates the
|
||||
// workspace to subsequent workspace-scoped pages — and then stripped. The active
|
||||
// workspace is preserved by the store + storage, not by the URL on these routes.
|
||||
const WORKSPACE_AGNOSTIC_PREFIXES = [
|
||||
'/workers',
|
||||
'/service_logs',
|
||||
'/instance_groups',
|
||||
'/concurrency_groups'
|
||||
]
|
||||
|
||||
/**
|
||||
* Whether the given route is workspace-agnostic (instance-level). On these
|
||||
* routes the logged layout strips `?workspace=` rather than seeding it.
|
||||
*/
|
||||
export function workspaceAgnosticRoute(pathname: string): boolean {
|
||||
return WORKSPACE_AGNOSTIC_PREFIXES.some(
|
||||
(prefix) => pathname === prefix || pathname.startsWith(prefix + '/')
|
||||
)
|
||||
}
|
||||
|
||||
@@ -38,7 +38,7 @@
|
||||
import CenteredModal from '$lib/components/CenteredModal.svelte'
|
||||
import { afterNavigate, beforeNavigate, replaceState } from '$app/navigation'
|
||||
import { goto } from '$lib/navigation'
|
||||
import { workspaceParamAllowed } from '$lib/workspaceParam'
|
||||
import { workspaceParamAllowed, workspaceAgnosticRoute } from '$lib/workspaceParam'
|
||||
import UserSettings from '$lib/components/UserSettings.svelte'
|
||||
import SuperadminSettings from '$lib/components/SuperadminSettings.svelte'
|
||||
import WindmillIcon from '$lib/components/icons/WindmillIcon.svelte'
|
||||
@@ -137,10 +137,24 @@
|
||||
// which also prevents a ping-pong with the URL → store sync in onQueryChange.
|
||||
function syncWorkspaceToUrl() {
|
||||
if (!BROWSER || !routerReady) return
|
||||
const ws = $workspaceStore
|
||||
if (!ws || !workspaceParamAllowed(page.url.pathname)) return
|
||||
if (page.url.searchParams.get('workspace') === ws) return
|
||||
const path = page.url.pathname
|
||||
if (!workspaceParamAllowed(path)) return
|
||||
const param = page.url.searchParams.get('workspace')
|
||||
try {
|
||||
if (workspaceAgnosticRoute(path)) {
|
||||
// Workspace-agnostic page: adopt an explicit ?workspace= into the store
|
||||
// (so it propagates to later scoped pages), then strip it for a clean
|
||||
// URL. The store/storage keep the workspace; the URL need not.
|
||||
if (!param) return
|
||||
if (param !== $workspaceStore) $workspaceStore = param
|
||||
const url = new URL(page.url)
|
||||
url.searchParams.delete('workspace')
|
||||
replaceState(url, page.state)
|
||||
return
|
||||
}
|
||||
// Workspace-scoped page: reflect the active workspace into the URL.
|
||||
const ws = $workspaceStore
|
||||
if (!ws || param === ws) return
|
||||
const url = new URL(page.url)
|
||||
url.searchParams.set('workspace', ws)
|
||||
replaceState(url, page.state)
|
||||
|
||||
Reference in New Issue
Block a user