[bugzilla-gnome-org-extensions] Add Splinter.publish_review XML-RPC method

[Krzesimir Nowak <krnowak src gnome org>]

commit 45f9f0ea1b9f4f79593f26b7572cbd5d80341b79
Author: Owen W. Taylor <otaylor fishsoup net>
Date:   Mon Sep 28 00:41:45 2009 -0400

    Add Splinter.publish_review XML-RPC method
    
    Add an XML-RPC method to "publish a review" - this is pretty much like
    Bug.add_comment(), but allows us to update the attachment status
    simultaneously and only send out one email.
    
    Use that, when the extension is available, to publish reviews.
    
    Add xmlrpc.cgi to the list of URL's whitelisted for forwarding
    in splinter_proxy.py, to allow using the proxy for developing against
    client code an extension-enabled server.

 extension/lib/WSSplinter.pm |  133 ++++++++++++++++++++++++++++++++++++++++--
 js/splinter.js              |   57 +++++++++++++-----
 proxy/splinter_proxy.py     |    3 +-
 3 files changed, 169 insertions(+), 24 deletions(-)
---
diff --git a/extension/lib/WSSplinter.pm b/extension/lib/WSSplinter.pm
index 3c3d3f4..771a7fd 100644
--- a/extension/lib/WSSplinter.pm
+++ b/extension/lib/WSSplinter.pm
@@ -10,20 +10,22 @@
 # implied. See the License for the specific language governing
 # rights and limitations under the License.
 #
-# The Original Code is the Splinter Bugzilla Extension.
+# The Original Code is the Bugzilla Bug Tracking System.
 #
-# The Initial Developer of the Original Code is Red Hat, Inc.
-# Portions created by Red Hat, Inc. are Copyright (C) 2009
-# Red Hat Inc. All Rights Reserved.
-#
-# Contributor(s):
-#   Owen Taylor <otaylor fishsoup net>
+# Contributor(s):  Frédéric Buclin <LpSolit gmail com>
+#                  Max Kanat-Alexander <mkanat bugzilla org>
+#                  Owen Taylor <otaylor fishsoup net>
 
 package extensions::splinter::lib::WSSplinter;
 use strict;
 use warnings;
 
 use Bugzilla;
+use Bugzilla::Attachment;
+use Bugzilla::BugMail;
+use Bugzilla::Constants;
+use Bugzilla::Field;
+use Bugzilla::Util qw(trim);
 
 use base qw(Bugzilla::WebService);
 
@@ -45,4 +47,121 @@ sub info {
     return $results;
 }
 
+# Make sure the current user has access to the specified attachment;
+# cut and paste from attachmnet.cgi
+sub check_can_access {
+    my $attachment = shift;
+    my $user = Bugzilla->user;
+
+    # Make sure the user is authorized to access this attachment's bug.
+    Bugzilla::Bug->check($attachment->bug_id);
+    if ($attachment->isprivate && $user->id != $attachment->attacher->id 
+        && !$user->is_insider) 
+    {
+        ThrowUserError('auth_failure', {action => 'access',
+                                        object => 'attachment'});
+    }
+    return 1;
+}
+
+# The idea of this method is to be able to
+#
+#  - Add a comment with says "Review of attachment <id>:" rather than
+#    "From update of attachment"
+#
+# and:
+#
+#  - Update the attachment status (in the future flags as well)
+#
+# While sending out only a single mail as the result. If we did one post
+# to processs_bug.cgi and one to attachment.cgi, we'd get two mails.
+#
+# Based upon WebServer::Bug::add_comment() and attachment.cgi
+sub publish_review {
+    my ($self, $params) = @_;
+
+    # The user must login in order to publish a review
+    Bugzilla->login(LOGIN_REQUIRED);
+
+    # Check parameters
+    defined $params->{attachment_id}
+        || ThrowCodeError('param_required', { param => 'attachment_id' });
+    my $review = $params->{review};
+    (defined $review && trim($review) ne '')
+        || ThrowCodeError('param_required', { param => 'review' });
+
+    my $attachment_status = $params->{attachment_status};
+    if (defined $attachment_status) {
+        my $field_object = new Bugzilla::Field({ name => 'attachments.status' });
+        my $legal_values = [map { $_->name } @{ $field_object->legal_values }];
+        check_field('attachments.status', $attachment_status, $legal_values);
+    }
+
+    my $attachment = new Bugzilla::Attachment($params->{attachment_id});
+    defined $attachment
+        || ThrowUserError("invalid_attach_id",
+                          { attach_id => $params->{attachment_id} });
+
+    # Publishing a review of an attachment you can't access doesn't leak
+    # information about that attachment, but it seems like bad policy to
+    # allow it.
+    check_can_access($attachment);
+
+    my $bug = new Bugzilla::Bug($attachment->bug_id);
+
+    Bugzilla->user->can_edit_product($bug->product_id)
+        || ThrowUserError("product_edit_denied", {product => $bug->product});
+
+    # This is a "magic string" used to identify review comments
+    my $comment = "Review of attachment " . $attachment->id . ":\n\n" . $review;
+
+    my $dbh = Bugzilla->dbh;
+
+    # Figure out when the changes were made.
+    my ($timestamp) = $dbh->selectrow_array("SELECT NOW()");
+
+    # Append review comment
+    $bug->add_comment($comment);
+
+    $dbh->bz_start_transaction();
+
+    if (defined $attachment_status && $attachment->status ne $attachment_status) {
+        # Note that this file needs to load properly even if the installation
+        # doesn't have attachment statuses (a bugzilla.gnome.org addition), so,
+        # for example, we wouldn't want an explicit 'use Bugzilla::AttachmentStatus'
+
+        # Update the attachment record in the database.
+        $dbh->do("UPDATE  attachments
+                  SET     status      = ?,
+                          modification_time = ?
+                  WHERE   attach_id   = ?",
+                  undef, ($attachment_status, $timestamp, $attachment->id));
+
+        my $updated_attachment = new Bugzilla::Attachment($attachment->id);
+
+        if ($attachment->status ne $updated_attachment->status) {
+            my $fieldid = get_field_id('attachments.status');
+            $dbh->do('INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when,
+                                               fieldid, removed, added)
+                           VALUES (?, ?, ?, ?, ?, ?, ?)',
+                     undef, ($bug->id, $attachment->id, Bugzilla->user->id,
+                             $timestamp, $fieldid,
+                             $attachment->status, $updated_attachment->status));
+
+            # Adding the comment will update the bug's delta_ts, so we don't need to do it here
+        }
+    }
+
+    # This actually adds the comment
+    $bug->update();
+
+    $dbh->bz_commit_transaction();
+
+    # Send mail.
+    Bugzilla::BugMail::Send($bug->bug_id, { changer => Bugzilla->user->login });
+
+    # Nothing very interesting to return on success, so just return an empty structure
+    return {};
+}
+
 1;
