Converting Moodle 1.9 Plug-ins to Moodle 2 - Activity Module Upgrade - Part 2

This is the second part in my Moodle 1.9 to Moodle 2 activity module migration.

When I left off, I had modified the installation and upgrade code using the new XMLDB/DDL changes. Now I'm going to focus on the DML changes.

The most obvious change I need to make throughout the module is changing all DML functions (get_record, set_record, etc.) to be methods of the global $DB object instead of standalone functions. So get_record becomes $DB->get_record, and so on. The other most obvious change is that the 'parameter / value' arguments that used to be passed to these functions are now replaced by an array with 'key => value' pairs instead. There are many other changes I will have to make, but I'll look at those as I come to them.

One of the tools this document points me to, is the 'check_db_syntax' helper script. This script is designed to locate all areas of a plug-in that need database code changed. The instructions are there to retrieve this script and execute. When I run this script on the code, I get a lot of information. I have provided the output below, but I have removed the items involving the backup and restore scripts, since I will deal with backup and restore separately:

Checking the /home/www/moodle.git/mod/stampcoll directory recursively
(executed from custom directory - false positive detection DISABLED!)
- /home/www/moodle.git/mod/stampcoll/mod_form.php: ... OK
- /home/www/moodle.git/mod/stampcoll/styles.php: ... OK

- /home/www/moodle.git/mod/stampcoll/lib.php:

* ERROR found!

