mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: a legacy draft occupies its destination, a superseded opening writes nothing, and a loaded flow draft keeps no base it lacks
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
493448b2ce
commit
d3b87cf70b
+2
-2
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "SELECT\n (SELECT typ::text FROM draft WHERE workspace_id = $1 AND path = $3\n AND typ::text = ANY($6::text[]) AND email = $4 LIMIT 1) as \"at_target\",\n EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $5\n AND typ = $2 AND email = $4\n AND position(chr(92) || 'u0000' in replace(value::text, chr(92) || chr(92), '')) > 0\n ) as \"poisoned!\",\n EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $5\n AND typ = $2 AND email IS NULL) as \"legacy!\" ",
|
||||
"query": "SELECT\n (SELECT typ::text FROM draft WHERE workspace_id = $1 AND path = $3\n AND typ::text = ANY($6::text[]) AND (email = $4 OR email IS NULL)\n LIMIT 1) as \"at_target\",\n EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $5\n AND typ = $2 AND email = $4\n AND position(chr(92) || 'u0000' in replace(value::text, chr(92) || chr(92), '')) > 0\n ) as \"poisoned!\",\n EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $5\n AND typ = $2 AND email IS NULL) as \"legacy!\" ",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
@@ -69,5 +69,5 @@
|
||||
null
|
||||
]
|
||||
},
|
||||
"hash": "4a125e25142bdfb71f011ffe08e7d0be51e007b2e117af240cd1cf035a6ef0ff"
|
||||
"hash": "3e3651d23d759a4cb3480afb5d12a05791832d91ca27ab2be3370ad19e46fc1a"
|
||||
}
|
||||
+2
-2
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "UPDATE draft\n SET path = $3,\n -- Both path keys, not just the typed one: the editors mirror the\n -- typed path into the other while it differs from the row's path,\n -- and the loaders prefer the mirror — left naming the old location\n -- it un-does this move on the next save. `create_missing = false`\n -- on both, so a draft carrying only one keeps only one.\n value = to_json(\n jsonb_set(\n jsonb_set(\n CASE WHEN $7::text IS NULL THEN to_jsonb(value)\n ELSE jsonb_set(to_jsonb(value), ARRAY['summary'], to_jsonb($7::text))\n END,\n ARRAY[$5::text], to_jsonb($3::text), false\n ),\n ARRAY[$8::text], to_jsonb($3::text), false\n )\n )\n WHERE workspace_id = $1\n AND path = $2\n AND typ = $4\n AND email = $6\n -- A pre-sanitizer NUL escape makes `to_jsonb` raise 22P05. Excluded\n -- here so the statement can't 500; reported below instead. Unlike the\n -- passive carry, rewriting the value IS this operation, so skipping it\n -- silently would move the row and leave its typed path stale.\n AND position(chr(92) || 'u0000' in replace(value::text, chr(92) || chr(92), '')) = 0\n -- Skipped on a summary-only edit, where the \"target\" row is this\n -- row and the guard would refuse the update against itself.\n AND ($2 = $3 OR NOT EXISTS (\n SELECT 1 FROM draft o\n WHERE o.workspace_id = $1 AND o.path = $3 AND o.typ::text = ANY($9::text[])\n AND o.email = $6\n ))\n RETURNING id",
|
||||
"query": "UPDATE draft\n SET path = $3,\n -- Both path keys, not just the typed one: the editors mirror the\n -- typed path into the other while it differs from the row's path,\n -- and the loaders prefer the mirror — left naming the old location\n -- it un-does this move on the next save. `create_missing = false`\n -- on both, so a draft carrying only one keeps only one.\n value = to_json(\n jsonb_set(\n jsonb_set(\n CASE WHEN $7::text IS NULL THEN to_jsonb(value)\n ELSE jsonb_set(to_jsonb(value), ARRAY['summary'], to_jsonb($7::text))\n END,\n ARRAY[$5::text], to_jsonb($3::text), false\n ),\n ARRAY[$8::text], to_jsonb($3::text), false\n )\n )\n WHERE workspace_id = $1\n AND path = $2\n AND typ = $4\n AND email = $6\n -- A pre-sanitizer NUL escape makes `to_jsonb` raise 22P05. Excluded\n -- here so the statement can't 500; reported below instead. Unlike the\n -- passive carry, rewriting the value IS this operation, so skipping it\n -- silently would move the row and leave its typed path stale.\n AND position(chr(92) || 'u0000' in replace(value::text, chr(92) || chr(92), '')) = 0\n -- Skipped on a summary-only edit, where the \"target\" row is this\n -- row and the guard would refuse the update against itself.\n AND ($2 = $3 OR NOT EXISTS (\n SELECT 1 FROM draft o\n WHERE o.workspace_id = $1 AND o.path = $3 AND o.typ::text = ANY($9::text[])\n -- The legacy row counts: a deploy at that path wipes it together with\n -- the caller's, so parking a second draft there discards edits the\n -- caller never saw.\n AND (o.email = $6 OR o.email IS NULL)\n ))\n RETURNING id",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
@@ -60,5 +60,5 @@
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "e27e447e4d9607fe280807e396049bbe31e4e938d9f53eeb659cdc49f9be8f2c"
|
||||
"hash": "cb135f028eb579ee912c618f48f38d99f84311ac3297f04bc19425a4bddceb33"
|
||||
}
|
||||
@@ -9805,7 +9805,7 @@ paths:
|
||||
format: date-time
|
||||
path:
|
||||
type: string
|
||||
description: "`saved` upserts only: where the draft is. Differs from the URL path when the item had moved away from it; the editor follows it there."
|
||||
description: "`saved` only, upsert or delete: where the write landed. Differs from the URL path when the item had moved away from it; the editor follows it there."
|
||||
required: [status, current_timestamp]
|
||||
|
||||
/w/{workspace}/drafts/move/{kind}/{path}:
|
||||
|
||||
@@ -739,7 +739,10 @@ async fn move_draft(
|
||||
AND ($2 = $3 OR NOT EXISTS (
|
||||
SELECT 1 FROM draft o
|
||||
WHERE o.workspace_id = $1 AND o.path = $3 AND o.typ::text = ANY($9::text[])
|
||||
AND o.email = $6
|
||||
-- The legacy row counts: a deploy at that path wipes it together with
|
||||
-- the caller's, so parking a second draft there discards edits the
|
||||
-- caller never saw.
|
||||
AND (o.email = $6 OR o.email IS NULL)
|
||||
))
|
||||
RETURNING id"#,
|
||||
&w_id,
|
||||
@@ -771,7 +774,8 @@ async fn move_draft(
|
||||
let row = sqlx::query!(
|
||||
r#"SELECT
|
||||
(SELECT typ::text FROM draft WHERE workspace_id = $1 AND path = $3
|
||||
AND typ::text = ANY($6::text[]) AND email = $4 LIMIT 1) as "at_target",
|
||||
AND typ::text = ANY($6::text[]) AND (email = $4 OR email IS NULL)
|
||||
LIMIT 1) as "at_target",
|
||||
EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $5
|
||||
AND typ = $2 AND email = $4
|
||||
AND position(chr(92) || 'u0000' in replace(value::text, chr(92) || chr(92), '')) > 0
|
||||
|
||||
@@ -472,7 +472,7 @@
|
||||
}
|
||||
}
|
||||
}
|
||||
async function syncWithDeployed() {
|
||||
async function syncWithDeployed(opening?: number) {
|
||||
const flow = await FlowService.getFlowByPath({
|
||||
workspace: opWorkspace!,
|
||||
// The draft row's own path, not `initialPath` — the route re-seeds that from
|
||||
@@ -482,6 +482,9 @@
|
||||
path: userDraftPath || initialPath,
|
||||
withStarredInfo: true
|
||||
})
|
||||
// A superseded opening must not write these: the current one would then render
|
||||
// and offer Take latest against the older head.
|
||||
if (opening != null && opening !== diffOpening) return
|
||||
deployedValue = replaceFalseWithUndefined({
|
||||
...flow,
|
||||
edited_at: undefined,
|
||||
@@ -1170,8 +1173,10 @@
|
||||
export async function openDiffDrawer() {
|
||||
const opening = ++diffOpening
|
||||
if (!savedFlow) return
|
||||
await syncWithDeployed()
|
||||
await syncWithDeployed(opening)
|
||||
const currentDraftTriggers = structuredClone(triggersState.getDraftTriggersSnapshot())
|
||||
// Blanking the drawer belongs to the opening that will fill it.
|
||||
if (opening !== diffOpening) return
|
||||
diffDrawer?.openDrawer()
|
||||
const currentFlow = flowStore.val
|
||||
const versions = await deployedVersionOptions()
|
||||
|
||||
@@ -616,7 +616,7 @@
|
||||
}
|
||||
}
|
||||
|
||||
async function syncWithDeployed() {
|
||||
async function syncWithDeployed(opening?: number) {
|
||||
const latestScript = await ScriptService.getScriptByPath({
|
||||
workspace: opWorkspace!,
|
||||
// The draft row's own path, not `initialPath` — that one tracks the path
|
||||
@@ -627,6 +627,9 @@
|
||||
withStarredInfo: true
|
||||
})
|
||||
|
||||
// A superseded opening must not write these: the current one would then render
|
||||
// and offer Take latest against the older head.
|
||||
if (opening != null && opening !== diffOpening) return
|
||||
deployedValue = replaceFalseWithUndefined({
|
||||
...latestScript,
|
||||
workspace_id: undefined,
|
||||
@@ -879,7 +882,7 @@
|
||||
if (!savedScript) {
|
||||
return
|
||||
}
|
||||
await syncWithDeployed()
|
||||
await syncWithDeployed(opening)
|
||||
|
||||
const currentDraftTriggers = structuredClone(triggersState.getDraftTriggersSnapshot())
|
||||
|
||||
@@ -896,6 +899,8 @@
|
||||
}
|
||||
if (current.assets && !current.assets.length) delete current.assets
|
||||
|
||||
// Blanking the drawer belongs to the opening that will fill it.
|
||||
if (opening !== diffOpening) return
|
||||
diffDrawer?.openDrawer()
|
||||
const headHash = (deployed as { hash?: string } | undefined)?.hash
|
||||
const versions = await deployedVersionOptions(headHash)
|
||||
|
||||
@@ -397,13 +397,16 @@
|
||||
}
|
||||
}
|
||||
|
||||
async function syncWithDeployed() {
|
||||
async function syncWithDeployed(opening?: number) {
|
||||
const deployedApp = await AppService.getAppByPath({
|
||||
workspace: opWorkspace!,
|
||||
path: appPath!,
|
||||
withStarredInfo: true
|
||||
})
|
||||
|
||||
// A superseded opening must not write these: the current one would then render
|
||||
// and offer Take latest against the older head.
|
||||
if (opening != null && opening !== diffOpening) return
|
||||
deployedBy = deployedApp.created_by
|
||||
const shownVersions = (deployedApp as { versions?: number[] }).versions
|
||||
deployedVersionShown = Array.isArray(shownVersions)
|
||||
@@ -461,8 +464,10 @@
|
||||
}
|
||||
|
||||
// deployedValue should be syncronized when we open Diff
|
||||
await syncWithDeployed()
|
||||
await syncWithDeployed(opening)
|
||||
|
||||
// Blanking the drawer belongs to the opening that will fill it.
|
||||
if (opening !== diffOpening) return
|
||||
diffDrawer?.openDrawer()
|
||||
const versions = await deployedVersionOptions()
|
||||
if (opening !== diffOpening) return
|
||||
|
||||
@@ -405,9 +405,12 @@
|
||||
: effectiveFlow
|
||||
flow = flowToRender
|
||||
if (pendingLoad) {
|
||||
// Their draft's base, not ours; see /scripts/edit.
|
||||
// Their draft's base, not ours; see /scripts/edit. With none, the merge above
|
||||
// would hand it the deployed one and the next save would persist that as its
|
||||
// base, so it is dropped rather than inherited.
|
||||
const theirs = (pendingLoad.value as { version_id?: number })?.version_id
|
||||
draftBaseVersion = theirs != null ? String(theirs) : undefined
|
||||
if (theirs == null) delete (flowToRender as any).version_id
|
||||
}
|
||||
if (pendingLoad && hasOwnDraft) {
|
||||
OtherUserDraftLoad.beginOverlay({
|
||||
|
||||
Reference in New Issue
Block a user