Merge branch 'master' into fix/removeuserstatuswwu

README.md

@@ -46,7 +46,7 @@ Moodle provides the following functionality when suspending a user:
 The plugin aims to make users that are suspended **anonymous**. Therefore, the following additional functionalities are provided:  
 - save necessary data in a shadow table to reactivate users when necessary (the table is called: `tool_cleanupusers_archive`),
 - hide all other references in the `user` table e.g. `username`, ` firstname`.
-    - The `username` is set to *Anonym* with the `userid` appended  
+    - The `username` is set to *anonym* with the `userid` appended  
       (usernames must be unique therefore the id is appended).
     - The field `firstname` is set to *Anonym*.
         - Consequently, references in e.g. the forum activity merely refer to a user called `Anonym`.

classes/archiveduser.php

@@ -174,7 +174,7 @@ public function delete_me() {
 
         $user = \core_user::get_user($this->id);
 
-        if ($user != false && $user->deleted == 0 && !is_siteadmin($user)) {
+        if ($user && $user->deleted == 0 && !is_siteadmin($user)) {
 
             $transaction = $DB->start_delegated_transaction();
 

classes/table/never_logged_in_table.php

@@ -14,16 +14,21 @@
 // You should have received a copy of the GNU General Public License
 // along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
 /**
- * Create an Custom sql_table for the tool_cleanupusers
+ * Create a custom sql_table for the tool_cleanupusers
  *
  * @package   tool_cleanupusers
  * @copyright 2018 N. Herrmann
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
 namespace tool_cleanupusers\table;
+
+use core_user\fields;
+
+defined('MOODLE_INTERNAL') || die();
+
 /**
- * Create a class for an Custom sql_table for the tool_cleanupusers
+ * Create a class for a custom sql_table for the tool_cleanupusers
  *
  * @package   tool_cleanupusers
  * @copyright 2018 N. Herrmann
@@ -61,7 +66,7 @@ public function __construct($users, $sqlwhere, $param) {
             $where .= ' AND ' . $sqlwhere;
         }
 
-        $this->set_sql('id, username, lastaccess, suspended, ' . get_all_user_name_fields(true), '{user}', $where, $param);
+        $this->set_sql('id, username, lastaccess, suspended, ' .implode(', ', fields::get_name_fields()), '{user}', $where, $param);
     }
 
     /**

classes/table/users_table.php

@@ -14,16 +14,21 @@
 // You should have received a copy of the GNU General Public License
 // along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
 /**
- * Create an Custom sql_table for the tool_cleanupusers
+ * Create a custom sql_table for the tool_cleanupusers
  *
  * @package   tool_cleanupusers
  * @copyright 2019 Justus Dieckmann
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
 namespace tool_cleanupusers\table;
+
+use core_user\fields;
+
+defined('MOODLE_INTERNAL') || die();
+
 /**
- * Create a class for an Custom sql_table for the tool_cleanupusers
+ * Create a class for a custom sql_table for the tool_cleanupusers
  *
  * @package   tool_cleanupusers
  * @copyright 2019 Justus Dieckmann
@@ -60,6 +65,6 @@ public function __construct($uniqueid, $users, $sqlwhere, $param) {
         if ($sqlwhere != null && $sqlwhere != '') {
             $where .= ' AND ' . $sqlwhere;
         }
-        $this->set_sql('id, username, lastaccess, ' . get_all_user_name_fields(true), '{user}', $where, $param);
+        $this->set_sql('id, username, lastaccess, ' .implode(', ', fields::get_name_fields()), '{user}', $where, $param);
     }
 }

lang/en/tool_cleanupusers.php

@@ -53,8 +53,8 @@
 $string['waittodelete'] = 'The user is suspended and will not be deleted in the next cron-job.';
 $string['e-mail-archived'] = 'In the last cron-job {$a} users were archived.';
 $string['e-mail-deleted'] = 'In the last cron-job {$a} users were deleted.';
-$string['errormessagenotactive'] = 'Not able to activate user.';
-$string['errormessagenotdelete'] = 'Not able to delete user.';
+$string['errormessagenotactive'] = 'Not able to activate user';
+$string['errormessagenotdelete'] = 'Not able to delete user';
 $string['errormessagenotsuspend'] = 'Not able to suspend user';
 $string['errormessagenoaction'] = 'The requested action could not be executed.';
 $string['errormessagesubplugin'] = 'The sub-plugin you selected is not available. The default will be used.';

tests/generator/lib.php

@@ -63,7 +63,7 @@ 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', 'lastaccess' => '',
+        $userneverloggedin = $generator->create_user(['username' => 'userneverloggedin',
             'suspended' => '0', ]);
         $userneverloggedin->realusername = $userneverloggedin->username;
         $useroneyearnotloggedin = $generator->create_user(['username' => 'useroneyearnotloggedin',
@@ -101,27 +101,27 @@ public function test_create_preparation () {
             true, false, true);
 
         $userduplicatedname = $generator->create_user(['username' => 'duplicatedname',
-            'suspended' => '1', 'firstname' => 'Anonym', ]);
+            'suspended' => '0', 'firstname' => 'Anonym', ]);
         $userduplicatedname->realusername = $userduplicatedname->username;
         $originaluser = $generator->create_user(['username' => 'anonym-z',
             'suspended' => '1', 'firstname' => 'Anonym', ]);
         $originaluser->realusername = $userduplicatedname->username;
         $DB->insert_record_raw('tool_cleanupusers_archive', ['id' => $originaluser->id,
-            'username' => $userduplicatedname->username, 'suspended' => 0, 'lastaccess' => $timestamponeyearago, ],
+            'username' => $userduplicatedname->username, 'suspended' => 0, 'lastaccess' => $tendaysago, ],
             true, false, true);
         $DB->insert_record_raw('tool_cleanupusers', ['id' => $originaluser->id, 'archived' => true,
             'timestamp' => $tendaysago, ], true, false, true);
 
-        $data['user'] = $user;
-        $data['userdeleted'] = $userdeleted;
-        $data['originaluser'] = $originaluser;
-        $data['userneverloggedin'] = $userneverloggedin;
-        $data['userduplicatedname'] = $userduplicatedname;
-        $data['useroneyearnotloggedin'] = $useroneyearnotloggedin;
-        $data['usersuspendedmanually'] = $usersuspendedmanually;
-        $data['usersuspendedbyplugin'] = $usersuspendedbyplugin;
-        $data['userinconsistentsuspended'] = $userinconsistentsuspended;
-        $data['usersuspendedbypluginandmanually'] = $usersuspendedbypluginandmanually;
+        $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.
     }

tests/tool_cleanupusers_test.php

@@ -51,7 +51,7 @@ protected function set_up() {
     }
 
     /**
-     * Function to test the the archive_me function in the archiveduser class. User used:
+     * 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
@@ -65,7 +65,7 @@ public function test_archiveduser_archiveme() {
 
         // Users that are archived will be marked as suspended in the user table and transfer their previous suspended
         // status in the tool_cleanupusers table.
-        // Additionally they will be anomynised in the user table. Firstname will be 'Anonym', Username will be 'anonym + id'.
+        // Additionally, they will be anonymized in the user table. Firstname will be 'Anonym', Username will be 'anonym + id'.
 
         $neutraltosuspended = new \tool_cleanupusers\archiveduser($data['user']->id, $data['user']->suspended,
             $data['user']->lastaccess, $data['user']->realusername, $data['user']->deleted);
@@ -96,11 +96,10 @@ public function test_archiveduser_archiveme() {
     }
 
     /**
-     * Function to test the the archive_me function in the archiveduser class. Users used:
+     * 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
-     *  userdeleted           | oneyearago    | no                 | yes                 | yes
      * @see \tool_cleanupusers\archiveduser
      * @covers \tool_cleanupusers\archiveduser delete_me function.
      */
@@ -125,7 +124,7 @@ public function test_archiveduser_deleteme() {
     }
 
     /**
-     * Function to test the the activate_me function in the archiveduser class. Users used:
+     * Function to test the activate_me function in the archiveduser class. Users used:
      * Username                         | signed in     | suspended manually | suspended by plugin | deleted
      * ----------------------------------------------------------------------------------------------------
      * usersuspendedbypluginandmanually | tendaysago    | yes                | yes                 | no
@@ -193,7 +192,8 @@ public function test_archiveduser_activateme() {
     }
 
     /**
-     * Tries to archive users which cannot be archived and therefore throws exception. Only uses a admin user.
+     * 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
      * @throws dml_exception
      * @covers \tool_cleanupusers\archiveduser archive_me function.
@@ -213,6 +213,16 @@ public function test_exception_archiveme () {
         $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($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();
+        $recordtooltable = $DB->get_record('tool_cleanupusers', array('id' => $data['usersuspendedmanually']->id));
+        $this->assertEmpty($recordtooltable);
+
         $this->resetAfterTest(true);
     }
 
@@ -232,7 +242,7 @@ public function test_exception_deleteme () {
         $data = $this->set_up();
         $this->assertNotEmpty($data);
 
-        // Trying to delete a user that is already deleted will throw a exception.
+        // Trying to delete a user that is already deleted will throw an exception.
         $alreadydeleted = new \tool_cleanupusers\archiveduser($data['userdeleted']->id, $data['userdeleted']->suspended,
             $data['userdeleted']->lastaccess, $data['userdeleted']->realusername, $data['userdeleted']->deleted);
         $this->expectException('tool_cleanupusers\cleanupusers_exception');
@@ -352,14 +362,16 @@ public function test_subpluginform() {
      * Executes and tests the cron-job. The following table illustrates what will happen to the users:
      * Username                          |   signed in    | suspended manually | suspended by plugin | deleted | action
      * -----------------------------------------------------------------------------------------------------------------
-     *  user                             | tendaysago    | no                 | no                  | no       |  -
+     *  user                             | tendaysago    | no                 | no                  | no       | -
      *  userdeleted                      | oneyearago    | no                 | yes                 | yes      | -
      *  userneverloggedin                | -             | no                 | no                  | no       | -
      *  usersuspendedmanually            | -             | yes                | no                  | no       | -
      *  useroneyearnotloggedin           | oneyearago    | no                 | no                  | no       | suspend
      *  usersuspendedbyplugin            | oneyearago    | yes                | yes                 | no       | delete
-     *  userinconsistentsuspended        | oneyearago    | no                 | partly              | no       | suspended
+     *  userinconsistentsuspended        | oneyearago    | no                 | partly              | no       | -
      *  usersuspendedbypluginandmanually | tendaysago    | yes                | yes                 | no       | activate
+     *  originaluser                     | tendaysago    | no                 | yes                 | no       | activate
+     *  userduplicatedname               | -             | no                 | no                  | no       | -
      * @throws dml_exception
      * @throws coding_exception
      * @covers \tool_cleanupusers\task\archive_user_task delete_me function.
@@ -393,11 +405,16 @@ public function test_cronjob() {
 
         $msg = str_replace(["\r\n", "\r", "\n", "<br>", "</br>"], '', $messages[0]->body);
 
-        $this->assertStringContainsString('In the last cron-job 1 users were archived', $msg);
-        $this->assertStringContainsString('In the last cron-job 2 users were deleted', $msg);
-        $this->assertStringContainsString('In the last cron-job 0 users caused exception and could not be deleted', $msg);
-        $this->assertStringContainsString('In the last cron-job 0 users caused exception and could not be suspended', $msg);
-        $this->assertStringContainsString('In the last cron-job 1 users caused exception and could not be reactivated', $msg);
+        $this->assertStringContainsString('In the last cron-job 1 users were archived',
+            $msg);  // useroneyearnotloggedin
+        $this->assertStringContainsString('In the last cron-job 1 users were deleted',
+            $msg);  // usersuspendedbyplugin
+        $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
+        $this->assertStringContainsString('In the last cron-job 1 users caused exception and could not be suspended',
+            $msg);  // userinconsistentsuspended
+        $this->assertStringContainsString('In the last cron-job 1 users caused exception and could not be reactivated',
+            $msg);  // originaluser
 
         // Users not changed by the Cronjob.
         $recordusertable = $DB->get_record('user', ['id' => $data['user']->id]);
@@ -432,12 +449,13 @@ public function test_cronjob() {
         $this->assertEquals(1, $recordusertable->suspended);
         $this->assertEquals(1, $recordusertable->deleted);
 
-        // User is suspended.
+        // User remains inconsistently suspended.
         $recordusertable = $DB->get_record('user', ['id' => $data['userinconsistentsuspended']->id]);
         $recordtooltable = $DB->get_record('tool_cleanupusers', ['id' => $data['userinconsistentsuspended']->id]);
         $recordtooltable2 = $DB->get_record('tool_cleanupusers_archive', ['id' => $data['userinconsistentsuspended']->id]);
+
         $this->assertNotEmpty($recordtooltable2);
-        $this->assertNotEmpty($recordtooltable);
+        $this->assertEmpty($recordtooltable);
         $this->assertEquals(1, $recordusertable->suspended);
         $this->assertEquals(0, $recordusertable->deleted);
 
@@ -477,7 +495,7 @@ private function assert_user_equals($expected, $actual) {
     }
 
     /**
-     * Test the the deprovisionuser cron-job complete event.
+     * Test the deprovisionuser cron-job complete event.
      *
      * @see \tool_cleanupusers\event\deprovisionusercronjob_completed
      * @covers \tool_cleanupusers\task\archive_user_task logging.

userstatus/timechecker/tests/generator/lib.php

@@ -72,11 +72,12 @@ public function test_create_preparation () {
 
         // User suspended by the plugin.
         $tendaysago = $mytimestamp - 864000;
-        $reactivate = $generator->create_user(['username' => 'Anonym', 'suspended' => 1]);
+        $reactivate = $generator->create_user(['username' => 'anonym', 'suspended' => 1]);
         $DB->insert_record_raw('tool_cleanupusers', ['id' => $reactivate->id, 'archived' => true,
             'timestamp' => $tendaysago, ], true, false, true);
         $DB->insert_record_raw('tool_cleanupusers_archive', ['id' => $reactivate->id, 'username' => 'reactivate',
             'suspended' => 1, 'lastaccess' => $tendaysago, ], true, false, true);
+
         $data['reactivate'] = $reactivate;
 
         return $data; // Return the user, course and group objects.