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 6c5bad35e1..750db92b15 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 @@ -73,7 +73,7 @@ }, observers: [ - '_userChanged(params.user)', + '_paramsChanged(params.*)', ], behaviors: [ @@ -95,20 +95,28 @@ return 'Dashboard for ' + user; }, - /** - * Allows a refresh if menu item is selected again. - */ - _userChanged(user) { - if (!user) { return; } + _paramsChanged(paramsChangeRecord) { + const params = paramsChangeRecord.base; + + if (!params.user && !params.sections) { + return; + } + + 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. - this.async( - () => this.fire('title-change', {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 sections = this._sectionMetadata.filter( - section => (user === 'self' || !section.selfOnly)); const queries = sections.map( section => this._dashboardQueryForSection(section, user)); 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 2edf26fb8e..40376adbc0 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 @@ -64,17 +64,18 @@ limitations under the License. }); test('viewing another user\'s dashboard omits selfOnly sections', () => { - element._sectionMetadata = [ - {query: '1'}, - {query: '2', selfOnly: true}, - ]; - - element.params = {user: 'self'}; + element.params = { + sections: [ + {query: '1'}, + {query: '2', selfOnly: true}, + ], + user: 'self', + }; flushAsynchronousOperations(); assert.isTrue( getChangesStub.calledWith(null, ['1', '2'], null, element.options)); - element.params = {user: 'user'}; + element.set('params.user', 'user'); flushAsynchronousOperations(); assert.isTrue( getChangesStub.calledWith(null, ['1'], null, element.options)); 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 211e32877b..a180dba3b3 100644 --- a/polygerrit-ui/app/elements/core/gr-router/gr-router.js +++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.js @@ -127,6 +127,16 @@ */ const LINE_ADDRESS_PATTERN = /^([ab]?)(\d+)$/; + /** + * Pattern to recognize '+' in url-encoded strings for replacement with ' '. + */ + const PLUS_PATTERN = /\+/g; + + /** + * Pattern to recognize leading '?' in window.location.search, for stripping. + */ + const QUESTION_PATTERN = /^\?*/; + // Polymer makes `app` intrinsically defined on the window by virtue of the // custom element having the id "app", but it is made explicit here. const app = document.querySelector('#app'); @@ -223,7 +233,21 @@ url = `/c/${params.changeNum}${range}`; } } else if (params.view === Gerrit.Nav.View.DASHBOARD) { - url = `/dashboard/${params.user || 'self'}`; + if (params.sections) { + // Custom dashboard. + const queryParams = params.sections.map(section => { + return encodeURIComponent(section.name) + '=' + + encodeURIComponent(section.query); + }); + if (params.title) { + queryParams.push('title=' + encodeURIComponent(params.title)); + } + const user = params.user ? params.user : ''; + url = `/dashboard/${user}?${queryParams.join('&')}`; + } else { + // User dashboard. + url = `/dashboard/${params.user || 'self'}`; + } } else if (params.view === Gerrit.Nav.View.DIFF) { let range = this._getPatchRangeExpression(params); if (range.length) { range = '/' + range; } @@ -594,19 +618,101 @@ }); }, - _handleDashboardRoute(data) { - if (!data.params[0]) { - this._redirect('/dashboard/self'); - return; + /** + * Decode an application/x-www-form-urlencoded string. + * + * @param {string} qs The application/x-www-form-urlencoded string. + * @return {string} The decoded string. + */ + _decodeQueryString(qs) { + return decodeURIComponent(qs.replace(PLUS_PATTERN, ' ')); + }, + + /** + * Parse a query string (e.g. window.location.search) into an array of + * name/value pairs. + * + * @param {string} qs The application/x-www-form-urlencoded query string. + * @return {!Array>} An array of name/value pairs, where each + * element is a 2-element array. + */ + _parseQueryString(qs) { + qs = qs.replace(QUESTION_PATTERN, ''); + if (!qs) { + return []; + } + const params = []; + qs.split('&').forEach(param => { + const idx = param.indexOf('='); + let name; + let value; + if (idx < 0) { + name = this._decodeQueryString(param); + value = ''; + } else { + name = this._decodeQueryString(param.substring(0, idx)); + value = this._decodeQueryString(param.substring(idx + 1)); + } + if (name) { + params.push([name, value]); + } + }); + return params; + }, + + /** + * Handle dashboard routes. These may be user, custom, or project + * dashboards. + * + * @param {!Object} data The parsed route data. + * @param {string=} opt_qs Optional query string associated with the route. + * If not given, window.location.search is used. (Used by tests). + */ + _handleDashboardRoute(data, opt_qs) { + // opt_qs may be provided by a test, and it may have a falsy value + const qs = opt_qs !== undefined ? opt_qs : window.location.search; + const queryParams = this._parseQueryString(qs); + let title = 'Custom Dashboard'; + const titleParam = queryParams.find( + elem => elem[0].toLowerCase() === 'title'); + if (titleParam) { + title = titleParam[1]; + } + const sectionParams = queryParams.filter( + elem => elem[0] && elem[1] && elem[0].toLowerCase() !== 'title'); + const sections = sectionParams.map(elem => { + return { + name: elem[0], + query: elem[1], + }; + }); + + if (sections.length > 0) { + // Custom dashboard view. + this._setParams({ + view: Gerrit.Nav.View.DASHBOARD, + user: data.params[0] || 'self', + sections, + title, + }); + return Promise.resolve(); } + if (!data.params[0] && sections.length === 0) { + // Redirect /dashboard/ -> /dashboard/self. + this._redirect('/dashboard/self'); + return Promise.resolve(); + } + + // User dashboard. We require viewing user to be logged in, else we + // redirect to login for self dashboard or simple owner search for + // other user dashboard. return this.$.restAPI.getLoggedIn().then(loggedIn => { if (!loggedIn) { if (data.params[0].toLowerCase() === 'self') { this._redirectToLogin(data.canonicalPath); } else { - // TODO: encode user or use _generateUrl. - this._redirect('/q/owner:' + data.params[0]); + this._redirect('/q/owner:' + encodeURIComponent(data.params[0])); } } else { this._setParams({ 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 8c9f3d8f17..d1117cfc4c 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 @@ -297,6 +297,48 @@ limitations under the License. actual = element._getPatchRangeExpression(params); assert.equal(actual, '2..'); }); + + suite('dashboard', () => { + test('self dashboard', () => { + const params = { + view: Gerrit.Nav.View.DASHBOARD, + }; + assert.equal(element._generateUrl(params), '/dashboard/self'); + }); + + test('user dashboard', () => { + const params = { + view: Gerrit.Nav.View.DASHBOARD, + user: 'user', + }; + assert.equal(element._generateUrl(params), '/dashboard/user'); + }); + + test('custom self dashboard, no title', () => { + const params = { + view: Gerrit.Nav.View.DASHBOARD, + sections: [ + {name: 'section 1', query: 'query 1'}, + {name: 'section 2', query: 'query 2'}, + ], + }; + assert.equal( + element._generateUrl(params), + '/dashboard/?section%201=query%201§ion%202=query%202'); + }); + + test('custom user dashboard, with title', () => { + const params = { + view: Gerrit.Nav.View.DASHBOARD, + user: 'user', + sections: [{name: 'name', query: 'query'}], + title: 'custom dashboard', + }; + assert.equal( + element._generateUrl(params), + '/dashboard/user?name=query&title=custom%20dashboard'); + }); + }); }); suite('param normalization', () => { @@ -514,7 +556,7 @@ limitations under the License. assert.isFalse(redirectStub.called); }); - test('redirects to dahsboard if logged in', () => { + test('redirects to dashboard if logged in', () => { sandbox.stub(element.$.restAPI, 'getLoggedIn') .returns(Promise.resolve(true)); const data = { @@ -625,35 +667,31 @@ limitations under the License. }); test('no user specified', () => { - const data = {canonicalPath: '/dashboard', params: {}}; - const result = element._handleDashboardRoute(data); - assert.isNotOk(result); - assert.isFalse(setParamsStub.called); - assert.isFalse(redirectToLoginStub.called); - assert.isTrue(redirectStub.called); - assert.equal(redirectStub.lastCall.args[0], '/dashboard/self'); + const data = {canonicalPath: '/dashboard/', params: {0: ''}}; + return element._handleDashboardRoute(data, '').then(() => { + assert.isFalse(setParamsStub.called); + assert.isFalse(redirectToLoginStub.called); + assert.isTrue(redirectStub.called); + assert.equal(redirectStub.lastCall.args[0], '/dashboard/self'); + }); }); - test('own dahsboard but signed out redirects to login', () => { + test('own dashboard but signed out redirects to login', () => { sandbox.stub(element.$.restAPI, 'getLoggedIn') .returns(Promise.resolve(false)); - const data = {canonicalPath: '/dashboard', params: {0: 'seLF'}}; - const result = element._handleDashboardRoute(data); - assert.isOk(result); - return result.then(() => { + const data = {canonicalPath: '/dashboard/', params: {0: 'seLF'}}; + return element._handleDashboardRoute(data, '').then(() => { assert.isTrue(redirectToLoginStub.calledOnce); assert.isFalse(redirectStub.called); assert.isFalse(setParamsStub.called); }); }); - test('non-self dahsboard but signed out does not redirect', () => { + test('non-self dashboard but signed out does not redirect', () => { sandbox.stub(element.$.restAPI, 'getLoggedIn') .returns(Promise.resolve(false)); - const data = {canonicalPath: '/dashboard', params: {0: 'foo'}}; - const result = element._handleDashboardRoute(data); - assert.isOk(result); - return result.then(() => { + const data = {canonicalPath: '/dashboard/', params: {0: 'foo'}}; + return element._handleDashboardRoute(data, '').then(() => { assert.isFalse(redirectToLoginStub.called); assert.isFalse(setParamsStub.called); assert.isTrue(redirectStub.calledOnce); @@ -661,13 +699,11 @@ limitations under the License. }); }); - test('dahsboard while signed in sets params', () => { + test('dashboard while signed in sets params', () => { sandbox.stub(element.$.restAPI, 'getLoggedIn') .returns(Promise.resolve(true)); - const data = {canonicalPath: '/dashboard', params: {0: 'foo'}}; - const result = element._handleDashboardRoute(data); - assert.isOk(result); - return result.then(() => { + const data = {canonicalPath: '/dashboard/', params: {0: 'foo'}}; + return element._handleDashboardRoute(data, '').then(() => { assert.isFalse(redirectToLoginStub.called); assert.isFalse(redirectStub.called); assert.isTrue(setParamsStub.calledOnce); @@ -677,6 +713,42 @@ limitations under the License. }); }); }); + + test('custom dashboard without title', () => { + const data = {canonicalPath: '/dashboard/', params: {0: ''}}; + return element._handleDashboardRoute(data, '?a=b&c&d=e').then(() => { + assert.isFalse(redirectToLoginStub.called); + assert.isFalse(redirectStub.called); + assert.isTrue(setParamsStub.calledOnce); + assert.deepEqual(setParamsStub.lastCall.args[0], { + view: Gerrit.Nav.View.DASHBOARD, + user: 'self', + sections: [ + {name: 'a', query: 'b'}, + {name: 'd', query: 'e'}, + ], + title: 'Custom Dashboard', + }); + }); + }); + + test('custom dashboard with title', () => { + const data = {canonicalPath: '/dashboard/', params: {0: ''}}; + return element._handleDashboardRoute(data, '?a=b&c&d=&=e&title=t') + .then(() => { + assert.isFalse(redirectToLoginStub.called); + assert.isFalse(redirectStub.called); + assert.isTrue(setParamsStub.calledOnce); + assert.deepEqual(setParamsStub.lastCall.args[0], { + view: Gerrit.Nav.View.DASHBOARD, + user: 'self', + sections: [ + {name: 'a', query: 'b'}, + ], + title: 't', + }); + }); + }); }); suite('group routes', () => { @@ -1177,5 +1249,31 @@ limitations under the License. }); }); }); + + suite('_parseQueryString', () => { + test('empty queries', () => { + assert.deepEqual(element._parseQueryString(''), []); + assert.deepEqual(element._parseQueryString('?'), []); + assert.deepEqual(element._parseQueryString('??'), []); + assert.deepEqual(element._parseQueryString('&&&'), []); + }); + + test('url decoding', () => { + assert.deepEqual(element._parseQueryString('+'), [[' ', '']]); + assert.deepEqual(element._parseQueryString('???+%3d+'), [[' = ', '']]); + assert.deepEqual( + element._parseQueryString('%6e%61%6d%65=%76%61%6c%75%65'), + [['name', 'value']]); + }); + + test('multiple parameters', () => { + assert.deepEqual( + element._parseQueryString('a=b&c=d&e=f'), + [['a', 'b'], ['c', 'd'], ['e', 'f']]); + assert.deepEqual( + element._parseQueryString('&a=b&&&e=f&'), + [['a', 'b'], ['e', 'f']]); + }); + }); }); 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 4e0f3b7235..20cc7eac46 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 @@ -753,6 +753,11 @@ // Response may be an array of changes OR an array of arrays of // changes. if (opt_query instanceof Array) { + // Normalize the response to look like a multi-query response + // when there is only one query. + if (opt_query.length === 1) { + response = [response]; + } for (const arr of response) { iterateOverChanges(arr); }