mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-08 08:04:25 +00:00
fix(frontend): make the archived route independent of write permissions
Three ways the archived-only placeholder failed to deliver what it promised: Reading archived items is not a write, but the notice offering them sat behind the create-permission gate — so an operator, or a workspace whose direct-deploy protection cleared `showEditButtons`, got "no items found" over items it could see and a toolbar now inert. The gate governs the create actions alone; the notice is shown to whoever the probe found something for. The probe answered once per workspace and was never invalidated, so archiving the last item left a cached "nothing archived" claiming the workspace was empty until a page load. `reloadItemsAndCounts` clears it. And it omitted `includeWithoutMain`, which the backend reads as excluding library scripts — a workspace holding only archived ones answered "empty". Always true here: hiding library scripts puts a filter in `activeFilters`, which `workspaceEmpty` requires to be empty. `whenIdle()` gains the two tests its contract deserves, since the reload correctness three rounds argued over rests on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012fRjnaHLwjpHN84gNNxah9
This commit is contained in:
Guilhem Lemouel
co-authored by
Claude Opus 5
Claude Opus 5
parent
f802962be2
commit
0aefea7ce3
@@ -700,6 +700,10 @@
|
||||
// runnables an owner holds. A scope change (sort/archive/kind/…) doesn't go
|
||||
// through here: the counts resource keys on those itself.
|
||||
async function reloadItemsAndCounts(): Promise<void> {
|
||||
// The answer can change with the rows: archiving the last item leaves the listing empty
|
||||
// with something archived behind it, and a cached "nothing archived" would then call
|
||||
// the workspace empty and hide the way to it until a page load.
|
||||
archivedProbe = undefined
|
||||
// A mutated row can be gone, or sit at a new path, afterwards: snapshot what
|
||||
// was on screen so the selection can drop what this reload removes instead of
|
||||
// keeping a dead path. `tick` lets the reloaded rows re-register first.
|
||||
@@ -1023,7 +1027,16 @@
|
||||
})
|
||||
async function probeArchived(workspace: string) {
|
||||
try {
|
||||
const res = await ScriptService.listRunnables({ workspace, showArchived: true, perPage: 1 })
|
||||
// `includeWithoutMain` to match the listing: the backend drops `auto_kind = 'lib'`
|
||||
// without it, so a workspace holding only archived library scripts would answer
|
||||
// "nothing archived". Always true here — hiding library scripts puts a filter in
|
||||
// `activeFilters`, which `workspaceEmpty` requires to be empty.
|
||||
const res = await ScriptService.listRunnables({
|
||||
workspace,
|
||||
showArchived: true,
|
||||
includeWithoutMain: true,
|
||||
perPage: 1
|
||||
})
|
||||
archivedProbe = { workspace, hasArchived: (res.items?.length ?? 0) > 0 }
|
||||
} catch (error) {
|
||||
console.error('Could not check for archived items:', error)
|
||||
@@ -1031,6 +1044,13 @@
|
||||
}
|
||||
}
|
||||
let emptyStateAnswered = $derived(archivedProbe?.workspace === $workspaceStore)
|
||||
/**
|
||||
* Whether this user may be offered the create actions. The empty state's template import
|
||||
* and create menu do no permission check of their own, so an operator — or a workspace
|
||||
* whose direct-deploy protection cleared `showEditButtons` — must not be shown them.
|
||||
* Reading archived items is not a write, so it is not gated on this.
|
||||
*/
|
||||
let canCreateHere = $derived(!$userStore?.operator && showEditButtons)
|
||||
|
||||
// The workspace itself holds nothing — no filter is narrowing the list away. It stays
|
||||
// false until the first load resolves: a skeleton already means "loading", and the
|
||||
@@ -1894,20 +1914,24 @@
|
||||
<!-- Pipelines aren't part of the text filter, so only fall through to show
|
||||
them (list rows / injected tree folders) when not actively searching;
|
||||
a no-match search still reads as empty. -->
|
||||
<!-- Same gate as the create menu above: the empty state offers a template import and
|
||||
that very menu, and neither does a permission check of its own. An operator, or a
|
||||
workspace whose direct-deploy protection cleared `showEditButtons`, gets the plain
|
||||
message instead of two actions it may not take. -->
|
||||
{#if workspaceEmpty && !$userStore?.operator && showEditButtons}
|
||||
{#if workspaceEmpty}
|
||||
<!-- Held until the archived probe answers rather than drawn and swapped: the two
|
||||
placeholders say different things, and showing the wrong one first says the
|
||||
workspace is empty when it is not. -->
|
||||
{#if emptyStateAnswered}
|
||||
<WorkspaceEmptyState
|
||||
archivedOnly={archivedProbe?.hasArchived === true}
|
||||
onPick={(project) => (hubPick = project)}
|
||||
onShowArchived={() => (filterValues.val = { ...filterValues.val, archived: true })}
|
||||
/>
|
||||
{#if archivedProbe?.hasArchived || canCreateHere}
|
||||
<!-- Shown to whoever has something to do here: the archived notice to
|
||||
everyone, since reading archived items is not a write, and the create
|
||||
actions only to a user who may take them. -->
|
||||
<WorkspaceEmptyState
|
||||
archivedOnly={archivedProbe?.hasArchived === true}
|
||||
canCreate={canCreateHere}
|
||||
onPick={(project) => (hubPick = project)}
|
||||
onShowArchived={() => (filterValues.val = { ...filterValues.val, archived: true })}
|
||||
/>
|
||||
{:else}
|
||||
<NoItemFound {activeFilters} />
|
||||
{/if}
|
||||
{/if}
|
||||
{:else}
|
||||
<NoItemFound {activeFilters} />
|
||||
|
||||
@@ -17,9 +17,15 @@
|
||||
archivedOnly?: boolean
|
||||
/** Switches the list to the archived view. */
|
||||
onShowArchived: () => void
|
||||
/**
|
||||
* Whether to offer the template import and the create menu. Neither checks permissions
|
||||
* itself, so an operator gets the state described without the two actions it may not
|
||||
* take — the archived link stays, since reading archived items is not a write.
|
||||
*/
|
||||
canCreate?: boolean
|
||||
}
|
||||
|
||||
let { onPick, archivedOnly = false, onShowArchived }: Props = $props()
|
||||
let { onPick, archivedOnly = false, onShowArchived, canCreate = true }: Props = $props()
|
||||
|
||||
// Row opacities: the list fading out of existence. Static on purpose — motion is what
|
||||
// makes a skeleton mean "loading", and this state means "empty".
|
||||
@@ -81,56 +87,58 @@
|
||||
{:else}
|
||||
Your scripts, flows and apps will show up here.
|
||||
{/if}
|
||||
<!-- The hub half goes when the instance has the hub turned off, and the remaining link
|
||||
{#if canCreate}
|
||||
<!-- The hub half goes when the instance has the hub turned off, and the remaining link
|
||||
opens the sentence instead of continuing it. -->
|
||||
{#if !$disableHubStore}
|
||||
<!-- Opens downward into the page rather than upward into the hero: the caption sits
|
||||
{#if !$disableHubStore}
|
||||
<!-- Opens downward into the page rather than upward into the hero: the caption sits
|
||||
high when the AI composer is hidden, so the room is below it. `fitViewport` caps
|
||||
the box on a short viewport, which is why the height below is definite and the
|
||||
list inside fills it — a squeezed box with a fixed-height list inside overflows
|
||||
its own frame. -->
|
||||
<Popover
|
||||
floatingConfig={{
|
||||
placement: 'bottom',
|
||||
strategy: 'absolute',
|
||||
gutter: 8,
|
||||
overflowPadding: 16,
|
||||
flip: { fallbackPlacements: ['top', 'bottom-start', 'top-start'] },
|
||||
fitViewport: true,
|
||||
overlap: false
|
||||
}}
|
||||
contentClasses="p-0 flex"
|
||||
contentStyle="height: min(72vh, 520px);"
|
||||
class="border-b border-transparent text-accent hover:border-accent"
|
||||
triggerAttrs={{ 'aria-label': 'Start from a template' }}
|
||||
on:openChange={(e) =>
|
||||
e.detail && logFeatureUsage('home', 'template_picker_open', { key: 'empty_state' })}
|
||||
>
|
||||
{#snippet trigger()}Start from a template{/snippet}
|
||||
{#snippet content({ close })}
|
||||
<HubTemplatePicker
|
||||
onPick={(project) => {
|
||||
close()
|
||||
onPick(project)
|
||||
}}
|
||||
/>
|
||||
{/snippet}
|
||||
</Popover>
|
||||
or
|
||||
{/if}
|
||||
<CreateActionsMenu source="empty_state" triggerElement={newLinkEl}>
|
||||
{#snippet trigger()}
|
||||
<!-- A bare <button> for a link inside a sentence, signed off by design: <Button>
|
||||
<Popover
|
||||
floatingConfig={{
|
||||
placement: 'bottom',
|
||||
strategy: 'absolute',
|
||||
gutter: 8,
|
||||
overflowPadding: 16,
|
||||
flip: { fallbackPlacements: ['top', 'bottom-start', 'top-start'] },
|
||||
fitViewport: true,
|
||||
overlap: false
|
||||
}}
|
||||
contentClasses="p-0 flex"
|
||||
contentStyle="height: min(72vh, 520px);"
|
||||
class="border-b border-transparent text-accent hover:border-accent"
|
||||
triggerAttrs={{ 'aria-label': 'Start from a template' }}
|
||||
on:openChange={(e) =>
|
||||
e.detail && logFeatureUsage('home', 'template_picker_open', { key: 'empty_state' })}
|
||||
>
|
||||
{#snippet trigger()}Start from a template{/snippet}
|
||||
{#snippet content({ close })}
|
||||
<HubTemplatePicker
|
||||
onPick={(project) => {
|
||||
close()
|
||||
onPick(project)
|
||||
}}
|
||||
/>
|
||||
{/snippet}
|
||||
</Popover>
|
||||
or
|
||||
{/if}
|
||||
<CreateActionsMenu source="empty_state" triggerElement={newLinkEl}>
|
||||
{#snippet trigger()}
|
||||
<!-- A bare <button> for a link inside a sentence, signed off by design: <Button>
|
||||
carries its own padding and background and cannot sit inline in running text.
|
||||
Inline links take `text-accent`, never a raw Tailwind blue.
|
||||
The full stop rides inside the snippet: across a component boundary Svelte
|
||||
keeps the markup whitespace, which would leave a gap before it. -->
|
||||
<button
|
||||
bind:this={newLinkEl}
|
||||
class="border-b border-transparent text-accent hover:border-accent"
|
||||
>{$disableHubStore ? 'Create a new one' : 'create a new one'}</button
|
||||
>.
|
||||
{/snippet}
|
||||
</CreateActionsMenu>
|
||||
<button
|
||||
bind:this={newLinkEl}
|
||||
class="border-b border-transparent text-accent hover:border-accent"
|
||||
>{$disableHubStore ? 'Create a new one' : 'create a new one'}</button
|
||||
>.
|
||||
{/snippet}
|
||||
</CreateActionsMenu>
|
||||
{/if}
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -150,6 +150,30 @@ describe('abandoning mid-import', () => {
|
||||
expect(run.itemResults.length).toBe(3)
|
||||
})
|
||||
|
||||
// What a caller acting on the run's leftovers depends on: `abandon()` only stops the next
|
||||
// phase, so a reload issued when it is called reads the workspace while the request already
|
||||
// sent is still landing. `whenIdle()` is the difference between reloading then and after.
|
||||
it('whenIdle resolves only once the abandoned run has stopped writing', async () => {
|
||||
const run = new ImportExecution(PLAN, deps)
|
||||
let idleResolved = false
|
||||
hooks.afterFirstItem = () => {
|
||||
run.abandon()
|
||||
void run.whenIdle().then(() => (idleResolved = true))
|
||||
// Still inside the run: the promise must not have resolved yet.
|
||||
expect(run.running).toBe(true)
|
||||
expect(idleResolved).toBe(false)
|
||||
}
|
||||
await run.run()
|
||||
await run.whenIdle()
|
||||
expect(run.running).toBe(false)
|
||||
expect(idleResolved).toBe(true)
|
||||
})
|
||||
|
||||
it('whenIdle resolves immediately when no run is in flight', async () => {
|
||||
const run = new ImportExecution(PLAN, deps)
|
||||
await expect(run.whenIdle()).resolves.toBeUndefined()
|
||||
})
|
||||
|
||||
it('stops the migrate row spinning when it is abandoned mid-migration', async () => {
|
||||
const run = new ImportExecution(PLAN, depsWithMigration)
|
||||
// After `onMigrationsStart`, which is where the row is actually set to running —
|
||||
|
||||
Reference in New Issue
Block a user