From 38e12dd8a2c0b35391f20b60e9e3e8643c08f404 Mon Sep 17 00:00:00 2001 From: Matt Tyson Date: Fri, 15 Jan 2016 14:44:09 +0000 Subject: 'Bug 1159057: change to create flags as part of bug creation process. r=gerv --- Bugzilla/API/1_0/Resource/Bug.pm | 30 ++++++++++++++------- Bugzilla/API/1_0/Util.pm | 5 ++-- Bugzilla/Bug.pm | 10 +++++++ Bugzilla/Flag.pm | 58 +++++++++++++++++++++++++++++----------- Bugzilla/WebService/Bug.pm | 33 ++++++++++++++++------- Bugzilla/WebService/Util.pm | 5 ++-- attachment.cgi | 5 ++-- post_bug.cgi | 20 +++++++++----- process_bug.cgi | 2 +- 9 files changed, 117 insertions(+), 51 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 - For time-tracking. Will be ignored for the same # reasons as C. +# C - 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 +=item C -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. diff --git a/attachment.cgi b/attachment.cgi index ae9efef6a..8a6672ae4 100755 --- a/attachment.cgi +++ b/attachment.cgi @@ -542,7 +542,8 @@ sub insert { } my ($flags, $new_flags) = Bugzilla::Flag->extract_flags_from_cgi( - $bug, $attachment, $vars, SKIP_REQUESTEE_ON_ERROR); + $vars, SKIP_REQUESTEE_ON_ERROR, + { bug => $bug, attachment => $attachment }); $attachment->set_flags($flags, $new_flags); # Insert a comment about the new attachment into the database. @@ -700,7 +701,7 @@ sub update { $bug->add_cc($user) if $cgi->param('addselfcc'); my ($flags, $new_flags) = - Bugzilla::Flag->extract_flags_from_cgi($bug, $attachment, $vars); + Bugzilla::Flag->extract_flags_from_cgi($vars, undef, { bug => $bug, attachment => $attachment }); if ($can_edit) { $attachment->set_flags($flags, $new_flags); diff --git a/post_bug.cgi b/post_bug.cgi index 78a18e759..a01cd11fb 100755 --- a/post_bug.cgi +++ b/post_bug.cgi @@ -125,6 +125,18 @@ foreach my $field (@multi_selects) { $bug_params{$field->name} = [$cgi->param($field->name)]; } + +my $product = Bugzilla::Product->check($bug_params{'product'}); +my $component_id = Bugzilla::Component->check({ + product => $product, + name => $bug_params{'component'}})->id; + +# Set bug flags. +my (undef, $flag_data) = Bugzilla::Flag->extract_flags_from_cgi($vars, SKIP_REQUESTEE_ON_ERROR,{ + product_id => $product->id, + component_id => $component_id }); +$bug_params{flags} = $flag_data; + my $bug = Bugzilla::Bug->create(\%bug_params); # Get the bug ID back and delete the token used to create this bug. @@ -180,7 +192,8 @@ if ($data_fh || $attach_text) { if ($attachment) { # Set attachment flags. my ($flags, $new_flags) = Bugzilla::Flag->extract_flags_from_cgi( - $bug, $attachment, $vars, SKIP_REQUESTEE_ON_ERROR); + $vars, SKIP_REQUESTEE_ON_ERROR, + { bug => $bug, attachment => $attachment }); $attachment->set_flags($flags, $new_flags); $attachment->update($timestamp); my $comment = $bug->comments->[0]; @@ -193,11 +206,6 @@ if ($data_fh || $attach_text) { } } -# Set bug flags. -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); $vars->{'id'} = $id; $vars->{'bug'} = $bug; diff --git a/process_bug.cgi b/process_bug.cgi index 6ed15a067..f67050bd1 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -347,7 +347,7 @@ if (defined $cgi->param('id')) { # product/component. The structure of flags code doesn't currently # allow them to be set using set_all. my ($flags, $new_flags) = Bugzilla::Flag->extract_flags_from_cgi( - $first_bug, undef, $vars); + $vars, undef, { bug => $first_bug } ); $first_bug->set_flags($flags, $new_flags); # Tags can only be set to one bug at once. -- cgit v1.2.3-24-g4f1b