From 951c80a4e98a428ce437aece1974b5b8e6756564 Mon Sep 17 00:00:00 2001
From: Abdo <abd.nh25@gmail.com>
Date: Fri, 28 Oct 2022 14:42:10 +0300
Subject: [PATCH] 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
---
 rslib/src/tags/reparent.rs | 51 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 49 insertions(+), 2 deletions(-)

diff --git a/rslib/src/tags/reparent.rs b/rslib/src/tags/reparent.rs
index bdedd38b0..40755f15a 100644
--- a/rslib/src/tags/reparent.rs
+++ b/rslib/src/tags/reparent.rs
@@ -38,7 +38,9 @@ impl Collection {
         let usn = self.usn()?;
         let mut matcher = TagMatcher::new(&join_tags(tags_to_reparent))?;
         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
             .storage
             .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.
 fn reparented_name(existing_name: &str, new_parent: Option<&str>) -> Option<String> {
     let existing_base = existing_name.rsplit("::").next().unwrap();
+    let existing_root = existing_name.split("::").next().unwrap();
     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
             None
         } else {
@@ -125,6 +129,8 @@ mod test {
         let mut col = open_test_collection();
         let nt = col.get_notetype_by_name("Basic")?.unwrap();
         for tag in &[
+            "a",
+            "ab",
             "another",
             "parent1::child1::grandchild1",
             "parent1::child1",
@@ -147,6 +153,8 @@ mod test {
         assert_eq!(
             alltags(&col),
             &[
+                "a",
+                "ab",
                 "parent1",
                 "parent1::another",
                 "parent1::child1",
@@ -164,6 +172,8 @@ mod test {
         assert_eq!(
             alltags(&col),
             &[
+                "a",
+                "ab",
                 "parent1",
                 "parent1::another",
                 "parent2",
@@ -178,6 +188,43 @@ mod test {
         assert_eq!(
             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",
                 "parent1",
                 "parent2",