From 04e52b874ef4495a1bc91b222a731d7a033b0d70 Mon Sep 17 00:00:00 2001 From: Jonah Kagan Date: Tue, 12 Nov 2024 10:54:15 -0800 Subject: [PATCH 1/2] Improve error message when using batch inventory but missing container Follow up to https://github.com/votingworks/arlo/pull/2011 to also show a similar error message when the Container column is missing, not only when it contains an invalid value. --- server/api/ballot_manifest.py | 24 +++++++------- ..._sample_extra_batches_by_counting_group.py | 31 +++++++++++++++++++ 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/server/api/ballot_manifest.py b/server/api/ballot_manifest.py index fdca19358..aa40d9102 100644 --- a/server/api/ballot_manifest.py +++ b/server/api/ballot_manifest.py @@ -125,11 +125,7 @@ def process() -> None: ) columns = [ - CSVColumnType( - CONTAINER, - CSVValueType.TEXT, - required_column=is_counting_group_required, - ), + CSVColumnType(CONTAINER, CSVValueType.TEXT, required_column=False), CSVColumnType( TABULATOR, CSVValueType.TEXT, @@ -158,14 +154,16 @@ def process() -> None: num_ballots = 0 for row_index, row in enumerate(manifest_csv): - counting_group = row.get(CONTAINER, None) - if ( - is_counting_group_required - and not counting_group in counting_group_allowset - ): - raise CSVParseError( - f"Invalid value for column \"Container\", row {row_index+2}: \"{counting_group}\". Use the Batch Audit File Preparation Tool to create your ballot manifest, or correct this value to one of the following: {', '.join(counting_group_allowlist)}." - ) + if is_counting_group_required: + if not CONTAINER in row: + raise CSVParseError( + 'Missing required column "Container". Use the Batch Audit File Preparation Tool to create your ballot manifest.' + ) + counting_group = row.get(CONTAINER) + if counting_group not in counting_group_allowset: + raise CSVParseError( + f"Invalid value for column \"Container\", row {row_index+2}: \"{counting_group}\". Use the Batch Audit File Preparation Tool to create your ballot manifest, or correct this value to one of the following: {', '.join(counting_group_allowlist)}." + ) batch = Batch( id=str(uuid.uuid4()), diff --git a/server/tests/batch_comparison/test_sample_extra_batches_by_counting_group.py b/server/tests/batch_comparison/test_sample_extra_batches_by_counting_group.py index 309a11896..3faf8c741 100644 --- a/server/tests/batch_comparison/test_sample_extra_batches_by_counting_group.py +++ b/server/tests/batch_comparison/test_sample_extra_batches_by_counting_group.py @@ -567,3 +567,34 @@ def test_sample_extra_batches_with_invalid_counting_group( }, }, ) + + # Missing the "Container" column + set_logged_in_user( + client, UserType.JURISDICTION_ADMIN, default_ja_email(election_id) + ) + rv = upload_ballot_manifest( + client, + io.BytesIO(b"Batch Name,Number of Ballots\n" b"Batch 1,500\n"), + election_id, + jurisdiction_ids[0], + ) + assert_ok(rv) + + rv = client.get( + f"/api/election/{election_id}/jurisdiction/{jurisdiction_ids[0]}/ballot-manifest" + ) + compare_json( + json.loads(rv.data), + { + "file": { + "name": asserts_startswith("manifest"), + "uploadedAt": assert_is_date, + }, + "processing": { + "status": ProcessingStatus.ERRORED, + "startedAt": assert_is_date, + "completedAt": assert_is_date, + "error": 'Missing required column "Container". Use the Batch Audit File Preparation Tool to create your ballot manifest.', + }, + }, + ) From 3473e3ba3b2012648b01ab2e1380ea0c61a0035d Mon Sep 17 00:00:00 2001 From: Jonah Kagan Date: Tue, 12 Nov 2024 11:11:04 -0800 Subject: [PATCH 2/2] Add explanatory comments and refactor for clarity --- server/api/ballot_manifest.py | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/server/api/ballot_manifest.py b/server/api/ballot_manifest.py index aa40d9102..1b35dd9ac 100644 --- a/server/api/ballot_manifest.py +++ b/server/api/ballot_manifest.py @@ -120,12 +120,14 @@ def process() -> None: AuditType.HYBRID, ] - is_counting_group_required = is_enabled_sample_extra_batches_by_counting_group( - jurisdiction.election - ) - columns = [ - CSVColumnType(CONTAINER, CSVValueType.TEXT, required_column=False), + CSVColumnType( + CONTAINER, + CSVValueType.TEXT, + # Optionally, users can include a "Container" column to help + # identify ballots in different storage locations. + required_column=False, + ), CSVColumnType( TABULATOR, CSVValueType.TEXT, @@ -147,6 +149,9 @@ def process() -> None: validate_is_not_batch_inventory_worksheet(manifest_file) manifest_csv = parse_csv(manifest_file, columns) + is_counting_group_required = is_enabled_sample_extra_batches_by_counting_group( + jurisdiction.election + ) counting_group_allowlist = [item.value for item in CountingGroup] counting_group_allowset = set(counting_group_allowlist) @@ -154,8 +159,13 @@ def process() -> None: num_ballots = 0 for row_index, row in enumerate(manifest_csv): + # For the "sample extra batches by counting group" feature, we + # require the Container column to identify the counting group, so we + # validate it here. We want to provide special custom error + # messages, so we don't use the built-in "required_column" option in + # CSVColumnType. if is_counting_group_required: - if not CONTAINER in row: + if CONTAINER not in row: raise CSVParseError( 'Missing required column "Container". Use the Batch Audit File Preparation Tool to create your ballot manifest.' )