Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
367609b
Attempting to fix import error
ivansg44 Feb 18, 2025
dc1514c
Revert "Attempting to fix import error"
ivansg44 Feb 18, 2025
b84f086
Resolve external webpack schema import in Jest
ivansg44 Feb 18, 2025
59c5869
Attempting to fix Validator tests
ivansg44 Feb 20, 2025
57c3683
Possibly temporary; remove rollup jest transform
ivansg44 Feb 20, 2025
765c7d4
Babel config file if we avoid rollup
ivansg44 Feb 20, 2025
da54ab4
Ignore new babel config in eslint config
ivansg44 Feb 20, 2025
2a7d64c
Stop direct matching undefined to missing props
ivansg44 Feb 20, 2025
daacbcf
Prettier
ivansg44 Feb 20, 2025
89818f0
Handle missing `slotTitle` cases in `fields.js`
ivansg44 Feb 20, 2025
69d7167
Fix potentially incorrect test output
ivansg44 Feb 20, 2025
f083306
Fix more potentially incorrect test outputs
ivansg44 Feb 20, 2025
b0d2962
Attempting to fix import error again
ivansg44 Feb 20, 2025
e1258b2
Revert "Attempting to fix import error again"
ivansg44 Feb 20, 2025
d1b381d
Testing if manifest mock is needed
ivansg44 Feb 20, 2025
89cb477
Re-add some removed code as comments
ivansg44 Mar 5, 2025
c016ff2
Some comments
ivansg44 Mar 5, 2025
3c4441c
Clarifying comments
ivansg44 Mar 5, 2025
c191a27
Remove rollup-jest
ivansg44 Mar 5, 2025
9c356d7
Logic change in Validator.js
ivansg44 Mar 6, 2025
c1a1e5b
Comment
ivansg44 Mar 6, 2025
ac0e877
Merge branch 'master' into fix_ci
ivansg44 Mar 6, 2025
ffd1a23
Prettier
ivansg44 Mar 6, 2025
124c491
Prettier; no unused vars
ivansg44 Mar 6, 2025
b94e0c8
Revert "Prettier; no unused vars"
ivansg44 Mar 6, 2025
d04261b
Trying to fix unused vars thing
ivansg44 Mar 6, 2025
cf5c465
wip
ivansg44 Mar 6, 2025
825f0ba
wip
ivansg44 Mar 6, 2025
e17f90b
Merge branch 'master' into fix_ci
ddooley Mar 17, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ module.exports = {
'lib/rollup.config.js',
'**/dist/**/*.js',
'.venv',
'babel.config.js',
],
env: {
browser: true,
Expand All @@ -17,6 +18,11 @@ module.exports = {
ecmaVersion: 'latest',
sourceType: 'module',
},
rules: {},
rules: {
'no-unused-vars': ['error', {
'argsIgnorePattern': '^_',
'varsIgnorePattern': '^_'
}],
},
Comment thread
ddooley marked this conversation as resolved.
plugins: ['jest'],
};
3 changes: 3 additions & 0 deletions babel.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
presets: [['@babel/preset-env', {targets: {node: 'current'}}]],
};
Comment thread
ddooley marked this conversation as resolved.
3 changes: 1 addition & 2 deletions lib/Toolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,6 @@ class Toolbar {

const schema_container =
this.context.template.default.schema.classes.Container;

// default schema is guaranteed to feature the Container
const Container = Object.entries(schema_container.attributes).reduce(
(acc, [cls_key, { name, range }]) => {
Expand Down Expand Up @@ -906,7 +905,7 @@ class Toolbar {
// classes. That is one to load.
if (!template_name && 'Container' in schema.classes) {
Object.entries(schema.classes.Container.attributes).forEach(
([class_name, class_obj]) => {
([_class_name, class_obj]) => {
if (class_obj.range in schema.classes) {
template_name = class_obj.range;
}
Expand Down
34 changes: 21 additions & 13 deletions lib/Validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,8 @@ class Validator {
// Issue: parse can fail on decimal but menu has "Missing"
if (parsed === undefined) {
parse_error = `Value does not match format for ${slotType.uri}`;
// We do not process any nested anyOf, allOf, etc.
return parse_error
Comment on lines +296 to +297
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a_big_or_small_number: {
name: 'a_big_or_small_number',
range: 'integer',
any_of: [{ maximum_value: 10 }, { minimum_value: 100 }],
}

'hello' will return a parse error === 'Value does not match format for xsd:integer'

The test suite expects us to return this error immediately, so we must return parse error here. Otherwise we will try to process the any_of unnecessarily.

Previously, there was a continue further down that I think was meant to stop such unnecessary processing of any_of, all_of, etc. But I had to comment it out because it was also stopping processing in situations without an initial parse error--where we actually want to process any_of, all_of, etc. e.g., '11' should return 'Value is greater than maximum value\nValue is less than minimum value'


//if (!(anyOfValidators.length || allOfValidators.length || exactlyOneOfValidators.length || noneOfValidators.length)) {
// return parse_error;
Expand Down Expand Up @@ -325,8 +327,10 @@ class Validator {
return 'Value does not match pattern';
}

// Here slotType value tested and is ok!
continue;
// The below code was preventing the remaining code from being
// reached, which included processing any_of, all_of, etc.
// // Here slotType value tested and is ok!
// continue;
Comment thread
ddooley marked this conversation as resolved.
}

// Here value didn't parse to slotType
Expand Down Expand Up @@ -379,17 +383,21 @@ class Validator {
}
}

if (
anyOfValidators.length ||
allOfValidators.length ||
exactlyOneOfValidators.length ||
noneOfValidators.length
) {
// We passed validation here which means a parse error can be overriden
} else if (parse_error.length) {
//There were no other ranges besides basic slotType so
return parse_error;
}
// The below code seems to imply that we can ignore parse errors, as
// long as nested anyOfs, allOfs, etc. are valid. The test suite seems
// to want something different, so we now return parse error
// immediately further up in the code.
// if (
// anyOfValidators.length ||
// allOfValidators.length ||
// exactlyOneOfValidators.length ||
// noneOfValidators.length
// ) {
// // We passed validation here which means a parse error can be overriden
// } else if (parse_error.length) {
// //There were no other ranges besides basic slotType so
// return parse_error;
// }
}
};

Expand Down
35 changes: 28 additions & 7 deletions lib/utils/fields.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,16 @@ export function slotNamesForTitlesMap(fields) {
const fieldSymbolsAtIndex = fieldSymbolsAtIndexMap(fields);
let tempObject = {};
for (let index in fieldSymbolsAtIndex) {
tempObject[fieldSymbolsAtIndex[index].slotTitle] =
fieldSymbolsAtIndex[index].slotName;
// The below code was running into issues when slotTitle did not exist
// tempObject[fieldSymbolsAtIndex[index].slotTitle] =
// fieldSymbolsAtIndex[index].slotName;

const slotName = fieldSymbolsAtIndex[index].slotName;
let slotTitle = fieldSymbolsAtIndex[index].slotTitle;
// If slotTitle not specified; use slotName as slotTitle
slotTitle = slotTitle === undefined ? slotName : slotTitle;

tempObject[slotTitle] = slotName;
Comment on lines +194 to +199
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this what we want? A test was failing previously because the fields global var in fields.test.js was missing slotTitle values. There are several lines of code downstream of this function that expect slotTitle values. I just re-used slotName as title as well to fix things.

}
return tempObject;
}
Expand All @@ -206,15 +214,28 @@ export function slotTitleForNameMap(fields) {
export function findFieldIndex(fields, key, translationMap = {}) {
// First try to find a direct match.
let index = fields.findIndex((f) => {
return f.name === key || f.alias === key;
// Consider the following situation:
// `key === undefined`
// `f = {'name': 'a', 'title': 'a'}`
// If we do not check whether `f` has `name` or `alias`, then:
// 1) `f.name === key || f.alias === key`
// 2) `'a' === undefined || undefined === undefined`
// 3) `false || true`
// 4) `true` --> direct match
// i.e., we directly match `undefined` to a defined field with name `a`
Comment thread
ddooley marked this conversation as resolved.
const sameName = Object.hasOwn(f, 'name') && f.name === key;
const sameAlias = Object.hasOwn(f, 'alias') && f.alias === key;
return sameName || sameAlias;
});

// If no direct match, try to find a match through the translation map.
if (index < 0 && translationMap[key]) {
const untranslatedKey = translationMap[key];
index = fields.findIndex(
(f) => f.name === untranslatedKey || f.alias === untranslatedKey
);
const translatedKey = translationMap[key];
index = fields.findIndex((f) => {
const sameName = Object.hasOwn(f, 'name') && f.name === translatedKey;
const sameAlias = Object.hasOwn(f, 'alias') && f.alias === translatedKey;
return sameName || sameAlias;
});
}

return index;
Expand Down
9 changes: 2 additions & 7 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@
"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",
Expand Down Expand Up @@ -99,13 +98,9 @@
"xlsx": "^0.18.5"
},
"jest": {
"transform": {
"\\.js$": [
"rollup-jest"
]
},
Comment thread
ddooley marked this conversation as resolved.
"moduleNameMapper": {
"^@/(.*)$": "<rootDir>/$1"
"^@/(.*)$": "<rootDir>/$1",
"schemas": "<rootDir>/web/schemas.js"
}
},
"packageManager": "yarn@1.22.22+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e"
Expand Down
8 changes: 4 additions & 4 deletions tests/fields.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ describe('dataObjectToArray', () => {
f: 33,
};
const dataArray = dataObjectToArray(dataObject, fields);
expect(dataArray).toEqual(['', '33.333', '', '', 'a; b; c', '33', '', '']);
expect(dataArray).toEqual(['', '33.333', '', '', 'a;b;c', '33', '', '']);
Comment thread
ddooley marked this conversation as resolved.
});
});

Expand Down Expand Up @@ -185,19 +185,19 @@ describe('formatMultivaluedValue', () => {
test('formats values with correct delimiter and space', () => {
const input = ['one two', 'three', 'four'];
const formatted = formatMultivaluedValue(input);
expect(formatted).toEqual('one two; three; four');
expect(formatted).toEqual('one two;three;four');
});

test('handles non-string values', () => {
const input = ['one two', 3, 'four'];
const formatted = formatMultivaluedValue(input);
expect(formatted).toEqual('one two; 3; four');
expect(formatted).toEqual('one two;3;four');
});

test('discards empty entries', () => {
const input = ['one two', '', 'three', null, 'four'];
const formatted = formatMultivaluedValue(input);
expect(formatted).toEqual('one two; three; four');
expect(formatted).toEqual('one two;three;four');
});

test('returns empty string for null or empty input', () => {
Expand Down
4 changes: 2 additions & 2 deletions tests/templates.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ describe('findBestLocaleMatch function', () => {
});
});

// Mocking the manifest data
jest.mock('@/web/templates/manifest.json');
// // Mocking the manifest data
// jest.mock('@/web/templates/manifest.json');
Comment thread
ddooley marked this conversation as resolved.

describe('Template utilities', () => {
// Reset all mocks after each test
Expand Down
Loading
Loading