Monday, October 29, 2012

Adding Moodle 1.9 Block Restore to Moodle 2.3 - Part 4

At this point, I think I need to step back and try to understand how the conversion process is executed so that I can make some good decisions on how to structure the programming solution. So, I'm going to make one more run through the code to see what is happening. I'll warn you now, this post will be difficult to read, and attempts to document my search-and-reverse-engineer the code for 1.9 conversion.

I feel like if I could just figure out how the moodle1_question_bank_handler class is used I could be most of the way there... It seems like most of the work is done in this class's process_question function, but I can find no place in Moodle where this function is actually called, meaning it must be a constructed function in the Moodle code.

As an aside, what I mean by a "constructed function" is that the function name that is called is actually determined by logic at execution time and stored in variable names. This is a strong programming feature in PHP that is used in Moodle. It allows for function calls to be dynamically constructed from variable names. While a powerful programming tool, it can make reverse engineering very difficult.

Given this, it is likely that the act of registering the question bank handler in moodle1_converter::register_handler()causes some chain of events that allows process_question to be called somewhere!

Inside moodle1_converter::register_handler() is where it begins to get obscure, but it looks like the handler information gets stored in the moodle1_converter::pathelements variable as well as within the moodle1_converter::xmlprocessor object. Searching for use of the pathelements variable, I find it processed in the moodle1_converter::process_chunk(), path_start_reached() and path_end_reached() functions. All of these functions have specific code sections set up to handle blocks in a similar way to modules.

Likely, the function call is created in process_chunk by the use of the get_processing_object() and the get_processing_method() functions. The only place I can find a likely definition of the get_processing_method() function is in the convert_path class. And convert_path objects are loaded and returned by the get_paths function in every handler including the moodle1_question_bank_handler.

And there it is! The handlers' get_paths function creates new instances of convert_path (located in "/backup/converter/moodle1/lib.php"), loading a name and a path argument. The name argument of this function is used to construct the "process_[name]" function. This in fact happens in the convert_path constructor with the line:
$this->set_processing_method('process_' . $name);
Also noted, is that this function definition is a default definition, meaning any implementing class can change this. This may come in handy when I am adding all blocks to be converted.

Looking specifically at the question handler again, it instantiates a convert_path object, passing the name "question" to it, which is used to define the processing method as "process_question". This function is then returned in the get_processing_method function call made in the moodle1_converter::process_chunk function call before dispatching the function itself. Now, I just need to determine what calls process_chunk.

Searching further, process_chunk() is called by moodle1_parser_processor::dispatch_chunk() which appears to be only called by the parent classes of this object.

Stopping to think, at this point, I probably don't have to worry about where the calls are made from. I can derive that, the act of loading convert_path objects with appropriate function call information will at some point, trigger their execution. So, I just need to jump back to the block handler code and redesign it so that it loads paths for all blocks. The act of loading functions for these will allow me to control the conversions! Eureka!

I think I now know enough to design the programming structure for the block conversion.

Friday, October 12, 2012

Adding Moodle 1.9 Block Restore to Moodle 2.3 - Part 3

This is part three of my series on writing the Moodle 1.9 to 2.3 block restore conversion code for Moodle core.

Up to this point, all I've really done is figured out some of the basic operation of the Moodle 1.9 restore converter, and verified my findings using some new test code. This exploration has shown me that the heavy lifting I need to do is convert the data format from a 1.9 backup file to the data format used by Moodle 2. Essentially, I need to look at what data was stored in a 1.9 backup, compare it to what is in a 2.x format and plan for that transformation.

Looking at the backup XML first, in 1.9 I see:
<BLOCK>
  <ID>319066</ID>
  <NAME>participants</NAME>
  <PAGEID>26328</PAGEID>
  <PAGETYPE>course-view</PAGETYPE>
  <POSITION>l</POSITION>
  <WEIGHT>0</WEIGHT>
  <VISIBLE>1</VISIBLE>
  <CONFIGDATA>Tjs=</CONFIGDATA>
  <ROLES_OVERRIDES>
  </ROLES_OVERRIDES>
  <ROLES_ASSIGNMENTS>
  </ROLES_ASSIGNMENTS>
