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

Implement contest choice name standardization flow #1948

Merged
merged 15 commits into from
Jun 24, 2024

Conversation

arsalansufi
Copy link
Contributor

@arsalansufi arsalansufi commented Jun 20, 2024

Overview

Issue link: #1917

This PR implements a contest choice name standardization flow that mirrors our existing contest name standardization flow!

We long ago made the assumption that, though contest names can vary across jurisdictions, choice names would not. That assumption has since been proven wrong, notably in Nevada and Washington. The new flow allows you to add choice names to your standardized contests file and map CVR choice names to those standardized choice names.

The new "Choice Names" column in the standardized contests file isn't required. If you don't include it, we'll still try to detect and warn you when contest choice names are inconsistent across jurisdictions, per existing logic. Main change to that existing warning is that it'll now suggest adding choice names to your standardized contests file as a workaround. I didn't want to require the new column because it might actually be a hindrance for some users. I'm thinking of OC, who audits many contests and has only one jurisdiction.

Note that contest name standardization does affect contest choice name standardization. We don't explicitly block contest choice name standardization on contest name standardization. You can start standardizing choice names before you standardize contest names. It just may be that standardizing contest names surfaces more choice names to be standardized.

Sample standardized contests file with choice names

We're using slashes as separators within the "Choice Names" field instead of commas since individual choice names may contain commas.

Contest Name,Jurisdictions,Choice Names
State Treasurer,all,"Conine, Zach/Elliott, Bryan/Fiore, Michele/Hendrickson, Margaret/None of These Candidates"

The template standardized contests file is unchanged and doesn't include the new column.

Screencaptures and screenshots

End-to-end, before adding choice names to the standardized contests file, after adding choice names to the standardized contests file, and through contest choice name standardization

e2e.mov

Getting started on contest choice name standardization before contest name standardization, completing contest name standardization, and revisiting contest choice name standardization

both-standardizations.mov

Completion of contest name standardization surfacing contest choice name standardization

both-standardizations-2.mov

Side-by-side of contest name and contest choice name standardization dialogs

contest-names contest-choice-names

Testing

  • Updated existing tests
  • Manually tested e2e happy path
  • Manually tested interplay between contest name and contest choice name standardization
  • Manually tested switching back and forth between including choice names in standardized contests file and not

Punted on new tests in this PR as it's large enough as is, and I wanted to get this reviewed while I work on tests in a follow-up PR. Also wanted to get Jonah's eyes on this core logic today, whereas I can have someone else review tests tomorrow when he's out.

@arsalansufi arsalansufi force-pushed the arsalan/choice-name-standardization branch from e4010af to fbcb47e Compare June 20, 2024 16:21
queryClient.invalidateQueries(
contestChoiceNameStandardizationsQueryKey(electionId)
)
queryClient.invalidateQueries(contestsQueryKey(electionId))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Invalidating the contests query is necessary because it's used to populate this table on the audit setup review screen. I add this invalidation in a few places in this commit.

table

Choice names do not match across jurisdictions. Below is an example of a
mismatch. Address these inconsistencies by adding choice names to your
standardized contests file or updating your CVR files.
</p>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In addition to pointing out the new resolution path, tweaked the copy to make it clearer that the example pointed out by this warning may be one of many mismatches and isn't the only mismatch.

!(
isContestNameStandardizationNeeded &&
isContestNameStandardizationOutstanding
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This block above is all existing code, just moved down and with variable names renamed (standardizations ➡️ contestNameStandardizations)

Some contest names in the CVR files do not match the
target/opportunistic contest names.
Some contest names in the uploaded CVR files do not match the
standardized contest names.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some more copy cleanup for clarity

standardizedContestChoiceNames,
updateStandardizations,
}) => {
const [formState, setFormState] = useState(standardizations)
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 considered using react-hook-form but chose not to for a few reasons:

  1. Our field identifiers can have dots in them (because choice names can have dots, e.g., "Joe Jr.") which can throw off how react-hook-form accesses state. There are workarounds, but they're a bit convoluted.
  2. This form is ultimately quite simple, and I found it easier to reason about manually than with a library

@@ -11,7 +11,7 @@ module.exports = {
// to be uncovered. Ideally, we can get this to 0 eventually, but this
// accounts for legacy uncovered code. All new code should be covered (so
// this number should only be getting closer to 0).
global: { branches: -176 },
global: { branches: -191 },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As noted in the PR description:

Punted on new tests in this PR as it's large enough as is, and I wanted to get this reviewed while I work on tests in a follow-up PR. Also wanted to get Jonah's eyes on this core logic today, whereas I can have someone else review tests tomorrow when he's out.

@arsalansufi arsalansufi requested a review from jonahkagan June 20, 2024 16:32
@arsalansufi arsalansufi marked this pull request as ready for review June 20, 2024 16:32
Copy link
Contributor

@jonahkagan jonahkagan left a comment

Choose a reason for hiding this comment

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

Looks great overall! Left a number of minor comments and one blocking comment, but approving since there are no major changes needed IMO

server/api/standardized_contests.py Outdated Show resolved Hide resolved
server/api/standardized_contests.py Outdated Show resolved Hide resolved
server/api/contests.py Outdated Show resolved Hide resolved
server/api/contests.py Outdated Show resolved Hide resolved
server/api/cvrs.py Outdated Show resolved Hide resolved
server/api/cvrs.py Outdated Show resolved Hide resolved
client/src/components/useContestNameStandardizations.ts Outdated Show resolved Hide resolved
client/src/components/AuditAdmin/Setup/Review/Review.tsx Outdated Show resolved Hide resolved
@arsalansufi arsalansufi force-pushed the arsalan/choice-name-standardization branch from fbcb47e to 17d0016 Compare June 22, 2024 20:43
@arsalansufi arsalansufi force-pushed the arsalan/choice-name-standardization branch 2 times, most recently from 77dc399 to 9328bc4 Compare June 23, 2024 23:33
@arsalansufi arsalansufi force-pushed the arsalan/choice-name-standardization branch from 9328bc4 to 8556ddd Compare June 24, 2024 00:03
@arsalansufi arsalansufi merged commit cae5e84 into main Jun 24, 2024
5 checks passed
@arsalansufi arsalansufi deleted the arsalan/choice-name-standardization branch June 24, 2024 02:19
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