MDL-61899 tool_dataprivacy: Fixes during integration review

* Disable tool by default.
* Add format columns for 'comments' and 'dpocomment' fields in
tool_dataprivacy_request table.
* Use data request exporter when sending email notification to DPO
This commit is contained in:
Jun Pataleta 2018-04-18 13:16:30 +08:00 committed by Eloy Lafuente (stronk7)
parent bb4030ff27
commit ba5b59c0af
6 changed files with 67 additions and 29 deletions

View File

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

View File

@ -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
],
];
}
}

View File

@ -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'))) {

View File

@ -9,11 +9,13 @@
<FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true"/>
<FIELD NAME="type" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="Data request type"/>
<FIELD NAME="comments" TYPE="text" NOTNULL="false" SEQUENCE="false" COMMENT="More details about the request"/>
<FIELD NAME="commentsformat" TYPE="int" LENGTH="2" NOTNULL="true" DEFAULT="0" SEQUENCE="false"/>
<FIELD NAME="userid" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="The user ID the request is being made for"/>
<FIELD NAME="requestedby" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="The user ID of the one making the request"/>
<FIELD NAME="status" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="The current status of the data request"/>
<FIELD NAME="dpo" TYPE="int" LENGTH="10" NOTNULL="false" DEFAULT="0" SEQUENCE="false" COMMENT="The user ID of the Data Protection Officer who is reviewing th request"/>
<FIELD NAME="dpocomment" TYPE="text" NOTNULL="false" SEQUENCE="false" COMMENT="DPO's comments (e.g. reason for rejecting the request, etc.)"/>
<FIELD NAME="dpocommentformat" TYPE="int" LENGTH="2" NOTNULL="true" DEFAULT="0" SEQUENCE="false"/>
<FIELD NAME="usermodified" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="The user who created/modified this request object"/>
<FIELD NAME="timecreated" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="The time this data request was created"/>
<FIELD NAME="timemodified" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="The last time this data request was updated"/>

View File

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

View File

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