Bug 415541: Implement $bug->set_flags() and $attachment->set_flags() - Patch by Frédéric Buclin <LpSolit@gmail.com> a=LpSolit

13 files changed, 692 insertions, 857 deletions

diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm
index 9748d17f4..752ddce9a 100644
--- a/Bugzilla/Attachment.pm
+++ b/Bugzilla/Attachment.pm
@@ -101,7 +101,6 @@ use constant UPDATE_COLUMNS => qw(
     ispatch
     isprivate
     mimetype
-    modification_time
 );
 
 use constant VALIDATORS => {
@@ -445,9 +444,9 @@ flags that have been set on the attachment
 
 sub flags {
     my $self = shift;
-    return $self->{flags} if exists $self->{flags};
 
-    $self->{flags} = Bugzilla::Flag->match({ 'attach_id' => $self->id });
+    # Don't cache it as it must be in sync with ->flag_types.
+    $self->{flags} = [map { @{$_->{flags}} } @{$self->flag_types}];
     return $self->{flags};
 }
 
@@ -471,7 +470,7 @@ sub flag_types {
                  component_id => $self->bug->component_id,
                  attach_id    => $self->id };
 
-    $self->{flag_types} = Bugzilla::Flag::_flag_types($vars);
+    $self->{flag_types} = Bugzilla::Flag->_flag_types($vars);
     return $self->{flag_types};
 }
 
