From f077323e6faa19e9219e3287e8ff6bb90dda14bd Mon Sep 17 00:00:00 2001 From: Geido <60598000+geido@users.noreply.github.com> Date: Tue, 26 Nov 2024 20:34:06 +0200 Subject: [PATCH] fix(Dashboard): Backward compatible shared_label_colors field (#31163) --- .../src/dashboard/actions/dashboardState.js | 14 +++++++--- .../components/SyncDashboardState/index.tsx | 5 +++- .../src/dashboard/containers/Chart.jsx | 5 +++- superset-frontend/src/utils/colorScheme.ts | 18 +++++++++++-- superset/dashboards/schemas.py | 26 +++++++++++++++++-- 5 files changed, 58 insertions(+), 10 deletions(-) diff --git a/superset-frontend/src/dashboard/actions/dashboardState.js b/superset-frontend/src/dashboard/actions/dashboardState.js index c2026955bdc4a..600098526bbe0 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.js +++ b/superset-frontend/src/dashboard/actions/dashboardState.js @@ -69,6 +69,7 @@ import { SET_IN_SCOPE_STATUS_OF_FILTERS } from './nativeFilters'; import getOverwriteItems from '../util/getOverwriteItems'; import { applyColors, + enforceSharedLabelsColorsArray, isLabelsColorMapSynced, getColorSchemeDomain, getColorNamespace, @@ -294,8 +295,9 @@ export function saveDashboardRequest(data, id, saveType) { const metadataCrossFiltersEnabled = data.metadata?.cross_filters_enabled; const colorScheme = data.metadata?.color_scheme; const customLabelsColor = data.metadata?.label_colors || {}; - const sharedLabelsColor = data.metadata?.shared_label_colors || []; - // making sure the data is what the backend expects + const sharedLabelsColor = enforceSharedLabelsColorsArray( + data.metadata?.shared_label_colors, + ); const cleanedData = { ...data, certified_by: certified_by || '', @@ -866,7 +868,9 @@ export const ensureSyncedSharedLabelsColors = dashboardState: { sharedLabelsColorsMustSync }, } = getState(); const updatedMetadata = { ...metadata }; - const sharedLabelsColors = metadata.shared_label_colors || []; + const sharedLabelsColors = enforceSharedLabelsColorsArray( + metadata.shared_label_colors, + ); const freshLabelsColors = getFreshSharedLabels( forceFresh ? [] : sharedLabelsColors, ); @@ -907,7 +911,9 @@ export const updateDashboardLabelsColor = const colorScheme = metadata.color_scheme; const labelsColorMapInstance = getLabelsColorMap(); const fullLabelsColors = metadata.map_label_colors || {}; - const sharedLabelsColors = metadata.shared_label_colors || []; + const sharedLabelsColors = enforceSharedLabelsColorsArray( + metadata.shared_label_colors, + ); const customLabelsColors = metadata.label_colors || {}; // for dashboards with no color scheme, the charts should always use their individual schemes diff --git a/superset-frontend/src/dashboard/components/SyncDashboardState/index.tsx b/superset-frontend/src/dashboard/components/SyncDashboardState/index.tsx index 174ec92d668db..fab9b9672d2c8 100644 --- a/superset-frontend/src/dashboard/components/SyncDashboardState/index.tsx +++ b/superset-frontend/src/dashboard/components/SyncDashboardState/index.tsx @@ -28,6 +28,7 @@ import { } from 'src/utils/localStorageHelpers'; import { RootState } from 'src/dashboard/types'; import { getActiveFilters } from 'src/dashboard/util/activeDashboardFilters'; +import { enforceSharedLabelsColorsArray } from 'src/utils/colorScheme'; type Props = { dashboardPageId: string }; @@ -67,7 +68,9 @@ const SyncDashboardState: FC = ({ dashboardPageId }) => { ({ dashboardInfo, dashboardState, nativeFilters, dataMask }) => ({ labelsColor: dashboardInfo.metadata?.label_colors || EMPTY_OBJECT, labelsColorMap: dashboardInfo.metadata?.map_label_colors || EMPTY_OBJECT, - sharedLabelsColors: dashboardInfo.metadata?.shared_label_colors || [], + sharedLabelsColors: enforceSharedLabelsColorsArray( + dashboardInfo.metadata?.shared_label_colors, + ), colorScheme: dashboardState?.colorScheme, chartConfiguration: dashboardInfo.metadata?.chart_configuration || EMPTY_OBJECT, diff --git a/superset-frontend/src/dashboard/containers/Chart.jsx b/superset-frontend/src/dashboard/containers/Chart.jsx index 940cdaaa1f038..9f00bd0bf7096 100644 --- a/superset-frontend/src/dashboard/containers/Chart.jsx +++ b/superset-frontend/src/dashboard/containers/Chart.jsx @@ -38,6 +38,7 @@ import { import getFormDataWithExtraFilters from 'src/dashboard/util/charts/getFormDataWithExtraFilters'; import Chart from 'src/dashboard/components/gridComponents/Chart'; import { PLACEHOLDER_DATASOURCE } from 'src/dashboard/constants'; +import { enforceSharedLabelsColorsArray } from 'src/utils/colorScheme'; const EMPTY_OBJECT = {}; @@ -66,7 +67,9 @@ function mapStateToProps( } = dashboardState; const labelsColor = dashboardInfo?.metadata?.label_colors || {}; const labelsColorMap = dashboardInfo?.metadata?.map_label_colors || {}; - const sharedLabelsColors = dashboardInfo?.metadata?.shared_label_colors || []; + const sharedLabelsColors = enforceSharedLabelsColorsArray( + dashboardInfo?.metadata?.shared_label_colors, + ); const ownColorScheme = chart.form_data?.color_scheme; // note: this method caches filters if possible to prevent render cascades const formData = getFormDataWithExtraFilters({ diff --git a/superset-frontend/src/utils/colorScheme.ts b/superset-frontend/src/utils/colorScheme.ts index 7bafc20cf4a98..1b9f2d12ecef2 100644 --- a/superset-frontend/src/utils/colorScheme.ts +++ b/superset-frontend/src/utils/colorScheme.ts @@ -24,13 +24,25 @@ import { } from '@superset-ui/core'; /** - * Forces falsy namespace values to undefined to default to GLOBAL + * Force falsy namespace values to undefined to default to GLOBAL * * @param namespace * @returns - namespace or default undefined */ export const getColorNamespace = (namespace?: string) => namespace || undefined; +/** + * + * Field shared_label_colors used to be a dict of all colors for all labels. + * Force shared_label_colors field to be a list of actual shared labels. + * + * @param sharedLabelsColors - the shared label colors list + * @returns string[] + */ +export const enforceSharedLabelsColorsArray = ( + sharedLabelsColors: string[] | Record | undefined, +) => (Array.isArray(sharedLabelsColors) ? sharedLabelsColors : []); + /** * Get labels shared across all charts in a dashboard. * Merges a fresh instance of shared label colors with a stored one. @@ -176,7 +188,9 @@ export const applyColors = ( CategoricalColorNamespace.getNamespace(colorNameSpace); const colorScheme = metadata?.color_scheme; const fullLabelsColor = metadata?.map_label_colors || {}; - const sharedLabels = metadata?.shared_label_colors || []; + const sharedLabels = enforceSharedLabelsColorsArray( + metadata?.shared_label_colors, + ); const customLabelsColor = metadata?.label_colors || {}; const sharedLabelsColor = getSharedLabelsColorMapEntries( fullLabelsColor, diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py index d855e22b87725..9f9e2f56ebad0 100644 --- a/superset/dashboards/schemas.py +++ b/superset/dashboards/schemas.py @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. import re -from typing import Any, Union +from typing import Any, Mapping, Union from marshmallow import fields, post_dump, post_load, pre_load, Schema from marshmallow.validate import Length, ValidationError @@ -116,6 +116,28 @@ def validate_json_metadata(value: Union[bytes, bytearray, str]) -> None: raise ValidationError(errors) +class SharedLabelsColorsField(fields.Field): + """ + A custom field that accepts either a list of strings or a dictionary. + """ + + def _deserialize( + self, + value: Union[list[str], dict[str, str]], + attr: Union[str, None], + data: Union[Mapping[str, Any], None], + **kwargs: dict[str, Any], + ) -> list[str]: + if isinstance(value, list): + if all(isinstance(item, str) for item in value): + return value + elif isinstance(value, dict): + # Enforce list (for backward compatibility) + return [] + + raise ValidationError("Not a valid list") + + class DashboardJSONMetadataSchema(Schema): # native_filter_configuration is for dashboard-native filters native_filter_configuration = fields.List(fields.Dict(), allow_none=True) @@ -137,7 +159,7 @@ class DashboardJSONMetadataSchema(Schema): color_namespace = fields.Str(allow_none=True) positions = fields.Dict(allow_none=True) label_colors = fields.Dict() - shared_label_colors = fields.List(fields.Str()) + shared_label_colors = SharedLabelsColorsField() map_label_colors = fields.Dict() color_scheme_domain = fields.List(fields.Str()) cross_filters_enabled = fields.Boolean(dump_default=True)