location reminder emails

[poilu]

Added 2 email templates to the reminder tab of the CommonsBooking settings page. These notifiactions are meant to be sent to locations before items are picked up or returned (booking starts or end) and can be skipped for certain user roles. The contact emails of the locations are used and need to be set. This reminder option has to be activated for each location individually on the corresponding admin page.

EDIT: TO-Do Dokumentation:

• https://commonsbooking.org/?p=1763

[hansmorb]

Huhu, vielen Dank für deinen PR. Lass die Übersetzungen ruhig erstmal raus, wir bauen die String später in den Master ein. Wenn du magst kannst du aber gerne die Übersetzungen in den PR schreiben, dann können wir das nachher einbauen.

[hansmorb]

Vielen Dank für den schönen PR! Ich habe nur ein paar Kleinigkeiten. Die Funktion der Nutzerrollenprüfung habe ich noch nicht ganz verstanden, könnte man da nicht vielleicht einfach nur für CB Manager das Senden von E-Mails auslassen?

[poilu]

Vielen Dank für den schönen PR! Ich habe nur ein paar Kleinigkeiten. Die Funktion der Nutzerrollenprüfung habe ich noch nicht ganz verstanden, könnte man da nicht vielleicht einfach nur für CB Manager das Senden von E-Mails auslassen?

Im Zusammenhang mit CB1 nutzen wir die Funktion, um Benachrichtigungen über Blocker-Buchungen (Usage-Restrictions) zu verhindern. Wenn wir das Plugin bei fLotte Berlin weiter nutzen, müssten wir dafür eine Lösung haben. Vielleicht ließe sich das aber auch besser an anderer Stelle mit einem Filter lösen. Was meinst du?

[poilu]

Huhu, vielen Dank für deinen PR. Lass die Übersetzungen ruhig erstmal raus, wir bauen die String später in den Master ein. Wenn du magst kannst du aber gerne die Übersetzungen in den PR schreiben, dann können wir das nachher einbauen.

Ok, dann ändere ich das wieder zurück auf den Stand vom master und merge den aktuellen Stand von da auch in den feature-Branch. Dann sollte es keine Konflikte geben, wenn der PR ins Haupt-Repo übernommen wird.

[hansmorb]

Vielen Dank für den schönen PR! Ich habe nur ein paar Kleinigkeiten. Die Funktion der Nutzerrollenprüfung habe ich noch nicht ganz verstanden, könnte man da nicht vielleicht einfach nur für CB Manager das Senden von E-Mails auslassen?

Im Zusammenhang mit CB1 nutzen wir die Funktion, um Benachrichtigungen über Blocker-Buchungen (Usage-Restrictions) zu verhindern. Wenn wir das Plugin bei fLotte Berlin weiter nutzen, müssten wir dafür eine Lösung haben. Vielleicht ließe sich das aber auch besser an anderer Stelle mit einem Filter lösen. Was meinst du?

Ne, ich frage nun konkret weil ihr damit ja wahrscheinlich abfangen wollt, dass die Buchungen von den Menschen denen die Station gehört mit erwähnt werden. Für diesen Fall könnte man aber statt der definierten Rollen auch einfach eine Checkbox nehmen die der Station zugewiesenen Nutzenden rausnimmt.

[codecov]

Codecov Report

Attention: Patch coverage is 47.95918% with 51 lines in your changes are missing coverage. Please review.

Project coverage is 45.83%. Comparing base (1b77c2b) to head (ef498a1).

Files	Patch %	Lines
src/Service/Booking.php	0.00%	23 Missing ⚠️
src/Service/Scheduler.php	0.00%	14 Missing ⚠️
src/Wordpress/CustomPostType/Location.php	0.00%	12 Missing ⚠️
src/Messages/LocationBookingReminderMessage.php	94.11%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1410      +/-   ##
============================================
+ Coverage     45.70%   45.83%   +0.13%     
- Complexity     2560     2572      +12     
============================================
  Files            93       95       +2     
  Lines         10096    10155      +59     
============================================
+ Hits           4614     4655      +41     
- Misses         5482     5500      +18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[poilu]

