opendev/gerrit
Code Issues Proposed changes

Add more fields to Gerrit.Nav.View and utilize in app

Browse Source 
Previously, the app relied on string matching of the view tag names in
order to perform view-based actions (e.g. routing).

This change refactors those instances to utilise the Gerrit.Nav.View
enum.

Bug: Issue 6708
Change-Id: If0212fde93e0167e3207af19006beee1a602df60
This commit is contained in:
Kasper Nilsson
2017-07-17 14:12:16 -07:00
parent 31c4e45f15
commit 3c2e121c9d
14 changed files with 65 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -17,6 +17,7 @@ limitations under the License.
<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../core/gr-navigation/gr-navigation.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../gr-change-list/gr-change-list.html">
<link rel="import" href="../../../styles/shared-styles.html">

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

@@ -100,7 +100,7 @@
},
_paramsChanged(value) {
if (value.view != this.tagName.toLowerCase()) { return; }
if (value.view !== Gerrit.Nav.View.SEARCH) { return; }
this._loading = true;
this._query = value.query;

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

@@ -176,7 +176,7 @@ limitations under the License.
done();
});
element.params = {view: 'gr-change-list-view', query: CHANGE_ID};
element.params = {view: Gerrit.Nav.View.SEARCH, query: CHANGE_ID};
});
test('Searching for a change num redirects to change', done => {
@@ -187,7 +187,7 @@ limitations under the License.
done();
});
element.params = {view: 'gr-change-list-view', query: '1'};
element.params = {view: Gerrit.Nav.View.SEARCH, query: '1'};
});
test('Commit hash redirects to change', done => {
@@ -198,7 +198,7 @@ limitations under the License.
done();
});
element.params = {view: 'gr-change-list-view', query: COMMIT_HASH};
element.params = {view: Gerrit.Nav.View.SEARCH, query: COMMIT_HASH};
});
test('Searching for an invalid change ID searches', () => {
@@ -206,7 +206,7 @@ limitations under the License.
.returns(Promise.resolve([]));
const stub = sandbox.stub(page, 'show');
element.params = {view: 'gr-change-list-view', query: CHANGE_ID};
element.params = {view: Gerrit.Nav.View.SEARCH, query: CHANGE_ID};
flushAsynchronousOperations();
assert.isFalse(stub.called);
@@ -217,7 +217,7 @@ limitations under the License.
.returns(Promise.resolve([{}, {}]));
const stub = sandbox.stub(page, 'show');
element.params = {view: 'gr-change-list-view', query: CHANGE_ID};
element.params = {view: Gerrit.Nav.View.SEARCH, query: CHANGE_ID};
flushAsynchronousOperations();
assert.isFalse(stub.called);

4
polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.html
View File

@@ -49,10 +49,10 @@ limitations under the License.
const getChangesStub = sandbox.stub(element, '_getChanges',
() => Promise.resolve());
element.params = {view: 'gr-dashboard-view'};
element.params = {view: Gerrit.Nav.View.DASHBOARD};
assert.equal(getChangesStub.callCount, 1);
element.params = {view: 'gr-dashboard-view'};
element.params = {view: Gerrit.Nav.View.DASHBOARD};
assert.equal(getChangesStub.callCount, 2);
});

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

