Rename ORM_MPTT::is_descendant() to ORM_MPTT::contains() to make the

API a little clearer.  Write a test for it, too.

3 files changed, 17 insertions, 4 deletions

diff --git a/modules/gallery/libraries/ORM_MPTT.php b/modules/gallery/libraries/ORM_MPTT.php
index 9b3e1f2b..83d2445c 100644
--- a/modules/gallery/libraries/ORM_MPTT.php
+++ b/modules/gallery/libraries/ORM_MPTT.php
@@ -110,7 +110,7 @@ class ORM_MPTT_Core extends ORM {
    * @param ORM $target
    * @return boolean
    */
-  function is_descendant($target) {
+  function contains($target) {
     return ($this->left_ptr <= $target->left_ptr && $this->right_ptr >= $target->right_ptr);
   }
 
@@ -212,7 +212,7 @@ class ORM_MPTT_Core extends ORM {
    * @return  ORM_MTPP
    */
   function move_to($target) {
-    if ($this->is_descendant($target)) {
+    if ($this->contains($target)) {
       throw new Exception("@todo INVALID_TARGET can't move item inside itself");
     }
 
diff --git a/modules/gallery/tests/ORM_MPTT_Test.php b/modules/gallery/tests/ORM_MPTT_Test.php
index f77f1f34..a749542b 100644
--- a/modules/gallery/tests/ORM_MPTT_Test.php
+++ b/modules/gallery/tests/ORM_MPTT_Test.php
@@ -97,6 +97,19 @@ class ORM_MPTT_Test extends Unit_Test_Case {
       $album1_2->children()->select_list());
   }
 
+  public function cant_move_parent_into_own_subtree_test() {
+    $album1 = album::create(item::root(), "move_to_test", "move_to_test");
+    $album2 = album::create($album1, "move_to_test", "move_to_test");
+    $album3 = album::create($album2, "move_to_test", "move_to_test");
+
+    try {
+      $album1->move_to($album3);
+      $self->assert_true(false, "We should be unable to move an item inside its own hierarchy");
+    } catch (Exception $e) {
+      // pass
+    }
+  }
+
   public function parent_test() {
     $root = ORM::factory("item", 1);
     $album = self::create_item_and_add_to_parent($root);
diff --git a/modules/gallery/views/move_tree.html.php b/modules/gallery/views/move_tree.html.php
index 623f80ee..e629e1bb 100644
--- a/modules/gallery/views/move_tree.html.php
+++ b/modules/gallery/views/move_tree.html.php
@@ -1,6 +1,6 @@
 <?php defined("SYSPATH") or die("No direct script access.") ?>
 <?= $parent->thumb_img(array(), 25); ?>
-<? if (!access::can("edit", $parent) || $source->is_descendant($parent)): ?>
+<? if (!access::can("edit", $parent) || $source->contains($parent)): ?>
 <a href="javascript:load_tree('<?= $parent->id ?>',1)"> <?= html::clean($parent->title) ?> <?= t("(locked)") ?> </a>
 <? else: ?>
 <a href="javascript:load_tree('<?= $parent->id ?>',0)"> <?= html::clean($parent->title) ?></a>
@@ -9,7 +9,7 @@
   <? foreach ($children as $child): ?>
   <li id="node_<?= $child->id ?>" class="node">
     <?= $child->thumb_img(array(), 25); ?>
-    <? if (!access::can("edit", $child) || $source->is_descendant($child)): ?>
+    <? if (!access::can("edit", $child) || $source->contains($child)): ?>
     <a href="javascript:load_tree('<?= $child->id ?>',1)"> <?= html::clean($child->title) ?> <?= t("(locked)") ?></a>
     <? else: ?>
     <a href="javascript:load_tree('<?= $child->id ?>',0)"> <?= html::clean($child->title) ?> </a>