Merge pull request #592 from b0ink/refactor/confirm-recursive-fix

refactor: require confirmation for recursive fix and resubmit

Author: b0ink

app/api/tasks_api.rb

@@ -158,6 +158,7 @@ class TasksApi < Grape::API
     optional :grade, type: Integer, desc: 'Grade value if task is a graded task (required if task definition is a graded task)'
     optional :quality_pts, type: Integer, desc: 'Quality points value if task has quality assessment'
     optional :discussed, type: Boolean, desc: 'Mark task as discussed'
+    optional :trigger_recursive_fix, desc: 'If marking fix and resubmit, recursively update preqreuisite submissions to fix'
   end
   put '/projects/:id/task_def_id/:task_definition_id' do
     project = Project.find(params[:id])
@@ -203,7 +204,7 @@ class TasksApi < Grape::API
         end
 
         logger.info "#{current_user.username} assessing task #{task.id} to #{params[:trigger]}"
-        result = task.trigger_transition(trigger: params[:trigger], by_user: current_user, quality: params[:quality_pts])
+        result = task.trigger_transition(trigger: params[:trigger], by_user: current_user, quality: params[:quality_pts], recursive_fix: params[:trigger_recursive_fix])
         if result.nil? && task.task_definition.restrict_status_updates
           error!({ error: 'This task can only be updated by your tutor.' }, 403)
         end

app/models/task.rb

@@ -522,7 +522,7 @@ def ensured_group_submission
     group.create_submission self, '', group.projects.map { |proj| { project: proj, pct: 100 / group.projects.count } }
   end
 
-  def trigger_transition(trigger: '', by_user: nil, bulk: false, group_transition: false, quality: 1)
+  def trigger_transition(trigger: '', by_user: nil, bulk: false, group_transition: false, quality: 1, recursive_fix: false)
     #
     # Ensure that assessor is allowed to update the task in the indicated way
     #
@@ -594,7 +594,7 @@ def trigger_transition(trigger: '', by_user: nil, bulk: false, group_transition:
         lc = comments.last
         # Prevent duplicate status comments during feedback
         unless lc && lc.user == by_user && lc.comment == status.name && (lc.content_type != 'status' || lc.task_status == status)
-          assess status, by_user
+          assess status, by_user, Time.zone.now, recursive_fix
 
           # Add a status comment for new assessments - only recorded on submitter's task in groups
           add_status_comment(by_user, status)
@@ -674,7 +674,7 @@ def grade_task(new_grade, ui = nil, grading_group = false)
     end
   end
 
-  def assess(task_status, assessor, assess_date = Time.zone.now)
+  def assess(task_status, assessor, assess_date = Time.zone.now, recursive_fix = false)
     # Set the task's status to the assessment outcome status
     # and flag it as no longer awaiting signoff
     self.task_status = task_status
@@ -725,7 +725,7 @@ def assess(task_status, assessor, assess_date = Time.zone.now)
         end
       end
 
-      if task_status == TaskStatus.fix_and_resubmit
+      if task_status == TaskStatus.fix_and_resubmit && recursive_fix
         # Look for other submitted tasks from this student that has this task as a prerequisite
         # If they are ready for feedback, automatically assess them to fix and resubmit
         dependents = TaskPrerequisite.where(prerequisite_id: task_definition.id)
@@ -738,7 +738,7 @@ def assess(task_status, assessor, assess_date = Time.zone.now)
 
           next unless task.task_status == TaskStatus.ready_for_feedback
           # Since we are calling this assess method again, we recursively check for more dependent tasks that need to be updated
-          task.assess(TaskStatus.fix_and_resubmit, assessor, assess_date)
+          task.assess(TaskStatus.fix_and_resubmit, assessor, assess_date, recursive_fix)
           task.add_status_comment(assessor, TaskStatus.fix_and_resubmit)
           task.add_text_comment(assessor, "**Automated comment**: A prerequisite task was updated to Fix and Resubmit, so this task was updated as well. You may need to review and update the prerequisite before resubmitting.")
         end

test/models/task_test.rb

@@ -1473,18 +1473,18 @@ def test_prerequisite_tasks_change_to_fix_and_resubmit
     assert_equal TaskStatus.ready_for_feedback, task3.task_status
     assert_equal TaskStatus.ready_for_feedback, task4.task_status
 
-    # Test case 1: Ensure parent prerequisite is not affected
+    # Test case 1: Without recursive_fix, dependent tasks should not be affected
     task2.assess(TaskStatus.fix_and_resubmit, tutor)
 
     task1.reload
     task2.reload
     task3.reload
     task4.reload
 
-    # Task 1 should not be affected if the dependent task is assessed to fix and resubmit
+    # Task 1 should not be affected, and recursive fixes should not run by default
     assert_equal TaskStatus.ready_for_feedback, task1.task_status, "Parent prerequisite should not be affected"
     assert_equal TaskStatus.fix_and_resubmit, task2.task_status, "Task should have updated to Fix and Resubmit"
-    assert_equal TaskStatus.fix_and_resubmit, task3.task_status, "Dependent task should have automatically moved to Fix and Resubmit"
+    assert_equal TaskStatus.ready_for_feedback, task3.task_status, "Dependent task should not change without recursive_fix"
     assert_equal TaskStatus.ready_for_feedback, task4.task_status # Task 4 has no prerequsite links
 
     # Reset status
@@ -1501,7 +1501,7 @@ def test_prerequisite_tasks_change_to_fix_and_resubmit
     task4.reload
 
     # Test case 2: Ensure dependent tasks are recursively moved to fix and resubmit
-    task1.assess(TaskStatus.fix_and_resubmit, tutor)
+    task1.assess(TaskStatus.fix_and_resubmit, tutor, Time.zone.now, true)
 
     task1.reload
     task2.reload
@@ -1529,7 +1529,7 @@ def test_prerequisite_tasks_change_to_fix_and_resubmit
     task4.reload
 
     # Test case 3: Ensure tasks that are not Ready for Feedback are not moved to Fix and resubmit
-    task1.assess(TaskStatus.fix_and_resubmit, tutor)
+    task1.assess(TaskStatus.fix_and_resubmit, tutor, Time.zone.now, true)
 
     task1.reload
     task2.reload