From 0c00f9b4ab4c6163a6cd797f3966ad592ff2c38a Mon Sep 17 00:00:00 2001 From: Shaan-Shoukath Date: Thu, 3 Sep 2026 21:19:10 +0530 Subject: [PATCH 01/26] =?UTF-8?q?feat(platform):=20phase=200=20=E2=80=94?= =?UTF-8?q?=20CI=20test=20workflow,=20request-id=20logging,=20metrics,=20p?= =?UTF-8?q?erf=20close-out?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Shaan-Shoukath Co-Authored-By: Claude Fable 5.1 --- .github/workflows/release.yml | 9 + .github/workflows/tests.yml | 204 ++++++++++++++++++ AGENTS.md | 14 +- CLAUDE.md | 14 +- backend/apps/audit/services.py | 15 +- backend/apps/backup/settings_policy.py | 4 +- backend/apps/events/admin.py | 38 +--- backend/apps/events/services_series.py | 8 +- .../apps/events/services_series_organizers.py | 72 +++++++ backend/apps/operations/urls.py | 2 + backend/apps/operations/views_metrics.py | 166 ++++++++++++++ backend/config/celery.py | 4 + backend/config/celery_signals.py | 50 +++++ backend/config/log_setup.py | 88 ++++++++ backend/config/request_id.py | 63 ++++++ backend/config/settings.py | 37 +++- backend/requirements.txt | 2 + .../events/test_series_organizer_admin.py | 134 ++++++++++++ backend/tests/perf/__init__.py | 0 backend/tests/perf/test_list_query_budgets.py | 136 ++++++++++++ backend/tests/test_log_setup.py | 60 ++++++ backend/tests/test_metrics_endpoint.py | 62 ++++++ backend/tests/test_request_id.py | 115 ++++++++++ docker-compose.prod.yml | 2 +- docker-compose.yml | 2 +- docs/INVARIANTS.md | 13 ++ docs/PROJECT-HISTORY.md | 13 ++ docs/PROJECT-STATUS.md | 12 ++ docs/performance-and-supabase-report.md | 31 +++ docs/self-hosting.md | 6 +- frontend/openapi-schema.json | 31 +++ frontend/src/generated/api.ts | 1 + 32 files changed, 1355 insertions(+), 53 deletions(-) create mode 100644 .github/workflows/tests.yml create mode 100644 backend/apps/events/services_series_organizers.py create mode 100644 backend/apps/operations/views_metrics.py create mode 100644 backend/config/celery_signals.py create mode 100644 backend/config/log_setup.py create mode 100644 backend/config/request_id.py create mode 100644 backend/tests/events/test_series_organizer_admin.py create mode 100644 backend/tests/perf/__init__.py create mode 100644 backend/tests/perf/test_list_query_budgets.py create mode 100644 backend/tests/test_log_setup.py create mode 100644 backend/tests/test_metrics_endpoint.py create mode 100644 backend/tests/test_request_id.py diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 8d50bcdc..d1a2ad65 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -15,8 +15,17 @@ permissions: packages: write jobs: + # Images are never published from a red tree: the whole test workflow runs first and + # every job below waits on it. + tests: + if: github.ref == 'refs/heads/main' + uses: ./.github/workflows/tests.yml + permissions: + contents: read + prepare: if: github.ref == 'refs/heads/main' + needs: tests runs-on: ubuntu-latest outputs: base: ${{ steps.release.outputs.base }} diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 00000000..38dc058b --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,204 @@ +name: Tests + +# The suite that every pull request must pass, and that the release workflow depends on. +# Environment parity with scripts/dev-local.sh is deliberate: the host topology there is +# what developers run, and a CI job that reproduced a different one would report reds +# nobody can see locally (docs/DEV-WORKFLOW.md, "Docker vs host test split"). +on: + pull_request: + branches: [dev, main] + push: + branches: [dev] + workflow_call: + workflow_dispatch: + +permissions: + contents: read + +env: + PYTHON_VERSION: "3.12" + NODE_VERSION: "22" + +jobs: + docs-drift: + name: CLAUDE.md and AGENTS.md are one document + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - run: diff CLAUDE.md AGENTS.md + + frontend: + name: Frontend typecheck, unit tests, build + runs-on: ubuntu-latest + timeout-minutes: 20 + defaults: + run: + working-directory: frontend + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: ${{ env.NODE_VERSION }} + cache: npm + cache-dependency-path: frontend/package-lock.json + - run: npm ci + - run: npx tsc -b + - run: npm test + - run: npm run build + + backend-host: + # Mirrors `./scripts/dev-local.sh test`: Postgres, Redis and MinIO as services, the + # Django test process on the runner. tests/backup and tests/tenant_migration are + # excluded here and run in the job below with a version-matched Postgres client. + name: Backend suite (host topology) + runs-on: ubuntu-latest + timeout-minutes: 120 + services: + postgres: + image: postgres:16 + env: + POSTGRES_USER: makerspace + POSTGRES_PASSWORD: makerspace + POSTGRES_DB: makerspace_manager + ports: ["5432:5432"] + options: >- + --health-cmd "pg_isready -U makerspace" + --health-interval 5s --health-timeout 5s --health-retries 20 + redis: + image: redis:7-alpine + ports: ["6379:6379"] + options: >- + --health-cmd "redis-cli ping" --health-interval 5s --health-timeout 5s --health-retries 20 + # GitHub Actions does not support YAML anchors, so this block is repeated verbatim in + # backend-pg-client below; keep the two in step. + env: + DATABASE_URL: postgres://makerspace:makerspace@localhost:5432/makerspace_manager + CELERY_BROKER_URL: redis://localhost:6379/0 + AWS_S3_ENDPOINT_URL: http://localhost:9200 + AWS_S3_PUBLIC_ENDPOINT_URL: http://localhost:9200 + PUBLIC_IMAGE_BASE_URL: http://localhost:9200/public-images + AWS_ACCESS_KEY_ID: minioadmin + AWS_SECRET_ACCESS_KEY: minioadmin + AWS_STORAGE_BUCKET_NAME: evidence + PUBLIC_IMAGE_BUCKET: public-images + STORAGE_PRESIGN_METHOD: post + DEBUG: "True" + SECRET_KEY: ci-only-secret-key-not-for-production + ALLOWED_HOSTS: localhost,127.0.0.1 + CORS_ALLOWED_ORIGINS: http://localhost:5000,http://localhost:5173 + AUTH_COOKIE_SECURE: "False" + AUTH_COOKIE_SAMESITE: Lax + EMAIL_BACKEND: django.core.mail.backends.console.EmailBackend + PYTHONDONTWRITEBYTECODE: "1" + steps: + - uses: actions/checkout@v4 + - name: Start MinIO and create the buckets the compose stack creates + run: | + set -euo pipefail + docker run -d --name minio -p 9200:9000 minio/minio:latest server /data + for _ in $(seq 1 30); do + curl -sf http://localhost:9200/minio/health/live && break + sleep 1 + done + docker run --rm --network host --entrypoint sh minio/mc:latest -c ' + mc alias set local http://localhost:9200 minioadmin minioadmin && + mc mb --ignore-existing local/evidence && + mc version enable local/evidence && + mc anonymous set none local/evidence && + mc mb --ignore-existing local/public-images && + mc version enable local/public-images && + mc anonymous set download local/public-images' + - uses: actions/setup-python@v5 + with: + python-version: ${{ env.PYTHON_VERSION }} + cache: pip + cache-dependency-path: backend/requirements.txt + - run: pip install -r backend/requirements.txt + - name: Django system checks + working-directory: backend + run: python manage.py check + - name: pytest (excluding the Postgres-client-bound suites) + working-directory: backend + run: >- + pytest -q -p no:cacheprovider + --ignore=tests/backup --ignore=tests/tenant_migration + + backend-pg-client: + # tests/backup and tests/tenant_migration shell out to pg_dump/pg_restore and refuse + # unless the client MAJOR equals the server's (16). The runner gets postgresql-client-16 + # from PGDG so postgres_client.client_binary resolves /usr/lib/postgresql/16/bin. + name: Backup and tenant-migration suites (pg client 16) + runs-on: ubuntu-latest + timeout-minutes: 90 + services: + postgres: + image: postgres:16 + env: + POSTGRES_USER: makerspace + POSTGRES_PASSWORD: makerspace + POSTGRES_DB: makerspace_manager + ports: ["5432:5432"] + options: >- + --health-cmd "pg_isready -U makerspace" + --health-interval 5s --health-timeout 5s --health-retries 20 + redis: + image: redis:7-alpine + ports: ["6379:6379"] + env: + DATABASE_URL: postgres://makerspace:makerspace@localhost:5432/makerspace_manager + CELERY_BROKER_URL: redis://localhost:6379/0 + AWS_S3_ENDPOINT_URL: http://localhost:9200 + AWS_S3_PUBLIC_ENDPOINT_URL: http://localhost:9200 + PUBLIC_IMAGE_BASE_URL: http://localhost:9200/public-images + AWS_ACCESS_KEY_ID: minioadmin + AWS_SECRET_ACCESS_KEY: minioadmin + AWS_STORAGE_BUCKET_NAME: evidence + PUBLIC_IMAGE_BUCKET: public-images + STORAGE_PRESIGN_METHOD: post + DEBUG: "True" + SECRET_KEY: ci-only-secret-key-not-for-production + ALLOWED_HOSTS: localhost,127.0.0.1 + CORS_ALLOWED_ORIGINS: http://localhost:5000,http://localhost:5173 + AUTH_COOKIE_SECURE: "False" + AUTH_COOKIE_SAMESITE: Lax + EMAIL_BACKEND: django.core.mail.backends.console.EmailBackend + PYTHONDONTWRITEBYTECODE: "1" + steps: + - uses: actions/checkout@v4 + - name: Install postgresql-client-16 and age + run: | + set -euo pipefail + sudo install -d /usr/share/postgresql-common/pgdg + curl -fsSL https://www.postgresql.org/media/keys/ACCC4CF8.asc \ + | sudo gpg --dearmor -o /usr/share/postgresql-common/pgdg/apt.postgresql.org.gpg + . /etc/os-release + echo "deb [signed-by=/usr/share/postgresql-common/pgdg/apt.postgresql.org.gpg] https://apt.postgresql.org/pub/repos/apt ${VERSION_CODENAME}-pgdg main" \ + | sudo tee /etc/apt/sources.list.d/pgdg.list + sudo apt-get update + sudo apt-get install -y --no-install-recommends postgresql-client-16 age + /usr/lib/postgresql/16/bin/pg_dump --version + - name: Start MinIO and create buckets + run: | + set -euo pipefail + docker run -d --name minio -p 9200:9000 minio/minio:latest server /data + for _ in $(seq 1 30); do + curl -sf http://localhost:9200/minio/health/live && break + sleep 1 + done + docker run --rm --network host --entrypoint sh minio/mc:latest -c ' + mc alias set local http://localhost:9200 minioadmin minioadmin && + mc mb --ignore-existing local/evidence && + mc version enable local/evidence && + mc anonymous set none local/evidence && + mc mb --ignore-existing local/public-images && + mc version enable local/public-images && + mc anonymous set download local/public-images' + - uses: actions/setup-python@v5 + with: + python-version: ${{ env.PYTHON_VERSION }} + cache: pip + cache-dependency-path: backend/requirements.txt + - run: pip install -r backend/requirements.txt + - name: pytest tests/backup tests/tenant_migration + working-directory: backend + run: pytest -q -p no:cacheprovider tests/backup tests/tenant_migration diff --git a/AGENTS.md b/AGENTS.md index f214858a..4600598c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -150,12 +150,14 @@ the Auth module** — forgetting this is a cross-tenant data leak, not just a bu the original file as a **thin re-export barrel** (explicit `from .submodule import (...)`, never `import *`) so `from app.views import X` and `views.X` keep resolving; for `admin.py` the barrel must still import the admin submodules so the `@admin.register` side effects fire. **The ceiling is enforced - on what you touch, and it is NOT currently met repo-wide: 37 backend files exceed 300 lines** — largest - first, `config/settings.py` (929, the accepted exception — Django settings are conventionally a single - file), `admin_api/urls.py` (825), `makerspaces/models.py` (682), `accounts/rbac.py` (609), - `inventory/availability.py` (596), `admin_api/serializers_makerspaces.py` (561), - `makerspaces/module_registry.py` (503), `machines/role_scope.py` (489). Measured 2026-08-20; an earlier - version of this line claimed every file but `settings.py` was compliant, which was false by 36 files. + on what you touch, and it is nearly met repo-wide: five `backend/apps/` files (non-migration, non-test) + exceed 300 lines** — `machines/access.py` (367), `makerspaces/module_registry.py` (310), + `inventory/middleware.py` (308), `tenant_migration/tenant_dump_authority.py` (305), + `tenant_migration/source_gate_guards.py` (301) — plus `config/settings.py` (1081, the accepted + exception — Django settings are conventionally a single file). `backend/tests/` is not held to the + ceiling. Frontend: eleven non-test, non-generated files exceed it, largest + `features/staff/panels/Inventory.tsx` (499) and `lib/api.ts` (439). Measured 2026-09-03; the previous + version of this line (37 files, 2026-08-20) was already stale by the time it was read. **Split an over-ceiling file in its own commit before adding to it**, and when splitting one that other modules import from, check for guards pinned to its path: `tests/makerspaces/test_tenant_servability_guard.py` pins two function *bodies* to `accounts/rbac.py` by `(path, function)`, and diff --git a/CLAUDE.md b/CLAUDE.md index f214858a..4600598c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -150,12 +150,14 @@ the Auth module** — forgetting this is a cross-tenant data leak, not just a bu the original file as a **thin re-export barrel** (explicit `from .submodule import (...)`, never `import *`) so `from app.views import X` and `views.X` keep resolving; for `admin.py` the barrel must still import the admin submodules so the `@admin.register` side effects fire. **The ceiling is enforced - on what you touch, and it is NOT currently met repo-wide: 37 backend files exceed 300 lines** — largest - first, `config/settings.py` (929, the accepted exception — Django settings are conventionally a single - file), `admin_api/urls.py` (825), `makerspaces/models.py` (682), `accounts/rbac.py` (609), - `inventory/availability.py` (596), `admin_api/serializers_makerspaces.py` (561), - `makerspaces/module_registry.py` (503), `machines/role_scope.py` (489). Measured 2026-08-20; an earlier - version of this line claimed every file but `settings.py` was compliant, which was false by 36 files. + on what you touch, and it is nearly met repo-wide: five `backend/apps/` files (non-migration, non-test) + exceed 300 lines** — `machines/access.py` (367), `makerspaces/module_registry.py` (310), + `inventory/middleware.py` (308), `tenant_migration/tenant_dump_authority.py` (305), + `tenant_migration/source_gate_guards.py` (301) — plus `config/settings.py` (1081, the accepted + exception — Django settings are conventionally a single file). `backend/tests/` is not held to the + ceiling. Frontend: eleven non-test, non-generated files exceed it, largest + `features/staff/panels/Inventory.tsx` (499) and `lib/api.ts` (439). Measured 2026-09-03; the previous + version of this line (37 files, 2026-08-20) was already stale by the time it was read. **Split an over-ceiling file in its own commit before adding to it**, and when splitting one that other modules import from, check for guards pinned to its path: `tests/makerspaces/test_tenant_servability_guard.py` pins two function *bodies* to `accounts/rbac.py` by `(path, function)`, and diff --git a/backend/apps/audit/services.py b/backend/apps/audit/services.py index e8f8d648..0dd31a61 100644 --- a/backend/apps/audit/services.py +++ b/backend/apps/audit/services.py @@ -128,7 +128,7 @@ def record(actor, action, *, makerspace=None, target=None, target_type="", meta= created_at=created_at, ) - return AuditLog.objects.create( + row = AuditLog.objects.create( actor_id=actor_id, action=action, target_type=target_type, @@ -139,3 +139,16 @@ def record(actor, action, *, makerspace=None, target=None, target_type="", meta= row_mac=row_mac, created_at=created_at, ) + # Correlation lives in the log line, not in `meta`: the row's MAC covers meta, and the + # request id is an operational breadcrumb rather than part of the attested record. The + # log formatter adds request_id, so `grep ` finds the request that wrote it. + logger.info( + "audit_recorded", + extra={ + "audit_event_uuid": str(event_uuid), + "audit_action": action, + "makerspace_id": makerspace_id, + "attested": row_mac is not None, + }, + ) + return row diff --git a/backend/apps/backup/settings_policy.py b/backend/apps/backup/settings_policy.py index 195b1cd1..59de44c9 100644 --- a/backend/apps/backup/settings_policy.py +++ b/backend/apps/backup/settings_policy.py @@ -103,6 +103,8 @@ class SettingPolicy: BUILD_GIT_DESCRIBE BUILD_GIT_SHA SETUP_MAKERSPACE_NAME SETUP_MAKERSPACE_SLUG SETUP_MODULE_PROFILE SETUP_SUPERADMIN_EMAIL SETUP_SUPERADMIN_PASSWORD SETUP_SUPERADMIN_USERNAME SPACEWORKS_OCI_DIGEST TOMBSTONED_APPS +LOG_LEVEL LOG_JSON METRICS_TOKEN SENTRY_DSN SENTRY_TRACES_SAMPLE_RATE SENTRY_ENVIRONMENT +CONN_HEALTH_CHECKS """.split()) # The source-gate lease and presign-drain settings are portable operational timing @@ -157,7 +159,7 @@ class SettingPolicy: secret_bearing=name in EXACT or name in { "AWS_SECRET_ACCESS_KEY", "EMAIL_HOST_PASSWORD", "CELERY_BROKER_URL", "CELERY_RESULT_BACKEND", "DATABASE_URL", "SPACEWORKS_RUNTIME_DATABASE_URL", - "BACKUP_ARCHIVE_SIGNING_PRIVATE_KEY", + "BACKUP_ARCHIVE_SIGNING_PRIVATE_KEY", "METRICS_TOKEN", "SENTRY_DSN", }, blocks_restore=name in EXACT or name in CAPABILITY or name in BLOCKING_VALUE, ) diff --git a/backend/apps/events/admin.py b/backend/apps/events/admin.py index f2a1f66b..f99d8295 100644 --- a/backend/apps/events/admin.py +++ b/backend/apps/events/admin.py @@ -2,7 +2,7 @@ from unfold.admin import ModelAdmin from apps.accounts import rbac -from apps.audit import services as audit +from apps.events import services_series_organizers from apps.events.models import Event, EventOrganizer, EventSeries, EventSeriesOrganizer from apps.separability.tombstones import app_is_tombstoned from config.admin_access import SuperuserOnlyModelAdmin @@ -65,42 +65,20 @@ def formfield_for_foreignkey(self, db_field, request, **kwargs): ) return super().formfield_for_foreignkey(db_field, request, **kwargs) + # Superadmin operations route through services, never the ORM: the service takes the + # series row lock and the events module lock, checks authority, projects the organizer + # onto every occurrence and writes the audit entry. The admin only chooses the rows. def save_model(self, request, obj, form, change): - if not change: - obj.created_by = request.user - super().save_model(request, obj, form, change) - for event in obj.series.occurrences.all(): - EventOrganizer.objects.get_or_create( - event=event, - organization=obj.organization, - defaults={"created_by": request.user, "source_series_organizer": obj}, - ) - audit.record( - request.user, - "event.series_organizer_created" if not change else "event.series_organizer_updated", - makerspace=obj.series.makerspace, - target=obj, - meta={"series_id": obj.series_id, "organization_slug": obj.organization.slug}, + services_series_organizers.add_series_organizer( + obj.series, actor=request.user, organization=obj.organization ) def delete_model(self, request, obj): - EventOrganizer.objects.filter(source_series_organizer=obj).delete() - audit.record( - request.user, "event.series_organizer_deleted", - makerspace=obj.series.makerspace, target=obj, - meta={"series_id": obj.series_id, "organization_slug": obj.organization.slug}, - ) - super().delete_model(request, obj) + services_series_organizers.remove_series_organizer(obj, actor=request.user) def delete_queryset(self, request, queryset): for obj in queryset.select_related("series__makerspace", "organization"): - EventOrganizer.objects.filter(source_series_organizer=obj).delete() - audit.record( - request.user, "event.series_organizer_deleted", - makerspace=obj.series.makerspace, target=obj, - meta={"series_id": obj.series_id, "organization_slug": obj.organization.slug}, - ) - super().delete_queryset(request, queryset) + services_series_organizers.remove_series_organizer(obj, actor=request.user) if not app_is_tombstoned("events"): diff --git a/backend/apps/events/services_series.py b/backend/apps/events/services_series.py index 7473ef47..cc711245 100644 --- a/backend/apps/events/services_series.py +++ b/backend/apps/events/services_series.py @@ -117,12 +117,10 @@ def _project_authority(series, event): "source_series_collaboration": source, }, ) + from apps.events.services_series_organizers import project_organizer + for source in series.organizers.all(): - EventOrganizer.objects.get_or_create( - event=event, - organization=source.organization, - defaults={"created_by": source.created_by, "source_series_organizer": source}, - ) + project_organizer(source, [event]) def _materialize_locked(series, *, actor, now): diff --git a/backend/apps/events/services_series_organizers.py b/backend/apps/events/services_series_organizers.py new file mode 100644 index 00000000..b06099ed --- /dev/null +++ b/backend/apps/events/services_series_organizers.py @@ -0,0 +1,72 @@ +"""Series-level organizer changes and their projection onto occurrences. + +Before this module the superadmin console added and removed ``EventSeriesOrganizer`` rows +directly in ``admin.py`` and re-implemented the occurrence projection inline, so the admin +path skipped the module lock and the authority check that ``services_series`` applies when +it materializes a series. Superadmin operations must route through services, never through +the ORM (``docs/PROJECT-STATUS.md``); this is the service. +""" +from django.core.exceptions import PermissionDenied +from django.db import transaction + +from apps.audit import services as audit +from apps.events.models_series import EventSeries, EventSeriesOrganizer +from apps.events.organizer_models import EventOrganizer +from apps.events.series_authority import can_manage_series +from apps.makerspaces.guards import require_module_locked + + +def project_organizer(source, events): + """Create the per-occurrence organizer rows a series organizer implies.""" + for event in events: + EventOrganizer.objects.get_or_create( + event=event, + organization=source.organization, + defaults={"created_by": source.created_by, "source_series_organizer": source}, + ) + + +def _lock(series): + locked = EventSeries.objects.select_for_update().get(pk=series.pk) + require_module_locked(locked.makerspace_id, "events") + return locked + + +def _meta(series, organization): + return {"series_id": series.pk, "organization_slug": organization.slug} + + +@transaction.atomic +def add_series_organizer(series, *, actor, organization): + locked = _lock(series) + if not can_manage_series(actor, locked): + raise PermissionDenied() + row, created = EventSeriesOrganizer.objects.get_or_create( + series=locked, organization=organization, defaults={"created_by": actor} + ) + project_organizer(row, locked.occurrences.all()) + audit.record( + actor, + "event.series_organizer_created" if created else "event.series_organizer_updated", + makerspace=locked.makerspace, + target=row, + meta=_meta(locked, organization), + ) + return row + + +@transaction.atomic +def remove_series_organizer(row, *, actor): + locked = _lock(row.series) + if not can_manage_series(actor, locked): + raise PermissionDenied() + organization = row.organization + EventOrganizer.objects.filter(source_series_organizer=row).delete() + audit.record( + actor, + "event.series_organizer_deleted", + makerspace=locked.makerspace, + target=row, + meta=_meta(locked, organization), + ) + row.delete() diff --git a/backend/apps/operations/urls.py b/backend/apps/operations/urls.py index 0b37ad75..24fd25b7 100644 --- a/backend/apps/operations/urls.py +++ b/backend/apps/operations/urls.py @@ -1,10 +1,12 @@ from django.urls import path from apps.operations import views +from apps.operations.views_metrics import MetricsView urlpatterns = [ path("health/", views.HealthView.as_view(), name="health"), path("health/readiness/", views.ReadinessView.as_view(), name="readiness"), + path("metrics/", MetricsView.as_view(), name="metrics"), path("admin/makerspace//dashboard", views.DashboardView.as_view(), name="operations-dashboard"), path("admin/makerspace//containers", views.ContainerListCreateView.as_view(), name="containers"), path("admin/containers/", views.ContainerDetailView.as_view(), name="container-detail"), diff --git a/backend/apps/operations/views_metrics.py b/backend/apps/operations/views_metrics.py new file mode 100644 index 00000000..703e4731 --- /dev/null +++ b/backend/apps/operations/views_metrics.py @@ -0,0 +1,166 @@ +"""Prometheus text exposition for the deployment. + +Deliberately dependency-free: the exposition format is a handful of lines and pulling in a +metrics library for six gauges would add a process-wide registry the tests then have to +reset. Everything here is a point-in-time read of state the platform already keeps. + +Access is a static bearer token. With no token configured the route answers 404, not 401, +so an unconfigured deployment does not even advertise that the surface exists. +""" +from datetime import timedelta + +from django.conf import settings +from django.db.models import Count, Max, Sum +from django.http import Http404, HttpResponse +from django.utils import timezone +from django.utils.crypto import constant_time_compare +from drf_spectacular.types import OpenApiTypes +from drf_spectacular.utils import extend_schema +from rest_framework.permissions import AllowAny +from rest_framework.views import APIView + +CONTENT_TYPE = "text/plain; version=0.0.4; charset=utf-8" + + +def _escape_label(value): + return str(value).replace("\\", "\\\\").replace('"', '\\"').replace("\n", "\\n") + + +class _Exposition: + def __init__(self): + self.lines = [] + + def gauge(self, name, help_text, samples): + """``samples`` is an iterable of (labels dict, value).""" + self.lines.append(f"# HELP {name} {help_text}") + self.lines.append(f"# TYPE {name} gauge") + for labels, value in samples: + rendered = ",".join( + f'{key}="{_escape_label(val)}"' for key, val in sorted(labels.items()) + ) + suffix = f"{{{rendered}}}" if rendered else "" + self.lines.append(f"{name}{suffix} {value}") + + def render(self): + return "\n".join(self.lines) + "\n" + + +def _celery_queue_lengths(): + """Queue depth from the broker; empty when the deployment runs tasks eagerly.""" + if getattr(settings, "CELERY_TASK_ALWAYS_EAGER", False): + return [] + try: + import redis + except ImportError: # pragma: no cover - redis is a hard requirement in practice + return [] + queue = getattr(settings, "CELERY_TASK_DEFAULT_QUEUE", None) or "celery" + try: + client = redis.Redis.from_url(settings.CELERY_BROKER_URL, socket_timeout=1) + return [({"queue": queue}, int(client.llen(queue)))] + except Exception: # broker unreachable: report nothing rather than fail the scrape + return [] + + +def _collect(): + from apps.evidence.retention_models import EvidenceObjectRetentionState + from apps.hardware_requests.models import HardwareRequest + from apps.integrations.models import EmailLog, NotificationDeliveryLog + from apps.makerspaces.models import Makerspace + + since = timezone.now() - timedelta(hours=1) + out = _Exposition() + + out.gauge( + "spaceworks_celery_queue_length", + "Messages waiting in the Celery broker queue.", + _celery_queue_lengths(), + ) + out.gauge( + "spaceworks_hardware_requests", + "Hardware requests by workflow status.", + [ + ({"status": row["status"]}, row["n"]) + for row in HardwareRequest.objects.values("status").annotate(n=Count("id")) + ], + ) + deliveries = [ + ({"channel": row["channel"], "status": row["status"]}, row["n"]) + for row in NotificationDeliveryLog.objects.filter(created_at__gte=since) + .values("channel", "status") + .annotate(n=Count("id")) + ] + deliveries += [ + ({"channel": "email", "status": row["status"]}, row["n"]) + for row in EmailLog.objects.filter(created_at__gte=since) + .values("status") + .annotate(n=Count("id")) + ] + out.gauge( + "spaceworks_notification_deliveries_last_hour", + "Notification delivery attempts in the last hour by channel and status.", + deliveries, + ) + out.gauge( + "spaceworks_storage_bytes_used", + "Managed object storage accounted to each makerspace.", + [ + ({"makerspace_id": row["id"]}, row["storage_bytes_used"]) + for row in Makerspace.objects.values("id", "storage_bytes_used") + ], + ) + out.gauge( + "spaceworks_storage_bytes_used_total", + "Managed object storage accounted across all makerspaces.", + [({}, Makerspace.objects.aggregate(total=Sum("storage_bytes_used"))["total"] or 0)], + ) + out.gauge( + "spaceworks_evidence_retention_states", + "Evidence photos by object-retention state.", + [ + ({"status": row["status"]}, row["n"]) + for row in EvidenceObjectRetentionState.objects.values("status").annotate(n=Count("id")) + ], + ) + last_expiry = EvidenceObjectRetentionState.objects.aggregate( + latest=Max("object_expired_at") + )["latest"] + out.gauge( + "spaceworks_evidence_retention_last_expiry_timestamp_seconds", + "Unix time of the most recent evidence object expiry (0 when none).", + [({}, int(last_expiry.timestamp()) if last_expiry else 0)], + ) + return out.render() + + +def _presented_token(request): + header = request.headers.get("Authorization", "") + if header.startswith("Bearer "): + return header[len("Bearer "):].strip() + return request.headers.get("X-Metrics-Token", "") + + +class MetricsView(APIView): + authentication_classes = [] + permission_classes = [AllowAny] + throttle_classes = [] + + @extend_schema( + tags=["Health"], + summary="Prometheus metrics", + description=( + "Prometheus text exposition of queue depth, request states, notification " + "delivery outcomes, storage accounting and evidence retention. Requires the " + "deployment's METRICS_TOKEN as a bearer token; 404 when no token is configured." + ), + request=None, + responses={(200, "text/plain"): OpenApiTypes.STR, 401: None, 404: None}, + ) + def get(self, request, *args, **kwargs): + expected = getattr(settings, "METRICS_TOKEN", "") + if not expected: + raise Http404 + if not constant_time_compare(_presented_token(request), expected): + response = HttpResponse("unauthorized\n", status=401, content_type="text/plain") + response["WWW-Authenticate"] = "Bearer" + return response + return HttpResponse(_collect(), content_type=CONTENT_TYPE) diff --git a/backend/config/celery.py b/backend/config/celery.py index 691493b7..236da1df 100644 --- a/backend/config/celery.py +++ b/backend/config/celery.py @@ -9,3 +9,7 @@ ) app.config_from_object("django.conf:settings", namespace="CELERY") app.autodiscover_tasks() + +# Registers the before_task_publish / task_prerun handlers that carry the request id +# from the web process into the worker. Import for its side effect; nothing to call. +import config.celery_signals # noqa: E402,F401 diff --git a/backend/config/celery_signals.py b/backend/config/celery_signals.py new file mode 100644 index 00000000..e3d0a2b5 --- /dev/null +++ b/backend/config/celery_signals.py @@ -0,0 +1,50 @@ +"""Carry the request id across the Celery boundary. + +``before_task_publish`` runs in the web process with the request's contextvar still bound, +so the id is copied into the message headers. ``task_prerun`` runs in the worker, where +Celery exposes custom headers as attributes on ``task.request``; the id is rebound there so +every log line the task emits shares it with the request that enqueued the work. + +Eager execution (``CELERY_TASK_ALWAYS_EAGER``) never publishes, but it also never leaves the +thread, so the contextvar is simply inherited and these handlers are harmless no-ops. +""" +from celery.signals import before_task_publish, task_postrun, task_prerun + +from config.request_id import get_request_id, reset_request_id, set_request_id + +HEADER = "spaceworks_request_id" +_TOKEN_ATTR = "_spaceworks_request_id_token" + + +@before_task_publish.connect +def propagate_request_id(headers=None, **_kwargs): + request_id = get_request_id() + if request_id and headers is not None: + headers.setdefault(HEADER, request_id) + + +def _header_from_task(task): + request = getattr(task, "request", None) + if request is None: + return None + value = getattr(request, HEADER, None) + if value: + return value + raw_headers = getattr(request, "headers", None) or {} + return raw_headers.get(HEADER) if isinstance(raw_headers, dict) else None + + +@task_prerun.connect +def bind_request_id(task=None, **_kwargs): + request_id = _header_from_task(task) + if request_id: + setattr(task.request, _TOKEN_ATTR, set_request_id(request_id)) + + +@task_postrun.connect +def unbind_request_id(task=None, **_kwargs): + request = getattr(task, "request", None) + token = getattr(request, _TOKEN_ATTR, None) if request is not None else None + if token is not None: + reset_request_id(token) + delattr(request, _TOKEN_ATTR) diff --git a/backend/config/log_setup.py b/backend/config/log_setup.py new file mode 100644 index 00000000..4b9cc8b7 --- /dev/null +++ b/backend/config/log_setup.py @@ -0,0 +1,88 @@ +"""Structured logging configuration. + +Production emits one JSON object per line so a log shipper can index by ``request_id``, +``logger`` or any ``extra=`` key without regex; local development keeps the plain +single-line format because a human is reading it. Both carry the request id from +``config.request_id`` so the two formats differ only in shape, never in content. + +Named ``log_setup`` rather than ``logging`` on purpose: a module called ``config.logging`` +is one careless ``sys.path`` entry away from shadowing the standard library. +""" +import json +import logging +from datetime import UTC, datetime + +from config.request_id import get_request_id + +# Attributes every LogRecord carries. Anything else on the record came from ``extra=`` and +# is worth surfacing as its own JSON key. +_STANDARD_RECORD_ATTRS = frozenset( + { + "args", "asctime", "created", "exc_info", "exc_text", "filename", "funcName", + "levelname", "levelno", "lineno", "message", "module", "msecs", "msg", "name", + "pathname", "process", "processName", "relativeCreated", "stack_info", "thread", + "threadName", "taskName", "request_id", + } +) + + +class RequestIdFilter(logging.Filter): + """Stamp the bound request id (or ``-``) on every record so formatters can rely on it.""" + + def filter(self, record): + record.request_id = get_request_id() or "-" + return True + + +class JsonFormatter(logging.Formatter): + def format(self, record): + payload = { + "ts": datetime.fromtimestamp(record.created, UTC).isoformat(timespec="milliseconds"), + "level": record.levelname, + "logger": record.name, + "message": record.getMessage(), + "request_id": getattr(record, "request_id", None) or "-", + } + for key, value in record.__dict__.items(): + if key not in _STANDARD_RECORD_ATTRS and not key.startswith("_"): + payload[key] = value + if record.exc_info: + payload["exception"] = self.formatException(record.exc_info) + if record.stack_info: + payload["stack"] = self.formatStack(record.stack_info) + return json.dumps(payload, default=str, ensure_ascii=False) + + +def build_logging(level: str, *, json_output: bool) -> dict: + formatter = "json" if json_output else "plain" + return { + "version": 1, + "disable_existing_loggers": False, + "filters": { + "request_id": {"()": "config.log_setup.RequestIdFilter"}, + }, + "formatters": { + "json": {"()": "config.log_setup.JsonFormatter"}, + "plain": { + "format": "%(asctime)s %(levelname)s %(name)s [%(request_id)s] %(message)s", + }, + }, + "handlers": { + "console": { + "class": "logging.StreamHandler", + "filters": ["request_id"], + "formatter": formatter, + }, + }, + "root": {"handlers": ["console"], "level": level}, + "loggers": { + # Django's own request/security loggers are noisy at DEBUG; hold them at the + # configured level but never below WARNING for the request logger, which + # otherwise duplicates every 4xx the view already reported. + "django": {"level": level, "propagate": True}, + "django.request": {"level": "WARNING", "propagate": True}, + "django.security": {"level": "WARNING", "propagate": True}, + "celery": {"level": level, "propagate": True}, + "apps": {"level": level, "propagate": True}, + }, + } diff --git a/backend/config/request_id.py b/backend/config/request_id.py new file mode 100644 index 00000000..5d6c217c --- /dev/null +++ b/backend/config/request_id.py @@ -0,0 +1,63 @@ +"""Per-request correlation id. + +One id follows a request through every log line it emits and into every Celery task it +enqueues, so a support question ("what happened when I pressed Issue at 14:02?") can be +answered by one grep instead of by guessing at timestamps. The id is stored in a +``contextvars.ContextVar`` rather than on the request object because the log formatter and +Celery signal handlers have no request to hand. + +The incoming ``X-Request-ID`` header is honoured only when it is short and plain ASCII: a +reverse proxy that already assigns ids should win, but a caller must not be able to inject +newlines or a kilobyte of junk into every log line. +""" +import contextvars +import re +import uuid + +REQUEST_ID_HEADER = "X-Request-ID" +_VALID_REQUEST_ID = re.compile(r"^[A-Za-z0-9_.:-]{1,64}$") + +_request_id: contextvars.ContextVar[str | None] = contextvars.ContextVar( + "spaceworks_request_id", default=None +) + + +def get_request_id() -> str | None: + return _request_id.get() + + +def set_request_id(value: str) -> contextvars.Token: + return _request_id.set(value) + + +def reset_request_id(token: contextvars.Token) -> None: + _request_id.reset(token) + + +def new_request_id() -> str: + return uuid.uuid4().hex + + +def normalize_request_id(candidate: str | None) -> str: + """Return the caller's id when it is safe to log, otherwise mint a fresh one.""" + if candidate and _VALID_REQUEST_ID.match(candidate): + return candidate + return new_request_id() + + +class RequestIdMiddleware: + """Bind a request id for the duration of the request and echo it on the response.""" + + def __init__(self, get_response): + self.get_response = get_response + + def __call__(self, request): + request_id = normalize_request_id(request.headers.get(REQUEST_ID_HEADER)) + request.request_id = request_id + token = set_request_id(request_id) + try: + response = self.get_response(request) + finally: + reset_request_id(token) + response[REQUEST_ID_HEADER] = request_id + return response diff --git a/backend/config/settings.py b/backend/config/settings.py index 13f65423..138f6092 100644 --- a/backend/config/settings.py +++ b/backend/config/settings.py @@ -7,6 +7,7 @@ from corsheaders.defaults import default_headers from django.core.exceptions import ImproperlyConfigured +from config.log_setup import build_logging from config.storage_validation import assert_distinct_storage_buckets BASE_DIR = Path(__file__).resolve().parent.parent @@ -193,6 +194,9 @@ def normalize_platform_domain_suffix(raw): "apps.backup.middleware.DeploymentRecoveryGateMiddleware", # Second, so it still wraps every view that could log a calendar-feed bearer token. "apps.events.middleware.CalendarFeedLogRedactionMiddleware", + # Binds the per-request correlation id before any layer below can log. The two gates + # above refuse without logging through it; that is the accepted cost of their position. + "config.request_id.RequestIdMiddleware", "apps.tenant_migration.middleware.SourceMigrationGateMiddleware", "apps.makerspaces.middleware.TenantHostValidationMiddleware", "django.middleware.security.SecurityMiddleware", @@ -232,7 +236,12 @@ def normalize_platform_domain_suffix(raw): WSGI_APPLICATION = "config.wsgi.application" DATABASES = {"default": env.db()} -DATABASES["default"]["CONN_MAX_AGE"] = env.int("CONN_MAX_AGE", default=0) +# Persistent connections by default: gunicorn's worker processes otherwise open and close +# a Postgres connection per request. Transaction-mode poolers (Supabase :6543, PgBouncer) +# hand back a different server connection each time, so deployments on one set +# CONN_MAX_AGE=0 explicitly -- .env.production.example and docs/deploy-production.md do. +DATABASES["default"]["CONN_MAX_AGE"] = env.int("CONN_MAX_AGE", default=60) +DATABASES["default"]["CONN_HEALTH_CHECKS"] = env.bool("CONN_HEALTH_CHECKS", default=True) DATABASES["default"]["DISABLE_SERVER_SIDE_CURSORS"] = env.bool( "DISABLE_SERVER_SIDE_CURSORS", default=False ) @@ -1079,3 +1088,29 @@ def cache_config(cache_url): {"name": "Notifications", "description": "Persistent staff inbox notifications."}, ], } + +# --- Observability ----------------------------------------------------------------------- +# JSON log lines in production (one object per line, request_id on every record); the plain +# single-line format when DEBUG, because a person is reading it. LOG_JSON overrides either. +LOG_LEVEL = env("LOG_LEVEL", default="INFO") +LOGGING = build_logging(LOG_LEVEL, json_output=env.bool("LOG_JSON", default=not DEBUG)) + +# Static bearer token for GET /api/v1/metrics/ (Prometheus text). Unset => the route is 404. +METRICS_TOKEN = env("METRICS_TOKEN", default="") + +# Error tracking is opt-in and only imported when a DSN is configured, so the SDK is never +# on the import path of a deployment that did not ask for it. PII stays off: scoped PII +# fields are encrypted at rest and must not leave the box through an error report. +SENTRY_DSN = env("SENTRY_DSN", default="") +if SENTRY_DSN: + import sentry_sdk + from sentry_sdk.integrations.celery import CeleryIntegration + from sentry_sdk.integrations.django import DjangoIntegration + + sentry_sdk.init( + dsn=SENTRY_DSN, + integrations=[DjangoIntegration(), CeleryIntegration()], + send_default_pii=False, + traces_sample_rate=env.float("SENTRY_TRACES_SAMPLE_RATE", default=0.0), + environment=env("SENTRY_ENVIRONMENT", default="production" if not DEBUG else "development"), + ) diff --git a/backend/requirements.txt b/backend/requirements.txt index c06d7eac..f4f8aa1f 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -32,3 +32,5 @@ celery[redis]>=5.4,<6 redis>=5.0,<7 dnspython>=2.7,<3 stripe==15.3.1 +# Imported only when SENTRY_DSN is set (config/settings.py); pinned so pip-audit tracks it. +sentry-sdk[django,celery]>=2.30,<3 diff --git a/backend/tests/events/test_series_organizer_admin.py b/backend/tests/events/test_series_organizer_admin.py new file mode 100644 index 00000000..7fc09567 --- /dev/null +++ b/backend/tests/events/test_series_organizer_admin.py @@ -0,0 +1,134 @@ +"""The superadmin series-organizer admin must route through the series service. + +Before phase 0 the admin wrote ``EventOrganizer`` rows directly, skipping the events module +lock, the series row lock and the authority check the service applies. +""" +from datetime import time, timedelta + +import pytest +from django.contrib.admin.sites import AdminSite +from django.test import RequestFactory +from django.utils import timezone +from rest_framework.exceptions import ValidationError + +from apps.accounts.models import User +from apps.audit.models import AuditLog +from apps.events import services_series, services_series_organizers +from apps.events.admin import EventSeriesOrganizerAdmin +from apps.events.models import EventOrganizer, EventSeriesOrganizer +from apps.makerspaces.models import Makerspace, MakerspaceMembership +from apps.organizations.models import Organization +from tests.module_helpers import disable_module + +pytestmark = pytest.mark.django_db + + +def _space(): + return Makerspace.objects.create(name="Series Space", slug="series-space") + + +def _manager(space): + user = User.objects.create_user( + username="series-manager", role=User.Role.SPACE_MANAGER, + access_status=User.AccessStatus.ACTIVE, + ) + MakerspaceMembership.objects.create( + user=user, makerspace=space, role=MakerspaceMembership.Role.SPACE_MANAGER + ) + return user + + +def _superadmin(): + return User.objects.create_user( + username="root", role=User.Role.SUPERADMIN, is_superuser=True, is_staff=True, + access_status=User.AccessStatus.ACTIVE, + ) + + +def _series(space, actor): + series, _occurrences = services_series.create_series( + makerspace=space, actor=actor, title="Weekly build night", + recurrence_timezone="UTC", + dtstart_local_date=(timezone.now() + timedelta(days=1)).date(), + dtstart_local_time=time(18), recurrence_rule="FREQ=DAILY", duration_minutes=90, + ) + return series + + +def _admin_request(user): + request = RequestFactory().post("/control/events/eventseriesorganizer/add/") + request.user = user + return request + + +def test_admin_add_projects_to_every_occurrence_through_the_service(): + space = _space() + series = _series(space, _manager(space)) + org = Organization.objects.create(name="Partner Org", slug="partner-org") + root = _superadmin() + admin = EventSeriesOrganizerAdmin(EventSeriesOrganizer, AdminSite()) + unsaved = EventSeriesOrganizer(series=series, organization=org) + + admin.save_model(_admin_request(root), unsaved, form=None, change=False) + + row = EventSeriesOrganizer.objects.get(series=series, organization=org) + assert row.created_by == root + occurrences = list(series.occurrences.all()) + assert occurrences, "create_series materializes at least one occurrence" + projected = EventOrganizer.objects.filter(source_series_organizer=row) + assert projected.count() == len(occurrences) + assert set(projected.values_list("event_id", flat=True)) == {e.pk for e in occurrences} + log = AuditLog.objects.get(action="event.series_organizer_created") + assert log.actor == root and log.makerspace == space + assert log.meta["series_id"] == series.pk + assert log.meta["organization_slug"] == "partner-org" + + +def test_admin_delete_removes_projection_and_audits(): + space = _space() + series = _series(space, _manager(space)) + org = Organization.objects.create(name="Partner Org", slug="partner-org") + root = _superadmin() + row = services_series_organizers.add_series_organizer(series, actor=root, organization=org) + assert EventOrganizer.objects.filter(source_series_organizer=row).exists() + + admin = EventSeriesOrganizerAdmin(EventSeriesOrganizer, AdminSite()) + admin.delete_model(_admin_request(root), row) + + assert not EventSeriesOrganizer.objects.filter(pk=row.pk).exists() + assert not EventOrganizer.objects.filter(organization=org).exists() + assert AuditLog.objects.filter(action="event.series_organizer_deleted").count() == 1 + + +def test_service_refuses_when_the_events_module_is_off(): + space = _space() + manager = _manager(space) + series = _series(space, manager) + org = Organization.objects.create(name="Partner Org", slug="partner-org") + disable_module(space, "events") + with pytest.raises(ValidationError): + services_series_organizers.add_series_organizer(series, actor=_superadmin(), organization=org) + assert not EventSeriesOrganizer.objects.exists() + + +def test_service_refuses_an_actor_without_series_authority(): + space = _space() + series = _series(space, _manager(space)) + org = Organization.objects.create(name="Partner Org", slug="partner-org") + outsider = User.objects.create_user( + username="outsider", role=User.Role.REQUESTER, access_status=User.AccessStatus.ACTIVE + ) + from django.core.exceptions import PermissionDenied + + with pytest.raises(PermissionDenied): + services_series_organizers.add_series_organizer(series, actor=outsider, organization=org) + + +def test_admin_no_longer_touches_organizer_rows_directly(): + import inspect + + from apps.events import admin as events_admin + + source = inspect.getsource(events_admin.EventSeriesOrganizerAdmin) + assert "EventOrganizer.objects" not in source + assert "audit.record" not in source diff --git a/backend/tests/perf/__init__.py b/backend/tests/perf/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/backend/tests/perf/test_list_query_budgets.py b/backend/tests/perf/test_list_query_budgets.py new file mode 100644 index 00000000..85d710a0 --- /dev/null +++ b/backend/tests/perf/test_list_query_budgets.py @@ -0,0 +1,136 @@ +"""Query budgets for the hot list endpoints. + +Each test seeds more rows than one page holds and asserts the request stays under a fixed +number of queries. A budget is a ceiling, not a target: the point is that adding a +``SerializerMethodField`` that hits the database per row turns a 6-query page into a +30-query page, and this file is what turns that into a red build instead of a slow queue. +Budgets are generous on purpose so a legitimate extra lookup does not need a ritual edit. +""" +import pytest +from django.urls import reverse + +from apps.hardware_requests.models import HardwareRequest +from apps.makerspaces.models import MakerspaceMembership +from apps.operations.models import StockTransfer +from tests.return_helpers import ( + authenticated_client, + make_accepted_request, + make_issued_request, + make_member, + make_product, + make_space, + make_user, +) + +pytestmark = pytest.mark.django_db + +ROWS = 30 # more than one PageNumberPagination page (24) +# Measured 2026-09-03: the staff endpoints cost a constant ~20 queries (session/JWT auth, +# membership + role resolution, module and servability checks, count + page) regardless of +# row count. An N+1 on a 30-row seed would put them at 50+, so 24 catches it with headroom. +BUDGET = 24 + + +@pytest.fixture +def space(): + return make_space("budget-space") + + +@pytest.fixture +def manager(space): + return make_member("budget-manager", space) + + +@pytest.fixture +def client(manager): + return authenticated_client(manager) + + +def _products(space, n=ROWS): + return [ + make_product(space, name=f"Tool {i:02d}", total_quantity=5, available_quantity=5) + for i in range(n) + ] + + +def _assert_page(response): + assert response.status_code == 200, response.content[:300] + payload = response.json() + if isinstance(payload, dict) and "results" in payload: + assert payload["count"] >= 1 + return payload + + +def test_public_inventory_list(space, django_assert_max_num_queries): + _products(space) + url = reverse("public-inventory", kwargs={"makerspace_slug": space.slug}) + from rest_framework.test import APIClient + + with django_assert_max_num_queries(BUDGET): + _assert_page(APIClient().get(url)) + + +def test_admin_inventory_list(space, client, django_assert_max_num_queries): + _products(space) + url = reverse("admin-inventory", kwargs={"makerspace_id": space.pk}) + with django_assert_max_num_queries(BUDGET): + _assert_page(client.get(url)) + + +def test_pending_requests_queue(space, client, django_assert_max_num_queries): + for product in _products(space): + request = make_accepted_request(space, product, 1) + HardwareRequest.objects.filter(pk=request.pk).update( + status=HardwareRequest.Status.PENDING_APPROVAL + ) + url = reverse("hardware_requests:pending-requests", kwargs={"makerspace_id": space.pk}) + with django_assert_max_num_queries(BUDGET): + _assert_page(client.get(url)) + + +def test_accepted_requests_queue(space, client, django_assert_max_num_queries): + for product in _products(space): + make_accepted_request(space, product, 1) + url = reverse("hardware_requests:accepted-requests", kwargs={"makerspace_id": space.pk}) + with django_assert_max_num_queries(BUDGET): + _assert_page(client.get(url)) + + +def test_active_loans_list(space, manager, client, django_assert_max_num_queries): + for product in _products(space, n=12): + make_issued_request(space, manager, [(product, 1)]) + url = reverse("hardware_requests:active-loans", kwargs={"makerspace_id": space.pk}) + with django_assert_max_num_queries(BUDGET): + _assert_page(client.get(url)) + + +def test_containers_list(space, client, django_assert_max_num_queries): + from tests.return_helpers import make_box + + for i in range(ROWS): + make_box(space, label=f"Box {i:02d}") + url = reverse("containers", kwargs={"makerspace_id": space.pk}) + with django_assert_max_num_queries(BUDGET): + _assert_page(client.get(url)) + + +def test_stock_transfers_list(space, manager, client, django_assert_max_num_queries): + for i in range(ROWS): + StockTransfer.objects.create( + makerspace=space, source_makerspace=space, created_by=manager, + reason=f"transfer {i}" + ) + url = reverse("stock-transfers", kwargs={"makerspace_id": space.pk}) + with django_assert_max_num_queries(BUDGET): + _assert_page(client.get(url)) + + +def test_membership_list(space, client, django_assert_max_num_queries): + for i in range(ROWS): + MakerspaceMembership.objects.create( + user=make_user(f"member-{i:02d}"), makerspace=space, + role=MakerspaceMembership.Role.INVENTORY_MANAGER, + ) + url = reverse("admin-membership-list-create", kwargs={"makerspace_id": space.pk}) + with django_assert_max_num_queries(BUDGET): + _assert_page(client.get(url)) diff --git a/backend/tests/test_log_setup.py b/backend/tests/test_log_setup.py new file mode 100644 index 00000000..60c279dc --- /dev/null +++ b/backend/tests/test_log_setup.py @@ -0,0 +1,60 @@ +import json +import logging + +from django.conf import settings + +from config.log_setup import JsonFormatter, RequestIdFilter, build_logging + + +def _record(**extra): + record = logging.LogRecord( + name="apps.test", level=logging.INFO, pathname=__file__, lineno=1, + msg="issued %s", args=("drill",), exc_info=None, + ) + for key, value in extra.items(): + setattr(record, key, value) + RequestIdFilter().filter(record) + return record + + +def test_json_formatter_emits_one_object_with_extras_and_request_id(): + line = JsonFormatter().format(_record(audit_event_uuid="u-1", makerspace_id=7)) + payload = json.loads(line) + assert payload["message"] == "issued drill" + assert payload["level"] == "INFO" + assert payload["logger"] == "apps.test" + assert payload["request_id"] == "-" + assert payload["audit_event_uuid"] == "u-1" + assert payload["makerspace_id"] == 7 + assert payload["ts"].endswith("+00:00") + assert "\n" not in line + + +def test_json_formatter_serialises_non_json_extras_and_exceptions(): + try: + raise ValueError("bad") + except ValueError: + import sys + record = _record(weird=object()) + record.exc_info = sys.exc_info() + payload = json.loads(JsonFormatter().format(record)) + assert payload["weird"].startswith(" links are emitted as bare diff --git a/docker-compose.yml b/docker-compose.yml index 7b2a571a..afb5e255 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -92,7 +92,7 @@ services: DATABASE_URL: ${DATABASE_URL:-postgres://${POSTGRES_APP_USER:-spaceworks_app}:${POSTGRES_APP_PASSWORD:-spaceworks-app-dev}@db:5432/makerspace_manager} SPACEWORKS_DB_POINTER_GENERATION: ${SPACEWORKS_DB_POINTER_GENERATION:-1} MANAGED_POSTGRES: ${MANAGED_POSTGRES:-False} - CONN_MAX_AGE: ${CONN_MAX_AGE:-0} + CONN_MAX_AGE: ${CONN_MAX_AGE:-60} DISABLE_SERVER_SIDE_CURSORS: ${DISABLE_SERVER_SIDE_CURSORS:-False} CORS_ALLOWED_ORIGINS: ${CORS_ALLOWED_ORIGINS:-http://localhost,http://localhost:5173} API_CLIENT_ENC_KEY: ${API_CLIENT_ENC_KEY:-} diff --git a/docs/INVARIANTS.md b/docs/INVARIANTS.md index 9494fabd..6ab97dae 100644 --- a/docs/INVARIANTS.md +++ b/docs/INVARIANTS.md @@ -1149,6 +1149,19 @@ loan shows the contact the borrower gave rather than the principal's internal `m the principal is refused at the write side by `accounts.principal_guards.refuse_anonymous_requester_access_mutation` — it would restrict every future account-less requester at once — and the read-side exclusion is the backstop for rows predating that guard. +**Observability (forward plan phase 0, 2026-09-03).** Every request carries an `X-Request-ID`: honoured +from the caller only when it matches `^[A-Za-z0-9_.:-]{1,64}$`, minted otherwise, bound in a +`contextvars.ContextVar` by `config.request_id.RequestIdMiddleware` (third in `MIDDLEWARE`, after the +recovery gate pinned first and the calendar-feed log redactor second) and echoed on the response. Log +records carry it through `config.log_setup.RequestIdFilter`; Celery messages carry it in a +`spaceworks_request_id` header (`config/celery_signals.py`). **The id never enters audit `meta`** — the +row MAC covers `meta`, and correlation is done from the `audit_recorded` log line that `record()` emits +with the row's `event_uuid`. `GET /api/v1/metrics/` (Prometheus text) fails closed: 404 when +`METRICS_TOKEN` is unset, 401 on a wrong bearer, and it exposes counts and ids only, never tenant +content. It is outside `HMAC_PROTECTED_PATH_PREFIXES`, so it must NOT be added to the API-client scope +registry (an entry there would be stale). Any new `env(...)` read in `settings.py` must be listed in +`apps/backup/settings_policy.py::ENV_SURFACE`, or the env-surface drift guard fails. + ## Handover roles and the retired Guest Admin **Guest Admin is no longer a built-in role** (migration `makerspaces/0052`); handover staff get a **custom diff --git a/docs/PROJECT-HISTORY.md b/docs/PROJECT-HISTORY.md index ef199fef..f89fb2ca 100644 --- a/docs/PROJECT-HISTORY.md +++ b/docs/PROJECT-HISTORY.md @@ -6,6 +6,19 @@ ## Condensed changelog (newest first — full detail in `git log`) +- **2026-09-03 — forward plan phase 0: CI that runs the suite, observability, performance close-out.** + `.github/workflows/tests.yml` runs the host-topology backend suite, the pg-client-16 backup and + tenant-migration suites, the frontend typecheck/tests/build and the CLAUDE.md/AGENTS.md drift check on + every pull request; the release workflow now depends on it, so an image is never published from a red + tree. Requests carry an `X-Request-ID` bound in a contextvar, stamped on every log line (JSON in + production), propagated into Celery, and correlated with audit rows through an `audit_recorded` log line + rather than by writing into attested `meta`. `GET /api/v1/metrics/` serves Prometheus text behind + `METRICS_TOKEN`; `SENTRY_DSN` opts into error tracking with PII off. `CONN_MAX_AGE` defaults to 60 with + health checks (pooler deployments keep 0). The June performance audit was re-measured item by item and + closed in its report; `tests/perf/` puts a query ceiling on the hot list endpoints. The superadmin + series-organizer admin, which wrote occurrence organizers straight to the ORM, now routes through a + series-level service with the module lock and authority check. The stale file-ceiling sentence in + `CLAUDE.md` was corrected (five `apps/` files over 300 lines, not 37). - **2026-09-03 — 0.8.2: GitHub Release history became permanent.** The release workflow had kept only the current and immediately previous release, deleting every older release **and its Git tag** on each run, which is why the Releases page never showed history. Neither is deleted any more. Container diff --git a/docs/PROJECT-STATUS.md b/docs/PROJECT-STATUS.md index bea08440..d2a196d0 100644 --- a/docs/PROJECT-STATUS.md +++ b/docs/PROJECT-STATUS.md @@ -67,6 +67,18 @@ analytics/ledger/exports, Users CRUD, and the FabLab modules). The detailed PRDs **internal planning docs kept local only** (gitignored); "PRD §N" references point to those. Google Sheets OAuth publishing, native apps, and physical label-printer control remain out of scope. +## Observability (phase 0 of the 2026-09-03 forward plan) + +Every request carries an `X-Request-ID` (honoured from a proxy when it is short plain ASCII, minted +otherwise), bound in `config/request_id.py` and echoed on the response. Log lines are JSON in production +(`config/log_setup.py`; `LOG_JSON` overrides), each stamped with that id, and Celery messages carry it into +the worker (`config/celery_signals.py`). Audit rows are correlated **through the log line** +(`audit_recorded` with the row's `event_uuid`), not by writing the id into `meta`, so the attested record +is unchanged. `GET /api/v1/metrics/` serves Prometheus text behind `METRICS_TOKEN` (404 when unset); +`SENTRY_DSN` opts into error tracking with PII off. CI: `.github/workflows/tests.yml` runs both test +topologies, the frontend build and the CLAUDE.md/AGENTS.md drift check, and the release workflow depends on +it. The forward plan itself is local-only under `docs/plans/2026-09-03-forward-plan/`. + Stack (in use): - **Backend:** Django 6 + Django REST Framework (`backend/`). Requires Python 3.12+. diff --git a/docs/performance-and-supabase-report.md b/docs/performance-and-supabase-report.md index 648d9f4d..160ab2bb 100644 --- a/docs/performance-and-supabase-report.md +++ b/docs/performance-and-supabase-report.md @@ -83,6 +83,37 @@ Email API free tier (or host SMTP) → notifications ``` **Final call:** fine for a demo or a very small makerspace with disciplined file cleanup; not a dependable "completely free" production deployment. The required code changes are: remove the purge superuser SQL (pick archive-only or trigger-relax), rework presigned uploads (POST→PUT or Supabase signed URLs), add a cron HTTP endpoint, and tighten upload caps + DB retention. + +## Status 2026-09-03 (phase 0 close-out) + +Re-measured on `dev` at `0039f2f9`. The report's `file:line` citations are from June and many files have +since moved or been retired; each verdict below names where the code is today. + +| # | Finding | Verdict | Where / why | +|---|---|---|---| +| 1 | `HardwareRequest` composite indexes | **Done** | `hardware_requests/models.py` carries `hwreq_ms_status_{created,issued,updated,closed}_idx` | +| 2 | `PrintPrinterSerializer` N+1 | **Superseded** | the printing kernel was tombstoned (`911f4589`); `serializers_printers.py` no longer exists | +| 3 | `PrintRequest` indexes | **Superseded** | same; print jobs are `machine_service` requests | +| 4 | Out-of-band work off the request thread | **Done for notifications**, open for exports | email + non-email channels run in `deliver_email_task` / `deliver_notification_task`; QR ZIP and XLSX exports remain synchronous → phase 6 (streamed exports) | +| 5 | Ledger in-memory sort | **Done** | `operations/ledger_query*.py` filters and paginates in SQL | +| 6 | Direct-loan `items` N+1 | **Done** | `direct_loan_views.py` uses `Prefetch("request__items", …)` | +| 7 | Operations list indexes | **Done** | `operations/models.py` indexes on transfers, stocktake, print batches | +| 8 | RBAC hidden/archived cache | **Declined for now** | `servability.unservable_makerspace_ids()` is two indexed queries; caching adds an invalidation surface for a cost not measured in production. Revisit with metrics from phase 0 | +| 9 | `BoxSerializer.get_qr_code_id` N+1 | **Done** | `views_containers.py` annotates `_active_qr_code_id` | +| 10 | `CONN_MAX_AGE` | **Done (this phase)** | default 60 + `CONN_HEALTH_CHECKS`; pooler deployments keep `0` | +| 11 | `QrScanEvent` indexes | **Done** | `qrscan_ms_qrcode_created_idx`, `qrscan_ms_context_idx` | +| 12 | `_summary` per-metric queries | **Superseded** | reports moved to the report registry (`operations/report_registry.py`) | +| 13 | Filament reports loop | **Superseded** | printing retired | +| 14 | Exports materialize everything | **Open → phase 6** | streamed CSV, write-only XLSX | +| 15 | Public print submit S3 HEAD in txn | **Superseded** | public printing intake replaced by machine-service intake | +| 16 | `procurement` unpaginated list | **Open → phase 1** | `procurement/views_items.py` still `pagination_class = None`; `select_related` is in place. Pagination changes the response shape, so it lands with the frontend list work | +| 17 | `require_module` double fetch | **Declined** | one indexed PK lookup; callers may pass the object | +| 18 | `staff_origin_scope` Python scan | **Open, low** | `makerspaces/origin_scope.py` still iterates servable makerspaces per request; bounded by makerspace count | +| 19 | Middleware re-resolves client | **Done** | `inventory/middleware.py` attaches `request.api_client` | + +Guard added: `tests/perf/test_list_query_budgets.py` puts a fixed query ceiling on the hot list endpoints +so an N+1 regression fails CI instead of slowing a queue. + --- ## Source agents diff --git a/docs/self-hosting.md b/docs/self-hosting.md index 68fb42d5..a42e7443 100644 --- a/docs/self-hosting.md +++ b/docs/self-hosting.md @@ -327,7 +327,11 @@ If an instance flips from managed → self-host after deploy, run | `HTTP_PORT` | no (default 80) | Published frontend port | | `EMAIL_*`, `DEFAULT_FROM_EMAIL` | no | Global fallback SMTP (per-makerspace SMTP overrides it) | | `MANAGED_POSTGRES` | no (default `False`) | `True` on managed Postgres (Supabase): purge suspends immutability triggers via a custom GUC instead of `session_replication_role` (which needs superuser) | -| `CONN_MAX_AGE` | no (default `0`) | Persistent DB connection lifetime; keep `0` on the Supabase transaction pooler | +| `CONN_MAX_AGE` | no (default `60`) | Persistent DB connection lifetime in seconds; set `0` on the Supabase transaction pooler (port 6543), which hands back a different server connection per transaction | +| `CONN_HEALTH_CHECKS` | no (default `True`) | Verify a persistent connection before reuse so a restarted Postgres does not surface as a request error | +| `LOG_LEVEL`, `LOG_JSON` | no (`INFO`; JSON when `DEBUG` is off) | Log verbosity and format. Every line carries the `X-Request-ID` of the request that produced it | +| `METRICS_TOKEN` | no (unset) | Bearer token for `GET /api/v1/metrics/` (Prometheus text). Unset means the route answers 404 | +| `SENTRY_DSN` | no (unset) | Opt-in error tracking; the SDK is only imported when set, and PII is never sent | | `DISABLE_SERVER_SIDE_CURSORS` | no (default `False`) | Set `True` on the Supabase transaction pooler (no server-side cursors) | | `STORAGE_PRESIGN_METHOD` | no (default `post`) | `put` for Supabase Storage presigned PUT uploads (server re-validates size at attach) | | `CRON_SECRET` | no (default empty) | Enables `POST /api/v1/internal/cron/return-reminders` (header `X-Cron-Secret`); 404s while unset | diff --git a/frontend/openapi-schema.json b/frontend/openapi-schema.json index 1ca116c3..0bfc1eec 100644 --- a/frontend/openapi-schema.json +++ b/frontend/openapi-schema.json @@ -36727,6 +36727,37 @@ } } }, + "/api/v1/metrics/": { + "get": { + "operationId": "api_v1_metrics_retrieve", + "description": "Prometheus text exposition of queue depth, request states, notification delivery outcomes, storage accounting and evidence retention. Requires the deployment's METRICS_TOKEN as a bearer token; 404 when no token is configured.", + "summary": "Prometheus metrics", + "tags": [ + "Health" + ], + "security": [ + {} + ], + "responses": { + "200": { + "content": { + "text/plain": { + "schema": { + "type": "string" + } + } + }, + "description": "" + }, + "401": { + "description": "No response body" + }, + "404": { + "description": "No response body" + } + } + } + }, "/api/v1/notifications/makerspace/{makerspace_id}": { "get": { "operationId": "api_v1_notifications_makerspace_list", diff --git a/frontend/src/generated/api.ts b/frontend/src/generated/api.ts index 83940401..aef6a000 100644 --- a/frontend/src/generated/api.ts +++ b/frontend/src/generated/api.ts @@ -483,6 +483,7 @@ export const openApiPaths = [ "/api/v1/memberships/invitations/{id}/claim", "/api/v1/memberships/me", "/api/v1/memberships/{id}/accept-invitation", + "/api/v1/metrics/", "/api/v1/notifications/makerspace/{makerspace_id}", "/api/v1/notifications/makerspace/{makerspace_id}/read-all", "/api/v1/notifications/makerspace/{makerspace_id}/unread-count", From 8be9047810b81eb945d640040e47ce970a6e6ef1 Mon Sep 17 00:00:00 2001 From: Shaan-Shoukath Date: Thu, 3 Sep 2026 21:49:57 +0530 Subject: [PATCH 02/26] refactor(frontend): split the eleven files over the 300-line ceiling, no behaviour change Co-Authored-By: Shaan-Shoukath Co-Authored-By: Claude Fable 5.1 --- .../inventory/PublicInventoryPage.tsx | 140 +---- .../publicInventory/PublicInventoryHeader.tsx | 92 ++++ .../publicInventory/useInventoryCart.ts | 64 +++ .../features/members/MemberProfilePanel.tsx | 186 +------ .../features/members/memberProfile/Field.tsx | 21 + .../members/memberProfile/ProjectEditor.tsx | 103 ++++ .../features/members/memberProfile/helpers.ts | 16 + .../features/members/memberProfile/types.ts | 38 ++ .../features/staff/DirectLoanIssueFields.tsx | 259 +++++++++ frontend/src/features/staff/DirectLoans.tsx | 315 +++-------- .../src/features/staff/DirectLoansTypes.ts | 35 ++ .../staff/NotificationDestinations.tsx | 331 +----------- frontend/src/features/staff/machinesApi.ts | 377 +++----------- .../src/features/staff/machinesApi/keys.ts | 24 + .../src/features/staff/machinesApi/logs.ts | 53 ++ .../staff/machinesApi/machineTypes.ts | 54 ++ .../features/staff/machinesApi/machines.ts | 94 ++++ .../src/features/staff/machinesApi/types.ts | 121 +++++ .../DestinationForm.tsx | 154 ++++++ .../DestinationRow.tsx | 64 +++ .../notificationDestinations/ScopePicker.tsx | 58 +++ .../notificationDestinations/channels.ts | 10 + .../notificationDestinations/scopeOptions.ts | 56 ++ .../staff/panels/AccountabilityPanel.tsx | 109 +--- .../staff/panels/AccountabilityTypes.ts | 42 ++ .../src/features/staff/panels/Inventory.tsx | 270 +--------- .../staff/panels/InventoryPanelParts.tsx | 206 ++++++++ .../staff/panels/InventoryPanelShared.ts | 62 +++ .../staff/panels/ProblemReportCard.tsx | 69 +++ .../features/staff/panels/ScannerPanel.tsx | 149 ++---- .../staff/panels/ScannerPanelForms.tsx | 164 ++++++ .../staff/panels/ScannerPanelTypes.ts | 17 + frontend/src/features/staff/panels/Users.tsx | 48 +- .../src/features/staff/panels/UsersModals.tsx | 338 +----------- .../src/features/staff/panels/users/forms.ts | 46 ++ .../panels/usersModals/AddStaffModal.tsx | 66 +++ .../usersModals/CreateMakerspaceModal.tsx | 61 +++ .../panels/usersModals/ResetPasswordModal.tsx | 102 ++++ .../panels/usersModals/RestrictUserModal.tsx | 43 ++ .../staff/panels/usersModals/shared.tsx | 45 ++ .../staff/panels/usersModals/types.ts | 19 + .../src/features/staff/useDirectLoanReturn.ts | 81 +++ frontend/src/lib/api.ts | 491 ++---------------- frontend/src/lib/api/client.ts | 169 ++++++ frontend/src/lib/api/errors.ts | 50 ++ frontend/src/lib/api/requests.ts | 112 ++++ frontend/src/lib/api/tenant.ts | 52 ++ frontend/src/lib/api/types.ts | 72 +++ 48 files changed, 3090 insertions(+), 2458 deletions(-) create mode 100644 frontend/src/features/inventory/publicInventory/PublicInventoryHeader.tsx create mode 100644 frontend/src/features/inventory/publicInventory/useInventoryCart.ts create mode 100644 frontend/src/features/members/memberProfile/Field.tsx create mode 100644 frontend/src/features/members/memberProfile/ProjectEditor.tsx create mode 100644 frontend/src/features/members/memberProfile/helpers.ts create mode 100644 frontend/src/features/members/memberProfile/types.ts create mode 100644 frontend/src/features/staff/DirectLoanIssueFields.tsx create mode 100644 frontend/src/features/staff/DirectLoansTypes.ts create mode 100644 frontend/src/features/staff/machinesApi/keys.ts create mode 100644 frontend/src/features/staff/machinesApi/logs.ts create mode 100644 frontend/src/features/staff/machinesApi/machineTypes.ts create mode 100644 frontend/src/features/staff/machinesApi/machines.ts create mode 100644 frontend/src/features/staff/machinesApi/types.ts create mode 100644 frontend/src/features/staff/notificationDestinations/DestinationForm.tsx create mode 100644 frontend/src/features/staff/notificationDestinations/DestinationRow.tsx create mode 100644 frontend/src/features/staff/notificationDestinations/ScopePicker.tsx create mode 100644 frontend/src/features/staff/notificationDestinations/channels.ts create mode 100644 frontend/src/features/staff/notificationDestinations/scopeOptions.ts create mode 100644 frontend/src/features/staff/panels/AccountabilityTypes.ts create mode 100644 frontend/src/features/staff/panels/InventoryPanelParts.tsx create mode 100644 frontend/src/features/staff/panels/InventoryPanelShared.ts create mode 100644 frontend/src/features/staff/panels/ProblemReportCard.tsx create mode 100644 frontend/src/features/staff/panels/ScannerPanelForms.tsx create mode 100644 frontend/src/features/staff/panels/ScannerPanelTypes.ts create mode 100644 frontend/src/features/staff/panels/users/forms.ts create mode 100644 frontend/src/features/staff/panels/usersModals/AddStaffModal.tsx create mode 100644 frontend/src/features/staff/panels/usersModals/CreateMakerspaceModal.tsx create mode 100644 frontend/src/features/staff/panels/usersModals/ResetPasswordModal.tsx create mode 100644 frontend/src/features/staff/panels/usersModals/RestrictUserModal.tsx create mode 100644 frontend/src/features/staff/panels/usersModals/shared.tsx create mode 100644 frontend/src/features/staff/panels/usersModals/types.ts create mode 100644 frontend/src/features/staff/useDirectLoanReturn.ts create mode 100644 frontend/src/lib/api/client.ts create mode 100644 frontend/src/lib/api/errors.ts create mode 100644 frontend/src/lib/api/requests.ts create mode 100644 frontend/src/lib/api/tenant.ts create mode 100644 frontend/src/lib/api/types.ts diff --git a/frontend/src/features/inventory/PublicInventoryPage.tsx b/frontend/src/features/inventory/PublicInventoryPage.tsx index 3ad148c9..d5e4dfa1 100644 --- a/frontend/src/features/inventory/PublicInventoryPage.tsx +++ b/frontend/src/features/inventory/PublicInventoryPage.tsx @@ -1,15 +1,10 @@ -import { useMemo, useState } from "react"; +import { useState } from "react"; import type { FormEvent } from "react"; -import { Link, useParams } from "react-router-dom"; +import { useParams } from "react-router-dom"; -import { MakerspaceBrand } from "../../components/MakerspaceBrand"; -import { MakerspaceMapLink } from "../../components/MakerspaceMapLink"; -import { SpaceWorksBadge } from "../../components/SpaceWorksLogo"; -import { ThemeToggle } from "../../components/ThemeToggle"; -import { ChartIcon, UserIcon } from "../../components/icons"; -import { Card, Field, IconLink } from "../../components/ui"; +import { Card, Field } from "../../components/ui"; import { useTenant, useTenantPath } from "../../lib/tenant"; -import type { Product, RequestCartItem } from "../../types/inventory"; +import type { Product } from "../../types/inventory"; import { ProductCard } from "./ProductCard"; import { ProductQuickViewModal } from "./ProductQuickViewModal"; import { @@ -22,6 +17,8 @@ import { } from "./PublicInventoryParts"; import { PublicRequestPanel } from "./PublicRequestPanel"; import { SkipLink } from "../../components/SkipLink"; +import { PublicInventoryHeader } from "./publicInventory/PublicInventoryHeader"; +import { useInventoryCart } from "./publicInventory/useInventoryCart"; import { usePublicCategories, usePublicInventory, useTenantBootstrap } from "./usePublicInventory"; const PAGE_SIZE = 24; @@ -35,7 +32,7 @@ export function PublicInventoryPage() { const [searchInput, setSearchInput] = useState(""); const [query, setQuery] = useState(""); const [view, setView] = useState({ kind: "all" }); - const [cart, setCart] = useState>({}); + const { cart, selectedItems, incrementItem, decrementItem, clearCart } = useInventoryCart(); const [selectedProduct, setSelectedProduct] = useState(null); const categoryParam = view.kind === "category" ? view.slug : ""; const sortParam = view.kind === "sort" ? view.sort : "name"; @@ -62,56 +59,6 @@ export function PublicInventoryPage() { 1, Math.ceil((inventoryQuery.data?.count ?? 0) / PAGE_SIZE), ); - const selectedItems = useMemo(() => Object.values(cart), [cart]); - - function maxQuantity(product: Product): number { - if ( - product.availability?.mode === "exact_count" && - typeof product.availability.count === "number" - ) { - return product.availability.count; - } - - return 99; - } - - function incrementItem(product: Product) { - if (product.availability?.label === "Unavailable") { - return; - } - - setCart((current) => { - const existing = current[product.id]; - const quantity = Math.min((existing?.quantity ?? 0) + 1, maxQuantity(product)); - return { - ...current, - [product.id]: { - productId: product.id, - name: product.name, - quantity, - }, - }; - }); - } - - function decrementItem(product: Product) { - setCart((current) => { - const existing = current[product.id]; - if (!existing || existing.quantity <= 1) { - const next = { ...current }; - delete next[product.id]; - return next; - } - - return { - ...current, - [product.id]: { - ...existing, - quantity: existing.quantity - 1, - }, - }; - }); - } function submitSearch(event: FormEvent) { event.preventDefault(); @@ -127,70 +74,13 @@ export function PublicInventoryPage() { return (
-
-
-

