Skip to content

feat(execom): added campus execom management with role assignments #2559

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
joyanna27 wants to merge 1 commit into from
+52 −1
Closed

feat(execom): added campus execom management with role assignments #2559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions api/campus_execom_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
from rest_framework.views import APIView
from rest_framework.response import Response
from db.organization import CampusExecom, Organization

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import of 'Organization' is not used.

Suggested change
from db.organization import CampusExecom, Organization
from db.organization import CampusExecom

Copilot uses AI. Check for mistakes.
from db.user import User

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import of 'User' is not used.

Suggested change
from db.user import User

Copilot uses AI. Check for mistakes.
import uuid

class CampusExecomView(APIView):
Comment on lines +5 to +7

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing authentication and authorization checks. The view should use authentication_classes like other views in the codebase (e.g., CustomizePermission) and role-based access control to ensure only authorized users can manage execom members.

Suggested change
import uuid
class CampusExecomView(APIView):
import uuid
from rest_framework.authentication import SessionAuthentication, BasicAuthentication
from rest_framework.permissions import BasePermission, SAFE_METHODS
class CampusExecomPermission(BasePermission):
"""
Permission class for CampusExecomView.
- Requires the user to be authenticated for all requests.
- Allows all authenticated users to perform safe (read-only) requests.
- Restricts modifying requests (POST, DELETE, etc.) to staff/superuser users.
"""
def has_permission(self, request, view):
user = getattr(request, "user", None)
if not user or not getattr(user, "is_authenticated", False):
return False
if request.method in SAFE_METHODS:
return True
# For write operations, require elevated privileges (staff or superuser).
return bool(
getattr(user, "is_staff", False) or getattr(user, "is_superuser", False)
)
class CampusExecomView(APIView):
authentication_classes = [SessionAuthentication, BasicAuthentication]
permission_classes = [CampusExecomPermission]

Copilot uses AI. Check for mistakes.

# 1. VIEW EXECOM MEMBERS (GET)
def get(self, request, campus_id):
members = CampusExecom.objects.filter(campus_id=campus_id).values(
'id', 'user__first_name', 'user__last_name', 'role', 'user__email'
)
return Response({'data': list(members)})
Comment on lines +10 to +14

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing validation for campus_id parameter. The endpoint should verify that the campus_id exists and is a valid Organization before querying CampusExecom records.

Copilot uses AI. Check for mistakes.

# 2. ADD MEMBER (POST)
def post(self, request, campus_id):
user_id = request.data.get('user_id')
role = request.data.get('role')

if not user_id or not role:
return Response({'message': 'User ID and Role are required'}, status=400)

if CampusExecom.objects.filter(campus_id=campus_id, user_id=user_id).exists():
return Response({'message': 'User is already in Execom'}, status=400)
Comment on lines +24 to +25

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existence check only prevents duplicates at the application level but not at the database level. This creates a race condition where two concurrent requests could both pass the check and create duplicate records. The unique constraint should be enforced at the database level through the model's Meta class.

Copilot uses AI. Check for mistakes.

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra leading whitespace before return statement. This line has inconsistent indentation with an extra space at the beginning.

Suggested change
return Response({'message': 'User is already in Execom'}, status=400)
return Response({'message': 'User is already in Execom'}, status=400)

Copilot uses AI. Check for mistakes.

CampusExecom.objects.create(
id=str(uuid.uuid4()),

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant UUID generation. The model already has a default UUID generator (uuid.uuid4), so manually generating and passing the id is unnecessary. You can remove the id parameter from the create() call and let Django handle it automatically.

Copilot uses AI. Check for mistakes.
user_id=user_id,
campus_id=campus_id,
role=role
)
Comment on lines +17 to +32

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing validation for user_id parameter. The endpoint should verify that the user_id exists and is a valid User before creating the CampusExecom record.

Copilot uses AI. Check for mistakes.
Comment on lines +17 to +32

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing validation for campus_id parameter. The endpoint should verify that the campus exists and is a valid Organization before attempting to add a member.

Copilot uses AI. Check for mistakes.
return Response({'message': 'Member added successfully'})

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing HTTP status code in success response. Following REST API best practices and consistency with other endpoints in the codebase, the response should include status=201 to indicate resource creation.

Copilot uses AI. Check for mistakes.

# 3. REMOVE MEMBER (DELETE)
def delete(self, request, campus_id, uid):
CampusExecom.objects.filter(id=uid).delete()

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing validation for uid and campus_id parameters. The endpoint should verify that the execom member exists and belongs to the specified campus before deleting. Currently, it only filters by id without checking if the campus_id matches, which could allow deletion of members from different campuses.

