Fix some issues with tag reparenting (#2146)
* Fix reparented_name not correctly handling tags that are prefixes of the new parent To reproduce the issue: 1. Add two tags: `a` and `ab`. 2. From the browser's sidebar, drag & drop `a` into `ab`. Result: panic * Fix reparent_tags panicking if new parent is a child of source tag This is the "foo, foo::bar" case that should be a no-op. * Add more tests for tag reparenting
This commit is contained in:
parent
9fb3eb86d5
commit
951c80a4e9
@ -38,7 +38,9 @@ impl Collection {
|
|||||||
let usn = self.usn()?;
|
let usn = self.usn()?;
|
||||||
let mut matcher = TagMatcher::new(&join_tags(tags_to_reparent))?;
|
let mut matcher = TagMatcher::new(&join_tags(tags_to_reparent))?;
|
||||||
let old_to_new_names = old_to_new_names(tags_to_reparent, new_parent);
|
let old_to_new_names = old_to_new_names(tags_to_reparent, new_parent);
|
||||||
|
if old_to_new_names.is_empty() {
|
||||||
|
return Ok(0);
|
||||||
|
}
|
||||||
let matched_notes = self
|
let matched_notes = self
|
||||||
.storage
|
.storage
|
||||||
.get_note_tags_by_predicate(|tags| matcher.is_match(tags))?;
|
.get_note_tags_by_predicate(|tags| matcher.is_match(tags))?;
|
||||||
@ -92,8 +94,10 @@ fn old_to_new_names(
|
|||||||
/// Returns None if new parent is a child of the tag to be reparented.
|
/// Returns None if new parent is a child of the tag to be reparented.
|
||||||
fn reparented_name(existing_name: &str, new_parent: Option<&str>) -> Option<String> {
|
fn reparented_name(existing_name: &str, new_parent: Option<&str>) -> Option<String> {
|
||||||
let existing_base = existing_name.rsplit("::").next().unwrap();
|
let existing_base = existing_name.rsplit("::").next().unwrap();
|
||||||
|
let existing_root = existing_name.split("::").next().unwrap();
|
||||||
if let Some(new_parent) = new_parent {
|
if let Some(new_parent) = new_parent {
|
||||||
if new_parent.starts_with(existing_name) {
|
let new_parent_root = new_parent.split("::").next().unwrap();
|
||||||
|
if new_parent_root == existing_root {
|
||||||
// foo onto foo::bar, or foo onto itself -> no-op
|
// foo onto foo::bar, or foo onto itself -> no-op
|
||||||
None
|
None
|
||||||
} else {
|
} else {
|
||||||
@ -125,6 +129,8 @@ mod test {
|
|||||||
let mut col = open_test_collection();
|
let mut col = open_test_collection();
|
||||||
let nt = col.get_notetype_by_name("Basic")?.unwrap();
|
let nt = col.get_notetype_by_name("Basic")?.unwrap();
|
||||||
for tag in &[
|
for tag in &[
|
||||||
|
"a",
|
||||||
|
"ab",
|
||||||
"another",
|
"another",
|
||||||
"parent1::child1::grandchild1",
|
"parent1::child1::grandchild1",
|
||||||
"parent1::child1",
|
"parent1::child1",
|
||||||
@ -147,6 +153,8 @@ mod test {
|
|||||||
assert_eq!(
|
assert_eq!(
|
||||||
alltags(&col),
|
alltags(&col),
|
||||||
&[
|
&[
|
||||||
|
"a",
|
||||||
|
"ab",
|
||||||
"parent1",
|
"parent1",
|
||||||
"parent1::another",
|
"parent1::another",
|
||||||
"parent1::child1",
|
"parent1::child1",
|
||||||
@ -164,6 +172,8 @@ mod test {
|
|||||||
assert_eq!(
|
assert_eq!(
|
||||||
alltags(&col),
|
alltags(&col),
|
||||||
&[
|
&[
|
||||||
|
"a",
|
||||||
|
"ab",
|
||||||
"parent1",
|
"parent1",
|
||||||
"parent1::another",
|
"parent1::another",
|
||||||
"parent2",
|
"parent2",
|
||||||
@ -178,6 +188,43 @@ mod test {
|
|||||||
assert_eq!(
|
assert_eq!(
|
||||||
alltags(&col),
|
alltags(&col),
|
||||||
&[
|
&[
|
||||||
|
"a",
|
||||||
|
"ab",
|
||||||
|
"another",
|
||||||
|
"parent1",
|
||||||
|
"parent2",
|
||||||
|
"parent2::child1",
|
||||||
|
"parent2::child1::grandchild1",
|
||||||
|
]
|
||||||
|
);
|
||||||
|
|
||||||
|
// parent1 onto parent1::child1 -> no-op
|
||||||
|
col.reparent_tags(
|
||||||
|
&["parent1".to_string()],
|
||||||
|
Some("parent1::child1".to_string()),
|
||||||
|
)?;
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
alltags(&col),
|
||||||
|
&[
|
||||||
|
"a",
|
||||||
|
"ab",
|
||||||
|
"another",
|
||||||
|
"parent1",
|
||||||
|
"parent2",
|
||||||
|
"parent2::child1",
|
||||||
|
"parent2::child1::grandchild1",
|
||||||
|
]
|
||||||
|
);
|
||||||
|
|
||||||
|
// tags that are prefixes of the new parent are handled correctly
|
||||||
|
col.reparent_tags(&["a".to_string()], Some("ab".to_string()))?;
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
alltags(&col),
|
||||||
|
&[
|
||||||
|
"ab",
|
||||||
|
"ab::a",
|
||||||
"another",
|
"another",
|
||||||
"parent1",
|
"parent1",
|
||||||
"parent2",
|
"parent2",
|
||||||
|
Loading…
Reference in New Issue
Block a user