Group.t: group karma tests, better clean-up at end

User.t: better clean-up at end
Scratch.t: no newlines in test output
Session/Scratch.pm: convert to use placeholders
Ad.pm: Add missing line of POD.
Group.pm: add method to get all users in a group by karma.
User.pm: isInGroup to use getKarmaUsers.

lib/WebGUI/AdSpace/Ad.pm

@@ -232,6 +232,8 @@ The number of clicks that have been purchased for this ad.
 
 =head4 impressionsBought
 
+The number of times the user has paid for this ad to be displayed on the site.
+
 =cut
 
 sub set {

lib/WebGUI/Group.pm

@@ -421,19 +421,17 @@ sub expireOffset {
 
 #-------------------------------------------------------------------
 
-=head2 externalUsers ( )
+=head2 getDatabaseUsers ( )
 
-Get the set of users allowed to be in this group via external means, such as database
-queries, LDAP and/or IP filtering.
+Get the set of users allowed to be in this group via a database query.
 
 =cut
 
-sub externalUsers {
+sub getDatabaseUsers {
 	my $self = shift;
+	my @dbUsers = ();
 	my $gid = $self->getId;
-	my @externalUsers = ();
-	my $isInGroup = $self->session->stow->get('isInGroup');
-        ### Check external database
+        ### Check db database
         if ($self->get("dbQuery") && defined $self->get("databaseLinkId")) {
 		my $dbLink = WebGUI::DatabaseLink->new($self->session,$self->get("databaseLinkId"));
 		my $dbh = $dbLink->db;
@@ -446,16 +444,29 @@ sub externalUsers {
 			}
 			else {
 				while(my ($userId)=$sth->array) {
-					push @externalUsers, $userId;
-					$isInGroup->{$userId}{$gid} = 1;
+					push @dbUsers, $userId;
 				}
 			}
 			$sth->finish;
 			$dbLink->disconnect;
                 }
         }
-	$self->session->stow->set("isInGroup",$isInGroup);
-	return \@externalUsers;
+	return \@dbUsers;
+}	
+
+#-------------------------------------------------------------------
+
+=head2 getKarmaUsers ( )
+
+Get the set of users allowed to be in this group via their current karma setting
+and this group's karmaThreshold.  All users with 
+
+=cut
+
+sub getKarmaUsers {
+	my $self = shift;
+	return [] unless $self->session->setting->get('useKarma');
+	return $self->session->db->buildArrayRef('select userId from users where karma >= ?', [$self->karmaThreshold]);
 }	
 
 #-------------------------------------------------------------------
@@ -591,7 +602,6 @@ A boolean value to determine whether the method should return the users directly
 A boolean that if set true will return the users list minus the expired groupings.
 
 =cut
-
 sub getUsers {
 	my $self = shift;
 	my $recursive = shift;
@@ -615,12 +625,19 @@ sub getUsers {
 		}
 		##Have to iterate twice due to the withoutExpired clause.
 		foreach my $groupId (@{ $groups }) {
-			push @externalUsers, @{  WebGUI::Group->new($self->session, $groupId)->externalUsers() };
+			my $extGroup = WebGUI::Group->new($self->session, $groupId);
+			push @externalUsers,
+				@{ $extGroup->getDatabaseUsers() },
+				@{ $extGroup->getKarmaUsers() },
+			;
 		}
 	}
 	$clause .= ")";
 	my @localUsers = $self->session->db->buildArray("select userId from groupings where $clause");
-	push @externalUsers,  @{ $self->externalUsers() };
+	push @externalUsers,
+		@{ $self->getDatabaseUsers() },
+		@{ $self->getKarmaUsers() },
+	;
 	my @users = ( @localUsers, @externalUsers );
 	return \@users;
 }

lib/WebGUI/Session/Scratch.pm

@@ -174,7 +174,7 @@ The current session.
 sub new {
 	my $class = shift;
 	my $session = shift;
-	my $data = $session->db->buildHashRef("select name,value from userSessionScratch where sessionId=".$session->db->quote($session->getId));
+	my $data = $session->db->buildHashRef("select name,value from userSessionScratch where sessionId=?",[$session->getId]);
 	bless {_session=>$session,_sessionId=>$session->getId, _data=>$data}, $class;
 }
 
@@ -215,7 +215,7 @@ sub set {
 	my $value = shift;
 	return undef unless ($name);
 	$self->{_data}{$name} = $value;
-	$self->session->db->write("replace into userSessionScratch (sessionId, name, value) values (".$self->session->db->quoteAndJoin([$self->{_sessionId}, $name, $value]).")");
+	$self->session->db->write("replace into userSessionScratch (sessionId, name, value) values (?,?,?)", [$self->{_sessionId}, $name, $value]);
 }
 
 

lib/WebGUI/User.pm

@@ -299,14 +299,6 @@ sub isInGroup {
                                 return 1;
                         }
                 }
