From 40ec9f1c49f6c550ae27a7bb0fbb0f3c8b631d77 Mon Sep 17 00:00:00 2001 From: Chris Date: Mon, 17 Aug 2026 20:16:21 -0700 Subject: [PATCH 1/2] Implement category management and improve project validation --- backend/app/main.py | 7 +- backend/app/models/project.py | 65 +++++++-- backend/app/routers/category_router.py | 15 ++ backend/app/routers/project_routers.py | 5 +- backend/app/schemas/project.py | 167 ++++++++++++++++++++--- backend/app/services/errors.py | 4 + backend/app/services/projects_service.py | 9 +- backend/pyproject.toml | 2 +- backend/tests/test_categories.py | 29 ++++ backend/tests/test_project_ownership.py | 30 ++++ backend/tests/test_project_validation.py | 141 ++++++++++++++++++- compose.yaml | 3 + 12 files changed, 432 insertions(+), 45 deletions(-) create mode 100644 backend/app/routers/category_router.py create mode 100644 backend/tests/test_categories.py diff --git a/backend/app/main.py b/backend/app/main.py index 3134189..dd0f3a9 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -9,6 +9,7 @@ from app.routers.project_routers import router as projects_router from app.routers.auth_router import router as auth_router from app.routers.profile_router import router as profile_router +from app.routers.category_router import router as categories_router from app.config import get_settings settings = get_settings() @@ -39,10 +40,8 @@ app.include_router(projects_router, prefix="/api") app.include_router(auth_router, prefix="/api") -app.include_router( - profile_router, - prefix="/api", -) +app.include_router(profile_router, prefix="/api") +app.include_router(categories_router, prefix="/api") @app.get("/api/health", tags=["system"]) diff --git a/backend/app/models/project.py b/backend/app/models/project.py index c4f1bb0..2460f82 100644 --- a/backend/app/models/project.py +++ b/backend/app/models/project.py @@ -1,9 +1,9 @@ from datetime import UTC, datetime from typing import TYPE_CHECKING +from uuid import UUID, uuid4 from sqlmodel import Field, Relationship from sqlmodel_toolkit import Model -from uuid import UUID, uuid4 if TYPE_CHECKING: from .category import Category @@ -20,35 +20,70 @@ class ProjectCategory(Model, table=True): __tablename__ = "project_categories" - project_id: UUID = Field(foreign_key="projects.id", primary_key=True, index=True) - category_id: UUID = Field(foreign_key="categories.id", primary_key=True, index=True) + project_id: UUID = Field( + foreign_key="projects.id", + primary_key=True, + index=True, + ) + category_id: UUID = Field( + foreign_key="categories.id", + primary_key=True, + index=True, + ) class Project(Model, table=True): - """A project that can be listed in the directory.""" + """A project listed in the Matrix directory.""" __tablename__ = "projects" id: UUID = Field(default_factory=uuid4, primary_key=True) - name: str = Field(max_length=100, index=True) - description: str - short_description: str = Field(max_length=240) + name: str = Field( + max_length=100, + index=True, + ) - repository_url: str | None = None - website_url: str | None = None - matrix_server_url: str | None = None + short_description: str = Field( + max_length=160, + ) - user_id: UUID = Field(foreign_key="users.id", index=True) + description: str = Field( + max_length=10_000, + ) - supports_e2ee: bool = False + repository_url: str | None = Field( + default=None, + max_length=255, + ) + website_url: str | None = Field( + default=None, + max_length=255, + ) + matrix_server_url: str | None = Field( + default=None, + max_length=255, + ) + + user_id: UUID = Field( + foreign_key="users.id", + index=True, + ) + + supports_e2ee: bool = Field(default=False) - owner: "User" = Relationship(back_populates="projects") + owner: "User" = Relationship( + back_populates="projects", + ) categories: list["Category"] = Relationship( back_populates="projects", link_model=ProjectCategory, ) - created_at: datetime = Field(default_factory=utc_now) - updated_at: datetime = Field(default_factory=utc_now) + created_at: datetime = Field( + default_factory=utc_now, + ) + updated_at: datetime = Field( + default_factory=utc_now, + ) diff --git a/backend/app/routers/category_router.py b/backend/app/routers/category_router.py new file mode 100644 index 0000000..1fb0597 --- /dev/null +++ b/backend/app/routers/category_router.py @@ -0,0 +1,15 @@ +from fastapi import APIRouter, Depends +from sqlmodel import Session, select + +from app.database import get_session +from app.models.category import Category +from app.schemas.category import CategoryRead + +router = APIRouter(prefix="/categories", tags=["categories"]) + + +@router.get("/", response_model=list[CategoryRead]) +def list_categories( + session: Session = Depends(get_session), +) -> list[Category]: + return list(session.exec(select(Category).order_by(Category.name)).all()) diff --git a/backend/app/routers/project_routers.py b/backend/app/routers/project_routers.py index 26ba635..5a4b5d5 100644 --- a/backend/app/routers/project_routers.py +++ b/backend/app/routers/project_routers.py @@ -79,7 +79,10 @@ def update_project( data, user_id=user.id, ) - except projects_service.CategoryNotFoundError as exc: + except ( + projects_service.CategoryNotFoundError, + projects_service.ProjectLinkRequiredError, + ) as exc: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail=str(exc), diff --git a/backend/app/schemas/project.py b/backend/app/schemas/project.py index ad72195..36633f8 100644 --- a/backend/app/schemas/project.py +++ b/backend/app/schemas/project.py @@ -1,51 +1,181 @@ +from datetime import datetime from typing import Any +from urllib.parse import urlsplit from uuid import UUID -from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator +from pydantic import ( + BaseModel, + ConfigDict, + Field, + field_validator, + model_validator, +) from .category import CategoryRead +def normalize_optional_http_url(value: Any) -> Any: + """Normalize blank URLs and reject non-HTTP(S) or relative URLs.""" + if value is None: + return None + + if not isinstance(value, str): + return value + + value = value.strip() + if not value: + return None + + parsed = urlsplit(value) + + if parsed.scheme not in {"http", "https"} or parsed.hostname is None: + raise ValueError("Must be an absolute HTTP or HTTPS URL") + + return value + + class ProjectCreate(BaseModel): """Schema for creating a new project.""" - name: str = Field(min_length=2, max_length=100) - description: str - short_description: str = Field(max_length=240) + name: str = Field( + min_length=2, + max_length=100, + ) + short_description: str = Field( + min_length=1, + max_length=160, + ) + description: str = Field( + min_length=1, + max_length=10_000, + ) - repository_url: str | None = None - website_url: str | None = None - matrix_server_url: str | None = None + repository_url: str | None = Field(default=None, max_length=255) + website_url: str | None = Field(default=None, max_length=255) + matrix_server_url: str | None = Field(default=None, max_length=255) supports_e2ee: bool = False category_ids: list[UUID] = Field(min_length=1) + @field_validator( + "name", + "short_description", + "description", + mode="before", + ) + @classmethod + def strip_required_text(cls, value: Any) -> Any: + if isinstance(value, str): + return value.strip() + + return value + + @field_validator( + "repository_url", + "website_url", + "matrix_server_url", + mode="before", + ) + @classmethod + def validate_optional_urls(cls, value: Any) -> Any: + return normalize_optional_http_url(value) + + @field_validator("category_ids") + @classmethod + def reject_duplicate_categories( + cls, + value: list[UUID], + ) -> list[UUID]: + if len(value) != len(set(value)): + raise ValueError("Categories must be unique") + + return value + + @model_validator(mode="after") + def require_project_link(self) -> "ProjectCreate": + """Require somewhere users can learn more about the project.""" + if self.repository_url is None and self.website_url is None: + raise ValueError("Provide at least a repository URL or website URL") + + return self + class ProjectUpdate(BaseModel): - name: str | None = Field(default=None, min_length=2, max_length=100) - description: str | None = None - short_description: str | None = Field(default=None, max_length=240) + """Schema for partially updating a project.""" + + name: str | None = Field( + default=None, + min_length=2, + max_length=100, + ) + short_description: str | None = Field( + default=None, + min_length=1, + max_length=160, + ) + description: str | None = Field( + default=None, + min_length=1, + max_length=10_000, + ) - repository_url: str | None = None - website_url: str | None = None - matrix_server_url: str | None = None + repository_url: str | None = Field(default=None, max_length=255) + website_url: str | None = Field(default=None, max_length=255) + matrix_server_url: str | None = Field(default=None, max_length=255) supports_e2ee: bool | None = None - category_ids: list[UUID] | None = Field(default=None, min_length=1) + category_ids: list[UUID] | None = Field( + default=None, + min_length=1, + ) @field_validator( "name", + "short_description", "description", + mode="before", + ) + @classmethod + def strip_updated_text(cls, value: Any) -> Any: + if isinstance(value, str): + return value.strip() + + return value + + @field_validator( + "repository_url", + "website_url", + "matrix_server_url", + mode="before", + ) + @classmethod + def validate_optional_urls(cls, value: Any) -> Any: + return normalize_optional_http_url(value) + + @field_validator("category_ids") + @classmethod + def reject_duplicate_categories( + cls, + value: list[UUID] | None, + ) -> list[UUID] | None: + if value is not None and len(value) != len(set(value)): + raise ValueError("Categories must be unique") + + return value + + @field_validator( + "name", "short_description", + "description", "supports_e2ee", "category_ids", mode="before", ) @classmethod def reject_explicit_null(cls, value: Any) -> Any: - """Reject null for required fields while allowing them to be omitted.""" + """Reject null for required fields while allowing omission.""" if value is None: raise ValueError("Field cannot be null") @@ -79,13 +209,15 @@ def from_user(cls, value: Any) -> Any: class ProjectRead(BaseModel): + """Public representation of a directory project.""" + model_config = ConfigDict(from_attributes=True) id: UUID name: str - description: str short_description: str + description: str repository_url: str | None website_url: str | None @@ -97,3 +229,6 @@ class ProjectRead(BaseModel): owner: ProjectOwnerRead categories: list[CategoryRead] = Field(default_factory=list) + + created_at: datetime + updated_at: datetime diff --git a/backend/app/services/errors.py b/backend/app/services/errors.py index 2ede0fd..3b439fd 100644 --- a/backend/app/services/errors.py +++ b/backend/app/services/errors.py @@ -1,2 +1,6 @@ class CategoryNotFoundError(Exception): pass + + +class ProjectLinkRequiredError(Exception): + pass diff --git a/backend/app/services/projects_service.py b/backend/app/services/projects_service.py index a67120c..222e336 100644 --- a/backend/app/services/projects_service.py +++ b/backend/app/services/projects_service.py @@ -7,7 +7,7 @@ from app.models.project import Project from app.schemas.project import ProjectCreate, ProjectUpdate -from .errors import CategoryNotFoundError +from .errors import CategoryNotFoundError, ProjectLinkRequiredError def list_projects(session: Session) -> list[Project]: @@ -74,6 +74,13 @@ def update_project( exclude={"category_ids"}, ) + repository_url = update_data.get("repository_url", project.repository_url) + website_url = update_data.get("website_url", project.website_url) + if repository_url is None and website_url is None: + raise ProjectLinkRequiredError( + "Provide at least a repository URL or website URL" + ) + for field, value in update_data.items(): setattr(project, field, value) diff --git a/backend/pyproject.toml b/backend/pyproject.toml index 3204f11..34187c8 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -53,7 +53,7 @@ addopts = [ "--strict-markers", "--cov=app", "--cov-report=term-missing", - "--cov-fail-under=65", + "--cov-fail-under=75", ] filterwarnings = [ "error", diff --git a/backend/tests/test_categories.py b/backend/tests/test_categories.py new file mode 100644 index 0000000..7803144 --- /dev/null +++ b/backend/tests/test_categories.py @@ -0,0 +1,29 @@ +from fastapi import FastAPI +from fastapi.testclient import TestClient +from sqlmodel import Session + +from app.database import get_session +from app.models.category import Category +from app.routers.category_router import router + + +def test_list_categories__expect_alphabetical_results(session: Session) -> None: + session.add_all( + [ + Category(name="Utility"), + Category(name="Dev tools"), + ] + ) + session.commit() + + app = FastAPI() + app.include_router(router, prefix="/api") + app.dependency_overrides[get_session] = lambda: session + + response = TestClient(app).get("/api/categories/") + + assert response.status_code == 200 + assert [category["name"] for category in response.json()] == [ + "Dev tools", + "Utility", + ] diff --git a/backend/tests/test_project_ownership.py b/backend/tests/test_project_ownership.py index c033758..b101d1b 100644 --- a/backend/tests/test_project_ownership.py +++ b/backend/tests/test_project_ownership.py @@ -1,9 +1,11 @@ +import pytest from sqlmodel import Session from app.models.category import Category from app.models.project import Project from app.models.user import User from app.schemas.project import ProjectUpdate +from app.services.errors import ProjectLinkRequiredError from app.services.projects_service import delete_project, update_project @@ -37,3 +39,31 @@ def test_project_writes__expect_only_owner_allowed(session: Session) -> None: assert delete_project(session, project.id, user_id=owner.id) assert session.get(Project, project.id) is None + + +def test_project_update__expect_at_least_one_project_link(session: Session) -> None: + owner = User(oidc_issuer="issuer", oidc_subject="owner") + category = Category(name="Bots") + session.add_all([owner, category]) + session.flush() + project = Project( + name="Test Bot", + description="Test bot", + short_description="Test bot", + repository_url="https://example.com/repository", + user_id=owner.id, + categories=[category], + ) + session.add(project) + session.commit() + + with pytest.raises(ProjectLinkRequiredError): + update_project( + session, + project.id, + ProjectUpdate(repository_url=None), + user_id=owner.id, + ) + + session.refresh(project) + assert project.repository_url == "https://example.com/repository" diff --git a/backend/tests/test_project_validation.py b/backend/tests/test_project_validation.py index 8a88c08..5f7f132 100644 --- a/backend/tests/test_project_validation.py +++ b/backend/tests/test_project_validation.py @@ -1,4 +1,6 @@ from collections.abc import Generator +from dataclasses import dataclass +from uuid import UUID from uuid import uuid4 import pytest @@ -10,13 +12,21 @@ from app.database import get_session from app.dependencies import get_current_user +from app.models.category import Category from app.models.user import User from app.routers.project_routers import router -from app.schemas.project import ProjectUpdate +from app.schemas.project import ProjectCreate, ProjectUpdate + + +@dataclass(frozen=True) +class ProjectClient: + client: TestClient + user_id: UUID + category_id: UUID @pytest.fixture -def client() -> Generator[TestClient, None, None]: +def project_client() -> Generator[ProjectClient, None, None]: engine = create_engine( "sqlite://", connect_args={"check_same_thread": False}, @@ -27,6 +37,15 @@ def client() -> Generator[TestClient, None, None]: app = FastAPI() app.include_router(router, prefix="/api") user = User(oidc_issuer="issuer", oidc_subject="owner") + category = Category(name="Bots") + + with Session(engine) as setup_session: + setup_session.add_all([user, category]) + setup_session.commit() + setup_session.refresh(user) + setup_session.refresh(category) + user_id = user.id + category_id = category.id def override_session() -> Generator[Session, None, None]: with Session(engine) as session: @@ -40,15 +59,15 @@ def override_current_user() -> User: try: with TestClient(app) as test_client: - yield test_client + yield ProjectClient(test_client, user_id, category_id) finally: engine.dispose() def test_create_with_no_categories__expect_validation_error( - client: TestClient, + project_client: ProjectClient, ) -> None: - response = client.post( + response = project_client.client.post( "/api/projects/", json={ "name": "Test Bot", @@ -62,10 +81,32 @@ def test_create_with_no_categories__expect_validation_error( assert response.json()["detail"][0]["loc"] == ["body", "category_ids"] +def test_create__expect_project_associated_with_authenticated_user( + project_client: ProjectClient, +) -> None: + response = project_client.client.post( + "/api/projects/", + json={ + "name": "Test Bot", + "description": "A useful bot.", + "short_description": "Useful bot", + "repository_url": "https://example.com/test-bot", + "category_ids": [str(project_client.category_id)], + }, + ) + + assert response.status_code == 201 + body = response.json() + assert body["user_id"] == str(project_client.user_id) + assert [category["id"] for category in body["categories"]] == [ + str(project_client.category_id) + ] + + def test_update_with_no_categories__expect_validation_error( - client: TestClient, + project_client: ProjectClient, ) -> None: - response = client.patch( + response = project_client.client.patch( f"/api/projects/{uuid4()}", json={"category_ids": []}, ) @@ -110,3 +151,89 @@ def test_update_with_omitted_fields__expect_only_supplied_fields_set() -> None: update = ProjectUpdate(name="Renamed Bot") assert update.model_dump(exclude_unset=True) == {"name": "Renamed Bot"} + + +def test_create__expect_required_text_trimmed() -> None: + project = ProjectCreate( + name=" Test Bot ", + description=" A useful bot. ", + short_description=" Useful bot ", + repository_url="https://example.com/test-bot", + category_ids=[uuid4()], + ) + + assert project.name == "Test Bot" + assert project.description == "A useful bot." + assert project.short_description == "Useful bot" + + +@pytest.mark.parametrize( + "field", + ["name", "description", "short_description"], +) +def test_create_with_whitespace_only_text__expect_validation_error( + field: str, +) -> None: + data = { + "name": "Test Bot", + "description": "A useful bot.", + "short_description": "Useful bot", + "category_ids": [uuid4()], + } + data[field] = " " + + with pytest.raises(ValidationError): + ProjectCreate.model_validate(data) + + +@pytest.mark.parametrize( + "url", + ["matrix:roomid/example.org", "/relative", "javascript:alert(1)"], +) +def test_create_with_invalid_url__expect_validation_error(url: str) -> None: + with pytest.raises(ValidationError, match="absolute HTTP or HTTPS URL"): + ProjectCreate( + name="Test Bot", + description="A useful bot.", + short_description="Useful bot", + repository_url=url, + category_ids=[uuid4()], + ) + + +def test_create_with_overlong_url__expect_validation_error() -> None: + url = "https://example.com/" + "x" * 236 + + with pytest.raises(ValidationError): + ProjectCreate( + name="Test Bot", + description="A useful bot.", + short_description="Useful bot", + repository_url=url, + category_ids=[uuid4()], + ) + + +def test_create_with_blank_optional_url__expect_null() -> None: + project = ProjectCreate( + name="Test Bot", + description="A useful bot.", + short_description="Useful bot", + repository_url="https://example.com/test-bot", + website_url=" ", + category_ids=[uuid4()], + ) + + assert project.website_url is None + + +def test_create_with_duplicate_categories__expect_validation_error() -> None: + category_id = uuid4() + + with pytest.raises(ValidationError, match="Categories must be unique"): + ProjectCreate( + name="Test Bot", + description="A useful bot.", + short_description="Useful bot", + category_ids=[category_id, category_id], + ) diff --git a/compose.yaml b/compose.yaml index 9c766dc..3387dcd 100644 --- a/compose.yaml +++ b/compose.yaml @@ -41,6 +41,9 @@ services: frontend: build: ./frontend + command: > + sh -c "npm install && + npm run dev -- --host 0.0.0.0" environment: FRONTEND_ORIGIN: ${FRONTEND_ORIGIN:-http://localhost:5173} VITE_API_URL: /api From 05794ab5ef4cd2db00a0f214bcf7fd833fdb35da Mon Sep 17 00:00:00 2001 From: Chris Date: Tue, 18 Aug 2026 08:56:19 -0700 Subject: [PATCH 2/2] remove unnecessary check --- backend/app/schemas/project.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/backend/app/schemas/project.py b/backend/app/schemas/project.py index 36633f8..c140994 100644 --- a/backend/app/schemas/project.py +++ b/backend/app/schemas/project.py @@ -16,9 +16,6 @@ def normalize_optional_http_url(value: Any) -> Any: """Normalize blank URLs and reject non-HTTP(S) or relative URLs.""" - if value is None: - return None - if not isinstance(value, str): return value