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

Standardize the handling of state variables, the passing of setters, and the use of constants in the TEFCA viewer #2573

Closed
bamader opened this issue Sep 18, 2024 · 3 comments
Assignees
Labels
tech debt Technical debt related work viper

Comments

@bamader
Copy link
Collaborator

bamader commented Sep 18, 2024

No description provided.

@bamader bamader added viper tech debt Technical debt related work labels Sep 18, 2024
@bamader
Copy link
Collaborator Author

bamader commented Sep 18, 2024

The mad rush to get the TEFCA viewer in a proper state before the connectathon unveiled that new feature development, particularly with respect to internal data use, is painful for developers. There are several reasons for this, some of which include: an unintuitive data flow with how state is created and passed between pages and components (e.g. replication of state with only minor name variations, some state being created at the top level and some being relegated to components), lack of standardization / conventions in variable naming and creation/passing, duplication of large numbers of constants for seemingly no purpose (the number of XXXXXtoQueryMap type structures we have is wildly confusing and unnecessary), different ways of handling clicks / submits / button access (some with useEffect, some with functions), different ways of managing the demoQueryPatientChange options (some use a dropdown, some use a constants map, and some use a .find() operation on a list of constants to pick out the right element), and more. Basically, it looks like the app was put together by a bunch of data engineers who just implemented the first thing they could to make the front end work and stop react from complaining to them. Now that we have a better idea as to the workflow of the app and the likely use cases, we should go back through and streamline the whole thing, including architecting pages, components, state, and data to reflect a unified design pattern instead of a slapdash assortment of "whatever works." Focus should be paid to instituting explicit conventions (for example, "top level pages should be the ones to create shared state, which should be passed to child components"; or, "all constants only used by a single service should go in the .tsx file for that service, and only constants shared across two or more services should go in constants.tsx").

@bamader
Copy link
Collaborator Author

bamader commented Sep 18, 2024

To elaborate, this refactor for organization and clarity should be on par with the refactor work done on the Orchestration service a while ago. That service was haphazard in passing data around and lacked a clean, streamlined design because stuff was just duct taped together to make it work. The same thing has happened in the TEFCA viewer, and so it deserves the same full sprint cleanup treatment we gave orchestration.

@fzhao99
Copy link
Collaborator

fzhao99 commented Sep 24, 2024

The team decided that doing a round of writing to discuss scope was a good next step. Doc started here: https://docs.google.com/document/d/1sUPmHkbFCzwVWA25X32N5xVytMoTSQrHLXcnP2Y48Us/edit

@fzhao99 fzhao99 closed this as completed Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech debt Technical debt related work viper
Projects
None yet
Development

No branches or pull requests

2 participants