Commit 1bfd85ee authored by Lyza Danger Gardner's avatar Lyza Danger Gardner Committed by Lyza Gardner

Rename references, module and functions related to config and settings

As foundational step for extending some configuration and settings
objects, add some clarification to differentiate between "configuration"
and "settings" in the sidebar, and rename the `fetch-config` module
to `build-settings` to better reflect its responsibilities.

Part of https://github.com/hypothesis/lms/issues/3647
parent 156ce129
...@@ -107,17 +107,19 @@ async function fetchGroupsAsync(config, rpcCall) { ...@@ -107,17 +107,19 @@ async function fetchGroupsAsync(config, rpcCall) {
} }
/** /**
* Fetch the host configuration and merge it with the app configuration from h. * Build a `SidebarSettings` object by merging the provided `ConfigFromSidebar`
* with `ConfigFromHost` from an appropriate source.
* *
* There are 2 ways to get the host config: * `ConfigFromHost` may come from either:
* Direct embed - From the hash string of the embedder frame. * - The URL hash of the iframe, written by the annotator when creating the
* RPC request to indicated parent frame * sidebar's iframe, OR
* - By sending an RPC request for host configuration to a known ancestor frame
* *
* @param {ConfigFromSidebar} appConfig * @param {ConfigFromSidebar} appConfig
* @param {Window} window_ - Test seam. * @param {Window} window_ - Test seam.
* @return {Promise<SidebarSettings>} - The merged settings. * @return {Promise<SidebarSettings>} - The merged settings.
*/ */
export async function fetchConfig(appConfig, window_ = window) { export async function buildSettings(appConfig, window_ = window) {
const hostConfig = hostPageConfig(window); const hostConfig = hostPageConfig(window);
const requestConfigFromFrame = hostConfig.requestConfigFromFrame; const requestConfigFromFrame = hostConfig.requestConfigFromFrame;
......
import { fetchConfig, $imports } from '../fetch-config'; import { buildSettings, $imports } from '../build-settings';
describe('sidebar/config/fetch-config', () => { describe('sidebar/config/build-settings', () => {
let fakeHostPageConfig; let fakeHostPageConfig;
let fakeJsonRpc; let fakeJsonRpc;
let fakeWindow; let fakeWindow;
...@@ -39,20 +39,20 @@ describe('sidebar/config/fetch-config', () => { ...@@ -39,20 +39,20 @@ describe('sidebar/config/fetch-config', () => {
$imports.$restore(); $imports.$restore();
}); });
describe('config/fetch-config', () => { describe('config/build-settings', () => {
context('direct embed', () => { context('direct embed', () => {
// no `requestConfigFromFrame` variable // no `requestConfigFromFrame` variable
// //
// Combine the settings rendered into the sidebar's HTML page // Combine the settings rendered into the sidebar's HTML page
// by h with the settings from `window.hypothesisConfig` in the parent // by h with the settings from `window.hypothesisConfig` in the parent
// window. // window.
it('adds the apiUrl to the merged result', async () => { it('adds the apiUrl to the merged settings object', async () => {
const sidebarSettings = await fetchConfig({}); const sidebarSettings = await buildSettings({});
assert.deepEqual(sidebarSettings, { apiUrl: fakeApiUrl() }); assert.deepEqual(sidebarSettings, { apiUrl: fakeApiUrl() });
}); });
it('does not fetch settings from ancestor frames', async () => { it('does not fetch host configuration from ancestor frames', async () => {
await fetchConfig({}); await buildSettings({});
assert.notCalled(fakeJsonRpc.call); assert.notCalled(fakeJsonRpc.call);
}); });
...@@ -60,7 +60,7 @@ describe('sidebar/config/fetch-config', () => { ...@@ -60,7 +60,7 @@ describe('sidebar/config/fetch-config', () => {
// hostPageConfig shall take precedent over appConfig // hostPageConfig shall take precedent over appConfig
const appConfig = { foo: 'bar', appType: 'via' }; const appConfig = { foo: 'bar', appType: 'via' };
fakeHostPageConfig.returns({ foo: 'baz' }); fakeHostPageConfig.returns({ foo: 'baz' });
const sidebarSettings = await fetchConfig(appConfig); const sidebarSettings = await buildSettings(appConfig);
assert.deepEqual(sidebarSettings, { assert.deepEqual(sidebarSettings, {
foo: 'baz', foo: 'baz',
appType: 'via', appType: 'via',
...@@ -87,7 +87,7 @@ describe('sidebar/config/fetch-config', () => { ...@@ -87,7 +87,7 @@ describe('sidebar/config/fetch-config', () => {
}); });
it('makes an RPC request to `requestConfig` ', async () => { it('makes an RPC request to `requestConfig` ', async () => {
await fetchConfig({}, fakeWindow); await buildSettings({}, fakeWindow);
assert.isTrue( assert.isTrue(
fakeJsonRpc.call.calledWithExactly( fakeJsonRpc.call.calledWithExactly(
fakeTopWindow, fakeTopWindow,
...@@ -107,7 +107,7 @@ describe('sidebar/config/fetch-config', () => { ...@@ -107,7 +107,7 @@ describe('sidebar/config/fetch-config', () => {
ancestorLevel: level, ancestorLevel: level,
}, },
}); });
await fetchConfig({}, fakeWindow); await buildSettings({}, fakeWindow);
// testId is a fake property used to assert the level of the fake window // testId is a fake property used to assert the level of the fake window
assert.equal(fakeJsonRpc.call.getCall(0).args[0].testId, level); assert.equal(fakeJsonRpc.call.getCall(0).args[0].testId, level);
}); });
...@@ -121,15 +121,15 @@ describe('sidebar/config/fetch-config', () => { ...@@ -121,15 +121,15 @@ describe('sidebar/config/fetch-config', () => {
}, },
}); });
await assert.rejects( await assert.rejects(
fetchConfig({}, fakeWindow), buildSettings({}, fakeWindow),
/The target parent frame has exceeded the ancestor tree|Try reducing the/g /The target parent frame has exceeded the ancestor tree|Try reducing the/g
); );
}); });
it('creates a merged config when the RPC requests returns the host config', async () => { it('creates merged settings when the RPC requests returns the host config', async () => {
const appConfig = { foo: 'bar', appType: 'via' }; const appConfig = { foo: 'bar', appType: 'via' };
fakeJsonRpc.call.resolves({ foo: 'baz' }); // host config fakeJsonRpc.call.resolves({ foo: 'baz' }); // host config
const result = await fetchConfig(appConfig, fakeWindow); const result = await buildSettings(appConfig, fakeWindow);
assert.deepEqual(result, { assert.deepEqual(result, {
foo: 'baz', foo: 'baz',
appType: 'via', appType: 'via',
...@@ -137,10 +137,10 @@ describe('sidebar/config/fetch-config', () => { ...@@ -137,10 +137,10 @@ describe('sidebar/config/fetch-config', () => {
}); });
}); });
it('rejects if fetching config fails` ', async () => { it('rejects if fetching host config fails` ', async () => {
fakeJsonRpc.call.rejects(new Error('Nope')); fakeJsonRpc.call.rejects(new Error('Nope'));
const appConfig = { foo: 'bar', appType: 'via' }; const appConfig = { foo: 'bar', appType: 'via' };
await assert.rejects(fetchConfig(appConfig, fakeWindow), 'Nope'); await assert.rejects(buildSettings(appConfig, fakeWindow), 'Nope');
}); });
it('returns the `groups` array with the initial host config request', async () => { it('returns the `groups` array with the initial host config request', async () => {
...@@ -149,18 +149,18 @@ describe('sidebar/config/fetch-config', () => { ...@@ -149,18 +149,18 @@ describe('sidebar/config/fetch-config', () => {
appType: 'via', appType: 'via',
}; };
fakeJsonRpc.call.onFirstCall().resolves({ foo: 'baz' }); // host config fakeJsonRpc.call.onFirstCall().resolves({ foo: 'baz' }); // host config
const result = await fetchConfig(appConfig, fakeWindow); const result = await buildSettings(appConfig, fakeWindow);
assert.deepEqual(result.services[0].groups, ['group1', 'group2']); assert.deepEqual(result.services[0].groups, ['group1', 'group2']);
}); });
it("creates a merged config where `groups` is a promise when its initial value is '$rpc:requestGroups'", async () => { it("creates merged settings where `groups` is a promise when its initial value is '$rpc:requestGroups'", async () => {
const appConfig = { const appConfig = {
services: [{ groups: '$rpc:requestGroups' }], services: [{ groups: '$rpc:requestGroups' }],
appType: 'via', appType: 'via',
}; };
fakeJsonRpc.call.onFirstCall().resolves({ foo: 'baz' }); // host config fakeJsonRpc.call.onFirstCall().resolves({ foo: 'baz' }); // host config
fakeJsonRpc.call.onSecondCall().resolves(['group1', 'group2']); // requestGroups fakeJsonRpc.call.onSecondCall().resolves(['group1', 'group2']); // requestGroups
const result = await fetchConfig(appConfig, fakeWindow); const result = await buildSettings(appConfig, fakeWindow);
assert.deepEqual(await result.services[0].groups, ['group1', 'group2']); assert.deepEqual(await result.services[0].groups, ['group1', 'group2']);
assert.isTrue( assert.isTrue(
fakeJsonRpc.call.getCall(1).calledWithExactly( fakeJsonRpc.call.getCall(1).calledWithExactly(
...@@ -180,14 +180,14 @@ describe('sidebar/config/fetch-config', () => { ...@@ -180,14 +180,14 @@ describe('sidebar/config/fetch-config', () => {
}; };
fakeJsonRpc.call.onFirstCall().resolves({ foo: 'baz' }); // host config fakeJsonRpc.call.onFirstCall().resolves({ foo: 'baz' }); // host config
fakeJsonRpc.call.onSecondCall().rejects(); // requestGroups fakeJsonRpc.call.onSecondCall().rejects(); // requestGroups
const result = await fetchConfig(appConfig, fakeWindow); const result = await buildSettings(appConfig, fakeWindow);
await assert.rejects( await assert.rejects(
result.services[0].groups, result.services[0].groups,
'Unable to fetch groups' 'Unable to fetch groups'
); );
}); });
it('creates a merged config and also adds back the `group` value from the host config', async () => { it('creates merged settings and also adds back the `group` value from the host config', async () => {
fakeHostPageConfig.returns({ fakeHostPageConfig.returns({
requestConfigFromFrame: { requestConfigFromFrame: {
origin: 'https://embedder.com', origin: 'https://embedder.com',
...@@ -198,7 +198,7 @@ describe('sidebar/config/fetch-config', () => { ...@@ -198,7 +198,7 @@ describe('sidebar/config/fetch-config', () => {
const appConfig = { foo: 'bar', appType: 'via' }; const appConfig = { foo: 'bar', appType: 'via' };
fakeJsonRpc.call.resolves({ foo: 'baz' }); fakeJsonRpc.call.resolves({ foo: 'baz' });
const result = await fetchConfig(appConfig, fakeWindow); const result = await buildSettings(appConfig, fakeWindow);
assert.deepEqual(result, { assert.deepEqual(result, {
foo: 'baz', foo: 'baz',
...@@ -222,7 +222,7 @@ describe('sidebar/config/fetch-config', () => { ...@@ -222,7 +222,7 @@ describe('sidebar/config/fetch-config', () => {
}, },
}); });
await assert.rejects( await assert.rejects(
fetchConfig({}, fakeWindow), buildSettings({}, fakeWindow),
'Improper `requestConfigFromFrame` object. Both `ancestorLevel` and `origin` need to be specified' 'Improper `requestConfigFromFrame` object. Both `ancestorLevel` and `origin` need to be specified'
); );
}); });
...@@ -235,7 +235,7 @@ describe('sidebar/config/fetch-config', () => { ...@@ -235,7 +235,7 @@ describe('sidebar/config/fetch-config', () => {
}, },
}); });
await assert.rejects( await assert.rejects(
fetchConfig({}, fakeWindow), buildSettings({}, fakeWindow),
'Improper `requestConfigFromFrame` object. Both `ancestorLevel` and `origin` need to be specified' 'Improper `requestConfigFromFrame` object. Both `ancestorLevel` and `origin` need to be specified'
); );
}); });
......
...@@ -2,7 +2,7 @@ import { parseJsonConfig } from '../boot/parse-json-config'; ...@@ -2,7 +2,7 @@ import { parseJsonConfig } from '../boot/parse-json-config';
import * as rendererOptions from '../shared/renderer-options'; import * as rendererOptions from '../shared/renderer-options';
import { checkEnvironment } from './config/check-env'; import { checkEnvironment } from './config/check-env';
import { fetchConfig } from './config/fetch-config'; import { buildSettings } from './config/build-settings';
import { import {
startServer as startRPCServer, startServer as startRPCServer,
preStartServer as preStartRPCServer, preStartServer as preStartRPCServer,
...@@ -11,9 +11,10 @@ import { disableOpenerForExternalLinks } from './util/disable-opener-for-externa ...@@ -11,9 +11,10 @@ import { disableOpenerForExternalLinks } from './util/disable-opener-for-externa
import * as sentry from './util/sentry'; import * as sentry from './util/sentry';
// Read settings rendered into sidebar app HTML by service/extension. // Read settings rendered into sidebar app HTML by service/extension.
const appConfig = /** @type {import('../types/config').ConfigFromSidebar} */ ( const configFromSidebar =
/** @type {import('../types/config').ConfigFromSidebar} */ (
parseJsonConfig(document) parseJsonConfig(document)
); );
// Check for known issues which may prevent the client from working. // Check for known issues which may prevent the client from working.
// //
...@@ -21,10 +22,10 @@ const appConfig = /** @type {import('../types/config').ConfigFromSidebar} */ ( ...@@ -21,10 +22,10 @@ const appConfig = /** @type {import('../types/config').ConfigFromSidebar} */ (
// and continue anyway. // and continue anyway.
const envOk = checkEnvironment(window); const envOk = checkEnvironment(window);
if (appConfig.sentry && envOk) { if (configFromSidebar.sentry && envOk) {
// Initialize Sentry. This is required at the top of this file // Initialize Sentry. This is required at the top of this file
// so that it happens early in the app's startup flow // so that it happens early in the app's startup flow
sentry.init(appConfig.sentry); sentry.init(configFromSidebar.sentry);
} }
// Prevent tab-jacking. // Prevent tab-jacking.
...@@ -140,10 +141,10 @@ import { Injector } from '../shared/injector'; ...@@ -140,10 +141,10 @@ import { Injector } from '../shared/injector';
/** /**
* Launch the client application corresponding to the current URL. * Launch the client application corresponding to the current URL.
* *
* @param {object} config * @param {import('../types/config').SidebarSettings} settings
* @param {HTMLElement} appEl - Root HTML container for the app * @param {HTMLElement} appEl - Root HTML container for the app
*/ */
function startApp(config, appEl) { function startApp(settings, appEl) {
const container = new Injector(); const container = new Injector();
// Register services. // Register services.
...@@ -175,7 +176,7 @@ function startApp(config, appEl) { ...@@ -175,7 +176,7 @@ function startApp(config, appEl) {
// that use them, since they don't depend on instances of other services. // that use them, since they don't depend on instances of other services.
container container
.register('$window', { value: window }) .register('$window', { value: window })
.register('settings', { value: config }); .register('settings', { value: settings });
// Initialize services. // Initialize services.
container.run(initServices); container.run(initServices);
...@@ -214,8 +215,8 @@ const appEl = /** @type {HTMLElement} */ ( ...@@ -214,8 +215,8 @@ const appEl = /** @type {HTMLElement} */ (
// Start capturing RPC requests before we start the RPC server (startRPCServer) // Start capturing RPC requests before we start the RPC server (startRPCServer)
preStartRPCServer(); preStartRPCServer();
fetchConfig(appConfig) buildSettings(configFromSidebar)
.then(config => { .then(settings => {
startApp(config, appEl); startApp(settings, appEl);
}) })
.catch(err => reportLaunchError(err, appEl)); .catch(err => reportLaunchError(err, appEl));
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment