From 1810ac50f2d0e2d05345e1e7ef35f91c08bf9337 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Fern=C3=A1ndez=20Haro?= Date: Tue, 19 Nov 2024 18:23:26 +0100 Subject: [PATCH 1/3] [Execution Context] Update on URL change --- .../src/execution_context_service.test.ts | 51 +++++++++++++++++++ .../src/execution_context_service.ts | 11 +++- .../src/core_system.ts | 1 + 3 files changed, 62 insertions(+), 1 deletion(-) diff --git a/packages/core/execution-context/core-execution-context-browser-internal/src/execution_context_service.test.ts b/packages/core/execution-context/core-execution-context-browser-internal/src/execution_context_service.test.ts index 868be40d4647f..e86cb0d6f2c79 100644 --- a/packages/core/execution-context/core-execution-context-browser-internal/src/execution_context_service.test.ts +++ b/packages/core/execution-context/core-execution-context-browser-internal/src/execution_context_service.test.ts @@ -9,6 +9,8 @@ import { BehaviorSubject, firstValueFrom } from 'rxjs'; import { analyticsServiceMock } from '@kbn/core-analytics-browser-mocks'; +import { applicationServiceMock } from '@kbn/core-application-browser-mocks'; +import type { InternalApplicationStart } from '@kbn/core-application-browser-internal'; import type { AnalyticsServiceSetup } from '@kbn/core-analytics-browser'; import type { ExecutionContextSetup } from '@kbn/core-execution-context-browser'; import { ExecutionContextService } from './execution_context_service'; @@ -18,14 +20,19 @@ describe('ExecutionContextService', () => { let curApp$: BehaviorSubject; let execService: ExecutionContextService; let analytics: jest.Mocked; + let history: jest.Mocked; beforeEach(() => { analytics = analyticsServiceMock.createAnalyticsServiceSetup(); + history = applicationServiceMock.createInternalStartContract().history as jest.Mocked< + InternalApplicationStart['history'] + >; execService = new ExecutionContextService(); execContext = execService.setup({ analytics }); curApp$ = new BehaviorSubject('app1'); execContext = execService.start({ curApp$, + history, }); }); @@ -96,6 +103,50 @@ describe('ExecutionContextService', () => { ); }); + it('url updates automatically when there is a navigation', async () => { + execContext.set({ + type: 'ghf', + meta: { + foo: 1, + }, + description: 'first set', + }); + + expect(execContext.get()).toMatchInlineSnapshot( + { + name: 'app1', + description: 'first set', + type: 'ghf', + url: '/', + }, + ` + Object { + "description": "first set", + "meta": Object { + "foo": 1, + }, + "name": "app1", + "type": "ghf", + "url": "/", + } + ` + ); + + history.listen.mock.calls[0][0]({ ...history.location, pathname: '/another-path' }, 'PUSH'); + + expect(execContext.get()).toMatchInlineSnapshot(` + Object { + "description": "first set", + "meta": Object { + "foo": 1, + }, + "name": "app1", + "type": "ghf", + "url": "/another-path", + } + `); + }); + it('sets context and adds current url and appid when getting it', () => { execContext.set({ type: 'ghf', diff --git a/packages/core/execution-context/core-execution-context-browser-internal/src/execution_context_service.ts b/packages/core/execution-context/core-execution-context-browser-internal/src/execution_context_service.ts index 9aadfe1448e70..5d5bf78b1f6bd 100644 --- a/packages/core/execution-context/core-execution-context-browser-internal/src/execution_context_service.ts +++ b/packages/core/execution-context/core-execution-context-browser-internal/src/execution_context_service.ts @@ -18,6 +18,7 @@ import type { ExecutionContextSetup, ExecutionContextStart, } from '@kbn/core-execution-context-browser'; +import type { InternalApplicationStart } from '@kbn/core-application-browser-internal'; // Should be exported from elastic/apm-rum export type LabelValue = string | number | boolean; @@ -32,6 +33,7 @@ export interface SetupDeps { export interface StartDeps { curApp$: Observable; + history: InternalApplicationStart['history']; } /** @internal */ @@ -75,7 +77,7 @@ export class ExecutionContextService return this.contract; } - public start({ curApp$ }: StartDeps) { + public start({ curApp$, history }: StartDeps) { const start = this.contract!; // Track app id changes and clear context on app change @@ -86,6 +88,13 @@ export class ExecutionContextService }) ); + // Track URL changes to make sure that we reflect the new path name + this.subscription.add( + history.listen((location) => { + start.set({ url: location.pathname }); + }) + ); + return start; } diff --git a/packages/core/root/core-root-browser-internal/src/core_system.ts b/packages/core/root/core-root-browser-internal/src/core_system.ts index 44e25b257e32c..6728a26d3c891 100644 --- a/packages/core/root/core-root-browser-internal/src/core_system.ts +++ b/packages/core/root/core-root-browser-internal/src/core_system.ts @@ -339,6 +339,7 @@ export class CoreSystem { const executionContext = this.executionContext.start({ curApp$: application.currentAppId$, + history: application.history, }); const chrome = await this.chrome.start({ From 57712db5f5c4e8fc7cdb9b1178bd337ee2489422 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Tue, 19 Nov 2024 17:42:42 +0000 Subject: [PATCH 2/3] [CI] Auto-commit changed files from 'node scripts/notice' --- .../core-execution-context-browser-internal/tsconfig.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/core/execution-context/core-execution-context-browser-internal/tsconfig.json b/packages/core/execution-context/core-execution-context-browser-internal/tsconfig.json index a8301ada1eaa4..ef931479e6a35 100644 --- a/packages/core/execution-context/core-execution-context-browser-internal/tsconfig.json +++ b/packages/core/execution-context/core-execution-context-browser-internal/tsconfig.json @@ -15,7 +15,9 @@ "@kbn/core-analytics-browser", "@kbn/core-analytics-browser-mocks", "@kbn/core-execution-context-common", - "@kbn/core-execution-context-browser" + "@kbn/core-execution-context-browser", + "@kbn/core-application-browser-mocks", + "@kbn/core-application-browser-internal" ], "exclude": [ "target/**/*", From e69e2aa4d6e693f9225aaa83566ba679b200d99a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Fern=C3=A1ndez=20Haro?= Date: Wed, 20 Nov 2024 12:12:18 +0100 Subject: [PATCH 3/3] Resolve circular dependencies --- .../src/execution_context_service.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/execution-context/core-execution-context-browser-internal/src/execution_context_service.ts b/packages/core/execution-context/core-execution-context-browser-internal/src/execution_context_service.ts index 5d5bf78b1f6bd..e1ac6cff6a5e9 100644 --- a/packages/core/execution-context/core-execution-context-browser-internal/src/execution_context_service.ts +++ b/packages/core/execution-context/core-execution-context-browser-internal/src/execution_context_service.ts @@ -8,6 +8,7 @@ */ import { compact, isEqual, isUndefined, omitBy } from 'lodash'; +import type { History } from 'history'; import type { Observable } from 'rxjs'; import { BehaviorSubject, Subscription } from 'rxjs'; import { map } from 'rxjs'; @@ -18,7 +19,6 @@ import type { ExecutionContextSetup, ExecutionContextStart, } from '@kbn/core-execution-context-browser'; -import type { InternalApplicationStart } from '@kbn/core-application-browser-internal'; // Should be exported from elastic/apm-rum export type LabelValue = string | number | boolean; @@ -33,7 +33,7 @@ export interface SetupDeps { export interface StartDeps { curApp$: Observable; - history: InternalApplicationStart['history']; + history: History; } /** @internal */