Skip to content

Commit

Permalink
MDL-64336 assign: Teachers should be able to see all submissions
Browse files Browse the repository at this point in the history 
Before this change a teacher would be able to see users listed if:

* They have an active enrolment and can submit
* They have an an inactive enrolment for a role that can submit

After this change they will additonally be able to see users listed:

* That have an active enrolment and have submitted
* That have an inactive enrolment and have submitted

This means that if an assignment has it's context frozen all users
that have made some form of submission will still be listed.

It will also apply if the submission capability is removed from a
user.

If a user's enrolment is deleted they will not be listed.

The submission and grading counts have also been updated so
they will reflect the new rules.
  • Loading branch information
@NeillM
NeillM committed Apr 21, 2021
1 parent 1f55fff commit 06a6980
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 6 deletions.
3 changes: 2 additions & 1 deletion mod/assign/externallib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2432,7 +2432,8 @@ function($e){
}

// Can edit its own submission?
$lastattempt->caneditowner = $cansubmit && $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;
}
Expand Down
45 changes: 40 additions & 5 deletions mod/assign/locallib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2091,6 +2091,33 @@ private function get_grading_sort_sql() {
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.
Expand All @@ -2113,6 +2140,8 @@ public function list_participants($currentgroup, $idsonly, $tablesort = false) {
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';
Expand Down Expand Up @@ -2159,7 +2188,7 @@ public function list_participants($currentgroup, $idsonly, $tablesort = false) {

$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
Expand Down Expand Up @@ -2217,12 +2246,18 @@ public function get_participant($userid) {
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;
}

/**
Expand Down Expand Up @@ -2311,7 +2346,7 @@ public function count_submissions_need_grading($currentgroup = null) {
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;
Expand Down Expand Up @@ -2426,7 +2461,7 @@ public function count_submissions_with_status($status, $currentgroup = null) {
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;
Expand Down
37 changes: 37 additions & 0 deletions mod/assign/tests/locallib_participants_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,11 @@

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);
Expand Down Expand Up @@ -118,6 +121,40 @@ public function test_list_participants_blind_marking() {
$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;

Expand Down
50 changes: 50 additions & 0 deletions mod/assign/tests/locallib_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1065,6 +1065,56 @@ public function test_get_participant_with_ungraded_submission() {
$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();
Expand Down

0 comments on commit 06a6980

Please sign in to comment.