Skip to content

Do not select all entities when omitting entity ID in service call#29178

Merged
balloob merged 5 commits into
devfrom
entity-id-omit-no-select-all
Dec 3, 2019
Merged

Do not select all entities when omitting entity ID in service call#29178
balloob merged 5 commits into
devfrom
entity-id-omit-no-select-all

Conversation

@balloob
Copy link
Copy Markdown
Member

@balloob balloob commented Nov 28, 2019

Breaking Change:

It is no longer possible to target all entities by not passing both entity_id and area_id to your service data. This feature was previously deprecated and has been printing warnings. The correct way to target all entities is to set entity_id: all

Description:

Remove the feature to automatically target all entities if you did not specify an entity.

Previously attempted in #25715.

entity_id has not been set to required because it is allowed to just specify area_id to select entities.

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@balloob balloob requested a review from a team as a code owner November 28, 2019 06:08
@probot-home-assistant probot-home-assistant Bot added core has-tests small-pr PRs with less than 30 lines. labels Nov 28, 2019
Copy link
Copy Markdown
Contributor

@amelchio amelchio left a comment

Choose a reason for hiding this comment

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

entity_id has not been set to required because it is allowed to just specify area_id to select entities.

So we now fail silently?

It would be quite useful to have the constraint (need one or the other) encoded in the schemas.

if ATTR_ENTITY_ID in call.data:
target_all_entities = call.data[ATTR_ENTITY_ID] == ENTITY_MATCH_ALL
else:
# Remove the service_name parameter along with this warning
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This comments says that further changes are needed if it is deleted :)

service_name,
ENTITY_MATCH_ALL,
)
target_all_entities = True
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should remove the auto target and convert the warning into error and fail on this service call

Comment thread tests/helpers/test_entity_component.py
Comment thread tests/helpers/test_service.py
@balloob balloob force-pushed the entity-id-omit-no-select-all branch from 9f68c31 to 73a6d52 Compare November 30, 2019 22:53
@balloob
Copy link
Copy Markdown
Member Author

balloob commented Nov 30, 2019

I've updated the code to always require either entity_id or area_id. I did some clean up in the process. The change was made by making sure that ENTITY_SERVICE_SCHEMA has been removed and replaced with a function that will extend the passed in schema, but also enforced either key is passed in.

@balloob balloob changed the title Do not select all entities when omitting entity ID Do not select all entities when omitting entity ID in service call Dec 1, 2019
@home-assistant home-assistant deleted a comment Dec 2, 2019
@home-assistant home-assistant deleted a comment Dec 2, 2019
@balloob balloob merged commit 02d9ed5 into dev Dec 3, 2019
@delete-merged-branch delete-merged-branch Bot deleted the entity-id-omit-no-select-all branch December 3, 2019 00:23
@lock lock Bot locked and limited conversation to collaborators Dec 4, 2019
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.

5 participants