Skip to content

Commit

Permalink
fix(Dashboard): Backward compatible shared_label_colors field (apache…
Browse files Browse the repository at this point in the history
  • Loading branch information
geido authored Nov 26, 2024
1 parent 7f2e752 commit f077323
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 10 deletions.
14 changes: 10 additions & 4 deletions superset-frontend/src/dashboard/actions/dashboardState.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ import { SET_IN_SCOPE_STATUS_OF_FILTERS } from './nativeFilters';
import getOverwriteItems from '../util/getOverwriteItems';
import {
applyColors,
enforceSharedLabelsColorsArray,
isLabelsColorMapSynced,
getColorSchemeDomain,
getColorNamespace,
Expand Down Expand Up @@ -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 || '',
Expand Down Expand Up @@ -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,
);
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 };

Expand Down Expand Up @@ -67,7 +68,9 @@ const SyncDashboardState: FC<Props> = ({ 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,
Expand Down
5 changes: 4 additions & 1 deletion superset-frontend/src/dashboard/containers/Chart.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};

Expand Down Expand Up @@ -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({
Expand Down
18 changes: 16 additions & 2 deletions superset-frontend/src/utils/colorScheme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string> | 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.
Expand Down Expand Up @@ -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,
Expand Down
26 changes: 24 additions & 2 deletions superset/dashboards/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down

0 comments on commit f077323

Please sign in to comment.