Added state support and added states for Germany and Austria

[marlonbasten]

I started working on adding support for states in countries. This will hopefully fix the issues Germany, Austria and India are currently having. ( Issues: #8 #12 )

I would really appreciate any constructive criticism and feedback!

States added:

• Germany (All states)
• Austria (All states)

Adds the following functionality:

Getting only nationwide holidays:

Holidays::for(Germany::make())->get();
Holidays::for(Austria::make())->get();

Getting nationwide and state holidays:

Holidays::for(Germany::make())
  ->inState(NorthRhineWestphalia::make())
  ->get();
Holidays::for(Austria::make())
  ->inState(Vienna::make())
  ->get();

Holidays::for(Germany::make())
  ->inState('nw')
  ->get();
Holidays::for(Austria::make())
  ->inState('wi')
  ->get();

or

Holidays::for(Germany::make(), state: NorthRhineWestphalia::make())
  ->get();
Holidays::for(Austria::make(), state: Vienna::make())
  ->get();

Holidays::for(Germany::make(), state: 'nw')
  ->get();
Holidays::for(Austria::make(), state: 'wi')
  ->get();

Get only state holidays:

State::findOrFail(Germany::make(), 'nw')->get();
State::findOrFail(Austria::make(), 'wi')->get();

[Nielsvanpach]

Hi @marlonbasten, thanks for your effort on this! As you might have noticed, this package is blowing up with PRs, so it takes a while to get through them.

I'm sure your approach works fine for Germany, but we decided to use a simpler solution for states and regions which is extendable for more use cases. I've added some docs on this: https://github.com/spatie/holidays?tab=readme-ov-file#contributing-a-new-country

Let me know what you think.

[marlonbasten]

Thanks for the effort @Nielsvanpach! 😊 I think this solution is also fine for adding regions.

It can get messy though as some countries have a lot of regions.

There should be some guard-rails implemented so we don't get classes that have a ton of methods in them. In the case of Germany i would have to add 16 methods (not including variable holidays). Well, I guess you could do that with a single method and match. But that match-statement can get large and produce calculations for variable holidays we don't need.

Also, many projects I've seen save the state/region in a separate column. So you would have to concatenate the string before you can get the state-specific holidays.

$region = $user->country . '-' . $user->region;
$holidays = Holidays::for(Austria::make(region: $region))->get();

Also, I don't know if this is a use-case for some people, but you cannot get holidays for a specific state like that.

Anyways, thanks so much for taking the time and I think both solutions have their pro's and con's! :)

[marlonbasten]

@Nielsvanpach I saw that someone approved the workflows for this PR 😄. I saw that PHPStan was failing so I fixed that. Even if this doesn't get merged, maybe something can be salvaged from this PR.

[eusonlito]

How many levels should be added?

In Spain we have national (country), by autonomous community (region), province (state) and city.

Is this a common case in other countries?

[marlonbasten]

How many levels should be added?

In Spain we have national (country), by autonomous community (region), province (state) and city.

Is this a common case in other countries?

@eusonlito We have something similar in Germany. But this is only an exception for a few cities. In my solution this could be solved like that:

(Disclaimer: My solution uses the name "state". So I'll call everything a State)

• Create a state class "GaliciaRegion" and add the holidays that apply to that region
• Create a state class "LugoProvince" and make it extend "GaliciaRegion" and merge the holidays of Galicia and Lugo in it
• Create a state class "LugoCity" and make it extend "LugoProvince", do the same as above.

Now you can get all holidays.

Get all national holidays:

Holidays::for(Spain::make())
  ->get();

Get holidays from Spain and the Galicia region:

Holidays::for(Spain::make(), state: GaliciaRegion::make())
  ->get();

Get holidays from Spain and the Lugo province in the Galicia region:

Holidays::for(Spain::make(), state: LugoProvince::make())
  ->get();

Get holidays from Spain and Lugo city in the Lugo province in the Galicia region:

Holidays::for(Spain::make(), state: LugoCity::make())
  ->get();

Could this solution work for you?

@Nielsvanpach This could also be an issue of the currently proposed solution. With my solution you can extend regions etc. easily. With the string solution like "es-ga", it is not so straight forward to extend regions.

If my PR has any chance I would be happy to change "state" to "region" as it is more general indeed 😄

[marlonbasten]

@Nielsvanpach Thanks for running the tests again! I genuinely do now know why this one test is failing. When looking at the logs everything seems to be alright, but the test is flagged as failed. At lest PHPStan is working now :D

[eusonlito]

@marlonbasten perhaps the solution is for everything to be a region?

And each region can extend to a parent region. This way you can create as many levels as you need depending on the location:

City > State > Region > Country

Holidays::for(LugoCity::make())
  ->get();
Holidays::for(LugoState::make())
  ->get();
Holidays::for(GaliciaRegion::make())
  ->get();
Holidays::for(SpainCountry::make())
  ->get();

In any case, you should be able to get only the holidays of a city, without including those of its province, region or country, since in many cases vacations are shown separately in the calendars (local, provincial, region, national).

[marlonbasten]

@eusonlito That would be an option too. But what about if you want to get the holidays for a city including the nationwide holidays? In Germany for example we have nationwide (country) holidays and state-specific holidays.

Those don't get separated and are always shown in one. So maybe I change the following in my PR:

• Change the name "state" to "region"
• Create logic to extend a Region but without merging the holidays of the extended region.

Then you could do the same as before but can now also do this for the regions separately without getting the "parents" holidays.

State::findOrFail(Spain::make(), LugoCity::make())->get();

@Nielsvanpach What is your opinion about that?

[eusonlito]

@marlonbasten yes, by default it is all holidays together, but perhaps an additional method should be added to make that returns only holidays specific to that region.

And... how about changing the directory structure for the holidays?

src/Locations/Spain/Spain.php // Country
src/Locations/Spain/Andalucia/Andalucia.php // Region
src/Locations/Spain/Andalucia/Sevilla/Sevilla.php // State
src/Locations/Spain/Andalucia/Sevilla/Sevilla/Sevilla.php // City
src/Locations/Spain/Andalucia/Sevilla/Sevilla/DosHermanas.php // City
src/Locations/Spain/Galicia/Galicia.php // Region
src/Locations/Spain/Galicia/Galicia/Lugo/Lugo.php // State
src/Locations/Spain/Galicia/Galicia/Lugo/Lugo/Lugo.php // City
src/Locations/Spain/Galicia/Galicia/Lugo/Lugo/Foz.php // City

😀

[marlonbasten]

@eusonlito I could do that. But I don't want to put any more time into this PR because I already got told that my solution will not be merged 😅

If any of the core members of Spatie agree to a solution proposed by @eusonlito or me I would be happy to take my time to make those changes! :)

[MeiKatz]

Just an idea: you always can add an extra method to your country class that returns another instance of another country class, but in this case it's for a region/federal state.

Holidays::for(Germany::make()->region("hh"))->get();

[marlonbasten]

@MeiKatz Smart and easy solution indeed! That would work well for Germany. I have an initial thought about that tho:

How would you get only the holidays that apply to that state? As far as I understand, most people in Spain want to show state/federal/etc.
holidays separately. Would you merge that in the state method somehow? Maybe implement a HasStates/HasRegions Interface?

Maybe I'll try to implement that locally and if I like it I'll change that in my PR.

On another note: Are you satisfied with the current official proposal about states/regions?

[marlonbasten]

Closing this PR now due to lack of communication.