mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-01-29 11:46:19 +01:00
Code Issues Projects Releases Wiki Activity

MDL-65169 core_message: user search respects profile visibility

Browse Source 
Two modes of visibility checking are supported, depending on the value
of the 'messagingallusers' site setting (site-wide messaging):
- If site-wide messaging is enabled, a user may only be returned in
search results if the searching user can view their profile somewhere
(either the site profile or any course profile).
- If site-wide messaging is disabled, a user may only be returned in
search results if the searching user shares a course with them and can
view their course profile in the shared course.
This commit is contained in:
Jake Dallimore 2019-03-28 11:51:07 +08:00
parent 31d3c76602
commit 3edac09063
9 changed files with 323 additions and 138 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

107
message/classes/api.php
View File

@ -352,9 +352,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
@ -365,73 +370,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)) {

33
message/classes/helper.php
View File

@ -772,4 +772,37 @@ class helper {
return $renderer->render_from_template($template, $templatecontext);
}
/**
* 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 [];
}
}

71
message/tests/api_test.php
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);
}
/**
@ -653,12 +660,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);
}
/**

15
message/tests/behat/message_delete_conversation.feature
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"

11
message/tests/behat/message_drawer_manage_contacts.feature
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"

49
message/tests/behat/message_manage_preferences.feature
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,21 +81,22 @@ 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"
Scenario: Allow send a message using Enter button
Scenario: Sending a message when 'User enter to send' is enabled
Given I log in as "student1"
And I open messaging
And I select "Student 2" user in messaging
@ -81,7 +104,7 @@ Feature: Manage preferences
And I press key "13" in "//textarea[@data-region='send-message-txt']" "xpath_element"
Then I should see "Hi!" in the "//*[@data-region='message-drawer']//div[@data-region='content-message-container']" "xpath_element"
Scenario: No allow to send a messade using Enter button
Scenario: Sending a message after 'Use enter to send' is disabled
Given I log in as "student1"
And I open messaging
And I open messaging settings preferences

58
message/tests/externallib_test.php
View File

@ -2554,11 +2554,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;
@ -2568,6 +2569,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);
@ -2578,6 +2581,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]);
@ -2619,7 +2627,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']);
@ -2639,11 +2648,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;
@ -2653,6 +2663,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);
@ -2663,6 +2675,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]);
@ -2704,24 +2721,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']);
}
/**
@ -2907,12 +2919,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']);
}
/**

110
message/tests/helper_test.php
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.
}
}

7
user/tests/behat/view_full_profile.feature
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.