- ERROR ( DML ) - line 22 : if ($stamps = get_records_select("stampcoll_stamps", "userid=$user->id AND stampcollid=$stampcoll->id")) {

- ERROR ( DML ) - line 88 : return insert_record("stampcoll", $stampcoll);

- ERROR ( DML ) - line 101 : return update_record('stampcoll', $stampcoll);

- ERROR ( DML ) - line 112 : if (! $stampcoll = get_record("stampcoll", "id", "$id")) {

- ERROR ( DML ) - line 118 : if (! delete_records("stampcoll_stamps", "stampcollid", "$stampcoll->id")) {

- ERROR ( DML ) - line 122 : if (! delete_records("stampcoll", "id", "$stampcoll->id")) {

- ERROR ( DML ) - line 140 : $students = get_records_sql("SELECT DISTINCT u.id, u.id

- ERROR ( OTHER ) - line 141 : FROM {$CFG->prefix}user u,

- ERROR ( OTHER ) - line 142 : {$CFG->prefix}stampcoll_stamps s

- ERROR ( DML ) - line 182 : return get_record("stampcoll", "id", $stampcollid);

- ERROR ( DML ) - line 192 : return get_records("stampcoll_stamps", "stampcollid", $stampcollid, "id");

- ERROR ( DML ) - line 202 : return get_record("stampcoll_stamps", "id", $stampid);

- /home/www/moodle.git/mod/stampcoll/editstamps.php:
* ERROR found!
- ERROR ( DML ) - line 13 : if (! $course = get_record("course", "id", $cm->course)) {
- ERROR ( DML ) - line 81 : if (! $newstamp->id = insert_record("stampcoll_stamps", $newstamp)) {
- ERROR ( DML ) - line 102 : if (! update_record("stampcoll_stamps", $updatedstamp)) {
- ERROR ( DML ) - line 120 : if (! delete_records("stampcoll_stamps", "id", $form->deletestamp)) {
- ERROR ( OTHER ) - line 241 : $sql = 'FROM '.$CFG->prefix.'user AS u '.
- ERROR ( OTHER ) - line 242 : 'LEFT JOIN '.$CFG->prefix.'stampcoll_stamps s ON u.id = s.userid AND s.stampcollid = '.$stampcoll->id.' '.
- ERROR ( DML ) - line 247 : if (($ausers = get_records_sql($select.$sql.$sort, $table->get_page_start(), $table->get_page_size())) !== false) {
- /home/www/moodle.git/mod/stampcoll/version.php: ... OK
- /home/www/moodle.git/mod/stampcoll/view.php:
* ERROR found!
- ERROR ( DML ) - line 14 : if (! $course = get_record("course", "id", $cm->course)) {
- ERROR ( OTHER ) - line 171 : $sql = 'FROM '.$CFG->prefix.'user AS u '.
- ERROR ( OTHER ) - line 172 : 'LEFT JOIN '.$CFG->prefix.'stampcoll_stamps s ON u.id = s.userid AND s.stampcollid = '.$stampcoll->id.' '.
- ERROR ( DML ) - line 181 : if (($ausers = get_records_sql($select.$sql.$sort)) !== false) {
- ERROR ( DML ) - line 184 : if (($ausers = get_records_sql($select.$sql.$sort, $table->get_page_start(), $table->get_page_size())) !== false) {
- /home/www/moodle.git/mod/stampcoll/caps.php: ... OK
- /home/www/moodle.git/mod/stampcoll/tabs.php: ... OK
- /home/www/moodle.git/mod/stampcoll/index.php:
* ERROR found!
- ERROR ( DML ) - line 8 : if (! $course = get_record('course', 'id', $id)) {
- /home/www/moodle.git/mod/stampcoll/db/log.php: ... OK
- /home/www/moodle.git/mod/stampcoll/db/install.xml: ... OK
- /home/www/moodle.git/mod/stampcoll/db/access.php: ... OK
- /home/www/moodle.git/mod/stampcoll/db/upgrade.php:
* ERROR found!
- ERROR ( DML ) - line 29 : if ($collections = get_records('stampcoll', 'publish', '0')) {
- ERROR ( DML ) - line 44 : if ($collections = get_records('stampcoll', 'publish', '2')) {
- ERROR ( DML ) - line 64 : if ($collections = get_records('stampcoll', 'teachercancollect', '1')) {

 This report gives me a pretty good list of code locations I need to fix.

 I will go through each of these and upgrade to the new method. Some examples of changes are, "editstamps.php" line 13, I change:

if (! $course = get_record("course", "id", $cm->course)) {
    error("Course is misconfigured");
}

to:

if (! $course = $DB->get_record("course", array("id" => $cm->course))) {
    error("Course is misconfigured");
}

In this case, I added the $DB-> to the call, and changed the parameters to a "key/value" array.

In "editstamps.php" line 81, I change:

if (! $newstamp->id = insert_record("stampcoll_stamps", $newstamp)) {
    error("Could not save new stamp");
}

to:

if (! $newstamp->id = $DB->insert_record("stampcoll_stamps", $newstamp)) {
    error("Could not save new stamp");
}

In this case, I only added the $DB->, since the rest of the function is the same as 1.9.

In "editstamps.php" line 240-247, I change:

$select = 'SELECT u.id, u.firstname, u.lastname, u.picture, COUNT(s.id) AS count ';
$sql = 'FROM '.$CFG->prefix.'user AS u '.
           'LEFT JOIN '.$CFG->prefix.'stampcoll_stamps s ON u.id = s.userid AND s.stampcollid = '.$stampcoll->id.' '.
           'WHERE '.$where.'u.id IN ('.implode(',', array_keys($users)).') GROUP BY u.id, u.firstname, u.lastname, u.picture ';

$table->pagesize($perpage, count($users));
   
if (($ausers = get_records_sql($select.$sql.$sort, $table->get_page_start(), $table->get_page_size())) !== false) {

to:

$select = "SELECT u.id, u.firstname, u.lastname, u.picture, COUNT(s.id) AS count ";
list($uids, $params) = $DB->get_in_or_equal(array_keys($users));
$params['stampcollid'] = $stampcoll->id;
$sql    = "FROM {user} AS u ".
          "LEFT JOIN {stampcoll_stamps} s ON u.id = s.userid AND s.stampcollid = :stampcollid ".
          "WHERE $where u.id $uids ".
          "GROUP BY u.id, u.firstname, u.lastname, u.picture ";

$table->pagesize($perpage, count($users));
   
$ausers = $DB->get_records_sql($select.$sql.$sort, $params, $table->get_page_start(), $table->get_page_size());

There are a number of DML changes here. First, I have added the standard $DB-> part. Next, I have removed the $CFG->prefix portions and instead enclosed the table names in "{}".

I also need to replace the PHP variables in the query with DML parameters. This query has two that I will deal with: $stampcoll->id and $users. The $stampcoll->id I replace with a named placeholder called ":stampcollid". The value of $stampcoll->id needs to be added to an array called $params, that I will pass to the DML function. The $users variable is a special case, because it needs to potentially be used in an "IN (...)" SQL statement.

In the old version, I created the necessary SQL using the code:

'u.id IN ('.implode(',', array_keys($users)).')

In the new version, I will use a new function created for this type of SQL statement, $DB->get_in_or_equal. This function takes an array and returns the proper SQL for "IN" if needed, or an "=" if there is only one value to worry about. My new code replaces the old code with this:

list($uids, $params) = $DB->get_in_or_equal(array_keys($users));

This statement returns the proper SQL structure in $uids, and creates the placeholder value in the $params variable. I do this before loading the 'stampcollid' placeholder value, as it creates the $params array that I need. After the array is created, I load the 'stampcollid' parameter value with:

$params['stampcollid'] = $stampcoll->id;

In the new SQL statement, you can see the use of ":stampcollid" and "$uids".


Finally, I modify the get_records_sql line significantly. I add the standard $DB->, I add $params as the second parameter and I completely remove the "if" statement from around the call. The reason I remove the "if" statement is that the DML records functions now always return an array. If no records are found, the array is empty. This means that the following "for" block will work no matter what is returned.


I also change the other two lines with database code and save "editstamps.php".

I make similar changes in "view.php", "index.php", "lib.php" and "upgrade.php" as listed in the "check_db_syntax" script output above. One thing to note, is that when I make a change in a function, like in "lib.php", I also add the code declaration "global $DB;". Without this, $DB is undefined in a function.


After making all the changes, I rerun the syntax script. The only changes noted are in the backup and restore code, which I will deal with later.


This code might run, but there are other changes I need to make still. So for now, I'll just leave it as it is.