Fix issue 455 – don't allow voter uploads when election tallying has begun#461
Merged
Fix issue 455 – don't allow voter uploads when election tallying has begun#461
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds safeguards to prevent voter file uploads after an election's tallying process has begun. The implementation adds a new can_add_voters_file() method to the Election model that checks whether uploads should be blocked based on the election's tallying state.
Changes:
- Added
can_add_voters_file()method to Election model that returns False when encrypted_tally or tallying_started_at is set - Updated voters_upload view to check permissions and raise PermissionDenied when uploads are blocked
- Modified voters_list template to show a disabled button with explanation when uploads are not allowed
- Added comprehensive test suite covering the new functionality
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| helios/models.py | Added can_add_voters_file() method that checks if voter uploads should be blocked based on tallying state |
| helios/views.py | Updated voters_list_pretty to pass upload permission status to template; updated voters_upload to enforce permission check |
| helios/templates/voters_list.html | Modified to conditionally render upload button as disabled with explanation text when uploads are blocked |
| helios/tests.py | Added VoterUploadRestrictionTests class with 5 test cases covering the new functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+2825
to
+2962
| class VoterUploadRestrictionTests(WebTest): | ||
| """Tests for voter upload restrictions when election is tallied (issue #455)""" | ||
| fixtures = ['users.json'] | ||
| allow_database_queries = True | ||
|
|
||
| def setUp(self): | ||
| self.user = auth_models.User.objects.get(user_id='ben@adida.net', user_type='google') | ||
|
|
||
| def setup_login(self): | ||
| """Set up admin login session""" | ||
| self.client.get("/") | ||
| session = self.client.session | ||
| session['user'] = {'type': self.user.user_type, 'user_id': self.user.user_id} | ||
| session.save() | ||
| self.client.get("/helios/elections/new") | ||
|
|
||
| def create_election(self, short_name): | ||
| """Create a basic election and return its ID""" | ||
| response = self.client.post("/helios/elections/new", { | ||
| "short_name": short_name, | ||
| "name": "Test Election", | ||
| "description": "Test election for voter upload restrictions", | ||
| "election_type": "referendum", | ||
| "use_voter_aliases": "0", | ||
| "use_advanced_audit_features": "1", | ||
| "private_p": "True", | ||
| "csrf_token": self.client.session["csrf_token"] | ||
| }) | ||
| self.assertRedirects(response) | ||
| election_id = re.search('/elections/([^/]+)/', str(response['location'])).group(1) | ||
| return election_id | ||
|
|
||
| def test_can_add_voters_file_returns_true_by_default(self): | ||
| """Test that can_add_voters_file returns True for a fresh election""" | ||
| self.setup_login() | ||
| election_id = self.create_election("test-upload-default") | ||
| election = models.Election.objects.get(uuid=election_id) | ||
|
|
||
| can_add, reason = election.can_add_voters_file() | ||
| self.assertTrue(can_add) | ||
| self.assertIsNone(reason) | ||
|
|
||
| def test_can_add_voters_file_returns_false_when_tallying_started(self): | ||
| """Test that can_add_voters_file returns False when tallying has started""" | ||
| self.setup_login() | ||
| election_id = self.create_election("test-upload-tallying-started") | ||
| election = models.Election.objects.get(uuid=election_id) | ||
|
|
||
| election.tallying_started_at = datetime.datetime.utcnow() | ||
| election.save() | ||
|
|
||
| can_add, reason = election.can_add_voters_file() | ||
| self.assertFalse(can_add) | ||
| self.assertEqual(reason, "Tallying has started") | ||
|
|
||
| def test_voter_upload_blocked_when_tallying_started(self): | ||
| """Test that voter upload view is blocked when tallying has started""" | ||
| self.setup_login() | ||
| election_id = self.create_election("test-upload-blocked-tallying") | ||
| election = models.Election.objects.get(uuid=election_id) | ||
|
|
||
| # First, verify upload works before tallying starts | ||
| with open("helios/fixtures/voter-file.csv") as f: | ||
| response = self.client.post( | ||
| "/helios/elections/%s/voters/upload" % election_id, | ||
| {"voters_file": f} | ||
| ) | ||
| self.assertContains(response, "first few rows") | ||
|
|
||
| # Confirm the upload | ||
| response = self.client.post( | ||
| "/helios/elections/%s/voters/upload" % election_id, | ||
| {"confirm_p": "1"} | ||
| ) | ||
| self.assertRedirects(response, "/helios/elections/%s/voters/list" % election_id) | ||
|
|
||
| # Now start tallying | ||
| election.refresh_from_db() | ||
| election.tallying_started_at = datetime.datetime.utcnow() | ||
| election.save() | ||
|
|
||
| # Try to upload more voters - should be blocked with 403 | ||
| with open("helios/fixtures/voter-file-2.csv") as f: | ||
| response = self.client.post( | ||
| "/helios/elections/%s/voters/upload" % election_id, | ||
| {"voters_file": f} | ||
| ) | ||
| self.assertStatusCode(response, 403) | ||
|
|
||
| # Also verify GET request is blocked | ||
| response = self.client.get("/helios/elections/%s/voters/upload" % election_id) | ||
| self.assertStatusCode(response, 403) | ||
|
|
||
| def test_voter_upload_allowed_when_open(self): | ||
| """Test that voter upload works normally when election is still open""" | ||
| self.setup_login() | ||
| election_id = self.create_election("test-upload-open") | ||
|
|
||
| # Verify upload works | ||
| with open("helios/fixtures/voter-file.csv") as f: | ||
| response = self.client.post( | ||
| "/helios/elections/%s/voters/upload" % election_id, | ||
| {"voters_file": f} | ||
| ) | ||
| self.assertContains(response, "first few rows") | ||
|
|
||
| # Confirm the upload | ||
| response = self.client.post( | ||
| "/helios/elections/%s/voters/upload" % election_id, | ||
| {"confirm_p": "1"} | ||
| ) | ||
| self.assertRedirects(response, "/helios/elections/%s/voters/list" % election_id) | ||
|
|
||
| # Verify voters were added | ||
| election = models.Election.objects.get(uuid=election_id) | ||
| self.assertEqual(election.voter_set.count(), 4) # voter-file.csv has 4 voters | ||
|
|
||
| def test_voters_list_shows_disabled_button_when_tallied(self): | ||
| """Test that voters list shows disabled upload button when election is tallied""" | ||
| self.setup_login() | ||
| election_id = self.create_election("test-upload-button-disabled") | ||
| election = models.Election.objects.get(uuid=election_id) | ||
|
|
||
| # Check the voters list page - button should be enabled | ||
| response = self.client.get("/helios/elections/%s/voters/list" % election_id) | ||
| self.assertStatusCode(response, 200) | ||
| self.assertContains(response, 'bulk upload voters</a>') | ||
|
|
||
| # Now start tallying | ||
| election.tallying_started_at = datetime.datetime.utcnow() | ||
| election.save() | ||
|
|
||
| # Check the voters list page - button should be disabled | ||
| response = self.client.get("/helios/elections/%s/voters/list" % election_id) | ||
| self.assertStatusCode(response, 200) | ||
| self.assertContains(response, 'disabled') | ||
| self.assertContains(response, 'Tallying has started') | ||
|
|
There was a problem hiding this comment.
The test suite covers the tallying_started_at condition but lacks coverage for the encrypted_tally condition. Since the model's can_add_voters_file() method checks both conditions, there should be a test that verifies voter uploads are blocked when encrypted_tally is set. Consider adding a test case that sets election.encrypted_tally to a non-null value and verifies that uploads are blocked with the "Election has been tallied" reason.
Add a modular `can_add_voters_file()` method to the Election model that returns whether voter file uploads are allowed. Uploads are blocked when: - encrypted tally exists (encrypted_tally is set) - tallying has started (tallying_started_at is set) The method is used in both the view (to raise PermissionDenied) and the template (to show a disabled button with explanation). Note: We intentionally do NOT block on voting_ended_at because the admin can extend voting, so ending voting alone doesn't mean the election is truly closed.
benadida
force-pushed
the
claude/fix-issue-455-iB4BT
branch
from
62a1b4e to
aacc059
Compare
This file contains hidden or 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
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.
Add a modular
can_add_voters_file()method to the Election model that returns whether voter file uploads are allowed. Uploads are blocked when:The method is used in both the view (to raise PermissionDenied) and the template (to show a disabled button with explanation).
Note: We intentionally do NOT block on voting_ended_at because the admin can extend voting, so ending voting alone doesn't mean the election is truly closed.
fixes #455