opendev/gerrit
Code Issues Proposed changes

Add hasParent attribute to change view/change actions/rebase dialog

Browse Source 
Get a hasParent attribute from the related changes endpoint, determining
if a change has a parent change that is not the tip of the branch.
This will be used by the rebase dialog to display tailored options
that make rebasing more intuitive. UI changes to follow.

This change also updates rebaseOnCurrent from being a property of
gr-change-view to getting added in the rest api interface as a property
of the rebase action.  This is why the property is removed from all
views until the reply dialog, where it is passed as a parameter.

Bug: Issue 5447
Change-Id: If31e4c42d8e82f00fba9fa98c28371046299eec4
This commit is contained in:
Becky Siegel
2017-02-14 11:42:08 -08:00
parent f1e305f296
commit bb3acdb083
11 changed files with 91 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
View File

@@ -104,10 +104,12 @@ limitations under the License.
</div>
<gr-overlay id="overlay" with-backdrop>
<gr-confirm-rebase-dialog id="confirmRebase"
rebase-on-current="[[rebaseOnCurrent]]"
class="confirmDialog"
on-confirm="_handleRebaseConfirm"
on-cancel="_handleConfirmDialogCancel"
branch="[[change.branch]]"
has-parent="[[hasParent]]"
rebase-on-current="[[revisionActions.rebase.rebaseOnCurrent]]"
hidden></gr-confirm-rebase-dialog>
<gr-confirm-cherrypick-dialog id="confirmCherrypick"
class="confirmDialog"

2
polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
View File

