Incorrect Christmas day for United Kingdom

[joshuabaker]

Christmas day is always on 25 December in the United Kingdom. A substitute bank holiday is created for both Christmas and Boxing Day when either of those days fall on a weekend.

Date	Day	Holiday
26 December 2016	Monday	Boxing Day
27 December 2016	Tuesday	Christmas Day (substitute day)

If a bank holiday is on a weekend, a ‘substitute’ weekday becomes a bank holiday, normally the following Monday.

Cited from the GOV.UK bank holidays page.

The logic appears to be slightly wrong in the UnitedKingdom provider. There actually needs to be (up to) 4 days representing Christmas and Boxing Day, in the event Christmas falls on a Sunday.

[joshuabaker]

Potential Solution

I’m not sure exactly how holiday labelling works (i.e. secondChristmasDay). Is there any issue calling it boxingDay and introducing substituteChristmasDay and substituteBoxingDay?

public function calculateChristmasHolidays()
{
    $christmasDay = new DateTime("$this->year-12-25", new DateTimeZone($this->timezone));
    $boxingDay    = new DateTime("$this->year-12-26", new DateTimeZone($this->timezone));

    $substituteChristmasDay = clone $christmasDay;
    $substituteBoxingDay = clone $boxingDay;

    switch ($christmasDay->format('w')) {
        case 0:
            $substituteChristmasDay->add(new DateInterval('P2D'));
            break;
        case 5:
            $substituteBoxingDay->add(new DateInterval('P2D'));
            break;
        case 6:
            $substituteChristmasDay->add(new DateInterval('P2D'));
            $substituteBoxingDay->add(new DateInterval('P2D'));
            break;
    }

    $this->addHoliday(new Holiday(
        'christmasDay',
        [],
        $christmasDay,
        $this->locale
    ));

    $this->addHoliday(new Holiday(
        'boxingDay',
        [],
        $boxingDay,
        $this->locale,
        $boxingDay->format('N') < 6 ? Holiday::TYPE_BANK : Holiday::TYPE_NATIONAL
    ));

    if ($christmasDay != $substituteChristmasDay) {
        $this->addHoliday(new Holiday(
            'substituteChristmasDay',
            [],
            $substituteChristmasDay,
            $this->locale,
            Holiday::TYPE_BANK
        ));
    }

    if ($boxingDay != $substituteBoxingDay) {
        $this->addHoliday(new Holiday(
            'substituteBoxingDay',
            [],
            $substituteBoxingDay,
            $this->locale,
            Holiday::TYPE_BANK
        ));
    }
}

[joshuabaker]

This might apply to logic used in other methods.

[stelgenhof]

Hi @joshuabaker Thanks for catching this one! Indeed this seems to be incorrect. The idea is that if any given holiday is being substituted/observed on a different date, the provider class should add another holiday date. The UK provider class doesn't indeed do that.

As for your question regarding the labels: there is no specific rule for the name, these are just internal ID's. Just to keep them consistent across countries these are named the same. The translations can be used to give the holiday a region specific name (i.e. "Boxing Day" vs "Second Christmas Day".

From the looks of it your solution looks good. Would you mind creating a PR including a modified unit test?

Thanks again for your feedback!

[stelgenhof]

@joshuabaker I have been kindly taken your solution (with some adaptions) into the main branch. Issue should be resolved now. I will today release 1.6.1 that includes this one as well :)