Bug 915685 - backport upstream bug 914986 and bug 917483 to bmo/4.2 for allowing attachment metadata editing in webservice API

4 files changed, 267 insertions, 5 deletions

diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm
index bf67b62fa..944337711 100644
--- a/Bugzilla/Attachment.pm
+++ b/Bugzilla/Attachment.pm
@@ -913,10 +913,11 @@ sub update {
     }
 
     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, $self->bug_id));
+        $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, $self->bug_id));
+        $self->{modification_time} = $timestamp;
     }
 
     return $changes;
diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm
index a107da923..b47cf7366 100644
--- a/Bugzilla/WebService/Bug.pm
+++ b/Bugzilla/WebService/Bug.pm
@@ -30,7 +30,7 @@ use Bugzilla::Constants;
 use Bugzilla::Error;
 use Bugzilla::Field;
 use Bugzilla::WebService::Constants;
-use Bugzilla::WebService::Util qw(filter filter_wants validate);
+use Bugzilla::WebService::Util qw(filter filter_wants validate translate);
 use Bugzilla::Bug;
 use Bugzilla::BugMail;
 use Bugzilla::Util qw(trick_taint trim);
@@ -68,6 +68,20 @@ use constant READ_ONLY => qw(
     search
 );
 
+use constant ATTACHMENT_MAPPED_SETTERS => {
+    file_name => 'filename',
+    summary   => 'description',
+};
+
+use constant ATTACHMENT_MAPPED_RETURNS => {
+    description => 'summary',
+    ispatch     => 'is_patch',
+    isprivate   => 'is_private',
+    isobsolete  => 'is_obsolete',
+    filename    => 'file_name',
+    mimetype    => 'content_type',
+};
+
 ######################################################
 # Add aliases here for old method name compatibility #
 ######################################################
@@ -788,6 +802,86 @@ sub add_attachment {
     return { attachments => \%attachments };
 }
 
