mirror of
https://github.com/mountain-loop/yaak.git
synced 2026-09-09 19:31:57 +02:00
refactor(import): call a turned-down resource ignored
"Not imported" was two words in a column of one-word states, and it read as a description of this run rather than the standing decision it is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
a3e6afcdc9
commit
82ee025cd3
@@ -272,7 +272,7 @@ function LoadedImportDataDialog({
|
|||||||
if (checked) {
|
if (checked) {
|
||||||
const byId = new Map(items.map((i) => [i.modelId, i]));
|
const byId = new Map(items.map((i) => [i.modelId, i]));
|
||||||
for (const ancestor of ancestorsOf(node.data, byId)) {
|
for (const ancestor of ancestorsOf(node.data, byId)) {
|
||||||
if (ancestor.action === "not_imported") targets.add(ancestor.modelId);
|
if (ancestor.action === "ignored") targets.add(ancestor.modelId);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
setItems((prev) => prev.map((i) => (targets.has(i.modelId) ? { ...i, selected: checked } : i)));
|
setItems((prev) => prev.map((i) => (targets.has(i.modelId) ? { ...i, selected: checked } : i)));
|
||||||
@@ -293,9 +293,9 @@ function LoadedImportDataDialog({
|
|||||||
for (const parent of ancestorsOf(item, byId)) {
|
for (const parent of ancestorsOf(item, byId)) {
|
||||||
if (parent.model !== "folder") break;
|
if (parent.model !== "folder") break;
|
||||||
const missing =
|
const missing =
|
||||||
(parent.action === "create" || parent.action === "not_imported") && !parent.selected;
|
(parent.action === "create" || parent.action === "ignored") && !parent.selected;
|
||||||
// A not-imported row stays checkable: checking it brings its folders back with it
|
// An ignored row stays checkable: checking it brings its folders back with it
|
||||||
if (missing && item.action !== "delete" && item.action !== "not_imported") {
|
if (missing && item.action !== "delete" && item.action !== "ignored") {
|
||||||
disabled.add(item.modelId);
|
disabled.add(item.modelId);
|
||||||
}
|
}
|
||||||
if (parent.action === "delete" && parent.selected && item.action === "delete") {
|
if (parent.action === "delete" && parent.selected && item.action === "delete") {
|
||||||
@@ -364,7 +364,7 @@ function LoadedImportDataDialog({
|
|||||||
checked={nodeCheckedStatus}
|
checked={nodeCheckedStatus}
|
||||||
onCheck={toggleNode}
|
onCheck={toggleNode}
|
||||||
isCheckboxDisabled={(n) => disabledIds.has(n.key)}
|
isCheckboxDisabled={(n) => disabledIds.has(n.key)}
|
||||||
isCollapsedByDefault={(n) => n.data.action === "not_imported"}
|
isCollapsedByDefault={(n) => n.data.action === "ignored"}
|
||||||
isRelevant={(n) => n.data.model === "workspace" || n.data.action !== "unchanged"}
|
isRelevant={(n) => n.data.model === "workspace" || n.data.action !== "unchanged"}
|
||||||
renderRow={(n) => <ImportTreeRow item={n.data} onResolveConflict={resolveConflict} />}
|
renderRow={(n) => <ImportTreeRow item={n.data} onResolveConflict={resolveConflict} />}
|
||||||
/>
|
/>
|
||||||
@@ -597,7 +597,7 @@ function ImportTreeRow({
|
|||||||
item.action === "update" && "text-info",
|
item.action === "update" && "text-info",
|
||||||
item.action === "delete" && "text-danger",
|
item.action === "delete" && "text-danger",
|
||||||
item.action === "keep_local" && item.selected && "text-warning",
|
item.action === "keep_local" && item.selected && "text-warning",
|
||||||
item.action === "not_imported" && "text-text-subtlest",
|
item.action === "ignored" && "text-text-subtlest",
|
||||||
)}
|
)}
|
||||||
>
|
>
|
||||||
{actionLabel(item)}
|
{actionLabel(item)}
|
||||||
@@ -619,8 +619,8 @@ function actionLabel(item: ImportPlanItem): string | null {
|
|||||||
return "removed";
|
return "removed";
|
||||||
case "keep_local":
|
case "keep_local":
|
||||||
return "edited";
|
return "edited";
|
||||||
case "not_imported":
|
case "ignored":
|
||||||
return "not imported";
|
return "ignored";
|
||||||
default:
|
default:
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
@@ -637,15 +637,15 @@ function actionHelp(item: ImportPlanItem): string | null {
|
|||||||
case "update":
|
case "update":
|
||||||
return help("Changed since the last import");
|
return help("Changed since the last import");
|
||||||
case "delete":
|
case "delete":
|
||||||
return item.reason === "moved_into_not_imported_folder"
|
return item.reason === "moved_into_ignored_folder"
|
||||||
? "Moved into a folder that isn't imported. Import that folder instead to follow the move"
|
? "Moved into an ignored folder. Import that folder instead to follow the move"
|
||||||
: "Deleted since the last import";
|
: "Deleted since the last import";
|
||||||
case "keep_local":
|
case "keep_local":
|
||||||
return help("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":
|
case "conflict":
|
||||||
return help("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":
|
case "ignored":
|
||||||
return "In the file, but not imported. Check it to import it";
|
return "In the file, but ignored. Check it to import it";
|
||||||
default:
|
default:
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
@@ -717,7 +717,7 @@ function togglesWith(root: ImportPlanItem, item: ImportPlanItem): boolean {
|
|||||||
if (item.action === "keep_local") {
|
if (item.action === "keep_local") {
|
||||||
return root.modelId === item.modelId && item.model !== "folder";
|
return root.modelId === item.modelId && item.model !== "folder";
|
||||||
}
|
}
|
||||||
return item.action === "create" || item.action === "update" || item.action === "not_imported";
|
return item.action === "create" || item.action === "update" || item.action === "ignored";
|
||||||
}
|
}
|
||||||
|
|
||||||
function nodeCheckedStatus(
|
function nodeCheckedStatus(
|
||||||
|
|||||||
@@ -113,9 +113,9 @@ fn format_skipped(items: &[ImportPlanItem]) -> Option<String> {
|
|||||||
if keep_local > 0 {
|
if keep_local > 0 {
|
||||||
parts.push(format!("{keep_local} with local edits"));
|
parts.push(format!("{keep_local} with local edits"));
|
||||||
}
|
}
|
||||||
let not_imported = count(ImportPlanAction::NotImported);
|
let ignored = count(ImportPlanAction::Ignored);
|
||||||
if not_imported > 0 {
|
if ignored > 0 {
|
||||||
parts.push(format!("{not_imported} previously not imported"));
|
parts.push(format!("{ignored} ignored"));
|
||||||
}
|
}
|
||||||
let unchanged = count(ImportPlanAction::Unchanged);
|
let unchanged = count(ImportPlanAction::Unchanged);
|
||||||
if unchanged > 0 {
|
if unchanged > 0 {
|
||||||
|
|||||||
@@ -307,7 +307,7 @@ fn re_import_leaves_deleted_resources_alone() {
|
|||||||
])
|
])
|
||||||
.assert()
|
.assert()
|
||||||
.success()
|
.success()
|
||||||
.stdout(contains("Skipped 1 previously not imported"));
|
.stdout(contains("Skipped 1 ignored"));
|
||||||
|
|
||||||
let query_manager = query_manager(data_dir);
|
let query_manager = query_manager(data_dir);
|
||||||
let requests =
|
let requests =
|
||||||
|
|||||||
+2
-2
@@ -63,7 +63,7 @@ export type ImportPlanAction =
|
|||||||
| "unchanged"
|
| "unchanged"
|
||||||
| "keep_local"
|
| "keep_local"
|
||||||
| "conflict"
|
| "conflict"
|
||||||
| "not_imported";
|
| "ignored";
|
||||||
|
|
||||||
export type ImportPlanItem = {
|
export type ImportPlanItem = {
|
||||||
action: ImportPlanAction;
|
action: ImportPlanAction;
|
||||||
@@ -86,7 +86,7 @@ export type ImportPlanItem = {
|
|||||||
/**
|
/**
|
||||||
* Extra context for an action that would otherwise be indistinguishable from its plain form.
|
* Extra context for an action that would otherwise be indistinguishable from its plain form.
|
||||||
*/
|
*/
|
||||||
export type ImportPlanReason = "moved_into_not_imported_folder";
|
export type ImportPlanReason = "moved_into_ignored_folder";
|
||||||
|
|
||||||
export type ImportPlanWarning = { title: string; detail: string };
|
export type ImportPlanWarning = { title: string; detail: string };
|
||||||
|
|
||||||
|
|||||||
Generated
+2
-2
@@ -63,7 +63,7 @@ export type ImportPlanAction =
|
|||||||
| "unchanged"
|
| "unchanged"
|
||||||
| "keep_local"
|
| "keep_local"
|
||||||
| "conflict"
|
| "conflict"
|
||||||
| "not_imported";
|
| "ignored";
|
||||||
|
|
||||||
export type ImportPlanItem = {
|
export type ImportPlanItem = {
|
||||||
action: ImportPlanAction;
|
action: ImportPlanAction;
|
||||||
@@ -86,7 +86,7 @@ export type ImportPlanItem = {
|
|||||||
/**
|
/**
|
||||||
* Extra context for an action that would otherwise be indistinguishable from its plain form.
|
* Extra context for an action that would otherwise be indistinguishable from its plain form.
|
||||||
*/
|
*/
|
||||||
export type ImportPlanReason = "moved_into_not_imported_folder";
|
export type ImportPlanReason = "moved_into_ignored_folder";
|
||||||
|
|
||||||
export type ImportPlanWarning = { title: string; detail: string };
|
export type ImportPlanWarning = { title: string; detail: string };
|
||||||
|
|
||||||
|
|||||||
@@ -170,7 +170,7 @@ pub enum ImportPlanAction {
|
|||||||
KeepLocal,
|
KeepLocal,
|
||||||
Conflict,
|
Conflict,
|
||||||
/// Present in the source but previously turned down; selecting it imports it again
|
/// Present in the source but previously turned down; selecting it imports it again
|
||||||
NotImported,
|
Ignored,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Extra context for an action that would otherwise be indistinguishable from its plain form.
|
/// Extra context for an action that would otherwise be indistinguishable from its plain form.
|
||||||
@@ -178,7 +178,7 @@ pub enum ImportPlanAction {
|
|||||||
#[serde(rename_all = "snake_case")]
|
#[serde(rename_all = "snake_case")]
|
||||||
#[ts(export, export_to = "gen_util.ts")]
|
#[ts(export, export_to = "gen_util.ts")]
|
||||||
pub enum ImportPlanReason {
|
pub enum ImportPlanReason {
|
||||||
MovedIntoNotImportedFolder,
|
MovedIntoIgnoredFolder,
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, TS)]
|
#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, TS)]
|
||||||
|
|||||||
+19
-19
@@ -311,7 +311,7 @@ fn commit_plan_in_tx(db: &ClientDb, plan: ImportPlan) -> Result<BatchUpsertResul
|
|||||||
ImportPlanAction::Create
|
ImportPlanAction::Create
|
||||||
| ImportPlanAction::Update
|
| ImportPlanAction::Update
|
||||||
| ImportPlanAction::KeepLocal
|
| ImportPlanAction::KeepLocal
|
||||||
| ImportPlanAction::NotImported => item.selected,
|
| ImportPlanAction::Ignored => item.selected,
|
||||||
ImportPlanAction::Conflict => {
|
ImportPlanAction::Conflict => {
|
||||||
item.resolution == Some(ImportConflictResolution::TakeSource)
|
item.resolution == Some(ImportConflictResolution::TakeSource)
|
||||||
}
|
}
|
||||||
@@ -323,7 +323,7 @@ fn commit_plan_in_tx(db: &ClientDb, plan: ImportPlan) -> Result<BatchUpsertResul
|
|||||||
// created inside it.
|
// created inside it.
|
||||||
let is_new_folder = |id: &str| {
|
let is_new_folder = |id: &str| {
|
||||||
items.get(id).is_some_and(|i| {
|
items.get(id).is_some_and(|i| {
|
||||||
matches!(i.action, ImportPlanAction::Create | ImportPlanAction::NotImported)
|
matches!(i.action, ImportPlanAction::Create | ImportPlanAction::Ignored)
|
||||||
})
|
})
|
||||||
};
|
};
|
||||||
let mut missing_folders: BTreeSet<String> = plan
|
let mut missing_folders: BTreeSet<String> = plan
|
||||||
@@ -526,7 +526,7 @@ fn record_import_source(
|
|||||||
| ImportPlanAction::Conflict,
|
| ImportPlanAction::Conflict,
|
||||||
) => Some((Some(model_id.to_string()), Some(content_hash(incoming()?)))),
|
) => Some((Some(model_id.to_string()), Some(content_hash(incoming()?)))),
|
||||||
// Turned down, so remember it as not wanted rather than offering it again
|
// Turned down, so remember it as not wanted rather than offering it again
|
||||||
Some(ImportPlanAction::Create | ImportPlanAction::NotImported) => Some((None, None)),
|
Some(ImportPlanAction::Create | ImportPlanAction::Ignored) => Some((None, None)),
|
||||||
Some(ImportPlanAction::Delete) if item.is_some_and(|i| i.selected) => {
|
Some(ImportPlanAction::Delete) if item.is_some_and(|i| i.selected) => {
|
||||||
Some((None, None))
|
Some((None, None))
|
||||||
}
|
}
|
||||||
@@ -798,7 +798,7 @@ fn merge_with_linked_source(
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
let not_imported_folders = plan
|
let ignored_folders = plan
|
||||||
.resources
|
.resources
|
||||||
.folders
|
.folders
|
||||||
.iter()
|
.iter()
|
||||||
@@ -811,14 +811,14 @@ fn merge_with_linked_source(
|
|||||||
.iter()
|
.iter()
|
||||||
.map(|v| (v.id.as_str(), v.folder_id.as_deref()))
|
.map(|v| (v.id.as_str(), v.folder_id.as_deref()))
|
||||||
.collect::<BTreeMap<_, _>>();
|
.collect::<BTreeMap<_, _>>();
|
||||||
let inside_a_not_imported_folder = |parent_id: Option<&str>| {
|
let inside_an_ignored_folder = |parent_id: Option<&str>| {
|
||||||
let mut seen = BTreeSet::new();
|
let mut seen = BTreeSet::new();
|
||||||
let mut next = parent_id;
|
let mut next = parent_id;
|
||||||
while let Some(id) = next {
|
while let Some(id) = next {
|
||||||
if !seen.insert(id) {
|
if !seen.insert(id) {
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
if not_imported_folders.contains(id) {
|
if ignored_folders.contains(id) {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
next = folder_parents.get(id).copied().flatten();
|
next = folder_parents.get(id).copied().flatten();
|
||||||
@@ -847,14 +847,14 @@ fn merge_with_linked_source(
|
|||||||
|
|
||||||
// Nothing can be created inside a folder that isn't imported, so it starts unchecked
|
// Nothing can be created inside a folder that isn't imported, so it starts unchecked
|
||||||
// and comes along only if the folder does.
|
// and comes along only if the folder does.
|
||||||
let reachable = !inside_a_not_imported_folder(parent_id.as_deref());
|
let reachable = !inside_an_ignored_folder(parent_id.as_deref());
|
||||||
let row = match status(&planned_id, resource) {
|
let row = match status(&planned_id, resource) {
|
||||||
KeyStatus::New => {
|
KeyStatus::New => {
|
||||||
items.push(item(ImportPlanAction::Create, reachable, None, None));
|
items.push(item(ImportPlanAction::Create, reachable, None, None));
|
||||||
return Ok(());
|
return Ok(());
|
||||||
}
|
}
|
||||||
KeyStatus::NotWanted => {
|
KeyStatus::NotWanted => {
|
||||||
items.push(item(ImportPlanAction::NotImported, false, None, None));
|
items.push(item(ImportPlanAction::Ignored, false, None, None));
|
||||||
return Ok(());
|
return Ok(());
|
||||||
}
|
}
|
||||||
KeyStatus::Wanted(row) => row,
|
KeyStatus::Wanted(row) => row,
|
||||||
@@ -866,7 +866,7 @@ fn merge_with_linked_source(
|
|||||||
ImportPlanAction::Delete,
|
ImportPlanAction::Delete,
|
||||||
false,
|
false,
|
||||||
None,
|
None,
|
||||||
Some(ImportPlanReason::MovedIntoNotImportedFolder),
|
Some(ImportPlanReason::MovedIntoIgnoredFolder),
|
||||||
));
|
));
|
||||||
return Ok(());
|
return Ok(());
|
||||||
}
|
}
|
||||||
@@ -1158,7 +1158,7 @@ fn validate_plan(plan: &ImportPlan) -> Result<()> {
|
|||||||
i.model_id == id
|
i.model_id == id
|
||||||
&& !matches!(
|
&& !matches!(
|
||||||
i.action,
|
i.action,
|
||||||
ImportPlanAction::Create | ImportPlanAction::NotImported
|
ImportPlanAction::Create | ImportPlanAction::Ignored
|
||||||
)
|
)
|
||||||
})
|
})
|
||||||
};
|
};
|
||||||
@@ -2334,7 +2334,7 @@ mod tests {
|
|||||||
|
|
||||||
let plan = replan(&query_manager, &workspace_id, imported_resources());
|
let plan = replan(&query_manager, &workspace_id, imported_resources());
|
||||||
let root = item_by_name(&plan, "Root Request");
|
let root = item_by_name(&plan, "Root Request");
|
||||||
assert_eq!(root.action, ImportPlanAction::NotImported, "deleting it means not wanting it");
|
assert_eq!(root.action, ImportPlanAction::Ignored, "deleting it means not wanting it");
|
||||||
assert!(!root.selected);
|
assert!(!root.selected);
|
||||||
assert_ne!(root.model_id, root_id);
|
assert_ne!(root.model_id, root_id);
|
||||||
|
|
||||||
@@ -2346,7 +2346,7 @@ mod tests {
|
|||||||
);
|
);
|
||||||
|
|
||||||
let plan = replan(&query_manager, &workspace_id, imported_resources());
|
let plan = replan(&query_manager, &workspace_id, imported_resources());
|
||||||
assert_eq!(item_by_name(&plan, "Root Request").action, ImportPlanAction::NotImported);
|
assert_eq!(item_by_name(&plan, "Root Request").action, ImportPlanAction::Ignored);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
@@ -2371,12 +2371,12 @@ mod tests {
|
|||||||
|
|
||||||
let plan = replan(&query_manager, &workspace_id, resources);
|
let plan = replan(&query_manager, &workspace_id, resources);
|
||||||
let extra = item_by_name(&plan, "Extra Request");
|
let extra = item_by_name(&plan, "Extra Request");
|
||||||
assert_eq!(extra.action, ImportPlanAction::NotImported, "deselecting it is remembered");
|
assert_eq!(extra.action, ImportPlanAction::Ignored, "deselecting it is remembered");
|
||||||
assert!(!extra.selected);
|
assert!(!extra.selected);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn restoring_a_not_imported_item_relinks_its_source_key() {
|
fn restoring_an_ignored_item_relinks_its_source_key() {
|
||||||
let (query_manager, _blob_manager, _rx) =
|
let (query_manager, _blob_manager, _rx) =
|
||||||
yaak_models::init_in_memory().expect("initialize database");
|
yaak_models::init_in_memory().expect("initialize database");
|
||||||
let committed = first_import(&query_manager);
|
let committed = first_import(&query_manager);
|
||||||
@@ -2390,7 +2390,7 @@ mod tests {
|
|||||||
commit_import_plan(&query_manager, plan).expect("commit with deselected create");
|
commit_import_plan(&query_manager, plan).expect("commit with deselected create");
|
||||||
|
|
||||||
let mut plan = replan(&query_manager, &workspace_id, resources.clone());
|
let mut plan = replan(&query_manager, &workspace_id, resources.clone());
|
||||||
assert_eq!(item_by_name(&plan, "Extra Request").action, ImportPlanAction::NotImported);
|
assert_eq!(item_by_name(&plan, "Extra Request").action, ImportPlanAction::Ignored);
|
||||||
select(&mut plan, "Extra Request", true);
|
select(&mut plan, "Extra Request", true);
|
||||||
commit_import_plan(&query_manager, plan).expect("commit with restored item");
|
commit_import_plan(&query_manager, plan).expect("commit with restored item");
|
||||||
|
|
||||||
@@ -2410,7 +2410,7 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn moving_into_a_not_imported_folder_offers_a_delete() {
|
fn moving_into_an_ignored_folder_offers_a_delete() {
|
||||||
let (query_manager, _blob_manager, _rx) =
|
let (query_manager, _blob_manager, _rx) =
|
||||||
yaak_models::init_in_memory().expect("initialize database");
|
yaak_models::init_in_memory().expect("initialize database");
|
||||||
let committed = first_import(&query_manager);
|
let committed = first_import(&query_manager);
|
||||||
@@ -2437,11 +2437,11 @@ mod tests {
|
|||||||
|
|
||||||
// The source moves an imported request into the folder that was turned down.
|
// The source moves an imported request into the folder that was turned down.
|
||||||
let plan = replan(&query_manager, &workspace_id, with_extra_folder(true));
|
let plan = replan(&query_manager, &workspace_id, with_extra_folder(true));
|
||||||
assert_eq!(item_by_name(&plan, "Extra Folder").action, ImportPlanAction::NotImported);
|
assert_eq!(item_by_name(&plan, "Extra Folder").action, ImportPlanAction::Ignored);
|
||||||
let root = item_by_name(&plan, "Root Request");
|
let root = item_by_name(&plan, "Root Request");
|
||||||
assert_eq!(root.action, ImportPlanAction::Delete);
|
assert_eq!(root.action, ImportPlanAction::Delete);
|
||||||
assert!(!root.selected, "a deletion is never applied by default");
|
assert!(!root.selected, "a deletion is never applied by default");
|
||||||
assert_eq!(root.reason, Some(ImportPlanReason::MovedIntoNotImportedFolder));
|
assert_eq!(root.reason, Some(ImportPlanReason::MovedIntoIgnoredFolder));
|
||||||
|
|
||||||
// Anything new inside that folder can't be created either, so it waits for the folder.
|
// Anything new inside that folder can't be created either, so it waits for the folder.
|
||||||
let mut resources = with_extra_folder(true);
|
let mut resources = with_extra_folder(true);
|
||||||
@@ -2466,7 +2466,7 @@ mod tests {
|
|||||||
let plan = replan(&query_manager, &workspace_id, with_extra_folder(true));
|
let plan = replan(&query_manager, &workspace_id, with_extra_folder(true));
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
item_by_name(&plan, "Root Request").action,
|
item_by_name(&plan, "Root Request").action,
|
||||||
ImportPlanAction::NotImported,
|
ImportPlanAction::Ignored,
|
||||||
"accepting the deletion means the resource is no longer wanted"
|
"accepting the deletion means the resource is no longer wanted"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user