Add ConfigFlow to Prowl integration #133771

mgust · 2024-12-22T00:43:43Z

Proposed change

Planning to modernise the Prowl component by moving to use pyprowl and give it a ConfigFlow UI rather than the legacy YAML config. As the component seems unmaintained and legacy, I am starting with adding tests for the existing code.

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

N/A

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
I have followed the perfect PR recommendations
The code has been formatted using Ruff (ruff format homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

home-assistant

Hello @mgust,

When attempting to inspect the commits of your pull request for CLA signature status among all authors we encountered commit(s) which were not linked to a GitHub account, thus not allowing us to determine their status(es).

The commits that are missing a linked GitHub account are the following:

1ff6a502565d58d3a3e0675296999ef2cd7ec89c - This commit has something that looks like an email address ([email protected]). Maybe try linking that to GitHub?.

Unfortunately, we are unable to accept this pull request until this situation is corrected.

Here are your options:

If you had an email address set for the commit that simply wasn't linked to your GitHub account you can link that email now and it will retroactively apply to your commits. The simplest way to do this is to click the link to one of the above commits and look for a blue question mark in a blue circle in the top left. Hovering over that bubble will show you what email address you used. Clicking on that button will take you to your email address settings on GitHub. Just add the email address on that page and you're all set. GitHub has more information about this option in their help center.
If you didn't use an email address at all, it was an invalid email, or it's one you can't link to your GitHub, you will need to change the authorship information of the commit and your global Git settings so this doesn't happen again going forward. GitHub provides some great instructions on how to change your authorship information in their help center.
- If you only made a single commit you should be able to run
```
git commit --amend --author="Author Name <[email protected]>"
```
  (substituting "Author Name" and "[email protected]" for your actual information) to set the authorship information.
- If you made more than one commit and the commit with the missing authorship information is not the most recent one you have two options:
  1. You can re-create all commits missing authorship information. This is going to be the easiest solution for developers that aren't extremely confident in their Git and command line skills.
  2. You can use this script that GitHub provides to rewrite history. Please note: this should be used only if you are very confident in your abilities and understand its impacts.
- Whichever method you choose, I will come by to re-check the pull request once you push the fixes to this branch.

We apologize for this inconvenience, especially since it usually bites new contributors to Home Assistant. We hope you understand the need for us to protect ourselves and the great community we all have built legally. The best thing to come out of this is that you only need to fix this once and it benefits the entire Home Assistant and GitHub community.

Thanks, I look forward to checking this PR again soon! ❤️

home-assistant · 2024-12-22T00:43:47Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

home-assistant

Hi @mgust

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

Stale

frenck

Honestly, looking at this PR...

I'm not fully following the reasoning on why? As in, this integration needs to move to the UI. It would way better effort spent in putting development time into that.

../Frenck

tests/components/prowl/test_notification.py

tests/components/prowl/conftest.py

mgust · 2024-12-23T09:26:29Z

On the practical side, I was adding the tests first to make sure I don't make things incompatible with the current setup.

I wasn't too sure of how split up you normally do PRs, the developer docs seemed to encourage just adding tests as a separate PR rather than just one PR to replace the entire component (the current component is tiny, so to move it to pyprowl, change to the non legacy setup method and add ConfigFlow a will essentially be a complete rewrite), but very happy to do it all in one go if it makes it easier to review.

…different base classes and added the reauth config flow

joostlek

I just checked with @jbouwh who introduced the notify entity and we agreed that ideally we have the config flow in 1 PR and add the migration to the notify entity in a followup PR.

mgust · 2025-05-15T23:08:51Z

I just checked with @jbouwh who introduced the notify entity and we agreed that ideally we have the config flow in 1 PR and add the migration to the notify entity in a followup PR.

Ok. I was attempting to support both the legacy YAML configuration at the same time as the ConfigFlow one where the YAML one would set up the legacy notify service and the ConfigFlow would do the new notify entity. And then in a later PR remove the legacy one in its entirety. But sure, I'll just change the ConfigFlow to use the legacy service.

joostlek · 2025-05-15T23:10:51Z

We should completely migrate the current yaml to a config flow and also import existing config into a config entry.

I love these migrations, so happy to help

mgust · 2025-05-15T23:39:35Z

We should completely migrate the current yaml to a config flow and also import existing config into a config entry.

I love these migrations, so happy to help

Oh, I see, so remove the YAML config entirely and just treat it as a breaking change for those who use it? I was playing around with using the YAML config to create a ConfigFlow entry as part of the startup to help with the migration (and hopefully the users would upgrade to that version before we remove the entire YAML parsing). But then again, maybe it gets too involved if it is easier just to mention it as a breaking change.

Thanks for the offer to help, it took me quite a while to get my head around the various initialization paths, so I wouldn't be at all surprised if I misunderstood something.

…r PR)

