Use _handleReload to reload instead of navigating to change directly * Remove reload-change event and use reload event as replacement. * Use _handleReloadChange as a central place for reloading the change. This change allows us to add checks if page should be reloaded or not at one single place. Change-Id: I43908be6a24edad52df269634205a5ee39a156dd
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js index 259fea8..510be16 100644 --- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js +++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -255,7 +255,7 @@ /** * Fired when the change should be reloaded. * - * @event reload-change + * @event reload */ /** @@ -1371,7 +1371,12 @@ case ChangeActions.REBASE_EDIT: case ChangeActions.REBASE: case ChangeActions.SUBMIT: - GerritNav.navigateToChange(this.change); + this.dispatchEvent(new CustomEvent('reload', + { + detail: {clearPatchset: true}, + bubbles: false, + composed: true, + })); break; case ChangeActions.REVERT_SUBMISSION: if (!obj.revert_changes || !obj.revert_changes.length) return; @@ -1381,8 +1386,12 @@ obj.revert_changes[0].topic); break; default: - this.dispatchEvent(new CustomEvent('reload-change', - {detail: {action: action.__key}, bubbles: false})); + this.dispatchEvent(new CustomEvent('reload', + { + detail: {action: action.__key, clearPatchset: true}, + bubbles: false, + composed: true, + })); break; } }); @@ -1433,8 +1442,12 @@ 'uploaded to this change.', action: 'Reload', callback: () => { - // Load the current change without any patch range. - GerritNav.navigateToChange(this.change); + this.dispatchEvent(new CustomEvent('reload', + { + detail: {clearPatchset: true}, + bubbles: false, + composed: true, + })); }, }, composed: true, bubbles: true,
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.js index 0655931..f5d2080 100644 --- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.js +++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.js
@@ -395,13 +395,14 @@ }); }); - test('rebase change calls navigateToChange', done => { - const navigateToChangeStub = sinon.stub(GerritNav, 'navigateToChange'); + test('rebase change fires reload event', done => { + const eventStub = sinon.stub(element, 'dispatchEvent'); sinon.stub(element.$.restAPI, 'getResponseObject').returns( Promise.resolve({})); element._handleResponse({__key: 'rebase'}, {}); flush(() => { - assert.isTrue(navigateToChangeStub.called); + assert.isTrue(eventStub.called); + assert.equal(eventStub.lastCall.args[0].type, 'reload'); done(); }); });
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 9547ce4..7b6156c 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
@@ -534,7 +534,8 @@ e => this._setActiveSecondaryTab(e)); this.addEventListener('reload', e => { e.stopPropagation(); - this._reload(); + this._reload(/* opt_isLocationChange= */false, + /* opt_clearPatchset= */e.detail && e.detail.clearPatchset); }); } @@ -1493,7 +1494,8 @@ _handleRefreshChange(e) { if (this.shouldSuppressKeyboardShortcut(e)) { return; } e.preventDefault(); - GerritNav.navigateToChange(this._change); + this._reload(/* opt_isLocationChange= */false, + /* opt_clearPatchset= */true); } _handleToggleChangeStar(e) { @@ -1584,10 +1586,6 @@ }); } - _handleReloadChange() { - return this._reload(); - } - _handleGetChangeDetailError(response) { this.dispatchEvent(new CustomEvent('page-error', { detail: {response}, @@ -1819,11 +1817,17 @@ * * @param {boolean=} opt_isLocationChange Reloads the related changes * when true and ends reporting events that started on location change. + * @param {boolean=} opt_clearPatchset Reloads the related changes + * ignoring any patchset choice made. * @return {Promise} A promise that resolves when the core data has loaded. * Some non-core data loading may still be in-flight when the core data * promise resolves. */ - _reload(opt_isLocationChange) { + _reload(opt_isLocationChange, opt_clearPatchset) { + if (opt_clearPatchset) { + GerritNav.navigateToChange(this._change); + return; + } this._loading = true; this._relatedChangesCollapsed = true; this.reporting.time(CHANGE_RELOAD_TIMING_LABEL); @@ -2149,10 +2153,10 @@ // Persist this alert. dismissOnNavigation: true, action: 'Reload', - callback: function() { - // Load the current change without any patch range. - GerritNav.navigateToChange(this._change); - }.bind(this), + callback: () => { + this._reload(/* opt_isLocationChange= */false, + /* opt_clearPatchset= */true); + }, }, composed: true, bubbles: true, }));
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts index 04a810f..6352238 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
@@ -409,7 +409,6 @@ edit-mode="[[_editMode]]" edit-based-on-current-patch-set="[[_hasEditBasedOnCurrentPatchSet(_allPatchSets)]]" private-by-default="[[_projectConfig.private_by_default]]" - on-reload-change="_handleReloadChange" on-edit-tap="_handleEditTap" on-stop-edit-tap="_handleStopEditTap" on-download-tap="_handleOpenDownloadDialog"
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.js index c853449..a66c6a2 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.js +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.js
@@ -662,6 +662,13 @@ }); }); + test('reload event from reply dialog is processed', () => { + const handleReloadStub = sinon.stub(element, '_reload'); + element.$.replyDialog.dispatchEvent(new CustomEvent('reload', + {detail: {clearPatchset: true}, bubbles: true, composed: true})); + assert.isTrue(handleReloadStub.called); + }); + test('shift + R should fetch and navigate to the latest patch set', done => { element._changeNum = '42'; @@ -681,16 +688,12 @@ actions: {}, }; - navigateToChangeStub.restore(); - navigateToChangeStub = sinon.stub(GerritNav, 'navigateToChange') - .callsFake((change, patchNum, basePatchNum) => { - assert.equal(change, element._change); - assert.isUndefined(patchNum); - assert.isUndefined(basePatchNum); - done(); - }); - + const reloadChangeStub = sinon.stub(element, '_reload'); MockInteractions.pressAndReleaseKeyOn(element, 82, 'shift', 'r'); + flush(() => { + assert.isTrue(reloadChangeStub.called); + done(); + }); }); test('d should open download overlay', () => { @@ -1308,7 +1311,7 @@ sinon.stub(element, '_reload').callsFake(() => Promise.resolve()); sinon.stub(element.$.relatedChanges, 'reload'); const e = {detail: {action: 'abandon'}}; - element._handleReloadChange(e).then(() => { + element._reload(e).then(() => { assert.isFalse(navigateToChangeStub.called); done(); });
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js index 1008c70..983d822 100644 --- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js +++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
@@ -33,7 +33,6 @@ import {PolymerElement} from '@polymer/polymer/polymer-element.js'; import {htmlTemplate} from './gr-reply-dialog_html.js'; import {GrReviewerSuggestionsProvider, SUGGESTIONS_PROVIDERS_USERS_TYPES} from '../../../scripts/gr-reviewer-suggestions-provider/gr-reviewer-suggestions-provider.js'; -import {GerritNav} from '../../core/gr-navigation/gr-navigation.js'; import {appContext} from '../../../services/app-context.js'; import {SpecialFilePath} from '../../../constants/constants.js'; import {ExperimentIds} from '../../../services/flags.js'; @@ -122,6 +121,12 @@ * @event send-disabled-changed */ + /** + * Fired to reload the change page. + * + * @event reload + */ + constructor() { super(); this.FocusTarget = FocusTarget; @@ -1043,8 +1048,8 @@ } _reload() { - // Load the current change without any patch range. - GerritNav.navigateToChange(this.change); + this.dispatchEvent(new CustomEvent('reload', + {detail: {clearPatchset: true}, bubbles: false, composed: true})); this.cancel(); }