From 0e966da5e61788f59e079b2ab499db738dbed8fa Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Thu, 14 Mar 2024 16:01:48 +0100 Subject: [PATCH] MDL-80973 tool_policy: Fix edge case --- admin/tool/policy/classes/api.php | 4 +++- admin/tool/policy/tests/api_test.php | 8 +++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/admin/tool/policy/classes/api.php b/admin/tool/policy/classes/api.php index 9a7af22f1dd..887147b912a 100644 --- a/admin/tool/policy/classes/api.php +++ b/admin/tool/policy/classes/api.php @@ -1034,7 +1034,9 @@ class api { } } - if ($user->policyagreed != $allresponded) { + // MDL-80973: At this point, the policyagreed value in DB could be 0 but $user->policyagreed could be 1 (as it was copied from $USER). + // So we need to ensure that the value in DB is set true if all policies were responded. + if ($user->policyagreed != $allresponded || $allresponded) { $user->policyagreed = $allresponded; $DB->set_field('user', 'policyagreed', $allresponded, ['id' => $user->id]); } diff --git a/admin/tool/policy/tests/api_test.php b/admin/tool/policy/tests/api_test.php index 0b428fd3c2b..5177eb284a5 100644 --- a/admin/tool/policy/tests/api_test.php +++ b/admin/tool/policy/tests/api_test.php @@ -579,7 +579,7 @@ class api_test extends \advanced_testcase { * Test that accepting policy updates 'policyagreed' */ public function test_accept_policies() { - global $DB; + global $DB, $USER; $this->resetAfterTest(); $this->setAdminUser(); @@ -623,6 +623,12 @@ class api_test extends \advanced_testcase { api::accept_policies([$policy3->id]); $this->assertEquals(1, $DB->get_field('user', 'policyagreed', ['id' => $user2->id])); + + // Ensure policies are always accepted when all are responded regardless the $USER->policyagreed value. + $USER->policyagreed = 1; + $DB->set_field('user', 'policyagreed', 0, ['id' => $user2->id]); + api::accept_policies([$policy3->id]); + $this->assertEquals(1, $DB->get_field('user', 'policyagreed', ['id' => $user2->id])); } /**