ENHANCEMENT: added tests for CMSPageHistoryController::VersionsForm(). BUGFIX: fixed VersionsForm hidden ID storing a reference to itself. ENHANCEMENT: changed url structure from /version/ to /show/ for consistency between CMSPageHistoryController and CMSMain. APICHANGE: moved performRollback() from CMSMain to CMSPageHistoryController

code/controllers/CMSMain.php

@@ -937,14 +937,6 @@ JS;
 		return $form->forTemplate();
 	}
 
-	function performRollback($id, $version) {
-		$record = DataObject::get_by_id($this->stat('tree_class'), $id);
-		if($record && !$record->canEdit()) return Security::permissionFailure($this);
-		
-		$record->doRollbackTo($version);
-		return $record;
-	}
-
 	function sendFormToBrowser($templateData) {
 		if(Director::is_ajax()) {
 			SSViewer::setOption('rewriteHashlinks', false);

code/controllers/CMSPageHistoryController.php

@@ -13,7 +13,6 @@ class CMSPageHistoryController extends CMSMain {
 	
 	static $allowed_actions = array(
 		'VersionsForm',
-		'version',
 		'compare'
 	);
 	
@@ -24,7 +23,7 @@ class CMSPageHistoryController extends CMSMain {
 	/**
 	 * @return array
 	 */
-	function version() {
+	function show() {
 		return array(
 			'EditForm' => $this->ShowVersionForm(
 				$this->request->param('VersionID')
@@ -43,6 +42,16 @@ class CMSPageHistoryController extends CMSMain {
 			)
 		);
 	}
+	
+	/**
+	 * @return array
+	 */
+	function rollback() {
+		return $this->doRollback(array(
+			'ID' => $this->currentPageID(),
+			'Version' => $this->request->param('VersionID')
+		), null);
+	}
 
 	/**
 	 * Returns the read only version of the edit form. Detaches all {@link FormAction} 
@@ -81,14 +90,14 @@ class CMSPageHistoryController extends CMSMain {
 			$field->reserveNL = true;
 		}
 		
-		$link = Controller::join_links(
-			$this->Link('version'),
-			$id
-		);
-		
-		$view = _t('CMSPageHistoryController.VIEW',"view");
-		
 		if($compareID) {
+			$link = Controller::join_links(
+				$this->Link('version'),
+				$id
+			);
+
+			$view = _t('CMSPageHistoryController.VIEW',"view");
+			
 			$message = sprintf(
 				_t('CMSPageHistoryController.COMPARINGVERSION',"Comparing versions %s and %s."),
 				sprintf('%s (<a href="%s">%s</a>)', $versionID, Controller::join_links($link, $versionID), $view),
@@ -99,12 +108,7 @@ class CMSPageHistoryController extends CMSMain {
 		}
 		else {
 			$message = sprintf(
-				_t('CMSPageHistoryController.VIEWINGVERSION',"Currently viewing version %s."),
-				sprintf('%s (<a href="%s">%s</a>)', 
-					$versionID, 
-					Controller::join_links($link, $versionID),
-					$view
-				)
+				_t('CMSPageHistoryController.VIEWINGVERSION',"Currently viewing version %s."), $versionID
 			);
 		}
 		
@@ -169,7 +173,7 @@ class CMSPageHistoryController extends CMSMain {
 						}
 					}
 					$version->CMSLink = sprintf('%s/%s/%s',
-						$this->Link('version'),
+						$this->Link('show'),
 						$version->ID,
 						$version->Version
 					);
@@ -200,7 +204,7 @@ class CMSPageHistoryController extends CMSMain {
 					$compareModeChecked
 				),
 				new LiteralField('VersionsHtml', $versionsHtml),
-				new HiddenField('ID', false, $id)
+				$hiddenID = new HiddenField('ID', false, "")
 			),
 			new FieldSet(
 				new FormAction(
@@ -213,6 +217,7 @@ class CMSPageHistoryController extends CMSMain {
 		);
 		
 		$form->loadDataFrom($this->request->requestVars());
+		$hiddenID->setValue($id);
 		$form->unsetValidator();
 		
 		return $form;
@@ -297,7 +302,54 @@ class CMSPageHistoryController extends CMSMain {
 	 * @return html
 	 */
 	function doRollback($data, $form) {
-		//
+		$this->extend('onBeforeRollback', $data['ID']);
+		$id = (isset($data['ID'])) ? (int) $data['ID'] : null;
+		$version = (isset($data['Version'])) ? (int) $data['Version'] : null;
+		
+		if(isset($data['Version']) && (bool)$data['Version']) {
+			$record = $this->performRollback($data['ID'], $data['Version']);
+			$message = sprintf(
+			_t('CMSMain.ROLLEDBACKVERSION',"Rolled back to version #%d.  New version number is #%d"),
+			$data['Version'],
+			$record->Version
+		);
+		} else {
+			$record = $this->performRollback($data['ID'], "Live");
+			$message = sprintf(
+				_t('CMSMain.ROLLEDBACKPUB',"Rolled back to published version. New version number is #%d"),
+				$record->Version
+			);
+		}
+		
+		if($this->isAjax()) {
+			$this->response->addHeader('X-Status', $message);
+			$form = $this->getEditForm($record->ID);
+		
+			return $form->forTemplate();
+		}
+
+		return array(
+			'EditForm' => $this->customise(array(
+				'Message' => $message,
+				'Status' => 'success'
+			))->renderWith('CMSMain_notice')
+		);
+	}
+	
+	/**
+	 * Performs a rollback of the a given 
+	 *
+	 * @param int $id record ID
+	 * @param int $version version ID to rollback to
+	 */
+	function performRollback($id, $version) {
+		$record = DataObject::get_by_id($this->stat('tree_class'), $id);
+		
+		if($record && !$record->canEdit()) return Security::permissionFailure($this);
+		
+		$record->doRollbackTo($version);
+		
+		return $record;
 	}
 
 	/**
@@ -361,31 +413,4 @@ class CMSPageHistoryController extends CMSMain {
 			return $form;
 		}
 	}
-
-	/**
-	 * Roll a page back to a previous version
-	 */
-	function rollback($data, $form) {
-		$this->extend('onBeforeRollback', $data['ID']);
-		
-		if(isset($data['Version']) && (bool)$data['Version']) {
-			$record = $this->performRollback($data['ID'], $data['Version']);
-			$message = sprintf(
-			_t('CMSMain.ROLLEDBACKVERSION',"Rolled back to version #%d.  New version number is #%d"),
-			$data['Version'],
-			$record->Version
-		);
-		} else {
-			$record = $this->performRollback($data['ID'], "Live");
-			$message = sprintf(
-				_t('CMSMain.ROLLEDBACKPUB',"Rolled back to published version. New version number is #%d"),
-				$record->Version
-			);
-		}
-		
-		$this->response->addHeader('X-Status', $message);
-		$form = $this->getEditForm($record->ID);
-		
-		return $form->forTemplate();
-	}
 }

javascript/CMSPageHistoryController.js

@@ -44,6 +44,9 @@
 			 *
 			 * Submits either the compare versions form or the view single form
 			 * display based on whether we have two or 1 option selected
+			 *
+			 * Todo:
+			 *		Handle coupling to admin url 
 			 */
 			onsubmit: function(e, d) {
 				var id, self = this;
@@ -51,7 +54,7 @@
 	
 				if(!id) return false;
 
-				var button, url, selected, to, from, compare;
+				var button, url, selected, to, from, compare, data;
 				
 				compare = (this.find(":input[name=CompareMode]").is(":checked"));
 				selected = this.find("table input[type=checkbox]").filter(":checked");
@@ -67,14 +70,12 @@
 				else {
 					to = selected.eq(0).val();
 					button = this.find(':submit[name=action_doShowVersion]');
-					url = 'admin/page/history/version/'+ [id,to].join('/') + "/";
+					url = 'admin/page/history/show/'+ [id,to].join('/') + "/";
 				}
 				
-				// we can access this comparsion directly in the url
 				window.History.pushState({selector: '.cms-content-fields form:first'}, '', url);
 				
-				var data = this.serializeArray();
-				
+				data = this.serializeArray();
 				data.push({
 					name: button.attr('name'), value: button.val()
 				});

tests/controller/CMSPageHistoryControllerTest.php

@@ -0,0 +1,102 @@
+<?php
+
+/**
+ * @package cms
+ * @subpackage tests
+ */
+
+class CMSPageHistoryControllerTest extends FunctionalTest {
+	
+	static $fixture_file = 'CMSPageHistoryControllerTest.yml';
+	
+	private $versionUnpublishedCheck, $versionPublishCheck, $versionUnpublishedCheck2;
+	private $page;
+	
+	function setUp() {
+		parent::setUp();
+		
+		$this->loginWithPermission('ADMIN');
+		
+		// creates a series of published, unpublished versions of a page
+		$this->page = new Page();
+		$this->page->URLSegment = "test";
+		$this->page->Content = "new content";
+		$this->page->write(); 
+		$this->versionUnpublishedCheck = $this->page->Version;
+		
+		$this->page->Content = "some further content";
+		$this->page->write();
+		$this->page->publish('Stage', 'Live');
+		$this->versionPublishCheck = $this->page->Version;
+		
+		$this->page->Content = "No, more changes please";
+		$this->page->Title = "Changing titles too";
+		$this->page->write();
+		$this->versionUnpublishedCheck2 = $this->page->Version;
+		
+		$this->page->Title = "Final Change";
+		$this->page->write();
+		$this->page->publish('Stage', 'Live');
+		$this->versionPublishCheck2 = $this->page->Version;
+	}
+	
+	function testGetEditForm() {
+
+	}
+
+	/**
+	 * @todo should be less tied to cms theme
+	 */
+	function testVersionsForm() {
+		$history = $this->get('admin/page/history/show/'. $this->page->ID);
+
+		$form = $this->cssParser()->getBySelector("#Form_VersionsForm");
+		$this->assertEquals(1, count($form));
+		
+		// check the page ID is present
+		$hidden = $form[0]->xpath("fieldset/input[@type='hidden']");
+		$this->assertFalse($hidden == null, 'Hidden ID field exists');
+		$this->assertEquals(4, (int) $hidden[0]->attributes()->value);
+		
+		// ensure that all the versions are present in the table and displayed
+		$rows = $form[0]->xpath("fieldset/table/tbody/tr");
+		
+		$this->assertFalse($hidden == null, "Versions exist in table");
+		$this->assertEquals(4, count($rows));
+		
+		$expected = array(
+			array('version' => $this->versionPublishCheck2, 'status' => 'published'),
+			array('version' => $this->versionUnpublishedCheck2, 'status' => 'internal'),
+			array('version' => $this->versionPublishCheck, 'status' => 'published'),
+			array('version' => $this->versionUnpublishedCheck, 'status' => 'internal')
+		);
+		
+		// goes the reverse order that we created in setUp();
+		$i = 0;
+		foreach($rows as $tr) {
+			$this->assertEquals(
+				sprintf('admin/page/history/show/%d/%d', $this->page->ID, $expected[$i]['version']), 
+				(string) $tr->attributes()->{'data-link'}
+			);
+			
+			$this->assertContains($expected[$i]['status'], (string) $tr->attributes()->class);
+			$i++;
+		}
+	}
+
+	function testDoForm() {
+		
+	}
+	
+	function testCompareForm() {
+		
+	}
+	
+	function testRevertForm() {
+		
+	}
+	
+	function testIsCompareMode() {
+		
+	}
+}

tests/controller/CMSPageHistoryControllerTest.yml

@@ -0,0 +1,10 @@
+Page:
+   page1:
+      Title: Page 1
+      Sort: 1
+   page2:
+      Title: Page 2
+      Sort: 2
+   page3:
+      Title: Page 3
+      Sort: 3