diff --git a/crates/engine/src/cmd/variables.rs b/crates/engine/src/cmd/variables.rs index 85e1454f3..d8c20e05a 100644 --- a/crates/engine/src/cmd/variables.rs +++ b/crates/engine/src/cmd/variables.rs @@ -184,7 +184,10 @@ fn duplicate(s: &mut Session, p: &Value) -> Result { let cmd = "variable.duplicate"; let v = req_string(cmd, p, "variable")?; let doc = s.doc_mut()?; - let pos = doc.variables.variables.iter().position(|x| x.id == v || x.name == v).ok_or_else(|| bad(cmd, "no such variable"))?; + // An exact id first, then a name: an earlier variable named like another's id must not be the + // one copied. + let vars = &doc.variables.variables; + let pos = vars.iter().position(|x| x.id == v).or_else(|| vars.iter().position(|x| x.name == v)).ok_or_else(|| bad(cmd, "no such variable"))?; let mut copy = doc.variables.variables.get(pos).cloned().ok_or_else(|| bad(cmd, "no such variable"))?; let taken = |n: &str| doc.variables.variables.iter().any(|x| x.collection == copy.collection && x.name == n); let name = (1..1000) diff --git a/crates/engine/src/variables_editor_tests.rs b/crates/engine/src/variables_editor_tests.rs index 806300c70..65cc04763 100644 --- a/crates/engine/src/variables_editor_tests.rs +++ b/crates/engine/src/variables_editor_tests.rs @@ -47,6 +47,18 @@ fn rename_describe_scope_and_duplicate() { assert_eq!(vars.get(&id).unwrap().scopes, brand.scopes); } +#[test] +fn duplicate_by_id_copies_that_variable_even_when_an_earlier_one_is_named_like_its_id() { + let mut s = Session::new(); + tokens(&mut s); + let brand = s.doc().unwrap().variables.get("brand").unwrap().id.clone(); + // The first variable in the list takes the later one's id as its name. + ex(&mut s, "variable.update", json!({"variable": "space/sm", "name": brand.clone()})); + let id = ex(&mut s, "variable.duplicate", json!({"variable": brand.clone()}))["id"].as_str().unwrap().to_string(); + let vars = &s.doc().unwrap().variables; + assert_eq!(vars.get(&id).unwrap().name, "brand copy"); +} + #[test] fn modes_and_collections_rename_and_delete() { let mut s = Session::new();