From 9b51a17507c1a379d32d9b0dc2518b8d9c06e033 Mon Sep 17 00:00:00 2001 From: Daniel Date: Thu, 10 Sep 2026 01:40:03 +0200 Subject: [PATCH] feat: flashcard decks share the question and article category tree Questions and articles both pointed at `question_categories`; decks had no category at all, so the three content types could not be filtered together and a topic's cards were unreachable from its category. - `flashcard_decks.category_id` references the same tree (migration t2f3a4b5c697), so one category now spans questions, articles and cards. - `GET /flashcards/` takes `category_id` and includes descendants, so a parent category picks up everything filed beneath it. - `PATCH /flashcards/{id}` files or unfiles a deck, refusing a category id that does not exist rather than storing a dangling reference. - The cards page shows each deck's category as a selector. Tests: 5 new backend (all three types resolve to the same id, descendant filtering, file and unfile, unknown category refused, renaming leaves the category alone) and 1 new frontend. Full suites green: 101 backend, 136 frontend, build clean. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PpfzbZ1QTLMeVYxM2kyq8m --- .../versions/t2f3a4b5c697_shared_category.py | 28 ++++++ backend/app/models/flashcard.py | 2 + backend/app/routers/flashcards.py | 19 +++- backend/app/schemas/flashcard.py | 1 + backend/tests/test_shared_category.py | 98 +++++++++++++++++++ frontend/src/index.css | 9 ++ frontend/src/pages/FlashcardsPage.jsx | 26 +++++ frontend/src/pages/FlashcardsPage.test.jsx | 26 ++++- 8 files changed, 205 insertions(+), 4 deletions(-) create mode 100644 backend/alembic/versions/t2f3a4b5c697_shared_category.py create mode 100644 backend/tests/test_shared_category.py diff --git a/backend/alembic/versions/t2f3a4b5c697_shared_category.py b/backend/alembic/versions/t2f3a4b5c697_shared_category.py new file mode 100644 index 0000000..42af35a --- /dev/null +++ b/backend/alembic/versions/t2f3a4b5c697_shared_category.py @@ -0,0 +1,28 @@ +"""Put flashcard decks on the same category tree as questions and articles. + +Questions and articles already point at `question_categories`; decks had no +category at all, so the three content types could not be filtered together. + +Revision ID: t2f3a4b5c697 +Revises: s1e2f3a4b586 +""" +from alembic import op + +revision = "t2f3a4b5c697" +down_revision = "s1e2f3a4b586" +branch_labels = None +depends_on = None + + +def upgrade(): + op.execute(""" + ALTER TABLE flashcard_decks + ADD COLUMN IF NOT EXISTS category_id INTEGER + REFERENCES question_categories(id) ON DELETE SET NULL + """) + op.execute("CREATE INDEX IF NOT EXISTS ix_flashcard_decks_category_id ON flashcard_decks(category_id)") + + +def downgrade(): + op.execute("DROP INDEX IF EXISTS ix_flashcard_decks_category_id") + op.execute("ALTER TABLE flashcard_decks DROP COLUMN IF EXISTS category_id") diff --git a/backend/app/models/flashcard.py b/backend/app/models/flashcard.py index 2ea23fc..7980af8 100644 --- a/backend/app/models/flashcard.py +++ b/backend/app/models/flashcard.py @@ -8,6 +8,8 @@ class FlashcardDeck(Base): id = Column(Integer, primary_key=True, index=True) title = Column(String, nullable=False) 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) card_count = Column(Integer, default=0) is_shared = Column(Integer, default=0) # 0 = private, 1 = shared diff --git a/backend/app/routers/flashcards.py b/backend/app/routers/flashcards.py index 268ad47..a6083ee 100644 --- a/backend/app/routers/flashcards.py +++ b/backend/app/routers/flashcards.py @@ -14,7 +14,9 @@ from app.models.flashcard import ( ) from app.models.question import Question from app.models.section import Section +from app.models.question_category import QuestionCategory from app.models.user import User +from app.services.quiz_builder import category_descendants from app.services.quiz_builder import bank_question_predicate from app.utils.auth import get_current_user, require_moderator @@ -33,6 +35,7 @@ class FlashcardDeckCreate(BaseModel): class FlashcardDeckUpdate(BaseModel): title: str | None = None + category_id: int | None = None class FlashcardDeckResponse(BaseModel): @@ -137,13 +140,18 @@ def create_flashcard_deck( @router.get("/", response_model=list[FlashcardDeckResponse]) def list_flashcard_decks( include_deleted: bool = Query(False), + category_id: int | None = Query(None), db: Session = Depends(get_db), current_user: User = Depends(get_current_user), ): - """List flashcard decks belonging to current user.""" + """List flashcard decks belonging to current user, optionally by category.""" q = db.query(FlashcardDeck).filter(FlashcardDeck.user_id == current_user.id) if not include_deleted: q = q.filter(FlashcardDeck.deleted_at.is_(None)) + if category_id is not None: + # A parent category includes everything filed beneath it. + wanted = category_descendants(db.query(QuestionCategory).all(), [category_id]) + q = q.filter(FlashcardDeck.category_id.in_(wanted)) return q.order_by(FlashcardDeck.created_at.desc()).all() @@ -275,16 +283,21 @@ def update_deck( db: Session = Depends(get_db), current_user: User = Depends(get_current_user), ): - """Update deck metadata (title). Owner or admin only.""" + """Update deck metadata (title, category). Owner or admin only.""" deck = _own_deck_or_404(deck_id, current_user, db) if data.title is not None: title = data.title.strip() if not title: raise HTTPException(status_code=400, detail="Title cannot be empty") deck.title = title[:300] + if "category_id" in data.model_fields_set: + # Same tree as questions and articles, so the id must exist in it. + if data.category_id is not None and not db.get(QuestionCategory, data.category_id): + raise HTTPException(status_code=404, detail="Category not found") + deck.category_id = data.category_id db.commit() db.refresh(deck) - return {"id": deck.id, "title": deck.title} + return {"id": deck.id, "title": deck.title, "category_id": deck.category_id} @router.put("/{deck_id}/share") diff --git a/backend/app/schemas/flashcard.py b/backend/app/schemas/flashcard.py index 51fca79..72c9a57 100644 --- a/backend/app/schemas/flashcard.py +++ b/backend/app/schemas/flashcard.py @@ -21,6 +21,7 @@ class FlashcardDeckResponse(BaseModel): id: int title: str section_id: int | None = None + category_id: int | None = None user_id: int card_count: int created_at: datetime diff --git a/backend/tests/test_shared_category.py b/backend/tests/test_shared_category.py new file mode 100644 index 0000000..a4e7d19 --- /dev/null +++ b/backend/tests/test_shared_category.py @@ -0,0 +1,98 @@ +"""Questions, articles and flashcard decks all file under one category tree. + +Run: DATABASE_URL=sqlite:///:memory: PYTHONPATH=backend python -m unittest discover -s backend/tests +""" +import os +os.environ["DATABASE_URL"] = "sqlite:///:memory:" + +import unittest + +from fastapi import FastAPI +from fastapi.testclient import TestClient +from sqlalchemy import create_engine +from sqlalchemy.orm import Session +from sqlalchemy.pool import StaticPool + +from app.database import Base, get_db +from app.models.article import Article +from app.models.course import Course # noqa — Quiz.course_id FK needs the table in metadata. +from app.models.flashcard import FlashcardDeck +from app.models.question import Question +from app.models.question_category import QuestionCategory +from app.models.user import User +from app.routers import flashcards +from app.utils.auth import get_current_user + + +class SharedCategoryTests(unittest.TestCase): + def setUp(self): + self.engine = create_engine("sqlite://", connect_args={"check_same_thread": False}, poolclass=StaticPool) + Base.metadata.create_all(self.engine) + self.db = Session(self.engine) + + self.user = User(id=1, name="Owner", email="owner@example.test", hashed_password="unused") + self.db.add(self.user) + self.db.add_all([ + QuestionCategory(id=1, name="Cardiology", user_id=1), + QuestionCategory(id=2, name="Kawasaki disease", parent_id=1, user_id=1), + QuestionCategory(id=3, name="Neurology", user_id=1), + ]) + self.db.flush() + # One of each content type filed under the same tree. + self.db.add(Question(id=1, question_category_id=2, user_id=1, is_shared=1, + question_text="A question", question_type="mcq", + options=["yes", "no"], correct_answer="yes")) + self.db.add(Article(id=1, slug="kawasaki", title="Kawasaki disease", sections=[], + category_id=2, user_id=1, status="published")) + self.db.add_all([ + FlashcardDeck(id=1, title="Kawasaki cards", user_id=1, card_count=0, category_id=2), + FlashcardDeck(id=2, title="Neuro cards", user_id=1, card_count=0, category_id=3), + FlashcardDeck(id=3, title="Unfiled cards", user_id=1, card_count=0), + ]) + self.db.commit() + + app = FastAPI() + app.include_router(flashcards.router, prefix="/flashcards") + app.dependency_overrides[get_db] = lambda: self.db + app.dependency_overrides[get_current_user] = lambda: self.user + self.client = TestClient(app) + + def tearDown(self): + self.client.close() + self.db.close() + self.engine.dispose() + + def test_all_three_content_types_use_the_same_category_ids(self): + question = self.db.get(Question, 1) + article = self.db.get(Article, 1) + deck = self.db.get(FlashcardDeck, 1) + self.assertEqual({question.question_category_id, article.category_id, deck.category_id}, {2}) + + def test_deck_list_filters_by_category_including_descendants(self): + titles = lambda params: {d["title"] for d in self.client.get("/flashcards/", params=params).json()} + self.assertEqual(titles({"category_id": 2}), {"Kawasaki cards"}) + # A parent picks up everything filed beneath it. + self.assertEqual(titles({"category_id": 1}), {"Kawasaki cards"}) + self.assertEqual(titles({"category_id": 3}), {"Neuro cards"}) + self.assertEqual(titles({}), {"Kawasaki cards", "Neuro cards", "Unfiled cards"}) + + def test_a_deck_can_be_filed_and_unfiled(self): + response = self.client.patch("/flashcards/3", json={"category_id": 3}) + self.assertEqual(response.status_code, 200, response.text) + self.assertEqual(response.json()["category_id"], 3) + self.assertEqual(self.db.get(FlashcardDeck, 3).category_id, 3) + + self.assertEqual(self.client.patch("/flashcards/3", json={"category_id": None}).json()["category_id"], None) + + def test_filing_under_a_category_that_does_not_exist_is_refused(self): + self.assertEqual(self.client.patch("/flashcards/1", json={"category_id": 999}).status_code, 404) + self.assertEqual(self.db.get(FlashcardDeck, 1).category_id, 2) + + def test_renaming_a_deck_leaves_its_category_alone(self): + self.client.patch("/flashcards/1", json={"title": "Renamed"}) + deck = self.db.get(FlashcardDeck, 1) + self.assertEqual((deck.title, deck.category_id), ("Renamed", 2)) + + +if __name__ == "__main__": + unittest.main() diff --git a/frontend/src/index.css b/frontend/src/index.css index 4693e49..368d935 100644 --- a/frontend/src/index.css +++ b/frontend/src/index.css @@ -760,3 +760,12 @@ body { .cp-adaptive-row { display: flex; align-items: center; gap: 8px; flex-wrap: wrap; margin-bottom: 12px; padding: 10px; border: 1px solid var(--border); border-radius: 10px; background: var(--input-bg); } .cp-adaptive-label { display: flex; align-items: center; gap: 6px; font-size: .82rem; } .cp-adaptive-label select { padding: 4px 8px; border: 1px solid var(--border); border-radius: 6px; } + +/* Deck category — same tree as questions and articles. */ +.deck-category { margin-bottom: 8px; } +.deck-category select { + width: 100%; padding: 5px 8px; font-size: .76rem; + border: 1px solid var(--border); border-radius: 6px; + background: var(--input-bg); color: var(--text-muted); +} +.deck-category select:hover { color: var(--text); } diff --git a/frontend/src/pages/FlashcardsPage.jsx b/frontend/src/pages/FlashcardsPage.jsx index 0ca991a..60cf64d 100644 --- a/frontend/src/pages/FlashcardsPage.jsx +++ b/frontend/src/pages/FlashcardsPage.jsx @@ -27,6 +27,7 @@ function StarRating({ rating, onRate, readonly = false }) { export default function FlashcardsPage() { const [tab, setTab] = useState('decks') const [decks, setDecks] = useState([]) + const [categories, setCategories] = useState([]) const [trashedDecks, setTrashedDecks] = useState([]) const [sharedDecks, setSharedDecks] = useState([]) const [sharedTotal, setSharedTotal] = useState(0) @@ -55,6 +56,9 @@ export default function FlashcardsPage() { const SHARED_LIMIT = 12 const loadDecks = () => { + api.get('/question-categories/') + .then(res => setCategories(Array.isArray(res.data) ? res.data : [])) + .catch(() => setCategories([])) api.get('/flashcards/').then(res => { setDecks(res.data) setLoading(false) @@ -121,6 +125,14 @@ export default function FlashcardsPage() { } catch { } } + const setDeckCategory = async (deckId, value) => { + const categoryId = value === '' ? null : Number(value) + try { + await api.patch(`/flashcards/${deckId}`, { category_id: categoryId }) + setDecks(prev => prev.map(d => d.id === deckId ? { ...d, category_id: categoryId } : d)) + } catch { } + } + const saveTitle = async (deckId, newTitle) => { const t = (newTitle || '').trim() if (!t) return @@ -250,6 +262,20 @@ export default function FlashcardsPage() {
{decks.map(deck => (
+
+ +
{editingDeckId === deck.id ? (
{ return Promise.resolve({ data: [] }) }) api.put.mockResolvedValue({ data: { linked: true } }) + api.patch.mockResolvedValue({ data: {} }) api.delete.mockResolvedValue({ data: null }) }) @@ -29,7 +30,30 @@ function renderPage() { } describe('cards page', () => { - it('uses the Cards label and honest empty deck state', async () => { + it('files a deck on the shared category tree', async () => { + const base = api.get.getMockImplementation() + api.get.mockImplementation(url => { + if (url === '/flashcards/') return Promise.resolve({ data: [ + { id: 1, title: 'Deck A', user_id: 1, card_count: 3, category_id: null, created_at: '2026-01-01T00:00:00' }, + ] }) + if (url === '/question-categories/') return Promise.resolve({ data: [ + { id: 7, name: 'Cardiology', parent_id: null, breadcrumbs: [{ id: 7, name: 'Cardiology' }] }, + ] }) + return base(url) + }) + render() + const select = await screen.findByLabelText('Category for Deck A') + // The options come from the same tree questions and articles use. + expect([...select.options].map(o => o.text)).toContain('Cardiology') + + await userEvent.selectOptions(select, '7') + await waitFor(() => expect(api.patch).toHaveBeenCalledWith('/flashcards/1', { category_id: 7 })) + + await userEvent.selectOptions(select, '') + await waitFor(() => expect(api.patch).toHaveBeenLastCalledWith('/flashcards/1', { category_id: null })) +}) + +it('uses the Cards label and honest empty deck state', async () => { renderPage() expect(await screen.findByRole('heading', { name: 'Cards' })).toBeInTheDocument() expect(await screen.findByText('No card decks yet. Create one from a document.')).toBeInTheDocument()