From 189464ade2aa32247d94f930f104877db5327eb6 Mon Sep 17 00:00:00 2001
From: Tim Hunt <T.J.Hunt@open.ac.uk>
Date: Fri, 19 Jan 2024 17:12:12 +0000
Subject: [PATCH] MDL-78547 questions: a way to replace a attempt in a usage

... without keeping the old attempt

Co-authored-by: Khoa Nguyen Dang <khoanguyend7@nashtechglobal.com>
---
 question/engine/questionusage.php         | 40 ++++++++++++++++++-----
 question/engine/tests/unitofwork_test.php | 21 ++++++++++++
 question/engine/upgrade.txt               |  7 ++++
 3 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/question/engine/questionusage.php b/question/engine/questionusage.php
index aaf4c86dc21..c1aa1542014 100644
--- a/question/engine/questionusage.php
+++ b/question/engine/questionusage.php
@@ -174,8 +174,10 @@ class question_usage_by_activity {
 
     /**
      * Add another question to this usage, in the place of an existing slot.
-     * The question_attempt that was in that slot is moved to the end at a new
-     * slot number, which is returned.
+     *
+     * Depending on $keepoldquestionattempt, the question_attempt that was in
+     * that slot is moved to the end at a new slot number, which is returned.
+     * Otherwise the existing attempt is completely removed and replaced.
      *
      * The added question is not started until you call {@link start_question()}
      * on it.
@@ -185,14 +187,18 @@ class question_usage_by_activity {
      * @param number $maxmark the maximum this question will be marked out of in
      *      this attempt (optional). If not given, the max mark from the $qa we
      *      are replacing is used.
+     * @param bool $keepoldquestionattempt if true (the default) we keep the existing
+     *      question_attempt, moving it to a new slot
      * @return int the new slot number of the question that was displaced.
      */
-    public function add_question_in_place_of_other($slot, question_definition $question, $maxmark = null) {
-        $newslot = $this->next_slot_number();
+    public function add_question_in_place_of_other(
+        $slot,
+        question_definition $question,
+        $maxmark = null,
+        bool $keepoldquestionattempt = true,
+    ) {
 
         $oldqa = $this->get_question_attempt($slot);
-        $oldqa->set_slot($newslot);
-        $this->questionattempts[$newslot] = $oldqa;
 
         if ($maxmark === null) {
             $maxmark = $oldqa->get_max_mark();
@@ -200,10 +206,26 @@ class question_usage_by_activity {
 
         $qa = new question_attempt($question, $this->get_id(), $this->observer, $maxmark);
         $qa->set_slot($slot);
-        $this->questionattempts[$slot] = $qa;
 
-        $this->observer->notify_attempt_moved($oldqa, $slot);
-        $this->observer->notify_attempt_added($qa);
+        if ($keepoldquestionattempt) {
+            $newslot = $this->next_slot_number();
+            $oldqa->set_slot($newslot);
+            $this->questionattempts[$newslot] = $oldqa;
+
+            $this->observer->notify_attempt_moved($oldqa, $slot);
+            $this->observer->notify_attempt_added($qa);
+
+        } else {
+            $newslot = $slot;
+            $qa->set_database_id($oldqa->get_database_id());
+
+            foreach ($oldqa->get_step_iterator() as $oldstep) {
+                $this->observer->notify_step_deleted($oldstep, $oldqa);
+            }
+            $this->observer->notify_attempt_modified($qa);
+        }
+
+        $this->questionattempts[$slot] = $qa;
 
         return $newslot;
     }
diff --git a/question/engine/tests/unitofwork_test.php b/question/engine/tests/unitofwork_test.php
index 7c85f348aa9..021e1389c80 100644
--- a/question/engine/tests/unitofwork_test.php
+++ b/question/engine/tests/unitofwork_test.php
@@ -507,4 +507,25 @@ class unitofwork_test extends \data_loading_method_test_base {
         $this->assertEquals(array($newslot => array('metathingy' => $this->quba->get_question_attempt($newslot))),
                 $this->observer->get_metadata_added());
     }
+
+    /**
+     * Test add_question_in_place_of_other function.
+     *
+     * @covers ::add_question_in_place_of_other
+     */
+    public function test_replace_old_attempt(): void {
+        // Create a new question.
+        $q = \test_question_maker::make_question('truefalse');
+        $currentquestion = $this->quba->get_question_attempt($this->slot)->get_question();
+        // Replace the current question in the slot with a new one.
+        $slot = $this->quba->add_question_in_place_of_other($this->slot, $q, null, false);
+        $newquestion = $this->quba->get_question_attempt($slot)->get_question();
+
+        $this->assertEquals($this->slot, $slot);
+        $this->assertEquals($q->name, $newquestion->name);
+        $this->assertCount(4, $this->observer->get_steps_deleted());
+        $this->assertCount(1, $this->observer->get_attempts_modified());
+        $this->assertCount(0, $this->observer->get_attempts_added());
+        $this->assertNotEquals($currentquestion->id, $newquestion->id);
+    }
 }
diff --git a/question/engine/upgrade.txt b/question/engine/upgrade.txt
index 28811c88a32..31e5dda60c0 100644
--- a/question/engine/upgrade.txt
+++ b/question/engine/upgrade.txt
@@ -1,5 +1,12 @@
 This files describes API changes for the core question engine.
 
+=== 4.3.4 ===
+
+* The method question_usage_by_activity::add_question_in_place_of_other has been made more flexible.
+  There is a new argument $keepoldquestionattempt. That defaults to true, which behaves the same as
+  the old API, but if you pass false, then the newly added question_attempt completely replaces the
+  existing one in-place.
+
 === 4.2 ===
 
 * A `$questionidentifier` property has been added to `\question_display_options` to enable question type plugins to associate the