diff --git a/backend/app/models/__init__.py b/backend/app/models/__init__.py index 17e36c9..3ac7fd4 100644 --- a/backend/app/models/__init__.py +++ b/backend/app/models/__init__.py @@ -8,6 +8,7 @@ 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 +from app.models.lab_reference import LabReference __all__ = [ "User", @@ -21,4 +22,5 @@ __all__ = [ "AIModelConfig", "Favorite", "UserNote", + "LabReference", ] diff --git a/backend/app/routers/attempts.py b/backend/app/routers/attempts.py index ac5e363..92e1ddc 100644 --- a/backend/app/routers/attempts.py +++ b/backend/app/routers/attempts.py @@ -227,7 +227,7 @@ def list_attempts( class ProgressSave(BaseModel): quiz_id: int attempt_id: int - answers: dict # {question_id: answer} + answers: dict[int, str] # {question_id: answer}; reject malformed cached UI values current_idx: int mode: str voice: str | None = None @@ -282,6 +282,7 @@ def save_progress( })) except Exception: logger.warning("Redis unavailable for progress save", exc_info=True) + raise HTTPException(503, "Progress could not be saved. Keep this tab open and retry.") return {"saved": True} @@ -373,7 +374,7 @@ def get_progress( return saved except Exception: logger.warning("Redis unavailable for progress retrieval", exc_info=True) - return None + raise HTTPException(503, "Saved progress is temporarily unavailable. Retry before starting.") @router.delete("/progress/{attempt_id}", status_code=204) diff --git a/backend/app/schemas/quiz.py b/backend/app/schemas/quiz.py index 19f2263..7d047eb 100644 --- a/backend/app/schemas/quiz.py +++ b/backend/app/schemas/quiz.py @@ -60,7 +60,7 @@ class QuizResponse(BaseModel): is_shared: int = 0 questions_per_attempt: int | None = None course_id: int | None = None - allow_review: int = 1 + allow_review: int | None = 1 max_attempts: int | None = None attempt_mode: str | None = None diff --git a/backend/tests/test_study_tools.py b/backend/tests/test_study_tools.py index a141457..8b6101e 100644 --- a/backend/tests/test_study_tools.py +++ b/backend/tests/test_study_tools.py @@ -77,6 +77,40 @@ class StudyToolTests(unittest.TestCase): self.assertEqual(empty['sample_size'], 0) self.assertTrue(all(row['percentage'] == 0 for row in empty['options'])) + def test_nullable_review_setting_serializes_and_course_stays_exam_only(self): + self.bank.db.query(Quiz).filter(Quiz.id == 1).update({'allow_review': None}) + self.bank.db.query(Quiz).filter(Quiz.id == 2).update({'allow_review': None, 'mode': 'learning', 'questions_count': 1}) + self.bank.db.add(fixtures.CourseEnrollment(course_id=1, user_id=2)) + self.bank.db.commit() + self.assertEqual(self.client.get('/quizzes/').status_code, 200) + response = self.client.get('/quizzes/1') + self.assertEqual(response.status_code, 200, response.text) + self.assertIsNone(response.json()['allow_review']) + self.bank.user = self.bank.peer + aid = self.start(2, 'study') + self.assertEqual(self.bank.db.get(QuizAttempt, aid).mode, 'exam') + response = self.client.get(f'/quizzes/2?attempt_id={aid}') + self.assertEqual(response.status_code, 200, response.text) + self.assertIsNone(response.json()['allow_review']) + self.assertNotIn('correct_answer', response.json()['questions'][0]) + self.assertEqual(self.client.get(f'/quizzes/2?attempt_id={aid}&study=true').status_code, 403) + + def test_progress_outage_returns_failure_not_empty_or_saved_success(self): + quiz_id = self.bank.generate(is_shared=True, category_ids=[1]).json()['id'] + aid = self.start(quiz_id, 'study') + redis = Mock() + redis.from_url.return_value.get.side_effect = ConnectionError('Synthetic cache outage') + redis.from_url.return_value.setex.side_effect = ConnectionError('Synthetic cache outage') + with patch.dict(sys.modules, {'redis': redis}): + self.assertEqual(self.client.get(f'/attempts/progress?quiz_id={quiz_id}').status_code, 503) + response = self.client.post('/attempts/progress', json={'quiz_id': quiz_id, 'attempt_id': aid, + 'answers': {'1': 'yes'}, 'current_idx': 0, 'mode': 'study'}) + self.assertEqual(response.status_code, 503, response.text) + self.assertIsNone(self.bank.db.get(QuizAttempt, aid).completed_at) + malformed = self.client.post('/attempts/progress', json={'quiz_id': quiz_id, 'attempt_id': aid, + 'answers': {'1': {'unexpected': 'object'}}, 'current_idx': 0, 'mode': 'study'}) + self.assertEqual(malformed.status_code, 422) + def test_lab_reference_permissions_validation_and_publication(self): payload = dict(name='Example test', group='Blood', reference_range='Example interval', units='example units', age_group='Defined study population', specimen='Serum', source='Educator-supplied source', source_url='https://example.test/reference') diff --git a/backend/tests/test_study_tools_migration.py b/backend/tests/test_study_tools_migration.py index 277c4dc..ec692bf 100644 --- a/backend/tests/test_study_tools_migration.py +++ b/backend/tests/test_study_tools_migration.py @@ -1,6 +1,8 @@ """Offline PostgreSQL DDL test for the study tools migration; no database access.""" import io import os +import subprocess +import sys from pathlib import Path import unittest from unittest.mock import patch @@ -13,6 +15,12 @@ from alembic.script import ScriptDirectory class StudyToolsMigrationTests(unittest.TestCase): + def test_alembic_model_imports_register_lab_table_in_fresh_process(self): + result = subprocess.run([sys.executable, '-c', + "import app.models; from app.database import Base; assert 'lab_reference_values' in Base.metadata.tables"], + capture_output=True, text=True, env={**os.environ, 'DATABASE_URL': 'sqlite:///:memory:'}) + self.assertEqual(result.returncode, 0, result.stderr) + def test_explicit_offline_upgrade_and_downgrade(self): output = io.StringIO() config = Config(output_buffer=output) diff --git a/docs/quiz-revamp-progress.md b/docs/quiz-revamp-progress.md index 5586c4b..18360e0 100644 --- a/docs/quiz-revamp-progress.md +++ b/docs/quiz-revamp-progress.md @@ -47,7 +47,9 @@ Added a safe arithmetic calculator (no eval), keyboard shortcuts that do not int Attempt mode is persisted server-side: a query flag cannot reveal active exam answers/statistics, and mismatched attempt/quiz/user IDs are rejected. Legacy attempts without stored mode resume as exam mode rather than exposing answers; their saved answers remain intact. The new `d94a26b8f302` migration must run before deployment. -Combined verification after integrating login removal: **22 backend tests passed in the deployed image; 48 frontend tests and production build passed.** Actual PostgreSQL mode/lab migration verification and independent review are being completed separately. +Combined verification after review fixes and login removal: **25 backend tests passed in the deployed image; 53 frontend tests and production build passed.** Actual PostgreSQL mode/lab upgrade, repeated upgrade, downgrade and re-upgrade passed; existing scores were preserved and the real lab ORM defaults/publication/FK were checked. + +Independent review identified a failed-resume overwrite path, nullable review-setting serialization, and missing Alembic lab-model registration. Fixes add retry-only resume recovery, re-read saved progress for reused attempts, explicit cache failure responses, autosave warnings, blocked navigation when suspension cannot save, nullable/fail-closed course review handling, and model registration verified in a fresh process. New tests cover each case. Follow-up independent review is pending. Browser checks used a loopback-only fixture with synthetic accounts/questions and in-memory data, not production authentication. Desktop layout, provisional/confirmed answers, live fixture response counts, calculator arithmetic, honest empty lab references, and review confirmation were checked. At 390×844 the document width was exactly 390 and no question, option, image or toolbar overflowed; a taller narrow frame captures the full page. A browser-profile reset interrupted the check; re-login restored the saved attempt successfully. diff --git a/frontend/src/pages/QuizPage.jsx b/frontend/src/pages/QuizPage.jsx index a7216d5..fe528c3 100644 --- a/frontend/src/pages/QuizPage.jsx +++ b/frontend/src/pages/QuizPage.jsx @@ -282,7 +282,7 @@ function QuizCodeBadge({ code }) { } function CourseQuizStart({ quiz, onStart }) { - const mode = quiz.mode === 'timed' || quiz.allow_review === 0 ? 'exam' : 'study' + const mode = quiz.mode === 'timed' || quiz.allow_review !== 1 ? 'exam' : 'study' const [error, setError] = useState('') const [starting, setStarting] = useState(false) const begin = async () => { @@ -453,6 +453,9 @@ export default function QuizPage() { const [responseStats, setResponseStats] = useState(null) const [statsError, setStatsError] = useState('') const [submitError, setSubmitError] = useState('') + const [resumeError, setResumeError] = useState('') + const [resumeRetry, setResumeRetry] = useState(0) + const [progressError, setProgressError] = useState('') const timerRef = useRef(null) const toastRef = useRef(null) const hasStarted = useRef(false) @@ -615,16 +618,18 @@ export default function QuizPage() { // Warn before tab/window close when mid-quiz useEffect(() => { if (!attemptId) return - const msg = timeLeft !== null - ? 'Your quiz is timed. If you close this tab, the timer will continue and auto-submit when time expires. Progress is saved.' - : 'You have an in-progress quiz. Your progress is saved.' + const msg = progressError || (timeLeft !== null + ? 'Your quiz is timed. Closing the tab leaves its timer running. Progress is saved while connected.' + : 'You have an in-progress quiz. Progress is saved while connected.') const handler = (e) => { e.preventDefault(); e.returnValue = msg } window.addEventListener('beforeunload', handler) return () => window.removeEventListener('beforeunload', handler) - }, [attemptId, timeLeft]) + }, [attemptId, timeLeft, progressError]) useEffect(() => { const load = async () => { + setLoading(true) + setResumeError('') try { const [quizRes, voicesRes, favoritesRes] = await Promise.all([ api.get(`/quizzes/${id}`), @@ -646,16 +651,18 @@ export default function QuizPage() { if (progressRes.data) { await resumeQuiz(progressRes.data, voicesRes.data) } - } catch {} + } catch { + setResumeError('Could not restore your saved attempt. Retry resume before starting; your saved answers have not been replaced.') + } } catch { navigate('/') } finally { setLoading(false) } } load() return () => clearInterval(timerRef.current) - }, [id]) + }, [id, resumeRetry]) const startQuiz = async (mode, voice, timerMinutes = null) => { - if (hasStarted.current) return + if (hasStarted.current || resumeError || loading) return hasStarted.current = true setSelectedVoice(voice) setStarting(true) @@ -668,6 +675,12 @@ export default function QuizPage() { mode = attemptRes.data.mode || mode setAttemptId(attemptRes.data.id) const aid = attemptRes.data.id + // A reused attempt may have newer progress from another tab/device. + const saved = await api.get('/attempts/progress', { params: { quiz_id: id }, headers: { 'x-quiz-session': SESSION_ID } }) + if (saved.data) { + await resumeQuiz(saved.data, voices) + return + } // Fetch quiz with attempt_id for question pool filtering let quizData = quiz @@ -694,7 +707,7 @@ export default function QuizPage() { time_left: mode === 'exam' && mins ? mins * 60 : null, started_at: now, total_time: mode === 'exam' && mins ? mins * 60 : null, - }, { headers: { 'x-quiz-session': SESSION_ID } }).catch(() => {}) + }, { headers: { 'x-quiz-session': SESSION_ID } }).catch(() => setProgressError('Autosave is unavailable. Keep this tab open and retry saving.')) } catch (err) { hasStarted.current = false throw err @@ -729,7 +742,9 @@ const timerStarted = timeLeft !== null started_at: startedAt, total_time: totalTime, ...overrides, - }, { headers: { 'x-quiz-session': SESSION_ID } }).catch(() => {}) + }, { headers: { 'x-quiz-session': SESSION_ID } }) + .then(() => setProgressError('')) + .catch(() => setProgressError('Autosave is unavailable. Keep this tab open and retry saving.')) }, [id, answers, currentIdx, attemptId, quizMode, selectedVoice, timeLeft, startedAt, totalTime]) // Save progress to Redis (survives logout/browser change) @@ -863,6 +878,7 @@ const timerStarted = timeLeft !== null if (loading) return
Loading quiz...
if (!quiz) return null + if (resumeError) return

{resumeError}

if (!quizMode) return (
@@ -960,6 +976,7 @@ const timerStarted = timeLeft !== null } {submitError &&
{submitError}
} + {progressError &&
{progressError}
} {/* In-app leave confirmation */} {leaveTarget && (
@@ -967,7 +984,7 @@ const timerStarted = timeLeft !== null
⏸

Suspend quiz?

- Your progress is saved — you can resume from where you left off. + We will save your current answers before leaving, so you can resume from here. {timeLeft !== null && ( <>

Timer will pause while you are away and resume when you return.
@@ -995,7 +1012,11 @@ const timerStarted = timeLeft !== null total_time: totalTime, suspended: true, }, { headers: { 'x-quiz-session': SESSION_ID } }) - } catch { } + } catch { + setProgressError('Could not save before leaving. Keep this tab open and retry saving.') + setLeaveTarget(null) + return + } } const target = leaveTarget setLeaveTarget(null) diff --git a/frontend/src/pages/QuizPage.test.jsx b/frontend/src/pages/QuizPage.test.jsx index 6d06a0c..ab3333b 100644 --- a/frontend/src/pages/QuizPage.test.jsx +++ b/frontend/src/pages/QuizPage.test.jsx @@ -45,8 +45,8 @@ beforeEach(() => { api.delete.mockResolvedValue({}) }) -function mount() { - render(} />Submitted results

} />) +function mount(entry = '/quizzes/10') { + render(} />Submitted results
} />) } async function begin(study = true) { mount() @@ -55,6 +55,86 @@ async function begin(study = true) { } describe('quiz player', () => { + it('retries failed resume without offering Start or clearing saved answers', async () => { + mode = 'study' + const originalGet = api.get.getMockImplementation() + let failDetail = true + api.get.mockImplementation((url, ...args) => { + if (url === '/attempts/progress') return Promise.resolve({ data: { attempt_id: 50, mode: 'study', current_idx: 0, answers: { 1: 'First answer' } } }) + if (url.includes('attempt_id=') && failDetail) { failDetail = false; return Promise.reject(new Error('Temporary detail failure')) } + return originalGet(url, ...args) + }) + mount() + expect(await screen.findByRole('alert')).toHaveTextContent('Could not restore your saved attempt') + expect(screen.queryByRole('button', { name: /Study Mode/ })).not.toBeInTheDocument() + expect(api.post).not.toHaveBeenCalled() + await userEvent.click(screen.getByRole('button', { name: 'Retry resume' })) + expect(await screen.findByText(/Full explanation, preserved without shortening/)).toBeInTheDocument() + fireEvent(window, new Event('pagehide')) + await waitFor(() => expect(api.post).toHaveBeenCalledWith('/attempts/progress', expect.objectContaining({ answers: { 1: 'First answer' } }), expect.any(Object))) + expect(api.post.mock.calls.some(([url]) => url.startsWith('/attempts/start'))).toBe(false) + expect(api.post.mock.calls.filter(([url]) => url === '/attempts/progress').every(([, body]) => body.answers[1] === 'First answer')).toBe(true) + }) + + it('does not treat a failed progress lookup as a new attempt', async () => { + const originalGet = api.get.getMockImplementation() + let failProgress = true + api.get.mockImplementation((url, ...args) => { + if (url === '/attempts/progress' && failProgress) { failProgress = false; return Promise.reject(new Error('Temporary progress failure')) } + return originalGet(url, ...args) + }) + mount() + await screen.findByRole('button', { name: 'Retry resume' }) + expect(screen.queryByRole('button', { name: /Study Mode/ })).not.toBeInTheDocument() + await userEvent.click(screen.getByRole('button', { name: 'Retry resume' })) + expect(await screen.findByRole('button', { name: /Study Mode/ })).toBeInTheDocument() + expect(api.post).not.toHaveBeenCalled() + }) + + it('restores progress when Start reuses an attempt updated by another tab', async () => { + const originalGet = api.get.getMockImplementation() + let lookups = 0 + api.get.mockImplementation((url, ...args) => { + if (url === '/attempts/progress' && ++lookups > 1) return Promise.resolve({ data: { attempt_id: 50, mode: 'study', current_idx: 0, answers: { 1: 'First answer' } } }) + return originalGet(url, ...args) + }) + await begin() + expect(await screen.findByText(/Full explanation, preserved without shortening/)).toBeInTheDocument() + fireEvent(window, new Event('pagehide')) + await waitFor(() => expect(api.post.mock.calls.some(([url]) => url === '/attempts/progress')).toBe(true)) + expect(api.post.mock.calls.filter(([url]) => url === '/attempts/progress').every(([, body]) => body.answers[1] === 'First answer')).toBe(true) + }) + + it('uses exam mode for a course with a null review setting', async () => { + const originalGet = api.get.getMockImplementation() + api.get.mockImplementation(async (url, ...args) => { + const response = await originalGet(url, ...args) + return url.startsWith('/quizzes/10') ? { data: { ...response.data, mode: 'learning', allow_review: null, course_id: 1 } } : response + }) + mount('/quizzes/10?return_to=%2Fcourses%2F1') + await userEvent.click(await screen.findByRole('button', { name: 'Begin Quiz' })) + await screen.findByText('Full first clinical question.') + expect(api.post).toHaveBeenCalledWith('/attempts/start?quiz_id=10&mode=exam') + expect(screen.queryByText(/Full explanation/)).not.toBeInTheDocument() + }) + + it('stays on the quiz when suspension cannot save and supports retry', async () => { + await begin(false) + api.post.mockClear() + const originalPost = api.post.getMockImplementation() + let failSaving = true + api.post.mockImplementation((url, ...args) => url === '/attempts/progress' && failSaving ? Promise.reject(new Error('Cache outage')) : originalPost(url, ...args)) + fireEvent.keyDown(window, { key: '1' }) + await userEvent.click(screen.getByRole('button', { name: '⏸ Suspend' })) + await userEvent.click(screen.getByRole('button', { name: 'Suspend & Leave' })) + expect(await screen.findByRole('alert')).toHaveTextContent('Keep this tab open') + expect(screen.getByText('Full first clinical question.')).toBeInTheDocument() + failSaving = false + await userEvent.click(screen.getByRole('button', { name: 'Retry saving' })) + await waitFor(() => expect(screen.queryByRole('alert')).not.toBeInTheDocument()) + expect(api.post.mock.calls.filter(([url]) => url === '/attempts/progress').every(([, body]) => body.answers[1] === 'First answer')).toBe(true) + }) + it('keeps a study selection provisional until confirmation and shows genuine response data', async () => { await begin() expect(screen.getByRole('link', { name: 'Neonatology' })).toHaveAttribute('href', '/quizzes/create?category=11')