test(dashboard): add edge case tests for legacy chart customization migration

[devin-ai-integration]

SUMMARY

Adds 25 new edge case tests to migrateChartCustomization.test.ts, increasing coverage of the legacy chart customization migration logic introduced in apache#37176. The existing 27 tests covered the happy paths; these new tests exercise boundary conditions across all exported functions.

New test coverage areas:

• isLegacyChartCustomizationFormat: hybrid objects (both customization and type present), non-object primitives (number, array), objects with type but no customization
• extractDatasetId (via migrateChartCustomization): falsy numbers (0), negative numbers, empty strings, dataset objects with non-numeric string values
• extractColumnName: empty string column
• Name resolution: fallback to '' when both customization.name and title are absent
• defaultDataMask groupby enrichment: scalar (non-array) filterState.value wrapping, merging into existing custom_form_data, preserving pre-set filterState.label, label generation from multiple values, skipping enrichment for falsy values (null, '', absent)
• controlValues merging: controlValues overriding base control properties, graceful handling when controlValues is absent
• removed flag: explicit false vs undefined
• migrateChartCustomizationArray: single-item arrays, order preservation with mixed legacy/new format items
• Falsy chartId: chartId: 0 treated as missing (see note below)

⚠️ Reviewer note — potential bug documented by tests:

The test 'migrateChartCustomization handles chartId as zero' documents that chartId: 0 results in chartsInScope: undefined. This is because the production code uses a truthiness check (legacy.chartId ? [legacy.chartId] : undefined), which treats 0 as falsy. If 0 is a valid chart ID in practice, this is a latent bug in migrateChartCustomization.ts:134. This PR only adds the test to document the behavior — no production code is changed.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A — test-only change.

TESTING INSTRUCTIONS

cd superset-frontend
npm run test -- --no-coverage src/dashboard/util/migrateChartCustomization.test.ts

All 52 tests should pass (27 existing + 25 new).

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

Link to Devin session: https://app.devin.ai/sessions/90d9e3cc63d645f1b37e4d43024eff9c
Requested by: @andreicinca

---

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment and CI monitoring

[devin-ai-integration]

Devin Review found 1 potential issue.

View 3 additional findings in Devin Review.

[devin-ai-integration]

🟡 Test codifies falsy-check bug: chartId 0 is silently dropped from chartsInScope

The test comment at line 928 explicitly says chartId as 0 (falsy but valid), acknowledging that 0 is a valid chart ID (the LegacyChartCustomizationItem type at superset-frontend/packages/superset-ui-core/src/query/types/Dashboard.ts:268 defines chartId?: number). However, the test assertion at line 943 expects chartsInScope to be undefined, codifying the falsy-check bug in the implementation at migrateChartCustomization.ts:134 (legacy.chartId ? [legacy.chartId] : undefined). When chartId is 0, the truthiness check treats it as falsy and drops it. The test should assert toEqual([0]) and the implementation should use legacy.chartId != null instead of a truthiness check.

Prompt for agents

The test 'migrateChartCustomization handles chartId as zero' at line 930 asserts that chartsInScope is undefined when chartId is 0, but this codifies a truthiness-check bug in the implementation.

Two changes are needed:

1. In superset-frontend/src/dashboard/util/migrateChartCustomization.ts at line 134, change:
   chartsInScope: legacy.chartId ? [legacy.chartId] : undefined,
   to:
   chartsInScope: legacy.chartId != null ? [legacy.chartId] : undefined,
   (This uses a null check instead of truthiness to handle chartId: 0 correctly.)

2. In the test file at line 943, change the assertion from:
   expect(result.chartsInScope).toBeUndefined();
   to:
   expect(result.chartsInScope).toEqual([0]);
   (This asserts the correct behavior: chartId 0 should be preserved in chartsInScope.)

---

Was this helpful? React with 👍 or 👎 to provide feedback.