Skip to content

Commit

Permalink
Improve holidays config form and naming
Browse files Browse the repository at this point in the history 
The holidays library is improving over time, let's make use of their
data for a more user-friendly experience.
  • Loading branch information
@bors-ltd
bors-ltd committed Jan 6, 2025
1 parent acd9597 commit 2b65865
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 20 deletions.
47 changes: 44 additions & 3 deletions homeassistant/components/holiday/config_flow.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from homeassistant.helpers.selector import (
CountrySelector,
CountrySelectorConfig,
SelectOptionDict,
SelectSelector,
SelectSelectorConfig,
SelectSelectorMode,
Expand All @@ -30,6 +31,32 @@
SUPPORTED_COUNTRIES = list_supported_countries(include_aliases=False)


def get_optional_provinces(country: str) -> list[Any]:
"""Return the country provinces (territories).
Some territories can have extra or different holidays
from another within the same country.
Some territories can have different names (aliases).
"""
province_options: list[Any] = []

if provinces := SUPPORTED_COUNTRIES[country]:
country_data = country_holidays(country, years=dt_util.utcnow().year)
if country_data.subdivisions_aliases and (
subdiv_aliases := country_data.get_subdivision_aliases()
):
# Remember the user choice among all of the aliases available
province_options = [
SelectOptionDict(value=alias, label=alias)
for aliases in subdiv_aliases.values()
for alias in aliases
]
else:
province_options = provinces

return province_options


def get_optional_categories(country: str) -> list[str]:
"""Return the country categories.
Expand All @@ -45,7 +72,7 @@ def get_optional_categories(country: str) -> list[str]:
def get_options_schema(country: str) -> vol.Schema:
"""Return the options schema."""
schema = {}
if provinces := SUPPORTED_COUNTRIES[country]:
if provinces := get_optional_provinces(country):
schema[vol.Optional(CONF_PROVINCE)] = SelectSelector(
SelectSelectorConfig(
options=provinces,
Expand All @@ -64,6 +91,12 @@ def get_options_schema(country: str) -> vol.Schema:
return vol.Schema(schema)


def get_province_code(country: str, province: str) -> str:
"""Return the ISO 3166 code for a given province alias, if any."""
country_data = country_holidays(country, province, years=dt_util.utcnow().year)
return country_data.subdivisions_aliases.get(province, province)


class HolidayConfigFlow(ConfigFlow, domain=DOMAIN):
"""Handle a config flow for Holiday."""

Expand Down Expand Up @@ -128,7 +161,11 @@ async def async_step_options(
data = {CONF_COUNTRY: country}
options: dict[str, Any] | None = None
if province := user_input.get(CONF_PROVINCE):
data[CONF_PROVINCE] = province
# Don't store the user alias but the stable ISO 3166 code
province_code = await self.hass.async_add_executor_job(
get_province_code, country, province
)
data[CONF_PROVINCE] = province_code
if categories := user_input.get(CONF_CATEGORIES):
options = {CONF_CATEGORIES: categories}

Expand Down Expand Up @@ -165,7 +202,11 @@ async def async_step_reconfigure(
data = {CONF_COUNTRY: country}
options: dict[str, Any] | None = None
if province := user_input.get(CONF_PROVINCE):
data[CONF_PROVINCE] = province
# Don't store the user alias but the stable ISO 3166 code
province_code = await self.hass.async_add_executor_job(
get_province_code, country, province
)
data[CONF_PROVINCE] = province_code
if categories := user_input.get(CONF_CATEGORIES):
options = {CONF_CATEGORIES: categories}

Expand Down
69 changes: 52 additions & 17 deletions tests/components/holiday/test_config_flow.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@ async def test_form(hass: HomeAssistant, mock_setup_entry: AsyncMock) -> None:
result3 = await hass.config_entries.flow.async_configure(
result2["flow_id"],
{
CONF_PROVINCE: "BW",
CONF_PROVINCE: "Baden-Württemberg",
},
)
await hass.async_block_till_done()

assert result3["type"] is FlowResultType.CREATE_ENTRY
assert result3["title"] == "Germany, BW"
assert result3["title"] == "Germany, Baden-Württemberg"
assert result3["data"] == {
"country": "DE",
"province": "BW",
Expand Down Expand Up @@ -131,7 +131,7 @@ async def test_single_combination_country_province(hass: HomeAssistant) -> None:
result_de_step2 = await hass.config_entries.flow.async_configure(
result_de_step1["flow_id"],
{
CONF_PROVINCE: data_de[CONF_PROVINCE],
CONF_PROVINCE: "Baden-Württemberg",
},
)
assert result_de_step2["type"] is FlowResultType.ABORT
Expand Down Expand Up @@ -172,13 +172,13 @@ async def test_form_babel_unresolved_language(hass: HomeAssistant) -> None:
result = await hass.config_entries.flow.async_configure(
result["flow_id"],
{
CONF_PROVINCE: "BW",
CONF_PROVINCE: "Baden-Württemberg",
},
)
await hass.async_block_till_done()

assert result["type"] is FlowResultType.CREATE_ENTRY
assert result["title"] == "Germany, BW"
assert result["title"] == "Germany, Baden-Württemberg"
assert result["data"] == {
"country": "DE",
"province": "BW",
Expand Down Expand Up @@ -219,13 +219,13 @@ async def test_form_babel_replace_dash_with_underscore(hass: HomeAssistant) -> N
result = await hass.config_entries.flow.async_configure(
result["flow_id"],
{
CONF_PROVINCE: "BW",
CONF_PROVINCE: "Baden-Württemberg",
},
)
await hass.async_block_till_done()

assert result["type"] is FlowResultType.CREATE_ENTRY
assert result["title"] == "Germany, BW"
assert result["title"] == "Germany, Baden-Württemberg"
assert result["data"] == {
"country": "DE",
"province": "BW",
Expand All @@ -237,7 +237,7 @@ async def test_reconfigure(hass: HomeAssistant) -> None:
"""Test reconfigure flow."""
entry = MockConfigEntry(
domain=DOMAIN,
title="Germany, BW",
title="Germany, Baden-Württemberg",
data={"country": "DE", "province": "BW"},
)
entry.add_to_hass(hass)
Expand All @@ -248,15 +248,15 @@ async def test_reconfigure(hass: HomeAssistant) -> None:
result = await hass.config_entries.flow.async_configure(
result["flow_id"],
{
CONF_PROVINCE: "NW",
CONF_PROVINCE: "Nordrhein-Westfalen",
},
)
await hass.async_block_till_done()

assert result["type"] is FlowResultType.ABORT
assert result["reason"] == "reconfigure_successful"
entry = hass.config_entries.async_get_entry(entry.entry_id)
assert entry.title == "Germany, NW"
assert entry.title == "Germany, Nordrhein-Westfalen"
assert entry.data == {"country": "DE", "province": "NW"}


Expand Down Expand Up @@ -297,7 +297,7 @@ async def test_reconfigure_incorrect_language(hass: HomeAssistant) -> None:

entry = MockConfigEntry(
domain=DOMAIN,
title="Germany, BW",
title="Germany, Baden-Württemberg",
data={"country": "DE", "province": "BW"},
)
entry.add_to_hass(hass)
Expand All @@ -308,15 +308,15 @@ async def test_reconfigure_incorrect_language(hass: HomeAssistant) -> None:
result = await hass.config_entries.flow.async_configure(
result["flow_id"],
{
CONF_PROVINCE: "NW",
CONF_PROVINCE: "Nordrhein-Westfalen",
},
)
await hass.async_block_till_done()

assert result["type"] is FlowResultType.ABORT
assert result["reason"] == "reconfigure_successful"
entry = hass.config_entries.async_get_entry(entry.entry_id)
assert entry.title == "Germany, NW"
assert entry.title == "Germany, Nordrhein-Westfalen"
assert entry.data == {"country": "DE", "province": "NW"}


Expand All @@ -325,13 +325,13 @@ async def test_reconfigure_entry_exists(hass: HomeAssistant) -> None:
"""Test reconfigure flow stops if other entry already exist."""
entry = MockConfigEntry(
domain=DOMAIN,
title="Germany, BW",
title="Germany, Baden-Württemberg",
data={"country": "DE", "province": "BW"},
)
entry.add_to_hass(hass)
entry2 = MockConfigEntry(
domain=DOMAIN,
title="Germany, NW",
title="Germany, Nordrhein-Westfalen",
data={"country": "DE", "province": "NW"},
)
entry2.add_to_hass(hass)
Expand All @@ -342,15 +342,15 @@ async def test_reconfigure_entry_exists(hass: HomeAssistant) -> None:
result = await hass.config_entries.flow.async_configure(
result["flow_id"],
{
CONF_PROVINCE: "NW",
CONF_PROVINCE: "Nordrhein-Westfalen",
},
)
await hass.async_block_till_done()

assert result["type"] is FlowResultType.ABORT
assert result["reason"] == "already_configured"
entry = hass.config_entries.async_get_entry(entry.entry_id)
assert entry.title == "Germany, BW"
assert entry.title == "Germany, Baden-Württemberg"
assert entry.data == {"country": "DE", "province": "BW"}


Expand Down Expand Up @@ -425,6 +425,41 @@ async def test_form_with_options(
assert state.state == STATE_OFF


async def test_form_with_subdivision_aliases(hass: HomeAssistant) -> None:
"""Test the flow with several aliases using the same subdivision code."""
await hass.config.async_set_time_zone("Europe/Paris")

result = await hass.config_entries.flow.async_init(
DOMAIN, context={"source": config_entries.SOURCE_USER}
)
assert result["type"] is FlowResultType.FORM

result = await hass.config_entries.flow.async_configure(
result["flow_id"],
{
CONF_COUNTRY: "FR",
},
)
await hass.async_block_till_done()

assert result["type"] is FlowResultType.FORM

result = await hass.config_entries.flow.async_configure(
result["flow_id"],
{
CONF_PROVINCE: "Alsace",
},
)
await hass.async_block_till_done(wait_background_tasks=True)

assert result["type"] is FlowResultType.CREATE_ENTRY
assert result["title"] == "France, Alsace"
assert result["data"] == {
CONF_COUNTRY: "FR",
CONF_PROVINCE: "GES",

Check warning on line 459 in tests/components/holiday/test_config_flow.py

View workflow job for this annotation

GitHub Actions / Check other linters 

GES ==> GOES, GUESS
}


@pytest.mark.usefixtures("mock_setup_entry")
async def test_options_abort_no_categories(hass: HomeAssistant) -> None:
"""Test the options flow abort if no categories to select."""
Expand Down

0 comments on commit 2b65865

Please sign in to comment.