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 @@ +
{{.QuestionText}}
{{.ModelAnswer}}
{{.Rubric}}
{{.Text}}
{{.GraderFeedback}}
{{.GradingError}}
{{.ReviewComment}}