Use editvisibility.cgi to enable/disable dependent values, not dependent fields

Bugzilla/Field.pm

@@ -634,6 +634,21 @@ sub has_visibility_value
     return $hash && $hash->{$value};
 }
 
+# Check if a value is enabled for some controlling value
+sub is_value_enabled
+{
+    my $self = shift;
+    my ($value, $visibility_value) = @_;
+    return 1 if !$self->value_field_id;
+    ref $_ and $_ = $_->id for $value, $visibility_value;
+    my $hash = Bugzilla->fieldvaluecontrol
+        ->{$self->value_field_id}
+        ->{values}
+        ->{$self->id}
+        ->{$value};
+    return $hash && $hash->{$visibility_value};
+}
+
 # Check visibility of field for a bug or for a hashref with default value names
 sub check_visibility
 {
@@ -1076,6 +1091,7 @@ sub update_controlled_values
     my $self = shift;
     my ($controlled_value_ids, $visibility_value_id) = @_;
     $controlled_value_ids ||= [];
+    $controlled_value_ids = [ $controlled_value_ids ] if !ref $controlled_value_ids;
     my $vis_field = $self->value_field;
     if (!$vis_field)
     {

editvalues.cgi

@@ -14,10 +14,6 @@
 # Contributor(s): Max Kanat-Alexander <mkanat@bugzilla.org>
 #                 Frédéric Buclin <LpSolit@gmail.com>
 
-# This is a script to edit the values of fields that have drop-down
-# or select boxes. It is largely a copy of editmilestones.cgi, but 
-# with some cleanup.
-
 use strict;
 use lib qw(. lib);
 
@@ -38,10 +34,6 @@ my $template = Bugzilla->template;
 my $ARGS     = $cgi->VarHash;
 my $vars     = {};
 
-# Replace this entry by separate entries in templates when
-# the documentation about legal values becomes bigger.
-$vars->{doc_section} = 'edit-values.html';
-
 Bugzilla->user->in_group('editvalues')
 || $ARGS->{field} eq 'keywords' && Bugzilla->user->in_group('editkeywords')
 || ThrowUserError('auth_failure', {
@@ -57,8 +49,7 @@ my $action = trim($ARGS->{action} || '');
 my $token  = $ARGS->{token};
 
 # Fields listed here must not be edited from this interface.
-my @non_editable_fields = qw(product);
+my %block_list = map { $_ => 1 } qw(product);
-my %block_list = map { $_ => 1 } @non_editable_fields;
 
 #
 # field = '' -> Show nice list of fields
@@ -119,57 +110,6 @@ if ($action eq 'new')
     display_field_values($vars);
 }
 
-#
-# action='control_list' -> enable/disable values controlled by this one
-#
-if ($action eq 'control_list')
-{
-    die('This field has no value field') unless $field->custom && $field->value_field;
-
-    my $step = $ARGS->{step} || 0;
-    my $visibility_value_id = $ARGS->{visibility_value_id};
-    if ($visibility_value_id)
-    {
-        $visibility_value_id = $field->value_field->value_type->new($visibility_value_id)->{id};
-    }
-
-    my $values = $ARGS->{values};
-    my $need_token = 0;
-
-    $vars->{visibility_value_id} = -1;
-    if ($visibility_value_id)
-    {
-        $vars->{visibility_value_id} = $visibility_value_id;
-        $vars->{default_value_hash} = $field->default_value_hash_for($visibility_value_id);
-        my %values = map { $_->{id} => $_ } @{$field->value_field->legal_values};
-        $vars->{field_value} = $values{$visibility_value_id};
-        $step++ unless $token;
-        $need_token = 1;
-        if ($token)
-        {
-            check_token_data($token, "edit_control_list");
-            $field->update_controlled_values($values, $visibility_value_id);
-            if ($field->default_field_id == $field->value_field_id)
-            {
-                $field->update_default_value($visibility_value_id, $field->type == FIELD_TYPE_MULTI_SELECT
-                    ? [ $cgi->param('default_value') ]
-                    : scalar $cgi->param('default_value'));
-            }
-            $step++;
-            $need_token = 0;
-            delete_token($token);
-        }
-    }
-
-    $vars->{step} = $step;
-    $vars->{token} = issue_session_token("edit_control_list") if $need_token;
-
-    $template->process("admin/fieldvalues/control-list.html.tmpl", $vars)
-        || ThrowTemplateError($template->error());
-
-    exit;
-}
-
 # After this, we always have a value
 my $value = $field->value_type->check(exists $ARGS->{value_old} ? $ARGS->{value_old} : $ARGS->{value});
 $vars->{value} = $value;

editvisibility.cgi

@@ -1,5 +1,5 @@
 #!/usr/bin/perl -wT
-# Enable/disable custom fields for a value of the controlling field
+# Enable/disable field values for a selected value of the controlling field
 # License: Dual-license GPL 3.0+ or MPL 1.1+
 # Contributor(s): Vitaliy Filippov <vitalif@mail.ru>, Vladimir Koptev <vladimir.koptev@gmail.com>
 
@@ -20,82 +20,36 @@ my $template = Bugzilla->template;
 my $vars = {};
 
 my $user = Bugzilla->login(LOGIN_REQUIRED);
-$user->in_group('editfields') || ThrowUserError('auth_failure', {
+$user->in_group('editvalues') || ThrowUserError('auth_failure', {
-    group  => "admin",
+    group  => 'admin',
-    action => "edit",
+    action => 'edit',
-    object => "custom_fields"});
+    object => 'fieldvalues'
+});
 
-my $field_name = trim($ARGS->{field} || '');
+my $deny_edit = { version => 1, target_milestone => 1, component => 1, product => 1 };
-my $value_name = trim($ARGS->{value} || '');
+my $field = Bugzilla->get_field($ARGS->{field});
-my $action     = trim($ARGS->{action} || '');
+ThrowUserError('fieldname_invalid', { field => { name => $ARGS->{field} } }) if !$field || $deny_edit->{$field->name};
-my $token      = $ARGS->{token};
+ThrowUserError('no_value_field', { field => $field }) unless $field->value_field;
 
-unless ($field_name)
+my $value = $field->value_field->value_type->check({ id => $ARGS->{visibility_value_id} });
+
+if ($ARGS->{action} eq 'save')
 {
-    ThrowUserError('no_valid_field', { field => 'field' });
+    check_token_data($ARGS->{token}, 'edit_visibility');
-}
+    $field->update_controlled_values($ARGS->{values}, $value->id);
-
+    delete_token($ARGS->{token});
-my $field = Bugzilla->get_field($field_name);
+    Bugzilla->add_result_message({ message => 'visibility_updated' });
-ThrowUserError('no_valid_field', { field => 'field' }) unless $field;
+    Bugzilla->save_session_data;
-
+    print Bugzilla->cgi->redirect('editvisibility.cgi?field='.$field->name.'&visibility_value_id='.$value->id);
-my $value = $field->value_type->check($value_name);
-ThrowUserError('no_valid_value', { field => 'value' }) unless $value;
-
-if ($field->name eq 'product')
-{
-    $user->check_can_admin_product($value->name);
-}
-
-#
-# action='' -> Show list of custom fields
-#
-unless ($action)
-{
-    $vars->{field} = $field;
-    $vars->{value} = $value;
-    $vars->{token} = issue_session_token('change_visibility');
-    $template->process('admin/custom_fields/visibility-list.html.tmpl', $vars)
-        || ThrowTemplateError($template->error);
     exit;
 }
 
-# update result page
+$vars->{field} = $field;
-if ($action eq 'update' && $token)
+$vars->{visibility_value} = $value;
-{
+$vars->{token} = issue_session_token('edit_visibility');
-    check_token_data($token, 'change_visibility');
 
-    my (@updated, @cleared);
+$template->process('admin/fieldvalues/control-list.html.tmpl', $vars)
-    for my $cfield (@{$field->controls_visibility_of})
+    || ThrowTemplateError($template->error());
-    {
+exit;
-        # check if it is 'visible for all'
-        my $visibility_values = $cfield->visibility_values || next;
-        # check if changed
-        next unless ($ARGS->{'visible_'.$cfield->id} xor $visibility_values->{$value->id});
-        if ($ARGS->{'visible_'.$cfield->id})
-        {
-            $visibility_values->{$value->id} = 1;
-            push @updated, $cfield->description;
-        }
-        elsif ($visibility_values->{$value->id})
-        {
-            delete $visibility_values->{$value->id};
-            push @cleared, $cfield->description;
-        }
-        $cfield->set_visibility_values([ keys %$visibility_values ]);
-    }
-
-    delete_token($token);
-
-    $vars->{field} = $field;
-    $vars->{value} = $value;
-    $vars->{update} = 1;
-    $vars->{updated} = join(', ', @updated);
-    $vars->{cleared} = join(', ', @cleared);
-    $template->process('admin/custom_fields/visibility-list.html.tmpl', $vars)
-        || ThrowTemplateError($template->error());
-    exit;
-}
-
-ThrowUserError('no_valid_action', { field => 'action'});
 
 __END__

template/en/default/admin/custom_fields/visibility-list.html.tmpl

@@ -1,60 +0,0 @@
-[%# Template for editvisibility.cgi
-  # License: Dual-license GPL 3.0+ or MPL 1.1+
-  # Contributor(s): Vitaliy Filippov <vitalif@mail.ru>, Vladimir Koptev <vladimir.koptev@gmail.com>
-  # %]
-
-[% PROCESS global/header.html.tmpl
-    title = "Выбор активных полей при установке значения поля $field.description в $value.name"
-    doc_section = "visibility-list.html"
-%]
-
-[% IF update %]
-    [% IF updated %]
-    <p>Следующие поля будут показаны для значения [% value.name | html %] поля [% field.description | html %]: <strong>[% updated | html %]</strong></p>
-    [% END %]
-    [% IF cleared %]
-    <p>Следующие поля будут скрыты для значения [% value.name | html %] поля [% field.description | html %]: <strong>[% cleared | html %]</strong></p>
-    [% END %]
-    [% IF !cleared && !updated %]
-    <p>Ничего не изменено</p>
-    [% END %]
-    <p><a href="editvisibility.cgi?field=[% field.name | url_quote %]&value=[% value.name | url_quote %]">К списку контролируемых полей</a></p>
-[% ELSE %]
-<h3>Выберите поля, которые будут отображаться, когда значение поля [% field.description | html %] установлено в [% value.name | html %]:</h3>
-
-<form action="editvisibility.cgi?field=[% field.name | url_quote %]&value=[% value.name | url_quote %]&action=update" method="POST">
-    <input type="hidden" name="token" value="[% token | html %]" />
-    <table id="admin_table" cellspacing="0" cellpadding="4">
-        <tr>
-            <th align="left">Поле</th>
-            <th align="left" colspan="2">Видимо ли</th>
-        </tr>
-        [% i = 0 %]
-        [% FOREACH cfield = Bugzilla.get_fields({ visibility_field_id => field.id }) %]
-        <tr class="[% IF i mod 2 == 0 %]odd[% ELSE %]even[% END %]_row">
-            <td>[% cfield.description | html %]</td>
-            <td align="center">
-                <input type="checkbox" name="visible_[% cfield.id %]" value="1"
-                    [% "checked=\"checked\"" IF !cfield.visibility_values || cfield.visibility_values.${value.id} %]
-                    [% "disabled=\"disabled\"" IF !cfield.visibility_values %] />
-            </td>
-            <td>
-            [% IF !cfield.visibility_values %]
-                Поле видимо для всех значений поля <strong>[% field.description | html %]</strong><br/>
-                <a href="editfields.cgi?action=edit&name=[% cfield.name | url_quote %]">Редактировать</a>
-            [% END %]
-            </td>
-        </tr>
-        [% i = i + 1 %]
-        [% END %]
-    </table>
-    <p><input type="submit" name="save" value="Сохранить" /></p>
-</form>
-<p>
-    <a href="[% IF field.name == "product" %]editproducts.cgi[% ELSE %]editvalues.cgi?field=[% field.name | url_quote %][% END %]">
-        К списку значений поля [% field.description | html %]
-    </a>
-</p>
-[% END %]
-
-[% PROCESS global/footer.html.tmpl %]

template/en/default/admin/fieldvalues/control-list-common.html.tmpl

@@ -28,7 +28,7 @@
 
 [% SET any_field = 0 %]
 [% FOR f = Bugzilla.get_fields({ obsolete => 0, value_field_id => this_field.id, sort => 1 }) %]
-  [% IF f.name != 'version' && f.name != 'target_milestone' && f.name != 'component' &&
+  [% IF f.name != 'version' && f.name != 'target_milestone' && f.name != 'component' && f.name != 'product' &&
     (f.visibility_field_id != f.value_field_id || f.has_visibility_value(this_value.id)) %]
     [% IF NOT any_field %]
     <tr>
@@ -40,7 +40,7 @@
       [% SET any_field = 1 %]
     [% ELSE %],
     [% END %]
-    <a href="editvalues.cgi?action=control_list&field=[% f.name | html %]&visibility_value_id=[% this_value.id %]">[% f.description | html %]</a>
+    <a href="editvisibility.cgi?field=[% f.name | html %]&visibility_value_id=[% this_value.id %]">[% f.description | html %]</a>
   [% END %]
 [% END %]
 [% IF any_field %]

template/en/default/admin/fieldvalues/control-list.html.tmpl

@@ -1,91 +1,56 @@
-[% IF step == 0;
+[%# Enable/disable field values for a selected value of its controlling value_field
-     SET title = "Выбор активных " _ field.description _ " для различных значений " _ field.value_field.description;
+  # License: Dual-license GPL 3.0+ or MPL 1.1+
-   ELSIF step == 1;
+  # Authors: Vitaliy Filippov <vitalif@mail.ru>, Vladimir Koptev <vladimir.koptev@gmail.com>
-     SET title = "Значения " _ field.description _ " для " _ field_value.name;
+  #%]
-   ELSIF step == 2;
+
-     SET title = "Значения " _ field.description _ " для " _ field_value.name _ " сохранены.";
+[% SET title = "Select Active " _ field.description _ "s For " _ field.value_field.description _ ' ' _ visibility_value.name | html %]
-   END;
-%]
 
 [% PROCESS global/header.html.tmpl %]
 
-<h3>Выбор активных [% field.description FILTER html %] для различных значений поля [% field.value_field.description FILTER html %]</h3>
+<h3>[% title %]</h3>
 
-<form action="editvalues.cgi?action=control_list&field=[% field.name FILTER html %][% '&visibility_value_id=' _ visibility_value_id IF visibility_value_id %]"
+<form action="editvisibility.cgi?field=[% field.name | html %]&visibility_value_id=[% visibility_value.id %]" method="POST">
-      method="[% IF step > 0 %]POST[% ELSE %]GET[% END %]"
+  <input type="hidden" name="token" value="[% token | html %]" />
-      name="setValuesForm">
+  <input type="hidden" name="action" value="save" />
-  [% IF token %]<input type="hidden" name="token" value="[% token %]"/>[% END %]
+  <table id="admin_table" cellpadding="4">
-  [% IF step == 0 %]
+    <tbody>
-    <input type="hidden" name="action" value="control_list"/>
+      <tr>
-    <input type="hidden" name="field" value="[% field.name FILTER html %]"/>
+        <th align="left">Active</th>
-    <p>
+        <th align="left">Value</th>
-      <strong>Выберите [% field.value_field.description FILTER html %]:&nbsp;</strong>
+      </tr>
-      <select name="visibility_value_id" id="visibility_value_id">
+      [% FOREACH value = field.legal_values %]
-        [% FOREACH field_value = field.value_field.legal_values %]
+      <tr class="[% loop.index % 2 == 1 ? 'odd' : 'even' +%]_row">
-          [% NEXT IF field_value.name == '' %]
+        <td align="center">
-            <option value="[% field_value.id FILTER none %]"[% ' selected="selected"' IF field_value.id == visibility_value_id %]>
+          <input type="checkbox" id="val_[% value.id %]" value="[% value.id %]" name="values"
-              [%- field_value.name FILTER html -%]
+            [% ' checked="checked"' IF field.is_value_enabled(value.id, visibility_value.id) %]/>
-            </option>
+        </td>
-        [% END %]
+        <td>
-      </select>
+          <label for="val_[% value.id %]">[% value.name | html %]</label>
-      <input type="submit" id="update" value="Далее"/>
+          <a title="Edit this value" href="editvalues.cgi?action=edit&field=[% field.name %]&value=[% value.name | uri %]">&rarr;</a>
-    </p>
+        </td>
-  [% ELSIF step == 1 %]
+      </tr>
-    <input type="hidden" name="step" value="[% step FILTER html %]"/>
-    <input type="hidden" name="visibility_value_id" value="[% visibility_value_id FILTER html %]"/>
-    <p>Значения <strong>[% field.description FILTER html %]</strong> для <strong>[% field_value.name FILTER html %]</strong>:</p>
-
-    [% IF field.default_field_id == field.value_field_id %]
-      <p>Значение по умолчанию:[% "<br />" IF field.type == constants.FIELD_TYPE_MULTI_SELECT %]
-      [% IF field.is_select %]
-      <select name="default_value" style="width: 400px" [% "multiple size=3" IF field.type == constants.FIELD_TYPE_MULTI_SELECT %]>
-        <option value="">---</option>
-        [% FOREACH v = field.legal_values %]
-        <option value="[% v.id | html %]"[% " selected" IF default_value_hash.${v.id} %]>[% v.name | html %]</option>
-        [% END %]
-      </select>
-      </p>
-      [% ELSE %]
-      <input type="text" name="default_value" value="[% field.default_value | html %]" style="width: 400px" />
       [% END %]
-    [% END %]
+    </tbody>
-
+  </table>
-    <table id="admin_table" cellpadding="4">
+  <br />
-      <tbody>
+  <input type="submit" value="Save" />
-        <tr>
-          <th align="left">Значение</th>
-          <th align="left">Активно</th>
-        </tr>
-        [% count = 0 %]
-        [% FOREACH value = field.legal_values %]
-          <tr class="[%+ count % 2 == 1 ? 'odd' : 'even' +%]_row">
-            <td>
-              <label for="val_[% value.id %]">[% value.name FILTER html %]</label>
-              <a title="Править значение..." href="editvalues.cgi?action=edit&field=[% field.name %]&value=[% value.name | uri %]">&rarr;</a>
-            </td>
-            <td align="center">
-              <input type="checkbox" id="val_[% value.id %]" value="[% value.id %]" name="values[]" onchange="changeDefaultEnabling(this);"[% ' checked="checked"' IF value.has_visibility_value(field_value.id) %]/>
-            </td>
-          </tr>
-          [% count = count + 1 %]
-        [% END %]
-      </tbody>
-    </table>
-    <br/>
-    <input type="submit" id="update" name="update" value="Сохранить"/>
-    <a href="editvalues.cgi?action=control_list&field=[% field.name FILTER html %]">К выбору <strong>[% field.value_field.description FILTER html %]</strong></a>
-    |
-  [% ELSIF step == 2 %]
-    <p>Значения <strong>[% field.description FILTER html %]</strong> для <strong>[% field_value.name FILTER html %]</strong> сохранены.</p>
-    <a href="editvalues.cgi?action=control_list&field=
-      [%- field.name FILTER url_quote %]&visibility_value_id=[% visibility_value_id %]">К выбору значений <strong>[% field.description FILTER html %]</strong> для <strong>[% field_value.name FILTER html %]</strong></a>
-    |
-    <a href="editvalues.cgi?action=control_list&field=
-      [%- field.name FILTER url_quote %]">К выбору <strong>[% field.value_field.description FILTER html %]</strong></a>
-    |
-  [% END; %]
-  <a href="editvalues.cgi?field=
-    [%- field.name FILTER url_quote %]">К списку значений <strong>[% field.description FILTER html %]</strong></a>
 </form>
 
+<p>
+  <a href="editvalues.cgi?field=[% field.name %]">Edit or add [% field.description | html %]s</a> |
+  [% IF field.value_field.name == 'product' %]
+  <a href="editproducts.cgi?action=edit&product=[% visibility_value.name | uri %]">Edit product [% visibility_value.name | html %]</a>
+  [% ELSIF field.value_field.name == 'classification' %]
+  <a href="editclassifications.cgi?action=edit&classification=[% visibility_value.name | uri %]">Edit classification [% visibility_value.name | html %]</a>
+  [% ELSIF field.value_field.name == 'target_milestone' %]
+  <a href="editmilestones.cgi?action=edit&product=[% visibility_value.product_obj.name | uri %]&milestone=[% visibility_value.name | uri %]">Edit milestone [% visibility_value.name | html %]</a>
+  [% ELSIF field.value_field.name == 'version' %]
+  <a href="editversions.cgi?action=edit&product=[% visibility_value.product_obj.name | uri %]&version=[% visibility_value.name | uri %]">Edit version [% visibility_value.name | html %]</a>
+  [% ELSIF field.value_field.name == 'component' %]
+  <a href="editcomponents.cgi?action=edit&product=[% visibility_value.product_obj.name | uri %]&component=[% visibility_value.name | uri %]">Edit component [% visibility_value.name | html %]</a>
+  [% ELSE %]
+  <a href="editvalues.cgi?action=edit&field=[% field.value_field.name %]&value=[% visibility_value.name | uri %]">Edit [% field.value_field.description | html %] [% visibility_value.name | html %]</a>
+  [% END %]
+</p>
+
 [% PROCESS global/footer.html.tmpl %]

template/en/default/global/messages.html.tmpl

@@ -101,6 +101,10 @@
   [% END %]
 [% END %]
 
+[% BLOCK msg_visibility_updated %]
+  <div class="message">Active values updated.</div>
+[% END %]
+
 [%# This is a list of all the possible messages. Please keep them in
   # alphabetical order by message tag, and leave a blank line between messages.
   #%]