Add support for Overseerr Issues feature #137969
Conversation
|
Hey there @joostlek, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
…w, and since there are two events we cannot remove the 'notification_type'
MartinHjelmare
changed the title
| @@ -18,9 +18,9 @@ async def async_get_config_entry_diagnostics( | |||
|
|
|||
| has_cloudhooks = CONF_CLOUDHOOK_URL in entry.data | |||
|
|
|||
| data = entry.runtime_data | |||
| data: OverseerrData = entry.runtime_data.data | |||
There was a problem hiding this comment.
| data: OverseerrData = entry.runtime_data.data | |
| data = entry.runtime_data.data |
Shouldn't be needed
There was a problem hiding this comment.
It's nice to have things typed, but I agree not useful here.
There was a problem hiding this comment.
Oh but the catch is that the entry already is typed, so we're able to find out what the type of entry.runtime_data.data is without explicitly defining it :)
| translation_key="last_issue_event", | ||
| event_types=[ | ||
| "reported", | ||
| "comment", |
There was a problem hiding this comment.
| "comment", | |
| "commented", |
I kinda like the idea of having the states answer the question "what happened?" "somebody ". But I can also see that it makes the code uglier, so I am fine with it as it is, just wanted to write down my thought
There was a problem hiding this comment.
I agree, this change will require a change in the upstream library aswell
There was a problem hiding this comment.
I just did some tests and the webhook returns the following:
{'notification_type': 'ISSUE_COMMENT', 'subject': "xxx", 'message': 'Ended early :(', 'media': {'media_type': 'tv', 'tmdb_id': xxx, 'tvdb_id': xxx, 'status': 'partially_available', 'status4k': 'unknown'}, 'request': None, 'issue': {'issue_id': 12, 'issue_type': 'VIDEO', 'issue_status': 'OPEN', 'reported_by_email': 'xxx', 'reported_by_username': 'xxx', 'reported_by_avatar': 'xxx', 'reported_by_settings_discord_id': '', 'reported_by_settings_telegram_chat_id': ''}}
So unfortunately we need to use comment :(
There was a problem hiding this comment.
Hmm, but in theory we can use a dict to map between the 2 values
There was a problem hiding this comment.
something like this? (last commit)
| @@ -108,7 +119,6 @@ def available(self) -> bool: | |||
|
|
|||
| def parse_event(event: dict[str, Any], nullable_fields: list[str]) -> dict[str, Any]: | |||
| """Parse event.""" | |||
| event.pop("notification_type") | |||
There was a problem hiding this comment.
I thought I removed this one as it is duplicate with the reported event
There was a problem hiding this comment.
I think it would also be nice to add a thing in test_event's test_entities that we already posted an issue update, so we can see the full state of the entity when it happened
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
|
I think that solves it, but I think we can just make sure we only do a |
I'm not sure where that would go, I've never used a call like that before. |
Proposed change
Adding support for Overseerr Issues
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: