diff --git a/classes/feedback/section.php b/classes/feedback/section.php index 92a94784..f17cb4cd 100644 --- a/classes/feedback/section.php +++ b/classes/feedback/section.php @@ -219,13 +219,14 @@ public function delete() { $DB->delete_records(self::TABLE, ['id' => $this->id]); // Resequence the section numbers as necessary. - $allsections = $DB->get_records(self::TABLE, ['surveyid' => $this->surveyid], 'section ASC'); - $count = 1; - foreach ($allsections as $id => $section) { - if ($section->section != $count) { - $DB->set_field(self::TABLE, 'section', $count, ['id' => $id]); + if ($allsections = $DB->get_records(self::TABLE, ['surveyid' => $this->surveyid], 'section ASC')) { + $count = 1; + foreach ($allsections as $id => $section) { + if ($section->section != $count) { + $DB->set_field(self::TABLE, 'section', $count, ['id' => $id]); + } + $count++; } - $count++; } } diff --git a/classes/question/question.php b/classes/question/question.php index b8049ab1..d38c7710 100644 --- a/classes/question/question.php +++ b/classes/question/question.php @@ -138,7 +138,7 @@ public function __construct($id = 0, $question = null, $context = null, $params if ($qtypes === null) { $qtypes = $DB->get_records('questionnaire_question_type', [], 'typeid', - 'typeid, type, has_choices, response_table'); + 'typeid, type, has_choices, response_table') ?? []; } if ($id) { @@ -280,14 +280,15 @@ private function get_dependencies() { global $DB; $this->dependencies = []; - $dependencies = $DB->get_records('questionnaire_dependency', - ['questionid' => $this->id , 'surveyid' => $this->surveyid], 'id ASC'); - foreach ($dependencies as $dependency) { - $this->dependencies[$dependency->id] = new \stdClass(); - $this->dependencies[$dependency->id]->dependquestionid = $dependency->dependquestionid; - $this->dependencies[$dependency->id]->dependchoiceid = $dependency->dependchoiceid; - $this->dependencies[$dependency->id]->dependlogic = $dependency->dependlogic; - $this->dependencies[$dependency->id]->dependandor = $dependency->dependandor; + if ($dependencies = $DB->get_records('questionnaire_dependency', + ['questionid' => $this->id , 'surveyid' => $this->surveyid], 'id ASC')) { + foreach ($dependencies as $dependency) { + $this->dependencies[$dependency->id] = new \stdClass(); + $this->dependencies[$dependency->id]->dependquestionid = $dependency->dependquestionid; + $this->dependencies[$dependency->id]->dependchoiceid = $dependency->dependchoiceid; + $this->dependencies[$dependency->id]->dependlogic = $dependency->dependlogic; + $this->dependencies[$dependency->id]->dependandor = $dependency->dependandor; + } } } diff --git a/classes/question/sectiontext.php b/classes/question/sectiontext.php index 93739291..46c59d63 100644 --- a/classes/question/sectiontext.php +++ b/classes/question/sectiontext.php @@ -98,14 +98,15 @@ protected function question_survey_display($response, $descendantsdata, $blankqu return ''; } - $fbsections = $DB->get_records('questionnaire_fb_sections', ['surveyid' => $this->surveyid]); $filteredsections = []; // In which section(s) is this question? - foreach ($fbsections as $key => $fbsection) { - if ($scorecalculation = section::decode_scorecalculation($fbsection->scorecalculation)) { - if (array_key_exists($this->id, $scorecalculation)) { - array_push($filteredsections, $fbsection->section); + if ($fbsections = $DB->get_records('questionnaire_fb_sections', ['surveyid' => $this->surveyid])) { + foreach ($fbsections as $key => $fbsection) { + if ($scorecalculation = section::decode_scorecalculation($fbsection->scorecalculation)) { + if (array_key_exists($this->id, $scorecalculation)) { + array_push($filteredsections, $fbsection->section); + } } } } diff --git a/lib.php b/lib.php index beded219..80fca52a 100644 --- a/lib.php +++ b/lib.php @@ -342,7 +342,7 @@ function questionnaire_get_user_grades($questionnaire, $userid=0) { $sql = "SELECT r.id, u.id AS userid, r.grade AS rawgrade, r.submitted AS dategraded, r.submitted AS datesubmitted FROM {user} u, {questionnaire_response} r WHERE u.id = r.userid AND r.questionnaireid = $questionnaire->id AND r.complete = 'y' $usersql"; - return $DB->get_records_sql($sql, $params); + return $DB->get_records_sql($sql, $params) ?? []; } /** @@ -1061,7 +1061,7 @@ function questionnaire_print_overview($courses, &$htmlarray) { // Deadline. $str .= $OUTPUT->box(get_string('closeson', 'questionnaire', userdate($questionnaire->closedate)), 'info'); $attempts = $DB->get_records('questionnaire_response', - ['questionnaireid' => $questionnaire->id, 'userid' => $USER->id, 'complete' => 'y']); + ['questionnaireid' => $questionnaire->id, 'userid' => $USER->id, 'complete' => 'y']) ?? []; $nbattempts = count($attempts); // Do not display a questionnaire as due if it can only be sumbitted once and it has already been submitted! diff --git a/locallib.php b/locallib.php index 24f365e0..9bc6ac70 100644 --- a/locallib.php +++ b/locallib.php @@ -168,7 +168,7 @@ function questionnaire_get_user_responses($questionnaireid, $userid, $complete=t WHERE questionnaireid = ? AND userid = ? ".$andcomplete." - ORDER BY submitted ASC ", array($questionnaireid, $userid)); + ORDER BY submitted ASC ", array($questionnaireid, $userid)) ?? []; } /** @@ -416,7 +416,7 @@ function questionnaire_get_survey_list($courseid=0, $type='') { $params = [$courseid]; } } - return $DB->get_records_sql($sql, $params); + return $DB->get_records_sql($sql, $params) ?? []; } /** @@ -738,72 +738,86 @@ function questionnaire_check_page_breaks($questionnaire) { $newpbids = array(); $delpb = 0; $sid = $questionnaire->survey->id; - $questions = $DB->get_records('questionnaire_question', ['surveyid' => $sid, 'deleted' => 'n'], 'id'); $positions = array(); - foreach ($questions as $key => $qu) { - $positions[$qu->position]['question_id'] = $key; - $positions[$qu->position]['type_id'] = $qu->type_id; - $positions[$qu->position]['qname'] = $qu->name; - $positions[$qu->position]['qpos'] = $qu->position; - - $dependencies = $DB->get_records('questionnaire_dependency', ['questionid' => $key , 'surveyid' => $sid], - 'id ASC', 'id, dependquestionid, dependchoiceid, dependlogic'); - $positions[$qu->position]['dependencies'] = $dependencies ?? []; + if ($questions = $DB->get_records('questionnaire_question', ['surveyid' => $sid, 'deleted' => 'n'], 'position')) { + foreach ($questions as $key => $qu) { + $newqu = new stdClass(); + $newqu->question_id = $key; + $newqu->type_id = $qu->type_id; + $newqu->qname = $qu->name; + $newqu->qpos = $qu->position; + + $dependencies = $DB->get_records('questionnaire_dependency', ['questionid' => $key, 'surveyid' => $sid], + 'id ASC', 'id, dependquestionid, dependchoiceid, dependlogic'); + $newqu->dependencies = $dependencies ?? []; + $positions[] = (array)$newqu; + } } $count = count($positions); - for ($i = $count; $i > 0; $i--) { + for ($i = $count - 1; $i >= 0; $i--) { $qu = $positions[$i]; $questionnb = $i; + $prevqu = null; + $prevtypeid = null; + if ($i > 0) { + $prevqu = $positions[$i - 1]; + $prevtypeid = $prevqu['type_id']; + } if ($qu['type_id'] == QUESPAGEBREAK) { $questionnb--; // If more than one consecutive page breaks, remove extra one(s). - $prevqu = null; - $prevtypeid = null; - if ($i > 1) { - $prevqu = $positions[$i - 1]; - $prevtypeid = $prevqu['type_id']; - } - // If $i == $count then remove that extra page break in last position. - if ($prevtypeid == QUESPAGEBREAK || $i == $count || $qu['qpos'] == 1) { + // Remove that extra page break in 1st position. + if ($prevtypeid == QUESPAGEBREAK || $i == $count - 1 || $qu['qpos'] == 1) { $qid = $qu['question_id']; $delpb ++; $msg .= get_string("checkbreaksremoved", "questionnaire", $delpb).'
'; // Need to reload questions. - $questions = $DB->get_records('questionnaire_question', ['surveyid' => $sid, 'deleted' => 'n'], 'id'); - $DB->set_field('questionnaire_question', 'deleted', 'y', ['id' => $qid, 'surveyid' => $sid]); - $select = 'surveyid = '.$sid.' AND deleted = \'n\' AND position > '. - $questions[$qid]->position; - if ($records = $DB->get_records_select('questionnaire_question', $select, null, 'position ASC')) { - foreach ($records as $record) { - $DB->set_field('questionnaire_question', 'position', $record->position - 1, ['id' => $record->id]); + if ($questions = $DB->get_records('questionnaire_question', ['surveyid' => $sid, 'deleted' => 'n'], 'id')) { + $DB->set_field('questionnaire_question', 'deleted', 'y', ['id' => $qid, 'surveyid' => $sid]); + $select = 'surveyid = ' . $sid . ' AND deleted = \'n\' AND position > ' . + $questions[$qid]->position; + if ($records = $DB->get_records_select('questionnaire_question', $select, null, 'position ASC')) { + foreach ($records as $record) { + $DB->set_field('questionnaire_question', 'position', $record->position - 1, ['id' => $record->id]); + } } } } } // Add pagebreak between question child and not dependent question that follows. if ($qu['type_id'] != QUESPAGEBREAK) { - $j = $i - 1; - if ($j != 0) { - $prevtypeid = $positions[$j]['type_id']; - $prevdependencies = $positions[$j]['dependencies']; - + if ($prevqu) { + $prevdependencies = $prevqu['dependencies']; $outerdependencies = count($qu['dependencies']) >= count($prevdependencies) ? $qu['dependencies'] : $prevdependencies; $innerdependencies = count($qu['dependencies']) < count($prevdependencies) ? $qu['dependencies'] : $prevdependencies; + $okeys = []; + $ikeys = []; foreach ($outerdependencies as $okey => $outerdependency) { foreach ($innerdependencies as $ikey => $innerdependency) { if ($outerdependency->dependquestionid === $innerdependency->dependquestionid && - $outerdependency->dependchoiceid === $innerdependency->dependchoiceid && - $outerdependency->dependlogic === $innerdependency->dependlogic) { - unset($outerdependencies[$okey]); - unset($innerdependencies[$ikey]); + $outerdependency->dependchoiceid === $innerdependency->dependchoiceid && + $outerdependency->dependlogic === $innerdependency->dependlogic) { + $okeys[] = $okey; + $ikeys[] = $ikey; } } } + foreach ($okeys as $key) { + if (key_exists($key, $outerdependencies)) { + unset($outerdependencies[$key]); + } + } + foreach ($ikeys as $key) { + if (key_exists($key, $innerdependencies)) { + unset($innerdependencies[$key]); + } + } + $diffdependencies = count($outerdependencies) + count($innerdependencies); if (($prevtypeid != QUESPAGEBREAK && $diffdependencies != 0) @@ -825,9 +839,8 @@ function questionnaire_check_page_breaks($questionnaire) { return (false); } $newpbids[] = $newqid; - $movetopos = $i; $questionnaire = new questionnaire($course, $cm, $questionnaire->id, null); - $questionnaire->move_question($newqid, $movetopos); + $questionnaire->move_question($newqid, $qu['qpos']); } } } diff --git a/questionnaire.class.php b/questionnaire.class.php index 0a2da798..215b53f4 100644 --- a/questionnaire.class.php +++ b/questionnaire.class.php @@ -864,7 +864,7 @@ public function get_responses($userid=false, $groupid=0) { } $sql .= ' ORDER BY r.id'; - return $DB->get_records_sql($sql, $params); + return $DB->get_records_sql($sql, $params) ?? []; } /** @@ -1774,42 +1774,45 @@ public function survey_copy($owner) { } // Replicate all dependency data. - $dependquestions = $DB->get_records('questionnaire_dependency', ['surveyid' => $this->survey->id], 'questionid'); - foreach ($dependquestions as $dquestion) { - $record = new stdClass(); - $record->questionid = $qidarray[$dquestion->questionid]; - $record->surveyid = $newsid; - $record->dependquestionid = $qidarray[$dquestion->dependquestionid]; - // The response may not use choice id's (example boolean). If not, just copy the value. - $responsetype = $this->questions[$dquestion->dependquestionid]->responsetype; - if ($responsetype->transform_choiceid($dquestion->dependchoiceid) == $dquestion->dependchoiceid) { - $record->dependchoiceid = $cidarray[$dquestion->dependchoiceid]; - } else { - $record->dependchoiceid = $dquestion->dependchoiceid; + if ($dependquestions = $DB->get_records('questionnaire_dependency', ['surveyid' => $this->survey->id], 'questionid')) { + foreach ($dependquestions as $dquestion) { + $record = new stdClass(); + $record->questionid = $qidarray[$dquestion->questionid]; + $record->surveyid = $newsid; + $record->dependquestionid = $qidarray[$dquestion->dependquestionid]; + // The response may not use choice id's (example boolean). If not, just copy the value. + $responsetype = $this->questions[$dquestion->dependquestionid]->responsetype; + if ($responsetype->transform_choiceid($dquestion->dependchoiceid) == $dquestion->dependchoiceid) { + $record->dependchoiceid = $cidarray[$dquestion->dependchoiceid]; + } else { + $record->dependchoiceid = $dquestion->dependchoiceid; + } + $record->dependlogic = $dquestion->dependlogic; + $record->dependandor = $dquestion->dependandor; + $DB->insert_record('questionnaire_dependency', $record); } - $record->dependlogic = $dquestion->dependlogic; - $record->dependandor = $dquestion->dependandor; - $DB->insert_record('questionnaire_dependency', $record); } // Replicate any feedback data. // TODO: Need to handle image attachments (same for other copies above). - $fbsections = $DB->get_records('questionnaire_fb_sections', ['surveyid' => $this->survey->id], 'id'); - foreach ($fbsections as $fbsid => $fbsection) { - $fbsection->surveyid = $newsid; - $scorecalculation = section::decode_scorecalculation($fbsection->scorecalculation); - $newscorecalculation = []; - foreach ($scorecalculation as $qid => $val) { - $newscorecalculation[$qidarray[$qid]] = $val; - } - $fbsection->scorecalculation = serialize($newscorecalculation); - unset($fbsection->id); - $newfbsid = $DB->insert_record('questionnaire_fb_sections', $fbsection); - $feedbackrecs = $DB->get_records('questionnaire_feedback', ['sectionid' => $fbsid], 'id'); - foreach ($feedbackrecs as $feedbackrec) { - $feedbackrec->sectionid = $newfbsid; - unset($feedbackrec->id); - $DB->insert_record('questionnaire_feedback', $feedbackrec); + if ($fbsections = $DB->get_records('questionnaire_fb_sections', ['surveyid' => $this->survey->id], 'id')) { + foreach ($fbsections as $fbsid => $fbsection) { + $fbsection->surveyid = $newsid; + $scorecalculation = section::decode_scorecalculation($fbsection->scorecalculation); + $newscorecalculation = []; + foreach ($scorecalculation as $qid => $val) { + $newscorecalculation[$qidarray[$qid]] = $val; + } + $fbsection->scorecalculation = serialize($newscorecalculation); + unset($fbsection->id); + $newfbsid = $DB->insert_record('questionnaire_fb_sections', $fbsection); + if ($feedbackrecs = $DB->get_records('questionnaire_feedback', ['sectionid' => $fbsid], 'id')) { + foreach ($feedbackrecs as $feedbackrec) { + $feedbackrec->sectionid = $newfbsid; + unset($feedbackrec->id); + $DB->insert_record('questionnaire_feedback', $feedbackrec); + } + } } } @@ -1843,22 +1846,24 @@ private function response_check_format($section, $formdata, $checkmissing = true } $qnum = $i - 1; - foreach ($this->questionsbysec[$section] as $questionid) { - $tid = $this->questions[$questionid]->type_id; - if ($tid != QUESSECTIONTEXT) { - $qnum++; - } - if (!$this->questions[$questionid]->response_complete($formdata)) { - $missing++; - $strnum = get_string('num', 'questionnaire').$qnum.'. '; - $strmissing .= $strnum; - // Pop-up notification at the point of the error. - $strnoti = get_string('missingquestion', 'questionnaire').$strnum; - $this->questions[$questionid]->add_notification($strnoti); - } - if (!$this->questions[$questionid]->response_valid($formdata)) { - $wrongformat++; - $strwrongformat .= get_string('num', 'questionnaire').$qnum.'. '; + if (key_exists($section, $this->questionsbysec)) { + foreach ($this->questionsbysec[$section] as $questionid) { + $tid = $this->questions[$questionid]->type_id; + if ($tid != QUESSECTIONTEXT) { + $qnum++; + } + if (!$this->questions[$questionid]->response_complete($formdata)) { + $missing++; + $strnum = get_string('num', 'questionnaire') . $qnum . '. '; + $strmissing .= $strnum; + // Pop-up notification at the point of the error. + $strnoti = get_string('missingquestion', 'questionnaire') . $strnum; + $this->questions[$questionid]->add_notification($strnoti); + } + if (!$this->questions[$questionid]->response_valid($formdata)) { + $wrongformat++; + $strwrongformat .= get_string('num', 'questionnaire') . $qnum . '. '; + } } } $message = ''; @@ -3014,7 +3019,7 @@ protected function get_survey_all_responses($rid = '', $userid = '', $groupid = // If a questionnaire is "public", and this is the master course, need to get responses from all instances. if ($this->survey_is_public_master()) { - $qids = array_keys($DB->get_records('questionnaire', ['sid' => $this->sid], 'id')); + $qids = array_keys($DB->get_records('questionnaire', ['sid' => $this->sid], 'id') ?? []); } else { $qids = $this->id; } @@ -3032,7 +3037,7 @@ protected function get_survey_all_responses($rid = '', $userid = '', $groupid = $allresponsessql .= " ORDER BY usrid, id"; $allresponses = $DB->get_recordset_sql($allresponsessql, $allresponsesparams); - return $allresponses; + return $allresponses ?? []; } /** @@ -3785,11 +3790,12 @@ public function response_analysis($rid, $resps, $compare, $isgroupmember, $allre $sectionlabel = $fbsections[$sectionid]->sectionlabel; $sectionheading = $fbsections[$sectionid]->sectionheading; - $feedbacks = $DB->get_records('questionnaire_feedback', ['sectionid' => $sectionid]); $labels = array(); - foreach ($feedbacks as $feedback) { - if ($feedback->feedbacklabel != '') { - $labels[] = $feedback->feedbacklabel; + if ($feedbacks = $DB->get_records('questionnaire_feedback', ['sectionid' => $sectionid])) { + foreach ($feedbacks as $feedback) { + if ($feedback->feedbacklabel != '') { + $labels[] = $feedback->feedbacklabel; + } } } $feedback = $DB->get_record_select('questionnaire_feedback', diff --git a/questions.php b/questions.php index 464b7e92..6175512c 100644 --- a/questions.php +++ b/questions.php @@ -85,7 +85,7 @@ $questionnaireid = $questionnaire->id; // Need to reload questions before setting deleted question to 'y'. - $questions = $DB->get_records('questionnaire_question', ['surveyid' => $sid, 'deleted' => 'n'], 'id'); + $questions = $DB->get_records('questionnaire_question', ['surveyid' => $sid, 'deleted' => 'n'], 'id') ?? []; $DB->set_field('questionnaire_question', 'deleted', 'y', ['id' => $qid, 'surveyid' => $sid]); // Delete all dependency records for this question. diff --git a/tests/generator/lib.php b/tests/generator/lib.php index bed87009..e4fb0149 100644 --- a/tests/generator/lib.php +++ b/tests/generator/lib.php @@ -22,6 +22,7 @@ global $CFG; require_once($CFG->dirroot.'/mod/questionnaire/locallib.php'); +require_once($CFG->dirroot . '/mod/questionnaire/classes/question/question.php'); /** * The mod_questionnaire data generator. @@ -33,6 +34,11 @@ */ class mod_questionnaire_generator extends testing_module_generator { + /** + * @var int Current position of assigned options. + */ + protected $curpos = 0; + /** * @var int keep track of how many questions have been created. */ @@ -570,8 +576,6 @@ public function create_response($questionresponses, $record = null, $complete = * @param int $number */ public function assign_opts($number = 5) { - static $curpos = 0; - $opts = 'blue, red, yellow, orange, green, purple, white, black, earth, wind, fire, space, car, truck, train' . ', van, tram, one, two, three, four, five, six, seven, eight, nine, ten, eleven, twelve, thirteen' . ', fourteen, fifteen, sixteen, seventeen, eighteen, nineteen, twenty, happy, sad, jealous, angry'; @@ -584,10 +588,10 @@ public function assign_opts($number = 5) { $retopts = []; while (count($retopts) < $number) { - $retopts[] = $opts[$curpos]; + $retopts[] = $opts[$this->curpos]; $retopts = array_unique($retopts); - if (++$curpos == $numopts) { - $curpos = 0; + if (++$this->curpos == $numopts) { + $this->curpos = 0; } } // Return re-indexed version of array (otherwise you can get a weird index of 1,2,5,9, etc). @@ -671,6 +675,7 @@ public function create_and_fully_populate($coursecount = 4, $studentcount = 20, $dg = $this->datagenerator; $qdg = $this; + $this->curpos = 0; $questiontypes = [QUESTEXT, QUESESSAY, QUESNUMERIC, QUESDATE, QUESRADIO, QUESDROP, QUESCHECK, QUESRATE]; $students = []; $courses = []; diff --git a/tests/privacy_provider_test.php b/tests/privacy_provider_test.php index 21ee99ea..c5bbf014 100644 --- a/tests/privacy_provider_test.php +++ b/tests/privacy_provider_test.php @@ -129,8 +129,8 @@ public function test_export_user_data() { $this->assertEquals('7. Numeric 1004', $data->responses[0]['questions'][7]->questionname); $this->assertEquals(83, $data->responses[0]['questions'][7]->answers[0]); $this->assertEquals('22. Rate Scale 1014', $data->responses[0]['questions'][22]->questionname); - $this->assertEquals('eleven = 1', $data->responses[0]['questions'][22]->answers[0]); - $this->assertEquals('eighteen = 3', $data->responses[0]['questions'][22]->answers[7]); + $this->assertEquals('fourteen = 1', $data->responses[0]['questions'][22]->answers[0]); + $this->assertEquals('happy = 3', $data->responses[0]['questions'][22]->answers[7]); } /** diff --git a/tests/questiontypes_test.php b/tests/questiontypes_test.php index be7aff5e..c4e3c393 100644 --- a/tests/questiontypes_test.php +++ b/tests/questiontypes_test.php @@ -29,6 +29,7 @@ global $CFG; require_once($CFG->dirroot.'/mod/questionnaire/locallib.php'); +require_once($CFG->dirroot . '/mod/questionnaire/classes/question/question.php'); /** * Unit tests for questionnaire_questiontypes_testcase. diff --git a/tests/responsetypes_test.php b/tests/responsetypes_test.php index 9c48b01c..c26efe54 100644 --- a/tests/responsetypes_test.php +++ b/tests/responsetypes_test.php @@ -31,6 +31,7 @@ require_once($CFG->dirroot.'/mod/questionnaire/locallib.php'); require_once($CFG->dirroot . '/mod/questionnaire/tests/generator_test.php'); require_once($CFG->dirroot . '/mod/questionnaire/tests/questiontypes_test.php'); +require_once($CFG->dirroot . '/mod/questionnaire/classes/question/question.php'); /** * Unit tests for questionnaire_responsetypes_testcase.