Completed
Push — master ( 8e9a24...5a1675 )
by Daniel
12s
created

CMSPageHistoryController::ShowVersionForm()   A

Complexity

Conditions 2
Paths 2

Size

Total Lines 8
Code Lines 5

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
dl 0
loc 8
rs 9.4285
c 0
b 0
f 0
cc 2
eloc 5
nc 2
nop 1
1
<?php
2
3
use SilverStripe\ORM\DataObject;
4
use SilverStripe\ORM\Versioning\Versioned;
5
use SilverStripe\Security\Security;
6
7
/**
8
 * @package cms
9
 * @subpackage controllers
10
 */
11
class CMSPageHistoryController extends CMSMain {
12
13
	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...
14
	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...
15
	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...
16
	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...
17
	private static $required_permission_codes = 'CMS_ACCESS_CMSMain';
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...
18
	private static $session_namespace = 'CMSMain';
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...
19
20
	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...
21
		'VersionsForm',
22
		'CompareVersionsForm',
23
		'show',
24
		'compare'
25
	);
26
27
	private static $url_handlers = 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...
28
		'$Action/$ID/$VersionID/$OtherVersionID' => 'handleAction'
29
	);
30
31
	public function getResponseNegotiator() {
32
		$negotiator = parent::getResponseNegotiator();
33
		$controller = $this;
34
		$negotiator->setCallback('CurrentForm', function() use(&$controller) {
35
			$form = $controller->ShowVersionForm($controller->getRequest()->param('VersionID'));
36
			if($form) return $form->forTemplate();
37
			else return $controller->renderWith($controller->getTemplatesWithSuffix('_Content'));
38
		});
39
		$negotiator->setCallback('default', function() use(&$controller) {
40
			return $controller->renderWith($controller->getViewer('show'));
41
		});
42
		return $negotiator;
43
	}
44
45
	/**
46
	 * @param SS_HTTPRequest $request
47
	 * @return array
48
	 */
49 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...
50
		$form = $this->ShowVersionForm($request->param('VersionID'));
51
52
		$negotiator = $this->getResponseNegotiator();
53
		$controller = $this;
54
		$negotiator->setCallback('CurrentForm', function() use(&$controller, &$form) {
55
			return $form ? $form->forTemplate() : $controller->renderWith($controller->getTemplatesWithSuffix('_Content'));
56
		});
57
		$negotiator->setCallback('default', function() use(&$controller, &$form) {
58
			return $controller->customise(array('EditForm' => $form))->renderWith($controller->getViewer('show'));
59
		});
60
61
		return $negotiator->respond($request);
62
	}
63
64
	/**
65
	 * @param SS_HTTPRequest $request
66
	 * @return array
67
	 */
68 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...
69
		$form = $this->CompareVersionsForm(
70
			$request->param('VersionID'),
71
			$request->param('OtherVersionID')
72
		);
73
74
		$negotiator = $this->getResponseNegotiator();
75
		$controller = $this;
76
		$negotiator->setCallback('CurrentForm', function() use(&$controller, &$form) {
77
			return $form ? $form->forTemplate() : $controller->renderWith($controller->getTemplatesWithSuffix('_Content'));
78
		});
79
		$negotiator->setCallback('default', function() use(&$controller, &$form) {
80
			return $controller->customise(array('EditForm' => $form))->renderWith($controller->getViewer('show'));
81
		});
82
83
		return $negotiator->respond($request);
84
	}
85
86
	public function getSilverStripeNavigator() {
87
		$record = $this->getRecord($this->currentPageID(), $this->getRequest()->param('VersionID'));
88
		if($record) {
89
			$navigator = new SilverStripeNavigator($record);
90
			return $navigator->renderWith($this->getTemplatesWithSuffix('_SilverStripeNavigator'));
0 ignored issues
show
Bug Best Practice introduced by
The return type of return $navigator->rende...lverStripeNavigator')); (SilverStripe\ORM\FieldType\DBHTMLText) is incompatible with the return type of the parent method LeftAndMain::getSilverStripeNavigator of type ArrayData.

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...
91
		} else {
92
			return false;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return false; (false) is incompatible with the return type of the parent method LeftAndMain::getSilverStripeNavigator of type ArrayData.

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...
93
		}
94
	}
95
96
	/**
97
	 * Returns the read only version of the edit form. Detaches all {@link FormAction}
98
	 * instances attached since only action relates to revert.
99
	 *
100
	 * Permission checking is done at the {@link CMSMain::getEditForm()} level.
101
	 *
102
	 * @param int $id ID of the record to show
103
	 * @param array $fields optional
104
	 * @param int $versionID
105
	 * @param int $compareID Compare mode
106
	 *
107
	 * @return Form
108
	 */
