Details
Minor
Description
The backup system allows lots of plugins to contribute to course backups, but not themes yet. Themes can create their own db tables and therefore may have per-course data. It would be good to allow themes to be included in backup.
Eloy has said this should be quite easy and should be OK to add, so we will code it and submit.
Gliffy Diagrams
Attachments
Activity
- All
- Comments
- Work Log
- History
- Activity
- Links Hierarchy
Also note - this ought to be documented better - perhaps I should put the 2 backup/restore plugin files from OU theme, onto the moodle docs wiki backup developer page in a section (or new page) about theme backup/restore? It's quite easy but without seeing the files I don't think anyone will figure it out. Here are the files for the record:
class backup_theme_ou_plugin extends backup_theme_plugin {
|
|
|
/**
|
* Returns the theme information to attach to course element
|
*/
|
protected function define_course_plugin_structure() {
|
if (!$this->is_current_theme('ou')) {
|
return null;
|
}
|
|
|
// Define virtual plugin element
|
$plugin = $this->get_plugin_element(null);
|
|
|
// Create plugin container element with standard name
|
$pluginwrapper = new backup_nested_element($this->get_recommended_name());
|
|
|
// Add wrapper to plugin
|
$plugin->add_child($pluginwrapper);
|
|
|
// Set up theme's own structure and add to wrapper
|
$studyplan = new backup_nested_element('ou', array('id'), array(
|
'variant'));
|
$pluginwrapper->add_child($studyplan);
|
|
|
// Use database to get source
|
$studyplan->set_source_table('theme_ou_courseoptions',
|
array('courseid' => backup::VAR_COURSEID));
|
|
|
// Include files which have theme_ou and area image and no itemid
|
$studyplan->annotate_files('theme_ou', 'image', null);
|
|
|
return $plugin;
|
}
|
}
|
|
|
class restore_theme_ou_plugin extends restore_theme_plugin {
|
|
|
/**
|
* Returns the paths to be handled by the plugin at course level
|
*/
|
protected function define_course_plugin_structure() {
|
$paths = array();
|
|
|
// Because of using get_recommended_name() it is able to find the
|
// correct path just by using the part inside the element name (which
|
// only has a /ou element).
|
$elepath = $this->get_pathfor('/ou');
|
|
|
// The 'ou' here defines that it will use the process_ou function
|
// to restore its element.
|
$paths[] = new restore_path_element('ou', $elepath);
|
|
|
return $paths;
|
}
|
|
|
/**
|
* Called after this runs for a course.
|
*/
|
function after_execute_course() {
|
// Need to restore file
|
$this->add_related_files('theme_ou', 'image', null);
|
}
|
|
|
/**
|
* Process the 'ou' element
|
*/
|
public function process_ou($data) {
|
global $DB;
|
|
|
// Get data record ready to insert in database
|
$data = (object)$data;
|
$data->courseid = $this->task->get_courseid();
|
|
|
// See if there is an existing record for this course
|
$existingid = $DB->get_field('theme_ou_courseoptions', 'id',
|
array('courseid'=>$data->courseid));
|
if ($existingid) {
|
$data->id = $existingid;
|
$DB->update_record('theme_ou_courseoptions', data);
|
} else {
|
$DB->insert_record('theme_ou_courseoptions', $data);
|
}
|
|
|
// No need to record the old/new id as nothing ever refers to
|
// the id of this table.
|
}
|
}
|
Hi Sam,
I'm going to integrated PULL-369 for now... to have it already available there.
Anyway... I think that we could, safely change your approach on backup to use the standard one already used @ course formats so it could look like this:
class backup_theme_ou_plugin extends backup_theme_plugin {
|
|
|
/**
|
* Returns the theme information to attach to course element
|
*/
|
protected function define_course_plugin_structure() {
|
|
|
// Define virtual plugin element
|
$plugin = $this->get_plugin_element(null, $this->get_theme_condition(), 'ou');
|
|
|
// Create plugin container element with standard name
|
$pluginwrapper = new backup_nested_element($this->get_recommended_name());
|
|
|
// Add wrapper to plugin
|
$plugin->add_child($pluginwrapper);
|
|
|
// Set up theme's own structure and add to wrapper
|
$studyplan = new backup_nested_element('ou', array('id'), array(
|
'variant'));
|
$pluginwrapper->add_child($studyplan);
|
|
|
// Use database to get source
|
$studyplan->set_source_table('theme_ou_courseoptions',
|
array('courseid' => backup::VAR_COURSEID));
|
|
|
// Include files which have theme_ou and area image and no itemid
|
$studyplan->annotate_files('theme_ou', 'image', null);
|
|
|
return $plugin;
|
}
|
}
|
The unique difference is that we change the initial is_current_theme() => return null condition at the beginning by proper condition when getting the theme plugin. Obviously if somebody wants to keep the condition clean, it can be done, though I think is better to show, as example, the condition in action in the proper place instead of the return null thing.
To achieve this all we need to do is (in backup_theme_plugin.class.php):
1) move the calculation of current theme to constructor and store it into $theme (or lazy load on 3 below)
2) delete the is_current_theme() method (not used any more, afaik).
3) implement the get_theme_condition() method that, simply, will do:
--
|
/**
|
* Return the condition encapsulated into sqlparam format
|
* to get evaluated by value, not by path nor processor setting
|
*/
|
protected function get_theme_condition() {
|
return array('sqlparam' => $this->theme);
|
}
|
I think that, this way, we'll have theme plugins working exactly the same than course formats ones, with the option to backup all still available (by omitting the condition). Although I wouldn't say publicly that "backup all" is the way, have concerns about that.
What do you think?
Note that, on restore... as far as the theme is something that can be defined out from the course scope... and by default restore restores as many paths as it can... perhaps you'll need some condition to check if the course being restored is going to, really, have the 'ou' theme or no. What if you restore it into one "standard" category? Do you want those theme/ou records restored?
Summary: I hate to decide if those bits of informations have to be really restored or no, grrr.
Re restore - I think it's easiest to just restore it if it is included in the backup, assuming the theme actually exists on target system (if not there is obviously no restore_theme_whatever to use) - it won't do any harm at least.
Otherwise, could use same logic (function) as in backup to determine current theme of newly created course, if required.
Re your proposed backup change - the condition thing is hard for me to understand, sorry. 
Here's what I think you are saying:
- In the constructor the theme_plugin will use the existing function I made in dbops (or move it, but anyway, that code) in order to work out the actual theme for the course. This will be stored in variable $this->theme. [NOTE: Should maybe rename to $this->actualtheme or $this->coursetheme or something, to indicate that it does not really relate to the theme of the plugin?]
- Somehow by some bizarre magic, the condition function shown actually compares the name of the plugin's theme (is that the 'ou' string in the 3rd param of get_plugin_element) and returns true if it matches the $this->theme.
If so then sure it should work! I can test it... Do you want me to make those code changes or are you planning to do it?
Hehe,
I think your thoughts about my sayings are ok, lol.
About the changes... if you've time/interest on it, plz do it. As far as you've a real theme plugin there... I think it's better. Else, I'll need to "invent" some theme backup & restore to check it works.
About restore, agree. We can restore all the existing (installed) plugins information. Each restore can decide if finally the information is restored or no, by using any logic in the corresponding process_xxx() methods.
And yes, the condition is dark/bizarre magic, but was the best solution I found when implementing the course formats, plugin. Surely it can be left hidden to developers by overriding the get_plugin_element() method in the backup theme plugin, but then we miss the ability to skip defining any condition and it will become mandatory.
Ciao 
I have made the changes Eloy suggested, as PULL-395.
The attached file fusionbackup.zip is a fake backup data folder for the 'fusion' theme. If you install this (for testing only) then when you backup a course using the 'fusion' theme, it will include theme data in course.xml [The actual content of this data is just the course format, which is a bit random.] If you restore a course containing this data, you'll see a print_object also showing the course format.
Documentation now written (assumes that the PULL-395 changes are implemented):
This issue is fixed in the latest 2.0.2+. Thanks Sam.
Note I have also fixed a bug where course theme setting was not restored correctly, as part of this change.
Filed PULL-369 as attempt at this.