Add check before requesting conflicts in related change view
Previously, an API request for getting conflicts was made no matter
what. This request was often very slow, and because the related change
element waits for many promises to resolve before rendering, it was a
bottleneck to display changes. In the GWT interface, that particular
call is only made if a change is open and mergable. With this change,
the same kind of checks have been added to Polygerrit in order to make
the relation chain appear faster in many cases.
Bug: Issue 4727
Change-Id: I9e0b42265a863c86de5eec3328b37e222eba2f97
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 cac45c6..c066b17 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
@@ -60,14 +60,18 @@
this._getSubmittedTogether().then(function(response) {
this._submittedTogether = response;
}.bind(this)),
- this._getConflicts().then(function(response) {
- this._conflicts = response;
- }.bind(this)),
this._getCherryPicks().then(function(response) {
this._cherryPicks = response;
}.bind(this)),
];
+ // Get conflicts if change is open and is mergeable.
+ if (this.changeIsOpen(this.change.status) && this.change.mergeable) {
+ promises.push(this._getConflicts().then(function(response) {
+ this._conflicts = response;
+ }.bind(this)));
+ }
+
promises.push(this._getServerConfig().then(function(config) {
if (this.change.topic && !config.change.submit_whole_topic) {
return this._getChangesWithSameTopic().then(function(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 f7864ce..21903d2 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
@@ -33,9 +33,15 @@
<script>
suite('gr-related-changes-list tests', function() {
var element;
+ var sandbox;
setup(function() {
element = fixture('basic');
+ sandbox = sinon.sandbox.create();
+ });
+
+ teardown(function() {
+ sandbox.restore();
});
test('connected revisions', function() {
@@ -223,5 +229,64 @@
assert.equal(element._computeChangeContainerClass(
change1, change2).indexOf('thisChange'), -1);
});
+
+ suite('get conflicts tests', function() {
+ var element;
+ var conflictsStub;
+
+ setup(function() {
+ element = fixture('basic');
+
+ sandbox.stub(element, '_getRelatedChanges',
+ function() { return Promise.resolve(); });
+ sandbox.stub(element, '_getSubmittedTogether',
+ function() { return Promise.resolve(); });
+ sandbox.stub(element, '_getCherryPicks',
+ function() { return Promise.resolve(); });
+ conflictsStub = sandbox.stub(element, '_getConflicts',
+ function() { return Promise.resolve(); });
+ });
+
+ test('request conflicts if open and mergeable', function() {
+ element.patchNum = 7;
+ element.change = {
+ status: 'NEW',
+ mergeable: true,
+ };
+ element.reload();
+ assert.isTrue(conflictsStub.called);
+ });
+
+ test('does not request conflicts if closed and mergeable', function() {
+ element.patchNum = 7;
+ element.change = {
+ status: 'MERGED',
+ mergeable: true,
+ };
+ element.reload();
+ assert.isFalse(conflictsStub.called);
+ });
+
+ test('does not request conflicts if open and not mergeable', function() {
+ element.patchNum = 7;
+ element.change = {
+ status: 'NEW',
+ mergeable: false,
+ };
+ element.reload();
+ assert.isFalse(conflictsStub.called);
+ });
+
+ test('does not request conflicts if closed and not mergeable',
+ function() {
+ element.patchNum = 7;
+ element.change = {
+ status: 'MERGED',
+ mergeable: false,
+ };
+ element.reload();
+ assert.isFalse(conflictsStub.called);
+ });
+ });
});
</script>
