MDL-60710 course: setting to show/hide duplicate coursecontact

admin/settings/appearance.php

@@ -218,6 +218,9 @@
     // coursecontact is the person responsible for course - usually manages enrolments, receives notification, etc.
     $temp = new admin_settingpage('coursecontact', new lang_string('courses'));
     $temp->add(new admin_setting_special_coursecontact());
+    $temp->add(new admin_setting_configcheckbox('coursecontactduplicates',
+            new lang_string('coursecontactduplicates', 'admin'),
+            new lang_string('coursecontactduplicates_desc', 'admin'), 0));
     $temp->add(new admin_setting_configcheckbox('courselistshortnames',
             new lang_string('courselistshortnames', 'admin'),
             new lang_string('courselistshortnames_desc', 'admin'), 0));

course/classes/list_element.php

@@ -151,37 +151,50 @@ public function get_course_contacts() {
             // No roles are configured to be displayed as course contacts.
             return array();
         }
+
+        if (!$this->has_course_contacts()) {
+            // No course contacts exist.
+            return array();
+        }
+
         if ($this->coursecontacts === null) {
             $this->coursecontacts = array();
-            $context = context_course::instance($this->id);
 
-            if (!isset($this->record->managers)) {
-                // Preload course contacts from DB.
-                $courses = array($this->id => &$this->record);
-                core_course_category::preload_course_contacts($courses);
-            }
+            $context = context_course::instance($this->id);
 
-            // Build return array with full roles names (for this course context) and users names.
             $canviewfullnames = has_capability('moodle/site:viewfullnames', $context);
+
+            $displayall = get_config('core', 'coursecontactduplicates');
+
             foreach ($this->record->managers as $ruser) {
-                if (isset($this->coursecontacts[$ruser->id])) {
-                    // Only display a user once with the highest sortorder role.
+                $processed = array_key_exists($ruser->id, $this->coursecontacts);
+                if (!$displayall && $processed) {
                     continue;
                 }
-                $user = new stdClass();
-                $user = username_load_fields_from_object($user, $ruser, null, array('id', 'username'));
-                $role = new stdClass();
-                $role->id = $ruser->roleid;
-                $role->name = $ruser->rolename;
-                $role->shortname = $ruser->roleshortname;
-                $role->coursealias = $ruser->rolecoursealias;
-
-                $this->coursecontacts[$user->id] = array(
-                    'user' => $user,
-                    'role' => $role,
-                    'rolename' => role_get_name($role, $context, ROLENAME_ALIAS),
-                    'username' => fullname($user, $canviewfullnames)
-                );
+
+                $role = (object)[
+                        'id'          => $ruser->roleid,
+                        'name'        => $ruser->rolename,
+                        'shortname'   => $ruser->roleshortname,
+                        'coursealias' => $ruser->rolecoursealias,
+                ];
+                $role->displayname = role_get_name($role, $context, ROLENAME_ALIAS);
+
+                if (!$processed) {
+                    $user = username_load_fields_from_object((object)[], $ruser, null, ['id', 'username']);
+                    $this->coursecontacts[$ruser->id] = [
+                            'user'     => $user,
+                            'username' => fullname($user, $canviewfullnames),
+
+                            // List of all roles.
+                            'roles'    => [],
+
+                            // Primary role of this user.
+                            'role'     => $role,
+                            'rolename' => $role->displayname,
+                    ];
+                }
+                $this->coursecontacts[$ruser->id]['roles'][$ruser->roleid] = $role;
             }
         }
         return $this->coursecontacts;

course/externallib.php

@@ -2263,8 +2263,13 @@ protected static function get_course_public_information(core_course_list_element
         foreach ($course->get_course_contacts() as $contact) {
              $coursecontacts[] = array(
                 'id' => $contact['user']->id,
-                'fullname' => $contact['username']
-            );
+                'fullname' => $contact['username'],
+                'roles' => array_map(function($role){
+                    return array('id' => $role->id, 'name' => $role->displayname);
+                }, $contact['role']),
+                'role' => array('id' => $contact['role']->id, 'name' => $contact['role']->displayname),
+                'rolename' => $contact['rolename']
+             );
         }
 
         // Allowed enrolment methods (maybe we can self-enrol).

course/renderer.php

@@ -1197,10 +1197,13 @@ protected function coursecat_coursebox_content(coursecat_helper $chelper, $cours
         // Display course contacts. See core_course_list_element::get_course_contacts().
         if ($course->has_course_contacts()) {
             $content .= html_writer::start_tag('ul', array('class' => 'teachers'));
-            foreach ($course->get_course_contacts() as $userid => $coursecontact) {
-                $name = $coursecontact['rolename'].': '.
+            foreach ($course->get_course_contacts() as $coursecontact) {
+                $rolenames = array_map(function ($role) {
+                    return $role->displayname;
+                }, $coursecontact['roles']);
+                $name = implode(", ", $rolenames).': '.
                         html_writer::link(new moodle_url('/user/view.php',
-                                array('id' => $userid, 'course' => SITEID)),
+                                array('id' => $coursecontact['user']->id, 'course' => SITEID)),
                             $coursecontact['username']);
                 $content .= html_writer::tag('li', $name);
             }

course/tests/behat/behat_course.php

@@ -1879,4 +1879,82 @@ public function i_navigate_to_course_participants() {
         $xpath = "//div[contains(@class,'block')]//li[p/*[string(.)=$coursestr or string(.)=$mycoursestr]]";
         $this->execute('behat_general::i_click_on_in_the', [get_string('participants'), 'link', $xpath, 'xpath_element']);
     }
+
+    /**
+     * Check that one teacher appears before another in the course contacts.
+     *
+     * @Given /^I should see teacher "(?P<pteacher_string>(?:[^"]|\\")*)" before "(?P<fteacher_string>(?:[^"]|\\")*)" in the course contact listing$/
+     *
+     * @param string $pteacher The first teacher to find
+     * @param string $fteacher The second teacher to find (should be after the first teacher)
+     *
+     * @throws ExpectationException
+     */
+    public function i_should_see_teacher_before($pteacher, $fteacher) {
+        $xpath = "//ul[contains(@class,'teachers')]//li//a[text()='{$pteacher}']/ancestor::li//following::a[text()='{$fteacher}']";
+        $msg = "Teacher {$pteacher} does not appear before Teacher {$fteacher}";
+        if (!$this->getSession()->getDriver()->find($xpath)) {
+            throw new ExpectationException($msg, $this->getSession());
+        }
+    }
+
+    /**
+     * Check that one teacher oes not appears after another in the course contacts.
+     *
+     * @Given /^I should not see teacher "(?P<fteacher_string>(?:[^"]|\\")*)" after "(?P<pteacher_string>(?:[^"]|\\")*)" in the course contact listing$/
+     *
+     * @param string $fteacher The teacher that should not be found (after the other teacher)
+     * @param string $pteacher The teacher after who the other should not be found (this teacher must be found!)
+     *
+     * @throws ExpectationException
+     */
+    public function i_should_not_see_teacher_after($fteacher, $pteacher) {
+        $xpathliteral = behat_context_helper::escape($pteacher);
+        $xpath = "/descendant-or-self::*[contains(., $xpathliteral)]" .
+                "[count(descendant::*[contains(., $xpathliteral)]) = 0]";
+        try {
+            $nodes = $this->find_all('xpath', $xpath);
+        } catch (ElementNotFoundException $e) {
+            throw new ExpectationException('"' . $pteacher . '" text was not found in the page', $this->getSession());
+        }
+        $xpath = "//ul[contains(@class,'teachers')]//li//a[text()='{$pteacher}']/ancestor::li//following::a[text()='{$fteacher}']";
+        $msg = "Teacher {$fteacher} appears after Teacher {$pteacher}";
+        if ($this->getSession()->getDriver()->find($xpath)) {
+            throw new ExpectationException($msg, $this->getSession());
+        }
+    }
+
+    /**
+     * Moves up the specified role.
+     *
+     * @Given /^I move up role "(?P<role_string>(?:[^"]|\\")*)" in the global role sortorder$/
+     * @param String $role
+     */
+    public function i_move_up_role($role) {
+        global $DB;
+        $roledb = $DB->get_record('role', array('shortname' => $role), 'id, sortorder', MUST_EXIST);
+        $query = "SELECT id, sortorder FROM {role} WHERE sortorder < ".$roledb->sortorder." ORDER BY sortorder DESC";
+        $previousroles = $DB->get_records_sql($query, null, 0, 1);
+        foreach ($previousroles as $id => $previousrole) {
+            switch_roles($previousrole, $roledb);
+            break;
+        }
+    }
+
+    /**
+     * Moves down the specified role.
+     *
+     * @Given /^I move down role "(?P<role_string>(?:[^"]|\\")*)" in the global role sortorder$/
+     * @param String $role
+     */
+    public function i_move_down_role($role) {
+        global $DB;
+        $roledb = $DB->get_record('role', array('shortname' => $role), 'id, sortorder', MUST_EXIST);
+        $query = "SELECT id, sortorder FROM {role} WHERE sortorder > " . $roledb->sortorder . " ORDER BY sortorder ASC";
+        $previousroles = $DB->get_records_sql($query, null, 0, 1);
+        foreach ($previousroles as $id => $previousrole) {
+            switch_roles($previousrole, $roledb);
+            break;
+        }
+    }
 }

course/tests/behat/course_contact.feature

@@ -0,0 +1,198 @@
+@core @core_course
+Feature: Test we can see coursecontacts.
+  As a student I need to see coursecontacts
+  As a admin I need to test we can resort coursecontacts.
+  As a admin I need to test we can show duplicate course contacts
+  Scenario: Test coursecontacts functionality
+    Given the following "categories" exist:
+      | name | category | idnumber |
+      | Cat 1 | 0 | CAT1 |
+    And the following "courses" exist:
+      | fullname | shortname | category | format |
+      | Course 1 | C1 | CAT1 | topics |
+    Given the following "users" exist:
+      | username | firstname | lastname | email |
+      | teacher1 | Teacher 1 | T | teacher1@example.com |
+      | teacher2 | Teacher 2 | T | teacher2@example.com |
+      | teacher3 | Teacher 3 | T | teacher3@example.com |
+      | manager1 | Manager 1 | M | manager1@example.com |
+      | student1 | Student 1 | S | student1@example.com |
+    And the following "course enrolments" exist:
+      | user | course | role |
+      | teacher1 | C1 | editingteacher |
+      | teacher1 | C1 | teacher |
+      | teacher2 | C1 | teacher |
+      | teacher3 | C1 | editingteacher |
+      | manager1 | C1 | manager |
+    And I log in as "admin"
+    And I am on course index
+    And I should see "Cat 1" in the "#region-main" "css_element"
+    And I follow "Cat 1"
+    And I wait until the page is ready
+    And I should see "Course 1" in the "#region-main" "css_element"
+    And I should see "Teacher 1" in the "#region-main" "css_element"
+    And I should not see "Teacher 2" in the "#region-main" "css_element"
+    And I log out
+    And I log in as "manager1"
+    And I am on course index
+    And I should see "Cat 1" in the "#region-main" "css_element"
+    And I follow "Cat 1"
+    And I wait until the page is ready
+    And I should see "Course 1" in the "#region-main" "css_element"
+    And I should see "Teacher 1" in the "#region-main" "css_element"
+    And I should not see "Teacher 2" in the "#region-main" "css_element"
+    And I log out
+    And I log in as "teacher1"
+    And I am on course index
+    And I should see "Cat 1" in the "#region-main" "css_element"
+    And I follow "Cat 1"
+    And I wait until the page is ready
+    And I should see "Course 1" in the "#region-main" "css_element"
+    And I should see "Teacher 1" in the "#region-main" "css_element"
+    And I should not see "Teacher 2" in the "#region-main" "css_element"
+    And I log out
+    And I log in as "student1"
+    And I am on course index
+    And I should see "Cat 1" in the "#region-main" "css_element"
+    And I follow "Cat 1"
+    And I wait until the page is ready
+    And I should see "Course 1" in the "#region-main" "css_element"
+    And I should see "Teacher 1" in the "#region-main" "css_element"
+    And I should not see "Teacher 2" in the "#region-main" "css_element"
+    And I should see teacher "Teacher 1 T" before "Teacher 3 T" in the course contact listing
+    And I should not see teacher "Teacher 1 T" after "Teacher 3 T" in the course contact listing
+    And I log out
+  Scenario: Test selection and duplicates of coursecontact roles
+    Given the following "categories" exist:
+      | name | category | idnumber |
+      | Cat 1 | 0 | CAT1 |
+    And the following "courses" exist:
+      | fullname | shortname | category | format |
+      | Course 1 | C1 | CAT1 | topics |
+    Given the following "users" exist:
+      | username | firstname | lastname | email |
+      | teacher1 | Teacher 1 | T | teacher1@example.com |
+      | teacher2 | Teacher 2 | T | teacher2@example.com |
+      | teacher3 | Teacher 3 | T | teacher3@example.com |
+      | manager1 | Manager 1 | M | manager1@example.com |
+      | student1 | Student 1 | S | student1@example.com |
+    And the following "course enrolments" exist:
+      | user | course | role |
+      | teacher1 | C1 | editingteacher |
+      | teacher1 | C1 | teacher |
+      | teacher2 | C1 | teacher |
+      | teacher3 | C1 | editingteacher |
+      | manager1 | C1 | manager |
+    And I log in as "admin"
+    And I am on course index
+    And I should see "Cat 1" in the "#region-main" "css_element"
+    And I follow "Cat 1"
+    And I wait until the page is ready
+    And I should see "Course 1" in the "#region-main" "css_element"
+    And I should see "Teacher 1" in the "#region-main" "css_element"
+    And I should not see "Teacher 2" in the "#region-main" "css_element"
+    And I should not see "Manager 1" in the "#region-main" "css_element"
+    And I am on site homepage
+    And I navigate to "Appearance > Courses" in site administration
+    And I set the following fields to these values:
+      | Manager | 1 |
+      | Non-editing teacher | 1 |
+      | Show duplicate course contacts | 1 |
+    And I press "Save changes"
+    And I am on course index
+    And I should see "Cat 1" in the "#region-main" "css_element"
+    And I follow "Cat 1"
+    And I wait until the page is ready
+    And I should see "Course 1" in the "#region-main" "css_element"
+    And I should see "Teacher 1" in the "#region-main" "css_element"
+    And I should see "Teacher 2" in the "#region-main" "css_element"
+    And I should see "Teacher 3" in the "#region-main" "css_element"
+    And I should see "Manager 1" in the "#region-main" "css_element"
+    And I should see teacher "Manager 1 M" before "Teacher 1 T" in the course contact listing
+    And I should see teacher "Teacher 1 T" before "Teacher 3 T" in the course contact listing
+    And I should see teacher "Teacher 3 T" before "Teacher 2 T" in the course contact listing
+    And I am on site homepage
+    And I navigate to "Appearance > Courses" in site administration
+    And I set the following fields to these values:
+      | Manager | 0 |
+    And I press "Save changes"
+    And I log out
+    And I log in as "admin"
+    And I am on course index
+    And I should see "Cat 1" in the "#region-main" "css_element"
+    And I follow "Cat 1"
+    And I wait until the page is ready
+    And I should see "Course 1" in the "#region-main" "css_element"
+    And I should see "Teacher 1" in the "#region-main" "css_element"
+    And I should not see "Manager 1" in the "#region-main" "css_element"
+  Scenario: Test selection of coursecontact roles
+    Given the following "categories" exist:
+      | name | category | idnumber |
+      | Cat 1 | 0 | CAT1 |
+    And the following "courses" exist:
+      | fullname | shortname | category | format |
+      | Course 1 | C1 | CAT1 | topics |
+    Given the following "users" exist:
+      | username | firstname | lastname | email |
+      | teacher1 | Teacher 1 | T | teacher1@example.com |
+      | teacher2 | Teacher 2 | T | teacher2@example.com |
+      | teacher3 | Teacher 3 | T | teacher3@example.com |
+      | manager1 | Manager 1 | M | manager1@example.com |
+      | manager2 | Manager 2 | M | manager1@example.com |
+      | student1 | Student 1 | S | student1@example.com |
+    And the following "course enrolments" exist:
+      | user | course | role |
+      | teacher1 | C1 | editingteacher |
+      | teacher1 | C1 | teacher |
+      | teacher2 | C1 | teacher |
+      | teacher3 | C1 | editingteacher |
+      | manager1 | C1 | manager |
+    And I log in as "admin"
+    And I navigate to "Appearance > Courses" in site administration
+    And I set the following fields to these values:
+      | Manager | Yes |
+      | Non-editing teacher | Yes |
+      | Show duplicate course contacts | Yes |
+    And I press "Save changes"
+    And I am on course index
+    And I should see "Cat 1" in the "#region-main" "css_element"
+    And I follow "Cat 1"
+    And I wait until the page is ready
+    And I should see "Course 1" in the "#region-main" "css_element"
+    And I should see "Teacher 1" in the "#region-main" "css_element"
+    And I should see "Teacher 2" in the "#region-main" "css_element"
+    And I should see "Teacher 3" in the "#region-main" "css_element"
+    And I should see "Manager 1" in the "#region-main" "css_element"
+    And I should see teacher "Manager 1 M" before "Teacher 1 T" in the course contact listing
+    And I should see teacher "Teacher 1 T" before "Teacher 3 T" in the course contact listing
+    And I should see teacher "Teacher 3 T" before "Teacher 2 T" in the course contact listing
+    And I navigate to "Appearance > Courses" in site administration
+    And I move up role "teacher" in the global role sortorder
+    And I move up role "teacher" in the global role sortorder
+    And I move up role "teacher" in the global role sortorder
+    And I move up role "teacher" in the global role sortorder
+    And I move up role "teacher" in the global role sortorder
+    And I move up role "teacher" in the global role sortorder
+    And I move up role "teacher" in the global role sortorder
+    And I move up role "teacher" in the global role sortorder
+    And I press "Save changes"
+    And I am on site homepage
+    And I follow "Course 1"
+    And I log out
+    And I log in as "admin"
+    And I am on course index
+    And I follow "Purge all caches"
+    And I wait until the page is ready
+    And I am on course index
+    And I should see "Cat 1" in the "#region-main" "css_element"
+    And I follow "Cat 1"
+    And I wait until the page is ready
+    And I should see "Course 1" in the "#region-main" "css_element"
+    And I should see "Teacher 1" in the "#region-main" "css_element"
+    And I should see "Teacher 2" in the "#region-main" "css_element"
+    And I should see "Teacher 3" in the "#region-main" "css_element"
+    And I should not see teacher "Teacher 2 T" after "Manager 1 M" in the course contact listing
+    And I should not see teacher "Teacher 2 T" after "Teacher 3 T" in the course contact listing
+    And I should see teacher "Teacher 1 T" before "Manager 1 M" in the course contact listing
+    And I should not see teacher "Teacher 2 T" after "Manager 1 M" in the course contact listing
+    And I should not see teacher "Teacher 2 T" after "Teacher 3 T" in the course contact listing