kylekatarnls / business-time

Carbon mixin to handle business days and opening hours
MIT License
297 stars 14 forks source link

Additional holidays not added when specifing incorrect region #33

Closed liepumartins closed 4 years ago

liepumartins commented 4 years ago

I bumped into strange behaviour and began to believe that holidays.with key was being ignored. Turns out, if region contains existing first part ('us-'), but second part is incorrect ("US"), with holidays are lost. However is one specifies completely non-existing region ("xx-xx"), custom holidays are added.

<?php

declare(strict_types = 1);

require_once 'vendor/autoload.php';

use Carbon\Carbon;
use Cmixin\BusinessTime;

$regions = ['us-national', 'us-US', 'xx-xx']; // existing, "half-existing", non-existing

foreach ($regions as $region) {
    $config = [
        'holidays' => [
            'region' => $region,
            'with' => [
                'custom' => '2020-09-16',
            ],
        ],
    ];

    BusinessTime::enable(Carbon::class, $config);

    echo $region . ' actually resolved as ' . Carbon::getHolidaysRegion() . PHP_EOL;
    print_r(Carbon::getHolidays());

    var_dump(Carbon::parse('2020-9-16')->isHoliday()); // should be true, but is not
}
kylekatarnls commented 4 years ago

Indeed, it sounds like the region fallback is actually proceeded after the with inclusion. It would need to change the steps order to store the custom holidays after the region is changed from us-US to us-national. This sounds feasible.

kylekatarnls commented 4 years ago

It has been fixed in the kylekatarnls/business-day dependency, you can upgrade to get it working with us-US region or similar.