From ff626f7bb1d16d693f9167371d040765ce6d85cd Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Fri, 29 Sep 2017 11:45:46 +0200 Subject: [PATCH] MDL-58711 mod_assign: Handle groups in get_submission_status --- mod/assign/externallib.php | 20 +++++++++++--- mod/assign/locallib.php | 15 ++++++++--- mod/assign/tests/externallib_test.php | 39 ++++++++++++++++++++------- mod/assign/upgrade.txt | 5 ++++ 4 files changed, 63 insertions(+), 16 deletions(-) diff --git a/mod/assign/externallib.php b/mod/assign/externallib.php index c4059c95f57..7de404c33bd 100644 --- a/mod/assign/externallib.php +++ b/mod/assign/externallib.php @@ -2285,6 +2285,8 @@ class mod_assign_external extends external_api { array( 'assignid' => new external_value(PARAM_INT, 'assignment instance id'), 'userid' => new external_value(PARAM_INT, 'user id (empty for current user)', VALUE_DEFAULT, 0), + 'groupid' => new external_value(PARAM_INT, 'filter by users in group (used for generating the grading summary). + Empty or 0 for all groups information.', VALUE_DEFAULT, 0), ) ); } @@ -2294,11 +2296,12 @@ class mod_assign_external extends external_api { * * @param int $assignid assignment instance id * @param int $userid user id (empty for current user) + * @param int $groupid filter by users in group id (used for generating the grading summary). Use 0 for all groups information. * @return array of warnings and grading, status, feedback and previous attempts information * @since Moodle 3.1 * @throws required_capability_exception */ - public static function get_submission_status($assignid, $userid = 0) { + public static function get_submission_status($assignid, $userid = 0, $groupid = 0) { global $USER; $warnings = array(); @@ -2306,6 +2309,7 @@ class mod_assign_external extends external_api { $params = array( 'assignid' => $assignid, 'userid' => $userid, + 'groupid' => $groupid, ); $params = self::validate_parameters(self::get_submission_status_parameters(), $params); @@ -2325,8 +2329,18 @@ class mod_assign_external extends external_api { $gradingsummary = $lastattempt = $feedback = $previousattempts = null; // Get the renderable since it contais all the info we need. - if ($assign->can_view_grades()) { - $gradingsummary = $assign->get_assign_grading_summary_renderable(); + if (!empty($params['groupid'])) { + $groupid = $params['groupid']; + // Determine is the group is visible to user. + if (!groups_group_visible($groupid, $course, $cm)) { + throw new moodle_exception('notingroup'); + } + } else { + // A null gorups means that following functions will calculate the current group. + $groupid = null; + } + if ($assign->can_view_grades($groupid)) { + $gradingsummary = $assign->get_assign_grading_summary_renderable($groupid); } // Retrieve the rest of the renderable objects. diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php index f528767b66d..01ddc959bda 100644 --- a/mod/assign/locallib.php +++ b/mod/assign/locallib.php @@ -3283,17 +3283,21 @@ class assign { /** * Does this user have view grade or grade permission for this assignment? * + * @param mixed $groupid int|null when is set to a value, use this group instead calculating it * @return bool */ - public function can_view_grades() { + public function can_view_grades($groupid = null) { // Permissions check. if (!has_any_capability(array('mod/assign:viewgrades', 'mod/assign:grade'), $this->context)) { return false; } // Checks for the edge case when user belongs to no groups and groupmode is sep. if ($this->get_course_module()->effectivegroupmode == SEPARATEGROUPS) { + if ($groupid === null) { + $groupid = groups_get_activity_allowed_groups($this->get_course_module()); + } $groupflag = has_capability('moodle/site:accessallgroups', $this->get_context()); - $groupflag = $groupflag || !empty(groups_get_activity_allowed_groups($this->get_course_module())); + $groupflag = $groupflag || !empty($groupid); return (bool)$groupflag; } return true; @@ -5298,16 +5302,19 @@ class assign { /** * Creates an assign_grading_summary renderable. * + * @param mixed $activitygroup int|null the group for calculating the grading summary (if null the function will determine it) * @return assign_grading_summary renderable object */ - public function get_assign_grading_summary_renderable() { + public function get_assign_grading_summary_renderable($activitygroup = null) { $instance = $this->get_instance(); $draft = ASSIGN_SUBMISSION_STATUS_DRAFT; $submitted = ASSIGN_SUBMISSION_STATUS_SUBMITTED; - $activitygroup = groups_get_activity_group($this->get_course_module()); + if ($activitygroup === null) { + $activitygroup = groups_get_activity_group($this->get_course_module()); + } if ($instance->teamsubmission) { $defaultteammembers = $this->get_submission_group_members(0, true); diff --git a/mod/assign/tests/externallib_test.php b/mod/assign/tests/externallib_test.php index e5a0ccf50ac..8f6b82021ce 100644 --- a/mod/assign/tests/externallib_test.php +++ b/mod/assign/tests/externallib_test.php @@ -1829,7 +1829,10 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { require_once($CFG->dirroot . '/mod/assign/tests/base_test.php'); // Create a course and assignment and users. - $course = self::getDataGenerator()->create_course(); + $course = self::getDataGenerator()->create_course(array('groupmode' => SEPARATEGROUPS, 'groupmodeforce' => 1)); + + $group1 = $this->getDataGenerator()->create_group(array('courseid' => $course->id)); + $group2 = $this->getDataGenerator()->create_group(array('courseid' => $course->id)); $generator = $this->getDataGenerator()->get_plugin_generator('mod_assign'); $params = array( @@ -1862,6 +1865,11 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { $teacherrole = $DB->get_record('role', array('shortname' => 'teacher')); $this->getDataGenerator()->enrol_user($teacher->id, $course->id, $teacherrole->id); + $this->getDataGenerator()->create_group_member(array('groupid' => $group1->id, 'userid' => $student1->id)); + $this->getDataGenerator()->create_group_member(array('groupid' => $group1->id, 'userid' => $teacher->id)); + $this->getDataGenerator()->create_group_member(array('groupid' => $group2->id, 'userid' => $student2->id)); + $this->getDataGenerator()->create_group_member(array('groupid' => $group2->id, 'userid' => $teacher->id)); + $this->setUser($student1); // Create a student1 with an online text submission. @@ -1900,7 +1908,7 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { $assign->submit_for_grading($data, $notices); } - return array($assign, $instance, $student1, $student2, $teacher); + return array($assign, $instance, $student1, $student2, $teacher, $group1, $group2); } /** @@ -1909,7 +1917,7 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { public function test_get_submission_status_in_draft_status() { $this->resetAfterTest(true); - list($assign, $instance, $student1, $student2, $teacher) = $this->create_submission_for_testing_status(); + list($assign, $instance, $student1, $student2, $teacher, $g1, $g2) = $this->create_submission_for_testing_status(); $studentsubmission = $assign->get_user_submission($student1->id, true); $result = mod_assign_external::get_submission_status($assign->get_instance()->id); @@ -1965,7 +1973,7 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { public function test_get_submission_status_in_submission_status() { $this->resetAfterTest(true); - list($assign, $instance, $student1, $student2, $teacher) = $this->create_submission_for_testing_status(true); + list($assign, $instance, $student1, $student2, $teacher, $g1, $g2) = $this->create_submission_for_testing_status(true); $result = mod_assign_external::get_submission_status($assign->get_instance()->id); // We expect debugging because of the $PAGE object, this won't happen in a normal WS request. @@ -1995,11 +2003,12 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { public function test_get_submission_status_in_submission_status_for_teacher() { $this->resetAfterTest(true); - list($assign, $instance, $student1, $student2, $teacher) = $this->create_submission_for_testing_status(true); + list($assign, $instance, $student1, $student2, $teacher, $g1, $g2) = $this->create_submission_for_testing_status(true); // Now, as teacher, see the grading summary. $this->setUser($teacher); - $result = mod_assign_external::get_submission_status($assign->get_instance()->id); + // First one group. + $result = mod_assign_external::get_submission_status($assign->get_instance()->id, 0, $g1->id); // We expect debugging because of the $PAGE object, this won't happen in a normal WS request. $this->assertDebuggingCalled(); $result = external_api::clean_returnvalue(mod_assign_external::get_submission_status_returns(), $result); @@ -2009,12 +2018,24 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { $this->assertFalse(isset($result['feedback'])); $this->assertFalse(isset($result['previousattempts'])); - $this->assertEquals(2, $result['gradingsummary']['participantcount']); + $this->assertEquals(1, $result['gradingsummary']['participantcount']); $this->assertEquals(0, $result['gradingsummary']['submissiondraftscount']); $this->assertEquals(1, $result['gradingsummary']['submissionsenabled']); $this->assertEquals(1, $result['gradingsummary']['submissionssubmittedcount']); $this->assertEquals(1, $result['gradingsummary']['submissionsneedgradingcount']); $this->assertFalse($result['gradingsummary']['warnofungroupedusers']); + + // Second group. + $result = mod_assign_external::get_submission_status($assign->get_instance()->id, 0, $g2->id); + $result = external_api::clean_returnvalue(mod_assign_external::get_submission_status_returns(), $result); + $this->assertCount(0, $result['warnings']); + $this->assertEquals(1, $result['gradingsummary']['participantcount']); + + // Should return also 1 participant if we allow the function to auto-select the group. + $result = mod_assign_external::get_submission_status($assign->get_instance()->id); + $result = external_api::clean_returnvalue(mod_assign_external::get_submission_status_returns(), $result); + $this->assertCount(0, $result['warnings']); + $this->assertEquals(1, $result['gradingsummary']['participantcount']); } /** @@ -2025,7 +2046,7 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { $this->resetAfterTest(true); - list($assign, $instance, $student1, $student2, $teacher) = $this->create_submission_for_testing_status(true); + list($assign, $instance, $student1, $student2, $teacher, $g1, $g2) = $this->create_submission_for_testing_status(true); $studentsubmission = $assign->get_user_submission($student1->id, true); $this->setUser($teacher); @@ -2118,7 +2139,7 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { public function test_get_submission_status_access_control() { $this->resetAfterTest(true); - list($assign, $instance, $student1, $student2, $teacher) = $this->create_submission_for_testing_status(); + list($assign, $instance, $student1, $student2, $teacher, $g1, $g2) = $this->create_submission_for_testing_status(); $this->setUser($student2); diff --git a/mod/assign/upgrade.txt b/mod/assign/upgrade.txt index e6cef2135bb..e29849006b3 100644 --- a/mod/assign/upgrade.txt +++ b/mod/assign/upgrade.txt @@ -1,5 +1,10 @@ This files describes API changes in the assign code. +=== 3.5 === +* Functions assign:get_assign_grading_summary_renderable, assign:can_view_submission and mod_assign_external::get_submission_status + now admit an additional group parameter. + This parameter can be used to force those functions to retrieve data only for the given group. + === 3.4 === * assign::add_attempt requires that set_most_recent_team_submission() be called if attempting to use this function with a team submission.