chore: Update color scheme when deleted or changed (#20589)

* feat(explore): Use v1/explore endpoint data instead of bootstrapData

* Add tests

* Fix ci

* Remove redundant dependency

* Use form_data_key in cypress tests

* Add auth headers to for data request

* Address comments

* Remove displaying danger toast

* Conditionally add auth headers

* Address comments

* Fix typing bug

* fix

* Fix opening dataset

* Fix sqllab chart create

* Run queries in parallel

* Fallback to default color scheme

* Fix dashboard id autofill

* Fix lint

* Fix test

* Fix hydrate action

* Update dashboard colors

* Add color scheme domain

* Add check for default scheme

* Make me pretty

* Clean up

* Nit

* Clean up

* Pretty

* Fix missing sequential

* Lint

* Enhance test

* Lint

Co-authored-by: Kamil Gabryjelski <kamil.gabryjelski@gmail.com>
This commit is contained in:
Geido 2022-07-26 01:50:49 +03:00 committed by GitHub
parent 3de641c3ae
commit 6b0c3032b2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 182 additions and 12 deletions

View File

@ -28,7 +28,16 @@ export default class ColorSchemeRegistry<T> extends RegistryWithDefaultKey<T> {
});
}
get(key?: string) {
return super.get(key) as T | undefined;
get(key?: string, strict = false) {
const target = super.get(key) as T | undefined;
// fallsback to default scheme if any
if (!strict && !target) {
const defaultKey = super.getDefaultKey();
if (defaultKey) {
return super.get(defaultKey) as T | undefined;
}
}
return target;
}
}

View File

@ -18,10 +18,26 @@
*/
import ColorSchemeRegistry from '../../src/color/ColorSchemeRegistry';
import schemes from '../../src/color/colorSchemes/categorical/d3';
import CategoricalScheme from '../../src/color/CategoricalScheme';
describe('ColorSchemeRegistry', () => {
it('exists', () => {
expect(ColorSchemeRegistry).toBeDefined();
expect(ColorSchemeRegistry).toBeInstanceOf(Function);
});
it('returns undefined', () => {
const registry = new ColorSchemeRegistry();
expect(registry.get('something')).toBeUndefined();
});
it('returns default', () => {
const registry = new ColorSchemeRegistry();
registry.registerValue('SUPERSET_DEFAULT', schemes[0]);
expect(registry.get('something')).toBeInstanceOf(CategoricalScheme);
});
it('returns undefined in strict mode', () => {
const registry = new ColorSchemeRegistry();
registry.registerValue('SUPERSET_DEFAULT', schemes[0]);
expect(registry.get('something', true)).toBeUndefined();
});
});

View File

