From 5e9b626eb0cb679b0e32f47f9c3fe0732165d993 Mon Sep 17 00:00:00 2001 From: curiosity Date: Tue, 31 Oct 2023 15:23:59 +0100 Subject: [PATCH] fix open db transaction on exceptions + other small improvements --- classes/archiveduser.php | 42 ++++++++++++++++-------------- classes/task/archive_user_task.php | 2 +- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/classes/archiveduser.php b/classes/archiveduser.php index 2d050c2..f997d1d 100644 --- 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 index 290c11d..8180c48 100644 --- 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; } }