From da0163157ebc82fa79d0b67542cb6201c56ef2a9 Mon Sep 17 00:00:00 2001 From: adi-herwana-nus Date: Fri, 25 Sep 2026 16:55:12 +0800 Subject: [PATCH 1/5] feat(rubric-grading): add support for auto-publish AI feedback - settings between publish immediately on answer, publish on finalise, generate draft (current, default), no feedback --- .../admin/assessment_settings_controller.rb | 1 + .../course/rubric/apply_evaluations_job.rb | 5 +- app/models/course.rb | 21 +++++ app/models/course/assessment/submission.rb | 26 ++++++ .../answer/ai_generated_post_service.rb | 60 ++++++++++++-- .../assessment_settings/edit.json.jbuilder | 3 + .../AssessmentSettingsForm.tsx | 46 ++++++++++- .../pages/AssessmentSettings/operations.ts | 1 + .../pages/AssessmentSettings/translations.ts | 25 ++++++ .../pages/CodaveriSettings/translations.ts | 3 +- client/app/types/course/admin/assessments.ts | 3 + client/locales/en.json | 22 ++++- client/locales/ko.json | 22 ++++- client/locales/zh.json | 22 ++++- .../assessment_settings_controller_spec.rb | 22 +++++ .../course/assessment/submission_spec.rb | 55 +++++++++++++ spec/models/course_spec.rb | 22 +++++ .../answer/ai_generated_post_service_spec.rb | 80 +++++++++++++++++++ 18 files changed, 424 insertions(+), 15 deletions(-) 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/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/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< /> )} + + ( + + + + + + + + + + )} + /> + '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_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) From aefc4ae30610a264a2efc7e4244d6dba46d0f1df Mon Sep 17 00:00:00 2001 From: adi-herwana-nus Date: Fri, 25 Sep 2026 18:04:26 +0800 Subject: [PATCH 2/5] refactor(rubric-grading): remove auto_gradable status mask and refine payload actions - AUTOGRADE_RUBRIC_SUCCESS can carry comments or grades, or both - published immediately comments render without refresh - fix feedback rating leakage to students --- .../course/assessment/questions_concern.rb | 8 +- .../_ai_generated_comment.json.jbuilder | 15 ++++ .../answer/_rubric_explanation.json.jbuilder | 13 +++ .../_forum_post_response.json.jbuilder | 6 +- .../_rubric_based_response.json.jbuilder | 20 +---- .../_rubric_based_response.json.jbuilder | 2 +- .../submissions/_question.json.jbuilder | 2 + .../discussion/posts/_post.json.jbuilder | 5 +- .../forum/posts/_post_list_data.json.jbuilder | 5 +- .../assessment/submission/actions/index.js | 8 +- .../answers/RubricBasedResponse/index.tsx | 7 +- .../adapters/RubricBasedResponseAdapter.tsx | 1 - .../components/ExplanationPanel.tsx | 9 +- .../submission/reducers/gradingResults.ts | 7 +- .../assessment/submission/question/types.ts | 2 + .../submission/submissions_controller_spec.rb | 88 +++++++++++++++++++ .../course/forum/topics_controller_spec.rb | 29 ++++++ spec/models/course/assessment_spec.rb | 23 +++++ 18 files changed, 212 insertions(+), 38 deletions(-) create mode 100644 app/views/course/assessment/answer/_ai_generated_comment.json.jbuilder create mode 100644 app/views/course/assessment/answer/_rubric_explanation.json.jbuilder 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/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/assessment/submission/actions/index.js b/client/app/bundles/course/assessment/submission/actions/index.js index 01db2cc813c..407d0bd9f7e 100644 --- a/client/app/bundles/course/assessment/submission/actions/index.js +++ b/client/app/bundles/course/assessment/submission/actions/index.js @@ -28,9 +28,11 @@ export function getEvaluationResult(submissionId, answerId, questionId) { type: actionTypes.AUTOGRADE_SUCCESS, payload: { ...data, answerId }, }); - // Any rubric-graded answer (RBR, forum post, ...) carries a category breakdown; refresh its rubric - // reference grade + categories so the moderation resets to the autograder's result. - if (data.categoryGrades) { + // A rubric-graded answer (RBR, forum post, ...) may return its category breakdown, which refreshes the + // rubric reference grade + categories so the moderation resets to the autograder's result, and/or the + // AI feedback comment grading just produced. Either may be absent: students do not see the breakdown + // until the submission is published, but may see a published comment straight away. + if (data.categoryGrades || data.aiGeneratedComment) { dispatch({ type: actionTypes.AUTOGRADE_RUBRIC_SUCCESS, payload: { diff --git a/client/app/bundles/course/assessment/submission/components/answers/RubricBasedResponse/index.tsx b/client/app/bundles/course/assessment/submission/components/answers/RubricBasedResponse/index.tsx index a061c246b74..8e6aecdeddc 100644 --- a/client/app/bundles/course/assessment/submission/components/answers/RubricBasedResponse/index.tsx +++ b/client/app/bundles/course/assessment/submission/components/answers/RubricBasedResponse/index.tsx @@ -1,13 +1,11 @@ import { FC } from 'react'; import { Controller, useFormContext } from 'react-hook-form'; -import { SubmissionQuestionData } from 'types/course/assessment/submission/question/types'; import UserHTMLText from 'lib/components/core/UserHTMLText'; import FormRichTextField from 'lib/components/form/fields/RichTextField'; interface RubricBasedResponseAnswerProps { answerId: number; - question: SubmissionQuestionData<'RubricBasedResponse'>; 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/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/gradingResults.ts b/client/app/bundles/course/assessment/submission/reducers/gradingResults.ts index 0db7a90132d..44b776d3641 100644 --- a/client/app/bundles/course/assessment/submission/reducers/gradingResults.ts +++ b/client/app/bundles/course/assessment/submission/reducers/gradingResults.ts @@ -56,10 +56,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 { @@ -163,6 +164,8 @@ export default createReducer( ].includes(action.type); }, (state, action) => { + if (!action.payload.categoryGrades) return; + state.categoryGrades[action.payload.questionId] = action.payload.categoryGrades; }, 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/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/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 From c586eecedd323980087b0affb5fad50ea0578331 Mon Sep 17 00:00:00 2001 From: adi-herwana-nus Date: Fri, 25 Sep 2026 18:22:14 +0800 Subject: [PATCH 3/5] style(submissions): rename javascript -> typescript converted files --- .../components/comment/{CommentCard.jsx => CommentCard.tsx} | 0 .../submission/containers/{Comments.jsx => Comments.tsx} | 0 .../submission/reducers/{commentForms.js => commentForms.ts} | 0 .../course/assessment/submission/reducers/{posts.js => posts.ts} | 0 .../assessment/submission/reducers/{topics.js => topics.ts} | 0 5 files changed, 0 insertions(+), 0 deletions(-) rename client/app/bundles/course/assessment/submission/components/comment/{CommentCard.jsx => CommentCard.tsx} (100%) rename client/app/bundles/course/assessment/submission/containers/{Comments.jsx => Comments.tsx} (100%) rename client/app/bundles/course/assessment/submission/reducers/{commentForms.js => commentForms.ts} (100%) rename client/app/bundles/course/assessment/submission/reducers/{posts.js => posts.ts} (100%) rename client/app/bundles/course/assessment/submission/reducers/{topics.js => topics.ts} (100%) diff --git a/client/app/bundles/course/assessment/submission/components/comment/CommentCard.jsx b/client/app/bundles/course/assessment/submission/components/comment/CommentCard.tsx similarity index 100% rename from client/app/bundles/course/assessment/submission/components/comment/CommentCard.jsx rename to client/app/bundles/course/assessment/submission/components/comment/CommentCard.tsx diff --git a/client/app/bundles/course/assessment/submission/containers/Comments.jsx b/client/app/bundles/course/assessment/submission/containers/Comments.tsx similarity index 100% rename from client/app/bundles/course/assessment/submission/containers/Comments.jsx rename to client/app/bundles/course/assessment/submission/containers/Comments.tsx diff --git a/client/app/bundles/course/assessment/submission/reducers/commentForms.js b/client/app/bundles/course/assessment/submission/reducers/commentForms.ts similarity index 100% rename from client/app/bundles/course/assessment/submission/reducers/commentForms.js rename to client/app/bundles/course/assessment/submission/reducers/commentForms.ts diff --git a/client/app/bundles/course/assessment/submission/reducers/posts.js b/client/app/bundles/course/assessment/submission/reducers/posts.ts similarity index 100% rename from client/app/bundles/course/assessment/submission/reducers/posts.js rename to client/app/bundles/course/assessment/submission/reducers/posts.ts diff --git a/client/app/bundles/course/assessment/submission/reducers/topics.js b/client/app/bundles/course/assessment/submission/reducers/topics.ts similarity index 100% rename from client/app/bundles/course/assessment/submission/reducers/topics.js rename to client/app/bundles/course/assessment/submission/reducers/topics.ts From f43a7751350a5e97ea9af90bf6dca5a7a46317e9 Mon Sep 17 00:00:00 2001 From: adi-herwana-nus Date: Fri, 25 Sep 2026 18:27:03 +0800 Subject: [PATCH 4/5] refactor(submissions): convert posts and topics reducers to TypeScript --- .../reducers/__test__/posts.test.ts | 70 ++++++++++ .../reducers/__test__/topics.test.ts | 69 ++++++++++ .../submission/reducers/gradingResults.ts | 73 ++++------ .../assessment/submission/reducers/posts.ts | 127 +++++++++++++----- .../assessment/submission/reducers/topics.ts | 119 ++++++++++------ .../course/assessment/submission/types.ts | 6 +- .../course/assessment/utils/matchers.ts | 7 + 7 files changed, 344 insertions(+), 127 deletions(-) create mode 100644 client/app/bundles/course/assessment/submission/reducers/__test__/posts.test.ts create mode 100644 client/app/bundles/course/assessment/submission/reducers/__test__/topics.test.ts create mode 100644 client/app/bundles/course/assessment/utils/matchers.ts 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/gradingResults.ts b/client/app/bundles/course/assessment/submission/reducers/gradingResults.ts index 44b776d3641..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'; @@ -68,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: {}, @@ -93,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 = {}; @@ -131,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 ( @@ -157,12 +137,10 @@ 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; @@ -172,11 +150,10 @@ export default createReducer( ); 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.ts b/client/app/bundles/course/assessment/submission/reducers/posts.ts index 0c76dbaefca..49a703728c8 100644 --- a/client/app/bundles/course/assessment/submission/reducers/posts.ts +++ b/client/app/bundles/course/assessment/submission/reducers/posts.ts @@ -1,40 +1,95 @@ -import { arrayToObjectWithKey } from 'utilities/array'; +// 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 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; - } +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.ts b/client/app/bundles/course/assessment/submission/reducers/topics.ts index d86cecfe6e7..1178da15b6f 100644 --- a/client/app/bundles/course/assessment/submission/reducers/topics.ts +++ b/client/app/bundles/course/assessment/submission/reducers/topics.ts @@ -1,47 +1,82 @@ -import { arrayToObjectWithKey } from 'utilities/array'; +// 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'; -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], +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; + }); }, - }; - } - case actions.DELETE_COMMENT_SUCCESS: { - const { topicId, postId } = action.payload; - return { - ...state, - [topicId]: { - ...state[topicId], - postIds: state[topicId].postIds.filter((id) => id !== postId), + ) + .addMatcher( + isOneOf(actions.CREATE_COMMENT_SUCCESS), + (state, action) => { + addPost(state, action.payload); }, - }; - } - default: - return state; - } -} + ) + .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/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); From 73d2caa272b974537b4c7cc6d030fd6dda6baecd Mon Sep 17 00:00:00 2001 From: adi-herwana-nus Date: Sat, 26 Sep 2026 18:37:13 +0800 Subject: [PATCH 5/5] refactor(comments): refactor comment UI to TypeScript --- .../comment/AiFeedbackCommentCard.tsx | 16 +- .../components/comment/CommentCard.tsx | 332 ++++++++--------- .../submission/containers/Comments.tsx | 282 +++++++------- .../reducers/__test__/commentForms.test.ts | 87 +++++ .../submission/reducers/commentForms.ts | 344 +++++++++++------- .../submission/selectors/commentForms.ts | 6 + .../submission/selectors/comments.ts | 4 + .../submission/manually_graded_spec.rb | 2 +- 8 files changed, 582 insertions(+), 491 deletions(-) create mode 100644 client/app/bundles/course/assessment/submission/reducers/__test__/commentForms.test.ts create mode 100644 client/app/bundles/course/assessment/submission/selectors/commentForms.ts 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.tsx b/client/app/bundles/course/assessment/submission/components/comment/CommentCard.tsx index dd473f7a613..a3b4f77be89 100644 --- a/client/app/bundles/course/assessment/submission/components/comment/CommentCard.tsx +++ b/client/app/bundles/course/assessment/submission/components/comment/CommentCard.tsx @@ -1,20 +1,17 @@ -import { Component } from 'react'; +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 PropTypes from 'prop-types'; +import { CommentPostMiniEntity } from 'types/course/comments'; 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', @@ -72,203 +69,158 @@ const styles = { 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 }); - } +} 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; +} - toggleEditMode() { - const { editMode } = this.state; - const { - handleChange, - post: { text }, - } = this.props; - this.setState({ editMode: !editMode }); +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); - } + }; - renderCommentContent() { - const { editMode } = this.state; - const { - editValue, - post: { text, id, workflowState }, - } = this.props; + const save = (): void => { + updateComment(editValue); + setEditMode(false); + }; - const isDraft = workflowState === POST_WORKFLOW_STATE.draft; - - if (editMode) { - return ( - <> - this.onChange(nextValue)} - value={editValue} - /> -
- - {isDraft ? ( - - ) : ( - - )} -
- - ); - } + const publish = (): void => { + publishComment?.(editValue); + setEditMode(false); + }; - 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; + const renderCommentContent = (): JSX.Element => { + if (!editMode) return ; 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} -
+ <> + +
+ + {isDraft ? ( + + ) : ( + + )}
-
{this.renderCommentContent()}
- } - onCancel={() => this.setState({ deleteConfirmation: false })} - onConfirm={() => this.onConfirmDelete()} - open={this.state.deleteConfirmation} + + ); + }; + + 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} +
- ); - } -} - -CommentCard.propTypes = { - post: postShape.isRequired, - editValue: PropTypes.string, - - handleChange: PropTypes.func, - updateComment: PropTypes.func, - deleteComment: PropTypes.func, - publishComment: PropTypes.func, - isUpdatingAnnotationAllowed: PropTypes.bool, +
{renderCommentContent()}
+
+ ); }; + +export default CommentCard; diff --git a/client/app/bundles/course/assessment/submission/containers/Comments.tsx b/client/app/bundles/course/assessment/submission/containers/Comments.tsx index 413ba2e9f5d..830b37ef6e1 100644 --- a/client/app/bundles/course/assessment/submission/containers/Comments.tsx +++ b/client/app/bundles/course/assessment/submission/containers/Comments.tsx @@ -1,10 +1,9 @@ -import { Component } from 'react'; +import { FC } 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 { useAppDispatch, useAppSelector } from 'lib/hooks/store'; import toast from 'lib/hooks/toast'; import * as commentActions from '../actions/comments'; @@ -12,170 +11,145 @@ 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 { 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'; -class VisibleComments extends Component { - static newCommentIdentifier(field) { - return `topic_${field}`; - } +// DOM id of the new-comment box, which the submission feature specs locate it by. +const newCommentIdentifier = (topicId: number): string => `topic_${topicId}`; - render() { - const { - commentForms, - posts, - topic, - handleCreateChange, - handleUpdateChange, - createComment, - updateComment, - deleteComment, - publishComment, - rateComment, - acceptComment, - rejectComment, - graderView, - renderDelayedCommentButton, - } = this.props; +interface Props { + topic: Topic; +} + +// The comment thread under one submission question. +const Comments: FC = ({ topic }) => { + const dispatch = useAppDispatch(); - return ( -
- - - + 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); - {posts.map((post) => { - const isVisible = - graderView || - (!post.isDelayed && - post.workflowState !== POST_WORKFLOW_STATE.draft); - if (!isVisible) { - return null; - } + 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.'); + }); - // 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 ( +
+ + + + {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 ( - deleteComment(post.id)} - editValue={commentForms.posts[post.id]} - handleChange={(value) => handleUpdateChange(post.id, value)} - isUpdatingAnnotationAllowed + // 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} - publishComment={(value) => publishComment(post.id, value)} - updateComment={(value) => updateComment(post.id, value)} + rateComment={(rating) => + dispatch( + commentActions.rateAiFeedback(topic.id, post.id, rating), + ) + } + rejectComment={(value) => + dispatch( + commentActions.rejectAiFeedback(topic.id, 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, + 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 ?? '')) + } + /> + ); + })} - 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, + + 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]} + /> +
+ ); }; -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/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/commentForms.ts b/client/app/bundles/course/assessment/submission/reducers/commentForms.ts index 16599d241d2..20d00541b9d 100644 --- a/client/app/bundles/course/assessment/submission/reducers/commentForms.ts +++ b/client/app/bundles/course/assessment/submission/reducers/commentForms.ts @@ -1,152 +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'; -const initialState = { +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, - annotationsDelayedComment: {}, + isUpdatingComment: false, topics: {}, posts: {}, + annotations: {}, }; -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]: '' }), - {}, +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, ), - annotations: action.payload.annotations.reduce( - (obj, annotation) => ({ ...obj, [annotation.fileId]: {} }), - {}, + (state, action) => { + state.posts[action.payload.postId] = action.payload.text; + }, + ) + .addMatcher( + isOneOf( + actions.UPDATE_COMMENT_SUCCESS, + actions.UPDATE_ANNOTATION_SUCCESS, ), - }; - case actions.CREATE_ANNOTATION_CHANGE: { - const { fileId, line, text } = action.payload; - return { - ...state, - annotations: { - ...state.annotations, - [fileId]: { ...state.annotations[fileId], [line]: text }, + (state, action) => { + state.isUpdatingComment = false; + state.posts[action.payload.id] = action.payload.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]: '', - }, + ) + .addMatcher( + isOneOf( + actions.DELETE_COMMENT_SUCCESS, + actions.DELETE_ANNOTATION_SUCCESS, + ), + (state, action) => { + delete state.posts[action.payload.postId]; }, - }; - } - case actions.CREATE_COMMENT_CHANGE: { - const { topicId, text } = action.payload; - return { - ...state, - topics: { - ...state.topics, - [topicId]: text, + ) + .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; }, - }; - } - case actions.CREATE_COMMENT_SUCCESS: { - const { topicId } = action.payload; - return { - ...state, - isSubmittingNormalComment: false, - isSubmittingDelayedComment: false, - topics: { - ...state.topics, - [topicId]: '', + ) + .addMatcher( + isOneOf( + actions.CREATE_COMMENT_FAILURE, + actions.CREATE_ANNOTATION_FAILURE, + ), + (state) => { + state.isSubmittingNormalComment = false; + state.isSubmittingDelayedComment = false; }, - }; - } - case actions.UPDATE_ANNOTATION_CHANGE: - case actions.UPDATE_COMMENT_CHANGE: { - const { postId, text } = action.payload; - return { - ...state, - posts: { - ...state.posts, - [postId]: text, + ) + .addMatcher( + isOneOf( + actions.UPDATE_COMMENT_FAILURE, + actions.UPDATE_ANNOTATION_FAILURE, + ), + (state) => { + state.isUpdatingComment = false; }, - }; - } - 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, + ) + .addMatcher( + isOneOf(actions.AUTOGRADE_SUCCESS), + (state, action) => { + action.payload.latestAnswer?.annotations?.forEach((annotation) => { + state.annotations[annotation.fileId] = {}; + }); }, - }; - } - 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; - } -} + ); + }, +}); + +export default commentFormsSlice.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/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