diff --git a/backend/.sqlx/query-07a759bde2efada8e71a29d059b5ef5575a908d8410418ef7fff1795bc0ec1f8.json b/backend/.sqlx/query-e34f4d6a151cc791960c61197a4b00ea9db2f6ee47dc154faefe4c06c41cf1cf.json similarity index 51% rename from backend/.sqlx/query-07a759bde2efada8e71a29d059b5ef5575a908d8410418ef7fff1795bc0ec1f8.json rename to backend/.sqlx/query-e34f4d6a151cc791960c61197a4b00ea9db2f6ee47dc154faefe4c06c41cf1cf.json index 0573b646b6..dcacf92573 100644 --- a/backend/.sqlx/query-07a759bde2efada8e71a29d059b5ef5575a908d8410418ef7fff1795bc0ec1f8.json +++ b/backend/.sqlx/query-e34f4d6a151cc791960c61197a4b00ea9db2f6ee47dc154faefe4c06c41cf1cf.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT d.email IS NULL as \"legacy!\", COALESCE(u.username, p.username) as username\n FROM draft d\n LEFT JOIN usr u ON u.workspace_id = d.workspace_id AND u.email = d.email\n LEFT JOIN password p ON p.email = d.email AND p.super_admin = true\n WHERE d.workspace_id = $1 AND d.path = $2 AND d.typ::text = ANY($3::text[])\n ORDER BY 2", + "query": "SELECT d.email IS NULL as \"legacy!\", COALESCE(u.username, p.username) as username\n FROM draft d\n LEFT JOIN usr u ON u.workspace_id = d.workspace_id AND u.email = d.email\n LEFT JOIN password p ON p.email = d.email AND p.super_admin = true\n WHERE d.workspace_id = $1 AND d.path = $2 AND d.typ::text = ANY($3::text[])\n -- A row of the consuming owner's that a move brought here from the very path\n -- being renamed is the draft this deploy carries, not an item in its way. Any\n -- other row of theirs is a second item and still collides.\n AND ($4::text IS NULL OR d.email IS DISTINCT FROM $4 OR NOT EXISTS (\n SELECT 1 FROM draft_move m\n WHERE m.workspace_id = $1 AND m.typ::text = ANY($3::text[])\n AND m.old_path = $5 AND m.new_path = $2\n AND (m.email IS NULL OR m.email = $4)\n ))\n ORDER BY 2", "describe": { "columns": [ { @@ -18,7 +18,9 @@ "Left": [ "Text", "Text", - "TextArray" + "TextArray", + "Text", + "Text" ] }, "nullable": [ @@ -26,5 +28,5 @@ null ] }, - "hash": "07a759bde2efada8e71a29d059b5ef5575a908d8410418ef7fff1795bc0ec1f8" + "hash": "e34f4d6a151cc791960c61197a4b00ea9db2f6ee47dc154faefe4c06c41cf1cf" } diff --git a/backend/tests/drafts_save_follows_move.rs b/backend/tests/drafts_save_follows_move.rs index b4697445c1..c87ad9adf7 100644 --- a/backend/tests/drafts_save_follows_move.rs +++ b/backend/tests/drafts_save_follows_move.rs @@ -480,3 +480,53 @@ async fn test_a_legacy_discard_follows_a_rename(db: Pool) -> anyhow::R ); Ok(()) } + +/// Deploying a draft that a move carried off an archived script: its parent is still the +/// version at the old path, so the deploy renames from there and carries what is left over +/// — onto the very draft being deployed. That row is this deploy's own, not an item in its +/// way, or the deploy is refused and every retry refuses again. +#[sqlx::test(fixtures("base", "drafts_save_follows_move"))] +async fn test_a_moved_draft_deploys_at_its_new_path(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + + sqlx::query("UPDATE script SET archived = true WHERE path = 'u/test-user/follow_a'") + .execute(&db) + .await?; + let resp = reqwest::Client::new() + .post(format!( + "http://localhost:{port}/api/w/test-workspace/drafts/move/script/u/test-user/follow_a" + )) + .header("Authorization", "Bearer SECRET_TOKEN") + .json(&json!({ "new_path": "u/test-user/follow_b" })) + .send() + .await?; + assert!( + resp.status().is_success(), + "move failed: {}", + resp.text().await? + ); + + // Deploy it where it now lives, still parented on the archived version it forked from. + let resp = reqwest::Client::new() + .post(format!( + "http://localhost:{port}/api/w/test-workspace/scripts/create" + )) + .header("Authorization", "Bearer SECRET_TOKEN") + .json(&json!({ + "path": "u/test-user/follow_b", + "parent_hash": HEAD_HASH, + "summary": "A", + "description": "", + "content": "export function main() { return 2 }", + "language": "deno", + "schema": {} + })) + .send() + .await?; + let status = resp.status(); + let body = resp.text().await?; + assert_eq!(status, 201, "the moved draft could not be deployed: {body}"); + Ok(()) +} diff --git a/backend/windmill-api-flows/src/flows.rs b/backend/windmill-api-flows/src/flows.rs index 92d291344b..8d9b222199 100644 --- a/backend/windmill-api-flows/src/flows.rs +++ b/backend/windmill-api-flows/src/flows.rs @@ -1426,6 +1426,7 @@ async fn update_flow( &[UserDraftItemKind::Flow], flow_path, &nf.path, + (!nf.skip_draft_deletion.unwrap_or(false)).then_some(authed.email.as_str()), ) .await?; } diff --git a/backend/windmill-api-scripts/src/scripts.rs b/backend/windmill-api-scripts/src/scripts.rs index 1eebfc1389..f0e11fe964 100644 --- a/backend/windmill-api-scripts/src/scripts.rs +++ b/backend/windmill-api-scripts/src/scripts.rs @@ -2322,6 +2322,7 @@ async fn create_script_internal<'c>( &[UserDraftItemKind::Script], p_path, &ns.path, + (!skip_draft_deletion).then_some(authed.email.as_str()), ) .await?; } diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index 9931198a75..bfc554e81b 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -3770,6 +3770,7 @@ async fn update_app_internal<'a>( &[UserDraftItemKind::App, UserDraftItemKind::RawApp], path, &npath, + (!ns.skip_draft_deletion.unwrap_or(false)).then_some(authed.email.as_str()), ) .await?; } diff --git a/backend/windmill-common/src/user_drafts.rs b/backend/windmill-common/src/user_drafts.rs index cf9fcee3fd..5aab500942 100644 --- a/backend/windmill-common/src/user_drafts.rs +++ b/backend/windmill-common/src/user_drafts.rs @@ -662,13 +662,16 @@ pub async fn delete_own_draft_for_path( /// /// A draft already at `new_path` occupies it the way a deployed item does, so the move is /// refused with `BadRequest` inside the deploy's transaction, refusing the rename itself: -/// moving onto it would merge two items or strand the row that lost. +/// moving onto it would merge two items or strand the row that lost. `consumed_by` names +/// the owner whose row there this deploy consumes, which is the draft being deployed after +/// an earlier move carried it, and so cannot be in its own way. pub async fn move_drafts_for_path( tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, w_id: &str, kinds: &[UserDraftItemKind], old_path: &str, new_path: &str, + consumed_by: Option<&str>, ) -> Result<()> { let typs = kinds.iter().map(|k| k.as_str()).collect::>(); // Named by workspace username, as the editors name other users' drafts: the @@ -679,10 +682,21 @@ pub async fn move_drafts_for_path( LEFT JOIN usr u ON u.workspace_id = d.workspace_id AND u.email = d.email LEFT JOIN password p ON p.email = d.email AND p.super_admin = true WHERE d.workspace_id = $1 AND d.path = $2 AND d.typ::text = ANY($3::text[]) + -- A row of the consuming owner's that a move brought here from the very path + -- being renamed is the draft this deploy carries, not an item in its way. Any + -- other row of theirs is a second item and still collides. + AND ($4::text IS NULL OR d.email IS DISTINCT FROM $4 OR NOT EXISTS ( + SELECT 1 FROM draft_move m + WHERE m.workspace_id = $1 AND m.typ::text = ANY($3::text[]) + AND m.old_path = $5 AND m.new_path = $2 + AND (m.email IS NULL OR m.email = $4) + )) ORDER BY 2"#, w_id, new_path, &typs as &[&str], + consumed_by, + old_path, ) .fetch_all(&mut **tx) .await?;