- Public Inventory -

-
-
-

- -

-

- Shared tools and equipment published by this makerspace. -

- -
-
-
- {bootstrap?.makerspace.public_stats_enabled ? ( - - - - ) : null} - - - - -
-
- -
- {inventoryQuery.data?.count ?? "-"} listed items -
- {modules.has("printing") ? ( - - Request a 3D print - - ) : null} - {modules.has("events") ? ( - - Events - - ) : null} - {modules.has("machines") ? ( - - Machines - - ) : null} - {modules.has("bookings") ? ( - - Book a space - - ) : null} -
-
-
-
-
+
setCart({})} + onClear={clearCart} disabled={!requestEnabled} requestAccess={bootstrap?.makerspace.request_access} /> diff --git a/frontend/src/features/inventory/publicInventory/PublicInventoryHeader.tsx b/frontend/src/features/inventory/publicInventory/PublicInventoryHeader.tsx new file mode 100644 index 00000000..48e342d8 --- /dev/null +++ b/frontend/src/features/inventory/publicInventory/PublicInventoryHeader.tsx @@ -0,0 +1,92 @@ +import { Link } from "react-router-dom"; + +import { MakerspaceBrand } from "../../../components/MakerspaceBrand"; +import { MakerspaceMapLink } from "../../../components/MakerspaceMapLink"; +import { SpaceWorksBadge } from "../../../components/SpaceWorksLogo"; +import { ThemeToggle } from "../../../components/ThemeToggle"; +import { ChartIcon, UserIcon } from "../../../components/icons"; +import { IconLink } from "../../../components/ui"; +import type { TenantBootstrap } from "../../../lib/api"; + +type PublicInventoryHeaderProps = { + displayName: string; + makerspace: TenantBootstrap["makerspace"] | undefined; + modules: Set; + tenantPath: (subpath?: string) => string; + listedCount: number | undefined; +}; + +export function PublicInventoryHeader({ + displayName, + makerspace, + modules, + tenantPath, + listedCount, +}: PublicInventoryHeaderProps) { + return ( +
+
+

+ Public Inventory +

+
+
+

+ +

+

+ Shared tools and equipment published by this makerspace. +

+ +
+
+
+ {makerspace?.public_stats_enabled ? ( + + + + ) : null} + + + + +
+
+ +
+ {listedCount ?? "-"} listed items +
+ {modules.has("printing") ? ( + + Request a 3D print + + ) : null} + {modules.has("events") ? ( + + Events + + ) : null} + {modules.has("machines") ? ( + + Machines + + ) : null} + {modules.has("bookings") ? ( + + Book a space + + ) : null} +
+
+
+
+
+ ); +} diff --git a/frontend/src/features/inventory/publicInventory/useInventoryCart.ts b/frontend/src/features/inventory/publicInventory/useInventoryCart.ts new file mode 100644 index 00000000..b74a4037 --- /dev/null +++ b/frontend/src/features/inventory/publicInventory/useInventoryCart.ts @@ -0,0 +1,64 @@ +import { useMemo, useState } from "react"; + +import type { Product, RequestCartItem } from "../../../types/inventory"; + +function maxQuantity(product: Product): number { + if ( + product.availability?.mode === "exact_count" && + typeof product.availability.count === "number" + ) { + return product.availability.count; + } + + return 99; +} + +/** Cart state for the public inventory page: a product-id keyed map plus its two mutators. */ +export function useInventoryCart() { + const [cart, setCart] = useState>({}); + const selectedItems = useMemo(() => Object.values(cart), [cart]); + + function incrementItem(product: Product) { + if (product.availability?.label === "Unavailable") { + return; + } + + setCart((current) => { + const existing = current[product.id]; + const quantity = Math.min((existing?.quantity ?? 0) + 1, maxQuantity(product)); + return { + ...current, + [product.id]: { + productId: product.id, + name: product.name, + quantity, + }, + }; + }); + } + + function decrementItem(product: Product) { + setCart((current) => { + const existing = current[product.id]; + if (!existing || existing.quantity <= 1) { + const next = { ...current }; + delete next[product.id]; + return next; + } + + return { + ...current, + [product.id]: { + ...existing, + quantity: existing.quantity - 1, + }, + }; + }); + } + + function clearCart() { + setCart({}); + } + + return { cart, selectedItems, incrementItem, decrementItem, clearCart }; +} diff --git a/frontend/src/features/members/MemberProfilePanel.tsx b/frontend/src/features/members/MemberProfilePanel.tsx index a28bdd3b..4d4ed6f1 100644 --- a/frontend/src/features/members/MemberProfilePanel.tsx +++ b/frontend/src/features/members/MemberProfilePanel.tsx @@ -3,62 +3,16 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { staffRequest } from "../../lib/api"; import { ImageUploader } from "../staff/ImageUploader"; +import { Field } from "./memberProfile/Field"; +import { tagsToText, textToTags } from "./memberProfile/helpers"; +import { ProjectEditor } from "./memberProfile/ProjectEditor"; +import { type MemberProfile, type ProjectDraft } from "./memberProfile/types"; -export type MemberProject = { - id: number; - title: string; - description: string; - links: { label: string; url: string }[]; - image_url: string | null; -}; - -export interface MemberProfileActivity { - events_attended?: number; - events_registered?: number; - recent_attended_events?: { id: number; title: string; starts_at: string }[]; -} - -export type MemberProfile = { - membership_id: number; - display_name: string; - is_visible: boolean; - show_attended_events: boolean; - headline: string; - institution: string; - bio: string; - avatar_url: string | null; - interests: string[]; - languages: string[]; - education: { institution: string; qualification?: string; year?: string }[]; - github_username: string; - github_contributions: number | null; - projects: MemberProject[]; - activity: MemberProfileActivity; -}; - -type ProjectDraft = { - id?: number; - title: string; - description: string; - links: { label: string; url: string }[]; -}; - -const tagsToText = (values: string[]) => values.join(", "); -const textToTags = (value: string) => - value.split(",").map((item) => item.trim()).filter(Boolean); - -const PROFILE_TONES = [ - "border-accent bg-accent/15", - "border-secondary bg-secondary/15", - "border-success bg-success/15", - "border-warn bg-warn/15", -] as const; - -function profileToneForIdentity(identity: string | number | undefined) { - if (identity === undefined || identity === "") return "border-secondary bg-secondary/15"; - const total = [...String(identity)].reduce((sum, character) => sum + character.codePointAt(0)!, 0); - return PROFILE_TONES[total % PROFILE_TONES.length]; -} +export type { + MemberProfile, + MemberProfileActivity, + MemberProject, +} from "./memberProfile/types"; /** * The member's own profile: what they choose to show the rest of their makerspace. @@ -287,125 +241,3 @@ export function MemberProfilePanel({ makerspaceId }: { makerspaceId: number }) {
); } - -function Field({ - id, - label, - hint, - children, -}: { - id: string; - label: string; - hint?: string; - children: React.ReactNode; -}) { - return ( -
- - {children} -
- ); -} - -function ProjectEditor({ - makerspaceId, - project, - imageUrl, - onChange, - onRemove, - onImageChanged, -}: { - makerspaceId: number; - project: ProjectDraft; - imageUrl: string | null; - onChange: (next: ProjectDraft) => void; - onRemove: () => void; - onImageChanged: () => void; -}) { - return ( -
- onChange({ ...project, title: event.target.value })} - /> -