Friday, November 9, 2012

Adding Moodle 1.9 Block Restore to Moodle 2.3 - Part 6

In my last post, I had managed to get a working system that would successfully convert blocks in a 1.9 backup file to a Moodle 2.3 restore. I need to do a bit more testing, and then look for a solution for blocks that have specific backup/restore needs.

Testing uncovered that trying to restore a 1.9 course that had multiple versions of the same block installed within it (such as multiple HTML blocks) would cause an error, indicating that a file already existed. Referring back to the new code, this is likely due to the new process_block() function. In that function, a new XML file is created using the name of the block. This means that if more than one of the same block is converted, the code would try and create more than one file with the same name. I need to change that code to make sure it creates a unique file name in this situation.

I change the code:
$this->open_xml_writer("course/blocks/{$data['name']}/block.xml");
...
$this->open_xml_writer("course/blocks/{$data['name']}/inforef.xml");
...
$this->open_xml_writer("course/blocks/{$data['name']}/roles.xml");
to:
$this->open_xml_writer("course/blocks/{$data['name']}_{$instanceid}/block.xml");
...
$this->open_xml_writer("course/blocks/{$data['name']}_{$instanceid}/inforef.xml");
...
$this->open_xml_writer("course/blocks/{$data['name']}_{$instanceid}/roles.xml");
By adding $instanceid to the file path, I guarantee that each XML file will have a unique name. A quick test proves that this is now working.

The next issue to tackle is what to do with blocks that have their own backup/restore needs. The one Moodle 2 block that seems to have this requirement is the block rss_client. It will need its own backup converter.

In the tracker Paul Nicholls started to tackle this by creating a new "blocks/rss_client/backup/moodle1/lib.php" file with an extended class. When he tested the first version, a bug was discovered in my moodle1_handlers_factory::get_plugin_handlers() function changes. If a block does have its own class extended, the file containing it is not included, meaning that the handler class is unknown and an error occurs during the conversion process. Paul has also provided a fix for this error using his own git repository. To fix my copy, I will use the power of git collaboration.

First, I need to be able to pull code from Paul's repository into my local one. I have already made this possible by adding his repository as a remote named "pauln" to my local one. The commit that fixes the code is identified, so I cherry-pick it into my repository with:
git fetch pauln
git cherry-pick 630756fe1063e96cc7a6057bd512aa4afa53bc61

This modifies the code from:
if ($type != "block") {
    if (!file_exists($handlerfile)) {
        continue;
    }
    require_once($handlerfile);
} else {
    if (!file_exists($handlerfile)) {
        $handlerclass = "moodle1_block_generic_handler";
    }
}
to:
if (file_exists($handlerfile)) {
    require_once($handlerfile);
} elseif ($type == 'block') {
    $handlerclass = "moodle1_block_generic_handler";
} else {
    continue;
}
Paul has also provided a new version of the rss_client block with the new handlers. Again, I cherry-pick using:
git cherry-pick d89ade28060ae858356ff4a42cf946a4c99878c0
This provides me with a working copy of the conversion code that will correctly add the extra functionality that the Moodle 2.3 rss_client block requires.

For the record, here is the code for the rss_client conversion handler:
class moodle1_block_rss_client_handler extends moodle1_block_handler {
    public function process_block(array $data) {
        parent::process_block($data);
        $instanceid = $data['id'];
        $contextid = $this->converter->get_contextid(CONTEXT_BLOCK, $data['id']);

        // Moodle 1.9 backups do not include sufficient data to restore feeds, so we need an empty shell rss_client.xml
        // for the restore process to find
        $this->open_xml_writer("course/blocks/{$data['name']}_{$instanceid}/rss_client.xml");
        $this->xmlwriter->begin_tag('block', array('id' => $instanceid, 'contextid' => $contextid, 'blockname' => 'rss_client'));
        $this->xmlwriter->begin_tag('rss_client', array('id' => $instanceid));
        $this->xmlwriter->full_tag('feeds', '');
        $this->xmlwriter->end_tag('rss_client');
        $this->xmlwriter->end_tag('block');
        $this->close_xml_writer();

        return $data;
    }
}
I don't think that this handler will be typical of 1.9 blocks that need to restore their own data though. As it turns out, although the rss_client block does store its own data (feed definitions) in its own table, the 1.9 code never backed that data up. So all 1.9 backups contain only the standard block information for these blocks. But, in Moodle 2.3, the restore operation expects that the feed definition data will be there, and if its not included (even empty) then it causes an error. To work around this, the conversion handler creates an empty version of the expected feed definition XML file for all 1.9 conversions. I will do an extra post later to show how a block that really has extra data would provide the correct conversion code.

