fix: a legacy discard follows the item's move record too

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ruben Fiszel
2026-09-15 00:34:58 +02:00
co-authored by Claude Opus 5
parent 7a345b6c7e
commit d5d04c2fb1
3 changed files with 64 additions and 25 deletions
@@ -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"
}
+38
View File
@@ -369,3 +369,41 @@ async fn test_a_poisoned_draft_follows_a_rename(db: Pool<Postgres>) -> 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<Postgres>) -> 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(())
}
+24 -23
View File
@@ -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