@ -240,6 +240,7 @@ export function saveDashboardRequest(data, id, saveType) {
...data.metadata,
color_namespace: data.metadata?.color_namespace || undefined,
color_scheme: data.metadata?.color_scheme || '',
color_scheme_domain: data.metadata?.color_scheme_domain || [],
expanded_slices: data.metadata?.expanded_slices || {},
label_colors: data.metadata?.label_colors || {},
shared_label_colors: data.metadata?.shared_label_colors || {},

View File

@ -18,13 +18,15 @@
*/
// ParentSize uses resize observer so the dashboard will update size
// when its container size changes, due to e.g., builder side panel opening
import React, { FC, useEffect, useMemo } from 'react';
import React, { FC, useCallback, useEffect, useMemo } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import {
FeatureFlag,
Filter,
Filters,
getCategoricalSchemeRegistry,
isFeatureEnabled,
SupersetClient,
} from '@superset-ui/core';
import { ParentSize } from '@vx/responsive';
import pick from 'lodash/pick';
@ -32,6 +34,7 @@ import Tabs from 'src/components/Tabs';
import DashboardGrid from 'src/dashboard/containers/DashboardGrid';
import {
ChartsState,
DashboardInfo,
DashboardLayout,
LayoutItem,
RootState,
@ -43,9 +46,13 @@ import {
import { getChartIdsInFilterScope } from 'src/dashboard/util/getChartIdsInFilterScope';
import findTabIndexByComponentId from 'src/dashboard/util/findTabIndexByComponentId';
import { setInScopeStatusOfFilters } from 'src/dashboard/actions/nativeFilters';
import { getRootLevelTabIndex, getRootLevelTabsComponent } from './utils';
import { findTabsWithChartsInScope } from '../nativeFilters/utils';
import { dashboardInfoChanged } from 'src/dashboard/actions/dashboardInfo';
import { setColorScheme } from 'src/dashboard/actions/dashboardState';
import { useComponentDidUpdate } from 'src/hooks/useComponentDidUpdate/useComponentDidUpdate';
import jsonStringify from 'json-stringify-pretty-compact';
import { NATIVE_FILTER_DIVIDER_PREFIX } from '../nativeFilters/FiltersConfigModal/utils';
import { findTabsWithChartsInScope } from '../nativeFilters/utils';
import { getRootLevelTabIndex, getRootLevelTabsComponent } from './utils';
type DashboardContainerProps = {
topLevelTabs?: LayoutItem;
@ -73,6 +80,9 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
const dashboardLayout = useSelector<RootState, DashboardLayout>(
state => state.dashboardLayout.present,
);
const dashboardInfo = useSelector<RootState, DashboardInfo>(
state => state.dashboardInfo,
);
const directPathToChild = useSelector<RootState, string[]>(
state => state.dashboardState.directPathToChild,
);
@ -122,10 +132,88 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
dispatch(setInScopeStatusOfFilters(scopes));
}, [nativeFilterScopes, dashboardLayout, dispatch]);
const verifyUpdateColorScheme = useCallback(() => {
const currentMetadata = dashboardInfo.metadata;
if (currentMetadata?.color_scheme) {
const metadata = { ...currentMetadata };
const colorScheme = metadata?.color_scheme;
const colorSchemeDomain = metadata?.color_scheme_domain || [];
const categoricalSchemes = getCategoricalSchemeRegistry();
const registryColorScheme =
categoricalSchemes.get(colorScheme, true) || undefined;
const registryColorSchemeDomain = registryColorScheme?.colors || [];
const defaultColorScheme = categoricalSchemes.defaultKey;
const colorSchemeExists = !!registryColorScheme;
const updateDashboardData = () => {
SupersetClient.put({
endpoint: `/api/v1/dashboard/${dashboardInfo.id}`,
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({
json_metadata: jsonStringify(metadata),
}),
}).catch(e => console.log(e));
};
const updateColorScheme = (scheme: string) => {
dispatch(setColorScheme(scheme));
};
const updateDashboard = () => {
dispatch(
dashboardInfoChanged({
metadata,
}),
);
updateDashboardData();
};
// selected color scheme does not exist anymore
// must fallback to the available default one
if (!colorSchemeExists) {
const updatedScheme =
defaultColorScheme?.toString() || 'supersetColors';
metadata.color_scheme = updatedScheme;
metadata.color_scheme_domain =
categoricalSchemes.get(defaultColorScheme)?.colors || [];
// reset shared_label_colors
// TODO: Requires regenerating the shared_label_colors after
// fixing a bug which affects their generation on dashboards with tabs
metadata.shared_label_colors = {};
updateColorScheme(updatedScheme);
updateDashboard();
} else {
// if this dashboard does not have a color_scheme_domain saved
// must create one and store it for the first time
if (colorSchemeExists && !colorSchemeDomain.length) {
metadata.color_scheme_domain = registryColorSchemeDomain;
updateDashboard();
}
// if the color_scheme_domain is not the same as the registry domain
// must update the existing color_scheme_domain
if (
colorSchemeExists &&
colorSchemeDomain.length &&
registryColorSchemeDomain.toString() !== colorSchemeDomain.toString()
) {
metadata.color_scheme_domain = registryColorSchemeDomain;
// reset shared_label_colors
// TODO: Requires regenerating the shared_label_colors after
// fixing a bug which affects their generation on dashboards with tabs
metadata.shared_label_colors = {};
updateColorScheme(colorScheme);
updateDashboard();
}
}
}
}, [charts]);
useComponentDidUpdate(verifyUpdateColorScheme);
const childIds: string[] = topLevelTabs
? topLevelTabs.children
: [DASHBOARD_GRID_ID];
const min = Math.min(tabIndex, childIds.length - 1);
const activeKey = min === 0 ? DASHBOARD_GRID_ID : min.toString();

View File

@ -262,6 +262,7 @@ test('submitting with onlyApply:false', async () => {
);
spyGetCategoricalSchemeRegistry.mockReturnValue({
keys: () => ['supersetColors'],
get: () => ['#FFFFFF', '#000000'],
} as any);
put.mockResolvedValue({
json: {
@ -289,7 +290,6 @@ test('submitting with onlyApply:false', async () => {
userEvent.click(screen.getByRole('button', { name: 'Save' }));
await waitFor(() => {
expect(props.onHide).toBeCalledTimes(1);
expect(props.onSubmit).toBeCalledTimes(1);
expect(props.onSubmit).toBeCalledWith({
certificationDetails: 'Sample certification',
@ -312,6 +312,7 @@ test('submitting with onlyApply:true', async () => {
);
spyGetCategoricalSchemeRegistry.mockReturnValue({
keys: () => ['supersetColors'],
get: () => ['#FFFFFF', '#000000'],
} as any);
spyIsFeatureEnabled.mockReturnValue(false);
const props = createProps();
@ -328,7 +329,6 @@ test('submitting with onlyApply:true', async () => {
userEvent.click(screen.getByRole('button', { name: 'Apply' }));
await waitFor(() => {
expect(props.onHide).toBeCalledTimes(1);
expect(props.onSubmit).toBeCalledTimes(1);
});
});

View File

@ -100,6 +100,7 @@ const PropertiesModal = ({
const [owners, setOwners] = useState<Owners>([]);
const [roles, setRoles] = useState<Roles>([]);
const saveLabel = onlyApply ? t('Apply') : t('Save');
const categoricalSchemeRegistry = getCategoricalSchemeRegistry();
const handleErrorResponse = async (response: Response) => {
const { error, statusText, message } = await getClientErrorObject(response);
@ -176,9 +177,13 @@ const PropertiesModal = ({
delete metadata.positions;
}
const metaDataCopy = { ...metadata };
if (metaDataCopy?.shared_label_colors) {
delete metaDataCopy.shared_label_colors;
}
delete metaDataCopy.color_scheme_domain;
setJsonMetadata(metaDataCopy ? jsonStringify(metaDataCopy) : '');
},
[form],
@ -264,7 +269,7 @@ const PropertiesModal = ({
{ updateMetadata = true } = {},
) => {
// check that color_scheme is valid
const colorChoices = getCategoricalSchemeRegistry().keys();
const colorChoices = categoricalSchemeRegistry.keys();
const jsonMetadataObj = getJsonMetadata();
// only fire if the color_scheme is present and invalid
@ -309,12 +314,25 @@ const PropertiesModal = ({
if (metadata?.shared_label_colors) {
delete metadata.shared_label_colors;
}
if (metadata?.color_scheme_domain) {
delete metadata.color_scheme_domain;
}
const colorMap = getSharedLabelColor().getColorMap(
colorNamespace,
currentColorScheme,
true,
);
metadata.shared_label_colors = colorMap;
if (metadata?.color_scheme) {
metadata.color_scheme_domain =
categoricalSchemeRegistry.get(colorScheme)?.colors || [];
} else {
metadata.color_scheme_domain = [];
}
currentJsonMetadata = jsonStringify(metadata);
}

View File

@ -81,6 +81,9 @@ export type DashboardInfo = {
native_filter_configuration: JsonObject;
show_native_filters: boolean;
chart_configuration: JsonObject;
color_scheme: string;
color_namespace: string;
color_scheme_domain: string[];
label_colors: JsonObject;
shared_label_colors: JsonObject;
};

View File

@ -26,7 +26,11 @@ import {
import { getChartKey } from 'src/explore/exploreUtils';
import { getControlsState } from 'src/explore/store';
import { Dispatch } from 'redux';
import { ensureIsArray } from '@superset-ui/core';
import {
ensureIsArray,
getCategoricalSchemeRegistry,
getSequentialSchemeRegistry,
} from '@superset-ui/core';
import {
getFormDataFromControls,
applyMapStateToPropsToControl,
@ -36,6 +40,11 @@ import { getUrlParam } from 'src/utils/urlUtils';
import { URL_PARAMS } from 'src/constants';
import { findPermission } from 'src/utils/findPermission';
enum ColorSchemeType {
CATEGORICAL = 'CATEGORICAL',
SEQUENTIAL = 'SEQUENTIAL',
}
export const HYDRATE_EXPLORE = 'HYDRATE_EXPLORE';
export const hydrateExplore =
({ form_data, slice, dataset }: ExplorePageInitialData) =>
@ -66,6 +75,31 @@ export const hydrateExplore =
initialExploreState,
initialFormData,
) as ControlStateMapping;
const colorSchemeKey = initialControls.color_scheme && 'color_scheme';
const linearColorSchemeKey =
initialControls.linear_color_scheme && 'linear_color_scheme';
// if the selected color scheme does not exist anymore
// fallbacks and selects the available default one
const verifyColorScheme = (type: ColorSchemeType) => {
const schemes =
type === 'CATEGORICAL'
? getCategoricalSchemeRegistry()
: getSequentialSchemeRegistry();
const key =
type === 'CATEGORICAL' ? colorSchemeKey : linearColorSchemeKey;
const registryDefaultScheme = schemes.defaultKey;
const defaultScheme =
type === 'CATEGORICAL' ? 'supersetColors' : 'superset_seq_1';
const currentScheme = initialFormData[key];
const colorSchemeExists = !!schemes.get(currentScheme, true);
if (currentScheme && !colorSchemeExists) {
initialControls[key].value = registryDefaultScheme || defaultScheme;
}
};
if (colorSchemeKey) verifyColorScheme(ColorSchemeType.CATEGORICAL);
if (linearColorSchemeKey) verifyColorScheme(ColorSchemeType.SEQUENTIAL);
const exploreState = {
// note this will add `form_data` to state,

View File

@ -266,7 +266,7 @@ class DashboardDAO(BaseDAO):
md["color_scheme"] = data.get("color_scheme", "")
md["label_colors"] = data.get("label_colors", {})
md["shared_label_colors"] = data.get("shared_label_colors", {})
md["color_scheme_domain"] = data.get("color_scheme_domain", [])
dashboard.json_metadata = json.dumps(md)
if commit:

View File

@ -129,6 +129,7 @@ class DashboardJSONMetadataSchema(Schema):
positions = fields.Dict(allow_none=True)
label_colors = fields.Dict()
shared_label_colors = fields.Dict()
color_scheme_domain = fields.List(fields.Str())
# used for v0 import/export
import_time = fields.Integer()
remote_id = fields.Integer()

View File

@ -72,7 +72,7 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi
"slug": "slug1_changed",
"position_json": '{"b": "B"}',
"css": "css_changed",
"json_metadata": '{"refresh_frequency": 30, "timed_refresh_immune_slices": [], "expanded_slices": {}, "color_scheme": "", "label_colors": {}, "shared_label_colors": {}}',
"json_metadata": '{"refresh_frequency": 30, "timed_refresh_immune_slices": [], "expanded_slices": {}, "color_scheme": "", "label_colors": {}, "shared_label_colors": {}, "color_scheme_domain": []}',
"published": False,
}