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

Model table update #157

Merged
merged 8 commits into from
Jan 9, 2025
Merged

Model table update #157

merged 8 commits into from
Jan 9, 2025

Conversation

pgm
Copy link
Contributor

@pgm pgm commented Dec 20, 2024

In 24Q4 some columns changed names and some additional columns have been added. Since the model table consistently changes from release to release, I changed strategy in this PR. The front end will always have a dependency on the schema of model.csv. However, that doesn't mean all of the portal-backend code needs to as well. Instead of mapping column names from model.csv -> snake case -> portal db -> json response -> react component, I've cut out several layers.

Now, any fields in that the portal depends on, should still go into the database, but for all columns, with column names as they appear in the Model.csv go into a single field as a json encoded dict, which the frontend can receive. There is also a interface automatically defined to ensure the frontend is kept in sync with the columns in the actual model.csv file.

This means that for columns that the backend doesn't care about the flow is:
model.csv -> db -> react component

Which eliminates a lot of boilerplate.

(Also this PR includes some small fixes to react warnings I saw when testing)

@pgm pgm requested a review from snwessel December 20, 2024 20:02
@pgm pgm requested a review from alimourey January 6, 2025 18:45
Copy link
Member

@snwessel snwessel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me (and much simpler!)

I appreciate all the explanatory comments.

@pgm pgm merged commit 8fb141d into master Jan 9, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants