Do not repeatedly add the 'revert' message modifications
As part of the fix for https://bugs.chromium.org/p/chromium/issues/detail?id=624383 an API event callback was added that allowed plugins to modify the revert message in the revert dialog. The problem is that the modified revert message persisted even when the dialog was cancelled. This caused the same block to be repeated added to the revert dialog. This change ensures that the revert message is constructed right before the dialog is shown. All modifications via the API event callback are now discarded if the dialog is cancelled. BUG= https://bugs.chromium.org/p/chromium/issues/detail?id=637300 Change-Id: Iac2725f6872913e9edf9022df928a36d4b6ad6e6
This commit is contained in:
@@ -274,6 +274,7 @@
|
|||||||
if (type === ActionType.REVISION) {
|
if (type === ActionType.REVISION) {
|
||||||
this._handleRevisionAction(key);
|
this._handleRevisionAction(key);
|
||||||
} else if (key === ChangeActions.REVERT) {
|
} else if (key === ChangeActions.REVERT) {
|
||||||
|
this.$.confirmRevertDialog.populateRevertMessage();
|
||||||
this.$.confirmRevertDialog.message = this._modifyRevertMsg();
|
this.$.confirmRevertDialog.message = this._modifyRevertMsg();
|
||||||
this._showActionDialog(this.$.confirmRevertDialog);
|
this._showActionDialog(this.$.confirmRevertDialog);
|
||||||
} else if (key === ChangeActions.ABANDON) {
|
} else if (key === ChangeActions.ABANDON) {
|
||||||
|
|||||||
@@ -244,18 +244,25 @@ limitations under the License.
|
|||||||
var newRevertMsg = 'Modified revert msg';
|
var newRevertMsg = 'Modified revert msg';
|
||||||
var modifyRevertMsgStub = sinon.stub(element, '_modifyRevertMsg',
|
var modifyRevertMsgStub = sinon.stub(element, '_modifyRevertMsg',
|
||||||
function() { return newRevertMsg; });
|
function() { return newRevertMsg; });
|
||||||
|
var populateRevertMsgStub = sinon.stub(
|
||||||
|
element.$.confirmRevertDialog, 'populateRevertMessage',
|
||||||
|
function() { return 'original msg'; });
|
||||||
flush(function() {
|
flush(function() {
|
||||||
var revertButton = element.$$('gr-button[data-action-key="revert"]');
|
var revertButton = element.$$('gr-button[data-action-key="revert"]');
|
||||||
MockInteractions.tap(revertButton);
|
MockInteractions.tap(revertButton);
|
||||||
|
|
||||||
assert.equal(element.$.confirmRevertDialog.message, newRevertMsg);
|
assert.equal(element.$.confirmRevertDialog.message, newRevertMsg);
|
||||||
|
|
||||||
|
populateRevertMsgStub.restore();
|
||||||
modifyRevertMsgStub.restore();
|
modifyRevertMsgStub.restore();
|
||||||
done();
|
done();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
test('works', function() {
|
test('works', function() {
|
||||||
|
var populateRevertMsgStub = sinon.stub(
|
||||||
|
element.$.confirmRevertDialog, 'populateRevertMessage',
|
||||||
|
function() { return 'original msg'; });
|
||||||
var revertButton = element.$$('gr-button[data-action-key="revert"]');
|
var revertButton = element.$$('gr-button[data-action-key="revert"]');
|
||||||
MockInteractions.tap(revertButton);
|
MockInteractions.tap(revertButton);
|
||||||
|
|
||||||
@@ -276,6 +283,7 @@ limitations under the License.
|
|||||||
'/revert', action, false, {
|
'/revert', action, false, {
|
||||||
message: 'foo message',
|
message: 'foo message',
|
||||||
}]);
|
}]);
|
||||||
|
populateRevertMsgStub.restore();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -32,15 +32,12 @@
|
|||||||
properties: {
|
properties: {
|
||||||
branch: String,
|
branch: String,
|
||||||
message: String,
|
message: String,
|
||||||
commitInfo: {
|
commitInfo: Object,
|
||||||
type: Object,
|
|
||||||
observer: '_commitInfoChanged',
|
|
||||||
},
|
|
||||||
},
|
},
|
||||||
|
|
||||||
_commitInfoChanged: function(commitInfo) {
|
populateRevertMessage: function() {
|
||||||
// Figure out what the revert title should be.
|
// Figure out what the revert title should be.
|
||||||
var originalTitle = commitInfo.message.split('\n')[0];
|
var originalTitle = this.commitInfo.message.split('\n')[0];
|
||||||
var revertTitle = 'Revert of ' + originalTitle;
|
var revertTitle = 'Revert of ' + originalTitle;
|
||||||
if (originalTitle.startsWith('Revert of ')) {
|
if (originalTitle.startsWith('Revert of ')) {
|
||||||
revertTitle = 'Reland of ' +
|
revertTitle = 'Reland of ' +
|
||||||
@@ -50,7 +47,7 @@
|
|||||||
originalTitle.substring('Reland of '.length);
|
originalTitle.substring('Reland of '.length);
|
||||||
}
|
}
|
||||||
// Add '> ' in front of the original commit text.
|
// Add '> ' in front of the original commit text.
|
||||||
var originalCommitText = commitInfo.message.replace(/^/gm, '> ');
|
var originalCommitText = this.commitInfo.message.replace(/^/gm, '> ');
|
||||||
|
|
||||||
this.message = revertTitle + '\n\n' +
|
this.message = revertTitle + '\n\n' +
|
||||||
'Reason for revert: <INSERT REASONING HERE>\n\n' +
|
'Reason for revert: <INSERT REASONING HERE>\n\n' +
|
||||||
|
|||||||
@@ -39,9 +39,10 @@ limitations under the License.
|
|||||||
});
|
});
|
||||||
|
|
||||||
test('single line', function() {
|
test('single line', function() {
|
||||||
assert(element.message == null);
|
assert.isNotOk(element.message);
|
||||||
element.commitInfo = { message: 'one line commit' };
|
element.commitInfo = {message: 'one line commit'};
|
||||||
console.log(element.message);
|
assert.isNotOk(element.message);
|
||||||
|
element.populateRevertMessage();
|
||||||
var expected = 'Revert of one line commit\n\n' +
|
var expected = 'Revert of one line commit\n\n' +
|
||||||
'Reason for revert: <INSERT REASONING HERE>\n\n' +
|
'Reason for revert: <INSERT REASONING HERE>\n\n' +
|
||||||
'Original issue\'s description:\n' +
|
'Original issue\'s description:\n' +
|
||||||
@@ -50,9 +51,10 @@ limitations under the License.
|
|||||||
});
|
});
|
||||||
|
|
||||||
test('multi line', function() {
|
test('multi line', function() {
|
||||||
assert(element.message == null);
|
assert.isNotOk(element.message);
|
||||||
element.commitInfo = { message: 'many lines\ncommit\n\nmessage\n' };
|
element.commitInfo = {message: 'many lines\ncommit\n\nmessage\n'};
|
||||||
console.log(element.message);
|
assert.isNotOk(element.message);
|
||||||
|
element.populateRevertMessage();
|
||||||
var expected = 'Revert of many lines\n\n' +
|
var expected = 'Revert of many lines\n\n' +
|
||||||
'Reason for revert: <INSERT REASONING HERE>\n\n' +
|
'Reason for revert: <INSERT REASONING HERE>\n\n' +
|
||||||
'Original issue\'s description:\n' +
|
'Original issue\'s description:\n' +
|
||||||
|
|||||||
Reference in New Issue
Block a user