feat(backend): add volunteer application endpoints #18

pH-li · 2025-11-21T02:25:30Z

Summary

Implements the volunteer application backend flow:

Public endpoint for submitting volunteer applications (always created as PENDING).
Admins can get a list of all volunteer applications (currently via role-based placeholder).
Approval/rejection flow that records reviewer data
On approval, creates a User with role VOLUNTEER and temporary password

Related Issues

Closes Volunteer Application API #6

Changes

Serializer
- Updated [VolunteerApplicationSerializer]
  - Force new applications to status="PENDING" on create.
  - Expose reviewed_at and reviewed_by as read-only fields.
Views
- Extended [VolunteerApplicationAPIView]
  - POST /api/volunteer-applications/ for public submission.
  - GET /api/volunteer-applications/ now restricted using a placeholder admin check:
    - Admin ≈ authenticated user with role == "ADMIN".
  - PATCH /api/volunteer-applications/:id/ for approving or rejecting applications.
- Added helpers:
  - [_is_admin(user)], is a PLACEHOLDER role check for admins.
  - [_handle_review_metadata(application)], sets reviewed_at and reviewed_by on first review.
  - [_handle_volunteer_user_creation(application)], on APPROVED, creates a [User] with role VOLUNTEER and a temporary password (invite/email flow to be implemented later).
- Wrapped [perform_update] in @transaction.atomic so status change, review metadata, and user creation are applied atomically.
URLs
- Confirmed router registration for:
  - volunteer-applications → [VolunteerApplicationAPIView] (basename volunteer-application).
Tests
- [users/tests.py]
  - [test_create_user]: verifies basic user creation.
  - [test_approve_application_sets_review_fields_and_creates_volunteer_user]: PATCH to APPROVED:
    - Sets reviewed_at.
    - Creates a [User] with role="VOLUNTEER" for the application email.
  - [test_reject_application_sets_review_fields_and_does_not_create_user]: PATCH to REJECTED:
    - Sets reviewed_at.
    - Does not create a [User] (since application was rejected)

How to Test

Run the pytest:

cd backend
pytest users/tests.py

Checklist

Tests added or updated
CI passes (lint, tests, build)
Documentation updated (if behavior changed)
No secrets or credentials committed

…er admin login for getApps

Copilot

Pull request overview

This PR implements a volunteer application submission and review system, enabling public users to submit applications and admins to approve/reject them. When approved, the system automatically creates a volunteer user account. The implementation adds REST API endpoints for volunteer application CRUD operations, though several security and design issues need to be addressed before merging.

Key Changes

Added VolunteerApplicationAPIView (ModelViewSet) with endpoints for creating, listing, retrieving, and updating volunteer applications
Implemented automatic user account creation with role "VOLUNTEER" upon application approval
Added approval workflow tracking with reviewed_at and reviewed_by fields

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 16 comments.

Show a summary per file

File	Description
backend/users/views.py	Implements the volunteer application viewset with submission, listing (admin-only), and approval/rejection logic with automatic user creation
backend/users/serializers.py	Adds serializer for volunteer applications, forcing new applications to PENDING status
backend/users/urls.py	Registers the volunteer application endpoints with the router
backend/users/tests.py	Adds tests for application approval/rejection flows and user creation
backend/core/urls.py	Enables the users API routes by uncommenting the include statement

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-01T22:46:18Z

backend/users/urls.py

+# This will create the following endpoints:
+# POST   /api/volunteer-applications/          - Create a new application (PENDING)
+# GET    /api/volunteer-applications/          - List all applications
+# GET    /api/volunteer-applications/{id}/     - Retrieve a specific application (admin only)


The comment states "Retrieve a specific application (admin only)" but there's no admin-only restriction implemented for the retrieve endpoint. The current implementation only protects the list endpoint. Either update the comment to reflect the actual implementation or add the missing admin protection to the retrieve endpoint.

Suggested change

# GET /api/volunteer-applications/{id}/ - Retrieve a specific application (admin only)

# GET /api/volunteer-applications/{id}/ - Retrieve a specific application

Copilot · 2025-12-01T22:46:18Z

backend/users/views.py

+
+    queryset = VolunteerApplication.objects.all()
+    serializer_class = VolunteerApplicationSerializer
+    permission_classes = [permissions.AllowAny]


The AllowAny permission class is too permissive for a ModelViewSet. This allows unrestricted access to all CRUD operations (GET, POST, PUT, PATCH, DELETE) on all volunteer applications, including retrieving and modifying individual applications. Consider using DRF's get_permissions() method to return different permission classes based on the action (e.g., AllowAny for create, but require authentication/admin for list, retrieve, update, destroy).

