Merge pull request #741 from Nickatak/refactor/serializers-read-write

refactor: Split serializers into Read/Write classes per resource

Author: RSkuma

backend/ctj_api/serializers.py

@@ -1,4 +1,18 @@
-"""DRF serializers for CTJ's domain models, consumed by the views in `ctj_api.views`."""
+"""DRF serializers for CTJ's domain models, consumed by the views in `ctj_api.views`.
+
+Serializer shape convention:
+- Each resource has a `XxxReadSerializer` for response shape and a
+  `XxxWriteSerializer` for request shape, when both are needed.
+- Read-only resources only have a Read serializer; a Write serializer
+  is added when a write endpoint is added.
+- Auto-managed fields (`id`, `created_at`, `updated_at`) and
+  request-stamped fields (e.g. `created_by`) are *absent* from
+  `XxxWriteSerializer.Meta.fields` rather than included with
+  `read_only=True`. Absence is the contract.
+
+See `docs/developer/backend.md` (`Serializer shape` section) for
+the full rule and rationale.
+"""
 
 from rest_framework import serializers
 
@@ -13,7 +27,7 @@
 )
 
 
-class CustomUserSerializer(serializers.ModelSerializer):
+class CustomUserReadSerializer(serializers.ModelSerializer):
     """Read serializer for `CustomUser` records.
 
     Note: the `opportunities` field is broken - it declares a writable
@@ -23,11 +37,11 @@ class CustomUserSerializer(serializers.ModelSerializer):
     Reads would fail with `AttributeError`; writes would fail
     attempting to set `instance.opportunities`. Currently masked
     because no exercised code path hits it. The right fix is to drop
-    the field entirely - deferred out of this docs-only PR. See
+    the field entirely - deferred out of this shape-only PR. See
     `archive/feat-auth-stage1` for a worked example of the removal.
 
     Used by:
-    - `UserDetail` (`GET /api/users/<uuid>/`).
+    - `user_detail` FBV (`GET /api/users/<uuid>/`).
     """
 
     opportunities = serializers.PrimaryKeyRelatedField(
@@ -52,17 +66,17 @@ class Meta:
         ]
 
 
-class OpportunitySerializer(serializers.ModelSerializer):
-    """Read/write serializer for `Opportunity` records.
+class OpportunityReadSerializer(serializers.ModelSerializer):
+    """Read serializer for `Opportunity` records.
 
-    `created_by` is exposed as a read-only string (the creator's email
-    via `source="created_by.email"`) rather than as the underlying
-    UUID FK. Clients cannot supply or override the field on create or
-    update; `OpportunityViewSet.perform_create` stamps it from
-    `request.user` automatically.
+    `created_by` is exposed as the creator's email string (via
+    `source="created_by.email"`) rather than the underlying UUID FK,
+    so list/retrieve responses surface a human-readable identity
+    rather than an internal ID.
 
     Used by:
-    - `OpportunityViewSet` (`/api/opportunities/`).
+    - `OpportunityViewSet.list` (`GET /api/opportunities/`).
+    - `OpportunityViewSet.retrieve` (`GET /api/opportunities/<pk>/`).
     """
 
     created_by = serializers.ReadOnlyField(source="created_by.email")
@@ -85,16 +99,46 @@ class Meta:
         ]
 
 
+class OpportunityWriteSerializer(serializers.ModelSerializer):
+    """Write serializer for `Opportunity` records.
+
+    Fields absent from `Meta.fields` are the contract for "client
+    cannot supply this on write":
+    - `id`, `created_at`, `updated_at`: auto-managed by Django.
+    - `created_by`: stamped by `OpportunityViewSet.perform_create`
+      from `request.user`; clients cannot supply or override it.
+
+    Used by:
+    - `OpportunityViewSet.create` (`POST /api/opportunities/`).
+    - `OpportunityViewSet.update` / `partial_update`
+      (`PUT/PATCH /api/opportunities/<pk>/`). Note: PATCH is currently
+      403'd by `OpportunityPermission` (no PATCH branch); flagged for
+      fix in `ctj_api.permissions`.
+    """
+
+    class Meta:
+        model = Opportunity
+        fields = [
+            "project",
+            "role",
+            "body",
+            "min_experience_required",
+            "min_hours_required",
+            "work_environment",
+            "skills_required_matrix",
+            "status",
+        ]
+
+
 class SkillMatrixSerializer(serializers.ModelSerializer):
     """Read/write serializer for `SkillMatrix` records.
 
-    Note: defined but never imported. `SkillMatrix` instances are
-    currently surfaced indirectly via `CustomUser.skills_learned_matrix`
-    and `Opportunity.skills_required_matrix` (each exposes the FK as
-    a UUID through DRF's default PK-related behavior on the parent
-    serializer). This serializer is dead code today; either wire it
-    up to a dedicated endpoint when one is needed, or remove it.
-    Deferred out of this docs-only PR.
+    Note: defined but never imported. Not split into Read/Write
+    because the class is currently dead code.
+    The cleanup PR drops it. If `SkillMatrix` becomes API-exposed
+    later, replace this with `SkillMatrixReadSerializer` (and
+    `SkillMatrixWriteSerializer` if a write endpoint is added).
+    Deferred out of this shape-only PR.
 
     Used by:
     - (none currently).
