Thursday, May 31, 2012

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

This is the sixth and final part in my Moodle 1.9 to Moodle 2 activity module migration series (apologies for the delay).


That last thing that I have to change in order for this plug-in to work completely is the backup and restore functions. Moodle has completely overhauled the backup and restore system for 2.x, and I need to rewrite these functions completely.

The main structural differences from 1.9 are pretty obvious. In 1.9, there were two files: "backuplib.php" and "restorelib.php" that were located in the modules main directory. In Moodle 2, these are gone, and in their place is a "backup" subdirectory, with one or more subdirectories supporting possible backup formats. For this purpose, I will focus on the "moodle2" subdirectory, which contains the code necessary to backup and restore in Moodle 2.

First things first, I need to tell Moodle that my module support backup. This is done using a function called stampcoll_supports in the "lib.php" file. This function allows me to more granularly define features that the module may or may not support. Mostly the defaults suffice, but in the case of backup, the default is "does not support".

I add the function and put a line in that indicates the module supports backup. This is what I add:
function stampcoll_supports($feature) {
    switch($feature) {
        case FEATURE_BACKUP_MOODLE2:    return true;

        default: return null;
    }
}
Now, the Moodle backup system knows, and will expect my module to provide backup and restore support code.

Next, I need to create the actual backup code. I create the subdirectory structure "backup/moodle2". The directory can use three files for backup: "backup_[modulename]_settingslib.php", "backup_[modulename]_stepslib.php" and "backup_[modulename]_activity_task.php". I don't actually need "backup_stampcoll_settingslib.php", as I will only be using defaults, but I'll include it for completeness and leave it with no code.

Starting with "backup_stampcoll_stepslib.php" is a good choice. This file allows me to define the data structure of my activity, so that the backup function can understand it. The module only has two tables, so it's fairly easy. (To make it even easier, "borrow" another module's code and change it - I used "/mod/choice").

Now, the "steps" file is really the meat of the whole operation. It defines the data structures and all dependencies. When I'm done, my file has the following code:
protected function define_structure() {

    // To know if we are including userinfo
    $userinfo = $this->get_setting_value('userinfo');

    // Define each element separated
    $stampcoll = new backup_nested_element('stampcoll', array('id'), array(
        'name', 'intro', 'introformat', 'image',
        'timemodified', 'displayzero', 'anonymous'));

    $stamps = new backup_nested_element('stamps');

    $stamp = new backup_nested_element('stamp', array('id'), array(
        'userid', 'giver', 'text', 'timemodified'));

    // Build the tree
    $stampcoll->add_child($stamps);
    $stamps->add_child($stamp);

    // Define sources
    $stampcoll->set_source_table('stampcoll', array('id' => backup::VAR_ACTIVITYID));

    // All the rest of elements only happen if we are including user info
    if ($userinfo) {
        $stamp->set_source_table('stampcoll_stamps', array('stampcollid' => '../../id'));
    }

    // Define id annotations
    $stamp->annotate_ids('user', 'userid');
    $stamp->annotate_ids('user', 'giver');

    // Define file annotations
    $stampcoll->annotate_files('mod_stampcoll', 'intro', null); // This file area hasn't itemid

    // Return the root element (stampcoll), wrapped into standard activity structure
    return $this->prepare_activity_structure($stampcoll);
}

The backup_nested_element objects define the XML data structure of my backup. Essentially it contains all of the necessary data. The id fields of data that will be created new are not included, since they will be discarded anyway. When there are multiple records per activity, like there is with "stamps", we create two XML levels: one for the type (stamps) and then one for each actual record (stamp). The add_child function creates the actual XML tree structure. I use set_source_table to define where the data comes from and the main key used to access it. Note that I only bother defining the "stamps" table if the backup is storing user information.

