fix: preserve saved quiz progress across resume failures
Block Start after failed resume, retry saved state, re-read reused-attempt progress, surface cache failures and prevent leaving on failed save. Handle nullable review settings and register lab model with Alembic. Verified 25 deployed-image backend tests, 53 frontend tests/build and PostgreSQL schema round-trip.
This commit is contained in:
Daniel
parent
a3a6ef7995
commit
c630b0270d
8 changed files with 166 additions and 18 deletions
|
|
@ -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",
|
||||
]
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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')
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
||||
|
|
|
|||
|
|
@ -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 <div className="loading"><div className="spinner"></div> Loading quiz...</div>
|
||||
if (!quiz) return null
|
||||
if (resumeError) return <div className="card" role="alert"><p>{resumeError}</p><button type="button" className="btn btn-primary" onClick={() => setResumeRetry(value => value + 1)}>Retry resume</button></div>
|
||||
|
||||
if (!quizMode) return (
|
||||
<div>
|
||||
|
|
@ -960,6 +976,7 @@ const timerStarted = timeLeft !== null
|
|||
<button type="button" className="quiz-complete-confirm" disabled={submitting} onClick={() => { setShowReview(false); handleSubmit(false) }}>Complete test</button>
|
||||
</QuizDialog>}
|
||||
{submitError && <div role="alert" className="quiz-submit-error">{submitError} <button type="button" disabled={submitting} onClick={() => handleSubmit(false)}>Retry submission</button></div>}
|
||||
{progressError && <div role="alert" className="quiz-submit-error">{progressError} <button type="button" onClick={() => saveProgressNow()}>Retry saving</button></div>}
|
||||
{/* In-app leave confirmation */}
|
||||
{leaveTarget && (
|
||||
<div style={{ position: 'fixed', inset: 0, background: 'rgba(0,0,0,0.6)', zIndex: 1000, display: 'flex', alignItems: 'center', justifyContent: 'center', padding: 16 }}>
|
||||
|
|
@ -967,7 +984,7 @@ const timerStarted = timeLeft !== null
|
|||
<div style={{ fontSize: '2rem', marginBottom: 12 }}>⏸</div>
|
||||
<h2 style={{ marginBottom: 8 }}>Suspend quiz?</h2>
|
||||
<p style={{ color: 'var(--text-muted)', fontSize: '0.875rem', marginBottom: 20 }}>
|
||||
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 && (
|
||||
<><br/><br/>
|
||||
<strong style={{ color: '#16a34a' }}>Timer will pause</strong> while you are away and resume when you return. <br/>
|
||||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -45,8 +45,8 @@ beforeEach(() => {
|
|||
api.delete.mockResolvedValue({})
|
||||
})
|
||||
|
||||
function mount() {
|
||||
render(<MemoryRouter initialEntries={['/quizzes/10']}><Routes><Route path="/quizzes/:id" element={<QuizPage />} /><Route path="/results/:id" element={<div>Submitted results</div>} /></Routes></MemoryRouter>)
|
||||
function mount(entry = '/quizzes/10') {
|
||||
render(<MemoryRouter initialEntries={[entry]}><Routes><Route path="/quizzes/:id" element={<QuizPage />} /><Route path="/results/:id" element={<div>Submitted results</div>} /></Routes></MemoryRouter>)
|
||||
}
|
||||
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')
|
||||
|
|
|
|||
Loading…
Reference in a new issue