Merge branch 'MDL-64336-m311' of https://github.com/NeillM/moodle into MOODLE_311_STABLE

mod/assign/externallib.php

@@ -2375,7 +2375,8 @@ class mod_assign_external extends external_api {
         }
 
         // Retrieve the rest of the renderable objects.
-        if (has_capability('mod/assign:submit', $assign->get_context(), $user)) {
+        $cansubmit = has_capability('mod/assign:submit', $context, $user, false);
+        if ($cansubmit || $assign->get_user_submission($user->id, false) !== false) {
             $lastattempt = $assign->get_assign_submission_status_renderable($user, true);
         }
 
@@ -2431,7 +2432,8 @@ class mod_assign_external extends external_api {
             }
 
             // Can edit its own submission?
-            $lastattempt->caneditowner = $assign->submissions_open($user->id) && $assign->is_any_submission_plugin_enabled();
+            $lastattempt->caneditowner = $cansubmit && $assign->submissions_open($user->id)
+                && $assign->is_any_submission_plugin_enabled();
 
             $result['lastattempt'] = $lastattempt;
         }

mod/assign/locallib.php

@@ -2091,6 +2091,33 @@ class assign {
         return $result;
     }
 
+    /**
+     * Returns array with sql code and parameters returning all ids of users who have submitted an assignment.
+     *
+     * @param int $group The group that the query is for.
+     * @return array list($sql, $params)
+     */
+    protected function get_submitted_sql($group = 0) {
+        // We need to guarentee unique table names.
+        static $i = 0;
+        $i++;
+        $prefix = 'sa' . $i . '_';
+        $params = [
+            "{$prefix}assignment" => (int) $this->get_instance()->id,
+            "{$prefix}status" => ASSIGN_SUBMISSION_STATUS_NEW,
+        ];
+        $capjoin = get_enrolled_with_capabilities_join($this->context, $prefix, '', $group, $this->show_only_active_users());
+        $params += $capjoin->params;
+        $sql = "SELECT {$prefix}s.userid
+                  FROM {assign_submission} {$prefix}s
+                  JOIN {user} {$prefix}u ON {$prefix}u.id = {$prefix}s.userid
+                  $capjoin->joins
+                 WHERE {$prefix}s.assignment = :{$prefix}assignment
+                   AND {$prefix}s.status <> :{$prefix}status
+                   AND $capjoin->wheres";
+        return array($sql, $params);
+    }
+
     /**
      * Load a list of users enrolled in the current course with the specified permission and group.
      * 0 for no group.
@@ -2098,6 +2125,7 @@ class assign {
      *
      * @param int $currentgroup
      * @param bool $idsonly
+     * @param bool $tablesort
      * @return array List of user records
      */
     public function list_participants($currentgroup, $idsonly, $tablesort = false) {
@@ -2113,6 +2141,8 @@ class assign {
         if (!isset($this->participants[$key])) {
             list($esql, $params) = get_enrolled_sql($this->context, 'mod/assign:submit', $currentgroup,
                     $this->show_only_active_users());
+            list($ssql, $sparams) = $this->get_submitted_sql($currentgroup);
+            $params += $sparams;
 
             $fields = 'u.*';
             $orderby = 'u.lastname, u.firstname, u.id';
@@ -2159,7 +2189,7 @@ class assign {
 
             $sql = "SELECT $fields
                       FROM {user} u
-                      JOIN ($esql) je ON je.id = u.id
+                      JOIN ($esql UNION $ssql) je ON je.id = u.id
                            $additionaljoins
                      WHERE u.deleted = 0
                            $additionalfilters
@@ -2217,12 +2247,18 @@ class assign {
             return null;
         }
 
-        if (!is_enrolled($this->context, $participant, 'mod/assign:submit', $this->show_only_active_users())) {
+        if (!is_enrolled($this->context, $participant, '', $this->show_only_active_users())) {
             return null;
         }
 
         $result = $this->get_submission_info_for_participants(array($participant->id => $participant));
-        return $result[$participant->id];
+
+        $submissioninfo = $result[$participant->id];
+        if (!$submissioninfo->submitted && !has_capability('mod/assign:submit', $this->context, $userid)) {
+            return null;
+        }
+
+        return $submissioninfo;
     }
 
     /**
@@ -2311,7 +2347,7 @@ class assign {
         if ($currentgroup === null) {
             $currentgroup = groups_get_activity_group($this->get_course_module(), true);
         }
-        list($esql, $params) = get_enrolled_sql($this->get_context(), 'mod/assign:submit', $currentgroup, true);
+        list($esql, $params) = get_enrolled_sql($this->get_context(), '', $currentgroup, true);
 
         $params['assignid'] = $this->get_instance()->id;
         $params['submitted'] = ASSIGN_SUBMISSION_STATUS_SUBMITTED;
@@ -2426,7 +2462,7 @@ class assign {
         if ($currentgroup === null) {
             $currentgroup = groups_get_activity_group($this->get_course_module(), true);
         }
-        list($esql, $params) = get_enrolled_sql($this->get_context(), 'mod/assign:submit', $currentgroup, true);
+        list($esql, $params) = get_enrolled_sql($this->get_context(), '', $currentgroup, true);
 
         $params['assignid'] = $this->get_instance()->id;
         $params['assignid2'] = $this->get_instance()->id;
@@ -4868,7 +4904,7 @@ class assign {
         if (has_any_capability(array('mod/assign:viewgrades', 'mod/assign:grade'), $this->context)) {
             return true;
         }
-        if ($userid == $USER->id && has_capability('mod/assign:submit', $this->context)) {
+        if ($userid == $USER->id) {
             return true;
         }
         return false;
@@ -5441,8 +5477,9 @@ class assign {
         $o = '';
 
         if ($this->can_view_submission($user->id)) {
-
+            $cansubmit = has_capability('mod/assign:submit', $this->get_context(), $user, false);
-            if (has_capability('mod/assign:submit', $this->get_context(), $user, false)) {
+            if ($cansubmit || $this->get_user_submission($user->id, false) !== false) {
+                // The user can submit, or has a submission.
                 $submissionstatus = $this->get_assign_submission_status_renderable($user, $showlinks);
                 $o .= $this->get_renderer()->render($submissionstatus);
             }
@@ -5471,6 +5508,10 @@ class assign {
      * @return bool
      */
     protected function show_submit_button($submission = null, $teamsubmission = null, $userid = null) {
+        if (!has_capability('mod/assign:submit', $this->get_context(), $userid, false)) {
+            // The user does not have the capability to submit.
+            return false;
+        }
         if ($teamsubmission) {
             if ($teamsubmission->status === ASSIGN_SUBMISSION_STATUS_SUBMITTED) {
                 // The assignment submission has been completed.

mod/assign/tests/locallib_participants_test.php

@@ -28,8 +28,11 @@ defined('MOODLE_INTERNAL') || die();
 
 global $CFG;
 require_once(__DIR__ . '/../locallib.php');
+require_once($CFG->dirroot . '/mod/assign/tests/generator.php');
 
 class mod_assign_locallib_participants extends advanced_testcase {
+    use mod_assign_test_generator;
+
     public function test_list_participants_blind_marking() {
         global $DB;
         $this->resetAfterTest(true);
@@ -118,6 +121,40 @@ class mod_assign_locallib_participants extends advanced_testcase {
         $this->assertEquals($newkeys, array_keys($table->rawdata));
     }
 
+    /**
+     * Tests that users who have a submission, but can no longer submit are listed.
+     */
+    public function test_list_participants_can_no_longer_submit() {
+        global $DB;
+        $this->resetAfterTest(true);
+        // Create a role that will prevent users submitting.
+        $role = self::getDataGenerator()->create_role();
+        assign_capability('mod/assign:submit', CAP_PROHIBIT, $role, context_system::instance());
+        // Create the test data.
+        $course = self::getDataGenerator()->create_course();
+        $coursecontext = context_course::instance($course->id);
+        $assign = $this->create_instance($course);
+        self::getDataGenerator()->create_and_enrol($course, 'teacher');
+        $student1 = self::getDataGenerator()->create_and_enrol($course, 'student');
+        $student2 = self::getDataGenerator()->create_and_enrol($course, 'student');
+        $cannotsubmit1 = self::getDataGenerator()->create_and_enrol($course, 'student');
+        $cannotsubmit2 = self::getDataGenerator()->create_and_enrol($course, 'student');
+        // Create submissions for some users.
+        $this->add_submission($student1, $assign);
+        $this->submit_for_grading($student1, $assign);
+        $this->add_submission($cannotsubmit1, $assign);
+        $this->submit_for_grading($cannotsubmit1, $assign);
+        // Remove the capability to submit from some users.
+        role_assign($role, $cannotsubmit1->id, $coursecontext);
+        role_assign($role, $cannotsubmit2->id, $coursecontext);
+        // Everything is setup for the test now.
+        $participants = $assign->list_participants(null, true);
+        self::assertCount(3, $participants);
+        self::assertArrayHasKey($student1->id, $participants);
+        self::assertArrayHasKey($student2->id, $participants);
+        self::assertArrayHasKey($cannotsubmit1->id, $participants);
+    }
+
     public function helper_add_submission($assign, $user, $data, $type) {
         global $USER;
 

mod/assign/tests/locallib_test.php

@@ -1065,6 +1065,56 @@ class mod_assign_locallib_testcase extends advanced_testcase {
         $this->assertFalse($participant->grantedextension);
     }
 
+    /**
+     * Tests that if a student with no submission who can no longer submit is not a participant.
+     */
+    public function test_get_participant_with_no_submission_no_capability() {
+        global $DB;
+        $this->resetAfterTest();
+        $course = self::getDataGenerator()->create_course();
+        $coursecontext = context_course::instance($course->id);
+        $assign = $this->create_instance($course);
+        $teacher = self::getDataGenerator()->create_and_enrol($course, 'teacher');
+        $student = self::getDataGenerator()->create_and_enrol($course, 'student');
+
+        // Remove the students capability to submit.
+        $role = $DB->get_field('role', 'id', ['shortname' => 'student']);
+        assign_capability('mod/assign:submit', CAP_PROHIBIT, $role, $coursecontext);
+
+        $participant = $assign->get_participant($student->id);
+
+        self::assertNull($participant);
+    }
+
+    /**
+     * Tests that if a student that has submitted but can no longer submit is a participant.
+     */
+    public function test_get_participant_with_submission_no_capability() {
+        global $DB;
+        $this->resetAfterTest();
+        $course = self::getDataGenerator()->create_course();
+        $coursecontext = context_course::instance($course->id);
+        $assign = $this->create_instance($course);
+        $teacher = self::getDataGenerator()->create_and_enrol($course, 'teacher');
+        $student = self::getDataGenerator()->create_and_enrol($course, 'student');
+
+        // Simulate a submission.
+        $this->add_submission($student, $assign);
+        $this->submit_for_grading($student, $assign);
+
+        // Remove the students capability to submit.
+        $role = $DB->get_field('role', 'id', ['shortname' => 'student']);
+        assign_capability('mod/assign:submit', CAP_PROHIBIT, $role, $coursecontext);
+
+        $participant = $assign->get_participant($student->id);
+
+        self::assertNotNull($participant);
+        self::assertEquals($student->id, $participant->id);
+        self::assertTrue($participant->submitted);
+        self::assertTrue($participant->requiregrading);
+        self::assertFalse($participant->grantedextension);
+    }
+
     public function test_get_participant_with_graded_submission() {
         $this->resetAfterTest();
         $course = $this->getDataGenerator()->create_course();
@@ -2345,6 +2395,25 @@ class mod_assign_locallib_testcase extends advanced_testcase {
         $output = $assign->view_student_summary($student, true);
         $this->assertDoesNotMatchRegularExpression('/Feedback/', $output,
             'Do not show feedback if the grade is hidden in the gradebook');
+
+        // Freeze the context.
+        $this->setAdminUser();
+        $context = $assign->get_context();
+        $CFG->contextlocking = true;
+        $context->set_locked(true);
+
+        // No feedback should be available because the grade is hidden.
+        $this->setUser($student);
+        $output = $assign->view_student_summary($student, true);
+        $this->assertDoesNotMatchRegularExpression('/Feedback/', $output, 'Do not show feedback if the grade is hidden in the gradebook');
+
+        // Show the feedback again - it should still be visible even in a frozen context.
+        $this->setUser($teacher);
+        $gradeitem->set_hidden(0, false);
+
+        $this->setUser($student);
+        $output = $assign->view_student_summary($student, true);
+        $this->assertMatchesRegularExpression('/Feedback/', $output, 'Show feedback if there is a grade');
     }
 
     public function test_show_student_summary_with_feedback() {