Merge branch 'MDL-60710-master' of git://github.com/abias/moodle

This commit is contained in:
David Monllao 2018-10-09 09:00:42 +02:00
commit bbb2141084
9 changed files with 417 additions and 28 deletions

View File

@ -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));

View File

@ -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);
}
// 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;

View File

@ -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).

View File

@ -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);
}

View File

@ -1879,4 +1879,48 @@ 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());
}
}
}

View File

@ -0,0 +1,151 @@
@core @core_course
Feature: Test if displaying the course contacts works correctly:
As a user I need to see the course contacts of a course.
As an admin I need to be able to control the appearance of the course contacts.
Background:
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 |
And the following "users" exist:
| username | firstname | lastname | email |
| teacher1 | Teacher | 1 | teacher1@example.com |
| teacher2 | Teacher | 2 | teacher2@example.com |
| teacher3 | Teacher | 3 | teacher3@example.com |
| manager1 | Manager | 1 | manager1@example.com |
| student1 | Student | 1 | 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 |
Scenario: Test general course contacts functionality for all user roles
Given I log in as "admin"
And I navigate to "Appearance > Courses" in site administration
And I set the following fields to these values:
| Manager | 0 |
| Teacher | 1 |
| Non-editing teacher | 0 |
| Display all course contact roles | 0 |
And I press "Save changes"
When 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"
Then I should see "Teacher 1" in the ".teachers" "css_element"
And I should not see "Teacher 2" in the ".teachers" "css_element"
And I should not see "Manager 1" in the ".teachers" "css_element"
When 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"
Then I should see "Teacher 1" in the ".teachers" "css_element"
And I should not see "Teacher 2" in the ".teachers" "css_element"
And I should not see "Manager 1" in the ".teachers" "css_element"
When 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"
Then I should see "Teacher 1" in the ".teachers" "css_element"
And I should not see "Teacher 2" in the ".teachers" "css_element"
And I should not see "Manager 1" in the ".teachers" "css_element"
When 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"
Then I should see "Teacher 1" in the ".teachers" "css_element"
And I should not see "Teacher 2" in the ".teachers" "css_element"
And I should not see "Manager 1" in the ".teachers" "css_element"
Scenario: Test course contact roles without displaying all roles
Given I log in as "admin"
And I navigate to "Appearance > Courses" in site administration
And I set the following fields to these values:
| Manager | 0 |
| Teacher | 1 |
| Non-editing teacher | 1 |
| Display all course contact roles | 0 |
And I press "Save changes"
When 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"
Then I should see "Teacher 1" in the ".teachers" "css_element"
And I should see "Teacher 2" in the ".teachers" "css_element"
And I should see "Teacher 3" in the ".teachers" "css_element"
And I should see "Teacher: Teacher 1" in the ".teachers" "css_element"
And I should not see "Teacher, Non-editing teacher: Teacher 1" in the ".teachers" "css_element"
And I should not see "Manager 1" in the ".teachers" "css_element"
Scenario: Test course contact roles with displaying all roles and standard sorting
Given I log in as "admin"
And I navigate to "Appearance > Courses" in site administration
And I set the following fields to these values:
| Manager | 0 |
| Teacher | 1 |
| Non-editing teacher | 1 |
| Display all course contact roles | 1 |
And I press "Save changes"
When 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"
Then I should see "Teacher 1" in the ".teachers" "css_element"
And I should see "Teacher 2" in the ".teachers" "css_element"
And I should see "Teacher 3" in the ".teachers" "css_element"
And I should see "Teacher, Non-editing teacher: Teacher 1" in the ".teachers" "css_element"
And I should not see "Teacher: Teacher 1" in the ".teachers" "css_element"
And I should not see "Manager 1" in the ".teachers" "css_element"
And I should see teacher "Teacher 1" before "Teacher 3" in the course contact listing
And I should see teacher "Teacher 3" before "Teacher 2" in the course contact listing
And I should not see teacher "Teacher 1" after "Teacher 3" in the course contact listing
And I should not see teacher "Teacher 3" after "Teacher 2" in the course contact listing
Scenario: Test course contact roles with displaying all roles and modified sorting
Given I log in as "admin"
And I navigate to "Appearance > Courses" in site administration
And I set the following fields to these values:
| Manager | 0 |
| Teacher | 1 |
| Non-editing teacher | 1 |
| Display all course contact roles | 1 |
And I press "Save changes"
And I navigate to "Users > Permissions > Define roles" in site administration
And I click on "Move up" "link" in the "//td[text()[contains(.,'Non-editing teacher')]]/parent::tr/td[contains(@class, 'lastcol')]" "xpath_element"
When 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"
Then I should see "Teacher 1" in the ".teachers" "css_element"
And I should see "Teacher 2" in the ".teachers" "css_element"
And I should see "Teacher 3" in the ".teachers" "css_element"
And I should see "Non-editing teacher, Teacher: Teacher 1" in the ".teachers" "css_element"
And I should not see "Non-editing teacher: Teacher 1" in the ".teachers" "css_element"
And I should not see "Manager 1" in the ".teachers" "css_element"
And I should see teacher "Teacher 1" before "Teacher 2" in the course contact listing
And I should see teacher "Teacher 2" before "Teacher 3" in the course contact listing
And I should not see teacher "Teacher 1" after "Teacher 2" in the course contact listing
And I should not see teacher "Teacher 2" after "Teacher 3" in the course contact listing

View File

@ -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);

View File

@ -1,7 +1,13 @@
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 ===
* There is a new capability 'moodle/course:setforcedlanguage' to control which users can force the course
language; create_course and update_course functions delegate access control to the caller code; if you
are calling those functions you may be interested in checking if the logged in user has 'moodle/course:setforcedlanguage' capability.

View File

@ -382,6 +382,8 @@ $string['cookiesecure'] = 'Secure cookies only';
$string['country'] = 'Default country';
$string['coursecontact'] = 'Course contacts';
$string['coursecontact_desc'] = 'This setting allows you to control who appears on the course description. Users need to have at least one of these roles in a course to be shown on the course description for that course.';
$string['coursecontactduplicates'] = 'Display all course contact roles';
$string['coursecontactduplicates_desc'] = 'If enabled, users with more than one of the selected course contact roles will be displayed in the course description with each of their roles. Otherwise, they will be displayed with only one role (whichever is listed highest in \'Define roles\' in the Site administration).';
$string['coursegraceperiodafter'] = 'Grace period for past courses';
$string['coursegraceperiodbefore'] = 'Grace period for future courses';
$string['courselistshortnames'] = 'Display extended course names';