feat(course-filters): tighten PATCH/POST contract (required name, for…

src/schemas/course_filter.py

@@ -1,6 +1,5 @@
 """Course filter schemas for request/response validation."""
 from datetime import datetime
-from typing import Optional
 
 from pydantic import BaseModel, ConfigDict, Field, field_validator
 
@@ -18,25 +17,39 @@ def _strip(cls, v: str) -> str:
             raise ValueError("name must not be blank")
         return v
 
-    model_config = ConfigDict(json_schema_extra={"example": {"name": "SQL"}})
+    # ``extra="forbid"`` lets us reject typos / stale clients up-front instead
+    # of silently dropping unknown keys (Pydantic's default).
+    model_config = ConfigDict(
+        extra="forbid",
+        json_schema_extra={"example": {"name": "SQL"}},
+    )
 
 
 class CourseFilterUpdate(BaseModel):
-    """Schema for renaming a course filter."""
+    """Schema for renaming a course filter.
+
+    Today the only editable field is ``name`` — and it is REQUIRED here, not
+    optional. Rationale: a PATCH with no editable field is a no-op, and
+    silently accepting ``{}`` lets buggy clients ship a deploy that „works"
+    in CI and surprises us in prod. If a second editable field is added later,
+    relax this back to ``Optional`` and add a model-level ``at-least-one-set``
+    validator.
+    """
 
-    name: Optional[str] = Field(None, description="Neuer Filter-String", min_length=1, max_length=255)
+    name: str = Field(..., description="Neuer Filter-String", min_length=1, max_length=255)
 
     @field_validator("name")
     @classmethod
-    def _strip(cls, v: Optional[str]) -> Optional[str]:
-        if v is None:
-            return v
+    def _strip(cls, v: str) -> str:
         v = v.strip()
         if not v:
             raise ValueError("name must not be blank")
         return v
 
-    model_config = ConfigDict(json_schema_extra={"example": {"name": "SQL Grundlagen"}})
+    model_config = ConfigDict(
+        extra="forbid",
+        json_schema_extra={"example": {"name": "SQL Grundlagen"}},
+    )
 
 
 class CourseFilterResponse(BaseModel):

src/services/course_filter_service.py

@@ -61,20 +61,20 @@ def create_filter(self, data: CourseFilterCreate) -> CourseFilter:
     def update_filter(self, filter_id: UUID, data: CourseFilterUpdate) -> CourseFilter:
         instance = self.get_filter(filter_id)
 
-        update_data = data.model_dump(exclude_unset=True)
-        if not update_data:
-            return instance
+        # ``name`` is required at the schema level, so ``new_name`` is always
+        # present and non-blank here. A no-op (same name) still flows through
+        # so ``updated_at`` advances — that's a fine default for PATCH.
+        new_name = data.name
 
-        new_name = update_data.get("name")
-        if new_name and new_name != instance.name:
+        if new_name != instance.name:
             existing = self.repo.get_by_name(new_name)
             if existing and existing.id != instance.id:
                 raise ConflictException(
                     f"Course filter with name '{new_name}' already exists"
                 )
 
         try:
-            updated = self.repo.update(filter_id, **update_data)
+            updated = self.repo.update(filter_id, name=new_name)
         except IntegrityError as e:
             self.db.rollback()
             logger.warning(

tests/api/test_course_filter_routes.py

@@ -359,3 +359,72 @@ def test_delete_filter_lecturer_returns_403():
 
     response = client.delete(f"/api/v1/course-filters/{uuid4()}")
     assert response.status_code == 403
+
+
+# ── strict schema: required + no extra fields ─────────────────────────────────
+
+
+def test_create_filter_rejects_extra_fields():
+    """``extra="forbid"`` blocks unknown keys so a typo / stale client surfaces
+    as 422 instead of being silently dropped."""
+    app.dependency_overrides[get_current_user] = _admin_user
+    app.dependency_overrides[get_db] = lambda: MagicMock()
+
+    response = client.post(
+        "/api/v1/course-filters",
+        json={"name": "SQL", "color": "red"},
+    )
+    assert response.status_code == 422
+
+
+def test_update_filter_empty_body_returns_422():
+    """``name`` is required on PATCH — an empty body is not a valid no-op."""
+    app.dependency_overrides[get_current_user] = _admin_user
+    app.dependency_overrides[get_db] = lambda: MagicMock()
+
+    response = client.patch(f"/api/v1/course-filters/{uuid4()}", json={})
+    assert response.status_code == 422
+
+
+def test_update_filter_rejects_extra_fields():
+    """Same forbid-extra contract on PATCH so frontend doesn't accidentally
+    POST fields that look editable but aren't."""
+    app.dependency_overrides[get_current_user] = _admin_user
+    app.dependency_overrides[get_db] = lambda: MagicMock()
+
+    response = client.patch(
+        f"/api/v1/course-filters/{uuid4()}",
+        json={"name": "x", "id": "spoofed"},
+    )
+    assert response.status_code == 422
+
+
+def test_update_filter_to_same_name_succeeds():
+    """No-op rename (same name) is intentionally allowed — flows through and
+    bumps ``updated_at``. The duplicate-check skips because the existing row
+    IS the same row."""
+    fid = str(uuid4())
+    existing = _make_filter("SQL", fid=fid)
+
+    repo = MagicMock()
+    repo.get_by_id.return_value = existing
+    # If the service mistakenly hit get_by_name here, it would short-circuit
+    # to 409 because existing.id matches. Make sure it never gets called.
+    repo.get_by_name.side_effect = AssertionError(
+        "get_by_name must not be called when name is unchanged"
+    )
+    repo.update.return_value = existing
+
+    app.dependency_overrides[get_current_user] = _admin_user
+    app.dependency_overrides[get_db] = lambda: MagicMock()
+
+    with patch(
+        "src.services.course_filter_service.CourseFilterRepository",
+        return_value=repo,
+    ):
+        response = client.patch(
+            f"/api/v1/course-filters/{fid}", json={"name": "SQL"}
+        )
+
+    assert response.status_code == 200
+    repo.update.assert_called_once()