add tests for scratch users and migrate getUsers,isInGroup to use it

lib/WebGUI/Group.pm

@@ -473,6 +473,46 @@ sub getKarmaUsers {
 
 #-------------------------------------------------------------------
 
+=head2 getScratchUsers ( )
+
+Get the set of users allowed to be in this group via session scratch variable settings
+and this group's scratchFilter.  The set is returned as an array ref.
+
+If no scratchFilter has been set for this group, returns an empty array ref.
+
+=cut
+
+sub getScratchUsers {
+	my $self = shift;
+	my $scratchFilter;
+	return [] unless $scratchFilter = $self->scratchFilter();
+
+	my $sessionId = $self->session->db->quote($self->session->getId());
+	my $time = $self->session->datetime->time();
+
+	$scratchFilter =~ s/\s//g;
+	my @filters = split /;/, $scratchFilter;
+
+	my @scratchClauses = ();
+	my @scratchPlaceholders = ();
+	foreach my $filter (@filters) {
+		my ($name, $value) = split /=/, $filter;
+		push @scratchClauses, "(s.name=? AND s.value=?)";
+		push @scratchPlaceholders, $name, $value;
+	}
+	my $scratchClause = join ' OR ', @scratchClauses;
+
+	my $query = <<EOQ;
+select u.userId from userSession u, userSessionScratch s where
+u.sessionId=s.sessionId AND
+u.expires > $time AND
+	( $scratchClause )
+EOQ
+	return $self->session->db->buildArrayRef($query, [ @scratchPlaceholders ]);
+}	
+
+#-------------------------------------------------------------------
+
 =head2 find ( session, name )
 
 An alternative to the constructor "new", use find as a constructor by name rather than id.
@@ -631,6 +671,7 @@ sub getUsers {
 			push @externalUsers,
 				@{ $extGroup->getDatabaseUsers() },
 				@{ $extGroup->getKarmaUsers() },
+				@{ $extGroup->getScratchUsers() },
 			;
 		}
 	}
@@ -639,6 +680,7 @@ sub getUsers {
 	push @externalUsers,
 		@{ $self->getDatabaseUsers() },
 		@{ $self->getKarmaUsers() },
+		@{ $self->getScratchUsers() },
 	;
 	my @users = ( @localUsers, @externalUsers );
 	return \@users;

lib/WebGUI/User.pm

@@ -286,20 +286,6 @@ sub isInGroup {
                 	return 1;
         	}
 	}
-        ### Check Scratch Variables 
-        if ($group->scratchFilter()) {
-		my $scratchFilter = $group->scratchFilter();
-                $scratchFilter =~ s/\s//g;
-                my @vars = split(";",$scratchFilter);
-                foreach my $var (@vars) {
-                        my ($name, $value) = split(/\=/,$var);
-                        if ($self->session->scratch->get($name) eq $value) {
-                                $isInGroup->{$uid}{$gid} = 1;
-				$self->session->stow->set("isInGroup",$isInGroup);
-                                return 1;
-                        }
-                }
-        }
 	 ### Check ldap
         if ($group->get("ldapGroup") && $group->get("ldapGroupProperty")) {
 		   # skip if not logged in
@@ -355,6 +341,13 @@ sub isInGroup {
 		$self->session->stow->set("isInGroup",$isInGroup);
 		return 1 if ($isInGroup->{$uid}{$gid});
 	}
+        if (my @scratchUsers = @{ $group->getScratchUsers() }) {
+		foreach my $extUserId ( @scratchUsers ) {
+			$isInGroup->{$extUserId}{$gid} = 1;
+		}
+		$self->session->stow->set("isInGroup",$isInGroup);
+		return 1 if ($isInGroup->{$uid}{$gid});
+	}
         ### Check for groups of groups.
         my $groups = $group->getGroupsIn(1);
         foreach (@{$groups}) {

t/Group.t

@@ -50,7 +50,7 @@ my @scratchTests = (
 			},
 );
 
-plan tests => (83 + scalar(@scratchTests)); # increment this value for each test you create
+plan tests => (85 + scalar(@scratchTests)); # increment this value for each test you create
 
 my $session = WebGUI::Test->session;
 
@@ -385,6 +385,18 @@ foreach my $scratchTest (@scratchTests) {
 	is($scratchTest->{user}->isInGroup($gS->getId), $scratchTest->{expect}, $scratchTest->{comment});
 }
 
+cmp_bag(
+	$gS->getScratchUsers,
+	[ (map { $_->{user}->userId() }  grep { $_->{expect} } @scratchTests) ],
+	'getScratchUsers'
+);
+
+cmp_bag(
+	$gS->getUsers,
+	[ (map { $_->{user}->userId() }  grep { $_->{expect} } @scratchTests) ],
+	'getUsers for group with scratch'
+);
+
 SKIP: {
 	skip("need to test expiration date in groupings interacting with recursive or not", 1);
 	ok(undef, "expiration date in groupings for getUser");
@@ -399,7 +411,10 @@ END {
 	}
 	$session->db->dbh->do('DROP TABLE IF EXISTS myUserTable');
 
+
 	foreach my $subSession (@sessionBank) {
+		$subSession->db->write("DELETE FROM userSession WHERE sessionId=?",[ $subSession->getId]);
+		$subSession->db->write("DELETE FROM userSessionScratch WHERE sessionId=?",[ $subSession->getId]);
 		$subSession->close() if (defined $subSession and ref $subSession eq 'WebGUI::Session');
 	}
 }