feat: set scores, confirm in bulk and regrade open answers over MCP
- SetScore (0..points in 0.5 steps, comment shown to the student), ConfirmBulk per quiz or attempt and Regrade through the quiz's grader - close() announces an attempt reviewed while it was open - quizzes_review_set_score, quizzes_review_confirm_bulk, quizzes_review_regrade; 409 already_reviewed and grader_manual
This commit is contained in:
@@ -13,9 +13,12 @@ package classes
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"math"
|
||||
"slices"
|
||||
"strconv"
|
||||
"strings"
|
||||
"time"
|
||||
"unicode/utf8"
|
||||
|
||||
"git.golem15.com/golem15/sm-quizzes-plugin/events"
|
||||
"git.golem15.com/golem15/sm-quizzes-plugin/models"
|
||||
@@ -35,9 +38,21 @@ const (
|
||||
// ReviewExcerptRunes is how much of a student's text a queue row shows.
|
||||
const ReviewExcerptRunes = 300
|
||||
|
||||
// ErrNoGraderScore is a confirm of an answer that has no grader score: it
|
||||
// is pending or failed, so the owner sets the score instead.
|
||||
var ErrNoGraderScore = errors.New("quizzes: the answer has no grader score to confirm")
|
||||
// MaxReviewComment is the longest comment to the student, in runes.
|
||||
const MaxReviewComment = 2000
|
||||
|
||||
// Errors of the review actions, answered as 409s with a code.
|
||||
var (
|
||||
// ErrNoGraderScore is a confirm of an answer that has no grader score:
|
||||
// it is pending or failed, so the owner sets the score instead.
|
||||
ErrNoGraderScore = errors.New("quizzes: the answer has no grader score to confirm")
|
||||
// ErrAlreadyReviewed is a regrade of one answer the owner already
|
||||
// reviewed: owner decisions are final (D-12).
|
||||
ErrAlreadyReviewed = errors.New("quizzes: the answer is already reviewed")
|
||||
// ErrGraderManual is a regrade on a quiz whose grader is manual or not
|
||||
// registered by any activated plugin.
|
||||
ErrGraderManual = errors.New("quizzes: the quiz has no grader that can grade again")
|
||||
)
|
||||
|
||||
// ReviewFilter narrows the review queue. Zero values do not filter; Status
|
||||
// is one CheckReviewStatus accepts.
|
||||
@@ -68,6 +83,68 @@ func (r Reviewer) ids() (user, admin *uint) {
|
||||
return user, admin
|
||||
}
|
||||
|
||||
// ReviewScope picks the open answers of a bulk action: one answer, one
|
||||
// attempt or one quiz (by slug). Exactly one is set; see CheckReviewScope.
|
||||
type ReviewScope struct {
|
||||
AnswerID uint
|
||||
AttemptID uint
|
||||
QuizSlug string
|
||||
}
|
||||
|
||||
// CheckReviewScope requires exactly one of the scope's keys: answer_id,
|
||||
// attempt_id or quiz when allowAnswer (regrade), attempt_id or quiz
|
||||
// otherwise (bulk confirm).
|
||||
func CheckReviewScope(scope ReviewScope, allowAnswer bool) []string {
|
||||
n := 0
|
||||
if scope.AttemptID != 0 {
|
||||
n++
|
||||
}
|
||||
if strings.TrimSpace(scope.QuizSlug) != "" {
|
||||
n++
|
||||
}
|
||||
if scope.AnswerID != 0 {
|
||||
if !allowAnswer {
|
||||
return []string{"exactly one of attempt_id and quiz is required"}
|
||||
}
|
||||
n++
|
||||
}
|
||||
if n == 1 {
|
||||
return nil
|
||||
}
|
||||
if allowAnswer {
|
||||
return []string{"exactly one of answer_id, attempt_id and quiz is required"}
|
||||
}
|
||||
return []string{"exactly one of attempt_id and quiz is required"}
|
||||
}
|
||||
|
||||
// CheckReviewPoints reports whether points is a score the owner may give a
|
||||
// question worth maxPoints: finite, 0..maxPoints (a negative maxPoints
|
||||
// counts as 0) and a multiple of 0.5.
|
||||
func CheckReviewPoints(points float64, maxPoints int) bool {
|
||||
if math.IsNaN(points) || math.IsInf(points, 0) {
|
||||
return false
|
||||
}
|
||||
full := float64(max(maxPoints, 0))
|
||||
return points >= 0 && points <= full && points*2 == math.Trunc(points*2)
|
||||
}
|
||||
|
||||
// CheckReviewComment cleans the owner's comment to the student: trimmed,
|
||||
// NUL bytes dropped, and nil when nothing is left. More than
|
||||
// MaxReviewComment runes is a problem.
|
||||
func CheckReviewComment(comment *string) (*string, []string) {
|
||||
if comment == nil {
|
||||
return nil, nil
|
||||
}
|
||||
c := strings.TrimSpace(strings.ReplaceAll(*comment, "\x00", ""))
|
||||
if c == "" {
|
||||
return nil, nil
|
||||
}
|
||||
if utf8.RuneCountInString(c) > MaxReviewComment {
|
||||
return nil, []string{"comment must be at most " + strconv.Itoa(MaxReviewComment) + " characters"}
|
||||
}
|
||||
return &c, nil
|
||||
}
|
||||
|
||||
// unreviewedStatuses are the grading statuses the owner still has to act
|
||||
// on.
|
||||
func unreviewedStatuses() []string {
|
||||
@@ -454,3 +531,305 @@ func (s *Service) fireAttemptReviewedIfDone(ctx context.Context, tx *gorm.DB, at
|
||||
})
|
||||
return nil
|
||||
}
|
||||
|
||||
// SetScore gives one open answer the owner's final score, on any status
|
||||
// (D-12): status reviewed, points as given, is_correct for full marks, and
|
||||
// the comment to the student replacing any earlier one (nil or blank
|
||||
// clears it). grader_points and grader_feedback stay for the record.
|
||||
// Points outside 0..question points or not a multiple of 0.5, and a
|
||||
// comment over MaxReviewComment runes, are a *ValidationError. A
|
||||
// completed attempt is recalculated; AnswerReviewedEvent, and
|
||||
// AttemptReviewedEvent when the attempt is fully reviewed, fire after the
|
||||
// commit.
|
||||
func (s *Service) SetScore(ctx context.Context, answerID uint, points float64, comment *string, by Reviewer) (*ReviewDetail, error) {
|
||||
comment, problems := CheckReviewComment(comment)
|
||||
if len(problems) > 0 {
|
||||
return nil, &ValidationError{Problems: problems}
|
||||
}
|
||||
var detail *ReviewDetail
|
||||
err := lagoon.Transaction(ctx, s.DB, func(ctx context.Context, tx *gorm.DB) error {
|
||||
t, err := lockReviewTarget(ctx, tx, answerID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if !CheckReviewPoints(points, t.Question.Points) {
|
||||
return &ValidationError{Problems: []string{
|
||||
"points must be between 0 and " + strconv.Itoa(max(t.Question.Points, 0)) + " in steps of 0.5",
|
||||
}}
|
||||
}
|
||||
if points == 0 {
|
||||
points = 0 // no negative zero in JSON
|
||||
}
|
||||
if err := s.storeReview(ctx, tx, t, points, comment, true, by); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := s.recalcAttempt(ctx, tx, t.Attempt.ID); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := s.fireAttemptReviewedIfDone(ctx, tx, t.Attempt.ID); err != nil {
|
||||
return err
|
||||
}
|
||||
detail, err = reviewItem(ctx, tx, answerID)
|
||||
return err
|
||||
})
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return detail, nil
|
||||
}
|
||||
|
||||
// ConfirmBulk confirms every graded answer of one quiz (QuizSlug) or one
|
||||
// attempt (AttemptID), exactly as Confirm does, and returns how many it
|
||||
// confirmed. Pending, failed and reviewed answers are left alone. The
|
||||
// affected attempts are locked in id order, recalculated once each, and
|
||||
// every changed answer and every completed attempt that is now fully
|
||||
// reviewed fire their events after the commit. A missing quiz or attempt
|
||||
// is ErrNotFound; a scope that is not exactly one of the two is a
|
||||
// *ValidationError.
|
||||
func (s *Service) ConfirmBulk(ctx context.Context, scope ReviewScope, by Reviewer) (int, error) {
|
||||
if problems := CheckReviewScope(scope, false); len(problems) > 0 {
|
||||
return 0, &ValidationError{Problems: problems}
|
||||
}
|
||||
confirmed := 0
|
||||
err := lagoon.Transaction(ctx, s.DB, func(ctx context.Context, tx *gorm.DB) error {
|
||||
confirmed = 0
|
||||
quiz, err := scopeQuiz(ctx, tx, scope)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
rows, attempts, err := lockScope(ctx, tx, scope, quiz, []string{models.GradingGraded})
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
questions, err := scopeQuestions(ctx, tx, rows)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
touched := map[uint]bool{}
|
||||
for _, row := range rows {
|
||||
t := &reviewTarget{Answer: row, Question: questions[row.QuestionID], Attempt: attempts[row.AttemptID]}
|
||||
done, err := s.confirmTarget(ctx, tx, t, by)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if done {
|
||||
confirmed++
|
||||
touched[row.AttemptID] = true
|
||||
}
|
||||
}
|
||||
for _, id := range sortedIDs(touched) {
|
||||
if err := s.recalcAttempt(ctx, tx, id); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := s.fireAttemptReviewedIfDone(ctx, tx, id); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
return nil
|
||||
})
|
||||
if err != nil {
|
||||
return 0, err
|
||||
}
|
||||
return confirmed, nil
|
||||
}
|
||||
|
||||
// Regrade sends open answers back to the quiz's grader, e.g. after a
|
||||
// rubric change (D-12): every pending, graded or failed answer of one
|
||||
// answer, attempt or quiz goes back to pending with the next grading_seq
|
||||
// (so a job for the old text is ignored), its grade, error and points
|
||||
// cleared, and is enqueued again. Answers of a test attempt that is still
|
||||
// open are not enqueued: closing the attempt enqueues them (UD-2).
|
||||
// Reviewed answers are skipped, as owner decisions are final; a single
|
||||
// reviewed answer is ErrAlreadyReviewed. A quiz whose grader is manual or
|
||||
// not registered is ErrGraderManual. Completed attempts are recalculated.
|
||||
// It returns how many answers were reset.
|
||||
func (s *Service) Regrade(ctx context.Context, scope ReviewScope) (int, error) {
|
||||
if problems := CheckReviewScope(scope, true); len(problems) > 0 {
|
||||
return 0, &ValidationError{Problems: problems}
|
||||
}
|
||||
regraded := 0
|
||||
err := lagoon.Transaction(ctx, s.DB, func(ctx context.Context, tx *gorm.DB) error {
|
||||
regraded = 0
|
||||
tx = tx.WithContext(ctx)
|
||||
quiz, err := scopeQuiz(ctx, tx, scope)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if _, ok := GraderFor(s.Graders, quiz); !ok {
|
||||
return ErrGraderManual
|
||||
}
|
||||
if scope.AnswerID != 0 {
|
||||
if err := refuseReviewed(ctx, tx, scope.AnswerID); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
rows, attempts, err := lockScope(ctx, tx, scope, quiz, unreviewedStatuses())
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if scope.AnswerID != 0 && len(rows) == 0 {
|
||||
// Reviewed between the first read and the lock.
|
||||
return refuseReviewed(ctx, tx, scope.AnswerID)
|
||||
}
|
||||
now := s.now().UTC()
|
||||
seqs := make(map[uint]int, len(rows))
|
||||
for _, row := range rows {
|
||||
seq := row.GradingSeq + 1
|
||||
if err := tx.Model(&models.AttemptAnswer{}).Where("id = ?", row.ID).Updates(map[string]any{
|
||||
"grading_status": models.GradingPending,
|
||||
"grading_seq": seq,
|
||||
"grading_attempts": 0,
|
||||
"grader": nil,
|
||||
"grader_points": nil,
|
||||
"grader_feedback": nil,
|
||||
"grading_error": nil,
|
||||
"graded_at": nil,
|
||||
"points": 0,
|
||||
"is_correct": false,
|
||||
"updated_at": now,
|
||||
}).Error; err != nil {
|
||||
return err
|
||||
}
|
||||
seqs[row.ID] = seq
|
||||
regraded++
|
||||
}
|
||||
touched := map[uint]bool{}
|
||||
for _, row := range rows {
|
||||
touched[row.AttemptID] = true
|
||||
}
|
||||
for _, id := range sortedIDs(touched) {
|
||||
if err := s.recalcAttempt(ctx, tx, id); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
for _, row := range rows {
|
||||
a := attempts[row.AttemptID]
|
||||
if a.CompletedAt == nil && IsTest(quiz) {
|
||||
continue
|
||||
}
|
||||
if err := s.enqueueGrading(ctx, tx, row.ID, seqs[row.ID], a); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
return nil
|
||||
})
|
||||
if err != nil {
|
||||
return 0, err
|
||||
}
|
||||
return regraded, nil
|
||||
}
|
||||
|
||||
// refuseReviewed is ErrAlreadyReviewed for a reviewed answer and
|
||||
// ErrNotFound for one that is missing or not open.
|
||||
func refuseReviewed(ctx context.Context, tx *gorm.DB, answerID uint) error {
|
||||
var row models.AttemptAnswer
|
||||
if err := tx.WithContext(ctx).Where("id = ? AND grading_status IS NOT NULL", answerID).Take(&row).Error; err != nil {
|
||||
return notFound(err)
|
||||
}
|
||||
if *row.GradingStatus == models.GradingReviewed {
|
||||
return ErrAlreadyReviewed
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// scopeQuiz resolves the quiz of a review scope: by slug, from the
|
||||
// attempt, or from the open answer's attempt. Anything missing is
|
||||
// ErrNotFound.
|
||||
func scopeQuiz(ctx context.Context, tx *gorm.DB, scope ReviewScope) (*models.Quiz, error) {
|
||||
tx = tx.WithContext(ctx)
|
||||
var quiz models.Quiz
|
||||
var err error
|
||||
switch {
|
||||
case scope.AnswerID != 0:
|
||||
err = tx.Where(`id = (SELECT a.quiz_id FROM golem15_quizzes_attempt_answers aa
|
||||
JOIN golem15_quizzes_attempts a ON a.id = aa.attempt_id
|
||||
WHERE aa.id = ? AND aa.grading_status IS NOT NULL)`, scope.AnswerID).Take(&quiz).Error
|
||||
case scope.AttemptID != 0:
|
||||
err = tx.Where("id = (SELECT quiz_id FROM golem15_quizzes_attempts WHERE id = ?)", scope.AttemptID).Take(&quiz).Error
|
||||
default:
|
||||
err = tx.Where("slug = ?", strings.TrimSpace(scope.QuizSlug)).Take(&quiz).Error
|
||||
}
|
||||
if err != nil {
|
||||
return nil, notFound(err)
|
||||
}
|
||||
return &quiz, nil
|
||||
}
|
||||
|
||||
// scopeRows narrows a query on the attempt answers to a scope of quiz.
|
||||
func scopeRows(q *gorm.DB, scope ReviewScope, quiz *models.Quiz) *gorm.DB {
|
||||
switch {
|
||||
case scope.AnswerID != 0:
|
||||
return q.Where("id = ?", scope.AnswerID)
|
||||
case scope.AttemptID != 0:
|
||||
return q.Where("attempt_id = ?", scope.AttemptID)
|
||||
}
|
||||
return q.Where("attempt_id IN (SELECT id FROM golem15_quizzes_attempts WHERE quiz_id = ?)", quiz.ID)
|
||||
}
|
||||
|
||||
// lockScope locks, in id order, the attempts that have open answers with
|
||||
// one of statuses in scope, then selects those answers again under the
|
||||
// locks (only of the locked attempts, so nothing changes underneath). It
|
||||
// returns the answers by attempt and id, and the locked attempts by id
|
||||
// with their quiz set.
|
||||
func lockScope(ctx context.Context, tx *gorm.DB, scope ReviewScope, quiz *models.Quiz, statuses []string) ([]models.AttemptAnswer, map[uint]*models.Attempt, error) {
|
||||
tx = tx.WithContext(ctx)
|
||||
var ids []uint
|
||||
if err := scopeRows(tx.Model(&models.AttemptAnswer{}), scope, quiz).
|
||||
Where("grading_status IN ?", statuses).Distinct("attempt_id").Order("attempt_id ASC").
|
||||
Pluck("attempt_id", &ids).Error; err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
attempts := make(map[uint]*models.Attempt, len(ids))
|
||||
locked := make([]uint, 0, len(ids))
|
||||
for _, id := range ids {
|
||||
a, err := lockAttempt(ctx, tx, id)
|
||||
if errors.Is(err, gorm.ErrRecordNotFound) {
|
||||
continue
|
||||
}
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
a.Quiz = quiz
|
||||
attempts[id] = a
|
||||
locked = append(locked, id)
|
||||
}
|
||||
if len(locked) == 0 {
|
||||
return nil, attempts, nil
|
||||
}
|
||||
var rows []models.AttemptAnswer
|
||||
if err := scopeRows(tx, scope, quiz).Where("grading_status IN ? AND attempt_id IN ?", statuses, locked).
|
||||
Order("attempt_id ASC, id ASC").Find(&rows).Error; err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
return rows, attempts, nil
|
||||
}
|
||||
|
||||
// scopeQuestions loads the questions of the given answers by id.
|
||||
func scopeQuestions(ctx context.Context, tx *gorm.DB, rows []models.AttemptAnswer) (map[uint]models.Question, error) {
|
||||
out := map[uint]models.Question{}
|
||||
if len(rows) == 0 {
|
||||
return out, nil
|
||||
}
|
||||
ids := make([]uint, 0, len(rows))
|
||||
for _, r := range rows {
|
||||
ids = append(ids, r.QuestionID)
|
||||
}
|
||||
var questions []models.Question
|
||||
if err := tx.WithContext(ctx).Where("id IN ?", ids).Find(&questions).Error; err != nil {
|
||||
return nil, err
|
||||
}
|
||||
for _, q := range questions {
|
||||
out[q.ID] = q
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// sortedIDs returns the keys of set in ascending order.
|
||||
func sortedIDs(set map[uint]bool) []uint {
|
||||
out := make([]uint, 0, len(set))
|
||||
for id := range set {
|
||||
out = append(out, id)
|
||||
}
|
||||
slices.Sort(out)
|
||||
return out
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package classes_test
|
||||
|
||||
import (
|
||||
"math"
|
||||
"slices"
|
||||
"strings"
|
||||
"testing"
|
||||
@@ -38,3 +39,77 @@ func TestCheckReviewStatus(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestCheckReviewPoints: 0..points in steps of 0.5, finite.
|
||||
func TestCheckReviewPoints(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
p float64
|
||||
max int
|
||||
want bool
|
||||
}{
|
||||
{0, 2, true}, {1.5, 2, true}, {2, 2, true}, {0, 0, true}, {0.5, 1, true},
|
||||
{-0.5, 2, false}, {2.5, 2, false}, {1.25, 2, false}, {0.1, 2, false},
|
||||
{math.NaN(), 2, false}, {math.Inf(1), 2, false}, {math.Inf(-1), 2, false},
|
||||
{0.5, 0, false}, {0, -1, true}, {1, -1, false},
|
||||
} {
|
||||
if got := classes.CheckReviewPoints(tc.p, tc.max); got != tc.want {
|
||||
t.Errorf("CheckReviewPoints(%v, %d) = %v, want %v", tc.p, tc.max, got, tc.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestCheckReviewComment: trimmed, blank is no comment, NUL dropped,
|
||||
// at most 2000 runes.
|
||||
func TestCheckReviewComment(t *testing.T) {
|
||||
str := func(s string) *string { return &s }
|
||||
if got, p := classes.CheckReviewComment(nil); got != nil || p != nil {
|
||||
t.Errorf("nil = %v %v", got, p)
|
||||
}
|
||||
if got, p := classes.CheckReviewComment(str(" \n ")); got != nil || p != nil {
|
||||
t.Errorf("spaces = %v %v, want no comment", got, p)
|
||||
}
|
||||
if got, p := classes.CheckReviewComment(str(" Dobrze, ale\x00 brak przykładu. ")); got == nil || *got != "Dobrze, ale brak przykładu." || p != nil {
|
||||
t.Errorf("trimmed = %v %v", got, p)
|
||||
}
|
||||
max := strings.Repeat("ł", classes.MaxReviewComment)
|
||||
if got, p := classes.CheckReviewComment(&max); got == nil || *got != max || p != nil {
|
||||
t.Errorf("2000 runes refused: %v", p)
|
||||
}
|
||||
over := max + "x"
|
||||
if got, p := classes.CheckReviewComment(&over); got != nil || len(p) != 1 || p[0] != "comment must be at most 2000 characters" {
|
||||
t.Errorf("2001 runes = %v %v, want one problem", got, p)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCheckReviewScope: exactly one key; answer_id only for regrade.
|
||||
func TestCheckReviewScope(t *testing.T) {
|
||||
regrade := "exactly one of answer_id, attempt_id and quiz is required"
|
||||
bulk := "exactly one of attempt_id and quiz is required"
|
||||
for name, tc := range map[string]struct {
|
||||
scope classes.ReviewScope
|
||||
allow bool
|
||||
want string
|
||||
}{
|
||||
"regrade answer": {classes.ReviewScope{AnswerID: 1}, true, ""},
|
||||
"regrade attempt": {classes.ReviewScope{AttemptID: 1}, true, ""},
|
||||
"regrade quiz": {classes.ReviewScope{QuizSlug: "q"}, true, ""},
|
||||
"regrade none": {classes.ReviewScope{}, true, regrade},
|
||||
"regrade blank quiz": {classes.ReviewScope{QuizSlug: " "}, true, regrade},
|
||||
"regrade two": {classes.ReviewScope{AnswerID: 1, QuizSlug: "q"}, true, regrade},
|
||||
"regrade three": {classes.ReviewScope{AnswerID: 1, AttemptID: 2, QuizSlug: "q"}, true, regrade},
|
||||
"bulk attempt": {classes.ReviewScope{AttemptID: 1}, false, ""},
|
||||
"bulk quiz": {classes.ReviewScope{QuizSlug: "q"}, false, ""},
|
||||
"bulk none": {classes.ReviewScope{}, false, bulk},
|
||||
"bulk two": {classes.ReviewScope{AttemptID: 1, QuizSlug: "q"}, false, bulk},
|
||||
"bulk answer": {classes.ReviewScope{AnswerID: 1}, false, bulk},
|
||||
"bulk answer and quiz": {classes.ReviewScope{AnswerID: 1, QuizSlug: "q"}, false, bulk},
|
||||
} {
|
||||
got := classes.CheckReviewScope(tc.scope, tc.allow)
|
||||
switch {
|
||||
case tc.want == "" && got != nil:
|
||||
t.Errorf("%s = %v, want no problem", name, got)
|
||||
case tc.want != "" && (len(got) != 1 || got[0] != tc.want):
|
||||
t.Errorf("%s = %v, want %q", name, got, tc.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -662,7 +662,8 @@ func (s *Service) Finish(ctx context.Context, userID, attemptID uint) (*AttemptR
|
||||
// answers, floors the total at 0 and stores the score. Finish closes at the
|
||||
// current time, an expired test closes at its deadline. A test then
|
||||
// enqueues one grading job per pending open answer (UD-2); every caller
|
||||
// sets attempt.Quiz.
|
||||
// sets attempt.Quiz. An attempt whose open answers were all reviewed
|
||||
// while it was open is announced as reviewed once it closes.
|
||||
func (s *Service) close(ctx context.Context, tx *gorm.DB, attempt *models.Attempt, at time.Time) error {
|
||||
tx = tx.WithContext(ctx)
|
||||
var points []float64
|
||||
@@ -685,7 +686,10 @@ func (s *Service) close(ctx context.Context, tx *gorm.DB, attempt *models.Attemp
|
||||
}).Error; err != nil {
|
||||
return err
|
||||
}
|
||||
return s.enqueuePending(ctx, tx, attempt)
|
||||
if err := s.enqueuePending(ctx, tx, attempt); err != nil {
|
||||
return err
|
||||
}
|
||||
return s.fireAttemptReviewedIfDone(ctx, tx, attempt.ID)
|
||||
}
|
||||
|
||||
// AttemptSummary is an attempt with its quiz and answered question count.
|
||||
|
||||
@@ -29,6 +29,10 @@ func writeReview(w http.ResponseWriter, status int, data any, err error) {
|
||||
switch {
|
||||
case errors.Is(err, classes.ErrNoGraderScore):
|
||||
reviewConflict(w, "The answer has no grader score to confirm; set the score instead.", "no_grader_score")
|
||||
case errors.Is(err, classes.ErrAlreadyReviewed):
|
||||
reviewConflict(w, "The answer is already reviewed; set the score instead.", "already_reviewed")
|
||||
case errors.Is(err, classes.ErrGraderManual):
|
||||
reviewConflict(w, "The quiz's grader is manual or not available, so nothing can be graded again.", "grader_manual")
|
||||
case errors.Is(err, classes.ErrNotFound):
|
||||
wire.WriteJSON(w, http.StatusNotFound, map[string]any{"error": true, "message": "Open answer, attempt or quiz not found."})
|
||||
default:
|
||||
@@ -143,3 +147,100 @@ func answerReview(w http.ResponseWriter, d *classes.ReviewDetail, err error) {
|
||||
}
|
||||
writeReview(w, http.StatusOK, reviewDetailJSON(*d), nil)
|
||||
}
|
||||
|
||||
// setScoreBody is the POST /{id}/score body.
|
||||
type setScoreBody struct {
|
||||
Points *float64 `json:"points"`
|
||||
Comment *string `json:"comment"`
|
||||
}
|
||||
|
||||
// ReviewSetScore answers POST /api/v1/quiz-reviews/{id}/score with
|
||||
// {points, comment}: the owner's final score on any status, with an
|
||||
// optional comment the student sees. Points outside 0..question points or
|
||||
// off the 0.5 steps, and a comment over 2000 characters, are a 422.
|
||||
func (h *Handlers) ReviewSetScore(w http.ResponseWriter, r *http.Request) {
|
||||
id, ok := reviewID(w, r)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
var body setScoreBody
|
||||
if !readManaged(w, r, &body) {
|
||||
return
|
||||
}
|
||||
comment, problems := classes.CheckReviewComment(body.Comment)
|
||||
if body.Points == nil {
|
||||
problems = append([]string{"points is required"}, problems...)
|
||||
}
|
||||
if refuse(w, problems) {
|
||||
return
|
||||
}
|
||||
by, ok := h.reviewer(w, r)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
svc, ok := h.service(w)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
d, err := svc.SetScore(r.Context(), id, *body.Points, comment, by)
|
||||
answerReview(w, d, err)
|
||||
}
|
||||
|
||||
// confirmBulkBody is the POST /confirm body.
|
||||
type confirmBulkBody struct {
|
||||
Quiz string `json:"quiz"`
|
||||
AttemptID uint `json:"attempt_id"`
|
||||
}
|
||||
|
||||
// ReviewConfirmBulk answers POST /api/v1/quiz-reviews/confirm with
|
||||
// exactly one of {quiz} and {attempt_id}: every graded answer in scope is
|
||||
// confirmed; it answers {confirmed: n}.
|
||||
func (h *Handlers) ReviewConfirmBulk(w http.ResponseWriter, r *http.Request) {
|
||||
var body confirmBulkBody
|
||||
if !readManaged(w, r, &body) {
|
||||
return
|
||||
}
|
||||
scope := classes.ReviewScope{AttemptID: body.AttemptID, QuizSlug: strings.TrimSpace(body.Quiz)}
|
||||
if refuse(w, classes.CheckReviewScope(scope, false)) {
|
||||
return
|
||||
}
|
||||
by, ok := h.reviewer(w, r)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
svc, ok := h.service(w)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
n, err := svc.ConfirmBulk(r.Context(), scope, by)
|
||||
writeReview(w, http.StatusOK, map[string]any{"confirmed": n}, err)
|
||||
}
|
||||
|
||||
// regradeBody is the POST /regrade body.
|
||||
type regradeBody struct {
|
||||
AnswerID uint `json:"answer_id"`
|
||||
AttemptID uint `json:"attempt_id"`
|
||||
Quiz string `json:"quiz"`
|
||||
}
|
||||
|
||||
// ReviewRegrade answers POST /api/v1/quiz-reviews/regrade with exactly one
|
||||
// of {answer_id}, {attempt_id} and {quiz}: the unreviewed open answers in
|
||||
// scope go back to the quiz's grader; it answers {regraded: n}. 409
|
||||
// grader_manual when the quiz has no grader, already_reviewed for a single
|
||||
// reviewed answer.
|
||||
func (h *Handlers) ReviewRegrade(w http.ResponseWriter, r *http.Request) {
|
||||
var body regradeBody
|
||||
if !readManaged(w, r, &body) {
|
||||
return
|
||||
}
|
||||
scope := classes.ReviewScope{AnswerID: body.AnswerID, AttemptID: body.AttemptID, QuizSlug: strings.TrimSpace(body.Quiz)}
|
||||
if refuse(w, classes.CheckReviewScope(scope, true)) {
|
||||
return
|
||||
}
|
||||
svc, ok := h.service(w)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
n, err := svc.Regrade(r.Context(), scope)
|
||||
writeReview(w, http.StatusOK, map[string]any{"regraded": n}, err)
|
||||
}
|
||||
|
||||
@@ -107,10 +107,12 @@ func TestReviewErrorMapping(t *testing.T) {
|
||||
code int
|
||||
key string
|
||||
}{
|
||||
"no grader score": {classes.ErrNoGraderScore, http.StatusConflict, "no_grader_score"},
|
||||
"not found": {classes.ErrNotFound, http.StatusNotFound, ""},
|
||||
"validation": {&classes.ValidationError{Problems: []string{"status must be one of"}}, http.StatusUnprocessableEntity, ""},
|
||||
"other": {errors.New("boom"), http.StatusInternalServerError, ""},
|
||||
"no grader score": {classes.ErrNoGraderScore, http.StatusConflict, "no_grader_score"},
|
||||
"already reviewed": {classes.ErrAlreadyReviewed, http.StatusConflict, "already_reviewed"},
|
||||
"grader manual": {classes.ErrGraderManual, http.StatusConflict, "grader_manual"},
|
||||
"not found": {classes.ErrNotFound, http.StatusNotFound, ""},
|
||||
"validation": {&classes.ValidationError{Problems: []string{"status must be one of"}}, http.StatusUnprocessableEntity, ""},
|
||||
"other": {errors.New("boom"), http.StatusInternalServerError, ""},
|
||||
} {
|
||||
rec := httptest.NewRecorder()
|
||||
writeReview(rec, 0, nil, tc.err)
|
||||
@@ -167,3 +169,47 @@ func TestReviewHandlersBeforeDatabase(t *testing.T) {
|
||||
t.Errorf("confirm without a user = %d, want 401", code)
|
||||
}
|
||||
}
|
||||
|
||||
// TestReviewWritesBeforeDatabase: the bodies of set_score, confirm_bulk
|
||||
// and regrade are checked before the database: unknown keys, a missing
|
||||
// points, a long comment and a scope that is not exactly one key are 422s.
|
||||
func TestReviewWritesBeforeDatabase(t *testing.T) {
|
||||
h := New(backpack.New(nil))
|
||||
long := strings.Repeat("x", 2001)
|
||||
id := map[string]string{"id": "3"}
|
||||
for name, tc := range map[string]struct {
|
||||
fn http.HandlerFunc
|
||||
body string
|
||||
want string
|
||||
}{
|
||||
"score empty body": {h.ReviewSetScore, "", "invalid JSON"},
|
||||
"score unknown key": {h.ReviewSetScore, `{"points":1,"status":"reviewed"}`, "invalid JSON"},
|
||||
"score without points": {h.ReviewSetScore, `{"comment":"ok"}`, "points is required"},
|
||||
"score long comment": {h.ReviewSetScore, `{"points":1,"comment":"` + long + `"}`, "comment must be at most 2000 characters"},
|
||||
"bulk none": {h.ReviewConfirmBulk, `{}`, "exactly one of attempt_id and quiz is required"},
|
||||
"bulk two": {h.ReviewConfirmBulk, `{"quiz":"q","attempt_id":4}`, "exactly one of attempt_id and quiz is required"},
|
||||
"bulk answer_id": {h.ReviewConfirmBulk, `{"answer_id":4}`, "invalid JSON"},
|
||||
"regrade none": {h.ReviewRegrade, `{}`, "exactly one of answer_id, attempt_id and quiz is required"},
|
||||
"regrade two": {h.ReviewRegrade, `{"answer_id":3,"quiz":"q"}`, "exactly one of answer_id, attempt_id and quiz is required"},
|
||||
} {
|
||||
code, out := call(t, tc.fn, "POST", "/", tc.body, 7, id)
|
||||
if msg, _ := out["message"].(string); code != http.StatusUnprocessableEntity || !strings.Contains(msg, tc.want) {
|
||||
t.Errorf("%s = %d %v, want 422 with %q", name, code, out, tc.want)
|
||||
}
|
||||
}
|
||||
for name, tc := range map[string]struct {
|
||||
fn http.HandlerFunc
|
||||
body string
|
||||
}{
|
||||
"score": {h.ReviewSetScore, `{"points":1.5,"comment":null}`},
|
||||
"bulk": {h.ReviewConfirmBulk, `{"quiz":"q"}`},
|
||||
"regrade": {h.ReviewRegrade, `{"attempt_id":4}`},
|
||||
} {
|
||||
if code, _ := call(t, tc.fn, "POST", "/", tc.body, 7, id); code != http.StatusServiceUnavailable {
|
||||
t.Errorf("%s without a database = %d, want 503", name, code)
|
||||
}
|
||||
}
|
||||
if code, _ := call(t, h.ReviewSetScore, "POST", "/", `{"points":1}`, 7, map[string]string{"id": "x"}); code != http.StatusNotFound {
|
||||
t.Errorf("score with a bad id = %d, want 404", code)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -21,6 +21,26 @@ type reviewAnswerInput struct {
|
||||
ID uint `json:"id" path:"id" jsonschema:"the answer_id, as listed by quizzes_review_list"`
|
||||
}
|
||||
|
||||
// reviewSetScoreInput is the input of quizzes_review_set_score.
|
||||
type reviewSetScoreInput struct {
|
||||
ID uint `json:"id" path:"id" jsonschema:"the answer_id, as listed by quizzes_review_list"`
|
||||
Points float64 `json:"points" jsonschema:"0 up to the question's points, in steps of 0.5"`
|
||||
Comment *string `json:"comment,omitempty" jsonschema:"optional comment the student sees, at most 2000 characters; leaving it out clears an earlier comment"`
|
||||
}
|
||||
|
||||
// reviewConfirmBulkInput is the input of quizzes_review_confirm_bulk.
|
||||
type reviewConfirmBulkInput struct {
|
||||
Quiz string `json:"quiz,omitempty" jsonschema:"confirm the graded answers of this quiz (its slug); give either quiz or attempt_id"`
|
||||
AttemptID uint `json:"attempt_id,omitempty" jsonschema:"confirm the graded answers of this attempt; give either quiz or attempt_id"`
|
||||
}
|
||||
|
||||
// reviewRegradeInput is the input of quizzes_review_regrade.
|
||||
type reviewRegradeInput struct {
|
||||
AnswerID uint `json:"answer_id,omitempty" jsonschema:"grade this one answer again; give exactly one of answer_id, attempt_id and quiz"`
|
||||
AttemptID uint `json:"attempt_id,omitempty" jsonschema:"grade the unreviewed answers of this attempt again"`
|
||||
Quiz string `json:"quiz,omitempty" jsonschema:"grade the unreviewed answers of this quiz (its slug) again"`
|
||||
}
|
||||
|
||||
// reviewTools are the owner's tools over the review queue of open answers
|
||||
// (D-11). They bind the routes under ReviewPrefix, which check the same
|
||||
// permission as the quiz tools (D-13).
|
||||
@@ -54,5 +74,35 @@ func reviewTools() []pact.MCPTool {
|
||||
Permissions: quizPermissions,
|
||||
Idempotent: &yes,
|
||||
},
|
||||
{
|
||||
Name: "review_set_score",
|
||||
Title: "Set the score of an open answer",
|
||||
Description: "Sets your own final score on one open answer, whatever its status (pending, graded, failed or reviewed): points from 0 up to the question's points in steps of 0.5 (anything else is refused with 422), and an optional comment of at most 2000 characters that the student sees with the answer; leaving the comment out clears an earlier one. status becomes reviewed and the score is final over any later grading; the grader's score and feedback stay for the record, and a finished attempt's score is recalculated. Returns the answer as quizzes_review_show does." + needsWrite,
|
||||
Method: http.MethodPost,
|
||||
Path: ReviewPrefix + "/{id}/score",
|
||||
Input: reviewSetScoreInput{},
|
||||
Permissions: quizPermissions,
|
||||
Idempotent: &yes,
|
||||
},
|
||||
{
|
||||
Name: "review_confirm_bulk",
|
||||
Title: "Confirm grader scores in bulk",
|
||||
Description: "Confirms the grader's score of every graded answer of one quiz (quiz, its slug) or of one attempt (attempt_id); give exactly one of the two. Each becomes reviewed with points = grader_points, as quizzes_review_confirm does; pending, failed and reviewed answers are left alone, so check failed ones with quizzes_review_list first. Finished attempts are recalculated. Returns {confirmed: the number of answers confirmed}; a second call confirms nothing new." + needsWrite,
|
||||
Method: http.MethodPost,
|
||||
Path: ReviewPrefix + "/confirm",
|
||||
Input: reviewConfirmBulkInput{},
|
||||
Permissions: quizPermissions,
|
||||
Idempotent: &yes,
|
||||
},
|
||||
{
|
||||
Name: "review_regrade",
|
||||
Title: "Grade open answers again",
|
||||
Description: "Re-runs the quiz's grader on one answer (answer_id), one attempt (attempt_id) or a whole quiz (quiz, its slug); give exactly one. Use it after changing a rubric or when the grader failed. Every pending, graded or failed answer in scope goes back to pending: the current grader scores, feedback and errors are discarded and the points drop to 0 until the grader scores it again (a finished attempt's score is recalculated). Answers of a test attempt that is still open are graded when it ends. Reviewed answers are skipped, and a single reviewed answer is refused with 409 code already_reviewed. Refused with 409 code grader_manual when the quiz's grader is manual or not available. Returns {regraded: the number of answers sent back}." + needsWrite,
|
||||
Method: http.MethodPost,
|
||||
Path: ReviewPrefix + "/regrade",
|
||||
Input: reviewRegradeInput{},
|
||||
Permissions: quizPermissions,
|
||||
Destructive: &yes,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
46
mcp_test.go
46
mcp_test.go
@@ -85,7 +85,10 @@ var pinnedTools = []string{
|
||||
"quizzes_quiz_stats",
|
||||
"quizzes_reorder_questions",
|
||||
"quizzes_review_confirm",
|
||||
"quizzes_review_confirm_bulk",
|
||||
"quizzes_review_list",
|
||||
"quizzes_review_regrade",
|
||||
"quizzes_review_set_score",
|
||||
"quizzes_review_show",
|
||||
"quizzes_show_quiz",
|
||||
"quizzes_unpublish",
|
||||
@@ -160,6 +163,49 @@ func TestQuizzesMCP(t *testing.T) {
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("review annotations", func(t *testing.T) {
|
||||
destructive := map[string]bool{"review_regrade": true}
|
||||
idempotent := map[string]bool{"review_confirm": true, "review_set_score": true, "review_confirm_bulk": true}
|
||||
n := 0
|
||||
for _, tool := range tools {
|
||||
if !strings.HasPrefix(tool.Name, "review_") {
|
||||
continue
|
||||
}
|
||||
n++
|
||||
if !strings.HasPrefix(tool.Path, ReviewPrefix) {
|
||||
t.Errorf("%s: path %s is not under %s", tool.Name, tool.Path, ReviewPrefix)
|
||||
}
|
||||
if got := tool.Destructive != nil && *tool.Destructive; got != destructive[tool.Name] {
|
||||
t.Errorf("%s: Destructive = %v, want %v", tool.Name, got, destructive[tool.Name])
|
||||
}
|
||||
if got := tool.Idempotent != nil && *tool.Idempotent; got != idempotent[tool.Name] {
|
||||
t.Errorf("%s: Idempotent = %v, want %v", tool.Name, got, idempotent[tool.Name])
|
||||
}
|
||||
suffix := needsWrite
|
||||
if tool.Method == http.MethodGet {
|
||||
suffix = needsRead
|
||||
}
|
||||
if !strings.HasSuffix(tool.Description, suffix) {
|
||||
t.Errorf("%s: description does not end with %q", tool.Name, suffix)
|
||||
}
|
||||
}
|
||||
if n != 6 {
|
||||
t.Errorf("%d review tools, want 6", n)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("no vendor names", func(t *testing.T) {
|
||||
for _, tool := range tools {
|
||||
// The golem15 namespace of the permission is not the bridge.
|
||||
text := strings.ReplaceAll(strings.ToLower(tool.Name+" "+tool.Title+" "+tool.Description+" "+tool.Path), "golem15.", "")
|
||||
for _, word := range []string{" ai ", "ai.", "openai", "anthropic", "claude", "gpt", "gemini", "llm", "language model", "bridge", "golem"} {
|
||||
if strings.Contains(" "+text+" ", word) {
|
||||
t.Errorf("%s mentions %q", tool.Name, word)
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("descriptions", func(t *testing.T) {
|
||||
for _, tool := range tools {
|
||||
if n := utf8.RuneCountInString(tool.Description); n > pier.MaxDescriptionRunes {
|
||||
|
||||
@@ -84,6 +84,9 @@ func TestRoutesRegistered(t *testing.T) {
|
||||
"GET /api/v1/quiz-reviews",
|
||||
"GET /api/v1/quiz-reviews/{id}",
|
||||
"POST /api/v1/quiz-reviews/{id}/confirm",
|
||||
"POST /api/v1/quiz-reviews/{id}/score",
|
||||
"POST /api/v1/quiz-reviews/confirm",
|
||||
"POST /api/v1/quiz-reviews/regrade",
|
||||
}
|
||||
var got []string
|
||||
for _, r := range router.Routes() {
|
||||
|
||||
@@ -90,6 +90,10 @@ func (p *Plugin) Routes(r pact.Router) error {
|
||||
g.Where("id", "[0-9]+")
|
||||
g.Post("/{id}/confirm", h.ReviewConfirm, "user.scope:write", perm)
|
||||
g.Where("id", "[0-9]+")
|
||||
g.Post("/{id}/score", h.ReviewSetScore, "user.scope:write", perm)
|
||||
g.Where("id", "[0-9]+")
|
||||
g.Post("/confirm", h.ReviewConfirmBulk, "user.scope:write", perm)
|
||||
g.Post("/regrade", h.ReviewRegrade, "user.scope:write", perm)
|
||||
})
|
||||
return nil
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user