From d9669db9b326c056b51aad7544ef57f7d4bfcbdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20S=CC=8Ckoda?= Date: Sun, 26 Aug 2012 15:50:40 +0200 Subject: [PATCH 1/2] MDL-35070 coding style cleanup in enrol_self --- enrol/self/db/access.php | 6 ++-- enrol/self/db/install.php | 4 +-- enrol/self/edit.php | 8 ++--- enrol/self/edit_form.php | 6 ++-- enrol/self/editenrolment.php | 54 ++++++++++++++----------------- enrol/self/editenrolment_form.php | 6 ++-- enrol/self/lang/en/enrol_self.php | 6 ++-- enrol/self/lib.php | 47 +++++++++++++-------------- enrol/self/locallib.php | 14 ++++---- enrol/self/settings.php | 4 +-- enrol/self/unenrolself.php | 8 ++--- enrol/self/version.php | 3 +- 12 files changed, 72 insertions(+), 94 deletions(-) diff --git a/enrol/self/db/access.php b/enrol/self/db/access.php index 314720c2969..9cdd3c3cc91 100644 --- a/enrol/self/db/access.php +++ b/enrol/self/db/access.php @@ -26,6 +26,7 @@ defined('MOODLE_INTERNAL') || die(); $capabilities = array( + /* Add or edit enrol-self instance in course. */ 'enrol/self:config' => array( 'captype' => 'write', @@ -36,6 +37,7 @@ $capabilities = array( ) ), + /* Manage user self-enrolments. */ 'enrol/self:manage' => array( 'captype' => 'write', @@ -46,6 +48,7 @@ $capabilities = array( ) ), + /* Voluntarily unenrol self from course - watch out for data loss. */ 'enrol/self:unenrolself' => array( 'captype' => 'write', 'contextlevel' => CONTEXT_COURSE, @@ -54,6 +57,7 @@ $capabilities = array( ) ), + /* Unenrol anybody from course (including self) - watch out for data loss. */ 'enrol/self:unenrol' => array( 'captype' => 'write', 'contextlevel' => CONTEXT_COURSE, @@ -64,5 +68,3 @@ $capabilities = array( ), ); - - diff --git a/enrol/self/db/install.php b/enrol/self/db/install.php index 82d8c2f1481..320fc08323d 100644 --- a/enrol/self/db/install.php +++ b/enrol/self/db/install.php @@ -1,5 +1,4 @@ get_record('course', array('id'=>$courseid), '*', MUST_EXIST); $context = context_course::instance($course->id, MUST_EXIST); @@ -51,7 +49,7 @@ if ($instanceid) { $instance = $DB->get_record('enrol', array('courseid'=>$course->id, 'enrol'=>'self', 'id'=>$instanceid), '*', MUST_EXIST); } else { require_capability('moodle/course:enrolconfig', $context); - // no instance yet, we have to add new instance + // No instance yet, we have to add new instance. navigation_node::override_active_url(new moodle_url('/enrol/instances.php', array('id'=>$course->id))); $instance = new stdClass(); $instance->id = null; diff --git a/enrol/self/edit_form.php b/enrol/self/edit_form.php index 1e80714e4d9..f094b9b7404 100644 --- a/enrol/self/edit_form.php +++ b/enrol/self/edit_form.php @@ -1,5 +1,4 @@ dirroot/enrol/locallib.php"); // Required for the course enrolment manager -require_once("$CFG->dirroot/enrol/renderer.php"); // Required for the course enrolment users table -require_once("$CFG->dirroot/enrol/self/editenrolment_form.php"); // Forms for this page +require_once("$CFG->dirroot/enrol/locallib.php"); // Required for the course enrolment manager. +require_once("$CFG->dirroot/enrol/renderer.php"); // Required for the course enrolment users table. +require_once("$CFG->dirroot/enrol/self/editenrolment_form.php"); // Forms for this page. -$ueid = required_param('ue', PARAM_INT); // user enrolment id -$filter = optional_param('ifilter', 0, PARAM_INT); // table filter for return url +$ueid = required_param('ue', PARAM_INT); +$filter = optional_param('ifilter', 0, PARAM_INT); // Table filter for return url. -// Get the user enrolment object -$ue = $DB->get_record('user_enrolments', array('id' => $ueid), '*', MUST_EXIST); -// Get the user for whom the enrolment is -$user = $DB->get_record('user', array('id'=>$ue->userid), '*', MUST_EXIST); -// Get the course the enrolment is to -list($ctxsql, $ctxjoin) = context_instance_preload_sql('c.id', CONTEXT_COURSE, 'ctx'); -$sql = "SELECT c.* $ctxsql +// Get the user enrolment object. +$ue = $DB->get_record('user_enrolments', array('id' => $ueid), '*', MUST_EXIST); +// Get the user for whom the enrolment is. +$user = $DB->get_record('user', array('id'=>$ue->userid), '*', MUST_EXIST); +// Get the course the enrolment is to. +$sql = "SELECT c.* FROM {course} c LEFT JOIN {enrol} e ON e.courseid = c.id - $ctxjoin WHERE e.id = :enrolid"; $params = array('enrolid' => $ue->enrolid); $course = $DB->get_record_sql($sql, $params, MUST_EXIST); -context_instance_preload($course); -// Make sure the course isn't the front page +// Make sure the course isn't the front page. if ($course->id == SITEID) { redirect(new moodle_url('/')); } -// Obvioulsy +// Obviously. require_login($course); -// The user must be able to manage self enrolments within the course +// The user must be able to manage self enrolments within the course. require_capability("enrol/self:manage", context_course::instance($course->id, MUST_EXIST)); -// Get the enrolment manager for this course +// Get the enrolment manager for this course. $manager = new course_enrolment_manager($PAGE, $course, $filter); -// Get an enrolment users table object. Doign this will automatically retrieve the the URL params +// Get an enrolment users table object. Doing this will automatically retrieve the the URL params // relating to table the user was viewing before coming here, and allows us to return the user to the // exact page of the users screen they can from. $table = new course_enrolment_users_table($manager, $PAGE); @@ -70,30 +66,28 @@ $table = new course_enrolment_users_table($manager, $PAGE); $usersurl = new moodle_url('/enrol/users.php', array('id' => $course->id)); // The URl to return the user too after this screen. $returnurl = new moodle_url($usersurl, $manager->get_url_params()+$table->get_url_params()); -// The URL of this page +// The URL of this page. $url = new moodle_url('/enrol/self/editenrolment.php', $returnurl->params()); $PAGE->set_url($url); $PAGE->set_pagelayout('admin'); navigation_node::override_active_url($usersurl); -// Gets the compontents of the user enrolment +// Gets the components of the user enrolment. list($instance, $plugin) = $manager->get_user_enrolment_components($ue); -// Check that the user can manage this instance, and that the instance is of the correct type +// Check that the user can manage this instance, and that the instance is of the correct type. if (!$plugin->allow_manage($instance) || $instance->enrol != 'self' || !($plugin instanceof enrol_self_plugin)) { print_error('erroreditenrolment', 'enrol'); } -// Get the self enrolment edit form +// Get the self enrolment edit form. $mform = new enrol_self_user_enrolment_form($url, array('user'=>$user, 'course'=>$course, 'ue'=>$ue)); $mform->set_data($PAGE->url->params()); -// Check the form hasn't been cancelled if ($mform->is_cancelled()) { redirect($returnurl); -} else if ($mform->is_submitted() && $mform->is_validated() && confirm_sesskey()) { - // The forms been submit, validated and the sesskey has been checked ... edit the enrolment. - $data = $mform->get_data(); + +} else if ($data = $mform->get_data()) { if ($manager->edit_enrolment($ue, $data)) { redirect($returnurl); } @@ -110,4 +104,4 @@ $PAGE->navbar->add($fullname); echo $OUTPUT->header(); echo $OUTPUT->heading($fullname); $mform->display(); -echo $OUTPUT->footer(); \ No newline at end of file +echo $OUTPUT->footer(); diff --git a/enrol/self/editenrolment_form.php b/enrol/self/editenrolment_form.php index 014312263e8..24d6525217c 100644 --- a/enrol/self/editenrolment_form.php +++ b/enrol/self/editenrolment_form.php @@ -1,5 +1,4 @@ . /** - * Strings for component 'enrol_self', language 'en', branch 'MOODLE_20_STABLE' + * Strings for component 'enrol_self', language 'en'. * - * @package enrol - * @subpackage self + * @package enrol_self * @copyright 2010 Petr Skoda {@link http://skodak.org} * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ diff --git a/enrol/self/lib.php b/enrol/self/lib.php index 8724aef8c39..fda2414301b 100644 --- a/enrol/self/lib.php +++ b/enrol/self/lib.php @@ -1,5 +1,4 @@ $courseid)); } @@ -180,7 +179,7 @@ class enrol_self_plugin extends enrol_plugin { global $CFG, $OUTPUT, $SESSION, $USER, $DB; if (isguestuser()) { - // can not enrol guest!! + // Can not enrol guest!! return null; } if ($DB->record_exists('user_enrolments', array('userid'=>$USER->id, 'enrolid'=>$instance->id))) { @@ -227,7 +226,7 @@ class enrol_self_plugin extends enrol_plugin { } $this->enrol_user($instance, $USER->id, $instance->roleid, $timestart, $timeend); - add_to_log($instance->courseid, 'course', 'enrol', '../enrol/users.php?id='.$instance->courseid, $instance->courseid); //there should be userid somewhere! + add_to_log($instance->courseid, 'course', 'enrol', '../enrol/users.php?id='.$instance->courseid, $instance->courseid); //TODO: There should be userid somewhere! if ($instance->password and $instance->customint1 and $data->enrolpassword !== $instance->password) { // it must be a group enrolment, let's assign group too @@ -242,7 +241,7 @@ class enrol_self_plugin extends enrol_plugin { } } } - // send welcome + // Send welcome message. if ($instance->customint4) { $this->email_welcome_message($instance, $USER); } @@ -258,7 +257,7 @@ class enrol_self_plugin extends enrol_plugin { /** * Add new instance of enrol plugin with default settings. - * @param object $course + * @param stdClass $course * @return int id of new instance */ public function add_default_instance($course) { @@ -279,10 +278,10 @@ class enrol_self_plugin extends enrol_plugin { } /** - * Send welcome email to specified user + * Send welcome email to specified user. * - * @param object $instance - * @param object $user user record + * @param stdClass $instance + * @param stdClass $user user record * @return void */ protected function email_welcome_message($instance, $user) { @@ -326,12 +325,12 @@ class enrol_self_plugin extends enrol_plugin { $contact = generate_email_supportuser(); } - //directly emailing welcome message rather than using messaging + // Directly emailing welcome message rather than using messaging. email_to_user($user, $contact, $subject, $messagetext, $messagehtml); } /** - * Enrol self cron support + * Enrol self cron support. * @return void */ public function cron() { @@ -345,10 +344,10 @@ class enrol_self_plugin extends enrol_plugin { $now = time(); - //note: the logic of self enrolment guarantees that user logged in at least once (=== u.lastaccess set) - // and that user accessed course at least once too (=== user_lastaccess record exists) + // Note: the logic of self enrolment guarantees that user logged in at least once (=== u.lastaccess set) + // and that user accessed course at least once too (=== user_lastaccess record exists). - // first deal with users that did not log in for a really long time + // First deal with users that did not log in for a really long time. $sql = "SELECT e.*, ue.userid FROM {user_enrolments} ue JOIN {enrol} e ON (e.id = ue.enrolid AND e.enrol = 'self' AND e.customint2 > 0) @@ -363,7 +362,7 @@ class enrol_self_plugin extends enrol_plugin { } $rs->close(); - // now unenrol from course user did not visit for a long time + // Now unenrol from course user did not visit for a long time. $sql = "SELECT e.*, ue.userid FROM {user_enrolments} ue JOIN {enrol} e ON (e.id = ue.enrolid AND e.enrol = 'self' AND e.customint2 > 0) @@ -382,7 +381,7 @@ class enrol_self_plugin extends enrol_plugin { } /** - * Gets an array of the user enrolment actions + * Gets an array of the user enrolment actions. * * @param course_enrolment_manager $manager * @param stdClass $ue A user enrolment object @@ -410,7 +409,7 @@ class enrol_self_plugin extends enrol_plugin { * Indicates API features that the enrol plugin supports. * * @param string $feature - * @return mixed True if yes (some features may use other values) + * @return mixed true if yes (some features may use other values) */ function enrol_self_supports($feature) { switch($feature) { diff --git a/enrol/self/locallib.php b/enrol/self/locallib.php index a189e7d4066..df78a2c1ce0 100644 --- a/enrol/self/locallib.php +++ b/enrol/self/locallib.php @@ -1,5 +1,4 @@ addElement('header', 'selfheader', $heading); if ($instance->customint3 > 0) { - // max enrol limit specified + // Max enrol limit specified. $count = $DB->count_records('user_enrolments', array('enrolid'=>$instance->id)); if ($count >= $instance->customint3) { - // bad luck, no more self enrolments here + // Bad luck, no more self enrolments here. $this->toomany = true; $mform->addElement('static', 'notice', '', get_string('maxenrolledreached', 'enrol_self')); return; @@ -65,7 +63,7 @@ class enrol_self_enrol_form extends moodleform { } if ($instance->password) { - //change the id of self enrolment key input as there can be multiple self enrolment methods + // Change the id of self enrolment key input as there can be multiple self enrolment methods. $mform->addElement('passwordunmask', 'enrolpassword', get_string('password', 'enrol_self'), array('id' => 'enrolpassword_'.$instance->id)); } else { @@ -109,7 +107,7 @@ class enrol_self_enrol_form extends moodleform { } } if (!$found) { - // we can not hint because there are probably multiple passwords + // We can not hint because there are probably multiple passwords. $errors['enrolpassword'] = get_string('passwordinvalid', 'enrol_self'); } diff --git a/enrol/self/settings.php b/enrol/self/settings.php index bab0909616a..c8c2f02fa0d 100644 --- a/enrol/self/settings.php +++ b/enrol/self/settings.php @@ -1,5 +1,4 @@ get_unenrolself_link($instance)) { redirect(new moodle_url('/course/view.php', array('id'=>$course->id))); } @@ -51,7 +49,7 @@ $PAGE->set_title($plugin->get_instance_name($instance)); if ($confirm and confirm_sesskey()) { $plugin->unenrol_user($instance, $USER->id); - add_to_log($course->id, 'course', 'unenrol', '../enrol/users.php?id='.$course->id, $course->id); //there should be userid somewhere! + add_to_log($course->id, 'course', 'unenrol', '../enrol/users.php?id='.$course->id, $course->id); //TODO: there should be userid somewhere! redirect(new moodle_url('/index.php')); } diff --git a/enrol/self/version.php b/enrol/self/version.php index e5a04c83918..1153b05a808 100644 --- a/enrol/self/version.php +++ b/enrol/self/version.php @@ -17,8 +17,7 @@ /** * Self enrolment plugin version specification. * - * @package enrol - * @subpackage self + * @package enrol_self * @copyright 2010 Petr Skoda {@link http://skodak.org} * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ From eef59b125435f674b3ddc6577d223f35dc579211 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20S=CC=8Ckoda?= Date: Mon, 27 Aug 2012 17:20:47 +0200 Subject: [PATCH 2/2] MDL-35070 fix incorrect enrol join --- enrol/self/editenrolment.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/enrol/self/editenrolment.php b/enrol/self/editenrolment.php index c63d7583c0a..e97bc2a9113 100644 --- a/enrol/self/editenrolment.php +++ b/enrol/self/editenrolment.php @@ -40,7 +40,7 @@ $user = $DB->get_record('user', array('id'=>$ue->userid), '*', MUST_EXIST); // Get the course the enrolment is to. $sql = "SELECT c.* FROM {course} c - LEFT JOIN {enrol} e ON e.courseid = c.id + JOIN {enrol} e ON e.courseid = c.id WHERE e.id = :enrolid"; $params = array('enrolid' => $ue->enrolid); $course = $DB->get_record_sql($sql, $params, MUST_EXIST);