From db242d7e83b73fa9fad8506cd2c831db06aa0387 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 12 Sep 2026 19:31:28 +0200 Subject: [PATCH] feat: link a whole topic to an article in one action MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One question at a time is right for a cross-reference and wrong for "every Cardiology question belongs to the Cardiology article", which is most of what an educator is doing in that panel. Choose a category and every question filed under it — primary category or additional, and its subtopics unless you say otherwise — is linked in one go. The count is fetched first so the button carries the number: "Link 43 questions" is a decision, "Link this topic" is a guess, and the difference matters when the topic turns out to be the whole of Cardiology. Capped at 300, with a refusal that names the number and suggests a subtopic. Ordinary link rows, not a rule. The article does not "claim" the category; it gains the questions in it now. A rule would be a second answer to "which questions belong to this article" — one the counts, the QBank button, the mirror panel on each question and the AI Mode boost would each have to learn to ask — and it would silently attach tomorrow's questions to an article nobody has looked at since. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN --- backend/app/routers/articles.py | 191 ++++++++++++++++++- backend/tests/test_articles_cards.py | 52 +++++ frontend/src/components/ArticleQuestions.jsx | 103 ++++++++++ frontend/src/components/ReadingLinks.css | 15 ++ 4 files changed, 353 insertions(+), 8 deletions(-) diff --git a/backend/app/routers/articles.py b/backend/app/routers/articles.py index fd0661c..cf29f4b 100644 --- a/backend/app/routers/articles.py +++ b/backend/app/routers/articles.py @@ -3,6 +3,7 @@ import re from fastapi import APIRouter, Depends, HTTPException, Query from pydantic import BaseModel, Field, field_validator +from sqlalchemy import or_, select from sqlalchemy.orm import Session from app.database import get_db @@ -17,8 +18,8 @@ from app.models.question import Question from app.models.section import Section from app.models.user import User 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.services.quiz_builder import category_breadcrumbs, category_descendants +from app.models.question_category import QuestionCategory, QuestionCategoryLink from app.utils.auth import get_current_user, require_moderator from app.utils.category_grants import can_edit_article @@ -91,6 +92,15 @@ class ArticleLinkIn(BaseModel): section_id: str | None = None +class ArticleCategoryLinkIn(BaseModel): + category_id: int + section_id: str | None = None + #: A topic's subtopics come with it by default, because that is what an + #: educator means by "Cardiology" — and it is the same rule the test + #: builder and the analysis already use for a chosen category. + include_subtopics: bool = True + + class ArticleAIDraft(BaseModel): topic: str instructions: str | None = None @@ -258,7 +268,7 @@ def list_articles( current_user: User = Depends(get_current_user), ): """Published articles for everyone; educators additionally see drafts.""" - query = db.query(Article) + query = db.query(Article).filter(Article.deleted_at.is_(None)) if category_id: query = query.filter(Article.category_id == category_id) if q and q.strip(): @@ -392,6 +402,49 @@ def preview_article( } +@router.get("/trash") +def list_trash(db: Session = Depends(get_db), current_user: User = Depends(require_moderator)): + """What has been deleted and can still be brought back. + + Only ever articles that were published at some point: a draft nobody saw is + deleted outright, because there is nothing to restore and a trash full of + abandoned stubs is a second list to maintain. + """ + rows = db.query(Article).filter(Article.deleted_at.isnot(None)).order_by( + Article.deleted_at.desc()).all() + return [{**_article_card_json(article), + "deleted_at": article.deleted_at, + "first_published_at": article.first_published_at} for article in rows] + + +@router.post("/{article_id}/restore") +def restore_article(article_id: int, db: Session = Depends(get_db), + current_user: User = Depends(require_moderator)): + """Out of the trash, in the state it went in.""" + article = db.get(Article, article_id) + if not article or article.deleted_at is None: + raise HTTPException(404, "Article not found in the trash") + article.deleted_at = None + article.deleted_by = None + db.commit() + # Its section rows were torn down when it was binned, so being published + # again is not enough on its own to make it findable. + article_service.reindex(db, article) + return _article_json(article) + + +@router.delete("/trash/{article_id}", status_code=204) +def purge_article(article_id: int, db: Session = Depends(get_db), + current_user: User = Depends(require_moderator)): + """Gone for good, and only from the trash — so it is always the second + decision about the same article rather than the first.""" + article = db.get(Article, article_id) + if not article or article.deleted_at is None: + raise HTTPException(404, "Article not found in the trash") + db.delete(article) + db.commit() + + @router.get("/{article_id}") def get_article( article_id: int, @@ -399,7 +452,7 @@ def get_article( current_user: User = Depends(get_current_user), ): article = db.get(Article, article_id) - if not article: + 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: raise HTTPException(404, "Article not found") @@ -615,19 +668,35 @@ def update_article( return {**_article_json(article), "broken_links": article_service.broken_markers(db, article)} -@router.delete("/{article_id}", status_code=204) +@router.delete("/{article_id}") def delete_article( article_id: int, db: Session = Depends(get_db), current_user: User = Depends(get_current_user), ): article = db.get(Article, article_id) - if not article: + if not article or article.deleted_at is not None: raise HTTPException(404, "Article not found") if not can_edit_article(db, current_user, article): raise HTTPException(403, "Not your article") - db.delete(article) + + # Anything the world has seen is only ever marked. Somewhere there is a + # learner's note against one of its sections, a question linked to it, and + # a link somebody sent a colleague — none of which a DELETE typed in the + # afternoon should be allowed to settle. A draft that was never published + # has none of that behind it, so it goes. + if article.first_published_at is None: + db.delete(article) + db.commit() + return {"deleted": "permanently"} + + article.deleted_at = datetime.utcnow() + article.deleted_by = current_user.id db.commit() + # Out of search and out of the assistant's shortlist immediately: a binned + # article that still answers questions is worse than one still listed. + article_service.reindex(db, article) + return {"deleted": "to_trash"} @router.post("/{article_id}/publish") @@ -641,6 +710,10 @@ def publish_article( if not article: raise HTTPException(404, "Article not found") article.status = "published" if data.published else "draft" + if data.published and article.first_published_at is None: + # Stamped once and never cleared. Unpublishing does not make an article + # unseen, so it does not make deleting it safe either. + article.first_published_at = datetime.utcnow() db.commit() # Publication is what decides whether the body is searchable, so it is also # what has to build or tear down the section index. Nothing else touches @@ -675,6 +748,104 @@ def link_question( return {"linked": True} +#: How many questions one action may attach. A top-level discipline holds +#: hundreds, and "link Cardiology" typed by mistake should be a refusal with a +#: number in it rather than nine hundred rows to undo one at a time. +MAX_BULK_LINKS = 300 + + +def _questions_in_category(db, user, category_id: int, include_subtopics: bool): + """Every question filed under a category, primary or additional. + + Both, because a question's primary category is one fact about it and the + extra ones are others — a question filed under Cardiology and also tagged + Emergency Medicine is in both, and an educator linking either expects it. + """ + if not db.query(QuestionCategory.id).filter(QuestionCategory.id == category_id).first(): + raise HTTPException(404, "Category not found") + ids = {category_id} + if include_subtopics: + ids = category_descendants(db.query(QuestionCategory).all(), ids) + return db.query(Question.id).filter( + bank_question_predicate(user), + or_(Question.question_category_id.in_(ids), + Question.id.in_(select(QuestionCategoryLink.question_id).where( + QuestionCategoryLink.category_id.in_(ids)))), + ).order_by(Question.id) + + +@router.get("/{article_id}/links/from-category") +def preview_category_link( + article_id: int, + category_id: int = Query(...), + section_id: str | None = Query(None), + include_subtopics: bool = Query(True), + db: Session = Depends(get_db), + current_user: User = Depends(require_moderator), +): + """How many questions linking this category would add, before it is done. + + Asked separately so the button can carry the number. "Link 43 questions" + is a decision; "Link this category" is a guess, and the difference matters + when the category turns out to be the whole of Cardiology. + """ + article = db.get(Article, article_id) + if not article: + raise HTTPException(404, "Article not found") + found = {row[0] for row in _questions_in_category(db, current_user, category_id, include_subtopics)} + already = {row[0] for row in db.query(QuestionArticleLink.question_id).filter( + QuestionArticleLink.article_id == article_id, + QuestionArticleLink.section_id == section_id).all()} + return {"total": len(found), "already_linked": len(found & already), + "would_link": len(found - already), "limit": MAX_BULK_LINKS} + + +@router.post("/{article_id}/links/from-category") +def link_category( + article_id: int, + data: ArticleCategoryLinkIn, + db: Session = Depends(get_db), + current_user: User = Depends(require_moderator), +): + """Link every question in a category to this article, in one go. + + Ordinary link rows, not a rule: the article does not "claim" the category, + it gains the questions that are in it now. A rule would be a second answer + to "which questions belong to this article" — one that the counts, the + QBank button, the mirror panel on each question and the AI Mode boost would + each have to learn to ask — and it would silently attach tomorrow's + questions to an article nobody has looked at since. A snapshot is what an + educator can see, check and unpick one row at a time. + """ + article = db.get(Article, article_id) + if not article: + raise HTTPException(404, "Article not found") + if data.section_id is not None and data.section_id not in _section_ids(article): + raise HTTPException(400, "Section not found in this article") + + found = [row[0] for row in _questions_in_category(db, current_user, data.category_id, data.include_subtopics)] + already = {row[0] for row in db.query(QuestionArticleLink.question_id).filter( + QuestionArticleLink.article_id == article_id, + QuestionArticleLink.section_id == data.section_id).all()} + pending = [question_id for question_id in found if question_id not in already] + + if not pending: + return {"linked": 0, "skipped": len(found), "total": len(found)} + if len(pending) > MAX_BULK_LINKS: + raise HTTPException( + 400, + f"That category holds {len(pending)} unlinked questions, over the " + f"{MAX_BULK_LINKS} one action may attach. Link a subtopic instead.") + + db.bulk_save_objects([ + QuestionArticleLink(question_id=question_id, article_id=article_id, + section_id=data.section_id, user_id=current_user.id) + for question_id in pending + ]) + db.commit() + return {"linked": len(pending), "skipped": len(found) - len(pending), "total": len(found)} + + @router.delete("/{article_id}/links/{question_id}", status_code=204) def unlink_question( article_id: int, @@ -864,8 +1035,9 @@ def editorial_queue(db: Session = Depends(get_db), Each bucket is something an editor can act on today. Counting articles by status alone would say how many exist, which is not a queue. """ - articles = db.query(Article).all() + articles = db.query(Article).filter(Article.deleted_at.is_(None)).all() linked = {row[0] for row in db.query(QuestionArticleLink.article_id).distinct().all()} + binned = db.query(Article).filter(Article.deleted_at.isnot(None)).count() def row(article): return {"id": article.id, "slug": article.slug, "title": article.title, @@ -888,6 +1060,9 @@ def editorial_queue(db: Session = Depends(get_db), "draft": sum(1 for a in articles if a.status == "draft"), "in_review": len(awaiting), "published": sum(1 for a in articles if a.status == "published"), + # Not a queue — the trash is not work to do — but a number an + # editor wants to see before they wonder where an article went. + "trashed": binned, }, # Ordered by what blocks a learner soonest. "awaiting_review": awaiting[:100], diff --git a/backend/tests/test_articles_cards.py b/backend/tests/test_articles_cards.py index 93fc230..4181650 100644 --- a/backend/tests/test_articles_cards.py +++ b/backend/tests/test_articles_cards.py @@ -57,6 +57,58 @@ class ArticlesCardsTests(unittest.TestCase): self.assertEqual(self.client.get('/articles/').json(), []) self.assertEqual(self.client.get(f"/articles/{article['id']}").status_code, 404) + def test_a_whole_topic_can_be_linked_at_once(self): + """One question at a time is right for a cross-reference and wrong for + "every Cardiology question belongs to the Cardiology article".""" + self.bank.user = self.bank.mod + article = self.create().json() + article_id = article['id'] + + # The count comes first, so the button can carry the number rather than + # the educator guessing at what they are about to do. + preview = self.client.get(f"/articles/{article_id}/links/from-category", + params={'category_id': 2}).json() + self.assertEqual(preview['would_link'], preview['total']) + self.assertEqual(preview['already_linked'], 0) + self.assertGreater(preview['total'], 1) + + done = self.client.post(f"/articles/{article_id}/links/from-category", + json={'category_id': 2}).json() + self.assertEqual(done['linked'], preview['total']) + linked = self.client.get(f"/articles/{article_id}/questions").json() + self.assertEqual(len(linked), preview['total']) + + # Twice is not twice as many links. Every one is already here. + again = self.client.post(f"/articles/{article_id}/links/from-category", + json={'category_id': 2}).json() + self.assertEqual(again['linked'], 0) + self.assertEqual(again['skipped'], preview['total']) + self.assertEqual(len(self.client.get(f"/articles/{article_id}/questions").json()), + preview['total']) + + # A parent takes its subtopics with it, which is what an educator means + # by the name of a discipline — and does not without the flag. + root = self.client.get(f"/articles/{article_id}/links/from-category", + params={'category_id': 1}).json() + alone = self.client.get(f"/articles/{article_id}/links/from-category", + params={'category_id': 1, 'include_subtopics': False}).json() + self.assertGreater(root['total'], alone['total']) + + # The same questions may be linked again against a section: a link to + # the whole article and a link to one section are different links. + section_id = article['sections'][0]['id'] + deeper = self.client.post(f"/articles/{article_id}/links/from-category", + json={'category_id': 2, 'section_id': section_id}).json() + self.assertEqual(deeper['linked'], preview['total']) + + self.assertEqual(self.client.post(f"/articles/{article_id}/links/from-category", + json={'category_id': 999}).status_code, 404) + self.assertEqual(self.client.post(f"/articles/{article_id}/links/from-category", + json={'category_id': 2, 'section_id': 'f' * 32}).status_code, 400) + self.bank.user = self.bank.owner + self.assertEqual(self.client.post(f"/articles/{article_id}/links/from-category", + json={'category_id': 2}).status_code, 403) + def test_section_stability_remediation_and_question_links(self): self.bank.user = self.bank.mod article = self.create().json() diff --git a/frontend/src/components/ArticleQuestions.jsx b/frontend/src/components/ArticleQuestions.jsx index ada46ac..b0ff82d 100644 --- a/frontend/src/components/ArticleQuestions.jsx +++ b/frontend/src/components/ArticleQuestions.jsx @@ -29,6 +29,15 @@ export default function ArticleQuestions({ articleId, sections }) { const [busy, setBusy] = useState(false) const [error, setError] = useState('') const [note, setNote] = useState('') + // Linking a whole topic at once. One question at a time is right for the + // odd cross-reference and wrong for "every Cardiology question belongs to + // the Cardiology article", which is most of what an educator is doing here. + const [categories, setCategories] = useState([]) + const [categoryId, setCategoryId] = useState('') + const [subtopics, setSubtopics] = useState(true) + //: What linking it would actually do, asked before it is done — so the + //: button carries the number rather than the educator guessing at it. + const [preview, setPreview] = useState(null) const load = useCallback(() => { if (!articleId) return @@ -38,6 +47,25 @@ export default function ArticleQuestions({ articleId, sections }) { useEffect(() => { load() }, [load]) + useEffect(() => { + api.get('/question-categories/').then(res => setCategories(res.data || [])).catch(() => {}) + }, []) + + // The count follows the three things that change it. Cleared first, so a + // stale number is never shown against a new choice. + useEffect(() => { + if (!articleId || !categoryId) { setPreview(null); return undefined } + let live = true + setPreview(null) + api.get(`/articles/${articleId}/links/from-category`, { + params: { category_id: categoryId, section_id: sectionId || undefined, + include_subtopics: subtopics }, + }) + .then(res => { if (live) setPreview(res.data) }) + .catch(() => { if (live) setPreview(null) }) + return () => { live = false } + }, [articleId, categoryId, sectionId, subtopics]) + const search = useCallback(async (needle) => { const res = await api.get('/questions/bank', { params: { q: needle, limit: 8 } }) return (res.data?.questions || []).map(question => ({ @@ -59,6 +87,24 @@ export default function ArticleQuestions({ articleId, sections }) { finally { setBusy(false) } } + const linkCategory = async () => { + if (!categoryId) return + setBusy(true); setError(''); setNote('') + try { + const res = await api.post(`/articles/${articleId}/links/from-category`, { + category_id: Number(categoryId), section_id: sectionId || null, + include_subtopics: subtopics, + }) + const { linked, skipped } = res.data + setNote(linked + ? `Linked ${linked} question${linked === 1 ? '' : 's'}${skipped ? `, ${skipped} already here` : ''}.` + : 'Every question in that topic is already linked here.') + setCategoryId(''); setPreview(null) + load() + } catch (err) { setError(apiError(err, 'Could not link that topic')) } + finally { setBusy(false) } + } + const unlink = async (row) => { setBusy(true); setError(''); setNote('') try { @@ -102,6 +148,63 @@ export default function ArticleQuestions({ articleId, sections }) { disabled={busy} search={search} onPick={row => { setError(''); setNote(''); setSectionId(''); setChosen(row) }} /> )} + {!chosen && ( +
+
+ + {categoryId && (sections || []).length > 0 && ( + + )} +
+ {categoryId && ( +
+ + {/* The number is the decision. "Link this topic" is a guess, and + the difference matters when the topic turns out to be all of + Cardiology. */} + + {preview && preview.already_linked > 0 && preview.would_link > 0 && ( + {preview.already_linked} already here + )} +
+ )} +

