From 0c56199637901bd710d392041b2c9bdc090b3fbe Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Tue, 11 Jun 2019 18:08:12 -0700 Subject: [PATCH] feat: allow chart plugin to be unregistered (#168) * feat: allow chart plugin to be unregistered * test: address edge cases --- .../src/models/ChartPlugin.ts | 22 ++++-- .../test/models/ChartPlugin.test.tsx | 71 +++++++++++++++++-- .../superset-ui-core/src/models/Plugin.ts | 4 ++ .../test/models/Plugin.test.ts | 7 ++ 4 files changed, 91 insertions(+), 13 deletions(-) diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/models/ChartPlugin.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/models/ChartPlugin.ts index 59151a4563b4d..06ea2cd0d120a 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/models/ChartPlugin.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/models/ChartPlugin.ts @@ -17,17 +17,17 @@ type ValueOrModuleWithValue = T | { default: T }; interface ChartPluginConfig { metadata: ChartMetadata; - // use buildQuery for immediate value + /** Use buildQuery for immediate value. For lazy-loading, use loadBuildQuery. */ buildQuery?: BuildQueryFunction; - // use loadBuildQuery for dynamic import (lazy-loading) + /** Use loadBuildQuery for dynamic import (lazy-loading) */ loadBuildQuery?: PromiseOrValueLoader>>; - // use transformProps for immediate value + /** Use transformProps for immediate value. For lazy-loading, use loadTransformProps. */ transformProps?: TransformProps; - // use loadTransformProps for dynamic import (lazy-loading) + /** Use loadTransformProps for dynamic import (lazy-loading) */ loadTransformProps?: PromiseOrValueLoader>; - // use Chart for immediate value + /** Use Chart for immediate value. For lazy-loading, use loadChart. */ Chart?: ChartType; - // use loadChart for dynamic import (lazy-loading) + /** Use loadChart for dynamic import (lazy-loading) */ loadChart?: PromiseOrValueLoader>; } @@ -92,6 +92,16 @@ export default class ChartPlugin extend return this; } + unregister() { + const { key = isRequired('config.key') } = this.config; + getChartMetadataRegistry().remove(key); + getChartComponentRegistry().remove(key); + getChartTransformPropsRegistry().remove(key); + getChartBuildQueryRegistry().remove(key); + + return this; + } + configure(config: { [key: string]: any }, replace?: boolean) { super.configure(config, replace); diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/test/models/ChartPlugin.test.tsx b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/test/models/ChartPlugin.test.tsx index 425358b8c5a69..f7d39ed8a2fe0 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/test/models/ChartPlugin.test.tsx +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/test/models/ChartPlugin.test.tsx @@ -8,6 +8,10 @@ import { ChartProps, BuildQueryFunction, TransformProps, + getChartMetadataRegistry, + getChartComponentRegistry, + getChartTransformPropsRegistry, + getChartBuildQueryRegistry, } from '../../src'; describe('ChartPlugin', () => { @@ -18,7 +22,7 @@ describe('ChartPlugin', () => { thumbnail: '', }); - const buildQuery = (_: ChartFormData) => ({ + const buildQuery = () => ({ datasource: { id: 1, type: DatasourceType.Table }, queries: [{ granularity: 'day' }], }); @@ -132,17 +136,70 @@ describe('ChartPlugin', () => { }); describe('.register()', () => { - const plugin = new ChartPlugin({ - metadata, - Chart: FakeChart, - buildQuery, + let plugin: ChartPlugin; + + beforeEach(() => { + plugin = new ChartPlugin({ + metadata, + Chart: FakeChart, + buildQuery, + }); }); + it('throws an error if key is not provided', () => { expect(() => plugin.register()).toThrowError(Error); - expect(() => plugin.configure({ key: 'abc' }).register()).not.toThrowError(Error); + expect(() => plugin.configure({ key: 'ab' }).register()).not.toThrowError(Error); + }); + it('add the plugin to the registries', () => { + plugin.configure({ key: 'cd' }).register(); + expect(getChartMetadataRegistry().get('cd')).toBe(metadata); + expect(getChartComponentRegistry().get('cd')).toBe(FakeChart); + expect(getChartTransformPropsRegistry().has('cd')).toEqual(true); + expect(getChartBuildQueryRegistry().get('cd')).toBe(buildQuery); + }); + it('does not register buildQuery when it is not specified in the ChartPlugin', () => { + new ChartPlugin({ + metadata, + Chart: FakeChart, + }) + .configure({ key: 'ef' }) + .register(); + expect(getChartBuildQueryRegistry().has('ef')).toEqual(false); + }); + it('returns itself', () => { + expect(plugin.configure({ key: 'gh' }).register()).toBe(plugin); + }); + }); + + describe('.unregister()', () => { + let plugin: ChartPlugin; + + beforeEach(() => { + plugin = new ChartPlugin({ + metadata, + Chart: FakeChart, + buildQuery, + }); + }); + + it('throws an error if key is not provided', () => { + expect(() => plugin.unregister()).toThrowError(Error); + expect(() => plugin.configure({ key: 'abc' }).unregister()).not.toThrowError(Error); + }); + it('removes the chart from the registries', () => { + plugin.configure({ key: 'def' }).register(); + expect(getChartMetadataRegistry().get('def')).toBe(metadata); + expect(getChartComponentRegistry().get('def')).toBe(FakeChart); + expect(getChartTransformPropsRegistry().has('def')).toEqual(true); + expect(getChartBuildQueryRegistry().get('def')).toBe(buildQuery); + plugin.unregister(); + expect(getChartMetadataRegistry().has('def')).toEqual(false); + expect(getChartComponentRegistry().has('def')).toEqual(false); + expect(getChartTransformPropsRegistry().has('def')).toEqual(false); + expect(getChartBuildQueryRegistry().has('def')).toEqual(false); }); it('returns itself', () => { - expect(plugin.configure({ key: 'abc' }).register()).toBe(plugin); + expect(plugin.configure({ key: 'xyz' }).unregister()).toBe(plugin); }); }); }); diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/models/Plugin.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/models/Plugin.ts index 3f92ca404b3c3..31dab73228b13 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/models/Plugin.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/models/Plugin.ts @@ -26,4 +26,8 @@ export default class Plugin { register() { return this; } + + unregister() { + return this; + } } diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/models/Plugin.test.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/models/Plugin.test.ts index 1a4c3c8d50974..3426fbd93144a 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/models/Plugin.test.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/models/Plugin.test.ts @@ -50,4 +50,11 @@ describe('Plugin', () => { expect(plugin.register()).toBe(plugin); }); }); + + describe('.unregister()', () => { + it('returns the plugin itself', () => { + const plugin = new Plugin(); + expect(plugin.unregister()).toBe(plugin); + }); + }); });