From e4d71c967ebbedbae912dfece364c8ac5c220237 Mon Sep 17 00:00:00 2001 From: Yulong Ruan Date: Wed, 7 Aug 2024 18:19:08 +0800 Subject: [PATCH 1/3] feat: allow to update section input on the fly This makes it possible a dashboard rendered by content management plugin to update the query, time range and filters on the fly Signed-off-by: Yulong Ruan --- .../public/components/card_container/types.ts | 5 + .../public/components/section_input.test.ts | 47 +++++++- .../public/components/section_input.ts | 17 ++- .../public/components/types.ts | 5 + .../content_management/public/plugin.ts | 1 + .../content_management_service.test.ts | 22 ++++ .../content_management_service.ts | 18 ++- .../services/content_management/page.test.ts | 103 ++++++++++++++++++ .../services/content_management/page.ts | 28 ++++- .../services/content_management/types.ts | 5 + .../content_management/public/types.ts | 10 +- 11 files changed, 247 insertions(+), 14 deletions(-) create mode 100644 src/plugins/content_management/public/components/types.ts diff --git a/src/plugins/content_management/public/components/card_container/types.ts b/src/plugins/content_management/public/components/card_container/types.ts index 0358dfecfc1..5c6ad129af6 100644 --- a/src/plugins/content_management/public/components/card_container/types.ts +++ b/src/plugins/content_management/public/components/card_container/types.ts @@ -14,3 +14,8 @@ export interface CardExplicitInput { } export type CardContainerInput = ContainerInput & { columns?: number }; + +/** + * The props which allow to be updated after card container was created + */ +export type CardContainerExplicitInput = Partial>; diff --git a/src/plugins/content_management/public/components/section_input.test.ts b/src/plugins/content_management/public/components/section_input.test.ts index 6ec40f950f8..be471c8428a 100644 --- a/src/plugins/content_management/public/components/section_input.test.ts +++ b/src/plugins/content_management/public/components/section_input.test.ts @@ -7,7 +7,7 @@ import { coreMock } from '../../../../core/public/mocks'; import { Content, Section } from '../services'; import { createCardInput, createDashboardInput } from './section_input'; -test('it should create input for card section', () => { +test('it should create card section input', () => { const section: Section = { id: 'section1', kind: 'card', order: 10 }; const content: Content = { id: 'content1', @@ -36,6 +36,41 @@ test('it should create input for card section', () => { }); }); +test('it should create card section input with explicit input specified', () => { + const section: Section = { + id: 'section1', + kind: 'card', + order: 10, + input: { title: 'new title', columns: 4 }, + }; + const content: Content = { + id: 'content1', + kind: 'card', + order: 0, + title: 'content title', + description: 'content description', + }; + const input = createCardInput(section, [content]); + expect(input).toEqual({ + id: 'section1', + title: 'new title', + hidePanelTitles: true, + viewMode: 'view', + columns: 4, + panels: { + content1: { + type: 'card_embeddable', + explicitInput: { + description: 'content description', + id: 'content1', + onClick: undefined, + title: 'content title', + }, + }, + }, + }); +}); + test('it should not allow to create card input with non-card section', () => { const section: Section = { id: 'section1', kind: 'dashboard', order: 10 }; const content: Content = { @@ -79,7 +114,12 @@ test('it should throw error if creating dashboard input with non-dashboard secti }); test('it should create dashboard input', async () => { - const section: Section = { id: 'section1', kind: 'dashboard', order: 10 }; + const section: Section = { + id: 'section1', + kind: 'dashboard', + order: 10, + input: { timeRange: { from: 'now-1d', to: 'now' } }, + }; const staticViz: Content = { id: 'content1', kind: 'visualization', @@ -112,6 +152,9 @@ test('it should create dashboard input', async () => { savedObjectsClient: clientMock, }); + // with explicit section input + expect(input.timeRange).toEqual({ from: 'now-1d', to: 'now' }); + expect(input.panels).toEqual({ content1: { explicitInput: { diff --git a/src/plugins/content_management/public/components/section_input.ts b/src/plugins/content_management/public/components/section_input.ts index cd6ec6f23a0..5a0b9ca370c 100644 --- a/src/plugins/content_management/public/components/section_input.ts +++ b/src/plugins/content_management/public/components/section_input.ts @@ -32,6 +32,7 @@ export const createCardInput = ( viewMode: ViewMode.VIEW, columns: section.columns, panels, + ...section.input, }; contents.forEach((content) => { @@ -74,7 +75,7 @@ export const createDashboardInput = async ( const h = 15; const counter = new BehaviorSubject(0); - contents.forEach(async (content, i) => { + contents.forEach(async (content) => { counter.next(counter.value + 1); try { if (content.kind === 'dashboard') { @@ -164,29 +165,27 @@ export const createDashboardInput = async ( } }); - /** - * TODO: the input should be hooked with query input - */ const input: DashboardContainerInput = { - viewMode: ViewMode.VIEW, panels, + id: section.id, + title: section.title ?? '', + viewMode: ViewMode.VIEW, + useMargins: true, isFullScreenMode: false, filters: [], - useMargins: true, - id: section.id, timeRange: { to: 'now', from: 'now-7d', }, - title: section.title ?? 'test', query: { query: '', - language: 'lucene', + language: 'kuery', }, refreshConfig: { pause: true, value: 15, }, + ...section.input, }; return new Promise((resolve) => { diff --git a/src/plugins/content_management/public/components/types.ts b/src/plugins/content_management/public/components/types.ts new file mode 100644 index 00000000000..65a9e01798f --- /dev/null +++ b/src/plugins/content_management/public/components/types.ts @@ -0,0 +1,5 @@ +import { DashboardContainerInput } from '../../../dashboard/public'; + +export type DashboardContainerExplicitInput = Partial< + Pick +>; diff --git a/src/plugins/content_management/public/plugin.ts b/src/plugins/content_management/public/plugin.ts index 065a85dd126..6b5feeaeb9a 100644 --- a/src/plugins/content_management/public/plugin.ts +++ b/src/plugins/content_management/public/plugin.ts @@ -64,6 +64,7 @@ export class ContentManagementPublicPlugin this.contentManagementService.start(); return { registerContentProvider: this.contentManagementService.registerContentProvider, + updatePageSection: this.contentManagementService.updatePageSection, renderPage: (id: string) => { const page = this.contentManagementService.getPage(id); if (page) { diff --git a/src/plugins/content_management/public/services/content_management/content_management_service.test.ts b/src/plugins/content_management/public/services/content_management/content_management_service.test.ts index a73d666387a..b68157838b0 100644 --- a/src/plugins/content_management/public/services/content_management/content_management_service.test.ts +++ b/src/plugins/content_management/public/services/content_management/content_management_service.test.ts @@ -69,3 +69,25 @@ test('it should throw error when register content provider with invalid target a }) ).toThrowError(); }); + +test('it should throw error if update page section with invalid target area', () => { + const cms = new ContentManagementService(); + cms.registerPage({ id: 'page1', sections: [{ id: 'section1', kind: 'card', order: 0 }] }); + expect(() => cms.updatePageSection('invalid_target_area', jest.fn())).toThrowError(); +}); + +test('it should update page section', () => { + const cms = new ContentManagementService(); + cms.registerPage({ id: 'page1', sections: [{ id: 'section1', kind: 'card', order: 0 }] }); + cms.updatePageSection( + 'page1/section1', + jest.fn().mockReturnValue({ id: 'section1', kind: 'card', input: { title: 'new title' } }) + ); + expect(cms.getPage('page1')?.getSections()).toHaveLength(1); + expect(cms.getPage('page1')?.getSections()[0]).toEqual({ + id: 'section1', + kind: 'card', + order: 0, + input: { title: 'new title' }, + }); +}); diff --git a/src/plugins/content_management/public/services/content_management/content_management_service.ts b/src/plugins/content_management/public/services/content_management/content_management_service.ts index 1babdb06089..1c12ad826f5 100644 --- a/src/plugins/content_management/public/services/content_management/content_management_service.ts +++ b/src/plugins/content_management/public/services/content_management/content_management_service.ts @@ -4,7 +4,7 @@ */ import { Page } from './page'; -import { ContentProvider, PageConfig } from './types'; +import { ContentProvider, PageConfig, Section } from './types'; export class ContentManagementService { contentProviders: Map = new Map(); @@ -48,6 +48,22 @@ export class ContentManagementService { } }; + updatePageSection = ( + targetArea: string, + callback: (section: Section | null, err?: Error) => Section | null + ) => { + const [pageId, sectionId] = targetArea.split('/'); + + if (!pageId || !sectionId) { + throw new Error('getTargetArea() should return a string in format {pageId}/{sectionId}'); + } + + const page = this.getPage(pageId); + if (page) { + page.updateSectionInput(sectionId, callback); + } + }; + setup() { return {}; } diff --git a/src/plugins/content_management/public/services/content_management/page.test.ts b/src/plugins/content_management/public/services/content_management/page.test.ts index 13a0ec03c45..3a960f2805a 100644 --- a/src/plugins/content_management/public/services/content_management/page.test.ts +++ b/src/plugins/content_management/public/services/content_management/page.test.ts @@ -161,3 +161,106 @@ test('it should only allow to add one dashboard to a section', () => { }, ]); }); + +test('it should update dashboard section with new input', () => { + const page = new Page({ id: 'page1' }); + page.createSection({ + id: 'section_id', + kind: 'dashboard', + order: 1000, + input: { timeRange: { from: 'now-7d', to: 'now' } }, + }); + expect(page.getSections()).toHaveLength(1); + expect(page.getSections()[0]).toEqual({ + id: 'section_id', + kind: 'dashboard', + order: 1000, + input: { timeRange: { from: 'now-7d', to: 'now' } }, + }); + + page.updateSectionInput('section_id', (section) => { + if (section?.kind === 'dashboard') { + return { ...section, input: { timeRange: { from: 'now-1d', to: 'now' } } }; + } + return section; + }); + + expect(page.getSections()[0]).toEqual({ + id: 'section_id', + kind: 'dashboard', + order: 1000, + input: { timeRange: { from: 'now-1d', to: 'now' } }, + }); +}); + +test('it should update card section with new input', () => { + const page = new Page({ id: 'page1' }); + page.createSection({ + id: 'section_id', + kind: 'card', + order: 1000, + input: {}, + }); + expect(page.getSections()).toHaveLength(1); + expect(page.getSections()[0]).toEqual({ + id: 'section_id', + kind: 'card', + order: 1000, + input: {}, + }); + + page.updateSectionInput('section_id', (section) => { + if (section?.kind === 'card') { + return { ...section, input: { title: 'new title' } }; + } + return section; + }); + + expect(page.getSections()[0]).toEqual({ + id: 'section_id', + kind: 'card', + order: 1000, + input: { title: 'new title' }, + }); +}); + +test('it should not allow to update section property other than `input`', () => { + const page = new Page({ id: 'page1' }); + page.createSection({ + id: 'section_id', + kind: 'dashboard', + order: 1000, + input: { timeRange: { from: 'now-7d', to: 'now' } }, + }); + expect(page.getSections()).toHaveLength(1); + expect(page.getSections()[0]).toEqual({ + id: 'section_id', + kind: 'dashboard', + order: 1000, + input: { timeRange: { from: 'now-7d', to: 'now' } }, + }); + + // update the section with new kind: custom and new id: section_id_new + page.updateSectionInput('section_id', (section) => { + if (section?.kind === 'dashboard') { + return { ...section, id: 'section_id_new', kind: 'custom', render: jest.fn() }; + } + return section; + }); + + // section should not changed as it only allows to update `section.input` field + expect(page.getSections()[0]).toEqual({ + id: 'section_id', + kind: 'dashboard', + order: 1000, + input: { timeRange: { from: 'now-7d', to: 'now' } }, + }); +}); + +test('it should callback with error if section not exist', () => { + const page = new Page({ id: 'page1' }); + const callbackMock = jest.fn(); + page.updateSectionInput('section_id_not_exist', callbackMock); + expect(callbackMock.mock.calls[0][0]).toBe(null); + expect(callbackMock.mock.calls[0][1]).toBeInstanceOf(Error); +}); diff --git a/src/plugins/content_management/public/services/content_management/page.ts b/src/plugins/content_management/public/services/content_management/page.ts index 07fb18451be..7e8c988882a 100644 --- a/src/plugins/content_management/public/services/content_management/page.ts +++ b/src/plugins/content_management/public/services/content_management/page.ts @@ -18,11 +18,15 @@ export class Page { this.config = pageConfig; } - createSection(section: Section) { + private setSection(section: Section) { this.sections.set(section.id, section); this.sections$.next(this.getSections()); } + createSection(section: Section) { + this.setSection(section); + } + getSections() { return [...this.sections.values()].sort((a, b) => a.order - b.order); } @@ -31,6 +35,28 @@ export class Page { return this.sections$; } + updateSectionInput( + sectionId: string, + callback: (section: Section | null, err?: Error) => Section | null + ) { + const section = this.sections.get(sectionId); + if (!section) { + callback(null, new Error(`Section id ${sectionId} not found`)); + return; + } + + const updated = callback(section); + if (updated) { + if (updated.kind === 'dashboard' && section.kind === 'dashboard') { + this.setSection({ ...section, input: updated.input }); + } + if (updated.kind === 'card' && section.kind === 'card') { + this.setSection({ ...section, input: updated.input }); + } + // TODO: we may need to support update input of `custom` section + } + } + addContent(sectionId: string, content: Content) { const sectionContents = this.contents.get(sectionId); if (sectionContents) { diff --git a/src/plugins/content_management/public/services/content_management/types.ts b/src/plugins/content_management/public/services/content_management/types.ts index 7f49732fc2c..5b9b4662d37 100644 --- a/src/plugins/content_management/public/services/content_management/types.ts +++ b/src/plugins/content_management/public/services/content_management/types.ts @@ -3,6 +3,9 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { CardContainerExplicitInput } from '../../components/card_container/types'; +import { DashboardContainerExplicitInput } from '../../components/types'; + export interface PageConfig { id: string; title?: string; @@ -25,6 +28,7 @@ export type Section = order: number; title?: string; description?: string; + input?: DashboardContainerExplicitInput; } | { kind: 'card'; @@ -32,6 +36,7 @@ export type Section = order: number; title?: string; columns?: number; + input?: CardContainerExplicitInput; }; export type Content = diff --git a/src/plugins/content_management/public/types.ts b/src/plugins/content_management/public/types.ts index 697ad4d3829..a2c1f506b9c 100644 --- a/src/plugins/content_management/public/types.ts +++ b/src/plugins/content_management/public/types.ts @@ -6,7 +6,7 @@ import React from 'react'; import { CoreStart } from 'opensearch-dashboards/public'; -import { ContentManagementService, ContentProvider } from './services'; +import { ContentManagementService, ContentProvider, Section } from './services'; import { EmbeddableSetup, EmbeddableStart } from '../../embeddable/public'; export interface ContentManagementPluginSetup { @@ -17,6 +17,14 @@ export interface ContentManagementPluginStart { * @experimental this API is experimental and might change in future releases */ registerContentProvider: (provider: ContentProvider) => void; + + /** + * @experimental this API is experimental and might change in future releases + */ + updatePageSection: ( + targetArea: string, + callback: (section: Section | null, err?: Error) => Section | null + ) => void; renderPage: (id: string) => React.ReactNode; } From b1de9c9eec58082638943b8e15fcf29737067d43 Mon Sep 17 00:00:00 2001 From: "opensearch-changeset-bot[bot]" <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Date: Thu, 8 Aug 2024 02:20:23 +0000 Subject: [PATCH 2/3] Changeset file for PR #7651 created/updated --- changelogs/fragments/7651.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/7651.yml diff --git a/changelogs/fragments/7651.yml b/changelogs/fragments/7651.yml new file mode 100644 index 00000000000..451ec7d8c44 --- /dev/null +++ b/changelogs/fragments/7651.yml @@ -0,0 +1,2 @@ +feat: +- [contentManagement] allow to update section input after page rendered ([#7651](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7651)) \ No newline at end of file From 5b4f30ebbdc5e6aa6ceb3d3514b939bf16e14dc4 Mon Sep 17 00:00:00 2001 From: Yulong Ruan Date: Thu, 8 Aug 2024 12:02:40 +0800 Subject: [PATCH 3/3] fix(linter): add license header Signed-off-by: Yulong Ruan --- src/plugins/content_management/public/components/types.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/plugins/content_management/public/components/types.ts b/src/plugins/content_management/public/components/types.ts index 65a9e01798f..810fbaff08a 100644 --- a/src/plugins/content_management/public/components/types.ts +++ b/src/plugins/content_management/public/components/types.ts @@ -1,3 +1,8 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + import { DashboardContainerInput } from '../../../dashboard/public'; export type DashboardContainerExplicitInput = Partial<