Fix issue 470

helios/models.py

@@ -443,13 +443,13 @@ def can_send_voter_emails(self):
 
     return (True, None)
 
-  def can_add_voters_file(self):
+  def can_modify_voters(self):
     """
-    Check if voter file uploads are allowed for this election.
-    Returns a tuple: (can_add: bool, reason: str|None)
+    Check if voter modifications (uploads, deletions) are allowed for this election.
+    Returns a tuple: (allowed: bool, reason: str|None)
 
-    Voter uploads are blocked once tallying has started,
-    as adding voters after vote counting would compromise election integrity.
+    Voter modifications are blocked once tallying has started,
+    as changing voters after vote counting would compromise election integrity.
     """
     if self.encrypted_tally:
       return (False, "Election has been tallied")

helios/templates/voters_list.html

@@ -64,10 +64,10 @@ <h3 class="title">{{election.name}} &mdash; Voters and Ballot Tracking Center <s
 <!-- Add a Voter: WORK HERE-->
 {% if upload_p and not election.openreg %}
 <p>
-{% if can_add_voters_file %}
+{% if can_modify_voters %}
 <a class="button" href="{% url "election@voters@upload" election_uuid=election.uuid %}">bulk upload voters</a>
 {% else %}
-<button class="button" disabled style="opacity: 0.5; cursor: not-allowed;" title="{{upload_disabled_reason}}">bulk upload voters (disabled: {{upload_disabled_reason}})</button>
+<button class="button" disabled style="opacity: 0.5; cursor: not-allowed;" title="{{modify_voters_disabled_reason}}">bulk upload voters (disabled: {{modify_voters_disabled_reason}})</button>
 {% endif %}
 </p>
 
@@ -161,7 +161,7 @@ <h3 class="title">{{election.name}} &mdash; Voters and Ballot Tracking Center <s
 [<span style="opacity: 0.5; cursor: not-allowed;" title="{{email_disabled_reason}}">email</span>]
 {% endif %}
 {% endif %}
-[<a onclick="return confirm('are you sure you want to remove {{voter.name}} ?');" href="{% url "election@voter@delete" election.uuid voter.uuid %}">x</a>]
+{% if can_modify_voters %}[<a onclick="return confirm('are you sure you want to remove {{voter.name}} ?');" href="{% url "election@voter@delete" election.uuid voter.uuid %}">x</a>]{% endif %}
 </td>
 <td>{{voter.voter_login_id}}</td>
 <td>{{voter.voter_email}}</td>

helios/tests.py

@@ -2845,25 +2845,25 @@ def setup_login(self):
         session['user'] = {'type': self.user.user_type, 'user_id': self.user.user_id}
         session.save()
 
-    def test_can_add_voters_file_allowed_by_default(self):
-        can_add, reason = self.election.can_add_voters_file()
-        self.assertTrue(can_add)
+    def test_can_modify_voters_allowed_by_default(self):
+        can_modify, reason = self.election.can_modify_voters()
+        self.assertTrue(can_modify)
         self.assertIsNone(reason)
 
-    def test_can_add_voters_file_blocked_when_encrypted_tally_exists(self):
+    def test_can_modify_voters_blocked_when_encrypted_tally_exists(self):
         # Set in memory only - the method just checks truthiness
         self.election.encrypted_tally = True
 
-        can_add, reason = self.election.can_add_voters_file()
-        self.assertFalse(can_add)
+        can_modify, reason = self.election.can_modify_voters()
+        self.assertFalse(can_modify)
         self.assertEqual(reason, "Election has been tallied")
 
-    def test_can_add_voters_file_blocked_when_tallying_started(self):
+    def test_can_modify_voters_blocked_when_tallying_started(self):
         self.election.tallying_started_at = datetime.datetime.utcnow()
         self.election.save()
 
-        can_add, reason = self.election.can_add_voters_file()
-        self.assertFalse(can_add)
+        can_modify, reason = self.election.can_modify_voters()
+        self.assertFalse(can_modify)
         self.assertEqual(reason, "Tallying has started")
 
     def test_voter_upload_view_returns_403_when_blocked(self):
@@ -2884,3 +2884,72 @@ def test_voters_list_shows_disabled_button_when_blocked(self):
         self.assertContains(response, 'disabled')
         self.assertContains(response, 'Tallying has started')
 
