From 8b84ae3e7651ac9869f40789e5b70f7561a1ab4d Mon Sep 17 00:00:00 2001
From: Colin Kuskie <colink@perldreamer.com>
Date: Mon, 26 Nov 2007 05:13:32 +0000
Subject: [PATCH] Up coverage of addGroups to 100%. Change tabs to spaces in
 group diagrams. Optimize the addGroups membership check so the expensive
 getGroupsIn doesn't have to take place. Add a label to the foreach.

---
 lib/WebGUI/Group.pm |  8 +++----
 t/Group.t           | 53 ++++++++++++++++++++++++---------------------
 2 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/lib/WebGUI/Group.pm b/lib/WebGUI/Group.pm
index dac2419c6..ce2ca5f22 100755
--- a/lib/WebGUI/Group.pm
+++ b/lib/WebGUI/Group.pm
@@ -124,15 +124,15 @@ sub addGroups {
 	my $self = shift;
 	my $groups = shift;
 	WebGUI::Cache->new($self->session, $self->getId)->delete;
-	foreach my $gid (@{$groups}) {
+	GROUP: foreach my $gid (@{$groups}) {
 		next if ($gid eq '1');
 		next if ($gid eq $self->getId);
 		my ($isIn) = $self->session->db->quickArray("select count(*) from groupGroupings where groupId=? and inGroup=?", [$gid, $self->getId]);
+        next GROUP if $isIn;
 		my $group = WebGUI::Group->new($self->session, $gid);
 		my $recursive = isIn($self->getId, @{$group->getGroupsIn(1)});
-		unless ($isIn || $recursive) {
-			$self->session->db->write("insert into groupGroupings (groupId,inGroup) values (?,?)",[$gid, $self->getId]);
-		}
+        next GROUP if $recursive;
+        $self->session->db->write("insert into groupGroupings (groupId,inGroup) values (?,?)",[$gid, $self->getId]);
 	}
 	$self->clearCaches();
 	return 1;
diff --git a/t/Group.t b/t/Group.t
index fab2e2748..51b2a9053 100644
--- a/t/Group.t
+++ b/t/Group.t
@@ -75,7 +75,7 @@ my @ipTests = (
 );
 
 
-plan tests => (138 + scalar(@scratchTests) + scalar(@ipTests)); # increment this value for each test you create
+plan tests => (139 + scalar(@scratchTests) + scalar(@ipTests)); # increment this value for each test you create
 
 my $session = WebGUI::Test->session;
 my $testCache = WebGUI::Cache->new($session, 'myTestKey');
@@ -243,9 +243,9 @@ cmp_bag($gA->getGroupsIn(),  [3], 'Group C is not a member of Group A');
 cmp_bag($gB->getGroupsIn(1), [$gA->getId, 3], 'Group C is not in Group B, recursively');
 cmp_bag($gC->getGroupsFor(), [], 'No groups contain Group C');
 
-#	B		Z
-#      / \	       / \
-#     A   C	      Y   X
+#   B       Z
+#  / \     / \
+# A   C	  Y   X
 
 $gB->addGroups([$gC->getId]);
 
@@ -258,13 +258,13 @@ $gZ->name('Group Z');
 
 $gZ->addGroups([$gX->getId, $gY->getId]);
 
-#		B
-#	       / \
-#	      A   C
-#	      |
-#	      Z
-#            / \
-#           X   Y
+#      B
+#	  / \
+#	 A   C
+#    |
+#    Z
+#   / \
+#  X   Y
 
 $gA->addGroups([$gZ->getId]);
 cmp_bag($gB->getGroupsIn(1), [$gA->getId, $gC->getId, $gZ->getId, $gY->getId, $gX->getId, 3], 'Add Z tree to A under B');
@@ -272,6 +272,9 @@ cmp_bag($gB->getGroupsIn(1), [$gA->getId, $gC->getId, $gZ->getId, $gY->getId, $g
 $gX->addGroups([$gA->getId]);
 cmp_bag($gX->getGroupsIn(), [3], 'Not able to add B tree under Z tree under X');
 
+$gZ->addGroups([$gX->getId]);
+cmp_bag($gZ->getGroupsIn(), [$gX->getId, $gY->getId, 3], 'Not able to add a group when it is already a member of a group');
+
 cmp_bag($gX->getAllGroupsFor(), [ map {$_->getId} ($gZ, $gA, $gB) ], 'getAllGroupsFor X');
 cmp_bag($gY->getAllGroupsFor(), [ map {$_->getId} ($gZ, $gA, $gB) ], 'getAllGroupsFor Y');
 cmp_bag($gZ->getAllGroupsFor(), [ map {$_->getId} ($gA, $gB) ], 'getAllGroupsFor Z');
@@ -406,13 +409,13 @@ my $gK = WebGUI::Group->new($session, "new");
 $gK->name('Group K');
 $gC->addGroups([$gK->getId]);
 
-#		B
-#	       / \
-#	      A   C
-#	      |   |
-#	      Z   K
-#            / \
-#           X   Y
+#      B
+#     / \
+#    A   C
+#    |   |
+#    Z   K
+#   / \
+#  X   Y
 
 $gK->karmaThreshold(5);
 
@@ -465,13 +468,13 @@ my $gS = WebGUI::Group->new($session, "new");
 $gS->name('Group S');
 $gC->addGroups([$gS->getId]);
 
-#		B
-#	       / \
-#	      A   C
-#	      |   | \
-#	      Z   K  S
-#            / \
-#           X   Y
+#        B
+#    	/ \
+#      A   C
+#      |   | \
+#      Z   K  S
+#     / \
+#    X   Y
 
 
 $gS->scratchFilter('name=Tom;airport=PDX');