fix: ScriptFixerDialog no longer overwrites unrelated scripts (Vibe Kanban) (#2078)
* 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<T>` to `Option<Option<T>>` 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.
This commit is contained in:
committed by
GitHub
parent
75beab35d9
commit
ede1511a50
1
Cargo.lock
generated
1
Cargo.lock
generated
@@ -1372,6 +1372,7 @@ dependencies = [
|
||||
"executors",
|
||||
"serde",
|
||||
"serde_json",
|
||||
"serde_with",
|
||||
"sqlx",
|
||||
"strum",
|
||||
"strum_macros",
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -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"
|
||||
|
||||
|
||||
@@ -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<String>,
|
||||
pub setup_script: Option<String>,
|
||||
pub cleanup_script: Option<String>,
|
||||
pub copy_files: Option<String>,
|
||||
pub parallel_setup_script: Option<bool>,
|
||||
pub dev_server_script: Option<String>,
|
||||
#[serde(
|
||||
default,
|
||||
skip_serializing_if = "Option::is_none",
|
||||
with = "double_option"
|
||||
)]
|
||||
#[ts(optional, type = "string | null")]
|
||||
pub display_name: Option<Option<String>>,
|
||||
|
||||
#[serde(
|
||||
default,
|
||||
skip_serializing_if = "Option::is_none",
|
||||
with = "double_option"
|
||||
)]
|
||||
#[ts(optional, type = "string | null")]
|
||||
pub setup_script: Option<Option<String>>,
|
||||
|
||||
#[serde(
|
||||
default,
|
||||
skip_serializing_if = "Option::is_none",
|
||||
with = "double_option"
|
||||
)]
|
||||
#[ts(optional, type = "string | null")]
|
||||
pub cleanup_script: Option<Option<String>>,
|
||||
|
||||
#[serde(
|
||||
default,
|
||||
skip_serializing_if = "Option::is_none",
|
||||
with = "double_option"
|
||||
)]
|
||||
#[ts(optional, type = "string | null")]
|
||||
pub copy_files: Option<Option<String>>,
|
||||
|
||||
#[serde(
|
||||
default,
|
||||
skip_serializing_if = "Option::is_none",
|
||||
with = "double_option"
|
||||
)]
|
||||
#[ts(optional, type = "boolean | null")]
|
||||
pub parallel_setup_script: Option<Option<bool>>,
|
||||
|
||||
#[serde(
|
||||
default,
|
||||
skip_serializing_if = "Option::is_none",
|
||||
with = "double_option"
|
||||
)]
|
||||
#[ts(optional, type = "string | null")]
|
||||
pub dev_server_script: Option<Option<String>>,
|
||||
}
|
||||
|
||||
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,
|
||||
|
||||
@@ -166,27 +166,16 @@ const ScriptFixerDialogImpl = NiceModal.create<ScriptFixerDialogProps>(
|
||||
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<UpdateRepo> =
|
||||
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<ScriptFixerDialogProps>(
|
||||
} 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<ScriptFixerDialogProps>(
|
||||
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<UpdateRepo> =
|
||||
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<ScriptFixerDialogProps>(
|
||||
} finally {
|
||||
setIsTesting(false);
|
||||
}
|
||||
}, [
|
||||
selectedRepoId,
|
||||
script,
|
||||
scriptType,
|
||||
workspaceId,
|
||||
queryClient,
|
||||
t,
|
||||
repos,
|
||||
]);
|
||||
}, [selectedRepoId, script, scriptType, workspaceId, queryClient, t]);
|
||||
|
||||
const dialogTitle =
|
||||
scriptType === 'setup'
|
||||
|
||||
@@ -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, };
|
||||
|
||||
|
||||
Reference in New Issue
Block a user