Skip to content

Commit

Permalink
Refactors changes to go by code review
Browse files Browse the repository at this point in the history
  • Loading branch information
@matheuszych
matheuszych committed Sep 10, 2024
1 parent e053147 commit 125fb7e
Show file tree
Hide file tree
Showing 18 changed files with 212 additions and 298 deletions.
12 changes: 3 additions & 9 deletions components/ILIAS/Test/classes/Confirmations/class.ilTestSettingsChangeConfirmationGUI.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,14 @@
*/
class ilTestSettingsChangeConfirmationGUI extends ilConfirmationGUI
{
protected ilObjTest $testOBJ;
protected RequestDataCollector $testrequest;
private ?string $oldQuestionSetType;
private ?string $newQuestionSetType;
private ?bool $questionLossInfoEnabled;

public function __construct(
private readonly HTTPServices $http,
private readonly Refinery $refinery,
ilObjTest $testOBJ
private readonly RequestDataCollector $testrequest,
protected readonly ilObjTest $testOBJ
) {
$this->testOBJ = $testOBJ;
$this->testrequest = new RequestDataCollector($this->http, $this->refinery);

parent::__construct();
}

Expand Down Expand Up @@ -102,7 +96,7 @@ public function build(): void

public function populateParametersFromPost(): void
{
foreach (array_keys($this->testrequest->getParsedBody()) as $key) {
foreach ($this->testrequest->getPostKeys() as $key) {
if ($key !== 'cmd') {
$value = $this->testrequest->getArrayOfStringsOrStringFromPost($key);

Expand Down
161 changes: 75 additions & 86 deletions components/ILIAS/Test/classes/class.ilObjTestGUI.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,9 +379,7 @@ public function executeCommand(): void
$this->tabs_gui,
$this->toolbar,
$this->test_question_set_config_factory->getQuestionSetConfig(),
$this->testrequest,
$this->http,
$this->refinery
$this->testrequest
);

$gui->setTestAccess($this->getTestAccess());
Expand Down Expand Up @@ -695,7 +693,7 @@ public function executeCommand(): void
$this->component_repository,
$this->getTestObject(),
$this->questionrepository,
$this->http,
$this->testrequest,
$this->ref_id
);
$this->ctrl->forwardCommand($gui);
Expand Down Expand Up @@ -1659,27 +1657,22 @@ public function confirmRemoveQuestionsObject(array $question_ids_to_remove = [])
$return_to = null;
$deleted_tmp = $question_ids_to_remove;
$first = array_shift($deleted_tmp);

foreach ($questions as $key => $value) {
if (!isset($question_ids_to_remove[$key])) {
if (!in_array($key, $question_ids_to_remove)) {
$prev = $key;
if (!$first) {
$return_to = $prev;
break;
} else {
continue;
}
continue;
}

if ($key !== $first) {
continue;
}

if (!is_null($prev)) {
$return_to = $prev;
break;
} elseif ($key === $first) {
if ($prev !== null) {
$return_to = $prev;
break;
}
$first = array_shift($deleted_tmp);
}

$first = array_shift($deleted_tmp);
}

if (
Expand Down Expand Up @@ -1802,49 +1795,47 @@ public function moveQuestionsObject(): void
}

/**
* Insert checked questions before the actual selection
* @throws ilCtrlException
*/
public function insertQuestionsBeforeObject(): void
* Insert checked questions before the actual selection
*/
public function insertQuestionsBeforeObject()
{
// get all questions to move
$move_questions = ilSession::get('tst_qst_move_' . $this->getTestObject()->getTestId());

$qst_ids = $this->testrequest->getArrayOfIntsFromPost('q_id');

$q_id_count = count($qst_ids);

if ($q_id_count === 0) {
$this->tpl->setOnScreenMessage('failure', $this->lng->txt('no_target_selected_for_move'), true);
$qst_ids = $this->testrequest->getQuestionIds();
if ($qst_ids === []) {
$this->tpl->setOnScreenMessage('failure', $this->lng->txt("no_target_selected_for_move"), true);
$this->ctrl->redirect($this, self::SHOW_QUESTIONS_CMD);
}
if ($q_id_count > 1) {
$this->tpl->setOnScreenMessage('failure', $this->lng->txt('too_many_targets_selected_for_move'), true);
if (count($qst_ids) > 1) {
$this->tpl->setOnScreenMessage('failure', $this->lng->txt("too_many_targets_selected_for_move"), true);
$this->ctrl->redirect($this, self::SHOW_QUESTIONS_CMD);
}
$insert_mode = 0;
$this->getTestObject()->moveQuestions(ilSession::get('tst_qst_move_' . $this->getTestObject()->getTestId()), $qst_ids[0], $insert_mode);
$this->tpl->setOnScreenMessage('success', $this->lng->txt('msg_questions_moved'), true);
$this->getTestObject()->moveQuestions(
ilSession::get('tst_qst_move_' . $this->getTestObject()->getTestId()),
$qst_ids[0],
$insert_mode
);
$this->tpl->setOnScreenMessage('success', $this->lng->txt("msg_questions_moved"), true);
ilSession::clear('tst_qst_move_' . $this->getTestObject()->getTestId());
$this->ctrl->redirect($this, self::SHOW_QUESTIONS_CMD);
$this->ctrl->redirect($this, "showQuestions");
}

/**
* Insert checked questions after the actual selection
* @throws ilCtrlException
*/
public function insertQuestionsAfterObject(): void
* Insert checked questions after the actual selection
*/
public function insertQuestionsAfterObject()
{
$qst_ids = $this->testrequest->getArrayOfIntsFromPost('q_id');

// get all questions to move
$move_questions = ilSession::get('tst_qst_move_' . $this->getTestObject()->getTestId());
$qst_ids = $this->testrequest->getQuestionIds();
if ($qst_ids === []) {
$this->tpl->setOnScreenMessage('failure', $this->lng->txt('no_target_selected_for_move'), true);
$this->tpl->setOnScreenMessage('failure', $this->lng->txt("no_target_selected_for_move"), true);
$this->ctrl->redirect($this, self::SHOW_QUESTIONS_CMD);
}
if (count($qst_ids) > 1) {
$this->tpl->setOnScreenMessage('failure', $this->lng->txt('too_many_targets_selected_for_move'), true);
$this->tpl->setOnScreenMessage('failure', $this->lng->txt("too_many_targets_selected_for_move"), true);
$this->ctrl->redirect($this, self::SHOW_QUESTIONS_CMD);
}
$insert_mode = 1;
Expand All @@ -1853,23 +1844,22 @@ public function insertQuestionsAfterObject(): void
$qst_ids[0],
$insert_mode
);
$this->tpl->setOnScreenMessage('success', $this->lng->txt('msg_questions_moved'), true);
$this->tpl->setOnScreenMessage('success', $this->lng->txt("msg_questions_moved"), true);
ilSession::clear('tst_qst_move_' . $this->getTestObject()->getTestId());
$this->ctrl->redirect($this, self::SHOW_QUESTIONS_CMD);
$this->ctrl->redirect($this, "showQuestions");
}

/**
* Insert questions from the questionbrowser into the test
* @throws ilCtrlException
*/
public function insertQuestionsObject(?array $selected_array = null): void
* Insert questions from the questionbrowser into the test
*
* @access public
*/
public function insertQuestionsObject(array $selected_array = null): void
{
$selected_array = $selected_array ?? $this->testrequest->getArrayOfIntsFromPost('q_id');

if ($selected_array === []) {
$this->tpl->setOnScreenMessage('info', $this->lng->txt('tst_insert_missing_question'), true);
$selected_array = $selected_array ?? $this->testrequest->getQuestionIds();
if (!count($selected_array)) {
$this->tpl->setOnScreenMessage('info', $this->lng->txt("tst_insert_missing_question"), true);
$this->ctrl->redirect($this, 'browseForQuestions');
return;
}

$manscoring = false;
Expand All @@ -1879,7 +1869,7 @@ public function insertQuestionsObject(?array $selected_array = null): void
$value
);
if (!$manscoring) {
$manscoring |= assQuestion::_needsManualScoring($value);
$manscoring = $manscoring | assQuestion::_needsManualScoring($value);
}
}
$this->getTestObject()->saveCompleteStatus($this->test_question_set_config_factory->getQuestionSetConfig());
Expand All @@ -1889,6 +1879,7 @@ public function insertQuestionsObject(?array $selected_array = null): void
$this->tpl->setOnScreenMessage('success', $this->lng->txt('tst_questions_inserted'), true);
}
$this->ctrl->redirect($this, self::SHOW_QUESTIONS_CMD);
return;
}

public function createQuestionFormObject(Form $form = null): void
Expand Down Expand Up @@ -2205,14 +2196,13 @@ public function exportLegacyLogsObject(): void
}

/**
* Evaluates the actions on the participants page
* @throws ilCtrlException
*/
* Evaluates the actions on the participants page
*/
public function participantsActionObject(): void
{
$command = $this->testrequest->getStringFromPost('command', '');

if ($command !== '') {
if ($command === '') {
$method = $command . 'Object';
if (method_exists($this, $method)) {
$this->$method();
Expand Down Expand Up @@ -2372,24 +2362,26 @@ public function defaultsObject()
/**
* Deletes selected test defaults
*/
public function deleteDefaultsObject(): void
public function deleteDefaultsObject()
{
$defaults_ids = $this->testrequest->getArrayOfIntsFromPost('chb_defaults');

foreach ($defaults_ids as $test_default_id) {
$this->getTestObject()->deleteDefaults($test_default_id);
}

if ($defaults_ids === []) {
if ($defaults_ids !== null && $defaults_ids !== []) {
foreach ($defaults_ids as $test_default_id) {
$this->getTestObject()->deleteDefaults($test_default_id);
}
} else {
$this->tpl->setOnScreenMessage('info', $this->lng->txt('select_one'));
}

$this->defaultsObject();
}

public function confirmedApplyDefaultsObject(): void
/**
*
*/
public function confirmedApplyDefaultsObject()
{
$this->applyDefaultsObject(true);
return;
}

/**
Expand All @@ -2399,7 +2391,7 @@ public function applyDefaultsObject($confirmed = false): void
{
$defaults = $this->testrequest->getArrayOfIntsFromPost('chb_defaults');

if (count($defaults) !== 1) {
if ($defaults !== []) {
$this->tpl->setOnScreenMessage('info', $this->lng->txt('tst_defaults_apply_select_one'));

$this->defaultsObject();
Expand Down Expand Up @@ -2440,7 +2432,7 @@ public function applyDefaultsObject($confirmed = false): void

default:

$confirmation = new ilTestSettingsChangeConfirmationGUI($this->http, $this->refinery, $this->getTestObject());
$confirmation = new ilTestSettingsChangeConfirmationGUI($this->testrequest, $this->getTestObject());

$confirmation->setFormAction($this->ctrl->getFormAction($this));
$confirmation->setCancel($this->lng->txt('cancel'), 'defaults');
Expand Down Expand Up @@ -2480,42 +2472,40 @@ public function applyDefaultsObject($confirmed = false): void
/**
* Adds the defaults of this test to the defaults
*/
public function addDefaultsObject(): void
public function addDefaultsObject()
{
$name = $this->testrequest->getStringFromPost('name');

if ($name === '') {
$this->tpl->setOnScreenMessage('info', $this->lng->txt('tst_defaults_enter_name'));
} else {
if ($name !== '') {
$this->getTestObject()->addDefaults($name);
} else {
$this->tpl->setOnScreenMessage('info', $this->lng->txt('tst_defaults_enter_name'));
}
$this->defaultsObject();
}

private function isCommandClassAnyInfoScreenChild(): bool
{
return isset(self::INFO_SCREEN_CHILD_CLASSES[$this->ctrl->getCmdClass()]);
if (in_array($this->ctrl->getCmdClass(), self::INFO_SCREEN_CHILD_CLASSES)) {
return true;
}

return false;
}

/**
* this one is called from the info button in the repository
* not very nice to set cmdClass/Cmd manually, if everything
* works through ilCtrl in the future this may be changed
*
* @throws ilCtrlException|ilDateTimeException
*/
public function infoScreenObject(): void
public function infoScreenObject()
{
// @todo: removed deprecated ilCtrl methods, this needs inspection by a maintainer.
// $this->ctrl->setCmd("showSummary");
// $this->ctrl->setCmdClass("ilinfoscreengui");
$this->infoScreen();
}

/**
* @throws ilCtrlException|ilDateTimeException
*/
public function redirectToInfoScreenObject(): void
public function redirectToInfoScreenObject()
{
// @todo: removed deprecated ilCtrl methods, this needs inspection by a maintainer.
// $this->ctrl->setCmd("showSummary");
Expand All @@ -2524,12 +2514,11 @@ public function redirectToInfoScreenObject(): void
}

/**
* show information screen
* @throws ilCtrlException|ilDateTimeException
*/
public function infoScreen(string $session_lock = '')
* show information screen
*/
public function infoScreen($session_lock = "")
{
if (!$this->access->checkAccess('visible', '', $this->ref_id) && !$this->access->checkAccess('read', '', $this->ref_id)) {
if (!$this->access->checkAccess("visible", "", $this->ref_id) && !$this->access->checkAccess("read", "", $_GET["ref_id"])) {
$this->redirectAfterMissingRead();
}

Expand Down
Loading

0 comments on commit 125fb7e

Please sign in to comment.