CMSPageHistoryController - Code Metrics - Inspection of "Part 1 of 5, remove JLayout and apply flexbox to p..." - silverstripe/silverstripe-cms - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#1638)

unknown

created 2016-10-10 21:12 UTC

CMSPageHistoryController C

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	434
Duplicated Lines	10.83 %

Coupling/Cohesion

Components	1
Dependencies	18

Importance

Changes

Metric	Value
wmc	48
lcom	1
cbo	18
dl	47
loc	434
rs	5.3384
c	0
b	0
f	0

11 Methods

Rating	Name	Duplication	Size	Complexity
A	getResponseNegotiator()	0	13	2
A	show()	14	14	2
A	compare()	17	17	2
A	getSilverStripeNavigator()	0	9	2
C	getEditForm()	0	76	9
A	ShowVersionForm()	0	8	2
A	Breadcrumbs()	0	5	1
C	VersionsForm()	0	84	9
B	doCompare()	8	28	3
B	doShowVersion()	8	27	5
C	CompareVersionsForm()	0	62	11

How to fix Duplicated Code Complexity

Duplicated Code

Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.

Common duplication problems, and corresponding solutions are:

If you have the same expression in different places: Extract expression to a method
If you have the same method in different sub-classes: Extract method, and pull up field to the parent class
If you have the same code in unrelated classes: Consider extracting the code to a new class, and injecting that class

Complex Class

Tip: Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

Complex classes like CMSPageHistoryController often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use CMSPageHistoryController, and based on these observations, apply Extract Interface, too.

<?php

namespace SilverStripe\CMS\Controllers;

use SilverStripe\CMS\Model\SiteTree;
use SilverStripe\Control\Controller;
use SilverStripe\Control\HTTPRequest;
use SilverStripe\Control\HTTPResponse;
use SilverStripe\Forms\CheckboxField;
use SilverStripe\Forms\FieldList;
use SilverStripe\Forms\Form;
use SilverStripe\Forms\FormAction;
use SilverStripe\Forms\HiddenField;
use SilverStripe\Forms\LiteralField;
use SilverStripe\ORM\FieldType\DBField;
use SilverStripe\ORM\FieldType\DBHTMLText;
use SilverStripe\ORM\Versioning\Versioned;
use SilverStripe\Security\Security;
use SilverStripe\View\ViewableData;

class CMSPageHistoryController extends CMSMain {

	private static $url_segment = 'pages/history';


	private static $url_rule = '/$Action/$ID/$VersionID/$OtherVersionID';


	private static $url_priority = 42;


	private static $menu_title = 'History';


	private static $required_permission_codes = 'CMS_ACCESS_CMSMain';


	private static $allowed_actions = array(

		'VersionsForm',
		'CompareVersionsForm',
		'show',
		'compare'
	);

	private static $url_handlers = array(

		'$Action/$ID/$VersionID/$OtherVersionID' => 'handleAction'
	);

	public function getResponseNegotiator() {
		$negotiator = parent::getResponseNegotiator();
		$controller = $this;
		$negotiator->setCallback('CurrentForm', function() use(&$controller) {
			$form = $controller->ShowVersionForm($controller->getRequest()->param('VersionID'));
			if($form) return $form->forTemplate();
			else return $controller->renderWith($controller->getTemplatesWithSuffix('_Content'));
		});
		$negotiator->setCallback('default', function() use(&$controller) {
			return $controller->renderWith($controller->getViewer('show'));
		});
		return $negotiator;
	}

