Apply Z-Wave JS add-on config reverts directly (#179899)

Co-authored-by: Paulus Schoutsen <balloob@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

homeassistant/components/zwave_js/config_flow.py

@@ -149,11 +149,6 @@ class SecurityKeys:
         }
 
 
-ADDON_USER_INPUT_MAP = {
-    CONF_ADDON_DEVICE: CONF_USB_PATH,
-    CONF_ADDON_SOCKET: CONF_SOCKET_PATH,
-} | {field.name: field.name for field in fields(SecurityKeys)}
-
 CONF_ADDON_RF_REGION = "rf_region"
 
 EXAMPLE_SERVER_URL = "ws://localhost:3000"
@@ -403,7 +398,6 @@ class ZWaveJSConfigFlow(ConfigFlow, domain=DOMAIN):
         self.install_task: asyncio.Task | None = None
         self.start_task: asyncio.Task | None = None
         self.version_info: VersionInfo | None = None
-        self.revert_reason: str | None = None
         self.backup_task: asyncio.Task | None = None
         self.restore_backup_task: asyncio.Task | None = None
         self.backup_data: bytes | None = None
@@ -1462,8 +1456,7 @@ class ZWaveJSConfigFlow(ConfigFlow, domain=DOMAIN):
 
         if user_input is not None:
             # Missing keys default to the current add-on config, so
-            # existing keys are preserved. The revert helper always passes
+            # existing keys are preserved.
-            # all keys, so the defaults never apply while reverting.
             self.security_keys = default_keys.updated_from_user_input(user_input)
             self.usb_path = user_input.get(CONF_USB_PATH) or None
             self.socket_path = user_input.get(CONF_SOCKET_PATH) or None
@@ -1646,11 +1639,6 @@ class ZWaveJSConfigFlow(ConfigFlow, domain=DOMAIN):
         """
         config_entry = self._reconfigure_config_entry
         assert config_entry is not None
-        if self.revert_reason:
-            self._addon_setup.original_config = None
-            reason = self.revert_reason
-            self.revert_reason = None
-            return await self.async_revert_addon_config(reason=reason)
 
         if not self.ws_address:
             discovery_info = await self._addon_setup.async_get_addon_discovery_info()
@@ -1801,32 +1789,20 @@ class ZWaveJSConfigFlow(ConfigFlow, domain=DOMAIN):
         return await self.async_step_installation_type()
 
     async def async_revert_addon_config(self, reason: str) -> ConfigFlowResult:
-        """Abort the options flow.
+        """Abort the flow.
 
         If the add-on options have been changed, revert those and restart add-on.
         """
-        # If reverting the add-on options failed, abort immediately.
+        _LOGGER.debug("Reverting add-on options, reason: %s", reason)
-        if self.revert_reason:
+        if (original_config := self._addon_setup.original_config) is None:
-            _LOGGER.error(
-                "Failed to revert add-on options before aborting flow, reason: %s",
-                reason,
-            )
-
-        if self.revert_reason or not self._addon_setup.original_config:
-            config_entry = self._reconfigure_config_entry
-            assert config_entry is not None
             self._async_schedule_entry_reload()
-            return self.async_abort(reason=reason)
+        else:
-
+            # Clear the abandoned-flow recovery state, so async_remove
-        self.revert_reason = reason
+            # doesn't restore the add-on config a second time.
-        original_config = self._addon_setup.original_config
+            self._addon_setup.original_config = None
-        addon_config_input = SecurityKeys.from_config(original_config).to_dict() | {
+            self._entry_unloaded_by_flow = False
-            ADDON_USER_INPUT_MAP[addon_key]: addon_val
+            await self._async_restore_addon_config_and_reload(original_config)
-            for addon_key, addon_val in original_config.items()
+        return self.async_abort(reason=reason)
-            if addon_key in (CONF_ADDON_DEVICE, CONF_ADDON_SOCKET)
-        }
-        _LOGGER.debug("Reverting app options, reason: %s", reason)
-        return await self.async_step_configure_addon_reconfigure(addon_config_input)
 
     async def _async_backup_network(self) -> None:
         """Backup the current network."""

tests/components/zwave_js/test_config_flow.py

@@ -4595,8 +4595,6 @@ async def different_device_server_version(*args):
                 "s2_access_control_key": "old456",
                 "s2_authenticated_key": "old789",
                 "s2_unauthenticated_key": "old987",
-                "lr_s2_access_control_key": "",
-                "lr_s2_authenticated_key": "",
             },
             0,
             different_device_server_version,
@@ -4673,17 +4671,8 @@ async def test_reconfigure_different_device(
     assert set_addon_options.call_args == call(
         "core_zwave_js", AddonsOptions(config=revert_addon_options)
     )
-    assert result["type"] is FlowResultType.SHOW_PROGRESS
-    assert result["step_id"] == "start_addon"
-
-    await hass.async_block_till_done()
-
     assert restart_addon.call_count == 2
     assert restart_addon.call_args == call("core_zwave_js")
-
-    result = await hass.config_entries.flow.async_configure(result["flow_id"])
-    await hass.async_block_till_done()
-
     assert result["type"] is FlowResultType.ABORT
     assert result["reason"] == "different_device"
     assert entry.data == data
@@ -4700,9 +4689,11 @@ async def test_reconfigure_different_device(
         "new_addon_options",
         "disconnect_calls",
         "restart_addon_side_effect",
+        "final_connect_calls",
+        "final_disconnect_calls",
     ),
     [
-        (
+        pytest.param(
             {},
             {
                 "device": "/test",
@@ -4734,8 +4725,11 @@ async def test_reconfigure_different_device(
             },
             0,
             [SupervisorError(), None],
+            2,
+            1,
+            id="revert_restart_success",
         ),
-        (
+        pytest.param(
             {},
             {
                 "device": "/test",
@@ -4770,6 +4764,9 @@ async def test_reconfigure_different_device(
                 SupervisorError(),
                 SupervisorError(),
             ],
+            1,
+            0,
+            id="revert_restart_failed",
         ),
     ],
 )
@@ -4785,6 +4782,8 @@ async def test_reconfigure_addon_restart_failed(
     form_data: dict[str, Any],
     new_addon_options: dict[str, Any],
     disconnect_calls: int,
+    final_connect_calls: int,
+    final_disconnect_calls: int,
 ) -> None:
     """Test reconfigure flow and add-on restart failure."""
     addon_options.update(old_addon_options)
@@ -4843,22 +4842,13 @@ async def test_reconfigure_addon_restart_failed(
     assert set_addon_options.call_args == call(
         "core_zwave_js", AddonsOptions(config=old_addon_options)
     )
-    assert result["type"] is FlowResultType.SHOW_PROGRESS
-    assert result["step_id"] == "start_addon"
-
-    await hass.async_block_till_done()
-
     assert restart_addon.call_count == 2
     assert restart_addon.call_args == call("core_zwave_js")
-
-    result = await hass.config_entries.flow.async_configure(result["flow_id"])
-    await hass.async_block_till_done()
-
     assert result["type"] is FlowResultType.ABORT
     assert result["reason"] == "addon_start_failed"
     assert entry.data == data
-    assert client.connect.call_count == 2
+    assert client.connect.call_count == final_connect_calls
-    assert client.disconnect.call_count == 1
+    assert client.disconnect.call_count == final_disconnect_calls
 
 
 @pytest.mark.usefixtures("supervisor", "addon_running", "restart_addon")