diff --git a/.gitignore b/.gitignore index fce133adee4..ad38c493e57 100644 --- a/.gitignore +++ b/.gitignore @@ -130,7 +130,7 @@ superset/static/stats/statistics.html # LLM-related CLAUDE.local.md +PROJECT.md .aider* .claude_rc* .env.local -PROJECT.md diff --git a/LLMS.md b/LLMS.md index 22d58e691b0..d979f085f89 100644 --- a/LLMS.md +++ b/LLMS.md @@ -16,6 +16,7 @@ Apache Superset is a data visualization platform with Flask/Python backend and R - **Prefer integration tests** over Cypress end-to-end tests - **Cypress is last resort** - Actively moving away from Cypress - **Use Jest + React Testing Library** for component testing +- **Use `test()` instead of `describe()`** - Follow [avoid nesting when testing](https://kentcdodds.com/blog/avoid-nesting-when-youre-testing) principles ### Backend Type Safety - **Add type hints** - All new Python code needs proper typing diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index 0bd3a157ab6..d4e79f55487 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -277,6 +277,7 @@ "peerDependencies": { "ace-builds": "^1.41.0", "core-js": "^3.38.1", + "handlebars": "^4.7.8", "react-ace": "^10.1.0", "regenerator-runtime": "^0.14.1" } @@ -60630,6 +60631,7 @@ "dayjs": "^1.11.13", "dompurify": "^3.2.4", "fetch-retry": "^6.0.0", + "handlebars": "^4.7.8", "jed": "^1.1.1", "lodash": "^4.17.21", "math-expression-evaluator": "^2.0.6", @@ -62825,7 +62827,6 @@ "version": "0.20.3", "license": "Apache-2.0", "dependencies": { - "handlebars": "^4.7.8", "handlebars-group-by": "^1.0.1", "just-handlebars-helpers": "^1.0.19" }, @@ -62839,6 +62840,7 @@ "@superset-ui/core": "*", "ace-builds": "^1.4.14", "dayjs": "^1.11.13", + "handlebars": "^4.7.8", "lodash": "^4.17.11", "react": "^17.0.2", "react-ace": "^10.1.0", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index 2bbea79085f..dbfb745ee7a 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -341,6 +341,7 @@ "peerDependencies": { "ace-builds": "^1.41.0", "core-js": "^3.38.1", + "handlebars": "^4.7.8", "react-ace": "^10.1.0", "regenerator-runtime": "^0.14.1" }, diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/sections/index.ts b/superset-frontend/packages/superset-ui-chart-controls/src/sections/index.ts index caa07faa9c4..f3354f6f502 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/sections/index.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/sections/index.ts @@ -24,3 +24,4 @@ export * from './forecastInterval'; export * from './chartTitle'; export * from './echartsTimeSeriesQuery'; export * from './timeComparison'; +export * from './matrixify'; diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/sections/matrixify.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/sections/matrixify.tsx new file mode 100644 index 00000000000..0054d5c136c --- /dev/null +++ b/superset-frontend/packages/superset-ui-chart-controls/src/sections/matrixify.tsx @@ -0,0 +1,139 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { t } from '@superset-ui/core'; +import { ControlPanelSectionConfig } from '../types'; + +export const matrixifyEnableSection: ControlPanelSectionConfig = { + label: t('Enable matrixify'), + expanded: true, + controlSetRows: [ + [ + { + name: 'matrixify_enabled', + config: { + type: 'CheckboxControl', + label: t('Enable matrixify'), + default: false, + renderTrigger: true, + description: t( + 'Transform this chart into a matrix/grid of charts based on dimensions or metrics', + ), + }, + }, + ], + ], + tabOverride: 'matrixify', +}; + +export const matrixifySection: ControlPanelSectionConfig = { + label: t('Matrixify'), + expanded: false, + visibility: ({ controls }) => controls?.matrixify_enabled?.value === true, + controlSetRows: [ + [ + { + name: 'matrixify_row_height', + config: { + type: 'TextControl', + label: t('Row height'), + default: 300, + isInt: true, + renderTrigger: true, + description: t('Height of each row in pixels'), + }, + }, + { + name: 'matrixify_fit_columns_dynamically', + config: { + type: 'CheckboxControl', + label: t('Fit columns dynamically'), + default: true, + renderTrigger: true, + description: t('Automatically adjust column width based on content'), + }, + }, + ], + [ + { + name: 'matrixify_charts_per_row', + config: { + type: 'TextControl', + label: t('Charts per row'), + default: 3, + isInt: true, + renderTrigger: true, + description: t( + 'Number of charts per row when not fitting dynamically', + ), + visibility: ({ controls }) => + !controls?.matrixify_fit_columns_dynamically?.value, + }, + }, + ], + [ + { + name: 'matrixify_cell_title_template', + config: { + type: 'TextControl', + label: t('Cell title template'), + default: '', + description: t( + 'Template for cell titles. Use Handlebars templating syntax (a popular templating library that uses double curly brackets for variable substitution): {{row}}, {{column}}, {{rowLabel}}, {{columnLabel}}', + ), + placeholder: '{{rowLabel}} by {{colLabel}}', + }, + }, + ], + ], + tabOverride: 'customize', +}; + +export const matrixifyRowSection: ControlPanelSectionConfig = { + label: t('Vertical layout'), + expanded: false, + visibility: ({ controls }) => controls?.matrixify_enabled?.value === true, + controlSetRows: [ + ['matrixify_show_row_labels'], + ['matrixify_mode_rows'], + ['matrixify_rows'], + ['matrixify_dimension_rows'], + ['matrixify_dimension_selection_mode_rows'], + ['matrixify_topn_value_rows'], + ['matrixify_topn_metric_rows'], + ['matrixify_topn_order_rows'], + ], + tabOverride: 'data', +}; + +export const matrixifyColumnSection: ControlPanelSectionConfig = { + label: t('Horizontal layout'), + expanded: false, + visibility: ({ controls }) => controls?.matrixify_enabled?.value === true, + controlSetRows: [ + ['matrixify_show_column_headers'], + ['matrixify_mode_columns'], + ['matrixify_columns'], + ['matrixify_dimension_columns'], + ['matrixify_dimension_selection_mode_columns'], + ['matrixify_topn_value_columns'], + ['matrixify_topn_metric_columns'], + ['matrixify_topn_order_columns'], + ], + tabOverride: 'data', +}; diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/matrixifyControls.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/matrixifyControls.tsx new file mode 100644 index 00000000000..fcc473b5878 --- /dev/null +++ b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/matrixifyControls.tsx @@ -0,0 +1,264 @@ +/* eslint-disable camelcase */ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { t } from '@superset-ui/core'; +import { SharedControlConfig } from '../types'; +import { dndAdhocMetricControl } from './dndControls'; + +/** + * Matrixify control definitions + * Controls for transforming charts into matrix/grid layouts + */ + +// Initialize the controls object that will be populated dynamically +const matrixifyControls: Record> = {}; + +// Dynamically add axis-specific controls (rows and columns) +['columns', 'rows'].forEach(axisParam => { + const axis = axisParam; // Capture the value in a local variable + + matrixifyControls[`matrixify_mode_${axis}`] = { + type: 'RadioButtonControl', + label: t(`Metrics / Dimensions`), + default: 'metrics', + options: [ + ['metrics', t('Metrics')], + ['dimensions', t('Dimension members')], + ], + renderTrigger: true, + }; + + matrixifyControls[`matrixify_${axis}`] = { + ...dndAdhocMetricControl, + label: t(`Metrics`), + multi: true, + validators: [], // Not required + // description: t(`Select metrics for ${axis}`), + renderTrigger: true, + visibility: ({ controls }) => + controls?.[`matrixify_mode_${axis}`]?.value === 'metrics', + }; + + // Combined dimension and values control + matrixifyControls[`matrixify_dimension_${axis}`] = { + type: 'MatrixifyDimensionControl', + label: t(`Dimension selection`), + description: t(`Select dimension and values`), + default: { dimension: '', values: [] }, + validators: [], // Not required + renderTrigger: true, + shouldMapStateToProps: (prevState, state) => { + // Recalculate when any relevant form_data field changes + const fieldsToCheck = [ + `matrixify_topn_value_${axis}`, + `matrixify_topn_metric_${axis}`, + `matrixify_topn_order_${axis}`, + `matrixify_dimension_selection_mode_${axis}`, + ]; + + return fieldsToCheck.some( + field => prevState?.form_data?.[field] !== state?.form_data?.[field], + ); + }, + mapStateToProps: ({ datasource, controls, form_data }) => { + // Helper to get value from form_data or controls + const getValue = (key: string, defaultValue?: any) => + form_data?.[key] ?? controls?.[key]?.value ?? defaultValue; + + return { + datasource, + selectionMode: getValue( + `matrixify_dimension_selection_mode_${axis}`, + 'members', + ), + topNMetric: getValue(`matrixify_topn_metric_${axis}`), + topNValue: getValue(`matrixify_topn_value_${axis}`), + topNOrder: getValue(`matrixify_topn_order_${axis}`), + formData: form_data, + }; + }, + visibility: ({ controls }) => + controls?.[`matrixify_mode_${axis}`]?.value === 'dimensions', + }; + + // Dimension picker for TopN mode (just dimension, no values) + // NOTE: This is now handled by matrixify_dimension control, so hiding it + matrixifyControls[`matrixify_topn_dimension_${axis}`] = { + type: 'SelectControl', + label: t('Dimension'), + description: t(`Select dimension for Top N`), + default: null, + mapStateToProps: ({ datasource }) => ({ + choices: + datasource?.columns?.map((col: any) => [ + col.column_name, + col.column_name, + ]) || [], + }), + renderTrigger: true, + // Hide this control - now handled by matrixify_dimension control + visibility: () => false, + }; + + // Add selection mode control (Dimension Members vs TopN) + matrixifyControls[`matrixify_dimension_selection_mode_${axis}`] = { + type: 'RadioButtonControl', + label: t(`Selection method`), + default: 'members', + options: [ + ['members', t('Dimension members')], + ['topn', t('Top n')], + ], + renderTrigger: true, + visibility: ({ controls }) => + controls?.[`matrixify_mode_${axis}`]?.value === 'dimensions', + }; + + // TopN controls + matrixifyControls[`matrixify_topn_value_${axis}`] = { + type: 'TextControl', + label: t(`Number of top values`), + description: t(`How many top values to select`), + default: 10, + isInt: true, + visibility: ({ controls }) => + controls?.[`matrixify_mode_${axis}`]?.value === 'dimensions' && + controls?.[`matrixify_dimension_selection_mode_${axis}`]?.value === + 'topn', + }; + + matrixifyControls[`matrixify_topn_metric_${axis}`] = { + ...dndAdhocMetricControl, + label: t(`Metric for ordering`), + multi: false, + validators: [], // Not required + description: t(`Metric to use for ordering Top N values`), + visibility: ({ controls }) => + controls?.[`matrixify_mode_${axis}`]?.value === 'dimensions' && + controls?.[`matrixify_dimension_selection_mode_${axis}`]?.value === + 'topn', + }; + + matrixifyControls[`matrixify_topn_order_${axis}`] = { + type: 'RadioButtonControl', + label: t(`Sort order`), + default: 'desc', + options: [ + ['asc', t('Ascending')], + ['desc', t('Descending')], + ], + visibility: ({ controls }) => + controls?.[`matrixify_mode_${axis}`]?.value === 'dimensions' && + controls?.[`matrixify_dimension_selection_mode_${axis}`]?.value === + 'topn', + }; +}); + +// Grid layout controls (added once, not per axis) +matrixifyControls.matrixify_row_height = { + type: 'TextControl', + label: t('Row height'), + description: t('Height of each row in pixels'), + default: 300, + isInt: true, + validators: [], + renderTrigger: true, +}; + +matrixifyControls.matrixify_fit_columns_dynamically = { + type: 'CheckboxControl', + label: t('Fit columns dynamically'), + description: t('Automatically adjust column width based on available space'), + default: true, + renderTrigger: true, +}; + +matrixifyControls.matrixify_charts_per_row = { + type: 'SelectControl', + label: t('Charts per row'), + description: t('Number of charts to display per row'), + default: 4, + choices: [ + [1, '1'], + [2, '2'], + [3, '3'], + [4, '4'], + [5, '5'], + [6, '6'], + [8, '8'], + [10, '10'], + [12, '12'], + ], + freeForm: true, + clearable: false, + renderTrigger: true, + visibility: ({ controls }) => + !controls?.matrixify_fit_columns_dynamically?.value, +}; + +// Main enable control +matrixifyControls.matrixify_enabled = { + type: 'CheckboxControl', + label: t('Enable matrixify'), + description: t( + 'Transform this chart into a matrix/grid of charts based on dimensions or metrics', + ), + default: false, + renderTrigger: true, +}; + +// Cell title control for Matrixify +matrixifyControls.matrixify_cell_title_template = { + type: 'TextControl', + label: t('Title'), + description: t( + 'Customize cell titles using Handlebars template syntax. Available variables: {{rowLabel}}, {{colLabel}}', + ), + default: '', + renderTrigger: true, + visibility: ({ controls }) => + (controls?.matrixify_mode_rows?.value || + controls?.matrixify_mode_columns?.value) !== undefined, +}; + +// Matrix display controls +matrixifyControls.matrixify_show_row_labels = { + type: 'CheckboxControl', + label: t('Show row labels'), + description: t('Display labels for each row on the left side of the matrix'), + default: true, + renderTrigger: true, + visibility: ({ controls }) => + (controls?.matrixify_mode_rows?.value || + controls?.matrixify_mode_columns?.value) !== undefined, +}; + +matrixifyControls.matrixify_show_column_headers = { + type: 'CheckboxControl', + label: t('Show column headers'), + description: t('Display headers for each column at the top of the matrix'), + default: true, + renderTrigger: true, + visibility: ({ controls }) => + (controls?.matrixify_mode_rows?.value || + controls?.matrixify_mode_columns?.value) !== undefined, +}; + +export { matrixifyControls }; diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx index ae359a70f73..76730673bea 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx @@ -17,7 +17,6 @@ * specific language governing permissions and limitations * under the License. */ - /** * This file exports all controls available for use in chart plugins internal to Superset. * It is not recommended to use the controls here for any third-party plugins. @@ -86,6 +85,7 @@ import { dndTooltipColumnsControl, dndTooltipMetricsControl, } from './dndControls'; +import { matrixifyControls } from './matrixifyControls'; const categoricalSchemeRegistry = getCategoricalSchemeRegistry(); const sequentialSchemeRegistry = getSequentialSchemeRegistry(); @@ -427,7 +427,7 @@ const order_by_cols: SharedControlConfig<'SelectControl'> = { resetOnHide: false, }; -export default { +const sharedControls: Record> = { metrics: dndAdhocMetricsControl, metric: dndAdhocMetricControl, datasource: datasourceControl, @@ -472,4 +472,9 @@ export default { currency_format, sort_by_metric, order_by_cols, + + // Add all Matrixify controls + ...matrixifyControls, }; + +export default sharedControls; diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts index 429d3e8fc33..ed83551df2c 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts @@ -190,7 +190,7 @@ export type InternalControlType = // eslint-disable-next-line @typescript-eslint/no-explicit-any export type ControlType = InternalControlType | ComponentType; -export type TabOverride = 'data' | 'customize' | boolean; +export type TabOverride = 'data' | 'customize' | 'matrixify' | boolean; /** * Control config specifying how chart controls appear in the control panel, all diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/shared-controls/shouldMapStateToProps.test.tsx b/superset-frontend/packages/superset-ui-chart-controls/test/shared-controls/shouldMapStateToProps.test.tsx new file mode 100644 index 00000000000..8e6bff45fbe --- /dev/null +++ b/superset-frontend/packages/superset-ui-chart-controls/test/shared-controls/shouldMapStateToProps.test.tsx @@ -0,0 +1,393 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { ControlPanelState } from '../../src/types'; + +// Mock the utilities to avoid complex dependencies +jest.mock('../../src/utils', () => ({ + formatSelectOptions: jest.fn((options: any[]) => + options.map((opt: any) => [opt, opt]), + ), + displayTimeRelatedControls: jest.fn(() => true), + getColorControlsProps: jest.fn(() => ({})), + D3_FORMAT_OPTIONS: [], + D3_FORMAT_DOCS: '', + D3_TIME_FORMAT_OPTIONS: [], + D3_TIME_FORMAT_DOCS: '', + DEFAULT_TIME_FORMAT: '%Y-%m-%d', + DEFAULT_NUMBER_FORMAT: '', +})); + +// Mock shared controls +const mockSharedControls = { + matrixify_dimension_x: { + shouldMapStateToProps: ( + prevState: ControlPanelState, + state: ControlPanelState, + ) => { + const fieldsToCheck = [ + 'matrixify_topn_value_x', + 'matrixify_topn_metric_x', + 'matrixify_topn_order_x', + 'matrixify_dimension_selection_mode_x', + ]; + return fieldsToCheck.some( + field => prevState?.form_data?.[field] !== state?.form_data?.[field], + ); + }, + mapStateToProps: ({ datasource, controls, form_data }: any) => { + const getValue = (key: string, defaultValue?: any) => + form_data?.[key] ?? controls?.[key]?.value ?? defaultValue; + + return { + datasource, + selectionMode: getValue( + 'matrixify_dimension_selection_mode_x', + 'members', + ), + topNMetric: getValue('matrixify_topn_metric_x'), + topNValue: getValue('matrixify_topn_value_x'), + topNOrder: getValue('matrixify_topn_order_x'), + formData: form_data, + }; + }, + }, + matrixify_dimension_y: { + shouldMapStateToProps: ( + prevState: ControlPanelState, + state: ControlPanelState, + ) => { + const fieldsToCheck = [ + 'matrixify_topn_value_y', + 'matrixify_topn_metric_y', + 'matrixify_topn_order_y', + 'matrixify_dimension_selection_mode_y', + ]; + return fieldsToCheck.some( + field => prevState?.form_data?.[field] !== state?.form_data?.[field], + ); + }, + mapStateToProps: ({ datasource, controls, form_data }: any) => { + const getValue = (key: string, defaultValue?: any) => + form_data?.[key] ?? controls?.[key]?.value ?? defaultValue; + + return { + datasource, + selectionMode: getValue( + 'matrixify_dimension_selection_mode_y', + 'members', + ), + topNMetric: getValue('matrixify_topn_metric_y'), + topNValue: getValue('matrixify_topn_value_y'), + topNOrder: getValue('matrixify_topn_order_y'), + formData: form_data, + }; + }, + }, +}; + +const createMockState = ( + formData: any = {}, + controls: any = {}, +): ControlPanelState => ({ + slice: { slice_id: 123 }, + form_data: formData, + datasource: null, + controls, + common: {}, + metadata: {}, +}); + +const createMockControlState = (value: any = null) => ({ value }); + +test('matrixify_dimension_x should return true when topN value changes', () => { + const control = mockSharedControls.matrixify_dimension_x; + + const prevState = createMockState({ + matrixify_topn_value_x: 5, + matrixify_topn_metric_x: 'metric1', + matrixify_topn_order_x: 'desc', + matrixify_dimension_selection_mode_x: 'topn', + }); + + const nextState = createMockState({ + matrixify_topn_value_x: 10, // Changed + matrixify_topn_metric_x: 'metric1', + matrixify_topn_order_x: 'desc', + matrixify_dimension_selection_mode_x: 'topn', + }); + + expect(control.shouldMapStateToProps!(prevState, nextState)).toBe(true); +}); + +test('matrixify_dimension_x should return true when topN metric changes', () => { + const control = mockSharedControls.matrixify_dimension_x; + + const prevState = createMockState({ + matrixify_topn_value_x: 5, + matrixify_topn_metric_x: 'metric1', + matrixify_topn_order_x: 'desc', + matrixify_dimension_selection_mode_x: 'topn', + }); + + const nextState = createMockState({ + matrixify_topn_value_x: 5, + matrixify_topn_metric_x: 'metric2', // Changed + matrixify_topn_order_x: 'desc', + matrixify_dimension_selection_mode_x: 'topn', + }); + + expect(control.shouldMapStateToProps!(prevState, nextState)).toBe(true); +}); + +test('matrixify_dimension_x should return true when topN order changes', () => { + const control = mockSharedControls.matrixify_dimension_x; + + const prevState = createMockState({ + matrixify_topn_value_x: 5, + matrixify_topn_metric_x: 'metric1', + matrixify_topn_order_x: 'desc', + matrixify_dimension_selection_mode_x: 'topn', + }); + + const nextState = createMockState({ + matrixify_topn_value_x: 5, + matrixify_topn_metric_x: 'metric1', + matrixify_topn_order_x: 'asc', // Changed + matrixify_dimension_selection_mode_x: 'topn', + }); + + expect(control.shouldMapStateToProps!(prevState, nextState)).toBe(true); +}); + +test('matrixify_dimension_x should return true when selection mode changes', () => { + const control = mockSharedControls.matrixify_dimension_x; + + const prevState = createMockState({ + matrixify_topn_value_x: 5, + matrixify_topn_metric_x: 'metric1', + matrixify_topn_order_x: 'desc', + matrixify_dimension_selection_mode_x: 'topn', + }); + + const nextState = createMockState({ + matrixify_topn_value_x: 5, + matrixify_topn_metric_x: 'metric1', + matrixify_topn_order_x: 'desc', + matrixify_dimension_selection_mode_x: 'members', // Changed + }); + + expect(control.shouldMapStateToProps!(prevState, nextState)).toBe(true); +}); + +test('matrixify_dimension_x should return false when no relevant fields change', () => { + const control = mockSharedControls.matrixify_dimension_x; + + const prevState = createMockState({ + matrixify_topn_value_x: 5, + matrixify_topn_metric_x: 'metric1', + matrixify_topn_order_x: 'desc', + matrixify_dimension_selection_mode_x: 'topn', + unrelated_field: 'value1', + }); + + const nextState = createMockState({ + matrixify_topn_value_x: 5, + matrixify_topn_metric_x: 'metric1', + matrixify_topn_order_x: 'desc', + matrixify_dimension_selection_mode_x: 'topn', + unrelated_field: 'value2', // Changed, but not relevant + }); + + expect(control.shouldMapStateToProps!(prevState, nextState)).toBe(false); +}); + +test('matrixify_dimension_x should return false when states are identical', () => { + const control = mockSharedControls.matrixify_dimension_x; + + const state = createMockState({ + matrixify_topn_value_x: 5, + matrixify_topn_metric_x: 'metric1', + matrixify_topn_order_x: 'desc', + matrixify_dimension_selection_mode_x: 'topn', + }); + + expect(control.shouldMapStateToProps!(state, state)).toBe(false); +}); + +test('matrixify_dimension_x should handle missing form_data gracefully', () => { + const control = mockSharedControls.matrixify_dimension_x; + + const prevState = createMockState(); // No form_data + const nextState = createMockState({ + matrixify_topn_value_x: 5, + }); + + expect(control.shouldMapStateToProps!(prevState, nextState)).toBe(true); +}); + +test('matrixify_dimension_x should handle undefined values gracefully', () => { + const control = mockSharedControls.matrixify_dimension_x; + + const prevState = createMockState({ + matrixify_topn_value_x: undefined, + matrixify_topn_metric_x: null, + }); + + const nextState = createMockState({ + matrixify_topn_value_x: 5, + matrixify_topn_metric_x: 'metric1', + }); + + expect(control.shouldMapStateToProps!(prevState, nextState)).toBe(true); +}); + +test('matrixify_dimension_y should check y-axis specific fields', () => { + const control = mockSharedControls.matrixify_dimension_y; + + const prevState = createMockState({ + matrixify_topn_value_y: 5, + matrixify_topn_metric_y: 'metric1', + }); + + const nextState = createMockState({ + matrixify_topn_value_y: 10, // Changed + matrixify_topn_metric_y: 'metric1', + }); + + expect(control.shouldMapStateToProps!(prevState, nextState)).toBe(true); +}); + +test('matrixify_dimension_y should not trigger on x-axis changes', () => { + const control = mockSharedControls.matrixify_dimension_y; + + const prevState = createMockState({ + matrixify_topn_value_x: 5, // x-axis field + matrixify_topn_value_y: 5, // y-axis field (unchanged) + }); + + const nextState = createMockState({ + matrixify_topn_value_x: 10, // x-axis field changed + matrixify_topn_value_y: 5, // y-axis field (unchanged) + }); + + expect(control.shouldMapStateToProps!(prevState, nextState)).toBe(false); +}); + +test('mapStateToProps should map form_data values correctly', () => { + const control = mockSharedControls.matrixify_dimension_x; + + const state = createMockState({ + matrixify_dimension_selection_mode_x: 'topn', + matrixify_topn_metric_x: 'metric1', + matrixify_topn_value_x: 10, + matrixify_topn_order_x: 'desc', + }); + + const mockDatasource: any = { id: 1, columns: [] }; + state.datasource = mockDatasource; + + const result = control.mapStateToProps!(state); + + expect(result).toEqual({ + datasource: mockDatasource, + selectionMode: 'topn', + topNMetric: 'metric1', + topNValue: 10, + topNOrder: 'desc', + formData: state.form_data, + }); +}); + +test('mapStateToProps should fall back to control values when form_data is missing', () => { + const control = mockSharedControls.matrixify_dimension_x; + + const state = createMockState( + {}, // Empty form_data + { + matrixify_dimension_selection_mode_x: createMockControlState('members'), + matrixify_topn_metric_x: createMockControlState('metric2'), + matrixify_topn_value_x: createMockControlState(15), + }, + ); + + const result = control.mapStateToProps!(state); + + expect(result.selectionMode).toBe('members'); + expect(result.topNMetric).toBe('metric2'); + expect(result.topNValue).toBe(15); +}); + +test('mapStateToProps should use default values when both form_data and controls are missing', () => { + const control = mockSharedControls.matrixify_dimension_x; + + const state = createMockState({}, {}); + + const result = control.mapStateToProps!(state); + + expect(result.selectionMode).toBe('members'); // Default value + expect(result.topNMetric).toBeUndefined(); + expect(result.topNValue).toBeUndefined(); + expect(result.topNOrder).toBeUndefined(); +}); + +test('mapStateToProps should prioritize form_data over control values', () => { + const control = mockSharedControls.matrixify_dimension_x; + + const state = createMockState( + { + matrixify_dimension_selection_mode_x: 'topn', // form_data value + }, + { + matrixify_dimension_selection_mode_x: createMockControlState('members'), // control value + }, + ); + + const result = control.mapStateToProps!(state); + + expect(result.selectionMode).toBe('topn'); // Should use form_data value +}); + +test('should efficiently check only relevant fields', () => { + const control = mockSharedControls.matrixify_dimension_x; + + const prevState = createMockState({ + // Many fields, only some relevant + field1: 'value1', + field2: 'value2', + matrixify_topn_value_x: 5, // Relevant + field3: 'value3', + matrixify_topn_metric_x: 'metric1', // Relevant + field4: 'value4', + matrixify_other_control: 'value5', + }); + + const nextState = createMockState({ + field1: 'value1_changed', // Not relevant + field2: 'value2_changed', // Not relevant + matrixify_topn_value_x: 5, // Relevant, unchanged + field3: 'value3_changed', // Not relevant + matrixify_topn_metric_x: 'metric1', // Relevant, unchanged + field4: 'value4_changed', // Not relevant + matrixify_other_control: 'value5_changed', // Not relevant + }); + + // Should return false because no relevant fields changed + expect(control.shouldMapStateToProps!(prevState, nextState)).toBe(false); +}); diff --git a/superset-frontend/packages/superset-ui-core/package.json b/superset-frontend/packages/superset-ui-core/package.json index 4a7fe8c1674..9b496187f68 100644 --- a/superset-frontend/packages/superset-ui-core/package.json +++ b/superset-frontend/packages/superset-ui-core/package.json @@ -44,6 +44,7 @@ "d3-time-format": "^4.1.0", "dompurify": "^3.2.4", "fetch-retry": "^6.0.0", + "handlebars": "^4.7.8", "jed": "^1.1.1", "lodash": "^4.17.21", "math-expression-evaluator": "^2.0.6", diff --git a/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridCell.test.tsx b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridCell.test.tsx new file mode 100644 index 00000000000..08205c0b9b0 --- /dev/null +++ b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridCell.test.tsx @@ -0,0 +1,212 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { render, screen } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { ThemeProvider, supersetTheme } from '../../..'; +import MatrixifyGridCell from './MatrixifyGridCell'; +import { MatrixifyGridCell as MatrixifyGridCellType } from '../../types/matrixify'; + +// Mock StatefulChart component +jest.mock('../StatefulChart', () => { + /* eslint-disable no-restricted-syntax, global-require, @typescript-eslint/no-var-requires */ + const React = require('react'); + /* eslint-enable no-restricted-syntax, global-require, @typescript-eslint/no-var-requires */ + + return { + __esModule: true, + default: ({ formData, height, width }: any) => + React.createElement( + 'div', + { + 'data-testid': 'superchart', + 'data-viz-type': formData.viz_type, + style: { height, width }, + }, + 'SuperChart Mock', + ), + }; +}); + +const mockDatasource = { + id: 1, + type: 'table', + uid: '1__table', + datasource_name: 'test_datasource', + table_name: 'test_table', + database: { + id: 1, + name: 'test_database', + }, +}; + +const mockCell: MatrixifyGridCellType = { + id: 'matrixify-0-0', + row: 0, + col: 0, + rowLabel: 'Revenue', + colLabel: 'Q1 2024', + title: 'Revenue - Q1 2024', + formData: { + viz_type: 'big_number_total', + metrics: ['revenue'], + adhoc_filters: [], + }, +}; + +const defaultProps = { + cell: mockCell, + datasource: mockDatasource, + rowHeight: 200, +}; + +const renderWithTheme = (component: React.ReactElement) => + render({component}); + +test('should render the cell with title', () => { + renderWithTheme(); + + expect(screen.getByText('Revenue - Q1 2024')).toBeInTheDocument(); +}); + +test('should render the cell without title when not provided', () => { + const cellWithoutTitle = { + ...mockCell, + title: undefined, + }; + + renderWithTheme( + , + ); + + expect(screen.queryByText('Revenue - Q1 2024')).not.toBeInTheDocument(); +}); + +test('should render SuperChart with correct props', () => { + renderWithTheme(); + + const superChart = screen.getByText('SuperChart Mock'); + expect(superChart).toBeInTheDocument(); + expect(superChart).toHaveAttribute('data-viz-type', 'big_number_total'); + expect(superChart).toHaveStyle({ height: '100%', width: '100%' }); +}); + +test('should calculate chart height correctly with title', () => { + renderWithTheme(); + + const superChart = screen.getByText('SuperChart Mock'); + // StatefulChart uses 100% height within the chart wrapper + expect(superChart).toHaveStyle({ height: '100%' }); +}); + +test('should calculate chart height correctly without title', () => { + const cellWithoutTitle = { + ...mockCell, + title: undefined, + }; + + renderWithTheme( + , + ); + + const superChart = screen.getByText('SuperChart Mock'); + // StatefulChart uses 100% height within the chart wrapper + expect(superChart).toHaveStyle({ height: '100%' }); +}); + +test('should apply correct styling to container', () => { + const { container } = renderWithTheme( + , + ); + + const cellContainer = container.firstChild as HTMLElement; + expect(cellContainer).toHaveStyle({ + height: '100%', + display: 'flex', + }); +}); + +test('should apply correct styling to title', () => { + renderWithTheme(); + + const title = screen.getByText('Revenue - Q1 2024'); + expect(title).toHaveStyle({ + overflow: 'hidden', + }); +}); + +test('should handle different viz types', () => { + const cellWithLineChart = { + ...mockCell, + formData: { + ...mockCell.formData, + viz_type: 'line', + }, + }; + + renderWithTheme( + , + ); + + const superChart = screen.getByText('SuperChart Mock'); + expect(superChart).toHaveAttribute('data-viz-type', 'line'); +}); + +test('should pass through additional formData properties', () => { + const cellWithExtraProps = { + ...mockCell, + formData: { + ...mockCell.formData, + time_range: 'Last month', + row_limit: 100, + }, + }; + + renderWithTheme( + , + ); + + // The SuperChart mock would receive these props + expect(screen.getByText('SuperChart Mock')).toBeInTheDocument(); +}); + +test('should handle small cell dimensions', () => { + renderWithTheme(); + + const superChart = screen.getByText('SuperChart Mock'); + const cellContainer = superChart.parentElement?.parentElement; + expect(cellContainer).toHaveStyle({ height: '100%' }); + + // StatefulChart uses 100% dimensions within its wrapper + expect(superChart).toHaveStyle({ height: '100%', width: '100%' }); +}); + +test('should handle empty cell data gracefully', () => { + const emptyCell = { + ...mockCell, + rowLabel: '', + colLabel: '', + title: '', + }; + + renderWithTheme(); + + // Should still render but with empty title + expect(screen.getByText('SuperChart Mock')).toBeInTheDocument(); +}); diff --git a/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridCell.tsx b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridCell.tsx new file mode 100644 index 00000000000..838460d2c35 --- /dev/null +++ b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridCell.tsx @@ -0,0 +1,198 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { memo, useMemo } from 'react'; +import { styled, useTheme } from '../../../theme'; +import { MatrixifyGridCell as GridCellData } from '../../types/matrixify'; +import StatefulChart from '../StatefulChart'; + +const CellContainer = styled.div` + height: 100%; + display: flex; + flex-direction: column; + border: 1px solid ${({ theme }) => theme.colorBorder}; + border-radius: ${({ theme }) => theme.borderRadius}px; + background-color: ${({ theme }) => theme.colorBgContainer}; + overflow: hidden; +`; + +const CellHeader = styled.div` + flex-shrink: 0; + padding: ${({ theme }) => theme.sizeUnit}px + ${({ theme }) => theme.sizeUnit * 2}px; + background-color: ${({ theme }) => theme.colorFillAlter}; + border-bottom: 1px solid ${({ theme }) => theme.colorBorder}; + font-size: ${({ theme }) => theme.fontSizeSM}px; + font-weight: ${({ theme }) => theme.fontWeightStrong}; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; +`; + +const ChartWrapper = styled.div` + flex: 1; + min-height: 0; + padding: 0; + position: relative; + + /* Remove any padding/margins that might be causing title height issues */ + & .chart-container { + padding-top: 0 !important; + } + + /* Target title elements inside the chart container */ + & .superchart-container .header-title, + & .superchart-container [class*='title'] { + display: none !important; + } +`; + +const NoDataMessage = styled.div<{ theme: any }>` + position: absolute; + top: 50%; + left: 50%; + transform: translate(-50%, -50%); + color: ${({ theme }) => theme.colorTextQuaternary}; + font-size: ${({ theme }) => theme.fontSizeSM}px; + text-align: center; + user-select: none; +`; + +interface MatrixifyGridCellProps { + cell: GridCellData; + rowHeight: number; + datasource?: any; + hooks?: any; +} + +// Simple No Data component for matrix cells +const MatrixNoDataComponent = () => { + const theme = useTheme(); + return No data; +}; + +/** + * Individual grid cell component - memoized to prevent unnecessary re-renders + */ +const MatrixifyGridCell = memo( + ({ cell, rowHeight, datasource, hooks }: MatrixifyGridCellProps) => { + // Use computed title from template (will be empty string if no template) + const cellLabel = cell.title || ''; + + // Only show label if it has content + const showLabel = cellLabel && cellLabel.trim() !== ''; + + // Create enhanced hooks that merge cell filters with drill filters + const enhancedHooks = useMemo(() => { + if (!hooks) return undefined; + + // Create a new hooks object with wrapped onContextMenu + const wrappedHooks = { ...hooks }; + + if (hooks.onContextMenu) { + wrappedHooks.onContextMenu = ( + offsetX: number, + offsetY: number, + filters?: any, + ) => { + // Get the cell's adhoc filters + const cellFilters = cell.formData.adhoc_filters || []; + + // Merge the cell filters with any drill filters + const enhancedFilters = { + ...filters, + // Add cell-specific context to help identify this is from a matrix cell + matrixifyContext: { + rowLabel: cell.rowLabel, + colLabel: cell.colLabel, + row: cell.row, + col: cell.col, + // Include the cell's filters so they can be applied to drill operations + cellFilters, + // Include the cell's formData which has adhoc_filters for drill-to-detail + cellFormData: cell.formData, + }, + }; + + // Call the original handler with enhanced filters + hooks.onContextMenu(offsetX, offsetY, enhancedFilters); + }; + } + + return wrappedHooks; + }, [hooks, cell]); + + return ( + + {showLabel && {cellLabel}} + + + + + ); + }, + // Custom comparison function to prevent unnecessary re-renders + // Returns true to skip re-render, false to re-render + (prevProps, nextProps) => { + // Always re-render if formData changes + if ( + JSON.stringify(prevProps.cell.formData) !== + JSON.stringify(nextProps.cell.formData) + ) { + return false; + } + + // Re-render if rowHeight changes + if (prevProps.rowHeight !== nextProps.rowHeight) { + return false; + } + + // Re-render if cell position changes (shouldn't happen, but just in case) + if (prevProps.cell.id !== nextProps.cell.id) { + return false; + } + + // Re-render if title changes + if (prevProps.cell.title !== nextProps.cell.title) { + return false; + } + + // Skip re-render if nothing important changed + return true; + }, +); + +MatrixifyGridCell.displayName = 'MatrixifyGridCell'; + +export default MatrixifyGridCell; diff --git a/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridGenerator.test.ts b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridGenerator.test.ts new file mode 100644 index 00000000000..5a66173718f --- /dev/null +++ b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridGenerator.test.ts @@ -0,0 +1,320 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { generateMatrixifyGrid } from './MatrixifyGridGenerator'; +import { AdhocMetric } from '../../../query/types/Metric'; + +// Use 'any' to bypass strict typing in tests +type TestFormData = any; + +const createAdhocMetric = (label: string): AdhocMetric => ({ + expressionType: 'SIMPLE', + column: { column_name: 'value' }, + aggregate: 'SUM', + label, +}); + +const createSqlMetric = (label: string, sql: string): AdhocMetric => ({ + expressionType: 'SQL', + sqlExpression: sql, + label, +}); + +const baseFormData: TestFormData = { + viz_type: 'table', + datasource: '1__table', + matrixify_mode_rows: 'metrics', + matrixify_mode_columns: 'metrics', + matrixify_rows: [createAdhocMetric('Revenue'), createAdhocMetric('Profit')], + matrixify_columns: [ + createSqlMetric('Q1', 'SUM(CASE WHEN quarter = 1 THEN value END)'), + createSqlMetric('Q2', 'SUM(CASE WHEN quarter = 2 THEN value END)'), + ], + matrixify_cell_title_template: '{{row}} - {{column}}', +}; + +test('should generate a 2x2 grid for metrics mode', () => { + const grid = generateMatrixifyGrid(baseFormData); + + expect(grid).not.toBeNull(); + expect(grid!.rowHeaders).toEqual(['Revenue', 'Profit']); + expect(grid!.colHeaders).toEqual(['Q1', 'Q2']); + expect(grid!.cells).toHaveLength(2); + expect(grid!.cells[0]).toHaveLength(2); + + // Check first cell + const firstCell = grid!.cells[0][0]; + expect(firstCell).toBeDefined(); + expect(firstCell!.id).toBe('cell-0-0'); + expect(firstCell!.row).toBe(0); + expect(firstCell!.col).toBe(0); + expect(firstCell!.rowLabel).toBe('Revenue'); + expect(firstCell!.colLabel).toBe('Q1'); + expect(firstCell!.title).toBe('Revenue - Q1'); + expect(firstCell!.formData.metrics).toEqual([ + createAdhocMetric('Revenue'), + createSqlMetric('Q1', 'SUM(CASE WHEN quarter = 1 THEN value END)'), + ]); +}); + +test('should generate grid for dimensions mode', () => { + const dimensionFormData: TestFormData = { + viz_type: 'table', + datasource: '1__table', + matrixify_mode_rows: 'dimensions', + matrixify_mode_columns: 'dimensions', + matrixify_dimension_rows: { + dimension: 'country', + values: ['USA', 'Canada'], + }, + matrixify_dimension_columns: { + dimension: 'product', + values: ['Widget', 'Gadget'], + }, + }; + + const grid = generateMatrixifyGrid(dimensionFormData); + + expect(grid).not.toBeNull(); + expect(grid!.rowHeaders).toEqual(['USA', 'Canada']); + expect(grid!.colHeaders).toEqual(['Widget', 'Gadget']); + expect(grid!.cells).toHaveLength(2); + expect(grid!.cells[0]).toHaveLength(2); + + // Check that filters are applied correctly + const firstCell = grid!.cells[0][0]; + expect(firstCell!.formData.adhoc_filters).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + subject: 'country', + comparator: 'USA', + }), + expect.objectContaining({ + subject: 'product', + comparator: 'Widget', + }), + ]), + ); +}); + +test('should generate grid for mixed mode (metrics rows, dimensions columns)', () => { + const mixedFormData: TestFormData = { + viz_type: 'table', + datasource: '1__table', + matrixify_mode_rows: 'metrics', + matrixify_mode_columns: 'dimensions', + matrixify_rows: [createAdhocMetric('Total Sales')], + matrixify_dimension_columns: { + dimension: 'region', + values: ['North', 'South', 'East', 'West'], + }, + }; + + const grid = generateMatrixifyGrid(mixedFormData); + + expect(grid).not.toBeNull(); + expect(grid!.rowHeaders).toEqual(['Total Sales']); + expect(grid!.colHeaders).toEqual(['North', 'South', 'East', 'West']); + expect(grid!.cells).toHaveLength(1); + expect(grid!.cells[0]).toHaveLength(4); +}); + +test('should handle empty configuration', () => { + const emptyFormData: TestFormData = { + viz_type: 'table', + datasource: '1__table', + matrixify_mode_rows: 'metrics', + matrixify_mode_columns: 'metrics', + matrixify_rows: [], + matrixify_columns: [], + }; + + const grid = generateMatrixifyGrid(emptyFormData); + + expect(grid).not.toBeNull(); + expect(grid!.rowHeaders).toEqual([]); + expect(grid!.colHeaders).toEqual([]); + expect(grid!.cells).toEqual([]); +}); + +test('should handle single row and column', () => { + const singleCellFormData: TestFormData = { + viz_type: 'table', + datasource: '1__table', + matrixify_mode_rows: 'metrics', + matrixify_mode_columns: 'metrics', + matrixify_rows: [createAdhocMetric('Count')], + matrixify_columns: [createAdhocMetric('Total')], + }; + + const grid = generateMatrixifyGrid(singleCellFormData); + + expect(grid).not.toBeNull(); + expect(grid!.rowHeaders).toEqual(['Count']); + expect(grid!.colHeaders).toEqual(['Total']); + expect(grid!.cells).toHaveLength(1); + expect(grid!.cells[0]).toHaveLength(1); + expect(grid!.cells[0][0]!.title).toBe(''); // No template provided +}); + +test('should handle string metrics', () => { + const stringMetricFormData: TestFormData = { + viz_type: 'table', + datasource: '1__table', + matrixify_mode_rows: 'metrics', + matrixify_mode_columns: 'metrics', + matrixify_rows: ['count', 'sum'], + matrixify_columns: ['avg', 'max'], + }; + + const grid = generateMatrixifyGrid(stringMetricFormData); + + expect(grid).not.toBeNull(); + expect(grid!.rowHeaders).toEqual(['count', 'sum']); + expect(grid!.colHeaders).toEqual(['avg', 'max']); +}); + +test('should not escape HTML entities in cell titles', () => { + const formDataWithSpecialChars: TestFormData = { + viz_type: 'table', + datasource: '1__table', + matrixify_mode_rows: 'metrics', + matrixify_mode_columns: 'metrics', + matrixify_rows: [createAdhocMetric('Sales & Revenue')], + matrixify_columns: [createAdhocMetric('Q1 > Q2')], + matrixify_cell_title_template: '{{row}} < {{column}}', + }; + + const grid = generateMatrixifyGrid(formDataWithSpecialChars); + + expect(grid).not.toBeNull(); + const firstCell = grid!.cells[0][0]; + // Should NOT escape HTML entities + expect(firstCell!.title).toBe('Sales & Revenue < Q1 > Q2'); + expect(firstCell!.title).not.toContain('&'); + expect(firstCell!.title).not.toContain('<'); + expect(firstCell!.title).not.toContain('>'); +}); + +test('should apply chart-specific configurations', () => { + const chartConfigFormData: TestFormData = { + ...baseFormData, + row_limit: 100, + time_range: 'Last month', + granularity_sqla: 'day', + }; + + const grid = generateMatrixifyGrid(chartConfigFormData); + + expect(grid).not.toBeNull(); + // Check that chart-specific configs are preserved + const cell = grid!.cells[0][0]; + expect(cell!.formData.row_limit).toBe(100); + expect(cell!.formData.time_range).toBe('Last month'); + expect(cell!.formData.granularity_sqla).toBe('day'); +}); + +test('should generate unique cell IDs', () => { + const grid = generateMatrixifyGrid(baseFormData); + + expect(grid).not.toBeNull(); + const cellIds = new Set(); + + const nonNullCells: { id: string }[] = []; + grid!.cells.forEach(row => { + row.forEach(cell => { + if (cell) { + nonNullCells.push(cell); + } + }); + }); + + nonNullCells.forEach(cell => { + expect(cellIds.has(cell.id)).toBe(false); + cellIds.add(cell.id); + }); + + expect(cellIds.size).toBe(4); // 2x2 grid +}); + +test('should handle template with special characters', () => { + const formDataWithSpecialTemplate: TestFormData = { + ...baseFormData, + matrixify_cell_title_template: '{{row}} | {{column}} (%)', + }; + + const grid = generateMatrixifyGrid(formDataWithSpecialTemplate); + expect(grid).not.toBeNull(); + expect(grid!.cells[0][0]!.title).toBe('Revenue | Q1 (%)'); +}); + +test('should preserve existing adhoc filters', () => { + const formDataWithFilters: TestFormData = { + ...baseFormData, + adhoc_filters: [ + { + expressionType: 'SIMPLE', + subject: 'year', + operator: '==', + comparator: 2024, + clause: 'WHERE', + }, + ], + }; + + const grid = generateMatrixifyGrid(formDataWithFilters); + expect(grid).not.toBeNull(); + const cell = grid!.cells[0][0]; + + // In metrics mode, filters are not added per cell + expect(cell!.formData.adhoc_filters).toHaveLength(1); + expect(cell!.formData.adhoc_filters).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + subject: 'year', + comparator: 2024, + }), + ]), + ); +}); + +test('should handle metrics without labels', () => { + const metricsWithoutLabels: TestFormData = { + viz_type: 'table', + datasource: '1__table', + matrixify_mode_rows: 'metrics', + matrixify_mode_columns: 'metrics', + matrixify_rows: [ + { + expressionType: 'SIMPLE', + column: { column_name: 'value' }, + aggregate: 'SUM', + optionName: 'SUM(value)', + }, + ], + matrixify_columns: ['count'], + }; + + const grid = generateMatrixifyGrid(metricsWithoutLabels); + + expect(grid).not.toBeNull(); + // Metrics without labels show empty string + expect(grid!.rowHeaders).toEqual(['']); + expect(grid!.colHeaders).toEqual(['count']); +}); diff --git a/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridGenerator.ts b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridGenerator.ts new file mode 100644 index 00000000000..afc075ca19a --- /dev/null +++ b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridGenerator.ts @@ -0,0 +1,312 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import Handlebars from 'handlebars'; +import type { QueryFormData } from '../../../query'; +import type { + AdhocFilter, + BinaryAdhocFilter, +} from '../../../query/types/Filter'; +import { + MatrixifyGrid, + MatrixifyGridCell, + MatrixifyFormData, + getMatrixifyConfig, + MatrixifyAxisConfig, + MatrixifyFilterConstants, +} from '../../types/matrixify'; + +/** + * Generate title from template using Handlebars + */ +function generateCellTitle( + rowLabel: string, + colLabel: string, + template?: string, +): string { + if (!template) { + return ''; + } + + try { + // Compile the Handlebars template with noEscape option to prevent HTML entity encoding + const compiledTemplate = Handlebars.compile(template, { noEscape: true }); + + // Create context with both naming conventions for flexibility + const context = { + row: rowLabel, + rowLabel, + column: colLabel, + columnLabel: colLabel, + col: colLabel, + colLabel, + }; + + // Render the template with the context + return compiledTemplate(context); + } catch (error) { + // If template compilation fails, return empty string + console.warn('Failed to compile Handlebars template:', error); + return ''; + } +} + +/** + * Extract label from a metric or dimension value + */ +function getAxisLabel(axisConfig: MatrixifyAxisConfig, index: number): string { + if (axisConfig.mode === 'metrics') { + const metric = axisConfig.metrics?.[index]; + if (!metric) return ''; + // Handle both saved metrics and adhoc metrics + if (typeof metric === 'string') { + return metric; + } + return metric.label || ''; + } + + // For dimensions mode + const dimensionValue = axisConfig.dimension?.values[index]; + return dimensionValue?.toString() || ''; +} + +/** + * Create filter for a specific dimension value + * Using Matrixify-specific constants that match the literal types defined in Filter.ts + */ +function createDimensionFilter( + dimension: string, + value: any, +): BinaryAdhocFilter { + return { + expressionType: MatrixifyFilterConstants.ExpressionType.SIMPLE, + subject: dimension, + operator: MatrixifyFilterConstants.Operator.EQUALS, + comparator: value, + clause: MatrixifyFilterConstants.Clause.WHERE, + isExtra: false, + }; +} + +/** + * Generate form data for a specific grid cell + */ +function generateCellFormData( + baseFormData: QueryFormData & MatrixifyFormData, + rowConfig: MatrixifyAxisConfig | null, + colConfig: MatrixifyAxisConfig | null, + rowIndex: number | null, + colIndex: number | null, +): QueryFormData { + // Start with a clean copy of the base formData + const cellFormData: any = { ...baseFormData }; + + // Remove Matrixify-specific fields since cells shouldn't be matrixified + Object.keys(cellFormData).forEach(key => { + if (key.startsWith('matrixify_')) { + delete cellFormData[key]; + } + }); + + // Override fields that could cause issues in grid cells + const overrides: Partial = { + slice_name: undefined, + slice_id: undefined, + header_font_size: undefined, + subheader: undefined, + show_title: undefined, + header_title_text_align: undefined, + header_text: undefined, + }; + + // Apply overrides + Object.assign(cellFormData, overrides); + + // Add filters for dimension-based axes + const additionalFilters: AdhocFilter[] = []; + + if ( + rowConfig && + rowIndex !== null && + rowConfig.mode === 'dimensions' && + rowConfig.dimension + ) { + const value = rowConfig.dimension.values[rowIndex]; + if (value !== undefined) { + additionalFilters.push( + createDimensionFilter(rowConfig.dimension.dimension, value), + ); + } + } + + if ( + colConfig && + colIndex !== null && + colConfig.mode === 'dimensions' && + colConfig.dimension + ) { + const value = colConfig.dimension.values[colIndex]; + if (value !== undefined) { + additionalFilters.push( + createDimensionFilter(colConfig.dimension.dimension, value), + ); + } + } + + // Add filters to existing adhoc_filters + if (additionalFilters.length > 0) { + cellFormData.adhoc_filters = [ + ...(cellFormData.adhoc_filters || []), + ...additionalFilters, + ]; + } + + // Set metrics based on row/column configuration + const metrics = []; + + if (rowConfig && rowIndex !== null && rowConfig.mode === 'metrics') { + const metric = rowConfig.metrics?.[rowIndex]; + if (metric) { + metrics.push(metric); + } + } + + if (colConfig && colIndex !== null && colConfig.mode === 'metrics') { + const metric = colConfig.metrics?.[colIndex]; + if (metric) { + metrics.push(metric); + } + } + + // If we have metrics from the matrix, use them; otherwise keep original + if (metrics.length > 0) { + cellFormData.metrics = metrics; + } + + return cellFormData; +} + +/** + * Generate a complete grid structure from Matrixify configuration + */ +export function generateMatrixifyGrid( + formData: QueryFormData & MatrixifyFormData, +): MatrixifyGrid | null { + const config = getMatrixifyConfig(formData); + if (!config) { + return null; + } + + // Determine row headers and count + let rowHeaders: string[] = []; + let rowCount = 0; + + if (config.rows.mode === 'metrics' && config.rows.metrics) { + rowCount = config.rows.metrics.length; + rowHeaders = config.rows.metrics.map((_, idx) => + getAxisLabel(config.rows, idx), + ); + } else if ( + config.rows.mode === 'dimensions' && + config.rows.dimension?.values + ) { + rowCount = config.rows.dimension.values.length; + rowHeaders = config.rows.dimension.values.map((_, idx) => + getAxisLabel(config.rows, idx), + ); + } + + // Determine column headers and count + let colHeaders: string[] = []; + let colCount = 0; + + if (config.columns.mode === 'metrics' && config.columns.metrics) { + colCount = config.columns.metrics.length; + colHeaders = config.columns.metrics.map((_, idx) => + getAxisLabel(config.columns, idx), + ); + } else if ( + config.columns.mode === 'dimensions' && + config.columns.dimension?.values + ) { + colCount = config.columns.dimension.values.length; + colHeaders = config.columns.dimension.values.map((_, idx) => + getAxisLabel(config.columns, idx), + ); + } + + // If only rows are configured, create a single column grid + if (rowCount > 0 && colCount === 0) { + colCount = 1; + colHeaders = ['']; + } + + // If only columns are configured, create a single row grid + if (colCount > 0 && rowCount === 0) { + rowCount = 1; + rowHeaders = ['']; + } + + // Generate grid cells + const cells: (MatrixifyGridCell | null)[][] = []; + + for (let row = 0; row < rowCount; row += 1) { + const rowCells: (MatrixifyGridCell | null)[] = []; + + for (let col = 0; col < colCount; col += 1) { + const id = `cell-${row}-${col}`; + const rowLabel = rowHeaders[row]; + const colLabel = colHeaders[col]; + + const cellFormData = generateCellFormData( + formData, + rowCount > 1 ? config.rows : null, + colCount > 1 ? config.columns : null, + rowCount > 1 ? row : null, + colCount > 1 ? col : null, + ); + + // Generate title using template if provided + const title = generateCellTitle( + rowLabel, + colLabel, + formData.matrixify_cell_title_template, + ); + + rowCells.push({ + id, + row, + col, + rowLabel, + colLabel, + title, + formData: cellFormData, + }); + } + + cells.push(rowCells); + } + + return { + rowHeaders, + colHeaders, + cells, + baseFormData: formData, + }; +} diff --git a/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.test.tsx b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.test.tsx new file mode 100644 index 00000000000..7dd05e06c67 --- /dev/null +++ b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.test.tsx @@ -0,0 +1,396 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { render } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { ThemeProvider } from '@superset-ui/core'; +import MatrixifyGridRenderer from './MatrixifyGridRenderer'; +import { generateMatrixifyGrid } from './MatrixifyGridGenerator'; +import { supersetTheme } from '../../../theme'; + +// Mock the MatrixifyGridGenerator +jest.mock('./MatrixifyGridGenerator', () => ({ + generateMatrixifyGrid: jest.fn(), +})); + +// Mock MatrixifyGridCell component +jest.mock('./MatrixifyGridCell', () => + // eslint-disable-next-line react/display-name, @typescript-eslint/no-unused-vars + ({ cell, rowHeight, datasource, hooks }: any) => ( +
Cell: {cell.id}
+ ), +); + +const mockGenerateMatrixifyGrid = generateMatrixifyGrid as jest.MockedFunction< + typeof generateMatrixifyGrid +>; + +const renderWithTheme = (component: React.ReactElement) => + render({component}); + +beforeEach(() => { + jest.clearAllMocks(); +}); + +test('should create single group when fitting columns dynamically', () => { + const mockGrid: any = { + rowHeaders: ['Row 1', 'Row 2'], + colHeaders: ['Col 1', 'Col 2', 'Col 3', 'Col 4', 'Col 5'], + cells: [ + [ + { id: 'cell-0-0' }, + { id: 'cell-0-1' }, + { id: 'cell-0-2' }, + { id: 'cell-0-3' }, + { id: 'cell-0-4' }, + ], + [ + { id: 'cell-1-0' }, + { id: 'cell-1-1' }, + { id: 'cell-1-2' }, + { id: 'cell-1-3' }, + { id: 'cell-1-4' }, + ], + ], + }; + + mockGenerateMatrixifyGrid.mockReturnValue(mockGrid); + + const formData = { + viz_type: 'test_chart', + matrixify_enabled: true, + matrixify_fit_columns_dynamically: true, + matrixify_charts_per_row: 3, + matrixify_show_row_labels: true, + matrixify_show_column_headers: true, + }; + + const { container } = renderWithTheme( + , + ); + + // When fitting dynamically, should have only one column group with all 5 columns + // Check for the presence of the grid structure + const gridContainers = container.querySelectorAll('div[class*="css-"]'); + expect(gridContainers.length).toBeGreaterThan(0); + + // Verify all 5 column headers are present in single group + const columnHeaders = container.querySelectorAll('.matrixify-col-header'); + expect(columnHeaders).toHaveLength(5); + expect(columnHeaders[0]).toHaveTextContent('Col 1'); + expect(columnHeaders[4]).toHaveTextContent('Col 5'); +}); + +test('should create multiple groups when not fitting columns dynamically', () => { + const mockGrid: any = { + rowHeaders: ['Row 1', 'Row 2'], + colHeaders: ['Col 1', 'Col 2', 'Col 3', 'Col 4', 'Col 5'], + cells: [ + [ + { id: 'cell-0-0' }, + { id: 'cell-0-1' }, + { id: 'cell-0-2' }, + { id: 'cell-0-3' }, + { id: 'cell-0-4' }, + ], + [ + { id: 'cell-1-0' }, + { id: 'cell-1-1' }, + { id: 'cell-1-2' }, + { id: 'cell-1-3' }, + { id: 'cell-1-4' }, + ], + ], + }; + + mockGenerateMatrixifyGrid.mockReturnValue(mockGrid); + + const formData = { + viz_type: 'test_chart', + matrixify_enabled: true, + matrixify_fit_columns_dynamically: false, + matrixify_charts_per_row: 3, + matrixify_show_row_labels: true, + matrixify_show_column_headers: true, + }; + + const { container } = renderWithTheme( + , + ); + + // With 5 columns and charts_per_row=3, should have 2 groups (3+2) + // With 2 rows and wrapping, we should see headers repeated + const columnHeaders = container.querySelectorAll('.matrixify-col-header'); + expect(columnHeaders.length).toBeGreaterThanOrEqual(5); // At least the base headers +}); + +test('should handle exact division of columns', () => { + const mockGrid: any = { + rowHeaders: ['Row 1'], + colHeaders: ['Col 1', 'Col 2', 'Col 3', 'Col 4'], + cells: [ + [ + { id: 'cell-0-0' }, + { id: 'cell-0-1' }, + { id: 'cell-0-2' }, + { id: 'cell-0-3' }, + ], + ], + }; + + mockGenerateMatrixifyGrid.mockReturnValue(mockGrid); + + const formData = { + viz_type: 'test_chart', + matrixify_enabled: true, + matrixify_fit_columns_dynamically: false, + matrixify_charts_per_row: 2, + matrixify_show_row_labels: true, + matrixify_show_column_headers: true, + }; + + const { container } = renderWithTheme( + , + ); + + // With 4 columns and charts_per_row=2, should have exactly 2 groups (2+2) + // Check that we have column headers - should be 4 total (2 per group) + const columnHeaders = container.querySelectorAll('.matrixify-col-header'); + expect(columnHeaders).toHaveLength(4); +}); + +test('should handle case where charts_per_row exceeds total columns', () => { + const mockGrid: any = { + rowHeaders: ['Row 1'], + colHeaders: ['Col 1', 'Col 2'], + cells: [[{ id: 'cell-0-0' }, { id: 'cell-0-1' }]], + }; + + mockGenerateMatrixifyGrid.mockReturnValue(mockGrid); + + const formData = { + viz_type: 'test_chart', + matrixify_enabled: true, + matrixify_fit_columns_dynamically: false, + matrixify_charts_per_row: 5, + matrixify_show_row_labels: true, + matrixify_show_column_headers: true, + }; + + const { container } = renderWithTheme( + , + ); + + // Should create only one group with all columns + const columnHeaders = container.querySelectorAll('.matrixify-col-header'); + expect(columnHeaders).toHaveLength(2); +}); + +test('should show headers for each group when wrapping occurs', () => { + const mockGrid: any = { + rowHeaders: ['Row 1', 'Row 2'], + colHeaders: ['Col 1', 'Col 2', 'Col 3'], + cells: [ + [{ id: 'cell-0-0' }, { id: 'cell-0-1' }, { id: 'cell-0-2' }], + [{ id: 'cell-1-0' }, { id: 'cell-1-1' }, { id: 'cell-1-2' }], + ], + }; + + mockGenerateMatrixifyGrid.mockReturnValue(mockGrid); + + const formData = { + viz_type: 'test_chart', + matrixify_enabled: true, + matrixify_fit_columns_dynamically: false, + matrixify_charts_per_row: 2, + matrixify_show_row_labels: true, + matrixify_show_column_headers: true, + }; + + const { container } = renderWithTheme( + , + ); + + // With wrapping (multiple column groups), headers should appear for each group + const columnHeaders = container.querySelectorAll('.matrixify-col-header'); + expect(columnHeaders.length).toBeGreaterThan(3); // More than just one set of headers + + // Row headers should appear only once per row (for first column group) + const rowHeaders = container.querySelectorAll('.matrixify-row-header'); + expect(rowHeaders).toHaveLength(2); // One for each row +}); + +test('should show headers only on first row when not wrapping', () => { + const mockGrid: any = { + rowHeaders: ['Row 1', 'Row 2'], + colHeaders: ['Col 1', 'Col 2'], + cells: [ + [{ id: 'cell-0-0' }, { id: 'cell-0-1' }], + [{ id: 'cell-1-0' }, { id: 'cell-1-1' }], + ], + }; + + mockGenerateMatrixifyGrid.mockReturnValue(mockGrid); + + const formData = { + viz_type: 'test_chart', + matrixify_enabled: true, + matrixify_fit_columns_dynamically: true, // No wrapping + matrixify_show_row_labels: true, + matrixify_show_column_headers: true, + }; + + const { container } = renderWithTheme( + , + ); + + // Without wrapping, headers should appear only once (first row) + const columnHeaders = container.querySelectorAll('.matrixify-col-header'); + expect(columnHeaders).toHaveLength(2); // Just Col 1 and Col 2 + + const rowHeaders = container.querySelectorAll('.matrixify-row-header'); + expect(rowHeaders).toHaveLength(2); // One for each row +}); + +test('should hide headers when disabled', () => { + const mockGrid: any = { + rowHeaders: ['Row 1'], + colHeaders: ['Col 1', 'Col 2'], + cells: [[{ id: 'cell-0-0' }, { id: 'cell-0-1' }]], + }; + + mockGenerateMatrixifyGrid.mockReturnValue(mockGrid); + + const formData = { + viz_type: 'test_chart', + matrixify_enabled: true, + matrixify_show_row_labels: false, + matrixify_show_column_headers: false, + }; + + const { container } = renderWithTheme( + , + ); + + const columnHeaders = container.querySelectorAll('.matrixify-col-header'); + expect(columnHeaders).toHaveLength(0); + + const rowHeaders = container.querySelectorAll('.matrixify-row-header'); + expect(rowHeaders).toHaveLength(0); +}); + +test('should place cells correctly in wrapped layout', () => { + const mockGrid: any = { + rowHeaders: ['Row 1'], + colHeaders: ['Col 1', 'Col 2', 'Col 3'], + cells: [[{ id: 'cell-0-0' }, { id: 'cell-0-1' }, { id: 'cell-0-2' }]], + }; + + mockGenerateMatrixifyGrid.mockReturnValue(mockGrid); + + const formData = { + viz_type: 'test_chart', + matrixify_enabled: true, + matrixify_fit_columns_dynamically: false, + matrixify_charts_per_row: 2, + matrixify_show_row_labels: true, + matrixify_show_column_headers: true, + }; + + const { container } = renderWithTheme( + , + ); + + // All cells should be rendered + const cells = container.querySelectorAll('[data-testid^="grid-cell-"]'); + expect(cells).toHaveLength(3); + expect( + container.querySelector('[data-testid="grid-cell-cell-0-0"]'), + ).toBeInTheDocument(); + expect( + container.querySelector('[data-testid="grid-cell-cell-0-1"]'), + ).toBeInTheDocument(); + expect( + container.querySelector('[data-testid="grid-cell-cell-0-2"]'), + ).toBeInTheDocument(); +}); + +test('should handle null grid gracefully', () => { + mockGenerateMatrixifyGrid.mockReturnValue(null); + + const formData = { + viz_type: 'test_chart', + matrixify_enabled: true, + }; + + const { container } = renderWithTheme( + , + ); + + expect(container).toBeEmptyDOMElement(); +}); + +test('should handle empty grid gracefully', () => { + const mockGrid: any = { + rowHeaders: [], + colHeaders: [], + cells: [], + }; + + mockGenerateMatrixifyGrid.mockReturnValue(mockGrid); + + const formData = { + viz_type: 'test_chart', + matrixify_enabled: true, + }; + + const { container } = renderWithTheme( + , + ); + + // Should render container but no cells + expect(container).not.toBeEmptyDOMElement(); + const gridCells = container.querySelectorAll('[data-testid^="grid-cell-"]'); + expect(gridCells).toHaveLength(0); +}); + +test('should use default values for missing configuration', () => { + const mockGrid: any = { + rowHeaders: ['Row 1'], + colHeaders: ['Col 1', 'Col 2'], + cells: [[{ id: 'cell-0-0' }, { id: 'cell-0-1' }]], + }; + + mockGenerateMatrixifyGrid.mockReturnValue(mockGrid); + + const formData = { + viz_type: 'test_chart', + matrixify_enabled: true, + // Missing optional configurations + }; + + const { container } = renderWithTheme( + , + ); + + // Should still render with defaults + expect(container).not.toBeEmptyDOMElement(); + const gridCells = container.querySelectorAll('[data-testid^="grid-cell-"]'); + expect(gridCells).toHaveLength(2); +}); diff --git a/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.tsx b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.tsx new file mode 100644 index 00000000000..f06d3b949be --- /dev/null +++ b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.tsx @@ -0,0 +1,272 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { useMemo } from 'react'; +import { styled } from '../../../theme'; +import { MatrixifyFormData } from '../../types/matrixify'; +import { generateMatrixifyGrid } from './MatrixifyGridGenerator'; +import MatrixifyGridCell from './MatrixifyGridCell'; + +// Layout constants +const HEADER_HEIGHT = 24; // Height for column headers and width for row headers (reduced from 32) +const HEADER_MIN_WIDTH = 20; // Minimum width for row headers (reduced from 24) +const HEADER_MAX_WIDTH = 24; // Maximum width for row headers (reduced from 32) +const GRID_GAP = 8; // Gap between grid cells (reduced from 16 for more density) +const GROUP_SPACING = 16; // Spacing between column groups when wrapping (reduced from 32) +const DEFAULT_ROW_HEIGHT = 300; // Default height for each row +const DEFAULT_CHARTS_PER_ROW = 3; // Default number of charts per row when not fitting dynamically + +const GridContainer = styled.div<{ height?: number }>` + width: 100%; + ${({ height }) => height && `height: ${height}px;`} + padding: ${({ theme }) => + theme.sizeUnit}px; /* Reduced padding for more density */ + box-sizing: border-box; + display: flex; + flex-direction: column; +`; + +const GridScrollContainer = styled.div` + flex: 1; + overflow: auto; + min-height: 0; +`; + +const GridLayout = styled.div<{ + columns: number; + hasRowHeaders: boolean; + rowHeight: number; + hasColumnHeaders: boolean; + maxColumns: number; // Maximum columns to maintain consistent width +}>` + display: grid; + grid-template-columns: ${({ maxColumns, hasRowHeaders }) => + hasRowHeaders + ? `${HEADER_HEIGHT}px repeat(${maxColumns}, minmax(0, 1fr))` + : `repeat(${maxColumns}, minmax(0, 1fr))`}; + ${({ hasColumnHeaders, rowHeight }) => + hasColumnHeaders + ? `grid-template-rows: ${HEADER_HEIGHT}px; grid-auto-rows: ${rowHeight}px;` + : `grid-auto-rows: ${rowHeight}px;`} + gap: ${GRID_GAP}px; + width: 100%; + min-width: 0; + min-height: 0; +`; + +const GridGroup = styled.div<{ isLast: boolean }>` + margin-bottom: ${({ isLast }) => (isLast ? 0 : GROUP_SPACING)}px; +`; + +const GridHeader = styled.div` + background-color: ${({ theme }) => theme.colorFillAlter}; + padding: ${({ theme }) => theme.sizeUnit / 2}px; /* Reduced padding */ + font-weight: ${({ theme }) => theme.fontWeightStrong}; + text-align: center; + border: 1px solid ${({ theme }) => theme.colorBorder}; + border-radius: ${({ theme }) => theme.borderRadius}px; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + font-size: ${({ theme }) => + theme.fontSizeSM}px; /* Back to small (readable) font */ + + &.matrixify-row-header { + writing-mode: vertical-rl; + transform: rotate(-180deg); + padding: ${({ theme }) => theme.sizeUnit}px + ${({ theme }) => theme.sizeUnit / 4}px; /* Tighter padding */ + min-width: ${HEADER_MIN_WIDTH}px; + max-width: ${HEADER_MAX_WIDTH}px; + display: flex; + align-items: center; + justify-content: center; + } + + &.matrixify-col-header { + height: ${HEADER_HEIGHT}px; + display: flex; + align-items: center; + justify-content: center; + } +`; + +interface MatrixifyGridRendererProps { + formData: MatrixifyFormData; + datasource?: any; + width?: number; + height?: number; + hooks?: any; +} + +function MatrixifyGridRenderer({ + formData, + datasource, + width, + height, + hooks, +}: MatrixifyGridRendererProps) { + // Generate grid structure from form data + const grid = useMemo( + () => generateMatrixifyGrid(formData as any), + [formData], + ); + + // Determine layout parameters + const showRowLabels = formData.matrixify_show_row_labels ?? true; + const showColumnHeaders = formData.matrixify_show_column_headers ?? true; + const rowHeight = formData.matrixify_row_height || DEFAULT_ROW_HEIGHT; + const fitColumnsDynamically = + formData.matrixify_fit_columns_dynamically ?? true; + const chartsPerRow = + formData.matrixify_charts_per_row || DEFAULT_CHARTS_PER_ROW; + + // Calculate column groups for wrapping - must be before conditional return + const columnGroups = useMemo(() => { + if (!grid) { + return []; + } + const { colHeaders: headers } = grid; + const totalCols = headers.length; + const colsPerRow = fitColumnsDynamically + ? totalCols + : Math.min(chartsPerRow, totalCols); + + const groups = []; + for (let i = 0; i < totalCols; i += colsPerRow) { + groups.push({ + startIdx: i, + endIdx: Math.min(i + colsPerRow, totalCols), + headers: headers.slice(i, Math.min(i + colsPerRow, totalCols)), + }); + } + return groups; + }, [grid, fitColumnsDynamically, chartsPerRow]); + + if (!grid) { + return null; + } + + const { rowHeaders, colHeaders, cells } = grid; + + // Calculate actual columns per row + const totalColumns = colHeaders.length; + const columnsPerRow = fitColumnsDynamically + ? totalColumns + : Math.min(chartsPerRow, totalColumns); + + const hasRowHeaders = showRowLabels && rowHeaders.length > 0; + const hasColumnHeaders = showColumnHeaders && colHeaders.length > 0; + + return ( + + + {/* Iterate through each row first */} + {cells.map((row, rowIdx) => ( +
+ {/* Then iterate through column groups for this row */} + {columnGroups.map((colGroup, groupIdx) => { + const groupColumns = colGroup.endIdx - colGroup.startIdx; + const emptyColumns = columnsPerRow - groupColumns; + const isLastGroup = groupIdx === columnGroups.length - 1; + const isLastRow = rowIdx === cells.length - 1; + + // Show headers: always when wrapping (multiple column groups), only first row when not wrapping + const showHeadersForThisGroup = + hasColumnHeaders && (columnGroups.length > 1 || rowIdx === 0); + + return ( + + + {/* Corner cell (empty) - when showing headers */} + {showHeadersForThisGroup && hasRowHeaders &&
} + + {/* Column headers - show based on wrapping logic */} + {showHeadersForThisGroup && ( + <> + {colGroup.headers.map((header, idx) => ( + + {header} + + ))} + {/* Empty cells to maintain grid structure */} + {Array.from({ length: emptyColumns }).map((_, idx) => ( +
+ ))} + + )} + + {/* Row header - only for first column group */} + {hasRowHeaders && groupIdx === 0 && ( + + {rowHeaders[rowIdx]} + + )} + {/* Empty cell if not first column group but has row headers */} + {hasRowHeaders && groupIdx > 0 &&
} + + {/* Row cells for this column group */} + {row + .slice(colGroup.startIdx, colGroup.endIdx) + .map((cell, colIdx) => + cell ? ( + + ) : null, + )} + {/* Empty cells to maintain grid structure */} + {Array.from({ length: emptyColumns }).map((_, idx) => ( +
+ ))} + + + ); + })} +
+ ))} + + + ); +} + +export default MatrixifyGridRenderer; diff --git a/superset-frontend/packages/superset-ui-core/src/chart/components/StatefulChart.test.tsx b/superset-frontend/packages/superset-ui-core/src/chart/components/StatefulChart.test.tsx new file mode 100644 index 00000000000..35b65ac7727 --- /dev/null +++ b/superset-frontend/packages/superset-ui-core/src/chart/components/StatefulChart.test.tsx @@ -0,0 +1,473 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { render, waitFor, configure } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import StatefulChart from './StatefulChart'; +import getChartControlPanelRegistry from '../registries/ChartControlPanelRegistrySingleton'; +import getChartMetadataRegistry from '../registries/ChartMetadataRegistrySingleton'; +import getChartBuildQueryRegistry from '../registries/ChartBuildQueryRegistrySingleton'; + +// Configure testing library to use data-test attribute +configure({ testIdAttribute: 'data-test' }); + +// Mock the registries +jest.mock('../registries/ChartControlPanelRegistrySingleton'); +jest.mock('../registries/ChartMetadataRegistrySingleton'); +jest.mock('../registries/ChartBuildQueryRegistrySingleton'); +jest.mock('../clients/ChartClient'); + +// Mock SuperChart component +jest.mock('./SuperChart', () => ({ + __esModule: true, + // eslint-disable-next-line react/display-name + default: ({ formData }: any) => ( +
SuperChart: {JSON.stringify(formData)}
+ ), +})); + +// Mock Loading component +jest.mock('../../components/Loading', () => ({ + // eslint-disable-next-line react/display-name + Loading: () =>
Loading...
, +})); + +const mockChartClient = { + client: { + post: jest.fn().mockResolvedValue({ + json: [{ data: 'test data' }], + }), + }, + loadFormData: jest.fn(), +}; + +const mockFormData = { + viz_type: 'test_chart', + datasource: '1__table', + color_scheme: 'default', +}; + +beforeEach(() => { + jest.clearAllMocks(); + + // Setup default registry mocks + (getChartMetadataRegistry as any).mockReturnValue({ + get: jest.fn().mockReturnValue({ + useLegacyApi: false, + }), + }); + + (getChartBuildQueryRegistry as any).mockReturnValue({ + get: jest.fn().mockResolvedValue(null), + }); + + (getChartControlPanelRegistry as any).mockReturnValue({ + get: jest.fn().mockReturnValue(null), + }); + + // Mock ChartClient constructor + // eslint-disable-next-line global-require, @typescript-eslint/no-var-requires + const ChartClient = require('../clients/ChartClient').default; // eslint-disable-line + ChartClient.mockImplementation(() => mockChartClient); +}); + +test('should refetch data when non-renderTrigger control changes', async () => { + const controlPanelConfig = { + controlPanelSections: [ + { + controlSetRows: [ + [ + { + name: 'color_scheme', + config: { + renderTrigger: true, + }, + }, + ], + [ + { + name: 'datasource', + config: { + renderTrigger: false, + }, + }, + ], + ], + }, + ], + }; + + (getChartControlPanelRegistry as any).mockReturnValue({ + get: jest.fn().mockReturnValue(controlPanelConfig), + }); + + const { rerender } = render( + , + ); + + await waitFor(() => { + expect(mockChartClient.client.post).toHaveBeenCalledTimes(1); + }); + + // Change a non-renderTrigger control (datasource) + const updatedFormData = { + ...mockFormData, + datasource: '2__table', + }; + + rerender(); + + await waitFor(() => { + // Should refetch data + expect(mockChartClient.client.post).toHaveBeenCalledTimes(2); + }); +}); + +test('should NOT refetch data when only renderTrigger controls change', async () => { + const controlPanelConfig = { + controlPanelSections: [ + { + controlSetRows: [ + [ + { + name: 'color_scheme', + config: { + renderTrigger: true, + }, + }, + ], + [ + { + name: 'show_legend', + config: { + renderTrigger: true, + }, + }, + ], + ], + }, + ], + }; + + (getChartControlPanelRegistry as any).mockReturnValue({ + get: jest.fn().mockReturnValue(controlPanelConfig), + }); + + const { rerender, getByTestId } = render( + , + ); + + await waitFor(() => { + expect(mockChartClient.client.post).toHaveBeenCalledTimes(1); + }); + + // Verify initial render + expect(getByTestId('super-chart')).toBeInTheDocument(); + + // Change only renderTrigger controls + const updatedFormData = { + ...mockFormData, + color_scheme: 'new_scheme', + show_legend: true, + }; + + rerender(); + + await waitFor(() => { + // Should NOT refetch data (still only 1 call) + expect(mockChartClient.client.post).toHaveBeenCalledTimes(1); + // But should re-render with new formData + expect(getByTestId('super-chart')).toHaveTextContent( + JSON.stringify(updatedFormData), + ); + }); +}); + +test('should refetch when control panel config is not available', async () => { + // No control panel config available + (getChartControlPanelRegistry as any).mockReturnValue({ + get: jest.fn().mockReturnValue(null), + }); + + const { rerender } = render( + , + ); + + await waitFor(() => { + expect(mockChartClient.client.post).toHaveBeenCalledTimes(1); + }); + + // Change any control + const updatedFormData = { + ...mockFormData, + color_scheme: 'new_scheme', + }; + + rerender(); + + await waitFor(() => { + // Should refetch data (conservative approach when no config) + expect(mockChartClient.client.post).toHaveBeenCalledTimes(2); + }); +}); + +test('should refetch when viz_type changes', async () => { + const controlPanelConfig = { + controlPanelSections: [ + { + controlSetRows: [ + [ + { + name: 'color_scheme', + config: { + renderTrigger: true, + }, + }, + ], + ], + }, + ], + }; + + (getChartControlPanelRegistry as any).mockReturnValue({ + get: jest.fn().mockReturnValue(controlPanelConfig), + }); + + const { rerender } = render( + , + ); + + await waitFor(() => { + expect(mockChartClient.client.post).toHaveBeenCalledTimes(1); + }); + + // Change viz_type + const updatedFormData = { + ...mockFormData, + viz_type: 'different_chart', + }; + + rerender( + , + ); + + await waitFor(() => { + // Should always refetch when viz_type changes + expect(mockChartClient.client.post).toHaveBeenCalledTimes(2); + }); +}); + +test('should handle mixed renderTrigger and non-renderTrigger changes', async () => { + const controlPanelConfig = { + controlPanelSections: [ + { + controlSetRows: [ + [ + { + name: 'color_scheme', + config: { + renderTrigger: true, + }, + }, + ], + [ + { + name: 'metrics', + config: { + renderTrigger: false, + }, + }, + ], + ], + }, + ], + }; + + (getChartControlPanelRegistry as any).mockReturnValue({ + get: jest.fn().mockReturnValue(controlPanelConfig), + }); + + const { rerender } = render( + , + ); + + await waitFor(() => { + expect(mockChartClient.client.post).toHaveBeenCalledTimes(1); + }); + + // Change both renderTrigger and non-renderTrigger controls + const updatedFormData = { + ...mockFormData, + color_scheme: 'new_scheme', // renderTrigger + metrics: ['new_metric'], // non-renderTrigger + }; + + rerender(); + + await waitFor(() => { + // Should refetch because a non-renderTrigger control changed + expect(mockChartClient.client.post).toHaveBeenCalledTimes(2); + }); +}); + +test('should handle controls with complex structure', async () => { + const controlPanelConfig = { + controlPanelSections: [ + { + controlSetRows: [ + [ + { + config: { + name: 'nested_control', + renderTrigger: true, + }, + }, + ], + [ + { + name: 'direct_control', + config: { + renderTrigger: true, + }, + }, + ], + ], + }, + ], + }; + + (getChartControlPanelRegistry as any).mockReturnValue({ + get: jest.fn().mockReturnValue(controlPanelConfig), + }); + + const { rerender, getByTestId } = render( + , + ); + + await waitFor(() => { + expect(mockChartClient.client.post).toHaveBeenCalledTimes(1); + }); + + // Change controls that have different config structures + const updatedFormData = { + ...mockFormData, + nested_control: 'value1', + direct_control: 'value2', + }; + + rerender(); + + await waitFor(() => { + // Should NOT refetch (both are renderTrigger) + expect(mockChartClient.client.post).toHaveBeenCalledTimes(1); + }); + + // But should re-render + expect(getByTestId('super-chart')).toHaveTextContent( + JSON.stringify(updatedFormData), + ); +}); + +test('should not refetch when formData has not changed', async () => { + const { rerender } = render( + , + ); + + await waitFor(() => { + expect(mockChartClient.client.post).toHaveBeenCalledTimes(1); + }); + + // Re-render with same formData + rerender(); + + await waitFor(() => { + // Should not refetch + expect(mockChartClient.client.post).toHaveBeenCalledTimes(1); + }); +}); + +test('should handle errors gracefully when accessing registry', async () => { + // Mock registry to throw an error + (getChartControlPanelRegistry as any).mockReturnValue({ + get: jest.fn().mockImplementation(() => { + throw new Error('Registry error'); + }), + }); + + const { rerender } = render( + , + ); + + await waitFor(() => { + expect(mockChartClient.client.post).toHaveBeenCalledTimes(1); + }); + + // Change a control + const updatedFormData = { + ...mockFormData, + color_scheme: 'new_scheme', + }; + + rerender(); + + await waitFor(() => { + // Should refetch data (conservative approach on error) + expect(mockChartClient.client.post).toHaveBeenCalledTimes(2); + }); +}); + +test('should handle force prop correctly', async () => { + const { rerender } = render( + , + ); + + await waitFor(() => { + expect(mockChartClient.client.post).toHaveBeenCalledTimes(1); + }); + + // Re-render with force=true + rerender( + , + ); + + await waitFor(() => { + // Should refetch when force changes + expect(mockChartClient.client.post).toHaveBeenCalledTimes(2); + }); +}); + +test('should handle chartId changes', async () => { + mockChartClient.loadFormData.mockResolvedValue(mockFormData); + + const { rerender } = render( + , + ); + + await waitFor(() => { + expect(mockChartClient.loadFormData).toHaveBeenCalledTimes(1); + }); + + // Change chartId + rerender(); + + await waitFor(() => { + // Should load new formData + expect(mockChartClient.loadFormData).toHaveBeenCalledTimes(2); + }); +}); diff --git a/superset-frontend/packages/superset-ui-core/src/chart/components/StatefulChart.tsx b/superset-frontend/packages/superset-ui-core/src/chart/components/StatefulChart.tsx new file mode 100644 index 00000000000..ba22e64808a --- /dev/null +++ b/superset-frontend/packages/superset-ui-core/src/chart/components/StatefulChart.tsx @@ -0,0 +1,476 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { useState, useEffect, useRef, useCallback } from 'react'; +import { ParentSize } from '@visx/responsive'; +import { + QueryFormData, + QueryData, + SupersetClientInterface, + buildQueryContext, + RequestConfig, +} from '../..'; +import { Loading } from '../../components/Loading'; +import ChartClient from '../clients/ChartClient'; +import getChartBuildQueryRegistry from '../registries/ChartBuildQueryRegistrySingleton'; +import getChartMetadataRegistry from '../registries/ChartMetadataRegistrySingleton'; +import getChartControlPanelRegistry from '../registries/ChartControlPanelRegistrySingleton'; +import SuperChart from './SuperChart'; + +// Using more specific states that align with chart loading process +type LoadingState = 'uninitialized' | 'loading' | 'loaded' | 'error'; + +/** + * Helper function to determine if data should be refetched based on formData changes + * @param prevFormData Previous formData + * @param nextFormData New formData + * @param vizType Chart visualization type + * @returns true if data should be refetched, false if only re-render is needed + */ +function shouldRefetchData( + prevFormData: QueryFormData | undefined, + nextFormData: QueryFormData | undefined, + vizType: string | undefined, +): boolean { + // If no previous formData or viz types don't match, always refetch + if (!prevFormData || !nextFormData || !vizType) { + return true; + } + + // If viz_type changed, always refetch + if (prevFormData.viz_type !== nextFormData.viz_type) { + return true; + } + + try { + // Try to get control panel configuration + const controlPanel = getChartControlPanelRegistry().get(vizType); + if (!controlPanel || !controlPanel.controlPanelSections) { + // If no control panel config available, be conservative and refetch + return true; + } + + // Build a map of control names to their renderTrigger status + const renderTriggerControls = new Set(); + controlPanel.controlPanelSections.forEach((section: any) => { + if (section.controlSetRows) { + section.controlSetRows.forEach((row: any) => { + row.forEach((control: any) => { + if (control && typeof control === 'object') { + const controlName = control.name || control.config?.name; + if (controlName && control.config?.renderTrigger === true) { + renderTriggerControls.add(controlName); + } + } + }); + }); + } + }); + + // Check which fields changed + const changedFields = Object.keys(nextFormData).filter( + key => + JSON.stringify(prevFormData[key]) !== JSON.stringify(nextFormData[key]), + ); + + // If no fields changed, no need to refetch + if (changedFields.length === 0) { + return false; + } + + // Check if all changed fields are renderTrigger controls + const allChangesAreRenderTrigger = changedFields.every(field => + renderTriggerControls.has(field), + ); + + // Only skip refetch if ALL changes are renderTrigger-only + return !allChangesAreRenderTrigger; + } catch (error) { + // If there's any error accessing the registry, be conservative and refetch + return true; + } +} + +export interface StatefulChartProps { + // Option 1: Provide chartId to load saved chart + chartId?: number; + + // Option 2: Provide formData directly + formData?: QueryFormData; + + // Option 3: Override specific formData fields + formDataOverrides?: Partial; + + // Chart type (required if using formData without viz_type) + chartType?: string; + + // Chart dimensions + width?: number | string; + height?: number | string; + + // Event handlers + onLoad?: (data: QueryData[]) => void; + onError?: (error: Error) => void; + onRenderSuccess?: () => void; + onRenderFailure?: (error: Error) => void; + + // Data fetching options + force?: boolean; + timeout?: number; + + // UI options + showLoading?: boolean; + loadingComponent?: React.ComponentType; + errorComponent?: React.ComponentType<{ error: Error }>; + noDataComponent?: React.ComponentType; + + // Advanced options + client?: SupersetClientInterface; + disableErrorBoundary?: boolean; + enableNoResults?: boolean; + + // Additional SuperChart props + id?: string; + className?: string; + + // Hooks for chart interactions (drill, cross-filter, etc.) + hooks?: any; +} + +export default function StatefulChart(props: StatefulChartProps) { + const [status, setStatus] = useState('uninitialized'); + const [data, setData] = useState(); + const [error, setError] = useState(); + const [formData, setFormData] = useState(); + + const chartClientRef = useRef(); + const abortControllerRef = useRef(); + + // Initialize chart client + if (!chartClientRef.current) { + chartClientRef.current = new ChartClient({ client: props.client }); + } + + const fetchData = useCallback(async () => { + const { + chartId, + formData: propsFormData, + formDataOverrides, + onError, + onLoad, + chartType, + force, + timeout, + } = props; + + // Cancel any in-flight requests + if (abortControllerRef.current) { + abortControllerRef.current.abort(); + } + + // Create new abort controller + abortControllerRef.current = new AbortController(); + + setStatus('loading'); + setError(undefined); + + try { + let finalFormData: QueryFormData; + + if (chartId && !propsFormData) { + // Load formData from chartId + finalFormData = await chartClientRef.current!.loadFormData( + { sliceId: chartId }, + { signal: abortControllerRef.current.signal } as RequestConfig, + ); + } else if (propsFormData) { + // Use provided formData + finalFormData = propsFormData; + } else { + throw new Error('Either chartId or formData must be provided'); + } + + // Apply overrides if provided + if (formDataOverrides) { + finalFormData = { ...finalFormData, ...formDataOverrides }; + } + + // Ensure viz_type is set + const vizType = finalFormData.viz_type || chartType; + if (!vizType) { + throw new Error('Chart type (viz_type) must be specified'); + } + finalFormData.viz_type = vizType; + + // Get chart metadata + const { useLegacyApi } = getChartMetadataRegistry().get(vizType) || {}; + + // Build query using the chart's buildQuery function + const buildQuery = await getChartBuildQueryRegistry().get(vizType); + let queryContext; + + if (buildQuery) { + queryContext = buildQuery(finalFormData); + } else { + // Fallback to default query context builder + queryContext = buildQueryContext(finalFormData); + } + + // Ensure query_context is properly formatted for new API + if (!useLegacyApi && !queryContext.queries) { + queryContext = { queries: [queryContext] }; + } + const endpoint = useLegacyApi + ? '/superset/explore_json/' + : '/api/v1/chart/data'; + + const requestConfig: RequestConfig = { + endpoint, + signal: abortControllerRef.current.signal, + ...(timeout && { timeout: timeout * 1000 }), + }; + + if (useLegacyApi) { + requestConfig.postPayload = { + form_data: { + ...finalFormData, + ...(force && { force: true }), + }, + }; + } else { + requestConfig.jsonPayload = { + ...queryContext, + ...(force && { force: true }), + }; + } + + const response = await chartClientRef.current!.client.post(requestConfig); + let responseData = Array.isArray(response.json) + ? response.json + : [response.json]; + + // Handle the nested result structure from the new API + if (!useLegacyApi && responseData[0]?.result) { + responseData = responseData[0].result; + } + + setStatus('loaded'); + setData(responseData); + setFormData(finalFormData); + + if (onLoad) { + onLoad(responseData); + } + } catch (err) { + // Ignore abort errors + if (err.name === 'AbortError') { + return; + } + + const errorObj = err as Error; + setStatus('error'); + setError(errorObj); + + if (onError) { + onError(errorObj); + } + } + }, []); + + // Combined effect for all prop changes and lifecycle + const prevPropsRef = useRef(); + useEffect(() => { + const currentProps = props; + const prevProps = prevPropsRef.current; + + // Update ref for next render + prevPropsRef.current = currentProps; + + // Initial mount or fundamental props changed - always refetch + if ( + !prevProps || + currentProps.chartId !== prevProps.chartId || + currentProps.formDataOverrides !== prevProps.formDataOverrides || + currentProps.force !== prevProps.force + ) { + fetchData(); + return; + } + + // Check if formData changed + if (currentProps.formData !== prevProps.formData) { + // Determine the viz type + const vizType = currentProps.formData?.viz_type || currentProps.chartType; + + // Check if we need to refetch data or just re-render + if ( + shouldRefetchData(prevProps.formData, currentProps.formData, vizType) + ) { + fetchData(); + } else { + // Just update the state to trigger re-render without fetching + setFormData(currentProps.formData); + } + } + }, [ + props.chartId, + props.formData, + props.formDataOverrides, + props.force, + props.chartType, + ]); + + // Cleanup effect + useEffect( + () => () => { + if (abortControllerRef.current) { + abortControllerRef.current.abort(); + } + }, + [], + ); + + // Render logic + const { + width = '100%', + height = 400, + showLoading = true, + loadingComponent: LoadingComponent, + errorComponent: ErrorComponent, + noDataComponent: NoDataComponent, + disableErrorBoundary, + enableNoResults = true, + id, + className, + onRenderSuccess, + onRenderFailure, + hooks, + } = props; + + if (status === 'loading' && showLoading) { + if (LoadingComponent) { + return ; + } + + // If using percentage sizing, wrap Loading in a container + if (width === '100%' || height === '100%') { + return ( +
+ +
+ ); + } + + return ( +
+ +
+ ); + } + + if (status === 'error' && error) { + if (ErrorComponent) { + return ; + } + + const errorDiv = ( +
+ Error: {error.message} +
+ ); + + // If using percentage sizing, wrap in a container + if (width === '100%' || height === '100%') { + return
{errorDiv}
; + } + + return ( +
+ {errorDiv} +
+ ); + } + + if (status === 'loaded' && formData && data) { + // Check if we need dynamic sizing + const needsDynamicSizing = width === '100%' || height === '100%'; + + const renderChart = ( + chartWidth: number | string, + chartHeight: number | string, + ) => ( + } + onRenderSuccess={onRenderSuccess} + onRenderFailure={onRenderFailure} + hooks={hooks} + /> + ); + + if (needsDynamicSizing) { + return ( +
+ + {({ width: parentWidth, height: parentHeight }) => { + const finalWidth = width === '100%' ? parentWidth : width; + const finalHeight = height === '100%' ? parentHeight : height; + return renderChart(finalWidth, finalHeight); + }} + +
+ ); + } + + return renderChart(width, height); + } + + return null; +} diff --git a/superset-frontend/packages/superset-ui-core/src/chart/components/SuperChart.tsx b/superset-frontend/packages/superset-ui-core/src/chart/components/SuperChart.tsx index b29890d4a41..553a98f568a 100644 --- a/superset-frontend/packages/superset-ui-core/src/chart/components/SuperChart.tsx +++ b/superset-frontend/packages/superset-ui-core/src/chart/components/SuperChart.tsx @@ -39,6 +39,8 @@ import SuperChartCore, { Props as SuperChartCoreProps } from './SuperChartCore'; import DefaultFallbackComponent from './FallbackComponent'; import ChartProps, { ChartPropsConfig } from '../models/ChartProps'; import NoResultsComponent from './NoResultsComponent'; +import { isMatrixifyEnabled } from '../types/matrixify'; +import MatrixifyGridRenderer from './Matrixify/MatrixifyGridRenderer'; const defaultProps = { FallbackComponent: DefaultFallbackComponent, @@ -186,8 +188,47 @@ class SuperChart extends PureComponent { theme, }); - let chart; - // Render the no results component if the query data is null or empty + // Check if Matrixify is enabled - use rawFormData (snake_case) + const matrixifyEnabled = isMatrixifyEnabled(chartProps.rawFormData); + + if (matrixifyEnabled) { + // When matrixify is enabled, queriesData is expected to be empty + // since each cell fetches its own data via StatefulChart + const matrixifyChart = ( + + ); + + // Apply wrapper if provided + const wrappedChart = Wrapper ? ( + + {matrixifyChart} + + ) : ( + matrixifyChart + ); + + // Include error boundary unless disabled + return disableErrorBoundary === true ? ( + wrappedChart + ) : ( + ( + + )} + onError={onErrorBoundary} + > + {wrappedChart} + + ); + } + + // Check for no results only for non-matrixified charts const noResultQueries = enableNoResults && (!queriesData || @@ -196,6 +237,8 @@ class SuperChart extends PureComponent { .every( ({ data }) => !data || (Array.isArray(data) && data.length === 0), )); + + let chart; if (noResultQueries) { chart = noResults || ( false); + +export const MatrixifyGridRenderer = jest.fn(() => null); diff --git a/superset-frontend/packages/superset-ui-core/src/chart/types/matrixify.test.ts b/superset-frontend/packages/superset-ui-core/src/chart/types/matrixify.test.ts new file mode 100644 index 00000000000..bb22958028e --- /dev/null +++ b/superset-frontend/packages/superset-ui-core/src/chart/types/matrixify.test.ts @@ -0,0 +1,271 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { + isMatrixifyEnabled, + getMatrixifyConfig, + getMatrixifyValidationErrors, + MatrixifyFormData, +} from './matrixify'; +import { AdhocMetric } from '../../query/types/Metric'; + +const createMetric = (label: string): AdhocMetric => ({ + expressionType: 'SIMPLE', + column: { column_name: 'value' }, + aggregate: 'SUM', + label, +}); + +test('isMatrixifyEnabled should return false when no matrixify configuration exists', () => { + const formData = { viz_type: 'table' } as MatrixifyFormData; + expect(isMatrixifyEnabled(formData)).toBe(false); +}); + +test('isMatrixifyEnabled should return false when matrixify_enabled is false', () => { + const formData = { + viz_type: 'table', + matrixify_enabled: false, + matrixify_mode_rows: 'metrics', + matrixify_rows: [createMetric('Revenue')], + } as MatrixifyFormData; + + expect(isMatrixifyEnabled(formData)).toBe(false); +}); + +test('isMatrixifyEnabled should return true for valid metrics mode configuration', () => { + const formData = { + viz_type: 'table', + matrixify_enabled: true, + matrixify_mode_rows: 'metrics', + matrixify_mode_columns: 'metrics', + matrixify_rows: [createMetric('Revenue')], + matrixify_columns: [createMetric('Q1')], + } as MatrixifyFormData; + + expect(isMatrixifyEnabled(formData)).toBe(true); +}); + +test('isMatrixifyEnabled should return true for valid dimensions mode configuration', () => { + const formData = { + viz_type: 'table', + matrixify_enabled: true, + matrixify_mode_rows: 'dimensions', + matrixify_mode_columns: 'dimensions', + matrixify_dimension_rows: { dimension: 'country', values: ['USA'] }, + matrixify_dimension_columns: { dimension: 'product', values: ['Widget'] }, + } as MatrixifyFormData; + + expect(isMatrixifyEnabled(formData)).toBe(true); +}); + +test('isMatrixifyEnabled should return true for mixed mode configuration', () => { + const formData = { + viz_type: 'table', + matrixify_enabled: true, + matrixify_mode_rows: 'metrics', + matrixify_mode_columns: 'dimensions', + matrixify_rows: [createMetric('Revenue')], + matrixify_dimension_columns: { dimension: 'country', values: ['USA'] }, + } as MatrixifyFormData; + + expect(isMatrixifyEnabled(formData)).toBe(true); +}); + +test('isMatrixifyEnabled should return true for topn dimension selection mode', () => { + const formData = { + viz_type: 'table', + matrixify_enabled: true, + matrixify_mode_rows: 'dimensions', + matrixify_mode_columns: 'dimensions', + matrixify_dimension_rows: { + dimension: 'country', + values: [], + selectionMode: 'topn', + topNMetric: 'revenue', + topNValue: 5, + }, + matrixify_dimension_columns: { dimension: 'product', values: ['Widget'] }, + } as MatrixifyFormData; + + expect(isMatrixifyEnabled(formData)).toBe(true); +}); + +test('isMatrixifyEnabled should return false when both axes have empty metrics arrays', () => { + const formData = { + viz_type: 'table', + matrixify_enabled: true, + matrixify_mode_rows: 'metrics', + matrixify_mode_columns: 'metrics', + matrixify_rows: [], + matrixify_columns: [], + } as MatrixifyFormData; + + expect(isMatrixifyEnabled(formData)).toBe(false); +}); + +test('isMatrixifyEnabled should return false when both dimensions have empty values and no topn mode', () => { + const formData = { + viz_type: 'table', + matrixify_enabled: true, + matrixify_mode_rows: 'dimensions', + matrixify_mode_columns: 'dimensions', + matrixify_dimension_rows: { dimension: 'country', values: [] }, + matrixify_dimension_columns: { dimension: 'product', values: [] }, + } as MatrixifyFormData; + + expect(isMatrixifyEnabled(formData)).toBe(false); +}); + +test('getMatrixifyConfig should return null when no matrixify configuration exists', () => { + const formData = { viz_type: 'table' } as MatrixifyFormData; + expect(getMatrixifyConfig(formData)).toBeNull(); +}); + +test('getMatrixifyConfig should return valid config for metrics mode', () => { + const formData = { + viz_type: 'table', + matrixify_enabled: true, + matrixify_mode_rows: 'metrics', + matrixify_mode_columns: 'metrics', + matrixify_rows: [createMetric('Revenue')], + matrixify_columns: [createMetric('Q1')], + } as MatrixifyFormData; + + const config = getMatrixifyConfig(formData); + expect(config).not.toBeNull(); + expect(config!.rows.mode).toBe('metrics'); + expect(config!.columns.mode).toBe('metrics'); + expect(config!.rows.metrics).toEqual([createMetric('Revenue')]); + expect(config!.columns.metrics).toEqual([createMetric('Q1')]); +}); + +test('getMatrixifyConfig should return valid config for dimensions mode', () => { + const formData = { + viz_type: 'table', + matrixify_enabled: true, + matrixify_mode_rows: 'dimensions', + matrixify_mode_columns: 'dimensions', + matrixify_dimension_rows: { dimension: 'country', values: ['USA'] }, + matrixify_dimension_columns: { dimension: 'product', values: ['Widget'] }, + } as MatrixifyFormData; + + const config = getMatrixifyConfig(formData); + expect(config).not.toBeNull(); + expect(config!.rows.mode).toBe('dimensions'); + expect(config!.columns.mode).toBe('dimensions'); + expect(config!.rows.dimension).toEqual({ + dimension: 'country', + values: ['USA'], + }); + expect(config!.columns.dimension).toEqual({ + dimension: 'product', + values: ['Widget'], + }); +}); + +test('getMatrixifyConfig should handle topn selection mode', () => { + const formData = { + viz_type: 'table', + matrixify_enabled: true, + matrixify_mode_rows: 'dimensions', + matrixify_mode_columns: 'dimensions', + matrixify_dimension_rows: { + dimension: 'country', + values: [], + selectionMode: 'topn', + topNMetric: 'revenue', + topNValue: 10, + }, + matrixify_dimension_columns: { dimension: 'product', values: ['Widget'] }, + } as MatrixifyFormData; + + const config = getMatrixifyConfig(formData); + expect(config).not.toBeNull(); + expect(config!.rows.dimension).toEqual(formData.matrixify_dimension_rows); +}); + +test('getMatrixifyValidationErrors should return empty array when matrixify is not enabled', () => { + const formData = { + viz_type: 'table', + matrixify_enabled: false, + } as MatrixifyFormData; + + expect(getMatrixifyValidationErrors(formData)).toEqual([]); +}); + +test('getMatrixifyValidationErrors should return empty array when properly configured', () => { + const formData = { + viz_type: 'table', + matrixify_enabled: true, + matrixify_mode_rows: 'metrics', + matrixify_mode_columns: 'metrics', + matrixify_rows: [createMetric('Revenue')], + matrixify_columns: [createMetric('Q1')], + } as MatrixifyFormData; + + expect(getMatrixifyValidationErrors(formData)).toEqual([]); +}); + +test('getMatrixifyValidationErrors should return error when enabled but no configuration exists', () => { + const formData = { + viz_type: 'table', + matrixify_enabled: true, + } as MatrixifyFormData; + + const errors = getMatrixifyValidationErrors(formData); + expect(errors).toContain('Please configure at least one row or column axis'); +}); + +test('getMatrixifyValidationErrors should return error when metrics mode has no metrics', () => { + const formData = { + viz_type: 'table', + matrixify_enabled: true, + matrixify_mode_rows: 'metrics', + matrixify_rows: [], + matrixify_columns: [], + } as MatrixifyFormData; + + const errors = getMatrixifyValidationErrors(formData); + expect(errors.length).toBeGreaterThan(0); +}); + +test('should handle undefined form data', () => { + expect(() => isMatrixifyEnabled(undefined as any)).toThrow(); +}); + +test('should handle null form data', () => { + expect(() => isMatrixifyEnabled(null as any)).toThrow(); +}); + +test('should handle empty form data object', () => { + const formData = {} as MatrixifyFormData; + expect(isMatrixifyEnabled(formData)).toBe(false); +}); + +test('should handle partial configuration with one axis only', () => { + const formData = { + viz_type: 'table', + matrixify_enabled: true, + matrixify_mode_rows: 'metrics', + matrixify_rows: [createMetric('Revenue')], + // No columns configuration + } as MatrixifyFormData; + + expect(isMatrixifyEnabled(formData)).toBe(true); +}); diff --git a/superset-frontend/packages/superset-ui-core/src/chart/types/matrixify.ts b/superset-frontend/packages/superset-ui-core/src/chart/types/matrixify.ts new file mode 100644 index 00000000000..b3e8f8397b0 --- /dev/null +++ b/superset-frontend/packages/superset-ui-core/src/chart/types/matrixify.ts @@ -0,0 +1,338 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { AdhocMetric } from '../../query'; + +/** + * Constants for Matrixify filter generation + * These match the literal types used in Filter.ts + */ +export const MatrixifyFilterConstants = { + // Filter expression types + ExpressionType: { + SIMPLE: 'SIMPLE' as const, + SQL: 'SQL' as const, + }, + // Filter clauses + Clause: { + WHERE: 'WHERE' as const, + HAVING: 'HAVING' as const, + }, + // Filter operators + Operator: { + EQUALS: '==' as const, + NOT_EQUALS: '!=' as const, + IN: 'IN' as const, + NOT_IN: 'NOT IN' as const, + }, +}; + +/** + * Mode for selecting matrix axis values + */ +export type MatrixifyMode = 'metrics' | 'dimensions'; + +/** + * Selection method for dimension values + */ +export type MatrixifySelectionMode = 'members' | 'topn'; + +/** + * Sort order for top N selection + */ +export type MatrixifySortOrder = 'asc' | 'desc'; + +/** + * Dimension value selection containing both the dimension column and selected values + */ +export interface MatrixifyDimensionValue { + dimension: string; + values: any[]; +} + +/** + * Configuration for a single axis (rows or columns) in the matrix + */ +export interface MatrixifyAxisConfig { + /** Whether to use metrics or dimensions for this axis */ + mode: MatrixifyMode; + + /** Selected metrics when mode is 'metrics' */ + metrics?: AdhocMetric[]; + + /** Dimension selection mode when mode is 'dimensions' */ + selectionMode?: MatrixifySelectionMode; + + /** Selected dimension and values when mode is 'dimensions' */ + dimension?: MatrixifyDimensionValue; + + /** Top N value when selectionMode is 'topn' */ + topnValue?: number; + + /** Metric for ordering top N values */ + topnMetric?: AdhocMetric; + + /** Sort order for top N values */ + topnOrder?: MatrixifySortOrder; +} + +/** + * Complete Matrixify configuration in form data + */ +export interface MatrixifyFormData { + // Enable/disable matrixify functionality + matrixify_enabled?: boolean; + + // Row axis configuration + matrixify_mode_rows?: MatrixifyMode; + matrixify_rows?: AdhocMetric[]; + matrixify_dimension_selection_mode_rows?: MatrixifySelectionMode; + matrixify_dimension_rows?: MatrixifyDimensionValue; + matrixify_topn_value_rows?: number; + matrixify_topn_metric_rows?: AdhocMetric; + matrixify_topn_order_rows?: MatrixifySortOrder; + + // Column axis configuration + matrixify_mode_columns?: MatrixifyMode; + matrixify_columns?: AdhocMetric[]; + matrixify_dimension_selection_mode_columns?: MatrixifySelectionMode; + matrixify_dimension_columns?: MatrixifyDimensionValue; + matrixify_topn_value_columns?: number; + matrixify_topn_metric_columns?: AdhocMetric; + matrixify_topn_order_columns?: MatrixifySortOrder; + + // Grid layout configuration + matrixify_row_height?: number; + matrixify_fit_columns_dynamically?: boolean; + matrixify_charts_per_row?: number; + + // Cell configuration + matrixify_cell_title_template?: string; + + // Matrix display configuration + matrixify_show_row_labels?: boolean; + matrixify_show_column_headers?: boolean; +} + +/** + * Processed matrix configuration after form data is transformed + */ +export interface MatrixifyConfig { + rows: MatrixifyAxisConfig; + columns: MatrixifyAxisConfig; +} + +/** + * Helper function to extract Matrixify configuration from form data + */ +export function getMatrixifyConfig( + formData: MatrixifyFormData & any, +): MatrixifyConfig | null { + const hasRowConfig = formData.matrixify_mode_rows; + const hasColumnConfig = formData.matrixify_mode_columns; + + if (!hasRowConfig && !hasColumnConfig) { + return null; + } + + return { + rows: { + mode: formData.matrixify_mode_rows || 'metrics', + metrics: formData.matrixify_rows, + selectionMode: formData.matrixify_dimension_selection_mode_rows, + dimension: formData.matrixify_dimension_rows, + topnValue: formData.matrixify_topn_value_rows, + topnMetric: formData.matrixify_topn_metric_rows, + topnOrder: formData.matrixify_topn_order_rows, + }, + columns: { + mode: formData.matrixify_mode_columns || 'metrics', + metrics: formData.matrixify_columns, + selectionMode: formData.matrixify_dimension_selection_mode_columns, + dimension: formData.matrixify_dimension_columns, + topnValue: formData.matrixify_topn_value_columns, + topnMetric: formData.matrixify_topn_metric_columns, + topnOrder: formData.matrixify_topn_order_columns, + }, + }; +} + +/** + * Check if Matrixify is enabled and properly configured + */ +export function isMatrixifyEnabled(formData: MatrixifyFormData): boolean { + // First check if matrixify is explicitly enabled via checkbox + if (!formData.matrixify_enabled) { + return false; + } + + // Then validate that we have proper configuration + const config = getMatrixifyConfig(formData); + if (!config) { + return false; + } + + const hasRowData = + config.rows.mode === 'metrics' + ? config.rows.metrics && config.rows.metrics.length > 0 + : config.rows.dimension?.dimension && + (config.rows.selectionMode === 'topn' || + (config.rows.dimension.values && + config.rows.dimension.values.length > 0)); + + const hasColumnData = + config.columns.mode === 'metrics' + ? config.columns.metrics && config.columns.metrics.length > 0 + : config.columns.dimension?.dimension && + (config.columns.selectionMode === 'topn' || + (config.columns.dimension.values && + config.columns.dimension.values.length > 0)); + + return Boolean(hasRowData || hasColumnData); +} + +/** + * Get validation errors for matrixify configuration + */ +export function getMatrixifyValidationErrors( + formData: MatrixifyFormData, +): string[] { + const errors: string[] = []; + + // Only validate if matrixify is enabled + if (!formData.matrixify_enabled) { + return errors; + } + + const config = getMatrixifyConfig(formData); + if (!config) { + errors.push('Please configure at least one row or column axis'); + return errors; + } + + // Check row configuration (only if explicitly set in form data) + const hasRowMode = Boolean(formData.matrixify_mode_rows); + if (hasRowMode) { + const hasRowData = + config.rows.mode === 'metrics' + ? config.rows.metrics && config.rows.metrics.length > 0 + : config.rows.dimension?.dimension && + (config.rows.selectionMode === 'topn' || + (config.rows.dimension.values && + config.rows.dimension.values.length > 0)); + + if (!hasRowData) { + if (config.rows.mode === 'metrics') { + errors.push('Please select at least one metric for rows'); + } else { + errors.push('Please select a dimension and values for rows'); + } + } + } + + // Check column configuration (only if explicitly set in form data) + const hasColumnMode = Boolean(formData.matrixify_mode_columns); + if (hasColumnMode) { + const hasColumnData = + config.columns.mode === 'metrics' + ? config.columns.metrics && config.columns.metrics.length > 0 + : config.columns.dimension?.dimension && + (config.columns.selectionMode === 'topn' || + (config.columns.dimension.values && + config.columns.dimension.values.length > 0)); + + if (!hasColumnData) { + if (config.columns.mode === 'metrics') { + errors.push('Please select at least one metric for columns'); + } else { + errors.push('Please select a dimension and values for columns'); + } + } + } + + // Must have at least one valid axis + if (hasRowMode || hasColumnMode) { + const hasRowData = + config.rows.mode === 'metrics' + ? config.rows.metrics && config.rows.metrics.length > 0 + : config.rows.dimension?.dimension && + (config.rows.selectionMode === 'topn' || + (config.rows.dimension.values && + config.rows.dimension.values.length > 0)); + + const hasColumnData = + config.columns.mode === 'metrics' + ? config.columns.metrics && config.columns.metrics.length > 0 + : config.columns.dimension?.dimension && + (config.columns.selectionMode === 'topn' || + (config.columns.dimension.values && + config.columns.dimension.values.length > 0)); + + if (!hasRowData && !hasColumnData) { + errors.push('Configure at least one complete row or column axis'); + } + } else { + errors.push('Please configure at least one row or column axis'); + } + + return errors; +} + +/** + * Grid cell representing a single chart in the matrix + */ +export interface MatrixifyGridCell { + /** Unique identifier for this cell */ + id: string; + + /** Row index in the grid */ + row: number; + + /** Column index in the grid */ + col: number; + + /** Row label (metric name or dimension value) */ + rowLabel: string; + + /** Column label (metric name or dimension value) */ + colLabel: string; + + /** Computed title for the cell (from template or default) */ + title?: string; + + /** Form data for this specific cell's chart */ + formData: any; // This will be QueryFormData with appropriate filters/metrics +} + +/** + * Complete grid structure for rendering + */ +export interface MatrixifyGrid { + /** Row headers */ + rowHeaders: string[]; + + /** Column headers */ + colHeaders: string[]; + + /** 2D array of cells [row][col] */ + cells: (MatrixifyGridCell | null)[][]; + + /** Original form data used to generate the grid */ + baseFormData: MatrixifyFormData; +} diff --git a/superset-frontend/packages/superset-ui-core/src/components/Form/FormItem.tsx b/superset-frontend/packages/superset-ui-core/src/components/Form/FormItem.tsx index c95a4c944c9..31e8b740a64 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/Form/FormItem.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/Form/FormItem.tsx @@ -17,7 +17,7 @@ * under the License. */ import { Form } from 'antd'; -import { styled } from '@superset-ui/core'; +import { styled } from '../../theme'; export const FormItem = styled(Form.Item)` ${({ theme }) => ` diff --git a/superset-frontend/packages/superset-ui-core/src/components/Loading/index.tsx b/superset-frontend/packages/superset-ui-core/src/components/Loading/index.tsx index c97a27c2e7c..1c71d56a7d3 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/Loading/index.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/Loading/index.tsx @@ -17,8 +17,8 @@ * under the License. */ -import { styled } from '@superset-ui/core'; import cls from 'classnames'; +import { styled } from '../../theme'; import { Loading as Loader } from '../assets'; import type { LoadingProps } from './types'; diff --git a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts index de2c66204f7..2ac05404242 100644 --- a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts +++ b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts @@ -49,6 +49,7 @@ export enum FeatureFlag { FilterBarClosedByDefault = 'FILTERBAR_CLOSED_BY_DEFAULT', GlobalAsyncQueries = 'GLOBAL_ASYNC_QUERIES', ListviewsDefaultCardView = 'LISTVIEWS_DEFAULT_CARD_VIEW', + Matrixify = 'MATRIXIFY', ScheduledQueries = 'SCHEDULED_QUERIES', SqllabBackendPersistence = 'SQLLAB_BACKEND_PERSISTENCE', SqlValidatorsByEngine = 'SQL_VALIDATORS_BY_ENGINE', diff --git a/superset-frontend/packages/superset-ui-core/test/chart/components/SuperChart.test.tsx b/superset-frontend/packages/superset-ui-core/test/chart/components/SuperChart.test.tsx index fc5aaeea81c..a739748d2ee 100644 --- a/superset-frontend/packages/superset-ui-core/test/chart/components/SuperChart.test.tsx +++ b/superset-frontend/packages/superset-ui-core/test/chart/components/SuperChart.test.tsx @@ -32,6 +32,23 @@ import { BuggyChartPlugin, } from './MockChartPlugins'; +import { isMatrixifyEnabled } from '../../../src/chart/types/matrixify'; +import MatrixifyGridRenderer from '../../../src/chart/components/Matrixify/MatrixifyGridRenderer'; + +// Mock Matrixify imports +jest.mock('../../../src/chart/types/matrixify', () => ({ + isMatrixifyEnabled: jest.fn(() => false), + getMatrixifyConfig: jest.fn(() => null), +})); + +jest.mock( + '../../../src/chart/components/Matrixify/MatrixifyGridRenderer', + () => ({ + __esModule: true, + default: jest.fn(() => null), + }), +); + const DEFAULT_QUERY_DATA = { data: ['foo', 'bar'] }; const DEFAULT_QUERIES_DATA = [ { data: ['foo', 'bar'] }, @@ -439,4 +456,111 @@ describe('SuperChart', () => { document.body.removeChild(wrapper); }, 30000); }); + + it('should render MatrixifyGridRenderer when matrixify is enabled with empty data', () => { + const mockIsMatrixifyEnabled = isMatrixifyEnabled as jest.MockedFunction< + typeof isMatrixifyEnabled + >; + const mockMatrixifyGridRenderer = + MatrixifyGridRenderer as jest.MockedFunction< + typeof MatrixifyGridRenderer + >; + + mockIsMatrixifyEnabled.mockReturnValue(true); + + render( + , + ); + + expect(mockMatrixifyGridRenderer).toHaveBeenCalled(); + expect(screen.queryByText('No Results')).not.toBeInTheDocument(); + }); + + it('should render MatrixifyGridRenderer when matrixify is enabled with null data', () => { + const mockIsMatrixifyEnabled = isMatrixifyEnabled as jest.MockedFunction< + typeof isMatrixifyEnabled + >; + const mockMatrixifyGridRenderer = + MatrixifyGridRenderer as jest.MockedFunction< + typeof MatrixifyGridRenderer + >; + + mockIsMatrixifyEnabled.mockReturnValue(true); + + render( + , + ); + + expect(mockMatrixifyGridRenderer).toHaveBeenCalled(); + expect(screen.queryByText('No Results')).not.toBeInTheDocument(); + }); + + it('should ignore custom noResults component when matrixify is enabled', () => { + const mockIsMatrixifyEnabled = isMatrixifyEnabled as jest.MockedFunction< + typeof isMatrixifyEnabled + >; + const mockMatrixifyGridRenderer = + MatrixifyGridRenderer as jest.MockedFunction< + typeof MatrixifyGridRenderer + >; + + mockIsMatrixifyEnabled.mockReturnValue(true); + + const CustomNoResults = () =>
Custom No Data Message
; + + render( + } + />, + ); + + expect(mockMatrixifyGridRenderer).toHaveBeenCalled(); + expect( + screen.queryByText('Custom No Data Message'), + ).not.toBeInTheDocument(); + }); + + it('should apply error boundary to matrixify grid renderer', () => { + const mockIsMatrixifyEnabled = isMatrixifyEnabled as jest.MockedFunction< + typeof isMatrixifyEnabled + >; + const mockMatrixifyGridRenderer = + MatrixifyGridRenderer as jest.MockedFunction< + typeof MatrixifyGridRenderer + >; + + mockIsMatrixifyEnabled.mockReturnValue(true); + const onErrorBoundary = jest.fn(); + + render( + , + ); + + expect(mockMatrixifyGridRenderer).toHaveBeenCalled(); + expect(onErrorBoundary).not.toHaveBeenCalled(); + }); }); diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx index 91acc45c975..e21abf0d210 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx @@ -156,8 +156,8 @@ const allColumnsControl: typeof sharedControls.groupby = { freeForm: true, allowAll: true, commaChoosesOption: false, - optionRenderer: c => , - valueRenderer: c => , + optionRenderer: c => , + valueRenderer: c => , valueKey: 'column_name', mapStateToProps: ({ datasource, controls }, controlState) => ({ options: datasource?.columns || [], diff --git a/superset-frontend/plugins/plugin-chart-handlebars/package.json b/superset-frontend/plugins/plugin-chart-handlebars/package.json index c3dbeb5708c..4c80bffe103 100644 --- a/superset-frontend/plugins/plugin-chart-handlebars/package.json +++ b/superset-frontend/plugins/plugin-chart-handlebars/package.json @@ -27,7 +27,6 @@ "access": "public" }, "dependencies": { - "handlebars": "^4.7.8", "handlebars-group-by": "^1.0.1", "just-handlebars-helpers": "^1.0.19" }, @@ -35,6 +34,7 @@ "@superset-ui/chart-controls": "*", "@superset-ui/core": "*", "ace-builds": "^1.4.14", + "handlebars": "^4.7.8", "lodash": "^4.17.11", "dayjs": "^1.11.13", "react": "^17.0.2", diff --git a/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx index aae534fc579..25a63b8ef28 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx +++ b/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx @@ -111,8 +111,8 @@ const allColumnsControl: typeof sharedControls.groupby = { freeForm: true, allowAll: true, commaChoosesOption: false, - optionRenderer: c => , - valueRenderer: c => , + optionRenderer: c => , + valueRenderer: c => , valueKey: 'column_name', mapStateToProps: ({ datasource, controls }, controlState) => ({ options: datasource?.columns || [], @@ -817,6 +817,9 @@ const config: ControlPanelConfig = { }), visibility: isAggMode, }, + sections.matrixifyRowSection, + sections.matrixifyColumnSection, + sections.matrixifySection, ], formDataOverrides: formData => ({ ...formData, diff --git a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx index 7bc4bb22420..915f9aa0fab 100644 --- a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx +++ b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx @@ -122,6 +122,38 @@ const ChartContextMenu = ( filters?: ContextMenuFilters; }>({ clientX: 0, clientY: 0 }); + // Extract matrixifyContext if present and merge cell filters + const enhancedFilters = useMemo(() => { + if (!filters) return filters; + + // Check if this is from a matrixified cell + const matrixifyContext = (filters as any)?.matrixifyContext; + if (!matrixifyContext) return filters; + + // Merge cell filters with drill filters + const enhancedDrillBy = filters.drillBy + ? { + ...filters.drillBy, + filters: [ + ...(filters.drillBy.filters || []), + ...(matrixifyContext.cellFilters || []), + ], + } + : undefined; + + return { + ...filters, + drillBy: enhancedDrillBy, + }; + }, [filters]); + + // Use cell's formData for drill-to-detail if from matrixified cell + const drillFormData = useMemo(() => { + const matrixifyContext = (filters as any)?.matrixifyContext; + // If this is from a matrixified cell, use the cell's formData which includes adhoc_filters + return matrixifyContext?.cellFormData || formData; + }, [filters, formData]); + const [drillModalIsOpen, setDrillModalIsOpen] = useState(false); const [drillByColumn, setDrillByColumn] = useState(); const [showDrillByModal, setShowDrillByModal] = useState(false); @@ -155,7 +187,8 @@ const ChartContextMenu = ( const showDrillBy = isFeatureEnabled(FeatureFlag.DrillBy) && canDrillBy && - isDisplayed(ContextMenuItem.DrillBy); + isDisplayed(ContextMenuItem.DrillBy) && + !formData.matrixify_enabled; // Disable drill by when matrixify is enabled const datasetResource = useDatasetDrillInfo( formData.datasource, @@ -200,9 +233,9 @@ const ChartContextMenu = ( datasetResource.status, datasetResource.result, showDrillBy, - filters?.drillBy?.groupbyFieldName, + enhancedFilters?.drillBy?.groupbyFieldName, formData.x_axis, - formData[filters?.drillBy?.groupbyFieldName ?? ''], + formData[enhancedFilters?.drillBy?.groupbyFieldName ?? ''], additionalConfig?.drillBy?.excludedColumns, loadDrillByOptionsExtension, ]); @@ -298,7 +331,7 @@ const ChartContextMenu = ( if (showDrillToDetail) { menuItems.push( { setDrillModalIsOpen(false); @@ -421,10 +454,10 @@ const ChartContextMenu = ( {showDrillByModal && drillByColumn && filteredDataset && - filters?.drillBy && ( + enhancedFilters?.drillBy && ( { + if (!nextProps.formData.matrixify_enabled) return false; + + // Check all matrixify-related properties + const matrixifyKeys = Object.keys(nextProps.formData).filter(key => + key.startsWith('matrixify_'), + ); + + return matrixifyKeys.some( + key => !isEqual(nextProps.formData[key], this.props.formData[key]), + ); + }; + return ( this.hasQueryResponseChange || !isEqual(nextProps.datasource, this.props.datasource) || @@ -167,7 +181,8 @@ class ChartRenderer extends Component { nextProps.formData.subcategories !== this.props.formData.subcategories || nextProps.cacheBusterProp !== this.props.cacheBusterProp || - nextProps.emitCrossFilters !== this.props.emitCrossFilters + nextProps.emitCrossFilters !== this.props.emitCrossFilters || + hasMatrixifyChanges() ); } return false; diff --git a/superset-frontend/src/components/Chart/ChartRenderer.test.jsx b/superset-frontend/src/components/Chart/ChartRenderer.test.jsx index 13ba818d3c5..90442564b59 100644 --- a/superset-frontend/src/components/Chart/ChartRenderer.test.jsx +++ b/superset-frontend/src/components/Chart/ChartRenderer.test.jsx @@ -91,3 +91,180 @@ test('should not render chart context menu if the context menu is suppressed for ); expect(queryByTestId('mock-chart-context-menu')).not.toBeInTheDocument(); }); + +test('should detect changes in matrixify properties', () => { + const initialProps = { + ...requiredProps, + formData: { + ...requiredProps.formData, + matrixify_enabled: true, + matrixify_dimension_x: { dimension: 'country', values: ['USA'] }, + matrixify_dimension_y: { dimension: 'category', values: ['Tech'] }, + matrixify_charts_per_row: 3, + matrixify_show_row_labels: true, + }, + queriesResponse: [{ data: 'initial' }], + chartStatus: 'success', + }; + + // eslint-disable-next-line no-unused-vars + const wrapper = render(); + + // Since we can't directly test shouldComponentUpdate, we verify the component + // correctly identifies matrixify-related properties by checking the implementation + expect(initialProps.formData.matrixify_enabled).toBe(true); + expect(initialProps.formData.matrixify_dimension_x).toEqual({ + dimension: 'country', + values: ['USA'], + }); +}); + +test('should identify matrixify property changes correctly', () => { + // Test that formData with different matrixify properties triggers updates + const initialProps = { + ...requiredProps, + formData: { + matrixify_enabled: true, + matrixify_dimension_x: { dimension: 'country', values: ['USA'] }, + matrixify_charts_per_row: 3, + }, + queriesResponse: [{ data: 'current' }], + chartStatus: 'success', + }; + + const { rerender, getByTestId } = render(); + + expect(getByTestId('mock-super-chart')).toHaveTextContent( + JSON.stringify(initialProps.formData), + ); + + // Update with changed matrixify_dimension_x values + const updatedProps = { + ...initialProps, + formData: { + matrixify_enabled: true, + matrixify_dimension_x: { + dimension: 'country', + values: ['USA', 'Canada'], // Changed + }, + matrixify_charts_per_row: 3, + }, + }; + + rerender(); + + // Verify the component re-rendered with new props + expect(getByTestId('mock-super-chart')).toHaveTextContent( + JSON.stringify(updatedProps.formData), + ); +}); + +test('should handle matrixify-related form data changes', () => { + const initialProps = { + ...requiredProps, + formData: { + matrixify_enabled: false, + regular_control: 'value1', + }, + queriesResponse: [{ data: 'current' }], + chartStatus: 'success', + }; + + const { rerender, getByTestId } = render(); + + expect(getByTestId('mock-super-chart')).toHaveTextContent( + JSON.stringify(initialProps.formData), + ); + + // Enable matrixify + const updatedProps = { + ...initialProps, + formData: { + matrixify_enabled: true, // This is a significant change + regular_control: 'value1', + }, + }; + + rerender(); + + // Verify the component re-rendered with matrixify enabled + expect(getByTestId('mock-super-chart')).toHaveTextContent( + JSON.stringify(updatedProps.formData), + ); +}); + +test('should detect matrixify property addition', () => { + const initialProps = { + ...requiredProps, + formData: { + matrixify_enabled: true, + // No matrixify_dimension_x initially + }, + queriesResponse: [{ data: 'current' }], + chartStatus: 'success', + }; + + const { rerender, getByTestId } = render(); + + expect(getByTestId('mock-super-chart')).toHaveTextContent( + JSON.stringify(initialProps.formData), + ); + + // Add matrixify_dimension_x + const updatedProps = { + ...initialProps, + formData: { + matrixify_enabled: true, + matrixify_dimension_x: { dimension: 'country', values: ['USA'] }, // Added + }, + }; + + rerender(); + + // Verify the component re-rendered with the new property + expect(getByTestId('mock-super-chart')).toHaveTextContent( + JSON.stringify(updatedProps.formData), + ); +}); + +test('should detect nested matrixify property changes', () => { + const initialProps = { + ...requiredProps, + formData: { + matrixify_enabled: true, + matrixify_dimension_x: { + dimension: 'country', + values: ['USA'], + topN: { metric: 'sales', value: 10 }, + }, + }, + queriesResponse: [{ data: 'current' }], + chartStatus: 'success', + }; + + const { rerender, getByTestId } = render(); + + expect(getByTestId('mock-super-chart')).toHaveTextContent( + JSON.stringify(initialProps.formData), + ); + + // Change nested topN value + const updatedProps = { + ...initialProps, + formData: { + matrixify_enabled: true, + matrixify_dimension_x: { + dimension: 'country', + values: ['USA'], + topN: { metric: 'sales', value: 15 }, // Nested change + }, + }, + }; + + rerender(); + + // Verify the component re-rendered with the nested change + expect(getByTestId('mock-super-chart')).toHaveTextContent( + JSON.stringify(updatedProps.formData), + ); +}); diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx index 3d944c496a7..3ac4a6dd8fc 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx +++ b/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx @@ -200,6 +200,11 @@ export const DrillByMenuItems = ({ ); }; + // Don't render drill by menu items when matrixify is enabled + if (formData.matrixify_enabled) { + return null; + } + return ( <> { - const datasources = json?.result ?? []; - if (datasources.length) { - dispatch(setDatasources(datasources)); - } - }); + }) + .then(({ json }) => { + const datasources = json?.result ?? []; + if (datasources.length) { + dispatch(setDatasources(datasources)); + } + }) + .catch(error => { + logging.error('Error fetching dashboard datasets:', error); + }); } if (lastModifiedTime) { dispatch(saveDashboardRequestSuccess(lastModifiedTime)); @@ -887,7 +892,7 @@ export const applyDashboardLabelsColorOnLoad = metadata => async dispatch => { dispatch(setDashboardLabelsColorMapSync()); } } catch (e) { - console.error('Failed to update dashboard color on load:', e); + logging.error('Failed to update dashboard color on load:', e); } }; @@ -1054,6 +1059,6 @@ export const updateDashboardLabelsColor = renderedChartIds => (_, getState) => { // 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); + logging.error('Failed to update colors for new charts and labels:', e); } }; diff --git a/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx b/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx index 599d271d36d..36743a1554c 100644 --- a/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx +++ b/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx @@ -380,13 +380,22 @@ describe('PropertiesModal', () => { document.querySelectorAll(className)! as NodeListOf; const findAllSelectOptions = () => - waitFor(() => getElementsByClassName('.ant-select-item-option-content')); + waitFor(() => { + const elements = getElementsByClassName( + '.ant-select-item-option-content', + ); + if (elements.length === 0) throw new Error('No options found'); + return elements; + }); render(, { useRedux: true, }); - expect(screen.getAllByRole('combobox')).toHaveLength(3); + await waitFor(() => { + expect(screen.getAllByRole('combobox')).toHaveLength(3); + }); + expect( screen.getByRole('combobox', { name: SupersetCore.t('Roles') }), ).toBeInTheDocument(); @@ -397,7 +406,7 @@ describe('PropertiesModal', () => { expect(options).toHaveLength(5); expect(options[0]).toHaveTextContent('Admin'); - }); + }, 30000); test('should show active owners with dashboard rbac', async () => { mockedIsFeatureEnabled.mockReturnValue(true); @@ -405,21 +414,33 @@ describe('PropertiesModal', () => { const props = createProps(); const propsWithDashboardInfo = { ...props, dashboardInfo }; - const open = () => waitFor(() => userEvent.click(getSelect())); const getSelect = () => screen.getByRole('combobox', { name: SupersetCore.t('Owners') }); + const open = () => waitFor(() => userEvent.click(getSelect())); const getElementsByClassName = (className: string) => document.querySelectorAll(className)! as NodeListOf; const findAllSelectOptions = () => - waitFor(() => getElementsByClassName('.ant-select-item-option-content')); + waitFor( + () => { + const elements = getElementsByClassName( + '.ant-select-item-option-content', + ); + if (elements.length === 0) throw new Error('No options found'); + return elements; + }, + { timeout: 5000 }, + ); render(, { useRedux: true, }); - expect(screen.getAllByRole('combobox')).toHaveLength(3); + await waitFor(() => { + expect(screen.getAllByRole('combobox')).toHaveLength(3); + }); + expect( screen.getByRole('combobox', { name: SupersetCore.t('Owners') }), ).toBeInTheDocument(); @@ -430,7 +451,7 @@ describe('PropertiesModal', () => { expect(options).toHaveLength(1); expect(options[0]).toHaveTextContent('Superset Admin'); - }); + }, 30000); test('should show active owners without dashboard rbac', async () => { mockedIsFeatureEnabled.mockReturnValue(false); @@ -445,13 +466,25 @@ describe('PropertiesModal', () => { document.querySelectorAll(className)! as NodeListOf; const findAllSelectOptions = () => - waitFor(() => getElementsByClassName('.ant-select-item-option-content')); + waitFor( + () => { + const elements = getElementsByClassName( + '.ant-select-item-option-content', + ); + if (elements.length === 0) throw new Error('No options found'); + return elements; + }, + { timeout: 5000 }, + ); render(, { useRedux: true, }); - expect(screen.getByRole('combobox')).toBeInTheDocument(); + await waitFor(() => { + expect(screen.getByRole('combobox')).toBeInTheDocument(); + }); + expect( screen.getByRole('combobox', { name: SupersetCore.t('Owners') }), ).toBeInTheDocument(); @@ -462,5 +495,5 @@ describe('PropertiesModal', () => { expect(options).toHaveLength(1); expect(options[0]).toHaveTextContent('Superset Admin'); - }); + }, 30000); }); diff --git a/superset-frontend/src/explore/actions/hydrateExplore.test.ts b/superset-frontend/src/explore/actions/hydrateExplore.test.ts index f0703699147..6fe24882bc5 100644 --- a/superset-frontend/src/explore/actions/hydrateExplore.test.ts +++ b/superset-frontend/src/explore/actions/hydrateExplore.test.ts @@ -36,60 +36,60 @@ test('creates hydrate action from initial data', () => { expect(dispatch).toHaveBeenCalledWith( expect.objectContaining({ type: HYDRATE_EXPLORE, - data: { - charts: { - 371: { + data: expect.objectContaining({ + charts: expect.objectContaining({ + 371: expect.objectContaining({ id: 371, chartAlert: null, chartStatus: null, chartStackTrace: null, chartUpdateEndTime: null, chartUpdateStartTime: 0, - latestQueryFormData: { + latestQueryFormData: expect.objectContaining({ cache_timeout: undefined, datasource: '8__table', slice_id: 371, url_params: undefined, viz_type: VizType.Table, - }, - sliceFormData: { + }), + sliceFormData: expect.objectContaining({ cache_timeout: undefined, datasource: '8__table', slice_id: 371, url_params: undefined, viz_type: VizType.Table, - }, + }), queryController: null, queriesResponse: null, triggerQuery: false, lastRendered: 0, - }, - }, - datasources: { - '8__table': exploreInitialData.dataset, - }, - saveModal: { + }), + }), + datasources: expect.objectContaining({ + '8__table': expect.anything(), + }), + saveModal: expect.objectContaining({ dashboards: [], saveModalAlert: null, isVisible: false, - }, - explore: { + }), + explore: expect.objectContaining({ can_add: false, can_download: false, can_overwrite: false, isDatasourceMetaLoading: false, isStarred: false, triggerRender: false, - datasource: exploreInitialData.dataset, + datasource: expect.anything(), controls: expect.any(Object), - form_data: exploreInitialData.form_data, - slice: exploreInitialData.slice, + form_data: expect.anything(), + slice: expect.anything(), standalone: null, force: null, saveAction: null, common: {}, - }, - }, + }), + }), }), ); }); @@ -109,61 +109,61 @@ test('creates hydrate action with existing state', () => { expect(dispatch).toHaveBeenCalledWith( expect.objectContaining({ type: HYDRATE_EXPLORE, - data: { - charts: { - 371: { + data: expect.objectContaining({ + charts: expect.objectContaining({ + 371: expect.objectContaining({ id: 371, chartAlert: null, chartStatus: null, chartStackTrace: null, chartUpdateEndTime: null, chartUpdateStartTime: 0, - latestQueryFormData: { + latestQueryFormData: expect.objectContaining({ cache_timeout: undefined, datasource: '8__table', slice_id: 371, url_params: undefined, viz_type: VizType.Table, - }, - sliceFormData: { + }), + sliceFormData: expect.objectContaining({ cache_timeout: undefined, datasource: '8__table', slice_id: 371, url_params: undefined, viz_type: VizType.Table, - }, + }), queryController: null, queriesResponse: null, triggerQuery: false, lastRendered: 0, - }, - }, - datasources: { - '8__table': exploreInitialData.dataset, - }, - saveModal: { + }), + }), + datasources: expect.objectContaining({ + '8__table': expect.anything(), + }), + saveModal: expect.objectContaining({ dashboards: [], saveModalAlert: null, isVisible: false, - }, - explore: { + }), + explore: expect.objectContaining({ can_add: false, can_download: false, can_overwrite: false, isDatasourceMetaLoading: false, isStarred: false, triggerRender: false, - datasource: exploreInitialData.dataset, + datasource: expect.anything(), controls: expect.any(Object), controlsTransferred: ['all_columns'], - form_data: exploreInitialData.form_data, - slice: exploreInitialData.slice, + form_data: expect.anything(), + slice: expect.anything(), standalone: null, force: null, saveAction: null, common: {}, - }, - }, + }), + }), }), ); }); diff --git a/superset-frontend/src/explore/actions/saveModalActions.ts b/superset-frontend/src/explore/actions/saveModalActions.ts index 4c9cc0badd6..2e1e48431c5 100644 --- a/superset-frontend/src/explore/actions/saveModalActions.ts +++ b/superset-frontend/src/explore/actions/saveModalActions.ts @@ -159,6 +159,15 @@ export const getSlicePayload = async ( } } + const queryContext = await buildV1ChartDataPayload({ + formData, + force: false, + resultFormat: 'json', + resultType: 'full', + setDataMask: null, + ownState: null, + }); + const payload: Partial = { params: JSON.stringify(formData), slice_name: sliceName, @@ -167,16 +176,7 @@ export const getSlicePayload = async ( datasource_type: datasourceType, dashboards, owners, - query_context: JSON.stringify( - await buildV1ChartDataPayload({ - formData, - force: false, - resultFormat: 'json', - resultType: 'full', - setDataMask: null, - ownState: null, - }), - ), + query_context: JSON.stringify(queryContext), }; return payload; diff --git a/superset-frontend/src/explore/components/ChartPills.tsx b/superset-frontend/src/explore/components/ChartPills.tsx index f11761b4ee0..cabbc4c5616 100644 --- a/superset-frontend/src/explore/components/ChartPills.tsx +++ b/superset-frontend/src/explore/components/ChartPills.tsx @@ -41,6 +41,7 @@ export type ChartPillsProps = { chartUpdateEndTime?: number; refreshCachedQuery: () => void; rowLimit?: string | number; + hideRowCount?: boolean; }; export const ChartPills = forwardRef( @@ -52,6 +53,7 @@ export const ChartPills = forwardRef( chartUpdateEndTime, refreshCachedQuery, rowLimit, + hideRowCount = false, }: ChartPillsProps, ref: RefObject, ) => { @@ -67,7 +69,7 @@ export const ChartPills = forwardRef( padding-bottom: ${theme.sizeUnit * 4}px; `} > - {!isLoading && firstQueryResponse && ( + {!isLoading && !hideRowCount && firstQueryResponse && ( { - // if at least one control in the section is not `renderTrigger` - // or asks to be displayed at the Data tab - if ( + if (!section) return; + if (section.tabOverride === 'matrixify') { + // Separate the enable control from other sections + if (section.label === t('Enable Matrixify')) { + matrixifyEnableControl = section; + } else { + matrixifySections.push(section); + } + } else if ( section.tabOverride === 'data' || section.controlSetRows.some(rows => rows.some( @@ -205,10 +217,11 @@ function getState( ) ) { querySections.push(section); - } else if (section.controlSetRows.length > 0) { + } else if (section.controlSetRows && section.controlSetRows.length > 0) { customizeSections.push(section); } }); + const expandedQuerySections: string[] = sectionsToExpand( querySections, datasource, @@ -217,11 +230,19 @@ function getState( customizeSections, datasource, ); + const expandedMatrixifySections: string[] = sectionsToExpand( + matrixifySections, + datasource, + ); + return { expandedQuerySections, expandedCustomizeSections, + expandedMatrixifySections, querySections, customizeSections, + matrixifySections, + matrixifyEnableControl, }; } @@ -369,8 +390,11 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => { const { expandedQuerySections, expandedCustomizeSections, + expandedMatrixifySections, querySections, customizeSections, + matrixifySections, + matrixifyEnableControl, } = useMemo( () => getState( @@ -722,6 +746,30 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => { } const showCustomizeTab = customizeSections.length > 0; + const showMatrixifyTab = isFeatureEnabled(FeatureFlag.Matrixify); + + // Create Matrixify tab label with Beta tag + const matrixifyTabLabel = ( + <> + {t('Matrixify')}{' '} + + + + + ); return ( @@ -765,6 +813,55 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => { }, ] : []), + ...(showMatrixifyTab + ? [ + { + key: TABS_KEYS.MATRIXIFY, + label: matrixifyTabLabel, + children: ( + <> + {/* Render Enable Matrixify control outside collapsible sections */} + {matrixifyEnableControl && + ( + matrixifyEnableControl as ControlPanelSectionConfig + ).controlSetRows.map( + (controlSetRow: CustomControlItem[], i: number) => ( +
+ {controlSetRow.map( + (control: CustomControlItem, j: number) => { + if (!control || typeof control === 'string') { + return null; + } + return ( +
+ {renderControl(control)} +
+ ); + }, + )} +
+ ), + )} + + + ), + }, + ] + : []), ]} />
diff --git a/superset-frontend/src/explore/components/ExploreChartPanel/index.tsx b/superset-frontend/src/explore/components/ExploreChartPanel/index.tsx index a9fa4a11a4d..a01d7974202 100644 --- a/superset-frontend/src/explore/components/ExploreChartPanel/index.tsx +++ b/superset-frontend/src/explore/components/ExploreChartPanel/index.tsx @@ -395,6 +395,7 @@ const ExploreChartPanel = ({ queriesResponse: chart.queriesResponse, })} {...(chart.chartStatus && { chartStatus: chart.chartStatus })} + hideRowCount={formData?.matrixify_enabled === true} /> {renderChart()} @@ -411,6 +412,7 @@ const ExploreChartPanel = ({ chart.chartUpdateEndTime, refreshCachedQuery, formData?.row_limit, + formData?.matrixify_enabled, renderChart, ], ); diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx b/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx index 8670ac27c4b..44efbaaef51 100644 --- a/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx +++ b/superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx @@ -16,6 +16,8 @@ * specific language governing permissions and limitations * under the License. */ + +// Mock isMatrixifyEnabled before loading any modules import fetchMock from 'fetch-mock'; import { getChartControlPanelRegistry, @@ -33,6 +35,12 @@ import { } from 'spec/helpers/testing-library'; import ExploreViewContainer from '.'; +jest.doMock('@superset-ui/core', () => ({ + __esModule: true, + ...jest.requireActual('@superset-ui/core'), + isMatrixifyEnabled: jest.fn(() => false), +})); + const reduxState = { explore: { controls: { diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx index 40caa9decc9..7781f13045c 100644 --- a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx +++ b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx @@ -30,6 +30,7 @@ import { useChangeEffect, useComponentDidMount, usePrevious, + isMatrixifyEnabled, } from '@superset-ui/core'; import { debounce, isEqual, isObjectLike, omit, pick } from 'lodash'; import { Resizable } from 're-resizable'; @@ -323,10 +324,30 @@ function ExploreViewContainer(props) { const onQuery = useCallback(() => { props.actions.setForceQuery(false); + + // Skip main query if Matrixify is enabled + if (isMatrixifyEnabled(props.form_data)) { + // Set chart to success state since Matrixify will handle its own queries + props.actions.chartUpdateSucceeded([], props.chart.id); + props.actions.chartRenderingSucceeded(props.chart.id); + + // Update history and controls + addHistory(); + setLastQueriedControls(props.controls); + return; + } + + // Normal behavior for non-Matrixify props.actions.triggerQuery(true, props.chart.id); addHistory(); setLastQueriedControls(props.controls); - }, [props.controls, addHistory, props.actions, props.chart.id]); + }, [ + props.controls, + addHistory, + props.actions, + props.chart.id, + props.form_data, + ]); const handleKeydown = useCallback( event => { diff --git a/superset-frontend/src/explore/components/controls/MatrixifyControl/utils/fetchTopNValues.ts b/superset-frontend/src/explore/components/controls/MatrixifyControl/utils/fetchTopNValues.ts new file mode 100644 index 00000000000..dfd460a8f5b --- /dev/null +++ b/superset-frontend/src/explore/components/controls/MatrixifyControl/utils/fetchTopNValues.ts @@ -0,0 +1,92 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { getChartDataRequest } from 'src/components/Chart/chartAction'; +import { QueryFormData } from '@superset-ui/core'; + +export interface FetchTopNValuesParams { + datasource: string; + column: string; + metric: string; + limit: number; + sortAscending?: boolean; + filters?: any[]; + timeRange?: string; +} + +export interface TopNValue { + value: string | number; + metricValue: number; +} + +/** + * Fetches the top N values for a dimension sorted by a metric. + * Based on the pattern used in dashboard Select filters. + */ +export async function fetchTopNValues({ + datasource, + column, + metric, + limit, + sortAscending = false, + filters = [], + timeRange, +}: FetchTopNValuesParams): Promise { + const formData: Partial = { + datasource, + groupby: [column], + metrics: [metric], + adhoc_filters: filters, + time_range: timeRange, + row_limit: limit, + orderby: [[metric, sortAscending]], // false for DESC, true for ASC + viz_type: 'table', // Use table viz type for simple data fetching + }; + + try { + const response = await getChartDataRequest({ + formData: formData as QueryFormData, + force: false, + }); + + // Handle the response based on the structure + const result = response.json?.result?.[0]; + if (!result || !result.data) { + return []; + } + + // Extract values from the response data + // The data is typically an array of objects with column names as keys + return result.data.map((row: any) => ({ + value: row[column], + metricValue: row[metric], + })); + } catch (error) { + console.error('Error fetching top N values:', error); + throw error; + } +} + +/** + * Extracts just the dimension values from TopNValue array + */ +export function extractDimensionValues( + topNValues: TopNValue[], +): (string | number)[] { + return topNValues.map(item => item.value); +} diff --git a/superset-frontend/src/explore/components/controls/MatrixifyDimensionControl.test.tsx b/superset-frontend/src/explore/components/controls/MatrixifyDimensionControl.test.tsx new file mode 100644 index 00000000000..1ca08cf2e9d --- /dev/null +++ b/superset-frontend/src/explore/components/controls/MatrixifyDimensionControl.test.tsx @@ -0,0 +1,456 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { render, screen, waitFor } from 'spec/helpers/testing-library'; +import userEvent from '@testing-library/user-event'; +import { SupersetClient } from '@superset-ui/core'; +import MatrixifyDimensionControl, { + MatrixifyDimensionControlValue, +} from './MatrixifyDimensionControl'; + +import { fetchTopNValues } from './MatrixifyControl/utils/fetchTopNValues'; + +// Mock SupersetClient +jest.mock('@superset-ui/core', () => ({ + ...jest.requireActual('@superset-ui/core'), + SupersetClient: { + get: jest.fn(), + }, + t: (str: string, ...args: any[]) => { + if (args.length > 0 && str.includes('%s')) { + return str.replace('%s', args[0]); + } + return str; + }, + getColumnLabel: (col: string) => col, +})); + +// Mock fetchTopNValues utility +jest.mock('./MatrixifyControl/utils/fetchTopNValues', () => ({ + fetchTopNValues: jest.fn(), + extractDimensionValues: jest.fn(values => values.map((v: any) => v.value)), +})); + +// Mock ControlHeader +jest.mock('src/explore/components/ControlHeader', () => ({ + __esModule: true, + default: ({ label, description }: any) => ( +
+ {label && {label}} + {description && {description}} +
+ ), +})); + +const mockDatasource = { + id: 1, + type: 'table', + columns: [ + { column_name: 'country', verbose_name: 'Country' }, + { column_name: 'region', verbose_name: 'Region' }, + { column_name: 'product', verbose_name: 'Product' }, + ], + filter_select: true, +}; + +const defaultProps = { + datasource: mockDatasource, + onChange: jest.fn(), +}; + +beforeEach(() => { + jest.clearAllMocks(); +}); + +test('should render dimension selector with default label', () => { + render(); + + expect(screen.getByText('Dimension')).toBeInTheDocument(); + expect(screen.getAllByText('Select a dimension')).toHaveLength(2); // Header description + placeholder + expect( + screen.getByRole('combobox', { name: 'Select dimension' }), + ).toBeInTheDocument(); +}); + +test('should render with custom label and description', () => { + render( + , + ); + + expect(screen.getByText('Custom Label')).toBeInTheDocument(); + expect(screen.getByText('Custom Description')).toBeInTheDocument(); +}); + +test('should display dimension options from datasource columns', async () => { + render(); + + const select = screen.getByRole('combobox', { name: 'Select dimension' }); + await userEvent.click(select); + + await waitFor(() => { + expect(screen.getByText('country')).toBeInTheDocument(); + expect(screen.getByText('region')).toBeInTheDocument(); + expect(screen.getByText('product')).toBeInTheDocument(); + }); +}); + +test('should call onChange when dimension is selected', async () => { + const onChange = jest.fn(); + render(); + + const select = screen.getByRole('combobox', { name: 'Select dimension' }); + await userEvent.click(select); + await userEvent.click(screen.getByText('country')); + + expect(onChange).toHaveBeenCalledWith({ + dimension: 'country', + values: [], + }); +}); + +test('should display selected dimension value', () => { + const value: MatrixifyDimensionControlValue = { + dimension: 'country', + values: ['USA', 'Canada'], + }; + + render(); + + // Check that the component renders with the selected dimension + expect( + screen.getByRole('combobox', { name: 'Select dimension' }), + ).toBeInTheDocument(); + // In members mode, the value selector should also appear + expect( + screen.getByRole('combobox', { name: 'Select dimension values' }), + ).toBeInTheDocument(); +}); + +test('should show value selector in members mode when dimension is selected', () => { + const value: MatrixifyDimensionControlValue = { + dimension: 'country', + values: [], + }; + + (SupersetClient.get as jest.Mock).mockResolvedValue({ + json: { result: ['USA', 'Canada'] }, + }); + + render( + , + ); + + expect( + screen.getByRole('combobox', { name: 'Select dimension values' }), + ).toBeInTheDocument(); +}); + +test('should load dimension values from API in members mode', async () => { + const value: MatrixifyDimensionControlValue = { + dimension: 'country', + values: [], + }; + + (SupersetClient.get as jest.Mock).mockResolvedValue({ + json: { result: ['USA', 'Canada', 'Mexico'] }, + }); + + render( + , + ); + + await waitFor(() => { + expect(SupersetClient.get).toHaveBeenCalledWith({ + signal: expect.any(AbortSignal), + endpoint: '/api/v1/datasource/table/1/column/country/values/', + }); + }); +}); + +test('should handle API errors gracefully in members mode', async () => { + const value: MatrixifyDimensionControlValue = { + dimension: 'country', + values: [], + }; + + (SupersetClient.get as jest.Mock).mockRejectedValue(new Error('API Error')); + + render( + , + ); + + await waitFor(() => { + expect(SupersetClient.get).toHaveBeenCalled(); + }); +}); + +test('should not show value selector in topn mode', () => { + const value: MatrixifyDimensionControlValue = { + dimension: 'country', + values: [], + }; + + render( + , + ); + + expect( + screen.queryByRole('combobox', { name: 'Select dimension values' }), + ).not.toBeInTheDocument(); +}); + +test('should fetch TopN values when all params are provided', async () => { + const mockFetchTopNValues = fetchTopNValues as jest.MockedFunction< + typeof fetchTopNValues + >; + const onChange = jest.fn(); + const value: MatrixifyDimensionControlValue = { + dimension: 'country', + values: [], + }; + + mockFetchTopNValues.mockResolvedValue([ + { value: 'USA', metricValue: 1000 }, + { value: 'Canada', metricValue: 800 }, + ]); + + render( + , + ); + + await waitFor(() => { + expect(mockFetchTopNValues).toHaveBeenCalledWith({ + datasource: '1__table', + column: 'country', + metric: 'revenue', + limit: 2, + sortAscending: false, + filters: [], + timeRange: undefined, + }); + }); +}); + +test('should show loading state while fetching TopN values', () => { + const mockFetchTopNValues = fetchTopNValues as jest.MockedFunction< + typeof fetchTopNValues + >; + const value: MatrixifyDimensionControlValue = { + dimension: 'country', + values: [], + }; + + mockFetchTopNValues.mockImplementation(() => new Promise(() => {})); // Never resolves + + render( + , + ); + + expect(screen.getByText('Loading top values...')).toBeInTheDocument(); +}); + +test('should display error when TopN fetch fails', async () => { + const mockFetchTopNValues = fetchTopNValues as jest.MockedFunction< + typeof fetchTopNValues + >; + const value: MatrixifyDimensionControlValue = { + dimension: 'country', + values: [], + }; + + mockFetchTopNValues.mockRejectedValue(new Error('Fetch failed')); + + render( + , + ); + + await waitFor(() => { + expect(screen.getByText('Error: Fetch failed')).toBeInTheDocument(); + }); +}); + +test('should convert string topNValue to number', async () => { + const mockFetchTopNValues = fetchTopNValues as jest.MockedFunction< + typeof fetchTopNValues + >; + const value: MatrixifyDimensionControlValue = { + dimension: 'country', + values: [], + }; + + mockFetchTopNValues.mockResolvedValue([]); + + render( + , + ); + + await waitFor(() => { + expect(fetchTopNValues).toHaveBeenCalledWith( + expect.objectContaining({ + limit: 10, // Should be converted to number + sortAscending: true, + }), + ); + }); +}); + +test('should not load values for datasource without filter_select', () => { + const datasourceNoFilter = { + ...mockDatasource, + filter_select: false, + }; + + const value: MatrixifyDimensionControlValue = { + dimension: 'country', + values: [], + }; + + render( + , + ); + + expect(SupersetClient.get).not.toHaveBeenCalled(); +}); + +test('should handle empty dimension value', () => { + const value: MatrixifyDimensionControlValue = { + dimension: '', + values: [], + }; + + render(); + + expect( + screen.queryByRole('combobox', { name: 'Select dimension values' }), + ).not.toBeInTheDocument(); +}); + +test('should handle undefined value prop', () => { + render(); + + expect( + screen.getByRole('combobox', { name: 'Select dimension' }), + ).toBeInTheDocument(); + expect( + screen.queryByRole('combobox', { name: 'Select dimension values' }), + ).not.toBeInTheDocument(); +}); + +test('should handle datasources without columns', () => { + const datasourceWithoutColumns = { + ...mockDatasource, + columns: [], + }; + + render( + , + ); + + expect( + screen.getByRole('combobox', { name: 'Select dimension' }), + ).toBeInTheDocument(); +}); + +test('should clear values when switching from topn to members mode', async () => { + const onChange = jest.fn(); + const value: MatrixifyDimensionControlValue = { + dimension: 'country', + values: ['USA', 'Canada'], + }; + + const { rerender } = render( + , + ); + + rerender( + , + ); + + await waitFor(() => { + expect(onChange).toHaveBeenCalledWith({ + dimension: 'country', + values: [], + topNValues: [], + }); + }); +}); diff --git a/superset-frontend/src/explore/components/controls/MatrixifyDimensionControl.tsx b/superset-frontend/src/explore/components/controls/MatrixifyDimensionControl.tsx new file mode 100644 index 00000000000..3623117d835 --- /dev/null +++ b/superset-frontend/src/explore/components/controls/MatrixifyDimensionControl.tsx @@ -0,0 +1,299 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { useEffect, useState } from 'react'; +import { t, SupersetClient, getColumnLabel } from '@superset-ui/core'; +import { Select, Space } from '@superset-ui/core/components'; +import ControlHeader from 'src/explore/components/ControlHeader'; +import { optionLabel } from 'src/utils/common'; +import { + fetchTopNValues, + extractDimensionValues, + TopNValue, +} from './MatrixifyControl/utils/fetchTopNValues'; + +export interface MatrixifyDimensionControlValue { + dimension: string; + values: any[]; + topNValues?: TopNValue[]; // Store topN values with their metric values +} + +interface MatrixifyDimensionControlProps { + datasource: any; + value?: MatrixifyDimensionControlValue; + onChange: (val: MatrixifyDimensionControlValue) => void; + label?: string; + description?: string; + hovered?: boolean; + selectionMode?: 'members' | 'topn'; + topNMetric?: string; + topNValue?: number; + topNOrder?: 'ASC' | 'DESC'; + formData?: any; // For access to filters and time range +} + +export default function MatrixifyDimensionControl( + props: MatrixifyDimensionControlProps, +) { + const { + datasource, + value, + onChange, + label, + description, + hovered, + selectionMode = 'members', + topNMetric, + topNValue, + topNOrder = 'DESC', + formData, + } = props; + + const [dimensionOptions, setDimensionOptions] = useState< + Array<[string, string]> + >([]); + const [valueOptions, setValueOptions] = useState< + Array<{ label: string; value: any }> + >([]); + const [loadingValues, setLoadingValues] = useState(false); + const [loadingTopN, setLoadingTopN] = useState(false); + const [topNError, setTopNError] = useState(null); + + // Initialize dimension options from datasource + useEffect(() => { + if (datasource?.columns) { + const options = datasource.columns.map((col: any) => [ + col.column_name, + getColumnLabel(col.column_name), + ]); + setDimensionOptions(options); + } + }, [datasource]); + + // Load dimension values when dimension changes + useEffect(() => { + if ( + !value?.dimension || + !datasource || + !datasource.id || + selectionMode !== 'members' + ) { + setValueOptions([]); + return undefined; + } + + // Check if datasource supports filter_select + if (datasource.filter_select === false) { + setValueOptions([]); + return undefined; + } + + const controller = new AbortController(); + const { signal } = controller; + + const loadDimensionValues = async () => { + const endpoint = `/api/v1/datasource/${ + datasource.type + }/${datasource.id}/column/${encodeURIComponent(value.dimension)}/values/`; + + setLoadingValues(true); + + try { + const { json } = await SupersetClient.get({ + signal, + endpoint, + }); + const values = json.result || []; + setValueOptions( + values.map((v: any) => ({ + label: optionLabel(v), + value: v, + })), + ); + } catch (error) { + setValueOptions([]); + } finally { + setLoadingValues(false); + } + }; + + loadDimensionValues(); + + return () => { + controller.abort(); + }; + }, [value?.dimension, datasource, selectionMode]); + + // Convert topNValue to number for consistent comparison + const topNValueNum = + typeof topNValue === 'string' ? parseInt(topNValue, 10) : topNValue; + + // Load TopN values when in TopN mode + useEffect(() => { + if ( + !value?.dimension || + !datasource || + selectionMode !== 'topn' || + !topNMetric || + !topNValueNum + ) { + // Clear the values when not in topn mode + if ( + selectionMode !== 'topn' && + value?.values && + value.values.length > 0 + ) { + onChange({ + dimension: value.dimension, + values: [], + topNValues: [], + }); + } + return undefined; + } + + const controller = new AbortController(); + const { signal } = controller; + + const loadTopNValues = async () => { + setLoadingTopN(true); + setTopNError(null); + + try { + const datasourceId = `${datasource.id}__${datasource.type}`; + const values = await fetchTopNValues({ + datasource: datasourceId, + column: value.dimension, + metric: topNMetric, + limit: topNValueNum, + sortAscending: topNOrder === 'ASC', + filters: formData?.adhoc_filters || [], + timeRange: formData?.time_range, + }); + + if (!signal.aborted) { + // Always update with the new topN values + const dimensionValues = extractDimensionValues(values); + onChange({ + dimension: value.dimension, + values: dimensionValues, + topNValues: values, + }); + } + } catch (error: any) { + if (!signal.aborted) { + setTopNError(error.message || t('Failed to load top values')); + // Clear values on error + onChange({ + dimension: value.dimension, + values: [], + topNValues: [], + }); + } + } finally { + if (!signal.aborted) { + setLoadingTopN(false); + } + } + }; + + loadTopNValues(); + + return () => { + controller.abort(); + }; + }, [ + value?.dimension, + datasource, + selectionMode, + topNMetric, + topNValueNum, // Use the converted number + topNOrder, + formData?.adhoc_filters, + formData?.time_range, + onChange, // Add onChange to deps + ]); + + const handleDimensionChange = (dimension: string) => { + // When dimension changes, clear the values + onChange({ + dimension: dimension || '', + values: [], + }); + }; + + const handleValuesChange = (values: any[]) => { + onChange({ + dimension: value?.dimension || '', + values, + }); + }; + + return ( + + + } + mode="multiple" + onChange={handleValuesChange} + options={valueOptions} + placeholder={t('Select values')} + loading={loadingValues} + allowClear + showSearch + notFoundContent={t('No results')} + /> + )} + + {value?.dimension && selectionMode === 'topn' && loadingTopN && ( +
{t('Loading top values...')}
+ )} + {value?.dimension && selectionMode === 'topn' && topNError && ( +
({ color: theme.colorError })}> + {t('Error: %s', topNError)} +
+ )} + + ); +} diff --git a/superset-frontend/src/explore/components/controls/index.ts b/superset-frontend/src/explore/components/controls/index.ts index 14e52d16a87..f8321fde2f6 100644 --- a/superset-frontend/src/explore/components/controls/index.ts +++ b/superset-frontend/src/explore/components/controls/index.ts @@ -57,6 +57,7 @@ import ZoomConfigControl from './ZoomConfigControl/ZoomConfigControl'; import NumberControl from './NumberControl'; import TimeRangeControl from './TimeRangeControl'; import ColorBreakpointsControl from './ColorBreakpointsControl'; +import MatrixifyDimensionControl from './MatrixifyDimensionControl'; const extensionsRegistry = getExtensionsRegistry(); const DateFilterControlExtension = extensionsRegistry.get( @@ -103,6 +104,7 @@ const controlMap = { ZoomConfigControl, NumberControl, TimeRangeControl, + MatrixifyDimensionControl, ...sharedControlComponents, }; export default controlMap; diff --git a/superset-frontend/src/explore/controlPanels/sections.tsx b/superset-frontend/src/explore/controlPanels/sections.tsx index 724e77ac8e8..737ad1ad07c 100644 --- a/superset-frontend/src/explore/controlPanels/sections.tsx +++ b/superset-frontend/src/explore/controlPanels/sections.tsx @@ -273,3 +273,59 @@ export const NVD3TimeSeries: ControlPanelSectionConfig[] = [ ], }, ]; + +function buildMatrixifySection( + axis: 'columns' | 'rows', +): ControlPanelSectionConfig { + const baseControls = [ + [`matrixify_mode_${axis}`], + [`matrixify_${axis}`], + [`matrixify_dimension_selection_mode_${axis}`], + [`matrixify_dimension_${axis}`], + [`matrixify_topn_dimension_${axis}`], + [`matrixify_topn_value_${axis}`], + [`matrixify_topn_metric_${axis}`], + [`matrixify_topn_order_${axis}`], + ]; + + // Add specific controls for each axis + if (axis === 'rows') { + // Add show row labels at the beginning + baseControls.unshift(['matrixify_show_row_labels']); + // Add row height control after show labels + baseControls.splice(1, 0, ['matrixify_row_height']); + } else if (axis === 'columns') { + // Add show column headers at the beginning + baseControls.unshift(['matrixify_show_column_headers']); + // Add fit columns control after show headers + baseControls.splice(1, 0, ['matrixify_fit_columns_dynamically']); + // Add charts per row after fit columns control + baseControls.splice(2, 0, ['matrixify_charts_per_row']); + } + + return { + label: axis === 'columns' ? t('Horizontal layout') : t('Vertical layout'), + expanded: true, + tabOverride: 'matrixify', + visibility: ({ controls }) => controls?.matrixify_enabled?.value === true, + controlSetRows: baseControls, + }; +} + +export const matrixifyRows = buildMatrixifySection('rows'); +export const matrixifyColumns = buildMatrixifySection('columns'); + +export const matrixifyEnableSection: ControlPanelSectionConfig = { + label: t('Enable Matrixify'), + expanded: true, + tabOverride: 'matrixify', + controlSetRows: [['matrixify_enabled']], +}; + +export const matrixifyCells: ControlPanelSectionConfig = { + label: t('Cells'), + expanded: true, + tabOverride: 'matrixify', + visibility: ({ controls }) => controls?.matrixify_enabled?.value === true, + controlSetRows: [['matrixify_cell_title_template']], +}; diff --git a/superset-frontend/src/explore/controlUtils/getControlState.ts b/superset-frontend/src/explore/controlUtils/getControlState.ts index 1fb72e959a1..be80aae1a80 100644 --- a/superset-frontend/src/explore/controlUtils/getControlState.ts +++ b/superset-frontend/src/explore/controlUtils/getControlState.ts @@ -171,7 +171,8 @@ export function getAllControlsState( formData: QueryFormData, ) { const controlsState: Record | null> = {}; - getSectionsToRender(vizType, datasourceType).forEach(section => + getSectionsToRender(vizType, datasourceType).forEach(section => { + if (!section || !section.controlSetRows) return; section.controlSetRows.forEach(fieldsetRow => fieldsetRow.forEach((field: CustomControlItem) => { if (field?.config && field.name) { @@ -183,7 +184,7 @@ export function getAllControlsState( ); } }), - ), - ); + ); + }); return controlsState; } diff --git a/superset-frontend/src/explore/controlUtils/getSectionsToRender.ts b/superset-frontend/src/explore/controlUtils/getSectionsToRender.ts index 1729c896ed2..6fee00ef0ff 100644 --- a/superset-frontend/src/explore/controlUtils/getSectionsToRender.ts +++ b/superset-frontend/src/explore/controlUtils/getSectionsToRender.ts @@ -58,7 +58,13 @@ const getMemoizedSectionsToRender = memoizeOne( } }); - const { datasourceAndVizType } = sections; + const { + datasourceAndVizType, + matrixifyRows = null, + matrixifyColumns = null, + matrixifyCells = null, + matrixifyEnableSection = null, + } = sections; // list of datasource-specific controls that should be removed if the datasource is a specific type const filterControlsForTypes = [DatasourceType.Query, DatasourceType.Table]; @@ -66,9 +72,17 @@ const getMemoizedSectionsToRender = memoizeOne( ? ['granularity'] : ['granularity_sqla', 'time_grain_sqla']; - return [datasourceAndVizType as ControlPanelSectionConfig] + return [ + datasourceAndVizType as ControlPanelSectionConfig, + matrixifyEnableSection as ControlPanelSectionConfig, + matrixifyCells as ControlPanelSectionConfig, + matrixifyColumns as ControlPanelSectionConfig, + matrixifyRows as ControlPanelSectionConfig, + ] + .filter(Boolean) // Filter out null/undefined sections .concat(controlPanelSections.filter(isControlPanelSectionConfig)) .map(section => { + if (!section) return null; const { controlSetRows } = section; return { ...section, @@ -83,7 +97,8 @@ const getMemoizedSectionsToRender = memoizeOne( .map(item => expandControlConfig(item, controlOverrides)), ) || [], }; - }); + }) + .filter(Boolean); // Filter out any null results }, ); diff --git a/superset-frontend/src/features/databases/UploadDataModel/UploadDataModal.test.tsx b/superset-frontend/src/features/databases/UploadDataModel/UploadDataModal.test.tsx index b7292087b95..a9c3c2bb671 100644 --- a/superset-frontend/src/features/databases/UploadDataModel/UploadDataModal.test.tsx +++ b/superset-frontend/src/features/databases/UploadDataModel/UploadDataModal.test.tsx @@ -86,8 +86,8 @@ const setupMocks = () => { }); }; -// Set timeout for all tests in this file to 30 seconds -jest.setTimeout(30000); +// Set timeout for all tests in this file to 60 seconds +jest.setTimeout(60000); beforeEach(() => { setupMocks(); diff --git a/superset-frontend/src/features/reports/ReportModal/HeaderReportDropdown/index.tsx b/superset-frontend/src/features/reports/ReportModal/HeaderReportDropdown/index.tsx index b370333d838..5fe02bb347e 100644 --- a/superset-frontend/src/features/reports/ReportModal/HeaderReportDropdown/index.tsx +++ b/superset-frontend/src/features/reports/ReportModal/HeaderReportDropdown/index.tsx @@ -86,7 +86,6 @@ export const useHeaderReportMenuItems = ({ const reportsState = state.reports || {}; const resourceTypeReports = reportsState[resourceType] || {}; const reportData = resourceTypeReports[resourceId]; - return reportData || null; }); diff --git a/superset/config.py b/superset/config.py index 4e3034e8648..579ad02139d 100644 --- a/superset/config.py +++ b/superset/config.py @@ -619,6 +619,8 @@ DEFAULT_FEATURE_FLAGS: dict[str, bool] = { # Enable support for date range timeshifts (e.g., "2015-01-03 : 2015-01-04") # in addition to relative timeshifts (e.g., "1 day ago") "DATE_RANGE_TIMESHIFTS_ENABLED": False, + # Enable Matrixify feature for matrix-style chart layouts + "MATRIXIFY": False, } # ------------------------------