diff --git a/classes/review.go b/classes/review.go index 5f3e5ac..601ca04 100644 --- a/classes/review.go +++ b/classes/review.go @@ -29,7 +29,8 @@ import ( // 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. + // pending, graded and failed, except those of a test attempt that is + // still running. It is the default filter. ReviewStatusOpen = "open" // ReviewStatusAll lists every open answer, reviewed ones included. ReviewStatusAll = "all" @@ -52,6 +53,10 @@ var ( // 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") + // ErrAttemptOpen is a review write on an answer of a test attempt that + // is still running: a test's results, the owner's review included, come + // only after it ends (Phase 7 D-07, D-08). + ErrAttemptOpen = errors.New("quizzes: the test attempt is still open") ) // ReviewFilter narrows the review queue. Zero values do not filter; Status @@ -287,6 +292,8 @@ func (r reviewScan) detail() *ReviewDetail { } // reviewWhere turns a filter into predicates appended to reviewFromSQL. +// The default open filter leaves out the answers of a test attempt that is +// still running: they cannot be reviewed until it ends (ErrAttemptOpen). func reviewWhere(f ReviewFilter, statuses []string) (string, []any) { var b strings.Builder var args []any @@ -294,6 +301,10 @@ func reviewWhere(f ReviewFilter, statuses []string) (string, []any) { b.WriteString(" AND aa.grading_status IN ?") args = append(args, statuses) } + if status := strings.TrimSpace(f.Status); status == "" || status == ReviewStatusOpen { + b.WriteString(" AND NOT (qz.mode = ? AND a.completed_at IS NULL)") + args = append(args, models.ModeTest) + } if slug := strings.TrimSpace(f.QuizSlug); slug != "" { b.WriteString(" AND qz.slug = ?") args = append(args, slug) @@ -369,7 +380,8 @@ type reviewTarget struct { // 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. +// not open, is ErrNotFound; an answer of a test attempt that is still +// running is ErrAttemptOpen. func lockReviewTarget(ctx context.Context, tx *gorm.DB, answerID uint) (*reviewTarget, error) { tx = tx.WithContext(ctx) var row models.AttemptAnswer @@ -388,6 +400,9 @@ func lockReviewTarget(ctx context.Context, tx *gorm.DB, answerID uint) (*reviewT return nil, notFound(err) } attempt.Quiz = &quiz + if Withheld(attempt) { + return nil, ErrAttemptOpen + } var question models.Question if err := tx.Where("id = ?", row.QuestionID).Take(&question).Error; err != nil { return nil, notFound(err) @@ -583,9 +598,10 @@ func (s *Service) SetScore(ctx context.Context, answerID uint, points float64, c // 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. +// reviewed fire their events after the commit. Answers of a test attempt +// that is still running are skipped; a scope of one such attempt is +// ErrAttemptOpen. 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} @@ -607,6 +623,12 @@ func (s *Service) ConfirmBulk(ctx context.Context, scope ReviewScope, by Reviewe } touched := map[uint]bool{} for _, row := range rows { + if Withheld(attempts[row.AttemptID]) { + if scope.AttemptID != 0 { + return ErrAttemptOpen + } + continue + } t := &reviewTarget{Answer: row, Question: questions[row.QuestionID], Attempt: attempts[row.AttemptID]} done, err := s.confirmTarget(ctx, tx, t, by) if err != nil { diff --git a/controllers/api/reviews.go b/controllers/api/reviews.go index 7a158bf..e250837 100644 --- a/controllers/api/reviews.go +++ b/controllers/api/reviews.go @@ -33,6 +33,8 @@ func writeReview(w http.ResponseWriter, status int, data any, err error) { 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.ErrAttemptOpen): + reviewConflict(w, "The test attempt is still open; review its answers once it ends.", "attempt_open") 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: diff --git a/controllers/reviews.go b/controllers/reviews.go index 4a1cb5f..a400b0f 100644 --- a/controllers/reviews.go +++ b/controllers/reviews.go @@ -245,6 +245,8 @@ func reviewError(err error) error { 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.ErrAttemptOpen): + return &cabana.ForbiddenError{Message: reviewsLang + "attempt_open"} case errors.Is(err, classes.ErrNotFound): return &cabana.ForbiddenError{Message: reviewsLang + "not_found"} } diff --git a/lang/en/lang.yaml b/lang/en/lang.yaml index 7aee6a0..f03319a 100644 --- a/lang/en/lang.yaml +++ b/lang/en/lang.yaml @@ -128,5 +128,6 @@ reviews: no_grader_score: This answer has no grader's score yet; enter the points yourself. already_reviewed: This answer is already verified. grader_manual: This quiz is graded manually; nothing can grade the answer again. + attempt_open: The test is still in progress; review this answer once it ends. not_found: This answer was not found. not_created: Open answers come from students; they cannot be added or deleted here. diff --git a/lang/pl/lang.yaml b/lang/pl/lang.yaml index d9d20dd..a467153 100644 --- a/lang/pl/lang.yaml +++ b/lang/pl/lang.yaml @@ -128,5 +128,6 @@ reviews: no_grader_score: Ta odpowiedź nie ma jeszcze wyniku oceniającego; wpisz punkty sam. already_reviewed: Ta odpowiedź jest już zweryfikowana. grader_manual: Ten quiz jest oceniany ręcznie; nie ma kto ocenić odpowiedzi ponownie. + attempt_open: Test wciąż trwa; oceń tę odpowiedź po jego zakończeniu. not_found: Nie znaleziono tej odpowiedzi. not_created: Odpowiedzi otwarte pochodzą od studentów; tutaj nie można ich dodawać ani usuwać. diff --git a/mcp_reviews.go b/mcp_reviews.go index 83d2ca8..babd342 100644 --- a/mcp_reviews.go +++ b/mcp_reviews.go @@ -49,7 +49,7 @@ func reviewTools() []pact.MCPTool { { Name: "review_list", Title: "List open answers to review", - Description: "Lists the answers students wrote to open questions, oldest change first, for you to review. status: open (default: pending, graded and failed, everything not reviewed yet), pending (waiting for the grader or for you), graded (the grader gave a score you have not confirmed), failed (the grader could not score it; set the score yourself), reviewed or all. Filters: quiz (slug), user_id, attempt_id; page and per_page (1-100, default 20). Each row: answer_id, status, quiz {id, slug, title, mode}, question {id, text, points}, attempt {id, completed, completed_at}, student {user_id, email, name, surname}, text (the first 300 characters, text_truncated when cut), grader_points, points, graded_at, reviewed_at, updated_at. Read one answer in full with quizzes_review_show. It shows student emails, names and answers." + needsRead, + Description: "Lists the answers students wrote to open questions, oldest change first, for you to review. status: open (default: pending, graded and failed, everything not reviewed yet, except answers of a test attempt still in progress), pending (waiting for the grader or for you), graded (the grader gave a score you have not confirmed), failed (the grader could not score it; set the score yourself), reviewed or all. Filters: quiz (slug), user_id, attempt_id; page and per_page (1-100, default 20). Each row: answer_id, status, quiz {id, slug, title, mode}, question {id, text, points}, attempt {id, completed, completed_at}, student {user_id, email, name, surname}, text (the first 300 characters, text_truncated when cut), grader_points, points, graded_at, reviewed_at, updated_at. Read one answer in full with quizzes_review_show. It shows student emails, names and answers." + needsRead, Method: http.MethodGet, Path: ReviewPrefix, Input: reviewListInput{}, @@ -67,7 +67,7 @@ func reviewTools() []pact.MCPTool { { Name: "review_confirm", Title: "Confirm the grader's score", - Description: "Confirms the grader's score of one graded answer: status becomes reviewed and points become grader_points, final over any later grading, and a finished attempt's score is recalculated. A pending or failed answer has no score to confirm and is refused with 409 code no_grader_score (set the score with quizzes_review_set_score instead); confirming a reviewed answer changes nothing. Returns the answer as quizzes_review_show does." + needsWrite, + Description: "Confirms the grader's score of one graded answer: status becomes reviewed and points become grader_points, final over any later grading, and a finished attempt's score is recalculated. A pending or failed answer has no score to confirm and is refused with 409 code no_grader_score (set the score with quizzes_review_set_score instead); confirming a reviewed answer changes nothing. An answer of a test attempt still in progress is refused with 409 code attempt_open. Returns the answer as quizzes_review_show does." + needsWrite, Method: http.MethodPost, Path: ReviewPrefix + "/{id}/confirm", Input: reviewAnswerInput{}, @@ -77,7 +77,7 @@ func reviewTools() []pact.MCPTool { { 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, + 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. An answer of a test attempt still in progress is refused with 409 code attempt_open. Returns the answer as quizzes_review_show does." + needsWrite, Method: http.MethodPost, Path: ReviewPrefix + "/{id}/score", Input: reviewSetScoreInput{}, @@ -87,7 +87,7 @@ func reviewTools() []pact.MCPTool { { 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, + 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. Answers of test attempts still in progress are skipped (an attempt_id of one is refused with 409 code attempt_open). 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{},