abrain / einsatzverwaltung

WordPress plugin for public incident reports for fire departments and other rescue services
https://einsatzverwaltung.org
GNU General Public License v2.0
57 stars 17 forks source link

Filters/Actions lassen sich nicht entfernen #122

Closed HeikoBornholdt closed 7 years ago

HeikoBornholdt commented 7 years ago

Moin,

ich möchte dein Plugin gerne an meine Bedürfnisse anpassen. Dazu müsste ich einige deiner Filters/Actions entfernen und gegen meine eigenen Austauschen. Leider hast du die Filters/Actions so hinzugefügt, dass sie nicht mehr entfernt werden können.

Siehe einsatzverwaltung-frontend.php:

    private function addHooks()
    {
        add_action('wp_enqueue_scripts', array($this, 'enqueueStyleAndScripts'));
        add_filter('the_content', array($this, 'renderContent'));
        add_filter('the_excerpt', array($this, 'filterEinsatzExcerpt'));
        add_filter('the_excerpt_rss', array($this, 'filterEinsatzExcerptFeed'));
        add_action('pre_get_posts', array($this, 'addReportsToQuery'));
    }

Es ist leider nicht möglich, die via array($this, '...') hinzugefügten Filters/Action mit remove_filter/remove_action zu adressieren.

Ich würde vorschlagen, die Filters/Action als statische Methoden hinzuzufügen (also z.B. array('Frontend', '...')). Siehe hierzu auch https://wordpress.stackexchange.com/questions/36013/remove-action-or-remove-filter-with-external-classes

Gib mir bitte Bescheid, wenn ich einen MR erstellen soll.

abrain commented 7 years ago

Bei statischen Methoden haut mir PHPMD eines auf den Deckel.

Static access causes unexchangeable dependencies to other classes and leads to hard to test code. Avoid using static access at all costs and instead inject dependencies through the constructor. The only case when static access is acceptable is when used for factory methods.

Quelle: https://phpmd.org/rules/cleancode.html#staticaccess

Ich könnte mir vorstellen eine globale Methode zu haben, mit der man die Instanz von Core bekommt und darüber die Referenzen zu den anderen Klassen beziehen kann.

Aber eine ganz andere Frage: Könnte die Änderung, die du vorhast, auch für andere interessant sein? Sprich: Könnte es ein reguläres Feature werden?

HeikoBornholdt commented 7 years ago

Ich könnte mir vorstellen eine globale Methode zu haben, mit der man die Instanz von Core bekommt und darüber die Referenzen zu den anderen Klassen beziehen kann.

Das habe ich eben bei mir ausprobiert. Damit könnte man deine Filters/Actions bei Bedarf wieder entfernen.

Ich könnte mir vorstellen eine globale Methode zu haben, mit der man die Instanz von Core bekommt und darüber die Referenzen zu den anderen Klassen beziehen kann.

Vorschlag: Den Core als Singleton und die anderen Instanzen dann via getter-Methoden zurückgeben.

Habe das direkt mal runtergetippt: https://github.com/abrain/einsatzverwaltung/compare/develop...heikobornholdt:122-filters

Sag Bescheid, wenn ich daraus einen Pull Request machen soll.

Aber eine ganz andere Frage: Könnte die Änderung, die du vorhast, auch für andere interessant sein? Sprich: Könnte es ein reguläres Feature werden?

Ich denke schon, dass auch andere einen Vorteil davon hätten. Man kann als Theme-Entwickler die unerwünschten Actions/Filters von deinem Plugin entfernen. Das Plugin lässt also allgemein besser an eigene Bedürfnisse anpassen.

In meinem konkreten Fall kann ich dann einfach via

remove_filter('the_content', array(Core::getInstance()->getFrontend(), 'renderContent'));`

die Standard-Formatierung von Einsatzberichten entfernen und dann eine eigene Formatierung via

add_filter('the_content', 'meine_einsatzbericht_formatierung');`

Ich wüsste nicht, wie ich es sonst bewerkstelligen könnte.

abrain commented 7 years ago

Aber eine ganz andere Frage: Könnte die Änderung, die du vorhast, auch für andere interessant sein? Sprich: Könnte es ein reguläres Feature werden?

Da habe ich mich vielleicht unklar ausgedrückt. Damit meinte ich nicht die Änderung, dass man Hooks entfernen kann, sondern was du eigentlich abändern wolltest. Ich hätte auch auf the_content getippt, weil die anderen Hooks in Frontend auch per Einstellungen manipuliert werden können ;)

Das klingt ja irgendwie nach #89, das schiebe ich schon lange vor mir her. Eigentlich müsste mit Util/Formatter.php schon die Grundlage gelegt sein. So wie man jetzt schon das Widget selbst gestalten kann, könnte man auch den Einsatzbericht mit Platzhaltern selbst designen.

