Proposal: Controllers should load using intendedTemplate()

Kirby 2 Solved ✅
1

I propose that a controller should run even if its corresponding template file doesn’t exist.

From my experience with Kirby and the use-cases of controllers (that I’ve dealt with), I don’t think it’d be a problem if the Page->controller method used $page->intendedTemplate() instead of $page->template().

But maybe some of you can identify some potential problems with this. Maybe some of you have dealt with a use case where this would be a problem.

Proposed code change

I propose a very simple change to the Page class:

public function controller($arguments = array()) {
    $controller = $this->kirby->registry->get(
        'controller',
        $this->intendedTemplate()
    );
    // ...
}

I don’t think this will break anything, but some of you may have some arguments against it or see some pitfalls. And I’m sure there are a couple unforeseen repercussions that may result from this.

I haven’t tested it much, but it works well in the site I’m currently developing. And a little more code may be needed, possibly to ensure that the default site.php controller is still loaded.

(I’d happily submit a pull request, but I figured some discussion is in order, especially for such a minor and potentially-breaking code change.)

Justification

There’s been plenty of confusion on Controllers in the forums. Much of that is due to Kirby’s Controllers behaving differently than what we expect from typical MVC style Controllers.

A Kirby Controller really acts more like a ViewModel, even though it can do typical MVC-Controller-like things (i.e. login a user and redirect, get data from an external API and prepare it for the template). The Kirby docs promote using Controllers for these kinds of things, and they do work well for that.

But, we’re forced to create an often unnecessary template file to get the controller to load and run.

Often, a template isn’t needed; the templates/default.php is sufficient, however we do want to run the controller’s code and we don’t want to have to duplicate the default template, or rip it all up into a bunch of snippets which are otherwise unnecessary.

You might argue that…

You may argue that controllers are for setting up data for specific templates. They’re for cleaning up a template.

And that’s true. And they’re quite useful for that purpose.

But the docs do suggest we use controllers for things like handling POST requests… And a controller is really the best place for it.

But again, we’re then forced to create a template file to get it to load. And that template file may not be necessary, making things less DRY.

Default/fallback site.php controller

The default controllers/site.php controller can be helpful in some of these cases, but it can get quite nasty. For example:

# controllers/site.php

return function ($site, $pages, $page) {

    // these `if` blocks are nasty
    if (kirby()->request()->method() == 'POST') {

        if ($page->template() == 'contact' || $page->intendedTemplate() == 'contact') {
            // do the necessary controller work for 
            // the contact controller   
        }

        if ($page->template() == 'something-else' || $page->intendedTemplate() == 'something-else') {
            // do the necessary work for the
            // something-else controller
        }
    }

    // then do the default stuff
    return [
        'today' => date(),
        'some_other' => 'arbitrary variable',
    ];
};

I could extend Kirby and Page myself

Sure, I could extend the main Page class to override just the controller method, then extend the main Kirby class to use my Page class instead… but that’s a bit excessive for such a simple change.

I have made some progress with this route, but it just feels excessive and doesn’t feel like a solid long-term fix. I’m sure I’d eventually encounter some problems when using the global kirby() function or when Kirby updates are applied.

Your thoughts?

If you see potential problems with this, please post them.

Otherwise, I’d love to see this change make it into a near future release.

2

Yes, I’ve seen this discussion:

But it doesn’t quite solve the problem for me. Especially if I’m not using native Kirby templates and snippets.

1 Like
3

I see your point that it can be very useful to use controllers just for handling POST requests etc. without using a separate template just for that. I actually had the same issue a few weeks ago and came to the same conclusion at first, however it isn’t that simple unfortunately.

As you wrote, a controller’s main purpose is to provide variables to the template. If there is no template that can actually handle those (like in the intendedTemplate situation), the main purpose is not there anymore. So if I have a default template, I expect to get variables from the default controller. If we changed the functionality here, users would need to duplicate the default controller in each specific controller. That’s not going to work.

A simpler solution is to use a “dummy” template like that:

<?php

// Load default template
require_once(__DIR__ . DS . 'default.php');

Kirby will then behave exactly like you want it to, however that approach is more explicit than using intendedTemplate internally.

4

Thanks, I figured there’d be some repercussions I hadn’t encountered.

This, I think, is the heart of the problem/confusion. It seems as if a Kirby Controller is intended to do two jobs, though it can’t if a specific template doesn’t exist.

I’m not going to propose changes for it, but man it would be nice if we could load a RequestController (for handling POST type stuff) in addition to loading a KirbyController (which is really more like a ViewModel or TemplateModel).

The dummy template would work, but it’s just not DRY.

I’ll find another way, no worries.