fix: review or resume, and an exam clock set by the questions
The card offered Review answers and Resume session at once on a session still in progress, which is the muddle: there is nothing to review yet and nothing to resume once it is done. It is one or the other now, and what decides it is whether anything is left to answer — not whether it was an exam or a study session, which have the same two states as each other. A study session keeps going until every question is answered and becomes the review at that point, without waiting to be handed in. Repeat is offered either way. The questions worth sitting again are worth sitting again now. "Skipped" meant gone past, and was shown for questions in a session still running that had not been reached. Those read "not yet answered". And a timed block is now ninety seconds a question, set from the count rather than asked for. Choosing a limit is a decision nobody has the information to make — the pace belongs to the exam being rehearsed, not to a preference — and a block sat at the wrong pace teaches the wrong pace. Forty questions is an hour. An explicit limit is still honoured. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN
This commit is contained in:
Daniel
parent
cc1c981b9e
commit
6abe3cca11
7 changed files with 157 additions and 48 deletions
|
|
@ -879,8 +879,17 @@ def attempt_analysis(
|
|||
"position": index,
|
||||
"question_id": row.question_id,
|
||||
"excerpt": (getattr(question, "question_text", "") or "")[:120],
|
||||
"status": ("correct" if row.is_correct else "skipped" if not row.user_answer else "incorrect")
|
||||
if graded else ("answered" if row.user_answer else "skipped"),
|
||||
# "Skipped" means you went past it. In a session still running you
|
||||
# have not been past it yet, so it says so — and in an exam that is
|
||||
# running, an answered one says only that.
|
||||
"status": (
|
||||
("correct" if row.is_correct else "skipped" if not row.user_answer else "incorrect")
|
||||
if attempt.completed_at
|
||||
else ("correct" if graded and row.is_correct
|
||||
else "incorrect" if graded and row.user_answer
|
||||
else "answered" if row.user_answer
|
||||
else "unanswered")
|
||||
),
|
||||
"difficulty": getattr(question, "difficulty", None),
|
||||
"category": categories.get(getattr(question, "question_category_id", None)),
|
||||
"seconds_spent": row.seconds_spent,
|
||||
|
|
|
|||
|
|
@ -193,6 +193,29 @@ class GenerateTestRequest(TestOptions):
|
|||
explicit_ids: list[int] = Field(default_factory=list)
|
||||
|
||||
|
||||
#: Seconds a timed block allows per question. The pace a real paper is sat at,
|
||||
#: so a block of forty runs an hour — and so a learner rehearsing on this bank
|
||||
#: is rehearsing the clock as well as the questions.
|
||||
SECONDS_PER_QUESTION = 90
|
||||
|
||||
|
||||
def exam_minutes(data, count: int) -> int | None:
|
||||
"""How long a timed block gets, rounded up to the minute.
|
||||
|
||||
Set from the number of questions rather than asked for. Choosing a limit
|
||||
is a decision nobody has the information to make — the pace is a property
|
||||
of the exam being rehearsed, not a preference — and a block sat at the
|
||||
wrong pace teaches the wrong pace. An explicit limit is still honoured for
|
||||
the cases that genuinely differ.
|
||||
"""
|
||||
if getattr(data, "mode", None) != "timed":
|
||||
return None
|
||||
asked = getattr(data, "time_limit_minutes", None)
|
||||
if asked:
|
||||
return asked
|
||||
return max(1, -(-(count * SECONDS_PER_QUESTION) // 60))
|
||||
|
||||
|
||||
def create_saved_test(db, user, data, question_ids):
|
||||
ids = list(dict.fromkeys(question_ids))
|
||||
if not 1 <= len(ids) <= 200:
|
||||
|
|
@ -203,7 +226,7 @@ def create_saved_test(db, user, data, question_ids):
|
|||
if query.count() != len(ids):
|
||||
raise HTTPException(400, "Some questions are missing, private, or unavailable for this test")
|
||||
quiz = Quiz(user_id=user.id, title=data.title, mode=data.mode,
|
||||
time_limit_minutes=data.time_limit_minutes if data.mode == "timed" else None,
|
||||
time_limit_minutes=exam_minutes(data, len(ids)),
|
||||
questions_count=len(ids), is_published=0, is_shared=int(data.is_shared))
|
||||
db.add(quiz)
|
||||
db.flush()
|
||||
|
|
|
|||
|
|
@ -416,3 +416,40 @@ class AdaptiveSelectionTests(unittest.TestCase):
|
|||
# Everything has been seen, so the whole selection is recycled.
|
||||
picked = adaptive_select(self.db, self.user, 1, [], "all", None)
|
||||
self.assertEqual(picked, [1])
|
||||
|
||||
|
||||
class ExamClockTests(unittest.TestCase):
|
||||
"""A timed block's length is a property of the exam, not a preference."""
|
||||
|
||||
def test_ninety_seconds_a_question_rounded_up_to_the_minute(self):
|
||||
from app.services.quiz_builder import exam_minutes
|
||||
|
||||
class Ask:
|
||||
mode = "timed"
|
||||
time_limit_minutes = None
|
||||
|
||||
# Forty questions is an hour, which is the pace a real paper is sat at.
|
||||
self.assertEqual(exam_minutes(Ask(), 40), 60)
|
||||
self.assertEqual(exam_minutes(Ask(), 20), 30)
|
||||
# Five is seven and a half minutes, and a block never gets less than a
|
||||
# minute however short it is.
|
||||
self.assertEqual(exam_minutes(Ask(), 5), 8)
|
||||
self.assertEqual(exam_minutes(Ask(), 0), 1)
|
||||
|
||||
def test_a_study_block_has_no_clock(self):
|
||||
from app.services.quiz_builder import exam_minutes
|
||||
|
||||
class Ask:
|
||||
mode = "study"
|
||||
time_limit_minutes = 30
|
||||
|
||||
self.assertIsNone(exam_minutes(Ask(), 40))
|
||||
|
||||
def test_an_explicit_limit_is_still_honoured(self):
|
||||
from app.services.quiz_builder import exam_minutes
|
||||
|
||||
class Ask:
|
||||
mode = "timed"
|
||||
time_limit_minutes = 15
|
||||
|
||||
self.assertEqual(exam_minutes(Ask(), 40), 15)
|
||||
|
|
|
|||
|
|
@ -310,7 +310,7 @@ class LiveAnalysisTests(unittest.TestCase):
|
|||
self.assertEqual(body["score"], 1)
|
||||
statuses = {q["status"] for q in body["questions"]}
|
||||
self.assertIn("correct", statuses)
|
||||
self.assertIn("skipped", statuses)
|
||||
self.assertIn("unanswered", statuses)
|
||||
# Nothing was written: submitting is what records answers.
|
||||
self.assertEqual(self.db.query(AttemptAnswer).filter_by(attempt_id=aid).count(), 0)
|
||||
|
||||
|
|
@ -335,7 +335,9 @@ class LiveAnalysisTests(unittest.TestCase):
|
|||
# How far through, which is not a leak, and nothing about rightness.
|
||||
self.assertEqual(body["answered"], 1)
|
||||
statuses = {q["status"] for q in body["questions"]}
|
||||
self.assertEqual(statuses, {"answered", "skipped"})
|
||||
# "Skipped" would mean gone past; in a session still running it has
|
||||
# not been reached.
|
||||
self.assertEqual(statuses, {"answered", "unanswered"})
|
||||
self.assertNotIn("correct", statuses)
|
||||
self.assertNotIn("incorrect", statuses)
|
||||
# And nothing to go back to yet: a recommendation is a verdict.
|
||||
|
|
|
|||
|
|
@ -82,6 +82,8 @@ td.an-col-q { display: flex; flex-wrap: wrap; align-items: baseline; gap: 6px; }
|
|||
.an-status.is-correct { background: var(--correct-bg); color: var(--correct-fg); }
|
||||
.an-status.is-incorrect { background: var(--wrong-bg); color: var(--wrong-fg); }
|
||||
.an-status.is-skipped { background: var(--bg); color: var(--text-muted); }
|
||||
/* Not reached yet, which is not the same as passed by. */
|
||||
.an-status.is-unanswered { background: var(--bg); color: var(--text-subtle); }
|
||||
|
||||
@media (max-width: 900px) {
|
||||
.an-page { grid-template-columns: 1fr; }
|
||||
|
|
|
|||
|
|
@ -136,6 +136,8 @@ export default function AnalysisSessionPage() {
|
|||
// donut by except answered and not. Showing a nought here would read as a
|
||||
// score, and a score before the exam is over is the exam defeated.
|
||||
const graded = data.graded !== false
|
||||
// Nothing left to answer is finished, whether or not it was handed in.
|
||||
const finished = !!data.completed_at || (data.total > 0 && data.answered >= data.total)
|
||||
const correct = graded ? data.score : 0
|
||||
const incorrect = graded ? data.answered - data.score : 0
|
||||
const answeredOnly = graded ? 0 : data.answered
|
||||
|
|
@ -222,48 +224,50 @@ export default function AnalysisSessionPage() {
|
|||
|
||||
{/* What to do about it, beneath the thing it is about. These were
|
||||
a row of buttons beside the page heading, which put the
|
||||
decision as far as possible from the result it follows from. */}
|
||||
decision as far as possible from the result it follows from.
|
||||
|
||||
Review or resume, never both. Whether a session is finished
|
||||
decides which — not whether it was an exam or a study session,
|
||||
which have the same two states as each other. Offering both at
|
||||
once was the muddle: a session you are part-way through has
|
||||
nothing to review yet, and a finished one has nothing to
|
||||
resume. */}
|
||||
<div className="an-actions">
|
||||
{data.not_started ? (
|
||||
// One label either way. The session exists the moment it is
|
||||
// made, so picking it up is resuming it whether or not a
|
||||
// question has been answered yet.
|
||||
{/* Repeating does not wait for the end: the questions worth
|
||||
sitting again are worth sitting again now. */}
|
||||
{data.questions?.length > 0 && (
|
||||
<button type="button" className="btn btn-secondary"
|
||||
onClick={() => setRepeating(true)}>Repeat session</button>
|
||||
)}
|
||||
{/* Review or resume, decided by whether there is anything left
|
||||
to do. A study session just keeps going until every question
|
||||
has been answered, and at that point there is nothing to
|
||||
resume — so it becomes the review without waiting to be
|
||||
handed in. A submitted session is finished either way. */}
|
||||
{finished ? (
|
||||
<Link className="btn btn-primary"
|
||||
to={`/results/${data.attempt_id ?? attemptId}`}>Review session</Link>
|
||||
) : (
|
||||
<Link className="btn btn-primary" to={`/study/${data.quiz_id}?start=1`}>
|
||||
Resume session
|
||||
</Link>
|
||||
) : (
|
||||
<>
|
||||
{/* Repeating rarely means all of it: the questions worth
|
||||
sitting again are the ones you got wrong and the ones you
|
||||
never reached. The dialog asks which, and how many. */}
|
||||
{data.questions?.length > 0 && (
|
||||
<button type="button" className="btn btn-secondary"
|
||||
onClick={() => setRepeating(true)}>Repeat session</button>
|
||||
)}
|
||||
<Link className="btn btn-secondary"
|
||||
to={`/results/${data.attempt_id ?? attemptId}`}>Review answers</Link>
|
||||
{!data.completed_at && (
|
||||
<Link className="btn btn-primary" to={`/study/${data.quiz_id}?start=1`}>
|
||||
Resume session
|
||||
</Link>
|
||||
)}
|
||||
{/* Deleting a session throws away answers the analysis is
|
||||
built from, so it is not a button sitting next to Resume.
|
||||
It is still offered: sessions are made freely here, and a
|
||||
mis-made one is clutter worth removing. */}
|
||||
{confirmDelete ? (
|
||||
<span className="an-confirm">
|
||||
<button type="button" className="btn btn-sm an-danger" disabled={deleting}
|
||||
onClick={deleteSession}>{deleting ? 'Deleting…' : 'Delete for good'}</button>
|
||||
<button type="button" className="btn btn-secondary btn-sm"
|
||||
onClick={() => setConfirmDelete(false)}>Keep</button>
|
||||
</span>
|
||||
) : (
|
||||
<button type="button" className="an-remove"
|
||||
onClick={() => setConfirmDelete(true)}>Delete session</button>
|
||||
)}
|
||||
</>
|
||||
)}
|
||||
|
||||
{/* Deleting a session throws away answers the analysis is built
|
||||
from, so it is not a button sitting next to the one you came
|
||||
here to press. It is still offered: sessions are made freely
|
||||
here, and a mis-made one is clutter worth removing. */}
|
||||
{!data.not_started && (confirmDelete ? (
|
||||
<span className="an-confirm">
|
||||
<button type="button" className="btn btn-sm an-danger" disabled={deleting}
|
||||
onClick={deleteSession}>{deleting ? 'Deleting…' : 'Delete for good'}</button>
|
||||
<button type="button" className="btn btn-secondary btn-sm"
|
||||
onClick={() => setConfirmDelete(false)}>Keep</button>
|
||||
</span>
|
||||
) : (
|
||||
<button type="button" className="an-remove"
|
||||
onClick={() => setConfirmDelete(true)}>Delete session</button>
|
||||
))}
|
||||
</div>
|
||||
</section>
|
||||
|
||||
|
|
@ -328,7 +332,11 @@ export default function AnalysisSessionPage() {
|
|||
</Link>
|
||||
{row.category && <em className="an-qcat">{row.category}</em>}
|
||||
</td>
|
||||
<td><span className={`an-status is-${row.status}`}>{row.status}</span></td>
|
||||
<td>
|
||||
<span className={`an-status is-${row.status}`}>
|
||||
{row.status === 'unanswered' ? 'not yet answered' : row.status}
|
||||
</span>
|
||||
</td>
|
||||
<td>
|
||||
{row.difficulty
|
||||
? <span className={`an-diff is-${row.difficulty}`}>{row.difficulty}</span>
|
||||
|
|
|
|||
|
|
@ -74,7 +74,7 @@ describe('a session nobody has sat', () => {
|
|||
mountQuiz()
|
||||
expect(await screen.findByRole('link', { name: 'Resume session' }))
|
||||
.toHaveAttribute('href', '/study/3?start=1')
|
||||
expect(screen.queryByRole('link', { name: 'Review answers' })).not.toBeInTheDocument()
|
||||
expect(screen.queryByRole('link', { name: /Review/ })).not.toBeInTheDocument()
|
||||
expect(screen.queryByRole('button', { name: 'Delete session' })).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
|
|
@ -92,7 +92,7 @@ describe('a session that has been sat', () => {
|
|||
mountAttempt()
|
||||
const figures = (await screen.findByText("2/2")).closest('.an-figures')
|
||||
expect(within(figures).getByText('50%')).toBeInTheDocument()
|
||||
expect(screen.getByRole('link', { name: 'Review answers' })).toHaveAttribute('href', '/results/91')
|
||||
expect(screen.getByRole('link', { name: 'Review session' })).toHaveAttribute('href', '/results/91')
|
||||
// Repeating opens a dialog: which outcomes, and how many. Sitting all of
|
||||
// it again is rarely what anyone wants.
|
||||
expect(screen.getByRole('button', { name: 'Repeat session' })).toBeInTheDocument()
|
||||
|
|
@ -127,10 +127,38 @@ describe('a session whose only attempt is still in progress', () => {
|
|||
await waitFor(() => expect(api.delete).toHaveBeenCalledWith('/attempts/77'))
|
||||
})
|
||||
|
||||
it('reviews that attempt too', async () => {
|
||||
it('offers to resume it, not to review it', async () => {
|
||||
mountQuiz()
|
||||
expect(await screen.findByRole('link', { name: 'Review answers' }))
|
||||
.toHaveAttribute('href', '/results/77')
|
||||
// Review or resume, never both: a session you are part-way through has
|
||||
// nothing to review yet, and whether it was an exam or a study session
|
||||
// does not come into it.
|
||||
expect(await screen.findByRole('link', { name: 'Resume session' }))
|
||||
.toHaveAttribute('href', '/study/3?start=1')
|
||||
expect(screen.queryByRole('link', { name: /Review/ })).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('a study session with every question answered', () => {
|
||||
it('becomes the review without waiting to be handed in', async () => {
|
||||
api.get.mockImplementation(url => {
|
||||
if (url === '/attempts/8/analysis') return Promise.resolve({ data: {
|
||||
attempt_id: 8, quiz_id: 3, title: 'All done', mode: 'study',
|
||||
completed_at: null, graded: true,
|
||||
total: 2, answered: 2, score: 1, percent: 50,
|
||||
seconds_total: null, seconds_per_question: 30,
|
||||
questions: [], recommendations: [], plan: null, not_started: false,
|
||||
} })
|
||||
if (url === '/attempts/sessions') return Promise.resolve({ data: [] })
|
||||
return Promise.resolve({ data: [] })
|
||||
})
|
||||
render(<MemoryRouter initialEntries={['/sessions/8']}><Routes>
|
||||
<Route path="/sessions/:attemptId" element={<AnalysisSessionPage />} />
|
||||
</Routes></MemoryRouter>)
|
||||
|
||||
// A study session keeps going until everything is answered. At that point
|
||||
// there is nothing left to resume, so it is the review.
|
||||
expect(await screen.findByRole('link', { name: 'Review session' })).toBeInTheDocument()
|
||||
expect(screen.queryByRole('link', { name: 'Resume session' })).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue