Address post-merge Ridwell code review (#58857)

homeassistant/components/ridwell/__init__.py

@@ -24,9 +24,6 @@ PLATFORMS: list[str] = ["sensor"]
 
 async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     """Set up Ridwell from a config entry."""
-    hass.data.setdefault(DOMAIN, {})
-    hass.data[DOMAIN][entry.entry_id] = {}
-
     session = aiohttp_client.async_get_clientsession(hass)
 
     try:
@@ -67,8 +64,11 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     )
 
     await coordinator.async_config_entry_first_refresh()
-    hass.data[DOMAIN][entry.entry_id][DATA_ACCOUNT] = accounts
+    hass.data.setdefault(DOMAIN, {})
-    hass.data[DOMAIN][entry.entry_id][DATA_COORDINATOR] = coordinator
+    hass.data[DOMAIN][entry.entry_id] = {
+        DATA_ACCOUNT: accounts,
+        DATA_COORDINATOR: coordinator,
+    }
 
     hass.config_entries.async_setup_platforms(entry, PLATFORMS)
 

homeassistant/components/ridwell/config_flow.py

@@ -9,7 +9,6 @@ import voluptuous as vol
 
 from homeassistant import config_entries
 from homeassistant.const import CONF_PASSWORD, CONF_USERNAME
-from homeassistant.core import callback
 from homeassistant.data_entry_flow import FlowResult
 from homeassistant.helpers import aiohttp_client, config_validation as cv
 from homeassistant.helpers.typing import ConfigType
@@ -38,25 +37,13 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
     def __init__(self) -> None:
         """Initialize."""
         self._password: str | None = None
-        self._reauthing: bool = False
         self._username: str | None = None
 
-    @callback
-    def _async_show_errors(
-        self, errors: dict, error_step_id: str, error_schema: vol.Schema
-    ) -> FlowResult:
-        """Show an error on the correct form."""
-        return self.async_show_form(
-            step_id=error_step_id,
-            data_schema=error_schema,
-            errors=errors,
-            description_placeholders={CONF_USERNAME: self._username},
-        )
-
     async def _async_validate(
         self, error_step_id: str, error_schema: vol.Schema
     ) -> FlowResult:
         """Validate input credentials and proceed accordingly."""
+        errors = {}
         session = aiohttp_client.async_get_clientsession(self.hass)
 
         if TYPE_CHECKING:
@@ -66,25 +53,28 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
         try:
             await async_get_client(self._username, self._password, session=session)
         except InvalidCredentialsError:
-            return self._async_show_errors(
+            errors["base"] = "invalid_auth"
-                {"base": "invalid_auth"}, error_step_id, error_schema
-            )
         except RidwellError as err:
             LOGGER.error("Unknown Ridwell error: %s", err)
