From 2fdf71ffc78e52faec8f3bae43c2e5dfb3ec44a8 Mon Sep 17 00:00:00 2001
From: Michael Hawkins <michaelh@moodle.com>
Date: Mon, 3 May 2021 22:26:38 +0800
Subject: [PATCH] MDL-71401 core_completion: Fix completion status caching

When an automatic completion condition may have had its state change,
we now unset the cached value for the user's completion in the relevant
activity, so up-to-date values are re-fetched and available to students.
The previous behaviour was that custom conditions would remain cached
until the activity reached overall completion.
---
 lib/completionlib.php            | 20 +++++++++++++++++++-
 lib/tests/completionlib_test.php |  6 +-----
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/lib/completionlib.php b/lib/completionlib.php
index faf7ffe30c5..9154d28ad61 100644
--- a/lib/completionlib.php
+++ b/lib/completionlib.php
@@ -599,6 +599,24 @@ class completion_info {
             }
         }
 
+        // Default to current user if one is not provided.
+        if ($userid == 0) {
+            $userid = $USER->id;
+        }
+
+        // Delete the cm's cached completion data for this user if automatic completion is enabled.
+        // This ensures any changes to the status of individual completion conditions in the activity will be fetched.
+        if ($cm->completion == COMPLETION_TRACKING_AUTOMATIC) {
+            $completioncache = cache::make('core', 'completion');
+            $completionkey = $userid . '_' . $this->course->id;
+            $completiondata = $completioncache->get($completionkey);
+
+            if ($completiondata !== false) {
+                unset($completiondata[$cm->id]);
+                $completioncache->set($completionkey, $completiondata);
+            }
+        }
+
         // Get current value of completion state and do nothing if it's same as
         // the possible result of this change. If the change is to COMPLETE and the
         // current value is one of the COMPLETE_xx subtypes, ignore that as well
@@ -634,7 +652,7 @@ class completion_info {
             $newstate = $this->internal_get_state($cm, $userid, $current);
         }
 
-        // If changed, update
+        // If the overall completion state has changed, update it in the cache.
         if ($newstate != $current->completionstate) {
             $current->completionstate = $newstate;
             $current->timemodified    = time();
diff --git a/lib/tests/completionlib_test.php b/lib/tests/completionlib_test.php
index 4065c74fcfd..aa7d567b75e 100644
--- a/lib/tests/completionlib_test.php
+++ b/lib/tests/completionlib_test.php
@@ -143,6 +143,7 @@ class core_completionlib_testcase extends advanced_testcase {
         $c->update_state($cm);
 
         // Enabled, but current state is same as possible result, do nothing.
+        $cm->completion = COMPLETION_TRACKING_AUTOMATIC;
         $c = $mockbuilder->getMock();
         $current = (object)array('completionstate' => COMPLETION_COMPLETE, 'overrideby' => null);
         $c->expects($this->once())
@@ -151,7 +152,6 @@ class core_completionlib_testcase extends advanced_testcase {
             ->will($this->returnValue(true));
         $c->expects($this->once())
             ->method('get_data')
-            ->with($cm, false, 0)
             ->will($this->returnValue($current));
         $c->update_state($cm, COMPLETION_COMPLETE);
 
@@ -165,7 +165,6 @@ class core_completionlib_testcase extends advanced_testcase {
             ->will($this->returnValue(true));
         $c->expects($this->once())
             ->method('get_data')
-            ->with($cm, false, 0)
             ->will($this->returnValue($current));
         $c->update_state($cm, COMPLETION_COMPLETE);
 
@@ -179,7 +178,6 @@ class core_completionlib_testcase extends advanced_testcase {
             ->will($this->returnValue(true));
         $c->expects($this->once())
             ->method('get_data')
-            ->with($cm, false, 0)
             ->will($this->returnValue($current));
         $c->update_state($cm, COMPLETION_COMPLETE);
 
@@ -191,7 +189,6 @@ class core_completionlib_testcase extends advanced_testcase {
             ->will($this->returnValue(true));
         $c->expects($this->once())
             ->method('get_data')
-            ->with($cm, false, 0)
             ->will($this->returnValue($current));
         $changed = clone($current);
         $changed->timemodified = time();
@@ -213,7 +210,6 @@ class core_completionlib_testcase extends advanced_testcase {
             ->will($this->returnValue(true));
         $c->expects($this->once())
             ->method('get_data')
-            ->with($cm, false, 0)
             ->will($this->returnValue($current));
         $c->expects($this->once())
             ->method('internal_get_state')