L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'MDL-65169-36' of https://github.com/snake/moodle into MOODLE_36_STABLE

Browse Source
This commit is contained in:
Sara Arjona
2019-05-02 15:29:22 +02:00
parent 687af3bae7 a1afcb2e7a
commit c487276abc
10 changed files with 344 additions and 148 deletions
Download Patch File Download Diff File Expand all files Collapse all files
message/classes/api.php
+46 -61
View File
@@ -332,9 +332,14 @@ class api {
}
// Let's get those non-contacts.
// If site wide messaging is enabled, we just fetch any matched users which are non-contacts.
if ($CFG->messagingallusers) {
$sql = "SELECT u.id
// Because we can't achieve all the required visibility checks in SQL, we'll iterate through the non-contact records
// and stop once we have enough matching the 'visible' criteria.
// TODO: MDL-63983 - Improve the performance of non-contact searches when site-wide messaging is disabled (default).
// Use a local generator to achieve this iteration.
$getnoncontactusers = function ($limitfrom = 0, $limitnum = 0) use($fullname, $exclude, $params, $excludeparams) {
global $DB;
$sql = "SELECT u.*
FROM {user} u
WHERE u.deleted = 0
AND u.confirmed = 1
@@ -345,73 +350,53 @@ class api {
WHERE (mc.userid = u.id AND mc.contactid = :userid1)
OR (mc.userid = :userid2 AND mc.contactid = u.id))
ORDER BY " . $DB->sql_fullname();
while ($records = $DB->get_records_sql($sql, $params + $excludeparams, $limitfrom, $limitnum)) {
yield $records;
$limitfrom += $limitnum;
}
};
$foundusers = $DB->get_records_sql($sql, $params + $excludeparams, $limitfrom, $limitnum);
} else {
require_once($CFG->dirroot . '/user/lib.php');
// If site-wide messaging is disabled, then we should only be able to search for users who we are allowed to see.
// Because we can't achieve all the required visibility checks in SQL, we'll iterate through the non-contact records
// and stop once we have enough matching the 'visible' criteria.
// TODO: MDL-63983 - Improve the performance of non-contact searches when site-wide messaging is disabled (default).
// Fetch in batches of $limitnum * 2 to improve the chances of matching a user without going back to the DB.
// The generator cannot function without a sensible limiter, so set one if this is not set.
$batchlimit = ($limitnum == 0) ? 20 : $limitnum;
// Use a local generator to achieve this iteration.
$getnoncontactusers = function ($limitfrom = 0, $limitnum = 0) use($fullname, $exclude, $params, $excludeparams) {
global $DB;
$sql = "SELECT u.*
FROM {user} u
WHERE u.deleted = 0
AND u.confirmed = 1
AND " . $DB->sql_like($fullname, ':search', false) . "
AND u.id $exclude
AND NOT EXISTS (SELECT mc.id
FROM {message_contacts} mc
WHERE (mc.userid = u.id AND mc.contactid = :userid1)
OR (mc.userid = :userid2 AND mc.contactid = u.id))
ORDER BY " . $DB->sql_fullname();
while ($records = $DB->get_records_sql($sql, $params + $excludeparams, $limitfrom, $limitnum)) {
yield $records;
$limitfrom += $limitnum;
}
};
// We need to make the offset param work with the generator.
// Basically, if we want to get say 10 records starting at the 40th record, we need to see 50 records and return only
// those after the 40th record. We can never pass the method's offset param to the generator as we need to manage the
// position within those valid records ourselves.
// See MDL-63983 dealing with performance improvements to this area of code.
$noofvalidseenrecords = 0;
$returnedusers = [];
foreach ($getnoncontactusers(0, $batchlimit) as $users) {
foreach ($users as $id => $user) {
// User visibility checks: only return users who are visible to the user performing the search.
// Which visibility check to use depends on the 'messagingallusers' (site wide messaging) setting:
// - If enabled, return matched users whose profiles are visible to the current user anywhere (site or course).
// - If disabled, only return matched users whose course profiles are visible to the current user.
$userdetails = \core_message\helper::search_get_user_details($user);
// Fetch in batches of $limitnum * 2 to improve the chances of matching a user without going back to the DB.
// The generator cannot function without a sensible limiter, so set one if this is not set.
$batchlimit = ($limitnum == 0) ? 20 : $limitnum;
// We need to make the offset param work with the generator.
// Basically, if we want to get say 10 records starting at the 40th record, we need to see 50 records and return only
// those after the 40th record. We can never pass the method's offset param to the generator as we need to manage the
// position within those valid records ourselves.
// See MDL-63983 dealing with performance improvements to this area of code.
$noofvalidseenrecords = 0;
$returnedusers = [];
foreach ($getnoncontactusers(0, $batchlimit) as $users) {
foreach ($users as $id => $user) {
$userdetails = \user_get_user_details_courses($user);
// Return the user only if the searched field is returned.
// Otherwise it means that the $USER was not allowed to search the returned user.
if (!empty($userdetails) and !empty($userdetails['fullname'])) {
// We know we've matched, but only save the record if it's within the offset area we need.
if ($limitfrom == 0) {
// No offset specified, so just save.
// Return the user only if the searched field is returned.
// Otherwise it means that the $USER was not allowed to search the returned user.
if (!empty($userdetails) and !empty($userdetails['fullname'])) {
// We know we've matched, but only save the record if it's within the offset area we need.
if ($limitfrom == 0) {
// No offset specified, so just save.
$returnedusers[$id] = $user;
} else {
// There is an offset in play.
// If we've passed enough records already (> offset value), then we can save this one.
if ($noofvalidseenrecords >= $limitfrom) {
$returnedusers[$id] = $user;
} else {
// There is an offset in play.
// If we've passed enough records already (> offset value), then we can save this one.
if ($noofvalidseenrecords >= $limitfrom) {
$returnedusers[$id] = $user;
}
}
if (count($returnedusers) == $limitnum) {
break 2;
}
$noofvalidseenrecords++;
}
if (count($returnedusers) == $limitnum) {
break 2;
}
$noofvalidseenrecords++;
}
}
$foundusers = $returnedusers;
}
$foundusers = $returnedusers;
$noncontacts = [];
if (!empty($foundusers)) {
message/classes/helper.php
+33
View File
@@ -638,4 +638,37 @@ class helper {
}
return $tmp;
}
/**
* Returns user details for a user, if they are visible to the current user in the message search.
*
* This method checks the visibility of a user specifically for the purpose of inclusion in the message search results.
* Visibility depends on the site-wide messaging setting 'messagingallusers':
* If enabled, visibility depends only on the core notion of visibility; a visible site or course profile.
* If disabled, visibility requires that the user be sharing a course with the searching user, and have a visible profile there.
* The current user is always returned.
*
* @param \stdClass $user
* @return array the array of userdetails, if visible, or an empty array otherwise.
*/
public static function search_get_user_details(\stdClass $user) : array {
global $CFG, $USER;
require_once($CFG->dirroot . '/user/lib.php');
if ($CFG->messagingallusers || $user->id == $USER->id) {
return \user_get_user_details_courses($user) ?? []; // This checks visibility of site and course profiles.
} else {
// Messaging specific: user must share a course with the searching user AND have a visible profile there.
$sharedcourses = enrol_get_shared_courses($USER, $user);
foreach ($sharedcourses as $course) {
if (user_can_view_profile($user, $course)) {
$userdetails = user_get_user_details($user, $course);
if (!is_null($userdetails)) {
return $userdetails;
}
}
}
}
return [];
}
}
message/tests/api_test.php
+38 -33
View File
@@ -314,11 +314,12 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
* profile.
*/
public function test_message_search_users_messagingallusers_disabled() {
global $DB;
$this->resetAfterTest();
// Create some users.
$users = [];
foreach (range(1, 7) as $i) {
foreach (range(1, 8) as $i) {
$user = new stdClass();
$user->firstname = ($i == 4) ? 'User' : 'User search'; // Ensure the fourth user won't match the search term.
$user->lastname = $i;
@@ -328,6 +329,8 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
// Enrol a few users in the same course, but leave them as non-contacts.
$course1 = $this->getDataGenerator()->create_course();
$course2 = $this->getDataGenerator()->create_course();
$this->setAdminUser();
$this->getDataGenerator()->enrol_user($users[1]->id, $course1->id);
$this->getDataGenerator()->enrol_user($users[6]->id, $course1->id);
@@ -338,6 +341,11 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
\core_message\api::add_contact($users[3]->id, $users[1]->id);
\core_message\api::add_contact($users[1]->id, $users[4]->id);
// Enrol a user as a teacher in the course, and make the teacher role a course contact role.
$this->getDataGenerator()->enrol_user($users[8]->id, $course2->id, 'editingteacher');
$teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher'));
set_config('coursecontact', $teacherrole->id);
// Create individual conversations between some users, one contact and one non-contact.
$ic1 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL,
[$users[1]->id, $users[2]->id]);
@@ -374,7 +382,8 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
$this->assertCount(0, $contacts[1]->conversations);
// Check that we retrieved the correct non-contacts.
// When site wide messaging is disabled, we expect to see only those users whose profiles we can view.
// When site wide messaging is disabled, we expect to see only those users who we share a course with and whose profiles
// are visible in that course. This excludes users like course contacts.
$this->assertCount(2, $noncontacts);
$this->assertEquals($users[6]->id, $noncontacts[0]->id);
$this->assertEquals($users[7]->id, $noncontacts[1]->id);
@@ -391,15 +400,16 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
/**
* Tests searching for users when site-wide messaging is enabled.
*
* This test verifies that any contacts are returned, as well as any non-contacts, regardless of whether the searching user
* can view their respective profile.
* This test verifies that any contacts are returned, as well as any non-contacts,
* provided the searching user can view their profile.
*/
public function test_message_search_users_messagingallusers_enabled() {
global $DB;
$this->resetAfterTest();
// Create some users.
$users = [];
foreach (range(1, 8) as $i) {
foreach (range(1, 9) as $i) {
$user = new stdClass();
$user->firstname = ($i == 4) ? 'User' : 'User search'; // Ensure the fourth user won't match the search term.
$user->lastname = $i;
@@ -407,18 +417,25 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
$users[$i] = $user;
}
// Enrol a few users in the same course, but leave them as non-contacts.
$course1 = $this->getDataGenerator()->create_course();
$coursecontext = \context_course::instance($course1->id);
// Enrol a few users in the same course, but leave them as non-contacts.
$this->setAdminUser();
$this->getDataGenerator()->enrol_user($users[1]->id, $course1->id);
$this->getDataGenerator()->enrol_user($users[6]->id, $course1->id);
$this->getDataGenerator()->enrol_user($users[7]->id, $course1->id);
$this->getDataGenerator()->enrol_user($users[1]->id, $course1->id, 'student');
$this->getDataGenerator()->enrol_user($users[6]->id, $course1->id, 'student');
$this->getDataGenerator()->enrol_user($users[7]->id, $course1->id, 'student');
// Add some other users as contacts.
\core_message\api::add_contact($users[1]->id, $users[2]->id);
\core_message\api::add_contact($users[3]->id, $users[1]->id);
\core_message\api::add_contact($users[1]->id, $users[4]->id);
// Enrol a user as a teacher in the course, and make the teacher role a course contact role.
$this->getDataGenerator()->enrol_user($users[9]->id, $course1->id, 'editingteacher');
$teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher'));
set_config('coursecontact', $teacherrole->id);
// Create individual conversations between some users, one contact and one non-contact.
$ic1 = \core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL,
[$users[1]->id, $users[2]->id]);
@@ -455,25 +472,15 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
$this->assertCount(0, $contacts[1]->conversations);
// Check that we retrieved the correct non-contacts.
// If site wide messaging is enabled, we expect to be able to search for any users.
$this->assertCount(4, $noncontacts);
$this->assertEquals($users[5]->id, $noncontacts[0]->id);
$this->assertEquals($users[6]->id, $noncontacts[1]->id);
$this->assertEquals($users[7]->id, $noncontacts[2]->id);
$this->assertEquals($users[8]->id, $noncontacts[3]->id);
// Verify the correct conversations were returned for the non-contacts.
$this->assertCount(0, $noncontacts[0]->conversations);
// If site wide messaging is enabled, we expect to only be able to search for users whose profiles we can view.
// In this case, as a student, that's the course contact for course2 and those noncontacts sharing a course with user1.
$this->assertCount(3, $noncontacts);
$this->assertEquals($users[6]->id, $noncontacts[0]->id);
$this->assertEquals($users[7]->id, $noncontacts[1]->id);
$this->assertEquals($users[9]->id, $noncontacts[2]->id);
$this->assertCount(1, $noncontacts[0]->conversations);
$this->assertCount(1, $noncontacts[1]->conversations);
$this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL,
$noncontacts[1]->conversations[$ic2->id]->type);
$this->assertCount(1, $noncontacts[2]->conversations);
$this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, $noncontacts[2]->conversations[$gc1->id]->type);
$this->assertCount(1, $noncontacts[3]->conversations);
$this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, $noncontacts[3]->conversations[$gc1->id]->type);
$this->assertCount(0, $noncontacts[2]->conversations);
}
/**
@@ -624,12 +631,10 @@ class core_message_api_testcase extends core_message_messagelib_testcase {
$this->assertEquals($users[3]->id, $contacts[1]->id);
// Check that we retrieved the correct non-contacts.
// Site-wide messaging is disabled, but since we can see all users, we expect to be able to search for any users.
$this->assertCount(4, $noncontacts);
$this->assertEquals($users[5]->id, $noncontacts[0]->id);
$this->assertEquals($users[6]->id, $noncontacts[1]->id);
$this->assertEquals($users[7]->id, $noncontacts[2]->id);
$this->assertEquals($users[8]->id, $noncontacts[3]->id);
// Site-wide messaging is disabled, so we expect to be able to search for any users whose profiles we can view.
$this->assertCount(2, $noncontacts);
$this->assertEquals($users[6]->id, $noncontacts[0]->id);
$this->assertEquals($users[7]->id, $noncontacts[1]->id);
}
/**
message/tests/behat/behat_message.php
+22 -12
View File
@@ -109,18 +109,7 @@ class behat_message extends behat_base {
public function i_select_user_in_messaging($userfullname) {
$this->i_open_messaging();
$this->execute('behat_general::i_click_on', [get_string('search', 'core'), 'field']);
$this->execute('behat_forms::i_set_the_field_with_xpath_to',
[
"//*[@data-region='message-drawer']//input[@data-region='search-input']",
$this->escape($userfullname)
]
);
$this->execute('behat_general::i_click_on', ['[data-action="search"]', 'css_element']);
$this->execute('behat_general::wait_until_the_page_is_ready');
$this->i_search_for_string_in_messaging($userfullname);
// Need to limit the click to the search results because the 'view-contact-profile' elements
// can occur in two separate divs on the page.
@@ -136,6 +125,27 @@ class behat_message extends behat_base {
$this->execute('behat_general::wait_until_the_page_is_ready');
}
/**
* Search for a string using the messaging search.
*
* @Given /^I search for "(?P<string>(?:[^"]|\\")*)" in messaging$/
* @param string $string the search string.
*/
public function i_search_for_string_in_messaging($string) {
$this->execute('behat_general::i_click_on', [get_string('search', 'core'), 'field']);
$this->execute('behat_forms::i_set_the_field_with_xpath_to',
[
"//*[@data-region='message-drawer']//input[@data-region='search-input']",
$this->escape($string)
]
);
$this->execute('behat_general::i_click_on', ['[data-action="search"]', 'css_element']);
$this->execute('behat_general::wait_until_the_page_is_ready');
}
/**
* Sends a message to the specified user from the logged user. The user full name should contain the first and last names.
*
message/tests/behat/message_delete_conversation.feature
+11 -4
View File
@@ -9,6 +9,13 @@ Feature: Message delete conversations
| username | firstname | lastname | email |
| student1 | Student | 1 | student1@example.com |
| student2 | Student | 2 | student2@example.com |
And the following "courses" exist:
| name | shortname |
| course1 | C1 |
And the following "course enrolments" exist:
| user | course | role |
| student1 | C1 | student |
| student2 | C1 | student |
And the following config values are set as admin:
| messaging | 1 |
| messagingallusers | 1 |
@@ -52,7 +59,7 @@ Feature: Message delete conversations
And I should see "Hi!" in the "Student 2" "group_message_conversation"
And I should see "##today##j F##" in the "Student 2" "group_message_conversation"
Scenario: Delete a stared conversation
Scenario: Delete a starred conversation
Given the following "favourite conversations" exist:
| user | contact |
| student1 | student2 |
@@ -77,7 +84,7 @@ Feature: Message delete conversations
And I should see "What do you need?" in the "Student 1" "group_message_conversation"
And I should see "##today##j F##" in the "Student 1" "group_message_conversation"
Scenario: Cancel deleting a stared conversation
Scenario: Cancel deleting a starred conversation
Given the following "favourite conversations" exist:
| user | contact |
| student1 | student2 |
@@ -95,7 +102,7 @@ Feature: Message delete conversations
And I should see "Hi!" in the "Student 2" "group_message_conversation"
And I should see "##today##j F##" in the "Student 2" "group_message_conversation"
Scenario: Check a deleted stared conversation is still stared
Scenario: Check a deleted starred conversation is still starred
Given the following "favourite conversations" exist:
| user | contact |
| student1 | student2 |
@@ -113,4 +120,4 @@ Feature: Message delete conversations
And I send "Hi!" message to "Student 2" user
And I go back in "view-conversation" message drawer
And I go back in "view-search" message drawer
And I should see "Student 2" in the "//*[@data-region='message-drawer']//div[@data-region='view-overview-favourites']" "xpath_element"
And I should see "Student 2" in the "//*[@data-region='message-drawer']//div[@data-region='view-overview-favourites']" "xpath_element"
message/tests/behat/message_drawer_manage_contacts.feature
+10 -1
View File
@@ -11,6 +11,15 @@ Feature: Manage contacts
| student2 | Student | 2 | student2@example.com |
| student3 | Student | 3 | student3@example.com |
| student4 | Student | 4 | student4@example.com |
And the following "courses" exist:
| fullname | shortname |
| course1 | C1 |
And the following "course enrolments" exist:
| user | course | role |
| student1 | C1 | student |
| student2 | C1 | student |
| student3 | C1 | student |
| student4 | C1 | student |
And the following "message contacts" exist:
| user | contact |
| student1 | student2 |
@@ -80,4 +89,4 @@ Feature: Manage contacts
And I click on "Remove from contacts" "link"
And I click on "Remove" "button"
And I go back in "view-conversation" message drawer
And I should see "No contacts" in the "//*[@data-region='empty-message-container']" "xpath_element"
And I should see "No contacts" in the "//*[@data-region='empty-message-container']" "xpath_element"
message/tests/behat/message_manage_preferences.feature
+35 -11
View File
@@ -5,6 +5,7 @@ Feature: Manage preferences
I need to be able to update my messaging preferences
Background:
# Note: This course is using separate groups mode.
Given the following "courses" exist:
| fullname | shortname | category | groupmode |
| Course 1 | C1 | 0 | 1 |
@@ -14,14 +15,20 @@ Feature: Manage preferences
| student2 | Student | 2 | student2@example.com |
| student3 | Student | 3 | student3@example.com |
| student4 | Student | 4 | student4@example.com |
| student5 | Student | 5 | student5@example.com |
And the following "course enrolments" exist:
| user | course | role |
| student1 | C1 | student |
| student2 | C1 | student |
| student3 | C1 | student |
| student4 | C1 | student |
And the following "groups" exist:
| name | course | idnumber | enablemessaging |
| Group 1 | C1 | G1 | 1 |
And the following "group members" exist:
| user | group |
| student1 | G1 |
| student4 | G1 |
And the following "message contacts" exist:
| user | contact |
| student1 | student2 |
@@ -29,7 +36,8 @@ Feature: Manage preferences
| messaging | 1 |
| messagingallusers | 1 |
Scenario: Allow send me a message whe you are a contact and the prefrence is my contacts only
# Recipient has 'My contacts only' set.
Scenario: Allow sending a message when you are a contact
Given I log in as "student1"
And I open messaging
And I open messaging settings preferences
@@ -40,7 +48,20 @@ Feature: Manage preferences
And I send "Hi!" message to "Student 1" user
And I should see "Hi!" in the "//*[@data-region='message-drawer']//div[@data-region='content-message-container']" "xpath_element"
Scenario: Not allowed to send a message if you are not contact to the sender or you are not in the same course
# Recipient has 'My contacts and anyone in my courses' set.
Scenario: Disallow sending a message if you are neither contacts with the recipient nor do you share a course
Given I log in as "student1"
And I open messaging
And I open messaging settings preferences
When I click on "//label[text()[contains(.,'My contacts and anyone in my courses')]]" "xpath_element"
And I log out
Then I log in as "student5"
And I open messaging
And I search for "Student 1" in messaging
And I should see "No results"
# Recipient has 'My contacts and anyone in my courses' set.
Scenario: Allow sending a message if you share a group in a shared course
Given I log in as "student1"
And I open messaging
And I open messaging settings preferences
@@ -48,10 +69,11 @@ Feature: Manage preferences
And I log out
Then I log in as "student4"
And I open messaging
And I select "Student 1" user in messaging
And I should see "You are unable to message this user" in the "//*[@data-region='content-messages-footer-unable-to-message']" "xpath_element"
And I send "Hi!" message to "Student 1" user
And I should see "Hi!" in the "//*[@data-region='message-drawer']//div[@data-region='content-message-container']" "xpath_element"
Scenario: Allow send me a message whe you are a contact and the prefrence is my contacts only
# Recipient has 'My contacts and anyone in my courses' set.
Scenario: Disallow sending a message if you are neither a contact, nor are in the same group in a shared course
Given I log in as "student1"
And I open messaging
And I open messaging settings preferences
@@ -59,16 +81,18 @@ Feature: Manage preferences
And I log out
Then I log in as "student3"
And I open messaging
And I send "Hi!" message to "Student 1" user
And I should see "Hi!" in the "//*[@data-region='message-drawer']//div[@data-region='content-message-container']" "xpath_element"
And I search for "Student 1" in messaging
And I should see "No results"
Scenario: Allowed to send a message if you are not contact to the sender and you are not in the same course
# Recipient has 'Anyone on the site' set. Only users whose profiles are visible can be found via the search.
Scenario: Disallow sending a message if you are neither a contact nor do you share a course with the user.
Given I log in as "student1"
And I open messaging
And I open messaging settings preferences
When I click on "//label[text()[contains(.,'Anyone on the site')]]" "xpath_element"
And I log out
Then I log in as "student4"
Then I log in as "student5"
And I open messaging
And I send "Hi!" message to "Student 1" user
And I should see "Hi!" in the "//*[@data-region='message-drawer']//div[@data-region='content-message-container']" "xpath_element"
And I search for "Student 1" in messaging
And I should see "No results"
message/tests/externallib_test.php
+34 -24
View File
@@ -2387,11 +2387,12 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
* profile.
*/
public function test_message_search_users_messagingallusers_disabled() {
global $DB;
$this->resetAfterTest();
// Create some users.
$users = [];
foreach (range(1, 7) as $i) {
foreach (range(1, 8) as $i) {
$user = new stdClass();
$user->firstname = ($i == 4) ? 'User' : 'User search'; // Ensure the fourth user won't match the search term.
$user->lastname = $i;
@@ -2401,6 +2402,8 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
// Enrol a few users in the same course, but leave them as non-contacts.
$course1 = $this->getDataGenerator()->create_course();
$course2 = $this->getDataGenerator()->create_course();
$this->setAdminUser();
$this->getDataGenerator()->enrol_user($users[1]->id, $course1->id);
$this->getDataGenerator()->enrol_user($users[6]->id, $course1->id);
@@ -2411,6 +2414,11 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
\core_message\api::add_contact($users[3]->id, $users[1]->id);
\core_message\api::add_contact($users[1]->id, $users[4]->id);
// Enrol a user as a teacher in the course, and make the teacher role a course contact role.
$this->getDataGenerator()->enrol_user($users[8]->id, $course2->id, 'editingteacher');
$teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher'));
set_config('coursecontact', $teacherrole->id);
// Create individual conversations between some users, one contact and one non-contact.
\core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL,
[$users[1]->id, $users[2]->id]);
@@ -2452,7 +2460,8 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
$this->assertCount(0, $contacts[1]['conversations']);
// Check that we retrieved the correct non-contacts.
// When site wide messaging is disabled, we expect to see only those users whose profiles we can view.
// When site wide messaging is disabled, we expect to see only those users who we share a course with and whose profiles
// are visible in that course. This excludes users like course contacts.
$this->assertCount(2, $noncontacts);
$this->assertEquals($users[6]->id, $noncontacts[0]['id']);
$this->assertEquals($users[7]->id, $noncontacts[1]['id']);
@@ -2472,11 +2481,12 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
* can view their respective profile.
*/
public function test_message_search_users_messagingallusers_enabled() {
global $DB;
$this->resetAfterTest();
// Create some users.
$users = [];
foreach (range(1, 8) as $i) {
foreach (range(1, 9) as $i) {
$user = new stdClass();
$user->firstname = ($i == 4) ? 'User' : 'User search'; // Ensure the fourth user won't match the search term.
$user->lastname = $i;
@@ -2486,6 +2496,8 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
// Enrol a few users in the same course, but leave them as non-contacts.
$course1 = $this->getDataGenerator()->create_course();
$course2 = $this->getDataGenerator()->create_course();
$this->setAdminUser();
$this->getDataGenerator()->enrol_user($users[1]->id, $course1->id);
$this->getDataGenerator()->enrol_user($users[6]->id, $course1->id);
@@ -2496,6 +2508,11 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
\core_message\api::add_contact($users[3]->id, $users[1]->id);
\core_message\api::add_contact($users[1]->id, $users[4]->id);
// Enrol a user as a teacher in the course, and make the teacher role a course contact role.
$this->getDataGenerator()->enrol_user($users[9]->id, $course2->id, 'editingteacher');
$teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher'));
set_config('coursecontact', $teacherrole->id);
// Create individual conversations between some users, one contact and one non-contact.
\core_message\api::create_conversation(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL,
[$users[1]->id, $users[2]->id]);
@@ -2537,24 +2554,19 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
$this->assertCount(0, $contacts[1]['conversations']);
// Check that we retrieved the correct non-contacts.
// If site wide messaging is enabled, we expect to be able to search for any users.
$this->assertCount(4, $noncontacts);
$this->assertEquals($users[5]->id, $noncontacts[0]['id']);
$this->assertEquals($users[6]->id, $noncontacts[1]['id']);
$this->assertEquals($users[7]->id, $noncontacts[2]['id']);
$this->assertEquals($users[8]->id, $noncontacts[3]['id']);
// If site wide messaging is enabled, we expect to be able to search for any users whose profiles we can view.
// In this case, as a student, that's the course contact for course2 and those noncontacts sharing a course with user1.
$this->assertCount(3, $noncontacts);
$this->assertEquals($users[6]->id, $noncontacts[0]['id']);
$this->assertEquals($users[7]->id, $noncontacts[1]['id']);
$this->assertEquals($users[9]->id, $noncontacts[2]['id']);
// Verify the correct conversations were returned for the non-contacts.
$this->assertCount(0, $noncontacts[0]['conversations']);
$this->assertCount(1, $noncontacts[0]['conversations']);
$this->assertCount(1, $noncontacts[1]['conversations']);
$this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, $noncontacts[1]['conversations'][0]['type']);
$this->assertCount(1, $noncontacts[2]['conversations']);
$this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, $noncontacts[2]['conversations'][0]['type']);
$this->assertCount(1, $noncontacts[3]['conversations']);
$this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, $noncontacts[3]['conversations'][0]['type']);
$this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_INDIVIDUAL, $noncontacts[0]['conversations'][0]['type']);
$this->assertEquals(\core_message\api::MESSAGE_CONVERSATION_TYPE_GROUP, $noncontacts[1]['conversations'][0]['type']);
$this->assertCount(0, $noncontacts[2]['conversations']);
}
/**
@@ -2710,12 +2722,10 @@ class core_message_externallib_testcase extends externallib_advanced_testcase {
$this->assertEquals($users[3]->id, $contacts[1]['id']);
// Check that we retrieved the correct non-contacts.
// Site-wide messaging is disabled, but since we can see all users, we expect to be able to search for any users.
$this->assertCount(4, $noncontacts);
$this->assertEquals($users[5]->id, $noncontacts[0]['id']);
$this->assertEquals($users[6]->id, $noncontacts[1]['id']);
$this->assertEquals($users[7]->id, $noncontacts[2]['id']);
$this->assertEquals($users[8]->id, $noncontacts[3]['id']);
// Site-wide messaging is disabled, so we expect to be able to search for any users whose profile we can view.
$this->assertCount(2, $noncontacts);
$this->assertEquals($users[6]->id, $noncontacts[0]['id']);
$this->assertEquals($users[7]->id, $noncontacts[1]['id']);
}
/**
message/tests/helper_test.php
+110
View File
@@ -67,4 +67,114 @@ class core_message_helper_testcase extends advanced_testcase {
$this->assertEquals($user4->id, array_shift($memberinfo)->id);
$this->assertEquals($user2->id, array_shift($memberinfo)->id);
}
/**
* Test search_get_user_details returns the correct profile data when $CFG->messagingallusers is disabled.
*/
public function test_search_get_user_details_sitewide_disabled() {
global $DB;
set_config('messagingallusers', false);
// Two students sharing course 1, visible profile within course (no groups).
$user1 = $this->getDataGenerator()->create_user();
$user2 = $this->getDataGenerator()->create_user();
$course1 = $this->getDataGenerator()->create_course((object) ['groupmode' => 0]);
$this->getDataGenerator()->enrol_user($user1->id, $course1->id);
$this->getDataGenerator()->enrol_user($user2->id, $course1->id);
// A teacher in course 1.
$user3 = $this->getDataGenerator()->create_user();
$this->getDataGenerator()->enrol_user($user3->id, $course1->id, 'editingteacher');
// Two students sharing course 2, separate groups (profiles not visible to one another).
// Note: no groups are created here, but separate groups mode alone is enough to restrict profile access.
$user4 = $this->getDataGenerator()->create_user();
$user5 = $this->getDataGenerator()->create_user();
$course2 = $this->getDataGenerator()->create_course((object) ['groupmode' => 1]);
$this->getDataGenerator()->enrol_user($user4->id, $course2->id);
$this->getDataGenerator()->enrol_user($user5->id, $course2->id);
// A teacher in course 2.
$user6 = $this->getDataGenerator()->create_user();
$this->getDataGenerator()->enrol_user($user6->id, $course2->id, 'editingteacher');
// Teacher and course contact in course 3.
$user7 = $this->getDataGenerator()->create_user();
$course3 = $this->getDataGenerator()->create_course();
$this->getDataGenerator()->enrol_user($user7->id, $course3->id, 'editingteacher');
$teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher'));
// Make teachers course contacts.
set_config('coursecontact', $teacherrole->id);
// User 1 should be able to see users within their course, but not course contacts or students in other courses.
$this->setUser($user1);
$this->assertNotEmpty(\core_message\helper::search_get_user_details($user2)); // Student in same course.
$this->assertEmpty(\core_message\helper::search_get_user_details($user4)); // Student in another course.
$this->assertNotEmpty(\core_message\helper::search_get_user_details($user3)); // Teacher in same course.
$this->assertEmpty(\core_message\helper::search_get_user_details($user7)); // Teacher (course contact) in another course.
// User 3 should be able to see the teacher in their own course, but not other students in that course nor course contacts
// or students in other courses.
$this->setUser($user4);
$this->assertEmpty(\core_message\helper::search_get_user_details($user5)); // Student in same course.
$this->assertEmpty(\core_message\helper::search_get_user_details($user1)); // Student in another course.
$this->assertNotEmpty(\core_message\helper::search_get_user_details($user6)); // Teacher in same course.
$this->assertEmpty(\core_message\helper::search_get_user_details($user7)); // Teacher (course contact) in another course.
}
/**
* Test search_get_user_details returns the correct profile data when $CFG->messagingallusers is enabled.
*/
public function test_search_get_user_details_sitewide_enabled() {
global $DB;
set_config('messagingallusers', true);
// Two students sharing course 1, visible profile within course (no groups).
$user1 = $this->getDataGenerator()->create_user();
$user2 = $this->getDataGenerator()->create_user();
$course1 = $this->getDataGenerator()->create_course((object) ['groupmode' => 0]);
$this->getDataGenerator()->enrol_user($user1->id, $course1->id);
$this->getDataGenerator()->enrol_user($user2->id, $course1->id);
// A teacher in course 1.
$user3 = $this->getDataGenerator()->create_user();
$this->getDataGenerator()->enrol_user($user3->id, $course1->id, 'editingteacher');
// Two students sharing course 2, separate groups (profiles not visible to one another).
// Note: no groups are created here, but separate groups mode alone is enough to restrict profile access.
$user4 = $this->getDataGenerator()->create_user();
$user5 = $this->getDataGenerator()->create_user();
$course2 = $this->getDataGenerator()->create_course((object) ['groupmode' => 1]);
$this->getDataGenerator()->enrol_user($user4->id, $course2->id);
$this->getDataGenerator()->enrol_user($user5->id, $course2->id);
// A teacher in course 2.
$user6 = $this->getDataGenerator()->create_user();
$this->getDataGenerator()->enrol_user($user6->id, $course2->id, 'editingteacher');
// Teacher and course contact in course 3.
$user7 = $this->getDataGenerator()->create_user();
$course3 = $this->getDataGenerator()->create_course();
$this->getDataGenerator()->enrol_user($user7->id, $course3->id, 'editingteacher');
$teacherrole = $DB->get_record('role', array('shortname' => 'editingteacher'));
// Make teachers course contacts.
set_config('coursecontact', $teacherrole->id);
// User 1 should be able to see users within their course and course contacts, but not students in other courses.
$this->setUser($user1);
$this->assertNotEmpty(\core_message\helper::search_get_user_details($user2)); // Student in same course.
$this->assertEmpty(\core_message\helper::search_get_user_details($user4)); // Student in another course.
$this->assertNotEmpty(\core_message\helper::search_get_user_details($user3)); // Teacher in same course.
$this->assertNotEmpty(\core_message\helper::search_get_user_details($user7)); // Teacher (course contact) in another course.
// User 3 should be able to see the teacher in their own course, but not other students in that course nor course contacts
// or students in other courses.
$this->setUser($user4);
$this->assertEmpty(\core_message\helper::search_get_user_details($user5)); // Student in same course.
$this->assertEmpty(\core_message\helper::search_get_user_details($user1)); // Student in another course.
$this->assertNotEmpty(\core_message\helper::search_get_user_details($user6)); // Teacher in same course.
$this->assertNotEmpty(\core_message\helper::search_get_user_details($user7)); // Teacher (course contact) in another course.
}
}
user/tests/behat/view_full_profile.feature
+5 -2
View File
@@ -83,9 +83,12 @@ Feature: Access to full profiles of users
And I click on "//div[@class='userselector']/descendant::option[contains(., 'Student 3')]" "xpath_element"
And I press "Add"
And I log out
# Message search will not return a course contact unless the searcher shares a course with them,
# or site-wide messaging is enabled ($CFG->messagingallusers).
When I log in as "student1"
And I view the "Student 3" contact in the message area
Then I should see "First access to site"
And I open messaging
And I search for "Student 3" in messaging
Then I should see "No results"
@javascript
Scenario: View full profiles of someone in the same group in a course with separate groups.