opendev/gerrit
Code Issues Proposed changes

PolyGerrit: Correctly detect the ACL for SetReadyForReview endpoint

Browse Source 
In I55dd6400 the ACL for set ready endpoint was extended to the
administrators group, but PolyGerrit UI was missed to be extended as
well. The PG UI is still checking whether to render Start Review button
for change owners only.

In I4598a6fb97 the ACL was extended even further for the project owners,
so that the check on the client side is fundamentally wrong. Instead,
the availability of the button must be solely derived from actions list
provided by the server.

Bug: Issue 9642
Change-Id: I08c35a610affc0d6e27bec80a3591d3ab9ebb990
This commit is contained in:
David Ostrovsky 2018-08-28 09:02:07 +02:00
parent 50acf00ad7
commit ff87bcb303
2 changed files with 21 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
View File

@ -98,7 +98,7 @@
},
_canStartReview: {
type: Boolean,
computed: '_computeCanStartReview(_loggedIn, _change, _account)',
computed: '_computeCanStartReview(_change)',
},
_comments: Object,
/** @type {?} */
@ -1109,9 +1109,9 @@
]);
},
_computeCanStartReview(loggedIn, change, account) {
return !!(loggedIn && change.work_in_progress &&
change.owner._account_id === account._account_id);
_computeCanStartReview(change) {
return !!(change.actions && change.actions.ready &&
change.actions.ready.enabled);
},
_computeReplyDisabled() { return false; },

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

@ -1087,18 +1087,24 @@ limitations under the License.
});
test('canStartReview computation', () => {
const account1 = {_account_id: 1};
const account2 = {_account_id: 2};
const change = {
owner: {_account_id: 1},
const change1 = {};
const change2 = {
actions: {
ready: {
enabled: true,
},
},
};
assert.isFalse(element._computeCanStartReview(true, change, account1));
change.work_in_progress = false;
assert.isFalse(element._computeCanStartReview(true, change, account1));
change.work_in_progress = true;
assert.isTrue(element._computeCanStartReview(true, change, account1));
assert.isFalse(element._computeCanStartReview(false, change, account1));
assert.isFalse(element._computeCanStartReview(true, change, account2));
const change3 = {
actions: {
ready: {
label: 'Ready for Review',
},
},
};
assert.isFalse(element._computeCanStartReview(change1));
assert.isTrue(element._computeCanStartReview(change2));
assert.isFalse(element._computeCanStartReview(change3));
});
test('header class computation', () => {