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

Initial validation highlighting #618

Merged
merged 9 commits into from
Aug 18, 2022
Merged

Conversation

HEdingfield
Copy link
Contributor

@HEdingfield HEdingfield commented Aug 17, 2022

  • Adds validation highlighting to the GUI when clicking the "add" buttons for candidates and CVRs.
  • Changes ContestConfig.validate() and associated methods to return a set of validation errors instead of an isValid boolean.
  • Fixes bug where treatBlankAsUndeclaredWriteIn validation failure for certain providers wouldn't actually fail validation.
  • Moves validation of provided overvote delimiter and overvote label into performBasicCvrSourceValidation so user is alerted when clicking the "add" button in the CVR tab.

First pass as outlined in #137 (comment).

… set of validation errors instead of an `isValid` boolean; fixes bug where `treatBlankAsUndeclaredWriteIn` validation failure for certain providers wouldn't fail validation.
…to `performBasicCvrSourceValidation` so user is alerted when clicking the "add" button in the CVR tab.
Copy link
Contributor

@tarheel tarheel left a comment

Choose a reason for hiding this comment

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

This is great! Could you post a screenshot or two?

src/main/java/network/brightspots/rcv/ContestConfig.java Outdated Show resolved Hide resolved
src/main/java/network/brightspots/rcv/ContestConfig.java Outdated Show resolved Hide resolved
src/main/java/network/brightspots/rcv/ContestConfig.java Outdated Show resolved Hide resolved
src/main/java/network/brightspots/rcv/ContestConfig.java Outdated Show resolved Hide resolved
} else {
candidateCodeSet.add(candidate.getCode());
}
}
}

if (candidateCodeSet.size() > 0 && candidateCodeSet.size() != candidateNameSet.size()) {
isValid = false;
validationErrors.add(ValidationError.CANDIDATE_DUPLICATE_CODES);
Logger.severe("If candidate codes are used, a unique code is required for each candidate!");
}

if (getNumDeclaredCandidates() < 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: s/< 1/== 0/

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 kind of in favor of keeping it this way in case some magic cosmic ray hits a computer and it results in a negative value.

src/main/java/network/brightspots/rcv/Main.java Outdated Show resolved Hide resolved
# Conflicts:
#	src/main/java/network/brightspots/rcv/ContestConfig.java
#	src/main/java/network/brightspots/rcv/TabulatorSession.java
Copy link
Contributor Author

@HEdingfield HEdingfield left a comment

Choose a reason for hiding this comment

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

Great review, thank you!

src/main/java/network/brightspots/rcv/Main.java Outdated Show resolved Hide resolved
src/main/java/network/brightspots/rcv/ContestConfig.java Outdated Show resolved Hide resolved
src/main/java/network/brightspots/rcv/ContestConfig.java Outdated Show resolved Hide resolved
src/main/java/network/brightspots/rcv/ContestConfig.java Outdated Show resolved Hide resolved
src/main/java/network/brightspots/rcv/ContestConfig.java Outdated Show resolved Hide resolved
src/main/java/network/brightspots/rcv/ContestConfig.java Outdated Show resolved Hide resolved
@HEdingfield
Copy link
Contributor Author

Screenshot here:
Capture

@HEdingfield HEdingfield merged commit 31f701e into develop Aug 18, 2022
@HEdingfield HEdingfield deleted the validation-highlighting branch August 18, 2022 00:20
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