diff --git a/backend/.sqlx/query-6fe3a24f32f07156c2b91794cfd61a4b8d33ffd83fa78c0f8ad2ec8a111f6bbd.json b/backend/.sqlx/query-e63da92247045f2aaaa62b9d6ee17510fd05d0b7c5311b61e7257ccd07b16da2.json similarity index 71% rename from backend/.sqlx/query-6fe3a24f32f07156c2b91794cfd61a4b8d33ffd83fa78c0f8ad2ec8a111f6bbd.json rename to backend/.sqlx/query-e63da92247045f2aaaa62b9d6ee17510fd05d0b7c5311b61e7257ccd07b16da2.json index 629b3a4acb..926add8159 100644 --- a/backend/.sqlx/query-6fe3a24f32f07156c2b91794cfd61a4b8d33ffd83fa78c0f8ad2ec8a111f6bbd.json +++ b/backend/.sqlx/query-e63da92247045f2aaaa62b9d6ee17510fd05d0b7c5311b61e7257ccd07b16da2.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT m.new_path FROM draft_move m\n WHERE m.workspace_id = $1 AND m.typ = $2 AND m.old_path = $3\n AND (m.email IS NULL OR m.email = $4)\n AND NOT EXISTS (\n SELECT 1 FROM draft d\n WHERE d.workspace_id = $1 AND d.typ = $2 AND d.path = $3 AND d.email = $4\n )\n ORDER BY m.email IS NULL\n LIMIT 1", + "query": "SELECT m.new_path FROM draft_move m\n WHERE m.workspace_id = $1 AND m.typ = $2 AND m.old_path = $3\n AND (m.email IS NULL OR m.email = $4)\n AND NOT EXISTS (\n SELECT 1 FROM draft d\n WHERE d.workspace_id = $1 AND d.typ = $2 AND d.path = $3\n AND d.email IS NOT DISTINCT FROM $4\n )\n ORDER BY m.email IS NULL\n LIMIT 1", "describe": { "columns": [ { @@ -55,5 +55,5 @@ false ] }, - "hash": "6fe3a24f32f07156c2b91794cfd61a4b8d33ffd83fa78c0f8ad2ec8a111f6bbd" + "hash": "e63da92247045f2aaaa62b9d6ee17510fd05d0b7c5311b61e7257ccd07b16da2" } diff --git a/backend/tests/drafts_save_follows_move.rs b/backend/tests/drafts_save_follows_move.rs index 6f3c7692ee..5357630737 100644 --- a/backend/tests/drafts_save_follows_move.rs +++ b/backend/tests/drafts_save_follows_move.rs @@ -369,3 +369,41 @@ async fn test_a_poisoned_draft_follows_a_rename(db: Pool) -> anyhow::R assert_eq!(row.2, "ab", "the NUL survived the rewrite"); Ok(()) } + +/// The legacy workspace-level row is carried by a rename like any other draft, and the +/// record that routes saves to it covers every caller — so discarding it from a page that +/// still names the old path has to reach it where it went. +#[sqlx::test(fixtures("base", "drafts_save_follows_move"))] +async fn test_a_legacy_discard_follows_a_rename(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + + sqlx::query( + r#"INSERT INTO draft (workspace_id, path, typ, value, email) + VALUES ('test-workspace', 'u/test-user/follow_a', 'script', + '{"path": "u/test-user/follow_a", "summary": "legacy", "content": "x"}', NULL)"#, + ) + .execute(&db) + .await?; + + rename(port, HEAD_HASH, "u/test-user/follow_b").await?; + + let resp = reqwest::Client::new() + .post(format!( + "http://localhost:{port}/api/w/test-workspace/drafts/update/script/u/test-user/follow_a" + )) + .header("Authorization", "Bearer SECRET_TOKEN") + .json(&json!({ "value": null, "legacy": true })) + .send() + .await?; + assert!(resp.status().is_success(), "discard failed: {}", resp.text().await?); + + let left: i64 = sqlx::query_scalar( + "SELECT count(*) FROM draft WHERE workspace_id = 'test-workspace' AND email IS NULL", + ) + .fetch_one(&db) + .await?; + assert_eq!(left, 0, "the legacy draft survived a discard aimed at its old path"); + Ok(()) +} diff --git a/backend/windmill-api/src/drafts.rs b/backend/windmill-api/src/drafts.rs index 1cb8c6cf4e..1aea4b428f 100644 --- a/backend/windmill-api/src/drafts.rs +++ b/backend/windmill-api/src/drafts.rs @@ -403,30 +403,31 @@ async fn update_draft( // — they keep the write gate. let is_own_discard = req.value.is_none() && !req.legacy; + // Whose row this write is for: the caller's, or the workspace-level one on a legacy + // DELETE (`legacy` is delete-only, so an upsert is the caller's own row either way). + // It picks both the record that applies — an item's move (`email IS NULL`) covers the + // legacy row too, since the same rename carried it — and the draft whose presence + // means this path is still the write's own. + let owner: Option<&str> = (!(req.legacy && req.value.is_none())).then_some(email.as_str()); // The caller's own draft-only move outranks the move of the deployed item. - // `legacy` names the workspace-level row, which no move record covers, and it is - // delete-only: an upsert writes the caller's own row and is routed like any other. - let moved_to = if req.legacy && req.value.is_none() { - None - } else { - sqlx::query_scalar!( - r#"SELECT m.new_path FROM draft_move m - WHERE m.workspace_id = $1 AND m.typ = $2 AND m.old_path = $3 - AND (m.email IS NULL OR m.email = $4) - AND NOT EXISTS ( - SELECT 1 FROM draft d - WHERE d.workspace_id = $1 AND d.typ = $2 AND d.path = $3 AND d.email = $4 - ) - ORDER BY m.email IS NULL - LIMIT 1"#, - &w_id, - kind as UserDraftItemKind, - url_path, - email, - ) - .fetch_optional(&db) - .await? - }; + let moved_to = sqlx::query_scalar!( + r#"SELECT m.new_path FROM draft_move m + WHERE m.workspace_id = $1 AND m.typ = $2 AND m.old_path = $3 + AND (m.email IS NULL OR m.email = $4) + AND NOT EXISTS ( + SELECT 1 FROM draft d + WHERE d.workspace_id = $1 AND d.typ = $2 AND d.path = $3 + AND d.email IS NOT DISTINCT FROM $4 + ) + ORDER BY m.email IS NULL + LIMIT 1"#, + &w_id, + kind as UserDraftItemKind, + url_path, + owner, + ) + .fetch_optional(&db) + .await?; let path: &str = moved_to.as_deref().unwrap_or(url_path); // Everything past here writes, so the gate applies from here on. Answered