109
	public function getEditForm($id = null, $fields = null, $versionID = null, $compareID = null) {
110
		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...
111
112
		$record = $this->getRecord($id, $versionID);
113
		$versionID = ($record) ? $record->Version : $versionID;
0 ignored issues
show
Bug introduced by
The property Version does not seem to exist. Did you mean versioning?

An attempt at access to an undefined property has been detected. This may either be a typographical error or the property has been renamed but there are still references to its old name.

If you really want to allow access to undefined properties, you can define magic methods to allow access. See the php core documentation on Overloading.

Loading history...
114
115
		$form = parent::getEditForm($record, ($record) ? $record->getCMSFields() : null);
0 ignored issues
show
Documentation introduced by
$record is of type object<SiteTree>, but the function expects a integer|null.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

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

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
116
		// Respect permission failures from parent implementation
117
		if(!($form instanceof Form)) return $form;
118
119
		// TODO: move to the SilverStripeNavigator structure so the new preview can pick it up.
120
		//$nav = new SilverStripeNavigatorItem_ArchiveLink($record);
121
122
		$form->setActions(new FieldList(
123
			$revert = FormAction::create('doRollback', _t('CMSPageHistoryController.REVERTTOTHISVERSION', 'Revert to this version'))->setUseButtonTag(true)
124
		));
125
126
		$fields = $form->Fields();
127
		$fields->removeByName("Status");
128
		$fields->push(new HiddenField("ID"));
129
		$fields->push(new HiddenField("Version"));
130
131
		$fields = $fields->makeReadonly();
132
133
		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...
134
			$link = Controller::join_links(
135
				$this->Link('show'),
136
				$id
137
			);
138
139
			$view = _t('CMSPageHistoryController.VIEW',"view");
140
141
			$message = _t(
142
				'CMSPageHistoryController.COMPARINGVERSION',
143
				"Comparing versions {version1} and {version2}.",
144
				array(
0 ignored issues
show
Documentation introduced by
array('version1' => spri...k, $compareID), $view)) is of type array<string,string,{"ve...","version2":"string"}>, but the function expects a string.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

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

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
145
					'version1' => sprintf('%s (<a href="%s">%s</a>)', $versionID, Controller::join_links($link, $versionID), $view),
146
					'version2' => sprintf('%s (<a href="%s">%s</a>)', $compareID, Controller::join_links($link, $compareID), $view)
147
				)
148
			);
149
150
			$revert->setReadonly(true);
151
		} else {
152
			if($record->isLatestVersion()) {
0 ignored issues
show
Documentation Bug introduced by
The method isLatestVersion does not exist on object<SiteTree>? Since you implemented __call, maybe consider adding a @method annotation.

If you implement __call and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class.

This is often the case, when __call is implemented by a parent class and only the child class knows which methods exist:

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 { }
Loading history...
153
				$message = _t('CMSPageHistoryController.VIEWINGLATEST', 'Currently viewing the latest version.');
154
			} else {
155
				$message = _t(
156
					'CMSPageHistoryController.VIEWINGVERSION',
157
					"Currently viewing version {version}.",
158
					array('version' => $versionID)
0 ignored issues
show
Documentation introduced by
array('version' => $versionID) is of type array<string,?,{"version":"?"}>, but the function expects a string.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

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

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
159
				);
160
			}
161
		}
162
163
		$fields->addFieldToTab('Root.Main',
164
			new LiteralField('CurrentlyViewingMessage', $this->customise(array(
165
				'Content' => $message,
166
				'Classes' => 'notice'
167
			))->renderWith(array('CMSMain_notice'))),
168
			"Title"
169
		);
170
171
		$form->setFields($fields->makeReadonly());
172
		$form->loadDataFrom(array(
173
			"ID" => $id,
174
			"Version" => $versionID,
175
		));
176
177
		if(($record && $record->isLatestVersion())) {
0 ignored issues
show
Documentation Bug introduced by
The method isLatestVersion does not exist on object<SiteTree>? Since you implemented __call, maybe consider adding a @method annotation.

If you implement __call and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class.

This is often the case, when __call is implemented by a parent class and only the child class knows which methods exist:

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 { }
Loading history...
178
			$revert->setReadonly(true);
179
		}
180
181
		$form->removeExtraClass('cms-content');
182
183
		return $form;
184
	}
185
186
187
	/**
188
	 * Version select form. Main interface between selecting versions to view
189
	 * and comparing multiple versions.
190
	 *
191
	 * Because we can reload the page directly to a compare view (history/compare/1/2/3)
192
	 * this form has to adapt to those parameters as well.
193
	 *
194
	 * @return Form
195
	 */
