From 66ff4bf27a197ef97a85a3980587771309866959 Mon Sep 17 00:00:00 2001 From: nicowre Date: Tue, 30 Jun 2026 13:02:10 +0200 Subject: [PATCH 1/2] feat(course-filters): admin-managed string filters for course names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a flat, admin-managed list of strings that the frontend renders as filter-chips above the course list. Filtering itself stays client-side — this resource only persists the labels. - Model + migration: course_filters table, name unique - Endpoints under /api/v1/course-filters: - GET list (paginated, ?search=) open to any authenticated user - POST / PATCH / DELETE — ADMIN only - Duplicate names mapped to 409 (pre-check + IntegrityError race fallback) - Tests: 15 cases covering happy paths, admin-only enforcement, duplicate handling, and 404s --- ...9c2a14e7b80_create_course_filters_table.py | 43 +++ src/api/__init__.py | 2 + src/api/course_filters.py | 127 ++++++ src/models/__init__.py | 2 + src/models/course_filter.py | 35 ++ src/repositories/course_filter_repository.py | 43 +++ src/schemas/course_filter.py | 60 +++ src/services/course_filter_service.py | 91 +++++ tests/api/test_course_filter_routes.py | 361 ++++++++++++++++++ 9 files changed, 764 insertions(+) create mode 100644 alembic/versions/f9c2a14e7b80_create_course_filters_table.py create mode 100644 src/api/course_filters.py create mode 100644 src/models/course_filter.py create mode 100644 src/repositories/course_filter_repository.py create mode 100644 src/schemas/course_filter.py create mode 100644 src/services/course_filter_service.py create mode 100644 tests/api/test_course_filter_routes.py diff --git a/alembic/versions/f9c2a14e7b80_create_course_filters_table.py b/alembic/versions/f9c2a14e7b80_create_course_filters_table.py new file mode 100644 index 0000000..a165a07 --- /dev/null +++ b/alembic/versions/f9c2a14e7b80_create_course_filters_table.py @@ -0,0 +1,43 @@ +"""create course_filters table + +Revision ID: f9c2a14e7b80 +Revises: ebc91d7b5d43 +Create Date: 2026-06-30 12:00:00.000000 + +Admin-verwaltete Filter-Strings für Kursnamen (Frontend-Chips). Die Filterung +selbst läuft client-seitig — diese Tabelle hält nur die Liste der Begriffe. +``name`` ist unique, damit doppelte Chips im UI vermieden werden. +""" +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + + +revision: str = 'f9c2a14e7b80' +down_revision: Union[str, Sequence[str], None] = 'ebc91d7b5d43' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + """Create course_filters table.""" + op.create_table( + 'course_filters', + sa.Column('id', sa.String(length=36), nullable=False), + sa.Column( + 'name', + sa.String(length=255), + nullable=False, + comment='Anzeige-/Such-String, den das Frontend gegen Kursnamen matcht', + ), + sa.Column('created_at', sa.DateTime(timezone=True), nullable=False), + sa.Column('updated_at', sa.DateTime(timezone=True), nullable=False), + sa.PrimaryKeyConstraint('id'), + sa.UniqueConstraint('name', name='uq_course_filters_name'), + ) + + +def downgrade() -> None: + """Drop course_filters table.""" + op.drop_table('course_filters') diff --git a/src/api/__init__.py b/src/api/__init__.py index f5b9dbd..97f417b 100644 --- a/src/api/__init__.py +++ b/src/api/__init__.py @@ -6,6 +6,7 @@ from src.api.openstack_projects import router as openstack_projects_router from src.api.template_version_files import router as template_version_files_router from src.api.courses import router as courses_router +from src.api.course_filters import router as course_filters_router from src.api.quotas import router as quotas_router from src.api.openstack_flavors import router as openstack_flavors_router from src.api.github_app import router as github_app_router @@ -20,6 +21,7 @@ api_router.include_router(openstack_projects_router) api_router.include_router(template_version_files_router) api_router.include_router(courses_router) +api_router.include_router(course_filters_router) api_router.include_router(quotas_router) api_router.include_router(openstack_flavors_router) api_router.include_router(github_app_router) diff --git a/src/api/course_filters.py b/src/api/course_filters.py new file mode 100644 index 0000000..29e09f2 --- /dev/null +++ b/src/api/course_filters.py @@ -0,0 +1,127 @@ +"""Course filter API endpoints. + +Admin-verwaltete Filter-Strings (Frontend-Chips), gegen die Kursnamen client- +seitig gematcht werden. Lese-Zugriff ist für alle eingeloggten User offen, +damit das Frontend die Chip-Leiste rendern kann; Schreiben ist Admin-only. +""" +from typing import Optional +from uuid import UUID + +from fastapi import APIRouter, Depends, Query, status + +from src.core.dependencies import ( + CurrentUser, + DBSession, + Pagination, + RequestID, + require_roles, +) +from src.core.response_builder import ResponseBuilder +from src.models.user import UserRole +from src.schemas.course_filter import ( + CourseFilterCreate, + CourseFilterResponse, + CourseFilterUpdate, +) +from src.services.course_filter_service import CourseFilterService + +router = APIRouter(prefix="/course-filters", tags=["course-filters"]) + + +@router.get("", response_model=None) +async def list_course_filters( + pagination: Pagination, + db: DBSession, + request_id: RequestID, + current_user: CurrentUser, + search: Optional[str] = Query(None, description="Substring-Suche auf name"), +): + """List course filters. Open to any authenticated user (frontend needs them).""" + service = CourseFilterService(db) + rows, total = service.list_filters( + skip=(pagination.page - 1) * pagination.page_size, + limit=pagination.page_size, + search=search, + ) + + payload = [ + CourseFilterResponse.model_validate(row).model_dump(mode="json") for row in rows + ] + + return ResponseBuilder.paginated( + data=payload, + page=pagination.page, + page_size=pagination.page_size, + total=total, + message="Course filters retrieved successfully", + request_id=request_id, + ) + + +@router.post( + "", + status_code=status.HTTP_201_CREATED, + response_model=None, + dependencies=[Depends(require_roles(UserRole.ADMIN))], +) +async def create_course_filter( + data: CourseFilterCreate, + db: DBSession, + request_id: RequestID, + current_user: CurrentUser, +): + """Create a new course filter (admin only).""" + service = CourseFilterService(db) + instance = service.create_filter(data) + + return ResponseBuilder.created( + data=CourseFilterResponse.model_validate(instance).model_dump(mode="json"), + message="Course filter created successfully", + request_id=request_id, + ) + + +@router.patch( + "/{filter_id}", + response_model=None, + dependencies=[Depends(require_roles(UserRole.ADMIN))], +) +async def update_course_filter( + filter_id: UUID, + data: CourseFilterUpdate, + db: DBSession, + request_id: RequestID, + current_user: CurrentUser, +): + """Rename a course filter (admin only).""" + service = CourseFilterService(db) + instance = service.update_filter(filter_id=filter_id, data=data) + + return ResponseBuilder.success( + data=CourseFilterResponse.model_validate(instance).model_dump(mode="json"), + message="Course filter updated successfully", + request_id=request_id, + ) + + +@router.delete( + "/{filter_id}", + status_code=status.HTTP_200_OK, + response_model=None, + dependencies=[Depends(require_roles(UserRole.ADMIN))], +) +async def delete_course_filter( + filter_id: UUID, + db: DBSession, + request_id: RequestID, + current_user: CurrentUser, +): + """Delete a course filter (admin only).""" + service = CourseFilterService(db) + service.delete_filter(filter_id=filter_id) + + return ResponseBuilder.success( + data=None, + message="Course filter deleted successfully", + request_id=request_id, + ) diff --git a/src/models/__init__.py b/src/models/__init__.py index 7721a48..1ab935e 100644 --- a/src/models/__init__.py +++ b/src/models/__init__.py @@ -2,6 +2,7 @@ # Import all model classes to make them available for SQLAlchemy relationship resolution from src.models.course import Course +from src.models.course_filter import CourseFilter from src.models.course_group import CourseGroup from src.models.course_member import CourseMember from src.models.deployment import Deployment, DeploymentStatus @@ -19,6 +20,7 @@ __all__ = [ "Course", + "CourseFilter", "CourseGroup", "CourseMember", "Deployment", diff --git a/src/models/course_filter.py b/src/models/course_filter.py new file mode 100644 index 0000000..5c4a366 --- /dev/null +++ b/src/models/course_filter.py @@ -0,0 +1,35 @@ +"""Course filter (frontend chip/blob) database model. + +Admin-verwaltete Strings, mit denen das Frontend Kursnamen filtert. Die Filter +sind reine Such-Begriffe (z. B. „SQL", „Web 2026") — sie werden NICHT einem +Kurs zugewiesen, sondern client-seitig auf ``Course.name`` angewandt. +""" +from datetime import datetime, timezone +from uuid import uuid4 + +from sqlalchemy import String, DateTime +from sqlalchemy.orm import Mapped, mapped_column + +from src.core.database import Base + + +class CourseFilter(Base): + """Filter-Tag für Kursnamen (Frontend-Chips).""" + + __tablename__ = "course_filters" + + id: Mapped[str] = mapped_column(String(36), primary_key=True, default=lambda: str(uuid4())) + name: Mapped[str] = mapped_column( + String(255), + nullable=False, + unique=True, + comment="Anzeige-/Such-String, den das Frontend gegen Kursnamen matcht", + ) + created_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), default=lambda: datetime.now(timezone.utc) + ) + updated_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), + default=lambda: datetime.now(timezone.utc), + onupdate=lambda: datetime.now(timezone.utc), + ) diff --git a/src/repositories/course_filter_repository.py b/src/repositories/course_filter_repository.py new file mode 100644 index 0000000..3e4f1ca --- /dev/null +++ b/src/repositories/course_filter_repository.py @@ -0,0 +1,43 @@ +"""Course filter repository for database operations.""" +from typing import Optional + +from sqlalchemy.orm import Session + +from src.models.course_filter import CourseFilter +from src.repositories.base_repository import BaseRepository + + +class CourseFilterRepository(BaseRepository[CourseFilter]): + """Repository for CourseFilter database operations.""" + + def __init__(self, db: Session): + super().__init__(CourseFilter, db) + + def get_by_name(self, name: str) -> Optional[CourseFilter]: + """Fetch a filter by its exact (case-sensitive) name.""" + return self.db.query(self.model).filter(self.model.name == name).first() + + def get_all_filtered( + self, + skip: int = 0, + limit: int = 100, + search: Optional[str] = None, + ) -> tuple[list[CourseFilter], int]: + """List filters with optional substring search and pagination. + + Returns: + Tuple of (rows, total count matching the search). + """ + query = self.db.query(self.model) + if search: + query = query.filter(self.model.name.ilike(f"%{search}%")) + + total = query.count() + rows = ( + query + .order_by(self.model.name.asc()) + .offset(skip) + .limit(limit) + .all() + ) + return rows, total diff --git a/src/schemas/course_filter.py b/src/schemas/course_filter.py new file mode 100644 index 0000000..ec763c8 --- /dev/null +++ b/src/schemas/course_filter.py @@ -0,0 +1,60 @@ +"""Course filter schemas for request/response validation.""" +from datetime import datetime +from typing import Optional + +from pydantic import BaseModel, ConfigDict, Field, field_validator + + +class CourseFilterCreate(BaseModel): + """Schema for creating a course filter.""" + + name: str = Field(..., description="Filter-String (z. B. „SQL“)", min_length=1, max_length=255) + + @field_validator("name") + @classmethod + 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"}}) + + +class CourseFilterUpdate(BaseModel): + """Schema for renaming a course filter.""" + + name: Optional[str] = Field(None, 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 + 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"}}) + + +class CourseFilterResponse(BaseModel): + """Schema for course filter response.""" + + id: str = Field(..., description="Filter-ID") + name: str = Field(..., description="Filter-String") + created_at: datetime = Field(..., description="Creation timestamp") + updated_at: datetime = Field(..., description="Last update timestamp") + + model_config = ConfigDict( + from_attributes=True, + json_schema_extra={ + "example": { + "id": "filter-123", + "name": "SQL", + "created_at": "2026-06-30T10:00:00Z", + "updated_at": "2026-06-30T10:00:00Z", + } + }, + ) diff --git a/src/services/course_filter_service.py b/src/services/course_filter_service.py new file mode 100644 index 0000000..f1ece4e --- /dev/null +++ b/src/services/course_filter_service.py @@ -0,0 +1,91 @@ +"""Course filter service for business logic.""" +import logging +from typing import Optional +from uuid import UUID + +from sqlalchemy.exc import IntegrityError +from sqlalchemy.orm import Session + +from src.core.exceptions import ConflictException, NotFoundException +from src.models.course_filter import CourseFilter +from src.repositories.course_filter_repository import CourseFilterRepository +from src.schemas.course_filter import CourseFilterCreate, CourseFilterUpdate + +logger = logging.getLogger(__name__) + + +class CourseFilterService: + """Service for course-filter business logic.""" + + def __init__(self, db: Session): + self.db = db + self.repo = CourseFilterRepository(db) + + def list_filters( + self, + skip: int = 0, + limit: int = 100, + search: Optional[str] = None, + ) -> tuple[list[CourseFilter], int]: + return self.repo.get_all_filtered(skip=skip, limit=limit, search=search) + + def get_filter(self, filter_id: str | UUID) -> CourseFilter: + instance = self.repo.get_by_id(filter_id) + if not instance: + raise NotFoundException(f"CourseFilter with ID {filter_id} not found") + return instance + + def create_filter(self, data: CourseFilterCreate) -> CourseFilter: + """Create a filter; rejects duplicate ``name`` with a 409. + + Vor-Check fängt den häufigen Fall sauber ab; die DB-Constraint bleibt + als Race-Schutz und wird ebenfalls auf 409 gemappt. + """ + if self.repo.get_by_name(data.name): + raise ConflictException(f"Course filter with name '{data.name}' already exists") + + try: + return self.repo.create(name=data.name) + except IntegrityError as e: + self.db.rollback() + # ``extra={"name": ...}`` would collide with ``LogRecord.name`` + # and raise inside logging.makeRecord. Use a distinct key. + logger.warning( + "Unique violation on course_filters.name (race)", + extra={"filter_name": data.name, "error": str(getattr(e, "orig", e))}, + ) + raise ConflictException( + f"Course filter with name '{data.name}' already exists" + ) + + def update_filter(self, filter_id: str | UUID, data: CourseFilterUpdate) -> CourseFilter: + instance = self.get_filter(filter_id) + + update_data = data.model_dump(exclude_unset=True) + if not update_data: + return instance + + new_name = update_data.get("name") + if new_name and 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) + except IntegrityError as e: + self.db.rollback() + logger.warning( + "Unique violation on course_filters.name during update (race)", + extra={"id": str(filter_id), "filter_name": new_name, "error": str(getattr(e, "orig", e))}, + ) + raise ConflictException( + f"Course filter with name '{new_name}' already exists" + ) + return updated or instance + + def delete_filter(self, filter_id: str | UUID) -> bool: + self.get_filter(filter_id) + return self.repo.delete(filter_id) diff --git a/tests/api/test_course_filter_routes.py b/tests/api/test_course_filter_routes.py new file mode 100644 index 0000000..3070b16 --- /dev/null +++ b/tests/api/test_course_filter_routes.py @@ -0,0 +1,361 @@ +"""Tests for /api/v1/course-filters endpoints. + +The resource is a flat admin-managed list of strings the frontend renders as +filter-chips above the course list. The interesting contract pieces: + +- Reads are open to any authenticated user (lecturer too — frontend needs them). +- Writes (POST/PATCH/DELETE) are ADMIN-only; a lecturer must get a 403. +- ``name`` is unique → duplicate create returns 409 (ConflictException), + the service falls back on IntegrityError if the pre-check races. +- 404 on unknown id for both PATCH and DELETE. + +We mock at the dependency level (DB session + ``get_current_user``) like the +sibling courses tests instead of spinning up a real DB. +""" +from datetime import datetime +from unittest.mock import MagicMock, patch +from uuid import uuid4 + +import pytest +from fastapi.testclient import TestClient +from sqlalchemy.exc import IntegrityError + +from src.core.dependencies import get_current_user, get_db +from src.main import app +from src.models.course_filter import CourseFilter +from src.models.user import UserRole + + +client = TestClient(app) + + +def _admin_user(): + return { + "sub": "admin-1", + "email": "admin@example.com", + "name": "Admin", + "preferred_username": "admin", + "roles": [UserRole.ADMIN.value], + "user_id": 1, + } + + +def _lecturer_user(): + return { + "sub": "lecturer-1", + "email": "lecturer@example.com", + "name": "Lecturer", + "preferred_username": "lecturer", + "roles": [UserRole.LECTURER.value], + "user_id": 2, + } + + +def _make_filter(name: str = "SQL", fid: str | None = None) -> CourseFilter: + """Build a ``CourseFilter`` row sufficient for response-validation. + + We use ``spec=CourseFilter`` so Pydantic's ``from_attributes`` sees the + expected fields without hitting the SQLAlchemy session. + """ + f = MagicMock(spec=CourseFilter) + f.id = fid or str(uuid4()) + f.name = name + f.created_at = datetime(2026, 6, 30, 10, 0, 0) + f.updated_at = datetime(2026, 6, 30, 10, 0, 0) + return f + + +@pytest.fixture(autouse=True) +def _clear_overrides(): + """Each test installs its own overrides; reset between cases.""" + yield + app.dependency_overrides.clear() + + +# ── list ────────────────────────────────────────────────────────────────────── + + +def test_list_filters_returns_paginated_payload(): + """A lecturer (non-admin) is allowed to read the list — the frontend chip + bar needs to render for every signed-in user.""" + rows = [_make_filter("SQL"), _make_filter("Web")] + + repo = MagicMock() + repo.get_all_filtered.return_value = (rows, len(rows)) + + app.dependency_overrides[get_current_user] = _lecturer_user + app.dependency_overrides[get_db] = lambda: MagicMock() + + with patch( + "src.services.course_filter_service.CourseFilterRepository", + return_value=repo, + ): + response = client.get("/api/v1/course-filters") + + assert response.status_code == 200 + body = response.json() + assert body["success"] is True + assert [item["name"] for item in body["data"]] == ["SQL", "Web"] + # Pagination block is present (ResponseBuilder.paginated contract). + assert body["pagination"]["total_items"] == 2 + + +def test_list_filters_passes_search_to_repo(): + """``?search=foo`` flows through to ``get_all_filtered(search="foo")`` so + the case-insensitive substring filter actually runs in the DB layer.""" + repo = MagicMock() + repo.get_all_filtered.return_value = ([], 0) + + 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.get("/api/v1/course-filters?search=sq") + + assert response.status_code == 200 + repo.get_all_filtered.assert_called_once() + assert repo.get_all_filtered.call_args.kwargs["search"] == "sq" + + +# ── create: admin path ──────────────────────────────────────────────────────── + + +def test_create_filter_admin_succeeds(): + """Happy path: admin posts a fresh name, repo creates and returns it.""" + created = _make_filter("SQL") + repo = MagicMock() + repo.get_by_name.return_value = None # not a duplicate + repo.create.return_value = created + + 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.post("/api/v1/course-filters", json={"name": "SQL"}) + + assert response.status_code == 201 + assert response.json()["data"]["name"] == "SQL" + repo.create.assert_called_once_with(name="SQL") + + +def test_create_filter_strips_whitespace(): + """``" SQL "`` → ``"SQL"``. The Pydantic validator trims before the + duplicate-check and the DB insert.""" + repo = MagicMock() + repo.get_by_name.return_value = None + repo.create.side_effect = lambda **kw: _make_filter(kw["name"]) + + 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.post( + "/api/v1/course-filters", json={"name": " SQL "} + ) + + assert response.status_code == 201 + assert repo.create.call_args.kwargs["name"] == "SQL" + + +def test_create_filter_blank_name_returns_422(): + """An all-whitespace name fails Pydantic validation before reaching the + service — duplicate-check would otherwise incorrectly look up ``""``.""" + app.dependency_overrides[get_current_user] = _admin_user + app.dependency_overrides[get_db] = lambda: MagicMock() + + response = client.post("/api/v1/course-filters", json={"name": " "}) + assert response.status_code == 422 + + +def test_create_filter_duplicate_name_returns_409(): + """Pre-check path: the name already exists → ConflictException → 409.""" + repo = MagicMock() + repo.get_by_name.return_value = _make_filter("SQL") + + 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.post("/api/v1/course-filters", json={"name": "SQL"}) + + assert response.status_code == 409 + assert "already exists" in response.json()["detail"] + repo.create.assert_not_called() + + +def test_create_filter_race_integrity_error_returns_409(): + """Race path: pre-check returns None but the INSERT trips the unique + constraint (concurrent admin). Service must catch IntegrityError and map + to 409, not bubble a 500.""" + repo = MagicMock() + repo.get_by_name.return_value = None + repo.create.side_effect = IntegrityError("INSERT", {}, Exception("dup")) + + 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.post("/api/v1/course-filters", json={"name": "SQL"}) + + assert response.status_code == 409 + + +# ── create: non-admin is rejected ───────────────────────────────────────────── + + +def test_create_filter_lecturer_returns_403(): + """Endpoint-level ``require_roles(ADMIN)`` blocks lecturers from writing.""" + app.dependency_overrides[get_current_user] = _lecturer_user + app.dependency_overrides[get_db] = lambda: MagicMock() + + response = client.post("/api/v1/course-filters", json={"name": "SQL"}) + assert response.status_code == 403 + + +# ── update: admin path ──────────────────────────────────────────────────────── + + +def test_update_filter_admin_renames(): + fid = str(uuid4()) + existing = _make_filter("SQL", fid=fid) + renamed = _make_filter("SQL Grundlagen", fid=fid) + + repo = MagicMock() + # get_by_id is called twice: get_filter() and then update()'s internal + # get_by_id. Return the same row both times. + repo.get_by_id.return_value = existing + repo.get_by_name.return_value = None # new name is free + repo.update.return_value = renamed + + 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 Grundlagen"}, + ) + + assert response.status_code == 200 + assert response.json()["data"]["name"] == "SQL Grundlagen" + + +def test_update_filter_to_existing_name_returns_409(): + """Renaming filter A to filter B's name must conflict.""" + fid_a = str(uuid4()) + fid_b = str(uuid4()) + a = _make_filter("Web", fid=fid_a) + b = _make_filter("SQL", fid=fid_b) + + repo = MagicMock() + repo.get_by_id.return_value = a + repo.get_by_name.return_value = b # taken by another row + + 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_a}", json={"name": "SQL"} + ) + + assert response.status_code == 409 + repo.update.assert_not_called() + + +def test_update_filter_unknown_id_returns_404(): + repo = MagicMock() + repo.get_by_id.return_value = None + + 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/{uuid4()}", json={"name": "new"} + ) + + assert response.status_code == 404 + + +def test_update_filter_lecturer_returns_403(): + app.dependency_overrides[get_current_user] = _lecturer_user + app.dependency_overrides[get_db] = lambda: MagicMock() + + response = client.patch( + f"/api/v1/course-filters/{uuid4()}", json={"name": "x"} + ) + assert response.status_code == 403 + + +# ── delete ──────────────────────────────────────────────────────────────────── + + +def test_delete_filter_admin_succeeds(): + fid = str(uuid4()) + repo = MagicMock() + repo.get_by_id.return_value = _make_filter("SQL", fid=fid) + repo.delete.return_value = True + + 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.delete(f"/api/v1/course-filters/{fid}") + + assert response.status_code == 200 + assert response.json()["success"] is True + # delete() accepts either positional or keyword id; just assert it was hit. + assert repo.delete.called + + +def test_delete_filter_unknown_id_returns_404(): + repo = MagicMock() + repo.get_by_id.return_value = None + + 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.delete(f"/api/v1/course-filters/{uuid4()}") + + assert response.status_code == 404 + repo.delete.assert_not_called() + + +def test_delete_filter_lecturer_returns_403(): + app.dependency_overrides[get_current_user] = _lecturer_user + app.dependency_overrides[get_db] = lambda: MagicMock() + + response = client.delete(f"/api/v1/course-filters/{uuid4()}") + assert response.status_code == 403 From aaa972716c697ce0e6a3e8f0e016dda95b725437 Mon Sep 17 00:00:00 2001 From: nicowre Date: Tue, 30 Jun 2026 13:07:13 +0200 Subject: [PATCH 2/2] fix(course-filters): tighten update/delete signatures to UUID mypy flagged str | UUID args being passed into BaseRepository.{update, delete}, which are annotated as UUID. The path parameters reaching these methods are already UUID (FastAPI parses them out of filter_id: UUID), so narrowing the service signatures matches the pattern in course_service and clears CI. --- src/services/course_filter_service.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/course_filter_service.py b/src/services/course_filter_service.py index f1ece4e..cfac42b 100644 --- a/src/services/course_filter_service.py +++ b/src/services/course_filter_service.py @@ -58,7 +58,7 @@ def create_filter(self, data: CourseFilterCreate) -> CourseFilter: f"Course filter with name '{data.name}' already exists" ) - def update_filter(self, filter_id: str | UUID, data: CourseFilterUpdate) -> 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) @@ -86,6 +86,6 @@ def update_filter(self, filter_id: str | UUID, data: CourseFilterUpdate) -> Cour ) return updated or instance - def delete_filter(self, filter_id: str | UUID) -> bool: + def delete_filter(self, filter_id: UUID) -> bool: self.get_filter(filter_id) return self.repo.delete(filter_id)