fix: show full geo name behavior on county maps#2706
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the map DataTable CSV-export behavior so “FullGeoName” becomes truly toggle-driven (instead of implicitly forced for county maps), while preserving legacy county-map behavior via a 4.26.4 config migration.
Changes:
- Add a 4.26.4 migration to enable
table.showFullGeoNameInCSVfor legacyus-countymaps (including those embedded in dashboards). - Extract “add FullGeoName column” logic into a dedicated
addOptionalFullGeoNameColumnhelper and apply it fromDataTable. - Add unit tests covering the helper behavior and the new migration behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/helpers/ver/4.26.4.ts | Adds migration step to force-enable FullGeoName CSV export for legacy county maps. |
| packages/core/helpers/ver/tests/4.26.4.test.ts | Adds tests validating migration behavior for map + dashboard cases. |
| packages/core/components/DataTable/helpers/addOptionalFullGeoNameColumn.ts | New helper to conditionally add FullGeoName column to CSV export output. |
| packages/core/components/DataTable/helpers/tests/addOptionalFullGeoNameColumn.test.ts | Adds vitest coverage for helper behavior across toggle on/off cases. |
| packages/core/components/DataTable/DataTable.tsx | Replaces inline FullGeoName CSV logic with the new helper call. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!config?.table?.showFullGeoNameInCSV || typeof formatLegendLocation !== 'function') { | ||
| return csvDataUpdated | ||
| } | ||
|
|
||
| return csvDataUpdated.map((row, index) => { | ||
| const originalRow = csvData[index] | ||
| if (!originalRow) { | ||
| console.warn('Data mismatch: originalRow missing.', { | ||
| index, | ||
| csvDataLength: csvData.length, | ||
| csvDataUpdatedLength: csvDataUpdated.length | ||
| }) | ||
| return row | ||
| } | ||
|
|
||
| return { | ||
| FullGeoName: formatLegendLocation(originalRow[config.columns.geo.name]), | ||
| ...row |
There was a problem hiding this comment.
addOptionalFullGeoNameColumn can throw if config.columns, config.columns.geo, or config.columns.geo.name is missing while table.showFullGeoNameInCSV is true (e.g., partially-migrated/hand-edited configs). Consider defensively checking for the geo column name (and the corresponding value on originalRow) and returning csvDataUpdated/row when unavailable to avoid runtime crashes during CSV export.
| config: any | ||
| csvData: Record<string, any>[] | ||
| csvDataUpdated: Record<string, any>[] | ||
| formatLegendLocation?: (key: string) => string |
There was a problem hiding this comment.
The formatLegendLocation type here is (key: string) => string, but DataTableProps.formatLegendLocation is typed as (row: string, runtimeLookup: string) => string. To keep types consistent and avoid subtle TS assignability issues, consider reusing the prop type (e.g., DataTableProps['formatLegendLocation']) or updating this signature to accept the optional second parameter.
| formatLegendLocation?: (key: string) => string | |
| formatLegendLocation?: (key: string, runtimeLookup?: string) => string |
No description provided.