opendev/gerrit
Code Issues Proposed changes

Rely on params change to set _lastSearchPage

Browse Source 
Gerrit remembers the last page that contains a change-list (either a
dashboard or a page with search results) so that users can return there
with the 'u' key.

This is done by saving the last relative URL as a property at the gr-app
level.

The app relied on the 'location-changed' event to set this property, but
in some cases that event is fired before the app params are set. One
such case is on a fresh reload of a search page.

In that case, the _handleSearchPageChange function has no way of knowing
what view is currently loaded, and prematurely exits.

With this change, Gerrit now observes the app params to set the
property.

Bug: Issue 7368
Change-Id: I62ead340677d9a416a4713ff003fc3ef18d3624f
This commit is contained in:
Kasper Nilsson 2018-04-13 10:33:29 -07:00
parent 2f90615543
commit c27a23c06b
2 changed files with 13 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
polygerrit-ui/app/elements/gr-app.js
View File

@ -98,6 +98,7 @@
observers: [
'_viewChanged(params.view)',
'_paramsChanged(params.*)',
],
behaviors: [
@ -227,15 +228,12 @@
pathname += '@' + hash;
}
this.set('_path', pathname);
this._handleSearchPageChange();
},
_handleSearchPageChange() {
if (!this.params) {
return;
}
_paramsChanged(paramsRecord) {
const params = paramsRecord.base;
const viewsToCheck = [Gerrit.Nav.View.SEARCH, Gerrit.Nav.View.DASHBOARD];
if (viewsToCheck.includes(this.params.view)) {
if (viewsToCheck.includes(params.view)) {
this.set('_lastSearchPage', location.pathname);
}
},

10
polygerrit-ui/app/elements/gr-app_test.html
View File

@ -55,6 +55,8 @@ limitations under the License.
});
},
getPreferences() { return Promise.resolve({my: []}); },
getDiffPreferences() { return Promise.resolve({}); },
getEditPreferences() { return Promise.resolve({}); },
getVersion() { return Promise.resolve(42); },
probePath() { return Promise.resolve(42); },
});
@ -87,7 +89,6 @@ limitations under the License.
hash: '#2',
host: location.host,
};
sandbox.stub(element, '_handleSearchPageChange');
element._handleLocationChange({detail: curLocation});
flush(() => {
@ -107,6 +108,13 @@ limitations under the License.
});
});
test('_paramsChanged sets search page', () => {
element._paramsChanged({base: {view: Gerrit.Nav.View.CHANGE}});
assert.notOk(element._lastSearchPage);
element._paramsChanged({base: {view: Gerrit.Nav.View.SEARCH}});
assert.ok(element._lastSearchPage);
});
suite('_jumpKeyPressed', () => {
let navStub;