Kurzfristige Abmeldungszeiträume
Closed this issue · 13 comments
Hallo @markocupic und vielen Dank für deine Erweiterung!
Wir nutzen sie für ein Fitnessstudio, das seine Kurse darüber buchen lässt. Allerdings ist es für uns ein Problem, dass Abmeldungen hard coded nur in Tagen vor Mitternacht des Starts des Events angegeben werden können.
In unserem Fall wäre eine Stunde vor Start des Events sinnvoll und wünschenswert.
Meine Frage ist, ob du grundsätzlich für pull requests in diese Richtung offen bist oder ob wir einfach einen privaten Fork deiner Erweiterung dafür machen sollen? Ich denke letztlich wird es auf ein simples Datim-Feld hinauslaufen mit sanity checks im save_callback(), à la keine Abmeldung nachdem das Event vorbei ist. (Nach Start des Events kann man ja vielleicht noch use cases dafür finden.)
Nachdem die Abmeldefrist bisher in Tagen angegeben wird, wäre es ja je nach Ausgestaltung ein BC break, deshalb würde ich verstehen wenn du das ablehnst, dann forken wir.
Hallo @philvdb
Bitte, bitte. Evtl. Gäbe es noch den Mittelweg. Ich baue kein neues Feld ein, dafür an geeigneter Stelle einen Hook, der allfällige zusätzliche Felder, die du in in der dcaconfig definierst, auswertet.
So liesse sich der BC Break vermeiden. Wo genau man den Hook placiert, habe ich jetzt noch nicht näher angeschaut.
Was meinst du?
Liebe Grüsse
Marko
Cool, freut mich dass wir keinen unabhängigen Fork machen müssen!
Ehrlich gesagt habe ich aber weiterhin das Gefühl, dass es langfristig weniger Kopfschmerzen machen würde, das (Datenbank-)Feld statt mit einer Ganzzahl von Tagen doch mit einem timestamp zu belegen. Der könnte natürlich auch gerne von einem save_callback im DCA dynamisch befüllt werden, sodass man (im Backend) auch weiterhin eine Zahl in Tagen angeben kann, die dann einfach zu einem Timestamp umgerechnet wird.
Dann bräuchte man in unserem Fall nur das DCA zu manipulieren und fertig.
Durch die Änderung der Bedeutung des Datenbankfelds bräuchte man aber natürlich eine Migration - oder man lebt etwas schmutziger und fügt klammheimlich eine if-Abfrage ein:
if ($this->objEvent->unsubscribeLimit < 1000)
{
// als Tage interpretieren
}
else
{
// als timestamp interpretieren
}
Ich weiß, ich fühle mich dabei auch schmutzig, aber es würde mMn nur versagen, wenn jemand als unsubscribeLimit irgendwas zwischen dem 1.1.1970 0:00 Uhr und 1.1.1970 0:17 Uhr angibt, was ja für Events in der Zukunft mal gar keinen Sinn macht (und beim Speichern eigentlich auch verhindert werden sollte?). So könnte man bestehende Einträge auch "klammheimlich" auf timestamps upgraden.
Man muss dabei beachten, dass das aktuelle unsubscribeLimit natürlich immer relativ ist und wenn jemand die Startzeit des Events verschiebt, verschiebt sich auch das Limit - das müsste man ebenfalls z. B. im Callback berücksichtigen.
Wenn das was ist womit du leben kannst mache ich dir einen PR dazu.
Ansonsten bin ich auch offen für einen Hook - worauf es uns ankommt wäre eben, das unsubscribeLimit fassen zu können, idealerweise als timestamp.
Okay, jetzt wo ich tatsächlich im Code angefangen habe denke ich man kann es auch ohne die unschöne Überladung des Tabellenfelds lösen. Einfach ein neues Feld namens unsubscribeLimitTstamp, das ein w50 direkt neben dem bestehenden Feld ist, default NULL aber wenn gesetzt, dann hat es Vorrang vor unsubscribeLimit (entsprechend im Label erklärt).
Man kann auch im save_callback die Sicherheitsfunktion einführen, dass wenn in unsubscribeLimit eine Anzahl !=0 gesetzt ist und aber trotzdem in unsubscribeLimitTstamp auch ein Zeitpunkt, dass man dann einen Fehler wirft und darauf aufmerksam macht, dass nicht beides gelten kann (Tage vor Event und fester Zeitpunkt).
Aber das wäre logischerweise komplett ohne BC break, es braucht nur einen Aufruf des Install-tools/Symfony-Schema-Command zum Hinzufügen des neuen Felds.
Wenn ich nichts weiter höre setze ich das um und schicke dir einen PR.
Added in version 3.7.0
Danke!
Schau mal:
7df8bdb
Wenn addTime nicht gesetzt ist, werden start- und endTime mit dem Wert von startDate bzw. endDate überschrieben.
@philvdb Wenn das Startdatum bei eingestellter Startzeit genau dem Abmeldefrist-Zeitpunkt entspricht, so ist der Wert des Abmeldefrist-Zeitpunktes 3600 Sekunden grösser. Gibt es da noch ein timezone Problem? Lust zu debuggen? ;-)
Wenn addTime nicht gesetzt ist, werden start- und endTime mit dem Wert von startDate bzw. endDate überschrieben.
What? Überschrieben? Was wird überschrieben? Ich hätte erwartet, wenn das zu (int) 0 evaluiert passiert einfach ein + 0 und es ist egal ob time gesetzt ist? Hab's zugegebenermaßen nicht getestet aber sehe jetzt auch nicht was überschrieben wird?
Wenn das Startdatum bei eingestellter Startzeit genau dem Abmeldefrist-Zeitpunkt entspricht, so ist der Wert des Abmeldefrist-Zeitpunktes 3600 Sekunden grösser.
Grrr, dieses Phänomen hat mich auch schon an anderer Stelle ereilt und ich habe letztlich die blöde Stunde abgezogen weil es einfach keinen Sinn ergab :) Aber wenn das nun ein größeres, universelles Problem ist kann ich es schon bei nächster Gelegenheit debuggen.
What? Überschrieben? Was wird überschrieben?
Wenn Der Fehler tritt auf, wenn addTime nicht angewählt ist. Dann nämlich bekommt startTime den Wert von startDate. Als timestamp bekommst du dann so schöne Werte wie 312012456 anstatt 156006228. Eben doppelt so gross. Deshalb braucht es die Fallunterscheidung:
und
calendar-event-booking-bundle/src/Contao/Dca/TlCalendarEvents.php
Lines 111 to 133 in 7df8bdb
Oh wow. Starttime ist dann also nicht 0 .. sorry, das hatte ich nicht erwartet. Danke für den Bugfix! Ich versuche wenn mal bisschen Zeit ist das mit der Stunde zu debuggen.
Ja, kein Problem. Vielleicht fällt mir auch noch was ein ;-)
Fixed in 20fce93
War wohl ein kleiner Denkfehler ;-)