From 1eead4b3976e5b84400c3f40a324e1a97977f6ee Mon Sep 17 00:00:00 2001 From: danielwiehl Date: Mon, 21 Aug 2023 15:51:02 +0200 Subject: [PATCH] fix(workbench): support application URL to contain view outlets of perspectival views Previously, opening the application with a URL containing view outlets of perspectival views caused the Angular error `NG04002` because the perspective layout was unavailable during initial navigation. Now we register routes based on the view outlets in the URL, not the views in the layout. fixes #474 --- projects/scion/e2e-testing/src/app.po.ts | 14 +++- .../src/workbench/page-object/view-page.po.ts | 37 +++++---- .../src/workbench/router.e2e-spec.ts | 80 +++++++++++++++++++ .../lib/routing/workbench-layout-differ.ts | 18 ++++- .../src/lib/routing/workbench-popup-differ.ts | 12 +-- .../routing/workbench-url-observer.service.ts | 32 ++++---- 6 files changed, 149 insertions(+), 44 deletions(-) diff --git a/projects/scion/e2e-testing/src/app.po.ts b/projects/scion/e2e-testing/src/app.po.ts index 674f31e2f..b841e39ed 100644 --- a/projects/scion/e2e-testing/src/app.po.ts +++ b/projects/scion/e2e-testing/src/app.po.ts @@ -61,7 +61,19 @@ export class AppPO { featureQueryParams.append('showNewTabAction', `${options.showNewTabAction}`); } - await this.page.goto(`${options?.url ?? ''}/?${this._workbenchStartupQueryParams.toString()}#/${featureQueryParams.toString() ? `?${featureQueryParams.toString()}` : ''}`); + // Perform navigation. + await this.page.goto((() => { + const [baseUrl = '/', hashedUrl = ''] = (options?.url?.split('#') ?? []); + + // Add startup query params to the base URL part. + const url = `${baseUrl}?${this._workbenchStartupQueryParams.toString()}#${hashedUrl}`; + if (!featureQueryParams.size) { + return url; + } + // Add feature query params to the hashed URL part. + return hashedUrl.includes('?') ? `${url}&${featureQueryParams}` : `${url}?${featureQueryParams}`; + })()); + // Wait until the workbench completed startup. await this.waitUntilWorkbenchStarted(); } diff --git a/projects/scion/e2e-testing/src/workbench/page-object/view-page.po.ts b/projects/scion/e2e-testing/src/workbench/page-object/view-page.po.ts index 5feb9c833..5613aaf03 100644 --- a/projects/scion/e2e-testing/src/workbench/page-object/view-page.po.ts +++ b/projects/scion/e2e-testing/src/workbench/page-object/view-page.po.ts @@ -23,38 +23,37 @@ import {SciKeyValuePO} from '../../@scion/components.internal/key-value.po'; */ export class ViewPagePO { - private readonly _locator: Locator; - + public readonly locator: Locator; public readonly viewPO: ViewPO; public readonly viewTabPO: ViewTabPO; constructor(appPO: AppPO, public viewId: string) { this.viewPO = appPO.view({viewId}); this.viewTabPO = appPO.view({viewId}).viewTab; - this._locator = this.viewPO.locator('app-view-page'); + this.locator = this.viewPO.locator('app-view-page'); } public async isPresent(): Promise { - return await this.viewTabPO.isPresent() && await isPresent(this._locator); + return await this.viewTabPO.isPresent() && await isPresent(this.locator); } public async isVisible(): Promise { - return await this.viewPO.isVisible() && await this._locator.isVisible(); + return await this.viewPO.isVisible() && await this.locator.isVisible(); } public async getViewId(): Promise { - return this._locator.locator('span.e2e-view-id').innerText(); + return this.locator.locator('span.e2e-view-id').innerText(); } public async getComponentInstanceId(): Promise { - return this._locator.locator('span.e2e-component-instance-id').innerText(); + return this.locator.locator('span.e2e-component-instance-id').innerText(); } public async getRouteParams(): Promise { - const accordionPO = new SciAccordionPO(this._locator.locator('sci-accordion.e2e-route-params')); + const accordionPO = new SciAccordionPO(this.locator.locator('sci-accordion.e2e-route-params')); await accordionPO.expand(); try { - return await new SciKeyValuePO(this._locator.locator('sci-key-value.e2e-route-params')).readEntries(); + return await new SciKeyValuePO(this.locator.locator('sci-key-value.e2e-route-params')).readEntries(); } finally { await accordionPO.collapse(); @@ -62,32 +61,32 @@ export class ViewPagePO { } public async enterTitle(title: string): Promise { - await this._locator.locator('input.e2e-title').fill(title); + await this.locator.locator('input.e2e-title').fill(title); } public async enterHeading(heading: string): Promise { - await this._locator.locator('input.e2e-heading').fill(heading); + await this.locator.locator('input.e2e-heading').fill(heading); } public async checkDirty(check: boolean): Promise { - await new SciCheckboxPO(this._locator.locator('sci-checkbox.e2e-dirty')).toggle(check); + await new SciCheckboxPO(this.locator.locator('sci-checkbox.e2e-dirty')).toggle(check); } public async checkClosable(check: boolean): Promise { - await new SciCheckboxPO(this._locator.locator('sci-checkbox.e2e-closable')).toggle(check); + await new SciCheckboxPO(this.locator.locator('sci-checkbox.e2e-closable')).toggle(check); } public async clickClose(): Promise { - const accordionPO = new SciAccordionPO(this._locator.locator('sci-accordion.e2e-view-actions')); + const accordionPO = new SciAccordionPO(this.locator.locator('sci-accordion.e2e-view-actions')); await accordionPO.expand(); - await this._locator.locator('button.e2e-close').click(); + await this.locator.locator('button.e2e-close').click(); } public async addViewAction(partAction: WorkbenchPartActionDescriptor, options?: {append?: boolean}): Promise { - const accordionPO = new SciAccordionPO(this._locator.locator('sci-accordion.e2e-part-actions')); + const accordionPO = new SciAccordionPO(this.locator.locator('sci-accordion.e2e-part-actions')); await accordionPO.expand(); try { - const inputLocator = this._locator.locator('input.e2e-part-actions'); + const inputLocator = this.locator.locator('input.e2e-part-actions'); if (options?.append ?? true) { const input = await inputLocator.inputValue() || null; const presentActions: WorkbenchPartActionDescriptor[] = coerceArray(input ? JSON.parse(input) : null); @@ -103,11 +102,11 @@ export class ViewPagePO { } public async enterFreeText(text: string): Promise { - await this._locator.locator('input.e2e-free-text').fill(text); + await this.locator.locator('input.e2e-free-text').fill(text); } public getFreeText(): Promise { - return this._locator.locator('input.e2e-free-text').inputValue(); + return this.locator.locator('input.e2e-free-text').inputValue(); } } diff --git a/projects/scion/e2e-testing/src/workbench/router.e2e-spec.ts b/projects/scion/e2e-testing/src/workbench/router.e2e-spec.ts index 726127382..05d4d91e5 100644 --- a/projects/scion/e2e-testing/src/workbench/router.e2e-spec.ts +++ b/projects/scion/e2e-testing/src/workbench/router.e2e-spec.ts @@ -12,6 +12,9 @@ import {expect} from '@playwright/test'; import {test} from '../fixtures'; import {RouterPagePO} from './page-object/router-page.po'; import {ViewPagePO} from './page-object/view-page.po'; +import {LayoutPagePO} from './page-object/layout-page.po'; +import {MPart, MTreeNode} from '../matcher/to-equal-workbench-layout.matcher'; +import {MAIN_AREA_PART_ID} from '../workbench.model'; test.describe('Workbench Router', () => { @@ -1165,4 +1168,81 @@ test.describe('Workbench Router', () => { // expect the test view to be the active view await expect(await testee1ViewPO.viewTab.isActive()).toBe(true); }); + + test('should support app URL to contain view outlets of views in the perspectival grid', async ({appPO, workbenchNavigator, page}) => { + await appPO.navigateTo({microfrontendSupport: false, perspectives: ['perspective']}); + + // Define perspective with a part on the left. + const perspectiveToggleButtonPO = await appPO.header.perspectiveToggleButton({perspectiveId: 'perspective'}); + await perspectiveToggleButtonPO.click(); + const layoutPagePO = await workbenchNavigator.openInNewTab(LayoutPagePO); + await layoutPagePO.addPart('left', {align: 'left', ratio: .25}); + + // Add view to the left perspectival part. + const routerPagePO = await workbenchNavigator.openInNewTab(RouterPagePO); + await routerPagePO.enterPath('test-view'); + await routerPagePO.enterTarget('blank'); + await routerPagePO.enterBlankPartId('left'); + await routerPagePO.clickNavigate(); + + // Expect the view to be opened in the left part. + await expect(appPO.workbenchLocator).toEqualWorkbenchLayout({ + peripheralGrid: { + root: new MTreeNode({ + direction: 'row', + ratio: .25, + child1: new MPart({ + id: 'left', + views: [{id: 'view.3'}], // test view page + activeViewId: 'view.3', + }), + child2: new MPart({id: MAIN_AREA_PART_ID}), + }), + }, + mainGrid: { + root: new MPart({ + id: await layoutPagePO.viewPO.part.getPartId(), + views: [{id: 'view.1'}, {id: 'view.2'}], // layout page, router page + activeViewId: 'view.2', + }), + activePartId: await layoutPagePO.viewPO.part.getPartId(), + }, + }); + + // Capture current URL. + const url = page.url(); + + // Clear the browser URL. + await page.goto('about:blank'); + + // WHEN: Opening the app with a URL that contains view outlets of views from the perspective grid + await appPO.navigateTo({url, microfrontendSupport: false, perspectives: ['perspective']}); + + // THEN: Expect the workbench layout to be restored. + await expect(appPO.workbenchLocator).toEqualWorkbenchLayout({ + peripheralGrid: { + root: new MTreeNode({ + direction: 'row', + ratio: .25, + child1: new MPart({ + id: 'left', + views: [{id: 'view.3'}], // test view page + activeViewId: 'view.3', + }), + child2: new MPart({id: MAIN_AREA_PART_ID}), + }), + }, + mainGrid: { + root: new MPart({ + id: await layoutPagePO.viewPO.part.getPartId(), + views: [{id: 'view.1'}, {id: 'view.2'}], // layout page, router page + activeViewId: 'view.2', + }), + activePartId: await layoutPagePO.viewPO.part.getPartId(), + }, + }); + + // Expect the test view to display. + await expect(new ViewPagePO(appPO, 'view.3').locator).toBeVisible(); + }); }); diff --git a/projects/scion/workbench/src/lib/routing/workbench-layout-differ.ts b/projects/scion/workbench/src/lib/routing/workbench-layout-differ.ts index fbd5dd4bd..85bbb7a96 100644 --- a/projects/scion/workbench/src/lib/routing/workbench-layout-differ.ts +++ b/projects/scion/workbench/src/lib/routing/workbench-layout-differ.ts @@ -10,6 +10,8 @@ import {Injectable, IterableChanges, IterableDiffer, IterableDiffers} from '@angular/core'; import {ɵWorkbenchLayout} from '../layout/ɵworkbench-layout'; +import {UrlTree} from '@angular/router'; +import {RouterUtils} from './router.util'; /** * Stateful differ for finding added/removed workbench layout elements. @@ -19,22 +21,26 @@ export class WorkbenchLayoutDiffer { private _partsDiffer: IterableDiffer; private _viewsDiffer: IterableDiffer; + private _viewOutletsDiffer: IterableDiffer; constructor(differs: IterableDiffers) { this._partsDiffer = differs.find([]).create(); this._viewsDiffer = differs.find([]).create(); + this._viewOutletsDiffer = differs.find([]).create(); } /** * Computes differences in the layout since last time {@link WorkbenchLayoutDiffer#diff} was invoked. */ - public diff(workbenchLayout?: ɵWorkbenchLayout): WorkbenchLayoutDiff { + public diff(workbenchLayout: ɵWorkbenchLayout | null, urlTree: UrlTree): WorkbenchLayoutDiff { const partIds = workbenchLayout?.parts().map(part => part.id) || []; const viewIds = workbenchLayout?.views().map(view => view.id) || []; + const viewOutlets = Object.keys(urlTree.root.children).filter(RouterUtils.isPrimaryRouteTarget); return new WorkbenchLayoutDiff({ parts: this._partsDiffer.diff(partIds), views: this._viewsDiffer.diff(viewIds), + viewOutlets: this._viewOutletsDiffer.diff(viewOutlets), }); } } @@ -50,12 +56,18 @@ export class WorkbenchLayoutDiff { public readonly addedViews = new Array(); public readonly removedViews = new Array(); - constructor(changes: {parts: IterableChanges | null; views: IterableChanges | null}) { + public readonly addedViewOutlets = new Array(); + public readonly removedViewOutlets = new Array(); + + constructor(changes: {parts: IterableChanges | null; views: IterableChanges | null; viewOutlets: IterableChanges | null}) { changes.parts?.forEachAddedItem(({item}) => this.addedParts.push(item)); changes.parts?.forEachRemovedItem(({item}) => this.removedParts.push(item)); changes.views?.forEachAddedItem(({item}) => this.addedViews.push(item)); changes.views?.forEachRemovedItem(({item}) => this.removedViews.push(item)); + + changes.viewOutlets?.forEachAddedItem(({item}) => this.addedViewOutlets.push(item)); + changes.viewOutlets?.forEachRemovedItem(({item}) => this.removedViewOutlets.push(item)); } public toString(): string { @@ -64,6 +76,8 @@ export class WorkbenchLayoutDiff { .concat(this.removedParts.length ? `removedParts=[${this.removedParts}]` : []) .concat(this.addedViews.length ? `addedViews=[${this.addedViews}]` : []) .concat(this.removedViews.length ? `removedViews=[${this.removedViews}]` : []) + .concat(this.addedViewOutlets.length ? `addedViewOutlets=[${this.addedViewOutlets}]` : []) + .concat(this.removedViewOutlets.length ? `removedViewOutlets=[${this.removedViewOutlets}]` : []) .join(', ')}`; } } diff --git a/projects/scion/workbench/src/lib/routing/workbench-popup-differ.ts b/projects/scion/workbench/src/lib/routing/workbench-popup-differ.ts index 559f299a7..1da893897 100644 --- a/projects/scion/workbench/src/lib/routing/workbench-popup-differ.ts +++ b/projects/scion/workbench/src/lib/routing/workbench-popup-differ.ts @@ -39,18 +39,18 @@ export class WorkbenchPopupDiffer { */ export class WorkbenchPopupDiff { - public readonly addedPopups = new Array(); - public readonly removedPopups = new Array(); + public readonly addedPopupOutlets = new Array(); + public readonly removedPopupOutlets = new Array(); constructor(changes: IterableChanges | null) { - changes?.forEachAddedItem(({item}) => this.addedPopups.push(item)); - changes?.forEachRemovedItem(({item}) => this.removedPopups.push(item)); + changes?.forEachAddedItem(({item}) => this.addedPopupOutlets.push(item)); + changes?.forEachRemovedItem(({item}) => this.removedPopupOutlets.push(item)); } public toString(): string { return `${new Array() - .concat(this.addedPopups.length ? `addedPopups=[${this.addedPopups}]` : []) - .concat(this.removedPopups.length ? `removedPopups=[${this.removedPopups}]` : []) + .concat(this.addedPopupOutlets.length ? `addedPopupOutlets=[${this.addedPopupOutlets}]` : []) + .concat(this.removedPopupOutlets.length ? `removedPopupOutlets=[${this.removedPopupOutlets}]` : []) .join(', ')}`; } } diff --git a/projects/scion/workbench/src/lib/routing/workbench-url-observer.service.ts b/projects/scion/workbench/src/lib/routing/workbench-url-observer.service.ts index 045e7e17c..0de925514 100644 --- a/projects/scion/workbench/src/lib/routing/workbench-url-observer.service.ts +++ b/projects/scion/workbench/src/lib/routing/workbench-url-observer.service.ts @@ -29,7 +29,6 @@ import {WorkbenchNavigationalStates} from './workbench-navigational-states'; import {MainAreaLayoutComponent} from '../layout/main-area-layout/main-area-layout.component'; import {PartComponent} from '../part/part.component'; import {MAIN_AREA_PART_ID} from '../layout/workbench-layout'; -import {RouterUtils} from './router.util'; import {canDeactivateWorkbenchView} from '../view/workbench-view-pre-destroy.guard'; import {resolveWorkbenchNavigationState} from './navigation-state.resolver'; import {takeUntilDestroyed} from '@angular/core/rxjs-interop'; @@ -85,7 +84,7 @@ export class WorkbenchUrlObserver { this._logger.debug(() => 'onNavigationCancel', LoggerNames.ROUTING, event); this.undoAuxiliaryRoutesRegistration(); this.undoWorkbenchLayoutDiffer(); - this.undoWorkbenchPoupDiffer(); + this.undoWorkbenchPopupDiffer(); this._workbenchRouter.setCurrentNavigationContext(null); } @@ -94,7 +93,7 @@ export class WorkbenchUrlObserver { this._logger.debug(() => 'onNavigationError', LoggerNames.ROUTING, event); this.undoAuxiliaryRoutesRegistration(); this.undoWorkbenchLayoutDiffer(); - this.undoWorkbenchPoupDiffer(); + this.undoWorkbenchPopupDiffer(); this._workbenchRouter.setCurrentNavigationContext(null); } @@ -138,7 +137,7 @@ export class WorkbenchUrlObserver { }); return { layout, - layoutDiff: this._workbenchLayoutDiffer.diff(layout), + layoutDiff: this._workbenchLayoutDiffer.diff(layout, urlTree), popupDiff: this._workbenchPopupDiffer.diff(urlTree), }; } @@ -148,14 +147,14 @@ export class WorkbenchUrlObserver { */ private registerAddedViewAuxiliaryRoutes(): void { const navigationContext = this._workbenchRouter.getCurrentNavigationContext(); - const addedViewIds = navigationContext.layoutDiff.addedViews.filter(RouterUtils.isPrimaryRouteTarget); + const addedViewOutlets = navigationContext.layoutDiff.addedViewOutlets; - const newAuxiliaryRoutes = this._auxRoutesRegistrator.registerOutletAuxiliaryRoutes(addedViewIds, { + const newAuxiliaryRoutes = this._auxRoutesRegistrator.registerOutletAuxiliaryRoutes(addedViewOutlets, { canDeactivate: [canDeactivateWorkbenchView], resolve: {[WorkbenchRouteData.state]: resolveWorkbenchNavigationState}, }); if (newAuxiliaryRoutes.length) { - this._logger.debug(() => `Registered auxiliary routes for views: ${addedViewIds}`, LoggerNames.ROUTING, newAuxiliaryRoutes); + this._logger.debug(() => `Registered auxiliary routes for views: ${addedViewOutlets}`, LoggerNames.ROUTING, newAuxiliaryRoutes); } } @@ -164,11 +163,11 @@ export class WorkbenchUrlObserver { */ public registerAddedPopupAuxiliaryRoutes(): void { const navigationContext = this._workbenchRouter.getCurrentNavigationContext(); - const addedPopupIds = navigationContext.popupDiff.addedPopups; + const addedPopupOutlets = navigationContext.popupDiff.addedPopupOutlets; - const newAuxiliaryRoutes = this._auxRoutesRegistrator.registerOutletAuxiliaryRoutes(addedPopupIds); + const newAuxiliaryRoutes = this._auxRoutesRegistrator.registerOutletAuxiliaryRoutes(addedPopupOutlets); if (newAuxiliaryRoutes.length) { - this._logger.debug(() => `Registered auxiliary routes for popups: ${addedPopupIds}`, LoggerNames.ROUTING, newAuxiliaryRoutes); + this._logger.debug(() => `Registered auxiliary routes for popups: ${addedPopupOutlets}`, LoggerNames.ROUTING, newAuxiliaryRoutes); } } @@ -179,7 +178,8 @@ export class WorkbenchUrlObserver { */ private undoWorkbenchLayoutDiffer(): void { const prevNavigateLayout = this._workbenchLayoutService.layout; // Layout in `WorkbenchLayoutService` is only updated after successful navigation - this._workbenchLayoutDiffer.diff(prevNavigateLayout ?? undefined); + const prevNavigateUrl = this._router.parseUrl(this._router.url); // Browser URL is only updated after successful navigation + this._workbenchLayoutDiffer.diff(prevNavigateLayout, prevNavigateUrl); } /** @@ -187,9 +187,9 @@ export class WorkbenchUrlObserver { * * Invoke this method after navigation failure or cancellation. The navigation is cancelled when guards perform a redirect or reject navigation. */ - private undoWorkbenchPoupDiffer(): void { - const prevNavigateUrl = this._router.url; // Browser URL is only updated after successful navigation - this._workbenchPopupDiffer.diff(this._router.parseUrl(prevNavigateUrl)); + private undoWorkbenchPopupDiffer(): void { + const prevNavigateUrl = this._router.parseUrl(this._router.url); // Browser URL is only updated after successful navigation + this._workbenchPopupDiffer.diff(prevNavigateUrl); } /** @@ -200,7 +200,7 @@ export class WorkbenchUrlObserver { private undoAuxiliaryRoutesRegistration(): void { const layoutDiff = this._workbenchRouter.getCurrentNavigationContext().layoutDiff; const popupDiff = this._workbenchRouter.getCurrentNavigationContext().popupDiff; - const addedOutlets: string[] = [...layoutDiff.addedViews.filter(RouterUtils.isPrimaryRouteTarget), ...popupDiff.addedPopups]; + const addedOutlets: string[] = [...layoutDiff.addedViewOutlets, ...popupDiff.addedPopupOutlets]; if (addedOutlets.length) { this._auxRoutesRegistrator.unregisterOutletAuxiliaryRoutes(addedOutlets); this._logger.debug(() => `Undo auxiliary routes registration for outlet(s): ${addedOutlets}`, LoggerNames.ROUTING); @@ -237,7 +237,7 @@ export class WorkbenchUrlObserver { private unregisterRemovedOutletAuxiliaryRoutes(): void { const layoutDiff = this._workbenchRouter.getCurrentNavigationContext().layoutDiff; const popupDiff = this._workbenchRouter.getCurrentNavigationContext().popupDiff; - const removedOutlets: string[] = [...layoutDiff.removedViews.filter(RouterUtils.isPrimaryRouteTarget), ...popupDiff.removedPopups]; + const removedOutlets: string[] = [...layoutDiff.removedViewOutlets, ...popupDiff.removedPopupOutlets]; if (removedOutlets.length) { this._logger.debug(() => 'Unregistering outlet auxiliary routes: ', LoggerNames.ROUTING, removedOutlets); this._auxRoutesRegistrator.unregisterOutletAuxiliaryRoutes(removedOutlets);