fix(Matrixify): readd matrixify_enable guard missing (#38759)

This commit is contained in:
Mehmet Salih Yavuz
2026-03-23 23:29:09 +03:00
committed by GitHub
parent e0b524fff2
commit 7222327992
11 changed files with 231 additions and 14 deletions

View File

@@ -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 { isMatrixifyVisible } from './matrixifyControls';
/**
* Helper to build a controls object matching the shape used by
* control panel visibility callbacks.
*/
function makeControls(
overrides: Record<string, unknown> = {},
): Record<string, { value: unknown }> {
const defaults: Record<string, unknown> = {
matrixify_enable: false,
matrixify_mode_rows: 'disabled',
matrixify_mode_columns: 'disabled',
matrixify_dimension_selection_mode_rows: 'members',
matrixify_dimension_selection_mode_columns: 'members',
};
const merged = { ...defaults, ...overrides };
return Object.fromEntries(
Object.entries(merged).map(([k, v]) => [k, { value: v }]),
);
}
// ── matrixify_enable guard ──────────────────────────────────────────
test('returns false when matrixify_enable is false, even with active axis modes', () => {
const controls = makeControls({
matrixify_enable: false,
matrixify_mode_rows: 'metrics',
matrixify_mode_columns: 'dimensions',
});
expect(isMatrixifyVisible(controls, 'rows')).toBe(false);
expect(isMatrixifyVisible(controls, 'columns')).toBe(false);
});
test('returns false when matrixify_enable is undefined (old form_data without the field)', () => {
const controls = makeControls({
matrixify_mode_rows: 'metrics',
});
delete (controls as any).matrixify_enable;
expect(isMatrixifyVisible(controls, 'rows')).toBe(false);
});
test('returns false when controls object is undefined', () => {
expect(isMatrixifyVisible(undefined, 'rows')).toBe(false);
});
// ── axis mode checks ────────────────────────────────────────────────
test('returns false when axis mode is disabled', () => {
const controls = makeControls({
matrixify_enable: true,
matrixify_mode_rows: 'disabled',
});
expect(isMatrixifyVisible(controls, 'rows')).toBe(false);
});
test('returns true when matrixify_enable is true and axis mode is metrics', () => {
const controls = makeControls({
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
});
expect(isMatrixifyVisible(controls, 'rows')).toBe(true);
});
test('returns true when matrixify_enable is true and axis mode is dimensions', () => {
const controls = makeControls({
matrixify_enable: true,
matrixify_mode_columns: 'dimensions',
});
expect(isMatrixifyVisible(controls, 'columns')).toBe(true);
});
// ── mode filter ─────────────────────────────────────────────────────
test('returns false when mode filter does not match axis value', () => {
const controls = makeControls({
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
});
expect(isMatrixifyVisible(controls, 'rows', 'dimensions')).toBe(false);
});
test('returns true when mode filter matches axis value', () => {
const controls = makeControls({
matrixify_enable: true,
matrixify_mode_rows: 'dimensions',
});
expect(isMatrixifyVisible(controls, 'rows', 'dimensions')).toBe(true);
});
// ── selectionMode filter ────────────────────────────────────────────
test('returns true when selectionMode matches', () => {
const controls = makeControls({
matrixify_enable: true,
matrixify_mode_rows: 'dimensions',
matrixify_dimension_selection_mode_rows: 'topn',
});
expect(isMatrixifyVisible(controls, 'rows', 'dimensions', 'topn')).toBe(true);
});
test('returns false when selectionMode does not match', () => {
const controls = makeControls({
matrixify_enable: true,
matrixify_mode_rows: 'dimensions',
matrixify_dimension_selection_mode_rows: 'members',
});
expect(isMatrixifyVisible(controls, 'rows', 'dimensions', 'topn')).toBe(
false,
);
});
test('ignores selectionMode filter when mode is metrics', () => {
const controls = makeControls({
matrixify_enable: true,
matrixify_mode_columns: 'metrics',
});
// selectionMode only applies to dimensions mode, should be ignored
expect(isMatrixifyVisible(controls, 'columns', 'metrics', 'topn')).toBe(true);
});

View File

@@ -36,6 +36,8 @@ const isMatrixifyVisible = (
mode?: 'metrics' | 'dimensions',
selectionMode?: 'members' | 'topn' | 'all',
) => {
if (controls?.matrixify_enable?.value !== true) return false;
const modeControl = `matrixify_mode_${axis}`;
const selectionModeControl = `matrixify_dimension_selection_mode_${axis}`;
@@ -372,4 +374,4 @@ matrixifyControls.matrixify_show_column_headers = {
visibility: ({ controls }) => isMatrixifyVisible(controls, 'columns'),
};
export { matrixifyControls };
export { matrixifyControls, isMatrixifyVisible };

View File

@@ -39,6 +39,7 @@ const createSqlMetric = (label: string, sql: string): AdhocMetric => ({
const baseFormData: TestFormData = {
viz_type: 'table',
datasource: '1__table',
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
matrixify_mode_columns: 'metrics',
matrixify_rows: [createAdhocMetric('Revenue'), createAdhocMetric('Profit')],
@@ -77,6 +78,7 @@ test('should generate grid for dimensions mode', () => {
const dimensionFormData: TestFormData = {
viz_type: 'table',
datasource: '1__table',
matrixify_enable: true,
matrixify_mode_rows: 'dimensions',
matrixify_mode_columns: 'dimensions',
matrixify_dimension_rows: {
@@ -117,6 +119,7 @@ test('should generate grid for mixed mode (metrics rows, dimensions columns)', (
const mixedFormData: TestFormData = {
viz_type: 'table',
datasource: '1__table',
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
matrixify_mode_columns: 'dimensions',
matrixify_rows: [createAdhocMetric('Total Sales')],
@@ -139,6 +142,7 @@ test('should handle empty configuration', () => {
const emptyFormData: TestFormData = {
viz_type: 'table',
datasource: '1__table',
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
matrixify_mode_columns: 'metrics',
matrixify_rows: [],
@@ -157,6 +161,7 @@ test('should handle single row and column', () => {
const singleCellFormData: TestFormData = {
viz_type: 'table',
datasource: '1__table',
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
matrixify_mode_columns: 'metrics',
matrixify_rows: [createAdhocMetric('Count')],
@@ -177,6 +182,7 @@ test('should handle string metrics', () => {
const stringMetricFormData: TestFormData = {
viz_type: 'table',
datasource: '1__table',
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
matrixify_mode_columns: 'metrics',
matrixify_rows: ['count', 'sum'],
@@ -194,6 +200,7 @@ test('should not escape HTML entities in cell titles', () => {
const formDataWithSpecialChars: TestFormData = {
viz_type: 'table',
datasource: '1__table',
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
matrixify_mode_columns: 'metrics',
matrixify_rows: [createAdhocMetric('Sales & Revenue')],
@@ -309,6 +316,7 @@ test('should generate single-column grid when only rows are configured', () => {
const rowsOnlyFormData: TestFormData = {
viz_type: 'table',
datasource: '1__table',
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
matrixify_rows: [createAdhocMetric('Revenue'), createAdhocMetric('Profit')],
// No column config
@@ -326,6 +334,7 @@ test('should generate single-row grid when only columns are configured', () => {
const colsOnlyFormData: TestFormData = {
viz_type: 'table',
datasource: '1__table',
matrixify_enable: true,
matrixify_mode_columns: 'metrics',
matrixify_columns: [
createSqlMetric('Q1', 'SUM(q1)'),
@@ -359,6 +368,7 @@ test('should return empty string header for null metric in array (line 76)', ()
const formData: TestFormData = {
viz_type: 'table',
datasource: '1__table',
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
matrixify_mode_columns: 'metrics',
matrixify_rows: [null],
@@ -373,6 +383,7 @@ test('should return empty string header for empty-string dimension value (line 8
const formData: TestFormData = {
viz_type: 'table',
datasource: '1__table',
matrixify_enable: true,
matrixify_mode_rows: 'dimensions',
matrixify_mode_columns: 'dimensions',
matrixify_dimension_rows: { dimension: 'country', values: [''] },
@@ -387,6 +398,7 @@ test('should skip dimension filter when value is undefined (lines 151, 165)', ()
const formData: TestFormData = {
viz_type: 'table',
datasource: '1__table',
matrixify_enable: true,
matrixify_mode_rows: 'dimensions',
matrixify_mode_columns: 'dimensions',
matrixify_dimension_rows: {
@@ -418,6 +430,7 @@ test('should handle metrics without labels', () => {
const metricsWithoutLabels: TestFormData = {
viz_type: 'table',
datasource: '1__table',
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
matrixify_mode_columns: 'metrics',
matrixify_rows: [

View File

@@ -137,9 +137,22 @@ test('getMatrixifyConfig should return null when no matrixify configuration exis
expect(getMatrixifyConfig(formData)).toBeNull();
});
test('getMatrixifyConfig should return null when matrixify_enable is false', () => {
const formData = {
viz_type: 'table',
matrixify_enable: false,
matrixify_mode_rows: 'metrics',
matrixify_mode_columns: 'metrics',
matrixify_rows: [createMetric('Revenue')],
matrixify_columns: [createMetric('Q1')],
} as MatrixifyFormData;
expect(getMatrixifyConfig(formData)).toBeNull();
});
test('getMatrixifyConfig should return valid config for metrics mode', () => {
const formData = {
viz_type: 'table',
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
matrixify_mode_columns: 'metrics',
matrixify_rows: [createMetric('Revenue')],
@@ -157,6 +170,7 @@ test('getMatrixifyConfig should return valid config for metrics mode', () => {
test('getMatrixifyConfig should return valid config for dimensions mode', () => {
const formData = {
viz_type: 'table',
matrixify_enable: true,
matrixify_mode_rows: 'dimensions',
matrixify_mode_columns: 'dimensions',
matrixify_dimension_rows: { dimension: 'country', values: ['USA'] },
@@ -180,6 +194,7 @@ test('getMatrixifyConfig should return valid config for dimensions mode', () =>
test('getMatrixifyConfig should handle topn selection mode', () => {
const formData = {
viz_type: 'table',
matrixify_enable: true,
matrixify_mode_rows: 'dimensions',
matrixify_mode_columns: 'dimensions',
matrixify_dimension_rows: {
@@ -207,9 +222,31 @@ test('getMatrixifyValidationErrors should return empty array when matrixify is n
expect(getMatrixifyValidationErrors(formData)).toEqual([]);
});
test('getMatrixifyValidationErrors should return empty array when matrixify_enable is false even with stale mode values', () => {
const formData = {
viz_type: 'table',
matrixify_enable: false,
matrixify_mode_rows: 'metrics',
matrixify_mode_columns: 'dimensions',
} as MatrixifyFormData;
expect(getMatrixifyValidationErrors(formData)).toEqual([]);
});
test('getMatrixifyValidationErrors should return empty array when matrixify_enable is undefined with stale defaults', () => {
const formData = {
viz_type: 'bar',
matrixify_mode_rows: 'metrics',
matrixify_rows: [],
} as MatrixifyFormData;
expect(getMatrixifyValidationErrors(formData)).toEqual([]);
});
test('getMatrixifyValidationErrors should return empty array when properly configured', () => {
const formData = {
viz_type: 'table',
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
matrixify_mode_columns: 'metrics',
matrixify_rows: [createMetric('Revenue')],
@@ -222,6 +259,7 @@ test('getMatrixifyValidationErrors should return empty array when properly confi
test('getMatrixifyValidationErrors should return error when enabled but no configuration exists', () => {
const formData = {
viz_type: 'table',
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
} as MatrixifyFormData;
@@ -232,6 +270,7 @@ test('getMatrixifyValidationErrors should return error when enabled but no confi
test('getMatrixifyValidationErrors should return error when metrics mode has no metrics', () => {
const formData = {
viz_type: 'table',
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
matrixify_rows: [],
matrixify_columns: [],
@@ -276,6 +315,7 @@ test('isMatrixifyEnabled should return false when switch is off even with valid
test('getMatrixifyValidationErrors should return dimension error for rows when dimension has no data', () => {
const formData = {
viz_type: 'table',
matrixify_enable: true,
matrixify_mode_rows: 'dimensions',
// No matrixify_dimension_rows set
matrixify_mode_columns: 'metrics',
@@ -289,6 +329,7 @@ test('getMatrixifyValidationErrors should return dimension error for rows when d
test('getMatrixifyValidationErrors should return metric error for columns when metrics array is empty', () => {
const formData = {
viz_type: 'table',
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
matrixify_rows: [createMetric('Revenue')],
matrixify_mode_columns: 'metrics',
@@ -302,6 +343,7 @@ test('getMatrixifyValidationErrors should return metric error for columns when m
test('getMatrixifyValidationErrors should return dimension error for columns when no dimension data', () => {
const formData = {
viz_type: 'table',
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
matrixify_rows: [createMetric('Revenue')],
matrixify_mode_columns: 'dimensions',
@@ -315,6 +357,7 @@ test('getMatrixifyValidationErrors should return dimension error for columns whe
test('getMatrixifyValidationErrors skips row check when matrixify_mode_rows is not set', () => {
const formData = {
viz_type: 'table',
matrixify_enable: true,
// No matrixify_mode_rows — hasRowMode = false
matrixify_mode_columns: 'metrics',
matrixify_columns: [createMetric('Q1')],
@@ -327,6 +370,7 @@ test('getMatrixifyValidationErrors skips row check when matrixify_mode_rows is n
test('getMatrixifyValidationErrors evaluates full && expression when dimension is set but values are empty', () => {
const formData = {
viz_type: 'table',
matrixify_enable: true,
matrixify_mode_rows: 'dimensions',
matrixify_dimension_rows: { dimension: 'country', values: [] },
matrixify_mode_columns: 'dimensions',

View File

@@ -154,6 +154,10 @@ function isAxisEnabled(mode?: MatrixifyMode): boolean {
export function getMatrixifyConfig(
formData: MatrixifyFormData & any,
): MatrixifyConfig | null {
if (formData.matrixify_enable !== true) {
return null;
}
const rowEnabled = isAxisEnabled(formData.matrixify_mode_rows);
const colEnabled = isAxisEnabled(formData.matrixify_mode_columns);
@@ -230,6 +234,10 @@ export function isMatrixifyEnabled(formData: MatrixifyFormData): boolean {
export function getMatrixifyValidationErrors(
formData: MatrixifyFormData,
): string[] {
if (formData.matrixify_enable !== true) {
return [];
}
const errors: string[] = [];
const rowEnabled = isAxisEnabled(formData.matrixify_mode_rows);

View File

@@ -187,10 +187,11 @@ const ChartContextMenu = (
canDrillBy &&
isDisplayed(ContextMenuItem.DrillBy) &&
!(
(formData.matrixify_mode_rows !== undefined &&
formData.matrixify_enable === true &&
((formData.matrixify_mode_rows !== undefined &&
formData.matrixify_mode_rows !== 'disabled') ||
(formData.matrixify_mode_columns !== undefined &&
formData.matrixify_mode_columns !== 'disabled')
(formData.matrixify_mode_columns !== undefined &&
formData.matrixify_mode_columns !== 'disabled'))
); // Disable drill by when matrixify is enabled
const datasetResource = useDatasetDrillInfo(

View File

@@ -212,6 +212,7 @@ test('should identify matrixify property changes correctly', () => {
formData: {
datasource: '',
viz_type: VizType.Table,
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
matrixify_dimension_x: { dimension: 'country', values: ['USA'] },
matrixify_charts_per_row: 3,
@@ -234,6 +235,7 @@ test('should identify matrixify property changes correctly', () => {
formData: {
datasource: '',
viz_type: VizType.Table,
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
matrixify_dimension_x: {
dimension: 'country',
@@ -277,6 +279,7 @@ test('should handle matrixify-related form data changes', () => {
formData: {
datasource: '',
viz_type: VizType.Table,
matrixify_enable: true,
matrixify_mode_rows: 'metrics', // This is a significant change
regular_control: 'value1',
},
@@ -296,6 +299,7 @@ test('should detect matrixify property addition', () => {
formData: {
datasource: '',
viz_type: VizType.Table,
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
// No matrixify_dimension_x initially
},
@@ -317,6 +321,7 @@ test('should detect matrixify property addition', () => {
formData: {
datasource: '',
viz_type: VizType.Table,
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
matrixify_dimension_x: { dimension: 'country', values: ['USA'] }, // Added
},
@@ -336,6 +341,7 @@ test('should detect nested matrixify property changes', () => {
formData: {
datasource: '',
viz_type: VizType.Table,
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
matrixify_dimension_x: {
dimension: 'country',
@@ -361,6 +367,7 @@ test('should detect nested matrixify property changes', () => {
formData: {
datasource: '',
viz_type: VizType.Table,
matrixify_enable: true,
matrixify_mode_rows: 'metrics',
matrixify_dimension_x: {
dimension: 'country',

View File

@@ -275,10 +275,11 @@ class ChartRenderer extends Component<ChartRendererProps, ChartRendererState> {
const nextFormData = nextProps.formData as JsonObject;
const currentFormData = this.props.formData as JsonObject;
const isMatrixifyEnabled =
(nextFormData.matrixify_mode_rows !== undefined &&
nextFormData.matrixify_enable === true &&
((nextFormData.matrixify_mode_rows !== undefined &&
nextFormData.matrixify_mode_rows !== 'disabled') ||
(nextFormData.matrixify_mode_columns !== undefined &&
nextFormData.matrixify_mode_columns !== 'disabled');
(nextFormData.matrixify_mode_columns !== undefined &&
nextFormData.matrixify_mode_columns !== 'disabled'));
if (!isMatrixifyEnabled) return false;
// Check all matrixify-related properties

View File

@@ -274,7 +274,7 @@ test('When menu item is clicked, call onSelection with clicked column and drill
test('matrixify_mode_rows enabled should not render component', () => {
const { container } = renderSubmenu({
formData: { ...defaultFormData, matrixify_mode_rows: 'metrics' },
formData: { ...defaultFormData, matrixify_enable: true, matrixify_mode_rows: 'metrics' },
});
expect(container).toBeEmptyDOMElement();
});

View File

@@ -179,10 +179,11 @@ export const DrillBySubmenu = ({
}
if (
(formData.matrixify_mode_rows !== undefined &&
formData.matrixify_enable === true &&
((formData.matrixify_mode_rows !== undefined &&
formData.matrixify_mode_rows !== 'disabled') ||
(formData.matrixify_mode_columns !== undefined &&
formData.matrixify_mode_columns !== 'disabled')
(formData.matrixify_mode_columns !== undefined &&
formData.matrixify_mode_columns !== 'disabled'))
) {
return null;
}

View File

@@ -455,10 +455,11 @@ const ExploreChartPanel = ({
})}
{...(chart.chartStatus && { chartStatus: chart.chartStatus })}
hideRowCount={
(formData?.matrixify_mode_rows !== undefined &&
formData?.matrixify_enable === true &&
((formData?.matrixify_mode_rows !== undefined &&
formData?.matrixify_mode_rows !== 'disabled') ||
(formData?.matrixify_mode_columns !== undefined &&
formData?.matrixify_mode_columns !== 'disabled')
(formData?.matrixify_mode_columns !== undefined &&
formData?.matrixify_mode_columns !== 'disabled'))
}
formData={formData}
/>