terminal42 / contao-notification_center

The most popular notification configuration extension for the Contao Open Source CMS!
64 stars 38 forks source link

Compatibility with Contao 4.7 #168

Closed reluem closed 5 years ago

reluem commented 5 years ago

Hi there,

with Contao 4.7, the tl_newsletter_recipients.token was removed in favor of the new opt-in. Contao in combination with the newsletter module and the notification center is thus not compatible any longer. Contao 4.7.1, NotificationCenter 1.5.3

Doctrine\DBAL\Exception\InvalidFieldNameException: An exception occurred while executing 'SELECT tl_newsletter_recipients.* FROM tl_newsletter_recipients WHERE tl_newsletter_recipients.token='121c28a6259ad49ff09fa8e759469315'':

SQLSTATE[42S22]: Column not found: 1054 Unknown column 'tl_newsletter_recipients.token' in 'where clause'

mlwebworker commented 5 years ago

Ich konnte in meiner Testinstallation kein offensichtliches Problem feststellen (Contao 4.7.1, NotificationCenter 1.5.3)

reluem commented 5 years ago

Zum Beispiel beim Bestätigen eines Newsletter Abos über den token. Bei 2 Test-Installationen habe ich außerdem das Install-Tool nicht ausführen können.

derastronaut commented 5 years ago

Bei 2 Test-Installationen habe ich außerdem das Install-Tool nicht ausführen können.

Das kann ich bestätigen. Ich habe in meinem Fall jede Erweiterung deinstalliert und nachdem das Notificationcenter deinstalliert war, kam ich ins Installtool.

mlwebworker commented 5 years ago

Mit dem Installtool hatte ich defintiv keine Probleme in meiner Testinstallation. Was die Bestätigung eines Newsletter Abos betrifft, das habe ich in meiner Testinstallation nicht nachgestellt. Vielleicht solltet Ihr zwei noch die genauen Informationen zu Contao-Version und Version des Notification Centers hier nachreichen.

derastronaut commented 5 years ago

In meinem Fall: Contao-Upgrade von 4.6.14 auf 4.7.0. NC sowohl 1.5.2 als auch 1.5.3.

Ich versuche es gerade mal mit einem Upgrade von 4.6.14 auf 4.7.1.

derastronaut commented 5 years ago

Beim Upgrade von 4.6.14 auf 4.7.1 erhalte ich nach dem Login ins Installtool eine weiße Seite. NC ist Version 1.5.3. Logs gibt es zu diesem Vorgang nicht.

Deinstalliere ich vor dem Contao-Upgrade das Notificationcenter, so komme ich nach erfolgtem Upgrade ins Installtool und kann das Upgrade ohne Murren abschließen.

Ich nutze PHP 7.2 (CGI).

richardhj commented 5 years ago

Auf den Opt-In Service in a3f3b28b7158c71b82ae52754f22176ed0b3f86a umgestellt.

Column not found: 1054 Unknown column 'tl_newsletter_recipients.token' in 'where clause'

Diese Fehlermeldung sollte nicht mehr auftauchen.

Bei dem Install-Tool sind mir nie Probleme begegnet. @reluem @derastronaut könnt ihr bitte testen, ob das Problem noch auftritt: composer.json

    "require": {
        "terminal42/notification_center": "dev-hotfix/1.5.4 as 1.5.4"
    },

Wenn ja, bitte das Install-Tool nochmal mittels app_dev.php aufrufen.

reluem commented 5 years ago

Newsletter abonnnieren und Token funktionieren jetzt. Im Opt-In Menüpunkt kann man aber einen Token erneut zuschicken, dabei tritt dann ein Fehler auf:

LogicException: Please provide subject and text to send the token

