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
This commit is contained in:
Stefan Beller 2015-07-30 11:18:22 -07:00
parent 55adf1e640
commit bd65aac4ce
5 changed files with 22 additions and 10 deletions

View File

@ -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;

View File

@ -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<RevisionInfo> revisions() /*-{ return this.revisions; }-*/;
public final native RevisionInfo revision(String n) /*-{ return this.revisions[n]; }-*/;
public final native JsArray<MessageInfo> 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;
}

View File

@ -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;

View File

@ -248,6 +248,22 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
return null;
}
/**
* Check if there are any problems with the given change. It doesn't take
* any problems of related changes into account.
* <p>
* @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();

View File

@ -64,9 +64,7 @@ public class SubmittedTogether implements RestReadView<ChangeResource> {
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();