Skip to content

Commit

Permalink
fix(Dashboard): Sync color configuration via dedicated endpoint
Browse files Browse the repository at this point in the history
  • Loading branch information
@geido
geido committed Dec 10, 2024
1 parent 0133bab commit a983319
Show file tree
Hide file tree
Showing 10 changed files with 453 additions and 130 deletions.
253 changes: 142 additions & 111 deletions superset-frontend/src/dashboard/actions/dashboardState.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ import { getActiveFilters } from 'src/dashboard/util/activeDashboardFilters';
import { safeStringify } from 'src/utils/safeStringify';
import { logEvent } from 'src/logger/actions';
import { LOG_ACTIONS_CONFIRM_OVERWRITE_DASHBOARD_METADATA } from 'src/logger/LogUtils';
import { isEqual } from 'lodash';
import { isEqual, sortBy } from 'lodash';
import { UPDATE_COMPONENTS_PARENTS_LIST } from './dashboardLayout';
import {
saveChartConfiguration,
Expand All @@ -73,8 +73,9 @@ import {
isLabelsColorMapSynced,
getColorSchemeDomain,
getColorNamespace,
getLabelsColorMapEntries,
getFreshLabelsColorMapEntries,
getFreshSharedLabels,
getDynamicLabelsColors,
} from '../../utils/colorScheme';

export const SET_UNSAVED_CHANGES = 'SET_UNSAVED_CHANGES';
Expand Down Expand Up @@ -253,15 +254,18 @@ export function setDashboardSharedLabelsColorsSynced() {
return { type: SET_DASHBOARD_SHARED_LABELS_COLORS_SYNCED };
}

export const setDashboardMetadata = updatedMetadata => async dispatch => {
dispatch(
dashboardInfoChanged({
metadata: {
...updatedMetadata,
},
}),
);
};
export const setDashboardMetadata =
updatedMetadata => async (dispatch, getState) => {
const { dashboardInfo } = getState();
dispatch(
dashboardInfoChanged({
metadata: {
...dashboardInfo.metadata,
...updatedMetadata,
},
}),
);
};

export function saveDashboardRequest(data, id, saveType) {
return (dispatch, getState) => {
Expand Down Expand Up @@ -320,7 +324,7 @@ export function saveDashboardRequest(data, id, saveType) {
expanded_slices: data.metadata?.expanded_slices || {},
label_colors: customLabelsColor,
shared_label_colors: getFreshSharedLabels(sharedLabelsColor),
map_label_colors: getLabelsColorMapEntries(customLabelsColor),
map_label_colors: getFreshLabelsColorMapEntries(customLabelsColor),
refresh_frequency: data.metadata?.refresh_frequency || 0,
timed_refresh_immune_slices:
data.metadata?.timed_refresh_immune_slices || [],
Expand Down Expand Up @@ -719,11 +723,18 @@ export function setDatasetsStatus(status) {
};
}

const storeDashboardMetadata = async (id, metadata) =>
const storeDashboardColorConfig = async (id, metadata) =>
SupersetClient.put({
endpoint: `/api/v1/dashboard/${id}`,
endpoint: `/api/v1/dashboard/${id}/colors?mark_updated=false`,
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ json_metadata: JSON.stringify(metadata) }),
body: JSON.stringify({
color_namespace: metadata.color_namespace,
color_scheme: metadata.color_scheme,
color_scheme_domain: metadata.color_scheme_domain || [],
shared_label_colors: metadata.shared_label_colors || [],
map_label_colors: metadata.map_label_colors || {},
label_colors: metadata.label_colors || {},
}),
});

/**
Expand All @@ -742,7 +753,7 @@ export const persistDashboardLabelsColor = () => async (dispatch, getState) => {
if (labelsColorMapMustSync || sharedLabelsColorsMustSync) {
dispatch(setDashboardLabelsColorMapSynced());
dispatch(setDashboardSharedLabelsColorsSynced());
storeDashboardMetadata(id, metadata);
storeDashboardColorConfig(id, metadata);
}
};

Expand All @@ -756,19 +767,21 @@ export const persistDashboardLabelsColor = () => async (dispatch, getState) => {
*/
export const applyDashboardLabelsColorOnLoad = metadata => async dispatch => {
try {
const updatedMetadata = { ...metadata };
const customLabelsColor = metadata.label_colors || {};
let hasChanged = false;

// backward compatibility of shared_label_colors
const sharedLabels = metadata.shared_label_colors || [];
if (!Array.isArray(sharedLabels) && Object.keys(sharedLabels).length > 0) {
hasChanged = true;
updatedMetadata.shared_label_colors = [];
dispatch(
setDashboardMetadata({
shared_label_colors: [],
}),
);
}
// backward compatibility of map_label_colors
const hasMapLabelColors =
Object.keys(metadata.map_label_colors || {}).length > 0;
const hasMapLabelColors = !!metadata.map_label_colors;

let updatedScheme = metadata.color_scheme;
const categoricalSchemes = getCategoricalSchemeRegistry();
Expand All @@ -780,11 +793,14 @@ export const applyDashboardLabelsColorOnLoad = metadata => async dispatch => {
const defaultScheme = categoricalSchemes.defaultKey;
const fallbackScheme = defaultScheme?.toString() || 'supersetColors';
hasChanged = true;

updatedScheme = fallbackScheme;
updatedMetadata.color_scheme = updatedScheme;

dispatch(setColorScheme(updatedScheme));
dispatch(
setDashboardMetadata({
color_scheme: updatedScheme,
}),
);
}

// the stored color domain registry and fresh might differ at this point
Expand All @@ -795,24 +811,28 @@ export const applyDashboardLabelsColorOnLoad = metadata => async dispatch => {

if (!isEqual(freshColorSchemeDomain, currentColorSchemeDomain)) {
hasChanged = true;
updatedMetadata.color_scheme_domain = freshColorSchemeDomain;
dispatch(
setDashboardMetadata({
color_scheme_domain: freshColorSchemeDomain,
}),
);
}

// if color scheme is invalid or map is missing, apply a fresh color map
// if valid, apply the stored map to keep consistency across refreshes
const shouldGoFresh = !hasMapLabelColors || hasInvalidColorScheme;
applyColors(updatedMetadata, shouldGoFresh);
applyColors(metadata, shouldGoFresh);

if (shouldGoFresh) {
// a fresh color map has been applied
// needs to be stored for consistency
hasChanged = true;
updatedMetadata.map_label_colors =
getLabelsColorMapEntries(customLabelsColor);
dispatch(
setDashboardMetadata({
map_label_colors: getFreshLabelsColorMapEntries(customLabelsColor),
}),
);
}

if (hasChanged) {
dispatch(setDashboardMetadata(updatedMetadata));
dispatch(setDashboardLabelsColorMapSync());
}
} catch (e) {
Expand All @@ -832,19 +852,28 @@ export const ensureSyncedLabelsColorMap = metadata => (dispatch, getState) => {
const {
dashboardState: { labelsColorMapMustSync },
} = getState();
const updatedMetadata = { ...metadata };
const customLabelsColor = metadata.label_colors || {};
const isMapSynced = isLabelsColorMapSynced(metadata);
const mustSync = !isMapSynced;

if (mustSync) {
const freshestColorMapEntries =
getLabelsColorMapEntries(customLabelsColor);
updatedMetadata.map_label_colors = freshestColorMapEntries;
dispatch(setDashboardMetadata(updatedMetadata));
const fullLabelsColors = getDynamicLabelsColors(
metadata.map_label_colors || {},
customLabelsColor,
);
const freshColorMapEntries =
getFreshLabelsColorMapEntries(customLabelsColor);
const isMapSynced = isLabelsColorMapSynced(
fullLabelsColors,
freshColorMapEntries,
customLabelsColor,
);

if (!isMapSynced) {
dispatch(
setDashboardMetadata({
map_label_colors: freshColorMapEntries,
}),
);
}

if (mustSync && !labelsColorMapMustSync) {
if (!isMapSynced && !labelsColorMapMustSync) {
// prepare to persist the just applied labels color map
dispatch(setDashboardLabelsColorMapSync());
}
Expand All @@ -867,23 +896,24 @@ export const ensureSyncedSharedLabelsColors =
const {
dashboardState: { sharedLabelsColorsMustSync },
} = getState();
const updatedMetadata = { ...metadata };
const sharedLabelsColors = enforceSharedLabelsColorsArray(
metadata.shared_label_colors,
);
const freshLabelsColors = getFreshSharedLabels(
forceFresh ? [] : sharedLabelsColors,
);
const isSharedLabelsColorsSynced = isEqual(
sharedLabelsColors,
freshLabelsColors,
sortBy(sharedLabelsColors),
sortBy(freshLabelsColors),
);

const mustSync = !isSharedLabelsColorsSynced;

if (mustSync) {
updatedMetadata.shared_label_colors = freshLabelsColors;
dispatch(setDashboardMetadata(updatedMetadata));
dispatch(
setDashboardMetadata({
shared_label_colors: freshLabelsColors,
}),
);
}

if (mustSync && !sharedLabelsColorsMustSync) {
Expand All @@ -901,77 +931,78 @@ export const ensureSyncedSharedLabelsColors =
* @param {*} renderedChartIds - the charts that have finished rendering
* @returns void
*/
export const updateDashboardLabelsColor =
renderedChartIds => (dispatch, getState) => {
try {
const {
dashboardInfo: { metadata },
charts,
} = getState();
const colorScheme = metadata.color_scheme;
const labelsColorMapInstance = getLabelsColorMap();
const fullLabelsColors = metadata.map_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
// this logic looks for unique labels (not shared across multiple charts) of each rendered chart
// it applies a new color to those unique labels when the applied scheme is not up to date
// while leaving shared label colors and custom label colors intact for color consistency
const shouldReset = [];
if (renderedChartIds.length > 0) {
const sharedLabelsSet = new Set(sharedLabelsColors);
renderedChartIds.forEach(id => {
const chart = charts[id];
const formData = chart.form_data || chart.latestQueryFormData;
// ensure charts have their original color scheme always available
labelsColorMapInstance.setOwnColorScheme(
formData.slice_id,
formData.color_scheme,
);
export const updateDashboardLabelsColor = renderedChartIds => (_, getState) => {
try {
const {
dashboardInfo: { metadata },
charts,
} = getState();
const colorScheme = metadata.color_scheme;
const labelsColorMapInstance = getLabelsColorMap();
const sharedLabelsColors = enforceSharedLabelsColorsArray(
metadata.shared_label_colors,
);
const customLabelsColors = metadata.label_colors || {};
const fullLabelsColors = getDynamicLabelsColors(
metadata.map_label_colors || {},
customLabelsColors,
);

// if dashboard has a scheme, charts should ignore individual schemes
// thus following logic is inapplicable if a dashboard color scheme exists
if (colorScheme) return;
// for dashboards with no color scheme, the charts should always use their individual schemes
// this logic looks for unique labels (not shared across multiple charts) of each rendered chart
// it applies a new color to those unique labels when the applied scheme is not up to date
// while leaving shared label colors and custom label colors intact for color consistency
const shouldReset = [];
if (renderedChartIds.length > 0) {
const sharedLabelsSet = new Set(sharedLabelsColors);
renderedChartIds.forEach(id => {
const chart = charts[id];
const formData = chart.form_data || chart.latestQueryFormData;
// ensure charts have their original color scheme always available
labelsColorMapInstance.setOwnColorScheme(
formData.slice_id,
formData.color_scheme,
);

const chartColorScheme = formData.color_scheme;
const currentChartConfig = labelsColorMapInstance.chartsLabelsMap.get(
formData.slice_id,
);
const currentChartLabels = currentChartConfig?.labels || [];
const uniqueChartLabels = currentChartLabels.filter(
l =>
!sharedLabelsSet.has(l) && !customLabelsColors.hasOwnProperty(l),
);
// if dashboard has a scheme, charts should ignore individual schemes
// thus following logic is inapplicable if a dashboard color scheme exists
if (colorScheme) return;

// Map unique labels to colors
const uniqueChartLabelsColor = new Set(
uniqueChartLabels.map(l => fullLabelsColors[l]).filter(Boolean),
);
const chartColorScheme = formData.color_scheme;
const currentChartConfig = labelsColorMapInstance.chartsLabelsMap.get(
formData.slice_id,
);
const currentChartLabels = currentChartConfig?.labels || [];
const uniqueChartLabels = currentChartLabels.filter(
l => !sharedLabelsSet.has(l) && !customLabelsColors.hasOwnProperty(l),
);

const expectedColorsForChartScheme = new Set(
getColorSchemeDomain(chartColorScheme),
);
// Map unique labels to colors
const uniqueChartLabelsColor = new Set(
uniqueChartLabels.map(l => fullLabelsColors[l]).filter(Boolean),
);

// Check if any unique label color is not in the expected colors set
const shouldResetColors = [...uniqueChartLabelsColor].some(
color => !expectedColorsForChartScheme.has(color),
);
const expectedColorsForChartScheme = new Set(
getColorSchemeDomain(chartColorScheme),
);

// Only push uniqueChartLabels if they require resetting
if (shouldResetColors) shouldReset.push(...uniqueChartLabels);
});
}
// Check if any unique label color is not in the expected colors set
const shouldResetColors = [...uniqueChartLabelsColor].some(
color => !expectedColorsForChartScheme.has(color),
);

// an existing map is available, use mrge option
// to only apply colors to newly found labels
const shouldGoFresh = shouldReset.length > 0 ? shouldReset : false;
const shouldMerge = !shouldGoFresh;
// re-apply the color map first to get fresh maps accordingly
applyColors(metadata, shouldGoFresh, shouldMerge);
} catch (e) {
console.error('Failed to update colors for new charts and labels:', e);
// Only push uniqueChartLabels if they require resetting
if (shouldResetColors) shouldReset.push(...uniqueChartLabels);
});
}
};

// an existing map is available, use mrge option
// to only apply colors to newly found labels
const shouldGoFresh = shouldReset.length > 0 ? shouldReset : false;
const shouldMerge = !shouldGoFresh;
// re-apply the color map first to get fresh maps accordingly
applyColors(metadata, shouldGoFresh, shouldMerge);
} catch (e) {
console.error('Failed to update colors for new charts and labels:', e);
}
};
4 changes: 2 additions & 2 deletions superset-frontend/src/dashboard/components/PropertiesModal/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ import { loadTags } from 'src/components/Tags/utils';
import {
applyColors,
getColorNamespace,
getLabelsColorMapEntries,
getFreshLabelsColorMapEntries,
} from 'src/utils/colorScheme';
import getOwnerName from 'src/utils/getOwnerName';
import Owner from 'src/types/Owner';
Expand Down Expand Up @@ -369,7 +369,7 @@ const PropertiesModal = ({
dispatch(
setDashboardMetadata({
...updatedDashboardMetadata,
map_label_colors: getLabelsColorMapEntries(customLabelColors),
map_label_colors: getFreshLabelsColorMapEntries(customLabelColors),
}),
);

Expand Down
Loading

0 comments on commit a983319

Please sign in to comment.