From d0726d9a034c63a59a80075b4170e3b75378550d Mon Sep 17 00:00:00 2001 From: dkutin Date: Fri, 6 May 2022 10:01:29 -0400 Subject: [PATCH] Performance improvements for large courses --- classes/external.php | 7 +- lib.php | 194 +++++++++++++++++++++++++++---------------- renderer.php | 6 +- report.php | 14 +++- view.php | 8 +- 5 files changed, 143 insertions(+), 86 deletions(-) diff --git a/classes/external.php b/classes/external.php index 4396877..c2dda71 100755 --- a/classes/external.php +++ b/classes/external.php @@ -79,7 +79,8 @@ public static function get_choicegroup_options($choicegroupid, $userid, $allopti require_capability('mod/choicegroup:choose', $context); $groupmode = groups_get_activity_groupmode($cm); - $allresponses = choicegroup_get_response_data($choicegroup, $cm, $groupmode, $choicegroup->onlyactive); + $current_group = $groupmode > 0 ? groups_get_activity_group($cm) : 0; + $options_count = choicegroup_get_options_count($choicegroup, $context, $current_group, $choicegroup->onlyactive); $answers = choicegroup_get_user_answer($choicegroup, $userid, true); foreach ($choicegroup->option as $optionid => $text) { @@ -91,8 +92,8 @@ public static function get_choicegroup_options($choicegroupid, $userid, $allopti $option['maxanswers'] = $choicegroup->maxanswers[$optionid]; $option['displaylayout'] = $choicegroup->display; - if (isset($allresponses[$text])) { - $option['countanswers'] = count($allresponses[$text]); + if (isset($options_count[$text])) { + $option['countanswers'] = $options_count[$text]; } else { $option['countanswers'] = 0; } diff --git a/lib.php b/lib.php index 7b25768..8b1f213 100644 --- a/lib.php +++ b/lib.php @@ -304,10 +304,10 @@ function choicegroup_update_instance($choicegroup) { * @param object $choicegroup * @param object $user * @param object $coursemodule - * @param array $allresponses + * @param int $groupmode * @return array */ -function choicegroup_prepare_options($choicegroup, $user, $coursemodule, $allresponses) { +function choicegroup_prepare_options($choicegroup, $user, $coursemodule, $groupmode) { $cdisplay = array('options'=>array()); @@ -315,9 +315,13 @@ function choicegroup_prepare_options($choicegroup, $user, $coursemodule, $allres $context = context_module::instance($coursemodule->id); $answers = choicegroup_get_user_answer($choicegroup, $user, true, true); + $currentgroup = $groupmode > 0 ? groups_get_activity_group($coursemodule) : 0; + $options_count = choicegroup_get_options_count($choicegroup, $context, $currentgroup, $choicegroup->onlyactive); + if (!isset($choicegroup->option)) { $choicegroup->option = []; } + foreach ($choicegroup->option as $optionid => $text) { if (isset($text)) { //make sure there are no dud entries in the db with blank text values. $option = new stdClass; @@ -327,8 +331,8 @@ function choicegroup_prepare_options($choicegroup, $user, $coursemodule, $allres $option->maxanswers = $choicegroup->maxanswers[$optionid]; $option->displaylayout = $choicegroup->display; - if (isset($allresponses[$text])) { - $option->countanswers = count($allresponses[$text]); + if (isset($options_count[$text])) { + $option->countanswers = $options_count[$text]; } else { $option->countanswers = 0; } @@ -431,28 +435,18 @@ function choicegroup_user_submit_response($formanswer, $choicegroup, $userid, $c /** * @param object $choicegroup - * @param array $allresponses * @param object $cm + * @param int $groupmode * @return void Output is echo'd */ -function choicegroup_show_reportlink($choicegroup, $allresponses, $cm) { - $responsecount = 0; - $respondents = array(); - foreach($allresponses as $optionid => $userlist) { - if ($optionid) { - $responsecount += count($userlist); - if ($choicegroup->multipleenrollmentspossible) { - foreach ($userlist as $user) { - if (!in_array($user->id, $respondents)) { - $respondents[] = $user->id; - } - } - } - } - } - echo '
'.get_string("viewallresponses", "choicegroup", $responsecount); - if ($choicegroup->multipleenrollmentspossible == 1) { - echo ' ' . get_string("byparticipants", "choicegroup", count($respondents)); +function choicegroup_show_reportlink($choicegroup, $cm, $groupmode) { + // Get the current group. + $currentgroup = $groupmode > 0 ? groups_get_activity_group($cm) : 0; + $responses = choicegroup_get_responses_count($choicegroup, $cm, $currentgroup, $choicegroup->onlyactive); + + echo '
'.get_string("viewallresponses", "choicegroup", $responses['responses']); + if ($choicegroup->multipleenrollmentspossible) { + echo ' ' . get_string("byparticipants", "choicegroup", $responses['respondants']); } echo '
'; } @@ -878,17 +872,16 @@ function choicegroup_reset_course_form_defaults($course) { } /** - * @global object - * @global object - * @global object * @uses CONTEXT_MODULE * @param object $choicegroup * @param object $cm * @param int $groupmode Group mode * @param bool $onlyactive Whether to get response data for active users only + * @param int $from + * @param int $limit * @return array */ -function choicegroup_get_response_data($choicegroup, $cm, $groupmode, $onlyactive) { +function choicegroup_get_response_data($choicegroup, $cm, $groupmode, $onlyactive, $from, $limit) { // Initialise the returned array, which is a matrix: $allresponses[responseid][userid] = responseobject. static $allresponses = array(); @@ -903,41 +896,35 @@ function choicegroup_get_response_data($choicegroup, $cm, $groupmode, $onlyactiv $currentgroup = 0; } - // First get all the users who have access here. - // To start with we assume they are all "unanswered" then move them later. $ctx = \context_module::instance($cm->id); - $users = get_enrolled_users($ctx, 'mod/choicegroup:choose', $currentgroup, 'u.*', 'u.lastname, u.firstname', 0, 0, $onlyactive); - if ($users) { - $modinfo = get_fast_modinfo($cm->course); - $cminfo = $modinfo->get_cm($cm->id); - $availability = new \core_availability\info_module($cminfo); - $users = $availability->filter_user_list($users); + $responses = choicegroup_get_responses($choicegroup, $ctx, $currentgroup, $onlyactive, $from, $limit); + if ($responses->valid()) { + foreach ($responses as $response) { + $allresponses[$response->groupid][$response->userid] = (object) [ + 'id' => $response->userid, + 'firstname' => $response->firstname, + 'lastname' => $response->lastname, + 'idnumber' => $response->idnumber, + 'email' => $response->email, + 'timemodified' => $response->timemodified, + ]; + } } - $allresponses[0] = $users; - - $responses = choicegroup_get_responses($choicegroup, $ctx, $currentgroup, $onlyactive); - foreach ($responses as $response){ - if (isset($users[$response->userid])) { - $allresponses[$response->groupid][$response->userid] = clone $users[$response->userid]; - $allresponses[$response->groupid][$response->userid]->timemodified = $response->timeadded; + $responses->close(); - unset($allresponses[0][$response->userid]); - } - } - return $allresponses; + return $allresponses; } -/* Return an array with the options selected of users of the $choicegroup - * +/** + * Return an array with the options selected of users of the $choicegroup. * @param object $choicegroup choicegroup record - * @param context_module $ctx Context instance + * @param object $ctx context module * @param int $currentgroup Current group * @param bool $onlyactive Whether to get responses for active users only - * @return array of selected options by all users + * @return \moodle_recordset of selected options by all users. */ -function choicegroup_get_responses($choicegroup, $ctx, $currentgroup, $onlyactive) { - +function choicegroup_get_responses($choicegroup, $ctx, $currentgroup, $onlyactive, $from, $limit){ global $DB; if (is_numeric($choicegroup)) { @@ -950,12 +937,87 @@ function choicegroup_get_responses($choicegroup, $ctx, $currentgroup, $onlyactiv list($esql, $params2) = get_enrolled_sql($ctx, 'mod/choicegroup:choose', $currentgroup, $onlyactive); $params = array_merge($params1, $params2); - $sql = 'SELECT gm.* FROM {user} u JOIN ('.$esql.') je ON je.id = u.id + // Add user fields to the result, so we do not have to query twice to display. + $sql = 'SELECT gm.*, u.firstname, u.lastname, u.idnumber, u.email FROM {user} u JOIN ('.$esql.') je ON je.id = u.id JOIN {groups_members} gm ON gm.userid = u.id AND groupid IN ( SELECT groupid FROM {choicegroup_options} WHERE choicegroupid=:choicegroupid) WHERE u.deleted = 0 ORDER BY u.lastname ASC,u.firstname ASC'; - return $DB->get_records_sql($sql, $params); + return $DB->get_recordset_sql($sql, $params, $from, $limit); +} + +/** + * @param object $choicegroup Choicegroup record. + * @param object $cm Course module. + * @param int $groupmode Group mode. + * @param bool $onlyactive Whether to get response data for active users only. + * @return array An array of the form `['responses' => [], 'respondants' => []]` containing the count of each. + */ +function choicegroup_get_responses_count($choicegroup, $cm, $currentgroup, $onlyactive) { + global $DB; + + $responses = 0; + $respondants = 0; + + $choicegroupid = is_numeric($choicegroup) ? $choicegroup : $choicegroup->id; + + list($insql, $params) = get_enrolled_sql(\context_module::instance($cm->id), 'mod/choicegroup:choose', $currentgroup, $onlyactive); + + // When grouped, will return a row for each user, with the column being the number of responses the user had submitted. + // This is useful for multipleenrollmentspossible, and allows us to use two variants of the same query. + $sql = " + SELECT COUNT('x') as responses FROM {user} u + JOIN ($insql) je ON je.id = u.id + JOIN {groups_members} gm ON gm.userid = u.id AND groupid IN ( + SELECT groupid FROM {choicegroup_options} WHERE choicegroupid = :choicegroupid + ) + WHERE u.deleted = 0 + " . ($choicegroup->multipleenrollmentspossible ? ' GROUP BY u.id' : ''); + + $params = array_merge($params, ['choicegroupid' => $choicegroupid]); + + if ($choicegroup->multipleenrollmentspossible) { + $rs = $DB->get_recordset_sql($sql, $params); + + if ($rs->valid()) { + foreach ($rs as $record) { + $respondants++; + $responses += $record->responses; + } + } + + $rs->close(); + } else { + $responses = $DB->count_records_sql($sql, $params); + } + + return ['responses' => $responses, 'respondants' => $respondants]; +} + +/** + * @param object $choicegroup Choicegroup record + * @param object $ctx The course module context. + * @return array An array where each entry is of the form `['' => ]` + */ +function choicegroup_get_options_count($choicegroup, $ctx, $currentgroup, $onlyactive) { + global $DB; + + list($insql, $params) = get_enrolled_sql($ctx, 'mod/choicegroup:choose', $currentgroup, $onlyactive); + + $sql = " + SELECT gm.groupid, SUM(1) as count FROM {user} u + JOIN ($insql) je ON je.id = u.id + JOIN {groups_members} gm ON gm.userid = u.id AND groupid IN ( + SELECT groupid FROM {choicegroup_options} WHERE choicegroupid = :choicegroupid + ) + WHERE u.deleted = 0 + GROUP BY gm.groupid"; + + $params = array_merge($params, ['choicegroupid' => $choicegroup->id]); + + $result = $DB->get_records_sql($sql, $params); + + return array_map(function ($record) { return $record->count; }, $result); } /** @@ -1015,26 +1077,12 @@ function choicegroup_extend_settings_navigation(settings_navigation $settings, n return false; } - // Big function, approx 6 SQL calls per user. - $allresponses = choicegroup_get_response_data($choicegroup, $PAGE->cm, $groupmode, $choicegroup->onlyactive); - - $responsecount = 0; - $respondents = array(); - foreach($allresponses as $optionid => $userlist) { - if ($optionid) { - $responsecount += count($userlist); - if ($choicegroup->multipleenrollmentspossible) { - foreach ($userlist as $user) { - if (!in_array($user->id, $respondents)) { - $respondents[] = $user->id; - } - } - } - } - } - $viewallresponsestext = get_string("viewallresponses", "choicegroup", $responsecount); + $currentgroup = $groupmode > 0 ? groups_get_activity_group($PAGE->cm) : 0; + $responses = choicegroup_get_responses_count($choicegroup, $PAGE->cm, $currentgroup, $choicegroup->onlyactive); + + $viewallresponsestext = get_string("viewallresponses", "choicegroup", $responses['responses']); if ($choicegroup->multipleenrollmentspossible == 1) { - $viewallresponsestext .= ' ' . get_string("byparticipants", "choicegroup", count($respondents)); + $viewallresponsestext .= ' ' . get_string("byparticipants", "choicegroup", $responses['respondants']); } $choicegroupnode->add($viewallresponsestext, new moodle_url('/mod/choicegroup/report.php', array('id'=>$PAGE->cm->id))); } diff --git a/renderer.php b/renderer.php index 02c6ee1..fd3f30a 100644 --- a/renderer.php +++ b/renderer.php @@ -260,6 +260,8 @@ public function display_publish_name_vertical($choicegroups) { ksort($choicegroups->options); $columns = array(); + $response_count = choicegroup_get_options_count($choicegroups, \context_module::instance($PAGE->cm->id), 0, $choicegroups->onlyactive); + foreach ($choicegroups->options as $optionid => $options) { $coldata = ''; if ($choicegroups->showunanswered && $optionid == 0) { @@ -268,8 +270,8 @@ public function display_publish_name_vertical($choicegroups) { $coldata .= html_writer::tag('div', format_string(choicegroup_get_option_text($choicegroups, $choicegroups->options[$optionid]->groupid)), array('class'=>'option')); } $numberofuser = 0; - if (!empty($options->user) && count($options->user) > 0) { - $numberofuser = count($options->user); + if ($response_count[$choicegroups->options[$optionid]->groupid] > 0) { + $numberofuser = $response_count[$choicegroups->options[$optionid]->groupid]; } $coldata .= html_writer::tag('div', ' ('.$numberofuser. ')', array('class'=>'numberofuser', 'title' => get_string('numberofuser', 'choicegroup'))); diff --git a/report.php b/report.php index 28d7e3f..752e5ac 100644 --- a/report.php +++ b/report.php @@ -31,14 +31,19 @@ $format = optional_param('format', CHOICEGROUP_PUBLISH_NAMES, PARAM_INT); $download = optional_param('download', '', PARAM_ALPHA); $action = optional_param('action', '', PARAM_ALPHA); +$limit = optional_param('limit', 50, PARAM_INT); +$page = optional_param('page', 0, PARAM_INT); $grpsmemberids = optional_param_array('grpsmemberid', array(), PARAM_INT); //get array of responses to delete. -$url = new moodle_url('/mod/choicegroup/report.php', array('id'=>$id)); +$url = new moodle_url('/mod/choicegroup/report.php', array('id' => $id, 'page' => $page, 'limit' => $limit)); if ($format !== CHOICEGROUP_PUBLISH_NAMES) { $url->param('format', $format); } if ($download !== '') { $url->param('download', $download); + + // If a download action has been sent, do not enforce a limit. + $limit = $page = 0; } if ($action !== '') { $url->param('action', $action); @@ -102,7 +107,8 @@ $groups_ids[] = $group->id; } } -$users = choicegroup_get_response_data($choicegroup, $cm, $groupmode, $choicegroup->onlyactive); + +$users = choicegroup_get_response_data($choicegroup, $cm, $groupmode, $choicegroup->onlyactive, $page * $limit, $limit); if ($download == "ods" && has_capability('mod/choicegroup:downloadresponses', $context)) { require_once("$CFG->libdir/odslib.class.php"); @@ -274,9 +280,13 @@ $choicegroup->maxanswers[0] = 0; } +$currentgroup = $groupmode > 0 ? groups_get_activity_group($cm) : 0; $results = prepare_choicegroup_show_results($choicegroup, $course, $cm, $users); +$count = choicegroup_get_responses_count($choicegroup, $cm, $currentgroup, $choicegroup->onlyactive); + $renderer = $PAGE->get_renderer('mod_choicegroup'); echo $renderer->display_result($results, has_capability('mod/choicegroup:readresponses', $context)); +echo $OUTPUT->paging_bar($count['responses'], $page, $limit, $url); //now give links for downloading spreadsheets. if (!empty($users) && has_capability('mod/choicegroup:downloadresponses',$context)) { diff --git a/view.php b/view.php index fa74df8..0cbf44e 100644 --- a/view.php +++ b/view.php @@ -199,12 +199,8 @@ groups_print_activity_menu($cm, $CFG->wwwroot . '/mod/choicegroup/view.php?id='.$id); } -// Big function, approx 6 SQL calls per user. -$allresponses = choicegroup_get_response_data($choicegroup, $cm, $groupmode, $choicegroup->onlyactive); - - if (has_capability('mod/choicegroup:readresponses', $context)) { - choicegroup_show_reportlink($choicegroup, $allresponses, $cm); + choicegroup_show_reportlink($choicegroup, $cm, $groupmode); } echo '
'; @@ -246,7 +242,7 @@ } } -$options = choicegroup_prepare_options($choicegroup, $USER, $cm, $allresponses); +$options = choicegroup_prepare_options($choicegroup, $USER, $cm, $groupmode); $renderer = $PAGE->get_renderer('mod_choicegroup'); if ( (!$current or $choicegroup->allowupdate) and $choicegroupopen and is_enrolled($context, null, 'mod/choicegroup:choose')) { // They haven't made their choicegroup yet or updates allowed and choicegroup is open