Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements soft delete functionality for elections by adding a deleted_p flag and deleted_at timestamp. Soft-deleted elections are hidden from default queries but remain accessible to administrators via direct UUID or short name lookups.
Changes:
- Added soft delete fields (deleted_p, deleted_at) to the Election model with custom manager to filter deleted elections by default
- Implemented soft_delete() and undelete() methods on the Election model with logging support
- Added a new delete view endpoint with POST/CSRF protection for deleting/undeleting elections
- Added comprehensive test coverage for the soft delete functionality
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| helios/models.py | Added ElectionManager custom manager, deleted_p and deleted_at fields, soft_delete()/undelete() methods, updated get_by_uuid() and get_by_short_name() to use objects_with_deleted, added DELETED/UNDELETED log constants |
| helios/views.py | Added one_election_delete() view with POST requirement and CSRF protection to soft delete/undelete elections |
| helios/tests.py | Added test_soft_delete() model test and ElectionDeleteViewTests class with comprehensive view tests |
| helios/migrations/0008_add_election_soft_delete.py | Migration to add deleted_p and deleted_at fields to Election model |
| helios/election_urls.py | Added route for /delete endpoint |
| helios/election_url_names.py | Added ELECTION_DELETE URL name constant |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
helios/views.py
Outdated
| if request.method == "POST": | ||
| check_csrf(request) | ||
|
|
||
| delete_p = request.POST.get('delete_p', '1') | ||
|
|
||
| if bool(int(delete_p)): | ||
| election.soft_delete() | ||
| else: | ||
| election.undelete() | ||
|
|
There was a problem hiding this comment.
The delete endpoint only performs the delete/undelete action when the request method is POST, but it always redirects to the election view page regardless of the request method. For GET requests, no action is taken but the user is still redirected. This behavior differs from typical patterns where GET requests without actions might return an error or show a confirmation page. Consider either requiring POST or handling GET requests explicitly (e.g., showing an error or confirmation page).
| if request.method == "POST": | |
| check_csrf(request) | |
| delete_p = request.POST.get('delete_p', '1') | |
| if bool(int(delete_p)): | |
| election.soft_delete() | |
| else: | |
| election.undelete() | |
| if request.method != "POST": | |
| return HttpResponseBadRequest("Delete/undelete election requires POST.") | |
| check_csrf(request) | |
| delete_p = request.POST.get('delete_p', '1') | |
| if bool(int(delete_p)): | |
| election.soft_delete() | |
| else: | |
| election.undelete() |
Implements soft delete for elections using a deleted_p flag and deleted_at timestamp. Elections marked as deleted are hidden from default queries but can still be accessed directly by admins via UUID or short name. Key changes: - Added deleted_p (boolean) and deleted_at (datetime) fields to Election model - Created ElectionManager to filter out deleted elections by default - Added objects_with_deleted manager for accessing all elections including deleted - Added soft_delete() and undelete() methods to Election model - Added is_deleted property for checking deletion status - Updated get_by_uuid and get_by_short_name to use objects_with_deleted - Added /delete endpoint with one_election_delete view - Added ELECTION_DELETE URL name - Added DELETED and UNDELETED log constants - Created migration 0008_add_election_soft_delete - Added comprehensive test coverage for soft delete functionality The system now behaves as if deleted elections don't exist in listings and queries, but admins can still access them directly for management or restoration purposes.
Changed the one_election_delete endpoint to properly use POST requests with CSRF protection instead of GET requests, following proper web design principles for state-changing operations. Changes: - Updated one_election_delete view to check for POST method - Added check_csrf(request) call for CSRF protection - Changed to read delete_p from request.POST instead of request.GET - Added comprehensive view tests in ElectionDeleteViewTests: - test_delete_requires_post: verifies GET doesn't delete - test_delete_with_post: tests soft delete via POST - test_undelete_with_post: tests undelete via POST - test_delete_requires_admin: verifies permission checks The endpoint now properly requires POST requests for all delete/undelete operations, preventing accidental deletions from GET requests or CSRF attacks.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Removed the deleted_p boolean flag and now use only the deleted_at timestamp field for soft delete functionality. This is cleaner and follows the pattern already established in the codebase (e.g., archived_at, frozen_at). Changes: - Removed deleted_p field from Election model - Updated ElectionManager to filter by deleted_at__isnull=True - Updated is_deleted property to check deleted_at is not None - Updated soft_delete() to only set deleted_at - Updated undelete() to only clear deleted_at - Updated migration to only add deleted_at field Semantics: - deleted_at = null means election is not deleted - deleted_at = timestamp means election was deleted at that time
Verifies that Election.get_by_user_as_admin() properly excludes soft-deleted elections from the admin dashboard. Tests both the original admin/creator and additional admins to ensure deleted elections are hidden from all admin users.
Added @require_http_methods(['POST']) decorator to one_election_delete view to enforce POST-only access at the framework level. This is cleaner than checking request.method inside the view. Changes: - Added require_http_methods import to top of views.py - Added @require_http_methods(['POST']) decorator to one_election_delete - Removed if request.method == 'POST' check from view body - Removed test_delete_requires_post test (now enforced by framework) GET requests to the delete endpoint will now return 405 Method Not Allowed automatically, making the endpoint more secure and explicit about its allowed methods.
c86a95f to
567a1cc
Compare
Added delete and undelete buttons in the election view template next to the archive button. The buttons use POST forms with CSRF protection. Changes: - Show [deleted] status indicator when election is soft-deleted - Add 'delete it' button with confirmation dialog for non-deleted elections - Add 'undelete it' button for deleted elections to restore them - Both buttons use POST forms with proper CSRF tokens The delete button confirms with user before submitting to prevent accidental deletions.
Changed the confirmation message to clarify that deleted elections remain visible to administrators but are hidden from everyone else. Before: 'It will be hidden but can be restored later.' After: 'It will still be visible to you and other administrators, but not to anyone else.'
Two key improvements to soft delete functionality:
1. Button logic changes in UI (election_view.html):
- Delete button only shown when election is archived
- Archive/unarchive buttons hidden when election is deleted
- Only show undelete button when election is deleted
2. Access control changes (security.py):
- Added check in election_view decorator to raise Http404 for deleted
elections when accessed by non-admin users
- Admins can still access deleted elections directly by URL
- Non-admins get 404 as if election doesn't exist
Tests added:
- test_deleted_election_not_accessible_to_non_admins: verifies 404
- test_deleted_election_accessible_to_admins: verifies admin access
This ensures deleted elections are truly invisible to regular users while
remaining accessible to administrators for management purposes.
Background tasks that fetch elections by ID now use objects_with_deleted instead of the default objects manager. This prevents tasks from failing with DoesNotExist errors if an election is soft-deleted while tasks are queued or running. Fixed in these tasks: - voters_email - voters_notify - election_compute_tally - tally_helios_decrypt - notify_admin_opted_out_voters - election_notify_admin Tasks should continue to operate on deleted elections since they were queued before deletion and represent legitimate work that should complete.
Changed get_by_uuid() and get_by_short_name() to respect the default manager's deleted filter instead of always bypassing it. Changes: - Added include_deleted parameter to get_by_uuid() and get_by_short_name() - By default (include_deleted=False), these methods use the default manager which filters out deleted elections - When include_deleted=True, they use objects_with_deleted manager - Updated election_view decorator to try default manager first, then retry with include_deleted=True if election not found and user is admin - Updated election_admin decorator to always use include_deleted=True since admins should be able to manage deleted elections - Updated trustee_check decorator to use include_deleted=True since trustees need access for decryption even if election is deleted - Updated tests to verify get_by_uuid respects the filter This properly follows the manager abstraction pattern: the default behavior excludes deleted elections, and callers must explicitly request them when needed.
Changed soft delete behavior so deleted elections are truly invisible to everyone except site administrators (who will access them through a special admin interface to be built later). Changes to election_view.html: - Removed [deleted] status indicator - Removed undelete button - Changed delete confirmation message to clarify only site admins can restore Changes to security.py: - election_view: No longer tries to fetch deleted elections - election_admin: No longer includes deleted elections - trustee_check: No longer includes deleted elections - Deleted elections return 404 for everyone except site admins Changes to views.py: - one_election_delete: Simplified to only handle deletion - Removed undelete functionality - After deletion, redirects to /helios/ since election is now invisible Changes to tests.py: - Removed test_undelete_with_post - Removed test_deleted_election_accessible_to_admins - Added test_deleted_election_not_accessible_to_election_admins - Verifies that even election admins get 404 for deleted elections Deleted elections now require site admin access through a future admin interface for restoration or management.
This commit adds a dedicated admin interface for site administrators to view, search, and restore soft-deleted elections. The interface includes: - New stats view for listing deleted elections with pagination - Search functionality to filter deleted elections by name - Individual undelete capability for restoring elections - Link to deleted elections page in main admin menu - Template for displaying deleted elections with relevant details Only users with admin_p=True can access these views.
Reverted background tasks to use standard Election.objects instead of objects_with_deleted. Deleted elections should only be accessible through the dedicated site admin interface, not through background tasks. Tasks will now fail gracefully if they attempt to access a deleted election, which is the correct behavior - deleted elections should be truly hidden.
Updated the undelete functionality to require POST with CSRF protection and added a confirmation dialog before restoring a deleted election. Changes: - Added @require_http_methods(["POST"]) decorator to undelete_election view - Added check_csrf(request) to verify CSRF token - Changed undelete button from link to form with confirmation dialog - Confirmation message: "Are you sure you want to restore this election? It will become visible to its administrators again."
fixes #161 by implementing soft delete.
At some point we'll garbage collect the soft-deleted elections.