From a20d0370207b0369325de653fea86e2a852afd9a Mon Sep 17 00:00:00 2001 From: Guilhem Lemouel Date: Wed, 26 Aug 2026 17:57:25 +0200 Subject: [PATCH] fix: address review findings on label colors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Key label rows by the verbatim name: item-side `labels text[]` stores whatever was typed, so trimming the key colored a different label — `" release "` wrote a row for `"release"` and the vocabulary then listed both. The trim now only answers whether the name is blank. Cache labels per workspace with a generation guard, so a response that lost the race no longer replaces a newer workspace's entry, and thread the operating workspace through `LabelsInput` instead of reading `$workspaceStore`: a session editor works on a fork while the sidebar still browses the origin, and a color must be written where the item lives. Also: carry `label_settings` into a forked workspace, bound the name length server-side, refetch the vocabulary on every suggestion-list open, and drop the banned `$bindable(false)` on an optional prop. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QNUCuRqmzNukjR7FDewN7W --- ...68a76a3cc6d19974be318944953c54ca86f3d.json | 15 +++++ ...a6ae28f1b817b2e1ff275a3f470bb88fc500c.json | 22 ------- backend/summarized_schema.txt | 2 + .../windmill-api-workspaces/src/workspaces.rs | 23 ++++++++ backend/windmill-api/src/labels.rs | 27 +++++++-- .../src/lib/components/AppConnectInner.svelte | 6 +- .../src/lib/components/FolderEditor.svelte | 2 +- .../src/lib/components/LabelsInput.svelte | 30 ++++++---- .../src/lib/components/ResourceForm.svelte | 2 +- .../src/lib/components/ScriptBuilder.svelte | 2 +- .../lib/components/SummaryPathDisplay.svelte | 16 ++++- .../src/lib/components/VariableForm.svelte | 2 +- .../apps/editor/AppEditorHeaderDeploy.svelte | 2 +- .../common/colorPicker/ColorSwatchGrid.svelte | 19 ++++-- .../flows/content/FlowSettings.svelte | 6 +- .../lib/components/labels/LabelBadge.svelte | 4 +- .../components/labels/LabelColorPicker.svelte | 5 +- .../labels/LabelManagerDrawer.svelte | 3 +- .../src/lib/components/labels/labelStore.ts | 58 +++++++++++++------ .../src/lib/components/runs/RunLabels.svelte | 4 +- .../schedules/ScheduleEditorInner.svelte | 2 +- 21 files changed, 168 insertions(+), 84 deletions(-) create mode 100644 backend/.sqlx/query-0d09c68d21552ca98d31e5c1dc468a76a3cc6d19974be318944953c54ca86f3d.json delete mode 100644 backend/.sqlx/query-7fc4dcc5f46da2a1ac74bf80237a6ae28f1b817b2e1ff275a3f470bb88fc500c.json diff --git a/backend/.sqlx/query-0d09c68d21552ca98d31e5c1dc468a76a3cc6d19974be318944953c54ca86f3d.json b/backend/.sqlx/query-0d09c68d21552ca98d31e5c1dc468a76a3cc6d19974be318944953c54ca86f3d.json new file mode 100644 index 0000000000..26ab443652 --- /dev/null +++ b/backend/.sqlx/query-0d09c68d21552ca98d31e5c1dc468a76a3cc6d19974be318944953c54ca86f3d.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO label_settings (workspace_id, name, color)\n SELECT $2, name, color\n FROM label_settings\n WHERE workspace_id = $1", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Varchar" + ] + }, + "nullable": [] + }, + "hash": "0d09c68d21552ca98d31e5c1dc468a76a3cc6d19974be318944953c54ca86f3d" +} diff --git a/backend/.sqlx/query-7fc4dcc5f46da2a1ac74bf80237a6ae28f1b817b2e1ff275a3f470bb88fc500c.json b/backend/.sqlx/query-7fc4dcc5f46da2a1ac74bf80237a6ae28f1b817b2e1ff275a3f470bb88fc500c.json deleted file mode 100644 index fe7587f243..0000000000 --- a/backend/.sqlx/query-7fc4dcc5f46da2a1ac74bf80237a6ae28f1b817b2e1ff275a3f470bb88fc500c.json +++ /dev/null @@ -1,22 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT DISTINCT unnest(labels) as \"label!\" FROM (\n SELECT labels FROM script WHERE workspace_id = $1 AND labels IS NOT NULL\n UNION ALL SELECT labels FROM flow WHERE workspace_id = $1 AND labels IS NOT NULL\n UNION ALL SELECT labels FROM resource WHERE workspace_id = $1 AND labels IS NOT NULL\n UNION ALL SELECT labels FROM variable WHERE workspace_id = $1 AND labels IS NOT NULL\n UNION ALL SELECT labels FROM schedule WHERE workspace_id = $1 AND labels IS NOT NULL\n UNION ALL SELECT labels FROM app WHERE workspace_id = $1 AND labels IS NOT NULL\n UNION ALL SELECT labels FROM folder WHERE workspace_id = $1 AND labels IS NOT NULL\n ) t ORDER BY 1", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "label!", - "type_info": "Text" - } - ], - "parameters": { - "Left": [ - "Text" - ] - }, - "nullable": [ - null - ] - }, - "hash": "7fc4dcc5f46da2a1ac74bf80237a6ae28f1b817b2e1ff275a3f470bb88fc500c" -} diff --git a/backend/summarized_schema.txt b/backend/summarized_schema.txt index a9865cebd3..9b1297e4a0 100644 --- a/backend/summarized_schema.txt +++ b/backend/summarized_schema.txt @@ -128,6 +128,8 @@ job_stats: workspace_id(char), job_id(uuid), metric_id(char), metric_name(char), kafka_pending_commits: id(bigint), workspace_id(char), kafka_trigger_path(char), topic(char), partition(int), offset(bigint), created_at(ts) FK: (workspace_id, kafka_trigger_path) -> kafka_trigger(workspace_id, path) kafka_trigger: path(char), kafka_resource_path(char), topics(char), group_id(char), script_path(char), is_flow(bool), workspace_id(char), edited_by(char), email(char), edited_at(ts), extra_perms(jsonb), server_id(char), last_server_ping(ts), error(text), error_handler_path(char), error_handler_args(jsonb), retry(jsonb), mode(trigger_mode), filters(jsonb[]), auto_commit(bool), labels(text[]) +label_settings: workspace_id(char), name(text), color(char) + FK: (workspace_id) -> workspace(id) log_file: hostname(char), log_ts(ts), ok_lines(bigint), err_lines(bigint), mode(log_mode), worker_group(char), file_path(char), json_fmt(bool) macro_definition: workspace_id(char), name(char), provider_path(char), params(text), body(text), is_table_macro(bool), created_at(ts) FK: (workspace_id) -> workspace(id) diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index e04e1c1ab9..0741f20444 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -5456,6 +5456,10 @@ async fn clone_workspace_data( // Clone folders clone_folders(tx, source_workspace_id, target_workspace_id).await?; + // Clone label colours. Folders and items carry their `labels` arrays above; + // without this the fork keeps every label and loses every colour. + clone_label_settings(tx, source_workspace_id, target_workspace_id).await?; + // Clone groups clone_groups(tx, source_workspace_id, target_workspace_id).await?; @@ -5917,6 +5921,25 @@ async fn clone_folders( Ok(()) } +async fn clone_label_settings( + tx: &mut Transaction<'_, Postgres>, + source_workspace_id: &str, + target_workspace_id: &str, +) -> Result<()> { + sqlx::query!( + "INSERT INTO label_settings (workspace_id, name, color) + SELECT $2, name, color + FROM label_settings + WHERE workspace_id = $1", + source_workspace_id, + target_workspace_id, + ) + .execute(&mut **tx) + .await?; + + Ok(()) +} + async fn clone_groups( tx: &mut Transaction<'_, Postgres>, source_workspace_id: &str, diff --git a/backend/windmill-api/src/labels.rs b/backend/windmill-api/src/labels.rs index 05e3747d89..a1799d62f0 100644 --- a/backend/windmill-api/src/labels.rs +++ b/backend/windmill-api/src/labels.rs @@ -19,13 +19,20 @@ use windmill_common::{ error::{Error, JsonResult, Result}, }; -/// Mirrors `NoteColor` in `frontend/src/lib/components/graph/labelColors.ts`. -/// Kept as a list rather than an enum so an unknown value coming back from an -/// older row is a rejected write, not a failed deserialization of the whole row. +/// Mirrors the `LabelColor` enum in `openapi.yaml`, from which +/// `frontend/src/lib/components/labels/labelColors.ts` is generated. Kept as a +/// list rather than an enum so an unknown value coming back from an older row is +/// a rejected write, not a failed deserialization of the whole row. const PALETTE: [&str; 10] = [ "yellow", "blue", "green", "purple", "pink", "orange", "red", "cyan", "lime", "gray", ]; +/// The column is TEXT so the registry never caps what a label may be called, but +/// a write still has to be bounded: the handler runs on the privileged pool, and +/// every row it accepts is returned to every member by `labels/list`. Well above +/// the 50 characters the label input allows. +const MAX_NAME_LEN: usize = 255; + pub fn workspaced_service() -> Router { Router::new() .route("/list", get(list_workspace_labels)) @@ -97,10 +104,20 @@ async fn update_label( } check_scopes(&authed, || "labels:write".to_string())?; - let name = nl.name.trim(); - if name.is_empty() { + // The name keys a label that already exists verbatim in some `labels text[]`, + // which stores whatever was typed. Trimming here would colour a *different* + // label — `" release "` would write a row for `"release"` — so the raw name is + // the key and the trim only answers whether it is blank. + let name = nl.name.as_str(); + if name.trim().is_empty() { return Err(Error::BadRequest("Label name cannot be empty".to_string())); } + if name.chars().count() > MAX_NAME_LEN { + return Err(Error::BadRequest(format!( + "Label name cannot exceed {} characters", + MAX_NAME_LEN + ))); + } match nl.color.as_deref() { Some(color) => { diff --git a/frontend/src/lib/components/AppConnectInner.svelte b/frontend/src/lib/components/AppConnectInner.svelte index b1adf4ab6a..c3ab69797b 100644 --- a/frontend/src/lib/components/AppConnectInner.svelte +++ b/frontend/src/lib/components/AppConnectInner.svelte @@ -1255,7 +1255,7 @@ kind="resource" /> - + {#if deployTo}