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.

Sign up for GitHub

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 Stop refering to "pages" when dealing with generic records #1867

Merged
merged 2 commits into from
Dec 12, 2024
Merged

API Stop refering to "pages" when dealing with generic records #1867

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1eb27c9
ENH Update localisation string keys
GuySartorelli Oct 17, 2024
b42c820
API Stop refering to "pages" when dealing with generic records
GuySartorelli Oct 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/dist/styles/bundle.css
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion client/src/styles/legacy/_style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -955,7 +955,7 @@ html, body {
margin-right: 10px;
}

.page-icon {
.record-icon {
display: inline-block;
margin-right: 6px;

Expand Down
14 changes: 7 additions & 7 deletions client/src/styles/legacy/_tree.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@
color: #FFF;
border: 0;
// Specificity override for icons in children pages
.jstree-pageicon {
.jstree-recordicon {
color: #FFF;
}
}
Expand All @@ -324,7 +324,7 @@
text-decoration: none;
margin-right: 2px;
}
.jstree-pageicon {
.jstree-recordicon {
margin-top: 3px;
margin-right: 5px;
}
Expand Down Expand Up @@ -668,7 +668,7 @@

/* Page icons */

.page-icon, a .jstree-pageicon {
.record-icon, a .jstree-recordicon {
display: block;
width: 16px;
height: 16px;
Expand All @@ -679,7 +679,7 @@
}
}

a .jstree-pageicon {
a .jstree-recordicon {
float: left;
margin-right: 4px;
position: relative;
Expand All @@ -692,7 +692,7 @@ a .jstree-pageicon {
}

@mixin tree-status-icon($label, $dotColor, $textColor, $bgColor) {
.cms-tree.jstree .status-#{$label} > a .jstree-pageicon:after {
.cms-tree.jstree .status-#{$label} > a .jstree-recordicon:after {
content:"";
display: block;
width: 8px;
Expand All @@ -711,7 +711,7 @@ a .jstree-pageicon {
border-color:$textColor;
}
// Dots
.cms-tree.jstree .status-#{$label} > a .jstree-pageicon:after {
.cms-tree.jstree .status-#{$label} > a .jstree-recordicon:after {
background-color:$dotColor;
box-shadow: 0 1px 1px rgba(0, 0, 0, 0.3), inset 0 0 0 1px $textColor;
}
Expand Down Expand Up @@ -760,7 +760,7 @@ a .jstree-pageicon {
.jstree-icon {
background-image: none !important;
}
.jstree-pageicon {
.jstree-recordicon {
background: url('../../images/throbber.gif') top left no-repeat;
}
}
Expand Down
8 changes: 7 additions & 1 deletion code/CMSBatchAction.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace SilverStripe\Admin;

use SilverStripe\CMS\Controllers\CMSMain;
use SilverStripe\CMS\Model\SiteTree;
use SilverStripe\Control\Controller;
use SilverStripe\Control\HTTPResponse;
Expand Down Expand Up @@ -107,9 +108,14 @@ public function batchaction(SS_List $objs, $helperMethod, $successMessage, $argu

// Now make sure the tree title is appropriately updated
$publishedRecord = DataObject::get_by_id($this->managedClass, $id);
if ($publishedRecord instanceof SiteTree) {
$treeTitle = CMSMain::singleton()->getRecordTreeMarkup($publishedRecord);
Comment on lines +111 to +112
Copy link
Member Author

@GuySartorelli GuySartorelli Dec 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This looks like new hard coupling - but this class was always intended to be used with SiteTree - or at least has really only been used with it until now in core. The SiteTree use statement was even already there.
The coupling will be removed in silverstripe/silverstripe-cms#2950

} else {
$treeTitle = $publishedRecord->TreeTitle;
}
if ($publishedRecord) {
$status['modified'][$id] = [
'TreeTitle' => $publishedRecord->TreeTitle,
'TreeTitle' => $treeTitle,
];
} else {
$status['deleted'][$id] = $id;
Expand Down
2 changes: 1 addition & 1 deletion code/CMSMenu.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ public static function get_cms_classes($root = null, $recursive = true, $sort =
if (!$root) {
$root = LeftAndMain::class;
}
$abstractClasses = [LeftAndMain::class, CMSMain::class];
$abstractClasses = [LeftAndMain::class];
Copy link
Member Author

Choose a reason for hiding this comment

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

This change is needed so that CMSMain can be included in the left menu and have its route built.
Without this, it was the CMSPagesController which was a very-nearly-useless subclass of CMSMain that was used.
That class has been merged into CMSMain so we need its menu item back

$subClasses = array_values(ClassInfo::subclassesFor($root) ?? []);
foreach ($subClasses as $className) {
if ($recursive && $className != $root) {
Expand Down
146 changes: 39 additions & 107 deletions code/LeftAndMain.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,8 @@ class LeftAndMain extends FormSchemaController implements PermissionProvider
*
* Determines what is managed in this interface, through
* {@link getEditForm()} and other logic.
*
* @config
* @var string
*/
private static $model_class = null;
private static ?string $model_class = null;

/**
* @var array
Expand All @@ -116,12 +113,9 @@ class LeftAndMain extends FormSchemaController implements PermissionProvider
];

/**
* Current pageID for this request
*
* @var null
* @deprecated 2.4.0 Will be renamed to recordID.
* Current record ID for this request
*/
protected $pageID = null;
protected $recordID = null;

/**
* Set by {@link LeftAndMainErrorExtension} if an http error occurs
Expand Down Expand Up @@ -770,15 +764,23 @@ public function PreviewPanel()
return null;
}

/**
* Get the class of the model which is managed by this controller.
* @return class-string<DataObject>
*/
public function getModelClass(): string
{
return static::config()->get('model_class') ?? '';
}

/**
* Get dataobject from the current ID
*
* @param int|DataObject $id ID or object
* @return DataObject
*/
public function getRecord($id)
public function getRecord($id): ?DataObject
{
$className = $this->config()->get('model_class');
$className = $this->getModelClass();
if (!$className) {
return null;
}
Expand Down Expand Up @@ -844,7 +846,7 @@ protected function getSearchFilter()
public function save(array $data, Form $form): HTTPResponse
{
$request = $this->getRequest();
$className = $this->config()->get('model_class');
$className = $this->getModelClass();

// Existing or new record?
$id = $data['ID'];
Expand All @@ -857,7 +859,7 @@ public function save(array $data, Form $form): HTTPResponse
$this->httpError(404, "Bad record ID #" . (int)$id);
}
} else {
if (!singleton($this->config()->get('model_class'))->canCreate()) {
if (!DataObject::singleton($className)->canCreate()) {
return Security::permissionFailure($this);
}
$record = $this->getNewItem($id, false);
Expand Down Expand Up @@ -893,7 +895,7 @@ public function save(array $data, Form $form): HTTPResponse
*/
public function getNewItem($id, $setID = true)
{
$class = $this->config()->get('model_class');
$class = $this->getModelClass();
$object = Injector::inst()->create($class);
if ($setID) {
$object->ID = $id;
Expand All @@ -903,7 +905,7 @@ public function getNewItem($id, $setID = true)

public function delete(array $data, Form $form): HTTPResponse
{
$className = $this->config()->get('model_class');
$className = $this->getModelClass();

$id = $data['ID'];
$record = DataObject::get_by_id($className, $id);
Expand Down Expand Up @@ -933,9 +935,9 @@ public function delete(array $data, Form $form): HTTPResponse
*
* This is a "pseudo-abstract" method, usually connected to a {@link getEditForm()}
* method in an entwine subclass. This method can accept a record identifier,
* selected either in custom logic, or through {@link currentPageID()}.
* selected either in custom logic, or through {@link currentRecordID()}.
* The form usually construct itself from {@link DataObject->getCMSFields()}
* for the specific managed subclass defined in {@link LeftAndMain::$model_class}.
* for the specific managed subclass defined in {@link LeftAndMain::getModelClass()}.
*
* @param HTTPRequest $request Passed if executing a HTTPRequest directly on the form.
* If empty, this is invoked as $EditForm in the template
Expand Down Expand Up @@ -988,7 +990,7 @@ public function getEditForm($id = null, $fields = null)
$fields->push(new HiddenField('ClassName'));
}

$modelClass = $this->config()->get('model_class');
$modelClass = $this->getModelClass();
if ($modelClass::has_extension(Hierarchy::class)
&& !$fields->dataFieldByName('ParentID')
) {
Expand Down Expand Up @@ -1142,7 +1144,7 @@ public function EditFormTools()
*/
public function batchactions()
{
return new CMSBatchActionHandler($this, 'batchactions', $this->config()->get('model_class'));
return new CMSBatchActionHandler($this, 'batchactions', $this->getModelClass());
}

/**
Expand Down Expand Up @@ -1225,66 +1227,24 @@ public function getSilverStripeNavigator(?DataObject $record = null)
* sources (in this order):
* - GET/POST parameter named 'ID'
* - URL parameter named 'ID'
* - Session value namespaced by classname, e.g. "CMSMain.currentPage"
*
* @return int
* @deprecated 5.4.0 use currentRecordID() instead.
*/
public function currentPageID()
{
Deprecation::notice('5.4.0', 'use currentRecordID() instead.');
return $this->currentRecordID();
}

/**
* Identifier for the currently shown record,
* in most cases a database ID. Inspects the following
* sources (in this order):
* - GET/POST parameter named 'ID'
* - URL parameter named 'ID'
* - Session value namespaced by classname, e.g. "CMSMain.currentPage"
*/
public function currentRecordID(): ?int
{
if ($this->pageID) {
return $this->pageID;
}
if ($this->getRequest()->requestVar('ID') && is_numeric($this->getRequest()->requestVar('ID'))) {
return (int) $this->getRequest()->requestVar('ID');
}

if ($this->getRequest()->requestVar('CMSMainCurrentPageID') && is_numeric($this->getRequest()->requestVar('CMSMainCurrentPageID'))) {
$id = null;
if ($this->recordID) {
$id = $this->recordID;
} elseif ($this->getRequest()->requestVar('ID') && is_numeric($this->getRequest()->requestVar('ID'))) {
$id = (int) $this->getRequest()->requestVar('ID');
} elseif ($this->getRequest()->requestVar('CMSMainCurrentRecordID') && is_numeric($this->getRequest()->requestVar('CMSMainCurrentRecordID'))) {
// see GridFieldDetailForm::ItemEditForm
return (int) $this->getRequest()->requestVar('CMSMainCurrentPageID');
$id = (int) $this->getRequest()->requestVar('CMSMainCurrentRecordID');
} elseif (isset($this->urlParams['ID']) && is_numeric($this->urlParams['ID'])) {
$id = (int) $this->urlParams['ID'];
} elseif (is_numeric($this->getRequest()->param('ID'))) {
$id = (int) $this->getRequest()->param('ID');
}

if (isset($this->urlParams['ID']) && is_numeric($this->urlParams['ID'])) {
return (int) $this->urlParams['ID'];
}

if (is_numeric($this->getRequest()->param('ID'))) {
return (int) $this->getRequest()->param('ID');
}

// Using session for this is deprecated - see https://github.com/silverstripe/silverstripe-admin/pull/19
$session = $this->getRequest()->getSession();
return $session->get($this->sessionNamespace() . ".currentPage") ?: null;
}

/**
* Forces the current page to be set in session,
* which can be retrieved later through {@link currentPageID()}.
* Keep in mind that setting an ID through GET/POST or
* as a URL parameter will overrule this value.
*
* @param int $id
* @deprecated 5.4.0 use setCurrentRecordID() instead.
*/
public function setCurrentPageID($id)
{
Deprecation::notice('5.4.0', 'use setCurrentRecordID() instead.');
$id = (int)$id;
$this->setCurrentRecordID($id);
$this->extend('updateCurrentRecordID', $id);
return $id;
}

/**
Expand All @@ -1293,22 +1253,7 @@ public function setCurrentPageID($id)
*/
public function setCurrentRecordID(?int $id): void
{
$this->pageID = $id;
// Setting session for this is deprecated - see https://github.com/silverstripe/silverstripe-admin/pull/19
$this->getRequest()->getSession()->set($this->sessionNamespace() . ".currentPage", $id);
}

/**
* Uses {@link getRecord()} and {@link currentPageID()}
* to get the currently selected record.
*
* @return DataObject
* @deprecated 5.4.0 use currentRecord() instead.
*/
public function currentPage()
{
Deprecation::notice('5.4.0', 'use currentRecord() instead.');
return $this->currentRecord();
$this->recordID = $id;
}

/**
Expand All @@ -1320,19 +1265,6 @@ public function currentRecord(): ?DataObject
return $this->getRecord($this->currentRecordID());
}

/**
* Compares a given record to the currently selected one (if any).
* Used for marking the current tree node.
*
* @return bool
* @deprecated 5.4.0 use isCurrentRecord() instead.
*/
public function isCurrentPage(DataObject $record)
{
Deprecation::notice('5.4.0', 'use isCurrentRecord() instead.');
return $this->isCurrentRecord($record);
}

/**
* Compares a given record to the currently selected one (if any).
* Used for marking the current tree node.
Expand Down Expand Up @@ -1525,7 +1457,7 @@ public function providePermissions()
$perms = [
"CMS_ACCESS_LeftAndMain" => [
'name' => _t(__CLASS__ . '.ACCESSALLINTERFACES', 'Access to all CMS sections'),
'category' => _t(Permission::class . '.CMS_ACCESS_CATEGORY', 'CMS Access'),
'category' => _t(__CLASS__ . '.CMS_ACCESS_CATEGORY', 'CMS Access'),
'help' => _t(__CLASS__ . '.ACCESSALLINTERFACESHELP', 'Overrules more specific access settings.'),
'sort' => -100
]
Expand Down Expand Up @@ -1557,11 +1489,11 @@ public function providePermissions()
$perms[$code] = [
// Item in permission selection identifying the admin section. Example: Access to 'Files & Images'
'name' => _t(
CMSMain::class . '.ACCESS',
__CLASS__ . '.ACCESS',
"Access to '{title}' section",
['title' => $title]
),
'category' => _t(Permission::class . '.CMS_ACCESS_CATEGORY', 'CMS Access')
'category' => _t(__CLASS__ . '.CMS_ACCESS_CATEGORY', 'CMS Access')
];
}

Expand Down
Loading
Loading