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..2c634c8 100644 --- a/README.md +++ b/README.md @@ -112,6 +112,23 @@ flutter run -d chrome \ --dart-define=STUDYOS_DEMO_OPENROUTER_API_KEY="$OPENROUTER_API_KEY" ``` +## Course Ratings Deployment + +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 \ + --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..dd9471b --- /dev/null +++ b/feedback_service/README.md @@ -0,0 +1,117 @@ +# StudyOS course ratings service + +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. + +## 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_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` + +## 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. + +## 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 +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/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 new file mode 100644 index 0000000..df7f510 --- /dev/null +++ b/feedback_service/compose.example.yml @@ -0,0 +1,36 @@ +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_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 + 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..637a7c0 --- /dev/null +++ b/feedback_service/config.py @@ -0,0 +1,65 @@ +from __future__ import annotations + +import os +import re +from dataclasses import dataclass +from pathlib import Path + + +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" + 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": + 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"), + 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() + ), + 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 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 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 new file mode 100644 index 0000000..75c32a8 --- /dev/null +++ b/feedback_service/db.py @@ -0,0 +1,315 @@ +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 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), + 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 ( + 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, course_id) + ); + 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), + 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 = 2; + """ + ) + 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: + 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 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, 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, course_id, rating, comment, comment_state, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?) + ON CONFLICT(installation_id, course_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, + course_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, course_id) + + def feedback_for_owner(self, installation_id: str, course_id: str): + with self.connect() as db: + return db.execute( + """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, 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 course_id=? AND deleted_at IS NULL""", + (timestamp, timestamp, installation_id, course_id), + ) + return result.rowcount == 1 + + 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 course_id=? AND deleted_at IS NULL", + (course_id,), + ).fetchone() + comments = db.execute( + """SELECT id, rating, comment, published_at FROM feedback + WHERE course_id=? AND deleted_at IS NULL AND comment_state='published' + ORDER BY published_at DESC, id LIMIT ? OFFSET ?""", + (course_id, limit, offset), + ).fetchall() + return { + "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}, + } + + def report( + self, + installation_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 course_id=? + AND comment_state='published' AND deleted_at IS NULL""", + (feedback_id, course_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, + course_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 course_id: + where += " AND f.course_id=?" + params.append(course_id) + with self.connect() as db: + return db.execute( + 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 + 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..9d8e4e3 --- /dev/null +++ b/feedback_service/main.py @@ -0,0 +1,307 @@ +from __future__ import annotations + +import hmac +import base64 +import binascii +import unicodedata +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 .catalog import CatalogUnavailable, StudyPlannerCatalog +from .db import Database +from .models import CourseSearchInput, FeedbackInput, ModerationInput, ReportInput +from .security import SlidingWindowLimiter, issue_token, token_hash + +bearer = HTTPBearer(auto_error=False) + + +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 Course Ratings", 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 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)], + ) -> 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.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( + course_id: str, + limit: Annotated[int, Query(ge=1, le=100)] = 20, + offset: Annotated[int, Query(ge=0, le=10_000)] = 0, + ) -> dict: + course_ref, course_number = course(course_id) + return database.public_feedback(course_ref, course_number, limit, offset) + + @app.get("/v1/courses/{course_id}/feedback/mine") + def own_feedback( + course_id: str, installation_id: str = Depends(installation) + ) -> dict: + 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/courses/{course_id}/feedback/mine") + def put_feedback( + course_id: str, + payload: FeedbackInput, + installation_id: str = Depends(limited_installation), + ) -> dict: + return dict( + database.upsert_feedback( + installation_id, course(course_id)[0], payload.rating, payload.comment + ) + ) + + @app.delete( + "/v1/courses/{course_id}/feedback/mine", + status_code=status.HTTP_204_NO_CONTENT, + ) + def delete_feedback( + course_id: str, installation_id: str = Depends(limited_installation) + ) -> Response: + 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/courses/{course_id}/feedback/{feedback_id}/reports", + status_code=status.HTTP_201_CREATED, + ) + def report_feedback( + course_id: str, + feedback_id: str, + payload: ReportInput, + installation_id: str = Depends(limited_installation), + ) -> dict[str, str]: + result = database.report( + installation_id, course(course_id)[0], 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", + 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 course_id is not None: + course_id = course(course_id)[0] + items = [ + dict(row) + for row in database.moderation_queue( + state, + course_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..98b1ad7 --- /dev/null +++ b/feedback_service/models.py @@ -0,0 +1,66 @@ +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 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") + + 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..4b09f63 --- /dev/null +++ b/feedback_service/tests/conftest.py @@ -0,0 +1,57 @@ +from pathlib import Path + +import pytest +from fastapi.testclient import TestClient + +from feedback_service.config import Settings +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( + Settings( + database_path=tmp_path / "feedback.sqlite3", + admin_token="test-admin-token-that-is-at-least-32-characters", + moderator_id="moderator@example.test", + 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 + + +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..f99686a --- /dev/null +++ b/feedback_service/tests/test_api.py @@ -0,0 +1,308 @@ +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 + + +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): + 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/courses/{COURSE_ID}/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_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 + 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, + COURSE_ID, + 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_invalid_course_and_health(client: TestClient): + assert client.get("/healthz").json() == {"status": "ok"} + 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 ( + 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] == 2 + try: + db.execute( + """INSERT INTO feedback + (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 + 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_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/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..f6b043f 100644 --- a/flutter_app/README.md +++ b/flutter_app/README.md @@ -50,6 +50,25 @@ flutter run -d chrome \ --dart-define=STUDYOS_DEMO_OPENROUTER_API_KEY="$OPENROUTER_API_KEY" ``` +## Course Ratings + +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. 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. + ## Native Bridge The current bridge initializes native Android managers, routes Android messages 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 5f48147..025a860 100644 --- a/flutter_app/lib/src/feedback_client.dart +++ b/flutter_app/lib/src/feedback_client.dart @@ -1,71 +1,232 @@ +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', +); 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({required String courseId}) async { + final response = await _response( + _httpClient.get( + _uri('/v1/courses/${Uri.encodeComponent(courseId)}/feedback/public'), + headers: _headers(), + ), + ); + final json = _successfulJson(response); + return FeedbackPublicSnapshot.fromJson(json); + } - Future submit({ - required String token, - required String message, - required String status, - String repository = studyOsFeedbackRepository, + 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/courses/${Uri.encodeComponent(courseId)}/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 int rating, + required String courseId, + String? comment, }) 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/courses/${Uri.encodeComponent(courseId)}/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/courses/${Uri.encodeComponent(courseId)}/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({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/courses/${Uri.encodeComponent(courseId)}/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, + required String courseId, + String? reason, + }) async { + final body = jsonEncode({'reason': reason?.trim()}); + var token = await _installationToken(); + final course = Uri.encodeComponent(courseId); + final feedback = Uri.encodeComponent(feedbackId); + var response = await _response( + _httpClient.post( + _uri('/v1/courses/$course/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/courses/$course/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; } - 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'); + 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'); + } + + 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.'); } - return buffer.toString().trimRight(); } - String _titleFrom(String message) { - final firstLine = message.split('\n').first.trim(); - if (firstLine.length <= 72) return firstLine; - return '${firstLine.substring(0, 69)}...'; + 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 json; + } + + 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..fc97b13 --- /dev/null +++ b/flutter_app/lib/src/feedback_models.dart @@ -0,0 +1,124 @@ +class FeedbackSubmission { + const FeedbackSubmission({ + required this.id, + required this.courseId, + required this.rating, + required this.comment, + required this.commentState, + required this.createdAt, + required this.updatedAt, + }); + + final String id; + final String courseId; + 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() ?? '', + courseId: json['course_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.courseId, + required this.courseNumber, + required this.ratingCount, + required this.averageRating, + required this.comments, + }); + + const FeedbackPublicSnapshot.empty(String courseId) + : this( + courseId: courseId, + courseNumber: '', + ratingCount: 0, + averageRating: null, + comments: const [], + ); + + final String courseId; + final String courseNumber; + 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( + 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 + ? 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/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 aa45ee6..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,13 +232,6 @@ class _SettingsViewState extends State { ], ), ), - const SizedBox(height: StudyOsSpacing.xl), - _SettingsSection( - title: 'Support', - child: SettingsCard( - children: [FeedbackSettingsCard(status: widget.status)], - ), - ), ], ); } 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..c85c6b9 100644 --- a/flutter_app/lib/src/widgets/feedback_settings_card.dart +++ b/flutter_app/lib/src/widgets/feedback_settings_card.dart @@ -1,26 +1,39 @@ 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({required this.status, this.client, super.key}); +class CourseFeedbackCard extends StatefulWidget { + const CourseFeedbackCard({required this.course, this.client, super.key}); - final String status; + 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; + 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 +42,56 @@ class _FeedbackSettingsCardState extends State { super.dispose(); } + Future _load() async { + if (!_client.isConfigured) { + if (mounted) setState(() => _isLoading = false); + return; + } + try { + 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; + _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( + courseId: widget.course.ratingCourseId, + rating: _rating, + comment: _controller.text, + ); + final snapshot = await _client.loadPublic( + courseId: widget.course.ratingCourseId, + ); + 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 +99,91 @@ 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(courseId: widget.course.ratingCourseId); + final snapshot = await _client.loadPublic( + courseId: widget.course.ratingCourseId, + ); + 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( + courseId: widget.course.ratingCourseId, + 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 +191,109 @@ 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', + widget.course.title, style: Theme.of(context).textTheme.titleMedium, ), const SizedBox(height: StudyOsSpacing.xs), Text( - 'Tell us what worked or what got in your way.', + '${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.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/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 f1a0e8c..9dd7060 100644 --- a/flutter_app/test/feedback_client_test.dart +++ b/flutter_app/test/feedback_client_test.dart @@ -1,41 +1,297 @@ +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 { - requestUri = request.url; - authorization = request.headers['Authorization'] ?? ''; - requestBody = request.body; - return http.Response('{}', 201); + 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); }), ); - await client.submit( - token: 'github-token', - message: 'Mensa widget would be useful', - status: 'Ready', + 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/courses/$_courseId/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(courseId: _courseId, 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(courseId: _courseId, 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({ + 'course_id': _courseId, + 'course_number': 'INFM1234', + 'rating': {'count': 2, 'average': 4.5}, + 'comments': [ + { + 'id': 'feedback-1', + 'rating': 4, + 'comment': 'Clear and useful', + 'published_at': '2026-07-16T16:00:00Z', + }, + ], + }), + 200, + ), + ), ); - 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')); + final snapshot = await client.loadPublic(courseId: _courseId); + + expect(snapshot.ratingCount, 2); + expect(snapshot.averageRating, 4.5); + expect(snapshot.comments.single.comment, 'Clear and useful'); }); - test('feedback client rejects missing token', () async { - final client = FeedbackClient(); + 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 { + requests.add(request); + return request.method == 'DELETE' + ? http.Response('', 204) + : http.Response('{"status":"recorded"}', 201); + }), + ); + await client.report( + courseId: _courseId, + feedbackId: 'feedback-1', + reason: 'spam', + ); + await client.deleteOwn(courseId: _courseId); + + expect( + requests.first.url.path, + '/v1/courses/$_courseId/feedback/feedback-1/reports', + ); + expect(requests.first.body, contains('spam')); + expect(requests.last.method, 'DELETE'); expect( - () => client.submit(token: '', message: 'Hello', status: 'Ready'), + 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(courseId: _courseId), 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(courseId: _courseId, 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(courseId: _courseId, 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( + () => client.loadPublic(courseId: _courseId), + throwsA( + isA().having( + (error) => error.message, + 'message', + 'The feedback service timed out.', + ), + ), + ); + }); + + 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.loadPublic(courseId: _courseId), + 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', + 'course_id': _courseId, + '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'; +const String _courseId = 'SU5GTTEyMzQ'; 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..f117639 --- /dev/null +++ b/flutter_app/test/feedback_settings_card_test.dart @@ -0,0 +1,284 @@ +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/course_catalog_models.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', + 'course_id': _courseId, + '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({ + 'course_id': _courseId, + 'course_number': 'INFM1234', + '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: CourseFeedbackCard(course: _course, 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: CourseFeedbackCard( + course: _course, + 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( + '{"course_id":"$_courseId","course_number":"INFM1234","rating":{"count":1,"average":2.0},"comments":[]}', + 200, + ); + } + return http.Response( + jsonEncode({ + 'id': 'feedback-1', + 'course_id': _courseId, + '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: CourseFeedbackCard(course: _course, 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( + '{"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', + 'course_id': _courseId, + '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: CourseFeedbackCard(course: _course, 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({ + 'course_id': _courseId, + 'course_number': 'INFM1234', + '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: CourseFeedbackCard(course: _course, 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'; +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'], +); diff --git a/flutter_app/test/widget_test.dart b/flutter_app/test/widget_test.dart index 5c048c9..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('Send feedback'), findsOneWidget); }); testWidgets('settings profile editor updates onboarding preferences', (