Controller->getViewer() doesn't pick up templates when inheriting a Controller

[kinglozzer]

Reported by @micmania1.

E.g. HomePage extends Page with no HomePage_Controller specified as it’s no longer required, however the HomePage template isn’t being picked up. Controller->getViewer() isn’t finding it.

Related to https://github.com/silverstripe/silverstripe-cms/pull/881, not a ‘regression’ as the functionality didn’t previously exist. However, without this the above change is pretty-much useless.

[micmania1]

I've taken a look at this and I think you could probably overload the getViewer method in ContentController which looks for model templates rather than controllers.

[kinglozzer]

I’ve also had another look at this, overloading getViewer() in ContentController doesn’t seem like the best way of approaching this - we’d likely have to duplicate most of the logic (such as the ‘hard-coded’ $this->templates stuff) or call parent::getViewer() first and modify the returned SSViewer object. I’ve had a go at fixing this, using an extension to update the list of templates, it’s working fine but would need more thorough testing (probably some unit tests as well):

https://github.com/kinglozzer/silverstripe-cms/commit/7b66dc1a3e2a3204cd5d5ec8dadb38dffc5720ea (framework commit for the extension hook https://github.com/kinglozzer/sapphire/commit/d69ac4a5e01fdfcde8101f2eb46de4c4b5f5027c)

@chillu Can I bug you for your thoughts on solving this?

[micmania1]

I was thinking something along the lines of this to avoid duplication:

ContentController:

public function getViewer($action) {
    // Hard-coded templates
    if(isset($this->templates[$action]) && $this->templates[$action]
        || (isset($this->templates['index']) && $this->templates['index'])
        || $this->template
    ) {
        return parent::getViewer($action);
    }

    // Find templates from $this->dataRecord
    $templates = array(); // etc...
    return new SSViewer($templates);
}

[kinglozzer]

That looks like a good solution, probably tidier than mine too :+1:. Am I right in thinking that ContentController->dataRecord always has to be an instance of Page? Or can people overload it? If they can overload it, we need to fall back to parent::getViewer() if it’s not an instance of Page, as otherwise we’d end up with an array of templates like array('NewsItem', 'DataObject', 'ViewableData', 'Object').

Also, we need to make sure we append ContentController_action (if there’s an action), ContentController and Controller to the list of templates to ensure it’s consistent with current behaviour & to avoid potentially breaking anything :)

[micmania1]

I'll fix this once https://github.com/silverstripe/silverstripe-framework/pull/2679 is implemented. I should be able to use that new method to get the template list rather than duplicating code.

[chillu]

Merged https://github.com/silverstripe/silverstripe-cms/pull/914, so this can be closed, right?