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.