From ede1511a502a2022790a99f5eb246b11416448b1 Mon Sep 17 00:00:00 2001 From: Louis Knight-Webb Date: Thu, 15 Jan 2026 15:49:01 +0000 Subject: [PATCH] fix: ScriptFixerDialog no longer overwrites unrelated scripts (Vibe Kanban) (#2078) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * All tasks are complete. Here's a summary of the changes made: ## Summary Fixed the issue where `ScriptFixerDialog.tsx` was replacing cleanup and setup scripts with `null` when editing a dev server script. ### Changes Made: 1. **Added `serde_with` dependency** (`Cargo.toml` and `crates/db/Cargo.toml`) - Added `serde_with = "3"` to enable the double option pattern 2. **Updated `UpdateRepo` struct** (`crates/db/src/models/repo.rs`) - Changed all fields from `Option` to `Option>` with the `double_option` serde helper - Added `#[ts(optional, type = "...")]` annotations for proper TypeScript type generation 3. **Updated `Repo::update` method** (`crates/db/src/models/repo.rs`) - Now properly handles three states: - `None` = field absent → preserve existing value - `Some(None)` = explicitly null → set to NULL - `Some(Some(v))` = has value → update to value 4. **Updated `ScriptFixerDialog.tsx`** (`frontend/src/components/dialogs/scripts/ScriptFixerDialog.tsx`) - Changed `handleSave` and `handleSaveAndTest` to only send the field being edited - Removed `repos` from the dependency arrays since it's no longer used in the save functions 5. **Regenerated TypeScript types** - `UpdateRepo` now has all fields as optional: `field?: type | null` ### Result: The API now supports true partial updates. Clients can send only the fields they want to update, and existing values are preserved for omitted fields. Sending `null` explicitly will clear a field to NULL. * Done. All Rust code has been formatted. --- Cargo.lock | 1 + Cargo.toml | 1 + crates/db/Cargo.toml | 1 + crates/db/src/models/repo.rs | 92 +++++++++++++++---- .../dialogs/scripts/ScriptFixerDialog.tsx | 70 ++++---------- shared/types.ts | 2 +- 6 files changed, 99 insertions(+), 68 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f1e793cb..4155fa9a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1372,6 +1372,7 @@ dependencies = [ "executors", "serde", "serde_json", + "serde_with", "sqlx", "strum", "strum_macros", diff --git a/Cargo.toml b/Cargo.toml index 2aa43f39..aedbbb9b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,6 +27,7 @@ tracing = "0.1.43" tracing-subscriber = { version = "0.3", features = ["env-filter", "fmt", "json"] } ts-rs = { git = "https://github.com/xazukx/ts-rs.git", branch = "use-ts-enum", features = ["uuid-impl", "chrono-impl", "no-serde-warnings", "serde-json-impl"] } schemars = { version = "1.0.4", features = ["derive", "chrono04", "uuid1", "preserve_order"] } +serde_with = "3" async-trait = "0.1" [profile.release] diff --git a/crates/db/Cargo.toml b/crates/db/Cargo.toml index 5a07e113..35573c35 100644 --- a/crates/db/Cargo.toml +++ b/crates/db/Cargo.toml @@ -15,6 +15,7 @@ sqlx = { version = "0.8.6", features = ["runtime-tokio", "tls-rustls-aws-lc-rs", chrono = { version = "0.4", features = ["serde"] } uuid = { version = "1.0", features = ["v4", "serde"] } ts-rs = { workspace = true } +serde_with = { workspace = true } strum = "0.27.2" strum_macros = "0.27.2" diff --git a/crates/db/src/models/repo.rs b/crates/db/src/models/repo.rs index abeb3517..cbb6e033 100644 --- a/crates/db/src/models/repo.rs +++ b/crates/db/src/models/repo.rs @@ -2,6 +2,7 @@ use std::path::{Path, PathBuf}; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; +use serde_with::rust::double_option; use sqlx::{Executor, FromRow, Sqlite, SqlitePool}; use thiserror::Error; use ts_rs::TS; @@ -35,12 +36,53 @@ pub struct Repo { #[derive(Debug, Clone, Deserialize, TS)] #[ts(export)] pub struct UpdateRepo { - pub display_name: Option, - pub setup_script: Option, - pub cleanup_script: Option, - pub copy_files: Option, - pub parallel_setup_script: Option, - pub dev_server_script: Option, + #[serde( + default, + skip_serializing_if = "Option::is_none", + with = "double_option" + )] + #[ts(optional, type = "string | null")] + pub display_name: Option>, + + #[serde( + default, + skip_serializing_if = "Option::is_none", + with = "double_option" + )] + #[ts(optional, type = "string | null")] + pub setup_script: Option>, + + #[serde( + default, + skip_serializing_if = "Option::is_none", + with = "double_option" + )] + #[ts(optional, type = "string | null")] + pub cleanup_script: Option>, + + #[serde( + default, + skip_serializing_if = "Option::is_none", + with = "double_option" + )] + #[ts(optional, type = "string | null")] + pub copy_files: Option>, + + #[serde( + default, + skip_serializing_if = "Option::is_none", + with = "double_option" + )] + #[ts(optional, type = "boolean | null")] + pub parallel_setup_script: Option>, + + #[serde( + default, + skip_serializing_if = "Option::is_none", + with = "double_option" + )] + #[ts(optional, type = "string | null")] + pub dev_server_script: Option>, } impl Repo { @@ -203,17 +245,33 @@ impl Repo { .await? .ok_or(RepoError::NotFound)?; - let display_name = payload - .display_name - .clone() - .unwrap_or(existing.display_name); - let setup_script = payload.setup_script.clone(); - let cleanup_script = payload.cleanup_script.clone(); - let copy_files = payload.copy_files.clone(); - let parallel_setup_script = payload - .parallel_setup_script - .unwrap_or(existing.parallel_setup_script); - let dev_server_script = payload.dev_server_script.clone(); + // None = don't update (use existing) + // Some(None) = set to NULL + // Some(Some(v)) = set to v + let display_name = match &payload.display_name { + None => existing.display_name, + Some(v) => v.clone().unwrap_or_default(), + }; + let setup_script = match &payload.setup_script { + None => existing.setup_script, + Some(v) => v.clone(), + }; + let cleanup_script = match &payload.cleanup_script { + None => existing.cleanup_script, + Some(v) => v.clone(), + }; + let copy_files = match &payload.copy_files { + None => existing.copy_files, + Some(v) => v.clone(), + }; + let parallel_setup_script = match &payload.parallel_setup_script { + None => existing.parallel_setup_script, + Some(v) => v.unwrap_or(false), + }; + let dev_server_script = match &payload.dev_server_script { + None => existing.dev_server_script, + Some(v) => v.clone(), + }; sqlx::query_as!( Repo, diff --git a/frontend/src/components/dialogs/scripts/ScriptFixerDialog.tsx b/frontend/src/components/dialogs/scripts/ScriptFixerDialog.tsx index af3aaeb9..9c13d66e 100644 --- a/frontend/src/components/dialogs/scripts/ScriptFixerDialog.tsx +++ b/frontend/src/components/dialogs/scripts/ScriptFixerDialog.tsx @@ -166,27 +166,16 @@ const ScriptFixerDialogImpl = NiceModal.create( setError(null); try { - // Build update data with all required fields - const selectedRepo = repos.find((r) => r.id === selectedRepoId); - const updateData: UpdateRepo = { - display_name: selectedRepo?.display_name ?? null, - setup_script: - scriptType === 'setup' - ? script.trim() || null - : (selectedRepo?.setup_script ?? null), - cleanup_script: - scriptType === 'cleanup' - ? script.trim() || null - : (selectedRepo?.cleanup_script ?? null), - copy_files: selectedRepo?.copy_files ?? null, - parallel_setup_script: selectedRepo?.parallel_setup_script ?? null, - dev_server_script: - scriptType === 'dev_server' - ? script.trim() || null - : (selectedRepo?.dev_server_script ?? null), - }; + // Only send the field being edited - other fields will be preserved by the backend + const scriptValue = script.trim() || null; + const updateData: Partial = + scriptType === 'setup' + ? { setup_script: scriptValue } + : scriptType === 'cleanup' + ? { cleanup_script: scriptValue } + : { dev_server_script: scriptValue }; - await repoApi.update(selectedRepoId, updateData); + await repoApi.update(selectedRepoId, updateData as UpdateRepo); // Invalidate repos cache queryClient.invalidateQueries({ queryKey: ['repos'] }); @@ -201,7 +190,7 @@ const ScriptFixerDialogImpl = NiceModal.create( } finally { setIsSaving(false); } - }, [selectedRepoId, script, scriptType, queryClient, modal, t, repos]); + }, [selectedRepoId, script, scriptType, queryClient, modal, t]); const handleSaveAndTest = useCallback(async () => { if (!selectedRepoId) return; @@ -210,27 +199,16 @@ const ScriptFixerDialogImpl = NiceModal.create( setError(null); try { - // First save the script - const selectedRepo = repos.find((r) => r.id === selectedRepoId); - const updateData: UpdateRepo = { - display_name: selectedRepo?.display_name ?? null, - setup_script: - scriptType === 'setup' - ? script.trim() || null - : (selectedRepo?.setup_script ?? null), - cleanup_script: - scriptType === 'cleanup' - ? script.trim() || null - : (selectedRepo?.cleanup_script ?? null), - copy_files: selectedRepo?.copy_files ?? null, - parallel_setup_script: selectedRepo?.parallel_setup_script ?? null, - dev_server_script: - scriptType === 'dev_server' - ? script.trim() || null - : (selectedRepo?.dev_server_script ?? null), - }; + // Only send the field being edited - other fields will be preserved by the backend + const scriptValue = script.trim() || null; + const updateData: Partial = + scriptType === 'setup' + ? { setup_script: scriptValue } + : scriptType === 'cleanup' + ? { cleanup_script: scriptValue } + : { dev_server_script: scriptValue }; - await repoApi.update(selectedRepoId, updateData); + await repoApi.update(selectedRepoId, updateData as UpdateRepo); // Invalidate repos cache queryClient.invalidateQueries({ queryKey: ['repos'] }); @@ -256,15 +234,7 @@ const ScriptFixerDialogImpl = NiceModal.create( } finally { setIsTesting(false); } - }, [ - selectedRepoId, - script, - scriptType, - workspaceId, - queryClient, - t, - repos, - ]); + }, [selectedRepoId, script, scriptType, workspaceId, queryClient, t]); const dialogTitle = scriptType === 'setup' diff --git a/shared/types.ts b/shared/types.ts index b0b32c1f..89f9989a 100644 --- a/shared/types.ts +++ b/shared/types.ts @@ -28,7 +28,7 @@ export type SearchMatchType = "FileName" | "DirectoryName" | "FullPath"; export type Repo = { id: string, path: string, name: string, display_name: string, setup_script: string | null, cleanup_script: string | null, copy_files: string | null, parallel_setup_script: boolean, dev_server_script: string | null, created_at: Date, updated_at: Date, }; -export type UpdateRepo = { display_name: string | null, setup_script: string | null, cleanup_script: string | null, copy_files: string | null, parallel_setup_script: boolean | null, dev_server_script: string | null, }; +export type UpdateRepo = { display_name?: string | null, setup_script?: string | null, cleanup_script?: string | null, copy_files?: string | null, parallel_setup_script?: boolean | null, dev_server_script?: string | null, }; export type ProjectRepo = { id: string, project_id: string, repo_id: string, };