-            return self._async_show_errors(
+            errors["base"] = "unknown"
-                {"base": "unknown"}, error_step_id, error_schema
+
+        if errors:
+            return self.async_show_form(
+                step_id=error_step_id,
+                data_schema=error_schema,
+                errors=errors,
+                description_placeholders={CONF_USERNAME: self._username},
             )
 
-        if self._reauthing:
+        if existing_entry := await self.async_set_unique_id(self._username):
-            if existing_entry := await self.async_set_unique_id(self._username):
+            self.hass.config_entries.async_update_entry(
-                self.hass.config_entries.async_update_entry(
+                existing_entry,
-                    existing_entry,
+                data={**existing_entry.data, CONF_PASSWORD: self._password},
-                    data={**existing_entry.data, CONF_PASSWORD: self._password},
+            )
-                )
+            self.hass.async_create_task(
-                self.hass.async_create_task(
+                self.hass.config_entries.async_reload(existing_entry.entry_id)
-                    self.hass.config_entries.async_reload(existing_entry.entry_id)
+            )
-                )
+            return self.async_abort(reason="reauth_successful")
-                return self.async_abort(reason="reauth_successful")
 
         return self.async_create_entry(
             title=self._username,
@@ -93,7 +83,6 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
 
     async def async_step_reauth(self, config: ConfigType) -> FlowResult:
         """Handle configuration by re-auth."""
-        self._reauthing = True
         self._username = config[CONF_USERNAME]
         return await self.async_step_reauth_confirm()
 

homeassistant/components/ridwell/sensor.py

@@ -2,22 +2,20 @@
 from __future__ import annotations
 
 from collections.abc import Mapping
-from datetime import date, datetime
 from typing import Any
 
-from aioridwell.client import RidwellAccount
+from aioridwell.client import RidwellAccount, RidwellPickupEvent
 
 from homeassistant.components.sensor import SensorEntity
 from homeassistant.config_entries import ConfigEntry
-from homeassistant.const import ATTR_ATTRIBUTION, DEVICE_CLASS_TIMESTAMP
+from homeassistant.const import DEVICE_CLASS_DATE
-from homeassistant.core import HomeAssistant, callback
+from homeassistant.core import HomeAssistant
 from homeassistant.helpers.entity_platform import AddEntitiesCallback
 from homeassistant.helpers.typing import StateType
 from homeassistant.helpers.update_coordinator import (
     CoordinatorEntity,
     DataUpdateCoordinator,
 )
-from homeassistant.util.dt import as_utc
 
 from .const import DATA_ACCOUNT, DATA_COORDINATOR, DOMAIN
 
@@ -26,14 +24,6 @@ ATTR_PICKUP_STATE = "pickup_state"
 ATTR_PICKUP_TYPES = "pickup_types"
 ATTR_QUANTITY = "quantity"
 
-DEFAULT_ATTRIBUTION = "Pickup data provided by Ridwell"
-
-
-@callback
-def async_get_utc_midnight(target_date: date) -> datetime:
-    """Get UTC midnight for a given date."""
-    return as_utc(datetime.combine(target_date, datetime.min.time()))
-
 
 async def async_setup_entry(
     hass: HomeAssistant, entry: ConfigEntry, async_add_entities: AddEntitiesCallback
@@ -49,7 +39,7 @@ async def async_setup_entry(
 class RidwellSensor(CoordinatorEntity, SensorEntity):
     """Define a Ridwell pickup sensor."""
 
-    _attr_device_class = DEVICE_CLASS_TIMESTAMP
+    _attr_device_class = DEVICE_CLASS_DATE
 
     def __init__(
         self, coordinator: DataUpdateCoordinator, account: RidwellAccount
@@ -67,7 +57,6 @@ class RidwellSensor(CoordinatorEntity, SensorEntity):
         event = self.coordinator.data[self._account.account_id]
 
         attrs: dict[str, Any] = {
-            ATTR_ATTRIBUTION: DEFAULT_ATTRIBUTION,
             ATTR_PICKUP_TYPES: {},
             ATTR_PICKUP_STATE: event.state,
         }
@@ -82,12 +71,12 @@ class RidwellSensor(CoordinatorEntity, SensorEntity):
                 # Ridwell's API will return distinct objects, even if they have the
                 # same name (e.g. two pickups of Latex Paint will show up as two
                 # objects) – so, we sum the quantities:
-                attrs[ATTR_PICKUP_TYPES][pickup.name]["quantity"] += pickup.quantity
+                attrs[ATTR_PICKUP_TYPES][pickup.name][ATTR_QUANTITY] += pickup.quantity
 
         return attrs
 
     @property
     def native_value(self) -> StateType:
         """Return the value reported by the sensor."""
-        event = self.coordinator.data[self._account.account_id]
+        event: RidwellPickupEvent = self.coordinator.data[self._account.account_id]
-        return async_get_utc_midnight(event.pickup_date).isoformat()
+        return event.pickup_date.isoformat()

homeassistant/components/ridwell/strings.json

@@ -21,7 +21,7 @@
       "unknown": "[%key:common::config_flow::error::unknown%]"
     },
     "abort": {
-      "already_configured": "[%key:common::config_flow::abort::already_configured_device%]",
+      "already_configured": "[%key:common::config_flow::abort::already_configured_account%]",
       "reauth_successful": "[%key:common::config_flow::abort::reauth_successful%]"
     }
   }

homeassistant/components/ridwell/translations/en.json

@@ -1,7 +1,7 @@
 {
     "config": {
         "abort": {
-            "already_configured": "Device is already configured",
+            "already_configured_account": "Account is already configured",
             "reauth_successful": "Re-authentication was successful"
         },
         "error": {

tests/components/ridwell/test_config_flow.py

@@ -107,10 +107,15 @@ async def test_step_user(hass: HomeAssistant, client_login) -> None:
 
 
 @pytest.mark.parametrize(
-    "client",
+    "client,error",
-    [AsyncMock(side_effect=InvalidCredentialsError)],
+    [
+        (AsyncMock(side_effect=InvalidCredentialsError), "invalid_auth"),
+        (AsyncMock(side_effect=RidwellError), "unknown"),
+    ],
 )
-async def test_step_user_invalid_credentials(hass: HomeAssistant, client_login) -> None:
+async def test_step_user_invalid_credentials(
+    hass: HomeAssistant, client_login, error
+) -> None:
     """Test that invalid credentials are handled correctly."""
     result = await hass.config_entries.flow.async_init(
         DOMAIN,
@@ -119,20 +124,4 @@ async def test_step_user_invalid_credentials(hass: HomeAssistant, client_login)
     )
 
     assert result["type"] == RESULT_TYPE_FORM
-    assert result["errors"] == {"base": "invalid_auth"}
+    assert result["errors"]["base"] == error
-
-
-@pytest.mark.parametrize(
-    "client",
-    [AsyncMock(side_effect=RidwellError)],
-)
-async def test_step_user_unknown_error(hass: HomeAssistant, client_login) -> None:
-    """Test that an unknown Ridwell error is handled correctly."""
-    result = await hass.config_entries.flow.async_init(
-        DOMAIN,
-        context={"source": config_entries.SOURCE_USER},
-        data={CONF_USERNAME: "user@email.com", CONF_PASSWORD: "password"},
-    )
-
-    assert result["type"] == RESULT_TYPE_FORM
-    assert result["errors"] == {"base": "unknown"}