Bug 348518: grant / reject options for flag listbox should only appear if user…

Bugzilla/Flag.pm

@@ -416,13 +416,10 @@ sub _validate {
             }
 
             # Throw an error if the user won't be allowed to set the flag.
-            if ($flag_type->grant_group
-                && !$requestee->in_group_id($flag_type->grant_group->id))
-            {
-                ThrowUserError('flag_requestee_needs_privs',
-                               {'requestee' => $requestee,
-                                'flagtype'  => $flag_type});
-            }
+            $requestee->can_set_flag($flag_type)
+              || ThrowUserError('flag_requestee_needs_privs',
+                                {'requestee' => $requestee,
+                                 'flagtype'  => $flag_type});
         }
     }
 
@@ -433,12 +430,10 @@ sub _validate {
     # - User in the request_group can clear pending requests and set flags
     #   and can rerequest set flags.
     return if (($status eq 'X' || $status eq '?')
-               && (!$flag_type->request_group
-                   || $user->in_group_id($flag_type->request_group->id)));
+               && $user->can_request_flag($flag_type));
 
     # - User in the grant_group can set/clear flags, including "+" and "-".
-    return if (!$flag_type->grant_group
-               || $user->in_group_id($flag_type->grant_group->id));
+    return if $user->can_set_flag($flag_type);
 
     # - Any other flag modification is denied
     ThrowUserError('flag_update_denied',

Bugzilla/User.pm

@@ -642,6 +642,21 @@ sub get_enterable_products {
     return $self->{enterable_products};
 }
 
+sub can_request_flag {
+    my ($self, $flag_type) = @_;
+
+    return ($self->can_set_flag($flag_type)
+            || !$flag_type->request_group
+            || $self->in_group_id($flag_type->request_group->id)) ? 1 : 0;
+}
+
+sub can_set_flag {
+    my ($self, $flag_type) = @_;
+
+    return (!$flag_type->grant_group
+            || $self->in_group_id($flag_type->grant_group->id)) ? 1 : 0;
+}
+
 # visible_groups_inherited returns a reference to a list of all the groups
 # whose members are visible to this user.
 sub visible_groups_inherited {
@@ -1741,6 +1756,24 @@ method should be called in such a case to force reresolution of these groups.
 
  Returns:     an array of product objects.
 
+=item C<can_request_flag($flag_type)>
+
+ Description: Checks whether the user can request flags of the given type.
+
+ Params:      $flag_type - a Bugzilla::FlagType object.
+
+ Returns:     1 if the user can request flags of the given type,
+              0 otherwise.
+
+=item C<can_set_flag($flag_type)>
+
+ Description: Checks whether the user can set flags of the given type.
+
+ Params:      $flag_type - a Bugzilla::FlagType object.
+
+ Returns:     1 if the user can set flags of the given type,
+              0 otherwise.
+
 =item C<get_userlist>
 
 Returns a reference to an array of users.  The array is populated with hashrefs

template/en/default/flag/list.html.tmpl

@@ -104,11 +104,18 @@
           <select id="flag-[% flag.id %]" name="flag-[% flag.id %]" 
                   title="[% type.description FILTER html %]"
                   onchange="toggleRequesteeField(this);">
-            <option value="X"></option>
+            [%# Only display statuses the user is allowed to set. %]
             [% IF type.is_active %]
-              <option value="+" [% "selected" IF flag.status == "+" %]>+</option>
-              <option value="-" [% "selected" IF flag.status == "-" %]>-</option>
-              [% IF type.is_requestable || flag.status == "?" %]
+              [% IF user.can_request_flag(type) %]
+                <option value="X"></option>
+              [% END %]
+              [% IF user.can_set_flag(type) || flag.status == "+" %]
+                <option value="+" [% "selected" IF flag.status == "+" %]>+</option>
+              [% END %]
+              [% IF user.can_set_flag(type) || flag.status == "-" %]
+                <option value="-" [% "selected" IF flag.status == "-" %]>-</option>
+              [% END %]
+              [% IF (type.is_requestable && user.can_request_flag(type)) || flag.status == "?" %]
                 <option value="?" [% "selected" IF flag.status == "?" %]>?</option>
               [% END %]
             [% ELSE %]
@@ -146,11 +153,14 @@
         <td>
           <select id="flag_type-[% type.id %]" name="flag_type-[% type.id %]" 
                   title="[% type.description FILTER html %]"
+                  [% " disabled=\"disabled\"" UNLESS user.can_request_flag(type) %]
                   onchange="toggleRequesteeField(this);">
             <option value="X"></option>
-            <option value="+">+</option>
-            <option value="-">-</option>
-            [% IF type.is_requestable %]
+            [% IF user.can_set_flag(type) %]
+              <option value="+">+</option>
+              <option value="-">-</option>
+            [% END %]
+            [% IF type.is_requestable && user.can_request_flag(type) %]
               <option value="?">?</option>
             [% END %]
           </select>
@@ -186,11 +196,14 @@
       <td>
         <select id="flag_type-[% type.id %]" name="flag_type-[% type.id %]" 
                 title="[% type.description FILTER html %]"
+                [% " disabled=\"disabled\"" UNLESS user.can_request_flag(type) %]
                 onchange="toggleRequesteeField(this);">
           <option value="X"></option>
-          <option value="+">+</option>
-          <option value="-">-</option>
-          [% IF type.is_requestable %]
+          [% IF user.can_set_flag(type) %]
+            <option value="+">+</option>
+            <option value="-">-</option>
+          [% END %]
+          [% IF type.is_requestable && user.can_request_flag(type) %]
             <option value="?">?</option>
           [% END %]
         </select>