diff --git a/app/controllers/course/assessment/question/programming_controller.rb b/app/controllers/course/assessment/question/programming_controller.rb index 1112ca4cc60..15b5c0b2b90 100644 --- a/app/controllers/course/assessment/question/programming_controller.rb +++ b/app/controllers/course/assessment/question/programming_controller.rb @@ -19,6 +19,7 @@ def new def create @programming_question.package_type = programming_question_params.key?(:file) ? :zip_upload : :online_editor + @programming_question.current = @programming_question process_package if @programming_question.save @@ -41,6 +42,27 @@ def edit def update result = @programming_question.class.transaction do + old_update_timestamp = @programming_question.snapshot_of_state_at + + # Duplicate the original question as a snapshot + snapshot = @programming_question.dup + snapshot.current = @programming_question + + snapshot.template_files = @programming_question.template_files.map do |template_file| + duplicated_template_file = template_file.dup + duplicated_template_file.question = snapshot + duplicated_template_file + end + + snapshot.test_cases = @programming_question.test_cases.map do |test_case| + duplicated_test_case = test_case.dup + duplicated_test_case.question = snapshot + + # Test case results aren't duplicated by default, so we do that now + duplicated_test_case.test_results = test_case.test_results.map(&:dup) if test_case.test_results.any? + duplicated_test_case + end + @question_assessment.skill_ids = programming_question_params[:question_assessment]. try(:[], :skill_ids) @programming_question.assign_attributes(programming_question_params. @@ -48,8 +70,34 @@ def update @programming_question.is_synced_with_codaveri = false process_package + update_timestamp = Time.current + @programming_question.updated_at = update_timestamp + @programming_question.snapshot_of_state_at = update_timestamp + @programming_question.snapshot_index = @programming_question.snapshot_index + 1 + raise ActiveRecord::Rollback unless @programming_question.save + if @programming_question.should_create_snapshot? + @programming_question.update_column(:import_job_id, nil) # maintains uniqueness constraint + snapshot.skip_process_package = true + snapshot.save! + + update_result = ActiveRecord::Base.connection.execute(<<-SQL.squish + UPDATE course_assessment_answer_programming_auto_gradings + SET question_snapshot_id = #{snapshot.id} + FROM course_assessment_answer_auto_gradings, course_assessment_answers, course_assessment_questions + WHERE course_assessment_answer_programming_auto_gradings.id = course_assessment_answer_auto_gradings.actable_id + AND course_assessment_answer_auto_gradings.answer_id = course_assessment_answers.id + AND course_assessment_questions.id = course_assessment_answers.question_id + AND course_assessment_questions.actable_id = #{@programming_question.id} + AND course_assessment_questions.actable_type = 'Course::Assessment::Question::Programming' + AND (course_assessment_answer_programming_auto_gradings.question_snapshot_id IS NULL + OR course_assessment_answer_programming_auto_gradings.question_snapshot_id = #{@programming_question.id}) + SQL + ) + + end + true end diff --git a/app/controllers/course/assessment/submission/answer/answers_controller.rb b/app/controllers/course/assessment/submission/answer/answers_controller.rb index 8f204621fae..1a65b79ff63 100644 --- a/app/controllers/course/assessment/submission/answer/answers_controller.rb +++ b/app/controllers/course/assessment/submission/answer/answers_controller.rb @@ -74,8 +74,8 @@ def last_attempt_answer_submitted_job(answer) attempts = submission.answers.from_question(answer.question_id) last_non_current_answer = attempts.reject(&:current_answer?).last - job = last_non_current_answer&.auto_grading&.job - job&.status == 'submitted' ? job : nil + jobs = last_non_current_answer&.auto_gradings&.map(&:job)&.compact&.select { |j| j.status == 'submitted' } + jobs&.first end def reattempt_and_grade_answer(answer) @@ -87,7 +87,8 @@ def reattempt_and_grade_answer(answer) # so destroy the failed job answer and re-grade the current entry. answer.class.transaction do last_answer = answer.submission.answers.select { |ans| ans.question_id == answer.question_id }.last - last_answer.destroy! if last_answer&.auto_grading&.job&.errored? + p({ laag: last_answer&.auto_gradings, should_destroy: last_answer&.auto_gradings&.any? { |ag| ag.job&.errored? } }) + last_answer.destroy! if last_answer&.auto_gradings&.any? { |ag| ag.job&.errored? } new_answer = reattempt_answer(answer, finalise: true) new_answer.auto_grade!(redirect_to_path: nil, reduce_priority: false) end diff --git a/app/controllers/course/assessment/submission/submissions_controller.rb b/app/controllers/course/assessment/submission/submissions_controller.rb index 5016818176e..1907ea3659d 100644 --- a/app/controllers/course/assessment/submission/submissions_controller.rb +++ b/app/controllers/course/assessment/submission/submissions_controller.rb @@ -371,12 +371,12 @@ def check_zombie_jobs # rubocop:disable Metrics/AbcSize, Metrics/PerceivedComple return if submitted_answers.empty? dead_answers = submitted_answers.select do |a| - job = a.auto_grading&.job + job = a.auto_gradings&.last&.job job&.submitted? && !job.in_queue? end dead_answers.each do |a| - old_job = a.auto_grading.job + old_job = a.auto_gradings&.last&.job job = a.auto_grade!(redirect_to_path: old_job.redirect_to, reduce_priority: true) logger.debug(message: 'Restart Answer Grading', answer_id: a.id, job_id: job.job.id, diff --git a/app/helpers/course/assessment/answer/programming_test_case_helper.rb b/app/helpers/course/assessment/answer/programming_test_case_helper.rb index 94d07cd22ed..4e90008f8f1 100644 --- a/app/helpers/course/assessment/answer/programming_test_case_helper.rb +++ b/app/helpers/course/assessment/answer/programming_test_case_helper.rb @@ -32,12 +32,13 @@ def get_output(test_case_result) # # @param [Hash] test_cases_by_type The test cases and their results keyed by type # @return [Hash] Failed test case and its result, if any - def get_failed_test_cases_by_type(test_cases_and_results) - {}.tap do |result| - test_cases_and_results.each do |test_case_type, test_cases_and_results_of_type| - result[test_case_type] = get_first_failed_test(test_cases_and_results_of_type) - end - end + def get_first_test_failures_by_type(test_cases_by_type, test_results_by_type) + test_cases_by_type.entries.map do |test_case_type, test_cases| + [ + test_case_type, + test_cases.find { |test_case| test_results_by_type[test_case.id]&.passed? } + ] + end.to_h end # Organize the test cases and test results into a hash, keyed by test case type. @@ -51,12 +52,13 @@ def get_failed_test_cases_by_type(test_cases_and_results) # @param [Hash] test_cases_by_type The test cases keyed by type # @param [Course::Assessment::Answer::ProgrammingAutoGrading] auto_grading Auto grading object # @return [Hash] The hash structure described above - def get_test_cases_and_results(test_cases_by_type, auto_grading) - results_hash = auto_grading ? auto_grading.test_results.includes(:test_case).group_by(&:test_case) : {} - test_cases_by_type.each do |type, test_cases| - test_cases_by_type[type] = - test_cases.map { |test_case| [test_case, results_hash[test_case]&.first] }. - sort_by { |test_case, _| test_case.identifier }.to_h + def get_test_results_by_type(test_cases_by_type, auto_grading) + results_hash = auto_grading ? auto_grading.test_results.group_by(&:test_case_id) : {} + test_cases_by_type.transform_values do |test_cases| + test_cases.map do |test_case| + result = results_hash[test_case.id].first + [test_case, result] + end.to_h end end diff --git a/app/jobs/course/assessment/answer/base_auto_grading_job.rb b/app/jobs/course/assessment/answer/base_auto_grading_job.rb index f4fb54b3b0f..aa86880b18d 100644 --- a/app/jobs/course/assessment/answer/base_auto_grading_job.rb +++ b/app/jobs/course/assessment/answer/base_auto_grading_job.rb @@ -34,13 +34,14 @@ def delayed_queue_name # @param [String|nil] redirect_to_path The path to be redirected after auto grading job was # finished. # @param [Course::Assessment::Answer] answer the answer to be graded. + # @param [Course::Assessment::AutoGrading] The auto grading result to save the results to. # @param [String] redirect_to_path The path to redirect when job finishes. - def perform_tracked(answer, redirect_to_path = nil) + def perform_tracked(answer, auto_grading, redirect_to_path = nil) ActsAsTenant.without_tenant do raise PriorityShouldBeLoweredError if !queue_name.include?('delayed') && answer.question.is_low_priority downgrade_if_timeout(answer.question) do - Course::Assessment::Answer::AutoGradingService.grade(answer) + Course::Assessment::Answer::AutoGradingService.grade(answer, auto_grading) end if update_exp?(answer.submission) diff --git a/app/models/course/assessment/answer.rb b/app/models/course/assessment/answer.rb index 478a08031fa..1850e7548ee 100644 --- a/app/models/course/assessment/answer.rb +++ b/app/models/course/assessment/answer.rb @@ -55,8 +55,9 @@ class Course::Assessment::Answer < ApplicationRecord belongs_to :submission, inverse_of: :answers belongs_to :question, class_name: 'Course::Assessment::Question', inverse_of: nil belongs_to :grader, class_name: 'User', inverse_of: nil, optional: true - has_one :auto_grading, class_name: 'Course::Assessment::Answer::AutoGrading', - dependent: :destroy, inverse_of: :answer, autosave: true + has_many :auto_gradings, -> { order(:created_at) }, + class_name: 'Course::Assessment::Answer::AutoGrading', + dependent: :destroy, inverse_of: :answer, autosave: true accepts_nested_attributes_for :actable @@ -80,13 +81,18 @@ class Course::Assessment::Answer < ApplicationRecord def auto_grade!(redirect_to_path: nil, reduce_priority: false) raise IllegalStateError if attempting? - ensure_auto_grading! + p({ ssq: self.question, ssq_is_saving_snapshots: self.question.is_saving_snapshots? }) + auto_grading = if self.question.is_saving_snapshots? + Course::Assessment::Answer::AutoGrading.create!(answer: self) + else + ensure_auto_grading! + end if grade_inline? - Course::Assessment::Answer::AutoGradingService.grade(self) + Course::Assessment::Answer::AutoGradingService.grade(self, auto_grading) nil else auto_grading_job_class(reduce_priority). - perform_later(self, redirect_to_path).tap do |job| + perform_later(self, auto_grading, redirect_to_path).tap do |job| auto_grading.update_column(:job_id, job.job_id) end end @@ -189,23 +195,9 @@ def validate_grade # Ensures that an auto grading record exists for this answer. # - # Use this to guarantee that an auto grading record exists, and retrieves it. This is because - # there can be a concurrent creation of such a record across two processes, and this can only - # be detected at the database level. - # - # The additional transaction is in place because a RecordNotUnique will cause the active - # transaction to be considered as errored, and needing a rollback. - # # @return [Course::Assessment::Answer::AutoGrading] def ensure_auto_grading! - ActiveRecord::Base.transaction(requires_new: true) do - auto_grading || create_auto_grading! - end - rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotUnique => e - raise e if e.is_a?(ActiveRecord::RecordInvalid) && e.record.errors[:answer_id].empty? - - association(:auto_grading).reload - auto_grading + auto_gradings&.last || Course::Assessment::Answer::AutoGrading.create!(answer: self) end def unsubmit @@ -213,7 +205,7 @@ def unsubmit self.grader = nil self.graded_at = nil self.submitted_at = nil - auto_grading&.mark_for_destruction + auto_gradings.map(&:mark_for_destruction) end def auto_grading_job_class(reduce_priority) diff --git a/app/models/course/assessment/answer/auto_grading.rb b/app/models/course/assessment/answer/auto_grading.rb index 00f00ec96b2..d9d18fc062e 100644 --- a/app/models/course/assessment/answer/auto_grading.rb +++ b/app/models/course/assessment/answer/auto_grading.rb @@ -4,14 +4,13 @@ class Course::Assessment::Answer::AutoGrading < ApplicationRecord validates :actable_type, length: { maximum: 255 }, allow_nil: true validates :answer, presence: true - validates :answer_id, uniqueness: { if: :answer_id_changed? }, allow_nil: true validates :job_id, uniqueness: { if: :job_id_changed? }, allow_nil: true validates :actable_type, uniqueness: { scope: [:actable_id], allow_nil: true, if: -> { actable_id? && actable_type_changed? } } validates :actable_id, uniqueness: { scope: [:actable_type], allow_nil: true, if: -> { actable_type? && actable_id_changed? } } - belongs_to :answer, class_name: 'Course::Assessment::Answer', inverse_of: :auto_grading + belongs_to :answer, class_name: 'Course::Assessment::Answer', inverse_of: :auto_gradings # @!attribute [r] job # This might be null if the job has been cleared. belongs_to :job, class_name: 'TrackableJob::Job', inverse_of: nil, optional: true diff --git a/app/models/course/assessment/answer/programming_auto_grading.rb b/app/models/course/assessment/answer/programming_auto_grading.rb index 85b3d3b4e49..fee6f200ee3 100644 --- a/app/models/course/assessment/answer/programming_auto_grading.rb +++ b/app/models/course/assessment/answer/programming_auto_grading.rb @@ -14,6 +14,11 @@ class Course::Assessment::Answer::ProgrammingAutoGrading < ApplicationRecord class_name: 'Course::Assessment::Answer::ProgrammingAutoGradingTestResult', foreign_key: :auto_grading_id, inverse_of: :auto_grading, dependent: :destroy + + belongs_to :question_snapshot, + class_name: 'Course::Assessment::Question::Programming', + foreign_key: :question_snapshot_id, + optional: true private diff --git a/app/models/course/assessment/question.rb b/app/models/course/assessment/question.rb index c4bb165fd8d..0b0f7e56042 100644 --- a/app/models/course/assessment/question.rb +++ b/app/models/course/assessment/question.rb @@ -31,6 +31,7 @@ class Course::Assessment::Question < ApplicationRecord delegate :to_partial_path, to: :actable delegate :question_type, to: :actable delegate :question_type_readable, to: :actable + delegate :is_saving_snapshots?, to: :actable, allow_nil: true # Checks if the given question is auto gradable. This defaults to false if the specific # question does not implement auto grading. If this returns true, +auto_grader+ is guaranteed diff --git a/app/models/course/assessment/question/forum_post_response.rb b/app/models/course/assessment/question/forum_post_response.rb index b37bf14549a..2693044172e 100644 --- a/app/models/course/assessment/question/forum_post_response.rb +++ b/app/models/course/assessment/question/forum_post_response.rb @@ -5,6 +5,10 @@ class Course::Assessment::Question::ForumPostResponse < ApplicationRecord validates :max_posts, presence: true, numericality: { only_integer: true } validate :allowable_max_post_count + def is_saving_snapshots? + false + end + def question_type 'ForumPostResponse' end diff --git a/app/models/course/assessment/question/multiple_response.rb b/app/models/course/assessment/question/multiple_response.rb index 3f19d02e8c6..876e6404462 100644 --- a/app/models/course/assessment/question/multiple_response.rb +++ b/app/models/course/assessment/question/multiple_response.rb @@ -20,6 +20,10 @@ class Course::Assessment::Question::MultipleResponse < ApplicationRecord # "any correct" allows it to have more than one correct answer. alias_method :multiple_choice?, :any_correct? + def is_saving_snapshots? + false + end + def auto_gradable? true end diff --git a/app/models/course/assessment/question/programming.rb b/app/models/course/assessment/question/programming.rb index 11aea92248c..d85874ca8d4 100644 --- a/app/models/course/assessment/question/programming.rb +++ b/app/models/course/assessment/question/programming.rb @@ -19,6 +19,8 @@ class Course::Assessment::Question::Programming < ApplicationRecord # rubocop:di acts_as :question, class_name: 'Course::Assessment::Question' + before_create :set_snapshot_attributes + after_destroy_commit :destroy_snapshots after_initialize :set_defaults before_save :process_package, unless: :skip_process_package? before_validation :assign_template_attributes @@ -45,6 +47,20 @@ class Course::Assessment::Question::Programming < ApplicationRecord # rubocop:di has_many :test_cases, class_name: 'Course::Assessment::Question::ProgrammingTestCase', dependent: :destroy, foreign_key: :question_id, inverse_of: :question + has_many :snapshots, class_name: 'Course::Assessment::Question::Programming', + foreign_key: :current_id, inverse_of: :current + + belongs_to :current, class_name: 'Course::Assessment::Question::Programming', + optional: true, inverse_of: :snapshots + + def should_create_snapshot? + !skip_process_package? + end + + def is_saving_snapshots? + true + end + def auto_gradable? !test_cases.empty? end @@ -100,7 +116,7 @@ def copy_template_files_to(answer) # # @return [Hash] A hash of the test cases keyed by test case type. def test_cases_by_type - test_cases.group_by(&:test_case_type) + test_cases.group_by(&:test_case_type).transform_values { |test_cases| test_cases.sort_by(&:identifier) } end def files_downloadable? @@ -167,6 +183,12 @@ def create_or_update_codaveri_problem private + def set_snapshot_attributes + self.current ||= self + self.snapshot_of_state_at ||= Time.current + self.snapshot_index ||= 0 + end + def set_defaults self.max_time_limit = DEFAULT_CPU_TIMEOUT self.skip_process_package = false @@ -262,6 +284,10 @@ def validate_codaveri_question 'Activate it in the course setting or switch this question into a non-codaveri type.') end end + + def destroy_snapshots + self.snapshots.where.not(id: self).destroy_all + end end def validate_language_enabled diff --git a/app/models/course/assessment/question/rubric_based_response.rb b/app/models/course/assessment/question/rubric_based_response.rb index bd0532f51d5..d7a9bf4f00d 100644 --- a/app/models/course/assessment/question/rubric_based_response.rb +++ b/app/models/course/assessment/question/rubric_based_response.rb @@ -21,6 +21,10 @@ def initialize_duplicate(duplicator, other) self.categories = duplicator.duplicate(other.categories) end + def is_saving_snapshots? + false + end + def auto_gradable? !categories.empty? && ai_grading_enabled? end diff --git a/app/models/course/assessment/question/scribing.rb b/app/models/course/assessment/question/scribing.rb index 664629dd474..7628cb49f53 100644 --- a/app/models/course/assessment/question/scribing.rb +++ b/app/models/course/assessment/question/scribing.rb @@ -3,6 +3,10 @@ class Course::Assessment::Question::Scribing < ApplicationRecord acts_as :question, class_name: 'Course::Assessment::Question' has_one_attachment + def is_saving_snapshots? + false + end + def to_partial_path 'course/assessment/question/scribing/scribing' end diff --git a/app/models/course/assessment/question/text_response.rb b/app/models/course/assessment/question/text_response.rb index c0531594a65..c92c3df014b 100644 --- a/app/models/course/assessment/question/text_response.rb +++ b/app/models/course/assessment/question/text_response.rb @@ -23,6 +23,10 @@ class Course::Assessment::Question::TextResponse < ApplicationRecord accepts_nested_attributes_for :groups, allow_destroy: true + def is_saving_snapshots? + false + end + def auto_gradable? if comprehension_question? groups.any?(&:auto_gradable_group?) diff --git a/app/models/course/assessment/question/voice_response.rb b/app/models/course/assessment/question/voice_response.rb index 41a09910a0a..9e90bdde4a3 100644 --- a/app/models/course/assessment/question/voice_response.rb +++ b/app/models/course/assessment/question/voice_response.rb @@ -2,6 +2,10 @@ class Course::Assessment::Question::VoiceResponse < ApplicationRecord acts_as :question, class_name: 'Course::Assessment::Question' + def is_saving_snapshots? + false + end + def attempt(submission, last_attempt = nil) answer = Course::Assessment::Answer::VoiceResponse.new(submission: submission, question: question) diff --git a/app/services/course/assessment/answer/auto_grading_service.rb b/app/services/course/assessment/answer/auto_grading_service.rb index ba497b8dd49..20ae4c701dd 100644 --- a/app/services/course/assessment/answer/auto_grading_service.rb +++ b/app/services/course/assessment/answer/auto_grading_service.rb @@ -5,9 +5,10 @@ class << self # +Course::Assessment::Answer::AutoGrading+ object. # # @param [Course::Assessment::Answer] answer The answer to be graded. - def grade(answer) + # @param [Course::Assessment::Answer::AutoGrading] auto_grading The auto grading object that will store the results. + def grade(answer, auto_grading) answer = if answer.question.auto_gradable? - pick_grader(answer.question).grade(answer) + pick_grader(answer.question).grade(answer, auto_grading) else assign_maximum_grade(answer) end @@ -19,7 +20,7 @@ def grade(answer) # Picks the grader to use for the given question. # # @param [Course::Assessment::Question] question The question that the needs to be graded. - # @return [Course::Assessment::Answer::AnswerAutoGraderService] The service object that can + # @return [Course::Assessment::Answer::AnswerAutoGradingService] The service object that can # grade this question. def pick_grader(question) question.auto_grader @@ -48,10 +49,11 @@ def assign_maximum_grade(answer) # and makes sure answer is in the correct state. # # @param [Course::Assessment::Answer] answer The answer to be graded. + # @param [Course::Assessment::Answer::AutoGrading] auto_grading The auto grading object that will store the results. # @return [Course::Assessment::Answer] The graded answer. Note that this answer is not persisted # yet. - def grade(answer) - grade = evaluate(answer) + def grade(answer, auto_grading) + grade = evaluate(answer, auto_grading) answer.evaluate! if answer.submission.assessment.autograded? @@ -66,8 +68,9 @@ def grade(answer) # subclasses. # # @param [Course::Assessment::Answer] answer The answer to be evaluated. + # @param [Course::Assessment::Answer::AutoGrading] auto_grading The auto grading object that will store the results. # @return [Integer] grade The grade of the answer. - def evaluate(_answer) + def evaluate(_answer, _auto_grading) raise 'Not Implemented' end end diff --git a/app/services/course/assessment/answer/multiple_response_auto_grading_service.rb b/app/services/course/assessment/answer/multiple_response_auto_grading_service.rb index 419ba97f917..6b1c5e556db 100644 --- a/app/services/course/assessment/answer/multiple_response_auto_grading_service.rb +++ b/app/services/course/assessment/answer/multiple_response_auto_grading_service.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true class Course::Assessment::Answer::MultipleResponseAutoGradingService < \ Course::Assessment::Answer::AutoGradingService - def evaluate(answer) + def evaluate(answer, auto_grading) answer.correct, grade, messages = evaluate_answer(answer.actable) - answer.auto_grading.result = { messages: messages } + auto_grading.result = { messages: messages } grade end diff --git a/app/services/course/assessment/answer/programming_auto_grading_service.rb b/app/services/course/assessment/answer/programming_auto_grading_service.rb index a53ab16c8f5..377cc58e0dc 100644 --- a/app/services/course/assessment/answer/programming_auto_grading_service.rb +++ b/app/services/course/assessment/answer/programming_auto_grading_service.rb @@ -1,9 +1,20 @@ # frozen_string_literal: true class Course::Assessment::Answer::ProgrammingAutoGradingService < \ Course::Assessment::Answer::AutoGradingService - def evaluate(answer) - answer.correct, grade, programming_auto_grading, = evaluate_answer(answer.actable) - programming_auto_grading.auto_grading = answer.auto_grading + def evaluate(answer, auto_grading) + # We pre-load the programming question, including test cases, + # to ensure consistency with at least one saved snapshot. + question = Course::Assessment::Answer.includes(question: {actable: [:test_cases ]}).find_by_id(answer.id).question.actable + + answer.correct, grade, programming_auto_grading, = evaluate_answer(answer.actable, question) + + # If the question was updated during the evaluation, we can still find + # our original question by comparing the updated_at timestamps. + programming_auto_grading.question_snapshot_id = + Course::Assessment::Question::Programming. + where(current_id: question.id, snapshot_index: question.snapshot_index).first&.id || question.id + programming_auto_grading.auto_grading = auto_grading + auto_grading.save! grade end @@ -14,9 +25,8 @@ def evaluate(answer) # @param [Course::Assessment::Answer::Programming] answer The answer specified by the student. # @return [Array<(Boolean, Integer, Course::Assessment::Answer::ProgrammingAutoGrading)>] The # correct status, grade and the programming auto grading record. - def evaluate_answer(answer) + def evaluate_answer(answer, question) course = answer.submission.assessment.course - question = answer.question.actable assessment = answer.submission.assessment question.max_time_limit = course.programming_max_time_limit question.attachment.open(binmode: true) do |temporary_file| diff --git a/app/services/course/assessment/answer/programming_codaveri_auto_grading_service.rb b/app/services/course/assessment/answer/programming_codaveri_auto_grading_service.rb index a418b7e65de..adc9bdc13b8 100644 --- a/app/services/course/assessment/answer/programming_codaveri_auto_grading_service.rb +++ b/app/services/course/assessment/answer/programming_codaveri_auto_grading_service.rb @@ -1,13 +1,21 @@ # frozen_string_literal: true class Course::Assessment::Answer::ProgrammingCodaveriAutoGradingService < Course::Assessment::Answer::AutoGradingService - def evaluate(answer) + def evaluate(answer, auto_grading) unless answer.submission.assessment.course.component_enabled?(Course::CodaveriComponent) raise CodaveriError, I18n.t('course.assessment.question.programming.question_type_codaveri_deactivated') end + # We pre-load the programming question, including test cases, + # to ensure consistency with at least one saved snapshot. + question = Course::Assessment::Answer.includes(question: {actable: [:test_cases ]}).find_by_id(answer.id).question.actable answer.correct, grade, programming_auto_grading, = evaluate_answer(answer.actable) - programming_auto_grading.auto_grading = answer.auto_grading + # If the question was updated during the evaluation, we can still find + # our original question by comparing the updated_at timestamps. + programming_auto_grading.question_snapshot_id = + Course::Assessment::Question::Programming. + where(current_id: question.id, snapshot_of_state_at: question.snapshot_of_state_at).first&.id || question.id + programming_auto_grading.auto_grading = auto_grading grade end diff --git a/app/services/course/assessment/answer/rubric_auto_grading_service.rb b/app/services/course/assessment/answer/rubric_auto_grading_service.rb index ea9a84c37d7..b1a3b0c308f 100644 --- a/app/services/course/assessment/answer/rubric_auto_grading_service.rb +++ b/app/services/course/assessment/answer/rubric_auto_grading_service.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true class Course::Assessment::Answer::RubricAutoGradingService < Course::Assessment::Answer::AutoGradingService - def evaluate(answer) + def evaluate(answer, auto_grading) answer.correct, grade, messages, feedback = evaluate_answer(answer.actable) - answer.auto_grading.result = { messages: messages } + auto_grading.result = { messages: messages } create_ai_generated_draft_post(answer, feedback) grade end diff --git a/app/services/course/assessment/answer/text_response_auto_grading_service.rb b/app/services/course/assessment/answer/text_response_auto_grading_service.rb index 10ffd6d3054..f6302fdf63b 100644 --- a/app/services/course/assessment/answer/text_response_auto_grading_service.rb +++ b/app/services/course/assessment/answer/text_response_auto_grading_service.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true class Course::Assessment::Answer::TextResponseAutoGradingService < \ Course::Assessment::Answer::AutoGradingService - def evaluate(answer) + def evaluate(answer, auto_grading) answer.correct, grade, messages = evaluate_answer(answer.actable) - answer.auto_grading.result = { messages: messages } + auto_grading.result = { messages: messages } grade end diff --git a/app/services/course/assessment/answer/text_response_comprehension_auto_grading_service.rb b/app/services/course/assessment/answer/text_response_comprehension_auto_grading_service.rb index 0d544bcdc49..6bf24bb0f84 100644 --- a/app/services/course/assessment/answer/text_response_comprehension_auto_grading_service.rb +++ b/app/services/course/assessment/answer/text_response_comprehension_auto_grading_service.rb @@ -2,9 +2,9 @@ require 'rwordnet' class Course::Assessment::Answer::TextResponseComprehensionAutoGradingService < \ Course::Assessment::Answer::AutoGradingService - def evaluate(answer) + def evaluate(answer, auto_grading) answer.correct, grade, messages = evaluate_answer(answer.actable) - answer.auto_grading.result = { messages: messages } + auto_grading.result = { messages: messages } grade end diff --git a/app/services/course/assessment/question/answers_evaluation_service.rb b/app/services/course/assessment/question/answers_evaluation_service.rb index 2a11e9e060f..d8065f97992 100644 --- a/app/services/course/assessment/question/answers_evaluation_service.rb +++ b/app/services/course/assessment/question/answers_evaluation_service.rb @@ -9,8 +9,23 @@ def initialize(question) end def call - @question.answers.without_attempting_state.find_each do |a| - a.auto_grade!(reduce_priority: true) + if @question.is_saving_snapshots? + # find_each queries objects in batches, the batching is based on the table primary key. + # In Rails 8, it will be possible to override the batch ordering using :cursor. + # For now, we have to manually query the latest_answer_ids first. + latest_answer_ids = @question.answers. + unscope(:order). + select('DISTINCT ON (submission_id) id'). + without_attempting_state. + order('submission_id ASC, created_at DESC, id ASC') + + Course::Assessment::Answer.where(id: latest_answer_ids).find_each do |a| + a.auto_grade!(reduce_priority: true) + end + else + @question.answers.without_attempting_state.find_each do |a| + a.auto_grade!(reduce_priority: true) + end end end end diff --git a/app/views/course/assessment/answer/multiple_responses/_multiple_response.json.jbuilder b/app/views/course/assessment/answer/multiple_responses/_multiple_response.json.jbuilder index 08f49a6b75c..4e6fd1ea67d 100644 --- a/app/views/course/assessment/answer/multiple_responses/_multiple_response.json.jbuilder +++ b/app/views/course/assessment/answer/multiple_responses/_multiple_response.json.jbuilder @@ -10,9 +10,9 @@ end last_attempt = last_attempt(answer) json.explanation do - if last_attempt&.auto_grading&.result + if last_attempt&.auto_gradings&.last&.result json.correct last_attempt.correct - json.explanations(last_attempt.auto_grading.result['messages'].map { |e| format_ckeditor_rich_text(e) }) + json.explanations(last_attempt.auto_gradings.last.result['messages'].map { |e| format_ckeditor_rich_text(e) }) end end diff --git a/app/views/course/assessment/answer/programming/_auto_grading.jbuilder b/app/views/course/assessment/answer/programming/_auto_grading.jbuilder new file mode 100644 index 00000000000..0420fa20b01 --- /dev/null +++ b/app/views/course/assessment/answer/programming/_auto_grading.jbuilder @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +json.id auto_grading.id +json.createdAt auto_grading.created_at.iso8601 +graded_snapshot = auto_grading.question_snapshot || question +json.gradedOnPastSnapshot graded_snapshot.current_id != graded_snapshot.id + +test_cases_by_type = graded_snapshot.test_cases_by_type +test_results_by_type = get_test_results_by_type(test_cases_by_type, auto_grading) + +show_stdout_and_stderr = (can_read_tests || current_course.show_stdout_and_stderr) && + auto_grading.exit_code != 0 + +job = auto_grading.job +if job + json.job do + json.path job_path(job) if job.submitted? + json.partial! "jobs/#{job.status}", job: job + end +end + +json.testCases do + displayed_test_case_types.each do |test_case_type| + json.set! test_case_type do + if test_cases_by_type[test_case_type].present? + json.array! test_cases_by_type[test_case_type] do |test_case| + json.partial! 'course/assessment/answer/programming/test_case', + test_case: test_case, + can_read_tests: can_read_tests + end + end + end + end +end + +json.testResults do + displayed_test_case_types.each do |test_case_type| + show_public = (test_case_type == 'public_test') && current_course.show_public_test_cases_output + show_testcase_outputs = can_read_tests || show_public + json.set! test_case_type do + if test_results_by_type[test_case_type].present? + test_results_by_type[test_case_type].entries.each do |test_case, test_result| + json.set! test_case.id do + json.partial! 'course/assessment/answer/programming/test_result', + test_result: test_result, + show_output: show_testcase_outputs + end + end + end + end + end +end + +json.(auto_grading, :stdout, :stderr) if show_stdout_and_stderr diff --git a/app/views/course/assessment/answer/programming/_programming.json.jbuilder b/app/views/course/assessment/answer/programming/_programming.json.jbuilder index be7e88ee0e9..26bb394f7b4 100644 --- a/app/views/course/assessment/answer/programming/_programming.json.jbuilder +++ b/app/views/course/assessment/answer/programming/_programming.json.jbuilder @@ -1,14 +1,21 @@ # frozen_string_literal: true submission = answer.submission assessment = submission.assessment -question = answer.question.specific # If a non current_answer is being loaded, use it instead of loading the last_attempt. is_current_answer = answer.current_answer? latest_answer = last_attempt(answer) attempt = is_current_answer ? latest_answer : answer -auto_grading = attempt&.auto_grading&.specific + +auto_grading = attempt&.auto_gradings&.last&.specific +question = auto_grading&.question_snapshot || answer.question.specific +json.autoGradingCount attempt&.auto_gradings&.count +json.gradedOnPastSnapshot answer.question.specific != question +test_cases_by_type = question.test_cases_by_type can_grade = can?(:grade, submission) +can_read_tests = can?(:read_tests, submission) +show_private = can_read_tests || (submission.published? && assessment.show_private?) +show_evaluation = can_read_tests || (submission.published? && assessment.show_evaluation?) # Required in response of reload_answer and submit_answer to update past answers with the latest_attempt # Removing this check will cause it to render the latest_answer recursively @@ -32,80 +39,88 @@ json.fields do end end -job = attempt&.auto_grading&.job - -if job - json.autograding do - json.path job_path(job) if job.submitted? - json.partial! "jobs/#{job.status}", job: job - end -end - -if attempt.submitted? && !attempt.auto_grading - json.autograding do - json.status :submitted +json.canReadTests can_read_tests +if attempt.submitted? && !attempt&.auto_gradings&.any? + json.autogradings do + json.child! do + json.job do + json.status :submitted + end + end end end -can_read_tests = can?(:read_tests, submission) -show_private = can_read_tests || (submission.published? && assessment.show_private?) -show_evaluation = can_read_tests || (submission.published? && assessment.show_evaluation?) - -test_cases_by_type = question.test_cases_by_type -test_cases_and_results = get_test_cases_and_results(test_cases_by_type, auto_grading) - -show_stdout_and_stderr = (can_read_tests || current_course.show_stdout_and_stderr) && - auto_grading && auto_grading&.exit_code != 0 - displayed_test_case_types = ['public_test'] displayed_test_case_types << 'private_test' if show_private displayed_test_case_types << 'evaluation_test' if show_evaluation +# If the answer has no auto gradings, include the test cases directly here. +# Otherwise, read the test cases from the auto grading. + json.testCases do - json.canReadTests can_read_tests displayed_test_case_types.each do |test_case_type| - show_public = (test_case_type == 'public_test') && current_course.show_public_test_cases_output - show_testcase_outputs = can_read_tests || show_public json.set! test_case_type do - if test_cases_and_results[test_case_type].present? - json.array! test_cases_and_results[test_case_type] do |test_case, test_result| - json.identifier test_case.identifier if can_read_tests - json.expression test_case.expression - json.expected test_case.expected - if test_result - json.output get_output(test_result) if show_testcase_outputs - json.passed test_result.passed? - end + if test_cases_by_type[test_case_type].present? + json.array! test_cases_by_type[test_case_type] do |test_case| + json.partial! 'course/assessment/answer/programming/test_case', + test_case: test_case, + can_read_tests: can_read_tests end end end end +end + +json.autogradings do + json.array! attempt&.auto_gradings&.map(&:specific)&.compact do |auto_grading| + json.partial! 'course/assessment/answer/programming/auto_grading', assessment: assessment, + submission: submission, + question: question, + auto_grading: auto_grading, + can_read_tests: can_read_tests, + displayed_test_case_types: displayed_test_case_types + end +end - json.(auto_grading, :stdout, :stderr) if show_stdout_and_stderr +job = attempt&.auto_gradings&.last&.job + +if job + json.autograding do + json.path job_path(job) if job.submitted? + json.partial! "jobs/#{job.status}", job: job + end end -failed_test_cases_by_type = get_failed_test_cases_by_type(test_cases_and_results) +if attempt.submitted? && !attempt&.auto_gradings&.any? + json.autograding do + json.status :submitted + end +end json.explanation do - if attempt + if auto_grading + test_results_by_type = get_test_results_by_type(test_cases_by_type, auto_grading) + first_failures_by_type = get_first_test_failures_by_type(test_cases_by_type, test_results_by_type) explanations = [] - if failed_test_cases_by_type['public_test'] - failed_test_cases_by_type['public_test'].each do |test_case, test_result| - explanations << format_ckeditor_rich_text(get_hint(test_case, test_result)) - end + if first_failures_by_type['public_test'] + explanations << format_ckeditor_rich_text(get_hint( + first_failures_by_type['public_test'], + test_results_by_type['public_test'][first_failures_by_type['public_test'].id] + )) json.failureType 'public_test' - elsif failed_test_cases_by_type['private_test'] - failed_test_cases_by_type['private_test'].each do |test_case, test_result| - explanations << format_ckeditor_rich_text(get_hint(test_case, test_result)) - end + elsif first_failures_by_type['private_test'] + explanations << format_ckeditor_rich_text(get_hint( + first_failures_by_type['private_test'], + test_results_by_type['private_test'][first_failures_by_type['private_test'].id] + )) json.failureType 'private_test' end - passed_evaluation_tests = failed_test_cases_by_type['evaluation_test'].blank? + passed_evaluation_tests = first_failures_by_type['evaluation_test'].blank? - json.correct attempt&.auto_grading && attempt&.correct && (can_grade ? passed_evaluation_tests : true) + json.correct attempt&.correct && (can_grade ? passed_evaluation_tests : true) json.explanations explanations end end diff --git a/app/views/course/assessment/answer/programming/_test_case.jbuilder b/app/views/course/assessment/answer/programming/_test_case.jbuilder new file mode 100644 index 00000000000..e833ed19915 --- /dev/null +++ b/app/views/course/assessment/answer/programming/_test_case.jbuilder @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +json.id test_case.id +json.identifier test_case.identifier if can_read_tests +json.expression test_case.expression +json.expected test_case.expected diff --git a/app/views/course/assessment/answer/programming/_test_result.jbuilder b/app/views/course/assessment/answer/programming/_test_result.jbuilder new file mode 100644 index 00000000000..e386658f48d --- /dev/null +++ b/app/views/course/assessment/answer/programming/_test_result.jbuilder @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +json.id test_result.test_case_id +json.output get_output(test_result) if show_output +json.passed test_result.passed? 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 b58e2b3a8da..683a4d998c7 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 @@ -15,7 +15,7 @@ end last_attempt = last_attempt(answer) attempt = answer.current_answer? ? last_attempt : answer -job = attempt&.auto_grading&.job +job = attempt&.auto_gradings&.last&.job if job json.autograding do @@ -24,7 +24,7 @@ if job end end -if attempt.submitted? && !attempt.auto_grading +if attempt.submitted? && !attempt.auto_gradings&.any? json.autograding do json.status :submitted end diff --git a/app/views/course/assessment/answer/text_responses/_text_response.json.jbuilder b/app/views/course/assessment/answer/text_responses/_text_response.json.jbuilder index 8bef8cd9244..b52e06ec95e 100644 --- a/app/views/course/assessment/answer/text_responses/_text_response.json.jbuilder +++ b/app/views/course/assessment/answer/text_responses/_text_response.json.jbuilder @@ -24,8 +24,8 @@ last_attempt = last_attempt(answer) json.explanation do json.correct last_attempt&.correct - if last_attempt&.auto_grading&.result - json.explanations(last_attempt.auto_grading.result['messages'].map { |e| format_ckeditor_rich_text(e) }) + if last_attempt&.auto_gradings&.last&.result + json.explanations(last_attempt.auto_gradings.last.result['messages'].map { |e| format_ckeditor_rich_text(e) }) else json.explanations [] end diff --git a/client/app/bundles/course/assessment/submission/components/AnswerDetails/ProgrammingAnswerDetails.tsx b/client/app/bundles/course/assessment/submission/components/AnswerDetails/ProgrammingAnswerDetails.tsx index 21368906286..5c8f3265096 100644 --- a/client/app/bundles/course/assessment/submission/components/AnswerDetails/ProgrammingAnswerDetails.tsx +++ b/client/app/bundles/course/assessment/submission/components/AnswerDetails/ProgrammingAnswerDetails.tsx @@ -6,9 +6,9 @@ import { useAppDispatch } from 'lib/hooks/store'; import { AnswerDetailsProps } from '../../types'; +import AutoGradingComponent from './ProgrammingComponent/AutoGradingComponent'; import CodaveriFeedbackStatus from './ProgrammingComponent/CodaveriFeedbackStatus'; import FileContent from './ProgrammingComponent/FileContent'; -import TestCases from './ProgrammingComponent/TestCases'; const ProgrammingAnswerDetails = ( props: AnswerDetailsProps, @@ -35,7 +35,9 @@ const ProgrammingAnswerDetails = ( file={file} /> ))} - + {answer.autogradings && answer.autogradings.length > 0 && ( + + )} ); diff --git a/client/app/bundles/course/assessment/submission/components/AnswerDetails/ProgrammingComponent/AutoGradingComponent.tsx b/client/app/bundles/course/assessment/submission/components/AnswerDetails/ProgrammingComponent/AutoGradingComponent.tsx new file mode 100644 index 00000000000..fbd656532b9 --- /dev/null +++ b/client/app/bundles/course/assessment/submission/components/AnswerDetails/ProgrammingComponent/AutoGradingComponent.tsx @@ -0,0 +1,200 @@ +import { FC, useState } from 'react'; +import { defineMessages } from 'react-intl'; +import { Alert, Divider, Typography } from '@mui/material'; +import { ProgrammingAutoGradingData } from 'types/course/assessment/submission/answer/programming'; + +import Link from 'lib/components/core/Link'; +import useTranslation from 'lib/hooks/useTranslation'; +import { formatLongDateTime } from 'lib/moment'; + +import OutputStream from './OutputStream'; +import TestCaseComponent from './TestCaseComponent'; + +interface Props { + autogradings: ProgrammingAutoGradingData[]; +} + +const translations = defineMessages({ + standardOutput: { + id: 'course.assessment.submission.TestCaseView.standardOutput', + defaultMessage: 'Standard Output', + }, + standardError: { + id: 'course.assessment.submission.TestCaseView.standardError', + defaultMessage: 'Standard Error', + }, + answerGradedOnPastSnapshot: { + id: 'course.assessment.submission.history.answerGradedOnPastSnapshot', + defaultMessage: + 'Changes have been made to the question after this autograding run.', + }, + multipleAutoGradingResults: { + id: 'course.assessment.submission.history.multipleAutoGradingResults', + defaultMessage: 'This answer has been autograded {count} times.', + }, + multipleAutoGradingVariedResults: { + id: 'course.assessment.submission.history.multipleAutoGradingVariedResults', + defaultMessage: + 'This answer has been autograded {count} times, some of which produced different results.', + }, + autoGradingItemTitle: { + id: 'course.assessment.statistics.autoGradingItemTitle', + defaultMessage: 'Graded At: {gradedAt}', + }, +}); + +const AutoGradingItemComponent: FC<{ testCase: ProgrammingAutoGradingData }> = ( + props, +) => { + const { testCase } = props; + const { t } = useTranslation(); + return ( +
+ {testCase.gradedOnPastSnapshot && ( + + {t(translations.answerGradedOnPastSnapshot)} + + )} + {testCase.testCases?.public_test && + testCase.testCases.public_test.length > 0 && ( + + )} + + {testCase.testCases?.private_test && + testCase.testCases.private_test.length > 0 && ( + + )} + + {testCase.testCases?.evaluation_test && + testCase.testCases.evaluation_test.length > 0 && ( + + )} + + + + +
+ ); +}; + +const AutoGradingComponent: FC = (props) => { + const { autogradings } = props; + const { t } = useTranslation(); + + const autogradingResultCounts = autogradings.map((autograding) => ({ + public_test: { + passed: + autograding.testCases?.public_test?.filter( + (test) => autograding.testResults?.public_test?.[test.id]?.passed, + ).length ?? 0, + total: autograding.testCases?.public_test?.length ?? 0, + }, + private_test: { + passed: + autograding.testCases?.private_test?.filter( + (test) => autograding.testResults?.private_test?.[test.id]?.passed, + ).length ?? 0, + total: autograding.testCases?.private_test?.length ?? 0, + }, + evaluation_test: { + passed: + autograding.testCases?.evaluation_test?.filter( + (test) => autograding.testResults?.evaluation_test?.[test.id]?.passed, + ).length ?? 0, + total: autograding.testCases?.evaluation_test?.length ?? 0, + }, + })); + + const autogradingResultsAllIdentical = autogradingResultCounts.reduce( + (acc, current) => { + if (!acc) return false; + const first = autogradingResultCounts[0]; + return ( + current.public_test.passed === first.public_test.passed && + current.private_test.passed === first.private_test.passed && + current.evaluation_test.passed === first.evaluation_test.passed + ); + }, + true, + ); + + const [isShowingPastGradings, setIsShowingPastGradings] = useState(false); + + const TogglePastGradingLink = (): JSX.Element => ( + { + setIsShowingPastGradings(!isShowingPastGradings); + }} + underline="always" + > + {isShowingPastGradings ? 'Hide' : 'Show all'} + + ); + + return ( +
+ {autogradings.length > 1 && autogradingResultsAllIdentical && ( + + {t(translations.multipleAutoGradingResults, { + count: autogradings.length, + })} +   + + + )} + + {autogradings.length > 1 && !autogradingResultsAllIdentical && ( + + {t(translations.multipleAutoGradingVariedResults, { + count: autogradings.length, + })} +   + + + )} + + {isShowingPastGradings && + autogradings.toReversed().map((autograding) => ( + <> + + {t(translations.autoGradingItemTitle, { + gradedAt: formatLongDateTime(autograding.createdAt), + })} + + + + + ))} + {!isShowingPastGradings && ( + + )} +
+ ); +}; + +export default AutoGradingComponent; diff --git a/client/app/bundles/course/assessment/submission/components/AnswerDetails/ProgrammingComponent/OutputStream.tsx b/client/app/bundles/course/assessment/submission/components/AnswerDetails/ProgrammingComponent/OutputStream.tsx new file mode 100644 index 00000000000..cb0d6dce763 --- /dev/null +++ b/client/app/bundles/course/assessment/submission/components/AnswerDetails/ProgrammingComponent/OutputStream.tsx @@ -0,0 +1,44 @@ +import { FC } from 'react'; +import { defineMessages, FormattedMessage } from 'react-intl'; +import { Chip } from '@mui/material'; + +import Accordion from 'lib/components/core/layouts/Accordion'; + +const translations = defineMessages({ + noOutputs: { + id: 'course.assessment.submission.TestCaseView.noOutputs', + defaultMessage: 'No outputs', + }, +}); + +interface OutputStreamProps { + outputStreamType: 'standardOutput' | 'standardError'; + output?: string; + title: string; +} + +const OutputStream: FC = (props) => { + const { outputStreamType, output, title } = props; + return ( + } + size="small" + variant="outlined" + /> + ) + } + id={outputStreamType} + title={title} + > +
{output}
+
+ ); +}; + +export default OutputStream; diff --git a/client/app/bundles/course/assessment/submission/components/AnswerDetails/ProgrammingComponent/TestCases.tsx b/client/app/bundles/course/assessment/submission/components/AnswerDetails/ProgrammingComponent/TestCaseComponent.tsx similarity index 63% rename from client/app/bundles/course/assessment/submission/components/AnswerDetails/ProgrammingComponent/TestCases.tsx rename to client/app/bundles/course/assessment/submission/components/AnswerDetails/ProgrammingComponent/TestCaseComponent.tsx index 7963e767d42..ea0f64e9b45 100644 --- a/client/app/bundles/course/assessment/submission/components/AnswerDetails/ProgrammingComponent/TestCases.tsx +++ b/client/app/bundles/course/assessment/submission/components/AnswerDetails/ProgrammingComponent/TestCaseComponent.tsx @@ -9,8 +9,10 @@ import { TableHead, TableRow, } from '@mui/material'; -import { TestCaseResult } from 'types/course/assessment/submission/answer/programming'; -import { TestCase } from 'types/course/statistics/answer'; +import { + ProgrammingTestCaseData, + ProgrammingTestResultData, +} from 'types/course/assessment/submission/answer/programming'; import Accordion from 'lib/components/core/layouts/Accordion'; import useTranslation from 'lib/hooks/useTranslation'; @@ -62,37 +64,24 @@ const translations = defineMessages({ id: 'course.assessment.submission.TestCaseView.standardError', defaultMessage: 'Standard Error', }, - noOutputs: { - id: 'course.assessment.submission.TestCaseView.noOutputs', - defaultMessage: 'No outputs', - }, }); -interface Props { - testCase: TestCase; -} - interface TestCaseComponentProps { - testCaseResults: TestCaseResult[]; + testCases: ProgrammingTestCaseData[]; + testResults?: Record; testCaseType: string; } -interface OutputStreamProps { - outputStreamType: 'standardOutput' | 'standardError'; - output?: string; -} - const TestCaseComponent: FC = (props) => { - const { testCaseResults, testCaseType } = props; + const { testCases, testResults, testCaseType } = props; const { t } = useTranslation(); - const isProgrammingAnswerEvaluated = - testCaseResults.filter((result) => !!result.output).length > 0; + const isProgrammingAnswerEvaluated = Boolean(testResults); - const numPassedTestCases = testCaseResults.filter( - (result) => result.passed, + const numPassedTestCases = testCases.filter( + (testCase) => testResults?.[testCase.id]?.passed, ).length; - const numTestCases = testCaseResults.length; + const numTestCases = testCases.length; const AllTestCasesPassedChip: FC = () => ( = (props) => { - {testCaseResults.map((result) => ( - + {testCases.map((testCase) => ( + ))} @@ -196,65 +189,4 @@ const TestCaseComponent: FC = (props) => { ); }; -const OutputStream: FC = (props) => { - const { outputStreamType, output } = props; - const { t } = useTranslation(); - return ( - } - size="small" - variant="outlined" - /> - ) - } - id={outputStreamType} - title={t(translations[outputStreamType])} - > -
{output}
-
- ); -}; - -const TestCases: FC = (props) => { - const { testCase } = props; - - return ( -
- {testCase.public_test && testCase.public_test.length > 0 && ( - - )} - - {testCase.private_test && testCase.private_test.length > 0 && ( - - )} - - {testCase.evaluation_test && testCase.evaluation_test.length > 0 && ( - - )} - - - - -
- ); -}; - -export default TestCases; +export default TestCaseComponent; diff --git a/client/app/bundles/course/assessment/submission/components/AnswerDetails/ProgrammingComponent/TestCaseRow.tsx b/client/app/bundles/course/assessment/submission/components/AnswerDetails/ProgrammingComponent/TestCaseRow.tsx index 3266022da80..85515f24467 100644 --- a/client/app/bundles/course/assessment/submission/components/AnswerDetails/ProgrammingComponent/TestCaseRow.tsx +++ b/client/app/bundles/course/assessment/submission/components/AnswerDetails/ProgrammingComponent/TestCaseRow.tsx @@ -1,12 +1,16 @@ import { FC, Fragment } from 'react'; import { Clear, Done } from '@mui/icons-material'; import { TableCell, TableRow, Typography } from '@mui/material'; -import { TestCaseResult } from 'types/course/assessment/submission/answer/programming'; +import { + ProgrammingTestCaseData, + ProgrammingTestResultData, +} from 'types/course/assessment/submission/answer/programming'; import ExpandableCode from 'lib/components/core/ExpandableCode'; interface Props { - result: TestCaseResult; + testCase: ProgrammingTestCaseData; + testResult?: ProgrammingTestResultData; } const TestCaseClassName = { @@ -16,17 +20,17 @@ const TestCaseClassName = { }; const TestCaseRow: FC = (props) => { - const { result } = props; + const { testCase, testResult } = props; const nameRegex = /\/?(\w+)$/; - const idMatch = result.identifier?.match(nameRegex); + const idMatch = testCase.identifier?.match(nameRegex); const truncatedIdentifier = idMatch ? idMatch[1] : ''; let testCaseResult = 'unattempted'; let testCaseIcon; - if (result.passed !== undefined) { - testCaseResult = result.passed ? 'correct' : 'wrong'; - testCaseIcon = result.passed ? ( + if (testResult?.passed !== undefined) { + testCaseResult = testResult.passed ? 'correct' : 'wrong'; + testCaseIcon = testResult.passed ? ( ) : ( @@ -34,7 +38,7 @@ const TestCaseRow: FC = (props) => { } return ( - + = (props) => { - {result.expression} + {testCase.expression} - {result.expected || ''} + {testCase.expected || ''} - {result.output || ''} + {testResult?.output || ''} {testCaseIcon} diff --git a/client/app/bundles/course/assessment/submission/containers/TestCaseView/index.jsx b/client/app/bundles/course/assessment/submission/containers/TestCaseView/index.jsx index 7cda3dc3a08..87b801d9d3c 100644 --- a/client/app/bundles/course/assessment/submission/containers/TestCaseView/index.jsx +++ b/client/app/bundles/course/assessment/submission/containers/TestCaseView/index.jsx @@ -1,3 +1,6 @@ +// TODO Drop this component entirely, in favor of TestCaseComponent. +// This requires rewriting (and renaming) testCases reducer to TypeScript. + import { Component, Fragment } from 'react'; import { defineMessages, FormattedMessage } from 'react-intl'; import { connect } from 'react-redux'; @@ -126,7 +129,7 @@ export class VisibleTestCaseView extends Component { ); } - renderTestCaseRow(testCase) { + renderTestCaseRow(testCase, testResult) { const { testCases: { canReadTests }, } = this.props; @@ -138,9 +141,9 @@ export class VisibleTestCaseView extends Component { let testCaseResult = 'unattempted'; let testCaseIcon; - if (testCase.passed !== undefined) { - testCaseResult = testCase.passed ? 'correct' : 'wrong'; - testCaseIcon = testCase.passed ? ( + if (testResult.passed !== undefined) { + testCaseResult = testResult?.passed ? 'correct' : 'wrong'; + testCaseIcon = testResult?.passed ? ( ) : ( @@ -177,7 +180,7 @@ export class VisibleTestCaseView extends Component { {(canReadTests || showPublicTestCasesOutput) && ( - {testCase.output || ''} + {testResult?.output || ''} )} @@ -187,23 +190,21 @@ export class VisibleTestCaseView extends Component { ); } - renderTestCases(testCases, testCaseType, warn) { + renderTestCases(testCases, testResults, testCaseType, warn) { const { - collapsible, testCases: { canReadTests }, graderView, } = this.props; const { showPublicTestCasesOutput } = this.props; - if (!testCases || testCases.length === 0) { + if (!testCases?.length) { return null; } - const isProgrammingAnswerEvaluated = - testCases.filter((testCase) => !!testCase.output).length > 0; + const isProgrammingAnswerEvaluated = Boolean(testResults); const numPassedTestCases = testCases.filter( - (testCase) => testCase.passed, + (testCase) => testResults?.[testCase.id]?.passed, ).length; const AllTestCasesPassedChip = () => ( @@ -278,7 +279,7 @@ export class VisibleTestCaseView extends Component { return ( } id={testCaseType} @@ -309,7 +310,9 @@ export class VisibleTestCaseView extends Component { - {testCases.map(this.renderTestCaseRow.bind(this))} + {testCases.map((testCase) => + this.renderTestCaseRow(testCase, testResults?.[testCase.id]), + )} @@ -323,8 +326,7 @@ export class VisibleTestCaseView extends Component { showEvaluation, graderView, isAutograding, - testCases, - collapsible, + testCases: { canReadTests, testCases, testResults, stdout, stderr }, showStdoutAndStderr, } = this.props; if (!testCases) { @@ -336,9 +338,9 @@ export class VisibleTestCaseView extends Component { const showPrivateTestToStudents = published && showPrivate; const showEvaluationTestToStudents = published && showEvaluation; const showPrivateTest = - (graderView && testCases.canReadTests) || showPrivateTestToStudents; + (graderView && canReadTests) || showPrivateTestToStudents; const showEvaluationTest = - (graderView && testCases.canReadTests) || showEvaluationTestToStudents; + (graderView && canReadTests) || showEvaluationTestToStudents; return (
@@ -348,11 +350,17 @@ export class VisibleTestCaseView extends Component { )} - {this.renderTestCases(testCases.public_test, 'publicTestCases', false)} + {this.renderTestCases( + testCases.public_test, + testResults?.public_test, + 'publicTestCases', + false, + )} {showPrivateTest && this.renderTestCases( testCases.private_test, + testResults?.private_test, 'privateTestCases', !showPrivateTestToStudents, )} @@ -360,23 +368,22 @@ export class VisibleTestCaseView extends Component { {showEvaluationTest && this.renderTestCases( testCases.evaluation_test, + testResults?.evaluation_test, 'evaluationTestCases', !showEvaluationTestToStudents, )} {showOutputStreams && - !collapsible && VisibleTestCaseView.renderOutputStream( 'standardOutput', - testCases.stdout, + stdout, !showStdoutAndStderr, )} {showOutputStreams && - !collapsible && VisibleTestCaseView.renderOutputStream( 'standardError', - testCases.stderr, + stderr, !showStdoutAndStderr, )}
@@ -395,28 +402,23 @@ VisibleTestCaseView.propTypes = { showPrivate: PropTypes.bool, showEvaluation: PropTypes.bool, isAutograding: PropTypes.bool, - collapsible: PropTypes.bool, testCases: PropTypes.shape({ canReadTests: PropTypes.bool, - evaluation_test: PropTypes.arrayOf(testCaseShape), - private_test: PropTypes.arrayOf(testCaseShape), - public_test: PropTypes.arrayOf(testCaseShape), + testCases: PropTypes.shape({ + evaluation_test: PropTypes.arrayOf(testCaseShape), + private_test: PropTypes.arrayOf(testCaseShape), + public_test: PropTypes.arrayOf(testCaseShape), + }), + testResults: PropTypes.object, stdout: PropTypes.string, stderr: PropTypes.string, }), }; function mapStateToProps({ assessments: { submission } }, ownProps) { - const { questionId, answerId, viewHistory } = ownProps; - let testCases; - let isAutograding; - if (viewHistory) { - testCases = submission.history.testCases[answerId]; - isAutograding = false; - } else { - testCases = submission.testCases[questionId]; - isAutograding = submission.questionsFlags[questionId].isAutograding; - } + const { questionId } = ownProps; + const testCases = submission.testCases[questionId]; + const isAutograding = submission.questionsFlags[questionId].isAutograding; return { submissionState: submission.submission.workflowState, @@ -425,7 +427,6 @@ function mapStateToProps({ assessments: { submission } }, ownProps) { showStdoutAndStderr: submission.submission.showStdoutAndStderr, showPrivate: submission.assessment.showPrivate, showEvaluation: submission.assessment.showEvaluation, - collapsible: viewHistory, isAutograding, testCases, }; diff --git a/client/app/bundles/course/assessment/submission/reducers/history/index.ts b/client/app/bundles/course/assessment/submission/reducers/history/index.ts index cedefbd7653..46f5f7cab66 100644 --- a/client/app/bundles/course/assessment/submission/reducers/history/index.ts +++ b/client/app/bundles/course/assessment/submission/reducers/history/index.ts @@ -180,10 +180,18 @@ export const historySlice = createSlice({ const { submissionId, questionId, answerItem } = action.payload; const submissionQuestionState = state[submissionId]?.[questionId]; if (submissionQuestionState?.details) { - submissionQuestionState.details.allAnswers.push(answerItem); - submissionQuestionState.details.sequenceViewSelectedAnswerIds.unshift( - answerItem.id, - ); + const answerIndex = + submissionQuestionState.details.allAnswers.findIndex( + (answer) => answer.id === answerItem.id, + ); + if (answerIndex >= 0) { + submissionQuestionState.details.allAnswers[answerIndex] = answerItem; + } else { + submissionQuestionState.details.allAnswers.push(answerItem); + submissionQuestionState.details.sequenceViewSelectedAnswerIds.unshift( + answerItem.id, + ); + } } }, updateSingleAnswerHistory: ( diff --git a/client/app/bundles/course/assessment/submission/reducers/questionsFlags.js b/client/app/bundles/course/assessment/submission/reducers/questionsFlags.js index 1a0ac99afb8..5544fd76c6a 100644 --- a/client/app/bundles/course/assessment/submission/reducers/questionsFlags.js +++ b/client/app/bundles/course/assessment/submission/reducers/questionsFlags.js @@ -5,18 +5,19 @@ function initQuestionsFlagsFromSubmissionPayload(payload) { const answer = payload.answers.find( (ans) => ans.questionId === question.id, ); + const lastAutogradingJob = answer?.autogradings?.at(-1)?.job; return { ...obj, [question.id]: { isResetting: false, isAutograding: - Boolean(answer?.autograding) && - answer?.autograding?.status === 'submitted', - jobUrl: answer?.autograding?.jobUrl, + Boolean(lastAutogradingJob) && + lastAutogradingJob.status === 'submitted', + jobUrl: lastAutogradingJob?.jobUrl, jobError: - Boolean(answer?.autograding) && - answer?.autograding?.status === 'errored', - jobErrorMessage: answer?.autograding?.errorMessage, + Boolean(lastAutogradingJob) && + lastAutogradingJob.status === 'errored', + jobErrorMessage: lastAutogradingJob?.errorMessage, }, }; }, {}); diff --git a/client/app/bundles/course/assessment/submission/reducers/testCases.js b/client/app/bundles/course/assessment/submission/reducers/testCases.js index 96625519c09..1b914007729 100644 --- a/client/app/bundles/course/assessment/submission/reducers/testCases.js +++ b/client/app/bundles/course/assessment/submission/reducers/testCases.js @@ -1,5 +1,19 @@ import actions from '../constants'; +function initQuestionStateFromAnswerPayload(answer) { + const lastAutograding = answer.autogradings?.at(-1); + if (lastAutograding) { + return { + canReadTests: answer.canReadTests, + testCases: lastAutograding.testCases, + testResults: lastAutograding.testResults, + stdout: lastAutograding.stdout, + stderr: lastAutograding.stderr, + }; + } + return { testCases: answer.testCases }; +} + export default function (state = {}, action) { switch (action.type) { case actions.FETCH_SUBMISSION_SUCCESS: @@ -13,7 +27,10 @@ export default function (state = {}, action) { return { ...state, ...action.payload.answers.reduce( - (obj, answer) => ({ ...obj, [answer.questionId]: answer.testCases }), + (obj, answer) => ({ + ...obj, + [answer.questionId]: initQuestionStateFromAnswerPayload(answer), + }), {}, ), }; @@ -29,7 +46,7 @@ export default function (state = {}, action) { } return obj; }, - { [questionId]: action.payload.testCases }, + { [questionId]: initQuestionStateFromAnswerPayload(action.payload) }, ); } case actions.REEVALUATE_FAILURE: @@ -41,19 +58,9 @@ export default function (state = {}, action) { // For each test case in each test type, add back the data without the output // and passed values. if (state[questionId]) { - Object.keys(state[questionId]).forEach((testType) => { - if ( - testType !== 'stdout' && - testType !== 'stderr' && - testType !== 'canReadTests' - ) { - questionState[testType] = state[questionId][testType].map( - (testCase) => ({ - identifier: testCase.identifier, - expression: testCase.expression, - expected: testCase.expected, - }), - ); + Object.keys(state[questionId]).forEach((key) => { + if (key !== 'testResults') { + questionState[key] = state[questionId][key]; } }); } diff --git a/client/app/types/course/assessment/submission/answer/programming.ts b/client/app/types/course/assessment/submission/answer/programming.ts index 3edc08e89e0..37f189e12e6 100644 --- a/client/app/types/course/assessment/submission/answer/programming.ts +++ b/client/app/types/course/assessment/submission/answer/programming.ts @@ -48,15 +48,6 @@ export interface Post { codaveriFeedback: CodaveriFeedback; } -export interface TestCase { - canReadTests: boolean; - public_test?: TestCaseResult[]; - private_test?: TestCaseResult[]; - evaluation_test?: TestCaseResult[]; - stdout?: string; - stderr?: string; -} - export interface CodaveriFeedback { jobId: string; jobStatus: keyof typeof JobStatus; @@ -70,6 +61,41 @@ export interface ProgrammingFieldData extends AnswerFieldBaseData { files_attributes: ProgrammingContent[]; } +export interface ProgrammingTestCaseData { + id: number; + identifier?: string; + expression: string; + expected: string; +} + +export interface ProgrammingTestResultData { + id: number; + output?: string; + passed: boolean; +} + +export interface ProgrammingAutoGradingData { + id: number; + createdAt: string; + job: JobStatusResponse & { + path?: string; + }; + testCases?: { + public_test?: ProgrammingTestCaseData[]; + private_test?: ProgrammingTestCaseData[]; + evaluation_test?: ProgrammingTestCaseData[]; + }; + testResults?: { + public_test?: Record; + private_test?: Record; + evaluation_test?: Record; + }; + + stdout?: string; + stderr?: string; + gradedOnPastSnapshot: boolean; +} + export interface ProgrammingAnswerData extends AnswerBaseData { questionType: QuestionType.Programming; fields: ProgrammingFieldData; @@ -78,18 +104,10 @@ export interface ProgrammingAnswerData extends AnswerBaseData { explanation: string[]; failureType: TestCaseType; }; - testCases: { - canReadTests: boolean; - public_test?: TestCaseResult[]; - private_test?: TestCaseResult[]; - evaluation_test?: TestCaseResult[]; - stdout?: string; - stderr?: string; - }; + canReadTests?: boolean; + autoGradingCount: number; + autogradings?: ProgrammingAutoGradingData[]; attemptsLeft?: number; - autograding?: JobStatusResponse & { - path?: string; - }; codaveriFeedback?: { jobId: string; jobStatus: keyof typeof JobStatus; @@ -108,6 +126,11 @@ export interface ProgrammingAnswerData extends AnswerBaseData { }; annotations?: Annotation[]; posts?: Post[]; + testCases?: { + public_test?: ProgrammingTestCaseData[]; + private_test?: ProgrammingTestCaseData[]; + evaluation_test?: ProgrammingTestCaseData[]; + }; } // FE Data Type diff --git a/client/app/types/course/statistics/answer.ts b/client/app/types/course/statistics/answer.ts index 9b5ec305edb..e177584814d 100644 --- a/client/app/types/course/statistics/answer.ts +++ b/client/app/types/course/statistics/answer.ts @@ -1,4 +1,4 @@ -import { JobStatus, JobStatusResponse } from 'types/jobs'; +import { JobStatus } from 'types/jobs'; import { UserBasicListData } from 'types/users'; import { QuestionType } from '../assessment/question'; @@ -8,8 +8,8 @@ import { MultipleResponseFieldData, } from '../assessment/submission/answer/multipleResponse'; import { + ProgrammingAutoGradingData, ProgrammingFieldData, - TestCaseResult, TestCaseType, } from '../assessment/submission/answer/programming'; import { ScribingFieldData } from '../assessment/submission/answer/scribing'; @@ -69,15 +69,6 @@ export interface Post { codaveriFeedback: CodaveriFeedback; } -export interface TestCase { - canReadTests: boolean; - public_test?: TestCaseResult[]; - private_test?: TestCaseResult[]; - evaluation_test?: TestCaseResult[]; - stdout?: string; - stderr?: string; -} - export interface CodaveriFeedback { jobId: string; jobStatus: keyof typeof JobStatus; @@ -93,11 +84,10 @@ export interface ProgrammingAnswerDetails explanation: string[]; failureType: TestCaseType; }; - testCases: TestCase; attemptsLeft?: number; - autograding?: JobStatusResponse & { - path?: string; - }; + canReadTests?: boolean; + autoGradingCount: number; + autogradings?: ProgrammingAutoGradingData[]; codaveriFeedback?: CodaveriFeedback; latestAnswer?: ProgrammingAnswerDetails & { annotations: Annotation[]; diff --git a/db/migrate/20250629052451_add_programming_question_current_ids.rb b/db/migrate/20250629052451_add_programming_question_current_ids.rb new file mode 100644 index 00000000000..a2712647dc0 --- /dev/null +++ b/db/migrate/20250629052451_add_programming_question_current_ids.rb @@ -0,0 +1,51 @@ +class AddProgrammingQuestionCurrentIds < ActiveRecord::Migration[7.2] + def up + # We set current_id values of all current questions to be equal to the question's own id. + # This causes problems if we attempt a cascading delete directly on DB. + add_reference :course_assessment_question_programming, + :current, + foreign_key: { to_table: :course_assessment_question_programming }, + null: true + add_column :course_assessment_question_programming, + :snapshot_index, + :integer + add_column :course_assessment_question_programming, + :snapshot_of_state_at, + :datetime + + Course::Assessment::Question::Programming.reset_column_information + migration_timestamp = Time.current + Course::Assessment::Question::Programming.update_all( + "current_id = id, snapshot_index = 0, snapshot_of_state_at = '#{migration_timestamp.utc.iso8601}'" + ) + + # Add not-null constraints after initialization + change_column :course_assessment_question_programming, + :snapshot_index, + :integer, + null: false + change_column :course_assessment_question_programming, + :snapshot_of_state_at, + :datetime, + null: false + + # This field is not initialized for performance reasons, + # so we interpret null values as meaning the grading was done with the current question. + add_reference :course_assessment_answer_programming_auto_gradings, + :question_snapshot, + foreign_key: { to_table: :course_assessment_question_programming }, + null: true + + remove_index :course_assessment_answer_auto_gradings, column: :answer_id + add_index :course_assessment_answer_auto_gradings, :answer_id + end + + def down + remove_index :course_assessment_answer_auto_gradings, :answer_id + add_index :course_assessment_answer_auto_gradings, :answer_id, unique: true + remove_reference :course_assessment_answer_programming_auto_gradings, :question_snapshot + remove_column :course_assessment_question_programming, :snapshot_of_state_at + remove_column :course_assessment_question_programming, :snapshot_index + remove_reference :course_assessment_question_programming, :current + end +end diff --git a/db/schema.rb b/db/schema.rb index 76e393407b4..fa595b58a2f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2025_06_19_030938) do +ActiveRecord::Schema[7.2].define(version: 2025_06_29_052451) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" enable_extension "uuid-ossp" @@ -100,7 +100,7 @@ t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false t.index ["actable_id", "actable_type"], name: "index_course_assessment_answer_auto_gradings_on_actable", unique: true - t.index ["answer_id"], name: "index_course_assessment_answer_auto_gradings_on_answer_id", unique: true + t.index ["answer_id"], name: "index_course_assessment_answer_auto_gradings_on_answer_id" t.index ["job_id"], name: "index_course_assessment_answer_auto_gradings_on_job_id", unique: true end @@ -142,6 +142,8 @@ t.text "stdout" t.text "stderr" t.integer "exit_code" + t.bigint "question_snapshot_id" + t.index ["question_snapshot_id"], name: "idx_on_question_snapshot_id_80edeb1ca8" end create_table "course_assessment_answer_programming_file_annotations", id: :serial, force: :cascade do |t| @@ -331,6 +333,10 @@ t.boolean "live_feedback_enabled", default: false, null: false t.string "live_feedback_custom_prompt", default: "", null: false t.boolean "is_synced_with_codaveri", default: false, null: false + t.bigint "current_id" + t.integer "snapshot_index", null: false + t.datetime "snapshot_of_state_at", null: false + t.index ["current_id"], name: "index_course_assessment_question_programming_on_current_id" t.index ["import_job_id"], name: "index_course_assessment_question_programming_on_import_job_id", unique: true t.index ["language_id"], name: "fk__course_assessment_question_programming_language_id" end @@ -1649,6 +1655,7 @@ add_foreign_key "course_assessment_answer_multiple_response_options", "course_assessment_answer_multiple_responses", column: "answer_id", name: "fk_course_assessment_answer_multiple_response_options_answer_id" add_foreign_key "course_assessment_answer_multiple_response_options", "course_assessment_question_multiple_response_options", column: "option_id", name: "fk_course_assessment_answer_multiple_response_options_option_id" add_foreign_key "course_assessment_answer_programming", "jobs", column: "codaveri_feedback_job_id", on_delete: :nullify + add_foreign_key "course_assessment_answer_programming_auto_gradings", "course_assessment_question_programming", column: "question_snapshot_id" add_foreign_key "course_assessment_answer_programming_file_annotations", "course_assessment_answer_programming_files", column: "file_id", name: "fk_course_assessment_answer_ed21459e7a2a5034dcf43a14812cb17d" add_foreign_key "course_assessment_answer_programming_files", "course_assessment_answer_programming", column: "answer_id", name: "fk_course_assessment_answer_programming_files_answer_id" add_foreign_key "course_assessment_answer_programming_test_results", "course_assessment_answer_programming_auto_gradings", column: "auto_grading_id", name: "fk_course_assessment_answer_e3d785447112439bb306849be8690102" @@ -1678,6 +1685,7 @@ add_foreign_key "course_assessment_question_bundles", "course_assessment_question_groups", column: "group_id" add_foreign_key "course_assessment_question_groups", "course_assessments", column: "assessment_id" add_foreign_key "course_assessment_question_multiple_response_options", "course_assessment_question_multiple_responses", column: "question_id", name: "fk_course_assessment_question_multiple_response_options_questio" + add_foreign_key "course_assessment_question_programming", "course_assessment_question_programming", column: "current_id" add_foreign_key "course_assessment_question_programming", "jobs", column: "import_job_id", name: "fk_course_assessment_question_programming_import_job_id", on_delete: :nullify add_foreign_key "course_assessment_question_programming", "polyglot_languages", column: "language_id", name: "fk_course_assessment_question_programming_language_id" add_foreign_key "course_assessment_question_programming_template_files", "course_assessment_question_programming", column: "question_id", name: "fk_course_assessment_questi_0788633b496294e558f55f2b41bc7c45" diff --git a/spec/controllers/course/assessment/question/programming_controller_update_spec.rb b/spec/controllers/course/assessment/question/programming_controller_update_spec.rb new file mode 100644 index 00000000000..f29e06736c7 --- /dev/null +++ b/spec/controllers/course/assessment/question/programming_controller_update_spec.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Course::Assessment::Question::ProgrammingController do + render_views + let(:instance) { Instance.default } + with_tenant(:instance) do + let(:programming_question) { nil } + let(:user) { create(:user) } + let(:course) { create(:course, creator: user) } + let(:assessment) { create(:assessment, course: course) } + let(:question_programming_attributes) do + attributes_for(:course_assessment_question_programming). + slice(:title, :description, :maximum_grade, :language, :memory_limit, + :time_limit).tap do |result| + result[:language_id] = result.delete(:language).id + end + end + let(:immutable_programming_question) do + create(:course_assessment_question_programming, assessment: assessment).tap do |question| + allow(question).to receive(:save).and_return(false) + allow(question).to receive(:destroy).and_return(false) + end + end + + before do + controller_sign_in(controller, user) + controller.instance_variable_set(:@programming_question, programming_question) + end + + describe '#update' do + subject do + request.accept = 'application/json' + patch :update, params: { + course_id: course, assessment_id: assessment, id: programming_question, + question_programming: question_programming_attributes + } + end + + let!(:existing_language) { Coursemology::Polyglot::Language.find_by(name: 'Python 3.10') } + + context 'when the programming question has data to snapshot' do + let(:programming_question) do + create(:course_assessment_question_programming, template_package: true, + assessment: assessment).tap do |question| + + question.test_cases = [ + build(:course_assessment_question_programming_test_case, question: nil, expression: '1', expected: '1'), + build(:course_assessment_question_programming_test_case, question: nil, expression: '2', expected: '2'), + ] + end + end + + context 'when the evaluation fields are updated' do + let(:question_programming_attributes) do + attributes_for(:course_assessment_question_programming).slice(:time_limit).tap do |result| + result[:time_limit] = 11 + end + end + + it 'updates the question successfully, and clones test case data successfully' do + prev_snapshot_of_state_at = programming_question.snapshot_of_state_at + subject + expect(response).to have_http_status(:ok) + + programming_question.reload + snapshots = programming_question.snapshots + expect(snapshots.size).to eq(2) + current_snapshot, old_snapshot = snapshots.partition { |s| s.id == programming_question.id }.map(&:first) + + expect(old_snapshot.time_limit).to eq(10) + expect(old_snapshot.snapshot_of_state_at).to eq(prev_snapshot_of_state_at) + expect(old_snapshot.snapshot_index).to eq(0) + + expect(current_snapshot.time_limit).to eq(11) + expect(current_snapshot.snapshot_of_state_at).to be > prev_snapshot_of_state_at + expect(current_snapshot.snapshot_index).to eq(1) + + old_test_cases = old_snapshot.test_cases.group_by(&:expression) + current_test_cases = current_snapshot.test_cases.group_by(&:expression) + + old_test_cases.each do |expr_key, expr_tests| + expect(current_test_cases[expr_key].count).to eq(1) + expect(current_test_cases[expr_key].first.id).to_not eq(expr_tests.first.id) + end + end + end + end + end + end +end diff --git a/spec/jobs/course/assessment/answer/auto_grading_job_spec.rb b/spec/jobs/course/assessment/answer/auto_grading_job_spec.rb index a2211486b82..9c70ead4aec 100644 --- a/spec/jobs/course/assessment/answer/auto_grading_job_spec.rb +++ b/spec/jobs/course/assessment/answer/auto_grading_job_spec.rb @@ -17,7 +17,7 @@ let!(:auto_grading) { create(:course_assessment_answer_auto_grading, answer: answer) } it 'can be queued' do - expect { subject.perform_later(answer) }.to \ + expect { subject.perform_later(answer, auto_grading) }.to \ have_enqueued_job(subject).exactly(:once).on_queue('highest') end @@ -28,7 +28,7 @@ end it 'can be queued with delayed_ queue' do - expect { subject.perform_later(answer) }.to \ + expect { subject.perform_later(answer, auto_grading) }.to \ have_enqueued_job(subject).exactly(:once).on_queue('delayed_highest') end end @@ -37,8 +37,8 @@ it 'evaluates answers and does not update the exp' do initial_points = submission.points_awarded - subject.perform_now(answer) - expect(answer).to be_graded + subject.perform_now(answer, auto_grading) + expect(answer, auto_grading).to be_graded expect(submission.points_awarded).to eq(initial_points) end end @@ -57,8 +57,8 @@ it 'evaluates answers and updates the exp' do initial_points = submission.points_awarded - subject.perform_now(answer) - expect(answer).to be_graded + subject.perform_now(answer, auto_grading) + expect(answer, auto_grading).to be_graded expect(answer.grade).to eq(question.maximum_grade) correct_exp = assessment.base_exp + assessment.time_bonus_exp expect(submission.points_awarded).to eq(correct_exp) diff --git a/spec/services/course/assessment/answer/programming_auto_grading_service_spec.rb b/spec/services/course/assessment/answer/programming_auto_grading_service_spec.rb index d78e4f2e7aa..6dbee172cce 100644 --- a/spec/services/course/assessment/answer/programming_auto_grading_service_spec.rb +++ b/spec/services/course/assessment/answer/programming_auto_grading_service_spec.rb @@ -195,14 +195,14 @@ it 'sets each test result as failed' do subject - answer.auto_grading.specific.test_results.each do |test_result| + answer.auto_gradings.first.specific.test_results.each do |test_result| expect(test_result).not_to be_passed end end it 'sets the message for each test result' do subject - answer.auto_grading.specific.test_results.each do |test_result| + answer.auto_gradings.first.specific.test_results.each do |test_result| expect(test_result.messages['error']). to eq 'course.assessment.answer.programming_auto_grading.grade.evaluation_failed_syntax' end @@ -210,11 +210,11 @@ it 'sets stdout, stderr and exit code for the programming autograding object' do subject - expect(answer.auto_grading.specific.stdout). + expect(answer.auto_gradings.first.specific.stdout). to eq "Makefile:6: recipe for target 'test' failed" - expect(answer.auto_grading.specific.stderr). + expect(answer.auto_gradings.first.specific.stderr). to eq "ImportError: No module named 'simulation'" - expect(answer.auto_grading.specific.exit_code).to eq 2 + expect(answer.auto_gradings.first.specific.exit_code).to eq 2 end end end diff --git a/spec/services/course/assessment/answer/programming_codaveri_auto_grading_service_spec.rb b/spec/services/course/assessment/answer/programming_codaveri_auto_grading_service_spec.rb index b068ba5bea7..3b5fe6d4e3a 100644 --- a/spec/services/course/assessment/answer/programming_codaveri_auto_grading_service_spec.rb +++ b/spec/services/course/assessment/answer/programming_codaveri_auto_grading_service_spec.rb @@ -97,7 +97,7 @@ before { subject.save! } it 'saves the specific auto_grading' do - auto_grading = answer.reload.auto_grading.actable + auto_grading = answer.reload.auto_gradings.first.actable expect(auto_grading).to be_present expect(auto_grading.test_results).to be_present @@ -197,7 +197,7 @@ expect(answer.grade).to eq(nil) expect(answer.correct).to eq(nil) expect(answer.graded_at).to eq(nil) - expect(answer.actable.auto_grading.actable).to eq(nil) + expect(answer.actable.auto_gradings.first.actable).to eq(nil) end end end