feat: the bank belongs to a role, not to a person
571 categories, 21 uploaded documents, 14 articles, 8 card decks, 30 shared tests and 2 questions carried somebody's name — mostly daniel@danvics.com, which is not even the working administrator any more. So "who may edit this" partly depended on who happened to create it, and handing the site to somebody else would have meant rewriting every one of those rows. Migration q6a7b8c9d0e1 empties those owner columns and makes them nullable, because ownerless is now a legitimate state and a NOT NULL owner is exactly what forced a name onto every row. Nothing is deleted and nothing moves. What keeps its owner, deliberately: attempts, notes, favourites, collections, folders, study-plan progress, and the quizzes that are somebody's own sittings rather than shared bank tests. study_plans needed nothing — it never had an owner column. Then the code, so it cannot grow back. Authorship is no longer a way in anywhere: may_edit_question and can_edit_article ask the role and the grants and nothing else; the article draft, status and delete paths lost their "or you wrote it" arm; decks are the bank's, so an educator reaches any of them and a learner reaches the shared ones; documents are the corpus, so they are editors-only rather than "mine"; and every creation path writes user_id NULL. The bank listing's "mine" facet went with it — it counted nothing and could only ever count nothing. Verified against production as a real learner account: every bank write 403s, admin settings 403, documents empty. As an admin, everything opens. Also: a category grant no longer offers Editorial in the menu. It offers Questions and Images, which is what a grant covers; Editorial is the whole library's review queue and its route is moderator-only, so the entry was a door that answered "Not yours to open". Six tests changed rather than deleted — they asserted the old model, and each now asserts the new one: writing an article does not make it yours, writing a question does not make it yours, an answer image is not opened by authorship, the tutor is not opened by authorship. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN
This commit is contained in:
parent
8d7485eaad
commit
b6cfcaa1e9
21 changed files with 222 additions and 111 deletions
70
backend/alembic/versions/q6a7b8c9d0e1_ownership_is_a_role.py
Normal file
70
backend/alembic/versions/q6a7b8c9d0e1_ownership_is_a_role.py
Normal file
|
|
@ -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
|
||||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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 —
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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})
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -1311,7 +1311,6 @@
|
|||
"query:favorites_only?",
|
||||
"query:folder_id?",
|
||||
"query:limit?",
|
||||
"query:my_questions?",
|
||||
"query:needs?",
|
||||
"query:offset?",
|
||||
"query:q?",
|
||||
|
|
|
|||
|
|
@ -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)))
|
||||
|
||||
|
|
|
|||
|
|
@ -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__":
|
||||
|
|
|
|||
|
|
@ -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})
|
||||
|
|
|
|||
|
|
@ -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' },
|
||||
|
|
|
|||
Loading…
Reference in a new issue