Bug 339750: Remove Bugzilla::Flag::GetBug - Patch by Frédéric Buclin <LpSolit@gmail.com> r/a=myk

6 files changed, 126 insertions, 201 deletions

diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm
index 8a3199003..bcea5b043 100644
--- a/Bugzilla/Flag.pm
+++ b/Bugzilla/Flag.pm
@@ -64,7 +64,6 @@ use Bugzilla::User;
 use Bugzilla::Config;
 use Bugzilla::Util;
 use Bugzilla::Error;
-use Bugzilla::Attachment;
 use Bugzilla::Mailer;
 use Bugzilla::Constants;
 use Bugzilla::Field;
@@ -298,13 +297,13 @@ sub validate {
                                    { flag_type  => $flag->{'type'},
                                      requestee  => $requestee,
                                      bug_id     => $bug_id,
-                                     attachment => $flag->{target}->{attachment}
+                                     attach_id  => $attach_id
                                    });
                 }
     
                 # Throw an error if the target is a private attachment and
                 # the requestee isn't in the group of insiders who can see it.
-                if ($flag->{target}->{attachment}
+                if ($attach_id
                     && $cgi->param('isprivate')
                     && Param("insidergroup")
                     && !$requestee->in_group(Param("insidergroup")))
@@ -313,7 +312,7 @@ sub validate {
                                    { flag_type  => $flag->{'type'},
                                      requestee  => $requestee,
                                      bug_id     => $bug_id,
-                                     attachment => $flag->{target}->{attachment}
+                                     attach_id  => $attach_id
                                    });
                 }
             }
