From 44e26f0615c237fa6759bc23845a96d773e0cba1 Mon Sep 17 00:00:00 2001 From: Dave Lawrence Date: Tue, 31 Dec 2013 08:58:57 -0500 Subject: Bug 756048 - Add and update bug and attachment flags using the WebService API r=sgreen,a=justdave --- Bugzilla/WebService/Bug.pm | 454 ++++++++++++++++++++++- Bugzilla/WebService/Constants.pm | 7 + Bugzilla/WebService/Server/REST/Resources/Bug.pm | 18 +- Bugzilla/WebService/Util.pm | 87 +++++ 4 files changed, 558 insertions(+), 8 deletions(-) (limited to 'Bugzilla') diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm index 8b3f58eb1..de7853092 100644 --- a/Bugzilla/WebService/Bug.pm +++ b/Bugzilla/WebService/Bug.pm @@ -18,7 +18,7 @@ use Bugzilla::Constants; use Bugzilla::Error; use Bugzilla::Field; use Bugzilla::WebService::Constants; -use Bugzilla::WebService::Util qw(filter filter_wants validate translate); +use Bugzilla::WebService::Util qw(extract_flags filter filter_wants validate translate); use Bugzilla::Bug; use Bugzilla::BugMail; use Bugzilla::Util qw(trick_taint trim diff_arrays detaint_natural); @@ -27,6 +27,8 @@ use Bugzilla::Milestone; use Bugzilla::Status; use Bugzilla::Token qw(issue_hash_token); use Bugzilla::Search; +use Bugzilla::Product; +use Bugzilla::FlagType; use Bugzilla::Search::Quicksearch; use List::Util qw(max); @@ -605,10 +607,15 @@ sub update { # have valid "set_" functions in Bugzilla::Bug, but shouldn't be # called using those field names. delete $values{dependencies}; - delete $values{flags}; + + my $flags = delete $values{flags}; foreach my $bug (@bugs) { $bug->set_all(\%values); + if ($flags) { + my ($old_flags, $new_flags) = extract_flags($flags, $bug); + $bug->set_flags($old_flags, $new_flags); + } } my %all_changes; @@ -617,7 +624,7 @@ sub update { $all_changes{$bug->id} = $bug->update(); } $dbh->bz_commit_transaction(); - + foreach my $bug (@bugs) { $bug->send_changes($all_changes{$bug->id}); } @@ -663,9 +670,29 @@ sub update { sub create { my ($self, $params) = @_; + my $dbh = Bugzilla->dbh; + Bugzilla->login(LOGIN_REQUIRED); + $params = Bugzilla::Bug::map_fields($params); + + my $flags = delete $params->{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(); + Bugzilla::BugMail::Send($bug->bug_id, { changer => $bug->reporter }); return { id => $self->type('int', $bug->bug_id) }; } @@ -739,6 +766,8 @@ sub add_attachment { $dbh->bz_start_transaction(); my $timestamp = $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)'); + my $flags = delete $params->{flags}; + foreach my $bug (@bugs) { my $attachment = Bugzilla::Attachment->create({ bug => $bug, @@ -750,6 +779,13 @@ sub add_attachment { ispatch => $params->{is_patch}, isprivate => $params->{is_private}, }); + + if ($flags) { + my ($old_flags, $new_flags) = extract_flags($flags, $bug, $attachment); + $attachment->set_flags($old_flags, $new_flags); + } + + $attachment->update($timestamp); my $comment = $params->{comment} || ''; $attachment->bug->add_comment($comment, { isprivate => $attachment->isprivate, @@ -781,9 +817,6 @@ sub update_attachment { delete $params->{$key}; } - # We can't update flags, and summary is really description - delete $params->{flags}; - $params = translate($params, ATTACHMENT_MAPPED_SETTERS); # Get all the attachments, after verifying that they exist and are editable @@ -801,9 +834,15 @@ sub update_attachment { $bugs{$bug->id} = $bug; } + my $flags = delete $params->{flags}; + # Update the values foreach my $attachment (@attachments) { $attachment->set_all($params); + if ($flags) { + my ($old_flags, $new_flags) = extract_flags($flags, $attachment->bug, $attachment); + $attachment->set_flags($old_flags, $new_flags); + } } $dbh->bz_start_transaction(); @@ -1011,6 +1050,38 @@ sub update_tags { return { changes => \%changes }; } +sub flag_types { + my ($self, $params) = @_; + my $dbh = Bugzilla->switch_to_shadow_db(); + my $user = Bugzilla->user; + + defined $params->{product} + || ThrowCodeError('param_required', + { function => 'Bug.flag_types', + param => 'product' }); + + my $product = delete $params->{product}; + my $component = delete $params->{component}; + + $product = Bugzilla::Product->check({ name => $product, cache => 1 }); + $component = Bugzilla::Component->check( + { name => $component, product => $product, cache => 1 }) if $component; + + my $flag_params = { product_id => $product->id }; + $flag_params->{component_id} = $component->id if $component; + my $matched_flag_types = Bugzilla::FlagType::match($flag_params); + + my $flag_types = { bug => [], attachment => [] }; + foreach my $flag_type (@$matched_flag_types) { + push(@{ $flag_types->{bug} }, $self->_flagtype_to_hash($flag_type, $product)) + if $flag_type->target_type eq 'bug'; + push(@{ $flag_types->{attachment} }, $self->_flagtype_to_hash($flag_type, $product)) + if $flag_type->target_type eq 'attachment'; + } + + return $flag_types; +} + sub update_comment_tags { my ($self, $params) = @_; @@ -1074,7 +1145,6 @@ sub search_comment_tags { return [ map { $_->tag } @$tags ]; } - ############################## # Private Helper Subroutines # ############################## @@ -1292,6 +1362,56 @@ sub _flag_to_hash { return $item; } +sub _flagtype_to_hash { + my ($self, $flagtype, $product) = @_; + my $user = Bugzilla->user; + + my @values = ('X'); + push(@values, '?') if ($flagtype->is_requestable && $user->can_request_flag($flagtype)); + push(@values, '+', '-') if $user->can_set_flag($flagtype); + + my $item = { + id => $self->type('int' , $flagtype->id), + name => $self->type('string' , $flagtype->name), + description => $self->type('string' , $flagtype->description), + type => $self->type('string' , $flagtype->target_type), + values => \@values, + is_active => $self->type('boolean', $flagtype->is_active), + is_requesteeble => $self->type('boolean', $flagtype->is_requesteeble), + is_multiplicable => $self->type('boolean', $flagtype->is_multiplicable) + }; + + if ($product) { + my $inclusions = $self->_flagtype_clusions_to_hash($flagtype->inclusions, $product->id); + my $exclusions = $self->_flagtype_clusions_to_hash($flagtype->exclusions, $product->id); + # if we have both inclusions and exclusions, the exclusions are redundant + $exclusions = [] if @$inclusions && @$exclusions; + # no need to return anything if there's just "any component" + $item->{inclusions} = $inclusions if @$inclusions && $inclusions->[0] ne ''; + $item->{exclusions} = $exclusions if @$exclusions && $exclusions->[0] ne ''; + } + + return $item; +} + +sub _flagtype_clusions_to_hash { + my ($self, $clusions, $product_id) = @_; + my $result = []; + foreach my $key (keys %$clusions) { + my ($prod_id, $comp_id) = split(/:/, $clusions->{$key}, 2); + if ($prod_id == 0 || $prod_id == $product_id) { + if ($comp_id) { + my $component = Bugzilla::Component->new({ id => $comp_id, cache => 1 }); + push @$result, $component->name; + } + else { + return [ '' ]; + } + } + } + return $result; +} + sub _add_update_tokens { my ($self, $params, $bugs, $hashes) = @_; @@ -1561,6 +1681,103 @@ You specified an invalid field name or id. =back +=head2 flag_types + +B + +=over + +=item B + +Get information about valid flag types that can be set for bugs and attachments. + +=item B + +You have several options for retreiving information about flag types. The first +part is the request method and the rest is the related path needed. + +To get information about all flag types for a product: + +GET /flag_types/ + +To get information about flag_types for a product and component: + +GET /flag_types// + +The returned data format is the same as below. + +=item B + +You must pass a product name and an optional component name. + +=over + +=item C (string) - The name of a valid product. + +=item C (string) - An optional valid component name associated with the product. + +=back + +=item B + +A hash containing a two keys, C and C. Each key value is an array of hashes, +containing the following keys: + +=over + +=item C + +C An integer id uniquely identifying this flag type. + +=item C + +C The name for the flag type. + +=item C + +C The target of the flag type which is either C or C. + +=item C + +C The description of the flag type. + +=item C + +C An array of string values that the user can set on the flag type. + +=item C + +C Users can ask specific other users to set flags of this type. + +=item C + +C Multiple flags of this type can be set for the same bug or attachment. + +=back + +=item B + +=over + +=item 106 (Product Access Denied) + +Either the product does not exist or you don't have access to it. + +=item 51 (Invalid Component) + +The component provided does not exist in the product. + +=back + +=item B + +=over + +=item Added in Bugzilla B<5.0>. + +=back + +=back =head2 legal_values @@ -2999,6 +3216,32 @@ with L. =item C (string) - A valid target milestone for this product. +=item C + +C An array of hashes with flags to add to the bug. To create a flag, +at least the status and the type_id or name must be provided. An optional +requestee can be passed if the flag type is requesteeble. + +=over + +=item C + +C The name of the flag type. + +=item C + +C The internal flag type id. + +=item C + +C The flags new status (i.e. "?", "+", "-" or "X" to clear a flag). + +=item C + +C The login of the requestee if the flag type is requesteeable. + +=back + =back In addition to the above parameters, if your installation has any custom @@ -3051,6 +3294,28 @@ that would cause a circular dependency between bugs. You tried to restrict the bug to a group which does not exist, or which you cannot use with this product. +=item 129 (Flag Status Invalid) + +The flag status is invalid. + +=item 130 (Flag Modification Denied) + +You tried to request, grant, or deny a flag but only a user with the required +permissions may make the change. + +=item 131 (Flag not Requestable from Specific Person) + +You can't ask a specific person for the flag. + +=item 133 (Flag Type not Unique) + +The flag type specified matches several flag types. You must specify +the type id value to update or add a flag. + +=item 134 (Inactive Flag Type) + +The flag type is inactive and cannot be used to create new flags. + =item 504 (Invalid User) Either the QA Contact, Assignee, or CC lists have some invalid user @@ -3159,6 +3424,32 @@ to the "insidergroup"), False if the attachment should be public. Defaults to False if not specified. +=item C + +C An array of hashes with flags to add to the attachment. to create a flag, +at least the status and the type_id or name must be provided. An optional requestee +can be passed if the flag type is requesteeble. + +=over + +=item C + +C The name of the flag type. + +=item C + +C The internal flag type id. + +=item C + +C The flags new status (i.e. "?", "+", "-" or "X" to clear a flag). + +=item C + +C The login of the requestee if the flag type is requesteeable. + +=back + =back =item B @@ -3172,6 +3463,28 @@ This method can throw all the same errors as L, plus: =over +=item 129 (Flag Status Invalid) + +The flag status is invalid. + +=item 130 (Flag Modification Denied) + +You tried to request, grant, or deny a flag but only a user with the required +permissions may make the change. + +=item 131 (Flag not Requestable from Specific Person) + +You can't ask a specific person for the flag. + +=item 133 (Flag Type not Unique) + +The flag type specified matches several flag types. You must specify +the type id value to update or add a flag. + +=item 134 (Inactive Flag Type) + +The flag type is inactive and cannot be used to create new flags. + =item 600 (Attachment Too Large) You tried to attach a file that was larger than Bugzilla will accept. @@ -3271,6 +3584,41 @@ to the "insidergroup"), False if the attachment should be public. C True if the attachment is obsolete, False otherwise. +=item C + +C An array of hashes with changes to the flags. The following values +can be specified. At least the status and one of type_id, id, or name must +be specified. If a type_id or name matches a single currently set flag, +the flag will be updated unless new is specified. + +=over + +=item C + +C The name of the flag that will be created or updated. + +=item C + +C The internal flag type id that will be created or updated. You will +need to specify the C if more than one flag type of the same name exists. + +=item C + +C The flags new status (i.e. "?", "+", "-" or "X" to clear a flag). + +=item C + +C The login of the requestee if the flag type is requesteeable. + +=item C + +C Use id to specify the flag to be updated. You will need to specify the C +if more than one flag is set of the same name. + +=item C + +C Set to true if you specifically want a new flag to be created. + =back =item B @@ -3336,6 +3684,33 @@ This method can throw all the same errors as L, plus: =over +=item 129 (Flag Status Invalid) + +The flag status is invalid. + +=item 130 (Flag Modification Denied) + +You tried to request, grant, or deny a flag but only a user with the required +permissions may make the change. + +=item 131 (Flag not Requestable from Specific Person) + +You can't ask a specific person for the flag. + +=item 132 (Flag not Unique) + +The flag specified has been set multiple times. You must specify the id +value to update the flag. + +=item 133 (Flag Type not Unique) + +The flag type specified matches several flag types. You must specify +the type id value to update or add a flag. + +=item 134 (Inactive Flag Type) + +The flag type is inactive and cannot be used to create new flags. + =item 601 (Invalid MIME Type) You specified a C argument that was blank, not a valid @@ -3361,6 +3736,7 @@ You did not specify a value for the C argument. =back +=back =head2 add_comment @@ -3603,6 +3979,43 @@ duplicate bugs. C The total estimate of time required to fix the bug, in hours. This is the I estimate, not the amount of time remaining to fix it. +=item C + +C An array of hashes with changes to the flags. The following values +can be specified. At least the status and one of type_id, id, or name must +be specified. If a type_id or name matches a single currently set flag, +the flag will be updated unless new is specified. + +=over + +=item C + +C The name of the flag that will be created or updated. + +=item C + +C The internal flag type id that will be created or updated. You will +need to specify the C if more than one flag type of the same name exists. + +=item C + +C The flags new status (i.e. "?", "+", "-" or "X" to clear a flag). + +=item C + +C The login of the requestee if the flag type is requesteeable. + +=item C + +C Use id to specify the flag to be updated. You will need to specify the C +if more than one flag is set of the same name. + +=item C + +C Set to true if you specifically want a new flag to be created. + +=back + =item C C The groups a bug is in. To modify this field, pass a hash, which @@ -3920,6 +4333,33 @@ field. You tried to change from one status to another, but the status workflow rules don't allow that change. +=item 129 (Flag Status Invalid) + +The flag status is invalid. + +=item 130 (Flag Modification Denied) + +You tried to request, grant, or deny a flag but only a user with the required +permissions may make the change. + +=item 131 (Flag not Requestable from Specific Person) + +You can't ask a specific person for the flag. + +=item 132 (Flag not Unique) + +The flag specified has been set multiple times. You must specify the id +value to update the flag. + +=item 133 (Flag Type not Unique) + +The flag type specified matches several flag types. You must specify +the type id value to update or add a flag. + +=item 134 (Inactive Flag Type) + +The flag type is inactive and cannot be used to create new flags. + =back =item B diff --git a/Bugzilla/WebService/Constants.pm b/Bugzilla/WebService/Constants.pm index 2c5bc31dd..5d955b96a 100644 --- a/Bugzilla/WebService/Constants.pm +++ b/Bugzilla/WebService/Constants.pm @@ -126,6 +126,13 @@ use constant WS_ERROR_CODE => { missing_resolution => 121, resolution_not_allowed => 122, illegal_bug_status_transition => 123, + # Flag errors + flag_status_invalid => 129, + flag_update_denied => 130, + flag_type_requestee_disabled => 131, + flag_not_unique => 132, + flag_type_not_unique => 133, + flag_type_inactive => 134, # Authentication errors are usually 300-400. invalid_login_or_password => 300, diff --git a/Bugzilla/WebService/Server/REST/Resources/Bug.pm b/Bugzilla/WebService/Server/REST/Resources/Bug.pm index ea420b4ed..d0f470fcd 100644 --- a/Bugzilla/WebService/Server/REST/Resources/Bug.pm +++ b/Bugzilla/WebService/Server/REST/Resources/Bug.pm @@ -151,7 +151,23 @@ sub _rest_resources { } } }, - + qr{^/flag_types/([^/]+)/([^/]+)$}, { + GET => { + method => 'flag_types', + params => sub { + return { product => $_[0], + component => $_[1] }; + } + } + }, + qr{^/flag_types/([^/]+)$}, { + GET => { + method => 'flag_types', + params => sub { + return { product => $_[0] }; + } + } + } ]; return $rest_resources; } diff --git a/Bugzilla/WebService/Util.pm b/Bugzilla/WebService/Util.pm index be7251532..364783216 100644 --- a/Bugzilla/WebService/Util.pm +++ b/Bugzilla/WebService/Util.pm @@ -10,6 +10,12 @@ package Bugzilla::WebService::Util; use 5.10.1; use strict; +use Bugzilla::Flag; +use Bugzilla::FlagType; +use Bugzilla::Error; + +use Storable qw(dclone); + use parent qw(Exporter); # We have to "require", not "use" this, because otherwise it tries to @@ -17,6 +23,7 @@ use parent qw(Exporter); require Test::Taint; our @EXPORT_OK = qw( + extract_flags filter filter_wants taint_data @@ -26,6 +33,80 @@ our @EXPORT_OK = qw( fix_credentials ); +sub extract_flags { + my ($flags, $bug, $attachment) = @_; + my (@new_flags, @old_flags); + + my $flag_types = $attachment ? $attachment->flag_types : $bug->flag_types; + my $current_flags = $attachment ? $attachment->flags : $bug->flags; + + # Copy the user provided $flags as we may call extract_flags more than + # once when editing multiple bugs or attachments. + my $flags_copy = dclone($flags); + + foreach my $flag (@$flags_copy) { + my $id = $flag->{id}; + my $type_id = $flag->{type_id}; + + my $new = delete $flag->{new}; + my $name = delete $flag->{name}; + + if ($id) { + my $flag_obj = grep($id == $_->id, @$current_flags); + $flag_obj || ThrowUserError('object_does_not_exist', + { class => 'Bugzilla::Flag', id => $id }); + } + elsif ($type_id) { + my $type_obj = grep($type_id == $_->id, @$flag_types); + $type_obj || ThrowUserError('object_does_not_exist', + { class => 'Bugzilla::FlagType', id => $type_id }); + if (!$new) { + my @flag_matches = grep($type_id == $_->type->id, @$current_flags); + @flag_matches > 1 && ThrowUserError('flag_not_unique', + { value => $type_id }); + if (!@flag_matches) { + delete $flag->{id}; + } + else { + delete $flag->{type_id}; + $flag->{id} = $flag_matches[0]->id; + } + } + } + elsif ($name) { + my @type_matches = grep($name eq $_->name, @$flag_types); + @type_matches > 1 && ThrowUserError('flag_type_not_unique', + { value => $name }); + @type_matches || ThrowUserError('object_does_not_exist', + { class => 'Bugzilla::FlagType', name => $name }); + if ($new) { + delete $flag->{id}; + $flag->{type_id} = $type_matches[0]->id; + } + else { + my @flag_matches = grep($name eq $_->type->name, @$current_flags); + @flag_matches > 1 && ThrowUserError('flag_not_unique', { value => $name }); + if (@flag_matches) { + $flag->{id} = $flag_matches[0]->id; + } + else { + delete $flag->{id}; + $flag->{type_id} = $type_matches[0]->id; + } + } + } + + if ($flag->{id}) { + push(@old_flags, $flag); + } + else { + push(@new_flags, $flag); + } + } + + return (\@old_flags, \@new_flags); +} + sub filter ($$;$) { my ($params, $hash, $prefix) = @_; my %newhash = %$hash; @@ -232,6 +313,12 @@ Allows for certain parameters related to authentication such as Bugzilla_login, Bugzilla_password, and Bugzilla_token to have shorter named equivalents passed in. This function converts the shorter versions to their respective internal names. +=head2 extract_flags + +Subroutine that takes a list of hashes that are potential flag changes for +both bugs and attachments. Then breaks the list down into two separate lists +based on if the change is to add a new flag or to update an existing flag. + =head1 B =over -- cgit v1.2.3-24-g4f1b