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

Return history for entities in the order they were requested #25560

Merged
merged 3 commits into from
Jul 31, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions homeassistant/components/history/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Provide pre-made queries on top of the recorder component."""
from collections import defaultdict
from collections.abc import Iterable
from datetime import timedelta
from itertools import groupby
import logging
Expand Down Expand Up @@ -211,6 +212,9 @@ def states_to_json(
axis correctly.
"""
result = defaultdict(list)
if isinstance(entity_ids, Iterable):
Copy link
Member

Choose a reason for hiding this comment

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

This is not a right check, a string is iterable too.

Copy link
Member

Choose a reason for hiding this comment

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

I guess it works because we don't pass a list. We should just guard against it being None

Copy link
Contributor Author

Choose a reason for hiding this comment

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

*Gasp! Stack overflow lied to me!

Thanks for fixing.
Will entity_ids NEVER be a single entity_id in a string?

Copy link
Member

Choose a reason for hiding this comment

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

based on the code, no.

for ent_id in entity_ids:
result[ent_id] = []

# Get the states at the start time
timer_start = time.perf_counter()
Expand Down
16 changes: 16 additions & 0 deletions tests/components/history/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,22 @@ def test_get_significant_states_include_exclude(self):
history.CONF_ENTITIES: ['media_player.test']}}})
self.check_significant_states(zero, four, states, config)

def test_get_significant_states_are_ordered(self):
"""Test order of results from get_significant_states

When entity ids are given, the results should be returned with the data
in the same order.
"""
zero, four, states = self.record_states()
entity_ids = ['media_player.test', 'media_player.test2']
hist = history.get_significant_states(
self.hass, zero, four, entity_ids, filters=history.Filters())
assert list(hist.keys()) == entity_ids
entity_ids = ['media_player.test2', 'media_player.test']
hist = history.get_significant_states(
self.hass, zero, four, entity_ids, filters=history.Filters())
assert list(hist.keys()) == entity_ids

def check_significant_states(self, zero, four, states, config):
"""Check if significant states are retrieved."""
filters = history.Filters()
Expand Down