Bug 512618: Make Bugzilla::Bug::choices return Field::Choice objects, not just values

Patch by Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=LpSolit

3 files changed, 34 insertions, 44 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 567c130f4..98547cd95 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -2108,6 +2108,7 @@ sub set_resolution {
     
     my $old_res = $self->resolution;
     $self->set('resolution', $value);
+    delete $self->{choices};
     my $new_res = $self->resolution;
 
     if ($new_res ne $old_res) {
@@ -2899,38 +2900,38 @@ sub user {
     return $self->{'user'};
 }
 
+# This is intended to get values that can be selected by the user in the
+# UI. It should not be used for security or validation purposes.
 sub choices {
     my $self = shift;
     return $self->{'choices'} if exists $self->{'choices'};
     return {} if $self->{'error'};
+    my $user = Bugzilla->user;
 
-    $self->{'choices'} = {};
-
-    my @prodlist = map {$_->name} @{Bugzilla->user->get_enterable_products};
+    my @products = @{ $user->get_enterable_products };
     # The current product is part of the popup, even if new bugs are no longer
     # allowed for that product
-    if (lsearch(\@prodlist, $self->product) < 0) {
-        push(@prodlist, $self->product);
-        @prodlist = sort @prodlist;
-    }
-
-    # Hack - this array contains "". See bug 106589.
-    my @res = grep ($_, @{get_legal_field_values('resolution')});
-
-    $self->{'choices'} =
-      {
-       'product' => \@prodlist,
-       'rep_platform' => get_legal_field_values('rep_platform'),
-       'priority'     => get_legal_field_values('priority'),
-       'bug_severity' => get_legal_field_values('bug_severity'),
-       'op_sys'       => get_legal_field_values('op_sys'),
-       'bug_status'   => get_legal_field_values('bug_status'),
-       'resolution'   => \@res,
-       'component'    => [map($_->name, @{$self->product_obj->components})],
-       'version'      => [map($_->name, @{$self->product_obj->versions})],
-       'target_milestone' => [map($_->name, @{$self->product_obj->milestones})],
-      };
+    if (!grep($_->name eq $self->product_obj->name, @products)) {
+        unshift(@products, $self->product_obj);
+    }
+
+    my %choices = (
+        product   => \@products,
+        component => $self->product_obj->components,
+        version   => $self->product_obj->versions,
+        target_milestone => $self->product_obj->milestones,
+    );
+
+    my $resolution_field = new Bugzilla::Field({ name => 'resolution' });
+    # Don't include the empty resolution in drop-downs.
+    my @resolutions = grep($_->name, @{ $resolution_field->legal_values });
+    # And don't include MOVED in the list unless the bug is already MOVED.
+    if ($self->resolution ne 'MOVED') {
+        @resolutions= grep { $_->name ne 'MOVED' } @resolutions;
+    }
+    $choices{'resolution'} = \@resolutions;
 
+    $self->{'choices'} = \%choices;
     return $self->{'choices'};
 }
 
diff --git a/template/en/default/bug/edit.html.tmpl b/template/en/default/bug/edit.html.tmpl
index d8a7fa4d7..76ca259e5 100644
--- a/template/en/default/bug/edit.html.tmpl
+++ b/template/en/default/bug/edit.html.tmpl
@@ -375,16 +375,9 @@
     [%#############%]
     
     <tr>
-       [% IF bug.check_can_change_field('product', 0, 1) %]
-         [% prod_list = user.get_enterable_products %]
-         [% IF NOT user.can_enter_product(bug.product) %]
-           [% prod_list.unshift(bug.product_obj) %]
-         [% END %]
-       [% END %]
-
        [% INCLUDE bug/field.html.tmpl
             bug = bug, field = select_fields.product,
-            override_legal_values = prod_list
+            override_legal_values = bug.choices.product
             desc_url = 'describecomponents.cgi', value = bug.product
             editable = bug.check_can_change_field('product', 0, 1) %]
     </tr>
@@ -1112,24 +1105,21 @@
 [%############################################################################%]
 
 [% BLOCK select %]
-  [% IF NOT no_td %]
   <td>
-  [% END %]
-    [% IF bug.check_can_change_field(selname, 0, 1) AND bug.choices.${selname}.size > 1 %]
+    [% IF bug.check_can_change_field(selname, 0, 1) 
+          AND bug.choices.${selname}.size > 1 %]
       <select id="[% selname %]" name="[% selname %]">
         [% FOREACH x = bug.choices.${selname} %]
-          <option value="[% x FILTER html %]"
-            [% " selected" IF x == bug.${selname} %]>[% x FILTER html %]
+          <option value="[% x.name FILTER html %]"
+            [% " selected" IF x.name == bug.${selname} %]>
+            [%- x.name FILTER html %]
           </option>
         [% END %]
       </select>
     [% ELSE %]
       [% bug.${selname} FILTER html %]
     [% END %]
-  [% IF NOT no_td %]
   </td>
-  [% END %]
-  [% no_td = 0 %]
 [% END %]
 
 [%############################################################################%]
diff --git a/template/en/default/bug/knob.html.tmpl b/template/en/default/bug/knob.html.tmpl
index cd586fceb..10d58e51b 100644
--- a/template/en/default/bug/knob.html.tmpl
+++ b/template/en/default/bug/knob.html.tmpl
@@ -147,10 +147,9 @@
 [% BLOCK select_resolution %]
   <select name="resolution" id="resolution">
     [% FOREACH r = bug.choices.resolution %]
-      [% NEXT IF r == "MOVED" && bug.resolution != "MOVED" %]
-      <option value="[% r FILTER html %]"
-      [% "selected" IF r == bug.resolution %]>
-        [% display_value("resolution", r) FILTER html %]</option>
+      <option value="[% r.name FILTER html %]"
+      [% ' selected="selected"' IF r.name == bug.resolution %]>
+        [% display_value("resolution", r.name) FILTER html %]</option>
     [% END %]
   </select>
 [% END %]