From 7881024ec4df4c888cd0d9fef0ff00493f37c2b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20S=CC=8Ckoda?= Date: Wed, 26 Sep 2012 17:13:42 +0200 Subject: [PATCH] MDL-31437 add group sync option to enrol_cohort This patch includes following changes and new features: * Group sync in enrol_cohort plugin. * Option for editing of role in existing cohort sync instance. * Group memberships are now restored after enrolments. * New enrol method for restore of protected group membership. * New component callback 'restore_group_member' for restore of general plugin group membership. * New component callback 'restore_role_assignment' for restore of general plugin role assignments. * Implemented group membership protection in enrolment UI. * Other minor fixes and cleanup. Notes: * The YUI base enrolment page is not reimplemented yet - see MDL-35618. --- backup/moodle2/restore_course_task.class.php | 3 + backup/moodle2/restore_stepslib.php | 136 +++++++++++---- enrol/cohort/addinstance.php | 71 -------- enrol/cohort/addinstance_form.php | 90 ---------- enrol/cohort/ajax.php | 14 +- enrol/cohort/edit.php | 104 +++++++++++ enrol/cohort/edit_form.php | 127 ++++++++++++++ enrol/cohort/lang/en/enrol_cohort.php | 3 + enrol/cohort/lib.php | 61 ++++++- enrol/cohort/locallib.php | 94 +++++++--- enrol/cohort/tests/sync_test.php | 171 +++++++++++++++++++ enrol/cohort/version.php | 4 +- enrol/locallib.php | 3 + enrol/manual/lib.php | 15 ++ enrol/renderer.php | 2 +- enrol/upgrade.txt | 1 + group/lib.php | 7 + lib/enrollib.php | 25 ++- lib/upgrade.txt | 4 + 19 files changed, 705 insertions(+), 230 deletions(-) delete mode 100644 enrol/cohort/addinstance.php delete mode 100644 enrol/cohort/addinstance_form.php create mode 100644 enrol/cohort/edit.php create mode 100644 enrol/cohort/edit_form.php diff --git a/backup/moodle2/restore_course_task.class.php b/backup/moodle2/restore_course_task.class.php index a43909b8bf3..8a8713a097c 100644 --- a/backup/moodle2/restore_course_task.class.php +++ b/backup/moodle2/restore_course_task.class.php @@ -76,6 +76,9 @@ class restore_course_task extends restore_task { $this->add_step(new restore_enrolments_structure_step('course_enrolments', 'enrolments.xml')); } + // Populate groups, this must be done after enrolments because only enrolled users may be in groups. + $this->add_step(new restore_groups_members_structure_step('create_groups_members', '../groups.xml')); + // Restore course role assignments and overrides (internally will observe the role_assignments setting), // this must be done after all users are enrolled. $this->add_step(new restore_ras_and_caps_structure_step('course_ras_and_caps', 'roles.xml')); diff --git a/backup/moodle2/restore_stepslib.php b/backup/moodle2/restore_stepslib.php index 77f82aea34a..afa6f5bf9d7 100644 --- a/backup/moodle2/restore_stepslib.php +++ b/backup/moodle2/restore_stepslib.php @@ -715,9 +715,6 @@ class restore_groups_structure_step extends restore_structure_step { $paths = array(); // Add paths here $paths[] = new restore_path_element('group', '/groups/group'); - if ($this->get_setting_value('users')) { - $paths[] = new restore_path_element('member', '/groups/group/group_members/group_member'); - } $paths[] = new restore_path_element('grouping', '/groups/groupings/grouping'); $paths[] = new restore_path_element('grouping_group', '/groups/groupings/grouping/grouping_groups/grouping_group'); @@ -769,33 +766,6 @@ class restore_groups_structure_step extends restore_structure_step { $this->set_mapping('group', $oldid, $newitemid, $restorefiles); } - public function process_member($data) { - global $DB; - - $data = (object)$data; // handy - - // get parent group->id - $data->groupid = $this->get_new_parentid('group'); - - // map user newitemid and insert if not member already - if ($data->userid = $this->get_mappingid('user', $data->userid)) { - if (!$DB->record_exists('groups_members', array('groupid' => $data->groupid, 'userid' => $data->userid))) { - // Check the componment, if any, exists - if (!empty($data->component)) { - $dir = get_component_directory($data->component); - if (!$dir || !is_dir($dir)) { - // Component does not exist on restored system; clear - // component and itemid - unset($data->component); - unset($data->itemid); - } - } - - $DB->insert_record('groups_members', $data); - } - } - } - public function process_grouping($data) { global $DB; @@ -866,6 +836,86 @@ class restore_groups_structure_step extends restore_structure_step { } +/** + * Structure step that will create all the needed group memberships + * by loading them from the groups.xml file performing the required matches. + */ +class restore_groups_members_structure_step extends restore_structure_step { + + protected $plugins = null; + + protected function define_structure() { + + $paths = array(); // Add paths here + + if ($this->get_setting_value('users')) { + $paths[] = new restore_path_element('group', '/groups/group'); + $paths[] = new restore_path_element('member', '/groups/group/group_members/group_member'); + } + + return $paths; + } + + public function process_group($data) { + $data = (object)$data; // handy + + // HACK ALERT! + // Not much to do here, this groups mapping should be already done from restore_groups_structure_step. + // Let's fake internal state to make $this->get_new_parentid('group') work. + + $this->set_mapping('group', $data->id, $this->get_mappingid('group', $data->id)); + } + + public function process_member($data) { + global $DB, $CFG; + require_once("$CFG->dirroot/group/lib.php"); + + // NOTE: Always use groups_add_member() because it triggers events and verifies if user is enrolled. + + $data = (object)$data; // handy + + // get parent group->id + $data->groupid = $this->get_new_parentid('group'); + + // map user newitemid and insert if not member already + if ($data->userid = $this->get_mappingid('user', $data->userid)) { + if (!$DB->record_exists('groups_members', array('groupid' => $data->groupid, 'userid' => $data->userid))) { + // Check the component, if any, exists. + if (empty($data->component)) { + groups_add_member($data->groupid, $data->userid); + + } else if ((strpos($data->component, 'enrol_') === 0)) { + // Deal with enrolment groups - ignore the component and just find out the instance via new id, + // it is possible that enrolment was restored using different plugin type. + if (!isset($this->plugins)) { + $this->plugins = enrol_get_plugins(true); + } + if ($enrolid = $this->get_mappingid('enrol', $data->itemid)) { + if ($instance = $DB->get_record('enrol', array('id'=>$enrolid))) { + if (isset($this->plugins[$instance->enrol])) { + $this->plugins[$instance->enrol]->restore_group_member($instance, $data->groupid, $data->userid); + } + } + } + + } else { + $dir = get_component_directory($data->component); + if ($dir and is_dir($dir)) { + if (component_callback($data->component, 'restore_group_member', array($this, $data), true)) { + return; + } + } + // Bad luck, plugin could not restore the data, let's add normal membership. + groups_add_member($data->groupid, $data->userid); + $message = "Restore of '$data->component/$data->itemid' group membership is not supported, using standard group membership instead."; + debugging($message); + $this->log($message, backup::LOG_WARNING); + } + } + } + } +} + /** * Structure step that will create all the needed scales * by loading them from the scales.xml @@ -1475,6 +1525,22 @@ class restore_ras_and_caps_structure_step extends restore_structure_step { } } } + + } else { + $data->roleid = $newroleid; + $data->userid = $newuserid; + $data->contextid = $contextid; + $dir = get_component_directory($data->component); + if ($dir and is_dir($dir)) { + if (component_callback($data->component, 'restore_role_assignment', array($this, $data), true)) { + return; + } + } + // Bad luck, plugin could not restore the data, let's add normal membership. + role_assign($data->roleid, $data->userid, $data->contextid); + $message = "Restore of '$data->component/$data->itemid' role assignments is not supported, using manual role assignments instead."; + debugging($message); + $this->log($message, backup::LOG_WARNING); } } @@ -1594,8 +1660,10 @@ class restore_enrolments_structure_step extends restore_structure_step { } else { if (!enrol_is_enabled($data->enrol) or !isset($this->plugins[$data->enrol])) { - debugging("Enrol plugin data can not be restored because it is not enabled, use migration to manual enrolments"); $this->set_mapping('enrol', $oldid, 0); + $message = "Enrol plugin '$data->enrol' data can not be restored because it is not enabled, use migration to manual enrolments"; + debugging($message); + $this->log($message, backup::LOG_WARNING); return; } if ($task = $this->get_task() and $task->get_target() == backup::TARGET_NEW_COURSE) { @@ -1796,7 +1864,7 @@ class restore_calendarevents_structure_step extends restore_structure_step { } public function process_calendarevents($data) { - global $DB; + global $DB, $SITE; $data = (object)$data; $oldid = $data->id; @@ -3204,10 +3272,10 @@ class restore_create_question_files extends restore_execution_step { */ class restore_process_file_aliases_queue extends restore_execution_step { - /** @var array internal cache for {@link choose_repository() */ + /** @var array internal cache for {@link choose_repository()} */ private $cachereposbyid = array(); - /** @var array internal cache for {@link choose_repository() */ + /** @var array internal cache for {@link choose_repository()} */ private $cachereposbytype = array(); /** diff --git a/enrol/cohort/addinstance.php b/enrol/cohort/addinstance.php deleted file mode 100644 index eacef31c451..00000000000 --- a/enrol/cohort/addinstance.php +++ /dev/null @@ -1,71 +0,0 @@ -. - -/** - * Adds new instance of enrol_cohort to specified course. - * - * @package enrol_cohort - * @copyright 2010 Petr Skoda {@link http://skodak.org} - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -require('../../config.php'); -require_once("$CFG->dirroot/enrol/cohort/addinstance_form.php"); -require_once("$CFG->dirroot/enrol/cohort/locallib.php"); - -$id = required_param('id', PARAM_INT); // Course id. - -$course = $DB->get_record('course', array('id'=>$id), '*', MUST_EXIST); -$context = context_course::instance($course->id, MUST_EXIST); - -require_login($course); -require_capability('moodle/course:enrolconfig', $context); -require_capability('enrol/cohort:config', $context); - -$PAGE->set_url('/enrol/cohort/addinstance.php', array('id'=>$course->id)); -$PAGE->set_pagelayout('admin'); - -navigation_node::override_active_url(new moodle_url('/enrol/instances.php', array('id'=>$course->id))); - -// Try and make the manage instances node on the navigation active. -$courseadmin = $PAGE->settingsnav->get('courseadmin'); -if ($courseadmin && $courseadmin->get('users') && $courseadmin->get('users')->get('manageinstances')) { - $courseadmin->get('users')->get('manageinstances')->make_active(); -} - - -$enrol = enrol_get_plugin('cohort'); -if (!$enrol->get_newinstance_link($course->id)) { - redirect(new moodle_url('/enrol/instances.php', array('id'=>$course->id))); -} - -$mform = new enrol_cohort_addinstance_form(NULL, $course); - -if ($mform->is_cancelled()) { - redirect(new moodle_url('/enrol/instances.php', array('id'=>$course->id))); - -} else if ($data = $mform->get_data()) { - $enrol->add_instance($course, array('customint1'=>$data->cohortid, 'roleid'=>$data->roleid)); - enrol_cohort_sync($course->id); - redirect(new moodle_url('/enrol/instances.php', array('id'=>$course->id))); -} - -$PAGE->set_heading($course->fullname); -$PAGE->set_title(get_string('pluginname', 'enrol_cohort')); - -echo $OUTPUT->header(); -$mform->display(); -echo $OUTPUT->footer(); diff --git a/enrol/cohort/addinstance_form.php b/enrol/cohort/addinstance_form.php deleted file mode 100644 index 9658b4934bd..00000000000 --- a/enrol/cohort/addinstance_form.php +++ /dev/null @@ -1,90 +0,0 @@ -. - -/** - * Adds instance form - * - * @package enrol_cohort - * @copyright 2010 Petr Skoda {@link http://skodak.org} - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - -defined('MOODLE_INTERNAL') || die(); - -require_once("$CFG->libdir/formslib.php"); - -class enrol_cohort_addinstance_form extends moodleform { - - protected $course; - - function definition() { - global $CFG, $DB; - - $mform = $this->_form; - $this->course = $this->_customdata; - $coursecontext = context_course::instance($this->course->id); - - $enrol = enrol_get_plugin('cohort'); - - $cohorts = array('' => get_string('choosedots')); - list($sqlparents, $params) = $DB->get_in_or_equal($coursecontext->get_parent_context_ids()); - $sql = "SELECT id, name, contextid - FROM {cohort} - WHERE contextid $sqlparents - ORDER BY name ASC"; - $rs = $DB->get_recordset_sql($sql, $params); - foreach ($rs as $c) { - $context = context::instance_by_id($c->contextid); - if (!has_capability('moodle/cohort:view', $context)) { - continue; - } - $cohorts[$c->id] = format_string($c->name); - } - $rs->close(); - - $roles = get_assignable_roles($coursecontext); - $roles[0] = get_string('none'); - $roles = array_reverse($roles, true); // Descending default sortorder. - - $mform->addElement('header','general', get_string('pluginname', 'enrol_cohort')); - - $mform->addElement('select', 'cohortid', get_string('cohort', 'cohort'), $cohorts); - $mform->addRule('cohortid', get_string('required'), 'required', null, 'client'); - - $mform->addElement('select', 'roleid', get_string('role'), $roles); - $mform->addRule('roleid', get_string('required'), 'required', null, 'client'); - $mform->setDefault('roleid', $enrol->get_config('roleid')); - - $mform->addElement('hidden', 'id', null); - $mform->setType('id', PARAM_INT); - - $this->add_action_buttons(true, get_string('addinstance', 'enrol')); - - $this->set_data(array('id'=>$this->course->id)); - } - - function validation($data, $files) { - global $DB; - - $errors = parent::validation($data, $files); - - if ($DB->record_exists('enrol', array('roleid'=>$data['roleid'], 'customint1'=>$data['cohortid'], 'courseid'=>$this->course->id, 'enrol'=>'cohort'))) { - $errors['cohortid'] = get_string('instanceexists', 'enrol_cohort'); - } - - return $errors; - } -} diff --git a/enrol/cohort/ajax.php b/enrol/cohort/ajax.php index c831ecd2ef4..6eec6ce726d 100644 --- a/enrol/cohort/ajax.php +++ b/enrol/cohort/ajax.php @@ -49,13 +49,18 @@ require_login($course); require_capability('moodle/course:enrolreview', $context); require_sesskey(); +if (!enrol_is_enabled('cohort')) { + // This should never happen, no need to invent new error strings. + throw new enrol_ajax_exception('errorenrolcohort'); +} + echo $OUTPUT->header(); // Send headers. $manager = new course_enrolment_manager($PAGE, $course); -$outcome = new stdClass; +$outcome = new stdClass(); $outcome->success = true; -$outcome->response = new stdClass; +$outcome->response = new stdClass(); $outcome->error = ''; switch ($action) { @@ -88,18 +93,21 @@ switch ($action) { enrol_cohort_sync($manager->get_course()->id); break; case 'enrolcohortusers': + //TODO: this should be moved to enrol_manual, see MDL-35618. require_capability('enrol/manual:enrol', $context); $roleid = required_param('roleid', PARAM_INT); $cohortid = required_param('cohortid', PARAM_INT); - $result = enrol_cohort_enrol_all_users($manager, $cohortid, $roleid); $roles = $manager->get_assignable_roles(); if (!enrol_cohort_can_view_cohort($cohortid) || !array_key_exists($roleid, $roles)) { throw new enrol_ajax_exception('errorenrolcohort'); } + + $result = enrol_cohort_enrol_all_users($manager, $cohortid, $roleid); if ($result === false) { throw new enrol_ajax_exception('errorenrolcohortusers'); } + $outcome->success = true; $outcome->response->users = $result; $outcome->response->title = get_string('success'); diff --git a/enrol/cohort/edit.php b/enrol/cohort/edit.php new file mode 100644 index 00000000000..ee9721004c1 --- /dev/null +++ b/enrol/cohort/edit.php @@ -0,0 +1,104 @@ +. + +/** + * Adds new instance of enrol_cohort to specified course. + * + * @package enrol_cohort + * @copyright 2010 Petr Skoda {@link http://skodak.org} + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +require('../../config.php'); +require_once("$CFG->dirroot/enrol/cohort/edit_form.php"); +require_once("$CFG->dirroot/enrol/cohort/locallib.php"); +require_once("$CFG->dirroot/group/lib.php"); + +$courseid = required_param('courseid', PARAM_INT); +$instanceid = optional_param('id', 0, PARAM_INT); + +$course = $DB->get_record('course', array('id'=>$courseid), '*', MUST_EXIST); +$context = context_course::instance($course->id, MUST_EXIST); + +require_login($course); +require_capability('moodle/course:enrolconfig', $context); +require_capability('enrol/cohort:config', $context); + +$PAGE->set_url('/enrol/cohort/edit.php', array('courseid'=>$course->id, 'id'=>$instanceid)); +$PAGE->set_pagelayout('admin'); + +$returnurl = new moodle_url('/enrol/instances.php', array('id'=>$course->id)); +if (!enrol_is_enabled('cohort')) { + redirect($returnurl); +} + +$enrol = enrol_get_plugin('cohort'); + +if ($instanceid) { + $instance = $DB->get_record('enrol', array('courseid'=>$course->id, 'enrol'=>'cohort', 'id'=>$instanceid), '*', MUST_EXIST); + +} else { + // No instance yet, we have to add new instance. + if (!$enrol->get_newinstance_link($course->id)) { + redirect($returnurl); + } + navigation_node::override_active_url(new moodle_url('/enrol/instances.php', array('id'=>$course->id))); + $instance = new stdClass(); + $instance->id = null; + $instance->courseid = $course->id; + $instance->enrol = 'cohort'; + $instance->customint1 = ''; // Cohort id. + $instance->customint2 = 0; // Optional group id. +} + +// Try and make the manage instances node on the navigation active. +$courseadmin = $PAGE->settingsnav->get('courseadmin'); +if ($courseadmin && $courseadmin->get('users') && $courseadmin->get('users')->get('manageinstances')) { + $courseadmin->get('users')->get('manageinstances')->make_active(); +} + + +$mform = new enrol_cohort_edit_form(null, array($instance, $enrol, $course)); + +if ($mform->is_cancelled()) { + redirect($returnurl); + +} else if ($data = $mform->get_data()) { + if ($data->id) { + // NOTE: no cohort changes here!!! + if ($data->roleid != $instance->roleid) { + // The sync script can only add roles, for perf reasons it does not modify them. + role_unassign_all(array('contextid'=>$context->id, 'roleid'=>$instance->roleid, 'component'=>'enrol_cohort', 'itemid'=>$instance->id)); + } + $instance->name = $data->name; + $instance->status = $data->status; + $instance->roleid = $data->roleid; + $instance->customint2 = $data->customint2; + $instance->timemodified = time(); + $DB->update_record('enrol', $instance); + } else { + $enrol->add_instance($course, array('name'=>$data->name, 'status'=>$data->status, 'customint1'=>$data->customint1, 'roleid'=>$data->roleid, 'customint2'=>$data->customint2)); + } + enrol_cohort_sync($course->id); + redirect($returnurl); +} + +$PAGE->set_heading($course->fullname); +$PAGE->set_title(get_string('pluginname', 'enrol_cohort')); + +echo $OUTPUT->header(); +$mform->display(); +echo $OUTPUT->footer(); diff --git a/enrol/cohort/edit_form.php b/enrol/cohort/edit_form.php new file mode 100644 index 00000000000..006e79c5998 --- /dev/null +++ b/enrol/cohort/edit_form.php @@ -0,0 +1,127 @@ +. + +/** + * Adds instance form + * + * @package enrol_cohort + * @copyright 2010 Petr Skoda {@link http://skodak.org} + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +require_once("$CFG->libdir/formslib.php"); + +class enrol_cohort_edit_form extends moodleform { + + function definition() { + global $CFG, $DB; + + $mform = $this->_form; + + list($instance, $plugin, $course) = $this->_customdata; + $coursecontext = context_course::instance($course->id); + + $enrol = enrol_get_plugin('cohort'); + + + $groups = array(0 => get_string('none')); + foreach (groups_get_all_groups($course->id) as $group) { + $groups[$group->id] = format_string($group->name, true, array('context'=>$coursecontext)); + } + + $mform->addElement('header','general', get_string('pluginname', 'enrol_cohort')); + + $mform->addElement('text', 'name', get_string('custominstancename', 'enrol')); + + $options = array(ENROL_INSTANCE_ENABLED => get_string('yes'), + ENROL_INSTANCE_DISABLED => get_string('no')); + $mform->addElement('select', 'status', get_string('status', 'enrol_cohort'), $options); + + if ($instance->id) { + if ($cohort = $DB->get_record('cohort', array('id'=>$instance->customint1))) { + $cohorts = array($instance->customint1=>format_string($cohort->name, true, array('context'=>context::instance_by_id($cohort->contextid)))); + } else { + $cohorts = array($instance->customint1=>get_string('error')); + } + $mform->addElement('select', 'customint1', get_string('cohort', 'cohort'), $cohorts); + $mform->setConstant('customint1', $instance->customint1); + $mform->hardFreeze('customint1', $instance->customint1); + + } else { + $cohorts = array('' => get_string('choosedots')); + list($sqlparents, $params) = $DB->get_in_or_equal($coursecontext->get_parent_context_ids()); + $sql = "SELECT id, name, idnumber, contextid + FROM {cohort} + WHERE contextid $sqlparents + ORDER BY name ASC, idnumber ASC"; + $rs = $DB->get_recordset_sql($sql, $params); + foreach ($rs as $c) { + $context = context::instance_by_id($c->contextid); + if (!has_capability('moodle/cohort:view', $context)) { + continue; + } + $cohorts[$c->id] = format_string($c->name); + } + $rs->close(); + $mform->addElement('select', 'customint1', get_string('cohort', 'cohort'), $cohorts); + $mform->addRule('customint1', get_string('required'), 'required', null, 'client'); + } + + $roles = get_assignable_roles($coursecontext); + $roles[0] = get_string('none'); + $roles = array_reverse($roles, true); // Descending default sortorder. + $mform->addElement('select', 'roleid', get_string('assignrole', 'enrol_cohort'), $roles); + $mform->setDefault('roleid', $enrol->get_config('roleid')); + if ($instance->id and !isset($roles[$instance->roleid])) { + if ($role = $DB->get_record('role', array('id'=>$instance->roleid))) { + $roles = role_fix_names($roles, $coursecontext, ROLENAME_ALIAS, true); + $roles[$instance->roleid] = role_get_name($role, $coursecontext); + } else { + $roles[$instance->roleid] = get_string('error'); + } + } + $mform->addElement('select', 'customint2', get_string('addgroup', 'enrol_cohort'), $groups); + + $mform->addElement('hidden', 'courseid', null); + $mform->setType('courseid', PARAM_INT); + + $mform->addElement('hidden', 'id', null); + $mform->setType('id', PARAM_INT); + + if ($instance->id) { + $this->add_action_buttons(true); + } else { + $this->add_action_buttons(true, get_string('addinstance', 'enrol')); + } + + $this->set_data($instance); + } + + function validation($data, $files) { + global $DB; + + $errors = parent::validation($data, $files); + + $params = array('roleid'=>$data['roleid'], 'customint1'=>$data['customint1'], 'courseid'=>$data['courseid'], 'id'=>$data['id']); + if ($DB->record_exists_select('enrol', "roleid = :roleid AND customint1 = :customint1 AND courseid = :courseid AND enrol = 'cohort' AND id <> :id", $params)) { + $errors['roleid'] = get_string('instanceexists', 'enrol_cohort'); + } + + return $errors; + } +} diff --git a/enrol/cohort/lang/en/enrol_cohort.php b/enrol/cohort/lang/en/enrol_cohort.php index b198b76b622..a74a480b926 100644 --- a/enrol/cohort/lang/en/enrol_cohort.php +++ b/enrol/cohort/lang/en/enrol_cohort.php @@ -22,10 +22,13 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ +$string['addgroup'] = 'Add to group'; $string['ajaxmore'] = 'More...'; +$string['assignrole'] = 'Assign role'; $string['cohortsearch'] = 'Search'; $string['cohort:config'] = 'Configure cohort instances'; $string['cohort:unenrol'] = 'Unenrol suspended users'; $string['instanceexists'] = 'Cohort is already synchronised with selected role'; $string['pluginname'] = 'Cohort sync'; $string['pluginname_desc'] = 'Cohort enrolment plugin synchronises cohort members with course participants.'; +$string['status'] = 'Active'; diff --git a/enrol/cohort/lib.php b/enrol/cohort/lib.php index 06219011cc3..56dc5c9b0bf 100644 --- a/enrol/cohort/lib.php +++ b/enrol/cohort/lib.php @@ -45,15 +45,17 @@ class enrol_cohort_plugin extends enrol_plugin { } else if (empty($instance->name)) { $enrol = $this->get_name(); + $cohort = $DB->get_record('cohort', array('id'=>$instance->customint1)); + $cohortname = format_string($cohort->name, true, array('context'=>context::instance_by_id($cohort->contextid))); if ($role = $DB->get_record('role', array('id'=>$instance->roleid))) { $role = role_get_name($role, context_course::instance($instance->courseid, IGNORE_MISSING)); - return get_string('pluginname', 'enrol_'.$enrol) . ' (' . format_string($DB->get_field('cohort', 'name', array('id'=>$instance->customint1))) . ' - ' . $role .')'; + return get_string('pluginname', 'enrol_'.$enrol) . ' (' . $cohortname . ' - ' . $role .')'; } else { - return get_string('pluginname', 'enrol_'.$enrol) . ' (' . format_string($DB->get_field('cohort', 'name', array('id'=>$instance->customint1))) . ')'; + return get_string('pluginname', 'enrol_'.$enrol) . ' (' . $cohortname . ')'; } } else { - return format_string($instance->name); + return format_string($instance->name, true, array('context'=>context_course::instance($instance->courseid))); } } @@ -67,7 +69,7 @@ class enrol_cohort_plugin extends enrol_plugin { return NULL; } // Multiple instances supported - multiple parent courses linked. - return new moodle_url('/enrol/cohort/addinstance.php', array('id'=>$courseid)); + return new moodle_url('/enrol/cohort/edit.php', array('courseid'=>$courseid)); } /** @@ -99,6 +101,29 @@ class enrol_cohort_plugin extends enrol_plugin { return false; } + /** + * Returns edit icons for the page with list of instances. + * @param stdClass $instance + * @return array + */ + public function get_action_icons(stdClass $instance) { + global $OUTPUT; + + if ($instance->enrol !== 'cohort') { + throw new coding_exception('invalid enrol instance!'); + } + $context = context_course::instance($instance->courseid); + + $icons = array(); + + if (has_capability('enrol/cohort:config', $context)) { + $editlink = new moodle_url("/enrol/cohort/edit.php", array('courseid'=>$instance->courseid, 'id'=>$instance->id)); + $icons[] = $OUTPUT->action_icon($editlink, new pix_icon('i/edit', get_string('edit'), 'core', array('class'=>'icon'))); + } + + return $icons; + } + /** * Called for all enabled enrol plugins that returned true from is_cron_required(). * @return void @@ -190,7 +215,7 @@ class enrol_cohort_plugin extends enrol_plugin { return false; } - $cohorturl = new moodle_url('/enrol/cohort/addinstance.php', array('id' => $course->id)); + $cohorturl = new moodle_url('/enrol/cohort/edit.php', array('courseid' => $course->id)); $button = new enrol_user_button($cohorturl, get_string('enrolcohort', 'enrol'), 'get'); $button->class .= ' enrol_cohort_plugin'; @@ -240,6 +265,10 @@ class enrol_cohort_plugin extends enrol_plugin { return; } + if (!empty($data->customint2)) { + $data->customint2 = $step->get_mappingid('group', $data->customint2); + } + if ($data->roleid and $DB->record_exists('cohort', array('id'=>$data->customint1))) { $instance = $DB->get_record('enrol', array('roleid'=>$data->roleid, 'customint1'=>$data->customint1, 'courseid'=>$course->id, 'enrol'=>$this->get_name())); if ($instance) { @@ -296,4 +325,26 @@ class enrol_cohort_plugin extends enrol_plugin { $this->enrol_user($instance, $userid, null, $data->timestart, $data->timeend, ENROL_USER_SUSPENDED); } } + + /** + * Restore user group membership. + * @param stdClass $instance + * @param int $groupid + * @param int $userid + */ + public function restore_group_member($instance, $groupid, $userid) { + // Nothing to do here, the group members are added in $this->restore_group_restored() + return; + } +} + +/** + * Prevent removal of enrol roles. + * @param int $itemid + * @param int $groupid + * @param int $userid + * @return bool + */ +function enrol_cohort_allow_group_member_remove($itemid, $groupid, $userid) { + return false; } diff --git a/enrol/cohort/locallib.php b/enrol/cohort/locallib.php index d26ee18b11a..83dd982e23d 100644 --- a/enrol/cohort/locallib.php +++ b/enrol/cohort/locallib.php @@ -40,7 +40,8 @@ class enrol_cohort_handler { * @return bool */ public static function member_added($ca) { - global $DB; + global $DB, $CFG; + require_once("$CFG->dirroot/group/lib.php"); if (!enrol_is_enabled('cohort')) { return true; @@ -68,6 +69,15 @@ class enrol_cohort_handler { unset($instance->roleexists); // No problem if already enrolled. $plugin->enrol_user($instance, $ca->userid, $instance->roleid, 0, 0, ENROL_USER_ACTIVE); + + // Sync groups. + if ($instance->customint2) { + if (!groups_is_member($instance->customint2, $ca->userid)) { + if ($group = $DB->get_record('groups', array('id'=>$instance->customint2, 'courseid'=>$instance->courseid))) { + groups_add_member($group->id, $ca->userid, 'enrol_cohort', $instance->id); + } + } + } } return true; @@ -147,6 +157,7 @@ class enrol_cohort_handler { */ function enrol_cohort_sync($courseid = NULL, $verbose = false) { global $CFG, $DB; + require_once("$CFG->dirroot/group/lib.php"); // Purge all roles if cohort sync disabled, those can be recreated later here by cron or CLI. if (!enrol_is_enabled('cohort')) { @@ -216,7 +227,7 @@ function enrol_cohort_sync($courseid = NULL, $verbose = false) { } $instance = $instances[$ue->enrolid]; if ($unenrolaction == ENROL_EXT_REMOVED_UNENROL) { - // Temove enrolment together with group membership, grades, preferences, etc. + // Remove enrolment together with group membership, grades, preferences, etc. $plugin->unenrol_user($instance, $ue->userid); if ($verbose) { mtrace(" unenrolling: $ue->userid ==> $instance->courseid via cohort $instance->customint1"); @@ -287,6 +298,48 @@ function enrol_cohort_sync($courseid = NULL, $verbose = false) { $rs->close(); + // Finally sync groups. + $onecourse = $courseid ? "AND e.courseid = :courseid" : ""; + + // Remove invalid. + $sql = "SELECT gm.*, e.courseid, g.name AS groupname + FROM {groups_members} gm + JOIN {groups} g ON (g.id = gm.groupid) + JOIN {enrol} e ON (e.enrol = 'cohort' AND e.courseid = g.courseid $onecourse) + JOIN {user_enrolments} ue ON (ue.userid = gm.userid AND ue.enrolid = e.id) + WHERE gm.component='enrol_cohort' AND gm.itemid = e.id AND g.id <> e.customint2"; + $params = array(); + $params['courseid'] = $courseid; + + $rs = $DB->get_recordset_sql($sql, $params); + foreach($rs as $gm) { + groups_remove_member($gm->groupid, $gm->userid); + if ($verbose) { + mtrace(" removing user from group: $gm->userid ==> $gm->courseid - $gm->groupname"); + } + } + $rs->close(); + + // Add missing. + $sql = "SELECT ue.*, g.id AS groupid, e.courseid, g.name AS groupname + FROM {user_enrolments} ue + JOIN {enrol} e ON (e.id = ue.enrolid AND e.enrol = 'cohort' $onecourse) + JOIN {groups} g ON (g.courseid = e.courseid AND g.id = e.customint2) + LEFT JOIN {groups_members} gm ON (gm.groupid = g.id AND gm.userid = ue.userid) + WHERE gm.id IS NULL"; + $params = array(); + $params['courseid'] = $courseid; + + $rs = $DB->get_recordset_sql($sql, $params); + foreach($rs as $ue) { + groups_add_member($ue->groupid, $ue->userid, 'enrol_cohort', $ue->enrolid); + if ($verbose) { + mtrace(" adding user to group: $ue->userid ==> $ue->courseid - $ue->groupname"); + } + } + $rs->close(); + + if ($verbose) { mtrace('...user enrolment synchronisation finished.'); } @@ -327,8 +380,8 @@ function enrol_cohort_enrol_all_users(course_enrolment_manager $manager, $cohort FROM {cohort_members} com LEFT JOIN ( SELECT * - FROM {user_enrolments} ue - WHERE ue.enrolid = :enrolid + FROM {user_enrolments} ue + WHERE ue.enrolid = :enrolid ) ue ON ue.userid=com.userid WHERE com.cohortid = :cohortid AND ue.id IS NULL"; $params = array('cohortid' => $cohortid, 'enrolid' => $instance->id); @@ -361,10 +414,10 @@ function enrol_cohort_get_cohorts(course_enrolment_manager $manager) { } } list($sqlparents, $params) = $DB->get_in_or_equal(get_parent_contexts($context)); - $sql = "SELECT id, name, contextid + $sql = "SELECT id, name, idnumber, contextid FROM {cohort} WHERE contextid $sqlparents - ORDER BY name ASC"; + ORDER BY name ASC, idnumber ASC"; $rs = $DB->get_recordset_sql($sql, $params); foreach ($rs as $c) { $context = context::instance_by_id($c->contextid); @@ -373,7 +426,7 @@ function enrol_cohort_get_cohorts(course_enrolment_manager $manager) { } $cohorts[$c->id] = array( 'cohortid'=>$c->id, - 'name'=>format_string($c->name), + 'name'=>format_string($c->name, true, array('context'=>context::instance_by_id($c->contextid))), 'users'=>$DB->count_records('cohort_members', array('cohortid'=>$c->id)), 'enrolled'=>in_array($c->id, $enrolled) ); @@ -423,7 +476,7 @@ function enrol_cohort_search_cohorts(course_enrolment_manager $manager, $offset } } - list($sqlparents, $params) = $DB->get_in_or_equal(get_parent_contexts($context)); + list($sqlparents, $params) = $DB->get_in_or_equal($context->get_parent_context_ids()); // Add some additional sensible conditions. $tests = array('contextid ' . $sqlparents); @@ -435,21 +488,20 @@ function enrol_cohort_search_cohorts(course_enrolment_manager $manager, $offset 'idnumber', 'description' ); - $searchparam = '%' . $search . '%'; + $searchparam = '%' . $DB->sql_like_escape($search) . '%'; foreach ($conditions as $key=>$condition) { - $conditions[$key] = $DB->sql_like($condition,"?", false); + $conditions[$key] = $DB->sql_like($condition, "?", false); $params[] = $searchparam; } $tests[] = '(' . implode(' OR ', $conditions) . ')'; } $wherecondition = implode(' AND ', $tests); - $fields = 'SELECT id, name, contextid, description'; - $countfields = 'SELECT COUNT(1)'; - $sql = " FROM {cohort} - WHERE $wherecondition"; - $order = ' ORDER BY name ASC'; - $rs = $DB->get_recordset_sql($fields . $sql . $order, $params, $offset); + $sql = "SELECT id, name, idnumber, contextid, description + FROM {cohort} + WHERE $wherecondition + ORDER BY name ASC, idnumber ASC"; + $rs = $DB->get_recordset_sql($sql, $params, $offset); // Produce the output respecting parameters. foreach ($rs as $c) { @@ -461,15 +513,15 @@ function enrol_cohort_search_cohorts(course_enrolment_manager $manager, $offset continue; } if ($limit === 0) { - // we have reached the required number of items and know that there are more, exit now. + // We have reached the required number of items and know that there are more, exit now. $offset--; break; } $cohorts[$c->id] = array( - 'cohortid'=>$c->id, - 'name'=> shorten_text(format_string($c->name), 35), - 'users'=>$DB->count_records('cohort_members', array('cohortid'=>$c->id)), - 'enrolled'=>in_array($c->id, $enrolled) + 'cohortid' => $c->id, + 'name' => shorten_text(format_string($c->name, true, array('context'=>context::instance_by_id($c->contextid))), 35), + 'users' => $DB->count_records('cohort_members', array('cohortid'=>$c->id)), + 'enrolled' => in_array($c->id, $enrolled) ); // Count items. $limit--; diff --git a/enrol/cohort/tests/sync_test.php b/enrol/cohort/tests/sync_test.php index b60a4cc4d90..9656e21dbce 100644 --- a/enrol/cohort/tests/sync_test.php +++ b/enrol/cohort/tests/sync_test.php @@ -28,6 +28,7 @@ defined('MOODLE_INTERNAL') || die(); global $CFG; require_once($CFG->dirroot.'/enrol/cohort/locallib.php'); require_once($CFG->dirroot.'/cohort/lib.php'); +require_once($CFG->dirroot.'/group/lib.php'); class enrol_cohort_testcase extends advanced_testcase { @@ -190,6 +191,48 @@ class enrol_cohort_testcase extends advanced_testcase { $this->assertFalse($DB->record_exists('enrol', array('id'=>$cohortinstance1->id))); $this->assertFalse($DB->record_exists('role_assignments', array('component'=>'enrol_cohort', 'itemid'=>$cohortinstance1->id))); + + // Test group sync. + + $id = groups_create_group((object)array('name'=>'Group 1', 'courseid'=>$course1->id)); + $group1 = $DB->get_record('groups', array('id'=>$id), '*', MUST_EXIST); + $id = groups_create_group((object)array('name'=>'Group 2', 'courseid'=>$course1->id)); + $group2 = $DB->get_record('groups', array('id'=>$id), '*', MUST_EXIST); + + $cohort1 = $this->getDataGenerator()->create_cohort(array('contextid'=>context_coursecat::instance($cat1->id)->id)); + $id = $cohortplugin->add_instance($course1, array('customint1'=>$cohort1->id, 'roleid'=>$studentrole->id, 'customint2'=>$group1->id)); + $cohortinstance1 = $DB->get_record('enrol', array('id'=>$id)); + + $this->assertEquals(4, $DB->count_records('user_enrolments', array())); + $this->assertEquals(2, $DB->count_records('role_assignments', array())); + + $this->assertTrue(is_enrolled(context_course::instance($course1->id), $user4)); + $this->assertTrue(groups_add_member($group1, $user4)); + $this->assertTrue(groups_add_member($group2, $user4)); + + $this->assertFalse(groups_is_member($group1->id, $user1->id)); + cohort_add_member($cohort1->id, $user1->id); + $this->assertTrue(groups_is_member($group1->id, $user1->id)); + $this->assertTrue($DB->record_exists('groups_members', array('groupid'=>$group1->id, 'userid'=>$user1->id, 'component'=>'enrol_cohort', 'itemid'=>$cohortinstance1->id))); + + cohort_add_member($cohort1->id, $user4->id); + $this->assertTrue(groups_is_member($group1->id, $user4->id)); + $this->assertFalse($DB->record_exists('groups_members', array('groupid'=>$group1->id, 'userid'=>$user4->id, 'component'=>'enrol_cohort', 'itemid'=>$cohortinstance1->id))); + + $cohortplugin->set_config('unenrolaction', ENROL_EXT_REMOVED_UNENROL); + + cohort_remove_member($cohort1->id, $user1->id); + $this->assertFalse(groups_is_member($group1->id, $user1->id)); + + cohort_remove_member($cohort1->id, $user4->id); + $this->assertTrue(groups_is_member($group1->id, $user4->id)); + $this->assertTrue(groups_is_member($group2->id, $user4->id)); + + $cohortplugin->set_config('unenrolaction', ENROL_EXT_REMOVED_SUSPENDNOROLES); + cohort_add_member($cohort1->id, $user1->id); + + cohort_remove_member($cohort1->id, $user1->id); + $this->assertTrue(groups_is_member($group1->id, $user1->id)); } public function test_sync_course() { @@ -307,6 +350,65 @@ class enrol_cohort_testcase extends advanced_testcase { enrol_cohort_sync($course1->id, false); $this->assertEquals(3, $DB->count_records('user_enrolments', array())); $this->assertEquals(3, $DB->count_records('role_assignments', array())); + + + // Test group sync. + + $this->disable_plugin(); // No event sync + + $id = groups_create_group((object)array('name'=>'Group 1', 'courseid'=>$course1->id)); + $group1 = $DB->get_record('groups', array('id'=>$id), '*', MUST_EXIST); + $id = groups_create_group((object)array('name'=>'Group 2', 'courseid'=>$course1->id)); + $group2 = $DB->get_record('groups', array('id'=>$id), '*', MUST_EXIST); + + $cohort1 = $this->getDataGenerator()->create_cohort(array('contextid'=>context_coursecat::instance($cat1->id)->id)); + $id = $cohortplugin->add_instance($course1, array('customint1'=>$cohort1->id, 'roleid'=>$studentrole->id, 'customint2'=>$group1->id)); + $cohortinstance1 = $DB->get_record('enrol', array('id'=>$id)); + + $this->assertTrue(is_enrolled(context_course::instance($course1->id), $user4)); + $this->assertTrue(groups_add_member($group1, $user4)); + $this->assertTrue(groups_add_member($group2, $user4)); + + $this->enable_plugin(); // No event sync + + $this->assertEquals(3, $DB->count_records('user_enrolments', array())); + $this->assertEquals(3, $DB->count_records('role_assignments', array())); + + $this->assertFalse(groups_is_member($group1->id, $user1->id)); + cohort_add_member($cohort1->id, $user1->id); + cohort_add_member($cohort1->id, $user4->id); + cohort_add_member($cohort2->id, $user4->id); + + enrol_cohort_sync($course1->id, false); + + $this->assertEquals(7, $DB->count_records('user_enrolments', array())); + $this->assertEquals(7, $DB->count_records('role_assignments', array())); + + $this->assertTrue(groups_is_member($group1->id, $user1->id)); + $this->assertTrue($DB->record_exists('groups_members', array('groupid'=>$group1->id, 'userid'=>$user1->id, 'component'=>'enrol_cohort', 'itemid'=>$cohortinstance1->id))); + + $this->assertTrue(groups_is_member($group1->id, $user4->id)); + $this->assertFalse($DB->record_exists('groups_members', array('groupid'=>$group1->id, 'userid'=>$user4->id, 'component'=>'enrol_cohort', 'itemid'=>$cohortinstance1->id))); + + $cohortinstance1->customint2 = $group2->id; + $DB->update_record('enrol', $cohortinstance1); + + enrol_cohort_sync($course1->id, false); + $this->assertFalse(groups_is_member($group1->id, $user1->id)); + $this->assertTrue(groups_is_member($group2->id, $user1->id)); + $this->assertTrue($DB->record_exists('groups_members', array('groupid'=>$group2->id, 'userid'=>$user1->id, 'component'=>'enrol_cohort', 'itemid'=>$cohortinstance1->id))); + + $this->assertTrue(groups_is_member($group1->id, $user4->id)); + $this->assertTrue(groups_is_member($group2->id, $user4->id)); + $this->assertFalse($DB->record_exists('groups_members', array('groupid'=>$group1->id, 'userid'=>$user4->id, 'component'=>'enrol_cohort', 'itemid'=>$cohortinstance1->id))); + $this->assertFalse($DB->record_exists('groups_members', array('groupid'=>$group2->id, 'userid'=>$user4->id, 'component'=>'enrol_cohort', 'itemid'=>$cohortinstance1->id))); + + cohort_remove_member($cohort1->id, $user1->id); + $this->assertFalse(groups_is_member($group1->id, $user1->id)); + + cohort_remove_member($cohort1->id, $user4->id); + $this->assertTrue(groups_is_member($group1->id, $user4->id)); + $this->assertTrue(groups_is_member($group2->id, $user4->id)); } public function test_sync_all_courses() { @@ -420,5 +522,74 @@ class enrol_cohort_testcase extends advanced_testcase { $this->assertEquals(3, $DB->count_records('user_enrolments', array())); $this->assertEquals(3, $DB->count_records('role_assignments', array())); + + // Test group sync. + + $this->disable_plugin(); // No event sync + + $id = groups_create_group((object)array('name'=>'Group 1', 'courseid'=>$course1->id)); + $group1 = $DB->get_record('groups', array('id'=>$id), '*', MUST_EXIST); + $id = groups_create_group((object)array('name'=>'Group 2', 'courseid'=>$course1->id)); + $group2 = $DB->get_record('groups', array('id'=>$id), '*', MUST_EXIST); + $id = groups_create_group((object)array('name'=>'Group 2', 'courseid'=>$course2->id)); + $group3 = $DB->get_record('groups', array('id'=>$id), '*', MUST_EXIST); + + $cohort1 = $this->getDataGenerator()->create_cohort(array('contextid'=>context_coursecat::instance($cat1->id)->id)); + $id = $cohortplugin->add_instance($course1, array('customint1'=>$cohort1->id, 'roleid'=>$studentrole->id, 'customint2'=>$group1->id)); + $cohortinstance1 = $DB->get_record('enrol', array('id'=>$id)); + + $this->assertTrue(groups_add_member($group1, $user4)); + $this->assertTrue(groups_add_member($group2, $user4)); + + $this->assertEquals(3, $DB->count_records('user_enrolments', array())); + $this->assertEquals(3, $DB->count_records('role_assignments', array())); + + $this->assertFalse(groups_is_member($group1->id, $user1->id)); + cohort_add_member($cohort1->id, $user1->id); + cohort_add_member($cohort1->id, $user4->id); + cohort_add_member($cohort2->id, $user4->id); + cohort_add_member($cohort2->id, $user3->id); + + $this->enable_plugin(); + + enrol_cohort_sync(null, false); + + $this->assertEquals(8, $DB->count_records('user_enrolments', array())); + $this->assertEquals(8, $DB->count_records('role_assignments', array())); + + $this->assertTrue(groups_is_member($group1->id, $user1->id)); + $this->assertTrue($DB->record_exists('groups_members', array('groupid'=>$group1->id, 'userid'=>$user1->id, 'component'=>'enrol_cohort', 'itemid'=>$cohortinstance1->id))); + + $this->assertTrue(is_enrolled(context_course::instance($course1->id), $user4)); + $this->assertTrue(groups_is_member($group1->id, $user4->id)); + $this->assertFalse($DB->record_exists('groups_members', array('groupid'=>$group1->id, 'userid'=>$user4->id, 'component'=>'enrol_cohort', 'itemid'=>$cohortinstance1->id))); + + $this->assertTrue(is_enrolled(context_course::instance($course2->id), $user3)); + $this->assertFalse(groups_is_member($group3->id, $user3->id)); + + $cohortinstance1->customint2 = $group2->id; + $DB->update_record('enrol', $cohortinstance1); + $cohortinstance3->customint2 = $group3->id; + $DB->update_record('enrol', $cohortinstance3); + + enrol_cohort_sync(null, false); + $this->assertFalse(groups_is_member($group1->id, $user1->id)); + $this->assertTrue(groups_is_member($group2->id, $user1->id)); + $this->assertTrue($DB->record_exists('groups_members', array('groupid'=>$group2->id, 'userid'=>$user1->id, 'component'=>'enrol_cohort', 'itemid'=>$cohortinstance1->id))); + + $this->assertTrue(groups_is_member($group1->id, $user4->id)); + $this->assertTrue(groups_is_member($group2->id, $user4->id)); + $this->assertFalse($DB->record_exists('groups_members', array('groupid'=>$group1->id, 'userid'=>$user4->id, 'component'=>'enrol_cohort', 'itemid'=>$cohortinstance1->id))); + $this->assertFalse($DB->record_exists('groups_members', array('groupid'=>$group2->id, 'userid'=>$user4->id, 'component'=>'enrol_cohort', 'itemid'=>$cohortinstance1->id))); + + $this->assertTrue(groups_is_member($group3->id, $user3->id)); + $this->assertTrue($DB->record_exists('groups_members', array('groupid'=>$group3->id, 'userid'=>$user3->id, 'component'=>'enrol_cohort', 'itemid'=>$cohortinstance3->id))); + + cohort_remove_member($cohort1->id, $user1->id); + $this->assertFalse(groups_is_member($group1->id, $user1->id)); + + cohort_remove_member($cohort1->id, $user4->id); + $this->assertTrue(groups_is_member($group1->id, $user4->id)); + $this->assertTrue(groups_is_member($group2->id, $user4->id)); } } diff --git a/enrol/cohort/version.php b/enrol/cohort/version.php index f23b2b3f6ce..6e8cf39adc4 100644 --- a/enrol/cohort/version.php +++ b/enrol/cohort/version.php @@ -24,7 +24,7 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2012061700; // The current plugin version (Date: YYYYMMDDXX) -$plugin->requires = 2012061700; // Requires this Moodle version +$plugin->version = 2012092800; // The current plugin version (Date: YYYYMMDDXX) +$plugin->requires = 2012092700; // Requires this Moodle version $plugin->component = 'enrol_cohort'; // Full name of the plugin (used for diagnostics) $plugin->cron = 60*60; // run cron every hour by default, it is not out-of-sync often diff --git a/enrol/locallib.php b/enrol/locallib.php index 596320a7ee6..edd068ab5fe 100644 --- a/enrol/locallib.php +++ b/enrol/locallib.php @@ -621,6 +621,9 @@ class course_enrolment_manager { global $DB; require_capability('moodle/course:managegroups', $this->context); $group = $this->get_group($groupid); + if (!groups_remove_member_allowed($group, $user)) { + return false; + } if (!$group) { return false; } diff --git a/enrol/manual/lib.php b/enrol/manual/lib.php index d55c68c6dfa..7fe0f56b2e8 100644 --- a/enrol/manual/lib.php +++ b/enrol/manual/lib.php @@ -480,4 +480,19 @@ class enrol_manual_plugin extends enrol_plugin { // we do not use component in manual or self enrol. role_assign($roleid, $userid, $contextid, '', 0); } + + /** + * Restore user group membership. + * @param stdClass $instance + * @param int $groupid + * @param int $userid + */ + public function restore_group_member($instance, $groupid, $userid) { + global $CFG; + require_once("$CFG->dirroot/group/lib.php"); + + // This might be called when forcing restore as manual enrolments. + + groups_add_member($groupid, $userid); + } } diff --git a/enrol/renderer.php b/enrol/renderer.php index c7fa8eaa244..d6308845e3f 100644 --- a/enrol/renderer.php +++ b/enrol/renderer.php @@ -235,7 +235,7 @@ class core_enrol_renderer extends plugin_renderer_base { $groupoutput = ''; foreach($groups as $groupid=>$name) { - if ($canmanagegroups) { + if ($canmanagegroups and groups_remove_member_allowed($groupid, $userid)) { $icon = html_writer::empty_tag('img', array('alt'=>get_string('removefromgroup', 'group', $name), 'src'=>$iconenrolremove)); $url = new moodle_url($pageurl, array('action'=>'removemember', 'group'=>$groupid, 'user'=>$userid)); $groupoutput .= html_writer::tag('div', $name . html_writer::link($url, $icon), array('class'=>'group', 'rel'=>$groupid)); diff --git a/enrol/upgrade.txt b/enrol/upgrade.txt index 85dbf7808e6..bf718dd3015 100644 --- a/enrol/upgrade.txt +++ b/enrol/upgrade.txt @@ -9,6 +9,7 @@ required changes in code: directly from database is not correct any more * new restore support: ENROL_RESTORE_EXACT, ENROL_RESTORE_NOUSERS and ENROL_RESTORE_CLASS were removed, implement new restore_* plugin methods instead +* new support for protected group membership linked to enrol instance other changes: * course enrolment manager now works with disabled plugins too diff --git a/group/lib.php b/group/lib.php index e16bc01e46d..9141ff32c44 100644 --- a/group/lib.php +++ b/group/lib.php @@ -44,11 +44,18 @@ function groups_add_member($grouporid, $userorid, $component=null, $itemid=0) { if (is_object($userorid)) { $userid = $userorid->id; $user = $userorid; + if (!isset($user->deleted)) { + $user = $DB->get_record('user', array('id'=>$userid), '*', MUST_EXIST); + } } else { $userid = $userorid; $user = $DB->get_record('user', array('id'=>$userid), '*', MUST_EXIST); } + if ($user->deleted) { + return false; + } + if (is_object($grouporid)) { $groupid = $grouporid->id; $group = $grouporid; diff --git a/lib/enrollib.php b/lib/enrollib.php index ca626060b60..ee7afbd2622 100644 --- a/lib/enrollib.php +++ b/lib/enrollib.php @@ -1339,6 +1339,7 @@ abstract class enrol_plugin { */ public function unenrol_user(stdClass $instance, $userid) { global $CFG, $USER, $DB; + require_once("$CFG->dirroot/group/lib.php"); $name = $this->get_name(); $courseid = $instance->courseid; @@ -1353,6 +1354,13 @@ abstract class enrol_plugin { return; } + // Remove all users groups linked to this enrolment instance. + if ($gms = $DB->get_records('groups_members', array('userid'=>$userid, 'component'=>'enrol_'.$name, 'itemid'=>$instance->id))) { + foreach ($gms as $gm) { + groups_remove_member($gm->groupid, $gm->userid); + } + } + role_unassign_all(array('userid'=>$userid, 'contextid'=>$context->id, 'component'=>'enrol_'.$name, 'itemid'=>$instance->id)); $DB->delete_records('user_enrolments', array('id'=>$ue->id)); @@ -1363,15 +1371,14 @@ abstract class enrol_plugin { $sql = "SELECT 'x' FROM {user_enrolments} ue JOIN {enrol} e ON (e.id = ue.enrolid) - WHERE ue.userid = :userid AND e.courseid = :courseid"; + WHERE ue.userid = :userid AND e.courseid = :courseid"; if ($DB->record_exists_sql($sql, array('userid'=>$userid, 'courseid'=>$courseid))) { $ue->lastenrol = false; events_trigger('user_unenrolled', $ue); // user still has some enrolments, no big cleanup yet + } else { // the big cleanup IS necessary! - - require_once("$CFG->dirroot/group/lib.php"); require_once("$CFG->libdir/gradelib.php"); // remove all remaining roles @@ -1620,6 +1627,7 @@ abstract class enrol_plugin { $participants->close(); // now clean up all remainders that were not removed correctly + $DB->delete_records('groups_members', array('itemid'=>$instance->id, 'component'=>$name)); $DB->delete_records('role_assignments', array('itemid'=>$instance->id, 'component'=>$name)); $DB->delete_records('user_enrolments', array('enrolid'=>$instance->id)); @@ -1808,4 +1816,15 @@ abstract class enrol_plugin { public function restore_role_assignment($instance, $roleid, $userid, $contextid) { // No role assignment by default, override if necessary. } + + /** + * Restore user group membership. + * @param stdClass $instance + * @param int $groupid + * @param int $userid + */ + public function restore_group_member($instance, $groupid, $userid) { + // Implement if you want to restore protected group memberships, + // usually this is not necessary because plugins should be able to recreate the memberships automatically. + } } diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 10c3bb14d18..38a5653f3a6 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -7,6 +7,10 @@ information provided here is intended especially for developers. and page_generic_activity. * use $CFG->googlemapkey3 instead of removed $CFG->googlemapkey and migrate to Google Maps API V3 * Function settings_navigation::add_course_editing_links() is completely removed +* New component and itemid columns in groups_members table - this allows plugin to create protected + group memberships using 'xx_yy_allow_group_member_remove' callback and there is also a new restore + callback 'xx_yy_restore_group_member()'. +* New general role assignment restore plugin callback 'xx_yy_restore_role_assignment()'. YUI changes: * moodle-enrol-notification has been renamed to moodle-core-notification