196
	public function VersionsForm() {
197
		$id = $this->currentPageID();
198
		$page = $this->getRecord($id);
199
		$versionsHtml = '';
200
201
		$action = $this->getRequest()->param('Action');
202
		$versionID = $this->getRequest()->param('VersionID');
203
		$otherVersionID = $this->getRequest()->param('OtherVersionID');
204
205
		$showUnpublishedChecked = 0;
206
		$compareModeChecked = ($action == "compare");
207
208
		if($page) {
209
			$versions = $page->allVersions();
0 ignored issues
show
Documentation Bug introduced by
The method allVersions does not exist on object<SiteTree>? Since you implemented __call, maybe consider adding a @method annotation.

If you implement __call and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class.

This is often the case, when __call is implemented by a parent class and only the child class knows which methods exist:

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 { }
Loading history...
210
			$versionID = (!$versionID) ? $page->Version : $versionID;
0 ignored issues
show
Bug introduced by
The property Version does not seem to exist. Did you mean versioning?

An attempt at access to an undefined property has been detected. This may either be a typographical error or the property has been renamed but there are still references to its old name.

If you really want to allow access to undefined properties, you can define magic methods to allow access. See the php core documentation on Overloading.

Loading history...
211
212
			if($versions) {
213
				foreach($versions as $k => $version) {
214
					$active = false;
215
216
					if($version->Version == $versionID || $version->Version == $otherVersionID) {
217
						$active = true;
218
219
						if(!$version->WasPublished) $showUnpublishedChecked = 1;
220
					}
221
222
					$version->Active = ($active);
223
				}
224
			}
225
226
			$vd = new ViewableData();
227
228
			$versionsHtml = $vd->customise(array(
229
				'Versions' => $versions
230
			))->renderWith('CMSPageHistoryController_versions');
231
		}
232
233
		$fields = new FieldList(
234
			new CheckboxField(
235
				'ShowUnpublished',
236
				_t('CMSPageHistoryController.SHOWUNPUBLISHED','Show unpublished versions'),
237
				$showUnpublishedChecked
238
			),
239
			new CheckboxField(
240
				'CompareMode',
241
				_t('CMSPageHistoryController.COMPAREMODE', 'Compare mode (select two)'),
242
				$compareModeChecked
243
			),
244
			new LiteralField('VersionsHtml', $versionsHtml),
0 ignored issues
show
Bug introduced by
It seems like $versionsHtml defined by $vd->customise(array('Ve...ryController_versions') on line 228 can also be of type object<SilverStripe\ORM\FieldType\DBHTMLText>; however, LiteralField::__construct() does only seem to accept string|object<FormField>, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
245
			$hiddenID = new HiddenField('ID', false, "")
0 ignored issues
show
Documentation introduced by
false is of type boolean, but the function expects a null|string.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

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

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
246
		);
247
248
		$actions = new FieldList(
249
			new FormAction(
250
				'doCompare', _t('CMSPageHistoryController.COMPAREVERSIONS','Compare Versions')
251
			),
252
			new FormAction(
253
				'doShowVersion', _t('CMSPageHistoryController.SHOWVERSION','Show Version')
254
			)
255
		);
256
257
		// Use <button> to allow full jQuery UI styling
258
		foreach($actions->dataFields() as $action) $action->setUseButtonTag(true);
259
260
		$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...
261
		$form = Form::create(
262
			$this,
263
			'VersionsForm',
264
			$fields,
265
			$actions
266
		)->setHTMLID('Form_VersionsForm');
267
		$form->loadDataFrom($this->getRequest()->requestVars());
0 ignored issues
show
Bug introduced by
It seems like $this->getRequest()->requestVars() targeting SS_HTTPRequest::requestVars() can also be of type null; however, Form::loadDataFrom() does only seem to accept array|object<SilverStripe\ORM\DataObject>, maybe add an additional type check?

This check looks at variables that are passed out again to other methods.

If the outgoing method call has stricter type requirements than the method itself, an issue is raised.

An additional type check may prevent trouble.

Loading history...
268
		$hiddenID->setValue($id);
269
		$form->unsetValidator();
270
271
		$form
272
			->addExtraClass('cms-versions-form') // placeholder, necessary for $.metadata() to work
273
			->setAttribute('data-link-tmpl-compare', Controller::join_links($this->Link('compare'), '%s', '%s', '%s'))
274
			->setAttribute('data-link-tmpl-show', Controller::join_links($this->Link('show'), '%s', '%s'));
275
276
		return $form;
277
	}
278
279
	/**
280
	 * Process the {@link VersionsForm} compare function between two pages.
281
	 *
282
	 * @param array
283
	 * @param Form
284
	 *
285
	 * @return html
286
	 */
287
	public function doCompare($data, $form) {
0 ignored issues
show
Unused Code introduced by
The parameter $form is not used and could be removed.

This check looks from parameters that have been defined for a function or method, but which are not used in the method body.

Loading history...
288
		$versions = $data['Versions'];
289
		if(count($versions) < 2) return null;
290
291
		$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...
292
		$version1 = array_shift($versions);
293
		$version2 = array_shift($versions);
294
295
		$form = $this->CompareVersionsForm($version1, $version2);
296
297
		// javascript solution, render into template
298 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...
299
			return $this->customise(array(
300
				"EditForm" => $form
301
			))->renderWith(array(
302
				$this->class . '_EditForm',
303
				'LeftAndMain_Content'
304
			));
305
		}
306
307
		// non javascript, redirect the user to the page
308
		$this->redirect(Controller::join_links(
309
			$this->Link('compare'),
310
			$version1,
311
			$version2
312
		));
313
	}
314
315
	/**
316
	 * Process the {@link VersionsForm} show version function. Only requires
317
	 * one page to be selected.
318
	 *
319
	 * @param array
320
	 * @param Form
321
	 *
322
	 * @return html
323
	 */
324
	public function doShowVersion($data, $form) {
0 ignored issues
show
Unused Code introduced by
The parameter $form is not used and could be removed.

This check looks from parameters that have been defined for a function or method, but which are not used in the method body.

Loading history...
325
		$versionID = null;
326
327
		if(isset($data['Versions']) && is_array($data['Versions'])) {
328
			$versionID  = array_shift($data['Versions']);
329
		}
330
331
		if(!$versionID) return;
332
333 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...
334
			return $this->customise(array(
335
				"EditForm" => $this->ShowVersionForm($versionID)
336
			))->renderWith(array(
337
				$this->class . '_EditForm',
338
				'LeftAndMain_Content'
339
			));
340
		}
341
342
		// non javascript, redirect the user to the page
343
		$this->redirect(Controller::join_links(
344
			$this->Link('version'),
345
			$versionID
346
		));
347
	}
348
349
	/**
350
	 * @param int|null $versionID
351
	 * @return Form
352
	 */
353
	public function ShowVersionForm($versionID = null) {
354
		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...
355
356
		$id = $this->currentPageID();
357
		$form = $this->getEditForm($id, null, $versionID);
358
359
		return $form;
360
	}
361
362
	/**
363
	 * @param int $versionID
364
	 * @param int $otherVersionID
365
	 * @return mixed
366
	 */
367
	public function CompareVersionsForm($versionID, $otherVersionID) {
368
		if($versionID > $otherVersionID) {
369
			$toVersion = $versionID;
370
			$fromVersion = $otherVersionID;
371
		} else {
372
			$toVersion = $otherVersionID;
373
			$fromVersion = $versionID;
374
		}
375
376
		if(!$toVersion || !$toVersion) {
377
			return false;
378
		}
379
380
		$id = $this->currentPageID();
381
		$page = DataObject::get_by_id("SiteTree", $id);
382
383
 		if($page && $page->exists()) {
384
			if(!$page->canView()) {
385
				return Security::permissionFailure($this);
386
			}
387
388
			$record = $page->compareVersions($fromVersion, $toVersion);
389
		}
390
391
		$fromVersionRecord = Versioned::get_version('SiteTree', $id, $fromVersion);
392
		$toVersionRecord = Versioned::get_version('SiteTree', $id, $toVersion);
393
394
		if(!$fromVersionRecord) {
395
			user_error("Can't find version $fromVersion of page $id", E_USER_ERROR);
396
		}
397
398
		if(!$toVersionRecord) {
399
			user_error("Can't find version $toVersion of page $id", E_USER_ERROR);
400
		}
401
402
		if(isset($record)) {
403
			$form = $this->getEditForm($id, null, null, true);
0 ignored issues
show
Documentation introduced by
true is of type boolean, but the function expects a integer|null.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

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

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
404
			$form->setActions(new FieldList());
405
			$form->addExtraClass('compare');
406
407
			// Comparison views shouldn't be editable.
408
			// Its important to convert fields *before* loading data,
409
			// as the comparison output is HTML and not valid values for the various field types
410
			$readonlyFields = $form->Fields()->makeReadonly();
411
			$form->setFields($readonlyFields);
412
413
			$form->loadDataFrom($record);
414
			$form->loadDataFrom(array(
415
				"ID" => $id,
416
				"Version" => $fromVersion,
417
			));
418
419
			foreach($form->Fields()->dataFields() as $field) {
420
				$field->dontEscape = true;
421
			}
422
423
			return $form;
424
		}
425
426
        	return false;
427
	}
428
429
	public function Breadcrumbs($unlinked = false) {
430
		$crumbs = parent::Breadcrumbs($unlinked);
431
		$crumbs[0]->Title = _t('CMSPagesController.MENUTITLE');
432
		return $crumbs;
433
	}
434
435
}
436