+
+class VoterDeleteRestrictionTests(WebTest):
+    """Tests for voter deletion restrictions when tallying has begun (issue #470)"""
+    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')
+        self.election, _ = models.Election.get_or_create(
+            short_name='test-delete-restriction',
+            name='Test Delete Restriction',
+            description='Test',
+            admin=self.user
+        )
+        if not self.election.uuid:
+            self.election.uuid = str(uuid.uuid4())
+            self.election.save()
+        # Create a voter to test deletion
+        self.voter = models.Voter.objects.create(
+            uuid=str(uuid.uuid4()),
+            election=self.election,
+            voter_email='voter@test.com',
+            voter_name='Test Voter'
+        )
+
+    def setup_login(self):
+        self.client.get("/")
+        session = self.client.session
+        session['user'] = {'type': self.user.user_type, 'user_id': self.user.user_id}
+        session.save()
+
+    def test_voter_delete_allowed_by_default(self):
+        """Voter deletion should be allowed before tallying starts"""
+        self.setup_login()
+        response = self.client.post("/helios/elections/%s/voters/%s/delete" % (
+            self.election.uuid, self.voter.uuid))
+        # Should redirect (302) on successful deletion
+        self.assertStatusCode(response, 302)
+
+    def test_voter_delete_blocked_when_tallying_started(self):
+        """Voter deletion should be blocked once tallying has started"""
+        self.setup_login()
+        self.election.tallying_started_at = datetime.datetime.utcnow()
+        self.election.save()
+
+        response = self.client.post("/helios/elections/%s/voters/%s/delete" % (
+            self.election.uuid, self.voter.uuid))
+        self.assertStatusCode(response, 403)
+
+    def test_voters_list_shows_delete_button_when_allowed(self):
+        """Voter list should show delete [x] button when deletion is allowed"""
+        self.setup_login()
+
+        response = self.client.get("/helios/elections/%s/voters/list" % self.election.uuid)
+        self.assertStatusCode(response, 200)
+        # Check for the delete link with [x] text
+        self.assertContains(response, '>x</a>]')
+
+    def test_voters_list_hides_delete_button_when_blocked(self):
+        """Voter list should hide delete [x] button when tallying has started"""
+        self.setup_login()
+        self.election.tallying_started_at = datetime.datetime.utcnow()
+        self.election.save()
+
+        response = self.client.get("/helios/elections/%s/voters/list" % self.election.uuid)
+        self.assertStatusCode(response, 200)
+        # Check that the delete link is not present
+        self.assertNotContains(response, '>x</a>]')
+

helios/views.py

@@ -1052,16 +1052,12 @@ def one_election_audited_ballots(request, election):
 @election_admin()
 def voter_delete(request, election, voter_uuid):
   """
-  Two conditions under which a voter can be deleted:
-  - election is not frozen or
-  - election is open reg
+  Voter deletion uses the same restrictions as voter file uploads:
+  blocked once tallying has started or election has been tallied,
+  as modifying voters after vote counting begins would compromise election integrity.
   """
-  ## FOR NOW we allow this to see if we can redefine the meaning of "closed reg" to be more flexible
-  # if election is frozen and has closed registration
-  #if election.frozen_at and (not election.openreg):
-  #  raise PermissionDenied()
-
-  if election.encrypted_tally:
+  can_delete, _ = election.can_modify_voters()
+  if not can_delete:
     raise PermissionDenied()
 
   voter = Voter.get_by_election_and_uuid(election, voter_uuid)
@@ -1457,8 +1453,8 @@ def voters_list_pretty(request, election):
   # Check if voter emails can be sent
   can_send_emails, email_disabled_reason = election.can_send_voter_emails()
 
-  # Check if voter uploads are allowed
-  can_add_voters_file, upload_disabled_reason = election.can_add_voters_file()
+  # Check if voter modifications (uploads, deletions) are allowed
+  can_modify_voters, modify_voters_disabled_reason = election.can_modify_voters()
 
   return render_template(request, 'voters_list',
                          {'election': election, 'voters_page': voters_page,
@@ -1468,8 +1464,8 @@ def voters_list_pretty(request, election):
                           'email_disabled_reason': email_disabled_reason,
                           'limit': limit, 'total_voters': total_voters,
                           'upload_p': VOTERS_UPLOAD,
-                          'can_add_voters_file': can_add_voters_file,
-                          'upload_disabled_reason': upload_disabled_reason,
+                          'can_modify_voters': can_modify_voters,
+                          'modify_voters_disabled_reason': modify_voters_disabled_reason,
                           'q' : q,
                           'voter_files': voter_files,
                           'categories': categories,
@@ -1625,7 +1621,7 @@ def voters_upload(request, election):
   """
 
   # don't allow voter upload when election is tallied
-  can_upload, reason = election.can_add_voters_file()
+  can_upload, reason = election.can_modify_voters()
   if not can_upload:
     raise PermissionDenied()