Permalink
Browse files

MDL-34727 restore UI: use checkboxes for userdata.

It was using select menus for the convenience of the code, but the
inconvenience of users.

The way this fix is done is a bit hacky, but it works, makes users'
lives much better, but it would be good if someone would dehackify
this in the future.
  • Loading branch information...
1 parent e203643 commit 57f9291b23410bc63f86b5ff8d42d7232c9569e1 @timhunt timhunt committed Aug 3, 2012
Showing with 45 additions and 14 deletions.
  1. +23 −7 backup/moodle2/restore_activity_task.class.php
  2. +22 −7 backup/moodle2/restore_section_task.class.php
@@ -293,30 +293,46 @@ protected function define_settings() {
// Define activity_userinfo. Dependent of:
// - users root setting
// - section_userinfo setting (if exists)
- // - activity_included setting
+ // - activity_included setting.
$settingname = $settingprefix . 'userinfo';
- $selectvalues = array(0=>get_string('no')); // Safer options
- $defaultvalue = false; // Safer default
+ $defaultvalue = false;
if (isset($this->info->settings[$settingname]) && $this->info->settings[$settingname]) { // Only enabled when available
- $selectvalues = array(1=>get_string('yes'), 0=>get_string('no'));
$defaultvalue = true;
}
+
$activity_userinfo = new restore_activity_userinfo_setting($settingname, base_setting::IS_BOOLEAN, $defaultvalue);
- $activity_userinfo->set_ui(new backup_setting_ui_select($activity_userinfo, get_string('includeuserinfo','backup'), $selectvalues));
+ if (!$defaultvalue) {
+ // This is a bit hacky, but if there is no user data to restore, then
+ // we replace the standard check-box with a select menu with the
+ // single choice 'No', and the select menu is clever enough that if
+ // there is only one choice, it just displays a static string.
+ //
+ // It would probably be better design to have a special UI class
+ // setting_ui_checkbox_or_no, rather than this hack, but I am not
+ // going to do that today.
+ $activity_userinfo->set_ui(new backup_setting_ui_select($activity_userinfo, '-',
+ array(0 => get_string('no'))));
+ } else {
+ $activity_userinfo->get_ui()->set_label('-');
+ }
+
$this->add_setting($activity_userinfo);
+
// Look for "users" root setting
$users = $this->plan->get_setting('users');
$users->add_dependency($activity_userinfo);
+
// Look for "section_userinfo" section setting (if exists)
$settingname = 'section_' . $this->info->sectionid . '_userinfo';
if ($this->plan->setting_exists($settingname)) {
$section_userinfo = $this->plan->get_setting($settingname);
$section_userinfo->add_dependency($activity_userinfo);
}
- // Look for "activity_included" setting
+
+ // Look for "activity_included" setting.
$activity_included->add_dependency($activity_userinfo);
- // End of common activity settings, let's add the particular ones
+ // End of common activity settings, let's add the particular ones.
$this->define_my_settings();
}
@@ -169,21 +169,36 @@ protected function define_settings() {
// Define section_userinfo. Dependent of:
// - users root setting
- // - section_included setting
+ // - section_included setting.
$settingname = $settingprefix . 'userinfo';
- $selectvalues = array(0=>get_string('no')); // Safer options
- $defaultvalue = false; // Safer default
+ $defaultvalue = false;
if (isset($this->info->settings[$settingname]) && $this->info->settings[$settingname]) { // Only enabled when available
- $selectvalues = array(1=>get_string('yes'), 0=>get_string('no'));
$defaultvalue = true;
}
+
$section_userinfo = new restore_section_userinfo_setting($settingname, base_setting::IS_BOOLEAN, $defaultvalue);
- $section_userinfo->set_ui(new backup_setting_ui_select($section_userinfo, get_string('includeuserinfo','backup'), $selectvalues));
+ if (!$defaultvalue) {
+ // This is a bit hacky, but if there is no user data to restore, then
+ // we replace the standard check-box with a select menu with the
+ // single choice 'No', and the select menu is clever enough that if
+ // there is only one choice, it just displays a static string.
+ //
+ // It would probably be better design to have a special UI class
+ // setting_ui_checkbox_or_no, rather than this hack, but I am not
+ // going to do that today.
+ $section_userinfo->set_ui(new backup_setting_ui_select($section_userinfo, get_string('includeuserinfo','backup'),
+ array(0 => get_string('no'))));
+ } else {
+ $section_userinfo->get_ui()->set_label(get_string('includeuserinfo','backup'));
+ }
+
$this->add_setting($section_userinfo);
- // Look for "users" root setting
+
+ // Look for "users" root setting.
$users = $this->plan->get_setting('users');
$users->add_dependency($section_userinfo);
- // Look for "section_included" section setting
+
+ // Look for "section_included" section setting.
$section_included->add_dependency($section_userinfo);
}
}

0 comments on commit 57f9291

Please sign in to comment.