feat: add Combined data table for cross-layer org-unit/spatial joins [PR8] - #3769
Draft
BRaimbault wants to merge 9 commits into
Draft
BRaimbault wants to merge 9 commits into
BRaimbault wants to merge 9 commits into
Conversation
BRaimbault
changed the base branch from
master
to
feat/datatable-pr7-refinements
September 10, 2026 12:47
Contributor
|
🚀 Deployed on https://pr-3769.maps.netlify.dhis2.org |
BRaimbault
force-pushed
the
feat/datatable-pr7-refinements
branch
from
September 10, 2026 12:49
9534ce2 to
7e387fd
Compare
…20543] Ports the state layer for the Combined view (cross-layer data table joins) from feat/datatable-pr7-multilayer: combinedView on state.dataTable, cross-layer selection on state.selection, combinedVisibleIds on state.ui, and per-layer combinedLayerKey/ combinedJoinConfig/combinedColumnConfig on state.map.mapViews, plus their action types/creators. Also fixes removeLayer to forward combinedLayerKey so the new LAYER_REMOVE cleanup in reducers/map.js can actually match. The map reducer's DATA_TABLE_COMBINED_VIEW_TOGGLE case (which creates the backing reference org-unit layer) is intentionally deferred to the next commit, since it depends on getDefaultReferenceRows, which in turn depends on the join utilities ported in that step. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…yer [DHIS2-20543] Ports the join-matching utilities from feat/datatable-pr7-multilayer: spatialJoin.js (point-in-polygon matching for the spatial join type) and combinedJoinMatch.js (org unit/parent-org-unit path matching, plus join-quality helpers used by the join controls UI later). Adds getDefaultReferenceRows to util/dataTable.js, which auto-selects the best candidate layer's org units to seed the Combined view's hidden reference layer (preferring finer org unit levels, then a fixed type priority order). This unblocks reducers/map.js's DATA_TABLE_COMBINED_VIEW_TOGGLE case (added in the previous commit's state scaffolding but left unwired pending this dependency), which creates that hidden combinedTableRef mapView on first toggle. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Ports the reference org-unit layer editing/loading/exclusion pieces from feat/datatable-pr7-multilayer: OrgUnitDialog.jsx/LayerEdit.jsx reuse the normal org-unit edit dialog for it (hiding the Style tab, swapping the modal title/submit label), a new ReferenceOrgUnitControl opens its editor, orgUnitLoader.js special-cases loading it (always counting org units without coordinates, suppressing the "no coordinates" warning, persisting combinedJoinConfig/ combinedColumnConfig/combinedLayerKey), and MapContainer.jsx/ plugin/Map.jsx/LayersPanel.jsx exclude the hidden layer from actual map rendering and the layers panel list. Also fixes two gaps found by review that a straight file-by-file port missed: - useLayersLoader.js had no loaders['combinedTableRef'] entry, so the reference layer would never actually load, permanently blocking the Interpretations panel and surfacing a "Could not load layer" alert. - RenderingStrategy.jsx iterates mapViews to gate the Split rendering strategy and wasn't excluding the reference layer either, unlike every other mapViews-iterating call site touched in this commit - this one doesn't exist on the source branch, so it's a fix beyond the straight port. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Ports the value-column and aggregation infrastructure from feat/datatable-pr7-multilayer: util/aggregation.js (generic SUM/AVERAGE/COUNT/MIN/MAX/STDDEV/VARIANCE plus DHIS2/Earth-Engine aggregation-type defaulting), the DATA_KEY_KIND_* constants, and a large block of new util/dataTable.js exports (getCombinedValueDataKeys, getCombinedLegendConfig, getDefaultCombinedAggregation, getFeatureCategoryKey, mergeCrossLayerIds, getUnionBounds) that decide, per layer type, which value/count/category columns a Combined-view join produces. isDataTableOpen/getLayerSelectedIds gain combinedView/ crossLayerIds awareness without changing their existing single-layer behavior. useCombinedTableData.js is the hook that ties it together: one row per reference org unit, joined layer values/counts/categories computed per row, reusing the same filterData/compareRows/ buildRowCells/getColumnDistinctValues pipeline the single-layer table already uses. getRowClickAction and friends in util/dataTable.js are deliberately untouched - this branch's row-selection anchor fix must not be overwritten by the source branch's older behavior. Also ports DataTableButton.jsx's combinedView-aware open/close/reopen logic (a real gap a straight per-file port would have missed, since it wasn't in this step's original file list - found via review). Note for follow-up (inherited from the source branch, not introduced here, not fixed to keep this a faithful port): a few latent correctness/efficiency issues survive unchanged from feat/datatable-pr7-multilayer - MIN/MAX aggregation spreads arrays into Math.min/max (RangeError risk on very large joins), multi-stat Earth Engine layers default every stat's aggregation type from stat[0], GEOJSON_URL_LAYER has no case in getCombinedValueDataKeys (falls back to an always-null generic value column), and getDefaultCombinedAggregation/ getFeatureCategoryKey are recomputed per-row instead of once per layer in useCombinedTableData's hot path. Worth a dedicated cleanup pass once the whole feature is ported and working end-to-end. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…DHIS2-20543] The highest-risk step of the port: re-adds combinedVisibleIds and crossLayerIds-aware hover/selection/visibility support to the base Layer class, which was removed from this branch alongside an unrelated independent refactor that renamed every lifecycle method (handle*->sync*, getHoverIds->getHoverId singular) and added a reference-equality guard to syncVisibleIds. - getHoverIds (plural, replacing getHoverId) merges a layer's own hover id with any crossLayerIds entry for it. - getSelectedIds delegates to util/dataTable.js's getLayerSelectedIds (already crossLayerIds-aware from an earlier commit). - getVisibleIds/getSelectionFilterIds combine the existing selection- filter logic with a new combinedVisibleIds intersection, using a new visibleIdsEqual (null = unrestricted, [] = hide everything - these are not the same thing, unlike the old `?? []` coercion this exact branch's own row-selection session fixed for a different case). - combinedVisibleIds threaded through MapContainer.jsx -> MapView.jsx -> Map.jsx/SplitView.jsx -> Layer. - util/map.js regains getLayerFeatureHighlight (per-overlay highlight filtering, crossLayerIds-aware) and fitCrossLayerZoomBounds (union- bbox zoom for a cross-layer feature), wired into Map.jsx. - onFeatureLeftClick now reports multiSelect on the clickFeature payload (consumed by a later port step). Also fixes two real gaps found by an extensive review pass, both directly relevant to code touched in this same step: - getLayerFeatureHighlight checked crossLayerIds[layerId] for bare truthiness instead of .length, inconsistent with the .length check right next to it in getSelectionFilterIds's isReferenced. - syncVisibleIds had no reference-equality fast path, unlike every sibling sync* method in this same class - added one, matching the established pattern, so an unrelated prop change (opacity, order, hover, highlight color) no longer recomputes visible ids on every layer on every update. Everything else the review surfaced (several near-duplicate id-lookup implementations, split-view not yet filtering per-layer feature/zoom, getHoverIds not deduping against crossLayerIds, fitCrossLayerZoomBounds being currently unreachable pending the context-menu step) was confirmed to already exist unchanged on the verified source branch - left alone to keep this a faithful port, tracked in project_datatable_pr8_port_progress memory for a later cleanup pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Ports the main Combined-view UI component from feat/datatable-pr7-multilayer:
CombinedDataTable.jsx (reuses TableVirtuosoComponents, CellValue,
SelectionCheckboxColumn, SortableColumnHeader, FilterInput, and the
dispatch-agnostic useRowClickSelection/useRowSelection hooks verbatim
from the single-layer table) and CombinedTableContextMenu.jsx (zoom to
feature/selected/filtered features, using the union-bbox zoom already
ported to util/dataTable.js/util/map.js).
Rewired the checkbox/row-selection wiring to match this branch's
current useRowClickSelection API (added when this branch fixed the
row-selection anchor bugs): destructure {onRowClick, onCheckboxToggle,
resetAnchor}, pass selectedIdSet, wire onCheckboxToggle with the click
event for shift-click-on-checkbox range-select, and reset the anchor
before onToggleSelectAll/onReverseSelection - mirroring DataTable.jsx
exactly.
An 8-angle review pass converged strongly (3 independent
confirmations) on one real bug: local `selectedIds` state was seeded
once from Redux and never resynced, so any Redux action that resets
selection (SELECTION_CLEAR, MAP_NEW, a reference-layer change) would
leave the table's checkboxes stale relative to the map. Fixed by
deriving selectedIds live via useMemo + the existing
getLayerSelectedIds util, exactly matching how DataTable.jsx already
does it - no more local selection state at all.
Also fixed three smaller gaps the review found: missing
computeItemKey on TableVirtuoso (index-based fallback risks
row-identity bugs across sort/filter), tableContext.layout hardcoded
to 'auto' instead of following columnWidths.length (misaligns the
pinned columns this file itself implements), and hasActiveFilters
hand-rolled instead of reusing the existing hasActiveDataTableFilters
util (was silently missing showOnlyFeaturesInView, incorrectly
disabling "Zoom to filtered features").
Remaining review findings (a real O(rows) recompute on every checkbox
click, an O(rows^2) risk in mergeCrossLayerIds, a stale shift-click
anchor edge case, and a couple of others) were confirmed inherited
unchanged from the verified source branch and are tracked in
project_datatable_pr8_port_progress memory for a follow-up pass -
CombinedDataTable has no caller yet (lands in the next step), so
several of these can't be verified live until then.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds the "Combined" pseudo-option to the layer selector, the join-layers popover (JoinLayersControl/LayerRow), and swaps in CombinedDataTable for DataTable when combinedView is active, integrated with this branch's existing debounced-search and usePanelHeights hooks rather than the old inline implementations from the source branch. Fixed beyond the straight port: JoinLayersControl's isServerClustered computed a bare `undefined` (not `false`) for layers without a serverCluster field, tripping LayerRow's required-bool PropTypes check; coerced with `!!(...)`. Also restored/added test coverage the port's verbatim BottomPanel.spec.jsx replacement left gaps in: the stale activeLayerId fallback, the Combined-to-single-layer selection transition, and the single-layer-mode Clear-filters branch. Confirmed NOT a bug, despite two review agents disagreeing: reducers/map.js's DATA_TABLE_COMBINED_VIEW_TOGGLE case (Step 1) synchronously creates the placeholder reference mapView in the same dispatch that flips dataTable.combinedView to true, so CombinedDataTable's referenceLayer prop is never actually undefined while combinedView is true - verified by reading both reducer cases directly rather than trusting either agent's trace. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…y [DHIS2-20543] Ports favorites.js's round-trip for the Combined table's per-layer join config, column config, and stable cross-save layer key, mirroring the existing dataTableColumnConfig pattern exactly (confirmed byte-identical to feat/datatable-pr7-multilayer's version of this file after applying). Fixed beyond the straight port: the new untouched-placeholder cleanup filter in cleanMapConfig read view.combinedJoinConfig as a flat property, but that shape only exists when cleanMapviewConfig is true. FileMenu.jsx's rename-only save path calls cleanMapConfig with cleanMapviewConfig: false on a freshly-fetched map whose combinedJoinConfig is still packed inside the unparsed config JSON string - so a Combined-table reference layer with join layers picked but no org units selected yet would have silently lost its entire join configuration on rename. Gated the filter on cleanMapviewConfig, consistent with every other check in this same function. Added test coverage for combinedColumnConfig/combinedLayerKey round-tripping (the source branch's tests only covered combinedJoinConfig) and for the cleanMapviewConfig: false regression. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Picks up the new translatable strings introduced across the PR8 port (join controls, zoom-menu labels, column headers, warnings). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
BRaimbault
force-pushed
the
feat/datatable-pr8-combined-table
branch
from
September 10, 2026 12:58
07a2ccc to
75036d4
Compare
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements DHIS2-20543
Supersedes #3741 — that PR bundled this together with the shared multi-layer tabs/rendering architecture on
feat/datatable-pr7-multilayer; it was split into PR7 (#3763, shared architecture) and this PR (the Combined data table itself), ported forward onto PR7's branch.Description
Adds the Combined data table — a new view that joins multiple map layers into a single table, keyed off a reference org-unit layer, instead of viewing one layer's data at a time.
Manually verified end-to-end against a live DHIS2 instance: building a map with two joinable layers, switching to Combined, configuring the join, row selection, and context-menu zoom, with values cross-checked against the raw analytics API.
Quality checklist
Add N/A to items that are not applicable.
Screenshots
supporting images