Skip to content

Commit

Permalink
code review
Browse files Browse the repository at this point in the history
  • Loading branch information
rdebleu committed Mar 28, 2024
1 parent fc3c317 commit acc63c0
Show file tree
Hide file tree
Showing 13 changed files with 163 additions and 88 deletions.
1 change: 0 additions & 1 deletion classes/enrol_coursecompleted_bulkdelete.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class enrol_coursecompleted_bulkdelete extends enrol_bulk_enrolment_operation {

/**
* Returns the identifier for this bulk operation. This is the key used when the plugin
* returns an array containing all of the bulk operations it supports.
Expand Down
3 changes: 1 addition & 2 deletions classes/enrol_coursecompleted_bulkedit.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class enrol_coursecompleted_bulkedit extends enrol_bulk_enrolment_operation {

/**
* Returns the identifier for this bulk operation. This is the key used when the plugin
* returns an array containing all of the bulk operations it supports.
Expand Down Expand Up @@ -106,7 +105,7 @@ public function process(course_enrolment_manager $manager, array $users, stdClas
$event->trigger();
}
}
list($ueidsql, $params) = $DB->get_in_or_equal($ueids, SQL_PARAMS_NAMED);
[$ueidsql, $params] = $DB->get_in_or_equal($ueids, SQL_PARAMS_NAMED);
if ($properties->status == ENROL_USER_ACTIVE || $properties->status == ENROL_USER_SUSPENDED) {
$updatesql[] = 'status = :status';
$params['status'] = (int)$properties->status;
Expand Down
10 changes: 7 additions & 3 deletions classes/observer.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class enrol_coursecompleted_observer {

/**
* Triggered when user completes a course.
*
Expand All @@ -50,8 +49,13 @@ public static function enroluser(\core\event\course_completed $event) {
if ($enrol->enrolperiod > 0) {
$enrol->enrolenddate = max(time(), $enrol->enrolstartdate) + $enrol->enrolperiod;
}
$plugin->enrol_user($enrol, $event->relateduserid, $enrol->roleid,
$enrol->enrolstartdate, $enrol->enrolenddate);
$plugin->enrol_user(
$enrol,
$event->relateduserid,
$enrol->roleid,
$enrol->enrolstartdate,
$enrol->enrolenddate
);
if ($enrol->customint2 > 0) {
$adhock = new \enrol_coursecompleted\task\send_welcome();
$adhock->set_custom_data(
Expand Down
1 change: 0 additions & 1 deletion classes/task/process_expirations.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class process_expirations extends \core\task\scheduled_task {

/**
* Name for this task.
*
Expand Down
10 changes: 6 additions & 4 deletions classes/task/send_welcome.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class send_welcome extends \core\task\adhoc_task {

/**
* Execute scheduled task
*
Expand All @@ -57,7 +56,7 @@ public function execute() {
$a->profileurl = "$CFG->wwwroot/user/view.php?id=$user->id&course=$data->courseid";
$a->completed = format_string($complcourse, true, ['context' => $context2]);
$custom = $DB->get_field('enrol', 'customtext1', ['id' => $data->enrolid]);
$key = ['{$a->coursename}', '{$a->completed}', '{$a->profileurl}', '{$a->fullname}', '{$a->email}'];
$key = ['{$a->coursename}', '{$a->completed}', '{$a->profileurl}', '{$a->fullname}', '{$a->email}'];
$value = [$a->coursename, $a->completed, $a->profileurl, fullname($user), $user->email];
if ($custom != '') {
$message = str_replace($key, $value, $custom);
Expand All @@ -68,8 +67,11 @@ public function execute() {
$messagehtml = $message;
} else {
// This is most probably the tag/newline soup known as FORMAT_MOODLE.
$messagehtml = format_text($message, FORMAT_MOODLE,
['context' => $context, 'para' => false, 'newlines' => true, 'filter' => true]);
$messagehtml = format_text(
$message,
FORMAT_MOODLE,
['context' => $context, 'para' => false, 'newlines' => true, 'filter' => true]
);
}
$subject = get_string('welcometocourse', 'moodle', $a->coursename);
// Directly emailing welcome message rather than using messaging.
Expand Down
25 changes: 16 additions & 9 deletions lib.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class enrol_coursecompleted_plugin extends enrol_plugin {

/** @var bool singleinstance. */
private $singleinstance = false;

Expand Down Expand Up @@ -138,7 +137,8 @@ public function get_user_enrolment_actions(course_enrolment_manager $manager, $u
new pix_icon('a/search', ''),
get_string('pluginname', 'report_completion'),
new moodle_url('/report/completion/index.php', ['course' => $id]),
['class' => 'originlink', 'rel' => $ue->id]);
['class' => 'originlink', 'rel' => $ue->id]
);
}
}
return $actions;
Expand Down Expand Up @@ -332,16 +332,19 @@ public function edit_instance_form($instance, MoodleQuickForm $mform, $context)
$mform->addHelpButton('customint3', 'group', 'enrol_coursecompleted');
$mform->setDefault('customint3', $this->get_config('keepgroup'));

