#268 - fix coding styles; improved echoing on CLI call

.github/workflows/moodle-ci.yml

@@ -28,10 +28,24 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        php: ['8.0', '8.1']
-        moodle-branch: ['MOODLE_402_STABLE', 'master']
+        php: ['8.0', '8.1', '8.2']
+        moodle-branch: ['MOODLE_402_STABLE', 'MOODLE_403_STABLE', 'MOODLE_404_STABLE']
         database: ['pgsql', 'mariadb']
         extensions: ["mbstring, pgsql, mysqli"]
+        # Whenever necessary:
+        # https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#example-including-new-combinations
+        exclude:
+          - moodle-branch: 'MOODLE_404_STABLE'
+            php: '8.0'
+        include:
+          - moodle-branch: 'MOODLE_404_STABLE'
+            php: '8.3'
+            extensions: "mbstring, pgsql, mysqli"
+            database: 'pgsql'
+          - moodle-branch: 'master'
+            php: '8.3'
+            extensions: "mbstring, pgsql, mysqli"
+            database: 'mariadb'
 
     steps:
       - name: Check out repository code

CHANGES.md

@@ -1,6 +1,11 @@
 Release notes
 =============
 
+2024060300
+
+1. 2024-06-03 #268 - CI: verify it is working from M4.1 to M4.4 and master. Solves #263 too.
+2. 2024-06-03 #268 - Fixed PHPDoc issues and revisited output from merger when running from CLI.
+
 2023061900
 
 1. 2023-06-19 #245 - Add list of incremental changes on file CHANGES.md.

lang/en/tool_mergeusers.php

@@ -1,4 +1,18 @@
 <?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
 
 /**
  * Define default English language strings for report
@@ -52,7 +66,7 @@
 into the second one (toid). The user 'toid' will include data from both users.";
 $string['cligathering:stopping'] = 'To stop merging, Ctrl+C or type -1 either on fromid or toid fields.';
 $string['cligathering:fromid'] = 'Source user id (fromid):';
-$string['cligathering:toid'] =   'Target user id   (toid):';
+$string['cligathering:toid'] = 'Target user id   (toid):';
 $string['viewlog'] = 'See merging logs';
 $string['loglist'] = 'All these records are merging actions done, showing if they went ok:';
 $string['newuseridonlog'] = 'User kept';
@@ -69,7 +83,7 @@
 $string['eventusermergedsuccess'] = 'Merging success';
 $string['eventusermergedfailure'] = 'Merge failed';
 
-// Settings page
+// Settings page.
 $string['transactions_setting'] = 'Only transactions allowed';
 $string['transactions_setting_desc'] = 'If enabled, merge users will not work
     at all on databases that do NOT support transactions (recommended).
@@ -94,14 +108,14 @@
     If you prefer, you can exclude any of those tables and include them in the
     merging process (not recommended).';
 
-//New strings
+// New strings.
 
-// Progress bar
+// Progress bar.
 $string['choose_users'] = 'Choose users to merge';
 $string['review_users'] = 'Confirm users to merge';
 $string['results'] = 'Merging results and log';
 
-// Form Strings
+// Form Strings.
 $string['form_header'] = 'Find users to merge';
 $string['form_description'] = '<p>You may search for users here if you don\'t
     know the user\'s username/id number. Otherwise you may expand the form to
@@ -121,21 +135,21 @@
     Are you sure you want to continue?';
 $string['clear_selection'] = 'Clear current user selection';
 
-// Merge users select table
+// Merge users select table.
 $string['olduser'] = 'User to remove';
 $string['newuser'] = 'User to keep';
 $string['saveselection_submit'] = 'Save selection';
 $string['userselecttable_legend'] = '<b>Select users to merge</b>';
 
-// Merge users review table
+// Merge users review table.
 $string['userreviewtable_legend'] = '<b>Review users to merge</b>';
 
-// Error string
+// Error string.
 $string['error_return'] = 'Return to search form';
 $string['no_saveselection'] = 'You did not select either an old or new user.';
 $string['invalid_option'] = 'Invalid form option';
 
-// Settings page
+// Settings page.
 $string['suspenduser_setting'] = 'Suspend old user';
 $string['suspenduser_setting_desc'] = 'If enabled, it suspends the old user
     automatically upon a succesful merging process, preventing the user
@@ -153,7 +167,7 @@
     in your Moodle installation, you can be quiet on running this plugin
     enabling or disabling this option.';
 
-// quiz attempts strings
+// Quiz attempts strings.
 $string['quizattemptsaction'] = 'How to resolve quiz attempts';
 $string['quizattemptsaction_desc'] = 'When merging quiz attempts there may exist three cases:
     <ol>
@@ -184,10 +198,10 @@
 $string['qa_grades'] = 'Grades recalculated for quizzes: {$a}.';
 
 $string['uniquekeynewidtomaintain'] = 'Keep new user\'s data';
-$string['uniquekeynewidtomaintain_desc'] = 'In case of conflict, 
-    like when the user.id related column is a unique key, this plugin will keep 
-    data from new user (by default). This also means that data from old user is 
-    deleted to keep the consistence. Otherwise, if you uncheck this option, 
+$string['uniquekeynewidtomaintain_desc'] = 'In case of conflict,
+    like when the user.id related column is a unique key, this plugin will keep
+    data from new user (by default). This also means that data from old user is
+    deleted to keep the consistence. Otherwise, if you uncheck this option,
     data from old user will be kept.';
 
 $string['starttime'] = 'Started merging at {$a}';

lib/merger.php

@@ -22,7 +22,11 @@
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
-require_once __DIR__ . '/autoload.php';
+defined('MOODLE_INTERNAL') || die;
+
+global $CFG;
+require_once($CFG->dirroot . '/lib/clilib.php');
+require_once(__DIR__ . '/autoload.php');
 
 class Merger {
     /**
@@ -38,14 +42,11 @@ public function __construct(MergeUserTool $mut) {
         $this->mut = $mut;
         $this->logger = new tool_mergeusers_logger();
 
-        // to catch Ctrl+C interruptions, we need this stuff.
+        // To catch Ctrl+C interruptions, we need this stuff.
         declare(ticks = 1);
 
         if (extension_loaded('pcntl')) {
-            pcntl_signal(SIGINT, array(
-                $this,
-                'aborting'
-            ));
+            pcntl_signal(SIGINT, [$this, 'aborting']);
         }
     }
 
@@ -55,9 +56,9 @@ public function __construct(MergeUserTool $mut) {
      */
     public function aborting($signo) {
         if (defined("CLI_SCRIPT")) {
-            echo "\n\n" . get_string('ok') . ", exit!\n\n";
+            echo "\n\nAborting!\n\n";
         }
-        exit(0); //quiting normally after all ;-)
+        exit(0); // Exiting without error.
     }
 
     /**
@@ -66,16 +67,23 @@ public function aborting($signo) {
      * @param Gathering $gathering List of merging actions.
      */
     public function merge(Gathering $gathering) {
+        $numberoperations = 0;
         foreach ($gathering as $action) {
             list($success, $log, $id) = $this->mut->merge($action->toid, $action->fromid);
 
-            // only shows results on cli script
+            // Only shows results on cli script.
             if (defined("CLI_SCRIPT")) {
-                echo (($success)?get_string("success"):get_string("error")) . ". Log id: " . $id . "\n\n";
+                $status = ($success) ? "Success" : "Error";
+
+                cli_writeln('');
+                cli_writeln("From {$action->fromid} to {$action->toid}: $status; Log id: $id");
+                cli_writeln('');
             }
+            $numberoperations++;
         }
+
         if (defined("CLI_SCRIPT")) {
-            echo get_string('ok') .", exit!\n\n";
+            cli_writeln("${numberoperations} merge operations performed. Bye!");
         }
     }
 }