at /Users/xyz/PhpstormProjects/xyz/vendor/contao/core-bundle/src/OptIn/OptInToken.php:105 at Contao\CoreBundle\OptIn\OptInToken->send() (/Users/xyz/PhpstormProjects/xyz/vendor/contao/core-bundle/src/Resources/contao/dca/tl_opt_in.php:185) at tl_opt_in->resendToken(object(DC_Table)) (/Users/xyz/PhpstormProjects/xyz/vendor/contao/core-bundle/src/Resources/contao/classes/Backend.php:427) at Contao\Backend->getBackendModule('opt_in', null) (/Users/xyz/PhpstormProjects/xyz/vendor/contao/core-bundle/src/Resources/contao/controllers/BackendMain.php:169) at Contao\BackendMain->run() (/Users/xyz/PhpstormProjects/xyz/vendor/contao/core-bundle/src/Controller/BackendController.php:48) at Contao\CoreBundle\Controller\BackendController->mainAction() (/Users/xyz/PhpstormProjects/xyz/vendor/symfony/http-kernel/HttpKernel.php:151) at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1) (/Users/xyz/PhpstormProjects/xyz/vendor/symfony/http-kernel/HttpKernel.php:68) at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true) (/Users/xyz/PhpstormProjects/xyz/vendor/symfony/http-kernel/Kernel.php:200) at Symfony\Component\HttpKernel\Kernel->handle(object(Request)) (/Users/xyz/PhpstormProjects/xyz/web/app_dev.php:83) at require('/xyz/xyz/PhpstormProjects/xyz/web/app_dev.php') (/Users/xyz/.composer/vendor/laravel/valet/server.php:158)

richardhj commented 5 years ago

Den Fehler habe ich auch bemerkt, kann man aber mit der aktuellen Contao Implementierung des Opt-in Service nicht beheben. Dies würde verlangen, dass die E-Mail über den Contao Core verschickt worden wäre.

reluem commented 5 years ago

Okay, ist ja auch eher ein Edge-Case…

Das install-tool spinnt leider trotzdem noch. Keine Fehlermeldung im Log leider.

richardhj commented 5 years ago

Aufruf mit app.dev.php/contao/install zeigt was?

reluem commented 5 years ago

Leider gar nichts, blank page.

Im Log steht auch nichts, deswegen ist es wirklich schwierig da irgendwas nachzuvollziehen!

aschempp commented 5 years ago

Den Fehler habe ich auch bemerkt, kann man aber mit der aktuellen Contao Implementierung des Opt-in Service nicht beheben. Dies würde verlangen, dass die E-Mail über den Contao Core verschickt worden wäre.

Sollten wir dazu einen Bug Report bei Contao machen?

richardhj commented 5 years ago

Sollten wir dazu einen Bug Report bei Contao machen?

Erschwerend kommt hinzu, dass eine Notification, mehrere E-Mails beinhalten kann. Hier ist nicht möglich zuzordnen, welche Nachricht die "wirkliche" Opt-In-E-Mail ist. Hier zum Vergleich nochmal das OptInModel von Contao https://github.com/contao/contao/blob/4.7.1/core-bundle/src/Resources/contao/models/OptInModel.php#L25-L27.

richardhj commented 5 years ago

Leider gar nichts, blank page. Im Log steht auch nichts, deswegen ist es wirklich schwierig da irgendwas nachzuvollziehen!

Ich habe mehrere Installationen auf Contao 4.7, auch inkl. Notification Center. Bei mir habe ich keine Probleme. Kann ich zum aktuellen Zeitpunkt auch nicht nachstellen und nachvollziehen.

aschempp commented 5 years ago

Sollten wir dazu einen Bug Report bei Contao machen?

Erschwerend kommt hinzu, dass eine Notification, mehrere E-Mails beinhalten kann. Hier ist nicht möglich zuzordnen, welche Nachricht die "wirkliche" Opt-In-E-Mail ist.

Stimmt, und wir wissen ja nicht einmal ob es eine E-Mail war (oder eine SMS etc.). In diesem Fall müsste Contao die Möglichkeit bieten dass keine E-Mail dazu gehört?

reluem commented 5 years ago

Leider gar nichts, blank page. Im Log steht auch nichts, deswegen ist es wirklich schwierig da irgendwas nachzuvollziehen!

Ich habe mehrere Installationen auf Contao 4.7, auch inkl. Notification Center. Bei mir habe ich keine Probleme. Kann ich zum aktuellen Zeitpunkt auch nicht nachstellen und nachvollziehen.

Um das nochmal zu konkretisieren, habe ich eine leere Installation aufgesetzt.

{
    "type": "project",
    "require": {
        "contao/calendar-bundle": "^4.7",
        "contao/comments-bundle": "^4.7",
        "contao/faq-bundle": "^4.7",
        "contao/listing-bundle": "^4.7",
        "contao/manager-bundle": "^4.7",
        "contao/news-bundle": "^4.7",
        "contao/newsletter-bundle": "^4.7",
        "terminal42/notification_center": "dev-hotfix/1.5.4 as 1.5.4"
    },
    "extra": {
        "contao-component-dir": "assets"
    },
    "scripts": {
        "post-install-cmd": [
            "Contao\\ManagerBundle\\Composer\\ScriptHandler::initializeApplication"
        ],
        "post-update-cmd": [
            "Contao\\ManagerBundle\\Composer\\ScriptHandler::initializeApplication"
        ]
    }
}

