summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDavid Lawrence <dkl@mozilla.com>2015-10-21 05:00:37 +0200
committerDavid Lawrence <dkl@mozilla.com>2015-10-21 05:00:37 +0200
commitc8cf2b58bae632b6801337971323d37cf9f5a0f0 (patch)
tree9825f8b4c9ace9343678595c3a6cbe14ce9bb6e9
parentd1b6f1d78bdcde363cbcc80c4e589742819a4e24 (diff)
downloadbugzilla-c8cf2b58bae632b6801337971323d37cf9f5a0f0.tar.gz
bugzilla-c8cf2b58bae632b6801337971323d37cf9f5a0f0.tar.xz
Bug 1209599 - group general preferences by category
-rw-r--r--Bugzilla/Install.pm9
-rw-r--r--Bugzilla/User/Setting.pm62
-rw-r--r--extensions/BMO/Extension.pm16
-rw-r--r--extensions/BugModal/Extension.pm14
-rw-r--r--extensions/Example/Extension.pm9
-rw-r--r--extensions/Gravatar/Extension.pm14
-rw-r--r--extensions/InlineHistory/Extension.pm7
-rw-r--r--extensions/Needinfo/Extension.pm7
-rw-r--r--extensions/OrangeFactor/Extension.pm7
-rw-r--r--extensions/RequestNagger/Extension.pm7
-rw-r--r--extensions/Review/Extension.pm7
-rw-r--r--skins/standard/admin.css10
-rw-r--r--template/en/default/account/prefs/settings.html.tmpl73
-rw-r--r--template/en/default/admin/settings/edit.html.tmpl4
-rwxr-xr-xuserprefs.cgi27
15 files changed, 177 insertions, 96 deletions
diff --git a/Bugzilla/Install.pm b/Bugzilla/Install.pm
index 9950b222a..715251154 100644
--- a/Bugzilla/Install.pm
+++ b/Bugzilla/Install.pm
@@ -281,13 +281,8 @@ sub update_settings {
}
my @settings = @{SETTINGS()};
- foreach my $setting (@settings) {
- add_setting($setting->{name},
- $setting->{options},
- $setting->{default},
- $setting->{subclass},
- undef,
- !$any_settings);
+ foreach my $params (@settings) {
+ add_setting($params);
}
}
diff --git a/Bugzilla/User/Setting.pm b/Bugzilla/User/Setting.pm
index e083a251e..f566b3459 100644
--- a/Bugzilla/User/Setting.pm
+++ b/Bugzilla/User/Setting.pm
@@ -66,10 +66,10 @@ sub new {
# to retrieve the information for this setting ourselves.
if (scalar @_ == 0) {
- my ($default, $is_enabled, $value);
- ($default, $is_enabled, $value, $subclass) =
+ my ($default, $is_enabled, $value, $category);
+ ($default, $is_enabled, $value, $subclass, $category) =
$dbh->selectrow_array(
- q{SELECT default_value, is_enabled, setting_value, subclass
+ q{SELECT default_value, is_enabled, setting_value, subclass, category
FROM setting
LEFT JOIN profile_setting
ON setting.name = profile_setting.setting_name
@@ -80,17 +80,18 @@ sub new {
# if not defined, then grab the default value
if (! defined $value) {
- ($default, $is_enabled, $subclass) =
+ ($default, $is_enabled, $subclass, $category) =
$dbh->selectrow_array(
- q{SELECT default_value, is_enabled, subclass
+ q{SELECT default_value, is_enabled, subclass, category
FROM setting
WHERE name = ?},
undef,
$setting_name);
}
- $self->{'is_enabled'} = $is_enabled;
+ $self->{'is_enabled'} = $is_enabled;
$self->{'default_value'} = $default;
+ $self->{'category'} = $category;
# IF the setting is enabled, AND the user has chosen a setting
# THEN return that value
@@ -109,6 +110,7 @@ sub new {
$self->{'value'} = shift;
$self->{'is_default'} = shift;
$subclass = shift;
+ $self->{'category'} = shift;
}
if ($subclass) {
eval('require ' . $class . '::' . $subclass);
@@ -129,14 +131,27 @@ sub new {
###############################
sub add_setting {
- my ($name, $values, $default_value, $subclass, $force_check,
- $silently) = @_;
+ my ($params) = @_;
+ my ($name, $options, $default, $subclass, $force_check, $silently, $category)
+ = @$params{qw( name options default subclass force_check silently category )};
my $dbh = Bugzilla->dbh;
+ # Categories were added later, so we need to check if the old
+ # setting has the correct category if provided
my $exists = _setting_exists($name);
+ if ($exists && $category) {
+ my $old_category = $dbh->selectrow_arrayref(
+ "SELECT category FROM setting WHERE name = ?", undef, $name);
+ if ($old_category ne $category) {
+ $dbh->do('UPDATE setting SET category = ? WHERE name = ?',
+ undef, $category, $name);
+ Bugzilla->memcached->clear_config();
+ }
+ }
+
return if ($exists && !$force_check);
- ($name && $default_value)
+ ($name && $default)
|| ThrowCodeError("setting_info_invalid");
if ($exists) {
@@ -144,8 +159,8 @@ sub add_setting {
$dbh->do('DELETE FROM setting_value WHERE name = ?', undef, $name);
$dbh->do('DELETE FROM setting WHERE name = ?', undef, $name);
# Remove obsolete user preferences for this setting.
- if (defined $values && scalar(@$values)) {
- my $list = join(', ', map {$dbh->quote($_)} @$values);
+ if (defined $options && scalar(@$options)) {
+ my $list = join(', ', map {$dbh->quote($_)} @$options);
$dbh->do("DELETE FROM profile_setting
WHERE setting_name = ? AND setting_value NOT IN ($list)",
undef, $name);
@@ -154,15 +169,15 @@ sub add_setting {
elsif (!$silently) {
print get_text('install_setting_new', { name => $name }) . "\n";
}
- $dbh->do(q{INSERT INTO setting (name, default_value, is_enabled, subclass)
- VALUES (?, ?, 1, ?)},
- undef, ($name, $default_value, $subclass));
+ $dbh->do(q{INSERT INTO setting (name, default_value, is_enabled, subclass, category)
+ VALUES (?, ?, 1, ?, ?)},
+ undef, ($name, $default, $subclass, $category));
my $sth = $dbh->prepare(q{INSERT INTO setting_value (name, value, sortindex)
VALUES (?, ?, ?)});
my $sortindex = 5;
- foreach my $key (@$values){
+ foreach my $key (@$options){
$sth->execute($name, $key, $sortindex);
$sortindex += 5;
}
@@ -177,7 +192,7 @@ sub get_all_settings {
my $rows = Bugzilla->memcached->get_config({ key => $cache_key });
if (!$rows) {
$rows = $dbh->selectall_arrayref(
- q{SELECT name, default_value, is_enabled, setting_value, subclass
+ q{SELECT name, default_value, is_enabled, setting_value, subclass, category
FROM setting
LEFT JOIN profile_setting
ON setting.name = profile_setting.setting_name
@@ -186,7 +201,7 @@ sub get_all_settings {
}
foreach my $row (@$rows) {
- my ($name, $default_value, $is_enabled, $value, $subclass) = @$row;
+ my ($name, $default_value, $is_enabled, $value, $subclass, $category) = @$row;
my $is_default;
@@ -199,7 +214,8 @@ sub get_all_settings {
$settings->{$name} = new Bugzilla::User::Setting(
$name, $user_id, $is_enabled,
- $default_value, $value, $is_default, $subclass);
+ $default_value, $value, $is_default,
+ $subclass, $category);
}
return $settings;
@@ -217,15 +233,15 @@ sub get_defaults {
$user_id ||= 0;
- my $rows = $dbh->selectall_arrayref(q{SELECT name, default_value, is_enabled, subclass
+ my $rows = $dbh->selectall_arrayref(q{SELECT name, default_value, is_enabled, subclass, category
FROM setting});
foreach my $row (@$rows) {
- my ($name, $default_value, $is_enabled, $subclass) = @$row;
+ my ($name, $default_value, $is_enabled, $subclass, $category) = @$row;
$default_settings->{$name} = new Bugzilla::User::Setting(
$name, $user_id, $is_enabled, $default_value, $default_value, 1,
- $subclass);
+ $subclass, $category);
}
return $default_settings;
@@ -347,7 +363,7 @@ $settings->{$setting_name} = new Bugzilla::User::Setting(
=over 4
-=item C<add_setting($name, \@values, $default_value, $subclass, $force_check)>
+=item C<add_setting($name, \@values, $default_value, $subclass, $force_check, $category)>
Description: Checks for the existence of a setting, and adds it
to the database if it does not yet exist.
@@ -361,6 +377,8 @@ Params: C<$name> - string - the name of the new setting
not stored in the DB.
C<$force_check> - boolean - when true, the existing setting
and all its values are deleted and replaced by new data.
+ C<$category> - string - Category the setting should be
+ grouped by.
Returns: a pointer to a hash of settings
diff --git a/extensions/BMO/Extension.pm b/extensions/BMO/Extension.pm
index 6e04c2637..67f7cc160 100644
--- a/extensions/BMO/Extension.pm
+++ b/extensions/BMO/Extension.pm
@@ -1089,13 +1089,21 @@ sub install_before_final_checks {
my ($self, $args) = @_;
# Add product chooser setting
- add_setting('product_chooser',
- ['pretty_product_chooser', 'full_product_chooser'],
- 'pretty_product_chooser');
+ add_setting({
+ name => 'product_chooser',
+ options => ['pretty_product_chooser', 'full_product_chooser'],
+ default => 'pretty_product_chooser',
+ category => 'User Interface'
+ });
# Add option to inject x-bugzilla headers into the message body to work
# around gmail filtering limitations
- add_setting('headers_in_body', ['on', 'off'], 'off');
+ add_setting({
+ name => 'headers_in_body',
+ options => ['on', 'off'],
+ default => 'off',
+ category => 'Email Notifications'
+ });
# Migrate from 'gmail_threading' setting to 'bugmail_new_prefix'
my $dbh = Bugzilla->dbh;
diff --git a/extensions/BugModal/Extension.pm b/extensions/BugModal/Extension.pm
index 2c60ef35a..e929f3fcb 100644
--- a/extensions/BugModal/Extension.pm
+++ b/extensions/BugModal/Extension.pm
@@ -292,8 +292,18 @@ sub webservice {
sub install_before_final_checks {
my ($self, $args) = @_;
- add_setting('ui_experiments', ['on', 'off'], 'off');
- add_setting('ui_remember_collapsed', ['on', 'off'], 'off');
+ add_setting({
+ name => 'ui_experiments',
+ options => ['on', 'off'],
+ default => 'off',
+ category => 'User Interface'
+ });
+ add_setting({
+ name => 'ui_remember_collapsed',
+ options => ['on', 'off'],
+ default => 'off',
+ category => 'User Interface'
+ });
# ensure the correct skin is being used
my $dbh = Bugzilla->dbh;
diff --git a/extensions/Example/Extension.pm b/extensions/Example/Extension.pm
index 5b76935e3..4fd2d987f 100644
--- a/extensions/Example/Extension.pm
+++ b/extensions/Example/Extension.pm
@@ -543,10 +543,11 @@ sub install_before_final_checks {
# Add a new user setting like this:
#
- # add_setting('product_chooser', # setting name
- # ['pretty', 'full', 'small'], # options
- # 'pretty'); # default
- #
+ # add_setting({
+ # name => 'product_chooser', # setting name
+ # options => ['pretty', 'full', 'small'], # options
+ # category => 'pretty' # default
+ # });
# To add descriptions for the setting and choices, add extra values to
# the hash defined in global/setting-descs.none.tmpl. Do this in a hook:
# hook/global/setting-descs-settings.none.tmpl .
diff --git a/extensions/Gravatar/Extension.pm b/extensions/Gravatar/Extension.pm
index 3338790e7..06e98fb78 100644
--- a/extensions/Gravatar/Extension.pm
+++ b/extensions/Gravatar/Extension.pm
@@ -38,8 +38,18 @@ sub _user_gravatar {
sub install_before_final_checks {
my ($self, $args) = @_;
- add_setting('show_gravatars', ['On', 'Off'], 'Off');
- add_setting('show_my_gravatar', ['On', 'Off'], 'On');
+ add_setting({
+ name => 'show_gravatars',
+ options => ['On', 'Off'],
+ default => 'Off',
+ category => 'Bug Editing'
+ });
+ add_setting({
+ name => 'show_my_gravatar',
+ options => ['On', 'Off'],
+ default => 'On',
+ category => 'Bug Editing'
+ });
}
__PACKAGE__->NAME;
diff --git a/extensions/InlineHistory/Extension.pm b/extensions/InlineHistory/Extension.pm
index 86536719f..d563bede8 100644
--- a/extensions/InlineHistory/Extension.pm
+++ b/extensions/InlineHistory/Extension.pm
@@ -231,7 +231,12 @@ sub _add_duplicates {
sub install_before_final_checks {
my ($self, $args) = @_;
- add_setting('inline_history', ['on', 'off'], 'off');
+ add_setting({
+ name => 'inline_history',
+ options => ['on', 'off'],
+ default => 'off',
+ category => 'Bug Editing'
+ });
}
__PACKAGE__->NAME;
diff --git a/extensions/Needinfo/Extension.pm b/extensions/Needinfo/Extension.pm
index 7d94fb9a4..c1659a1eb 100644
--- a/extensions/Needinfo/Extension.pm
+++ b/extensions/Needinfo/Extension.pm
@@ -58,7 +58,12 @@ sub install_update_db {
sub install_before_final_checks {
my ($self, $args) = @_;
- add_setting('block_needinfo', ['on', 'off'], 'off');
+ add_setting({
+ name => 'block_needinfo',
+ options => ['on', 'off'],
+ default => 'off',
+ category => 'Reviews and Needinfo'
+ });
}
# Clear the needinfo? flag if comment is being given by
diff --git a/extensions/OrangeFactor/Extension.pm b/extensions/OrangeFactor/Extension.pm
index af629e323..14d500ba0 100644
--- a/extensions/OrangeFactor/Extension.pm
+++ b/extensions/OrangeFactor/Extension.pm
@@ -37,7 +37,12 @@ sub template_before_process {
sub install_before_final_checks {
my ($self, $args) = @_;
- add_setting('orange_factor', ['on', 'off'], 'off');
+ add_setting({
+ name => 'orange_factor',
+ options => ['on', 'off'],
+ default => 'off',
+ category => 'User Interface'
+ });
}
__PACKAGE__->NAME;
diff --git a/extensions/RequestNagger/Extension.pm b/extensions/RequestNagger/Extension.pm
index 169f76b1e..63d79040e 100644
--- a/extensions/RequestNagger/Extension.pm
+++ b/extensions/RequestNagger/Extension.pm
@@ -379,7 +379,12 @@ sub install_filesystem {
sub install_before_final_checks {
my ($self, $args) = @_;
- add_setting('request_nagging', ['on', 'off'], 'on');
+ add_setting({
+ name => 'request_nagging',
+ options => ['on', 'off'],
+ default => 'on',
+ category => 'Reviews and Needinfo'
+ });
}
__PACKAGE__->NAME;
diff --git a/extensions/Review/Extension.pm b/extensions/Review/Extension.pm
index 84ab3e77b..4fd965b4c 100644
--- a/extensions/Review/Extension.pm
+++ b/extensions/Review/Extension.pm
@@ -1041,7 +1041,12 @@ sub install_filesystem {
sub install_before_final_checks {
my ($self, $args) = @_;
- add_setting('block_reviews', ['on', 'off'], 'off');
+ add_setting({
+ name => 'block_reviews',
+ options => ['on', 'off'],
+ default => 'off',
+ category => 'Reviews and Needinfo'
+ });
}
sub config_modify_panels {
diff --git a/skins/standard/admin.css b/skins/standard/admin.css
index 6a91965e4..2b56c1831 100644
--- a/skins/standard/admin.css
+++ b/skins/standard/admin.css
@@ -231,6 +231,16 @@ input[disabled] {
background: #fff;
}
+.category_header {
+ text-align: left;
+ font-weight: bold;
+ font-size: larger;
+}
+
+.setting_label {
+ text-align: right;
+}
+
/* mfa */
#mfa {
diff --git a/template/en/default/account/prefs/settings.html.tmpl b/template/en/default/account/prefs/settings.html.tmpl
index 0147f95ef..b09d7a491 100644
--- a/template/en/default/account/prefs/settings.html.tmpl
+++ b/template/en/default/account/prefs/settings.html.tmpl
@@ -16,9 +16,9 @@
#%]
[%# INTERFACE:
- # setting_names: an array of strings
- # settings: a hash of hashes, keyed by setting_name.
- # Each hash contains:
+ # settings: a hash of hashes, keyed by category ame.
+ # Each hash value is a list of hashes containing:
+ # name - string (name of the setting)
# is_enabled - boolean
# default_value - string (global default for this setting)
# value - string (user-defined preference)
@@ -29,7 +29,9 @@
[% PROCESS "global/setting-descs.none.tmpl" %]
-[% IF settings.size %]
+[% SET category_names = settings.keys.sort %]
+
+[% IF category_names.size %]
[% UNLESS has_settings_enabled %]
<p class="criticalmessages">
All user preferences have been disabled by the
@@ -39,49 +41,48 @@
[% END %]
<table border="0" cellpadding="8">
- [% FOREACH name = setting_names %]
- [% default_name = name _ '-isdefault' %]
- [% default_val = settings.${name}.default_value %]
- <tr id="[% name FILTER html %]_row">
- <td align="right">
- [% setting_descs.$name OR name FILTER html %]
+ [% FOREACH category = category_names %]
+ <tr>
+ <td class="category_header">
+ [% category FILTER html %]
+ </td>
+ </tr>
+ [% FOREACH setting = settings.$category %]
+ [% setting_name = setting._setting_name %]
+ [% default_name = setting_name _ '-isdefault' %]
+ [% default_val = setting.default_value %]
+ <tr id="[% setting_name FILTER html %]_row">
+ <td class="setting_label">
+ [% setting_descs.$setting_name OR setting_name FILTER html %]
</td>
- <td>
- [% IF settings.${name}.is_enabled %]
- <select name="[% name FILTER html %]" id="[% name FILTER html %]">
- <option value="[% default_name FILTER html %]"
- [% ' selected="selected"' IF settings.${name}.is_default %]>
- Site Default ([% setting_descs.${default_val} OR default_val FILTER html %])
+ <td class="setting_choice">
+ <select name="[% setting_name FILTER html %]" id="[% setting_name FILTER html %]">
+ <option value="[% default_name FILTER html %]"
+ [% ' selected="selected"' IF setting.is_default %]>
+ Site Default ([% setting_descs.${default_val} OR default_val FILTER html %])
+ </option>
+ [% FOREACH x = setting.legal_values %]
+ <option value="[% x FILTER html %]"
+ [% ' selected="selected"'
+ IF x == setting.value
+ AND NOT setting.is_default %]>
+ [% setting_descs.${x} OR x FILTER html %]
</option>
- [% FOREACH x = settings.${name}.legal_values %]
- <option value="[% x FILTER html %]"
- [% ' selected="selected"'
- IF x == settings.${name}.value
- AND NOT settings.${name}.is_default %]>
- [% setting_descs.${x} OR x FILTER html %]
- </option>
- [% END %]
- </select>
- [% IF name == "api_key_only" %]
- [% INCLUDE "mfa/protected.html.tmpl" %]
[% END %]
- [% ELSE %]
- <select name="[% name FILTER html %]" id="[% name FILTER html %]" disabled="disabled">
- <option value="[% default_name FILTER html %]">
- Site Default ([% setting_descs.${default_val} OR default_val FILTER html %])
- </option>
- </select>
+ </select>
+ [% IF setting_name == "api_key_only" %]
+ [% INCLUDE "mfa/protected.html.tmpl" %]
[% END %]
</td>
</tr>
[% END %]
+ [% END %]
</table>
[% END %]
-<br>
<script>
-YAHOO.util.Event.onDOMReady(function() {
+$().ready(function() {
var id = document.location.hash.substring(1) + '_row';
- YAHOO.util.Dom.addClass(id, 'highlighted');
+ $('#' + id).addClass('highlighted');
});
</script>
diff --git a/template/en/default/admin/settings/edit.html.tmpl b/template/en/default/admin/settings/edit.html.tmpl
index 7f95f883e..eeb6c3203 100644
--- a/template/en/default/admin/settings/edit.html.tmpl
+++ b/template/en/default/admin/settings/edit.html.tmpl
@@ -51,6 +51,7 @@ page, and the Default Value will automatically apply to everyone.
<table border="1" cellpadding="4">
<tr>
<th>Preference Text</th>
+ <th>Category</th>
<th>Default Value</th>
<th>Enabled</th>
</tr>
@@ -61,6 +62,9 @@ page, and the Default Value will automatically apply to everyone.
<td align="right">
[% setting_descs.$name OR name FILTER html %]
</td>
+ <td align="left">
+ [% settings.$name.category FILTER html %]
+ </td>
<td>
<select name="[% name FILTER html %]" id="[% name FILTER html %]">
[% FOREACH x = settings.${name}.legal_values %]
diff --git a/userprefs.cgi b/userprefs.cgi
index bd1bb8ab7..3f5d2038c 100755
--- a/userprefs.cgi
+++ b/userprefs.cgi
@@ -228,21 +228,20 @@ sub DisableAccount {
sub DoSettings {
my $user = Bugzilla->user;
- my $settings = $user->settings;
- $vars->{'settings'} = $settings;
-
- my @setting_list = sort keys %$settings;
- $vars->{'setting_names'} = \@setting_list;
-
- $vars->{'has_settings_enabled'} = 0;
- # Is there at least one user setting enabled?
- foreach my $setting_name (@setting_list) {
- if ($settings->{"$setting_name"}->{'is_enabled'}) {
- $vars->{'has_settings_enabled'} = 1;
- last;
- }
+ my %settings;
+ my $has_settings_enabled = 0;
+ foreach my $name (sort keys %{ $user->settings }) {
+ my $setting = $user->settings->{$name};
+ next if !$setting->{is_enabled};
+ my $category = $setting->{category};
+ $settings{$category} ||= [];
+ push(@{ $settings{$category} }, $setting);
+ $has_settings_enabled = 1 if $setting->{is_enabled};
}
- $vars->{'dont_show_button'} = !$vars->{'has_settings_enabled'};
+
+ $vars->{settings} = \%settings;
+ $vars->{has_settings_enabled} = $has_settings_enabled;
+ $vars->{dont_show_button} = !$has_settings_enabled;
}
sub SaveSettings {