diff --git a/backend/alembic/env.py b/backend/alembic/env.py index 1289767..171c346 100644 --- a/backend/alembic/env.py +++ b/backend/alembic/env.py @@ -21,7 +21,7 @@ if _db_url: if config.config_file_name is not None: fileConfig(config.config_file_name) -from app.models import User, PDFDocument, Section, Quiz, Question, QuizAttempt, AttemptAnswer, ReminderSchedule, UserNote # noqa +from app.models import User, PDFDocument, Section, Quiz, Question, QuizAttempt, AttemptAnswer, UserNote # noqa from app.database import Base target_metadata = Base.metadata diff --git a/backend/alembic/versions/a1b2c3d4e5f6_drop_reminders.py b/backend/alembic/versions/a1b2c3d4e5f6_drop_reminders.py new file mode 100644 index 0000000..6d80e58 --- /dev/null +++ b/backend/alembic/versions/a1b2c3d4e5f6_drop_reminders.py @@ -0,0 +1,35 @@ +"""Remove quiz reminders. + +Revision ID: a1b2c3d4e5f6 +Revises: f0a1b2c3d4e5 +""" +import sqlalchemy as sa +from alembic import op +from sqlalchemy import inspect + +revision = "a1b2c3d4e5f6" +down_revision = "f0a1b2c3d4e5" +branch_labels = None +depends_on = None + + +def upgrade(): + # Emailed nudges to retake a quiz below 75%. The platform is about sessions + # and topics now, and a scheduler existed solely to send these. + if "reminder_schedules" in inspect(op.get_bind()).get_table_names(): + op.drop_table("reminder_schedules") + + +def downgrade(): + op.create_table( + "reminder_schedules", + sa.Column("id", sa.Integer, primary_key=True), + sa.Column("user_id", sa.Integer, sa.ForeignKey("users.id", ondelete="CASCADE"), nullable=False), + sa.Column("quiz_id", sa.Integer, sa.ForeignKey("quizzes.id", ondelete="CASCADE"), nullable=False), + sa.Column("next_reminder_at", sa.DateTime, nullable=False), + sa.Column("interval_days", sa.Integer, server_default="1"), + sa.Column("performance_score", sa.Float, server_default="0"), + sa.Column("is_active", sa.Boolean, server_default=sa.true()), + sa.Column("created_at", sa.DateTime, server_default=sa.func.now()), + sa.Column("updated_at", sa.DateTime, server_default=sa.func.now()), + ) diff --git a/backend/app/main.py b/backend/app/main.py index ad0dfa6..241492d 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -13,7 +13,6 @@ from app.database import engine, Base, SessionLocal from app.routers import auth, documents, quizzes, attempts, admin, tts, nextcloud, categories, questions, question_categories, favorites, teach, contact, tags, flashcards, courses, mobile, mynote, exams from app.routers import study_tools, uploads, articles, comments, share, collections, study_plans, media, search, ai_mode from app.utils.auth import get_password_hash -from app.utils.scheduler import start_scheduler, stop_scheduler def seed_admin(): @@ -574,15 +573,10 @@ async def lifespan(app: FastAPI): os.makedirs(settings.CHROMA_PERSIST_DIR, exist_ok=True) seed_admin() seed_default_models() - # Scheduler and backfill must only run in one worker to avoid duplicate jobs / race conditions - is_primary = _acquire_singleton_lock() - if is_primary: + # The backfill must run in one worker only, or several race each other. + if _acquire_singleton_lock(): backfill_embeddings() - start_scheduler() yield - # Shutdown - if is_primary: - stop_scheduler() app = FastAPI( diff --git a/backend/app/models/__init__.py b/backend/app/models/__init__.py index 4eba7fc..ce0a2b9 100644 --- a/backend/app/models/__init__.py +++ b/backend/app/models/__init__.py @@ -4,7 +4,6 @@ from app.models.section import Section from app.models.quiz import Quiz from app.models.question import Question from app.models.attempt import QuizAttempt, AttemptAnswer -from app.models.reminder import ReminderSchedule from app.models.ai_model_config import AIModelConfig from app.models.favorite import Favorite from app.models.user_note import UserNote @@ -23,7 +22,6 @@ __all__ = [ "Question", "QuizAttempt", "AttemptAnswer", - "ReminderSchedule", "AIModelConfig", "Favorite", "UserNote", diff --git a/backend/app/models/quiz.py b/backend/app/models/quiz.py index 1748e19..d191c0c 100644 --- a/backend/app/models/quiz.py +++ b/backend/app/models/quiz.py @@ -43,4 +43,3 @@ class Quiz(Base): viewonly=True, # mutations handled explicitly via QuizQuestionLink ) attempts = relationship("QuizAttempt", back_populates="quiz", cascade="all, delete-orphan") - reminders = relationship("ReminderSchedule", cascade="all, delete-orphan", foreign_keys="ReminderSchedule.quiz_id") diff --git a/backend/app/models/reminder.py b/backend/app/models/reminder.py deleted file mode 100644 index 90fd648..0000000 --- a/backend/app/models/reminder.py +++ /dev/null @@ -1,23 +0,0 @@ -from datetime import datetime - -from sqlalchemy import Column, Integer, Float, Boolean, DateTime, ForeignKey -from sqlalchemy.orm import relationship - -from app.database import Base - - -class ReminderSchedule(Base): - __tablename__ = "reminder_schedules" - - id = Column(Integer, primary_key=True, index=True) - user_id = Column(Integer, ForeignKey("users.id", ondelete="CASCADE"), nullable=False) - quiz_id = Column(Integer, ForeignKey("quizzes.id", ondelete="CASCADE"), nullable=False) - next_reminder_at = Column(DateTime, nullable=False) - interval_days = Column(Integer, default=1) - performance_score = Column(Float, default=0.0) - is_active = Column(Boolean, default=True) - created_at = Column(DateTime, default=datetime.utcnow) - updated_at = Column(DateTime, default=datetime.utcnow, onupdate=datetime.utcnow) - - user = relationship("User", back_populates="reminders") - quiz = relationship("Quiz", back_populates="reminders") diff --git a/backend/app/models/user.py b/backend/app/models/user.py index e088ad0..2b7099e 100644 --- a/backend/app/models/user.py +++ b/backend/app/models/user.py @@ -25,7 +25,6 @@ class User(Base): documents = relationship("PDFDocument", back_populates="user") quizzes = relationship("Quiz", back_populates="user") attempts = relationship("QuizAttempt", back_populates="user") - reminders = relationship("ReminderSchedule", back_populates="user") favorites = relationship("Favorite", back_populates="user", cascade="all, delete-orphan") note = relationship("UserNote", back_populates="user", cascade="all, delete-orphan", uselist=False) diff --git a/backend/app/routers/attempts.py b/backend/app/routers/attempts.py index 95dc2ab..b1d7663 100644 --- a/backend/app/routers/attempts.py +++ b/backend/app/routers/attempts.py @@ -176,13 +176,6 @@ def submit_attempt( percentage = (score / attempt.total_questions * 100) if attempt.total_questions > 0 else 0 - # Update reminder schedule (skip course quizzes) - if not is_course_quiz: - try: - from app.services.reminder_service import update_reminder_schedule - update_reminder_schedule(db, current_user.id, attempt.quiz_id, percentage) - except Exception: - logger.warning("Failed to update reminder schedule for quiz %d", attempt.quiz_id, exc_info=True) return AttemptDetail( id=attempt.id, @@ -401,7 +394,7 @@ def delete_attempt( db: Session = Depends(get_db), current_user: User = Depends(get_current_user), ): - """Delete own attempt + answers + reminders for that quiz. Cannot delete course quiz attempts.""" + """Delete own attempt and its answers. Cannot delete course quiz attempts.""" attempt = db.query(QuizAttempt).filter( QuizAttempt.id == attempt_id, QuizAttempt.user_id == current_user.id, @@ -416,11 +409,6 @@ def delete_attempt( quiz_id = attempt.quiz_id # Delete all attempts for this quiz by this user (wipe history) - from app.models.reminder import ReminderSchedule - db.query(ReminderSchedule).filter( - ReminderSchedule.user_id == current_user.id, - ReminderSchedule.quiz_id == quiz_id, - ).delete() db.delete(attempt) db.commit() diff --git a/backend/app/routers/media.py b/backend/app/routers/media.py index d2fe3a6..7fe3aef 100644 --- a/backend/app/routers/media.py +++ b/backend/app/routers/media.py @@ -16,6 +16,7 @@ from app.database import get_db from app.models.media import MediaAsset, MediaLibrary, MediaLibraryGrant, MediaTagLink from app.models.user import User from app.services import embedding_service, storage_service +from app.models.question_media import QuestionMedia from app.services.search_service import hybrid_ids from app.utils.auth import get_current_user, require_moderator @@ -23,7 +24,28 @@ router = APIRouter() log = logging.getLogger(__name__) MAX_IMAGE_BYTES = 12 * 1024 * 1024 -ALLOWED_TYPES = {"image/png", "image/jpeg", "image/gif", "image/webp", "image/svg+xml"} +# A murmur is thirty seconds of audio and a bedside clip is a few megabytes, so +# they get their own ceiling rather than being squeezed under the image one. +MAX_MEDIA_BYTES = 60 * 1024 * 1024 +IMAGE_TYPES = {"image/png", "image/jpeg", "image/gif", "image/webp", "image/svg+xml"} +# Heart sounds are the reason this exists: a murmur cannot be shown as a picture. +AUDIO_TYPES = {"audio/mpeg", "audio/mp3", "audio/wav", "audio/x-wav", + "audio/ogg", "audio/webm", "audio/mp4", "audio/aac"} +VIDEO_TYPES = {"video/mp4", "video/webm", "video/ogg", "video/quicktime"} +ALLOWED_TYPES = IMAGE_TYPES | AUDIO_TYPES | VIDEO_TYPES + + +def kind_for(content_type: str) -> str: + """Which of the three a file is. The player differs for each.""" + if content_type in AUDIO_TYPES: + return "audio" + if content_type in VIDEO_TYPES: + return "video" + return "image" + + +def limit_for(kind: str) -> int: + return MAX_IMAGE_BYTES if kind == "image" else MAX_MEDIA_BYTES def readable_libraries(db: Session, user: User) -> set[int] | None: @@ -159,7 +181,14 @@ def list_media( total = query.count() assets = query.order_by(MediaAsset.id.desc()).offset(offset).limit(limit).all() tags = _tags_for(db, [a.id for a in assets]) - return {"total": total, "images": [_asset_json(a, tags.get(a.id, [])) for a in assets]} + # How many questions point at each one. Renaming and moving are safe because + # the link is the id; deleting is the one act that cannot be undone, so the + # count travels with the row and is shown before anyone presses delete. + used = dict(db.query(QuestionMedia.media_id, func.count(QuestionMedia.id)).filter( + QuestionMedia.media_id.in_([a.id for a in assets] or [0]) + ).group_by(QuestionMedia.media_id).all()) if assets else {} + return {"total": total, "images": [ + {**_asset_json(a, tags.get(a.id, [])), "used_by": used.get(a.id, 0)} for a in assets]} @router.post("/upload", status_code=201) @@ -180,10 +209,13 @@ def upload_media( raise HTTPException(400, "Choose one of your libraries for this image") if file.content_type not in ALLOWED_TYPES: - raise HTTPException(400, "Upload a PNG, JPEG, GIF, WebP or SVG image") - data = file.file.read(MAX_IMAGE_BYTES + 1) - if len(data) > MAX_IMAGE_BYTES: - raise HTTPException(413, f"Keep images under {MAX_IMAGE_BYTES // (1024 * 1024)} MB") + raise HTTPException(400, "Upload an image (PNG, JPEG, GIF, WebP, SVG), " + "audio (MP3, WAV, OGG, M4A) or video (MP4, WebM, MOV)") + kind = kind_for(file.content_type) + ceiling = limit_for(kind) + data = file.file.read(ceiling + 1) + if len(data) > ceiling: + raise HTTPException(413, f"Keep {kind} under {ceiling // (1024 * 1024)} MB") if not data: raise HTTPException(400, "That file is empty") @@ -193,7 +225,7 @@ def upload_media( asset = MediaAsset( path=key, title=title or file.filename, caption=caption, alt_text=alt_text, - kind="image", library_id=library_id, user_id=current_user.id, + kind=kind, library_id=library_id, user_id=current_user.id, storage="s3" if storage_service.using_s3() else "local", byte_size=len(data), ) db.add(asset) @@ -259,8 +291,19 @@ def update_media(media_id: int, data: MediaUpdate, db: Session = Depends(get_db) @router.delete("/{media_id}", status_code=204) -def delete_media(media_id: int, db: Session = Depends(get_db), +def delete_media(media_id: int, force: bool = False, db: Session = Depends(get_db), current_user: User = Depends(require_moderator)): + """Delete an image. Refused while a question still uses it, unless forced. + + A question refers to a figure by id, so renaming it or moving it between + libraries never breaks anything. Deleting does, and silently โ€” the cascade + would take the link with it and the question would simply stop having a + picture. So the count has to be faced first. + """ + used = db.query(QuestionMedia).filter(QuestionMedia.media_id == media_id).count() + if used and not force: + raise HTTPException(409, f"{used} question{'s' if used > 1 else ''} still use this. " + "Detach it there first, or delete it anyway.") asset = db.get(MediaAsset, media_id) if not asset: raise HTTPException(404, "Image not found") diff --git a/backend/app/services/email_service.py b/backend/app/services/email_service.py index 3ba6d6e..02447a5 100644 --- a/backend/app/services/email_service.py +++ b/backend/app/services/email_service.py @@ -150,25 +150,3 @@ We received a request to reset your password. Click below to choose a new one. await _send(to_email, subject, _wrap(subject, md)) -async def send_reminder_email(email: str, user_name: str, quiz_title: str, score: float, next_date: str): - subject = f"Time to review: {quiz_title}" - note = "Great work โ€” keep the streak going! ๐ŸŒŸ" if score >= 75 else "A bit more practice will get you there. ๐Ÿ“š" - md = f"""# Time to review - -Hi **{user_name}**, - -Your spaced repetition schedule says it's time to revisit **{quiz_title}**. - -## Last score - -**{score:.0f}%** โ€” {note} - -Reviewing at spaced intervals is one of the most effective ways to retain medical knowledge long-term. Each session strengthens the memory trace. - -[button:Take Quiz Now]({settings.APP_URL}/quizzes) - ---- - -You're receiving this because you have active quiz reminders. To stop, disable reminders in your account settings. -""" - await _send(email, subject, _wrap(subject, md)) diff --git a/backend/app/services/reminder_service.py b/backend/app/services/reminder_service.py deleted file mode 100644 index 977eac9..0000000 --- a/backend/app/services/reminder_service.py +++ /dev/null @@ -1,61 +0,0 @@ -from datetime import datetime, timedelta - -from sqlalchemy.orm import Session - -from app.models.reminder import ReminderSchedule - -# SM-2 simplified intervals in days -INTERVALS = [1, 3, 7, 14, 30] - - -def update_reminder_schedule( - db: Session, - user_id: int, - quiz_id: int, - score_percentage: float, -): - """Update or create a reminder schedule based on quiz performance.""" - reminder = db.query(ReminderSchedule).filter( - ReminderSchedule.user_id == user_id, - ReminderSchedule.quiz_id == quiz_id, - ).first() - - if not reminder: - reminder = ReminderSchedule( - user_id=user_id, - quiz_id=quiz_id, - performance_score=score_percentage, - interval_days=INTERVALS[0], - next_reminder_at=datetime.utcnow() + timedelta(days=INTERVALS[0]), - is_active=True, - ) - db.add(reminder) - else: - reminder.performance_score = score_percentage - reminder.is_active = True - - # Find current interval index - current_idx = 0 - for i, interval in enumerate(INTERVALS): - if reminder.interval_days <= interval: - current_idx = i - break - - if score_percentage < 75: - # Below threshold: reset to shortest interval - new_idx = 0 - elif score_percentage < 90: - # Decent performance: advance one step - new_idx = min(current_idx + 1, len(INTERVALS) - 1) - else: - # Excellent performance: advance or deactivate - if current_idx >= len(INTERVALS) - 1: - reminder.is_active = False - db.commit() - return - new_idx = min(current_idx + 2, len(INTERVALS) - 1) - - reminder.interval_days = INTERVALS[new_idx] - reminder.next_reminder_at = datetime.utcnow() + timedelta(days=INTERVALS[new_idx]) - - db.commit() diff --git a/backend/app/utils/scheduler.py b/backend/app/utils/scheduler.py deleted file mode 100644 index 688e941..0000000 --- a/backend/app/utils/scheduler.py +++ /dev/null @@ -1,98 +0,0 @@ -import asyncio -import logging -from datetime import datetime - -from apscheduler.schedulers.background import BackgroundScheduler - -from app.database import SessionLocal -from app.models.reminder import ReminderSchedule -from app.models.user import User -from app.models.quiz import Quiz -from app.models.attempt import QuizAttempt - -logger = logging.getLogger(__name__) - -scheduler = BackgroundScheduler() - - -def check_and_send_reminders(): - """Check for due reminders and send emails.""" - db = SessionLocal() - try: - due_reminders = db.query(ReminderSchedule).filter( - ReminderSchedule.is_active == True, - ReminderSchedule.next_reminder_at <= datetime.utcnow(), - ).all() - - if not due_reminders: - return - - logger.info(f"Found {len(due_reminders)} due reminders") - - for reminder in due_reminders: - user = db.query(User).filter(User.id == reminder.user_id).first() - quiz = db.query(Quiz).filter(Quiz.id == reminder.quiz_id).first() - - if not user or not quiz or quiz.deleted_at or quiz.course_id: - reminder.is_active = False - continue - - # Deactivate if the user has no completed attempts left for this quiz - # (e.g., they deleted their attempts โ€” spaced repetition no longer applies). - has_completed_attempt = db.query(QuizAttempt.id).filter( - QuizAttempt.user_id == user.id, - QuizAttempt.quiz_id == quiz.id, - QuizAttempt.completed_at.isnot(None), - ).first() is not None - if not has_completed_attempt: - reminder.is_active = False - continue - - # Respect the user's opt-out preference (canonical source: Postgres). - if user.reminders_disabled: - continue - - # Send email asynchronously - try: - from app.services.email_service import send_reminder_email - loop = asyncio.new_event_loop() - loop.run_until_complete( - send_reminder_email( - email=user.email, - user_name=user.name, - quiz_title=quiz.title, - score=reminder.performance_score, - next_date=reminder.next_reminder_at.strftime("%Y-%m-%d"), - ) - ) - loop.close() - except Exception as e: - logger.error(f"Failed to send reminder {reminder.id}: {e}") - - # Schedule next reminder - from datetime import timedelta - reminder.next_reminder_at = datetime.utcnow() + timedelta(days=reminder.interval_days) - - db.commit() - except Exception as e: - logger.exception(f"Scheduler error: {e}") - finally: - db.close() - - -def start_scheduler(): - """Start the APScheduler with daily reminder check.""" - scheduler.add_job( - check_and_send_reminders, - "interval", - hours=24, - id="reminder_check", - replace_existing=True, - ) - scheduler.start() - logger.info("Scheduler started โ€” reminder check runs every 24 hours") - - -def stop_scheduler(): - if scheduler.running: - scheduler.shutdown() diff --git a/backend/scripts/link_articles.py b/backend/scripts/link_articles.py new file mode 100644 index 0000000..19b25fa --- /dev/null +++ b/backend/scripts/link_articles.py @@ -0,0 +1,116 @@ +"""Cross-reference the articles to each other. + +The marker system exists โ€” `[[7|Febrile seizures]]` resolves by id, survives a +rename, and shows a preview on hover โ€” and not one article used it. Every article +was written in isolation, so a piece on croup mentions stridor and epiglottitis +and offers no way to reach either. + +This reads what is already written and links it: where an article's prose names +another article, the first mention becomes a link. By id, so a later rename +cannot break it. + +Deliberately conservative, because a wrong link is worse than a missing one: + + * only the first mention in a section, so prose is not peppered with the same + link five times; + * whole words only, case-insensitively, longest title first โ€” "Otitis media + with effusion" wins over "Otitis media" where both would match; + * never inside an existing link, a marker, a heading, a code span or a table; + * never an article linking to itself; + * titles under four characters are skipped, since short ones collide. + + docker compose exec backend python -m scripts.link_articles + docker compose exec backend python -m scripts.link_articles --apply +""" +import re +import sys + +from app.database import SessionLocal +from app.models.article import Article + +MIN_TITLE = 4 +# Only the body of a section, never a heading, a link, a marker or code. +SKIP = re.compile(r"(\[\[[^\]]*\]\]|\[[^\]]*\]\([^)]*\)|`[^`]*`|^\s{0,3}#{1,6}.*$|^\s*\|.*$)", re.M) + + +def link_text(text: str, titles: list[tuple[str, int, str]], self_id: int) -> tuple[str, int]: + """Link the first mention of each other article. Returns (text, count).""" + if not text: + return text, 0 + + # Carve out everything that must not be touched, link the rest, put it back. + holes: list[str] = [] + + def stash(match: re.Match) -> str: + holes.append(match.group(0)) + return f"\x00{len(holes) - 1}\x00" + + working = SKIP.sub(stash, text) + + linked = 0 + for lowered, article_id, display in titles: + if article_id == self_id: + continue + pattern = re.compile(rf"(?= MIN_TITLE), + key=lambda row: -len(row[0]), + ) + print(f" articles: {len(articles)} linkable titles: {len(titles)}") + + touched = links = 0 + for article in articles: + sections = article.sections or [] + changed = False + for section in sections: + body, count = link_text(section.get("content"), titles, article.id) + if count: + section["content"] = body + links += count + changed = True + summary, count = link_text(article.summary, titles, article.id) + if count: + article.summary = summary + links += count + changed = True + if changed: + touched += 1 + if apply_changes: + from sqlalchemy.orm.attributes import flag_modified + + article.sections = sections + flag_modified(article, "sections") + + print(f" articles gaining links: {touched}") + print(f" links added : {links}") + if not apply_changes: + print("\n Re-run with --apply.") + return 0 + db.commit() + print("\n Linked by id, so renaming an article cannot break them.") + finally: + db.close() + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/backend/scripts/retitle_figures.py b/backend/scripts/retitle_figures.py index d90ef5c..9174bed 100644 --- a/backend/scripts/retitle_figures.py +++ b/backend/scripts/retitle_figures.py @@ -75,7 +75,29 @@ def main(): for asset, _old, new in changed: asset.title = new[:300] + db.flush() + + # A legend is what a reader sees in an explanation, so twelve figures all + # called "Pediatric Surgery" is twelve legends that say nothing. Only the + # repeats are numbered; a subject used once stays clean. + from collections import defaultdict + + by_title = defaultdict(list) + for asset in db.query(MediaAsset).order_by(MediaAsset.id).all(): + by_title[asset.title].append(asset) + questions = dict(db.execute(sa_text( + "SELECT media_id, MIN(question_id) FROM question_media GROUP BY media_id")).fetchall()) + numbered = 0 + for title, assets in by_title.items(): + if len(assets) < 2: + continue + for asset in assets: + question_id = questions.get(asset.id) + asset.title = (f"{title} ยท Q{question_id}" if question_id + else f"{title} ยท #{asset.id}")[:300] + numbered += 1 db.commit() + print(f" disambiguated: {numbered} repeated subjects") print(f"\n retitled: {len(changed)} (no link changed; the id is the link)") finally: db.close() diff --git a/docs/TODO.md b/docs/TODO.md index 06c9f23..386a9a5 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -36,11 +36,31 @@ Captured so nothing is lost while the article writing runs. - [ ] **Adaptive session** โ€” questions ordered by what would help most, with an explanation of how it decides. -### Settings +### Settings and access - [ ] **Revamp the settings page** โ€” called out as the worst screen in the app. Work out what belongs there at all before restyling it: account, theme, exam objective, voice, integrations, data. Several of those have drifted in because there was nowhere else to put them. +- [ ] **Access as a tree** โ€” grant a person articles, an image library, a + question folder, or a category branch, at any depth, with "everything" as + one option rather than a checkbox per row. Three grant tables already + exist (`category_grants`, `media_library_grants`, question folders); they + need one way in rather than three screens, and inheritance down the tree + so granting a branch grants what is under it. + +### Media +- [x] **Audio and video** โ€” a murmur cannot be shown as a picture. Uploads take + MP3/WAV/OGG/M4A and MP4/WebM/MOV alongside images, with their own 60 MB + ceiling, and each renders as what it is. +- [x] **Editing opens over the grid**, not inside a cell that stretches its + column and shoves the neighbours out of line. +- [x] **Every figure named by its subject**, with the question number appended + only where a subject repeats โ€” a legend a reader sees in an explanation + should not be twelve identical words. +- [x] **Deleting a figure in use is refused** unless forced, with the count + shown. Renaming and moving are safe because the link is the id. +- [ ] **Library filter in the picker** is in; a library *rail* in the picker + would be better on a wide screen. ### Editor and figures - [x] **Rich editing on the question page** โ€” no new platform needed: Milkdown diff --git a/frontend/src/components/ArticleReader.jsx b/frontend/src/components/ArticleReader.jsx index 0dabe50..02ac6b2 100644 --- a/frontend/src/components/ArticleReader.jsx +++ b/frontend/src/components/ArticleReader.jsx @@ -1,37 +1,14 @@ import { useEffect, useRef, useState } from 'react' -import ReactMarkdown from 'react-markdown' -import remarkGfm from 'remark-gfm' -import ArticleLink from './ArticleLink' -import { markdownImageUrl } from '../utils/uploads' - -// [[febrile-seizures]] and [[Febrile seizures|febrile-seizures]] are how an -// educator writes a cross-reference without having to know an article's numeric -// id, which changes nothing for them and everything for a link that has to last. -const WIKI_LINK = /\[\[([^\]|]+?)(?:\|([a-z0-9-]+))?\]\]/g -const expandWikiLinks = (text) => (text || '').replace(WIKI_LINK, (_m, label, slug) => - `[${label.trim()}](/articles/${(slug || label).trim().toLowerCase()})`) - -const internalSlug = (href) => { - const match = /^\/articles\/(?:s\/)?([a-z0-9-]+)\/?$/.exec(href || '') - return match ? match[1] : null -} - +import RichText from './RichText' +/** + * Article prose goes through the same renderer as everything else. + * + * This was a second Markdown pipeline with its own cross-reference regex, and it + * only understood the old slug form โ€” so `[[292|Febrile seizures]]`, which is + * what the linker writes, printed as literal brackets. + */ export function Markdown({ children, attemptId }) { - // Educator content renders as Markdown only; raw HTML is escaped, not executed. - return ( - , - a: ({ node, href, children: kids, ...props }) => { - const slug = internalSlug(href) - // A link into the library stays in the app and shows what it leads to; - // only links off the site get a new tab. - if (slug) return {kids} - return {kids} - }, - }}> - {expandWikiLinks(children)} - - ) + return } /** @@ -76,6 +53,16 @@ export default function ArticleReader({ article, activeSection = '', onOpenSecti const present = VIEWS.filter(v => everySection.some(sec => sec.variant === v.key)) const [view, setView] = useState(present[0]?.key || 'long') const allSections = everySection.filter(sec => sec.variant === view) + + // One section is not a contents page. Short and Clinical are usually a single + // section each, so collapsing them by default showed a heading and a blank + // space โ€” which read as the view having no content at all. + useEffect(() => { + const tops = allSections.filter(sec => !sec.parent_id) + if (tops.length === 1) { + setOpenIds(Object.fromEntries(allSections.map(sec => [sec.id, true]))) + } + }, [view, article]) // Element ids are prefixed because two readers can share a page, and a // duplicate id would point the pane's contents at the article behind it. const sectionElement = (secId) => root.current?.querySelector(`#section-${idPrefix}${secId}`) diff --git a/frontend/src/components/ImagePicker.css b/frontend/src/components/ImagePicker.css index b086a63..bcb552e 100644 --- a/frontend/src/components/ImagePicker.css +++ b/frontend/src/components/ImagePicker.css @@ -53,3 +53,12 @@ .ip-panel { max-height: 92vh; border-radius: 14px 14px 0 0; } .ip-grid { grid-template-columns: repeat(auto-fill, minmax(140px, 1fr)); } } + +.ip-library { + min-height: 40px; padding: 8px 11px; cursor: pointer; + border: 1px solid var(--border); border-radius: 9px; + background: var(--input-bg); color: var(--text); font: inherit; font-size: 0.85rem; +} +/* Audio has no thumbnail to speak of; it gets the row rather than a square. */ +.ip-thumb audio, .media-thumb audio { width: 100%; } +.ip-thumb video, .media-thumb video { max-width: 100%; max-height: 100%; } diff --git a/frontend/src/components/ImagePicker.jsx b/frontend/src/components/ImagePicker.jsx index 4d0f485..6068d7e 100644 --- a/frontend/src/components/ImagePicker.jsx +++ b/frontend/src/components/ImagePicker.jsx @@ -1,6 +1,6 @@ import { useCallback, useEffect, useRef, useState } from 'react' import api from '../api/client' -import { uploadUrl } from '../utils/uploads' +import MediaTile from './MediaTile' import './ImagePicker.css' /** @@ -16,6 +16,10 @@ import './ImagePicker.css' export default function ImagePicker({ open, onPick, onClose, title = 'Choose an image' }) { const [query, setQuery] = useState('') const [images, setImages] = useState([]) + const [libraries, setLibraries] = useState([]) + // Narrowing by library is how you find a figure when the bank holds hundreds: + // "the radiology one" is a far better first cut than a search term. + const [library, setLibrary] = useState('all') const [loading, setLoading] = useState(false) const [error, setError] = useState('') const [busy, setBusy] = useState(false) @@ -23,11 +27,14 @@ export default function ImagePicker({ open, onPick, onClose, title = 'Choose an const load = useCallback(() => { setLoading(true) - api.get('/media/', { params: query.trim() ? { q: query.trim(), limit: 40 } : { limit: 40 } }) + const params = { limit: 40 } + if (query.trim()) params.q = query.trim() + if (library !== 'all') params.library_id = library + api.get('/media/', { params }) .then(res => setImages(res.data?.images || [])) .catch(() => setError('Could not load the image bank')) .finally(() => setLoading(false)) - }, [query]) + }, [query, library]) useEffect(() => { if (!open) return @@ -35,6 +42,11 @@ export default function ImagePicker({ open, onPick, onClose, title = 'Choose an return () => clearTimeout(timer) }, [open, load, query]) + useEffect(() => { + if (!open || libraries.length) return + api.get('/media/libraries').then(res => setLibraries(res.data || [])).catch(() => setLibraries([])) + }, [open, libraries.length]) + useEffect(() => { if (!open) return const onKey = (event) => { if (event.key === 'Escape') onClose() } @@ -73,8 +85,17 @@ export default function ImagePicker({ open, onPick, onClose, title = 'Choose an
setQuery(e.target.value)} - placeholder="Search by what the image shows" aria-label="Search images" /> - + placeholder="Search by what it shows" aria-label="Search images" /> + + {libraries.length > 0 && ( + + )}
@@ -90,7 +111,7 @@ export default function ImagePicker({ open, onPick, onClose, title = 'Choose an
  • - deleteAttempt(a.attempt_id)} - /> - - - ))} - - - ) -} diff --git a/frontend/src/components/MediaTile.jsx b/frontend/src/components/MediaTile.jsx new file mode 100644 index 0000000..a30563d --- /dev/null +++ b/frontend/src/components/MediaTile.jsx @@ -0,0 +1,19 @@ +import { uploadUrl } from '../utils/uploads' + +/** + * One item from the bank, drawn as whatever it is. + * + * Heart sounds are the reason this exists: an audio file rendered as an + * is a broken icon, and a video rendered as one is the same. The kind decides + * the element; everything else about the tile stays identical. + */ +export default function MediaTile({ item, className = '', controls = true }) { + const src = uploadUrl(item.path) + if (item.kind === 'audio') { + return
  • - {image.alt_text + {/* The id is what you type into a question, so it has to be readable without opening anything. */} #{image.id} @@ -209,8 +208,18 @@ export default function MediaPage() { {confirmDelete === image.id && (
    - Delete image #{image.id}? Questions using it will lose their picture. - + {/* Renaming and moving are safe โ€” a question refers to a + figure by id. Deleting is the one act that breaks the + link, so the count is faced before the button. */} + + {image.used_by > 0 + ? `${image.used_by} question${image.used_by > 1 ? 's' : ''} use this. Deleting takes the figure off ${image.used_by > 1 ? 'them' : 'it'}.` + : 'Nothing uses this. Delete it?'} + +
    )} @@ -236,7 +245,7 @@ export default function MediaPage() {
    - {image.alt_text +
    diff --git a/frontend/src/pages/MediaPage.test.jsx b/frontend/src/pages/MediaPage.test.jsx index b993a87..69a2931 100644 --- a/frontend/src/pages/MediaPage.test.jsx +++ b/frontend/src/pages/MediaPage.test.jsx @@ -74,14 +74,24 @@ describe('image bank', () => { await waitFor(() => expect(api.get).toHaveBeenCalledWith('/media/', { params: { library_id: 1 } })) }) - it('warns what a deletion costs before doing it', async () => { + it('counts what a deletion costs before doing it', async () => { + mockApi([{ ...images[0], used_by: 3 }, { ...images[1], used_by: 0 }]) mount() await screen.findByText('#11') api.delete.mockResolvedValue({}) + + // Renaming and moving are safe โ€” a question refers to a figure by id. + // Deleting is the one act that breaks the link, so the count is faced first. await userEvent.click(screen.getByRole('button', { name: 'Delete image 11' })) - expect(screen.getByRole('alert')).toHaveTextContent('Questions using it will lose their picture') + expect(screen.getByRole('alert')).toHaveTextContent('3 questions use this') + await userEvent.click(screen.getByRole('button', { name: 'Delete anyway' })) + await waitFor(() => expect(api.delete).toHaveBeenCalledWith('/media/11', { params: { force: true } })) + + // Nothing pointing at it needs no forcing. + await userEvent.click(screen.getByRole('button', { name: 'Delete image 12' })) + expect(screen.getByRole('alert')).toHaveTextContent('Nothing uses this') await userEvent.click(screen.getByRole('button', { name: 'Delete' })) - await waitFor(() => expect(api.delete).toHaveBeenCalledWith('/media/11')) + await waitFor(() => expect(api.delete).toHaveBeenCalledWith('/media/12', { params: {} })) }) it('keeps library creation and deletion to moderators', async () => {