MDL-81803 tool_dataprivacy: observe disabled expiration time.

A non-positive integer value should indicate "No time limit".
paulholden · Jun 11, 2024 · 3a08862 · 3a08862
1 parent c8c84b4
commit 3a08862
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 4 deletions.
diff --git a/admin/tool/dataprivacy/classes/data_request.php b/admin/tool/dataprivacy/classes/data_request.php
@@ -155,9 +155,9 @@ public static function is_expired(data_request $request) {
                 case api::DATAREQUEST_STATUS_EXPIRED:
                     $result = true;
                     break;
-                // Complete requests are expired if the expiry time has elapsed.
+                // Complete requests are expired if the expiry time is a positive value, and has elapsed.
                 case api::DATAREQUEST_STATUS_DOWNLOAD_READY:
-                    $expiryseconds = get_config('tool_dataprivacy', 'privacyrequestexpiry');
+                    $expiryseconds = (int) get_config('tool_dataprivacy', 'privacyrequestexpiry');
                     if ($expiryseconds > 0 && time() >= ($request->get('timemodified') + $expiryseconds)) {
                         $result = true;
                     }
@@ -178,7 +178,12 @@ public static function is_expired(data_request $request) {
     public static function get_expired_requests($userid = 0) {
         global $DB;
 
-        $expiryseconds = get_config('tool_dataprivacy', 'privacyrequestexpiry');
+        // Complete requests are expired if the expiry time is a positive value, and has elapsed.
+        $expiryseconds = (int) get_config('tool_dataprivacy', 'privacyrequestexpiry');
+        if ($expiryseconds <= 0) {
+            return [];
+        }
+
         $expirytime = strtotime("-{$expiryseconds} second");
         $table = self::TABLE;
         $sqlwhere = 'type = :export_type AND status = :completestatus AND timemodified <= :expirytime';

diff --git a/admin/tool/dataprivacy/tests/expired_data_requests_test.php b/admin/tool/dataprivacy/tests/expired_data_requests_test.php
@@ -26,10 +26,11 @@
  * Expired data requests tests.
  *
  * @package    tool_dataprivacy
+ * @covers     \tool_dataprivacy\data_request
  * @copyright  2018 Michael Hawkins
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-class expired_data_requests_test extends data_privacy_testcase {
+final class expired_data_requests_test extends data_privacy_testcase {
 
     /**
      * Test tearDown.
@@ -107,6 +108,52 @@ public function test_data_request_expiry() {
         $this->assertEquals(0, $DB->count_records('files', $fileconditions));
     }
 
+    /**
+     * Test that data requests are not expired when expiration is disabled (set to zero)
+     */
+    public function test_data_request_expiry_never(): void {
+        global $DB;
+
+        $this->resetAfterTest();
+
+        \core_privacy\local\request\writer::setup_real_writer_instance();
+
+        // Disable request expiry.
+        set_config('privacyrequestexpiry', 0, 'tool_dataprivacy');
+
+        // Create and approve data request.
+        $user = $this->getDataGenerator()->create_user();
+        $usercontext = \context_user::instance($user->id);
+
+        $this->setUser($user->id);
+        $datarequest = api::create_data_request($user->id, api::DATAREQUEST_TYPE_EXPORT);
+        $requestid = $datarequest->get('id');
+
+        $this->setAdminUser();
+        api::approve_data_request($requestid);
+
+        ob_start();
+        $this->runAdhocTasks('\tool_dataprivacy\task\process_data_request_task');
+        ob_end_clean();
+
+        // Run expiry deletion - should not affect test export.
+        $expiredrequests = data_request::get_expired_requests();
+        $this->assertEmpty($expiredrequests);
+        data_request::expire($expiredrequests);
+
+        // Confirm approved and exported.
+        $request = new data_request($requestid);
+        $this->assertEquals(api::DATAREQUEST_STATUS_DOWNLOAD_READY, $request->get('status'));
+        $fileconditions = [
+            'userid' => $user->id,
+            'component' => 'tool_dataprivacy',
+            'filearea' => 'export',
+            'itemid' => $requestid,
+            'contextid' => $usercontext->id,
+        ];
+        $this->assertEquals(2, $DB->count_records('files', $fileconditions));
+    }
+
     /**
      * Test for \tool_dataprivacy\data_request::is_expired()
      * Tests for the expected request status to protect from false positive/negative,