	/**
	 * @param HTTPRequest $request
	 * @return array
	 */
	public function show($request) {

		$form = $this->ShowVersionForm($request->param('VersionID'));

		$negotiator = $this->getResponseNegotiator();
		$controller = $this;
		$negotiator->setCallback('CurrentForm', function() use(&$controller, &$form) {
			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 $negotiator->respond($request);
	}

	/**
	 * @param HTTPRequest $request
	 * @return array
	 */
	public function compare($request) {

		$form = $this->CompareVersionsForm(
			$request->param('VersionID'),
			$request->param('OtherVersionID')
		);

		$negotiator = $this->getResponseNegotiator();
		$controller = $this;
		$negotiator->setCallback('CurrentForm', function() use(&$controller, &$form) {
			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 $negotiator->respond($request);
	}

	public function getSilverStripeNavigator() {
		$record = $this->getRecord($this->currentPageID(), $this->getRequest()->param('VersionID'));
		if($record) {
			$navigator = new SilverStripeNavigator($record);
			return $navigator->renderWith($this->getTemplatesWithSuffix('_SilverStripeNavigator'));
		} else {
			return false;
class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}
		}
	}

	/**
	 * Returns the read only version of the edit form. Detaches all {@link FormAction}
	 * instances attached since only action relates to revert.
	 *
	 * Permission checking is done at the {@link CMSMain::getEditForm()} level.
	 *
	 * @param int $id ID of the record to show
	 * @param array $fields optional
	 * @param int $versionID
	 * @param int $compareID Compare mode
	 *
	 * @return Form
	 */
	public function getEditForm($id = null, $fields = null, $versionID = null, $compareID = null) {
		if(!$id) $id = $this->currentPageID();
0   == false // true
0   == null  // true
123 == false // false
123 == null  // false

// It is often better to use strict comparison
0 === false // false
0 === null  // false

		$record = $this->getRecord($id, $versionID);
		$versionID = ($record) ? $record->Version : $versionID;


		$form = parent::getEditForm($record, ($record) ? $record->getCMSFields() : null);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
		// Respect permission failures from parent implementation
		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(
			$revert = FormAction::create('doRollback', _t('CMSPageHistoryController.REVERTTOTHISVERSION', 'Revert to this version'))->setUseButtonTag(true)
		));

		$fields = $form->Fields();
		$fields->removeByName("Status");
		$fields->push(new HiddenField("ID"));
		$fields->push(new HiddenField("Version"));

		$fields = $fields->makeReadonly();

		if($compareID) {
0   == false // true
0   == null  // true
123 == false // false
123 == null  // false

// It is often better to use strict comparison
0 === false // false
0 === null  // false
			$link = Controller::join_links(
				$this->Link('show'),
				$id
			);

			$view = _t('CMSPageHistoryController.VIEW',"view");

			$message = _t(
				'CMSPageHistoryController.COMPARINGVERSION',
				"Comparing versions {version1} and {version2}.",
				array(
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
					'version1' => sprintf('%s (<a href="%s">%s</a>)', $versionID, Controller::join_links($link, $versionID), $view),
					'version2' => sprintf('%s (<a href="%s">%s</a>)', $compareID, Controller::join_links($link, $compareID), $view)
				)
			);

			$revert->setReadonly(true);
		} else {
			if($record->isLatestVersion()) {
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
				$message = _t('CMSPageHistoryController.VIEWINGLATEST', 'Currently viewing the latest version.');
			} else {
				$message = _t(
					'CMSPageHistoryController.VIEWINGVERSION',
					"Currently viewing version {version}.",
					array('version' => $versionID)
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
				);
			}
		}

		$fields->addFieldToTab('Root.Main',
			new LiteralField('CurrentlyViewingMessage', $this->customise(array(
				'Content' => DBField::create_field('HTMLFragment', $message),
				'Classes' => 'notice'
			))->renderWith($this->getTemplatesWithSuffix('_notice'))),
			"Title"
		);

		$form->setFields($fields->makeReadonly());
		$form->loadDataFrom(array(
			"ID" => $id,
			"Version" => $versionID,
		));

		if(($record && $record->isLatestVersion())) {
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
			$revert->setReadonly(true);
		}

		$form->removeExtraClass('cms-content');

		return $form;
	}


	/**
	 * Version select form. Main interface between selecting versions to view
	 * and comparing multiple versions.
	 *
	 * Because we can reload the page directly to a compare view (history/compare/1/2/3)
	 * this form has to adapt to those parameters as well.
	 *
	 * @return Form
	 */
	public function VersionsForm() {
		$id = $this->currentPageID();
		$page = $this->getRecord($id);
		$versionsHtml = '';

		$action = $this->getRequest()->param('Action');
		$versionID = $this->getRequest()->param('VersionID');
		$otherVersionID = $this->getRequest()->param('OtherVersionID');

		$showUnpublishedChecked = 0;
		$compareModeChecked = ($action == "compare");

		if($page) {
			$versions = $page->allVersions();
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
			$versionID = (!$versionID) ? $page->Version : $versionID;


			if($versions) {
				foreach($versions as $k => $version) {
					$active = false;

					if($version->Version == $versionID || $version->Version == $otherVersionID) {
						$active = true;

						if(!$version->WasPublished) $showUnpublishedChecked = 1;
					}

					$version->Active = ($active);
				}
			}

			$vd = new ViewableData();

			$versionsHtml = $vd->customise(array(
				'Versions' => $versions
			))->renderWith($this->getTemplatesWithSuffix('_versions'));
		}

		$fields = new FieldList(
			new CheckboxField(
				'ShowUnpublished',
				_t('CMSPageHistoryController.SHOWUNPUBLISHED','Show unpublished versions'),
				$showUnpublishedChecked
			),
			new CheckboxField(
				'CompareMode',
				_t('CMSPageHistoryController.COMPAREMODE', 'Compare mode (select two)'),
				$compareModeChecked
			),
			new LiteralField('VersionsHtml', $versionsHtml),
/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}
			$hiddenID = new HiddenField('ID', false, "")
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
		);

		$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) {
			/** @var FormAction $action */
			$action->setUseButtonTag(true);
		}

		$form = Form::create(
			$this,
			'VersionsForm',
			$fields,
			$actions
		)->setHTMLID('Form_VersionsForm');
		$form->loadDataFrom($this->getRequest()->requestVars());
		$hiddenID->setValue($id);
		$form->unsetValidator();

		$form
			->addExtraClass('cms-versions-form') // placeholder, necessary for $.metadata() to work
			->setAttribute('data-link-tmpl-compare', Controller::join_links($this->Link('compare'), '%s', '%s', '%s'))
			->setAttribute('data-link-tmpl-show', Controller::join_links($this->Link('show'), '%s', '%s'));

		return $form;
	}