Aber ansonsten ist auch der Einwand mit den Hooks valide, du kannst das gerne als PR einreichen.

HeikoBornholdt commented 7 years ago

Ich wollte sowohl die Ansicht einzelner, als auch die Auflistung mehrerer Einsätze in meinem Theme nach unseren Bedürfnissen anpassen. Es wird gewünscht, dass die Darstellung sich nahe an unsere noch aktuellen Datenbank orientiert: Auflistung: https://www.feuerwehr-pinneberg.de/eins%C3%A4tze/ Einzelansicht: https://www.feuerwehr-pinneberg.de/eins%C3%A4tze/2017/4774-brennt-unrat-berliner-str/

89 wünscht sich scheinbar entsprechende Optionen im Adminbereich von WordPress.

Wenn du #130 übernommen hast, kann man @sebastianroming immerhin anbieten, dass er nun über ein Theme die Darstellung anpassen kann :) .

Wegen der Datei Util/Formatter.php muss ich auch noch einmal auf dich zukommen. Aber dafür muss erst einmal #130 übernommen werden.

HeikoBornholdt commented 5 years ago

Moin,

leider besteht das Problem mit der Version 1.5 wieder. Ich komme nicht mehr an das Frontend-Objekt und kann die Filter nicht mehr austauschen.

abrain commented 5 years ago

Stimmt, das hat sich wieder geändert.

Meine Frage wäre, ob du die bestehenden Filter überhaupt aushängen musst. Wenn du sie mit einer höheren Priorität registrierst, wird dein Filter später ausgeführt und bestimmt damit was angezeigt wird. Wäre das eine Möglichkeit?

HeikoBornholdt commented 5 years ago

Meine Filter müssen dann mit der bereits manipulierten Ausgabe arbeiten. Ich müsste dann die bisherige Ausgabe parsen und die ungewollten Änderungen rückgängig machen. Das klingt nicht so robust.

abrain commented 5 years ago

Das stimmt. Ich kann dir als Übergangslösung mal dieses Stück Code anbieten, das den Filter findet und entfernt:

add_action('init', function() {
    global $wp_filter;
    $hook = 'the_content';
    $prio = 9;
    if (isset($wp_filter[$hook]) && array_key_exists($prio, $wp_filter[$hook]->callbacks)) {
        foreach ($wp_filter[$hook]->callbacks[$prio] as $key => $callback) {
            if (is_array($callback['function']) && $callback['function'][0] instanceof \abrain\Einsatzverwaltung\Frontend) {
                unset($wp_filter[$hook]->callbacks[$prio][$key]);
            }
        }
    }
}, 20);

Was müsste das Plugin bzw. die Templatefunktion mehr können, damit du die Anpassung ohne eigenen Code vornehmen kannst?

HeikoBornholdt commented 5 years ago

Vielen Dank. Ich habe es jetzt über die Templatefunktion gelöst, welche ich um eigene Platzhalter erweitert habe. Diese Platzhalter werden dann in einem eigenen Filter ersetzt.

Was mir fehlt:

Abgesehen von 1. Punkt sind das alles sehr spezielle Anforderungen. So habe ich das aktuell für mich gelöst

add_filter('the_content', function($content) {
    global $post;

    if (get_post_type() == 'einsatz') {
        // entferne Einsatzbericht-Überschrift, wenn es keinen gibt
        if (empty(get_the_content())) {
            $content = str_replace('<h2>Einsatzbericht</h2>', '', $content);
        }

        $einsatz_options = new abrain\Einsatzverwaltung\Options();
        $einsatz_permalinkController = new abrain\Einsatzverwaltung\PermalinkController();
        $einsatz_formatter = new abrain\Einsatzverwaltung\Util\Formatter($einsatz_options, $einsatz_permalinkController);
        $report = new abrain\Einsatzverwaltung\Model\IncidentReport($post);

        $content = preg_replace_callback('/%FF10incidentType%/', function() use ($einsatz_formatter, $report) {
            $art = $einsatz_formatter->getTypeOfIncident($report, false, false);

            if ($report->isFalseAlarm()) {
                $art = (empty($art) ? 'Fehlalarm' : $art.' (Fehlalarm)');
            }

            $einsatzart_description = trim($report->getTypeOfIncident()->description);
            if (empty($einsatzart_description)) { 
                return $art;
            } 
            else {
                return "<abbr title=\"" . $art. "\">" . $einsatzart_description . "</abbr>";
            }
        }, $content);

        $content = preg_replace_callback('/%FF10vehicles%/', function() use ($einsatz_formatter, $report) {
            $forces = $einsatz_formatter->getVehicles($report, true, false);
            $additionalForces = $einsatz_formatter->getAdditionalForces($report, false, false);

            return $forces . ($additionalForces ? ', ' . $additionalForces : '');
        }, $content);
    }

    return $content;
});