$mform->addElement('advcheckbox', 'customint2', get_string('categoryemail', 'admin'),
get_string('welcome', 'enrol_coursecompleted'));
$mform->addElement(
'advcheckbox',
'customint2',
get_string('categoryemail', 'admin'),
get_string('welcome', 'enrol_coursecompleted')
);
$mform->addHelpButton('customint2', 'welcome', 'enrol_coursecompleted');
$mform->setDefault('customint2', $this->get_config('welcome'));

$arr = ['cols' => '60', 'rows' => '8'];
$mform->addElement('textarea', 'customtext1', get_string('customwelcome', 'enrol_coursecompleted'), $arr);
$mform->addHelpButton('customtext1', 'customwelcome', 'enrol_coursecompleted');
$mform->disabledIf('customtext1', 'customint2', 'notchecked');

}

/**
Expand Down Expand Up @@ -379,9 +382,11 @@ public function edit_instance_validation($data, $files, $instance, $context) {
if (!empty($data['enrolenddate']) && $data['enrolenddate'] < $data['enrolstartdate']) {
$errors['enrolenddate'] = get_string('enrolenddaterror', 'enrol_paypal');
}
if (empty($data['customint1']) ||
if (
empty($data['customint1']) ||
$data['customint1'] == 1 ||
!$DB->record_exists('course', ['id' => $data['customint1']])) {
!$DB->record_exists('course', ['id' => $data['customint1']])
) {
$errors['customint'] = get_string('error_nonexistingcourse', 'tool_generator');
}
}
Expand Down Expand Up @@ -521,8 +526,10 @@ public static function enrol_past(int $courseid = 0) {
$plugin = \enrol_get_plugin('coursecompleted');
$condition = 'course = ? AND timecompleted > 0';
foreach ($enrols as $enrol) {
if ($DB->record_exists('role', ['id' => $enrol->roleid]) &&
$DB->record_exists('course', ['id' => $enrol->courseid])) {
if (
$DB->record_exists('role', ['id' => $enrol->roleid]) &&
$DB->record_exists('course', ['id' => $enrol->courseid])
) {
if ($enrol->enrolperiod > 0) {
$enrol->enrolenddate = max(time(), $enrol->enrolstartdate) + $enrol->enrolperiod;
}
Expand Down
19 changes: 12 additions & 7 deletions manage.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,13 @@
foreach ($candidates as $candidate) {
$user = \core_user::get_user($candidate);
if (!empty($user) && !$user->deleted) {
$enrol->enrol_user($instance,
$candidate,
$instance->roleid,
$instance->enrolstartdate,
$instance->enrolenddate);
$enrol->enrol_user(
$instance,
$candidate,
$instance->roleid,
$instance->enrolstartdate,
$instance->enrolenddate
);
\enrol_coursecompleted_plugin::keepingroup($instance, $candidate);
mark_user_dirty($candidate);
echo '.';
Expand All @@ -90,8 +92,11 @@
}
}
$link = new \moodle_url($PAGE->url, ['enrolid' => $enrolid, 'action' => 'enrol', 'sesskey' => sesskey()]);
echo $OUTPUT->confirm(implode(', ', $allusers), new \single_button($link, get_string('manual:enrol', 'enrol_manual')),
$cancelurl);
echo $OUTPUT->confirm(
implode(', ', $allusers),
new \single_button($link, get_string('manual:enrol', 'enrol_manual')),
$cancelurl
);
} else {
echo $OUTPUT->box(get_string('nousersfound')) . $br . $OUTPUT->single_button($cancelurl, get_string('cancel'));
}
Expand Down
81 changes: 54 additions & 27 deletions settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,46 +26,73 @@
defined('MOODLE_INTERNAL') || die();

if ($ADMIN->fulltree) {

// General settings.
$settings->add(new admin_setting_heading('enrol_coursecompleted_settings', '',
get_string('pluginname_desc', 'enrol_coursecompleted')));
$settings->add(new admin_setting_heading(
'enrol_coursecompleted_settings',
'',
get_string('pluginname_desc', 'enrol_coursecompleted')
));
// Enrol instance defaults.
$settings->add(new admin_setting_heading('enrol_coursecompleted_defaults',
get_string('enrolinstancedefaults', 'admin'), get_string('enrolinstancedefaults_desc', 'admin')));
$settings->add(new admin_setting_configcheckbox('enrol_coursecompleted/defaultenrol',
get_string('defaultenrol', 'enrol'), get_string('defaultenrol_desc', 'enrol'), 0));
$settings->add(new admin_setting_heading(
'enrol_coursecompleted_defaults',
get_string('enrolinstancedefaults', 'admin'),
get_string('enrolinstancedefaults_desc', 'admin')
));
$settings->add(new admin_setting_configcheckbox(
'enrol_coursecompleted/defaultenrol',
get_string('defaultenrol', 'enrol'),
get_string('defaultenrol_desc', 'enrol'),
0
));

if (!during_initial_install()) {
$options = [
ENROL_EXT_REMOVED_KEEP => get_string('extremovedkeep', 'enrol'),
ENROL_EXT_REMOVED_SUSPENDNOROLES => get_string('extremovedsuspendnoroles', 'enrol'),
ENROL_EXT_REMOVED_UNENROL => get_string('extremovedunenrol', 'enrol'),
];
$settings->add(new admin_setting_configselect('enrol_coursecompleted/expiredaction',
get_string('expiredaction', 'enrol_paypal'),
get_string('expiredaction_help', 'enrol_paypal'),
ENROL_EXT_REMOVED_SUSPENDNOROLES,
$options));
$settings->add(new admin_setting_configselect(
'enrol_coursecompleted/expiredaction',
get_string('expiredaction', 'enrol_paypal'),
get_string('expiredaction_help', 'enrol_paypal'),
ENROL_EXT_REMOVED_SUSPENDNOROLES,
$options
));

$options = get_default_enrol_roles(context_system::instance());
$student = get_archetype_roles('student');
$student = reset($student);
$settings->add(new admin_setting_configselect('enrol_coursecompleted/roleid',
get_string('defaultrole', 'enrol_coursecompleted'),
get_string('defaultrole_desc', 'enrol_coursecompleted'),
$student->id,
$options));
$settings->add(new admin_setting_configselect(
'enrol_coursecompleted/roleid',
get_string('defaultrole', 'enrol_coursecompleted'),
get_string('defaultrole_desc', 'enrol_coursecompleted'),
$student->id,
$options
));
}
$settings->add(new admin_setting_configduration('enrol_coursecompleted/enrolperiod',
get_string('enrolperiod', 'enrol_paypal'),
get_string('enrolperiod_desc', 'enrol_paypal'),
0));
$settings->add(new admin_setting_configcheckbox('enrol_coursecompleted/welcome',
get_string('welcome', 'enrol_coursecompleted'), get_string('welcome_help', 'enrol_coursecompleted'), 1));
$settings->add(new admin_setting_configduration(
'enrol_coursecompleted/enrolperiod',
get_string('enrolperiod', 'enrol_paypal'),
get_string('enrolperiod_desc', 'enrol_paypal'),
0
));
$settings->add(new admin_setting_configcheckbox(
'enrol_coursecompleted/welcome',
get_string('welcome', 'enrol_coursecompleted'),
get_string('welcome_help', 'enrol_coursecompleted'),
1
));

$settings->add(new admin_setting_configcheckbox('enrol_coursecompleted/svglearnpath',
get_string('svglearnpath', 'enrol_coursecompleted'), get_string('svglearnpath_help', 'enrol_coursecompleted'), 1));
$settings->add(new admin_setting_configcheckbox('enrol_coursecompleted/keepgroup',
get_string('keepgroup', 'enrol_coursecompleted'), get_string('keepgroup_help', 'enrol_coursecompleted'), 1));
$settings->add(new admin_setting_configcheckbox(
'enrol_coursecompleted/svglearnpath',
get_string('svglearnpath', 'enrol_coursecompleted'),
get_string('svglearnpath_help', 'enrol_coursecompleted'),
1
));
$settings->add(new admin_setting_configcheckbox(
'enrol_coursecompleted/keepgroup',
get_string('keepgroup', 'enrol_coursecompleted'),
get_string('keepgroup_help', 'enrol_coursecompleted'),
1
));
}
7 changes: 3 additions & 4 deletions tests/bulk_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
* @coversDefaultClass \enrol_coursecompleted
*/
final class bulk_test extends \advanced_testcase {

/**
* Setup to ensure that fixtures are loaded.
*/
Expand Down Expand Up @@ -90,7 +89,7 @@ public function test_bulkdeleted(): void {
$user->enrolments = [$enr];
$this->assertfalse($operation->process($manager, [$user], new stdClass()));
$this->setAdminUser();
$this->assertTrue($operation->process($manager, [$user] , new stdClass()));
$this->assertTrue($operation->process($manager, [$user], new stdClass()));
$this->assertNotEmpty($operation->get_form(null, ['users' => [$user]]));
}

Expand Down Expand Up @@ -131,11 +130,11 @@ public function test_bulkedit(): void {
$properties->timeend = time() + 1000;
$this->assertfalse($operation->process($manager, [$user], $properties));
$this->setAdminUser();
$this->assertTrue($operation->process($manager, [$user] , $properties));
$this->assertTrue($operation->process($manager, [$user], $properties));
$properties->status = 99;
$properties->timestart = null;
$properties->timeend = null;
$this->assertTrue($operation->process($manager, [$user] , $properties));
$this->assertTrue($operation->process($manager, [$user], $properties));
$this->assertNotEmpty($operation->get_form(null, ['users' => [$user]]));
}
}
Loading

0 comments on commit acc63c0

Please sign in to comment.