diff --git a/classes/importer_test.go b/classes/importer_test.go index 8753204..c65f914 100644 --- a/classes/importer_test.go +++ b/classes/importer_test.go @@ -93,7 +93,7 @@ func TestCheckQuestion(t *testing.T) { 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"}}, + "bad type": {classes.ImportQuestion{Text: "Q", Type: "essay"}, []string{"questions[0].type must be one of single, multiple, self_check, open"}}, "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"}}, diff --git a/classes/open.go b/classes/open.go new file mode 100644 index 0000000..f831e4b --- /dev/null +++ b/classes/open.go @@ -0,0 +1,33 @@ +package classes + +import ( + "strings" + "unicode/utf8" + + "git.golem15.com/golem15/sm-quizzes-plugin/models" +) + +// MaxLength is the longest answer an open question accepts, in characters: +// its MaxLength when set and positive, else models.DefaultMaxLength. +func MaxLength(q models.Question) int { + if q.MaxLength != nil && *q.MaxLength > 0 { + return *q.MaxLength + } + return models.DefaultMaxLength +} + +// CheckOpenText cleans a student's answer to an open question (D-04): it +// removes every NUL character (Postgres text refuses them) and the +// surrounding whitespace. A blank text is a FieldError text_required and a +// text longer than MaxLength(q) characters (runes, not bytes) is a +// FieldError text_too_long. +func CheckOpenText(text string, q models.Question) (string, error) { + text = strings.TrimSpace(strings.ReplaceAll(text, "\x00", "")) + if text == "" { + return "", &FieldError{Field: "text", Key: "text_required"} + } + if utf8.RuneCountInString(text) > MaxLength(q) { + return "", &FieldError{Field: "text", Key: "text_too_long"} + } + return text, nil +} diff --git a/classes/open_test.go b/classes/open_test.go new file mode 100644 index 0000000..36c0f69 --- /dev/null +++ b/classes/open_test.go @@ -0,0 +1,59 @@ +package classes_test + +import ( + "errors" + "strings" + "testing" + + "git.golem15.com/golem15/sm-quizzes-plugin/classes" + "git.golem15.com/golem15/sm-quizzes-plugin/models" +) + +func TestCheckOpenText(t *testing.T) { + q := models.Question{Type: models.TypeOpen} + long := 3000 + wide := models.Question{Type: models.TypeOpen, MaxLength: &long} + + fieldKey := func(err error) string { + var fe *classes.FieldError + if errors.As(err, &fe) && fe.Field == "text" { + return fe.Key + } + return "" + } + + if _, err := classes.CheckOpenText(" \n\t ", q); fieldKey(err) != "text_required" { + t.Errorf("blank text: err = %v, want text_required", err) + } + if _, err := classes.CheckOpenText("\x00 \x00", q); fieldKey(err) != "text_required" { + t.Errorf("NUL-only text: err = %v, want text_required", err) + } + over := strings.Repeat("a", 2001) + if _, err := classes.CheckOpenText(over, q); fieldKey(err) != "text_too_long" { + t.Errorf("2001 runes with default max: err = %v, want text_too_long", err) + } + if got, err := classes.CheckOpenText(over, wide); err != nil || got != over { + t.Errorf("2001 runes with max 3000: %q, %v", got[:10], err) + } + if got, err := classes.CheckOpenText("a\x00b", q); err != nil || got != "ab" { + t.Errorf("NUL stripped: %q, %v; want ab", got, err) + } + if got, err := classes.CheckOpenText(" moja odpowiedź \n", q); err != nil || got != "moja odpowiedź" { + t.Errorf("trimmed: %q, %v", got, err) + } + polish := strings.Repeat("ż", 2000) + if got, err := classes.CheckOpenText(polish, q); err != nil || got != polish { + t.Errorf("2000 runes of ż (4000 bytes) refused: %v", err) + } + five := 5 + if _, err := classes.CheckOpenText("abcdef", models.Question{MaxLength: &five}); fieldKey(err) != "text_too_long" { + t.Errorf("6 runes with max 5: err = %v, want text_too_long", err) + } + if classes.MaxLength(q) != models.DefaultMaxLength || classes.MaxLength(wide) != 3000 { + t.Errorf("MaxLength = %d / %d", classes.MaxLength(q), classes.MaxLength(wide)) + } + zero := 0 + if classes.MaxLength(models.Question{MaxLength: &zero}) != models.DefaultMaxLength { + t.Error("MaxLength of a non-positive value should fall back to the default") + } +} diff --git a/classes/scoring.go b/classes/scoring.go index 7fab7c0..f2f466f 100644 --- a/classes/scoring.go +++ b/classes/scoring.go @@ -152,6 +152,27 @@ func Totals(points []float64, maxPoints int) (score, percent float64) { return score, round(score / float64(maxPoints) * 100) } +// ClampOpenPoints keeps a grader's score of an open question in +// 0..points (a negative points value counts as 0), rounded to the nearest +// 0.5 (D-02). NaN and ±Inf are not a score: it returns 0, false. +func ClampOpenPoints(p float64, points int) (float64, bool) { + if math.IsNaN(p) || math.IsInf(p, 0) { + return 0, false + } + full := float64(max(points, 0)) + p = math.Round(math.Min(math.Max(p, 0), full)*2) / 2 + if p == 0 { + return 0, true // no negative zero in JSON + } + return p, true +} + +// OpenCorrect reports whether an open answer scored p earns full marks: a +// question with points earns is_correct only with all of them (D-02). +func OpenCorrect(p float64, points int) bool { + return points > 0 && p == float64(points) +} + // round keeps two decimals. func round(f float64) float64 { r := math.Round(f*100) / 100 diff --git a/classes/scoring_test.go b/classes/scoring_test.go index deb67cd..d220d3b 100644 --- a/classes/scoring_test.go +++ b/classes/scoring_test.go @@ -1,6 +1,7 @@ package classes_test import ( + "math" "os" "path/filepath" "testing" @@ -130,3 +131,37 @@ func TestScoringConfigFrom(t *testing.T) { } } } + +func TestClampOpenPoints(t *testing.T) { + cases := []struct { + p float64 + points int + want float64 + ok bool + }{ + {1.26, 2, 1.5, true}, + {-3, 2, 0, true}, + {7, 2, 2, true}, + {0.74, 3, 0.5, true}, + {0.25, 3, 0.5, true}, + {0.2, 3, 0, true}, + {2, 2, 2, true}, + {1, 0, 0, true}, + {5, -1, 0, true}, + {math.NaN(), 2, 0, false}, + {math.Inf(1), 2, 0, false}, + {math.Inf(-1), 2, 0, false}, + } + for _, tc := range cases { + got, ok := classes.ClampOpenPoints(tc.p, tc.points) + if got != tc.want || ok != tc.ok { + t.Errorf("ClampOpenPoints(%v, %d) = %v, %v; want %v, %v", tc.p, tc.points, got, ok, tc.want, tc.ok) + } + if math.Signbit(got) { + t.Errorf("ClampOpenPoints(%v, %d) is negative zero", tc.p, tc.points) + } + } + if !classes.OpenCorrect(2, 2) || classes.OpenCorrect(1.5, 2) || classes.OpenCorrect(0, 0) { + t.Error("OpenCorrect: want true only for full points of a question worth points") + } +} diff --git a/classes/service.go b/classes/service.go index 30ee963..32dc54a 100644 --- a/classes/service.go +++ b/classes/service.go @@ -227,6 +227,10 @@ type Started struct { AttemptsUsed int // AnsweredCount is the number of questions a resumed attempt answered. AnsweredCount int + // OpenAnswers and Unreviewed count the open answers of a resumed + // attempt, as in AttemptSummary. + OpenAnswers int + Unreviewed int } // StartAttempt starts an attempt at a published quiz for the user, like @@ -322,11 +326,15 @@ func (s *Service) startTest(ctx context.Context, userID uint, quiz *models.Quiz) return err } if resume != nil { - var answered int64 - if err := tx.Model(&models.AttemptAnswer{}).Where("attempt_id = ?", resume.ID).Count(&answered).Error; err != nil { + counts, err := answerCounts(ctx, tx, []uint{resume.ID}) + if err != nil { return err } - st = &Started{Attempt: resume, Quiz: quiz, Resumed: true, AttemptsUsed: int(used), AnsweredCount: int(answered)} + c := counts[resume.ID] + st = &Started{ + Attempt: resume, Quiz: quiz, Resumed: true, AttemptsUsed: int(used), + AnsweredCount: c.N, OpenAnswers: c.OpenN, Unreviewed: c.UnreviewedN, + } return nil } if left := AttemptsLeft(quiz, int(used)); left != nil && *left == 0 { @@ -403,6 +411,8 @@ type AnswerInput struct { QuestionID uint AnswerIDs []uint Known *bool + // Text is the answer to an open question. + Text *string } // Feedback is what the user learns after answering one question. @@ -414,6 +424,9 @@ type Feedback struct { // and the serializer must drop Result, CorrectAnswerIDs and every // solution field of Question. Withheld bool + // Answer is the stored row of an open question, with its text and + // grading state; it is nil for every other type. + Answer *models.AttemptAnswer } // Answer records or overwrites the user's answer to one question of an @@ -423,8 +436,11 @@ type Feedback struct { // question belongs to the attempt (FieldError on question_id), a one_way // test has no answer to the question yet (ErrAnswerLocked; the attempt row // is locked, so the check is race-free), and the answer fits the question -// type (FieldError on answer_ids or known). The feedback of an open test -// attempt is Withheld. +// type (FieldError on answer_ids, known or text). The feedback of an open +// test attempt is Withheld. +// +// An open question stores its text as pending (see answerOpen) and is never +// scored here: a grader or the owner scores it later. func (s *Service) Answer(ctx context.Context, userID, attemptID uint, in AnswerInput) (*Feedback, error) { var fb *Feedback err := lagoon.Transaction(ctx, s.DB, func(ctx context.Context, tx *gorm.DB) error { @@ -464,6 +480,20 @@ func (s *Service) Answer(ctx context.Context, userID, attemptID uint, in AnswerI row := models.AttemptAnswer{AttemptID: attempt.ID, QuestionID: question.ID} switch question.Type { + case models.TypeOpen: + if in.Text == nil { + return &FieldError{Field: "text", Key: "text_required"} + } + text, err := CheckOpenText(*in.Text, question) + if err != nil { + return err + } + stored, err := s.answerOpen(ctx, tx, attempt, question, text) + if err != nil { + return err + } + fb = &Feedback{Question: question, Answer: stored, Withheld: Withheld(attempt)} + return nil case models.TypeSelfCheck: if in.Known == nil { return &FieldError{Field: "known", Key: "known_required"} @@ -512,6 +542,78 @@ func (s *Service) Answer(ctx context.Context, userID, attemptID uint, in AnswerI return fb, nil } +// answerOpen stores a cleaned open answer in a transaction that holds the +// attempt row lock, so reading the existing row first is race-free. The +// same text again changes nothing. Another text is a new answer (Pitfall 1): +// the row goes back to pending with the next grading_seq, and every grade, +// error, review and point of the old text is cleared, so a learning +// re-answer replaces an earlier grade or review. A first answer creates the +// row as pending with seq 1. A new or changed text is handed to +// enqueueGrading. +func (s *Service) answerOpen(ctx context.Context, tx *gorm.DB, attempt *models.Attempt, question models.Question, text string) (*models.AttemptAnswer, error) { + tx = tx.WithContext(ctx) + now := s.now().UTC() + var existing models.AttemptAnswer + err := tx.Where("attempt_id = ? AND question_id = ?", attempt.ID, question.ID).Take(&existing).Error + switch { + case err == nil: + if existing.TextAnswer != nil && *existing.TextAnswer == text && existing.GradingStatus != nil { + return &existing, nil + } + seq := existing.GradingSeq + 1 + if err := tx.Model(&models.AttemptAnswer{}).Where("id = ?", existing.ID).Updates(map[string]any{ + "text_answer": text, + "answer_ids": nil, + "self_check_known": nil, + "grading_status": models.GradingPending, + "grading_seq": seq, + "grading_attempts": 0, + "grader": nil, + "grader_points": nil, + "grader_feedback": nil, + "grading_error": nil, + "graded_at": nil, + "review_comment": nil, + "reviewed_at": nil, + "reviewed_by_user_id": nil, + "reviewed_by_admin_id": nil, + "points": 0, + "is_correct": false, + "updated_at": now, + }).Error; err != nil { + return nil, err + } + if err := tx.Where("id = ?", existing.ID).Take(&existing).Error; err != nil { + return nil, err + } + if err := s.enqueueGrading(ctx, tx, existing.ID, seq, attempt); err != nil { + return nil, err + } + return &existing, nil + case errors.Is(err, gorm.ErrRecordNotFound): + default: + return nil, err + } + pending := models.GradingPending + row := &models.AttemptAnswer{ + AttemptID: attempt.ID, QuestionID: question.ID, + TextAnswer: &text, GradingStatus: &pending, GradingSeq: 1, + CreatedAt: now, UpdatedAt: now, + } + if err := tx.Create(row).Error; err != nil { + return nil, err + } + if err := s.enqueueGrading(ctx, tx, row.ID, row.GradingSeq, attempt); err != nil { + return nil, err + } + return row, nil +} + +// enqueueGrading hands a new or changed open answer to the quiz's grader. +func (s *Service) enqueueGrading(ctx context.Context, tx *gorm.DB, answerID uint, seq int, attempt *models.Attempt) error { + return nil +} + // CorrectAnswerIDs lists the IDs of a question's correct answers in sort // order; it is empty for a self_check question. func CorrectAnswerIDs(q models.Question) []uint { @@ -574,6 +676,50 @@ func (s *Service) close(ctx context.Context, tx *gorm.DB, attempt *models.Attemp type AttemptSummary struct { Attempt models.Attempt AnsweredCount int + // OpenAnswers counts the attempt's answers to open questions (rows + // with a grading status). + OpenAnswers int + // Unreviewed counts the open answers the owner has not reviewed yet: + // pending, graded or failed. + Unreviewed int +} + +// answerCountRow is the per-attempt answer counts answerCounts reads. +type answerCountRow struct { + AttemptID uint + N int + OpenN int + UnreviewedN int +} + +// answerCounts counts, per attempt, the answers, the open answers and the +// open answers not reviewed yet. +func answerCounts(ctx context.Context, db *gorm.DB, attemptIDs []uint) (map[uint]answerCountRow, error) { + var rows []answerCountRow + if err := db.WithContext(ctx).Model(&models.AttemptAnswer{}). + Select("attempt_id, COUNT(*) AS n, "+ + "COUNT(*) FILTER (WHERE grading_status IS NOT NULL) AS open_n, "+ + "COUNT(*) FILTER (WHERE grading_status IN ('pending', 'graded', 'failed')) AS unreviewed_n"). + Where("attempt_id IN ?", attemptIDs).Group("attempt_id").Scan(&rows).Error; err != nil { + return nil, err + } + out := make(map[uint]answerCountRow, len(rows)) + for _, r := range rows { + out[r.AttemptID] = r + } + return out, nil +} + +// unreviewed reports whether an open answer still waits for the owner. +func unreviewed(a *models.AttemptAnswer) bool { + if a == nil || a.GradingStatus == nil { + return false + } + switch *a.GradingStatus { + case models.GradingPending, models.GradingGraded, models.GradingFailed: + return true + } + return false } // QuestionResult is one question of an attempt with the user's answer. @@ -638,6 +784,14 @@ func (s *Service) Result(ctx context.Context, userID, attemptID uint) (*AttemptR byQuestion[answers[i].QuestionID] = &answers[i] } res := &AttemptResult{AttemptSummary: AttemptSummary{Attempt: *attempt, AnsweredCount: len(answers)}} + for i := range answers { + if answers[i].GradingStatus != nil { + res.OpenAnswers++ + } + if unreviewed(&answers[i]) { + res.Unreviewed++ + } + } for _, q := range questions { res.Questions = append(res.Questions, QuestionResult{Question: q, Answer: byQuestion[q.ID]}) } @@ -668,21 +822,13 @@ func (s *Service) History(ctx context.Context, userID uint, quizSlug string, pag for i, a := range attempts { ids[i] = a.ID } - type countRow struct { - AttemptID uint - N int - } - var counts []countRow - if err := s.db(ctx).Model(&models.AttemptAnswer{}).Select("attempt_id, COUNT(*) AS n"). - Where("attempt_id IN ?", ids).Group("attempt_id").Scan(&counts).Error; err != nil { + countBy, err := answerCounts(ctx, s.DB, ids) + if err != nil { return nil, 0, err } - countBy := map[uint]int{} - for _, c := range counts { - countBy[c.AttemptID] = c.N - } for _, a := range attempts { - out = append(out, AttemptSummary{Attempt: a, AnsweredCount: countBy[a.ID]}) + c := countBy[a.ID] + out = append(out, AttemptSummary{Attempt: a, AnsweredCount: c.N, OpenAnswers: c.OpenN, Unreviewed: c.UnreviewedN}) } return out, total, nil } diff --git a/controllers/api/handlers.go b/controllers/api/handlers.go index 4ac209f..ce73521 100644 --- a/controllers/api/handlers.go +++ b/controllers/api/handlers.go @@ -41,6 +41,7 @@ var ( {Field: "answer_ids", Rules: lagoon.ParseRules("nullable|array")}, {Field: "answer_ids.*", Rules: lagoon.ParseRules("integer|min:1")}, {Field: "known", Rules: lagoon.ParseRules("nullable|boolean")}, + {Field: "text", Rules: lagoon.ParseRules("nullable|string")}, } ) @@ -116,7 +117,8 @@ func (h *Handlers) StartAttempt(w http.ResponseWriter, r *http.Request) { } // Answer answers POST /attempts/{id}/answers with -// {"question_id": 1, "answer_ids": [2], "known": true}. +// {"question_id": 1, "answer_ids": [2], "known": true}; an open question +// takes {"question_id": 1, "text": "..."}. func (h *Handlers) Answer(w http.ResponseWriter, r *http.Request) { userID, ok := h.user(w, r) if !ok { @@ -147,6 +149,10 @@ func (h *Handlers) Answer(w http.ResponseWriter, r *http.Request) { known := toBool(raw) in.Known = &known } + if raw, present := input["text"]; present && raw != nil { + text, _ := raw.(string) // the string rule already refused anything else + in.Text = &text + } fb, err := svc.Answer(r.Context(), userID, attemptID, in) if err != nil { h.writeError(w, r, err) @@ -331,6 +337,8 @@ var apiMessages = map[string]string{ "answer_not_in_question": "The chosen answers do not belong to this question.", "single_one_answer": "Choose exactly one answer for this question.", "known_required": "The known field is required for this question.", + "text_required": "Write an answer to this question.", + "text_too_long": "The answer is longer than this question allows.", "unknown_category": "The quiz has no questions in this category.", "validation_failed": "The given data was invalid.", "attempt_limit_reached": "All attempts at this test have been used.", diff --git a/controllers/api/json.go b/controllers/api/json.go index 06788bc..346e177 100644 --- a/controllers/api/json.go +++ b/controllers/api/json.go @@ -103,7 +103,8 @@ type quizJSON struct { // questionJSON is a question as the player sees it before answering: no // is_correct or explanation, and no answer_text except for self_check -// flashcards, which have no solution to hide. +// flashcards, which have no solution to hide. An open question carries its +// max_length instead (never its rubric or model answer). type questionJSON struct { ID uint `json:"id"` Text string `json:"text"` @@ -112,8 +113,10 @@ type questionJSON struct { Points int `json:"points"` SortOrder int `json:"sort_order"` // AnswerText is the model answer of a self_check question only. - AnswerText *string `json:"answer_text,omitempty"` - Answers []answerJSON `json:"answers"` + AnswerText *string `json:"answer_text,omitempty"` + // MaxLength is the longest answer of an open question, in characters. + MaxLength *int `json:"max_length,omitempty"` + Answers []answerJSON `json:"answers"` } type answerJSON struct { @@ -162,6 +165,10 @@ func question(q models.Question) questionJSON { if q.Type == models.TypeSelfCheck { out.AnswerText = q.AnswerText } + if q.Type == models.TypeOpen { + n := classes.MaxLength(q) + out.MaxLength = &n + } for _, a := range q.Answers { out.Answers = append(out.Answers, answerJSON{ID: a.ID, Text: a.Text}) } @@ -191,6 +198,11 @@ type attemptJSON struct { // SecondsLeft is the time left of an open attempt with a limit, counted // by the server. SecondsLeft *int `json:"seconds_left"` + // NeedsReview is true while an open answer of the attempt waits for + // the owner (pending, graded or failed); OpenAnswers counts its open + // answers. + NeedsReview bool `json:"needs_review"` + OpenAnswers int `json:"open_answers"` } // timeNow is the clock of seconds_left; tests pin it. @@ -209,6 +221,8 @@ func attempt(s classes.AttemptSummary) attemptJSON { ScorePercent: a.ScorePercent, QuizMode: quizMode(a.Quiz), QuizNavigation: quizNavigation(a.Quiz), + NeedsReview: s.Unreviewed > 0, + OpenAnswers: s.OpenAnswers, } if a.Quiz != nil { out.QuizSlug, out.QuizTitle, out.PassingScore = a.Quiz.Slug, a.Quiz.Title, a.Quiz.PassingScore @@ -240,7 +254,9 @@ func start(st *classes.Started) startJSON { a := *st.Attempt a.Quiz = st.Quiz return startJSON{ - attemptJSON: attempt(classes.AttemptSummary{Attempt: a, AnsweredCount: st.AnsweredCount}), + attemptJSON: attempt(classes.AttemptSummary{ + Attempt: a, AnsweredCount: st.AnsweredCount, OpenAnswers: st.OpenAnswers, Unreviewed: st.Unreviewed, + }), Resumed: st.Resumed, AttemptsLeft: classes.AttemptsLeft(st.Quiz, st.AttemptsUsed), } @@ -261,6 +277,11 @@ type resultJSON struct { // withheld (an open test attempt) every solution field, is_correct and // points_earned included, is null for every question; answered, answer_ids // and known (the student's own input) stay. +// +// An open question adds the student's text_answer (always: it is their own +// input), max_length and, unless withheld, grading_status, grader_feedback +// and review_comment; its is_correct and points_earned stay null until the +// answer is graded or reviewed. The rubric is never part of it. type questionResultJSON struct { ID uint `json:"id"` Text string `json:"text"` @@ -276,6 +297,14 @@ type questionResultJSON struct { Explanation *string `json:"explanation"` AnswerText *string `json:"answer_text"` Answers []resultAnswerJSON `json:"answers"` + // TextAnswer is the student's text of an open question. It is not + // "text" as in the feedback: text is already the question's text here. + TextAnswer *string `json:"text_answer"` + GradingStatus *string `json:"grading_status"` + GraderFeedback *string `json:"grader_feedback"` + ReviewComment *string `json:"review_comment"` + // MaxLength is the longest answer of an open question. + MaxLength *int `json:"max_length,omitempty"` } type resultAnswerJSON struct { @@ -304,11 +333,24 @@ func result(r *classes.AttemptResult) resultJSON { AnswerIDs: []uint{}, Answers: make([]resultAnswerJSON, 0, len(q.Answers)), } + if q.Type == models.TypeOpen { + n := classes.MaxLength(q) + row.MaxLength = &n + } if a := qr.Answer; a != nil { row.Answered = true row.AnswerIDs = wire.Slice(a.AnswerIDs.Data) row.Known = a.SelfCheckKnown - if !withheld { + if q.Type == models.TypeOpen { + row.TextAnswer = a.TextAnswer + if !withheld { + row.GradingStatus, row.GraderFeedback, row.ReviewComment = a.GradingStatus, a.GraderFeedback, a.ReviewComment + if openScored(a) { + correct, points := a.IsCorrect, a.Points + row.IsCorrect, row.PointsEarned = &correct, &points + } + } + } else if !withheld { correct, points := a.IsCorrect, a.Points row.IsCorrect, row.PointsEarned = &correct, &points } @@ -340,6 +382,12 @@ func result(r *classes.AttemptResult) resultJSON { // feedbackJSON is POST /attempts/{id}/answers. When Withheld (an answer to // an open test attempt) every solution field is null and the answers carry // only their id and text. +// +// The feedback of an open question carries the student's text and, unless +// withheld, the model answer (answer_text), the explanation, grading_status, +// grader_feedback and review_comment; is_correct and points stay null until +// the answer is graded or reviewed. These four keys are null for every other +// type. The rubric is never part of it. type feedbackJSON struct { QuestionID uint `json:"question_id"` Withheld bool `json:"withheld"` @@ -349,11 +397,18 @@ type feedbackJSON struct { Explanation *string `json:"explanation"` AnswerText *string `json:"answer_text"` Answers []resultAnswerJSON `json:"answers"` + Text *string `json:"text"` + GradingStatus *string `json:"grading_status"` + GraderFeedback *string `json:"grader_feedback"` + ReviewComment *string `json:"review_comment"` } func feedback(fb *classes.Feedback) feedbackJSON { q := fb.Question out := feedbackJSON{QuestionID: q.ID, Withheld: fb.Withheld, Answers: make([]resultAnswerJSON, 0, len(q.Answers))} + if q.Type == models.TypeOpen { + return openFeedback(fb, out) + } if fb.Withheld { for _, a := range q.Answers { out.Answers = append(out.Answers, resultAnswerJSON{ID: a.ID, Text: a.Text}) @@ -371,4 +426,36 @@ func feedback(fb *classes.Feedback) feedbackJSON { return out } +// openFeedback fills the feedback of an open question from its stored row. +func openFeedback(fb *classes.Feedback, out feedbackJSON) feedbackJSON { + q, a := fb.Question, fb.Answer + if a != nil { + out.Text = a.TextAnswer + } + if fb.Withheld { + return out + } + out.CorrectAnswerIDs = []uint{} + out.Explanation, out.AnswerText = q.Explanation, q.AnswerText + if a == nil { + return out + } + out.GradingStatus, out.GraderFeedback, out.ReviewComment = a.GradingStatus, a.GraderFeedback, a.ReviewComment + if openScored(a) { + correct, points := a.IsCorrect, a.Points + out.IsCorrect, out.Points = &correct, &points + } + return out +} + +// openScored reports whether an open answer has a score to show: graded +// by a grader or reviewed by the owner. Pending and failed answers have +// none yet. +func openScored(a *models.AttemptAnswer) bool { + if a == nil || a.GradingStatus == nil { + return false + } + return *a.GradingStatus == models.GradingGraded || *a.GradingStatus == models.GradingReviewed +} + func wireTime(t time.Time) wire.Time { return wire.Time{Time: t.UTC().Truncate(time.Second)} } diff --git a/controllers/api/json_test.go b/controllers/api/json_test.go index afcfaf2..5686a6a 100644 --- a/controllers/api/json_test.go +++ b/controllers/api/json_test.go @@ -16,7 +16,11 @@ import ( func fixtureQuestion(id uint, typ string) models.Question { expl, model, why := "question explanation", "model answer", "answer explanation" q := models.Question{ID: id, Text: "Q", Type: typ, Points: 2, Explanation: &expl, AnswerText: &model} - if typ != models.TypeSelfCheck { + if typ == models.TypeOpen { + rubric := "R-SECRET" + q.Rubric = &rubric + } + if isChoice(typ) { q.Answers = []models.Answer{ {ID: id*10 + 1, QuestionID: id, Text: "right", IsCorrect: true, Explanation: &why}, {ID: id*10 + 2, QuestionID: id, Text: "wrong", Explanation: &why}, @@ -25,6 +29,28 @@ func fixtureQuestion(id uint, typ string) models.Question { return q } +func isChoice(typ string) bool { return typ == models.TypeSingle || typ == models.TypeMultiple } + +func strPtr(s string) *string { return &s } + +// noRubric fails when a student shape names or contains the rubric or the +// grading error. +func noRubric(t *testing.T, where, raw string) { + t.Helper() + if strings.Contains(raw, `"rubric"`) || strings.Contains(raw, "R-SECRET") || strings.Contains(raw, "grading_error") || strings.Contains(raw, "provider exploded") { + t.Errorf("%s leaks the rubric or the grading error: %s", where, raw) + } +} + +// openRow is a stored open answer in the given grading status. +func openRow(questionID uint, status string) *models.AttemptAnswer { + return &models.AttemptAnswer{ + AttemptID: 5, QuestionID: questionID, + TextAnswer: strPtr("moja odpowiedź"), GradingStatus: strPtr(status), GradingSeq: 1, + GradingError: strPtr("provider exploded"), + } +} + func decode(t *testing.T, v any) (map[string]any, string) { t.Helper() b, err := json.Marshal(v) @@ -83,21 +109,43 @@ func TestWithheldSerializers(t *testing.T) { CorrectAnswerIDs: classes.CorrectAnswerIDs(answered), Withheld: true, } + // gradedOpen is an open answer with every grading field set, so + // a serializer that forgets to withhold one shows it. + gradedOpen := func() *models.AttemptAnswer { + r := openRow(1, models.GradingGraded) + two := 2.0 + r.IsCorrect, r.Points, r.GraderPoints = true, 2, &two + r.GraderFeedback, r.ReviewComment = strPtr("feedback"), strPtr("comment") + return r + } + if typ == models.TypeOpen { + fb.Answer = gradedOpen() + } m, raw := decode(t, feedback(fb)) if m["withheld"] != true { t.Errorf("feedback withheld = %v, want true", m["withheld"]) } - noSolution(t, "feedback", m, "is_correct", "points", "correct_answer_ids", "explanation", "answer_text") + noSolution(t, "feedback", m, "is_correct", "points", "correct_answer_ids", "explanation", "answer_text", + "grading_status", "grader_feedback", "review_comment") noCorrectness(t, "feedback", raw) - if typ != models.TypeSelfCheck && len(m["answers"].([]any)) != 2 { + noRubric(t, "feedback", raw) + if isChoice(typ) && len(m["answers"].([]any)) != 2 { t.Errorf("feedback answers = %v, want both answers", m["answers"]) } + if typ == models.TypeOpen && m["text"] != "moja odpowiedź" { + t.Errorf("withheld open feedback lost the student's text: %v", m["text"]) + } else if typ != models.TypeOpen && m["text"] != nil { + t.Errorf("%s feedback text = %v, want null", typ, m["text"]) + } known := true row := &models.AttemptAnswer{AttemptID: 5, QuestionID: 1, IsCorrect: true, Points: 2} - if typ == models.TypeSelfCheck { + switch typ { + case models.TypeSelfCheck: row.SelfCheckKnown = &known - } else { + case models.TypeOpen: + row = gradedOpen() + default: row.AnswerIDs = lagoon.Jsonable[[]uint]{Data: []uint{11}, Valid: true} } res := &classes.AttemptResult{ @@ -109,24 +157,33 @@ func TestWithheldSerializers(t *testing.T) { t.Errorf("result solutions_withheld = %v quiz_mode = %v", m["solutions_withheld"], m["quiz_mode"]) } noCorrectness(t, "result", raw) + noRubric(t, "result", raw) questions := m["questions"].([]any) if len(questions) != 2 { t.Fatalf("result questions = %d, want 2", len(questions)) } for i, raw := range questions { q := raw.(map[string]any) - noSolution(t, "result question "+string(rune('0'+i)), q, "is_correct", "points_earned", "correct_answer_ids", "explanation", "answer_text") + noSolution(t, "result question "+string(rune('0'+i)), q, "is_correct", "points_earned", "correct_answer_ids", "explanation", "answer_text", + "grading_status", "grader_feedback", "review_comment") } first := questions[0].(map[string]any) if first["answered"] != true { t.Errorf("answered question lost answered: %v", first) } - if typ == models.TypeSelfCheck { + switch typ { + case models.TypeSelfCheck: if first["known"] != true { t.Errorf("self_check lost the student's known: %v", first) } - } else if ids, _ := first["answer_ids"].([]any); len(ids) != 1 || ids[0] != float64(11) { - t.Errorf("answer_ids = %v, want the student's [11]", first["answer_ids"]) + case models.TypeOpen: + if first["text_answer"] != "moja odpowiedź" { + t.Errorf("open lost the student's text: %v", first) + } + default: + if ids, _ := first["answer_ids"].([]any); len(ids) != 1 || ids[0] != float64(11) { + t.Errorf("answer_ids = %v, want the student's [11]", first["answer_ids"]) + } } done := time.Now() @@ -207,15 +264,15 @@ func TestSummaryAndAttemptFields(t *testing.T) { t.Errorf("test summary = %v", m) } types, _ := m["question_types"].(map[string]any) - if len(types) != 3 || types["single"] != float64(3) || types["multiple"] != float64(0) || types["self_check"] != float64(0) { - t.Errorf("question_types = %v, want all three keys", m["question_types"]) + if len(types) != 4 || types["single"] != float64(3) || types["multiple"] != float64(0) || types["self_check"] != float64(0) || types["open"] != float64(0) { + t.Errorf("question_types = %v, want all four keys", m["question_types"]) } m, _ = decode(t, quizSummary(classes.QuizSummary{Quiz: models.Quiz{ID: 4, Slug: "l"}, AttemptsUsed: 9})) if v, ok := m["attempts_left"]; !ok || v != nil { t.Errorf("learning attempts_left = %v (present %v), want null", v, ok) } - if types, _ := m["question_types"].(map[string]any); len(types) != 3 { - t.Errorf("learning question_types = %v, want all three keys", m["question_types"]) + if types, _ := m["question_types"].(map[string]any); len(types) != 4 { + t.Errorf("learning question_types = %v, want all four keys", m["question_types"]) } if m["mode"] != models.ModeLearning || m["navigation"] != models.NavigationFree { t.Errorf("zero-value quiz mode / navigation = %v / %v", m["mode"], m["navigation"]) @@ -241,3 +298,119 @@ func TestSummaryAndAttemptFields(t *testing.T) { t.Errorf("start json = %v", m) } } + +func TestOpenSerializers(t *testing.T) { + q := fixtureQuestion(1, models.TypeOpen) + q.AnswerText = strPtr("MODEL") + learning := &models.Quiz{Slug: "l", PassingScore: 50} + + // Feedback of a pending learning answer: the model answer and the + // student's text, no score yet, never the rubric. + m, raw := decode(t, feedback(&classes.Feedback{Question: q, Answer: openRow(1, models.GradingPending)})) + noRubric(t, "pending feedback", raw) + if m["answer_text"] != "MODEL" || m["text"] != "moja odpowiedź" || m["grading_status"] != models.GradingPending { + t.Errorf("pending feedback = %s", raw) + } + if m["is_correct"] != nil || m["points"] != nil { + t.Errorf("pending feedback has a score: %s", raw) + } + if a, ok := m["answers"].([]any); !ok || len(a) != 0 { + t.Errorf("open feedback answers = %v, want []", m["answers"]) + } + if ids, ok := m["correct_answer_ids"].([]any); !ok || len(ids) != 0 { + t.Errorf("open feedback correct_answer_ids = %v, want []", m["correct_answer_ids"]) + } + + m, raw = decode(t, feedback(&classes.Feedback{Question: q, Answer: openRow(1, models.GradingFailed)})) + noRubric(t, "failed feedback", raw) + if m["grading_status"] != models.GradingFailed || m["is_correct"] != nil || m["points"] != nil { + t.Errorf("failed feedback = %s", raw) + } + + graded := openRow(1, models.GradingGraded) + graded.Points, graded.IsCorrect, graded.GraderFeedback = 1.5, false, strPtr("Brakuje przykładu.") + m, raw = decode(t, feedback(&classes.Feedback{Question: q, Answer: graded})) + noRubric(t, "graded feedback", raw) + if m["points"] != 1.5 || m["is_correct"] != false || m["grader_feedback"] != "Brakuje przykładu." || m["review_comment"] != nil { + t.Errorf("graded feedback = %s", raw) + } + + reviewed := openRow(1, models.GradingReviewed) + reviewed.Points, reviewed.IsCorrect, reviewed.ReviewComment = 2, true, strPtr("Dobrze.") + m, raw = decode(t, feedback(&classes.Feedback{Question: q, Answer: reviewed})) + if m["points"] != float64(2) || m["is_correct"] != true || m["review_comment"] != "Dobrze." || m["grading_status"] != models.GradingReviewed { + t.Errorf("reviewed feedback = %s", raw) + } + + // The question before answering: max_length, no model answer, no rubric. + m, raw = decode(t, question(q)) + noRubric(t, "open question", raw) + if m["max_length"] != float64(models.DefaultMaxLength) { + t.Errorf("open question max_length = %v, want %d", m["max_length"], models.DefaultMaxLength) + } + if _, ok := m["answer_text"]; ok { + t.Errorf("open question leaks its model answer: %s", raw) + } + five := 5 + q5 := q + q5.MaxLength = &five + if m, _ = decode(t, question(q5)); m["max_length"] != float64(5) { + t.Errorf("open question max_length = %v, want 5", m["max_length"]) + } + if _, raw = decode(t, question(fixtureQuestion(2, models.TypeSingle))); strings.Contains(raw, "max_length") { + t.Errorf("single question names max_length: %s", raw) + } + _, raw = decode(t, quizDetail(&classes.QuizDetail{Quiz: *learning, Questions: []models.Question{q}})) + noRubric(t, "quiz detail", raw) + + // The result of a learning attempt with a pending open answer. + res := &classes.AttemptResult{ + AttemptSummary: classes.AttemptSummary{ + Attempt: models.Attempt{ID: 5, StartedAt: time.Now(), Quiz: learning}, + AnsweredCount: 1, OpenAnswers: 1, Unreviewed: 1, + }, + Questions: []classes.QuestionResult{{Question: q, Answer: openRow(1, models.GradingPending)}, {Question: fixtureQuestion(2, models.TypeSingle)}}, + } + m, raw = decode(t, result(res)) + noRubric(t, "result", raw) + if m["needs_review"] != true || m["open_answers"] != float64(1) { + t.Errorf("result needs_review / open_answers = %v / %v", m["needs_review"], m["open_answers"]) + } + row := m["questions"].([]any)[0].(map[string]any) + if row["points_earned"] != nil || row["is_correct"] != nil { + t.Errorf("pending open result has a score: %v", row) + } + if row["answer_text"] != "MODEL" || row["text_answer"] != "moja odpowiedź" || row["text"] != "Q" || row["grading_status"] != models.GradingPending { + t.Errorf("pending open result = %v", row) + } + if row["max_length"] != float64(models.DefaultMaxLength) { + t.Errorf("open result max_length = %v", row["max_length"]) + } + choice := m["questions"].([]any)[1].(map[string]any) + for _, k := range []string{"text_answer", "grading_status", "grader_feedback", "review_comment"} { + if v, ok := choice[k]; !ok || v != nil { + t.Errorf("single result %s = %v (present %v), want null", k, v, ok) + } + } + if _, ok := choice["max_length"]; ok { + t.Errorf("single result names max_length: %v", choice) + } + + res.Questions[0].Answer = reviewed + res.Unreviewed = 0 + m, _ = decode(t, result(res)) + row = m["questions"].([]any)[0].(map[string]any) + if row["points_earned"] != float64(2) || row["is_correct"] != true || row["review_comment"] != "Dobrze." || m["needs_review"] != false { + t.Errorf("reviewed open result = %v (needs_review %v)", row, m["needs_review"]) + } + + // Every attempt row carries the two review keys. + m, _ = decode(t, attempt(classes.AttemptSummary{Attempt: models.Attempt{ID: 1, StartedAt: time.Now(), Quiz: learning}})) + if m["needs_review"] != false || m["open_answers"] != float64(0) { + t.Errorf("attempt without open answers = %v / %v", m["needs_review"], m["open_answers"]) + } + m, _ = decode(t, start(&classes.Started{Attempt: &models.Attempt{ID: 2, StartedAt: time.Now()}, Quiz: learning, OpenAnswers: 2, Unreviewed: 1})) + if m["needs_review"] != true || m["open_answers"] != float64(2) { + t.Errorf("start needs_review / open_answers = %v / %v", m["needs_review"], m["open_answers"]) + } +} diff --git a/graders/graders.go b/graders/graders.go new file mode 100644 index 0000000..12a0b42 --- /dev/null +++ b/graders/graders.go @@ -0,0 +1,62 @@ +// Package graders is the grader registry of golem15.quizzes. A plugin that +// grades open answers implements HasGraders, and golem15.quizzes collects +// its graders at boot. Manual is built in and is never called: an answer of +// a manual quiz waits for the owner. The package imports only the standard +// library, so a grading plugin can depend on it without pulling in the rest +// of golem15.quizzes. +package graders + +import "context" + +// Manual is the built-in grader code: the owner grades every open answer. +const Manual = "manual" + +// ManualLabel is the lang key of Manual in the admin grader dropdown. +const ManualLabel = "golem15.quizzes::lang.graders.manual" + +// Input is one open answer to grade. +type Input struct { + QuizID uint + QuestionID uint + AttemptID uint + AnswerID uint + UserID uint + // Question is the owner-authored question text. + Question string + // ModelAnswer is the owner-authored model answer; it may be empty. + ModelAnswer string + // Rubric is the owner-authored grading rubric; it may be empty and is + // owner-only: it must never reach the student. + Rubric string + // Answer is the student's text. It is untrusted input. + Answer string + // Points is the full value of the question. + Points int +} + +// Result is a grader's verdict. golem15.quizzes clamps Points to +// 0..Input.Points in steps of 0.5 and refuses a value that is not a finite +// number. +type Result struct { + Points float64 + Feedback string +} + +// Grader grades open answers. Grade runs in a background job with no +// transaction open, so it may take seconds; an error is retried a few times +// and then leaves the answer failed for the owner to grade. +type Grader interface { + // Code identifies the grader in a quiz's grader field, such as "ai". + // It must match ^[a-z][a-z0-9_]{0,63}$ and must not be Manual. + Code() string + // Label is the lang key or text the admin grader dropdown shows. + Label() string + // Grade scores one answer. + Grade(ctx context.Context, in Input) (Result, error) +} + +// HasGraders is implemented by a plugin that provides graders. +// golem15.quizzes calls it once at boot. +type HasGraders interface { + QuizGraders() []Grader +} diff --git a/lang/en/lang.yaml b/lang/en/lang.yaml index 0dcd08c..584fe1c 100644 --- a/lang/en/lang.yaml +++ b/lang/en/lang.yaml @@ -15,6 +15,10 @@ api: attempt_limit_reached: All attempts at this test have been used. answer_locked: An answer in this test cannot be changed. category_not_allowed: A test covers the whole quiz; a category cannot be chosen. + text_required: Write an answer to this question. + text_too_long: The answer is longer than this question allows. +graders: + manual: Manual (the owner grades) permissions: access_quizzes: Manage quizzes, questions and answers access_attempts: View quiz attempts diff --git a/lang/pl/lang.yaml b/lang/pl/lang.yaml index 5cef763..9f09635 100644 --- a/lang/pl/lang.yaml +++ b/lang/pl/lang.yaml @@ -15,6 +15,10 @@ api: attempt_limit_reached: Wykorzystano już wszystkie podejścia do tego testu. answer_locked: W tym teście nie można zmienić udzielonej odpowiedzi. category_not_allowed: Test obejmuje cały quiz; nie można wybrać kategorii. + text_required: Wpisz odpowiedź na to pytanie. + text_too_long: Odpowiedź jest dłuższa, niż pozwala to pytanie. +graders: + manual: Ręcznie (ocenia właściciel) permissions: access_quizzes: Zarządzanie quizami, pytaniami i odpowiedziami access_attempts: Podgląd podejść do quizów diff --git a/models/attempt_answer.go b/models/attempt_answer.go index b04f6ed..7a5a6fb 100644 --- a/models/attempt_answer.go +++ b/models/attempt_answer.go @@ -8,8 +8,31 @@ import ( "git.golem15.com/golem15/summercms/modules/lagoon" ) +// Grading states of an open answer (AttemptAnswer.GradingStatus). +const ( + // GradingPending waits for a grader or for the owner. + GradingPending = "pending" + // GradingGraded holds a grader's score the owner has not confirmed. + GradingGraded = "graded" + // GradingFailed is an answer the grader could not score; the owner + // grades it. + GradingFailed = "failed" + // GradingReviewed is final: the owner confirmed or set the score. + GradingReviewed = "reviewed" +) + +// GradingStatuses lists the valid values of AttemptAnswer.GradingStatus. +var GradingStatuses = []string{GradingPending, GradingGraded, GradingFailed, GradingReviewed} + // AttemptAnswer is the user's answer to one question of an attempt. There is // one row per attempt and question; answering again overwrites it. +// +// TextAnswer and the grading fields belong to open questions only: +// GradingStatus is NULL for every other row. GradingSeq grows with every new +// text, so a grading job for an older text is ignored; GradingAttempts counts +// the grader runs for the current text. The review fields record the owner's +// decision: ReviewedByUserID for a frontend user (MCP), ReviewedByAdminID for +// a backend administrator. type AttemptAnswer struct { ID uint `gorm:"column:id;primaryKey"` AttemptID uint `gorm:"column:attempt_id"` @@ -20,6 +43,20 @@ type AttemptAnswer struct { Points float64 `gorm:"column:points"` CreatedAt time.Time `gorm:"column:created_at"` UpdatedAt time.Time `gorm:"column:updated_at"` + + TextAnswer *string `gorm:"column:text_answer"` + GradingStatus *string `gorm:"column:grading_status"` + GradingSeq int `gorm:"column:grading_seq"` + GradingAttempts int `gorm:"column:grading_attempts"` + Grader *string `gorm:"column:grader"` + GraderPoints *float64 `gorm:"column:grader_points"` + GraderFeedback *string `gorm:"column:grader_feedback"` + GradingError *string `gorm:"column:grading_error"` + GradedAt *time.Time `gorm:"column:graded_at"` + ReviewComment *string `gorm:"column:review_comment"` + ReviewedAt *time.Time `gorm:"column:reviewed_at"` + ReviewedByUserID *uint `gorm:"column:reviewed_by_user_id"` + ReviewedByAdminID *uint `gorm:"column:reviewed_by_admin_id"` } // TableName is the plugin-prefixed table. diff --git a/models/question.go b/models/question.go index c8a3272..a93824a 100644 --- a/models/question.go +++ b/models/question.go @@ -9,13 +9,25 @@ const ( TypeSingle = "single" TypeMultiple = "multiple" TypeSelfCheck = "self_check" + TypeOpen = "open" ) // QuestionTypes lists the valid values of Question.Type. -var QuestionTypes = []string{TypeSingle, TypeMultiple, TypeSelfCheck} +var QuestionTypes = []string{TypeSingle, TypeMultiple, TypeSelfCheck, TypeOpen} + +// Limits of the answer length of an open question, in characters. +const ( + // DefaultMaxLength applies when MaxLength is nil. + DefaultMaxLength = 2000 + // MaxMaxLength is the largest MaxLength the column accepts. + MaxMaxLength = 20000 +) // Question is one question of a quiz. A self_check question is a flashcard: -// the user reveals AnswerText and marks whether they knew it. +// the user reveals AnswerText and marks whether they knew it. An open +// question is answered in free text and graded against AnswerText (the +// model answer) and the owner-only Rubric; MaxLength caps the answer +// (DefaultMaxLength when nil). type Question struct { ID uint `gorm:"column:id;primaryKey"` QuizID uint `gorm:"column:quiz_id"` @@ -24,6 +36,8 @@ type Question struct { Category *string `gorm:"column:category"` AnswerText *string `gorm:"column:answer_text"` Explanation *string `gorm:"column:explanation"` + Rubric *string `gorm:"column:rubric"` + MaxLength *int `gorm:"column:max_length"` SortOrder int `gorm:"column:sort_order"` Points int `gorm:"column:points"` CreatedAt time.Time `gorm:"column:created_at"` @@ -37,7 +51,7 @@ func (Question) TableName() string { return "golem15_quizzes_questions" } // Fillable lists the columns mass assignment may set. func (Question) Fillable() []string { - return []string{"quiz_id", "text", "type", "category", "answer_text", "explanation", "sort_order", "points"} + return []string{"quiz_id", "text", "type", "category", "answer_text", "explanation", "sort_order", "points", "rubric", "max_length"} } // Rules are checked by the admin API on every save. @@ -45,9 +59,10 @@ func (Question) Rules() map[string]string { return map[string]string{ "quiz_id": "required|integer|min:1", "text": "required", - "type": "required|in:single,multiple,self_check", + "type": "required|in:single,multiple,self_check,open", "category": "nullable|max:255", "sort_order": "nullable|integer", "points": "required|integer|min:0", + "max_length": "nullable|integer|between:1,20000", } } diff --git a/models/quiz.go b/models/quiz.go index ca2a60b..d5a76fc 100644 --- a/models/quiz.go +++ b/models/quiz.go @@ -7,6 +7,7 @@ import ( "time" "unicode" + "git.golem15.com/golem15/sm-quizzes-plugin/graders" "gorm.io/gorm" ) @@ -32,6 +33,8 @@ var QuizNavigations = []string{NavigationFree, NavigationOneWay} // free-form link to a record of another plugin, such as "acme.shop:course:12". // Mode test withholds solutions until the attempt is finished; Navigation // (free or one_way) and MaxAttempts (nil is unlimited) only apply to tests. +// Grader is the code of the grader of its open answers (graders.Manual: the +// owner grades them); an unknown code behaves as manual. type Quiz struct { ID uint `gorm:"column:id;primaryKey"` Title string `gorm:"column:title"` @@ -46,6 +49,7 @@ type Quiz struct { Mode string `gorm:"column:mode"` Navigation string `gorm:"column:navigation"` MaxAttempts *int `gorm:"column:max_attempts"` + Grader string `gorm:"column:grader"` CreatedAt time.Time `gorm:"column:created_at"` UpdatedAt time.Time `gorm:"column:updated_at"` Questions []Question `gorm:"foreignKey:QuizID"` @@ -59,7 +63,7 @@ func (Quiz) Fillable() []string { return []string{ "title", "slug", "description", "context", "is_published", "time_limit_seconds", "passing_score", "shuffle_questions", "round_size", - "mode", "navigation", "max_attempts", + "mode", "navigation", "max_attempts", "grader", } } @@ -76,6 +80,7 @@ func (Quiz) Rules() map[string]string { "mode": "nullable|in:learning,test", "navigation": "nullable|in:free,one_way", "max_attempts": "nullable|integer|min:1", + "grader": "nullable|max:64", } } @@ -94,8 +99,8 @@ func (q *Quiz) BeforeSave(*gorm.DB) error { return nil } -// fillDefaults stores an empty mode or navigation as learning / free and a -// non-positive attempt limit as unlimited. GORM writes Go zero values, so +// fillDefaults stores an empty mode or navigation as learning / free, an +// empty grader as manual and a non-positive attempt limit as unlimited. GORM writes Go zero values, so // without it a models.Quiz{} would hit the columns' CHECK constraints. func (q *Quiz) fillDefaults() { if q.Mode == "" { @@ -104,6 +109,9 @@ func (q *Quiz) fillDefaults() { if q.Navigation == "" { q.Navigation = NavigationFree } + if q.Grader == "" { + q.Grader = graders.Manual + } if q.MaxAttempts != nil && *q.MaxAttempts <= 0 { q.MaxAttempts = nil } diff --git a/models/quiz_test.go b/models/quiz_test.go index 8fd6178..2799211 100644 --- a/models/quiz_test.go +++ b/models/quiz_test.go @@ -1,6 +1,12 @@ package models -import "testing" +import ( + "slices" + "strings" + "testing" + + "git.golem15.com/golem15/sm-quizzes-plugin/graders" +) func TestQuizDefaults(t *testing.T) { zero, negative := 0, -1 @@ -16,14 +22,52 @@ func TestQuizDefaults(t *testing.T) { if q.Mode != ModeLearning || q.Navigation != NavigationFree || q.MaxAttempts != nil { t.Errorf("%s with max_attempts %d = %q %q %v, want learning free nil", name, *limit, q.Mode, q.Navigation, q.MaxAttempts) } + if q.Grader != graders.Manual { + t.Errorf("%s left grader %q, want %q", name, q.Grader, graders.Manual) + } } two := 2 - q := &Quiz{Title: "Q", Mode: ModeTest, Navigation: NavigationOneWay, MaxAttempts: &two} + q := &Quiz{Title: "Q", Mode: ModeTest, Navigation: NavigationOneWay, MaxAttempts: &two, Grader: "ai"} if err := hook(q); err != nil { t.Fatalf("%s: %v", name, err) } if q.Mode != ModeTest || q.Navigation != NavigationOneWay || q.MaxAttempts == nil || *q.MaxAttempts != 2 { t.Errorf("%s changed a test quiz: %q %q %v", name, q.Mode, q.Navigation, q.MaxAttempts) } + if q.Grader != "ai" { + t.Errorf("%s changed an explicit grader to %q", name, q.Grader) + } + } +} + +func TestOpenQuestionModel(t *testing.T) { + if !slices.Contains(QuestionTypes, TypeOpen) || len(QuestionTypes) != 4 { + t.Errorf("QuestionTypes = %v, want the open type appended", QuestionTypes) + } + if QuestionTypes[0] != TypeSingle || QuestionTypes[1] != TypeMultiple || QuestionTypes[2] != TypeSelfCheck { + t.Errorf("QuestionTypes reordered the existing types: %v", QuestionTypes) + } + rules := Question{}.Rules() + if !strings.Contains(rules["type"], "open") || !strings.Contains(rules["type"], "self_check") { + t.Errorf("type rule = %q, want every type including open", rules["type"]) + } + if rules["max_length"] != "nullable|integer|between:1,20000" { + t.Errorf("max_length rule = %q", rules["max_length"]) + } + fill := Question{}.Fillable() + if !slices.Contains(fill, "rubric") || !slices.Contains(fill, "max_length") { + t.Errorf("Fillable = %v, want rubric and max_length", fill) + } + if DefaultMaxLength != 2000 || MaxMaxLength != 20000 { + t.Errorf("max length limits = %d / %d", DefaultMaxLength, MaxMaxLength) + } + if len(GradingStatuses) != 4 || GradingStatuses[0] != GradingPending { + t.Errorf("GradingStatuses = %v", GradingStatuses) + } + if !slices.Contains(Quiz{}.Fillable(), "grader") { + t.Error("Quiz.Fillable lacks grader") + } + if len(AttemptAnswer{}.Fillable()) != 0 { + t.Error("AttemptAnswer.Fillable must stay empty") } } diff --git a/plugin_test.go b/plugin_test.go index 7d2a50b..4a3b9dd 100644 --- a/plugin_test.go +++ b/plugin_test.go @@ -43,8 +43,8 @@ func TestPluginActivates(t *testing.T) { if len(migrations) == 0 { t.Fatal("no migrations") } - if got := migrations[len(migrations)-1].ID; got != "20261109120000_add_test_mode_to_quizzes" { - t.Errorf("last migration = %s, want 20261109120000_add_test_mode_to_quizzes", got) + if got := migrations[len(migrations)-1].ID; got != "20261109180000_add_open_questions_and_grading" { + t.Errorf("last migration = %s, want 20261109180000_add_open_questions_and_grading", got) } if app.Config.Bool("golem15.quizzes.scoring.multiple_partial") { t.Error("scoring.multiple_partial defaults to true, want false") diff --git a/registry.gen.go b/registry.gen.go index 6117492..6416719 100644 --- a/registry.gen.go +++ b/registry.gen.go @@ -27,6 +27,7 @@ func generatedMigrations() []*gormigrate.Migration { updates.AddCategoryToAttempts(), updates.AddCompletedAttemptsIndex(), updates.AddTestModeToQuizzes(), + updates.AddOpenQuestionsAndGrading(), } } diff --git a/updates/20261109180000_add_open_questions_and_grading.go b/updates/20261109180000_add_open_questions_and_grading.go new file mode 100644 index 0000000..6418b5d --- /dev/null +++ b/updates/20261109180000_add_open_questions_and_grading.go @@ -0,0 +1,108 @@ +// Code generated by summer make. DO NOT EDIT. + +package updates + +import ( + "fmt" + + "github.com/go-gormigrate/gormigrate/v2" + "gorm.io/gorm" +) + +// AddOpenQuestionsAndGrading returns the 20261109180000_add_open_questions_and_grading gormigrate entry. +// It adds the open question type (answered in free text, graded against the +// model answer and an owner-only rubric), the quiz grader and the grading +// and review state of each attempt answer. Every existing question, quiz and +// answer keeps today's behaviour: the grader is manual, no question is open +// and every existing answer row has no grading state. +func AddOpenQuestionsAndGrading() *gormigrate.Migration { + return &gormigrate.Migration{ + ID: "20261109180000_add_open_questions_and_grading", + Migrate: func(tx *gorm.DB) error { + // The original type CHECK is an unnamed inline constraint, so + // it is found by the column it checks rather than by an assumed + // name. Its definition cannot be matched on "IN": Postgres + // stores it as type = ANY (ARRAY[...]). + for _, stmt := range []string{ + `DO $$ +DECLARE c RECORD; +BEGIN + FOR c IN + SELECT con.conname FROM pg_constraint con + JOIN pg_attribute att ON att.attrelid = con.conrelid AND att.attname = 'type' + WHERE con.conrelid = 'golem15_quizzes_questions'::regclass + AND con.contype = 'c' + AND con.conkey = ARRAY[att.attnum] + LOOP + EXECUTE format('ALTER TABLE golem15_quizzes_questions DROP CONSTRAINT %I', c.conname); + END LOOP; +END $$`, + `ALTER TABLE golem15_quizzes_questions + ADD CONSTRAINT golem15_quizzes_questions_type_check CHECK (type IN ('single', 'multiple', 'self_check', 'open')), + ADD COLUMN rubric TEXT NULL, + ADD COLUMN max_length INTEGER NULL CHECK (max_length IS NULL OR max_length BETWEEN 1 AND 20000)`, + `ALTER TABLE golem15_quizzes_quizzes + ADD COLUMN grader VARCHAR(64) NOT NULL DEFAULT 'manual'`, + `ALTER TABLE golem15_quizzes_attempt_answers + ADD COLUMN text_answer TEXT NULL, + ADD COLUMN grading_status VARCHAR(20) NULL CHECK (grading_status IS NULL OR grading_status IN ('pending', 'graded', 'failed', 'reviewed')), + ADD COLUMN grading_seq INTEGER NOT NULL DEFAULT 0, + ADD COLUMN grading_attempts INTEGER NOT NULL DEFAULT 0, + ADD COLUMN grader VARCHAR(64) NULL, + ADD COLUMN grader_points DOUBLE PRECISION NULL, + ADD COLUMN grader_feedback TEXT NULL, + ADD COLUMN grading_error TEXT NULL, + ADD COLUMN graded_at TIMESTAMPTZ NULL, + ADD COLUMN review_comment TEXT NULL, + ADD COLUMN reviewed_at TIMESTAMPTZ NULL, + ADD COLUMN reviewed_by_user_id INTEGER NULL, + ADD COLUMN reviewed_by_admin_id INTEGER NULL`, + `CREATE INDEX golem15_quizzes_attempt_answers_grading_index ON golem15_quizzes_attempt_answers (grading_status) + WHERE grading_status IN ('pending', 'graded', 'failed')`, + } { + if err := tx.Exec(stmt).Error; err != nil { + return err + } + } + return nil + }, + Rollback: func(tx *gorm.DB) error { + var n int64 + if err := tx.Raw(`SELECT COUNT(*) FROM golem15_quizzes_questions WHERE type = 'open'`).Scan(&n).Error; err != nil { + return err + } + if n > 0 { + return fmt.Errorf("quizzes: rollback refused: %d open questions exist; delete them first", n) + } + for _, stmt := range []string{ + `DROP INDEX IF EXISTS golem15_quizzes_attempt_answers_grading_index`, + `ALTER TABLE golem15_quizzes_attempt_answers + DROP COLUMN IF EXISTS reviewed_by_admin_id, + DROP COLUMN IF EXISTS reviewed_by_user_id, + DROP COLUMN IF EXISTS reviewed_at, + DROP COLUMN IF EXISTS review_comment, + DROP COLUMN IF EXISTS graded_at, + DROP COLUMN IF EXISTS grading_error, + DROP COLUMN IF EXISTS grader_feedback, + DROP COLUMN IF EXISTS grader_points, + DROP COLUMN IF EXISTS grader, + DROP COLUMN IF EXISTS grading_attempts, + DROP COLUMN IF EXISTS grading_seq, + DROP COLUMN IF EXISTS grading_status, + DROP COLUMN IF EXISTS text_answer`, + `ALTER TABLE golem15_quizzes_quizzes DROP COLUMN IF EXISTS grader`, + `ALTER TABLE golem15_quizzes_questions + DROP COLUMN IF EXISTS max_length, + DROP COLUMN IF EXISTS rubric, + DROP CONSTRAINT IF EXISTS golem15_quizzes_questions_type_check`, + `ALTER TABLE golem15_quizzes_questions + ADD CONSTRAINT golem15_quizzes_questions_type_check CHECK (type IN ('single', 'multiple', 'self_check'))`, + } { + if err := tx.Exec(stmt).Error; err != nil { + return err + } + } + return nil + }, + } +}