Skip to content

Commit

Permalink
Remove TODOs, complete test suite for templates
Browse files Browse the repository at this point in the history
  • Loading branch information
kennethbruskiewicz committed Nov 7, 2023
1 parent 455a109 commit 2e9c7f5
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 35 deletions.
7 changes: 4 additions & 3 deletions lib/DataHarmonizer.js
Original file line number Diff line number Diff line change
Expand Up @@ -637,9 +637,9 @@ class DataHarmonizer {
'This template had no sections and fields: ' + this.template_name
);
}
// hooks, since passing them as callbacks would result in recurisve dependencies
this.hot
.addHook('afterChange', function (changes, source) {
if (!!this.hot && typeof this.hot.addHook !== "undefined") {
// hooks, since passing them as callbacks would result in recurisve dependencies
this.hot.addHook('afterChange', function (changes, source) {
if (source !== 'loadData') {
// Not triggered by the initial load
for (let i = 0; i < changes.length; i++) {
Expand All @@ -656,6 +656,7 @@ class DataHarmonizer {
}
})
.bind(self);
}
}

addRowsToBottom(numRows) {
Expand Down
2 changes: 1 addition & 1 deletion lib/utils/files.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export async function fetchFileAsync(filePath) {

return await response.text();
} catch (error) {
console.error(`Failed to load ${filePath}`);
console.error(`Failed to load ${filePath}: ${error}`);
throw error;
}
}
Expand Down
7 changes: 4 additions & 3 deletions lib/utils/i18n.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,9 @@ export const resources = {
// ... other languages
};

Object.entries(resources).forEach(([lang, { translation }]) => {
i18next.addResources(lang, 'translation', translation);
});
// TODO
// Object.entries(resources).forEach(([lang, { translation }]) => {
// i18next.addResource(lang, 'translation', translation);
// });

export { transformLangFirstSpec, transformStructFirstSpec };
32 changes: 20 additions & 12 deletions lib/utils/templates.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ const isSchema = (el) => el.name === 'schema.json';
const isDocumentation = (el) => el.extension === '.md';
const isLocale = (el) => el.name === 'locales';

export async function accessTemplate(template_name) {
export async function accessTemplate(template_name, fetchFileImpl=fetchFileAsync) {
const template = template_manifest.children.find(
(el) => el.name === template_name
);
return template ? processNode(template) : null;
return template ? processNode(template, fetchFileImpl) : null;
}

async function processNode(node) {
async function processNode(node, fetchFileImpl=fetchFileAsync) {
const { name, children } = node;

// Base case
Expand All @@ -28,19 +28,19 @@ async function processNode(node) {
await Promise.all(
children.map(async (child) => {
if (isSchema(child)) {
let schemaData = await fetchFileAsync(child.path);
let schemaData = await fetchFileImpl(child.path);
schema.push(schemaData);
} else if (isDocumentation(child)) {
let docData = await fetchFileAsync(child.path);
let docData = await fetchFileImpl(child.path);
documentation.push(docData);
} else if (isLocale(child)) {
const processedLocale = await processNode(child);
const processedLocale = await processNode(child, fetchFileImpl);
if (processedLocale.length) {
locales.push(processedLocale);
}
} else {
// Handle other generic nodes recursively
const processedChild = await processNode(child);
const processedChild = await processNode(child, fetchFileImpl);
if (processedChild.length) {
locales.push(processedChild);
}
Expand Down Expand Up @@ -188,12 +188,13 @@ class TemplateProxy {
* @returns {Proxy} - Returns a Proxy instance that serves localized or default data.
*/

static async create(template_name, locale) {
static async create(template_name, { locale = null, overrides = {} }) {

const instance = new TemplateProxy();
await instance.init(template_name, locale);
await instance._init(template_name, locale, overrides);

// NOTE: Why Proxy? it allows us to call all of the template's properties in an "inherited" fashion
return new Proxy(instance, {
const proxy = new Proxy(instance, {
// NOTE: rather than getters in the outside object, pass them through here
get: function (target, property) {
if (property === 'updateLocale') {
Expand All @@ -214,6 +215,9 @@ class TemplateProxy {
return target.current.bind(target)()[property];
},
});
// initiate with a given locale
if (locale !== null) proxy.updateLocale(locale)
return proxy;
}

/**
Expand All @@ -224,8 +228,11 @@ class TemplateProxy {
*
* @private
*/
async init(template_name, locale = null) {
const template = buildTemplate(await accessTemplate(template_name));
async _init(template_name, locale = null, overrides = {}) {

// required for testability
const fetchFileImpl = !!overrides.fetchFileImpl ? overrides.fetchFileImpl : fetchFileAsync;
const template = buildTemplate(await accessTemplate(template_name, fetchFileImpl));

Check failure on line 235 in lib/utils/templates.js

View workflow job for this annotation

GitHub Actions / test

Redundant double negation

Object.assign(this, {
_name: template_name,
Expand All @@ -236,6 +243,7 @@ class TemplateProxy {
default: template.default,
localized: locale ? template.locales[locale] : null,
});

}

current() {
Expand Down
10 changes: 5 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@
"@rollup/plugin-image": "^2.1.1",
"@rollup/plugin-json": "^4.1.0",
"@rollup/plugin-node-resolve": "^13.3.0",
"rollup-plugin-polyfill-node": "^0.12.0",
"os-browserify": "^0.3.0",
"bootstrap": "4.3.1",
"copy-webpack-plugin": "^11.0.0",
"css-loader": "^6.7.1",
Expand All @@ -61,11 +59,13 @@
"jest": "^28.1.3",
"jquery": "3.5.1",
"node-libs-browser": "^2.2.1",
"os-browserify": "^0.3.0",
"popper.js": "^1.16.1",
"prettier": "2.7.0",
"rimraf": "^3.0.2",
"rollup": "^2.75.6",
"rollup-jest": "^3.0.0",
"rollup-plugin-polyfill-node": "^0.12.0",
"rollup-plugin-string": "^3.0.0",
"rollup-plugin-styles": "^4.0.0",
"rollup-plugin-terser": "^7.0.2",
Expand All @@ -82,13 +82,13 @@
"file-saver": "^2.0.5",
"flatpickr": "^4.6.13",
"handsontable": "13.1.0",
"i18next": "^23.6.0",
"jquery-i18next": "^1.2.1",
"language-tags": "^1.0.9",
"linkify-it": "^4.0.1",
"markdown-it": "^13.0.2",
"punycode": "^2.3.1",
"sheetclip": "^0.3.0",
"i18next": "^23.5.1",
"jquery-i18next": "^1.2.1",
"language-tags": "^1.0.9",
"sifter": "^0.5.4",
"xlsx": "^0.18.5"
},
Expand Down
43 changes: 33 additions & 10 deletions tests/templates.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,30 @@ template.schema.prefixes.linkml.prefix_prefix == 'linkml'
template.schema.default.prefixes.linkml.prefix_prefix == 'linkml'
*/

// TODO
describe('TemplateProxy', () => {
describe('Template', () => {
let proxy;

beforeEach(async () => {
// Mock the actual buildTemplate with our version

const overrides = {
fetchFileImpl: jest.fn(async function(filePath) {
try {

Check failure on line 40 in tests/templates.test.js

View workflow job for this annotation

GitHub Actions / test

'require' is not defined
const response = Promise.resolve(require('../web/'+filePath));
return response;
} catch (error) {
console.error(`Failed to load ${filePath}: ${error}`);
throw error;
}
})
};

// initiate localized
proxy = await Template.create('test', 'de');
proxy = await Template.create('test', { locale: 'de', overrides });
});

test('should return localized property if it exists', () => {
expect(proxy.schema.name).toBe('AMBR_de');
// proxy.updateLocale('de');
expect(proxy.schema.name).toBe('TEST_de');
expect(proxy.schema.name).toBe(proxy.localized.schema.name);
});

Expand All @@ -51,14 +63,14 @@ describe('TemplateProxy', () => {

test('should switch to a new locale and return appropriate data', () => {
proxy.updateLocale('fr');
expect(proxy.schema.name).toBe('AMBR_fr');
expect(proxy.schema.name).toBe('TEST_fr');
expect(proxy.schema.name).toBe(proxy.localized.schema.name);
expect(proxy.schema.description).toBe('french_description');
});

test('should return to the default locale when updating empty', () => {
proxy.updateLocale();
expect(proxy.schema.name).toBe('AMBR');
expect(proxy.schema.name).toBe('TEST');
expect(proxy.schema.name).toBe(proxy.default.schema.name);
expect(proxy.schema.description).toBe(proxy.default.schema.description);
});
Expand Down Expand Up @@ -135,20 +147,31 @@ describe('Template utilities', () => {
jest.clearAllMocks();
});

// TODO
describe('accessTemplate', () => {
const overrides = {
fetchFileImpl: jest.fn(async function(filePath) {
try {

Check failure on line 153 in tests/templates.test.js

View workflow job for this annotation

GitHub Actions / test

'require' is not defined
const response = Promise.resolve(require('../web/'+filePath));
return response;
} catch (error) {
console.error(`Failed to load ${filePath}: ${error}`);
throw error;
}
})
};

it('should return the correct template if it exists', async () => {
const mockTemplate = { name: 'template1' };
jest.mock('@/web/templates/manifest.json', () => ({
children: [mockTemplate],
}));

const result = await accessTemplate('test');
const result = await accessTemplate('test', overrides.fetchFileImpl);
expect(result[0]).toBe('test');
});

it('should return null if the template does not exist', async () => {
const result = await accessTemplate('non-existent-template');
const result = await accessTemplate('non-existent-template', overrides.fetchFileImpl);
expect(result).toBeNull();
});
});
Expand Down
2 changes: 1 addition & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4182,7 +4182,7 @@ hyperformula@^2.4.0:
tiny-emitter "^2.1.0"
unorm "^1.6.0"

i18next@^23.5.1:
i18next@^23.6.0:
version "23.6.0"
resolved "https://registry.yarnpkg.com/i18next/-/i18next-23.6.0.tgz#c6e996cfd3fef0bf60be3b7c581c35338dba5a71"
integrity sha512-z0Cxr0MGkt+kli306WS4nNNM++9cgt2b2VCMprY92j+AIab/oclgPxdwtTZVLP1zn5t5uo8M6uLsZmYrcjr3HA==
Expand Down

0 comments on commit 2e9c7f5

Please sign in to comment.