Fix replying on changes which have been auto-closed by Gerrit

If the commit of the current patch set of an open change is pushed
directly into the repository Gerrit detects this and closes the
corresponding change automatically. This means the change is set to
status MERGED. In this case it can happen that the change has no
approvals although it is in state MERGED. In this situation replying
to the change fails due to the following error in the web UI:

  SEVERE: Exception caught: (TypeError) : Cannot read property 'values' of undefined
  Class$S471: Exception caught: (TypeError) : Cannot read property 'values' of undefined
    at Unknown.Throwable_2(gerrit_ui-0.js)
    ...
  Caused by: Class$S171: (TypeError) : Cannot read property 'values' of undefined
    at Unknown.$_values(gerrit_ui-0.js)
    at Unknown.$values_0(gerrit_ui-0.js)
    at Unknown.$valueSet(gerrit_ui-0.js)
    at Unknown.renderLabels(gerrit_ui-0.js)
    at Unknown.ReplyBox(gerrit_ui-0.js)
    at Unknown.onReply_2(gerrit_ui-0.js)
    ...

This error occurs because in the ChangeInfo that is returned from the
Get Change Details REST endpoint, the 'permitted_labels' field
contains labels that are missing in the 'labels' field. The problem is
that ChangeJson computes the 'labels' field for closed changes based
on the approvals of the change, but since they are none, 'labels' is
empty.

This problem was introduced by change Ie8a185 (Store SubmitRecords in
index). Before this change 'permitted_labels' were not set for closed
changes. Setting 'permitted_labels' on closed changes was not intended
by change Ie8a185, but it accidentally set allowClosed in the submit
rule options when computing the submit records in
ChangeJson#permittedLabels which decide for which labels the permitted
information is included.

Setting allowClose to false in the submit rule options would fix the
issue, as then 'permitted_labels' would no longer be set on closed
changes. However we can't easily do this, because then we have no
submit records with matching options stored in the index and for
change queries we don't want to lazily load the submit records if they
are missing in the index.

Instead we check the change status and only set 'permitted_labels' if
the change is open.

Setting 'permitted_labels' for closed changes enabled the voting
buttons on closed changes, which is desired for merged changes after
change Ie337b5 added support for voting on merged changes. However it
also enabled the voting buttons (but not voting) on abandoned changes
which we don't want. In a follow-up change we should reenable the
voting buttons on merged changes by setting 'permitted_labels' and
'labels' for merged changes correctly.

Change-Id: I025b6436b18db3be22cb40b185405d06808d4cdf
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2016-10-26 13:34:17 +02:00
parent 492ef018f6
commit af1d440289
2 changed files with 54 additions and 1 deletions

View File

@@ -504,7 +504,9 @@ public class ChangeJson {
// list permitted labels, since users can't vote on those patch sets.
if (!limitToPsId.isPresent()
|| limitToPsId.get().equals(in.currentPatchSetId())) {
out.permittedLabels = permittedLabels(ctl, cd);
out.permittedLabels = cd.change().getStatus().isOpen()
? permittedLabels(ctl, cd)
: ImmutableMap.of();
}
out.removableReviewers = removableReviewers(ctl, out.labels.values());