fix: prevent duplicate layers when re-opening a map from Data Visualizer - #3746
Draft
karolinelien wants to merge 5 commits into
Draft
fix: prevent duplicate layers when re-opening a map from Data Visualizer#3746karolinelien wants to merge 5 commits into
karolinelien wants to merge 5 commits into
Conversation
OpenAsMapDialog auto-added single-item/Earth Engine layers by dispatching directly from the render body. Since that path is async, extra re-renders while it was in flight (e.g. while useSetting/legend fetches resolved) re-triggered the dispatch, producing duplicate layers and leaving the map in a broken state. Move the auto-add into an effect guarded to run once. AI Assisted
Contributor
|
🚀 Deployed on https://pr-3746.maps.netlify.dhis2.org |
karolinelien
marked this pull request as ready for review
July 30, 2026 16:33
- Guard the manual Proceed button against double-click duplicate dispatch - Fix Cancel dispatching the action creator instead of dispatch() - Ensure clearAnalyticalObject always runs via try/finally, even if the async add throws - Strengthen tests: StrictMode double-invocation, Proceed double-click, Cancel dispatch - Trim useEffect comment to one line per repo convention AI Assisted
The layer-visibility flag compared a data item id against a dimension object, so every layer created from an analytical object was added switched off -- the other half of the "map fails to render" symptom. Also address review feedback on the duplicate-layer fix: - Collapse hasAutoAdded/isAddingRef into a single hasAddedRef; adding always ends in clearAnalyticalObject(), which unmounts the dialog, so the guard never needs resetting - Disable Proceed while adding, so a double-click gets feedback instead of a silent no-op - Null-check the earth engine layer source before dereferencing it, and drop the inert try/finally around that synchronous path - Preselect the first data item if the analytical object resolves after mount, instead of leaving [undefined] selected - Explain why the effect deps are load-bearing - Cover the earth engine path and layer visibility in the tests AI Assisted Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DHIS2-15762 is the hash-routing feature that introduced the latent bug, not a ticket tracking it. AI Assisted Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
karolinelien
marked this pull request as draft
July 30, 2026 20:30
Resolves the SonarCloud javascript:S7755 code smell and matches the existing idiom in thematicLoader/classify. AI Assisted Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
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-XXXX
Description
Reproduction: open a single-layer map in Maps, use the layer's "Open in Data Visualizer app" menu action, then in Data Visualizer click "Open as map" without making any changes. The map fails to render and the layer list shows two duplicate copies of the same layer.
Two independent bugs in
OpenAsMapDialogcombine to produce that:1. Duplicate layers. The dialog auto-added the layer (for the single-data-item / Earth Engine cases, where no user picker is needed) by dispatching directly from the render body. That path is async, and any re-render while it was in flight re-invoked the same dispatch, adding the layer twice with two different generated ids. Nothing downstream could catch it:
LAYER_ADDassignsid: generateUid()and only dedupes onaction.payload.id, which the payload never carries.This is a long-standing latent bug from #3009 (the class-to-function refactor of this component) — it just needed an extra re-render window to manifest.
Fix: move the auto-add into a
useEffectguarded by a ref so it runs once per mount.2. The layer was added switched off, which is why the map rendered empty.
isVisiblewas computed asdataId === allDataDimensions[len - 1]— an id string compared against a dimension object, so it was alwaysfalse, and the reducer'sisVisible ?? truefallback can't rescue an explicitfalse. Fix: compare against the last id in the loop.Also fixed along the way:
clearAnalyticalObjectaction creator straight toonClick, so it was invoked with the click event and never dispatched — the button did nothing.layerSource.defaultAggregationsbefore null-checkinglayerSource.clearAnalyticalObject()now always runs viafinally, so a rejected legend-set fetch no longer leaves the dialog stuck.[undefined]selected with Proceed enabled.AI Assisted
Quality checklist
Add N/A to items that are not applicable.
ToDos
Known issues
Screenshots
N/A