From b1e63fc7e2eaacd5b8c30c0e6f2f11932b86e94a Mon Sep 17 00:00:00 2001 From: Silvia Pinheiro Date: Wed, 16 Jan 2019 15:20:47 +0000 Subject: [PATCH 1/2] MDL-62836 badges: Potential badge recipients by group This affects the case when a badge is setup with the criteria "manual issue by role". The non-editing teacher can see all participants of the course, when awarding a badge, even with SEPARATE GROUPS mode enabled. With this change, when the non-editing teacher chooses a group, and searches for a user, the results are filtered for that group. The same happens for "Existing badge recipients by group" when revoking a badge. --- badges/award.php | 22 +++++++++++++++-- badges/lib/awardlib.php | 54 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 71 insertions(+), 5 deletions(-) diff --git a/badges/award.php b/badges/award.php index 18242b98b54..8f8669829f0 100644 --- a/badges/award.php +++ b/badges/award.php @@ -90,6 +90,19 @@ if (empty($acceptedroles)) { die(); } +// Get groupmode and currentgroup before going further. +$groupmode = groups_get_course_groupmode($COURSE); // Groups are being used. +$currentgroup = groups_get_course_group($COURSE, true); // Get active group. + +// Check groupmode (SEPARATEGROUPS), currentgroup and capability (or admin). +if ($groupmode == SEPARATEGROUPS && empty($currentgroup) && + !has_capability('moodle/site:accessallgroups', $context) && !is_siteadmin() ) { + echo $OUTPUT->header(); + echo $OUTPUT->heading(get_string("notingroup")); + echo $OUTPUT->footer(); + die(); +} + if (count($acceptedroles) > 1) { // If there is more than one role that can award a badge, prompt user to make a selection. // If it is an admin, include all accepted roles, otherwise only the ones that current user has in this context. @@ -151,8 +164,10 @@ $options = array( 'badgeid' => $badge->id, 'context' => $context, 'issuerid' => $USER->id, - 'issuerrole' => $issuerrole->roleid - ); + 'issuerrole' => $issuerrole->roleid, + 'currentgroup' => $currentgroup, + 'url' => $url, + ); $existingselector = new badge_existing_users_selector('existingrecipients', $options); $recipientselector = new badge_potential_users_selector('potentialrecipients', $options); $recipientselector->set_existing_recipients($existingselector->find_users('')); @@ -193,6 +208,9 @@ if ($award && data_submitted() && has_capability('moodle/badges:awardbadge', $co echo $OUTPUT->header(); echo $OUTPUT->heading($strrecipients); +// Print group selector/dropdown menu (find out current groups mode). +groups_print_course_menu($COURSE, $url); + if (count($acceptedroles) > 1) { echo $OUTPUT->box($roleselect); } diff --git a/badges/lib/awardlib.php b/badges/lib/awardlib.php index 4425540cf49..f085c56cd01 100644 --- a/badges/lib/awardlib.php +++ b/badges/lib/awardlib.php @@ -52,6 +52,18 @@ abstract class badge_award_selector_base extends user_selector_base { */ protected $issuerid = null; + /** + * The return address. Accepts either a string or a moodle_url. + * @var string $url + */ + public $url; + + /** + * The current group being displayed. + * @var int $currentgroup + */ + public $currentgroup; + /** * Constructor method * @param string $name @@ -77,6 +89,15 @@ abstract class badge_award_selector_base extends user_selector_base { if (isset($options['issuerrole'])) { $this->issuerrole = $options['issuerrole']; } + if (isset($options['url'])) { + $this->url = $options['url']; + } + if (isset($options['currentgroup'])) { + $this->currentgroup = $options['currentgroup']; + } else { + // Returns group active in course, changes the group by default if 'group' page param present. + $this->currentgroup = groups_get_course_group($COURSE, true); + } } /** @@ -92,8 +113,27 @@ abstract class badge_award_selector_base extends user_selector_base { $options['badgeid'] = $this->badgeid; $options['issuerid'] = $this->issuerid; $options['issuerrole'] = $this->issuerrole; + // These will be used to filter potential badge recipients when searching. + $options['currentgroup'] = $this->currentgroup; return $options; } + + /** + * Restricts the selection of users to display, according to the groups they belong. + * + * @return array + */ + protected function get_groups_sql() { + $groupsql = ''; + $groupwheresql = ''; + $groupwheresqlparams = array(); + if ($this->currentgroup) { + $groupsql = ' JOIN {groups_members} gm ON gm.userid = u.id '; + $groupwheresql = ' AND gm.groupid = :gr_grpid '; + $groupwheresqlparams = array('gr_grpid' => $this->currentgroup); + } + return array($groupsql, $groupwheresql, $groupwheresqlparams); + } } /** @@ -141,8 +181,10 @@ class badge_potential_users_selector extends badge_award_selector_base { $wherecondition = ' WHERE ' . implode(' AND ', $whereconditions); } + list($groupsql, $groupwheresql, $groupwheresqlparams) = $this->get_groups_sql(); + list($esql, $eparams) = get_enrolled_sql($this->context, 'moodle/badges:earnbadge', 0, true); - $params = array_merge($params, $eparams); + $params = array_merge($params, $eparams, $groupwheresqlparams); $fields = 'SELECT ' . $this->required_fields_sql('u'); $countfields = 'SELECT COUNT(u.id)'; @@ -153,7 +195,9 @@ class badge_potential_users_selector extends badge_award_selector_base { $sql = " FROM {user} u JOIN ($esql) je ON je.id = u.id LEFT JOIN {badge_manual_award} bm ON (bm.recipientid = u.id AND bm.badgeid = :badgeid AND bm.issuerrole = :issuerrole) - $wherecondition AND bm.id IS NULL"; + $groupsql + $wherecondition AND bm.id IS NULL + $groupwheresql"; list($sort, $sortparams) = users_order_by_sql('u', $search, $this->accesscontext); $order = ' ORDER BY ' . $sort; @@ -204,12 +248,16 @@ class badge_existing_users_selector extends badge_award_selector_base { $fields = $this->required_fields_sql('u'); list($sort, $sortparams) = users_order_by_sql('u', $search, $this->accesscontext); - $params = array_merge($params, $eparams, $sortparams); + list($groupsql, $groupwheresql, $groupwheresqlparams) = $this->get_groups_sql(); + + $params = array_merge($params, $eparams, $sortparams, $groupwheresqlparams); $recipients = $DB->get_records_sql("SELECT $fields FROM {user} u JOIN ($esql) je ON je.id = u.id JOIN {badge_manual_award} s ON s.recipientid = u.id + $groupsql WHERE $wherecondition AND s.badgeid = :badgeid AND s.issuerrole = :issuerrole + $groupwheresql ORDER BY $sort", $params); return array(get_string('existingrecipients', 'badges') => $recipients); From ae1556ad935e93bb9e85a18b88e0fb5dea637441 Mon Sep 17 00:00:00 2001 From: Silvia Pinheiro Date: Thu, 31 Jan 2019 11:58:28 +0000 Subject: [PATCH 2/2] MDL-62836 badges: Add behat test for the new group selector --- badges/tests/behat/award_badge_groups.feature | 130 ++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 badges/tests/behat/award_badge_groups.feature diff --git a/badges/tests/behat/award_badge_groups.feature b/badges/tests/behat/award_badge_groups.feature new file mode 100644 index 00000000000..90683b37b6e --- /dev/null +++ b/badges/tests/behat/award_badge_groups.feature @@ -0,0 +1,130 @@ +@core @core_badges @_file_upload +Feature: Award badges with separate groups + In order to award badges to users for their achievements + As a teacher + I need to award badges only to users in the groups I have access + + Background: + Given the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | Teacher | 1 | teacher1@example.com | + | teacher2 | Teacher | 2 | teacher2@example.com | + | student1 | Student | 1 | student1@example.com | + | student2 | Student | 2 | student2@example.com | + And the following "courses" exist: + | fullname | shortname | category | groupmode | + | Course 1 | C1 | 0 | 1 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | teacher2 | C1 | teacher | + | student1 | C1 | student | + | student2 | C1 | student | + And the following "groups" exist: + | name | course | idnumber | + | Class A | C1 | CA | + | Class B | C1 | CB | + And the following "group members" exist: + | user | group | + | student1 | CB | + | teacher1 | CB | + | student2 | CA | + | teacher2 | CA | + And I log in as "teacher1" + And I am on "Course 1" course homepage + And I navigate to "Edit settings" in current page administration + And I expand all fieldsets + And I set the field "Group mode" to "Separate groups" + And I press "Save and display" + And I navigate to "Badges > Add a new badge" in current page administration + And I follow "Add a new badge" + And I set the following fields to these values: + | Name | Course Badge | + | Description | Course badge description | + | issuername | Tester of course badge | + And I upload "badges/tests/behat/badge.png" file to "Image" filemanager + And I press "Create badge" + And I set the field "type" to "Manual issue by role" + And I expand all fieldsets + And I set the field "Teacher" to "1" + And I set the field "Non-editing teacher" to "1" + # Set to ANY of the roles awards badge. + And I set the field "Any of the selected roles awards the badge" to "1" + And I press "Save" + And I press "Enable access" + And I press "Continue" + And I log out + + @javascript + Scenario: Award course badge as non-editing teacher with only one group + When I log in as "teacher2" + And I am on "Course 1" course homepage + And I navigate to "Badges > Manage badges" in current page administration + And I follow "Manage badges" + And I follow "Course Badge" + And I press "Award badge" + And I set the field "role" to "Non-editing teacher" + # Teacher 2 should see a "Separate groups" label with the group he is in + Then I should see "Separate groups: Class A" + # Teacher 2 should only see the users who belong to the same group as he does + And I should see "Student 2" + And I should not see "Student 1" + # Non-editing teacher can award the badge + And I set the field "potentialrecipients[]" to "Student 2 (student2@example.com)" + And I press "Award badge" + And I follow "Course Badge" + And I should see "Recipients (1)" + And I log out + And I log in as "student2" + And I follow "Profile" in the user menu + And I click on "Course 1" "link" in the "region-main" "region" + And I should see "Course Badge" + And I log out + + @javascript + Scenario: Award course badge as non-editing teacher with more than one group + Given I log in as "teacher1" + And I am on "Course 1" course homepage + And I navigate to "Users > Groups" in current page administration + And I follow "Groups" + And I set the field "groups" to "Class B (2)" + And I press "Add/remove users" + And I set the field "addselect" to "Teacher 2 (teacher2@example.com)" + And I press "Add" + And I log out + When I log in as "teacher2" + And I am on "Course 1" course homepage + And I navigate to "Badges > Manage badges" in current page administration + And I follow "Manage badges" + And I follow "Course Badge" + And I press "Award badge" + And I set the field "role" to "Non-editing teacher" + # Teacher 2 should see a "Separate groups" label and a dropdown menu with the groups he belongs to + And I set the field "Separate groups" to "Class A" + Then I should see "Student 2" + And I should not see "Student 1" + And I set the field "Separate groups" to "Class B" + And I should see "Student 1" + And I should not see "Student 2" + And I log out + + @javascript + Scenario: Award course badge as non-editing teacher without any group + Given I log in as "teacher1" + And I am on "Course 1" course homepage + And I navigate to "Users > Groups" in current page administration + And I follow "Groups" + And I set the field "groups" to "Class A (2)" + And I press "Add/remove users" + And I set the field "removeselect" to "Teacher 2 (teacher2@example.com)" + And I press "Remove" + And I press "Back to groups" + And I log out + When I log in as "teacher2" + And I am on "Course 1" course homepage + And I navigate to "Badges > Manage badges" in current page administration + And I follow "Manage badges" + And I follow "Course Badge" + And I press "Award badge" + # Teacher 2 shouldn't be able to go further + Then I should see "Sorry, but you need to be part of a group to see this page." \ No newline at end of file