diff --git a/admin/tool/dataprivacy/classes/external.php b/admin/tool/dataprivacy/classes/external.php index 3dd839e0d54..20c28f9187c 100644 --- a/admin/tool/dataprivacy/classes/external.php +++ b/admin/tool/dataprivacy/classes/external.php @@ -689,6 +689,7 @@ class external extends external_api { * @throws restricted_context_exception */ public static function get_users($query) { + global $DB; $params = external_api::validate_parameters(self::get_users_parameters(), [ 'query' => $query ]); @@ -703,15 +704,30 @@ class external extends external_api { // Exclude admins and guest user. $excludedusers = array_keys(get_admins()) + [guest_user()->id]; $sort = 'lastname ASC, firstname ASC'; - $fields = 'id, email, ' . $allusernames; - $users = get_users(true, $query, true, $excludedusers, $sort, '', '', 0, 30, $fields); + $fields = 'id,' . $allusernames; + + $extrafields = get_extra_user_fields($context); + if (!empty($extrafields)) { + $fields .= ',' . implode(',', $extrafields); + } + + list($sql, $params) = users_search_sql($query, '', false, $extrafields, $excludedusers); + $users = $DB->get_records_select('user', $sql, $params, $sort, $fields, 0, 30); + $useroptions = []; foreach ($users as $user) { - $useroptions[$user->id] = (object)[ + $useroption = (object)[ 'id' => $user->id, - 'fullname' => fullname($user), - 'email' => $user->email + 'fullname' => fullname($user) ]; + $useroption->extrafields = []; + foreach ($extrafields as $extrafield) { + $useroption->extrafields[] = (object)[ + 'name' => $extrafield, + 'value' => $user->$extrafield + ]; + } + $useroptions[$user->id] = $useroption; } return $useroptions; @@ -729,7 +745,13 @@ class external extends external_api { [ 'id' => new external_value(core_user::get_property_type('id'), 'ID of the user'), 'fullname' => new external_value(core_user::get_property_type('firstname'), 'The fullname of the user'), - 'email' => new external_value(core_user::get_property_type('email'), 'The user\'s email address', VALUE_OPTIONAL), + 'extrafields' => new external_multiple_structure( + new external_single_structure([ + 'name' => new external_value(PARAM_TEXT, 'Name of the extrafield.'), + 'value' => new external_value(PARAM_TEXT, 'Value of the extrafield.') + ] + ), 'List of extra fields', VALUE_OPTIONAL + ) ] )); } diff --git a/admin/tool/dataprivacy/templates/form-user-selector-suggestion.mustache b/admin/tool/dataprivacy/templates/form-user-selector-suggestion.mustache index 759650c4c9a..0f8443d313e 100644 --- a/admin/tool/dataprivacy/templates/form-user-selector-suggestion.mustache +++ b/admin/tool/dataprivacy/templates/form-user-selector-suggestion.mustache @@ -32,10 +32,21 @@ Example context (json): { "fullname": "Admin User", - "email": "admin@example.com" + "extrafields": [ + { + "name": "email", + "value": "admin@example.com" + }, + { + "name": "phone1", + "value": "0123456789" + } + ] } }} {{fullname}} - {{email}} + {{#extrafields}} + {{value}} + {{/extrafields}} diff --git a/admin/tool/dataprivacy/tests/behat/dataexport.feature b/admin/tool/dataprivacy/tests/behat/dataexport.feature index 1947f62f149..78751ccd8a2 100644 --- a/admin/tool/dataprivacy/tests/behat/dataexport.feature +++ b/admin/tool/dataprivacy/tests/behat/dataexport.feature @@ -6,21 +6,29 @@ Feature: Data export from the privacy API Background: Given the following "users" exist: - | username | firstname | lastname | - | victim | Victim User | 1 | - | parent | Long-suffering | Parent | + | username | firstname | lastname | institution | + | victim | Victim User | 1 | University1 | + | victim2 | Victim User | 2 | University2 | + | requester | The | Requester | University3 | + | parent | Long-suffering | Parent | | And the following "roles" exist: | shortname | name | archetype | | tired | Tired | | And the following "permission overrides" exist: - | capability | permission | role | contextlevel | reference | - | tool/dataprivacy:makedatarequestsforchildren | Allow | tired | System | | + | capability | permission | role | contextlevel | reference | + | tool/dataprivacy:makedatarequestsforchildren | Allow | tired | System | | + | tool/dataprivacy:managedatarequests | Allow | manager | System | | + | moodle/site:viewuseridentity | Prevent | manager | System | | And the following "role assigns" exist: | user | role | contextlevel | reference | | parent | tired | User | victim | + And the following "system role assigns" exist: + | user | role | contextlevel | + | requester | manager | User | And the following config values are set as admin: | contactdataprotectionofficer | 1 | tool_dataprivacy | | privacyrequestexpiry | 55 | tool_dataprivacy | + | dporoles | 1 | tool_dataprivacy | And the following data privacy "categories" exist: | name | | Site category | @@ -127,3 +135,19 @@ Feature: Data export from the privacy API And I should see "Expired" in the "Victim User 1" "table_row" And I should not see "Actions" + + @javascript + Scenario: Test search for user using extra field. + Given the following "permission overrides" exist: + | capability | permission | role | contextlevel | reference | + | moodle/site:viewuseridentity | Allow | manager | System | | + And the following config values are set as admin: + | showuseridentity | institution | + And I log in as "requester" + And I navigate to "Users > Privacy and policies > Data requests" in site administration + And I follow "New request" + And I set the field "Search" to "University1" + Then I should see "Victim User 1" + When I reload the page + And I set the field "Search" to "University2" + Then I should see "Victim User 2" diff --git a/admin/tool/dataprivacy/tests/external_test.php b/admin/tool/dataprivacy/tests/external_test.php index b17811a0409..7cb7df730b8 100644 --- a/admin/tool/dataprivacy/tests/external_test.php +++ b/admin/tool/dataprivacy/tests/external_test.php @@ -970,4 +970,143 @@ class tool_dataprivacy_external_testcase extends externallib_advanced_testcase { $this->expectException(required_capability_exception::class); $result = external::bulk_deny_data_requests([$requestid1]); } + + /** + * Test for external::get_users(), case search using non-identity field without + * facing any permission problem. + * + * @throws coding_exception + * @throws dml_exception + * @throws invalid_parameter_exception + * @throws required_capability_exception + * @throws restricted_context_exception + */ + public function test_get_users_using_using_non_identity() { + $this->resetAfterTest(); + $context = context_system::instance(); + $requester = $this->getDataGenerator()->create_user(); + $role = $this->getDataGenerator()->create_role(); + role_assign($role, $requester->id, $context); + assign_capability('tool/dataprivacy:managedatarequests', CAP_ALLOW, $role, $context); + $this->setUser($requester); + + $this->getDataGenerator()->create_user([ + 'firstname' => 'First Student' + ]); + $student2 = $this->getDataGenerator()->create_user([ + 'firstname' => 'Second Student' + ]); + + $results = external::get_users('Second'); + $this->assertCount(1, $results); + $this->assertEquals((object)[ + 'id' => $student2->id, + 'fullname' => fullname($student2), + 'extrafields' => [] + ], $results[$student2->id]); + } + + /** + * Test for external::get_users(), case search using identity field but + * don't have "moodle/site:viewuseridentity" permission. + * + * @throws coding_exception + * @throws dml_exception + * @throws invalid_parameter_exception + * @throws required_capability_exception + * @throws restricted_context_exception + */ + public function test_get_users_using_identity_without_permission() { + global $CFG; + + $this->resetAfterTest(); + $CFG->showuseridentity = 'institution'; + + // Create requester user and assign correct capability. + $context = context_system::instance(); + $requester = $this->getDataGenerator()->create_user(); + $role = $this->getDataGenerator()->create_role(); + role_assign($role, $requester->id, $context); + assign_capability('tool/dataprivacy:managedatarequests', CAP_ALLOW, $role, $context); + $this->setUser($requester); + + $this->getDataGenerator()->create_user([ + 'institution' => 'University1' + ]); + + $results = external::get_users('University1'); + $this->assertEmpty($results); + } + + /** + * Test for external::get_users(), case search using disabled identity field + * even they have "moodle/site:viewuseridentity" permission. + * + * @throws coding_exception + * @throws dml_exception + * @throws invalid_parameter_exception + * @throws required_capability_exception + * @throws restricted_context_exception + */ + public function test_get_users_using_field_not_in_identity() { + $this->resetAfterTest(); + + $context = context_system::instance(); + $requester = $this->getDataGenerator()->create_user(); + $role = $this->getDataGenerator()->create_role(); + role_assign($role, $requester->id, $context); + assign_capability('tool/dataprivacy:managedatarequests', CAP_ALLOW, $role, $context); + assign_capability('moodle/site:viewuseridentity', CAP_ALLOW, $role, $context); + $this->setUser($requester); + + $this->getDataGenerator()->create_user([ + 'institution' => 'University1' + ]); + + $results = external::get_users('University1'); + $this->assertEmpty($results); + } + + /** + * Test for external::get_users(), case search using enabled identity field + * with "moodle/site:viewuseridentity" permission. + * + * @throws coding_exception + * @throws dml_exception + * @throws invalid_parameter_exception + * @throws required_capability_exception + * @throws restricted_context_exception + */ + public function test_get_users() { + global $CFG; + $this->resetAfterTest(); + $CFG->showuseridentity = 'institution'; + $context = context_system::instance(); + $requester = $this->getDataGenerator()->create_user(); + $role = $this->getDataGenerator()->create_role(); + role_assign($role, $requester->id, $context); + assign_capability('tool/dataprivacy:managedatarequests', CAP_ALLOW, $role, $context); + assign_capability('moodle/site:viewuseridentity', CAP_ALLOW, $role, $context); + $this->setUser($requester); + + $student1 = $this->getDataGenerator()->create_user([ + 'institution' => 'University1' + ]); + $this->getDataGenerator()->create_user([ + 'institution' => 'University2' + ]); + + $results = external::get_users('University1'); + $this->assertCount(1, $results); + $this->assertEquals((object)[ + 'id' => $student1->id, + 'fullname' => fullname($student1), + 'extrafields' => [ + 0 => (object)[ + 'name' => 'institution', + 'value' => 'University1' + ] + ] + ], $results[$student1->id]); + } } diff --git a/admin/tool/dataprivacy/version.php b/admin/tool/dataprivacy/version.php index a56cc8ac865..d910730dcd9 100644 --- a/admin/tool/dataprivacy/version.php +++ b/admin/tool/dataprivacy/version.php @@ -24,6 +24,6 @@ defined('MOODLE_INTERNAL') || die; -$plugin->version = 2018120300; +$plugin->version = 2018011500; $plugin->requires = 2018112800; // Moodle 3.5dev (Build 2018031600) and upwards. $plugin->component = 'tool_dataprivacy';