diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeDetailService.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeDetailService.java index 2d7a28bbe1..58244159f5 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeDetailService.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeDetailService.java @@ -15,7 +15,6 @@ package com.google.gerrit.common.data; import com.google.gerrit.common.audit.Audit; -import com.google.gerrit.common.auth.SignInRequired; import com.google.gerrit.reviewdb.client.AccountDiffPreference; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gwtjsonrpc.common.AsyncCallback; @@ -31,8 +30,4 @@ public interface ChangeDetailService extends RemoteJsonService { @Audit void patchSetDetail2(PatchSet.Id baseId, PatchSet.Id key, AccountDiffPreference diffPrefs, AsyncCallback callback); - - @SignInRequired - void patchSetPublishDetail(PatchSet.Id key, - AsyncCallback callback); } 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 75bc5cafd3..96073cc525 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 @@ -23,6 +23,8 @@ import com.google.gerrit.client.patches.CommentEditorPanel; import com.google.gerrit.client.projects.ConfigInfoCache; import com.google.gerrit.client.rpc.CallbackGroup; import com.google.gerrit.client.rpc.GerritCallback; +import com.google.gerrit.client.rpc.NativeMap; +import com.google.gerrit.client.rpc.NativeString; import com.google.gerrit.client.rpc.Natives; import com.google.gerrit.client.rpc.RestApi; import com.google.gerrit.client.rpc.ScreenLoadCallback; @@ -31,12 +33,16 @@ import com.google.gerrit.client.ui.CommentLinkProcessor; import com.google.gerrit.client.ui.PatchLink; import com.google.gerrit.client.ui.SmallHeading; import com.google.gerrit.common.PageLinks; +import com.google.gerrit.common.changes.ListChangesOption; +import com.google.gerrit.common.changes.Side; import com.google.gerrit.common.data.ChangeDetail; -import com.google.gerrit.common.data.PatchSetPublishDetail; +import com.google.gerrit.common.data.SubmitTypeRecord; import com.google.gerrit.reviewdb.client.Change; 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.Project; +import com.google.gwt.core.client.JsArray; import com.google.gwt.core.client.JsArrayString; import com.google.gwt.event.dom.client.ClickEvent; import com.google.gwt.event.dom.client.ClickHandler; @@ -57,6 +63,7 @@ import com.google.gwtjsonrpc.common.VoidResult; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.EnumSet; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -66,7 +73,6 @@ 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; @@ -79,6 +85,9 @@ public class PublishCommentScreen extends AccountScreen implements private boolean saveStateOnUnload = true; private List commentEditors; private ChangeInfo change; + private ChangeInfo detail; + private NativeMap> drafts; + private SubmitTypeRecord submitTypeRecord; private CommentLinkProcessor commentLinkProcessor; public PublishCommentScreen(final PatchSet.Id psi) { @@ -149,50 +158,63 @@ public class PublishCommentScreen extends AccountScreen implements protected void onLoad() { super.onLoad(); - CallbackGroup cbs = new CallbackGroup(); + CallbackGroup group = new CallbackGroup(); + RestApi call = ChangeApi.detail(patchSetId.getParentKey().get()); + ChangeList.addOptions(call, EnumSet.of( + ListChangesOption.CURRENT_ACTIONS, + ListChangesOption.ALL_REVISIONS, + ListChangesOption.ALL_COMMITS)); + call.get(group.add(new GerritCallback() { + @Override + public void onSuccess(ChangeInfo result) { + detail = result; + } + })); + ChangeApi.revision(patchSetId) + .view("submit_type") + .get(group.add(new GerritCallback() { + @Override + public void onSuccess(NativeString result) { + submitTypeRecord = SubmitTypeRecord.OK( + Project.SubmitType.valueOf(result.asString())); + } + public void onFailure(Throwable caught) {} + })); + ChangeApi.revision(patchSetId.getParentKey().get(), "" + patchSetId.get()) + .view("drafts") + .get(group.add(new AsyncCallback>>() { + @Override + public void onSuccess(NativeMap> result) { + drafts = result; + } + public void onFailure(Throwable caught) {} + })); ChangeApi.revision(patchSetId).view("review") - .get(cbs.add(new AsyncCallback() { - @Override - public void onSuccess(ChangeInfo result) { - result.init(); - change = result; - } + .get(group.addFinal(new GerritCallback() { + @Override + public void onSuccess(ChangeInfo result) { + result.init(); + change = result; + preDisplay(result); + } + })); + } + private void preDisplay(final ChangeInfo info) { + ConfigInfoCache.get(info.project_name_key(), + new ScreenLoadCallback(this) { @Override - public void onFailure(Throwable caught) { - // Handled by ScreenLoadCallback.onFailure(). - } - })); - Util.DETAIL_SVC.patchSetPublishDetail(patchSetId, cbs.addFinal( - new ScreenLoadCallback(this) { - @Override - protected void preDisplay(final PatchSetPublishDetail result) { + protected void preDisplay(ConfigInfoCache.Entry result) { send.setEnabled(true); - PublishCommentScreen.this.preDisplay(result, this); + commentLinkProcessor = result.getCommentLinkProcessor(); + setTheme(result.getTheme()); + displayScreen(); } @Override protected void postDisplay() { message.setFocus(true); } - })); - } - - private void preDisplay(final PatchSetPublishDetail pubDetail, - final ScreenLoadCallback origCb) { - ConfigInfoCache.get(pubDetail.getChange().getProject(), - new AsyncCallback() { - @Override - public void onSuccess(ConfigInfoCache.Entry result) { - commentLinkProcessor = result.getCommentLinkProcessor(); - setTheme(result.getTheme()); - display(pubDetail); - } - - @Override - public void onFailure(Throwable caught) { - origCb.onFailure(caught); - } }); } @@ -317,14 +339,13 @@ public class PublishCommentScreen extends AccountScreen implements body.add(vp); } - private void display(final PatchSetPublishDetail r) { - ChangeDetail changeDetail = new ChangeDetail(); - changeDetail.setChange(r.getChange()); + private void displayScreen() { + ChangeDetail r = ChangeDetailCache.reverse(detail); setPageTitle(Util.M.publishComments(r.getChange().getKey().abbreviate(), patchSetId.get())); - descBlock.display(changeDetail, null, false, r.getPatchSetInfo(), r.getAccounts(), - r.getSubmitTypeRecord(), commentLinkProcessor); + descBlock.display(r, null, false, r.getCurrentPatchSetDetail().getInfo(), + r.getAccounts(), submitTypeRecord, commentLinkProcessor); if (r.getChange().getStatus().isOpen()) { initApprovals(approvalPanel); @@ -339,14 +360,13 @@ public class PublishCommentScreen extends AccountScreen implements draftsPanel.clear(); commentEditors = new ArrayList(); - revision = r.getPatchSetInfo().getRevId(); - if (!r.getDrafts().isEmpty()) { + if (!drafts.isEmpty()) { draftsPanel.add(new SmallHeading(Util.C.headingPatchComments())); Panel panel = null; String priorFile = ""; - for (final PatchLineComment c : r.getDrafts()) { + for (final PatchLineComment c : draftList()) { final Patch.Key patchKey = c.getKey().getParentKey(); final String fn = patchKey.get(); if (!fn.equals(priorFile)) { @@ -375,7 +395,7 @@ public class PublishCommentScreen extends AccountScreen implements } } - submit.setVisible(r.canSubmit()); + submit.setVisible(true/* TODO canSubmit? */); } private void onSend(final boolean submit) { @@ -411,7 +431,7 @@ public class PublishCommentScreen extends AccountScreen implements enableForm(false); new RestApi("/changes/") .id(String.valueOf(patchSetId.getParentKey().get())) - .view("revisions").id(revision).view("review") + .view("revisions").id(patchSetId.get()).view("review") .post(data, new GerritCallback() { @Override public void onSuccess(ReviewInput result) { @@ -432,7 +452,9 @@ public class PublishCommentScreen extends AccountScreen implements } private void submit() { - ChangeApi.submit(patchSetId.getParentKey().get(), revision, + ChangeApi.submit( + patchSetId.getParentKey().get(), + "" + patchSetId.get(), new GerritCallback() { public void onSuccess(SubmitInfo result) { saveStateOnUnload = false; @@ -456,6 +478,33 @@ public class PublishCommentScreen extends AccountScreen implements Gerrit.display(PageLinks.toChange(ck), new ChangeScreen(ck)); } + private List draftList() { + List d = new ArrayList(); + List paths = new ArrayList(drafts.keySet()); + Collections.sort(paths); + for (String path : paths) { + JsArray comments = drafts.get(path); + for (int i = 0; i < comments.length(); i++) { + d.add(toComment(path, comments.get(i))); + } + } + return d; + } + + private PatchLineComment toComment(String path, CommentInfo i) { + PatchLineComment p = new PatchLineComment( + new PatchLineComment.Key( + new Patch.Key(patchSetId, path), + i.id()), + i.line(), + Gerrit.getUserAccount().getId(), + i.in_reply_to(), + i.updated()); + p.setMessage(i.message()); + p.setSide((short) (i.side() == Side.PARENT ? 0 : 1)); + return p; + } + private static class ValueRadioButton extends RadioButton { final LabelInfo label; final String value; diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailServiceImpl.java index 1e73f09c52..538275fb06 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailServiceImpl.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailServiceImpl.java @@ -16,7 +16,6 @@ package com.google.gerrit.httpd.rpc.changedetail; import com.google.gerrit.common.data.ChangeDetailService; import com.google.gerrit.common.data.PatchSetDetail; -import com.google.gerrit.common.data.PatchSetPublishDetail; import com.google.gerrit.reviewdb.client.AccountDiffPreference; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gwtjsonrpc.common.AsyncCallback; @@ -24,14 +23,11 @@ import com.google.inject.Inject; class ChangeDetailServiceImpl implements ChangeDetailService { private final PatchSetDetailFactory.Factory patchSetDetail; - private final PatchSetPublishDetailFactory.Factory patchSetPublishDetail; @Inject ChangeDetailServiceImpl( - final PatchSetDetailFactory.Factory patchSetDetail, - final PatchSetPublishDetailFactory.Factory patchSetPublishDetail) { + final PatchSetDetailFactory.Factory patchSetDetail) { this.patchSetDetail = patchSetDetail; - this.patchSetPublishDetail = patchSetPublishDetail; } public void patchSetDetail(PatchSet.Id id, @@ -43,9 +39,4 @@ class ChangeDetailServiceImpl implements ChangeDetailService { AccountDiffPreference diffPrefs, AsyncCallback callback) { patchSetDetail.create(baseId, id, diffPrefs).to(callback); } - - public void patchSetPublishDetail(final PatchSet.Id id, - final AsyncCallback callback) { - patchSetPublishDetail.create(id).to(callback); - } } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeModule.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeModule.java index d7b2c6813c..840137a19f 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeModule.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeModule.java @@ -29,7 +29,6 @@ public class ChangeModule extends RpcServletModule { @Override protected void configure() { factory(PatchSetDetailFactory.Factory.class); - factory(PatchSetPublishDetailFactory.Factory.class); } }); rpc(ChangeDetailServiceImpl.class); diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java deleted file mode 100644 index d0101a688e..0000000000 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java +++ /dev/null @@ -1,164 +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.httpd.rpc.changedetail; - -import com.google.gerrit.common.data.LabelType; -import com.google.gerrit.common.data.PatchSetPublishDetail; -import com.google.gerrit.common.data.PermissionRange; -import com.google.gerrit.common.data.SubmitRecord; -import com.google.gerrit.httpd.rpc.Handler; -import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.reviewdb.client.PatchLineComment; -import com.google.gerrit.reviewdb.client.PatchSet; -import com.google.gerrit.reviewdb.client.PatchSetInfo; -import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gerrit.server.IdentifiedUser; -import com.google.gerrit.server.account.AccountInfoCacheFactory; -import com.google.gerrit.server.patch.PatchSetInfoFactory; -import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; -import com.google.gerrit.server.project.ChangeControl; -import com.google.gerrit.server.project.NoSuchChangeException; -import com.google.gwtorm.server.OrmException; -import com.google.inject.Inject; -import com.google.inject.assistedinject.Assisted; - -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -final class PatchSetPublishDetailFactory extends Handler { - interface Factory { - PatchSetPublishDetailFactory create(PatchSet.Id patchSetId); - } - - private final PatchSetInfoFactory infoFactory; - private final ReviewDb db; - private final ChangeControl.Factory changeControlFactory; - private final AccountInfoCacheFactory aic; - private final IdentifiedUser user; - - private final PatchSet.Id patchSetId; - - private PatchSetInfo patchSetInfo; - private Change change; - private List drafts; - - @Inject - PatchSetPublishDetailFactory(final PatchSetInfoFactory infoFactory, - final ReviewDb db, - final AccountInfoCacheFactory.Factory accountInfoCacheFactory, - final ChangeControl.Factory changeControlFactory, - final IdentifiedUser user, @Assisted final PatchSet.Id patchSetId) { - this.infoFactory = infoFactory; - this.db = db; - this.changeControlFactory = changeControlFactory; - this.aic = accountInfoCacheFactory.create(); - this.user = user; - - this.patchSetId = patchSetId; - } - - @Override - public PatchSetPublishDetail call() throws OrmException, - PatchSetInfoNotAvailableException, NoSuchChangeException { - final Change.Id changeId = patchSetId.getParentKey(); - final ChangeControl control = changeControlFactory.validateFor(changeId); - change = control.getChange(); - PatchSet patchSet = db.patchSets().get(patchSetId); - patchSetInfo = infoFactory.get(change, patchSet); - drafts = db.patchComments().draftByPatchSetAuthor(patchSetId, user.getAccountId()).toList(); - - aic.want(change.getOwner()); - - PatchSetPublishDetail detail = new PatchSetPublishDetail(); - detail.setPatchSetInfo(patchSetInfo); - detail.setChange(change); - detail.setDrafts(drafts); - - if (change.getStatus().isOpen() - && patchSetId.equals(change.currentPatchSetId())) { - // TODO Push this selection of labels down into the Prolog interpreter. - // Ideally we discover the labels the user can apply here based on doing - // a findall() over the space of labels they can apply combined against - // the submit rule, thereby skipping any mutually exclusive cases. However - // those are not common, so it might just be reasonable to take this - // simple approach. - - Map rangeByName = - new HashMap(); - for (PermissionRange r : control.getLabelRanges()) { - if (r.isLabel()) { - rangeByName.put(r.getLabel(), r); - } - } - - boolean couldSubmit = false; - List submitRecords = control.canSubmit(db, patchSet); - for (SubmitRecord rec : submitRecords) { - if (rec.status == SubmitRecord.Status.OK) { - couldSubmit = true; - } - - if (rec.labels != null) { - int ok = 0; - - for (SubmitRecord.Label lbl : rec.labels) { - aic.want(lbl.appliedBy); - - boolean canMakeOk = false; - PermissionRange range = rangeByName.get(lbl.label); - if (range != null) { - LabelType lt = control.getLabelTypes().byLabel(lbl.label); - if (lt != null && lt.getMax().getValue() == range.getMax()) { - canMakeOk = true; - } - } - - switch (lbl.status) { - case OK: - case MAY: - ok++; - break; - - case NEED: - if (canMakeOk) { - ok++; - } - break; - - case IMPOSSIBLE: - case REJECT: - break; - } - } - - if (rec.status == SubmitRecord.Status.NOT_READY - && ok == rec.labels.size()) { - couldSubmit = true; - } - } - } - - if (couldSubmit && control.getRefControl().canSubmit()) { - detail.setCanSubmit(true); - } - } - - detail.setSubmitTypeRecord(control.getSubmitTypeRecord(db, patchSet)); - detail.setAccounts(aic.create()); - - return detail; - } -}