From 8601c569647db5f0eaffb3bbf009c791277c2aa6 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Tue, 21 May 2024 10:59:35 +0100 Subject: [PATCH] MDL-81803 tool_dataprivacy: observe disabled expiration time. A non-positive integer value should indicate "No time limit". --- .../tool/dataprivacy/classes/data_request.php | 11 +++-- .../tests/expired_data_requests_test.php | 49 ++++++++++++++++++- 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/admin/tool/dataprivacy/classes/data_request.php b/admin/tool/dataprivacy/classes/data_request.php index 816ef5c53e1..640b5e5290a 100644 --- a/admin/tool/dataprivacy/classes/data_request.php +++ b/admin/tool/dataprivacy/classes/data_request.php @@ -156,9 +156,9 @@ class data_request extends persistent { 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; } @@ -179,7 +179,12 @@ class data_request extends persistent { 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 index 0259a0774fc..8201df73004 100644 --- a/admin/tool/dataprivacy/tests/expired_data_requests_test.php +++ b/admin/tool/dataprivacy/tests/expired_data_requests_test.php @@ -26,10 +26,11 @@ require_once('data_privacy_testcase.php'); * 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 @@ class expired_data_requests_test extends data_privacy_testcase { $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,