From e1980c9cceca11d146fc699de50213c3b2b560b8 Mon Sep 17 00:00:00 2001 From: Paladox none Date: Thu, 14 Feb 2019 01:11:48 +0000 Subject: Fix support to use "Default Base For Merges" preference * This implements the setting. * This also uses it for picking rather to show Parent 1 or Auto Merge first. Bug: Issue 8838 Change-Id: I6888b449acb285081db427119990120eb8875e6c --- .../change/gr-change-view/gr-change-view.html | 2 + .../change/gr-change-view/gr-change-view.js | 61 +++++++++++++++++++--- .../change/gr-change-view/gr-change-view_test.html | 5 +- .../gr-file-list-header/gr-file-list-header.html | 3 +- .../gr-file-list-header/gr-file-list-header.js | 9 +--- .../gr-settings-view/gr-settings-view.html | 12 +++++ .../settings/gr-settings-view/gr-settings-view.js | 1 + .../gr-settings-view/gr-settings-view_test.html | 3 ++ 8 files changed, 77 insertions(+), 19 deletions(-) diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html index 4f5bf94795..82279bd5fb 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html @@ -40,6 +40,7 @@ limitations under the License. + @@ -525,6 +526,7 @@ limitations under the License. all-patch-sets="[[_allPatchSets]]" change="[[_change]]" change-num="[[_changeNum]]" + revision-info="[[_revisionInfo]]" change-comments="[[_changeComments]]" commit-info="[[_commitInfo]]" change-url="[[_computeChangeUrl(_change)]]" diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js index 909a1ee0ec..470f8596c1 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js @@ -122,6 +122,7 @@ type: Object, value: {}, }, + _prefs: Object, /** @type {?} */ _changeComments: Object, _canStartReview: { @@ -134,6 +135,10 @@ type: Object, observer: '_changeChanged', }, + _revisionInfo: { + type: Object, + computed: '_getRevisionInfo(_change)', + }, /** @type {?} */ _commitInfo: Object, _currentRevision: { @@ -262,6 +267,7 @@ 'fullscreen-overlay-closed': '_handleShowBackgroundContent', 'diff-comments-modified': '_handleReloadCommentThreads', }, + observers: [ '_labelsChanged(_change.labels.*)', '_paramsAndChangeChanged(params, _change)', @@ -335,7 +341,7 @@ _setDiffViewMode(opt_reset) { if (!opt_reset && this.viewState.diffViewMode) { return; } - return this.$.restAPI.getPreferences().then( prefs => { + return this._getPreferences().then( prefs => { if (!this.viewState.diffMode) { this.set('viewState.diffMode', prefs.default_diff_view); } @@ -790,10 +796,11 @@ _changeChanged(change) { if (!change || !this._patchRange || !this._allPatchSets) { return; } - this.set('_patchRange.basePatchNum', - this._patchRange.basePatchNum || 'PARENT'); - this.set('_patchRange.patchNum', - this._patchRange.patchNum || + + const parent = this._getBasePatchNum(change, this._patchRange); + + this.set('_patchRange.basePatchNum', parent); + this.set('_patchRange.patchNum', this._patchRange.patchNum || this.computeLatestPatchNum(this._allPatchSets)); // Reset the related changes toggle in the event it was previously @@ -804,6 +811,35 @@ this.fire('title-change', {title}); }, + /** + * Gets base patch number, if is a parent try and + * decide from preference weather to default to `auto merge` + * or `Parent 1`. + * @param {Object} change + * @param {Object} patchRange + * @return {number|string} + */ + _getBasePatchNum(change, patchRange) { + if (patchRange.basePatchNum && + patchRange.basePatchNum !== 'PARENT') { + return patchRange.basePatchNum; + } + + const revisionInfo = this._getRevisionInfo(change); + if (!revisionInfo) return 'PARENT'; + + const parentCounts = revisionInfo.getParentCountMap(); + // check that there is at least 2 parents otherwise fall back to 1, + // which means there is only one parent. + const parentCount = parentCounts.hasOwnProperty(1) ? + parentCounts[1] : 1; + + const preferFirst = this._prefs && + this._prefs.default_base_for_merges === 'FIRST_PARENT'; + + return parentCount > 1 && preferFirst ? -1 : 'PARENT'; + }, + _computeChangeUrl(change) { return Gerrit.Nav.getUrlForChange(change); }, @@ -1057,6 +1093,10 @@ }); }, + _getPreferences() { + return this.$.restAPI.getPreferences(); + }, + _updateRebaseAction(revisionActions) { if (revisionActions && revisionActions.rebase) { revisionActions.rebase.rebaseOnCurrent = @@ -1110,9 +1150,12 @@ const detailCompletes = this.$.restAPI.getChangeDetail( this._changeNum, this._handleGetChangeDetailError.bind(this)); const editCompletes = this._getEdit(); + const prefCompletes = this._getPreferences(); + + return Promise.all([detailCompletes, editCompletes, prefCompletes]) + .then(([change, edit, prefs]) => { + this._prefs = prefs; - return Promise.all([detailCompletes, editCompletes]) - .then(([change, edit]) => { if (!change) { return ''; } @@ -1661,6 +1704,10 @@ e.detail.starred); }, + _getRevisionInfo(change) { + return new Gerrit.RevisionInfo(change); + }, + _computeCurrentRevision(currentRevision, revisions) { return revisions && revisions[currentRevision]; }, diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html index f6acef68a1..926895b1fb 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html @@ -1580,8 +1580,8 @@ limitations under the License. }); test('_selectedRevision updates when patchNum is changed', () => { - const revision1 = {_number: 1, commit: {}}; - const revision2 = {_number: 2, commit: {}}; + const revision1 = {_number: 1, commit: {parents: []}}; + const revision2 = {_number: 2, commit: {parents: []}}; sandbox.stub(element.$.restAPI, 'getChangeDetail').returns( Promise.resolve({ revisions: { @@ -1594,6 +1594,7 @@ limitations under the License. change_id: 'loremipsumdolorsitamet', })); sandbox.stub(element, '_getEdit').returns(Promise.resolve()); + sandbox.stub(element, '_getPreferences').returns(Promise.resolve({})); element._patchRange = {patchNum: '2'}; return element._getChangeDetail().then(() => { assert.strictEqual(element._selectedRevision, revision2); diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.html b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.html index 142e706b00..22c476cc42 100644 --- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.html +++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.html @@ -28,7 +28,6 @@ limitations under the License. - @@ -164,7 +163,7 @@ limitations under the License. base-patch-num="[[basePatchNum]]" available-patches="[[allPatchSets]]" revisions="[[change.revisions]]" - revision-info="[[_revisionInfo]]" + revision-info="[[revisionInfo]]" on-patch-range-change="_handlePatchChange"> diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js index 665472bcfc..c6e369c0cc 100644 --- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js +++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js @@ -84,10 +84,7 @@ type: Boolean, computed: '_computeDescriptionReadOnly(loggedIn, change, account)', }, - _revisionInfo: { - type: Object, - computed: '_getRevisionInfo(change)', - }, + revisionInfo: Object, }, behaviors: [ @@ -230,10 +227,6 @@ return 'patchInfoOldPatchSet'; }, - _getRevisionInfo(change) { - return new Gerrit.RevisionInfo(change); - }, - _hideIncludedIn(change) { return change && change.status === MERGED_STATUS ? '' : 'hide'; }, diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html index 799cf5943b..be7f3da842 100644 --- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html +++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html @@ -194,6 +194,18 @@ limitations under the License. +
+ Default Base For Merges + + + + + +
Diff view diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js index 0ce8ce0c80..ec98ad8778 100644 --- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js +++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js @@ -25,6 +25,7 @@ 'diff_view', 'publish_comments_on_push', 'work_in_progress_by_default', + 'default_base_for_merges', 'signed_off_by', 'email_format', 'size_bar_in_change_table', diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html index f47816f4b1..9864a864a2 100644 --- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html +++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html @@ -88,6 +88,7 @@ limitations under the License. diff_view: 'UNIFIED_DIFF', email_strategy: 'ENABLED', email_format: 'HTML_PLAINTEXT', + default_base_for_merges: 'FIRST_PARENT', size_bar_in_change_table: true, my: [ @@ -168,6 +169,8 @@ limitations under the License. .firstElementChild.bindValue, preferences.email_strategy); assert.equal(valueOf('Email format', 'preferences') .firstElementChild.bindValue, preferences.email_format); + assert.equal(valueOf('Default Base For Merges', 'preferences') + .firstElementChild.bindValue, preferences.default_base_for_merges); assert.equal(valueOf('Diff view', 'preferences') .firstElementChild.bindValue, preferences.diff_view); assert.equal(valueOf('Show size bars in file list', 'preferences') -- cgit v1.2.3