Skip to content

Commit

Permalink
MDL-31414 mod_assign: Code cleanup for notification improvements
Browse files Browse the repository at this point in the history
* No need to manually call update of providers that happens automatically.
* Added upgrade code for the new sendlatenotifications field.
* Added AMOS syntax for the two renamed strings
* Refactored assign::cron to use half as many DB queries
* Cleaned up unused vars and globals, coding style and phpdocs.
* Removed string notifications added previously but never used.

AMOS BEGIN
   MOV [emailgradermail,mod_assign],[gradersubmissionupdatedtext,mod_assign]
   MOV [emailgradermailhtml,mod_assign],[gradersubmissionupdatedhtml,mod_assign]
AMOS END
  • Loading branch information
Sam Hemelryk committed May 22, 2012
1 parent 75f87a5 commit 3f7b501
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 68 deletions.
2 changes: 1 addition & 1 deletion mod/assign/db/messages.php
Expand Up @@ -24,7 +24,7 @@

$messageproviders = array (

/// Ordinary assignment submissions
// Ordinary assignment submissions
'assign_student_notification' => array (
'capability' => 'mod/assign:submit'
),
Expand Down
18 changes: 17 additions & 1 deletion mod/assign/db/upgrade.php
Expand Up @@ -28,9 +28,25 @@
* @return bool
*/
function xmldb_assign_upgrade($oldversion) {
global $CFG, $DB;

$dbman = $DB->get_manager();

if ($oldversion < 2012051700) {
message_update_providers('mod_assign');

// Define field sendlatenotifications to be added to assign
$table = new xmldb_table('assign');
$field = new xmldb_field('sendlatenotifications', XMLDB_TYPE_INTEGER, '2', null, XMLDB_NOTNULL, null, '0', 'sendnotifications');

// Conditionally launch add field sendlatenotifications
if (!$dbman->field_exists($table, $field)) {
$dbman->add_field($table, $field);
}

// Assign savepoint reached.
upgrade_mod_savepoint(true, 2012051700, 'assign');
}

return true;
}

Expand Down
3 changes: 1 addition & 2 deletions mod/assign/lang/en/assign.php
Expand Up @@ -172,7 +172,6 @@
$string['notsubmittedyet'] = 'Not submitted yet';
$string['notifications'] = 'Notifications';
$string['nousersselected'] = 'No users selected';
$string['notification'] = 'Notification';
$string['numberofdraftsubmissions'] = 'Drafts';
$string['numberofparticipants'] = 'Participants';
$string['numberofsubmittedassignments'] = 'Submitted';
Expand Down Expand Up @@ -262,4 +261,4 @@
$string['viewownsubmissionstatus'] = 'View own submission status page.';
$string['viewsubmissionforuser'] = 'View submission for user: {$a}';
$string['viewsubmission'] = 'View submission';
$string['viewsubmissiongradingtable'] = 'View submission grading table.';
$string['viewsubmissiongradingtable'] = 'View submission grading table.';
145 changes: 86 additions & 59 deletions mod/assign/locallib.php
Expand Up @@ -1071,86 +1071,118 @@ private function pack_files($filesforzipping) {
}

/**
* Cron function to be run periodically according to the moodle cron
* Finds all assignment notifications that have yet to be mailed out, and mails them
* Finds all assignment notifications that have yet to be mailed out, and mails them.
*
* Cron function to be run periodically according to the moodle cron
*
* @return bool
*/
static function cron() {
global $CFG, $DB;

// used to cache DB lookups
$croncache = array();
global $DB;

// only ever send a max of one days worth of updates
$yesterday = time() - (24 * 3600);
$timenow = time();

// email students about new feedback
$submissions = $DB->get_records_sql("SELECT s.*, a.course, a.name, g.*, g.id as gradeid, g.timemodified as lastmodified
FROM {assign} a
JOIN {assign_grades} g ON g.assignment = a.id
LEFT JOIN {assign_submission} s ON s.assignment = a.id AND s.userid = g.userid
WHERE g.timemodified >= :yesterday
AND g.timemodified <= :today
AND g.mailed = 0", array('yesterday'=>$yesterday, 'today'=>$timenow));
// Collect all submissions from the past 24 hours that require mailing.
$sql = "SELECT s.*, a.course, a.name, g.*, g.id as gradeid, g.timemodified as lastmodified
FROM {assign} a
JOIN {assign_grades} g ON g.assignment = a.id
LEFT JOIN {assign_submission} s ON s.assignment = a.id AND s.userid = g.userid
WHERE g.timemodified >= :yesterday AND
g.timemodified <= :today AND
g.mailed = 0";
$params = array('yesterday' => $yesterday, 'today' => $timenow);
$submissions = $DB->get_records_sql($sql, $params);

mtrace('Processing ' . count($submissions) . ' assignment submissions ...');

// Preload courses we are going to need those.
$courseids = array();
foreach ($submissions as $submission) {
$courseids[] = $submission->course;
}
// Filter out duplicates
$courseids = array_unique($courseids);
$ctxselect = context_helper::get_preload_record_columns_sql('ctx');
list($courseidsql, $params) = $DB->get_in_or_equal($courseids, SQL_PARAMS_NAMED);
$sql = "SELECT c.*, {$ctxselect}
FROM {course} c
LEFT JOIN {context} ctx ON ctx.instanceid = c.id AND ctx.contextlevel = :contextlevel
WHERE c.id {$courseidsql}";
$params['contextlevel'] = CONTEXT_COURSE;
$courses = $DB->get_records_sql($sql, $params);
// Clean up... this could go on for a while.
unset($courseids);
unset($ctxselect);
unset($courseidsql);
unset($params);

// Simple array we'll use for caching modules.
$modcache = array();

// Email students about new feedback
foreach ($submissions as $submission) {

mtrace("Processing assignment submission $submission->id ...");

// do not cache user lookups - could be too many
if (! $user = $DB->get_record("user", array("id"=>$submission->userid))) {
if (!$user = $DB->get_record("user", array("id"=>$submission->userid))) {
mtrace("Could not find user $submission->userid");
continue;
}

// use a cache to prevent the same DB queries happening over and over
if (! $course = array_search('course_' . $submission->course, $croncache)) {
if (! $course = $DB->get_record("course", array("id"=>$submission->course))) {
mtrace("Could not find course $submission->course");
continue;
}
$croncache['course_' . $submission->course] = $course;
if (!array_key_exists($submission->course, $courses)) {
mtrace("Could not find course $submission->course");
continue;
}
$course = $courses[$submission->course];
if (isset($course->ctxid)) {
// Context has not yet been preloaded. Do so now.
context_helper::preload_from_record($course);
}

/// Override the language and timezone of the "current" user, so that
/// mail is customised for the receiver.
// Override the language and timezone of the "current" user, so that
// mail is customised for the receiver.
cron_setup_user($user, $course);

// context lookups are already cached
$coursecontext = context_course::instance($submission->course);
$courseshortname = format_string($course->shortname, true, array('context' => $coursecontext));
$coursecontext = context_course::instance($course->id);
if (!is_enrolled($coursecontext, $user->id)) {
$courseshortname = format_string($course->shortname, true, array('context' => $coursecontext));
mtrace(fullname($user)." not an active participant in " . $courseshortname);
continue;
}

if (! $grader = $DB->get_record("user", array("id"=>$submission->grader))) {
if (!$grader = $DB->get_record("user", array("id"=>$submission->grader))) {
mtrace("Could not find grader $submission->grader");
continue;
}


if (! $mod = array_search('mod_' . $submission->assignment, $croncache)) {
if (!array_key_exists($submission->assignment, $modcache)) {
if (! $mod = get_coursemodule_from_instance("assign", $submission->assignment, $course->id)) {
mtrace("Could not find course module for assignment id $submission->assignment");
continue;
}
$croncache['mod_' . $submission->assignment] = $mod;
$modcache[$submission->assignment] = $mod;
} else {
$mod = $modcache[$submission->assignment];
}

// context lookups are already cached
$contextmodule = context_module::instance($mod->id);

if (! $mod->visible) { /// Hold mail notification for hidden assignments until later
if (!$mod->visible) {
// Hold mail notification for hidden assignments until later
continue;
}

// need to send this to the student
self::send_assignment_notification($grader, $user, 'feedbackavailable', 'assign_student_notification', $submission->lastmodified, $mod, $contextmodule, $course, get_string('modulename', 'assign'), $submission->name);
$messagetype = 'feedbackavailable';
$eventtype = 'assign_student_notification';
$updatetime = $submission->lastmodified;
$modulename = get_string('modulename', 'assign');
self::send_assignment_notification($grader, $user, $messagetype, $eventtype, $updatetime, $mod, $contextmodule, $course, $modulename, $submission->name);

$grade = new stdClass();
$grade->id = $submission->gradeid;
Expand All @@ -1162,8 +1194,10 @@ static function cron() {
mtrace('Done processing ' . count($submissions) . ' assignment submissions');

cron_setup_user();
// free up memory
unset($croncache);

// Free up memory just to be sure
unset($courses);
unset($modcache);

return true;
}
Expand Down Expand Up @@ -2294,7 +2328,7 @@ private function get_graders($userid) {
}

/**
* Format a notification based on it's type
* Format a notification for plain text
*
* @param string $messagetype
* @param stdClass $info
Expand All @@ -2314,10 +2348,16 @@ private static function format_notification_message_text($messagetype, $info, $c
}

/**
* Format a notification based on it's type
* Format a notification for HTML
*
* @param string $messagetype
* @param stdClass $info
* @param stdClass $course
* @param stdClass $context
* @param string $modulename
* @param stdClass $coursemodule
* @param string $assignmentname
* @param stdClass $info
*/
private static function format_notification_message_html($messagetype, $info, $course, $context, $modulename, $coursemodule, $assignmentname) {
global $CFG;
Expand All @@ -2334,8 +2374,6 @@ private static function format_notification_message_html($messagetype, $info, $c
/**
* email someone about something (static so it can be called from cron)
*
* @global stdClass $CFG
* @global moodle_database $DB
* @param stdClass $userfrom
* @param stdClass $userto
* @param string $messagetype
Expand All @@ -2345,16 +2383,13 @@ private static function format_notification_message_html($messagetype, $info, $c
* @param stdClass $context
* @param stdClass $course
* @param string $modulename
* @param string $moduleplural
* @param string $assignmentname
* @return void
*/
public static function send_assignment_notification($userfrom, $userto, $messagetype, $eventtype,
$updatetime, $coursemodule, $context, $course,
$modulename, $assignmentname) {
global $CFG, $DB;

$strnotification = get_string('notification', 'assign');
global $CFG;

$info = new stdClass();
$info->username = fullname($userfrom, true);
Expand Down Expand Up @@ -2383,25 +2418,25 @@ public static function send_assignment_notification($userfrom, $userto, $message
$eventdata->contexturlname = $info->assignment;

message_send($eventdata);

}

/**
* email someone about something
*
* @global stdClass $CFG
* @global moodle_database $DB
* @param stdClass $submission
* @param stdClass $userfrom
* @param stdClass $userto
* @param string $messagetype
* @param string $eventtype
* @param int $updatetime
* @return void
*/
public function send_notification($userfrom, $userto, $messagetype, $eventtype, $updatetime) {
self::send_assignment_notification($userfrom, $userto, $messagetype, $eventtype, $updatetime, $this->get_course_module(), $this->get_context(), $this->get_course(), $this->get_module_name(), $this->get_instance()->name);
}

/**
* email student upon successful submission
* Email student upon successful submission
*
* @global stdClass $CFG
* @global moodle_database $DB
* @param stdClass $submission
* @return void
Expand All @@ -2410,19 +2445,17 @@ private function email_student_submission_receipt(stdClass $submission) {
global $DB;

$adminconfig = $this->get_admin_config();
if (!$adminconfig->submissionreceipts) { // No need to do anything
if (!$adminconfig->submissionreceipts) {
// No need to do anything
return;
}

$user = $DB->get_record('user', array('id'=>$submission->userid), '*', MUST_EXIST);

$this->send_notification($user, $user, 'submissionreceipt', 'assign_student_notification', $submission->timemodified);
}

/**
* email graders upon student submissions
* Email graders upon student submissions
*
* @global stdClass $CFG
* @global moodle_database $DB
* @param stdClass $submission
* @return void
Expand All @@ -2437,13 +2470,7 @@ private function email_graders(stdClass $submission) {
}

$user = $DB->get_record('user', array('id'=>$submission->userid), '*', MUST_EXIST);

if ($teachers = $this->get_graders($user->id)) {

$strassignments = $this->get_module_name_plural();
$strassignment = $this->get_module_name();
$strsubmitted = get_string('submitted', 'assign');

foreach ($teachers as $teacher) {
$this->send_notification($user, $teacher, 'gradersubmissionupdated', 'assign_grader_notification', $submission->timemodified);
}
Expand Down
3 changes: 1 addition & 2 deletions mod/assign/settings.php
Expand Up @@ -54,5 +54,4 @@
new lang_string('configshowrecentsubmissions', 'assign'), 0));
$settings->add(new admin_setting_configcheckbox('assign/submissionreceipts',
get_string('sendsubmissionreceipts', 'mod_assign'), get_string('sendsubmissionreceipts_help', 'mod_assign'), 1));

}
}
6 changes: 3 additions & 3 deletions mod/assign/version.php
Expand Up @@ -24,9 +24,9 @@

defined('MOODLE_INTERNAL') || die();

$module->component = 'mod_assign';
$module->version = 2012051700;
$module->requires = 2012050300; // Requires this Moodle version
$module->component = 'mod_assign'; // Full name of the plugin (used for diagnostics)
$module->version = 2012051700; // The current module version (Date: YYYYMMDDXX)
$module->requires = 2012050300; // Requires this Moodle version
$module->cron = 60;


0 comments on commit 3f7b501

Please sign in to comment.