From 54bba9b12b3c69cd9e7438d34e0c1e935b7b8e60 Mon Sep 17 00:00:00 2001 From: Hubert Borderiou Date: Tue, 7 Jan 2014 10:22:54 +0100 Subject: [PATCH 1/9] In question authoring, display Comment editor for all test configuration - ref #6619 --- main/exercice/admin.php | 7 +++++ .../exercice/global_multiple_answer.class.php | 17 +++--------- main/exercice/multiple_answer.class.php | 17 +++--------- .../multiple_answer_combination.class.php | 16 +++-------- main/exercice/unique_answer.class.php | 21 +++++++-------- .../unique_answer_no_option.class.php | 27 +++++-------------- 6 files changed, 35 insertions(+), 70 deletions(-) diff --git a/main/exercice/admin.php b/main/exercice/admin.php index 996157fb7f..5af8cfa82c 100644 --- a/main/exercice/admin.php +++ b/main/exercice/admin.php @@ -513,6 +513,13 @@ if (!$newQuestion && !$modifyQuestion && !$editQuestion && !isset($_GET['hotspot require 'question_list_admin.inc.php'; } +// if we are in question authoring, display warning to user is feedback not shown at the end of the test -ref #6619 +// this test to displau only message in the question authoring page and not in the question list page too +// if (is_object($objQuestion) && $objExercise->selectFeedbackType() == EXERCISE_FEEDBACK_TYPE_EXAM && ($newQuestion || $modifyQuestion || $editQuestion)) { +if ($objExercise->selectFeedbackType() == EXERCISE_FEEDBACK_TYPE_EXAM) { + Display::display_normal_message(get_lang("TestFeedbackNotShown")); +} + Session::write('objExercise', $objExercise); Session::write('objQuestion', $objQuestion); Session::write('objAnswer', $objAnswer); diff --git a/main/exercice/global_multiple_answer.class.php b/main/exercice/global_multiple_answer.class.php index d4b97b7251..1083ca62ba 100644 --- a/main/exercice/global_multiple_answer.class.php +++ b/main/exercice/global_multiple_answer.class.php @@ -40,10 +40,8 @@ if (!class_exists('GlobalMultipleAnswer')): ' . get_lang('Answer') . ' '; - // Espace entre l'entete et les r�ponses - if ($obj_ex->selectFeedbackType() != EXERCISE_FEEDBACK_TYPE_EXAM) { - $html .='' . get_lang('Comment') . ''; - } + // Espace entre l'entete et les reponses + $html .='' . get_lang('Comment') . ''; $html .=''; @@ -116,10 +114,7 @@ if (!class_exists('GlobalMultipleAnswer')): $form->addElement('html_editor', 'answer[' . $i . ']', null, 'style="vertical-align:middle"', array('ToolbarSet' => 'TestProposedAnswer', 'Width' => '100%', 'Height' => '100')); $form->addRule('answer[' . $i . ']', get_lang('ThisFieldIsRequired'), 'required'); - // show comment when feedback is enable - if ($obj_ex->selectFeedbackType() != EXERCISE_FEEDBACK_TYPE_EXAM) { - $form->addElement('html_editor', 'comment[' . $i . ']', null, 'style="vertical-align:middle"', array('ToolbarSet' => 'TestProposedAnswer', 'Width' => '100%', 'Height' => '100')); - } + $form->addElement('html_editor', 'comment[' . $i . ']', null, 'style="vertical-align:middle"', array('ToolbarSet' => 'TestProposedAnswer', 'Width' => '100%', 'Height' => '100')); $form->addElement('html', ''); } @@ -243,11 +238,7 @@ if (!class_exists('GlobalMultipleAnswer')): ' . get_lang("Choice") . ' ' . get_lang("ExpectedChoice") . ' ' . get_lang("Answer") . ''; - if ($feedback_type != EXERCISE_FEEDBACK_TYPE_EXAM) { - $header .= '' . get_lang("Comment") . ''; - } else { - $header .= ' '; - } + $header .= '' . get_lang("Comment") . ''; $header .= ''; return $header; } diff --git a/main/exercice/multiple_answer.class.php b/main/exercice/multiple_answer.class.php index 1f6059f5b8..12bb91cc14 100644 --- a/main/exercice/multiple_answer.class.php +++ b/main/exercice/multiple_answer.class.php @@ -58,11 +58,9 @@ class MultipleAnswer extends Question { '.get_lang('Answer').' '; // show column comment when feedback is enable - if ($obj_ex->selectFeedbackType() != EXERCISE_FEEDBACK_TYPE_EXAM ) { - $html .=' - '.get_lang('Comment').' - '; - } + $html .=' + '.get_lang('Comment').' + '; $html .= ' '.get_lang('Weighting').' @@ -121,10 +119,7 @@ class MultipleAnswer extends Question { $form->addElement('html_editor', 'answer['.$i.']',null, 'style="vertical-align:middle"', array('ToolbarSet' => 'TestProposedAnswer', 'Width' => '100%', 'Height' => '100')); $form->addRule('answer['.$i.']', get_lang('ThisFieldIsRequired'), 'required'); - // show comment when feedback is enable - if ($obj_ex->selectFeedbackType() != EXERCISE_FEEDBACK_TYPE_EXAM) { $form->addElement('html_editor', 'comment['.$i.']',null, 'style="vertical-align:middle"', array('ToolbarSet' => 'TestProposedAnswer', 'Width' => '100%', 'Height' => '100')); - } $form->addElement('text', 'weighting['.$i.']',null, array('class' => "span1", 'value' => '0')); $form -> addElement ('html', ''); @@ -213,11 +208,7 @@ class MultipleAnswer extends Question { '.get_lang("Choice").' '. get_lang("ExpectedChoice").' '. get_lang("Answer").''; - if ($feedback_type != EXERCISE_FEEDBACK_TYPE_EXAM) { - $header .= ''.get_lang("Comment").''; - } else { - $header .= ' '; - } + $header .= ''.get_lang("Comment").''; $header .= ''; return $header; } diff --git a/main/exercice/multiple_answer_combination.class.php b/main/exercice/multiple_answer_combination.class.php index 61455021da..e631e7f4ab 100644 --- a/main/exercice/multiple_answer_combination.class.php +++ b/main/exercice/multiple_answer_combination.class.php @@ -56,9 +56,7 @@ class MultipleAnswerCombination extends Question { '.get_lang('Answer').' '; // show column comment when feedback is enable - if ($obj_ex->selectFeedbackType() != EXERCISE_FEEDBACK_TYPE_EXAM ) { - $html .=''.get_lang('Comment').''; - } + $html .=''.get_lang('Comment').''; $html .= ''; $form -> addElement ('label', get_lang('Answers').'
', $html); @@ -113,10 +111,8 @@ class MultipleAnswerCombination extends Question { $form->addElement('html_editor', 'answer['.$i.']',null, 'style="vertical-align:middle"', array('ToolbarSet' => 'TestProposedAnswer', 'Width' => '100%', 'Height' => '100')); $form->addRule('answer['.$i.']', get_lang('ThisFieldIsRequired'), 'required'); - if ($obj_ex->selectFeedbackType() != EXERCISE_FEEDBACK_TYPE_EXAM) { - $form->addElement('html_editor', 'comment['.$i.']',null, 'style="vertical-align:middle"', array('ToolbarSet' => 'TestProposedAnswer', 'Width' => '100%', 'Height' => '100')); - } - //only 1 answer the all deal ... + $form->addElement('html_editor', 'comment['.$i.']',null, 'style="vertical-align:middle"', array('ToolbarSet' => 'TestProposedAnswer', 'Width' => '100%', 'Height' => '100')); + //only 1 answer the all deal ... //$form->addElement('text', 'weighting['.$i.']',null, 'style="vertical-align:middle;margin-left: 0em;" size="5" value="10"'); $form -> addElement ('html', ''); @@ -216,11 +212,7 @@ class MultipleAnswerCombination extends Question { '.get_lang("Choice").' '. get_lang("ExpectedChoice").' '. get_lang("Answer").''; - if ($feedback_type != EXERCISE_FEEDBACK_TYPE_EXAM) { - $header .= ''.get_lang("Comment").''; - } else { - $header .= ' '; - } + $header .= ''.get_lang("Comment").''; $header .= ''; return $header; } diff --git a/main/exercice/unique_answer.class.php b/main/exercice/unique_answer.class.php index 5fafc74c4f..b6050b0fc1 100644 --- a/main/exercice/unique_answer.class.php +++ b/main/exercice/unique_answer.class.php @@ -63,15 +63,16 @@ class UniqueAnswer extends Question { $feedback_title=''; $comment_title=''; - if ($obj_ex->selectFeedbackType() == EXERCISE_FEEDBACK_TYPE_END) { - $comment_title = ''.get_lang('Comment').''; - } elseif ($obj_ex->selectFeedbackType() == EXERCISE_FEEDBACK_TYPE_DIRECT) { + if ($obj_ex->selectFeedbackType() == EXERCISE_FEEDBACK_TYPE_DIRECT) { //Scenario $editor_config['Width'] = '250'; $editor_config['Height'] = '110'; $comment_title = ''.get_lang('Comment').''; $feedback_title = ''.get_lang('Scenario').''; } + else { + $comment_title = ''.get_lang('Comment').''; + } $html=' @@ -197,10 +198,7 @@ class UniqueAnswer extends Question { $form->addRule('answer['.$i.']', get_lang('ThisFieldIsRequired'), 'required'); - if ($obj_ex->selectFeedbackType() == EXERCISE_FEEDBACK_TYPE_END) { - // feedback - $form->addElement('html_editor', 'comment['.$i.']', null, 'style="vertical-align:middle"', $editor_config); - } elseif ($obj_ex->selectFeedbackType() == EXERCISE_FEEDBACK_TYPE_DIRECT) { + if ($obj_ex->selectFeedbackType() == EXERCISE_FEEDBACK_TYPE_DIRECT) { $form->addElement('html_editor', 'comment['.$i.']', null, 'style="vertical-align:middle"', $editor_config); // Direct feedback @@ -215,6 +213,9 @@ class UniqueAnswer extends Question { $renderer->setElementTemplate(''); } @@ -354,11 +355,7 @@ class UniqueAnswer extends Question { '; - if ($feedback_type != EXERCISE_FEEDBACK_TYPE_EXAM) { - $header .= ''; - } else { - $header .= ''; - } + $header .= ''; $header .= ''; return $header; } diff --git a/main/exercice/unique_answer_no_option.class.php b/main/exercice/unique_answer_no_option.class.php index f2cd81cfd7..5ca94fca57 100644 --- a/main/exercice/unique_answer_no_option.class.php +++ b/main/exercice/unique_answer_no_option.class.php @@ -64,14 +64,15 @@ class UniqueAnswerNoOption extends Question { $feedback_title=''; $comment_title=''; - if ($obj_ex->selectFeedbackType()==0) { - $comment_title = ''; - } elseif ($obj_ex->selectFeedbackType()==1) { + if ($obj_ex->selectFeedbackType()==1) { $editor_config['Width'] = '250'; $editor_config['Height'] = '110'; $comment_title = ''; $feedback_title = ''; } + else { + $comment_title = ''; + } $html='
{error}
{element}', 'scenario'); } + else { + $form->addElement('html_editor', 'comment['.$i.']', null, 'style="vertical-align:middle"', $editor_config); + } $form->addElement('text', 'weighting['.$i.']', null, array('class' => "span1", 'value' => '0')); $form->addElement ('html', '
'.get_lang("Choice").' '. get_lang("ExpectedChoice").' '. get_lang("Answer").''.get_lang("Comment").' '.get_lang("Comment").'
'.get_lang('Comment').''.get_lang('Comment').''.get_lang('Scenario').''.get_lang('Comment').'
@@ -196,12 +197,7 @@ class UniqueAnswerNoOption extends Question { $form->addElement('radio', 'correct', null, null, $i, 'class="checkbox" style="margin-left: 0em;"'); $form->addElement('html_editor', 'answer['.$i.']', null, 'style="vertical-align:middle"', $editor_config); - if ($obj_ex->selectFeedbackType() == EXERCISE_FEEDBACK_TYPE_END) { - // feedback - $form->addElement('html_editor', 'comment['.$i.']', null, 'style="vertical-align:middle"', $editor_config); - } elseif ($obj_ex->selectFeedbackType() == EXERCISE_FEEDBACK_TYPE_DIRECT) { - - } + $form->addElement('html_editor', 'comment['.$i.']', null, 'style="vertical-align:middle"', $editor_config); $form->addElement('text', 'weighting['.$i.']', null, array('class' => "span1", 'value' => '0')); $form->addElement('html', ''); $i++; @@ -238,12 +234,7 @@ class UniqueAnswerNoOption extends Question { $form->addRule('answer['.$i.']', get_lang('ThisFieldIsRequired'), 'required'); - if ($obj_ex->selectFeedbackType() == EXERCISE_FEEDBACK_TYPE_END) { - // feedback - $form->addElement('html_editor', 'comment['.$i.']', null, 'style="vertical-align:middle"', $editor_config); - } elseif ($obj_ex->selectFeedbackType() == EXERCISE_FEEDBACK_TYPE_DIRECT) { - - } + $form->addElement('html_editor', 'comment['.$i.']', null, 'style="vertical-align:middle"', $editor_config); //$form->addElement('select', 'destination'.$i, get_lang('SelectQuestion').' : ',$select_question,'multiple'); @@ -406,11 +397,7 @@ class UniqueAnswerNoOption extends Question { '; - if ($feedback_type != EXERCISE_FEEDBACK_TYPE_EXAM) { - $header .= ''; - } else { - $header .= ''; - } + $header .= ''; $header .= ''; return $header; } From 79ca9bc15a7eae7dc9acfab4f582a6d795535b99 Mon Sep 17 00:00:00 2001 From: Hubert Borderiou Date: Tue, 7 Jan 2014 13:36:59 +0100 Subject: [PATCH 2/9] Review Fill the blank test allowed char - ref #6810 --- main/exercice/exercise.class.php | 10 +-- main/exercice/exercise.lib.php | 118 +++++++++++++++++++------------ 2 files changed, 78 insertions(+), 50 deletions(-) diff --git a/main/exercice/exercise.class.php b/main/exercice/exercise.class.php index cd8198eab6..0dbf79072d 100644 --- a/main/exercice/exercise.class.php +++ b/main/exercice/exercise.class.php @@ -1169,7 +1169,7 @@ class Exercise { $defaults['randomAnswers'] = $this->selectRandomAnswers(); $defaults['exerciseType'] = $this->selectType(); - $defaults['exerciseTitle'] = api_html_entity_decode($this->selectTitle()); + $defaults['exerciseTitle'] = $this->selectTitle(); $defaults['exerciseDescription'] = $this->selectDescription(); $defaults['exerciseAttempts'] = $this->selectAttempts(); $defaults['exerciseFeedbackType'] = $this->selectFeedbackType(); @@ -1212,7 +1212,7 @@ class Exercise { $defaults['pass_percentage'] = ''; } } else { - $defaults['exerciseTitle'] = api_html_entity_decode($this->selectTitle()); + $defaults['exerciseTitle'] = $this->selectTitle(); $defaults['exerciseDescription'] = $this->selectDescription(); } if (api_get_setting('search_enabled') === 'true') { @@ -1250,7 +1250,7 @@ class Exercise { */ function processCreation($form, $type = '') { - $this->updateTitle(api_htmlentities($form->getSubmitValue('exerciseTitle'))); + $this->updateTitle(htmlentities($form->getSubmitValue('exerciseTitle'))); $this->updateDescription($form->getSubmitValue('exerciseDescription')); $this->updateAttempts($form->getSubmitValue('exerciseAttempts')); $this->updateFeedbackType($form->getSubmitValue('exerciseFeedbackType')); @@ -2367,7 +2367,7 @@ class Exercise { $answer .= '' . $user_tags[$i] . ''; } else { // adds a tabulation if no word has been typed by the student - $answer .= '   '; + $answer .= ''; // remove   that causes issue } } else { // switchable fill in the blanks @@ -2388,7 +2388,7 @@ class Exercise { $answer .= '' . $user_tags[$i] . ''; } else { // adds a tabulation if no word has been typed by the student - $answer .= '   '; + $answer .= ''; // remove   that causes issue } } // adds the correct word, followed by ] to close the blank diff --git a/main/exercice/exercise.lib.php b/main/exercice/exercise.lib.php index 8152ce3a9e..03546f3424 100644 --- a/main/exercice/exercise.lib.php +++ b/main/exercice/exercise.lib.php @@ -403,65 +403,93 @@ function showQuestion($questionId, $only_questions = false, $origin = false, $cu $s.=''; } elseif ($answerType == FILL_IN_BLANKS) { + /* + * In the FILL_IN_BLANKS test + * you mustn't have [ and ] in the textarea + * you mustn't have :: in the textarea + * the text to find mustn't be empty or contains only spaces + * the text to find mustn't contains HTML tags + * the text to find mustn't contains char " + */ + list($answer) = explode('::', $answer); - //Correct answer + // $correct_answer_list array of array with correct anwsers 0=> [0=>[\p] 1=>[plop]] api_preg_match_all('/\[[^]]+\]/', $answer, $correct_answer_list); - //Student's answezr + // get student answer to display it if student go back to previous fillBlank answer question in a test if (isset($user_choice[0]['answer'])) { api_preg_match_all('/\[[^]]+\]/', $user_choice[0]['answer'], $student_answer_list); - $student_answer_list = $student_answer_list[0]; + $student_answer_list_tobecleaned = $student_answer_list[0]; + $student_answer_list = array(); + // here we got the student answer in a test + // let's clean up the results + /* + Array + ( + [0] => Array + ( + [0] => [yer / ici] + [1] => [plop / /p] + ) + ) + */ + for ($i=0; $i < count($student_answer_list_tobecleaned); $i++) { + $answer_corrected = $student_answer_list_tobecleaned[$i]; + /* + * we got if student answer is wrong + * [rrr / /p] + * or if student answer is good + * [plop / plop] + * or if student didn't answer [] + */ + $answer_corrected = api_preg_replace('| / .*$|', '', $answer_corrected); + /* + * we got [rrr or [plop or [ + */ + $answer_corrected = api_preg_replace('/^\[/', '', $answer_corrected); + /* + * we got rrr or plop + * non breakable spaces     from /main/exercice/exercise.class.php have been removed l 2391 and l 2370 + */ + $answer_corrected = api_preg_replace('|^|', '', $answer_corrected); + $answer_corrected = api_preg_replace('|$|', '', $answer_corrected); + $answer_corrected = '['.$answer_corrected.']'; + /* + * we got [rrr] or [plop] or [] + */ + $student_answer_list[] = $answer_corrected; + } } - //If debug + // If display preview of answer in test view for exemple, set the student answer to the correct answers if ($debug_mark_answer) { + // contain the rights answers surronded with brackets $student_answer_list = $correct_answer_list[0]; } - if (!empty($correct_answer_list) && !empty($student_answer_list)) { - $correct_answer_list = $correct_answer_list[0]; - $i = 0; - - foreach ($correct_answer_list as $correct_item) { - $value = null; - if (isset($student_answer_list[$i]) && !empty($student_answer_list[$i])) { - - //Cleaning student answer list - $value = strip_tags($student_answer_list[$i]); - $value = api_substr($value, 1, api_strlen($value)-2); - $value = explode('/', $value); - - if (!empty($value[0])) { - $value = str_replace(' ', '', trim($value[0])); - } - //var_dump($correct_item); - //$correct_item = preg_quote($correct_item); - // to prevent error if there is a / in the text to find - //$correct_item = api_preg_replace('|/|', '\/', $correct_item); - - $size = strlen($correct_item); - $attributes['class'] = detectInputAppropriateClass($size); - - //$answer = api_preg_replace('/'.$correct_item.'/', Display::input('text', "choice[$questionId][]", $value, $attributes), $answer, 1); - $answer = str_replace($correct_item, Display::input('text', "choice[$questionId][]", $value, $attributes), $answer); - } - $i++; - } - } else { - - foreach ($correct_answer_list[0] as $item) { - $size = strlen($item); - $attributes['class'] = detectInputAppropriateClass($size); - - //$pattern = '/\['.$item.'+\]/'; - //$answer = api_preg_replace($pattern, Display::input('text', "choice[$questionId][]", '', $attributes), $answer); - $answer = str_replace($item, Display::input('text', "choice[$questionId][]", '', $attributes), $answer); + // Split the response by bracket + // tab_comments is an array with text surrounding the text to find + // we add a space before and after the answer_question to be sure to have a block of text before and after [xxx] patterns + // so we have n text to find ([xxx]) and n+1 blockc of texts before, beteween and after the text to find + $tab_comments = api_preg_split('/\[[^]]+\]/', ' '.$answer.' '); + + if (!empty($correct_answer_list) && !empty($student_answer_list)) { + $answer = ""; + $i = 0; + foreach ($student_answer_list as $student_item) { + $student_response = api_substr($student_item, 1, api_strlen($student_item) - 2); // remove surronding brackets + $answer .= $tab_comments[$i].Display::input('text', "choice[$questionId][]", $student_response); + $i++; } - //$answer = api_preg_replace('/\[[^]]+\]/', Display::input('text', "choice[$questionId][]", '', $attributes), $answer); - } - + $answer .= $tab_comments[$i]; + } else { + // display exercice with empty input fields + // every [xxx] are replaced with an empty input field + $answer = api_preg_replace('/\[[^]]+\]/', Display::input('text', "choice[$questionId][]", '', $attributes), $answer); + } $s .= $answer; + } elseif ($answerType == MATCHING) { // matching type, showing suggestions and answers // TODO: replace $answerId by $numAnswer From edb349cae55a812a912fa74e525405cce8546677 Mon Sep 17 00:00:00 2001 From: Hubert Borderiou Date: Tue, 7 Jan 2014 14:02:43 +0100 Subject: [PATCH 3/9] Display hidden tests in Recycle question menu - ref #6819 --- main/exercice/exercise.lib.php | 15 ++++++++++----- main/exercice/question_pool.php | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/main/exercice/exercise.lib.php b/main/exercice/exercise.lib.php index 03546f3424..e336638ebf 100644 --- a/main/exercice/exercise.lib.php +++ b/main/exercice/exercise.lib.php @@ -1508,23 +1508,28 @@ function get_exercise_by_id($exerciseId = 0) { return Database::select('*', $TBL_EXERCICES, $conditions); } /** - * Getting all active exercises from a course from a session (if a session_id is provided we will show all the exercises in the course + all exercises in the session) + * Getting all exercises (active only or all) from a course from a session (if a session_id is provided we will show all the exercises in the course + all exercises in the session) * @param array course data * @param int session id - * @param int course c_id + * @param int course c_id + * @param boolean only_active_exercices * @return array array with exercise data * modified by Hubert Borderiou */ -function get_all_exercises_for_course_id($course_info = null, $session_id = 0, $course_id=0) { +function get_all_exercises_for_course_id($course_info = null, $session_id = 0, $course_id=0, $only_active_exercices = true) { + $sql_active_exercices = ""; + if (!$only_active_exercices) { + $sql_active_exercices = " OR active != 1 "; + } $TBL_EXERCICES = Database :: get_course_table(TABLE_QUIZ_TEST); if ($session_id == -1) { $session_id = 0; } if ($session_id == 0) { - $conditions = array('where'=>array('active = ? AND session_id = ? AND c_id = ?'=>array('1', $session_id, $course_id)), 'order'=>'title'); + $conditions = array('where'=>array("(active = ? $sql_active_exercices) AND session_id = ? AND c_id = ?" => array('1', $session_id, $course_id)), 'order'=>'title'); } else { //All exercises - $conditions = array('where'=>array('active = ? AND (session_id = 0 OR session_id = ? ) AND c_id=?' =>array('1', $session_id, $course_id)), 'order'=>'title'); + $conditions = array('where'=>array("(active = ? $sql_active_exercices) AND (session_id = 0 OR session_id = ? ) AND c_id=?" => array('1', $session_id, $course_id)), 'order'=>'title'); } return Database::select('*',$TBL_EXERCICES, $conditions); } diff --git a/main/exercice/question_pool.php b/main/exercice/question_pool.php index 002b250a81..e8736752c0 100644 --- a/main/exercice/question_pool.php +++ b/main/exercice/question_pool.php @@ -322,7 +322,7 @@ echo Display::form_row(get_lang("QuestionCategory"), $selectCourseCateogry); // Get exercice list for this course -$exercise_list = get_all_exercises_for_course_id($course_info, $session_id, $selected_course); +$exercise_list = get_all_exercises_for_course_id($course_info, $session_id, $selected_course, false); //Exercise List $my_exercise_list = array(); $my_exercise_list['0'] = get_lang('AllExercises'); From adbd1a60b5caa9449b54768ca21f4655c8ad949c Mon Sep 17 00:00:00 2001 From: Hubert Borderiou Date: Wed, 8 Jan 2014 10:05:28 +0100 Subject: [PATCH 4/9] Coding convention and replace some api_get_course_id() with api_get_course_int_id() --- index.php | 116 +++++++++++++++++----------------- main/auth/cas/logincas.php | 11 ++-- main/auth/gotocourse.php | 5 +- main/inc/lib/main_api.lib.php | 94 +++++++++++---------------- main/inc/local.inc.php | 5 +- 5 files changed, 105 insertions(+), 126 deletions(-) diff --git a/index.php b/index.php index 007411c9cb..3c6601230b 100644 --- a/index.php +++ b/index.php @@ -25,21 +25,21 @@ $this_section = SECTION_CAMPUS; $header_title = null; if (!api_is_anonymous()) { - $header_title = " "; + $header_title = " "; } $htmlHeadXtra[] = api_get_jquery_libraries_js(array('bxslider')); $htmlHeadXtra[] =' '; //set cookie for check if client browser are cookies enabled @@ -51,7 +51,7 @@ $controller = new IndexManager($header_title); $loginFailed = isset($_GET['loginFailed']) ? true : isset($loginFailed); if (!empty($_GET['logout'])) { - $controller->logout(); + $controller->logout(); } /* Table definitions */ @@ -70,16 +70,16 @@ $_setting['display_courses_to_anonymous_users'] = 'true'; * @todo This piece of code should probably move to local.inc.php where the actual login / logout procedure is handled. The real use of this code block should be seriously considered as well. This form should just use a security token and get done with it. */ if (isset($_GET['submitAuth']) && $_GET['submitAuth'] == 1) { - $i = api_get_anonymous_id(); - event_system(LOG_ATTEMPTED_FORCED_LOGIN, 'tried_hacking_get', $_SERVER['REMOTE_ADDR'].(empty($_POST['login'])?'':'/'.$_POST['login']),null,$i); - echo 'Attempted breakin - sysadmins notified.'; - session_destroy(); - die(); + $i = api_get_anonymous_id(); + event_system(LOG_ATTEMPTED_FORCED_LOGIN, 'tried_hacking_get', $_SERVER['REMOTE_ADDR'].(empty($_POST['login'])?'':'/'.$_POST['login']),null,$i); + echo 'Attempted breakin - sysadmins notified.'; + session_destroy(); + die(); } // Delete session neccesary for legal terms if (api_get_setting('allow_terms_conditions') == 'true') { - unset($_SESSION['term_and_condition']); + unset($_SESSION['term_and_condition']); } //If we are not logged in and customapages activated if (!api_get_user_id() && CustomPages::enabled()) { @@ -98,38 +98,39 @@ if (!api_get_user_id() && CustomPages::enabled()) { */ if (!empty($_POST['submitAuth'])) { - // The user has been already authenticated, we are now to find the last login of the user. - if (isset ($_user['user_id'])) { - $track_login_table = Database :: get_statistic_table(TABLE_STATISTIC_TRACK_E_LOGIN); - $sql_last_login = "SELECT UNIX_TIMESTAMP(login_date) + // The user has been already authenticated, we are now to find the last login of the user. + if (isset ($_user['user_id'])) { + $track_login_table = Database :: get_statistic_table(TABLE_STATISTIC_TRACK_E_LOGIN); + $sql_last_login = "SELECT UNIX_TIMESTAMP(login_date) FROM $track_login_table WHERE login_user_id = '".$_user['user_id']."' ORDER BY login_date DESC LIMIT 1"; - $result_last_login = Database::query($sql_last_login); - if (!$result_last_login) { - if (Database::num_rows($result_last_login) > 0) { - $user_last_login_datetime = Database::fetch_array($result_last_login); - $user_last_login_datetime = $user_last_login_datetime[0]; - Session::write('user_last_login_datetime',$user_last_login_datetime); - } - } - Database::free_result($result_last_login); - - //event_login(); - if (api_is_platform_admin()) { - // decode all open event informations and fill the track_c_* tables - include api_get_path(LIBRARY_PATH).'stats.lib.inc.php'; - decodeOpenInfos(); - } - } - // End login -- if ($_POST['submitAuth']) -} else { - // Only if login form was not sent because if the form is sent the user was already on the page. - event_open(); + $result_last_login = Database::query($sql_last_login); + if (!$result_last_login) { + if (Database::num_rows($result_last_login) > 0) { + $user_last_login_datetime = Database::fetch_array($result_last_login); + $user_last_login_datetime = $user_last_login_datetime[0]; + Session::write('user_last_login_datetime',$user_last_login_datetime); + } + } + Database::free_result($result_last_login); + + //event_login(); + if (api_is_platform_admin()) { + // decode all open event informations and fill the track_c_* tables + include api_get_path(LIBRARY_PATH).'stats.lib.inc.php'; + decodeOpenInfos(); + } + } + // End login -- if ($_POST['submitAuth']) +} +else { + // Only if login form was not sent because if the form is sent the user was already on the page. + event_open(); } if (api_get_setting('display_categories_on_homepage') == 'true') { - $controller->tpl->assign('course_category_block', $controller->return_courses_in_categories()); + $controller->tpl->assign('course_category_block', $controller->return_courses_in_categories()); } // Facebook connexion, if activated @@ -141,14 +142,15 @@ $controller->set_login_form(); //@todo move this inside the IndexManager if (!api_is_anonymous()) { - $controller->tpl->assign('profile_block', $controller->return_profile_block()); + $controller->tpl->assign('profile_block', $controller->return_profile_block()); $controller->tpl->assign('user_image_block', $controller->return_user_image_block()); - if (api_is_platform_admin()) { - $controller->tpl->assign('course_block', $controller->return_course_block()); - } else { - $controller->tpl->assign('teacher_block', $controller->return_teacher_link()); - } + if (api_is_platform_admin()) { + $controller->tpl->assign('course_block', $controller->return_course_block()); + } + else { + $controller->tpl->assign('teacher_block', $controller->return_teacher_link()); + } } $hot_courses = null; @@ -163,18 +165,18 @@ if (!isset($_REQUEST['include'])) { $announcements_block = $controller->return_announcements(); } -$controller->tpl->assign('hot_courses', $hot_courses); -$controller->tpl->assign('announcements_block', $announcements_block); -$controller->tpl->assign('home_page_block', $controller->return_home_page()); +$controller->tpl->assign('hot_courses', $hot_courses); +$controller->tpl->assign('announcements_block', $announcements_block); +$controller->tpl->assign('home_page_block', $controller->return_home_page()); -$controller->tpl->assign('navigation_course_links', $controller->return_navigation_links()); +$controller->tpl->assign('navigation_course_links', $controller->return_navigation_links()); -$controller->tpl->assign('notice_block', $controller->return_notice()); -$controller->tpl->assign('main_navigation_block', $controller->return_navigation_links()); -$controller->tpl->assign('help_block', $controller->return_help()); +$controller->tpl->assign('notice_block', $controller->return_notice()); +$controller->tpl->assign('main_navigation_block', $controller->return_navigation_links()); +$controller->tpl->assign('help_block', $controller->return_help()); if (api_is_platform_admin() || api_is_drh()) { - $controller->tpl->assign('skills_block', $controller->return_skills_links()); + $controller->tpl->assign('skills_block', $controller->return_skills_links()); } // direct login to course diff --git a/main/auth/cas/logincas.php b/main/auth/cas/logincas.php index 0bc7c331c1..af8cee4841 100644 --- a/main/auth/cas/logincas.php +++ b/main/auth/cas/logincas.php @@ -21,10 +21,10 @@ global $cas_auth_ver, $cas_auth_server, $cas_auth_port, $cas_auth_uri; /* If we are not logged and in our browser enter an URL with a name of a course e.g. http://www.chamilo.fr/chamilo/courses/COURSTESTOSETE/?id_session=0 -we go to page api_not_allowed : -> Vous n'etes pas autorise e acceder e cette page. -> Soit votre connexion a expire, soit vous essayez d'acceder e une page pour laquelle vous ne disposez pas des permissions suffisantes. -> Veuillez vous identifier e nouveau depuis la page d'accueil +We go to page api_not_allowed : +> You are not allowed to see this page. +> Sorry, you are not allowed to access this page, or maybe your connection has expired. +> Please click your browser's \"Back\" button or follow the link below to return to the previous page If we click on the link to go to homepage, some datas are entered in $_SESSION and if we enter our CAS loggin, we go to api_not_allowad_page again and again As a result, if we are not logged on, we have to destroy the session variables, before calling CAS page @@ -45,7 +45,6 @@ if (cas_configured()) { } phpCAS::forceAuthentication(); header('Location: '.api_get_path(WEB_PATH).api_get_setting('page_after_login')); -} -else { +} else { header('Location: '.api_get_path(WEB_PATH)); } diff --git a/main/auth/gotocourse.php b/main/auth/gotocourse.php index 1eda5b52bd..f9b854c291 100644 --- a/main/auth/gotocourse.php +++ b/main/auth/gotocourse.php @@ -1,7 +1,7 @@ assign('content', '

'.get_lang('LoginToGoToThisCourse').'

'.$msg); $tpl->display_one_col_template(); -} -else { +} else { api_delete_firstpage_parameter(); Header('Location: '.api_get_path(WEB_PATH).'index.php'); } diff --git a/main/inc/lib/main_api.lib.php b/main/inc/lib/main_api.lib.php index 952bdfd8a8..48ef886da2 100644 --- a/main/inc/lib/main_api.lib.php +++ b/main/inc/lib/main_api.lib.php @@ -2406,15 +2406,11 @@ function api_is_course_session_coach($user_id, $course_code, $session_id) { /** * Checks whether the current user is a course or session coach - * @param int $session_id optional, session id - * @param string $course_code optional, course code - * @param int $userId The user ID + * @param int - optional, session id + * @param string - optional, course code * @return boolean True if current user is a course or session coach */ -function api_is_coach($session_id = 0, $course_code = null, $userId = null) { - if (empty($userId)) { - $userId = api_get_user_id(); - } +function api_is_coach($session_id = 0, $course_code = null) { if (!empty($session_id)) { $session_id = intval($session_id); } else { @@ -2438,7 +2434,7 @@ function api_is_coach($session_id = 0, $course_code = null, $userId = null) { if (!empty($course_code)) { $sql = "SELECT DISTINCT id, name, date_start, date_end FROM $session_table INNER JOIN $session_rel_course_rel_user_table session_rc_ru - ON session_rc_ru.id_user = '".$userId."' + ON session_rc_ru.id_user = '".api_get_user_id()."' WHERE session_rc_ru.course_code = '$course_code' AND session_rc_ru.status = 2 AND session_rc_ru.id_session = '$session_id'"; @@ -2449,7 +2445,7 @@ function api_is_coach($session_id = 0, $course_code = null, $userId = null) { if (!empty($session_id)) { $sql = "SELECT DISTINCT id, name, date_start, date_end FROM $session_table - WHERE session.id_coach = '".$userId."' AND id = '$session_id' + WHERE session.id_coach = '".api_get_user_id()."' AND id = '$session_id' ORDER BY date_start, date_end, name"; $result = Database::query($sql); if (!empty($sessionIsCoach)) { @@ -2463,57 +2459,37 @@ function api_is_coach($session_id = 0, $course_code = null, $userId = null) { /** * Checks whether the current user is a session administrator - * @param int $userId The user ID * @return boolean True if current user is a course administrator */ -function api_is_session_admin($userId = null) { - if (!empty($userId)) { - $_user = api_get_user_info($userId); - } else { - global $_user; - } +function api_is_session_admin() { + global $_user; return isset($_user['status']) && $_user['status'] == SESSIONADMIN; } /** * Checks whether the current user is a human resources manager - * @param int $userId The user ID * @return boolean True if current user is a human resources manager */ -function api_is_drh($userId = null) { - if (!empty($userId)) { - $_user = api_get_user_info($userId); - } else { - global $_user; - } +function api_is_drh() { + global $_user; return isset($_user['status']) && $_user['status'] == DRH; } /** * Checks whether the current user is a student - * @param int $userId The user ID * @return boolean True if current user is a human resources manager */ -function api_is_student($userId = null) { - if (!empty($userId)) { - $_user = api_get_user_info($userId); - } else { - global $_user; - } +function api_is_student() { + global $_user; return isset($_user['status']) && $_user['status'] == STUDENT; } /** * Checks whether the current user is a teacher - * @param int $userId The user ID * @return boolean True if current user is a human resources manager */ -function api_is_teacher($userId = null) { - if (!empty($userId)) { - $_user = api_get_user_info($userId); - } else { - global $_user; - } +function api_is_teacher() { + global $_user; return isset($_user['status']) && $_user['status'] == COURSEMANAGER; } @@ -2953,7 +2929,8 @@ function api_not_found($print_headers = false) { * @version 1.0, February 2004 * @version dokeos 1.8, August 2006 */ -function api_not_allowed($print_headers = false, $message = null) { +function api_not_allowed($print_headers = false, $message = null) +{ if (api_get_setting('sso_authentication') === 'true') { global $osso; if ($osso) { @@ -3004,7 +2981,7 @@ function api_not_allowed($print_headers = false, $message = null) { exit; } - if (!empty($_SERVER['REQUEST_URI']) && (!empty($_GET['cidReq']) || $this_section == SECTION_MYPROFILE)) { + if (!empty($_SERVER['REQUEST_URI']) && (!empty($_GET['cidReq']) || $this_section == SECTION_MYPROFILE || $this_section == SECTION_PLATFORM_ADMIN)) { //only display form and return to the previous URL if there was a course ID included if ($user_id != 0 && !api_is_anonymous()) { @@ -3015,7 +2992,7 @@ function api_not_allowed($print_headers = false, $message = null) { } if (!is_null(api_get_course_id())) { - api_set_firstpage_parameter(api_get_course_id()); + api_set_firstpage_parameter(api_get_course_int_id()); } // If the user has no user ID, then his session has expired @@ -3057,12 +3034,11 @@ function api_not_allowed($print_headers = false, $message = null) { // Check if the cookies are enabled. If are enabled and if no course ID was included in the requested URL, then the user has either lost his session or is anonymous, so redirect to homepage if( !isset($_COOKIE['TestCookie']) && empty($_COOKIE['TestCookie']) ) { $msg = Display::return_message(get_lang('NoCookies').'

'.get_lang('BackTo').' '.get_lang('CampusHomepage').'
', 'error', false); - } - else { + } else { // The session is over and we were not in a course, // or we try to get directly to a private course without being logged - if (!is_null(api_get_course_id())) { - api_set_firstpage_parameter(api_get_course_id()); + if (!is_null(api_get_course_int_id())) { + api_set_firstpage_parameter(api_get_course_int_id()); $action = api_get_self().'?'.Security::remove_XSS($_SERVER['QUERY_STRING']); $action = str_replace('&', '&', $action); $form = new FormValidator('formLogin', 'post', $action, null, array('class'=>'form-stacked')); @@ -3075,7 +3051,7 @@ function api_not_allowed($print_headers = false, $message = null) { $msg .= '

'.get_lang('LoginToGoToThisCourse').'

'; if (api_is_cas_activated()) { $msg .= Display::return_message(sprintf(get_lang('YouHaveAnInstitutionalAccount'), api_get_setting("Institution")), '', false); - $msg .= Display::div("
".getCASLogoHTML()." ".sprintf(get_lang('LoginWithYourAccount'), api_get_setting("Institution"))."

", array('align'=>'center')); + $msg .= Display::div("
".getCASLogoHTML()." ".sprintf(get_lang('LoginWithYourAccount'), api_get_setting("Institution"))."

", array('align'=>'center')); $msg .= Display::return_message(get_lang('YouDontHaveAnInstitutionAccount')); $msg .= "

".get_lang('LoginWithExternalAccount')."

"; $msg .= "
"; @@ -3087,8 +3063,7 @@ function api_not_allowed($print_headers = false, $message = null) { $msg .= "
"; } $msg .= '

'.get_lang('ReturnToCourseHomepage').'

'; - } - else { + } else { // we were not in a course, return to home page $msg = Display::return_message(get_lang('NotAllowed').'

'.get_lang('ReturnToCourseHomepage').'
', 'error', false); } @@ -6909,32 +6884,37 @@ function api_elog($string, $dump = 0) } -/* +/** * Set the cookie to go directly to the course code $in_firstpage * after login + * @param in_firstpage is the course code of the course to go */ -function api_set_firstpage_parameter($in_firstpage) { +function api_set_firstpage_parameter($in_firstpage) +{ setcookie("GotoCourse", $in_firstpage); } -/* +/** * Delete the cookie to go directly to the course code $in_firstpage * after login */ -function api_delete_firstpage_parameter() { +function api_delete_firstpage_parameter() +{ setcookie("GotoCourse", "", time() - 3600); } -/* - * Return true if course_code for direct course access after login is set +/** + * @return true if course_code for direct course access after login is set */ -function exist_firstpage_parameter() { +function exist_firstpage_parameter() +{ return (isset($_COOKIE['GotoCourse']) && $_COOKIE['GotoCourse'] != ""); } -/* - * +/** + * @return return the course_code of the course where user login */ -function api_get_firstpage_parameter() { +function api_get_firstpage_parameter() +{ return $_COOKIE['GotoCourse']; } diff --git a/main/inc/local.inc.php b/main/inc/local.inc.php index 07036afde0..9a3d8ebc49 100644 --- a/main/inc/local.inc.php +++ b/main/inc/local.inc.php @@ -1233,15 +1233,14 @@ if (isset($_cid)) { // direct login to course if ((isset($cas_login) && $cas_login && exist_firstpage_parameter()) - || ($logging_in && exist_firstpage_parameter())){ + || ($logging_in && exist_firstpage_parameter())) { $redir_coursecode = api_get_firstpage_parameter(); api_delete_firstpage_parameter(); // delete the cookie if (CourseManager::course_code_exists($redir_coursecode)) { $_SESSION['noredirection'] = false; $_SESSION['request_uri'] = api_get_path(WEB_COURSE_PATH).$redir_coursecode; } -} -elseif (api_user_is_login() && exist_firstpage_parameter()) { +} elseif (api_user_is_login() && exist_firstpage_parameter()) { $redir_coursecode = api_get_firstpage_parameter(); api_delete_firstpage_parameter(); // delete the cookie if (CourseManager::course_code_exists($redir_coursecode)) { From 6af43bc26be8b639a1cac30451d72babf69b4358 Mon Sep 17 00:00:00 2001 From: Hubert Borderiou Date: Wed, 8 Jan 2014 10:16:59 +0100 Subject: [PATCH 5/9] Minor correction --- main/inc/lib/main_api.lib.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main/inc/lib/main_api.lib.php b/main/inc/lib/main_api.lib.php index 48ef886da2..e2e9f09a67 100644 --- a/main/inc/lib/main_api.lib.php +++ b/main/inc/lib/main_api.lib.php @@ -2992,7 +2992,7 @@ function api_not_allowed($print_headers = false, $message = null) } if (!is_null(api_get_course_id())) { - api_set_firstpage_parameter(api_get_course_int_id()); + api_set_firstpage_parameter(api_get_course_id()); } // If the user has no user ID, then his session has expired @@ -3038,7 +3038,7 @@ function api_not_allowed($print_headers = false, $message = null) // The session is over and we were not in a course, // or we try to get directly to a private course without being logged if (!is_null(api_get_course_int_id())) { - api_set_firstpage_parameter(api_get_course_int_id()); + api_set_firstpage_parameter(api_get_course_id()); $action = api_get_self().'?'.Security::remove_XSS($_SERVER['QUERY_STRING']); $action = str_replace('&', '&', $action); $form = new FormValidator('formLogin', 'post', $action, null, array('class'=>'form-stacked')); From 3eb08bbace9855c900426ac19fe10c4dd625175c Mon Sep 17 00:00:00 2001 From: Hubert Borderiou Date: Wed, 8 Jan 2014 10:24:30 +0100 Subject: [PATCH 6/9] Coding convention --- main/exercice/exercise_admin.php | 294 +++++++++++++++---------------- 1 file changed, 147 insertions(+), 147 deletions(-) diff --git a/main/exercice/exercise_admin.php b/main/exercice/exercise_admin.php index 6159795d82..d7cf0b19d9 100644 --- a/main/exercice/exercise_admin.php +++ b/main/exercice/exercise_admin.php @@ -24,53 +24,53 @@ require_once 'exercise.lib.php'; $this_section = SECTION_COURSES; if (!api_is_allowed_to_edit(null,true)) { - api_not_allowed(true); + api_not_allowed(true); } $htmlHeadXtra[] = ''; + function feedbackselection() { + var index = document.exercise_admin.exerciseFeedbackType.selectedIndex; + + if (index == \'1\') { + document.exercise_admin.exerciseType[1].checked=true; + document.exercise_admin.exerciseType[0].disabled=true; + } else { + document.exercise_admin.exerciseType[0].disabled=false; + } + } + + function option_time_expired() { + if(document.getElementById(\'timercontrol\').style.display == \'none\') + { + document.getElementById(\'timercontrol\').style.display = \'block\'; + } else { + document.getElementById(\'timercontrol\').style.display = \'none\'; + } + } + + function check_per_page_one() { + /*if (document.getElementById(\'divtimecontrol\').style.display==\'none\') { + document.getElementById(\'divtimecontrol\').style.display=\'block\'; + document.getElementById(\'divtimecontrol\').display=block; + document.getElementById(\'timecontrol\').display=none; + }*/ + document.getElementById(\'exerciseType_0\').checked=true; + } + + function check_per_page_all() { + /*if (document.getElementById(\'divtimecontrol\').style.display==\'block\') { + document.getElementById(\'divtimecontrol\').style.display=\'none\'; + document.getElementById(\'enabletimercontroltotalminutes\').value=\'\'; + }*/ + + if (document.getElementById(\'exerciseType_1\') && document.getElementById(\'exerciseType_1\').checked) { + document.getElementById(\'exerciseType_0\').checked = true; + } + } + + function check_feedback() { + if (document.getElementById(\'result_disabled_1\').checked == true) { + document.getElementById(\'result_disabled_0\').checked = true; + } + } + + function check_direct_feedback() { + document.getElementById(\'option_page_one\').checked = true; + document.getElementById(\'result_disabled_0\').checked = true; + } + + function check_results_disabled() { + document.getElementById(\'exerciseType_2\').checked = true; + } + '; // to correct #4029 Random and number of attempt menu empty added window.onload=advanced_parameters; $htmlHeadXtra[] = '
'.get_lang("Choice").' '. get_lang("ExpectedChoice").' '. get_lang("Answer").''.get_lang("Comment").' '.get_lang("Comment").'