In num_public_holiday iteration over timestamp can be faulty with time changes

dbeniamine commented 10 months ago

Bug

num_public_holidays computes the number of holidays week-end and holidays by iterating over timestamp using the function dol_time_plus_duree, but some days are longer than 24 hours due to summer time - winter time switches.

Environment Version

17.0.2

Environment OS

Debian 11

Environment Web server

Apache/2.4.56 (Debian)

Environment PHP

7.4.33

Environment Database

10.3.31-MariaDB-0+deb10u1

Environment URL(s)

Maybe several

Expected and actual behavior

num_public_holidays computes the number of holidays week-end and holidays by iterating over timestamp using the function dol_time_plus_duree, but some days are longer than 24 hours due to summer time - winter time switches.

For instance in 2023 : 1679788800 is march 26th at midnight GMT, and 24 h later 1679871600 is march 26 GMT at 23h not march 27.

This shifts the days when computing num_public_holidays and a friday that is holiday and after this date, for instance 14 july 2023, will be seen as a saturday due to the timeshift by the function.

So if you asks for the number of open days in France between march 1 2023 and july 15 2023 you will find one more open day than expected.

This might be related to #15425

Steps to reproduce the behavior

Asks for the number of open days in France between march 1 2023 and july 15 2023 you will find one more open day than expected.

Attached files

No response

dbeniamine commented 10 months ago

If i add 5 hours (using dol_time_plus_duree) to my timestamp before calling num_open_day (so timestamps are at 5 am GMT instead of midnight) march 26th 2023 is only counted once

dbeniamine commented 9 months ago

After a few month, I had the same issue with the time change on october 29th.

I manage to build a work around (a num_open_day wrapper` and a proof of concept to improve the timestamp iterator).

The following patch is a proof of concept : with it there is no more errors around time changes. However it forces timestampStart to be at midnight, we should keep the original utc time of timestampEnd in the date construction.

diff --git a/htdocs/core/lib/date.lib.php b/htdocs/core/lib/date.lib.php
index 636a4ef4907..50bfa0e8a63 100644
--- a/htdocs/core/lib/date.lib.php
+++ b/htdocs/core/lib/date.lib.php
@@ -967,7 +967,8 @@ function num_public_holiday($timestampStart, $timestampEnd, $country_code = '',
                }

                // Increase number of days (on go up into loop)
-               $timestampStart = dol_time_plus_duree($timestampStart, 1, 'd');
+               $gmt = new DateTimeZone('UTC');
+               $timestampStart = (new DateTime(dol_print_date(dol_time_plus_duree($timestampStart, 1, 'd'), '%Y-%m-%d'), $gmt))->getTimestamp();
                //var_dump($jour.' '.$mois.' '.$annee.' '.$timestampStart);

                $i++;

The workaround, retrieve the time transitions between the given timestamp (on the server timezone). Then it calls num_open_day for each transition with dates inside with the same offset. If the time transition is negative, num_public_holiday will not count the last day as a holiday because the computed timestampStart will be after timestampEnd, so we need to call num_open_day specifically on this day.

My code below:

function get_gmt_timestamp($tms)
{
    $gmt = new DateTimeZone('UTC');
    $date = new DateTime(dol_print_date($tms, '%Y-%m-%d'), $gmt);
    return $date->getTimestamp();
}

/**
 *  Function to return number of working days (and text of units) between two dates (working days)
 *  This is a fix to https://github.com/Dolibarr/dolibarr/issues/25789 built over num_open_day
 *
 *  @param      int         $timestampStart     Timestamp for start date (date must be UTC to avoid calculation errors)
 *  @param      int         $timestampEnd       Timestamp for end date (date must be UTC to avoid calculation errors)
 *  @param      int         $inhour             0: return number of days, 1: return number of hours
 *  @param      int         $lastday            We include last day, 0: no, 1:yes
 *  @param      int         $halfday            Tag to define half day when holiday start and end
 *  @param      string      $country_code       Country code (company country code if not defined)
 *  @return     int|string                      Number of days or hours or string if error
 *  @seealso num_between_day(), num_public_holiday()
 */
function tl_num_open_day($timestampStart, $timestampEnd, $inhour = 0, $lastday = 0, $halfday = 0, $country_code = '')
{
    require_once DOL_DOCUMENT_ROOT.'/core/lib/date.lib.php';
    $tz = new DateTimeZone(getServerTimeZoneString());
    $transitions = $tz->getTransitions($timestampStart, $timestampEnd);
    $carry = 0;
    $start = $timestampStart;
    $end = 0;
    foreach ($transitions as $i => $t) {
        if ($i === 0 ) {
            $oldoffset = $t['offset'];
            continue; // First transition is timestampStart
        }
        $offset_change = $t['offset'] - $oldoffset;
        $oldoffset = $t['offset'];
        $end = get_gmt_timestamp($t['ts']);
        $num = num_open_day($start, $end, $inhour, $end < $timestampEnd ? 1 : $lastday, $halfday, $country_code);
        $start = get_gmt_timestamp(dol_time_plus_duree($end, 1, 'd'));
        if ($offset_change < 0 && $end < $timestampEnd) {
            // As the offset is < 0, num_public_holiday will not count the last day
            // see https://github.com/Dolibarr/dolibarr/issues/25789
            // if it is not an open day, we need to remove it
            $open = num_open_day($end, $end, 0, 1);
            if ($open === 0 ) {
                $num--;
            }
        }
        $carry += $num;
    }
    if ($end < $timestampEnd) {
        $num = num_open_day($start, $timestampEnd, $inhour, $lastday, $halfday, $country_code);
        $carry += $num;
    }
    return $carry;
}

Dolibarr / dolibarr