@@ -361,26 +360,30 @@ sub snapshot {
 
 =over
 
-=item C<process($target, $timestamp, $cgi)>
+=item C<process($bug, $attachment, $timestamp, $cgi)>
 
 Processes changes to flags.
 
-The target is the bug or attachment this flag is about, the timestamp
-is the date/time the bug was last touched (so that changes to the flag
-can be stamped with the same date/time), the cgi is the CGI object
-used to obtain the flag fields that the user submitted.
+The bug and/or the attachment objects are the ones this flag is about,
+the timestamp is the date/time the bug was last touched (so that changes
+to the flag can be stamped with the same date/time), the cgi is the CGI
+object used to obtain the flag fields that the user submitted.
 
 =back
 
 =cut
 
 sub process {
-    my ($bug_id, $attach_id, $timestamp, $cgi) = @_;
-
+    my ($bug, $attachment, $timestamp, $cgi) = @_;
     my $dbh = Bugzilla->dbh;
-    my $target = get_target($bug_id, $attach_id);
-    # Make sure the target exists.
-    return unless $target->{'exists'};
+
+    # Make sure the bug (and attachment, if given) exists and is accessible
+    # to the current user. Moreover, if an attachment object is passed,
+    # make sure it belongs to the given bug.
+    return if ($bug->error || ($attachment && $bug->bug_id != $attachment->bug_id));
+
+    my $bug_id = $bug->bug_id;
+    my $attach_id = $attachment ? $attachment->id : undef;
 
     # Use the date/time we were given if possible (allowing calling code
     # to synchronize the comment's timestamp with those of other records).
@@ -390,14 +393,14 @@ sub process {
     my @old_summaries = snapshot($bug_id, $attach_id);
 
     # Cancel pending requests if we are obsoleting an attachment.
-    if ($attach_id && $cgi->param('isobsolete')) {
-        CancelRequests($bug_id, $attach_id);
+    if ($attachment && $cgi->param('isobsolete')) {
+        CancelRequests($bug, $attachment);
     }
 
     # Create new flags and update existing flags.
-    my $new_flags = FormToNewFlags($target, $cgi);
-    foreach my $flag (@$new_flags) { create($flag, $timestamp) }
-    modify($cgi, $timestamp);
+    my $new_flags = FormToNewFlags($bug, $attachment, $cgi);
+    foreach my $flag (@$new_flags) { create($flag, $bug, $attachment, $timestamp) }
+    modify($bug, $attachment, $cgi, $timestamp);
 
     # In case the bug's product/component has changed, clear flags that are
     # no longer valid.
@@ -414,7 +417,7 @@ sub process {
             AND i.type_id IS NULL",
         undef, $bug_id);
 
-    foreach my $flag_id (@$flag_ids) { clear($flag_id) }
+    foreach my $flag_id (@$flag_ids) { clear($flag_id, $bug, $attachment) }
 
     $flag_ids = $dbh->selectcol_arrayref(
         "SELECT flags.id 
@@ -426,7 +429,7 @@ sub process {
         AND (bugs.component_id = e.component_id OR e.component_id IS NULL)",
         undef, $bug_id);
 
-    foreach my $flag_id (@$flag_ids) { clear($flag_id) }
+    foreach my $flag_id (@$flag_ids) { clear($flag_id, $bug, $attachment) }
 
     # Take a snapshot of flags after changes.
     my @new_summaries = snapshot($bug_id, $attach_id);
@@ -458,7 +461,7 @@ sub update_activity {
 
 =over
 
-=item C<create($flag, $timestamp)>
+=item C<create($flag, $bug, $attachment, $timestamp)>
 
 Creates a flag record in the database.
 
@@ -467,18 +470,17 @@ Creates a flag record in the database.
 =cut
 
 sub create {
-    my ($flag, $timestamp) = @_;
+    my ($flag, $bug, $attachment, $timestamp) = @_;
     my $dbh = Bugzilla->dbh;
 
-    my $attach_id;
-    $attach_id = $flag->{target}->{attachment}->{id} if $flag->{target}->{attachment};
+    my $attach_id = $attachment ? $attachment->id : undef;
     my $requestee_id;
     $requestee_id = $flag->{'requestee'}->id if $flag->{'requestee'};
 
     $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'}, $flag->{'target'}->{'bug'}->{'id'},
+              undef, ($flag->{'type'}->{'id'}, $bug->bug_id,
                       $attach_id, $requestee_id, $flag->{'setter'}->id,
                       $flag->{'status'}, $timestamp, $timestamp));
 
@@ -489,14 +491,14 @@ sub create {
         $flag->{'addressee'} = $flag->{'requestee'};
     }
 
-    notify($flag, "request/email.txt.tmpl");
+    notify($flag, $bug, $attachment);
 }
 
 =pod
 
 =over
 
-=item C<modify($cgi, $timestamp)>
+=item C<modify($bug, $attachment, $cgi, $timestamp)>
 
 Modifies flags in the database when a user changes them.
 
@@ -505,7 +507,7 @@ Modifies flags in the database when a user changes them.
 =cut
 
 sub modify {
-    my ($cgi, $timestamp) = @_;
+    my ($bug, $attachment, $cgi, $timestamp) = @_;
     my $setter = Bugzilla->user;
     my $dbh = Bugzilla->dbh;
 
@@ -538,11 +540,10 @@ sub modify {
             # Create new flags like the existing one for each additional person.
             foreach my $login (@requestees) {
                 create({ type      => $flag->{type} ,
-                         target    => $flag->{target} , 
                          setter    => $setter, 
                          status    => "?",
                          requestee => new Bugzilla::User(login_to_id($login)) },
-                       $timestamp);
+                       $bug, $attachment, $timestamp);
             }
         }
         else {
@@ -599,7 +600,7 @@ sub modify {
                 $flag->{'addressee'} = $requester;
             }
 
-            notify($flag, "request/email.txt.tmpl");
+            notify($flag, $bug, $attachment);
         }
         elsif ($status eq '?') {
             # Get the requestee, if any.
@@ -633,10 +634,10 @@ sub modify {
                 $flag->{'addressee'} = $flag->{'requestee'};
             }
 
-            notify($flag, "request/email.txt.tmpl");
+            notify($flag, $bug, $attachment);
         }
         elsif ($status eq 'X') {
-            clear($flag->{'id'});
+            clear($flag->{'id'}, $bug, $attachment);
         }
 
         push(@flags, $flag);
@@ -649,7 +650,7 @@ sub modify {
 
 =over
 
-=item C<clear($id)>
+=item C<clear($id, $bug, $attachment)>
 
 Remove a flag from the DB.
 
@@ -658,7 +659,7 @@ Remove a flag from the DB.
 =cut
 
 sub clear {
-    my ($id) = @_;
+    my ($id, $bug, $attachment) = @_;
     my $dbh = Bugzilla->dbh;
 
     my $flag = get($id);
@@ -681,7 +682,7 @@ sub clear {
         $flag->{'addressee'} = $requester;
     }
 
-    notify($flag, "request/email.txt.tmpl");
+    notify($flag, $bug, $attachment);
 }
 
 
@@ -693,7 +694,7 @@ sub clear {
 
 =over
 
-=item C<FormToNewFlags($target, $cgi)>
+=item C<FormToNewFlags($bug, $attachment, $cgi)>
 
 Checks whether or not there are new flags to create and returns an
 array of flag objects. This array is then passed to Flag::create().
@@ -703,7 +704,7 @@ array of flag objects. This array is then passed to Flag::create().
 =cut
 
 sub FormToNewFlags {
-    my ($target, $cgi) = @_;
+    my ($bug, $attachment, $cgi) = @_;
     my $dbh = Bugzilla->dbh;
     my $setter = Bugzilla->user;
     
@@ -715,9 +716,9 @@ sub FormToNewFlags {
 
     # Get a list of active flag types available for this target.
     my $flag_types = Bugzilla::FlagType::match(
-        { 'target_type'  => $target->{'type'},
-          'product_id'   => $target->{'bug'}->{'product_id'},
-          'component_id' => $target->{'bug'}->{'component_id'},
+        { 'target_type'  => $attachment ? 'attachment' : 'bug',
+          'product_id'   => $bug->{'product_id'},
+          'component_id' => $bug->{'component_id'},
           'is_active'    => 1 });
 
     my @flags;
@@ -730,10 +731,9 @@ sub FormToNewFlags {
         # Get the number of flags of this type already set for this target.
         my $has_flags = count(
             { 'type_id'     => $type_id,
-              'target_type' => $target->{'type'},
-              'bug_id'      => $target->{'bug'}->{'id'},
-              'attach_id'   => $target->{'attachment'} ?
-                                 $target->{'attachment'}->{'id'} : undef });
+              'target_type' => $attachment ? 'attachment' : 'bug',
+              'bug_id'      => $bug->bug_id,
+              'attach_id'   => $attachment ? $attachment->id : undef });
 
         # Do not create a new flag of this type if this flag type is
         # not multiplicable and already has a flag set.
@@ -747,7 +747,6 @@ sub FormToNewFlags {
             foreach my $login (@logins) {
                 my $requestee = new Bugzilla::User(login_to_id($login));
                 push (@flags, { type      => $flag_type ,
-                                target    => $target , 
                                 setter    => $setter , 
                                 status    => $status ,
                                 requestee => $requestee });
@@ -756,7 +755,6 @@ sub FormToNewFlags {
         }
         else {
             push (@flags, { type   => $flag_type ,
-                            target => $target , 
                             setter => $setter , 
                             status => $status });
         }
@@ -770,85 +768,7 @@ sub FormToNewFlags {
 
 =over
 
-=item C<GetBug($id)>
-
-Returns a hash of information about a target bug.
-
-=back
-
-=cut
-
-# Ideally, we'd use Bug.pm, but it's way too heavyweight, and it can't be
-# made lighter without totally rewriting it, so we'll use this function
-# until that one gets rewritten.
-sub GetBug {
-    my ($id) = @_;
-    my $dbh = Bugzilla->dbh;
-
-    my $bug = $dbh->selectrow_hashref('SELECT 1 AS existence, bugs.bug_id AS id,
-                                               short_desc AS summary,
-                                               product_id, component_id,
-                                               COUNT(bug_group_map.group_id) AS restricted
-                                          FROM bugs
-                                     LEFT JOIN bug_group_map
-                                            ON bugs.bug_id = bug_group_map.bug_id
-                                         WHERE bugs.bug_id = ? ' .
-                                        $dbh->sql_group_by('bugs.bug_id',
-                                              'short_desc, product_id, component_id'),
-                                        undef, $id);
-
-    # 'exists' is a reserved word in MySQL.
-    $bug->{'exists'} = $bug->{'existence'};
-    return $bug;
-}
-
-=pod
-
-=over
-
-=item C<get_target($bug_id, $attach_id)>
-
-Someone please document this function.
-
-=back
-
-=cut
-
-sub get_target {
-    my ($bug_id, $attach_id) = @_;
-    
-    # Create an object representing the target bug/attachment.
-    my $target = { 'exists' => 0 };
-
-    if ($attach_id) {
-        $target->{'attachment'} = Bugzilla::Attachment->get($attach_id);
-        if ($bug_id) {
-            # Make sure the bug and attachment IDs correspond to each other
-            # (i.e. this is the bug to which this attachment is attached).
-            if (!$target->{'attachment'}
-                || $target->{'attachment'}->{'bug_id'} != $bug_id)
-            {
-              return { 'exists' => 0 };
-            }
-        }
-        $target->{'bug'} = GetBug($bug_id);
-        $target->{'exists'} = 1;
-        $target->{'type'} = "attachment";
-    }
-    elsif ($bug_id) {
-        $target->{'bug'} = GetBug($bug_id);
-        $target->{'exists'} = $target->{'bug'}->{'exists'};
-        $target->{'type'} = "bug";
-    }
-
-    return $target;
-}
-
-=pod
-
-=over
-
-=item C<notify($flag, $template_file)>
+=item C<notify($flag, $bug, $attachment)>
 
 Sends an email notification about a flag being created, fulfilled
 or deleted.
@@ -858,27 +778,25 @@ or deleted.
 =cut
 
 sub notify {
-    my ($flag, $template_file) = @_;
+    my ($flag, $bug, $attachment) = @_;
 
     my $template = Bugzilla->template;
 
     # There is nobody to notify.
     return unless ($flag->{'addressee'} || $flag->{'type'}->{'cc_list'});
 
-    my $attachment_is_private = $flag->{'target'}->{'attachment'} ?
-      $flag->{'target'}->{'attachment'}->{'isprivate'} : undef;
+    my $attachment_is_private = $attachment ? $attachment->isprivate : undef;
 
     # If the target bug is restricted to one or more groups, then we need
     # to make sure we don't send email about it to unauthorized users
     # on the request type's CC: list, so we have to trawl the list for users
     # not in those groups or email addresses that don't have an account.
-    if ($flag->{'target'}->{'bug'}->{'restricted'} || $attachment_is_private) {
+    if ($bug->groups || $attachment_is_private) {
         my @new_cc_list;
         foreach my $cc (split(/[, ]+/, $flag->{'type'}->{'cc_list'})) {
             my $ccuser = Bugzilla::User->new_from_login($cc) || next;
 
-            next if $flag->{'target'}->{'bug'}->{'restricted'}
-              && !$ccuser->can_see_bug($flag->{'target'}->{'bug'}->{'id'});
+            next if ($bug->groups && !$ccuser->can_see_bug($bug->bug_id));
             next if $attachment_is_private
               && Param("insidergroup")
               && !$ccuser->in_group(Param("insidergroup"));
@@ -895,9 +813,12 @@ sub notify {
                     split(/[, ]+/, $flag->{'type'}->{'cc_list'}))
     {
         next unless $to;
-        my $vars = { 'flag' => $flag, 'to' => $to };
+        my $vars = { 'flag'       => $flag,
+                     'to'         => $to,
+                     'bug'        => $bug,
+                     'attachment' => $attachment};
         my $message;
-        my $rv = $template->process($template_file, $vars, \$message);
+        my $rv = $template->process("request/email.txt.tmpl", $vars, \$message);
         if (!$rv) {
             Bugzilla->cgi->header();
             ThrowTemplateError($template->error());
@@ -909,7 +830,7 @@ sub notify {
 
 # Cancel all request flags from the attachment being obsoleted.
 sub CancelRequests {
-    my ($bug_id, $attach_id, $timestamp) = @_;
+    my ($bug, $attachment, $timestamp) = @_;
     my $dbh = Bugzilla->dbh;
 
     my $request_ids =
@@ -919,20 +840,21 @@ sub CancelRequests {
                                   WHERE flags.attach_id = ?
                                   AND flags.status = '?'
                                   AND attachments.isobsolete = 0",
-                                  undef, $attach_id);
+                                  undef, $attachment->id);
 
     return if (!scalar(@$request_ids));
 
     # Take a snapshot of flags before any changes.
-    my @old_summaries = snapshot($bug_id, $attach_id) if ($timestamp);
-    foreach my $flag (@$request_ids) { clear($flag) }
+    my @old_summaries = snapshot($bug->bug_id, $attachment->id) if ($timestamp);
+    foreach my $flag (@$request_ids) { clear($flag, $bug, $attachment) }
 
     # If $timestamp is undefined, do not update the activity table
     return unless ($timestamp);
 
     # Take a snapshot of flags after any changes.
-    my @new_summaries = snapshot($bug_id, $attach_id);
-    update_activity($bug_id, $attach_id, $timestamp, \@old_summaries, \@new_summaries);
+    my @new_summaries = snapshot($bug->bug_id, $attachment->id);
+    update_activity($bug->bug_id, $attachment->id, $timestamp,
+                    \@old_summaries, \@new_summaries);
 }
 
 ######################################################################
@@ -988,7 +910,7 @@ sub sqlify_criteria {
 
 =over
 
-=item C<perlify_record($exists, $id, $type_id, $bug_id, $attach_id, $requestee_id, $setter_id, $status)>
+=item C<perlify_record($id, $type_id, $bug_id, $attach_id, $requestee_id, $setter_id, $status)>
 
 Converts a row from the database into a Perl record.
 
@@ -999,21 +921,20 @@ Converts a row from the database into a Perl record.
 =cut
 
 sub perlify_record {
-    my ($id, $type_id, $bug_id, $attach_id, 
+    my ($id, $type_id, $bug_id, $attach_id,
         $requestee_id, $setter_id, $status) = @_;
-    
+
     return undef unless $id;
-    
+
     my $flag =
       {
         id        => $id ,
         type      => Bugzilla::FlagType::get($type_id) ,
-        target    => get_target($bug_id, $attach_id) , 
         requestee => $requestee_id ? new Bugzilla::User($requestee_id) : undef,
         setter    => new Bugzilla::User($setter_id) ,
         status    => $status , 
       };
-    
+
     return $flag;
 }
 
diff --git a/attachment.cgi b/attachment.cgi
index 6fa5ea942..c79ebf2cb 100755
--- a/attachment.cgi
+++ b/attachment.cgi
@@ -268,49 +268,39 @@ sub validatePrivate
     $cgi->param('isprivate', $cgi->param('isprivate') ? 1 : 0);
 }
 
-sub validateObsolete
-{
-  my @obsolete_ids = ();
-  my $dbh = Bugzilla->dbh;
-  
+sub validateObsolete {
   # Make sure the attachment id is valid and the user has permissions to view
   # the bug to which it is attached.
+  my @obsolete_attachments;
   foreach my $attachid ($cgi->param('obsolete')) {
     my $vars = {};
     $vars->{'attach_id'} = $attachid;
-    
+
     detaint_natural($attachid)
       || ThrowCodeError("invalid_attach_id_to_obsolete", $vars);
-  
-    my ($bugid, $isobsolete, $description) = $dbh->selectrow_array(
-            "SELECT bug_id, isobsolete, description 
-             FROM attachments WHERE attach_id = ?", undef, $attachid);
 
-    # Make sure the attachment exists in the database.
-    ThrowUserError("invalid_attach_id", $vars) unless $bugid;
+    my $attachment = Bugzilla::Attachment->get($attachid);
 
+    # Make sure the attachment exists in the database.
+    ThrowUserError("invalid_attach_id", $vars) unless $attachment;
 
+    $vars->{'description'} = $attachment->description;
 
-    $vars->{'description'} = $description;
-    
-    if ($bugid != $cgi->param('bugid'))
-    {
+    if ($attachment->bug_id != $cgi->param('bugid')) {
       $vars->{'my_bug_id'} = $cgi->param('bugid');
-      $vars->{'attach_bug_id'} = $bugid;
+      $vars->{'attach_bug_id'} = $attachment->bug_id;
       ThrowCodeError("mismatched_bug_ids_on_obsolete", $vars);
     }
 
-    if ( $isobsolete )
-    {
+    if ($attachment->isobsolete) {
       ThrowCodeError("attachment_already_obsolete", $vars);
     }
 
     # Check that the user can modify this attachment
     validateCanEdit($attachid);
-    push(@obsolete_ids, $attachid);
+    push(@obsolete_attachments, $attachment);
   }
-
-  return @obsolete_ids;
+  return @obsolete_attachments;
 }
 
 # Returns 1 if the parameter is a content-type viewable in this browser
@@ -797,8 +787,8 @@ sub insert
                                                         $bugid, $user,
                                                         $timestamp, \$vars);
     my $isprivate = $cgi->param('isprivate') ? 1 : 0;
-    my @obsolete_ids = ();
-    @obsolete_ids = validateObsolete() if $cgi->param('obsolete');
+    my @obsolete_attachments;
+    @obsolete_attachments = validateObsolete() if $cgi->param('obsolete');
 
   # Insert a comment about the new attachment into the database.
   my $comment = "Created an attachment (id=$attachid)\n" .
@@ -809,17 +799,19 @@ sub insert
 
   # Make existing attachments obsolete.
   my $fieldid = get_field_id('attachments.isobsolete');
-  foreach my $obsolete_id (@obsolete_ids) {
+  my $bug = new Bugzilla::Bug($bugid, $user->id);
+
+  foreach my $obsolete_attachment (@obsolete_attachments) {
       # If the obsolete attachment has request flags, cancel them.
       # This call must be done before updating the 'attachments' table.
-      Bugzilla::Flag::CancelRequests($bugid, $obsolete_id, $timestamp);
+      Bugzilla::Flag::CancelRequests($bug, $obsolete_attachment, $timestamp);
 
       $dbh->do("UPDATE attachments SET isobsolete = 1 " . 
-              "WHERE attach_id = ?", undef, $obsolete_id);
+              "WHERE attach_id = ?", undef, $obsolete_attachment->id);
       $dbh->do("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when,
                                           fieldid, removed, added) 
               VALUES (?,?,?,?,?,?,?)", undef, 
-              $bugid, $obsolete_id, $user->id, $timestamp, $fieldid, 0, 1);
+              $bugid, $obsolete_attachment->id, $user->id, $timestamp, $fieldid, 0, 1);
   }
 
   # Assign the bug to the user, if they are allowed to take it
@@ -865,7 +857,10 @@ sub insert
   }   
   
   # Create flags.
-  Bugzilla::Flag::process($bugid, $attachid, $timestamp, $cgi);
+  # Update the bug object with updated data.
+  $bug = new Bugzilla::Bug($bugid, $user->id);
+  my $attachment = Bugzilla::Attachment->get($attachid);
+  Bugzilla::Flag::process($bug, $attachment, $timestamp, $cgi);
    
   # Define the variables and functions that will be passed to the UI template.
   $vars->{'mailrecipients'} =  { 'changer' => $user->login,
@@ -963,8 +958,9 @@ sub update
     Bugzilla::Flag::validate($cgi, $bugid, $attach_id);
     Bugzilla::FlagType::validate($cgi, $bugid, $attach_id);
 
-  # Lock database tables in preparation for updating the attachment.
-  $dbh->bz_lock_tables('attachments WRITE', 'flags WRITE' ,
+    my $bug = new Bugzilla::Bug($bugid, $userid);
+    # Lock database tables in preparation for updating the attachment.
+    $dbh->bz_lock_tables('attachments WRITE', 'flags WRITE' ,
           'flagtypes READ', 'fielddefs READ', 'bugs_activity WRITE',
           'flaginclusions AS i READ', 'flagexclusions AS e READ',
           # cc, bug_group_map, user_group_map, and groups are in here so we
@@ -974,7 +970,7 @@ sub update
           # Bugzilla::User can flatten groups.
           'bugs WRITE', 'profiles READ', 'email_setting READ',
           'cc READ', 'bug_group_map READ', 'user_group_map READ',
-          'group_group_map READ', 'groups READ');
+          'group_group_map READ', 'groups READ', 'group_control_map READ');
 
   # Get a copy of the attachment record before we make changes
   # so we can record those changes in the activity table.
@@ -999,7 +995,8 @@ sub update
   # to attachments so that we can delete pending requests if the user
   # is obsoleting this attachment without deleting any requests
   # the user submits at the same time.
-  Bugzilla::Flag::process($bugid, $attach_id, $timestamp, $cgi);
+  my $attachment = Bugzilla::Attachment->get($attach_id);
+  Bugzilla::Flag::process($bug, $attachment, $timestamp, $cgi);
 
   # Update the attachment record in the database.
   $dbh->do("UPDATE  attachments 
diff --git a/editflagtypes.cgi b/editflagtypes.cgi
index e055e9d9d..0386fca06 100755
--- a/editflagtypes.cgi
+++ b/editflagtypes.cgi
@@ -41,6 +41,8 @@ use Bugzilla::Group;
 use Bugzilla::Util;
 use Bugzilla::Product;
 use Bugzilla::Component;
+use Bugzilla::Bug;
+use Bugzilla::Attachment;
 
 use List::Util qw(reduce);
 
@@ -384,10 +386,10 @@ sub update {
     validateAndSubmit($id);
 
     $dbh->bz_unlock_tables();
-    
+
     # Clear existing flags for bugs/attachments in categories no longer on 
     # the list of inclusions or that have been added to the list of exclusions.
-    my $flag_ids = $dbh->selectcol_arrayref('SELECT flags.id
+    my $flags =    $dbh->selectall_arrayref('SELECT flags.id, flags.bug_id, flags.attach_id
                                                FROM flags
                                          INNER JOIN bugs
                                                  ON flags.bug_id = bugs.bug_id
@@ -400,11 +402,14 @@ sub update {
                                               WHERE flags.type_id = ?
                                                 AND i.type_id IS NULL',
                                              undef, $id);
-    foreach my $flag_id (@$flag_ids) {
-        Bugzilla::Flag::clear($flag_id);
+    foreach my $flag (@$flags) {
+        my ($flag_id, $bug_id, $attach_id) = @$flag;
+        my $bug = new Bugzilla::Bug($bug_id, $user->id);
+        my $attachment = $attach_id ? Bugzilla::Attachment->get($attach_id) : undef;
+        Bugzilla::Flag::clear($flag_id, $bug, $attachment);
     }
-    
-    $flag_ids = $dbh->selectcol_arrayref('SELECT flags.id 
+
+    $flags =    $dbh->selectall_arrayref('SELECT flags.id, flags.bug_id, flags.attach_id
                                             FROM flags
                                       INNER JOIN bugs 
                                               ON flags.bug_id = bugs.bug_id
@@ -416,10 +421,13 @@ sub update {
                                              AND (bugs.component_id = e.component_id
                                                   OR e.component_id IS NULL)',
                                           undef, $id);
-    foreach my $flag_id (@$flag_ids) {
-        Bugzilla::Flag::clear($flag_id);
+    foreach my $flag (@$flags) {
+        my ($flag_id, $bug_id, $attach_id) = @$flag;
+        my $bug = new Bugzilla::Bug($bug_id, $user->id);
+        my $attachment = $attach_id ? Bugzilla::Attachment->get($attach_id) : undef;
+        Bugzilla::Flag::clear($flag_id, $bug, $attachment);
     }
-    
+
     $vars->{'name'} = $cgi->param('name');
     $vars->{'message'} = "flag_type_changes_saved";
 
diff --git a/process_bug.cgi b/process_bug.cgi
index 51853de18..ee21b3aef 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -2122,7 +2122,7 @@ foreach my $id (@idlist) {
         }
     }
     # Set and update flags.
-    Bugzilla::Flag::process($id, undef, $timestamp, $cgi);
+    Bugzilla::Flag::process($new_bug_obj, undef, $timestamp, $cgi);
 
     if ($bug_changed) {
         $dbh->do(q{UPDATE bugs SET delta_ts = ? WHERE bug_id = ?},
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index dd2e529c2..3b901929f 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -446,7 +446,7 @@
     You asked [% requestee.identity FILTER html %]
     for <code>[% flag_type.name FILTER html %]</code> on [% terms.bug %] 
     [%+ bug_id FILTER html -%]
-    [% IF attachment %], attachment [% attachment.id FILTER html %][% END %], 
+    [% IF attach_id > 0 %], attachment [% attach_id FILTER html %][% END %],
     but that [% terms.bug %] has been restricted to users in certain groups, 
     and the user you asked isn't in all the groups to which 
     the [% terms.bug %] has been restricted.
@@ -463,7 +463,7 @@
     You asked [% requestee.identity FILTER html %]
     for <code>[% flag_type.name FILTER html %]</code> on 
     [%+ terms.bug %] [%+ bug_id FILTER html %],
-    attachment [% attachment.id FILTER html %], but that attachment
+    attachment [% attach_id FILTER html %], but that attachment
     is restricted to users in the [% Param("insidergroup") FILTER html %] group,
     and the user you asked isn't in that group.  Please choose someone else
     to ask, or ask an administrator to add the user to the group.
diff --git a/template/en/default/request/email.txt.tmpl b/template/en/default/request/email.txt.tmpl
index 6e92c72ca..3fb08c447 100644
--- a/template/en/default/request/email.txt.tmpl
+++ b/template/en/default/request/email.txt.tmpl
@@ -23,9 +23,8 @@
 
 [% PROCESS global/variables.none.tmpl %]
 
-[% bugidsummary = flag.target.bug.id _ ': ' _ flag.target.bug.summary %]
-[% attidsummary = flag.target.attachment.id _ ': ' _
-                  flag.target.attachment.summary %]
+[% bugidsummary = bug.bug_id _ ': ' _ bug.short_desc %]
+[% attidsummary = attachment.id _ ': ' _ attachment.description %]
 [% statuses = { '+' => "granted" , '-' => 'denied' , 'X' => "cancelled" ,
                 '?' => "asked" } %]
 [% IF flag.status == '?' %]
@@ -39,9 +38,9 @@
 [% END %]
 From: bugzilla-request-daemon
 To: [% to %]
-Subject: [% flag.type.name %] [%+ subject_status %]: [[% terms.Bug %] [%+ flag.target.bug.id %]] [% flag.target.bug.summary %]
-[%- IF flag.target.attachment.exists %] :
-  [Attachment [% flag.target.attachment.id %]] [% flag.target.attachment.summary %][% END %]
+Subject: [% flag.type.name %] [%+ subject_status %]: [[% terms.Bug %] [%+ bug.bug_id %]] [% bug.short_desc %]
+[%- IF attachment %] :
+  [Attachment [% attachment.id %]] [% attachment.description %][% END %]
 
 [%+ USE wrap -%]
 [%- FILTER bullet = wrap(80) -%]
@@ -50,13 +49,13 @@ Subject: [% flag.type.name %] [%+ subject_status %]: [[% terms.Bug %] [%+ flag.t
 
 [% terms.Bug %] [%+ bugidsummary %]
 [% END %]
-[%+ Param('urlbase') %]show_bug.cgi?id=[% flag.target.bug.id %]
-[% IF flag.target.attachment.exists %]
+[%+ Param('urlbase') %]show_bug.cgi?id=[% bug.bug_id %]
+[% IF attachment %]
 
 [% FILTER bullet = wrap(80) %]
 Attachment [% attidsummary %]
 [%- END %]
-[%+ Param('urlbase') %]attachment.cgi?id=[% flag.target.attachment.id %]&action=edit
+[%+ Param('urlbase') %]attachment.cgi?id=[% attachment.id %]&action=edit
 [%- END %]
 [%- FILTER bullet = wrap(80) %]