Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 4 additions & 18 deletions homeassistant/components/anglian_water/config_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
vol.Required(CONF_PASSWORD): selector.TextSelector(
selector.TextSelectorConfig(type=selector.TextSelectorType.PASSWORD)
),
vol.Required(CONF_ACCOUNT_NUMBER): selector.TextSelector(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we get the account number from the API and set it as default value? Or would that be counter productive?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would require an extra step in the config flow as we can only get that information following a successful auth (following KISS - https://en.wikipedia.org/wiki/KISS_principle).

Perhaps for a follow up PR I could look at that what was originally proposed in the initial PR to add a select field with valid accounts for a release next year (it feels odd to say that)? More work needs to be done on the library to support that first.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right - the extra step makes sense. What I don't understand yet is why inputting the number solves the problem, does the API currently select the first "working" account automatically? Or what exactly is going wrong here? The other thing is: How do we get users for which this is currently broken back to a working state, is there a way to guide them without deleting the entry first

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inputting the number solves the problem as currently the account number is collected from the claims in the authentication token, not the actual API itself. It seems Anglian Water don't keep their Azure B2C users up to date with whats on their internal SAP database and therefore when you move house (or the account owner changes), you are issued with a new account number, yet retain access to your old and inactive account number which is what is still stored in the B2C user profile.

Apologies if this doesn't make sense, I don't really know how best to explain this as it does get quite complicated the more you dig.

Users will need to delete the entry, there isn't any possible way to migrate them to a working state because we won't be able to know what their account numbers are and what is "correct".

Related discussion: #156225 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its worth noting I only have access to my own account which is effectively the golden path, there are no public API docs apart from what I've figured out from de-obfuscating frontend javascript & watching network requests in the browser.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking if we can detect a broken entry and offer the user with a repair where they have to enter the missing account info - tbf I'm not even sure it would be worth the effort. I agree though that a select would be a bit nicer, but that's too much effort?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh yeah, I see what you mean now. I think it still might not quite work how we think because the account is always populated anyway in the config entry data.

A select would be a "nice to have", but I think that is currently too much effort for the immediate problem, especially as despite adding functionality to retrieve associated accounts (pantherale0/pyanglianwater#37) to the library, it still needs to validate each account which requires refactoring the library to support multiple accounts in one initialization (if that makes sense), I've already started this, but the code isn't published yet because it currently breaks too many things.

}
)

Expand Down Expand Up @@ -68,34 +69,19 @@ async def async_step_user(
self.hass,
cookie_jar=CookieJar(quote_cookie=False),
),
account_number=user_input.get(CONF_ACCOUNT_NUMBER),
account_number=user_input[CONF_ACCOUNT_NUMBER],
)
)
if isinstance(validation_response, BaseAuth):
account_number = (
user_input.get(CONF_ACCOUNT_NUMBER)
or validation_response.account_number
)
await self.async_set_unique_id(account_number)
await self.async_set_unique_id(user_input[CONF_ACCOUNT_NUMBER])
self._abort_if_unique_id_configured()
return self.async_create_entry(
title=account_number,
title=user_input[CONF_ACCOUNT_NUMBER],
data={
**user_input,
CONF_ACCESS_TOKEN: validation_response.refresh_token,
CONF_ACCOUNT_NUMBER: account_number,
},
)
if validation_response == "smart_meter_unavailable":
return self.async_show_form(
step_id="user",
data_schema=STEP_USER_DATA_SCHEMA.extend(
{
vol.Required(CONF_ACCOUNT_NUMBER): selector.TextSelector(),
}
),
errors={"base": validation_response},
)
errors["base"] = validation_response

return self.async_show_form(
Expand Down
5 changes: 5 additions & 0 deletions tests/components/anglian_water/test_config_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ async def test_full_flow(
user_input={
CONF_USERNAME: USERNAME,
CONF_PASSWORD: PASSWORD,
CONF_ACCOUNT_NUMBER: ACCOUNT_NUMBER,
},
)

Expand Down Expand Up @@ -74,6 +75,7 @@ async def test_already_configured(
user_input={
CONF_USERNAME: USERNAME,
CONF_PASSWORD: PASSWORD,
CONF_ACCOUNT_NUMBER: ACCOUNT_NUMBER,
},
)

Expand Down Expand Up @@ -107,6 +109,7 @@ async def test_auth_recover_exception(
user_input={
CONF_USERNAME: USERNAME,
CONF_PASSWORD: PASSWORD,
CONF_ACCOUNT_NUMBER: ACCOUNT_NUMBER,
},
)

Expand All @@ -123,6 +126,7 @@ async def test_auth_recover_exception(
user_input={
CONF_USERNAME: USERNAME,
CONF_PASSWORD: PASSWORD,
CONF_ACCOUNT_NUMBER: ACCOUNT_NUMBER,
},
)

Expand Down Expand Up @@ -164,6 +168,7 @@ async def test_account_recover_exception(
user_input={
CONF_USERNAME: USERNAME,
CONF_PASSWORD: PASSWORD,
CONF_ACCOUNT_NUMBER: ACCOUNT_NUMBER,
},
)

Expand Down
Loading