From a31d81b963cdb24349d35795b29070fe2b50ce1b Mon Sep 17 00:00:00 2001 From: Franck Nijhof Date: Fri, 4 Sep 2026 09:43:32 +0200 Subject: [PATCH] Fix Synology DSM backup share step losing its input (#181271) --- .../components/synology_dsm/config_flow.py | 20 +++-- .../synology_dsm/test_config_flow.py | 76 ++++++++++++++++++- 2 files changed, 84 insertions(+), 12 deletions(-) diff --git a/homeassistant/components/synology_dsm/config_flow.py b/homeassistant/components/synology_dsm/config_flow.py index 07ed847bdc1c..2654d74fdd5f 100644 --- a/homeassistant/components/synology_dsm/config_flow.py +++ b/homeassistant/components/synology_dsm/config_flow.py @@ -237,7 +237,8 @@ class SynologyDSMFlowHandler(ConfigFlow, domain=DOMAIN): self.shares = await self.api.file.get_shared_folders(only_writable=True) if self.shares and not backup_path: - return await self.async_step_backup_share(user_input) + self.saved_user_input = user_input + return await self.async_step_backup_share() # unique_id should be serial for services purpose existing_entry = await self.async_set_unique_id(serial, raise_on_progress=False) @@ -409,16 +410,13 @@ class SynologyDSMFlowHandler(ConfigFlow, domain=DOMAIN): return await self.async_step_user(user_input) async def async_step_backup_share( - self, user_input: dict[str, Any], errors: dict[str, str] | None = None + self, user_input: dict[str, Any] | None = None ) -> ConfigFlowResult: """Select backup location.""" if TYPE_CHECKING: assert self.shares is not None - if not self.saved_user_input: - self.saved_user_input = user_input - - if CONF_BACKUP_PATH not in user_input and CONF_BACKUP_SHARE not in user_input: + if user_input is None: return self.async_show_form( step_id="backup_share", data_schema=vol.Schema( @@ -435,15 +433,15 @@ class SynologyDSMFlowHandler(ConfigFlow, domain=DOMAIN): vol.Required( CONF_BACKUP_PATH, default=f"{DEFAULT_BACKUP_PATH}_{slugify(self.hass.config.location_name)}", - ): str, + ): vol.All(str, vol.Length(min=1)), } ), ) - user_input = {**self.saved_user_input, **user_input} - self.saved_user_input = {} - - return await self.async_step_user(user_input) + # The credentials stay available, so a retry after a failure still has them + return await self.async_validate_input_create_entry( + {**self.saved_user_input, **user_input}, step_id="user" + ) def _async_get_existing_entry(self, discovered_mac: str) -> ConfigEntry | None: """See if we already have a configured NAS with this MAC address.""" diff --git a/tests/components/synology_dsm/test_config_flow.py b/tests/components/synology_dsm/test_config_flow.py index 7c17d8116af7..941fce9f9d64 100644 --- a/tests/components/synology_dsm/test_config_flow.py +++ b/tests/components/synology_dsm/test_config_flow.py @@ -37,7 +37,7 @@ from homeassistant.const import ( CONF_VERIFY_SSL, ) from homeassistant.core import HomeAssistant -from homeassistant.data_entry_flow import FlowResultType +from homeassistant.data_entry_flow import FlowResultType, InvalidData from homeassistant.helpers.service_info.ssdp import ( ATTR_UPNP_FRIENDLY_NAME, ATTR_UPNP_SERIAL, @@ -383,6 +383,80 @@ async def test_user_with_filestation( assert result["data"] == snapshot +@pytest.mark.usefixtures("mock_setup_entry") +async def test_backup_share_form_reopened( + hass: HomeAssistant, + service_with_filestation: MagicMock, +) -> None: + """Test the backup location form can be shown again without losing the input.""" + with patch( + "homeassistant.components.synology_dsm.config_flow.SynologyDSM", + return_value=service_with_filestation, + ): + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_USER}, + data={ + CONF_HOST: HOST, + CONF_PORT: PORT, + CONF_SSL: USE_SSL, + CONF_VERIFY_SSL: VERIFY_SSL, + CONF_USERNAME: USERNAME, + CONF_PASSWORD: PASSWORD, + }, + ) + + assert result["type"] is FlowResultType.FORM + assert result["step_id"] == "backup_share" + + # The dialog is closed and reopened before a location is picked + result = await hass.config_entries.flow.async_configure(result["flow_id"]) + + assert result["type"] is FlowResultType.FORM + assert result["step_id"] == "backup_share" + + result = await hass.config_entries.flow.async_configure( + result["flow_id"], + {CONF_BACKUP_SHARE: "/ha_backup", CONF_BACKUP_PATH: "automatic_ha_backups"}, + ) + + assert result["type"] is FlowResultType.CREATE_ENTRY + assert result["result"].unique_id == SERIAL + + +@pytest.mark.usefixtures("mock_setup_entry") +async def test_backup_share_requires_a_path( + hass: HomeAssistant, + service_with_filestation: MagicMock, +) -> None: + """Test an empty backup path is rejected instead of looping the flow.""" + with patch( + "homeassistant.components.synology_dsm.config_flow.SynologyDSM", + return_value=service_with_filestation, + ): + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": SOURCE_USER}, + data={ + CONF_HOST: HOST, + CONF_PORT: PORT, + CONF_SSL: USE_SSL, + CONF_VERIFY_SSL: VERIFY_SSL, + CONF_USERNAME: USERNAME, + CONF_PASSWORD: PASSWORD, + }, + ) + + assert result["type"] is FlowResultType.FORM + assert result["step_id"] == "backup_share" + + with pytest.raises(InvalidData): + await hass.config_entries.flow.async_configure( + result["flow_id"], + {CONF_BACKUP_SHARE: "/ha_backup", CONF_BACKUP_PATH: ""}, + ) + + @pytest.mark.usefixtures("mock_setup_entry") async def test_reauth(hass: HomeAssistant, service: MagicMock) -> None: """Test reauthentication."""