@@ -482,10 +481,34 @@ sub flag_types {
 sub set_content_type { $_[0]->set('mimetype', $_[1]); }
 sub set_description  { $_[0]->set('description', $_[1]); }
 sub set_filename     { $_[0]->set('filename', $_[1]); }
-sub set_is_obsolete  { $_[0]->set('isobsolete', $_[1]); }
 sub set_is_patch     { $_[0]->set('ispatch', $_[1]); }
 sub set_is_private   { $_[0]->set('isprivate', $_[1]); }
 
+sub set_is_obsolete  {
+    my ($self, $obsolete) = @_;
+
+    my $old = $self->isobsolete;
+    $self->set('isobsolete', $obsolete);
+    my $new = $self->isobsolete;
+
+    # If the attachment is being marked as obsolete, cancel pending requests.
+    if ($new && $old != $new) {
+        my @requests = grep { $_->status eq '?' } @{$self->flags};
+        return unless scalar @requests;
+
+        my %flag_ids = map { $_->id => 1 } @requests;
+        foreach my $flagtype (@{$self->flag_types}) {
+            @{$flagtype->{flags}} = grep { !$flag_ids{$_->id} } @{$flagtype->{flags}};
+        }
+    }
+}
+
+sub set_flags {
+    my ($self, $flags, $new_flags) = @_;
+
+    Bugzilla::Flag->set_flag($self, $_) foreach (@$flags, @$new_flags);
+}
+
 sub _check_bug {
     my ($invocant, $bug) = @_;
     my $user = Bugzilla->user;
@@ -799,7 +822,7 @@ Params:     takes a hashref with the following keys:
             parameter has no effect.
             C<mimetype> - string - a valid MIME type.
             C<creation_ts> - string (optional) - timestamp of the insert
-            as returned by SELECT NOW().
+            as returned by SELECT LOCALTIMESTAMP(0).
             C<ispatch> - boolean (optional, default false) - true if the
             attachment is a patch.
             C<isprivate> - boolean (optional, default false) - true if
@@ -887,7 +910,7 @@ sub run_create_validators {
     $params->{ispatch} = $params->{ispatch} ? 1 : 0;
     $params->{filename} = $class->_check_filename($params->{filename}, $params->{isurl});
     $params->{mimetype} = $class->_check_content_type($params->{mimetype});
-    $params->{creation_ts} ||= Bugzilla->dbh->selectrow_array('SELECT NOW()');
+    $params->{creation_ts} ||= Bugzilla->dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
     $params->{modification_time} = $params->{creation_ts};
     $params->{submitter_id} = Bugzilla->user->id || ThrowCodeError('invalid_user');
 
@@ -898,14 +921,14 @@ sub update {
     my $self = shift;
     my $dbh = Bugzilla->dbh;
     my $user = Bugzilla->user;
-    my $bug = $self->bug;
-
-    my $timestamp = shift || $dbh->selectrow_array('SELECT NOW()');
-    $self->{modification_time} = $timestamp;
+    my $timestamp = shift || $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
 
     my ($changes, $old_self) = $self->SUPER::update(@_);
-    # Ignore this change.
-    delete $changes->{modification_time};
+
+    my ($removed, $added) = Bugzilla::Flag->update_flags($self, $old_self, $timestamp);
+    if ($removed || $added) {
+        $changes->{'flagtypes.name'} = [$removed, $added];
+    }
 
     # Record changes in the activity table.
     my $sth = $dbh->prepare('INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when,
@@ -914,14 +937,17 @@ sub update {
 
     foreach my $field (keys %$changes) {
         my $change = $changes->{$field};
-        my $fieldid = get_field_id("attachments.$field");
-        $sth->execute($bug->id, $self->id, $user->id, $timestamp,
+        $field = "attachments.$field" unless $field eq "flagtypes.name";
+        my $fieldid = get_field_id($field);
+        $sth->execute($self->bug_id, $self->id, $user->id, $timestamp,
                       $fieldid, $change->[0], $change->[1]);
     }
 
     if (scalar(keys %$changes)) {
+      $dbh->do('UPDATE attachments SET modification_time = ? WHERE attach_id = ?',
+               undef, ($timestamp, $self->id));
       $dbh->do('UPDATE bugs SET delta_ts = ? WHERE bug_id = ?',
-               undef, $timestamp, $bug->id);
+               undef, ($timestamp, $self->bug_id));
     }
 
     return $changes;
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 64a53b8a1..64627f4e9 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -590,7 +590,7 @@ sub run_create_validators {
     # Callers cannot set Reporter, currently.
     $params->{reporter} = $class->_check_reporter();
 
-    $params->{creation_ts} ||= Bugzilla->dbh->selectrow_array('SELECT NOW()');
+    $params->{creation_ts} ||= Bugzilla->dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
     $params->{delta_ts} = $params->{creation_ts};
 
     if ($params->{estimated_time}) {
@@ -646,7 +646,7 @@ sub update {
     my $dbh = Bugzilla->dbh;
     # XXX This is just a temporary hack until all updating happens
     # inside this function.
-    my $delta_ts = shift || $dbh->selectrow_array("SELECT NOW()");
+    my $delta_ts = shift || $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
 
     my ($changes, $old_bug) = $self->SUPER::update(@_);
 
@@ -774,7 +774,13 @@ sub update {
         $changes->{'bug_group'} = [join(', ', @removed_names),
                                    join(', ', @added_names)];
     }
-    
+
+    # Flags
+    my ($removed, $added) = Bugzilla::Flag->update_flags($self, $old_bug, $delta_ts);
+    if ($removed || $added) {
+        $changes->{'flagtypes.name'} = [$removed, $added];
+    }
+
     # Comments
     foreach my $comment (@{$self->{added_comments} || []}) {
         my $columns = join(',', keys %$comment);
@@ -1931,6 +1937,11 @@ sub set_dup_id {
 }
 sub set_estimated_time { $_[0]->set('estimated_time', $_[1]); }
 sub _set_everconfirmed { $_[0]->set('everconfirmed', $_[1]); }
+sub set_flags {
+    my ($self, $flags, $new_flags) = @_;
+
+    Bugzilla::Flag->set_flag($self, $_) foreach (@$flags, @$new_flags);
+}
 sub set_op_sys         { $_[0]->set('op_sys',        $_[1]); }
 sub set_platform       { $_[0]->set('rep_platform',  $_[1]); }
 sub set_priority       { $_[0]->set('priority',      $_[1]); }
@@ -2632,10 +2643,18 @@ sub flag_types {
                  component_id => $self->{component_id},
                  bug_id       => $self->bug_id };
 
-    $self->{'flag_types'} = Bugzilla::Flag::_flag_types($vars);
+    $self->{'flag_types'} = Bugzilla::Flag->_flag_types($vars);
     return $self->{'flag_types'};
 }
 
+sub flags {
+    my $self = shift;
+
+    # Don't cache it as it must be in sync with ->flag_types.
+    $self->{flags} = [map { @{$_->{flags}} } @{$self->flag_types}];
+    return $self->{flags};
+}
+
 sub isopened {
     my $self = shift;
     return is_open_state($self->{bug_status}) ? 1 : 0;
diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm
index 66c392198..6efe0b431 100644
--- a/Bugzilla/Flag.pm
+++ b/Bugzilla/Flag.pm
@@ -53,6 +53,8 @@ whose names start with _ or a re specifically noted as being private.
 
 =cut
 
+use Scalar::Util qw(blessed);
+
 use Bugzilla::FlagType;
 use Bugzilla::Hook;
 use Bugzilla::User;
@@ -69,21 +71,44 @@ use base qw(Bugzilla::Object Exporter);
 ####    Initialization     ####
 ###############################
 
-use constant DB_COLUMNS => qw(
-    flags.id
-    flags.type_id
-    flags.bug_id
-    flags.attach_id
-    flags.requestee_id
-    flags.setter_id
-    flags.status
-);
-
 use constant DB_TABLE => 'flags';
 use constant LIST_ORDER => 'id';
 
 use constant SKIP_REQUESTEE_ON_ERROR => 1;
 
+use constant DB_COLUMNS => qw(
+    id
+    type_id
+    bug_id
+    attach_id
+    requestee_id
+    setter_id
+    status
+);
+
+use constant REQUIRED_CREATE_FIELDS => qw(
+    attach_id
+    bug_id
+    setter_id
+    status
+    type_id
+);
+
+use constant UPDATE_COLUMNS => qw(
+    requestee_id
+    setter_id
+    status
+    type_id
+);
+
+use constant VALIDATORS => {
+};
+
+use constant UPDATE_VALIDATORS => {
+    setter => \&_check_setter,
+    status => \&_check_status,
+};
+
 ###############################
 ####      Accessors      ######
 ###############################
@@ -116,11 +141,14 @@ Returns the status '+', '-', '?' of the flag.
 
 =cut
 
-sub id     { return $_[0]->{'id'};     }
-sub name   { return $_[0]->type->name; }
-sub bug_id { return $_[0]->{'bug_id'}; }
-sub attach_id { return $_[0]->{'attach_id'}; }
-sub status { return $_[0]->{'status'}; }
+sub id           { return $_[0]->{'id'};           }
+sub name         { return $_[0]->type->name;       }
+sub type_id      { return $_[0]->{'type_id'};      }
+sub bug_id       { return $_[0]->{'bug_id'};       }
+sub attach_id    { return $_[0]->{'attach_id'};    }
+sub status       { return $_[0]->{'status'};       }
+sub setter_id    { return $_[0]->{'setter_id'};    }
+sub requestee_id { return $_[0]->{'requestee_id'}; }
 
 ###############################
 ####       Methods         ####
@@ -184,6 +212,14 @@ sub attachment {
     return $self->{'attachment'};
 }
 
+sub bug {
+    my $self = shift;
+
+    require Bugzilla::Bug;
+    $self->{'bug'} ||= new Bugzilla::Bug($self->bug_id);
+    return $self->{'bug'};
+}
+
 ################################
 ## Searching/Retrieving Flags ##
 ################################
@@ -268,251 +304,171 @@ sub count {
 # Creating and Modifying
 ######################################################################
 
-=pod
-
-=over
-
-=item C<validate($bug_id, $attach_id, $skip_requestee_on_error)>
-
-Validates fields containing flag modifications.
-
-If the attachment is new, it has no ID yet and $attach_id is set
-to -1 to force its check anyway.
-
-=back
-
-=cut
-
-sub validate {
-    my ($bug_id, $attach_id, $skip_requestee_on_error) = @_;
-    my $cgi = Bugzilla->cgi;
-    my $dbh = Bugzilla->dbh;
-
-    # Get a list of flags to validate.  Uses the "map" function
-    # to extract flag IDs from form field names by matching fields
-    # whose name looks like "flag_type-nnn" (new flags) or "flag-nnn"
-    # (existing flags), where "nnn" is the ID, and returning just
-    # the ID portion of matching field names.
-    my @flagtype_ids = map(/^flag_type-(\d+)$/ ? $1 : (), $cgi->param());
-    my @flag_ids = map(/^flag-(\d+)$/ ? $1 : (), $cgi->param());
-
-    return unless (scalar(@flagtype_ids) || scalar(@flag_ids));
-
-    # No flag reference should exist when changing several bugs at once.
-    ThrowCodeError("flags_not_available", { type => 'b' }) unless $bug_id;
+sub set_flag {
+    my ($class, $obj, $params) = @_;
 
-    # We don't check that these new flags are valid for this bug/attachment,
-    # because the bug may be moved into another product meanwhile.
-    # This check will be done later when creating new flags, see FormToNewFlags().
+    my ($bug, $attachment);
+    if (blessed($obj) && $obj->isa('Bugzilla::Attachment')) {
+        $attachment = $obj;
+        $bug = $attachment->bug;
+    }
+    elsif (blessed($obj) && $obj->isa('Bugzilla::Bug')) {
+        $bug = $obj;
+    }
+    else {
+        ThrowCodeError('flag_unexpected_object', { 'caller' => ref $obj });
+    }
 
-    if (scalar(@flag_ids)) {
-        # No reference to existing flags should exist when creating a new
-        # attachment.
-        if ($attach_id && ($attach_id < 0)) {
-            ThrowCodeError('flags_not_available', { type => 'a' });
+    # Update (or delete) an existing flag.
+    if ($params->{id}) {
+        my $flag = $class->check({ id => $params->{id} });
+
+        # Security check: make sure the flag belongs to the bug/attachment.
+        # We don't check that the user editing the flag can see
+        # the bug/attachment. That's the job of the caller.
+        ($attachment && $flag->attach_id && $attachment->id == $flag->attach_id)
+          || (!$attachment && !$flag->attach_id && $bug->id == $flag->bug_id)
+          || ThrowCodeError('invalid_flag_association',
+                            { bug_id    => $bug->id,
+                              attach_id => $attachment ? $attachment->id : undef });
+
+        # Extract the current flag object from the object.
+        my ($obj_flagtype) = grep { $_->id == $flag->type_id } @{$obj->flag_types};
+        # If no flagtype can be found for this flag, this means the bug is being
+        # moved into a product/component where the flag is no longer valid.
+        # So either we can attach the flag to another flagtype having the same
+        # name, or we remove the flag.
+        if (!$obj_flagtype) {
+            my $success = $flag->retarget($obj);
+            return unless $success;
+
+            ($obj_flagtype) = grep { $_->id == $flag->type_id } @{$obj->flag_types};
+            push(@{$obj_flagtype->{flags}}, $flag);
         }
+        my ($obj_flag) = grep { $_->id == $flag->id } @{$obj_flagtype->{flags}};
+        # If the flag has the correct type but cannot be found above, this means
+        # the flag is going to be removed (e.g. because this is a pending request
+        # and the attachment is being marked as obsolete).
+        return unless $obj_flag;
 
-        # Make sure all existing flags belong to the bug/attachment
-        # they pretend to be.
-        my $field = ($attach_id) ? "attach_id" : "bug_id";
-        my $field_id = $attach_id || $bug_id;
-        my $not = ($attach_id) ? "" : "NOT";
-
-        my $invalid_data =
-            $dbh->selectrow_array(
-                      "SELECT 1 FROM flags
-                        WHERE " 
-                       . $dbh->sql_in('id', \@flag_ids) 
-                       . " AND ($field != ? OR attach_id IS $not NULL) "
-                       . $dbh->sql_limit(1), undef, $field_id);
-
-        if ($invalid_data) {
-            ThrowCodeError('invalid_flag_association',
-                           { bug_id    => $bug_id,
-                             attach_id => $attach_id });
-        }
+        $class->_validate($obj_flag, $obj_flagtype, $params, $bug, $attachment);
     }
-
-    # Validate new flags.
-    foreach my $id (@flagtype_ids) {
-        my $status = $cgi->param("flag_type-$id");
-        my @requestees = $cgi->param("requestee_type-$id");
-        my $private_attachment = $cgi->param('isprivate') ? 1 : 0;
-
+    # Create a new flag.
+    elsif ($params->{type_id}) {
         # Don't bother validating types the user didn't touch.
-        next if $status eq 'X';
-
-        # Make sure the flag type exists. If it doesn't, FormToNewFlags()
-        # will ignore it, so it's safe to ignore it here.
-        my $flag_type = new Bugzilla::FlagType($id);
-        next unless $flag_type;
+        return if $params->{status} eq 'X';
+
+        my $flagtype = Bugzilla::FlagType->check({ id => $params->{type_id} });
+        # Security check: make sure the flag type belongs to the bug/attachment.
+        ($attachment && $flagtype->target_type eq 'attachment'
+          && scalar(grep { $_->id == $flagtype->id } @{$attachment->flag_types}))
+          || (!$attachment && $flagtype->target_type eq 'bug'
+                && scalar(grep { $_->id == $flagtype->id } @{$bug->flag_types}))
+          || ThrowCodeError('invalid_flag_association',
+                            { bug_id    => $bug->id,
+                              attach_id => $attachment ? $attachment->id : undef });
 
         # Make sure the flag type is active.
-        unless ($flag_type->is_active) {
-            ThrowCodeError('flag_type_inactive', {'type' => $flag_type->name});
-        }
-
-        _validate(undef, $flag_type, $status, undef, \@requestees, $private_attachment,
-                  $bug_id, $attach_id, $skip_requestee_on_error);
-    }
+        $flagtype->is_active
+          || ThrowCodeError('flag_type_inactive', { type => $flagtype->name });
 
-    # Validate existing flags.
-    foreach my $id (@flag_ids) {
-        my $status = $cgi->param("flag-$id");
-        my @requestees = $cgi->param("requestee-$id");
-        my $private_attachment = $cgi->param('isprivate') ? 1 : 0;
+        # Extract the current flagtype object from the object.
+        my ($obj_flagtype) = grep { $_->id == $flagtype->id } @{$obj->flag_types};
 
-        # Make sure the flag exists. If it doesn't, process() will ignore it,
-        # so it's safe to ignore it here.
-        my $flag = new Bugzilla::Flag($id);
-        next unless $flag;
+        # We cannot create a new flag if there is already one and this
+        # flag type is not multiplicable.
+        if (!$flagtype->is_multiplicable) {
+            if (scalar @{$obj_flagtype->{flags}}) {
+                ThrowUserError('flag_type_not_multiplicable', { type => $flagtype });
+            }
+        }
 
-        _validate($flag, $flag->type, $status, undef, \@requestees, $private_attachment,
-                  undef, undef, $skip_requestee_on_error);
+        $class->_validate(undef, $obj_flagtype, $params, $bug, $attachment);
+    }
+    else {
+        ThrowCodeError('param_required', { function => $class . '->set_flag',
+                                           param    => 'id/type_id' });
     }
 }
 
 sub _validate {
-    my ($flag, $flag_type, $status, $setter, $requestees, $private_attachment,
-        $bug_id, $attach_id, $skip_requestee_on_error) = @_;
-
-    # By default, the flag setter (or requester) is the current user.
-    $setter ||= Bugzilla->user;
-
-    my $id = $flag ? $flag->id : $flag_type->id; # Used in the error messages below.
-    $bug_id ||= $flag->bug_id;
-    $attach_id ||= $flag->attach_id if $flag; # Maybe it's a bug flag.
-
-    # Make sure the user chose a valid status.
-    grep($status eq $_, qw(X + - ?))
-      || ThrowCodeError('flag_status_invalid',
-                        { id => $id, status => $status });
-
-    # Make sure the user didn't request the flag unless it's requestable.
-    # If the flag existed and was requested before it became unrequestable,
-    # leave it as is.
-    if ($status eq '?'
-        && (!$flag || $flag->status ne '?')
-        && !$flag_type->is_requestable)
+    my ($class, $flag, $flag_type, $params, $bug, $attachment) = @_;
+
+    # If it's a new flag, let's create it now.
+    my $obj_flag = $flag || bless({ type_id   => $flag_type->id,
+                                    status    => '',
+                                    bug_id    => $bug->id,
+                                    attach_id => $attachment ?
+                                                   $attachment->id : undef},
+                                    $class);
+
+    my $old_status = $obj_flag->status;
+    my $old_requestee_id = $obj_flag->requestee_id;
+
+    $obj_flag->_set_status($params->{status});
+    $obj_flag->_set_requestee($params->{requestee}, $attachment, $params->{skip_roe});
+
+    # The setter field MUST NOT be updated if neither the status
+    # nor the requestee fields changed.
+    if (($obj_flag->status ne $old_status)
+        # The requestee ID can be undefined.
+        || (($obj_flag->requestee_id || 0) != ($old_requestee_id || 0)))
     {
-        ThrowCodeError('flag_status_invalid',
-                       { id => $id, status => $status });
+        $obj_flag->_set_setter($params->{setter});
     }
 
-    # Make sure the user didn't specify a requestee unless the flag
-    # is specifically requestable. For existing flags, if the requestee
-    # was set before 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) {
-        my $old_requestee = ($flag && $flag->requestee) ?
-                                $flag->requestee->login : '';
-        my $new_requestee = join('', @$requestees);
-        if ($new_requestee && $new_requestee ne $old_requestee) {
-            ThrowCodeError('flag_requestee_disabled',
-                           { type => $flag_type });
-        }
+    # If the flag is deleted, remove it from the list.
+    if ($obj_flag->status eq 'X') {
+        @{$flag_type->{flags}} = grep { $_->id != $obj_flag->id } @{$flag_type->{flags}};
     }
-
-    # 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
-        && scalar(@$requestees) > 1)
-    {
-        ThrowUserError('flag_not_multiplicable', { type => $flag_type });
+    # Add the newly created flag to the list.
+    elsif (!$obj_flag->id) {
+        push(@{$flag_type->{flags}}, $obj_flag);
     }
+}
 
-    # 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) {
-        my $old_requestee = ($flag && $flag->requestee) ?
-                                $flag->requestee->login : '';
-
-        my @legal_requestees;
-        foreach my $login (@$requestees) {
-            if ($login eq $old_requestee) {
-                # This requestee was already set. Leave him alone.
-                push(@legal_requestees, $login);
-                next;
-            }
+=pod
 
-            # We know the requestee exists because we ran
-            # Bugzilla::User::match_field before getting here.
-            my $requestee = new Bugzilla::User({ name => $login });
+=over
 
-            # Throw an error if the user can't see the bug.
-            # Note that if permissions on this bug are changed,
-            # can_see_bug() will refer to old settings.
-            if (!$requestee->can_see_bug($bug_id)) {
-                next if $skip_requestee_on_error;
-                ThrowUserError('flag_requestee_unauthorized',
-                               { flag_type  => $flag_type,
-                                 requestee  => $requestee,
-                                 bug_id     => $bug_id,
-                                 attach_id  => $attach_id });
-            }
+=item C<create($flag, $timestamp)>
 
-            # 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 ($attach_id
-                && $private_attachment
-                && Bugzilla->params->{'insidergroup'}
-                && !$requestee->in_group(Bugzilla->params->{'insidergroup'}))
-            {
-                next if $skip_requestee_on_error;
-                ThrowUserError('flag_requestee_unauthorized_attachment',
-                               { flag_type  => $flag_type,
-                                 requestee  => $requestee,
-                                 bug_id     => $bug_id,
-                                 attach_id  => $attach_id });
-            }
+Creates a flag record in the database.
 
-            # Throw an error if the user won't be allowed to set the flag.
-            if (!$requestee->can_set_flag($flag_type)) {
-                next if $skip_requestee_on_error;
-                ThrowUserError('flag_requestee_needs_privs',
-                               {'requestee' => $requestee,
-                                'flagtype'  => $flag_type});
-            }
+=back
 
-            # This requestee can be set.
-            push(@legal_requestees, $login);
-        }
+=cut
 
-        # Update the requestee list for this flag.
-        if (scalar(@legal_requestees) < scalar(@$requestees)) {
-            my $field_name = 'requestee_type-' . $flag_type->id;
-            Bugzilla->cgi->delete($field_name);
-            Bugzilla->cgi->param(-name => $field_name, -value => \@legal_requestees);
-        }
-    }
+sub create {
+    my ($class, $flag, $timestamp) = @_;
+    $timestamp ||= Bugzilla->dbh->selectrow_array('SELECT NOW()');
 
-    # Make sure the user is authorized to modify flags, see bug 180879
-    # - The flag exists and is unchanged.
-    return if ($flag && ($status eq $flag->status));
+    my $params = {};
+    my @columns = grep { $_ ne 'id' } $class->DB_COLUMNS;
+    $params->{$_} = $flag->{$_} foreach @columns;
 
-    # - User in the request_group can clear pending requests and set flags
-    #   and can rerequest set flags.
-    return if (($status eq 'X' || $status eq '?')
-               && $setter->can_request_flag($flag_type));
+    $params->{creation_date} = $params->{modification_date} = $timestamp;
+    $flag = $class->SUPER::create($params);
+    return $flag;
+}
+
+sub update {
+    my $self = shift;
+    my $dbh = Bugzilla->dbh;
+    my $timestamp = shift || $dbh->selectrow_array('SELECT NOW()');
 
-    # - User in the grant_group can set/clear flags, including "+" and "-".
-    return if $setter->can_set_flag($flag_type);
+    my $changes = $self->SUPER::update(@_);
 
-    # - Any other flag modification is denied
-    ThrowUserError('flag_update_denied',
-                    { name       => $flag_type->name,
-                      status     => $status,
-                      old_status => $flag ? $flag->status : 'X' });
+    if (scalar(keys %$changes)) {
+        $dbh->do('UPDATE flags SET modification_date = ? WHERE id = ?',
+                 undef, ($timestamp, $self->id));
+    }
+    return $changes;
 }
 
 sub snapshot {
-    my ($class, $bug_id, $attach_id) = @_;
+    my ($class, $flags) = @_;
 
-    my $flags = $class->match({ 'bug_id'    => $bug_id,
-                                'attach_id' => $attach_id });
     my @summaries;
     foreach my $flag (@$flags) {
         my $summary = $flag->setter->nick . ':' . $flag->type->name . $flag->status;
@@ -522,479 +478,378 @@ sub snapshot {
     return @summaries;
 }
 
+sub update_activity {
+    my ($class, $old_summaries, $new_summaries) = @_;
 
-=pod
-
-=over
-
-=item C<process($bug, $attachment, $timestamp, $hr_vars)>
-
-Processes changes to flags.
-
-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).
-
-=back
-
-=cut
-
-sub process {
-    my ($class, $bug, $attachment, $timestamp, $hr_vars) = @_;
-    my $dbh = Bugzilla->dbh;
-    my $cgi = Bugzilla->cgi;
-
-    # 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).
-    $timestamp ||= $dbh->selectrow_array('SELECT NOW()');
-
-    # Take a snapshot of flags before any changes.
-    my @old_summaries = $class->snapshot($bug_id, $attach_id);
+    my ($removed, $added) = diff_arrays($old_summaries, $new_summaries);
+    if (scalar @$removed || scalar @$added) {
+        # Remove flag requester/setter information
+        foreach (@$removed, @$added) { s/^[^:]+:// }
 
-    # Cancel pending requests if we are obsoleting an attachment.
-    if ($attachment && $cgi->param('isobsolete')) {
-        $class->CancelRequests($bug, $attachment);
+        $removed = join(", ", @$removed);
+        $added = join(", ", @$added);
+        return ($removed, $added);
     }
+    return ();
+}
 
-    # Create new flags and update existing flags.
-    my $new_flags = FormToNewFlags($bug, $attachment, $cgi, $hr_vars);
-    foreach my $flag (@$new_flags) { create($flag, $bug, $attachment, $timestamp) }
-    modify($bug, $attachment, $cgi, $timestamp);
+sub update_flags {
+    my ($class, $self, $old_self, $timestamp) = @_;
 
-    # In case the bug's product/component has changed, clear flags that are
-    # no longer valid.
-    my $flag_ids = $dbh->selectcol_arrayref(
-        "SELECT DISTINCT flags.id
-           FROM flags
-     INNER JOIN bugs
-             ON flags.bug_id = bugs.bug_id
-      LEFT JOIN flaginclusions AS i
-             ON flags.type_id = i.type_id 
-            AND (bugs.product_id = i.product_id OR i.product_id IS NULL)
-            AND (bugs.component_id = i.component_id OR i.component_id IS NULL)
-          WHERE bugs.bug_id = ?
-            AND i.type_id IS NULL",
-        undef, $bug_id);
+    my @old_summaries = $class->snapshot($old_self->flags);
+    my %old_flags = map { $_->id => $_ } @{$old_self->flags};
 
-    my $flags = Bugzilla::Flag->new_from_list($flag_ids);
-    foreach my $flag (@$flags) {
-        my $is_retargetted = retarget($flag, $bug);
-        unless ($is_retargetted) {
-            clear($flag, $bug, $flag->attachment);
-            $hr_vars->{'message'} = 'flag_cleared';
+    foreach my $new_flag (@{$self->flags}) {
+        if (!$new_flag->id) {
+            # This is a new flag.
+            my $flag = $class->create($new_flag, $timestamp);
+            $new_flag->{id} = $flag->id;
+            $class->notify($new_flag, undef, $self);
+        }
+        else {
+            $new_flag->update($timestamp);
+            $class->notify($new_flag, $old_flags{$new_flag->id}, $self);
+            delete $old_flags{$new_flag->id};
         }
     }
-
-    $flag_ids = $dbh->selectcol_arrayref(
-        "SELECT DISTINCT flags.id
-        FROM flags, bugs, flagexclusions e
-        WHERE bugs.bug_id = ?
-        AND flags.bug_id = bugs.bug_id
-        AND flags.type_id = e.type_id
-        AND (bugs.product_id = e.product_id OR e.product_id IS NULL)
-        AND (bugs.component_id = e.component_id OR e.component_id IS NULL)",
-        undef, $bug_id);
-
-    $flags = Bugzilla::Flag->new_from_list($flag_ids);
-    foreach my $flag (@$flags) {
-        my $is_retargetted = retarget($flag, $bug);
-        clear($flag, $bug, $flag->attachment) unless $is_retargetted;
+    # These flags have been deleted.
+    foreach my $old_flag (values %old_flags) {
+        $class->notify(undef, $old_flag, $self);
+        $old_flag->remove_from_db();
     }
 
-    # Take a snapshot of flags after changes.
-    my @new_summaries = $class->snapshot($bug_id, $attach_id);
+    # If the bug has been moved into another product or component,
+    # we must also take care of attachment flags which are no longer valid,
+    # as well as all bug flags which haven't been forgotten above.
+    if ($self->isa('Bugzilla::Bug')
+        && ($self->{_old_product_name} || $self->{_old_component_name}))
+    {
+        my @removed = $class->force_cleanup($self);
+        push(@old_summaries, @removed);
+    }
 
-    update_activity($bug_id, $attach_id, $timestamp, \@old_summaries, \@new_summaries);
+    my @new_summaries = $class->snapshot($self->flags);
+    my @changes = $class->update_activity(\@old_summaries, \@new_summaries);
 
-    Bugzilla::Hook::process('flag-end_of_update', { bug       => $bug,
+    Bugzilla::Hook::process('flag-end_of_update', { object    => $self,
                                                     timestamp => $timestamp,
                                                     old_flags => \@old_summaries,
                                                     new_flags => \@new_summaries,
                                                   });
+    return @changes;
 }
 
-sub update_activity {
-    my ($bug_id, $attach_id, $timestamp, $old_summaries, $new_summaries) = @_;
-    my $dbh = Bugzilla->dbh;
+sub retarget {
+    my ($self, $obj) = @_;
 
-    my ($removed, $added) = diff_arrays($old_summaries, $new_summaries);
-    if (scalar @$removed || scalar @$added) {
-        # Remove flag requester/setter information
-        foreach (@$removed, @$added) { s/^[^:]+:// }
+    my @flagtypes = grep { $_->name eq $self->type->name } @{$obj->flag_types};
 
-        $removed = join(", ", @$removed);
-        $added = join(", ", @$added);
-        my $field_id = get_field_id('flagtypes.name');
-        $dbh->do('INSERT INTO bugs_activity
-                  (bug_id, attach_id, who, bug_when, fieldid, removed, added)
-                  VALUES (?, ?, ?, ?, ?, ?, ?)',
-                  undef, ($bug_id, $attach_id, Bugzilla->user->id,
-                  $timestamp, $field_id, $removed, $added));
-
-        $dbh->do('UPDATE bugs SET delta_ts = ? WHERE bug_id = ?',
-                  undef, ($timestamp, $bug_id));
+    my $success = 0;
+    foreach my $flagtype (@flagtypes) {
+        next if !$flagtype->is_active;
+        next if (!$flagtype->is_multiplicable && scalar @{$flagtype->{flags}});
+
+        $self->{type_id} = $flagtype->id;
+        delete $self->{type};
+        $success = 1;
+        last;
     }
+    return $success;
 }
 
-=pod
-
-=over
-
-=item C<create($flag, $bug, $attachment, $timestamp)>
+# In case the bug's product/component has changed, clear flags that are
+# no longer valid.
+sub force_cleanup {
+    my ($class, $bug) = @_;
+    my $dbh = Bugzilla->dbh;
 
-Creates a flag record in the database.
+    my $flag_ids = $dbh->selectcol_arrayref(
+        'SELECT DISTINCT flags.id
+           FROM flags
+          INNER JOIN bugs
+                ON flags.bug_id = bugs.bug_id
+           LEFT JOIN flaginclusions AS i
+                ON flags.type_id = i.type_id
+                AND (bugs.product_id = i.product_id OR i.product_id IS NULL)
+                AND (bugs.component_id = i.component_id OR i.component_id IS NULL)
+          WHERE bugs.bug_id = ? AND i.type_id IS NULL',
+         undef, $bug->id);
 
-=back
+    my @removed = $class->force_retarget($flag_ids, $bug);
 
-=cut
+    $flag_ids = $dbh->selectcol_arrayref(
+        'SELECT DISTINCT flags.id
+           FROM flags, bugs, flagexclusions e
+          WHERE bugs.bug_id = ?
+                AND flags.bug_id = bugs.bug_id
+                AND flags.type_id = e.type_id
+                AND (bugs.product_id = e.product_id OR e.product_id IS NULL)
+                AND (bugs.component_id = e.component_id OR e.component_id IS NULL)',
+         undef, $bug->id);
+
+    push(@removed , $class->force_retarget($flag_ids, $bug));
+    return @removed;
+}
 
-sub create {
-    my ($flag, $bug, $attachment, $timestamp) = @_;
+sub force_retarget {
+    my ($class, $flag_ids, $bug) = @_;
     my $dbh = Bugzilla->dbh;
 
-    my $attach_id = $attachment ? $attachment->id : undef;
-    my $requestee_id;
-    # Be careful! At this point, $flag is *NOT* yet an object!
-    $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, $bug->bug_id,
-                      $attach_id, $requestee_id, $flag->{'setter'}->id,
-                      $flag->{'status'}, $timestamp, $timestamp));
-
-    # Now that the new flag has been added to the DB, create a real flag object.
-    # This is required to call notify() correctly.
-    my $flag_id = $dbh->bz_last_key('flags', 'id');
-    $flag = new Bugzilla::Flag($flag_id);
-
-    # Send an email notifying the relevant parties about the flag creation.
-    if ($flag->requestee && $flag->requestee->wants_mail([EVT_FLAG_REQUESTED])) {
-        $flag->{'addressee'} = $flag->requestee;
+    my $flags = $class->new_from_list($flag_ids);
+    my @removed;
+    foreach my $flag (@$flags) {
+        # $bug is undefined when e.g. editing inclusion and exclusion lists.
+        my $obj = $flag->attachment || $bug || $flag->bug;
+        my $is_retargetted = $flag->retarget($obj);
+        if ($is_retargetted) {
+            $dbh->do('UPDATE flags SET type_id = ? WHERE id = ?',
+                     undef, ($flag->type_id, $flag->id));
+        }
+        else {
+            # Track deleted attachment flags.
+            push(@removed, $class->snapshot([$flag])) if $flag->attach_id;
+            $class->notify(undef, $flag, $bug || $flag->bug);
+            $flag->remove_from_db();
+        }
     }
-
-    notify($flag, $bug, $attachment);
-
-    # Return the new flag object.
-    return $flag;
+    return @removed;
 }
 
-=pod
-
-=over
-
-=item C<modify($bug, $attachment, $cgi, $timestamp)>
-
-Modifies flags in the database when a user changes them.
-
-=back
-
-=cut
-
-sub modify {
-    my ($bug, $attachment, $cgi, $timestamp) = @_;
-    my $setter = Bugzilla->user;
-    my $dbh = Bugzilla->dbh;
-
-    # Extract a list of flags from the form data.
-    my @ids = map(/^flag-(\d+)$/ ? $1 : (), $cgi->param());
+###############################
+####      Validators     ######
+###############################
 
-    # Loop over flags and update their record in the database if necessary.
-    # Two kinds of changes can happen to a flag: it can be set to a different
-    # state, and someone else can be asked to set it.  We take care of both
-    # those changes.
-    my @flags;
-    foreach my $id (@ids) {
-        my $flag = new Bugzilla::Flag($id);
-        # If the flag no longer exists, ignore it.
-        next unless $flag;
+sub _set_requestee {
+    my ($self, $requestee, $attachment, $skip_requestee_on_error) = @_;
 
-        my $status = $cgi->param("flag-$id");
+    # Used internally to check if the requestee is retargetting the request.
+    $self->{_old_requestee_id} = $self->requestee ? $self->requestee->id : 0;
+    $self->{requestee} =
+      $self->_check_requestee($requestee, $attachment, $skip_requestee_on_error);
 
-        # If the user entered more than one name into the requestee field
-        # (i.e. they want more than one person to set the flag) we can reuse
-        # the existing flag for the first person (who may well be the existing
-        # requestee), but we have to create new flags for each additional.
-        my @requestees = $cgi->param("requestee-$id");
-        my $requestee_email;
-        if ($status eq "?"
-            && scalar(@requestees) > 1
-            && $flag->type->is_multiplicable)
-        {
-            # The first person, for which we'll reuse the existing flag.
-            $requestee_email = shift(@requestees);
+    $self->{requestee_id} =
+      $self->{requestee} ? $self->{requestee}->id : undef;
+}
 
-            # Create new flags like the existing one for each additional person.
-            foreach my $login (@requestees) {
-                create({ type      => $flag->type,
-                         setter    => $setter, 
-                         status    => "?",
-                         requestee => new Bugzilla::User({ name => $login }) },
-                       $bug, $attachment, $timestamp);
-            }
-        }
-        else {
-            $requestee_email = trim($cgi->param("requestee-$id") || '');
-        }
+sub _set_setter {
+    my ($self, $setter) = @_;
 
-        # Ignore flags the user didn't change. There are two components here:
-        # either the status changes (trivial) or the requestee changes.
-        # Change of either field will cause full update of the flag.
+    $self->set('setter', $setter);
+    $self->{setter_id} = $self->setter->id;
+}
 
-        my $status_changed = ($status ne $flag->status);
+sub _set_status {
+    my ($self, $status) = @_;
 
-        # Requestee is considered changed, if all of the following apply:
-        # 1. Flag status is '?' (requested)
-        # 2. Flag can have a requestee
-        # 3. The requestee specified on the form is different from the 
-        #    requestee specified in the db.
+    # Store the old flag status. It's needed by _check_setter().
+    $self->{_old_status} = $self->status;
+    $self->set('status', $status);
+}
 
-        my $old_requestee = $flag->requestee ? $flag->requestee->login : '';
+sub _check_requestee {
+    my ($self, $requestee, $attachment, $skip_requestee_on_error) = @_;
 
-        my $requestee_changed = 
-          ($status eq "?" && 
-           $flag->type->is_requesteeble &&
-           $old_requestee ne $requestee_email);
+    # If the flag status is not "?", then no requestee can be defined.
+    return undef if ($self->status ne '?');
 
-        next unless ($status_changed || $requestee_changed);
+    # Store this value before updating the flag object.
+    my $old_requestee = $self->requestee ? $self->requestee->login : '';
 
-        # Since the status is validated, we know it's safe, but it's still
-        # tainted, so we have to detaint it before using it in a query.
-        trick_taint($status);
+    if ($self->status eq '?' && $requestee) {
+        $requestee = Bugzilla::User->check($requestee);
+    }
+    else {
+        undef $requestee;
+    }
 
-        if ($status eq '+' || $status eq '-') {
-            $dbh->do('UPDATE flags
-                         SET setter_id = ?, requestee_id = NULL,
-                             status = ?, modification_date = ?
-                       WHERE id = ?',
-                       undef, ($setter->id, $status, $timestamp, $flag->id));
-
-            # If the status of the flag was "?", we have to notify
-            # the requester (if he wants to).
-            my $requester;
-            if ($flag->status eq '?') {
-                $requester = $flag->setter;
-                $flag->{'requester'} = $requester;
+    if ($requestee && $requestee->login ne $old_requestee) {
+        # Make sure the user didn't specify a requestee unless the flag
+        # is specifically requestable. For existing flags, if the requestee
+        # was set before the flag became specifically unrequestable, the
+        # user can either remove him or leave him alone.
+        ThrowCodeError('flag_requestee_disabled', { type => $self->type })
+          if !$self->type->is_requesteeble;
+
+        # Make sure the requestee can see the bug.
+        # Note that can_see_bug() will query the DB, so if the bug
+        # is being added/removed from some groups and these changes
+        # haven't been committed to the DB yet, they won't be taken
+        # into account here. In this case, old restrictions matters.
+        if (!$requestee->can_see_bug($self->bug_id)) {
+            if ($skip_requestee_on_error) {
+                undef $requestee;
             }
-            # Now update the flag object with its new values.
-            $flag->{'setter'} = $setter;
-            $flag->{'requestee'} = undef;
-            $flag->{'requestee_id'} = undef;
-            $flag->{'status'} = $status;
-
-            # Send an email notifying the relevant parties about the fulfillment,
-            # including the requester.
-            if ($requester && $requester->wants_mail([EVT_REQUESTED_FLAG])) {
-                $flag->{'addressee'} = $requester;
+            else {
+                ThrowUserError('flag_requestee_unauthorized',
+                               { flag_type  => $self->type,
+                                 requestee  => $requestee,
+                                 bug_id     => $self->bug_id,
+                                 attach_id  => $self->attach_id });
             }
-
-            notify($flag, $bug, $attachment);
         }
-        elsif ($status eq '?') {
-            # If the one doing the change is the requestee, then this means he doesn't
-            # want to reply to the request and he simply reassigns the request to
-            # someone else. In this case, we keep the requester unaltered.
-            my $new_setter = $setter;
-            if ($flag->requestee && $flag->requestee->id == $setter->id) {
-                $new_setter = $flag->setter;
-            }
-
-            # Get the requestee, if any.
-            my $requestee_id;
-            if ($requestee_email) {
-                $requestee_id = login_to_id($requestee_email);
-                $flag->{'requestee'} = new Bugzilla::User($requestee_id);
-                $flag->{'requestee_id'} = $requestee_id;
+        # Make sure the requestee can see the private attachment.
+        elsif ($self->attach_id && $attachment->isprivate && !$requestee->is_insider) {
+            if ($skip_requestee_on_error) {
+                undef $requestee;
             }
             else {
-                # If the status didn't change but we only removed the
-                # requestee, we have to clear the requestee field.
-                $flag->{'requestee'} = undef;
-                $flag->{'requestee_id'} = undef;
-            }
-
-            # Update the database with the changes.
-            $dbh->do('UPDATE flags
-                         SET setter_id = ?, requestee_id = ?,
-                             status = ?, modification_date = ?
-                       WHERE id = ?',
-                       undef, ($new_setter->id, $requestee_id, $status,
-                               $timestamp, $flag->id));
-
-            # Now update the flag object with its new values.
-            $flag->{'setter'} = $new_setter;
-            $flag->{'status'} = $status;
-
-            # Send an email notifying the relevant parties about the request.
-            if ($flag->requestee && $flag->requestee->wants_mail([EVT_FLAG_REQUESTED])) {
-                $flag->{'addressee'} = $flag->requestee;
+                ThrowUserError('flag_requestee_unauthorized_attachment',
+                               { flag_type  => $self->type,
+                                 requestee  => $requestee,
+                                 bug_id     => $self->bug_id,
+                                 attach_id  => $self->attach_id });
             }
-
-            notify($flag, $bug, $attachment);
         }
-        elsif ($status eq 'X') {
-            clear($flag, $bug, $attachment);
+        # Make sure the user is allowed to set the flag.
+        elsif (!$requestee->can_set_flag($self->type)) {
+            if ($skip_requestee_on_error) {
+                undef $requestee;
+            }
+            else {
+                ThrowUserError('flag_requestee_needs_privs',
+                               {'requestee' => $requestee,
+                                'flagtype'  => $self->type});
+            }
         }
-
-        push(@flags, $flag);
     }
-
-    return \@flags;
+    return $requestee;
 }
 
-=pod
-
-=over
-
-=item C<retarget($flag, $bug)>
+sub _check_setter {
+    my ($self, $setter) = @_;
 
-Change the type of the flag, if possible. The new flag type must have
-the same name as the current flag type, must exist in the product and
-component the bug is in, and the current settings of the flag must pass
-validation. If no such flag type can be found, the type remains unchanged.
+    # By default, the currently logged in user is the setter.
+    $setter ||= Bugzilla->user;
+    (blessed($setter) && $setter->isa('Bugzilla::User') && $setter->id)
+      || ThrowCodeError('invalid_user');
 
-Retargetting flags is a good way to keep flags when moving bugs from one
-product where a flag type is available to another product where the flag
-type is unavailable, but another flag type having the same name exists.
-Most of the time, if they have the same name, this means that they have
-the same meaning, but with different settings.
+    # set_status() has already been called. So this refers
+    # to the new flag status.
+    my $status = $self->status;
 
-=back
-
-=cut
+    # Make sure the user is authorized to modify flags, see bug 180879:
+    # - The flag exists and is unchanged.
+    # - Users in the request_group can clear pending requests and set flags
+    #   and can rerequest set flags.
+    # - Users in the grant_group can set/clear flags, including "+" and "-".
+    unless (($status eq $self->{_old_status})
+            || (($status eq 'X' || $status eq '?')
+                && $setter->can_request_flag($self->type))
+            || $setter->can_set_flag($self->type))
+    {
+        ThrowUserError('flag_update_denied',
+                        { name       => $self->type->name,
+                          status     => $status,
+                          old_status => $self->{_old_status} });
+    }
 
-sub retarget {
-    my ($flag, $bug) = @_;
-    my $dbh = Bugzilla->dbh;
+    # If the requester is retargetting the request, we don't
+    # update the setter, so that the setter gets the notification.
+    if ($status eq '?' && $self->{_old_requestee_id} == $setter->id) {
+        return $self->setter;
+    }
+    return $setter;
+}
 
-    # We are looking for flagtypes having the same name as the flagtype
-    # to which the current flag belongs, and being in the new product and
-    # component of the bug.
-    my $flagtypes = Bugzilla::FlagType::match(
-                        {'name'         => $flag->name,
-                         'target_type'  => $flag->type->target_type,
-                         'is_active'    => 1,
-                         'product_id'   => $bug->product_id,
-                         'component_id' => $bug->component_id});
-
-    # If we found no flagtype, the flag will be deleted.
-    return 0 unless scalar(@$flagtypes);
-
-    # If we found at least one, change the type of the flag,
-    # assuming the setter/requester is allowed to set/request flags
-    # belonging to this flagtype.
-    my $requestee = $flag->requestee ? [$flag->requestee->login] : [];
-    my $is_private = ($flag->attachment) ? $flag->attachment->isprivate : 0;
-    my $is_retargetted = 0;
-
-    foreach my $flagtype (@$flagtypes) {
-        # Get the number of flags of this type already set for this target.
-        my $has_flags = __PACKAGE__->count(
-            { 'type_id'     => $flagtype->id,
-              'bug_id'      => $bug->bug_id,
-              'attach_id'   => $flag->attach_id });
+sub _check_status {
+    my ($self, $status) = @_;
 
-        # Do not create a new flag of this type if this flag type is
-        # not multiplicable and already has a flag set.
-        next if (!$flagtype->is_multiplicable && $has_flags);
-
-        # Check user privileges.
-        my $error_mode_cache = Bugzilla->error_mode;
-        Bugzilla->error_mode(ERROR_MODE_DIE);
-        eval {
-            _validate(undef, $flagtype, $flag->status, $flag->setter,
-                      $requestee, $is_private, $bug->bug_id, $flag->attach_id);
-        };
-        Bugzilla->error_mode($error_mode_cache);
-        # If the validation failed, then we cannot use this flagtype.
-        next if ($@);
-
-        # Checks are successful, we can retarget the flag to this flagtype.
-        $dbh->do('UPDATE flags SET type_id = ? WHERE id = ?',
-                  undef, ($flagtype->id, $flag->id));
-
-        $is_retargetted = 1;
-        last;
+    # - Make sure the status is valid.
+    # - Make sure the user didn't request the flag unless it's requestable.
+    #   If the flag existed and was requested before it became unrequestable,
+    #   leave it as is.
+    if (!grep($status eq $_ , qw(X + - ?))
+        || ($status eq '?' && $self->status ne '?' && !$self->type->is_requestable))
+    {
+        ThrowCodeError('flag_status_invalid', { id     => $self->id,
+                                                status => $status });
     }
-    return $is_retargetted;
+    return $status;
 }
 
+######################################################################
+# Utility Functions
+######################################################################
+
 =pod
 
 =over
 
-=item C<clear($flag, $bug, $attachment)>
+=item C<extract_flags_from_cgi($bug, $attachment, $hr_vars)>
 
-Remove a flag from the DB.
+Checks whether or not there are new flags to create and returns an
+array of hashes. This array is then passed to Flag::create().
 
 =back
 
 =cut
 
-sub clear {
-    my ($flag, $bug, $attachment) = @_;
-    my $dbh = Bugzilla->dbh;
+sub extract_flags_from_cgi {
+    my ($class, $bug, $attachment, $vars, $skip) = @_;
+    my $cgi = Bugzilla->cgi;
 
-    $dbh->do('DELETE FROM flags WHERE id = ?', undef, $flag->id);
+    my $match_status = Bugzilla::User::match_field($cgi, {
+        '^requestee(_type)?-(\d+)$' => { 'type' => 'multi' },
+    }, $skip);
 
-    # If we cancel a pending request, we have to notify the requester
-    # (if he wants to).
-    my $requester;
-    if ($flag->status eq '?') {
-        $requester = $flag->setter;
-        $flag->{'requester'} = $requester;
+    $vars->{'match_field'} = 'requestee';
+    if ($match_status == USER_MATCH_FAILED) {
+        $vars->{'message'} = 'user_match_failed';
     }
-
-    # Now update the flag object to its new values. The last
-    # requester/setter and requestee are kept untouched (for the
-    # record). Else we could as well delete the flag completely.
-    $flag->{'exists'} = 0;    
-    $flag->{'status'} = "X";
-
-    if ($requester && $requester->wants_mail([EVT_REQUESTED_FLAG])) {
-        $flag->{'addressee'} = $requester;
+    elsif ($match_status == USER_MATCH_MULTIPLE) {
+        $vars->{'message'} = 'user_match_multiple';
     }
 
-    notify($flag, $bug, $attachment);
-}
-
-
-######################################################################
-# Utility Functions
-######################################################################
-
-=pod
+    # Extract a list of flag type IDs from field names.
+    my @flagtype_ids = map(/^flag_type-(\d+)$/ ? $1 : (), $cgi->param());
+    @flagtype_ids = grep($cgi->param("flag_type-$_") ne 'X', @flagtype_ids);
 
-=over
+    # Extract a list of existing flag IDs.
+    my @flag_ids = map(/^flag-(\d+)$/ ? $1 : (), $cgi->param());
 
-=item C<FormToNewFlags($bug, $attachment, $cgi, $hr_vars)>
+    return () if (!scalar(@flagtype_ids) && !scalar(@flag_ids));
 
-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().
+    my (@new_flags, @flags);
+    foreach my $flag_id (@flag_ids) {
+        my $flag = $class->new($flag_id);
+        # If the flag no longer exists, ignore it.
+        next unless $flag;
 
-=back
+        my $status = $cgi->param("flag-$flag_id");
 
-=cut
+        # If the user entered more than one name into the requestee field
+        # (i.e. they want more than one person to set the flag) we can reuse
+        # the existing flag for the first person (who may well be the existing
+        # requestee), but we have to create new flags for each additional requestee.
+        my @requestees = $cgi->param("requestee-$flag_id");
+        my $requestee_email;
+        if ($status eq "?"
+            && scalar(@requestees) > 1
+            && $flag->type->is_multiplicable)
+        {
+            # The first person, for which we'll reuse the existing flag.
+            $requestee_email = shift(@requestees);
 
-sub FormToNewFlags {
-    my ($bug, $attachment, $cgi, $hr_vars) = @_;
-    my $dbh = Bugzilla->dbh;
-    my $setter = Bugzilla->user;
-    
-    # Extract a list of flag type IDs from field names.
-    my @type_ids = map(/^flag_type-(\d+)$/ ? $1 : (), $cgi->param());
-    @type_ids = grep($cgi->param("flag_type-$_") ne 'X', @type_ids);
+            # Create new flags like the existing one for each additional person.
+            foreach my $login (@requestees) {
+                push(@new_flags, { type_id   => $flag->type_id,
+                                   status    => "?",
+                                   requestee => $login,
+                                   skip_roe  => $skip });
+            }
+        }
+        elsif ($status eq "?" && scalar(@requestees)) {
+            # If there are several requestees and the flag type is not multiplicable,
+            # this will fail. But that's the job of the validator to complain. All
+            # we do here is to extract and convert data from the CGI.
+            $requestee_email = trim($cgi->param("requestee-$flag_id") || '');
+        }
 
-    return () unless scalar(@type_ids);
+        push(@flags, { id        => $flag_id,
+                       status    => $status,
+                       requestee => $requestee_email,
+                       skip_roe  => $skip });
+    }
 
     # Get a list of active flag types available for this product/component.
     my $flag_types = Bugzilla::FlagType::match(
@@ -1002,15 +857,14 @@ sub FormToNewFlags {
           'component_id' => $bug->{'component_id'},
           'is_active'    => 1 });
 
-    foreach my $type_id (@type_ids) {
+    foreach my $flagtype_id (@flagtype_ids) {
         # Checks if there are unexpected flags for the product/component.
-        if (!scalar(grep { $_->id == $type_id } @$flag_types)) {
-            $hr_vars->{'message'} = 'unexpected_flag_types';
+        if (!scalar(grep { $_->id == $flagtype_id } @$flag_types)) {
+            $vars->{'message'} = 'unexpected_flag_types';
             last;
         }
     }
 
-    my @flags;
     foreach my $flag_type (@$flag_types) {
         my $type_id = $flag_type->id;
 
@@ -1019,10 +873,10 @@ sub FormToNewFlags {
         next unless ($flag_type->target_type eq 'bug' xor $attachment);
 
         # We are only interested in flags the user tries to create.
-        next unless scalar(grep { $_ == $type_id } @type_ids);
+        next unless scalar(grep { $_ == $type_id } @flagtype_ids);
 
         # Get the number of flags of this type already set for this target.
-        my $has_flags = __PACKAGE__->count(
+        my $has_flags = $class->count(
             { 'type_id'     => $type_id,
               'target_type' => $attachment ? 'attachment' : 'bug',
               'bug_id'      => $bug->bug_id,
@@ -1036,65 +890,23 @@ sub FormToNewFlags {
         trick_taint($status);
 
         my @logins = $cgi->param("requestee_type-$type_id");
-        if ($status eq "?" && scalar(@logins) > 0) {
+        if ($status eq "?" && scalar(@logins)) {
             foreach my $login (@logins) {
-                push (@flags, { type      => $flag_type ,
-                                setter    => $setter , 
-                                status    => $status ,
-                                requestee => 
-                                    new Bugzilla::User({ name => $login }) });
+                push (@new_flags, { type_id   => $type_id,
+                                    status    => $status,
+                                    requestee => $login,
+                                    skip_roe  => $skip });
                 last unless $flag_type->is_multiplicable;
             }
         }
         else {
-            push (@flags, { type   => $flag_type ,
-                            setter => $setter , 
-                            status => $status });
+            push (@new_flags, { type_id => $type_id,
+                                status  => $status });
         }
     }
 
-    # Return the list of flags.
-    return \@flags;
-}
-
-# This is a helper to set flags on a new bug or attachment.
-# For existing bugs and attachments, errors must be reported.
-sub set_flags {
-    my ($class, $bug, $attachment, $timestamp, $vars) = @_;
-    my $cgi = Bugzilla->cgi;
-
-    # The order of these function calls is important, as Flag::validate
-    # assumes User::match_field has ensured that the
-    # values in the requestee fields are legitimate user email addresses.
-    my $match_status = Bugzilla::User::match_field($cgi, {
-        '^requestee(_type)?-(\d+)$' => { 'type' => 'multi' },
-    }, MATCH_SKIP_CONFIRM);
-
-    $vars->{'match_field'} = 'requestee';
-    if ($match_status == USER_MATCH_FAILED) {
-        $vars->{'message'} = 'user_match_failed';
-    }
-    elsif ($match_status == USER_MATCH_MULTIPLE) {
-        $vars->{'message'} = 'user_match_multiple';
-    }
-
-    # 1. Add flags, if any. To avoid dying if something goes wrong
-    # while processing flags, we will eval() flag validation.
-    #
-    # 2. Flag::validate() should not detect any reference to existing flags
-    # when creating a new attachment. Setting the third param to -1 will
-    # force this function to check this point.
-    my $error_mode_cache = Bugzilla->error_mode;
-    Bugzilla->error_mode(ERROR_MODE_DIE);
-    eval {
-        validate($bug->bug_id, $attachment ? -1 : undef, SKIP_REQUESTEE_ON_ERROR);
-        $class->process($bug, $attachment, $timestamp, $vars);
-    };
-    Bugzilla->error_mode($error_mode_cache);
-    if ($@) {
-        $vars->{'message'} = 'flag_creation_failed';
-        $vars->{'flag_creation_error'} = $@;
-    }
+    # Return the list of flags to update and/or to create.
+    return (\@flags, \@new_flags);
 }
 
 =pod
@@ -1111,10 +923,41 @@ or deleted.
 =cut
 
 sub notify {
-    my ($flag, $bug, $attachment) = @_;
+    my ($class, $flag, $old_flag, $obj) = @_;
 
-    # There is nobody to notify.
-    return unless ($flag->{'addressee'} || $flag->type->cc_list);
+    my ($bug, $attachment);
+    if (blessed($obj) && $obj->isa('Bugzilla::Attachment')) {
+        $attachment = $obj;
+        $bug = $attachment->bug;
+    }
+    elsif (blessed($obj) && $obj->isa('Bugzilla::Bug')) {
+        $bug = $obj;
+    }
+    else {
+        # Not a good time to throw an error.
+        return;
+    }
+
+    my $addressee;
+    # If the flag is set to '?', maybe the requestee wants a notification.
+    if ($flag && $flag->requestee_id
+        && (!$old_flag || ($old_flag->requestee_id || 0) != $flag->requestee_id))
+    {
+        if ($flag->requestee->wants_mail([EVT_FLAG_REQUESTED])) {
+            $addressee = $flag->requestee;
+        }
+    }
+    elsif ($old_flag && $old_flag->status eq '?'
+           && (!$flag || $flag->status ne '?'))
+    {
+        if ($old_flag->setter->wants_mail([EVT_REQUESTED_FLAG])) {
+            $addressee = $old_flag->setter;
+        }
+    }
+
+    my $cc_list = $flag ? $flag->type->cc_list : $old_flag->type->cc_list;
+    # Is there someone to notify?
+    return unless ($addressee || $cc_list);
 
     # 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
@@ -1124,7 +967,7 @@ sub notify {
     my $attachment_is_private = $attachment ? $attachment->isprivate : undef;
 
     my %recipients;
-    foreach my $cc (split(/[, ]+/, $flag->type->cc_list)) {
+    foreach my $cc (split(/[, ]+/, $cc_list)) {
         my $ccuser = new Bugzilla::User({ name => $cc });
         next if (scalar(@bug_in_groups) && (!$ccuser || !$ccuser->can_see_bug($bug->bug_id)));
         next if $attachment_is_private && (!$ccuser || !$ccuser->is_insider);
@@ -1134,16 +977,15 @@ sub notify {
     }
 
     # Only notify if the addressee is allowed to receive the email.
-    if ($flag->{'addressee'} && $flag->{'addressee'}->email_enabled) {
-        $recipients{$flag->{'addressee'}->email} = $flag->{'addressee'};
+    if ($addressee && $addressee->email_enabled) {
+        $recipients{$addressee->email} = $addressee;
     }
     # Process and send notification for each recipient.
     # If there are users in the CC list who don't have an account,
     # use the default language for email notifications.
     my $default_lang;
     if (grep { !$_ } values %recipients) {
-        my $default_user = new Bugzilla::User();
-        $default_lang = $default_user->settings->{'lang'}->{'value'};
+        $default_lang = Bugzilla::User->new()->settings->{'lang'}->{'value'};
     }
 
     foreach my $to (keys %recipients) {
@@ -1152,6 +994,7 @@ sub notify {
         my $thread_user_id = $recipients{$to} ? $recipients{$to}->id : 0;
     
         my $vars = { 'flag'            => $flag,
+                     'old_flag'        => $old_flag,
                      'to'              => $to,
                      'bug'             => $bug,
                      'attachment'      => $attachment,
@@ -1170,43 +1013,12 @@ sub notify {
     }
 }
 
-# Cancel all request flags from the attachment being obsoleted.
-sub CancelRequests {
-    my ($class, $bug, $attachment, $timestamp) = @_;
-    my $dbh = Bugzilla->dbh;
-
-    my $request_ids =
-        $dbh->selectcol_arrayref("SELECT flags.id
-                                  FROM flags
-                                  LEFT JOIN attachments ON flags.attach_id = attachments.attach_id
-                                  WHERE flags.attach_id = ?
-                                  AND flags.status = '?'
-                                  AND attachments.isobsolete = 0",
-                                  undef, $attachment->id);
-
-    return if (!scalar(@$request_ids));
-
-    # Take a snapshot of flags before any changes.
-    my @old_summaries = $class->snapshot($bug->bug_id, $attachment->id)
-        if ($timestamp);
-    my $flags = Bugzilla::Flag->new_from_list($request_ids);
-    foreach my $flag (@$flags) { 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 = $class->snapshot($bug->bug_id, $attachment->id);
-    update_activity($bug->bug_id, $attachment->id, $timestamp,
-                    \@old_summaries, \@new_summaries);
-}
-
 # This is an internal function used by $bug->flag_types
 # and $attachment->flag_types to collect data about available
 # flag types and existing flags set on them. You should never
 # call this function directly.
 sub _flag_types {
-    my $vars = shift;
+    my ($class, $vars) = @_;
 
     my $target_type = $vars->{target_type};
     my $flags;
@@ -1214,15 +1026,15 @@ sub _flag_types {
     # Retrieve all existing flags for this bug/attachment.
     if ($target_type eq 'bug') {
         my $bug_id = delete $vars->{bug_id};
-        $flags = Bugzilla::Flag->match({target_type => 'bug', bug_id => $bug_id});
+        $flags = $class->match({target_type => 'bug', bug_id => $bug_id});
     }
     elsif ($target_type eq 'attachment') {
         my $attach_id = delete $vars->{attach_id};
-        $flags = Bugzilla::Flag->match({attach_id => $attach_id});
+        $flags = $class->match({attach_id => $attach_id});
     }
     else {
         ThrowCodeError('bad_arg', {argument => 'target_type',
-                                   function => 'Bugzilla::Flag::_flag_types'});
+                                   function => $class . '->_flag_types'});
     }
 
     # Get all available flag types for the given product and component.
@@ -1231,10 +1043,11 @@ sub _flag_types {
     $_->{flags} = [] foreach @$flag_types;
     my %flagtypes = map { $_->id => $_ } @$flag_types;
 
-    # Group existing flags per type.
-    # Call the internal 'type_id' variable instead of the method
-    # to not create a flagtype object.
-    push(@{$flagtypes{$_->{type_id}}->{flags}}, $_) foreach @$flags;
+    # Group existing flags per type, and skip those becoming invalid
+    # (which can happen when a bug is being moved into a new product
+    # or component).
+    @$flags = grep { exists $flagtypes{$_->type_id} } @$flags;
+    push(@{$flagtypes{$_->type_id}->{flags}}, $_) foreach @$flags;
 
     return [sort {$a->sortkey <=> $b->sortkey || $a->name cmp $b->name} values %flagtypes];
 }
diff --git a/Bugzilla/Hook.pm b/Bugzilla/Hook.pm
index 5bc2e7716..a8f61a415 100644
--- a/Bugzilla/Hook.pm
+++ b/Bugzilla/Hook.pm
@@ -377,7 +377,7 @@ Params:
 
 =head2 flag-end_of_update
 
-This happens at the end of L<Bugzilla::Flag/process>, after all other changes
+This happens at the end of L<Bugzilla::Flag/update_flags>, after all other changes
 are made to the database and after emails are sent. It gives you a before/after
 snapshot of flags so you can react to specific flag changes.
 This generally occurs inside a database transaction.
@@ -389,7 +389,7 @@ Params:
 
 =over
 
-=item C<bug> - The changed bug object.
+=item C<object> - The changed bug or attachment object.
 
 =item C<timestamp> - The timestamp used for all updates in this transaction.
 
diff --git a/attachment.cgi b/attachment.cgi
index f6549841f..b62e4f23c 100755
--- a/attachment.cgi
+++ b/attachment.cgi
@@ -469,26 +469,16 @@ sub insert {
          store_in_file => scalar $cgi->param('bigfile'),
          });
 
-    Bugzilla::Flag->set_flags($bug, $attachment, $timestamp, $vars);
-
-    my $fieldid = get_field_id('attachments.isobsolete');
-
     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($bug, $obsolete_attachment, $timestamp);
-
-        $dbh->do('UPDATE attachments SET isobsolete = 1, modification_time = ?
-                  WHERE attach_id = ?',
-                 undef, ($timestamp, $obsolete_attachment->id));
-
-        $dbh->do('INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when,
-                                             fieldid, removed, added)
-                  VALUES (?,?,?,?,?,?,?)',
-                  undef, ($bug->bug_id, $obsolete_attachment->id, $user->id,
-                          $timestamp, $fieldid, 0, 1));
+        $obsolete_attachment->set_is_obsolete(1);
+        $obsolete_attachment->update($timestamp);
     }
 
+    my ($flags, $new_flags) = Bugzilla::Flag->extract_flags_from_cgi(
+                                  $bug, $attachment, $vars, SKIP_REQUESTEE_ON_ERROR);
+    $attachment->set_flags($flags, $new_flags);
+    $attachment->update($timestamp);
+
     # Insert a comment about the new attachment into the database.
     my $comment = "Created an attachment (id=" . $attachment->id . ")\n" .
                   $attachment->description . "\n";
@@ -627,27 +617,18 @@ sub update {
         $bug->add_comment($comment, { isprivate => $attachment->isprivate });
     }
 
-    # The order of these function calls is important, as Flag::validate
-    # assumes User::match_field has ensured that the values in the
-    # requestee fields are legitimate user email addresses.
-    Bugzilla::User::match_field($cgi, {
-        '^requestee(_type)?-(\d+)$' => { 'type' => 'multi' }
-    });
-    Bugzilla::Flag::validate($bug->id, $attachment->id);
-
+    my ($flags, $new_flags) = Bugzilla::Flag->extract_flags_from_cgi($bug, $attachment, $vars);
+    $attachment->set_flags($flags, $new_flags);
 
     # Figure out when the changes were made.
-    my ($timestamp) = $dbh->selectrow_array("SELECT NOW()");
-    
-    # Update flags.  We have to do this before committing changes
-    # 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($bug, $attachment, $timestamp, $vars);
+    my $timestamp = $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
 
-    $attachment->update($timestamp);
+    my $changes = $attachment->update($timestamp);
+    # If there are changes, we updated delta_ts in the DB. We have to
+    # reflect this change in the bug object.
+    $bug->{delta_ts} = $timestamp if scalar(keys %$changes);
     # Commit the comment, if any.
-    $bug->update();
+    $bug->update($timestamp);
 
     # Commit the transaction now that we are finished updating the database.
     $dbh->bz_commit_transaction();
diff --git a/editflagtypes.cgi b/editflagtypes.cgi
index 4dbaae573..b730ae2e5 100755
--- a/editflagtypes.cgi
+++ b/editflagtypes.cgi
@@ -413,11 +413,7 @@ sub update {
                                               WHERE flags.type_id = ?
                                                 AND i.type_id IS NULL',
                                              undef, $id);
-    my $flags = Bugzilla::Flag->new_from_list($flag_ids);
-    foreach my $flag (@$flags) {
-        my $bug = new Bugzilla::Bug($flag->bug_id);
-        Bugzilla::Flag::clear($flag, $bug, $flag->attachment);
-    }
+    Bugzilla::Flag->force_retarget($flag_ids);
 
     $flag_ids = $dbh->selectcol_arrayref('SELECT DISTINCT flags.id
                                             FROM flags
@@ -431,11 +427,7 @@ sub update {
                                              AND (bugs.component_id = e.component_id
                                                   OR e.component_id IS NULL)',
                                           undef, $id);
-    $flags = Bugzilla::Flag->new_from_list($flag_ids);
-    foreach my $flag (@$flags) {
-        my $bug = new Bugzilla::Bug($flag->bug_id);
-        Bugzilla::Flag::clear($flag, $bug, $flag->attachment);
-    }
+    Bugzilla::Flag->force_retarget($flag_ids);
 
     # Now silently remove requestees from flags which are no longer
     # specifically requestable.
diff --git a/editusers.cgi b/editusers.cgi
index 3d8b66184..b415fd0b0 100755
--- a/editusers.cgi
+++ b/editusers.cgi
@@ -481,35 +481,36 @@ if ($action eq 'search') {
     my $sth_set_bug_timestamp =
         $dbh->prepare('UPDATE bugs SET delta_ts = ? WHERE bug_id = ?');
 
-    # Reference removals which need LogActivityEntry.
-    my $statement_flagupdate = 'UPDATE flags set requestee_id = NULL
-                                 WHERE bug_id = ?
-                                   AND attach_id %s
-                                   AND requestee_id = ?';
-    my $sth_flagupdate_attachment =
-        $dbh->prepare(sprintf($statement_flagupdate, '= ?'));
-    my $sth_flagupdate_bug =
-        $dbh->prepare(sprintf($statement_flagupdate, 'IS NULL'));
-
-    my $buglist = $dbh->selectall_arrayref('SELECT DISTINCT bug_id, attach_id
-                                              FROM flags
-                                             WHERE requestee_id = ?',
-                                           undef, $otherUserID);
-
-    foreach (@$buglist) {
-        my ($bug_id, $attach_id) = @$_;
-        my @old_summaries = Bugzilla::Flag->snapshot($bug_id, $attach_id);
-        if ($attach_id) {
-            $sth_flagupdate_attachment->execute($bug_id, $attach_id, $otherUserID);
+    my $sth_updateFlag = $dbh->prepare('INSERT INTO bugs_activity
+                  (bug_id, attach_id, who, bug_when, fieldid, removed, added)
+                  VALUES (?, ?, ?, ?, ?, ?, ?)');
+
+    # Flags
+    my $flag_ids =
+      $dbh->selectcol_arrayref('SELECT id FROM flags WHERE requestee_id = ?',
+                                undef, $otherUserID);
+
+    my $flags = Bugzilla::Flag->new_from_list($flag_ids);
+
+    $dbh->do('UPDATE flags SET requestee_id = NULL, modification_date = ?
+              WHERE requestee_id = ?', undef, ($timestamp, $otherUserID));
+
+    # We want to remove the requestee but leave the requester alone,
+    # so we have to log these changes manually.
+    my %bugs;
+    push(@{$bugs{$_->bug_id}->{$_->attach_id || 0}}, $_) foreach @$flags;
+    my $fieldid = get_field_id('flagtypes.name');
+    foreach my $bug_id (keys %bugs) {
+        foreach my $attach_id (keys %{$bugs{$bug_id}}) {
+            my @old_summaries = Bugzilla::Flag->snapshot($bugs{$bug_id}->{$attach_id});
+            $_->_set_requestee() foreach @{$bugs{$bug_id}->{$attach_id}};
+            my @new_summaries = Bugzilla::Flag->snapshot($bugs{$bug_id}->{$attach_id});
+            my ($removed, $added) =
+              Bugzilla::Flag->update_activity(\@old_summaries, \@new_summaries);
+            $sth_updateFlag->execute($bug_id, $attach_id || undef, $userid,
+                                     $timestamp, $fieldid, $removed, $added);
         }
-        else {
-            $sth_flagupdate_bug->execute($bug_id, $otherUserID);
-        }
-        my @new_summaries = Bugzilla::Flag->snapshot($bug_id, $attach_id);
-        # Let update_activity do all the dirty work, including setting
-        # the bug timestamp.
-        Bugzilla::Flag::update_activity($bug_id, $attach_id, $timestamp,
-                                        \@old_summaries, \@new_summaries);
+        $sth_set_bug_timestamp->execute($timestamp, $bug_id);
         $updatedbugs{$bug_id} = 1;
     }
 
@@ -536,9 +537,8 @@ if ($action eq 'search') {
              ($otherUserID, $otherUserID));
 
     # Deletions in referred tables which need LogActivityEntry.
-    $buglist = $dbh->selectcol_arrayref('SELECT bug_id FROM cc
-                                          WHERE who = ?',
-                                        undef, $otherUserID);
+    my $buglist = $dbh->selectcol_arrayref('SELECT bug_id FROM cc WHERE who = ?',
+                                            undef, $otherUserID);
     $dbh->do('DELETE FROM cc WHERE who = ?', undef, $otherUserID);
     foreach my $bug_id (@$buglist) {
         LogActivityEntry($bug_id, 'cc', $otherUser->login, '', $userid,
diff --git a/extensions/example/code/flag-end_of_update.pl b/extensions/example/code/flag-end_of_update.pl
index 6371bd154..e1705cc57 100644
--- a/extensions/example/code/flag-end_of_update.pl
+++ b/extensions/example/code/flag-end_of_update.pl
@@ -26,15 +26,15 @@ use Bugzilla::Util qw(diff_arrays);
 # This code doesn't actually *do* anything, it's just here to show you
 # how to use this hook.
 my $args = Bugzilla->hook_args;
-my ($bug, $timestamp, $old_flags, $new_flags) = 
-    @$args{qw(bug timestamp old_flags new_flags)};
+my ($object, $timestamp, $old_flags, $new_flags) =
+    @$args{qw(object timestamp old_flags new_flags)};
 my ($removed, $added) = diff_arrays($old_flags, $new_flags);
 my ($granted, $denied) = (0, 0);
 foreach my $new_flag (@$added) {
     $granted++ if $new_flag =~ /\+$/;
     $denied++ if $new_flag =~ /-$/;
 }
-my $bug_id = $bug->id;
+my $bug_id = (ref $object eq 'Bugzilla::Bug') ? $object->id : $object->bug_id;
 my $result = "$granted flags were granted and $denied flags were denied"
              . " on bug $bug_id at $timestamp.";
 # Uncomment this line to see $result in your webserver's error log whenever
diff --git a/post_bug.cgi b/post_bug.cgi
index 9a933fc1a..997b621ad 100755
--- a/post_bug.cgi
+++ b/post_bug.cgi
@@ -104,7 +104,7 @@ if (defined $cgi->param('maketemplate')) {
 umask 0;
 
 # get current time
-my $timestamp = $dbh->selectrow_array(q{SELECT NOW()});
+my $timestamp = $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
 
 # Group Validation
 my @selected_groups;
@@ -219,7 +219,10 @@ if (defined($cgi->upload('data')) || $cgi->param('attachurl')) {
 
     if ($attachment) {
         # Set attachment flags.
-        Bugzilla::Flag->set_flags($bug, $attachment, $timestamp, $vars);
+        my ($flags, $new_flags) = Bugzilla::Flag->extract_flags_from_cgi(
+                                      $bug, $attachment, $vars, SKIP_REQUESTEE_ON_ERROR);
+        $attachment->set_flags($flags, $new_flags);
+        $attachment->update($timestamp);
 
         # Update the comment to include the new attachment ID.
         # This string is hardcoded here because Template::quoteUrls()
@@ -246,7 +249,10 @@ if (defined($cgi->upload('data')) || $cgi->param('attachurl')) {
 }
 
 # Set bug flags.
-Bugzilla::Flag->set_flags($bug, undef, $timestamp, $vars);
+my ($flags, $new_flags) = Bugzilla::Flag->extract_flags_from_cgi($bug, undef, $vars,
+                                                             SKIP_REQUESTEE_ON_ERROR);
+$bug->set_flags($flags, $new_flags);
+$bug->update($timestamp);
 
 # Email everyone the details of the new bug 
 $vars->{'mailrecipients'} = {'changer' => $user->login};
diff --git a/process_bug.cgi b/process_bug.cgi
index 9faaf7445..f10467d4e 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -143,22 +143,13 @@ if (defined $cgi->param('dontchange')) {
 }
 
 # do a match on the fields if applicable
-
-# The order of these function calls is important, as Flag::validate
-# assumes User::match_field has ensured that the values
-# in the requestee fields are legitimate user email addresses.
-&Bugzilla::User::match_field($cgi, {
+Bugzilla::User::match_field($cgi, {
     'qa_contact'                => { 'type' => 'single' },
     'newcc'                     => { 'type' => 'multi'  },
     'masscc'                    => { 'type' => 'multi'  },
     'assigned_to'               => { 'type' => 'single' },
-    '^requestee(_type)?-(\d+)$' => { 'type' => 'multi'  },
 });
 
-# Validate flags in all cases. validate() should not detect any
-# reference to flags if $cgi->param('id') is undefined.
-Bugzilla::Flag::validate($cgi->param('id'));
-
 print $cgi->header() unless Bugzilla->usage_mode == USAGE_MODE_EMAIL;
 
 # Check for a mid-air collision. Currently this only works when updating
@@ -280,6 +271,12 @@ foreach my $bug (@bug_objects) {
     $product_change ||= $changed;
 }
 
+# Flags should be set AFTER the bug has been moved into another product/component.
+if ($cgi->param('id')) {
+    my ($flags, $new_flags) = Bugzilla::Flag->extract_flags_from_cgi($first_bug, undef, $vars);
+    $first_bug->set_flags($flags, $new_flags);
+}
+
 if ($cgi->param('id') && (defined $cgi->param('dependson')
                           || defined $cgi->param('blocked')) )
 {
@@ -586,9 +583,6 @@ foreach my $bug (@bug_objects) {
         CheckIfVotedConfirmed($bug->id);
     }
 
-    # Set and update flags.
-    Bugzilla::Flag->process($bug, undef, $timestamp, $vars);
-
     $dbh->bz_commit_transaction();
 
     ###############
diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl
index 91207a20e..97fd59d21 100644
--- a/template/en/default/global/code-error.html.tmpl
+++ b/template/en/default/global/code-error.html.tmpl
@@ -222,15 +222,10 @@
     but you tried to flag it as obsolete while creating a new attachment to 
     [% terms.bug %] [%+ my_bug_id FILTER html %].
 
-  [% ELSIF error == "flags_not_available" %]
-    [% title = "Flag Editing not Allowed" %]
-    [% IF type == "b" %]
-      Flags cannot be set or changed when
-      changing several [% terms.bugs %] at once.
-    [% ELSE %]
-      References to existing flags when creating
-      a new attachment are invalid.
-    [% END %] 
+  [% ELSIF error == "flag_unexpected_object" %]
+    [% title = "Object Not Recognized" %]
+    Flags cannot be set for objects of type [% caller FILTER html %].
+    They can only be set for [% terms.bugs %] and attachments.
 
   [% ELSIF error == "flag_requestee_disabled" %]
     [% title = "Flag not Requestable from Specific Person" %]
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 69811f210..a30f29706 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -568,12 +568,6 @@
     <br>Alternately, if your attachment is an image, you could convert
     it to a compressible format like JPG or PNG and try again.
 
-  [% ELSIF error == "flag_not_multiplicable" %]
-    [% docslinks = {'flags-overview.html' => 'An overview on Flags',
-                    'flags.html' => 'Using Flags'} %]
-    You can't ask more than one person at a time for
-    <em>[% type.name FILTER html %]</em>.
-
   [% ELSIF error == "flag_requestee_needs_privs" %]
     [% title = "Flag Requestee Needs Privileges" %]
     [% requestee.identity FILTER html %] does not have permission to set the
@@ -632,6 +626,12 @@
     The name <em>[% name FILTER html %]</em> must be 1-50 characters long
     and must not contain any spaces or commas.
 
+  [% ELSIF error == "flag_type_not_multiplicable" %]
+    [% docslinks = {'flags-overview.html' => 'An overview on Flags',
+                    'flags.html' => 'Using Flags'} %]
+    You cannot have several <em>[% type.name FILTER html %]</em> flags
+    for this [% IF attachment %] attachment [% ELSE %] [%+ terms.bug %] [% END %].
+
   [% ELSIF error == "flag_update_denied" %]
     [% title = "Flag Modification Denied" %]
     [% admindocslinks = {'flags-overview.html#flags-admin'  => 'Administering Flags',
@@ -1158,9 +1158,11 @@
   [% ELSIF error == "no_bugs_in_list" %]
     [% title = "Delete Tag?" %]
     This will remove all [% terms.bugs %] from the
-    [% tag FILTER html %] tag. This will delete the tag completely. Click
+    <em>[% name FILTER html %]</em> tag. This will delete the tag completely. Click
     <a href="buglist.cgi?cmdtype=dorem&amp;remaction=forget&amp;namedcmd=
-    [%- tag FILTER url_quote %]">here</a> if you really want to delete it.
+    [%- name FILTER url_quote %]&amp;token=
+    [%- issue_hash_token([query_id, name]) FILTER url_quote %]">here</a>
+    if you really want to delete it.
 
   [% ELSIF error == "no_bugs_to_remove" %]
     [% title = "No Tag Selected" %]
@@ -1742,6 +1744,10 @@
     milestone
   [% ELSIF class == "Bugzilla::Status" %]
     status
+  [% ELSIF class == "Bugzilla::Flag" %]
+    flag
+  [% ELSIF class == "Bugzilla::FlagType" %]
+    flagtype
   [% ELSIF class == "Bugzilla::Field" %]
     field
   [% ELSIF ( matches = class.match('^Bugzilla::Field::Choice::(.+)') ) %]
diff --git a/template/en/default/request/email.txt.tmpl b/template/en/default/request/email.txt.tmpl
index 81948c42c..adc0faa1c 100644
--- a/template/en/default/request/email.txt.tmpl
+++ b/template/en/default/request/email.txt.tmpl
@@ -24,28 +24,31 @@
 
 [% bugidsummary = bug.bug_id _ ': ' _ bug.short_desc %]
 [% attidsummary = attachment.id _ ': ' _ attachment.description %]
+[% flagtype_name = flag ? flag.type.name : old_flag.type.name %]
 [% statuses = { '+' => "granted" , '-' => 'denied' , 'X' => "canceled" ,
                 '?' => "asked" } %]
 
 [% to_identity = "" %]
 [% on_behalf_of = 0 %]
-[% IF flag.status == '?' %]
+[% action = flag.status || 'X' %]
+
+[% IF flag && flag.status == '?' %]
   [% subject_status = "requested" %]
-  [% IF flag.setter.id == user.id %]
+  [% IF flag.setter_id == user.id %]
     [% to_identity = flag.requestee.identity _ " for" %]
   [% ELSE %]
     [% on_behalf_of = 1 %]
     [% IF flag.requestee %][% to_identity = " to " _ flag.requestee.identity %][% END %]
   [% END %]
 [% ELSE %]
-  [% IF flag.requester %]
-    [% to_identity = flag.requester.identity _ "'s request for" %]
+  [% IF old_flag && old_flag.status == '?' %]
+    [% to_identity = old_flag.setter.identity _ "'s request for" %]
   [% END %]
-  [% subject_status = statuses.${flag.status} %]
+  [% subject_status = statuses.$action %]
 [% END %]
 From: [% Param('mailfrom') %]
 To: [% to %]
-Subject: [% flag.type.name %] [%+ subject_status %]: [[% terms.Bug %] [%+ bug.bug_id %]] [% bug.short_desc %]
+Subject: [% flagtype_name %] [%+ subject_status %]: [[% terms.Bug %] [%+ bug.bug_id %]] [% bug.short_desc %]
 [%- IF attachment %] :
   [Attachment [% attachment.id %]] [% attachment.description %][% END %]
 X-Bugzilla-Type: request
@@ -55,10 +58,10 @@ X-Bugzilla-Type: request
 [%- FILTER bullet = wrap(80) -%]
 
 [% IF on_behalf_of %]
-[% user.identity %] has reassigned [% flag.setter.identity %]'s request for [% flag.type.name %]
+[% user.identity %] has reassigned [% flag.setter.identity %]'s request for [% flagtype_name %]
 [% to_identity %]:
 [% ELSE %]
-[% user.identity %] has [% statuses.${flag.status} %] [%+ to_identity %] [%+ flag.type.name %]:
+[% user.identity %] has [% statuses.$action %] [%+ to_identity %] [%+ flagtype_name %]:
 [% END %]
 
 [% terms.Bug %] [%+ bugidsummary %]