From 6796569466a2fade3aea0e3c3c5fde0f50c336da Mon Sep 17 00:00:00 2001 From: Gregory Schier Date: Wed, 2 Sep 2026 11:24:25 -0700 Subject: [PATCH] feat(import): say which fields differ, and stop offering to apply nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A row marked "edited" or "conflict" gave no way to tell what actually differs, so a resource that drifted from the file — however it drifted — reads as an unexplained accusation. The plan now carries the field names the source and the local copy disagree on, and the row's tooltip lists them. The preview's primary button also read "Apply" when the current selection would change nothing. It now reads "Done", since committing an empty selection only records the decisions made in the preview. Co-Authored-By: Claude Opus 5 --- .../components/ImportDataDialog.tsx | 17 ++++++-- .../yaak-rpc-schema/bindings/gen_util.ts | 4 ++ crates/yaak-models/bindings/gen_util.ts | 4 ++ crates/yaak-models/src/util.rs | 3 ++ crates/yaak/src/import.rs | 39 ++++++++++++++++--- 5 files changed, 57 insertions(+), 10 deletions(-) diff --git a/apps/yaak-client/components/ImportDataDialog.tsx b/apps/yaak-client/components/ImportDataDialog.tsx index ec7e5805..47c16c7e 100644 --- a/apps/yaak-client/components/ImportDataDialog.tsx +++ b/apps/yaak-client/components/ImportDataDialog.tsx @@ -345,6 +345,7 @@ function LoadedImportDataDialog({ modelId: existing?.id ?? planned?.id ?? "workspace", name: existing?.name ?? planned?.name ?? "New workspace", selected: true, + changedFields: [], }, children: itemTree, }; @@ -408,7 +409,7 @@ function LoadedImportDataDialog({ ? "Importing" : changeCount > 0 ? `Apply ${changeCount} ${changeCount === 1 ? "Change" : "Changes"}` - : "Apply"} + : "Done"} @@ -626,19 +627,23 @@ function actionLabel(item: ImportPlanItem): string | null { } function actionHelp(item: ImportPlanItem): string | null { + const help = (text: string) => + item.changedFields.length > 0 + ? `${text} · ${item.changedFields.map(fieldLabel).join(", ")}` + : text; switch (item.action) { case "create": return "Added since the last import"; case "update": - return "Changed since the last import"; + return help("Changed since the last import"); case "delete": return item.reason === "moved_into_not_imported_folder" ? "Moved into a folder that isn't imported. Import that folder instead to follow the move" : "Deleted since the last import"; case "keep_local": - return "Local edits made since the last import. Importing will revert them if checked"; + return help("Local edits made since the last import. Importing will revert them if checked"); case "conflict": - return "Changed both here and in the file since the last import"; + return help("Changed both here and in the file since the last import"); case "not_imported": return "In the file, but not imported. Check it to import it"; default: @@ -646,6 +651,10 @@ function actionHelp(item: ImportPlanItem): string | null { } } +function fieldLabel(field: string): string { + return field.replace(/([A-Z])/g, " $1").toLowerCase(); +} + /** Every plan item above `item`, nearest first. */ function ancestorsOf(item: ImportPlanItem, byId: Map): ImportPlanItem[] { const ancestors: ImportPlanItem[] = []; diff --git a/crates/common/yaak-rpc-schema/bindings/gen_util.ts b/crates/common/yaak-rpc-schema/bindings/gen_util.ts index f599771f..7fd672a5 100644 --- a/crates/common/yaak-rpc-schema/bindings/gen_util.ts +++ b/crates/common/yaak-rpc-schema/bindings/gen_util.ts @@ -77,6 +77,10 @@ export type ImportPlanItem = { selected: boolean; resolution?: ImportConflictResolution; reason?: ImportPlanReason; + /** + * Fields where the source and the local copy disagree, so the preview can say why + */ + changedFields: Array; }; /** diff --git a/crates/yaak-models/bindings/gen_util.ts b/crates/yaak-models/bindings/gen_util.ts index f599771f..7fd672a5 100644 --- a/crates/yaak-models/bindings/gen_util.ts +++ b/crates/yaak-models/bindings/gen_util.ts @@ -77,6 +77,10 @@ export type ImportPlanItem = { selected: boolean; resolution?: ImportConflictResolution; reason?: ImportPlanReason; + /** + * Fields where the source and the local copy disagree, so the preview can say why + */ + changedFields: Array; }; /** diff --git a/crates/yaak-models/src/util.rs b/crates/yaak-models/src/util.rs index f44d09dd..7e1c5d67 100644 --- a/crates/yaak-models/src/util.rs +++ b/crates/yaak-models/src/util.rs @@ -205,6 +205,9 @@ pub struct ImportPlanItem { pub resolution: Option, #[ts(optional)] pub reason: Option, + /// Fields where the source and the local copy disagree, so the preview can say why + #[serde(default)] + pub changed_fields: Vec, } #[derive(Debug, Deserialize, Serialize, TS)] diff --git a/crates/yaak/src/import.rs b/crates/yaak/src/import.rs index 6e1056a5..415f4d65 100644 --- a/crates/yaak/src/import.rs +++ b/crates/yaak/src/import.rs @@ -842,6 +842,7 @@ fn merge_with_linked_source( selected, resolution, reason, + changed_fields: Vec::new(), }; // Nothing can be created inside a folder that isn't imported, so it starts unchecked @@ -870,13 +871,15 @@ fn merge_with_linked_source( return Ok(()); } - let incoming = serde_json::to_value(&any)?; - let current = current_models.get(&planned_id).cloned().unwrap_or_default(); + let incoming = comparable(serde_json::to_value(&any)?); + let current = comparable(current_models.get(&planned_id).cloned().unwrap_or_default()); let (source_changed, local_changed) = match recorded_hash(row) { - Some(hash) => (content_hash(incoming) != hash, content_hash(current) != hash), + Some(hash) => { + (hash_comparable(&incoming) != hash, hash_comparable(¤t) != hash) + } // Without a recorded version, all that can be told is whether the two sides differ None => { - let differs = comparable(incoming) != comparable(current); + let differs = incoming != current; (differs, differs) } }; @@ -891,7 +894,9 @@ fn merge_with_linked_source( Some(ImportConflictResolution::KeepMine), ), }; - items.push(item(action, selected, resolution, None)); + let mut planned = item(action, selected, resolution, None); + planned.changed_fields = changed_fields(&incoming, ¤t); + items.push(planned); Ok(()) }; @@ -946,6 +951,7 @@ fn merge_with_linked_source( selected: false, resolution: None, reason: None, + changed_fields: Vec::new(), }); } @@ -965,6 +971,7 @@ fn create_only_items(plan: &ImportPlan) -> Vec { selected: true, resolution: None, reason: None, + changed_fields: Vec::new(), }); }; for v in &plan.resources.folders { @@ -1014,10 +1021,29 @@ const CONTENT_HASH_VERSION: &str = "v1:"; /// Identifies a resource's content well enough to tell "changed since the last import" from /// "unchanged", without keeping a copy of every imported resource around. fn content_hash(value: Value) -> String { - let canonical = serde_json::to_string(&sorted_keys(comparable(value))).unwrap_or_default(); + hash_comparable(&comparable(value)) +} + +fn hash_comparable(value: &Value) -> String { + let canonical = serde_json::to_string(&sorted_keys(value.clone())).unwrap_or_default(); format!("{CONTENT_HASH_VERSION}{:x}", Sha256::digest(canonical.as_bytes())) } +/// The fields two comparable forms disagree on, so a preview row can explain itself. +fn changed_fields(incoming: &Value, current: &Value) -> Vec { + let (Some(incoming), Some(current)) = (incoming.as_object(), current.as_object()) else { + return Vec::new(); + }; + incoming + .keys() + .chain(current.keys()) + .collect::>() + .into_iter() + .filter(|key| incoming.get(*key) != current.get(*key)) + .cloned() + .collect() +} + /// A hash written by a version this build doesn't understand says nothing about the resource. fn recorded_hash(row: &ImportSourceResource) -> Option<&str> { let hash = row.content_hash.as_deref()?; @@ -2079,6 +2105,7 @@ mod tests { let nested = item_by_name(&plan, "Nested Request"); assert_eq!(nested.action, ImportPlanAction::KeepLocal); assert!(!nested.selected); + assert_eq!(nested.changed_fields, vec!["url".to_string()], "the preview names the edit"); let extra = item_by_name(&plan, "Extra Request"); assert_eq!(extra.action, ImportPlanAction::Create); assert!(extra.selected);