Bug 683025 - Add a check_for_edit to Bugzilla::Bug to return the bug object

if the user can edit the bug
r=mkanat, a=mkanat

4 files changed, 17 insertions, 37 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 7745a9809..23e07979f 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -403,6 +403,16 @@ sub check {
     return $self;
 }
 
+sub check_for_edit {
+    my $class = shift;
+    my $bug = $class->check(@_);
+
+    Bugzilla->user->can_edit_product($bug->product_id)
+        || ThrowUserError("product_edit_denied", { product => $bug->product });
+
+    return $bug;
+}
+
 sub check_is_visible {
     my $self = shift;
     my $user = Bugzilla->user;
diff --git a/Bugzilla/BugUrl/Bugzilla/Local.pm b/Bugzilla/BugUrl/Bugzilla/Local.pm
index 233acbe66..c052d7d3b 100644
--- a/Bugzilla/BugUrl/Bugzilla/Local.pm
+++ b/Bugzilla/BugUrl/Bugzilla/Local.pm
@@ -119,7 +119,7 @@ sub _check_value {
     }
 
     my $ref_bug_id  = $uri->query_param('id');
-    my $ref_bug     = Bugzilla::Bug->check($ref_bug_id);
+    my $ref_bug     = Bugzilla::Bug->check_for_edit($ref_bug_id);
     my $self_bug_id = $params->{bug_id};
     $params->{ref_bug} = $ref_bug;
 
@@ -127,12 +127,6 @@ sub _check_value {
         ThrowUserError('see_also_self_reference');
     }
  
-    my $product = $ref_bug->product_obj;
-    if (!Bugzilla->user->can_edit_product($product->id)) {
-        ThrowUserError("product_edit_denied",
-                       { product => $product->name });
-    }
-
     return $uri;
 }
 
diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm
index 7844b4e97..63d04bb0b 100644
--- a/Bugzilla/WebService/Bug.pm
+++ b/Bugzilla/WebService/Bug.pm
@@ -481,7 +481,7 @@ sub update {
     my $ids = delete $params->{ids};
     defined $ids || ThrowCodeError('param_required', { param => 'ids' });
 
-    my @bugs = map { Bugzilla::Bug->check($_) } @$ids;
+    my @bugs = map { Bugzilla::Bug->check_for_edit($_) } @$ids;
 
     my %values = %$params;
     $values{other_bugs} = \@bugs;
@@ -497,11 +497,6 @@ sub update {
     delete $values{flags};
 
     foreach my $bug (@bugs) {
-        if (!$user->can_edit_product($bug->product_obj->id) ) {
-            ThrowUserError("product_edit_denied",
-                          { product => $bug->product });
-        }
-
         $bug->set_all(\%values);
     }
 
@@ -632,11 +627,7 @@ sub add_attachment {
     defined $params->{data}
         || ThrowCodeError('param_required', { param => 'data' });
 
-    my @bugs = map { Bugzilla::Bug->check($_) } @{ $params->{ids} };
-    foreach my $bug (@bugs) {
-        Bugzilla->user->can_edit_product($bug->product_id)
-          || ThrowUserError("product_edit_denied", {product => $bug->product});
-    }
+    my @bugs = map { Bugzilla::Bug->check_for_edit($_) } @{ $params->{ids} };
 
     my @created;
     $dbh->bz_start_transaction();
@@ -681,11 +672,8 @@ sub add_comment {
     (defined $comment && trim($comment) ne '')
         || ThrowCodeError('param_required', { param => 'comment' });
     
-    my $bug = Bugzilla::Bug->check($params->{id});
+    my $bug = Bugzilla::Bug->check_for_edit($params->{id});
 
-    $user->can_edit_product($bug->product_id)
-        || ThrowUserError("product_edit_denied", {product => $bug->product});
-    
     # Backwards-compatibility for versions before 3.6    
     if (defined $params->{private}) {
         $params->{is_private} = delete $params->{private};
@@ -726,10 +714,7 @@ sub update_see_also {
 
     my @bugs;
     foreach my $id (@{ $params->{ids} }) {
-        my $bug = Bugzilla::Bug->check($id);
-        $user->can_edit_product($bug->product_id)
-            || ThrowUserError("product_edit_denied", 
-                              { product => $bug->product });
+        my $bug = Bugzilla::Bug->check_for_edit($id);
         push(@bugs, $bug);
         if ($remove) {
             $bug->remove_see_also($_) foreach @$remove;
diff --git a/process_bug.cgi b/process_bug.cgi
index 9ba03e277..dc5ad9138 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -96,14 +96,14 @@ sub should_set {
 # Create a list of objects for all bugs being modified in this request.
 my @bug_objects;
 if (defined $cgi->param('id')) {
-  my $bug = Bugzilla::Bug->check(scalar $cgi->param('id'));
+  my $bug = Bugzilla::Bug->check_for_edit(scalar $cgi->param('id'));
   $cgi->param('id', $bug->id);
   push(@bug_objects, $bug);
 } else {
     foreach my $i ($cgi->param()) {
         if ($i =~ /^id_([1-9][0-9]*)/) {
             my $id = $1;
-            push(@bug_objects, Bugzilla::Bug->check($id));
+            push(@bug_objects, Bugzilla::Bug->check_for_edit($id));
         }
     }
 }
@@ -213,15 +213,6 @@ else {
     $action = 'nothing';
 }
 
-# For each bug, we have to check if the user can edit the bug the product
-# is currently in, before we allow them to change anything.
-foreach my $bug (@bug_objects) {
-    if (!$user->can_edit_product($bug->product_obj->id)) {
-        ThrowUserError("product_edit_denied",
-                      { product => $bug->product });
-    }
-}
-
 # Component, target_milestone, and version are in here just in case
 # the 'product' field wasn't defined in the CGI. It doesn't hurt to set
 # them twice.