feat: edit quiz questions over MCP

- quizzes_create_question, update_question, delete_question and
  reorder_questions on /api/v1/quizzes/{slug}/questions
- update_question replaces answers by id (kept, created, deleted), so
  answer ids stored in past attempts stay valid; foreign ids are refused
- classes.CheckQuestion holds the importer's per-question rules, shared
  with the manage routes; attempt scores are never touched (D-16)
This commit is contained in:
Jakub Zych
2026-11-09 14:33:56 +01:00
parent 4a60eda425
commit c6d8d1a973
11 changed files with 792 additions and 44 deletions

View File

@@ -97,40 +97,7 @@ func ParseImport(raw []byte) (*ImportQuiz, error) {
problems = append(problems, "questions must not be empty")
}
for i, q := range doc.Questions {
at := fmt.Sprintf("questions[%d]", i)
if strings.TrimSpace(q.Text) == "" {
problems = append(problems, at+".text is required")
}
if !slices.Contains(models.QuestionTypes, q.Type) {
problems = append(problems, fmt.Sprintf("%s.type must be one of %s", at, strings.Join(models.QuestionTypes, ", ")))
continue
}
if q.Points != nil && *q.Points < 0 {
problems = append(problems, at+".points must not be negative")
}
correct := 0
for j, a := range q.Answers {
if strings.TrimSpace(a.Text) == "" {
problems = append(problems, fmt.Sprintf("%s.answers[%d].text is required", at, j))
}
if a.IsCorrect {
correct++
}
}
switch q.Type {
case models.TypeSingle:
if len(q.Answers) < 2 || correct != 1 {
problems = append(problems, at+": a single question needs at least two answers and exactly one correct")
}
case models.TypeMultiple:
if len(q.Answers) < 2 || correct < 1 {
problems = append(problems, at+": a multiple question needs at least two answers and one or more correct")
}
case models.TypeSelfCheck:
if len(q.Answers) > 0 {
problems = append(problems, at+": a self_check question has no answers; use answer_text")
}
}
problems = append(problems, CheckQuestion(fmt.Sprintf("questions[%d]", i), q)...)
}
if len(problems) > 0 {
return nil, fmt.Errorf("invalid quiz: %s", strings.Join(problems, "; "))
@@ -138,6 +105,49 @@ func ParseImport(raw []byte) (*ImportQuiz, error) {
return &doc, nil
}
// CheckQuestion returns the problems of one question, each prefixed with
// at (such as "questions[0]"): the text is required, the type is one of
// models.QuestionTypes, points are not negative, answer texts are
// required, a single question has at least two answers and exactly one
// correct, a multiple question at least two and one or more correct, and a
// self_check question none. The importer and the manage routes share it.
func CheckQuestion(at string, q ImportQuestion) []string {
var problems []string
if strings.TrimSpace(q.Text) == "" {
problems = append(problems, at+".text is required")
}
if !slices.Contains(models.QuestionTypes, q.Type) {
return append(problems, fmt.Sprintf("%s.type must be one of %s", at, strings.Join(models.QuestionTypes, ", ")))
}
if q.Points != nil && *q.Points < 0 {
problems = append(problems, at+".points must not be negative")
}
correct := 0
for j, a := range q.Answers {
if strings.TrimSpace(a.Text) == "" {
problems = append(problems, fmt.Sprintf("%s.answers[%d].text is required", at, j))
}
if a.IsCorrect {
correct++
}
}
switch q.Type {
case models.TypeSingle:
if len(q.Answers) < 2 || correct != 1 {
problems = append(problems, at+": a single question needs at least two answers and exactly one correct")
}
case models.TypeMultiple:
if len(q.Answers) < 2 || correct < 1 {
problems = append(problems, at+": a multiple question needs at least two answers and one or more correct")
}
case models.TypeSelfCheck:
if len(q.Answers) > 0 {
problems = append(problems, at+": a self_check question has no answers; use answer_text")
}
}
return problems
}
// ImportResult reports what an import wrote.
type ImportResult struct {
Quiz models.Quiz

View File

@@ -1,6 +1,7 @@
package classes_test
import (
"encoding/json"
"os"
"strings"
"testing"
@@ -82,3 +83,35 @@ func TestParseImportTestMode(t *testing.T) {
t.Errorf("sample = %+v %v, want no test-mode keys", doc, err)
}
}
// TestCheckQuestion: the per-question rules ParseImport applies, shared
// with the manage routes; ParseImport reports exactly these problems.
func TestCheckQuestion(t *testing.T) {
neg := -1
for name, tc := range map[string]struct {
q classes.ImportQuestion
want []string
}{
"no text": {classes.ImportQuestion{Type: "self_check"}, []string{"questions[0].text is required"}},
"bad type": {classes.ImportQuestion{Text: "Q", Type: "essay"}, []string{"questions[0].type must be one of single, multiple, self_check"}},
"negative points": {classes.ImportQuestion{Text: "Q", Type: "self_check", Points: &neg}, []string{"questions[0].points must not be negative"}},
"single two right": {classes.ImportQuestion{Text: "Q", Type: "single", Answers: []classes.ImportAnswer{{Text: "a", IsCorrect: true}, {Text: "b", IsCorrect: true}}}, []string{"questions[0]: a single question needs at least two answers and exactly one correct"}},
"multiple one item": {classes.ImportQuestion{Text: "Q", Type: "multiple", Answers: []classes.ImportAnswer{{Text: "a", IsCorrect: true}}}, []string{"questions[0]: a multiple question needs at least two answers and one or more correct"}},
"self_check answer": {classes.ImportQuestion{Text: "Q", Type: "self_check", Answers: []classes.ImportAnswer{{Text: "a"}}}, []string{"questions[0]: a self_check question has no answers; use answer_text"}},
"blank answer": {classes.ImportQuestion{Text: "Q", Type: "single", Answers: []classes.ImportAnswer{{Text: " ", IsCorrect: true}, {Text: "b"}}}, []string{"questions[0].answers[0].text is required"}},
"valid single": {classes.ImportQuestion{Text: "Q", Type: "single", Answers: []classes.ImportAnswer{{Text: "a", IsCorrect: true}, {Text: "b"}}}, nil},
} {
got := classes.CheckQuestion("questions[0]", tc.q)
if strings.Join(got, "; ") != strings.Join(tc.want, "; ") {
t.Errorf("%s: %q, want %q", name, got, tc.want)
}
doc, _ := json.Marshal(map[string]any{"title": "T", "questions": []classes.ImportQuestion{tc.q}})
_, err := classes.ParseImport(doc)
switch {
case tc.want == nil && err != nil:
t.Errorf("%s: ParseImport: %v", name, err)
case tc.want != nil && (err == nil || err.Error() != "invalid quiz: "+strings.Join(tc.want, "; ")):
t.Errorf("%s: ParseImport = %v, want the same problems", name, err)
}
}
}

View File

@@ -8,6 +8,7 @@ package classes
import (
"context"
"errors"
"fmt"
"math"
"slices"
"strings"
@@ -20,6 +21,10 @@ import (
"gorm.io/gorm/clause"
)
// ErrQuestionNotFound is a question id that is not a question of the quiz.
// It is an ErrNotFound.
var ErrQuestionNotFound = fmt.Errorf("%w: question", ErrNotFound)
// ValidationError is manage input that breaks a rule (a 422). Problems
// read like the importer's: "title is required".
type ValidationError struct {
@@ -591,3 +596,325 @@ func BuildStats(quiz models.Quiz, questions []models.Question, students []Studen
}
return out
}
// ManagedAnswerInput is one answer of a question write. An ID keeps and
// updates that answer of the question; no ID creates one. Explanation nil
// keeps a kept answer's explanation, "" clears it.
type ManagedAnswerInput struct {
ID *uint `json:"id"`
Text string `json:"text"`
IsCorrect bool `json:"is_correct"`
Explanation *string `json:"explanation"`
}
// QuestionInput is the fields of a question create or partial update; nil
// means not sent. Answers, when sent, is the question's full new answer
// list in order. An empty string clears category, answer_text and
// explanation.
type QuestionInput struct {
Text *string `json:"text"`
Type *string `json:"type"`
Category *string `json:"category"`
AnswerText *string `json:"answer_text"`
Explanation *string `json:"explanation"`
Points *int `json:"points"`
SortOrder *int `json:"sort_order"`
Answers *[]ManagedAnswerInput `json:"answers"`
}
// MergeQuestion applies the sent fields of in, and its answers list when
// sent, to current (a zero Question for a create) and returns the result
// in the importer's shape, for CheckQuestion. An answer id that is not an
// answer of current, or is listed twice, is a problem.
func MergeQuestion(current models.Question, in QuestionInput) (ImportQuestion, []string) {
q := ImportQuestion{
Text: current.Text, Type: current.Type, Category: current.Category,
AnswerText: current.AnswerText, Explanation: current.Explanation,
}
points := current.Points
if current.ID == 0 {
points = 1
}
if in.Text != nil {
q.Text = strings.TrimSpace(*in.Text)
}
if in.Type != nil {
q.Type = *in.Type
}
if in.Category != nil {
q.Category = nilIfBlank(*in.Category, true)
}
if in.AnswerText != nil {
q.AnswerText = nilIfBlank(*in.AnswerText, false)
}
if in.Explanation != nil {
q.Explanation = nilIfBlank(*in.Explanation, false)
}
if in.Points != nil {
points = *in.Points
}
q.Points = &points
var problems []string
if in.Category != nil && utf8.RuneCountInString(strings.TrimSpace(*in.Category)) > 255 {
problems = append(problems, "category must be at most 255 characters")
}
if in.Answers == nil {
for _, a := range current.Answers {
q.Answers = append(q.Answers, ImportAnswer{Text: a.Text, IsCorrect: a.IsCorrect, Explanation: a.Explanation})
}
return q, problems
}
byID := map[uint]models.Answer{}
for _, a := range current.Answers {
byID[a.ID] = a
}
seen := map[uint]bool{}
q.Answers = make([]ImportAnswer, 0, len(*in.Answers))
for i, a := range *in.Answers {
out := ImportAnswer{Text: strings.TrimSpace(a.Text), IsCorrect: a.IsCorrect, Explanation: a.Explanation}
if a.ID != nil {
kept, ok := byID[*a.ID]
switch {
case !ok:
problems = append(problems, fmt.Sprintf("answers[%d].id does not belong to this question", i))
case seen[*a.ID]:
problems = append(problems, fmt.Sprintf("answers[%d].id is listed twice", i))
case a.Explanation == nil:
out.Explanation = kept.Explanation
}
seen[*a.ID] = true
}
if out.Explanation != nil {
out.Explanation = nilIfBlank(*out.Explanation, false)
}
q.Answers = append(q.Answers, out)
}
return q, problems
}
// CheckNewQuestion is the rule check of a create, before any database
// access: text and type are required, answers carry no id, and the merged
// question passes CheckQuestion.
func CheckNewQuestion(in QuestionInput) []string {
var problems []string
if in.Text == nil {
problems = append(problems, "text is required")
}
if in.Type == nil {
problems = append(problems, "type is required")
}
if len(problems) > 0 {
return problems
}
merged, problems := MergeQuestion(models.Question{}, in)
if in.SortOrder != nil && *in.SortOrder < 1 {
problems = append(problems, "sort_order must be at least 1")
}
return append(problems, CheckQuestion("question", merged)...)
}
// managedQuizID loads the id of the quiz with this slug, or ErrNotFound.
func managedQuizID(tx *gorm.DB, slug string) (uint, error) {
var quiz models.Quiz
if err := lockedQuiz(tx, slug, &quiz); err != nil {
return 0, err
}
return quiz.ID, nil
}
// CreateQuestion adds a question with its answers to the quiz with this
// slug: at sort_order when given, else after the last question. Points
// default to 1; answers are ordered as sent. A missing slug is ErrNotFound.
func CreateQuestion(ctx context.Context, db *gorm.DB, slug string, in QuestionInput) (*models.Question, error) {
if err := invalid(CheckNewQuestion(in)); err != nil {
return nil, err
}
merged, _ := MergeQuestion(models.Question{}, in)
var q models.Question
err := lagoon.Transaction(ctx, db, func(ctx context.Context, tx *gorm.DB) error {
tx = tx.WithContext(ctx)
quizID, err := managedQuizID(tx, slug)
if err != nil {
return err
}
order := 0
if in.SortOrder != nil {
order = *in.SortOrder
} else if err := tx.Model(&models.Question{}).Where("quiz_id = ?", quizID).
Select("COALESCE(MAX(sort_order), 0) + 1").Scan(&order).Error; err != nil {
return err
}
q = models.Question{
QuizID: quizID, Text: merged.Text, Type: merged.Type, Category: merged.Category,
AnswerText: merged.AnswerText, Explanation: merged.Explanation, SortOrder: order, Points: *merged.Points,
}
if err := tx.Omit(clause.Associations).Create(&q).Error; err != nil {
return err
}
for i, a := range merged.Answers {
ans := models.Answer{QuestionID: q.ID, Text: a.Text, IsCorrect: a.IsCorrect, Explanation: a.Explanation, SortOrder: i + 1}
if err := tx.Omit(clause.Associations).Create(&ans).Error; err != nil {
return err
}
q.Answers = append(q.Answers, ans)
}
return nil
})
if err != nil {
return nil, err
}
return &q, nil
}
// UpdateQuestion changes only the sent fields of one question of the quiz
// with this slug. When answers is sent it is the full new list: answers
// with an id are updated in place (so the answer ids stored in past
// attempts stay valid), answers without one are created, answers missing
// from the list are deleted, and the order is the list order. The merged
// question must pass CheckQuestion. Stored attempt answers and scores are
// never touched (D-16). A missing quiz is ErrNotFound, a missing question
// ErrQuestionNotFound.
func UpdateQuestion(ctx context.Context, db *gorm.DB, slug string, id uint, in QuestionInput) (*models.Question, error) {
if in.SortOrder != nil && *in.SortOrder < 1 {
return nil, invalid([]string{"sort_order must be at least 1"})
}
var q models.Question
err := lagoon.Transaction(ctx, db, func(ctx context.Context, tx *gorm.DB) error {
tx = tx.WithContext(ctx)
quizID, err := managedQuizID(tx, slug)
if err != nil {
return err
}
err = tx.Clauses(clause.Locking{Strength: "UPDATE", Table: clause.Table{Name: clause.CurrentTable}}).
Where("id = ? AND quiz_id = ?", id, quizID).Take(&q).Error
if errors.Is(err, gorm.ErrRecordNotFound) {
return ErrQuestionNotFound
}
if err != nil {
return err
}
if err := tx.Where("question_id = ?", q.ID).Order("sort_order ASC, id ASC").Find(&q.Answers).Error; err != nil {
return err
}
merged, problems := MergeQuestion(q, in)
if err := invalid(append(problems, CheckQuestion("question", merged)...)); err != nil {
return err
}
q.Text, q.Type, q.Category = merged.Text, merged.Type, merged.Category
q.AnswerText, q.Explanation, q.Points = merged.AnswerText, merged.Explanation, *merged.Points
if in.SortOrder != nil {
q.SortOrder = *in.SortOrder
}
if err := tx.Omit(clause.Associations).Save(&q).Error; err != nil {
return err
}
if in.Answers == nil {
return nil
}
current := map[uint]models.Answer{}
for _, a := range q.Answers {
current[a.ID] = a
}
answers := make([]models.Answer, 0, len(merged.Answers))
keep := map[uint]bool{}
for i, a := range merged.Answers {
ans := models.Answer{QuestionID: q.ID}
if sent := (*in.Answers)[i]; sent.ID != nil {
ans = current[*sent.ID]
keep[ans.ID] = true
}
ans.Text, ans.IsCorrect, ans.Explanation, ans.SortOrder = a.Text, a.IsCorrect, a.Explanation, i+1
if err := tx.Omit(clause.Associations).Save(&ans).Error; err != nil {
return err
}
answers = append(answers, ans)
}
for _, a := range q.Answers {
if !keep[a.ID] {
if err := tx.Where("id = ?", a.ID).Delete(&models.Answer{}).Error; err != nil {
return err
}
}
}
q.Answers = answers
return nil
})
if err != nil {
return nil, err
}
return &q, nil
}
// DeleteQuestion removes one question of the quiz with this slug. The
// foreign keys cascade to its answers and the attempt answers given to
// it; finished attempts keep their stored scores (D-16).
func DeleteQuestion(ctx context.Context, db *gorm.DB, slug string, id uint) error {
return lagoon.Transaction(ctx, db, func(ctx context.Context, tx *gorm.DB) error {
tx = tx.WithContext(ctx)
quizID, err := managedQuizID(tx, slug)
if err != nil {
return err
}
res := tx.Where("id = ? AND quiz_id = ?", id, quizID).Delete(&models.Question{})
if res.Error != nil {
return res.Error
}
if res.RowsAffected == 0 {
return ErrQuestionNotFound
}
return nil
})
}
// CheckQuestionOrder is the check of a reorder list before any database
// access: not empty, no id twice.
func CheckQuestionOrder(ids []uint) []string {
seen := map[uint]bool{}
for _, id := range ids {
if seen[id] {
return []string{"question_ids must list every question of the quiz exactly once"}
}
seen[id] = true
}
if len(ids) == 0 {
return []string{"question_ids must list every question of the quiz exactly once"}
}
return nil
}
// ReorderQuestions sets sort_order = index + 1 for ids, which must be
// exactly the quiz's question ids, and returns the managed quiz.
func ReorderQuestions(ctx context.Context, db *gorm.DB, slug string, ids []uint) (*models.Quiz, error) {
if err := invalid(CheckQuestionOrder(ids)); err != nil {
return nil, err
}
err := lagoon.Transaction(ctx, db, func(ctx context.Context, tx *gorm.DB) error {
tx = tx.WithContext(ctx)
quizID, err := managedQuizID(tx, slug)
if err != nil {
return err
}
var have []uint
if err := tx.Model(&models.Question{}).Where("quiz_id = ?", quizID).Pluck("id", &have).Error; err != nil {
return err
}
sorted := slices.Clone(ids)
slices.Sort(sorted)
slices.Sort(have)
if !slices.Equal(sorted, have) {
return invalid([]string{"question_ids must list every question of the quiz exactly once"})
}
now := time.Now().UTC()
for i, id := range ids {
if err := tx.Model(&models.Question{}).Where("id = ?", id).
Updates(map[string]any{"sort_order": i + 1, "updated_at": now}).Error; err != nil {
return err
}
}
return nil
})
if err != nil {
return nil, err
}
return ManagedQuiz(ctx, db, slug)
}

View File

@@ -112,3 +112,74 @@ func TestBuildStats(t *testing.T) {
t.Errorf("most_chosen_wrong by count = %+v, want 103", w)
}
}
func uid(n uint) *uint { return &n }
// TestQuestionInputRules: MergeQuestion applies a partial update and the
// answers list to the current question, and with CheckQuestion refuses
// what the importer would refuse.
func TestQuestionInputRules(t *testing.T) {
single := models.Question{ID: 1, Text: "Stolica?", Type: models.TypeSingle, Points: 2, Answers: []models.Answer{
{ID: 10, QuestionID: 1, Text: "Warszawa", IsCorrect: true, Explanation: str("Od 1596.")},
{ID: 11, QuestionID: 1, Text: "Kraków"},
}}
card := models.Question{ID: 2, Text: "Odporność?", Type: models.TypeSelfCheck, AnswerText: str("Zdolność"), Points: 1}
check := func(current models.Question, in classes.QuestionInput) (classes.ImportQuestion, []string) {
merged, problems := classes.MergeQuestion(current, in)
return merged, append(problems, classes.CheckQuestion("question", merged)...)
}
merged, problems := check(single, classes.QuestionInput{Text: str(" Stolica Polski? ")})
if len(problems) != 0 || merged.Text != "Stolica Polski?" || *merged.Points != 2 || len(merged.Answers) != 2 || *merged.Answers[0].Explanation != "Od 1596." {
t.Errorf("text only: %+v %v", merged, problems)
}
answers := []classes.ManagedAnswerInput{{ID: uid(99), Text: "Gdańsk", IsCorrect: true}, {ID: uid(11), Text: "Kraków"}}
if _, problems := check(single, classes.QuestionInput{Answers: &answers}); !strings.Contains(strings.Join(problems, "; "), "answers[0].id does not belong to this question") {
t.Errorf("foreign answer id: %v", problems)
}
twice := []classes.ManagedAnswerInput{{ID: uid(10), Text: "Warszawa", IsCorrect: true}, {ID: uid(10), Text: "Kraków"}}
if _, problems := check(single, classes.QuestionInput{Answers: &twice}); !strings.Contains(strings.Join(problems, "; "), "listed twice") {
t.Errorf("answer id twice: %v", problems)
}
two := []classes.ManagedAnswerInput{{ID: uid(10), Text: "Warszawa", IsCorrect: true}, {ID: uid(11), Text: "Kraków", IsCorrect: true}}
if _, problems := check(single, classes.QuestionInput{Answers: &two}); !strings.Contains(strings.Join(problems, "; "), "exactly one correct") {
t.Errorf("single with two correct: %v", problems)
}
if _, problems := check(single, classes.QuestionInput{Type: str(models.TypeSelfCheck)}); !strings.Contains(strings.Join(problems, "; "), "self_check question has no answers") {
t.Errorf("type to self_check with answers left: %v", problems)
}
kept := []classes.ManagedAnswerInput{{ID: uid(10), Text: "Warszawa", IsCorrect: true}, {Text: "Gdańsk", Explanation: str("")}}
merged, problems = check(single, classes.QuestionInput{Answers: &kept, Explanation: str("")})
if len(problems) != 0 || len(merged.Answers) != 2 || *merged.Answers[0].Explanation != "Od 1596." || merged.Answers[1].Explanation != nil || merged.Explanation != nil {
t.Errorf("kept and new answers: %+v %v", merged, problems)
}
toSingle := []classes.ManagedAnswerInput{{Text: "Tak", IsCorrect: true}, {Text: "Nie"}}
merged, problems = check(card, classes.QuestionInput{Type: str(models.TypeSingle), Answers: &toSingle, AnswerText: str("")})
if len(problems) != 0 || merged.Type != models.TypeSingle || merged.AnswerText != nil {
t.Errorf("self_check to single: %+v %v", merged, problems)
}
if problems := classes.CheckNewQuestion(classes.QuestionInput{Text: str("Q")}); !strings.Contains(strings.Join(problems, "; "), "type is required") {
t.Errorf("create without type: %v", problems)
}
withID := []classes.ManagedAnswerInput{{ID: uid(10), Text: "a", IsCorrect: true}, {Text: "b"}}
if problems := classes.CheckNewQuestion(classes.QuestionInput{Text: str("Q"), Type: str("single"), Answers: &withID}); !strings.Contains(strings.Join(problems, "; "), "does not belong") {
t.Errorf("create with an answer id: %v", problems)
}
if problems := classes.CheckNewQuestion(classes.QuestionInput{Text: str("Q"), Type: str("self_check")}); len(problems) != 0 {
t.Errorf("minimal self_check create: %v", problems)
}
}
// TestCheckQuestionOrder: an empty list and a repeated id are refused
// before the database; a missing or foreign id is refused by
// ReorderQuestions against the quiz's questions.
func TestCheckQuestionOrder(t *testing.T) {
for _, ids := range [][]uint{nil, {}, {1, 2, 1}} {
if problems := classes.CheckQuestionOrder(ids); len(problems) != 1 {
t.Errorf("%v: %v, want one problem", ids, problems)
}
}
if problems := classes.CheckQuestionOrder([]uint{3, 1, 2}); len(problems) != 0 {
t.Errorf("valid order: %v", problems)
}
}