Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Tech Debt]: Refactor Group Definitions #1322

Open
gislawill opened this issue Jul 26, 2024 · 2 comments
Open

[Tech Debt]: Refactor Group Definitions #1322

gislawill opened this issue Jul 26, 2024 · 2 comments
Assignees
Labels
enhancement New feature or request triage to be triaged for next action

Comments

@gislawill
Copy link
Collaborator

gislawill commented Jul 26, 2024

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
Loading

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 gislawill added enhancement New feature or request triage to be triaged for next action labels Jul 26, 2024
@gislawill gislawill self-assigned this Jul 26, 2024
@gislawill
Copy link
Collaborator Author

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

@ericboucher
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request triage to be triaged for next action
Projects
None yet
Development

No branches or pull requests

2 participants