Bug 140999: Users without edit permissions for an attachment should still be able to make comments - Patch by Frédéric Buclin <LpSolit@gmail.com> a=LpSolit

1 files changed, 45 insertions, 37 deletions

diff --git a/attachment.cgi b/attachment.cgi
index bbbf4afb3..32f6e5ec0 100755
--- a/attachment.cgi
+++ b/attachment.cgi
@@ -572,37 +572,39 @@ sub update {
     my $attachment = validateID();
     my $bug = $attachment->bug;
     $attachment->_check_bug;
-    $attachment->validate_can_edit($bug->product_id); # FIXME: allow comments anyway.
-
-    $attachment->set_description(scalar $cgi->param('description'));
-    $attachment->set_is_patch(scalar $cgi->param('ispatch'));
-    $attachment->set_content_type(scalar $cgi->param('contenttypeentry'));
-    $attachment->set_is_obsolete(scalar $cgi->param('isobsolete'));
-    $attachment->set_is_private(scalar $cgi->param('isprivate'));
-    $attachment->set_filename(scalar $cgi->param('filename'));
-
-    # Now make sure the attachment has not been edited since we loaded the page.
-    if (defined $cgi->param('delta_ts')
-        && $cgi->param('delta_ts') ne $attachment->modification_time)
-    {
-        ($vars->{'operations'}) =
-            Bugzilla::Bug::GetBugActivity($bug->id, $attachment->id, $cgi->param('delta_ts'));
-
-        # The token contains the old modification_time. We need a new one.
-        $cgi->param('token', issue_hash_token([$attachment->id, $attachment->modification_time]));
-
-        # If the modification date changed but there is no entry in
-        # the activity table, this means someone commented only.
-        # In this case, there is no reason to midair.
-        if (scalar(@{$vars->{'operations'}})) {
-            $cgi->param('delta_ts', $attachment->modification_time);
-            $vars->{'attachment'} = $attachment;
-
-            print $cgi->header();
-            # Warn the user about the mid-air collision and ask them what to do.
-            $template->process("attachment/midair.html.tmpl", $vars)
-              || ThrowTemplateError($template->error());
-            exit;
+    my $can_edit = $attachment->validate_can_edit($bug->product_id);
+
+    if ($can_edit) {
+        $attachment->set_description(scalar $cgi->param('description'));
+        $attachment->set_is_patch(scalar $cgi->param('ispatch'));
+        $attachment->set_content_type(scalar $cgi->param('contenttypeentry'));
+        $attachment->set_is_obsolete(scalar $cgi->param('isobsolete'));
+        $attachment->set_is_private(scalar $cgi->param('isprivate'));
+        $attachment->set_filename(scalar $cgi->param('filename'));
+
+        # Now make sure the attachment has not been edited since we loaded the page.
+        if (defined $cgi->param('delta_ts')
+            && $cgi->param('delta_ts') ne $attachment->modification_time)
+        {
+            ($vars->{'operations'}) =
+                Bugzilla::Bug::GetBugActivity($bug->id, $attachment->id, $cgi->param('delta_ts'));
+
+            # The token contains the old modification_time. We need a new one.
+            $cgi->param('token', issue_hash_token([$attachment->id, $attachment->modification_time]));
+
+            # If the modification date changed but there is no entry in
+            # the activity table, this means someone commented only.
+            # In this case, there is no reason to midair.
+            if (scalar(@{$vars->{'operations'}})) {
+                $cgi->param('delta_ts', $attachment->modification_time);
+                $vars->{'attachment'} = $attachment;
+
+                print $cgi->header();
+                # Warn the user about the mid-air collision and ask them what to do.
+                $template->process("attachment/midair.html.tmpl", $vars)
+                  || ThrowTemplateError($template->error());
+                exit;
+            }
         }
     }
 
@@ -622,16 +624,22 @@ sub update {
         $bug->add_comment($comment, { isprivate => $attachment->isprivate });
     }
 
-    my ($flags, $new_flags) = Bugzilla::Flag->extract_flags_from_cgi($bug, $attachment, $vars);
-    $attachment->set_flags($flags, $new_flags);
+    if ($can_edit) {
+        my ($flags, $new_flags) =
+          Bugzilla::Flag->extract_flags_from_cgi($bug, $attachment, $vars);
+        $attachment->set_flags($flags, $new_flags);
+    }
 
     # Figure out when the changes were made.
     my $timestamp = $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
 
-    my $changes = $attachment->update($timestamp);
-    # If there are changes, we updated delta_ts in the DB. We have to
-    # reflect this change in the bug object.
-    $bug->{delta_ts} = $timestamp if scalar(keys %$changes);
+    if ($can_edit) {
+        my $changes = $attachment->update($timestamp);
+        # If there are changes, we updated delta_ts in the DB. We have to
+        # reflect this change in the bug object.
+        $bug->{delta_ts} = $timestamp if scalar(keys %$changes);
+    }
+
     # Commit the comment, if any.
     $bug->update($timestamp);