diff --git a/backend/app/main.py b/backend/app/main.py index 241492d..01bc441 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -11,6 +11,7 @@ from app.logging_config import setup_logging setup_logging(settings.LOG_LEVEL) from app.database import engine, Base, SessionLocal from app.routers import auth, documents, quizzes, attempts, admin, tts, nextcloud, categories, questions, question_categories, favorites, teach, contact, tags, flashcards, courses, mobile, mynote, exams +from app.routers import access from app.routers import study_tools, uploads, articles, comments, share, collections, study_plans, media, search, ai_mode from app.utils.auth import get_password_hash @@ -612,6 +613,7 @@ app.include_router(uploads.router) app.include_router(auth.router, prefix="/api/auth", tags=["auth"]) app.include_router(articles.router, prefix="/api/articles", tags=["articles"]) +app.include_router(access.router, prefix="/api/access", tags=["access"]) app.include_router(exams.router, prefix="/api/exams", tags=["exams"]) app.include_router(study_plans.router, prefix="/api/study-plans", tags=["study-plans"]) app.include_router(media.router, prefix="/api/media", tags=["media"]) diff --git a/backend/app/routers/access.py b/backend/app/routers/access.py new file mode 100644 index 0000000..3c22ed4 --- /dev/null +++ b/backend/app/routers/access.py @@ -0,0 +1,164 @@ +"""Who may edit what — every kind of access in one place. + +Access was granted in three separate screens over two tables: category grants +from the question manager, media-library grants from the image bank, and +nothing at all for articles. Nobody could see what one person actually had. + +This is one surface over the same tables. It does not invent a new permission +model; it reads and writes the grants that already exist, and adds the one +answer that was missing — "everything", which is not a grant but the moderator +role, and says so rather than pretending otherwise. + +Inheritance is already how category grants work: a grant on a branch covers +every category beneath it, the questions filed under them, and now the articles +too. +""" +from fastapi import APIRouter, Depends, HTTPException +from pydantic import BaseModel +from sqlalchemy import func +from sqlalchemy.orm import Session + +from app.database import get_db +from app.models.article import Article +from app.models.category_grant import CategoryGrant +from app.models.media import MediaAsset, MediaLibrary, MediaLibraryGrant +from app.models.question import Question +from app.models.question_category import QuestionCategory +from app.models.user import User +from app.utils.auth import require_moderator +from app.utils.category_grants import _descendants + +router = APIRouter() + + +class GrantWrite(BaseModel): + kind: str # category | library + target_id: int + + +class RoleWrite(BaseModel): + #: "moderator" is everything; "user" is only what the grants below cover. + role: str + + +def _user_json(user: User) -> dict: + return {"id": user.id, "name": user.name, "email": user.email, "role": user.role} + + +@router.get("/") +def access_overview(db: Session = Depends(get_db), current_user: User = Depends(require_moderator)): + """Everyone who can be given access, and what each of them holds.""" + users = db.query(User).order_by(User.name).all() + category_grants: dict[int, list[int]] = {} + for user_id, category_id in db.query(CategoryGrant.user_id, CategoryGrant.category_id).filter( + CategoryGrant.category_id.isnot(None)).all(): + category_grants.setdefault(user_id, []).append(category_id) + library_grants: dict[int, list[int]] = {} + for user_id, library_id in db.query(MediaLibraryGrant.user_id, MediaLibraryGrant.library_id).all(): + library_grants.setdefault(user_id, []).append(library_id) + + rows = [] + for user in users: + categories = category_grants.get(user.id, []) + # What the grants actually reach, once the tree is walked. + covered = _descendants(db, set(categories)) + rows.append({ + **_user_json(user), + "everything": bool(user.is_moderator), + "categories": sorted(categories), + "categories_covered": len(covered), + "libraries": sorted(library_grants.get(user.id, [])), + }) + return {"users": rows} + + +@router.get("/tree") +def access_tree(db: Session = Depends(get_db), current_user: User = Depends(require_moderator)): + """What there is to grant: the category tree, and the image libraries.""" + direct = dict(db.query(Question.question_category_id, func.count(Question.id)) + .filter(Question.question_category_id.isnot(None)) + .group_by(Question.question_category_id).all()) + articles = dict(db.query(Article.category_id, func.count(Article.id)) + .filter(Article.category_id.isnot(None)) + .group_by(Article.category_id).all()) + categories = [{ + "id": cat.id, + "name": cat.name, + "parent_id": cat.parent_id, + "questions": direct.get(cat.id, 0), + "articles": articles.get(cat.id, 0), + } for cat in db.query(QuestionCategory).order_by(QuestionCategory.name).all()] + + assets = dict(db.query(MediaAsset.library_id, func.count(MediaAsset.id)) + .filter(MediaAsset.library_id.isnot(None)) + .group_by(MediaAsset.library_id).all()) + libraries = [{"id": lib.id, "name": lib.name, "assets": assets.get(lib.id, 0)} + for lib in db.query(MediaLibrary).order_by(MediaLibrary.name).all()] + return {"categories": categories, "libraries": libraries} + + +def _target_user(db: Session, user_id: int) -> User: + user = db.get(User, user_id) + if not user: + raise HTTPException(404, "User not found") + return user + + +@router.post("/{user_id}/grants", status_code=201) +def add_grant(user_id: int, data: GrantWrite, db: Session = Depends(get_db), + current_user: User = Depends(require_moderator)): + user = _target_user(db, user_id) + if user.is_moderator: + raise HTTPException(400, "This person already has everything; a grant would add nothing") + + if data.kind == "category": + if not db.get(QuestionCategory, data.target_id): + raise HTTPException(404, "Category not found") + if db.query(CategoryGrant.id).filter_by(category_id=data.target_id, user_id=user_id).first(): + raise HTTPException(409, "Already granted") + db.add(CategoryGrant(category_id=data.target_id, user_id=user_id, granted_by=current_user.id)) + elif data.kind == "library": + if not db.get(MediaLibrary, data.target_id): + raise HTTPException(404, "Library not found") + if db.query(MediaLibraryGrant.id).filter_by(library_id=data.target_id, user_id=user_id).first(): + raise HTTPException(409, "Already granted") + db.add(MediaLibraryGrant(library_id=data.target_id, user_id=user_id, granted_by=current_user.id)) + else: + raise HTTPException(400, "kind must be category or library") + db.commit() + return {"granted": data.kind, "target_id": data.target_id, "user_id": user_id} + + +@router.delete("/{user_id}/grants/{kind}/{target_id}", status_code=204) +def remove_grant(user_id: int, kind: str, target_id: int, db: Session = Depends(get_db), + current_user: User = Depends(require_moderator)): + model = {"category": CategoryGrant, "library": MediaLibraryGrant}.get(kind) + if model is None: + raise HTTPException(400, "kind must be category or library") + column = "category_id" if kind == "category" else "library_id" + grant = db.query(model).filter_by(user_id=user_id, **{column: target_id}).first() + if not grant: + raise HTTPException(404, "Grant not found") + db.delete(grant) + db.commit() + + +@router.put("/{user_id}/role") +def set_role(user_id: int, data: RoleWrite, db: Session = Depends(get_db), + current_user: User = Depends(require_moderator)): + """Give or take away "everything". + + This is a role change, not a grant, and the two are kept distinct on + purpose: a wildcard grant that silently meant everything would be the kind + of permission nobody can audit. + """ + if data.role not in ("user", "moderator"): + raise HTTPException(400, "role must be user or moderator") + user = _target_user(db, user_id) + if user.role == "admin": + raise HTTPException(400, "An administrator's role is not changed here") + if user.id == current_user.id: + raise HTTPException(400, "Change someone else's access, not your own") + user.role = data.role + db.commit() + return _user_json(user) diff --git a/backend/app/routers/articles.py b/backend/app/routers/articles.py index 3aac176..cedd2a8 100644 --- a/backend/app/routers/articles.py +++ b/backend/app/routers/articles.py @@ -21,6 +21,7 @@ from app.services.quiz_builder import bank_question_predicate from app.services.quiz_builder import category_breadcrumbs from app.models.question_category import QuestionCategory from app.utils.auth import get_current_user, require_moderator +from app.utils.category_grants import can_edit_article import logging from datetime import datetime @@ -506,7 +507,7 @@ def update_article( article = db.get(Article, article_id) if not article: raise HTTPException(404, "Article not found") - if not (current_user.is_moderator or article.user_id == current_user.id): + if not can_edit_article(db, current_user, article): raise HTTPException(403, "Not your article") _validate_sections(data.sections) if data.category_id and not db.get(QuestionCategory, data.category_id): @@ -557,7 +558,7 @@ def delete_article( article = db.get(Article, article_id) if not article: raise HTTPException(404, "Article not found") - if not (current_user.is_moderator or article.user_id == current_user.id): + if not can_edit_article(db, current_user, article): raise HTTPException(403, "Not your article") db.delete(article) db.commit() @@ -737,7 +738,7 @@ def list_revisions(article_id: int, db: Session = Depends(get_db), article = db.get(Article, article_id) if not article: raise HTTPException(404, "Article not found") - if not (current_user.is_moderator or article.user_id == current_user.id): + if not can_edit_article(db, current_user, article): raise HTTPException(403, "Not your article") rows = db.query(ArticleRevision).filter( ArticleRevision.article_id == article_id).order_by(ArticleRevision.id.desc()).limit(30).all() diff --git a/backend/app/utils/category_grants.py b/backend/app/utils/category_grants.py index 0582710..d1ed2e0 100644 --- a/backend/app/utils/category_grants.py +++ b/backend/app/utils/category_grants.py @@ -38,6 +38,34 @@ def _descendants(db: Session, roots: set[int]) -> set[int]: +def granted_category_scope(db: Session, user: User) -> set[int] | None: + """Categories this user's grants cover, including everything beneath them. + + None means every category — a moderator. An empty set means none. + """ + if user.is_moderator: + return None + roots = {row[0] for row in db.query(CategoryGrant.category_id).filter( + CategoryGrant.user_id == user.id, CategoryGrant.category_id.isnot(None)).all()} + return _descendants(db, roots) + + +def can_edit_article(db: Session, user: User, article) -> bool: + """Whether this user may change this article. + + A moderator may, and so may its author. Beyond that, an article is filed + under a category, so a grant over that branch covers the reading in it as + well as the questions — otherwise an educator given a branch can edit its + questions and not the article they are meant to be read with. + """ + if user.is_moderator or article.user_id == user.id: + return True + if article.category_id is None: + return False + scope = granted_category_scope(db, user) + return scope is not None and article.category_id in scope + + def question_scope_predicate(db: Session, user: User): """A filter over Question covering everything this user may edit, or None for all. diff --git a/backend/tests/test_access.py b/backend/tests/test_access.py new file mode 100644 index 0000000..386588b --- /dev/null +++ b/backend/tests/test_access.py @@ -0,0 +1,143 @@ +"""Who may edit what, read and written in one place. + +Disposable SQLite. The interesting cases are the ones a permissions screen +gets wrong: a grant that silently means everything, a checkbox that changes +nothing because a parent already covers it, and an article an educator can +see but not edit in a branch they were given. +""" +import sys +import unittest +from types import ModuleType +from unittest.mock import patch + +import test_quiz_builder as fixtures +from app.models.article import Article +from app.models.category_grant import CategoryGrant +from app.models.media import MediaLibrary, MediaLibraryGrant +from app.routers import access +from app.utils.category_grants import can_edit_article, granted_category_scope + +with patch.dict(sys.modules, {"app.services.quiz_service": ModuleType("app.services.quiz_service")}): + pass + + +class AccessTests(unittest.TestCase): + def setUp(self): + self.bank = fixtures.BuilderTests() + self.bank.setUp() + self.client = self.bank.client + self.db = self.bank.db + self.client.app.include_router(access.router, prefix="/access") + self.db.add(MediaLibrary(id=1, name="Rashes")) + self.db.add(MediaLibrary(id=2, name="ECGs")) + # Categories from the fixture: 1 Root > 2 Child > 3 Leaf, and 4 Empty. + self.db.add(Article(id=50, slug="root-reading", title="Root reading", sections=[], + category_id=2, user_id=3, status="published")) + self.db.add(Article(id=51, slug="elsewhere", title="Elsewhere", sections=[], + category_id=4, user_id=3, status="published")) + self.db.commit() + self.bank.user = self.bank.mod + + def tearDown(self): + self.bank.tearDown() + + def peer(self): + return self.bank.peer + + # ── reading ─────────────────────────────────────────────────────────────── + + def test_the_overview_says_what_each_person_actually_reaches(self): + self.db.add(CategoryGrant(category_id=1, user_id=self.peer().id)) + self.db.add(MediaLibraryGrant(library_id=1, user_id=self.peer().id)) + self.db.commit() + rows = {row["id"]: row for row in self.client.get("/access/").json()["users"]} + row = rows[self.peer().id] + self.assertEqual(row["categories"], [1]) + self.assertEqual(row["libraries"], [1]) + # Root, Child and Leaf — the branch, not just the row that was ticked. + self.assertEqual(row["categories_covered"], 3) + self.assertFalse(row["everything"]) + self.assertTrue(rows[self.bank.mod.id]["everything"]) + + def test_the_tree_carries_what_is_in_each_branch(self): + tree = self.client.get("/access/tree").json() + by_id = {c["id"]: c for c in tree["categories"]} + self.assertEqual(by_id[2]["articles"], 1) + self.assertEqual({lib["name"] for lib in tree["libraries"]}, {"Rashes", "ECGs"}) + + def test_only_a_moderator_may_look(self): + self.bank.user = self.peer() + self.assertEqual(self.client.get("/access/").status_code, 403) + self.assertEqual(self.client.get("/access/tree").status_code, 403) + + # ── writing ─────────────────────────────────────────────────────────────── + + def test_granting_a_branch_and_taking_it_back(self): + uid = self.peer().id + response = self.client.post(f"/access/{uid}/grants", json={"kind": "category", "target_id": 1}) + self.assertEqual(response.status_code, 201, response.text) + self.assertEqual(self.db.query(CategoryGrant).filter_by(user_id=uid).count(), 1) + # Twice is refused rather than duplicated. + self.assertEqual(self.client.post(f"/access/{uid}/grants", + json={"kind": "category", "target_id": 1}).status_code, 409) + self.assertEqual(self.client.delete(f"/access/{uid}/grants/category/1").status_code, 204) + self.assertEqual(self.db.query(CategoryGrant).filter_by(user_id=uid).count(), 0) + + def test_a_grant_to_someone_who_already_has_everything_is_refused(self): + response = self.client.post(f"/access/{self.bank.mod.id}/grants", + json={"kind": "category", "target_id": 1}) + self.assertEqual(response.status_code, 400) + self.assertIn("already has everything", response.json()["detail"]) + + def test_everything_is_a_role_change_and_is_named_as_one(self): + uid = self.peer().id + response = self.client.put(f"/access/{uid}/role", json={"role": "moderator"}) + self.assertEqual(response.status_code, 200, response.text) + self.db.expire_all() + self.assertEqual(self.db.get(type(self.peer()), uid).role, "moderator") + self.assertTrue({r["id"]: r for r in self.client.get("/access/").json()["users"]}[uid]["everything"]) + + def test_you_cannot_change_your_own_access(self): + self.assertEqual( + self.client.put(f"/access/{self.bank.mod.id}/role", json={"role": "user"}).status_code, 400) + + def test_an_unknown_kind_is_refused_rather_than_ignored(self): + uid = self.peer().id + self.assertEqual(self.client.post(f"/access/{uid}/grants", + json={"kind": "everything", "target_id": 1}).status_code, 400) + self.assertEqual(self.client.delete(f"/access/{uid}/grants/everything/1").status_code, 400) + + # ── what a grant actually buys ──────────────────────────────────────────── + + def test_a_branch_covers_the_articles_filed_under_it(self): + peer = self.peer() + root = self.db.get(Article, 50) # category 2, inside the branch + far = self.db.get(Article, 51) # category 4, outside it + self.assertFalse(can_edit_article(self.db, peer, root)) + + self.db.add(CategoryGrant(category_id=1, user_id=peer.id)) + self.db.commit() + self.assertEqual(granted_category_scope(self.db, peer), {1, 2, 3}) + # The reading in the branch, but not the reading outside it. + self.assertTrue(can_edit_article(self.db, peer, root)) + self.assertFalse(can_edit_article(self.db, peer, far)) + + def test_an_author_keeps_their_own_article_and_a_moderator_has_all_of_them(self): + own = Article(id=52, slug="mine", title="Mine", sections=[], category_id=4, + user_id=self.peer().id, status="draft") + self.db.add(own) + self.db.commit() + self.assertTrue(can_edit_article(self.db, self.peer(), own)) + self.assertTrue(can_edit_article(self.db, self.bank.mod, self.db.get(Article, 51))) + + def test_an_article_filed_nowhere_is_not_reachable_by_any_branch(self): + loose = Article(id=53, slug="loose", title="Loose", sections=[], category_id=None, + user_id=3, status="published") + self.db.add(loose) + self.db.add(CategoryGrant(category_id=1, user_id=self.peer().id)) + self.db.commit() + self.assertFalse(can_edit_article(self.db, self.peer(), loose)) + + +if __name__ == "__main__": + unittest.main() diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index ec13ce1..960c809 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -37,6 +37,7 @@ const AnalysisSessionPage = lazy(() => import('./pages/AnalysisSessionPage')) const AiModePage = lazy(() => import('./pages/AiModePage')) const MediaPage = lazy(() => import('./pages/MediaPage')) const EditorialPage = lazy(() => import('./pages/EditorialPage')) +const AccessPage = lazy(() => import('./pages/AccessPage')) const StudyPlansPage = lazy(() => import('./pages/StudyPlansPage')) const StudyPlanPage = lazy(() => import('./pages/StudyPlanPage')) const StudyPlanBlockPage = lazy(() => import('./pages/StudyPlanBlockPage')) @@ -165,6 +166,7 @@ function AppRoutes() { } /> } /> } /> + } /> } /> } /> diff --git a/frontend/src/pages/AccessPage.css b/frontend/src/pages/AccessPage.css new file mode 100644 index 0000000..65f9a34 --- /dev/null +++ b/frontend/src/pages/AccessPage.css @@ -0,0 +1,85 @@ +/* Who may edit what. People down the left, one person's access on the right. */ + +.ac-page { max-width: 1180px; margin: 0 auto; padding-bottom: 48px; } + +.ac-head { display: flex; justify-content: space-between; align-items: flex-start; gap: 16px; flex-wrap: wrap; margin-bottom: 16px; } +.ac-head h1 { margin: 0 0 4px; font-size: 1.4rem; font-weight: 700; } +.ac-head p { margin: 0; max-width: 62ch; font-size: 0.86rem; line-height: 1.6; color: var(--text-muted); } +.ac-error { margin: 0 0 14px; padding: 10px 13px; font-size: 0.85rem; color: var(--wrong-fg); background: var(--wrong-bg); border: 1px solid var(--wrong-bd); border-radius: 8px; } + +.ac-body { display: grid; grid-template-columns: 260px minmax(0, 1fr); gap: 20px; align-items: start; } + +/* ── People ───────────────────────────────────────────────────────── */ +.ac-people { background: var(--card-bg); border: 1px solid var(--border); border-radius: 12px; overflow: hidden; position: sticky; top: 76px; } +.ac-search { + width: calc(100% - 20px); margin: 10px; padding: 8px 11px; + /* 16px on touch: iOS zooms in on anything smaller and never zooms back. */ + font-size: 16px; font-family: inherit; + border: 1px solid var(--border); border-radius: 8px; background: var(--input-bg); color: var(--text); +} +@media (min-width: 700px) { .ac-search { font-size: 0.85rem; } } +.ac-people ul { list-style: none; margin: 0; padding: 0; max-height: 60vh; overflow-y: auto; } +.ac-people li + li { border-top: 1px solid var(--border); } +.ac-people button { + display: flex; flex-direction: column; gap: 3px; width: 100%; min-height: 44px; + padding: 10px 14px; text-align: left; font: inherit; + background: none; border: 0; cursor: pointer; color: var(--text); +} +.ac-people button:hover { background: var(--bg); } +.ac-people button.is-active { background: var(--option-sel-bg); box-shadow: inset 3px 0 0 var(--primary); } +.ac-person-name { font-size: 0.9rem; font-weight: 600; } +.ac-people button.is-active .ac-person-name { color: var(--primary); } +.ac-person-meta { font-size: 0.76rem; color: var(--text-muted); } + +/* ── One person ───────────────────────────────────────────────────── */ +.ac-detail { min-width: 0; background: var(--card-bg); border: 1px solid var(--border); border-radius: 12px; } +.ac-person-head { padding: 16px 20px; border-bottom: 1px solid var(--border); } +.ac-person-head h2 { margin: 0; font-size: 1.05rem; font-weight: 650; } +.ac-person-head span { font-size: 0.82rem; color: var(--text-muted); } + +.ac-section { padding: 16px 20px; } +.ac-section + .ac-section { border-top: 1px solid var(--border); } +.ac-section h3 { + display: flex; align-items: baseline; gap: 10px; flex-wrap: wrap; + margin: 0 0 12px; font-size: 0.7rem; font-weight: 700; letter-spacing: 0.07em; + text-transform: uppercase; color: var(--text-subtle); +} +.ac-section h3 small { font-size: 0.72rem; font-weight: 500; letter-spacing: 0; text-transform: none; color: var(--text-muted); } +.ac-note, .ac-empty { margin: 0; padding: 14px 20px; font-size: 0.86rem; line-height: 1.6; color: var(--text-muted); } +.ac-section .ac-empty { padding: 0; } + +.ac-everything { display: flex; gap: 12px; align-items: flex-start; cursor: pointer; } +.ac-everything input { width: 18px; height: 18px; margin-top: 2px; flex-shrink: 0; } +.ac-everything strong { display: block; font-size: 0.92rem; margin-bottom: 3px; } +.ac-everything small { display: block; font-size: 0.8rem; line-height: 1.6; color: var(--text-muted); } + +/* ── Tree ─────────────────────────────────────────────────────────── */ +.ac-tree, .ac-tree ul { list-style: none; margin: 0; padding: 0; } +.ac-row { display: flex; align-items: center; gap: 8px; min-height: 38px; padding: 2px 6px; border-radius: 7px; } +.ac-row:hover { background: var(--bg); } +.ac-twisty { + flex-shrink: 0; width: 22px; height: 22px; display: inline-flex; align-items: center; justify-content: center; + background: none; border: 0; cursor: pointer; color: var(--text-muted); font-size: 0.72rem; +} +.ac-label { display: flex; align-items: center; gap: 9px; flex: 1; min-width: 0; cursor: pointer; } +.ac-label input { width: 17px; height: 17px; flex-shrink: 0; } +.ac-label input:disabled { cursor: default; } +.ac-name { font-size: 0.88rem; overflow-wrap: anywhere; } +.ac-node.is-granted > .ac-row .ac-name { font-weight: 650; color: var(--primary); } +.ac-node.is-covered > .ac-row .ac-name { color: var(--text-muted); } + +.ac-counts { display: flex; gap: 10px; flex-shrink: 0; font-size: 0.74rem; color: var(--text-subtle); } +.ac-inherited { font-style: italic; } + +.ac-libraries { list-style: none; margin: 0; padding: 0; display: flex; flex-direction: column; gap: 2px; } +.ac-libraries label { display: flex; align-items: center; gap: 9px; min-height: 40px; padding: 2px 6px; border-radius: 7px; cursor: pointer; } +.ac-libraries label:hover { background: var(--bg); } +.ac-libraries input { width: 17px; height: 17px; } +.ac-libraries .ac-counts { margin-left: auto; } + +@media (max-width: 860px) { + .ac-body { grid-template-columns: minmax(0, 1fr); } + .ac-people { position: static; } + .ac-people ul { max-height: 240px; } + .ac-counts { display: none; } +} diff --git a/frontend/src/pages/AccessPage.jsx b/frontend/src/pages/AccessPage.jsx new file mode 100644 index 0000000..ae0fa32 --- /dev/null +++ b/frontend/src/pages/AccessPage.jsx @@ -0,0 +1,266 @@ +import { useCallback, useEffect, useMemo, useState } from 'react' +import { Link } from 'react-router-dom' +import api from '../api/client' +import { useAuth } from '../context/AuthContext' +import './AccessPage.css' + +const apiError = (err, fallback) => { + const detail = err?.response?.data?.detail + return typeof detail === 'string' ? detail : fallback +} + +/** Children by parent id, so the tree can be walked without repeated scans. */ +function buildTree(categories) { + const byParent = new Map() + for (const cat of categories) { + const key = cat.parent_id ?? 0 + if (!byParent.has(key)) byParent.set(key, []) + byParent.get(key).push(cat) + } + return byParent +} + +/** + * One branch of the category tree. + * + * A branch that is granted shows its children as covered rather than as + * separately grantable: the grant already reaches them, and offering a + * checkbox that changes nothing is how a permissions screen starts lying. + */ +function Branch({ node, byParent, granted, covered, onToggle, busy, depth = 0 }) { + const [open, setOpen] = useState(depth === 0) + const children = byParent.get(node.id) || [] + const isGranted = granted.has(node.id) + const isCovered = !isGranted && covered.has(node.id) + + return ( +
  • +
    + {children.length > 0 ? ( + + ) :
    + + {open && children.length > 0 && ( +
      + {children.map(child => ( + + ))} +
    + )} +
  • + ) +} + +/** + * Who may edit what, in one place. + * + * Access lived in three screens over two tables — category grants in the + * question manager, library grants in the image bank, nothing for articles — + * so nobody could see what one person actually held. This shows one person at + * a time and everything they have. + */ +export default function AccessPage() { + const { user: me } = useAuth() + const [rows, setRows] = useState([]) + const [tree, setTree] = useState({ categories: [], libraries: [] }) + const [selectedId, setSelectedId] = useState(null) + const [loading, setLoading] = useState(true) + const [busy, setBusy] = useState(false) + const [error, setError] = useState('') + const [query, setQuery] = useState('') + + const load = useCallback(() => { + Promise.all([api.get('/access/'), api.get('/access/tree')]) + .then(([overview, structure]) => { + setRows(overview.data.users || []) + setTree(structure.data) + }) + .catch(err => setError(apiError(err, 'Could not load access'))) + .finally(() => setLoading(false)) + }, []) + + useEffect(() => { load() }, [load]) + + const selected = rows.find(row => row.id === selectedId) || null + const byParent = useMemo(() => buildTree(tree.categories), [tree.categories]) + const roots = byParent.get(0) || [] + + const granted = useMemo(() => new Set(selected?.categories || []), [selected]) + // What the grants reach without being named: the descendants of each. + const covered = useMemo(() => { + const out = new Set() + const walk = (id) => { + for (const child of byParent.get(id) || []) { + out.add(child.id) + walk(child.id) + } + } + granted.forEach(walk) + return out + }, [granted, byParent]) + + const shown = useMemo(() => { + const needle = query.trim().toLowerCase() + if (!needle) return rows + return rows.filter(row => `${row.name} ${row.email}`.toLowerCase().includes(needle)) + }, [rows, query]) + + const change = async (fn, failure) => { + setBusy(true); setError('') + try { await fn(); load() } + catch (err) { setError(apiError(err, failure)) } + finally { setBusy(false) } + } + + const toggleGrant = (kind, targetId, on) => change( + () => (on + ? api.post(`/access/${selectedId}/grants`, { kind, target_id: targetId }) + : api.delete(`/access/${selectedId}/grants/${kind}/${targetId}`)), + on ? 'Could not grant that' : 'Could not remove that') + + const setRole = (role) => change( + () => api.put(`/access/${selectedId}/role`, { role }), 'Could not change that') + + if (loading) return
    + + return ( +
    +
    +
    +

    Access

    +

    + What each person may edit. A branch covers everything beneath it — its + questions and the articles filed under them — and image libraries are + granted one at a time. +

    +
    + Back to settings +
    + + {error &&

    {error}

    } + +
    + + +
    + {!selected ? ( +

    Choose someone to see what they can edit.

    + ) : ( + <> +
    +

    {selected.name}

    + {selected.email} +
    + + {selected.role === 'admin' ? ( +

    + An administrator already manages everything, and that is not changed here. +

    + ) : ( +
    +

    Everything

    + +
    + )} + + {!selected.everything && selected.role !== 'admin' && ( + <> +
    +

    + Question and article branches + {selected.categories_covered > 0 && ( + {selected.categories_covered} categories reached + )} +

    + {roots.length === 0 ?

    No categories yet.

    : ( +
      + {roots.map(node => ( + toggleGrant('category', id, on)} /> + ))} +
    + )} +
    + +
    +

    Image libraries

    + {tree.libraries.length === 0 ?

    No libraries yet.

    : ( +
      + {tree.libraries.map(library => ( +
    • + +
    • + ))} +
    + )} +
    + + )} + + )} +
    +
    +
    + ) +} diff --git a/frontend/src/pages/AccessPage.test.jsx b/frontend/src/pages/AccessPage.test.jsx new file mode 100644 index 0000000..c9c259b --- /dev/null +++ b/frontend/src/pages/AccessPage.test.jsx @@ -0,0 +1,124 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { render, screen, waitFor, within } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { MemoryRouter } from 'react-router-dom' +import AccessPage from './AccessPage' +import api from '../api/client' + +vi.mock('../api/client', () => ({ default: { get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn() } })) +vi.mock('../context/AuthContext', () => ({ useAuth: () => ({ user: { id: 9, name: 'Mod', role: 'moderator' } }) })) + +const TREE = { + categories: [ + { id: 1, name: 'Cardiology', parent_id: null, questions: 0, articles: 1 }, + { id: 2, name: 'Kawasaki disease', parent_id: 1, questions: 40, articles: 1 }, + { id: 3, name: 'Neurology', parent_id: null, questions: 12, articles: 0 }, + ], + libraries: [{ id: 1, name: 'Rashes', assets: 30 }], +} + +const users = (over = {}) => ({ + users: [ + { id: 5, name: 'Ada', email: 'ada@example.test', role: 'user', everything: false, + categories: [], categories_covered: 0, libraries: [], ...over }, + { id: 9, name: 'Mod', email: 'mod@example.test', role: 'moderator', everything: true, + categories: [], categories_covered: 0, libraries: [] }, + ], +}) + +const mock = (overview = users()) => api.get.mockImplementation(url => { + if (url === '/access/') return Promise.resolve({ data: overview }) + if (url === '/access/tree') return Promise.resolve({ data: TREE }) + return Promise.resolve({ data: {} }) +}) + +const mount = () => render() +const pick = async (name) => userEvent.click(await screen.findByRole('button', { name: new RegExp(name) })) + +describe('access', () => { + beforeEach(() => { vi.clearAllMocks(); mock() }) + + it('summarises what each person holds before you open them', async () => { + mock(users({ categories: [1], categories_covered: 2, libraries: [1] })) + mount() + expect(await screen.findByText('1 branch, 1 library')).toBeInTheDocument() + expect(screen.getByText('Everything')).toBeInTheDocument() + }) + + it('says "No access" rather than leaving it blank', async () => { + mount() + expect(await screen.findByText('No access')).toBeInTheDocument() + }) + + it('grants a branch', async () => { + mount() + await pick('Ada') + api.post.mockResolvedValue({ data: {} }) + await userEvent.click(screen.getByRole('checkbox', { name: 'Grant Cardiology' })) + await waitFor(() => expect(api.post).toHaveBeenCalledWith( + '/access/5/grants', { kind: 'category', target_id: 1 })) + }) + + it('shows a child as covered by its parent, and will not let it be ticked separately', async () => { + mock(users({ categories: [1], categories_covered: 2 })) + mount() + await pick('Ada') + const child = screen.getByRole('checkbox', { name: 'Grant Kawasaki disease' }) + expect(child).toBeChecked() + expect(child).toBeDisabled() + expect(screen.getByText('covered by a parent')).toBeInTheDocument() + // A sibling outside the branch is still free to grant. + expect(screen.getByRole('checkbox', { name: 'Grant Neurology' })).toBeEnabled() + }) + + it('removes a branch that was granted', async () => { + mock(users({ categories: [3], categories_covered: 1 })) + mount() + await pick('Ada') + api.delete.mockResolvedValue({ data: {} }) + await userEvent.click(screen.getByRole('checkbox', { name: 'Grant Neurology' })) + await waitFor(() => expect(api.delete).toHaveBeenCalledWith('/access/5/grants/category/3')) + }) + + it('grants an image library on its own', async () => { + mount() + await pick('Ada') + api.post.mockResolvedValue({ data: {} }) + await userEvent.click(screen.getByRole('checkbox', { name: 'Grant Rashes' })) + await waitFor(() => expect(api.post).toHaveBeenCalledWith( + '/access/5/grants', { kind: 'library', target_id: 1 })) + }) + + it('treats "everything" as a role change, not a grant', async () => { + mount() + await pick('Ada') + api.put.mockResolvedValue({ data: {} }) + await userEvent.click(screen.getByRole('checkbox', { name: 'Give access to everything' })) + await waitFor(() => expect(api.put).toHaveBeenCalledWith('/access/5/role', { role: 'moderator' })) + // Not a grant: nothing was written to the grants table. + expect(api.post).not.toHaveBeenCalled() + }) + + it('hides the branches once someone has everything, because they no longer apply', async () => { + mock(users({ everything: true, role: 'moderator' })) + mount() + await pick('Ada') + await waitFor(() => expect(screen.getByRole('checkbox', { name: 'Give access to everything' })).toBeChecked()) + expect(screen.queryByRole('checkbox', { name: 'Grant Cardiology' })).not.toBeInTheDocument() + expect(screen.queryByRole('checkbox', { name: 'Grant Rashes' })).not.toBeInTheDocument() + }) + + it('will not let a moderator change their own access', async () => { + mount() + await pick('Mod') + expect(screen.getByRole('checkbox', { name: 'Give access to everything' })).toBeDisabled() + }) + + it('surfaces a refusal instead of silently doing nothing', async () => { + mount() + await pick('Ada') + api.post.mockRejectedValue({ response: { data: { detail: 'Already granted' } } }) + await userEvent.click(screen.getByRole('checkbox', { name: 'Grant Neurology' })) + expect(await screen.findByRole('alert')).toHaveTextContent('Already granted') + }) +}) diff --git a/frontend/src/pages/ArticleSplitView.test.jsx b/frontend/src/pages/ArticleSplitView.test.jsx index b9c5218..b412451 100644 --- a/frontend/src/pages/ArticleSplitView.test.jsx +++ b/frontend/src/pages/ArticleSplitView.test.jsx @@ -44,13 +44,15 @@ describe('reading a cross-reference beside the article', () => { }) }) - // The preview card appears on a 350ms timer (ArticleLink.HOVER_DELAY). Under - // the full parallel run the box is busy enough that the old 2s allowance ran - // out, and a different case failed each time. The wait is generous because - // this test is about the split view, not about how fast the machine is. + // Focus, not hover. Hovering starts a 350ms timer before the card appears + // (ArticleLink.HOVER_DELAY) and under the full parallel run the box was busy + // enough to outlast the query, failing a different case each time. Focus + // reveals the card immediately — it is a deliberate act, so the component + // does not make a keyboard reader wait — which takes the wall clock out of a + // test that is about the split view, not about how fast the machine is. const openSplit = async (name) => { - await userEvent.hover(screen.getByRole('link', { name })) - const card = await screen.findByRole('tooltip', {}, { timeout: 8000 }) + screen.getByRole('link', { name }).focus() + const card = await screen.findByRole('tooltip') await userEvent.click(within(card).getByRole('button', { name: /split view/i })) } diff --git a/frontend/src/pages/SettingsPage.jsx b/frontend/src/pages/SettingsPage.jsx index 0b8f92e..c81e28d 100644 --- a/frontend/src/pages/SettingsPage.jsx +++ b/frontend/src/pages/SettingsPage.jsx @@ -221,6 +221,7 @@ function AdminSection() { { to: '/upload', icon: '📄', label: 'Upload PDF', desc: 'Add new documents' }, { to: '/categories', icon: '🗂️', label: 'Taxonomy', desc: 'Topics, systems, symptoms, diseases' }, { to: '/editorial', icon: '✍️', label: 'Editorial', desc: 'Article drafts and references' }, + { to: '/access', icon: '🔑', label: 'Access', desc: 'Who may edit what' }, { to: '/trash', icon: '🗑️', label: 'Trash', desc: 'Restore deleted tests' }, { to: '/jobs', icon: '📋', label: 'Extraction jobs', desc: 'Extraction history' }, ].map(item => (