fix: move null is zero to data source, fix bugs by joaquimds · Pull Request #340 · commonknowledge/ts-mapped

joaquimds · 2026-02-26T15:35:40Z

No description provided.

Copilot

Pull request overview

This PR moves the “treat empty values as zero” behavior from per-map-view configuration to the DataSource model/config, and updates stats calculation + UI to reflect the new source of truth.

Changes:

Persist nullIsZero on DataSource (schema + migration) and allow updating it via dataSource.updateConfig.
Remove nullIsZero/areaDataNullIsZero from area stats inputs and map view config, and update area stats computation to use dataSource.nullIsZero.
Update UI: remove the per-view toggle, add a DataSource configuration toggle, and improve Legend label rendering with value labels.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/server/trpc/routers/dataSource.ts	Includes `nullIsZero` in DataSource config updates.
src/server/trpc/routers/area.ts	Removes `nullIsZero` from the area stats input.
src/server/stats/index.ts	Stats now use `dataSource.nullIsZero` (and adjust MODE selection).
src/server/models/MapView.ts	Removes `areaDataNullIsZero` from persisted view config schema.
src/server/models/DataSource.ts	Adds `nullIsZero` to the DataSource Zod schema/type.
src/app/map/[id]/utils/mapView.ts	Removes default `areaDataNullIsZero` from new view configs.
src/app/map/[id]/data.ts	Stops sending `nullIsZero` to the stats query and removes it from reset deps.
src/app/map/[id]/components/controls/VisualisationPanel/VisualisationPanel.tsx	Removes the per-view “Treat empty values as zero” switch; minor className update.
src/app/map/[id]/components/Legend.tsx	Uses `valueLabels` for legend labels; adjusts tick generation based on labels.
src/app/(private)/data-sources/[id]/components/ConfigurationForm.tsx	Adds a DataSource-level “Treat empty values as zero” switch and includes it in updates.
migrations/1772117945260_data_source_null_is_zero.ts	Adds `nullIsZero` column to `dataSource`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-26T15:42:26Z

src/app/map/[id]/components/Legend.tsx

    if (hasValueLabels) {
-      numTicks = Object.keys(valueLabels).length;
+      values = Object.keys(valueLabels)
+        .map(Number)
+        .filter((v) => v >= colorScheme.minValue && v <= colorScheme.maxValue)
+        .toSorted();
+      numTicks = values.length;
      denom = Math.max(numTicks - 1, 1);
-      values = Object.keys(valueLabels).map(Number).toSorted();
    }


When hasValueLabels is true, keys are filtered to those within [minValue, maxValue]. If none of the configured labels fall in-range, this sets numTicks to 0 and renders no tick labels (but still reserves the taller h-10 layout). Consider falling back to the default tick generation when the filtered list is empty (or at least keeping a minimum of 2 ticks).

Copilot · 2026-02-26T15:42:26Z

src/server/trpc/routers/dataSource.ts

        dateFormat: input.dateFormat,
        public: input.public,
        naIsNull: input.naIsNull,
+        nullIsZero: input.nullIsZero,
      } as DataSourceUpdate;


The new nullIsZero flag is persisted via updateConfig, but the mutation handler only enqueues an importDataSource job when certain config fields change. If naIsNull (and possibly nullIsZero, depending on intent) should take effect on existing imported records, it likely needs to be included in the “configFieldsChanged” check; otherwise the UI’s “Save and import” flow won’t actually import when toggling these flags.

Copilot · 2026-02-26T15:42:27Z

src/app/(private)/data-sources/[id]/components/ConfigurationForm.tsx

    updateDataSourceConfig({
      dataSourceId: dataSource.id,
      columnRoles,
      geocodingConfig: validGeocodingConfig,
      autoImport,
      dateFormat,
      public: isPublic,
      naIsNull,
+      nullIsZero,
    });


After saving nullIsZero, the client only invalidates dataSource.checkWebhookStatus. Since map rendering/stats now depend on dataSource.nullIsZero, consider also invalidating cached data source queries (e.g. dataSource.byId, dataSource.listReadable) and/or area.stats so other parts of the app don’t keep using stale cached results.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2026-02-26T15:42:27Z

src/app/map/[id]/data.ts

  const {
    calculationType,
    areaDataColumn: column,
    areaDataSecondaryColumn: secondaryColumn,
    areaDataSourceId: dataSourceId,
-    areaDataNullIsZero: nullIsZero,
    areaSetGroupCode,
  } = viewConfig;



