diff --git a/backend/alembic/versions/q6a7b8c9d0e1_ownership_is_a_role.py b/backend/alembic/versions/q6a7b8c9d0e1_ownership_is_a_role.py new file mode 100644 index 0000000..96175ff --- /dev/null +++ b/backend/alembic/versions/q6a7b8c9d0e1_ownership_is_a_role.py @@ -0,0 +1,70 @@ +"""Bank content belongs to a role, not to a person. + +The bank was full of rows with somebody's name on them: 571 categories, 21 +uploaded documents, 14 articles, 8 card decks, the shared tests. Most of them +said daniel@danvics.com, an address that is not even the working administrator +any more — so "who may edit this" partly depended on who happened to have +created it years ago, and handing the site to somebody else would have meant +rewriting every one of those rows. + +Ownership of the bank is now the `admin` role plus CategoryGrant, and these +columns are emptied to say so. Nothing is deleted and no row moves: only the +name comes off. + +What keeps its owner, deliberately, because it is genuinely one person's: + quiz_attempts, question_notes, article_section_notes, favorites, + user_collections, question_folders, study_plan_* progress, and the quizzes + that are somebody's own sittings (is_shared = 0) rather than bank tests. + +study_plans needed nothing: it never had an owner column. + +Revision ID: q6a7b8c9d0e1 +Revises: p5f6a7b8c9d0 +""" +from alembic import op +import sqlalchemy as sa + +revision = "q6a7b8c9d0e1" +down_revision = "p5f6a7b8c9d0" +branch_labels = None +depends_on = None + +#: Everything in the bank, and the condition that picks the bank rows out of a +#: table that also holds personal ones. +BANK = [ + ("question_categories", None), + ("articles", None), + ("pdf_documents", None), + ("flashcard_decks", None), + ("questions", None), + ("media_assets", None), + ("media_libraries", None), + ("quiz_categories", None), + # A shared test is the bank's; an unshared one is a sitting of somebody's + # own and stays theirs. + ("quizzes", "is_shared = 1"), +] + + +#: Tables whose user_id was declared NOT NULL. "Ownerless" is now a legitimate +#: state for bank content, so the column has to be allowed to say so — and a +#: NOT NULL owner column is exactly the thing that forced a person's name onto +#: every row in the first place. +NULLABLE = ("question_categories", "pdf_documents", "flashcard_decks", + "quiz_categories", "quizzes") + + +def upgrade(): + for table in NULLABLE: + op.alter_column(table, "user_id", existing_type=sa.Integer(), nullable=True) + for table, where in BANK: + clause = f" AND {where}" if where else "" + op.execute(sa.text( + f"UPDATE {table} SET user_id = NULL WHERE user_id IS NOT NULL{clause}")) + + +def downgrade(): + # Deliberately not reversible. The names are not recorded anywhere once + # they are gone, and inventing an owner would be worse than admitting the + # information is not here: restore from the dump taken beside this change. + pass diff --git a/backend/app/models/flashcard.py b/backend/app/models/flashcard.py index 152f5a4..d70a4b7 100644 --- a/backend/app/models/flashcard.py +++ b/backend/app/models/flashcard.py @@ -11,7 +11,7 @@ class FlashcardDeck(Base): section_id = Column(Integer, ForeignKey("sections.id", ondelete="SET NULL"), nullable=True) # Same tree as questions and articles, so one category filters all three. category_id = Column(Integer, ForeignKey("question_categories.id", ondelete="SET NULL"), nullable=True, index=True) - user_id = Column(Integer, ForeignKey("users.id", ondelete="CASCADE"), nullable=False) + user_id = Column(Integer, ForeignKey("users.id", ondelete="CASCADE"), nullable=True) card_count = Column(Integer, default=0) is_shared = Column(Integer, default=0) # 0 = private, 1 = shared created_at = Column(DateTime, default=datetime.utcnow) diff --git a/backend/app/models/pdf_document.py b/backend/app/models/pdf_document.py index 3087dd4..5639178 100644 --- a/backend/app/models/pdf_document.py +++ b/backend/app/models/pdf_document.py @@ -10,7 +10,7 @@ class PDFDocument(Base): __tablename__ = "pdf_documents" id = Column(Integer, primary_key=True, index=True) - user_id = Column(Integer, ForeignKey("users.id", ondelete="CASCADE"), nullable=False) + user_id = Column(Integer, ForeignKey("users.id", ondelete="CASCADE"), nullable=True) filename = Column(String, nullable=False) original_filename = Column(String, nullable=False) total_pages = Column(Integer, nullable=True) diff --git a/backend/app/models/question_category.py b/backend/app/models/question_category.py index b0957bf..21a98e5 100644 --- a/backend/app/models/question_category.py +++ b/backend/app/models/question_category.py @@ -11,7 +11,7 @@ class QuestionCategory(Base): parent_id = Column(Integer, ForeignKey("question_categories.id", ondelete="RESTRICT", name="fk_question_categories_parent"), nullable=True) name = Column(String, nullable=False) description = Column(Text, nullable=True) - user_id = Column(Integer, ForeignKey("users.id"), nullable=False) + user_id = Column(Integer, ForeignKey("users.id"), nullable=True) # The organ system this topic belongs to, said once here rather than # inferred from a keyword on each question. A category's discipline is # where it sits in the tree; its system is a separate fact about it — diff --git a/backend/app/models/quiz.py b/backend/app/models/quiz.py index 20aedfb..815202f 100644 --- a/backend/app/models/quiz.py +++ b/backend/app/models/quiz.py @@ -21,7 +21,7 @@ class Quiz(Base): # Cascades: deleting an account is documented as removing what it made. # Declared here because the database has always done it, and a model that # says nothing is a model that will be believed. - user_id = Column(Integer, ForeignKey("users.id", ondelete="CASCADE"), nullable=False) + user_id = Column(Integer, ForeignKey("users.id", ondelete="CASCADE"), nullable=True) category_id = Column(Integer, ForeignKey("quiz_categories.id", ondelete="SET NULL"), nullable=True) title = Column(String, nullable=False) questions_count = Column(Integer, default=0) diff --git a/backend/app/routers/articles.py b/backend/app/routers/articles.py index 076693e..4ae8951 100644 --- a/backend/app/routers/articles.py +++ b/backend/app/routers/articles.py @@ -422,7 +422,7 @@ def create_article( slug=data.slug, title=data.title, summary=data.summary, content=data.content, sections=[section.model_dump() for section in data.sections], category_id=data.category_id, section_id=data.section_id, - user_id=current_user.id, status="draft", + user_id=None, status="draft", ) db.add(article) db.flush() @@ -483,7 +483,7 @@ def article_by_slug(slug: str, db: Session = Depends(get_db), article = article_service.resolve_slug(db, slug) if not article: raise HTTPException(404, "Article not found") - if article.status != "published" and not current_user.is_moderator and article.user_id != current_user.id: + if article.status != "published" and not current_user.is_moderator: raise HTTPException(404, "Article not found") # `moved` tells the caller to correct its URL rather than keep using the old one. return {"id": article.id, "slug": article.slug, "title": article.title, @@ -504,7 +504,7 @@ def preview_article( article = article_service.resolve_slug(db, slug) if not article: raise HTTPException(404, "Article not found") - if article.status != "published" and not current_user.is_moderator and article.user_id != current_user.id: + if article.status != "published" and not current_user.is_moderator: raise HTTPException(404, "Article not found") return { "id": article.id, @@ -568,7 +568,7 @@ def get_article( article = db.get(Article, article_id) if not article or article.deleted_at is not None: raise HTTPException(404, "Article not found") - if article.status != "published" and not current_user.is_moderator and article.user_id != current_user.id: + if article.status != "published" and not current_user.is_moderator: raise HTTPException(404, "Article not found") _record_view(db, current_user, article) data = _article_json(article) @@ -577,7 +577,7 @@ def get_article( data["sections"] = [s for s in article_service.normalise_sections(article.sections or []) if s.get("variant") in allowed] # An editor has to see the whole article to edit it; a learner does not. - if current_user.is_moderator or article.user_id == current_user.id: + if current_user.is_moderator: data["all_variants"] = article_service.available_variants(article) categories = db.query(QuestionCategory).all() data["category_breadcrumbs"] = category_breadcrumbs(categories, article.category_id) if article.category_id else [] @@ -593,7 +593,7 @@ def _readable_article(db, user, article_id: int) -> Article: article = db.get(Article, article_id) if not article: raise HTTPException(404, "Article not found") - if article.status != "published" and not user.is_moderator and article.user_id != user.id: + if article.status != "published" and not user.is_moderator: raise HTTPException(404, "Article not found") return article @@ -1120,17 +1120,15 @@ def set_article_status(article_id: int, data: StatusIn, db: Session = Depends(ge current_user: User = Depends(get_current_user)): """Move an article through draft → in review → published. - An author may send their own work for review; only a moderator may publish, - because publishing is the point at which nobody checks it again. + Editorial work, so an editor does it. It used to let an author move their + own article along, which was the last place authorship still bought + anything — and an article has no author now: it belongs to the library. """ article = db.get(Article, article_id) if not article: raise HTTPException(404, "Article not found") - is_author = article.user_id == current_user.id - if not (current_user.is_moderator or is_author): - raise HTTPException(403, "Not your article") - if data.status == "published" and not current_user.is_moderator: - raise HTTPException(403, "Only a moderator can publish an article") + if not current_user.is_moderator: + raise HTTPException(403, "Editorial work is for educators and administrators") try: article_service.set_status(db, article, data.status, current_user.id) except ValueError as error: @@ -1163,7 +1161,7 @@ def read_revision(article_id: int, revision_id: int, db: Session = Depends(get_d if not revision or revision.article_id != article_id: raise HTTPException(404, "Revision not found") article = db.get(Article, article_id) - if not (current_user.is_moderator or (article and article.user_id == current_user.id)): + if not current_user.is_moderator: raise HTTPException(403, "Not your article") return {"id": revision.id, "title": revision.title, "summary": revision.summary, "content": revision.content, "sections": revision.sections, diff --git a/backend/app/routers/categories.py b/backend/app/routers/categories.py index 96d19e4..c568384 100644 --- a/backend/app/routers/categories.py +++ b/backend/app/routers/categories.py @@ -47,7 +47,7 @@ def create_category( existing = db.query(QuizCategory).filter(QuizCategory.name == data.name.strip()).first() if existing: raise HTTPException(status_code=400, detail="Category already exists") - cat = QuizCategory(name=data.name.strip(), user_id=current_user.id) + cat = QuizCategory(name=data.name.strip(), user_id=None) db.add(cat) db.commit() db.refresh(cat) diff --git a/backend/app/routers/documents.py b/backend/app/routers/documents.py index 34546cd..5d3099f 100644 --- a/backend/app/routers/documents.py +++ b/backend/app/routers/documents.py @@ -42,7 +42,10 @@ def upload_document( # Create DB record doc = PDFDocument( - user_id=current_user.id, + # No owner. Bank content belongs to the admin role and to whoever + # holds a grant over it, never to whoever happened to create it — + # otherwise ownership grows back one upload at a time. + user_id=None, filename=safe_name, original_filename=file.filename, status="processing", @@ -83,19 +86,19 @@ def list_documents( db: Session = Depends(get_db), current_user: User = Depends(get_current_user), ): - if current_user.is_moderator: - docs = db.query(PDFDocument).order_by(PDFDocument.uploaded_at.desc()).all() - else: - docs = db.query(PDFDocument).filter(PDFDocument.user_id == current_user.id).order_by(PDFDocument.uploaded_at.desc()).all() - return docs + # An uploaded source belongs to the bank, not to whoever carried it in, so + # there is no "my documents" any more — only the corpus, and only for the + # people who work on it. + if not current_user.is_moderator: + return [] + return db.query(PDFDocument).order_by(PDFDocument.uploaded_at.desc()).all() def _get_doc_or_404(document_id: int, current_user: User, db) -> PDFDocument: - """Fetch document; moderators can access any, users only their own.""" - query = db.query(PDFDocument).filter(PDFDocument.id == document_id) + """Fetch a document. The corpus is the bank's, so this is editors only.""" if not current_user.is_moderator: - query = query.filter(PDFDocument.user_id == current_user.id) - doc = query.first() + raise HTTPException(status_code=404, detail="Document not found") + doc = db.query(PDFDocument).filter(PDFDocument.id == document_id).first() if not doc: raise HTTPException(status_code=404, detail="Document not found") return doc diff --git a/backend/app/routers/flashcards.py b/backend/app/routers/flashcards.py index 7c8caf6..1fe79f5 100644 --- a/backend/app/routers/flashcards.py +++ b/backend/app/routers/flashcards.py @@ -98,12 +98,19 @@ class CardEdit(BaseModel): image_path: str | None = None -def _own_deck_or_404(deck_id: int, current_user: User, db: Session) -> FlashcardDeck: +def _deck_or_404(deck_id: int, current_user: User, db: Session) -> FlashcardDeck: + """A deck, for somebody who works on decks. + + A deck used to be its creator's, and access turned on that. Decks are + study material the whole site reads, so they belong to the bank: an + educator or an admin reaches any of them, and nobody else reaches one + this way. Studying a shared deck goes through the shared routes. + """ deck = db.query(FlashcardDeck).filter(FlashcardDeck.id == deck_id).first() if not deck: raise HTTPException(status_code=404, detail="Deck not found") - if deck.user_id != current_user.id and not current_user.is_admin: - raise HTTPException(status_code=403, detail="Not your deck") + if not current_user.is_moderator: + raise HTTPException(status_code=404, detail="Deck not found") return deck @@ -117,7 +124,7 @@ def _own_deck_to_edit(deck_id: int, current_user: User, db: Session) -> Flashcar """ if not current_user.is_moderator: raise HTTPException(status_code=403, detail="Writing cards needs educator access") - return _own_deck_or_404(deck_id, current_user, db) + return _deck_or_404(deck_id, current_user, db) # ── Deck endpoints ─────────────────────────────────────────────────── @@ -199,8 +206,14 @@ def list_flashcard_decks( db: Session = Depends(get_db), current_user: User = Depends(get_current_user), ): - """List flashcard decks belonging to current user, optionally by category.""" - q = db.query(FlashcardDeck).filter(FlashcardDeck.user_id == current_user.id) + """The bank's decks, optionally by category. Educators only. + + There is no "my decks" any more — a deck is not anybody's. A learner + studies what has been shared with them, which is the /shared listing. + """ + if not current_user.is_moderator: + return [] + q = db.query(FlashcardDeck) if not include_deleted: q = q.filter(FlashcardDeck.deleted_at.is_(None)) if category_id is not None: @@ -215,9 +228,10 @@ def list_trashed_decks( db: Session = Depends(get_db), current_user: User = Depends(get_current_user), ): - """List soft-deleted decks for current user's trash.""" + """Decks in the bin. Editors' business, like the decks themselves.""" + if not current_user.is_moderator: + return [] return db.query(FlashcardDeck).filter( - FlashcardDeck.user_id == current_user.id, FlashcardDeck.deleted_at.isnot(None), ).order_by(FlashcardDeck.deleted_at.desc()).all() @@ -261,18 +275,15 @@ def list_shared_decks( FlashcardDeck.created_at.desc(), ).offset(offset).limit(limit).all() - user_cache: dict[int, str] = {} result = [] for deck, avg_r, r_count in decks_with_rating: - if deck.user_id not in user_cache: - owner = db.query(User).filter(User.id == deck.user_id).first() - user_cache[deck.user_id] = owner.name if owner else "Unknown" result.append({ "id": deck.id, "title": deck.title, "card_count": deck.card_count, - "user_id": deck.user_id, - "owner_name": user_cache[deck.user_id], + # No owner to name. A shared deck is the library's. + "user_id": None, + "owner_name": "PedsHub", "is_shared": deck.is_shared, "created_at": deck.created_at, "avg_rating": round(float(avg_r), 1), @@ -293,7 +304,7 @@ def get_flashcard_deck( deck = db.query(FlashcardDeck).filter(FlashcardDeck.id == deck_id).first() if not deck: raise HTTPException(status_code=404, detail="Deck not found") - if deck.user_id != current_user.id and not current_user.is_admin and not deck.is_shared: + if not deck.is_shared and not current_user.is_moderator: raise HTTPException(status_code=403, detail="Not your deck") return deck @@ -316,7 +327,7 @@ def study_deck(deck_id: int, db: Session = Depends(get_db), deck = db.query(FlashcardDeck).filter(FlashcardDeck.id == deck_id).first() if not deck: raise HTTPException(status_code=404, detail="Deck not found") - if deck.user_id != current_user.id and not current_user.is_admin and not deck.is_shared: + if not deck.is_shared and not current_user.is_moderator: raise HTTPException(status_code=403, detail="Not your deck") cards = db.query(Flashcard).filter(Flashcard.deck_id == deck_id).order_by(Flashcard.id).all() @@ -337,7 +348,7 @@ def review_card(card_id: int, data: CardVerdict, db: Session = Depends(get_db), if not card: raise HTTPException(status_code=404, detail="Card not found") deck = db.get(FlashcardDeck, card.deck_id) - if deck and deck.user_id != current_user.id and not current_user.is_admin and not deck.is_shared: + if deck and not deck.is_shared and not current_user.is_moderator: raise HTTPException(status_code=403, detail="Not your deck") card_review.record(db, current_user.id, card_id, data.outcome) return {"card_id": card_id, "outcome": data.outcome} @@ -360,7 +371,7 @@ def cards_for_question(question_id: int, db: Session = Depends(get_db), return [{"card_id": card.id, "deck_id": deck.id, "deck_title": deck.title, "front": card.front} for card, deck in rows - if deck.user_id == current_user.id or deck.is_shared or current_user.is_admin] + if deck.is_shared or current_user.is_moderator] @router.delete("/{deck_id}", status_code=204) @@ -371,7 +382,7 @@ def delete_flashcard_deck( current_user: User = Depends(get_current_user), ): """Soft-delete a deck (moves to trash). Use ?permanent=true to permanently delete.""" - deck = _own_deck_or_404(deck_id, current_user, db) + deck = _deck_or_404(deck_id, current_user, db) if permanent: db.delete(deck) else: @@ -386,7 +397,7 @@ def restore_flashcard_deck( current_user: User = Depends(get_current_user), ): """Restore a soft-deleted deck from trash.""" - deck = _own_deck_or_404(deck_id, current_user, db) + deck = _deck_or_404(deck_id, current_user, db) if not deck.deleted_at: raise HTTPException(status_code=400, detail="Deck is not in trash") deck.deleted_at = None @@ -427,7 +438,7 @@ def toggle_share_deck( current_user: User = Depends(get_current_user), ): """Toggle sharing on a deck. Owner or admin.""" - deck = _own_deck_or_404(deck_id, current_user, db) + deck = _deck_or_404(deck_id, current_user, db) deck.is_shared = 0 if deck.is_shared else 1 db.commit() return {"id": deck.id, "is_shared": deck.is_shared} @@ -503,10 +514,10 @@ def browse_flashcards( db: Session = Depends(get_db), current_user: User = Depends(get_current_user), ): - """Browse flashcards across user's own decks.""" - # Get user's active (non-deleted) deck IDs + """Browse the cards in the bank's decks. Educators; learners study shared.""" + if not current_user.is_moderator: + return {"total": 0, "cards": []} own_deck_ids = [d.id for d in db.query(FlashcardDeck.id).filter( - FlashcardDeck.user_id == current_user.id, FlashcardDeck.deleted_at.is_(None), ).all()] if not own_deck_ids: @@ -571,8 +582,9 @@ def browse_flashcard_ids( current_user: User = Depends(get_current_user), ): """Return just IDs for matching cards (for select-all).""" + if not current_user.is_moderator: + return [] own_deck_ids = [d.id for d in db.query(FlashcardDeck.id).filter( - FlashcardDeck.user_id == current_user.id, FlashcardDeck.deleted_at.is_(None), ).all()] if not own_deck_ids: @@ -619,7 +631,7 @@ def update_flashcard( if not card: raise HTTPException(status_code=404, detail="Card not found") deck = db.query(FlashcardDeck).filter(FlashcardDeck.id == card.deck_id).first() - if deck and deck.user_id != current_user.id and not current_user.is_admin: + if deck and not current_user.is_moderator: raise HTTPException(status_code=403, detail="Not your card") if data.front is not None: card.front = data.front @@ -644,7 +656,7 @@ def delete_flashcard( if not card: raise HTTPException(status_code=404, detail="Card not found") deck = db.query(FlashcardDeck).filter(FlashcardDeck.id == card.deck_id).first() - if deck and deck.user_id != current_user.id and not current_user.is_admin: + if deck and not current_user.is_moderator: raise HTTPException(status_code=403, detail="Not your card") db.delete(card) if deck and deck.card_count > 0: @@ -694,7 +706,7 @@ def _own_card_or_404(card_id: int, current_user: User, db: Session) -> Flashcard if not card: raise HTTPException(status_code=404, detail="Card not found") deck = db.get(FlashcardDeck, card.deck_id) - if deck and deck.user_id != current_user.id and not current_user.is_admin: + if deck and not current_user.is_moderator: raise HTTPException(status_code=403, detail="Not your card") return card @@ -710,7 +722,7 @@ def list_card_links( if not card: raise HTTPException(status_code=404, detail="Card not found") deck = db.get(FlashcardDeck, card.deck_id) - if deck and deck.user_id != current_user.id and not deck.is_shared and not current_user.is_admin: + if deck and not deck.is_shared and not current_user.is_moderator: raise HTTPException(status_code=403, detail="Not your card") questions = db.query(Question.id, Question.question_text) if not current_user.is_moderator: @@ -827,7 +839,7 @@ def cards_for_target( FlashcardArticleLink.article_id == article_id) cards = [] for card, deck in query.all(): - if not current_user.is_moderator and deck.user_id != current_user.id and not deck.is_shared: + if not current_user.is_moderator and not deck.is_shared: continue cards.append({"id": card.id, "deck_id": deck.id, "deck_title": deck.title, "front": card.front, "back": card.back, "image_path": card.image_path}) diff --git a/backend/app/routers/media.py b/backend/app/routers/media.py index 27c31d6..52a3835 100644 --- a/backend/app/routers/media.py +++ b/backend/app/routers/media.py @@ -123,7 +123,7 @@ def create_library(data: LibraryIn, db: Session = Depends(get_db), current_user: User = Depends(require_moderator)): if db.query(MediaLibrary.id).filter(func.lower(MediaLibrary.name) == data.name.strip().lower()).first(): raise HTTPException(409, "A library with that name already exists") - library = MediaLibrary(name=data.name.strip(), description=data.description, user_id=current_user.id) + library = MediaLibrary(name=data.name.strip(), description=data.description, user_id=None) db.add(library) db.commit() db.refresh(library) @@ -255,7 +255,7 @@ def upload_media( asset = MediaAsset( path=key, title=title or file.filename, caption=caption, alt_text=alt_text, - kind=kind, library_id=library_id, user_id=current_user.id, + kind=kind, library_id=library_id, user_id=None, storage="s3" if storage_service.using_s3() else "local", byte_size=len(data), ) db.add(asset) diff --git a/backend/app/routers/question_categories.py b/backend/app/routers/question_categories.py index 95e91eb..d132bd4 100644 --- a/backend/app/routers/question_categories.py +++ b/backend/app/routers/question_categories.py @@ -200,7 +200,7 @@ def create_question_category(data: QCatCreate, db: Session = Depends(get_db), current_user: User = Depends(require_moderator)): cats = validate_category(db, data) cat = QuestionCategory(name=data.name.strip(), description=data.description, - parent_id=data.parent_id, user_id=current_user.id) + parent_id=data.parent_id, user_id=None) db.add(cat) db.commit() db.refresh(cat) diff --git a/backend/app/routers/questions.py b/backend/app/routers/questions.py index caddf18..cf4822d 100644 --- a/backend/app/routers/questions.py +++ b/backend/app/routers/questions.py @@ -84,12 +84,10 @@ def _question_for_delete(db: Session, question_id: int, current_user: User) -> Q is_mod = current_user.role in ("admin", "moderator") scope = manageable_categories(db, current_user) granted = scope is not None and bool(scope) and question_in_scope(db, scope, question) + # Role or grant, and nothing else. "I created it" used to be a third way + # in; a question belongs to the bank, not to whoever typed it. if not is_mod and not granted: - if question.user_id != current_user.id: - raise HTTPException(status_code=403, detail="Not authorized to delete this question") - # Regular users can only delete questions they created (no source quiz) - if question.source_quiz_id is not None: - raise HTTPException(status_code=403, detail="Only moderators can delete extracted questions") + raise HTTPException(status_code=403, detail="Not authorized to delete this question") return question @@ -107,9 +105,10 @@ def list_trashed_questions( query = db.query(Question).filter(Question.deleted_at.isnot(None)) if current_user.role not in ("admin", "moderator"): scope = manageable_categories(db, current_user) - clause = Question.user_id == current_user.id - if scope: - clause = clause | Question.question_category_id.in_(scope) + # What your grants cover. Nothing is yours by authorship any more, so + # an educator with no grant sees an empty bin rather than the handful + # of rows that happened to carry their id. + clause = Question.question_category_id.in_(scope) if scope else Question.id.is_(None) query = query.filter(clause) rows = query.order_by(Question.deleted_at.desc()).limit(min(limit, 500)).all() categories = { @@ -330,7 +329,6 @@ def get_question_bank( needs: Literal["category", "explanation", "difficulty", "private"] | None = Query( None, description="Editorial gap filter used by the question manager"), favorites_only: bool = Query(False), - my_questions: bool = Query(False, description="Show only questions created by current user"), difficulty: Literal["easy", "medium", "hard"] | None = Query(None), article_ids: str | None = Query(None, description="Comma-separated article IDs (OR filter)"), folder_id: int | None = Query(None, description="Only the questions in this folder"), @@ -345,9 +343,6 @@ def get_question_bank( exam_filter = exam_scope_predicate(db, current_user) if exam_filter is not None: query = query.filter(exam_filter) - if my_questions: - query = query.filter(Question.user_id == current_user.id) - if difficulty: query = query.filter(Question.difficulty == difficulty) if article_ids: @@ -446,8 +441,6 @@ def get_question_bank( else: editable = {row[0] for row in db.query(Question.id).filter( Question.id.in_([qu.id for qu in questions]), scope).all()} if questions else set() - editable |= {qu.id for qu in questions - if qu.user_id is not None and qu.user_id == current_user.id} link_rows = db.query(QuestionCategoryLink.question_id, QuestionCategoryLink.category_id).filter( QuestionCategoryLink.question_id.in_([qu.id for qu in questions])).all() if questions else [] extra_map: dict[int, list[int]] = {} @@ -554,7 +547,8 @@ def create_question_manually( key_points=key_points, attending_tip=(data.attending_tip or None), difficulty=data.difficulty, - user_id=current_user.id, + # Ownerless, like everything else in the bank. + user_id=None, ) db.add(question) db.commit() @@ -1094,7 +1088,6 @@ def question_manager_summary( "no_difficulty": base.filter(Question.difficulty.is_(None)).scalar() or 0, "no_explanation": base.filter( or_(Question.explanation.is_(None), Question.explanation.in_(blank))).scalar() or 0, - "mine": base.filter(Question.user_id == current_user.id).scalar() or 0, "scoped": scope is not None, } @@ -1209,7 +1202,7 @@ def upload_questions_csv( if cat_name: cat_lower = cat_name.lower() if cat_lower not in cat_cache: - new_cat = QuestionCategory(name=cat_name, user_id=current_user.id) + new_cat = QuestionCategory(name=cat_name, user_id=None) db.add(new_cat) db.flush() cat_cache[cat_lower] = new_cat.id @@ -1403,7 +1396,7 @@ def import_qti( options=options if options else None, correct_answer=correct_answer, explanation=explanation or None, - user_id=current_user.id, + user_id=None, ) db.add(q) created.append(q) diff --git a/backend/app/services/article_writer.py b/backend/app/services/article_writer.py index 7a08c17..3dc9e96 100644 --- a/backend/app/services/article_writer.py +++ b/backend/app/services/article_writer.py @@ -198,7 +198,7 @@ def write_article(db: Session, topic: str, category_id: int | None = None, article = Article( slug=slug, title=topic[:300], summary=(data.get("summary") or "").strip()[:2000] or None, - content=None, sections=sections, category_id=category_id, user_id=user_id, + content=None, sections=sections, category_id=category_id, user_id=None, # Never published by generation. A person decides that. status="draft", # References come from the retrieved metadata, not from the model, so a diff --git a/backend/app/tasks/quiz_tasks.py b/backend/app/tasks/quiz_tasks.py index b7b8d98..f5631b3 100644 --- a/backend/app/tasks/quiz_tasks.py +++ b/backend/app/tasks/quiz_tasks.py @@ -1142,7 +1142,7 @@ def generate_article_draft(self, job_id: str, user_id: int, topic: str, # drafts never showed a single one of them. article = Article(slug=slug, title=title, summary=str(data.get("summary", "") or "")[:2000], content=str(data.get("content", "") or ""), sections=sections, - user_id=user_id, status="draft", + user_id=None, status="draft", generated_by=str(ai_model_id or "ai")[:80]) db.add(article) # The reference list is written from the records rather than by the @@ -1233,7 +1233,7 @@ def generate_article_cards(self, job_id: str, user_id: int, article_id: int, # article belongs to the same topic, and asking somebody to # choose the category again is asking them to repeat a fact # the system already knows. - deck = FlashcardDeck(title=f"Cards: {article.title}", user_id=user_id, + deck = FlashcardDeck(title=f"Cards: {article.title}", user_id=None, category_id=article.category_id, card_count=0, is_shared=0) db.add(deck) db.flush() diff --git a/backend/app/utils/category_grants.py b/backend/app/utils/category_grants.py index 96c9649..f78e943 100644 --- a/backend/app/utils/category_grants.py +++ b/backend/app/utils/category_grants.py @@ -59,12 +59,16 @@ def granted_category_scope(db: Session, user: User) -> set[int] | None: 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. + A moderator may. 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. + + Authorship is not a way in. An article belongs to the library, not to + whoever typed it, and the address that wrote most of this one is not even + the working administrator any more. """ - if user.is_moderator or article.user_id == user.id: + if user.is_moderator: return True if article.category_id is None: return False diff --git a/backend/app/utils/quiz_access.py b/backend/app/utils/quiz_access.py index 50c8325..b472598 100644 --- a/backend/app/utils/quiz_access.py +++ b/backend/app/utils/quiz_access.py @@ -54,14 +54,18 @@ def set_quiz_shared(db, quiz, user, shared): def may_edit_question(db, question, user) -> bool: """Whether this person writes this question, rather than sits it. - Three ways in: moderation, authorship, or an editorial grant that covers - where the question is filed. It is what separates reading an answer because - it is your job from reading it because you found the URL. + Two ways in: the admin role, or an editorial grant that covers where the + question is filed. It is what separates reading an answer because it is + your job from reading it because you found the URL. + + Authorship used to be a third way. It is not one any more: nothing in the + bank belongs to a person, so "I made this" is not a claim the bank can + check or would honour if it could. Rights come from the role and from + grants, and only from those — which is the point of having a grant system + at all. """ if user.is_moderator: return True - if question.user_id is not None and question.user_id == user.id: - return True from app.utils.category_grants import question_scope_predicate predicate = question_scope_predicate(db, user) if predicate is None: diff --git a/backend/tests/api-contract.json b/backend/tests/api-contract.json index b8e28ab..f9f9daa 100644 --- a/backend/tests/api-contract.json +++ b/backend/tests/api-contract.json @@ -1311,7 +1311,6 @@ "query:favorites_only?", "query:folder_id?", "query:limit?", - "query:my_questions?", "query:needs?", "query:offset?", "query:q?", diff --git a/backend/tests/test_access.py b/backend/tests/test_access.py index 386588b..92b72e5 100644 --- a/backend/tests/test_access.py +++ b/backend/tests/test_access.py @@ -122,11 +122,22 @@ class AccessTests(unittest.TestCase): 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): + def test_writing_an_article_does_not_make_it_yours(self): + """Authorship was a way in. It is not one any more. + + An article belongs to the library, and who may change it is the admin + role or a grant over the branch it is filed under — not whoever + happened to type it. The address that wrote most of this library is not + even the working administrator any more, which is the whole argument. + """ 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.assertFalse(can_edit_article(self.db, self.peer(), own)) + # A grant over where it is filed is a way in, and so is moderation. + self.db.add(CategoryGrant(category_id=4, user_id=self.peer().id)) + 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))) diff --git a/backend/tests/test_question_detail_access.py b/backend/tests/test_question_detail_access.py index 3f1fa68..791a1e3 100644 --- a/backend/tests/test_question_detail_access.py +++ b/backend/tests/test_question_detail_access.py @@ -51,11 +51,15 @@ class QuestionDetailAccessTests(unittest.TestCase): self.assertEqual(body["explanation"], "Full explanation") self.assertEqual(body["explanation_image_path"], "answer.png") - def test_the_author_of_a_question_writes_it(self): - # Question 3 is the owner's own. + def test_writing_a_question_does_not_make_it_yours(self): + """Authorship is not a claim the bank honours. + + Question 3 carries the owner's id, from before the bank stopped + belonging to people. Reading its answer takes the admin role, a grant, + or an attempt — the same as anybody else's. + """ self.bank.user = self.bank.owner - body = self.detail(3) - self.assertEqual(body["correct_answer"], "yes") + self.assertIsNone(self.detail(3)["correct_answer"]) if __name__ == "__main__": diff --git a/backend/tests/test_related_privacy.py b/backend/tests/test_related_privacy.py index 45384ab..ae0b976 100644 --- a/backend/tests/test_related_privacy.py +++ b/backend/tests/test_related_privacy.py @@ -91,13 +91,15 @@ class PrivacyTests(unittest.TestCase): self.assertEqual(self.chat(qid).status_code, 403) for boundary in (self.quota, self.model, self.similar, self.ai): boundary.assert_not_called() - res = self.chat(3) # the owner's own question - self.assertEqual(res.status_code, 200, res.text) - # The context search is run as the caller, not as whoever wrote it. - self.assertEqual(self.similar.call_args.args[2].id, self.owner.id) - self.assertEqual(self.similar.call_args.args[1].id, 3) + # Question 3 used to open here because this learner wrote it. Nothing + # in the bank belongs to a person now, so authorship buys nothing and + # it is refused like the rest. + self.assertEqual(self.chat(3).status_code, 403) self.login(self.mod) self.assertEqual(self.chat(4).status_code, 200) + # The context search is run as the caller, not as whoever wrote it. + self.assertEqual(self.similar.call_args.args[2].id, self.mod.id) + self.assertEqual(self.similar.call_args.args[1].id, 4) def test_tutor_attempt_modes_pool_and_review(self): # The tutor's site switch lives in Redis, which these tests share with @@ -159,15 +161,18 @@ class PrivacyTests(unittest.TestCase): self.client.headers.clear() self.assertEqual(self.client.get('/uploads/questions/stem-4.png').status_code, 401) self.login(self.owner, cookie=True) - # An answer image is answer-side: 'answer-3' opens because question 3 is - # this learner's own, and 'answer-1' is checked below because it is not. - for path in ('stem-1', 'stem-3', 'stem-4', 'answer-3'): + # Stems are readable in the bank. Answer images are not: 'answer-3' + # used to open because this learner wrote question 3, and authorship + # is no longer a claim the bank honours. + for path in ('stem-1', 'stem-3', 'stem-4'): res = self.client.get(f'/uploads/questions/{path}.png') self.assertEqual(res.status_code, 200, res.text) self.assertEqual(res.headers['cache-control'], 'private, no-store') self.assertEqual(res.headers['vary'], 'Cookie, Authorization') # Somebody else's answer, with no attempt behind it: refused. - self.assertEqual(self.client.get('/uploads/questions/answer-1.png').status_code, 404) + for answer in ('answer-1', 'answer-3'): + self.assertEqual( + self.client.get(f'/uploads/questions/{answer}.png').status_code, 404, answer) # A derivative may be kept by the browser that asked for it — never by # a shared cache, which in front of access-controlled images is how one # learner is served another's private figure. The original still says @@ -330,7 +335,10 @@ class PrivacyTests(unittest.TestCase): self.file(path) self.db.commit() self.login(self.mod) - self.assertEqual(self.client.get(f'/flashcards/{deck.id}').status_code, 403) + # A deck is the bank's, so an educator reaches it. The image on + # the card is a separate question, and that is the boundary this + # test is about: it stays refused. + self.assertEqual(self.client.get(f'/flashcards/{deck.id}').status_code, 200) self.assertEqual(self.client.get('/uploads/' + path).status_code, 404) before = self.db.query(Question).count() response = self.client.post('/questions/create', json={**payload, 'image_path': path}) diff --git a/frontend/src/components/Navbar.jsx b/frontend/src/components/Navbar.jsx index 4466411..6d6086c 100644 --- a/frontend/src/components/Navbar.jsx +++ b/frontend/src/components/Navbar.jsx @@ -212,9 +212,14 @@ export default function Navbar({ onSignIn, onRegister, onSearch }) { // What you have put aside: the star and any libraries you keep. Beside the // bank because that is where things get put aside from. { to: '/collections', label: 'Collections' }, + // A grant over a branch makes you an editor of what is in it: the + // questions and the images they use. It does not make you an editor of + // the library — Editorial is the whole site's review queue, and its route + // is moderator-only, so offering it to a grant holder was offering a door + // that answers "Not yours to open". ...(canManageQuestions ? [{ to: '/questions/manage', label: 'Questions' }, - { to: '/media', label: 'Images' }, - { to: '/editorial', label: 'Editorial' }] : []), + { to: '/media', label: 'Images' }] : []), + ...(isModerator ? [{ to: '/editorial', label: 'Editorial' }] : []), { to: '/study-plans', label: 'Study plans' }, { to: '/articles', label: 'Reading' }, { to: '/flashcards', label: 'Cards' },