diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchDetailService.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchDetailService.java index 91ecb92d34..3c0c584985 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchDetailService.java +++ b/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 callback); - @Audit - @SignInRequired - void publishComments(PatchSet.Id psid, String message, - Set approvals, - AsyncCallback callback); - @Audit @SignInRequired void addReviewers(Change.Id id, List reviewers, boolean confirmed, diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java index 7a02d92ffd..329e653099 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java +++ b/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; } } 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 9c45486c98..8806f5b030 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 @@ -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 approvalButtons; private ChangeDescriptionBlock descBlock; private ApprovalTable approvals; @@ -252,8 +253,7 @@ public class PublishCommentScreen extends AccountScreen implements continue; } - final ValueRadioButton b = - new ValueRadioButton(buttonValue, ct.getCategory().getName()); + ValueRadioButton b = new ValueRadioButton(ct.getCategory(), buttonValue); 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(); + revision = r.getPatchSetInfo().getRevId(); if (!r.getDrafts().isEmpty()) { draftsPanel.add(new SmallHeading(Util.C.headingPatchComments())); @@ -348,20 +349,23 @@ public class PublishCommentScreen extends AccountScreen implements } private void onSend2(final boolean submit) { - final Map values = - new HashMap(); + ReviewInput data = ReviewInput.create(); + 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 HashSet(values.values()), - new GerritCallback() { - public void onSuccess(final VoidResult result) { - if(submit) { + new RestApi("/changes/" + patchSetId.getParentKey().get() + + "/revisions/" + revision + "/review") + .data(data) + .post(new GerritCallback() { + @Override + public void onSuccess(ReviewInput result) { + if (submit) { submit(); } else { saveStateOnUnload = false; @@ -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() { @@ -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) { - super(label); + ValueRadioButton(ApprovalCategory c, ApprovalCategoryValue v) { + super(c.getLabelName()); + category = c; value = v; } } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java index 40c9b843ab..232a79c758 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java +++ b/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 tags, - final AsyncCallback cb) { - Handler.wrap(publishCommentsFactory.create(psid, msg, tags, false)).to(cb); - } - /** * Update the reviewed status for the file by user @code{account} */ 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 5011f31bf3..cc188439ba 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 @@ -51,14 +51,14 @@ import java.util.Iterator; import java.util.List; import java.util.Map; -class PostReview implements RestModifyView { +public class PostReview implements RestModifyView { private static final Logger log = LoggerFactory.getLogger(PostReview.class); - static class Input { + public static class Input { @DefaultInput - String message; + public String message; - Map labels; + public Map labels; Map> comments; /** @@ -68,16 +68,16 @@ class PostReview implements RestModifyView { * 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; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java index a8f3ae2bdd..604f934174 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java +++ b/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); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java deleted file mode 100644 index fdabcaaedb..0000000000 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PublishComments.java +++ /dev/null @@ -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 { - private static final Logger log = - LoggerFactory.getLogger(PublishComments.class); - - public interface Factory { - PublishComments create(PatchSet.Id patchSetId, String messageText, - Set approvals, boolean forceMessage); - } - - private final SchemaFactory 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 approvals; - private final boolean forceMessage; - - private Change change; - private PatchSet patchSet; - private ChangeMessage message; - private List drafts; - - @Inject - PublishComments(final SchemaFactory 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 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 dirty = new HashSet(); - final List ins = new ArrayList(); - final List upd = new ArrayList(); - final Collection all = - db.patchSetApprovals().byPatchSet(patchSetId).toList(); - final Map 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 mine( - Collection all) { - Map r = - new HashMap(); - 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 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 changed = - new HashMap(); - 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)"); - } - } - } -} diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ApproveOption.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ApproveOption.java index a1b8988406..b631553a89 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ApproveOption.java +++ b/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 { return false; } - ApprovalCategory.Id getCategoryId() { - return type.getCategory().getId(); + String getLabelName() { + return type.getCategory().getLabelName(); } public static class Handler extends OneArgumentOptionHandler { diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java index 2bfa14aac9..6e4f03cbbe 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java +++ b/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 abandonProvider; @Inject - private PublishComments.Factory publishCommentsFactory; + private Provider reviewProvider; @Inject private PublishDraft.Factory publishDraftFactory; @@ -195,23 +199,29 @@ public class ReviewCommand extends SshCommand { changeComment = ""; } - Set aps = new HashSet(); + 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) {