</BLOCK>
In 2.3, I see:
<block id="10" contextid="278" version="2012061700">
  <blockname>participants</blockname>
  <parentcontextid>16</parentcontextid>
  <showinsubcontexts>0</showinsubcontexts>
  <pagetypepattern>course-view-*</pagetypepattern>
  <subpagepattern>$@NULL@$</subpagepattern>
  <defaultregion>side-pre</defaultregion>
  <defaultweight>2</defaultweight>
  <configdata></configdata>
  <block_positions>
    <block_position id="1">
      <contextid>16</contextid>
      <pagetype>course-view-topics</pagetype>
      <subpage></subpage>
      <visible>1</visible>
      <region>side-post</region>
      <weight>1</weight>
    </block_position>
  </block_positions>
</block>
I need to analyze this data and structures and determine how I migrate the old to the new.

The easy ones (I believe) should be able to transfer "as is":
  • ID => id
  • NAME => blockname
  • CONFIGDATA => configdata
The others will require some transformation processing. I'll look at each one.

contextid - This would be the context instance id value of the block on the system it was backed up from. Since this is not kept in 1.9 backups, I'll need to create one. This is a common problem with all 1.9 converters, so there should be examples I can borrow.

version - This would be the version string of the block when it was backed up. This is not in the 1.9 backup information. I'll have to find what I can use in its place, possibly leaving it blank.

parentcontextid - This is likely the context instance id of the context the block is installed on. This is not part of the 1.9 backup, but the page id (PAGEID) is. I may be able to use it, since what is probably most important is consistency for any block on that page.

showinsubcontexts - This is a new feature of blocks previously not available in Moodle 1.9. I should be able to just default this to zero.

pagetypepattern - This is a new setting for blocks in M2. This looks to be similar to PAGETYPE, but is a setting that impacts what pages it will displayed on. This will require a bit of investigation.

subpagepattern - This is a new feature of blocks previously not available in Moodle 1.9. It looks like I should be able to default this to $@NULL@$.

defaultregion, defaultweight - These are other new settings for M2 and can probably be set to default values.

block_positions - The block positions structure's elements pagetype, visible, region and weight will be based on the PAGETYPE, VISIBLE, POSITION, and WEIGHT elements but will require some transforms. The other elements in there, id, contextid and subpage will require more investigation.

So, I have some idea of what I need to do with the block data. Now I need to plan for where that will happen (so I can play with code and see what I need to do).

There is already a moodle1_block_handler class defined in the "handlerlib.php" file. This has been created primarily to be extended by any block that needs it. Recall from my previous post, that all handlers get registered for use in the moodle1_handlers_factory::get_handlers() function. In this function, any block that provides a moodle1_block_handler instance also gets registered. I need to also register a handler for all blocks to do the generic work associated with every block.I'm thinking, this should all be part of the moodle1_block_handler class, such that every block is managed by functions in this class. And that class should be responsible for calling any specific block backup code provided by those blocks. I'm going to try designing this approach to see if it makes sense.

After I've bounced this idea of some of the developers in the tracker, I'll come back to this.

Thursday, October 11, 2012

Adding Moodle 1.9 Block Restore to Moodle 2.3 - Part 2

This is part two of my series on writing the Moodle 1.9 to 2.3 block restore conversion code for Moodle core.

When I finished part one, I had discovered that I could begin defining block restore handlers by creating a converter file and providing a minimum handler function. But, my handler was not yet providing the minimum amount of information to remove the warning from the restore logs.

Looking back at the document, I see that the get_paths() function needs to return an array for all XML elements of the restore. My "participants" block needs a path "/MOODLE_BACKUP/COURSE/BLOCKS/BLOCK/PARTICIPANTS" defined. This can be confirmed by the warning in the log file. As an experiment, I change the function in "/blocks/participants/backup/moodle1/lib.php" to look like:
public function get_paths() {
    return array(
        new convert_path('participants', '/MOODLE_BACKUP/COURSE/BLOCKS/BLOCK/PARTICIPANTS'),
    );
}
I also note that the document indicates that one process_xxx() function must exist for every path defined in get_paths(). So I also add:
public function process_participants($data) {
}
Retrying my restore, and reviewing the restore logs, the warning is now gone for my "participants" block. I am definitely on the right track.

Next, I'm going to review the "Data structure transformations" section to see what I need to do.

This section shows that the get_paths() function also provides transformation information for data elements that have changed from 1.9 to 2.x. This documentation is specific to modules but does help understand what needs to be done for blocks. I do know that quite a bit of the block data structure changed in Moodle 2, so I will need to create a transformation. But, in general, the block data that changed is generic to all blocks. I don't think I should be creating general block data transformation code in a specific block's handler. I need to do more research.