@@ -429,7 +429,7 @@
},
_paramsChanged(value) {
if (value.view !== this.tagName.toLowerCase()) {
if (value.view !== Gerrit.Nav.View.CHANGE) {
this._initialLoadComplete = false;
return;
}

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

@@ -614,7 +614,7 @@ limitations under the License.
const relatedClearSpy = sandbox.spy(element.$.relatedChanges, 'clear');
const value = {
view: 'gr-change-view',
view: Gerrit.Nav.View.CHANGE,
patchNum: '1',
};
element._paramsChanged(value);
@@ -636,7 +636,7 @@ limitations under the License.
() => { return Promise.resolve(); });
const value = {
view: 'gr-change-view',
view: Gerrit.Nav.View.CHANGE,
};
element._paramsChanged(value);
assert.isTrue(reloadStub.calledOnce);
@@ -1092,7 +1092,7 @@ limitations under the License.
// simulate reloading component, which is done when route
// changes to match a regex of change view type.
element._paramsChanged({view: 'gr-change-view'});
element._paramsChanged({view: Gerrit.Nav.View.CHANGE});
});
test('scrollTop is reset when new change is loaded', () => {

12
polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html
View File

@@ -83,7 +83,7 @@ limitations under the License.
</tr>
</tbody>
<!-- Change View -->
<tbody hidden$="[[!_computeInView(view, 'gr-change-view')]]" hidden>
<tbody hidden$="[[!_computeInView(view, 'change')]]" hidden>
<tr>
<td></td><td class="header">Navigation</td>
</tr>
@@ -101,7 +101,7 @@ limitations under the License.
</tr>
</tbody>
<!-- Diff View -->
<tbody hidden$="[[!_computeInView(view, 'gr-diff-view')]]" hidden>
<tbody hidden$="[[!_computeInView(view, 'diff')]]" hidden>
<tr>
<td></td><td class="header">Navigation</td>
</tr>
@@ -136,7 +136,7 @@ limitations under the License.
<table>
<!-- Change List -->
<tbody hidden$="[[!_computeInView(view, 'gr-change-list-view')]]" hidden>
<tbody hidden$="[[!_computeInView(view, 'search')]]" hidden>
<tr>
<td></td><td class="header">Change list</td>
</tr>
@@ -172,7 +172,7 @@ limitations under the License.
</tr>
</tbody>
<!-- Dashboard -->
<tbody hidden$="[[!_computeInView(view, 'gr-dashboard-view')]]" hidden>
<tbody hidden$="[[!_computeInView(view, 'dashboard')]]" hidden>
<tr>
<td></td><td class="header">Dashboard</td>
</tr>
@@ -200,7 +200,7 @@ limitations under the License.
</tr>
</tbody>
<!-- Change View -->
<tbody hidden$="[[!_computeInView(view, 'gr-change-view')]]" hidden>
<tbody hidden$="[[!_computeInView(view, 'change')]]" hidden>
<tr>
<td></td><td class="header">Actions</td>
</tr>
@@ -318,7 +318,7 @@ limitations under the License.
</tr>
</tbody>
<!-- Diff View -->
<tbody hidden$="[[!_computeInView(view, 'gr-diff-view')]]" hidden>
<tbody hidden$="[[!_computeInView(view, 'diff')]]" hidden>
<tr>
<td></td><td class="header">Actions</td>
</tr>

6
polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.html
View File

@@ -57,9 +57,13 @@ limitations under the License.
window.Gerrit.Nav = {
View: {
ADMIN: 'admin',
CHANGE: 'change',
SEARCH: 'search',
AGREEMENTS: 'agreements',
DASHBOARD: 'dashboard',
DIFF: 'diff',
SEARCH: 'search',
SETTINGS: 'settings',
},
/** @type {Function} */

44
polygerrit-ui/app/elements/core/gr-router/gr-router.js
View File

@@ -105,7 +105,7 @@
page('/dashboard/(.*)', loadUser, data => {
restAPI.getLoggedIn().then(loggedIn => {
if (loggedIn) {
data.params.view = 'gr-dashboard-view';
data.params.view = Gerrit.Nav.View.DASHBOARD;
app.params = data.params;
} else {
page.redirect('/login/' + encodeURIComponent(data.canonicalPath));
@@ -118,7 +118,7 @@
restAPI.getLoggedIn().then(loggedIn => {
if (loggedIn) {
app.params = {
view: 'gr-admin-view',
view: Gerrit.Nav.View.ADMIN,
adminView: 'gr-admin-group-list',
offset: data.params[1] || 0,
filter: null,
@@ -133,7 +133,7 @@
restAPI.getLoggedIn().then(loggedIn => {
if (loggedIn) {
app.params = {
view: 'gr-admin-view',
view: Gerrit.Nav.View.ADMIN,
adminView: 'gr-admin-group-list',
offset: data.params.offset,
filter: data.params.filter,
@@ -148,7 +148,7 @@
restAPI.getLoggedIn().then(loggedIn => {
if (loggedIn) {
app.params = {
view: 'gr-admin-view',
view: Gerrit.Nav.View.ADMIN,
adminView: 'gr-admin-group-list',
filter: data.params.filter || null,
};
@@ -161,7 +161,7 @@
// Matches /admin/projects/<project>,branches[,<offset>].
page(/^\/admin\/projects\/(.+),branches(,(.+))?$/, loadUser, data => {
app.params = {
view: 'gr-admin-view',
view: Gerrit.Nav.View.ADMIN,
adminView: 'gr-project-detail-list',
detailType: 'branches',
project: data.params[0],
@@ -173,7 +173,7 @@
page('/admin/projects/:project,branches/q/filter::filter,:offset',
loadUser, data => {
app.params = {
view: 'gr-admin-view',
view: Gerrit.Nav.View.ADMIN,
adminView: 'gr-project-detail-list',
detailType: 'branches',
project: data.params.project,
@@ -185,7 +185,7 @@
page('/admin/projects/:project,branches/q/filter::filter',
loadUser, data => {
app.params = {
view: 'gr-admin-view',
view: Gerrit.Nav.View.ADMIN,
adminView: 'gr-project-detail-list',
detailType: 'branches',
project: data.params.project,
@@ -196,7 +196,7 @@
// Matches /admin/projects/<project>,tags[,<offset>].
page(/^\/admin\/projects\/(.+),tags(,(.+))?$/, loadUser, data => {
app.params = {
view: 'gr-admin-view',
view: Gerrit.Nav.View.ADMIN,
adminView: 'gr-project-detail-list',
detailType: 'tags',
project: data.params[0],
@@ -208,7 +208,7 @@
page('/admin/projects/:project,tags/q/filter::filter,:offset',
loadUser, data => {
app.params = {
view: 'gr-admin-view',
view: Gerrit.Nav.View.ADMIN,
adminView: 'gr-project-detail-list',
detailType: 'tags',
project: data.params.project,
@@ -220,7 +220,7 @@
page('/admin/projects/:project,tags/q/filter::filter',
loadUser, data => {
app.params = {
view: 'gr-admin-view',
view: Gerrit.Nav.View.ADMIN,
adminView: 'gr-project-detail-list',
detailType: 'tags',
project: data.params.project,
@@ -231,7 +231,7 @@
// Matches /admin/projects[,<offset>][/].
page(/^\/admin\/projects(,(\d+))?(\/)?$/, loadUser, data => {
app.params = {
view: 'gr-admin-view',
view: Gerrit.Nav.View.ADMIN,
adminView: 'gr-admin-project-list',
offset: data.params[1] || 0,
filter: null,
@@ -240,7 +240,7 @@
page('/admin/projects/q/filter::filter,:offset', loadUser, data => {
app.params = {
view: 'gr-admin-view',
view: Gerrit.Nav.View.ADMIN,
adminView: 'gr-admin-project-list',
offset: data.params.offset,
filter: data.params.filter,
@@ -249,7 +249,7 @@
page('/admin/projects/q/filter::filter', loadUser, data => {
app.params = {
view: 'gr-admin-view',
view: Gerrit.Nav.View.ADMIN,
adminView: 'gr-admin-project-list',
filter: data.params.filter || null,
};
@@ -258,7 +258,7 @@
// Matches /admin/projects/<project>
page(/^\/admin\/projects\/(.+)$/, loadUser, data => {
app.params = {
view: 'gr-admin-view',
view: Gerrit.Nav.View.ADMIN,
project: data.params[0],
adminView: 'gr-project',
};
@@ -268,7 +268,7 @@
restAPI.getLoggedIn().then(loggedIn => {
if (loggedIn) {
app.params = {
view: 'gr-admin-view',
view: Gerrit.Nav.View.ADMIN,
adminView: 'gr-admin-plugin-list',
};
} else {
@@ -280,7 +280,7 @@
page('/admin/(.*)', loadUser, data => {
restAPI.getLoggedIn().then(loggedIn => {
if (loggedIn) {
data.params.view = 'gr-admin-view';
data.params.view = Gerrit.Nav.View.ADMIN;
data.params.placeholder = true;
app.params = data.params;
} else {
@@ -290,7 +290,7 @@
});
function queryHandler(data) {
data.params.view = 'gr-change-list-view';
data.params.view = Gerrit.Nav.View.SEARCH;
app.params = data.params;
}
@@ -349,7 +349,7 @@
changeNum: ctx.params[0],
basePatchNum: ctx.params[3],
patchNum: ctx.params[5],
view: 'gr-change-view',
view: Gerrit.Nav.View.CHANGE,
};
// Don't allow diffing the same patch number against itself.
@@ -375,7 +375,7 @@
patchNum: ctx.params[4],
path: ctx.params[5],
hash: ctx.hash,
view: 'gr-diff-view',
view: Gerrit.Nav.View.DIFF,
};
// Don't allow diffing the same patch number against itself.
if (params.basePatchNum === params.patchNum) {
@@ -406,7 +406,7 @@
page(/^\/settings\/(agreements|new-agreement)/, loadUser, data => {
restAPI.getLoggedIn().then(loggedIn => {
if (loggedIn) {
data.params.view = 'gr-cla-view';
data.params.view = Gerrit.Nav.View.AGREEMENTS;
app.params = data.params;
} else {
page.redirect('/login/' + encodeURIComponent(data.canonicalPath));
@@ -418,7 +418,7 @@
restAPI.getLoggedIn().then(loggedIn => {
if (loggedIn) {
app.params = {
view: 'gr-settings-view',
view: Gerrit.Nav.View.SETTINGS,
emailToken: data.params[0],
};
} else {
@@ -430,7 +430,7 @@
page(/^\/settings\/?/, data => {
restAPI.getLoggedIn().then(loggedIn => {
if (loggedIn) {
app.params = {view: 'gr-settings-view'};
app.params = {view: Gerrit.Nav.View.SETTINGS};
} else {
page.show('/login/' + encodeURIComponent(data.canonicalPath));
}

1
polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
View File

@@ -18,6 +18,7 @@ limitations under the License.
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html">
<link rel="import" href="../../../behaviors/rest-client-behavior/rest-client-behavior.html">
<link rel="import" href="../../../bower_components/iron-dropdown/iron-dropdown.html">
<link rel="import" href="../../core/gr-navigation/gr-navigation.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
<link rel="import" href="../../shared/gr-fixed-panel/gr-fixed-panel.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">

2
polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
View File

@@ -449,7 +449,7 @@
},
_paramsChanged(value) {
if (value.view != this.tagName.toLowerCase()) { return; }
if (value.view !== Gerrit.Nav.View.DIFF) { return; }
this._loadHash(this.params.hash);

6
polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
View File

@@ -469,7 +469,7 @@ limitations under the License.
element._loggedIn = true;
element.params = {
view: 'gr-diff-view',
view: Gerrit.Nav.View.DIFF,
changeNum: '42',
patchNum: '2',
basePatchNum: '1',
@@ -493,7 +493,7 @@ limitations under the License.
});
});
test('hash is determined from parmas', done => {
test('hash is determined from params', done => {
stub('gr-rest-api-interface', {
getDiffComments() { return Promise.resolve({}); },
});
@@ -502,7 +502,7 @@ limitations under the License.
element._loggedIn = true;
element.params = {
view: 'gr-diff-view',
view: Gerrit.Nav.View.DIFF,
changeNum: '42',
patchNum: '2',
basePatchNum: '1',

12
polygerrit-ui/app/elements/gr-app.html
View File

@@ -31,6 +31,10 @@ limitations under the License.
});
</script>
<link rel="import" href="../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html">
<link rel="import" href="../styles/app-theme.html">
<link rel="import" href="../styles/shared-styles.html">
<link rel="import" href="./admin/gr-admin-view/gr-admin-view.html">
<link rel="import" href="./change-list/gr-change-list-view/gr-change-list-view.html">
<link rel="import" href="./change-list/gr-dashboard-view/gr-dashboard-view.html">
@@ -38,6 +42,7 @@ limitations under the License.
<link rel="import" href="./core/gr-error-manager/gr-error-manager.html">
<link rel="import" href="./core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html">
<link rel="import" href="./core/gr-main-header/gr-main-header.html">
<link rel="import" href="./core/gr-navigation/gr-navigation.html">
<link rel="import" href="./core/gr-reporting/gr-reporting.html">
<link rel="import" href="./core/gr-router/gr-router.html">
<link rel="import" href="./diff/gr-diff-view/gr-diff-view.html">
@@ -46,13 +51,6 @@ limitations under the License.
<link rel="import" href="./settings/gr-cla-view/gr-cla-view.html">
<link rel="import" href="./settings/gr-registration-dialog/gr-registration-dialog.html">
<link rel="import" href="./settings/gr-settings-view/gr-settings-view.html">
<link rel="import" href="./shared/gr-fixed-panel/gr-fixed-panel.html">
<link rel="import" href="./shared/gr-overlay/gr-overlay.html">
<link rel="import" href="./shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html">
<link rel="import" href="../styles/app-theme.html">
<link rel="import" href="../styles/shared-styles.html">
<script src="../scripts/util.js"></script>

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

@@ -124,13 +124,13 @@
_viewChanged(view) {
this.$.errorView.classList.remove('show');
this.set('_showChangeListView', view === 'gr-change-list-view');
this.set('_showDashboardView', view === 'gr-dashboard-view');
this.set('_showChangeView', view === 'gr-change-view');
this.set('_showDiffView', view === 'gr-diff-view');
this.set('_showSettingsView', view === 'gr-settings-view');
this.set('_showAdminView', view === 'gr-admin-view');
this.set('_showCLAView', view === 'gr-cla-view');
this.set('_showChangeListView', view === Gerrit.Nav.View.SEARCH);
this.set('_showDashboardView', view === Gerrit.Nav.View.DASHBOARD);
this.set('_showChangeView', view === Gerrit.Nav.View.CHANGE);
this.set('_showDiffView', view === Gerrit.Nav.View.DIFF);
this.set('_showSettingsView', view === Gerrit.Nav.View.SETTINGS);
this.set('_showAdminView', view === Gerrit.Nav.View.ADMIN);
this.set('_showCLAView', view === Gerrit.Nav.View.AGREEMENTS);
if (this.params.justRegistered) {
this.$.registration.open();
}
@@ -193,7 +193,7 @@
if (!this.params) {
return;
}
const viewsToCheck = ['gr-change-list-view', 'gr-dashboard-view'];
const viewsToCheck = [Gerrit.Nav.View.SEARCH, Gerrit.Nav.View.DASHBOARD];
if (viewsToCheck.includes(this.params.view)) {
this.set('_lastSearchPage', location.pathname);
}
@@ -218,7 +218,7 @@
_handleAccountDetailUpdate(e) {
this.$.mainHeader.reload();
if (this.params.view === 'gr-settings-view') {
if (this.params.view === Gerrit.Nav.View.SETTINGS) {
this.$$('gr-settings-view').reloadAccountDetail();
}
},