Startdatum für Kalender wird manchmal vom falschen Zeitrahmen genommen

[nelarsen]

Eine Verleihstation hat regelmäßig an drei Arbeitstagen der Woche geöffnet und außerdem am ersten Samstag im Monat. Für die regelmäßigen drei Tage habe ich einen Zeitrahmen für das ganze Jahr angelegt und für jeden 1. Samstag im Monat zusätzlich einen eintägigen Zeitrahmen. Für ein Jahr ergibt sich somit 13 Zeitrahmen. Das scheint grundsätzlich zu funktionieren - ich hoffe, dass es so vorgesehen ist?
Aber seit dem 3.2. (erster Samstag im Februar) werden im Kalender zum Artikel erst Termine ab dem 02.03. angezeigt. Das ist der erste Samstag im März. Im Februar gibt es buchbare Termine vom "Hauptzeitrahmen" für das ganze Jahr. Das Problem dürfte in Calendar.php um getClosestBookableTimeFrameForToday() sein. Ich vermute, dass die Logik fehlerhaft ist, wenn die buchbare Tage für ein Lastenrad sich aus mehreren Zeitrahmen zusammensetzen.
Im Anhang habe ich das Problem ein bisschen genauer dokumentiert.
closestTimeframeBug.docx

[hansmorb]

Danke, ich konnte den Fehler nachvollziehen. Zum Überbrücken kanst du auch dem durchgehenden Zeitrahmen (also bei euch der Mo-Fr Zeitrahmen) ein Startdatum geben welches näher am aktuellen Datum liegt.

[chriwen]

@nelarsen Vielen Dank für deinen ausführlichen Report. Nach meinen Überlegungen ist dies ein Sonderfall, der im Prinzip nur auftritt, wenn für bestimmte Szenarien wirklich nur ein einziger Tag als buchbarer Zeitrahmen definiert wird. Im Standardfall sollten Zeitrahmen ja für längere Perioden angelegt sein. Um mit diese Sonderfälle mit eintägigen Timeframes umzugehen, könnten wir die Funktion folgendermaßen anpassen:

`private static function getClosestBookableTimeFrameForToday( $bookableTimeframes ): ?\CommonsBooking\Model\Timeframe {
// Sort timeframes by startdate proximity to current time
usort(
$bookableTimeframes,
function ( \CommonsBooking\Model\Timeframe $item1, \CommonsBooking\Model\Timeframe $item2 ) {
$currentDate = strtotime(date('Y-m-d')); // Get the current date at midnight for comparison
$item1StartDateDistance = abs( $currentDate - strtotime(date('Y-m-d', $item1->getStartDate())) );
$item1EndDateDistance = abs( $currentDate - strtotime(date('Y-m-d', $item1->getEndDate())) );
$item1SmallestDistance = min( $item1StartDateDistance, $item1EndDateDistance );

        $item2StartDateDistance = abs( $currentDate - strtotime(date('Y-m-d', $item2->getStartDate())) );
        $item2EndDateDistance   = abs( $currentDate - strtotime(date('Y-m-d', $item2->getEndDate())) );
        $item2SmallestDistance  = min( $item2StartDateDistance, $item2EndDateDistance );

        return $item1SmallestDistance <=> $item2SmallestDistance;
    }
);

// Filter out the timeframes to ensure they are candidates for being the closest or next closest
$filteredTimeframes = array_filter($bookableTimeframes, function($timeframe) use ($bookableTimeframes) {
    return date('Y-m-d', $timeframe->getStartDate()) === date('Y-m-d', $timeframe->getEndDate());
});

// If the closest timeframe has the same start and end date, attempt to return the next one
if (count($filteredTimeframes) > 0) {
    $closestTimeframe = array_shift($filteredTimeframes); // Get the closest timeframe
    if (!empty($filteredTimeframes)) {
        // If there's another timeframe, check if the next one is actually closer without the same start and end date
        $nextClosestTimeframe = array_shift($filteredTimeframes);
        if ($nextClosestTimeframe) {
            return $nextClosestTimeframe; // Return the next closest timeframe
        }
    }
    return $closestTimeframe; // Return the closest if it's the only one or if the next closest doesn't exist
}

return null; // Return null if no valid timeframe is found

}
`
@hansmorb Siehst du da ein Problem in der Anpassung?

[hansmorb]

@chriwen
Nicht nur für einen einzigen Tag, schau mal im verlinkten PR den Testcase an. da wird ein Zeitrahmen mit -50 Tagen und +365 Tagen erstellt und einer in +20 Tagen bis +40 Tagen.

Die Funktion nimmt nur den kleinsten Abstand zu entweder Anfang und Ende des nächsten Zeitrahmens. Vielleicht wurde die Funktion mal erstellt als sich Zeitrahmen noch nicht überlappen konnten, ansonsten ergibt das nämlich nicht so viel Sinn.

Ich hätte eher gesagt, dass wir statt der Funktion den Zeitrahmen mit dem geringsten Startdatum nehmen außer er ist schon verstrichen. Aber vielleicht können wir da auch nochmal ein paar mehr Edge Cases definieren und dafür dann Tests schreiben um zu schauen, dass das auch wirklich mit allen Konfigurationen funktioniert.

[hansmorb]

Extrem krude Zeichnung aber die getClosestBookableTimeframe nimmt aktuell TF 2 als "closest bookable", weil der kleinste Abstand von entweder Start oder Ende zu "now" genommen wird. Der Abstand vom Start von 1 und now und Ende von 1 und now ist größer als der Abstand von now zum Start von 2, deshalb wird 2 als der closest bookable Timeframe angesehen und das Startdatum aller Zeitrahmen irgendwo in der Zukunft angesetzt.

Deshalb könnte das Problem auch behoben werden indem der Start von 1 möglichst nah an "now" gesetzt wird.

[chriwen]

Die Ursprungsidee, warum wir den nächstmöglichen Zeitrahmen überhaupt suchen lag darin begründet, dass wir ja einen Zeitrahmen benötigen, um die Konfiguration der Maximalen Buchungsdauer, maximaler Buchungszeitraum auszulesen. Da diese Konfiguration ja über alle Zeitrahmen in einem Zeitfenster eindeutig sein muss, können wir diese ja nicht einfach nur aus einem beliebigen Zeitrahmen entnehmen. Deshalb war die Idee, einen Zeitrahmen zu nutzen der zur aktuellen Zeit eben gerade gültigkeit hat.

Ich denke aber, dass dein Vorschlag @hansmorb einfach den ersten gültigen Zeitrahmen zu nehmen, der noch nicht abgelaufen ist, genauso funktionieren kann. Hier sollten wir zwar auch wieder die eintägigen ausschließen, aber im Grunde sollte das funktionieren.

Alternativ bestünde die konzeptionelle Möglichkeit zu entscheiden, dass man die Zeitrahmenübergreifende Konfigurationen (max-days, lastbookable Day etc.) auf den Standort auslagert, bzw. dort optional einen default-Wert einstellen lässt, der dann (bei aktivierter Option) diese Einstellung automatisch für alle Zeitrahmen an dieser Station setzt. So hätten wir immer eine gültige Grundkonfiguration.

[hansmorb]

@chriwen Warum genau müssen wir dann 1-Tägige TFs davon ausnehmen?