From bd65aac4ce653ebb4fa3ed18809d20f89ce0de79 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Thu, 30 Jul 2015 11:18:22 -0700 Subject: [PATCH] Determine submittability on the server side We do not transport all labels to the client side as certain labels may be hidden from some users. To display the correct state for submittability for each change, we should rely on the server stating if it deems a change to be submittable as that is the same code as in the Merging procedure, so there are no differences. Change-Id: I44d39a6e777147371936567b868ba517780271a1 --- .../gerrit/extensions/common/ChangeInfo.java | 1 + .../google/gerrit/client/changes/ChangeInfo.java | 7 ------- .../google/gerrit/server/change/ChangeJson.java | 4 ++++ .../com/google/gerrit/server/change/Submit.java | 16 ++++++++++++++++ .../gerrit/server/change/SubmittedTogether.java | 4 +--- 5 files changed, 22 insertions(+), 10 deletions(-) diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeInfo.java index 13baf6b215..cdfe0c6372 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeInfo.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeInfo.java @@ -35,6 +35,7 @@ public class ChangeInfo { public Boolean starred; public Boolean reviewed; public Boolean mergeable; + public Boolean submittable; public Integer insertions; public Integer deletions; diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java index 9a5a4f2f7d..5b7e304cf6 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java @@ -107,7 +107,6 @@ public class ChangeInfo extends JavaScriptObject { public final native LabelInfo label(String n) /*-{ return this.labels[n]; }-*/; public final native String currentRevision() /*-{ return this.current_revision; }-*/; public final native void setCurrentRevision(String r) /*-{ this.current_revision = r; }-*/; - private final native void setSubmittable(boolean x) /*-{ this.submittable = x; }-*/; public final native NativeMap revisions() /*-{ return this.revisions; }-*/; public final native RevisionInfo revision(String n) /*-{ return this.revisions[n]; }-*/; public final native JsArray messages() /*-{ return this.messages; }-*/; @@ -135,7 +134,6 @@ public class ChangeInfo extends JavaScriptObject { public final boolean submittable() { init(); - getMissingLabelIndex(); return _submittable(); } @@ -143,8 +141,6 @@ public class ChangeInfo extends JavaScriptObject { /*-{ return this.submittable ? true : false; }-*/; /** - * As a side effect this.submittable is evaluated and set accordingly. - * * @return the index of the missing label or -1 * if no label is missing, or if more than one label is missing. */ @@ -168,7 +164,6 @@ public class ChangeInfo extends JavaScriptObject { if (ret != -1) { // more than one label is missing, so it's unclear which to quick // approve, return -1 - setSubmittable(false); return -1; } else { ret = i; @@ -181,11 +176,9 @@ public class ChangeInfo extends JavaScriptObject { case REJECT: // Submit cannot happen, do not quick approve. case IMPOSSIBLE: - setSubmittable(false); return -1; } } - setSubmittable(ret == -1); return ret; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java index dc7f1b32d4..acdf004b94 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java @@ -137,6 +137,7 @@ public class ChangeJson { private final GitRepositoryManager repoManager; private final ProjectCache projectCache; private final MergeUtil.Factory mergeUtilFactory; + private final Submit submit; private final IdentifiedUser.GenericFactory userFactory; private final ChangeData.Factory changeDataFactory; private final FileInfoJson fileInfoJson; @@ -161,6 +162,7 @@ public class ChangeJson { GitRepositoryManager repoManager, ProjectCache projectCache, MergeUtil.Factory mergeUtilFactory, + Submit submit, IdentifiedUser.GenericFactory uf, ChangeData.Factory cdf, FileInfoJson fileInfoJson, @@ -180,6 +182,7 @@ public class ChangeJson { this.repoManager = repoManager; this.userFactory = uf; this.projectCache = projectCache; + this.submit = submit; this.mergeUtilFactory = mergeUtilFactory; this.fileInfoJson = fileInfoJson; this.accountLoaderFactory = ailf; @@ -383,6 +386,7 @@ public class ChangeJson { // the response and avoid making a request to /submit_type from the UI. out.mergeable = in.getStatus() == Change.Status.MERGED ? null : cd.isMergeable(); + out.submittable = submit.submittable(cd); ChangedLines changedLines = cd.changedLines(); if (changedLines != null) { out.insertions = changedLines.insertions; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java index 2b8e3bcde2..b10907f6d7 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java @@ -248,6 +248,22 @@ public class Submit implements RestModifyView, return null; } + /** + * Check if there are any problems with the given change. It doesn't take + * any problems of related changes into account. + *

+ * @param cd the change to check for submittability + * @return if the change has any problems for submission + */ + public boolean submittable(ChangeData cd) { + try { + MergeOp.checkSubmitRule(cd); + return true; + } catch (ResourceConflictException | OrmException e) { + return false; + } + } + @Override public UiAction.Description getDescription(RevisionResource resource) { PatchSet.Id current = resource.getChange().currentPatchSetId(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/SubmittedTogether.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/SubmittedTogether.java index 71e25a5806..1f6dfe4b2f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/SubmittedTogether.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/SubmittedTogether.java @@ -64,9 +64,7 @@ public class SubmittedTogether implements RestReadView { if (cs.ids().size() > 1) { return json.create(EnumSet.of( ListChangesOption.CURRENT_REVISION, - ListChangesOption.CURRENT_COMMIT, - ListChangesOption.DETAILED_LABELS, - ListChangesOption.LABELS)) + ListChangesOption.CURRENT_COMMIT)) .format(cs.ids()); } else { return Collections.emptyList();