From 9860498f57bb9915555d17b3a61e247b451bac63 Mon Sep 17 00:00:00 2001 From: Adrian Molina Date: Mon, 25 May 2026 14:36:20 -0400 Subject: [PATCH 01/10] fix(uve): prevent breadcrumb from updating with stale page data during navigation Guard $updateBreadcrumbEffect with status === UVE_STATUS.LOADED so the breadcrumb only updates once the new page's data has fully resolved from the API. Previously, keeping the previous pageAsset visible during fetch (introduced in #35539) caused the effect to fire with stale title + new URL, and addNewBreadcrumb's URL deduplication then blocked the correct update from ever landing. Closes #35807 Co-Authored-By: Claude Sonnet 4.6 --- .../dot-ema-shell.component.spec.ts | 41 ++++++++++++++++++- .../dot-ema-shell/dot-ema-shell.component.ts | 4 +- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts index d08e2a52073c..5b1429e9beb9 100644 --- a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts +++ b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts @@ -8,7 +8,7 @@ import { } from '@ngneat/spectator/jest'; import { patchState } from '@ngrx/signals'; import { MockComponent } from 'ng-mocks'; -import { of } from 'rxjs'; +import { Subject, of } from 'rxjs'; import { Location } from '@angular/common'; import { provideHttpClient } from '@angular/common/http'; @@ -1144,6 +1144,45 @@ describe('DotEmaShellComponent', () => { }) ); }); + + it('should not update breadcrumb with stale data while page is loading, then update once loaded', async () => { + // Initialize with the first page fully loaded + spectator.detectChanges(); + await spectator.fixture.whenStable(); + spectator.detectChanges(); + mockGlobalStore.addNewBreadcrumb.mockClear(); + + // Hold the HTTP response so we can inspect state during LOADING + const pendingRequest$ = new Subject(); + jest.spyOn(dotPageApiService, 'get').mockReturnValue(pendingRequest$); + + // Navigate to a new page + store.pageLoad({ ...INITIAL_PAGE_PARAMS, url: '/new-page' }); + spectator.detectChanges(); + + // While status is LOADING, stale pageAsset is present but breadcrumb must not be updated + expect(mockGlobalStore.addNewBreadcrumb).not.toHaveBeenCalled(); + + // Resolve the HTTP response with the new page's data + const newPageResponse = { + ...MOCK_RESPONSE_HEADLESS, + page: { ...MOCK_RESPONSE_HEADLESS.page, title: 'New Page Title', identifier: '999' } + }; + pendingRequest$.next(newPageResponse); + pendingRequest$.complete(); + + await spectator.fixture.whenStable(); + spectator.detectChanges(); + + // After loading completes, breadcrumb must reflect the new page + expect(mockGlobalStore.addNewBreadcrumb).toHaveBeenCalledWith( + expect.objectContaining({ + label: 'New Page Title', + id: '999', + url: '/new-page' + }) + ); + }); }); }); diff --git a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts index 0099305c7305..2aad6e9f1526 100644 --- a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts +++ b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts @@ -208,8 +208,8 @@ export class DotEmaShellComponent implements OnInit, OnDestroy { readonly $updateBreadcrumbEffect = effect(() => { const page = this.uveStore.pageAsset()?.page; - - if (page) { + const status = this.uveStore.uveStatus(); + if (page && status === UVE_STATUS.LOADED) { this.#globalStore.addNewBreadcrumb({ label: page?.title, url: this.uveStore.pageParams().url, From 88de18777c9601ec0c9c0f4303510263a90005e5 Mon Sep 17 00:00:00 2001 From: Adrian Molina Date: Mon, 25 May 2026 14:49:52 -0400 Subject: [PATCH 02/10] fix(uve): guard breadcrumb against null pageParams on destroy and dead optional chains Add untracked() on pageParams to prevent TypeError when resetPageParams() nulls the signal during ngOnDestroy before the effect is torn down. Also remove dead optional chaining on page.title/page.identifier since page is already narrowed truthy by the if-guard. Adds a test covering the destroy path: resetPageParams() while status is LOADED must not throw and must not call addNewBreadcrumb. Co-Authored-By: Claude Sonnet 4.6 --- .../dot-ema-shell/dot-ema-shell.component.spec.ts | 15 +++++++++++++++ .../lib/dot-ema-shell/dot-ema-shell.component.ts | 8 +++++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts index 5b1429e9beb9..a11decda18e8 100644 --- a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts +++ b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts @@ -1145,6 +1145,21 @@ describe('DotEmaShellComponent', () => { ); }); + it('should not throw and should not call addNewBreadcrumb when resetPageParams nulls pageParams before effect tears down', async () => { + spectator.detectChanges(); + await spectator.fixture.whenStable(); + spectator.detectChanges(); + mockGlobalStore.addNewBreadcrumb.mockClear(); + + // Simulate ngOnDestroy: pageParams becomes null while status stays LOADED + expect(() => { + store.resetPageParams(); + spectator.detectChanges(); + }).not.toThrow(); + + expect(mockGlobalStore.addNewBreadcrumb).not.toHaveBeenCalled(); + }); + it('should not update breadcrumb with stale data while page is loading, then update once loaded', async () => { // Initialize with the first page fully loaded spectator.detectChanges(); diff --git a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts index 2aad6e9f1526..861dfe3a64e9 100644 --- a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts +++ b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts @@ -11,6 +11,7 @@ import { OnDestroy, OnInit, signal, + untracked, ViewChild } from '@angular/core'; import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; @@ -209,11 +210,12 @@ export class DotEmaShellComponent implements OnInit, OnDestroy { readonly $updateBreadcrumbEffect = effect(() => { const page = this.uveStore.pageAsset()?.page; const status = this.uveStore.uveStatus(); + if (page && status === UVE_STATUS.LOADED) { this.#globalStore.addNewBreadcrumb({ - label: page?.title, - url: this.uveStore.pageParams().url, - id: `${page?.identifier}` + label: page.title, + url: untracked(() => this.uveStore.pageParams()?.url), + id: `${page.identifier}` }); } }); From b28ddc97eca4c1b74e9424c2ecf8a3d3fe5cb1b8 Mon Sep 17 00:00:00 2001 From: Adrian Molina Date: Mon, 25 May 2026 14:52:10 -0400 Subject: [PATCH 03/10] refactor(uve): extract untracked pageParams url into a variable Co-Authored-By: Claude Sonnet 4.6 --- .../portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts index 861dfe3a64e9..df46f58f46f1 100644 --- a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts +++ b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts @@ -210,11 +210,12 @@ export class DotEmaShellComponent implements OnInit, OnDestroy { readonly $updateBreadcrumbEffect = effect(() => { const page = this.uveStore.pageAsset()?.page; const status = this.uveStore.uveStatus(); + const url = untracked(() => this.uveStore.pageParams()?.url); if (page && status === UVE_STATUS.LOADED) { this.#globalStore.addNewBreadcrumb({ label: page.title, - url: untracked(() => this.uveStore.pageParams()?.url), + url, id: `${page.identifier}` }); } From 47faa2b947cb7ca0cac6b736eb5c7ab996a1822b Mon Sep 17 00:00:00 2001 From: Adrian Molina Date: Mon, 25 May 2026 15:06:28 -0400 Subject: [PATCH 04/10] fix(uve): address review comments on breadcrumb effect - Move untracked url read inside the if-guard so it only runs when the breadcrumb is actually going to be updated - Add inline comment explaining the dual purpose of untracked (per store CLAUDE.md rule on documenting non-obvious untracked usage) - Fix destroy regression test to actually exercise the async teardown race: cycle uveStatus after resetPageParams() so the effect re-fires with pageParams = null, confirming no TypeError is thrown Co-Authored-By: Claude Sonnet 4.6 --- .../dot-ema-shell.component.spec.ts | 23 +++++++++++-------- .../dot-ema-shell/dot-ema-shell.component.ts | 6 ++++- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts index a11decda18e8..43609bedc8ed 100644 --- a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts +++ b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts @@ -69,7 +69,7 @@ import { DotEmaDialogComponent } from '../components/dot-ema-dialog/dot-ema-dial import { DotActionUrlService } from '../services/dot-action-url/dot-action-url.service'; import { DotPageApiService } from '../services/dot-page-api/dot-page-api.service'; import { DEFAULT_PERSONA, PERSONA_KEY } from '../shared/consts'; -import { FormStatus, NG_CUSTOM_EVENTS } from '../shared/enums'; +import { FormStatus, NG_CUSTOM_EVENTS, UVE_STATUS } from '../shared/enums'; import { dotPropertiesServiceMock, MOCK_RESPONSE_HEADLESS, @@ -1145,19 +1145,20 @@ describe('DotEmaShellComponent', () => { ); }); - it('should not throw and should not call addNewBreadcrumb when resetPageParams nulls pageParams before effect tears down', async () => { + it('should not throw when resetPageParams() nulls pageParams and a tracked dep re-fires the effect', async () => { spectator.detectChanges(); await spectator.fixture.whenStable(); spectator.detectChanges(); mockGlobalStore.addNewBreadcrumb.mockClear(); - // Simulate ngOnDestroy: pageParams becomes null while status stays LOADED - expect(() => { - store.resetPageParams(); - spectator.detectChanges(); - }).not.toThrow(); + // ngOnDestroy calls resetPageParams() (pageParams = null) but Angular tears + // down effects asynchronously, so the effect can re-run in that window. + // Cycling uveStatus on a tracked dep simulates that re-fire with null pageParams. + store.resetPageParams(); + patchState(store, { uveStatus: UVE_STATUS.LOADING }); + patchState(store, { uveStatus: UVE_STATUS.LOADED }); - expect(mockGlobalStore.addNewBreadcrumb).not.toHaveBeenCalled(); + expect(() => spectator.detectChanges()).not.toThrow(); }); it('should not update breadcrumb with stale data while page is loading, then update once loaded', async () => { @@ -1181,7 +1182,11 @@ describe('DotEmaShellComponent', () => { // Resolve the HTTP response with the new page's data const newPageResponse = { ...MOCK_RESPONSE_HEADLESS, - page: { ...MOCK_RESPONSE_HEADLESS.page, title: 'New Page Title', identifier: '999' } + page: { + ...MOCK_RESPONSE_HEADLESS.page, + title: 'New Page Title', + identifier: '999' + } }; pendingRequest$.next(newPageResponse); pendingRequest$.complete(); diff --git a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts index df46f58f46f1..99ca0a1071a2 100644 --- a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts +++ b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts @@ -210,9 +210,13 @@ export class DotEmaShellComponent implements OnInit, OnDestroy { readonly $updateBreadcrumbEffect = effect(() => { const page = this.uveStore.pageAsset()?.page; const status = this.uveStore.uveStatus(); - const url = untracked(() => this.uveStore.pageParams()?.url); if (page && status === UVE_STATUS.LOADED) { + // untracked: (a) prevents URL-only changes from re-triggering the breadcrumb, + // (b) avoids a TypeError crash when ngOnDestroy calls resetPageParams() (pageParams = null) + // before Angular tears down the effect asynchronously. + const url = untracked(() => this.uveStore.pageParams()?.url); + this.#globalStore.addNewBreadcrumb({ label: page.title, url, From a9d1e22f3ad365da1fddcc4f24816ed8d975fd79 Mon Sep 17 00:00:00 2001 From: Adrian Molina Date: Thu, 28 May 2026 10:21:33 -0400 Subject: [PATCH 05/10] fix(uve): update breadcrumb tests to match dotAdmin URL format Co-Authored-By: Claude Sonnet 4.6 --- .../dot-ema-shell.component.spec.ts | 8 ++++---- .../dot-ema-shell/dot-ema-shell.component.ts | 18 +++++++++++++----- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts index 43609bedc8ed..881392dbb60a 100644 --- a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts +++ b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts @@ -1086,7 +1086,7 @@ describe('DotEmaShellComponent', () => { expect.objectContaining({ label: expect.any(String), id: '123', - url: 'index' + url: expect.stringContaining('url=index') }) ); }); @@ -1127,7 +1127,7 @@ describe('DotEmaShellComponent', () => { expect.objectContaining({ label: 'Other Page', id: '456', - url: '/other-page' + url: expect.stringContaining('url=%2Fother-page') }) ); }); @@ -1140,7 +1140,7 @@ describe('DotEmaShellComponent', () => { expect(mockGlobalStore.addNewBreadcrumb).toHaveBeenCalledWith( expect.objectContaining({ - url: INITIAL_PAGE_PARAMS.url + url: expect.stringContaining('url=index') }) ); }); @@ -1199,7 +1199,7 @@ describe('DotEmaShellComponent', () => { expect.objectContaining({ label: 'New Page Title', id: '999', - url: '/new-page' + url: expect.stringContaining('url=%2Fnew-page') }) ); }); diff --git a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts index 99ca0a1071a2..2b3f11e82e73 100644 --- a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts +++ b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts @@ -212,10 +212,19 @@ export class DotEmaShellComponent implements OnInit, OnDestroy { const status = this.uveStore.uveStatus(); if (page && status === UVE_STATUS.LOADED) { - // untracked: (a) prevents URL-only changes from re-triggering the breadcrumb, - // (b) avoids a TypeError crash when ngOnDestroy calls resetPageParams() (pageParams = null) - // before Angular tears down the effect asynchronously. - const url = untracked(() => this.uveStore.pageParams()?.url); + // untracked: (a) evita re-ejecutar el breadcrumb solo por cambios de URL, + // (b) evita TypeError si ngOnDestroy llama resetPageParams() antes de que + // Angular destruya el effect de forma asíncrona. + const url = untracked(() => { + const params = this.uveStore.pageFriendlyParams(); + const baseClientHost = this.#activatedRoute.snapshot.data?.uveConfig?.url; + const cleanedParams = normalizeQueryParams(params, baseClientHost); + const paramsString = new URLSearchParams(cleanedParams).toString(); + + return `/dotAdmin/#/edit-page/content?${paramsString}`; + }); + + this.#globalStore.addNewBreadcrumb({ label: page.title, @@ -224,7 +233,6 @@ export class DotEmaShellComponent implements OnInit, OnDestroy { }); } }); - ngOnInit(): void { const params = this.#getPageParams(); const viewParams = this.#getViewParams(params.mode); From fbb731fe825c9d2b4030218054e8fc09341e9800 Mon Sep 17 00:00:00 2001 From: Adrian Molina Date: Thu, 28 May 2026 10:45:11 -0400 Subject: [PATCH 06/10] test(uve): replace stale-data test with navigation accumulation test Co-Authored-By: Claude Sonnet 4.6 --- .../dot-ema-shell.component.spec.ts | 99 +++---------------- 1 file changed, 13 insertions(+), 86 deletions(-) diff --git a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts index 37efb885a844..32a40e9ea14f 100644 --- a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts +++ b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts @@ -8,7 +8,7 @@ import { } from '@ngneat/spectator/jest'; import { patchState } from '@ngrx/signals'; import { MockComponent } from 'ng-mocks'; -import { Subject, of } from 'rxjs'; +import { of } from 'rxjs'; import { Location } from '@angular/common'; import { provideHttpClient } from '@angular/common/http'; @@ -1161,104 +1161,31 @@ describe('DotEmaShellComponent', () => { expect(() => spectator.detectChanges()).not.toThrow(); }); - it('should not update breadcrumb with stale data while page is loading, then update once loaded', async () => { - // Initialize with the first page fully loaded + it('should replace breadcrumb on navigation, not accumulate stale entries', async () => { + // Page A fully loaded spectator.detectChanges(); await spectator.fixture.whenStable(); spectator.detectChanges(); mockGlobalStore.addNewBreadcrumb.mockClear(); - // Hold the HTTP response so we can inspect state during LOADING - const pendingRequest$ = new Subject(); - jest.spyOn(dotPageApiService, 'get').mockReturnValue(pendingRequest$); - - // Navigate to a new page - store.pageLoad({ ...INITIAL_PAGE_PARAMS, url: '/new-page' }); - spectator.detectChanges(); - - // While status is LOADING, stale pageAsset is present but breadcrumb must not be updated - expect(mockGlobalStore.addNewBreadcrumb).not.toHaveBeenCalled(); - - // Resolve the HTTP response with the new page's data - const newPageResponse = { - ...MOCK_RESPONSE_HEADLESS, - page: { - ...MOCK_RESPONSE_HEADLESS.page, - title: 'New Page Title', - identifier: '999' - } - }; - pendingRequest$.next(newPageResponse); - pendingRequest$.complete(); - - await spectator.fixture.whenStable(); - spectator.detectChanges(); - - // After loading completes, breadcrumb must reflect the new page - expect(mockGlobalStore.addNewBreadcrumb).toHaveBeenCalledWith( - expect.objectContaining({ - label: 'New Page Title', - id: '999', - url: expect.stringContaining('url=%2Fnew-page') - }) - ); - }); - - it('should not throw when resetPageParams() nulls pageParams and a tracked dep re-fires the effect', async () => { - spectator.detectChanges(); - await spectator.fixture.whenStable(); - spectator.detectChanges(); - mockGlobalStore.addNewBreadcrumb.mockClear(); - - // ngOnDestroy calls resetPageParams() (pageParams = null) but Angular tears - // down effects asynchronously, so the effect can re-run in that window. - // Cycling uveStatus on a tracked dep simulates that re-fire with null pageParams. - store.resetPageParams(); - patchState(store, { uveStatus: UVE_STATUS.LOADING }); - patchState(store, { uveStatus: UVE_STATUS.LOADED }); - - expect(() => spectator.detectChanges()).not.toThrow(); - }); - - it('should not update breadcrumb with stale data while page is loading, then update once loaded', async () => { - // Initialize with the first page fully loaded - spectator.detectChanges(); - await spectator.fixture.whenStable(); - spectator.detectChanges(); - mockGlobalStore.addNewBreadcrumb.mockClear(); - - // Hold the HTTP response so we can inspect state during LOADING - const pendingRequest$ = new Subject(); - jest.spyOn(dotPageApiService, 'get').mockReturnValue(pendingRequest$); - - // Navigate to a new page - store.pageLoad({ ...INITIAL_PAGE_PARAMS, url: '/new-page' }); - spectator.detectChanges(); - - // While status is LOADING, stale pageAsset is present but breadcrumb must not be updated - expect(mockGlobalStore.addNewBreadcrumb).not.toHaveBeenCalled(); - - // Resolve the HTTP response with the new page's data - const newPageResponse = { + // Navigate to Page B + const pageBResponse = { ...MOCK_RESPONSE_HEADLESS, - page: { - ...MOCK_RESPONSE_HEADLESS.page, - title: 'New Page Title', - identifier: '999' - } + page: { ...MOCK_RESPONSE_HEADLESS.page, title: 'Page B', identifier: '456' } }; - pendingRequest$.next(newPageResponse); - pendingRequest$.complete(); + jest.spyOn(dotPageApiService, 'get').mockReturnValue(of(pageBResponse)); + store.pageLoad({ ...INITIAL_PAGE_PARAMS, url: '/page-b' }); await spectator.fixture.whenStable(); spectator.detectChanges(); - // After loading completes, breadcrumb must reflect the new page + // Called exactly once — with Page B data, never with stale Page A data + expect(mockGlobalStore.addNewBreadcrumb).toHaveBeenCalledTimes(1); expect(mockGlobalStore.addNewBreadcrumb).toHaveBeenCalledWith( expect.objectContaining({ - label: 'New Page Title', - id: '999', - url: '/new-page' + label: 'Page B', + id: '456', + url: expect.stringContaining('url=%2Fpage-b') }) ); }); From 609ae9767f4c852c63864aec7b25a4ac6ea006f5 Mon Sep 17 00:00:00 2001 From: Adrian Molina Date: Thu, 28 May 2026 10:49:37 -0400 Subject: [PATCH 07/10] fix(uve): update breadcrumb comments for clarity and accuracy Revised comments in the DotEmaShellComponent to enhance understanding of the breadcrumb functionality. The changes clarify the purpose of the untracked function, specifically its role in preventing unnecessary re-execution on URL changes and avoiding potential TypeErrors during component destruction. --- .../src/lib/dot-ema-shell/dot-ema-shell.component.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts index 2b3f11e82e73..c77d1029a033 100644 --- a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts +++ b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts @@ -212,9 +212,9 @@ export class DotEmaShellComponent implements OnInit, OnDestroy { const status = this.uveStore.uveStatus(); if (page && status === UVE_STATUS.LOADED) { - // untracked: (a) evita re-ejecutar el breadcrumb solo por cambios de URL, - // (b) evita TypeError si ngOnDestroy llama resetPageParams() antes de que - // Angular destruya el effect de forma asíncrona. + // untracked: (a) prevents URL-only changes from re-triggering the breadcrumb, + // (b) avoids a TypeError when ngOnDestroy calls resetPageParams() (pageParams = null) + // before Angular tears down the effect asynchronously. const url = untracked(() => { const params = this.uveStore.pageFriendlyParams(); const baseClientHost = this.#activatedRoute.snapshot.data?.uveConfig?.url; @@ -224,8 +224,6 @@ export class DotEmaShellComponent implements OnInit, OnDestroy { return `/dotAdmin/#/edit-page/content?${paramsString}`; }); - - this.#globalStore.addNewBreadcrumb({ label: page.title, url, From 3d0e994fab02a44a5ada27536c31a26f1636f43d Mon Sep 17 00:00:00 2001 From: Adrian Molina Date: Thu, 28 May 2026 12:55:49 -0400 Subject: [PATCH 08/10] test(uve): enhance breadcrumb loading state handling in tests Updated the DotEmaShellComponent tests to better handle loading states during navigation. Introduced a Subject to simulate pending requests, ensuring that breadcrumb actions are correctly suppressed while data is loading. This change improves the accuracy of the tests by reflecting the component's behavior during asynchronous data fetching. --- .../dot-ema-shell.component.spec.ts | 18 ++++++++++++++---- .../dot-ema-shell/dot-ema-shell.component.ts | 8 +++----- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts index 32a40e9ea14f..48ae42043e0f 100644 --- a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts +++ b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts @@ -8,7 +8,7 @@ import { } from '@ngneat/spectator/jest'; import { patchState } from '@ngrx/signals'; import { MockComponent } from 'ng-mocks'; -import { of } from 'rxjs'; +import { Subject, of } from 'rxjs'; import { Location } from '@angular/common'; import { provideHttpClient } from '@angular/common/http'; @@ -1168,14 +1168,24 @@ describe('DotEmaShellComponent', () => { spectator.detectChanges(); mockGlobalStore.addNewBreadcrumb.mockClear(); - // Navigate to Page B + // Hold the response to inspect the LOADING window + const pendingRequest$ = new Subject(); + jest.spyOn(dotPageApiService, 'get').mockReturnValue(pendingRequest$); + + store.pageLoad({ ...INITIAL_PAGE_PARAMS, url: '/page-b' }); + spectator.detectChanges(); + + // While LOADING, stale Page A data is present — breadcrumb must not fire + expect(mockGlobalStore.addNewBreadcrumb).not.toHaveBeenCalled(); + + // Resolve with Page B data const pageBResponse = { ...MOCK_RESPONSE_HEADLESS, page: { ...MOCK_RESPONSE_HEADLESS.page, title: 'Page B', identifier: '456' } }; - jest.spyOn(dotPageApiService, 'get').mockReturnValue(of(pageBResponse)); + pendingRequest$.next(pageBResponse); + pendingRequest$.complete(); - store.pageLoad({ ...INITIAL_PAGE_PARAMS, url: '/page-b' }); await spectator.fixture.whenStable(); spectator.detectChanges(); diff --git a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts index c77d1029a033..99be5b40ecf9 100644 --- a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts +++ b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts @@ -212,16 +212,14 @@ export class DotEmaShellComponent implements OnInit, OnDestroy { const status = this.uveStore.uveStatus(); if (page && status === UVE_STATUS.LOADED) { - // untracked: (a) prevents URL-only changes from re-triggering the breadcrumb, - // (b) avoids a TypeError when ngOnDestroy calls resetPageParams() (pageParams = null) - // before Angular tears down the effect asynchronously. + // untracked: prevents URL-only signal changes from re-triggering the breadcrumb. const url = untracked(() => { const params = this.uveStore.pageFriendlyParams(); const baseClientHost = this.#activatedRoute.snapshot.data?.uveConfig?.url; const cleanedParams = normalizeQueryParams(params, baseClientHost); - const paramsString = new URLSearchParams(cleanedParams).toString(); + const urlTree = this.#router.createUrlTree([], { queryParams: cleanedParams }); - return `/dotAdmin/#/edit-page/content?${paramsString}`; + return `/dotAdmin/#${urlTree.toString()}`; }); this.#globalStore.addNewBreadcrumb({ From 19abea2e0b3bfd14fb9b40c616b0ab715d5a2c9c Mon Sep 17 00:00:00 2001 From: Adrian Molina Date: Thu, 28 May 2026 13:31:38 -0400 Subject: [PATCH 09/10] fix(uve): prevent breadcrumb addition with null URL in DotEmaShellComponent Updated the DotEmaShellComponent to ensure that breadcrumbs are only added when a valid URL is present. This change prevents potential errors when the page parameters are not available, enhancing the stability of breadcrumb functionality. Additionally, updated the corresponding test to match the new URL validation logic. --- .../dot-ema-shell/dot-ema-shell.component.spec.ts | 2 +- .../lib/dot-ema-shell/dot-ema-shell.component.ts | 15 ++++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts index 48ae42043e0f..39b8f40a5179 100644 --- a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts +++ b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.spec.ts @@ -1140,7 +1140,7 @@ describe('DotEmaShellComponent', () => { expect(mockGlobalStore.addNewBreadcrumb).toHaveBeenCalledWith( expect.objectContaining({ - url: expect.stringContaining('url=index') + url: expect.stringMatching(/^\/dotAdmin\/#.*url=index/) }) ); }); diff --git a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts index 99be5b40ecf9..2777d08a1711 100644 --- a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts +++ b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts @@ -214,6 +214,8 @@ export class DotEmaShellComponent implements OnInit, OnDestroy { if (page && status === UVE_STATUS.LOADED) { // untracked: prevents URL-only signal changes from re-triggering the breadcrumb. const url = untracked(() => { + if (!this.uveStore.pageParams()) return null; + const params = this.uveStore.pageFriendlyParams(); const baseClientHost = this.#activatedRoute.snapshot.data?.uveConfig?.url; const cleanedParams = normalizeQueryParams(params, baseClientHost); @@ -222,13 +224,16 @@ export class DotEmaShellComponent implements OnInit, OnDestroy { return `/dotAdmin/#${urlTree.toString()}`; }); - this.#globalStore.addNewBreadcrumb({ - label: page.title, - url, - id: `${page.identifier}` - }); + if (url) { + this.#globalStore.addNewBreadcrumb({ + label: page.title, + url, + id: `${page.identifier}` + }); + } } }); + ngOnInit(): void { const params = this.#getPageParams(); const viewParams = this.#getViewParams(params.mode); From 19516d8329a07c326d81be6e16de3a761c000a08 Mon Sep 17 00:00:00 2001 From: Adrian Molina Date: Thu, 28 May 2026 16:03:39 -0400 Subject: [PATCH 10/10] refactor(uve): enhance breadcrumb handling in DotEmaShellComponent Updated the DotEmaShellComponent to utilize signalMethod for breadcrumb updates, ensuring that breadcrumbs are added only when valid page data is available. This change improves the clarity of the breadcrumb logic and eliminates the need for untracked functions, enhancing overall component performance and maintainability. --- .../dot-ema-shell/dot-ema-shell.component.ts | 42 +++++++++---------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts index 2777d08a1711..aa8340076b96 100644 --- a/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts +++ b/core-web/libs/portlets/edit-ema/portlet/src/lib/dot-ema-shell/dot-ema-shell.component.ts @@ -1,4 +1,4 @@ -import { patchState } from '@ngrx/signals'; +import { patchState, signalMethod } from '@ngrx/signals'; import { Location } from '@angular/common'; import { @@ -11,7 +11,6 @@ import { OnDestroy, OnInit, signal, - untracked, ViewChild } from '@angular/core'; import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; @@ -34,7 +33,7 @@ import { PageScannerToolType } from '@dotcms/portlets/dot-ema/ui'; import { GlobalStore } from '@dotcms/store'; -import { UVE_MODE } from '@dotcms/types'; +import { DotCMSPage, UVE_MODE } from '@dotcms/types'; import { DotInfoPageComponent, DotMessagePipe, DotNotLicenseComponent, InfoPage } from '@dotcms/ui'; import { EditEmaNavigationBarComponent } from './components/edit-ema-navigation-bar/edit-ema-navigation-bar.component'; @@ -207,33 +206,32 @@ export class DotEmaShellComponent implements OnInit, OnDestroy { this.#updateLocation(cleanedParams); }); - readonly $updateBreadcrumbEffect = effect(() => { + readonly $breadcrumbPage = computed(() => { const page = this.uveStore.pageAsset()?.page; const status = this.uveStore.uveStatus(); - if (page && status === UVE_STATUS.LOADED) { - // untracked: prevents URL-only signal changes from re-triggering the breadcrumb. - const url = untracked(() => { - if (!this.uveStore.pageParams()) return null; + return page && status === UVE_STATUS.LOADED ? page : null; + }); - const params = this.uveStore.pageFriendlyParams(); - const baseClientHost = this.#activatedRoute.snapshot.data?.uveConfig?.url; - const cleanedParams = normalizeQueryParams(params, baseClientHost); - const urlTree = this.#router.createUrlTree([], { queryParams: cleanedParams }); + readonly $updateBreadcrumb = signalMethod((page) => { + if (!page || !this.uveStore.pageParams()) return; - return `/dotAdmin/#${urlTree.toString()}`; - }); + const params = this.uveStore.pageFriendlyParams(); + const baseClientHost = this.#activatedRoute.snapshot.data?.uveConfig?.url; + const cleanedParams = normalizeQueryParams(params, baseClientHost); + const urlTree = this.#router.createUrlTree([], { queryParams: cleanedParams }); - if (url) { - this.#globalStore.addNewBreadcrumb({ - label: page.title, - url, - id: `${page.identifier}` - }); - } - } + this.#globalStore.addNewBreadcrumb({ + label: page.title, + url: `/dotAdmin/#${urlTree.toString()}`, + id: `${page.identifier}` + }); }); + constructor() { + this.$updateBreadcrumb(this.$breadcrumbPage); + } + ngOnInit(): void { const params = this.#getPageParams(); const viewParams = this.#getViewParams(params.mode);