Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API Use the new SingleRecordAdmin class #179

Merged

Conversation

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anything removed from this class was either not necessary (likely related to legacy CMS 3 or older code that no longer exists) or is already being done in one of the superclasses.

I've added some strong typing as a matter of course.

Comment on lines -294 to -310
public function getCMSActions()
{
if (Permission::check('ADMIN') || Permission::check('EDIT_SITECONFIG')) {
$actions = FieldList::create(
FormAction::create(
'save_siteconfig',
_t('SilverStripe\\CMS\\Controllers\\CMSMain.SAVE', 'Save')
)->addExtraClass('btn-primary font-icon-save')
);
} else {
$actions = FieldList::create();
}

$this->extend('updateCMSActions', $actions);

return $actions;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let the admin handle the actions. LeftAndMain automatically adds a save action.
This method exists on DataObject so we don't need to deprecate this.

Comment on lines -329 to -331

static::singleton()->extend('updateCurrentSiteConfig', $siteConfig);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this because we no longer call SiteConfig::current_site_config() in the admin to get the config we want to edit. Instead, this same logic is duplicated there (wtihout the extension hook).

I've checked and nothing in core or supported modules (including fluent and subsites) uses this hook (and according to GitHub nor does anything else in the universe) and I also checked with our in-house development teams who weren't able to find any usage of it in their private repositories either. Seems perfectly safe to remove - but I'll mention it in the changelog to be cover our bases.

Comment on lines +459 to +466
public function canDelete($member = null)
{
$extended = $this->extendedCan(__FUNCTION__, $member);
if ($extended !== null) {
return $extended;
}
return false;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed to prevent a delete button from being presented to admin users. No one should be able to delete SiteConfig records by default.

@GuySartorelli GuySartorelli marked this pull request as ready for review November 1, 2024 01:33
@GuySartorelli GuySartorelli force-pushed the pulls/6/singlerecordadmin branch from 5ac38d0 to 0aa9555 Compare November 6, 2024 01:33
@emteknetnz emteknetnz merged commit 9c3f0e1 into silverstripe:6 Nov 6, 2024
4 of 8 checks passed
@emteknetnz emteknetnz deleted the pulls/6/singlerecordadmin branch November 6, 2024 04:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants