From 74ddcaaa6fc37741bff18858ddf2f92d4a67587d Mon Sep 17 00:00:00 2001 From: Davo Smith Date: Mon, 17 Jun 2019 15:30:47 +0100 Subject: [PATCH 1/3] MDL-64811 mod_assign: warn about 'Default group' submissions --- mod/assign/lang/en/assign.php | 1 + mod/assign/locallib.php | 11 ++- mod/assign/renderable.php | 8 ++ mod/assign/renderer.php | 4 +- .../tests/behat/submit_without_group.feature | 93 +++++++++++++++++++ 5 files changed, 114 insertions(+), 3 deletions(-) diff --git a/mod/assign/lang/en/assign.php b/mod/assign/lang/en/assign.php index e5033dc1066..a4320069d0f 100644 --- a/mod/assign/lang/en/assign.php +++ b/mod/assign/lang/en/assign.php @@ -576,6 +576,7 @@ $string['timeremaining'] = 'Time remaining'; $string['timeremainingcolon'] = 'Time remaining: {$a}'; $string['togglezoom'] = 'Zoom in/out of region'; $string['ungroupedusers'] = 'The setting \'Require group to make submission\' is enabled and some users are either not a member of any group, or are a member of more than one group, so are unable to make submissions.'; +$string['ungroupedusersoptional'] = 'The setting \'Students submit in groups\' is enabled and some users are either not a member of any group, or are a member of more than one group. Please be aware that these students will submit as members of the \'Default group\'.'; $string['unlocksubmissionforstudent'] = 'Allow submissions for student: (id={$a->id}, fullname={$a->fullname}).'; $string['unlocksubmissions'] = 'Unlock submissions'; $string['unlimitedattempts'] = 'Unlimited'; diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php index bef787c7c01..5d696339acf 100644 --- a/mod/assign/locallib.php +++ b/mod/assign/locallib.php @@ -5532,8 +5532,15 @@ class assign { } if ($instance->teamsubmission) { + $warnofungroupedusers = assign_grading_summary::WARN_GROUPS_NO; $defaultteammembers = $this->get_submission_group_members(0, true); - $warnofungroupedusers = (count($defaultteammembers) > 0 && $instance->preventsubmissionnotingroup); + if (count($defaultteammembers) > 0) { + if ($instance->preventsubmissionnotingroup) { + $warnofungroupedusers = assign_grading_summary::WARN_GROUPS_REQUIRED; + } else { + $warnofungroupedusers = assign_grading_summary::WARN_GROUPS_OPTIONAL; + } + } $summary = new assign_grading_summary($this->count_teams($activitygroup), $instance->submissiondrafts, @@ -5561,7 +5568,7 @@ class assign { $this->get_course_module()->id, $this->count_submissions_need_grading($activitygroup), $instance->teamsubmission, - false, + assign_grading_summary::WARN_GROUPS_NO, $this->can_grade(), $isvisible); } diff --git a/mod/assign/renderable.php b/mod/assign/renderable.php index 604f11cc2a4..27c40438dc2 100644 --- a/mod/assign/renderable.php +++ b/mod/assign/renderable.php @@ -753,6 +753,13 @@ class assign_grading_summary implements renderable { /** @var boolean isvisible - Is the assignment's context module visible to students? */ public $isvisible = true; + /** @var string no warning needed about group submissions */ + const WARN_GROUPS_NO = 'nowarning'; + /** @var string warn about group submissions, as groups are required */ + const WARN_GROUPS_REQUIRED = 'warnrequired'; + /** @var string warn about group submissions, as some will submit as 'Default group' */ + const WARN_GROUPS_OPTIONAL = 'warnoptional'; + /** * constructor * @@ -766,6 +773,7 @@ class assign_grading_summary implements renderable { * @param int $coursemoduleid * @param int $submissionsneedgradingcount * @param bool $teamsubmission + * @param string $warnofungroupedusers * @param bool $cangrade * @param bool $isvisible */ diff --git a/mod/assign/renderer.php b/mod/assign/renderer.php index 32de4ef12b0..1046f9abd3f 100644 --- a/mod/assign/renderer.php +++ b/mod/assign/renderer.php @@ -279,8 +279,10 @@ class mod_assign_renderer extends plugin_renderer_base { // Status. if ($summary->teamsubmission) { - if ($summary->warnofungroupedusers) { + if ($summary->warnofungroupedusers === assign_grading_summary::WARN_GROUPS_REQUIRED) { $o .= $this->output->notification(get_string('ungroupedusers', 'assign')); + } else if ($summary->warnofungroupedusers === assign_grading_summary::WARN_GROUPS_OPTIONAL) { + $o .= $this->output->notification(get_string('ungroupedusersoptional', 'assign')); } $this->add_table_row_tuple($t, get_string('numberofteams', 'assign'), diff --git a/mod/assign/tests/behat/submit_without_group.feature b/mod/assign/tests/behat/submit_without_group.feature index 6830f3d1d66..56c5a9a1573 100644 --- a/mod/assign/tests/behat/submit_without_group.feature +++ b/mod/assign/tests/behat/submit_without_group.feature @@ -129,3 +129,96 @@ Feature: Submit assignment without group And I should see "The setting 'Require group to make submission' is enabled and some users are either not a member of any group, or are a member of more than one group, so are unable to make submissions." And I navigate to "View all submissions" in current page administration And I should see "Member of more than one group, so unable to make submissions." in the "Student 3" "table_row" + + Scenario: All users are in groups, so no warning messages needed. + Given the following "courses" exist: + | fullname | shortname | groupmode | + | Course 1 | C1 | 0 | + And the following "activities" exist: + | activity | course | idnumber | name | intro | assignsubmission_onlinetext_enabled | preventsubmissionnotingroup | teamsubmission | + | assign | C1 | assign1 | Allow default group | Test assignment description | 1 | 0 | 1 | + And the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | Teacher | 1 | teacher1@example.com | + | student1 | Student | 1 | student1@example.com | + | student2 | Student | 2 | student2@example.com | + And the following "groups" exist: + | name | course | idnumber | + | Group 1 | C1 | G1 | + | Group 2 | C1 | G2 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | + | student2 | C1 | student | + And the following "group members" exist: + | user | group | + | student1 | G1 | + | student2 | G2 | + When I log in as "teacher1" + And I am on "Course 1" course homepage + And I follow "Allow default group" + Then I should not see "The setting 'Require group to make submission\' is enabled and some users are either not a member of any group, or are a member of more than one group, so are unable to make submissions." + And I should not see "The setting 'Students submit in groups' is enabled and some users are either not a member of any group, or are a member of more than one group. Please be aware that these students will submit as members of the 'Default group'." + + Scenario: One user is not in a group, so should see a warning about default group submission + Given the following "courses" exist: + | fullname | shortname | groupmode | + | Course 1 | C1 | 0 | + And the following "activities" exist: + | activity | course | idnumber | name | intro | assignsubmission_onlinetext_enabled | preventsubmissionnotingroup | teamsubmission | + | assign | C1 | assign1 | Allow default group | Test assignment description | 1 | 0 | 1 | + And the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | Teacher | 1 | teacher1@example.com | + | student1 | Student | 1 | student1@example.com | + | student2 | Student | 2 | student2@example.com | + And the following "groups" exist: + | name | course | idnumber | + | Group 1 | C1 | G1 | + | Group 2 | C1 | G2 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | + | student2 | C1 | student | + And the following "group members" exist: + | user | group | + | student1 | G1 | + When I log in as "teacher1" + And I am on "Course 1" course homepage + And I follow "Allow default group" + Then I should not see "The setting 'Require group to make submission\' is enabled and some users are either not a member of any group, or are a member of more than one group, so are unable to make submissions." + And I should see "The setting 'Students submit in groups' is enabled and some users are either not a member of any group, or are a member of more than one group. Please be aware that these students will submit as members of the 'Default group'." + + Scenario: One user is a member of multiple groups, so should see a warning about default group submission + Given the following "courses" exist: + | fullname | shortname | groupmode | + | Course 1 | C1 | 0 | + And the following "activities" exist: + | activity | course | idnumber | name | intro | assignsubmission_onlinetext_enabled | preventsubmissionnotingroup | teamsubmission | + | assign | C1 | assign1 | Allow default group | Test assignment description | 1 | 0 | 1 | + And the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | Teacher | 1 | teacher1@example.com | + | student1 | Student | 1 | student1@example.com | + | student2 | Student | 2 | student2@example.com | + And the following "groups" exist: + | name | course | idnumber | + | Group 1 | C1 | G1 | + | Group 2 | C1 | G2 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | + | student2 | C1 | student | + And the following "group members" exist: + | user | group | + | student1 | G1 | + | student2 | G1 | + | student2 | G2 | + When I log in as "teacher1" + And I am on "Course 1" course homepage + And I follow "Allow default group" + Then I should not see "The setting 'Require group to make submission\' is enabled and some users are either not a member of any group, or are a member of more than one group, so are unable to make submissions." + And I should see "The setting 'Students submit in groups' is enabled and some users are either not a member of any group, or are a member of more than one group. Please be aware that these students will submit as members of the 'Default group'." From 8773296b779abca66692845a074149f01bc074ed Mon Sep 17 00:00:00 2001 From: Davo Smith Date: Mon, 1 Jul 2019 11:22:57 +0100 Subject: [PATCH 2/3] MDL-64811 mod_assign: fix webservice return type --- mod/assign/externallib.php | 6 ++++-- mod/assign/renderable.php | 2 +- mod/assign/tests/externallib_test.php | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/mod/assign/externallib.php b/mod/assign/externallib.php index 7bb0361c2cb..b7325e2afbd 100644 --- a/mod/assign/externallib.php +++ b/mod/assign/externallib.php @@ -2509,8 +2509,10 @@ class mod_assign_external extends external_api { 'submissionsenabled' => new external_value(PARAM_BOOL, 'Whether submissions are enabled or not.'), 'submissionssubmittedcount' => new external_value(PARAM_INT, 'Number of submissions in submitted status.'), 'submissionsneedgradingcount' => new external_value(PARAM_INT, 'Number of submissions that need grading.'), - 'warnofungroupedusers' => new external_value(PARAM_BOOL, 'Whether we need to warn people that there - are users without groups.'), + 'warnofungroupedusers' => new external_value(PARAM_ALPHA, 'Whether we need to warn people that there + are users without groups (\'warningrequired\'), warn + people there are users who will submit in the default + group (\'warningoptional\') or no warning (\'\').'), ), 'Grading information.', VALUE_OPTIONAL ), 'lastattempt' => new external_single_structure( diff --git a/mod/assign/renderable.php b/mod/assign/renderable.php index 27c40438dc2..86c3787a6ef 100644 --- a/mod/assign/renderable.php +++ b/mod/assign/renderable.php @@ -754,7 +754,7 @@ class assign_grading_summary implements renderable { public $isvisible = true; /** @var string no warning needed about group submissions */ - const WARN_GROUPS_NO = 'nowarning'; + const WARN_GROUPS_NO = false; /** @var string warn about group submissions, as groups are required */ const WARN_GROUPS_REQUIRED = 'warnrequired'; /** @var string warn about group submissions, as some will submit as 'Default group' */ diff --git a/mod/assign/tests/externallib_test.php b/mod/assign/tests/externallib_test.php index 30ebeb5769a..dda4e5ee019 100644 --- a/mod/assign/tests/externallib_test.php +++ b/mod/assign/tests/externallib_test.php @@ -2029,7 +2029,7 @@ class mod_assign_external_testcase extends externallib_advanced_testcase { $this->assertEquals(0, $result['gradingsummary']['submissiondraftscount']); $this->assertEquals(1, $result['gradingsummary']['submissionssubmittedcount']); // One student from G1 submitted. $this->assertEquals(1, $result['gradingsummary']['submissionsneedgradingcount']); // One student from G1 submitted. - $this->assertFalse($result['gradingsummary']['warnofungroupedusers']); + $this->assertEmpty($result['gradingsummary']['warnofungroupedusers']); // Second group. $result = mod_assign_external::get_submission_status($assign->get_instance()->id, 0, $g2->id); From 07aafc9f806bba8d3bda870f0d64d67a4468a1ee Mon Sep 17 00:00:00 2001 From: Davo Smith Date: Tue, 9 Jul 2019 09:01:44 +0100 Subject: [PATCH 3/3] MDL-64811 mod_assign: add webservice return type change to upgrade.txt --- mod/assign/upgrade.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mod/assign/upgrade.txt b/mod/assign/upgrade.txt index 0c755af7a6e..f0c9e8cc432 100644 --- a/mod/assign/upgrade.txt +++ b/mod/assign/upgrade.txt @@ -1,4 +1,7 @@ This files describes API changes in the assign code. +=== 3.8 === +* Webservice function mod_assign_get_submission_status, return value 'warnofungroupedusers', changed from PARAM_BOOL to PARAM_ALPHA. See the description for possible values. + === 3.7 === * Submissions plugins should implement the "remove" function to remove data when "Remove submission" is used.