'Bug 1159057: change to create flags as part of bug creation process. r=gerv

6 files changed, 99 insertions, 42 deletions

diff --git a/Bugzilla/API/1_0/Resource/Bug.pm b/Bugzilla/API/1_0/Resource/Bug.pm
index c6ae6a19a..a8565b7a0 100644
--- a/Bugzilla/API/1_0/Resource/Bug.pm
+++ b/Bugzilla/API/1_0/Resource/Bug.pm
@@ -820,7 +820,7 @@ sub update {
     foreach my $bug (@bugs) {
         $bug->set_all(\%values);
         if ($flags) {
-            my ($old_flags, $new_flags) = extract_flags($flags, $bug);
+            my ($old_flags, $new_flags) = extract_flags($flags, $bug->flag_types, $bug->flags);
             $bug->set_flags($old_flags, $new_flags);
         }
     }
@@ -885,19 +885,29 @@ sub create {
 
     my $flags = delete $params->{flags};
 
+    if ($flags) {
+        my $product = Bugzilla::Product->check($params->{product});
+        my $component = Bugzilla::Component->check({
+            product => $product,
+            name => $params->{component}
+        });
+        my $flag_types = Bugzilla::FlagType::match({
+            product_id => $product->id,
+            component_id => $component->id,
+            is_active => 1,
+        });
+
+        my(undef, $new_flags) =  extract_flags($flags, $flag_types);
+
+        $params->{flags} = $new_flags;
+    }
+
     # We start a nested transaction in case flag setting fails
     # we want the bug creation to roll back as well.
     $dbh->bz_start_transaction();
 
     my $bug = Bugzilla::Bug->create($params);
 
-    # Set bug flags
-    if ($flags) {
-        my ($flags, $new_flags) = extract_flags($flags, $bug);
-        $bug->set_flags($flags, $new_flags);
-        $bug->update($bug->creation_ts);
-    }
-
     $dbh->bz_commit_transaction();
 
     $bug->send_changes();
@@ -989,7 +999,7 @@ sub add_attachment {
         });
 
         if ($flags) {
-            my ($old_flags, $new_flags) = extract_flags($flags, $bug, $attachment);
+            my ($old_flags, $new_flags) = extract_flags($flags, $attachment->flag_types, $attachment->flags);
             $attachment->set_flags($old_flags, $new_flags);
         }
 
@@ -1066,7 +1076,7 @@ sub update_attachment {
     # Update the values
     foreach my $attachment (@attachments) {
         my ($update_flags, $new_flags) = $flags
-            ? extract_flags($flags, $attachment->bug, $attachment)
+            ? extract_flags($flags, $attachment->flag_types, $attachment->flags)
             : ([], []);
         if ($attachment->validate_can_edit) {
             $attachment->set_all($params);
diff --git a/Bugzilla/API/1_0/Util.pm b/Bugzilla/API/1_0/Util.pm
index d22935f6e..afdde314e 100644
--- a/Bugzilla/API/1_0/Util.pm
+++ b/Bugzilla/API/1_0/Util.pm
@@ -51,11 +51,10 @@ our @EXPORT = qw(
 );
 
 sub extract_flags {
-    my ($flags, $bug, $attachment) = @_;
+    my ($flags, $flag_types, $current_flags) = @_;
     my (@new_flags, @old_flags);
 
-    my $flag_types    = $attachment ? $attachment->flag_types : $bug->flag_types;
-    my $current_flags = $attachment ? $attachment->flags : $bug->flags;
+    $current_flags //= [];
 
     # Copy the user provided $flags as we may call extract_flags more than
     # once when editing multiple bugs or attachments.
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index bd3bf587c..4f12c179f 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -689,6 +689,7 @@ sub possible_duplicates {
 #                     user is not a member of the timetrackinggroup.
 # C<deadline>       - For time-tracking. Will be ignored for the same
 #                     reasons as C<estimated_time>.
+# C<flags>        - An array of flags that will be applied to the bug.
 sub create {
     my ($class, $params) = @_;
     my $dbh = Bugzilla->dbh;
@@ -724,6 +725,7 @@ sub create {
     my $is_markdown      = delete $params->{is_markdown};
     my $see_also         = delete $params->{see_also};
     my $comment_tags     = delete $params->{comment_tags};
+    my $flags            = delete $params->{flags};
 
     # We don't want the bug to appear in the system until it's correctly
     # protected by groups.
@@ -806,6 +808,14 @@ sub create {
         delete $bug->{_update_ref_bugs};
     }
 
+    # Apply any flags.
+    if (defined $flags) {
+        $bug->set_flags($flags);
+        foreach my $flag (@{$bug->flags}) {
+            Bugzilla::Flag->create($flag);
+        }
+    }
+
     # Comment #0 handling...
 
     # We now have a bug id so we can fill this out
diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm
index 6d056e7e6..bf37f2a9a 100644
--- a/Bugzilla/Flag.pm
+++ b/Bugzilla/Flag.pm
@@ -795,19 +795,41 @@ sub _check_status {
 
 =over
 
-=item C<extract_flags_from_cgi($bug, $attachment, $hr_vars)>
+=item C<extract_flags_from_cgi($bug, $hr_vars, $skip, $args)>
 
-Checks whether or not there are new flags to create and returns an
-array of hashes. This array is then passed to Flag::create().
+Checks if are new flags to create and returns an array of hashes.
+This array is then passed to Flag::create().
+
+$args is a hash that can be one of two things.
+
+It can contain a bug object, and optionally an attachment object.
+
+OR
+
+It can contain both a product_id AND a component id.
 
 =back
 
 =cut
 
 sub extract_flags_from_cgi {
-    my ($class, $bug, $attachment, $vars, $skip) = @_;
+    my ($class, $vars, $skip, $args) = @_;
+
     my $cgi = Bugzilla->cgi;
 
+    my ($bug, $attachment, $component_id, $product_id);
+
+    if (defined($args->{bug})) {
+        $bug = $args->{bug};
+        $component_id = $bug->component_id;
+        $product_id = $bug->product_id;
+        $attachment = $args->{attachment} if defined $args->{attachment};
+    }
+    elsif (defined($args->{product_id})) {
+        $product_id = $args->{product_id};
+        $component_id = $args->{component_id};
+    }
+
     my $match_status = Bugzilla::User::match_field({
         '^requestee(_type)?-(\d+)$' => { 'type' => 'multi' },
     }, undef, $skip);
@@ -873,8 +895,8 @@ sub extract_flags_from_cgi {
 
     # Get a list of active flag types available for this product/component.
     my $flag_types = Bugzilla::FlagType::match(
-        { 'product_id'   => $bug->{'product_id'},
-          'component_id' => $bug->{'component_id'},
+        { 'product_id'   => $product_id,
+          'component_id' => $component_id,
           'is_active'    => 1 });
 
     foreach my $flagtype_id (@flagtype_ids) {
@@ -895,16 +917,20 @@ sub extract_flags_from_cgi {
         # We are only interested in flags the user tries to create.
         next unless scalar(grep { $_ == $type_id } @flagtype_ids);
 
-        # Get the number of flags of this type already set for this target.
-        my $has_flags = $class->count(
-            { 'type_id'     => $type_id,
-              '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.
-        next if (!$flag_type->is_multiplicable && $has_flags);
+        # If $bug is not defined, then we are creating a flag for an as
+        # yet uncreated bug.
+        if (defined $bug) {
+            # Get the number of flags of this type already set for this target.
+            my $has_flags = $class->count({
+                'type_id'     => $type_id,
+                '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.
+            next if (!$flag_type->is_multiplicable && $has_flags);
+        }
 
         my $status = $cgi->param("flag_type-$type_id");
         trick_taint($status);
diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm
index cc40259f8..a922c2bf0 100644
--- a/Bugzilla/WebService/Bug.pm
+++ b/Bugzilla/WebService/Bug.pm
@@ -681,7 +681,7 @@ sub update {
     foreach my $bug (@bugs) {
         $bug->set_all(\%values);
         if ($flags) {
-            my ($old_flags, $new_flags) = extract_flags($flags, $bug);
+            my ($old_flags, $new_flags) = extract_flags($flags, $bug->flag_types, $bug->flags);
             $bug->set_flags($old_flags, $new_flags);
         }
     }
@@ -747,6 +747,23 @@ sub create {
     $params = Bugzilla::Bug::map_fields($params);
 
     my $flags = delete $params->{flags};
+    # Set bug flags
+    if ($flags) {
+        my $product = Bugzilla::Product->check($params->{product});
+        my $component = Bugzilla::Component->check({
+            product => $product,
+            name => $params->{component}
+        });
+        my $flag_types = Bugzilla::FlagType::match({
+            product_id => $product->id,
+            component_id => $component->id,
+            is_active => 1,
+        });
+
+        my(undef, $new_flags) = extract_flags($flags, $flag_types);
+
+        $params->{flags} = $new_flags;
+    }
 
     # We start a nested transaction in case flag setting fails
     # we want the bug creation to roll back as well.
@@ -754,13 +771,6 @@ sub create {
 
     my $bug = Bugzilla::Bug->create($params);
 
-    # Set bug flags
-    if ($flags) {
-        my ($flags, $new_flags) = extract_flags($flags, $bug);
-        $bug->set_flags($flags, $new_flags);
-        $bug->update($bug->creation_ts);
-    }
-
     $dbh->bz_commit_transaction();
 
     $bug->send_changes();
@@ -853,7 +863,10 @@ sub add_attachment {
         });
 
         if ($flags) {
-            my ($old_flags, $new_flags) = extract_flags($flags, $bug, $attachment);
+            my ($old_flags, $new_flags) = extract_flags($flags,
+                $attachment->flag_types,
+                $attachment->flags);
+
             $attachment->set_flags($old_flags, $new_flags);
         }
 
@@ -938,7 +951,7 @@ sub update_attachment {
     # Update the values
     foreach my $attachment (@attachments) {
         my ($update_flags, $new_flags) = $flags
-            ? extract_flags($flags, $attachment->bug, $attachment)
+            ? extract_flags($flags, $attachment->flag_types, $attachment->flags)
             : ([], []);
         if ($attachment->validate_can_edit) {
             $attachment->set_all($params);
diff --git a/Bugzilla/WebService/Util.pm b/Bugzilla/WebService/Util.pm
index cbbc47921..052f7714d 100644
--- a/Bugzilla/WebService/Util.pm
+++ b/Bugzilla/WebService/Util.pm
@@ -37,11 +37,10 @@ our @EXPORT_OK = qw(
 );
 
 sub extract_flags {
-    my ($flags, $bug, $attachment) = @_;
+    my ($flags, $flag_types, $current_flags) = @_;
     my (@new_flags, @old_flags);
 
-    my $flag_types    = $attachment ? $attachment->flag_types : $bug->flag_types;
-    my $current_flags = $attachment ? $attachment->flags : $bug->flags;
+    $current_flags //= [];
 
     # Copy the user provided $flags as we may call extract_flags more than
     # once when editing multiple bugs or attachments.