Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance improvements for large courses #164

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Performance improvements for large courses #164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions classes/external.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
}
Expand Down
194 changes: 121 additions & 73 deletions lib.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,20 +304,24 @@ 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());

$cdisplay['limitanswers'] = true;
$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;
Expand All @@ -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;
}
Expand Down Expand Up @@ -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 '<div class="reportlink"><a href="report.php?id='.$cm->id.'">'.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 '<div class="reportlink"><a href="report.php?id='.$cm->id.'">'.get_string("viewallresponses", "choicegroup", $responses['responses']);
if ($choicegroup->multipleenrollmentspossible) {
echo ' ' . get_string("byparticipants", "choicegroup", $responses['respondants']);
}
echo '</a></div>';
}
Expand Down Expand Up @@ -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();

Expand All @@ -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)) {
Expand All @@ -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 `['<groupid>' => <num_responses>]`
*/
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);
}

/**
Expand Down Expand Up @@ -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)));
}
Expand Down
6 changes: 4 additions & 2 deletions renderer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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')));
Expand Down
14 changes: 12 additions & 2 deletions report.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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)) {
Expand Down
8 changes: 2 additions & 6 deletions view.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 '<div class="clearer"></div>';
Expand Down Expand Up @@ -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
Expand Down