Bug 343810: Remove Bugzilla::FlagType::get() and implement real flagtype objects…

Bugzilla/Flag.pm

@@ -104,9 +104,9 @@ Returns the status '+', '-', '?' of the flag.
 
 =cut
 
-sub id     { return $_[0]->{'id'};         }
-sub name   { return $_[0]->type->{'name'}; }
-sub status { return $_[0]->{'status'};     }
+sub id     { return $_[0]->{'id'};     }
+sub name   { return $_[0]->type->name; }
+sub status { return $_[0]->{'status'}; }
 
 ###############################
 ####       Methods         ####
@@ -118,7 +118,7 @@ sub status { return $_[0]->{'status'};     }
 
 =item C<type>
 
-Returns the type of the flag, as pseudo Bugzilla::FlagType object.
+Returns the type of the flag, as a Bugzilla::FlagType object.
 
 =item C<setter>
 
@@ -136,7 +136,7 @@ Bugzilla::User object.
 sub type {
     my $self = shift;
 
-    $self->{'type'} ||= Bugzilla::FlagType::get($self->{'type_id'});
+    $self->{'type'} ||= new Bugzilla::FlagType($self->{'type_id'});
     return $self->{'type'};
 }
 
@@ -291,7 +291,7 @@ sub validate {
         # as is.
         if ($status eq '?'
             && $flag->status ne '?'
-            && !$flag->type->{is_requestable})
+            && !$flag->type->is_requestable)
         {
             ThrowCodeError("flag_status_invalid", 
                            { id => $id, status => $status });
@@ -302,7 +302,7 @@ sub validate {
         # the flag became specifically unrequestable, don't let the user
         # change the requestee, but let the user remove it by entering
         # an empty string for the requestee.
-        if ($status eq '?' && !$flag->type->{is_requesteeble}) {
+        if ($status eq '?' && !$flag->type->is_requesteeble) {
             my $old_requestee = $flag->requestee ? $flag->requestee->login : '';
             my $new_requestee = join('', @requestees);
             if ($new_requestee && $new_requestee ne $old_requestee) {
@@ -314,7 +314,7 @@ sub validate {
         # Make sure the user didn't enter multiple requestees for a flag
         # that can't be requested from more than one person at a time.
         if ($status eq '?'
-            && !$flag->type->{is_multiplicable}
+            && !$flag->type->is_multiplicable
             && scalar(@requestees) > 1)
         {
             ThrowUserError("flag_not_multiplicable", { type => $flag->type });
@@ -323,7 +323,7 @@ sub validate {
         # Make sure the requestees are authorized to access the bug.
         # (and attachment, if this installation is using the "insider group"
         # feature and the attachment is marked private).
-        if ($status eq '?' && $flag->type->{is_requesteeble}) {
+        if ($status eq '?' && $flag->type->is_requesteeble) {
             my $old_requestee = $flag->requestee ? $flag->requestee->login : '';
             foreach my $login (@requestees) {
                 next if $login eq $old_requestee;
@@ -365,20 +365,19 @@ sub validate {
         # - The flag is unchanged
         next if ($status eq $flag->status);
 
-        # - User in the $request_gid group can clear pending requests and set flags
+        # - User in the request_group can clear pending requests and set flags
         #   and can rerequest set flags.
         next if (($status eq 'X' || $status eq '?')
-                 && (!$flag->type->{request_gid}
-                     || $user->in_group_id($flag->type->{request_gid})));
+                 && (!$flag->type->request_group
+                     || $user->in_group_id($flag->type->request_group->id)));
 
-        # - User in the $grant_gid group can set/clear flags,
-        #   including "+" and "-"
-        next if (!$flag->type->{grant_gid}
-                 || $user->in_group_id($flag->type->{grant_gid}));
+        # - User in the grant_group can set/clear flags, including "+" and "-".
+        next if (!$flag->type->grant_group
+                 || $user->in_group_id($flag->type->grant_group->id));
 
         # - Any other flag modification is denied
         ThrowUserError("flag_update_denied",
-                        { name       => $flag->type->{name},
+                        { name       => $flag->type->name,
                           status     => $status,
                           old_status => $flag->status });
     }
@@ -391,7 +390,7 @@ sub snapshot {
                         'attach_id' => $attach_id });
     my @summaries;
     foreach my $flag (@$flags) {
-        my $summary = $flag->type->{'name'} . $flag->status;
+        my $summary = $flag->type->name . $flag->status;
         $summary .= "(" . $flag->requestee->login . ")" if $flag->requestee;
         push(@summaries, $summary);
     }
@@ -524,7 +523,7 @@ sub create {
     $dbh->do('INSERT INTO flags (type_id, bug_id, attach_id, requestee_id,
                                  setter_id, status, creation_date, modification_date)
               VALUES (?, ?, ?, ?, ?, ?, ?, ?)',
-              undef, ($flag->{'type'}->{'id'}, $bug->bug_id,
+              undef, ($flag->{'type'}->id, $bug->bug_id,
                       $attach_id, $requestee_id, $flag->{'setter'}->id,
                       $flag->{'status'}, $timestamp, $timestamp));
 
@@ -571,6 +570,8 @@ sub modify {
     my @flags;
     foreach my $id (@ids) {
         my $flag = new Bugzilla::Flag($id);
+        # If the flag no longer exists, ignore it.
+        next unless $flag;
 
         my $status = $cgi->param("flag-$id");
 
@@ -582,7 +583,7 @@ sub modify {
         my $requestee_email;
         if ($status eq "?"
             && scalar(@requestees) > 1
-            && $flag->type->{is_multiplicable})
+            && $flag->type->is_multiplicable)
         {
             # The first person, for which we'll reuse the existing flag.
             $requestee_email = shift(@requestees);
@@ -616,7 +617,7 @@ sub modify {
 
         my $requestee_changed = 
           ($status eq "?" && 
-           $flag->type->{'is_requesteeble'} &&
+           $flag->type->is_requesteeble &&
            $old_requestee ne $requestee_email);
 
         next unless ($status_changed || $requestee_changed);
@@ -770,7 +771,7 @@ sub FormToNewFlags {
 
     my @flags;
     foreach my $flag_type (@$flag_types) {
-        my $type_id = $flag_type->{'id'};
+        my $type_id = $flag_type->id;
 
         # We are only interested in flags the user tries to create.
         next unless scalar(grep { $_ == $type_id } @type_ids);
@@ -784,7 +785,7 @@ sub FormToNewFlags {
 
         # Do not create a new flag of this type if this flag type is
         # not multiplicable and already has a flag set.
-        next if (!$flag_type->{'is_multiplicable'} && $has_flags);
+        next if (!$flag_type->is_multiplicable && $has_flags);
 
         my $status = $cgi->param("flag_type-$type_id");
         trick_taint($status);
@@ -796,7 +797,7 @@ sub FormToNewFlags {
                                 setter    => $setter , 
                                 status    => $status ,
                                 requestee => Bugzilla::User->new_from_login($login) });
-                last if !$flag_type->{'is_multiplicable'};
+                last unless $flag_type->is_multiplicable;
             }
         }
         else {
@@ -829,7 +830,7 @@ sub notify {
     my $template = Bugzilla->template;
 
     # There is nobody to notify.
-    return unless ($flag->{'addressee'} || $flag->type->{'cc_list'});
+    return unless ($flag->{'addressee'} || $flag->type->cc_list);
 
     my $attachment_is_private = $attachment ? $attachment->isprivate : undef;
 
@@ -839,7 +840,7 @@ sub notify {
     # not in those groups or email addresses that don't have an account.
     if ($bug->groups || $attachment_is_private) {
         my @new_cc_list;
-        foreach my $cc (split(/[, ]+/, $flag->type->{'cc_list'})) {
+        foreach my $cc (split(/[, ]+/, $flag->type->cc_list)) {
             my $ccuser = Bugzilla::User->new_from_login($cc) || next;
 
             next if ($bug->groups && !$ccuser->can_see_bug($bug->bug_id));
@@ -852,11 +853,11 @@ sub notify {
     }
 
     # If there is nobody left to notify, return.
-    return unless ($flag->{'addressee'} || $flag->type->{'cc_list'});
+    return unless ($flag->{'addressee'} || $flag->type->cc_list);
 
     # Process and send notification for each recipient
     foreach my $to ($flag->{'addressee'} ? $flag->{'addressee'}->email : '',
-                    split(/[, ]+/, $flag->type->{'cc_list'}))
+                    split(/[, ]+/, $flag->type->cc_list))
     {
         next unless $to;
         my $vars = { 'flag'       => $flag,

Bugzilla/User.pm

@@ -980,7 +980,7 @@ sub match_field {
                 elsif ($field_name =~ /^requestee_type-(\d+)$/) {
                     require Bugzilla::FlagType;
                     $expanded_fields->{$field_name}->{'flag_type'} = 
-                      Bugzilla::FlagType::get($1);
+                      new Bugzilla::FlagType($1);
                 }
             }
         }

attachment.cgi

@@ -713,8 +713,7 @@ sub enter
                                               'product_id'   => $product_id,
                                               'component_id' => $component_id});
   $vars->{'flag_types'} = $flag_types;
-  $vars->{'any_flags_requesteeble'} = grep($_->{'is_requesteeble'},
-                                           @$flag_types);
+  $vars->{'any_flags_requesteeble'} = grep($_->is_requesteeble, @$flag_types);
 
   print $cgi->header();
 
@@ -835,11 +834,11 @@ sub edit {
                                                'product_id'   => $product_id ,
                                                'component_id' => $component_id });
   foreach my $flag_type (@$flag_types) {
-    $flag_type->{'flags'} = Bugzilla::Flag::match({ 'type_id'   => $flag_type->{'id'},
+    $flag_type->{'flags'} = Bugzilla::Flag::match({ 'type_id'   => $flag_type->id,
                                                     'attach_id' => $attachment->id });
   }
   $vars->{'flag_types'} = $flag_types;
-  $vars->{'any_flags_requesteeble'} = grep($_->{'is_requesteeble'}, @$flag_types);
+  $vars->{'any_flags_requesteeble'} = grep($_->is_requesteeble, @$flag_types);
   $vars->{'attachment'} = $attachment;
   $vars->{'bugsummary'} = $bugsummary; 
   $vars->{'isviewable'} = $isviewable; 

importxml.pl

@@ -238,24 +238,24 @@ sub flag_handler {
     # If this is the case, we will only match the first one.
     my $ftype;
     foreach my $f ( @{$flag_types} ) {
-        if ( $f->{'name'} eq $name) {
+        if ( $f->name eq $name) {
             $ftype = $f;
             last;
         }
     }
 
     if ($ftype) {    # We found the flag in the list
-        my $grant_gid = $ftype->{'grant_gid'};
+        my $grant_group = $ftype->grant_group;
         if (( $status eq '+' || $status eq '-' ) 
-            && $grant_gid && !$setter->in_group_id($grant_gid)) {
+            && $grant_group && !$setter->in_group_id($grant_group->id)) {
             $err = "Setter $setter_login on $type flag $name ";
             $err .= "is not in the Grant Group\n";
             $err .= "   Dropping flag $name\n";
             return $err;
         }
-        my $request_gid = $ftype->{'request_gid'};
-        if ($request_gid 
-            && $status eq '?' && !$setter->in_group_id($request_gid)) {
+        my $request_group = $ftype->request_group;
+        if ($request_group
+            && $status eq '?' && !$setter->in_group_id($request_group->id)) {
             $err = "Setter $setter_login on $type flag $name ";
             $err .= "is not in the Request Group\n";
             $err .= "   Dropping flag $name\n";
@@ -263,9 +263,7 @@ sub flag_handler {
         }
 
         # Take the first flag_type that matches
-        my $ftypeid   = $ftype->{'id'};
-        my $is_active = $ftype->{'is_active'};
-        unless ($is_active) {
+        unless ($ftype->is_active) {
             $err = "Flag $name is not active in this database\n";
             $err .= "   Dropping flag $name\n";
             return $err;
@@ -275,7 +273,7 @@ sub flag_handler {
                  (type_id, status, bug_id, attach_id, creation_date, 
                   setter_id, requestee_id)
                   VALUES (?, ?, ?, ?, ?, ?, ?)", undef,
-            ($ftypeid, $status, $bugid, $attachid, $timestamp,
+            ($ftype->id, $status, $bugid, $attachid, $timestamp,
             $setter_id, $requestee_id));
     }
     else {

request.cgi

@@ -198,14 +198,10 @@ sub queue {
     if (defined $form_type && !grep($form_type eq $_, ("", "all"))) {
         # Check if any matching types are for attachments.  If not, don't show
         # the attachment column in the report.
-        my $types = Bugzilla::FlagType::match({ 'name' => $form_type });
-        my $has_attachment_type = 0;
-        foreach my $type (@$types) {
-            if ($type->{'target_type'} eq "attachment") {
-                $has_attachment_type = 1;
-                last;
-            }
-        }
+        my $has_attachment_type =
+            Bugzilla::FlagType::count({ 'name' => $form_type,
+                                        'target_type' => 'attachment' });
+
         if (!$has_attachment_type) { push(@excluded_columns, 'attachment') }
 
         my $quoted_form_type = $dbh->quote($form_type);

template/en/default/admin/flag-type/confirm-delete.html.tmpl

@@ -29,7 +29,7 @@
 %]
 
 <p>
-   There are [% flag_count %] flags of type [% name FILTER html %].
+   There are [% flag_type.flag_count %] flags of type [% name FILTER html %].
    If you delete this type, those flags will also be deleted.  Note that
    instead of deleting the type you can
    <a href="editflagtypes.cgi?action=deactivate&amp;id=[% flag_type.id %]">deactivate it</a>,

template/en/default/admin/flag-type/edit.html.tmpl

@@ -192,7 +192,7 @@
       <td>
         the group allowed to grant/deny flags of this type
         (to allow all users to grant/deny these flags, select no group)<br>
-        [% PROCESS select selname = "grant_gid" %]
+        [% PROCESS select selname = "grant_group" %]
       </td>
     </tr>
 
@@ -202,7 +202,7 @@
         if flags of this type are requestable, the group allowed to request them
         (to allow all users to request these flags, select no group)<br>
         Note that the request group alone has no effect if the grant group is not defined!<br>
-        [% PROCESS select selname = "request_gid" %]
+        [% PROCESS select selname = "request_group" %]
       </td>
     </tr>
 
@@ -232,7 +232,8 @@
     <option value="">(no group)</option>
     [% FOREACH group = groups %]
       <option value="[% group.name FILTER html %]"
-        [% " selected" IF type.${selname} == group.name %]>[% group.name FILTER html %]
+        [% " selected" IF (type.${selname} && type.${selname}.name == group.name) %]>
+      [%- group.name FILTER html %]
       </option>
     [% END %]
   </select>

template/en/default/admin/flag-type/list.html.tmpl

@@ -51,7 +51,7 @@
 <p>
   You can restrict the list of flag types to those available for a given product
   and component. If a product is selected with no component, only flag types
-  which are available to ALL components of the product are shown.
+  which are available to at least one component of the product are shown.
 </p>
 
 <form action="editflagtypes.cgi" method="get">
@@ -59,8 +59,8 @@
     <tr>
       <th><label for="product">Product:</label></th>
       <td>
-        <select name="product" onchange="selectProduct(this.form, 'product', 'component', '__All__');">
-          <option value="">__All__</option>
+        <select name="product" onchange="selectProduct(this.form, 'product', 'component', '__Any__');">
+          <option value="">__Any__</option>
           [% FOREACH prod = products %]
             <option value="[% prod.name FILTER html %]"
                     [% " selected" IF selected_product == prod.name %]>
@@ -71,7 +71,7 @@
       <th><label for="component">Component:</label></th>
       <td>
         <select name="component">
-          <option value="">__All__</option>
+          <option value="">__Any__</option>
           [% FOREACH comp = components %]
             <option value="[% comp FILTER html %]"
                     [% " selected" IF selected_component == comp %]>
@@ -152,8 +152,8 @@
             <span class="multiplicable">multiplicable</span>
           [% END %]
         </td>
-        <td>[% type.grant_group_name FILTER html %]</td>
-        <td>[% type.request_group_name FILTER html %]</td>
+        <td>[% IF type.grant_group %][% type.grant_group.name FILTER html %][% END %]</td>
+        <td>[% IF type.request_group %][% type.request_group.name FILTER html %][% END %]</td>
         <td>
           <a href="editflagtypes.cgi?action=copy&amp;id=[% type.id %]">Copy</a>
           | <a href="editflagtypes.cgi?action=confirmdelete&amp;id=[% type.id %]"

template/en/default/filterexceptions.pl

@@ -527,7 +527,7 @@
 ],
 
 'admin/flag-type/confirm-delete.html.tmpl' => [
-  'flag_count', 
+  'flag_type.flag_count',
   'flag_type.id', 
 ],
 

template/en/default/global/code-error.html.tmpl

@@ -227,10 +227,6 @@
     [% END %]
     is invalid.
 
-  [% ELSIF error == "flag_type_id_invalid" %]
-    The flag type ID <em>[% id FILTER html %]</em> is not
-    a positive integer.
-
   [% ELSIF error == "flag_type_inactive" %]
     [% title = "Inactive Flag Types" %]
     Some flag types are inactive and cannot be used to create new flags.