From 2ad4e8b1ecade5dbd2f65ab699af1bb36d625ff6 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Mon, 9 Nov 2026 18:31:19 +0100 Subject: [PATCH] feat: review open answers in the admin and pick a grader per quiz - Do oceny admin screen (golem15.quizzes.reviews) over the open answers, with a read-only context partial, score and comment virtual fields saved through SetScore, confirm and regrade record actions, bulk confirm - open question type, rubric and max_length in the question form; grader dropdown on the quiz form from the grader registry - Review model with quiz, status and student filters --- admin_test.go | 71 +++- controllers/quizzes.go | 30 ++ controllers/reviews.go | 509 ++++++++++++++++++++++++ controllers/reviews/_review_context.htm | 48 +++ controllers/reviews/config_filter.yaml | 16 + controllers/reviews/config_form.yaml | 15 + controllers/reviews/config_list.yaml | 16 + controllers/reviews_test.go | 211 ++++++++++ lang/en/lang.yaml | 51 ++- lang/pl/lang.yaml | 51 ++- models/filters.go | 80 ++++ models/questions/fields.yaml | 11 + models/quizzes/columns.yaml | 2 + models/quizzes/fields.yaml | 7 + models/review.go | 60 +++ models/reviews/columns.yaml | 21 + models/reviews/fields.yaml | 19 + plugin.go | 29 +- plugin_test.go | 4 +- registry.gen.go | 2 + 20 files changed, 1244 insertions(+), 9 deletions(-) create mode 100644 controllers/reviews.go create mode 100644 controllers/reviews/_review_context.htm create mode 100644 controllers/reviews/config_filter.yaml create mode 100644 controllers/reviews/config_form.yaml create mode 100644 controllers/reviews/config_list.yaml create mode 100644 controllers/reviews_test.go create mode 100644 models/review.go create mode 100644 models/reviews/columns.yaml create mode 100644 models/reviews/fields.yaml diff --git a/admin_test.go b/admin_test.go index be602fb..9b2ee07 100644 --- a/admin_test.go +++ b/admin_test.go @@ -2,12 +2,16 @@ package quizzes_test import ( "slices" + "strings" "testing" "git.golem15.com/golem15/sm-quizzes-plugin/models" "git.golem15.com/golem15/summercms/modules/bouncer" "git.golem15.com/golem15/summercms/modules/cabana" + "git.golem15.com/golem15/summercms/modules/lagoon" "git.golem15.com/golem15/summercms/modules/pact" + "gorm.io/gorm" + "gorm.io/gorm/utils/tests" ) func TestAdminCompiles(t *testing.T) { @@ -37,7 +41,7 @@ func TestAdminCompiles(t *testing.T) { } } slices.Sort(ids) - want := []string{"golem15.quizzes.answers", "golem15.quizzes.attempts", "golem15.quizzes.questions", "golem15.quizzes.quizzes"} + want := []string{"golem15.quizzes.answers", "golem15.quizzes.attempts", "golem15.quizzes.questions", "golem15.quizzes.quizzes", "golem15.quizzes.reviews"} if !slices.Equal(ids, want) { t.Errorf("controllers = %v, want %v", ids, want) } @@ -52,17 +56,78 @@ func TestAdminCompiles(t *testing.T) { } nav := p.(pact.HasNavigation).Navigation() - if len(nav) != 1 || nav[0].Label != "golem15.quizzes::lang.plugin.name" || len(nav[0].SideMenu) != 4 { + if len(nav) != 1 || nav[0].Label != "golem15.quizzes::lang.plugin.name" || len(nav[0].SideMenu) != 5 { t.Errorf("navigation = %+v", nav) + } else { + var codes []string + for _, item := range nav[0].SideMenu { + codes = append(codes, item.Code) + } + if !slices.Equal(codes, []string{"quizzes", "questions", "answers", "reviews", "attempts"}) { + t.Errorf("side menu = %v, want reviews after answers", codes) + } + if review := nav[0].SideMenu[3]; review.Controller != "golem15.quizzes.reviews" || !slices.Equal(review.Permissions, []string{"golem15.quizzes.access_quizzes"}) { + t.Errorf("reviews entry = %+v", review) + } } } func TestQuestionFilterScopes(t *testing.T) { q := models.Question{} - if got := q.FilterOptions(models.ScopeByType); len(got) != 3 { + if got := q.FilterOptions(models.ScopeByType); len(got) != 4 || got[3].Value != models.TypeOpen { t.Errorf("type options = %v", got) } if got := q.FilterScope("unknown", nil, "x"); got != nil { t.Errorf("FilterScope on a nil db = %v", got) } } + +func TestReviewFilterScopes(t *testing.T) { + r := models.Review{} + if got := r.FilterScopes(); !slices.Equal(got, []string{models.ScopeByQuiz, models.ScopeByStatus, models.ScopeByStudent}) { + t.Errorf("FilterScopes = %v", got) + } + opts := r.FilterOptions(models.ScopeByStatus) + var values []string + for _, o := range opts { + values = append(values, o.Value) + if o.Label != "golem15.quizzes::lang.reviews.status_"+o.Value { + t.Errorf("status %s label = %q", o.Value, o.Label) + } + } + if !slices.Equal(values, []string{"pending", "graded", "failed", "reviewed"}) { + t.Errorf("status options = %v", values) + } + if got := r.FilterScope("unknown", nil, "x"); got != nil { + t.Errorf("FilterScope on a nil db = %v", got) + } + + db, err := gorm.Open(tests.DummyDialector{}, &gorm.Config{DryRun: true}) + if err != nil { + t.Fatalf("dry-run db: %v", err) + } + sqlOf := func(scope string, value any) string { + var rows []models.Review + stmt := r.FilterScope(scope, db.Session(&gorm.Session{DryRun: true}).Model(&models.Review{}), value).Find(&rows).Statement + return stmt.SQL.String() + } + if got := sqlOf(models.ScopeByStatus, []any{"graded"}); !strings.Contains(got, "golem15_quizzes_attempt_answers.grading_status IN") { + t.Errorf("status SQL = %s", got) + } + if got := sqlOf(models.ScopeByStatus, []any{"bogus"}); !strings.Contains(got, "1 = 0") { + t.Errorf("unknown status SQL = %s, want no rows", got) + } + if got := sqlOf(models.ScopeByQuiz, []any{"3"}); !strings.Contains(got, "question_id IN (SELECT id FROM golem15_quizzes_questions WHERE quiz_id IN") { + t.Errorf("quiz SQL = %s", got) + } + if got := sqlOf(models.ScopeByStudent, []any{"7"}); !strings.Contains(got, "attempt_id IN (SELECT id FROM golem15_quizzes_attempts WHERE user_id IN") { + t.Errorf("student SQL = %s", got) + } + if got := sqlOf(models.ScopeByQuiz, nil); strings.Contains(got, "question_id IN") { + t.Errorf("empty value SQL = %s, want no filter", got) + } + + if name, err := lagoon.MorphType(nil, &models.Review{}); err != nil || name == "golem15_quizzes_attempt_answers" { + t.Errorf("Review morph type = %q, %v; it must differ from AttemptAnswer's table", name, err) + } +} diff --git a/controllers/quizzes.go b/controllers/quizzes.go index dd0798e..21956e7 100644 --- a/controllers/quizzes.go +++ b/controllers/quizzes.go @@ -3,6 +3,7 @@ package controllers import ( + "git.golem15.com/golem15/sm-quizzes-plugin/graders" "git.golem15.com/golem15/sm-quizzes-plugin/models" "git.golem15.com/golem15/summercms/modules/pact" ) @@ -11,8 +12,17 @@ var ( _ pact.AdminController = quizzesAdmin{} _ pact.AdminRecordSource = quizzesAdmin{} _ pact.AdminPermissioned = quizzesAdmin{} + + _ pact.DropdownOptionsProvider = quizzesAdmin{} ) +// GraderOptions lists the grader dropdown choices: the registry golem15.quizzes +// built at boot. The plugin sets it at Boot; nil offers manual only. +// +// TODO: requires SummerCMS change. DropdownOptions receives neither a +// context nor the app, so the registry comes through a package-level hook. +var GraderOptions func() []graders.Option + // quizzesAdmin lists and edits quizzes. Questions have their own controller // filtered by quiz. // @@ -33,5 +43,25 @@ func (quizzesAdmin) RequiredPermissions() []string { return []string{"golem15.quizzes.access_quizzes"} } +// DropdownOptions serves the grader dropdown (options: getGraderOptions): +// manual, then every grader an activated plugin registered. Other fields +// have no method-backed options. +func (quizzesAdmin) DropdownOptions(field string) []pact.Option { + if field != "grader" { + return nil + } + opts := []graders.Option{{Code: graders.Manual, Label: graders.ManualLabel}} + if GraderOptions != nil { + if got := GraderOptions(); len(got) > 0 { + opts = got + } + } + out := make([]pact.Option, 0, len(opts)) + for _, o := range opts { + out = append(out, pact.Option{Value: o.Code, Label: o.Label}) + } + return out +} + // QuizzesController returns the golem15.quizzes.quizzes admin controller. func QuizzesController() pact.AdminController { return quizzesAdmin{} } diff --git a/controllers/reviews.go b/controllers/reviews.go new file mode 100644 index 0000000..4a1cb5f --- /dev/null +++ b/controllers/reviews.go @@ -0,0 +1,509 @@ +package controllers + +import ( + "context" + "encoding/json" + "errors" + "math" + "strconv" + "strings" + "time" + + "git.golem15.com/golem15/sm-quizzes-plugin/classes" + "git.golem15.com/golem15/sm-quizzes-plugin/graders" + "git.golem15.com/golem15/sm-quizzes-plugin/models" + "git.golem15.com/golem15/summercms/modules/bouncer" + "git.golem15.com/golem15/summercms/modules/cabana" + "git.golem15.com/golem15/summercms/modules/pact" + "gorm.io/gorm" +) + +var ( + _ pact.AdminController = reviewsAdmin{} + _ pact.AdminRecordSource = reviewsAdmin{} + _ pact.AdminPermissioned = reviewsAdmin{} + _ pact.ListExtendQuery = reviewsAdmin{} + _ pact.FormExtendQuery = reviewsAdmin{} + _ pact.FormRecordTitle = reviewsAdmin{} + _ pact.FormBeforeCreate = reviewsAdmin{} + _ pact.FormBeforeDelete = reviewsAdmin{} + _ pact.FormVirtualFields = reviewsAdmin{} + _ pact.FormRules = reviewsAdmin{} + _ pact.FormAfterUpdate = reviewsAdmin{} + _ pact.HasAdminBulkActions = reviewsAdmin{} + _ pact.HasAdminRecordActions = reviewsAdmin{} + _ pact.AdminPartialData = reviewsAdmin{} +) + +// Reviews builds the review service of an admin request: over the request's +// write transaction (cabana.TxFromContext) when there is one, else over the +// application's database. The plugin sets it at Boot. +// +// TODO: requires SummerCMS change. Admin hooks receive no app, so, as +// models.FilterDB, the service comes through a package-level hook. +var Reviews func(ctx context.Context) *classes.Service + +// reviewsLang prefixes the lang keys of the review screen. +const reviewsLang = "golem15.quizzes::lang.reviews." + +// The virtual fields of the review form and its partial. +const ( + fieldReviewPoints = "review_points" + fieldReviewComment = "review_comment" + partialReview = "review_context" +) + +// errReviewsUnavailable is an admin request before the plugin booted. +var errReviewsUnavailable = errors.New("quizzes: the review service is not available") + +// reviewsAdmin is the "Do oceny" screen: the open answers of every quiz, +// with the grader's result, the owner's score and comment, and the confirm +// and regrade actions. Answers come from students, so it never creates or +// deletes one; every write goes through the review service shared with the +// MCP review tools. +type reviewsAdmin struct{} + +func (reviewsAdmin) ID() string { return "golem15.quizzes.reviews" } +func (reviewsAdmin) ModelName() string { return "Review" } +func (reviewsAdmin) ConfigDir() string { return "controllers/reviews" } + +// NewRecord returns the model the generic admin handlers query. +func (reviewsAdmin) NewRecord() any { return &models.Review{} } + +// RequiredPermissions is the quiz editors' permission (D-13): reviewing +// needs no permission of its own. +func (reviewsAdmin) RequiredPermissions() []string { + return []string{"golem15.quizzes.access_quizzes"} +} + +// openAnswers keeps the rows of open questions: only they have a grading +// status. +func openAnswers(db *gorm.DB) *gorm.DB { + return db.Where("golem15_quizzes_attempt_answers.grading_status IS NOT NULL") +} + +// ListExtendQuery lists open answers only. +func (reviewsAdmin) ListExtendQuery(_ context.Context, db *gorm.DB) *gorm.DB { return openAnswers(db) } + +// FormExtendQuery opens open answers only. It preloads nothing: the admin +// save writes the loaded record back, and a preloaded question or attempt +// would be written with it. +func (reviewsAdmin) FormExtendQuery(_ context.Context, db *gorm.DB) *gorm.DB { return openAnswers(db) } + +// FormRecordTitle names an answer by its quiz and student. +func (reviewsAdmin) FormRecordTitle(ctx context.Context, _ *gorm.DB, record any) (string, error) { + rec, ok := record.(*models.Review) + if !ok || rec == nil { + return "", nil + } + svc, err := reviewService(ctx) + if err != nil { + return "#" + strconv.FormatUint(uint64(rec.ID), 10), nil + } + item, err := svc.ReviewItem(ctx, rec.ID) + if err != nil { + return "#" + strconv.FormatUint(uint64(rec.ID), 10), nil + } + return item.QuizTitle + " · " + item.Email, nil +} + +// refusedWrite is the answer to a create or a delete: open answers come +// from students. +func refusedWrite() error { + return &cabana.ForbiddenError{Message: reviewsLang + "not_created"} +} + +// FormBeforeCreate refuses: open answers come from students. +func (reviewsAdmin) FormBeforeCreate(context.Context, any) error { return refusedWrite() } + +// FormBeforeDelete refuses: an answer is removed with its attempt or quiz. +func (reviewsAdmin) FormBeforeDelete(context.Context, any) error { return refusedWrite() } + +// FormVirtualFields are the owner's score and comment. +func (reviewsAdmin) FormVirtualFields() []string { + return []string{fieldReviewPoints, fieldReviewComment} +} + +// FormRules check the virtual fields; SetScore checks the points against +// the question. +func (reviewsAdmin) FormRules(context.Context, string) map[string]string { + return map[string]string{ + fieldReviewPoints: "nullable|numeric|min:0", + fieldReviewComment: "nullable|max:" + strconv.Itoa(classes.MaxReviewComment), + } +} + +// FormAfterUpdate stores the owner's score and comment through SetScore in +// the save transaction. An empty form saves nothing. +func (reviewsAdmin) FormAfterUpdate(ctx context.Context, model any) error { + values, _ := cabana.VirtualFieldsFromContext(ctx) + points, comment, err := reviewValues(values) + if err != nil || points == nil { + return err + } + rec, ok := model.(*models.Review) + if !ok || rec == nil { + return errors.New("quizzes: review form model is not a review") + } + svc, err := reviewService(ctx) + if err != nil { + return err + } + _, err = svc.SetScore(ctx, rec.ID, *points, comment, classes.Reviewer{AdminID: adminID(ctx)}) + return reviewError(err) +} + +// reviewValues reads the virtual fields: review_points as a number or a +// numeric string (a decimal comma is accepted) and review_comment trimmed. +// No points and no comment is nothing to save (nil, nil, nil); a comment +// without points, or points that are not a number, is a +// *cabana.ValidationError on review_points. +func reviewValues(values map[string]any) (*float64, *string, error) { + var comment *string + if raw, ok := values[fieldReviewComment].(string); ok { + if c := strings.TrimSpace(raw); c != "" { + comment = &c + } + } + points, ok := numberValue(values[fieldReviewPoints]) + if !ok { + return nil, nil, pointsError("points_invalid") + } + if points == nil { + if comment != nil { + return nil, nil, pointsError("points_required") + } + return nil, nil, nil + } + return points, comment, nil +} + +// numberValue reads a submitted number: nil or "" is no value; a value +// that is not a finite number is false. +func numberValue(v any) (*float64, bool) { + var f float64 + switch n := v.(type) { + case nil: + return nil, true + case float64: + f = n + case float32: + f = float64(n) + case int: + f = float64(n) + case int64: + f = float64(n) + case uint: + f = float64(n) + case uint64: + f = float64(n) + case json.Number: + parsed, err := n.Float64() + if err != nil { + return nil, false + } + f = parsed + case string: + s := strings.TrimSpace(n) + if s == "" { + return nil, true + } + parsed, err := strconv.ParseFloat(strings.Replace(s, ",", ".", 1), 64) + if err != nil { + return nil, false + } + f = parsed + default: + return nil, false + } + if math.IsNaN(f) || math.IsInf(f, 0) { + return nil, false + } + return &f, true +} + +// pointsError is a validation error on review_points with a reviews lang +// key. +func pointsError(key string) error { + return &cabana.ValidationError{Details: map[string]any{fieldReviewPoints: []string{reviewsLang + key}}} +} + +// reviewError maps the review service's refusals to admin answers: a +// rule broken by the score becomes a validation error on review_points, +// a state conflict a refusal with its message. Anything else passes. +func reviewError(err error) error { + if err == nil { + return nil + } + var invalid *classes.ValidationError + switch { + case errors.As(err, &invalid): + return pointsError("points_invalid") + case errors.Is(err, classes.ErrNoGraderScore): + return &cabana.ForbiddenError{Message: reviewsLang + "no_grader_score"} + case errors.Is(err, classes.ErrAlreadyReviewed): + return &cabana.ForbiddenError{Message: reviewsLang + "already_reviewed"} + case errors.Is(err, classes.ErrGraderManual): + return &cabana.ForbiddenError{Message: reviewsLang + "grader_manual"} + case errors.Is(err, classes.ErrNotFound): + return &cabana.ForbiddenError{Message: reviewsLang + "not_found"} + } + return err +} + +// reviewService is the review service of this request. +func reviewService(ctx context.Context) (*classes.Service, error) { + if Reviews == nil { + return nil, errReviewsUnavailable + } + svc := Reviews(ctx) + if svc == nil || svc.DB == nil { + return nil, errReviewsUnavailable + } + return svc, nil +} + +// adminID is the signed-in administrator of an admin request. +func adminID(ctx context.Context) uint { + if p, ok := bouncer.User(ctx); ok && p != nil { + return p.ID + } + return 0 +} + +// reviewer is the administrator as a review's author. +func reviewer(ctx context.Context) classes.Reviewer { + return classes.Reviewer{AdminID: adminID(ctx)} +} + +// AdminRecordActions are confirm (a graded answer's score becomes final) +// and regrade (the answer goes back to the quiz's grader). +func (reviewsAdmin) AdminRecordActions() []pact.AdminRecordAction { + return []pact.AdminRecordAction{ + { + Name: "confirm", + Label: reviewsLang + "confirm", + Applies: func(_ context.Context, record any) (bool, error) { + rec, ok := record.(*models.Review) + return ok && rec != nil && rec.ReviewStatus() == models.GradingGraded, nil + }, + Run: func(ctx context.Context, in pact.AdminRecordActionInput) (pact.AdminRecordActionResult, error) { + svc, err := reviewService(ctx) + if err != nil { + return pact.AdminRecordActionResult{}, err + } + if _, err := svc.Confirm(ctx, uint(in.RecordID), reviewer(ctx)); err != nil { + return pact.AdminRecordActionResult{}, reviewError(err) + } + return pact.AdminRecordActionResult{Message: reviewsLang + "confirmed"}, nil + }, + }, + { + Name: "regrade", + Label: reviewsLang + "regrade", + Confirm: reviewsLang + "regrade_confirm", + Applies: func(ctx context.Context, record any) (bool, error) { + rec, ok := record.(*models.Review) + if !ok || rec == nil { + return false, nil + } + switch rec.ReviewStatus() { + case models.GradingPending, models.GradingGraded, models.GradingFailed: + default: + return false, nil + } + return canRegrade(ctx, rec) + }, + Run: func(ctx context.Context, in pact.AdminRecordActionInput) (pact.AdminRecordActionResult, error) { + svc, err := reviewService(ctx) + if err != nil { + return pact.AdminRecordActionResult{}, err + } + if _, err := svc.Regrade(ctx, classes.ReviewScope{AnswerID: uint(in.RecordID)}); err != nil { + return pact.AdminRecordActionResult{}, reviewError(err) + } + return pact.AdminRecordActionResult{Message: reviewsLang + "regraded"}, nil + }, + }, + } +} + +// canRegrade reports whether the answer's quiz has a grader an activated +// plugin registered (not manual). +func canRegrade(ctx context.Context, rec *models.Review) (bool, error) { + svc, err := reviewService(ctx) + if err != nil { + return false, nil + } + quiz := quizOf(rec) + if quiz == nil { + var q models.Quiz + err := svc.DB.WithContext(ctx). + Where("id = (SELECT quiz_id FROM golem15_quizzes_attempts WHERE id = ?)", rec.AttemptID). + Take(&q).Error + if errors.Is(err, gorm.ErrRecordNotFound) { + return false, nil + } + if err != nil { + return false, err + } + quiz = &q + } + _, ok := classes.GraderFor(svc.Graders, quiz) + return ok, nil +} + +// quizOf is the record's quiz when it was loaded with it. +func quizOf(rec *models.Review) *models.Quiz { + if rec.Attempt != nil && rec.Attempt.Quiz != nil { + return rec.Attempt.Quiz + } + if rec.Question != nil && rec.Question.Quiz != nil { + return rec.Question.Quiz + } + return nil +} + +// AdminBulkActions is confirm: every selected graded answer gets its +// grader's score as final; other statuses are skipped. +func (reviewsAdmin) AdminBulkActions() []pact.AdminBulkAction { + return []pact.AdminBulkAction{{ + Name: "confirm", + Label: reviewsLang + "confirm_selected", + Run: func(ctx context.Context, in pact.AdminBulkActionInput) (pact.AdminBulkActionResult, error) { + svc, err := reviewService(ctx) + if err != nil { + return pact.AdminBulkActionResult{}, err + } + by := reviewer(ctx) + affected := 0 + for _, record := range in.Records { + rec, ok := record.(*models.Review) + if !ok || rec == nil || rec.ReviewStatus() != models.GradingGraded { + continue + } + if _, err := svc.Confirm(ctx, rec.ID, by); err != nil { + return pact.AdminBulkActionResult{}, reviewError(err) + } + affected++ + } + return pact.AdminBulkActionResult{Affected: affected}, nil + }, + }} +} + +// reviewContextView is the view model of the _review_context partial: plain +// strings, numbers and booleans only, so the template's escaping covers +// every value (the student's text and the grader's feedback are untrusted). +type reviewContextView struct { + QuizTitle string + StudentEmail string + StudentName string + QuestionText string + QuestionPoints int + ModelAnswer string + Rubric string + MaxLength int + Text string + Status string + StatusLabel string + Grader string + GraderLabel string + HasGraderPoints bool + GraderPoints float64 + GraderFeedback string + GradingError string + GradingAttempts int + GradedAt string + Points float64 + ReviewComment string + Reviewed bool + ReviewedAt string +} + +// PartialData builds the review context of an answer from the review +// service; an unknown partial is an error. +func (reviewsAdmin) PartialData(ctx context.Context, name string, record any) (any, error) { + if name != partialReview { + return nil, errors.New("quizzes: unknown partial " + name) + } + rec, ok := record.(*models.Review) + if !ok || rec == nil || rec.ID == 0 { + return reviewContextView{}, nil + } + svc, err := reviewService(ctx) + if err != nil { + return nil, err + } + item, err := svc.ReviewItem(ctx, rec.ID) + if err != nil { + return nil, err + } + return newReviewContextView(item, svc.Graders), nil +} + +// newReviewContextView copies a review detail into the partial's view +// model. +func newReviewContextView(item *classes.ReviewDetail, reg *graders.Registry) reviewContextView { + v := reviewContextView{ + QuizTitle: item.QuizTitle, + StudentEmail: item.Email, + StudentName: strings.TrimSpace(deref(item.Name) + " " + deref(item.Surname)), + QuestionText: item.QuestionText, + QuestionPoints: item.QuestionPoints, + ModelAnswer: deref(item.AnswerText), + Rubric: deref(item.Rubric), + MaxLength: item.MaxLength, + Text: item.Text, + Status: item.Status, + StatusLabel: reviewsLang + "status_" + item.Status, + Grader: deref(item.Grader), + GraderFeedback: deref(item.GraderFeedback), + GradingError: deref(item.GradingError), + GradingAttempts: item.GradingAttempts, + GradedAt: formatTime(item.GradedAt), + Points: item.Points, + ReviewComment: deref(item.ReviewComment), + Reviewed: item.Status == models.GradingReviewed, + ReviewedAt: formatTime(item.ReviewedAt), + } + if item.GraderPoints != nil { + v.HasGraderPoints, v.GraderPoints = true, *item.GraderPoints + } + v.GraderLabel = graderLabel(reg, v.Grader) + return v +} + +// graderLabel is the lang key or name of a grader code: Manual's label, a +// registered grader's label, else the code itself. +func graderLabel(reg *graders.Registry, code string) string { + if code == "" { + return "" + } + if code == graders.Manual { + return graders.ManualLabel + } + if g, ok := reg.Get(code); ok { + if label := g.Label(); label != "" { + return label + } + } + return code +} + +func deref(s *string) string { + if s == nil { + return "" + } + return *s +} + +func formatTime(t *time.Time) string { + if t == nil { + return "" + } + return t.Local().Format("2006-01-02 15:04") +} + +// ReviewsController returns the golem15.quizzes.reviews admin controller. +func ReviewsController() pact.AdminController { return reviewsAdmin{} } diff --git a/controllers/reviews/_review_context.htm b/controllers/reviews/_review_context.htm new file mode 100644 index 0000000..8cd13c2 --- /dev/null +++ b/controllers/reviews/_review_context.htm @@ -0,0 +1,48 @@ +
+{{with .Data}} +
+
{{trans "golem15.quizzes::lang.reviews.quiz"}}
+
{{.QuizTitle}}
+
{{trans "golem15.quizzes::lang.reviews.student"}}
+
{{if .StudentName}}{{.StudentName}} {{.StudentEmail}}{{else}}{{.StudentEmail}}{{end}}
+
{{trans "golem15.quizzes::lang.reviews.question"}} ({{.QuestionPoints}} {{trans "golem15.quizzes::lang.reviews.points_short"}})
+

{{.QuestionText}}

+{{if .ModelAnswer}} +
{{trans "golem15.quizzes::lang.reviews.model_answer"}}
+

{{.ModelAnswer}}

+{{end}} +{{if .Rubric}} +
{{trans "golem15.quizzes::lang.reviews.rubric"}}
+

{{.Rubric}}

+{{end}} +
{{trans "golem15.quizzes::lang.reviews.text"}} {{trans "golem15.quizzes::lang.reviews.max_length"}}: {{.MaxLength}}
+
{{.Text}}
+
{{trans "golem15.quizzes::lang.reviews.status"}}
+
{{trans .StatusLabel}}
+{{if .Grader}} +
{{trans "golem15.quizzes::lang.reviews.grader"}}
+
{{trans .GraderLabel}} {{trans "golem15.quizzes::lang.reviews.grading_attempts"}}: {{.GradingAttempts}}{{if .GradedAt}} · {{trans "golem15.quizzes::lang.reviews.graded_at"}}: {{.GradedAt}}{{end}}
+{{end}} +{{if .HasGraderPoints}} +
{{trans "golem15.quizzes::lang.reviews.grader_points"}}
+
{{.GraderPoints}}
+{{end}} +{{if .GraderFeedback}} +
{{trans "golem15.quizzes::lang.reviews.grader_feedback"}}
+

{{.GraderFeedback}}

+{{end}} +{{if .GradingError}} +
{{trans "golem15.quizzes::lang.reviews.grading_error"}}
+

{{.GradingError}}

+{{end}} +{{if .Reviewed}} +
{{trans "golem15.quizzes::lang.reviews.points"}}
+
{{.Points}} / {{.QuestionPoints}}{{if .ReviewedAt}} {{trans "golem15.quizzes::lang.reviews.reviewed_at"}}: {{.ReviewedAt}}{{end}}
+{{end}} +{{if .ReviewComment}} +
{{trans "golem15.quizzes::lang.reviews.review_comment"}}
+

{{.ReviewComment}}

+{{end}} +
+{{end}} +
diff --git a/controllers/reviews/config_filter.yaml b/controllers/reviews/config_filter.yaml new file mode 100644 index 0000000..85fe237 --- /dev/null +++ b/controllers/reviews/config_filter.yaml @@ -0,0 +1,16 @@ +scopes: + quiz: + label: golem15.quizzes::lang.reviews.quiz + modelClass: Quiz + nameFrom: title + scope: filterByQuiz + status: + label: golem15.quizzes::lang.reviews.status + modelClass: Review + nameFrom: grading_status + scope: filterByStatus + student: + label: golem15.quizzes::lang.reviews.student + modelClass: Review + nameFrom: grading_status + scope: filterByStudent diff --git a/controllers/reviews/config_form.yaml b/controllers/reviews/config_form.yaml new file mode 100644 index 0000000..d1190d6 --- /dev/null +++ b/controllers/reviews/config_form.yaml @@ -0,0 +1,15 @@ +name: golem15.quizzes::lang.reviews.answer +form: ~/plugins/golem15/quizzes/models/reviews/fields.yaml +modelClass: Review +defaultRedirect: golem15/quizzes/reviews + +# Open answers come from students: there is no create. Saving the form +# stores the owner's score and comment, then returns to the preview. +update: + redirect: golem15/quizzes/reviews/preview/:id + redirectClose: golem15/quizzes/reviews + +# The preview screen shows the answer and offers the record actions. +preview: {} + +recordActions: [confirm, regrade] diff --git a/controllers/reviews/config_list.yaml b/controllers/reviews/config_list.yaml new file mode 100644 index 0000000..b93bb09 --- /dev/null +++ b/controllers/reviews/config_list.yaml @@ -0,0 +1,16 @@ +list: ~/plugins/golem15/quizzes/models/reviews/columns.yaml +modelClass: Review +title: golem15.quizzes::lang.reviews.title +recordUrl: golem15/quizzes/reviews/preview/:id +recordsPerPage: 50 +perPageOptions: [20, 50, 100] +showCheckboxes: true +defaultSort: + column: updated_at + direction: asc +filter: config_filter.yaml +bulkActions: [confirm] +toolbar: + buttons: [] + search: + prompt: backend::lang.list.search_prompt diff --git a/controllers/reviews_test.go b/controllers/reviews_test.go new file mode 100644 index 0000000..d0fc518 --- /dev/null +++ b/controllers/reviews_test.go @@ -0,0 +1,211 @@ +package controllers + +import ( + "bytes" + "errors" + "html/template" + "os" + "reflect" + "slices" + "strings" + "testing" + + "git.golem15.com/golem15/sm-quizzes-plugin/classes" + "git.golem15.com/golem15/sm-quizzes-plugin/graders" + "git.golem15.com/golem15/summercms/modules/cabana" + "git.golem15.com/golem15/summercms/modules/pact" +) + +func pointsDetail(t *testing.T, err error) string { + t.Helper() + var invalid *cabana.ValidationError + if !errors.As(err, &invalid) { + t.Fatalf("err = %v, want a cabana.ValidationError", err) + } + msgs, _ := invalid.Details[fieldReviewPoints].([]string) + if len(msgs) != 1 { + t.Fatalf("details = %v, want one message on review_points", invalid.Details) + } + return msgs[0] +} + +func TestReviewValues(t *testing.T) { + points, comment, err := reviewValues(map[string]any{"review_points": "1.5", "review_comment": " dobrze "}) + if err != nil || points == nil || *points != 1.5 || comment == nil || *comment != "dobrze" { + t.Errorf("1.5 + comment = %v, %v, %v", points, comment, err) + } + if points, _, err := reviewValues(map[string]any{"review_points": float64(2)}); err != nil || points == nil || *points != 2 { + t.Errorf("number 2 = %v, %v", points, err) + } + if points, _, err := reviewValues(map[string]any{"review_points": 2}); err != nil || points == nil || *points != 2 { + t.Errorf("int 2 = %v, %v", points, err) + } + if points, _, err := reviewValues(map[string]any{"review_points": "1,5"}); err != nil || points == nil || *points != 1.5 { + t.Errorf("decimal comma = %v, %v", points, err) + } + if points, comment, err := reviewValues(map[string]any{"review_points": 0.0, "review_comment": " "}); err != nil || points == nil || *points != 0 || comment != nil { + t.Errorf("zero without comment = %v, %v, %v", points, comment, err) + } + for _, values := range []map[string]any{nil, {}, {"review_points": "", "review_comment": ""}, {"review_points": nil}} { + if points, comment, err := reviewValues(values); points != nil || comment != nil || err != nil { + t.Errorf("%v = %v, %v, %v, want nothing to save", values, points, comment, err) + } + } + _, _, err = reviewValues(map[string]any{"review_comment": "bez punktów"}) + if got := pointsDetail(t, err); got != reviewsLang+"points_required" { + t.Errorf("comment without points = %q", got) + } + for _, bad := range []any{"abc", "NaN", "Inf", true} { + _, _, err := reviewValues(map[string]any{"review_points": bad}) + if got := pointsDetail(t, err); got != reviewsLang+"points_invalid" { + t.Errorf("%v = %q", bad, got) + } + } +} + +func TestReviewErrorMapping(t *testing.T) { + if got := pointsDetail(t, reviewError(&classes.ValidationError{Problems: []string{"points"}})); got != reviewsLang+"points_invalid" { + t.Errorf("validation = %q", got) + } + for err, key := range map[error]string{ + classes.ErrNoGraderScore: "no_grader_score", + classes.ErrAlreadyReviewed: "already_reviewed", + classes.ErrGraderManual: "grader_manual", + classes.ErrNotFound: "not_found", + } { + var refused *cabana.ForbiddenError + if !errors.As(reviewError(err), &refused) || refused.Message != reviewsLang+key { + t.Errorf("%v = %v, want refusal %s", err, reviewError(err), key) + } + } + other := errors.New("db down") + if got := reviewError(other); got != other { + t.Errorf("other error = %v", got) + } + if reviewError(nil) != nil { + t.Error("nil error mapped") + } +} + +func TestGraderDropdown(t *testing.T) { + saved := GraderOptions + t.Cleanup(func() { GraderOptions = saved }) + + GraderOptions = nil + want := []pact.Option{{Value: graders.Manual, Label: "golem15.quizzes::lang.graders.manual"}} + if got := (quizzesAdmin{}).DropdownOptions("grader"); !slices.Equal(got, want) { + t.Errorf("no hook = %v, want %v", got, want) + } + + GraderOptions = func() []graders.Option { + return []graders.Option{{Code: graders.Manual, Label: graders.ManualLabel}, {Code: "ai", Label: "vendor.plugin::lang.grader"}} + } + want = []pact.Option{{Value: "manual", Label: graders.ManualLabel}, {Value: "ai", Label: "vendor.plugin::lang.grader"}} + if got := (quizzesAdmin{}).DropdownOptions("grader"); !slices.Equal(got, want) { + t.Errorf("hook = %v, want %v", got, want) + } + if got := (quizzesAdmin{}).DropdownOptions("mode"); got != nil { + t.Errorf("mode = %v, want nil", got) + } +} + +func TestReviewContextViewIsPlain(t *testing.T) { + typ := reflect.TypeOf(reviewContextView{}) + for i := range typ.NumField() { + f := typ.Field(i) + switch f.Type.Kind() { + case reflect.String, reflect.Bool, + reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, + reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, + reflect.Float32, reflect.Float64: + default: + t.Errorf("field %s is %s; the view model holds strings, numbers and booleans only", f.Name, f.Type) + } + if f.Type.PkgPath() != "" { + t.Errorf("field %s has the named type %s", f.Name, f.Type) + } + } + + name, surname := "Anna", "Nowak" + grader, feedback := "ai", "ok" + gp := 1.5 + v := newReviewContextView(&classes.ReviewDetail{ + ReviewRow: classes.ReviewRow{Status: "graded", QuizTitle: "Q", Email: "a@example.com", Name: &name, Surname: &surname, Text: "", GraderPoints: &gp}, + Grader: &grader, + GraderFeedback: &feedback, + }, nil) + if v.StudentName != "Anna Nowak" || v.StatusLabel != reviewsLang+"status_graded" || !v.HasGraderPoints || v.GraderPoints != 1.5 { + t.Errorf("view = %+v", v) + } + if v.GraderLabel != "ai" { + t.Errorf("unregistered grader label = %q, want the code", v.GraderLabel) + } + if v.Text != "" || v.GraderFeedback != "ok" { + t.Errorf("texts are copied raw for the template to escape: %+v", v) + } +} + +func TestReviewsControllerShape(t *testing.T) { + c := reviewsAdmin{} + if c.ID() != "golem15.quizzes.reviews" || c.ConfigDir() != "controllers/reviews" { + t.Errorf("controller = %s %s", c.ID(), c.ConfigDir()) + } + if !slices.Equal(c.RequiredPermissions(), []string{"golem15.quizzes.access_quizzes"}) { + t.Errorf("permissions = %v", c.RequiredPermissions()) + } + if !slices.Equal(c.FormVirtualFields(), []string{"review_points", "review_comment"}) { + t.Errorf("virtual fields = %v", c.FormVirtualFields()) + } + var refused *cabana.ForbiddenError + if err := c.FormBeforeCreate(t.Context(), nil); !errors.As(err, &refused) { + t.Errorf("create = %v, want a refusal", err) + } + if err := c.FormBeforeDelete(t.Context(), nil); !errors.As(err, &refused) { + t.Errorf("delete = %v, want a refusal", err) + } + var names []string + for _, a := range c.AdminRecordActions() { + names = append(names, a.Name) + } + if !slices.Equal(names, []string{"confirm", "regrade"}) { + t.Errorf("record actions = %v", names) + } + if bulk := c.AdminBulkActions(); len(bulk) != 1 || bulk[0].Name != "confirm" { + t.Errorf("bulk actions = %+v", bulk) + } + if _, err := c.PartialData(t.Context(), "other", nil); err == nil { + t.Error("unknown partial gave no error") + } +} + +func TestReviewContextEscapes(t *testing.T) { + src, err := os.ReadFile("reviews/_review_context.htm") + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(src), "safe") || strings.Contains(string(src), "style=") { + t.Error("the partial must not mark text as safe or use inline styles") + } + tpl, err := template.New("review_context").Funcs(template.FuncMap{"trans": func(k string) string { return k }}).Parse(string(src)) + if err != nil { + t.Fatalf("parse: %v", err) + } + var out bytes.Buffer + view := reviewContextView{ + Text: "", GraderFeedback: "", + ReviewComment: "c", Rubric: "r", Grader: "ai", GraderLabel: "ai", Reviewed: true, + StatusLabel: reviewsLang + "status_reviewed", + } + if err := tpl.Execute(&out, map[string]any{"Data": view}); err != nil { + t.Fatalf("execute: %v", err) + } + html := out.String() + for _, raw := range []string{"