opendev/gerrit
Code Issues Proposed changes

Show related changes when conflicts do not resolve

Browse Source 
The server is known to throw 400 errors when it finds too many
conflicts. The client expects the resolved promise to return an array.
Because the template still tries to render the array (even though it is
not visible), having _conflicts be undefined causes rendering to break.

This change adds a check for an undefined API response and sets _changes
to be an empty array in that case.

Bug: Issue 6981
Change-Id: Ifee888a3ced02b0d9383cc9110b062b9cbc57848
This commit is contained in:
Becky Siegel
2017-08-18 16:49:56 -07:00
parent cf830b86b0
commit a7016e39ae
2 changed files with 39 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.js
View File

@@ -102,7 +102,9 @@
// Get conflicts if change is open and is mergeable.
if (this.changeIsOpen(this.change.status) && this.change.mergeable) {
promises.push(this._getConflicts().then(response => {
this._conflicts = response;
// Because the server doesn't always return a response and the
// template expects an array, always return an array.
this._conflicts = response ? response : [];
}));
}

45
polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.html
View File

@@ -231,6 +231,34 @@ limitations under the License.
change1, change2).indexOf('thisChange'), -1);
});
suite('_getConflicts resolves undefined', () => {
let element;
setup(() => {
element = fixture('basic');
sandbox.stub(element, '_getRelatedChanges')
.returns(Promise.resolve({changes: []}));
sandbox.stub(element, '_getSubmittedTogether')
.returns(Promise.resolve());
sandbox.stub(element, '_getCherryPicks')
.returns(Promise.resolve());
sandbox.stub(element, '_getConflicts')
.returns(Promise.resolve());
});
test('_conflicts are an empty array', () => {
element.patchNum = 7;
element.change = {
change_id: 123,
status: 'NEW',
mergeable: true,
};
element.reload();
assert.equal(element._conflicts.length, 0);
});
});
suite('get conflicts tests', () => {
let element;
let conflictsStub;
@@ -238,15 +266,14 @@ limitations under the License.
setup(() => {
element = fixture('basic');
sandbox.stub(element, '_getRelatedChanges', () => {
return Promise.resolve({changes: []});
});
sandbox.stub(element, '_getSubmittedTogether',
() => { return Promise.resolve(); });
sandbox.stub(element, '_getCherryPicks',
() => { return Promise.resolve(); });
conflictsStub = sandbox.stub(element, '_getConflicts',
() => { return Promise.resolve(['test data']); });
sandbox.stub(element, '_getRelatedChanges')
.returns(Promise.resolve({changes: []}));
sandbox.stub(element, '_getSubmittedTogether')
.returns(Promise.resolve());
sandbox.stub(element, '_getCherryPicks')
.returns(Promise.resolve());
conflictsStub = sandbox.stub(element, '_getConflicts')
.returns(Promise.resolve());
});
test('request conflicts if open and mergeable', () => {