@@ -110,11 +154,13 @@ class Meta:
         ]
 
 
-class CommunityOfPracticeSerializer(serializers.ModelSerializer):
+class CommunityOfPracticeReadSerializer(serializers.ModelSerializer):
     """Read serializer for `CommunityOfPractice` records.
 
     Used by:
-    - `CommunityOfPracticeViewSet` (`/api/communityOfPractice/`).
+    - `community_of_practice_list` FBV (`GET /api/communityOfPractice/`).
+    - `community_of_practice_detail` FBV
+      (`GET /api/communityOfPractice/<uuid:pk>/`).
     """
 
     class Meta:
@@ -128,35 +174,38 @@ class Meta:
         ]
 
 
-class RoleSerializer(serializers.ModelSerializer):
+class RoleReadSerializer(serializers.ModelSerializer):
     """Read serializer for `Role` records.
 
     Used by:
-    - `RoleViewSet` (`/api/roles/`).
+    - `role_list` FBV (`GET /api/roles/`).
+    - `role_detail` FBV (`GET /api/roles/<uuid:pk>/`).
     """
 
     class Meta:
         model = Role
         fields = ["id", "title", "community_of_practice", "created_at", "updated_at"]
 
 
-class SkillSerializer(serializers.ModelSerializer):
+class SkillReadSerializer(serializers.ModelSerializer):
     """Read serializer for `Skill` records.
 
     Used by:
-    - `SkillViewSet` (`/api/skills/`).
+    - `skill_list` FBV (`GET /api/skills/`).
+    - `skill_detail` FBV (`GET /api/skills/<uuid:pk>/`).
     """
 
     class Meta:
         model = Skill
         fields = ["id", "name", "communities_of_practice", "created_at", "updated_at"]
 
 
-class ProjectSerializer(serializers.ModelSerializer):
+class ProjectReadSerializer(serializers.ModelSerializer):
     """Read serializer for `Project` records.
 
     Used by:
-    - `ProjectViewSet` (`/api/projects/`).
+    - `project_list` FBV (`GET /api/projects/`).
+    - `project_detail` FBV (`GET /api/projects/<uuid:pk>/`).
     """
 
     class Meta:

backend/ctj_api/views.py

@@ -40,12 +40,13 @@
 )
 from ctj_api.permissions import OpportunityPermission, UserDetailPermission
 from ctj_api.serializers import (
-    CommunityOfPracticeSerializer,
-    CustomUserSerializer,
-    OpportunitySerializer,
-    ProjectSerializer,
-    RoleSerializer,
-    SkillSerializer,
+    CommunityOfPracticeReadSerializer,
+    CustomUserReadSerializer,
+    OpportunityReadSerializer,
+    OpportunityWriteSerializer,
+    ProjectReadSerializer,
+    RoleReadSerializer,
+    SkillReadSerializer,
 )
 
 start_time = time.time()
@@ -160,7 +161,7 @@ def user_detail(request, pk):
     # object-level (`has_object_permission`) check has to be triggered
     # explicitly in FBVs since there's no APIView class to auto-call it.
     request.parser_context["view"].check_object_permissions(request, user)
-    serializer = CustomUserSerializer(user)
+    serializer = CustomUserReadSerializer(user)
     return Response(serializer.data)
 
 
@@ -195,8 +196,8 @@ class OpportunityViewSet(viewsets.ModelViewSet):
     - IsAuthenticatedOrReadOnly + OpportunityPermission
 
     Errors:
-    - 400: Validation error on create/update (`OpportunitySerializer`
-      rejected the payload).
+    - 400: Validation error on create/update
+      (`OpportunityWriteSerializer` rejected the payload).
     - 401: Unauthenticated mutation.
     - 403: `OpportunityPermission` denied (e.g. non-PM trying to
       create, non-creator trying to update). Note: PATCH is always
@@ -206,12 +207,22 @@ class OpportunityViewSet(viewsets.ModelViewSet):
     """
 
     queryset = Opportunity.objects.all()
-    serializer_class = OpportunitySerializer
+    serializer_class = OpportunityReadSerializer
     permission_classes = (
         permissions.IsAuthenticatedOrReadOnly,
         OpportunityPermission,
     )
 
+    def get_serializer_class(self):
+        # Dispatch by action: list/retrieve return the Read shape;
+        # create/update/destroy accept the Write shape. The class-level
+        # `serializer_class = OpportunityReadSerializer` above is the
+        # safe fallback (Read) if `self.action` is None during schema
+        # introspection.
+        if self.action in ("list", "retrieve"):
+            return OpportunityReadSerializer
+        return OpportunityWriteSerializer
+
     def perform_create(self, serializer):
         serializer.save(created_by=self.request.user)
 
@@ -229,7 +240,7 @@ def community_of_practice_list(request):
 
     Flow:
     1. Fetch all `CommunityOfPractice` rows.
-    2. Serialize via `CommunityOfPracticeSerializer` and return 200.
+    2. Serialize via `CommunityOfPracticeReadSerializer` and return 200.
 
     URL:
     - GET /api/communityOfPractice/
@@ -241,7 +252,7 @@ def community_of_practice_list(request):
     - (none)
     """
     cops = CommunityOfPractice.objects.all()
