mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-20 16:03:21 +00:00
fix(frontend): warn before running a newly-created datatable migration out of order
The row-level Run action warns when earlier migrations are still pending, but the create-and-run paths ran a just-created migration with `only` directly, applying it ahead of older pending migrations without that confirmation. Reuse the same "Run migration out of order" confirmation across all create-and-run paths via a shared helper (datatableMigrationUtils): - NewDataTableMigrationModal "Create and run" (and the DDL guard path) - DatatableSchemaDiff fork→parent merge - dbOps schema ops (DB manager create/alter/drop) — the pure factory throws a MigrationRunCancelled sentinel on decline, which DBTableEditor treats as a silent cancel Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
78b68bee33
commit
b2643d8b63
@@ -20,6 +20,10 @@
|
||||
import type { SelectedTable } from './DBManager.svelte'
|
||||
import { getDbFeatures } from './apps/components/display/dbtable/dbFeatures'
|
||||
import { resource } from 'runed'
|
||||
import ConfirmationModal from './common/confirmationModal/ConfirmationModal.svelte'
|
||||
import { createAsyncConfirmationModal } from './common/confirmationModal/asyncConfirmationModal.svelte'
|
||||
import Portal from '$lib/components/Portal.svelte'
|
||||
import { outOfOrderRunMessage } from './workspaceSettings/datatableMigrationUtils'
|
||||
|
||||
interface Props {
|
||||
input?: DbInput
|
||||
@@ -52,6 +56,8 @@
|
||||
|
||||
let dbSchema: DBSchema | undefined = $derived(input && $dbSchemas[getDbSchemasPath(input)])
|
||||
|
||||
const outOfOrderModal = createAsyncConfirmationModal()
|
||||
|
||||
function getDbSchemasPath(input: DbInput): string {
|
||||
switch (input.type) {
|
||||
case 'database':
|
||||
@@ -163,7 +169,13 @@
|
||||
})}
|
||||
dbSchemaOps={dbSchemaOpsWithPreviewScripts({
|
||||
input: _input,
|
||||
workspace: $workspaceStore
|
||||
workspace: $workspaceStore,
|
||||
confirmRunOutOfOrder: (pending) =>
|
||||
outOfOrderModal.ask({
|
||||
title: 'Run migration out of order',
|
||||
confirmationText: 'Run anyway',
|
||||
children: outOfOrderRunMessage(pending)
|
||||
})
|
||||
})}
|
||||
initialTableKey={input.specificTable}
|
||||
initialSchemaKey={input.specificSchema}
|
||||
@@ -215,3 +227,7 @@
|
||||
</Pane>
|
||||
</Splitpanes>
|
||||
{/if}
|
||||
|
||||
<Portal>
|
||||
<ConfirmationModal {...outOfOrderModal.props} />
|
||||
</Portal>
|
||||
|
||||
@@ -52,6 +52,7 @@
|
||||
import Popover from './meltComponents/Popover.svelte'
|
||||
import ConfirmationModal from './common/confirmationModal/ConfirmationModal.svelte'
|
||||
import { sendUserToast } from '$lib/toast'
|
||||
import { MigrationRunCancelled } from './dbOps'
|
||||
import { getFlatTableNamesFromSchema, type DBSchema } from '$lib/stores'
|
||||
import { twMerge } from 'tailwind-merge'
|
||||
import DarkModeObserver from './DarkModeObserver.svelte'
|
||||
@@ -460,9 +461,13 @@
|
||||
askingForConfirmation && (askingForConfirmation.loading = true)
|
||||
await onConfirm({ values })
|
||||
} catch (e) {
|
||||
let msg: string | undefined = (e as any)?.body ?? (e as Error)?.message
|
||||
if (typeof msg !== 'string') msg = e ? JSON.stringify(e) : 'An error occurred'
|
||||
sendUserToast(msg, true)
|
||||
// User declined the out-of-order run warning: silent cancel,
|
||||
// leave the editor open so they can adjust or run earlier first.
|
||||
if (!(e instanceof MigrationRunCancelled)) {
|
||||
let msg: string | undefined = (e as any)?.body ?? (e as Error)?.message
|
||||
if (typeof msg !== 'string') msg = e ? JSON.stringify(e) : 'An error occurred'
|
||||
sendUserToast(msg, true)
|
||||
}
|
||||
}
|
||||
askingForConfirmation = undefined
|
||||
},
|
||||
|
||||
@@ -201,6 +201,12 @@
|
||||
import YAML from 'yaml'
|
||||
import DrawerContent from './common/drawer/DrawerContent.svelte'
|
||||
import ConfirmationModal from './common/confirmationModal/ConfirmationModal.svelte'
|
||||
import { createAsyncConfirmationModal } from './common/confirmationModal/asyncConfirmationModal.svelte'
|
||||
import Portal from '$lib/components/Portal.svelte'
|
||||
import {
|
||||
pendingMigrations,
|
||||
outOfOrderRunMessage
|
||||
} from './workspaceSettings/datatableMigrationUtils'
|
||||
import Alert from './common/alert/Alert.svelte'
|
||||
import ResizeTransitionWrapper from './common/ResizeTransitionWrapper.svelte'
|
||||
|
||||
@@ -229,6 +235,7 @@
|
||||
let migrationSql = $state('')
|
||||
let migrationRunning = $state(false)
|
||||
let confirmDeployOpen = $state(false)
|
||||
const outOfOrderModal = createAsyncConfirmationModal()
|
||||
|
||||
async function loadDiffs() {
|
||||
loading = true
|
||||
@@ -360,14 +367,27 @@
|
||||
// prevents. Let the error propagate (fail closed, handled by the outer
|
||||
// catch); only fall back to raw apply when the API explicitly returns
|
||||
// enabled === false.
|
||||
const targetUsesMigrations = (
|
||||
await WorkspaceService.getDatatableMigrationsStatus({
|
||||
workspace: targetWorkspace,
|
||||
datatableName: dtName
|
||||
})
|
||||
).enabled
|
||||
const status = await WorkspaceService.getDatatableMigrationsStatus({
|
||||
workspace: targetWorkspace,
|
||||
datatableName: dtName
|
||||
})
|
||||
|
||||
if (targetUsesMigrations) {
|
||||
if (status.enabled) {
|
||||
// The merge migration gets the highest timestamp, so any still-pending
|
||||
// migration on the target is earlier: running only the merge applies it
|
||||
// out of order. Warn like the row-level Run action does.
|
||||
const pending = pendingMigrations(status.migrations)
|
||||
if (pending.length > 0) {
|
||||
const confirmed = await outOfOrderModal.ask({
|
||||
title: 'Run migration out of order',
|
||||
confirmationText: 'Run anyway',
|
||||
children: outOfOrderRunMessage(pending.length)
|
||||
})
|
||||
if (!confirmed) {
|
||||
migrationRunning = false
|
||||
return
|
||||
}
|
||||
}
|
||||
const forkName =
|
||||
$userWorkspaces.find((w) => w.id === currentWorkspaceId)?.name || currentWorkspaceId
|
||||
const migName = `merge_${forkName}`.replace(/[^a-zA-Z0-9_-]+/g, '_')
|
||||
@@ -635,3 +655,7 @@
|
||||
>{migrationSql}</pre
|
||||
>
|
||||
</ConfirmationModal>
|
||||
|
||||
<Portal>
|
||||
<ConfirmationModal {...outOfOrderModal.props} />
|
||||
</Portal>
|
||||
|
||||
@@ -10,6 +10,7 @@ import { stringifySchema } from './copilot/lib'
|
||||
import type { DbInput, DbType } from './dbTypes'
|
||||
import { assert } from '$lib/utils'
|
||||
import { WorkspaceService } from '$lib/gen'
|
||||
import { pendingMigrations } from './workspaceSettings/datatableMigrationUtils'
|
||||
import {
|
||||
buildTableEditorValues,
|
||||
type TableEditorValues
|
||||
@@ -243,12 +244,25 @@ export type IDbSchemaOps = {
|
||||
}) => Promise<TableEditorValues>
|
||||
}
|
||||
|
||||
/** Thrown by a schema op when the user declines the out-of-order run warning.
|
||||
* Callers should treat it as a silent cancel (no error toast). */
|
||||
export class MigrationRunCancelled extends Error {
|
||||
constructor() {
|
||||
super('Migration run cancelled')
|
||||
this.name = 'MigrationRunCancelled'
|
||||
}
|
||||
}
|
||||
|
||||
export function dbSchemaOpsWithPreviewScripts({
|
||||
workspace,
|
||||
input
|
||||
input,
|
||||
confirmRunOutOfOrder
|
||||
}: {
|
||||
workspace: string
|
||||
input: DbInput
|
||||
/** Asked before running a just-created migration ahead of `pendingCount`
|
||||
* still-pending earlier ones. Return false to abort (throws MigrationRunCancelled). */
|
||||
confirmRunOutOfOrder?: (pendingCount: number) => Promise<boolean>
|
||||
}): IDbSchemaOps {
|
||||
const dbType = getDbType(input)
|
||||
const dbArg = getDatabaseArg(input)
|
||||
@@ -322,13 +336,22 @@ export function dbSchemaOpsWithPreviewScripts({
|
||||
// that would run the change untracked (schema drift) — exactly what this
|
||||
// feature prevents. Let the error propagate (fail closed); only fall back to
|
||||
// ad-hoc when there's no data table, or `enabled === false` is returned.
|
||||
const migrationsEnabled = datatableName
|
||||
? (await WorkspaceService.getDatatableMigrationsStatus({ workspace, datatableName })).enabled
|
||||
: false
|
||||
if (!datatableName || !migrationsEnabled) {
|
||||
const status = datatableName
|
||||
? await WorkspaceService.getDatatableMigrationsStatus({ workspace, datatableName })
|
||||
: undefined
|
||||
if (!datatableName || !status?.enabled) {
|
||||
await runScriptAndPollResult({ workspace, requestBody: { args: dbArg, content, language } })
|
||||
return
|
||||
}
|
||||
// The new migration gets the highest timestamp, so any still-pending
|
||||
// migration is earlier: running only this one applies it out of order.
|
||||
// Warn like the row-level Run action does (skipped if no confirm hook).
|
||||
if (confirmRunOutOfOrder) {
|
||||
const pending = pendingMigrations(status.migrations).length
|
||||
if (pending > 0 && !(await confirmRunOutOfOrder(pending))) {
|
||||
throw new MigrationRunCancelled()
|
||||
}
|
||||
}
|
||||
const codeUp = wrapMigration(await expandMarker(workspace, language, content))
|
||||
// Down migrations are only generated for Postgres for now.
|
||||
let codeDown: string | undefined
|
||||
|
||||
@@ -10,6 +10,10 @@
|
||||
import { WorkspaceService, type DatatableMigration } from '$lib/gen'
|
||||
import { sendUserToast } from '$lib/toast'
|
||||
import { tick } from 'svelte'
|
||||
import ConfirmationModal from '../common/confirmationModal/ConfirmationModal.svelte'
|
||||
import { createAsyncConfirmationModal } from '../common/confirmationModal/asyncConfirmationModal.svelte'
|
||||
import Portal from '$lib/components/Portal.svelte'
|
||||
import { fetchPendingMigrations, outOfOrderRunMessage } from './datatableMigrationUtils'
|
||||
|
||||
let {
|
||||
workspace,
|
||||
@@ -54,6 +58,8 @@
|
||||
let codeDown = $state('')
|
||||
let creating = $state(false)
|
||||
|
||||
const confirmationModal = createAsyncConfirmationModal()
|
||||
|
||||
// Frame the migration body in an explicit transaction so it applies atomically.
|
||||
function wrapInTransaction(body: string): string {
|
||||
return `BEGIN;\n\n${body}\n\nEND;`
|
||||
@@ -91,6 +97,24 @@
|
||||
sendUserToast("Invalid migration name: use only letters, digits, '_' and '-'", true)
|
||||
return
|
||||
}
|
||||
if (run) {
|
||||
// A new migration gets the highest timestamp, so any still-pending
|
||||
// migration is earlier: running only this one applies it out of order.
|
||||
// Warn just like the row-level Run action does. Best-effort: if the
|
||||
// status can't be fetched, fall through and let the run/rollback handle it.
|
||||
let pending: Awaited<ReturnType<typeof fetchPendingMigrations>> = []
|
||||
try {
|
||||
pending = await fetchPendingMigrations(workspace, datatable)
|
||||
} catch {}
|
||||
if (pending.length > 0) {
|
||||
const confirmed = await confirmationModal.ask({
|
||||
title: 'Run migration out of order',
|
||||
confirmationText: 'Run anyway',
|
||||
children: outOfOrderRunMessage(pending.length)
|
||||
})
|
||||
if (!confirmed) return
|
||||
}
|
||||
}
|
||||
creating = true
|
||||
try {
|
||||
const created = await WorkspaceService.createDatatableMigration({
|
||||
@@ -200,3 +224,7 @@
|
||||
</div>
|
||||
</div>
|
||||
</Modal2>
|
||||
|
||||
<Portal>
|
||||
<ConfirmationModal {...confirmationModal.props} />
|
||||
</Portal>
|
||||
|
||||
@@ -0,0 +1,30 @@
|
||||
import { WorkspaceService, type DatatableMigrationWithStatus } from '$lib/gen'
|
||||
|
||||
/**
|
||||
* Migrations that are defined but not yet applied. A newly-created migration
|
||||
* always gets the highest timestamp, so every pending migration is "earlier":
|
||||
* running the new one on its own would apply it ahead of them (out of order).
|
||||
*/
|
||||
export function pendingMigrations(
|
||||
migrations: DatatableMigrationWithStatus[]
|
||||
): DatatableMigrationWithStatus[] {
|
||||
return migrations.filter((m) => m.status !== 'ran')
|
||||
}
|
||||
|
||||
/** Fetch the data table's migration status and return the pending ones. */
|
||||
export async function fetchPendingMigrations(
|
||||
workspace: string,
|
||||
datatableName: string
|
||||
): Promise<DatatableMigrationWithStatus[]> {
|
||||
const { migrations } = await WorkspaceService.getDatatableMigrationsStatus({
|
||||
workspace,
|
||||
datatableName
|
||||
})
|
||||
return pendingMigrations(migrations)
|
||||
}
|
||||
|
||||
/** Confirmation copy shown before running a just-created migration ahead of
|
||||
* `count` still-pending earlier ones (mirrors the row-level Run warning). */
|
||||
export function outOfOrderRunMessage(count: number): string {
|
||||
return `${count} earlier migration(s) have not been run yet. This migration might depend on them. Run it anyway?`
|
||||
}
|
||||
Reference in New Issue
Block a user