Ab dem Installieren des NC kommt die weiße Seite im Install-Tool. Hoster: all-inkl PHP Version 7.2.14-nmm1

richardhj commented 5 years ago

Stimmt, und wir wissen ja nicht einmal ob es eine E-Mail war (oder eine SMS etc.). In diesem Fall müsste Contao die Möglichkeit bieten dass keine E-Mail dazu gehört?

🔜 contao/contao#387

Um das nochmal zu konkretisieren, habe ich eine leere Installation aufgesetzt.

Ich habe die gleiche Installation aufgesetzt, auch PHP 7.2, gleiche composer.json. keine Probleme. Guck nochmal, ob es bei All-Inkl. einen anderen Error-Log gibt. Unter Umständen werden manche PHP-/Apache-Fehler nicht in der var/logs Log-Datei geloggt.

reluem commented 5 years ago

Okay, das scheint wirklich zu funktionieren.

Ein Memory Limit Problem mit dem Install-Tool? 🤨

[07-Mar-2019 11:41:42 Europe/Berlin] PHP Fatal error: Allowed memory size of 268435456 bytes exhausted (tried to allocate 1212416 bytes) in /www/htdocs/xyz/xyz/var/cache/prod/ContainerOXgeyBp/appContao_ManagerBundle_HttpKernel_ContaoKernelProdContainer.php on line 408 [07-Mar-2019 11:41:42 Europe/Berlin] PHP Fatal error: Allowed memory size of 268435456 bytes exhausted (tried to allocate 1212416 bytes) in /www/htdocs/xyz/xyz/var/cache/prod/ContainerOXgeyBp/appContao_ManagerBundle_HttpKernel_ContaoKernelProdContainer.php on line 413

Mit einem erhöhten Memory Limit in der .htaccess funktioniert alles wieder. Das Installtool braucht aber mit dem NC 300-400MB memory, während die normale Installation nur ca 40Mb benötigt.

richardhj commented 5 years ago

Mit einem erhöhten Memory Limit in der .htaccess funktioniert alles wieder. Das Installtool braucht aber mit dem NC 300-400MB memory, während die normale Installation nur ca 40Mb benötigt.

@aschempp Can we address this issue? May there be a problem generating the SQL statements?? Idk. Otherwise, I'll close this issue.

Toflar commented 5 years ago

This looks really weird to me. I don't see why we would increase memory usage that much. Can you reproduce this on your system? I don't think it's related to NC to be honest.

richardhj commented 5 years ago

In my test system with the above-mentioned composer.json I do have a peak memory usage of 2.0 MB as per Symfony profiler (Tab performance) and a peak memory usage of 16.0 MB after running php vendor/bin/contao-console cache:clear --env=dev --no-warmup.

Toflar commented 5 years ago

So it's defnitely not an NC problem. Maybe a templates folder with 7 billion files in it which don't belong there?

reluem commented 5 years ago

In my test system with the above-mentioned composer.json I do have a peak memory usage of 2.0 MB as per Symfony profiler (Tab performance) and a peak memory usage of 16.0 MB after running php vendor/bin/contao-console cache:clear --env=dev --no-warmup.

in this exact setup I have a peak of 370MB. This is just a test installation of core + NC without any files/templates at all. I can provide access to the system if you can't reproduce this on your systems…

aschempp commented 5 years ago

Ich bezweifle auch stark dass die Probleme direkt etwas mit dem Notification Center zu tun haben. @derastronaut lass das doch bitte von einem Entwickler deines Vertrauens auf deinem System analyiseren, solch einen Service können wir für unsere kostenlosen Erweiterungen leider nicht anbieten.

richardhj commented 5 years ago

@reluem bei einem aktuellen Projekt, gehostet auf All-Inkl, soll auch der Memory-Limit von 256M überstiegen werden, wenn nur die Artikel oder die Seitenstruktur aufgerufen werden soll. Keine Ahnung, was da bei All-Inkl los ist. Auf jeden Fall nicht mit dem NC related.