From f98cd91a130fc18f0cdfe34241c84877a4bd6b4a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Mudr=C3=A1k?= <david@moodle.com>
Date: Mon, 24 Sep 2018 14:20:43 +0200
Subject: [PATCH] MDL-62309 tool_policy: Improve
 api::is_user_version_accepted() return

The method now returns three-state logic. A bool value true/false is
returned if the user has accepted/rejected the policy, respectively. A
null value is returned if the user did not express their agreement in
either way yet.

This allows to distinguish between "rejected the policy" and "did not
say anything about it yet" cases.
---
 admin/tool/policy/classes/api.php    | 16 +++++++++-------
 admin/tool/policy/tests/api_test.php | 23 +++++++++++++++++++++++
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/admin/tool/policy/classes/api.php b/admin/tool/policy/classes/api.php
index 39d49a6f328e5..fc60543ebe74a 100644
--- a/admin/tool/policy/classes/api.php
+++ b/admin/tool/policy/classes/api.php
@@ -309,8 +309,8 @@ public static function can_user_view_policy_version($policy, $behalfid = null, $
             return true;
         }
 
-        // Users have access to all the policies they have ever accepted.
-        if (static::is_user_version_accepted($userid, $policy->id)) {
+        // Users have access to all the policies they have ever accepted/declined.
+        if (static::is_user_version_accepted($userid, $policy->id) !== null) {
             return true;
         }
 
@@ -719,20 +719,22 @@ public static function get_user_version_acceptance($userid, $versionid, $accepta
     }
 
     /**
-     * Returns version acceptance for this user.
+     * Did the user accept the given policy version?
      *
      * @param int $userid User identifier.
      * @param int $versionid Policy version identifier.
-     * @param array|null $acceptances Iist of policy version acceptances indexed by versionid.
-     * @return bool True if this user has accepted this policy version; false otherwise.
+     * @param array|null $acceptances Pre-loaded list of policy version acceptances indexed by versionid.
+     * @return bool|null True/false if this user accepted/declined the policy; null otherwise.
      */
     public static function is_user_version_accepted($userid, $versionid, $acceptances = null) {
+
         $acceptance = static::get_user_version_acceptance($userid, $versionid, $acceptances);
+
         if (!empty($acceptance)) {
-            return $acceptance->status;
+            return (bool) $acceptance->status;
         }
 
-        return false;
+        return null;
     }
 
     /**
diff --git a/admin/tool/policy/tests/api_test.php b/admin/tool/policy/tests/api_test.php
index ed5f3cb640391..d91794f66c50e 100644
--- a/admin/tool/policy/tests/api_test.php
+++ b/admin/tool/policy/tests/api_test.php
@@ -632,4 +632,27 @@ public function test_login_with_handler_without_policies() {
 
         require_login(null, false, null, false, true);
     }
+
+    /**
+     * Test the three-state logic of the value returned by {@link api::is_user_version_accepted()}.
+     */
+    public function test_is_user_version_accepted() {
+
+        $preloadedacceptances = [
+            4 => (object) [
+                'policyversionid' => 4,
+                'mainuserid' => 13,
+                'status' => 1,
+            ],
+            6 => (object) [
+                'policyversionid' => 6,
+                'mainuserid' => 13,
+                'status' => 0,
+            ],
+        ];
+
+        $this->assertTrue(api::is_user_version_accepted(13, 4, $preloadedacceptances));
+        $this->assertFalse(api::is_user_version_accepted(13, 6, $preloadedacceptances));
+        $this->assertNull(api::is_user_version_accepted(13, 5, $preloadedacceptances));
+    }
 }