fix: Don't run the query string actions until after initialization (#1579)

This commit is contained in:
Dermot Duffy
2024-09-24 17:56:43 -07:00
committed by GitHub
parent 1e0a990855
commit 06345ca01d
7 changed files with 84 additions and 126 deletions
+13 -7
View File
@@ -2,9 +2,9 @@ import { LitElement, ReactiveControllerHost } from 'lit';
import { ActionEventTarget } from '../action-handler-directive';
import { setOrRemoveAttribute } from '../utils/basic';
import { isCardInPanel } from '../utils/ha';
import { ActionExecutionRequestEventTarget } from './actions/utils/execution-request';
import { InitializationAspect } from './initialization-manager';
import { CardElementAPI } from './types';
import { ActionExecutionRequestEventTarget } from './actions/utils/execution-request';
export type ScrollCallback = () => void;
export type MenuToggleCallback = () => void;
@@ -120,16 +120,19 @@ export class CardElementManager {
// See: https://github.com/home-assistant/frontend/blob/273992c8e9c3062c6e49481b6d7d688a07067232/src/common/navigate.ts#L43
window.addEventListener(
'location-changed',
this._api.getQueryStringManager().executeAll,
this._api.getQueryStringManager().requestExecution,
);
// Listen for history state changes (i.e. user using the browser
// back/forward controls).
window.addEventListener('popstate', this._api.getQueryStringManager().executeAll);
window.addEventListener(
'popstate',
this._api.getQueryStringManager().requestExecution,
);
// Manually call the location change handler as the card will be
// Manually request query string execute as the card will be
// disconnected/reconnected when dashboard 'tab' changes happen within HA.
this._api.getQueryStringManager().executeAll();
this._api.getQueryStringManager().requestExecution();
// Make sure reconnections call the initialization code.
this._element.requestUpdate();
@@ -180,8 +183,11 @@ export class CardElementManager {
window.removeEventListener(
'location-changed',
this._api.getQueryStringManager().executeAll,
this._api.getQueryStringManager().requestExecution,
);
window.removeEventListener(
'popstate',
this._api.getQueryStringManager().requestExecution,
);
window.removeEventListener('popstate', this._api.getQueryStringManager().executeAll);
}
}
+14 -20
View File
@@ -14,27 +14,27 @@ interface QueryStringViewIntent {
export class QueryStringManager {
protected _api: CardQueryStringAPI;
protected _shouldRun = true;
constructor(api: CardQueryStringAPI) {
this._api = api;
}
public hasViewRelatedActions(): boolean {
return !!this._calculateIntent().view;
public hasViewRelatedActionsToRun(): boolean {
return !!this._calculateIntent().view && this._shouldRun;
}
public executeNonViewRelated = async (): Promise<void> => {
await this._executeNonViewRelated(this._calculateIntent());
public requestExecution = (): void => {
this._shouldRun = true;
this._api.getCardElementManager().update();
};
public executeViewRelated = async (): Promise<void> => {
await this._executeViewRelated(this._calculateIntent());
};
public executeAll = async (): Promise<void> => {
const intent = this._calculateIntent();
await this._executeViewRelated(intent);
await this._executeNonViewRelated(intent);
public executeIfNecessary = async (): Promise<void> => {
if (this._shouldRun) {
this._shouldRun = false;
await this._executeViewRelated(this._calculateIntent());
await this._executeNonViewRelated(this._calculateIntent());
}
};
protected async _executeViewRelated(intent: QueryStringViewIntent): Promise<void> {
@@ -63,15 +63,9 @@ export class QueryStringManager {
}
protected async _executeNonViewRelated(intent: QueryStringViewIntent): Promise<void> {
if (
// Only execute non-view actions when the card has rendered at least once.
!this._api.getCardElementManager().hasUpdated() ||
!intent.other?.length
) {
return;
if (intent.other) {
await this._api.getActionsManager().executeActions(intent.other);
}
await this._api.getActionsManager().executeActions(intent.other);
}
protected _calculateIntent(): QueryStringViewIntent {
+5 -12
View File
@@ -134,18 +134,11 @@ export class ViewManager implements ViewManagerInterface {
}
public initialize = async (): Promise<boolean> => {
// Set a view on initial load. However, if the query string contains a view
// related action, we don't set any view here and allow that content to be
// triggered by the firstUpdated() call that runs query string actions. To
// do otherwise may cause a race condition between the default view and the
// querystring view, see:
// https://github.com/dermotduffy/frigate-hass-card/issues/1200
const hasViewRelatedActions = this._api
.getQueryStringManager()
.hasViewRelatedActions();
if (hasViewRelatedActions) {
await this._api.getQueryStringManager().executeViewRelated();
} else {
// If the query string contains a view related action, we don't set any view
// here and allow that action to be triggered by the next call of to execute
// query actions (called at least once per render cycle).
// Related: https://github.com/dermotduffy/frigate-hass-card/issues/1200
if (!this._api.getQueryStringManager().hasViewRelatedActionsToRun()) {
await this.setViewDefaultWithNewQuery({ failSafe: true });
}
return true;
+4 -3
View File
@@ -302,9 +302,10 @@ class FrigateCard extends LitElement {
`;
}
protected firstUpdated(): void {
// Execute query string actions after first render is complete.
this._controller.getQueryStringManager().executeNonViewRelated();
protected updated(): void {
if (this._controller.getInitializationManager().isInitializedMandatory()) {
this._controller.getQueryStringManager().executeIfNecessary();
}
}
protected _renderInDialogIfNecessary(contents: TemplateResult): TemplateResult | void {
@@ -71,7 +71,7 @@ describe('InitializationManager', () => {
vi.mocked(api.getHASSManager().getHASS).mockReturnValue(createHASS());
vi.mocked(api.getConfigManager().getConfig).mockReturnValue(createConfig());
vi.mocked(api.getMessageManager().hasMessage).mockReturnValue(false);
vi.mocked(api.getQueryStringManager().hasViewRelatedActions).mockReturnValue(
vi.mocked(api.getQueryStringManager().hasViewRelatedActionsToRun).mockReturnValue(
false,
);
vi.mocked(loadLanguages).mockResolvedValue(true);
@@ -120,7 +120,7 @@ describe('InitializationManager', () => {
vi.mocked(api.getHASSManager().getHASS).mockReturnValue(createHASS());
vi.mocked(api.getConfigManager().getConfig).mockReturnValue(createConfig());
vi.mocked(api.getMessageManager().hasMessage).mockReturnValue(true);
vi.mocked(api.getQueryStringManager().hasViewRelatedActions).mockReturnValue(
vi.mocked(api.getQueryStringManager().hasViewRelatedActionsToRun).mockReturnValue(
false,
);
vi.mocked(loadLanguages).mockResolvedValue(true);
@@ -22,9 +22,9 @@ describe('QueryStringManager', () => {
vi.mocked(api.getMessageManager().hasMessage).mockReturnValue(true);
const manager = new QueryStringManager(api);
await manager.executeAll();
expect(manager.hasViewRelatedActionsToRun()).toBeFalsy();
await manager.executeIfNecessary();
expect(manager.hasViewRelatedActions()).toBeFalsy();
expect(api.getActionsManager().executeActions).not.toBeCalled();
expect(api.getViewManager().setViewByParameters).not.toBeCalled();
});
@@ -49,9 +49,10 @@ describe('QueryStringManager', () => {
vi.mocked(api.getCardElementManager().hasUpdated).mockReturnValue(false);
const manager = new QueryStringManager(api);
await manager.executeAll();
expect(manager.hasViewRelatedActionsToRun()).toBeTruthy();
await manager.executeIfNecessary();
expect(manager.hasViewRelatedActionsToRun()).toBeFalsy();
expect(manager.hasViewRelatedActions()).toBeTruthy();
expect(api.getViewManager().setViewByParametersWithNewQuery).toBeCalledWith({
params: {
view: viewName,
@@ -72,9 +73,9 @@ describe('QueryStringManager', () => {
vi.mocked(api.getCardElementManager().hasUpdated).mockReturnValue(true);
const manager = new QueryStringManager(api);
await manager.executeAll();
expect(manager.hasViewRelatedActionsToRun()).toBeFalsy();
await manager.executeIfNecessary();
expect(manager.hasViewRelatedActions()).toBeFalsy();
expect(api.getActionsManager().executeActions).toBeCalledWith([
{
action: 'fire-dom-event',
@@ -93,11 +94,11 @@ describe('QueryStringManager', () => {
const manager = new QueryStringManager(api);
await manager.executeAll();
expect(manager.hasViewRelatedActionsToRun()).toBeTruthy();
await manager.executeIfNecessary();
expect(manager.hasViewRelatedActionsToRun()).toBeFalsy();
expect(api.getViewManager().setViewDefaultWithNewQuery).toBeCalled();
expect(manager.hasViewRelatedActions()).toBeTruthy();
expect(api.getActionsManager().executeActions).not.toBeCalled();
expect(api.getViewManager().setViewByParameters).not.toBeCalled();
});
@@ -108,15 +109,15 @@ describe('QueryStringManager', () => {
vi.mocked(api.getCardElementManager().hasUpdated).mockReturnValue(true);
const manager = new QueryStringManager(api);
await manager.executeAll();
expect(manager.hasViewRelatedActionsToRun()).toBeTruthy();
await manager.executeIfNecessary();
expect(manager.hasViewRelatedActionsToRun()).toBeFalsy();
expect(api.getViewManager().setViewByParametersWithNewQuery).toBeCalledWith({
params: {
camera: 'camera.office',
},
});
expect(manager.hasViewRelatedActions()).toBeTruthy();
expect(api.getActionsManager().executeActions).not.toBeCalled();
expect(api.getViewManager().setViewDefault).not.toBeCalled();
});
@@ -127,14 +128,15 @@ describe('QueryStringManager', () => {
vi.mocked(api.getCardElementManager().hasUpdated).mockReturnValue(true);
const manager = new QueryStringManager(api);
await manager.executeAll();
expect(manager.hasViewRelatedActionsToRun()).toBeTruthy();
await manager.executeIfNecessary();
expect(manager.hasViewRelatedActionsToRun()).toBeFalsy();
expect(api.getViewManager().setViewByParametersWithNewQuery).toBeCalledWith({
modifiers: [expect.any(SubstreamSelectViewModifier)],
params: {},
});
expect(manager.hasViewRelatedActions()).toBeTruthy();
expect(api.getActionsManager().executeActions).not.toBeCalled();
expect(api.getViewManager().setViewDefault).not.toBeCalled();
});
@@ -148,9 +150,9 @@ describe('QueryStringManager', () => {
vi.mocked(api.getCardElementManager().hasUpdated).mockReturnValue(true);
const manager = new QueryStringManager(api);
await manager.executeAll();
expect(manager.hasViewRelatedActionsToRun()).toBeFalsy();
await manager.executeIfNecessary();
expect(manager.hasViewRelatedActions()).toBeFalsy();
expect(api.getActionsManager().executeActions).not.toBeCalled();
expect(api.getViewManager().setViewDefault).not.toBeCalled();
expect(api.getViewManager().setViewByParameters).not.toBeCalled();
@@ -166,9 +168,9 @@ describe('QueryStringManager', () => {
vi.mocked(api.getCardElementManager().hasUpdated).mockReturnValue(true);
const manager = new QueryStringManager(api);
await manager.executeAll();
expect(manager.hasViewRelatedActionsToRun()).toBeFalsy();
await manager.executeIfNecessary();
expect(manager.hasViewRelatedActions()).toBeFalsy();
expect(api.getActionsManager().executeActions).not.toBeCalled();
expect(api.getViewManager().setViewDefault).not.toBeCalled();
expect(api.getViewManager().setViewByParameters).not.toBeCalled();
@@ -193,9 +195,10 @@ describe('QueryStringManager', () => {
vi.mocked(api.getCardElementManager().hasUpdated).mockReturnValue(true);
const manager = new QueryStringManager(api);
await manager.executeAll();
expect(manager.hasViewRelatedActionsToRun()).toBeTruthy();
await manager.executeIfNecessary();
expect(manager.hasViewRelatedActionsToRun()).toBeFalsy();
expect(manager.hasViewRelatedActions()).toBeTruthy();
expect(api.getViewManager().setViewByParametersWithNewQuery).toBeCalledWith({
params: {
view: viewName,
@@ -204,26 +207,6 @@ describe('QueryStringManager', () => {
});
});
describe('should not execute non-view actions without an initial update', () => {
it.each([
['camera_ui' as const],
['download' as const],
['expand' as const],
['menu_toggle' as const],
])('%s', async (action: string) => {
setQueryString(`?frigate-card-action.id.${action}=value`);
const api = createCardAPI();
vi.mocked(api.getCardElementManager().hasUpdated).mockReturnValue(false);
const manager = new QueryStringManager(api);
await manager.executeAll();
expect(api.getActionsManager().executeActions).not.toBeCalled();
expect(api.getViewManager().setViewDefault).not.toBeCalled();
expect(api.getViewManager().setViewByParameters).not.toBeCalled();
});
});
describe('should handle conflicting but valid actions', () => {
it('view and default with camera and substream specified', async () => {
setQueryString(
@@ -236,7 +219,7 @@ describe('QueryStringManager', () => {
vi.mocked(api.getCardElementManager().hasUpdated).mockReturnValue(true);
const manager = new QueryStringManager(api);
await manager.executeAll();
await manager.executeIfNecessary();
expect(api.getViewManager().setViewDefaultWithNewQuery).toBeCalledWith({
params: {
@@ -256,7 +239,7 @@ describe('QueryStringManager', () => {
vi.mocked(api.getCardElementManager().hasUpdated).mockReturnValue(true);
const manager = new QueryStringManager(api);
await manager.executeAll();
await manager.executeIfNecessary();
expect(api.getViewManager().setViewByParametersWithNewQuery).toBeCalledWith({
params: {
@@ -266,47 +249,26 @@ describe('QueryStringManager', () => {
});
});
describe('should not execute view related actions', () => {
it.each([
['clip' as const],
['clips' as const],
['default' as const],
['diagnostics' as const],
['image' as const],
['live' as const],
['recording' as const],
['recordings' as const],
['snapshot' as const],
['snapshots' as const],
['timeline' as const],
])('%s', async (viewName: string) => {
setQueryString(`?frigate-card-action.id.${viewName}=`);
const api = createCardAPI();
vi.mocked(api.getCardElementManager().hasUpdated).mockReturnValue(true);
const manager = new QueryStringManager(api);
it('should only execute when needed', async () => {
setQueryString('?frigate-card-action.id.live_substream_select=camera.office_hd');
const api = createCardAPI();
vi.mocked(api.getCardElementManager().hasUpdated).mockReturnValue(true);
const manager = new QueryStringManager(api);
await manager.executeNonViewRelated();
expect(manager.hasViewRelatedActionsToRun()).toBeTruthy();
await manager.executeIfNecessary();
expect(manager.hasViewRelatedActionsToRun()).toBeFalsy();
expect(api.getViewManager().setViewByParametersWithNewQuery).toBeCalledTimes(1);
expect(api.getViewManager().setViewByParameters).not.toBeCalled();
expect(api.getViewManager().setViewDefault).not.toBeCalled();
});
});
await manager.executeIfNecessary();
expect(manager.hasViewRelatedActionsToRun()).toBeFalsy();
expect(api.getViewManager().setViewByParametersWithNewQuery).toBeCalledTimes(1);
describe('should not execute non-view related actions', () => {
it.each([
['camera_ui' as const],
['download' as const],
['expand' as const],
['menu_toggle' as const],
])('%s', async (viewName: string) => {
setQueryString(`?frigate-card-action.id.${viewName}=`);
const api = createCardAPI();
vi.mocked(api.getCardElementManager().hasUpdated).mockReturnValue(true);
const manager = new QueryStringManager(api);
manager.requestExecution();
await manager.executeViewRelated();
expect(api.getActionsManager().executeActions).not.toBeCalled();
});
expect(manager.hasViewRelatedActionsToRun()).toBeTruthy();
await manager.executeIfNecessary();
expect(manager.hasViewRelatedActionsToRun()).toBeFalsy();
expect(api.getViewManager().setViewByParametersWithNewQuery).toBeCalledTimes(2);
});
});
@@ -382,11 +382,13 @@ describe('hasMajorMediaChange', () => {
const api = createCardAPI();
const factory = mock<ViewFactory>();
const manager = new ViewManager(api, factory);
vi.mocked(api.getQueryStringManager().hasViewRelatedActions).mockReturnValue(true);
vi.mocked(api.getQueryStringManager().hasViewRelatedActionsToRun).mockReturnValue(
true,
);
expect(await manager.initialize()).toBeTruthy();
expect(api.getQueryStringManager().executeViewRelated).toBeCalled();
expect(manager.hasView()).toBeFalsy();
});
});
});