From a7016e39ae53e81446eb9ad26f2545ca08b3de09 Mon Sep 17 00:00:00 2001 From: Becky Siegel Date: Fri, 18 Aug 2017 16:49:56 -0700 Subject: [PATCH] Show related changes when conflicts do not resolve 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 --- .../gr-related-changes-list.js | 4 +- .../gr-related-changes-list_test.html | 45 +++++++++++++++---- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.js b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.js index 4ef04289d9..f330581682 100644 --- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.js +++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.js @@ -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 : []; })); } diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.html b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.html index 63364336f1..66d6b0268f 100644 --- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.html +++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.html @@ -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', () => {