lib/table/quizattemptsmerger.php

@@ -1,5 +1,4 @@
 <?php
-
 // This file is part of Moodle - http://moodle.org/
 //
 // Moodle is free software: you can redistribute it and/or modify
@@ -15,7 +14,6 @@
 // You should have received a copy of the GNU General Public License
 // along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
 
-use mod_quiz\grade_calculator;
 use mod_quiz\quiz_settings;
 
 defined('MOODLE_INTERNAL') || die();
@@ -145,7 +143,7 @@ protected function renumber($data, &$actionLog, &$errorMessages)
 
         $tableName = $CFG->prefix . $data['tableName'];
 
-        // we want to find all quiz attempts made from both users if any.
+        // We want to find all quiz attempts made from both users if any.
         $sql = "
             SELECT *
             FROM
@@ -157,7 +155,7 @@ protected function renumber($data, &$actionLog, &$errorMessages)
 
         $allAttempts = $DB->get_records_sql($sql, array($data['fromid'], $data['toid']));
 
-        // when there are attempts, check what we have to do with them.
+        // When there are attempts, check what we have to do with them.
         if ($allAttempts) {
 
             $toid = $data['toid'];
@@ -166,25 +164,25 @@ protected function renumber($data, &$actionLog, &$errorMessages)
                 ' WHERE id = ',
             );
 
-            // list of quiz ids necessary to recalculate.
+            // List of quiz ids necessary to recalculate.
             $quizzes = array();
-            // list of attempts organized by quiz id
+            // List of attempts organized by quiz id.
             $attemptsByQuiz = array();
-            // list of users that have attempts per quiz
+            // List of users that have attempts per quiz.
             $userids = array();
 
-            // organize all attempts by quiz and userid
+            // Organize all attempts by quiz and userid.
             foreach ($allAttempts as $attempt) {
                 $attemptsByQuiz[$attempt->quiz][] = $attempt;
                 $userids[$attempt->quiz][$attempt->userid] = $attempt->userid;
             }
 
