diff --git a/backend/.sqlx/query-b465d4abec4411a59c693efbe569e8c1508b37ca3ee1eef39fb1896c19c61abc.json b/backend/.sqlx/query-b465d4abec4411a59c693efbe569e8c1508b37ca3ee1eef39fb1896c19c61abc.json new file mode 100644 index 0000000000..b6c6a9b92e --- /dev/null +++ b/backend/.sqlx/query-b465d4abec4411a59c693efbe569e8c1508b37ca3ee1eef39fb1896c19c61abc.json @@ -0,0 +1,17 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE draft\n SET value = to_json(\n jsonb_set(to_jsonb(value), ARRAY[$1::text], $2::text::jsonb, false)\n )\n WHERE id = ANY($3) AND email = $4", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text", + "Int8Array", + "Text" + ] + }, + "nullable": [] + }, + "hash": "b465d4abec4411a59c693efbe569e8c1508b37ca3ee1eef39fb1896c19c61abc" +} diff --git a/backend/windmill-api-flows/src/flows.rs b/backend/windmill-api-flows/src/flows.rs index 2fdc56fe31..ba3c0c7349 100644 --- a/backend/windmill-api-flows/src/flows.rs +++ b/backend/windmill-api-flows/src/flows.rs @@ -1385,8 +1385,10 @@ async fn update_flow( if is_new_path { // Everything left at the old path is a draft this deploy didn't consume // — teammates' rows, and the deployer's own when the caller asked us to - // keep it (a move re-deploys the DEPLOYED content, not the draft). - // Carry them rather than strand them. + // keep it. Carry them rather than strand them. Only the deployer's own + // row is restamped: this runs on any path-changing deploy, content edits + // included, and a teammate whose draft claimed the new head would lose + // their stale-draft warning. let outcome = windmill_common::user_drafts::move_drafts_for_path( &mut tx, &w_id, @@ -1397,6 +1399,7 @@ async fn update_flow( UserDraftItemKind::Flow .base_version_field() .map(|f| (f, version.to_string())), + &authed.email, ) .await?; if outcome.left_behind > 0 { diff --git a/backend/windmill-api-scripts/src/scripts.rs b/backend/windmill-api-scripts/src/scripts.rs index 557dd432b5..b84bff672c 100644 --- a/backend/windmill-api-scripts/src/scripts.rs +++ b/backend/windmill-api-scripts/src/scripts.rs @@ -2137,8 +2137,10 @@ async fn create_script_internal<'c>( if p_path != &ns.path { // Everything left at the old path is a draft this deploy didn't // consume — teammates' rows, and the deployer's own when the caller - // asked us to keep it (a move re-deploys the DEPLOYED content, not - // the draft). Carry them rather than strand them. + // asked us to keep it. Carry them rather than strand them. Only the + // deployer's own row is restamped: this runs on any path-changing + // deploy, content edits included, and a teammate whose draft claimed + // the new head would lose their stale-draft warning. let outcome = windmill_common::user_drafts::move_drafts_for_path( &mut tx, &w_id, @@ -2150,6 +2152,7 @@ async fn create_script_internal<'c>( UserDraftItemKind::Script .base_version_field() .map(|f| (f, format!("\"{}\"", hash))), + &authed.email, ) .await?; if outcome.left_behind > 0 { diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index 91f479a4d7..e890ee1c2d 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -3450,8 +3450,10 @@ async fn update_app_internal<'a>( if npath != path { // Everything left at the old path is a draft this deploy didn't consume // — teammates' rows, and the deployer's own when the caller asked us to - // keep it (a move re-deploys the DEPLOYED content, not the draft). - // Carry them rather than strand them. + // keep it. Carry them rather than strand them. Only the deployer's own + // row is restamped: this runs on any path-changing deploy, content edits + // included, and a teammate whose draft claimed the new head would lose + // their stale-draft warning. let outcome = windmill_common::user_drafts::move_drafts_for_path( &mut tx, &w_id, @@ -3462,6 +3464,7 @@ async fn update_app_internal<'a>( UserDraftItemKind::App .base_version_field() .map(|f| (f, v_id.to_string())), + &authed.email, ) .await?; if outcome.left_behind > 0 { diff --git a/backend/windmill-api/src/drafts.rs b/backend/windmill-api/src/drafts.rs index a6030c1f3d..073c49442c 100644 --- a/backend/windmill-api/src/drafts.rs +++ b/backend/windmill-api/src/drafts.rs @@ -355,6 +355,24 @@ struct DraftBaseVersion { /// Apps / raw apps: `app_version.id`. #[serde(default)] parent_version: Option, + /// The user-typed target path — `UserDraftItemKind::typed_path_field`. Both + /// spellings live here so the one cheap parse answers the version question + /// and the staged-rename question together; serde skips every other key, and + /// an app draft's payload runs to hundreds of KB. + #[serde(default)] + path: Option, + #[serde(default)] + draft_path: Option, +} + +impl DraftBaseVersion { + /// The typed path for this kind, mirroring `typed_path_field`. + fn typed_path(&self, kind: UserDraftItemKind) -> Option<&str> { + match kind { + UserDraftItemKind::Script => self.path.as_deref(), + _ => self.draft_path.as_deref(), + } + } } /// Where the item that used to live at `path` went, for a draft still bound to @@ -510,14 +528,7 @@ async fn resolve_moved_to( // Present and naming the old path ⇒ repoint. Absent ⇒ omit, so the patch // never manufactures a target the draft did not have. Naming anywhere else // ⇒ omit, so a staged rename is preserved. - let repoint_path = serde_json::from_str::(value) - .ok() - .and_then(|v| { - v.get(kind.typed_path_field()) - .and_then(|p| p.as_str()) - .map(|p| p == path) - }) - .unwrap_or(false); + let repoint_path = base.typed_path(kind) == Some(path); Ok(moved.map(|(new_path, new_by, head)| { let mut patch = serde_json::Map::new(); if repoint_path { diff --git a/backend/windmill-common/src/user_drafts.rs b/backend/windmill-common/src/user_drafts.rs index 0e0f2f61ac..86d2da7f5b 100644 --- a/backend/windmill-common/src/user_drafts.rs +++ b/backend/windmill-common/src/user_drafts.rs @@ -576,10 +576,10 @@ pub async fn delete_own_draft_for_path( /// filter: teammates' rows and the legacy NULL-email row follow too. /// /// `typed_path_field` is the draft JSON key holding the user-typed target path -/// (`path` for scripts, `draft_path` for flows/apps). It is rewritten whenever -/// present — a target staged against the old location would otherwise un-move -/// the item the next time that draft is deployed. Absent means "same as the -/// row's path", which the move already fixed. +/// (`path` for scripts, `draft_path` for flows/apps). It is re-pointed only when +/// it still names `old_path`: absent means "wherever my row sits", which `SET +/// path` already fixed, and anything else is a rename the user staged in their +/// own editor, which deploying their draft should still honour. /// /// `base_version` restamps the version the draft forked from so the carried /// draft doesn't read as stale against the version the move just created. The @@ -588,6 +588,14 @@ pub async fn delete_own_draft_for_path( /// `parent_version` are numbers. Only restamps rows that already carry the /// field. /// +/// The restamp is confined to `restamp_email`'s own row, and that is a safety +/// property, not an optimisation. This function runs on ANY deploy that changed +/// the path, including one that renamed and edited in the same breath. Saying +/// "you are up to date with the new head" to a teammate's draft would delete the +/// stale-draft warning they need, and they would then deploy straight over the +/// edit. Only the deployer knows their own draft is not behind the version they +/// just pushed. +/// /// A row whose owner already has a draft at `new_path` stays put: the target /// draft is work in its own right and is never overwritten. Those rows are /// reported as `left_behind` rather than swallowed — they are exactly the @@ -601,6 +609,7 @@ pub async fn move_drafts_for_path( new_path: &str, typed_path_field: &str, base_version: Option<(&str, String)>, + restamp_email: &str, ) -> Result { let typs = kinds.iter().map(|k| k.as_str()).collect::>(); // The typed path is only followed along when it still names the OLD path. @@ -646,10 +655,11 @@ pub async fn move_drafts_for_path( SET value = to_json( jsonb_set(to_jsonb(value), ARRAY[$1::text], $2::text::jsonb, false) ) - WHERE id = ANY($3)"#, + WHERE id = ANY($3) AND email = $4"#, field, version, &moved, + restamp_email, ) .execute(&mut **tx) .await?; diff --git a/frontend/src/lib/components/home/Item.svelte b/frontend/src/lib/components/home/Item.svelte index f9e30eaf65..aa1d948999 100644 --- a/frontend/src/lib/components/home/Item.svelte +++ b/frontend/src/lib/components/home/Item.svelte @@ -56,8 +56,12 @@ homeSelection.register(b) return () => homeSelection.unregister(b.key) }) + // Gated on `available`, not just on having an item: it is what excludes + // operators and the read-only embedded lists. Without it they would get a + // checkbox and a Select entry that tick rows while `active` stays false + // forever, so the bar that clears the selection never appears. let rowSelection: RowSelection | undefined = $derived( - bulkItem && homeSelection + bulkItem && homeSelection?.available ? { key: bulkItem.key, selected: homeSelection.has(bulkItem.key),