Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support shorthand logical operators in script sequences #71022

Merged
merged 1 commit into from
Apr 29, 2022
Merged

Conversation

frenck
Copy link
Member

@frenck frenck commented Apr 28, 2022

Proposed change

Support shorthand logical conditions support in script sequences/automation actions.

Frontend PR: home-assistant/frontend#12509

closes #70947

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)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

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
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

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

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.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

@@ -1599,7 +1599,7 @@ def determine_script_action(action: dict[str, Any]) -> str:
if CONF_WAIT_TEMPLATE in action:
return SCRIPT_ACTION_WAIT_TEMPLATE

if CONF_CONDITION in action:
if any(key in action for key in (CONF_CONDITION, "and", "or", "not")):
Copy link
Member

Choose a reason for hiding this comment

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

This means that none of the schemas below can have and, or, not as config parameters or else it will be marked as a condition.

Copy link
Member Author

@frenck frenck Apr 28, 2022

Choose a reason for hiding this comment

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

Yes 🤷

As they are logical operators already present in conditions now, I think that is OK. There are no current conflicts and worst case, if needed, the logic determining this can be extended/refined in the future to support cases otherwise (e.g., by changing detection order).

@thomasloven
Copy link
Contributor

This also needs special consideration in the frontend. Automation editor and trace graphs.

See home-assistant/frontend#12473

Copy link
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

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

This needs a frontend PR. CC @thomasloven

@thomasloven
Copy link
Contributor

I'll try to take a look at it tomorrow.

@frenck
Copy link
Member Author

frenck commented Apr 29, 2022

@thomasloven

- id: '1649679845278'
  alias: UI Shorthand tests
  description: ''
  trigger:
  - platform: event
    event_type: test
  condition: []
  action:
    or: 
     - and:
        not: '{{ False }}'
  mode: single

CleanShot 2022-04-29 at 10 36 42

@Mariusthvdb
Copy link
Contributor

just let me add this, as said in Dm: thanks to the team for fixing this! so appreciated.

@frenck frenck merged commit 682ac52 into dev Apr 29, 2022
@frenck frenck deleted the frenck-2022-0963 branch April 29, 2022 16:06
@github-actions github-actions bot locked and limited conversation to collaborators Apr 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

shorthand condition in action block not valid?
5 participants