From 8645a28f3eac14651c7a222f8f2ad927bf08f3b3 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Fri, 9 Sep 2011 15:53:02 +0200 Subject: [PATCH 1/5] MDL-28488 backup - activity duplication requires 2 capabilities in ctx --- course/lib.php | 17 ++++++++++------- course/modduplicate.php | 1 + 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/course/lib.php b/course/lib.php index fe8630fb5fa..f98e9b3f884 100644 --- a/course/lib.php +++ b/course/lib.php @@ -3161,13 +3161,16 @@ function make_editing_buttons(stdClass $mod, $absolute = true, $moveselect = tru array('class' => 'editing_update', 'title' => $str->update) ); - // Duplicate - $actions[] = new action_link( - new moodle_url($baseurl, array('duplicate' => $mod->id)), - new pix_icon('t/copy', $str->duplicate, 'moodle', array('class' => 'iconsmall')), - null, - array('class' => 'editing_duplicate', 'title' => $str->duplicate) - ); + // Duplicate (require both target import caps to be able to duplicate, see modduplicate.php) + $dupecaps = array('moodle/backup:backuptargetimport', 'moodle/restore:restoretargetimport'); + if (has_all_capabilities($dupecaps, $coursecontext)) { + $actions[] = new action_link( + new moodle_url($baseurl, array('duplicate' => $mod->id)), + new pix_icon('t/copy', $str->duplicate, 'moodle', array('class' => 'iconsmall')), + null, + array('class' => 'editing_duplicate', 'title' => $str->duplicate) + ); + } // Delete $actions[] = new action_link( diff --git a/course/modduplicate.php b/course/modduplicate.php index 632f191853e..a9006b11ad3 100644 --- a/course/modduplicate.php +++ b/course/modduplicate.php @@ -44,6 +44,7 @@ $section = $DB->get_record('course_sections', array('id' => $cm->section, 'co require_login($course); require_sesskey(); require_capability('moodle/course:manageactivities', $context); +// Require both target import caps to be able to duplicate, see make_editing_buttons() require_capability('moodle/backup:backuptargetimport', $context); require_capability('moodle/restore:restoretargetimport', $context); From 77c2ca69af7a70e59613cf54e62f4a45d4ff75a2 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Fri, 9 Sep 2011 15:54:53 +0200 Subject: [PATCH 2/5] 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 9530e1ed6874259f8167e0f6d279342fc9e6ba15 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Sun, 11 Sep 2011 12:03:11 +0200 Subject: [PATCH 3/5] 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 45b1ab3d012..f94775c966f 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 f71435d0a3a..ecd7141719d 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 2a4ba40f3179aebe1f8fb38895b863c5c9ab4a2e Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Sun, 11 Sep 2011 19:09:59 +0200 Subject: [PATCH 4/5] 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 59fc0cbdb71e96771fa2e6ccaa152330d8eb8e26 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Sun, 11 Sep 2011 19:35:56 +0200 Subject: [PATCH 5/5] 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',