API Make CMSMain more generic #3035
Conversation
GuySartorelli
force-pushed
the
pulls/6.0/generic-cmsmain
branch
3 times, most recently
from
8365dd3 to
83bd75e
Compare
| if (!DataObject::singleton($modelClass)->getSortField()) { | ||
| throw new LogicException($modelClass . ' must define a sort field'); | ||
| } |
There was a problem hiding this comment.
This is debatable - I've got it here mostly because I don't want to rework the UI elements that assume records can be sorted. Realistically there probably isn't much value in using this UI if your record isn't sortable anyway.
| $controller = $this; | ||
| if (static::class !== CMSMain::class) { | ||
| $controller = CMSMain::singleton(); | ||
| } |
There was a problem hiding this comment.
This is a little untidy but necessary for now. Will be cleared up as part of #2951 which removes the CMSMain subclasses.
| // Hardcoded check for SiteTree | ||
| // Ideally CMSMain will get an abstract parent class with all of the generic logic, | ||
| // and then this conditional check can be removed in CMSMain itself. | ||
| if (is_a($this->getModelClass(), SiteTree::class, true)) { | ||
| return SiteConfig::current_site_config()->canCreateTopLevel(); | ||
| } |
There was a problem hiding this comment.
Another thing that will be tidied up as part of #2951
| // Hardcoded check for SiteTree | ||
| // Ideally CMSMain will get an abstract parent class with all of the generic logic, | ||
| // and then this conditional check can be removed in CMSMain itself. | ||
| if (is_a($this->getModelClass(), SiteTree::class, true)) { | ||
| return (bool) Permission::check('SITETREE_REORGANISE'); | ||
| } |
There was a problem hiding this comment.
Another thing that will be tidied up as part of #2951
There was a problem hiding this comment.
This whole class was just a couple of templates and a form. It's been moved into CMSMainAddForm, the associated template, and an action on CMSMain to get the form.
There was a problem hiding this comment.
This whole class was effectively just a placeholder to render some templates. Those templates have been distilled into CMSMain_LeftPanel.ss which is included in the appropriate places in CMSMain_Content.ss and CMSMain_Tools.ss
Note I'm not removing CMSPageEditController in this PR because it wasn't removable when the bulk of this work was done. That can be removed in #2951 which has an explicit AC for that purpose.
There was a problem hiding this comment.
There was quite a lot of duplication between this and the old CMSPagesController templates. That duplication is now distilled into the CMSMain_LeftPanel.ss template.
There was a problem hiding this comment.
This just had a pointless include in it
| /** | ||
| * This provides information required to generate the search form | ||
| * and can be modified on extensions through updateSearchContext | ||
| * | ||
| * @return \SilverStripe\ORM\Search\SearchContext | ||
| * @deprecated 5.4.0 Will be replaced with SilverStripe\CMS\Model\SiteTree::getDefaultSearchContext(). | ||
| */ | ||
| public function getSearchContext() | ||
| { | ||
| Deprecation::notice('5.4.0', 'Will be replaced with ' . SiteTree::class . '::getDefaultSearchContext().'); | ||
| $context = SiteTree::singleton()->getDefaultSearchContext(); | ||
|
|
||
| $this->extend('updateSearchContext', $context); | ||
|
|
||
| return $context; | ||
| } |
There was a problem hiding this comment.
Not needed - this is just basically adding an extra extension hook on top of getDefaultSearchContext().
If we think that's valuable, it should be added to DataObject::getDefaultSearchContext() instead - though the fact that method doesn't already have an extension hook implies one isn't needed there. People can also override getDefaultSearchContext() in their Page or other classes to return different search contexts.
| if (!$parentID && isset($data['Parent'])) { | ||
| $page = SiteTree::get_by_link($data['Parent']); | ||
| if ($page) { | ||
| $parentID = $page->ID; | ||
| } | ||
| } |
There was a problem hiding this comment.
This specific piece of code isn't replicated in the new form or action.
It was originally introduced in c247db5 which doesn't really make it clear what the intended use case of it is. It doesn't seem to be needed at all, nothing that I can see sets the Parent key in the data array.
GuySartorelli
force-pushed
the
pulls/6.0/generic-cmsmain
branch
from
83bd75e to
28ca200
Compare
Remove hardcoded references to pages and SiteTree Remove assumption that records are versioned Remove or validate assumptions about methods on the model class Improve general architecture of CMSMain
GuySartorelli
force-pushed
the
pulls/6.0/generic-cmsmain
branch
from
28ca200 to
b92ad3a
Compare
Replaces #3017
Relies on changes in silverstripe/silverstripe-admin#1837
Issue
SiteTreeinCMSMain#2947