-    serializer = CommunityOfPracticeSerializer(cops, many=True)
+    serializer = CommunityOfPracticeReadSerializer(cops, many=True)
     return Response(serializer.data)
 
 
@@ -254,7 +265,7 @@ def community_of_practice_detail(request, pk):
 
     Flow:
     1. Look up the row by primary-key UUID.
-    2. Serialize via `CommunityOfPracticeSerializer` and return 200.
+    2. Serialize via `CommunityOfPracticeReadSerializer` and return 200.
 
     URL:
     - GET /api/communityOfPractice/<uuid:pk>/
@@ -266,7 +277,7 @@ def community_of_practice_detail(request, pk):
     - 404: No CoP exists with the given UUID.
     """
     cop = get_object_or_404(CommunityOfPractice, pk=pk)
-    serializer = CommunityOfPracticeSerializer(cop)
+    serializer = CommunityOfPracticeReadSerializer(cop)
     return Response(serializer.data)
 
 
@@ -283,7 +294,7 @@ def role_list(request):
 
     Flow:
     1. Fetch all `Role` rows.
-    2. Serialize via `RoleSerializer` and return 200.
+    2. Serialize via `RoleReadSerializer` and return 200.
 
     URL:
     - GET /api/roles/
@@ -295,7 +306,7 @@ def role_list(request):
     - (none)
     """
     roles = Role.objects.all()
-    serializer = RoleSerializer(roles, many=True)
+    serializer = RoleReadSerializer(roles, many=True)
     return Response(serializer.data)
 
 
@@ -308,7 +319,7 @@ def role_detail(request, pk):
 
     Flow:
     1. Look up the row by primary-key UUID.
-    2. Serialize via `RoleSerializer` and return 200.
+    2. Serialize via `RoleReadSerializer` and return 200.
 
     URL:
     - GET /api/roles/<uuid:pk>/
@@ -320,7 +331,7 @@ def role_detail(request, pk):
     - 404: No role exists with the given UUID.
     """
     role = get_object_or_404(Role, pk=pk)
-    serializer = RoleSerializer(role)
+    serializer = RoleReadSerializer(role)
     return Response(serializer.data)
 
 
@@ -337,7 +348,7 @@ def skill_list(request):
 
     Flow:
     1. Fetch all `Skill` rows.
-    2. Serialize via `SkillSerializer` and return 200.
+    2. Serialize via `SkillReadSerializer` and return 200.
 
     URL:
     - GET /api/skills/
@@ -349,7 +360,7 @@ def skill_list(request):
     - (none)
     """
     skills = Skill.objects.all()
-    serializer = SkillSerializer(skills, many=True)
+    serializer = SkillReadSerializer(skills, many=True)
     return Response(serializer.data)
 
 
@@ -362,7 +373,7 @@ def skill_detail(request, pk):
 
     Flow:
     1. Look up the row by primary-key UUID.
-    2. Serialize via `SkillSerializer` and return 200.
+    2. Serialize via `SkillReadSerializer` and return 200.
 
     URL:
     - GET /api/skills/<uuid:pk>/
@@ -374,7 +385,7 @@ def skill_detail(request, pk):
     - 404: No skill exists with the given UUID.
     """
     skill = get_object_or_404(Skill, pk=pk)
-    serializer = SkillSerializer(skill)
+    serializer = SkillReadSerializer(skill)
     return Response(serializer.data)
 
 
@@ -390,7 +401,7 @@ def project_list(request):
 
     Flow:
     1. Fetch all `Project` rows.
-    2. Serialize via `ProjectSerializer` and return 200.
+    2. Serialize via `ProjectReadSerializer` and return 200.
 
     URL:
     - GET /api/projects/
@@ -402,7 +413,7 @@ def project_list(request):
     - (none)
     """
     projects = Project.objects.all()
-    serializer = ProjectSerializer(projects, many=True)
+    serializer = ProjectReadSerializer(projects, many=True)
     return Response(serializer.data)
 
 
@@ -415,7 +426,7 @@ def project_detail(request, pk):
 
     Flow:
     1. Look up the row by primary-key UUID.
-    2. Serialize via `ProjectSerializer` and return 200.
+    2. Serialize via `ProjectReadSerializer` and return 200.
 
     URL:
     - GET /api/projects/<uuid:pk>/
@@ -427,5 +438,5 @@ def project_detail(request, pk):
     - 404: No project exists with the given UUID.
     """
     project = get_object_or_404(Project, pk=pk)
-    serializer = ProjectSerializer(project)
+    serializer = ProjectReadSerializer(project)
     return Response(serializer.data)