Suggested change
CampusExecom.objects.filter(id=uid).delete()
member_qs = CampusExecom.objects.filter(id=uid, campus_id=campus_id)
if not member_qs.exists():
return Response({'message': 'Member not found for this campus'}, status=404)
member_qs.delete()

Copilot uses AI. Check for mistakes.

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DELETE operation returns a success message even when the member doesn't exist. This could be misleading. Consider checking if the member exists before deletion and returning an appropriate error if not found.

Suggested change
CampusExecom.objects.filter(id=uid).delete()
deleted_count, _ = CampusExecom.objects.filter(id=uid).delete()
if deleted_count == 0:
return Response({'message': 'Member not found'}, status=404)

Copilot uses AI. Check for mistakes.
Comment on lines +36 to +37

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent parameter naming. The URL parameter is named 'uid' but it represents an execom member ID. Consider renaming to 'execom_id' or 'member_id' for clarity and consistency with the domain model.

Suggested change
def delete(self, request, campus_id, uid):
CampusExecom.objects.filter(id=uid).delete()
def delete(self, request, campus_id, execom_id):
CampusExecom.objects.filter(id=execom_id).delete()

Copilot uses AI. Check for mistakes.
return Response({'message': 'Member removed successfully'})

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing HTTP status code in success response. Following REST API best practices, the DELETE endpoint should return status=204 (No Content) for successful deletion or status=200 with a message.

Suggested change
return Response({'message': 'Member removed successfully'})
return Response({'message': 'Member removed successfully'}, status=200)

Copilot uses AI. Check for mistakes.
4 changes: 3 additions & 1 deletion api/urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import debug_toolbar
from django.urls import path, include

from.campus_execom_views import CampusExecomView

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space after dot in import statement. Should be 'from .' instead of 'from.'

Suggested change
from.campus_execom_views import CampusExecomView
from .campus_execom_views import CampusExecomView

Copilot uses AI. Check for mistakes.
# app_name will help us do a reverse look-up latter.
urlpatterns = [
path('register/', include('api.register.urls')),
Expand All @@ -16,4 +16,6 @@
path('launchpad/', include('api.launchpad.urls')),
path('donate/', include('api.donate.urls')),
path("__debug__/", include(debug_toolbar.urls)),
path('campus/<str:campus_id>/execom/',CampusExecomView.as_view()),
path('campus/<str:campus_id>/execom/<str:uid>/',CampusExecomView.as_view()),
Comment on lines +19 to +20

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space after comma in path function call. This is inconsistent with Python style guidelines (PEP 8) which recommend a space after commas.

Suggested change
path('campus/<str:campus_id>/execom/',CampusExecomView.as_view()),
path('campus/<str:campus_id>/execom/<str:uid>/',CampusExecomView.as_view()),
path('campus/<str:campus_id>/execom/', CampusExecomView.as_view()),
path('campus/<str:campus_id>/execom/<str:uid>/', CampusExecomView.as_view()),

Copilot uses AI. Check for mistakes.
Comment on lines +19 to +20

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space after comma in path function call. This is inconsistent with Python style guidelines (PEP 8) which recommend a space after commas.

Suggested change
path('campus/<str:campus_id>/execom/',CampusExecomView.as_view()),
path('campus/<str:campus_id>/execom/<str:uid>/',CampusExecomView.as_view()),
path('campus/<str:campus_id>/execom/', CampusExecomView.as_view()),
path('campus/<str:campus_id>/execom/<str:uid>/', CampusExecomView.as_view()),

Copilot uses AI. Check for mistakes.
]
11 changes: 11 additions & 0 deletions db/organization.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,3 +236,14 @@ class UnverifiedOrganization(models.Model):
class Meta:
managed = False
db_table = 'unverified_organization'


class CampusExecom(models.Model):
id = models.CharField(primary_key=True, max_length=36, default=uuid.uuid4)

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The UUID default should be callable (uuid.uuid4) not called (uuid.uuid4()). This is inconsistent with other models like State, Zone, and District in the same file which use uuid.uuid4 without parentheses. Using uuid.uuid4() means the same UUID will be used for all instances created during the same module import, while uuid.uuid4 (callable) generates a new UUID for each instance.

Copilot uses AI. Check for mistakes.
user = models.ForeignKey(User, on_delete=models.CASCADE, related_name='campus_execom_roles')
campus = models.ForeignKey(Organization, on_delete=models.CASCADE, related_name='execom_members')
role = models.CharField(max_length=100)
created_at = models.DateTimeField(auto_now_add=True)

class Meta:
db_table = 'campus_execom'

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing unique constraint on user-campus combination. A user should only have one role per campus. Add a unique_together constraint in the Meta class to prevent duplicate entries for the same user and campus combination.

Suggested change
db_table = 'campus_execom'
db_table = 'campus_execom'
unique_together = ('user', 'campus')

Copilot uses AI. Check for mistakes.