diff options
author | Dave Lawrence <dlawrence@mozilla.com> | 2014-01-01 21:23:43 +0100 |
---|---|---|
committer | Dave Lawrence <dlawrence@mozilla.com> | 2014-01-01 21:23:43 +0100 |
commit | df7f46c1ff8a9bc903e040349024d5c0c2f5be94 (patch) | |
tree | fc03f97b788358617bdf90e3109ced11489f45fe | |
parent | cae26fc87f03b8fb44376932c06e2d3d7411a065 (diff) | |
download | bugzilla-df7f46c1ff8a9bc903e040349024d5c0c2f5be94.tar.gz bugzilla-df7f46c1ff8a9bc903e040349024d5c0c2f5be94.tar.xz |
Bug 918384 - backport upstream bug 756048 to bmo/4.2 to allow setting bug/attachment flags using the webservices
-rw-r--r-- | Bugzilla/WebService/Bug.pm | 452 | ||||
-rw-r--r-- | Bugzilla/WebService/Constants.pm | 7 | ||||
-rw-r--r-- | Bugzilla/WebService/Server/REST/Resources/Bug.pm | 18 | ||||
-rw-r--r-- | Bugzilla/WebService/Util.pm | 88 | ||||
-rw-r--r-- | template/en/default/global/user-error.html.tmpl | 10 |
5 files changed, 567 insertions, 8 deletions
diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm index fbf084862..ab2e85a93 100644 --- a/Bugzilla/WebService/Bug.pm +++ b/Bugzilla/WebService/Bug.pm @@ -31,7 +31,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 detaint_natural); @@ -40,6 +40,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); @@ -628,7 +630,8 @@ 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) { if (!$user->can_edit_product($bug->product_obj->id) ) { @@ -637,6 +640,10 @@ sub update { } $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; @@ -645,7 +652,7 @@ sub update { $all_changes{$bug->id} = $bug->update(); } $dbh->bz_commit_transaction(); - + foreach my $bug (@bugs) { $bug->send_changes($all_changes{$bug->id}); } @@ -698,6 +705,7 @@ sub update { sub create { my ($self, $params) = @_; + my $dbh = Bugzilla->dbh; # BMO: Don't allow updating of bugs if disabled if (Bugzilla->params->{disable_bug_updates}) { @@ -713,7 +721,24 @@ sub create { } $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) }; } @@ -797,6 +822,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, @@ -808,6 +835,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, @@ -840,9 +874,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 @@ -860,9 +891,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(); @@ -1050,6 +1087,38 @@ sub attachments { return { bugs => \%bugs, attachments => \%attachments }; } +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) = @_; @@ -1113,7 +1182,6 @@ sub search_comment_tags { return [ map { $_->tag } @$tags ]; } - ############################## # Private Helper Subroutines # ############################## @@ -1323,6 +1391,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) = @_; @@ -1584,6 +1702,103 @@ You specified an invalid field name or id. =back +=head2 flag_types + +B<UNSTABLE> + +=over + +=item B<Description> + +Get information about valid flag types that can be set for bugs and attachments. + +=item B<REST> + +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/<product> + +To get information about flag_types for a product and component: + +GET /flag_types/<product>/<component> + +The returned data format is the same as below. + +=item B<Params> + +You must pass a product name and an optional component name. + +=over + +=item C<product> (string) - The name of a valid product. + +=item C<component> (string) - An optional valid component name associated with the product. + +=back + +=item B<Returns> + +A hash containing a two keys, C<bug> and C<attachment>. Each key value is an array of hashes, +containing the following keys: + +=over + +=item C<id> + +C<int> An integer id uniquely identifying this flag type. + +=item C<name> + +C<string> The name for the flag type. + +=item C<type> + +C<string> The target of the flag type which is either C<bug> or C<attachment>. + +=item C<description> + +C<string> The description of the flag type. + +=item C<values> + +C<array> An array of string values that the user can set on the flag type. + +=item C<is_requesteeble> + +C<boolean> Users can ask specific other users to set flags of this type. + +=item C<is_multiplicable> + +C<boolean> Multiple flags of this type can be set for the same bug or attachment. + +=back + +=item B<Errors> + +=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<History> + +=over + +=item Added in Bugzilla B<5.0>. + +=back + +=back =head2 legal_values @@ -3021,6 +3236,32 @@ Note that only certain statuses can be set on bug creation. =item C<target_milestone> (string) - A valid target milestone for this product. +=item C<flags> + +C<array> 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<name> + +C<string> The name of the flag type. + +=item C<type_id> + +C<int> The internal flag type id. + +=item C<status> + +C<string> The flags new status (i.e. "?", "+", "-" or "X" to clear a flag). + +=item C<requestee> + +C<string> 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 @@ -3073,6 +3314,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 @@ -3178,6 +3441,32 @@ to the "insidergroup"), False if the attachment should be public. Defaults to False if not specified. +=item C<flags> + +C<array> 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<name> + +C<string> The name of the flag type. + +=item C<type_id> + +C<int> The internal flag type id. + +=item C<status> + +C<string> The flags new status (i.e. "?", "+", "-" or "X" to clear a flag). + +=item C<requestee> + +C<string> The login of the requestee if the flag type is requesteeable. + +=back + =back =item B<Returns> @@ -3192,6 +3481,28 @@ This method can throw all the same errors as L</get>, 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. @@ -3289,6 +3600,41 @@ to the "insidergroup"), False if the attachment should be public. C<boolean> True if the attachment is obsolete, False otherwise. +=item C<flags> + +C<array> 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<name> + +C<string> The name of the flag that will be created or updated. + +=item C<type_id> + +C<int> The internal flag type id that will be created or updated. You will +need to specify the C<type_id> if more than one flag type of the same name exists. + +=item C<status> + +C<string> The flags new status (i.e. "?", "+", "-" or "X" to clear a flag). + +=item C<requestee> + +C<string> The login of the requestee if the flag type is requesteeable. + +=item C<id> + +C<int> Use id to specify the flag to be updated. You will need to specify the C<id> +if more than one flag is set of the same name. + +=item C<new> + +C<boolean> Set to true if you specifically want a new flag to be created. + =back =item B<Returns> @@ -3354,6 +3700,33 @@ This method can throw all the same errors as L</get>, 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<content_type> argument that was blank, not a valid @@ -3379,6 +3752,7 @@ You did not specify a value for the C<summary> argument. =back +=back =head2 add_comment @@ -3624,6 +3998,43 @@ duplicate bugs. C<double> The total estimate of time required to fix the bug, in hours. This is the I<total> estimate, not the amount of time remaining to fix it. +=item C<flags> + +C<array> 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<name> + +C<string> The name of the flag that will be created or updated. + +=item C<type_id> + +C<int> The internal flag type id that will be created or updated. You will +need to specify the C<type_id> if more than one flag type of the same name exists. + +=item C<status> + +C<string> The flags new status (i.e. "?", "+", "-" or "X" to clear a flag). + +=item C<requestee> + +C<string> The login of the requestee if the flag type is requesteeable. + +=item C<id> + +C<int> Use id to specify the flag to be updated. You will need to specify the C<id> +if more than one flag is set of the same name. + +=item C<new> + +C<boolean> Set to true if you specifically want a new flag to be created. + +=back + =item C<groups> C<hash> The groups a bug is in. To modify this field, pass a hash, which @@ -3942,6 +4353,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<History> diff --git a/Bugzilla/WebService/Constants.pm b/Bugzilla/WebService/Constants.pm index 87d890176..d0ce26caf 100644 --- a/Bugzilla/WebService/Constants.pm +++ b/Bugzilla/WebService/Constants.pm @@ -134,6 +134,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_username_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 a0421ad9a..27e06a1dd 100644 --- a/Bugzilla/WebService/Util.pm +++ b/Bugzilla/WebService/Util.pm @@ -21,6 +21,13 @@ package Bugzilla::WebService::Util; use strict; + +use Bugzilla::Flag; +use Bugzilla::FlagType; +use Bugzilla::Error; + +use Storable qw(dclone); + use base qw(Exporter); # We have to "require", not "use" this, because otherwise it tries to @@ -28,6 +35,7 @@ use base qw(Exporter); require Test::Taint; our @EXPORT_OK = qw( + extract_flags filter filter_wants taint_data @@ -37,6 +45,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; @@ -235,6 +317,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<Methods in need of POD> =over diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index 5851d439f..46e181409 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -737,6 +737,16 @@ You are not allowed to edit properties of the '[% flagtype.name FILTER html %]' flag type, because this flag type is not available for the products you can administer. + [% ELSIF error == "flag_not_unique" %] + [% title = "Flag not Unique" %] + The flag '[% value FILTER html %]' has been set multiple times. + You must specify the id value to update the flag. + + [% ELSIF error == "flag_type_not_unique" %] + [% title = "Flag Type not Unique" %] + The flag type '[% value FILTER html %]' matches several flag types. + You must specify the type id value to update or add a flag. + [% ELSIF error == "flag_type_not_multiplicable" %] [% docslinks = {'flags-overview.html' => 'An overview on Flags', 'flags.html' => 'Using Flags'} %] |