-            // processing attempts quiz by quiz
+            // Processing attempts quiz by quiz.
             foreach ($attemptsByQuiz as $quiz => $attempts) {
 
-                // do nothing when there is only the target user.
+                // Do nothing when there is only the target user.
                 if (count($userids[$quiz]) === 1 && isset($userids[$quiz][$toid])) {
-                    // all attempts are for the target user only; do nothing.
+                    // All attempts are for the target user only; do nothing.
                     continue;
                 }
 
@@ -204,11 +202,11 @@ protected function renumber($data, &$actionLog, &$errorMessages)
                 // the $max value to their attempt column.
                 //
                 //
-                // total number of attempts from both users.
+                // Total number of attempts from both users.
                 $max = count($attempts);
-                // update the list of quiz ids to be recalculated its grade.
+                // Update the list of quiz ids to be recalculated its grade.
                 $quizzes[$quiz] = $quiz;
-                // number of attempt when renumbering
+                // Number of attempt when renumbering
                 $nattempt = 1;
 
                 // Renumber all attempts and updating userid when necessary.
@@ -230,11 +228,10 @@ protected function renumber($data, &$actionLog, &$errorMessages)
                     }
 
                     $nattempt++;
-                    unset($sets); // free mem
+                    unset($sets); // Free mem.
                 }
 
-                // Remove the offset of $max from their attempt column to make
-                // them start by 1 as expected.
+                // Remove the offset of $max from their attempt column to make them start by 1 as expected.
                 $updateAll = "UPDATE " . $tableName .
                     " SET attempt = attempt - $max " .
                     " WHERE quiz = $quiz AND userid = $toid";
@@ -247,7 +244,7 @@ protected function renumber($data, &$actionLog, &$errorMessages)
                 }
             }
 
-            // recalculate grades for updated quizzes.
+            // Recalculate grades for updated quizzes.
             $this->updateAllQuizzes($data, $quizzes, $actionLog);
         }
     }
@@ -276,7 +273,7 @@ protected function updateAllRecords($data, $recordsToModify, $fieldName, &$actio
     protected function updateAllQuizzes($data, $ids, &$actionLog)
     {
         if (empty($ids)) {
-            // if no ids... do nothing.
+            // If no ids... do nothing.
             return;
         }
 
@@ -302,8 +299,8 @@ protected function updateQuizzes(array $ids, array &$actionLog)
         if ($quizzes) {
             $actionLog[] = get_string('qa_grades', 'tool_mergeusers', implode(', ', array_keys($quizzes)));
             foreach ($quizzes as $quiz) {
-                // https://moodle.org/mod/forum/discuss.php?d=258979
-                // recalculate grades for affected quizzes.
+                // See https://moodle.org/mod/forum/discuss.php?d=258979.
+                // Recalculate grades for affected quizzes.
                 $quizobj = quiz_settings::create($quiz->id);
                 $quizobj->get_grade_calculator()->recompute_all_final_grades();
             }

settings.php

@@ -61,19 +61,19 @@
         get_string('suspenduser_setting_desc', 'tool_mergeusers'),
         1));
 
-    $supporting_lang = (tool_mergeusers_transactionssupported()) ? 'transactions_supported' : 'transactions_not_supported';
+    $supportinglang = (tool_mergeusers_transactionssupported()) ? 'transactions_supported' : 'transactions_not_supported';
 
     $settings->add(new admin_setting_configcheckbox('tool_mergeusers/transactions_only',
         get_string('transactions_setting', 'tool_mergeusers'),
         get_string('transactions_setting_desc', 'tool_mergeusers') . '<br /><br />' .
-            get_string($supporting_lang, 'tool_mergeusers'),
+            get_string($supportinglang, 'tool_mergeusers'),
         1));
 
     $exceptionoptions = tool_mergeusers_build_exceptions_options();
     $settings->add(new admin_setting_configmultiselect('tool_mergeusers/excluded_exceptions',
         get_string('excluded_exceptions', 'tool_mergeusers'),
         get_string('excluded_exceptions_desc', 'tool_mergeusers', $exceptionoptions->defaultvalue),
-        array($exceptionoptions->defaultkey), //default value: empty => apply all exceptions.
+        array($exceptionoptions->defaultkey), // Default value: empty => apply all exceptions.
         $exceptionoptions->options));
 
     // Quiz attempts.
@@ -90,6 +90,6 @@
         get_string('uniquekeynewidtomaintain_desc', 'tool_mergeusers'),
         1));
 
-    // Add settings
+    // Add settings.
     $ADMIN->add('tools', $settings);
 }

tests/quiz_test.php

@@ -13,6 +13,7 @@
 //
 // You should have received a copy of the GNU General Public License
 // along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
 use mod_quiz\quiz_settings;
 
 /**

version.php

@@ -30,7 +30,7 @@
 
 defined('MOODLE_INTERNAL') || die;
 
-$plugin->version   = 2023061900;
+$plugin->version   = 2024060300;
 $plugin->requires  = 2023042400; // Moodle 4.2, 24 April 2023, https://moodledev.io/general/releases#moodle-42
 $plugin->component = 'tool_mergeusers';
 $plugin->maturity = MATURITY_STABLE;