Fix Synology DSM backup share step losing its input (#181271)

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."""

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."""