From c34e0b6cd50fd4480325a4cb44572e63c8babeb7 Mon Sep 17 00:00:00 2001 From: Logan Hanks Date: Tue, 3 Oct 2017 01:40:54 -0700 Subject: [PATCH] Implement project dashboard view in PolyGerrit Project dashboards are defined by files at special refs in a project (or a project's inheritence tree). https://gerrit-review.googlesource.com/Documentation/user-dashboards.html#project-dashboards This change includes some refactoring of gr-dashboard-view to better accommodate the variety of dashboards it supports. It also comes with more tests and a fix for a minor regression (special suffixes that are used in the query to populate the items in a dashboard section, but should be dropped in the href used in the section title hyperlink). Bug: Issue 7319 Bug: Issue 7335 Change-Id: Iffd7484b0d4628b7a4a483c895c96179d7fbecda --- .../java/com/google/gerrit/client/Gerrit.java | 6 + .../google/gerrit/httpd/raw/StaticModule.java | 3 +- .../base-url-behavior/base-url-behavior.html | 8 +- .../base-url-behavior_test.html | 6 + .../gr-dashboard-view/gr-dashboard-view.js | 120 ++++++--- .../gr-dashboard-view_test.html | 253 ++++++++++++++---- .../app/elements/core/gr-router/gr-router.js | 15 ++ .../core/gr-router/gr-router_test.html | 12 + .../gr-rest-api-interface.js | 15 ++ .../gr-rest-api-interface_test.html | 9 + polygerrit-ui/server.go | 2 +- 11 files changed, 364 insertions(+), 85 deletions(-) diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/Gerrit.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/Gerrit.java index e02c4e0b80..6138e8bfbb 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/Gerrit.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/Gerrit.java @@ -561,6 +561,12 @@ public class Gerrit implements EntryPoint { if (Location.getPath().endsWith("/") && tokens[0].startsWith("/")) { tokens[0] = tokens[0].substring(1); } + if (tokens[0].startsWith("projects/") && tokens[0].contains(",dashboards/")) { + // Rewrite project dashboard URIs to a new format, because otherwise + // "/projects/..." would be served as an API request. + tokens[0] = "p/" + tokens[0].substring("projects/".length()); + tokens[0] = tokens[0].replace(",dashboards/", "/+/dashboard/"); + } builder.setPath(Location.getPath() + tokens[0]); if (tokens.length == 2) { builder.setHash(tokens[1]); diff --git a/java/com/google/gerrit/httpd/raw/StaticModule.java b/java/com/google/gerrit/httpd/raw/StaticModule.java index c20a2b0d12..e1c094c03f 100644 --- a/java/com/google/gerrit/httpd/raw/StaticModule.java +++ b/java/com/google/gerrit/httpd/raw/StaticModule.java @@ -72,7 +72,8 @@ public class StaticModule extends ServletModule { *

Supports {@code "/*"} as a trailing wildcard. */ public static final ImmutableList POLYGERRIT_INDEX_PATHS = - ImmutableList.of("/", "/c/*", "/q/*", "/x/*", "/admin/*", "/dashboard/*", "/settings/*"); + ImmutableList.of( + "/", "/c/*", "/p/*", "/q/*", "/x/*", "/admin/*", "/dashboard/*", "/settings/*"); // TODO(dborowitz): These fragments conflict with the REST API // namespace, so they will need to use a different path. //"/groups/*", diff --git a/polygerrit-ui/app/behaviors/base-url-behavior/base-url-behavior.html b/polygerrit-ui/app/behaviors/base-url-behavior/base-url-behavior.html index cda8c53090..027d481080 100644 --- a/polygerrit-ui/app/behaviors/base-url-behavior/base-url-behavior.html +++ b/polygerrit-ui/app/behaviors/base-url-behavior/base-url-behavior.html @@ -20,6 +20,8 @@ limitations under the License. window.Gerrit = window.Gerrit || {}; + const PROJECT_DASHBOARD_PATTERN = /\/p\/(.+)\/\+\/dashboard\/(.*)/; + /** @polymerBehavior Gerrit.BaseUrlBehavior */ Gerrit.BaseUrlBehavior = { /** @return {string} */ @@ -29,7 +31,11 @@ limitations under the License. computeGwtUrl(path) { const base = this.getBaseUrl(); - const clientPath = path.substring(base.length); + let clientPath = path.substring(base.length); + const match = clientPath.match(PROJECT_DASHBOARD_PATTERN); + if (match) { + clientPath = `/projects/${match[1]},dashboards/${match[2]}`; + } return base + '/?polygerrit=0#' + clientPath; }, }; diff --git a/polygerrit-ui/app/behaviors/base-url-behavior/base-url-behavior_test.html b/polygerrit-ui/app/behaviors/base-url-behavior/base-url-behavior_test.html index b7c29dc777..1fa8f3493a 100644 --- a/polygerrit-ui/app/behaviors/base-url-behavior/base-url-behavior_test.html +++ b/polygerrit-ui/app/behaviors/base-url-behavior/base-url-behavior_test.html @@ -72,5 +72,11 @@ limitations under the License. '/r/?polygerrit=0#/c/1/' ); }); + + test('computeGwtUrl for project dashboard', () => { + assert.deepEqual( + element.computeGwtUrl('/r/p/gerrit/proj/+/dashboard/main:default'), + '/r/?polygerrit=0#/projects/gerrit/proj,dashboards/main:default'); + }); }); diff --git a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.js b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.js index 4d39a90a64..6acd9fa1b0 100644 --- a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.js +++ b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.js @@ -14,6 +14,9 @@ (function() { 'use strict'; + const PROJECT_PLACEHOLDER_PATTERN = /\$\{project\}/g; + const USER_PLACEHOLDER_PATTERN = /\$\{user\}/g; + // NOTE: These queries are tested in Java. Any changes made to definitions // here require corresponding changes to: // gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -104,60 +107,107 @@ ); }, + _getProjectDashboard(project, dashboard) { + const errFn = response => { + this.fire('page-error', {response}); + }; + return this.$.restAPI.getDashboard( + project, dashboard, errFn).then(response => { + if (!response) { + return; + } + return { + title: response.title, + sections: response.sections.map(section => { + const suffix = response.foreach ? ' ' + response.foreach : ''; + return { + name: section.name, + query: + section.query.replace( + PROJECT_PLACEHOLDER_PATTERN, project) + suffix, + }; + }), + }; + }); + }, + + _getUserDashboard(user, sections, title) { + sections = sections + .filter(section => (user === 'self' || !section.selfOnly)) + .map(section => { + const dashboardSection = { + name: section.name, + query: section.query.replace(USER_PLACEHOLDER_PATTERN, user), + }; + if (section.suffixForDashboard) { + dashboardSection.suffixForDashboard = section.suffixForDashboard; + } + return dashboardSection; + }); + return Promise.resolve({title, sections}); + }, + _computeTitle(user) { - if (user === 'self') { + if (!user || user === 'self') { return 'My Reviews'; } return 'Dashboard for ' + user; }, + _isViewActive(params) { + return params.view === Gerrit.Nav.View.DASHBOARD; + }, + _paramsChanged(paramsChangeRecord) { const params = paramsChangeRecord.base; - if (!params.user && !params.sections) { - return; + if (!this._isViewActive(params)) { + return Promise.resolve(); } const user = params.user || 'self'; - const sections = (params.sections || DEFAULT_SECTIONS).filter( - section => (user === 'self' || !section.selfOnly)); - const title = params.title || this._computeTitle(user); // NOTE: This method may be called before attachment. Fire title-change // in an async so that attachment to the DOM can take place first. + const title = params.title || this._computeTitle(user); this.async(() => this.fire('title-change', {title})); - // Return if params indicate no longer in view. - if (!user && sections === DEFAULT_SECTIONS) { - return; - } - this._loading = true; - const queries = - sections.map( - section => this._dashboardQueryForSection(section, user)); - this.$.restAPI.getChanges(null, queries, null, this.options) - .then(results => { - this._results = sections.map((section, i) => { - return { - sectionName: section.name, - query: queries[i], - results: results[i], - }; - }); - this._loading = false; - }).catch(err => { - this._loading = false; - console.warn(err.message); - }); - }, - _dashboardQueryForSection(section, user) { - const query = - section.suffixForDashboard ? - section.query + ' ' + section.suffixForDashboard : - section.query; - return query.replace(/\$\{user\}/g, user); + const dashboardPromise = params.project ? + this._getProjectDashboard(params.project, params.dashboard) : + this._getUserDashboard( + params.user || 'self', + params.sections || DEFAULT_SECTIONS, + params.title || this._computeTitle(params.user)); + + return dashboardPromise.then(dashboard => { + if (!dashboard) { + this._loading = false; + return; + } + const queries = dashboard.sections.map(section => { + if (section.suffixForDashboard) { + return section.query + ' ' + section.suffixForDashboard; + } + return section.query; + }); + const req = + this.$.restAPI.getChanges(null, queries, null, this.options); + return req.then(response => { + this._loading = false; + this._results = response.map((results, i) => { + return { + sectionName: dashboard.sections[i].name, + query: dashboard.sections[i].query, + results, + }; + }); + }); + }).catch(err => { + this._loading = false; + console.warn(err); + }); }, _computeUserHeaderClass(userParam) { diff --git a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.html b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.html index 40376adbc0..f7d933a977 100644 --- a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.html +++ b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.html @@ -35,67 +35,226 @@ limitations under the License. suite('gr-dashboard-view tests', () => { let element; let sandbox; + let paramsChangedPromise; setup(() => { element = fixture('basic'); sandbox = sinon.sandbox.create(); getChangesStub = sandbox.stub(element.$.restAPI, 'getChanges', - () => Promise.resolve()); + () => Promise.resolve([])); + + let resolver; + paramsChangedPromise = new Promise(resolve => { + resolver = resolve; + }); + const paramsChanged = element._paramsChanged.bind(element); + sandbox.stub(element, '_paramsChanged', params => { + paramsChanged(params).then(resolver()); + }); }); teardown(() => { sandbox.restore(); }); - test('nothing happens when user param is falsy', () => { - element.params = {}; - flushAsynchronousOperations(); - assert.equal(getChangesStub.callCount, 0); - - element.params = {user: ''}; - flushAsynchronousOperations(); - assert.equal(getChangesStub.callCount, 0); - }); - - test('content is refreshed when user param is updated', () => { - element.params = {user: 'self'}; - flushAsynchronousOperations(); - assert.equal(getChangesStub.callCount, 1); - }); - - test('viewing another user\'s dashboard omits selfOnly sections', () => { - element.params = { - sections: [ - {query: '1'}, - {query: '2', selfOnly: true}, - ], - user: 'self', - }; - flushAsynchronousOperations(); - assert.isTrue( - getChangesStub.calledWith(null, ['1', '2'], null, element.options)); - - element.set('params.user', 'user'); - flushAsynchronousOperations(); - assert.isTrue( - getChangesStub.calledWith(null, ['1'], null, element.options)); - }); - - test('_dashboardQueryForSection', () => { - const query = 'query for ${user}'; - const suffixForDashboard = 'suffix for ${user}'; - assert.equal( - element._dashboardQueryForSection({query}, 'user'), - 'query for user'); - assert.equal( - element._dashboardQueryForSection( - {query, suffixForDashboard}, 'user'), - 'query for user suffix for user'); - }); - test('_computeTitle', () => { assert.equal(element._computeTitle('self'), 'My Reviews'); assert.equal(element._computeTitle('not self'), 'Dashboard for not self'); }); + + suite('_isViewActive', () => { + test('nothing happens when user param is falsy', () => { + element.params = {}; + flushAsynchronousOperations(); + assert.equal(getChangesStub.callCount, 0); + + element.params = {user: ''}; + flushAsynchronousOperations(); + assert.equal(getChangesStub.callCount, 0); + }); + + test('content is refreshed when user param is updated', () => { + element.params = { + view: Gerrit.Nav.View.DASHBOARD, + user: 'self', + }; + return paramsChangedPromise.then(() => { + assert.equal(getChangesStub.callCount, 1); + }); + }); + }); + + suite('selfOnly sections', () => { + test('viewing self dashboard includes selfOnly sections', () => { + element.params = { + view: Gerrit.Nav.View.DASHBOARD, + sections: [ + {query: '1'}, + {query: '2', selfOnly: true}, + ], + user: 'user', + }; + return paramsChangedPromise.then(() => { + assert.isTrue( + getChangesStub.calledWith( + null, ['1'], null, element.options)); + }); + }); + + test('viewing another user\'s dashboard omits selfOnly sections', () => { + element.params = { + view: Gerrit.Nav.View.DASHBOARD, + sections: [ + {query: '1'}, + {query: '2', selfOnly: true}, + ], + user: 'self', + }; + return paramsChangedPromise.then(() => { + assert.isTrue( + getChangesStub.calledWith( + null, ['1', '2'], null, element.options)); + }); + }); + }); + + test('suffixForDashboard is included in getChanges query', () => { + element.params = { + view: Gerrit.Nav.View.DASHBOARD, + sections: [ + {query: '1'}, + {query: '2', suffixForDashboard: 'suffix'}, + ], + }; + return paramsChangedPromise.then(() => { + assert.isTrue(getChangesStub.calledOnce); + assert.deepEqual( + getChangesStub.firstCall.args, + [null, ['1', '2 suffix'], null, element.options]); + }); + }); + + suite('_getProjectDashboard', () => { + test('dashboard with foreach', () => { + sandbox.stub(element.$.restAPI, 'getDashboard', () => Promise.resolve({ + title: 'title', + // Note: ${project} should not be resolved in foreach! + foreach: 'foreach for ${project}', + sections: [ + {name: 'section 1', query: 'query 1'}, + {name: 'section 2', query: '${project} query 2'}, + ], + })); + return element._getProjectDashboard('project', '').then(dashboard => { + assert.deepEqual( + dashboard, + { + title: 'title', + sections: [ + {name: 'section 1', query: 'query 1 foreach for ${project}'}, + { + name: 'section 2', + query: 'project query 2 foreach for ${project}', + }, + ], + }); + }); + }); + + test('dashboard without foreach', () => { + sandbox.stub(element.$.restAPI, 'getDashboard', () => Promise.resolve({ + title: 'title', + sections: [ + {name: 'section 1', query: 'query 1'}, + {name: 'section 2', query: '${project} query 2'}, + ], + })); + return element._getProjectDashboard('project', '').then(dashboard => { + assert.deepEqual( + dashboard, + { + title: 'title', + sections: [ + {name: 'section 1', query: 'query 1'}, + {name: 'section 2', query: 'project query 2'}, + ], + }); + }); + }); + }); + + suite('_getUserDashboard', () => { + const sections = [ + {name: 'section 1', query: 'query 1'}, + {name: 'section 2', query: 'query 2 for ${user}'}, + {name: 'section 3', query: 'self only query', selfOnly: true}, + {name: 'section 4', query: 'query 4', suffixForDashboard: 'suffix'}, + ]; + + test('dashboard for self', () => { + return element._getUserDashboard('self', sections, 'title') + .then(dashboard => { + assert.deepEqual( + dashboard, + { + title: 'title', + sections: [ + {name: 'section 1', query: 'query 1'}, + {name: 'section 2', query: 'query 2 for self'}, + {name: 'section 3', query: 'self only query'}, + { + name: 'section 4', + query: 'query 4', + suffixForDashboard: 'suffix', + }, + ], + }); + }); + }); + + test('dashboard for other user', () => { + return element._getUserDashboard('user', sections, 'title') + .then(dashboard => { + assert.deepEqual( + dashboard, + { + title: 'title', + sections: [ + {name: 'section 1', query: 'query 1'}, + {name: 'section 2', query: 'query 2 for user'}, + { + name: 'section 4', + query: 'query 4', + suffixForDashboard: 'suffix', + }, + ], + }); + }); + }); + }); + + test('_computeUserHeaderClass', () => { + assert.equal(element._computeUserHeaderClass(undefined), 'hide'); + assert.equal(element._computeUserHeaderClass(''), 'hide'); + assert.equal(element._computeUserHeaderClass('self'), 'hide'); + assert.equal(element._computeUserHeaderClass('user'), ''); + }); + + test('404 page', done => { + const response = {status: 404}; + sandbox.stub( + element.$.restAPI, 'getDashboard', (project, dashboard, errFn) => { + errFn(response); + }); + element.addEventListener('page-error', e => { + assert.strictEqual(e.detail.response, response); + done(); + }); + element.params = { + view: Gerrit.Nav.View.DASHBOARD, + project: 'project', + dashboard: 'dashboard', + }; + }); }); diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.js b/polygerrit-ui/app/elements/core/gr-router/gr-router.js index c31855a21b..fbdce6247c 100644 --- a/polygerrit-ui/app/elements/core/gr-router/gr-router.js +++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.js @@ -19,6 +19,7 @@ DASHBOARD: /^\/dashboard\/(.+)$/, CUSTOM_DASHBOARD: /^\/dashboard\/?$/, + PROJECT_DASHBOARD: /^\/p\/(.+)\/\+\/dashboard\/(.+)/, AGREEMENTS: /^\/settings\/(agreements|new-agreement)/, REGISTER: /^\/register(\/.*)?$/, @@ -308,6 +309,9 @@ } const user = params.user ? params.user : ''; return `/dashboard/${user}?${queryParams.join('&')}`; + } else if (params.project) { + // Project dashboard. + return `/p/${params.project}/+/dashboard/${params.dashboard}`; } else { // User dashboard. return `/dashboard/${params.user || 'self'}`; @@ -556,6 +560,9 @@ this._mapRoute(RoutePattern.CUSTOM_DASHBOARD, '_handleCustomDashboardRoute'); + this._mapRoute(RoutePattern.PROJECT_DASHBOARD, + '_handleProjectDashboardRoute'); + this._mapRoute(RoutePattern.GROUP_INFO, '_handleGroupInfoRoute', true); this._mapRoute(RoutePattern.GROUP_AUDIT_LOG, '_handleGroupAuditLogRoute', @@ -818,6 +825,14 @@ return Promise.resolve(); }, + _handleProjectDashboardRoute(data) { + this._setParams({ + view: Gerrit.Nav.View.DASHBOARD, + project: data.params[0], + dashboard: decodeURIComponent(data.params[1]), + }); + }, + _handleGroupInfoRoute(data) { this._redirect('/admin/groups/' + encodeURIComponent(data.params[0])); }, diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html index 2e86f73804..c7f251ec98 100644 --- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html +++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html @@ -157,6 +157,7 @@ limitations under the License. '_handleImproperlyEncodedPlusRoute', '_handlePassThroughRoute', '_handleProjectAccessRoute', + '_handleProjectDashboardRoute', '_handleProjectListFilterOffsetRoute', '_handleProjectListFilterRoute', '_handleProjectListOffsetRoute', @@ -366,6 +367,17 @@ limitations under the License. element._generateUrl(params), '/dashboard/user?name=query&title=custom%20dashboard'); }); + + test('project dashboard', () => { + const params = { + view: Gerrit.Nav.View.DASHBOARD, + project: 'gerrit/project', + dashboard: 'default:main', + }; + assert.equal( + element._generateUrl(params), + '/p/gerrit/project/+/dashboard/default:main'); + }); }); suite('groups', () => { diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js index 45def55441..4cf05fa949 100644 --- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js +++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js @@ -2037,5 +2037,20 @@ return result; }); }, + + /** + * Fetch a project dashboard definition. + * https://gerrit-review.googlesource.com/Documentation/rest-api-projects.html#get-dashboard + * @param {string} project + * @param {string} dashboard + * @param {function(?Response, string=)=} opt_errFn + * passed as null sometimes. + * @return {!Promise} + */ + getDashboard(project, dashboard, opt_errFn) { + const url = '/projects/' + encodeURIComponent(project) + '/dashboards/' + + encodeURIComponent(dashboard); + return this._fetchSharedCacheURL(url, opt_errFn); + }, }); })(); diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html index fb5d475e81..2a01032abe 100644 --- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html +++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html @@ -1172,5 +1172,14 @@ limitations under the License. }); }); }); + + test('getDashboard', () => { + const fetchStub = sandbox.stub(element, '_fetchSharedCacheURL'); + element.getDashboard('gerrit/project', 'default:main'); + assert.isTrue(fetchStub.calledOnce); + assert.equal( + fetchStub.lastCall.args[0], + '/projects/gerrit%2Fproject/dashboards/default%3Amain'); + }); }); diff --git a/polygerrit-ui/server.go b/polygerrit-ui/server.go index 79cf4bf55d..5be1c60203 100644 --- a/polygerrit-ui/server.go +++ b/polygerrit-ui/server.go @@ -200,7 +200,7 @@ type server struct{} // Any path prefixes that should resolve to index.html. var ( - fePaths = []string{"/q/", "/c/", "/dashboard/", "/admin/"} + fePaths = []string{"/q/", "/c/", "/p/", "/dashboard/", "/admin/"} issueNumRE = regexp.MustCompile(`^\/\d+\/?$`) )