From 54c4fb5f569279f36fcec611287e6a7a9a30727c Mon Sep 17 00:00:00 2001 From: Richard Kroegel <42204099+rikroe@users.noreply.github.com> Date: Tue, 8 Oct 2024 07:42:10 +0200 Subject: [PATCH] BMW: Add reconfiguration flow (#127726) * BMW: Add reconfiguration flow * Implement requested changes -------- Co-authored-by: epenet <6771947+epenet@users.noreply.github.com> * Abort if unique_id changes, small adjustments --------- Co-authored-by: epenet <6771947+epenet@users.noreply.github.com> --- .../bmw_connected_drive/config_flow.py | 41 +++++-- .../bmw_connected_drive/strings.json | 4 +- .../bmw_connected_drive/__init__.py | 2 +- .../bmw_connected_drive/test_config_flow.py | 110 +++++++++++++++++- 4 files changed, 142 insertions(+), 15 deletions(-) diff --git a/homeassistant/components/bmw_connected_drive/config_flow.py b/homeassistant/components/bmw_connected_drive/config_flow.py index 8132d241ca4..3468ee25ca1 100644 --- a/homeassistant/components/bmw_connected_drive/config_flow.py +++ b/homeassistant/components/bmw_connected_drive/config_flow.py @@ -13,6 +13,7 @@ import voluptuous as vol from homeassistant.config_entries import ( SOURCE_REAUTH, + SOURCE_RECONFIGURE, ConfigEntry, ConfigFlow, ConfigFlowResult, @@ -20,6 +21,7 @@ from homeassistant.config_entries import ( ) from homeassistant.const import CONF_PASSWORD, CONF_REGION, CONF_SOURCE, CONF_USERNAME from homeassistant.core import HomeAssistant, callback +from homeassistant.data_entry_flow import AbortFlow from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.selector import SelectSelector, SelectSelectorConfig @@ -72,7 +74,8 @@ class BMWConfigFlow(ConfigFlow, domain=DOMAIN): VERSION = 1 - _reauth_entry: ConfigEntry + _existing_entry_data: Mapping[str, Any] | None = None + _existing_entry_unique_id: str | None = None async def async_step_user( self, user_input: dict[str, Any] | None = None @@ -83,9 +86,14 @@ class BMWConfigFlow(ConfigFlow, domain=DOMAIN): if user_input is not None: unique_id = f"{user_input[CONF_REGION]}-{user_input[CONF_USERNAME]}" - if self.source != SOURCE_REAUTH: + if self.source not in {SOURCE_REAUTH, SOURCE_RECONFIGURE}: await self.async_set_unique_id(unique_id) self._abort_if_unique_id_configured() + elif ( + self.source in {SOURCE_REAUTH, SOURCE_RECONFIGURE} + and unique_id != self._existing_entry_unique_id + ): + raise AbortFlow("account_mismatch") info = None try: @@ -102,23 +110,22 @@ class BMWConfigFlow(ConfigFlow, domain=DOMAIN): if info: if self.source == SOURCE_REAUTH: - self.hass.config_entries.async_update_entry( - self._reauth_entry, data=entry_data + return self.async_update_reload_and_abort( + self._get_reauth_entry(), data=entry_data ) - self.hass.async_create_task( - self.hass.config_entries.async_reload( - self._reauth_entry.entry_id - ) + if self.source == SOURCE_RECONFIGURE: + return self.async_update_reload_and_abort( + self._get_reconfigure_entry(), + data=entry_data, ) - return self.async_abort(reason="reauth_successful") - return self.async_create_entry( title=info["title"], data=entry_data, ) schema = self.add_suggested_values_to_schema( - DATA_SCHEMA, self._reauth_entry.data if self.source == SOURCE_REAUTH else {} + DATA_SCHEMA, + self._existing_entry_data, ) return self.async_show_form(step_id="user", data_schema=schema, errors=errors) @@ -127,7 +134,17 @@ class BMWConfigFlow(ConfigFlow, domain=DOMAIN): self, entry_data: Mapping[str, Any] ) -> ConfigFlowResult: """Handle configuration by re-auth.""" - self._reauth_entry = self._get_reauth_entry() + self._existing_entry_data = entry_data + self._existing_entry_unique_id = self._get_reauth_entry().unique_id + return await self.async_step_user() + + async def async_step_reconfigure( + self, user_input: dict[str, Any] | None = None + ) -> ConfigFlowResult: + """Handle a reconfiguration flow initialized by the user.""" + reconfigure_entry = self._get_reconfigure_entry() + self._existing_entry_data = reconfigure_entry.data + self._existing_entry_unique_id = reconfigure_entry.unique_id return await self.async_step_user() @staticmethod diff --git a/homeassistant/components/bmw_connected_drive/strings.json b/homeassistant/components/bmw_connected_drive/strings.json index c59900ef4f9..fed71f85e35 100644 --- a/homeassistant/components/bmw_connected_drive/strings.json +++ b/homeassistant/components/bmw_connected_drive/strings.json @@ -15,7 +15,9 @@ }, "abort": { "already_configured": "[%key:common::config_flow::abort::already_configured_account%]", - "reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]" + "reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]", + "reconfigure_successful": "[%key:common::config_flow::abort::reconfigure_successful%]", + "account_mismatch": "Username and region are not allowed to change" } }, "options": { diff --git a/tests/components/bmw_connected_drive/__init__.py b/tests/components/bmw_connected_drive/__init__.py index 655955ff9aa..4d280a1d0e5 100644 --- a/tests/components/bmw_connected_drive/__init__.py +++ b/tests/components/bmw_connected_drive/__init__.py @@ -40,7 +40,7 @@ FIXTURE_CONFIG_ENTRY = { }, "options": {CONF_READ_ONLY: False}, "source": config_entries.SOURCE_USER, - "unique_id": f"{FIXTURE_USER_INPUT[CONF_REGION]}-{FIXTURE_USER_INPUT[CONF_REGION]}", + "unique_id": f"{FIXTURE_USER_INPUT[CONF_REGION]}-{FIXTURE_USER_INPUT[CONF_USERNAME]}", } diff --git a/tests/components/bmw_connected_drive/test_config_flow.py b/tests/components/bmw_connected_drive/test_config_flow.py index f71730fcc17..9d4d15703f2 100644 --- a/tests/components/bmw_connected_drive/test_config_flow.py +++ b/tests/components/bmw_connected_drive/test_config_flow.py @@ -13,7 +13,7 @@ from homeassistant.components.bmw_connected_drive.const import ( CONF_READ_ONLY, CONF_REFRESH_TOKEN, ) -from homeassistant.const import CONF_PASSWORD, CONF_USERNAME +from homeassistant.const import CONF_PASSWORD, CONF_REGION, CONF_USERNAME from homeassistant.core import HomeAssistant from homeassistant.data_entry_flow import FlowResultType @@ -193,6 +193,14 @@ async def test_reauth(hass: HomeAssistant) -> None: assert result["step_id"] == "user" assert result["errors"] == {} + suggested_values = { + key: key.description.get("suggested_value") + for key in result["data_schema"].schema + } + assert suggested_values[CONF_USERNAME] == FIXTURE_USER_INPUT[CONF_USERNAME] + assert suggested_values[CONF_PASSWORD] == wrong_password + assert suggested_values[CONF_REGION] == FIXTURE_USER_INPUT[CONF_REGION] + result2 = await hass.config_entries.flow.async_configure( result["flow_id"], FIXTURE_USER_INPUT ) @@ -203,3 +211,103 @@ async def test_reauth(hass: HomeAssistant) -> None: assert config_entry.data == FIXTURE_COMPLETE_ENTRY assert len(mock_setup_entry.mock_calls) == 2 + + +async def test_reauth_unique_id_abort(hass: HomeAssistant) -> None: + """Test aborting the reauth form if unique_id changes.""" + with patch( + "bimmer_connected.api.authentication.MyBMWAuthentication.login", + side_effect=login_sideeffect, + autospec=True, + ): + wrong_password = "wrong" + + config_entry_with_wrong_password = deepcopy(FIXTURE_CONFIG_ENTRY) + config_entry_with_wrong_password["data"][CONF_PASSWORD] = wrong_password + + config_entry = MockConfigEntry(**config_entry_with_wrong_password) + config_entry.add_to_hass(hass) + + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + assert config_entry.data == config_entry_with_wrong_password["data"] + + result = await config_entry.start_reauth_flow(hass) + assert result["type"] is FlowResultType.FORM + assert result["step_id"] == "user" + assert result["errors"] == {} + + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], {**FIXTURE_USER_INPUT, CONF_REGION: "north_america"} + ) + await hass.async_block_till_done() + + assert result2["type"] is FlowResultType.ABORT + assert result2["reason"] == "account_mismatch" + assert config_entry.data == config_entry_with_wrong_password["data"] + + +async def test_reconfigure(hass: HomeAssistant) -> None: + """Test the reconfiguration form.""" + with patch( + "bimmer_connected.api.authentication.MyBMWAuthentication.login", + side_effect=login_sideeffect, + autospec=True, + ): + config_entry = MockConfigEntry(**FIXTURE_CONFIG_ENTRY) + config_entry.add_to_hass(hass) + + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + result = await config_entry.start_reconfigure_flow(hass) + assert result["type"] is FlowResultType.FORM + assert result["step_id"] == "user" + assert result["errors"] == {} + + suggested_values = { + key: key.description.get("suggested_value") + for key in result["data_schema"].schema + } + assert suggested_values[CONF_USERNAME] == FIXTURE_USER_INPUT[CONF_USERNAME] + assert suggested_values[CONF_PASSWORD] == FIXTURE_USER_INPUT[CONF_PASSWORD] + assert suggested_values[CONF_REGION] == FIXTURE_USER_INPUT[CONF_REGION] + + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], FIXTURE_USER_INPUT + ) + await hass.async_block_till_done() + + assert result2["type"] is FlowResultType.ABORT + assert result2["reason"] == "reconfigure_successful" + assert config_entry.data == FIXTURE_COMPLETE_ENTRY + + +async def test_reconfigure_unique_id_abort(hass: HomeAssistant) -> None: + """Test aborting the reconfiguration form if unique_id changes.""" + with patch( + "bimmer_connected.api.authentication.MyBMWAuthentication.login", + side_effect=login_sideeffect, + autospec=True, + ): + config_entry = MockConfigEntry(**FIXTURE_CONFIG_ENTRY) + config_entry.add_to_hass(hass) + + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + result = await config_entry.start_reconfigure_flow(hass) + assert result["type"] is FlowResultType.FORM + assert result["step_id"] == "user" + assert result["errors"] == {} + + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + {**FIXTURE_USER_INPUT, CONF_USERNAME: "somebody@email.com"}, + ) + await hass.async_block_till_done() + + assert result2["type"] is FlowResultType.ABORT + assert result2["reason"] == "account_mismatch" + assert config_entry.data == FIXTURE_COMPLETE_ENTRY