fix: remove request superglobal access by replacing it with InternalR…

…equestService (41683/41684)
ILIAS-eLearning · Nov 5, 2024 · e249fde · e249fde
1 parent e032226
commit e249fde
Showing 1 changed file with 49 additions and 53 deletions.
diff --git a/Modules/Test/classes/class.ilObjTestGUI.php b/Modules/Test/classes/class.ilObjTestGUI.php
@@ -91,6 +91,9 @@ class ilObjTestGUI extends ilObjectGUI implements ilCtrlBaseClassInterface
 
     protected array $ui;
 
+    protected const INSERT_MODE_BEFORE = 0;
+    protected const INSERT_MODE_AFTER = 1;
+
     /**
      * Constructor
      * @access public
@@ -1885,14 +1888,12 @@ public function removeQuestionsObject()
     */
     public function moveQuestionsObject()
     {
-        $selected_questions = null;
-        $selected_questions = $_POST['q_id'];
-        if ($selected_questions === null && is_numeric($_GET['q_id'])) {
-            $selected_questions = [$_GET['q_id']];
-        }
-        if (is_array($selected_questions)) {
+        $selected_questions = $this->testrequest->getQuestionIds()
+            ? $this->testrequest->getQuestionIds()
+            : [$this->testrequest->getQuestionId()];
+        if (count($selected_questions) > 0) {
             ilSession::set('tst_qst_move_' . $this->object->getTestId(), $selected_questions);
-            $this->tpl->setOnScreenMessage('success', $this->lng->txt("msg_selected_for_move"), true);
+            $this->tpl->setOnScreenMessage('success', $this->lng->txt('msg_selected_for_move'), true);
         } else {
             $this->tpl->setOnScreenMessage('failure', $this->lng->txt('no_selection_for_move'), true);
         }
@@ -1904,44 +1905,38 @@ public function moveQuestionsObject()
     */
     public function insertQuestionsBeforeObject()
     {
-        // get all questions to move
-        $move_questions = ilSession::get('tst_qst_move_' . $this->object->getTestId());
-
-        if (!is_array($_POST['q_id']) || 0 === count($_POST['q_id'])) {
-            $this->tpl->setOnScreenMessage('failure', $this->lng->txt("no_target_selected_for_move"), true);
-            $this->ctrl->redirect($this, 'questions');
-        }
-        if (count($_POST['q_id']) > 1) {
-            $this->tpl->setOnScreenMessage('failure', $this->lng->txt("too_many_targets_selected_for_move"), true);
-            $this->ctrl->redirect($this, 'questions');
-        }
-        $insert_mode = 0;
-        $this->object->moveQuestions(ilSession::get('tst_qst_move_' . $this->object->getTestId()), $_POST['q_id'][0], $insert_mode);
-        $this->tpl->setOnScreenMessage('success', $this->lng->txt("msg_questions_moved"), true);
-        ilSession::clear('tst_qst_move_' . $this->object->getTestId());
-        $this->ctrl->redirect($this, "questions");
+        $this->insertQuestionsBeforeOrAfter(self::INSERT_MODE_BEFORE);
     }
 
     /**
     * Insert checked questions after the actual selection
     */
     public function insertQuestionsAfterObject()
     {
-        // get all questions to move
-        $move_questions = ilSession::get('tst_qst_move_' . $this->object->getTestId());
-        if (!is_array($_POST['q_id']) || 0 === count($_POST['q_id'])) {
-            $this->tpl->setOnScreenMessage('failure', $this->lng->txt("no_target_selected_for_move"), true);
+        $this->insertQuestionsBeforeOrAfter(self::INSERT_MODE_AFTER);
+    }
+
+    /**
+     * @param integer $insert_mode 0, if insert before the target position, 1 if insert after the target position
+     */
+    protected function insertQuestionsBeforeOrAfter(int $insert_mode)
+    {
+        $target_questions = $this->testrequest->getQuestionIds()
+            ? $this->testrequest->getQuestionIds()
+            : [$this->testrequest->getQuestionId()];
+
+        if (count($target_questions) === 0) {
+            $this->tpl->setOnScreenMessage('failure', $this->lng->txt('no_target_selected_for_move'), true);
             $this->ctrl->redirect($this, 'questions');
-        }
-        if (count($_POST['q_id']) > 1) {
-            $this->tpl->setOnScreenMessage('failure', $this->lng->txt("too_many_targets_selected_for_move"), true);
+        } elseif (count($target_questions) > 1) {
+            $this->tpl->setOnScreenMessage('failure', $this->lng->txt('too_many_targets_selected_for_move'), true);
             $this->ctrl->redirect($this, 'questions');
         }
-        $insert_mode = 1;
-        $this->object->moveQuestions(ilSession::get('tst_qst_move_' . $this->object->getTestId()), $_POST['q_id'][0], $insert_mode);
-        $this->tpl->setOnScreenMessage('success', $this->lng->txt("msg_questions_moved"), true);
+
+        $this->object->moveQuestions(ilSession::get('tst_qst_move_' . $this->object->getTestId()), $target_questions[0], $insert_mode);
+        $this->tpl->setOnScreenMessage('success', $this->lng->txt('msg_questions_moved'), true);
         ilSession::clear('tst_qst_move_' . $this->object->getTestId());
-        $this->ctrl->redirect($this, "questions");
+        $this->ctrl->redirect($this, 'questions');
     }
 
     /**
@@ -1951,27 +1946,28 @@ public function insertQuestionsAfterObject()
     */
     public function insertQuestionsObject()
     {
-        $selected_array = (is_array($_POST['q_id'])) ? $_POST['q_id'] : array();
-        if (!count($selected_array)) {
-            $this->tpl->setOnScreenMessage('info', $this->lng->txt("tst_insert_missing_question"), true);
-            $this->ctrl->redirect($this, "browseForQuestions");
+        $selected_questions = $this->testrequest->getQuestionIds()
+            ? $this->testrequest->getQuestionIds()
+            : [$this->testrequest->getQuestionId()];
+
+        if (count($selected_questions) === 0) {
+            $this->tpl->setOnScreenMessage('info', $this->lng->txt('tst_insert_missing_question'), true);
+            $this->ctrl->redirect($this, 'browseForQuestions');
+        }
+
+        $man_scoring = false;
+        foreach ($selected_questions as $key => $value) {
+            $this->object->insertQuestion($this->testQuestionSetConfigFactory->getQuestionSetConfig(), $value);
+            $man_scoring = $man_scoring || assQuestion::_needsManualScoring($value);
+        }
+
+        $this->object->saveCompleteStatus($this->testQuestionSetConfigFactory->getQuestionSetConfig());
+        if ($man_scoring) {
+            $this->tpl->setOnScreenMessage('info', $this->lng->txt('manscoring_hint'), true);
         } else {
-            $manscoring = false;
-            foreach ($selected_array as $key => $value) {
-                $this->object->insertQuestion($this->testQuestionSetConfigFactory->getQuestionSetConfig(), $value);
-                if (!$manscoring) {
-                    $manscoring = $manscoring | assQuestion::_needsManualScoring($value);
-                }
-            }
-            $this->object->saveCompleteStatus($this->testQuestionSetConfigFactory->getQuestionSetConfig());
-            if ($manscoring) {
-                $this->tpl->setOnScreenMessage('info', $this->lng->txt("manscoring_hint"), true);
-            } else {
-                $this->tpl->setOnScreenMessage('success', $this->lng->txt("tst_questions_inserted"), true);
-            }
-            $this->ctrl->redirect($this, "questions");
-            return;
+            $this->tpl->setOnScreenMessage('success', $this->lng->txt('tst_questions_inserted'), true);
         }
+        $this->ctrl->redirect($this, 'questions');
     }
 
     public function addQuestionObject()