Copilot · 2025-12-01T22:46:19Z

backend/users/tests.py

+
+
+@pytest.mark.django_db
+def test_approve_volunteer_application():


Missing test coverage for the public POST endpoint to create volunteer applications. Add a test that verifies: 1) Anyone can submit an application via POST, 2) The application is created with status="PENDING", and 3) The response includes the expected fields.

Copilot · 2025-12-01T22:46:19Z

backend/users/views.py

+    @transaction.atomic
+    def perform_update(self, serializer):
+        old_status = serializer.instance.status
+        application = serializer.save()
+
+        if old_status == "PENDING" and application.status in {"APPROVED", "REJECTED"}:
+            self._handle_review_metadata(application)
+            self._handle_volunteer_user_creation(application)


Using PATCH on the main resource for approval/rejection is not RESTful best practice. Consider implementing dedicated action endpoints (e.g., @action(detail=True, methods=['post']) def approve(...) and @action(detail=True, methods=['post']) def reject(...)) that would create endpoints like POST /api/volunteer-applications/{id}/approve/ and POST /api/volunteer-applications/{id}/reject/. This makes the API more explicit and allows for additional approval/rejection metadata (e.g., rejection reasons) in the request body.

Copilot · 2025-12-01T22:46:19Z

backend/users/views.py

+    def get_serializer_class(self):
+        """Return appropriate serializer based on action."""
+        if self.action == "create":
+            return VolunteerApplicationSerializer
+        return VolunteerApplicationSerializer  # Can update this later, just for formality
+


The get_serializer_class() method always returns the same serializer regardless of action, making it redundant. Either remove this method entirely (and rely on the serializer_class attribute) or add a meaningful distinction between actions if different serializers are needed in the future.

Suggested change

def get_serializer_class(self):

"""Return appropriate serializer based on action."""

if self.action == "create":

return VolunteerApplicationSerializer

return VolunteerApplicationSerializer # Can update this later, just for formality

backend/users/views.py

Copilot · 2025-12-01T22:46:21Z

backend/users/views.py

+        if user is not None and getattr(user, "is_authenticated", False) and application.reviewed_by is None:
+            application.reviewed_by = user
+
+        application.save()


The _handle_review_metadata() method calls application.save() which saves the application outside the serializer's save flow. Since this method is called after serializer.save() in perform_update(), it performs a second database write. This can cause issues with the transaction and the fields might not be properly updated through the serializer. Instead, set the fields before calling serializer.save() or pass them to the serializer's save method.

Copilot · 2025-12-01T22:46:21Z

backend/users/serializers.py

+    class Meta:
+        model = VolunteerApplication
+        fields = ["id", "name", "email", "motivation_text", "status", "created_at", "reviewed_at", "reviewed_by"]
+        read_only_fields = ["id", "created_at", "reviewed_at", "reviewed_by"]


The status field is exposed as writable in the serializer, allowing public users to submit applications with any status (including "APPROVED" or "REJECTED") by including the status in the POST request body. Even though the create() method forces status to "PENDING", the field should be added to read_only_fields to make the API contract clearer and prevent status manipulation on updates.

Suggested change

read_only_fields = ["id", "created_at", "reviewed_at", "reviewed_by"]

read_only_fields = ["id", "created_at", "reviewed_at", "reviewed_by", "status"]

Copilot · 2025-12-01T22:46:22Z

backend/users/tests.py

+
+
+@pytest.mark.django_db
+def test_approve_volunteer_application():


Missing test coverage for edge cases: 1) Approving an application when a user with the same email already exists (should not create duplicate), 2) Attempting to change status from APPROVED back to PENDING or REJECTED (should review metadata behavior), 3) Directly updating to APPROVED without going through PENDING first.

backend/users/tests.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 10 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-01T22:56:25Z

backend/core/settings.py

+# Secret key fallback for pytest on github
+if not SECRET_KEY:
+    SECRET_KEY = "test-secret-key"


Using a hardcoded SECRET_KEY as a fallback is a security risk. If the environment variable is not set in production (e.g., due to misconfiguration), Django will use this weak, publicly-visible key, making the application vulnerable to session hijacking, CSRF attacks, and other security issues.

Instead of a fallback, the application should fail explicitly when SECRET_KEY is not set:

