feat: list, show and confirm open answers over MCP
- classes/review.go: review queue, detail and Confirm, shared by MCP and the admin screen; AnswerReviewed and AttemptReviewed after commit - /api/v1/quiz-reviews routes behind the quizzes permission - quizzes_review_list, quizzes_review_show, quizzes_review_confirm
This commit is contained in:
Jakub Zych
456
classes/review.go
Normal file
456
classes/review.go
Normal file
@@ -0,0 +1,456 @@
|
||||
package classes
|
||||
|
||||
// The owner's review queue of open answers: one service shared by the MCP
|
||||
// review routes (/api/v1/quiz-reviews) and the admin "Do oceny" screen.
|
||||
// Reviewers hold the golem15.quizzes.access_quizzes permission (D-13); the
|
||||
// routes and the admin controller check it, this file trusts its caller.
|
||||
//
|
||||
// Every write runs in lagoon.Transaction on s.DB, so a Service whose DB is
|
||||
// an outer transaction (an admin save) nests as a savepoint. A write locks
|
||||
// the answer's attempt first, as grading does, recalculates a completed
|
||||
// attempt and fires its events only after the commit (D-14).
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"slices"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"git.golem15.com/golem15/sm-quizzes-plugin/events"
|
||||
"git.golem15.com/golem15/sm-quizzes-plugin/models"
|
||||
"git.golem15.com/golem15/summercms/modules/lagoon"
|
||||
"gorm.io/gorm"
|
||||
)
|
||||
|
||||
// Review status filters besides the grading statuses themselves.
|
||||
const (
|
||||
// ReviewStatusOpen is every answer the owner has not reviewed yet:
|
||||
// pending, graded and failed. It is the default filter.
|
||||
ReviewStatusOpen = "open"
|
||||
// ReviewStatusAll lists every open answer, reviewed ones included.
|
||||
ReviewStatusAll = "all"
|
||||
)
|
||||
|
||||
// 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")
|
||||
|
||||
// ReviewFilter narrows the review queue. Zero values do not filter; Status
|
||||
// is one CheckReviewStatus accepts.
|
||||
type ReviewFilter struct {
|
||||
QuizSlug string
|
||||
Status string
|
||||
UserID uint
|
||||
AttemptID uint
|
||||
}
|
||||
|
||||
// Reviewer is who reviews: a frontend user over MCP (UserID, the API
|
||||
// token's user) or a backend administrator (AdminID). A zero id is stored
|
||||
// as NULL (Pitfall 7: the two live in different tables).
|
||||
type Reviewer struct {
|
||||
UserID uint
|
||||
AdminID uint
|
||||
}
|
||||
|
||||
func (r Reviewer) ids() (user, admin *uint) {
|
||||
if r.UserID != 0 {
|
||||
id := r.UserID
|
||||
user = &id
|
||||
}
|
||||
if r.AdminID != 0 {
|
||||
id := r.AdminID
|
||||
admin = &id
|
||||
}
|
||||
return user, admin
|
||||
}
|
||||
|
||||
// unreviewedStatuses are the grading statuses the owner still has to act
|
||||
// on.
|
||||
func unreviewedStatuses() []string {
|
||||
return []string{models.GradingPending, models.GradingGraded, models.GradingFailed}
|
||||
}
|
||||
|
||||
// CheckReviewStatus maps a status filter to the grading statuses to list:
|
||||
// "" and open give pending, graded and failed, a grading status gives
|
||||
// itself and all gives nil (no filter). Anything else is a problem naming
|
||||
// the valid values.
|
||||
func CheckReviewStatus(status string) ([]string, []string) {
|
||||
status = strings.TrimSpace(status)
|
||||
switch status {
|
||||
case "", ReviewStatusOpen:
|
||||
return unreviewedStatuses(), nil
|
||||
case ReviewStatusAll:
|
||||
return nil, nil
|
||||
}
|
||||
if slices.Contains(models.GradingStatuses, status) {
|
||||
return []string{status}, nil
|
||||
}
|
||||
return nil, []string{"status must be one of open, pending, graded, failed, reviewed, all"}
|
||||
}
|
||||
|
||||
// ReviewRow is one open answer of the review queue. In a queue row Text is
|
||||
// cut to ReviewExcerptRunes+1 runes, enough to tell that it was cut; the
|
||||
// detail carries the whole text.
|
||||
type ReviewRow struct {
|
||||
AnswerID uint
|
||||
Status string
|
||||
QuizID uint
|
||||
QuizSlug string
|
||||
QuizTitle string
|
||||
QuizMode string
|
||||
QuestionID uint
|
||||
QuestionText string
|
||||
QuestionPoints int
|
||||
AttemptID uint
|
||||
AttemptCompletedAt *time.Time
|
||||
UserID uint
|
||||
Email string
|
||||
Name *string
|
||||
Surname *string
|
||||
Text string
|
||||
GraderPoints *float64
|
||||
Points float64
|
||||
GradedAt *time.Time
|
||||
ReviewedAt *time.Time
|
||||
UpdatedAt time.Time
|
||||
}
|
||||
|
||||
// ReviewDetail is one open answer with everything the owner reviews it
|
||||
// against: the model answer and the rubric (owner-only), the grader's
|
||||
// result or error and the review so far. MaxLength is the question's
|
||||
// effective limit.
|
||||
type ReviewDetail struct {
|
||||
ReviewRow
|
||||
AnswerText *string
|
||||
Rubric *string
|
||||
MaxLength int
|
||||
Grader *string
|
||||
GraderFeedback *string
|
||||
GradingError *string
|
||||
ReviewComment *string
|
||||
GradingAttempts int
|
||||
IsCorrect bool
|
||||
ReviewedByUserID *uint
|
||||
ReviewedByAdminID *uint
|
||||
}
|
||||
|
||||
// reviewFromSQL joins an open answer to its question, attempt, quiz and
|
||||
// student. Only open answers (a grading status) are in the queue.
|
||||
const reviewFromSQL = `
|
||||
FROM golem15_quizzes_attempt_answers aa
|
||||
JOIN golem15_quizzes_questions q ON q.id = aa.question_id
|
||||
JOIN golem15_quizzes_attempts a ON a.id = aa.attempt_id
|
||||
JOIN golem15_quizzes_quizzes qz ON qz.id = a.quiz_id
|
||||
JOIN users u ON u.id = a.user_id
|
||||
WHERE aa.grading_status IS NOT NULL`
|
||||
|
||||
// reviewRowColumnsSQL are the columns of a ReviewRow but the text.
|
||||
const reviewRowColumnsSQL = `aa.id AS answer_id, aa.grading_status AS status,
|
||||
qz.id AS quiz_id, qz.slug AS quiz_slug, qz.title AS quiz_title, qz.mode AS quiz_mode,
|
||||
q.id AS question_id, q.text AS question_text, q.points AS question_points,
|
||||
a.id AS attempt_id, a.completed_at AS attempt_completed_at,
|
||||
a.user_id AS user_id, COALESCE(u.email, '') AS email, u.name AS name, u.surname AS surname,
|
||||
aa.grader_points AS grader_points, aa.points AS points,
|
||||
aa.graded_at AS graded_at, aa.reviewed_at AS reviewed_at, aa.updated_at AS updated_at`
|
||||
|
||||
// reviewQueueSQL selects the queue rows; the filter predicates, the order
|
||||
// and the page are appended as bound parameters.
|
||||
const reviewQueueSQL = `SELECT ` + reviewRowColumnsSQL + `,
|
||||
LEFT(COALESCE(aa.text_answer, ''), 301) AS text` + reviewFromSQL
|
||||
|
||||
// reviewDetailSQL selects one open answer by id with the detail columns.
|
||||
const reviewDetailSQL = `SELECT ` + reviewRowColumnsSQL + `,
|
||||
COALESCE(aa.text_answer, '') AS text,
|
||||
q.answer_text AS answer_text, q.rubric AS rubric, q.max_length AS max_length,
|
||||
aa.grader AS grader, aa.grader_feedback AS grader_feedback, aa.grading_error AS grading_error,
|
||||
aa.review_comment AS review_comment, aa.grading_attempts AS grading_attempts,
|
||||
aa.is_correct AS is_correct,
|
||||
aa.reviewed_by_user_id AS reviewed_by_user_id, aa.reviewed_by_admin_id AS reviewed_by_admin_id` +
|
||||
reviewFromSQL + ` AND aa.id = ?`
|
||||
|
||||
// reviewScan is what the review queries scan into.
|
||||
type reviewScan struct {
|
||||
ReviewRow
|
||||
AnswerText *string
|
||||
Rubric *string
|
||||
MaxLength *int
|
||||
Grader *string
|
||||
GraderFeedback *string
|
||||
GradingError *string
|
||||
ReviewComment *string
|
||||
GradingAttempts int
|
||||
IsCorrect bool
|
||||
ReviewedByUserID *uint
|
||||
ReviewedByAdminID *uint
|
||||
}
|
||||
|
||||
func (r reviewScan) detail() *ReviewDetail {
|
||||
maxLength := models.DefaultMaxLength
|
||||
if r.MaxLength != nil {
|
||||
maxLength = *r.MaxLength
|
||||
}
|
||||
return &ReviewDetail{
|
||||
ReviewRow: r.ReviewRow,
|
||||
AnswerText: r.AnswerText,
|
||||
Rubric: r.Rubric,
|
||||
MaxLength: maxLength,
|
||||
Grader: r.Grader,
|
||||
GraderFeedback: r.GraderFeedback,
|
||||
GradingError: r.GradingError,
|
||||
ReviewComment: r.ReviewComment,
|
||||
GradingAttempts: r.GradingAttempts,
|
||||
IsCorrect: r.IsCorrect,
|
||||
ReviewedByUserID: r.ReviewedByUserID,
|
||||
ReviewedByAdminID: r.ReviewedByAdminID,
|
||||
}
|
||||
}
|
||||
|
||||
// reviewWhere turns a filter into predicates appended to reviewFromSQL.
|
||||
func reviewWhere(f ReviewFilter, statuses []string) (string, []any) {
|
||||
var b strings.Builder
|
||||
var args []any
|
||||
if statuses != nil {
|
||||
b.WriteString(" AND aa.grading_status IN ?")
|
||||
args = append(args, statuses)
|
||||
}
|
||||
if slug := strings.TrimSpace(f.QuizSlug); slug != "" {
|
||||
b.WriteString(" AND qz.slug = ?")
|
||||
args = append(args, slug)
|
||||
}
|
||||
if f.UserID != 0 {
|
||||
b.WriteString(" AND a.user_id = ?")
|
||||
args = append(args, f.UserID)
|
||||
}
|
||||
if f.AttemptID != 0 {
|
||||
b.WriteString(" AND aa.attempt_id = ?")
|
||||
args = append(args, f.AttemptID)
|
||||
}
|
||||
return b.String(), args
|
||||
}
|
||||
|
||||
// ReviewQueue lists the open answers matching f, oldest change first, one
|
||||
// page at a time (perPage 1..100, default 20), with the total count. An
|
||||
// invalid status is a *ValidationError.
|
||||
func (s *Service) ReviewQueue(ctx context.Context, f ReviewFilter, page, perPage int) ([]ReviewRow, int64, error) {
|
||||
statuses, problems := CheckReviewStatus(f.Status)
|
||||
if len(problems) > 0 {
|
||||
return nil, 0, &ValidationError{Problems: problems}
|
||||
}
|
||||
if page < 1 {
|
||||
page = 1
|
||||
}
|
||||
if perPage < 1 {
|
||||
perPage = 20
|
||||
}
|
||||
perPage = min(perPage, 100)
|
||||
where, args := reviewWhere(f, statuses)
|
||||
var total int64
|
||||
if err := s.db(ctx).Raw("SELECT COUNT(*)"+reviewFromSQL+where, args...).Scan(&total).Error; err != nil {
|
||||
return nil, 0, err
|
||||
}
|
||||
var scans []reviewScan
|
||||
q := reviewQueueSQL + where + " ORDER BY aa.updated_at ASC, aa.id ASC LIMIT ? OFFSET ?"
|
||||
if err := s.db(ctx).Raw(q, append(args, perPage, (page-1)*perPage)...).Scan(&scans).Error; err != nil {
|
||||
return nil, 0, err
|
||||
}
|
||||
out := make([]ReviewRow, 0, len(scans))
|
||||
for _, sc := range scans {
|
||||
out = append(out, sc.ReviewRow)
|
||||
}
|
||||
return out, total, nil
|
||||
}
|
||||
|
||||
// ReviewItem loads one open answer with its detail. A missing answer, or
|
||||
// one that is not an open answer, is ErrNotFound.
|
||||
func (s *Service) ReviewItem(ctx context.Context, answerID uint) (*ReviewDetail, error) {
|
||||
return reviewItem(ctx, s.db(ctx), answerID)
|
||||
}
|
||||
|
||||
func reviewItem(ctx context.Context, db *gorm.DB, answerID uint) (*ReviewDetail, error) {
|
||||
var scans []reviewScan
|
||||
if err := db.WithContext(ctx).Raw(reviewDetailSQL, answerID).Scan(&scans).Error; err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if len(scans) == 0 {
|
||||
return nil, ErrNotFound
|
||||
}
|
||||
return scans[0].detail(), nil
|
||||
}
|
||||
|
||||
// reviewTarget is an open answer loaded for a review write, with its
|
||||
// question and its attempt (quiz set); the attempt row is locked.
|
||||
type reviewTarget struct {
|
||||
Answer models.AttemptAnswer
|
||||
Question models.Question
|
||||
Attempt *models.Attempt
|
||||
}
|
||||
|
||||
// lockReviewTarget locks the attempt of an open answer and reads the
|
||||
// answer again under the lock, so a grading job or a re-answer cannot
|
||||
// change it in between. A missing answer or attempt, or an answer that is
|
||||
// not open, is ErrNotFound.
|
||||
func lockReviewTarget(ctx context.Context, tx *gorm.DB, answerID uint) (*reviewTarget, error) {
|
||||
tx = tx.WithContext(ctx)
|
||||
var row models.AttemptAnswer
|
||||
if err := tx.Where("id = ? AND grading_status IS NOT NULL", answerID).Take(&row).Error; err != nil {
|
||||
return nil, notFound(err)
|
||||
}
|
||||
attempt, err := lockAttempt(ctx, tx, row.AttemptID)
|
||||
if err != nil {
|
||||
return nil, notFound(err)
|
||||
}
|
||||
if err := tx.Where("id = ? AND grading_status IS NOT NULL", answerID).Take(&row).Error; err != nil {
|
||||
return nil, notFound(err)
|
||||
}
|
||||
var quiz models.Quiz
|
||||
if err := tx.Where("id = ?", attempt.QuizID).Take(&quiz).Error; err != nil {
|
||||
return nil, notFound(err)
|
||||
}
|
||||
attempt.Quiz = &quiz
|
||||
var question models.Question
|
||||
if err := tx.Where("id = ?", row.QuestionID).Take(&question).Error; err != nil {
|
||||
return nil, notFound(err)
|
||||
}
|
||||
return &reviewTarget{Answer: row, Question: question, Attempt: attempt}, nil
|
||||
}
|
||||
|
||||
// notFound maps gorm.ErrRecordNotFound to ErrNotFound.
|
||||
func notFound(err error) error {
|
||||
if errors.Is(err, gorm.ErrRecordNotFound) {
|
||||
return ErrNotFound
|
||||
}
|
||||
return err
|
||||
}
|
||||
|
||||
// Confirm makes the grader's score of a graded answer final: status
|
||||
// reviewed, points = grader_points (D-12). A reviewed answer is returned
|
||||
// unchanged, without a write or an event; a pending or failed answer has
|
||||
// no score to confirm (ErrNoGraderScore). A completed attempt is
|
||||
// recalculated, and AnswerReviewedEvent, plus AttemptReviewedEvent when
|
||||
// the attempt is now fully reviewed, fire after the commit.
|
||||
func (s *Service) Confirm(ctx context.Context, answerID uint, by Reviewer) (*ReviewDetail, error) {
|
||||
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 done, err := s.confirmTarget(ctx, tx, t, by); err != nil {
|
||||
return err
|
||||
} else if done {
|
||||
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
|
||||
}
|
||||
|
||||
// confirmTarget confirms one locked answer and reports whether it wrote:
|
||||
// false for a reviewed answer, ErrNoGraderScore without a grader score.
|
||||
// The caller recalculates the attempt.
|
||||
func (s *Service) confirmTarget(ctx context.Context, tx *gorm.DB, t *reviewTarget, by Reviewer) (bool, error) {
|
||||
status := ""
|
||||
if t.Answer.GradingStatus != nil {
|
||||
status = *t.Answer.GradingStatus
|
||||
}
|
||||
switch {
|
||||
case status == models.GradingReviewed:
|
||||
return false, nil
|
||||
case status != models.GradingGraded || t.Answer.GraderPoints == nil:
|
||||
return false, ErrNoGraderScore
|
||||
}
|
||||
return true, s.storeReview(ctx, tx, t, *t.Answer.GraderPoints, t.Answer.ReviewComment, false, by)
|
||||
}
|
||||
|
||||
// storeReview writes the owner's decision on one locked open answer and
|
||||
// registers its AnswerReviewedEvent. setComment replaces review_comment
|
||||
// with comment (nil clears it); otherwise the comment stays. grader_points
|
||||
// and grader_feedback stay for the record.
|
||||
func (s *Service) storeReview(ctx context.Context, tx *gorm.DB, t *reviewTarget, points float64, comment *string, setComment bool, by Reviewer) error {
|
||||
now := s.now().UTC()
|
||||
user, admin := by.ids()
|
||||
cols := map[string]any{
|
||||
"grading_status": models.GradingReviewed,
|
||||
"points": points,
|
||||
"is_correct": OpenCorrect(points, t.Question.Points),
|
||||
"reviewed_at": now,
|
||||
"reviewed_by_user_id": user,
|
||||
"reviewed_by_admin_id": admin,
|
||||
"updated_at": now,
|
||||
}
|
||||
if setComment {
|
||||
cols["review_comment"] = comment
|
||||
} else {
|
||||
comment = t.Answer.ReviewComment
|
||||
}
|
||||
if err := tx.WithContext(ctx).Model(&models.AttemptAnswer{}).Where("id = ?", t.Answer.ID).Updates(cols).Error; err != nil {
|
||||
return err
|
||||
}
|
||||
fireAfterCommit(ctx, s, tx, events.AnswerReviewedEventName, &events.AnswerReviewedEvent{
|
||||
QuizID: t.Attempt.QuizID,
|
||||
QuestionID: t.Question.ID,
|
||||
AttemptID: t.Attempt.ID,
|
||||
AnswerID: t.Answer.ID,
|
||||
UserID: t.Attempt.UserID,
|
||||
Points: points,
|
||||
MaxPoints: t.Question.Points,
|
||||
GraderPoints: t.Answer.GraderPoints,
|
||||
Comment: comment,
|
||||
ReviewerUserID: user,
|
||||
ReviewerAdminID: admin,
|
||||
})
|
||||
return nil
|
||||
}
|
||||
|
||||
// fireAttemptReviewedIfDone registers AttemptReviewedEvent for a completed
|
||||
// attempt that has at least one open answer and none left pending, graded
|
||||
// or failed, with the attempt's stored score; passed compares its percent
|
||||
// with the quiz's passing score. Call it after the attempt is
|
||||
// recalculated, inside the transaction that holds its lock. Without an
|
||||
// event bus it reads nothing.
|
||||
func (s *Service) fireAttemptReviewedIfDone(ctx context.Context, tx *gorm.DB, attemptID uint) error {
|
||||
if s == nil || s.Events == nil {
|
||||
return nil
|
||||
}
|
||||
tx = tx.WithContext(ctx)
|
||||
var a models.Attempt
|
||||
if err := tx.Preload("Quiz").Where("id = ?", attemptID).Take(&a).Error; err != nil {
|
||||
if errors.Is(err, gorm.ErrRecordNotFound) {
|
||||
return nil
|
||||
}
|
||||
return err
|
||||
}
|
||||
if a.CompletedAt == nil || a.Quiz == nil {
|
||||
return nil
|
||||
}
|
||||
counts, err := answerCounts(ctx, tx, []uint{attemptID})
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if c := counts[attemptID]; c.OpenN == 0 || c.UnreviewedN > 0 {
|
||||
return nil
|
||||
}
|
||||
fireAfterCommit(ctx, s, tx, events.AttemptReviewedEventName, &events.AttemptReviewedEvent{
|
||||
QuizID: a.QuizID,
|
||||
AttemptID: a.ID,
|
||||
UserID: a.UserID,
|
||||
ScorePoints: a.ScorePoints,
|
||||
MaxPoints: a.MaxPoints,
|
||||
ScorePercent: a.ScorePercent,
|
||||
Passed: a.ScorePercent != nil && *a.ScorePercent >= float64(a.Quiz.PassingScore),
|
||||
})
|
||||
return nil
|
||||
}
|
||||
40
classes/review_test.go
Normal file
40
classes/review_test.go
Normal file
@@ -0,0 +1,40 @@
|
||||
package classes_test
|
||||
|
||||
import (
|
||||
"slices"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"git.golem15.com/golem15/sm-quizzes-plugin/classes"
|
||||
)
|
||||
|
||||
// TestCheckReviewStatus: "" and open list the three unreviewed statuses,
|
||||
// a grading status lists itself, all lists everything (nil), and any
|
||||
// other value is a problem naming the valid values.
|
||||
func TestCheckReviewStatus(t *testing.T) {
|
||||
open := []string{"pending", "graded", "failed"}
|
||||
for in, want := range map[string][]string{
|
||||
"": open,
|
||||
"open": open,
|
||||
" open ": open,
|
||||
"pending": {"pending"},
|
||||
"graded": {"graded"},
|
||||
"failed": {"failed"},
|
||||
"reviewed": {"reviewed"},
|
||||
"all": nil,
|
||||
} {
|
||||
got, problems := classes.CheckReviewStatus(in)
|
||||
if len(problems) != 0 {
|
||||
t.Errorf("%q: problems %v", in, problems)
|
||||
}
|
||||
if !slices.Equal(got, want) || (want == nil) != (got == nil) {
|
||||
t.Errorf("%q = %v, want %v", in, got, want)
|
||||
}
|
||||
}
|
||||
for _, in := range []string{"done", "Open", "pending,graded"} {
|
||||
got, problems := classes.CheckReviewStatus(in)
|
||||
if got != nil || len(problems) != 1 || !strings.Contains(problems[0], "open, pending, graded, failed, reviewed, all") {
|
||||
t.Errorf("%q = %v %v, want one problem listing the valid values", in, got, problems)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user