From 2a2d3c85bb7f6498537f32bf906549693a99e765 Mon Sep 17 00:00:00 2001
From: Laurent David <laurent.david@moodle.com>
Date: Tue, 30 May 2023 15:22:15 +0200
Subject: [PATCH] MDL-71414 mod_h5pactivity: Fix issue with drop down report

* Drag and Drop report were missing answers when target could be dropped on multiple
drop zones.
---
 .../classes/output/result/matching.php        | 77 ++++++++++---------
 .../tests/output/result/result_test.php       | 64 ++++++++++-----
 2 files changed, 88 insertions(+), 53 deletions(-)

diff --git a/mod/h5pactivity/classes/output/result/matching.php b/mod/h5pactivity/classes/output/result/matching.php
index 645afdfdcc6..2979fa183a8 100644
--- a/mod/h5pactivity/classes/output/result/matching.php
+++ b/mod/h5pactivity/classes/output/result/matching.php
@@ -27,7 +27,6 @@ namespace mod_h5pactivity\output\result;
 defined('MOODLE_INTERNAL') || die();
 
 use mod_h5pactivity\output\result;
-use renderer_base;
 
 /**
  * Class to display H5P matching result.
@@ -40,7 +39,7 @@ class matching extends result {
     /**
      * Return the options data structure.
      *
-     * @return array of options
+     * @return array|null of options
      */
     protected function export_options(): ?array {
         // Suppose H5P choices have only list of valid answers.
@@ -50,9 +49,9 @@ class matching extends result {
 
         // Get sources (options).
         if (isset($additionals->source)) {
-            $options = $this->get_descriptions($additionals->source);
+            $sources = $this->get_descriptions($additionals->source);
         } else {
-            $options = [];
+            $sources = [];
         }
 
         // Get targets.
@@ -61,55 +60,63 @@ class matching extends result {
         } else {
             $targets = [];
         }
+        // Create original options array.
+        $options = array_map(function ($source) {
+            $cloneddraggable = clone $source;
+            $cloneddraggable->correctanswers = [];
+            return $cloneddraggable;
+        }, $sources);
 
-        // Correct answers.
+        // Fill options with correct answers flags if they exist.
         foreach ($correctpattern as $pattern) {
             if (!is_array($pattern) || count($pattern) != 2) {
                 continue;
             }
-            // One pattern must be from options and the other from targets.
-            if (isset($options[$pattern[0]]) && isset($targets[$pattern[1]])) {
-                $option = $options[$pattern[0]];
-                $target = $targets[$pattern[1]];
-            } else if (isset($targets[$pattern[0]]) && isset($options[$pattern[1]])) {
-                $option = $options[$pattern[1]];
+            // We assume here that the activity is following the convention sets in:
+            // https://github.com/h5p/h5p-php-report/blob/master/type-processors/matching-processor.class.php
+            // i.e. source is index 1 and dropzone is index 0.
+            if (isset($sources[$pattern[1]]) && isset($targets[$pattern[0]])) {
                 $target = $targets[$pattern[0]];
-            } else {
-                $option = null;
-            }
-            if ($option) {
-                $option->correctanswer = $this->get_answer(parent::TEXT, $target->description);
-                $option->correctanswerid = $target->id;
+                $source = $sources[$pattern[1]];
+                $currentoption = $options[$source->id];
+                $currentoption->correctanswers[$target->id] = $target->description;
             }
         }
 
-        // User responses.
+        // Fill in user responses.
         foreach ($this->response as $response) {
             if (!is_array($response) || count($response) != 2) {
                 continue;
             }
-            // One repsonse must be from options and the other from targets.
-            if (isset($options[$response[0]]) && isset($targets[$response[1]])) {
-                $option = $options[$response[0]];
-                $target = $targets[$response[1]];
-                $answer = $response[1];
-            } else if (isset($targets[$response[0]]) && isset($options[$response[1]])) {
-                $option = $options[$response[1]];
+            if (isset($sources[$response[1]]) && isset($targets[$response[0]])) {
+                $source = $sources[$response[1]];
                 $target = $targets[$response[0]];
                 $answer = $response[0];
-            } else {
-                $option = null;
-            }
-            if ($option) {
-                if (isset($option->correctanswerid) && $option->correctanswerid == $answer) {
-                    $state = parent::CORRECT;
-                } else {
-                    $state = parent::INCORRECT;
+                $option = $options[$source->id] ?? null;
+                if ($option) {
+                    if (isset($option->correctanswers[$answer])) {
+                        $state = parent::CORRECT;
+                    } else {
+                        $state = parent::INCORRECT;
+                    }
+                    $option->useranswer = $this->get_answer($state, $target->description);
                 }
-                $option->useranswer = $this->get_answer($state, $target->description);
             }
         }
-        return $options;
+
+        // Fill in unchecked options.
+        foreach ($options as $option) {
+            if (!isset($option->useranswer)) {
+                $option->useranswer = $this->get_answer(parent::UNCHECKED, '');
+            }
+        }
+
+        // Now flattern correct answers.
+        foreach ($options as $option) {
+            $option->correctanswer = $this->get_answer( parent::TEXT, join(', ', $option->correctanswers));
+            unset($option->correctanswers);
+        }
+        return array_values($options);
     }
 
     /**
diff --git a/mod/h5pactivity/tests/output/result/result_test.php b/mod/h5pactivity/tests/output/result/result_test.php
index 731089aae01..297b84f14f6 100644
--- a/mod/h5pactivity/tests/output/result/result_test.php
+++ b/mod/h5pactivity/tests/output/result/result_test.php
@@ -78,8 +78,13 @@ class result_test extends \advanced_testcase {
 
         $data = $exportoptions->invoke($resultoutput);
         $useranswersdata = array_map(function($item) {
-            return $item->useranswer;
+            return $item->useranswer ?? null;
         }, $data);
+        $keys = array_map(function($item) {
+            return $item->description . ' - ' . ($item->correctanswer->answer ?? '');
+        }, $data);
+
+        $useranswersdata = array_combine($keys, $useranswersdata);
         $this->assertEquals($expecteduseranswers, $useranswersdata);
     }
 
@@ -103,8 +108,8 @@ class result_test extends \advanced_testcase {
                         . '"https:\\/\\/h5p.org\\/x-api\\/alternatives":[["cat"],["dog"]]},"contextExtensions":{}}',
                 ],
                 'useranswers' => [
-                    (object) ['answer' => 'Cat', 'incorrect' => true],
-                    (object) ['answer' => 'dog', 'correct' => true],
+                    'Gap #1 - cat' => (object) ['answer' => 'Cat', 'incorrect' => true],
+                    'Gap #2 - dog' => (object) ['answer' => 'dog', 'correct' => true],
                 ],
             ],
             'fill-in with case insensitive' => [
@@ -120,8 +125,31 @@ class result_test extends \advanced_testcase {
                         . '"https:\\/\\/h5p.org\\/x-api\\/alternatives":[["cat"],["dog"]]},"contextExtensions":{}}',
                 ],
                 'useranswers' => [
-                    (object) ['answer' => 'Cat', 'correct' => true],
-                    (object) ['answer' => 'dog', 'correct' => true],
+                    'Gap #1 - cat' => (object) ['answer' => 'Cat', 'correct' => true],
+                    'Gap #2 - dog' => (object) ['answer' => 'dog', 'correct' => true],
+                ],
+            ],
+            'drag and drop' => [
+                'result' => [
+                    'interactiontype' => 'matching',
+                    'description' => 'Drag and Drop Test',
+                    'correctpattern' => '["0[.]0[,]0[.]2[,]1[.]1[,]1[.]0"]',
+                    'response' => '0[.]0[,]1[.]1[,]0[.]2[,]0[.]3',
+                    'additionals' => '{"source":[{"id":"0","description":{"en-US":"Answer 1 (DZ1 and DZ2)\n"}},'
+                        . '{"id":"1","description":{"en-US":"Anwser 2 (DZ2)\n"}},'
+                        . '{"id":"2","description":{"en-US":"Anwser 3 (DZ1)\n"}},'
+                        . '{"id":"3","description":{"en-US":"Anwser 4 (neither)\n"}}],'
+                        . '"target":[{"id":"0","description":{"en-US":"Dropzone 1\n"}},'
+                        . '{"id":"1","description":{"en-US":"Dropzone 2\n"}}],'
+                        . '"extensions":{"http:\/\/h5p.org\/x-api\/h5p-local-content-id":41,'
+                        . '"http:\/\/h5p.org\/x-api\/h5p-subContentId":"59590246-f16e-4855-8dd6-c80e892ef96b"},'
+                        . '"contextExtensions":{}}',
+                ],
+                'useranswers' => [
+                    'Answer 1 (DZ1 and DZ2) - Dropzone 1, Dropzone 2' => (object) ['answer' => 'Dropzone 1', 'correct' => true, ],
+                    'Anwser 2 (DZ2) - Dropzone 2' => (object) ['answer' => 'Dropzone 2', 'correct' => true, ],
+                    'Anwser 3 (DZ1) - Dropzone 1' => (object) ['answer' => 'Dropzone 1', 'correct' => true, ],
+                    'Anwser 4 (neither) - ' => (object) ['answer' => 'Dropzone 1', 'incorrect' => true, ]
                 ],
             ],
             'sort the paragraph text' => [
@@ -141,11 +169,11 @@ class result_test extends \advanced_testcase {
                         . '"https:\\/\\/h5p.org\\/x-api\\/duplicates-interchangeable":1},"contextExtensions":{}}',
                 ],
                 'useranswers' => [
-                    (object) ['answer' => 'Correct answer', 'pass' => true, ],
-                    (object) ['answer' => 'Correct answer', 'pass' => true, ],
-                    (object) ['answer' => 'Incorrect answer', 'fail' => true, ],
-                    (object) ['answer' => 'Incorrect answer', 'fail' => true, ],
-                    (object) ['answer' => 'Correct answer', 'pass' => true, ],
+                    '#1 - First I wake up at 7.30 am' => (object) ['answer' => 'Correct answer', 'pass' => true, ],
+                    '#2 - Next I get dressed' => (object) ['answer' => 'Correct answer', 'pass' => true, ],
+                    '#3 - Afterward I have breakfast' => (object) ['answer' => 'Incorrect answer', 'fail' => true, ],
+                    '#4 - I brush my teeth' => (object) ['answer' => 'Incorrect answer', 'fail' => true, ],
+                    '#5 - Finally I go school' => (object) ['answer' => 'Correct answer', 'pass' => true, ],
                 ],
             ],
             'sequencing images' => [
@@ -162,14 +190,14 @@ class result_test extends \advanced_testcase {
                         . '{"http:\/\/h5p.org\/x-api\/h5p-local-content-id":43},"contextExtensions":{}}',
                 ],
                 'useranswers' => [
-                    (object) ['answer' => 'Correct answer', 'pass' => true, ],
-                    (object) ['answer' => 'Correct answer', 'pass' => true, ],
-                    (object) ['answer' => 'Incorrect answer', 'fail' => true, ],
-                    (object) ['answer' => 'Incorrect answer', 'fail' => true, ],
-                    (object) ['answer' => 'Incorrect answer', 'fail' => true, ],
-                    (object) ['answer' => 'Incorrect answer', 'fail' => true, ],
-                    (object) ['answer' => 'Correct answer', 'pass' => true, ],
-                    (object) ['answer' => 'Correct answer', 'pass' => true, ],
+                    '#1 - Mercury' => (object) ['answer' => 'Correct answer', 'pass' => true, ],
+                    '#2 - Mars' => (object) ['answer' => 'Correct answer', 'pass' => true, ],
+                    '#3 - Earth' => (object) ['answer' => 'Incorrect answer', 'fail' => true, ],
+                    '#4 - Venus' => (object) ['answer' => 'Incorrect answer', 'fail' => true, ],
+                    '#5 - Uranus' => (object) ['answer' => 'Incorrect answer', 'fail' => true, ],
+                    '#6 - Neptune' => (object) ['answer' => 'Incorrect answer', 'fail' => true, ],
+                    '#7 - Saturn' => (object) ['answer' => 'Correct answer', 'pass' => true, ],
+                    '#8 - Jupiter' => (object) ['answer' => 'Correct answer', 'pass' => true, ],
                 ],
             ]
         ];