From 70cfc878b9b1d63a8b07f5d4d4255ca5273b7bee Mon Sep 17 00:00:00 2001
From: Jonathon Fowler <jf@jonof.id.au>
Date: Fri, 5 Dec 2014 19:44:49 +1000
Subject: [PATCH] MDL-45324 assign: notify when workflow is Off, or if state is
Released
When marking workflow is enabled, students will be notified only when
the workflow state transitions to 'Released'. Until that happens,
sending of messages will be held and the 'Notify students' grading
form option will be locked.
Additionally, the batch set marking workflow state facility gains
the 'Notify students' option.
Credit to Steve Upton and David Balch for the basis of this patch.
---
.../batchsetmarkingworkflowstateform.php | 24 ++++++++++++
mod/assign/gradeform.php | 6 +++
mod/assign/lang/en/assign.php | 1 +
mod/assign/locallib.php | 35 ++++++++++++++---
mod/assign/tests/locallib_test.php | 38 +++++++++++++++++++
5 files changed, 98 insertions(+), 6 deletions(-)
diff --git a/mod/assign/batchsetmarkingworkflowstateform.php b/mod/assign/batchsetmarkingworkflowstateform.php
index cfa561cef60..91cb7eb3e1f 100644
--- a/mod/assign/batchsetmarkingworkflowstateform.php
+++ b/mod/assign/batchsetmarkingworkflowstateform.php
@@ -49,6 +49,11 @@ class mod_assign_batch_set_marking_workflow_state_form extends moodleform {
$options = $params['markingworkflowstates'];
$mform->addElement('select', 'markingworkflowstate', get_string('markingworkflowstate', 'assign'), $options);
+ // Don't allow notification to be sent until in "Released" state.
+ $mform->addElement('selectyesno', 'sendstudentnotifications', get_string('sendstudentnotifications', 'assign'));
+ $mform->setDefault('sendstudentnotifications', 0);
+ $mform->disabledIf('sendstudentnotifications', 'markingworkflowstate', 'neq', ASSIGN_MARKING_WORKFLOW_STATE_RELEASED);
+
$mform->addElement('hidden', 'id');
$mform->setType('id', PARAM_INT);
$mform->addElement('hidden', 'action', 'setbatchmarkingworkflowstate');
@@ -59,5 +64,24 @@ class mod_assign_batch_set_marking_workflow_state_form extends moodleform {
}
+ /**
+ * Validate the submitted form data.
+ *
+ * @param array $data array of ("fieldname"=>value) of submitted data
+ * @param array $files array of uploaded files "element_name"=>tmp_file_path
+ * @return array of "element_name"=>"error_description" if there are errors
+ */
+ public function validation($data, $files) {
+ $errors = parent::validation($data, $files);
+
+ // As the implementation of this feature exists currently, no user will see a validation
+ // failure from this form, but this check ensures the form won't validate if someone
+ // manipulates the 'sendstudentnotifications' field's disabled attribute client-side.
+ if (!empty($data['sendstudentnotifications']) && $data['markingworkflowstate'] != ASSIGN_MARKING_WORKFLOW_STATE_RELEASED) {
+ $errors['sendstudentnotifications'] = get_string('studentnotificationworkflowstateerror', 'assign');
+ }
+
+ return $errors;
+ }
}
diff --git a/mod/assign/gradeform.php b/mod/assign/gradeform.php
index 751d220b2cb..2aab572bab6 100644
--- a/mod/assign/gradeform.php
+++ b/mod/assign/gradeform.php
@@ -77,6 +77,12 @@ class mod_assign_grade_form extends moodleform {
global $DB;
$errors = parent::validation($data, $files);
$instance = $this->assignment->get_instance();
+
+ if ($instance->markingworkflow && !empty($data['sendstudentnotifications']) &&
+ $data['workflowstate'] != ASSIGN_MARKING_WORKFLOW_STATE_RELEASED) {
+ $errors['sendstudentnotifications'] = get_string('studentnotificationworkflowstateerror', 'assign');
+ }
+
// Advanced grading.
if (!array_key_exists('grade', $data)) {
return $errors;
diff --git a/mod/assign/lang/en/assign.php b/mod/assign/lang/en/assign.php
index 6652b1839fc..da89afce19c 100644
--- a/mod/assign/lang/en/assign.php
+++ b/mod/assign/lang/en/assign.php
@@ -353,6 +353,7 @@ $string['setmarkerallocationforlog'] = 'Set marking allocation : (id={$a->id}, f
$string['settings'] = 'Assignment settings';
$string['showrecentsubmissions'] = 'Show recent submissions';
$string['status'] = 'Status';
+$string['studentnotificationworkflowstateerror'] = 'Marking workflow state must be \'Released\' to notify students.';
$string['submissioncopiedtext'] = 'You have made a copy of your previous
assignment submission for \'{$a->assignment}\'
diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php
index e61707071d0..f36a2d25288 100644
--- a/mod/assign/locallib.php
+++ b/mod/assign/locallib.php
@@ -1643,8 +1643,12 @@ class assign {
$timenow = time();
$lastcron = $DB->get_field('modules', 'lastcron', array('name' => 'assign'));
- // Collect all submissions from the past 24 hours that require mailing.
- // Submissions are excluded if the assignment is hidden in the gradebook.
+ // Collect all submissions that require mailing.
+ // Submissions are included if all are true:
+ // - The assignment is visible in the gradebook.
+ // - No previous notification has been sent.
+ // - If marking workflow is not enabled, the grade was updated in the past 24 hours, or
+ // if marking workflow is enabled, the workflow state is at 'released'.
$sql = "SELECT g.id as gradeid, a.course, a.name, a.blindmarking, a.revealidentities,
g.*, g.timemodified as lastmodified, cm.id as cmid
FROM {assign} a
@@ -1653,12 +1657,16 @@ class assign {
JOIN {course_modules} cm ON cm.course = a.course AND cm.instance = a.id
JOIN {modules} md ON md.id = cm.module AND md.name = 'assign'
JOIN {grade_items} gri ON gri.iteminstance = a.id AND gri.courseid = a.course AND gri.itemmodule = md.name
- WHERE g.timemodified >= :yesterday AND
- g.timemodified <= :today AND
+ WHERE ((a.markingworkflow = 0 AND g.timemodified >= :yesterday AND g.timemodified <= :today) OR
+ (a.markingworkflow = 1 AND uf.workflowstate = :wfreleased)) AND
uf.mailed = 0 AND gri.hidden = 0
ORDER BY a.course, cm.id";
- $params = array('yesterday' => $yesterday, 'today' => $timenow);
+ $params = array(
+ 'yesterday' => $yesterday,
+ 'today' => $timenow,
+ 'wfreleased' => ASSIGN_MARKING_WORKFLOW_STATE_RELEASED,
+ );
$submissions = $DB->get_records_sql($sql, $params);
if (!empty($submissions)) {
@@ -6081,10 +6089,15 @@ class assign {
// Get assignment visibility information for student.
$modinfo = get_fast_modinfo($settings->course, $userid);
$cm = $modinfo->get_cm($this->get_course_module()->id);
- // Don't allow notification to be sent if student can't access assignment.
+
+ // Don't allow notification to be sent if the student can't access the assignment,
+ // or until in "Released" state if using marking workflow.
if (!$cm->uservisible) {
$mform->setDefault('sendstudentnotifications', 0);
$mform->freeze('sendstudentnotifications');
+ } else if ($this->get_instance()->markingworkflow) {
+ $mform->setDefault('sendstudentnotifications', 0);
+ $mform->disabledIf('sendstudentnotifications', 'workflowstate', 'neq', ASSIGN_MARKING_WORKFLOW_STATE_RELEASED);
} else {
$mform->setDefault('sendstudentnotifications', $this->get_instance()->sendstudentnotifications);
}
@@ -6360,6 +6373,16 @@ class assign {
$flags->workflowstate = $state;
+ // Clear the mailed flag if notification is requested, the student hasn't been
+ // notified previously, the student can access the assignment, and the state
+ // is "Released".
+ $modinfo = get_fast_modinfo($this->course, $userid);
+ $cm = $modinfo->get_cm($this->get_course_module()->id);
+ if ($formdata->sendstudentnotifications && $flags->mailed != 1 &&
+ $cm->uservisible && $state == ASSIGN_MARKING_WORKFLOW_STATE_RELEASED) {
+ $flags->mailed = 0;
+ }
+
$gradingdisabled = $this->grading_disabled($userid);
// Will not apply update if user does not have permission to assign this workflow state.
diff --git a/mod/assign/tests/locallib_test.php b/mod/assign/tests/locallib_test.php
index 94d10fdfe54..36945d812f9 100644
--- a/mod/assign/tests/locallib_test.php
+++ b/mod/assign/tests/locallib_test.php
@@ -889,6 +889,44 @@ class mod_assign_locallib_testcase extends mod_assign_base_testcase {
$this->assertEquals($assign->get_instance()->name, $messages[0]->contexturlname);
}
+ /**
+ * Test delivery of grade notifications as controlled by marking workflow.
+ */
+ public function test_markingworkflow_cron() {
+ // First run cron so there are no messages waiting to be sent (from other tests).
+ cron_setup_user();
+ assign::cron();
+
+ // Now create an assignment with marking workflow enabled.
+ $this->setUser($this->editingteachers[0]);
+ $assign = $this->create_instance(array('sendstudentnotifications' => 1, 'markingworkflow' => 1));
+
+ // Simulate adding a grade.
+ $this->setUser($this->teachers[0]);
+ $data = new stdClass();
+ $data->grade = '50.0';
+
+ // This student will not receive notification.
+ $data->workflowstate = ASSIGN_MARKING_WORKFLOW_STATE_READYFORRELEASE;
+ $assign->testable_apply_grade_to_user($data, $this->students[0]->id, 0);
+
+ // This student will receive notification.
+ $data->workflowstate = ASSIGN_MARKING_WORKFLOW_STATE_RELEASED;
+ $assign->testable_apply_grade_to_user($data, $this->students[1]->id, 0);
+
+ // Now run cron and see that one message was sent.
+ $this->preventResetByRollback();
+ $sink = $this->redirectMessages();
+ cron_setup_user();
+ $this->expectOutputRegex('/Done processing 1 assignment submissions/');
+ assign::cron();
+
+ $messages = $sink->get_messages();
+ $this->assertEquals(1, count($messages));
+ $this->assertEquals($messages[0]->useridto, $this->students[1]->id);
+ $this->assertEquals($assign->get_instance()->name, $messages[0]->contexturlname);
+ }
+
public function test_is_graded() {
$this->setUser($this->editingteachers[0]);
$assign = $this->create_instance();