Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft: Add Support for Switzerland #49

Merged
merged 8 commits into from
Jan 27, 2024

Conversation

Martin-Welte
Copy link
Contributor

TL;DR

  • Adds Support for Switzerland

Additional Information

until discussion about region support is wrapped up, this PR includes the holidays for switzerland that the public transport system, the post and nation-wide organisations follow.

Source

Copy link
Member

@Nielsvanpach Nielsvanpach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Could you rebase, that way I can run the tests.

I've also added notes on region based holidays:
https://github.com/spatie/holidays?tab=readme-ov-file#contributing-a-new-country

src/Countries/Switzerland.php Outdated Show resolved Hide resolved
src/Countries/Switzerland.php Outdated Show resolved Hide resolved
@Martin-Welte Martin-Welte force-pushed the feature/switzerland branch 2 times, most recently from ce95b9e to 4c006cb Compare January 21, 2024 15:47
@Martin-Welte Martin-Welte force-pushed the feature/switzerland branch 2 times, most recently from 5db0936 to 685ab66 Compare January 22, 2024 14:10
{
return array_merge([
'Neujahr' => '01-01',
'Berchtoldstag' => '01-02',
Copy link

@herpaderpaldent herpaderpaldent Jan 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a public holiday in every canton. In the resource (wikipedia) you can see that this day is not official everyware.
F.e. Kt. UR does not celebrate this day.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image @herpaderpaldent this PR covers only the non-regional holidays until now. You can read the description of the PR which days are included and why.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Martin-Welte i am sorry i can not see a description to this PR that would have explained this. However the simplification is understandable until the regional support is settled.

May i suggest changing the source towards the original source and not wikipedia?

@Martin-Welte Martin-Welte changed the title Add Support for Switzerland Draft: Add Support for Switzerland Jan 22, 2024
@herpaderpaldent
Copy link

Hi @Martin-Welte

I am sorry to bother you, i already started forking your work and play around with the cantons based on the BFJ List. However something i noticed we haven't discussed: Language.

How is multilanguage support handled with this package? Right now the german variants of the holidays is present but we have 4 official languages in switzerland.
Any idea regarding this? Maybe @Nielsvanpach ?

@Martin-Welte
Copy link
Contributor Author

@herpaderpaldent there is a discussion here about multi lang support: #131

@Martin-Welte
Copy link
Contributor Author

@herpaderpaldent I would love to see your shot at cantons. I am still pondering what the best solution would be here.

@herpaderpaldent
Copy link

herpaderpaldent commented Jan 22, 2024

@herpaderpaldent I would love to see your shot at cantons. I am still pondering what the best solution would be here.

This is what i have been tinkering with tonight. Let me know what you think: https://github.com/herpaderpaldent/holidays/blob/feature/switzerland-with-regions/src/Countries/Switzerland.php

As a proof of concept i have chosen the canton of valais, since it has multilanguage requirements, no 'Karfreitag' and an additional eastern+60 day.

PS: i have not added tests yet, please have mercy ;-)

@Martin-Welte
Copy link
Contributor Author

@herpaderpaldent Thank you for your PR.
Each canton has their own set of holidays, so it's unfortunateley not dependant on a language. I am also working on an approach that could then be translated easily in the future.
For now i was thinking something along these lines:

$regions = [
  'ZH' => [
    'Neujahr' => '01-01',
    'Karfreitag' => $easter->subDays(2),
    'Ostermontag' => $easter->addDay(),
    'Tag der Arbeit' => '05-01',
    'Auffahrt' => $easter->addDays(39),
    'Pfingstmontag' => $easter->addDays(50),
    'Bundesfeier' => '08-01',
    'Weihnachtstag' => '12-25',
    'Stephanstag' => '12-26',
  ],
];

return $regions[$this->region];

that way we could define each canton seperateley, and have it nice and tidy. Instead of using the german words i would opt for translation keys which we can then translate via a seperate locale in the future.

@herpaderpaldent
Copy link

Hey

I cannot really follow your comment my attempt allready reflects the fact that every canton has its own set of holidays:

f.e.:

private function getCantonHolidays(string $canton, int $year): array
    {
        return match ($canton) {
            'vs' => $this->getVSHolidays($year),
            default => [],
        };
    }
    
    private function getVSHolidays(int $year)
    {
        $eastern_holidays = $this->getEasterHolidays($year, with_easter_minus_two: false, add_corpus_christi: true);

        return array_merge([
            $this->getLocaleHoliday('01-01') => '01-01',
            $this->getLocaleHoliday('01-02') => '01-02',
            $this->getLocaleHoliday('03-19') => '03-19', // St. Joseph
            $this->getLocaleHoliday('08-15') => '08-15', // Assumption
            $this->getLocaleHoliday('11-01') => '11-01', // All Saints
            $this->getLocaleHoliday('12-08') => '12-08', // Immaculate Conception
            $this->getLocaleHoliday('25-12') => '12-25',
            $this->getLocaleHoliday('26-12') => '12-26',
        ], $eastern_holidays);
    }

Although having slept a night i would aim for a better translation approach then the current. F.e. having the Translations in a single constant and use english as key for the translation. Fallback to the first language as default.

const HOLIDAYS = [
        'languages' => ['de', 'fr', 'it', 'rom'],
        'New Year' => ['Neujahrstag', 'Nouvel an', 'Capodanno', 'Nadal'],
.....
];

@Martin-Welte
Copy link
Contributor Author

@herpaderpaldent I had time today to update it today, now we can discuss it.
What I wanted to ensure was having the variable holidays clearly defined, so that it's rather obvious which holidays will be used. It did take a bit of typing to ensure it though.

@Martin-Welte
Copy link
Contributor Author

@Nielsvanpach btw, there is a rudimentary translation function in there, if you want to take a look. Right now it only translates into the 3 official languages of switzerland.

@herpaderpaldent
Copy link

I don't like your solution. Way to much boilerplate.

Have you even checked out my approach? You have not even given feedback to it

@Nielsvanpach
Copy link
Member

I've added support for multi languages: #177

@Martin-Welte
Copy link
Contributor Author

@Nielsvanpach thank you for the info. I will update the PR End of Week

@herpaderpaldent not yet, i had limited time until now, but i will take a look. I wanted to make sure you can see how i pictured my version. I am sorry, but unfortunately I have quite the bussy week. If you have time in the meantime you could comment, what you would do differently and why. Which parts of your implementation would you say, would be most crucial to take into consideration?

@Kenny1291
Copy link
Contributor

Hello guys, just dropping some info here about public holidays in Switzerland.

Swiss National Day (August 1) is the only federal public holiday. Cantons have their own additional public holidays.
The only holidays that every Canton has in common are:

  • New Year's Day
  • Ascension Day
  • Christmas Day

Labour Law

Copy link
Member

@Nielsvanpach Nielsvanpach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Nielsvanpach Nielsvanpach merged commit 4c5ff96 into spatie:main Jan 27, 2024
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants