silverstripe / silverstripe-cms

Silverstripe CMS - this is a module for Silverstripe Framework rather than a standalone app. Use https://github.com/silverstripe/silverstripe-installer/ to set this up.
http://silverstripe.org/
BSD 3-Clause "New" or "Revised" License
516 stars 333 forks source link

POC DO NOT MERGE - Make CMSMain work with other models #2939

Closed GuySartorelli closed 6 months ago

GuySartorelli commented 6 months ago

This is a proof of concept as part of https://github.com/silverstripe/silverstripe-cms/issues/2933 to see what's needed to make CMSMain work with classes that aren't SiteTree.

To play around with this, install silverstripe/taxonomy and add the following YAML config to your project:

SilverStripe\Taxonomy\TaxonomyTerm:
  cms_edit_owner: SilverStripe\CMS\Controllers\CMSMain
  extensions:
    - SilverStripe\Admin\CMSEditLinkExtension

SilverStripe\CMS\Controllers\CMSMain:
  tree_class: SilverStripe\Taxonomy\TaxonomyTerm

Your /admin/pages will now be working with taxonomy terms, rather than pages.

Theoretically this could work with any DataObject class which has CMSEditLinkExtension and Hierarchy applied.

What would still need to be done besides this

  1. Refactor CMSPagesController, CMSPageEditController, CMSPageSettingsController, CMSPageHistoryViewerController, and CMSPageAddController so you could have multiple instances of this going at once, e.g. one admin for pages, one for taxonomies, etc - without having to subclass all of these controllers as well.
    • Ideally these wouldn't be subclasses of LeftAndMain at all - either this could all be handled in the main HierarchyModelAdmin class (see below) or these could all be more generic controllers.
    • Either way they need to not be subclasses of CMSMain, but instead get any information they need by calling methods on CMSMain (i.e. have the CMSMain instance passed to their constructor) or having that information passed through to them via setters or via their constructor.
  2. Have this logic all in a superclass called something more generic, like HierarchyModelAdmin (which would belong in silverstripe/admin along with all appropriate JS), and have CMSMain be a subclass of HierarchyModelAdmin with its tree_class set to SiteTree
    • While moving the JS to silverstripe/admin, it'd be worth finding all JS logic that's duplicated and making it so that all works in the same way for ModelAdmin and this new class.
  3. Obviously we'd need to add in appropriate type checking, make sure the tree class is hierarchical and has a cms edit link.
  4. In a few places it'd pay to add early returns when versioned isn't applied to the record
  5. This POC does a lot of lazy work to avoid refactoring - e.g. there's a lot of methods now duplicated between SiteTree and CMSMain. These should go into a new class (probably an extension) that can be applied to any classes that want to live in this admin type. Things like tree title vs menu title vs title, icon, status flags, tree node classes, etc
    • This will end up meaning if you want to add a model to this admin, it must have Hierarchy, CMSPageEditController, and this new extension. But that also means you can just apply 3 extensions to any model and boom, you're ready to go.
    • Some things like the breadcrumbs for list view shouldn't be on the model - just leave that in the controller or fully template-ify it
    • Some things like status flags should probably have default functionality on the controller but use invokeWithExtensions (or just a hasMethod() check) to allow the model to update it
  6. There's a lot of wording to adjust that assumes this is for pages.
  7. Batch actions assume Versioned is applied and probably also have other assumptions that make them not work
  8. The way search works for CMSMain is just completely incompatible with these changes. It has a hardcoded form, and a different code path for the actual search functionality. We should marry up the way GridFieldFilterHeader and the filter/search in here work.
    • Probably have a new controller for CMS search. This gets the form using similar logic to the current GridFieldFilterHeader logic
    • Current fields in CMSMain::getSearchForm() get moved into SiteTree::searchableFields()
    • SiteTree gets a custom SearchContext subclass which leverages CMSSiteTreeFilter for the field that uses it (if that's necessary - I haven't looked too deep into the search code path for sitetree)
  9. I haven't really looked at all at the history tab - it might need some work. MVP would be to just not show that for anything other than SiteTree and handle making it work as a separate effort.
  10. A whole bunch of new unit and behat tests would need to be written
  11. The root of the site tree is the site name, which makes sense for pages but doesn't really make sense for much of anything else. Might be worth just removing that root, or making it configurable, or something.

Stretch goal

Make this handle multiple classes in separate tabs, like ModelAdmin does via its managed_models config. Then you could still have a single "Taxonomies" CMS section, but it could handle both TaxonomyTerm and TaxonomyType with this hierarchical style.

Note that TaxonomyType isn't hierarchical - so either we'd need to change that, or better yet AAALLL of this logic could just get bundled together with ModelAdmin, so you can have for example one tab for managing TaxonomyTerm which uses the tree structure, and one tab for managing TaxonomyType which uses the normal ModelAdmin grid structure.

GuySartorelli commented 6 months ago

Closing - no need to keep it open, the code will still be there even when it's closed.