yakamara / yform

YForm für REDAXO 5 – Formulare im Frontend und Backend mit Verwaltung von Datenbank-Tabellen.
MIT License
77 stars 55 forks source link

Namespaces: Missing `$table` declaration for model class `FriendsOfRedaxo\Stellenangebote\Entry` oder `stellenangebote` #1497

Closed alxndr-w closed 7 months ago

alxndr-w commented 7 months ago

Gibt es eine Möglichkeit, Klassen mit und ohne Namespaces parallel zu nutzen, ohne dass YOrm die Model Class dafür fehlt?

https://github.com/alexplusde/stellenangebote/blob/507daabe0cabcff41e394007b665bb838993b8d7/boot.php#L16-L68

Andernfalls lässt sich bei YOrm-basierten Addons kein schrittweiser Übergang vollziehen.

RuntimeException: Missing $table declaration for model class "FriendsOfRedaxo\Stellenangebote\Entry" File: redaxo/src/addons/yform/plugins/manager/lib/yform/manager/dataset.php Line: 758

Stacktrace | Function | File | Line | | ------------------------------------------------- | ------------------------------------------------------------------------------- | -------- | | rex_yform_manager_dataset::modelToTable | redaxo/src/addons/yform/plugins/manager/lib/yform/manager/dataset.php | 148 | | rex_yform_manager_dataset::query | redaxo/src/addons/stellenangebote/lib/stellenangebote.php | 616 | | FriendsOfRedaxo\Stellenangebote\Entry::findOnline | redaxo/src/addons/stellenangebote/fragments/stellenangebote/uebersicht-home.php | 14 | | require | redaxo/src/core/lib/fragment.php | 114 | | rex_fragment->{closure} | redaxo/src/core/lib/util/timer.php | 56 | | rex_timer::measure | redaxo/src/core/lib/fragment.php | 112 | | rex_fragment->parse | redaxo/src/addons/plus_bs5/lib/bs5_fragment.php | 10 | | bs5_fragment->parse | rex:///module/46/output | 30 | | require | redaxo/cache/addons/structure/38.1.content | 210 | | require | redaxo/src/addons/structure/plugins/content/lib/article_content.php | 122 | | rex_article_content->getArticle | rex:///template/1 | 8 | | require | redaxo/src/addons/structure/plugins/content/lib/article_content_base.php | 443 | | rex_article_content_base->{closure} | redaxo/src/core/lib/util/timer.php | 56 | | rex_timer::measure | redaxo/src/addons/structure/plugins/content/lib/article_content_base.php | 440 | | rex_article_content_base->getArticleTemplate | redaxo/src/addons/structure/plugins/content/boot.php | 58 | | rex_package::{closure} | redaxo/src/core/lib/extension.php | 52 | | rex_extension::{closure} | redaxo/src/core/lib/util/timer.php | 56 | | rex_timer::measure | redaxo/src/core/lib/extension.php | 42 | | rex_extension::registerPoint | redaxo/src/core/frontend.php | 22 | | require | redaxo/src/core/boot.php | 157 | | require | index.php | 9 |
System report (REDAXO 5.17.0, PHP 8.3.2, MariaDB 10.6.16) | REDAXO | | | ------------: | :--------- | | Version | 5.17.0 | | PHP | | | ------------: | :--------- | | Version | 8.3.2 | | OPcache | yes | | Xdebug | no | | Database | | | ------------: | :-------------- | | Version | MariaDB 10.6.16 | | Character set | utf8mb4 | | Server | | | ------------: | :--------- | | OS | Linux | | SAPI | fpm-fcgi | | Webserver | Apache | | Request | | | ------------: | :--------------- | | Browser | Chrome/123.0.0.0 | | Protocol | HTTP/2.0 | | HTTPS | yes | | Packages | | | -----------------------: | :---------- | | accessdenied | 2.1.0 | | adminer | 2.0.1 | | auto_delete | 1.0.2 | | backup | 2.9.1 | | be_style | 3.4.0 | | be_style/customizer | 3.3.1 | | be_style/redaxo | 3.3.0 | | be_style_fluent | 4.0.1 | | bloecks | 4.0.2 | | bloecks/cutncopy | 4.0.2 | | bs5_iconpicker | 1.0.7 | | cronjob | 2.11.0 | | debug | 1.3.1 | | developer | 3.9.2 | | install | 2.12.1 | | maintenance | 2.9.2 | | media_manager | 2.16.0 | | media_manager_responsive | 1.1.5 | | mediapool | 2.14.0 | | metainfo | 2.11.0 | | mform | 7.4.3 | | multiupload | 2.1.3 | | neues | 4.1.0-beta1 | | phpmailer | 2.13.1 | | plus_bs5 | 0.1.5 | | project | dev | | project_manager | 1.5.2 | | project_manager/client | 1.1.0 | | qanda | 2.2.1 | | quick_navigation | 6.2.1 | | redactor | 2.4.2 | | speed_up | 1.4.0 | | staff | 1.4.0 | | statistics | 3.1.0 | | stellenangebote | 3.0.0-beta2 | | structure | 2.17.0 | | structure/content | 2.17.0 | | url | 2.1.0 | | users | 2.11.0 | | wenns_sein_muss | 4.0.6 | | yform | 4.2.1 | | yform/email | 4.2.1 | | yform/manager | 4.2.1 | | yform_adminer | 1.4.0 | | yform_field | 2.5.0 | | yform_geo_osm | 1.2.9 | | yform_quick_edit | 1.0.1 | | yform_spam_protection | 1.2.4 | | yform_usability | 2.1.4 | | yrewrite | 2.10.0 | | yrewrite_metainfo | 1.1.2 |
christophboecker commented 7 months ago