mgust · 2025-05-16T01:04:57Z

We should completely migrate the current yaml to a config flow and also import existing config into a config entry.
I love these migrations, so happy to help

Oh, I see, so remove the YAML config entirely and just treat it as a breaking change for those who use it? I was playing around with using the YAML config to create a ConfigFlow entry as part of the startup to help with the migration (and hopefully the users would upgrade to that version before we remove the entire YAML parsing). But then again, maybe it gets too involved if it is easier just to mention it as a breaking change.

Thanks for the offer to help, it took me quite a while to get my head around the various initialization paths, so I wouldn't be at all surprised if I misunderstood something.

Ah, just remembered why I was struggling. When using the BaseNotificationService as the base class of LegacyProwlNotificationService adding it using AddConfigEntryEntitiesCallback in async_setup_entry causes missing attribute issues, as it expects a new style Entity class I believe. If I try to mix in the NotifyEntity class as well (which appeared to be the right way to get said attributes when looking at other components), it clashes with the BaseNotificationService.

And if I remove the BaseNotificationService it doesn't play well with the legacy notification platform initialization which expects that as the base class.

Any suggestions @joostlek of how to move from the YAML to ConfigFlow without adding the new style of base class? (Or, perhaps I've misunderstood how the two initializations work, wouldn't be at all surprised if that is the case 😄 )

emontnemery · 2025-06-25T10:33:54Z

Config entries added by this PR should use the modern NotifyEntity, not the legacy notify services.
There should be a config entry per API key, and notifiers should be stored as subentries.

In follow-up PRs which implement migration from yaml to config entry, a config entry should be created per API key with a subentry for each configured notify.

@mgust please reach out on Discord, I'm@emontnemery there too, if you're stuck and need some help to get the PR moving again

…r a later PR)" This reverts commit 9105bf7.

… to allow the new ProwlNotificationService to co-exist).

emontnemery · 2025-06-30T08:17:53Z

tests/components/prowl/test_config_flow.py

+)
+
+
+@pytest.mark.asyncio


We don't need this, we have configured pytest for asyncio. Please remove it from all tests.

emontnemery · 2025-06-30T08:21:14Z

tests/components/prowl/test_notification.py

+@pytest.mark.asyncio
+async def test_send_notification(hass: HomeAssistant, mock_pyprowl_success) -> None:
+    """Sending a notification message via Prowl."""
+    prowl = ProwlNotificationService(hass, TEST_NAME, TEST_API_KEY)


Don't test this class directly. Instead, set up a config entry, then call the notify service and check that the mocked API was called. In that way, tests make sure we can set up a config entry and the config entry creates entities which are working as expected.

emontnemery · 2025-06-30T08:21:30Z

homeassistant/components/prowl/notify.py

@@ -76,3 +84,72 @@ async def async_send_message(self, message, **kwargs):
                )
        except TimeoutError:
            _LOGGER.error("Timeout accessing Prowl at %s", url)
+
+
+class ProwlNotificationService(NotifyEntity):


Suggested change

class ProwlNotificationService(NotifyEntity):

class ProwlNotificationEntity(NotifyEntity):

homeassistant/components/prowl/notify.py

emontnemery · 2025-06-30T08:22:57Z

homeassistant/components/prowl/notify.py

+
+
+class ProwlNotificationService(NotifyEntity):
+    """Implement the notification service for Prowl."""


Suggested change

"""Implement the notification service for Prowl."""

"""Implement the notification service for Prowl.

This class is used for Prowl config entries.

"""

emontnemery · 2025-06-30T08:23:26Z

tests/components/prowl/test_notification.py

Rename to test_notify.py to match the implementation.

emontnemery · 2025-06-30T08:24:37Z

tests/components/prowl/test_notification.py

To ensure the PR doesn't break existing user setups, could we add tests which mimic prowl set up via configuration.yaml?
Ideally, add those tests in a preliminary PR.

