mirror of
https://github.com/mountain-loop/yaak.git
synced 2026-09-06 18:07:18 +02:00
feat(import): say which fields differ, and stop offering to apply nothing
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
72d3bda769
commit
6796569466
@@ -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"}
|
||||
</Button>
|
||||
</HStack>
|
||||
</VStack>
|
||||
@@ -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<string, ImportPlanItem>): ImportPlanItem[] {
|
||||
const ancestors: ImportPlanItem[] = [];
|
||||
|
||||
@@ -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<string>;
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
Generated
+4
@@ -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<string>;
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
@@ -205,6 +205,9 @@ pub struct ImportPlanItem {
|
||||
pub resolution: Option<ImportConflictResolution>,
|
||||
#[ts(optional)]
|
||||
pub reason: Option<ImportPlanReason>,
|
||||
/// Fields where the source and the local copy disagree, so the preview can say why
|
||||
#[serde(default)]
|
||||
pub changed_fields: Vec<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize, Serialize, TS)]
|
||||
|
||||
@@ -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<ImportPlanItem> {
|
||||
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<String> {
|
||||
let (Some(incoming), Some(current)) = (incoming.as_object(), current.as_object()) else {
|
||||
return Vec::new();
|
||||
};
|
||||
incoming
|
||||
.keys()
|
||||
.chain(current.keys())
|
||||
.collect::<BTreeSet<_>>()
|
||||
.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);
|
||||
|
||||
Reference in New Issue
Block a user