fix open db transaction on exceptions + other small improvements

eLearning-TUDarmstadt · Oct 31, 2023 · 5e9b626 · 5e9b626
1 parent 5e77737
commit 5e9b626
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 21 deletions.
diff --git a/classes/archiveduser.php b/classes/archiveduser.php
@@ -133,28 +133,30 @@ public function archive_me() {
      */
     public function activate_me() {
         global $DB;
-        $transaction = $DB->start_delegated_transaction();
-        $user = \core_user::get_user($this->id);
-
-        // Deletes record of plugin table tool_cleanupusers.
-        if (!$DB->record_exists('tool_cleanupusers', array('id' => $user->id))) {
-            throw new cleanupusers_exception(get_string('errormessagenotactive', 'tool_cleanupusers'));
-        } else if (!$DB->record_exists('tool_cleanupusers_archive', array('id' => $user->id))) {
-            throw new cleanupusers_exception(get_string('errormessagenotactive', 'tool_cleanupusers'));
-        } else if ($DB->record_exists('user', array('username' => $this->username))) {
-            throw new cleanupusers_exception(get_string('errormessagenotactive', 'tool_cleanupusers'));
-        } else {
-            // Both record exist so we have a user which can be reactivated.
-            $DB->delete_records('tool_cleanupusers', array('id' => $user->id));
-            // If the user is in table replace data.
-            $shadowuser = $DB->get_record('tool_cleanupusers_archive', array('id' => $user->id));
+        try {
+            $transaction = $DB->start_delegated_transaction();
+            // Deletes record of plugin table tool_cleanupusers.
+            if (!$DB->record_exists('tool_cleanupusers', array('id' => $this->id))) {
+                throw new cleanupusers_exception("Failed to reactivate " . $this->username . " : user not found in tool_cleanupusers");
+            } else if (!$DB->record_exists('tool_cleanupusers_archive', array('id' => $this->id))) {
+                throw new cleanupusers_exception("Failed to reactivate " . $this->username . " : user not found in tool_cleanupusers_archive");
+            } else if ($DB->record_exists('user', array('username' => $this->username))) {
+                throw new cleanupusers_exception("Failed to reactivate " . $this->username . " : user already in user table");
+            } else {
+                // Both record exist, so we have a user which can be reactivated.
+                // If the user is in table replace data.
+                $shadowuser = $DB->get_record('tool_cleanupusers_archive', array('id' => $this->id));
 
-            $DB->update_record('user', $shadowuser);
-            // Delete records from tool_cleanupusers_archive table.
-            $DB->delete_records('tool_cleanupusers_archive', array('id' => $user->id));
+                user_update_user($shadowuser, false);
+                // Delete records from tool_cleanupusers and tool_cleanupusers_archive tables.
+                $DB->delete_records('tool_cleanupusers', array('id' => $this->id));
+                $DB->delete_records('tool_cleanupusers_archive', array('id' => $this->id));
+            }
+            $transaction->allow_commit();
+        }
+         catch (\Throwable $e) {
+            $transaction->rollback($e);
         }
-        // Gets the new user for additional checks.
-        $transaction->allow_commit();
     }
 
     /**

diff --git a/classes/task/archive_user_task.php b/classes/task/archive_user_task.php
@@ -161,7 +161,7 @@ private function change_user_deprovisionstatus($userarray, $intention) {
                         // No default since if-clause checks the intention parameter.
                     }
                     $countersuccess++;
-                } catch (cleanupusers_exception $e) {
+                } catch (\Throwable $e) {
                     $failures[$key] = $user->id;
                 }
             }