Vielen Dank für den schönen PR! Ich habe nur ein paar Kleinigkeiten. Die Funktion der Nutzerrollenprüfung habe ich noch nicht ganz verstanden, könnte man da nicht vielleicht einfach nur für CB Manager das Senden von E-Mails auslassen?

Im Zusammenhang mit CB1 nutzen wir die Funktion, um Benachrichtigungen über Blocker-Buchungen (Usage-Restrictions) zu verhindern. Wenn wir das Plugin bei fLotte Berlin weiter nutzen, müssten wir dafür eine Lösung haben. Vielleicht ließe sich das aber auch besser an anderer Stelle mit einem Filter lösen. Was meinst du?

Ne, ich frage nun konkret weil ihr damit ja wahrscheinlich abfangen wollt, dass die Buchungen von den Menschen denen die Station gehört mit erwähnt werden. Für diesen Fall könnte man aber statt der definierten Rollen auch einfach eine Checkbox nehmen die der Station zugewiesenen Nutzenden rausnimmt.

So ganz abschätzen lässt sich noch nicht, wie sich mglw. das Nutzzungsszenario verändern wird. Möglicherweise brauchen wir die Blocker-Rolle gar nicht mehr. Mir scheint, es ist sinnvoll zweigleisig zufahren: die von dir vorgeschlagene Checkbox für dem Item zugeordnete CB-Manager und eine/n Action/Filter einzuführen, die/der es ermöglicht, spezifischer Emails abzufangen.

[hansmorb]

Oder alternativ einen kleinen Check, ob die E-Mail vom Buchungsautor gleich der zu benachrichtigenden E-Mail ist. Dann werden einfach doppelte Email verhindert. Thomas Richter ***@***.***> schrieb am Do., 16. Nov. 2023, 13:12:

…

Vielen Dank für den schönen PR! Ich habe nur ein paar Kleinigkeiten. Die Funktion der Nutzerrollenprüfung habe ich noch nicht ganz verstanden, könnte man da nicht vielleicht einfach nur für CB Manager das Senden von E-Mails auslassen? Im Zusammenhang mit CB1 nutzen wir die Funktion, um Benachrichtigungen über Blocker-Buchungen (Usage-Restrictions) zu verhindern. Wenn wir das Plugin bei fLotte Berlin weiter nutzen, müssten wir dafür eine Lösung haben. Vielleicht ließe sich das aber auch besser an anderer Stelle mit einem Filter lösen. Was meinst du? Ne, ich frage nun konkret weil ihr damit ja wahrscheinlich abfangen wollt, dass die Buchungen von den Menschen denen die Station gehört mit erwähnt werden. Für diesen Fall könnte man aber statt der definierten Rollen auch einfach eine Checkbox nehmen die der Station zugewiesenen Nutzenden rausnimmt. So ganz abschätzen lässt sich noch nicht, wie sich mglw. das Nutzzungsszenario verändern wird. Möglicherweise brauchen wir die Blocker-Rolle gar nicht mehr. Mir scheint, es ist sinnvoll zweigleisig zufahren: die von dir vorgeschlagene Checkbox für dem Item zugeordnete CB-Manager und eine/n Action/Filter einzuführen, die/der es ermöglicht, spezifischer Emails abzufangen. — Reply to this email directly, view it on GitHub <#1410 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABRCVSDUTLODWTXRUV3Y36LYEZJRTAVCNFSM6AAAAAA7LCYIQOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJUHE4DMNJZHA> . You are receiving this because you commented.Message ID: ***@***.***>

[poilu]

Wegen der bestehenden Unklarheiten, ob und wie Einschränkungen gebraucht werden, hab ich den Teil mit den Nutzer-Rollen jetzt rausgenommen und stattdessen einen Filter vor dem Senden eingebaut. Dann ließe sich so etwas oder ähnliches bei konkretem Bedarf auch unabhängig vom Plugin umsetzen.

[hansmorb]

Okay, wenn alles fertig ist dann stell den PR gerne auf "Review" :)

[hansmorb]

