mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-19 16:02:14 +00:00
fix(frontend): hide the fork workspace banner from operators (#10575)
* fix(frontend): hide the fork workspace banner from operators * fix(frontend): scope the operator gate to the workspace its role was fetched for * fix(frontend): drop superseded whoami responses instead of writing a stale role * fix(frontend): guard the remaining workspace-switch userStore writers
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
<script lang="ts">
|
||||
import { workspaceStore, userWorkspaces } from '$lib/stores'
|
||||
import { workspaceStore, userWorkspaces, userStore, type UserExt } from '$lib/stores'
|
||||
import { ScriptService } from '$lib/gen'
|
||||
import type { WorkspaceComparison } from '$lib/gen'
|
||||
import { fetchWorkspaceComparison } from '$lib/workspaceComparison'
|
||||
@@ -26,12 +26,25 @@
|
||||
// prefix) also avoids a parentless "Fork of ()" banner when the linkage is dropped.
|
||||
let isFork = $derived(parentWorkspaceId != null)
|
||||
let isDevWorkspace = $derived(currentWorkspaceData?.is_dev_workspace ?? false)
|
||||
// Operators run scripts and flows, they never deploy a fork, so the banner and
|
||||
// its CTA are noise for them. Gates the fetches too, not just the markup: the
|
||||
// fork/parent comparison is an expensive tally no operator can act on.
|
||||
//
|
||||
// Only a role fetched for the workspace we are on answers this. `$workspaceStore`
|
||||
// flips synchronously on a switch while `$userStore` still holds the workspace we
|
||||
// left, so trusting it unqualified would flash the banner at (and start the tally
|
||||
// for) an operator entering a fork from a workspace where they are not one.
|
||||
function isConfirmedNonOperator(user: UserExt | undefined, ws: string | undefined): boolean {
|
||||
return !!user && !!ws && user.workspace_id === ws && !user.operator
|
||||
}
|
||||
let isNotOperator = $derived(isConfirmedNonOperator($userStore, $workspaceStore))
|
||||
let showBanner = $derived(isFork && isNotOperator)
|
||||
|
||||
// Drafts in this fork. When the fork is otherwise in sync with its parent, a
|
||||
// user with only pending drafts should still get the draft CTA (mirrors the
|
||||
// non-fork WorkspaceDraftsBanner). Pass undefined when not a fork so it doesn't
|
||||
// fetch.
|
||||
const drafts = useWorkspaceDrafts(() => (isFork ? ($workspaceStore ?? undefined) : undefined))
|
||||
// non-fork WorkspaceDraftsBanner). Pass undefined when the banner is hidden so
|
||||
// it doesn't fetch.
|
||||
const drafts = useWorkspaceDrafts(() => (showBanner ? ($workspaceStore ?? undefined) : undefined))
|
||||
const draftCount = $derived(drafts.count)
|
||||
|
||||
// Every read of `comparison` that decides what the banner says or where its button
|
||||
@@ -63,10 +76,12 @@
|
||||
resetCiTestSummary()
|
||||
}
|
||||
|
||||
// `isNotOperator` is a dependency of its own: it only turns true once this
|
||||
// workspace's role has landed, which is after the switch that triggered it.
|
||||
$effect(() => {
|
||||
;[$workspaceStore, parentWorkspaceId]
|
||||
;[$workspaceStore, parentWorkspaceId, isNotOperator]
|
||||
untrack(() => {
|
||||
if (isFork && $workspaceStore) {
|
||||
if (showBanner && $workspaceStore) {
|
||||
checkForChanges()
|
||||
} else {
|
||||
dropComparison()
|
||||
@@ -75,7 +90,7 @@
|
||||
})
|
||||
|
||||
onMount(() => {
|
||||
if (isFork && $workspaceStore) {
|
||||
if (showBanner && $workspaceStore) {
|
||||
checkForChanges()
|
||||
} else {
|
||||
dropComparison()
|
||||
@@ -229,7 +244,7 @@
|
||||
}
|
||||
</script>
|
||||
|
||||
{#if isFork}
|
||||
{#if showBanner}
|
||||
<!-- Side padding mirrors the page content container below, so the banner
|
||||
stays aligned with it instead of bleeding to the viewport edges. -->
|
||||
<div class="w-full text-xs max-w-7xl mx-auto px-4 sm:px-8 pt-2">
|
||||
|
||||
@@ -17,6 +17,11 @@ import { DEFAULT_HUB_BASE_URL } from './hub'
|
||||
import type { DbManagerUriState } from './components/dbManagerDrawerModel.svelte'
|
||||
|
||||
export interface UserExt {
|
||||
// Workspace this membership was fetched for. `$workspaceStore` flips
|
||||
// synchronously on a switch while the new `whoami` is still in flight, so a
|
||||
// consumer whose behavior depends on the role must compare this against the
|
||||
// active workspace rather than read a role that still describes the previous one.
|
||||
workspace_id: string
|
||||
email: string
|
||||
name?: string
|
||||
username: string
|
||||
|
||||
@@ -4,15 +4,16 @@ import type { UserExt } from './stores.js'
|
||||
export async function getUserExt(workspace: string): Promise<UserExt | undefined> {
|
||||
try {
|
||||
const user = await UserService.whoami({ workspace })
|
||||
return mapUserToUserExt(user)
|
||||
return mapUserToUserExt(user, workspace)
|
||||
} catch (error) {
|
||||
return undefined
|
||||
}
|
||||
}
|
||||
|
||||
function mapUserToUserExt(user: User): UserExt {
|
||||
function mapUserToUserExt(user: User, workspace: string): UserExt {
|
||||
const ext: UserExt = {
|
||||
...user,
|
||||
workspace_id: workspace,
|
||||
groups: user.groups!,
|
||||
pgroups: user.groups!.map((x) => `g/${x}`)
|
||||
}
|
||||
|
||||
@@ -307,6 +307,13 @@
|
||||
}
|
||||
}
|
||||
const user = await getUserExt(workspace)
|
||||
// Every workspace change starts a fetch without cancelling the one before it,
|
||||
// so a slow response can land after a faster one for the workspace the user
|
||||
// has since moved to. The store must describe the active workspace: letting a
|
||||
// superseded response write would leave every role gate reading the one we left.
|
||||
if ($workspaceStore !== workspace) {
|
||||
return
|
||||
}
|
||||
if (!deepEqual(user, $userStore)) {
|
||||
userStore.set(user)
|
||||
}
|
||||
@@ -628,8 +635,15 @@
|
||||
timeout = undefined
|
||||
} else if (!u) {
|
||||
timeout = setTimeout(async () => {
|
||||
if (!$userStore && $workspaceStore) {
|
||||
$userStore = await getUserExt($workspaceStore)
|
||||
const ws = $workspaceStore
|
||||
if (!$userStore && ws) {
|
||||
const user = await getUserExt(ws)
|
||||
// Recovers the workspace that was left without a role. A switch
|
||||
// mid-flight has already started the fetch for the new one, so this
|
||||
// answer describes the workspace we left.
|
||||
if ($workspaceStore === ws) {
|
||||
$userStore = user
|
||||
}
|
||||
}
|
||||
}, 5000)
|
||||
}
|
||||
|
||||
@@ -130,10 +130,18 @@
|
||||
if ($userStore) {
|
||||
console.log(`Welcome back ${$userStore.username} to ${$workspaceStore}`)
|
||||
} else {
|
||||
$userStore = await getUserExt($workspaceStore)
|
||||
if (!$userStore) {
|
||||
const ws = $workspaceStore
|
||||
const user = await getUserExt(ws)
|
||||
// A switch mid-flight means this answers for the workspace we left, and
|
||||
// that switch has already started the fetch answering for the active
|
||||
// one: neither this role nor its failure describes where we are now.
|
||||
if ($workspaceStore !== ws) {
|
||||
return
|
||||
}
|
||||
if (!user) {
|
||||
throw Error('Not logged in')
|
||||
}
|
||||
$userStore = user
|
||||
}
|
||||
} else {
|
||||
if (
|
||||
@@ -240,7 +248,11 @@
|
||||
|
||||
if (workspace && user) {
|
||||
const newUser = await getUserExt(workspace)
|
||||
if (!deepEqual(newUser, $userStore)) {
|
||||
// Refreshes the workspace that was active when the tick started; a
|
||||
// switch mid-flight makes this answer describe the one we left.
|
||||
if ($workspaceStore !== workspace) {
|
||||
console.debug('workspace changed during user refresh, dropping')
|
||||
} else if (!deepEqual(newUser, $userStore)) {
|
||||
userStore.set(newUser)
|
||||
console.info('refreshed user')
|
||||
} else {
|
||||
|
||||
Reference in New Issue
Block a user