From d6ebf0d5818b85504ab1a2b107a26040a5fc5ce9 Mon Sep 17 00:00:00 2001 From: Tony Butler Date: Thu, 6 May 2021 18:22:58 +0100 Subject: [PATCH 1/3] mod_choicegroup: Add option to filter out expired/suspended users --- .../moodle2/backup_choicegroup_stepslib.php | 2 +- classes/external.php | 3 ++- db/install.xml | 3 ++- db/upgrade.php | 17 +++++++++++++++-- lang/en/choicegroup.php | 1 + lib.php | 19 +++++++++++++++---- mod_form.php | 4 +++- report.php | 2 +- version.php | 5 ++--- view.php | 3 ++- 10 files changed, 44 insertions(+), 15 deletions(-) diff --git a/backup/moodle2/backup_choicegroup_stepslib.php b/backup/moodle2/backup_choicegroup_stepslib.php index 61b00ca..c01f7cd 100644 --- a/backup/moodle2/backup_choicegroup_stepslib.php +++ b/backup/moodle2/backup_choicegroup_stepslib.php @@ -42,7 +42,7 @@ protected function define_structure() { 'multipleenrollmentspossible', 'showresults', 'display', 'allowupdate', 'showunanswered', 'limitanswers', 'timeopen', 'timeclose', 'timemodified', - 'completionsubmit', 'sortgroupsby')); + 'completionsubmit', 'sortgroupsby', 'onlyactive')); $options = new backup_nested_element('options'); diff --git a/classes/external.php b/classes/external.php index b358510..4396877 100755 --- a/classes/external.php +++ b/classes/external.php @@ -78,7 +78,8 @@ public static function get_choicegroup_options($choicegroupid, $userid, $allopti self::validate_context($context); require_capability('mod/choicegroup:choose', $context); - $allresponses = choicegroup_get_response_data($choicegroup, $cm); // Big function, approx 6 SQL calls per user + $groupmode = groups_get_activity_groupmode($cm); + $allresponses = choicegroup_get_response_data($choicegroup, $cm, $groupmode, $choicegroup->onlyactive); $answers = choicegroup_get_user_answer($choicegroup, $userid, true); foreach ($choicegroup->option as $optionid => $text) { diff --git a/db/install.xml b/db/install.xml index 529d99d..d023fa2 100644 --- a/db/install.xml +++ b/db/install.xml @@ -23,7 +23,8 @@ - + + diff --git a/db/upgrade.php b/db/upgrade.php index 3a660e4..af0afde 100644 --- a/db/upgrade.php +++ b/db/upgrade.php @@ -82,7 +82,20 @@ function xmldb_choicegroup_upgrade($oldversion) { upgrade_mod_savepoint(true, 2021071400, 'choicegroup'); } - return true; -} + if ($oldversion < 2021080500) { + // Define field onlyactive to be added to choicegroup. + $table = new xmldb_table('choicegroup'); + $field = new xmldb_field('onlyactive', XMLDB_TYPE_INTEGER, '1', null, XMLDB_NOTNULL, null, '0', 'sortgroupsby'); + // Conditionally launch add field onlyactive. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + // Group choice savepoint reached. + upgrade_mod_savepoint(true, 2021080500, 'choicegroup'); + } + + return true; +} diff --git a/lang/en/choicegroup.php b/lang/en/choicegroup.php index 010a5cb..da12dbd 100644 --- a/lang/en/choicegroup.php +++ b/lang/en/choicegroup.php @@ -78,6 +78,7 @@ $string['notanswered'] = 'Not answered yet'; $string['notenrolledchoose'] = 'Sorry, only enrolled users are allowed to make choices.'; $string['notopenyet'] = 'Sorry, this activity is not available until {$a}'; +$string['onlyactive'] = 'Filter out response data for users with expired or suspended enrolments'; $string['option'] = 'Group'; $string['pluginadministration'] = 'Choice administration'; $string['pluginname'] = 'Group choice'; diff --git a/lib.php b/lib.php index d2ec625..075f0fe 100644 --- a/lib.php +++ b/lib.php @@ -884,20 +884,29 @@ function choicegroup_reset_course_form_defaults($course) { * @uses CONTEXT_MODULE * @param object $choicegroup * @param object $cm + * @param int $groupmode + * @param bool $onlyactive Whether to get response data for active users only * @return array */ -function choicegroup_get_response_data($choicegroup, $cm) { +function choicegroup_get_response_data($choicegroup, $cm, $groupmode, $onlyactive) { // Initialise the returned array, which is a matrix: $allresponses[responseid][userid] = responseobject. static $allresponses = array(); if (count($allresponses)) { return $allresponses; } - + + // Get the current group. + if ($groupmode > 0) { + $currentgroup = groups_get_activity_group($cm); + } else { + $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', 0, 'u.*', 'u.lastname ASC,u.firstname ASC'); + $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); @@ -1003,7 +1012,9 @@ function choicegroup_extend_settings_navigation(settings_navigation $settings, n print_error('invalidcoursemodule'); return false; } - $allresponses = choicegroup_get_response_data($choicegroup, $PAGE->cm, $groupmode); // Big function, approx 6 SQL calls per user + + // Big function, approx 6 SQL calls per user. + $allresponses = choicegroup_get_response_data($choicegroup, $PAGE->cm, $groupmode, $choicegroup->onlyactive); $responsecount = 0; $respondents = array(); diff --git a/mod_form.php b/mod_form.php index 21f03e7..71c5c85 100644 --- a/mod_form.php +++ b/mod_form.php @@ -121,6 +121,9 @@ function definition() $mform->addElement('selectyesno', 'showunanswered', get_string("showunanswered", "choicegroup")); + $mform->addElement('selectyesno', 'onlyactive', get_string('onlyactive', 'choicegroup')); + $mform->setDefault('onlyactive', 0); + $menuoptions = array(); $menuoptions[0] = get_string('disable'); $menuoptions[1] = get_string('enable'); @@ -328,4 +331,3 @@ function completion_rule_enabled($data) } } - diff --git a/report.php b/report.php index d11e0e4..28d7e3f 100644 --- a/report.php +++ b/report.php @@ -102,7 +102,7 @@ $groups_ids[] = $group->id; } } -$users = choicegroup_get_response_data($choicegroup, $cm, $groupmode); +$users = choicegroup_get_response_data($choicegroup, $cm, $groupmode, $choicegroup->onlyactive); if ($download == "ods" && has_capability('mod/choicegroup:downloadresponses', $context)) { require_once("$CFG->libdir/odslib.class.php"); diff --git a/version.php b/version.php index 6f30c55..9eef7a4 100644 --- a/version.php +++ b/version.php @@ -26,11 +26,10 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2021071400; +$plugin->version = 2021080500; $plugin->requires = 2018051700; // Moodle 3.5 $plugin->maturity = MATURITY_STABLE; -$plugin->release = '1.31 for Moodle 3.5-3.11 (Build: 2021060200)'; +$plugin->release = '1.32 for Moodle 3.5-3.11 (Build: 2021080500)'; $plugin->component = 'mod_choicegroup'; $plugin->cron = 0; - diff --git a/view.php b/view.php index 90db998..7ce905a 100644 --- a/view.php +++ b/view.php @@ -199,7 +199,8 @@ groups_print_activity_menu($cm, $CFG->wwwroot . '/mod/choicegroup/view.php?id='.$id); } -$allresponses = choicegroup_get_response_data($choicegroup, $cm); // Big function, approx 6 SQL calls per user +// 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)) { From 2b2107814d5beba238e0e26d3afe06774ea8398f Mon Sep 17 00:00:00 2001 From: Tony Butler Date: Fri, 7 May 2021 18:23:27 +0100 Subject: [PATCH 2/3] mod_choicegroup: Respect 'onlyactive' option when listing group members --- lib.php | 12 +++++++----- renderer.php | 8 ++++++-- view.php | 4 ++-- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/lib.php b/lib.php index 075f0fe..296b3af 100644 --- a/lib.php +++ b/lib.php @@ -884,7 +884,7 @@ function choicegroup_reset_course_form_defaults($course) { * @uses CONTEXT_MODULE * @param object $choicegroup * @param object $cm - * @param int $groupmode + * @param int $groupmode Group mode * @param bool $onlyactive Whether to get response data for active users only * @return array */ @@ -916,7 +916,7 @@ function choicegroup_get_response_data($choicegroup, $cm, $groupmode, $onlyactiv $allresponses[0] = $users; - $responses = choicegroup_get_responses($choicegroup, $ctx); + $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]; @@ -931,10 +931,12 @@ function choicegroup_get_response_data($choicegroup, $cm, $groupmode, $onlyactiv /* Return an array with the options selected of users of the $choicegroup * * @param object $choicegroup choicegroup record - * @param object $cm course module object + * @param context_module $ctx Context instance + * @param int $currentgroup Current group + * @param bool $onlyactive Whether to get responses for active users only * @return array of selected options by all users */ -function choicegroup_get_responses($choicegroup, $cm){ +function choicegroup_get_responses($choicegroup, $ctx, $currentgroup, $onlyactive) { global $DB; @@ -945,7 +947,7 @@ function choicegroup_get_responses($choicegroup, $cm){ } $params1 = array('choicegroupid'=>$choicegroupid); - list($esql, $params2) = get_enrolled_sql($cm, 'mod/choicegroup:choose', 0); + 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 diff --git a/renderer.php b/renderer.php index a438e7b..7147f9b 100644 --- a/renderer.php +++ b/renderer.php @@ -42,10 +42,11 @@ class mod_choicegroup_renderer extends plugin_renderer_base { * @param bool $choicegroupopen * @param bool $disabled * @param bool $multipleenrollmentspossible + * @param bool $onlyactive * * @return string */ - public function display_options($options, $coursemoduleid, $vertical = true, $publish = false, $limitanswers = false, $showresults = false, $current = false, $choicegroupopen = false, $disabled = false, $multipleenrollmentspossible = false) { + public function display_options($options, $coursemoduleid, $vertical = true, $publish = false, $limitanswers = false, $showresults = false, $current = false, $choicegroupopen = false, $disabled = false, $multipleenrollmentspossible = false, $onlyactive = false) { global $DB, $PAGE, $choicegroup_groups, $choicegroup_users; $target = new moodle_url('/mod/choicegroup/view.php'); @@ -114,10 +115,14 @@ public function display_options($options, $coursemoduleid, $vertical = true, $pu } } + $context = \context_course::instance($group->courseid); $labeltext = html_writer::tag('label', format_string($group->name), array('for' => 'choiceid_' . $option->attributes->value)); $group_members = $DB->get_records('groups_members', array('groupid' => $group->id)); $group_members_names = array(); foreach ($group_members as $group_member) { + if (!is_enrolled($context, $group_member->userid, '', $onlyactive)) { + continue; + } $group_user = (isset($choicegroup_users[$group_member->userid])) ? ($choicegroup_users[$group_member->userid]) : ($DB->get_record('user', array('id' => $group_member->userid))); $group_members_names[] = $group_user->lastname . ', ' . $group_user->firstname; } @@ -127,7 +132,6 @@ public function display_options($options, $coursemoduleid, $vertical = true, $pu $option->attributes->disabled=true; $availableoption--; } - $context = \context_course::instance($group->courseid); $labeltext .= html_writer::tag('div', format_text(file_rewrite_pluginfile_urls($group->description, 'pluginfile.php', $context->id, diff --git a/view.php b/view.php index 7ce905a..fa74df8 100644 --- a/view.php +++ b/view.php @@ -251,10 +251,10 @@ 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 - echo $renderer->display_options($options, $cm->id, $choicegroup->display, $choicegroup->publish, $choicegroup->limitanswers, $choicegroup->showresults, $current, $choicegroupopen, false, $choicegroup->multipleenrollmentspossible); + echo $renderer->display_options($options, $cm->id, $choicegroup->display, $choicegroup->publish, $choicegroup->limitanswers, $choicegroup->showresults, $current, $choicegroupopen, false, $choicegroup->multipleenrollmentspossible, $choicegroup->onlyactive); } else { // form can not be updated - echo $renderer->display_options($options, $cm->id, $choicegroup->display, $choicegroup->publish, $choicegroup->limitanswers, $choicegroup->showresults, $current, $choicegroupopen, true, $choicegroup->multipleenrollmentspossible); + echo $renderer->display_options($options, $cm->id, $choicegroup->display, $choicegroup->publish, $choicegroup->limitanswers, $choicegroup->showresults, $current, $choicegroupopen, true, $choicegroup->multipleenrollmentspossible, $choicegroup->onlyactive); } $choicegroupformshown = true; From 509c5824b7ed7fe8028f668f6281eac8cfce59fb Mon Sep 17 00:00:00 2001 From: Tony Butler Date: Mon, 21 Jun 2021 15:13:02 +0100 Subject: [PATCH 3/3] mod_choicegroup: Improve efficiency and use fullnamedisplay setting --- renderer.php | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/renderer.php b/renderer.php index 7147f9b..02c6ee1 100644 --- a/renderer.php +++ b/renderer.php @@ -47,7 +47,7 @@ class mod_choicegroup_renderer extends plugin_renderer_base { * @return string */ public function display_options($options, $coursemoduleid, $vertical = true, $publish = false, $limitanswers = false, $showresults = false, $current = false, $choicegroupopen = false, $disabled = false, $multipleenrollmentspossible = false, $onlyactive = false) { - global $DB, $PAGE, $choicegroup_groups, $choicegroup_users; + global $DB, $PAGE, $choicegroup_groups; $target = new moodle_url('/mod/choicegroup/view.php'); $attributes = array('method'=>'POST', 'action'=>$target, 'class'=> 'tableform'); @@ -117,16 +117,11 @@ public function display_options($options, $coursemoduleid, $vertical = true, $pu $context = \context_course::instance($group->courseid); $labeltext = html_writer::tag('label', format_string($group->name), array('for' => 'choiceid_' . $option->attributes->value)); - $group_members = $DB->get_records('groups_members', array('groupid' => $group->id)); + $group_members = get_enrolled_users($context, '', $group->id, 'u.*', 'u.lastname, u.firstname', 0, 0, $onlyactive); $group_members_names = array(); foreach ($group_members as $group_member) { - if (!is_enrolled($context, $group_member->userid, '', $onlyactive)) { - continue; - } - $group_user = (isset($choicegroup_users[$group_member->userid])) ? ($choicegroup_users[$group_member->userid]) : ($DB->get_record('user', array('id' => $group_member->userid))); - $group_members_names[] = $group_user->lastname . ', ' . $group_user->firstname; + $group_members_names[] = fullname($group_member); } - sort($group_members_names); if (!empty($option->attributes->disabled) || ($limitanswers && sizeof($group_members) >= $option->maxanswers) && empty($option->attributes->checked)) { $labeltext .= ' ' . html_writer::tag('em', get_string('full', 'choicegroup')); $option->attributes->disabled=true;