At this point, I am going to do a bit more code searching, to see if I can figure out what is happening internally. I know that everything essentially starts in the "/backup/converter/moodle1/lib.php" file with the moodle1_converter class. Inside that class, I find the init() function, which adds the MOD and BLOCK plug-ins to the object. Much of this is done through calls to the moodle1_handlers_factory class in the "/backup/converter/moodle1/handlerlib.php" file. Specifically this code, brings in each block's conversion handler:
$handlers = array_merge($handlers, self::get_plugin_handlers('block', $converter));
...
protected static function get_plugin_handlers($type, moodle1_converter $converter) {
    global $CFG;

    $handlers = array();
    $plugins = get_plugin_list($type);
    foreach ($plugins as $name => $dir) {
        $handlerfile  = $dir . '/backup/moodle1/lib.php';
        $handlerclass = "moodle1_{$type}_{$name}_handler";
        if (!file_exists($handlerfile)) {
            continue;
        }
        require_once($handlerfile);

        if (!class_exists($handlerclass)) {
            throw new moodle1_convert_exception('missing_handler_class', $handlerclass);
        }
        $handlers[] = new $handlerclass($converter, $type, $name);
    }
    return $handlers;
}
What this is doing is looking at each block in the system, and seeing if each has a converter handler available, by looking for a "/blocks/[blockname]/backup/moodle1/lib.php" file, and then checking to see if it has a moodle1_block_[blockname]_handler class within it. If there is, then that handler gets registered for use in the conversion. If the file does not exist, then it moves on. If the file exists, but the class does not, then an error is thrown.

This code explains what I've seen in my restore tests. Originally, I was seeing the restore log indicating that none of the blocks had attached handlers. This is because there was no file with the appropriate class for any of the blocks. When I added that file and class to the "participants" block, I initially saw an error. The error occurred because the handler was registered, but the class was not providing the proper functions. Once the function was provided, I no longer saw the "no handler attached" message in the restore log, because my handler had been successfully registered in the code above.

I also notice that in the moodle1_handlers_factory::get_handlers() function, there are a number of handlers registered that are contained within the same file, using the code:
$handlers = array(
    new moodle1_root_handler($converter),
    new moodle1_info_handler($converter),
    new moodle1_course_header_handler($converter),
    new moodle1_course_outline_handler($converter),
    new moodle1_roles_definition_handler($converter),
    new moodle1_question_bank_handler($converter),
    new moodle1_scales_handler($converter),
    new moodle1_outcomes_handler($converter),
    new moodle1_gradebook_handler($converter),
);
I am now thinking, that I need to start with a more generic block handler, similar to one of the above handlers. Looking back at David's comment in the original tracker issue about following "a pattern similar to the one we used for Question bank conversion" leads me to believe this is the right track. Next, I will need to explore how questions are handled in the converter.


Adding Moodle 1.9 Block Restore to Moodle 2.3 - Part 1

I discovered recently, when trying to restore a Moodle 1.9 course into a Moodle 2.3 site, that the blocks that were part of the 1.9 course were not restored at all to the Moodle 2.3 course. This is a problem for me and my clients.

To look into the situation, I went to the Moodle tracker to see if  there was any information about this. I found MDL-32880.

After some communication through the issue, and through other development channels, I discovered that in fact that functionality had not made it into Moodle 2 yet. This led me to start looking for what it would take to get this functionality done, and if you look at the tracker issue, well, led me to be the one assigned to the task! Be careful when you offer to help.

Anyway, since I now have to figure out what to do, how the Moodle 1.0 to Moodle 2 conversion works and how the entire Moodle 2 restore framework works, I am inviting you along.

This series will document my efforts to build the Moodle 1.9 to Moodle 2 course block restore functionality. You should have a good working knowledge of PHP and Moodle code to be able to follow along. And, I since this is a work in progress, I cannot guarantee success. But you should at least learn what I do.

. . .