-        }
-        ### Check karma levels.
-        if ($self->session->setting->get("useKarma")) {
-                if ($self->karma >= $group->get("karmaThreshold")) {
-                        $isInGroup->{$uid}{$gid} = 1;
-			$self->session->stow->set("isInGroup",$isInGroup);
-                        return 1;
-                }
         }
 	 ### Check ldap
         if ($group->get("ldapGroup") && $group->get("ldapGroupProperty")) {
@@ -349,9 +341,15 @@ sub isInGroup {
 		   }
 		}
 		
-        if ($group->get("dbQuery") && defined $group->get("databaseLinkId")) {
-		my @externalUsers = @{ $group->externalUsers() } ;
-		foreach my $extUserId ( @externalUsers ) {
+        if (my @dbUsers = @{ $group->getDatabaseUsers() }) {
+		foreach my $extUserId ( @dbUsers ) {
+			$isInGroup->{$extUserId}{$gid} = 1;
+		}
+		$self->session->stow->set("isInGroup",$isInGroup);
+		return 1 if ($isInGroup->{$uid}{$gid});
+	}
+        if (my @karmaUsers = @{ $group->getKarmaUsers() }) {
+		foreach my $extUserId ( @karmaUsers ) {
 			$isInGroup->{$extUserId}{$gid} = 1;
 		}
 		$self->session->stow->set("isInGroup",$isInGroup);
@@ -401,8 +399,8 @@ sub karma {
 	if (defined $amount && defined $source && defined $description) {
 		$self->uncache;
 		$self->{_user}{karma} += $amount;
-		$self->session->db->write("update users set karma=karma+".$self->session->db->quote($amount)." where userId=".$self->session->db->quote($self->userId));
-        	$self->session->db->write("insert into karmaLog values (".$self->session->db->quote($self->userId).",$amount,".$self->session->db->quote($source).",".$self->session->db->quote($description).",".$self->session->datetime->time().")");
+		$self->session->db->write("update users set karma=karma+? where userId=?", [$amount, $self->userId]);
+        	$self->session->db->write("insert into karmaLog values (?,?,?,?,?)",[$self->userId, $amount, $source, $description, $self->session->datetime->time()]);
 	}
         return $self->{_user}{karma};
 }

t/Group.t

@@ -18,7 +18,7 @@ use WebGUI::Utility;
 
 use WebGUI::User;
 use WebGUI::Group;
-use Test::More tests => 78; # increment this value for each test you create
+use Test::More tests => 82; # increment this value for each test you create
 use Test::Deep;
 
 my $session = WebGUI::Test->session;
@@ -186,7 +186,11 @@ $user->delete;
 ##Build a group of users and add them to various groups to test fetching users
 
 my @crowd = map { WebGUI::User->new($session, "new") } 0..7;
-my @mob = map { WebGUI::User->new($session, "new") } 0..2;
+my @mob;
+foreach my $idx (0..2) {
+	$mob[$idx] = WebGUI::User->new($session, "new");
+	$mob[$idx]->username("mob$idx");
+}
 
 my @bUsers = map { $_->userId } @crowd[0,1];
 my @aUsers = map { $_->userId } @crowd[2,3];
@@ -208,6 +212,8 @@ cmp_bag($gA->getUsers(1), [@aUsers, @zUsers, 3], 'users in group A, recursively'
 cmp_bag($gC->getUsers(1), [@cUsers, 3], 'users in group C, recursively');
 cmp_bag($gZ->getUsers(1), [@zUsers, 3], 'users in group Z, recursively');
 
+##Database based user membership in groups
+
 $session->db->dbh->do('DROP TABLE IF EXISTS myUserTable');
 $session->db->dbh->do(q!CREATE TABLE myUserTable (userId varchar(22) binary NOT NULL default '', PRIMARY KEY(userId)) TYPE=InnoDB!);
 
@@ -226,35 +232,80 @@ is( $gY->databaseLinkId, 0, "Group Y's databaseLinkId is set to WebGUI");
 $gY->dbQuery(q!select userId from myUserTable!);
 is( $session->stow->get('isInGroup'), undef, 'setting dbQuery clears cached isInGroup');
 
-$session->user({userId => $mob[0]->userId});
-is( $session->user->userId, $mob[0]->userId, 'mob[0] set to be current user');
-$session->errorHandler->warn('checking mob[0] group membership');
+my @mobIds = map { $_->userId } @mob;
+
+cmp_bag(
+	$gY->getDatabaseUsers(),
+	\@mobIds,
+	'all mob users in list of group Y users from database'
+);
+
 is( $mob[0]->isInGroup($gY->getId), 1, 'mob[0] is in group Y after setting dbQuery');
 is( $mob[0]->isInGroup($gZ->getId), 1, 'mob[0] isInGroup Z');
 
 ok( isIn($mob[0]->userId, @{ $gY->getUsers() }), 'mob[0] in list of group Y users');
 ok( !isIn($mob[0]->userId, @{ $gZ->getUsers() }), 'mob[0] not in list of group Z users');
 
-$session->errorHandler->warn('getUsers checks');
-$session->errorHandler->warn('mob[0] userId = '. $mob[0]->userId);
 ok( isIn($mob[0]->userId, @{ $gZ->getUsers(1) }), 'mob[0] in list of group Z users, recursively');
 
+my $gK = WebGUI::Group->new($session, "new");
+$gK->name('Group K');
+$gC->addGroups([$gK->getId]);
+$gK->karmaThreshold(5);
+
+my @chameleons =  ();
+foreach my $idx (0..3) {
+	$chameleons[$idx] = WebGUI::User->new($session, "new");
+	$chameleons[$idx]->username("chameleon$idx");
+}
+
+foreach my $idx (0..3) {
+	$chameleons[$idx]->karma(5*$idx, 'testCode', 'testable karma, dude');
+}
+
+is_deeply(
+	[ (map { $_->karma() }  @chameleons) ],
+	[0, 5, 10, 15],
+	'karma level checks'
+);
+
+my $defaultKarmaSetting = $session->setting->get('useKarma');
+
+$session->setting->set('useKarma', 0) if $defaultKarmaSetting;
+
+is_deeply(
+	[ (map { $_->isInGroup($gK->getId) }  @chameleons) ],
+	[0, 0, 0, 0],
+	'karma disabled in settings, no users in group'
+);
+
+$session->setting->set('useKarma', 1);
+$session->stow->delete('isInGroup'); ##Clear cache since previous data is wrong
+
+is_deeply(
+	[ (map { $_->isInGroup($gK->getId) }  @chameleons) ],
+	[0, 1, 1, 1],
+	'chameleons 1, 2 and 3 are in group K via karma threshold'
+);
+
+is_deeply(
+	$gK->getKarmaUsers,
+	[ (map { $_->userId() }  @chameleons[1..3]) ],
+	'chameleons 1, 2 and 3 are group K karma users'
+);
+
+$session->setting->set('useKarma', $defaultKarmaSetting);
+
 SKIP: {
 	skip("need to test expiration date in groupings interacting with recursive or not", 1);
 	ok(undef, "expiration date in groupings for getUser");
 }
 
 END {
-	(defined $gX and ref $gX eq 'WebGUI::Group') and $gX->delete;
-	(defined $gY and ref $gY eq 'WebGUI::Group') and $gY->delete;
-	(defined $gZ and ref $gZ eq 'WebGUI::Group') and $gZ->delete;
-	(defined $gA and ref $gA eq 'WebGUI::Group') and $gA->delete;
-	(defined $gB and ref $gB eq 'WebGUI::Group') and $gB->delete;
-	(defined $gC and ref $gC eq 'WebGUI::Group') and $gC->delete;
-	(defined $g2 and ref $g2 eq 'WebGUI::Group') and $g2->delete;
-	(defined $g  and ref $g  eq 'WebGUI::Group') and $g->delete;
-	(defined $user  and ref $g  eq 'WebGUI::User') and $g->delete;
-	foreach my $dude (@crowd, @mob) {
+	foreach my $testGroup ($gX, $gY, $gZ, $gA, $gB, $gC, $g, $gK) {
+		$testGroup->delete if (defined $testGroup and ref $testGroup eq 'WebGUI::Group');
+	}
+	foreach my $dude (@crowd, @mob, @chameleons, $user) {
 		$dude->delete if (defined $dude and ref $dude eq 'WebGUI::User');
 	}
 	$session->db->dbh->do('DROP TABLE IF EXISTS myUserTable');

t/Session/Scratch.t

@@ -31,7 +31,7 @@ for (my $count = 1; $count <= $maxCount; $count++){
 
 
 for (my $count = 1; $count <= $maxCount; $count++){
-   is($scratch->get("Test$count"), $count, "Passed set/get $count\n");
+   is($scratch->get("Test$count"), $count, "Passed set/get $count");
 }
 
 

t/User.t

@@ -289,3 +289,8 @@ WebGUI::Group->new($session, '7')->addUsers([1]);
 
 ok($visitor->isInGroup(1), "Visitor added back to group Visitor");
 ok($visitor->isInGroup(7), "Visitor added back to group Everyone");
+
+END {
+        (defined $user  and ref $user  eq 'WebGUI::User') and $user->delete;
+}
+