in Sachen boot.php

[christophboecker]

Ich kommentiere Dir einfach mal den Code der boot.php auf diesem Wege, da viele Punkte so oder anders gemacht werden können.

Die Abfragen auf Safemode (... !rex::isSafeMode() ...) sind doch eigentlich überflüssig sein, da das Addon im Safemode eh nicht aufgerufen wird?

Wenn das Addon eigene YForm-Tabellen mitführt, sollte yform und yform/manager in der package.yml unter requires: aufgeführt sein. Damit wäre die Absicherung via boot.php überflüssig.

Wenn man unbedingt abfragen will, bietet sich rex_plugin::get('yform','manager')->isAvailable() an, denn die fraglichen Teile von YForm sind im Manager-Plugin zu finden. Und da kein Plugin ohne Addon existiert, ist die Addon-Verfügbarkeit gleich mit gesichert.

Im Grunde gilt das auch für das Cronjob-Addon; es sei denn, die Verfügbarkeit des Cronjobs ist optional. Ansonsten würde ich auch diese Abhängigkeit in der package.ym absichern und nicht mehr weiter abfragen.

Den Tabellennamen könnte man Prefix-sicher über rex::getTable ermitteln:

rex_yform_manager_dataset::setModelClass(
    rex::getTable('blaupause'),
    Blaupause::class
);

In allen weiteren Situationen würde ich was immer geht über die Modelclass holen. also z.B. statt

$_csrf_key = rex_yform_manager_table::get('rex_blaupause')->getCSRFKey();
$params['table_name'] = 'rex_blaupause';

wäre das

$_csrf_key = Blaupause::table()->getCSRFKey();
$params['table_name'] = Blaupause::table()->getTablename();

Indirekte Abrufe statt 'rex_blaupause' oder rex::getTable('blaupause'): Dadurch kommt der Tabellenname nicht mehr direkt im Code vor und es gibt genau eine Stelle, an der der Tabellenname konkret geschrieben wird, nämlich bei der Zuweisung der Modelclass. Das eröffnet z.B. Testmöglichkeiten einfach durch Umschwenken an einer Stelle auf 'rex_blaupause_test' oder so. Und der Tabellenname muss nicht mehr ganz so klartext-artig sein, um dennoch lesbaren Code zu haben.

Weiter unten ...

if (rex::isBackend() && \rex_addon::get('blaupause') && \rex_addon::get('blaupause')->isAvailable() && !rex::isSafeMode()) {    
    ....
    if (rex::isBackend() && !empty($_REQUEST)) {
        ....
    }
}

... ist die zweite Abfrage auf rex::isBackend() überflüssig. Außerdem frage ich mich, ob die Abfrage auf $_REQUEST wirklich notwendig ist. Und wenn nötig würde ich sie nach einigen Wortgefechten mit RexStan als 0 < count($_REQUEST) schreiben.

Und zum Code für den Plus-Button (SuperIdee. imho):

$_csrf_key = rex_yform_manager_table::get('rex_blaupause')->getCSRFKey();

$token = rex_csrf_token::factory($_csrf_key)->getUrlParams();

$params = [];
$params['table_name'] = 'rex_blaupause'; // Tabellenname anpassen
$params['rex_yform_manager_popup'] = '0';
$params['_csrf_token'] = $token['_csrf_token'];
$params['func'] = 'add';

Die Methode getUrlParams liefert ja nur ein Array-Element mit dem Key '_csrf_token'; der String ist als rex_csrf_token::PARAM erhältlich. Ich würde update-sicher diese Referenz nehmen. Aber noch einfach geht es, wenn man die Rückgabe gleich als $params-Nucleus nimmt:

$_csrf_key = Blaupause::table()->getCSRFKey();

$params = rex_csrf_token::factory($_csrf_key)->getUrlParams();

$params['table_name'] = Blaupause::table()->getTablename(); // Tabellenname anpassen
$params['rex_yform_manager_popup'] = '0';
$params['func'] = 'add';

Ich finde die Überlegung ganz charmant, tabellenbezogenen Code wie z.B. Callback-Methoden (customvalidate, customaction, EPs) immer dann in die Modelclass zu packen, wenn es klar für diese Tabelle relevante Callback sind. Bei mir wird daher aus

rex_extension::register('REX_YFORM_SAVED', function (rex_extension_point $ep) {
    // Mein Code, oder meine Funktion / statische Methode aufrufen
});

entweder spezifisch:

rex_extension::register('REX_YFORM_SAVED', Blaupause::epYformDataList(...));

etwas komplexer aber allgemeingültig:

rex_extension::register('YFORM_DATA_LIST',
    static function (rex_extension_point $ep) {
        $method = 'epYformDataList';
        $class = rex_yform_manager_dataset::getModelClass($ep->getParam('table')->getTableName());
        if (method_exists($class, $method)) {
            return call_user_func([$class, $method], $ep);
        }
    },
);

---

Man könnte auch den Modelclasses eine Methode für den Tabellennamen mitgeben:

class Blaupause extends rex_yform_manager_dataset {
    ...
    public static function tablename() : string
    {
         return self::table()->getTablename();
    }
    ...
}
 

[alxndr-w]

Klingt an vielen Stellen sinnvoll und Unstimmigkeiten sind da bei mir auch historisch gewachsen. Da passiert es leicht, dass Fehler wieder und wieder kopiert wurden aus diesem Template heraus.

Wenn, dann macht es für mich besonders Sinn, diese Änderungen dann auch in meinen anderen Add-ons nachzuziehen und das überfordert mich aktuell ein wenig.

Ich versuche Mal zeitnah auf die Punkte einzugehen.

[alxndr-w]

PRs nehme ich dazu gerne an. Blaupause würde ich sonst erst vor dem nächsten darauf basierten Addon weiterentwickeln.