Nur mal für mich zum Versehen: ich nutze ja auch mittlerweile oft und gerne Namespaces und Modelclasses. Den Fehler verstehe ich nicht. Wie kann man den nachstellen?

DanielWeitenauer commented 7 months ago

Es kann immer nur eine Klasse auf eine Yform-Tabelle gemappt werden. Aber du kannst für eine der Klassen eine View anlegen und in Yform registrieren.

alxndr-w commented 7 months ago

@DanielWeitenauer Über Views habe ich als Addon-Entwickler allerdings auch keine Kontrolle.

@christophboecker wenn man auf Namespace-Schreibweise umstellen will, aber die Kompatibilitätsklassen noch mitliefern will. Nützt dann im YForm-Kontext nicht - sie können nicht gemeinsam verwendet werden.

DanielWeitenauer commented 7 months ago

@alxndr-w du kannst über MySQL doch jederzeit eine View einer Tabelle erstellen, die du vorher angelegt hast und diese dann in Yform registrieren? Ich habe die Technik schon mehrfach genutzt, um zwei Versionen einer Model-Klasse parallel zu betreiben. Ich erinnere mich, dass du das Vorgehen selbst schon öfter mal vorgeschlagen hast.

alxndr-w commented 7 months ago

Ja, aber ich habe als Addon-Entwickler keinen Einfluss aufs Hosting und möchte das nicht zur Voraussetzung zur Verwendung meines Addons machen, nur um Namespaces parallel zur alten Schreibweise zu ermöglichen.

DanielWeitenauer commented 7 months ago

Wäre aus meiner Sicht jedoch die einfachste Möglichkeit, einen einfachen Upgrade-Pfad anzubieten. Alternativ kannst du wahrscheinlich alle statischen Getter von \rex_yform_manager_dataset & Co. in deiner Klasse überschreiben und per parent::* mit der gewünschten Tabelle aufrufen. Das Mapping ist im Endeffekt ja nur eine Convenience-Option. Alle statischen Getter erlauben es, einen Tabellennamen zu übergeben. Die Mapping-Methoden selbst sind leider final. https://github.com/yakamara/yform/blob/master/plugins/manager/lib/yform/manager/dataset.php#L63 Oder man schaut mal, ob es die Möglichkeit gibt, einen EP zu Manipulation ins Mapping einzubauen.

christophboecker commented 7 months ago

Ich hab das Problem nun verstanden, nachgebaut und würde mal sagen: da sind auch irgendwann Grenzen erreicht.

Wenn ich ein Addon wie z.B. Geolocation (das ja nun längst auf Namespace läuft) heute umstellen müsste, würde ich den harten Schnitt machen. Lieber ein paar Worte mehr in die Umstellanleitung schreiben. Die Umstellung der Anwender muss eh kommen. Also neues Major-Release und fertig.

Einen view zu nutzen bedeutet doch, dass man die Tabelle ein zweites mal in YForm anlegt, doppelte Formulare pflegt, usw. ? Macht dem, der die Convinience-Methode anbieten will, einen Haufen Arbeit. ......

Just my 2 cent.

DanielWeitenauer commented 7 months ago

Hängt davon ab, wie du ein Haufen Arbeit definierst. Es lässt sich mit einer Handvoll Datenbank-Befehlen umsetzen. Ein View ist nur eine Kopie einer Datentabelle, daher müssen Formulare nicht doppelt gepflegt werden. Es wird lediglich für YOrm eine Kopie der Feld-Definitionen benötigt. Die User bekommen von der Doppelung nichts mit. Und da es ja nur ein Angebot eines bequemen Update-Pfades ist, muss es nicht für alle zukünftigen Eventualitäten vorbereitet sein.

