Fix ability to set HEOS options (#138235)

homeassistant/components/heos/config_flow.py

@@ -5,15 +5,16 @@ import logging
 from typing import TYPE_CHECKING, Any
 from urllib.parse import urlparse
 
-from pyheos import CommandAuthenticationError, Heos, HeosError, HeosOptions
+from pyheos import (
+    CommandAuthenticationError,
+    ConnectionState,
+    Heos,
+    HeosError,
+    HeosOptions,
+)
 import voluptuous as vol
 
-from homeassistant.config_entries import (
+from homeassistant.config_entries import ConfigFlow, ConfigFlowResult, OptionsFlow
-    ConfigEntryState,
-    ConfigFlow,
-    ConfigFlowResult,
-    OptionsFlow,
-)
 from homeassistant.const import CONF_HOST, CONF_PASSWORD, CONF_USERNAME
 from homeassistant.core import callback
 from homeassistant.helpers import selector
@@ -48,13 +49,19 @@ async def _validate_host(host: str, errors: dict[str, str]) -> bool:
 
 
 async def _validate_auth(
-    user_input: dict[str, str], heos: Heos, errors: dict[str, str]
+    user_input: dict[str, str], entry: HeosConfigEntry, errors: dict[str, str]
 ) -> bool:
     """Validate authentication by signing in or out, otherwise populate errors if needed."""
+    can_validate = (
+        hasattr(entry, "runtime_data")
+        and entry.runtime_data.heos.connection_state is ConnectionState.CONNECTED
+    )
     if not user_input:
         # Log out (neither username nor password provided)
+        if not can_validate:
+            return True
         try:
-            await heos.sign_out()
+            await entry.runtime_data.heos.sign_out()
         except HeosError:
             errors["base"] = "unknown"
             _LOGGER.exception("Unexpected error occurred during sign-out")
@@ -73,8 +80,12 @@ async def _validate_auth(
         return False
 
     # Attempt to login (both username and password provided)
+    if not can_validate:
+        return True
     try:
-        await heos.sign_in(user_input[CONF_USERNAME], user_input[CONF_PASSWORD])
+        await entry.runtime_data.heos.sign_in(
+            user_input[CONF_USERNAME], user_input[CONF_PASSWORD]
+        )
     except CommandAuthenticationError as err:
         errors["base"] = "invalid_auth"
         _LOGGER.warning("Failed to sign-in to HEOS Account: %s", err)
@@ -86,7 +97,7 @@ async def _validate_auth(
     else:
         _LOGGER.debug(
             "Successfully signed-in to HEOS Account: %s",
-            heos.signed_in_username,
+            entry.runtime_data.heos.signed_in_username,
         )
         return True
 
@@ -205,8 +216,7 @@ class HeosFlowHandler(ConfigFlow, domain=DOMAIN):
         errors: dict[str, str] = {}
         entry: HeosConfigEntry = self._get_reauth_entry()
         if user_input is not None:
-            assert entry.state is ConfigEntryState.LOADED
+            if await _validate_auth(user_input, entry, errors):
-            if await _validate_auth(user_input, entry.runtime_data.heos, errors):
                 return self.async_update_reload_and_abort(entry, options=user_input)
 
         return self.async_show_form(
@@ -227,8 +237,7 @@ class HeosOptionsFlowHandler(OptionsFlow):
         """Manage the options."""
         errors: dict[str, str] = {}
         if user_input is not None:
-            entry: HeosConfigEntry = self.config_entry
+            if await _validate_auth(user_input, self.config_entry, errors):
-            if await _validate_auth(user_input, entry.runtime_data.heos, errors):
                 return self.async_create_entry(data=user_input)
 
         return self.async_show_form(

tests/components/heos/__init__.py

@@ -2,7 +2,7 @@
 
 from unittest.mock import AsyncMock
 
-from pyheos import Heos, HeosGroup, HeosOptions, HeosPlayer
+from pyheos import ConnectionState, Heos, HeosGroup, HeosOptions, HeosPlayer
 
 
 class MockHeos(Heos):
@@ -60,3 +60,7 @@ class MockHeos(Heos):
     def mock_set_signed_in_username(self, signed_in_username: str | None) -> None:
         """Set the signed in status on the mock instance."""
         self._signed_in_username = signed_in_username
+
+    def mock_set_connection_state(self, connection_state: ConnectionState) -> None:
+        """Set the connection state on the mock instance."""
+        self._connection._state = connection_state

tests/components/heos/test_config_flow.py

@@ -2,7 +2,13 @@
 
 from typing import Any
 
-from pyheos import CommandAuthenticationError, CommandFailedError, HeosError, HeosSystem
+from pyheos import (
+    CommandAuthenticationError,
+    CommandFailedError,
+    ConnectionState,
+    HeosError,
+    HeosSystem,
+)
 import pytest
 
 from homeassistant.components.heos.const import DOMAIN
@@ -232,6 +238,7 @@ async def test_options_flow_signs_in(
     """Test options flow signs-in with entered credentials."""
     config_entry.add_to_hass(hass)
     assert await hass.config_entries.async_setup(config_entry.entry_id)
+    controller.mock_set_connection_state(ConnectionState.CONNECTED)
 
     # Start the options flow. Entry has not current options.
     assert CONF_USERNAME not in config_entry.options
@@ -271,6 +278,7 @@ async def test_options_flow_signs_out(
     """Test options flow signs-out when credentials cleared."""
     config_entry.add_to_hass(hass)
     assert await hass.config_entries.async_setup(config_entry.entry_id)
+    controller.mock_set_connection_state(ConnectionState.CONNECTED)
 
     # Start the options flow. Entry has not current options.
     result = await hass.config_entries.options.async_init(config_entry.entry_id)
@@ -319,6 +327,7 @@ async def test_options_flow_missing_one_param_recovers(
     """Test options flow signs-in after recovering from only username or password being entered."""
     config_entry.add_to_hass(hass)
     assert await hass.config_entries.async_setup(config_entry.entry_id)
+    controller.mock_set_connection_state(ConnectionState.CONNECTED)
 
     # Start the options flow. Entry has not current options.
     assert CONF_USERNAME not in config_entry.options
@@ -347,6 +356,86 @@ async def test_options_flow_missing_one_param_recovers(
     assert result["type"] is FlowResultType.CREATE_ENTRY
 
 
+async def test_options_flow_sign_in_setup_error_saves(
+    hass: HomeAssistant, config_entry: MockConfigEntry, controller: MockHeos
+) -> None:
+    """Test options can still be updated when the integration failed to set up."""
+    config_entry.add_to_hass(hass)
+    controller.get_players.side_effect = ValueError("Unexpected error")
+    await hass.config_entries.async_setup(config_entry.entry_id)
+    assert config_entry.state is ConfigEntryState.SETUP_ERROR
+
+    result = await hass.config_entries.options.async_init(config_entry.entry_id)
+    # Enter valid credentials
+    user_input = {CONF_USERNAME: "user", CONF_PASSWORD: "pass"}
+    result = await hass.config_entries.options.async_configure(
+        result["flow_id"], user_input
+    )
+    assert controller.sign_in.call_count == 0
+    assert controller.sign_out.call_count == 0
+    assert config_entry.options == user_input
+    assert result["data"] == user_input
+    assert result["type"] is FlowResultType.CREATE_ENTRY
+
+
+async def test_options_flow_sign_out_setup_error_saves(
+    hass: HomeAssistant, config_entry: MockConfigEntry, controller: MockHeos
+) -> None:
+    """Test options can still be cleared when the integration failed to set up."""
+    config_entry.add_to_hass(hass)
+    controller.get_players.side_effect = ValueError("Unexpected error")
+    await hass.config_entries.async_setup(config_entry.entry_id)
+    assert config_entry.state is ConfigEntryState.SETUP_ERROR
+
+    result = await hass.config_entries.options.async_init(config_entry.entry_id)
+    # Enter valid credentials
+    result = await hass.config_entries.options.async_configure(result["flow_id"], {})
+    assert controller.sign_in.call_count == 0
+    assert controller.sign_out.call_count == 0
+    assert config_entry.options == {}
+    assert result["data"] == {}
+    assert result["type"] is FlowResultType.CREATE_ENTRY
+
+
+async def test_options_flow_sign_in_not_connected_saves(
+    hass: HomeAssistant, config_entry: MockConfigEntry, controller: MockHeos
+) -> None:
+    """Test options can still be updated when not connected to the HEOS device."""
+    config_entry.add_to_hass(hass)
+    assert await hass.config_entries.async_setup(config_entry.entry_id)
+    controller.mock_set_connection_state(ConnectionState.RECONNECTING)
+
+    result = await hass.config_entries.options.async_init(config_entry.entry_id)
+    # Enter valid credentials
+    user_input = {CONF_USERNAME: "user", CONF_PASSWORD: "pass"}
+    result = await hass.config_entries.options.async_configure(
+        result["flow_id"], user_input
+    )
+    assert controller.sign_in.call_count == 0
+    assert controller.sign_out.call_count == 0
+    assert config_entry.options == user_input
+    assert result["data"] == user_input
+    assert result["type"] is FlowResultType.CREATE_ENTRY
+
+
+async def test_options_flow_sign_out_not_connected_saves(
+    hass: HomeAssistant, config_entry: MockConfigEntry, controller: MockHeos
+) -> None:
+    """Test options can still be cleared when not connected to the HEOS device."""
+    config_entry.add_to_hass(hass)
+    assert await hass.config_entries.async_setup(config_entry.entry_id)
+    controller.mock_set_connection_state(ConnectionState.RECONNECTING)
+
+    result = await hass.config_entries.options.async_init(config_entry.entry_id)
+    # Enter valid credentials
+    result = await hass.config_entries.options.async_configure(result["flow_id"], {})
+    assert controller.sign_in.call_count == 0
+    assert controller.sign_out.call_count == 0
+    assert config_entry.options == {}
+    assert result["data"] == {}
+    assert result["type"] is FlowResultType.CREATE_ENTRY
+
+
 @pytest.mark.parametrize(
     ("error", "expected_error_key"),
     [
@@ -368,6 +457,7 @@ async def test_reauth_signs_in_aborts(
     """Test reauth flow signs-in with entered credentials and aborts."""
     config_entry.add_to_hass(hass)
     assert await hass.config_entries.async_setup(config_entry.entry_id)
+    controller.mock_set_connection_state(ConnectionState.CONNECTED)
     result = await config_entry.start_reauth_flow(hass)
     assert config_entry.state is ConfigEntryState.LOADED
 
@@ -407,6 +497,7 @@ async def test_reauth_signs_out(
     """Test reauth flow signs-out when credentials cleared and aborts."""
     config_entry.add_to_hass(hass)
     assert await hass.config_entries.async_setup(config_entry.entry_id)
+    controller.mock_set_connection_state(ConnectionState.CONNECTED)
     result = await config_entry.start_reauth_flow(hass)
     assert config_entry.state is ConfigEntryState.LOADED
 
@@ -457,6 +548,7 @@ async def test_reauth_flow_missing_one_param_recovers(
     """Test reauth flow signs-in after recovering from only username or password being entered."""
     config_entry.add_to_hass(hass)
     assert await hass.config_entries.async_setup(config_entry.entry_id)
+    controller.mock_set_connection_state(ConnectionState.CONNECTED)
 
     # Start the options flow. Entry has not current options.
     result = await config_entry.start_reauth_flow(hass)
@@ -484,3 +576,51 @@ async def test_reauth_flow_missing_one_param_recovers(
     assert config_entry.options[CONF_PASSWORD] == user_input[CONF_PASSWORD]
     assert result["reason"] == "reauth_successful"
     assert result["type"] is FlowResultType.ABORT
+
+
+async def test_reauth_updates_when_not_connected(
+    hass: HomeAssistant, config_entry: MockConfigEntry, controller: MockHeos
+) -> None:
+    """Test reauth flow signs-in with entered credentials and aborts."""
+    config_entry.add_to_hass(hass)
+    assert await hass.config_entries.async_setup(config_entry.entry_id)
+    controller.mock_set_connection_state(ConnectionState.RECONNECTING)
+
+    result = await config_entry.start_reauth_flow(hass)
+    assert result["step_id"] == "reauth_confirm"
+    assert result["errors"] == {}
+    assert result["type"] is FlowResultType.FORM
+
+    # Valid credentials signs-in, updates options, and aborts
+    user_input = {CONF_USERNAME: "user", CONF_PASSWORD: "pass"}
+    result = await hass.config_entries.flow.async_configure(
+        result["flow_id"], user_input
+    )
+    assert controller.sign_in.call_count == 0
+    assert controller.sign_out.call_count == 0
+    assert config_entry.options[CONF_USERNAME] == user_input[CONF_USERNAME]
+    assert config_entry.options[CONF_PASSWORD] == user_input[CONF_PASSWORD]
+    assert result["reason"] == "reauth_successful"
+    assert result["type"] is FlowResultType.ABORT
+
+
+async def test_reauth_clears_when_not_connected(
+    hass: HomeAssistant, config_entry: MockConfigEntry, controller: MockHeos
+) -> None:
+    """Test reauth flow signs-out with entered credentials and aborts."""
+    config_entry.add_to_hass(hass)
+    assert await hass.config_entries.async_setup(config_entry.entry_id)
+    controller.mock_set_connection_state(ConnectionState.RECONNECTING)
+
+    result = await config_entry.start_reauth_flow(hass)
+    assert result["step_id"] == "reauth_confirm"
+    assert result["errors"] == {}
+    assert result["type"] is FlowResultType.FORM
+
+    # Valid credentials signs-out, updates options, and aborts
+    result = await hass.config_entries.flow.async_configure(result["flow_id"], {})
+    assert controller.sign_in.call_count == 0
+    assert controller.sign_out.call_count == 0
+    assert config_entry.options == {}
+    assert result["reason"] == "reauth_successful"
+    assert result["type"] is FlowResultType.ABORT