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

Migrate election primary precinct split fixture to use generated version #5732

Merged
merged 18 commits into from
Dec 12, 2024

Conversation

carolinemodic
Copy link
Contributor

Overview

Migrates all usages of electionPrimaryWithPrecinctSplits to use newly generated fixtures with grid layouts and translations. I am not migrating everything to use election packages rather then election.json files. There is code in VxAdmin especially (but also other places) constructing election packages out to election files for tests and ideally those will instead just configure from the election package now exported. However I think it will be easier to update those tests once all of the fixtures they are using are converted AND I don't think its strictly necessary to migrate them all so I'm holding off on that for now.

I considered adding a asElectionPackage() function automatically added to the generated .zip.ts file in res-to-ts but the code that handles election package parsing right now lives in libs/backend and imports helpers from libs/utils neither of which are current dependencies of libs/fixtures. Ultimately its not that hard to just call another helper function when you are using the fixture and I didn't want to over-complicate the dependency graph so I did not add that.

The toElectionPackage() function that is added to election.json.ts functions I hope to remove at the end of this work and migrate anything using that function to instead load in the ElectionPackage from the .zip export. I'm intentionally NOT exporting that from the fixtures index.ts file to make sure any usages are updated, for this particular election there were none.

Demo Video or Screenshot

N/A

Testing Plan

Tests passing

@carolinemodic carolinemodic marked this pull request as ready for review December 11, 2024 21:29
@carolinemodic carolinemodic requested review from jonahkagan and removed request for eventualbuddha December 11, 2024 21:29
@carolinemodic carolinemodic force-pushed the caro/migrate_election_primary_precinct_split branch from 29bc2e9 to c0530b3 Compare December 12, 2024 17:19
JSON.stringify(electionWithGridLayouts)
).unsafeUnwrap();
return sortedElectionWithGridLayouts;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this specifically about the field order? If, so maybe include that in the comment

Why does this matter now and didn't matter before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not entirely sure what this is mattering now and not before other then this specific flow of creating/reading elections not existing previously. Before this specific flow rewrote a new election json instead of relying on an old one matching. Basically the issue is without this change the generate-election-packages script would plop gridLayouts at the end of the JSON. When libs/hmpb generate-fixtures was reading it in and creating new ballots it would get a new hash of the election after reading in where gridLayouts are now higher up in the order of keys so the hashes don't match even though the contents are identical.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will update the comment

@carolinemodic carolinemodic force-pushed the caro/migrate_election_primary_precinct_split branch from c0530b3 to 5ce9b08 Compare December 12, 2024 19:24
@carolinemodic carolinemodic merged commit 970ce58 into main Dec 12, 2024
62 checks passed
@carolinemodic carolinemodic deleted the caro/migrate_election_primary_precinct_split branch December 12, 2024 23:35
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