opendev/gerrit
Code Issues Proposed changes

Pass config into plugin loader correctly

Browse Source 
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
This commit is contained in:
Viktar Donich
2017-08-10 16:02:48 -07:00
parent c85a1db488
commit ffe8864542
7 changed files with 50 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata-it_test.html
View File

@@ -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');

10
polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.html
View File

@@ -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);

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

@@ -206,7 +206,7 @@ limitations under the License.
<gr-reporting id="reporting"></gr-reporting>
<gr-router id="router"></gr-router>
<gr-plugin-host id="plugins"
config="[[_serverConfig.plugin]]">
config="[[_serverConfig]]">
</gr-plugin-host>
<gr-external-style id="externalStyle" name="app-theme"></gr-external-style>
</template>

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

@@ -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);
});
});
});

5
polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.js
View File

@@ -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.

47
polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.html
View File

@@ -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));

8
polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js
View File

@@ -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');