From 8614792bdf17547d651d1d1d36f56b600671e80c Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 12 Sep 2026 04:41:27 +0200 Subject: [PATCH] fix: the other registration form, our own transcriber first, and a rail that stops answering the wrong question MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Invite-only was set and the sign-up form had nowhere to type a code. There are two registration forms — /register and the modal on the landing page — and only the first had been taught about invite codes. The modal is the one most people meet, so turning the gate on failed everybody with "an invite code is required" and no field to satisfy it. It now asks the same signup-policy question and shows the same field. Dictation records to our own transcriber first and falls back to the browser's recogniser only where recording is unavailable. It was the other way round for speed, but the browser's speech stack announces itself to the user in ways we do not control — Firefox interrupts the page with a warning about a missing Speech Dispatcher library, which is alarming and is not about us. A draft question could be promoted into the bank with no category. That question would reach nothing: no discipline, no organ system, no relevance, no row on any tab of the analysis — in the bank and invisible to every page that counts. Promotion now refuses, before an id is spent. And "Your overall analysis" is out of the session rail. It put lifetime figures one click away while you were standing in front of a single session, which is the thing that was supposed to have moved to the Performance tab. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN --- backend/app/services/draft_questions.py | 9 ++- backend/tests/test_draft_questions.py | 52 +++++++++++++++- frontend/src/components/AnalysisShell.css | 14 +---- frontend/src/components/SessionRail.jsx | 19 +++--- frontend/src/components/SessionRail.test.jsx | 9 ++- frontend/src/hooks/useDictation.js | 42 +++++++------ frontend/src/hooks/useDictation.test.jsx | 62 ++++++++++--------- .../src/pages/AnalysisSessionPage.test.jsx | 7 ++- frontend/src/pages/LandingPage.jsx | 39 +++++++++++- 9 files changed, 171 insertions(+), 82 deletions(-) diff --git a/backend/app/services/draft_questions.py b/backend/app/services/draft_questions.py index d236d14..e35ea4d 100644 --- a/backend/app/services/draft_questions.py +++ b/backend/app/services/draft_questions.py @@ -53,7 +53,14 @@ def accept(db: Session, draft: DraftQuestion, batch: DraftBatch, user: User) -> raise HTTPException(400, f"Not ready: {', '.join(faults)}") category_id = category_for(draft, batch) - if category_id is not None and not db.get(QuestionCategory, category_id): + # A question with no category reaches nothing: no discipline, no organ + # system, no relevance, and no row on any tab of the analysis. It would sit + # in the bank and be invisible to every page that counts. The batch carries + # one so that saying it once covers the whole extraction. + if category_id is None: + raise HTTPException(400, "Give this batch a category before accepting from it — " + "a question filed nowhere is invisible to the analysis") + if not db.get(QuestionCategory, category_id): raise HTTPException(400, "The category this batch files into no longer exists") question = Question( diff --git a/backend/tests/test_draft_questions.py b/backend/tests/test_draft_questions.py index 2d92855..4de962e 100644 --- a/backend/tests/test_draft_questions.py +++ b/backend/tests/test_draft_questions.py @@ -7,19 +7,20 @@ corrected would be sitting in the bank while it was wrong. """ import unittest -from fastapi import FastAPI +from fastapi import FastAPI, HTTPException from fastapi.testclient import TestClient from sqlalchemy import create_engine from sqlalchemy.orm import Session from sqlalchemy.pool import StaticPool -import test_quiz_builder # noqa: F401 — imports every model +import test_quiz_builder as fixtures # noqa: F401 — imports every model from app.database import Base, get_db from app.models.draft_question import DraftBatch, DraftQuestion from app.models.question import Question from app.models.question_category import QuestionCategory from app.models.user import User from app.routers import drafts +from app.services import draft_questions from app.utils.auth import get_current_user @@ -161,3 +162,50 @@ class DraftTests(unittest.TestCase): if __name__ == "__main__": unittest.main() + + +class UnfiledDraftTests(unittest.TestCase): + """A question filed nowhere is invisible to every page that counts it. + + No category means no discipline, no organ system, no relevance and no row + on any tab of the analysis — it would sit in the bank and be seen by + nobody. The batch carries the category so saying it once covers the whole + extraction. + """ + + def setUp(self): + self.bank = fixtures.BuilderTests() + self.bank.setUp() + self.db = self.bank.db + + def tearDown(self): + self.bank.tearDown() + + def draft_in(self, category_id): + batch = DraftBatch(title="Unfiled batch", category_id=category_id, + created_by=1, status="open") + self.db.add(batch) + self.db.flush() + draft = DraftQuestion( + batch_id=batch.id, question_text="A 2-year-old with a barking cough.", + question_type="mcq", options=["Croup", "Epiglottitis"], + correct_answer="Croup", status="pending") + self.db.add(draft) + self.db.commit() + return draft, batch + + def test_an_unfiled_draft_is_refused_and_takes_no_id(self): + draft, batch = self.draft_in(None) + before = self.db.query(Question).count() + with self.assertRaises(HTTPException) as caught: + draft_questions.accept(self.db, draft, batch, self.bank.owner) + self.assertEqual(caught.exception.status_code, 400) + self.assertIn("category", caught.exception.detail) + # Nothing was created, so no permanent id was spent on a refusal. + self.assertEqual(self.db.query(Question).count(), before) + self.assertEqual(draft.status, "pending") + + def test_a_filed_draft_carries_the_batch_s_category_through(self): + draft, batch = self.draft_in(1) + question = draft_questions.accept(self.db, draft, batch, self.bank.owner) + self.assertEqual(question.question_category_id, 1) diff --git a/frontend/src/components/AnalysisShell.css b/frontend/src/components/AnalysisShell.css index 5db557b..f132d8d 100644 --- a/frontend/src/components/AnalysisShell.css +++ b/frontend/src/components/AnalysisShell.css @@ -64,15 +64,6 @@ } .ax-rail-show:hover { color: var(--primary); border-color: var(--primary); } -.ax-rail-all { - display: flex; flex-direction: column; gap: 3px; padding: 13px 16px; - text-decoration: none; color: var(--text); border-bottom: 1px solid var(--border); - flex-shrink: 0; -} -.ax-rail-all:hover { background: var(--bg); } -.ax-rail-all strong { font-size: 0.88rem; font-weight: 650; } -.ax-rail-all span { font-size: 0.76rem; color: var(--text-muted); } - .ax-rail-search { flex-shrink: 0; margin: 10px 16px; padding: 8px 11px; /* 16px on touch: iOS zooms the page in on any smaller font when a field @@ -103,10 +94,10 @@ .ax-rail-empty { margin: 0; padding: 14px 16px; font-size: 0.83rem; color: var(--text-muted); } /* Which one you are reading. */ -.ax-rail-all.active, .ax-rail-list a.active { +.ax-rail-list a.active { background: var(--option-sel-bg); box-shadow: inset 3px 0 0 var(--primary); } -.ax-rail-all.active strong, .ax-rail-list a.active .ax-rail-title { color: var(--primary); } +.ax-rail-list a.active .ax-rail-title { color: var(--primary); } /* ── Narrow ───────────────────────────────────────────────────────── */ /* Below 1000px the rail stops being a column. It becomes a band above the @@ -125,7 +116,6 @@ } .ax-layout.is-collapsed .ax-rail { display: flex; max-height: none; } .ax-layout.is-collapsed .ax-rail-list, - .ax-layout.is-collapsed .ax-rail-all, .ax-layout.is-collapsed .ax-rail-search { display: none; } .ax-rail-head { border-bottom: 0; } .ax-rail-show { display: none; } diff --git a/frontend/src/components/SessionRail.jsx b/frontend/src/components/SessionRail.jsx index 406cfe3..0608c4f 100644 --- a/frontend/src/components/SessionRail.jsx +++ b/frontend/src/components/SessionRail.jsx @@ -6,9 +6,15 @@ import SessionProgress from './SessionProgress' * The list of sessions, alongside whatever session you are reading. * * Sessions and analysis are the same subject, so this is the navigation for - * both: the overall picture sits at the top, every session follows, and the - * one you are looking at is marked. It replaced a separate sessions page, - * which listed the same rows a second time under a different heading. + * both: every session, with the one you are looking at marked. It replaced a + * separate sessions page, which listed the same rows a second time under a + * different heading. + * + * The overall picture is not in here. It used to sit at the top of the rail, + * which put lifetime figures one click away while you were standing in front + * of a single session — a question nobody asked there. It lives on the + * Analysis page's Performance tab, which is the page for it, and the section + * bar is how you get back to it. * * Nothing is truncated. A learner asking "what have I done" wants the whole * answer, and the search box is what narrows it. Styling lives in @@ -33,13 +39,6 @@ export default function SessionRail({ sessions, open, onToggle, loading }) { {open && ( <> - {/* `end` so this only lights up on /sessions itself, not on every - session underneath it. */} - - Your overall analysis - Everything you have practised - - {sessions.length > 6 && ( setQuery(e.target.value)} diff --git a/frontend/src/components/SessionRail.test.jsx b/frontend/src/components/SessionRail.test.jsx index d77fb6f..5e5a111 100644 --- a/frontend/src/components/SessionRail.test.jsx +++ b/frontend/src/components/SessionRail.test.jsx @@ -20,10 +20,13 @@ const mount = (props = {}) => render( ) describe('the session rail', () => { - it('puts the overall analysis above the sessions themselves', () => { + it('holds sessions and nothing else', () => { mount() - expect(screen.getByRole('link', { name: /Your overall analysis/ })) - .toHaveAttribute('href', '/sessions') + // The overall picture used to sit at the top of this rail, which put + // lifetime figures one click away while you were standing in front of a + // single session. It lives on the Analysis page's Performance tab. + expect(screen.queryByRole('link', { name: /overall analysis/ })).not.toBeInTheDocument() + expect(screen.getByRole('link', { name: /Board Review I\b/ })).toBeInTheDocument() }) it('never launches a test — an unsat session opens its overview instead', () => { diff --git a/frontend/src/hooks/useDictation.js b/frontend/src/hooks/useDictation.js index 32ed506..1fab84e 100644 --- a/frontend/src/hooks/useDictation.js +++ b/frontend/src/hooks/useDictation.js @@ -4,14 +4,15 @@ import api from '../api/client' /** * Speaking instead of typing. * - * Two ways to do it, and the browser's own is tried first: `SpeechRecognition` - * transcribes as you talk, costs nothing, and needs no round trip. Where it is - * missing — Firefox, and every browser on iOS that is not Safari — the audio - * is recorded and sent to our own transcriber instead, which is slower but - * works everywhere a microphone does. + * Two ways to do it, and ours is tried first: record the audio and send it to + * our own transcriber. It is a round trip, but it behaves the same in every + * browser, and the browser's own speech stack announces itself to the user in + * ways we do not control — Firefox interrupts the page with a warning about a + * missing system library, which is alarming and is not about us. * - * The fallback is also taken when the browser's recogniser fails mid-sentence, - * because a half-finished dictation is worse than a slow one. + * `SpeechRecognition` is the fallback, for a browser that cannot record. It + * transcribes as you talk and costs nothing, so it is a fine second choice + * where recording is unavailable. */ /** Read when asked, not when this module loads: a constant captured at import @@ -50,11 +51,6 @@ export default function useDictation({ onText, onError } = {}) { /** Record, then send it to our own transcriber. */ const record = useCallback(async () => { - if (!canRecord()) { - onError?.('This browser cannot record audio. Try typing instead.') - cleanup() - return - } try { stream.current = await navigator.mediaDevices.getUserMedia({ audio: true }) } catch { @@ -86,11 +82,13 @@ export default function useDictation({ onText, onError } = {}) { setListening(true) }, [cleanup, onError, onText]) - const start = useCallback(async () => { - if (listening || transcribing) return + /** The browser's own recogniser, where recording is not available. */ + const listen = useCallback(async () => { const Engine = recognition() - if (!Engine) { await record(); return } - + if (!Engine) { + onError?.('This browser cannot record audio. Try typing instead.') + return + } heard.current = '' const engine = new Engine() engine.lang = navigator.language || 'en-US' @@ -108,8 +106,7 @@ export default function useDictation({ onText, onError } = {}) { onError?.('No microphone. Check the permission for this site.') return } - if (canRecord()) record() - else onError?.('Could not hear anything. Try typing instead.') + onError?.('Could not hear anything. Try typing instead.') } engine.onend = () => { setListening(false) @@ -119,7 +116,14 @@ export default function useDictation({ onText, onError } = {}) { recogniser.current = engine engine.start() setListening(true) - }, [cleanup, listening, onError, onText, record, transcribing]) + }, [cleanup, onError, onText]) + + const start = useCallback(async () => { + if (listening || transcribing) return + // Ours first. + if (canRecord()) await record() + else await listen() + }, [listen, listening, record, transcribing]) const stop = useCallback(() => { if (recogniser.current) { recogniser.current.stop(); return } diff --git a/frontend/src/hooks/useDictation.test.jsx b/frontend/src/hooks/useDictation.test.jsx index e29c987..5e0d234 100644 --- a/frontend/src/hooks/useDictation.test.jsx +++ b/frontend/src/hooks/useDictation.test.jsx @@ -31,6 +31,7 @@ const withBrowserRecogniser = () => { } const withMediaRecorderOnly = () => { + engines = [] delete window.SpeechRecognition delete window.webkitSpeechRecognition const track = { stop: vi.fn() } @@ -56,7 +57,27 @@ afterEach(() => { delete navigator.mediaDevices }) -it('uses the browser recogniser when there is one, and never calls the server', async () => { +it('records and sends to our own transcriber, even where the browser has its own', async () => { + // Ours first. The browser's speech stack announces itself to the user in + // ways we do not control — Firefox interrupts the page with a warning about + // a missing system library, which is alarming and is not about us. + withMediaRecorderOnly() + withBrowserRecogniser() + api.post.mockResolvedValue({ data: { text: 'A neonate with bilious vomiting' } }) + render() + await userEvent.click(screen.getByRole('button')) + await waitFor(() => expect(screen.getByRole('button')).toHaveTextContent('listening')) + await userEvent.click(screen.getByRole('button')) + await waitFor(() => expect(api.post).toHaveBeenCalledWith('/tts/transcribe', expect.any(FormData))) + await waitFor(() => expect(document.title).toBe('A neonate with bilious vomiting')) + // The browser recogniser was never constructed, so it never spoke up. + expect(engines).toHaveLength(0) +}) + +it('uses the browser recogniser only where there is no recording', async () => { + engines = [] + delete navigator.mediaDevices + delete window.MediaRecorder withBrowserRecogniser() render() await userEvent.click(screen.getByRole('button')) @@ -66,42 +87,23 @@ it('uses the browser recogniser when there is one, and never calls the server', engine.onresult({ resultIndex: 0, results: [{ isFinal: true, 0: { transcript: 'What causes stridor' } }] }) await userEvent.click(screen.getByRole('button')) await waitFor(() => expect(document.title).toBe('What causes stridor')) - // Free and local: nothing was uploaded. expect(api.post).not.toHaveBeenCalled() }) -it('records and sends to our own transcriber where the browser has none', async () => { - withMediaRecorderOnly() - api.post.mockResolvedValue({ data: { text: 'A neonate with bilious vomiting' } }) - render() - await userEvent.click(screen.getByRole('button')) - await waitFor(() => expect(screen.getByRole('button')).toHaveTextContent('listening')) - await userEvent.click(screen.getByRole('button')) - await waitFor(() => expect(api.post).toHaveBeenCalledWith('/tts/transcribe', expect.any(FormData))) - await waitFor(() => expect(document.title).toBe('A neonate with bilious vomiting')) -}) - -it('falls back to recording when the browser recogniser fails mid-sentence', async () => { - withBrowserRecogniser() - const track = { stop: vi.fn() } - navigator.mediaDevices = { getUserMedia: vi.fn().mockResolvedValue({ getTracks: () => [track] }) } - window.MediaRecorder = function Recorder() { this.start = vi.fn(); this.state = 'recording'; return this } - render() - await userEvent.click(screen.getByRole('button')) - engines[engines.length - 1].onerror({ error: 'network' }) - // A half-finished dictation is worse than a slow one. - await waitFor(() => expect(navigator.mediaDevices.getUserMedia).toHaveBeenCalled()) -}) - it('a refused microphone is an answer, not a fault to route around', async () => { - withBrowserRecogniser() - navigator.mediaDevices = { getUserMedia: vi.fn() } const onError = vi.fn() + // Recording is available and the learner says no. Both halves must be + // present or the hook would take the recogniser instead, which is a + // different test. + window.MediaRecorder = function Recorder() { this.start = vi.fn(); return this } + navigator.mediaDevices = { getUserMedia: vi.fn().mockRejectedValue(new Error('denied')) } + withBrowserRecogniser() render() await userEvent.click(screen.getByRole('button')) - engines[engines.length - 1].onerror({ error: 'not-allowed' }) - expect(onError).toHaveBeenCalledWith('No microphone. Check the permission for this site.') - expect(navigator.mediaDevices.getUserMedia).not.toHaveBeenCalled() + // No silent hop to the browser recogniser to ask a second time. + await waitFor(() => expect(onError).toHaveBeenCalledWith( + 'No microphone. Check the permission for this site.')) + expect(engines).toHaveLength(0) }) it('offers nothing at all where neither exists', () => { diff --git a/frontend/src/pages/AnalysisSessionPage.test.jsx b/frontend/src/pages/AnalysisSessionPage.test.jsx index 38a9c2b..3ba6b86 100644 --- a/frontend/src/pages/AnalysisSessionPage.test.jsx +++ b/frontend/src/pages/AnalysisSessionPage.test.jsx @@ -121,11 +121,12 @@ describe('a session that has been sat', () => { expect(screen.queryByText(/still unanswered/)).not.toBeInTheDocument() }) - it('keeps the session rail beside it', async () => { + it('keeps the session rail beside it, holding only sessions', async () => { mountAttempt() const rail = await screen.findByRole('complementary', { name: 'Sessions' }) - expect(within(rail).getByRole('link', { name: /Your overall analysis/ })) - .toHaveAttribute('href', '/sessions') + expect(within(rail).getByText('My Custom Test')).toBeInTheDocument() + // Lifetime figures are not the question in front of one session. + expect(within(rail).queryByRole('link', { name: /overall analysis/ })).not.toBeInTheDocument() }) }) diff --git a/frontend/src/pages/LandingPage.jsx b/frontend/src/pages/LandingPage.jsx index 59e7e5b..723df08 100644 --- a/frontend/src/pages/LandingPage.jsx +++ b/frontend/src/pages/LandingPage.jsx @@ -183,6 +183,20 @@ function AuthModal({ mode, onClose, onSwitch }) { const [resendSent, setResendSent] = useState(false) const [resending, setResending] = useState(false) const [registered, setRegistered] = useState(false) + // Whether this site is invite only. The register page asks the same + // question; this form is the other door and used not to ask at all, so + // turning the gate on left everyone here failing with "an invite code is + // required" and nowhere to put one. + const [inviteRequired, setInviteRequired] = useState(false) + const [inviteCode, setInviteCode] = useState('') + + useEffect(() => { + let live = true + api.get('/auth/signup-policy') + .then(res => { if (live) setInviteRequired(!!res.data?.invite_required) }) + .catch(() => {}) + return () => { live = false } + }, []) const reset = () => { setError(''); setUnverified(false); setResendSent(false); setRegistered(false) } const switchMode = (m) => { reset(); setName(''); setEmail(''); setPassword(''); onSwitch(m) } @@ -206,7 +220,11 @@ function AuthModal({ mode, onClose, onSwitch }) { setError('') setLoading(true) try { - const res = await api.post('/auth/register', { email, password, name, turnstile_token: turnstileToken || null }) + const res = await api.post('/auth/register', { + email, password, name, + turnstile_token: turnstileToken || null, + invite_code: inviteCode.trim() || null, + }) if (res.data.requires_verification) { setRegistered(true) } else { @@ -324,8 +342,25 @@ function AuthModal({ mode, onClose, onSwitch }) { setPassword(e.target.value)} required minLength={8} /> + {inviteRequired && ( +
+ + {/* Asked for only where it is needed. The form never says + whether a code is valid before the account is made — + that would be a place to guess them. */} + setInviteCode(e.target.value.toUpperCase())} /> + + This site is invite only. + +
+ )} -