if not SECRET_KEY: raise ValueError("SECRET_KEY environment variable must be set")

If a fallback is needed specifically for testing, use pytest configuration (e.g., conftest.py) to set it only in test environments.

Suggested change

# Secret key fallback for pytest on github

if not SECRET_KEY:

SECRET_KEY = "test-secret-key"

# Ensure SECRET_KEY is set

if not SECRET_KEY:

raise ValueError("SECRET_KEY environment variable must be set")

Copilot · 2025-12-01T22:56:26Z

backend/users/views.py

@@ -1,35 +1,80 @@
 from rest_framework import viewsets, permissions, status
 from rest_framework.decorators import action


The action decorator is imported but never used in this file. Remove this unused import to keep the code clean.

Suggested change

from rest_framework.decorators import action

Copilot · 2025-12-01T22:56:26Z

backend/users/views.py

+        application.save()
+


The application.save() call causes the application to be saved twice within the same transaction: once by serializer.save() (line 76) and again here. This is inefficient and could lead to race conditions or unexpected behavior.

Instead, modify the fields without calling save() and let the transaction commit handle the single save operation. The @transaction.atomic decorator will ensure all changes are committed together.

Suggested change

application.save()

Copilot · 2025-12-01T22:56:26Z

backend/users/tests.py

    assert user.check_password("testpass123")
+
+
+@pytest.mark.django_db


Missing test coverage for the public POST endpoint (/api/volunteer-applications/) that allows unauthenticated users to submit volunteer applications. This is a core feature mentioned in the PR description.

Add a test that:

POSTs a new application without authentication

Verifies the response status is 201

Confirms the application is created with status="PENDING"

Ensures reviewed_at and reviewed_by are null for new submissions

Copilot · 2025-12-01T22:56:26Z

backend/users/tests.py

+    application.refresh_from_db()
+    assert application.status == "APPROVED"
+    assert application.reviewed_at is not None
+


The test verifies that reviewed_at is set, but doesn't verify that reviewed_by is also set when an application is approved. According to the PR description and the _handle_review_metadata implementation, both fields should be populated when an authenticated admin reviews an application.

Add an assertion to verify application.reviewed_by is set to the reviewing user (or None if unauthenticated, which highlights the permission issue).

Copilot · 2025-12-01T22:56:27Z

backend/users/views.py

+    API endpoint for submitting volunteer applications.
+
+    Currently contains:
+        POST /api/volunteer-applications/


The docstring is incomplete and doesn't accurately describe the full functionality of this ViewSet. It only mentions the POST endpoint, but the class also implements:

GET /api/volunteer-applications/ (admin-only list)

GET /api/volunteer-applications/{id}/ (retrieve)

PATCH /api/volunteer-applications/{id}/ (approve/reject)

Update the docstring to document all available endpoints and their access restrictions.

Suggested change

API endpoint for submitting volunteer applications.

Currently contains:

POST /api/volunteer-applications/

API endpoint for managing volunteer applications.

Endpoints:

- POST /api/volunteer-applications/

Submit a new volunteer application. (Open to all)

- GET /api/volunteer-applications/

List all volunteer applications. (Admin only)

- GET /api/volunteer-applications/{id}/

Retrieve a specific volunteer application. (Admin only)

- PATCH /api/volunteer-applications/{id}/

Approve or reject a volunteer application. (Admin only)

Access:

- POST: Any user (no authentication required)

- GET (list/retrieve), PATCH: Admin users only (role == "ADMIN")

Copilot · 2025-12-01T22:56:27Z

backend/users/views.py

+    def list(self, request, *args, **kwargs):
+        """List applications; restrict to admin users using role PLACEHOLDER."""
+        user = getattr(request, "user", None)
+        if not (user is not None and getattr(user, "is_authenticated", False) and self._is_admin(user)):
+            return Response({"detail": "Admin only"}, status=status.HTTP_403_FORBIDDEN)
+
+        return super().list(request, *args, **kwargs)


Only the list() method has admin-only protection, but other methods like retrieve(), update(), partial_update(), and destroy() are not protected. This means:

Anyone can GET individual applications (potentially exposing PII)

Anyone can update/delete applications (due to AllowAny permissions)

Override these methods or use get_permissions() to ensure proper access control for all actions. Typically, only admins should be able to view individual applications and update their status.

Copilot · 2025-12-01T22:56:27Z

backend/users/tests.py

+    application.refresh_from_db()
+    assert application.status == "REJECTED"
+    assert application.reviewed_at is not None
+


