MDL-63401 tool_dataprivacy: Allow expiriration of users without end date
authorAndrew Nicols <andrew@nicols.co.uk>
Thu, 27 Sep 2018 04:05:39 +0000 (12:05 +0800)
committerAndrew Nicols <andrew@nicols.co.uk>
Mon, 8 Oct 2018 12:49:59 +0000 (20:49 +0800)
admin/tool/dataprivacy/classes/expired_contexts_manager.php
admin/tool/dataprivacy/lang/en/tool_dataprivacy.php
admin/tool/dataprivacy/settings.php
admin/tool/dataprivacy/tests/expired_contexts_test.php
admin/tool/dataprivacy/version.php

index c660c01..cfdd378 100644 (file)
@@ -228,6 +228,7 @@ class expired_contexts_manager {
 
     /**
      * Get the full nested set of expiry data given appropriate SQL.
+     * Only contexts which have expired will be included.
      *
      * @param   string      $sql The SQL used to select the nested information.
      * @param   array       $params The params required by the SQL.
@@ -316,9 +317,31 @@ class expired_contexts_manager {
 
             if (!$shouldskip) {
                 $courses = enrol_get_users_courses($context->instanceid, false, ['enddate']);
+                $requireenddate = self::require_all_end_dates_for_user_deletion();
+
                 foreach ($courses as $course) {
-                    if (empty($course->enddate) || $course->enddate >= time()) {
-                        // This user still has an active enrolment.
+                    if (empty($course->enddate)) {
+                        // This course has no end date.
+                        if ($requireenddate) {
+                            // Course end dates are required, and this course has no end date.
+                            $shouldskip = true;
+                            break;
+                        }
+
+                        // Course end dates are not required. The subsequent checks are pointless at this time so just
+                        // skip them.
+                        continue;
+                    }
+
+                    if ($course->enddate >= time()) {
+                        // This course is still in the future.
+                        $shouldskip = true;
+                        break;
+                    }
+
+                    // This course has an end date which is in the past.
+                    if (!self::is_course_expired($course)) {
+                        // This course has not expired yet.
                         $shouldskip = true;
                         break;
                     }
@@ -476,6 +499,17 @@ class expired_contexts_manager {
         delete_user($user);
     }
 
+    /**
+     * Whether end dates are required on all courses in order for a user to be expired from them.
+     *
+     * @return bool
+     */
+    protected static function require_all_end_dates_for_user_deletion() : bool {
+        $requireenddate = get_config('tool_dataprivacy', 'requireallenddatesforuserdeletion');
+
+        return !empty($requireenddate);
+    }
+
     /**
      * Check that the requirements to start deleting contexts are satisified.
      *
@@ -616,6 +650,19 @@ class expired_contexts_manager {
         return $expiredctx;
     }
 
+    /**
+     * Check whether the course has expired.
+     *
+     * @param   \stdClass   $course
+     * @return  bool
+     */
+    protected static function is_course_expired(\stdClass $course) : bool {
+        $context = \context_course::instance($course->id);
+        $expiryrecords = self::get_nested_expiry_info_for_courses($context->path);
+
+        return !empty($expiryrecords[$context->path]) && $expiryrecords[$context->path]->info->is_fully_expired();
+    }
+
     /**
      * Create a new instance of the privacy manager.
      *
index 7b8aa51..1ddfdc9 100644 (file)
@@ -246,6 +246,15 @@ $string['requesttypeexport'] = 'Export all of my personal data';
 $string['requesttypeexportshort'] = 'Export';
 $string['requesttypeothers'] = 'General inquiry';
 $string['requesttypeothersshort'] = 'Message';
+$string['requireallenddatesforuserdeletion'] = 'Consider courses without end date as active';
+$string['requireallenddatesforuserdeletion_desc'] = 'When calculating user expiry, several factors are considered:
+
+* the user\'s last login time is compared against the retention period for users; and
+* whether the user is actively enrolled in any courses.
+
+When checking the active enrolment of a corse, if the course has no end date then this setting is used to determine whether that course is considered active or not.
+
+If the course has no end date, and this setting is enabled, then the user cannot be deleted.';
 $string['requiresattention'] = 'Requires attention.';
 $string['requiresattentionexplanation'] = 'This plugin does not implement the Moodle privacy API. If this plugin stores any personal data it will not be able to be exported or deleted through Moodle\'s privacy system.';
 $string['resultdeleted'] = 'You recently requested to have your account and personal data in {$a} to be deleted. This process has been completed and you will no longer be able to log in.';
index b902d52..ad5bdf2 100644 (file)
@@ -60,6 +60,12 @@ if ($hassiteconfig) {
                     new lang_string('dporolemapping_desc', 'tool_dataprivacy'), null, $roles)
             );
         }
+
+        // When calculating user expiry, should courses which have no end date be considered.
+        $privacysettings->add(new admin_setting_configcheckbox('tool_dataprivacy/requireallenddatesforuserdeletion',
+                new lang_string('requireallenddatesforuserdeletion', 'tool_dataprivacy'),
+                new lang_string('requireallenddatesforuserdeletion_desc', 'tool_dataprivacy'),
+                0));
     }
 }
 
index fffe475..37cb86b 100644 (file)
@@ -218,17 +218,20 @@ class tool_dataprivacy_expired_contexts_testcase extends advanced_testcase {
     }
 
     /**
-     * Ensure that a user with a lastaccess in the past and no active enrolments is flagged for deletion.
+     * Ensure that a user with a lastaccess in the past and expired enrolments.
      */
-    public function test_flag_user_past_lastaccess_enrol_expired() {
+    public function test_flag_user_past_lastaccess_unexpired_past_enrolment() {
         $this->resetAfterTest();
 
-        $this->setup_basics('PT1H', 'PT1H', 'P5Y');
+        $this->setup_basics('PT1H', 'PT1H', 'P1Y');
 
         $user = $this->getDataGenerator()->create_user(['lastaccess' => time() - YEARSECS]);
-        $course = $this->getDataGenerator()->create_course(['startdate' => time() - (YEARSECS * 2), 'enddate' => time() - DAYSECS]);
+        $course = $this->getDataGenerator()->create_course(['startdate' => time() - YEARSECS, 'enddate' => time() - WEEKSECS]);
         $this->getDataGenerator()->enrol_user($user->id, $course->id, 'student');
 
+        $otheruser = $this->getDataGenerator()->create_user();
+        $this->getDataGenerator()->enrol_user($otheruser->id, $course->id, 'student');
+
         $this->setUser($user);
         $block = $this->create_user_block('Title', 'Content', FORMAT_PLAIN);
         $context = \context_block::instance($block->instance->id);
@@ -238,7 +241,98 @@ class tool_dataprivacy_expired_contexts_testcase extends advanced_testcase {
         $manager = new \tool_dataprivacy\expired_contexts_manager();
         list($flaggedcourses, $flaggedusers) = $manager->flag_expired_contexts();
 
-        // Although there is a block in the user context, everything in the user context is regarded as one.
+        $this->assertEquals(0, $flaggedcourses);
+        $this->assertEquals(0, $flaggedusers);
+    }
+
+    /**
+     * Ensure that a user with a lastaccess in the past and expired enrolments.
+     */
+    public function test_flag_user_past_lastaccess_expired_enrolled() {
+        $this->resetAfterTest();
+
+        $this->setup_basics('PT1H', 'PT1H', 'PT1H');
+
+        $user = $this->getDataGenerator()->create_user(['lastaccess' => time() - YEARSECS]);
+        $course = $this->getDataGenerator()->create_course(['startdate' => time() - YEARSECS, 'enddate' => time() - WEEKSECS]);
+        $this->getDataGenerator()->enrol_user($user->id, $course->id, 'student');
+
+        $otheruser = $this->getDataGenerator()->create_user();
+        $this->getDataGenerator()->enrol_user($otheruser->id, $course->id, 'student');
+
+        $this->setUser($user);
+        $block = $this->create_user_block('Title', 'Content', FORMAT_PLAIN);
+        $context = \context_block::instance($block->instance->id);
+        $this->setUser();
+
+        // Flag all expired contexts.
+        $manager = new \tool_dataprivacy\expired_contexts_manager();
+        list($flaggedcourses, $flaggedusers) = $manager->flag_expired_contexts();
+
+        $this->assertEquals(1, $flaggedcourses);
+        $this->assertEquals(1, $flaggedusers);
+    }
+
+    /**
+     * Ensure that a user with a lastaccess in the past and enrolments without a course end date are respected
+     * correctly.
+     */
+    public function test_flag_user_past_lastaccess_missing_enddate_required() {
+        $this->resetAfterTest();
+
+        $this->setup_basics('PT1H', 'PT1H', 'PT1H');
+
+        $user = $this->getDataGenerator()->create_user(['lastaccess' => time() - YEARSECS]);
+        $course = $this->getDataGenerator()->create_course();
+        $this->getDataGenerator()->enrol_user($user->id, $course->id, 'student');
+
+        $otheruser = $this->getDataGenerator()->create_user();
+        $this->getDataGenerator()->enrol_user($otheruser->id, $course->id, 'student');
+
+        $this->setUser($user);
+        $block = $this->create_user_block('Title', 'Content', FORMAT_PLAIN);
+        $context = \context_block::instance($block->instance->id);
+        $this->setUser();
+
+        // Ensure that course end dates are not required.
+        set_config('requireallenddatesforuserdeletion', 1, 'tool_dataprivacy');
+
+        // Flag all expired contexts.
+        $manager = new \tool_dataprivacy\expired_contexts_manager();
+        list($flaggedcourses, $flaggedusers) = $manager->flag_expired_contexts();
+
+        $this->assertEquals(0, $flaggedcourses);
+        $this->assertEquals(0, $flaggedusers);
+    }
+
+    /**
+     * Ensure that a user with a lastaccess in the past and enrolments without a course end date are respected
+     * correctly when the end date is not required.
+     */
+    public function test_flag_user_past_lastaccess_missing_enddate_not_required() {
+        $this->resetAfterTest();
+
+        $this->setup_basics('PT1H', 'PT1H', 'PT1H');
+
+        $user = $this->getDataGenerator()->create_user(['lastaccess' => time() - YEARSECS]);
+        $course = $this->getDataGenerator()->create_course();
+        $this->getDataGenerator()->enrol_user($user->id, $course->id, 'student');
+
+        $otheruser = $this->getDataGenerator()->create_user();
+        $this->getDataGenerator()->enrol_user($otheruser->id, $course->id, 'student');
+
+        $this->setUser($user);
+        $block = $this->create_user_block('Title', 'Content', FORMAT_PLAIN);
+        $context = \context_block::instance($block->instance->id);
+        $this->setUser();
+
+        // Ensure that course end dates are required.
+        set_config('requireallenddatesforuserdeletion', 0, 'tool_dataprivacy');
+
+        // Flag all expired contexts.
+        $manager = new \tool_dataprivacy\expired_contexts_manager();
+        list($flaggedcourses, $flaggedusers) = $manager->flag_expired_contexts();
+
         $this->assertEquals(0, $flaggedcourses);
         $this->assertEquals(1, $flaggedusers);
     }
@@ -1283,31 +1377,31 @@ class tool_dataprivacy_expired_contexts_testcase extends advanced_testcase {
         $this->assertEquals(0, $flaggedusers);
 
         // Ensure that the record currently exists.
-        $expiredcontext =  expired_context::get_record(['contextid' => $context->id]);
+        $expiredcontext = expired_context::get_record(['contextid' => $context->id]);
         $this->assertNotFalse($expiredcontext);
 
         // Approve it.
         $expiredcontext->set('status', expired_context::STATUS_APPROVED)->save();
 
-        // Process deletions
+        // Process deletions.
         list($processedcourses, $processedusers) = $manager->process_approved_deletions();
 
         $this->assertEquals(1, $processedcourses);
         $this->assertEquals(0, $processedusers);
 
         // Ensure that the record still exists.
-        $expiredcontext =  expired_context::get_record(['contextid' => $context->id]);
+        $expiredcontext = expired_context::get_record(['contextid' => $context->id]);
         $this->assertNotFalse($expiredcontext);
 
         // Remove the actual course.
         delete_course($course->id, false);
 
         // The record will still exist until we flag it again.
-        $expiredcontext =  expired_context::get_record(['contextid' => $context->id]);
+        $expiredcontext = expired_context::get_record(['contextid' => $context->id]);
         $this->assertNotFalse($expiredcontext);
 
         list($flaggedcourses, $flaggedusers) = $manager->flag_expired_contexts();
-        $expiredcontext =  expired_context::get_record(['contextid' => $context->id]);
+        $expiredcontext = expired_context::get_record(['contextid' => $context->id]);
         $this->assertFalse($expiredcontext);
     }
 
index a65e257..ad971e5 100644 (file)
@@ -24,6 +24,6 @@
 
 defined('MOODLE_INTERNAL') || die;
 
-$plugin->version   = 2018091100;
+$plugin->version   = 2018092500;
 $plugin->requires  = 2018050800;        // Moodle 3.5dev (Build 2018031600) and upwards.
 $plugin->component = 'tool_dataprivacy';