Skip to content

Commit 42d868c

Browse files
TobyBackstromclaude
andcommitted
fix(ngx-dashboard): make loadDashboard() survive dashboardData re-emissions
Two mechanisms could silently revert an imperative loadDashboard() call: 1. The constructor's dashboardData effect had an #isInitialized flag that was set but never checked - any re-emission of the input signal (e.g. from toSignal() on an HTTP observable) re-ran the effect and clobbered prior imperative loads. 2. loadDashboard() unconditionally overwrote the store's dashboardId from incoming data, re-keying DashboardBridgeService and potentially cross-wiring sibling dashboards. Guard the effect so it only applies the first emission, and preserve the store's existing dashboardId on subsequent imperative loads. The dashboardId field in DashboardDataDto is now treated as informational metadata on re-import, matching the semantics consumers already assume. Adds two regression tests and updates the DTO TSDoc. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent d4eb629 commit 42d868c

4 files changed

Lines changed: 112 additions & 4 deletions

File tree

projects/ngx-dashboard/src/lib/__tests__/dashboard-component-widget-state-integration.spec.ts

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -727,4 +727,97 @@ describe('DashboardComponent - Widget State Integration', () => {
727727
expect(menuService.lastSelectedWidgetTypeId()).toBeNull();
728728
});
729729
});
730+
731+
describe('loadDashboard() persistence', () => {
732+
it('should preserve imperative loadDashboard() across dashboardData re-emissions', async () => {
733+
// Regression: the dashboardData effect used to re-run on every input
734+
// emission, silently overwriting any prior imperative loadDashboard()
735+
// call. The #isInitialized guard prevents subsequent emissions from
736+
// clobbering the manual load.
737+
const initialData: DashboardDataDto = {
738+
version: '1.1.0',
739+
dashboardId: '7',
740+
rows: 8,
741+
columns: 16,
742+
gutterSize: '0.5em',
743+
cells: [],
744+
};
745+
746+
const importedData: DashboardDataDto = {
747+
version: '1.1.0',
748+
dashboardId: '3',
749+
rows: 7,
750+
columns: 16,
751+
gutterSize: '0.5em',
752+
cells: [
753+
{
754+
row: 1,
755+
col: 1,
756+
rowSpan: 1,
757+
colSpan: 1,
758+
widgetTypeid: 'test-widget',
759+
widgetState: { value: 'imported', counter: 42, modified: false },
760+
},
761+
],
762+
};
763+
764+
fixture.componentRef.setInput('dashboardData', initialData);
765+
fixture.detectChanges();
766+
await fixture.whenStable();
767+
768+
// Imperative import — consumers call this after the user picks a file.
769+
component.loadDashboard(importedData);
770+
fixture.detectChanges();
771+
await fixture.whenStable();
772+
773+
// Simulate an upstream re-emission (e.g. a toSignal() backed by an
774+
// HTTP observable producing another value after the manual load).
775+
fixture.componentRef.setInput('dashboardData', { ...initialData });
776+
fixture.detectChanges();
777+
await fixture.whenStable();
778+
779+
const exported = component.exportDashboard();
780+
// dashboardId stays pinned to the mount-time value — the incoming
781+
// id is treated as informational metadata, not an identity change.
782+
expect(exported.dashboardId).toBe('7');
783+
// But the imported layout and cells must win.
784+
expect(exported.rows).toBe(7);
785+
expect(exported.cells).toHaveSize(1);
786+
expect(exported.cells[0].widgetTypeid).toBe('test-widget');
787+
});
788+
789+
it('should preserve dashboardId across imperative loadDashboard() calls', async () => {
790+
// Option C: the store's dashboardId is set once on initial mount and
791+
// ignored on subsequent loadDashboard() calls. This keeps bridge
792+
// registration stable when importing a file from another dashboard.
793+
const initialData: DashboardDataDto = {
794+
version: '1.1.0',
795+
dashboardId: 'dashboard-a',
796+
rows: 5,
797+
columns: 5,
798+
gutterSize: '1em',
799+
cells: [],
800+
};
801+
802+
fixture.componentRef.setInput('dashboardData', initialData);
803+
fixture.detectChanges();
804+
await fixture.whenStable();
805+
806+
component.loadDashboard({
807+
version: '1.1.0',
808+
dashboardId: 'dashboard-b',
809+
rows: 4,
810+
columns: 4,
811+
gutterSize: '1em',
812+
cells: [],
813+
});
814+
fixture.detectChanges();
815+
await fixture.whenStable();
816+
817+
const exported = component.exportDashboard();
818+
expect(exported.dashboardId).toBe('dashboard-a');
819+
expect(exported.rows).toBe(4);
820+
expect(exported.columns).toBe(4);
821+
});
822+
});
730823
});

projects/ngx-dashboard/src/lib/dashboard/dashboard.component.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,12 @@ export class DashboardComponent implements OnChanges {
8383
this.#bridge.unregisterDashboard(this.#store);
8484
});
8585

86-
// Initialize from dashboardData
86+
// Initialize from dashboardData. Only applies the first emission so that
87+
// subsequent re-emissions (e.g. from toSignal() on an HTTP observable)
88+
// don't silently overwrite an imperative loadDashboard() call.
8789
effect(() => {
8890
const data = this.dashboardData();
89-
if (data) {
91+
if (data && !this.#isInitialized) {
9092
this.#store.loadDashboard(data);
9193
// Register with bridge service after dashboard ID is set
9294
this.#bridge.updateDashboardRegistration(this.#store);

projects/ngx-dashboard/src/lib/models/dashboard-data.dto.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,14 @@ export interface DashboardDataDto {
88
/** Version for future compatibility and migration support */
99
version: string;
1010

11-
/** Unique dashboard identifier managed by the client */
11+
/**
12+
* Unique dashboard identifier managed by the client.
13+
* Set once when a dashboard is first mounted via `[dashboardData]`.
14+
* On subsequent imperative `loadDashboard()` calls this field is treated
15+
* as informational metadata (i.e. where the file came from) — the store's
16+
* existing id is preserved so that bridge registration stays stable and
17+
* Export→Import across dashboards works without rewriting the id.
18+
*/
1219
dashboardId: string;
1320

1421
/** Grid dimensions */

projects/ngx-dashboard/src/lib/store/dashboard-store.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,8 +241,14 @@ export const DashboardStore = signalStore(
241241
widgetsById[WidgetIdUtils.toString(widgetId)] = cell;
242242
});
243243

244+
// Adopt the incoming dashboardId only on the initial load (when the
245+
// store's id is still empty). On subsequent imperative imports the
246+
// existing id is preserved so that bridge registration stays stable
247+
// and Export→Import across dashboards "just works" without requiring
248+
// consumers to rewrite the id in the file.
249+
const currentId = store.dashboardId();
244250
patchState(store, {
245-
dashboardId: data.dashboardId,
251+
...(currentId ? {} : { dashboardId: data.dashboardId }),
246252
rows: data.rows,
247253
columns: data.columns,
248254
gutterSize: data.gutterSize,

0 commit comments

Comments
 (0)