Merge pull request #199 from catalyst/issue-195-fix-sql-for-master-branch

issue 195: fix SQL for checking recent logins

Author: ScottVerbeek

cli/clean.php

@@ -15,6 +15,8 @@
 // along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
 
 /**
+ * CLI script to perform a data wash.
+ *
  * @package    local_datacleaner
  * @copyright  2015 Brendan Heywood <brendan@catalyst-au.net>
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
@@ -28,7 +30,7 @@
 
 // Now get cli options.
 list($options, $unrecognized) = cli_get_params(
-    array(
+    [
         'help' => false,
         'force' => false,
         'filter' => false,
@@ -38,11 +40,11 @@
         'dryrun' => false,
         'verbose' => false,
         'reset' => false,
-    ),
-    array(
+    ],
+    [
         'h' => 'help',
         'v' => 'verbose',
-    )
+    ]
 );
 
 if ($unrecognized) {

cli/lib.php

@@ -15,13 +15,14 @@
 // along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
 
 /**
- * @package    cleaner
+ * Library functions for data cleaner.
+ *
+ * @package    local_datacleaner
  * @copyright  2015 Catalyst IT
  * @author     Nigel Cunningham <nigelc@catalyst-au.net>
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
-defined('MOODLE_INTERNAL') || die();
 
 /**
  * Print a message to the terminal.
@@ -89,43 +90,33 @@ function safety_checks($dryrun) {
     $minutes = $timetoshowusers / 60;
     $now = time();
     $timefrom = $now - $timetoshowusers; // Unlike original code, don't care about caches for this.
-    $params = array('now' => $now, 'timefrom' => $timefrom);
-
-    $csql = "SELECT COUNT(u.id)
-               FROM {user} u
-              WHERE u.lastaccess > :timefrom
-                AND u.lastaccess <= :now
-                AND u.deleted = 0";
-
-    if ($DB->count_records_sql($csql, $params)) {
-        $namefields = "u." . implode(', u.', get_all_user_name_fields());
-
-        $sql = "SELECT u.id, u.username, {$namefields}
-                  FROM {user} u
-                 WHERE u.lastaccess > :timefrom
-                   AND u.lastaccess <= :now
-                   AND u.deleted = 0
-              GROUP BY u.id
-              ORDER BY lastaccess DESC ";
-        $users = $DB->get_records_sql($sql, $params);
-
-        $message = "The following users have logged in within the last {$minutes} minutes:\n";
-        $nonadmins = 0;
-        foreach ($users as $user) {
-            $message .= ' - ' . fullname($user) . ' (' . $user->username . ')';
-            if (is_siteadmin($user)) {
-                $message .= ' (siteadmin)';
-            } else {
-                $nonadmins++;
-            }
-            $message .= "\n";
+    $params = ['timefrom' => $timefrom];
+
+    $namefields = "u." . implode(', u.', \core_user\fields::get_name_fields());
+
+    $sql = "SELECT u.id, u.username, {$namefields}
+              FROM {user} u
+             WHERE u.lastaccess > :timefrom
+               AND u.deleted = 0
+             ORDER BY lastaccess DESC ";
+    $users = $DB->get_records_sql($sql, $params);
+
+    $message = "The following users have logged in within the last {$minutes} minutes:\n";
+    $nonadmins = 0;
+    foreach ($users as $user) {
+        $message .= ' - ' . fullname($user) . ' (' . $user->username . ')';
+        if (is_siteadmin($user)) {
+            $message .= ' (siteadmin)';
+        } else {
+            $nonadmins++;
         }
+        $message .= "\n";
+    }
 
-        if ($nonadmins) {
-            abort_message($abort, $message);
-            abort_message("There are non site-administrators in the list of recent users.", true);
-            $willdie = true;
-        }
+    if ($nonadmins) {
+        abort_message($abort, $message);
+        abort_message($abort, "There are non site-administrators in the list of recent users.", true);
+        $willdie = true;
     }
 
     // 3. Has cron run recently?