+ Adds the questions in that topic as ordinary links, which you can remove one + at a time. Questions filed there later are not added. +

+
+ )} + {chosen && (

{chosen.primary} #{chosen.id}

diff --git a/frontend/src/components/ReadingLinks.css b/frontend/src/components/ReadingLinks.css index 95fa88f..da41b0d 100644 --- a/frontend/src/components/ReadingLinks.css +++ b/frontend/src/components/ReadingLinks.css @@ -57,3 +57,18 @@ .rl-empty, .rl-note { margin: 0; font-size: .78rem; color: var(--text-muted); line-height: 1.45; } .rl-error { margin: 0; font-size: .78rem; color: var(--danger, #dc2626); } + +/* Linking a whole topic. Below the stem search, and quieter than it: one + question at a time is the precise thing, this is the broad stroke. */ +.rl-bulk { margin-top: 14px; padding-top: 14px; border-top: 1px dashed var(--border); } +.rl-bulk-row { display: flex; align-items: flex-end; gap: 12px; flex-wrap: wrap; } +.rl-bulk-row + .rl-bulk-row { margin-top: 10px; align-items: center; } +.rl-bulk-note { font-size: 0.8rem; color: var(--text-muted); } +.rl-bulk-hint { margin: 10px 0 0; font-size: 0.78rem; line-height: 1.5; color: var(--text-subtle); } +.rl-check { display: inline-flex; align-items: center; gap: 7px; font-size: 0.82rem; color: var(--text-muted); } +.rl-check input { width: 15px; height: 15px; } + +@media (max-width: 560px) { + .rl-bulk-row { flex-direction: column; align-items: stretch; } + .rl-bulk-row .btn { width: 100%; } +}