fixing cms page history controller to use new page id param

code/controllers/CMSPageHistoryController.php

@@ -14,6 +14,7 @@ class CMSPageHistoryController extends CMSMain {
 	private static $session_namespace = 'CMSMain';
 
 	private static $allowed_actions = array(
+		'EditForm',
 		'VersionsForm',
 		'CompareVersionsForm',
 		'show',
@@ -21,16 +22,27 @@ class CMSPageHistoryController extends CMSMain {
 	);
 
 	private static $url_handlers = array(
-		'$Action/$ID/$VersionID/$OtherVersionID' => 'handleAction'
+		'EditForm/$ID/$VersionID' => 'EditForm',
+		'$Action/$ID/$VersionID/$OtherVersionID' => 'handleAction',
 	);
 
+	/**
+	 * Current version ID for this request. Can be 0 for latest version
+	 *
+	 * @var int
+	 */
+	protected $versionID = null;
+
 	public function getResponseNegotiator() {
 		$negotiator = parent::getResponseNegotiator();
 		$controller = $this;
 		$negotiator->setCallback('CurrentForm', function() use(&$controller) {
-			$form = $controller->ShowVersionForm($controller->getRequest()->param('VersionID'));
+			$form = $controller->getEditForm();
-			if($form) return $form->forTemplate();
+			if ($form) {
-			else return $controller->renderWith($controller->getTemplatesWithSuffix('_Content'));
+				return $form->forTemplate();
+			} else {
+				return $controller->renderWith($controller->getTemplatesWithSuffix('_Content'));
+			}
 		});
 		$negotiator->setCallback('default', function() use(&$controller) {
 			return $controller->renderWith($controller->getViewer('show'));
@@ -40,18 +52,28 @@ class CMSPageHistoryController extends CMSMain {
 
 	/**
 	 * @param SS_HTTPRequest $request
-	 * @return array
+	 * @return SS_HTTPResponse
 	 */
 	public function show($request) {
-		$form = $this->ShowVersionForm($request->param('VersionID'));
+		// Record id and version for this request
+		$id = $request->param('ID');
+		$this->setCurrentPageID($id);
+		$versionID = $request->param('VersionID');
+		$this->setVersionID($versionID);
+
+		$form = $this->getEditForm();
 
 		$negotiator = $this->getResponseNegotiator();
 		$controller = $this;
 		$negotiator->setCallback('CurrentForm', function() use(&$controller, &$form) {
-			return $form ? $form->forTemplate() : $controller->renderWith($controller->getTemplatesWithSuffix('_Content'));
+			return $form
+				? $form->forTemplate()
+				: $controller->renderWith($controller->getTemplatesWithSuffix('_Content'));
 		});
 		$negotiator->setCallback('default', function() use(&$controller, &$form) {
-			return $controller->customise(array('EditForm' => $form))->renderWith($controller->getViewer('show'));
+			return $controller
+				->customise(array('EditForm' => $form))
+				->renderWith($controller->getViewer('show'));
 		});
 
 		return $negotiator->respond($request);
@@ -59,9 +81,12 @@ class CMSPageHistoryController extends CMSMain {
 
 	/**
 	 * @param SS_HTTPRequest $request
-	 * @return array
+	 * @return SS_HTTPResponse
 	 */
 	public function compare($request) {
+		$id = $request->param('ID');
+		$this->setCurrentPageID($id);
+
 		$form = $this->CompareVersionsForm(
 			$request->param('VersionID'),
 			$request->param('OtherVersionID')
@@ -70,10 +95,14 @@ class CMSPageHistoryController extends CMSMain {
 		$negotiator = $this->getResponseNegotiator();
 		$controller = $this;
 		$negotiator->setCallback('CurrentForm', function() use(&$controller, &$form) {
-			return $form ? $form->forTemplate() : $controller->renderWith($controller->getTemplatesWithSuffix('_Content'));
+			return $form
+				? $form->forTemplate()
+				: $controller->renderWith($controller->getTemplatesWithSuffix('_Content'));
 		});
 		$negotiator->setCallback('default', function() use(&$controller, &$form) {
-			return $controller->customise(array('EditForm' => $form))->renderWith($controller->getViewer('show'));
+			return $controller
+				->customise(array('EditForm' => $form))
+				->renderWith($controller->getViewer('show'));
 		});
 
 		return $negotiator->respond($request);
@@ -89,6 +118,23 @@ class CMSPageHistoryController extends CMSMain {
 		}
 	}
 
+	/**
+	 * @param SS_HTTPRequest $request
+	 * @return Form
+	 */
+	public function EditForm($request = null) {
+		if ($request) {
+			// Validate VersionID is present
+			$versionID = $request->param('VersionID');
+			if (!isset($versionID)) {
+				$this->httpError(400);
+				return null;
+			}
+			$this->setVersionID($versionID);
+		}
+		return parent::EditForm($request);
+	}
+
 	/**
 	 * Returns the read only version of the edit form. Detaches all {@link FormAction}
 	 * instances attached since only action relates to revert.
@@ -103,21 +149,40 @@ class CMSPageHistoryController extends CMSMain {
 	 * @return Form
 	 */
 	public function getEditForm($id = null, $fields = null, $versionID = null, $compareID = null) {
-		if(!$id) $id = $this->currentPageID();
+		if (!$id) {
+			$id = $this->currentPageID();
+		}
+		if (!$versionID) {
+			$versionID = $this->getVersionID();
+		}
 
 		$record = $this->getRecord($id, $versionID);
-		$versionID = ($record) ? $record->Version : $versionID;
+		if (!$record) {
+			return $this->EmptyForm();
+		}
 
-		$form = parent::getEditForm($record, ($record) ? $record->getCMSFields() : null);
+		// Refresh version ID
+		$versionID = $record->Version;
+		$this->setVersionID($versionID);
+
+		// Get edit form
+		$form = parent::getEditForm($record, $record->getCMSFields());
 		// Respect permission failures from parent implementation
-		if(!($form instanceof Form)) return $form;
+		if (!($form instanceof Form)) {
+			return $form;
+		}
 
 		// TODO: move to the SilverStripeNavigator structure so the new preview can pick it up.
 		//$nav = new SilverStripeNavigatorItem_ArchiveLink($record);
 
-		$form->setActions(new FieldList(
+		$actions = new FieldList(
-			$revert = FormAction::create('doRollback', _t('CMSPageHistoryController.REVERTTOTHISVERSION', 'Revert to this version'))->setUseButtonTag(true)
+			$revert = FormAction::create(
-		));
+				'doRollback',
+				_t('CMSPageHistoryController.REVERTTOTHISVERSION', 'Revert to this version')
+			)->setUseButtonTag(true)
+		);
+		$actions->setForm($form);
+		$form->setActions($actions);
 
 		$fields = $form->Fields();
 		$fields->removeByName("Status");
@@ -170,12 +235,14 @@ class CMSPageHistoryController extends CMSMain {
 			"Version" => $versionID,
 		));
 
-		if(($record && $record->isLatestVersion())) {
+		if ($record->isLatestVersion()) {
 			$revert->setReadonly(true);
 		}
 
 		$form->removeExtraClass('cms-content');
 
+		$form->setFormAction(Controller::join_links($form->FormAction(), $id, $versionID));
+
 		return $form;
 	}
 
@@ -241,23 +308,11 @@ class CMSPageHistoryController extends CMSMain {
 			$hiddenID = new HiddenField('ID', false, "")
 		);
 
-		$actions = new FieldList(
-			new FormAction(
-				'doCompare', _t('CMSPageHistoryController.COMPAREVERSIONS','Compare Versions')
-			),
-			new FormAction(
-				'doShowVersion', _t('CMSPageHistoryController.SHOWVERSION','Show Version')
-			)
-		);
-
-		// Use <button> to allow full jQuery UI styling
-		foreach($actions->dataFields() as $action) $action->setUseButtonTag(true);
-
 		$form = CMSForm::create(
 			$this,
 			'VersionsForm',
 			$fields,
-			$actions
+			new FieldList()
 		)->setHTMLID('Form_VersionsForm');
 		$form->setResponseNegotiator($this->getResponseNegotiator());
 		$form->loadDataFrom($this->getRequest()->requestVars());
@@ -272,89 +327,6 @@ class CMSPageHistoryController extends CMSMain {
 		return $form;
 	}
 
-	/**
-	 * Process the {@link VersionsForm} compare function between two pages.
-	 *
-	 * @param array
-	 * @param Form
-	 *
-	 * @return html
-	 */
-	public function doCompare($data, $form) {
-		$versions = $data['Versions'];
-		if(count($versions) < 2) return null;
-		
-		$id = $this->currentPageID();
-		$version1 = array_shift($versions);
-		$version2 = array_shift($versions);
-
-		$form = $this->CompareVersionsForm($version1, $version2);
-
-		// javascript solution, render into template
-		if($this->getRequest()->isAjax()) {
-			return $this->customise(array(
-				"EditForm" => $form
-			))->renderWith(array(
-				$this->class . '_EditForm',
-				'LeftAndMain_Content'
-			));
-		}
-		
-		// non javascript, redirect the user to the page
-		$this->redirect(Controller::join_links(
-			$this->Link('compare'),
-			$version1,
-			$version2
-		));
-	}
-
-	/**
-	 * Process the {@link VersionsForm} show version function. Only requires
-	 * one page to be selected.
-	 *
-	 * @param array
-	 * @param Form
-	 *
-	 * @return html
-	 */	
-	public function doShowVersion($data, $form) {
-		$versionID = null;
-		
-		if(isset($data['Versions']) && is_array($data['Versions'])) {
-			$versionID  = array_shift($data['Versions']);
-		}
-		
-		if(!$versionID) return;
-		
-		if($request->isAjax()) {
-			return $this->customise(array(
-				"EditForm" => $this->ShowVersionForm($versionID)
-			))->renderWith(array(
-				$this->class . '_EditForm',
-				'LeftAndMain_Content'
-			));
-		}
-
-		// non javascript, redirect the user to the page
-		$this->redirect(Controller::join_links(
-			$this->Link('version'),
-			$versionID
-		));
-	}
-
-	/**
-	 * @param int|null $versionID
-	 * @return Form
-	 */
-	public function ShowVersionForm($versionID = null) {
-		if(!$versionID) return null;
-
-		$id = $this->currentPageID();
-		$form = $this->getEditForm($id, null, $versionID);
-
-		return $form;
-	}
-	
 	/**
 	 * @param int $versionID
 	 * @param int $otherVersionID
@@ -417,7 +389,7 @@ class CMSPageHistoryController extends CMSMain {
 			return $form;
 		}
 
-        	return false;
+		return false;
 	}
 
 	public function Breadcrumbs($unlinked = false) {
@@ -426,4 +398,24 @@ class CMSPageHistoryController extends CMSMain {
 		return $crumbs;
 	}
 
+	/**
+	 * Set current version ID
+	 *
+	 * @param int $versionID
+	 * @return $this
+	 */
+	public function setVersionID($versionID) {
+		$this->versionID = $versionID;
+		return $this;
+	}
+
+	/**
+	 * Get current version ID
+	 *
+	 * @return int
+	 */
+	public function getVersionID() {
+		return $this->versionID;
+	}
+
 }