Refactor logbook data fetch logic into reusable class

[balloob]

Breaking change

Proposed change

The logbook fetch data logic was spread over 3 different places (panel, more info, card). This puts it all in 1 re-usable class.

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New feature (thank you!)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Example configuration

Additional information

• This PR fixes or closes issue: fixes #
• This PR is related to issue or discussion:
• Link to documentation pull request:

Checklist

• The code change is tested and works locally.
• There is no commented out code in this PR.
• Tests have been added to verify that the new code works.

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

• Documentation added/updated for www.home-assistant.io

[bdraco]

Also when I create 3 logbook cards its calling config/auth/list 3x every time I load the tab

[bdraco]

Here is the yaml I'm using for the logbook card testing

  - theme: Backend-selected
    title: Logbook Card Test
    path: logbook-card-test
    badges: []
    cards:
      - type: logbook
        entities:
          - light.master_hall_lights_g
          - light.kitchen_recessed_lights_g
          - light.upstairs_hall_large_lights_g
          - automation.elk_system_has_motion
      - type: logbook
        entities:
          - cover.driveway_gate_local
          - cover.large_garage_door_local
          - cover.small_garage_door_local
      - type: logbook
        entities:
          - cover.entry_front_door_shade
          - automation.front_door_doorbell

[bdraco]

I think the config/auth/list being called every time is an existing issue. Here is what gets fired off when I load a more-info

[bdraco]

console error was caused by bad testing on my part - the cache not being fully cleared.

The duplicate config/auth/list calls are definitely there though

[bdraco]

The users being fetched every time isn't a bug, its just polling because there is no listener for to know when the users change. We can't add one because we need to know when they are updated as well.

home-assistant/core#71965

[bdraco]

Getting that same trace on the related logbook entries under traces. Tried a different browser as well to make sure it wasn't a cache issue

Uncaught (in promise) TypeError: Cannot use 'in' operator to search for 'recent' in undefined
    at HaLogbook.updated (ha-logbook.ts?d9f8:125:1)
    at HaLogbook._$didUpdate (reactive-element.ts?9d98:1247:1)
    at HaLogbook.performUpdate (reactive-element.ts?9d98:1230:1)
    at HaLogbook.scheduleUpdate (reactive-element.ts?9d98:1149:1)
    at HaLogbook.__enqueueUpdate (reactive-element.ts?9d98:1121:1)

[bdraco]

The traces just needed -renderer bdraco@1a6085c

[bdraco]

Selecting a single entity isn't working.
Navigating to /logbook?start_date=2022-05-01T18%3A00%3A00.000Z&end_date=2022-05-25T21%3A00%3A00.000Z&entity_id=select.addressable_v3_5v_588eb7_ic_type also doesn't pickup the entity. You get all the results

[bdraco]

Refreshing isn't working.

I think it needs this.shadowRoot?.querySelector("ha-logbook")?.refresh(); but even then its going to be throttled.

I guess it never makes sense to allow refresh if the previous call's endRange was less than 60s before now at the time it was refreshed since there will never be any new results.

[bdraco]

I wonder if it would reduce the latency by any perceptible amount if the get_events call fired first and then trace/contexts and then config/auth/list

[balloob]

Thanks for testng. Pushed a bunch of bug fixes. I have not added caching of requests for users/contexts yet when multiple cards are visible. I did add a 60s throttle so they are only checked every 60 seconds on a per-card basis.

I tried creating the get_events promise first before refreshing users/context, but fetching events does a bunch of extra work checking for caching so it wasn't firing it prior to the others, so reverted that change.

This is only the first pass. I think that we can spend future PRs in optimizing the fetching of data, including using https://github.com/home-assistant/frontend/blob/refactor-logbook/src/common/util/time-cache-function-promise.ts#L12

[bdraco]

Pulling and doing another test cycle

[bdraco]

Existing issue: If I pop up the more info card from the main logbook page and click Show More on a specific entity the url changes but it doesn't refresh

[bdraco]

New issue: When clicking refresh, the spinner no longer appears, and the button doesn't grey out. You can quickly drive the load up on the instance by clicking it multiple times thinking its not working.

Original behavior

New behavior

[bdraco]

Existing issue: Not sure how to resolve this one, but if you have hit show more and have selected a specific entity for all of recorded history, and then dismiss that entity you can end up selecting the entire events and states tables

[bdraco]

Not sure if its just perception but the data parsing on the frontend seems snappier 👍

[bdraco]

Existing issue: the drop down will let me select entities that will never have logbook history such as sensors with UOM or entities with a state class

[bdraco]

I beat it up the best I could. That was all I found

[bdraco]

New issue:
Loading the logbook from the left sidebar icon loads config/auth/list and trace/contexts twice.

[balloob]

Fixed more bugs:

• ✅ Show more links from more info dialog now update logbook page
• ✅ Spinner now appears when hitting refresh.
  • ⚠️ I did not grey out the refresh button as that state is currently not coupled. I did make it that the refresh button has no effect when a fetch is in progress.
• ❌ Long time span will still be loaded when entity filter removed
• ✅ Entity filter now filters out sensors with either a UoM or a state class
• ✅ Opening logbook from sidebar no longer loads auth + context twice

[bdraco]

Retesting now..

[bdraco]

• ✅ Show more links from more info dialog now update logbook page

Confirmed

• ✅ Spinner now appears when hitting refresh.

Confirmed

• ⚠️ I did not grey out the refresh button as that state is currently not coupled. I did make it that the refresh button has no effect when a fetch is in progress.

Agree this is better

• ❌ Long time span will still be loaded when entity filter removed

Still haven't come up with a good answer on what to do here

• ✅ Entity filter now filters out sensors with either a UoM or a state class

Confirmed. So nice.

• ✅ Opening logbook from sidebar no longer loads auth + context twice

Confirmed

[bdraco]

Did manage to get a weird error Could not load logbook: entries.reverse is not a function but cannot repro now

[bdraco]

/logbook?start_date=2022-05-18T04%3A00%3A00.000Z&end_date=2022-05-18T07%3A00%3A00.000Z

This url repos it

[bdraco]

Oh, I think its just selecting a time window in the future

[bdraco]

Yup logbook?start_date=2022-05-30T04%3A00%3A00.000Z&end_date=2022-05-31T07%3A00%3A00.000Z and it happens as well

[bdraco]

diff --git a/src/data/logbook.ts b/src/data/logbook.ts
index 9e5f2ac4a..d492d767d 100644
--- a/src/data/logbook.ts
+++ b/src/data/logbook.ts
@@ -118,7 +118,7 @@ export const getLogbookDataCache = async (
     startDate,
     endDate,
     entityId !== ALL_ENTITIES ? entityId : undefined
-  ).then((entries) => entries.reverse());
+  ).then((entries) => entries.length ? entries.reverse() : []);
   return DATA_CACHE[cacheKey][entityId];
 };
 

[balloob]

That issue is caused by a bug in the backend. Fix here home-assistant/core#72021

[bdraco]

That issue is caused by a bug in the backend. Fix here home-assistant/core#72021

Good catch 👍