Improve holidays config form and naming

[bors-ltd]

The holidays library is improving over time, let's make use of their data for a more user-friendly experience.

Proposed change

I'm changing how the list of provinces is presented to the end user, with user-friendly aliases when available instead of the ISO 3166-2 codes. And then I'm changing how the service is named once created, to use that alias instead of the code. For example, "Germany, Baden-Württemberg" instead of "Germany, BW" (and Baden-Württemberg was listed in the configuration form, along with the other Landers).

Rationale: I cannot speak for the German culture in my example above, but as a (mainland) Frenchman, I didn't understand what Home Assistant was asking me with a list of codes like MQ or RE. So I cheked the holidays library documentation, which wasn't describing them either.

So I contributed to holidays (done), and now I'm using those changes in the holiday integration for a better user experience.

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New integration (thank you!)
• New feature (which adds functionality to an existing integration)
• Deprecation (breaking change to happen in the future)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Additional information

I'm flagging this as a new feature, because it changes the behaviour of the holiday integration, but it's not really new.

• This PR fixes or closes issue: fixes Improvement to holidays config form #133589
• This PR is related to issue:
• Link to documentation pull request:

Checklist

• The code change is tested and works locally.
• Local tests pass. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist
• I have followed the perfect PR recommendations
• The code has been formatted using Ruff (ruff format homeassistant tests)
• Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

• I have reviewed two other open pull requests in this repository.

[home-assistant]

Hey there @jrieger, @gjohansson-ST, mind taking a look at this pull request as it has been labeled with an integration (holiday) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of holiday can trigger bot actions by commenting:

• @home-assistant close Closes the pull request.
• @home-assistant rename Awesome new title Renames the pull request.
• @home-assistant reopen Reopen the pull request.
• @home-assistant unassign holiday Removes the current integration label and assignees on the pull request, add the integration domain after the command.
• @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
• @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

[gjohansson-ST]

It's not breaking as it only influence new setups.
Move that part of the description (friendly name instead of code when exists) to the proposed change section.

[home-assistant]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

[bors-ltd]

I realised I'm not using the holidays library as intended, and I'm just trying to bypass its features, and make things worse in the end.

I should have seen it from my own country:

 'GES': ['Alsace', 'Champagne-Ardenne', 'Lorraine'],

the same code being used for three different regions.

But to take another country as an example:

{'1': ['Burgenland', 'Bgld', 'B'],

Austria and Malaysia are indifferently using numerical codes, plain name, abbreviations, and even a single letter when possible.

New Zealand is pushing it further with using an English name, along with the Maori one.

The thing is, holidays already supports this:

>>> nz_english = country_holidays('NZ', 'Auckland')
>>> nz_english.subdiv
'Auckland'
>>> nz_maori = country_holidays('NZ', 'Tāmaki-Makaurau')
>>> nz_maori.subdiv
'Tāmaki-Makaurau'

The library accepts and preserves the user choice, and that's that choice I need in the end to name the entity.

I'm in favour of user choice, so I'll rework my PR to store the alias in the province configuration. So that in the end, if the user chose "New Zealand" and "Tāmaki-Makaurau" in the config form, the entity will be named "New Zealand, Tāmaki-Makaurau" as requested.

And of course, I already started taking all of your suggestions into account.

[gjohansson-ST]

We should not store the alias, only the code. And in the case above for New Zealand the resulting entry should be New Zealand, Auckland, Tāmaki-Makaurau (or something similar).

The reason is because these are aliases and prone to be changed etc. So that would result in a config entry to break on startup as example in a future bump of the lib.

Also agree some of the aliases are quite weird now looking at it, but I guess that's just how it is and we'll have to live with it. Alternatively we could just use the first alias in the list, but then in the example above with NZ someone might not be soo happy if they really want to have the Maori one.

[bors-ltd]

Alright, I won't change my PR as much, and users can always manually rename the entity after creation.

Ideally, holidays would store both, maybe I can contribute something later.

[gjohansson-ST]

We can keep a local variable for the user choice in the config flow and use that for setting the title (the entity is based on the config entry title as well). Then we store the code, but for the user it's set according to the choice they made.

It would require a few more lines of code and the selector needs to use the labels (and lookup the code later).

I think that's probably the route we should take.

[bors-ltd]

Updated my PR with all of your suggestions, but not pushing the "Ready for review" button yet. I'll now look into storing the user choice for the time of the configuration, but keep the ISO code for long-term storage, which is kind of the best of both worlds.

[bors-ltd]

Alright, I think I got it. There's just a compromise, I still have to keep the ISO codes in the valid choices, or an existing config entry wouldn't be validated. So I pushed them to the end of the list, listing the user-friendly names first.

I changed my new test to cover another use case: several provinces using the same subdivision code, but let the user pick the one they want.

[bors-ltd]

I'm thinking about splitting province and province_str into two schema entries, let the user choose province_str, but make province hidden, filled by the code. We keep a trace of the user choice, but only province is validating the configuration. I yet have to check if and how the API makes it possible.

[gjohansson-ST]

Why do we need to store the user choice?

[bors-ltd]

I imagine it's best to preselect the current choice when reconfiguring the entity. But I don't know the Home Assistant guidelines. My goal is also to leave a clean and maintainable code, and I'm not happy with having to list all of the ISO codes only to make the latest version of my PR pass the tests (https://github.com/home-assistant/core/pull/133663/files#diff-3b5137df71d390af3dd7d897ac751d57aa24720e019374c706c0bd5ad4ebbc69R54). So I was thinking about a way to hide those codes from the user, while still persisting them in the final configuration.

[bors-ltd]

Please forget about my blather above, the misreading of a test failure led me to think HA was validating the entity's current data against the setup schema. Which was not possible when we store the ISO code, but only expose the aliases.

In the end, it was just a matter of updating the test to the new behaviour (if you're curious).

I'm now happy with the final result. While I think I addressed all of your requests, the PR is still shown as "changes requested", but I can't see what to resolve.

And thank you for your patience.

[gjohansson-ST]

In #133608 I am just combining all the aliases. While of course it looks nicer with only grabbing the selected alias from the user, it also complicates the whole thing and we need to store both the code (it would break existing integrations otherwise) and the selected alias (I am a bit concerned about the future of these).

Besides that CI is failing so needs to be fixed 👍

[bors-ltd]

So I think you will agree with me the (hopefully) final form of this PR is minimal and legible.

I don't think it was necessary to add more tests, as I don't change the behaviour in the form nor in naming the entity. Just presenting user-friendly names to the user but still use the codes internally.

I also kept the naming scheme of the entity, e.g. "Germany, BW". Yes it would be cool to have the full name, but as we saw, it's too much taxing on the code and maintaining it. Plus it has the advantage of being neutral when the province has several aliases. The user can always rename the entity as they wish afterwards.

[gjohansson-ST]

Thanks @bors-ltd 👍