Provide `mergeable` to SHOW_CHANGE handlers

When the SKIP_MERGEABLE option was enabled for change detail requests,
the components in PG that depend on mergeability were refactored to get
mergeability info from properties besides the change detail. However,
some plugins depended on mergeability and used the detail response to
get it.

With this change, SHOW_CHANGE event details provide a new third `info`
parameter, which is an object containing a `mergeable` boolean. This
new info parameter becomes the preferred way for plugins to get
mergeability information.

The change detail object provided to SHOW_CHANGE handlers is given a
`mergeable` boolean getter for backwards compatibility with existing
plugins. The getter emits a deprecation warning and should be removed
after plugins have migrated.

Some logic in `_handleShowChange` is unrolled from the loop.

Bug: Issue 8231
Change-Id: Id2680abcd8baf62e64b6638be1c478da0490098f
This commit is contained in:
Wyatt Allen 2018-01-25 14:04:26 -08:00
parent e9838e645a
commit 95a4d12570
5 changed files with 59 additions and 22 deletions

View File

@ -167,7 +167,8 @@ Supported events:
* `showchange`: Invoked when a change is made visible. A
link:rest-api-changes.html#change-info[ChangeInfo] and
link:rest-api-changes.html#revision-info[RevisionInfo]
are passed as arguments.
are passed as arguments. PolyGerrit provides a third parameter which
is an object with a `mergeable` boolean.
* `submitchange`: Invoked when the submit button is clicked
on a change. A link:rest-api-changes.html#change-info[ChangeInfo]

View File

@ -554,10 +554,7 @@
patchRange.patchNum = this.computeLatestPatchNum(this._allPatchSets);
}
this._reloadPatchNumDependentResources().then(() => {
this.$.jsAPI.handleEvent(this.$.jsAPI.EventType.SHOW_CHANGE, {
change: this._change,
patchNum: patchRange.patchNum,
});
this._sendShowChangeEvent();
});
return;
}
@ -570,15 +567,20 @@
});
},
_sendShowChangeEvent() {
this.$.jsAPI.handleEvent(this.$.jsAPI.EventType.SHOW_CHANGE, {
change: this._change,
patchNum: this._patchRange.patchNum,
info: {mergeable: this._mergeable},
});
},
_performPostLoadTasks() {
this.$.relatedChanges.reload();
this._maybeShowReplyDialog();
this._maybeShowRevertDialog();
this.$.jsAPI.handleEvent(this.$.jsAPI.EventType.SHOW_CHANGE, {
change: this._change,
patchNum: this._patchRange.patchNum,
});
this._sendShowChangeEvent();
this.async(() => {
if (this.viewState.scrollTop) {

View File

@ -1459,6 +1459,22 @@ limitations under the License.
});
});
test('_sendShowChangeEvent', () => {
element._change = {labels: {}};
element._patchRange = {patchNum: 4};
element._mergeable = true;
const showStub = sandbox.stub(element.$.jsAPI, 'handleEvent');
element._sendShowChangeEvent();
assert.isTrue(showStub.calledOnce);
assert.equal(
showStub.lastCall.args[0], element.$.jsAPI.EventType.SHOW_CHANGE);
assert.deepEqual(showStub.lastCall.args[1], {
change: {labels: {}},
patchNum: 4,
info: {mergeable: true},
});
});
suite('_handleEditTap', () => {
let fireEdit;

View File

@ -120,18 +120,34 @@
},
_handleShowChange(detail) {
for (const cb of this._getEventCallbacks(EventType.SHOW_CHANGE)) {
const change = detail.change;
const patchNum = detail.patchNum;
let revision;
for (const rev of Object.values(change.revisions || {})) {
if (this.patchNumEquals(rev._number, patchNum)) {
revision = rev;
break;
}
// Note (issue 8221) Shallow clone the change object and add a mergeable
// getter with deprecation warning. This makes the change detail appear as
// though SKIP_MERGEABLE was not set, so that plugins that expect it can
// still access.
//
// This clone and getter can be removed after plugins migrate to use
// info.mergeable.
const change = Object.assign({
get mergeable() {
console.warn('Accessing change.mergeable from SHOW_CHANGE is ' +
'deprecated! Use info.mergeable instead.');
return detail.info.mergeable;
},
}, detail.change);
const patchNum = detail.patchNum;
const info = detail.info;
let revision;
for (const rev of Object.values(change.revisions || {})) {
if (this.patchNumEquals(rev._number, patchNum)) {
revision = rev;
break;
}
}
for (const cb of this._getEventCallbacks(EventType.SHOW_CHANGE)) {
try {
cb(change, revision);
cb(change, revision, info);
} catch (err) {
console.error(err);
}

View File

@ -179,15 +179,17 @@ limitations under the License.
_number: 42,
revisions: {def: {_number: 2}, abc: {_number: 1}},
};
const expectedChange = Object.assign({mergeable: false}, testChange);
plugin.on(element.EventType.SHOW_CHANGE, throwErrFn);
plugin.on(element.EventType.SHOW_CHANGE, (change, revision) => {
assert.deepEqual(change, testChange);
plugin.on(element.EventType.SHOW_CHANGE, (change, revision, info) => {
assert.deepEqual(change, expectedChange);
assert.deepEqual(revision, testChange.revisions.abc);
assert.deepEqual(info, {mergeable: false});
assert.isTrue(errorStub.calledOnce);
done();
});
element.handleEvent(element.EventType.SHOW_CHANGE,
{change: testChange, patchNum: 1});
{change: testChange, patchNum: 1, info: {mergeable: false}});
});
test('handleEvent awaits plugins load', done => {