Ich kann aus irgendeinem Grund nicht auf euren Branch pushen. Ich habe noch zwei Änderungen:

1. Zum Fix der E-Mail-Absender Sonderzeichen falsch dargestellt #1433 müssen noch, analog zur aktuellen BookingReminderMessage (dafür müsst ihr erst mit dem Master mergen), diese Zeilen geändert werden:
   

2. Ich hätte das noch als Description für die Switches in der OptionsArray eingefügt:
   'desc' => esc_html__( 'The reminders need to be enabled for all locations individually. This is only the main on/off switch.', 'commonsbooking' ),

[hansmorb]

Vielen Dank für die Anpassungen, das passt so für mich. Vielleicht noch zu diskutieren: Ist die Standardnachricht an die Station zu knapp gehalten?

Vielleicht sollten noch Informationen zu dem Buchenden und bei Bedarf ein Buchungscode hinzugefügt werden. Dann wäre es "out of the box" noch nützlicher.

Vorschlag:

<h2>Hi,</h2>
<p>The booking period for the item {{item:post_title}} at {{location:post_title}} will end soon.<br>
The booking period: {{booking:formattedBookingDate}}<br><br>
This item has been booked by {{user:first_name}} {{user:last_name}} ( {{user:user_email}} ). <br>


{{booking:getEmailSignature}}

[hansmorb]

Da für den Versand der Erinnerungsemails die gleiche Standort-Emailadresse verwendet wird, wie für die Buchungskopien, ist es momentan nicht möglich die Erinnerungsemails zu versenden, ohne auch Buchungskopien zu versenden. Letzteres passiert nämlich, sobald eine Adresse eingetragen wird, während die Erinnerungsmails durch eine Checkbox aktiviert werden. Es ist zu überlegen, ob dies nicht auch für die Buchungskopie der Fall sein sollte (es würde dem CB legacy Verhalten entsprechen). Dafür muss aber sichergestellt werden, dass beim Plugin-Update das bisherige Verhalten beibehalten wird, also für Standorte mit eingetragenen Kontakt-Emailadressen die entsprechende Checkbox programmatisch gesetzt wird.

Habe gerade leider nicht so super viel Zeit deinen Code da gegenzulesen aber wurde das nicht mit #1515 für euch gelöst?

[hansmorb]

Achso, und zu dem Upgrade: Mittlerweile sind wir ja auf 2.9 also wäre das wenn dann 2.9.1. Wenn ihr da 2.8.6. eintragt dann wird die Funktion bei einem Upgrade von 2.9 auf 2.9.1 nicht mehr ausgeführt. Es wird also nur etwas ausgeführt, was auf >=2.8.6 updatet.

[poilu]

Habe gerade leider nicht so super viel Zeit deinen Code da gegenzulesen aber wurde das nicht mit #1515 für euch gelöst?

Ja, hat sich damit erledigt. Das war mir entgangen, dass es dieses Ticket gibt.

[hansmorb]

Ai sorry, dass du dir da die doppelte Arbeit gemacht hast

[hansmorb]

Könnt ihr vielleicht noch den vorgeschlagenen Text mit den Nutzendendaten übernehmen? Und note to self ist noch Unit Tests zu bauen. Das muss aber nach dem merge geschehen, da ich nicht auf den Branch pushen kann.

[hansmorb]

@datengraben Kannst du da nochmal drüberschauen? Ich habe jetzt doch ein wenig mehr refactored, primär habe ich den MessageRecipient als Klasse hinzugefügt, damit auch E-Mails an Personen versendet werden können die kein \WP_User sind (wie z.B. die Location Emails).

Dann ist mir noch aufgefallen, dass die Service\Booking Klasse viele Methoden enthält die einfach nur Logik nachbauen die eigentlich so schon im Repository existieren, die Funktionen habe ich dann stattdessen verwendet und sie zur Sicherheit nochmal mit mehr Tests versehen. Leider habe ich es nicht hingekriegt auch funktionierende Tests für die Klasse zu schreiben aber da es jetzt nicht mehr von ungetesteten Queries abhängt ist das vielleicht sicherer als die vorherige Implementierung.