Skip to content

Commit 5f1a047

Browse files
committed
fix: address Copilot review feedback
- RetryManager offline short-circuit: add ErrorStore.hasAnyPanelOffline() so retries skip correctly under Favorites multi-panel watch (per-entity panel-offline keys), not just the legacy single-unnamed key. - FavoritesCache / MonitoringStatusCache: track inflight generation so invalidate() supersedes pending requests; fetch() after invalidate() issues a fresh call instead of awaiting a stale in-flight promise. - header-renderer buildSheddingLegendHTML: escape label / icon / color / textLabel via escapeHtml to match the escaping used elsewhere in the renderer and guard against stray markup in i18n strings. - CHANGELOG: correct 'Persisted per device' wording — list-columns storage is a single localStorage key, global to the browser.
1 parent ca3a460 commit 5f1a047

10 files changed

Lines changed: 113 additions & 26 deletions

CHANGELOG.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@
66

77
- **Compact expanded list rows** — Expanding a row in By Activity / By Area now shows only the chart. The gear icon and a real toggle-pill (arm-protected by the
88
slide-to-confirm) moved onto the always-visible list row so expanding no longer duplicates information above the chart.
9-
- **Configurable list view columns** — 1 / 2 / 3 column grid for By Activity and By Area, set in Graph Settings → List View Columns. Persisted per device via
10-
localStorage. Narrow viewports (< 600px) force single-column regardless. Expanded charts stay in their own column so row-to-chart association stays clear.
9+
- **Configurable list view columns** — 1 / 2 / 3 column grid for By Activity and By Area, set in Graph Settings → List View Columns. Persisted in localStorage
10+
as a browser-wide preference (single key, not scoped per device). Narrow viewports (< 600px) force single-column regardless. Expanded charts stay in their own
11+
column so row-to-chart association stays clear.
1112
- **Favorites per-panel status grid** — The Favorites view now renders a responsive grid of per-contributing-panel status cards (Site / Grid / Upstream /
1213
Downstream / Solar / Battery) below the slider + W/A row. One card per panel that contributes to the Favorites set; live values update on each tick.
1314
- **Slide-to-arm in Favorites** — Favorites view header now hosts the slide-confirm control so tappable ON/OFF toggles in list rows can actually fire. The

dist/span-panel-card.js

Lines changed: 5 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

dist/span-panel.js

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/core/error-store.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,20 @@ export class ErrorStore {
129129
return this._persistent.has(key);
130130
}
131131

