diff --git a/code/CMSProfileController.php b/code/CMSProfileController.php index ea179c638..fe6157dae 100644 --- a/code/CMSProfileController.php +++ b/code/CMSProfileController.php @@ -2,17 +2,13 @@ namespace SilverStripe\Admin; -use SilverStripe\CMS\Controllers\CMSMain; use SilverStripe\Control\HTTPResponse; use SilverStripe\Forms\Form; use SilverStripe\Forms\HiddenField; -use SilverStripe\Forms\FormAction; -use SilverStripe\Model\List\ArrayList; use SilverStripe\Security\Member; -use SilverStripe\Security\Permission; use SilverStripe\Security\Security; -class CMSProfileController extends LeftAndMain +class CMSProfileController extends SingleRecordAdmin { private static $url_segment = 'myprofile'; @@ -20,95 +16,61 @@ class CMSProfileController extends LeftAndMain private static $required_permission_codes = 'CMS_ACCESS'; - private static $tree_class = Member::class; + private static $model_class = Member::class; private static $ignore_menuitem = true; - public function getEditForm($id = null, $fields = null) - { - $this->setCurrentPageID(Security::getCurrentUser()->ID); - - $form = parent::getEditForm($id, $fields); - - if ($form instanceof HTTPResponse) { - return $form; - } - - $form->Fields()->removeByName('LastVisited'); - $form->Fields()->push(new HiddenField('ID', null, Security::getCurrentUser()->ID)); - $form->Actions()->push( - FormAction::create('save', _t(CMSMain::class . '.SAVE', 'Save')) - ->addExtraClass('btn-primary font-icon-save') - ->setUseButtonTag(true) - ); - - $form->Actions()->removeByName('action_delete'); + private static bool $only_one_record = false; - if ($member = Security::getCurrentUser()) { - $form->setValidator($member->getValidator()); - } else { - $form->setValidator(Member::singleton()->getValidator()); - } + private static bool $allow_new_record = false; - if ($form->Fields()->hasTabSet()) { - $form->Fields()->findOrMakeTab('Root')->setTemplate('SilverStripe\\Forms\\CMSTabSet'); + protected function init() + { + parent::init(); + if (!$this->getResponse()->isRedirect()) { + $this->setCurrentPageID(Security::getCurrentUser()->ID); } + } - $form->addExtraClass('member-profile-form root-form cms-edit-form center fill-height'); - + public function getEditForm($id = null, $fields = null): Form + { + $form = parent::getEditForm($id, $fields); + $form->Fields()->push(HiddenField::create('ID', null, Security::getCurrentUser()->ID)); return $form; } public function canView($member = null) { $currentUser = Security::getCurrentUser(); - if (!$member && $member !== false) { $member = $currentUser; } - - // cms menus only for logged-in members + // cms menus are only for logged-in members if (!$member) { return false; } - // Check they are trying to edit themselves and they have permissions return $member->ID === $currentUser->ID && parent::canView($member); } public function save(array $data, Form $form): HTTPResponse { - $member = Member::get()->byID($data['ID']); - if (!$member) { - $this->httpError(404); + // Make sure the ID is a positive number + $id = $data['ID'] ?? null; + if (!is_numeric($id) || $id < 1) { + $this->httpError(400); } - $origLocale = $member->Locale; - - if (!$member->canEdit()) { - $form->sessionMessage(_t(__CLASS__.'.CANTEDIT', 'You don\'t have permission to do that'), 'bad'); - return $this->redirectBack(); - } - + // Get the current member locale so we can see if it changes + $member = Member::get()->byID($data['ID']); + $origLocale = $member?->Locale; + // actual save (along with error handling e.g. if there's no member with that ID) is handled by the parent class $response = parent::save($data, $form); - + // If the member locale changes, reload the full CMS so localisation can kick in. + // Otherwise only the edit form will reload. if (isset($data['Locale']) && $origLocale != $data['Locale']) { $response->addHeader('X-Reload', true); $response->addHeader('X-ControllerURL', $this->Link()); } - return $response; } - - /** - * Only show first element, as the profile form is limited to editing - * the current member it doesn't make much sense to show the member name - * in the breadcrumbs. - * - * @param bool $unlinked - */ - public function Breadcrumbs($unlinked = false) - { - $items = parent::Breadcrumbs($unlinked); - return new ArrayList([$items[0]]); - } } diff --git a/code/LeftAndMain.php b/code/LeftAndMain.php index 19a97d3fc..efc4deb46 100644 --- a/code/LeftAndMain.php +++ b/code/LeftAndMain.php @@ -99,7 +99,7 @@ class LeftAndMain extends AdminController implements PermissionProvider * @config * @var string */ - private static $tree_class = null; + private static $model_class = null; /** * @var array @@ -941,7 +941,7 @@ public function PreviewPanel() */ public function getRecord($id) { - $className = $this->config()->get('tree_class'); + $className = $this->config()->get('model_class'); if (!$className) { return null; } @@ -1007,7 +1007,7 @@ protected function getSearchFilter() public function save(array $data, Form $form): HTTPResponse { $request = $this->getRequest(); - $className = $this->config()->get('tree_class'); + $className = $this->config()->get('model_class'); // Existing or new record? $id = $data['ID']; @@ -1020,7 +1020,7 @@ public function save(array $data, Form $form): HTTPResponse $this->httpError(404, "Bad record ID #" . (int)$id); } } else { - if (!singleton($this->config()->get('tree_class'))->canCreate()) { + if (!singleton($this->config()->get('model_class'))->canCreate()) { return Security::permissionFailure($this); } $record = $this->getNewItem($id, false); @@ -1056,7 +1056,7 @@ public function save(array $data, Form $form): HTTPResponse */ public function getNewItem($id, $setID = true) { - $class = $this->config()->get('tree_class'); + $class = $this->config()->get('model_class'); $object = Injector::inst()->create($class); if ($setID) { $object->ID = $id; @@ -1066,7 +1066,7 @@ public function getNewItem($id, $setID = true) public function delete(array $data, Form $form): HTTPResponse { - $className = $this->config()->get('tree_class'); + $className = $this->config()->get('model_class'); $id = $data['ID']; $record = DataObject::get_by_id($className, $id); @@ -1098,7 +1098,7 @@ public function delete(array $data, Form $form): HTTPResponse * method in an entwine subclass. This method can accept a record identifier, * selected either in custom logic, or through {@link currentPageID()}. * The form usually construct itself from {@link DataObject->getCMSFields()} - * for the specific managed subclass defined in {@link LeftAndMain::$tree_class}. + * for the specific managed subclass defined in {@link LeftAndMain::$model_class}. * * @param HTTPRequest $request Passed if executing a HTTPRequest directly on the form. * If empty, this is invoked as $EditForm in the template @@ -1151,8 +1151,8 @@ public function getEditForm($id = null, $fields = null) $fields->push(new HiddenField('ClassName')); } - $tree_class = $this->config()->get('tree_class'); - if ($tree_class::has_extension(Hierarchy::class) + $modelClass = $this->config()->get('model_class'); + if ($modelClass::has_extension(Hierarchy::class) && !$fields->dataFieldByName('ParentID') ) { $fields->push(new HiddenField('ParentID')); @@ -1173,14 +1173,14 @@ public function getEditForm($id = null, $fields = null) if (!$actions || !$actions->count()) { if ($record->hasMethod('canEdit') && $record->canEdit()) { $actions->push( - FormAction::create('save', _t(CMSMain::class . '.SAVE', 'Save')) + FormAction::create('save', _t(__CLASS__ . '.SAVE', 'Save')) ->addExtraClass('btn btn-primary') ->addExtraClass('font-icon-add-circle') ); } if ($record->hasMethod('canDelete') && $record->canDelete()) { $actions->push( - FormAction::create('delete', _t(ModelAdmin::class . '.DELETE', 'Delete')) + FormAction::create('delete', _t(__CLASS__ . '.DELETE', 'Delete')) ->addExtraClass('btn btn-secondary') ); } @@ -1223,24 +1223,7 @@ public function getEditForm($id = null, $fields = null) $form->addExtraClass('cms-previewable'); } $form->addExtraClass('fill-height'); - - if ($record->hasMethod('getCMSCompositeValidator')) { - // As of framework v4.7, a CompositeValidator is always available form a DataObject, but it may be - // empty (which is fine) - $form->setValidator($record->getCMSCompositeValidator()); - } elseif ($record->hasMethod('getCMSValidator')) { - // BC support for framework < v4.7 - $validator = $record->getCMSValidator(); - - // The clientside (mainly LeftAndMain*.js) rely on ajax responses - // which can be evaluated as javascript, hence we need - // to override any global changes to the validation handler. - if ($validator) { - $form->setValidator($validator); - } - } else { - $form->unsetValidator(); - } + $form->setValidator($record->getCMSCompositeValidator()); // Check if this form is readonly if (!$record->canEdit()) { @@ -1330,7 +1313,7 @@ public function EditFormTools() */ public function batchactions() { - return new CMSBatchActionHandler($this, 'batchactions', $this->config()->get('tree_class')); + return new CMSBatchActionHandler($this, 'batchactions', $this->config()->get('model_class')); } /** diff --git a/code/SecurityAdmin.php b/code/SecurityAdmin.php index b817ae5c8..561f282e0 100755 --- a/code/SecurityAdmin.php +++ b/code/SecurityAdmin.php @@ -58,7 +58,7 @@ class SecurityAdmin extends ModelAdmin implements PermissionProvider private static $menu_priority = 0; - private static $tree_class = Group::class; + private static $model_class = Group::class; private static $required_permission_codes = 'CMS_ACCESS_SecurityAdmin'; diff --git a/code/SingleRecordAdmin.php b/code/SingleRecordAdmin.php new file mode 100644 index 000000000..d85af6faa --- /dev/null +++ b/code/SingleRecordAdmin.php @@ -0,0 +1,56 @@ +Fields()->hasTabSet()) { + $form->Fields()->findOrMakeTab('Root')->setTemplate('SilverStripe/Forms/CMSTabSet'); + } + return $form; + } + + public function getRecord($id): ?DataObject + { + if (!$id) { + return $this->getSingleRecord(); + } + return parent::getRecord($id); + } + + protected function getSingleRecord(): ?DataObject + { + $record = null; + $modelClass = static::config()->get('model_class'); + if (static::config()->get('only_one_record')) { + $record = DataObject::get_one($modelClass); + } + if (!$record && static::config()->get('allow_new_record')) { + $record = $modelClass::create(); + } + return $record; + } +} diff --git a/tests/behat/features/profile.feature b/tests/behat/features/profile.feature index 77bc78720..b81914004 100644 --- a/tests/behat/features/profile.feature +++ b/tests/behat/features/profile.feature @@ -65,3 +65,7 @@ Feature: Manage my own settings And I select "German (Germany)" from "Interface Language" And I press the "Save" button Then I should see "Sprache" + + Scenario: Breadcrumbs are in the correct place + Then I should see "Main" in the ".cms-content-header-tabs" element + And I should not see "Main" in the "#Form_EditForm" element diff --git a/tests/php/LeftAndMainTest/MyTreeController.php b/tests/php/LeftAndMainTest/MyTreeController.php index 711e64976..0e09d1466 100644 --- a/tests/php/LeftAndMainTest/MyTreeController.php +++ b/tests/php/LeftAndMainTest/MyTreeController.php @@ -9,7 +9,7 @@ class MyTreeController extends LeftAndMain implements TestOnly { private static $url_segment = 'mytree/edit'; - private static $tree_class = MyTree::class; + private static $model_class = MyTree::class; private static $allowed_actions = [ 'EditForm' diff --git a/tests/php/SingleRecordAdminTest.php b/tests/php/SingleRecordAdminTest.php new file mode 100644 index 000000000..48a53ac0b --- /dev/null +++ b/tests/php/SingleRecordAdminTest.php @@ -0,0 +1,117 @@ +removeAll(); + } + + public static function provideGetRecord(): array + { + return [ + [ + 'recordExists' => false, + 'onlyOneRecord' => false, + 'allowNew' => false, + 'useFixtureID' => false, + 'expected' => null, + ], + [ + 'recordExists' => false, + 'onlyOneRecord' => false, + 'allowNew' => false, + 'useFixtureID' => true, + 'expected' => null, + ], + [ + 'recordExists' => true, + 'onlyOneRecord' => false, + 'allowNew' => false, + 'useFixtureID' => true, + 'expected' => 'existing', + ], + [ + 'recordExists' => true, + 'onlyOneRecord' => false, + 'allowNew' => false, + 'useFixtureID' => false, + 'expected' => null, + ], + [ + 'recordExists' => true, + 'onlyOneRecord' => true, + 'allowNew' => false, + 'useFixtureID' => false, + 'expected' => 'existing', + ], + [ + 'recordExists' => true, + 'onlyOneRecord' => false, + 'allowNew' => true, + 'useFixtureID' => false, + 'expected' => 'new', + ], + [ + 'recordExists' => true, + 'onlyOneRecord' => false, + 'allowNew' => true, + 'useFixtureID' => true, + 'expected' => 'existing', + ], + [ + 'recordExists' => true, + 'onlyOneRecord' => true, + 'allowNew' => true, + 'useFixtureID' => true, + 'expected' => 'existing', + ], + ]; + } + + #[DataProvider('provideGetRecord')] + public function testGetRecord( + bool $recordExists, + bool $onlyOneRecord, + bool $allowNew, + bool $useFixtureID, + ?string $expected + ): void { + $admin = new TestAdmin(); + if ($recordExists) { + $record = new TestRecord(); + $recordID = $record->write(); + } else { + $recordID = 9999; + } + TestAdmin::config()->set('only_one_record', $onlyOneRecord); + TestAdmin::config()->set('allow_new_record', $allowNew); + + if ($useFixtureID) { + $result = $admin->getRecord($recordID); + } else { + $result = $admin->getRecord(0); + } + + if ($expected === 'existing') { + $this->assertInstanceOf(TestRecord::class, $result); + $this->assertSame($recordID, $result->ID); + } elseif ($expected === 'new') { + $this->assertInstanceOf(TestRecord::class, $result); + $this->assertSame(0, $result->ID); + } else { + $this->assertNull($result); + } + } +} diff --git a/tests/php/SingleRecordAdminTest/TestAdmin.php b/tests/php/SingleRecordAdminTest/TestAdmin.php new file mode 100644 index 000000000..d03aa1345 --- /dev/null +++ b/tests/php/SingleRecordAdminTest/TestAdmin.php @@ -0,0 +1,11 @@ +