nullIsZero no longer participates in the area.stats query input/query key. Since the server-side stats result now depends on dataSource.nullIsZero, React Query can keep serving cached stats after the flag changes (e.g., user updates the data source config in another tab) until a manual invalidation/refetch happens. Consider invalidating trpc.area.stats queries when dataSource.updateConfig succeeds, or include a stable dataSource config version (or nullIsZero) in the query key/reset key so stats recompute immediately after this setting changes.

Copilot · 2026-02-26T15:46:41Z

@joaquimds I've opened a new pull request, #341, to work on those changes. Once the pull request is ready, I'll request review from you.

…onfig save Co-authored-by: joaquimds <12935136+joaquimds@users.noreply.github.com>

Invalidate additional caches after data source config save

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-26T16:12:46Z

src/server/stats/index.ts

+  // Coalesce mode empty values to 0 if the column is numeric and nullIsZero is set
+  const modeSelect =
+    columnDef.type === ColumnType.Number && dataSource.nullIsZero
+      ? sql`MODE () WITHIN GROUP (ORDER BY COALESCE(NULLIF(json->>${column}, ''), '0'))`
+      : sql`MODE () WITHIN GROUP (ORDER BY NULLIF(json->>${column}, ''))`;


The new MODE query path also changes behavior when nullIsZero is enabled (and now treats empty strings as NULL when disabled). Please add a focused test for CalculationType.Mode on a numeric column with blank values to lock in the intended behavior for both nullIsZero=true and nullIsZero=false.

Copilot · 2026-02-26T16:12:47Z

src/server/stats/index.ts

+  // Coalesce numeric empty values to 0 if set
+  const numberSelect = dataSource.nullIsZero
    ? sql`(COALESCE(NULLIF(json->>${column}, ''), '0'))::float`
    : sql`(NULLIF(json->>${column}, ''))::float`;


getColumnValueByArea now changes numeric aggregation semantics based on dataSource.nullIsZero (coalescing empty strings to 0). There are existing Vitest feature tests that exercise getAreaStats, but none cover the new nullIsZero behavior; please add a test case that imports a CSV with blank numeric values and asserts that Sum/Avg differ when nullIsZero is enabled vs disabled.

…edge/ts-mapped into feat/fix-null-is-zero-issues

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-26T18:11:32Z

src/app/map/[id]/components/controls/VisualisationPanel/VisualisationPanel.tsx

                >
                  <SelectTrigger
-                    className="w-full min-w-0"
+                    className={cn("w-full min-w-0", SELECT_TO_BUTTON_CLASSES)}


The constant SELECT_TO_BUTTON_CLASSES is referenced but not visible in the diff. Ensure this constant is properly imported or defined in this file.

src/app/map/[id]/components/Legend.tsx

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix: move null is zero to data source, fix bugs

b0e2850

joaquimds requested a review from Copilot February 26, 2026 15:35

Copilot started reviewing on behalf of joaquimds February 26, 2026 15:36 View session

Copilot AI reviewed Feb 26, 2026

View reviewed changes

Initial plan

8115d80

Copilot AI mentioned this pull request Feb 26, 2026

Invalidate additional caches after data source config save #341

Merged

Copilot AI and others added 2 commits February 26, 2026 15:51

fix: invalidate dataSource.byId, listReadable, and area.stats after c…

c4d7e3b

…onfig save Co-authored-by: joaquimds <12935136+joaquimds@users.noreply.github.com>

Merge pull request #341 from commonknowledge/copilot/sub-pr-340

2434a34

Invalidate additional caches after data source config save

joaquimds requested a review from Copilot February 26, 2026 16:07

Copilot started reviewing on behalf of joaquimds February 26, 2026 16:08 View session

Copilot AI reviewed Feb 26, 2026

View reviewed changes

joaquimds added 2 commits February 26, 2026 19:07

feat: add area stats test

ec7ce72

Merge branch 'feat/fix-null-is-zero-issues' of github.com:commonknowl…

f9b198d

…edge/ts-mapped into feat/fix-null-is-zero-issues

joaquimds requested a review from Copilot February 26, 2026 18:07

Copilot started reviewing on behalf of joaquimds February 26, 2026 18:08 View session

Copilot AI reviewed Feb 26, 2026

View reviewed changes

joaquimds and others added 2 commits February 26, 2026 19:12

Update src/app/map/[id]/components/Legend.tsx

0845a5f

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

fix: safer handling of non-numeric value labels in legend

da79fe8

joaquimds merged commit e54925d into main Feb 26, 2026
1 check passed

joaquimds deleted the feat/fix-null-is-zero-issues branch February 26, 2026 18:15

Uh oh!

Conversation

joaquimds commented Feb 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

joaquimds Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Feb 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants