mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-05 08:02:18 +00:00
fix: supply the three APIs the import wizard already calls
`AppConnectDrawer`, `ImportProjectStep` and `execution.svelte.ts` landed calling into props and exports that were never committed alongside them, so the branch did not type-check. Each half is here now: - `AppConnectInner.fillPath` — connect into a resource that already exists instead of refusing the path. The import creates every resource as an empty stub, so without it the connect flow can only ever say "already exists, delete it or pick another path". Opt-in: unset, the flow still refuses to write over anything, which is what `ResourcePicker` and the resources page rely on. - `ProjectContentBadges.contentSummary` — the badge counts as one line of text, for the import step's task row. Shares `kinds()` with the badges so a project cannot be counted two ways. - `installProject.onMigrationsStart` — fires before the reviewed migrations run, which is the only signal that phase has begun; the import step draws them as their own checklist row off the back of it. Also fixes the wizard wedging itself shut: `requestClose` set `dismissing` and cleared it after awaiting the confirmation, so an `ask` that threw left the flag set — and the backdrop, Escape and the close button all return early on it, leaving a reload as the only way out. Now `finally`, plus a reset on open, since a promise that never settles never reaches `finally`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Guilhem Lemouel
co-authored by
Claude Opus 5
Claude Opus 5
parent
a566b5ee17
commit
9c1ef71270
@@ -52,6 +52,15 @@
|
||||
manual?: boolean
|
||||
express?: boolean
|
||||
workspace?: string
|
||||
/**
|
||||
* Fill an existing resource instead of creating one. The path is fixed to it and the
|
||||
* "already exists" guard becomes an update, so a caller holding a resource that is
|
||||
* already there — the import wizard's empty stubs — can connect into it rather than
|
||||
* making the user delete it first and retype the path.
|
||||
*
|
||||
* Opt-in: without it this flow still refuses to write over anything.
|
||||
*/
|
||||
fillPath?: string
|
||||
}
|
||||
|
||||
let {
|
||||
@@ -61,7 +70,8 @@
|
||||
disabled = $bindable(false),
|
||||
manual = $bindable(true),
|
||||
express = false,
|
||||
workspace = undefined
|
||||
workspace = undefined,
|
||||
fillPath = undefined
|
||||
}: Props = $props()
|
||||
|
||||
let effectiveWorkspace = $derived(workspace ?? $workspaceStore!)
|
||||
@@ -553,8 +563,9 @@
|
||||
valueToken = data.res
|
||||
responseExtra = data.extra ?? {}
|
||||
step = 4
|
||||
if (express) {
|
||||
path = `u/${$userStore?.username}/${resourceType}_${new Date().getTime()}`
|
||||
// `fillPath` decides the path as surely as express does, so neither stops here.
|
||||
if (fillPath || express) {
|
||||
path = fillPath ?? `u/${$userStore?.username}/${resourceType}_${new Date().getTime()}`
|
||||
next()
|
||||
}
|
||||
}
|
||||
@@ -689,8 +700,8 @@
|
||||
grant_type: 'client_credentials' // Mark this token as client_credentials
|
||||
}
|
||||
step = 4
|
||||
if (express) {
|
||||
path = `u/${$userStore?.username}/${resourceType}_${new Date().getTime()}`
|
||||
if (fillPath || express) {
|
||||
path = fillPath ?? `u/${$userStore?.username}/${resourceType}_${new Date().getTime()}`
|
||||
next()
|
||||
}
|
||||
} catch (error) {
|
||||
@@ -749,7 +760,10 @@
|
||||
path
|
||||
})
|
||||
|
||||
if (exists) {
|
||||
// Filling one names its path up front; anything else reaching an occupied path got
|
||||
// there by the user typing it, which is the case worth refusing.
|
||||
const filling = exists && !!fillPath && path === fillPath
|
||||
if (exists && !filling) {
|
||||
throw Error(`Resource at path ${path} already exists. Delete it or pick another path`)
|
||||
}
|
||||
|
||||
@@ -895,17 +909,27 @@
|
||||
}
|
||||
}
|
||||
|
||||
await ResourceService.createResource({
|
||||
workspace: effectiveWorkspace,
|
||||
requestBody: {
|
||||
resource_type: resourceType,
|
||||
if (filling) {
|
||||
// The stub the import made carries no description, so this is the one chance to
|
||||
// give it one; its resource_type and path are already what we want.
|
||||
await ResourceService.updateResource({
|
||||
workspace: effectiveWorkspace,
|
||||
path,
|
||||
value: resourceValue,
|
||||
description,
|
||||
labels,
|
||||
ws_specific: wsSpecific
|
||||
}
|
||||
})
|
||||
requestBody: { value: resourceValue, description }
|
||||
})
|
||||
} else {
|
||||
await ResourceService.createResource({
|
||||
workspace: effectiveWorkspace,
|
||||
requestBody: {
|
||||
resource_type: resourceType,
|
||||
path,
|
||||
value: resourceValue,
|
||||
description,
|
||||
labels,
|
||||
ws_specific: wsSpecific
|
||||
}
|
||||
})
|
||||
}
|
||||
dispatch('refresh', path)
|
||||
dispatch('close')
|
||||
sendUserToast(
|
||||
|
||||
@@ -8,6 +8,30 @@
|
||||
triggers?: number
|
||||
migrations?: number
|
||||
}
|
||||
|
||||
/** The kinds, in the order a project is read. Shared by the badges and the sentence
|
||||
* below so the same project can never be counted two ways. */
|
||||
function kinds(counts: ProjectContentCounts) {
|
||||
return [
|
||||
{ label: 'app', count: counts.apps },
|
||||
{ label: 'flow', count: counts.flows },
|
||||
{ label: 'script', count: counts.scripts },
|
||||
{ label: 'resource', count: counts.resources },
|
||||
{ label: 'trigger', count: counts.triggers ?? 0 },
|
||||
{ label: 'data table migration', count: counts.migrations ?? 0 }
|
||||
].filter((c) => c.count > 0)
|
||||
}
|
||||
|
||||
/**
|
||||
* The same counts as one line of text, for callers with a row to sit on rather than
|
||||
* a space for chips — the import step names them beside the task that imports them.
|
||||
* Empty when a project has nothing in it, so a caller can drop the whole phrase.
|
||||
*/
|
||||
export function contentSummary(counts: ProjectContentCounts): string {
|
||||
return kinds(counts)
|
||||
.map((c) => `${c.count} ${c.label}${c.count === 1 ? '' : 's'}`)
|
||||
.join(', ')
|
||||
}
|
||||
</script>
|
||||
|
||||
<script lang="ts">
|
||||
@@ -28,16 +52,15 @@
|
||||
//
|
||||
// Zero counts are dropped rather than shown: a project with no apps should read
|
||||
// as "no apps", not as a "0 apps" chip the eye has to discount.
|
||||
const shown = $derived(
|
||||
[
|
||||
{ label: 'app', count: counts.apps, icon: LayoutDashboard },
|
||||
{ label: 'flow', count: counts.flows, icon: BarsStaggered },
|
||||
{ label: 'script', count: counts.scripts, icon: Code2 },
|
||||
{ label: 'resource', count: counts.resources, icon: Database },
|
||||
{ label: 'trigger', count: counts.triggers ?? 0, icon: Zap },
|
||||
{ label: 'data table migration', count: counts.migrations ?? 0, icon: Table2 }
|
||||
].filter((c) => c.count > 0)
|
||||
)
|
||||
const ICONS: Record<string, any> = {
|
||||
app: LayoutDashboard,
|
||||
flow: BarsStaggered,
|
||||
script: Code2,
|
||||
resource: Database,
|
||||
trigger: Zap,
|
||||
'data table migration': Table2
|
||||
}
|
||||
const shown = $derived(kinds(counts).map((c) => ({ ...c, icon: ICONS[c.label] })))
|
||||
</script>
|
||||
|
||||
<div class="flex flex-wrap items-center gap-1.5">
|
||||
|
||||
@@ -399,6 +399,9 @@
|
||||
|
||||
function reset(from: WizardResume | undefined) {
|
||||
resumedPath = from?.resourcePath
|
||||
// A pending confirmation that never settled leaves `dismissing` true, and `finally`
|
||||
// cannot clear what never resolves — so a fresh open always starts dismissable.
|
||||
dismissing = false
|
||||
wiz = newWizardState({
|
||||
name: from?.name || defaultTableName(),
|
||||
projectName: from?.projectName || defaultProjectName(),
|
||||
@@ -771,19 +774,25 @@
|
||||
return
|
||||
}
|
||||
dismissing = true
|
||||
const confirmed = await confirmationModal.ask({
|
||||
title: 'Leave without adding a data table?',
|
||||
// A run that failed and was sent back to be edited leaves whatever it got through
|
||||
// behind it, so promising otherwise would be a lie exactly when it matters most.
|
||||
children: leftBehind
|
||||
? 'The setup that already ran left what it created behind, and what you have filled in here will be lost.'
|
||||
: 'Nothing has been created yet, and what you have filled in here will be lost.',
|
||||
confirmationText: 'Discard'
|
||||
})
|
||||
dismissing = false
|
||||
// Re-read rather than trust the entry check: a run can start while the dialog is up, and
|
||||
// answering Discard would otherwise tear the modal down in the middle of it.
|
||||
if (confirmed && !preventClose) close()
|
||||
// `finally`, because the flag is what blocks a second attempt: an `ask` that throws
|
||||
// would otherwise leave the dialog permanently undismissable — the backdrop, Escape
|
||||
// and the close button all return early here, so the only way out would be a reload.
|
||||
try {
|
||||
const confirmed = await confirmationModal.ask({
|
||||
title: 'Leave without adding a data table?',
|
||||
// A run that failed and was sent back to be edited leaves whatever it got through
|
||||
// behind it, so promising otherwise would be a lie exactly when it matters most.
|
||||
children: leftBehind
|
||||
? 'The setup that already ran left what it created behind, and what you have filled in here will be lost.'
|
||||
: 'Nothing has been created yet, and what you have filled in here will be lost.',
|
||||
confirmationText: 'Discard'
|
||||
})
|
||||
// Re-read rather than trust the entry check: a run can start while the dialog is up, and
|
||||
// answering Discard would otherwise tear the modal down in the middle of it.
|
||||
if (confirmed && !preventClose) close()
|
||||
} finally {
|
||||
dismissing = false
|
||||
}
|
||||
}
|
||||
|
||||
function close() {
|
||||
|
||||
@@ -263,10 +263,14 @@ export async function installProject(args: {
|
||||
exportData: ProjectExport
|
||||
folder: string
|
||||
migrations: ProjectMigration[]
|
||||
/** Called once, before the reviewed migrations are applied, when there are any. Lets a
|
||||
* caller show them as their own step rather than folding them into the item import. */
|
||||
onMigrationsStart?: () => void
|
||||
hasEeLicense: boolean
|
||||
onResult: (r: InstallResult) => void
|
||||
}): Promise<void> {
|
||||
const { workspace, exportData, folder, migrations, hasEeLicense, onResult } = args
|
||||
const { workspace, exportData, folder, migrations, hasEeLicense, onResult, onMigrationsStart } =
|
||||
args
|
||||
|
||||
const record = (path: string, p: Promise<unknown>): Promise<void> =>
|
||||
p.then(
|
||||
@@ -393,6 +397,7 @@ export async function installProject(args: {
|
||||
}
|
||||
|
||||
// Apply the reviewed data table migrations after items exist.
|
||||
if (migrations.length) onMigrationsStart?.()
|
||||
for (const m of migrations) {
|
||||
await record(
|
||||
`data table: ${m.datatable_name}`,
|
||||
|
||||
Reference in New Issue
Block a user