Fixes #15651: Moving a category from a subcategory to root fails

Normation · Sep 6, 2019 · 2a0aae2 · 2a0aae2
1 parent 641b744
commit 2a0aae2
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 4 deletions.
diff --git a/...re/src/main/scala/com/normation/cfclerk/services/TechniqueLibraryUpdateNotification.scala b/...re/src/main/scala/com/normation/cfclerk/services/TechniqueLibraryUpdateNotification.scala
@@ -50,6 +50,7 @@ object TechniqueCategoryModType {
   case class Updated(id: TechniqueCategory) extends TechniqueCategoryModType // change in name, description
   case class Added(cat: TechniqueCategory, parentId: TechniqueCategoryId) extends TechniqueCategoryModType
   case class Deleted(cat: TechniqueCategory) extends TechniqueCategoryModType
+  // be careful - the newId is NOT the parent ID, but the full new ID so that renamed are alse taken into account
   case class Moved(oldId: TechniqueCategoryId, newId: TechniqueCategoryId) extends TechniqueCategoryModType
 }
 

diff --git a/...der-core/src/main/scala/com/normation/cfclerk/services/impl/TechniqueRepositoryImpl.scala b/...der-core/src/main/scala/com/normation/cfclerk/services/impl/TechniqueRepositoryImpl.scala
@@ -131,9 +131,10 @@ class TechniqueRepositoryImpl(
                   notInBoth.toList.flatMap { changedId =>
                     // hypothesis: directory names for category are unique in technique lib
                     (oldInfo.allCategories.find(_._2.subCategoryIds.exists(_.name == changedId.name)), techniqueInfosCache.allCategories.find(_._2.subCategoryIds.exists(_.name == changedId.name))) match {
-                      case (Some((oldId, _)), Some((newId, _))) =>
-                        if(oldId != newId) Moved(changedId, newId) :: Nil
-                        else Nil
+                      case (Some((oldParentId, _)), Some((newParentId, _))) =>
+                        if(oldParentId != newParentId) {
+                          Moved(SubTechniqueCategoryId(changedId.name, oldParentId), SubTechniqueCategoryId(changedId.name, newParentId)) :: Nil
+                        } else Nil
 
                       case (None, Some((newId, _))) =>
                         //new cat should be in new info

diff --git a/...r/rudder-core/src/test/scala/com/normation/cfclerk/services/TechniqueRepositoryTest.scala b/...r/rudder-core/src/test/scala/com/normation/cfclerk/services/TechniqueRepositoryTest.scala
@@ -168,6 +168,8 @@ class TechniqueRepositoryTest extends Specification with Loggable with AfterAll
     ).asInstanceOf[SubTechniqueCategory]
   }
 
+  val rootCategory = fsRepos.getTechniqueLibrary
+
   /**
    * Add a switch to be able to see tmp files (not clean themps) with
    * -Dtests.clean.tmp=false
@@ -198,7 +200,7 @@ class TechniqueRepositoryTest extends Specification with Loggable with AfterAll
     fsRepos.update(modid, actor, None)
     val cat = getCategory("test1")
 
-    (testCallback.categories must containTheSameElementsAs(Seq(TechniqueCategoryModType.Added(cat, cat.id.parentId)))) and
+    (testCallback.categories must containTheSameElementsAs(Seq(TechniqueCategoryModType.Added(cat, rootCategory.id)))) and
     (ldapRepo.added must beEqualTo(cat.id.name.value :: Nil) )
   }
 
@@ -214,6 +216,7 @@ class TechniqueRepositoryTest extends Specification with Loggable with AfterAll
     (ldapRepo.moved must beEqualTo((cat1.id.name.value, "Active Techniques", Some(cat2.id.name.value)) :: Nil) )
   }
 
+
   "We can delete an empty category" in {
     val cat2 = getCategory("test2")
     File(techniqueRoot, "test2").delete()
@@ -249,4 +252,23 @@ class TechniqueRepositoryTest extends Specification with Loggable with AfterAll
     (ldapRepo.moved must beEqualTo(("fileDistribution", "Active Techniques", Some("fileDistribution2")) :: Nil) ) and
     (ldapRepo.updatedTechniques must containTheSameElementsAs(Seq("copyGitFile", "fileTemplate")) )
   }
+
+  "Moving a sub-category to root works" in {
+    ldapRepo.moved = Nil
+    ldapRepo.added = Nil
+
+    createCategory("fileDistribution2/fileSecurity", "File Security")
+    fsRepos.update(modid, actor, None)
+    val cat = getCategory("fileSecurity")
+    (techniqueRoot / "fileDistribution2" / "fileSecurity").moveTo(techniqueRoot / "fileSecurity")
+    addCommitAll("Move a sub-category to root one")
+    fsRepos.update(modid, actor, None)
+    val cat2 = getCategory("fileSecurity")
+
+    (testCallback.categories must containTheSameElementsAs(Seq(TechniqueCategoryModType.Moved(cat.id, cat2.id)))) and
+    (fsRepos.getAllCategories.get(cat2.id) must beSome[TechniqueCategory]) and
+    (ldapRepo.moved must beEqualTo(("fileSecurity", "Active Techniques", None) :: Nil) ) and
+    (ldapRepo.updatedTechniques must containTheSameElementsAs(Seq()) )
+
+  }
 }