Create / Save snapshots of programming questions on edit

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,15 +42,62 @@ 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.
                                               except(:question_assessment))
       @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
 

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

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,

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
 

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)

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,31 +195,17 @@ 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
     self.grade = nil
     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)

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

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
 

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

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

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

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

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

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

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?)

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)