From b96e7fa6b23e888e03fb2ef665706f8468676061 Mon Sep 17 00:00:00 2001 From: Adrian Greeve Date: Wed, 10 Oct 2018 13:27:23 +0800 Subject: [PATCH 1/4] MDL-63531 mod_assign: Update mod assign to use new interface. This introduces a new interface for assign sub-plugins and updates the mod_assign provider to implement the new general interface for deleting data for users in a context. --- .../privacy/assign_plugin_request_data.php | 90 +++++++ mod/assign/classes/privacy/provider.php | 182 ++++++++++++-- mod/assign/tests/privacy_test.php | 235 ++++++++++++++++++ 3 files changed, 483 insertions(+), 24 deletions(-) diff --git a/mod/assign/classes/privacy/assign_plugin_request_data.php b/mod/assign/classes/privacy/assign_plugin_request_data.php index c93597234f3..4bb664d9cc2 100644 --- a/mod/assign/classes/privacy/assign_plugin_request_data.php +++ b/mod/assign/classes/privacy/assign_plugin_request_data.php @@ -48,6 +48,15 @@ class assign_plugin_request_data { /** @var object If set then only export data related directly to this user. */ protected $user; + /** @var array The user IDs of the users that will be affected. */ + protected $userids; + + /** @var array The submissions related to the users added. */ + protected $submissions = []; + + /** @var array The grades related to the users added. */ + protected $grades = []; + /** @var assign The assign object */ protected $assign; @@ -69,6 +78,16 @@ class assign_plugin_request_data { $this->assign = $assign; } + /** + * Method for adding an array of user IDs. This will do a query to populate the submissions and grades + * for these users. + * + * @param array $userids User IDs to do something with. + */ + public function set_userids(array $userids) { + $this->userids = $userids; + } + /** * Getter for this attribute. * @@ -113,4 +132,75 @@ class assign_plugin_request_data { public function get_assign() { return $this->assign; } + + /** + * A method to conveniently fetch the assign id. + * + * @return int The assign id. + */ + public function get_assignid() { + return $this->assign->get_instance()->id; + } + + /** + * Get all of the user IDs + * + * @return array User IDs + */ + public function get_userids() { + return $this->userids; + } + + /** + * Returns all of the submission IDs + * + * @return array submission IDs + */ + public function get_submissionids() { + return array_keys($this->submissions); + } + + /** + * Returns the submissions related to the user IDs + * + * @return array User submissions. + */ + public function get_submissions() { + return $this->submissions; + } + + /** + * Returns the grade IDs related to the user IDs + * + * @return array User grade IDs. + */ + public function get_gradeids() { + return array_keys($this->grades); + } + + /** + * Returns the grades related to the user IDs + * + * @return array User grades. + */ + public function get_grades() { + return $this->grades; + } + + /** + * Fetches all of the submissions and grades related to the User IDs provided. Use get_grades, get_submissions etc to + * retrieve this information. + */ + public function populate_submissions_and_grades() { + global $DB; + + if (empty($this->get_userids())) { + throw new \coding_exception('Please use set_userids() before calling this method.'); + } + + list($sql, $params) = $DB->get_in_or_equal($this->get_userids(), SQL_PARAMS_NAMED); + $params['assign'] = $this->get_assign()->get_instance()->id; + $this->submissions = $DB->get_records_select('assign_submission', "assignment = :assign AND userid $sql", $params); + $this->grades = $DB->get_records_select('assign_grades', "assignment = :assign AND userid $sql", $params); + } } diff --git a/mod/assign/classes/privacy/provider.php b/mod/assign/classes/privacy/provider.php index 56a7736d81d..fc0e0ab8b6f 100644 --- a/mod/assign/classes/privacy/provider.php +++ b/mod/assign/classes/privacy/provider.php @@ -29,14 +29,13 @@ defined('MOODLE_INTERNAL') || die(); require_once($CFG->dirroot . '/mod/assign/locallib.php'); use \core_privacy\local\metadata\collection; -use \core_privacy\local\metadata\provider as metadataprovider; use \core_privacy\local\request\contextlist; -use \core_privacy\local\request\plugin\provider as pluginprovider; -use \core_privacy\local\request\user_preference_provider as preference_provider; use \core_privacy\local\request\writer; use \core_privacy\local\request\approved_contextlist; use \core_privacy\local\request\transform; use \core_privacy\local\request\helper; +use \core_privacy\local\request\userlist; +use \core_privacy\local\request\approved_userlist; use \core_privacy\manager; /** @@ -46,11 +45,21 @@ use \core_privacy\manager; * @copyright 2018 Adrian Greeve * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class provider implements metadataprovider, pluginprovider, preference_provider { +class provider implements + \core_privacy\local\metadata\provider, + \core_privacy\local\request\plugin\provider, + \core_privacy\local\request\user_preference_provider, + \core_privacy\local\request\core_userlist_provider { /** Interface for all assign submission sub-plugins. */ const ASSIGNSUBMISSION_INTERFACE = 'mod_assign\privacy\assignsubmission_provider'; + /** Interface for all assign submission sub-plugins. This allows for deletion of users with a context. */ + const ASSIGNSUBMISSION_USER_INTERFACE = 'mod_assign\privacy\assignsubmission_user_provider'; + + /** Interface for all assign feedback sub-plugins. This allows for deletion of users with a context. */ + const ASSIGNFEEDBACK_USER_INTERFACE = 'mod_assign\privacy\assignfeedback_user_provider'; + /** Interface for all assign feedback sub-plugins. */ const ASSIGNFEEDBACK_INTERFACE = 'mod_assign\privacy\assignfeedback_provider'; @@ -192,6 +201,76 @@ class provider implements metadataprovider, pluginprovider, preference_provider return $contextlist; } + /** + * Get the list of contexts that contain user information for the specified user. + * + * @param userlist $userlist The userlist containing the list of users who have data in this context/plugin combination. + */ + public static function get_users_in_context(userlist $userlist) { + + $context = $userlist->get_context(); + if ($context->contextlevel != CONTEXT_MODULE) { + return; + } + + $params = [ + 'modulename' => 'assign', + 'contextid' => $context->id, + 'contextlevel' => CONTEXT_MODULE + ]; + + $sql = "SELECT g.userid, g.grader + FROM {context} ctx + JOIN {course_modules} cm ON cm.id = ctx.instanceid + JOIN {modules} m ON m.id = cm.module AND m.name = :modulename + JOIN {assign} a ON a.id = cm.instance + JOIN {assign_grades} g ON a.id = g.assignment + WHERE ctx.id = :contextid AND ctx.contextlevel = :contextlevel"; + $userlist->add_from_sql('userid', $sql, $params); + $userlist->add_from_sql('grader', $sql, $params); + + $sql = "SELECT o.userid + FROM {context} ctx + JOIN {course_modules} cm ON cm.id = ctx.instanceid + JOIN {modules} m ON m.id = cm.module AND m.name = :modulename + JOIN {assign} a ON a.id = cm.instance + JOIN {assign_overrides} o ON a.id = o.assignid + WHERE ctx.id = :contextid AND ctx.contextlevel = :contextlevel"; + $userlist->add_from_sql('userid', $sql, $params); + + $sql = "SELECT s.userid + FROM {context} ctx + JOIN {course_modules} cm ON cm.id = ctx.instanceid + JOIN {modules} m ON m.id = cm.module AND m.name = :modulename + JOIN {assign} a ON a.id = cm.instance + JOIN {assign_submission} s ON a.id = s.assignment + WHERE ctx.id = :contextid AND ctx.contextlevel = :contextlevel"; + $userlist->add_from_sql('userid', $sql, $params); + + $sql = "SELECT uf.userid + FROM {context} ctx + JOIN {course_modules} cm ON cm.id = ctx.instanceid + JOIN {modules} m ON m.id = cm.module AND m.name = :modulename + JOIN {assign} a ON a.id = cm.instance + JOIN {assign_user_flags} uf ON a.id = uf.assignment + WHERE ctx.id = :contextid AND ctx.contextlevel = :contextlevel"; + $userlist->add_from_sql('userid', $sql, $params); + + $sql = "SELECT um.userid + FROM {context} ctx + JOIN {course_modules} cm ON cm.id = ctx.instanceid + JOIN {modules} m ON m.id = cm.module AND m.name = :modulename + JOIN {assign} a ON a.id = cm.instance + JOIN {assign_user_mapping} um ON a.id = um.assignment + WHERE ctx.id = :contextid AND ctx.contextlevel = :contextlevel"; + $userlist->add_from_sql('userid', $sql, $params); + + manager::plugintype_class_callback('assignsubmission', self::ASSIGNSUBMISSION_USER_INTERFACE, + 'get_userids_from_context', [$userlist]); + manager::plugintype_class_callback('assignfeedback', self::ASSIGNFEEDBACK_USER_INTERFACE, + 'get_userids_from_context', [$userlist]); + } + /** * Write out the user data filtered by contexts. * @@ -265,7 +344,7 @@ class provider implements metadataprovider, pluginprovider, preference_provider } // Time to roll my own method for deleting overrides. - static::delete_user_overrides($assign); + static::delete_overrides_for_users($assign); $DB->delete_records('assign_submission', ['assignment' => $assign->get_instance()->id]); $DB->delete_records('assign_user_flags', ['assignment' => $assign->get_instance()->id]); $DB->delete_records('assign_user_mapping', ['assignment' => $assign->get_instance()->id]); @@ -311,7 +390,7 @@ class provider implements metadataprovider, pluginprovider, preference_provider } } - static::delete_user_overrides($assign, $user); + static::delete_overrides_for_users($assign, [$user->id]); $DB->delete_records('assign_user_flags', ['assignment' => $assignid, 'userid' => $user->id]); $DB->delete_records('assign_user_mapping', ['assignment' => $assignid, 'userid' => $user->id]); $DB->delete_records('assign_grades', ['assignment' => $assignid, 'userid' => $user->id]); @@ -320,32 +399,87 @@ class provider implements metadataprovider, pluginprovider, preference_provider } /** - * Deletes assignment overrides. + * Delete multiple users within a single context. * - * @param \assign $assign The assignment object - * @param \stdClass $user The user object if we are deleting only the overrides for one user. + * @param approved_userlist $userlist The approved context and user information to delete information for. */ - protected static function delete_user_overrides(\assign $assign, \stdClass $user = null) { + public static function delete_data_for_users(approved_userlist $userlist) { global $DB; + $context = $userlist->get_context(); + if ($context->contextlevel != CONTEXT_MODULE) { + return; + } + + $userids = $userlist->get_userids(); + + $assign = new \assign($context, null, null); $assignid = $assign->get_instance()->id; - $params = (isset($user)) ? ['assignid' => $assignid, 'userid' => $user->id] : ['assignid' => $assignid]; + $requestdata = new assign_plugin_request_data($context, $assign); + $requestdata->set_userids($userids); + $requestdata->populate_submissions_and_grades(); + manager::plugintype_class_callback('assignsubmission', self::ASSIGNSUBMISSION_USER_INTERFACE, 'delete_submissions', + [$requestdata]); + manager::plugintype_class_callback('assignfeedback', self::ASSIGNFEEDBACK_USER_INTERFACE, 'delete_feedback_for_grades', + [$requestdata]); - $overrides = $DB->get_records('assign_overrides', $params); - if (!empty($overrides)) { - foreach ($overrides as $override) { - - // First delete calendar events associated with this override. - $conditions = ['modulename' => 'assign', 'instance' => $assignid]; - if (isset($user)) { - $conditions['userid'] = $user->id; - } - $DB->delete_records('event', $conditions); - - // Next delete the overrides. - $DB->delete_records('assign_overrides', ['id' => $override->id]); + // Update this function to delete advanced grading information. + $gradingmanager = get_grading_manager($context, 'mod_assign', 'submissions'); + $controller = $gradingmanager->get_active_controller(); + if (isset($controller)) { + $gradeids = $requestdata->get_gradeids(); + // Careful here, if no gradeids are provided then all data is deleted for the context. + if (!empty($gradeids)) { + \core_grading\privacy\provider::delete_data_for_instances($context, $gradeids); } } + + static::delete_overrides_for_users($assign, $userids); + list($sql, $params) = $DB->get_in_or_equal($userids, SQL_PARAMS_NAMED); + $params['assignment'] = $assignid; + $DB->delete_records_select('assign_user_flags', "assignment = :assignment AND userid $sql", $params); + $DB->delete_records_select('assign_user_mapping', "assignment = :assignment AND userid $sql", $params); + $DB->delete_records_select('assign_grades', "assignment = :assignment AND userid $sql", $params); + $DB->delete_records_select('assign_submission', "assignment = :assignment AND userid $sql", $params); + } + + /** + * Deletes assignment overrides in bulk + * + * @param \assign $assign The assignment object + * @param array $userids An array of user IDs + */ + protected static function delete_overrides_for_users(\assign $assign, array $userids = []) { + global $DB; + $assignid = $assign->get_instance()->id; + + $usersql = ''; + $params = ['assignid' => $assignid]; + if (!empty($userids)) { + list($usersql, $userparams) = $DB->get_in_or_equal($userids, SQL_PARAMS_NAMED); + $params = array_merge($params, $userparams); + $overrides = $DB->get_records_select('assign_overrides', "assignid = :assignid AND userid $usersql", $params); + } else { + $overrides = $DB->get_records('assign_overrides', $params); + } + if (!empty($overrides)) { + $params = ['modulename' => 'assign', 'instance' => $assignid]; + if (!empty($userids)) { + $params = array_merge($params, $userparams); + $DB->delete_records_select('event', "modulename = :modulename AND instance = :instance AND userid $usersql", + $params); + // Setting up for the next query. + $params = $userparams; + $usersql = "AND userid $usersql"; + } else { + $DB->delete_records('event', $params); + // Setting up for the next query. + $params = []; + } + list($overridesql, $overrideparams) = $DB->get_in_or_equal(array_keys($overrides), SQL_PARAMS_NAMED); + $params = array_merge($params, $overrideparams); + $DB->delete_records_select('assign_overrides', "id $overridesql $usersql", $params); + } } /** diff --git a/mod/assign/tests/privacy_test.php b/mod/assign/tests/privacy_test.php index bcae99d1771..185d5aaf8e6 100644 --- a/mod/assign/tests/privacy_test.php +++ b/mod/assign/tests/privacy_test.php @@ -148,6 +148,95 @@ class mod_assign_privacy_testcase extends provider_testcase { $this->assertEmpty(array_diff($usercontextids, $contextlist->get_contextids())); } + /** + * Test returning a list of user IDs related to a context (assign). + */ + public function test_get_users_in_context() { + global $DB; + + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(); + + // Only made a comment on a submission. + $user1 = $this->getDataGenerator()->create_user(); + // User 2 only has information about an activity override. + $user2 = $this->getDataGenerator()->create_user(); + // User 3 made a submission. + $user3 = $this->getDataGenerator()->create_user(); + // User 4 makes a submission and it is marked by the teacher. + $user4 = $this->getDataGenerator()->create_user(); + // Grading and providing feedback as a teacher. + $user5 = $this->getDataGenerator()->create_user(); + // This user has no entries and should not show up. + $user6 = $this->getDataGenerator()->create_user(); + + $this->getDataGenerator()->enrol_user($user1->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user2->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user3->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user4->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user5->id, $course->id, 'editingteacher'); + $this->getDataGenerator()->enrol_user($user6->id, $course->id, 'student'); + + $assign1 = $this->create_instance(['course' => $course, + 'assignsubmission_onlinetext_enabled' => true, + 'assignfeedback_comments_enabled' => true]); + $assign2 = $this->create_instance(['course' => $course]); + + $context = $assign1->get_context(); + + // Jam an entry in the comments table for user 1. + $comment = (object) [ + 'contextid' => $context->id, + 'component' => 'assignsubmission_comments', + 'commentarea' => 'submission_comments', + 'itemid' => 5, + 'content' => 'A comment by user 1', + 'format' => 0, + 'userid' => $user1->id, + 'timecreated' => time() + ]; + $DB->insert_record('comments', $comment); + + $this->setUser($user5); // Set the user to the teacher. + + $overridedata = new \stdClass(); + $overridedata->assignid = $assign1->get_instance()->id; + $overridedata->userid = $user2->id; + $overridedata->duedate = time(); + $overridedata->allowsubmissionsfromdate = time(); + $overridedata->cutoffdate = time(); + $DB->insert_record('assign_overrides', $overridedata); + + $submissiontext = 'My first submission'; + $submission = $this->create_submission($assign1, $user3, $submissiontext); + + $submissiontext = 'My first submission'; + $submission = $this->create_submission($assign1, $user4, $submissiontext); + + $this->setUser($user5); + + $grade = '72.00'; + $teachercommenttext = 'This is better. Thanks.'; + $data = new \stdClass(); + $data->attemptnumber = 1; + $data->grade = $grade; + $data->assignfeedbackcomments_editor = ['text' => $teachercommenttext, 'format' => FORMAT_MOODLE]; + + // Give the submission a grade. + $assign1->save_grade($user4->id, $data); + + $userlist = new \core_privacy\local\request\userlist($context, 'assign'); + provider::get_users_in_context($userlist); + $userids = $userlist->get_userids(); + $this->assertTrue(in_array($user1->id, $userids)); + $this->assertTrue(in_array($user2->id, $userids)); + $this->assertTrue(in_array($user3->id, $userids)); + $this->assertTrue(in_array($user4->id, $userids)); + $this->assertTrue(in_array($user5->id, $userids)); + $this->assertFalse(in_array($user6->id, $userids)); + } + /** * Test that a student with multiple submissions and grades is returned with the correct data. */ @@ -577,4 +666,150 @@ class mod_assign_privacy_testcase extends provider_testcase { // The remaining event should be for user 1. $this->assertEquals($user1->id, $record->userid); } + + /** + * A test for deleting all user data for a bunch of users. + */ + public function test_delete_data_for_users() { + global $DB; + + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(); + + // Only made a comment on a submission. + $user1 = $this->getDataGenerator()->create_user(); + // User 2 only has information about an activity override. + $user2 = $this->getDataGenerator()->create_user(); + // User 3 made a submission. + $user3 = $this->getDataGenerator()->create_user(); + // User 4 makes a submission and it is marked by the teacher. + $user4 = $this->getDataGenerator()->create_user(); + // Grading and providing feedback as a teacher. + $user5 = $this->getDataGenerator()->create_user(); + // This user has entries in assignment 2 and should not have their data deleted. + $user6 = $this->getDataGenerator()->create_user(); + + $this->getDataGenerator()->enrol_user($user1->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user2->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user3->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user4->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user5->id, $course->id, 'editingteacher'); + $this->getDataGenerator()->enrol_user($user6->id, $course->id, 'student'); + + $assign1 = $this->create_instance(['course' => $course, + 'assignsubmission_onlinetext_enabled' => true, + 'assignfeedback_comments_enabled' => true]); + $assign2 = $this->create_instance(['course' => $course, + 'assignsubmission_onlinetext_enabled' => true, + 'assignfeedback_comments_enabled' => true]); + + $context = $assign1->get_context(); + + // Jam an entry in the comments table for user 1. + $comment = (object) [ + 'contextid' => $context->id, + 'component' => 'assignsubmission_comments', + 'commentarea' => 'submission_comments', + 'itemid' => 5, + 'content' => 'A comment by user 1', + 'format' => 0, + 'userid' => $user1->id, + 'timecreated' => time() + ]; + $DB->insert_record('comments', $comment); + + $this->setUser($user5); // Set the user to the teacher. + + $overridedata = new \stdClass(); + $overridedata->assignid = $assign1->get_instance()->id; + $overridedata->userid = $user2->id; + $overridedata->duedate = time(); + $overridedata->allowsubmissionsfromdate = time(); + $overridedata->cutoffdate = time(); + $DB->insert_record('assign_overrides', $overridedata); + + $submissiontext = 'My first submission'; + $submission = $this->create_submission($assign1, $user3, $submissiontext); + + $submissiontext = 'My first submission'; + $submission = $this->create_submission($assign1, $user4, $submissiontext); + + $submissiontext = 'My first submission'; + $submission = $this->create_submission($assign2, $user6, $submissiontext); + + $this->setUser($user5); + + $grade = '72.00'; + $teachercommenttext = 'This is better. Thanks.'; + $data = new \stdClass(); + $data->attemptnumber = 1; + $data->grade = $grade; + $data->assignfeedbackcomments_editor = ['text' => $teachercommenttext, 'format' => FORMAT_MOODLE]; + + // Give the submission a grade. + $assign1->save_grade($user4->id, $data); + + $this->setUser($user5); + + $grade = '81.00'; + $teachercommenttext = 'This is nice.'; + $data = new \stdClass(); + $data->attemptnumber = 1; + $data->grade = $grade; + $data->assignfeedbackcomments_editor = ['text' => $teachercommenttext, 'format' => FORMAT_MOODLE]; + + // Give the submission a grade. + $assign2->save_grade($user6->id, $data); + + // Check data is in place. + $data = $DB->get_records('assign_submission'); + // We should have one entry for user 3 and two entries each for user 4 and 6. + $this->assertCount(5, $data); + $usercounts = [ + $user3->id => 0, + $user4->id => 0, + $user6->id => 0 + ]; + foreach ($data as $datum) { + $usercounts[$datum->userid]++; + } + $this->assertEquals(1, $usercounts[$user3->id]); + $this->assertEquals(2, $usercounts[$user4->id]); + $this->assertEquals(2, $usercounts[$user6->id]); + + $data = $DB->get_records('assign_grades'); + // Two entries in assign_grades, one for each grade given. + $this->assertCount(2, $data); + + $data = $DB->get_records('assign_overrides'); + $this->assertCount(1, $data); + + $data = $DB->get_records('comments'); + $this->assertCount(1, $data); + + $userlist = new \core_privacy\local\request\approved_userlist($context, 'assign', [$user1->id, $user2->id]); + provider::delete_data_for_users($userlist); + + $data = $DB->get_records('assign_overrides'); + $this->assertEmpty($data); + + $data = $DB->get_records('comments'); + $this->assertEmpty($data); + + $data = $DB->get_records('assign_submission'); + // No change here. + $this->assertCount(5, $data); + + $userlist = new \core_privacy\local\request\approved_userlist($context, 'assign', [$user3->id, $user5->id]); + provider::delete_data_for_users($userlist); + + $data = $DB->get_records('assign_submission'); + // Only the record for user 3 has been deleted. + $this->assertCount(4, $data); + + $data = $DB->get_records('assign_grades'); + // Grades should be unchanged. + $this->assertCount(2, $data); + } } From d8cd86e1b1803198a1b943e3d8869daac28cbec2 Mon Sep 17 00:00:00 2001 From: Adrian Greeve Date: Fri, 5 Oct 2018 15:44:55 +0800 Subject: [PATCH 2/4] MDL-63531 assignsubmission: Update to use the new interface. This updates the plugins to use the new inteface for deleting data for users in a context. --- .../assignsubmission_user_provider.php | 54 +++++++++++ .../comments/classes/privacy/provider.php | 42 +++++++- .../comments/tests/privacy_test.php | 96 +++++++++++++++++++ .../file/classes/privacy/provider.php | 47 ++++++++- .../submission/file/tests/privacy_test.php | 73 ++++++++++++++ .../onlinetext/classes/privacy/provider.php | 46 ++++++++- .../onlinetext/tests/privacy_test.php | 54 ++++++++++- 7 files changed, 401 insertions(+), 11 deletions(-) create mode 100644 mod/assign/classes/privacy/assignsubmission_user_provider.php diff --git a/mod/assign/classes/privacy/assignsubmission_user_provider.php b/mod/assign/classes/privacy/assignsubmission_user_provider.php new file mode 100644 index 00000000000..1a9331cd0e9 --- /dev/null +++ b/mod/assign/classes/privacy/assignsubmission_user_provider.php @@ -0,0 +1,54 @@ +. + +/** + * This file contains the assignsubmission_user_provider interface. + * + * Assignment Sub plugins should implement this if they store personal information and can retrieve a userid. + * + * @package mod_assign + * @copyright 2018 Adrian Greeve + * + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +namespace mod_assign\privacy; + +use core_privacy\local\request\userlist; + +defined('MOODLE_INTERNAL') || die(); + +interface assignsubmission_user_provider extends \core_privacy\local\request\plugin\subplugin_provider { + + /** + * If you have tables that contain userids and you can generate entries in your tables without creating an + * entry in the assign_submission table then please fill in this method. + * + * @param userlist $userlist The userlist object + */ + public static function get_userids_from_context(userlist $userlist); + + /** + * Deletes all submissions for the submission ids / userids provided in a context. + * assign_plugin_request_data contains: + * - context + * - assign object + * - submission ids (pluginids) + * - user ids + * @param assign_plugin_request_data $deletedata A class that contains the relevant information required for deletion. + */ + public static function delete_submissions(assign_plugin_request_data $deletedata); + +} \ No newline at end of file diff --git a/mod/assign/submission/comments/classes/privacy/provider.php b/mod/assign/submission/comments/classes/privacy/provider.php index 6f95fd2487d..bc7507bd003 100644 --- a/mod/assign/submission/comments/classes/privacy/provider.php +++ b/mod/assign/submission/comments/classes/privacy/provider.php @@ -41,7 +41,9 @@ use \mod_assign\privacy\assign_plugin_request_data; * @copyright 2018 Adrian Greeve * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class provider implements metadataprovider, \mod_assign\privacy\assignsubmission_provider { +class provider implements metadataprovider, + \mod_assign\privacy\assignsubmission_provider, + \mod_assign\privacy\assignsubmission_user_provider { /** * Return meta data about this plugin. @@ -89,6 +91,28 @@ class provider implements metadataprovider, \mod_assign\privacy\assignsubmission $useridlist->add_from_sql($sql, $params); } + /** + * If you have tables that contain userids and you can generate entries in your tables without creating an + * entry in the assign_submission table then please fill in this method. + * + * @param \core_privacy\local\request\userlist $userlist The userlist object + */ + public static function get_userids_from_context(\core_privacy\local\request\userlist $userlist) { + $context = $userlist->get_context(); + if ($context->contextlevel != CONTEXT_MODULE) { + return; + } + $sql = "SELECT userid + FROM {comments} + WHERE contextid = :contextid + AND component = :component + AND commentarea = :commentarea"; + $userlist->add_from_sql('userid', $sql, [ + 'contextid' => $context->id, + 'component' => 'assignsubmission_comments', + 'commentarea' => 'submission_comments']); + } + /** * Export all user data for this plugin. * @@ -128,4 +152,20 @@ class provider implements metadataprovider, \mod_assign\privacy\assignsubmission [$exportdata->get_context()->id]); comments_provider::delete_comments_for_user($contextlist, 'assignsubmission_comments', 'submission_comments'); } + + /** + * Deletes all submissions for the submission ids / userids provided in a context. + * assign_plugin_request_data contains: + * - context + * - assign object + * - submission ids (pluginids) + * - user ids + * @param assign_plugin_request_data $deletedata A class that contains the relevant information required for deletion. + */ + public static function delete_submissions(assign_plugin_request_data $deletedata) { + $userlist = new \core_privacy\local\request\approved_userlist($deletedata->get_context(), 'assignsubmission_comments', + $deletedata->get_userids()); + comments_provider::delete_comments_for_users($userlist, 'assignsubmission_comments', 'submission_comments'); + } + } diff --git a/mod/assign/submission/comments/tests/privacy_test.php b/mod/assign/submission/comments/tests/privacy_test.php index e5917e66d35..c02e7959abf 100644 --- a/mod/assign/submission/comments/tests/privacy_test.php +++ b/mod/assign/submission/comments/tests/privacy_test.php @@ -134,6 +134,51 @@ class assignsubmission_comments_privacy_testcase extends \mod_assign\tests\mod_a $this->assertEquals($user1->id, $useridlist->get_userids()[0]->id); } + /** + * Test returning users related to a given context. + */ + public function test_get_userids_from_context() { + // Get a bunch of users making comments. + // Some in one context some in another. + $this->resetAfterTest(); + $course = $this->getDataGenerator()->create_course(); + // Only in first context. + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(); + // First and second context. + $user3 = $this->getDataGenerator()->create_user(); + // Second context only. + $user4 = $this->getDataGenerator()->create_user(); + $this->getDataGenerator()->enrol_user($user1->id, $course->id, 'student'); + $assign1 = $this->create_instance(['course' => $course]); + $assign2 = $this->create_instance(['course' => $course]); + + $assigncontext1 = $assign1->get_context(); + $assigncontext2 = $assign2->get_context(); + + $user1comment = 'Comment from user 1'; + list($plugin, $submission, $comment) = $this->create_comment_submission($assign1, $user1, $user1comment); + $user2comment = 'From user 2'; + $this->setUser($user2); + $comment->add($user2comment); + $user3comment = 'User 3 comment'; + $this->setUser($user3); + $comment->add($user3comment); + $user4comment = 'Comment from user 4'; + list($plugin, $submission, $comment) = $this->create_comment_submission($assign2, $user4, $user4comment); + $user3secondcomment = 'Comment on user 4 post.'; + $this->setUser($user3); + $comment->add($user3comment); + + $userlist = new \core_privacy\local\request\userlist($assigncontext1, 'assignsubmission_comments'); + \assignsubmission_comments\privacy\provider::get_userids_from_context($userlist); + $userids = $userlist->get_userids(); + $this->assertCount(3, $userids); + // Two is the key for user 3. + $this->assertEquals(2, array_search($user3->id, $userids)); + $this->assertFalse(array_search($user4->id, $userids)); + } + /** * Test that comments are exported for a user. */ @@ -259,4 +304,55 @@ class assignsubmission_comments_privacy_testcase extends \mod_assign\tests\mod_a $this->assertNotEquals($user1->id, $result->userid); } } + + /** + * Test deletion of all submissions for a context works. + */ + public function test_delete_submissions() { + global $DB; + // Get a bunch of users making comments. + // Some in one context some in another. + $this->resetAfterTest(); + $course = $this->getDataGenerator()->create_course(); + // Only in first context. + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(); + // First and second context. + $user3 = $this->getDataGenerator()->create_user(); + // Second context only. + $user4 = $this->getDataGenerator()->create_user(); + $this->getDataGenerator()->enrol_user($user1->id, $course->id, 'student'); + $assign1 = $this->create_instance(['course' => $course]); + $assign2 = $this->create_instance(['course' => $course]); + + $assigncontext1 = $assign1->get_context(); + $assigncontext2 = $assign2->get_context(); + + $user1comment = 'Comment from user 1'; + list($plugin, $submission, $comment) = $this->create_comment_submission($assign1, $user1, $user1comment); + $user2comment = 'From user 2'; + $this->setUser($user2); + $comment->add($user2comment); + $user3comment = 'User 3 comment'; + $this->setUser($user3); + $comment->add($user3comment); + $user4comment = 'Comment from user 4'; + list($plugin, $submission, $comment) = $this->create_comment_submission($assign2, $user4, $user4comment); + $user3secondcomment = 'Comment on user 4 post.'; + $this->setUser($user3); + $comment->add($user3comment); + + // There should be three entries. One for the first three users. + $results = $DB->get_records('comments', ['contextid' => $assigncontext1->id]); + $this->assertCount(3, $results); + + $deletedata = new \mod_assign\privacy\assign_plugin_request_data($assigncontext1, $assign1); + $deletedata->set_userids([$user1->id, $user3->id]); + \assignsubmission_comments\privacy\provider::delete_submissions($deletedata); + + // We should be left with just a comment from user 2. + $results = $DB->get_records('comments', ['contextid' => $assigncontext1->id]); + $this->assertCount(1, $results); + $this->assertEquals($user2comment, current($results)->content); + } } diff --git a/mod/assign/submission/file/classes/privacy/provider.php b/mod/assign/submission/file/classes/privacy/provider.php index a10e67a281b..f02f6dc70ab 100644 --- a/mod/assign/submission/file/classes/privacy/provider.php +++ b/mod/assign/submission/file/classes/privacy/provider.php @@ -29,7 +29,6 @@ defined('MOODLE_INTERNAL') || die(); require_once($CFG->dirroot . '/mod/assign/locallib.php'); use \core_privacy\local\metadata\collection; -use \core_privacy\local\metadata\provider as metadataprovider; use \core_privacy\local\request\writer; use \core_privacy\local\request\contextlist; use \mod_assign\privacy\assign_plugin_request_data; @@ -41,7 +40,10 @@ use \mod_assign\privacy\assign_plugin_request_data; * @copyright 2018 Adrian Greeve * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class provider implements metadataprovider, \mod_assign\privacy\assignsubmission_provider { +class provider implements + \core_privacy\local\metadata\provider, + \mod_assign\privacy\assignsubmission_provider, + \mod_assign\privacy\assignsubmission_user_provider { /** * Return meta data about this plugin. @@ -73,6 +75,16 @@ class provider implements metadataprovider, \mod_assign\privacy\assignsubmission // No need. } + /** + * If you have tables that contain userids and you can generate entries in your tables without creating an + * entry in the assign_submission table then please fill in this method. + * + * @param userlist $userlist The userlist object + */ + public static function get_userids_from_context(\core_privacy\local\request\userlist $userlist) { + // Not required. + } + /** * Export all user data for this plugin. * @@ -122,7 +134,7 @@ class provider implements metadataprovider, \mod_assign\privacy\assignsubmission } /** - * A call to this method should delete user data (where practicle) using the userid and submission. + * A call to this method should delete user data (where practical) using the userid and submission. * * @param assign_plugin_request_data $deletedata Details about the user and context to focus the deletion. */ @@ -137,7 +149,32 @@ class provider implements metadataprovider, \mod_assign\privacy\assignsubmission $fs->delete_area_files($deletedata->get_context()->id, 'assignsubmission_file', ASSIGNSUBMISSION_FILE_FILEAREA, $submissionid); - $DB->delete_records('assignsubmission_file', ['assignment' => $deletedata->get_assign()->get_instance()->id, - 'submission' => $submissionid]); + $DB->delete_records('assignsubmission_file', ['assignment' => $deletedata->get_assignid(), 'submission' => $submissionid]); + } + + /** + * Deletes all submissions for the submission ids / userids provided in a context. + * assign_plugin_request_data contains: + * - context + * - assign object + * - submission ids (pluginids) + * - user ids + * @param assign_plugin_request_data $deletedata A class that contains the relevant information required for deletion. + */ + public static function delete_submissions(assign_plugin_request_data $deletedata) { + global $DB; + + \core_plagiarism\privacy\provider::delete_plagiarism_for_users($deletedata->get_userids(), $deletedata->get_context()); + + if (empty($deletedata->get_submissionids())) { + return; + } + $fs = get_file_storage(); + list($sql, $params) = $DB->get_in_or_equal($deletedata->get_submissionids(), SQL_PARAMS_NAMED); + $fs->delete_area_files_select($deletedata->get_context()->id, 'assignsubmission_file', ASSIGNSUBMISSION_FILE_FILEAREA, + $sql, $params); + + $params['assignid'] = $deletedata->get_assignid(); + $DB->delete_records_select('assignsubmission_file', "assignment = :assignid AND submission $sql", $params); } } diff --git a/mod/assign/submission/file/tests/privacy_test.php b/mod/assign/submission/file/tests/privacy_test.php index 61f68412ad7..99dda2620f9 100644 --- a/mod/assign/submission/file/tests/privacy_test.php +++ b/mod/assign/submission/file/tests/privacy_test.php @@ -172,4 +172,77 @@ class assignsubmission_file_privacy_testcase extends \mod_assign\tests\mod_assig // There should be files here. $this->assertFalse($plugin2->is_empty($submission2)); } + + /** + * Test deletion of bulk submissions for a context. + */ + public function test_delete_submissions() { + global $DB; + + $this->resetAfterTest(); + // Create course, assignment, submission, and then a feedback comment. + $course = $this->getDataGenerator()->create_course(); + // Student. + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(); + $user3 = $this->getDataGenerator()->create_user(); + $user4 = $this->getDataGenerator()->create_user(); + + $this->getDataGenerator()->enrol_user($user1->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user2->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user3->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user4->id, $course->id, 'student'); + + $assign1 = $this->create_instance(['course' => $course]); + $assign2 = $this->create_instance(['course' => $course]); + + $context1 = $assign1->get_context(); + $context2 = $assign2->get_context(); + + $student1filename = 'user1file.pdf'; + list($plugin1, $submission1) = $this->create_file_submission($assign1, $user1, $student1filename); + $student2filename = 'user2file.pdf'; + list($plugin2, $submission2) = $this->create_file_submission($assign1, $user2, $student2filename); + $student3filename = 'user3file.pdf'; + list($plugin3, $submission3) = $this->create_file_submission($assign1, $user3, $student3filename); + $student4filename = 'user4file.pdf'; + list($plugin4, $submission4) = $this->create_file_submission($assign2, $user4, $student4filename); + $student5filename = 'user5file.pdf'; + list($plugin5, $submission5) = $this->create_file_submission($assign2, $user3, $student5filename); + + $submissionids = [ + $submission1->id, + $submission3->id + ]; + + $userids = [ + $user1->id, + $user3->id + ]; + + $data = $DB->get_records('files', ['contextid' => $context1->id, 'component' => 'assignsubmission_file']); + $this->assertCount(6, $data); + + $data = $DB->get_records('assignsubmission_file', ['assignment' => $assign1->get_instance()->id]); + $this->assertCount(3, $data); + + // Records in the second assignment (not being touched). + $data = $DB->get_records('assignsubmission_file', ['assignment' => $assign2->get_instance()->id]); + $this->assertCount(2, $data); + + $deletedata = new \mod_assign\privacy\assign_plugin_request_data($context1, $assign1); + $deletedata->set_userids($userids); + $deletedata->populate_submissions_and_grades(); + \assignsubmission_file\privacy\provider::delete_submissions($deletedata); + $data = $DB->get_records('files', ['contextid' => $context1->id, 'component' => 'assignsubmission_file']); + $this->assertCount(2, $data); + + // Submission 1 and 3 have been removed. We should be left with submission2. + $data = $DB->get_records('assignsubmission_file', ['assignment' => $assign1->get_instance()->id]); + $this->assertCount(1, $data); + + // This should be untouched. + $data = $DB->get_records('assignsubmission_file', ['assignment' => $assign2->get_instance()->id]); + $this->assertCount(2, $data); + } } diff --git a/mod/assign/submission/onlinetext/classes/privacy/provider.php b/mod/assign/submission/onlinetext/classes/privacy/provider.php index fc5e2746312..fa660b9769a 100644 --- a/mod/assign/submission/onlinetext/classes/privacy/provider.php +++ b/mod/assign/submission/onlinetext/classes/privacy/provider.php @@ -29,7 +29,6 @@ defined('MOODLE_INTERNAL') || die(); require_once($CFG->dirroot . '/mod/assign/locallib.php'); use \core_privacy\local\metadata\collection; -use \core_privacy\local\metadata\provider as metadataprovider; use \core_privacy\local\request\writer; use \core_privacy\local\request\contextlist; use \mod_assign\privacy\assign_plugin_request_data; @@ -41,7 +40,10 @@ use \mod_assign\privacy\assign_plugin_request_data; * @copyright 2018 Adrian Greeve * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class provider implements metadataprovider, \mod_assign\privacy\assignsubmission_provider { +class provider implements + \core_privacy\local\metadata\provider, + \mod_assign\privacy\assignsubmission_provider, + \mod_assign\privacy\assignsubmission_user_provider { /** * Return meta data about this plugin. @@ -79,6 +81,16 @@ class provider implements metadataprovider, \mod_assign\privacy\assignsubmission // No need. } + /** + * If you have tables that contain userids and you can generate entries in your tables without creating an + * entry in the assign_submission table then please fill in this method. + * + * @param \core_privacy\local\request\userlist $userlist The userlist object + */ + public static function get_userids_from_context(\core_privacy\local\request\userlist $userlist) { + // Not required. + } + /** * Export all user data for this plugin. * @@ -136,7 +148,7 @@ class provider implements metadataprovider, \mod_assign\privacy\assignsubmission ASSIGNSUBMISSION_ONLINETEXT_FILEAREA); // Delete the records in the table. - $DB->delete_records('assignsubmission_onlinetext', ['assignment' => $requestdata->get_assign()->get_instance()->id]); + $DB->delete_records('assignsubmission_onlinetext', ['assignment' => $requestdata->get_assignid()]); } /** @@ -157,7 +169,33 @@ class provider implements metadataprovider, \mod_assign\privacy\assignsubmission $submissionid); // Delete the records in the table. - $DB->delete_records('assignsubmission_onlinetext', ['assignment' => $deletedata->get_assign()->get_instance()->id, + $DB->delete_records('assignsubmission_onlinetext', ['assignment' => $deletedata->get_assignid(), 'submission' => $submissionid]); } + + /** + * Deletes all submissions for the submission ids / userids provided in a context. + * assign_plugin_request_data contains: + * - context + * - assign object + * - submission ids (pluginids) + * - user ids + * @param assign_plugin_request_data $deletedata A class that contains the relevant information required for deletion. + */ + public static function delete_submissions(assign_plugin_request_data $deletedata) { + global $DB; + + \core_plagiarism\privacy\provider::delete_plagiarism_for_users($deletedata->get_userids(), $deletedata->get_context()); + if (empty($deletedata->get_submissionids())) { + return; + } + + $fs = get_file_storage(); + list($sql, $params) = $DB->get_in_or_equal($deletedata->get_submissionids(), SQL_PARAMS_NAMED); + $fs->delete_area_files_select($deletedata->get_context()->id, + 'assignsubmission_onlinetext', ASSIGNSUBMISSION_ONLINETEXT_FILEAREA, $sql, $params); + + $params['assignid'] = $deletedata->get_assignid(); + $DB->delete_records_select('assignsubmission_onlinetext', "assignment = :assignid AND submission $sql", $params); + } } diff --git a/mod/assign/submission/onlinetext/tests/privacy_test.php b/mod/assign/submission/onlinetext/tests/privacy_test.php index ce1dd5062ad..ed7342c307a 100644 --- a/mod/assign/submission/onlinetext/tests/privacy_test.php +++ b/mod/assign/submission/onlinetext/tests/privacy_test.php @@ -67,7 +67,7 @@ class assignsubmission_online_privacy_testcase extends \mod_assign\tests\mod_ass * Quick test to make sure that get_metadata returns something. */ public function test_get_metadata() { - $collection = new \core_privacy\local\metadata\collection('assignsubmission_file'); + $collection = new \core_privacy\local\metadata\collection('assignsubmission_onlinetext'); $collection = \assignsubmission_onlinetext\privacy\provider::get_metadata($collection); $this->assertNotEmpty($collection); } @@ -161,4 +161,56 @@ class assignsubmission_online_privacy_testcase extends \mod_assign\tests\mod_ass // But there is for the second submission. $this->assertFalse($plugin2->is_empty($submission2)); } + + public function test_delete_submissions() { + global $DB; + + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(); + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(); + $user3 = $this->getDataGenerator()->create_user(); + // Only makes submissions in the second assignment. + $user4 = $this->getDataGenerator()->create_user(); + + $this->getDataGenerator()->enrol_user($user1->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user2->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user3->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user4->id, $course->id, 'student'); + + $assign1 = $this->create_instance(['course' => $course]); + $assign2 = $this->create_instance(['course' => $course]); + + $context1 = $assign1->get_context(); + $context2 = $assign2->get_context(); + + $student1text = 'Student one\'s text.'; + list($plugin1, $submission1) = $this->create_online_submission($assign1, $user1, $student1text); + $student2text = 'Student two\'s text.'; + list($plugin2, $submission2) = $this->create_online_submission($assign1, $user2, $student2text); + $student3text = 'Student two\'s text.'; + list($plugin3, $submission3) = $this->create_online_submission($assign1, $user3, $student3text); + // Now for submissions in assignment two. + $student3text2 = 'Student two\'s text for the second assignment.'; + list($plugin4, $submission4) = $this->create_online_submission($assign2, $user3, $student3text2); + $student4text = 'Student four\'s text.'; + list($plugin5, $submission5) = $this->create_online_submission($assign2, $user4, $student4text); + + $data = $DB->get_records('assignsubmission_onlinetext', ['assignment' => $assign1->get_instance()->id]); + $this->assertCount(3, $data); + // Delete the submissions for user 1 and 3. + $requestdata = new \mod_assign\privacy\assign_plugin_request_data($context1, $assign1); + $requestdata->set_userids([$user1->id, $user2->id]); + $requestdata->populate_submissions_and_grades(); + \assignsubmission_onlinetext\privacy\provider::delete_submissions($requestdata); + + // There should only be one record left for assignment one. + $data = $DB->get_records('assignsubmission_onlinetext', ['assignment' => $assign1->get_instance()->id]); + $this->assertCount(1, $data); + + // Check that the second assignment has not been touched. + $data = $DB->get_records('assignsubmission_onlinetext', ['assignment' => $assign2->get_instance()->id]); + $this->assertCount(2, $data); + } } From 6592598f71306716952f194cf748ef1363bd313f Mon Sep 17 00:00:00 2001 From: Adrian Greeve Date: Wed, 10 Oct 2018 08:55:20 +0800 Subject: [PATCH 3/4] MDL-63531 assignfeedback: Update to use the new privacy interface. This now uses the new interface to allow the deletion of users based on a context. --- .../privacy/assignfeedback_user_provider.php | 52 ++++++++++++ .../comments/classes/privacy/provider.php | 49 +++++++++++- .../feedback/comments/tests/privacy_test.php | 79 +++++++++++++++++++ .../editpdf/classes/privacy/provider.php | 46 +++++++++-- .../feedback/editpdf/tests/privacy_test.php | 74 +++++++++++++++++ .../file/classes/privacy/provider.php | 49 +++++++++--- .../feedback/file/tests/privacy_test.php | 59 ++++++++++++++ 7 files changed, 388 insertions(+), 20 deletions(-) create mode 100644 mod/assign/classes/privacy/assignfeedback_user_provider.php diff --git a/mod/assign/classes/privacy/assignfeedback_user_provider.php b/mod/assign/classes/privacy/assignfeedback_user_provider.php new file mode 100644 index 00000000000..9048430a3bb --- /dev/null +++ b/mod/assign/classes/privacy/assignfeedback_user_provider.php @@ -0,0 +1,52 @@ +. + +/** + * This file contains the assignfeedback_user_provider interface. + * + * Assignment Sub plugins should implement this if they store personal information and can retrieve a userid. + * + * @package mod_assign + * @copyright 2018 Adrian Greeve + * + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +namespace mod_assign\privacy; + +defined('MOODLE_INTERNAL') || die(); + +interface assignfeedback_user_provider extends \core_privacy\local\request\plugin\subplugin_provider { + + /** + * If you have tables that contain userids and you can generate entries in your tables without creating an + * entry in the assign_grades table then please fill in this method. + * + * @param \core_privacy\local\request\userlist $userlist The userlist object + */ + public static function get_userids_from_context(\core_privacy\local\request\userlist $userlist); + + /** + * Deletes all feedback for the grade ids / userids provided in a context. + * assign_plugin_request_data contains: + * - context + * - assign object + * - grade ids (pluginids) + * - user ids + * @param assign_plugin_request_data $deletedata A class that contains the relevant information required for deletion. + */ + public static function delete_feedback_for_grades(assign_plugin_request_data $deletedata); + +} \ No newline at end of file diff --git a/mod/assign/feedback/comments/classes/privacy/provider.php b/mod/assign/feedback/comments/classes/privacy/provider.php index 9302526749a..e6453e6591b 100644 --- a/mod/assign/feedback/comments/classes/privacy/provider.php +++ b/mod/assign/feedback/comments/classes/privacy/provider.php @@ -29,8 +29,6 @@ defined('MOODLE_INTERNAL') || die(); require_once($CFG->dirroot . '/mod/assign/locallib.php'); use \core_privacy\local\metadata\collection; -use \core_privacy\local\metadata\provider as metadataprovider; -use \mod_assign\privacy\assignfeedback_provider; use \core_privacy\local\request\writer; use \core_privacy\local\request\contextlist; use \mod_assign\privacy\assign_plugin_request_data; @@ -43,7 +41,10 @@ use \mod_assign\privacy\useridlist; * @copyright 2018 Adrian Greeve * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class provider implements metadataprovider, assignfeedback_provider { +class provider implements + \core_privacy\local\metadata\provider, + \mod_assign\privacy\assignfeedback_provider, + \mod_assign\privacy\assignfeedback_user_provider { /** * Return meta data about this plugin. @@ -83,6 +84,16 @@ class provider implements metadataprovider, assignfeedback_provider { // Not required. } + /** + * If you have tables that contain userids and you can generate entries in your tables without creating an + * entry in the assign_grades table then please fill in this method. + * + * @param \core_privacy\local\request\userlist $userlist The userlist object + */ + public static function get_userids_from_context(\core_privacy\local\request\userlist $userlist) { + // Not required. + } + /** * Export all user data for this plugin. * @@ -146,7 +157,37 @@ class provider implements metadataprovider, assignfeedback_provider { $fs->delete_area_files($requestdata->get_context()->id, ASSIGNFEEDBACK_COMMENTS_COMPONENT, ASSIGNFEEDBACK_COMMENTS_FILEAREA, $requestdata->get_pluginobject()->id); - $DB->delete_records('assignfeedback_comments', ['assignment' => $requestdata->get_assign()->get_instance()->id, + $DB->delete_records('assignfeedback_comments', ['assignment' => $requestdata->get_assignid(), 'grade' => $requestdata->get_pluginobject()->id]); } + + /** + * Deletes all feedback for the grade ids / userids provided in a context. + * assign_plugin_request_data contains: + * - context + * - assign object + * - grade ids (pluginids) + * - user ids + * @param assign_plugin_request_data $deletedata A class that contains the relevant information required for deletion. + */ + public static function delete_feedback_for_grades(assign_plugin_request_data $deletedata) { + global $DB; + if (empty($deletedata->get_gradeids())) { + return; + } + + list($sql, $params) = $DB->get_in_or_equal($deletedata->get_gradeids(), SQL_PARAMS_NAMED); + + $fs = new \file_storage(); + $fs->delete_area_files_select( + $deletedata->get_context()->id, + ASSIGNFEEDBACK_COMMENTS_COMPONENT, + ASSIGNFEEDBACK_COMMENTS_FILEAREA, + $sql, + $params + ); + + $params['assignment'] = $deletedata->get_assignid(); + $DB->delete_records_select('assignfeedback_comments', "assignment = :assignment AND grade $sql", $params); + } } diff --git a/mod/assign/feedback/comments/tests/privacy_test.php b/mod/assign/feedback/comments/tests/privacy_test.php index 276b9cf6423..2cfe8d555ef 100644 --- a/mod/assign/feedback/comments/tests/privacy_test.php +++ b/mod/assign/feedback/comments/tests/privacy_test.php @@ -267,4 +267,83 @@ class assignfeedback_comments_privacy_testcase extends \mod_assign\tests\mod_ass $this->assertEquals('feedback1.txt', $file->get_filename()); $this->assertEquals($grade2->id, $file->get_itemid()); } + + /** + * Test that a grade item is deleted for a user. + */ + public function test_delete_feedback_for_grades() { + $this->resetAfterTest(); + // Create course, assignment, submission, and then a feedback comment. + $course = $this->getDataGenerator()->create_course(); + // Student. + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(); + $user3 = $this->getDataGenerator()->create_user(); + $user4 = $this->getDataGenerator()->create_user(); + // Teacher. + $user5 = $this->getDataGenerator()->create_user(); + $this->getDataGenerator()->enrol_user($user1->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user2->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user3->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user4->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user5->id, $course->id, 'editingteacher'); + $assign1 = $this->create_instance(['course' => $course]); + $assign2 = $this->create_instance(['course' => $course]); + + $feedbacktext = '

first comment for this test

'; + list($plugin1, $grade1) = $this->create_feedback($assign1, $user1, $user5, 'Submission text', $feedbacktext); + $feedbacktext = '

Comment for second student.

'; + list($plugin2, $grade2) = $this->create_feedback($assign1, $user2, $user5, 'Submission text', $feedbacktext); + $feedbacktext = '

Comment for third student.

'; + list($plugin3, $grade3) = $this->create_feedback($assign1, $user3, $user5, 'Submission text', $feedbacktext); + $feedbacktext = '

Comment for third student in the second assignment.

'; + list($plugin4, $grade4) = $this->create_feedback($assign2, $user3, $user5, 'Submission text', $feedbacktext); + $feedbacktext = '

Comment for fourth student in the second assignment.

'; + list($plugin5, $grade5) = $this->create_feedback($assign2, $user4, $user5, 'Submission text', $feedbacktext); + + // Check that we have data. + $feedbackcomments = $plugin1->get_feedback_comments($grade1->id); + $this->assertNotEmpty($feedbackcomments); + $feedbackcomments = $plugin2->get_feedback_comments($grade2->id); + $this->assertNotEmpty($feedbackcomments); + $feedbackcomments = $plugin3->get_feedback_comments($grade3->id); + $this->assertNotEmpty($feedbackcomments); + $feedbackcomments = $plugin4->get_feedback_comments($grade4->id); + $this->assertNotEmpty($feedbackcomments); + $feedbackcomments = $plugin5->get_feedback_comments($grade5->id); + $this->assertNotEmpty($feedbackcomments); + + $fs = new file_storage(); + // 6 including directories for assign 1. + // 4 including directories for assign 2. + $this->assertCount(6, $fs->get_area_files($assign1->get_context()->id, + ASSIGNFEEDBACK_COMMENTS_COMPONENT, ASSIGNFEEDBACK_COMMENTS_FILEAREA)); + $this->assertCount(4, $fs->get_area_files($assign2->get_context()->id, + ASSIGNFEEDBACK_COMMENTS_COMPONENT, ASSIGNFEEDBACK_COMMENTS_FILEAREA)); + + $deletedata = new \mod_assign\privacy\assign_plugin_request_data($assign1->get_context(), $assign1); + $deletedata->set_userids([$user1->id, $user3->id]); + $deletedata->populate_submissions_and_grades(); + assignfeedback_comments\privacy\provider::delete_feedback_for_grades($deletedata); + + // Check that grade 1 and grade 3 have been removed. + $feedbackcomments = $plugin1->get_feedback_comments($grade1->id); + $this->assertEmpty($feedbackcomments); + $feedbackcomments = $plugin2->get_feedback_comments($grade2->id); + $this->assertNotEmpty($feedbackcomments); + $feedbackcomments = $plugin3->get_feedback_comments($grade3->id); + $this->assertEmpty($feedbackcomments); + $feedbackcomments = $plugin4->get_feedback_comments($grade4->id); + $this->assertNotEmpty($feedbackcomments); + $feedbackcomments = $plugin5->get_feedback_comments($grade5->id); + $this->assertNotEmpty($feedbackcomments); + + // We have deleted two from assign 1, and none from assign 2. + // 2 including directories for assign 1. + // 4 including directories for assign 2. + $this->assertCount(2, $fs->get_area_files($assign1->get_context()->id, + ASSIGNFEEDBACK_COMMENTS_COMPONENT, ASSIGNFEEDBACK_COMMENTS_FILEAREA)); + $this->assertCount(4, $fs->get_area_files($assign2->get_context()->id, + ASSIGNFEEDBACK_COMMENTS_COMPONENT, ASSIGNFEEDBACK_COMMENTS_FILEAREA)); + } } diff --git a/mod/assign/feedback/editpdf/classes/privacy/provider.php b/mod/assign/feedback/editpdf/classes/privacy/provider.php index 71706046d67..c3fb4da4bf0 100644 --- a/mod/assign/feedback/editpdf/classes/privacy/provider.php +++ b/mod/assign/feedback/editpdf/classes/privacy/provider.php @@ -29,7 +29,6 @@ defined('MOODLE_INTERNAL') || die(); require_once($CFG->dirroot . '/mod/assign/locallib.php'); use \core_privacy\local\metadata\collection; -use \core_privacy\local\metadata\provider as metadataprovider; use \mod_assign\privacy\assignfeedback_provider; use \core_privacy\local\request\writer; use \core_privacy\local\request\contextlist; @@ -43,7 +42,10 @@ use \mod_assign\privacy\useridlist; * @copyright 2018 Adrian Greeve * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class provider implements metadataprovider, assignfeedback_provider { +class provider implements + \core_privacy\local\metadata\provider, + \mod_assign\privacy\assignfeedback_provider, + \mod_assign\privacy\assignfeedback_user_provider { /** * Return meta data about this plugin. @@ -83,6 +85,16 @@ class provider implements metadataprovider, assignfeedback_provider { // Not required. } + /** + * If you have tables that contain userids and you can generate entries in your tables without creating an + * entry in the assign_grades table then please fill in this method. + * + * @param \core_privacy\local\request\userlist $userlist The userlist object + */ + public static function get_userids_from_context(\core_privacy\local\request\userlist $userlist) { + // Not required. + } + /** * Export all user data for this plugin. * @@ -127,21 +139,41 @@ class provider implements metadataprovider, assignfeedback_provider { * @param assign_plugin_request_data $requestdata Data useful for deleting user data. */ public static function delete_feedback_for_grade(assign_plugin_request_data $requestdata) { + $requestdata->set_userids([$requestdata->get_user()->id]); + $requestdata->populate_submissions_and_grades(); + self::delete_feedback_for_grades($requestdata); + } + + + /** + * Deletes all feedback for the grade ids / userids provided in a context. + * assign_plugin_request_data contains: + * - context + * - assign object + * - grade ids (pluginids) + * - user ids + * @param assign_plugin_request_data $deletedata A class that contains the relevant information required for deletion. + */ + public static function delete_feedback_for_grades(assign_plugin_request_data $deletedata) { global $DB; - $assign = $requestdata->get_assign(); + if (empty($deletedata->get_gradeids())) { + return; + } + + $assign = $deletedata->get_assign(); $plugin = $assign->get_plugin_by_type('assignfeedback', 'editpdf'); $fileareas = $plugin->get_file_areas(); $fs = get_file_storage(); + list($sql, $params) = $DB->get_in_or_equal($deletedata->get_gradeids(), SQL_PARAMS_NAMED); foreach ($fileareas as $filearea => $notused) { // Delete pdf files. - $fs->delete_area_files($requestdata->get_context()->id, 'assignfeedback_editpdf', - $filearea, $requestdata->get_pluginobject()->id); + $fs->delete_area_files_select($deletedata->get_context()->id, 'assignfeedback_editpdf', $filearea, $sql, $params); } // Remove table entries. - $DB->delete_records('assignfeedback_editpdf_annot', ['gradeid' => $requestdata->get_pluginobject()->id]); - $DB->delete_records('assignfeedback_editpdf_cmnt', ['gradeid' => $requestdata->get_pluginobject()->id]); + $DB->delete_records_select('assignfeedback_editpdf_annot', "gradeid $sql", $params); + $DB->delete_records_select('assignfeedback_editpdf_cmnt', "gradeid $sql", $params); // Submission records in assignfeedback_editpdf_queue will be cleaned up in a scheduled task } } diff --git a/mod/assign/feedback/editpdf/tests/privacy_test.php b/mod/assign/feedback/editpdf/tests/privacy_test.php index 2291d6cd09f..af07abe51b9 100644 --- a/mod/assign/feedback/editpdf/tests/privacy_test.php +++ b/mod/assign/feedback/editpdf/tests/privacy_test.php @@ -241,4 +241,78 @@ class assignfeedback_editpdf_privacy_testcase extends \mod_assign\tests\mod_assi // Check that user 2 data is still there. $this->assertFalse($plugin2->is_empty($grade2)); } + + /** + * Test that a grade item is deleted for a user. + */ + public function test_delete_feedback_for_grades() { + global $DB; + + $this->resetAfterTest(); + // Create course, assignment, submission, and then a feedback comment. + $course = $this->getDataGenerator()->create_course(); + // Students. + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(); + $user3 = $this->getDataGenerator()->create_user(); + $user4 = $this->getDataGenerator()->create_user(); + // Teacher. + $user5 = $this->getDataGenerator()->create_user(); + $this->getDataGenerator()->enrol_user($user1->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user2->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user3->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user4->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user5->id, $course->id, 'editingteacher'); + $assign1 = $this->create_instance(['course' => $course, + 'assignsubmission_file_enabled' => 1, + 'assignsubmission_file_maxfiles' => 1, + 'assignfeedback_editpdf_enabled' => 1, + 'assignsubmission_file_maxsizebytes' => 1000000]); + + $assign2 = $this->create_instance(['course' => $course, + 'assignsubmission_file_enabled' => 1, + 'assignsubmission_file_maxfiles' => 1, + 'assignfeedback_editpdf_enabled' => 1, + 'assignsubmission_file_maxsizebytes' => 1000000]); + + $context = $assign1->get_context(); + + list($plugin1, $grade1, $storedfile1) = $this->create_feedback($assign1, $user1, $user5); + list($plugin2, $grade2, $storedfile2) = $this->create_feedback($assign1, $user2, $user5); + list($plugin3, $grade3, $storedfile3) = $this->create_feedback($assign1, $user3, $user5); + list($plugin4, $grade4, $storedfile4) = $this->create_feedback($assign2, $user3, $user5); + list($plugin5, $grade5, $storedfile5) = $this->create_feedback($assign2, $user4, $user5); + + // Check that we have data. + $this->assertFalse($plugin1->is_empty($grade1)); + $this->assertFalse($plugin2->is_empty($grade2)); + $this->assertFalse($plugin3->is_empty($grade3)); + $this->assertFalse($plugin4->is_empty($grade4)); + $this->assertFalse($plugin5->is_empty($grade5)); + + // Check that there are also files generated. + $files = $DB->get_records('files', ['component' => 'assignfeedback_editpdf', 'filearea' => 'download']); + $this->assertCount(10, $files); + + $deletedata = new assign_plugin_request_data($context, $assign1); + $deletedata->set_userids([$user1->id, $user3->id]); + $deletedata->populate_submissions_and_grades(); + \assignfeedback_editpdf\privacy\provider::delete_feedback_for_grades($deletedata); + + // Check that we now have no data for user 1. + $this->assertTrue($plugin1->is_empty($grade1)); + // Check that user 2 data is still there. + $this->assertFalse($plugin2->is_empty($grade2)); + // User 3 in assignment 1 should be gone. + $this->assertTrue($plugin3->is_empty($grade3)); + // User 3 in assignment 2 should still be here. + $this->assertFalse($plugin4->is_empty($grade4)); + // User 4 in assignment 2 should also still be here. + $this->assertFalse($plugin5->is_empty($grade5)); + + // Check the files as well. + $files = $DB->get_records('files', ['component' => 'assignfeedback_editpdf', 'filearea' => 'download']); + // We should now only have six records here. + $this->assertCount(6, $files); + } } diff --git a/mod/assign/feedback/file/classes/privacy/provider.php b/mod/assign/feedback/file/classes/privacy/provider.php index f2f609fd2ff..839e9577531 100644 --- a/mod/assign/feedback/file/classes/privacy/provider.php +++ b/mod/assign/feedback/file/classes/privacy/provider.php @@ -29,9 +29,7 @@ defined('MOODLE_INTERNAL') || die(); require_once($CFG->dirroot . '/mod/assign/locallib.php'); use \core_privacy\local\metadata\collection; -use \core_privacy\local\metadata\provider as metadataprovider; -use core_privacy\local\request\contextlist; -use \mod_assign\privacy\assignfeedback_provider; +use \core_privacy\local\request\contextlist; use \mod_assign\privacy\assign_plugin_request_data; use mod_assign\privacy\useridlist; @@ -42,7 +40,10 @@ use mod_assign\privacy\useridlist; * @copyright 2018 Adrian Greeve * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class provider implements metadataprovider, assignfeedback_provider { +class provider implements + \core_privacy\local\metadata\provider, + \mod_assign\privacy\assignfeedback_provider, + \mod_assign\privacy\assignfeedback_user_provider { /** * Return meta data about this plugin. @@ -75,6 +76,16 @@ class provider implements metadataprovider, assignfeedback_provider { // Not required. } + /** + * If you have tables that contain userids and you can generate entries in your tables without creating an + * entry in the assign_grades table then please fill in this method. + * + * @param \core_privacy\local\request\userlist $userlist The userlist object + */ + public static function get_userids_from_context(\core_privacy\local\request\userlist $userlist) { + // Not required. + } + /** * Export all user data for this plugin. * @@ -121,20 +132,40 @@ class provider implements metadataprovider, assignfeedback_provider { * @param assign_plugin_request_data $requestdata Data useful for deleting user data. */ public static function delete_feedback_for_grade(assign_plugin_request_data $requestdata) { + $requestdata->set_userids([$requestdata->get_user()->id]); + $requestdata->populate_submissions_and_grades(); + self::delete_feedback_for_grades($requestdata); + } + + + /** + * Deletes all feedback for the grade ids / userids provided in a context. + * assign_plugin_request_data contains: + * - context + * - assign object + * - grade ids (pluginids) + * - user ids + * @param assign_plugin_request_data $deletedata A class that contains the relevant information required for deletion. + */ + public static function delete_feedback_for_grades(assign_plugin_request_data $deletedata) { global $DB; - $assign = $requestdata->get_assign(); + if (empty($deletedata->get_gradeids())) { + return; + } + + $assign = $deletedata->get_assign(); $plugin = $assign->get_plugin_by_type('assignfeedback', 'file'); $fileareas = $plugin->get_file_areas(); $fs = get_file_storage(); + list($sql, $params) = $DB->get_in_or_equal($deletedata->get_gradeids(), SQL_PARAMS_NAMED); + $params['assignment'] = $deletedata->get_assignid(); foreach ($fileareas as $filearea => $notused) { // Delete feedback files. - $fs->delete_area_files($requestdata->get_context()->id, 'assignfeedback_file', $filearea, - $requestdata->get_pluginobject()->id); + $fs->delete_area_files_select($deletedata->get_context()->id, 'assignfeedback_file', $filearea, $sql, $params); } // Delete table entries. - $DB->delete_records('assignfeedback_file', ['assignment' => $requestdata->get_assign()->get_instance()->id, - 'grade' => $requestdata->get_pluginobject()->id]); + $DB->delete_records_select('assignfeedback_file', "assignment = :assignment AND grade $sql", $params); } } diff --git a/mod/assign/feedback/file/tests/privacy_test.php b/mod/assign/feedback/file/tests/privacy_test.php index 5537755d86c..38bd49b77f7 100644 --- a/mod/assign/feedback/file/tests/privacy_test.php +++ b/mod/assign/feedback/file/tests/privacy_test.php @@ -206,4 +206,63 @@ class assignfeedback_file_privacy_testcase extends \mod_assign\tests\mod_assign_ // User 2's data should still be intact. $this->assertFalse($plugin2->is_empty($grade2)); } + + /** + * Test that a grade item is deleted for a user. + */ + public function test_delete_feedback_for_grades() { + $this->resetAfterTest(); + // Create course, assignment, submission, and then a feedback comment. + $course = $this->getDataGenerator()->create_course(); + // Students. + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(); + $user3 = $this->getDataGenerator()->create_user(); + $user4 = $this->getDataGenerator()->create_user(); + // Teacher. + $user5 = $this->getDataGenerator()->create_user(); + $this->getDataGenerator()->enrol_user($user1->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user2->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user3->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user4->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($user5->id, $course->id, 'editingteacher'); + $assign1 = $this->create_instance(['course' => $course]); + $assign2 = $this->create_instance(['course' => $course]); + + $context = $assign1->get_context(); + + $feedbacktext = '

first comment for this test

'; + list($plugin1, $grade1) = $this->create_feedback($assign1, $user1, $user5, 'Submission text', $feedbacktext); + $feedbacktext = '

Comment for second submission.

'; + list($plugin2, $grade2) = $this->create_feedback($assign1, $user2, $user5, 'Submission text', $feedbacktext); + $feedbacktext = '

Comment for second submission.

'; + list($plugin3, $grade3) = $this->create_feedback($assign1, $user3, $user5, 'Submission text', $feedbacktext); + $feedbacktext = '

Comment for second submission.

'; + list($plugin4, $grade4) = $this->create_feedback($assign2, $user3, $user5, 'Submission text', $feedbacktext); + $feedbacktext = '

Comment for second submission.

'; + list($plugin5, $grade5) = $this->create_feedback($assign2, $user4, $user5, 'Submission text', $feedbacktext); + + // Check that we have data. + $this->assertFalse($plugin1->is_empty($grade1)); + $this->assertFalse($plugin2->is_empty($grade2)); + $this->assertFalse($plugin3->is_empty($grade3)); + $this->assertFalse($plugin4->is_empty($grade4)); + $this->assertFalse($plugin5->is_empty($grade5)); + + $deletedata = new assign_plugin_request_data($context, $assign1); + $deletedata->set_userids([$user1->id, $user3->id]); + $deletedata->populate_submissions_and_grades(); + \assignfeedback_file\privacy\provider::delete_feedback_for_grades($deletedata); + + // Check that we now have no data. + $this->assertTrue($plugin1->is_empty($grade1)); + // User 2's data should still be intact. + $this->assertFalse($plugin2->is_empty($grade2)); + // User 3's data in assignment 1 should be gone. + $this->assertTrue($plugin3->is_empty($grade3)); + // User 3's data in assignment 2 should still be intact. + $this->assertFalse($plugin4->is_empty($grade4)); + // User 4's data in assignment 2 should still be intact. + $this->assertFalse($plugin5->is_empty($grade5)); + } } From d2148f32ca6cb8adf3ffc8436f395f51a34e8b20 Mon Sep 17 00:00:00 2001 From: Adrian Greeve Date: Tue, 16 Oct 2018 10:07:59 +0800 Subject: [PATCH 4/4] MDL-63531 privacy: Update to comments provider to simplfy sql. This updates the get_users_in_context_from_sql query to include the context id to simplify the calls made from other components. --- comment/classes/privacy/provider.php | 16 ++++++++++++++-- .../comments/classes/privacy/provider.php | 11 ++--------- mod/data/classes/privacy/provider.php | 17 +---------------- mod/glossary/classes/privacy/provider.php | 18 ++---------------- mod/wiki/classes/privacy/provider.php | 12 +----------- 5 files changed, 20 insertions(+), 54 deletions(-) diff --git a/comment/classes/privacy/provider.php b/comment/classes/privacy/provider.php index 96db6f4295e..7ca3b461db1 100644 --- a/comment/classes/privacy/provider.php +++ b/comment/classes/privacy/provider.php @@ -235,17 +235,29 @@ class provider implements \core_privacy\local\metadata\provider, \core_privacy\l * @param string $alias An alias prefix to use for comment selects to avoid interference with your own sql. * @param string $component The component to check. * @param string $area The comment area to check. + * @param int $contextid The context id. * @param string $insql The SQL to use in a sub-select for the itemid query. * @param array $params The params required for the insql. */ public static function get_users_in_context_from_sql( - userlist $userlist, string $alias, string $component, string $area, string $insql, $params) { + userlist $userlist, string $alias, string $component, string $area, int $contextid = null, string $insql = '', + array $params = []) { + + if ($insql != '') { + $insql = "AND {$alias}.itemid {$insql}"; + } + $contextsql = ''; + if (isset($contextid)) { + $contextsql = "AND {$alias}.contextid = :{$alias}contextid"; + $params["{$alias}contextid"] = $contextid; + } + // Comment authors. $sql = "SELECT {$alias}.userid FROM {comments} {$alias} WHERE {$alias}.component = :{$alias}component AND {$alias}.commentarea = :{$alias}commentarea - AND {$alias}.itemid IN ({$insql})"; + $contextsql $insql"; $params["{$alias}component"] = $component; $params["{$alias}commentarea"] = $area; diff --git a/mod/assign/submission/comments/classes/privacy/provider.php b/mod/assign/submission/comments/classes/privacy/provider.php index bc7507bd003..8a7336c86a8 100644 --- a/mod/assign/submission/comments/classes/privacy/provider.php +++ b/mod/assign/submission/comments/classes/privacy/provider.php @@ -102,15 +102,8 @@ class provider implements metadataprovider, if ($context->contextlevel != CONTEXT_MODULE) { return; } - $sql = "SELECT userid - FROM {comments} - WHERE contextid = :contextid - AND component = :component - AND commentarea = :commentarea"; - $userlist->add_from_sql('userid', $sql, [ - 'contextid' => $context->id, - 'component' => 'assignsubmission_comments', - 'commentarea' => 'submission_comments']); + comments_provider::get_users_in_context_from_sql($userlist, 'c', 'assignsubmission_comments', 'submission_comments', + $context->id); } /** diff --git a/mod/data/classes/privacy/provider.php b/mod/data/classes/privacy/provider.php index 10b6d7b531e..b80cfffbcf2 100644 --- a/mod/data/classes/privacy/provider.php +++ b/mod/data/classes/privacy/provider.php @@ -162,22 +162,7 @@ class provider implements $userlist->add_from_sql('userid', $sql, $params); // Find users with comments. - $sql = "SELECT dr.id - FROM {context} c - JOIN {course_modules} cm ON cm.id = c.instanceid AND c.contextlevel = :contextlevel - JOIN {modules} m ON m.id = cm.module AND m.name = :modname - JOIN {data} d ON d.id = cm.instance - JOIN {data_records} dr ON dr.dataid = d.id - WHERE c.id = :contextid"; - - $params = [ - 'modname' => 'data', - 'contextid' => $context->id, - 'contextlevel' => CONTEXT_MODULE, - ]; - - \core_comment\privacy\provider::get_users_in_context_from_sql( - $userlist, 'com', 'mod_data', 'database_entry', $sql, $params); + \core_comment\privacy\provider::get_users_in_context_from_sql($userlist, 'com', 'mod_data', 'database_entry', $context->id); // Find users with ratings. $sql = "SELECT dr.id diff --git a/mod/glossary/classes/privacy/provider.php b/mod/glossary/classes/privacy/provider.php index 4c630f65375..66cf6966cf5 100644 --- a/mod/glossary/classes/privacy/provider.php +++ b/mod/glossary/classes/privacy/provider.php @@ -136,22 +136,8 @@ class provider implements $userlist->add_from_sql('userid', $sql, $params); // Find users with glossary comments. - $sql = "SELECT ge.id - FROM {context} c - JOIN {course_modules} cm ON cm.id = c.instanceid AND c.contextlevel = :contextlevel - JOIN {modules} m ON m.id = cm.module AND m.name = :modname - JOIN {glossary} g ON g.id = cm.instance - JOIN {glossary_entries} ge ON ge.glossaryid = g.id - WHERE c.id = :contextid"; - - $params = [ - 'contextid' => $context->id, - 'contextlevel' => CONTEXT_MODULE, - 'modname' => 'glossary', - ]; - - \core_comment\privacy\provider::get_users_in_context_from_sql( - $userlist, 'com', 'mod_glossary', 'glossary_entry', $sql, $params); + \core_comment\privacy\provider::get_users_in_context_from_sql($userlist, 'com', 'mod_glossary', 'glossary_entry', + $context->id); // Find users with glossary ratings. $sql = "SELECT ge.id diff --git a/mod/wiki/classes/privacy/provider.php b/mod/wiki/classes/privacy/provider.php index 7a094d1de56..469ef286742 100644 --- a/mod/wiki/classes/privacy/provider.php +++ b/mod/wiki/classes/privacy/provider.php @@ -184,17 +184,7 @@ class provider implements WHERE ctx.id = :contextid"; $userlist->add_from_sql('userid', $sql, $params); - - $sql = " - SELECT p.id - FROM {modules} m - JOIN {course_modules} cm ON cm.module = m.id AND m.name = :modname - JOIN {context} ctx ON ctx.instanceid = cm.id AND ctx.contextlevel = :contextlevel - JOIN {wiki_subwikis} s ON cm.instance = s.wikiid - JOIN {wiki_pages} p ON p.subwikiid = s.id - WHERE ctx.id = :contextid"; - - \core_comment\privacy\provider::get_users_in_context_from_sql($userlist, 'com', 'mod_wiki', 'wiki_page', $sql, $params); + \core_comment\privacy\provider::get_users_in_context_from_sql($userlist, 'com', 'mod_wiki', 'wiki_page', $context->id); } /**