New extender for adding variables to HtmlDocument payload

[franzliedke]

Fixes #1602

Changes proposed in this pull request:
Add a document() method to the Frontend extender as proposed in #1602.

Reviewers should focus on:
The method accepts a closure as well as a class name, to allow for more complex logic to be encapsulated in a class.

Confirmed

• Backend changes: tests are green (run php vendor/bin/phpunit).

Required changes:

• Fix database columns in flarum/statistics
• Adopt extender in flarum/statistics

[dsevillamartin]

@franzliedke I think that @tobscure had in mind (and mentioned a few times) is that Flarum flattens the extend.php array, so you can use the extenders in multiple files and require them (however that's done) in extend.php

How would it work with the Listener, anyway? I don't see a specific method being called or anything... though that may be because of my nonexistent Laravel knowledge.

[franzliedke]

@datitisev You mean having a separate file with a rather long extender callback instead of my proposed solution with an extra class?

Not sure whether that works here, as we're just calling one more method on the Frontend extender, which is very short otherwise.

[franzliedke]

How would it work with the Listener, anyway? I don't see a specific method being called or anything... though that may be because of my nonexistent Laravel knowledge.

The class would have an __invoke method, which means it can be treated like a closure or any other callable.

[tobyzerner]

Is it necessary to add a new collection of "callbacks" to the HtmlDocumentFactory? That's what "content" is for. We could do this instead:

$content = new class implements ContentInterface {
    public $callback;
    public function populate(HtmlDocument $view, Request $request)
    {
        $this->callback($view, $request);
    }
});

$content->callback = $callback;

$view->add($content);

[franzliedke]

Hmm, good point. What about getting rid of ContentInterface and making those classes invokable, too?

[franzliedke]

Or we can keep the interface, and the existing listener class would implement that interface instead of being invokable.

[tobyzerner]

I like your second suggestion, as per my comment in the other thread :)

[franzliedke]

I've done this now. I changed ContentInterface to use __invoke instead of populate, so that we can just drop in closures instead (we weren't type-hinting anyway).

[franzliedke]

All done from my side.

[franzliedke]

flarum/statistics can now get by without implementing the ContentInterface (it just defines __invoke, without any implements ContentInterface). This would mean we can consider ContentInterface private API for now, and I kind of like that.