132+
/**
133+
* True when any watched panel is currently marked offline. Covers both
134+
* the legacy single-unnamed key (``panel-offline``) used by per-panel
135+
* views and the per-entity keys (``panel-offline:<entityId>``) used by
136+
* the Favorites multi-panel watch. ``RetryManager`` uses this to
137+
* short-circuit retries without needing to know the naming mode.
138+
*/
139+
hasAnyPanelOffline(): boolean {
140+
for (const key of this._persistent.keys()) {
141+
if (key === "panel-offline" || key.startsWith("panel-offline:")) return true;
142+
}
143+
return false;
144+
}
145+
132146
/**
133147
* Subscribe to state changes. The callback is called after every `add`,
134148
* `remove`, `clear`, or transient auto-dismiss. Returns an unsubscribe fn.

src/core/favorites-store.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ export async function removeFavorite(hass: HomeAssistant, entityId: string): Pro
8383
export class FavoritesCache {
8484
private _map: FavoritesMap | null;
8585
private _lastFetch: number;
86-
private _inflight: Promise<FavoritesMap> | null;
86+
private _inflight: { gen: number; promise: Promise<FavoritesMap> } | null;
8787
private _generation: number;
8888
private _errorStore: ErrorStore | null = null;
8989
private _retry: RetryManager | null = null;
@@ -106,13 +106,16 @@ export class FavoritesCache {
106106

107107
async fetch(hass: HomeAssistant): Promise<FavoritesMap> {
108108
const now = Date.now();
109-
if (this._inflight) return this._inflight;
109+
// Only dedupe onto an in-flight request from the current generation.
110+
// Requests predating the last invalidate() must not be reused, or
111+
// the caller would await a stale promise whose result is dropped.
112+
if (this._inflight && this._inflight.gen === this._generation) return this._inflight.promise;
110113
if (this._map && now - this._lastFetch < FAVORITES_POLL_INTERVAL_MS) {
111114
return this._map;
112115
}
113116

114117
const requestGen = this._generation;
115-
this._inflight = (async () => {
118+
const promise = (async (): Promise<FavoritesMap> => {
116119
try {
117120
const msg = {
118121
type: "call_service",
@@ -145,10 +148,16 @@ export class FavoritesCache {
145148
}
146149
return this._map ?? {};
147150
} finally {
148-
this._inflight = null;
151+
// Only clear the slot if it still points at this request; a
152+
// later fetch() that ran after invalidate() may have replaced
153+
// it with a newer in-flight promise we must not clobber.
154+
if (this._inflight?.gen === requestGen) {
155+
this._inflight = null;
156+
}
149157
}
150158
})();
151-
return this._inflight;
159+
this._inflight = { gen: requestGen, promise };
160+
return promise;
152161
}
153162

154163
invalidate(): void {

src/core/header-renderer.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,20 @@ export function buildSheddingLegendHTML(): string {
2626
${Object.entries(SHEDDING_PRIORITIES)
2727
.filter(([key]: [string, SheddingPriorityDef]) => key !== "unknown")
2828
.map(([, cfg]: [string, SheddingPriorityDef]) => {
29+
const icon = escapeHtml(cfg.icon);
30+
const color = escapeHtml(cfg.color);
31+
const label = escapeHtml(cfg.label());
2932
let icons: string;
3033
if (cfg.icon2) {
31-
icons = `<ha-icon icon="${cfg.icon}" style="color:${cfg.color}"></ha-icon><ha-icon class="shedding-legend-secondary" icon="${cfg.icon2}" style="color:${cfg.color}"></ha-icon>`;
34+
const icon2 = escapeHtml(cfg.icon2);
35+
icons = `<ha-icon icon="${icon}" style="color:${color}"></ha-icon><ha-icon class="shedding-legend-secondary" icon="${icon2}" style="color:${color}"></ha-icon>`;
3236
} else if (cfg.textLabel) {
33-
icons = `<ha-icon icon="${cfg.icon}" style="color:${cfg.color}"></ha-icon><span class="shedding-legend-text" style="color:${cfg.color}">${cfg.textLabel}</span>`;
37+
const textLabel = escapeHtml(cfg.textLabel);
38+
icons = `<ha-icon icon="${icon}" style="color:${color}"></ha-icon><span class="shedding-legend-text" style="color:${color}">${textLabel}</span>`;
3439
} else {
35-
icons = `<ha-icon icon="${cfg.icon}" style="color:${cfg.color}"></ha-icon>`;
40+
icons = `<ha-icon icon="${icon}" style="color:${color}"></ha-icon>`;
3641
}
37-
return `<div class="shedding-legend-item">${icons}<span class="shedding-legend-label">${cfg.label()}</span></div>`;
42+
return `<div class="shedding-legend-item">${icons}<span class="shedding-legend-label">${label}</span></div>`;
3843
})
3944
.join("")}
4045
</div>`;

src/core/monitoring-status.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ interface CallServiceResponse {
1717
export class MonitoringStatusCache {
1818
private _status: MonitoringStatus | null = null;
1919
private _lastFetch: number = 0;
20-
private _inflight: Promise<MonitoringStatus | null> | null = null;
20+
private _inflight: { gen: number; promise: Promise<MonitoringStatus | null> } | null = null;
2121
private _generation: number = 0;
2222
private _errorStore: ErrorStore | null = null;
2323
private _retry: RetryManager | null = null;
@@ -41,13 +41,16 @@ export class MonitoringStatusCache {
4141
*/
4242
async fetch(hass: HomeAssistant, configEntryId?: string | null): Promise<MonitoringStatus | null> {
4343
const now = Date.now();
44-
if (this._inflight) return this._inflight;
44+
// Only dedupe onto an in-flight request from the current generation.
45+
// Requests predating the last invalidate() must not be reused, or the
46+
// caller would await a stale promise whose result is silently dropped.
47+
if (this._inflight && this._inflight.gen === this._generation) return this._inflight.promise;
4548
if (this._status && now - this._lastFetch < MONITORING_POLL_INTERVAL_MS) {
4649
return this._status;
4750
}
4851

4952
const requestGen = this._generation;
50-
this._inflight = (async () => {
53+
const promise = (async (): Promise<MonitoringStatus | null> => {
5154
try {
5255
const serviceData: Record<string, string> = {};
5356
if (configEntryId) serviceData.config_entry_id = configEntryId;
@@ -85,10 +88,16 @@ export class MonitoringStatusCache {
8588
}
8689
return null;
8790
} finally {
88-
this._inflight = null;
91+
// Only clear the slot if it still points at this request; a
92+
// later fetch() that ran after invalidate() may have replaced
93+
// it with a newer in-flight promise we must not clobber.
94+
if (this._inflight?.gen === requestGen) {
95+
this._inflight = null;
96+
}
8997
}
9098
})();
91-
return this._inflight;
99+
this._inflight = { gen: requestGen, promise };
100+
return promise;
92101
}
93102

94103
/** Force the next fetch() call to re-query the backend. */

src/core/retry-manager.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,11 @@ export class RetryManager {
3838
}
3939

4040
private async _withRetry<T>(fn: () => Promise<T>, maxRetries: number, errorId: string, errorMessage?: string): Promise<T> {
41-
// Short-circuit if panel is offline — single attempt, no retries
42-
if (this._store.hasPersistent("panel-offline")) {
41+
// Short-circuit if any watched panel is offline — single attempt,
42+
// no retries. Uses ``hasAnyPanelOffline`` so it covers both the
43+
// legacy single-unnamed key and the per-entity keys used by the
44+
// Favorites multi-panel watch.
45+
if (this._store.hasAnyPanelOffline()) {
4346
try {
4447
const result = await fn();
4548
this._store.remove(errorId);

tests/favorites-cache.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,39 @@ describe("FavoritesCache", () => {
5858
expect(second).toEqual({ panelA: { circuits: ["c1"], sub_devices: [] } });
5959
});
6060

61+
it("fetch() after invalidate() issues a fresh request even while an earlier fetch is pending", async () => {
62+
// Resolve manually so we can control the ordering: pre-toggle response
63+
// stays pending until we release it.
64+
let resolveFirst!: (resp: { response: { favorites: Record<string, { circuits: string[]; sub_devices: string[] }> } }) => void;
65+
const firstPromise = new Promise<{ response: { favorites: Record<string, { circuits: string[]; sub_devices: string[] }> } }>(resolve => {
66+
resolveFirst = resolve;
67+
});
68+
const secondResponse = { response: { favorites: { panelA: { circuits: ["c1", "c2"], sub_devices: [] } } } };
69+
70+
let i = 0;
71+
const callWS = vi.fn(async () => {
72+
const idx = i;
73+
i += 1;
74+
return idx === 0 ? firstPromise : secondResponse;
75+
});
76+
const hass = { states: {}, services: {}, language: "en", callWS } as unknown as HomeAssistant;
77+
78+
const first = cache.fetch(hass);
79+
cache.invalidate();
80+
81+
// Second fetch arrives while first is still in flight but after
82+
// invalidate(). It must not dedupe onto the stale request; it must
83+
// issue a new backend call and return the post-invalidate data.
84+
const second = cache.fetch(hass);
85+
expect(callWS).toHaveBeenCalledTimes(2);
86+
87+
resolveFirst({ response: { favorites: { panelA: { circuits: ["c1"], sub_devices: [] } } } });
88+
89+
const [firstResult, secondResult] = await Promise.all([first, second]);
90+
expect(firstResult).toEqual({ panelA: { circuits: ["c1"], sub_devices: [] } });
91+
expect(secondResult).toEqual({ panelA: { circuits: ["c1", "c2"], sub_devices: [] } });
92+
});
93+
6194
it("clear() drops the cached map and bumps generation", async () => {
6295
const { hass } = makeHass([{ panelA: { circuits: ["c1"], sub_devices: [] } }, { panelA: { circuits: [], sub_devices: [] } }]);
6396
await cache.fetch(hass);

tests/retry-manager.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,4 +225,17 @@ describe("RetryManager — panel offline short-circuit", () => {
225225
// No transient error should be added since fn() succeeded
226226
expect(store.active.filter(e => !e.persistent)).toHaveLength(0);
227227
});
228+
229+
it("short-circuits on per-entity panel-offline keys (Favorites multi-panel watch)", async () => {
230+
const hass = makeHass();
231+
// Simulate the Favorites-mode watch: per-entity key, not the legacy unnamed key.
232+
store.add({ key: "panel-offline:binary_sensor.span_panel_2_panel_on", level: "error", message: "Panel 2 unreachable", persistent: true });
233+
234+
vi.mocked(hass.callWS).mockRejectedValueOnce(new Error("offline"));
235+
236+
await expect(manager.callWS(hass, { type: "span/get_panel" }, { errorId: "test-offline-ws-named" })).rejects.toThrow("offline");
237+
238+
// Only 1 attempt — no retries, despite the key being per-entity rather than the legacy name.
239+
expect(hass.callWS).toHaveBeenCalledTimes(1);
240+
});
228241
});

0 commit comments

Comments
 (0)