DanielWeitenauer commented 7 months ago

Ich würde den Pfad auch nur kurzfristig anbieten. Also als letzte Minor-Version eines Major-Releases und dann mit dem Sprung auf die nächste Major-Version wieder entfernen.

alxndr-w commented 7 months ago

Also für kleinere Add-ons ist mir das zu viel kostenloser Service. Ich mache dann lieber eine dicke fette Warnung hin und biete einen Knopf an, alle Datenbank-Module automatisch zu aktualisieren. Etwaige Fragmente im Dateisystem zu aktualisieren sollte für Entwickler hoffentlich kein Problem sein.

christophboecker commented 7 months ago

@DanielWeitenauer Hi, Daniel, vieleicht verstehe ich es ja doch nicht. Mit dem View erhalte ich einen zweiten Tabellennamen. Damit muss ich dann konsequenter weise auch die Tabelle (View) in YForm importieren und ein Formular anlegen als Kopie des Originals. Klar, macht man nicht zu Fuß, sondern per SQL. Traue ich meinem SQL wenn ich das noch nicht gemacht habe? Also erstmal testen und Fehler ausbügeln. Dann habe ich zwei Tabellen. Je nach dem, was der Anwender des Addons mit der Dataset-Klasse anstellen will, kann es wieder zu Verwirrung und anderen Problemen kommen, wenn da plötzlich ein anderer Tabellenname auftaucht. Muss man dann auch wieder abfangen mit Erklärung und notfalls mit Support.

Das meine ich mit zu viel Arbeit. Deshalb .... Major Release und fertig.

DanielWeitenauer commented 7 months ago

@christophboecker Das Ursprungsthema war: Gibt es eine Möglichkeit, zwei unterschiedliche Dataset-Klassen parallel mit der gleichen Datentabelle zu nutzen. Und das ist recht einfach möglich und generisch auf jede Tabelle übertragbar. Ich habe mal rausgesucht, wie ich es gemacht habe:

<?php
$sql = rex_sql::factory();

// View anlegen
$sql->setQuery('CREATE VIEW my_view AS SELECT * FROM my_table;');

// Tabellendefinition kopieren
// Tabellensichtbarkeit auf hidden stellen, um Verwirrung zu minimieren
$sql->setQuery('INSERT INTO my_table (status, table_name, name, description, list_amount, list_sortfield, list_sortorder, prio, search, hidden, export, import, mass_deletion, mass_edit, schema_overwrite, history, createdate, updatedate, createuser, updateuser) 
SELECT
status,
"my_view",
name, description, list_amount, list_sortfield,  list_sortorder,  prio, search, 
"1",
export, import, mass_deletion, mass_edit, schema_overwrite, history, createdate, updatedate, createuser, updateuser
FROM rex_yform_table WHERE table_name = "my_table"');

// Felddefinitionen kopieren
// Die Felddefinitionen werden dynamisch erweitert, also aktuelle Spaltenliste holen
$fields = array_map(function ($field) {
    return '`'.$field.'`';
}, array_filter(array_column(rex_sql::showColumns('rex_yform_field'), 'name'), function ($field) {
    return $field !== 'id';
}));
$sql->setQuery('INSERT INTO rex_yform_field ('.implode(',', $fields).') 
SELECT
"my_view",
'.implode(',', array_filter($fields, function ($field) {return $field !== '`table_name`';})).'
FROM rex_yform_field WHERE table_name = "my_table"');

rex_addon::get('yform')->clearCache();

rex_yform_manager_dataset::setModelClass('my_view', my_view_class::class);

Und nicht vergessen: es geht nur darum, einen sanften Übergang zwischen zwei Major-Releases zu schaffen, es soll kein Dauerzustand bleiben. Denn die Umstellung auf Namespaces ist ja auch nur ein erster Schritt bei der Umstellung auf REDAXO 6.0. Support lässt sich leichter bewerkstelligen, wenn das Addon durchgängig funktionsfähig bleibt. Wenn erstmal nichts mehr geht und X Module, Templates, Klassen etc. durchforstet werden müssen ist, denke ich, mehr Panik angesagt. Ich würde den Redaxo-Entwickler:innen zurtrauen, dass sie zwischen einer "alten" und einer "neuen" Version einer Klasse unterscheiden können, wenn für sie lediglich nötig ist den alten mit dem neuen Klassennamen auszutauschen. Und zuletzt: diese Änderung würde ich tatsächlich als einzige in einem letzten z.b. 1.9.99-Release herausgeben, bevor der 2.0.0-Release kommt. Also wirklich nur als Service für einen sanften Übergang.