Skip to content

Conversation

@iMicknl
Copy link
Owner

@iMicknl iMicknl commented Dec 29, 2025

Fixes #1860

Fixes wrong typing in Execution (see https://github.com/home-assistant/core/blob/ee6886dabde435cd82e69dd6cbefa3dbb15d5122/homeassistant/components/overkiz/executor.py#L129-L130).

Breaking changes

  • client.get_scenarios() is replaced by client.get_action_groups()
  • Scenario() model is replaced by ActionGroup(), where creation_timeandmetadata` are now optional fields

Fixes

  • client.get_current_executions() is now properly typed, previously the Execution() model returned a list type for action_group, what should be a dict type.

@iMicknl iMicknl changed the base branch from main to v2/main December 29, 2025 11:33
@iMicknl iMicknl added this to the 2.0 milestone Dec 29, 2025
@iMicknl iMicknl marked this pull request as ready for review December 29, 2025 20:26
@iMicknl iMicknl requested a review from tetienne as a code owner December 29, 2025 20:26
Copilot AI review requested due to automatic review settings December 29, 2025 20:26
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR renames the Scenario class to ActionGroup to better reflect its purpose and fixes incorrect typing in the Execution class. The Execution.action_group field is changed from list[dict[str, Any]] to a properly typed ActionGroup object, addressing issue #1860.

Key changes:

  • Renamed Scenario class to ActionGroup with updated field optionality (creation_time, metadata, oid/id)
  • Updated Execution.action_group type from list[dict[str, Any]] to ActionGroup with proper initialization
  • Renamed client.get_scenarios() method to client.get_action_groups()

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
pyoverkiz/models.py Renamed Scenario to ActionGroup, made creation_time and metadata optional, added dual oid/id support, and updated Execution to use typed ActionGroup instead of dict
pyoverkiz/client.py Renamed get_scenarios() method to get_action_groups() and updated imports from Scenario to ActionGroup
tests/test_client.py Updated test method name from test_get_scenarios to test_get_action_groups and renamed test variables accordingly
tests/fixtures/exec/current-tahoma-switch.json Added new test fixture showing an Execution response with an embedded actionGroup object
Comments suppressed due to low confidence (1)

pyoverkiz/models.py:628

  • The field declarations for id and oid are marked as required (no default values), but the __init__ method allows both to be None initially (lines 635-636). This creates a type inconsistency. Additionally, based on the test fixture in tests/fixtures/exec/current-tahoma-switch.json, the actionGroup embedded in an Execution response doesn't include an id or oid field, which will cause the initialization to fail with the ValueError at line 648 when ActionGroup(**action_group) is called from Execution.__init__ (line 593).

Consider making both id and oid optional in the field declarations to match the actual API responses, or adjust the logic to handle cases where neither field is present in the input data.

    id: str = field(repr=obfuscate_id)
    creation_time: int | None = None
    last_update_time: int | None = None
    label: str = field(repr=obfuscate_string)
    metadata: str | None = None
    shortcut: bool | None = None
    notification_type_mask: int | None = None
    notification_condition: str | None = None
    notification_text: str | None = None
    notification_title: str | None = None
    actions: list[Action]
    oid: str = field(repr=obfuscate_id)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@iMicknl iMicknl merged commit 348d1c2 into v2/main Dec 29, 2025
6 checks passed
@iMicknl iMicknl deleted the v2/action_groups branch December 29, 2025 20:39
@iMicknl iMicknl mentioned this pull request Dec 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Change Scenario to ActionGroup

2 participants