Completed
Pull Request — master (#1458)
by Sam
02:39
created

CMSPageHistoryController::CompareVersionsForm()   C

Complexity

Conditions 10
Paths 28

Size

Total Lines 55
Code Lines 32

Duplication

Lines 0
Ratio 0 %

Importance

Changes 1
Bugs 0 Features 0
Metric Value
c 1
b 0
f 0
dl 0
loc 55
rs 6.8372
cc 10
eloc 32
nc 28
nop 2

How to fix   Long Method    Complexity   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

1
<?php
2
3
/**
4
 * @package cms
5
 * @subpackage controllers
6
 */
7
class CMSPageHistoryController extends CMSMain {
0 ignored issues
show
Bug introduced by
There is one abstract method isCurrentPage in this class; you could implement it, or declare this class as abstract.
Loading history...
8
9
	private static $url_segment = 'pages/history';
0 ignored issues
show
Comprehensibility introduced by
Consider using a different property name as you override a private property of the parent class.
Loading history...
10
	private static $url_rule = '/$Action/$ID/$VersionID/$OtherVersionID';
0 ignored issues
show
Comprehensibility introduced by
Consider using a different property name as you override a private property of the parent class.
Loading history...
11
	private static $url_priority = 42;
0 ignored issues
show
Comprehensibility introduced by
Consider using a different property name as you override a private property of the parent class.
Loading history...
12
	private static $menu_title = 'History';
0 ignored issues
show
Comprehensibility introduced by
Consider using a different property name as you override a private property of the parent class.
Loading history...
13
	private static $required_permission_codes = 'CMS_ACCESS_CMSMain';
14
	private static $session_namespace = 'CMSMain';
15
16
	private static $allowed_actions = array(
0 ignored issues
show
Comprehensibility introduced by
Consider using a different property name as you override a private property of the parent class.
Loading history...
17
		'VersionsForm',
18
		'CompareVersionsForm',
19
		'show',
20
		'compare'
21
	);
22
23
	private static $url_handlers = array(
24
		'$Action/$ID/$VersionID/$OtherVersionID' => 'handleAction'
25
	);
26
27
	public function getResponseNegotiator() {
28
		$negotiator = parent::getResponseNegotiator();
29
		$controller = $this;
30
		$negotiator->setCallback('CurrentForm', function() use(&$controller) {
31
			$form = $controller->ShowVersionForm($controller->getRequest()->param('VersionID'));
32
			if($form) return $form->forTemplate();
33
			else return $controller->renderWith($controller->getTemplatesWithSuffix('_Content'));
34
		});
35
		$negotiator->setCallback('default', function() use(&$controller) {
36
			return $controller->renderWith($controller->getViewer('show'));
37
		});
38
		return $negotiator;
39
	}
40
41
	/**
42
	 * @param SS_HTTPRequest $request
43
	 * @return array
44
	 */
45 View Code Duplication
	public function show($request) {
0 ignored issues
show
Duplication introduced by
This method seems to be duplicated in your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
46
		$form = $this->ShowVersionForm($request->param('VersionID'));
47
48
		$negotiator = $this->getResponseNegotiator();
49
		$controller = $this;
50
		$negotiator->setCallback('CurrentForm', function() use(&$controller, &$form) {
51
			return $form ? $form->forTemplate() : $controller->renderWith($controller->getTemplatesWithSuffix('_Content'));
52
		});
53
		$negotiator->setCallback('default', function() use(&$controller, &$form) {
54
			return $controller->customise(array('EditForm' => $form))->renderWith($controller->getViewer('show'));
55
		});
56
57
		return $negotiator->respond($request);
58
	}
59
60
	/**
61
	 * @param SS_HTTPRequest $request
62
	 * @return array
63
	 */
64 View Code Duplication
	public function compare($request) {
0 ignored issues
show
Duplication introduced by
This method seems to be duplicated in your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
65
		$form = $this->CompareVersionsForm(
66
			$request->param('VersionID'),
67
			$request->param('OtherVersionID')
68
		);
69
70
		$negotiator = $this->getResponseNegotiator();
71
		$controller = $this;
72
		$negotiator->setCallback('CurrentForm', function() use(&$controller, &$form) {
73
			return $form ? $form->forTemplate() : $controller->renderWith($controller->getTemplatesWithSuffix('_Content'));
74
		});
75
		$negotiator->setCallback('default', function() use(&$controller, &$form) {
76
			return $controller->customise(array('EditForm' => $form))->renderWith($controller->getViewer('show'));
77
		});
78
79
		return $negotiator->respond($request);
80
	}
81
82
	public function getSilverStripeNavigator() {
83
		$record = $this->getRecord($this->currentPageID(), $this->getRequest()->param('VersionID'));
84
		if($record) {
85
			$navigator = new SilverStripeNavigator($record);
86
			return $navigator->renderWith($this->getTemplatesWithSuffix('_SilverStripeNavigator'));
87
		} else {
88
			return false;
89
		}
90
	}
91
92
	/**
93
	 * Returns the read only version of the edit form. Detaches all {@link FormAction}
94
	 * instances attached since only action relates to revert.
95
	 *
96
	 * Permission checking is done at the {@link CMSMain::getEditForm()} level.
97
	 *
98
	 * @param int $id ID of the record to show
99
	 * @param array $fields optional
100
	 * @param int $versionID
101
	 * @param int $compareID Compare mode
102
	 *
103
	 * @return Form
104
	 */
105
	public function getEditForm($id = null, $fields = null, $versionID = null, $compareID = null) {
106
		if(!$id) $id = $this->currentPageID();
0 ignored issues
show
Bug Best Practice introduced by
The expression $id of type integer|null is loosely compared to false; this is ambiguous if the integer can be zero. You might want to explicitly use === null instead.

In PHP, under loose comparison (like ==, or !=, or switch conditions), values of different types might be equal.

For integer values, zero is a special case, in particular the following results might be unexpected:

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
Loading history...
107
108
		$record = $this->getRecord($id, $versionID);
109
		$versionID = ($record) ? $record->Version : $versionID;
110
111
		$form = parent::getEditForm($record, ($record) ? $record->getCMSFields() : null);
112
		// Respect permission failures from parent implementation
113
		if(!($form instanceof Form)) return $form;
0 ignored issues
show
Bug introduced by
The class Form does not exist. Did you forget a USE statement, or did you not list all dependencies?

This error could be the result of:

1. Missing dependencies

PHP Analyzer uses your composer.json file (if available) to determine the dependencies of your project and to determine all the available classes and functions. It expects the composer.json to be in the root folder of your repository.

Are you sure this class is defined by one of your dependencies, or did you maybe not list a dependency in either the require or require-dev section?

2. Missing use statement

PHP does not complain about undefined classes in ìnstanceof checks. For example, the following PHP code will work perfectly fine:

if ($x instanceof DoesNotExist) {
    // Do something.
}

If you have not tested against this specific condition, such errors might go unnoticed.

Loading history...
114
115
		// TODO: move to the SilverStripeNavigator structure so the new preview can pick it up.
116
		//$nav = new SilverStripeNavigatorItem_ArchiveLink($record);
117
118
		$form->setActions(new FieldList(
119
			$revert = FormAction::create('doRollback', _t('CMSPageHistoryController.REVERTTOTHISVERSION', 'Revert to this version'))->setUseButtonTag(true)
120
		));
121
122
		$fields = $form->Fields();
123
		$fields->removeByName("Status");
124
		$fields->push(new HiddenField("ID"));
125
		$fields->push(new HiddenField("Version"));
126
127
		$fields = $fields->makeReadonly();
128
129
		if($compareID) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $compareID of type integer|null is loosely compared to true; this is ambiguous if the integer can be zero. You might want to explicitly use !== null instead.

In PHP, under loose comparison (like ==, or !=, or switch conditions), values of different types might be equal.

For integer values, zero is a special case, in particular the following results might be unexpected:

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
Loading history...
130
			$link = Controller::join_links(
131
				$this->Link('show'),
132
				$id
133
			);
134
135
			$view = _t('CMSPageHistoryController.VIEW',"view");
136
137
			$message = _t(
138
				'CMSPageHistoryController.COMPARINGVERSION',
139
				"Comparing versions {version1} and {version2}.",
140
				array(
141
					'version1' => sprintf('%s (<a href="%s">%s</a>)', $versionID, Controller::join_links($link, $versionID), $view),
142
					'version2' => sprintf('%s (<a href="%s">%s</a>)', $compareID, Controller::join_links($link, $compareID), $view)
143
				)
144
			);
145
146
			$revert->setReadonly(true);
147
		} else {
148
			if($record->isLatestVersion()) {
149
				$message = _t('CMSPageHistoryController.VIEWINGLATEST', 'Currently viewing the latest version.');
150
			} else {
151
				$message = _t(
152
					'CMSPageHistoryController.VIEWINGVERSION',
153
					"Currently viewing version {version}.",
154
					array('version' => $versionID)
155
				);
156
			}
157
		}
158
159
		$fields->addFieldToTab('Root.Main',
160
			new LiteralField('CurrentlyViewingMessage', $this->customise(array(
161
				'Content' => $message,
162
				'Classes' => 'notice'
163
			))->renderWith(array('CMSMain_notice'))),
164
			"Title"
165
		);
166
167
		$form->setFields($fields->makeReadonly());
168
		$form->loadDataFrom(array(
169
			"ID" => $id,
170
			"Version" => $versionID,
171
		));
172
173
		if(($record && $record->isLatestVersion())) {
174
			$revert->setReadonly(true);
175
		}
176
177
		$form->removeExtraClass('cms-content');
178
179
		return $form;
180
	}
181
182
183
	/**
184
	 * Version select form. Main interface between selecting versions to view
185
	 * and comparing multiple versions.
186
	 *
187
	 * Because we can reload the page directly to a compare view (history/compare/1/2/3)
188
	 * this form has to adapt to those parameters as well.
189
	 *
190
	 * @return Form
191
	 */
192
	public function VersionsForm() {
193
		$id = $this->currentPageID();
194
		$page = $this->getRecord($id);
195
		$versionsHtml = '';
196
197
		$action = $this->getRequest()->param('Action');
198
		$versionID = $this->getRequest()->param('VersionID');
199
		$otherVersionID = $this->getRequest()->param('OtherVersionID');
200
201
		$showUnpublishedChecked = 0;
202
		$compareModeChecked = ($action == "compare");
203
204
		if($page) {
205
			$versions = $page->allVersions();
206
			$versionID = (!$versionID) ? $page->Version : $versionID;
207
208
			if($versions) {
209
				foreach($versions as $k => $version) {
210
					$active = false;
211
212
					if($version->Version == $versionID || $version->Version == $otherVersionID) {
213
						$active = true;
214
215
						if(!$version->WasPublished) $showUnpublishedChecked = 1;
216
					}
217
218
					$version->Active = ($active);
219
				}
220
			}
221
222
			$vd = new ViewableData();
223
224
			$versionsHtml = $vd->customise(array(
225
				'Versions' => $versions
226
			))->renderWith('CMSPageHistoryController_versions');
227
		}
228
229
		$fields = new FieldList(
230
			new CheckboxField(
231
				'ShowUnpublished',
232
				_t('CMSPageHistoryController.SHOWUNPUBLISHED','Show unpublished versions'),
233
				$showUnpublishedChecked
234
			),
235
			new CheckboxField(
236
				'CompareMode',
237
				_t('CMSPageHistoryController.COMPAREMODE', 'Compare mode (select two)'),
238
				$compareModeChecked
239
			),
240
			new LiteralField('VersionsHtml', $versionsHtml),
241
			$hiddenID = new HiddenField('ID', false, "")
242
		);
243
244
		$actions = new FieldList(
245
			new FormAction(
246
				'doCompare', _t('CMSPageHistoryController.COMPAREVERSIONS','Compare Versions')
247
			),
248
			new FormAction(
249
				'doShowVersion', _t('CMSPageHistoryController.SHOWVERSION','Show Version')
250
			)
251
		);
252
253
		// Use <button> to allow full jQuery UI styling
254
		foreach($actions->dataFields() as $action) $action->setUseButtonTag(true);
255
256
		$negotiator = $this->getResponseNegotiator();
0 ignored issues
show
Unused Code introduced by
$negotiator is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
257
		$form = Form::create(
258
			$this,
259
			'VersionsForm',
260
			$fields,
261
			$actions
262
		)->setHTMLID('Form_VersionsForm');
263
		$form->loadDataFrom($this->getRequest()->requestVars());
264
		$hiddenID->setValue($id);
265
		$form->unsetValidator();
266
267
		$form
268
			->addExtraClass('cms-versions-form') // placeholder, necessary for $.metadata() to work
269
			->setAttribute('data-link-tmpl-compare', Controller::join_links($this->Link('compare'), '%s', '%s', '%s'))
270
			->setAttribute('data-link-tmpl-show', Controller::join_links($this->Link('show'), '%s', '%s'));
271
272
		return $form;
273
	}
274
275
	/**
276
	 * Process the {@link VersionsForm} compare function between two pages.
277
	 *
278
	 * @param array
279
	 * @param Form
280
	 *
281
	 * @return html
282
	 */
283
	public function doCompare($data, $form) {
284
		$versions = $data['Versions'];
285
		if(count($versions) < 2) return null;
286
287
		$id = $this->currentPageID();
0 ignored issues
show
Unused Code introduced by
$id is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
288
		$version1 = array_shift($versions);
289
		$version2 = array_shift($versions);
290
291
		$form = $this->CompareVersionsForm($version1, $version2);
292
293
		// javascript solution, render into template
294 View Code Duplication
		if($this->getRequest()->isAjax()) {
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
295
			return $this->customise(array(
296
				"EditForm" => $form
297
			))->renderWith(array(
298
				$this->class . '_EditForm',
299
				'LeftAndMain_Content'
300
			));
301
		}
302
303
		// non javascript, redirect the user to the page
304
		$this->redirect(Controller::join_links(
305
			$this->Link('compare'),
306
			$version1,
307
			$version2
308
		));
309
	}
310
311
	/**
312
	 * Process the {@link VersionsForm} show version function. Only requires
313
	 * one page to be selected.
314
	 *
315
	 * @param array
316
	 * @param Form
317
	 *
318
	 * @return html
319
	 */
320
	public function doShowVersion($data, $form) {
321
		$versionID = null;
322
323
		if(isset($data['Versions']) && is_array($data['Versions'])) {
324
			$versionID  = array_shift($data['Versions']);
325
		}
326
327
		if(!$versionID) return;
328
329 View Code Duplication
		if($request->isAjax()) {
0 ignored issues
show
Bug introduced by
The variable $request does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
Duplication introduced by
This code seems to be duplicated across your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
330
			return $this->customise(array(
331
				"EditForm" => $this->ShowVersionForm($versionID)
332
			))->renderWith(array(
333
				$this->class . '_EditForm',
334
				'LeftAndMain_Content'
335
			));
336
		}
337
338
		// non javascript, redirect the user to the page
339
		$this->redirect(Controller::join_links(
340
			$this->Link('version'),
341
			$versionID
342
		));
343
	}
344
345
	/**
346
	 * @param int|null $versionID
347
	 * @return Form
348
	 */
349
	public function ShowVersionForm($versionID = null) {
350
		if(!$versionID) return null;
0 ignored issues
show
Bug Best Practice introduced by
The expression $versionID of type integer|null is loosely compared to false; this is ambiguous if the integer can be zero. You might want to explicitly use === null instead.

In PHP, under loose comparison (like ==, or !=, or switch conditions), values of different types might be equal.

For integer values, zero is a special case, in particular the following results might be unexpected:

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
Loading history...
351
352
		$id = $this->currentPageID();
353
		$form = $this->getEditForm($id, null, $versionID);
354
355
		return $form;
356
	}
357
358
	/**
359
	 * @param int $versionID
360
	 * @param int $otherVersionID
361
	 * @return Form
362
	 */
363
	public function CompareVersionsForm($versionID, $otherVersionID) {
364
		if($versionID > $otherVersionID) {
365
			$toVersion = $versionID;
366
			$fromVersion = $otherVersionID;
367
		} else {
368
			$toVersion = $otherVersionID;
369
			$fromVersion = $versionID;
370
		}
371
372
		if(!$toVersion || !$fromVersion) return false;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return false; (false) is incompatible with the return type documented by CMSPageHistoryController::CompareVersionsForm of type Form.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

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());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
373
374
		$id = $this->currentPageID();
375
		$page = DataObject::get_by_id("SiteTree", $id);
376
377
		if($page && !$page->canView()) {
378
			return Security::permissionFailure($this);
379
		}
380
381
		$record = $page->compareVersions($fromVersion, $toVersion);
382
383
		$fromVersionRecord = Versioned::get_version('SiteTree', $id, $fromVersion);
384
		$toVersionRecord = Versioned::get_version('SiteTree', $id, $toVersion);
385
386
		if(!$fromVersionRecord) {
387
			user_error("Can't find version $fromVersion of page $id", E_USER_ERROR);
388
		}
389
390
		if(!$toVersionRecord) {
391
			user_error("Can't find version $toVersion of page $id", E_USER_ERROR);
392
		}
393
394
		if($record) {
395
			$form = $this->getEditForm($id, null, null, true);
396
			$form->setActions(new FieldList());
397
			$form->addExtraClass('compare');
398
399
			// Comparison views shouldn't be editable.
400
			// Its important to convert fields *before* loading data,
401
			// as the comparison output is HTML and not valid values for the various field types
402
			$readonlyFields = $form->Fields()->makeReadonly();
403
			$form->setFields($readonlyFields);
404
405
			$form->loadDataFrom($record);
406
			$form->loadDataFrom(array(
407
				"ID" => $id,
408
				"Version" => $fromVersion,
409
			));
410
411
			foreach($form->Fields()->dataFields() as $field) {
412
				$field->dontEscape = true;
413
			}
414
415
			return $form;
416
		}
417
	}
418
419
	public function Breadcrumbs($unlinked = false) {
420
		$crumbs = parent::Breadcrumbs($unlinked);
421
		$crumbs[0]->Title = _t('CMSPagesController.MENUTITLE');
422
		return $crumbs;
423
	}
424
425
}
426