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

Correct calls to subscription.async_unsubscribe_topics #19414

Merged
merged 3 commits into from
Dec 19, 2018

Conversation

emontnemery
Copy link
Contributor

@emontnemery emontnemery commented Dec 17, 2018

Description:

Fix subscription.async_unsubscribe_topics to return the new (empty) subscription state
Also, when calling subscription.async_unsubscribe_topics, update the entity's subscription state

Background:

When Entity registry is called, async_registry_updated(self, old, new) in helpers/entity.py is called.
If entity_id is changed, this will first remove, and then again add, the entity.

Related issue (if applicable): fixes #19352

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.

@emontnemery emontnemery requested a review from a team as a code owner December 17, 2018 20:01
@ghost ghost assigned emontnemery Dec 17, 2018
@ghost ghost added the in progress label Dec 17, 2018
@emontnemery emontnemery added this to the 0.84.3 milestone Dec 17, 2018
@emontnemery emontnemery modified the milestones: 0.84.3, 0.84.4 Dec 17, 2018
@MartinHjelmare
Copy link
Member

We should add tests.

@OttoWinter OttoWinter mentioned this pull request Dec 18, 2018
3 tasks
@balloob
Copy link
Member

balloob commented Dec 18, 2018

Yeah this should have been caught by tests. Please include a test to test this.

@emontnemery emontnemery force-pushed the fix_async_unsubscribe_topics branch from 9d5d20f to 6d22d43 Compare December 18, 2018 21:23
@home-assistant home-assistant deleted a comment from houndci-bot Dec 18, 2018
@home-assistant home-assistant deleted a comment from houndci-bot Dec 18, 2018
@home-assistant home-assistant deleted a comment from houndci-bot Dec 18, 2018
@home-assistant home-assistant deleted a comment from houndci-bot Dec 18, 2018
@home-assistant home-assistant deleted a comment from houndci-bot Dec 18, 2018
@home-assistant home-assistant deleted a comment from houndci-bot Dec 18, 2018
@home-assistant home-assistant deleted a comment from houndci-bot Dec 18, 2018
@home-assistant home-assistant deleted a comment from houndci-bot Dec 18, 2018
@home-assistant home-assistant deleted a comment from houndci-bot Dec 18, 2018
@home-assistant home-assistant deleted a comment from houndci-bot Dec 18, 2018
@home-assistant home-assistant deleted a comment from houndci-bot Dec 18, 2018
@home-assistant home-assistant deleted a comment from houndci-bot Dec 18, 2018
@home-assistant home-assistant deleted a comment from houndci-bot Dec 18, 2018
@home-assistant home-assistant deleted a comment from houndci-bot Dec 18, 2018
@home-assistant home-assistant deleted a comment from houndci-bot Dec 18, 2018
@home-assistant home-assistant deleted a comment from houndci-bot Dec 18, 2018
@emontnemery
Copy link
Contributor Author

Test cases added.
(And I messed up a bit when pushing the testcases, Hound was not impressed)

@emontnemery emontnemery force-pushed the fix_async_unsubscribe_topics branch from 6d22d43 to 5aa5e1b Compare December 19, 2018 11:17
@balloob balloob merged commit 1568de6 into home-assistant:dev Dec 19, 2018
@ghost ghost removed the in progress label Dec 19, 2018
@balloob balloob removed this from the 0.84.4 milestone Dec 19, 2018
@balloob
Copy link
Member

balloob commented Dec 19, 2018

Cleared milestone as it does not cleanly apply to master

@emontnemery
Copy link
Contributor Author

OK. Should I open a PR which does apply to master, or this can wait for 0.85?

@balloob
Copy link
Member

balloob commented Dec 19, 2018

Let's wait for 85, I can't do another hot fix or I'll go crazy.

@emontnemery emontnemery deleted the fix_async_unsubscribe_topics branch December 19, 2018 18:59
dshokouhi pushed a commit to dshokouhi/home-assistant that referenced this pull request Dec 25, 2018
…t#19414)

* Correct calls to subscription.async_unsubscribe_topics

* Review comments

* Add testcases
@balloob balloob mentioned this pull request Jan 10, 2019
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.

Impossible to turn off mqtt light after changing entity name in frontend
5 participants