From 9cb135db20b63bf445da91fcf2de2fbf3194d738 Mon Sep 17 00:00:00 2001 From: Shamim Rezaie Date: Wed, 26 Jun 2019 20:09:03 +1000 Subject: [PATCH] MDL-61114 mod_assign: assignment overrides to observe group membership --- mod/assign/lang/en/assign.php | 2 +- mod/assign/override_form.php | 32 ++++- mod/assign/overrides.php | 107 +++++++++++----- .../tests/behat/assign_group_override.feature | 100 +++++++++++++-- .../tests/behat/assign_user_override.feature | 118 ++++++++++++++++-- 5 files changed, 300 insertions(+), 59 deletions(-) diff --git a/mod/assign/lang/en/assign.php b/mod/assign/lang/en/assign.php index fbc47e07841..a58062b5d47 100644 --- a/mod/assign/lang/en/assign.php +++ b/mod/assign/lang/en/assign.php @@ -284,7 +284,7 @@ $string['gradingstudent'] = 'Grading student'; $string['gradingsummary'] = 'Grading summary'; $string['groupoverrides'] = 'Group overrides'; $string['groupoverridesdeleted'] = 'Group overrides deleted'; -$string['groupsnone'] = 'There are no groups in this course'; +$string['groupsnone'] = 'No groups you can access.'; $string['hidegrader'] = 'Hide grader identity from students'; $string['hidegrader_help'] = 'Hides the identity of any user who grades an assignment submission, so students can\'t see who marked their work.'; $string['hideshow'] = 'Hide/Show'; diff --git a/mod/assign/override_form.php b/mod/assign/override_form.php index 0c68b771006..f7eb984c6bb 100644 --- a/mod/assign/override_form.php +++ b/mod/assign/override_form.php @@ -85,13 +85,16 @@ class assign_override_form extends moodleform { * Define this form - called by the parent constructor */ protected function definition() { - global $CFG, $DB; + global $DB; $cm = $this->cm; $mform = $this->_form; $mform->addElement('header', 'override', get_string('override', 'assign')); + $assigngroupmode = groups_get_activity_groupmode($cm); + $accessallgroups = ($assigngroupmode == NOGROUPS) || has_capability('moodle/site:accessallgroups', $this->context); + if ($this->groupmode) { // Group override. if ($this->groupid) { @@ -107,7 +110,8 @@ class assign_override_form extends moodleform { $mform->freeze('sortorder'); } else { // Prepare the list of groups. - $groups = groups_get_all_groups($cm->course); + // Only include the groups the current can access. + $groups = $accessallgroups ? groups_get_all_groups($cm->course) : groups_get_activity_allowed_groups($cm); if (empty($groups)) { // Generate an error. $link = new moodle_url('/mod/assign/overrides.php', array('cmid' => $cm->id)); @@ -140,8 +144,27 @@ class assign_override_form extends moodleform { $mform->freeze('userid'); } else { // Prepare the list of users. - $users = get_enrolled_users($this->context, '', 0, - 'u.id, u.email, ' . get_all_user_name_fields(true, 'u')); + $users = []; + list($sort) = users_order_by_sql('u'); + + // Get the list of appropriate users, depending on whether and how groups are used. + if ($accessallgroups) { + $users = get_enrolled_users($this->context, '', 0, + 'u.id, u.email, ' . get_all_user_name_fields(true, 'u'), $sort); + } else if ($groups = groups_get_activity_allowed_groups($cm)) { + $enrolledjoin = get_enrolled_join($this->context, 'u.id'); + $userfields = 'u.id, u.email, ' . get_all_user_name_fields(true, 'u'); + list($ingroupsql, $ingroupparams) = $DB->get_in_or_equal(array_keys($groups), SQL_PARAMS_NAMED); + $params = $enrolledjoin->params + $ingroupparams; + $sql = "SELECT $userfields + FROM {user} u + JOIN {groups_members} gm ON gm.userid = u.id + {$enrolledjoin->joins} + WHERE gm.groupid $ingroupsql + AND {$enrolledjoin->wheres} + ORDER BY $sort"; + $users = $DB->get_records_sql($sql, $params); + } // Filter users based on any fixed restrictions (groups, profile). $info = new \core_availability\info_module($cm); @@ -233,7 +256,6 @@ class assign_override_form extends moodleform { * @return array of "element_name"=>"error_description" if there are errors */ public function validation($data, $files) { - global $COURSE, $DB; $errors = parent::validation($data, $files); $mform =& $this->_form; diff --git a/mod/assign/overrides.php b/mod/assign/overrides.php index 4a7c6894a7d..1125b734f05 100644 --- a/mod/assign/overrides.php +++ b/mod/assign/overrides.php @@ -38,13 +38,20 @@ $redirect = $CFG->wwwroot.'/mod/assign/overrides.php?cmid=' . $cmid . '&mode list($course, $cm) = get_course_and_cm_from_cmid($cmid, 'assign'); $assign = $DB->get_record('assign', array('id' => $cm->instance), '*', MUST_EXIST); +require_login($course, false, $cm); + +$context = context_module::instance($cm->id); + +// Check the user has the required capabilities to list overrides. +require_capability('mod/assign:manageoverrides', $context); + +$assigngroupmode = groups_get_activity_groupmode($cm); +$accessallgroups = ($assigngroupmode == NOGROUPS) || has_capability('moodle/site:accessallgroups', $context); + $overridecountgroup = $DB->count_records('assign_overrides', array('userid' => null, 'assignid' => $assign->id)); -// Get the course groups. -$groups = groups_get_all_groups($cm->course); -if ($groups === false) { - $groups = array(); -} +// Get the course groups that the current user can access. +$groups = $accessallgroups ? groups_get_all_groups($cm->course) : groups_get_activity_allowed_groups($cm); // Default mode is "group", unless there are no groups. if ($mode != "user" and $mode != "group") { @@ -60,13 +67,6 @@ $url = new moodle_url('/mod/assign/overrides.php', array('cmid' => $cm->id, 'mod $PAGE->set_url($url); -require_login($course, false, $cm); - -$context = context_module::instance($cm->id); - -// Check the user has the required capabilities to list overrides. -require_capability('mod/assign:manageoverrides', $context); - if ($action == 'movegroupoverride') { $id = required_param('id', PARAM_INT); $dir = required_param('dir', PARAM_ALPHA); @@ -86,38 +86,63 @@ echo $OUTPUT->heading(format_string($assign->name, true, array('context' => $con // Delete orphaned group overrides. $sql = 'SELECT o.id - FROM {assign_overrides} o LEFT JOIN {groups} g - ON o.groupid = g.id - WHERE o.groupid IS NOT NULL - AND g.id IS NULL - AND o.assignid = ?'; + FROM {assign_overrides} o + LEFT JOIN {groups} g ON o.groupid = g.id + WHERE o.groupid IS NOT NULL + AND g.id IS NULL + AND o.assignid = ?'; $params = array($assign->id); $orphaned = $DB->get_records_sql($sql, $params); if (!empty($orphaned)) { $DB->delete_records_list('assign_overrides', 'id', array_keys($orphaned)); } +$overrides = []; + // Fetch all overrides. if ($groupmode) { $colname = get_string('group'); - $sql = 'SELECT o.*, g.name - FROM {assign_overrides} o - JOIN {groups} g ON o.groupid = g.id - WHERE o.assignid = :assignid - ORDER BY o.sortorder'; - $params = array('assignid' => $assign->id); + // To filter the result by the list of groups that the current user has access to. + if ($groups) { + $params = ['assignid' => $assign->id]; + list($insql, $inparams) = $DB->get_in_or_equal(array_keys($groups), SQL_PARAMS_NAMED); + $params += $inparams; + + $sql = "SELECT o.*, g.name + FROM {assign_overrides} o + JOIN {groups} g ON o.groupid = g.id + WHERE o.assignid = :assignid AND g.id $insql + ORDER BY o.sortorder"; + + $overrides = $DB->get_records_sql($sql, $params); + } } else { $colname = get_string('user'); list($sort, $params) = users_order_by_sql('u'); - $sql = 'SELECT o.*, ' . get_all_user_name_fields(true, 'u') . ' - FROM {assign_overrides} o - JOIN {user} u ON o.userid = u.id - WHERE o.assignid = :assignid - ORDER BY ' . $sort; $params['assignid'] = $assign->id; -} -$overrides = $DB->get_records_sql($sql, $params); + if ($accessallgroups) { + $sql = 'SELECT o.*, ' . get_all_user_name_fields(true, 'u') . ' + FROM {assign_overrides} o + JOIN {user} u ON o.userid = u.id + WHERE o.assignid = :assignid + ORDER BY ' . $sort; + + $overrides = $DB->get_records_sql($sql, $params); + } else if ($groups) { + list($insql, $inparams) = $DB->get_in_or_equal(array_keys($groups), SQL_PARAMS_NAMED); + $params += $inparams; + + $sql = 'SELECT o.*, ' . get_all_user_name_fields(true, 'u') . ' + FROM {assign_overrides} o + JOIN {user} u ON o.userid = u.id + JOIN {groups_members} gm ON u.id = gm.userid + WHERE o.assignid = :assignid AND gm.groupid ' . $insql . ' + ORDER BY ' . $sort; + + $overrides = $DB->get_records_sql($sql, $params); + } +} // Initialise table. $table = new html_table(); @@ -278,13 +303,31 @@ if ($groupmode) { } else { $users = array(); // See if there are any users in the assign. - $users = get_enrolled_users($context); + if ($accessallgroups) { + $users = get_enrolled_users($context, '', 0, 'u.id'); + $nousermessage = get_string('usersnone', 'assign'); + } else if ($groups) { + $enrolledjoin = get_enrolled_join($context, 'u.id'); + list($ingroupsql, $ingroupparams) = $DB->get_in_or_equal(array_keys($groups), SQL_PARAMS_NAMED); + $params = $enrolledjoin->params + $ingroupparams; + $sql = "SELECT u.id + FROM {user} u + JOIN {groups_members} gm ON gm.userid = u.id + {$enrolledjoin->joins} + WHERE gm.groupid $ingroupsql + AND {$enrolledjoin->wheres} + ORDER BY $sort"; + $users = $DB->get_records_sql($sql, $params); + $nousermessage = get_string('usersnone', 'assign'); + } else { + $nousermessage = get_string('groupsnone', 'assign'); + } $info = new \core_availability\info_module($cm); $users = $info->filter_user_list($users); if (empty($users)) { // There are no users. - echo $OUTPUT->notification(get_string('usersnone', 'assign'), 'error'); + echo $OUTPUT->notification($nousermessage, 'error'); $options['disabled'] = true; } echo $OUTPUT->single_button($overrideediturl->out(true, diff --git a/mod/assign/tests/behat/assign_group_override.feature b/mod/assign/tests/behat/assign_group_override.feature index a0af24f003a..aaff9409dcb 100644 --- a/mod/assign/tests/behat/assign_group_override.feature +++ b/mod/assign/tests/behat/assign_group_override.feature @@ -29,18 +29,13 @@ Feature: Assign group override | student1 | G1 | | student2 | G2 | | student3 | G1 | - And I log in as "teacher1" - And I am on "Course 1" course homepage with editing mode on - And I add a "Assignment" to section "1" and I fill the form with: - | Assignment name | Test assignment name | - | Description | Submit your online text | - | assignsubmission_onlinetext_enabled | 1 | - | assignsubmission_onlinetext_wordlimit_enabled | 1 | - | assignsubmission_onlinetext_wordlimit | 10 | - | assignsubmission_file_enabled | 0 | - | gradingduedate[enabled] | 0 | + And the following "activities" exist: + | activity | name | intro | course | idnumber | assignsubmission_onlinetext_enabled | + | assign | Test assignment name | Submit your online text | C1 | assign1 | 1 | Scenario: Add, modify then delete a group override + Given I log in as "teacher1" + And I am on "Course 1" course homepage with editing mode on When I follow "Test assignment name" And I navigate to "Group overrides" in current page administration And I press "Add group override" @@ -64,6 +59,8 @@ Feature: Assign group override And I should not see "Group 1" Scenario: Duplicate a user override + Given I log in as "teacher1" + And I am on "Course 1" course homepage with editing mode on When I follow "Test assignment name" And I navigate to "Group overrides" in current page administration And I press "Add group override" @@ -86,6 +83,8 @@ Feature: Assign group override And I should see "Group 2" Scenario: Allow a group to have a different due date + Given I log in as "teacher1" + And I am on "Course 1" course homepage with editing mode on When I follow "Test assignment name" And I navigate to "Edit settings" in current page administration And I set the following fields to these values: @@ -122,6 +121,8 @@ Feature: Assign group override And I should see "Wednesday, 1 January 2020, 8:00" Scenario: Allow a group to have a different cut off date + Given I log in as "teacher1" + And I am on "Course 1" course homepage with editing mode on When I follow "Test assignment name" And I navigate to "Edit settings" in current page administration And I set the following fields to these values: @@ -158,6 +159,8 @@ Feature: Assign group override And I should see "You have not made a submission yet." Scenario: Allow a group to have a different start date + Given I log in as "teacher1" + And I am on "Course 1" course homepage with editing mode on When I follow "Test assignment name" And I navigate to "Edit settings" in current page administration And I set the following fields to these values: @@ -196,6 +199,8 @@ Feature: Assign group override @javascript Scenario: Add both a user and group override and verify that both are applied correctly + Given I log in as "teacher1" + And I am on "Course 1" course homepage with editing mode on When I follow "Test assignment name" And I navigate to "Edit settings" in current page administration And I set the following fields to these values: @@ -248,3 +253,78 @@ Feature: Assign group override And I am on "Course 1" course homepage And I follow "Test assignment name" And I should see "This assignment will accept submissions from Wednesday, 1 January 2020, 8:00" + + Scenario: Override a group when teacher is in no group, and does not have accessallgroups permission, and the activity's group mode is "separate groups" + Given the following "permission overrides" exist: + | capability | permission | role | contextlevel | reference | + | moodle/site:accessallgroups | Prevent | editingteacher | Course | C1 | + And the following "activities" exist: + | activity | name | intro | course | idnumber | groupmode | + | assign | Assignment 2 | Assignment 2 description | C1 | assign2 | 1 | + When I log in as "teacher1" + And I am on "Course 1" course homepage + And I follow "Assignment 2" + And I navigate to "Group overrides" in current page administration + Then I should see "No groups you can access." + And the "Add group override" "button" should be disabled + + Scenario: A teacher without accessallgroups permission should only be able to add group override for groups that he/she is a member of, + when the activity's group mode is "separate groups" + Given the following "permission overrides" exist: + | capability | permission | role | contextlevel | reference | + | moodle/site:accessallgroups | Prevent | editingteacher | Course | C1 | + And the following "activities" exist: + | activity | name | intro | course | idnumber | groupmode | + | assign | Assignment 2 | Assignment 2 description | C1 | assign2 | 1 | + And the following "group members" exist: + | user | group | + | teacher1 | G1 | + When I log in as "teacher1" + And I am on "Course 1" course homepage + And I follow "Assignment 2" + And I navigate to "Group overrides" in current page administration + And I press "Add group override" + Then the "Override group" select box should contain "Group 1" + And the "Override group" select box should not contain "Group 2" + + Scenario: A teacher without accessallgroups permission should only be able to see the group overrides for groups that he/she is a member of, + when the activity's group mode is "separate groups" + Given the following "permission overrides" exist: + | capability | permission | role | contextlevel | reference | + | moodle/site:accessallgroups | Prevent | editingteacher | Course | C1 | + And the following "activities" exist: + | activity | name | intro | course | idnumber | groupmode | + | assign | Assignment 2 | Assignment 2 description | C1 | assign2 | 1 | + And the following "group members" exist: + | user | group | + | teacher1 | G1 | + And I log in as "admin" + And I am on "Course 1" course homepage + And I follow "Assignment 2" + And I navigate to "Group overrides" in current page administration + And I press "Add group override" + And I set the following fields to these values: + | Override group | Group 1 | + | id_allowsubmissionsfromdate_enabled | 1 | + | allowsubmissionsfromdate[day] | 1 | + | allowsubmissionsfromdate[month] | January | + | allowsubmissionsfromdate[year] | 2020 | + | allowsubmissionsfromdate[hour] | 08 | + | allowsubmissionsfromdate[minute] | 00 | + And I press "Save and enter another override" + And I set the following fields to these values: + | Override group | Group 2 | + | id_allowsubmissionsfromdate_enabled | 1 | + | allowsubmissionsfromdate[day] | 1 | + | allowsubmissionsfromdate[month] | January | + | allowsubmissionsfromdate[year] | 2020 | + | allowsubmissionsfromdate[hour] | 08 | + | allowsubmissionsfromdate[minute] | 00 | + And I press "Save" + And I log out + When I log in as "teacher1" + And I am on "Course 1" course homepage + And I follow "Assignment 2" + And I navigate to "Group overrides" in current page administration + Then I should see "Group 1" in the ".generaltable" "css_element" + And I should not see "Group 2" in the ".generaltable" "css_element" diff --git a/mod/assign/tests/behat/assign_user_override.feature b/mod/assign/tests/behat/assign_user_override.feature index e1c711d30a1..20ba84726fd 100644 --- a/mod/assign/tests/behat/assign_user_override.feature +++ b/mod/assign/tests/behat/assign_user_override.feature @@ -1,4 +1,4 @@ -@mod @mod_assign @javascript +@mod @mod_assign Feature: Assign user override In order to grant a student special access to an assignment As a teacher @@ -18,18 +18,14 @@ Feature: Assign user override | teacher1 | C1 | editingteacher | | student1 | C1 | student | | student2 | C1 | student | - And I log in as "teacher1" - And I am on "Course 1" course homepage with editing mode on - And I add a "Assignment" to section "1" and I fill the form with: - | Assignment name | Test assignment name | - | Description | Submit your online text | - | assignsubmission_onlinetext_enabled | 1 | - | assignsubmission_onlinetext_wordlimit_enabled | 1 | - | assignsubmission_onlinetext_wordlimit | 10 | - | assignsubmission_file_enabled | 0 | - | gradingduedate[enabled] | 0 | + And the following "activities" exist: + | activity | name | intro | course | idnumber | assignsubmission_onlinetext_enabled | + | assign | Test assignment name | Submit your online text | C1 | assign1 | 1 | + @javascript Scenario: Add, modify then delete a user override + Given I log in as "teacher1" + And I am on "Course 1" course homepage with editing mode on When I follow "Test assignment name" And I navigate to "User overrides" in current page administration And I press "Add user override" @@ -52,7 +48,10 @@ Feature: Assign user override And I press "Continue" And I should not see "Sam1 Student1" + @javascript Scenario: Duplicate a user override + Given I log in as "teacher1" + And I am on "Course 1" course homepage with editing mode on When I follow "Test assignment name" And I navigate to "User overrides" in current page administration And I press "Add user override" @@ -74,7 +73,10 @@ Feature: Assign user override And I should see "Tuesday, 1 January 2030, 8:00" And I should see "Sam2 Student2" + @javascript Scenario: Allow a user to have a different due date + Given I log in as "teacher1" + And I am on "Course 1" course homepage with editing mode on When I follow "Test assignment name" And I navigate to "Edit settings" in current page administration And I set the following fields to these values: @@ -110,7 +112,10 @@ Feature: Assign user override And I follow "Test assignment name" And I should see "Wednesday, 1 January 2020, 8:00" + @javascript Scenario: Allow a user to have a different cut off date + Given I log in as "teacher1" + And I am on "Course 1" course homepage with editing mode on When I follow "Test assignment name" And I navigate to "Edit settings" in current page administration And I set the following fields to these values: @@ -146,7 +151,10 @@ Feature: Assign user override And I follow "Test assignment name" And I should see "You have not made a submission yet." + @javascript Scenario: Allow a user to have a different start date + Given I log in as "teacher1" + And I am on "Course 1" course homepage with editing mode on When I follow "Test assignment name" And I navigate to "Edit settings" in current page administration And I set the following fields to these values: @@ -181,3 +189,91 @@ Feature: Assign user override And I am on "Course 1" course homepage And I follow "Test assignment name" And I should not see "This assignment will accept submissions from Wednesday, 1 January 2020, 8:00" + + Scenario: Override a user when teacher is in no group, and does not have accessallgroups permission, and the activity's group mode is "separate groups" + Given the following "permission overrides" exist: + | capability | permission | role | contextlevel | reference | + | moodle/site:accessallgroups | Prevent | editingteacher | Course | C1 | + And the following "activities" exist: + | activity | name | intro | course | idnumber | groupmode | + | assign | Assignment 2 | Assignment 2 description | C1 | assign2 | 1 | + When I log in as "teacher1" + And I am on "Course 1" course homepage + And I follow "Assignment 2" + And I navigate to "User overrides" in current page administration + Then I should see "No groups you can access." + And the "Add user override" "button" should be disabled + + Scenario: A teacher without accessallgroups permission should only be able to add user override for users that he/she shares groups with, + when the activity's group mode is "separate groups" + Given the following "permission overrides" exist: + | capability | permission | role | contextlevel | reference | + | moodle/site:accessallgroups | Prevent | editingteacher | Course | C1 | + And the following "activities" exist: + | activity | name | intro | course | idnumber | groupmode | + | assign | Assignment 2 | Assignment 2 description | C1 | assign2 | 1 | + And the following "groups" exist: + | name | course | idnumber | + | Group 1 | C1 | G1 | + | Group 2 | C1 | G2 | + And the following "group members" exist: + | user | group | + | teacher1 | G1 | + | student1 | G1 | + | student2 | G2 | + When I log in as "teacher1" + And I am on "Course 1" course homepage + And I follow "Assignment 2" + And I navigate to "User overrides" in current page administration + And I press "Add user override" + Then the "Override user" select box should contain "Sam1 Student1, student1@example.com" + And the "Override user" select box should not contain "Sam2 Student2, student2@example.com" + + @javascript + Scenario: A teacher without accessallgroups permission should only be able to see the user override for users that he/she shares groups with, + when the activity's group mode is "separate groups" + Given the following "permission overrides" exist: + | capability | permission | role | contextlevel | reference | + | moodle/site:accessallgroups | Prevent | editingteacher | Course | C1 | + And the following "activities" exist: + | activity | name | intro | course | idnumber | groupmode | + | assign | Assignment 2 | Assignment 2 description | C1 | assign2 | 1 | + And the following "groups" exist: + | name | course | idnumber | + | Group 1 | C1 | G1 | + | Group 2 | C1 | G2 | + And the following "group members" exist: + | user | group | + | teacher1 | G1 | + | student1 | G1 | + | student2 | G2 | + And I log in as "admin" + And I am on "Course 1" course homepage + And I follow "Assignment 2" + And I navigate to "User overrides" in current page administration + And I press "Add user override" + And I set the following fields to these values: + | Override user | Student1 | + | id_allowsubmissionsfromdate_enabled | 1 | + | allowsubmissionsfromdate[day] | 1 | + | allowsubmissionsfromdate[month] | January | + | allowsubmissionsfromdate[year] | 2015 | + | allowsubmissionsfromdate[hour] | 08 | + | allowsubmissionsfromdate[minute] | 00 | + And I press "Save and enter another override" + And I set the following fields to these values: + | Override user | Student2 | + | id_allowsubmissionsfromdate_enabled | 1 | + | allowsubmissionsfromdate[day] | 1 | + | allowsubmissionsfromdate[month] | January | + | allowsubmissionsfromdate[year] | 2015 | + | allowsubmissionsfromdate[hour] | 08 | + | allowsubmissionsfromdate[minute] | 00 | + And I press "Save" + And I log out + When I log in as "teacher1" + And I am on "Course 1" course homepage + And I follow "Assignment 2" + And I navigate to "User overrides" in current page administration + Then I should see "Student1" in the ".generaltable" "css_element" + And I should not see "Student2" in the ".generaltable" "css_element"