+sub update_attachment {
+    my ($self, $params) = validate(@_, 'ids');
+
+    my $user = Bugzilla->login(LOGIN_REQUIRED);
+    my $dbh = Bugzilla->dbh;
+
+    my $ids = delete $params->{ids};
+    defined $ids || ThrowCodeError('param_required', { param => 'ids' });
+
+    # Some fields cannot be sent to set_all
+    foreach my $key (qw(login password token)) {
+        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
+    my @attachments = ();
+    my %bugs = ();
+    foreach my $id (@$ids) {
+        my $attachment = Bugzilla::Attachment->new($id)
+          || ThrowUserError("invalid_attach_id", { attach_id => $id });
+        my $bug = $attachment->bug;
+        $attachment->_check_bug;
+        $attachment->validate_can_edit($bug->product_id)
+          || ThrowUserError("illegal_attachment_edit", { attach_id => $id });
+
+        push @attachments, $attachment;
+        $bugs{$bug->id} = $bug;
+    }
+
+    # Update the values
+    foreach my $attachment (@attachments) {
+        $attachment->set_all($params);
+    }
+
+    $dbh->bz_start_transaction();
+
+    # Do the actual update and get information to return to user
+    my @result;
+    foreach my $attachment (@attachments) {
+        my $changes = $attachment->update();
+
+        $changes = translate($changes, ATTACHMENT_MAPPED_RETURNS);
+
+        my %hash = (
+            id               => $self->type('int', $attachment->id),
+            last_change_time => $self->type('dateTime', $attachment->modification_time),
+            changes          => {},
+        );
+
+        foreach my $field (keys %$changes) {
+            my $change = $changes->{$field};
+
+            # We normalize undef to an empty string, so that the API
+            # stays consistent for things like Deadline that can become
+            # empty.
+            $hash{changes}->{$field} = {
+                removed => $self->type('string', $change->[0] // ''),
+                added   => $self->type('string', $change->[1] // '')
+            };
+        }
+
+        push(@result, \%hash);
+    }
+
+    $dbh->bz_commit_transaction();
+
+    # Email users about the change
+    foreach my $bug (values %bugs) {
+        Bugzilla::BugMail::Send($bug->id, { 'changer' => $user });
+    }
+
+    # Return the information to the user
+    return { attachments => \@result };
+}
+
 sub add_comment {
     my ($self, $params) = @_;
     
@@ -2969,6 +3063,156 @@ You set the "data" field to an empty string.
 =back
 
 
+=head2 update_attachment
+
+B<UNSTABLE>
+
+=over
+
+=item B<Description>
+
+This allows you to update attachment metadata in Bugzilla.
+
+=item B<REST>
+
+To update attachment metadata on a current attachment:
+
+PUT /bug/attachment/<attach_id>
+
+The params to include in the POST body, as well as the returned
+data format are the same as below. The C<ids> param will be
+overridden as it it pulled from the URL path.
+
+=item B<Params>
+
+=over
+
+=item C<ids>
+
+B<Required> C<array> An array of integers -- the ids of the attachments you
+want to update.
+
+=item C<file_name>
+
+C<string> The "file name" that will be displayed
+in the UI for this attachment.
+
+=item C<summary>
+
+C<string> A short string describing the
+attachment.
+
+=item C<content_type>
+
+C<string> The MIME type of the attachment, like
+C<text/plain> or C<image/png>.
+
+=item C<is_patch>
+
+C<boolean> True if Bugzilla should treat this attachment as a patch.
+If you specify this, you do not need to specify a C<content_type>.
+The C<content_type> of the attachment will be forced to C<text/plain>.
+
+=item C<is_private>
+
+C<boolean> True if the attachment should be private (restricted
+to the "insidergroup"), False if the attachment should be public.
+
+=item C<is_obsolete>
+
+C<boolean> True if the attachment is obsolete, False otherwise.
+
+=back
+
+=item B<Returns>
+
+A C<hash> with a single field, "attachment". This points to an array of hashes
+with the following fields:
+
+=over
+
+=item C<id>
+
+C<int> The id of the attachment that was updated.
+
+=item C<last_change_time>
+
+C<dateTime> The exact time that this update was done at, for this attachment.
+If no update was done (that is, no fields had their values changed and
+no comment was added) then this will instead be the last time the attachment
+was updated.
+
+=item C<changes>
+
+C<hash> The changes that were actually done on this bug. The keys are
+the names of the fields that were changed, and the values are a hash
+with two keys:
+
+=over
+
+=item C<added> (C<string>) The values that were added to this field.
+possibly a comma-and-space-separated list if multiple values were added.
+
+=item C<removed> (C<string>) The values that were removed from this
+field.
+
+=back
+
+=back
+
+Here's an example of what a return value might look like:
+
+ {
+   attachments => [
+     {
+       id    => 123,
+       last_change_time => '2010-01-01T12:34:56',
+       changes => {
+         summary => {
+           removed => 'Sample ptach',
+           added   => 'Sample patch'
+         },
+         is_obsolete => {
+           removed => '0',
+           added   => '1',
+         }
+       },
+     }
+   ]
+ }
+
+=item B<Errors>
+
+This method can throw all the same errors as L</get>, plus:
+
+=over
+
+=item 601 (Invalid MIME Type)
+
+You specified a C<content_type> argument that was blank, not a valid
+MIME type, or not a MIME type that Bugzilla accepts for attachments.
+
+=item 603 (File Name Not Specified)
+
+You did not specify a valid for the C<file_name> argument.
+
+=item 604 (Summary Required)
+
+You did not specify a value for the C<summary> argument.
+
+=back
+
+=item B<History>
+
+=over
+
+=item Added in Bugzilla B<5.0>.
+
+=back
+
+=back
+
+
 =head2 add_comment
 
 B<STABLE>
diff --git a/Bugzilla/WebService/Server/REST/Resources/Bug.pm b/Bugzilla/WebService/Server/REST/Resources/Bug.pm
index a82625be7..98ae6049c 100644
--- a/Bugzilla/WebService/Server/REST/Resources/Bug.pm
+++ b/Bugzilla/WebService/Server/REST/Resources/Bug.pm
@@ -94,6 +94,12 @@ sub _rest_resources {
                 params => sub {
                     return { attachment_ids => [ $_[0] ] };
                 }
+            },
+            PUT => {
+                method => 'update_attachment',
+                params => sub {
+                    return { ids => [ $_[0] ] };
+                }
             }
         },
         qr{^/field/bug$}, {
diff --git a/Bugzilla/WebService/Util.pm b/Bugzilla/WebService/Util.pm
index 622e6890b..a0421ad9a 100644
--- a/Bugzilla/WebService/Util.pm
+++ b/Bugzilla/WebService/Util.pm
@@ -32,6 +32,7 @@ our @EXPORT_OK = qw(
     filter_wants
     taint_data
     validate
+    translate
     params_to_objects
     fix_credentials
 );
@@ -131,6 +132,16 @@ sub validate  {
     return ($self, $params);
 }
 
+sub translate {
+    my ($params, $mapped) = @_;
+    my %changes;
+    while (my ($key,$value) = each (%$params)) {
+        my $new_field = $mapped->{$key} || $key;
+        $changes{$new_field} = $value;
+    }
+    return \%changes;
+}
+
 sub params_to_objects {
     my ($params, $class) = @_;
     my (@objects, @objects_by_ids);