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

admin/settings/appearance.php

@@ -218,6 +218,9 @@ preferences,moodle|/user/preferences.php|t/preferences',
     // 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 @@ class core_course_list_element implements IteratorAggregate {
             // 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 @@ class core_course_external extends external_api {
         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 @@ class core_course_renderer extends plugin_renderer_base {
         // 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 @@ class behat_course extends behat_base {
         $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

+@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

course/tests/category_test.php

@@ -570,6 +570,9 @@ class core_course_category_testcase extends advanced_testcase {
 
     public function test_course_contacts() {
         global $DB, $CFG;
+
+        set_config('coursecontactduplicates', false);
+
         $teacherrole = $DB->get_record('role', array('shortname'=>'editingteacher'));
         $managerrole = $DB->get_record('role', array('shortname'=>'manager'));
         $studentrole = $DB->get_record('role', array('shortname'=>'student'));
@@ -681,8 +684,167 @@ class core_course_category_testcase extends advanced_testcase {
 
         // Suspend user 4 and make sure he is no longer in contacts of course 1 in category 4.
         $manual->enrol_user($enrol[4][1], $user[4], $teacherrole->id, 0, 0, ENROL_USER_SUSPENDED);
+        $allcourses = core_course_category::get(0)->get_courses(array(
+                'recursive' => true,
+                'coursecontacts' => true,
+                'sort' => array('idnumber' => 1))
+        );
+        $contacts = $allcourses[$course[4][1]]->get_course_contacts();
+        $this->assertCount(1, $contacts);
+        $contact = reset($contacts);
+        $this->assertEquals('F5 L5', $contact['username']);
+
+        $CFG->coursecontact = $oldcoursecontact;
+    }
+
+    public function test_course_contacts_with_duplicates() {
+        global $DB, $CFG;
+
+        set_config('coursecontactduplicates', true);
+
+        $displayall = get_config('core', 'coursecontactduplicates');
+        $this->assertEquals(true, $displayall);
+
+        $teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher'));
+        $managerrole = $DB->get_record('role', array('shortname' => 'manager'));
+        $studentrole = $DB->get_record('role', array('shortname' => 'student'));
+        $oldcoursecontact = $CFG->coursecontact;
+
+        $CFG->coursecontact = $managerrole->id. ','. $teacherrole->id;
+
+        /*
+        * User is listed in course contacts for the course if he has one of the
+        * "course contact" roles ($CFG->coursecontact) AND is enrolled in the course.
+        * If the user has several roles all roles are displayed, but each role only once per user.
+        */
+
+        /*
+        * Test case:
+        *
+        * == Cat1 (user2 has teacher role)
+        *    == Cat2
+        *      -- course21 (user2 is enrolled as manager) | [Expected] Manager: F2 L2
+        *      -- course22 (user2 is enrolled as student) | [Expected] Teacher: F2 L2
+        *      == Cat4 (user2 has manager role)
+        *        -- course41 (user4 is enrolled as teacher, user5 is enrolled as manager)
+        *                                                 | [Expected] Manager: F5 L5, Teacher: F4 L4
+        *        -- course42 (user2 is enrolled as teacher) | [Expected] Manager: F2 L2
+        *    == Cat3 (user3 has manager role)
+        *      -- course31 (user3 is enrolled as student) | [Expected] Manager: F3 L3
+        *      -- course32                                | [Expected]
+        *    -- course11 (user1 is enrolled as teacher)   | [Expected] Teacher: F1 L1
+        *    -- course12 (user1 has teacher role)         | [Expected]
+        *                 also user4 is enrolled as teacher but enrolment is not active
+        */
+        $category = $course = $enrol = $user = array();
+        $category[1] = core_course_category::create(array('name' => 'Cat1'))->id;
+        $category[2] = core_course_category::create(array('name' => 'Cat2', 'parent' => $category[1]))->id;
+        $category[3] = core_course_category::create(array('name' => 'Cat3', 'parent' => $category[1]))->id;
+        $category[4] = core_course_category::create(array('name' => 'Cat4', 'parent' => $category[2]))->id;
+        foreach (array(1, 2, 3, 4) as $catid) {
+            foreach (array(1, 2) as $courseid) {
+                $course[$catid][$courseid] = $this->getDataGenerator()->create_course(array(
+                        'idnumber' => 'id'.$catid.$courseid,
+                        'category' => $category[$catid])
+                )->id;
+                $enrol[$catid][$courseid] = $DB->get_record(
+                        'enrol',
+                        array('courseid' => $course[$catid][$courseid], 'enrol' => 'manual'),
+                        '*',
+                        MUST_EXIST
+                );
+            }
+        }
+        foreach (array(1, 2, 3, 4, 5) as $userid) {
+            $user[$userid] = $this->getDataGenerator()->create_user(array(
+                            'firstname' => 'F'.$userid,
+                            'lastname' => 'L'.$userid)
+            )->id;
+        }
+
+        $manual = enrol_get_plugin('manual');
+
+        // Nobody is enrolled now and course contacts are empty.
+        $allcourses = core_course_category::get(0)->get_courses(array(
+                'recursive' => true,
+                'coursecontacts' => true,
+                'sort' => array('idnumber' => 1))
+        );
+        foreach ($allcourses as $onecourse) {
+            $this->assertEmpty($onecourse->get_course_contacts());
+        }
+
+        // Cat1: user2 has teacher role.
+        role_assign($teacherrole->id, $user[2], context_coursecat::instance($category[1]));
+        // Course21: user2 is enrolled as manager.
+        $manual->enrol_user($enrol[2][1], $user[2], $managerrole->id);
+        // Course22: user2 is enrolled as student.
+        $manual->enrol_user($enrol[2][2], $user[2], $studentrole->id);
+        // Cat4: user2 has manager role.
+        role_assign($managerrole->id, $user[2], context_coursecat::instance($category[4]));
+        // Course41: user4 is enrolled as teacher, user5 is enrolled as manager.
+        $manual->enrol_user($enrol[4][1], $user[4], $teacherrole->id);
+        $manual->enrol_user($enrol[4][1], $user[5], $managerrole->id);
+        // Course42: user2 is enrolled as teacher.
+        $manual->enrol_user($enrol[4][2], $user[2], $teacherrole->id);
+        // Cat3: user3 has manager role.
+        role_assign($managerrole->id, $user[3], context_coursecat::instance($category[3]));
+        // Course31: user3 is enrolled as student.
+        $manual->enrol_user($enrol[3][1], $user[3], $studentrole->id);
+        // Course11: user1 is enrolled as teacher and user4 is enrolled as teacher and has manager role.
+        $manual->enrol_user($enrol[1][1], $user[1], $teacherrole->id);
+        $manual->enrol_user($enrol[1][1], $user[4], $teacherrole->id);
+        role_assign($managerrole->id, $user[4], context_course::instance($course[1][1]));
+        // Course12: user1 has teacher role, but is not enrolled, as well as user4 is enrolled as teacher, but user4's enrolment is
+        // not active.
+        role_assign($teacherrole->id, $user[1], context_course::instance($course[1][2]));
+        $manual->enrol_user($enrol[1][2], $user[4], $teacherrole->id, 0, 0, ENROL_USER_SUSPENDED);
+
         $allcourses = core_course_category::get(0)->get_courses(
-            array('recursive' => true, 'coursecontacts' => true, 'sort' => array('idnumber' => 1)));
+                array('recursive' => true, 'coursecontacts' => true, 'sort' => array('idnumber' => 1)));
+        // Simplify the list of contacts for each course (similar as renderer would do).
+        $contacts = array();
+        foreach (array(1, 2, 3, 4) as $catid) {
+            foreach (array(1, 2) as $courseid) {
+                $tmp = array();
+                foreach ($allcourses[$course[$catid][$courseid]]->get_course_contacts() as $contact) {
+                    $rolenames = array_map(function ($role) {
+                        return $role->displayname;
+                    }, $contact['roles']);
+                    $tmp[] = implode(", ", $rolenames). ': '.
+                            $contact['username'];
+                }
+                $contacts[$catid][$courseid] = join(', ', $tmp);
+            }
+        }
+
+        // Assert:
+        // Course21: user2 is enrolled as manager. [Expected] Manager: F2 L2, Teacher: F2 L2.
+        $this->assertSame('Manager, Teacher: F2 L2', $contacts[2][1]);
+        // Course22: user2 is enrolled as student. [Expected] Teacher: F2 L2.
+        $this->assertSame('Teacher: F2 L2', $contacts[2][2]);
+        // Course41: user4 is enrolled as teacher, user5 is enrolled as manager. [Expected] Manager: F5 L5, Teacher: F4 L4.
+        $this->assertSame('Manager: F5 L5, Teacher: F4 L4', $contacts[4][1]);
+        // Course42: user2 is enrolled as teacher. [Expected] Manager: F2 L2, Teacher: F2 L2.
+        $this->assertSame('Manager, Teacher: F2 L2', $contacts[4][2]);
+        // Course31: user3 is enrolled as student. [Expected] Manager: F3 L3.
+        $this->assertSame('Manager: F3 L3', $contacts[3][1]);
+        // Course32: nobody is enrolled. [Expected] (nothing).
+        $this->assertSame('', $contacts[3][2]);
+        // Course11: user1 is enrolled as teacher and user4 is enrolled as teacher and has manager role. [Expected] Manager: F4 L4,
+        // Teacher: F1 L1, Teacher: F4 L4.
+        $this->assertSame('Manager, Teacher: F4 L4, Teacher: F1 L1', $contacts[1][1]);
+        // Course12: user1 has teacher role, but is not enrolled, as well as user4 is enrolled as teacher, but user4's enrolment is
+        // not active. [Expected] (nothing).
+        $this->assertSame('', $contacts[1][2]);
+
+        // Suspend user 4 and make sure he is no longer in contacts of course 1 in category 4.
+        $manual->enrol_user($enrol[4][1], $user[4], $teacherrole->id, 0, 0, ENROL_USER_SUSPENDED);
+        $allcourses = core_course_category::get(0)->get_courses(array(
+                'recursive' => true,
+                'coursecontacts' => true,
+                'sort' => array('idnumber' => 1)
+        ));
         $contacts = $allcourses[$course[4][1]]->get_course_contacts();
         $this->assertCount(1, $contacts);
         $contact = reset($contacts);

course/upgrade.txt

 This files describes API changes in /course/*,
 information provided here is intended especially for developers.
 
+=== 3.6 ===
+
+  * External function core_course_external::get_course_public_information now returns the roles and the primary role of course
+   contacts.
+
 === 3.5 ===
+