diff --git a/admin/tool/dataprivacy/classes/api.php b/admin/tool/dataprivacy/classes/api.php index bba717cbf31..5038e98eb20 100644 --- a/admin/tool/dataprivacy/classes/api.php +++ b/admin/tool/dataprivacy/classes/api.php @@ -36,7 +36,7 @@ use moodle_exception; use moodle_url; use required_capability_exception; use stdClass; -use tool_dataprivacy\local\helper; +use tool_dataprivacy\external\data_request_exporter; use tool_dataprivacy\task\initiate_data_request_task; use tool_dataprivacy\task\process_data_request_task; @@ -393,12 +393,18 @@ class api { public static function notify_dpo($dpo, data_request $request) { global $PAGE, $SITE; + $output = $PAGE->get_renderer('tool_dataprivacy'); + + $usercontext = \context_user::instance($request->get('requestedby')); + $requestexporter = new data_request_exporter($request, ['context' => $usercontext]); + $requestdata = $requestexporter->export($output); + // Create message to send to the Data Protection Officer(s). $typetext = null; - $typetext = helper::get_request_type_string($request->get('type')); + $typetext = $requestdata->typename; $subject = get_string('datarequestemailsubject', 'tool_dataprivacy', $typetext); - $requestedby = core_user::get_user($request->get('requestedby')); + $requestedby = $requestdata->requestedbyuser; $datarequestsurl = new moodle_url('/admin/tool/dataprivacy/datarequests.php'); $message = new message(); $message->courseid = $SITE->id; @@ -406,7 +412,7 @@ class api { $message->name = 'contactdataprotectionofficer'; $message->userfrom = $requestedby; $message->replyto = $requestedby->email; - $message->replytoname = fullname($requestedby->email); + $message->replytoname = $requestedby->fullname; $message->subject = $subject; $message->fullmessageformat = FORMAT_HTML; $message->notification = 1; @@ -415,20 +421,19 @@ class api { // Prepare the context data for the email message body. $messagetextdata = [ - 'requestedby' => fullname($requestedby), + 'requestedby' => $requestedby->fullname, 'requesttype' => $typetext, - 'requestdate' => userdate($request->get('timecreated')), - 'requestcomments' => text_to_html($request->get('comments')), + 'requestdate' => userdate($requestdata->timecreated), + 'requestcomments' => $requestdata->messagehtml, 'datarequestsurl' => $datarequestsurl ]; - $requestingfor = core_user::get_user($request->get('userid')); + $requestingfor = $requestdata->foruser; if ($requestedby->id == $requestingfor->id) { $messagetextdata['requestfor'] = $messagetextdata['requestedby']; } else { - $messagetextdata['requestfor'] = fullname($requestingfor); + $messagetextdata['requestfor'] = $requestingfor->fullname; } - $output = $PAGE->get_renderer('tool_dataprivacy'); // Email the data request to the Data Protection Officer(s)/Admin(s). $messagetextdata['dponame'] = fullname($dpo); // Render message email body. diff --git a/admin/tool/dataprivacy/classes/data_request.php b/admin/tool/dataprivacy/classes/data_request.php index ffad4ffee33..d5ab2187636 100644 --- a/admin/tool/dataprivacy/classes/data_request.php +++ b/admin/tool/dataprivacy/classes/data_request.php @@ -56,6 +56,16 @@ class data_request extends persistent { 'type' => PARAM_TEXT, 'default' => '' ], + 'commentsformat' => [ + 'choices' => [ + FORMAT_HTML, + FORMAT_MOODLE, + FORMAT_PLAIN, + FORMAT_MARKDOWN + ], + 'type' => PARAM_INT, + 'default' => FORMAT_PLAIN + ], 'userid' => [ 'default' => 0, 'type' => PARAM_INT @@ -88,6 +98,16 @@ class data_request extends persistent { 'type' => PARAM_TEXT, 'null' => NULL_ALLOWED ], + 'dpocommentformat' => [ + 'choices' => [ + FORMAT_HTML, + FORMAT_MOODLE, + FORMAT_PLAIN, + FORMAT_MARKDOWN + ], + 'type' => PARAM_INT, + 'default' => FORMAT_PLAIN + ], ]; } } diff --git a/admin/tool/dataprivacy/classes/external/data_request_exporter.php b/admin/tool/dataprivacy/classes/external/data_request_exporter.php index 7ba3266ebce..99963493317 100644 --- a/admin/tool/dataprivacy/classes/external/data_request_exporter.php +++ b/admin/tool/dataprivacy/classes/external/data_request_exporter.php @@ -127,6 +127,8 @@ class data_request_exporter extends persistent_exporter { $user = core_user::get_user($requestedbyid, '*', MUST_EXIST); $userexporter = new user_summary_exporter($user); $values['requestedbyuser'] = $userexporter->export($output); + } else { + $values['requestedbyuser'] = $values['foruser']; } if (!empty($this->persistent->get('dpo'))) { diff --git a/admin/tool/dataprivacy/db/install.xml b/admin/tool/dataprivacy/db/install.xml index 7528ae6339d..65d8926a90e 100644 --- a/admin/tool/dataprivacy/db/install.xml +++ b/admin/tool/dataprivacy/db/install.xml @@ -9,11 +9,13 @@ + + diff --git a/admin/tool/dataprivacy/settings.php b/admin/tool/dataprivacy/settings.php index 9ba6bdadb4e..3f28b0018dd 100644 --- a/admin/tool/dataprivacy/settings.php +++ b/admin/tool/dataprivacy/settings.php @@ -28,10 +28,10 @@ if ($hassiteconfig) { $privacysettings = $ADMIN->locate('privacysettings'); if ($ADMIN->fulltree) { - // Contact data protection officer. + // Contact data protection officer. Disabled by default. $privacysettings->add(new admin_setting_configcheckbox('tool_dataprivacy/contactdataprotectionofficer', new lang_string('contactdataprotectionofficer', 'tool_dataprivacy'), - new lang_string('contactdataprotectionofficer_desc', 'tool_dataprivacy'), 1) + new lang_string('contactdataprotectionofficer_desc', 'tool_dataprivacy'), 0) ); // Fetch roles that are assignable. diff --git a/admin/tool/dataprivacy/tests/api_test.php b/admin/tool/dataprivacy/tests/api_test.php index 12d436c61f3..62d29c147aa 100644 --- a/admin/tool/dataprivacy/tests/api_test.php +++ b/admin/tool/dataprivacy/tests/api_test.php @@ -217,16 +217,16 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { * Test for api::can_contact_dpo() */ public function test_can_contact_dpo() { - // Default ('contactdataprotectionofficer' is enabled by default). - $this->assertTrue(api::can_contact_dpo()); - - // Disable. - set_config('contactdataprotectionofficer', 0, 'tool_dataprivacy'); + // Default ('contactdataprotectionofficer' is disabled by default). $this->assertFalse(api::can_contact_dpo()); - // Enable again. + // Enable. set_config('contactdataprotectionofficer', 1, 'tool_dataprivacy'); $this->assertTrue(api::can_contact_dpo()); + + // Disable again. + set_config('contactdataprotectionofficer', 0, 'tool_dataprivacy'); + $this->assertFalse(api::can_contact_dpo()); } /** @@ -467,9 +467,10 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { */ public function notify_dpo_provider() { return [ - [api::DATAREQUEST_TYPE_EXPORT, 'requesttypeexport'], - [api::DATAREQUEST_TYPE_DELETE, 'requesttypedelete'], - [api::DATAREQUEST_TYPE_OTHERS, 'requesttypeothers'], + [false, api::DATAREQUEST_TYPE_EXPORT, 'requesttypeexport', 'Export my user data'], + [false, api::DATAREQUEST_TYPE_DELETE, 'requesttypedelete', 'Delete my user data'], + [false, api::DATAREQUEST_TYPE_OTHERS, 'requesttypeothers', 'Nothing. Just wanna say hi'], + [true, api::DATAREQUEST_TYPE_EXPORT, 'requesttypeexport', 'Admin export data of another user'], ]; } @@ -477,20 +478,28 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { * Test for api::notify_dpo() * * @dataProvider notify_dpo_provider + * @param bool $byadmin Whether the admin requests data on behalf of the user * @param int $type The request type * @param string $typestringid The request lang string identifier + * @param string $comments The requestor's message to the DPO. */ - public function test_notify_dpo($type, $typestringid) { + public function test_notify_dpo($byadmin, $type, $typestringid, $comments) { $generator = new testing_data_generator(); $user1 = $generator->create_user(); - - // Make a data request as user 1. - $this->setUser($user1); - $request = api::create_data_request($user1->id, $type); - - $sink = $this->redirectMessages(); // Let's just use admin as DPO (It's the default if not set). $dpo = get_admin(); + if ($byadmin) { + $this->setAdminUser(); + $requestedby = $dpo; + } else { + $this->setUser($user1); + $requestedby = $user1; + } + + // Make a data request for user 1. + $request = api::create_data_request($user1->id, $type, $comments); + + $sink = $this->redirectMessages(); $messageid = api::notify_dpo($dpo, $request); $this->assertNotFalse($messageid); $messages = $sink->get_messages(); @@ -498,7 +507,7 @@ class tool_dataprivacy_api_testcase extends advanced_testcase { $message = reset($messages); // Check some of the message properties. - $this->assertEquals($user1->id, $message->useridfrom); + $this->assertEquals($requestedby->id, $message->useridfrom); $this->assertEquals($dpo->id, $message->useridto); $typestring = get_string($typestringid, 'tool_dataprivacy'); $subject = get_string('datarequestemailsubject', 'tool_dataprivacy', $typestring);