From fb1aeb84612ab9f21acdc69d3bdaa2466718e6cf Mon Sep 17 00:00:00 2001 From: Julio Montoya Date: Mon, 22 Jun 2020 10:09:56 +0200 Subject: [PATCH] Fix exercise edition UI see BT#17438 --- main/exercise/exercise.class.php | 108 ++++++++++++------ main/exercise/exercise_admin.php | 19 ++- main/inc/lib/pear/HTML/QuickForm/checkbox.php | 4 +- main/inc/lib/pear/HTML/QuickForm/element.php | 12 +- main/inc/lib/pear/HTML/QuickForm/input.php | 4 +- main/inc/lib/pear/HTML/QuickForm/radio.php | 18 +-- 6 files changed, 90 insertions(+), 75 deletions(-) diff --git a/main/exercise/exercise.class.php b/main/exercise/exercise.class.php index ec06e62222..81162fec29 100755 --- a/main/exercise/exercise.class.php +++ b/main/exercise/exercise.class.php @@ -1203,6 +1203,30 @@ class Exercise return Database::num_rows($result) > 0; } + public function hasQuestionWithTypeNotInList(array $questionTypeList) + { + if (empty($questionTypeList)) { + return false; + } + + $questionTypeToString = implode("','", array_map('intval', $questionTypeList)); + + $table = Database::get_course_table(TABLE_QUIZ_TEST_QUESTION); + $tableQuestion = Database::get_course_table(TABLE_QUIZ_QUESTION); + $sql = "SELECT q.id + FROM $table e + INNER JOIN $tableQuestion q + ON (e.question_id = q.id AND e.c_id = q.c_id) + WHERE + q.type NOT IN ('$questionTypeToString') AND + e.c_id = {$this->course_id} AND + e.exercice_id = ".$this->id; + + $result = Database::query($sql); + + return Database::num_rows($result) > 0; + } + /** * changes the exercise title. * @@ -1633,7 +1657,7 @@ class Exercise api_get_user_id() ); - if (api_get_setting('search_enabled') == 'true') { + if (api_get_setting('search_enabled') === 'true') { $this->search_engine_edit(); } } else { @@ -1739,7 +1763,7 @@ class Exercise $this->course ); - if (api_get_setting('search_enabled') == 'true' && extension_loaded('xapian')) { + if (api_get_setting('search_enabled') === 'true' && extension_loaded('xapian')) { $this->search_engine_save(); } } @@ -1988,7 +2012,7 @@ class Exercise ); $skillList = []; - if ($type === 'full') { + if ('full' === $type) { // Can't modify a DirectFeedback question. if (!in_array($this->getFeedbackType(), [EXERCISE_FEEDBACK_TYPE_DIRECT, EXERCISE_FEEDBACK_TYPE_POPUP])) { $this->setResultFeedbackGroup($form); @@ -2024,7 +2048,7 @@ class Exercise $form->addGroup($radios, null, get_lang('QuestionsPerPage')); } else { // if is Direct feedback but has not questions we can allow to modify the question type - if ($this->getQuestionCount() === 0) { + if (0 === $this->getQuestionCount()) { $this->setResultFeedbackGroup($form); $this->setResultDisabledGroup($form); @@ -2040,12 +2064,13 @@ class Exercise ); $form->addGroup($radios, null, get_lang('ExerciseType')); } else { + $this->setResultFeedbackGroup($form, true); $group = $this->setResultDisabledGroup($form); $group->freeze(); // we force the options to the DirectFeedback exercisetype - $form->addElement('hidden', 'exerciseFeedbackType', $this->getFeedbackType()); - $form->addElement('hidden', 'exerciseType', ONE_PER_PAGE); + //$form->addElement('hidden', 'exerciseFeedbackType', $this->getFeedbackType()); + //$form->addElement('hidden', 'exerciseType', ONE_PER_PAGE); // Type of questions disposition on page $radios[] = $form->createElement( @@ -2545,11 +2570,10 @@ class Exercise } } - /** - * @param $form - */ - public function setResultFeedbackGroup(FormValidator $form) + public function setResultFeedbackGroup(FormValidator $form, $checkFreeze = true) { + $freeze = false; + // Feedback type. $feedback = []; $feedback[] = $form->createElement( @@ -2564,43 +2588,47 @@ class Exercise ] ); - if (api_get_setting('enable_quiz_scenario') === 'true') { - // Can't convert a question from one feedback to another - // if there is more than 1 question already added - if ($this->selectNbrQuestions() == 0) { - $feedback[] = $form->createElement( - 'radio', - 'exerciseFeedbackType', - null, - get_lang('DirectFeedback'), - EXERCISE_FEEDBACK_TYPE_DIRECT, - [ - 'id' => 'exerciseType_'.EXERCISE_FEEDBACK_TYPE_DIRECT, - 'onclick' => 'check_direct_feedback()', - ] - ); + $freeze = true; + if ('true' === api_get_setting('enable_quiz_scenario')) { + if (0 === $this->getQuestionCount()) { + $freeze = false; + } else { + $hasDifferentQuestion = $this->hasQuestionWithTypeNotInList([UNIQUE_ANSWER, HOT_SPOT_DELINEATION]); + if (false === $hasDifferentQuestion) { + $freeze = false; + } } } - $feedback[] = $form->createElement( + $direct = $form->createElement( 'radio', 'exerciseFeedbackType', null, - get_lang('ExerciseDirectPopUp'), - EXERCISE_FEEDBACK_TYPE_POPUP, - ['id' => 'exerciseType_'.EXERCISE_FEEDBACK_TYPE_POPUP, 'onclick' => 'check_direct_feedback()'] + get_lang('DirectFeedback'), + EXERCISE_FEEDBACK_TYPE_DIRECT, + [ + 'id' => 'exerciseType_'.EXERCISE_FEEDBACK_TYPE_DIRECT, + 'onclick' => 'check_direct_feedback()', + ] ); + if ($freeze) { + $direct->freeze(); + } + + $feedback[] = $direct; + + $feedback[] = $form->createElement( 'radio', 'exerciseFeedbackType', null, - get_lang('NoFeedback'), - EXERCISE_FEEDBACK_TYPE_EXAM, - ['id' => 'exerciseType_'.EXERCISE_FEEDBACK_TYPE_EXAM] + get_lang('ExerciseDirectPopUp'), + EXERCISE_FEEDBACK_TYPE_POPUP, + ['id' => 'exerciseType_'.EXERCISE_FEEDBACK_TYPE_POPUP, 'onclick' => 'check_direct_feedback()'] ); - $form->addGroup( + $group = $form->addGroup( $feedback, null, [ @@ -2608,6 +2636,10 @@ class Exercise get_lang('FeedbackDisplayOptions'), ] ); + + if ($freeze) { + //$group->freeze(); + } } /** @@ -2625,6 +2657,12 @@ class Exercise $this->updateAttempts($form->getSubmitValue('exerciseAttempts')); $this->updateFeedbackType($form->getSubmitValue('exerciseFeedbackType')); $this->updateType($form->getSubmitValue('exerciseType')); + + // If direct feedback then force to One per page + if (EXERCISE_FEEDBACK_TYPE_DIRECT == $form->getSubmitValue('exerciseFeedbackType')) { + $this->updateType(ONE_PER_PAGE); + } + $this->setRandom($form->getSubmitValue('randomQuestions')); $this->updateRandomAnswers($form->getSubmitValue('randomAnswers')); $this->updateResultsDisabled($form->getSubmitValue('results_disabled')); @@ -10415,13 +10453,11 @@ class Exercise ); if (in_array($this->getFeedbackType(), [EXERCISE_FEEDBACK_TYPE_DIRECT, EXERCISE_FEEDBACK_TYPE_POPUP])) { - $group = $form->addGroup( + return $form->addGroup( $resultDisabledGroup, null, get_lang('ShowResultsToStudents') ); - - return $group; } $resultDisabledGroup[] = $form->createElement( diff --git a/main/exercise/exercise_admin.php b/main/exercise/exercise_admin.php index ecc8da5489..1bb8ad873e 100755 --- a/main/exercise/exercise_admin.php +++ b/main/exercise/exercise_admin.php @@ -112,22 +112,20 @@ $htmlHeadXtra[] = ''; -// to correct #4029 Random and number of attempt menu empty added window.onload=advanced_parameters; -$htmlHeadXtra[] = ''; $objExercise = new Exercise(); $course_id = api_get_course_int_id(); -//INIT FORM if (isset($_GET['exerciseId'])) { $form = new FormValidator( 'exercise_admin', @@ -147,7 +145,6 @@ if (isset($_GET['exerciseId'])) { $objExercise->createForm($form); -// VALIDATE FORM if ($form->validate()) { $objExercise->processCreation($form); if ($form->getSubmitValue('edit') === 'true') { diff --git a/main/inc/lib/pear/HTML/QuickForm/checkbox.php b/main/inc/lib/pear/HTML/QuickForm/checkbox.php index 0df072c5f2..792ee074f6 100755 --- a/main/inc/lib/pear/HTML/QuickForm/checkbox.php +++ b/main/inc/lib/pear/HTML/QuickForm/checkbox.php @@ -232,9 +232,9 @@ class HTML_QuickForm_checkbox extends HTML_QuickForm_input if ($this->getChecked()) { return '[x]'. $this->_getPersistantData(); - } else { - return '[ ]'; } + + return '[ ]'; } /** diff --git a/main/inc/lib/pear/HTML/QuickForm/element.php b/main/inc/lib/pear/HTML/QuickForm/element.php index 285e3b1d9b..fcd5391736 100755 --- a/main/inc/lib/pear/HTML/QuickForm/element.php +++ b/main/inc/lib/pear/HTML/QuickForm/element.php @@ -322,14 +322,12 @@ class HTML_QuickForm_element extends HTML_Common 'name' => $this->getName(), 'value' => $this->getValue() ) + (isset($id)? array('id' => $id): array())) . ' />'; - } /** * Returns whether or not the element is frozen * * @since 1.3 - * @access public * @return bool */ public function isFrozen() @@ -343,10 +341,8 @@ class HTML_QuickForm_element extends HTML_Common * * @param bool $persistant True if persistant value * @since 2.0 - * @access public - * @return void */ - function setPersistantFreeze($persistant=false) + public function setPersistantFreeze($persistant=false) { $this->_persistantFreeze = $persistant; } @@ -357,8 +353,6 @@ class HTML_QuickForm_element extends HTML_Common * @param string $label Display text for the element * @param string $label_for Optionally add a "for" attribute * @since 1.3 - * @access public - * @return void */ public function setLabel($label, $labelFor = null) { @@ -372,7 +366,6 @@ class HTML_QuickForm_element extends HTML_Common * Returns display text for the element * * @since 1.3 - * @access public * @return string */ public function getLabel() @@ -383,10 +376,9 @@ class HTML_QuickForm_element extends HTML_Common /** * Returns "for" attribute for the element * - * @access public * @return string */ - function getLabelFor() + public function getLabelFor() { return $this->_label_for; } diff --git a/main/inc/lib/pear/HTML/QuickForm/input.php b/main/inc/lib/pear/HTML/QuickForm/input.php index c4c1dd0fcc..a326d87176 100755 --- a/main/inc/lib/pear/HTML/QuickForm/input.php +++ b/main/inc/lib/pear/HTML/QuickForm/input.php @@ -109,9 +109,9 @@ class HTML_QuickForm_input extends HTML_QuickForm_element { if ($this->isFrozen()) { return $this->getFrozenHtml(); - } else { - return $this->_getTabs().'_getAttrString($this->_attributes).' />'; } + + return $this->_getTabs().'_getAttrString($this->_attributes).' />'; } /** diff --git a/main/inc/lib/pear/HTML/QuickForm/radio.php b/main/inc/lib/pear/HTML/QuickForm/radio.php index 1118083202..935255e4f4 100755 --- a/main/inc/lib/pear/HTML/QuickForm/radio.php +++ b/main/inc/lib/pear/HTML/QuickForm/radio.php @@ -33,12 +33,6 @@ */ class HTML_QuickForm_radio extends HTML_QuickForm_input { - /** - * Radio display text - * @var string - * @since 1.1 - * @access private - */ public $_text = ''; public $labelClass; public $radioClass; @@ -54,7 +48,6 @@ class HTML_QuickForm_radio extends HTML_QuickForm_input * * @return void * @since 1.0 - * @access public */ public function __construct( $elementName = null, @@ -92,7 +85,6 @@ class HTML_QuickForm_radio extends HTML_QuickForm_input * * @return string * @since 1.0 - * @access public */ public function toHtml() { @@ -120,7 +112,7 @@ class HTML_QuickForm_radio extends HTML_QuickForm_input return $label; } - return HTML_QuickForm_input::toHtml().$label; + return parent::toHtml().$label; } /** @@ -128,7 +120,6 @@ class HTML_QuickForm_radio extends HTML_QuickForm_input * * @return string * @since 1.0 - * @access public */ public function getChecked() { @@ -140,16 +131,15 @@ class HTML_QuickForm_radio extends HTML_QuickForm_input * * @return string * @since 1.0 - * @access public */ public function getFrozenHtml() { if ($this->getChecked()) { - return '(x)'. + return '
(x)'. $this->_getPersistantData(); - } else { - return '( )'; } + + return '
( )'; } /**