From da9e416b8ed41a0cbb219ad32b7456dd6997d09d Mon Sep 17 00:00:00 2001 From: Diego Imbert <70353967+diegoimbert@users.noreply.github.com> Date: Mon, 11 May 2026 09:56:52 +0200 Subject: [PATCH] workspace specific nit fixes (#9072) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: capture linked variables in trash on bulk resource delete delete_resources_bulk grew linked-variable cascade deletion in an earlier commit on this branch but only mirrored the deletion side of delete_resource — not the trashbin capture side. Linked variables deleted via bulk were permanently lost while their single-delete counterparts could be recovered from trash. Fetch each resource's linked variable rows as JSON before bulk delete and stash them under `trash_data['linked_variables']` of that resource's trash entry, matching the shape produced by single-resource delete. * fix: ws_specific cleanup gaps in variable rename + bulk delete; tooltip Four spots: 1. update_variable rename block: when a variable is renamed and a linked resource at the same path is renamed alongside, also move any explicit ws_specific 'resource' marker from the old path to the new one. Symmetric with what update_resource already does for ws_specific 'variable'. 2. delete_variables_bulk: clean ws_specific 'resource' rows for any linked resource paths before the resource DELETE. Without this, bulk-delete leaves orphaned markers that would cause a freshly recreated resource at the same path to be falsely treated as workspace-specific. (linked_resource trash capture is already present in the bulk path — the reviewer note about that was inaccurate against the current code.) 3. list_ws_specific: ORDER BY item_kind, path so the CLI sees a stable list across pulls/pushes — cheap on a small per-workspace row set and avoids spurious diffs. 4. VariableForm tooltip: mirror the resource form so users who find a variable already toggled know it may have been auto-marked by a workspace-specific resource referencing it, and that disabling doesn't retroactively un-mark the referencing resource. * sqlx prepare --- ...1f7f387f5055c47f493271d26731336257384.json | 10 +++--- ...631af9f524389309f17ef04f70c773b1d5e75.json | 28 +++++++++++++++ ...153c43903f929ae5d62fbba12610f89c36d55.json | 2 +- .../windmill-api-workspaces/src/workspaces.rs | 1 + backend/windmill-store/src/resources.rs | 35 +++++++++++++++++-- backend/windmill-store/src/variables.rs | 25 +++++++++++++ .../src/lib/components/VariableForm.svelte | 2 +- 7 files changed, 94 insertions(+), 9 deletions(-) create mode 100644 backend/.sqlx/query-290599fc173947acb518344d6fb631af9f524389309f17ef04f70c773b1d5e75.json diff --git a/backend/.sqlx/query-07168aaf14cb6beff0ad4274b441f7f387f5055c47f493271d26731336257384.json b/backend/.sqlx/query-07168aaf14cb6beff0ad4274b441f7f387f5055c47f493271d26731336257384.json index e7ed0aee65..d29a18c691 100644 --- a/backend/.sqlx/query-07168aaf14cb6beff0ad4274b441f7f387f5055c47f493271d26731336257384.json +++ b/backend/.sqlx/query-07168aaf14cb6beff0ad4274b441f7f387f5055c47f493271d26731336257384.json @@ -46,11 +46,11 @@ ] }, "nullable": [ - false, - false, - false, - false, - false, + true, + true, + true, + true, + true, true, true ] diff --git a/backend/.sqlx/query-290599fc173947acb518344d6fb631af9f524389309f17ef04f70c773b1d5e75.json b/backend/.sqlx/query-290599fc173947acb518344d6fb631af9f524389309f17ef04f70c773b1d5e75.json new file mode 100644 index 0000000000..ad296e9eda --- /dev/null +++ b/backend/.sqlx/query-290599fc173947acb518344d6fb631af9f524389309f17ef04f70c773b1d5e75.json @@ -0,0 +1,28 @@ +{ + "db_name": "PostgreSQL", + "query": "\n SELECT s.item_kind, s.path\n FROM ws_specific s\n WHERE s.workspace_id = $1\n AND (\n (s.item_kind = 'resource' AND EXISTS (\n SELECT 1 FROM resource r\n WHERE r.workspace_id = s.workspace_id AND r.path = s.path\n ))\n OR (s.item_kind = 'variable' AND EXISTS (\n SELECT 1 FROM variable v\n WHERE v.workspace_id = s.workspace_id AND v.path = s.path\n ))\n )\n ORDER BY s.item_kind, s.path\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "item_kind", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "path", + "type_info": "Varchar" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + false, + false + ] + }, + "hash": "290599fc173947acb518344d6fb631af9f524389309f17ef04f70c773b1d5e75" +} diff --git a/backend/.sqlx/query-5a219a2532517869578c4504ff3153c43903f929ae5d62fbba12610f89c36d55.json b/backend/.sqlx/query-5a219a2532517869578c4504ff3153c43903f929ae5d62fbba12610f89c36d55.json index 36ddb8ab9f..713ccb9dd3 100644 --- a/backend/.sqlx/query-5a219a2532517869578c4504ff3153c43903f929ae5d62fbba12610f89c36d55.json +++ b/backend/.sqlx/query-5a219a2532517869578c4504ff3153c43903f929ae5d62fbba12610f89c36d55.json @@ -15,7 +15,7 @@ ] }, "nullable": [ - true + null ] }, "hash": "5a219a2532517869578c4504ff3153c43903f929ae5d62fbba12610f89c36d55" diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index f479b8c719..94e2026ad3 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -7351,6 +7351,7 @@ async fn list_ws_specific( WHERE v.workspace_id = s.workspace_id AND v.path = s.path )) ) + ORDER BY s.item_kind, s.path "#, &w_id ) diff --git a/backend/windmill-store/src/resources.rs b/backend/windmill-store/src/resources.rs index 6b169a0c0d..8aa88bad86 100644 --- a/backend/windmill-store/src/resources.rs +++ b/backend/windmill-store/src/resources.rs @@ -1238,10 +1238,39 @@ async fn delete_resources_bulk( .await?; if let Some(res_data) = trash_resource { + // Per-resource linked vars so each resource's trash entry carries + // exactly the variables that vanished with it (matching the + // single-delete shape: trash_data["linked_variables"]). + let mut this_linked: Vec = Vec::new(); if let Some(value) = res_data.get("value") { - collect_var_refs(value, &mut linked_var_paths); + collect_var_refs(value, &mut this_linked); + } + this_linked.sort(); + this_linked.dedup(); + + let trash_linked_vars: Vec = if this_linked.is_empty() { + Vec::new() + } else { + let placeholders: Vec = this_linked + .iter() + .enumerate() + .map(|(i, _)| format!("${}", i + 2)) + .collect(); + let query = format!( + "SELECT to_jsonb(t) FROM variable t WHERE workspace_id = $1 AND path IN ({})", + placeholders.join(", ") + ); + let mut q = sqlx::query_scalar::<_, serde_json::Value>(&query).bind(&w_id); + for var_path in &this_linked { + q = q.bind(var_path); + } + q.fetch_all(&mut *tx).await? + }; + + let mut trash_data = serde_json::json!({"row": res_data}); + if !trash_linked_vars.is_empty() { + trash_data["linked_variables"] = serde_json::Value::Array(trash_linked_vars); } - let trash_data = serde_json::json!({"row": res_data}); windmill_common::trashbin::move_to_trash( &mut *tx, &w_id, @@ -1251,6 +1280,8 @@ async fn delete_resources_bulk( &authed.username, ) .await?; + + linked_var_paths.extend(this_linked); } } linked_var_paths.sort(); diff --git a/backend/windmill-store/src/variables.rs b/backend/windmill-store/src/variables.rs index a9f8734647..c50049fcd6 100644 --- a/backend/windmill-store/src/variables.rs +++ b/backend/windmill-store/src/variables.rs @@ -755,6 +755,17 @@ async fn delete_variables_bulk( ) .fetch_all(&mut *tx) .await?; + // Mirror single delete_variable: clean the linked-resource ws_specific + // markers BEFORE deleting the resource rows so they don't survive as + // orphans. A resource later created at the same path would otherwise + // inherit a stale ws_specific flag. + sqlx::query!( + "DELETE FROM ws_specific WHERE workspace_id = $1 AND item_kind = 'resource' AND path = ANY($2)", + w_id, + &deleted_paths + ) + .execute(&mut *tx) + .await?; sqlx::query!( "DELETE FROM resource WHERE path = ANY($1) AND workspace_id = $2", &deleted_paths, @@ -1019,6 +1030,20 @@ async fn update_variable( ) .execute(&mut *tx) .await?; + + // The linked resource at the same path is renamed above; move + // its ws_specific 'resource' marker too so an explicitly-flagged + // resource doesn't lose its ws_specific status on rename and + // doesn't leave a stale marker at the old path. Symmetric with + // update_resource's rename block. + sqlx::query!( + "UPDATE ws_specific SET path = $1 WHERE workspace_id = $2 AND item_kind = 'resource' AND path = $3", + npath, + w_id, + path + ) + .execute(&mut *tx) + .await?; } } diff --git a/frontend/src/lib/components/VariableForm.svelte b/frontend/src/lib/components/VariableForm.svelte index 167172450e..0ffa4d8ba6 100644 --- a/frontend/src/lib/components/VariableForm.svelte +++ b/frontend/src/lib/components/VariableForm.svelte @@ -86,7 +86,7 @@ {#if deployTo}