From c0353b9acf091c7df380715e8e7b12cc6e26a05f Mon Sep 17 00:00:00 2001 From: StudyOS Org Date: Thu, 16 Jul 2026 22:13:13 +0000 Subject: [PATCH 1/3] feat: add moderated service feedback --- .github/workflows/flutter-ci.yml | 31 ++ .gitignore | 4 + README.md | 16 + docs/university_feature_port.md | 9 +- feedback_service/Dockerfile | 17 ++ feedback_service/README.md | 95 ++++++ feedback_service/__init__.py | 1 + feedback_service/backup.py | 55 ++++ feedback_service/compose.example.yml | 35 +++ feedback_service/config.py | 58 ++++ feedback_service/db.py | 279 +++++++++++++++++ feedback_service/main.py | 238 +++++++++++++++ feedback_service/models.py | 51 ++++ feedback_service/requirements-dev.txt | 4 + feedback_service/requirements.txt | 2 + feedback_service/security.py | 59 ++++ feedback_service/tests/__init__.py | 1 + feedback_service/tests/conftest.py | 40 +++ feedback_service/tests/test_api.py | 261 ++++++++++++++++ feedback_service/tests/test_backup.py | 20 ++ feedback_service/tests/test_rate_limit.py | 28 ++ flutter_app/README.md | 16 + flutter_app/lib/src/feedback_client.dart | 258 +++++++++++++--- flutter_app/lib/src/feedback_models.dart | 120 ++++++++ flutter_app/lib/src/feedback_token_store.dart | 27 ++ flutter_app/lib/src/views/settings_view.dart | 4 +- .../lib/src/widgets/feedback_components.dart | 88 ++++++ .../src/widgets/feedback_settings_card.dart | 259 +++++++++++++--- flutter_app/test/feedback_client_test.dart | 288 ++++++++++++++++-- .../test/feedback_settings_card_test.dart | 265 ++++++++++++++++ flutter_app/test/widget_test.dart | 2 +- 31 files changed, 2519 insertions(+), 112 deletions(-) create mode 100644 feedback_service/Dockerfile create mode 100644 feedback_service/README.md create mode 100644 feedback_service/__init__.py create mode 100644 feedback_service/backup.py create mode 100644 feedback_service/compose.example.yml create mode 100644 feedback_service/config.py create mode 100644 feedback_service/db.py create mode 100644 feedback_service/main.py create mode 100644 feedback_service/models.py create mode 100644 feedback_service/requirements-dev.txt create mode 100644 feedback_service/requirements.txt create mode 100644 feedback_service/security.py create mode 100644 feedback_service/tests/__init__.py create mode 100644 feedback_service/tests/conftest.py create mode 100644 feedback_service/tests/test_api.py create mode 100644 feedback_service/tests/test_backup.py create mode 100644 feedback_service/tests/test_rate_limit.py create mode 100644 flutter_app/lib/src/feedback_models.dart create mode 100644 flutter_app/lib/src/feedback_token_store.dart create mode 100644 flutter_app/lib/src/widgets/feedback_components.dart create mode 100644 flutter_app/test/feedback_settings_card_test.dart diff --git a/.github/workflows/flutter-ci.yml b/.github/workflows/flutter-ci.yml index 7485dd3..372281b 100644 --- a/.github/workflows/flutter-ci.yml +++ b/.github/workflows/flutter-ci.yml @@ -5,6 +5,7 @@ on: paths: - ".github/workflows/flutter-ci.yml" - "README.md" + - "feedback_service/**" - "flutter_app/**" push: branches: @@ -14,6 +15,7 @@ on: paths: - ".github/workflows/flutter-ci.yml" - "README.md" + - "feedback_service/**" - "flutter_app/**" workflow_dispatch: inputs: @@ -35,6 +37,35 @@ env: FLUTTER_APP_DIR: flutter_app jobs: + validate-feedback-service: + name: Validate feedback service + runs-on: ubuntu-latest + + steps: + - name: Check out repository + uses: actions/checkout@v6 + + - name: Set up Python + uses: actions/setup-python@v6 + with: + python-version: "3.12" + cache: pip + cache-dependency-path: | + feedback_service/requirements.txt + feedback_service/requirements-dev.txt + + - name: Install feedback dependencies + run: python -m pip install --requirement feedback_service/requirements-dev.txt + + - name: Lint feedback service + run: python -m ruff check feedback_service + + - name: Check feedback formatting + run: python -m ruff format --check feedback_service + + - name: Test feedback service + run: python -m pytest -q feedback_service/tests + validate: name: Validate Flutter app runs-on: ubuntu-latest diff --git a/.gitignore b/.gitignore index aa724b7..1f2bef1 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,10 @@ /.idea/navEditor.xml /.idea/assetWizardSettings.xml .DS_Store +__pycache__/ +*.py[cod] +.pytest_cache/ +.ruff_cache/ /build /captures .externalNativeBuild diff --git a/README.md b/README.md index 8de804c..6293256 100644 --- a/README.md +++ b/README.md @@ -112,6 +112,22 @@ flutter run -d chrome \ --dart-define=STUDYOS_DEMO_OPENROUTER_API_KEY="$OPENROUTER_API_KEY" ``` +## Feedback Deployment + +The Settings feedback card connects to the containerized SQLite service under +`feedback_service/`. Deploy that service behind HTTPS, then provide its public +base URL to Flutter builds: + +```sh +flutter build web --release \ + --dart-define=STUDYOS_FEEDBACK_API_URL=https://feedback.example.edu +``` + +The URL isn't a secret. Moderator credentials remain server-side environment +variables; never embed them—or a GitHub write token—in a Flutter build. See +`feedback_service/README.md` for deployment, moderation, backup, retention, and +restore guidance. + ## Migration Notes - Flutter owns the main chat UI, input bar, status display, navigation, and diff --git a/docs/university_feature_port.md b/docs/university_feature_port.md index eaf9305..05916de 100644 --- a/docs/university_feature_port.md +++ b/docs/university_feature_port.md @@ -60,11 +60,10 @@ setup in Settings with secure local storage and expose only a compact Home shortcut once configured, so the card is useful without taking over the main app navigation. -11. Add feedback without a database or GitHub account requirement for students. - The app creates GitHub issues directly with a developer-provided build - credential for `Tue-StudyOS/StudyOS_Agent` with `Issues: write`, for example - through `--dart-define=STUDYOS_FEEDBACK_TOKEN=...`. Do not expose this token - in the user-facing app. +11. Route ratings and optional comments through the dedicated feedback API. + The API issues a pseudonymous installation token that the app keeps in + secure local storage; release builds must never contain a GitHub write token + or moderator credential. Comments require human approval before publication. ## UX Shape diff --git a/feedback_service/Dockerfile b/feedback_service/Dockerfile new file mode 100644 index 0000000..0a454ab --- /dev/null +++ b/feedback_service/Dockerfile @@ -0,0 +1,17 @@ +FROM python:3.12-slim AS runtime + +ENV PYTHONDONTWRITEBYTECODE=1 \ + PYTHONUNBUFFERED=1 \ + PIP_NO_CACHE_DIR=1 + +RUN addgroup --system --gid 10001 feedback \ + && adduser --system --uid 10001 --ingroup feedback --home /app feedback +WORKDIR /app +COPY feedback_service/requirements.txt ./feedback_service/requirements.txt +RUN pip install --requirement feedback_service/requirements.txt +COPY feedback_service ./feedback_service +RUN mkdir /data /backups && chown feedback:feedback /data /backups + +USER feedback +EXPOSE 8080 +CMD ["uvicorn", "feedback_service.main:app", "--host", "0.0.0.0", "--port", "8080", "--no-access-log", "--proxy-headers", "--forwarded-allow-ips", "127.0.0.1"] diff --git a/feedback_service/README.md b/feedback_service/README.md new file mode 100644 index 0000000..21c81c2 --- /dev/null +++ b/feedback_service/README.md @@ -0,0 +1,95 @@ +# StudyOS feedback service + +A small FastAPI service backed by SQLite. Ratings are public immediately while +optional comments enter a moderation queue. Anonymous installation credentials +are random 256-bit bearer tokens; only their SHA-256 digests are stored. +Moderation is manual by default, so there is no third-party API, data transfer, +or usage-based moderation bill. + +## Run + +Copy `compose.example.yml`, generate a long random `FEEDBACK_ADMIN_TOKEN`, set +the exact frontend origins in `FEEDBACK_CORS_ORIGINS`, and run +`docker compose up --build`. The SQLite database is kept in the named volume. +The API schema is available at `/docs`; health checks use `/healthz`. + +Never embed the admin token in a client. Put this service behind HTTPS and a +reverse proxy. The in-process limits (10 installation issuances per peer and 30 +writes per author per minute by default) are only a deterministic safety net; +the proxy must enforce distributed IP and global limits, body-size limits, and +request timeouts. Configure trusted proxy IPs in the container command before +relying on forwarded client addresses. Token issuance is intentionally +anonymous and therefore Sybil-prone; comments default to `pending` for this +reason. + +## Configuration + +- `FEEDBACK_DATABASE_PATH` (default `/data/feedback.sqlite3`) +- `FEEDBACK_ADMIN_TOKEN` (admin endpoints remain inaccessible when empty) +- `FEEDBACK_MODERATOR_ID` (audit identity bound to the admin token) +- `FEEDBACK_SERVICE_IDS` (comma-separated, default `studyos-agent`) +- `FEEDBACK_CORS_ORIGINS` (comma-separated exact origins; empty disables CORS) +- `FEEDBACK_INSTALLATION_RATE_LIMIT` and `FEEDBACK_AUTHOR_RATE_LIMIT` + +## Moderation operations + +List pending or reported comments from a trusted admin machine, then publish, +reject, or delete the text. Admin deletion leaves the numeric rating active; +owner deletion removes both rating and comment from public results. + +```sh +curl -H "X-Admin-Token: $FEEDBACK_ADMIN_TOKEN" \ + -H "X-Moderator-Id: $FEEDBACK_MODERATOR_ID" \ + "https://feedback.example.edu/v1/admin/moderation?state=pending&limit=50" + +curl -X POST \ + -H "X-Admin-Token: $FEEDBACK_ADMIN_TOKEN" \ + -H "X-Moderator-Id: $FEEDBACK_MODERATOR_ID" \ + -H "Content-Type: application/json" \ + -d '{"action":"publish","reason":"manual review"}' \ + "https://feedback.example.edu/v1/admin/feedback/FEEDBACK_ID/moderation" +``` + +Every moderation action records the supplied moderator identifier in +`moderation_audit`; reject/delete actions require a reason. The configured ID is +bound to the admin secret instead of trusting an arbitrary request label. Use +separate service deployments/credentials if independent moderator attribution is +required. Reasons and reports are never returned by +public endpoints. Rotate the admin token after suspected exposure. This version +intentionally has no automated third-party moderation: +provider privacy, retention, failure behavior, and billing must be reviewed +before such an integration is enabled, and failures must fall back to pending +human review. + +## Launch and retention checklist + +Before public deployment, the team must name a moderation owner, response +expectation, and escalation/contact path. Publish a short notice covering the +pseudonymous installation identifier, public ratings/comments, report and +moderation processing, deletion behavior, backup location/expiry, and operator +contact. Choose and automate retention for rejected/deleted text, audit events, +reports, inactive installations, and backups; this repository does not invent a +course-wide retention period. Do not collect university credentials, email +addresses, profile data, or raw device fingerprints in this service. + +## Backup and restore + +Create an online, transactionally consistent backup inside the running +container and verify it before copying it to separate storage: + +```sh +docker compose exec feedback python -m feedback_service.backup create \ + /data/feedback.sqlite3 /backups/feedback-$(date +%F).sqlite3 +docker compose exec feedback python -m feedback_service.backup verify \ + /backups/feedback-$(date +%F).sqlite3 +``` + +Backups still contain comments and pseudonymous installation hashes; restrict +their access and define an expiry matching the project's retention notice. To +restore, stop the service, verify the selected backup, replace +`/data/feedback.sqlite3`, remove stale `-wal`/`-shm` sidecars, then restart and +check `/healthz` plus a public read. Practice this against a disposable volume +before launch. + +Run one application process per SQLite volume; scale at the reverse proxy only +after moving to a multi-writer database. diff --git a/feedback_service/__init__.py b/feedback_service/__init__.py new file mode 100644 index 0000000..62d5bc6 --- /dev/null +++ b/feedback_service/__init__.py @@ -0,0 +1 @@ +"""Small, self-hosted feedback service for StudyOS.""" diff --git a/feedback_service/backup.py b/feedback_service/backup.py new file mode 100644 index 0000000..43d4f4a --- /dev/null +++ b/feedback_service/backup.py @@ -0,0 +1,55 @@ +from __future__ import annotations + +import argparse +import os +import sqlite3 +from pathlib import Path + + +def create_backup(source: Path, destination: Path) -> None: + if source.resolve() == destination.resolve(): + raise ValueError("source and destination must differ") + destination.parent.mkdir(parents=True, exist_ok=True) + temporary = destination.with_name(f".{destination.name}.tmp") + temporary.unlink(missing_ok=True) + try: + with ( + sqlite3.connect(source) as source_db, + sqlite3.connect(temporary) as backup_db, + ): + source_db.backup(backup_db) + result = backup_db.execute("PRAGMA integrity_check").fetchone()[0] + if result != "ok": + raise RuntimeError(f"backup integrity check failed: {result}") + os.replace(temporary, destination) + finally: + temporary.unlink(missing_ok=True) + + +def verify_backup(path: Path) -> None: + uri = f"file:{path.resolve()}?mode=ro" + with sqlite3.connect(uri, uri=True) as database: + result = database.execute("PRAGMA integrity_check").fetchone()[0] + if result != "ok": + raise RuntimeError(f"backup integrity check failed: {result}") + + +def main() -> None: + parser = argparse.ArgumentParser( + description="Create or verify a StudyOS feedback backup" + ) + subcommands = parser.add_subparsers(dest="command", required=True) + create = subcommands.add_parser("create") + create.add_argument("source", type=Path) + create.add_argument("destination", type=Path) + verify = subcommands.add_parser("verify") + verify.add_argument("path", type=Path) + arguments = parser.parse_args() + if arguments.command == "create": + create_backup(arguments.source, arguments.destination) + else: + verify_backup(arguments.path) + + +if __name__ == "__main__": + main() diff --git a/feedback_service/compose.example.yml b/feedback_service/compose.example.yml new file mode 100644 index 0000000..e9f7bba --- /dev/null +++ b/feedback_service/compose.example.yml @@ -0,0 +1,35 @@ +services: + feedback: + build: + context: .. + dockerfile: feedback_service/Dockerfile + read_only: true + tmpfs: + - /tmp:size=16m,noexec,nosuid + cap_drop: + - ALL + security_opt: + - no-new-privileges:true + environment: + FEEDBACK_ADMIN_TOKEN: ${FEEDBACK_ADMIN_TOKEN:?set a long random admin token} + FEEDBACK_MODERATOR_ID: ${FEEDBACK_MODERATOR_ID:-studyos-admin} + FEEDBACK_CORS_ORIGINS: ${FEEDBACK_CORS_ORIGINS:-} + FEEDBACK_SERVICE_IDS: studyos-agent + volumes: + - feedback-data:/data + - feedback-backups:/backups + ports: + - "127.0.0.1:8080:8080" + healthcheck: + test: ["CMD", "python", "-c", "import urllib.request; urllib.request.urlopen('http://127.0.0.1:8080/healthz', timeout=2)"] + interval: 30s + timeout: 3s + retries: 3 + mem_limit: 128m + cpus: 0.5 + pids_limit: 100 + restart: unless-stopped + +volumes: + feedback-data: + feedback-backups: diff --git a/feedback_service/config.py b/feedback_service/config.py new file mode 100644 index 0000000..c2507c8 --- /dev/null +++ b/feedback_service/config.py @@ -0,0 +1,58 @@ +from __future__ import annotations + +import os +import re +from dataclasses import dataclass +from pathlib import Path + + +def _csv(value: str) -> tuple[str, ...]: + return tuple(item.strip() for item in value.split(",") if item.strip()) + + +MODERATOR_ID = re.compile(r"^[a-zA-Z0-9._@-]{2,80}$") + + +@dataclass(frozen=True) +class Settings: + database_path: Path = Path("/data/feedback.sqlite3") + admin_token: str = "" + moderator_id: str = "studyos-admin" + allowed_service_ids: tuple[str, ...] = ("studyos-agent",) + cors_origins: tuple[str, ...] = () + installation_limit_per_minute: int = 10 + feedback_limit_per_minute: int = 30 + + @classmethod + def from_env(cls) -> "Settings": + return cls( + database_path=Path( + os.getenv("FEEDBACK_DATABASE_PATH", "/data/feedback.sqlite3") + ), + admin_token=os.getenv("FEEDBACK_ADMIN_TOKEN", ""), + moderator_id=os.getenv("FEEDBACK_MODERATOR_ID", "studyos-admin"), + allowed_service_ids=_csv( + os.getenv("FEEDBACK_SERVICE_IDS", "studyos-agent") + ), + cors_origins=_csv(os.getenv("FEEDBACK_CORS_ORIGINS", "")), + installation_limit_per_minute=int( + os.getenv("FEEDBACK_INSTALLATION_RATE_LIMIT", "10") + ), + feedback_limit_per_minute=int( + os.getenv("FEEDBACK_AUTHOR_RATE_LIMIT", "30") + ), + ) + + def validate(self) -> None: + if not self.allowed_service_ids: + raise RuntimeError( + "FEEDBACK_SERVICE_IDS must contain at least one service ID" + ) + if self.admin_token and len(self.admin_token) < 32: + raise RuntimeError( + "FEEDBACK_ADMIN_TOKEN must contain at least 32 characters" + ) + if not MODERATOR_ID.fullmatch(self.moderator_id): + raise RuntimeError("FEEDBACK_MODERATOR_ID is invalid") + if self.installation_limit_per_minute < 1 or self.feedback_limit_per_minute < 1: + raise RuntimeError("rate limits must be positive") diff --git a/feedback_service/db.py b/feedback_service/db.py new file mode 100644 index 0000000..460f8ef --- /dev/null +++ b/feedback_service/db.py @@ -0,0 +1,279 @@ +from __future__ import annotations + +import sqlite3 +import uuid +from datetime import UTC, datetime +from pathlib import Path + + +def now() -> str: + return datetime.now(UTC).isoformat() + + +class Database: + def __init__(self, path: Path): + self.path = path + + def connect(self) -> sqlite3.Connection: + connection = sqlite3.connect(self.path, timeout=5) + connection.row_factory = sqlite3.Row + connection.execute("PRAGMA foreign_keys = ON") + connection.execute("PRAGMA busy_timeout = 5000") + return connection + + def migrate(self) -> None: + self.path.parent.mkdir(parents=True, exist_ok=True) + with self.connect() as db: + db.execute("PRAGMA journal_mode = WAL") + version = db.execute("PRAGMA user_version").fetchone()[0] + if version == 0: + db.executescript( + """ + CREATE TABLE installations ( + id TEXT PRIMARY KEY, + token_hash TEXT NOT NULL UNIQUE, + created_at TEXT NOT NULL + ); + CREATE TABLE feedback ( + id TEXT PRIMARY KEY, + installation_id TEXT NOT NULL REFERENCES installations(id), + service_id TEXT NOT NULL, + rating INTEGER NOT NULL CHECK (rating BETWEEN 1 AND 5), + comment TEXT CHECK (comment IS NULL OR length(comment) <= 1000), + comment_state TEXT NOT NULL CHECK ( + comment_state IN ('none','pending','published','rejected','deleted') + ), + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL, + published_at TEXT, + deleted_at TEXT, + UNIQUE (installation_id, service_id) + ); + CREATE INDEX feedback_public_idx ON feedback(service_id, deleted_at, comment_state); + CREATE TABLE reports ( + id TEXT PRIMARY KEY, + feedback_id TEXT NOT NULL REFERENCES feedback(id), + reporter_installation_id TEXT NOT NULL REFERENCES installations(id), + reason TEXT CHECK (reason IS NULL OR length(reason) <= 500), + created_at TEXT NOT NULL, + UNIQUE (feedback_id, reporter_installation_id) + ); + CREATE TABLE moderation_audit ( + id TEXT PRIMARY KEY, + feedback_id TEXT NOT NULL REFERENCES feedback(id), + previous_state TEXT NOT NULL, + action TEXT NOT NULL, + reason TEXT, + moderator_id TEXT NOT NULL, + created_at TEXT NOT NULL + ); + PRAGMA user_version = 1; + """ + ) + elif version != 1: + raise RuntimeError(f"unsupported database schema version: {version}") + + def create_installation(self, digest: str) -> None: + with self.connect() as db: + db.execute( + "INSERT INTO installations (id, token_hash, created_at) VALUES (?, ?, ?)", + (str(uuid.uuid4()), digest, now()), + ) + + def installation_id(self, digest: str) -> str | None: + with self.connect() as db: + row = db.execute( + "SELECT id FROM installations WHERE token_hash = ?", (digest,) + ).fetchone() + return row["id"] if row else None + + def upsert_feedback( + self, installation_id: str, service_id: str, rating: int, comment: str | None + ): + timestamp = now() + state = "pending" if comment else "none" + with self.connect() as db: + row = db.execute( + """INSERT INTO feedback + (id, installation_id, service_id, rating, comment, comment_state, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?) + ON CONFLICT(installation_id, service_id) DO UPDATE SET + rating=excluded.rating, + comment=excluded.comment, + comment_state=excluded.comment_state, + updated_at=excluded.updated_at, + published_at=NULL, + deleted_at=NULL + RETURNING id""", + ( + str(uuid.uuid4()), + installation_id, + service_id, + rating, + comment, + state, + timestamp, + timestamp, + ), + ).fetchone() + feedback_id = row["id"] + # Reports describe the previous comment revision. Resubmission + # returns the comment to review and must not inherit them. + db.execute("DELETE FROM reports WHERE feedback_id=?", (feedback_id,)) + return self.feedback_for_owner(installation_id, service_id) + + def feedback_for_owner(self, installation_id: str, service_id: str): + with self.connect() as db: + return db.execute( + """SELECT id, service_id, rating, comment, comment_state, created_at, updated_at + FROM feedback WHERE installation_id=? AND service_id=? AND deleted_at IS NULL""", + (installation_id, service_id), + ).fetchone() + + def delete_feedback(self, installation_id: str, service_id: str) -> bool: + timestamp = now() + with self.connect() as db: + result = db.execute( + """UPDATE feedback SET comment=NULL, comment_state='deleted', deleted_at=?, updated_at=? + WHERE installation_id=? AND service_id=? AND deleted_at IS NULL""", + (timestamp, timestamp, installation_id, service_id), + ) + return result.rowcount == 1 + + def public_feedback(self, service_id: str, limit: int, offset: int) -> dict: + with self.connect() as db: + aggregate = db.execute( + "SELECT COUNT(*) count, AVG(rating) average FROM feedback WHERE service_id=? AND deleted_at IS NULL", + (service_id,), + ).fetchone() + comments = db.execute( + """SELECT id, rating, comment, published_at FROM feedback + WHERE service_id=? AND deleted_at IS NULL AND comment_state='published' + ORDER BY published_at DESC, id LIMIT ? OFFSET ?""", + (service_id, limit, offset), + ).fetchall() + return { + "service_id": service_id, + "rating": {"count": aggregate["count"], "average": aggregate["average"]}, + "comments": [dict(row) for row in comments], + "pagination": {"limit": limit, "offset": offset}, + } + + def report( + self, + installation_id: str, + service_id: str, + feedback_id: str, + reason: str | None, + ) -> str: + with self.connect() as db: + target = db.execute( + """SELECT installation_id FROM feedback WHERE id=? AND service_id=? + AND comment_state='published' AND deleted_at IS NULL""", + (feedback_id, service_id), + ).fetchone() + if not target: + return "not_found" + if target["installation_id"] == installation_id: + return "own" + try: + db.execute( + "INSERT INTO reports VALUES (?, ?, ?, ?, ?)", + (str(uuid.uuid4()), feedback_id, installation_id, reason, now()), + ) + except sqlite3.IntegrityError: + return "duplicate" + return "created" + + def moderation_queue( + self, + state: str, + service_id: str | None, + limit: int, + offset: int, + ): + where = "f.deleted_at IS NULL AND " + params: list[str] = [] + if state == "reported": + where += "f.comment_state='published' AND EXISTS (SELECT 1 FROM reports x WHERE x.feedback_id=f.id)" + else: + where += "f.comment_state=?" + params.append(state) + if service_id: + where += " AND f.service_id=?" + params.append(service_id) + with self.connect() as db: + return db.execute( + f"""SELECT f.id, f.service_id, f.rating, f.comment, f.comment_state, + f.created_at, f.updated_at, COUNT(r.id) report_count + FROM feedback f LEFT JOIN reports r ON r.feedback_id=f.id + WHERE {where} GROUP BY f.id ORDER BY f.updated_at + LIMIT ? OFFSET ?""", # noqa: S608 (fixed clauses) + [*params, limit, offset], + ).fetchall() + + def reports_for_feedback(self, feedback_id: str): + with self.connect() as db: + return db.execute( + """SELECT reason, created_at FROM reports + WHERE feedback_id=? ORDER BY created_at""", + (feedback_id,), + ).fetchall() + + def moderate( + self, + feedback_id: str, + action: str, + reason: str | None, + moderator_id: str, + ): + with self.connect() as db: + # Serialize the read/transition/audit sequence so concurrent + # moderators cannot record the same previous state. + db.execute("BEGIN IMMEDIATE") + row = db.execute( + "SELECT comment_state, comment FROM feedback WHERE id=? AND deleted_at IS NULL", + (feedback_id,), + ).fetchone() + if not row: + return None + allowed_states = { + "publish": {"pending", "rejected"}, + "reject": {"pending", "published"}, + "delete": {"pending", "published", "rejected"}, + } + if row["comment_state"] not in allowed_states[action]: + return "invalid" + timestamp = now() + new_state = { + "publish": "published", + "reject": "rejected", + "delete": "deleted", + }[action] + # Admin deletion removes only the comment. The rating remains in the + # aggregate; only an owner's DELETE removes their entire feedback. + deleted_at = None + comment = None if action == "delete" else row["comment"] + published_at = timestamp if action == "publish" else None + db.execute( + """UPDATE feedback SET comment_state=?, comment=?, published_at=?, deleted_at=?, updated_at=? + WHERE id=?""", + (new_state, comment, published_at, deleted_at, timestamp, feedback_id), + ) + db.execute( + "INSERT INTO moderation_audit VALUES (?, ?, ?, ?, ?, ?, ?)", + ( + str(uuid.uuid4()), + feedback_id, + row["comment_state"], + action, + reason, + moderator_id, + timestamp, + ), + ) + return { + "id": feedback_id, + "comment_state": new_state, + "updated_at": timestamp, + } diff --git a/feedback_service/main.py b/feedback_service/main.py new file mode 100644 index 0000000..3558f09 --- /dev/null +++ b/feedback_service/main.py @@ -0,0 +1,238 @@ +from __future__ import annotations + +import hmac +from contextlib import asynccontextmanager +from typing import Annotated + +from fastapi import ( + Depends, + FastAPI, + Header, + HTTPException, + Query, + Request, + Response, + status, +) +from fastapi.middleware.cors import CORSMiddleware +from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer + +from .config import MODERATOR_ID, Settings +from .db import Database +from .models import FeedbackInput, ModerationInput, ReportInput +from .security import SlidingWindowLimiter, issue_token, token_hash + +bearer = HTTPBearer(auto_error=False) + + +def create_app(settings: Settings | None = None) -> FastAPI: + settings = settings or Settings.from_env() + settings.validate() + database = Database(settings.database_path) + installation_limiter = SlidingWindowLimiter(settings.installation_limit_per_minute) + author_limiter = SlidingWindowLimiter(settings.feedback_limit_per_minute) + + @asynccontextmanager + async def lifespan(_: FastAPI): + database.migrate() + yield + + app = FastAPI(title="StudyOS Feedback Service", version="1.0.0", lifespan=lifespan) + app.state.database = database + if settings.cors_origins: + app.add_middleware( + CORSMiddleware, + allow_origins=list(settings.cors_origins), + allow_credentials=False, + allow_methods=["GET", "POST", "PUT", "DELETE"], + allow_headers=[ + "Authorization", + "Content-Type", + "X-Admin-Token", + "X-Moderator-Id", + ], + ) + + def service(service_id: str) -> str: + if service_id not in settings.allowed_service_ids: + raise HTTPException(status.HTTP_404_NOT_FOUND, "service not found") + return service_id + + def installation( + credentials: Annotated[HTTPAuthorizationCredentials | None, Depends(bearer)], + ) -> str: + if ( + not credentials + or credentials.scheme.lower() != "bearer" + or not 32 <= len(credentials.credentials) <= 256 + ): + raise HTTPException(status.HTTP_401_UNAUTHORIZED, "invalid bearer token") + installation_id = database.installation_id(token_hash(credentials.credentials)) + if not installation_id: + raise HTTPException(status.HTTP_401_UNAUTHORIZED, "invalid bearer token") + return installation_id + + def limited_installation( + credentials: Annotated[HTTPAuthorizationCredentials | None, Depends(bearer)], + ) -> str: + installation_id = installation(credentials) + if not author_limiter.allow(installation_id): + raise HTTPException( + status.HTTP_429_TOO_MANY_REQUESTS, "rate limit exceeded" + ) + return installation_id + + def admin( + x_admin_token: Annotated[str | None, Header()] = None, + x_moderator_id: Annotated[str | None, Header()] = None, + ) -> str: + if ( + not settings.admin_token + or not x_admin_token + or not hmac.compare_digest(x_admin_token, settings.admin_token) + ): + raise HTTPException(status.HTTP_401_UNAUTHORIZED, "invalid admin token") + if ( + not x_moderator_id + or not MODERATOR_ID.fullmatch(x_moderator_id) + or not hmac.compare_digest(x_moderator_id, settings.moderator_id) + ): + raise HTTPException(status.HTTP_400_BAD_REQUEST, "invalid moderator id") + return settings.moderator_id + + @app.get("/healthz") + def health() -> dict[str, str]: + with database.connect() as db: + db.execute("SELECT 1").fetchone() + return {"status": "ok"} + + @app.post("/v1/installations", status_code=status.HTTP_201_CREATED) + def create_installation(request: Request) -> dict[str, str]: + peer = request.client.host if request.client else "unknown" + if not installation_limiter.allow(peer): + raise HTTPException( + status.HTTP_429_TOO_MANY_REQUESTS, "rate limit exceeded" + ) + token = issue_token() + database.create_installation(token_hash(token)) + return {"installation_token": token, "token_type": "bearer"} + + @app.get("/v1/services/{service_id}/feedback/public") + def public_feedback( + service_id: str, + limit: Annotated[int, Query(ge=1, le=100)] = 20, + offset: Annotated[int, Query(ge=0, le=10_000)] = 0, + ) -> dict: + return database.public_feedback(service(service_id), limit, offset) + + @app.get("/v1/services/{service_id}/feedback/mine") + def own_feedback( + service_id: str, installation_id: str = Depends(installation) + ) -> dict: + row = database.feedback_for_owner(installation_id, service(service_id)) + if not row: + raise HTTPException(status.HTTP_404_NOT_FOUND, "feedback not found") + return dict(row) + + @app.put("/v1/services/{service_id}/feedback/mine") + def put_feedback( + service_id: str, + payload: FeedbackInput, + installation_id: str = Depends(limited_installation), + ) -> dict: + return dict( + database.upsert_feedback( + installation_id, service(service_id), payload.rating, payload.comment + ) + ) + + @app.delete( + "/v1/services/{service_id}/feedback/mine", + status_code=status.HTTP_204_NO_CONTENT, + ) + def delete_feedback( + service_id: str, installation_id: str = Depends(limited_installation) + ) -> Response: + if not database.delete_feedback(installation_id, service(service_id)): + raise HTTPException(status.HTTP_404_NOT_FOUND, "feedback not found") + return Response(status_code=status.HTTP_204_NO_CONTENT) + + @app.post( + "/v1/services/{service_id}/feedback/{feedback_id}/reports", + status_code=status.HTTP_201_CREATED, + ) + def report_feedback( + service_id: str, + feedback_id: str, + payload: ReportInput, + installation_id: str = Depends(limited_installation), + ) -> dict[str, str]: + result = database.report( + installation_id, service(service_id), feedback_id, payload.reason + ) + if result == "not_found": + raise HTTPException( + status.HTTP_404_NOT_FOUND, "published comment not found" + ) + if result == "own": + raise HTTPException( + status.HTTP_400_BAD_REQUEST, "cannot report own comment" + ) + if result == "duplicate": + raise HTTPException(status.HTTP_409_CONFLICT, "comment already reported") + return {"status": "recorded"} + + @app.get("/v1/admin/moderation") + def moderation_queue( + state: Annotated[str, Query(pattern="^(pending|reported)$")] = "pending", + service_id: str | None = None, + limit: Annotated[int, Query(ge=1, le=100)] = 50, + offset: Annotated[int, Query(ge=0, le=10_000)] = 0, + _moderator_id: str = Depends(admin), + ) -> dict: + if service_id is not None: + service(service_id) + items = [ + dict(row) + for row in database.moderation_queue( + state, + service_id, + limit, + offset, + ) + ] + if state == "reported": + for item in items: + item["reports"] = [ + dict(report) for report in database.reports_for_feedback(item["id"]) + ] + return { + "items": items, + "limit": limit, + "offset": offset, + } + + @app.post("/v1/admin/feedback/{feedback_id}/moderation") + def moderate( + feedback_id: str, + payload: ModerationInput, + moderator_id: str = Depends(admin), + ) -> dict: + result = database.moderate( + feedback_id, + payload.action, + payload.reason, + moderator_id, + ) + if result is None: + raise HTTPException(status.HTTP_404_NOT_FOUND, "feedback not found") + if result == "invalid": + raise HTTPException( + status.HTTP_409_CONFLICT, "invalid moderation state transition" + ) + return result + + return app + + +app = create_app() diff --git a/feedback_service/models.py b/feedback_service/models.py new file mode 100644 index 0000000..29693ca --- /dev/null +++ b/feedback_service/models.py @@ -0,0 +1,51 @@ +from __future__ import annotations + +import re +from typing import Literal + +from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator + +CONTROL_CHARACTERS = re.compile(r"[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]") + + +def clean_text(value: str | None) -> str | None: + if value is None: + return None + value = value.strip() + if not value: + return None + if CONTROL_CHARACTERS.search(value): + raise ValueError("control characters are not allowed") + return value + + +class FeedbackInput(BaseModel): + model_config = ConfigDict(extra="forbid") + + rating: int = Field(ge=1, le=5, strict=True) + comment: str | None = Field(default=None, max_length=1000) + + _clean_comment = field_validator("comment")(clean_text) + + +class ReportInput(BaseModel): + model_config = ConfigDict(extra="forbid") + + reason: str | None = Field(default=None, max_length=500) + + _clean_reason = field_validator("reason")(clean_text) + + +class ModerationInput(BaseModel): + model_config = ConfigDict(extra="forbid") + + action: Literal["publish", "reject", "delete"] + reason: str | None = Field(default=None, max_length=500) + + _clean_reason = field_validator("reason")(clean_text) + + @model_validator(mode="after") + def require_action_reason(self): + if self.action in {"reject", "delete"} and not self.reason: + raise ValueError("reason is required when rejecting or deleting a comment") + return self diff --git a/feedback_service/requirements-dev.txt b/feedback_service/requirements-dev.txt new file mode 100644 index 0000000..bb5e663 --- /dev/null +++ b/feedback_service/requirements-dev.txt @@ -0,0 +1,4 @@ +-r requirements.txt +httpx>=0.28,<1 +pytest>=8,<9 +ruff>=0.9,<1 diff --git a/feedback_service/requirements.txt b/feedback_service/requirements.txt new file mode 100644 index 0000000..827aded --- /dev/null +++ b/feedback_service/requirements.txt @@ -0,0 +1,2 @@ +fastapi>=0.115,<1 +uvicorn[standard]>=0.34,<1 diff --git a/feedback_service/security.py b/feedback_service/security.py new file mode 100644 index 0000000..27da061 --- /dev/null +++ b/feedback_service/security.py @@ -0,0 +1,59 @@ +from __future__ import annotations + +import hashlib +import secrets +import threading +import time +from collections import defaultdict, deque +from collections.abc import Callable + + +def issue_token() -> str: + return secrets.token_urlsafe(32) + + +def token_hash(token: str) -> str: + return hashlib.sha256(token.encode("utf-8")).hexdigest() + + +class SlidingWindowLimiter: + """Process-local limiter; a reverse proxy must enforce distributed limits.""" + + def __init__( + self, + limit: int, + window_seconds: float = 60, + clock: Callable[[], float] = time.monotonic, + ): + self.limit = limit + self.window_seconds = window_seconds + self.clock = clock + self._events: dict[str, deque[float]] = defaultdict(deque) + self._lock = threading.Lock() + self._last_cleanup = self.clock() + + def allow(self, key: str) -> bool: + now = self.clock() + cutoff = now - self.window_seconds + with self._lock: + if now - self._last_cleanup >= self.window_seconds: + stale_keys = [ + tracked_key + for tracked_key, values in self._events.items() + if not values or values[-1] <= cutoff + ] + for stale_key in stale_keys: + self._events.pop(stale_key, None) + self._last_cleanup = now + events = self._events[key] + while events and events[0] <= cutoff: + events.popleft() + if len(events) >= self.limit: + return False + events.append(now) + return True + + @property + def tracked_keys(self) -> int: + with self._lock: + return len(self._events) diff --git a/feedback_service/tests/__init__.py b/feedback_service/tests/__init__.py new file mode 100644 index 0000000..6dfefdf --- /dev/null +++ b/feedback_service/tests/__init__.py @@ -0,0 +1 @@ +"""Feedback service tests.""" diff --git a/feedback_service/tests/conftest.py b/feedback_service/tests/conftest.py new file mode 100644 index 0000000..6d2c62e --- /dev/null +++ b/feedback_service/tests/conftest.py @@ -0,0 +1,40 @@ +from pathlib import Path + +import pytest +from fastapi.testclient import TestClient + +from feedback_service.config import Settings +from feedback_service.main import create_app + + +@pytest.fixture +def client(tmp_path: Path): + app = create_app( + Settings( + database_path=tmp_path / "feedback.sqlite3", + admin_token="test-admin-token-that-is-at-least-32-characters", + moderator_id="moderator@example.test", + allowed_service_ids=("studyos-agent",), + installation_limit_per_minute=20, + feedback_limit_per_minute=20, + ) + ) + with TestClient(app) as test_client: + yield test_client + + +def issue(client: TestClient) -> str: + response = client.post("/v1/installations") + assert response.status_code == 201 + return response.json()["installation_token"] + + +def auth(token: str) -> dict[str, str]: + return {"Authorization": f"Bearer {token}"} + + +def admin() -> dict[str, str]: + return { + "X-Admin-Token": "test-admin-token-that-is-at-least-32-characters", + "X-Moderator-Id": "moderator@example.test", + } diff --git a/feedback_service/tests/test_api.py b/feedback_service/tests/test_api.py new file mode 100644 index 0000000..196b066 --- /dev/null +++ b/feedback_service/tests/test_api.py @@ -0,0 +1,261 @@ +import sqlite3 +from concurrent.futures import ThreadPoolExecutor + +from fastapi.testclient import TestClient + +from feedback_service.security import token_hash +from feedback_service.tests.conftest import admin, auth, issue + + +MINE = "/v1/services/studyos-agent/feedback/mine" +PUBLIC = "/v1/services/studyos-agent/feedback/public" + + +def put(client: TestClient, token: str, rating: int = 5, comment: str | None = None): + return client.put( + MINE, headers=auth(token), json={"rating": rating, "comment": comment} + ) + + +def test_token_is_stored_hashed_and_feedback_is_owned(client: TestClient): + owner = issue(client) + stranger = issue(client) + created = put(client, owner, 4, "Useful service") + assert created.status_code == 200 + assert created.json()["comment_state"] == "pending" + + assert client.get(MINE, headers=auth(owner)).status_code == 200 + assert client.get(MINE, headers=auth(stranger)).status_code == 404 + assert client.get(MINE, headers=auth("not-a-token")).status_code == 401 + + database = client.app.state.database + with database.connect() as db: + stored = db.execute("SELECT token_hash FROM installations").fetchall() + assert all(row["token_hash"] not in (owner, stranger) for row in stored) + + +def test_rating_comment_bounds_and_plain_text_validation(client: TestClient): + token = issue(client) + for rating in (0, 6, 1.5, "5"): + assert put(client, token, rating).status_code == 422 + assert put(client, token, 5, "x" * 1001).status_code == 422 + assert put(client, token, 5, "bad\x00text").status_code == 422 + assert put(client, token, 1, " \n ").json()["comment_state"] == "none" + + +def test_pending_comment_hidden_but_rating_aggregated(client: TestClient): + first = issue(client) + second = issue(client) + assert put(client, first, 5, "pending").status_code == 200 + assert put(client, second, 3).status_code == 200 + + public = client.get(PUBLIC).json() + assert public["rating"] == {"count": 2, "average": 4.0} + assert public["comments"] == [] + + feedback_id = client.get(MINE, headers=auth(first)).json()["id"] + moderated = client.post( + f"/v1/admin/feedback/{feedback_id}/moderation", + headers=admin(), + json={"action": "publish", "reason": "reviewed"}, + ) + assert moderated.status_code == 200 + comments = client.get(PUBLIC).json()["comments"] + assert comments[0]["comment"] == "pending" + assert set(comments[0]) == {"id", "rating", "comment", "published_at"} + + +def test_report_queue_duplicate_and_comment_deletion_keep_rating(client: TestClient): + owner = issue(client) + reporter = issue(client) + feedback = put(client, owner, 2, "publish me").json() + client.post( + f"/v1/admin/feedback/{feedback['id']}/moderation", + headers=admin(), + json={"action": "publish"}, + ) + report_url = f"/v1/services/studyos-agent/feedback/{feedback['id']}/reports" + assert client.post(report_url, headers=auth(owner), json={}).status_code == 400 + assert ( + client.post( + report_url, headers=auth(reporter), json={"reason": "spam"} + ).status_code + == 201 + ) + assert client.post(report_url, headers=auth(reporter), json={}).status_code == 409 + + queue = client.get("/v1/admin/moderation?state=reported", headers=admin()) + assert queue.status_code == 200 + assert queue.json()["items"][0]["report_count"] == 1 + assert queue.json()["items"][0]["reports"][0]["reason"] == "spam" + + deleted = client.post( + f"/v1/admin/feedback/{feedback['id']}/moderation", + headers=admin(), + json={"action": "delete", "reason": "confirmed"}, + ) + assert deleted.json()["comment_state"] == "deleted" + public = client.get(PUBLIC).json() + assert public["comments"] == [] + assert public["rating"] == {"count": 1, "average": 2.0} + mine = client.get(MINE, headers=auth(owner)).json() + assert mine["comment"] is None + assert mine["comment_state"] == "deleted" + + with client.app.state.database.connect() as db: + audit = db.execute( + """SELECT previous_state, action, reason, moderator_id + FROM moderation_audit ORDER BY created_at""" + ).fetchall() + assert [tuple(row) for row in audit] == [ + ("pending", "publish", None, "moderator@example.test"), + ("published", "delete", "confirmed", "moderator@example.test"), + ] + + # A new comment revision must not inherit reports for the old text. + assert put(client, owner, 4, "new revision").status_code == 200 + client.post( + f"/v1/admin/feedback/{feedback['id']}/moderation", + headers=admin(), + json={"action": "publish"}, + ) + assert ( + client.get("/v1/admin/moderation?state=reported", headers=admin()).json()[ + "items" + ] + == [] + ) + + +def test_owner_delete_removes_rating_and_can_upsert_again(client: TestClient): + token = issue(client) + assert put(client, token, 5, "bye").status_code == 200 + assert client.delete(MINE, headers=auth(token)).status_code == 204 + assert client.get(MINE, headers=auth(token)).status_code == 404 + assert client.get(PUBLIC).json()["rating"]["count"] == 0 + assert put(client, token, 3).status_code == 200 + assert client.get(PUBLIC).json()["rating"] == {"count": 1, "average": 3.0} + + +def test_concurrent_upserts_keep_one_owned_feedback(client: TestClient): + token = issue(client) + database = client.app.state.database + installation_id = database.installation_id(token_hash(token)) + + with ThreadPoolExecutor(max_workers=4) as executor: + results = list( + executor.map( + lambda rating: database.upsert_feedback( + installation_id, + "studyos-agent", + rating, + None, + ), + (1, 2, 3, 4), + ) + ) + + assert all(result is not None for result in results) + with database.connect() as db: + count = db.execute( + "SELECT COUNT(*) FROM feedback WHERE installation_id=?", + (installation_id,), + ).fetchone()[0] + assert count == 1 + + +def test_admin_auth_and_pending_queue(client: TestClient): + token = issue(client) + feedback_id = put(client, token, 5, "review").json()["id"] + assert client.get("/v1/admin/moderation").status_code == 401 + assert ( + client.get( + "/v1/admin/moderation", + headers={ + "X-Admin-Token": "test-admin-token-that-is-at-least-32-characters" + }, + ).status_code + == 400 + ) + queue = client.get("/v1/admin/moderation", headers=admin()).json()["items"] + assert queue[0]["id"] == feedback_id + assert queue[0]["comment_state"] == "pending" + + rejected = client.post( + f"/v1/admin/feedback/{feedback_id}/moderation", + headers=admin(), + json={"action": "reject", "reason": "not suitable"}, + ) + assert rejected.json()["comment_state"] == "rejected" + assert client.get(PUBLIC).json()["comments"] == [] + assert ( + client.post( + f"/v1/admin/feedback/{feedback_id}/moderation", + headers=admin(), + json={"action": "delete"}, + ).status_code + == 422 + ) + + +def test_moderation_transitions_are_serialized_and_validated(client: TestClient): + owner = issue(client) + feedback_id = put(client, owner, 5, "review once").json()["id"] + database = client.app.state.database + + with ThreadPoolExecutor(max_workers=2) as executor: + results = list( + executor.map( + lambda _: database.moderate( + feedback_id, + "publish", + None, + "moderator@example.test", + ), + range(2), + ) + ) + + assert sum(isinstance(result, dict) for result in results) == 1 + assert results.count("invalid") == 1 + with database.connect() as db: + audit = db.execute( + "SELECT previous_state, action FROM moderation_audit WHERE feedback_id=?", + (feedback_id,), + ).fetchall() + assert [tuple(row) for row in audit] == [("pending", "publish")] + + star_only = put(client, issue(client), 3).json()["id"] + response = client.post( + f"/v1/admin/feedback/{star_only}/moderation", + headers=admin(), + json={"action": "publish"}, + ) + assert response.status_code == 409 + + +def test_unknown_service_and_health(client: TestClient): + assert client.get("/healthz").json() == {"status": "ok"} + assert client.get("/v1/services/unknown/feedback/public").status_code == 404 + assert client.get(f"{PUBLIC}?limit=101").status_code == 422 + assert client.get(f"{PUBLIC}?offset=10001").status_code == 422 + assert ( + client.get("/v1/admin/moderation?limit=101", headers=admin()).status_code == 422 + ) + + +def test_database_enforces_foreign_keys_and_wal(client: TestClient): + with client.app.state.database.connect() as db: + assert db.execute("PRAGMA foreign_keys").fetchone()[0] == 1 + assert db.execute("PRAGMA journal_mode").fetchone()[0] == "wal" + assert db.execute("PRAGMA user_version").fetchone()[0] == 1 + try: + db.execute( + """INSERT INTO feedback + (id, installation_id, service_id, rating, comment_state, created_at, updated_at) + VALUES ('bad', 'missing', 'studyos-agent', 5, 'none', 'now', 'now')""" + ) + except sqlite3.IntegrityError: + pass + else: + raise AssertionError("foreign key constraint was not enforced") diff --git a/feedback_service/tests/test_backup.py b/feedback_service/tests/test_backup.py new file mode 100644 index 0000000..285e3a9 --- /dev/null +++ b/feedback_service/tests/test_backup.py @@ -0,0 +1,20 @@ +import sqlite3 +from pathlib import Path + +from feedback_service.backup import create_backup, verify_backup + + +def test_online_backup_can_be_opened_as_a_restored_database(tmp_path: Path): + source = tmp_path / "feedback.sqlite3" + backup = tmp_path / "backups" / "feedback-backup.sqlite3" + with sqlite3.connect(source) as database: + database.execute("CREATE TABLE example (value TEXT NOT NULL)") + database.execute("INSERT INTO example VALUES ('restorable')") + + create_backup(source, backup) + verify_backup(backup) + + with sqlite3.connect(backup) as restored: + assert ( + restored.execute("SELECT value FROM example").fetchone()[0] == "restorable" + ) diff --git a/feedback_service/tests/test_rate_limit.py b/feedback_service/tests/test_rate_limit.py new file mode 100644 index 0000000..fb4339a --- /dev/null +++ b/feedback_service/tests/test_rate_limit.py @@ -0,0 +1,28 @@ +from feedback_service.security import SlidingWindowLimiter + + +def test_sliding_window_limiter_is_deterministic(): + current = [100.0] + limiter = SlidingWindowLimiter(limit=2, window_seconds=60, clock=lambda: current[0]) + + assert limiter.allow("author") + assert limiter.allow("author") + assert not limiter.allow("author") + assert limiter.allow("someone-else") + current[0] = 160.0 + assert limiter.allow("author") + + +def test_sliding_window_evicts_inactive_keys(): + current = [0.0] + limiter = SlidingWindowLimiter( + limit=2, + window_seconds=10, + clock=lambda: current[0], + ) + assert limiter.allow("old-client") + assert limiter.tracked_keys == 1 + + current[0] = 11.0 + assert limiter.allow("new-client") + assert limiter.tracked_keys == 1 diff --git a/flutter_app/README.md b/flutter_app/README.md index f627c0a..6a7f96b 100644 --- a/flutter_app/README.md +++ b/flutter_app/README.md @@ -50,6 +50,22 @@ flutter run -d chrome \ --dart-define=STUDYOS_DEMO_OPENROUTER_API_KEY="$OPENROUTER_API_KEY" ``` +## Feedback Service + +Ratings and comments use the repository's containerized feedback service. Point +the app at an HTTPS deployment at build time; plain HTTP is accepted only for a +loopback development endpoint: + +```sh +flutter run -d chrome \ + --dart-define=STUDYOS_FEEDBACK_API_URL=https://feedback.example.edu +``` + +The endpoint is public configuration, not a secret. The service creates a +pseudonymous installation token on first submission, and the app stores it with +`flutter_secure_storage` for later updates, deletion, and reports. Never put the +service's moderator token or a GitHub credential in a Dart define. + ## Native Bridge The current bridge initializes native Android managers, routes Android messages diff --git a/flutter_app/lib/src/feedback_client.dart b/flutter_app/lib/src/feedback_client.dart index 5f48147..ff05566 100644 --- a/flutter_app/lib/src/feedback_client.dart +++ b/flutter_app/lib/src/feedback_client.dart @@ -1,71 +1,237 @@ +import 'dart:async'; import 'dart:convert'; import 'package:http/http.dart' as http; -const studyOsFeedbackRepository = 'Tue-StudyOS/StudyOS_Agent'; -const studyOsFeedbackToken = String.fromEnvironment('STUDYOS_FEEDBACK_TOKEN'); +import 'feedback_models.dart'; +import 'feedback_token_store.dart'; + +const studyOsFeedbackApiUrl = String.fromEnvironment( + 'STUDYOS_FEEDBACK_API_URL', +); +const studyOsFeedbackServiceId = 'studyos-agent'; class FeedbackClient { - FeedbackClient({http.Client? httpClient}) - : _httpClient = httpClient ?? http.Client(); + FeedbackClient({ + String baseUrl = studyOsFeedbackApiUrl, + http.Client? httpClient, + FeedbackTokenStore? tokenStore, + this.requestTimeout = const Duration(seconds: 10), + }) : _baseUri = _parseBaseUri(baseUrl), + _httpClient = httpClient ?? http.Client(), + _tokenStore = tokenStore ?? SecureFeedbackTokenStore(); + final Uri? _baseUri; final http.Client _httpClient; + final FeedbackTokenStore _tokenStore; + final Duration requestTimeout; + + bool get isConfigured => _baseUri != null; + + Future loadPublic({ + String serviceId = studyOsFeedbackServiceId, + }) async { + final response = await _response( + _httpClient.get( + _uri('/v1/services/${Uri.encodeComponent(serviceId)}/feedback/public'), + headers: _headers(), + ), + ); + final json = _successfulJson(response); + return FeedbackPublicSnapshot.fromJson(json); + } + + Future loadOwn({ + String serviceId = studyOsFeedbackServiceId, + }) async { + final token = await _tokenStore.read(); + if (token == null || token.isEmpty) return null; + final response = await _response( + _httpClient.get( + _uri('/v1/services/${Uri.encodeComponent(serviceId)}/feedback/mine'), + headers: _headers(token: token), + ), + ); + if (response.statusCode == 401) { + await _tokenStore.clear(); + return null; + } + if (response.statusCode == 404) return null; + return FeedbackSubmission.fromJson(_successfulJson(response)); + } - Future submit({ - required String token, - required String message, - required String status, - String repository = studyOsFeedbackRepository, + Future submit({ + required int rating, + String? comment, + String serviceId = studyOsFeedbackServiceId, }) async { - final trimmedMessage = message.trim(); - if (trimmedMessage.isEmpty) { - throw const FeedbackException('Write a short feedback note first.'); + if (rating < 1 || rating > 5) { + throw const FeedbackException('Choose a rating from 1 to 5 stars.'); } - final trimmedToken = token.trim(); - if (trimmedToken.isEmpty) { - throw const FeedbackException('Feedback is not set up yet.'); + final trimmedComment = comment?.trim() ?? ''; + if (trimmedComment.length > 1000) { + throw const FeedbackException( + 'Keep the feedback comment under 1,000 characters.', + ); } - if (!repository.contains('/')) { - throw const FeedbackException('Feedback repository is invalid.'); + final body = jsonEncode({ + 'rating': rating, + 'comment': trimmedComment.isEmpty ? null : trimmedComment, + }); + var token = await _installationToken(); + var response = await _response( + _httpClient.put( + _uri('/v1/services/${Uri.encodeComponent(serviceId)}/feedback/mine'), + headers: _headers(token: token), + body: body, + ), + ); + if (response.statusCode == 401) { + await _tokenStore.clear(); + token = await _installationToken(); + response = await _response( + _httpClient.put( + _uri('/v1/services/${Uri.encodeComponent(serviceId)}/feedback/mine'), + headers: _headers(token: token), + body: body, + ), + ); } + return FeedbackSubmission.fromJson(_successfulJson(response)); + } - final response = await _httpClient.post( - Uri.https('api.github.com', '/repos/$repository/issues'), - headers: { - 'Accept': 'application/vnd.github+json', - 'Authorization': 'Bearer $trimmedToken', - 'Content-Type': 'application/json', - 'X-GitHub-Api-Version': '2022-11-28', - }, - body: jsonEncode({ - 'title': _titleFrom(trimmedMessage), - 'body': _bodyFrom(trimmedMessage, status), - }), + Future deleteOwn({String serviceId = studyOsFeedbackServiceId}) async { + final token = await _tokenStore.read(); + if (token == null || token.isEmpty) { + throw const FeedbackException('There is no saved feedback to delete.'); + } + final response = await _response( + _httpClient.delete( + _uri('/v1/services/${Uri.encodeComponent(serviceId)}/feedback/mine'), + headers: _headers(token: token), + ), ); - if (response.statusCode < 200 || response.statusCode >= 300) { - throw FeedbackException( - 'Feedback failed with HTTP ${response.statusCode}.', + if (response.statusCode != 204) _successfulJson(response); + } + + Future report({ + required String feedbackId, + String? reason, + String serviceId = studyOsFeedbackServiceId, + }) async { + final body = jsonEncode({'reason': reason?.trim()}); + var token = await _installationToken(); + final service = Uri.encodeComponent(serviceId); + final feedback = Uri.encodeComponent(feedbackId); + var response = await _response( + _httpClient.post( + _uri('/v1/services/$service/feedback/$feedback/reports'), + headers: _headers(token: token), + body: body, + ), + ); + if (response.statusCode == 401) { + await _tokenStore.clear(); + token = await _installationToken(); + response = await _response( + _httpClient.post( + _uri('/v1/services/$service/feedback/$feedback/reports'), + headers: _headers(token: token), + body: body, + ), + ); + } + _successfulJson(response); + } + + Future _installationToken() async { + final existing = await _tokenStore.read(); + if (existing != null && existing.isNotEmpty) return existing; + final response = await _response( + _httpClient.post(_uri('/v1/installations'), headers: _headers()), + ); + final json = _successfulJson(response); + final token = json['installation_token']?.toString() ?? ''; + if (token.length < 32) { + throw const FeedbackException( + 'The feedback service returned an invalid installation token.', ); } + await _tokenStore.write(token); + return token; + } + + Uri _uri(String path) { + final baseUri = _baseUri; + if (baseUri == null) { + throw const FeedbackException('Feedback is not configured yet.'); + } + final relativePath = path.startsWith('/') ? path.substring(1) : path; + final basePath = baseUri.path.endsWith('/') + ? baseUri.path + : '${baseUri.path}/'; + return baseUri.replace(path: '$basePath$relativePath'); } - String _bodyFrom(String message, String status) { - final buffer = StringBuffer() - ..writeln(message) - ..writeln() - ..writeln('---') - ..writeln('- Source: StudyOS mobile app'); - final trimmedStatus = status.trim(); - if (trimmedStatus.isNotEmpty) { - buffer.writeln('- App status: $trimmedStatus'); + Map _headers({String? token}) => { + 'Accept': 'application/json', + 'Content-Type': 'application/json', + if (token != null) 'Authorization': 'Bearer $token', + }; + + Future _response(Future request) async { + try { + return await request.timeout(requestTimeout); + } on TimeoutException { + throw const FeedbackException('The feedback service timed out.'); + } on http.ClientException { + throw const FeedbackException('The feedback service is unreachable.'); + } + } + + Map _successfulJson(http.Response response) { + Map json = const {}; + if (response.body.isNotEmpty) { + try { + final decoded = jsonDecode(response.body); + if (decoded is Map) json = Map.from(decoded); + } on FormatException { + if (response.statusCode >= 200 && response.statusCode < 300) { + throw const FeedbackException( + 'The feedback service returned an invalid response.', + ); + } + } + } + if (response.statusCode < 200 || response.statusCode >= 300) { + final rawDetail = json['detail']?.toString().trim(); + final detail = rawDetail != null && rawDetail.length > 240 + ? '${rawDetail.substring(0, 237)}...' + : rawDetail; + throw FeedbackException( + detail == null || detail.isEmpty + ? 'Feedback failed with HTTP ${response.statusCode}.' + : detail, + ); } - return buffer.toString().trimRight(); + return json; } - String _titleFrom(String message) { - final firstLine = message.split('\n').first.trim(); - if (firstLine.length <= 72) return firstLine; - return '${firstLine.substring(0, 69)}...'; + static Uri? _parseBaseUri(String value) { + final uri = Uri.tryParse(value.trim()); + if (uri == null || + !uri.hasAuthority || + uri.userInfo.isNotEmpty || + uri.hasQuery || + uri.hasFragment) { + return null; + } + final loopback = + uri.host == 'localhost' || uri.host == '127.0.0.1' || uri.host == '::1'; + if (uri.scheme != 'https' && !(loopback && uri.scheme == 'http')) { + return null; + } + return uri; } } diff --git a/flutter_app/lib/src/feedback_models.dart b/flutter_app/lib/src/feedback_models.dart new file mode 100644 index 0000000..596ecae --- /dev/null +++ b/flutter_app/lib/src/feedback_models.dart @@ -0,0 +1,120 @@ +class FeedbackSubmission { + const FeedbackSubmission({ + required this.id, + required this.serviceId, + required this.rating, + required this.comment, + required this.commentState, + required this.createdAt, + required this.updatedAt, + }); + + final String id; + final String serviceId; + final int rating; + final String? comment; + final String commentState; + final DateTime createdAt; + final DateTime updatedAt; + + bool get commentIsPending => commentState == 'pending'; + + String? get commentStatusMessage => switch (commentState) { + 'pending' => 'Your comment is awaiting moderator review.', + 'published' => 'Your comment is published.', + 'rejected' => 'Your comment was not published after review.', + 'deleted' => 'Your comment was removed; your rating is still counted.', + _ => null, + }; + + factory FeedbackSubmission.fromJson(Map json) { + return FeedbackSubmission( + id: json['id']?.toString() ?? '', + serviceId: json['service_id']?.toString() ?? '', + rating: (json['rating'] as num?)?.toInt() ?? 0, + comment: _optionalText(json['comment']), + commentState: json['comment_state']?.toString() ?? 'none', + createdAt: _dateTime(json['created_at']), + updatedAt: _dateTime(json['updated_at']), + ); + } +} + +class PublishedFeedbackComment { + const PublishedFeedbackComment({ + required this.id, + required this.rating, + required this.comment, + required this.publishedAt, + }); + + final String id; + final int rating; + final String comment; + final DateTime publishedAt; + + factory PublishedFeedbackComment.fromJson(Map json) { + return PublishedFeedbackComment( + id: json['id']?.toString() ?? '', + rating: (json['rating'] as num?)?.toInt() ?? 0, + comment: json['comment']?.toString() ?? '', + publishedAt: _dateTime(json['published_at']), + ); + } +} + +class FeedbackPublicSnapshot { + const FeedbackPublicSnapshot({ + required this.serviceId, + required this.ratingCount, + required this.averageRating, + required this.comments, + }); + + const FeedbackPublicSnapshot.empty(String serviceId) + : this( + serviceId: serviceId, + ratingCount: 0, + averageRating: null, + comments: const [], + ); + + final String serviceId; + final int ratingCount; + final double? averageRating; + final List comments; + + factory FeedbackPublicSnapshot.fromJson(Map json) { + final rawRating = json['rating']; + final rating = rawRating is Map + ? Map.from(rawRating) + : const {}; + final rawComments = json['comments']; + return FeedbackPublicSnapshot( + serviceId: json['service_id']?.toString() ?? '', + ratingCount: (rating['count'] as num?)?.toInt() ?? 0, + averageRating: (rating['average'] as num?)?.toDouble(), + comments: rawComments is List + ? rawComments + .whereType() + .map( + (item) => PublishedFeedbackComment.fromJson( + Map.from(item), + ), + ) + .where((item) => item.id.isNotEmpty && item.comment.isNotEmpty) + .toList(growable: false) + : const [], + ); + } +} + +String? _optionalText(Object? value) { + final text = value?.toString(); + return text == null || text.isEmpty ? null : text; +} + +DateTime _dateTime(Object? value) { + return DateTime.tryParse(value?.toString() ?? '') ?? + DateTime.fromMillisecondsSinceEpoch(0, isUtc: true); +} diff --git a/flutter_app/lib/src/feedback_token_store.dart b/flutter_app/lib/src/feedback_token_store.dart new file mode 100644 index 0000000..f57ac40 --- /dev/null +++ b/flutter_app/lib/src/feedback_token_store.dart @@ -0,0 +1,27 @@ +import 'package:flutter_secure_storage/flutter_secure_storage.dart'; + +abstract interface class FeedbackTokenStore { + Future read(); + + Future write(String token); + + Future clear(); +} + +class SecureFeedbackTokenStore implements FeedbackTokenStore { + SecureFeedbackTokenStore({FlutterSecureStorage? storage}) + : _storage = storage ?? const FlutterSecureStorage(); + + static const String _key = 'studyos.feedback.installationToken.v1'; + + final FlutterSecureStorage _storage; + + @override + Future read() => _storage.read(key: _key); + + @override + Future write(String token) => _storage.write(key: _key, value: token); + + @override + Future clear() => _storage.delete(key: _key); +} diff --git a/flutter_app/lib/src/views/settings_view.dart b/flutter_app/lib/src/views/settings_view.dart index aa45ee6..77dcd82 100644 --- a/flutter_app/lib/src/views/settings_view.dart +++ b/flutter_app/lib/src/views/settings_view.dart @@ -236,9 +236,7 @@ class _SettingsViewState extends State { const SizedBox(height: StudyOsSpacing.xl), _SettingsSection( title: 'Support', - child: SettingsCard( - children: [FeedbackSettingsCard(status: widget.status)], - ), + child: SettingsCard(children: const [FeedbackSettingsCard()]), ), ], ); diff --git a/flutter_app/lib/src/widgets/feedback_components.dart b/flutter_app/lib/src/widgets/feedback_components.dart new file mode 100644 index 0000000..e75a892 --- /dev/null +++ b/flutter_app/lib/src/widgets/feedback_components.dart @@ -0,0 +1,88 @@ +import 'package:flutter/material.dart'; + +import '../feedback_models.dart'; + +class FeedbackRatingSummary extends StatelessWidget { + const FeedbackRatingSummary({required this.snapshot, super.key}); + + final FeedbackPublicSnapshot? snapshot; + + @override + Widget build(BuildContext context) { + final count = snapshot?.ratingCount ?? 0; + final average = snapshot?.averageRating; + return Text( + count == 0 + ? 'No ratings yet.' + : '${average?.toStringAsFixed(1) ?? '–'} / 5 from $count ${count == 1 ? 'rating' : 'ratings'}', + style: Theme.of(context).textTheme.bodyMedium, + ); + } +} + +class FeedbackStarSelector extends StatelessWidget { + const FeedbackStarSelector({ + required this.value, + required this.onChanged, + super.key, + }); + + final int value; + final ValueChanged onChanged; + + @override + Widget build(BuildContext context) => Semantics( + label: value == 0 ? 'No rating selected' : '$value out of 5 stars selected', + child: Row( + children: List.generate(5, (index) { + final rating = index + 1; + return IconButton( + tooltip: '$rating ${rating == 1 ? 'star' : 'stars'}', + onPressed: () => onChanged(rating), + icon: Icon( + rating <= value ? Icons.star_rounded : Icons.star_outline_rounded, + color: rating <= value ? Colors.amber.shade700 : null, + ), + ); + }), + ), + ); +} + +class PublishedFeedbackComments extends StatelessWidget { + const PublishedFeedbackComments({ + required this.comments, + required this.reportingId, + required this.onReport, + super.key, + }); + + final List comments; + final String? reportingId; + final ValueChanged onReport; + + @override + Widget build(BuildContext context) => Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text('Published comments', style: Theme.of(context).textTheme.titleSmall), + ...comments.map( + (comment) => ListTile( + contentPadding: EdgeInsets.zero, + title: Text(comment.comment), + subtitle: Text('${comment.rating} / 5 stars'), + trailing: IconButton( + tooltip: 'Report comment', + onPressed: reportingId == null ? () => onReport(comment) : null, + icon: reportingId == comment.id + ? const SizedBox.square( + dimension: 18, + child: CircularProgressIndicator(strokeWidth: 2), + ) + : const Icon(Icons.flag_outlined), + ), + ), + ), + ], + ); +} diff --git a/flutter_app/lib/src/widgets/feedback_settings_card.dart b/flutter_app/lib/src/widgets/feedback_settings_card.dart index 00c602c..1674dcf 100644 --- a/flutter_app/lib/src/widgets/feedback_settings_card.dart +++ b/flutter_app/lib/src/widgets/feedback_settings_card.dart @@ -1,12 +1,13 @@ import 'package:flutter/material.dart'; import '../feedback_client.dart'; +import '../feedback_models.dart'; import '../studyos_theme.dart'; +import 'feedback_components.dart'; class FeedbackSettingsCard extends StatefulWidget { - const FeedbackSettingsCard({required this.status, this.client, super.key}); + const FeedbackSettingsCard({this.client, super.key}); - final String status; final FeedbackClient? client; @override @@ -15,12 +16,22 @@ class FeedbackSettingsCard extends StatefulWidget { class _FeedbackSettingsCardState extends State { late final TextEditingController _controller; + late final FeedbackClient _client; + FeedbackPublicSnapshot? _snapshot; + FeedbackSubmission? _ownFeedback; + String? _error; + String? _reportingId; + int _rating = 0; + bool _isLoading = true; bool _isSending = false; + bool _isDeleting = false; @override void initState() { super.initState(); _controller = TextEditingController(); + _client = widget.client ?? FeedbackClient(); + _load(); } @override @@ -29,16 +40,52 @@ class _FeedbackSettingsCardState extends State { super.dispose(); } + Future _load() async { + if (!_client.isConfigured) { + if (mounted) setState(() => _isLoading = false); + return; + } + try { + final snapshot = await _client.loadPublic(); + final own = await _client.loadOwn(); + if (!mounted) return; + setState(() { + _snapshot = snapshot; + _ownFeedback = own; + _rating = own?.rating ?? 0; + _controller.text = own?.comment ?? ''; + _error = null; + }); + } on FeedbackException catch (error) { + if (mounted) setState(() => _error = error.message); + } finally { + if (mounted) setState(() => _isLoading = false); + } + } + Future _send() async { + if (_rating == 0) { + _showMessage('Choose a star rating first.'); + return; + } setState(() => _isSending = true); try { - await (widget.client ?? FeedbackClient()).submit( - token: studyOsFeedbackToken, - message: _controller.text, - status: widget.status, + final own = await _client.submit( + rating: _rating, + comment: _controller.text, + ); + final snapshot = await _client.loadPublic(); + if (!mounted) return; + setState(() { + _ownFeedback = own; + _snapshot = snapshot; + _error = null; + }); + _showMessage( + own.commentIsPending + ? 'Rating saved. Your comment is awaiting review.' + : 'Rating saved.', ); - _controller.clear(); - _showMessage('Feedback sent.'); } on FeedbackException catch (error) { _showMessage(error.message); } finally { @@ -46,6 +93,85 @@ class _FeedbackSettingsCardState extends State { } } + Future _delete() async { + final confirmed = await showDialog( + context: context, + builder: (context) => AlertDialog( + title: const Text('Delete your feedback?'), + content: const Text( + 'Your rating and comment will disappear from public results.', + ), + actions: [ + TextButton( + onPressed: () => Navigator.pop(context, false), + child: const Text('Cancel'), + ), + FilledButton( + onPressed: () => Navigator.pop(context, true), + child: const Text('Delete'), + ), + ], + ), + ); + if (confirmed != true || !mounted) return; + setState(() => _isDeleting = true); + try { + await _client.deleteOwn(); + final snapshot = await _client.loadPublic(); + if (!mounted) return; + setState(() { + _ownFeedback = null; + _snapshot = snapshot; + _rating = 0; + _controller.clear(); + }); + _showMessage('Feedback deleted.'); + } on FeedbackException catch (error) { + _showMessage(error.message); + } finally { + if (mounted) setState(() => _isDeleting = false); + } + } + + Future _report(PublishedFeedbackComment comment) async { + var reportReason = ''; + final reason = await showDialog( + context: context, + builder: (context) => AlertDialog( + title: const Text('Report comment'), + content: TextField( + maxLength: 500, + maxLines: 3, + onChanged: (value) => reportReason = value, + decoration: const InputDecoration( + labelText: 'Reason (optional)', + hintText: 'Spam, abuse, personal data…', + ), + ), + actions: [ + TextButton( + onPressed: () => Navigator.pop(context), + child: const Text('Cancel'), + ), + FilledButton( + onPressed: () => Navigator.pop(context, reportReason), + child: const Text('Report'), + ), + ], + ), + ); + if (reason == null || !mounted) return; + setState(() => _reportingId = comment.id); + try { + await _client.report(feedbackId: comment.id, reason: reason); + _showMessage('Comment reported for review.'); + } on FeedbackException catch (error) { + _showMessage(error.message); + } finally { + if (mounted) setState(() => _reportingId = null); + } + } + void _showMessage(String text) { if (!mounted) return; ScaffoldMessenger.of(context).showSnackBar(SnackBar(content: Text(text))); @@ -53,44 +179,105 @@ class _FeedbackSettingsCardState extends State { @override Widget build(BuildContext context) { + if (!_client.isConfigured) { + return const _UnavailableFeedback(); + } return Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ - Text( - 'Help improve StudyOS', - style: Theme.of(context).textTheme.titleMedium, - ), + Text('Rate StudyOS', style: Theme.of(context).textTheme.titleMedium), const SizedBox(height: StudyOsSpacing.xs), Text( - 'Tell us what worked or what got in your way.', + 'Ratings are public. Comments appear after moderation.', style: Theme.of(context).textTheme.bodyMedium, ), - const SizedBox(height: StudyOsSpacing.md), - TextField( - controller: _controller, - minLines: 3, - maxLines: 6, - textCapitalization: TextCapitalization.sentences, - decoration: const InputDecoration( - labelText: 'Feedback', - hintText: 'What should we improve?', + const SizedBox(height: StudyOsSpacing.sm), + if (_isLoading) + const LinearProgressIndicator() + else ...[ + FeedbackRatingSummary(snapshot: _snapshot), + const SizedBox(height: StudyOsSpacing.sm), + FeedbackStarSelector( + value: _rating, + onChanged: (value) => setState(() => _rating = value), ), - ), - const SizedBox(height: StudyOsSpacing.md), - SizedBox( - width: double.infinity, - child: OutlinedButton.icon( - onPressed: _isSending ? null : _send, - icon: _isSending - ? const SizedBox.square( - dimension: 18, - child: CircularProgressIndicator(strokeWidth: 2), - ) - : const Icon(Icons.feedback_outlined), - label: const Text('Send feedback'), + const SizedBox(height: StudyOsSpacing.sm), + TextField( + controller: _controller, + minLines: 2, + maxLines: 5, + maxLength: 1000, + textCapitalization: TextCapitalization.sentences, + decoration: const InputDecoration( + labelText: 'Comment (optional)', + hintText: 'What worked, or what should improve?', + ), ), - ), + if (_error != null) ...[ + Text( + _error!, + style: TextStyle(color: Theme.of(context).colorScheme.error), + ), + const SizedBox(height: StudyOsSpacing.sm), + ], + Row( + children: [ + Expanded( + child: OutlinedButton.icon( + onPressed: _isSending || _isDeleting ? null : _send, + icon: _isSending + ? const SizedBox.square( + dimension: 18, + child: CircularProgressIndicator(strokeWidth: 2), + ) + : const Icon(Icons.star_outline_rounded), + label: Text( + _ownFeedback == null ? 'Send feedback' : 'Update feedback', + ), + ), + ), + if (_ownFeedback != null) ...[ + const SizedBox(width: StudyOsSpacing.sm), + IconButton( + tooltip: 'Delete my feedback', + onPressed: _isSending || _isDeleting ? null : _delete, + icon: _isDeleting + ? const SizedBox.square( + dimension: 18, + child: CircularProgressIndicator(strokeWidth: 2), + ) + : const Icon(Icons.delete_outline), + ), + ], + ], + ), + if (_ownFeedback?.commentStatusMessage != null) + Padding( + padding: const EdgeInsets.only(top: StudyOsSpacing.xs), + child: Text(_ownFeedback!.commentStatusMessage!), + ), + if (_snapshot?.comments.isNotEmpty == true) ...[ + const Divider(height: StudyOsSpacing.xl), + PublishedFeedbackComments( + comments: _snapshot!.comments, + reportingId: _reportingId, + onReport: _report, + ), + ], + ], ], ); } } + +class _UnavailableFeedback extends StatelessWidget { + const _UnavailableFeedback(); + + @override + Widget build(BuildContext context) => const ListTile( + contentPadding: EdgeInsets.zero, + leading: Icon(Icons.feedback_outlined), + title: Text('Feedback is unavailable'), + subtitle: Text('This build is not connected to a feedback service.'), + ); +} diff --git a/flutter_app/test/feedback_client_test.dart b/flutter_app/test/feedback_client_test.dart index f1a0e8c..5cf4010 100644 --- a/flutter_app/test/feedback_client_test.dart +++ b/flutter_app/test/feedback_client_test.dart @@ -1,41 +1,287 @@ +import 'dart:async'; +import 'dart:convert'; + import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; import 'package:http/testing.dart'; import 'package:studyos_agent/src/feedback_client.dart'; +import 'package:studyos_agent/src/feedback_token_store.dart'; void main() { - test('feedback client creates a GitHub issue directly', () async { - late Uri requestUri; - late String authorization; - late String requestBody; + test('submit bootstraps an installation and stores only its token', () async { + final requests = []; + final tokenStore = _MemoryTokenStore(); + final client = FeedbackClient( + baseUrl: 'https://feedback.studyos.test', + tokenStore: tokenStore, + httpClient: MockClient((request) async { + requests.add(request); + if (request.url.path == '/v1/installations') { + return http.Response( + jsonEncode({ + 'installation_token': _newToken, + 'token_type': 'bearer', + }), + 201, + ); + } + return http.Response(_submissionJson, 200); + }), + ); + + final feedback = await client.submit( + rating: 4, + comment: ' Useful, but needs clearer sources. ', + ); + + expect(requests.map((item) => item.url.path), [ + '/v1/installations', + '/v1/services/studyos-agent/feedback/mine', + ]); + expect(requests.last.headers['Authorization'], 'Bearer $_newToken'); + expect(requests.last.body, contains('Useful, but needs clearer sources.')); + expect(tokenStore.token, _newToken); + expect(feedback.rating, 4); + expect(feedback.commentIsPending, isTrue); + }); + + test('existing installation token is reused for updates', () async { + var requestCount = 0; + final client = FeedbackClient( + baseUrl: 'https://feedback.studyos.test', + tokenStore: _MemoryTokenStore( + 'saved-installation-token-that-is-long-enough', + ), + httpClient: MockClient((request) async { + requestCount += 1; + expect( + request.headers['Authorization'], + 'Bearer saved-installation-token-that-is-long-enough', + ); + return http.Response(_submissionJson, 200); + }), + ); + + await client.submit(rating: 5); + + expect(requestCount, 1); + }); + + test( + 'stale installation token is replaced once after unauthorized', + () async { + final tokenStore = _MemoryTokenStore( + 'stale-installation-token-that-is-long-enough', + ); + var putCount = 0; + final client = FeedbackClient( + baseUrl: 'https://feedback.studyos.test', + tokenStore: tokenStore, + httpClient: MockClient((request) async { + if (request.method == 'PUT') { + putCount += 1; + return putCount == 1 + ? http.Response('{"detail":"invalid bearer token"}', 401) + : http.Response(_submissionJson, 200); + } + expect(request.url.path, '/v1/installations'); + return http.Response( + jsonEncode({ + 'installation_token': _newToken, + 'token_type': 'bearer', + }), + 201, + ); + }), + ); + + await client.submit(rating: 4); + + expect(putCount, 2); + expect(tokenStore.token, _newToken); + }, + ); + + test('public response parses aggregate and published comments', () async { + final client = FeedbackClient( + baseUrl: 'https://feedback.studyos.test', + tokenStore: _MemoryTokenStore(), + httpClient: MockClient( + (_) async => http.Response( + jsonEncode({ + 'service_id': 'studyos-agent', + 'rating': {'count': 2, 'average': 4.5}, + 'comments': [ + { + 'id': 'feedback-1', + 'rating': 4, + 'comment': 'Clear and useful', + 'published_at': '2026-07-16T16:00:00Z', + }, + ], + }), + 200, + ), + ), + ); + + final snapshot = await client.loadPublic(); + + expect(snapshot.ratingCount, 2); + expect(snapshot.averageRating, 4.5); + expect(snapshot.comments.single.comment, 'Clear and useful'); + }); + + test('delete and report use the stored installation identity', () async { + final requests = []; final client = FeedbackClient( + baseUrl: 'https://feedback.studyos.test', + tokenStore: _MemoryTokenStore( + 'saved-installation-token-that-is-long-enough', + ), httpClient: MockClient((request) async { - requestUri = request.url; - authorization = request.headers['Authorization'] ?? ''; - requestBody = request.body; - return http.Response('{}', 201); + requests.add(request); + return request.method == 'DELETE' + ? http.Response('', 204) + : http.Response('{"status":"recorded"}', 201); }), ); - await client.submit( - token: 'github-token', - message: 'Mensa widget would be useful', - status: 'Ready', + await client.report(feedbackId: 'feedback-1', reason: 'spam'); + await client.deleteOwn(); + + expect( + requests.first.url.path, + '/v1/services/studyos-agent/feedback/feedback-1/reports', + ); + expect(requests.first.body, contains('spam')); + expect(requests.last.method, 'DELETE'); + expect( + requests.every( + (request) => + request.headers['Authorization'] == + 'Bearer saved-installation-token-that-is-long-enough', + ), + isTrue, + ); + }); + + test('loadOwn does not create an installation just to read', () async { + final client = FeedbackClient( + baseUrl: 'https://feedback.studyos.test', + tokenStore: _MemoryTokenStore(), + httpClient: MockClient((_) async => throw StateError('not expected')), + ); + + expect(await client.loadOwn(), isNull); + }); + + test('rejects insecure non-loopback endpoint and invalid ratings', () async { + final insecure = FeedbackClient( + baseUrl: 'http://feedback.example.com', + tokenStore: _MemoryTokenStore(), + ); + final local = FeedbackClient( + baseUrl: 'http://localhost:8080', + tokenStore: _MemoryTokenStore(), + ); + + expect(insecure.isConfigured, isFalse); + expect(local.isConfigured, isTrue); + expect(() => local.submit(rating: 0), throwsA(isA())); + }); + + test('surfaces bounded API error details', () async { + final client = FeedbackClient( + baseUrl: 'https://feedback.studyos.test', + tokenStore: _MemoryTokenStore( + 'saved-installation-token-that-is-long-enough', + ), + httpClient: MockClient( + (_) async => http.Response( + jsonEncode({'detail': 'Rating is rate limited.'}), + 429, + ), + ), + ); + + expect( + () => client.submit(rating: 3), + throwsA( + isA().having( + (error) => error.message, + 'message', + 'Rating is rate limited.', + ), + ), + ); + }); + + test('maps a stalled request to a feedback timeout', () async { + final client = FeedbackClient( + baseUrl: 'https://feedback.studyos.test', + tokenStore: _MemoryTokenStore(), + requestTimeout: const Duration(milliseconds: 1), + httpClient: MockClient((_) => Completer().future), ); - expect(requestUri.host, 'api.github.com'); - expect(requestUri.path, '/repos/Tue-StudyOS/StudyOS_Agent/issues'); - expect(authorization, 'Bearer github-token'); - expect(requestBody, contains('Mensa widget would be useful')); - expect(requestBody, contains('StudyOS mobile app')); + expect( + () => client.loadPublic(), + throwsA( + isA().having( + (error) => error.message, + 'message', + 'The feedback service timed out.', + ), + ), + ); }); - test('feedback client rejects missing token', () async { - final client = FeedbackClient(); + test('maps transport failures to an unreachable error', () async { + final client = FeedbackClient( + baseUrl: 'https://feedback.studyos.test', + tokenStore: _MemoryTokenStore(), + httpClient: MockClient( + (_) async => throw http.ClientException('offline'), + ), + ); expect( - () => client.submit(token: '', message: 'Hello', status: 'Ready'), - throwsA(isA()), + () => client.loadPublic(), + throwsA( + isA().having( + (error) => error.message, + 'message', + 'The feedback service is unreachable.', + ), + ), ); }); } + +class _MemoryTokenStore implements FeedbackTokenStore { + _MemoryTokenStore([this.token]); + + String? token; + + @override + Future clear() async => token = null; + + @override + Future read() async => token; + + @override + Future write(String value) async => token = value; +} + +final String _submissionJson = jsonEncode({ + 'id': 'feedback-1', + 'service_id': 'studyos-agent', + 'rating': 4, + 'comment': 'Useful, but needs clearer sources.', + 'comment_state': 'pending', + 'created_at': '2026-07-16T16:00:00Z', + 'updated_at': '2026-07-16T16:00:00Z', +}); + +const String _newToken = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'; diff --git a/flutter_app/test/feedback_settings_card_test.dart b/flutter_app/test/feedback_settings_card_test.dart new file mode 100644 index 0000000..db523b2 --- /dev/null +++ b/flutter_app/test/feedback_settings_card_test.dart @@ -0,0 +1,265 @@ +import 'dart:convert'; + +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:studyos_agent/src/feedback_client.dart'; +import 'package:studyos_agent/src/feedback_token_store.dart'; +import 'package:studyos_agent/src/widgets/feedback_settings_card.dart'; + +void main() { + testWidgets('submits stars and comment through the feedback API', ( + tester, + ) async { + final requests = []; + final client = FeedbackClient( + baseUrl: 'https://feedback.studyos.test', + tokenStore: _MemoryTokenStore(), + httpClient: MockClient((request) async { + requests.add(request); + if (request.url.path == '/v1/installations') { + return http.Response( + jsonEncode({ + 'installation_token': _token, + 'token_type': 'bearer', + }), + 201, + ); + } + if (request.method == 'PUT') { + return http.Response( + jsonEncode({ + 'id': 'feedback-1', + 'service_id': 'studyos-agent', + 'rating': 4, + 'comment': 'Helpful service catalog', + 'comment_state': 'pending', + 'created_at': '2026-07-16T16:00:00Z', + 'updated_at': '2026-07-16T16:00:00Z', + }), + 200, + ); + } + return http.Response( + jsonEncode({ + 'service_id': 'studyos-agent', + 'rating': { + 'count': request.method == 'GET' && requests.length > 3 ? 1 : 0, + 'average': requests.length > 3 ? 4.0 : null, + }, + 'comments': [], + }), + 200, + ); + }), + ); + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SingleChildScrollView( + child: FeedbackSettingsCard(client: client), + ), + ), + ), + ); + await tester.pumpAndSettle(); + + await tester.tap(find.byTooltip('4 stars')); + await tester.enterText(find.byType(TextField), 'Helpful service catalog'); + await tester.tap(find.text('Send feedback')); + await tester.pumpAndSettle(); + + expect(find.text('Update feedback'), findsOneWidget); + expect( + find.text('Your comment is awaiting moderator review.'), + findsOneWidget, + ); + final update = requests.singleWhere((request) => request.method == 'PUT'); + expect(update.body, contains('"rating":4')); + expect(update.body, contains('Helpful service catalog')); + }); + + testWidgets('shows an explicit unavailable state without an endpoint', ( + tester, + ) async { + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: FeedbackSettingsCard( + client: FeedbackClient( + baseUrl: '', + tokenStore: _MemoryTokenStore(), + ), + ), + ), + ), + ); + await tester.pumpAndSettle(); + + expect(find.text('Feedback is unavailable'), findsOneWidget); + expect(find.text('Send feedback'), findsNothing); + }); + + testWidgets('shows the moderator state of an existing comment', ( + tester, + ) async { + final client = FeedbackClient( + baseUrl: 'https://feedback.studyos.test', + tokenStore: _MemoryTokenStore(_token), + httpClient: MockClient((request) async { + if (request.url.path.endsWith('/public')) { + return http.Response( + '{"service_id":"studyos-agent","rating":{"count":1,"average":2.0},"comments":[]}', + 200, + ); + } + return http.Response( + jsonEncode({ + 'id': 'feedback-1', + 'service_id': 'studyos-agent', + 'rating': 2, + 'comment': 'Needs work', + 'comment_state': 'rejected', + 'created_at': '2026-07-16T16:00:00Z', + 'updated_at': '2026-07-16T16:00:00Z', + }), + 200, + ); + }), + ); + + await tester.pumpWidget( + MaterialApp( + home: Scaffold(body: FeedbackSettingsCard(client: client)), + ), + ); + await tester.pumpAndSettle(); + + expect( + find.text('Your comment was not published after review.'), + findsOneWidget, + ); + expect(find.text('Update feedback'), findsOneWidget); + }); + + testWidgets('confirms and deletes owned feedback', (tester) async { + final requests = []; + var deleted = false; + final client = FeedbackClient( + baseUrl: 'https://feedback.studyos.test', + tokenStore: _MemoryTokenStore(_token), + httpClient: MockClient((request) async { + requests.add(request); + if (request.method == 'DELETE') { + deleted = true; + return http.Response('', 204); + } + if (request.url.path.endsWith('/public')) { + return http.Response( + '{"service_id":"studyos-agent","rating":{"count":${deleted ? 0 : 1},"average":${deleted ? 'null' : '5.0'}},"comments":[]}', + 200, + ); + } + return http.Response( + jsonEncode({ + 'id': 'feedback-1', + 'service_id': 'studyos-agent', + 'rating': 5, + 'comment': null, + 'comment_state': 'none', + 'created_at': '2026-07-16T16:00:00Z', + 'updated_at': '2026-07-16T16:00:00Z', + }), + 200, + ); + }), + ); + + await tester.pumpWidget( + MaterialApp( + home: Scaffold(body: FeedbackSettingsCard(client: client)), + ), + ); + await tester.pumpAndSettle(); + await tester.tap(find.byTooltip('Delete my feedback')); + await tester.pumpAndSettle(); + await tester.tap(find.widgetWithText(FilledButton, 'Delete')); + await tester.pumpAndSettle(); + + expect(requests.any((request) => request.method == 'DELETE'), isTrue); + expect(find.text('Send feedback'), findsOneWidget); + expect(find.text('No ratings yet.'), findsOneWidget); + }); + + testWidgets('reports a published comment with a reason', (tester) async { + final requests = []; + final client = FeedbackClient( + baseUrl: 'https://feedback.studyos.test', + tokenStore: _MemoryTokenStore(_token), + httpClient: MockClient((request) async { + requests.add(request); + if (request.method == 'POST') { + return http.Response('{"status":"recorded"}', 201); + } + if (request.url.path.endsWith('/mine')) { + return http.Response('{"detail":"feedback not found"}', 404); + } + return http.Response( + jsonEncode({ + 'service_id': 'studyos-agent', + 'rating': {'count': 1, 'average': 1.0}, + 'comments': [ + { + 'id': 'feedback-other', + 'rating': 1, + 'comment': 'Contains personal details', + 'published_at': '2026-07-16T16:00:00Z', + }, + ], + }), + 200, + ); + }), + ); + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SingleChildScrollView( + child: FeedbackSettingsCard(client: client), + ), + ), + ), + ); + await tester.pumpAndSettle(); + await tester.tap(find.byTooltip('Report comment')); + await tester.pumpAndSettle(); + await tester.enterText(find.byType(TextField).last, 'personal data'); + await tester.tap(find.widgetWithText(FilledButton, 'Report')); + await tester.pumpAndSettle(); + + final report = requests.singleWhere((request) => request.method == 'POST'); + expect(report.url.path, contains('/feedback-other/reports')); + expect(report.body, contains('personal data')); + expect(find.text('Comment reported for review.'), findsOneWidget); + }); +} + +class _MemoryTokenStore implements FeedbackTokenStore { + _MemoryTokenStore([this.token]); + + String? token; + + @override + Future clear() async => token = null; + + @override + Future read() async => token; + + @override + Future write(String value) async => token = value; +} + +const String _token = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'; diff --git a/flutter_app/test/widget_test.dart b/flutter_app/test/widget_test.dart index 5c048c9..cfafffc 100644 --- a/flutter_app/test/widget_test.dart +++ b/flutter_app/test/widget_test.dart @@ -194,7 +194,7 @@ void main() { expect(savedConfig?.cloudModel, 'studyos'); expect(savedKey, 'secret'); expect(find.text('Stored securely on this device.'), findsOneWidget); - expect(find.text('Send feedback'), findsOneWidget); + expect(find.text('Feedback is unavailable'), findsOneWidget); }); testWidgets('settings profile editor updates onboarding preferences', ( From bba4d0b3527ce6412f646e6a4c336255d3ec8f1f Mon Sep 17 00:00:00 2001 From: StudyOS Org Date: Fri, 17 Jul 2026 09:02:14 +0000 Subject: [PATCH 2/3] feat: add course lookup and ratings --- README.md | 9 +- feedback_service/README.md | 32 ++- feedback_service/catalog.py | 132 +++++++++++ feedback_service/compose.example.yml | 3 +- feedback_service/config.py | 33 +-- feedback_service/db.py | 98 ++++++--- feedback_service/main.py | 121 +++++++--- feedback_service/models.py | 15 ++ feedback_service/tests/conftest.py | 21 +- feedback_service/tests/test_api.py | 65 +++++- feedback_service/tests/test_catalog.py | 55 +++++ feedback_service/tests/test_migration.py | 26 +++ flutter_app/README.md | 13 +- flutter_app/lib/src/app_router.dart | 27 +++ .../lib/src/course_catalog_client.dart | 110 ++++++++++ .../lib/src/course_catalog_models.dart | 45 ++++ flutter_app/lib/src/feedback_client.dart | 31 ++- flutter_app/lib/src/feedback_models.dart | 20 +- .../lib/src/views/course_ratings_view.dart | 207 ++++++++++++++++++ flutter_app/lib/src/views/settings_view.dart | 6 - .../src/widgets/feedback_settings_card.dart | 40 +++- .../secondary_destinations_drawer.dart | 5 + .../test/course_catalog_client_test.dart | 55 +++++ flutter_app/test/feedback_client_test.dart | 38 ++-- .../test/feedback_settings_card_test.dart | 43 +++- 25 files changed, 1084 insertions(+), 166 deletions(-) create mode 100644 feedback_service/catalog.py create mode 100644 feedback_service/tests/test_catalog.py create mode 100644 feedback_service/tests/test_migration.py create mode 100644 flutter_app/lib/src/course_catalog_client.dart create mode 100644 flutter_app/lib/src/course_catalog_models.dart create mode 100644 flutter_app/lib/src/views/course_ratings_view.dart create mode 100644 flutter_app/test/course_catalog_client_test.dart diff --git a/README.md b/README.md index 6293256..2c634c8 100644 --- a/README.md +++ b/README.md @@ -112,11 +112,12 @@ flutter run -d chrome \ --dart-define=STUDYOS_DEMO_OPENROUTER_API_KEY="$OPENROUTER_API_KEY" ``` -## Feedback Deployment +## Course Ratings Deployment -The Settings feedback card connects to the containerized SQLite service under -`feedback_service/`. Deploy that service behind HTTPS, then provide its public -base URL to Flutter builds: +The **Course ratings** view connects to the containerized SQLite service under +`feedback_service/`. That service proxies the shared StudyPlanner catalog and +stores pseudonymous star ratings plus moderated comments. Deploy it behind +HTTPS, then provide its public base URL to Flutter builds: ```sh flutter build web --release \ diff --git a/feedback_service/README.md b/feedback_service/README.md index 21c81c2..dd9471b 100644 --- a/feedback_service/README.md +++ b/feedback_service/README.md @@ -1,8 +1,10 @@ -# StudyOS feedback service +# StudyOS course ratings service -A small FastAPI service backed by SQLite. Ratings are public immediately while -optional comments enter a moderation queue. Anonymous installation credentials -are random 256-bit bearer tokens; only their SHA-256 digests are stored. +A small FastAPI service backed by SQLite. It proxies public, read-only course +lookup to StudyPlanner and stores ratings by stable course number. Ratings are +public immediately while optional comments enter a moderation queue. Anonymous +installation credentials are random 256-bit bearer tokens; only their SHA-256 +digests are stored. Moderation is manual by default, so there is no third-party API, data transfer, or usage-based moderation bill. @@ -27,7 +29,8 @@ reason. - `FEEDBACK_DATABASE_PATH` (default `/data/feedback.sqlite3`) - `FEEDBACK_ADMIN_TOKEN` (admin endpoints remain inaccessible when empty) - `FEEDBACK_MODERATOR_ID` (audit identity bound to the admin token) -- `FEEDBACK_SERVICE_IDS` (comma-separated, default `studyos-agent`) +- `FEEDBACK_CATALOG_API_URL` (default: the public StudyPlanner API) +- `FEEDBACK_CATALOG_RATE_LIMIT` (per peer/minute; default `30`) - `FEEDBACK_CORS_ORIGINS` (comma-separated exact origins; empty disables CORS) - `FEEDBACK_INSTALLATION_RATE_LIMIT` and `FEEDBACK_AUTHOR_RATE_LIMIT` @@ -61,6 +64,25 @@ provider privacy, retention, failure behavior, and billing must be reviewed before such an integration is enabled, and failures must fall back to pending human review. +## Course identity and privacy boundary + +`POST /v1/courses/search` forwards only the public search text to the shared +StudyPlanner catalog and caches up to 64 bounded results in memory for five +minutes. POST keeps search terms out of URL history and ordinary access logs; +operators must still avoid request-body logging. ALMA +credentials, cookies, enrollment lists, profiles, and timetables never reach +this service. Course feedback is grouped longitudinally by course number, so +different semesters contribute to one course's aggregate. + +The “My courses” shortcuts are computed in the app and are not uploaded as a +list; selecting one sends its title as a catalog search. Installation tokens do +not prove a unique person, university membership, +or enrollment. These are community ratings, not verified-student reviews. + +Schema v1 belonged to the abandoned app/service-rating prototype. The service +refuses to relabel that data as course feedback; back it up and recreate the +volume before starting this schema-v2 build. + ## Launch and retention checklist Before public deployment, the team must name a moderation owner, response diff --git a/feedback_service/catalog.py b/feedback_service/catalog.py new file mode 100644 index 0000000..db80bcc --- /dev/null +++ b/feedback_service/catalog.py @@ -0,0 +1,132 @@ +from __future__ import annotations + +import json +import math +import threading +import time +import urllib.error +import urllib.request +from collections import OrderedDict +from dataclasses import dataclass + + +class CatalogUnavailable(Exception): + pass + + +@dataclass(frozen=True) +class _CacheEntry: + expires_at: float + value: dict + + +class StudyPlannerCatalog: + def __init__( + self, + base_url: str, + timeout_seconds: float = 8, + cache_seconds: float = 300, + max_cache_entries: int = 64, + ): + self.search_url = f"{base_url.rstrip('/')}/api/ai/catalog/search" + self.timeout_seconds = timeout_seconds + self.cache_seconds = cache_seconds + self.max_cache_entries = max_cache_entries + self._cache: OrderedDict[tuple[str, int], _CacheEntry] = OrderedDict() + self._lock = threading.Lock() + + @property + def cache_entries(self) -> int: + with self._lock: + return len(self._cache) + + def search(self, query: str, limit: int) -> dict: + key = (query.casefold(), limit) + current = time.monotonic() + with self._lock: + cached = self._cache.get(key) + if cached and cached.expires_at > current: + self._cache.move_to_end(key) + return cached.value + + request = urllib.request.Request( + self.search_url, + data=json.dumps( + {"query": query, "limit": limit, "periodId": "all"} + ).encode(), + headers={ + "Accept": "application/json", + "Content-Type": "application/json", + "User-Agent": "StudyOS-Course-Ratings/1.0", + }, + method="POST", + ) + try: + with urllib.request.urlopen( + request, timeout=self.timeout_seconds + ) as response: + raw = response.read(256_001) + except (OSError, urllib.error.HTTPError, urllib.error.URLError) as error: + raise CatalogUnavailable( + "course catalog is temporarily unavailable" + ) from error + if len(raw) > 256_000: + raise CatalogUnavailable("course catalog response is too large") + try: + payload = json.loads(raw) + courses = payload["courses"] + if not isinstance(courses, list): + raise TypeError + except (json.JSONDecodeError, KeyError, TypeError) as error: + raise CatalogUnavailable( + "course catalog returned an invalid response" + ) from error + + cleaned = [self._course(item) for item in courses[:limit]] + value = { + "courses": [course for course in cleaned if course is not None], + "count": payload.get("count", len(courses)), + "truncated": bool(payload.get("truncated", False)), + } + with self._lock: + self._cache[key] = _CacheEntry(current + self.cache_seconds, value) + self._cache.move_to_end(key) + while len(self._cache) > self.max_cache_entries: + self._cache.popitem(last=False) + return value + + @staticmethod + def _course(item: object) -> dict | None: + if not isinstance(item, dict): + return None + required = ("courseId", "courseNumber", "title") + if any(not isinstance(item.get(key), str) for key in required): + return None + course_id = item["courseId"].strip()[:80] + number = item["courseNumber"].strip()[:120] + title = item["title"].strip()[:300] + if not course_id or not number or not title: + return None + period = item.get("periodLabel") + lecturer = item.get("lecturer") + ects = item.get("ects") + types = item.get("types") + return { + "courseId": course_id, + "courseNumber": number, + "title": title, + "periodLabel": period.strip()[:80] if isinstance(period, str) else "", + "ects": ects + if not isinstance(ects, bool) + and isinstance(ects, (int, float)) + and math.isfinite(ects) + else None, + "lecturer": lecturer.strip()[:200] + if isinstance(lecturer, str) and lecturer.strip() + else None, + "types": [ + value.strip()[:80] for value in types[:10] if isinstance(value, str) + ] + if isinstance(types, list) + else [], + } diff --git a/feedback_service/compose.example.yml b/feedback_service/compose.example.yml index e9f7bba..df7f510 100644 --- a/feedback_service/compose.example.yml +++ b/feedback_service/compose.example.yml @@ -14,7 +14,8 @@ services: FEEDBACK_ADMIN_TOKEN: ${FEEDBACK_ADMIN_TOKEN:?set a long random admin token} FEEDBACK_MODERATOR_ID: ${FEEDBACK_MODERATOR_ID:-studyos-admin} FEEDBACK_CORS_ORIGINS: ${FEEDBACK_CORS_ORIGINS:-} - FEEDBACK_SERVICE_IDS: studyos-agent + FEEDBACK_CATALOG_API_URL: ${FEEDBACK_CATALOG_API_URL:-https://studyplanner-api.ben-tischberger.workers.dev} + FEEDBACK_CATALOG_RATE_LIMIT: ${FEEDBACK_CATALOG_RATE_LIMIT:-30} volumes: - feedback-data:/data - feedback-backups:/backups diff --git a/feedback_service/config.py b/feedback_service/config.py index c2507c8..637a7c0 100644 --- a/feedback_service/config.py +++ b/feedback_service/config.py @@ -6,10 +6,6 @@ from pathlib import Path -def _csv(value: str) -> tuple[str, ...]: - return tuple(item.strip() for item in value.split(",") if item.strip()) - - MODERATOR_ID = re.compile(r"^[a-zA-Z0-9._@-]{2,80}$") @@ -18,10 +14,11 @@ class Settings: database_path: Path = Path("/data/feedback.sqlite3") admin_token: str = "" moderator_id: str = "studyos-admin" - allowed_service_ids: tuple[str, ...] = ("studyos-agent",) + catalog_api_url: str = "https://studyplanner-api.ben-tischberger.workers.dev" cors_origins: tuple[str, ...] = () installation_limit_per_minute: int = 10 feedback_limit_per_minute: int = 30 + catalog_search_limit_per_minute: int = 30 @classmethod def from_env(cls) -> "Settings": @@ -31,28 +28,38 @@ def from_env(cls) -> "Settings": ), admin_token=os.getenv("FEEDBACK_ADMIN_TOKEN", ""), moderator_id=os.getenv("FEEDBACK_MODERATOR_ID", "studyos-admin"), - allowed_service_ids=_csv( - os.getenv("FEEDBACK_SERVICE_IDS", "studyos-agent") + catalog_api_url=os.getenv( + "FEEDBACK_CATALOG_API_URL", + "https://studyplanner-api.ben-tischberger.workers.dev", + ), + cors_origins=tuple( + item.strip() + for item in os.getenv("FEEDBACK_CORS_ORIGINS", "").split(",") + if item.strip() ), - cors_origins=_csv(os.getenv("FEEDBACK_CORS_ORIGINS", "")), installation_limit_per_minute=int( os.getenv("FEEDBACK_INSTALLATION_RATE_LIMIT", "10") ), feedback_limit_per_minute=int( os.getenv("FEEDBACK_AUTHOR_RATE_LIMIT", "30") ), + catalog_search_limit_per_minute=int( + os.getenv("FEEDBACK_CATALOG_RATE_LIMIT", "30") + ), ) def validate(self) -> None: - if not self.allowed_service_ids: - raise RuntimeError( - "FEEDBACK_SERVICE_IDS must contain at least one service ID" - ) if self.admin_token and len(self.admin_token) < 32: raise RuntimeError( "FEEDBACK_ADMIN_TOKEN must contain at least 32 characters" ) if not MODERATOR_ID.fullmatch(self.moderator_id): raise RuntimeError("FEEDBACK_MODERATOR_ID is invalid") - if self.installation_limit_per_minute < 1 or self.feedback_limit_per_minute < 1: + if not self.catalog_api_url.startswith("https://"): + raise RuntimeError("FEEDBACK_CATALOG_API_URL must use HTTPS") + if ( + self.installation_limit_per_minute < 1 + or self.feedback_limit_per_minute < 1 + or self.catalog_search_limit_per_minute < 1 + ): raise RuntimeError("rate limits must be positive") diff --git a/feedback_service/db.py b/feedback_service/db.py index 460f8ef..75c32a8 100644 --- a/feedback_service/db.py +++ b/feedback_service/db.py @@ -34,10 +34,16 @@ def migrate(self) -> None: token_hash TEXT NOT NULL UNIQUE, created_at TEXT NOT NULL ); + CREATE TABLE courses ( + id TEXT PRIMARY KEY, + course_number TEXT NOT NULL, + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL + ); CREATE TABLE feedback ( id TEXT PRIMARY KEY, installation_id TEXT NOT NULL REFERENCES installations(id), - service_id TEXT NOT NULL, + course_id TEXT NOT NULL REFERENCES courses(id), rating INTEGER NOT NULL CHECK (rating BETWEEN 1 AND 5), comment TEXT CHECK (comment IS NULL OR length(comment) <= 1000), comment_state TEXT NOT NULL CHECK ( @@ -47,9 +53,9 @@ def migrate(self) -> None: updated_at TEXT NOT NULL, published_at TEXT, deleted_at TEXT, - UNIQUE (installation_id, service_id) + UNIQUE (installation_id, course_id) ); - CREATE INDEX feedback_public_idx ON feedback(service_id, deleted_at, comment_state); + CREATE INDEX feedback_public_idx ON feedback(course_id, deleted_at, comment_state); CREATE TABLE reports ( id TEXT PRIMARY KEY, feedback_id TEXT NOT NULL REFERENCES feedback(id), @@ -67,10 +73,15 @@ def migrate(self) -> None: moderator_id TEXT NOT NULL, created_at TEXT NOT NULL ); - PRAGMA user_version = 1; + PRAGMA user_version = 2; """ ) - elif version != 1: + elif version == 1: + raise RuntimeError( + "prerelease service-feedback schema cannot be migrated as course ratings; " + "back up and recreate the database" + ) + elif version != 2: raise RuntimeError(f"unsupported database schema version: {version}") def create_installation(self, digest: str) -> None: @@ -87,17 +98,39 @@ def installation_id(self, digest: str) -> str | None: ).fetchone() return row["id"] if row else None + def register_courses(self, courses: list[tuple[str, str]]) -> None: + timestamp = now() + with self.connect() as db: + db.executemany( + """INSERT INTO courses (id, course_number, created_at, updated_at) + VALUES (?, ?, ?, ?) + ON CONFLICT(id) DO UPDATE SET + course_number=excluded.course_number, + updated_at=excluded.updated_at""", + [ + (course_id, number, timestamp, timestamp) + for course_id, number in courses + ], + ) + + def course_number(self, course_id: str) -> str | None: + with self.connect() as db: + row = db.execute( + "SELECT course_number FROM courses WHERE id=?", (course_id,) + ).fetchone() + return row["course_number"] if row else None + def upsert_feedback( - self, installation_id: str, service_id: str, rating: int, comment: str | None + self, installation_id: str, course_id: str, rating: int, comment: str | None ): timestamp = now() state = "pending" if comment else "none" with self.connect() as db: row = db.execute( """INSERT INTO feedback - (id, installation_id, service_id, rating, comment, comment_state, created_at, updated_at) + (id, installation_id, course_id, rating, comment, comment_state, created_at, updated_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?) - ON CONFLICT(installation_id, service_id) DO UPDATE SET + ON CONFLICT(installation_id, course_id) DO UPDATE SET rating=excluded.rating, comment=excluded.comment, comment_state=excluded.comment_state, @@ -108,7 +141,7 @@ def upsert_feedback( ( str(uuid.uuid4()), installation_id, - service_id, + course_id, rating, comment, state, @@ -120,40 +153,43 @@ def upsert_feedback( # Reports describe the previous comment revision. Resubmission # returns the comment to review and must not inherit them. db.execute("DELETE FROM reports WHERE feedback_id=?", (feedback_id,)) - return self.feedback_for_owner(installation_id, service_id) + return self.feedback_for_owner(installation_id, course_id) - def feedback_for_owner(self, installation_id: str, service_id: str): + def feedback_for_owner(self, installation_id: str, course_id: str): with self.connect() as db: return db.execute( - """SELECT id, service_id, rating, comment, comment_state, created_at, updated_at - FROM feedback WHERE installation_id=? AND service_id=? AND deleted_at IS NULL""", - (installation_id, service_id), + """SELECT id, course_id, rating, comment, comment_state, created_at, updated_at + FROM feedback WHERE installation_id=? AND course_id=? AND deleted_at IS NULL""", + (installation_id, course_id), ).fetchone() - def delete_feedback(self, installation_id: str, service_id: str) -> bool: + def delete_feedback(self, installation_id: str, course_id: str) -> bool: timestamp = now() with self.connect() as db: result = db.execute( """UPDATE feedback SET comment=NULL, comment_state='deleted', deleted_at=?, updated_at=? - WHERE installation_id=? AND service_id=? AND deleted_at IS NULL""", - (timestamp, timestamp, installation_id, service_id), + WHERE installation_id=? AND course_id=? AND deleted_at IS NULL""", + (timestamp, timestamp, installation_id, course_id), ) return result.rowcount == 1 - def public_feedback(self, service_id: str, limit: int, offset: int) -> dict: + def public_feedback( + self, course_id: str, course_number: str, limit: int, offset: int + ) -> dict: with self.connect() as db: aggregate = db.execute( - "SELECT COUNT(*) count, AVG(rating) average FROM feedback WHERE service_id=? AND deleted_at IS NULL", - (service_id,), + "SELECT COUNT(*) count, AVG(rating) average FROM feedback WHERE course_id=? AND deleted_at IS NULL", + (course_id,), ).fetchone() comments = db.execute( """SELECT id, rating, comment, published_at FROM feedback - WHERE service_id=? AND deleted_at IS NULL AND comment_state='published' + WHERE course_id=? AND deleted_at IS NULL AND comment_state='published' ORDER BY published_at DESC, id LIMIT ? OFFSET ?""", - (service_id, limit, offset), + (course_id, limit, offset), ).fetchall() return { - "service_id": service_id, + "course_id": course_id, + "course_number": course_number, "rating": {"count": aggregate["count"], "average": aggregate["average"]}, "comments": [dict(row) for row in comments], "pagination": {"limit": limit, "offset": offset}, @@ -162,15 +198,15 @@ def public_feedback(self, service_id: str, limit: int, offset: int) -> dict: def report( self, installation_id: str, - service_id: str, + course_id: str, feedback_id: str, reason: str | None, ) -> str: with self.connect() as db: target = db.execute( - """SELECT installation_id FROM feedback WHERE id=? AND service_id=? + """SELECT installation_id FROM feedback WHERE id=? AND course_id=? AND comment_state='published' AND deleted_at IS NULL""", - (feedback_id, service_id), + (feedback_id, course_id), ).fetchone() if not target: return "not_found" @@ -188,7 +224,7 @@ def report( def moderation_queue( self, state: str, - service_id: str | None, + course_id: str | None, limit: int, offset: int, ): @@ -199,12 +235,12 @@ def moderation_queue( else: where += "f.comment_state=?" params.append(state) - if service_id: - where += " AND f.service_id=?" - params.append(service_id) + if course_id: + where += " AND f.course_id=?" + params.append(course_id) with self.connect() as db: return db.execute( - f"""SELECT f.id, f.service_id, f.rating, f.comment, f.comment_state, + f"""SELECT f.id, f.course_id, f.rating, f.comment, f.comment_state, f.created_at, f.updated_at, COUNT(r.id) report_count FROM feedback f LEFT JOIN reports r ON r.feedback_id=f.id WHERE {where} GROUP BY f.id ORDER BY f.updated_at diff --git a/feedback_service/main.py b/feedback_service/main.py index 3558f09..9d8e4e3 100644 --- a/feedback_service/main.py +++ b/feedback_service/main.py @@ -1,6 +1,9 @@ from __future__ import annotations import hmac +import base64 +import binascii +import unicodedata from contextlib import asynccontextmanager from typing import Annotated @@ -18,26 +21,44 @@ from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer from .config import MODERATOR_ID, Settings +from .catalog import CatalogUnavailable, StudyPlannerCatalog from .db import Database -from .models import FeedbackInput, ModerationInput, ReportInput +from .models import CourseSearchInput, FeedbackInput, ModerationInput, ReportInput from .security import SlidingWindowLimiter, issue_token, token_hash bearer = HTTPBearer(auto_error=False) -def create_app(settings: Settings | None = None) -> FastAPI: +def course_reference(course_number: str) -> str | None: + normalized = " ".join(unicodedata.normalize("NFKC", course_number).split()).upper() + if not normalized: + return None + reference = ( + base64.urlsafe_b64encode(normalized.encode()).decode("ascii").rstrip("=") + ) + return reference if 2 <= len(reference) <= 160 else None + + +def create_app( + settings: Settings | None = None, catalog: StudyPlannerCatalog | None = None +) -> FastAPI: settings = settings or Settings.from_env() settings.validate() database = Database(settings.database_path) + catalog = catalog or StudyPlannerCatalog(settings.catalog_api_url) installation_limiter = SlidingWindowLimiter(settings.installation_limit_per_minute) author_limiter = SlidingWindowLimiter(settings.feedback_limit_per_minute) + catalog_limiter = SlidingWindowLimiter(settings.catalog_search_limit_per_minute) + catalog_global_limiter = SlidingWindowLimiter( + settings.catalog_search_limit_per_minute * 10 + ) @asynccontextmanager async def lifespan(_: FastAPI): database.migrate() yield - app = FastAPI(title="StudyOS Feedback Service", version="1.0.0", lifespan=lifespan) + app = FastAPI(title="StudyOS Course Ratings", version="1.0.0", lifespan=lifespan) app.state.database = database if settings.cors_origins: app.add_middleware( @@ -53,10 +74,27 @@ async def lifespan(_: FastAPI): ], ) - def service(service_id: str) -> str: - if service_id not in settings.allowed_service_ids: - raise HTTPException(status.HTTP_404_NOT_FOUND, "service not found") - return service_id + def course(course_id: str) -> tuple[str, str]: + """Validate a URL-safe base64 course number without trusting titles.""" + if not 2 <= len(course_id) <= 160: + raise HTTPException(status.HTTP_404_NOT_FOUND, "course not found") + try: + padding = "=" * (-len(course_id) % 4) + raw = base64.b64decode(course_id + padding, altchars=b"-_", validate=True) + course_number = raw.decode("utf-8") + except (binascii.Error, UnicodeDecodeError): + raise HTTPException(status.HTTP_404_NOT_FOUND, "course not found") from None + if not 1 <= len(course_number) <= 120 or any( + ord(char) < 32 or ord(char) == 127 for char in course_number + ): + raise HTTPException(status.HTTP_404_NOT_FOUND, "course not found") + canonical = base64.urlsafe_b64encode(raw).decode("ascii").rstrip("=") + if not hmac.compare_digest(canonical, course_id): + raise HTTPException(status.HTTP_404_NOT_FOUND, "course not found") + registered_number = database.course_number(course_id) + if registered_number is None or course_reference(course_number) != course_id: + raise HTTPException(status.HTTP_404_NOT_FOUND, "course not found") + return course_id, registered_number def installation( credentials: Annotated[HTTPAuthorizationCredentials | None, Depends(bearer)], @@ -117,58 +155,89 @@ def create_installation(request: Request) -> dict[str, str]: database.create_installation(token_hash(token)) return {"installation_token": token, "token_type": "bearer"} - @app.get("/v1/services/{service_id}/feedback/public") + @app.post("/v1/courses/search") + def search_courses( + payload: CourseSearchInput, + request: Request, + ) -> dict: + peer = request.client.host if request.client else "unknown" + if not catalog_limiter.allow(peer) or not catalog_global_limiter.allow("all"): + raise HTTPException( + status.HTTP_429_TOO_MANY_REQUESTS, "rate limit exceeded" + ) + try: + result = catalog.search(payload.query, payload.limit) + except CatalogUnavailable as error: + raise HTTPException( + status.HTTP_503_SERVICE_UNAVAILABLE, str(error) + ) from error + registrations = [] + rateable_courses = [] + for item in result["courses"]: + rating_id = course_reference(item["courseNumber"]) + if rating_id is None: + continue + item["ratingCourseId"] = rating_id + registrations.append((rating_id, item["courseNumber"])) + rateable_courses.append(item) + result["courses"] = rateable_courses + database.register_courses(registrations) + return result + + @app.get("/v1/courses/{course_id}/feedback/public") def public_feedback( - service_id: str, + course_id: str, limit: Annotated[int, Query(ge=1, le=100)] = 20, offset: Annotated[int, Query(ge=0, le=10_000)] = 0, ) -> dict: - return database.public_feedback(service(service_id), limit, offset) + course_ref, course_number = course(course_id) + return database.public_feedback(course_ref, course_number, limit, offset) - @app.get("/v1/services/{service_id}/feedback/mine") + @app.get("/v1/courses/{course_id}/feedback/mine") def own_feedback( - service_id: str, installation_id: str = Depends(installation) + course_id: str, installation_id: str = Depends(installation) ) -> dict: - row = database.feedback_for_owner(installation_id, service(service_id)) + course_ref, _ = course(course_id) + row = database.feedback_for_owner(installation_id, course_ref) if not row: raise HTTPException(status.HTTP_404_NOT_FOUND, "feedback not found") return dict(row) - @app.put("/v1/services/{service_id}/feedback/mine") + @app.put("/v1/courses/{course_id}/feedback/mine") def put_feedback( - service_id: str, + course_id: str, payload: FeedbackInput, installation_id: str = Depends(limited_installation), ) -> dict: return dict( database.upsert_feedback( - installation_id, service(service_id), payload.rating, payload.comment + installation_id, course(course_id)[0], payload.rating, payload.comment ) ) @app.delete( - "/v1/services/{service_id}/feedback/mine", + "/v1/courses/{course_id}/feedback/mine", status_code=status.HTTP_204_NO_CONTENT, ) def delete_feedback( - service_id: str, installation_id: str = Depends(limited_installation) + course_id: str, installation_id: str = Depends(limited_installation) ) -> Response: - if not database.delete_feedback(installation_id, service(service_id)): + if not database.delete_feedback(installation_id, course(course_id)[0]): raise HTTPException(status.HTTP_404_NOT_FOUND, "feedback not found") return Response(status_code=status.HTTP_204_NO_CONTENT) @app.post( - "/v1/services/{service_id}/feedback/{feedback_id}/reports", + "/v1/courses/{course_id}/feedback/{feedback_id}/reports", status_code=status.HTTP_201_CREATED, ) def report_feedback( - service_id: str, + course_id: str, feedback_id: str, payload: ReportInput, installation_id: str = Depends(limited_installation), ) -> dict[str, str]: result = database.report( - installation_id, service(service_id), feedback_id, payload.reason + installation_id, course(course_id)[0], feedback_id, payload.reason ) if result == "not_found": raise HTTPException( @@ -185,18 +254,18 @@ def report_feedback( @app.get("/v1/admin/moderation") def moderation_queue( state: Annotated[str, Query(pattern="^(pending|reported)$")] = "pending", - service_id: str | None = None, + course_id: str | None = None, limit: Annotated[int, Query(ge=1, le=100)] = 50, offset: Annotated[int, Query(ge=0, le=10_000)] = 0, _moderator_id: str = Depends(admin), ) -> dict: - if service_id is not None: - service(service_id) + if course_id is not None: + course_id = course(course_id)[0] items = [ dict(row) for row in database.moderation_queue( state, - service_id, + course_id, limit, offset, ) diff --git a/feedback_service/models.py b/feedback_service/models.py index 29693ca..98b1ad7 100644 --- a/feedback_service/models.py +++ b/feedback_service/models.py @@ -28,6 +28,21 @@ class FeedbackInput(BaseModel): _clean_comment = field_validator("comment")(clean_text) +class CourseSearchInput(BaseModel): + model_config = ConfigDict(extra="forbid") + + query: str = Field(min_length=2, max_length=120) + limit: int = Field(default=20, ge=1, le=25, strict=True) + + @field_validator("query") + @classmethod + def clean_query(cls, value: str) -> str: + cleaned = clean_text(value) + if cleaned is None or len(cleaned) < 2: + raise ValueError("query must contain at least two characters") + return cleaned + + class ReportInput(BaseModel): model_config = ConfigDict(extra="forbid") diff --git a/feedback_service/tests/conftest.py b/feedback_service/tests/conftest.py index 6d2c62e..4b09f63 100644 --- a/feedback_service/tests/conftest.py +++ b/feedback_service/tests/conftest.py @@ -7,6 +7,22 @@ from feedback_service.main import create_app +class FakeCatalog: + def search(self, query: str, limit: int) -> dict: + return { + "courses": [ + { + "courseId": "598", + "courseNumber": "INFM1234", + "title": f"{query} course", + "periodLabel": "Sommer 2026", + } + ][:limit], + "count": 1, + "truncated": False, + } + + @pytest.fixture def client(tmp_path: Path): app = create_app( @@ -14,12 +30,13 @@ def client(tmp_path: Path): database_path=tmp_path / "feedback.sqlite3", admin_token="test-admin-token-that-is-at-least-32-characters", moderator_id="moderator@example.test", - allowed_service_ids=("studyos-agent",), installation_limit_per_minute=20, feedback_limit_per_minute=20, - ) + ), + catalog=FakeCatalog(), # type: ignore[arg-type] ) with TestClient(app) as test_client: + app.state.database.register_courses([("SU5GTTEyMzQ", "INFM1234")]) yield test_client diff --git a/feedback_service/tests/test_api.py b/feedback_service/tests/test_api.py index 196b066..f99686a 100644 --- a/feedback_service/tests/test_api.py +++ b/feedback_service/tests/test_api.py @@ -1,14 +1,18 @@ import sqlite3 +import base64 from concurrent.futures import ThreadPoolExecutor from fastapi.testclient import TestClient +from feedback_service.main import course_reference from feedback_service.security import token_hash from feedback_service.tests.conftest import admin, auth, issue -MINE = "/v1/services/studyos-agent/feedback/mine" -PUBLIC = "/v1/services/studyos-agent/feedback/public" +COURSE_NUMBER = "INFM1234" +COURSE_ID = base64.urlsafe_b64encode(COURSE_NUMBER.encode()).decode().rstrip("=") +MINE = f"/v1/courses/{COURSE_ID}/feedback/mine" +PUBLIC = f"/v1/courses/{COURSE_ID}/feedback/public" def put(client: TestClient, token: str, rating: int = 5, comment: str | None = None): @@ -74,7 +78,7 @@ def test_report_queue_duplicate_and_comment_deletion_keep_rating(client: TestCli headers=admin(), json={"action": "publish"}, ) - report_url = f"/v1/services/studyos-agent/feedback/{feedback['id']}/reports" + report_url = f"/v1/courses/{COURSE_ID}/feedback/{feedback['id']}/reports" assert client.post(report_url, headers=auth(owner), json={}).status_code == 400 assert ( client.post( @@ -137,6 +141,40 @@ def test_owner_delete_removes_rating_and_can_upsert_again(client: TestClient): assert client.get(PUBLIC).json()["rating"] == {"count": 1, "average": 3.0} +def test_feedback_is_isolated_between_registered_courses(client: TestClient): + other_number = "INFM5678" + other_id = base64.urlsafe_b64encode(other_number.encode()).decode().rstrip("=") + client.app.state.database.register_courses([(other_id, other_number)]) + token = issue(client) + other_mine = f"/v1/courses/{other_id}/feedback/mine" + other_public = f"/v1/courses/{other_id}/feedback/public" + + assert put(client, token, 5).status_code == 200 + assert ( + client.put( + other_mine, headers=auth(token), json={"rating": 2, "comment": None} + ).status_code + == 200 + ) + assert client.get(PUBLIC).json()["rating"] == {"count": 1, "average": 5.0} + assert client.get(other_public).json()["rating"] == { + "count": 1, + "average": 2.0, + } + assert client.delete(other_mine, headers=auth(token)).status_code == 204 + assert client.get(PUBLIC).json()["rating"]["count"] == 1 + assert client.get(other_public).json()["rating"]["count"] == 0 + + +def test_unregistered_course_bucket_is_rejected(client: TestClient): + unknown = base64.urlsafe_b64encode(b"INFM9999").decode().rstrip("=") + assert client.get(f"/v1/courses/{unknown}/feedback/public").status_code == 404 + + +def test_course_reference_rejects_normalized_unicode_expansion(): + assert course_reference("💥" * 120) is None + + def test_concurrent_upserts_keep_one_owned_feedback(client: TestClient): token = issue(client) database = client.app.state.database @@ -147,7 +185,7 @@ def test_concurrent_upserts_keep_one_owned_feedback(client: TestClient): executor.map( lambda rating: database.upsert_feedback( installation_id, - "studyos-agent", + COURSE_ID, rating, None, ), @@ -234,9 +272,17 @@ def test_moderation_transitions_are_serialized_and_validated(client: TestClient) assert response.status_code == 409 -def test_unknown_service_and_health(client: TestClient): +def test_invalid_course_and_health(client: TestClient): assert client.get("/healthz").json() == {"status": "ok"} - assert client.get("/v1/services/unknown/feedback/public").status_code == 404 + assert client.get("/v1/courses/unknown!/feedback/public").status_code == 404 + catalog = client.post("/v1/courses/search", json={"query": "machine", "limit": 1}) + assert catalog.status_code == 200 + assert catalog.json()["courses"][0]["courseNumber"] == COURSE_NUMBER + assert catalog.json()["courses"][0]["ratingCourseId"] == COURSE_ID + assert client.post("/v1/courses/search", json={"query": "x"}).status_code == 422 + public = client.get(PUBLIC).json() + assert public["course_id"] == COURSE_ID + assert public["course_number"] == COURSE_NUMBER assert client.get(f"{PUBLIC}?limit=101").status_code == 422 assert client.get(f"{PUBLIC}?offset=10001").status_code == 422 assert ( @@ -248,12 +294,13 @@ def test_database_enforces_foreign_keys_and_wal(client: TestClient): with client.app.state.database.connect() as db: assert db.execute("PRAGMA foreign_keys").fetchone()[0] == 1 assert db.execute("PRAGMA journal_mode").fetchone()[0] == "wal" - assert db.execute("PRAGMA user_version").fetchone()[0] == 1 + assert db.execute("PRAGMA user_version").fetchone()[0] == 2 try: db.execute( """INSERT INTO feedback - (id, installation_id, service_id, rating, comment_state, created_at, updated_at) - VALUES ('bad', 'missing', 'studyos-agent', 5, 'none', 'now', 'now')""" + (id, installation_id, course_id, rating, comment_state, created_at, updated_at) + VALUES ('bad', 'missing', ?, 5, 'none', 'now', 'now')""", + (COURSE_ID,), ) except sqlite3.IntegrityError: pass diff --git a/feedback_service/tests/test_catalog.py b/feedback_service/tests/test_catalog.py new file mode 100644 index 0000000..5e67f24 --- /dev/null +++ b/feedback_service/tests/test_catalog.py @@ -0,0 +1,55 @@ +import json + +from feedback_service.catalog import StudyPlannerCatalog + + +class FakeResponse: + def __init__(self, payload: dict): + self.payload = json.dumps(payload).encode() + + def __enter__(self): + return self + + def __exit__(self, *_args): + return None + + def read(self, _limit: int) -> bytes: + return self.payload + + +def test_catalog_cache_is_bounded_and_response_is_typed(monkeypatch): + payload = { + "courses": [ + { + "courseId": "598", + "courseNumber": "INFM1234", + "title": "Machine Learning", + "periodLabel": "Sommer 2026", + "ects": float("nan"), + "schedule": ["not forwarded"], + }, + {"courseId": 123, "courseNumber": "bad", "title": "bad"}, + ] + } + monkeypatch.setattr( + "feedback_service.catalog.urllib.request.urlopen", + lambda *_args, **_kwargs: FakeResponse(payload), + ) + catalog = StudyPlannerCatalog("https://catalog.test", max_cache_entries=2) + + first = catalog.search("first", 20) + catalog.search("second", 20) + catalog.search("third", 20) + + assert catalog.cache_entries == 2 + assert first["courses"] == [ + { + "courseId": "598", + "courseNumber": "INFM1234", + "title": "Machine Learning", + "periodLabel": "Sommer 2026", + "ects": None, + "lecturer": None, + "types": [], + } + ] diff --git a/feedback_service/tests/test_migration.py b/feedback_service/tests/test_migration.py new file mode 100644 index 0000000..87c8ee6 --- /dev/null +++ b/feedback_service/tests/test_migration.py @@ -0,0 +1,26 @@ +import sqlite3 +from pathlib import Path + +from feedback_service.db import Database + + +def test_v1_service_feedback_requires_explicit_recreation(tmp_path: Path): + path = tmp_path / "feedback.sqlite3" + with sqlite3.connect(path) as db: + db.executescript( + """ + CREATE TABLE feedback ( + id TEXT PRIMARY KEY, + service_id TEXT NOT NULL + ); + INSERT INTO feedback VALUES ('feedback-1', 'legacy-course'); + PRAGMA user_version = 1; + """ + ) + + try: + Database(path).migrate() + except RuntimeError as error: + assert "cannot be migrated as course ratings" in str(error) + else: + raise AssertionError("a service-feedback database must not be relabeled") diff --git a/flutter_app/README.md b/flutter_app/README.md index 6a7f96b..f6b043f 100644 --- a/flutter_app/README.md +++ b/flutter_app/README.md @@ -50,18 +50,21 @@ flutter run -d chrome \ --dart-define=STUDYOS_DEMO_OPENROUTER_API_KEY="$OPENROUTER_API_KEY" ``` -## Feedback Service +## Course Ratings -Ratings and comments use the repository's containerized feedback service. Point -the app at an HTTPS deployment at build time; plain HTTP is accepted only for a -loopback development endpoint: +Course lookup, ratings, and comments use the repository's containerized service. +It proxies the public StudyPlanner catalog so Flutter web does not depend on the +catalog's browser CORS policy. Point the app at an HTTPS deployment at build +time; plain HTTP is accepted only for a loopback development endpoint: ```sh flutter run -d chrome \ --dart-define=STUDYOS_FEEDBACK_API_URL=https://feedback.example.edu ``` -The endpoint is public configuration, not a secret. The service creates a +The endpoint is public configuration, not a secret. Open **Course ratings** in +the secondary navigation, search by title/number, and select a result. The +service creates a pseudonymous installation token on first submission, and the app stores it with `flutter_secure_storage` for later updates, deletion, and reports. Never put the service's moderator token or a GitHub credential in a Dart define. diff --git a/flutter_app/lib/src/app_router.dart b/flutter_app/lib/src/app_router.dart index c6502b0..b46bf55 100644 --- a/flutter_app/lib/src/app_router.dart +++ b/flutter_app/lib/src/app_router.dart @@ -7,6 +7,7 @@ import 'models.dart'; import 'onboarding_flow.dart'; import 'studyos_theme.dart'; import 'views/chat_route.dart'; +import 'views/course_ratings_view.dart'; import 'views/home_view.dart'; import 'views/maps_view.dart'; import 'views/memories_view.dart'; @@ -133,6 +134,13 @@ GoRouter buildAppRouter({ ), ), ), + GoRoute( + path: '/courses', + builder: (context, state) => _ScopedAppRoute( + controller: shellController(), + child: _CourseRatingsRoute(controller: shellController()), + ), + ), GoRoute( path: '/maps', builder: (context, state) => _ScopedAppRoute( @@ -282,6 +290,25 @@ class _MapsRoute extends StatelessWidget { } } +class _CourseRatingsRoute extends StatelessWidget { + const _CourseRatingsRoute({required this.controller}); + + final AppShellController? controller; + + @override + Widget build(BuildContext context) { + final controller = this.controller ?? AppShellScope.of(context); + return ListenableBuilder( + listenable: controller, + builder: (context, _) => _RouteScaffold( + title: 'Course ratings', + showTitle: false, + child: CourseRatingsView(academicStatus: controller.academicStatus), + ), + ); + } +} + class _MemoriesRoute extends StatelessWidget { const _MemoriesRoute({required this.controller}); diff --git a/flutter_app/lib/src/course_catalog_client.dart b/flutter_app/lib/src/course_catalog_client.dart new file mode 100644 index 0000000..e525baa --- /dev/null +++ b/flutter_app/lib/src/course_catalog_client.dart @@ -0,0 +1,110 @@ +import 'dart:async'; +import 'dart:convert'; + +import 'package:http/http.dart' as http; + +import 'course_catalog_models.dart'; +import 'feedback_client.dart'; + +class CourseCatalogClient { + CourseCatalogClient({ + String baseUrl = studyOsFeedbackApiUrl, + http.Client? httpClient, + this.requestTimeout = const Duration(seconds: 12), + }) : _baseUri = _parseBaseUri(baseUrl), + _httpClient = httpClient ?? http.Client(); + + final Uri? _baseUri; + final http.Client _httpClient; + final Duration requestTimeout; + + bool get isConfigured => _baseUri != null; + + Future> search( + String query, { + int limit = 20, + }) async { + final trimmed = query.trim(); + if (trimmed.length < 2) return const []; + final baseUri = _baseUri; + if (baseUri == null) { + throw const CourseCatalogException('Course lookup is not configured.'); + } + try { + final response = await _httpClient + .post( + _uri(baseUri, '/v1/courses/search'), + headers: const { + 'Accept': 'application/json', + 'Content-Type': 'application/json', + }, + body: jsonEncode({ + 'query': trimmed, + 'limit': limit.clamp(1, 25), + }), + ) + .timeout(requestTimeout); + if (response.statusCode < 200 || response.statusCode >= 300) { + throw CourseCatalogException( + 'Course lookup failed with HTTP ${response.statusCode}.', + ); + } + final decoded = jsonDecode(response.body); + if (decoded is! Map || decoded['courses'] is! List) { + throw const CourseCatalogException( + 'Course lookup returned an invalid response.', + ); + } + return (decoded['courses'] as List) + .whereType() + .map( + (item) => + CourseCatalogEntry.fromJson(Map.from(item)), + ) + .where( + (course) => + course.catalogId.isNotEmpty && + course.ratingCourseId.isNotEmpty && + course.courseNumber.isNotEmpty && + course.title.isNotEmpty, + ) + .toList(growable: false); + } on TimeoutException { + throw const CourseCatalogException('Course lookup timed out.'); + } on http.ClientException { + throw const CourseCatalogException('Course lookup is unreachable.'); + } on FormatException { + throw const CourseCatalogException( + 'Course lookup returned an invalid response.', + ); + } + } + + static Uri? _parseBaseUri(String value) { + final uri = Uri.tryParse(value.trim()); + if (uri == null || !uri.hasAuthority || uri.userInfo.isNotEmpty) { + return null; + } + final loopback = + uri.host == 'localhost' || uri.host == '127.0.0.1' || uri.host == '::1'; + if (uri.scheme != 'https' && !(loopback && uri.scheme == 'http')) { + return null; + } + if (uri.hasQuery || uri.hasFragment) return null; + return uri; + } + + static Uri _uri(Uri baseUri, String path) { + final relativePath = path.startsWith('/') ? path.substring(1) : path; + final basePath = baseUri.path.endsWith('/') + ? baseUri.path + : '${baseUri.path}/'; + return baseUri.replace(path: '$basePath$relativePath'); + } +} + +class CourseCatalogException implements Exception { + const CourseCatalogException(this.message); + + final String message; +} diff --git a/flutter_app/lib/src/course_catalog_models.dart b/flutter_app/lib/src/course_catalog_models.dart new file mode 100644 index 0000000..ffcd11e --- /dev/null +++ b/flutter_app/lib/src/course_catalog_models.dart @@ -0,0 +1,45 @@ +class CourseCatalogEntry { + const CourseCatalogEntry({ + required this.catalogId, + required this.ratingCourseId, + required this.courseNumber, + required this.title, + required this.periodLabel, + required this.ects, + required this.lecturer, + required this.types, + }); + + final String catalogId; + final String ratingCourseId; + final String courseNumber; + final String title; + final String periodLabel; + final double? ects; + final String? lecturer; + final List types; + + factory CourseCatalogEntry.fromJson(Map json) { + final rawTypes = json['types']; + return CourseCatalogEntry( + catalogId: json['courseId']?.toString() ?? '', + ratingCourseId: json['ratingCourseId']?.toString() ?? '', + courseNumber: json['courseNumber']?.toString().trim() ?? '', + title: json['title']?.toString().trim() ?? '', + periodLabel: json['periodLabel']?.toString().trim() ?? '', + ects: json['ects'] is num ? (json['ects'] as num).toDouble() : null, + lecturer: _optionalText(json['lecturer']), + types: rawTypes is List + ? rawTypes + .map((item) => item.toString().trim()) + .where((item) => item.isNotEmpty) + .toList(growable: false) + : const [], + ); + } +} + +String? _optionalText(Object? value) { + final text = value?.toString().trim(); + return text == null || text.isEmpty ? null : text; +} diff --git a/flutter_app/lib/src/feedback_client.dart b/flutter_app/lib/src/feedback_client.dart index ff05566..025a860 100644 --- a/flutter_app/lib/src/feedback_client.dart +++ b/flutter_app/lib/src/feedback_client.dart @@ -9,7 +9,6 @@ import 'feedback_token_store.dart'; const studyOsFeedbackApiUrl = String.fromEnvironment( 'STUDYOS_FEEDBACK_API_URL', ); -const studyOsFeedbackServiceId = 'studyos-agent'; class FeedbackClient { FeedbackClient({ @@ -28,12 +27,10 @@ class FeedbackClient { bool get isConfigured => _baseUri != null; - Future loadPublic({ - String serviceId = studyOsFeedbackServiceId, - }) async { + Future loadPublic({required String courseId}) async { final response = await _response( _httpClient.get( - _uri('/v1/services/${Uri.encodeComponent(serviceId)}/feedback/public'), + _uri('/v1/courses/${Uri.encodeComponent(courseId)}/feedback/public'), headers: _headers(), ), ); @@ -41,14 +38,12 @@ class FeedbackClient { return FeedbackPublicSnapshot.fromJson(json); } - Future loadOwn({ - String serviceId = studyOsFeedbackServiceId, - }) async { + Future loadOwn({required String courseId}) async { final token = await _tokenStore.read(); if (token == null || token.isEmpty) return null; final response = await _response( _httpClient.get( - _uri('/v1/services/${Uri.encodeComponent(serviceId)}/feedback/mine'), + _uri('/v1/courses/${Uri.encodeComponent(courseId)}/feedback/mine'), headers: _headers(token: token), ), ); @@ -62,8 +57,8 @@ class FeedbackClient { Future submit({ required int rating, + required String courseId, String? comment, - String serviceId = studyOsFeedbackServiceId, }) async { if (rating < 1 || rating > 5) { throw const FeedbackException('Choose a rating from 1 to 5 stars.'); @@ -81,7 +76,7 @@ class FeedbackClient { var token = await _installationToken(); var response = await _response( _httpClient.put( - _uri('/v1/services/${Uri.encodeComponent(serviceId)}/feedback/mine'), + _uri('/v1/courses/${Uri.encodeComponent(courseId)}/feedback/mine'), headers: _headers(token: token), body: body, ), @@ -91,7 +86,7 @@ class FeedbackClient { token = await _installationToken(); response = await _response( _httpClient.put( - _uri('/v1/services/${Uri.encodeComponent(serviceId)}/feedback/mine'), + _uri('/v1/courses/${Uri.encodeComponent(courseId)}/feedback/mine'), headers: _headers(token: token), body: body, ), @@ -100,14 +95,14 @@ class FeedbackClient { return FeedbackSubmission.fromJson(_successfulJson(response)); } - Future deleteOwn({String serviceId = studyOsFeedbackServiceId}) async { + Future deleteOwn({required String courseId}) async { final token = await _tokenStore.read(); if (token == null || token.isEmpty) { throw const FeedbackException('There is no saved feedback to delete.'); } final response = await _response( _httpClient.delete( - _uri('/v1/services/${Uri.encodeComponent(serviceId)}/feedback/mine'), + _uri('/v1/courses/${Uri.encodeComponent(courseId)}/feedback/mine'), headers: _headers(token: token), ), ); @@ -116,16 +111,16 @@ class FeedbackClient { Future report({ required String feedbackId, + required String courseId, String? reason, - String serviceId = studyOsFeedbackServiceId, }) async { final body = jsonEncode({'reason': reason?.trim()}); var token = await _installationToken(); - final service = Uri.encodeComponent(serviceId); + final course = Uri.encodeComponent(courseId); final feedback = Uri.encodeComponent(feedbackId); var response = await _response( _httpClient.post( - _uri('/v1/services/$service/feedback/$feedback/reports'), + _uri('/v1/courses/$course/feedback/$feedback/reports'), headers: _headers(token: token), body: body, ), @@ -135,7 +130,7 @@ class FeedbackClient { token = await _installationToken(); response = await _response( _httpClient.post( - _uri('/v1/services/$service/feedback/$feedback/reports'), + _uri('/v1/courses/$course/feedback/$feedback/reports'), headers: _headers(token: token), body: body, ), diff --git a/flutter_app/lib/src/feedback_models.dart b/flutter_app/lib/src/feedback_models.dart index 596ecae..fc97b13 100644 --- a/flutter_app/lib/src/feedback_models.dart +++ b/flutter_app/lib/src/feedback_models.dart @@ -1,7 +1,7 @@ class FeedbackSubmission { const FeedbackSubmission({ required this.id, - required this.serviceId, + required this.courseId, required this.rating, required this.comment, required this.commentState, @@ -10,7 +10,7 @@ class FeedbackSubmission { }); final String id; - final String serviceId; + final String courseId; final int rating; final String? comment; final String commentState; @@ -30,7 +30,7 @@ class FeedbackSubmission { factory FeedbackSubmission.fromJson(Map json) { return FeedbackSubmission( id: json['id']?.toString() ?? '', - serviceId: json['service_id']?.toString() ?? '', + courseId: json['course_id']?.toString() ?? '', rating: (json['rating'] as num?)?.toInt() ?? 0, comment: _optionalText(json['comment']), commentState: json['comment_state']?.toString() ?? 'none', @@ -65,21 +65,24 @@ class PublishedFeedbackComment { class FeedbackPublicSnapshot { const FeedbackPublicSnapshot({ - required this.serviceId, + required this.courseId, + required this.courseNumber, required this.ratingCount, required this.averageRating, required this.comments, }); - const FeedbackPublicSnapshot.empty(String serviceId) + const FeedbackPublicSnapshot.empty(String courseId) : this( - serviceId: serviceId, + courseId: courseId, + courseNumber: '', ratingCount: 0, averageRating: null, comments: const [], ); - final String serviceId; + final String courseId; + final String courseNumber; final int ratingCount; final double? averageRating; final List comments; @@ -91,7 +94,8 @@ class FeedbackPublicSnapshot { : const {}; final rawComments = json['comments']; return FeedbackPublicSnapshot( - serviceId: json['service_id']?.toString() ?? '', + courseId: json['course_id']?.toString() ?? '', + courseNumber: json['course_number']?.toString() ?? '', ratingCount: (rating['count'] as num?)?.toInt() ?? 0, averageRating: (rating['average'] as num?)?.toDouble(), comments: rawComments is List diff --git a/flutter_app/lib/src/views/course_ratings_view.dart b/flutter_app/lib/src/views/course_ratings_view.dart new file mode 100644 index 0000000..d4c24a7 --- /dev/null +++ b/flutter_app/lib/src/views/course_ratings_view.dart @@ -0,0 +1,207 @@ +import 'dart:async'; + +import 'package:flutter/material.dart'; + +import '../academic_models.dart'; +import '../course_catalog_client.dart'; +import '../course_catalog_models.dart'; +import '../studyos_theme.dart'; +import '../widgets/feedback_settings_card.dart'; + +class CourseRatingsView extends StatefulWidget { + const CourseRatingsView({ + required this.academicStatus, + this.catalogClient, + super.key, + }); + + final AcademicStatusSnapshot? academicStatus; + final CourseCatalogClient? catalogClient; + + @override + State createState() => _CourseRatingsViewState(); +} + +class _CourseRatingsViewState extends State { + late final TextEditingController _searchController; + late final CourseCatalogClient _catalogClient; + Timer? _debounce; + List _courses = const []; + CourseCatalogEntry? _selected; + String? _error; + bool _isSearching = false; + + @override + void initState() { + super.initState(); + _searchController = TextEditingController(); + _catalogClient = widget.catalogClient ?? CourseCatalogClient(); + } + + @override + void dispose() { + _debounce?.cancel(); + _searchController.dispose(); + super.dispose(); + } + + void _queueSearch(String query) { + _debounce?.cancel(); + _debounce = Timer(const Duration(milliseconds: 350), () => _search(query)); + } + + Future _search(String query) async { + final trimmed = query.trim(); + if (trimmed.length < 2) { + if (mounted) { + setState(() { + _courses = const []; + _error = null; + _isSearching = false; + _selected = null; + }); + } + return; + } + setState(() { + _isSearching = true; + _error = null; + _selected = null; + }); + try { + final courses = await _catalogClient.search(trimmed); + if (!mounted || _searchController.text.trim() != trimmed) return; + setState(() => _courses = courses); + } on CourseCatalogException catch (error) { + if (mounted) setState(() => _error = error.message); + } finally { + if (mounted && _searchController.text.trim() == trimmed) { + setState(() => _isSearching = false); + } + } + } + + void _searchFor(String query) { + _searchController.text = query; + _searchController.selection = TextSelection.collapsed(offset: query.length); + _search(query); + } + + List get _myCourseQueries { + final seen = {}; + return (widget.academicStatus?.entries ?? const []) + .map((entry) => entry.title.trim()) + .where((title) => title.length >= 2 && seen.add(title.toLowerCase())) + .take(6) + .toList(growable: false); + } + + @override + Widget build(BuildContext context) { + final myCourses = _myCourseQueries; + return ListView( + padding: const EdgeInsets.only( + top: StudyOsSpacing.xl, + bottom: StudyOsSpacing.xxl, + ), + children: [ + Text( + 'Course ratings', + style: Theme.of(context).textTheme.headlineSmall, + ), + const SizedBox(height: StudyOsSpacing.xs), + Text( + 'Find a course in the shared StudyPlanner catalog, then read or add a community rating.', + style: Theme.of(context).textTheme.bodyMedium, + ), + if (myCourses.isNotEmpty) ...[ + const SizedBox(height: StudyOsSpacing.lg), + Text('MY COURSES', style: Theme.of(context).textTheme.labelMedium), + const SizedBox(height: StudyOsSpacing.sm), + Wrap( + spacing: StudyOsSpacing.sm, + runSpacing: StudyOsSpacing.sm, + children: myCourses + .map( + (title) => ActionChip( + avatar: const Icon(Icons.school_outlined, size: 18), + label: Text(title, overflow: TextOverflow.ellipsis), + onPressed: () => _searchFor(title), + ), + ) + .toList(growable: false), + ), + ], + const SizedBox(height: StudyOsSpacing.lg), + TextField( + controller: _searchController, + onChanged: _queueSearch, + onSubmitted: _search, + textInputAction: TextInputAction.search, + decoration: InputDecoration( + labelText: 'Search the course catalog', + hintText: 'Course title or number', + prefixIcon: const Icon(Icons.search_rounded), + suffixIcon: _isSearching + ? const Padding( + padding: EdgeInsets.all(14), + child: CircularProgressIndicator(strokeWidth: 2), + ) + : null, + ), + ), + if (_error != null) ...[ + const SizedBox(height: StudyOsSpacing.sm), + Text( + _error!, + style: TextStyle(color: Theme.of(context).colorScheme.error), + ), + ], + if (!_isSearching && + _error == null && + _searchController.text.trim().length >= 2 && + _courses.isEmpty) ...[ + const SizedBox(height: StudyOsSpacing.lg), + const Text('No matching courses found.'), + ], + if (_courses.isNotEmpty) ...[ + const SizedBox(height: StudyOsSpacing.lg), + ..._courses.map( + (course) => Card( + child: ListTile( + title: Text(course.title), + subtitle: Text( + [ + course.courseNumber, + if (course.periodLabel.isNotEmpty) course.periodLabel, + if (course.ects != null) + '${course.ects!.toStringAsCompact()} ECTS', + ].join(' · '), + ), + trailing: const Icon(Icons.chevron_right_rounded), + onTap: () => setState(() => _selected = course), + ), + ), + ), + ], + if (_selected != null) ...[ + const Divider(height: StudyOsSpacing.xxl), + CourseFeedbackCard( + key: ValueKey(_selected!.ratingCourseId), + course: _selected!, + ), + ], + const SizedBox(height: StudyOsSpacing.lg), + Text( + 'Your enrollment list stays on this device. Choosing a shortcut sends that course title to this service and the public StudyPlanner catalog as a search. Ratings are pseudonymous community feedback, not verified enrollment reviews.', + style: Theme.of(context).textTheme.bodySmall, + ), + ], + ); + } +} + +extension on double { + String toStringAsCompact() => + this == roundToDouble() ? toInt().toString() : toStringAsFixed(1); +} diff --git a/flutter_app/lib/src/views/settings_view.dart b/flutter_app/lib/src/views/settings_view.dart index 77dcd82..96845e4 100644 --- a/flutter_app/lib/src/views/settings_view.dart +++ b/flutter_app/lib/src/views/settings_view.dart @@ -4,7 +4,6 @@ import '../assistant_copy.dart'; import '../models.dart'; import '../native_bridge.dart'; import '../studyos_theme.dart'; -import '../widgets/feedback_settings_card.dart'; import '../widgets/cloud_assistant_settings.dart'; import '../widgets/local_model_settings_card.dart'; import '../widgets/profile_row.dart'; @@ -233,11 +232,6 @@ class _SettingsViewState extends State { ], ), ), - const SizedBox(height: StudyOsSpacing.xl), - _SettingsSection( - title: 'Support', - child: SettingsCard(children: const [FeedbackSettingsCard()]), - ), ], ); } diff --git a/flutter_app/lib/src/widgets/feedback_settings_card.dart b/flutter_app/lib/src/widgets/feedback_settings_card.dart index 1674dcf..c85c6b9 100644 --- a/flutter_app/lib/src/widgets/feedback_settings_card.dart +++ b/flutter_app/lib/src/widgets/feedback_settings_card.dart @@ -2,19 +2,21 @@ import 'package:flutter/material.dart'; import '../feedback_client.dart'; import '../feedback_models.dart'; +import '../course_catalog_models.dart'; import '../studyos_theme.dart'; import 'feedback_components.dart'; -class FeedbackSettingsCard extends StatefulWidget { - const FeedbackSettingsCard({this.client, super.key}); +class CourseFeedbackCard extends StatefulWidget { + const CourseFeedbackCard({required this.course, this.client, super.key}); + final CourseCatalogEntry course; final FeedbackClient? client; @override - State createState() => _FeedbackSettingsCardState(); + State createState() => _CourseFeedbackCardState(); } -class _FeedbackSettingsCardState extends State { +class _CourseFeedbackCardState extends State { late final TextEditingController _controller; late final FeedbackClient _client; FeedbackPublicSnapshot? _snapshot; @@ -46,8 +48,9 @@ class _FeedbackSettingsCardState extends State { return; } try { - final snapshot = await _client.loadPublic(); - final own = await _client.loadOwn(); + final courseId = widget.course.ratingCourseId; + final snapshot = await _client.loadPublic(courseId: courseId); + final own = await _client.loadOwn(courseId: courseId); if (!mounted) return; setState(() { _snapshot = snapshot; @@ -71,10 +74,13 @@ class _FeedbackSettingsCardState extends State { setState(() => _isSending = true); try { final own = await _client.submit( + courseId: widget.course.ratingCourseId, rating: _rating, comment: _controller.text, ); - final snapshot = await _client.loadPublic(); + final snapshot = await _client.loadPublic( + courseId: widget.course.ratingCourseId, + ); if (!mounted) return; setState(() { _ownFeedback = own; @@ -116,8 +122,10 @@ class _FeedbackSettingsCardState extends State { if (confirmed != true || !mounted) return; setState(() => _isDeleting = true); try { - await _client.deleteOwn(); - final snapshot = await _client.loadPublic(); + await _client.deleteOwn(courseId: widget.course.ratingCourseId); + final snapshot = await _client.loadPublic( + courseId: widget.course.ratingCourseId, + ); if (!mounted) return; setState(() { _ownFeedback = null; @@ -163,7 +171,11 @@ class _FeedbackSettingsCardState extends State { if (reason == null || !mounted) return; setState(() => _reportingId = comment.id); try { - await _client.report(feedbackId: comment.id, reason: reason); + await _client.report( + courseId: widget.course.ratingCourseId, + feedbackId: comment.id, + reason: reason, + ); _showMessage('Comment reported for review.'); } on FeedbackException catch (error) { _showMessage(error.message); @@ -185,10 +197,14 @@ class _FeedbackSettingsCardState extends State { return Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ - Text('Rate StudyOS', style: Theme.of(context).textTheme.titleMedium), + Text( + widget.course.title, + style: Theme.of(context).textTheme.titleMedium, + ), const SizedBox(height: StudyOsSpacing.xs), Text( - 'Ratings are public. Comments appear after moderation.', + '${widget.course.courseNumber} · ${widget.course.periodLabel}\n' + 'Community ratings are public. Comments appear after moderation.', style: Theme.of(context).textTheme.bodyMedium, ), const SizedBox(height: StudyOsSpacing.sm), diff --git a/flutter_app/lib/src/widgets/secondary_destinations_drawer.dart b/flutter_app/lib/src/widgets/secondary_destinations_drawer.dart index 65b8b3c..f362133 100644 --- a/flutter_app/lib/src/widgets/secondary_destinations_drawer.dart +++ b/flutter_app/lib/src/widgets/secondary_destinations_drawer.dart @@ -23,6 +23,11 @@ class SecondaryDestinationsDrawer extends StatelessWidget { label: 'Notes', path: '/memories', ), + _DrawerDestination( + icon: Icons.rate_review_outlined, + label: 'Course ratings', + path: '/courses', + ), _DrawerDestination( icon: Icons.map_outlined, label: 'Map', diff --git a/flutter_app/test/course_catalog_client_test.dart b/flutter_app/test/course_catalog_client_test.dart new file mode 100644 index 0000000..b4f94c3 --- /dev/null +++ b/flutter_app/test/course_catalog_client_test.dart @@ -0,0 +1,55 @@ +import 'dart:convert'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:studyos_agent/src/course_catalog_client.dart'; + +void main() { + test( + 'searches the ratings service catalog proxy and parses courses', + () async { + late http.Request captured; + final client = CourseCatalogClient( + baseUrl: 'https://feedback.studyos.test', + httpClient: MockClient((request) async { + captured = request; + return http.Response( + jsonEncode({ + 'courses': [ + { + 'courseId': '598', + 'ratingCourseId': 'SU5GTTEyMzQ', + 'courseNumber': 'INFM1234', + 'title': 'Machine Learning', + 'periodLabel': 'Sommer 2026', + 'ects': 6.0, + 'lecturer': 'Ada Example', + 'types': ['Lecture'], + }, + ], + }), + 200, + ); + }), + ); + + final courses = await client.search('machine learning'); + + expect(captured.method, 'POST'); + expect(captured.url.path, '/v1/courses/search'); + expect(captured.body, contains('machine learning')); + expect(courses.single.courseNumber, 'INFM1234'); + expect(courses.single.ratingCourseId, 'SU5GTTEyMzQ'); + }, + ); + + test('does not call the catalog for a one-character query', () async { + final client = CourseCatalogClient( + baseUrl: 'https://feedback.studyos.test', + httpClient: MockClient((_) async => throw StateError('not expected')), + ); + + expect(await client.search('x'), isEmpty); + }); +} diff --git a/flutter_app/test/feedback_client_test.dart b/flutter_app/test/feedback_client_test.dart index 5cf4010..9dd7060 100644 --- a/flutter_app/test/feedback_client_test.dart +++ b/flutter_app/test/feedback_client_test.dart @@ -30,13 +30,14 @@ void main() { ); final feedback = await client.submit( + courseId: _courseId, rating: 4, comment: ' Useful, but needs clearer sources. ', ); expect(requests.map((item) => item.url.path), [ '/v1/installations', - '/v1/services/studyos-agent/feedback/mine', + '/v1/courses/$_courseId/feedback/mine', ]); expect(requests.last.headers['Authorization'], 'Bearer $_newToken'); expect(requests.last.body, contains('Useful, but needs clearer sources.')); @@ -62,7 +63,7 @@ void main() { }), ); - await client.submit(rating: 5); + await client.submit(courseId: _courseId, rating: 5); expect(requestCount, 1); }); @@ -95,7 +96,7 @@ void main() { }), ); - await client.submit(rating: 4); + await client.submit(courseId: _courseId, rating: 4); expect(putCount, 2); expect(tokenStore.token, _newToken); @@ -109,7 +110,8 @@ void main() { httpClient: MockClient( (_) async => http.Response( jsonEncode({ - 'service_id': 'studyos-agent', + 'course_id': _courseId, + 'course_number': 'INFM1234', 'rating': {'count': 2, 'average': 4.5}, 'comments': [ { @@ -125,7 +127,7 @@ void main() { ), ); - final snapshot = await client.loadPublic(); + final snapshot = await client.loadPublic(courseId: _courseId); expect(snapshot.ratingCount, 2); expect(snapshot.averageRating, 4.5); @@ -147,12 +149,16 @@ void main() { }), ); - await client.report(feedbackId: 'feedback-1', reason: 'spam'); - await client.deleteOwn(); + await client.report( + courseId: _courseId, + feedbackId: 'feedback-1', + reason: 'spam', + ); + await client.deleteOwn(courseId: _courseId); expect( requests.first.url.path, - '/v1/services/studyos-agent/feedback/feedback-1/reports', + '/v1/courses/$_courseId/feedback/feedback-1/reports', ); expect(requests.first.body, contains('spam')); expect(requests.last.method, 'DELETE'); @@ -173,7 +179,7 @@ void main() { httpClient: MockClient((_) async => throw StateError('not expected')), ); - expect(await client.loadOwn(), isNull); + expect(await client.loadOwn(courseId: _courseId), isNull); }); test('rejects insecure non-loopback endpoint and invalid ratings', () async { @@ -188,7 +194,10 @@ void main() { expect(insecure.isConfigured, isFalse); expect(local.isConfigured, isTrue); - expect(() => local.submit(rating: 0), throwsA(isA())); + expect( + () => local.submit(courseId: _courseId, rating: 0), + throwsA(isA()), + ); }); test('surfaces bounded API error details', () async { @@ -206,7 +215,7 @@ void main() { ); expect( - () => client.submit(rating: 3), + () => client.submit(courseId: _courseId, rating: 3), throwsA( isA().having( (error) => error.message, @@ -226,7 +235,7 @@ void main() { ); expect( - () => client.loadPublic(), + () => client.loadPublic(courseId: _courseId), throwsA( isA().having( (error) => error.message, @@ -247,7 +256,7 @@ void main() { ); expect( - () => client.loadPublic(), + () => client.loadPublic(courseId: _courseId), throwsA( isA().having( (error) => error.message, @@ -276,7 +285,7 @@ class _MemoryTokenStore implements FeedbackTokenStore { final String _submissionJson = jsonEncode({ 'id': 'feedback-1', - 'service_id': 'studyos-agent', + 'course_id': _courseId, 'rating': 4, 'comment': 'Useful, but needs clearer sources.', 'comment_state': 'pending', @@ -285,3 +294,4 @@ final String _submissionJson = jsonEncode({ }); const String _newToken = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'; +const String _courseId = 'SU5GTTEyMzQ'; diff --git a/flutter_app/test/feedback_settings_card_test.dart b/flutter_app/test/feedback_settings_card_test.dart index db523b2..f117639 100644 --- a/flutter_app/test/feedback_settings_card_test.dart +++ b/flutter_app/test/feedback_settings_card_test.dart @@ -6,6 +6,7 @@ import 'package:http/http.dart' as http; import 'package:http/testing.dart'; import 'package:studyos_agent/src/feedback_client.dart'; import 'package:studyos_agent/src/feedback_token_store.dart'; +import 'package:studyos_agent/src/course_catalog_models.dart'; import 'package:studyos_agent/src/widgets/feedback_settings_card.dart'; void main() { @@ -31,7 +32,7 @@ void main() { return http.Response( jsonEncode({ 'id': 'feedback-1', - 'service_id': 'studyos-agent', + 'course_id': _courseId, 'rating': 4, 'comment': 'Helpful service catalog', 'comment_state': 'pending', @@ -43,7 +44,8 @@ void main() { } return http.Response( jsonEncode({ - 'service_id': 'studyos-agent', + 'course_id': _courseId, + 'course_number': 'INFM1234', 'rating': { 'count': request.method == 'GET' && requests.length > 3 ? 1 : 0, 'average': requests.length > 3 ? 4.0 : null, @@ -59,7 +61,7 @@ void main() { MaterialApp( home: Scaffold( body: SingleChildScrollView( - child: FeedbackSettingsCard(client: client), + child: CourseFeedbackCard(course: _course, client: client), ), ), ), @@ -87,7 +89,8 @@ void main() { await tester.pumpWidget( MaterialApp( home: Scaffold( - body: FeedbackSettingsCard( + body: CourseFeedbackCard( + course: _course, client: FeedbackClient( baseUrl: '', tokenStore: _MemoryTokenStore(), @@ -111,14 +114,14 @@ void main() { httpClient: MockClient((request) async { if (request.url.path.endsWith('/public')) { return http.Response( - '{"service_id":"studyos-agent","rating":{"count":1,"average":2.0},"comments":[]}', + '{"course_id":"$_courseId","course_number":"INFM1234","rating":{"count":1,"average":2.0},"comments":[]}', 200, ); } return http.Response( jsonEncode({ 'id': 'feedback-1', - 'service_id': 'studyos-agent', + 'course_id': _courseId, 'rating': 2, 'comment': 'Needs work', 'comment_state': 'rejected', @@ -132,7 +135,9 @@ void main() { await tester.pumpWidget( MaterialApp( - home: Scaffold(body: FeedbackSettingsCard(client: client)), + home: Scaffold( + body: CourseFeedbackCard(course: _course, client: client), + ), ), ); await tester.pumpAndSettle(); @@ -158,14 +163,14 @@ void main() { } if (request.url.path.endsWith('/public')) { return http.Response( - '{"service_id":"studyos-agent","rating":{"count":${deleted ? 0 : 1},"average":${deleted ? 'null' : '5.0'}},"comments":[]}', + '{"course_id":"$_courseId","course_number":"INFM1234","rating":{"count":${deleted ? 0 : 1},"average":${deleted ? 'null' : '5.0'}},"comments":[]}', 200, ); } return http.Response( jsonEncode({ 'id': 'feedback-1', - 'service_id': 'studyos-agent', + 'course_id': _courseId, 'rating': 5, 'comment': null, 'comment_state': 'none', @@ -179,7 +184,9 @@ void main() { await tester.pumpWidget( MaterialApp( - home: Scaffold(body: FeedbackSettingsCard(client: client)), + home: Scaffold( + body: CourseFeedbackCard(course: _course, client: client), + ), ), ); await tester.pumpAndSettle(); @@ -208,7 +215,8 @@ void main() { } return http.Response( jsonEncode({ - 'service_id': 'studyos-agent', + 'course_id': _courseId, + 'course_number': 'INFM1234', 'rating': {'count': 1, 'average': 1.0}, 'comments': [ { @@ -228,7 +236,7 @@ void main() { MaterialApp( home: Scaffold( body: SingleChildScrollView( - child: FeedbackSettingsCard(client: client), + child: CourseFeedbackCard(course: _course, client: client), ), ), ), @@ -263,3 +271,14 @@ class _MemoryTokenStore implements FeedbackTokenStore { } const String _token = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'; +const String _courseId = 'SU5GTTEyMzQ'; +const CourseCatalogEntry _course = CourseCatalogEntry( + catalogId: '598', + ratingCourseId: _courseId, + courseNumber: 'INFM1234', + title: 'Machine Learning', + periodLabel: 'Sommer 2026', + ects: 6, + lecturer: null, + types: ['Lecture'], +); From d90f3788f3a71fe49877cff7285df81f68526745 Mon Sep 17 00:00:00 2001 From: StudyOS Org Date: Fri, 17 Jul 2026 09:04:46 +0000 Subject: [PATCH 3/3] test: remove obsolete agent feedback assertion --- flutter_app/test/widget_test.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/flutter_app/test/widget_test.dart b/flutter_app/test/widget_test.dart index cfafffc..d63b619 100644 --- a/flutter_app/test/widget_test.dart +++ b/flutter_app/test/widget_test.dart @@ -194,7 +194,6 @@ void main() { expect(savedConfig?.cloudModel, 'studyos'); expect(savedKey, 'secret'); expect(find.text('Stored securely on this device.'), findsOneWidget); - expect(find.text('Feedback is unavailable'), findsOneWidget); }); testWidgets('settings profile editor updates onboarding preferences', (