The first struggle I have come up against is understanding how Moodle 1.9 courses get identified and how the execution structure works to convert and restore them into Moodle 2. There are some hints in the tracker item, referring to handlers and restore log output. For example, there is reference to the following messages in the restore log when blocks are attempted:
[Wed Aug 29 15:25:32 2012] [warn] (moodle1) no handler attached /MOODLE_BACKUP/COURSE/BLOCKS/BLOCK/PARTICIPANTS
[Wed Aug 29 15:25:32 2012] [warn] (moodle1) no handler attached /MOODLE_BACKUP/COURSE/BLOCKS/BLOCK/ACTIVITY_MODULES
[Wed Aug 29 15:25:32 2012] [warn] (moodle1) no handler attached /MOODLE_BACKUP/COURSE/BLOCKS/BLOCK/SEARCH_FORUMS
[Wed Aug 29 15:25:32 2012] [warn] (moodle1) no handler attached /MOODLE_BACKUP/COURSE/BLOCKS/BLOCK/ADMIN
[Wed Aug 29 15:25:32 2012] [warn] (moodle1) no handler attached /MOODLE_BACKUP/COURSE/BLOCKS/BLOCK/COURSE_LIST
[Wed Aug 29 15:25:32 2012] [warn] (moodle1) no handler attached /MOODLE_BACKUP/COURSE/BLOCKS/BLOCK/NEWS_ITEMS
[Wed Aug 29 15:25:32 2012] [warn] (moodle1) no handler attached /MOODLE_BACKUP/COURSE/BLOCKS/BLOCK/CALENDAR_UPCOMING
[Wed Aug 29 15:25:32 2012] [warn] (moodle1) no handler attached /MOODLE_BACKUP/COURSE/BLOCKS/BLOCK/RECENT_ACTIVITY
As well, there is a recommendation to look at the "/backup/converter/moodle1/handlerlib.php" file.

From this, I learn three things. First, the warning message comes from the process_chunk function in "/backup/converter/moodle1/lib.php". Second, there is an empty, defined block handler in "/backup/converter/moodle1/handlerlib.php" called moodle1_block_handler. Third, I still have no idea how any of this works together.

After a few more questions and a few more code searches, I have some more useful information. First, there is an architectural document called "Backup 1.9 conversion for developers" that begins to describe the process of conversion and the execution flow of the restore process. Second the log information for the restore is in the "moodledata" directory located in "/temp/backup" in files called "{restore-controller-id}.log". This will help me to run some tests and determine if I am on the correct track.

Because there isn't always a good document to follow when building something like this, I try to use what's there to determine what is needed. The log file and the code in process_chunk and moodle1_block_handler are telling me that the Moodle 1.9 to 2 restore conversion process already is set up to try and perform block conversion. I want to find out how much is already there for me.

Looking at the document I referred to, and the code used by course modules, I can see that any block will probably need a "backup/moodle1/lib.php" file to define the restore conversion code. And looking at the warnings in the restore log file, it looks like that file needs to have a block restore handler. And, looking at the way modules are restored, I believe that handler needs to extend the moodle1_block_handler class and add the block name to the extended class like moodle1_block_[blockname]_handler.

Armed with that, I'm going to experiment using the "Participants" block. I create a new "/blocks/participants/backup/moodle1/lib.php" file as follows:
<?php

/**
 * Provides support for the conversion of moodle1 backup to the moodle2 format
 *
 * @package    block_participants
 * @copyright  2012 Mike Churchward (mike@remote-learner.net)
 * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
 */

defined('MOODLE_INTERNAL') || die();

/**
 * Block Participants conversion handler
 */
class moodle1_block_participants_handler extends moodle1_block_handler {

}
Now, I am going to retry my Moodle 1.9 restore, and see what happens. If I get it right, the warning for that block should not appear in the restore log. Re-running the restore operation, I am stopped with the error:
Fatal error: Call to undefined method moodle1_block_participants_handler::get_paths() in /home/www/moodlehq.git/backup/converter/moodle1/lib.php on line 138
Believe it or not, this is reassuring. My addition to this block changed the way the restore operation worked. This means that there are definitely hooks already in the 1.9 to 2 restore conversion for blocks that are called when present. In this case, my new class was instantiated, executed and found wanting. But, it was executed!

Looking at the document I referred to, I see that it mentions that all module handlers must define the function "get_paths". This is also the function noted as missing in the error message I received. I'm going to add that function, but have it return an empty array for now, like:
public function get_paths() {
    return array(
    );
}
Retrying my restore operation, yields no errors. No blocks either; but I didn't expect any yet. And, unfortunately, I am still getting the "[warn] (moodle1) no handler attached /MOODLE_BACKUP/COURSE/BLOCKS/BLOCK/PARTICIPANTS" warning in the restore log. Still, I am pretty sure I now know where my work needs to begin. I will look more deeply into what the minimum handler requirements are, and see where I can go from there.

I'll pick this up in my next post.

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.