WFP-VAM / prism-app

PRISM is an interactive map-based dashboard that simplifies the integration of geospatial data on hazards, along with information on socioeconomic vulnerability
MIT License
47 stars 34 forks source link

[Tech Debt]: Refactor Group Definitions #1322

Open gislawill opened 1 month ago

gislawill commented 1 month ago

Provide a clear and concise description of what you want to happen.

Problem PRISM runs on layer definitions that are defined in our JSON configurations. The vast majority of the application operates under the assumption that the layer definitions are static meaning the layer definitions are constant and do not change during runtime. These layer definitions are not stored in Redux (they're not app state) and we do not have any listeners that adjust the user experience if the layer definitions update (once again, they're not app state). To enforce the static/constant nature of our layer definitions, our layer definitions will even throw errors if part of the app attempts to write to them (though not consistently it appears).

However, the "groups" property is currently written after initialization to the layer definitions by a format function in the LeftPanel (formatLayersCategories in MapView/LeftPanel/utils.ts). This breaks our assumption that layer definitions are static and could easily lead to bugs in the future (the layer definitions updates but the app isn't listening to changes)

graph 
    subgraph "Actual Data Flow (in practice)"
        direction RL
        D[App Configurations] --> E["App State (Redux)"]
        E --> F[User Interface]
        F --> E
        F --> D
    end

    subgraph "Intended Data Flow"
        direction LR
        A[App Configurations] --> B["App State (Redux)"]
        B --> C[User Interface]
        C --> B
    end

Proposed Resolution To resolve this, we should refactor how groups are added to our Layers Definitions. This should occur in the Layer Definition initialization step (LayerDefinitions in config/utils.ts). Once defined there, the groups definition in formatLayersCategories category can be removed and the CommonLayerProps update from 52dbd65 can be removed.

Is there anything else you can add about the proposal? You might want to link to related issues here, if you haven't already.

This issue was identified when resolving a bug in #1301. See this comment thread for details.

gislawill commented 1 month ago

Heads up @wadhwamatic and @ericboucher. Eric, please feel welcome to add any details I missed or suggestions you have

ericboucher commented 1 month ago

The one caveat I would add is that at some points there was some ideas about letting users add layers and test layer configs in the app directly. So we might need to consider that while doing this refactor.