From ffe88645424e4b34e170acd9e0bee29feedcfbd3 Mon Sep 17 00:00:00 2001 From: Viktar Donich Date: Thu, 10 Aug 2017 16:02:48 -0700 Subject: [PATCH] Pass config into plugin loader correctly Plugin section that includes .js and .html plugins was passed instead of the entire config. That is required to correctly recognize default site theme. Also fixes plugin name handling for site theme served from /static/gerrit-theme.html. Change-Id: I27ecc9f8698cb1ea359971ebeb6b14788fef51fd --- .../gr-change-metadata-it_test.html | 10 ++-- .../gr-reply-dialog-it_test.html | 10 ++-- polygerrit-ui/app/elements/gr-app.html | 2 +- polygerrit-ui/app/elements/gr-app_test.html | 10 ++-- .../plugins/gr-plugin-host/gr-plugin-host.js | 5 +- .../gr-plugin-host/gr-plugin-host_test.html | 47 ++++++++++--------- .../gr-js-api-interface/gr-public-js-api.js | 8 +++- 7 files changed, 50 insertions(+), 42 deletions(-) diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata-it_test.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata-it_test.html index b3478906ac..6129e9bcfa 100644 --- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata-it_test.html +++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata-it_test.html @@ -90,10 +90,12 @@ limitations under the License. setup(done => { const pluginHost = fixture('plugin-host'); pluginHost.config = { - js_resource_paths: [], - html_resource_paths: [ - new URL('test/plugin.html', window.location.href).toString(), - ], + plugin: { + js_resource_paths: [], + html_resource_paths: [ + new URL('test/plugin.html', window.location.href).toString(), + ], + }, }; element = fixture('element'); const importSpy = sandbox.spy(element.$.externalStyle, '_import'); diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.html index e76a9e5fa7..babd95c0fb 100644 --- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.html +++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.html @@ -132,10 +132,12 @@ limitations under the License. test('lgtm plugin', done => { const pluginHost = fixture('plugin-host'); pluginHost.config = { - js_resource_paths: [], - html_resource_paths: [ - new URL('test/plugin.html', window.location.href).toString(), - ], + plugin: { + js_resource_paths: [], + html_resource_paths: [ + new URL('test/plugin.html', window.location.href).toString(), + ], + }, }; element = fixture('basic'); setupElement(element); diff --git a/polygerrit-ui/app/elements/gr-app.html b/polygerrit-ui/app/elements/gr-app.html index 07ecb8d96c..defbe8a405 100644 --- a/polygerrit-ui/app/elements/gr-app.html +++ b/polygerrit-ui/app/elements/gr-app.html @@ -206,7 +206,7 @@ limitations under the License. + config="[[_serverConfig]]"> diff --git a/polygerrit-ui/app/elements/gr-app_test.html b/polygerrit-ui/app/elements/gr-app_test.html index 905c5c4523..3712ffa169 100644 --- a/polygerrit-ui/app/elements/gr-app_test.html +++ b/polygerrit-ui/app/elements/gr-app_test.html @@ -50,7 +50,7 @@ limitations under the License. getConfig() { return Promise.resolve({ gerrit: {web_uis: ['GWT', 'POLYGERRIT']}, - plugin: {js_resource_paths: []}, + plugin: {}, }); }, getPreferences() { return Promise.resolve({my: []}); }, @@ -100,11 +100,9 @@ limitations under the License. }); }); - test('passes config to gr-plugin-host', done => { - element.$.restAPI.getConfig.lastCall.returnValue.then(config => { - const pluginConfig = config.plugin; - assert.deepEqual(element.$.plugins.config, pluginConfig); - done(); + test('passes config to gr-plugin-host', () => { + return element.$.restAPI.getConfig.lastCall.returnValue.then(config => { + assert.deepEqual(element.$.plugins.config, config); }); }); }); diff --git a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.js b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.js index 03b6d56ff1..1ce86200fa 100644 --- a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.js +++ b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.js @@ -29,8 +29,9 @@ ], _configChanged(config) { - const jsPlugins = config.js_resource_paths || []; - const htmlPlugins = config.html_resource_paths || []; + const plugins = config.plugin; + const jsPlugins = plugins.js_resource_paths || []; + const htmlPlugins = plugins.html_resource_paths || []; const defaultTheme = config.default_theme; if (defaultTheme) { // Make theme first to be first to load. diff --git a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.html b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.html index 2cacede7d4..27adbe175b 100644 --- a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.html +++ b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.html @@ -48,15 +48,17 @@ limitations under the License. test('counts plugins', () => { sandbox.stub(Gerrit, '_setPluginsCount'); element.config = { - html_resource_paths: ['foo/bar', 'baz'], - js_resource_paths: ['42'], + plugin: { + html_resource_paths: ['foo/bar', 'baz'], + js_resource_paths: ['42'], + }, }; assert.isTrue(Gerrit._setPluginsCount.calledWith(3)); }); test('imports relative html plugins from config', () => { element.config = { - html_resource_paths: ['foo/bar', 'baz'], + plugin: {html_resource_paths: ['foo/bar', 'baz']}, }; assert.isTrue(element.importHref.calledWith( '/foo/bar', Gerrit._pluginInstalled, Gerrit._pluginInstalled, true)); @@ -67,8 +69,7 @@ limitations under the License. test('imports relative html plugins from config with a base url', () => { sandbox.stub(element, 'getBaseUrl').returns('/the-base'); element.config = { - html_resource_paths: ['foo/bar', 'baz'], - }; + plugin: {html_resource_paths: ['foo/bar', 'baz']}}; assert.isTrue(element.importHref.calledWith( '/the-base/foo/bar', Gerrit._pluginInstalled, Gerrit._pluginInstalled, true)); @@ -79,10 +80,12 @@ limitations under the License. test('imports absolute html plugins from config', () => { element.config = { - html_resource_paths: [ - 'http://example.com/foo/bar', - 'https://example.com/baz', - ], + plugin: { + html_resource_paths: [ + 'http://example.com/foo/bar', + 'https://example.com/baz', + ], + }, }; assert.isTrue(element.importHref.calledWith( 'http://example.com/foo/bar', Gerrit._pluginInstalled, @@ -93,17 +96,13 @@ limitations under the License. }); test('adds js plugins from config to the body', () => { - element.config = { - js_resource_paths: ['foo/bar', 'baz'], - }; + element.config = {plugin: {js_resource_paths: ['foo/bar', 'baz']}}; assert.isTrue(document.body.appendChild.calledTwice); }); test('imports relative js plugins from config', () => { sandbox.stub(element, '_createScriptTag'); - element.config = { - js_resource_paths: ['foo/bar', 'baz'], - }; + element.config = {plugin: {js_resource_paths: ['foo/bar', 'baz']}}; assert.isTrue(element._createScriptTag.calledWith('/foo/bar')); assert.isTrue(element._createScriptTag.calledWith('/baz')); }); @@ -111,9 +110,7 @@ limitations under the License. test('imports relative html plugins from config with a base url', () => { sandbox.stub(element, '_createScriptTag'); sandbox.stub(element, 'getBaseUrl').returns('/the-base'); - element.config = { - js_resource_paths: ['foo/bar', 'baz'], - }; + element.config = {plugin: {js_resource_paths: ['foo/bar', 'baz']}}; assert.isTrue(element._createScriptTag.calledWith('/the-base/foo/bar')); assert.isTrue(element._createScriptTag.calledWith('/the-base/baz')); }); @@ -121,10 +118,12 @@ limitations under the License. test('imports absolute html plugins from config', () => { sandbox.stub(element, '_createScriptTag'); element.config = { - js_resource_paths: [ - 'http://example.com/foo/bar', - 'https://example.com/baz', - ], + plugin: { + js_resource_paths: [ + 'http://example.com/foo/bar', + 'https://example.com/baz', + ], + }, }; assert.isTrue(element._createScriptTag.calledWith( 'http://example.com/foo/bar')); @@ -135,7 +134,9 @@ limitations under the License. test('default theme is loaded with html plugins', () => { element.config = { default_theme: '/oof', - html_resource_paths: ['some'], + plugin: { + html_resource_paths: ['some'], + }, }; assert.isTrue(element.importHref.calledWith( '/oof', Gerrit._pluginInstalled, Gerrit._pluginInstalled, true)); diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js index a631c2faa0..d1b94171a5 100644 --- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js +++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js @@ -67,12 +67,16 @@ const base = Gerrit.BaseUrlBehavior.getBaseUrl(); this._url = new URL(opt_url); - if (!this._url.pathname.startsWith(base + '/plugins')) { + const pathname = this._url.pathname.replace(base, ''); + // Site theme is server from predefined path. + if (pathname === '/static/gerrit-theme.html') { + this._name = 'gerrit-theme'; + } else if (!pathname.startsWith('/plugins')) { console.warn('Plugin not being loaded from /plugins base path:', this._url.href, '— Unable to determine name.'); return; } - this._name = this._url.pathname.replace(base, '').split('/')[2]; + this._name = pathname.split('/')[2]; } Plugin._sharedAPIElement = document.createElement('gr-js-api-interface');