Merge "Update web UI to use /review"

gerrit-common/src/main/java/com/google/gerrit/common/data/PatchDetailService.java

@@ -18,17 +18,16 @@ import com.google.gerrit.common.audit.Audit;
 import com.google.gerrit.common.auth.SignInRequired;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountDiffPreference;
-import com.google.gerrit.reviewdb.client.ApprovalCategoryValue;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Patch;
+import com.google.gerrit.reviewdb.client.Patch.Key;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.Patch.Key;
 import com.google.gwtjsonrpc.common.AsyncCallback;
 import com.google.gwtjsonrpc.common.RemoteJsonService;
 import com.google.gwtjsonrpc.common.RpcImpl;
-import com.google.gwtjsonrpc.common.VoidResult;
 import com.google.gwtjsonrpc.common.RpcImpl.Version;
+import com.google.gwtjsonrpc.common.VoidResult;
 
 import java.util.List;
 import java.util.Set;
@@ -65,12 +64,6 @@ public interface PatchDetailService extends RemoteJsonService {
   @SignInRequired
   void deleteDraftPatchSet(PatchSet.Id psid, AsyncCallback<ChangeDetail> callback);
 
-  @Audit
-  @SignInRequired
-  void publishComments(PatchSet.Id psid, String message,
-      Set<ApprovalCategoryValue.Id> approvals,
-      AsyncCallback<VoidResult> callback);
-
   @Audit
   @SignInRequired
   void addReviewers(Change.Id id, List<String> reviewers, boolean confirmed,

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java

@@ -70,7 +70,7 @@ public class ChangeApi {
     return new RestApi("/changes/" + id + "/" + action);
   }
 
-  private static String emptyToNull(String str) {
+  public static String emptyToNull(String str) {
     return str == null || str.isEmpty() ? null : str;
   }
 }

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java

@@ -17,8 +17,8 @@ package com.google.gerrit.client.changes;
 import com.google.gerrit.client.Gerrit;
 import com.google.gerrit.client.patches.CommentEditorContainer;
 import com.google.gerrit.client.patches.CommentEditorPanel;
-import com.google.gerrit.client.patches.PatchUtil;
 import com.google.gerrit.client.rpc.GerritCallback;
+import com.google.gerrit.client.rpc.RestApi;
 import com.google.gerrit.client.rpc.ScreenLoadCallback;
 import com.google.gerrit.client.ui.AccountScreen;
 import com.google.gerrit.client.ui.PatchLink;
@@ -36,6 +36,7 @@ import com.google.gerrit.reviewdb.client.Patch;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
+import com.google.gwt.core.client.JavaScriptObject;
 import com.google.gwt.event.dom.client.ClickEvent;
 import com.google.gwt.event.dom.client.ClickHandler;
 import com.google.gwt.user.client.ui.Button;
@@ -53,7 +54,6 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 
@@ -62,6 +62,7 @@ public class PublishCommentScreen extends AccountScreen implements
   private static SavedState lastState;
 
   private final PatchSet.Id patchSetId;
+  private String revision;
   private Collection<ValueRadioButton> approvalButtons;
   private ChangeDescriptionBlock descBlock;
   private ApprovalTable approvals;
@@ -252,8 +253,7 @@ public class PublishCommentScreen extends AccountScreen implements
         continue;
       }
 
-      final ValueRadioButton b =
+      ValueRadioButton b = new ValueRadioButton(ct.getCategory(), buttonValue);
-          new ValueRadioButton(buttonValue, ct.getCategory().getName());
       b.setText(buttonValue.format());
 
       if (lastState != null && patchSetId.equals(lastState.patchSetId)
@@ -292,6 +292,7 @@ public class PublishCommentScreen extends AccountScreen implements
 
     draftsPanel.clear();
     commentEditors = new ArrayList<CommentEditorPanel>();
+    revision = r.getPatchSetInfo().getRevId();
 
     if (!r.getDrafts().isEmpty()) {
       draftsPanel.add(new SmallHeading(Util.C.headingPatchComments()));
@@ -348,19 +349,22 @@ public class PublishCommentScreen extends AccountScreen implements
   }
 
   private void onSend2(final boolean submit) {
-    final Map<ApprovalCategory.Id, ApprovalCategoryValue.Id> values =
+    ReviewInput data = ReviewInput.create();
-        new HashMap<ApprovalCategory.Id, ApprovalCategoryValue.Id>();
+    data.message(ChangeApi.emptyToNull(message.getText().trim()));
+    data.init();
     for (final ValueRadioButton b : approvalButtons) {
       if (b.getValue()) {
-        values.put(b.value.getCategoryId(), b.value.getId());
+        data.label(b.category.getLabelName(), b.value.getValue());
       }
     }
 
     enableForm(false);
-    PatchUtil.DETAIL_SVC.publishComments(patchSetId, message.getText().trim(),
+    new RestApi("/changes/" + patchSetId.getParentKey().get()
-        new HashSet<ApprovalCategoryValue.Id>(values.values()),
+        + "/revisions/" + revision + "/review")
-        new GerritCallback<VoidResult>() {
+      .data(data)
-          public void onSuccess(final VoidResult result) {
+      .post(new GerritCallback<ReviewInput>() {
+          @Override
+          public void onSuccess(ReviewInput result) {
             if (submit) {
               submit();
             } else {
@@ -377,6 +381,23 @@ public class PublishCommentScreen extends AccountScreen implements
         });
   }
 
+  private static class ReviewInput extends JavaScriptObject {
+    static ReviewInput create() {
+      return (ReviewInput) createObject();
+    }
+
+    final native void message(String m) /*-{ if(m)this.message=m; }-*/;
+    final native void label(String n, short v) /*-{ this.labels[n]=v; }-*/;
+    final native void init() /*-{
+      this.labels = {};
+      this.strict_labels = true;
+      this.drafts = 'PUBLISH';
+    }-*/;
+
+    protected ReviewInput() {
+    }
+  }
+
   private void submit() {
     Util.MANAGE_SVC.submit(patchSetId,
         new GerritCallback<ChangeDetail>() {
@@ -399,10 +420,12 @@ public class PublishCommentScreen extends AccountScreen implements
   }
 
   private static class ValueRadioButton extends RadioButton {
+    final ApprovalCategory category;
     final ApprovalCategoryValue value;
 
-    ValueRadioButton(final ApprovalCategoryValue v, final String label) {
+    ValueRadioButton(ApprovalCategory c, ApprovalCategoryValue v) {
-      super(label);
+      super(c.getLabelName());
+      category = c;
       value = v;
     }
   }

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java

@@ -24,25 +24,22 @@ import com.google.gerrit.common.data.ReviewResult;
 import com.google.gerrit.common.data.ReviewerResult;
 import com.google.gerrit.common.errors.NoSuchEntityException;
 import com.google.gerrit.httpd.rpc.BaseServiceImplementation;
-import com.google.gerrit.httpd.rpc.Handler;
 import com.google.gerrit.httpd.rpc.changedetail.ChangeDetailFactory;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountDiffPreference;
 import com.google.gerrit.reviewdb.client.AccountPatchReview;
 import com.google.gerrit.reviewdb.client.ApprovalCategory;
-import com.google.gerrit.reviewdb.client.ApprovalCategoryValue;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Patch;
+import com.google.gerrit.reviewdb.client.Patch.Key;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
-import com.google.gerrit.reviewdb.client.Patch.Key;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.account.AccountInfoCacheFactory;
 import com.google.gerrit.server.changedetail.DeleteDraftPatchSet;
 import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
-import com.google.gerrit.server.patch.PublishComments;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.workflow.FunctionState;
@@ -71,7 +68,6 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
   private final DeleteDraftPatchSet.Factory deleteDraftPatchSetFactory;
   private final RemoveReviewerHandler.Factory removeReviewerHandlerFactory;
   private final FunctionState.Factory functionStateFactory;
-  private final PublishComments.Factory publishCommentsFactory;
   private final PatchScriptFactory.Factory patchScriptFactoryFactory;
   private final SaveDraft.Factory saveDraftFactory;
   private final ChangeDetailFactory.Factory changeDetailFactory;
@@ -87,7 +83,6 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
       final DeleteDraftPatchSet.Factory deleteDraftPatchSetFactory,
       final FunctionState.Factory functionStateFactory,
       final PatchScriptFactory.Factory patchScriptFactoryFactory,
-      final PublishComments.Factory publishCommentsFactory,
       final SaveDraft.Factory saveDraftFactory,
       final ChangeDetailFactory.Factory changeDetailFactory) {
     super(schema, currentUser);
@@ -100,7 +95,6 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
     this.deleteDraftPatchSetFactory = deleteDraftPatchSetFactory;
     this.functionStateFactory = functionStateFactory;
     this.patchScriptFactoryFactory = patchScriptFactoryFactory;
-    this.publishCommentsFactory = publishCommentsFactory;
     this.saveDraftFactory = saveDraftFactory;
     this.changeDetailFactory = changeDetailFactory;
   }
@@ -178,12 +172,6 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
     });
   }
 
-  public void publishComments(final PatchSet.Id psid, final String msg,
-      final Set<ApprovalCategoryValue.Id> tags,
-      final AsyncCallback<VoidResult> cb) {
-    Handler.wrap(publishCommentsFactory.create(psid, msg, tags, false)).to(cb);
-  }
-
   /**
    * Update the reviewed status for the file by user @code{account}
    */

gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java

@@ -51,14 +51,14 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
-class PostReview implements RestModifyView<RevisionResource, Input> {
+public class PostReview implements RestModifyView<RevisionResource, Input> {
   private static final Logger log = LoggerFactory.getLogger(PostReview.class);
 
-  static class Input {
+  public static class Input {
     @DefaultInput
-    String message;
+    public String message;
 
-    Map<String, Short> labels;
+    public Map<String, Short> labels;
     Map<String, List<Comment>> comments;
 
     /**
@@ -68,16 +68,16 @@ class PostReview implements RestModifyView<RevisionResource, Input> {
      * execute anyway, but the proposed labels given by the user will be
      * modified to be the "best" value allowed by the access controls.
      */
-    boolean strictLabels = true;
+    public boolean strictLabels = true;
 
     /**
      * How to process draft comments already in the database that were not also
      * described in this input request.
      */
-    DraftHandling drafts = DraftHandling.DELETE;
+    public DraftHandling drafts = DraftHandling.DELETE;
   }
 
-  static enum DraftHandling {
+  public static enum DraftHandling {
     DELETE, PUBLISH, KEEP;
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java

@@ -44,7 +44,6 @@ import com.google.gerrit.server.mail.RebasedPatchSetSender;
 import com.google.gerrit.server.mail.ReplacePatchSetSender;
 import com.google.gerrit.server.mail.RestoredSender;
 import com.google.gerrit.server.patch.AddReviewer;
-import com.google.gerrit.server.patch.PublishComments;
 import com.google.gerrit.server.patch.RemoveReviewer;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.CreateProject;
@@ -81,7 +80,6 @@ public class GerritRequestModule extends FactoryModule {
     factory(AddReviewerSender.Factory.class);
     factory(CreateChangeSender.Factory.class);
     factory(DeleteDraftPatchSet.Factory.class);
-    factory(PublishComments.Factory.class);
     factory(PublishDraft.Factory.class);
     factory(RebaseChange.Factory.class);
     factory(ReplacePatchSetSender.Factory.class);

gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java

@@ -1,380 +0,0 @@
-// Copyright (C) 2009 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server.patch;
-
-import com.google.gerrit.common.ChangeHooks;
-import com.google.gerrit.common.data.ApprovalType;
-import com.google.gerrit.common.data.ApprovalTypes;
-import com.google.gerrit.reviewdb.client.ApprovalCategory;
-import com.google.gerrit.reviewdb.client.ApprovalCategoryValue;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.ChangeMessage;
-import com.google.gerrit.reviewdb.client.PatchLineComment;
-import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.PatchSetApproval;
-import com.google.gerrit.reviewdb.client.PatchSetInfo;
-import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.ChangeUtil;
-import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.git.WorkQueue;
-import com.google.gerrit.server.mail.CommentSender;
-import com.google.gerrit.server.project.ChangeControl;
-import com.google.gerrit.server.project.InvalidChangeOperationException;
-import com.google.gerrit.server.project.NoSuchChangeException;
-import com.google.gerrit.server.util.RequestScopePropagator;
-import com.google.gerrit.server.workflow.FunctionState;
-import com.google.gwtjsonrpc.common.VoidResult;
-import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.SchemaFactory;
-import com.google.inject.Inject;
-import com.google.inject.assistedinject.Assisted;
-
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.Callable;
-
-public class PublishComments implements Callable<VoidResult> {
-  private static final Logger log =
-      LoggerFactory.getLogger(PublishComments.class);
-
-  public interface Factory {
-    PublishComments create(PatchSet.Id patchSetId, String messageText,
-        Set<ApprovalCategoryValue.Id> approvals, boolean forceMessage);
-  }
-
-  private final SchemaFactory<ReviewDb> schemaFactory;
-  private final ReviewDb db;
-  private final IdentifiedUser user;
-  private final ApprovalTypes types;
-  private final CommentSender.Factory commentSenderFactory;
-  private final PatchSetInfoFactory patchSetInfoFactory;
-  private final ChangeControl.Factory changeControlFactory;
-  private final FunctionState.Factory functionStateFactory;
-  private final ChangeHooks hooks;
-  private final WorkQueue workQueue;
-  private final RequestScopePropagator requestScopePropagator;
-
-  private final PatchSet.Id patchSetId;
-  private final String messageText;
-  private final Set<ApprovalCategoryValue.Id> approvals;
-  private final boolean forceMessage;
-
-  private Change change;
-  private PatchSet patchSet;
-  private ChangeMessage message;
-  private List<PatchLineComment> drafts;
-
-  @Inject
-  PublishComments(final SchemaFactory<ReviewDb> sf, final ReviewDb db,
-      final IdentifiedUser user,
-      final ApprovalTypes approvalTypes,
-      final CommentSender.Factory commentSenderFactory,
-      final PatchSetInfoFactory patchSetInfoFactory,
-      final ChangeControl.Factory changeControlFactory,
-      final FunctionState.Factory functionStateFactory,
-      final ChangeHooks hooks,
-      final WorkQueue workQueue,
-      final RequestScopePropagator requestScopePropagator,
-
-      @Assisted final PatchSet.Id patchSetId,
-      @Assisted final String messageText,
-      @Assisted final Set<ApprovalCategoryValue.Id> approvals,
-      @Assisted final boolean forceMessage) {
-    this.schemaFactory = sf;
-    this.db = db;
-    this.user = user;
-    this.types = approvalTypes;
-    this.patchSetInfoFactory = patchSetInfoFactory;
-    this.commentSenderFactory = commentSenderFactory;
-    this.changeControlFactory = changeControlFactory;
-    this.functionStateFactory = functionStateFactory;
-    this.hooks = hooks;
-    this.workQueue = workQueue;
-    this.requestScopePropagator = requestScopePropagator;
-
-    this.patchSetId = patchSetId;
-    this.messageText = messageText;
-    this.approvals = approvals;
-    this.forceMessage = forceMessage;
-  }
-
-  @Override
-  public VoidResult call() throws NoSuchChangeException,
-      InvalidChangeOperationException, OrmException {
-    final Change.Id changeId = patchSetId.getParentKey();
-    final ChangeControl ctl = changeControlFactory.validateFor(changeId);
-    change = ctl.getChange();
-    patchSet = db.patchSets().get(patchSetId);
-    if (patchSet == null) {
-      throw new NoSuchChangeException(changeId);
-    }
-    drafts = drafts();
-
-    db.changes().beginTransaction(changeId);
-    try {
-      publishDrafts();
-
-      final boolean isCurrent = patchSetId.equals(change.currentPatchSetId());
-      if (isCurrent && change.getStatus().isOpen()) {
-        publishApprovals(ctl);
-      } else if (approvals.isEmpty() || forceMessage) {
-        publishMessageOnly();
-      } else {
-        throw new InvalidChangeOperationException("Change is closed");
-      }
-
-      touchChange();
-      db.commit();
-    } finally {
-      db.rollback();
-    }
-
-    email();
-    fireHook();
-    return VoidResult.INSTANCE;
-  }
-
-  private void publishDrafts() throws OrmException {
-    for (final PatchLineComment c : drafts) {
-      c.setStatus(PatchLineComment.Status.PUBLISHED);
-      c.updated();
-    }
-    db.patchComments().update(drafts);
-  }
-
-  private void publishApprovals(ChangeControl ctl)
-      throws InvalidChangeOperationException, OrmException {
-    ChangeUtil.updated(change);
-
-    final Set<ApprovalCategory.Id> dirty = new HashSet<ApprovalCategory.Id>();
-    final List<PatchSetApproval> ins = new ArrayList<PatchSetApproval>();
-    final List<PatchSetApproval> upd = new ArrayList<PatchSetApproval>();
-    final Collection<PatchSetApproval> all =
-        db.patchSetApprovals().byPatchSet(patchSetId).toList();
-    final Map<ApprovalCategory.Id, PatchSetApproval> mine = mine(all);
-
-    // Ensure any new approvals are stored properly.
-    //
-    for (final ApprovalCategoryValue.Id want : approvals) {
-      PatchSetApproval a = mine.get(want.getParentKey());
-      if (a == null) {
-        a = new PatchSetApproval(new PatchSetApproval.Key(//
-            patchSetId, user.getAccountId(), want.getParentKey()), want.get());
-        a.cache(change);
-        ins.add(a);
-        all.add(a);
-        mine.put(a.getCategoryId(), a);
-        dirty.add(a.getCategoryId());
-      }
-    }
-
-    // Normalize all of the items the user is changing.
-    //
-    final FunctionState functionState =
-        functionStateFactory.create(ctl, patchSetId, all);
-    for (final ApprovalCategoryValue.Id want : approvals) {
-      final PatchSetApproval a = mine.get(want.getParentKey());
-      final short o = a.getValue();
-      a.setValue(want.get());
-      a.cache(change);
-      if (!ApprovalCategory.SUBMIT.equals(a.getCategoryId())) {
-        functionState.normalize(types.byId(a.getCategoryId()), a);
-      }
-      if (want.get() != a.getValue()) {
-        throw new InvalidChangeOperationException(
-            types.byId(a.getCategoryId()).getCategory().getLabelName()
-            + "=" + want.get() + " not permitted");
-      }
-      if (o != a.getValue()) {
-        // Value changed, ensure we update the database.
-        //
-        a.setGranted();
-        dirty.add(a.getCategoryId());
-      }
-      if (!ins.contains(a)) {
-        upd.add(a);
-      }
-    }
-
-    // Format a message explaining the actions taken.
-    //
-    final StringBuilder msgbuf = new StringBuilder();
-    for (final ApprovalType at : types.getApprovalTypes()) {
-      if (dirty.contains(at.getCategory().getId())) {
-        final PatchSetApproval a = mine.get(at.getCategory().getId());
-        if (a.getValue() == 0 && ins.contains(a)) {
-          // Don't say "no score" for an initial entry.
-          continue;
-        }
-
-        final ApprovalCategoryValue val = at.getValue(a);
-        if (msgbuf.length() > 0) {
-          msgbuf.append("; ");
-        }
-        if (val != null && val.getName() != null && !val.getName().isEmpty()) {
-          msgbuf.append(val.getName());
-        } else {
-          msgbuf.append(at.getCategory().getName());
-          msgbuf.append(" ");
-          if (a.getValue() > 0) msgbuf.append('+');
-          msgbuf.append(a.getValue());
-        }
-      }
-    }
-
-    // Update dashboards for everyone else.
-    //
-    for (PatchSetApproval a : all) {
-      if (!user.getAccountId().equals(a.getAccountId())) {
-        a.cache(change);
-        upd.add(a);
-      }
-    }
-
-    db.patchSetApprovals().update(upd);
-    db.patchSetApprovals().insert(ins);
-
-    summarizeInlineComments(msgbuf);
-    message(msgbuf.toString());
-  }
-
-  private void publishMessageOnly() throws OrmException {
-    StringBuilder msgbuf = new StringBuilder();
-    summarizeInlineComments(msgbuf);
-    message(msgbuf.toString());
-  }
-
-  private void message(String actions) throws OrmException {
-    if ((actions == null || actions.isEmpty())
-        && (messageText == null || messageText.isEmpty())) {
-      // They had nothing to say?
-      //
-      return;
-    }
-
-    final StringBuilder msgbuf = new StringBuilder();
-    msgbuf.append("Patch Set " + patchSetId.get() + ":");
-    if (actions != null && !actions.isEmpty()) {
-      msgbuf.append(" ");
-      msgbuf.append(actions);
-    }
-    msgbuf.append("\n\n");
-    msgbuf.append(messageText != null ? messageText : "");
-
-    message = new ChangeMessage(new ChangeMessage.Key(change.getId(),//
-        ChangeUtil.messageUUID(db)), user.getAccountId(), patchSetId);
-    message.setMessage(msgbuf.toString());
-    db.changeMessages().insert(Collections.singleton(message));
-  }
-
-  private Map<ApprovalCategory.Id, PatchSetApproval> mine(
-      Collection<PatchSetApproval> all) {
-    Map<ApprovalCategory.Id, PatchSetApproval> r =
-        new HashMap<ApprovalCategory.Id, PatchSetApproval>();
-    for (PatchSetApproval a : all) {
-      if (user.getAccountId().equals(a.getAccountId())) {
-        r.put(a.getCategoryId(), a);
-      }
-    }
-    return r;
-  }
-
-  private void touchChange() {
-    try {
-      ChangeUtil.touch(change, db);
-    } catch (OrmException e) {
-    }
-  }
-
-  private List<PatchLineComment> drafts() throws OrmException {
-    return db.patchComments().draftByPatchSetAuthor(patchSetId, user.getAccountId()).toList();
-  }
-
-  private void email() {
-    if (message == null) {
-      return;
-    }
-
-    workQueue.getDefaultQueue()
-        .submit(requestScopePropagator.wrap(new Runnable() {
-      @Override
-      public void run() {
-        PatchSetInfo patchSetInfo;
-        try {
-          ReviewDb reviewDb = schemaFactory.open();
-          try {
-            patchSetInfo = patchSetInfoFactory.get(reviewDb, patchSetId);
-          } finally {
-            reviewDb.close();
-          }
-        } catch (PatchSetInfoNotAvailableException e) {
-          log.error("Cannot read PatchSetInfo of " + patchSetId, e);
-          return;
-        } catch (Exception e) {
-          log.error("Cannot email comments for " + patchSetId, e);
-          return;
-        }
-
-        try {
-          final CommentSender cm = commentSenderFactory.create(change);
-          cm.setFrom(user.getAccountId());
-          cm.setPatchSet(patchSet, patchSetInfo);
-          cm.setChangeMessage(message);
-          cm.setPatchLineComments(drafts);
-          cm.send();
-        } catch (Exception e) {
-          log.error("Cannot email comments for " + patchSetId, e);
-        }
-      }
-
-      @Override
-      public String toString() {
-        return "send-email comments";
-      }
-    }));
-  }
-
-  private void fireHook() throws OrmException {
-    final Map<ApprovalCategory.Id, ApprovalCategoryValue.Id> changed =
-        new HashMap<ApprovalCategory.Id, ApprovalCategoryValue.Id>();
-    for (ApprovalCategoryValue.Id v : approvals) {
-      changed.put(v.getParentKey(), v);
-    }
-
-    hooks.doCommentAddedHook(change, user.getAccount(), patchSet, messageText, changed, db);
-  }
-
-  private void summarizeInlineComments(StringBuilder in) {
-    if (!drafts.isEmpty()) {
-      if (in.length() != 0) {
-        in.append("\n\n");
-      }
-      if (drafts.size() == 1) {
-        in.append("(1 inline comment)");
-      } else {
-        in.append("(" + drafts.size() + " inline comments)");
-      }
-    }
-  }
-}

gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ApproveOption.java

@@ -15,7 +15,6 @@
 package com.google.gerrit.sshd.commands;
 
 import com.google.gerrit.common.data.ApprovalType;
-import com.google.gerrit.reviewdb.client.ApprovalCategory;
 import com.google.gerrit.reviewdb.client.ApprovalCategoryValue;
 
 import org.kohsuke.args4j.CmdLineException;
@@ -100,8 +99,8 @@ final class ApproveOption implements Option, Setter<Short> {
     return false;
   }
 
-  ApprovalCategory.Id getCategoryId() {
+  String getLabelName() {
-    return type.getCategory().getId();
+    return type.getCategory().getLabelName();
   }
 
   public static class Handler extends OneArgumentOptionHandler<Short> {

gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java

@@ -14,11 +14,14 @@
 
 package com.google.gerrit.sshd.commands;
 
+import com.google.common.base.Strings;
+import com.google.common.collect.Maps;
 import com.google.gerrit.common.data.ApprovalType;
 import com.google.gerrit.common.data.ApprovalTypes;
 import com.google.gerrit.common.data.ReviewResult;
 import com.google.gerrit.common.data.ReviewResult.Error.Type;
 import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.reviewdb.client.ApprovalCategory;
 import com.google.gerrit.reviewdb.client.ApprovalCategoryValue;
@@ -28,11 +31,12 @@ import com.google.gerrit.reviewdb.client.RevId;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.change.Abandon;
 import com.google.gerrit.server.change.ChangeResource;
+import com.google.gerrit.server.change.PostReview;
+import com.google.gerrit.server.change.RevisionResource;
 import com.google.gerrit.server.changedetail.DeleteDraftPatchSet;
 import com.google.gerrit.server.changedetail.PublishDraft;
 import com.google.gerrit.server.changedetail.RestoreChange;
 import com.google.gerrit.server.changedetail.Submit;
-import com.google.gerrit.server.patch.PublishComments;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.InvalidChangeOperationException;
 import com.google.gerrit.server.project.NoSuchChangeException;
@@ -124,7 +128,7 @@ public class ReviewCommand extends SshCommand {
   private Provider<Abandon> abandonProvider;
 
   @Inject
-  private PublishComments.Factory publishCommentsFactory;
+  private Provider<PostReview> reviewProvider;
 
   @Inject
   private PublishDraft.Factory publishDraftFactory;
@@ -195,23 +199,29 @@ public class ReviewCommand extends SshCommand {
       changeComment = "";
     }
 
-    Set<ApprovalCategoryValue.Id> aps = new HashSet<ApprovalCategoryValue.Id>();
+    PostReview.Input review = new PostReview.Input();
+    review.message = Strings.emptyToNull(changeComment);
+    review.labels = Maps.newTreeMap();
+    review.drafts = PostReview.DraftHandling.PUBLISH;
+    review.strictLabels = false;
     for (ApproveOption ao : optionList) {
       Short v = ao.value();
       if (v != null) {
-        aps.add(new ApprovalCategoryValue.Id(ao.getCategoryId(), v));
+        review.labels.put(ao.getLabelName(), v);
       }
     }
 
     try {
-      publishCommentsFactory.create(patchSetId, changeComment, aps, forceMessage).call();
+      ChangeControl ctl =
+          changeControlFactory.controlFor(patchSetId.getParentKey());
+      reviewProvider.get().apply(new RevisionResource(
+          new ChangeResource(ctl),
+          db.patchSets().get(patchSetId)), review);
 
       if (abandonChange) {
         final Abandon abandon = abandonProvider.get();
         final Abandon.Input input = new Abandon.Input();
         input.message = changeComment;
-        ChangeControl ctl =
-            changeControlFactory.controlFor(patchSetId.getParentKey());
         try {
           abandon.apply(new ChangeResource(ctl), input);
         } catch(AuthException e) {
@@ -234,6 +244,10 @@ public class ReviewCommand extends SshCommand {
       throw error(e.getMessage());
     } catch (IllegalStateException e) {
       throw error(e.getMessage());
+    } catch (AuthException e) {
+      throw error(e.getMessage());
+    } catch (BadRequestException e) {
+      throw error(e.getMessage());
     }
 
     if (publishPatchSet) {