@@ -129,8 +129,8 @@
changeNum: String,
changeStatus: String,
commitNum: String,
hasParent: Boolean,
patchNum: String,
rebaseOnCurrent: Boolean,
commitMessage: {
type: String,
value: '',

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

@@ -314,8 +314,8 @@ limitations under the License.
on-tap="_handleReplyTap">[[_replyButtonLabel]]</gr-button>
<gr-change-actions id="actions"
change="[[_change]]"
has-parent="[[hasParent]]"
actions="[[_change.actions]]"
rebase-on-current="[[_rebaseOnCurrent]]"
revision-actions="[[_currentRevisionActions]]"
change-num="[[_changeNum]]"
change-status="[[_change.status]]"
@@ -369,6 +369,7 @@ limitations under the License.
<div class="relatedChanges">
<gr-related-changes-list id="relatedChanges"
change="[[_change]]"
has-parent="{{hasParent}}"
patch-num="[[_computeLatestPatchNum(_allPatchSets)]]">
</gr-related-changes-list>
</div>

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

@@ -56,6 +56,7 @@
value: function() { return {}; },
},
backPage: String,
hasParent: Boolean,
serverConfig: Object,
keyEventTarget: {
type: Object,
@@ -829,7 +830,8 @@
_updateRebaseAction: function(revisionActions) {
if (revisionActions && revisionActions.rebase) {
this._rebaseOnCurrent = !!revisionActions.rebase.enabled;
revisionActions.rebase.rebaseOnCurrent =
!!revisionActions.rebase.enabled;
revisionActions.rebase.enabled = true;
}
return revisionActions;

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

@@ -219,17 +219,18 @@ limitations under the License.
assert.equal(element._updateRebaseAction(currentRevisionActions),
currentRevisionActions);
assert.isTrue(element._rebaseOnCurrent);
assert.isTrue(currentRevisionActions.rebase.enabled);
assert.isTrue(currentRevisionActions.rebase.rebaseOnCurrent);
var newRevisionActions = currentRevisionActions
delete newRevisionActions.rebase.enabled;
delete currentRevisionActions.rebase.enabled;
// When rebase is not enabled initially, rebaseOnCurrent should be set to
// false.
assert.equal(element._updateRebaseAction(newRevisionActions),
assert.equal(element._updateRebaseAction(currentRevisionActions),
currentRevisionActions);
assert.isFalse(element._rebaseOnCurrent);
assert.isTrue(currentRevisionActions.rebase.enabled);
assert.isFalse(currentRevisionActions.rebase.rebaseOnCurrent);
});
test('_reload is called when an approved label is removed', function() {

2
polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.js
View File

@@ -31,6 +31,8 @@
properties: {
base: String,
branch: String,
hasParent: Boolean,
clearParent: {
type: Boolean,
value: false,

2
polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html
View File

@@ -256,7 +256,7 @@ limitations under the License.
id: '8c19ccc949c6d482b061be6a28e10782abf0e7af',
}];
element.messages = messages;
element.comments = comments
element.comments = comments;
flushAsynchronousOperations();
var messageEls = Polymer.dom(element.root).querySelectorAll('gr-message');
assert.equal(messageEls.length, 1);

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

@@ -19,7 +19,12 @@
properties: {
change: Object,
hasParent: {
type: Boolean,
notify: true,
},
patchNum: String,
parentChange: Object,
hidden: {
type: Boolean,
value: false,
@@ -56,6 +61,10 @@
var promises = [
this._getRelatedChanges().then(function(response) {
this._relatedResponse = response;
this.hasParent = this._calculateHasParent(this.change.change_id,
response.changes);
}.bind(this)),
this._getSubmittedTogether().then(function(response) {
this._submittedTogether = response;
@@ -88,6 +97,20 @@
}.bind(this));
},
/**
* Determines whether or not the given change has a parent change. If there
* is a relation chain, and the change id is not the last item of the
* relation chain, there is a parent.
* @param {Number} currentChangeId
* @param {Array} relatedChanges
* @return {Boolean}
*/
_calculateHasParent: function(currentChangeId, relatedChanges) {
return relatedChanges.length > 0 &&
relatedChanges[relatedChanges.length - 1].change_id !==
currentChangeId;
},
_getRelatedChanges: function() {
return this.$.restAPI.getRelatedChanges(this.change._number,
this.patchNum);

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

@@ -238,7 +238,9 @@ limitations under the License.
element = fixture('basic');
sandbox.stub(element, '_getRelatedChanges',
function() { return Promise.resolve(); });
function() {
return Promise.resolve({changes: []});
});
sandbox.stub(element, '_getSubmittedTogether',
function() { return Promise.resolve(); });
sandbox.stub(element, '_getCherryPicks',
@@ -250,6 +252,7 @@ limitations under the License.
test('request conflicts if open and mergeable', function() {
element.patchNum = 7;
element.change = {
change_id: 123,
status: 'NEW',
mergeable: true,
};
@@ -260,6 +263,7 @@ limitations under the License.
test('does not request conflicts if closed and mergeable', function() {
element.patchNum = 7;
element.change = {
change_id: 123,
status: 'MERGED',
mergeable: true,
};
@@ -270,6 +274,7 @@ limitations under the License.
test('does not request conflicts if open and not mergeable', function() {
element.patchNum = 7;
element.change = {
change_id: 123,
status: 'NEW',
mergeable: false,
};
@@ -281,6 +286,7 @@ limitations under the License.
function() {
element.patchNum = 7;
element.change = {
change_id: 123,
status: 'MERGED',
mergeable: false,
};
@@ -288,5 +294,22 @@ limitations under the License.
assert.isFalse(conflictsStub.called);
});
});
test('_calculateHasParent', function() {
var changeId = 123;
var relatedChanges = [];
assert.equal(element._calculateHasParent(changeId, relatedChanges),
false);
relatedChanges.push({change_id: 123});
assert.equal(element._calculateHasParent(changeId, relatedChanges),
false);
relatedChanges.push({change_id: 234});
assert.equal(element._calculateHasParent(changeId, relatedChanges),
true);
});
});
</script>

2
polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
View File

@@ -513,6 +513,8 @@
function(revisionActions) {
// The rebase button on change screen is always enabled.
if (revisionActions.rebase) {
revisionActions.rebase.rebaseOnCurrent =
!!revisionActions.rebase.enabled;
revisionActions.rebase.enabled = true;
}
return revisionActions;

31
polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
View File

@@ -383,20 +383,37 @@ limitations under the License.
]);
});
test('rebase always enabled', function(done) {
suite('rebase action', function() {
var resolveFetchJSON;
sandbox.stub(element, 'fetchJSON').returns(
new Promise(function(resolve) {
resolveFetchJSON = resolve;
}));
element.getChangeRevisionActions('42', '1337').then(
setup(function() {
sandbox.stub(element, 'fetchJSON').returns(
new Promise(function(resolve) {
resolveFetchJSON = resolve;
}));
});
test('no rebase on current', function(done) {
element.getChangeRevisionActions('42', '1337').then(
function(response) {
assert.isTrue(response.rebase.enabled);
assert.isFalse(response.rebase.rebaseOnCurrent);
done();
});
resolveFetchJSON({rebase: {}});
resolveFetchJSON({rebase: {}});
});
test('rebase on current', function(done) {
element.getChangeRevisionActions('42', '1337').then(
function(response) {
assert.isTrue(response.rebase.enabled);
assert.isTrue(response.rebase.rebaseOnCurrent);
done();
});
resolveFetchJSON({rebase: {enabled: true}});
});
});
test('server error', function(done) {
var getResponseObjectStub = sandbox.stub(element, 'getResponseObject');
sandbox.stub(window, 'fetch', function() {