fix: resolve latest review findings

Category quiz creation counts extra-linked questions; primary category is validated on edit; statistics dedupe collapses case variants. 60 backend tests pass.
This commit is contained in:
Daniel 2026-09-08 19:53:25 +02:00
parent 179e51d143
commit ff1aee6fad
5 changed files with 17 additions and 9 deletions

View file

@ -8,7 +8,7 @@ from app.database import get_db
from app.models.question import Question
from app.models.question_category import QuestionCategory, QuestionCategoryLink
from app.models.user import User
from app.services.quiz_builder import (bank_query, category_descendants, category_breadcrumbs,
from app.services.quiz_builder import (bank_query, filtered_bank_query, category_descendants, category_breadcrumbs,
validate_parent, GenerateTestRequest, generate_test)
from app.utils.auth import get_current_user, require_moderator
@ -107,11 +107,13 @@ def create_quiz_from_question_category(cat_id: int, title: str, mode: str = "tim
time_limit_minutes: int | None = None, count: int | None = None,
db: Session = Depends(get_db), current_user: User = Depends(get_current_user)):
ids = category_descendants(db.query(QuestionCategory).all(), [cat_id])
available = bank_query(db, current_user).filter(Question.question_category_id.in_(ids)).count()
available = filtered_bank_query(db, current_user, [cat_id]).count()
if mode not in ("timed", "learning") or not title.strip() or len(title) > 200 or (time_limit_minutes is not None and time_limit_minutes <= 0):
raise HTTPException(400, "Provide a title, valid mode, and positive time limit")
if available == 0:
raise HTTPException(400, "No available questions in this category")
count = available if count is None else count
if not 1 <= count <= 200:
raise HTTPException(400, "Select between 1 and 200 available questions using Create Custom Test")
if not 1 <= count <= min(200, available):
raise HTTPException(400, "Select between 1 and the available questions using Create Custom Test")
return generate_test(db, current_user, GenerateTestRequest(title=title, mode=mode,
time_limit_minutes=time_limit_minutes, count=count, category_ids=[cat_id]))

View file

@ -89,6 +89,8 @@ def edit_question(
if question.user_id != current_user.id and not is_mod:
raise HTTPException(status_code=403, detail="Not authorized to edit this question")
values = validate_image_attachments(db, current_user, data.model_dump(exclude_unset=True))
if values.get("question_category_id") is not None and not db.get(QuestionCategory, values["question_category_id"]):
raise HTTPException(400, "Category not found")
explanations = values.pop("option_explanations", None)
if explanations is not None:
values["option_explanations"] = validate_option_explanations(

View file

@ -227,9 +227,9 @@ def question_responses(attempt_id: int, question_id: int, db: Session = Depends(
for answer, count in rows:
if answer and answer.strip():
counts[answer.strip().casefold()] += count
# Duplicate option strings must not double-count the same answer.
options = list(dict.fromkeys(question.options or []))
stats = [{"option": option, "count": counts[option.strip().casefold()]} for option in options]
# Duplicate option strings must not double-count the same answer; case variants collapse too.
options = list(dict.fromkeys((option.casefold() for option in (question.options or []))))
stats = [{"option": option, "count": counts[option.casefold()]} for option in options]
sample_size = sum(option["count"] for option in stats)
for option in stats:
option["percentage"] = round(100 * option["count"] / sample_size, 1) if sample_size else 0

View file

@ -39,6 +39,9 @@ class MultiCategoryTests(unittest.TestCase):
self.assertEqual([row['id'] for row in bank['questions']], [3])
self.assertEqual(set(bank['questions'][0]['category_ids']), {2, 3, 4})
self.assertEqual(self.client.get('/questions/bank/ids', params={'category_ids': '4'}).json(), [3])
# Category quiz creation counts extra-linked questions too.
created = self.client.post('/question-categories/4/create-quiz', params={'title': 'Linked quiz', 'mode': 'learning'})
self.assertEqual(created.status_code, 200, created.text)
# Peers cannot see the private question through the shared category.
self.bank.user = self.bank.peer
self.assertEqual(self.client.get('/questions/builder/count', params={'category_ids': [4]}).json()['count'], 0)
@ -46,6 +49,7 @@ class MultiCategoryTests(unittest.TestCase):
def test_additional_categories_replace_validate_and_ignore_primary(self):
self.bank.user = self.bank.owner
self.assertEqual(self.client.patch('/questions/3', json={'additional_category_ids': [999]}).status_code, 400)
self.assertEqual(self.client.patch('/questions/3', json={'question_category_id': 999}).status_code, 400)
self.assertEqual(self.client.patch('/questions/3', json={'additional_category_ids': [3, 4, 4]}).status_code, 200)
links = {link.category_id for link in self.bank.db.query(QuestionCategoryLink).filter_by(question_id=3)}
self.assertEqual(links, {4}) # Primary is ignored; duplicates collapsed.

View file

@ -126,9 +126,9 @@ class StudyToolTests(unittest.TestCase):
self.assertEqual(response['sample_size'], 1)
self.assertEqual([row['count'] for row in response['options']], [1, 0])
self.assertEqual(response['options'][0]['percentage'], 100.0)
# Duplicate option strings must not double-count.
# Duplicate option strings must not double-count; case variants collapse too.
question = self.bank.db.get(fixtures.Question, 1)
question.options = ['yes', 'yes']
question.options = ['yes', 'YES']
self.bank.db.commit()
response = self.client.get(f'/study-tools/attempts/{study}/questions/1/responses').json()
self.assertEqual(response['sample_size'], 1)