diff --git a/backend/alembic/versions/d0e1f2a3b4c5_repetition.py b/backend/alembic/versions/d0e1f2a3b4c5_repetition.py new file mode 100644 index 0000000..0082625 --- /dev/null +++ b/backend/alembic/versions/d0e1f2a3b4c5_repetition.py @@ -0,0 +1,28 @@ +"""Mark a repeated session as a repetition. + +Sitting the same questions again is practice, not a new measurement: the +answers have already been seen, so getting them right the second time says +nothing about whether they were known. Such a session is still analysed on its +own page — that is the point of repeating it — but it is left out of the +figures that claim to say how much of the bank you know. + +Revision ID: d0e1f2a3b4c5 +Revises: c9d0e1f2a3b4 +""" +import sqlalchemy as sa +from alembic import op + +revision = "d0e1f2a3b4c5" +down_revision = "c9d0e1f2a3b4" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column("quizzes", sa.Column("is_repetition", sa.Integer(), server_default="0")) + op.create_index("ix_quizzes_is_repetition", "quizzes", ["is_repetition"]) + + +def downgrade() -> None: + op.drop_index("ix_quizzes_is_repetition", table_name="quizzes") + op.drop_column("quizzes", "is_repetition") diff --git a/backend/app/models/quiz.py b/backend/app/models/quiz.py index 49a54dc..9be04ed 100644 --- a/backend/app/models/quiz.py +++ b/backend/app/models/quiz.py @@ -32,6 +32,11 @@ class Quiz(Base): deleted_at = Column(DateTime, nullable=True) # soft delete — null = active is_published = Column(Integer, default=1) # 1 = visible to users, 0 = hidden (admin only) is_shared = Column(Integer, default=0) # 0=private, 1=shared (visible to other users) + # A repetition is practice, not a new measurement. You have already seen + # these questions and their answers, so getting them right the second time + # says nothing about whether you knew them — it cannot raise a percentage + # that is meant to mean "how much of this do you know". + is_repetition = Column(Integer, default=0, index=True) course_id = Column(Integer, ForeignKey("courses.id", ondelete="CASCADE"), nullable=True) # set = course-only quiz, hidden from main page max_attempts = Column(Integer, nullable=True) # null = unlimited questions_per_attempt = Column(Integer, nullable=True) # null = all; set = random subset from pool diff --git a/backend/app/routers/attempts.py b/backend/app/routers/attempts.py index a78791c..95c2adc 100644 --- a/backend/app/routers/attempts.py +++ b/backend/app/routers/attempts.py @@ -429,44 +429,10 @@ def reset_all_practice_data( return {"removed": removed} -@router.delete("/{attempt_id}", status_code=204) -def delete_attempt( - attempt_id: int, - db: Session = Depends(get_db), - current_user: User = Depends(get_current_user), -): - """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, - ).first() - if not attempt: - raise HTTPException(status_code=404, detail="Attempt not found") - - # Block deletion of course quiz attempts - quiz = db.query(Quiz).filter(Quiz.id == attempt.quiz_id).first() - if quiz and quiz.course_id: - raise HTTPException(status_code=403, detail="Cannot delete course quiz attempts") - - quiz_id = attempt.quiz_id - db.delete(attempt) - db.flush() - # A block is done only while a completed attempt says so. Deleting the one - # that completed it puts the block back to unfinished. - still_done = db.query(QuizAttempt.id).filter( - QuizAttempt.quiz_id == quiz_id, QuizAttempt.user_id == current_user.id, - QuizAttempt.completed_at.isnot(None)).first() - if not still_done: - unmark_block_complete(db, current_user.id, quiz_id) - db.commit() - # Any saved in-progress state and device lock go with it. - try: - import redis as redis_lib - from app.config import settings - r = redis_lib.from_url(settings.REDIS_URL, decode_responses=True) - r.delete(progress_key(current_user.id, attempt_id), active_key(current_user.id, attempt_id)) - except Exception: - logger.warning("Redis unavailable when deleting attempt %s", attempt_id, exc_info=True) +# Deleting a single attempt is gone. A session is a record of work done, and +# removing one edits the history every figure on the analysis is computed from +# — which turns a measurement into something somebody chose. Starting again is +# offered whole instead: POST /attempts/reset-all, under Settings, Your data. @router.get("/quiz/{quiz_id}/in-progress") @@ -541,6 +507,10 @@ def get_quiz_history( QuizAttempt.user_id == current_user.id, QuizAttempt.completed_at.isnot(None), Quiz.course_id.is_(None), + # A repetition is practice, not a new measurement: the answers have + # already been seen, so getting them right again says nothing about + # whether they were known. It has its own analysis; it is not in this. + (Quiz.is_repetition == 0) | (Quiz.is_repetition.is_(None)), (QuizAttempt.expired == 0) | (QuizAttempt.expired.is_(None)), ) .order_by(QuizAttempt.completed_at) @@ -585,6 +555,10 @@ def get_dashboard_stats( QuizAttempt.user_id == current_user.id, QuizAttempt.completed_at.isnot(None), Quiz.course_id.is_(None), + # A repetition is practice, not a new measurement: the answers have + # already been seen, so getting them right again says nothing about + # whether they were known. It has its own analysis; it is not in this. + (Quiz.is_repetition == 0) | (Quiz.is_repetition.is_(None)), (QuizAttempt.expired == 0) | (QuizAttempt.expired.is_(None)), ) .distinct().count() @@ -597,6 +571,10 @@ def get_dashboard_stats( QuizAttempt.user_id == current_user.id, QuizAttempt.completed_at.isnot(None), Quiz.course_id.is_(None), + # A repetition is practice, not a new measurement: the answers have + # already been seen, so getting them right again says nothing about + # whether they were known. It has its own analysis; it is not in this. + (Quiz.is_repetition == 0) | (Quiz.is_repetition.is_(None)), (QuizAttempt.expired == 0) | (QuizAttempt.expired.is_(None)), ) .all() diff --git a/backend/app/routers/study_tools.py b/backend/app/routers/study_tools.py index 980242b..830be31 100644 --- a/backend/app/routers/study_tools.py +++ b/backend/app/routers/study_tools.py @@ -184,6 +184,10 @@ def performance_by_category(db: Session = Depends(get_db), user: User = Depends( Question, Question.id == AttemptAnswer.question_id).filter( QuizAttempt.user_id == user.id, QuizAttempt.completed_at.isnot(None), or_(QuizAttempt.expired == 0, QuizAttempt.expired.is_(None)), Quiz.course_id.is_(None), + # A repetition is practice, not a new measurement: the answers have + # already been seen, so getting them right again says nothing about + # whether they were known. It has its own analysis; it is not in this. + or_(Quiz.is_repetition == 0, Quiz.is_repetition.is_(None)), # A skipped question is not a wrong answer. Counting it as one made a # 360-question sitting that was never worked through read as 0% accuracy # across every category it touched. @@ -277,6 +281,10 @@ def study_recommendations( QuizAttempt.completed_at.isnot(None), or_(QuizAttempt.expired == 0, QuizAttempt.expired.is_(None)), Quiz.course_id.is_(None), + # A repetition is practice, not a new measurement: the answers have + # already been seen, so getting them right again says nothing about + # whether they were known. It has its own analysis; it is not in this. + or_(Quiz.is_repetition == 0, Quiz.is_repetition.is_(None)), *([exam_filter] if exam_filter is not None else []), ).all() diff --git a/backend/app/services/quiz_builder.py b/backend/app/services/quiz_builder.py index 03466d1..c92d7fc 100644 --- a/backend/app/services/quiz_builder.py +++ b/backend/app/services/quiz_builder.py @@ -163,6 +163,9 @@ class TestOptions(BaseModel): mode: Literal["timed", "learning"] = "timed" time_limit_minutes: int | None = Field(default=None, gt=0) is_shared: bool = False + #: Sitting the same questions again. Analysed on its own page, left out of + #: the figures that claim to say how much of the bank you know. + is_repetition: bool = False @field_validator("title") @classmethod @@ -227,7 +230,8 @@ def create_saved_test(db, user, data, question_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=exam_minutes(data, len(ids)), - questions_count=len(ids), is_published=0, is_shared=int(data.is_shared)) + questions_count=len(ids), is_published=0, is_shared=int(data.is_shared), + is_repetition=int(getattr(data, "is_repetition", False))) db.add(quiz) db.flush() add_questions_to_quiz(db, quiz.id, ids) diff --git a/backend/tests/test_session_lifecycle.py b/backend/tests/test_session_lifecycle.py index 38619f8..a2123a1 100644 --- a/backend/tests/test_session_lifecycle.py +++ b/backend/tests/test_session_lifecycle.py @@ -45,6 +45,9 @@ def fake_redis(store): class SessionLifecycleTests(unittest.TestCase): + # Deleting a single attempt is gone: a session is a record of work done, + # and removing one edits the history every figure on the analysis is + # computed from. Starting again is offered whole, via reset-all. def setUp(self): self.bank = fixtures.BuilderTests() self.bank.setUp() @@ -160,38 +163,6 @@ class SessionLifecycleTests(unittest.TestCase): # ── delete ──────────────────────────────────────────────────────────────── - def test_deleting_the_attempt_that_completed_a_block_makes_the_block_unfinished(self): - self.db.add(StudyPlan(id=1, slug="p", name="Plan", kind="set", is_published=1)) - self.db.flush() - self.db.add(StudyPlanBlock(id=10, plan_id=1, position=1, title="Block 1", question_ids=[1, 2])) - self.db.commit() - quiz_id = self.client.post("/study-plans/blocks/10/start", params={"mode": "timed"}).json()["id"] - self.db.add(QuizAttempt(id=800, quiz_id=quiz_id, user_id=self.bank.owner.id, mode="exam", - total_questions=2, score=2, started_at=datetime(2026, 1, 1), - completed_at=datetime(2026, 1, 1, 1))) - self.db.add(AttemptAnswer(attempt_id=800, question_id=1, user_answer="A", is_correct=True)) - progress = self.db.query(StudyPlanBlockProgress).filter_by(quiz_id=quiz_id).one() - progress.completed_at = datetime(2026, 1, 1, 1) - self.db.commit() - self.store[f"quiz_progress:{self.bank.owner.id}:800"] = "{}" - - with patch.dict(sys.modules, {"redis": self.redis}): - self.assertEqual(self.client.delete("/attempts/800").status_code, 204) - self.db.expire_all() - self.assertIsNone(self.db.get(QuizAttempt, 800)) - self.assertEqual(self.db.query(AttemptAnswer).filter_by(attempt_id=800).count(), 0) - self.assertIsNone(self.db.get(StudyPlanBlockProgress, progress.id).completed_at) - self.assertEqual(self.db.get(StudyPlanBlockProgress, progress.id).quiz_id, quiz_id) - self.assertNotIn(f"quiz_progress:{self.bank.owner.id}:800", self.store) - - def test_someone_elses_attempt_cannot_be_deleted(self): - quiz_id, aid = self.timed_quiz() - self.bank.user = self.bank.peer - with patch.dict(sys.modules, {"redis": self.redis}): - self.assertEqual(self.client.delete(f"/attempts/{aid}").status_code, 404) - - # ── reset ───────────────────────────────────────────────────────────────── - def test_reset_removes_what_was_practised_and_keeps_what_was_made(self): quiz_id, aid = self.timed_quiz() uid = self.bank.owner.id diff --git a/frontend/src/components/CategoryTree.css b/frontend/src/components/CategoryTree.css new file mode 100644 index 0000000..54cd083 --- /dev/null +++ b/frontend/src/components/CategoryTree.css @@ -0,0 +1,39 @@ +/* The tree carries its own appearance. + * + * These rules lived in QuestionBankPage.css, so the tree looked right on the + * bank and took whatever the host page did to a label everywhere else — in the + * question editor that centred the name, leaving it floating in the middle of + * the row with the count adrift at the far right. A component that is used in + * four places should look the same in all four. + */ +.category-tree { list-style: none; margin: 0; padding: 0; max-height: 46vh; overflow-y: auto; } +.category-tree ul { list-style: none; margin: 0 0 0 16px; padding: 0; } +.category-tree li { margin: 1px 0; } + +.category-tree label { + display: flex; align-items: baseline; gap: 8px; + font-size: 0.84rem; text-align: left; cursor: pointer; +} +.category-tree label > input[type="checkbox"] { flex: none; } +.category-tree-name { flex: 1; min-width: 0; text-align: left; overflow-wrap: anywhere; } +.category-tree-count { flex: none; color: var(--text-muted); font-size: 0.74rem; } +.category-tree-excluded { opacity: 0.5; } + +.category-tree-branch { display: flex; align-items: center; cursor: pointer; list-style: none; } +.category-tree-branch::-webkit-details-marker { display: none; } +.category-tree-branch > label { flex: 1; min-width: 0; } +.category-tree-chevron { + display: inline-block; width: 7px; height: 7px; flex-shrink: 0; + border-right: 2px solid var(--text-muted); border-bottom: 2px solid var(--text-muted); + transform: rotate(-45deg); margin-right: 6px; transition: transform 0.15s ease; +} +details[open] > .category-tree-branch .category-tree-chevron { transform: rotate(45deg); } + +.category-tree-search { + width: 100%; margin-bottom: 8px; padding: 8px 10px; + /* 16px on touch: iOS zooms in on anything smaller and never zooms back. */ + font-size: 16px; font-family: inherit; + border: 1px solid var(--border); border-radius: 8px; + background: var(--input-bg); color: var(--text); +} +@media (min-width: 700px) { .category-tree-search { font-size: 0.84rem; } } diff --git a/frontend/src/components/CategoryTree.jsx b/frontend/src/components/CategoryTree.jsx index 9385270..b701e54 100644 --- a/frontend/src/components/CategoryTree.jsx +++ b/frontend/src/components/CategoryTree.jsx @@ -1,4 +1,5 @@ import { useState } from 'react' +import './CategoryTree.css' export default function CategoryTree({ categories, selectedIds, onToggle, excludedId = null, searchable = false }) { const [query, setQuery] = useState('') diff --git a/frontend/src/components/RepeatSession.jsx b/frontend/src/components/RepeatSession.jsx index 3916dad..3d104b1 100644 --- a/frontend/src/components/RepeatSession.jsx +++ b/frontend/src/components/RepeatSession.jsx @@ -61,9 +61,12 @@ export default function RepeatSession({ title, rows, onClose }) { // Shuffled, so repeating twice is not the same order twice. const ids = [...pool].sort(() => Math.random() - 0.5).slice(0, count) const res = await api.post('/questions/from-bank', { - title: `${title} — again`, + title: `${title} (repetition)`, question_ids: ids, mode: 'study', + // Practice, not a new measurement: it is analysed on its own page but + // left out of the figures that say how much of the bank you know. + is_repetition: true, }) navigate(`/study/${res.data.id}?start=1`) } catch (err) { diff --git a/frontend/src/pages/AnalysisSessionPage.jsx b/frontend/src/pages/AnalysisSessionPage.jsx index fbd755d..6e43ae7 100644 --- a/frontend/src/pages/AnalysisSessionPage.jsx +++ b/frontend/src/pages/AnalysisSessionPage.jsx @@ -86,27 +86,9 @@ export default function AnalysisSessionPage() { const [sort, setSort] = useState('position') // Ten at a time: a session of forty is a table nobody reads to the end of. const [page, setPage] = useState(0) - const [confirmDelete, setConfirmDelete] = useState(false) const [repeating, setRepeating] = useState(false) - const [deleting, setDeleting] = useState(false) const navigate = useNavigate() - const deleteSession = async () => { - setDeleting(true) - try { - // The attempt the page is showing, not the one named in the URL: reached - // by quiz (a session whose only attempt is still in progress) there is no - // attemptId in the path, and this deleted "/attempts/undefined". - await api.delete(`/attempts/${data?.attempt_id ?? attemptId}`) - navigate('/sessions', { replace: true }) - } catch { - setDeleting(false) - setError('Could not delete this session') - } - } - - // Addressed by attempt, or — for a session nobody has sat — by quiz, which - // answers with the same shape at zero. The page does not branch on which. const load = useCallback(() => { setLoading(true) const url = attemptId ? `/attempts/${attemptId}/analysis` : `/attempts/quiz/${quizId}/analysis` @@ -163,12 +145,6 @@ export default function AnalysisSessionPage() { )} - {confirmDelete && ( -
- This removes the attempt and its answers. Your overall statistics are - recalculated without it, and it cannot be undone. -
- )} {/* The figures below are all zero and every row reads "skipped", which is the true picture. Saying why keeps that from looking like a @@ -253,21 +229,11 @@ export default function AnalysisSessionPage() { )} - {/* 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 ? ( - - - - - ) : ( - - ))} + {/* No delete. A session is a record of work done, and removing + one edits the history the analysis is computed from — which + makes every figure on this page a number somebody chose + rather than a number they earned. Starting again is offered + whole, under Settings, Your data. */} diff --git a/frontend/src/pages/AnalysisSessionPage.test.jsx b/frontend/src/pages/AnalysisSessionPage.test.jsx index 8742fed..c079aa0 100644 --- a/frontend/src/pages/AnalysisSessionPage.test.jsx +++ b/frontend/src/pages/AnalysisSessionPage.test.jsx @@ -75,7 +75,7 @@ describe('a session nobody has sat', () => { expect(await screen.findByRole('link', { name: 'Resume session' })) .toHaveAttribute('href', '/study/3?start=1') expect(screen.queryByRole('link', { name: /Review/ })).not.toBeInTheDocument() - expect(screen.queryByRole('button', { name: 'Delete session' })).not.toBeInTheDocument() + expect(screen.queryByRole('button', { name: /Delete/ })).toBeNull() }) it('asks the quiz endpoint, since there is no attempt to ask about', async () => { @@ -96,7 +96,10 @@ describe('a session that has been sat', () => { // 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() - expect(screen.getByRole('button', { name: 'Delete session' })).toBeInTheDocument() + // No delete. A session is a record of work done; removing one edits the + // history every figure here is computed from. Starting again is offered + // whole, under Settings, Your data. + expect(screen.queryByRole('button', { name: /Delete/ })).toBeNull() // Nothing outstanding, so no "still unanswered" note. expect(screen.queryByText(/still unanswered/)).not.toBeInTheDocument() }) @@ -117,16 +120,6 @@ describe('a session whose only attempt is still in progress', () => { mock({ ...SAT, attempt_id: 77, completed_at: null, answered: 0, score: 0, percent: 0 }) }) - it('deletes the attempt the page is showing, not the one named in the URL', async () => { - mountQuiz() - await screen.findByRole('button', { name: 'Delete session' }) - await userEvent.click(screen.getByRole('button', { name: 'Delete session' })) - api.delete.mockResolvedValue({}) - await userEvent.click(screen.getByRole('button', { name: 'Delete for good' })) - // It used to send /attempts/undefined, and nothing happened. - await waitFor(() => expect(api.delete).toHaveBeenCalledWith('/attempts/77')) - }) - it('offers to resume it, not to review it', async () => { mountQuiz() // Review or resume, never both: a session you are part-way through has diff --git a/frontend/src/pages/QuestionBankPage.css b/frontend/src/pages/QuestionBankPage.css index 91a627c..201fe78 100644 --- a/frontend/src/pages/QuestionBankPage.css +++ b/frontend/src/pages/QuestionBankPage.css @@ -25,8 +25,14 @@ .category-tree { list-style: none; margin: 0; padding: 0; max-height: 46vh; overflow-y: auto; } .category-tree ul { list-style: none; margin: 0 0 0 16px; padding: 0; } .category-tree li { margin: 1px 0; } -.category-tree label { display: flex; align-items: baseline; gap: 6px; font-size: .84rem; cursor: pointer; } -.category-tree-count { color: var(--text-muted); font-size: .74rem; } +.category-tree label { display: flex; align-items: baseline; gap: 8px; font-size: .84rem; cursor: pointer; text-align: left; } +.category-tree label > input[type="checkbox"] { flex: none; } +/* The name owns the row and reads from the left, wherever the tree is used. + It had no styles of its own, so it took whatever the host page did to a + label — which in the question editor centred it, leaving the name floating + in the middle with the count adrift at the far right. */ +.category-tree-name { flex: 1; min-width: 0; text-align: left; overflow-wrap: anywhere; } +.category-tree-count { flex: none; color: var(--text-muted); font-size: .74rem; } .category-tree-excluded { opacity: .5; } .category-tree-branch { display: flex; align-items: center; cursor: pointer; list-style: none; } .category-tree-branch::-webkit-details-marker { display: none; } diff --git a/frontend/src/pages/QuestionEditPage.css b/frontend/src/pages/QuestionEditPage.css index 485c29f..f9387da 100644 --- a/frontend/src/pages/QuestionEditPage.css +++ b/frontend/src/pages/QuestionEditPage.css @@ -119,8 +119,15 @@ @media (max-width: 900px) { - .qe-grid { grid-template-columns: 1fr; } - .qe-aside { position: static; } + /* minmax(0, 1fr), not 1fr: a plain `1fr` track takes its minimum from its + content, so one child that will not shrink — the formatting bar, a long + word, a wide table — pushes the column past the window. The page then had + padding down its left and none down its right, because the right was off + the screen rather than absent. */ + .qe-grid { grid-template-columns: minmax(0, 1fr); } + .qe-aside { position: static; min-width: 0; } + /* The unnamed column holding the cards. */ + .qe-grid > * { min-width: 0; } } @media (max-width: 640px) { .qe-top-actions { width: 100%; }