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:
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.
| 1 | <?php |
||
| 7 | class CMSPageHistoryController extends CMSMain { |
||
|
|
|||
| 8 | |||
| 9 | private static $url_segment = 'pages/history'; |
||
| 10 | private static $url_rule = '/$Action/$ID/$VersionID/$OtherVersionID'; |
||
| 11 | private static $url_priority = 42; |
||
| 12 | private static $menu_title = 'History'; |
||
| 13 | private static $required_permission_codes = 'CMS_ACCESS_CMSMain'; |
||
| 14 | private static $session_namespace = 'CMSMain'; |
||
| 15 | |||
| 16 | private static $allowed_actions = array( |
||
| 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() { |
||
| 40 | |||
| 41 | /** |
||
| 42 | * @param SS_HTTPRequest $request |
||
| 43 | * @return array |
||
| 44 | */ |
||
| 45 | View Code Duplication | public function show($request) { |
|
| 59 | |||
| 60 | /** |
||
| 61 | * @param SS_HTTPRequest $request |
||
| 62 | * @return array |
||
| 63 | */ |
||
| 64 | View Code Duplication | public function compare($request) { |
|
| 81 | |||
| 82 | public function getSilverStripeNavigator() { |
||
| 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(); |
||
| 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; |
||
| 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) { |
||
| 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(); |
||
| 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(); |
||
| 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()) { |
|
| 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()) { |
|
| 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) { |
||
| 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; |
||
| 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) { |
||
| 424 | |||
| 425 | } |
||
| 426 |