Improve error message when using batch inventory but missing container #2056
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Follow up to #2011 to also show a similar error message when the Container column is missing, not only when it contains an invalid value.
kshen0
approved these changes
Nov 12, 2024
server/api/ballot_manifest.py
Outdated
| CSVValueType.TEXT, | ||
| required_column=is_counting_group_required, | ||
| ), | ||
| CSVColumnType(CONTAINER, CSVValueType.TEXT, required_column=False), |
There was a problem hiding this comment.
It might be slightly confusing to readers that we indicate the column isn't required, but later on enforce it in L158. Thoughts on allowing an error message override to be specified in the CSVColumnType? I could see it being overkill, so if you agree, maybe just add a comment here on why we mark required_column=False and enforce with special logic later.
There was a problem hiding this comment.
Good point, I'll add a comment!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #2054
Follow up to #2011 to also show a similar error message when the Container column is missing, not only when it contains an invalid value.
The previous message was just
Missing required column "Container", which wasn't clear enough.