From 906775d505d3c5b24949144e489cfe51310c0bc7 Mon Sep 17 00:00:00 2001
From: Colin Kuskie <colink@perldreamer.com>
Date: Fri, 23 Feb 2007 04:59:06 +0000
Subject: [PATCH] AdSpace has 100% coverage, aside from some conditionals in
 the set method. Found and fixed an off by 1 error in displayImpressions that
 allowed boughtImpressions+1 impressions to be displayed.

---
 docs/changelog/7.x.x.txt |  1 +
 lib/WebGUI/AdSpace.pm    |  3 ++-
 t/AdSpace.t              | 31 +++++++++++++++++++++++++------
 3 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt
index cc6270e82..6675c1607 100644
--- a/docs/changelog/7.x.x.txt
+++ b/docs/changelog/7.x.x.txt
@@ -1,4 +1,5 @@
 7.3.11
+ - fix: Off by 1 error while displaying ad impressions (perlDreamer Consulting, LLC)
 
 7.3.10
  - fix: Calendar Update Feeds now handles iCalendar "DURATION" field
diff --git a/lib/WebGUI/AdSpace.pm b/lib/WebGUI/AdSpace.pm
index b4d58227a..4ba842bc1 100644
--- a/lib/WebGUI/AdSpace.pm
+++ b/lib/WebGUI/AdSpace.pm
@@ -134,7 +134,8 @@ sub displayImpression {
 	my ($id, $ad, $priority, $clicks, $clicksBought, $impressions, $impressionsBought) = $self->session->db->quickArray("select adId, renderedAd, priority, clicks, clicksBought, impressions, impressionsBought from advertisement where adSpaceId=? and isActive=1 order by nextInPriority asc limit 1",[$self->getId]);
 	unless ($dontCount) {
 		my $isActive = 1;
-		if ($clicks >= $clicksBought && $impressions >= $impressionsBought) {
+		if ($clicks >= $clicksBought && $impressions >= ($impressionsBought-1)) {
+		#if ($clicks >= $clicksBought && $impressions >= $impressionsBought) {
 			$isActive = 0;
 		}
 		$self->session->db->write("update advertisement set impressions=impressions+1, nextInPriority=?, isActive=? where adId=?", 
diff --git a/t/AdSpace.t b/t/AdSpace.t
index acee1a758..9a97d77d3 100644
--- a/t/AdSpace.t
+++ b/t/AdSpace.t
@@ -31,7 +31,7 @@ my $newAdSpaceSettings = {
     height             => "300",
 };
 
-my $numTests = 26; # increment this value for each test you create
+my $numTests = 31; # increment this value for each test you create
 $numTests += 2 * scalar keys %{ $newAdSpaceSettings };
 ++$numTests; ##For conditional testing on module load
 
@@ -40,7 +40,7 @@ plan tests => $numTests;
 my $loaded = use_ok('WebGUI::AdSpace');
 
 my $session = WebGUI::Test->session;
-my ($adSpace, $alfred, $alfred2, $bruce, $catWoman, $twoFaceClone, $defaultAdSpace );
+my ($adSpace, $alfred, $alfred2, $bruce, $catWoman, $villianClone, $defaultAdSpace );
 my ($jokerAd, $penguinAd, $twoFaceAd);
 
 SKIP: {
@@ -118,6 +118,8 @@ SKIP: {
             richMedia  => 'Joker',
             priority   => 2,
             isActive   => 1,
+            clicksBought      => 0,
+            impressionsBought => 2,
         }
     );
     $penguinAd = WebGUI::AdSpace::Ad->create($session, $bruce->getId,
@@ -126,8 +128,10 @@ SKIP: {
             url        => '/fishy',
             type       => 'rich',
             richMedia  => 'Penguin',
-            priority   => 3,
+            priority   => 1,
             isActive   => 1,
+            clicksBought      => 4,
+            impressionsBought => 0,
         }
     );
     $twoFaceAd = WebGUI::AdSpace::Ad->create($session, $catWoman->getId,
@@ -196,9 +200,24 @@ SKIP: {
         'displayImpression set the nextInPriority correctly'
     );
 
-    $twoFaceClone = WebGUI::AdSpace::Ad->new($session, $twoFaceAd->getId);
-    is($twoFaceClone->get('isActive'), 0, 'displayImpression deactivates an ad if enough impressions and clicks are bought');
+    my ($twoFaceIsActive) = $session->db->quickArray('select isActive from advertisement where adId=?',[$twoFaceAd->getId]);
+    is($twoFaceIsActive, 0, 'displayImpression deactivates an ad if enough impressions and clicks are bought');
 
+    $session->db->write('update advertisement set nextInPriority=UNIX_TIMESTAMP()+100000 where adId=?',[$jokerAd->getId]);
+    is($bruce->displayImpression(), $penguinAd->get('renderedAd'), 'displayImpression returns earliest by nextInPriority, penguin has 3 clicks');
+    WebGUI::AdSpace->countClick($session, $penguinAd->getId); ##4 clicks
+    is($bruce->displayImpression(), $penguinAd->get('renderedAd'), 'displayImpression returns still returns penguinAd, but deactivates it after 4 clicks');
+
+    my ($penguinActive) = $session->db->quickArray('select isActive from advertisement where adId=?',[$penguinAd->getId]);
+    is($penguinActive, 0, 'displayImpression deactiveated penguinAd');
+    is($bruce->displayImpression(), $jokerAd->get('renderedAd'), 'displayImpression now returns jokerAd');
+
+    my ($jokerActive) = $session->db->quickArray('select isActive from advertisement where adId=?',[$jokerAd->getId]);
+    is($jokerActive, 1, 'displayImpression did not deactiveate jokerAd after one impression');
+
+    $bruce->displayImpression();
+    ($jokerActive) = $session->db->quickArray('select isActive from advertisement where adId=?',[$jokerAd->getId]);
+    is($jokerActive, 0, 'displayImpression deactivated jokerAd after two impressions');
 }
 
 END {
@@ -208,7 +227,7 @@ END {
         }
     }
 
-    foreach my $advert ($jokerAd, $penguinAd, $twoFaceClone, $twoFaceAd) {
+    foreach my $advert ($jokerAd, $penguinAd, $villianClone, $twoFaceAd) {
         if (defined $advert and ref $advert eq 'WebGUI::AdSpace::Ad') {
             $advert->delete;
         }