Complex classes like GridFieldDetailForm_ItemRequest 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 GridFieldDetailForm_ItemRequest, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 228 | class GridFieldDetailForm_ItemRequest extends RequestHandler { |
||
| 229 | |||
| 230 | private static $allowed_actions = array( |
||
| 231 | 'edit', |
||
| 232 | 'view', |
||
| 233 | 'ItemEditForm' |
||
| 234 | ); |
||
| 235 | |||
| 236 | /** |
||
| 237 | * |
||
| 238 | * @var GridField |
||
| 239 | */ |
||
| 240 | protected $gridField; |
||
| 241 | |||
| 242 | /** |
||
| 243 | * |
||
| 244 | * @var GridField_URLHandler |
||
| 245 | */ |
||
| 246 | protected $component; |
||
| 247 | |||
| 248 | /** |
||
| 249 | * |
||
| 250 | * @var DataObject |
||
| 251 | */ |
||
| 252 | protected $record; |
||
| 253 | |||
| 254 | /** |
||
| 255 | * This represents the current parent RequestHandler (which does not necessarily need to be a Controller). |
||
| 256 | * It allows us to traverse the RequestHandler chain upwards to reach the Controller stack. |
||
| 257 | * |
||
| 258 | * @var RequestHandler |
||
| 259 | */ |
||
| 260 | protected $popupController; |
||
| 261 | |||
| 262 | /** |
||
| 263 | * |
||
| 264 | * @var string |
||
| 265 | */ |
||
| 266 | protected $popupFormName; |
||
| 267 | |||
| 268 | /** |
||
| 269 | * @var String |
||
| 270 | */ |
||
| 271 | protected $template = 'GridFieldItemEditView'; |
||
| 272 | |||
| 273 | private static $url_handlers = array( |
||
| 274 | '$Action!' => '$Action', |
||
| 275 | '' => 'edit', |
||
| 276 | ); |
||
| 277 | |||
| 278 | /** |
||
| 279 | * |
||
| 280 | * @param GridFIeld $gridField |
||
| 281 | * @param GridField_URLHandler $component |
||
| 282 | * @param DataObject $record |
||
| 283 | * @param RequestHandler $requestHandler |
||
| 284 | * @param string $popupFormName |
||
| 285 | */ |
||
| 286 | public function __construct($gridField, $component, $record, $requestHandler, $popupFormName) { |
||
| 294 | |||
| 295 | public function Link($action = null) { |
||
| 299 | |||
| 300 | public function view($request) { |
||
| 322 | |||
| 323 | public function edit($request) { |
||
| 342 | |||
| 343 | /** |
||
| 344 | * Builds an item edit form. The arguments to getCMSFields() are the popupController and |
||
| 345 | * popupFormName, however this is an experimental API and may change. |
||
| 346 | * |
||
| 347 | * @todo In the future, we will probably need to come up with a tigher object representing a partially |
||
| 348 | * complete controller with gaps for extra functionality. This, for example, would be a better way |
||
| 349 | * of letting Security/login put its log-in form inside a UI specified elsewhere. |
||
| 350 | * |
||
| 351 | * @return Form |
||
| 352 | */ |
||
| 353 | public function ItemEditForm() { |
||
| 354 | $list = $this->gridField->getList(); |
||
| 355 | |||
| 356 | if (empty($this->record)) { |
||
| 357 | $controller = $this->getToplevelController(); |
||
| 358 | $url = $controller->getRequest()->getURL(); |
||
| 359 | $noActionURL = $controller->removeAction($url); |
||
| 360 | $controller->getResponse()->removeHeader('Location'); //clear the existing redirect |
||
| 361 | return $controller->redirect($noActionURL, 302); |
||
| 362 | } |
||
| 363 | |||
| 364 | $canView = $this->record->canView(); |
||
| 365 | $canEdit = $this->record->canEdit(); |
||
| 366 | $canDelete = $this->record->canDelete(); |
||
| 367 | $canCreate = $this->record->canCreate(); |
||
| 368 | |||
| 369 | if(!$canView) { |
||
| 370 | $controller = $this->getToplevelController(); |
||
| 371 | // TODO More friendly error |
||
| 372 | return $controller->httpError(403); |
||
| 373 | } |
||
| 374 | |||
| 375 | // Build actions |
||
| 376 | $actions = $this->getFormActions(); |
||
| 377 | |||
| 378 | // If we are creating a new record in a has-many list, then |
||
| 379 | // pre-populate the record's foreign key. |
||
| 380 | if($list instanceof HasManyList && !$this->record->isInDB()) { |
||
| 381 | $key = $list->getForeignKey(); |
||
| 382 | $id = $list->getForeignID(); |
||
| 383 | $this->record->$key = $id; |
||
| 384 | } |
||
| 385 | |||
| 386 | $fields = $this->component->getFields(); |
||
| 387 | if(!$fields) $fields = $this->record->getCMSFields(); |
||
| 388 | |||
| 389 | // If we are creating a new record in a has-many list, then |
||
| 390 | // Disable the form field as it has no effect. |
||
| 391 | if($list instanceof HasManyList) { |
||
| 392 | $key = $list->getForeignKey(); |
||
| 393 | |||
| 394 | if($field = $fields->dataFieldByName($key)) { |
||
| 395 | $fields->makeFieldReadonly($field); |
||
| 396 | } |
||
| 397 | } |
||
| 398 | |||
| 399 | // Caution: API violation. Form expects a Controller, but we are giving it a RequestHandler instead. |
||
| 400 | // Thanks to this however, we are able to nest GridFields, and also access the initial Controller by |
||
| 401 | // dereferencing GridFieldDetailForm_ItemRequest->getController() multiple times. See getToplevelController |
||
| 402 | // below. |
||
| 403 | $form = new Form( |
||
| 404 | $this, |
||
| 405 | 'ItemEditForm', |
||
| 406 | $fields, |
||
| 407 | $actions, |
||
| 408 | $this->component->getValidator() |
||
| 409 | ); |
||
| 410 | |||
| 411 | $form->loadDataFrom($this->record, $this->record->ID == 0 ? Form::MERGE_IGNORE_FALSEISH : Form::MERGE_DEFAULT); |
||
| 412 | |||
| 413 | if($this->record->ID && !$canEdit) { |
||
| 414 | // Restrict editing of existing records |
||
| 415 | $form->makeReadonly(); |
||
| 416 | // Hack to re-enable delete button if user can delete |
||
| 417 | if ($canDelete) { |
||
| 418 | $form->Actions()->fieldByName('action_doDelete')->setReadonly(false); |
||
| 419 | } |
||
| 420 | } elseif(!$this->record->ID && !$canCreate) { |
||
| 421 | // Restrict creation of new records |
||
| 422 | $form->makeReadonly(); |
||
| 423 | } |
||
| 424 | |||
| 425 | // Load many_many extraData for record. |
||
| 426 | // Fields with the correct 'ManyMany' namespace need to be added manually through getCMSFields(). |
||
| 427 | if($list instanceof ManyManyList) { |
||
| 428 | $extraData = $list->getExtraData('', $this->record->ID); |
||
| 429 | $form->loadDataFrom(array('ManyMany' => $extraData)); |
||
| 430 | } |
||
| 431 | |||
| 432 | // TODO Coupling with CMS |
||
| 433 | $toplevelController = $this->getToplevelController(); |
||
| 434 | if($toplevelController && $toplevelController instanceof LeftAndMain) { |
||
| 435 | // Always show with base template (full width, no other panels), |
||
| 436 | // regardless of overloaded CMS controller templates. |
||
| 437 | // TODO Allow customization, e.g. to display an edit form alongside a search form from the CMS controller |
||
| 438 | $form->setTemplate('LeftAndMain_EditForm'); |
||
| 439 | $form->addExtraClass('cms-content cms-edit-form center'); |
||
| 440 | $form->setAttribute('data-pjax-fragment', 'CurrentForm Content'); |
||
| 441 | if($form->Fields()->hasTabset()) { |
||
| 442 | $form->Fields()->findOrMakeTab('Root')->setTemplate('CMSTabSet'); |
||
| 443 | $form->addExtraClass('cms-tabset'); |
||
| 444 | } |
||
| 445 | |||
| 446 | $form->Backlink = $this->getBackLink(); |
||
| 447 | } |
||
| 448 | |||
| 449 | $cb = $this->component->getItemEditFormCallback(); |
||
| 450 | if($cb) $cb($form, $this); |
||
| 451 | $this->extend("updateItemEditForm", $form); |
||
| 452 | return $form; |
||
| 453 | } |
||
| 454 | |||
| 455 | /** |
||
| 456 | * Build the set of form field actions for this DataObject |
||
| 457 | * |
||
| 458 | * @return FieldList |
||
| 459 | */ |
||
| 460 | protected function getFormActions() { |
||
| 501 | |||
| 502 | /** |
||
| 503 | * Traverse the nested RequestHandlers until we reach something that's not GridFieldDetailForm_ItemRequest. |
||
| 504 | * This allows us to access the Controller responsible for invoking the top-level GridField. |
||
| 505 | * This should be equivalent to getting the controller off the top of the controller stack via Controller::curr(), |
||
| 506 | * but allows us to avoid accessing the global state. |
||
| 507 | * |
||
| 508 | * GridFieldDetailForm_ItemRequests are RequestHandlers, and as such they are not part of the controller stack. |
||
| 509 | * |
||
| 510 | * @return Controller |
||
| 511 | */ |
||
| 512 | protected function getToplevelController() { |
||
| 519 | |||
| 520 | protected function getBackLink(){ |
||
| 536 | |||
| 537 | /** |
||
| 538 | * Get the list of extra data from the $record as saved into it by |
||
| 539 | * {@see Form::saveInto()} |
||
| 540 | * |
||
| 541 | * Handles detection of falsey values explicitly saved into the |
||
| 542 | * DataObject by formfields |
||
| 543 | * |
||
| 544 | * @param DataObject $record |
||
| 545 | * @param SS_List $list |
||
| 546 | * @return array List of data to write to the relation |
||
| 547 | */ |
||
| 548 | protected function getExtraSavedData($record, $list) { |
||
| 563 | |||
| 564 | public function doSave($data, $form) { |
||
| 596 | |||
| 597 | /** |
||
| 598 | * Response object for this request after a successful save |
||
| 599 | * |
||
| 600 | * @param bool $isNewRecord True if this record was just created |
||
| 601 | * @return SS_HTTPResponse|HTMLText |
||
| 602 | */ |
||
| 603 | protected function redirectAfterSave($isNewRecord) { |
||
| 620 | |||
| 621 | public function httpError($errorCode, $errorMessage = null) { |
||
| 625 | |||
| 626 | /** |
||
| 627 | * Loads the given form data into the underlying dataobject and relation |
||
| 628 | * |
||
| 629 | * @param array $data |
||
| 630 | * @param Form $form |
||
| 631 | * @throws ValidationException On error |
||
| 632 | * @return DataObject Saved record |
||
| 633 | */ |
||
| 634 | protected function saveFormIntoRecord($data, $form) { |
||
| 656 | |||
| 657 | /** |
||
| 658 | * Generate a response object for a form validation error |
||
| 659 | * |
||
| 660 | * @param Form $form The source form |
||
| 661 | * @param ValidationException $e The validation error message |
||
| 662 | * @return SS_HTTPResponse |
||
| 663 | * @throws SS_HTTPResponse_Exception |
||
| 664 | */ |
||
| 665 | protected function generateValidationResponse($form, $e) { |
||
| 682 | |||
| 683 | |||
| 684 | public function doDelete($data, $form) { |
||
| 718 | |||
| 719 | /** |
||
| 720 | * @param String |
||
| 721 | */ |
||
| 722 | public function setTemplate($template) { |
||
| 726 | |||
| 727 | /** |
||
| 728 | * @return String |
||
| 729 | */ |
||
| 730 | public function getTemplate() { |
||
| 733 | |||
| 734 | /** |
||
| 735 | * @return Controller |
||
| 736 | */ |
||
| 737 | public function getController() { |
||
| 740 | |||
| 741 | /** |
||
| 742 | * @return GridField |
||
| 743 | */ |
||
| 744 | public function getGridField() { |
||
| 747 | |||
| 748 | /** |
||
| 749 | * @return DataObject |
||
| 750 | */ |
||
| 751 | public function getRecord() { |
||
| 754 | |||
| 755 | /** |
||
| 756 | * CMS-specific functionality: Passes through navigation breadcrumbs |
||
| 757 | * to the template, and includes the currently edited record (if any). |
||
| 758 | * see {@link LeftAndMain->Breadcrumbs()} for details. |
||
| 759 | * |
||
| 760 | * @param boolean $unlinked |
||
| 761 | * @return ArrayData |
||
| 762 | */ |
||
| 763 | public function Breadcrumbs($unlinked = false) { |
||
| 782 | } |
||
| 783 |
Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: