From bd95bcaf6149b04ce4478d257098b469646f0f7d Mon Sep 17 00:00:00 2001
From: Sean Harvey <halkyon@gmail.com>
Date: Mon, 26 Mar 2012 22:05:38 +1300
Subject: [PATCH] BUGFIX Nested Group records should be removed, along with the
 parent.

---
 security/Group.php           | 18 +++++++++++-------
 tests/security/GroupTest.php | 22 +++++++++++++---------
 tests/security/GroupTest.yml |  5 ++++-
 3 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/security/Group.php b/security/Group.php
index b4b8fde83..6290016f1 100755
--- a/security/Group.php
+++ b/security/Group.php
@@ -337,17 +337,21 @@ class Group extends DataObject {
 			if(!$this->Code) $this->setCode($this->Title);
 		}
 	}
-	
-	function onAfterDelete() {
-		parent::onAfterDelete();
-		
+
+	function onBeforeDelete() {
+		parent::onBeforeDelete();
+
+		// if deleting this group, delete it's children as well
+		foreach($this->Groups() as $group) {
+			$group->delete();
+		}
+
 		// Delete associated permissions
-		$permissions = $this->Permissions();
-		foreach ( $permissions as $permission ) {
+		foreach($this->Permissions() as $permission) {
 			$permission->delete();
 		}
 	}
-	
+
 	/**
 	 * Checks for permission-code CMS_ACCESS_SecurityAdmin.
 	 * If the group has ADMIN permissions, it requires the user to have ADMIN permissions as well.
diff --git a/tests/security/GroupTest.php b/tests/security/GroupTest.php
index b3e0c08ec..2d364168d 100644
--- a/tests/security/GroupTest.php
+++ b/tests/security/GroupTest.php
@@ -94,15 +94,6 @@ class GroupTest extends FunctionalTest {
 
 	   }
 	
-	function testDelete() {
-		$adminGroup = $this->objFromFixture('Group', 'admingroup');
-		
-		$adminGroup->delete();
-		
-		$this->assertEquals(0, DataObject::get('Group', "\"ID\"={$adminGroup->ID}")->count(), 'Group is removed');
-		$this->assertEquals(0, DataObject::get('Permission',"\"GroupID\"={$adminGroup->ID}")->count(), 'Permissions removed along with the group');
-	}
-	
 	function testCollateAncestorIDs() {
 		$parentGroup = $this->objFromFixture('Group', 'parentgroup');
 		$childGroup = $this->objFromFixture('Group', 'childgroup');
@@ -128,6 +119,19 @@ class GroupTest extends FunctionalTest {
 			'Orphaned nodes dont contain invalid parent IDs'
 		);
 	}
+
+	public function testDelete() {
+		$group = $this->objFromFixture('Group', 'parentgroup');
+		$groupID = $group->ID;
+		$childGroupID = $this->idFromFixture('Group', 'childgroup');
+		$group->delete();
+
+		$this->assertEquals(0, DataObject::get('Group', "\"ID\" = {$groupID}")->Count(), 'Group is removed');
+		$this->assertEquals(0, DataObject::get('Permission', "\"GroupID\" = {$groupID}")->Count(), 'Permissions removed along with the group');
+		$this->assertEquals(0, DataObject::get('Group', "\"ParentID\" = {$groupID}")->Count(), 'Child groups are removed');
+		$this->assertEquals(0, DataObject::get('Group', "\"ParentID\" = {$childGroupID}")->Count(), 'Grandchild groups are removed');
+	}
+
 }
 
 class GroupTest_Member extends Member implements TestOnly {
diff --git a/tests/security/GroupTest.yml b/tests/security/GroupTest.yml
index a0c3d8a97..13ca9f5ce 100644
--- a/tests/security/GroupTest.yml
+++ b/tests/security/GroupTest.yml
@@ -6,6 +6,9 @@ Group:
    childgroup:
       Code: childgroup
       Parent: =>Group.parentgroup
+   grandchildgroup:
+      Code: grandchildgroup
+      Parent: =>Group.childgroup
    group1:
       Title: Group 1
    group2:
@@ -26,4 +29,4 @@ GroupTest_Member:
 Permission:
    admincode:
       Code: ADMIN
-      Group: =>Group.admingroup
\ No newline at end of file
+      Group: =>Group.admingroup