liip / LiipMonitorBundle

Integrates the LiipMonitor library into Symfony
http://liip.ch
467 stars 103 forks source link

Can't override template #235

Closed knallcharge closed 4 years ago

knallcharge commented 4 years ago

Hi, I'm trying to override the main template of the bundle to integrate the view in a project. I have created the directory templates/bundles/LiipMonitorBundle/health/ and added index.html.php to that directory, but the changes to this file don't show the view is still loaded from the vendor-directory. Using "debug:twig" gives me this output:

  @LiipMonitor              /templates/bundles/LiipMonitorBundle\
                            vendor\liip\monitor-bundle/Resources/views\

so I guess the path must be right. Is it because the template is not .html.twig but rather .html.php? I have also tried creating a index.html.twig in my new directory just in case Symfony will look for twig-files, but that also didn't help.

Using Symfony 4.4.5 and LiipMonitorBundle 2.12.2

kbond commented 4 years ago

Hi @Knallcharge, this bundle handles the template in a strange way. The original reasoning was to decouple this bundle from twig I think. You're right, it needs to be a php file - it can be customized in the config (liip_monitor.view_template) - keep in mind this should be an absolute path to a php file...

To be honest, I have never used the bundle's built-in controller/template. I've always just created my own controller and used twig as normal.

Here is a quick and dirty solution:

public function healthAction()
{
    $reporter = new ArrayReporter();
    $runner = $this->get('liip_monitor.runner');
    $runner->addReporter($reporter);
    $runner->run();

    return $this->render('admin/health.html.twig', ['results' => $reporter->getResults()]);
}
# admin/health.html.twig

{% extends 'admin/layout.html.twig' %}

{% block title %}System Health Status{% endblock %}

{% block content %}
    <table class="table">
        <tbody>
        {% for result in results %}
            <tr class="{{ result.status == 1 ? 'warning' : result.status == 2 ? 'info' : result.status == 3 ? 'error' : 'success' }}">
                <td>{{ result.checkName }}</td>
                <td>{{ result.status ? result.message : null }}</td>
            </tr>
        {% endfor %}
        </tbody>
    </table>
{% endblock %}

This doesn't do anything fancy with ajax but it gets the job done and can be themed.

knallcharge commented 4 years ago

Hi @kbond , thanks for getting back! Tried the config-way, works right away, but I find it strange to use absolute paths as the path is subject to change when I deploy the project to another system. So I tried your quick and dirty solution, thanks for sharing, but the service "liip_monitor.runner" can't be found ("_Service "liipmonitor.runner" not found: even though it exists in the app's container, the container inside "App\Controller\AdminController" is a smaller service locator that only knows about..." - using dependeny injection tells me the service is there but should be aliased to "liip_monitor.runner_default"?), is there a special configuration needed? (the default config doesn't include any definition for runner)

kbond commented 4 years ago

Ah sorry, I copied that from an old app not using the new controller best practices.

I think if you inject Liip\MonitorBundle\Runner into your controller (assuming your controller has the controller.service_arguments tag) that will work:

use Liip\MonitorBundle\Runner;

//...

public function healthAction(Runner $runner)
{
    $reporter = new ArrayReporter();
    $runner->addReporter($reporter);
    $runner->run();

    return $this->render('admin/health.html.twig', ['results' => $reporter->getResults()]);
}

Yes, the bundle's controller/template is very awkward to use. I personally think we should remove it completely. Maybe have just a controller that exposes a simple json api. Theming the dashboard is so subjective, I don't think this bundle should try and provide a solution. I will be pushing for this in a 3.x release.

knallcharge commented 4 years ago

Using dependeny injection worked, I only had to add Liip\MonitorBundle\Runner: '@liip_monitor.runner_default' to my services.yaml, thanks for your support!

I agree that the controller/template-usage in this bundle is strange, I don't know if removing it completely is necessary or if it would be sufficient if the simple overriding of templates worked just like it should. But I'm fine with this workaround, closing this.