Always allow commenting a patch set

Prevent giving review scores but allow commenting for a change in
CI states (STAGING, STAGED or INTEGRATING). No need to block it with
error message like before. Also if the state has changed to one of
CI states while reviewing, review scores are dropped, just comments
are saved.

Task-number: QTQAINFRA-858
Change-Id: I684f004a1d680b2db184fedb50d936337f603721
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@digia.com>
Reviewed-by: Ismo Haataja <ismo.haataja@digia.com>

4 files changed, 29 insertions, 10 deletions

diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java
index ed519e9ead..ec737ea901 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java
@@ -634,15 +634,7 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel
     b.addClickHandler(new ClickHandler() {
       @Override
       public void onClick(final ClickEvent event) {
-        if (changeDetail.getChange().getStatus() == Change.Status.INTEGRATING
-            || changeDetail.getChange().getStatus() == Change.Status.STAGED
-            || changeDetail.getChange().getStatus() == Change.Status.STAGING) {
-          alertMessageBox(Util.C.headingReviewDisabled(),
-              Util.C.messageReviewDisabled())
-              .center();
-        } else {
-          Gerrit.display(Dispatcher.toPublish(patchSet.getId()));
-        }
+        Gerrit.display(Dispatcher.toPublish(patchSet.getId()));
       }
     });
     actionsPanel.add(b);
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java
index 46607c4749..d1aee8c4a2 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java
@@ -15,6 +15,7 @@
 
 package com.google.gerrit.client.changes;
 
+import com.google.gerrit.client.ErrorDialog;
 import com.google.gerrit.client.Gerrit;
 import com.google.gerrit.client.changes.ChangeInfo.ApprovalInfo;
 import com.google.gerrit.client.changes.ChangeInfo.LabelInfo;
@@ -340,7 +341,7 @@ public class PublishCommentScreen extends AccountScreen implements
     descBlock.display(changeDetail, null, false, r.getPatchSetInfo(), r.getAccounts(),
        r.getSubmitTypeRecord(), commentLinkProcessor);
 
-    if (r.getChange().getStatus().isOpen()) {
+    if (r.getChange().getStatus().isOpen() && !r.getChange().getStatus().isCI()) {
       initApprovals(approvalPanel);
       approvals.display(change);
     } else {
@@ -442,6 +443,9 @@ public class PublishCommentScreen extends AccountScreen implements
               staging();
             } else {
               saveStateOnUnload = false;
+              if (!result.getMessage().isEmpty()) {
+                new ErrorDialog(result.getMessage()).center();
+              }
               goChange();
             }
           }
@@ -467,6 +471,8 @@ public class PublishCommentScreen extends AccountScreen implements
       this.drafts = 'PUBLISH';
     }-*/;
 
+    public final native String getMessage() /*-{ return this.message; }-*/;
+
     protected ReviewInput() {
     }
   }
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java
index 14a32b94a8..98fbf37cfe 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java
@@ -342,6 +342,11 @@ public final class Change {
       return closed;
     }
 
+    public boolean isCI() {
+      return (code == STATUS_INTEGRATING
+          || code == STATUS_STAGED || code == STATUS_STAGING);
+    }
+
     public static Status forCode(final char c) {
       for (final Status s : Status.values()) {
         if (s.code == c) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
index 51720f2104..026a269e22 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
@@ -106,6 +106,7 @@ public class PostReview implements RestModifyView<RevisionResource, Input> {
 
   static class Output {
     Map<String, Short> labels;
+    String message;
   }
 
   private final ReviewDb db;
@@ -178,6 +179,14 @@ public class PostReview implements RestModifyView<RevisionResource, Input> {
 
     Output output = new Output();
     output.labels = input.labels;
+    if (input.labels != null && change.getStatus().isCI()) {
+      output.message =
+          "The change was staged while you were reviewing it. " +
+          "Due to this, only your comments were published, while your review scores were dropped.";
+    }
+    else {
+      output.message = "";
+    }
     return output;
   }
 
@@ -359,6 +368,13 @@ public class PostReview implements RestModifyView<RevisionResource, Input> {
         // TODO Allow updating some labels even when closed.
         continue;
       }
+      if (change.getStatus().isCI()) {
+        // If the state of a change is INTEGRATING, STAGED or STAGING review
+        // scores are not allowed. This check is necessary here because some
+        // other user may have changed the state of a change while another
+        // user is reviewing it. Scores are dropped but comments are kept.
+        continue;
+      }
 
       PatchSetApproval c = current.remove(name);
       if (ent.getValue() == null || ent.getValue() == 0) {