Skip to content

Commit

Permalink
feat: [contentManagement] allow to update section input after page re…
Browse files Browse the repository at this point in the history
…ndered (#7651)

* 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 <ruanyl@amazon.com>

* Changeset file for PR #7651 created/updated

* fix(linter): add license header

Signed-off-by: Yulong Ruan <ruanyl@amazon.com>

---------

Signed-off-by: Yulong Ruan <ruanyl@amazon.com>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
(cherry picked from commit b68754e)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
1 parent 3fad78c commit 5ffc485
Show file tree
Hide file tree
Showing 12 changed files with 254 additions and 14 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/7651.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
feat:
- [contentManagement] allow to update section input after page rendered ([#7651](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/7651))
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,8 @@ export interface CardExplicitInput {
}

export type CardContainerInput = ContainerInput<CardExplicitInput> & { columns?: number };

/**
* The props which allow to be updated after card container was created
*/
export type CardContainerExplicitInput = Partial<Pick<CardContainerInput, 'title' | 'columns'>>;
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export const createCardInput = (
viewMode: ViewMode.VIEW,
columns: section.columns,
panels,
...section.input,
};

contents.forEach((content) => {
Expand Down Expand Up @@ -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) => {

Check warning on line 78 in src/plugins/content_management/public/components/section_input.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/content_management/public/components/section_input.ts#L78

Added line #L78 was not covered by tests
counter.next(counter.value + 1);
try {
if (content.kind === 'dashboard') {
Expand Down Expand Up @@ -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<DashboardContainerInput>((resolve) => {
Expand Down
10 changes: 10 additions & 0 deletions src/plugins/content_management/public/components/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { DashboardContainerInput } from '../../../dashboard/public';

export type DashboardContainerExplicitInput = Partial<
Pick<DashboardContainerInput, 'filters' | 'timeRange' | 'query'>
>;
1 change: 1 addition & 0 deletions src/plugins/content_management/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import { Page } from './page';
import { ContentProvider, PageConfig } from './types';
import { ContentProvider, PageConfig, Section } from './types';

export class ContentManagementService {
contentProviders: Map<string, ContentProvider> = new Map();
Expand Down Expand Up @@ -48,6 +48,22 @@ export class ContentManagementService {
}
};

updatePageSection = (

Check warning on line 51 in src/plugins/content_management/public/services/content_management/content_management_service.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/content_management/public/services/content_management/content_management_service.ts#L51

Added line #L51 was not covered by tests
targetArea: string,
callback: (section: Section | null, err?: Error) => Section | null
) => {
const [pageId, sectionId] = targetArea.split('/');

Check warning on line 55 in src/plugins/content_management/public/services/content_management/content_management_service.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/content_management/public/services/content_management/content_management_service.ts#L55

Added line #L55 was not covered by tests

if (!pageId || !sectionId) {
throw new Error('getTargetArea() should return a string in format {pageId}/{sectionId}');

Check warning on line 58 in src/plugins/content_management/public/services/content_management/content_management_service.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/content_management/public/services/content_management/content_management_service.ts#L58

Added line #L58 was not covered by tests
}

const page = this.getPage(pageId);

Check warning on line 61 in src/plugins/content_management/public/services/content_management/content_management_service.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/content_management/public/services/content_management/content_management_service.ts#L61

Added line #L61 was not covered by tests
if (page) {
page.updateSectionInput(sectionId, callback);

Check warning on line 63 in src/plugins/content_management/public/services/content_management/content_management_service.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/content_management/public/services/content_management/content_management_service.ts#L63

Added line #L63 was not covered by tests
}
};

setup() {
return {};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Check warning on line 27 in src/plugins/content_management/public/services/content_management/page.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/content_management/public/services/content_management/page.ts#L27

Added line #L27 was not covered by tests
}

getSections() {
return [...this.sections.values()].sort((a, b) => a.order - b.order);
}
Expand All @@ -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);

Check warning on line 42 in src/plugins/content_management/public/services/content_management/page.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/content_management/public/services/content_management/page.ts#L42

Added line #L42 was not covered by tests
if (!section) {
callback(null, new Error(`Section id ${sectionId} not found`));
return;

Check warning on line 45 in src/plugins/content_management/public/services/content_management/page.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/content_management/public/services/content_management/page.ts#L44-L45

Added lines #L44 - L45 were not covered by tests
}

const updated = callback(section);

Check warning on line 48 in src/plugins/content_management/public/services/content_management/page.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/content_management/public/services/content_management/page.ts#L48

Added line #L48 was not covered by tests
if (updated) {
if (updated.kind === 'dashboard' && section.kind === 'dashboard') {
this.setSection({ ...section, input: updated.input });

Check warning on line 51 in src/plugins/content_management/public/services/content_management/page.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/content_management/public/services/content_management/page.ts#L51

Added line #L51 was not covered by tests
}
if (updated.kind === 'card' && section.kind === 'card') {
this.setSection({ ...section, input: updated.input });

Check warning on line 54 in src/plugins/content_management/public/services/content_management/page.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/content_management/public/services/content_management/page.ts#L54

Added line #L54 was not covered by tests
}
// TODO: we may need to support update input of `custom` section
}
}

addContent(sectionId: string, content: Content) {
const sectionContents = this.contents.get(sectionId);
if (sectionContents) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -25,13 +28,15 @@ export type Section =
order: number;
title?: string;
description?: string;
input?: DashboardContainerExplicitInput;
}
| {
kind: 'card';
id: string;
order: number;
title?: string;
columns?: number;
input?: CardContainerExplicitInput;
};

export type Content =
Expand Down
Loading

0 comments on commit 5ffc485

Please sign in to comment.