	/**
	 * Process the {@link VersionsForm} compare function between two pages.
	 *
	 * @param array $data
	 * @param Form $form
	 * @return HTTPResponse|DBHTMLText
	 */
	public function doCompare($data, $form) {

		$versions = $data['Versions'];
		if(count($versions) < 2) {
			return null;
		}

		$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(
				static::class . '_EditForm',
				'LeftAndMain_Content'
			));
		}

		// non javascript, redirect the user to the page
		return $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 DBHTMLText|HTTPResponse
	 */
	public function doShowVersion($data, $form) {

		$versionID = null;

		if(isset($data['Versions']) && is_array($data['Versions'])) {
			$versionID  = array_shift($data['Versions']);
		}

		if(!$versionID) {
			return null;
		}

		$request = $this->getRequest();
		if($request->isAjax()) {

			return $this->customise(array(
				"EditForm" => $this->ShowVersionForm($versionID)
			))->renderWith(array(
				static::class . '_EditForm',
				'LeftAndMain_Content'
			));
		}

		// non javascript, redirect the user to the page
		return $this->redirect(Controller::join_links(
			$this->Link('version'),
			$versionID
		));
	}

	/**
	 * @param int|null $versionID
	 * @return Form
	 */
	public function ShowVersionForm($versionID = null) {
		if(!$versionID) return null;
0   == false // true
0   == null  // true
123 == false // false
123 == null  // false

// It is often better to use strict comparison
0 === false // false
0 === null  // false

		$id = $this->currentPageID();
		$form = $this->getEditForm($id, null, $versionID);

		return $form;
	}

	/**
	 * @param int $versionID
	 * @param int $otherVersionID
	 * @return mixed
	 */
	public function CompareVersionsForm($versionID, $otherVersionID) {
		if($versionID > $otherVersionID) {
			$toVersion = $versionID;
			$fromVersion = $otherVersionID;
		} else {
			$toVersion = $otherVersionID;
			$fromVersion = $versionID;
		}

		if(!$toVersion || !$fromVersion) {
			return null;
		}

		$id = $this->currentPageID();
		/** @var SiteTree $page */
		$page = SiteTree::get()->byID($id);

		$record = null;
 		if($page && $page->exists()) {
			if(!$page->canView()) {
				return Security::permissionFailure($this);
			}

			$record = $page->compareVersions($fromVersion, $toVersion);
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
		}

		$fromVersionRecord = Versioned::get_version('SilverStripe\\CMS\\Model\\SiteTree', $id, $fromVersion);
		$toVersionRecord = Versioned::get_version('SilverStripe\\CMS\\Model\\SiteTree', $id, $toVersion);

		if(!$fromVersionRecord) {
			user_error("Can't find version $fromVersion of page $id", E_USER_ERROR);
		}

		if(!$toVersionRecord) {
			user_error("Can't find version $toVersion of page $id", E_USER_ERROR);
		}

		if(!$record) {
			return null;
		}
		$form = $this->getEditForm($id, null, null, true);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
		$form->setActions(new FieldList());
		$form->addExtraClass('compare');

		// Comparison views shouldn't be editable.
		// Its important to convert fields *before* loading data,
		// as the comparison output is HTML and not valid values for the various field types
		$readonlyFields = $form->Fields()->makeReadonly();
		$form->setFields($readonlyFields);

		$form->loadDataFrom($record);
		$form->loadDataFrom(array(
			"ID" => $id,
			"Version" => $fromVersion,
		));

		foreach($form->Fields()->dataFields() as $field) {
			$field->dontEscape = true;
		}

		return $form;
	}

	public function Breadcrumbs($unlinked = false) {
		$crumbs = parent::Breadcrumbs($unlinked);
		$crumbs[0]->Title = _t('CMSPagesController.MENUTITLE');
		return $crumbs;
	}

}


silverstripe / silverstripe-cms