BUG Fixed parsing of spaces and other whitespace in tag clouds. Fixes #59

BUG Fixed incorrect encoding of SelectedAuthor and SelectedTag; Now correctly cast for templates using the `cast` config, not within filtering.
BUG Fixed TagCloudWidget.popularities config from being incorrectly accessed as a static property
BUG Fixed TagCloudWidget::getCMSFields triggering extend('updateCMSFields') twice
BUG Fixed TagCloudWidget::getTagsCollection discarding tag label capitalisation
BUG Fixed TagCloudWidget::getTagsCollection not correctly respecting minimum tag counts (as well as maximum tag counts) when determining the popularity CSS class to assign.
Test cases for TagCloudWidget
API BlogEntry::TagNames now safely extracts tags from a blog entry as an associative 'lowercase' => 'Entered Tag' format
PHPDoc fixes
Removed trailing '?>' tags from PHP files

code/BlogEntry.php

@@ -87,25 +87,40 @@ class BlogEntry extends Page {
 	}
 	
 	/**
-	 * Returns the tags added to this blog entry
+	 * Safely split and parse all distinct tags assigned to this BlogEntry
+	 * 
+	 * @return array Associative array of lowercase tag to native case tags
 	 */
-	function TagsCollection() {
+	public function TagNames() {
+		$tags = preg_split("/\s*,\s*/", trim($this->Tags));
+		$results = array();
+		foreach($tags as $tag) {
+			if($tag) $results[mb_strtolower($tag)] = $tag;
+		}
+		return $results;
+	}
+	
+	/**
+	 * Returns the tags added to this blog entry
+	 * 
+	 * @return ArrayList List of ArrayData with Tag, Link, and URLTag keys
+	 */
+	public function TagsCollection() {
 
-		$tags = preg_split(" *, *", trim($this->Tags));
+		$tags = $this->TagNames();
 		$output = new ArrayList();
 		
-		$link = $this->getParent() ? $this->getParent()->Link('tag') : '';
+		$link = ($parent = $this->getParent()) ? $parent->Link('tag') : '';
-		foreach($tags as $tag) {
+		foreach($tags as $tag => $tagLabel) {
+			$urlKey = urlencode($tag);
 			$output->push(new ArrayData(array(
-				'Tag' => Convert::raw2xml($tag),
+				'Tag' => $tagLabel,
-				'Link' => $link . '/' . urlencode($tag),
+				'Link' => Controller::join_links($link, $urlKey),
-				'URLTag' => urlencode($tag)
+				'URLTag' => $urlKey
 			)));
 		}
 		
-		if($this->Tags) {
+		return $output;
-			return $output;
-		}
 	}
 
 	function Content() {	

code/BlogTree.php

@@ -46,6 +46,7 @@ class BlogTree extends Page {
 	 *
 	 * @param $page allows you to force a specific page, otherwise,
 	 * 				uses current
+	 * @return BlogTree
 	 */
 	static function current($page = null) {
 		
@@ -147,12 +148,14 @@ class BlogTree extends Page {
 		
 	/**
 	 * Get entries in this blog.
-	 * @param string limit A clause to insert into the limit clause.
+	 * 
-	 * @param string tag Only get blog entries with this tag
+	 * @param string $limit A clause to insert into the limit clause.
-	 * @param string date Only get blog entries on this date - either a year, or a year-month eg '2008' or '2008-02'
+	 * @param string $tag Only get blog entries with this tag
-	 * @param callback retrieveCallback A function to call with pagetype, filter and limit for custom blog sorting or filtering
+	 * @param string $date Only get blog entries on this date - either a year, or a year-month eg '2008' or '2008-02'
-	 * @param string $where
+	 * @param callable $retrieveCallback A function to call with pagetype, filter and limit for custom blog
-	 * @return DataObjectSet
+	 * sorting or filtering
+	 * @param string $filter Filter condition
+	 * @return PaginatedList The list of entries in a paginated list
 	 */
 	public function Entries($limit = '', $tag = '', $date = '', $retrieveCallback = null, $filter = '') {
 		
@@ -229,6 +232,11 @@ class BlogTree_Controller extends Page_Controller {
 		'date'
 	);
 	
+	private static $casting = array(
+		'SelectedTag' => 'Text',
+		'SelectedAuthor' => 'Text'
+	);
+	
 	function init() {
 		parent::init();
 		
@@ -237,7 +245,13 @@ class BlogTree_Controller extends Page_Controller {
 		Requirements::themedCSS("blog","blog");
 	}
 
-	function BlogEntries($limit = null) {
+	/**
+	 * Determine selected BlogEntry items to show on this page
+	 * 
+	 * @param int $limit
+	 * @return PaginatedList
+	 */
+	public function BlogEntries($limit = null) {
 		require_once('Zend/Date.php');
 		
 		if($limit === null) $limit = BlogTree::$default_entries_limit;
@@ -275,14 +289,14 @@ class BlogTree_Controller extends Page_Controller {
 	/**
 	 * This will create a <link> tag point to the RSS feed
 	 */
-	function IncludeBlogRSS() {
+	public function IncludeBlogRSS() {
 		RSSFeed::linkToFeed($this->Link('rss'), _t('BlogHolder.RSSFEED',"RSS feed of these blogs"));
 	}
 	
 	/**
 	 * Get the rss feed for this blog holder's entries
 	 */
-	function rss() {
+	public function rss() {
 		global $project_name;
 
 		$blogName = $this->Title;
@@ -299,7 +313,7 @@ class BlogTree_Controller extends Page_Controller {
 	/**
 	 * Protection against infinite loops when an RSS widget pointing to this page is added to this page
 	 */
-	function defaultAction($action) {
+	public function defaultAction($action) {
 		if(stristr($_SERVER['HTTP_USER_AGENT'], 'SimplePie')) return $this->rss();
 		
 		return parent::defaultAction($action);
@@ -308,23 +322,22 @@ class BlogTree_Controller extends Page_Controller {
 	/**
 	 * Return the currently viewing tag used in the template as $Tag 
 	 *
-	 * @return String
+	 * @return string
 	 */
-	function SelectedTag() {
+	public function SelectedTag() {
 		if ($this->request->latestParam('Action') == 'tag') {
 			$tag = $this->request->latestParam('ID');
-			$tag = urldecode($tag);
+			return urldecode($tag);
-			return Convert::raw2xml($tag);
+		}
-	}
 		return '';
 	}
 	
 	/**
 	 * Return the selected date from the blog tree
 	 *
-	 * @return Date
+	 * @return string
 	 */
-	function SelectedDate() {
+	public function SelectedDate() {
 		if($this->request->latestParam('Action') == 'date') {
 			$year = $this->request->latestParam('ID');
 			$month = $this->request->latestParam('OtherID');
@@ -344,21 +357,23 @@ class BlogTree_Controller extends Page_Controller {
 	}
 
 	/**
-	 * @return String
+	 * @return string
 	 */
-	function SelectedAuthor() {
+	public function SelectedAuthor() {
 		if($this->request->getVar('author')) {
 			$hasAuthor = BlogEntry::get()->filter('Author', $this->request->getVar('author'))->Count();
-			return $hasAuthor ? Convert::raw2xml($this->request->getVar('author')) : null;
+			return $hasAuthor
+				? $this->request->getVar('author')
+				: null;
 		} elseif($this->request->getVar('authorID')) {
 			$hasAuthor = BlogEntry::get()->filter('AuthorID', $this->request->getVar('authorID'))->Count();
 			if($hasAuthor) {
 				$member = Member::get()->byId($this->request->getVar('authorID'));
 				if($member) {
 					if($member->hasMethod('BlogAuthorTitle')) {
-						return Convert::raw2xml($member->BlogAuthorTitle);
+						return $member->BlogAuthorTitle;
 					} else {
-						return Convert::raw2xml($member->Title);
+						return $member->Title;
 					}
 				} else {
 					return null;
@@ -367,7 +382,11 @@ class BlogTree_Controller extends Page_Controller {
 		}
 	}
 	
-	function SelectedNiceDate(){
+	/**
+	 * 
+	 * @return string
+	 */
+	public function SelectedNiceDate(){
 		$date = $this->SelectedDate();
 		
 		if(strpos($date, '-')) {

code/widgets/TagCloudWidget.php

@@ -22,127 +22,134 @@ if(class_exists('Widget')) {
 		);
 
 		private static $cmsTitle = "Tag Cloud";
+		
 		private static $description = "Shows a tag cloud of tags on your blog.";
 
-		private static $popularities = array( 'not-popular', 'not-very-popular', 'somewhat-popular', 'popular', 'very-popular', 'ultra-popular' );
+		/**
+		 * List of popularity classes in order of least to most popular
+		 *
+		 * @config
+		 * @var array
+		 */
+		private static $popularities = array(
+			'not-popular',
+			'not-very-popular',
+			'somewhat-popular',
+			'popular',
+			'very-popular',
+			'ultra-popular'
+		);
 
-		function getCMSFields() {
+		public function getCMSFields() {
-			$fields = parent::getCMSFields(); 
+			
+			$this->beforeUpdateCMSFields(function($fields) {
+				$fields->merge(
+					new FieldList(
+						new TextField("Title", _t("TagCloudWidget.TILE", "Title")),
+						new TextField("Limit", _t("TagCloudWidget.LIMIT", "Limit number of tags")),
+						new OptionsetField(
+							"Sortby",
+							_t("TagCloudWidget.SORTBY", "Sort by"),
+							array(
+								"alphabet" => _t("TagCloudWidget.SBAL", "alphabet"),
+								"frequency" => _t("TagCloudWidget.SBFREQ", "frequency")
+							)
+						)
+					)
+				);
+			});
 
-			$fields->merge(
+			return parent::getCMSFields();
-
-				new FieldList(
-					new TextField("Title", _t("TagCloudWidget.TILE", "Title")),
-					new TextField("Limit", _t("TagCloudWidget.LIMIT", "Limit number of tags")),
-					new OptionsetField("Sortby",_t("TagCloudWidget.SORTBY","Sort by"),array("alphabet"=>_t("TagCloudWidget.SBAL", "alphabet"),"frequency"=>_t("TagCloudWidget.SBFREQ", "frequency")))
-				)
-			);
-
-			$this->extend('updateCMSFields', $fields);
-
-			return $fields;
 		}
 
 		function Title() {
 			return $this->Title ? $this->Title : _t('TagCloudWidget.DEFAULTTITLE', 'Tag Cloud');
 		}
+		
+		/**
+		 * Current BlogTree used as the container for this tagcloud.
+		 * Used by {@link TagCloudWidgetTest} for testing
+		 * 
+		 * @var BlogTree
+		 */
+		public static $container = null;
 
+		/**
+		 * Return all sorted tags in the system
+		 * 
+		 * @return ArrayList
+		 */
 		function getTagsCollection() {
 			Requirements::themedCSS("tagcloud");
 
-			$allTags = array();
+			// Ensure there is a valid BlogTree with entries
-			$max = 0;
+			$container = BlogTree::current(self::$container);
-			$container = BlogTree::current();
+			if(	!$container
+				|| !($entries = $container->Entries())
+				|| $entries->count() == 0
+			) return null;
 
-			$entries = $container->Entries();
+			// Extract all tags from each entry
-
+			$tagCounts = array(); // Mapping of tag => frequency
-			if($entries) {
+			$tagLabels = array(); // Mapping of tag => label
-				foreach($entries as $entry) {
+			foreach($entries as $entry) {
-					$theseTags = preg_split(" *, *", mb_strtolower(trim($entry->Tags)));
+				$theseTags = $entry->TagNames();
-					foreach($theseTags as $tag) {
+				foreach($theseTags as $tag => $tagLabel) {
-						if($tag != "") {
+					$tagLabels[$tag] = $tagLabel;
-							$allTags[$tag] = isset($allTags[$tag]) ? $allTags[$tag] + 1 : 1; //getting the count into key => value map
+					//getting the count into key => value map
-							$max = ($allTags[$tag] > $max) ? $allTags[$tag] : $max;
+					$tagCounts[$tag] = isset($tagCounts[$tag]) ? $tagCounts[$tag] + 1 : 1;
-						}
-					}
 				}
+			}
+			if(empty($tagCounts)) return null;
+			$minCount = min($tagCounts);
+			$maxCount = max($tagCounts);
 
-				if($allTags) {		
+			// Apply sorting mechanism
-					//TODO: move some or all of the sorts to the database for more efficiency
+			if($this->Sortby == "alphabet") {
-					if($this->Limit > 0) $allTags = array_slice($allTags, 0, $this->Limit, true);
+				// Sort by name
+				ksort($tagCounts);
+			} else {
+				 // Sort by frequency
+				uasort($tagCounts, function($a, $b) {
+					return $b - $a;
+				});
+			}
+			
+			// Apply limiting
+			if($this->Limit > 0) $tagCounts = array_slice($tagCounts, 0, $this->Limit, true);
 
-					if($this->Sortby == "alphabet"){
+			// Calculate buckets of popularities
-						$this->natksort($allTags);
+			$numsizes = count(array_unique($tagCounts)); //Work out the number of different sizes
-					} else{
+			$popularities = self::config()->popularities;
-						uasort($allTags, array($this, "column_sort_by_popularity")); // sort by frequency
+			$buckets = count($popularities);
-					}
 
-					$sizes = array();	
+			// If there are more frequencies than buckets, divide frequencies into buckets
-					foreach ($allTags as $tag => $count) $sizes[$count] = true;
+			if ($numsizes > $buckets) $numsizes = $buckets;
+			
+			// Adjust offset to use central buckets (if using a subset of available buckets)
+			$offset = round(($buckets - $numsizes)/2);
 
-					$offset = 0; 
+			$output = new ArrayList();
-					$numsizes = count($sizes)-1; //Work out the number of different sizes
+			foreach($tagCounts as $tag => $count) {
-					$buckets = count(self::$popularities)-1;
+				
-
+				// Find position of $count in the selected range, adjusted for bucket range used
-					// If there are more frequencies than buckets, divide frequencies into buckets
+				if($maxCount == $minCount) {
-					if ($numsizes > $buckets) {
+					$popularities = $offset;
-						$numsizes = $buckets;
+				} else {
-					}
+					$popularity = round(
-					// Otherwise center use central buckets
+						($count-$minCount) / ($maxCount-$minCount) * ($numsizes-1)
-					else {
+					) + $offset;
-						$offset   = round(($buckets-$numsizes)/2);
-					}
-
-					foreach($allTags as $tag => $count) {
-						$popularity = round($count / $max * $numsizes) + $offset; $popularity=min($buckets,$popularity);
-						$class = self::$popularities[$popularity];
-
-						$allTags[$tag] = array(
-							"Tag" => $tag,
-							"Count" => $count,
-							"Class" => $class,
-							"Link" => $container->Link('tag') . '/' . urlencode($tag)		
-						);
-					}
 				}
+				$class = $popularities[$popularity];
 
-				$output = new ArrayList();
+				$output->push(new ArrayData(array(
-				foreach($allTags as $tag => $fields) {
+					"Tag" => $tagLabels[$tag],
-					$output->push(new ArrayData($fields));
+					"Count" => $count,
-				}
+					"Class" => $class,
-
+					"Link" => Controller::join_links($container->Link('tag'), urlencode($tag))
-				return $output;	
+				)));
 			}
-
+			return $output;
-			return;		
-		}
-
-		/**
-		 * Helper method to compare 2 Vars to work out the results.
-		 * @param mixed
-		 * @param mixed
-		 * @return int
-		 */
-		private function column_sort_by_popularity($a, $b){
-			if($a == $b) {
-				$result  = 0;
-			} 
-			else {
-				$result = $b - $a;
-			}
-			return $result;
-		}
-
-		private function natksort(&$aToBeSorted) {
-			$aResult = array();
-			$aKeys = array_keys($aToBeSorted);
-			natcasesort($aKeys);
-			foreach ($aKeys as $sKey) {
-				$aResult[$sKey] = $aToBeSorted[$sKey];
-			}
-			$aToBeSorted = $aResult;
-
-			return true;
 		}
 	}
 }

templates/TagCloudWidget.ss

@@ -1,5 +1,5 @@
 <p class="tagcloud">
 	<% loop TagsCollection %>
-		<a href="$Link" class="$Class">$Tag</a> 
+		<a href="$Link" class="$Class">$Tag.XML</a> 
 	<% end_loop %>
 </p>

tests/BlogEntryTest.php

@@ -1,4 +1,5 @@
 <?php
+
 /**
  * @package blog
  * @subpackage tests
@@ -6,7 +7,10 @@
 class BlogEntryTest extends SapphireTest {
 	static $fixture_file = 'blog/tests/BlogTest.yml';
 	
-	function testBBCodeContent() {
+	/**
+	 * Tests BBCode functionality
+	 */
+	public function testBBCodeContent() {
 		$tmpFlag = BlogEntry::$allow_wysiwyg_editing; 
 		BlogEntry::$allow_wysiwyg_editing = false; 
 		
@@ -16,8 +20,11 @@ class BlogEntryTest extends SapphireTest {
 		$this->assertEquals('<p><a href="admin">the CMS</a></p>', $entry->Content()->value);
 		BlogEntry::$allow_wysiwyg_editing = $tmpFlag; 
 	}
-	
+
-	function testContent() {
+	/**
+	 * Tests BlogEntry::Content method
+	 */
+	public function testContent() {
 		$tmpFlag = BlogEntry::$allow_wysiwyg_editing; 
 		BlogEntry::$allow_wysiwyg_editing = true; 
 		
@@ -28,4 +35,24 @@ class BlogEntryTest extends SapphireTest {
 		BlogEntry::$allow_wysiwyg_editing = $tmpFlag; 
 	}
 	
-}
+	/**
+	 * Tests TagCollection parsing of tags
+	 */
+	public function testTagging() {
+		$entry = new BlogEntry();
+		$entry->Tags = 'damian,Bob, andrew , multiple words, thing,tag,item , Andrew';
+		$tags = $entry->TagNames();
+		ksort($tags);
+		
+		$this->assertEquals(array(
+			'andrew' => 'Andrew',
+			'bob' => 'Bob',
+			'damian' => 'damian',
+			'item' => 'item',
+			'multiple words' => 'multiple words',
+			'tag' => 'tag',
+			'thing' => 'thing'
+		), $tags);
+	}
+	
+}

tests/BlogHolderTest.php

@@ -73,5 +73,3 @@ class BlogHolderTest extends SapphireTest {
 	}
 
 }
-
-?>

tests/BlogTreeTest.php

@@ -104,5 +104,3 @@ class BlogTreeTest extends SapphireTest {
 		
 	}
 }
-
-?>

tests/TagCloudWidgetTest.php

@@ -0,0 +1,95 @@
+<?php
+
+class TagCloudWidgetTest extends SapphireTest {
+	
+	public function setUp() {
+		parent::setUp();
+		
+		if(!class_exists('Widget')) return;
+		
+		// holder
+		$holder = new BlogHolder();
+		$holder->Title = 'Holder';
+		$holder->write();
+		TagCloudWidget::$container = $holder;
+		
+		// Save all pages
+		$page = new BlogEntry();
+		$page->Tags = 'Ultra, Very, Popular, Somewhat, NotVery, NotPopular';
+		$page->ParentID = $holder->ID;
+		$page->write();
+		$page = new BlogEntry();
+		$page->Tags = 'Ultra, Very, Popular, Somewhat, NotVery';
+		$page->ParentID = $holder->ID;
+		$page->write();
+		$page = new BlogEntry();
+		$page->Tags = 'Ultra, Very, Popular, Somewhat';
+		$page->ParentID = $holder->ID;
+		$page->write();
+		$page = new BlogEntry();
+		$page->Tags = 'Ultra, Very, Popular';
+		$page->ParentID = $holder->ID;
+		$page->write();
+		$page = new BlogEntry();
+		$page->Tags = 'Ultra, Very, Popular';
+		$page->ParentID = $holder->ID;
+		$page->write();
+		$page = new BlogEntry();
+		$page->Tags = 'Ultra, Very';
+		$page->ParentID = $holder->ID;
+		$page->write();
+		$page = new BlogEntry();
+		$page->Tags = 'Ultra';
+		$page->ParentID = $holder->ID;
+		$page->write();
+		$page = new BlogEntry();
+		$page->Tags = '';
+		$page->ParentID = $holder->ID;
+		$page->write();
+	}
+	
+	public function tearDown() {
+		parent::tearDown();
+		
+		if(!class_exists('Widget')) return;
+		
+		TagCloudWidget::$container = null;
+	}
+	
+	/**
+	 * Test that tags are correctly extracted from a blog tree
+	 */
+	public function testGetTags() {
+		
+		if(!class_exists('Widget')) $this->markTestSkipped('This test requires the Widget module');
+		
+		// Test sorting by alphabetic
+		$widget = new TagCloudWidget();
+		$widget->Sortby = 'alphabet';
+		$tags = $widget->getTagsCollection()->toNestedArray();
+		$this->assertEquals($tags[0]['Tag'], 'NotPopular');
+		$this->assertEquals($tags[0]['Class'], 'not-popular');
+		$this->assertEquals($tags[0]['Count'], 1);
+		$this->assertEquals($tags[3]['Tag'], 'Somewhat');
+		$this->assertEquals($tags[3]['Class'], 'somewhat-popular');
+		$this->assertEquals($tags[3]['Count'], 3);
+		$this->assertEquals($tags[5]['Tag'], 'Very');
+		$this->assertEquals($tags[5]['Class'], 'very-popular');
+		$this->assertEquals($tags[5]['Count'], 6);
+		
+		// Test sorting by frequency
+		$widget = new TagCloudWidget();
+		$widget->Sortby = 'frequency';
+		$tags = $widget->getTagsCollection()->toNestedArray();
+		$this->assertEquals($tags[0]['Tag'], 'Ultra');
+		$this->assertEquals($tags[0]['Class'], 'ultra-popular');
+		$this->assertEquals($tags[0]['Count'], 7);
+		$this->assertEquals($tags[3]['Tag'], 'Somewhat');
+		$this->assertEquals($tags[3]['Class'], 'somewhat-popular');
+		$this->assertEquals($tags[3]['Count'], 3);
+		$this->assertEquals($tags[5]['Tag'], 'NotPopular');
+		$this->assertEquals($tags[5]['Class'], 'not-popular');
+		$this->assertEquals($tags[5]['Count'], 1);
+	}
+
+}