Kuestenschmiede / ForumBundle

The contribution brick of the Contao GIS-kit con4gis. Powerful discussion forum amd ticketcenter for Contao.
https://forum.con4gis.org
GNU Lesser General Public License v3.0
5 stars 3 forks source link

Erweiterung der Notification Tags bei Benachrichtigung von neuen Posts #24

Closed markocupic closed 5 years ago

markocupic commented 5 years ago

Hallo liebes Küstenschmiede Team Ich versuche seit Stunden die Notification Tokens zu erweitern. Ich habe dafür eine neues lokales Bundle gebaut. Im Backend funktioniert die Zuweisung im Notification Center soweit.

Im Frontend generiere ich eine Fehlermeldung " The token 'user_*' has not been defined." Um den Tokens die Werte zu übergeben, nutze ich den sendNotificationMessage-Hook des Notification Centers.

Die Fehlermeldung wird hier geworfen. Kommentiere ich die Zeile aus, funktioniert auch alles wunderbar. Es scheint, als ob der Hook zu spät kommt und die Tokens zu früh auf Inhalt geprüft werden.

Zu finden ist die Erweiterung hier: https://github.com/markocupic/jade_bundles/tree/master/src/markocupic/contao-con4gis-forum

Wie gelingt es mir dies updatesicher zu programmieren?

Freue mich auf einen Hint ;-)

Besten Dank!

markocupic commented 5 years ago

Update: Erwähnte Fehlermeldung in C4GForumSubscription tritt nicht auf, wenn ich in der config.php die NC-Settings nur fürs Backend zulasse.

if(TL_MODE === 'BE')
{
    // Notification Center-Einstellungen 
}

Siehe

Komischerweise werden die Notifications bei neuen Posts dann trotzdem gesendet.

coastforge-rro commented 5 years ago

Sorry für die späte Rückmeldung.

Wir haben das Versenden der Notifications inklusive der Zuweisung der Token gekapselt, um sicherzustellen, dass immer alle Token gesetzt sind. Der Hook des Notification Centers greift an der Stelle nicht (und kann das auch nicht, unabhängig von der Reihenfolge). Die Werte werden in C4GForumSubscription ab Zeile 379 gesetzt. Da müsste man ansetzen. Wir ziehen die Möglichkeit in Betracht, eigene Hooks in con4gis einzubauen, damit könnten wir an der Stelle weitere Token befüllen. Leider fehlt uns dafür momentan die Zeit.

Im Moment gibt es also keine Möglichkeit, das updatesicher aufzubauen.

markocupic commented 5 years ago

Hallo @coastforge-rro

Besten Dank für die Rückmeldung. Ich konnte das hier updatesicher aufbauen. Unschön ist, dass hier geprüft wird, ob die Tokens befüllt sind oder nicht. Zu diesem Zeitpunkt haben meine Custom-Tokens noch keinen Wert zugewiesen. Eben deshalb wird dann eine Fehlermeldung geworfen. Meine Zuweisung kommt dann erst über den sendNotificationMessage-Hook. Ich frage mich, wieso die Prüfung an der Stelle nötig ist. Wäre sie nicht, würde es sehr gut funktionieren. Evtl. könnte man die Prüfung auf nur einige wenige Tokens beschränken.

Nun gut... Gelöst habe ich das Problem, indem in der config.php meiner Extension die Tokens nur im Backend Mode gesetzt werden. Ist halt nicht gerade state of the art.

Wäre schön dazu ein Feedback zu kriegen.

Lg Marko

coastforge-rro commented 5 years ago

Achso. Jetzt habe ich die Lösung auch verstanden. Da müsste sich was machen lassen. Ich melde mich später (heute) nochmal.

Die Prüfung nehmen wir vor, da wir in der Vergangenheit Probleme mit nicht gefüllten Token hatten. Wir hielten es für besser, die Notification in dem Fall nicht zu verschicken, statt ungefüllte Token als Klartext in der Mail zu haben.

coastforge-rro commented 5 years ago

Mit den jüngsten Änderungen ist es möglich, in der Config weitere Token hinzuzufügen, ohne dass Fehler verursacht werden. Es werden lediglich die standardmäßig vorhandenen Token geprüft.

Die hinzugefügten Token müssen natürlich weiterhin mit einem Hook ins NC gefüllt werden.

Ein Update ist heute geplant.

markocupic commented 5 years ago

Das ist super! Vielen Dank dafür ;-) Liebe Grüsse Marko