From a376a2b06d0f32693df4a1890ed040a3e6ae5d06 Mon Sep 17 00:00:00 2001 From: Lukas Eichenauer Date: Mon, 4 Nov 2024 13:32:22 +0100 Subject: [PATCH] fix: remove request superglobal access by replacing it with InternalRequestService (41683/41684) --- Modules/Test/classes/class.ilObjTestGUI.php | 102 ++++++++++---------- 1 file changed, 49 insertions(+), 53 deletions(-) diff --git a/Modules/Test/classes/class.ilObjTestGUI.php b/Modules/Test/classes/class.ilObjTestGUI.php index 3668f51a4e1b..0ace7c8bb809 100755 --- 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 @@ -1881,14 +1884,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); } @@ -1900,22 +1901,7 @@ 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); } /** @@ -1923,21 +1909,30 @@ public function insertQuestionsBeforeObject() */ 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'); } /** @@ -1947,27 +1942,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()