fix(dataset): add missing currency_code_column to DatasetPostSchema (#38853)

superset-frontend/packages/superset-ui-chart-controls/src/utils/getColorFormatters.ts

@@ -350,7 +350,7 @@ export const getReadableTextColor = (
 
   const { r: bgR, g: bgG, b: bgB, a: bgAlpha } = background.toRgb();
   const { r: surfaceR, g: surfaceG, b: surfaceB } = surface.toRgb();
-  const alpha = bgAlpha ?? 1;
+  const alpha = bgAlpha;
 
   const compositeColor = tinycolor({
     r: bgR * alpha + surfaceR * (1 - alpha),

superset-frontend/packages/superset-ui-chart-controls/test/utils/getColorFormatters.test.ts

@@ -129,6 +129,11 @@ test('getReadableTextColor blends alpha over the provided surface', () => {
   );
 });
 
+test('getReadableTextColor returns undefined for invalid colors', () => {
+  expect(getReadableTextColor('not-a-color', '#ffffff')).toBeUndefined();
+  expect(getReadableTextColor('#111111', 'not-a-color')).toBeUndefined();
+});
+
 test('getTextColorForBackground prefers explicit text color', () => {
   expect(
     getTextColorForBackground(
@@ -145,6 +150,10 @@ test('getNormalizedTextColor removes alpha from explicit text colors', () => {
   );
 });
 
+test('getNormalizedTextColor preserves invalid explicit text colors', () => {
+  expect(getNormalizedTextColor('not-a-color')).toBe('not-a-color');
+});
+
 test('getTextColorForBackground normalizes explicit text color alpha', () => {
   expect(
     getTextColorForBackground(

superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridGenerator.test.ts

@@ -196,6 +196,25 @@ test('should handle string metrics', () => {
   expect(grid!.colHeaders).toEqual(['avg', 'max']);
 });
 
+test('should skip missing column metrics when generating cell form data', () => {
+  const missingColumnMetricFormData: TestFormData = {
+    viz_type: 'table',
+    datasource: '1__table',
+    matrixify_enable: true,
+    matrixify_mode_rows: 'metrics',
+    matrixify_mode_columns: 'metrics',
+    matrixify_rows: [createAdhocMetric('Revenue')],
+    matrixify_columns: [null],
+  };
+
+  const grid = generateMatrixifyGrid(missingColumnMetricFormData);
+
+  expect(grid).not.toBeNull();
+  expect(grid!.cells[0][0]!.formData.metrics).toEqual([
+    createAdhocMetric('Revenue'),
+  ]);
+});
+
 test('should not escape HTML entities in cell titles', () => {
   const formDataWithSpecialChars: TestFormData = {
     viz_type: 'table',

superset-frontend/packages/superset-ui-core/src/chart/types/matrixify.test.ts

@@ -149,6 +149,15 @@ test('getMatrixifyConfig should return null when matrixify_enable is false', ()
   expect(getMatrixifyConfig(formData)).toBeNull();
 });
 
+test('getMatrixifyConfig should return null when no axes are enabled', () => {
+  const formData = {
+    viz_type: 'table',
+    matrixify_enable: true,
+  } as MatrixifyFormData;
+
+  expect(getMatrixifyConfig(formData)).toBeNull();
+});
+
 test('getMatrixifyConfig should return valid config for metrics mode', () => {
   const formData = {
     viz_type: 'table',
@@ -212,6 +221,24 @@ test('getMatrixifyConfig should handle topn selection mode', () => {
   expect(config!.rows.dimension).toEqual(formData.matrixify_dimension_rows);
 });
 
+test('getMatrixifyConfig should preserve ascending topn order when explicitly disabled', () => {
+  const formData = {
+    viz_type: 'table',
+    matrixify_enable: true,
+    matrixify_mode_rows: 'dimensions',
+    matrixify_mode_columns: 'dimensions',
+    matrixify_dimension_rows: { dimension: 'country', values: ['USA'] },
+    matrixify_dimension_columns: { dimension: 'product', values: ['Widget'] },
+    matrixify_topn_order_rows: false,
+    matrixify_topn_order_columns: false,
+  } as MatrixifyFormData;
+
+  const config = getMatrixifyConfig(formData);
+  expect(config).not.toBeNull();
+  expect(config!.rows.topnOrder).toBe('asc');
+  expect(config!.columns.topnOrder).toBe('asc');
+});
+
 test('getMatrixifyValidationErrors should return empty array when matrixify is not enabled', () => {
   const formData = {
     viz_type: 'table',
@@ -256,6 +283,15 @@ test('getMatrixifyValidationErrors should return empty array when properly confi
   expect(getMatrixifyValidationErrors(formData)).toEqual([]);
 });
 
+test('getMatrixifyValidationErrors should return empty array when enabled with no active axes', () => {
+  const formData = {
+    viz_type: 'table',
+    matrixify_enable: true,
+  } as MatrixifyFormData;
+
+  expect(getMatrixifyValidationErrors(formData)).toEqual([]);
+});
+
 test('getMatrixifyValidationErrors should return error when enabled but no configuration exists', () => {
   const formData = {
     viz_type: 'table',

superset-frontend/packages/superset-ui-core/src/chart/types/matrixify.ts

@@ -200,10 +200,7 @@ export function isMatrixifyEnabled(formData: MatrixifyFormData): boolean {
     return false;
   }
 
-  const config = getMatrixifyConfig(formData);
-  if (!config) {
-    return false;
-  }
+  const config = getMatrixifyConfig(formData)!;
 
   const hasRowData =
     rowEnabled &&
@@ -247,11 +244,7 @@ export function getMatrixifyValidationErrors(
     return errors;
   }
 
-  const config = getMatrixifyConfig(formData);
-  if (!config) {
-    errors.push('Please configure at least one row or column axis');
-    return errors;
-  }
+  const config = getMatrixifyConfig(formData)!;
 
   // Check row configuration (only if explicitly set in form data)
   const hasRowMode = Boolean(formData.matrixify_mode_rows);

superset/datasets/schemas.py

@@ -167,6 +167,7 @@ class DatasetPostSchema(Schema):
     external_url = fields.String(allow_none=True)
     normalize_columns = fields.Boolean(load_default=False)
     always_filter_main_dttm = fields.Boolean(load_default=False)
+    currency_code_column = fields.String(allow_none=True, validate=Length(0, 250))
     template_params = fields.String(allow_none=True)
     uuid = fields.UUID(allow_none=True)
 

tests/integration_tests/datasets/api_tests.py

@@ -861,6 +861,29 @@ class TestDatasetApi(SupersetTestCase):
         assert alpha in model.owners
         self.items_to_delete = [model]
 
+    @pytest.mark.usefixtures("load_energy_table_with_slice")
+    def test_create_dataset_with_currency_code_column(self):
+        """
+        Dataset API: Test create dataset persists currency_code_column
+        """
+
+        energy_usage_ds = self.get_energy_usage_dataset()
+        self.login(ALPHA_USERNAME)
+        table_data = {
+            "database": energy_usage_ds.database_id,
+            "table_name": "energy_usage_virtual_currency_column",
+            "sql": "select * from energy_usage",
+            "currency_code_column": "currency",
+        }
+        if schema := get_example_default_schema():
+            table_data["schema"] = schema
+        rv = self.post_assert_metric("/api/v1/dataset/", table_data, "post")
+        assert rv.status_code == 201
+        data = json.loads(rv.data.decode("utf-8"))
+        model = db.session.query(SqlaTable).get(data.get("id"))
+        assert model.currency_code_column == "currency"
+        self.items_to_delete = [model]
+
     @unittest.skip("test is failing stochastically")
     def test_create_dataset_same_name_different_schema(self):
         if backend() == "sqlite":

tests/unit_tests/datasets/schema_tests.py

@@ -48,35 +48,60 @@ def test_validate_python_date_format_raises(payload) -> None:
         validate_python_date_format(payload)
 
 
-def test_dataset_put_schema_includes_currency_code_column() -> None:
-    """Test that DatasetPutSchema properly handles currency_code_column field."""
-    from superset.datasets.schemas import DatasetPutSchema
+def test_dataset_post_schema_has_all_put_scalar_fields() -> None:
+    """
+    Every scalar model field accepted by DatasetPutSchema should also be accepted
+    by DatasetPostSchema, unless it is intentionally excluded (fields that only
+    make sense after a dataset already exists).
 
-    schema = DatasetPutSchema()
+    This prevents the class of bug where a new column is added to the update
+    schema but forgotten in the create schema.
+    """
+    from superset.datasets.schemas import DatasetPostSchema, DatasetPutSchema
 
-    # Dataset with currency code column
+    # Fields that are intentionally only on Put: they require an existing dataset
+    # or are populated server-side during creation.
+    put_only_fields = {
+        "columns",
+        "metrics",
+        "folders",
+        "database_id",  # Post uses "database" (integer id) instead
+        "description",
+        "main_dttm_col",
+        "filter_select_enabled",
+        "fetch_values_predicate",
+        "offset",
+        "default_endpoint",
+        "cache_timeout",
+        "is_sqllab_view",
+        "extra",
+    }
+
+    put_fields = set(DatasetPutSchema().fields.keys())
+    post_fields = set(DatasetPostSchema().fields.keys())
+
+    missing = put_fields - post_fields - put_only_fields
+    assert missing == set(), (
+        f"Fields {missing} are in DatasetPutSchema but missing from "
+        f"DatasetPostSchema. Either add them to DatasetPostSchema or to "
+        f"the put_only_fields exclusion list in this test."
+    )
+
+
+def test_dataset_post_schema_includes_currency_code_column() -> None:
+    """Test that DatasetPostSchema accepts currency_code_column."""
+    from superset.datasets.schemas import DatasetPostSchema
+
+    schema = DatasetPostSchema()
     data = {
+        "database": 1,
+        "table_name": "virtual_dataset",
         "currency_code_column": "currency",
     }
     result = schema.load(data)
     assert result["currency_code_column"] == "currency"
 
 
-def test_dataset_put_schema_currency_code_column_optional() -> None:
-    """Test that currency_code_column is optional in DatasetPutSchema."""
-    from superset.datasets.schemas import DatasetPutSchema
-
-    schema = DatasetPutSchema()
-
-    # Dataset without currency code column (should not fail)
-    data: dict[str, str | None] = {}
-    result = schema.load(data)
-    assert (
-        "currency_code_column" not in result
-        or result.get("currency_code_column") is None
-    )
-
-
 def test_dataset_metrics_put_schema_parses_currency_string() -> None:
     """Test that DatasetMetricsPutSchema parses string currency payloads."""
     from superset.datasets.schemas import DatasetMetricsPutSchema