There are no other core Moodle blocks that require any specific restore conversion code, so I am essentially done the coding that is required by the tracker issue. Next, I'm going to ready the code for use by the HQ integrators and complete the tracker data so that they can consider it for inclusion into core.

First things I need to do is make sure my git repo is in a good state. To start with, I will make sure that the main branch of Moodle my development branch was based off of is up to date with the latest upstream Moodle code:
git checkout MOODLE_23_STABLE
git fetch upstream
git pull upstream
git push origin MOODLE_23_STABLE
This part isn't necessary, but I want to make my development branch's name a little more manageable, so I rename my MOODLE_23_STABLE_MDL-32880 branch to MDL-32880_2.3. This is purely for optics.

Now, I want to compress all of the work I have done on this issue into one commit. This makes for easier documentation and understanding, since several of my commits were code tests that were later undone. The "rebase" function of git allows me to do this. From the branch MDL-32880_2.3, I issue the command:
git rebase -i MOODLE_23_STABLE
This command will merge all of my commits with the base branch specified, keeping them in my branch, but allowing me to compress some things. The "-i" portion stands for "interactive" and brings up an editor screen that allows me to change what happens:
pick 00cab4e MDL-32880 - Playing with ideas.
pick c7849a8 MDL-32880 - Playing with more ideas.
pick 5036ed8 MDL-32880: moodle1 backup converter: add basic block handler
pick b346f7e MDL-32880 - Adding generic block conversion handlers.
pick 6a3045e MDL-32880 - Named block XML file with instance, and corrected use of currentmod to currentblock.
pick 7b9e3ec MDL-32880: moodle1 backup converter: Include custom block handers if present
pick 6fbf1fb MDL-32880: Add moodle1 backup converter for rss_client block.
pick 1012e75 MDL-32880 - Trying to use convert API in specific block instance.
pick 0b86f82 Revert "MDL-32880 - Trying to use convert API in specific block instance."

# Rebase ce44bf4..2bf7777 onto ce44bf4
#
# Commands:
#  p, pick = use commit
#  r, reword = use commit, but edit the commit message
#  e, edit = use commit, but stop for amending
#  s, squash = use commit, but meld into previous commit
#  f, fixup = like "squash", but discard this commit's log message
#  x, exec = run command (the rest of the line) using shell
#
# If you remove a line here THAT COMMIT WILL BE LOST.
# However, if you remove everything, the rebase will be aborted.
#
This screen shows all of my commits in my development branch with "pick" and their information next to them. I want to compress all of these changes into one commit, so I change all but the first "pick" to "squash", and save the edits. Once I do this, I have told git to combine and merge all of the commits into one diff commit. The next thing git does is allow me to create a new commit message for the new combined commit, which I simplify to:
"MDL-32880 - Adding 1.9 block conversion to M2 restore."

Now my MDL-32880_2.3 development branch is up to date with the main Moodle code, and contains just one committed change containing all of the necessary changes for the block restore conversion. This will make it easier for others to review and include in their testing.

The last things that need to be done in order to have this considered for inclusion into core is to complete the rest of the tracker information. Refer to the tacker item MDL-32880 once again to see the results. In this case, Paul has completed all of the necessary information which includes:
  • Testing instructions: Specific step-by-step instructions on how to verify that the code will do what it says. Paul has also attached a 1.9 backup file containing all core blocks.
  • Pull from Repository: Git repository containing the new code.
  • Pull X.X Branch: The name of the branch in the repository containing the code for the specified "X.X" version. For mine, it would be "MDL-32880_2.3".
  • Pull X.X Diff URL: A link to the repository that specifies a "diff" between the codebase and the changes, so that reviewer can see a compare view. This is made easy in github, using the "branches" tab and selecting your base branch from the drop-down, and the selecting the specific branch to compare against. For mine, it produces this link: https://github.com/mchurchward/moodle/compare/MOODLE_23_STABLE...MDL-32880_2.3
 I will continue this series next by providing an example of a 1.9 block that does convert its own data.