opendev/gerrit
Code Issues Proposed changes

Don’t allow setting of labels on old patch sets

Browse Source 
There is no check on the backend for this. Ideally there should be.

Bug: Issue 4114
Change-Id: Ie6089929a30f81ec8e252cf00ca36cbd88ad9000
This commit is contained in:
Andrew Bonventre 2016-05-13 13:20:18 -04:00
parent b1921cd670
commit 21ea71953f
4 changed files with 51 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
View File

@ -299,6 +299,7 @@ limitations under the License.
<gr-reply-dialog id="replyDialog"
change-num="[[_changeNum]]"
patch-num="[[_patchRange.patchNum]]"
revisions="[[_change.revisions]]"
labels="[[_change.labels]]"
permitted-labels="[[_change.permitted_labels]]"
diff-drafts="[[_diffDrafts]]"

37
polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
View File

@ -59,6 +59,9 @@ limitations under the License.
border: none;
width: 100%;
}
.labelsNotShown {
color: #666;
}
.labelContainer:not(:first-of-type) {
margin-top: .5em;
}
@ -112,19 +115,27 @@ limitations under the License.
bind-value="{{draft}}"></iron-autogrow-textarea>
</section>
<section class="labelsContainer">
<template is="dom-repeat"
items="[[_computeLabelArray(permittedLabels)]]" as="label">
<div class="labelContainer">
<span class="labelName">[[label]]</span>
<iron-selector data-label$="[[label]]"
selected="[[_computeIndexOfLabelValue(labels, permittedLabels, label, _account)]]">
<template is="dom-repeat"
items="[[_computePermittedLabelValues(permittedLabels, label)]]"
as="value">
<gr-button data-value$="[[value]]">[[value]]</gr-button>
</template>
</iron-selector>
</div>
<template is="dom-if" if="[[_computeShowLabels(patchNum, revisions)]]">
<template is="dom-repeat"
items="[[_computeLabelArray(permittedLabels)]]" as="label">
<div class="labelContainer">
<span class="labelName">[[label]]</span>
<iron-selector data-label$="[[label]]"
selected="[[_computeIndexOfLabelValue(labels, permittedLabels, label, _account)]]">
<template is="dom-repeat"
items="[[_computePermittedLabelValues(permittedLabels, label)]]"
as="value">
<gr-button data-value$="[[value]]">[[value]]</gr-button>
</template>
</iron-selector>
</div>
</template>
</template>
<template is="dom-if" if="[[!_computeShowLabels(patchNum, revisions)]]">
<span class="labelsNotShown">
Labels are not shown because this is not the most recent patch set.
<a href$="/c/[[changeNum]]">Go to the latest patch set.</a>
</span>
</template>
</section>
<section class="draftsContainer" hidden$="[[_computeHideDraftList(diffDrafts)]]">

11
polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
View File

@ -32,6 +32,7 @@
properties: {
changeNum: String,
patchNum: String,
revisions: Object,
disabled: {
type: Boolean,
value: false,
@ -64,6 +65,16 @@
}.bind(this));
},
_computeShowLabels: function(patchNum, revisions) {
var num = parseInt(patchNum, 10);
for (var rev in revisions) {
if (revisions[rev]._number > num) {
return false;
}
}
return true;
},
_computeHideDraftList: function(drafts) {
return Object.keys(drafts || {}).length == 0;
},

15
polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
View File

@ -84,7 +84,21 @@ limitations under the License.
MockInteractions.tap(element.$$('.cancel'));
});
test('show/hide labels', function() {
var revisions = {
rev1: {_number: 1},
rev2: {_number: 2},
};
assert.isFalse(element._computeShowLabels('1', revisions));
assert.isTrue(element._computeShowLabels('2', revisions));
});
test('label picker', function(done) {
var showLabelsStub = sinon.stub(element, '_computeShowLabels',
function() { return true; });
element.revisions = {};
element.patchNum = '';
// Async tick is needed because iron-selector content is distributed and
// distributed content requires an observer to be set up.
flush(function() {
@ -118,6 +132,7 @@ limitations under the License.
'Element should be enabled when done sending reply.');
assert.equal(element.draft.length, 0);
saveReviewStub.restore();
showLabelsStub.restore();
done();
});