From eb2a0f06455206302d3cf8722842f42aa5d2080e Mon Sep 17 00:00:00 2001 From: curiosity Date: Tue, 5 Dec 2023 15:57:55 +0100 Subject: [PATCH] fix unit tests and activate_me() duplicate check, throw an exception for users not suspended by plugin (on deletion/activation) --- classes/archiveduser.php | 57 ++-- classes/table/never_logged_in_table.php | 7 +- classes/table/users_table.php | 2 - tests/generator/lib.php | 28 +- tests/generator_test.php | 2 + tests/tool_cleanupusers_test.php | 308 +++++++++--------- .../tests/userstatus_timechecker_test.php | 11 +- .../tests/userstatus_userstatuswwu_test.php | 11 +- version.php | 2 +- 9 files changed, 232 insertions(+), 196 deletions(-) diff --git a/classes/archiveduser.php b/classes/archiveduser.php index c74f713..f550d7f 100644 --- a/classes/archiveduser.php +++ b/classes/archiveduser.php @@ -71,8 +71,8 @@ public function __construct($id, $suspended, $lastaccess, $username, $deleted) { /** * Suspends the user. - * Therefore, makes an entry in tool_cleanupusers and tool_cleanupusers_archive tables. Throws an exception when the user - * is already suspended. + * Therefore, makes an entry in tool_cleanupusers and tool_cleanupusers_archive tables. + * Throws an exception when the user is already suspended. * @throws \Throwable */ public function archive_me() { @@ -82,9 +82,11 @@ public function archive_me() { $user = \core_user::get_user($this->id); $transaction = $DB->start_delegated_transaction(); if ($user->suspended == 1) { - throw new cleanupusers_exception("Failed to suspend " . $user->username . " : user already suspended"); + throw new cleanupusers_exception("Failed to suspend " . $user->username . + " : user is already suspended"); } else if (!($user->username == \core_user::clean_field($user->username, 'username'))) { - throw new cleanupusers_exception("Failed to suspend " . $user->username . " : username is not cleaned"); + throw new cleanupusers_exception("Failed to suspend " . $user->username . + " : username is not cleaned"); } else { // We are already getting the shadowuser here to keep the original suspended status. $shadowuser = clone $user; @@ -116,7 +118,7 @@ public function archive_me() { /** * Reactivates the user. - * Therefore, deletes the entry in the tool_cleanupusers table and throws an exception when no entry is available (if suspended by the plugin). + * Therefore, deletes the entry in the tool_cleanupusers table and throws an exception when no entry is available. * @throws \Throwable */ public function activate_me() { @@ -128,23 +130,27 @@ public function activate_me() { // User was suspended by the plugin. if ($DB->record_exists('tool_cleanupusers', ['id' => $user->id])) { if (!$DB->record_exists('tool_cleanupusers_archive', ['id' => $user->id])) { - throw new cleanupusers_exception("Failed to reactivate " . $user->username . " : user suspended by plugin has no entry in archive"); - } else if ($DB->record_exists('user', ['username' => $user->username])) { - throw new cleanupusers_exception("Failed to reactivate " . $user->username . " : user suspended by plugin already in user table"); + throw new cleanupusers_exception("Failed to reactivate " . $user->username . + " : user suspended by the plugin has no entry in archive"); } else { - // Both records 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', ['id' => $user->id]); - user_update_user($shadowuser, false); - // Delete records from tool_cleanupusers and tool_cleanupusers_archive tables. - $DB->delete_records('tool_cleanupusers', ['id' => $user->id]); - $DB->delete_records('tool_cleanupusers_archive', ['id' => $user->id]); + if ($DB->record_exists('user', ['username' => $shadowuser->username])) { + throw new cleanupusers_exception("Failed to reactivate " . $user->username . + " : user suspended by the plugin already in user table"); + } else { + // Both records exist, so we have a user which can be reactivated. + // If the user is in table replace data. + + user_update_user($shadowuser, false); + // Delete records from tool_cleanupusers and tool_cleanupusers_archive tables. + $DB->delete_records('tool_cleanupusers', ['id' => $user->id]); + $DB->delete_records('tool_cleanupusers_archive', ['id' => $user->id]); + } } - } - // User was suspended manually. - else { - $user->suspended = 0; - user_update_user($user, false); + } else { + // User was suspended manually. + throw new cleanupusers_exception("Failed to reactivate " . $user->username . + " : user not suspended by the plugin"); } $transaction->allow_commit(); } catch (\Throwable $e) { @@ -156,7 +162,7 @@ public function activate_me() { * Deletes the user. * * Therefore, - * (1) Deletes the entry in the tool_cleanupusers and the tool_cleanupusers_archive tables (if suspended by the plugin). + * (1) Deletes the entry in the tool_cleanupusers and the tool_cleanupusers_archive tables. * (2) Hashes the username with the sha256 function. * (3) Calls the moodle core delete_user function. * @@ -171,16 +177,17 @@ public function delete_me() { // User was suspended by the plugin. if ($DB->record_exists('tool_cleanupusers', ['id' => $user->id])) { if (!$DB->record_exists('tool_cleanupusers_archive', ['id' => $user->id])) { - throw new cleanupusers_exception("Failed to delete " . $user->username . " : user suspended by plugin has no entry in archive"); + throw new cleanupusers_exception("Failed to delete " . $user->username . + " : user suspended by the plugin has no entry in archive"); } else { // Deletes the records in both plugin tables. $DB->delete_records('tool_cleanupusers', ['id' => $user->id]); $DB->delete_records('tool_cleanupusers_archive', ['id' => $user->id]); } - } - // User was suspended manually. - else { - // Nothing to delete in plugin tables. + } else { + // User was suspended manually. + throw new cleanupusers_exception("Failed to delete " . $user->username . + " : user not suspended by the plugin"); } // To secure that plugins that reference the user table do not fail create empty user with a hash as username. $newusername = hash('md5', $user->username); diff --git a/classes/table/never_logged_in_table.php b/classes/table/never_logged_in_table.php index 23a5111..5fc2d91 100644 --- a/classes/table/never_logged_in_table.php +++ b/classes/table/never_logged_in_table.php @@ -25,8 +25,6 @@ use core_user\fields; -defined('MOODLE_INTERNAL') || die(); - /** * Create a class for a custom sql_table for the tool_cleanupusers * @@ -64,7 +62,10 @@ public function __construct($users, $sqlwhere, $param) { $where .= ' AND ' . $sqlwhere; } - $this->set_sql('id, username, lastaccess, suspended, ' . implode(', ', fields::get_name_fields()), '{user}', $where, $param); + $this->set_sql('id, username, lastaccess, suspended, ' . implode( + ', ', + fields::get_name_fields() + ), '{user}', $where, $param); } /** diff --git a/classes/table/users_table.php b/classes/table/users_table.php index 1e3b7ae..8762f49 100644 --- a/classes/table/users_table.php +++ b/classes/table/users_table.php @@ -25,8 +25,6 @@ use core_user\fields; -defined('MOODLE_INTERNAL') || die(); - /** * Create a class for a custom sql_table for the tool_cleanupusers * diff --git a/tests/generator/lib.php b/tests/generator/lib.php index cc62d09..f8a8f51 100644 --- a/tests/generator/lib.php +++ b/tests/generator/lib.php @@ -62,18 +62,21 @@ public function test_create_preparation() { $user = $generator->create_user(['username' => 'user', 'lastaccess' => $tendaysago, 'suspended' => '0']); $user->realusername = $user->username; + $userneverloggedin = $generator->create_user(['username' => 'userneverloggedin', 'suspended' => '0']); $userneverloggedin->realusername = $userneverloggedin->username; + $useroneyearnotloggedin = $generator->create_user(['username' => 'useroneyearnotloggedin', 'lastaccess' => $timestamponeyearago, 'suspended' => '0']); $useroneyearnotloggedin->realusername = $userneverloggedin->username; + $usersuspendedbypluginandmanually = $generator->create_user(['username' => 'anonym-x', 'suspended' => '1']); - $usersuspendedbypluginandmanually->realusername = 'Somerealusername'; + $usersuspendedbypluginandmanually->realusername = 'somerealusername'; $DB->insert_record_raw('tool_cleanupusers', ['id' => $usersuspendedbypluginandmanually->id, 'archived' => 1, 'timestamp' => $tendaysago], true, false, true); $DB->insert_record_raw('tool_cleanupusers_archive', ['id' => $usersuspendedbypluginandmanually->id, - 'username' => 'Somerealusername', 'suspended' => $usersuspendedbypluginandmanually->suspended, + 'username' => 'somerealusername', 'suspended' => $usersuspendedbypluginandmanually->suspended, 'lastaccess' => $tendaysago], true, false, true); $usersuspendedmanually = $generator->create_user(['username' => 'usersuspendedmanually', 'suspended' => '1']); @@ -112,6 +115,7 @@ public function test_create_preparation() { $userduplicatedname = $generator->create_user(['username' => 'duplicatedname', 'suspended' => '0', 'firstname' => 'Anonym']); $userduplicatedname->realusername = $userduplicatedname->username; + $originaluser = $generator->create_user(['username' => 'anonym-z', 'suspended' => '1', 'firstname' => 'Anonym']); $originaluser->realusername = $userduplicatedname->username; @@ -126,16 +130,16 @@ public function test_create_preparation() { $DB->insert_record_raw('tool_cleanupusers', ['id' => $originaluser->id, 'archived' => true, 'timestamp' => $tendaysago], true, false, true); - $data['user'] = $user; // logged in recently, no action - $data['userdeleted'] = $userdeleted; // already deleted, filtered by cronjob - $data['originaluser'] = $originaluser; // cannot reactivate, username busy - $data['userneverloggedin'] = $userneverloggedin; // never logged in, no action - $data['userduplicatedname'] = $userduplicatedname; // never logged in, no action - $data['useroneyearnotloggedin'] = $useroneyearnotloggedin; // suspend - $data['usersuspendedmanually'] = $usersuspendedmanually; // not marked by timechecker?, no action - $data['usersuspendedbyplugin'] = $usersuspendedbyplugin; // delete - $data['userinconsistentsuspended'] = $userinconsistentsuspended; // cannot suspend, suspended = 1 already - $data['usersuspendedbypluginandmanually'] = $usersuspendedbypluginandmanually; // reactivate + $data['user'] = $user; // Logged in recently, no action. + $data['userdeleted'] = $userdeleted; // Already deleted, filtered by cronjob. + $data['originaluser'] = $originaluser; // Cannot reactivate, username busy. + $data['userneverloggedin'] = $userneverloggedin; // Never logged in, no action. + $data['userduplicatedname'] = $userduplicatedname; // Never logged in, no action. + $data['useroneyearnotloggedin'] = $useroneyearnotloggedin; // Suspend. + $data['usersuspendedmanually'] = $usersuspendedmanually; // Not marked by timechecker?, no action. + $data['usersuspendedbyplugin'] = $usersuspendedbyplugin; // Delete. + $data['userinconsistentsuspended'] = $userinconsistentsuspended; // Cannot suspend, suspended = 1 already. + $data['usersuspendedbypluginandmanually'] = $usersuspendedbypluginandmanually; // Reactivate. return $data; // Return the user, course and group objects. } diff --git a/tests/generator_test.php b/tests/generator_test.php index e37b44a..c904c8c 100644 --- a/tests/generator_test.php +++ b/tests/generator_test.php @@ -21,6 +21,8 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ +namespace tool_cleanupusers; + /** * PHPUnit data class generator testcase * diff --git a/tests/tool_cleanupusers_test.php b/tests/tool_cleanupusers_test.php index 0c35af8..3c4587f 100644 --- a/tests/tool_cleanupusers_test.php +++ b/tests/tool_cleanupusers_test.php @@ -22,7 +22,8 @@ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -use tool_cleanupusers\task; +namespace tool_cleanupusers; +use advanced_testcase; /** * Testcase class for executing phpunit test for the moodle tool_cleanupusers plugin. @@ -31,6 +32,13 @@ * @group tool_cleanupusers * @copyright 2016/17 N Herrmann * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * + * @covers \tool_cleanupusers\archiveduser::archive_me() + * @covers \tool_cleanupusers\archiveduser::delete_me() + * @covers \tool_cleanupusers\archiveduser::activate_me() + * @covers \tool_cleanupusers\subplugin_select_form::validation() + * @covers \tool_cleanupusers\task\archive_user_task::execute() + * */ class tool_cleanupusers_test extends advanced_testcase { /** Get data from generator. @@ -49,8 +57,8 @@ protected function set_up() { * Function to test the archive_me function in the archiveduser class. User used: * Username | signed in | suspended manually | suspended by plugin | deleted * ------------------------------------------------------------------------------------------ - * user | yes | no | no | no - * @see \tool_cleanupusers\archiveduser + * user | tendaysago | no | no | no + * @see archiveduser */ public function test_archiveduser_archiveme() { global $DB; @@ -61,14 +69,14 @@ public function test_archiveduser_archiveme() { // status in the tool_cleanupusers table. // Additionally, they will be anonymized in the user table. Firstname will be 'Anonym', Username will be 'anonym + id'. - $neutraltosuspended = new \tool_cleanupusers\archiveduser( + $user = new archiveduser( $data['user']->id, $data['user']->suspended, $data['user']->lastaccess, $data['user']->realusername, $data['user']->deleted ); - $neutraltosuspended->archive_me(); + $user->archive_me(); $recordtooltable = $DB->get_record('tool_cleanupusers', ['id' => $data['user']->id]); $recordshadowtable = $DB->get_record('tool_cleanupusers_archive', ['id' => $data['user']->id]); $recordusertable = $DB->get_record('user', ['id' => $data['user']->id]); @@ -82,33 +90,51 @@ public function test_archiveduser_archiveme() { } /** - * Function to test the archive_me function in the archiveduser class. Users used: - * Username | signed in | suspended manually | suspended by plugin | deleted - * ------------------------------------------------------------------------------------------ - * suspendedtodelete | no | yes | no | no - * @see \tool_cleanupusers\archiveduser + * Function to test the delete_me function in the archiveduser class. Users used: + * Username | signed in | suspended manually | suspended by plugin | deleted + * ---------------------------------------------------------------------------------------------------- + * usersuspendedbypluginandmanually | tendaysago | yes | yes | no + * usersuspendedbyplugin | oneyearago | no | yes | no + * @see archiveduser */ public function test_archiveduser_deleteme() { global $DB; $data = $this->set_up(); $this->assertNotEmpty($data); + // Users that are deleted will be marked as deleted in the user table. // The entry the tool_cleanupusers table will be deleted. - $suspendedtodelete = new \tool_cleanupusers\archiveduser( + $usersuspendedbypluginandmanually = new archiveduser( $data['usersuspendedbypluginandmanually']->id, $data['usersuspendedbypluginandmanually']->id, $data['usersuspendedbypluginandmanually']->lastaccess, $data['usersuspendedbypluginandmanually']->realusername, $data['usersuspendedbypluginandmanually']->deleted ); - $suspendedtodelete->delete_me(); + $usersuspendedbypluginandmanually->delete_me(); $recordtooltable = $DB->get_record('tool_cleanupusers', ['id' => $data['usersuspendedbypluginandmanually']->id]); $recordusertable = $DB->get_record('user', ['id' => $data['usersuspendedbypluginandmanually']->id]); $this->assertEquals(1, $recordusertable->deleted); $this->assertNotEquals($data['usersuspendedbypluginandmanually']->id, $recordusertable->username); $this->assertNotEmpty($recordusertable); $this->assertEmpty($recordtooltable); + + $usersuspendedbyplugin = new archiveduser( + $data['usersuspendedbyplugin']->id, + $data['usersuspendedbyplugin']->id, + $data['usersuspendedbyplugin']->lastaccess, + $data['usersuspendedbyplugin']->realusername, + $data['usersuspendedbyplugin']->deleted + ); + $usersuspendedbyplugin->delete_me(); + $recordtooltable = $DB->get_record('tool_cleanupusers', ['id' => $data['usersuspendedbyplugin']->id]); + $recordusertable = $DB->get_record('user', ['id' => $data['usersuspendedbyplugin']->id]); + $this->assertEquals(1, $recordusertable->deleted); + $this->assertNotEquals($data['usersuspendedbyplugin']->id, $recordusertable->username); + $this->assertNotEmpty($recordusertable); + $this->assertEmpty($recordtooltable); + $this->resetAfterTest(true); } @@ -117,8 +143,8 @@ public function test_archiveduser_deleteme() { * Username | signed in | suspended manually | suspended by plugin | deleted * ---------------------------------------------------------------------------------------------------- * usersuspendedbypluginandmanually | tendaysago | yes | yes | no - * usersuspendedbyplugin | oneyearago | yes | yes | no - * @see \tool_cleanupusers\archiveduser + * usersuspendedbyplugin | oneyearago | no | yes | no + * @see archiveduser */ public function test_archiveduser_activateme() { global $DB; @@ -128,26 +154,6 @@ public function test_archiveduser_activateme() { // Users that are activated will be written with their original values to the 'user' table. // The records in the 'tool_cleanupuser' and 'toll_cleanupuser_archive' table will be deleted. - $usersuspendedbyplugin = new \tool_cleanupusers\archiveduser( - $data['usersuspendedbyplugin']->id, - $data['usersuspendedbyplugin']->suspended, - $data['usersuspendedbyplugin']->lastaccess, - $data['usersuspendedbyplugin']->realusername, - $data['usersuspendedbyplugin']->deleted - ); - - $usersuspendedbyplugin->activate_me(); - $recordtooltable = $DB->get_record('tool_cleanupusers', ['id' => $data['usersuspendedbyplugin']->id]); - $recordtooltable2 = $DB->get_record('tool_cleanupusers_archive', ['id' => $data['usersuspendedbyplugin']->id]); - $recordusertable = $DB->get_record('user', ['id' => $data['usersuspendedbyplugin']->id]); - // Name changed. - $this->assertNotEquals($data['usersuspendedbyplugin']->username, $recordusertable->username); - $this->assertEquals(0, $recordusertable->suspended); - $this->assertEmpty($recordtooltable); - $this->assertEmpty($recordtooltable2); - - // Since the Shadowtable states that the user was previously suspended he/she is marked as suspended in the ... - // ... real table. $usersuspendedbypluginandmanually = new \tool_cleanupusers\archiveduser( $data['usersuspendedbypluginandmanually']->id, $data['usersuspendedbypluginandmanually']->suspended, @@ -165,77 +171,55 @@ public function test_archiveduser_activateme() { ['id' => $data['usersuspendedbypluginandmanually']->id] ); $recordusertable = $DB->get_record('user', ['id' => $data['usersuspendedbypluginandmanually']->id]); - $this->assertEquals('Somerealusername', $recordusertable->username); + $this->assertEquals('somerealusername', $recordusertable->username); $this->assertEquals(1, $recordusertable->suspended); $this->assertEmpty($recordtooltable); $this->assertEmpty($recordtooltable2); - // User has a duplicate user in the usertable with different id but same username. - $usersuspendedbypluginandmanually = new \tool_cleanupusers\archiveduser( - $data['originaluser']->id, - 0, - $data['originaluser']->lastaccess, - $data['userduplicatedname']->realusername, - $data['originaluser']->deleted - ); - $this->expectException(\tool_cleanupusers\cleanupusers_exception::class); - $usersuspendedbypluginandmanually->activate_me(); - $recordtooltableoriginaluser = $DB->get_record( - 'tool_cleanupusers', - ['id' => $data['originaluser']->id] - ); - $recordtooltable2originaluser = $DB->get_record( - 'tool_cleanupusers_archive', - ['id' => $data['originaluser']->id] + $usersuspendedbyplugin = new archiveduser( + $data['usersuspendedbyplugin']->id, + $data['usersuspendedbyplugin']->suspended, + $data['usersuspendedbyplugin']->lastaccess, + $data['usersuspendedbyplugin']->realusername, + $data['usersuspendedbyplugin']->deleted ); - $recordusertableoriginal = $DB->get_record('user', ['id' => $data['originaluser']->id]); - $recordusertableduplicate = $DB->get_record('user', ['id' => $data['userduplicatedname']->id]); - $this->assertEquals('duplicatedname', $recordusertableoriginal->username); - $this->assertNotEquals('duplicatedname', $recordusertableduplicate->username); - $this->assertEquals(0, $recordusertableoriginal->suspended); - $this->assertEmpty($recordtooltableoriginaluser); - $this->assertEmpty($recordtooltable2originaluser); + $usersuspendedbyplugin->activate_me(); + $recordtooltable = $DB->get_record('tool_cleanupusers', ['id' => $data['usersuspendedbyplugin']->id]); + $recordtooltable2 = $DB->get_record('tool_cleanupusers_archive', ['id' => $data['usersuspendedbyplugin']->id]); + $recordusertable = $DB->get_record('user', ['id' => $data['usersuspendedbyplugin']->id]); + $this->assertEquals('usersuspendedbyplugin', $recordusertable->username); + $this->assertEquals(0, $recordusertable->suspended); + $this->assertEmpty($recordtooltable); + $this->assertEmpty($recordtooltable2); $this->resetAfterTest(true); } /** * Tries to archive users which cannot be archived and therefore throws exception. - * Only uses an admin user and a user that was already suspended manually. - * @throws \tool_cleanupusers\cleanupusers_exception + * Only uses a user that was already suspended manually. + * Username | signed in | suspended manually | suspended by plugin | deleted + * -------------------------------------------------------------------------------------------------------- + * usersuspendedmanually | - | yes | no | no + * @throws cleanupusers_exception * @throws dml_exception */ public function test_exception_archiveme() { - global $DB, $USER; + global $DB; $data = $this->set_up(); $this->assertNotEmpty($data); - $this->setAdminUser(); - // Admin Users will not be archived. - $this->setAdminUser(); - $adminaccount = new \tool_cleanupusers\archiveduser( - $USER->id, - $USER->suspended, - $USER->lastaccess, - $USER->username, - $USER->deleted - ); $this->expectException('tool_cleanupusers\cleanupusers_exception'); - $this->expectExceptionMessage('Not able to suspend user'); - $adminaccount->archive_me(); - $recordtooltable = $DB->get_record('tool_cleanupusers', ['id' => $USER->id]); - $this->assertEmpty($recordtooltable); // Trying to suspend a user that is already manually suspended will throw an exception. - $suspendedmanually = new \tool_cleanupusers\archiveduser( + + $usersuspendedmanually = new archiveduser( $data['usersuspendedmanually']->id, $data['usersuspendedmanually']->suspended, $data['usersuspendedmanually']->lastaccess, $data['usersuspendedmanually']->realusername, $data['usersuspendedmanually']->deleted ); - $this->expectException('tool_cleanupusers\cleanupusers_exception'); - $this->expectExceptionMessage('Not able to suspend user'); - $suspendedmanually->archive_me(); + $usersuspendedmanually->archive_me(); $recordtooltable = $DB->get_record('tool_cleanupusers', ['id' => $data['usersuspendedmanually']->id]); $this->assertEmpty($recordtooltable); @@ -248,26 +232,26 @@ public function test_exception_archiveme() { * -------------------------------------------------------------------------------------------------------- * userdeleted | oneyearago | no | yes | yes * userinconsistentsuspended | oneyearago | no | partly | no - * admin | - | no | no | no - * @throws \tool_cleanupusers\cleanupusers_exception + * usersuspendedmanually | - | yes | no | no + * @throws cleanupusers_exception * @throws dml_exception */ public function test_exception_deleteme() { - global $DB, $USER; + global $DB; $data = $this->set_up(); $this->assertNotEmpty($data); + $this->expectException('tool_cleanupusers\cleanupusers_exception'); // Trying to delete a user that is already deleted will throw an exception. - $alreadydeleted = new \tool_cleanupusers\archiveduser( + + $userdeleted = new archiveduser( $data['userdeleted']->id, $data['userdeleted']->suspended, $data['userdeleted']->lastaccess, $data['userdeleted']->realusername, $data['userdeleted']->deleted ); - $this->expectException('tool_cleanupusers\cleanupusers_exception'); - $this->expectExceptionMessage('Not able to delete user'); - $alreadydeleted->delete_me(); + $userdeleted->delete_me(); $recordtooltable = $DB->get_record('tool_cleanupusers', ['id' => $data['userdeleted']->id]); $recordtooltable2 = $DB->get_record('tool_cleanupusers_archive', ['id' => $data['userdeleted']->id]); $recordusertable = $DB->get_record('user', ['id' => $data['userdeleted']->id]); @@ -278,37 +262,36 @@ public function test_exception_deleteme() { // Remark: There is no need to set expected exception multiple times, it is set for the whole method. // Deleting a user who was inconsistently stored by the plugin (only in one table) will throw an exception. - $alreadydeleted = new \tool_cleanupusers\archiveduser( + + $userinconsistentsuspended = new archiveduser( $data['userinconsistentsuspended']->id, $data['userinconsistentsuspended']->suspended, $data['userinconsistentsuspended']->lastaccess, $data['userinconsistentsuspended']->realusername, $data['userinconsistentsuspended']->deleted ); - $alreadydeleted->delete_me(); + $userinconsistentsuspended->delete_me(); $recordtooltable = $DB->get_record('tool_cleanupusers', ['id' => $data['userinconsistentsuspended']->id]); $recordtooltable2 = $DB->get_record('tool_cleanupusers_archive', ['id' => $data['userinconsistentsuspended']->id]); $recordusertable = $DB->get_record('user', ['id' => $data['userinconsistentsuspended']->id]); - $this->assertEquals(1, $recordusertable->deleted); - $this->assertEquals($data['userinconsistentsuspended']->username, $recordusertable->username); - $this->assertNotEmpty($recordtooltable); - $this->assertEmpty($recordtooltable2); + $this->assertEquals(0, $recordusertable->deleted); + $this->assertEquals('userinconsistentarchivedbyplugin', $recordusertable->username); + $this->assertEmpty($recordtooltable); + $this->assertNotEmpty($recordtooltable2); - // Admins can not be deleted. - $this->setAdminUser(); - $adminaccount = new \tool_cleanupusers\archiveduser( - $USER->id, - $USER->suspended, - $USER->lastaccess, - $USER->username, - $USER->deleted + $usersuspendedmanually = new archiveduser( + $data['usersuspendedmanually']->id, + $data['usersuspendedmanually']->suspended, + $data['usersuspendedmanually']->lastaccess, + $data['usersuspendedmanually']->realusername, + $data['usersuspendedmanually']->deleted ); - $adminaccount->delete_me(); - $recordtooltable = $DB->get_record('tool_cleanupusers', ['id' => $USER->id]); - $recordtooltable2 = $DB->get_record('tool_cleanupusers_archive', ['id' => $USER->id]); - $recordusertable = $DB->get_record('user', ['id' => $USER->id]); - $this->assertEquals(1, $recordusertable->deleted); - $this->assertEquals($USER->username, $recordusertable->username); + $usersuspendedmanually->delete_me(); + $recordtooltable = $DB->get_record('tool_cleanupusers', ['id' => $data['usersuspendedmanually']->id]); + $recordtooltable2 = $DB->get_record('tool_cleanupusers_archive', ['id' => $data['usersuspendedmanually']->id]); + $recordusertable = $DB->get_record('user', ['id' => $data['usersuspendedmanually']->id]); + $this->assertEquals(0, $recordusertable->deleted); + $this->assertEquals('usersuspendedmanually', $recordusertable->username); $this->assertEmpty($recordtooltable); $this->assertEmpty($recordtooltable2); @@ -320,51 +303,73 @@ public function test_exception_deleteme() { * Username | signed in | suspended manually | suspended by plugin | deleted * -------------------------------------------------------------------------------------------------------- * userinconsistentsuspended | oneyearago | no | partly | no - * admin | - | no | no | no - * @throws \tool_cleanupusers\cleanupusers_exception + * originaluser | tendaysago | no | yes | no + * usersuspendedmanually | - | yes | no | no + * @throws cleanupusers_exception * @throws dml_exception */ public function test_exception_activateme() { - global $DB, $USER; + global $DB; $data = $this->set_up(); $this->assertNotEmpty($data); - - // Admins can not be deleted. - $this->setAdminUser(); - $adminaccount = new \tool_cleanupusers\archiveduser( - $USER->id, - $USER->suspended, - $USER->lastaccess, - $USER->username, - $USER->deleted - ); $this->expectException('tool_cleanupusers\cleanupusers_exception'); - $this->expectExceptionMessage('Not able to activate user'); - $adminaccount->activate_me(); - $recordtooltable = $DB->get_record('tool_cleanupusers', ['id' => $USER->id]); - $recordtooltable2 = $DB->get_record('tool_cleanupusers_archive', ['id' => $USER->id]); - $recordusertable = $DB->get_record('user', ['id' => $USER->id]); - $this->assertEquals(1, $recordusertable->deleted); - $this->assertEquals($USER->username, $recordusertable->username); - $this->assertEmpty($recordtooltable); - $this->assertEmpty($recordtooltable2); - // Remark: There is no need to set expected exception multiple times, it is set for the whole method. - // When entry in tool_cleanupusers_archive table is deleted user can not be updated. - $useraccount = new \tool_cleanupusers\archiveduser( + $userinconsistentsuspended = new archiveduser( $data['userinconsistentsuspended']->id, $data['userinconsistentsuspended']->suspended, $data['userinconsistentsuspended']->lastaccess, $data['userinconsistentsuspended']->realusername, $data['userinconsistentsuspended']->deleted ); - $useraccount->activate_me(); + $userinconsistentsuspended->activate_me(); $recordtooltable = $DB->get_record('tool_cleanupusers', ['id' => $data['userinconsistentsuspended']->id]); $recordtooltable2 = $DB->get_record('tool_cleanupusers_archive', ['id' => $data['userinconsistentsuspended']->id]); $recordusertable = $DB->get_record('user', ['id' => $data['userinconsistentsuspended']->id]); - $this->assertEquals(1, $recordusertable->deleted); - $this->assertEquals($data['userinconsistentsuspended']->username, $recordusertable->username); - $this->assertNotEmpty($recordtooltable); + $this->assertEquals(1, $recordusertable->suspended); + $this->assertEquals('userinconsistentarchivedbyplugin', $recordusertable->username); + $this->assertEmpty($recordtooltable); + $this->assertNotEmpty($recordtooltable2); + + // User has a duplicate user in the usertable with different id but same username. + + $originaluser = new archiveduser( + $data['originaluser']->id, + 0, + $data['originaluser']->lastaccess, + $data['userduplicatedname']->realusername, + $data['originaluser']->deleted + ); + $originaluser->activate_me(); + $recordtooltableoriginaluser = $DB->get_record( + 'tool_cleanupusers', + ['id' => $data['originaluser']->id] + ); + $recordtooltable2originaluser = $DB->get_record( + 'tool_cleanupusers_archive', + ['id' => $data['originaluser']->id] + ); + $recordusertableoriginal = $DB->get_record('user', ['id' => $data['originaluser']->id]); + $recordusertableduplicate = $DB->get_record('user', ['id' => $data['userduplicatedname']->id]); + $this->assertEquals('duplicatedname', $recordusertableoriginal->username); + $this->assertEquals('duplicatedname', $recordusertableduplicate->username); + $this->assertEquals(1, $recordusertableoriginal->suspended); + $this->assertNotEmpty($recordtooltableoriginaluser); + $this->assertNotEmpty($recordtooltable2originaluser); + + $usersuspendedmanually = new archiveduser( + $data['usersuspendedmanually']->id, + $data['usersuspendedmanually']->suspended, + $data['usersuspendedmanually']->lastaccess, + $data['usersuspendedmanually']->realusername, + $data['usersuspendedmanually']->deleted + ); + $usersuspendedmanually->activate_me(); + $recordtooltable = $DB->get_record('tool_cleanupusers', ['id' => $data['usersuspendedmanually']->id]); + $recordtooltable2 = $DB->get_record('tool_cleanupusers_archive', ['id' => $data['usersuspendedmanually']->id]); + $recordusertable = $DB->get_record('user', ['id' => $data['usersuspendedmanually']->id]); + $this->assertEquals(1, $recordusertable->suspended); + $this->assertEquals('usersuspendedmanually', $recordusertable->username); + $this->assertEmpty($recordtooltable); $this->assertEmpty($recordtooltable2); $this->resetAfterTest(true); @@ -373,14 +378,14 @@ public function test_exception_activateme() { /** * Test the sub-plugin_select_form. * - * @see \tool_cleanupusers\subplugin_select_form + * @see subplugin_select_form */ public function test_subpluginform() { $data = $this->set_up(); $this->assertNotEmpty($data); // Validation with existing sub-plugin returns true. - $subpluginform = new tool_cleanupusers\subplugin_select_form(); + $subpluginform = new subplugin_select_form(); $validationdata = ["subplugin" => 'timechecker']; $return = $subpluginform->validation($validationdata, null); $this->assertEquals(true, $return); @@ -402,7 +407,7 @@ public function test_subpluginform() { * userneverloggedin | - | no | no | no | - * usersuspendedmanually | - | yes | no | no | - * useroneyearnotloggedin | oneyearago | no | no | no | suspend - * usersuspendedbyplugin | oneyearago | yes | yes | no | delete + * usersuspendedbyplugin | oneyearago | no | yes | no | delete * userinconsistentsuspended | oneyearago | no | partly | no | - * usersuspendedbypluginandmanually | tendaysago | yes | yes | no | activate * originaluser | tendaysago | no | yes | no | activate @@ -417,7 +422,7 @@ public function test_cronjob() { // Set up mail configuration. unset_config('noemailever'); $sink = $this->redirectEmails(); - $cronjob = new tool_cleanupusers\task\archive_user_task(); + $cronjob = new task\archive_user_task(); $name = $cronjob->get_name(); $this->assertEquals(get_string('archive_user_task', 'tool_cleanupusers'), $name); @@ -436,7 +441,7 @@ public function test_cronjob() { // Run cron-job with timechecker plugin. set_config('cleanupusers_subplugin', 'timechecker', 'tool_cleanupusers'); - $cronjob = new tool_cleanupusers\task\archive_user_task(); + $cronjob = new task\archive_user_task(); $cronjob->execute(); // Administrator should have received an email. $messages = $sink->get_messages(); @@ -447,23 +452,27 @@ public function test_cronjob() { $this->assertStringContainsString( 'In the last cron-job 1 users were archived', $msg - ); // useroneyearnotloggedin + ); // Useroneyearnotloggedin. $this->assertStringContainsString( 'In the last cron-job 1 users were deleted', $msg - ); // usersuspendedbyplugin + ); // Usersuspendedbyplugin. + $this->assertStringContainsString( + 'In the last cron-job 1 users were reactivated', + $msg + ); // Usersuspendedbypluginandmanually. $this->assertStringContainsString( 'In the last cron-job 1 users caused exception and could not be deleted', $msg - ); // 236465(from this function) and userdeleted, but deleted users are already filtered + ); // User 236465 (from this function) and userdeleted, but deleted users are already filtered. $this->assertStringContainsString( 'In the last cron-job 1 users caused exception and could not be suspended', $msg - ); // userinconsistentsuspended + ); // Userinconsistentsuspended. $this->assertStringContainsString( 'In the last cron-job 1 users caused exception and could not be reactivated', $msg - ); // originaluser + ); // Originaluser. // Users not changed by the Cronjob. $recordusertable = $DB->get_record('user', ['id' => $data['user']->id]); @@ -522,7 +531,6 @@ public function test_cronjob() { $this->resetAfterTest(); } - /** * * Testing equality of userdata arrays disregarding the realusername field. @@ -549,7 +557,7 @@ public function assert_user_equals($expected, $actual) { /** * Test the deprovisionuser cron-job complete event. * - * @see \tool_cleanupusers\event\deprovisionusercronjob_completed + * @see event\deprovisionusercronjob_completed */ public function test_logging() { $data = $this->set_up(); @@ -559,7 +567,7 @@ public function test_logging() { $eventsink = $this->redirectEvents(); set_config('cleanupusers_subplugin', 'timechecker', 'tool_cleanupusers'); - $cronjob = new tool_cleanupusers\task\archive_user_task(); + $cronjob = new task\archive_user_task(); $cronjob->execute(); $sink = $this->redirectEmails(); $sink->get_messages(); @@ -567,7 +575,7 @@ public function test_logging() { $eventsink->close(); $found = false; foreach ($triggered as $event) { - if ($event instanceof \tool_cleanupusers\event\deprovisionusercronjob_completed) { + if ($event instanceof event\deprovisionusercronjob_completed) { $this->assertTrue(true, 'Completion event triggered.'); $this->assertTrue($event->timecreated >= $timestamp, 'Completion event triggered correctly.'); $found = true; diff --git a/userstatus/timechecker/tests/userstatus_timechecker_test.php b/userstatus/timechecker/tests/userstatus_timechecker_test.php index eea7728..7e72e14 100644 --- a/userstatus/timechecker/tests/userstatus_timechecker_test.php +++ b/userstatus/timechecker/tests/userstatus_timechecker_test.php @@ -21,7 +21,10 @@ * @copyright 2016/17 N Herrmann * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -use userstatus_timechecker\timechecker; + +namespace userstatus_timechecker; +use advanced_testcase; + /** * The class contains a test script for the moodle userstatus_timechecker * @@ -30,6 +33,12 @@ * @group tool_cleanupusers_timechecker * @copyright 2016/17 N Herrmann * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * + * @covers \userstatus_timechecker\timechecker::get_to_suspend() + * @covers \userstatus_timechecker\timechecker::get_never_logged_in() + * @covers \userstatus_timechecker\timechecker::get_to_delete() + * @covers \userstatus_timechecker\timechecker::get_to_reactivate() + * */ class userstatus_timechecker_test extends advanced_testcase { /** diff --git a/userstatus/userstatuswwu/tests/userstatus_userstatuswwu_test.php b/userstatus/userstatuswwu/tests/userstatus_userstatuswwu_test.php index 9e21b27..f7032f1 100644 --- a/userstatus/userstatuswwu/tests/userstatus_userstatuswwu_test.php +++ b/userstatus/userstatuswwu/tests/userstatus_userstatuswwu_test.php @@ -22,8 +22,9 @@ * @copyright 2017 N Herrmann * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -defined('MOODLE_INTERNAL') || die(); -use userstatus_userstatuswwu\userstatuswwu; + +namespace userstatus_userstatuswwu; +use advanced_testcase; /** * The class contains a test script for the moodle userstatus_userstatuswwu @@ -34,6 +35,12 @@ * @group tool_cleanupusers_userstatuswwu * @copyright 2017 N Herrmann * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * + * @covers \userstatus_userstatuswwu\userstatuswwu::get_to_suspend() + * @covers \userstatus_userstatuswwu\userstatuswwu::get_never_logged_in() + * @covers \userstatus_userstatuswwu\userstatuswwu::get_to_delete() + * @covers \userstatus_userstatuswwu\userstatuswwu::get_to_reactivate() + * */ class userstatus_userstatuswwu_test extends advanced_testcase { /** diff --git a/version.php b/version.php index 47b3979..54ec775 100644 --- a/version.php +++ b/version.php @@ -23,7 +23,7 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2023061300; // The current plugin version (Date: YYYYMMDDXX). +$plugin->version = 2023120500; // The current plugin version (Date: YYYYMMDDXX). $plugin->requires = 2022041900; // Requires Moodle version 4.0 or higher. $plugin->component = 'tool_cleanupusers'; // Full name of the plugin (used for diagnostics). $plugin->release = 'v1.0-r2';