fix: a reused destination retires the routes pointing at it, and draft_base stays out of diffs

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ruben Fiszel
2026-09-16 08:41:26 +02:00
co-authored by Claude Opus 5
parent d17f8fc111
commit 63cc0181de
9 changed files with 107 additions and 6 deletions
@@ -1,16 +1,17 @@
{
"db_name": "PostgreSQL",
"query": "DELETE FROM draft_move WHERE workspace_id = $1 AND typ::text = ANY($2::text[]) AND old_path = $3",
"query": "DELETE FROM draft_move WHERE workspace_id = $1 AND typ::text = ANY($2::text[])\n AND (old_path = $3 OR (new_path = $3 AND $4::text IS NULL))",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Text",
"TextArray",
"Text",
"Text"
]
},
"nullable": []
},
"hash": "b6af374e5043a207eae6bce80d3eccda82aec0ae80ccdf2445570e369e0d815a"
"hash": "9723edfd6de38cf0bf21123a46d88fa7d0b650b5cfd80c099f785f5e411062e5"
}
@@ -0,0 +1,18 @@
{
"db_name": "PostgreSQL",
"query": "DELETE FROM draft_move\n WHERE workspace_id = $1 AND typ::text = ANY($2::text[])\n AND new_path = $4 AND old_path <> $3 AND ($5::text IS NULL OR email = $5)",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Text",
"TextArray",
"Text",
"Text",
"Text"
]
},
"nullable": []
},
"hash": "9d1b202844e6935e636d1570a92d46af4419dda9410ffc46f19bc38e56efe609"
}
+55
View File
@@ -530,3 +530,58 @@ async fn test_a_moved_draft_deploys_at_its_new_path(db: Pool<Postgres>) -> anyho
assert_eq!(status, 201, "the moved draft could not be deployed: {body}");
Ok(())
}
/// A route is only as good as the item it points at: when an unrelated item claims the
/// destination, a save still addressed to the old path must stay where it is rather than
/// land on that item's draft.
#[sqlx::test(fixtures("base", "drafts_save_follows_move"))]
async fn test_a_reused_destination_ends_the_route(db: Pool<Postgres>) -> 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?
);
// Someone else's item takes the destination, and the moved draft goes with the
// deploy that consumes it.
sqlx::query("DELETE FROM draft WHERE workspace_id = 'test-workspace' AND path = 'u/test-user/follow_b'")
.execute(&db)
.await?;
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",
"summary": "unrelated",
"description": "",
"content": "export function main() { return 3 }",
"language": "deno",
"schema": {}
}))
.send()
.await?;
assert_eq!(resp.status(), 201, "create failed: {}", resp.text().await?);
assert_eq!(
save_at(port, "u/test-user/follow_a", "after the destination was reused").await?,
"u/test-user/follow_a",
"a save was routed onto the item that now owns the destination"
);
Ok(())
}
+1
View File
@@ -719,6 +719,7 @@ async fn create_flow(
&w_id,
&[UserDraftItemKind::Flow],
&nf.path,
None,
)
.await?;
@@ -2426,6 +2426,7 @@ async fn create_script_internal<'c>(
&w_id,
&[UserDraftItemKind::Script],
&ns.path,
p_path_opt.as_deref(),
)
.await?;
if p_hashes.is_some() && !p_hashes.unwrap().is_empty() {
+1
View File
@@ -2611,6 +2611,7 @@ async fn create_app_internal<'a>(
&w_id,
&[UserDraftItemKind::App, UserDraftItemKind::RawApp],
&app.path,
None,
)
.await?;
let id = sqlx::query_scalar!(
+24 -3
View File
@@ -790,6 +790,21 @@ pub async fn record_draft_move(
)
.execute(&mut **tx)
.await?;
// Routes that ended at the destination before this move describe drafts that were
// carried there for an item this one is replacing: left alive, a save addressed to
// the start of that chain would land on this move's draft instead.
sqlx::query!(
"DELETE FROM draft_move
WHERE workspace_id = $1 AND typ::text = ANY($2::text[])
AND new_path = $4 AND old_path <> $3 AND ($5::text IS NULL OR email = $5)",
w_id,
&typs as &[&str],
old_path,
new_path,
email,
)
.execute(&mut **tx)
.await?;
sqlx::query!(
"UPDATE draft_move SET new_path = $4
WHERE workspace_id = $1 AND typ::text = ANY($2::text[])
@@ -842,8 +857,11 @@ pub async fn record_draft_move(
Ok(())
}
/// Drop the move records leaving `path`: an item was just created there, and saves
/// addressed to it are its own.
/// Drop the move records at `path`: an item was just created there, so saves addressed to
/// it are its own (records leaving `path`). A deploy that is not a rename also drops the
/// records arriving, which point at an item that no longer owns the path; `keep_from`,
/// the path a rename came from, suppresses that, since the chain ending here is the one
/// this very deploy just wrote.
///
/// **The caller must have authorized the deploy that created the item first.**
/// Dropping a record sends later draft writes at `path` back to `path`.
@@ -852,13 +870,16 @@ pub async fn clear_draft_moves_from(
w_id: &str,
kinds: &[UserDraftItemKind],
path: &str,
keep_from: Option<&str>,
) -> Result<()> {
let typs = kinds.iter().map(|k| k.as_str()).collect::<Vec<_>>();
sqlx::query!(
"DELETE FROM draft_move WHERE workspace_id = $1 AND typ::text = ANY($2::text[]) AND old_path = $3",
"DELETE FROM draft_move WHERE workspace_id = $1 AND typ::text = ANY($2::text[])
AND (old_path = $3 OR (new_path = $3 AND $4::text IS NULL))",
w_id,
&typs as &[&str],
path,
keep_from,
)
.execute(&mut **tx)
.await?;
+1
View File
@@ -1442,6 +1442,7 @@ const CLEANED_VALUE_KEYS = new Set([
'draft_only',
'draft_saved_at',
'draft_created_at',
'draft_base',
'is_draft',
'other_drafts_users',
'created_at',
+3 -1
View File
@@ -262,7 +262,7 @@ export async function getDraftDiffValues(
// draft-table row (e.g. a flow created via createFlow(draft_only: true), like
// `u/admin/new`). There `draft` is null, so the draft side must fall back to
// the row's own value — otherwise the diff "after" is empty and nothing shows.
// Strip overlay metadata (is_draft / draft_saved_at / no_deployed /
// Strip overlay metadata (is_draft / draft_saved_at / draft_base / no_deployed /
// other_drafts_users) from the deployed side so the diff doesn't show the
// per-user markers as noise.
if (kind === 'script') {
@@ -271,6 +271,7 @@ export async function getDraftDiffValues(
draft,
is_draft: _i,
draft_saved_at: _c,
draft_base: _b,
no_deployed,
other_drafts_users: _o,
hash: _h,
@@ -289,6 +290,7 @@ export async function getDraftDiffValues(
draft,
is_draft: _i,
draft_saved_at: _c,
draft_base: _b,
no_deployed,
other_drafts_users: _o,
version_id: _v,