diff --git a/js/splinter.js b/js/splinter.js
index 6ffe659..d11a452 100644
--- a/js/splinter.js
+++ b/js/splinter.js
@@ -3,6 +3,7 @@ include('Bug');
 include('Patch');
 include('Review');
 include('ReviewStorage');
+include('XmlRpc');
 
 var reviewStorage;
 var attachmentId;
@@ -94,8 +95,6 @@ function publishReview() {
     saveComment();
     theReview.setIntro($("#myComment").val());
 
-    var comment = "Review of attachment " + attachmentId + ":\n\n" + theReview;
-
     var newStatus = null;
     if (theAttachment.status && $("#attachmentStatus").val() != theAttachment.status) {
         newStatus = $("#attachmentStatus").val();
@@ -107,20 +106,46 @@ function publishReview() {
         document.location = newPageUrl(theBug.id);
     }
 
-    addComment(theBug, comment,
-               function(detail) {
-                   if (newStatus)
-                       updateAttachmentStatus(theAttachment, newStatus,
-                                              success,
-                                              function() {
-                                                  displayError("Published review; patch status could not be 
updated.");
-                                              });
-                   else
-                       success();
-               },
-               function(detail) {
-                   displayError("Failed to publish review.");
-               });
+    if (configHaveExtension) {
+        var params = {
+            attachment_id: theAttachment.id,
+            review: theReview.toString()
+        };
+
+        if (newStatus != null)
+            params['attachment_status'] = newStatus;
+
+        XmlRpc.call({
+                        url: '/xmlrpc.cgi',
+                        name: 'Splinter.publish_review',
+                        params: params,
+                        error: function(message) {
+                            displayError("Failed to publish review: " + message);
+                        },
+                        fault: function(faultCode, faultString) {
+                            displayError("Failed to publish review: " + faultString);
+                        },
+                        success: function(result) {
+                            success();
+                        }
+                    });
+    } else {
+        var comment = "Review of attachment " + attachmentId + ":\n\n" + theReview;
+        addComment(theBug, comment,
+                   function(detail) {
+                       if (newStatus)
+                           updateAttachmentStatus(theAttachment, newStatus,
+                                                  success,
+                                                  function() {
+                                                      displayError("Published review; patch status could not 
be updated.");
+                                                  });
+                       else
+                           success();
+                   },
+                   function(detail) {
+                       displayError("Failed to publish review.");
+                   });
+    }
 }
 
 function hideSaveDraftNotice() {
diff --git a/proxy/splinter_proxy.py b/proxy/splinter_proxy.py
index 15d9031..7ac785f 100755
--- a/proxy/splinter_proxy.py
+++ b/proxy/splinter_proxy.py
@@ -22,7 +22,8 @@ import xmlrpclib
 PROXIED_PATHS = [
     "/attachment.cgi",
     "/process_bug.cgi",
-    "/show_bug.cgi"
+    "/show_bug.cgi",
+    "/xmlrpc.cgi"
 ]
 
 def is_proxied(path):