diff --git a/app/controllers/course/admin/assessment_settings_controller.rb b/app/controllers/course/admin/assessment_settings_controller.rb index b73077f6508..e87420aa0d5 100644 --- a/app/controllers/course/admin/assessment_settings_controller.rb +++ b/app/controllers/course/admin/assessment_settings_controller.rb @@ -81,6 +81,7 @@ def category_params # :allow_randomization, :allow_mrq_options_randomization, :programming_max_time_limit, + :rubric_grading_feedback_workflow, :rubric_grading_prompt_enabled, :rubric_grading_prompt, assessment_categories_attributes: [ diff --git a/app/jobs/course/rubric/apply_evaluations_job.rb b/app/jobs/course/rubric/apply_evaluations_job.rb index 9a41510bf9f..570c30c14c5 100644 --- a/app/jobs/course/rubric/apply_evaluations_job.rb +++ b/app/jobs/course/rubric/apply_evaluations_job.rb @@ -23,7 +23,10 @@ def apply(rubric, answer) grading = Course::Rubric::GradingEvaluationMirrorService.mirror(answer, evaluation) grade = Course::Rubric::GradingEvaluationMirrorService.total_grade(grading, answer.question.maximum_grade) answer.update_column(:grade, grade) - Course::Assessment::Answer::AiGeneratedPostService.new(answer, grading.feedback).create_ai_generated_draft_post + # force_draft: applying from the playground is a staff action over a whole class of answers, so it + # drafts even where the course publishes student-triggered feedback automatically. + Course::Assessment::Answer::AiGeneratedPostService. + new(answer, grading.feedback, force_draft: true).create_ai_generated_draft_post end # Runs the LLM once for an answer with no (visible) playground evaluation for this rubric, populating a diff --git a/app/models/concerns/course/assessment/questions_concern.rb b/app/models/concerns/course/assessment/questions_concern.rb index 8a26cd7a298..5a673dbd327 100644 --- a/app/models/concerns/course/assessment/questions_concern.rb +++ b/app/models/concerns/course/assessment/questions_concern.rb @@ -75,7 +75,13 @@ def correctly_answered_questions(submission) where(id: correctly_answered_question_ids(submission)) end + # A rubric-graded question counts as answered once the student has submitted it at least once: rubric grading + # has no notion of a wrong answer, and a student should not be held on a step by how long grading takes, or by + # whether it succeeds at all. def correctly_answered_question_ids(submission) - submission.answers.where(correct: true).select(:question_id) + submitted_rubric_answers = submission.answers.where(question_id: where(grading_mode: :rubric).select(:id)). + where.not(workflow_state: :attempting) + + submission.answers.where(correct: true).or(submitted_rubric_answers).select(:question_id) end end diff --git a/app/models/course.rb b/app/models/course.rb index f8e5faf6cdf..ba0db49745a 100644 --- a/app/models/course.rb +++ b/app/models/course.rb @@ -40,6 +40,7 @@ class Course < ApplicationRecord # rubocop:disable Metrics/ClassLength validates :conditional_satisfiability_evaluation_time, presence: true validates :ssid_folder_id, uniqueness: { if: :ssid_folder_id_changed? }, allow_nil: true validate :validate_rubric_grading_model + validate :validate_rubric_grading_feedback_workflow validate :validate_rubric_grading_model_options validate :validate_rubric_grading_system_prompt @@ -290,6 +291,18 @@ def rubric_grading_prompt=(prompt) settings(:course_assessments_component).rubric_grading_prompt = prompt.presence end + # How generated rubric feedback reaches students -- drafted for staff approval (the default), published + # to the student directly, or not generated at all. See + # Course::Assessment::Answer::AiGeneratedPostService::FEEDBACK_WORKFLOWS for the modes. + def rubric_grading_feedback_workflow + settings(:course_assessments_component).rubric_grading_feedback_workflow.presence || + Course::Assessment::Answer::AiGeneratedPostService::DEFAULT_FEEDBACK_WORKFLOW + end + + def rubric_grading_feedback_workflow=(workflow) + settings(:course_assessments_component).rubric_grading_feedback_workflow = workflow.presence + end + # The model rubric grading runs on, blank meaning the LlmAdapter's DEFAULT_MODEL. Admin-only (see # Course::AssessmentsAbilityComponent), as are the two settings below. def rubric_grading_model @@ -481,6 +494,14 @@ def nearest_forum_discussions(query_embedding, limit: 3) private + def validate_rubric_grading_feedback_workflow + workflow = settings(:course_assessments_component).rubric_grading_feedback_workflow + return if workflow.blank? + return if Course::Assessment::Answer::AiGeneratedPostService::FEEDBACK_WORKFLOWS.include?(workflow) + + errors.add(:rubric_grading_feedback_workflow, "is not a supported feedback workflow: #{workflow}") + end + def validate_rubric_grading_model return if rubric_grading_model.blank? return if Course::Rubric::LlmService::LlmAdapter.available_models.include?(rubric_grading_model) diff --git a/app/models/course/assessment/submission.rb b/app/models/course/assessment/submission.rb index 79d0da66f78..cd657a83205 100644 --- a/app/models/course/assessment/submission.rb +++ b/app/models/course/assessment/submission.rb @@ -22,6 +22,7 @@ class Course::Assessment::Submission < ApplicationRecord after_save :auto_grade_submission, if: :submitted? after_save :retrieve_codaveri_feedback, if: :submitted? + after_save :publish_ai_generated_feedback, if: :submitted? after_create :schedule_force_submission, if: :attempting? workflow do @@ -447,6 +448,31 @@ def auto_grade_submission end end + # Releases the AI rubric feedback drafted while this submission was being attempted, for courses that hold + # it back until the student finalises (see AiGeneratedPostService::PUBLISH_ON_FINALISE). Comments generated + # *by* finalising are published as they are created -- this callback runs before those grading jobs, so it + # only ever sees the earlier drafts. + def publish_ai_generated_feedback + return unless saved_change_to_workflow_state? + return unless assessment.course.rubric_grading_feedback_workflow == + Course::Assessment::Answer::AiGeneratedPostService::PUBLISH_ON_FINALISE + + ActiveRecord.after_all_transactions_commit do + drafts = Course::Discussion::Post.where(topic: submission_questions.map(&:discussion_topic)). + where(is_ai_generated: true, workflow_state: 'draft').to_a + # One at a time with update! -- not the workflow event, and not update_all -- so each post's own callbacks + # fire, which snapshot the final text into the feedback rating. + drafts.each { |post| post.update!(workflow_state: 'published') } + + # Nobody is waiting to approve them any more, so their topics leave the staff pending queues. Topic has no + # save callbacks, so one statement is equivalent to unmark_as_pending on each -- including the updated_at + # bump, which read tracking keys off (acts_as_readable on: :updated_at). Only these drafts' topics: one + # whose comment staff accepted earlier may be pending again because the student has since replied. + Course::Discussion::Topic.where(id: drafts.map(&:topic_id), pending_staff_reply: true). + update_all(pending_staff_reply: false, updated_at: Time.current) + end + end + # Retrieve codaveri feedback only for current answers of codaveri programming question type # for finalised submissions. def retrieve_codaveri_feedback diff --git a/app/services/course/assessment/answer/ai_generated_post_service.rb b/app/services/course/assessment/answer/ai_generated_post_service.rb index 210524a5721..42b2d3ac1f2 100644 --- a/app/services/course/assessment/answer/ai_generated_post_service.rb +++ b/app/services/course/assessment/answer/ai_generated_post_service.rb @@ -1,16 +1,41 @@ # frozen_string_literal: true class Course::Assessment::Answer::AiGeneratedPostService + # How a course delivers generated rubric feedback to its students (Course#rubric_grading_feedback_workflow, + # configured under Course Settings > Assessments). Mirrors the Codaveri component's feedback_workflow, with + # the publish case split in two: rubric answers are graded both when a student submits a single answer and + # when they finalise the submission. + # + # * +none+ -- grade the answer, but draft no feedback comment at all. + # * +draft+ -- staff accept or reject the comment before the student sees it (default). + # * +publish_on_answer_submit+ -- the student sees the comment as soon as the answer is graded. + # * +publish_on_finalise+ -- drafted while the submission is being attempted, published (see + # Course::Assessment::Submission#publish_ai_generated_feedback) once it + # is finalised. + NO_FEEDBACK = 'none' + DRAFT_FEEDBACK = 'draft' + PUBLISH_ON_ANSWER_SUBMIT = 'publish_on_answer_submit' + PUBLISH_ON_FINALISE = 'publish_on_finalise' + FEEDBACK_WORKFLOWS = [NO_FEEDBACK, DRAFT_FEEDBACK, PUBLISH_ON_ANSWER_SUBMIT, PUBLISH_ON_FINALISE].freeze + DEFAULT_FEEDBACK_WORKFLOW = DRAFT_FEEDBACK + # @param [Course::Assessment::Answer] answer The answer to create/update the post for # @param [String] feedback The feedback text to include in the post - def initialize(answer, content) + # @param [Boolean] force_draft Whether to draft the comment regardless of the course's workflow. Set by + # staff-initiated generation (applying playground evaluations), which may publish to a whole class at + # once and so is never delivered to students without a person deciding to. + def initialize(answer, content, force_draft: false) @answer = answer @content = content + @force_draft = force_draft end - # Creates or updates AI-generated draft feedback post for the answer + # Creates or updates the AI-generated feedback post for the answer, drafted or published according to the + # course's feedback workflow. # @return [void] def create_ai_generated_draft_post + return if feedback_workflow == NO_FEEDBACK + submission_question = @answer.submission.submission_questions.find_by(question_id: @answer.question_id) return unless submission_question @@ -26,6 +51,26 @@ def create_ai_generated_draft_post private + # @return [String] one of FEEDBACK_WORKFLOWS + def feedback_workflow + return DRAFT_FEEDBACK if @force_draft + + @feedback_workflow ||= @answer.submission.assessment.course.rubric_grading_feedback_workflow + end + + # Whether this comment reaches the student without a staff decision. +publish_on_finalise+ waits: a + # comment drafted mid-attempt stays a draft, and the submission publishes it when it is finalised, while + # a comment generated after that point (finalising triggers grading of the remaining answers) is published + # straight away. + # @return [Boolean] + def publish_immediately? + case feedback_workflow + when PUBLISH_ON_ANSWER_SUBMIT then true + when PUBLISH_ON_FINALISE then !@answer.submission.attempting? + else false + end + end + # Builds a draft post with AI-generated feedback # @param [Course::Assessment::SubmissionQuestion] submission_question The submission question # @return [Course::Discussion::Post] The built post @@ -35,7 +80,7 @@ def build_draft_post(submission_question) updater: User.system, text: @content, is_ai_generated: true, - workflow_state: 'draft', + workflow_state: publish_immediately? ? 'published' : 'draft', title: @answer.submission.assessment.title ) end @@ -51,9 +96,12 @@ def save_draft_post(submission_question, post) end post.save! submission_question.save! + # The rating is initialized whichever way the comment is delivered, so the generated text is always + # snapshotted. A published comment carries no rating UI today (the card is draft-only), so its rating + # stays unrated -- the same trade the Codaveri publish workflow makes. initialize_rating(post) create_topic_subscription(post.topic) - post.topic.mark_as_pending + post.topic.mark_as_pending unless post.published? end end @@ -128,7 +176,9 @@ def create_topic_subscription(discussion_topic) end end - # Finds the latest AI-generated draft post for the submission question + # Finds the latest AI-generated draft post for the submission question. Only drafts are updated in place, + # so under a publish workflow a re-graded answer gains a new comment instead of having one the student may + # already have read rewritten underneath them. # @param [Course::Assessment::SubmissionQuestion] submission_question The submission question # @return [Course::Discussion::Post, nil] The latest AI-generated draft post or nil if none exists def find_existing_ai_draft_post(submission_question) diff --git a/app/views/course/admin/assessment_settings/edit.json.jbuilder b/app/views/course/admin/assessment_settings/edit.json.jbuilder index 07bde6d3b03..93fca3f2c73 100644 --- a/app/views/course/admin/assessment_settings/edit.json.jbuilder +++ b/app/views/course/admin/assessment_settings/edit.json.jbuilder @@ -6,6 +6,9 @@ json.showStdoutAndStderr current_course.show_stdout_and_stderr || false json.allowRandomization current_course.allow_randomization || false json.allowMrqOptionsRandomization current_course.allow_mrq_options_randomization || false json.maxProgrammingTimeLimit current_course.programming_max_time_limit if can?(:manage, :all) +# How generated rubric feedback reaches students. Unlike the model configuration below, this is a teaching +# decision, so any course manager may change it. +json.rubricGradingFeedbackWorkflow current_course.rubric_grading_feedback_workflow json.rubricGradingPromptEnabled current_course.rubric_grading_prompt_enabled json.rubricGradingPrompt current_course.rubric_grading_prompt || '' diff --git a/app/views/course/assessment/answer/_ai_generated_comment.json.jbuilder b/app/views/course/assessment/answer/_ai_generated_comment.json.jbuilder new file mode 100644 index 00000000000..0b71ea4604a --- /dev/null +++ b/app/views/course/assessment/answer/_ai_generated_comment.json.jbuilder @@ -0,0 +1,15 @@ +# frozen_string_literal: true +# The latest AI feedback comment on this answer's question, so the client can show it as soon as grading +# finishes rather than on the next full page load. Which state that is depends on the course's feedback +# workflow: a draft for staff to review, or already published to the student. Visibility follows the +# submission's own comment rules (see _topics.json.jbuilder): staff see every state, students only what has +# been published. +posts = answer.submission.submission_questions.find_by(question_id: answer.question_id)&.discussion_topic&.posts +ai_generated_comment = posts&.select do |post| + post.is_ai_generated && (can_grade || post.published?) +end&.last +if ai_generated_comment + json.aiGeneratedComment do + json.partial! ai_generated_comment + end +end diff --git a/app/views/course/assessment/answer/_rubric_explanation.json.jbuilder b/app/views/course/assessment/answer/_rubric_explanation.json.jbuilder new file mode 100644 index 00000000000..238b1f0aa0c --- /dev/null +++ b/app/views/course/assessment/answer/_rubric_explanation.json.jbuilder @@ -0,0 +1,13 @@ +# frozen_string_literal: true +# The step-by-step gate for a rubric-graded answer: the Continue and Finalise buttons read +correct+, as does the +# server's maxStep (see Course::Assessment::QuestionsConcern#correctly_answered_question_ids). Rubric grading has +# no notion of a wrong answer, so +correct+ means "submitted at least once" -- the student may move on as soon as +# they have submitted, however long grading takes or whether it succeeds. Null until then, which hides the panel. +# +# Unlike other answer types this is not gated on can_read_grade?: it carries no grade, only whether the student +# has submitted, which they already know. +submitted = last_attempt.present? && !last_attempt.attempting? +json.explanation do + json.correct submitted ? true : nil + json.explanations [] +end diff --git a/app/views/course/assessment/answer/forum_post_responses/_forum_post_response.json.jbuilder b/app/views/course/assessment/answer/forum_post_responses/_forum_post_response.json.jbuilder index 27cd08bfac0..c05fdea08f0 100644 --- a/app/views/course/assessment/answer/forum_post_responses/_forum_post_response.json.jbuilder +++ b/app/views/course/assessment/answer/forum_post_responses/_forum_post_response.json.jbuilder @@ -19,9 +19,9 @@ last_attempt = last_attempt(answer) # the plain grade field, so emitting an (empty) breakdown would misroute their save through the rubric path. if answer.question.grading_mode_rubric? json.partial! 'course/assessment/answer/rubric_category_grades', answer: answer, can_grade: can_grade -end - -if answer.can_read_grade?(current_ability) + json.partial! 'course/assessment/answer/ai_generated_comment', answer: answer, can_grade: can_grade + json.partial! 'course/assessment/answer/rubric_explanation', last_attempt: last_attempt +elsif answer.can_read_grade?(current_ability) json.explanation do json.correct last_attempt&.correct json.explanations [] diff --git a/app/views/course/assessment/answer/rubric_based_responses/_rubric_based_response.json.jbuilder b/app/views/course/assessment/answer/rubric_based_responses/_rubric_based_response.json.jbuilder index 27cdc2e15d9..b7dd472eda0 100644 --- a/app/views/course/assessment/answer/rubric_based_responses/_rubric_based_response.json.jbuilder +++ b/app/views/course/assessment/answer/rubric_based_responses/_rubric_based_response.json.jbuilder @@ -32,24 +32,8 @@ end json.partial! 'course/assessment/answer/rubric_category_grades', answer: answer, can_grade: can_grade -if can_grade - posts = answer.submission.submission_questions.find_by(question_id: answer.question_id)&.discussion_topic&.posts - ai_generated_comment = posts&.select do |post| - post.is_ai_generated && post.workflow_state == 'draft' - end&.last - if ai_generated_comment - json.aiGeneratedComment do - json.partial! ai_generated_comment - end - end -end - -if answer.can_read_grade?(current_ability) - json.explanation do - json.correct last_attempt&.correct - json.explanations [] - end -end +json.partial! 'course/assessment/answer/ai_generated_comment', answer: answer, can_grade: can_grade +json.partial! 'course/assessment/answer/rubric_explanation', last_attempt: last_attempt if answer.current_answer? && !last_attempt.current_answer? json.latestAnswer do diff --git a/app/views/course/assessment/question/rubric_based_responses/_rubric_based_response.json.jbuilder b/app/views/course/assessment/question/rubric_based_responses/_rubric_based_response.json.jbuilder index 03f9e4cea42..cf3facaf6ff 100644 --- a/app/views/course/assessment/question/rubric_based_responses/_rubric_based_response.json.jbuilder +++ b/app/views/course/assessment/question/rubric_based_responses/_rubric_based_response.json.jbuilder @@ -1,7 +1,7 @@ # frozen_string_literal: true json.aiGradingEnabled question.ai_grading_enabled? if can_grade -json.autogradable false +json.autogradable question.auto_gradable? json.templateText question.template_text json.partial! 'course/assessment/question/rubric_categories', diff --git a/app/views/course/assessment/submission/submissions/_question.json.jbuilder b/app/views/course/assessment/submission/submissions/_question.json.jbuilder index 01c3f5f18e6..b39bb5933aa 100644 --- a/app/views/course/assessment/submission/submissions/_question.json.jbuilder +++ b/app/views/course/assessment/submission/submissions/_question.json.jbuilder @@ -12,5 +12,7 @@ end json.canViewHistory question.history_viewable? json.type question.question_type +# 'rubric' questions have no notion of a wrong answer, which the client needs to word the answer panel. +json.gradingMode question.grading_mode json.partial! question, question: question.specific, can_grade: can_grade, answer: answer diff --git a/app/views/course/discussion/posts/_post.json.jbuilder b/app/views/course/discussion/posts/_post.json.jbuilder index d27cd743f4b..34c2bf2f22e 100644 --- a/app/views/course/discussion/posts/_post.json.jbuilder +++ b/app/views/course/discussion/posts/_post.json.jbuilder @@ -36,7 +36,10 @@ end # Lazily backfills the rating for pre-feature (or straggler) AI drafts so they render with the rateable card. post.ensure_generated_rating! if post.is_ai_generated && post.workflow_state == 'draft' ai_feedback_rating = post.is_ai_generated ? post.ai_feedback_rating : nil -if ai_feedback_rating +# Staff-only. The rating holds the AI's original text (which staff may have edited before publishing), the edited +# text, and the staff member's score -- none of which is for the student the comment is addressed to. Gated on the +# ability the rating endpoint authorizes, so exactly the users who can rate the comment receive it. +if ai_feedback_rating && can?(:update, ai_feedback_rating) json.generatedRating do # `type` discriminates which rating endpoint the client should call (see RateableGeneratedCommentCard). json.type 'rubric_feedback' diff --git a/app/views/course/forum/posts/_post_list_data.json.jbuilder b/app/views/course/forum/posts/_post_list_data.json.jbuilder index 6d75ccd802b..a2dd9a01d52 100644 --- a/app/views/course/forum/posts/_post_list_data.json.jbuilder +++ b/app/views/course/forum/posts/_post_list_data.json.jbuilder @@ -17,7 +17,10 @@ json.isAiGenerated post.is_ai_generated # Lazily backfills the rating for pre-feature (or straggler) AI drafts so they render with the rateable card. post.ensure_generated_rating! if post.is_ai_generated && post.workflow_state == 'draft' rag_wise_rating = post.is_ai_generated ? post.rag_wise_rating : nil -if rag_wise_rating +# Staff-only. The rating holds the AI's original answer (which staff may have edited before publishing), the edited +# text, the staff member's score and the answer's quality scores -- none of which is for the students reading the +# forum. Gated on the ability the rating endpoint authorizes, so exactly the users who can rate it receive it. +if rag_wise_rating && can?(:update, rag_wise_rating) json.generatedRating do # `type` discriminates which rating endpoint the client should call (see RateableGeneratedCommentCard). json.type 'rag_wise' diff --git a/client/app/bundles/course/admin/pages/AssessmentSettings/AssessmentSettingsForm.tsx b/client/app/bundles/course/admin/pages/AssessmentSettings/AssessmentSettingsForm.tsx index 34eb97204ad..76a202a7d8e 100644 --- a/client/app/bundles/course/admin/pages/AssessmentSettings/AssessmentSettingsForm.tsx +++ b/client/app/bundles/course/admin/pages/AssessmentSettings/AssessmentSettingsForm.tsx @@ -1,9 +1,10 @@ import { forwardRef } from 'react'; import { Controller } from 'react-hook-form'; -import { InputAdornment, Typography } from '@mui/material'; +import { InputAdornment, RadioGroup, Typography } from '@mui/material'; import { AssessmentSettingsData } from 'types/course/admin/assessments'; import * as yup from 'yup'; +import RadioButton from 'lib/components/core/buttons/RadioButton'; import Section from 'lib/components/core/layouts/Section'; import Subsection from 'lib/components/core/layouts/Subsection'; import FormCheckboxField from 'lib/components/form/fields/CheckboxField'; @@ -321,6 +322,49 @@ const AssessmentsSettingsForm = forwardRef< /> )} + + ( + + + + + + + + + + )} + /> + ; readOnly: boolean; saveAnswerAndUpdateClientVersion: (answerId: number) => void; } @@ -15,8 +13,7 @@ interface RubricBasedResponseAnswerProps { const RubricBasedResponseAnswer: FC = ( props, ) => { - const { question, answerId, readOnly, saveAnswerAndUpdateClientVersion } = - props; + const { answerId, readOnly, saveAnswerAndUpdateClientVersion } = props; const { control } = useFormContext(); @@ -48,7 +45,7 @@ const RubricBasedResponseAnswer: FC = ( shrink: true, }} multiline - renderIf={!readOnly && !question.autogradable} + renderIf={!readOnly} variant="standard" /> )} diff --git a/client/app/bundles/course/assessment/submission/components/answers/adapters/RubricBasedResponseAdapter.tsx b/client/app/bundles/course/assessment/submission/components/answers/adapters/RubricBasedResponseAdapter.tsx index 82a9b2b4549..a928c987586 100644 --- a/client/app/bundles/course/assessment/submission/components/answers/adapters/RubricBasedResponseAdapter.tsx +++ b/client/app/bundles/course/assessment/submission/components/answers/adapters/RubricBasedResponseAdapter.tsx @@ -10,7 +10,6 @@ const RubricBasedResponseAdapter = ( diff --git a/client/app/bundles/course/assessment/submission/components/comment/AiFeedbackCommentCard.tsx b/client/app/bundles/course/assessment/submission/components/comment/AiFeedbackCommentCard.tsx index 83a7395178b..e6904bbac35 100644 --- a/client/app/bundles/course/assessment/submission/components/comment/AiFeedbackCommentCard.tsx +++ b/client/app/bundles/course/assessment/submission/components/comment/AiFeedbackCommentCard.tsx @@ -1,18 +1,14 @@ import { FC } from 'react'; import { Avatar } from '@mui/material'; +import { CommentPostMiniEntity } from 'types/course/comments'; import RateableGeneratedCommentCard from 'lib/components/core/RateableGeneratedCommentCard'; -// The submission bundle types its posts with PropTypes (see ../../propTypes#postShape); this captures just -// the fields the card reads. -interface AiFeedbackPost { - id: number; - text: string; - canUpdate: boolean; - createdAt: string; - creator?: { imageUrl?: string }; - generatedRating?: { rating: number | null } | null; -} +// The fields of a submission post (see the posts slice) that the card reads. +type AiFeedbackPost = Pick< + CommentPostMiniEntity, + 'id' | 'text' | 'canUpdate' | 'createdAt' | 'creator' | 'generatedRating' +>; interface Props { post: AiFeedbackPost; diff --git a/client/app/bundles/course/assessment/submission/components/comment/CommentCard.jsx b/client/app/bundles/course/assessment/submission/components/comment/CommentCard.jsx deleted file mode 100644 index dd473f7a613..00000000000 --- a/client/app/bundles/course/assessment/submission/components/comment/CommentCard.jsx +++ /dev/null @@ -1,274 +0,0 @@ -import { Component } from 'react'; -import { defineMessages, FormattedMessage } from 'react-intl'; -import { CheckCircleOutline } from '@mui/icons-material'; -import { Avatar, Button, CardHeader, IconButton, Tooltip } from '@mui/material'; -import { grey, orange } from '@mui/material/colors'; -import PropTypes from 'prop-types'; - -import DeleteButton from 'lib/components/core/buttons/DeleteButton'; -import EditButton from 'lib/components/core/buttons/EditButton'; -import ConfirmationDialog from 'lib/components/core/dialogs/ConfirmationDialog'; -import CKEditorRichText from 'lib/components/core/fields/CKEditorRichText'; -import UserHTMLText from 'lib/components/core/UserHTMLText'; -import { POST_WORKFLOW_STATE } from 'lib/constants/sharedConstants'; -import { formatLongDateTime } from 'lib/moment'; - -import { postShape } from '../../propTypes'; - -const translations = defineMessages({ - deleteConfirmation: { - id: 'course.assessment.submission.comment.CommentCard.deleteConfirmation', - defaultMessage: 'Are you sure you want to delete this comment?', - }, - cancel: { - id: 'course.assessment.submission.comment.CommentCard.cancel', - defaultMessage: 'Cancel', - }, - save: { - id: 'course.assessment.submission.comment.CommentCard.save', - defaultMessage: 'Save', - }, - publish: { - id: 'course.assessment.submission.comment.CommentCard.publish', - defaultMessage: 'Publish', - }, - isAiGenerated: { - id: 'course.assessment.submission.comment.CommentCard.isAiGenerated', - defaultMessage: 'AI Generated Comment', - }, -}); - -const styles = { - avatar: { - height: '25px', - width: '25px', - }, - card: { - marginBottom: 10, - borderStyle: 'solid', - borderWidth: 0.2, - borderColor: grey[400], - borderRadius: 3, - }, - header: { - display: 'flex', - alignItems: 'center', - justifyContent: 'space-between', - backgroundColor: grey[100], - borderRadius: 3, - }, - delayedHeader: { - display: 'flex', - alignItems: 'center', - justifyContent: 'space-between', - backgroundColor: orange[100], - borderRadius: 3, - }, - cardHeader: { - padding: 6, - }, - buttonContainer: { - display: 'flex', - marginRight: 5, - marginBottom: 2, - }, - headerButtonHidden: { - height: 35, - width: 40, - minWidth: 40, - }, - commentContent: { - wordWrap: 'break-word', - padding: 7, - }, -}; - -export default class CommentCard extends Component { - static editPostIdentifier(field) { - return `edit_post_${field}`; - } - - static postIdentifier(field) { - return `post_${field}`; - } - - constructor(props) { - super(props); - this.state = { - editMode: false, - deleteConfirmation: false, - }; - } - - onChange(nextValue) { - const { handleChange } = this.props; - handleChange(nextValue); - } - - onConfirmDelete() { - const { deleteComment } = this.props; - deleteComment(); - this.setState({ deleteConfirmation: false }); - } - - onDelete() { - this.setState({ deleteConfirmation: true }); - } - - onSave() { - const { editValue } = this.props; - this.props.updateComment(editValue); - this.setState({ editMode: false }); - } - - onPublish() { - const { editValue } = this.props; - this.props.publishComment(editValue); - this.setState({ editMode: false }); - } - - toggleEditMode() { - const { editMode } = this.state; - const { - handleChange, - post: { text }, - } = this.props; - this.setState({ editMode: !editMode }); - handleChange(text); - } - - renderCommentContent() { - const { editMode } = this.state; - const { - editValue, - post: { text, id, workflowState }, - } = this.props; - - const isDraft = workflowState === POST_WORKFLOW_STATE.draft; - - if (editMode) { - return ( - <> - this.onChange(nextValue)} - value={editValue} - /> -
- - {isDraft ? ( - - ) : ( - - )} -
- - ); - } - - return ; - } - - render() { - const { - creator: { name, imageUrl }, - createdAt, - canUpdate, - canDestroy, - id, - isDelayed, - isAiGenerated, - } = this.props.post; - - const { post, isUpdatingAnnotationAllowed, editValue, publishComment } = - this.props; - - const isDraft = post.workflowState === POST_WORKFLOW_STATE.draft; - - return ( -
-
- - ) - } - style={styles.cardHeader} - subheader={`${formatLongDateTime(createdAt)}${ - isDelayed ? ' (delayed comment)' : '' - }`} - subheaderTypographyProps={{ display: 'block' }} - title={ - isAiGenerated && isDraft ? ( - - ) : ( - name - ) - } - titleTypographyProps={{ - display: 'block', - fontSize: '1.5rem', - }} - /> -
- {isDraft && ( - }> - publishComment(editValue)} - > - - - - )} - {canUpdate && isUpdatingAnnotationAllowed ? ( - this.toggleEditMode()} - /> - ) : null} - {canDestroy && isUpdatingAnnotationAllowed ? ( - this.onDelete()} - /> - ) : null} -
-
-
{this.renderCommentContent()}
- } - onCancel={() => this.setState({ deleteConfirmation: false })} - onConfirm={() => this.onConfirmDelete()} - open={this.state.deleteConfirmation} - /> -
- ); - } -} - -CommentCard.propTypes = { - post: postShape.isRequired, - editValue: PropTypes.string, - - handleChange: PropTypes.func, - updateComment: PropTypes.func, - deleteComment: PropTypes.func, - publishComment: PropTypes.func, - isUpdatingAnnotationAllowed: PropTypes.bool, -}; diff --git a/client/app/bundles/course/assessment/submission/components/comment/CommentCard.tsx b/client/app/bundles/course/assessment/submission/components/comment/CommentCard.tsx new file mode 100644 index 00000000000..a3b4f77be89 --- /dev/null +++ b/client/app/bundles/course/assessment/submission/components/comment/CommentCard.tsx @@ -0,0 +1,226 @@ +import { FC, useState } from 'react'; +import { defineMessages, FormattedMessage } from 'react-intl'; +import { CheckCircleOutline } from '@mui/icons-material'; +import { Avatar, Button, CardHeader, IconButton, Tooltip } from '@mui/material'; +import { grey, orange } from '@mui/material/colors'; +import { CommentPostMiniEntity } from 'types/course/comments'; + +import DeleteButton from 'lib/components/core/buttons/DeleteButton'; +import EditButton from 'lib/components/core/buttons/EditButton'; +import CKEditorRichText from 'lib/components/core/fields/CKEditorRichText'; +import UserHTMLText from 'lib/components/core/UserHTMLText'; +import { POST_WORKFLOW_STATE } from 'lib/constants/sharedConstants'; +import { formatLongDateTime } from 'lib/moment'; + +const translations = defineMessages({ + deleteConfirmation: { + id: 'course.assessment.submission.comment.CommentCard.deleteConfirmation', + defaultMessage: 'Are you sure you want to delete this comment?', + }, + cancel: { + id: 'course.assessment.submission.comment.CommentCard.cancel', + defaultMessage: 'Cancel', + }, + save: { + id: 'course.assessment.submission.comment.CommentCard.save', + defaultMessage: 'Save', + }, + publish: { + id: 'course.assessment.submission.comment.CommentCard.publish', + defaultMessage: 'Publish', + }, + isAiGenerated: { + id: 'course.assessment.submission.comment.CommentCard.isAiGenerated', + defaultMessage: 'AI Generated Comment', + }, +}); + +const styles = { + avatar: { + height: '25px', + width: '25px', + }, + card: { + marginBottom: 10, + borderStyle: 'solid', + borderWidth: 0.2, + borderColor: grey[400], + borderRadius: 3, + }, + header: { + display: 'flex', + alignItems: 'center', + justifyContent: 'space-between', + backgroundColor: grey[100], + borderRadius: 3, + }, + delayedHeader: { + display: 'flex', + alignItems: 'center', + justifyContent: 'space-between', + backgroundColor: orange[100], + borderRadius: 3, + }, + cardHeader: { + padding: 6, + }, + buttonContainer: { + display: 'flex', + marginRight: 5, + marginBottom: 2, + }, + commentContent: { + wordWrap: 'break-word', + padding: 7, + }, +} as const; + +// DOM ids the submission feature specs locate comments by. +const postIdentifier = (postId: number): string => `post_${postId}`; +const editPostIdentifier = (postId: number): string => `edit_post_${postId}`; + +interface Props { + post: CommentPostMiniEntity; + // The edit box's current text, held in the commentForms slice so it survives re-renders. + editValue?: string; + handleChange: (value: string) => void; + updateComment: (value?: string) => void; + deleteComment: () => void; + // Only question comments can be drafts awaiting publication; annotations do not pass this. + publishComment?: (value?: string) => void; + isUpdatingAnnotationAllowed?: boolean; +} + +const CommentCard: FC = (props) => { + const { + post, + editValue, + handleChange, + updateComment, + deleteComment, + publishComment, + isUpdatingAnnotationAllowed, + } = props; + const { + id, + text, + creator, + createdAt, + canUpdate, + canDestroy, + isDelayed, + isAiGenerated, + } = post; + + const [editMode, setEditMode] = useState(false); + + const isDraft = post.workflowState === POST_WORKFLOW_STATE.draft; + + const startEditing = (): void => { + setEditMode(true); + handleChange(text); + }; + + const save = (): void => { + updateComment(editValue); + setEditMode(false); + }; + + const publish = (): void => { + publishComment?.(editValue); + setEditMode(false); + }; + + const renderCommentContent = (): JSX.Element => { + if (!editMode) return ; + + return ( + <> + +
+ + {isDraft ? ( + + ) : ( + + )} +
+ + ); + }; + + return ( +
+
+ + ) + } + style={styles.cardHeader} + subheader={`${formatLongDateTime(createdAt)}${ + isDelayed ? ' (delayed comment)' : '' + }`} + subheaderTypographyProps={{ display: 'block' }} + title={ + isAiGenerated && isDraft ? ( + + ) : ( + creator.name + ) + } + titleTypographyProps={{ + display: 'block', + fontSize: '1.5rem', + }} + /> +
+ {isDraft && ( + }> + publishComment?.(editValue)} + > + + + + )} + {canUpdate && isUpdatingAnnotationAllowed ? ( + + ) : null} + {canDestroy && isUpdatingAnnotationAllowed ? ( + + } + disabled={false} + // Not awaited, so the prompt closes on confirmation as it always has, and cannot be confirmed twice + // while the request is in flight. + onClick={async (): Promise => deleteComment()} + /> + ) : null} +
+
+
{renderCommentContent()}
+
+ ); +}; + +export default CommentCard; diff --git a/client/app/bundles/course/assessment/submission/containers/Comments.jsx b/client/app/bundles/course/assessment/submission/containers/Comments.jsx deleted file mode 100644 index 413ba2e9f5d..00000000000 --- a/client/app/bundles/course/assessment/submission/containers/Comments.jsx +++ /dev/null @@ -1,181 +0,0 @@ -import { Component } from 'react'; -import { FormattedMessage } from 'react-intl'; -import { connect } from 'react-redux'; -import { Typography } from '@mui/material'; -import PropTypes from 'prop-types'; - -import { POST_WORKFLOW_STATE } from 'lib/constants/sharedConstants'; -import toast from 'lib/hooks/toast'; - -import * as commentActions from '../actions/comments'; -import AiFeedbackCommentCard from '../components/comment/AiFeedbackCommentCard'; -import CommentCard from '../components/comment/CommentCard'; -import CommentField from '../components/comment/CommentField'; -import { workflowStates } from '../constants'; -import { postShape, topicShape } from '../propTypes'; -import translations from '../translations'; - -class VisibleComments extends Component { - static newCommentIdentifier(field) { - return `topic_${field}`; - } - - render() { - const { - commentForms, - posts, - topic, - handleCreateChange, - handleUpdateChange, - createComment, - updateComment, - deleteComment, - publishComment, - rateComment, - acceptComment, - rejectComment, - graderView, - renderDelayedCommentButton, - } = this.props; - - return ( -
- - - - - {posts.map((post) => { - const isVisible = - graderView || - (!post.isDelayed && - post.workflowState !== POST_WORKFLOW_STATE.draft); - if (!isVisible) { - return null; - } - - // An AI-generated draft still awaiting a staff decision uses the rateable card (rate -> edit -> - // accept/reject) in place of the plain comment card + Publish button. - if ( - post.isAiGenerated && - post.workflowState === POST_WORKFLOW_STATE.draft && - post.generatedRating - ) { - return ( - // Key on createdAt so a re-generated draft (fresh timestamp) remounts with the new content, - // resetting the card's local edit state. - acceptComment(post.id, value)} - deleteComment={() => deleteComment(post.id)} - post={post} - rateComment={(rating) => rateComment(post.id, rating)} - rejectComment={(value) => rejectComment(post.id, value)} - /> - ); - } - - return ( - deleteComment(post.id)} - editValue={commentForms.posts[post.id]} - handleChange={(value) => handleUpdateChange(post.id, value)} - isUpdatingAnnotationAllowed - post={post} - publishComment={(value) => publishComment(post.id, value)} - updateComment={(value) => updateComment(post.id, value)} - /> - ); - })} - - -
- ); - } -} - -VisibleComments.propTypes = { - commentForms: PropTypes.shape({ - topics: PropTypes.objectOf(PropTypes.string), - posts: PropTypes.objectOf(PropTypes.string), - isSubmittingNormalComment: PropTypes.bool, - isSubmittingDelayedComment: PropTypes.bool, - isUpdatingComment: PropTypes.bool, - }), - posts: PropTypes.arrayOf(postShape), - topic: topicShape, - graderView: PropTypes.bool.isRequired, - renderDelayedCommentButton: PropTypes.bool, - - handleCreateChange: PropTypes.func.isRequired, - handleUpdateChange: PropTypes.func.isRequired, - createComment: PropTypes.func.isRequired, - updateComment: PropTypes.func.isRequired, - deleteComment: PropTypes.func.isRequired, - publishComment: PropTypes.func.isRequired, - rateComment: PropTypes.func.isRequired, - acceptComment: PropTypes.func.isRequired, - rejectComment: PropTypes.func.isRequired, -}; - -function mapStateToProps({ assessments: { submission } }, ownProps) { - const { topic } = ownProps; - const renderDelayedCommentButton = - submission.submission.graderView && - !submission.assessment.autograded && - (submission.submission.workflowState === workflowStates.Submitted || - submission.submission.workflowState === workflowStates.Graded); - return { - commentForms: submission.commentForms, - posts: submission.topics[topic.id].postIds.map( - (postId) => submission.posts[postId], - ), - graderView: submission.submission.graderView, - renderDelayedCommentButton, - }; -} - -function mapDispatchToProps(dispatch, ownProps) { - const { topic } = ownProps; - - return { - handleCreateChange: (comment) => - dispatch(commentActions.onCreateChange(topic.id, comment)), - handleUpdateChange: (postId, comment) => - dispatch(commentActions.onUpdateChange(postId, comment)), - createComment: (comment, isDelayedComment = false) => - dispatch( - commentActions.create( - topic.submissionQuestionId, - comment, - isDelayedComment, - ), - ) - .then(() => toast.success('Successfully created comment.')) - .catch(() => toast.error('Failed to create comment.')), - updateComment: (postId, comment) => - dispatch(commentActions.update(topic.id, postId, comment)), - deleteComment: (postId) => - dispatch(commentActions.destroy(topic.id, postId)), - publishComment: (postId, comment) => - dispatch(commentActions.publish(topic.id, postId, comment)), - rateComment: (postId, rating) => - dispatch(commentActions.rateAiFeedback(topic.id, postId, rating)), - acceptComment: (postId, comment) => - dispatch(commentActions.acceptAiFeedback(topic.id, postId, comment)), - rejectComment: (postId, comment) => - dispatch(commentActions.rejectAiFeedback(topic.id, postId, comment)), - }; -} - -const Comments = connect(mapStateToProps, mapDispatchToProps)(VisibleComments); -export default Comments; diff --git a/client/app/bundles/course/assessment/submission/containers/Comments.tsx b/client/app/bundles/course/assessment/submission/containers/Comments.tsx new file mode 100644 index 00000000000..830b37ef6e1 --- /dev/null +++ b/client/app/bundles/course/assessment/submission/containers/Comments.tsx @@ -0,0 +1,155 @@ +import { FC } from 'react'; +import { FormattedMessage } from 'react-intl'; +import { Typography } from '@mui/material'; + +import { POST_WORKFLOW_STATE } from 'lib/constants/sharedConstants'; +import { useAppDispatch, useAppSelector } from 'lib/hooks/store'; +import toast from 'lib/hooks/toast'; + +import * as commentActions from '../actions/comments'; +import AiFeedbackCommentCard from '../components/comment/AiFeedbackCommentCard'; +import CommentCard from '../components/comment/CommentCard'; +import CommentField from '../components/comment/CommentField'; +import { workflowStates } from '../constants'; +import { getAssessment } from '../selectors/assessments'; +import { getCommentForms } from '../selectors/commentForms'; +import { getCommentPosts } from '../selectors/comments'; +import { getSubmission } from '../selectors/submissions'; +import { getTopics } from '../selectors/topics'; +import translations from '../translations'; +import { Topic } from '../types'; + +// DOM id of the new-comment box, which the submission feature specs locate it by. +const newCommentIdentifier = (topicId: number): string => `topic_${topicId}`; + +interface Props { + topic: Topic; +} + +// The comment thread under one submission question. +const Comments: FC = ({ topic }) => { + const dispatch = useAppDispatch(); + + const commentForms = useAppSelector(getCommentForms); + const allPosts = useAppSelector(getCommentPosts); + // Read by id rather than off the prop, so the thread follows the store as comments come and go. + const postIds = useAppSelector((state) => getTopics(state)[topic.id].postIds); + const graderView = useAppSelector((state) => getSubmission(state).graderView); + const workflowState = useAppSelector( + (state) => getSubmission(state).workflowState, + ); + const autograded = useAppSelector((state) => getAssessment(state).autograded); + + const posts = postIds.map((postId) => allPosts[postId]); + const renderDelayedCommentButton = + graderView && + !autograded && + (workflowState === workflowStates.Submitted || + workflowState === workflowStates.Graded); + + const createComment = ( + comment: string, + isDelayedComment = false, + ): Promise => + dispatch( + commentActions.create( + topic.submissionQuestionId, + comment, + isDelayedComment, + ), + ) + .then(() => { + toast.success('Successfully created comment.'); + }) + .catch(() => { + toast.error('Failed to create comment.'); + }); + + return ( +
+ + + + + {posts.map((post) => { + const isVisible = + graderView || + (!post.isDelayed && post.workflowState !== POST_WORKFLOW_STATE.draft); + if (!isVisible) { + return null; + } + + // An AI-generated draft still awaiting a staff decision uses the rateable card (rate -> edit -> + // accept/reject) in place of the plain comment card + Publish button. + if ( + post.isAiGenerated && + post.workflowState === POST_WORKFLOW_STATE.draft && + post.generatedRating + ) { + return ( + // Key on createdAt so a re-generated draft (fresh timestamp) remounts with the new content, + // resetting the card's local edit state. + + dispatch( + commentActions.acceptAiFeedback(topic.id, post.id, value), + ) + } + deleteComment={() => + dispatch(commentActions.destroy(topic.id, post.id)) + } + post={post} + rateComment={(rating) => + dispatch( + commentActions.rateAiFeedback(topic.id, post.id, rating), + ) + } + rejectComment={(value) => + dispatch( + commentActions.rejectAiFeedback(topic.id, post.id, value), + ) + } + /> + ); + } + + return ( + + dispatch(commentActions.destroy(topic.id, post.id)) + } + editValue={commentForms.posts[post.id]} + handleChange={(value) => + dispatch(commentActions.onUpdateChange(post.id, value)) + } + isUpdatingAnnotationAllowed + post={post} + publishComment={(value) => + dispatch(commentActions.publish(topic.id, post.id, value ?? '')) + } + updateComment={(value) => + dispatch(commentActions.update(topic.id, post.id, value ?? '')) + } + /> + ); + })} + + + dispatch(commentActions.onCreateChange(topic.id, comment)) + } + inputId={newCommentIdentifier(topic.id)} + isSubmittingDelayedComment={commentForms.isSubmittingDelayedComment} + isSubmittingNormalComment={commentForms.isSubmittingNormalComment} + isUpdatingComment={commentForms.isUpdatingComment} + renderDelayedCommentButton={renderDelayedCommentButton} + value={commentForms.topics[topic.id]} + /> +
+ ); +}; + +export default Comments; diff --git a/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/components/ExplanationPanel.tsx b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/components/ExplanationPanel.tsx index 44178537129..6585526c009 100644 --- a/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/components/ExplanationPanel.tsx +++ b/client/app/bundles/course/assessment/submission/pages/SubmissionEditIndex/components/ExplanationPanel.tsx @@ -53,12 +53,17 @@ const ExplanationPanel: FC = (props) => { return null; } + // Rubric grading has no notion of a wrong answer, so for it `correct` only records that the answer was + // submitted (see the server's _rubric_explanation partial). + const judgesCorrectness = + question.autogradable && question.gradingMode !== 'rubric'; + const getExplanationTitle = (): string => { - if (explanation.correct && question.autogradable) { + if (explanation.correct && judgesCorrectness) { return t(translations.correct); } - if (explanation.correct && !question.autogradable) { + if (explanation.correct) { return t(translations.answerSubmitted); } diff --git a/client/app/bundles/course/assessment/submission/reducers/__test__/commentForms.test.ts b/client/app/bundles/course/assessment/submission/reducers/__test__/commentForms.test.ts new file mode 100644 index 00000000000..39075e7c080 --- /dev/null +++ b/client/app/bundles/course/assessment/submission/reducers/__test__/commentForms.test.ts @@ -0,0 +1,87 @@ +import actions from '../../constants'; +import reducer, { CommentFormsState } from '../commentForms'; + +const loaded = (): CommentFormsState => + reducer(undefined, { + type: actions.FETCH_SUBMISSION_SUCCESS, + payload: { topics: [{ id: 1 }], annotations: [{ fileId: 7 }] }, + }); + +describe('commentForms reducer', () => { + it('starts every box empty when the submission loads, keeping post edits', () => { + const editing = reducer(loaded(), { + type: actions.UPDATE_COMMENT_CHANGE, + payload: { postId: 10, text: 'draft edit' }, + }); + const reloaded = reducer(editing, { + type: actions.FETCH_SUBMISSION_SUCCESS, + payload: { topics: [{ id: 2 }], annotations: [] }, + }); + + expect(reloaded.topics).toEqual({ 2: '' }); + expect(reloaded.annotations).toEqual({}); + expect(reloaded.posts).toEqual({ 10: 'draft edit' }); + }); + + it('clears the new-comment box once the comment is created', () => { + const typed = reducer(loaded(), { + type: actions.CREATE_COMMENT_CHANGE, + payload: { topicId: 1, text: 'hello' }, + }); + const submitting = reducer(typed, { + type: actions.CREATE_COMMENT_REQUEST, + isDelayedComment: true, + }); + const created = reducer(submitting, { + type: actions.CREATE_COMMENT_SUCCESS, + payload: { topicId: 1 }, + }); + + expect(typed.topics[1]).toBe('hello'); + expect(submitting.isSubmittingDelayedComment).toBe(true); + expect(submitting.isSubmittingNormalComment).toBe(false); + expect(created.topics[1]).toBe(''); + expect(created.isSubmittingDelayedComment).toBe(false); + }); + + it('opens an annotation box on a file that has none yet', () => { + const state = reducer(loaded(), { + type: actions.CREATE_ANNOTATION_CHANGE, + payload: { fileId: 8, line: 3, text: 'look here' }, + }); + + expect(state.annotations[8]).toEqual({ 3: 'look here' }); + }); + + it('tracks an edit through to the saved text, then forgets it on delete', () => { + const updating = reducer(loaded(), { + type: actions.UPDATE_COMMENT_REQUEST, + }); + const updated = reducer(updating, { + type: actions.UPDATE_COMMENT_SUCCESS, + payload: { id: 10, text: 'saved' }, + }); + const deleted = reducer(updated, { + type: actions.DELETE_COMMENT_SUCCESS, + payload: { postId: 10 }, + }); + + expect(updating.isUpdatingComment).toBe(true); + expect(updated.isUpdatingComment).toBe(false); + expect(updated.posts[10]).toBe('saved'); + expect(deleted.posts).toEqual({}); + }); + + it("empties the annotation boxes of a graded answer's files", () => { + const typed = reducer(loaded(), { + type: actions.CREATE_ANNOTATION_CHANGE, + payload: { fileId: 7, line: 1, text: 'unsent' }, + }); + const graded = reducer(typed, { + type: actions.AUTOGRADE_SUCCESS, + payload: { latestAnswer: { annotations: [{ fileId: 7 }] } }, + }); + + expect(graded.annotations[7]).toEqual({}); + }); +}); diff --git a/client/app/bundles/course/assessment/submission/reducers/__test__/posts.test.ts b/client/app/bundles/course/assessment/submission/reducers/__test__/posts.test.ts new file mode 100644 index 00000000000..1ee52debe87 --- /dev/null +++ b/client/app/bundles/course/assessment/submission/reducers/__test__/posts.test.ts @@ -0,0 +1,70 @@ +import { CommentPostMiniEntity } from 'types/course/comments'; + +import actions from '../../constants'; +import reducer, { PostsState } from '../posts'; + +const buildPost = ( + id: number, + overrides: Partial = {}, +): CommentPostMiniEntity => + ({ + id, + topicId: 1, + text: `post ${id}`, + workflowState: 'published', + isAiGenerated: false, + ...overrides, + }) as CommentPostMiniEntity; + +describe('posts reducer', () => { + const existing: PostsState = { 1: buildPost(1) }; + + it('merges a batch of posts into the existing ones by id', () => { + const state = reducer(existing, { + type: actions.FETCH_SUBMISSION_SUCCESS, + payload: { posts: [buildPost(1, { text: 'edited' }), buildPost(2)] }, + }); + + expect(state[1].text).toBe('edited'); + expect(state[2]).toEqual(buildPost(2)); + }); + + it('upserts a single created or edited post', () => { + const state = reducer(existing, { + type: actions.UPDATE_COMMENT_SUCCESS, + payload: buildPost(1, { text: 'edited' }), + }); + + expect(state[1].text).toBe('edited'); + }); + + it('adds the AI feedback comment that rubric grading returned', () => { + const comment = buildPost(3, { isAiGenerated: true }); + const state = reducer(existing, { + type: actions.AUTOGRADE_RUBRIC_SUCCESS, + payload: { questionId: 1, aiGeneratedComment: comment }, + }); + + expect(state[3]).toEqual(comment); + }); + + // A student sees the rubric breakdown only once the submission is published, so the action often arrives + // carrying neither -- it must leave the posts untouched rather than insert an undefined entry. + it('leaves the posts untouched when rubric grading returned no comment', () => { + const state = reducer(existing, { + type: actions.AUTOGRADE_RUBRIC_SUCCESS, + payload: { questionId: 1 }, + }); + + expect(state).toBe(existing); + }); + + it('removes a deleted post', () => { + const state = reducer( + { ...existing, 2: buildPost(2) }, + { type: actions.DELETE_COMMENT_SUCCESS, payload: { postId: 2 } }, + ); + + expect(Object.keys(state)).toEqual(['1']); + }); +}); diff --git a/client/app/bundles/course/assessment/submission/reducers/__test__/topics.test.ts b/client/app/bundles/course/assessment/submission/reducers/__test__/topics.test.ts new file mode 100644 index 00000000000..1efc538c258 --- /dev/null +++ b/client/app/bundles/course/assessment/submission/reducers/__test__/topics.test.ts @@ -0,0 +1,69 @@ +import { CommentPostMiniEntity } from 'types/course/comments'; + +import actions from '../../constants'; +import { Topic, TopicState } from '../../types'; +import reducer from '../topics'; + +const buildTopic = (id: number, postIds: number[] = []): Topic => ({ + id, + questionId: id, + submissionQuestionId: id, + postIds, +}); + +const buildPost = (id: number, topicId: number): CommentPostMiniEntity => + ({ id, topicId }) as CommentPostMiniEntity; + +describe('topics reducer', () => { + const existing: TopicState = { 1: buildTopic(1, [10]) }; + + it('merges the submission topics into the existing ones by id', () => { + const state = reducer(existing, { + type: actions.FETCH_SUBMISSION_SUCCESS, + payload: { topics: [buildTopic(1, [10, 11]), buildTopic(2)] }, + }); + + expect(state[1].postIds).toEqual([10, 11]); + expect(state[2]).toEqual(buildTopic(2)); + }); + + it('records a created comment on its topic, once', () => { + const created = reducer(existing, { + type: actions.CREATE_COMMENT_SUCCESS, + payload: buildPost(11, 1), + }); + const createdAgain = reducer(created, { + type: actions.CREATE_COMMENT_SUCCESS, + payload: buildPost(11, 1), + }); + + expect(createdAgain[1].postIds).toEqual([10, 11]); + }); + + it('records the AI feedback comment that rubric grading returned', () => { + const state = reducer(existing, { + type: actions.AUTOGRADE_RUBRIC_SUCCESS, + payload: { questionId: 1, aiGeneratedComment: buildPost(12, 1) }, + }); + + expect(state[1].postIds).toEqual([10, 12]); + }); + + it('leaves the topics untouched when rubric grading returned no comment', () => { + const state = reducer(existing, { + type: actions.AUTOGRADE_RUBRIC_SUCCESS, + payload: { questionId: 1 }, + }); + + expect(state).toBe(existing); + }); + + it('drops a deleted comment from its topic', () => { + const state = reducer(existing, { + type: actions.DELETE_COMMENT_SUCCESS, + payload: { topicId: 1, postId: 10 }, + }); + + expect(state[1].postIds).toEqual([]); + }); +}); diff --git a/client/app/bundles/course/assessment/submission/reducers/commentForms.js b/client/app/bundles/course/assessment/submission/reducers/commentForms.js deleted file mode 100644 index 16599d241d2..00000000000 --- a/client/app/bundles/course/assessment/submission/reducers/commentForms.js +++ /dev/null @@ -1,152 +0,0 @@ -import actions from '../constants'; - -const initialState = { - isSubmittingNormalComment: false, - isSubmittingDelayedComment: false, - annotationsDelayedComment: {}, - topics: {}, - posts: {}, -}; - -export default function (state = initialState, action) { - switch (action.type) { - case actions.FETCH_SUBMISSION_SUCCESS: - return { - ...state, - topics: action.payload.topics.reduce( - (obj, topic) => ({ ...obj, [topic.id]: '' }), - {}, - ), - annotations: action.payload.annotations.reduce( - (obj, annotation) => ({ ...obj, [annotation.fileId]: {} }), - {}, - ), - }; - case actions.CREATE_ANNOTATION_CHANGE: { - const { fileId, line, text } = action.payload; - return { - ...state, - annotations: { - ...state.annotations, - [fileId]: { ...state.annotations[fileId], [line]: text }, - }, - }; - } - case actions.CREATE_ANNOTATION_SUCCESS: { - const { fileId, line } = action.payload; - return { - ...state, - isSubmittingNormalComment: false, - isSubmittingDelayedComment: false, - annotations: { - ...state.annotations, - [fileId]: { - ...state.annotations[fileId], - [line]: '', - }, - }, - }; - } - case actions.CREATE_COMMENT_CHANGE: { - const { topicId, text } = action.payload; - return { - ...state, - topics: { - ...state.topics, - [topicId]: text, - }, - }; - } - case actions.CREATE_COMMENT_SUCCESS: { - const { topicId } = action.payload; - return { - ...state, - isSubmittingNormalComment: false, - isSubmittingDelayedComment: false, - topics: { - ...state.topics, - [topicId]: '', - }, - }; - } - case actions.UPDATE_ANNOTATION_CHANGE: - case actions.UPDATE_COMMENT_CHANGE: { - const { postId, text } = action.payload; - return { - ...state, - posts: { - ...state.posts, - [postId]: text, - }, - }; - } - case actions.UPDATE_ANNOTATION_SUCCESS: - case actions.UPDATE_COMMENT_SUCCESS: { - const { id } = action.payload; - return { - ...state, - isUpdatingComment: false, - posts: { - ...state.posts, - [id]: action.payload.text, - }, - }; - } - case actions.DELETE_ANNOTATION_SUCCESS: - case actions.DELETE_COMMENT_SUCCESS: { - return { - ...state, - posts: Object.keys(state.posts).reduce((obj, key) => { - if (key !== action.payload.postId.toString()) { - return { ...obj, [key]: state.posts[key] }; - } - return obj; - }, {}), - }; - } - case actions.CREATE_ANNOTATION_REQUEST: - case actions.CREATE_COMMENT_REQUEST: - return { - ...state, - isSubmittingNormalComment: !action.isDelayedComment, - isSubmittingDelayedComment: action.isDelayedComment, - }; - case actions.UPDATE_ANNOTATION_REQUEST: - case actions.UPDATE_COMMENT_REQUEST: - return { - ...state, - isUpdatingComment: true, - }; - case actions.CREATE_ANNOTATION_FAILURE: - case actions.CREATE_COMMENT_FAILURE: - return { - ...state, - isSubmittingNormalComment: false, - isSubmittingDelayedComment: false, - }; - case actions.UPDATE_ANNOTATION_FAILURE: - case actions.UPDATE_COMMENT_FAILURE: - return { - ...state, - isUpdatingComment: false, - }; - case actions.AUTOGRADE_SUCCESS: { - const { latestAnswer } = action.payload; - if (latestAnswer && latestAnswer.annotations) { - return { - ...state, - annotations: { - ...state.annotations, - ...latestAnswer.annotations.reduce( - (obj, annotation) => ({ ...obj, [annotation.fileId]: {} }), - {}, - ), - }, - }; - } - return state; - } - default: - return state; - } -} diff --git a/client/app/bundles/course/assessment/submission/reducers/commentForms.ts b/client/app/bundles/course/assessment/submission/reducers/commentForms.ts new file mode 100644 index 00000000000..20d00541b9d --- /dev/null +++ b/client/app/bundles/course/assessment/submission/reducers/commentForms.ts @@ -0,0 +1,224 @@ +// What the user is typing into this submission's comment boxes -- new comments on each question, new annotations on +// each programming file line, and edits to existing posts -- plus whether a comment is being submitted or updated. +import { createSlice } from '@reduxjs/toolkit'; + +import { isOneOf } from '../../utils/matchers'; +import actions from '../constants'; + +export interface CommentFormsState { + isSubmittingNormalComment: boolean; + isSubmittingDelayedComment: boolean; + isUpdatingComment: boolean; + // The new-comment box of each question's topic, keyed by topic id. + topics: Record; + // The edit box of each comment or annotation post, keyed by post id. + posts: Record; + // The new-annotation box on each line of each programming file, keyed by file id, then line number. + annotations: Record>; +} + +interface SubmissionFetchedAction { + type: typeof actions.FETCH_SUBMISSION_SUCCESS; + payload: { topics?: { id: number }[]; annotations?: { fileId: number }[] }; +} + +interface NewCommentChangedAction { + type: typeof actions.CREATE_COMMENT_CHANGE; + payload: { topicId: number; text: string }; +} + +interface CommentCreatedAction { + type: typeof actions.CREATE_COMMENT_SUCCESS; + payload: { topicId: number }; +} + +interface NewAnnotationChangedAction { + type: typeof actions.CREATE_ANNOTATION_CHANGE; + payload: { fileId: number; line: number; text: string }; +} + +interface AnnotationCreatedAction { + type: typeof actions.CREATE_ANNOTATION_SUCCESS; + payload: { fileId: number; line: number }; +} + +interface PostEditChangedAction { + type: + | typeof actions.UPDATE_COMMENT_CHANGE + | typeof actions.UPDATE_ANNOTATION_CHANGE; + payload: { postId: number; text: string }; +} + +interface PostUpdatedAction { + type: + | typeof actions.UPDATE_COMMENT_SUCCESS + | typeof actions.UPDATE_ANNOTATION_SUCCESS; + payload: { id: number; text: string }; +} + +interface PostDeletedAction { + type: + | typeof actions.DELETE_COMMENT_SUCCESS + | typeof actions.DELETE_ANNOTATION_SUCCESS; + payload: { postId: number }; +} + +interface CreateRequestedAction { + type: + | typeof actions.CREATE_COMMENT_REQUEST + | typeof actions.CREATE_ANNOTATION_REQUEST; + isDelayedComment?: boolean; +} + +// Grading an answer may return its latest attempt, whose programming files start with empty annotation boxes. +interface AnswerAutogradedAction { + type: typeof actions.AUTOGRADE_SUCCESS; + payload: { latestAnswer?: { annotations?: { fileId: number }[] } }; +} + +const initialState: CommentFormsState = { + isSubmittingNormalComment: false, + isSubmittingDelayedComment: false, + isUpdatingComment: false, + topics: {}, + posts: {}, + annotations: {}, +}; + +const annotationBoxesOf = ( + state: CommentFormsState, + fileId: number, +): Record => { + state.annotations[fileId] ??= {}; + return state.annotations[fileId]; +}; + +export const commentFormsSlice = createSlice({ + name: 'commentForms', + initialState, + reducers: {}, + extraReducers: (builder) => { + builder + .addMatcher( + isOneOf(actions.FETCH_SUBMISSION_SUCCESS), + (state, action) => { + // Replaced rather than merged: a freshly loaded submission starts every box empty. + state.topics = Object.fromEntries( + (action.payload.topics ?? []).map((topic) => [topic.id, '']), + ); + state.annotations = Object.fromEntries( + (action.payload.annotations ?? []).map((annotation) => [ + annotation.fileId, + {}, + ]), + ); + }, + ) + .addMatcher( + isOneOf(actions.CREATE_COMMENT_CHANGE), + (state, action) => { + state.topics[action.payload.topicId] = action.payload.text; + }, + ) + .addMatcher( + isOneOf(actions.CREATE_COMMENT_SUCCESS), + (state, action) => { + state.isSubmittingNormalComment = false; + state.isSubmittingDelayedComment = false; + state.topics[action.payload.topicId] = ''; + }, + ) + .addMatcher( + isOneOf(actions.CREATE_ANNOTATION_CHANGE), + (state, action) => { + const { fileId, line, text } = action.payload; + annotationBoxesOf(state, fileId)[line] = text; + }, + ) + .addMatcher( + isOneOf(actions.CREATE_ANNOTATION_SUCCESS), + (state, action) => { + const { fileId, line } = action.payload; + state.isSubmittingNormalComment = false; + state.isSubmittingDelayedComment = false; + annotationBoxesOf(state, fileId)[line] = ''; + }, + ) + .addMatcher( + isOneOf( + actions.UPDATE_COMMENT_CHANGE, + actions.UPDATE_ANNOTATION_CHANGE, + ), + (state, action) => { + state.posts[action.payload.postId] = action.payload.text; + }, + ) + .addMatcher( + isOneOf( + actions.UPDATE_COMMENT_SUCCESS, + actions.UPDATE_ANNOTATION_SUCCESS, + ), + (state, action) => { + state.isUpdatingComment = false; + state.posts[action.payload.id] = action.payload.text; + }, + ) + .addMatcher( + isOneOf( + actions.DELETE_COMMENT_SUCCESS, + actions.DELETE_ANNOTATION_SUCCESS, + ), + (state, action) => { + delete state.posts[action.payload.postId]; + }, + ) + .addMatcher( + isOneOf( + actions.CREATE_COMMENT_REQUEST, + actions.CREATE_ANNOTATION_REQUEST, + ), + (state, action) => { + state.isSubmittingNormalComment = !action.isDelayedComment; + state.isSubmittingDelayedComment = Boolean(action.isDelayedComment); + }, + ) + .addMatcher( + isOneOf( + actions.UPDATE_COMMENT_REQUEST, + actions.UPDATE_ANNOTATION_REQUEST, + ), + (state) => { + state.isUpdatingComment = true; + }, + ) + .addMatcher( + isOneOf( + actions.CREATE_COMMENT_FAILURE, + actions.CREATE_ANNOTATION_FAILURE, + ), + (state) => { + state.isSubmittingNormalComment = false; + state.isSubmittingDelayedComment = false; + }, + ) + .addMatcher( + isOneOf( + actions.UPDATE_COMMENT_FAILURE, + actions.UPDATE_ANNOTATION_FAILURE, + ), + (state) => { + state.isUpdatingComment = false; + }, + ) + .addMatcher( + isOneOf(actions.AUTOGRADE_SUCCESS), + (state, action) => { + action.payload.latestAnswer?.annotations?.forEach((annotation) => { + state.annotations[annotation.fileId] = {}; + }); + }, + ); + }, +}); + +export default commentFormsSlice.reducer; diff --git a/client/app/bundles/course/assessment/submission/reducers/gradingResults.ts b/client/app/bundles/course/assessment/submission/reducers/gradingResults.ts index 0db7a90132d..1602f0eca62 100644 --- a/client/app/bundles/course/assessment/submission/reducers/gradingResults.ts +++ b/client/app/bundles/course/assessment/submission/reducers/gradingResults.ts @@ -12,6 +12,7 @@ import { } from 'types/course/assessment/submission/answer/programming'; import { TextResponseSolutionResult } from 'types/course/assessment/submission/answer/textResponse'; +import { isOneOf } from '../../utils/matchers'; import actions from '../constants'; import { CategoryGradeType } from '../types'; @@ -56,10 +57,11 @@ interface AnswerDataAction { } // UPDATE_RUBRIC (grader edits a category) / AUTOGRADE_RUBRIC_SUCCESS carry the affected question's id + its -// refreshed category breakdown. +// refreshed category breakdown. AUTOGRADE_RUBRIC_SUCCESS may omit the breakdown (a student before publication +// receives only the AI feedback comment), in which case the existing breakdown stands. interface RubricUpdateAction { type: typeof actions.UPDATE_RUBRIC | typeof actions.AUTOGRADE_RUBRIC_SUCCESS; - payload: { questionId: number; categoryGrades: CategoryGradeType[] }; + payload: { questionId: number; categoryGrades?: CategoryGradeType[] }; } interface QuestionIdAction { @@ -67,23 +69,6 @@ interface QuestionIdAction { questionId: number; } -interface UnknownAction { - type: Exclude< - string, - | AnswerDataArrayAction['type'] - | AnswerDataAction['type'] - | QuestionIdAction['type'] - | RubricUpdateAction['type'] - >; -} - -type Action = - | AnswerDataArrayAction - | AnswerDataAction - | QuestionIdAction - | RubricUpdateAction - | UnknownAction; - export default createReducer( { solutionResults: {}, @@ -92,18 +77,16 @@ export default createReducer( }, (builder) => { builder.addMatcher( - (action: Action): action is AnswerDataArrayAction => { - return [ - actions.FETCH_SUBMISSION_SUCCESS, - actions.FINALISE_SUCCESS, - actions.UNSUBMIT_SUCCESS, - actions.SAVE_ALL_GRADE_SUCCESS, - actions.SAVE_GRADE_SUCCESS, - actions.MARK_SUCCESS, - actions.UNMARK_SUCCESS, - actions.PUBLISH_SUCCESS, - ].includes(action.type); - }, + isOneOf( + actions.FETCH_SUBMISSION_SUCCESS, + actions.FINALISE_SUCCESS, + actions.UNSUBMIT_SUCCESS, + actions.SAVE_ALL_GRADE_SUCCESS, + actions.SAVE_GRADE_SUCCESS, + actions.MARK_SUCCESS, + actions.UNMARK_SUCCESS, + actions.PUBLISH_SUCCESS, + ), (state, action) => { const newSolutionResults: Record = {}; @@ -130,14 +113,12 @@ export default createReducer( ); builder.addMatcher( - (action: Action): action is AnswerDataAction => { - return [ - actions.SAVE_ANSWER_SUCCESS, - actions.REEVALUATE_SUCCESS, - actions.AUTOGRADE_SUCCESS, - actions.RESET_SUCCESS, - ].includes(action.type); - }, + isOneOf( + actions.SAVE_ANSWER_SUCCESS, + actions.REEVALUATE_SUCCESS, + actions.AUTOGRADE_SUCCESS, + actions.RESET_SUCCESS, + ), (state, action) => { const answer = action.payload; if ( @@ -156,24 +137,23 @@ export default createReducer( ); builder.addMatcher( - (action: Action): action is RubricUpdateAction => { - return [ - actions.UPDATE_RUBRIC, - actions.AUTOGRADE_RUBRIC_SUCCESS, - ].includes(action.type); - }, + isOneOf( + actions.UPDATE_RUBRIC, + actions.AUTOGRADE_RUBRIC_SUCCESS, + ), (state, action) => { + if (!action.payload.categoryGrades) return; + state.categoryGrades[action.payload.questionId] = action.payload.categoryGrades; }, ); builder.addMatcher( - (action: Action): action is QuestionIdAction => { - return [actions.REEVALUATE_FAILURE, actions.AUTOGRADE_FAILURE].includes( - action.type, - ); - }, + isOneOf( + actions.REEVALUATE_FAILURE, + actions.AUTOGRADE_FAILURE, + ), (state, action) => { // Clear the previous test results, keeping the test case definitions so the panel still // lists what would have been run. diff --git a/client/app/bundles/course/assessment/submission/reducers/posts.js b/client/app/bundles/course/assessment/submission/reducers/posts.js deleted file mode 100644 index 0c76dbaefca..00000000000 --- a/client/app/bundles/course/assessment/submission/reducers/posts.js +++ /dev/null @@ -1,40 +0,0 @@ -import { arrayToObjectWithKey } from 'utilities/array'; - -import actions from '../constants'; - -export default function (state = {}, action) { - switch (action.type) { - case actions.FETCH_SUBMISSION_SUCCESS: - case actions.FETCH_ANNOTATION_SUCCESS: - return { - ...state, - ...arrayToObjectWithKey(action.payload.posts, 'id'), - }; - case actions.AUTOGRADE_RUBRIC_SUCCESS: - case actions.CREATE_COMMENT_SUCCESS: - case actions.UPDATE_COMMENT_SUCCESS: - case actions.CREATE_ANNOTATION_SUCCESS: - case actions.UPDATE_ANNOTATION_SUCCESS: { - const post = - action.type === actions.AUTOGRADE_RUBRIC_SUCCESS - ? action.payload.aiGeneratedComment - : action.payload; - if (!post) return state; - const { id } = post; - return { - ...state, - [id]: post, - }; - } - case actions.DELETE_COMMENT_SUCCESS: - case actions.DELETE_ANNOTATION_SUCCESS: - return Object.keys(state).reduce((obj, key) => { - if (key !== action.payload.postId.toString()) { - return { ...obj, [key]: state[key] }; - } - return obj; - }, {}); - default: - return state; - } -} diff --git a/client/app/bundles/course/assessment/submission/reducers/posts.ts b/client/app/bundles/course/assessment/submission/reducers/posts.ts new file mode 100644 index 00000000000..49a703728c8 --- /dev/null +++ b/client/app/bundles/course/assessment/submission/reducers/posts.ts @@ -0,0 +1,95 @@ +// Discussion posts on this submission -- question comments and programming annotations alike -- keyed by post +// id. Posts arrive with the submission, as comments and annotations are created, edited or deleted, and as the +// AI feedback comment that rubric grading produces. +import { createSlice } from '@reduxjs/toolkit'; +import { CommentPostMiniEntity } from 'types/course/comments'; + +import { isOneOf } from '../../utils/matchers'; +import actions from '../constants'; + +export type PostsState = Record; + +// A batch of posts: the whole submission's, or one programming file's annotations. +interface PostsLoadedAction { + type: + | typeof actions.FETCH_SUBMISSION_SUCCESS + | typeof actions.FETCH_ANNOTATION_SUCCESS; + payload: { posts?: CommentPostMiniEntity[] }; +} + +// A single post that was created or edited. +interface PostSavedAction { + type: + | typeof actions.CREATE_COMMENT_SUCCESS + | typeof actions.UPDATE_COMMENT_SUCCESS + | typeof actions.CREATE_ANNOTATION_SUCCESS + | typeof actions.UPDATE_ANNOTATION_SUCCESS; + payload: CommentPostMiniEntity; +} + +// Grading a rubric answer may produce an AI feedback comment -- a draft for staff, or one already published to +// the student, depending on the course's feedback workflow. Absent when there is none the viewer may see. +interface RubricAutogradedAction { + type: typeof actions.AUTOGRADE_RUBRIC_SUCCESS; + payload: { aiGeneratedComment?: CommentPostMiniEntity }; +} + +interface PostDeletedAction { + type: + | typeof actions.DELETE_COMMENT_SUCCESS + | typeof actions.DELETE_ANNOTATION_SUCCESS; + payload: { postId: number }; +} + +const initialState: PostsState = {}; + +export const postsSlice = createSlice({ + name: 'posts', + initialState, + reducers: {}, + extraReducers: (builder) => { + builder + .addMatcher( + isOneOf( + actions.FETCH_SUBMISSION_SUCCESS, + actions.FETCH_ANNOTATION_SUCCESS, + ), + (state, action) => { + action.payload.posts?.forEach((post) => { + state[post.id] = post; + }); + }, + ) + .addMatcher( + isOneOf( + actions.CREATE_COMMENT_SUCCESS, + actions.UPDATE_COMMENT_SUCCESS, + actions.CREATE_ANNOTATION_SUCCESS, + actions.UPDATE_ANNOTATION_SUCCESS, + ), + (state, action) => { + state[action.payload.id] = action.payload; + }, + ) + .addMatcher( + isOneOf(actions.AUTOGRADE_RUBRIC_SUCCESS), + (state, action) => { + const { aiGeneratedComment } = action.payload; + if (aiGeneratedComment) { + state[aiGeneratedComment.id] = aiGeneratedComment; + } + }, + ) + .addMatcher( + isOneOf( + actions.DELETE_COMMENT_SUCCESS, + actions.DELETE_ANNOTATION_SUCCESS, + ), + (state, action) => { + delete state[action.payload.postId]; + }, + ); + }, +}); + +export default postsSlice.reducer; diff --git a/client/app/bundles/course/assessment/submission/reducers/topics.js b/client/app/bundles/course/assessment/submission/reducers/topics.js deleted file mode 100644 index d86cecfe6e7..00000000000 --- a/client/app/bundles/course/assessment/submission/reducers/topics.js +++ /dev/null @@ -1,47 +0,0 @@ -import { arrayToObjectWithKey } from 'utilities/array'; - -import actions from '../constants'; - -export default function (state = {}, action) { - switch (action.type) { - case actions.FETCH_SUBMISSION_SUCCESS: - return { - ...state, - ...arrayToObjectWithKey(action.payload.topics, 'id'), - }; - case actions.AUTOGRADE_RUBRIC_SUCCESS: - case actions.CREATE_COMMENT_SUCCESS: { - if ( - action.type === actions.AUTOGRADE_RUBRIC_SUCCESS && - !action.payload.aiGeneratedComment - ) { - return state; - } - const { topicId, id: postId } = - action.type === actions.AUTOGRADE_RUBRIC_SUCCESS - ? action.payload.aiGeneratedComment - : action.payload; - return { - ...state, - [topicId]: { - ...state[topicId], - postIds: state[topicId].postIds.includes(postId) - ? state[topicId].postIds - : [...state[topicId].postIds, postId], - }, - }; - } - case actions.DELETE_COMMENT_SUCCESS: { - const { topicId, postId } = action.payload; - return { - ...state, - [topicId]: { - ...state[topicId], - postIds: state[topicId].postIds.filter((id) => id !== postId), - }, - }; - } - default: - return state; - } -} diff --git a/client/app/bundles/course/assessment/submission/reducers/topics.ts b/client/app/bundles/course/assessment/submission/reducers/topics.ts new file mode 100644 index 00000000000..1178da15b6f --- /dev/null +++ b/client/app/bundles/course/assessment/submission/reducers/topics.ts @@ -0,0 +1,82 @@ +// The discussion topics of this submission's questions, keyed by topic id. Each tracks the ids of its posts (the +// posts themselves live in the posts slice), so a post created, deleted, or produced by rubric grading as AI +// feedback is reflected here too. +import { createSlice } from '@reduxjs/toolkit'; +import { CommentPostMiniEntity } from 'types/course/comments'; + +import { isOneOf } from '../../utils/matchers'; +import actions from '../constants'; +import { Topic, TopicState } from '../types'; + +interface TopicsLoadedAction { + type: typeof actions.FETCH_SUBMISSION_SUCCESS; + payload: { topics?: Topic[] }; +} + +interface CommentCreatedAction { + type: typeof actions.CREATE_COMMENT_SUCCESS; + payload: CommentPostMiniEntity; +} + +// Grading a rubric answer may produce an AI feedback comment (see the posts slice). Absent when there is none +// the viewer may see. +interface RubricAutogradedAction { + type: typeof actions.AUTOGRADE_RUBRIC_SUCCESS; + payload: { aiGeneratedComment?: CommentPostMiniEntity }; +} + +interface CommentDeletedAction { + type: typeof actions.DELETE_COMMENT_SUCCESS; + payload: { topicId: number; postId: number }; +} + +const addPost = (state: TopicState, post: CommentPostMiniEntity): void => { + const topic = state[post.topicId]; + if (topic && !topic.postIds.includes(post.id)) { + topic.postIds.push(post.id); + } +}; + +const initialState: TopicState = {}; + +export const topicsSlice = createSlice({ + name: 'topics', + initialState, + reducers: {}, + extraReducers: (builder) => { + builder + .addMatcher( + isOneOf(actions.FETCH_SUBMISSION_SUCCESS), + (state, action) => { + action.payload.topics?.forEach((topic) => { + state[topic.id] = topic; + }); + }, + ) + .addMatcher( + isOneOf(actions.CREATE_COMMENT_SUCCESS), + (state, action) => { + addPost(state, action.payload); + }, + ) + .addMatcher( + isOneOf(actions.AUTOGRADE_RUBRIC_SUCCESS), + (state, action) => { + const { aiGeneratedComment } = action.payload; + if (aiGeneratedComment) addPost(state, aiGeneratedComment); + }, + ) + .addMatcher( + isOneOf(actions.DELETE_COMMENT_SUCCESS), + (state, action) => { + const { topicId, postId } = action.payload; + const topic = state[topicId]; + if (topic) { + topic.postIds = topic.postIds.filter((id) => id !== postId); + } + }, + ); + }, +}); + +export default topicsSlice.reducer; diff --git a/client/app/bundles/course/assessment/submission/selectors/commentForms.ts b/client/app/bundles/course/assessment/submission/selectors/commentForms.ts new file mode 100644 index 00000000000..514c84d8faa --- /dev/null +++ b/client/app/bundles/course/assessment/submission/selectors/commentForms.ts @@ -0,0 +1,6 @@ +import { AppState } from 'store'; + +import { CommentFormsState } from '../reducers/commentForms'; + +export const getCommentForms = (state: AppState): CommentFormsState => + state.assessments.submission.commentForms; diff --git a/client/app/bundles/course/assessment/submission/selectors/comments.ts b/client/app/bundles/course/assessment/submission/selectors/comments.ts index 6d70fe68396..82684e8d6e9 100644 --- a/client/app/bundles/course/assessment/submission/selectors/comments.ts +++ b/client/app/bundles/course/assessment/submission/selectors/comments.ts @@ -7,6 +7,10 @@ const getLocalState = ( return state.assessments.submission.posts; }; +export const getCommentPosts = ( + state: AppState, +): Record => getLocalState(state); + export const getCommentPostById = ( state: AppState, postId: number | null, diff --git a/client/app/bundles/course/assessment/submission/types.ts b/client/app/bundles/course/assessment/submission/types.ts index 4b5a1de2fd1..79addc5b111 100644 --- a/client/app/bundles/course/assessment/submission/types.ts +++ b/client/app/bundles/course/assessment/submission/types.ts @@ -171,7 +171,11 @@ export interface GradingState { expMultiplier: number; } -interface Topic { +// The discussion topic of one submission question, as sent with the submission. +export interface Topic { + id: number; + questionId: number; + submissionQuestionId: number; postIds: number[]; } diff --git a/client/app/bundles/course/assessment/utils/matchers.ts b/client/app/bundles/course/assessment/utils/matchers.ts new file mode 100644 index 00000000000..093d3f1de25 --- /dev/null +++ b/client/app/bundles/course/assessment/utils/matchers.ts @@ -0,0 +1,7 @@ +// Matches the submission bundle's legacy action types (see ../constants). Those actions are dispatched by the +// bundle's thunks rather than by the slices that react to them, so slices match them by type in extraReducers +// rather than handling them as their own reducers. +export const isOneOf = + (...types: string[]) => + (action: { type: string }): action is A => + types.includes(action.type); diff --git a/client/app/types/course/admin/assessments.ts b/client/app/types/course/admin/assessments.ts index 5c9f319a416..6af6cae28fa 100644 --- a/client/app/types/course/admin/assessments.ts +++ b/client/app/types/course/admin/assessments.ts @@ -6,6 +6,8 @@ export interface AssessmentSettingsData { categories: AssessmentCategory[]; canCreateCategories: boolean; maxProgrammingTimeLimit?: number; + // How generated rubric feedback reaches students; see AiGeneratedPostService::FEEDBACK_WORKFLOWS. + rubricGradingFeedbackWorkflow: string; rubricGradingPromptEnabled: boolean; rubricGradingPrompt?: string; // AI model configuration, restricted to instance and system admins. When the viewer may not manage it the @@ -76,6 +78,7 @@ export interface AssessmentSettingsPostData { allow_randomization?: AssessmentSettingsData['allowRandomization']; allow_mrq_options_randomization?: AssessmentSettingsData['allowMrqOptionsRandomization']; programming_max_time_limit: AssessmentSettingsData['maxProgrammingTimeLimit']; + rubric_grading_feedback_workflow?: AssessmentSettingsData['rubricGradingFeedbackWorkflow']; rubric_grading_prompt_enabled?: AssessmentSettingsData['rubricGradingPromptEnabled']; rubric_grading_prompt?: AssessmentSettingsData['rubricGradingPrompt']; // Only sent when the viewer may manage them; the server drops them from the permitted params otherwise. diff --git a/client/app/types/course/assessment/submission/question/types.ts b/client/app/types/course/assessment/submission/question/types.ts index a858b2072a3..78f19e15566 100644 --- a/client/app/types/course/assessment/submission/question/types.ts +++ b/client/app/types/course/assessment/submission/question/types.ts @@ -8,6 +8,8 @@ interface QuestionData { autogradable: boolean; canViewHistory: boolean; type: QuestionType; + // 'rubric' questions are graded against a rubric and have no notion of a wrong answer. + gradingMode: 'default' | 'rubric'; } interface MultipleResponseQuestionData { diff --git a/client/locales/en.json b/client/locales/en.json index ff297d6fe62..f72a9b768f3 100644 --- a/client/locales/en.json +++ b/client/locales/en.json @@ -458,6 +458,24 @@ "course.admin.AssessmentSettings.errorOccurredWhenMovingTabs": { "defaultMessage": "An error occurred while moving the tabs." }, + "course.admin.AssessmentSettings.feedbackWorkflow": { + "defaultMessage": "Feedback comment delivery" + }, + "course.admin.AssessmentSettings.feedbackWorkflowDraft": { + "defaultMessage": "Generate draft comments for staff approval" + }, + "course.admin.AssessmentSettings.feedbackWorkflowHint": { + "defaultMessage": "AI can generate personalised comments for each answer in addition to the numeric grade." + }, + "course.admin.AssessmentSettings.feedbackWorkflowNone": { + "defaultMessage": "Generate no comments" + }, + "course.admin.AssessmentSettings.feedbackWorkflowPublishOnAnswerSubmit": { + "defaultMessage": "Publish comments immediately" + }, + "course.admin.AssessmentSettings.feedbackWorkflowPublishOnFinalise": { + "defaultMessage": "Publish comments when submission is finalised" + }, "course.admin.AssessmentSettings.gradingModel": { "defaultMessage": "Grading model" }, @@ -561,7 +579,7 @@ "defaultMessage": "The AI model used by Codaveri to generate help conversations with students for programming questions." }, "course.admin.CodaveriSettings.codaveriSystemPromptDescription": { - "defaultMessage": "You may customize the behavior of the Codaveri model by providing instructions here. {br} When assisting students, these instructions will be followed in addition to any you have set on the question itself.{br}To reference question-specific details, you may use the following variables within the prompt, writing them with brackets as shown below:" + "defaultMessage": "The Codaveri system prompt controls AI behavior when interacting with students." }, "course.admin.CodaveriSettings.codaveriSystemPromptProblemDescriptionLine": { "defaultMessage": "{problemDescriptionVar} : The full description of the coding problem." @@ -585,7 +603,7 @@ "defaultMessage": "Generate no feedback" }, "course.admin.CodaveriSettings.feedbackWorkflowDraft": { - "defaultMessage": "Generate feedback as a draft requiring approval from staff" + "defaultMessage": "Generate feedback as a draft for staff approval" }, "course.admin.CodaveriSettings.feedbackWorkflowPublish": { "defaultMessage": "Publish feedback directly to student" diff --git a/client/locales/ko.json b/client/locales/ko.json index e2f97086290..5b615b888cf 100644 --- a/client/locales/ko.json +++ b/client/locales/ko.json @@ -458,6 +458,24 @@ "course.admin.AssessmentSettings.errorOccurredWhenMovingTabs": { "defaultMessage": "탭을 이동하는 동안 오류가 발생했습니다." }, + "course.admin.AssessmentSettings.feedbackWorkflow": { + "defaultMessage": "피드백 코멘트 전달 방식" + }, + "course.admin.AssessmentSettings.feedbackWorkflowDraft": { + "defaultMessage": "스태프 승인용 초안 코멘트 생성" + }, + "course.admin.AssessmentSettings.feedbackWorkflowHint": { + "defaultMessage": "AI는 숫자 점수와 함께 각 답안에 대한 맞춤형 코멘트를 생성할 수 있습니다." + }, + "course.admin.AssessmentSettings.feedbackWorkflowNone": { + "defaultMessage": "코멘트를 생성하지 않음" + }, + "course.admin.AssessmentSettings.feedbackWorkflowPublishOnAnswerSubmit": { + "defaultMessage": "코멘트를 즉시 공개" + }, + "course.admin.AssessmentSettings.feedbackWorkflowPublishOnFinalise": { + "defaultMessage": "제출이 확정되면 코멘트 공개" + }, "course.admin.AssessmentSettings.gradingModel": { "defaultMessage": "채점 모델" }, @@ -612,7 +630,7 @@ "defaultMessage": "Codaveri가 프로그래밍 문제에 대한 학생 도움 대화를 생성하는 데 사용하는 AI 모델입니다." }, "course.admin.CodaveriSettings.codaveriSystemPromptDescription": { - "defaultMessage": "여기에서 지침을 제공하여 Codaveri 모델의 동작을 사용자 지정할 수 있습니다.{br} 학생을 도울 때, 이 지침은 문제 자체에 설정한 지침에 추가로 따르게 됩니다.{br} 문제별 세부 사항을 참조하려면, 아래와 같이 대괄호로 작성된 다음 변수를 프롬프트에서 사용할 수 있습니다:" + "defaultMessage": "Codaveri 시스템 프롬프트는 학생과 상호작용할 때 AI의 동작을 제어합니다." }, "course.admin.CodaveriSettings.codaveriSystemPromptProblemDescriptionLine": { "defaultMessage": "{problemDescriptionVar} : 코딩 문제의 전체 설명" @@ -657,7 +675,7 @@ "defaultMessage": "프로그래밍 질문이 포함된 제출물이 완료되었을 때," }, "course.admin.CodaveriSettings.feedbackWorkflowDraft": { - "defaultMessage": "직원 승인이 필요한 초안으로 피드백 생성" + "defaultMessage": "스태프 승인용 초안으로 피드백 생성" }, "course.admin.CodaveriSettings.feedbackWorkflowNone": { "defaultMessage": "피드백 생성 안 함" diff --git a/client/locales/zh.json b/client/locales/zh.json index d30289a11c4..df01fa46194 100644 --- a/client/locales/zh.json +++ b/client/locales/zh.json @@ -458,6 +458,24 @@ "course.admin.AssessmentSettings.errorOccurredWhenMovingTabs": { "defaultMessage": "移动标签时出错。" }, + "course.admin.AssessmentSettings.feedbackWorkflow": { + "defaultMessage": "反馈评语发布方式" + }, + "course.admin.AssessmentSettings.feedbackWorkflowDraft": { + "defaultMessage": "生成评语草稿供教职人员审核" + }, + "course.admin.AssessmentSettings.feedbackWorkflowHint": { + "defaultMessage": "除数值成绩外,AI 还可以为每个答案生成个性化评语。" + }, + "course.admin.AssessmentSettings.feedbackWorkflowNone": { + "defaultMessage": "不生成评语" + }, + "course.admin.AssessmentSettings.feedbackWorkflowPublishOnAnswerSubmit": { + "defaultMessage": "立即发布评语" + }, + "course.admin.AssessmentSettings.feedbackWorkflowPublishOnFinalise": { + "defaultMessage": "提交确认后发布评语" + }, "course.admin.AssessmentSettings.gradingModel": { "defaultMessage": "评分模型" }, @@ -561,7 +579,7 @@ "defaultMessage": "Codaveri 使用的 AI 模型,用于为学生的编程问题生成帮助对话。" }, "course.admin.CodaveriSettings.codaveriSystemPromptDescription": { - "defaultMessage": "您可以在此提供指令以自定义 Codaveri 模型的行为。{br} 在帮助学生时,这些指令将与您在具体题目中设置的指令一并执行。{br}要引用题目相关的细节,您可以在提示中使用以下变量,并按照如下方式用括号写出:" + "defaultMessage": "Codaveri 系统提示用于控制 AI 与学生互动时的行为。" }, "course.admin.CodaveriSettings.codaveriSystemPromptProblemDescriptionLine": { "defaultMessage": "{problemDescriptionVar} :编程问题的完整描述。" @@ -591,7 +609,7 @@ "defaultMessage": "不生成反馈" }, "course.admin.CodaveriSettings.feedbackWorkflowDraft": { - "defaultMessage": "生成草稿反馈,需要工作人员批准" + "defaultMessage": "生成反馈草稿供教职人员审核" }, "course.admin.CodaveriSettings.feedbackWorkflowPublish": { "defaultMessage": "直接向学生发布反馈" diff --git a/spec/controllers/course/admin/assessment_settings_controller_spec.rb b/spec/controllers/course/admin/assessment_settings_controller_spec.rb index 2129bce294f..da11d302139 100644 --- a/spec/controllers/course/admin/assessment_settings_controller_spec.rb +++ b/spec/controllers/course/admin/assessment_settings_controller_spec.rb @@ -105,6 +105,28 @@ end end + describe '#update persisting the rubric feedback workflow' do + # A teaching decision rather than an operational one, so an ordinary course manager may change it. + subject do + patch :update, as: :json, params: { + course_id: course, course: { rubric_grading_feedback_workflow: 'publish_on_finalise' } + } + end + + it 'stores the workflow on the course settings' do + expect(subject).to render_template(:edit) + expect(course.reload.rubric_grading_feedback_workflow).to eq('publish_on_finalise') + end + + it 'rejects an unsupported workflow' do + patch :update, as: :json, params: { + course_id: course, course: { rubric_grading_feedback_workflow: 'publish_whenever' } + } + expect(response).to have_http_status(:bad_request) + expect(course.reload.rubric_grading_feedback_workflow).to eq('draft') + end + end + describe '#update persisting the course-wide rubric grading prompt' do subject do patch :update, as: :json, params: { diff --git a/spec/controllers/course/assessment/submission/submissions_controller_spec.rb b/spec/controllers/course/assessment/submission/submissions_controller_spec.rb index 5aa00a954e3..ed88e784562 100644 --- a/spec/controllers/course/assessment/submission/submissions_controller_spec.rb +++ b/spec/controllers/course/assessment/submission/submissions_controller_spec.rb @@ -256,6 +256,94 @@ expect(json_result['questionId']).to eq answer.question.id end end + + # The client shows a freshly generated AI comment from this payload, so it has to carry the comment in + # whichever state the course's feedback workflow left it -- and respect who may see that state. + context 'when the answer has AI-generated feedback comments' do + render_views + let(:rubric_assessment) { create(:assessment, :published_with_rubric_question, course: course) } + let(:question) { rubric_assessment.questions.first } + let(:student) { create(:course_student, course: course) } + let(:student_submission) do + create(:submission, :attempting, assessment: rubric_assessment, creator: student.user, + course_user: student) + end + let(:rubric_answer) do + create(:course_assessment_answer_rubric_based_response, :submitted, + question: question, submission: student_submission).answer + end + let(:topic) do + create(:course_assessment_submission_question, submission: student_submission, question: question). + acting_as + end + let!(:published_comment) do + create(:course_discussion_post, topic: topic, is_ai_generated: true, workflow_state: 'published', + created_at: 2.minutes.ago) + end + subject do + post :reload_answer, params: { + course_id: course, assessment_id: rubric_assessment.id, + id: student_submission.id, answer_id: rubric_answer.id, format: :json + } + end + let(:ai_generated_comment_id) { JSON.parse(subject.body).dig('aiGeneratedComment', 'id') } + # Staff edited the AI's wording before publishing, then rated it. + let(:rate_published_comment) do + Course::Rubric::AnswerEvaluation.create!(answer: rubric_answer, evaluation_type: :grading, + feedback: 'The AI wording'). + ratings.create!(post: published_comment, rating: 2, original_feedback: 'The AI wording', + edited_feedback: 'The staff wording', creator: User.system, updater: User.system) + end + let(:generated_rating) { JSON.parse(subject.body).dig('aiGeneratedComment', 'generatedRating') } + + context 'when the student reloads it' do + # Not by redefining :user, which is also the course's creator. + before { controller_sign_in(controller, student.user) } + + it 'includes the published comment' do + expect(ai_generated_comment_id).to eq(published_comment.id) + end + + it 'never includes a draft, even a newer one' do + create(:course_discussion_post, topic: topic, is_ai_generated: true, workflow_state: 'draft', + created_at: 1.minute.ago) + expect(ai_generated_comment_id).to eq(published_comment.id) + end + + it "withholds the comment's rating, and with it the AI's original wording" do + rate_published_comment + expect(ai_generated_comment_id).to eq(published_comment.id) + expect(generated_rating).to be_nil + expect(subject.body).not_to include('The AI wording') + end + end + + # The Continue and Finalise buttons gate on explanation.correct. Students are not otherwise shown grades + # when partial submission is allowed, which used to withhold the explanation and leave them stuck. + context 'when partial submission is allowed' do + before do + rubric_assessment.update!(allow_partial_submission: true, show_mcq_answer: true) + controller_sign_in(controller, student.user) + end + + it 'lets the student move on once the answer has been submitted' do + expect(JSON.parse(subject.body).dig('explanation', 'correct')).to be(true) + end + end + + context 'when staff reload it' do + it 'includes the latest comment, draft or not' do + draft = create(:course_discussion_post, topic: topic, is_ai_generated: true, workflow_state: 'draft', + created_at: 1.minute.ago) + expect(ai_generated_comment_id).to eq(draft.id) + end + + it "includes the comment's rating" do + rate_published_comment + expect(generated_rating).to include('rating' => 2, 'originalContent' => 'The AI wording') + end + end + end end describe '#generate_live_feedback' do diff --git a/spec/controllers/course/forum/topics_controller_spec.rb b/spec/controllers/course/forum/topics_controller_spec.rb index 923d2a9033a..5784a73c90c 100644 --- a/spec/controllers/course/forum/topics_controller_spec.rb +++ b/spec/controllers/course/forum/topics_controller_spec.rb @@ -32,6 +32,35 @@ subject expect(topic.reload.posts.any? { |post| post.unread?(user) }).to be(false) end + + # The rating of an AI answer is staff data: the AI's original answer (possibly edited by staff before + # publishing), the staff member's score, and the answer's quality scores. + context 'when the topic has a rated AI-generated answer' do + render_views + let!(:ai_answer) do + create(:course_discussion_post, topic: topic.acting_as, is_ai_generated: true, + workflow_state: 'published') + end + let!(:rating) { create(:course_forum_rag_wise_rating, post: ai_answer, rating: 4) } + let(:ai_answer_json) { JSON.parse(subject.body)['posts'].find { |post| post['id'] == ai_answer.id } } + + context 'when a student views the topic' do + let(:user) { create(:course_student, course: course).user } + + it 'shows the answer without its rating' do + expect(ai_answer_json).to be_present + expect(ai_answer_json).not_to have_key('generatedRating') + end + end + + context 'when a teaching assistant views the topic' do + let(:user) { create(:course_teaching_assistant, course: course).user } + + it 'includes the rating' do + expect(ai_answer_json['generatedRating']).to include('rating' => 4) + end + end + end end describe '#destroy' do diff --git a/spec/features/course/assessment/submission/manually_graded_spec.rb b/spec/features/course/assessment/submission/manually_graded_spec.rb index d7dab05a1b7..5d900c948da 100644 --- a/spec/features/course/assessment/submission/manually_graded_spec.rb +++ b/spec/features/course/assessment/submission/manually_graded_spec.rb @@ -142,7 +142,7 @@ # Delete the reply first('button.delete-comment').click - accept_confirm_dialog do + accept_confirm_dialog('button.prompt-primary-btn') do wait_for_job end diff --git a/spec/models/course/assessment/submission_spec.rb b/spec/models/course/assessment/submission_spec.rb index 44d4a0451c8..223623dc1dd 100644 --- a/spec/models/course/assessment/submission_spec.rb +++ b/spec/models/course/assessment/submission_spec.rb @@ -344,6 +344,61 @@ end end + # A course that holds AI rubric feedback back until finalisation releases it here; feedback generated + # by the finalise itself is published as it is created (see AiGeneratedPostService). + describe 'releasing held-back AI rubric feedback' do + let(:submission_question) do + create(:course_assessment_submission_question, submission: submission, + question: assessment.questions.first) + end + let!(:ai_draft) do + create(:course_discussion_post, topic: submission_question.acting_as, is_ai_generated: true, + workflow_state: 'draft').tap { |post| post.topic.mark_as_pending } + end + let!(:staff_draft) do + create(:course_discussion_post, topic: submission_question.acting_as, workflow_state: 'draft') + end + + context 'when the course publishes rubric feedback on finalisation' do + before { course.update!(rubric_grading_feedback_workflow: 'publish_on_finalise') } + + it 'publishes the AI draft and clears the pending flag, leaving staff drafts alone' do + submission.update!('finalise' => 'true') + + expect(ai_draft.reload.workflow_state).to eq('published') + expect(ai_draft.topic.reload.pending_staff_reply).to be(false) + expect(staff_draft.reload.workflow_state).to eq('draft') + end + + # A comment the staff accepted ahead of finalisation is finished business. The release must skip + # it -- and so must not clear a pending flag that a later student reply raised, which is the way + # touching it would actually cost someone something. + it 'leaves a comment accepted before finalisation, and its topic, alone' do + ai_draft.update!(workflow_state: 'published') + ai_draft.topic.unmark_as_pending + + create(:course_discussion_post, topic: submission_question.acting_as, + creator: submission.creator) + submission_question.acting_as.mark_as_pending + + expect { submission.update!('finalise' => 'true') }. + not_to(change { Course::Discussion::Post.count }) + + expect(ai_draft.reload.workflow_state).to eq('published') + expect(submission_question.acting_as.reload.pending_staff_reply).to be(true) + end + end + + context 'when the course drafts rubric feedback for approval' do + it 'leaves the AI draft waiting for a staff decision' do + submission.update!('finalise' => 'true') + + expect(ai_draft.reload.workflow_state).to eq('draft') + expect(ai_draft.topic.reload.pending_staff_reply).to be(true) + end + end + end + # The finalise transaction commits the student's work, so nothing derived from it — the # personalised timeline, the notification, the Cikgo push — may be able to roll it back. The # first two also leave the request entirely, as jobs, because they call out to a third party diff --git a/spec/models/course/assessment_spec.rb b/spec/models/course/assessment_spec.rb index 21eb3415858..2df5243ddc0 100644 --- a/spec/models/course/assessment_spec.rb +++ b/spec/models/course/assessment_spec.rb @@ -273,6 +273,29 @@ it { is_expected.to be_nil } end + + # Rubric grading has no notion of a wrong answer, so a rubric-graded question is passed by submitting it + # once -- without waiting on, or depending on the success of, the grading itself. + context 'when the first question is rubric-graded' do + let(:assessment_traits) { [:with_rubric_question, :with_mcq_question] } + let(:rubric_question) { assessment.questions.first } + + it 'is still unanswered while the student has only drafted an answer' do + expect(rubric_question).to be_grading_mode_rubric + rubric_question.attempt(submission).save! + + is_expected.to eq(rubric_question) + end + + it 'counts as answered once submitted, however grading turns out' do + answer = rubric_question.attempt(submission) + answer.finalise! + answer.save! + expect(answer.correct).to be_nil + + is_expected.to eq(assessment.questions.second) + end + end end end diff --git a/spec/models/course_spec.rb b/spec/models/course_spec.rb index db1af5ea040..cc88ffd7a05 100644 --- a/spec/models/course_spec.rb +++ b/spec/models/course_spec.rb @@ -373,6 +373,28 @@ end end + describe 'the rubric feedback workflow' do + let(:course) { create(:course) } + + it 'defaults to drafting feedback for approval' do + expect(course.rubric_grading_feedback_workflow).to eq('draft') + end + + it 'accepts every supported workflow' do + Course::Assessment::Answer::AiGeneratedPostService::FEEDBACK_WORKFLOWS.each do |workflow| + course.rubric_grading_feedback_workflow = workflow + expect(course).to be_valid + expect(course.rubric_grading_feedback_workflow).to eq(workflow) + end + end + + it 'rejects an unsupported workflow' do + course.rubric_grading_feedback_workflow = 'publish_whenever' + expect(course).not_to be_valid + expect(course.errors[:rubric_grading_feedback_workflow]).to be_present + end + end + describe 'the AI grading model configuration' do let(:course) { create(:course) } diff --git a/spec/services/course/assessment/answer/ai_generated_post_service_spec.rb b/spec/services/course/assessment/answer/ai_generated_post_service_spec.rb index fe150367971..d4e6263d07b 100644 --- a/spec/services/course/assessment/answer/ai_generated_post_service_spec.rb +++ b/spec/services/course/assessment/answer/ai_generated_post_service_spec.rb @@ -167,6 +167,86 @@ end end + describe 'the course feedback workflow' do + let(:submission_question) do + create(:course_assessment_submission_question, submission: submission, question: question.acting_as) + end + let(:service) { described_class.new(answer, 'generated feedback') } + let(:post) { submission_question.posts.reload.last } + before do + assessment.course.update!(rubric_grading_feedback_workflow: workflow) + allow(answer.submission).to receive(:submission_questions).and_return(double(find_by: submission_question)) + end + + context 'when the course generates no feedback' do + let(:workflow) { 'none' } + + it 'writes no comment, leaving the answer graded all the same' do + expect { service.create_ai_generated_draft_post }.not_to(change { Course::Discussion::Post.count }) + end + end + + context 'when the course drafts feedback for approval' do + let(:workflow) { 'draft' } + + it 'drafts the comment and leaves the topic pending' do + service.create_ai_generated_draft_post + + expect(post.workflow_state).to eq('draft') + expect(post.topic.pending_staff_reply).to be true + end + end + + context 'when the course publishes feedback on answer submission' do + let(:workflow) { 'publish_on_answer_submit' } + + it 'publishes the comment without queueing it for staff' do + service.create_ai_generated_draft_post + + expect(post.workflow_state).to eq('published') + expect(post.topic.pending_staff_reply).to be false + end + + it 'still snapshots the generated text, leaving the rating unscored' do + evaluation = Course::Rubric::AnswerEvaluation.create!(answer: answer, evaluation_type: :grading, + feedback: 'generated feedback') + service.create_ai_generated_draft_post + + rating = evaluation.ratings.last + expect(rating.original_feedback).to eq('generated feedback') + expect(rating.rating).to be_nil + end + + it 'drafts instead when the generation is staff-initiated' do + described_class.new(answer, 'generated feedback', force_draft: true).create_ai_generated_draft_post + + expect(post.workflow_state).to eq('draft') + expect(post.topic.pending_staff_reply).to be true + end + end + + context 'when the course publishes feedback on finalisation' do + let(:workflow) { 'publish_on_finalise' } + + it 'holds the comment back while the submission is still being attempted' do + service.create_ai_generated_draft_post + + expect(post.workflow_state).to eq('draft') + end + + context 'once the submission has been finalised' do + let(:submission) { create(:submission, :submitted, assessment: assessment) } + + it 'publishes the comment as it is generated' do + service.create_ai_generated_draft_post + + expect(post.workflow_state).to eq('published') + expect(post.topic.pending_staff_reply).to be false + end + end + end + end + describe 'AI feedback rating lifecycle' do let(:submission_question) do create(:course_assessment_submission_question, submission: submission, question: question.acting_as)