Skip to content
This repository has been archived by the owner on Feb 21, 2024. It is now read-only.

Commit

Permalink
fix unit tests and activate_me() duplicate check, throw an exception …
Browse files Browse the repository at this point in the history 
…for users not suspended by plugin (on deletion/activation)
  • Loading branch information
@my-curiosity
my-curiosity committed Dec 5, 2023
1 parent 3a3dcd7 commit eb2a0f0
Show file tree
Hide file tree
Showing 9 changed files with 232 additions and 196 deletions.
57 changes: 32 additions & 25 deletions classes/archiveduser.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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;
Expand Down Expand Up @@ -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() {
Expand All @@ -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) {
Expand All @@ -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.
*
Expand All @@ -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);
Expand Down
7 changes: 4 additions & 3 deletions classes/table/never_logged_in_table.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@

use core_user\fields;

defined('MOODLE_INTERNAL') || die();

/**
* Create a class for a custom sql_table for the tool_cleanupusers
*
Expand Down Expand Up @@ -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);
}

/**
Expand Down
2 changes: 0 additions & 2 deletions classes/table/users_table.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@

use core_user\fields;

defined('MOODLE_INTERNAL') || die();

/**
* Create a class for a custom sql_table for the tool_cleanupusers
*
Expand Down
28 changes: 16 additions & 12 deletions tests/generator/lib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
Expand Down Expand Up @@ -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;
Expand All @@ -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.
}
Expand Down
2 changes: 2 additions & 0 deletions tests/generator_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down
Loading

0 comments on commit eb2a0f0

Please sign in to comment.