From f850a316f7657bf090b5f10957ad9cf97214dc8d Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Fri, 9 Sep 2011 15:54:53 +0200 Subject: [PATCH 1/4] MDL-28488 backup - make backup and import capabilities independent Before this change, in order to perform one course import it was necessary to have both the backupcourse and the backuptargetimport capabilities. After agreement now each one will control its own backup mode. Same applies for restore. --- backup/util/checks/backup_check.class.php | 46 +++++++++++----------- backup/util/checks/restore_check.class.php | 46 +++++++++++----------- 2 files changed, 44 insertions(+), 48 deletions(-) diff --git a/backup/util/checks/backup_check.class.php b/backup/util/checks/backup_check.class.php index 39d0e96fa1a..63ec0cd16a8 100644 --- a/backup/util/checks/backup_check.class.php +++ b/backup/util/checks/backup_check.class.php @@ -102,45 +102,31 @@ abstract class backup_check { // Note: all the checks along the function MUST be performed for $userid, that // is the user who "requested" the course backup, not current $USER at all!! - // First of all, check the main backup[course|section|activity] principal caps - // Lacking the corresponding one makes this to break with exception always + // First of all, decide which caps/contexts are we going to check + // for common backups (general, automated...) based exclusively + // in the type (course, section, activity). And store them into + // one capability => context array structure + $typecapstocheck = array(); switch ($type) { case backup::TYPE_1COURSE : $DB->get_record('course', array('id' => $id), '*', MUST_EXIST); // course exists - if (!has_capability('moodle/backup:backupcourse', $coursectx, $userid)) { - $a = new stdclass(); - $a->userid = $userid; - $a->courseid = $courseid; - $a->capability = 'moodle/backup:backupcourse'; - throw new backup_controller_exception('backup_user_missing_capability', $a); - } + $typecapstocheck['moodle/backup:backupcourse'] = $coursectx; break; case backup::TYPE_1SECTION : $DB->get_record('course_sections', array('course' => $courseid, 'id' => $id), '*', MUST_EXIST); // sec exists - if (!has_capability('moodle/backup:backupsection', $coursectx, $userid)) { - $a = new stdclass(); - $a->userid = $userid; - $a->courseid = $courseid; - $a->capability = 'moodle/backup:backupsection'; - throw new backup_controller_exception('backup_user_missing_capability', $a); - } + $typecapstocheck['moodle/backup:backupsection'] = $coursectx; break; case backup::TYPE_1ACTIVITY : get_coursemodule_from_id(null, $id, $courseid, false, MUST_EXIST); // cm exists $modulectx = get_context_instance(CONTEXT_MODULE, $id); - if (!has_capability('moodle/backup:backupactivity', $modulectx, $userid)) { - $a = new stdclass(); - $a->userid = $userid; - $a->cmid = $id; - $a->capability = 'moodle/backup:backupactivity'; - throw new backup_controller_exception('backup_user_missing_capability', $a); - } + $typecapstocheck['moodle/backup:backupactivity'] = $modulectx; break; default : - print_error('unknownbackuptype'); + throw new backup_controller_exception('backup_unknown_backup_type', $type); } // Now, if backup mode is hub or import, check userid has permissions for those modes + // other modes will perform common checks only (backupxxxx capabilities in $typecapstocheck) switch ($mode) { case backup::MODE_HUB: if (!has_capability('moodle/backup:backuptargethub', $coursectx, $userid)) { @@ -160,6 +146,18 @@ abstract class backup_check { throw new backup_controller_exception('backup_user_missing_capability', $a); } break; + // Common backup (general, automated...), let's check all the $typecapstocheck + // capability => context pairs + default: + foreach ($typecapstocheck as $capability => $context) { + if (!has_capability($capability, $context, $userid)) { + $a = new stdclass(); + $a->userid = $userid; + $a->courseid = $courseid; + $a->capability = $capability; + throw new backup_controller_exception('backup_user_missing_capability', $a); + } + } } // Now, enforce 'moodle/backup:userinfo' to 'users' setting, applying changes if allowed, diff --git a/backup/util/checks/restore_check.class.php b/backup/util/checks/restore_check.class.php index 20e1f94c331..0617732b8b9 100644 --- a/backup/util/checks/restore_check.class.php +++ b/backup/util/checks/restore_check.class.php @@ -68,41 +68,27 @@ abstract class restore_check { // Note: all the checks along the function MUST be performed for $userid, that // is the user who "requested" the course restore, not current $USER at all!! - // First of all, check the main restore[course|section|activity] principal caps - // Lacking the corresponding one makes this to break with exception always + // First of all, decide which caps/contexts are we going to check + // for common backups (general, automated...) based exclusively + // in the type (course, section, activity). And store them into + // one capability => context array structure + $typecapstocheck = array(); switch ($type) { case backup::TYPE_1COURSE : - if (!has_capability('moodle/restore:restorecourse', $coursectx, $userid)) { - $a = new stdclass(); - $a->userid = $userid; - $a->courseid = $courseid; - $a->capability = 'moodle/restore:restorecourse'; - throw new restore_controller_exception('restore_user_missing_capability', $a); - } + $typecapstocheck['moodle/restore:restorecourse'] = $coursectx; break; case backup::TYPE_1SECTION : - if (!has_capability('moodle/restore:restoresection', $coursectx, $userid)) { - $a = new stdclass(); - $a->userid = $userid; - $a->courseid = $courseid; - $a->capability = 'moodle/restore:restoresection'; - throw new restore_controller_exception('restore_user_missing_capability', $a); - } + $typecapstocheck['moodle/restore:restoresection'] = $coursectx; break; case backup::TYPE_1ACTIVITY : - if (!has_capability('moodle/restore:restoreactivity', $coursectx, $userid)) { - $a = new stdclass(); - $a->userid = $userid; - $a->courseid = $courseid; - $a->capability = 'moodle/restore:restoreactivity'; - throw new restore_controller_exception('restore_user_missing_capability', $a); - } + $typecapstocheck['moodle/restore:restoreactivity'] = $coursectx; break; default : - print_error('unknownrestoretype'); + throw new restore_controller_exception('restore_unknown_restore_type', $type); } // Now, if restore mode is hub or import, check userid has permissions for those modes + // other modes will perform common checks only (restorexxxx capabilities in $typecapstocheck) switch ($mode) { case backup::MODE_HUB: if (!has_capability('moodle/restore:restoretargethub', $coursectx, $userid)) { @@ -122,6 +108,18 @@ abstract class restore_check { throw new restore_controller_exception('restore_user_missing_capability', $a); } break; + // Common backup (general, automated...), let's check all the $typecapstocheck + // capability => context pairs + default: + foreach ($typecapstocheck as $capability => $context) { + if (!has_capability($capability, $context, $userid)) { + $a = new stdclass(); + $a->userid = $userid; + $a->courseid = $courseid; + $a->capability = $capability; + throw new restore_controller_exception('restore_user_missing_capability', $a); + } + } } // Now, enforce 'moodle/restore:userinfo' to 'users' setting, applying changes if allowed, From a31173a6a64b338aad5f1ceeecb64366ebb60adf Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Sun, 11 Sep 2011 12:03:11 +0200 Subject: [PATCH 2/4] MDL-28488 backup - fixup some strings and delete legacy ones --- admin/settings/courses.php | 4 ++-- lang/en/moodle.php | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/admin/settings/courses.php b/admin/settings/courses.php index 13291c78093..4dcf00faec9 100644 --- a/admin/settings/courses.php +++ b/admin/settings/courses.php @@ -142,7 +142,7 @@ if ($hassiteconfig $temp->add(new admin_setting_heading('automatedsettings', get_string('automatedsettings','backup'), '')); - $temp->add(new admin_setting_configcheckbox('backup/backup_auto_users', get_string('users'), get_string('backupusershelp'), 1)); + $temp->add(new admin_setting_configcheckbox('backup/backup_auto_users', get_string('generalusers', 'backup'), get_string('configgeneralusers', 'backup'), 1)); $temp->add(new admin_setting_configcheckbox('backup/backup_auto_role_assignments', get_string('generalroleassignments','backup'), get_string('configgeneralroleassignments','backup'), 1)); $temp->add(new admin_setting_configcheckbox('backup/backup_auto_user_files', get_string('generaluserfiles', 'backup'), get_string('configgeneraluserfiles','backup'), 1)); $temp->add(new admin_setting_configcheckbox('backup/backup_auto_activities', get_string('generalactivities','backup'), get_string('configgeneralactivities','backup'), 1)); @@ -150,7 +150,7 @@ if ($hassiteconfig $temp->add(new admin_setting_configcheckbox('backup/backup_auto_filters', get_string('generalfilters','backup'), get_string('configgeneralfilters','backup'), 1)); $temp->add(new admin_setting_configcheckbox('backup/backup_auto_comments', get_string('generalcomments','backup'), get_string('configgeneralcomments','backup'), 1)); $temp->add(new admin_setting_configcheckbox('backup/backup_auto_userscompletion', get_string('generaluserscompletion','backup'), get_string('configgeneraluserscompletion','backup'), 1)); - $temp->add(new admin_setting_configcheckbox('backup/backup_auto_logs', get_string('logs'), get_string('backuplogshelp'), 0)); + $temp->add(new admin_setting_configcheckbox('backup/backup_auto_logs', get_string('generallogs', 'backup'), get_string('configgenerallogs', 'backup'), 0)); $temp->add(new admin_setting_configcheckbox('backup/backup_auto_histories', get_string('generalhistories','backup'), get_string('configgeneralhistories','backup'), 0)); diff --git a/lang/en/moodle.php b/lang/en/moodle.php index 009ce6669b8..09ffd04008a 100644 --- a/lang/en/moodle.php +++ b/lang/en/moodle.php @@ -178,7 +178,6 @@ $string['backupincludemoduleuserdatahelp'] = 'Choose whether you want to include $string['backupkeephelp'] = 'How many recent backups for each course do you want to keep? (older ones will be deleted automatically)'; $string['backuplogdetailed'] = 'Detailed execution log'; $string['backuploglaststatus'] = 'Last execution log'; -$string['backuplogshelp'] = 'If enabled, then course logs will be included in automated backups'; $string['backupmissinguserinfoperms'] = 'Note: This backup contains no user data. Exercise and Workshop activities will not be included in the backup, since these modules are not compatible with this type of backup.'; $string['backupnext'] = 'Next backup'; $string['backupnonisowarning'] = 'Warning: this backup is from a non-Unicode version of Moodle (pre 1.6). If this backup contains any non-ISO-8859-1 texts then they may be CORRUPTED if you try to restore them to this Unicode version of Moodle. See the Backup FAQ for more information about how to recover this backup correctly.'; @@ -189,7 +188,6 @@ $string['backupsitefileshelp'] = 'If enabled then site files used in courses wil $string['backuptakealook'] = 'Please take a look at your backup logs in: {$a}'; $string['backupuserfileshelp'] = 'Choose whether user files (eg profile images) should be included in automated backups'; -$string['backupusershelp'] = 'Select whether you want to include all the users in the server or only the needed users for each course'; $string['backupversion'] = 'Backup version'; $string['block'] = 'Block'; $string['blockconfiga'] = 'Configuring a {$a} block'; From 53c074a81c79ce1b258b2709c4fa91af3b273845 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Sun, 11 Sep 2011 19:09:59 +0200 Subject: [PATCH 3/4] MDL-28488 backup - '[backup|restore]:configure cap does not control import mode anymore --- backup/util/checks/backup_check.class.php | 20 ++++++++++++-------- backup/util/checks/restore_check.class.php | 16 ++++++++++------ 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/backup/util/checks/backup_check.class.php b/backup/util/checks/backup_check.class.php index 63ec0cd16a8..881c73c3ae2 100644 --- a/backup/util/checks/backup_check.class.php +++ b/backup/util/checks/backup_check.class.php @@ -217,15 +217,19 @@ abstract class backup_check { } // Check the user has the ability to configure the backup. If not then we need - // to lock all settings by permission so that no changes can be made. - $hasconfigcap = has_capability('moodle/backup:configure', $coursectx, $userid); - if (!$hasconfigcap) { - $settings = $backup_controller->get_plan()->get_settings(); - foreach ($settings as $setting) { - if ($setting->get_name()=='filename') { - continue; + // to lock all settings by permission so that no changes can be made. This does + // not apply to the import facility, where the activities must be always enabled + // to be able to pick them + if ($mode != backup::MODE_IMPORT) { + $hasconfigcap = has_capability('moodle/backup:configure', $coursectx, $userid); + if (!$hasconfigcap) { + $settings = $backup_controller->get_plan()->get_settings(); + foreach ($settings as $setting) { + if ($setting->get_name() == 'filename') { + continue; + } + $setting->set_status(base_setting::LOCKED_BY_PERMISSION); } - $setting->set_status(base_setting::LOCKED_BY_PERMISSION); } } diff --git a/backup/util/checks/restore_check.class.php b/backup/util/checks/restore_check.class.php index 0617732b8b9..915112cf230 100644 --- a/backup/util/checks/restore_check.class.php +++ b/backup/util/checks/restore_check.class.php @@ -156,12 +156,16 @@ abstract class restore_check { } // Check the user has the ability to configure the restore. If not then we need - // to lock all settings by permission so that no changes can be made. - $hasconfigcap = has_capability('moodle/restore:configure', $coursectx, $userid); - if (!$hasconfigcap) { - $settings = $restore_controller->get_plan()->get_settings(); - foreach ($settings as $setting) { - $setting->set_status(base_setting::LOCKED_BY_PERMISSION); + // to lock all settings by permission so that no changes can be made. This does + // not apply to the import facility, where all the activities (picked on backup) + // are restored automatically without restore UI + if ($mode != backup::MODE_IMPORT) { + $hasconfigcap = has_capability('moodle/restore:configure', $coursectx, $userid); + if (!$hasconfigcap) { + $settings = $restore_controller->get_plan()->get_settings(); + foreach ($settings as $setting) { + $setting->set_status(base_setting::LOCKED_BY_PERMISSION); + } } } From d561fa9527c0f319f6d6ab99c74f6b6392402b93 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Sun, 11 Sep 2011 19:35:56 +0200 Subject: [PATCH 4/4] MDL-28488 backup - conditionaly apply defaults based on mode Some modes of backup cannot be preloaded with general backup settings because they may conflict with the definitions required/set by the mode (for example, import backups are, by definition, without users always, so we cannot apply one default in the opposite direction). So, after the patch, current general_backup setting will be only applied to general backups, import/hub modes won't be using any default and automated backup defaults should be loaded here (TODO) --- backup/controller/backup_controller.class.php | 2 +- .../dbops/backup_controller_dbops.class.php | 30 ++++++++++++++++++- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/backup/controller/backup_controller.class.php b/backup/controller/backup_controller.class.php index cd054815134..1bab62be627 100644 --- a/backup/controller/backup_controller.class.php +++ b/backup/controller/backup_controller.class.php @@ -339,7 +339,7 @@ class backup_controller extends backup implements loggable { protected function apply_defaults() { $this->log('applying plan defaults', backup::LOG_DEBUG); - backup_controller_dbops::apply_general_config_defaults($this); + backup_controller_dbops::apply_config_defaults($this); $this->set_status(backup::STATUS_CONFIGURED); } } diff --git a/backup/util/dbops/backup_controller_dbops.class.php b/backup/util/dbops/backup_controller_dbops.class.php index d760871869b..cd89d82f55f 100644 --- a/backup/util/dbops/backup_controller_dbops.class.php +++ b/backup/util/dbops/backup_controller_dbops.class.php @@ -401,12 +401,40 @@ abstract class backup_controller_dbops extends backup_dbops { return $DB->get_record('course', array('id' => $courseid), 'fullname, shortname, startdate'); } + /** + * Sets the default values for the settings in a backup operation + * + * Based on the mode of the backup it will delegate the process to + * other methods like {@link apply_general_config_defaults} ... + * to get proper defaults loaded + * + * @param backup_controller $controller + */ + public static function apply_config_defaults(backup_controller $controller) { + // Based on the mode of the backup (general, automated, import, hub...) + // decide the action to perform to get defaults loaded + $mode = $controller->get_mode(); + + switch ($mode) { + case backup::MODE_GENERAL: + // Load the general defaults + self::apply_general_config_defaults($controller); + break; + case backup::MODE_AUTOMATED: + // TODO: Move the loading from automatic stuff to here + break; + default: + // Nothing to do for other modes (IMPORT/HUB...). Some day we + // can define defaults (admin UI...) for them if we want to + } + } + /** * Sets the controller settings default values from the backup config. * * @param backup_controller $controller */ - public static function apply_general_config_defaults(backup_controller $controller) { + private static function apply_general_config_defaults(backup_controller $controller) { $settings = array( // Config name => Setting name 'backup_general_users' => 'users',