Skip to content

Commit d9f04d6

Browse files
Superset Devclaude
andcommitted
fix(plugin-chart-country-map): admin_level string/number coercion + lower row_limit default
Two user-visible bugs fixed: 1. **"No GeoJSON URL resolved" with admin_level + country set.** The SelectControl serializes admin_level as a string ('0' / '1' / 'aggregated'), but transformProps was comparing against numbers (`adminLevel === 1`), so real-world charts silently fell through every branch and returned a null URL. Existing tests passed because they used number values directly. Normalize to string at the top of transformProps and compare against string constants; add two regression tests that pass the string form-data values reality actually sends. 2. **Default row_limit of 50000 exceeds many deployments' configured ROW_LIMIT ceiling, blocking Update Chart.** Choropleths key one row per region — even the densest country maps (France 101 departments, India 36 states, US 51 territories) are well under 10k rows. Override the shared default to 10000 via controlOverrides. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent ba4367d commit d9f04d6

3 files changed

Lines changed: 43 additions & 3 deletions

File tree

superset-frontend/plugins/plugin-chart-country-map/src/plugin/controlPanel.tsx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,14 @@ const config: ControlPanelConfig = {
574574
linear_color_scheme: {
575575
renderTrigger: true,
576576
},
577+
// Choropleths key one row per region — even the densest country maps
578+
// (e.g. France 101 departments, India 36 states, US 51 territories)
579+
// are well under 10k rows. The shared row_limit default of 50k is
580+
// both wasteful and exceeds many deployments' configured ROW_LIMIT
581+
// ceiling, blocking Update Chart with no visible explanation.
582+
row_limit: {
583+
default: 10000,
584+
},
577585
},
578586
// formDataOverrides runs when the user switches a chart's viz_type to
579587
// this plugin. We use it for two jobs:

superset-frontend/plugins/plugin-chart-country-map/src/plugin/transformProps.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,16 +53,21 @@ export default function transformProps(
5353
const data = (queriesData?.[0]?.data as Record<string, unknown>[]) ?? [];
5454

5555
const worldview = formData.worldview || 'ukr';
56-
const adminLevel = formData.admin_level ?? 0;
56+
// SelectControl serializes the admin_level value as a string ('0' /
57+
// '1' / 'aggregated'), but the form_data type allows number 0/1 too
58+
// (legacy migration, jest tests). Normalize to a string before any
59+
// comparison so we don't silently fall through every branch and
60+
// return a null URL.
61+
const adminLevel = String(formData.admin_level ?? '0');
5762

5863
let geoJsonUrl: string | null = null;
5964
if (formData.composite) {
6065
geoJsonUrl = `${GEOJSON_BASE}/composite_${formData.composite}_${SHARED_ADMIN1_WORLDVIEW}.geo.json`;
6166
} else if (formData.region_set && formData.country) {
6267
geoJsonUrl = `${GEOJSON_BASE}/regional_${formData.country}_${formData.region_set}_${SHARED_ADMIN1_WORLDVIEW}.geo.json`;
63-
} else if (adminLevel === 1 && formData.country) {
68+
} else if (adminLevel === '1' && formData.country) {
6469
geoJsonUrl = `${GEOJSON_BASE}/${SHARED_ADMIN1_WORLDVIEW}_admin1_${formData.country}.geo.json`;
65-
} else if (adminLevel === 0) {
70+
} else if (adminLevel === '0') {
6671
geoJsonUrl = `${GEOJSON_BASE}/${worldview}_admin0.geo.json`;
6772
}
6873

superset-frontend/plugins/plugin-chart-country-map/test/plugin/transformProps.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,33 @@ test('Admin 1 stays on the shared (ukr) file regardless of worldview', () => {
6363
);
6464
});
6565

66+
// Regression: SelectControl serializes admin_level as a string '1', but
67+
// the form_data type also allows the number 1 (legacy migration, jest
68+
// cases). Both must resolve to the same Admin 1 URL — previously the
69+
// number-only check meant real-world string values silently fell
70+
// through every branch and the chart rendered "No GeoJSON URL resolved".
71+
test('Admin 1 with string admin_level resolves the URL too', () => {
72+
const out = transformProps(
73+
buildChartProps({
74+
admin_level: '1' as unknown as 1,
75+
country: 'FRA',
76+
worldview: 'ukr',
77+
}),
78+
);
79+
expect(out.geoJsonUrl).toBe(
80+
'/static/assets/country-maps/ukr_admin1_FRA.geo.json',
81+
);
82+
});
83+
84+
test('Admin 0 with string admin_level resolves the URL too', () => {
85+
const out = transformProps(
86+
buildChartProps({ admin_level: '0' as unknown as 0, worldview: 'ukr' }),
87+
);
88+
expect(out.geoJsonUrl).toBe(
89+
'/static/assets/country-maps/ukr_admin0.geo.json',
90+
);
91+
});
92+
6693
test('Region set + country → regional aggregation URL (shared)', () => {
6794
const out = transformProps(
6895
buildChartProps({

0 commit comments

Comments
 (0)