Allow certain fields in Ad and AdSpace to be cleared. Add tests

to verify that it works.

docs/changelog/7.x.x.txt

@@ -26,6 +26,7 @@
  - fixed: Template Assets broken
  - fixed: edit operation sql error, Thingy (Yung Han Khoe)
  - fixed: Thingy: default thing property hidden (Yung Han Khoe) 
+ - fixed: Ad Space Description Text Keeps Repopulating
 
 7.5.20
  - fixed: DataForm acknowledgement screen shows incorrect value for Date/Time fields

lib/WebGUI/AdSpace.pm

@@ -333,10 +333,15 @@ The height, in pixels, of this ad space.
 sub set {
 	my $self = shift;
 	my $properties = shift || {};
-	##create requires a name, default will never be used
-	$self->{_properties}{name} = $properties->{name} || $self->{_properties}{name} || "Unnamed";
-	$self->{_properties}{title} = $properties->{title} || $self->{_properties}{title} || "Untitled";
-	$self->{_properties}{description} = $properties->{description} || $self->{_properties}{description};
+
+	##create requires a name, default will never be used.  This prevents the name from being
+    ##erased
+	$self->{_properties}{name}  = $properties->{name} || $self->{_properties}{name}  || "Unnamed";
+
+    ##Allow title and description to be cleared
+	$self->{_properties}{title} = exists $properties->{title} ?  $properties->{title}
+                                : $self->{_properties}{title} || "Untitled";
+	$self->{_properties}{description} = exists $properties->{description} ? $properties->{description} : $self->{_properties}{description};
 	$self->{_properties}{costPerImpression} = exists $properties->{costPerImpression} ? $properties->{costPerImpression} : $self->{_properties}{costPerImpression};
 	$self->{_properties}{costPerClick} = exists $properties->{costPerClick} ? $properties->{costPerClick} : $self->{_properties}{costPerClick};
 	$self->{_properties}{minimumImpressions} = $properties->{minimumImpressions} || $self->{_properties}{minimumImpressions};

lib/WebGUI/AdSpace/Ad.pm

@@ -247,7 +247,8 @@ sub set {
 	$self->{_properties}{title} = $properties->{title} || $self->{_properties}{title} || "Untitled";
 	$self->{_properties}{clicksBought} = $properties->{clicksBought} || $self->{_properties}{clicksBought};
 	$self->{_properties}{impressionsBought} = $properties->{impressionsBought} || $self->{_properties}{impressionsBought};
-	$self->{_properties}{url} = $properties->{url} || $self->{_properties}{url};
+	$self->{_properties}{url}    = exists $properties->{url}    ? $properties->{url}    : $self->{_properties}{url};
+	$self->{_properties}{adText} = exists $properties->{adText} ? $properties->{adText} : $self->{_properties}{adText};
 	$self->{_properties}{adText} = $properties->{adText} || $self->{_properties}{adText};
 	$self->{_properties}{storageId} = $properties->{storageId} || $self->{_properties}{storageId};
 	$self->{_properties}{richMedia} = $properties->{richMedia} || $self->{_properties}{richMedia};

t/AdSpace.t

@@ -31,7 +31,7 @@ my $newAdSpaceSettings = {
     height             => "300",
 };
 
-my $numTests = 32; # increment this value for each test you create
+my $numTests = 35; # increment this value for each test you create
 $numTests += 2 * scalar keys %{ $newAdSpaceSettings };
 ++$numTests; ##For conditional testing on module load
 
@@ -105,6 +105,11 @@ SKIP: {
             sprintf "empty call to set does not change %s", $setting);
     }
 
+    $catWoman->set({title => '', name => '', description => '', });
+    is ($catWoman->get('title'),       '', 'set can clear the title');
+    is ($catWoman->get('description'), '', 'set can clear the title');
+    is ($catWoman->get('name' ), $newAdSpaceSettings->{'name'}, 'set can not clear the name');
+
     ##Create a set of ads for general purpose testing
 
     ##The Joker and Penguin Ads go in the bruce adSpace

t/AdSpace/Ad.t

@@ -36,7 +36,7 @@ my $newAdSettings = {
     priority          => "0",
 };
 
-my $numTests = 29; # increment this value for each test you create
+my $numTests = 33; # increment this value for each test you create
 $numTests += scalar keys %{ $newAdSettings };
 ++$numTests; ##For conditional testing on module load
 
@@ -66,11 +66,11 @@ SKIP: {
 
     undef $ad2;
 
-	my $data = $session->db->quickHashRef("select adId, adSpaceId from advertisement where adId=?",[$ad->getId]);
+    my $data = $session->db->quickHashRef("select adId, adSpaceId from advertisement where adId=?",[$ad->getId]);
 
-	ok(exists $data->{adId}, "create()");
-	is($data->{adId}, $ad->getId, "getId()");
-	is($data->{adSpaceId}, $ad->get('adSpaceId'), "get() adSpaceId");
+    ok(exists $data->{adId}, "create()");
+    is($data->{adId}, $ad->getId, "getId()");
+    is($data->{adSpaceId}, $ad->get('adSpaceId'), "get() adSpaceId");
 
     foreach my $setting (keys %{ $newAdSettings } ) {
         is($newAdSettings->{$setting}, $ad->get($setting),
@@ -79,28 +79,28 @@ SKIP: {
 
     $richAd = WebGUI::AdSpace::Ad->create($session, $adSpace->getId);
     $richAd->set({
-    	type      => 'rich',
-	richMedia => 'This is rich, ^@;'
+        type      => 'rich',
+        richMedia => 'This is rich, ^@;'
     });
     my $renderedAd = $richAd->get('renderedAd');
     my $userName = $session->user->username;
     like($renderedAd, qr/This is rich, $userName/, 'Rich media ads render macros');
 
     ##In this series of tests, we'll render a text ad and then pick it apart and make
-    ##sure that all th requisite components are in there.
+    ##sure that all the requisite components are in there.
     $adSpace->set({
-	width => 102,
-	height => 202
+        width => 102,
+        height => 202
     });
 
     $textAd = WebGUI::AdSpace::Ad->create($session, $adSpace->getId);
     $textAd->set({
-    	type        => 'text',
-	borderColor => 'black',
-	backgroundColor => 'white',
-	textColor => 'blue',
-	title => 'This is a text ad',
-	adText => 'Will hack for Gooey dolls.',
+        type        => 'text',
+        borderColor => 'black',
+        backgroundColor => 'white',
+        textColor => 'blue',
+        title => 'This is a text ad',
+        adText => 'Will hack for Gooey dolls.',
     });
     my $renderedTextAd = $textAd->get('renderedAd');
     
@@ -137,8 +137,8 @@ SKIP: {
 
     ##Ditto for the image ad
     $adSpace->set({
-	width  => 250,
-	height => 250
+        width  => 250,
+        height => 250
     });
 
     $imageAd = WebGUI::AdSpace::Ad->create($session, $adSpace->getId);
@@ -196,18 +196,26 @@ SKIP: {
     $setAd->set({priority=>0});
     is($setAd->get('priority'), 0, 'set priority=0');
 
+    $setAd->set({ title => 'myTitle', url => 'http://www.nowhere.com', adText => 'Performing a valuable service for the community'});
+    is($setAd->get('url'),    'http://www.nowhere.com',                          'set: url');
+    is($setAd->get('adText'), 'Performing a valuable service for the community', 'set: adText');
+
+    $setAd->set({ url => '', adText => ''});
+    is($setAd->get('url'),    '', 'set: clearing url');
+    is($setAd->get('adText'), '', 'set: clearing adText');
+
 }
 
 END {
-	foreach my $advertisement ($ad, $richAd, $textAd, $imageAd, $nonAd, $setAd) {
-		if (defined $advertisement and ref $advertisement eq 'WebGUI::AdSpace::Ad') {
-			$advertisement->delete;
-		}
-	}
-	if (defined $adSpace and ref $adSpace eq 'WebGUI::AdSpace') {
-		$adSpace->delete;
-	}
-	if (defined $imageStorage and ref $imageStorage eq 'WebGUI::Storage::Image') {
-		$imageStorage->delete;
-	}
+        foreach my $advertisement ($ad, $richAd, $textAd, $imageAd, $nonAd, $setAd) {
+                if (defined $advertisement and ref $advertisement eq 'WebGUI::AdSpace::Ad') {
+                        $advertisement->delete;
+                }
+        }
+        if (defined $adSpace and ref $adSpace eq 'WebGUI::AdSpace') {
+                $adSpace->delete;
+        }
+        if (defined $imageStorage and ref $imageStorage eq 'WebGUI::Storage::Image') {
+                $imageStorage->delete;
+        }
 }