Lastly, the annotate_ids function tells the backup system to remember specific data identifiers that may change when restored. The first argument is the type of Moodle variable, and the second is the identifier I specified in my definitions (see http://docs.moodle.org/dev/Backup_2.0_for_developers#annotate_is_important for all variable types). In this case, both are user identifiers: one for the user receiving the stamp and one for the user giving the stamp.

Then, I create "backup_stampcoll_activity_task.php". This is the main file of the backup process, uses the other two files and calls all the specific steps. This is a pretty standard file, and all I really need to do is change all of the "choice" strings (from the module I borrowed from) to "stampcoll" strings. I'm not including the code here, but you can get it from the git repo.

That should give me all I need to successfully backup. When I run it in a test course, I now see my module as an option to backup, and when I execute the backup, it is successful. Unfortunately I can't verify it without the restore code.

Again, borrowing from the "choice" module, I create my "restore_stampcoll_stepslib.php" file. This is pretty much  a search-and-replace of the "choice" string for the "stampcoll" string again. But I do need to spend a little bit more effort in the process_stampcoll and process_stampcoll_stamp functions:
protected function process_stampcoll($data) {
    global $DB;

    $data = (object)$data;
    $oldid = $data->id;
    $data->course = $this->get_courseid();

    $data->timemodified = $this->apply_date_offset($data->timemodified);

    // insert the stampcoll record
    $newitemid = $DB->insert_record('stampcoll', $data);
    // immediately after inserting "activity" record, call this
    $this->apply_activity_instance($newitemid);
}

protected function process_stampcoll_stamp($data) {
    global $DB;

    $data = (object)$data;
    $oldid = $data->id;

    $data->stampcollid = $this->get_new_parentid('stampcoll');
    $data->userid = $this->get_mappingid('user', $data->userid);
    $data->giver = $this->get_mappingid('user', $data->giver);
    $data->timemodified = $this->apply_date_offset($data->timemodified);

    $newitemid = $DB->insert_record('stampcoll_stamps', $data);
    // No need to save this mapping as far as nothing depend on it
    // (child paths, file areas nor links decoder)
}
Essentially, I just add the apply_date_offset function to the only time fields I have. And I use the get_mappingid function to get the correct user id's for the two user fields in the "stamp" table. That's pretty much it.

The "restore_stampcoll_activity_task.php" file is similarly a search-and-replace task, with the exception of the define_restore_log_rules function. I need to modify the rules variable to only return logs that my module creates:
static public function define_restore_log_rules() {
    $rules = array();

    $rules[] = new restore_log_rule('stampcoll', 'add', 'view.php?id={course_module}', '{stampcoll}');
    $rules[] = new restore_log_rule('stampcoll', 'update', 'view.php?id={course_module}', '{stampcoll}');
    $rules[] = new restore_log_rule('stampcoll', 'view', 'view.php?id={course_module}', '{stampcoll}');
    $rules[] = new restore_log_rule('stampcoll', 'update stamp', 'editstamps.php?id={course_module}', '{stampcoll}');
    $rules[] = new restore_log_rule('stampcoll', 'delete stamp', 'editstamps.php?id={course_module}', '{stampcoll}');

    return $rules;
}
Now that I have my restore code, I can apply it to the backup I made and verify the function. I run the restore and achieve success!

I kind of rushed through this, and didn't touch on everything, so I encourage you to really read the backup documentation. It is very thorough.

This concludes my postings on module migration from Moodle 1.9 to Moodle 2.1. Please feel free to post comments and make suggestions.

Thursday, January 12, 2012

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

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

Before I carry on too much farther, there are a couple of other fixes I need to do. One, there are two more occurrences of get_record that did not get caught with the original sweep in "lib.php". And two, there are multiple occurrences of the error function throughout. I need to change error() to print_error(). You can see these changes in the repo.

Next, I need to fix up the "index.php" script using the same navigation and display changes described in my last post. One extra bit I need to add to this script is the call:
$PAGE->set_pagelayout('incourse');
This sets the layout of the page to a default display reserved for module index displays.

Similar to before, I change:
$navigation = build_navigation($strstamps);
print_header_simple($strstamps, '', $navigation, '', '', true, '', navmenu($course));
to:
$PAGE->navbar->add($strstamps);
$PAGE->set_heading(format_string($course->fullname));
$PAGE->set_title(get_string('modulename', 'stampcoll').' '.get_string('activities'));
echo $OUTPUT->header();
In the index code, the old print_table($table) is used. This has been changed to the new html_writer::table($table) construct. Additionally, where before the $table variable could be any standard object, now it must be an instance of the html_table class. So, I add the line:
$table = new html_table();
to a place before it is used, and I change:
print_table($table);
to:
echo html_writer::table($table);
I make a few more of the standard output changes, and all seems to be well.

Something I missed earlier, is that I need to rename the "styles.php" file to "styles.css". This is an easy change, but there is still the possibility that the CSS is not rendering as it did in 1.9, since it may depend on styles that have changed. I'm not going to go to that level here, but keep in mind for your plug-in, that you may need to make changes to your stylesheet to have it display the way it did in Moodle 1.9.

Next up is a problem appearing on the "Edit stamps" page. I can see a broken image where I added a stamp, and the warning message:
Notice: Undefined property: stdClass::$pixpath in /home/www/moodle.git/mod/stampcoll/editstamps.php on line 285 Notice: Undefined property: stdClass::$pixpath in /home/www/moodle.git/mod/stampcoll/editstamps.php on line 289
Here, the problem is with the code $CFG->pixpath.'/t/preview.gif' and $CFG->pixpath.'/t/delete.gif. In Moodle 2, the $CFG->pixpath and $CFG->modpixpath were removed, and replaced with an $OUTPUT->pix_url() function. In this case, I replace:
$CFG->pixpath.'/t/preview.gif'
$CFG->pixpath.'/t/delete.gif
with:
$OUTPUT->pix_url('t/preview')
$OUTPUT->pix_url('t/delete')
Note that both the opening '/' and the extension of the image have been removed in the new code to conform with the function's requirements.

Next, I test the delete function using the newly visible delete icon. After the confirmation page, I see a warning display before the page redirects. The warning says:
You should really redirect before you start page output
  • line 566 of /lib/outputrenderers.php: call to debugging()
  • line 2503 of /lib/weblib.php: call to core_renderer->redirect_message()
  • line 128 of /mod/stampcoll/editstamps.php: call to redirect()
Here, the problem is with the code structure. Because the script has already started page output, by outputting the header, Moodle is telling me I should not redirect to another page. This should be fixed by restructuring the code so that redirects are called before any page output. I'm not going to do that for now, but keep in mind that for good code, this should be done.

One other issue I can see. In the 1.9 version, hovering my mouse over a stamp image, displayed information about that stamp, including any messages entered with it. This is not happening here. Something is wrong with the javascript. Looking deeper, I see that the problem here is that the code is using "Overlib", which was a Javascript library included in older Moodle versions. For Moodle 2, this was removed with the intention of using YUI libraries instead (see MDL-21533). Fixing this will require some research. I'll work on that in the next post.

Monday, December 19, 2011

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


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

When I left off, I had completed the necessary changes the allowed me to successfully install, add and edit instances of the stamp collection module. Now it is time to start making it function.

In Moodle 2, the entire navigation and page display functions have changed. I need to change all of the code that does this. This document describes the changes, but it is outdated so there will be inconsistencies.

To check where my code is, I click on an instance of a module that I've added to a course. Right away I see several warnings and errors I'll need to fix. I'll start with my "view.php" script.

One of the first things I need is to set the URL of my display page into the global $PAGE variable. This identifies the page's URL and any necessary parameters to the internal systems, such as navigation, so that links back to the page get rendered properly. In my module, I add the lines:
$params = array();
$params['id'] = $id;
if ($view) {
    $params['view'] = $view;
}
if ($page) {
    $params['page'] = $page;
}
$PAGE->set_url('/mod/stampcoll/view.php', $params);
before the require_course_login function.

This code, combined with the require_course_login call, pretty much sets up everything I need for the page and navigation display, and gets rid of the first group of warnings..

Next on my list is changing all of the output functions to use the global $OUTPUT variable. Essentially, everything that used to write output to the screen should now come from a function of the object $OUTPUT. So print_box would become $OUTPUT->box, and so on.

I'll start with the print_header functions. I find two, both very similar to this:
$navigation = build_navigation('', $cm);
print_header_simple(format_string($stampcoll->name), "",
                 $navigation, "", "", true, '', navmenu($course, $cm));
In Moodle 2, these functions are replaced with the simpler $OUTPUT->header(), but I need to first specify all of the arguments that were passed to the old function. Also, the navigation has already been handled by the page setup functions and the course login functions. So I completely remove these lines of code. Then, before the code block where the first print_header_simple was, I add the lines:
$PAGE->set_title(format_string($stampcoll->name));
$PAGE->set_heading(format_string($course->fullname));
echo $OUTPUT->header();
These three lines replace the navigation and header printing functions for the script, and when I access the module instance, displays the page heading again.

Next up, I look for the "print_" functions. When I look at a page of my module now, I can see warnings for these functions. The ones I replace are the "print_heading" and the "print_box..." functions. I believe I can pretty much simply replace "print_" with "echo $OUTPUT->". After doing that, and revisiting the screen, the warnings are gone.

Next, I click on the "Edit stamps" tab link. I get essentially the same warning and errors as on the view page. Looks like I need to make the same changes in "editstamps.php" as I did in "view.php".

Now, I'll do some testing as a student. I login as a student and access the activity. Since I have no stamps, I am presented with an empty display and the message "Number of your stamps: $a". I'm pretty sure that "$a" should not be there. This will require a change to the language files, "lang/*/stampcoll.php".

Essentially, in Moodle 1.9 and below, language strings could specify variables that would be loaded into the string at runtime with the string "$a". In Moodle 2, the "$a" needs to be enclosed in braces ("{}"). I open the "lang/en/stampcoll.php" file (the language I'm using), and replace all "$a" with "{$a}" and save. Before I check if it worked, I remember to go to the "Site administration / Development" menu and click "Purge all caches". Going back to my stamp collection instance as a student, and clicking it, shows that the message now says: "Number of your stamps: 0". That seems more correct.

Okay, now that I have students in my course, I should go back and check how the adding stamps functions works. Accessing it again as the "admin" user, I get a problem right away:
ERROR: Mixed types of sql query parameters!!

More information about this error
Stack trace:

    line 704 of /lib/dml/moodle_database.php: dml_exception thrown
    line 804 of /lib/dml/mysqli_native_moodle_database.php: call to moodle_database->fix_sql_params()
    line 191 of /mod/stampcoll/view.php: call to mysqli_native_moodle_database->get_records_sql()
This is a problem that I had not encountered yet. Doing some debugging, I discover that the code:
if ($where = $table->get_sql_where()) {
    $where .= ' AND ';
}
no longer works as it did before. In fact, it now works in a similar fashion to the $DB->get_in_or_equal() function. I need to do some heavier refactoring in this code.


First, I need to change the code above to:
list($where, $w_params) = $table->get_sql_where();
if ($where) {
    $where .= ' AND ';
}
I also discover another problem. I am using a named parameter in my SQL, ":stampcollid". But the get_sql_where and the get_in_or_equal functions use positional parameters for the SQL. This is the "mixed types of sql query parameters" part of the error message. What this means is that the parameter array I pass to my SQL query must provide the replacement arguments in the same order they appear in the SQL query. My SQL query must use "?" instead of ":variable" replacement strings. So refactoring, this chunk of code now looks like:
list($where, $w_params) = $table->get_sql_where();
if ($where) {
    $where .= ' AND ';
}

if ($sort = $table->get_sql_sort()) {
    $sort = ' ORDER BY '.$sort;
}

$select = 'SELECT u.id, u.firstname, u.lastname, u.picture, COUNT(s.id) AS count ';
list($uids, $u_params) = $DB->get_in_or_equal(array_keys($users));

$params = array();
$params[] = $stampcoll->id;
$params = array_merge($params, $w_params);
$params = array_merge($params, $u_params);

$sql = "FROM {user} u ".
       "LEFT JOIN {stampcoll_stamps} s ON u.id = s.userid AND s.stampcollid = ? ".
       "WHERE $where (u.id $uids) ".
       "GROUP BY u.id, u.firstname, u.lastname, u.picture ";

if (!$stampcoll->displayzero) {
    $sql .= 'HAVING COUNT(s.id) > 0 ';
}
Note that I have replaced ":stampcollid" with "?", and moved around the order that the $params array gets loaded so that the $where and $uids positions appear after the first "?".

One more issue crops up after this. The $OUTPUT->user_picture is not passing the correct parameters. This is because I simply replaced the old print function with $OUTPUT-> one. The old function took a user id. The new one takes a user object. The other parameters are different too, so I change it to:
$picture = $OUTPUT->user_picture($auser, array('courseid' => $course->id));
This gets me most of the way there. But I'm still getting an error about a missing "imagealt" field. A little research, and I determine that the user records that I am passing to the user_picture function are missing a field that is required by that function. Looking at other examples, I notice that the are all calling a new function, user_picture::fields, to get the fields they request from their database call for user records. It appears that this function returns a list of user record field names suitable for providing in an SQL query, that are the minimum required for other user functions. This seems to be the new preferred way of specifying user record fields, rather than specifying them directly in the SQL. So to fix my code, I replace:

$select = 'SELECT u.id, u.firstname, u.lastname, u.picture, COUNT(s.id) AS count ';
with:
$userfields = user_picture::fields('u');
$select = "SELECT {$userfields}, COUNT(s.id) AS count ";
This change makes the "imagealt" error go away, and displays the user pictures in the list.

Next, I am getting errors related to using helpbutton(). Essentially, the use of this function has been replaced by $OUTPUT->help_icon. This is similar to what was changed in the forms. Basically, the old way of providing a full help file that appeared in a pop-up, has been replaced by a help string that shows in a mouse-over tip. So, I have to replace the function call, and move the help message from the help file into the language file as a language string.

The first change is changing:
helpbutton('pagesize', get_string('studentsperpage','stampcoll'), 'stampcoll');
to:
echo $OUTPUT->help_icon('studentsperpage', 'stampcoll');
In the old version, a help icon labeled with the language string "studentsperpage" would be rendered, that when clicked, would load a pop-up with the file "pagesize.html" in it. In the new version, an icon will be displayed that when clicked, will display the string from "studentsperpage_help" in the pop-up tip window. So, I also need to bring the text from the old help file into a new language string. In this case, there was no help file in the old version, so I'll just create a new string:
$string['studentsperpage_help'] = 'Set the number of students you want to display per page';
I have to make this same series of changes in "editstamps.php" and one more for a "showupdateforms" help string.

At this point, I have migrated all of the basic functions in the main files to Moodle 2. I still have work to do though. I need to check the rest of the scripts for all of the same changes I have performed so far, and there are still more migration tasks. For now, I am calling it a day.

Friday, December 9, 2011

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

When I left off, I had completed bulk changing the DML function code for the module. Now I'll move on to other functional and output changes.

One of the key changes that affects this module is the standard use of the "intro" field. This is the field that typically provided the textual description for an instance of the module. In Moodle 1.9, some modules called it "intro". Others called it "description". This module called it "text". The point is, it was not standard and was not necessarily the same in all modules. In Moodle 2, it is standard, and needs to be called "intro".

To do this, I will first need to change the database code that sets it up, and add an upgrade script for existing installations.

For the "install.xml" file, there are three changes of "text" to "intro" I need to do: the actual entry for the "text" field's "NAME" parameter and the two entries on either side of it's "NEXT" and "PREVIOUS" parameters.

Along with "intro", Moodle 2 now requires that the module have an "introformat" field in the database table. This needs to be added to the "install.xml" file as well as the upgrade script. For this module, I need to change the old "format" field to "introformat". I'm not including the XML code here, but you can see it if you check it out of Git.

Next, I need to add an upgrade script to handle changing the database field name for sites that are upgrading from an older version. I add the following lines to the bottom of "db/upgrade.php" in my module:
if ($oldversion < 2010080300) {

/// Rename field text on table stampcoll to intro
    $table = new xmldb_table('stampcoll');
    $field = new xmldb_field('text', XMLDB_TYPE_TEXT, 'small', null, XMLDB_NOTNULL, null, null, 'name');

/// Launch rename field description
    $dbman->rename_field($table, $field, 'intro');


/// Define field introformat to be added to data
    $field = new xmldb_field('format', XMLDB_TYPE_INTEGER, '4', XMLDB_UNSIGNED, XMLDB_NOTNULL, null, '0', 'intro');

/// Launch rename field introformat
    $dbman->rename_field($table, $field, 'introformat');

    // conditionally migrate to html format in intro
    if ($CFG->texteditors !== 'textarea') {
        $rs = $DB->get_recordset('stampcoll', array('introformat'=>FORMAT_MOODLE), '', 'id,intro,introformat');
        foreach ($rs as $d) {
            $d->intro       = text_to_html($d->intro, false, false, true);
            $d->introformat = FORMAT_HTML;
            $DB->update_record('stampcoll', $d);
            upgrade_set_timeout();
        }
        $rs->close();
    }
/// stampcoll savepoint reached
    upgrade_mod_savepoint(true, 2010080300, 'stampcoll');
}
This block of code will rename the "text" field of the "stampcoll" table to "intro" and the "format" field to "introformat". It will also reset the format values where necessary for existing instances. Just to be sure, I test this by manually changing the version of the "stampcoll" entry to a lesser number, and then hitting the "Notifications" screen of my site. As expected, the "stampcoll" table "text" and the "format" fields are renamed.

Lastly I test the XML changes by removing the "stampcoll" module, and then allowing it to be reinstalled from the "Notifications" screen. This works too.

After making those changes, I need to then go and change every use of the "text" variable to "intro". This will also be important in my restore code that I will deal with later on, since older restore sets will be using "text" and not "intro". Searching, and being careful to find only the correct places, I replace the few "text" instances that need replacing (in "lib.php" and "view.php"). There will be changes in backup and restore as well, but I'm doing that later.

I don't need to worry about "introformat", since that is a variable used internally by the API, and not directly by my code. 

Next, I'm going to change the module edit form in "mod_form.php". Most of this remains the same as before, but there are some crucial differences.

The first and easiest is changing the "intro" component. In Moodle 1.9. this involved several lines. In Moodle 2, thanks to the standardization of the "intro" field (which I just fixed), it is now only one line. So I change the lines:
/// text (description)
    $mform->addElement('htmleditor', 'text', get_string('description'));
    $mform->setType('text', PARAM_RAW);
    //$mform->addRule('text', get_string('required'), 'required', null, 'client');
    $mform->setHelpButton('text', array('writing', 'richtext'), false, 'editorhelpbutton');
/// introformat
    $mform->addElement('format', 'introformat', get_string('format'));
to:
/// intro
    $this->add_intro_editor(true, get_string('description'));
The next change I have to make involves the help system. Now, in this particular module, there are no specific help buttons added. But if there were, I would need to change each occurrence of:
$mform->setHelpButton('elementname', array('helpfilename', get_string('helpstring', 'component'), 'component'));
to:
$mform->addHelpButton('elementname', 'helpstring', 'component');
The function name changes, and the arguments are simplified. Additionally, all old "help" files in your language directories need to removed, and their contents added to the main language file, assigned to the string "helpstring_help".

One thing to consider at this point, is that the help buttons that are added now are mouseover text areas. The amount of text displayed there should be suitably limited. If you need more help content, you can create HTML files elsewhere, and link to them in the help string. Moodle docs provides a space to create this content if you want to use it.

One thing I will change for this module is the help text for the main edit screen, up next to the title. In Moodle 1.9, this content was in the "lang/en_utf8/help/stampcoll/mods.html" file. In Moodle 2, this is in the language string "modulename_help". I add a shortened version of this text to that string in my "lang/en/stampcoll.php" file.

These changes should be all I need to be able to add and edit instances of the module. I test it to make sure, and indeed I can successfully add and edit instances. I'm far from done though. Clicking on an added module will display a lot of errors still. In the next part, I'll work on fixing those.

Make sure you get the latest code from Git, to follow along.

Monday, November 28, 2011

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.

Tuesday, November 22, 2011

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

Continuing on with my Moodle 1.9 to Moodle 2 code migration series, I next want to tackle an activity module. I took a look around the "contrib" areas for a suitable module that needed attention. Alas, I could not find one that was simple enough to do in this blog and hadn't already been migrated. But, I have chosen to do David Mudrack's Stamp Collection module. I have spoken to David, and he is in the process of migrating this, so my version won't be official. Still, it is a good module to migrate as it is straightforward and will allow me to demonstrate the key areas that need to be changed.

I have created a repository for the code, forked from David's. The MOODLE_19_STABLE branch contains the code that will be modified. The MOODLE_21_STABLE branch contains my changes as I migrate the module. Start with the MOODLE_19_STABLE branch if you want to follow along.

When I did the block migration I attempted to install the block with the old code first. With an activity module, I could do this, but I already know with the amount of changes necessary, it will fail. And because of the way Moodle works, as long as that old code is there, I will not be able to get past the upgrade screen as an administrator. So instead, I am going to try and create a methodical upgrade process here.

I'm going to start by referring to the "Migrating contrib code to 2.0" document. This document is for exactly what I'm doing.

Now, here I will diverge for the first time (yes, already). One of the key things missing here is the new language string requirements.

In Moodle 1.9, all plug-ins had language subdirectories with the suffix "_utf8" in their name. In Moodle 2, this suffix is removed. So one of the first things I need to do is rename all of my language subdirectories in this fashion. For example, I rename my '/mod/stampcoll/lang/en_utf8' directory to '/mod/stampcoll/lang/en'. The other key change is to add two new strings to my language file: 'pluginname' and 'pluginadministration'. I add the following to my '/mod/stampcoll/lang/en/stampcoll.php' file:
$string['pluginadministration'] = 'Stamp collections administration';
$string['pluginname'] = 'Stamp collections';
There are some other key changes that I will have to make to my language files, but I will put that off until later.

Back to the migration document, the first thing it notes is that I need a 'requires' statement in my version file. Without this, my module will not pass the installation stage.

Looking at my 'version.php' file, I specifically change:
$module->requires = 2007101508;  // Requires this Moodle version - 1.9 Beta 4
to:
$module->requires = 2010080300;  // Requires this Moodle version
I also upgrade the version of the module to reflect that it is new. For now, I have selected the same version number as the "requires".

Next, the document refers to the database layer. There are many changes in Moodle 2 that impact database functions. I'm going to focus on the XMLDB/DDL changes first, since they impact the actual installation and upgrading of the module.

I have opened up the "db/install.xml" file in a text editor. At the bottom of that file, there is XML code framed in <STATEMENTS> tags. In 1.9, this section was used to execute any specific data statements that needed to be executed after the data tables were set up. Primarily, this section contained records to enter into the Moodle "log_display" table. In Moodle 2, log display entries go into a separate file, "db/log.php". If there are any other data statements required for the module, they now belong in "db/install.php".

In my "db/install.xml" file, I have:
<STATEMENTS>
  <STATEMENT NAME="insert log_display" TYPE="insert" TABLE="log_display" COMMENT="Initial insert of records on table log_display">
    <SENTENCES>
      <SENTENCE TEXT="(module, action, mtable, field) VALUES ('stampcoll', 'view', 'stampcoll', 'name')" />
      <SENTENCE TEXT="(module, action, mtable, field) VALUES ('stampcoll', 'update', 'stampcoll', 'name')" />
      <SENTENCE TEXT="(module, action, mtable, field) VALUES ('stampcoll', 'add', 'stampcoll', 'name')" />
      <SENTENCE TEXT="(module, action, mtable, field) VALUES ('stampcoll', 'update stamp', 'user', 'concat(firstname, \' \', lastname)')" />
      <SENTENCE TEXT="(module, action, mtable, field) VALUES ('stampcoll', 'delete stamp', 'user', 'concat(firstname, \' \', lastname)')" />
    </SENTENCES>
  </STATEMENT>
</STATEMENTS>
I remove all of these statements from the file, and create a new "db/log.php" file containing:
$logs = array(
    array('module'=>'stampcoll', 'action'=>'view', 'mtable'=>'stampcoll', 'field'=>'name'),
    array('module'=>'stampcoll', 'action'=>'update', 'mtable'=>'stampcoll', 'field'=>'name'),
    array('module'=>'stampcoll', 'action'=>'add', 'mtable'=>'stampcoll', 'field'=>'name'),
    array('module'=>'stampcoll', 'action'=>'update stamp', 'mtable'=>'user', 'field'=>'concat(firstname, \' \', lastname)'),
    array('module'=>'stampcoll', 'action'=>'delete stamp', 'mtable'=>'user', 'field'=>'concat(firstname, \' \', lastname)'),
    );
There are no other statements I need to worry about, so I don't need a "db/install.php" file.

Next, I need to remove all "ENUM" statements from the XML. "ENUM" is no longer a valid attribute in Moodle, so the presence of these statements will be flagged as a warning. I could manually remove them from the "install.xml" file right now, but I'm going to leave them. Moodle provides me a simpler method using the XMLDB editor that I will use later.

Finally, I need to make some changes to the existing "db/upgrade.php" script. As documented, there are many changes to be made, most of them straightforward. The summary of what I chang is:
  • Change the $db global declaration to $DB.
  • Add the line "$dbman = $DB->get_manager();".
  • Change all of the "XMLDB" functions to "xmldb_" functions.
  • Change all of the "setAttributes" functions to "set_attributes", removing the sixth and seventh parameters in the few places they are provided.
  • Add $dbman-> to the DDL functions.
  • Remove the use of $result from the DDL functions.
  • Remove all other uses of $result and set the return value to "true".
  • Add upgrade_mod_savepoint calls at the end of each upgrade block.
When completed, I have a "mostly" working upgrade script. There are some changes I will need to make at the start of this script, where DML function calls are used. This will not impact a fresh install though, so I will do that when I do the main DML changes later.

There seems to be one other change I need to make, that I cannot find documented anywhere. In "db/access.php", the variable name $mod_stampcoll_capabilities needs to be changed to $capabilities.
    These should be all the changes I need to have the module install correctly in Moodle 2. Time to give it a shot! Visiting the site notifications page, offers me the option to install the module. I hit the "Upgrade" button, and it installs without errors or warnings.

    Now, there is one more thing I'm going to fix before leaving the installation code. Remember that I noted that "ENUM" statements needed to be removed from the "install.xml" file? Now we're going to use the Moodle tool to do this.

    I select the "XMLDB editor" option from the "Site administration / Development" menu. From there, I look for the "mod/stampcoll/db" entry and click the "Load" link. If I click the "XML" link now, I will see the "ENUM" statements still present. Instead, I click the "Edit" link. This brings me to a screen like the image below. Clicking either of the "XML" links shows me XML without the "ENUM" statements.


    Now, at worst case, you can cut the XML from this display and paste it back into your XML file. At best, you can set your directory permissions so that the application can write to it, and save the changes right from there. Both will work fine, and both will result in XML with the "ENUM" statements gone, which is what Moodle 2 wants.

    Just to make sure my XML is okay, I un-installed my Stamp Collection module, and then re-installed it. Everything worked fine.

    That will be all I do for this part. Next, we will look at the other changes to make the module functional.

    Wednesday, November 16, 2011

    Converting M1.9 Plug-ins to M2 - Block Part 7 - Cleaning Up

    This is part eight of my series concerning porting Moodle 1.9 code to Moodle 2.

    In part four, I ran into an issue where the Ajax script was spitting out warning messages, specifically:
    Coding problem: $PAGE->context was not set. You may have forgotten to call require_login() or $PAGE->set_context(). The page may not display correctly as a result
    I resolved this by adding a require_login and set_context function calls.

    Tim Hunt suggested in the comments that this might be unnecessary if resolved by setting the constant AJAX_SCRIPT to "true". Tim also provided me an example in the Moodle repo to look at.

    I have tried this. I added the line define('AJAX_SCRIPT', true) to the top of my script, and removed the $PAGE->set_context($block->context) line. Unfortunately this caused the warning to come back, leaving me to think that I still need to have the set_context function to complete the rendering function. I left the require_login there as it is more appropriate and secure for that function to be there.

    That said, there are probably good reasons to leave that AJAX_SCRIPT constant in there, so I have looked to see what it does. From the code I have looked at, and some of the issues in the tracker, I believe that setting this constant should prevent some potential problems by setting the page renderer appropriately. This should prevent unnecessary output from normal HTML page output, and package exceptions differently. To be safe, I will leave it there even though it did not solve the problem I hoped it would.

    In part six, I added a global configuration to the block to so that the defaults for instance configurations could be specified. In my original design, I modified the specialization function so that instead of using hard-coded defaults for the instance settings, it checked for a global default setting first. This meant that for each instance setting, one database call was made. An example of what I have is this:
    if(empty($this->config->search_string)) {
        if (($defaultsearch = get_config('block_twitter_search', 'defaultsearch')) === false){
            $defaultsearch = '#moodle';
        }
        $this->config->search_string = $defaultsearch;
    }
    This code is repeated three times; one for each configuration variable. Each one of the calls to get_config is one database access.

    In the comments for this post, Tim Hunt pointed out that get_config can be called without the second argument. When the function is used without that argument, it returns an object with all of the global configuration variables for the plug-in as variables (properties) of that object. Thus, I can get all of the configuration variables with just one database access and reduce the load on the system.

    So, I will create a new function called get_global_config as follows:
    function get_global_config() {
        if (!isset($this->globalconfig)) {
            if (($this->globalconfig = get_config('block_twitter_search')) == null) {
                $this->globalconfig = new stdClass;
                $this->globalconfig->defaultsearch = '#moodle';
                $this->globalconfig->defaultnumtweets = 10;
                $this->globalconfig->defaultpolltime = 30000;
            }
        }
        return $this->globalconfig;
    }
    This function creates a new variable within my block containing the global settings. If there are no global settings, it uses hard-coded defaults. It also returns those settings in case a calling function wishes to use what it set that way. I will use this function to set the instance configuration variables if they have not been specified for the instance in my specialization function:
    function specialization() {
        $this->get_global_config();

        if(!isset($this->config->search_string)) {
            $this->config->search_string = $this->globalconfig->defaultsearch;
        }
        if(!isset($this->config->no_tweets)){
            $this->config->no_tweets = $this->globalconfig->defaultnumtweets;
        }
        if(!isset($this->config->polltime)){
            $this->config->polltime = $this->globalconfig->defaultpolltime;
        }
    }
    By doing things this way, I have reduced the database accesses required for each block instance by two thirds. You may also notice that I changed the use of "empty" statements to "!isset" statements. The reason for this is that empty is true for many conditions including blank and zero values. isset is only true if the variable actually exists. The polltime setting is supposed to be allowed to be zero, so using empty would not have allowed that.

    When I did some further testing, including uninstalling the block completely from my test site, I discovered that the global configuration values were not removed from the 'config_plugins' table. This means that there are remnants of the block left in the database after I have deleted the block. Ideally, Moodle should handle this with the uninstall process automatically, but after looking through the code in detail, I have determined that it does not. I have logged an issue with the Moodle team in the Moodle Tracker describing this problem. You can see it under MDL-30327.

    Until that bug is fixed, if I can solve the problem, I should. So, I will add a new method to my block called before_delete. This function is part of the blocks API, and is called by the system when the block is being uninstalled but before the uninstall operation completes. I will use this function to delete the global configuration data specifically from the block code when it is being deleted. My new function looks like this:
    function before_delete() {
        unset_all_config_for_plugin('block_twitter_search');
    }
    With the addition of this function, my block now completely cleans up after itself when it is removed.

    In part 6, when I was creating the configuration code, I noted that the global settings link for the block was visible in the menu tree, but would not be visible in the main block management page unless there was a has_config method defined that returned "true". In the comments to this post, Tim Hunt suggested that this may in fact be a bug, rather than intended functionality. I have therefore added MDL-30332 to the tracker for the Moodle team to look over.


    There have been requests to make the code I am playing with available via Git. To that end, I have set up a public repository called "moodleaddons" at Github. There are two branches available there: MOODLE_19_STABLE which contains the code I started with, and MOODLE_21_STABLE which contains the code as it is now. Feel free to grab that code and play with it anyway you'd like. If you are not a Git user, there is a link to grab a zip of the code as well.

    Lastly, Kevin Hughes, the original code author has now made his Moodle 2 version available at the Moodle downloads site. He has commented that he will look at the changes done in this series for possible inclusion into his release.

    I am going to put this code away for now, and will look for a good Moodle module to focus on next. Please let me know if you have any suggestions for code that I can look at.