Similar to the approval test, this test should also verify that reviewed_by is set when an application is rejected. The _handle_review_metadata method sets both reviewed_at and reviewed_by for any status change to APPROVED or REJECTED.

Add an assertion to verify application.reviewed_by is properly set.

Copilot · 2025-12-01T22:56:27Z

backend/users/tests.py

    assert user.check_password("testpass123")
+
+
+@pytest.mark.django_db


Missing test coverage for important edge cases:

Approving an application when a User with that email already exists (should not create duplicate user)

Multiple status changes on the same application (ensure reviewed_at and reviewed_by are only set on first review)

Changing status from PENDING to PENDING (should not trigger review metadata or user creation)

Add tests for these scenarios to ensure the logic in _handle_review_metadata and _handle_volunteer_user_creation handles edge cases correctly.

Copilot · 2025-12-01T22:56:27Z

backend/users/tests.py

@@ -1,6 +1,10 @@
 import pytest
+from django.test import Client


Import of 'reverse' is not used.

qiuethan

Please resolve some of the copilot issues first, then re-request review :)

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

pH-li added 2 commits November 20, 2025 00:28

Added intial endpoint setup for volunteer applications with placehold…

1783a7d

…er admin login for getApps

Fixed reformmating issues for the lint test

d328812

pH-li requested review from Edwz208 and qiuethan November 21, 2025 02:29

pH-li self-assigned this Nov 21, 2025

pH-li added 4 commits November 20, 2025 21:30

Forgot to reformat one more file

adb776f

Fix tests.py formatting

b9ab29e

Fix tests.py formatting ++

d513eac

updated to work with sqlite and pass tests

a780ba5

Copilot AI review requested due to automatic review settings December 1, 2025 22:41

Copilot started reviewing on behalf of pH-li December 1, 2025 22:41 View session

reformat

93e630f

Copilot finished reviewing on behalf of pH-li December 1, 2025 22:44

Copilot AI reviewed Dec 1, 2025

View reviewed changes

pH-li and others added 2 commits December 1, 2025 17:48

Added a test secret key for the env so that my tests could run

2a47429

Update backend/users/views.py

9630ede

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings December 1, 2025 22:51

Update backend/users/tests.py

357a7d4

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot started reviewing on behalf of pH-li December 1, 2025 22:52 View session

Copilot finished reviewing on behalf of pH-li December 1, 2025 22:55

Copilot AI reviewed Dec 1, 2025

View reviewed changes

qiuethan requested changes Dec 9, 2025

View reviewed changes

qiuethan added 2 commits December 16, 2025 19:57

Merge branch 'main' into feature/volunteer-application-endpoint

30bd270

Remove the test-secret-key

79be2c7

Copilot AI review requested due to automatic review settings December 17, 2025 00:58

Copilot AI reviewed Dec 17, 2025

View reviewed changes

	# GET /api/volunteer-applications/{id}/ - Retrieve a specific application (admin only)
	# GET /api/volunteer-applications/{id}/ - Retrieve a specific application



		@pytest.mark.django_db
		def test_approve_volunteer_application():

	read_only_fields = ["id", "created_at", "reviewed_at", "reviewed_by"]
	read_only_fields = ["id", "created_at", "reviewed_at", "reviewed_by", "status"]

		@@ -1,35 +1,80 @@
		from rest_framework import viewsets, permissions, status
		from rest_framework.decorators import action

		assert user.check_password("testpass123")


		@pytest.mark.django_db

-    API endpoint for submitting volunteer applications.
-    Currently contains:
-        POST /api/volunteer-applications/
+    API endpoint for managing volunteer applications.
+    Endpoints:
+        - POST /api/volunteer-applications/
+            Submit a new volunteer application. (Open to all)
+        - GET /api/volunteer-applications/
+            List all volunteer applications. (Admin only)
+        - GET /api/volunteer-applications/{id}/
+            Retrieve a specific volunteer application. (Admin only)
+        - PATCH /api/volunteer-applications/{id}/
+            Approve or reject a volunteer application. (Admin only)
+    Access:
+        - POST: Any user (no authentication required)
+        - GET (list/retrieve), PATCH: Admin users only (role == "ADMIN")

feat(backend): add volunteer application endpoints #18

Are you sure you want to change the base?

feat(backend): add volunteer application endpoints #18

Uh oh!

Conversation

pH-li commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Issues

Changes

How to Test

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

qiuethan left a comment

pH-li commented Nov 21, 2025 •

edited

Loading