Learnpath: Fix prerequisites on test with random questions: was showing wrong maximum - refs GH#3612

Author: @BorjaSanchezBeezNest

public/main/exercise/admin.php

@@ -348,7 +348,8 @@ if ($inATest) {
         $originalSelectionType = $objExercise->questionSelectionType;
         $objExercise->questionSelectionType = EX_Q_SELECTION_ORDERED;
 
-        $fullQuestionsScore = array_reduce(
+        $outMaxScore = 0;
+        $outMaxScore = array_reduce(
             $objExercise->selectQuestionList(true, true),
             function ($acc, $questionId) {
                 $objQuestionTmp = Question::read($questionId);
@@ -362,7 +363,15 @@ if ($inATest) {
         $alert .= sprintf(
             get_lang('%d questions, for a total score (all questions) of %s.'),
             $nbrQuestions,
-            $fullQuestionsScore
+            $outMaxScore
+        );
+    }
+    if ($objExercise->random > 0) {
+        $alert .= '<br />'.sprintf(get_lang('OnlyXQuestionsPickedRandomly'), $objExercise->random);
+        $alert .= sprintf(
+            '<br>'.get_lang('XQuestionsSelectedWithTotalScoreY'),
+            $objExercise->random,
+            $maxScoreAllQuestions
         );
     }
     if ($objExercise->random > 0) {

public/main/exercise/exercise.class.php

@@ -7883,60 +7883,47 @@ class Exercise
     /**
      * Calculate the max_score of the quiz, depending of question inside, and quiz advanced option.
      */
-    public function get_max_score()
+    public function getMaxScore()
     {
-        $out_max_score = 0;
+        $outMaxScore = 0;
         // list of question's id !!! the array key start at 1 !!!
         $questionList = $this->selectQuestionList(true);
 
-        // test is randomQuestions - see field random of test
-        if ($this->random > 0 && 0 == $this->randomByCat) {
-            $numberRandomQuestions = $this->random;
-            $questionScoreList = [];
-            foreach ($questionList as $questionId) {
-                $tmpobj_question = Question::read($questionId);
-                if (is_object($tmpobj_question)) {
-                    $questionScoreList[] = $tmpobj_question->weighting;
-                }
-            }
-
-            rsort($questionScoreList);
-            // add the first $numberRandomQuestions value of score array to get max_score
-            for ($i = 0; $i < min($numberRandomQuestions, count($questionScoreList)); $i++) {
-                $out_max_score += $questionScoreList[$i];
-            }
-        } elseif ($this->random > 0 && $this->randomByCat > 0) {
+        if ($this->random > 0 && $this->randomByCat > 0) {
             // test is random by category
             // get the $numberRandomQuestions best score question of each category
             $numberRandomQuestions = $this->random;
-            $tab_categories_scores = [];
+            $tabCategoriesScores = [];
             foreach ($questionList as $questionId) {
-                $question_category_id = TestCategory::getCategoryForQuestion($questionId);
-                if (!is_array($tab_categories_scores[$question_category_id])) {
-                    $tab_categories_scores[$question_category_id] = [];
+                $questionCategoryId = TestCategory::getCategoryForQuestion($questionId);
+                if (!is_array($tabCategoriesScores[$questionCategoryId])) {
+                    $tabCategoriesScores[$questionCategoryId] = [];
                 }
-                $tmpobj_question = Question::read($questionId);
-                if (is_object($tmpobj_question)) {
-                    $tab_categories_scores[$question_category_id][] = $tmpobj_question->weighting;
+                $tmpObjQuestion = Question::read($questionId);
+                if (is_object($tmpObjQuestion)) {
+                    $tabCategoriesScores[$questionCategoryId][] = $tmpObjQuestion->weighting;
                 }
             }
 
             // here we've got an array with first key, the category_id, second key, score of question for this cat
-            foreach ($tab_categories_scores as $tab_scores) {
-                rsort($tab_scores);
-                for ($i = 0; $i < min($numberRandomQuestions, count($tab_scores)); $i++) {
-                    $out_max_score += $tab_scores[$i];
+            foreach ($tabCategoriesScores as $tabScores) {
+                rsort($tabScores);
+                $tabScoresCount = count($tabScores);
+                for ($i = 0; $i < min($numberRandomQuestions, $tabScoresCount); $i++) {
+                    $outMaxScore += $tabScores[$i];
                 }
             }
-        } else {
-            // standard test, just add each question score
-            foreach ($questionList as $questionId) {
-                $question = Question::read($questionId, $this->course);
-                $out_max_score += $question->weighting;
-            }
+
+            return $outMaxScore;
+        }
+
+        // standard test, just add each question score
+        foreach ($questionList as $questionId) {
+            $question = Question::read($questionId, $this->course);
+            $outMaxScore += $question->weighting;
         }
 
-        return $out_max_score;
+        return $outMaxScore;
     }
 
     /**
@@ -8691,7 +8678,7 @@ class Exercise
                         ->setUserScoreList($students)
                         ->setBestScore($bestResult)
                         ->setAverageScore($average)
-                        ->setScoreWeight($this->get_max_score());
+                        ->setScoreWeight($this->getMaxScore());
                     $em->persist($exerciseLink);
                     $em->flush();
                 }

public/main/exercise/exercise_result.php

@@ -126,7 +126,7 @@ if ('learnpath' === $origin) {
     ';
 }
 
-$i = $total_score = $max_score = 0;
+$i = $total_score = $maxScore = 0;
 $remainingMessage = '';
 $attemptButton = '';
 
@@ -188,7 +188,7 @@ if (!empty($exercise_stat_info)) {
     $total_score = $exercise_stat_info['score'];
 }
 
-$max_score = $objExercise->get_max_score();
+$maxScore = $objExercise->getMaxScore();
 
 if ('embeddable' === $origin) {
     $pageTop .= showEmbeddableFinishButton();
@@ -298,7 +298,7 @@ if (!in_array($origin, ['learnpath', 'embeddable', 'mobileapp'])) {
     Session::write('attempt_remaining', $remainingMessage);
 
     // Record the results in the learning path, using the SCORM interface (API)
-    $pageBottom .= "<script>window.parent.API.void_save_asset('$total_score', '$max_score', 0, 'completed');</script>";
+    $pageBottom .= "<script>window.parent.API.void_save_asset('$total_score', '$maxScore', 0, 'completed');</script>";
     $pageBottom .= '<script type="text/javascript">'.$href.'</script>';
 
     $showFooter = false;

public/main/lp/learnpath.class.php

@@ -401,7 +401,7 @@ class learnpath
      * @param string  $title
      * @param string  $description
      * @param int     $prerequisites
-     * @param int     $max_time_allowed
+     * @param int     $maxTimeAllowed
      * @param int     $userId
      *
      * @return int
@@ -414,7 +414,7 @@ class learnpath
         $title,
         $description = '',
         $prerequisites = 0,
-        $max_time_allowed = 0
+        $maxTimeAllowed = 0
     ) {
         $type = empty($type) ? 'dir' : $type;
         $course_id = $this->course_info['real_id'];
@@ -424,16 +424,16 @@ class learnpath
             $course_id = $this->course_info['real_id'];
         }
         $id = (int) $id;
-        $max_time_allowed = (int) $max_time_allowed;
-        if (empty($max_time_allowed)) {
-            $max_time_allowed = 0;
+        $maxTimeAllowed = (int) $maxTimeAllowed;
+        if (empty($maxTimeAllowed)) {
+            $maxTimeAllowed = 0;
         }
-        $max_score = 100;
+        $maxScore = 100;
         if ('quiz' === $type && $id) {
             // Disabling the exercise if we add it inside a LP
             $exercise = new Exercise($course_id);
             $exercise->read($id);
-            $max_score = $exercise->get_max_score();
+            $maxScore = $exercise->getMaxScore();
 
             $exercise->disable();
             $exercise->save();
@@ -446,8 +446,8 @@ class learnpath
             ->setPath($id)
             ->setLp(api_get_lp_entity($this->get_id()))
             ->setItemType($type)
-            ->setMaxScore($max_score)
-            ->setMaxTimeAllowed($max_time_allowed)
+            ->setMaxScore($maxScore)
+            ->setMaxTimeAllowed($maxTimeAllowed)
             ->setPrerequisite($prerequisites)
             //->setDisplayOrder($display_order + 1)
             //->setNextItemId((int) $next)
@@ -6009,9 +6009,9 @@ class learnpath
      *
      * @return string HTML form
      */
-    public function display_item_prerequisites_form(CLpItem $lpItem)
+    public function displayItemPrerequisitesForm(CLpItem $lpItem)
     {
-        $course_id = api_get_course_int_id();
+        $courseId = api_get_course_int_id();
         $preRequisiteId = $lpItem->getPrerequisite();
         $itemId = $lpItem->getIid();
 
@@ -6039,8 +6039,8 @@ class learnpath
         $return .= '</tr>';
 
         // @todo use entitites
-        $tbl_lp_item = Database::get_course_table(TABLE_LP_ITEM);
-        $sql = "SELECT * FROM $tbl_lp_item
+        $tblLpItem = Database::get_course_table(TABLE_LP_ITEM);
+        $sql = "SELECT * FROM $tblLpItem
                 WHERE lp_id = ".$this->lp_id;
         $result = Database::query($sql);
 
@@ -6063,32 +6063,28 @@ class learnpath
         $currentItemId = $itemId;
         $options = [
             'decorate' => true,
-            'rootOpen' => function() {
+            'rootOpen' => function () {
                 return '';
             },
-            'rootClose' => function() {
+            'rootClose' => function () {
                 return '';
             },
-            'childOpen' => function() {
+            'childOpen' => function () {
                 return '';
             },
             'childClose' => '',
             'nodeDecorator' => function ($item) use (
                 $currentItemId,
                 $preRequisiteId,
-                $course_id,
+                $courseId,
                 $selectedMaxScore,
                 $selectedMinScore,
                 $displayOrder,
                 $lpItemRepo,
                 $em
             ) {
-                $mainUrl = '';
-                $fullTitle = $item['title'];
-                $title = cut($fullTitle, self::MAX_LP_ITEM_TITLE_LENGTH);
                 $itemId = $item['iid'];
                 $type = $item['itemType'];
-                $lpId = $this->get_id();
                 $iconName = str_replace(' ', '', $type);
                 $icon = Display::return_icon(
                     'lp_'.$iconName.'.png',
@@ -6096,7 +6092,6 @@ class learnpath
                     [],
                     ICON_SIZE_TINY
                 );
-                $url = $mainUrl.'&view=build&id='.$itemId.'&lp_id='.$lpId;
 
                 if ($itemId == $currentItemId) {
                     return '';
@@ -6134,16 +6129,16 @@ class learnpath
                 $return .= '</td>';
 
                 if (TOOL_QUIZ == $type) {
-                    // lets update max_score Tests information depending of the Tests Advanced properties
-                    $exercise = new Exercise($course_id);
+                    // let's update max_score Tests information depending of the Tests Advanced properties
+                    $exercise = new Exercise($courseId);
                     /** @var CLpItem $itemEntity */
                     $itemEntity = $lpItemRepo->find($itemId);
                     $exercise->read($item['path']);
-                    $itemEntity->setMaxScore($exercise->get_max_score());
+                    $itemEntity->setMaxScore($exercise->getMaxScore());
                     $em->persist($itemEntity);
                     $em->flush($itemEntity);
 
-                    $item['maxScore'] = $exercise->get_max_score();
+                    $item['maxScore'] = $exercise->getMaxScore();
 
                     if (empty($selectedMinScoreValue) && !empty($masteryScoreAsMinValue)) {
                         // Backwards compatibility with 1.9.x use mastery_score as min value

public/main/lp/lp_edit_item_prereq.php

@@ -61,7 +61,7 @@ if (isset($is_success) && true == $is_success) {
     $right .= Display::return_message(get_lang('Prerequisites to the current learning object have been added.'));
 } else {
     $right .= $lp->displayItemMenu($lpItem);
-    $right .= $lp->display_item_prerequisites_form($lpItem);
+    $right .= $lp->displayItemPrerequisitesForm($lpItem);
 }
 
 $tpl = new Template(get_lang('Prerequisites'));