I did, but Frenck suggested we add the ConfigFlow first. (#133771 (review))

yaml setup should still work (via import) after migrating to a config flow. With the tests we can ensure nothing breaks.
I suggest to start with a PR (before his one) that adds tests for the YAML setup.
After switching to discovery the YAML config can be passed via discovery during the entry setup.
The same config then can be used to setup the entity.
The import ensures a smooth migration.
A repair issue should shown as long the YAML is configured.

NoRi2909 · 2025-06-30T13:16:01Z

homeassistant/components/prowl/strings.json

+  "config": {
+    "step": {
+      "user": {
+        "description": "Enter name of Prowl key and the API key.",


Perhaps we can avoid repeating "key" here? For example by using:

Suggested change

"description": "Enter name of Prowl key and the API key.",

"description": "Enter the Prowl API key and its name.",

Just a suggestion, perhaps the "name" can be explained differently, too.

Hmm, fair shout. Maybe "Identifier" might clearer here. "Enter the Prowl API key and an identifier for it."?

I don't know about the workflow to obtain the API key and the name the user should enter here. I'm just a translator so I check if the string is easy to understand for translation in Lokalise where we don't see any additional information.

Perhaps you can describe where to get them by adding a few words. This should make clear if you should also replace "name" with "ID" or "Identifier".

The identifier/name is just so the user can have multiple API keys simultaneously (In case they have several users that need different notifications). It isn't used in the calls to the Prowl service. It'd correspond to the "name" parameter in the YAML config.

https://www.home-assistant.io/integrations/prowl/

jbouwh · 2025-06-30T15:52:58Z

Ok. I was attempting to support both the legacy YAML configuration at the same time as the ConfigFlow one where the YAML one would set up the legacy notify service and the ConfigFlow would do the new notify entity. And then in a later PR remove the legacy one in its entirety. But sure, I'll just change the ConfigFlow to use the legacy service.

I suggest we add the config flow first, and add a step_import to import the YAML into an entry (or subentry).
In a follow up PR we can deprecate the legacy service and start a repair flow if it is still being used. And a new notify entities as a replacement. The message priority option can be added to the option flow or a subentry flow.

jbouwh · 2025-06-30T15:39:40Z

homeassistant/components/prowl/notify.py

    """Get the Prowl notification service."""
-    return ProwlNotificationService(hass, config[CONF_API_KEY])
+    return LegacyProwlNotificationService(hass, config[CONF_API_KEY])


May be we can deprecate the legacy notify service, and start a repair flow, when the legacy service is still being used?There is a helper you can use:

core/homeassistant/components/notify/repairs.py

Lines 12 to 53 in b52a248

@callback

def migrate_notify_issue(

hass: HomeAssistant,

domain: str,

integration_title: str,

breaks_in_ha_version: str,

service_name: str | None = None,

) -> None:

"""Ensure an issue is registered."""

if service_name is not None:

ir.async_create_issue(

hass,

DOMAIN,

f"migrate_notify_{domain}_{service_name}",

breaks_in_ha_version=breaks_in_ha_version,

issue_domain=domain,

is_fixable=True,

is_persistent=True,

translation_key="migrate_notify_service",

translation_placeholders={

"domain": domain,

"integration_title": integration_title,

"service_name": service_name,

},

severity=ir.IssueSeverity.WARNING,

)

return

ir.async_create_issue(

hass,

DOMAIN,

f"migrate_notify_{domain}",

breaks_in_ha_version=breaks_in_ha_version,

issue_domain=domain,

is_fixable=True,

is_persistent=True,

translation_key="migrate_notify",

translation_placeholders={

"domain": domain,

"integration_title": integration_title,

},

severity=ir.IssueSeverity.WARNING,

)

For some examples on how to use it:

https://github.com/home-assistant/core/pull/118855/files

To implement the priority option, you could either add this to the option flow (follow up PR), en extra entity, or a subentry flow that allows to configure priority.

Oh, neat, that sounds like a great way to do the actual migration. I'll do that in a follow-up PR once I've managed to get this one through. :)

…ocstring tweaks, removed unneeded asyncio from pytest

home-assistant bot previously requested changes Dec 22, 2024

View reviewed changes

home-assistant bot added cla-error code-quality has-tests integration: prowl small-pr PRs with less than 30 lines. labels Dec 22, 2024

home-assistant bot marked this pull request as draft December 22, 2024 00:43

home-assistant bot added the Quality Scale: legacy label Dec 22, 2024

mgust force-pushed the prowl-tests branch from 1ff6a50 to ec42ec0 Compare December 22, 2024 00:50

home-assistant bot added the cla-needed label Dec 22, 2024

home-assistant bot previously requested changes Dec 22, 2024

View reviewed changes

home-assistant bot added cla-recheck cla-signed and removed cla-recheck cla-needed labels Dec 22, 2024

mgust marked this pull request as ready for review December 22, 2024 00:53

gjohansson-ST added cla-recheck and removed cla-error labels Dec 22, 2024

home-assistant bot removed the cla-recheck label Dec 22, 2024

frenck requested changes Dec 22, 2024

View reviewed changes

tests/components/prowl/test_notification.py Outdated Show resolved Hide resolved

tests/components/prowl/conftest.py Outdated Show resolved Hide resolved

home-assistant bot marked this pull request as draft December 22, 2024 22:10

mgust added 2 commits February 6, 2025 22:06

Add tests for the Prowl component.

52806d3

Moved to use pyprowl rather than calling the API directly

126fdfe

MartinHjelmare changed the title ~~Add tests for the Prowl component.~~ Add tests for the Prowl integration Feb 22, 2025

mgust added 2 commits March 25, 2025 22:26

Moved the Prowl service to use ConfigFlow

32b55f5

Split out the legacy configuration to a separate class to handle the …

e226e17

…different base classes and added the reauth config flow

Merge remote-tracking branch 'upstream/dev' into prowl-tests

e74e23a

mgust force-pushed the prowl-tests branch from ec42ec0 to e74e23a Compare April 19, 2025 19:45

Changed the translations to use standardised ones

be40142

mgust changed the title ~~Add tests for the Prowl integration~~ Add ConfigFlow to Prowl integration Apr 19, 2025

Merge branch 'dev' into prowl-tests

200036b

mgust marked this pull request as ready for review April 19, 2025 22:15

home-assistant bot requested a review from frenck April 19, 2025 22:15

joostlek requested changes May 13, 2025

View reviewed changes

home-assistant bot marked this pull request as draft May 13, 2025 14:49

mgust added 2 commits May 16, 2025 01:03

Removed the migration to the new notify entity (to be left for a late…

9105bf7

…r PR)

Merge remote-tracking branch 'upstream/dev' into prowl-tests

529a0ba

mgust force-pushed the prowl-tests branch from 95cc8a8 to 529a0ba Compare May 16, 2025 00:04

mgust added 2 commits June 26, 2025 20:58

Revert "Removed the migration to the new notify entity (to be left fo…

db32198

…r a later PR)" This reverts commit 9105bf7.

Restored the old Prowl service to the state it was in (but renamed it…

f7eb1af

… to allow the new ProwlNotificationService to co-exist).

emontnemery reviewed Jun 30, 2025

View reviewed changes

NoRi2909 reviewed Jun 30, 2025

View reviewed changes

jbouwh requested changes Jun 30, 2025

View reviewed changes

Updated the name of the ProwlNotificationService, added a few minor d…

1d9ffc0

…ocstring tweaks, removed unneeded asyncio from pytest

	class ProwlNotificationService(NotifyEntity):
	class ProwlNotificationEntity(NotifyEntity):



		class ProwlNotificationService(NotifyEntity):
		"""Implement the notification service for Prowl."""

	"description": "Enter name of Prowl key and the API key.",
	"description": "Enter the Prowl API key and its name.",

	@callback
	def migrate_notify_issue(
	hass: HomeAssistant,
	domain: str,
	integration_title: str,
	breaks_in_ha_version: str,
	service_name: str \| None = None,
	) -> None:
	"""Ensure an issue is registered."""
	if service_name is not None:
	ir.async_create_issue(
	hass,
	DOMAIN,
	f"migrate_notify_{domain}_{service_name}",
	breaks_in_ha_version=breaks_in_ha_version,
	issue_domain=domain,
	is_fixable=True,
	is_persistent=True,
	translation_key="migrate_notify_service",
	translation_placeholders={
	"domain": domain,
	"integration_title": integration_title,
	"service_name": service_name,
	},
	severity=ir.IssueSeverity.WARNING,
	)
	return
	ir.async_create_issue(
	hass,
	DOMAIN,
	f"migrate_notify_{domain}",
	breaks_in_ha_version=breaks_in_ha_version,
	issue_domain=domain,
	is_fixable=True,
	is_persistent=True,
	translation_key="migrate_notify",
	translation_placeholders={
	"domain": domain,
	"integration_title": integration_title,
	},
	severity=ir.IssueSeverity.WARNING,
	)

		)


		@pytest.mark.asyncio

Uh oh!

Add ConfigFlow to Prowl integration #133771

Are you sure you want to change the base?

Add ConfigFlow to Prowl integration #133771

Uh oh!

Conversation

mgust commented Dec 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed change

Type of change

Additional information

Checklist

Uh oh!

home-assistant bot left a comment

Choose a reason for hiding this comment

Uh oh!

home-assistant bot commented Dec 22, 2024

Uh oh!

home-assistant bot left a comment

Choose a reason for hiding this comment

Uh oh!

frenck left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mgust commented Dec 23, 2024

Uh oh!

joostlek left a comment

Choose a reason for hiding this comment

Uh oh!

mgust commented May 15, 2025

Uh oh!

joostlek commented May 15, 2025

Uh oh!

mgust commented May 15, 2025

Uh oh!

mgust commented May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

emontnemery commented Jun 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jbouwh Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jbouwh commented Jun 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mgust commented Dec 22, 2024 •

edited

Loading

mgust commented May 16, 2025 •

edited

Loading

jbouwh Jun 30, 2025 •

edited

Loading