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.

mod/assign/batchsetmarkingworkflowstateform.php

@@ -49,6 +49,11 @@ public function definition() {
         $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 @@ public function definition() {
 
     }
 
+    /**
+     * 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;
+    }
 }
 

mod/assign/gradeform.php

@@ -77,6 +77,12 @@ public function validation($data, $files) {
         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;

mod/assign/lang/en/assign.php

@@ -348,6 +348,7 @@
 $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}\'
 

mod/assign/locallib.php

@@ -1608,8 +1608,12 @@ public static function cron() {
         $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
@@ -1618,12 +1622,16 @@ public static function cron() {
                  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)) {
@@ -5947,10 +5955,15 @@ public function add_grade_form_elements(MoodleQuickForm $mform, stdClass $data,
         // 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);
         }
@@ -6226,6 +6239,16 @@ protected function process_set_batch_marking_workflow_state() {
 
                 $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.

mod/assign/tests/locallib_test.php

@@ -782,6 +782,44 @@ public function test_cron() {
         $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();