From 756d7305944ba54e9b962ede968e7e19a1d9c996 Mon Sep 17 00:00:00 2001 From: nickevansuk <2616208+nickevansuk@users.noreply.github.com> Date: Wed, 5 Apr 2023 09:58:44 +0100 Subject: [PATCH 1/2] feat: Data quality measures --- src/classes/model-node.js | 16 ++- src/index.js | 4 +- src/measures/exclusion-modes.js | 6 + src/measures/index.js | 6 + src/measures/profile-processor.js | 50 +++++++ src/measures/profiles/accessibility.js | 21 +++ src/measures/profiles/common.js | 174 +++++++++++++++++++++++++ src/rules/rule.js | 2 + src/validate-spec.js | 21 ++- src/validate.js | 34 +++++ 10 files changed, 331 insertions(+), 3 deletions(-) create mode 100644 src/measures/exclusion-modes.js create mode 100644 src/measures/index.js create mode 100644 src/measures/profile-processor.js create mode 100644 src/measures/profiles/accessibility.js create mode 100644 src/measures/profiles/common.js diff --git a/src/classes/model-node.js b/src/classes/model-node.js index 618288cc..ff78c7b8 100644 --- a/src/classes/model-node.js +++ b/src/classes/model-node.js @@ -22,20 +22,34 @@ const ModelNode = class { getPath(...fields) { const path = []; + const tree = []; let node = this; do { if (typeof node.arrayIndex !== 'undefined') { path.unshift(node.arrayIndex); + tree.unshift({ + arrayIndex: node.arrayIndex, + }); } path.unshift(node.name); + tree.unshift({ + name: node.name, + type: node.model.type, + }); node = node.parentNode; } while (node !== null); for (const field of fields) { if (typeof field !== 'undefined') { path.push(field); + tree.push({ + name: field, + }); } } - return jp.stringify(path).replace(/([\][])\1+/g, '$1'); + return { + string: jp.stringify(path).replace(/([\][])\1+/g, '$1'), + tree, + }; } static checkInheritRule(rule, field) { diff --git a/src/index.js b/src/index.js index c960c2c1..49af2f9d 100644 --- a/src/index.js +++ b/src/index.js @@ -4,7 +4,7 @@ */ const defaultRules = require('./rules'); const { - validate, isRpdeFeed, + validate, validateWithMeasures, addMeasures, isRpdeFeed, } = require('./validate'); const Rule = require('./rules/rule'); const ValidationError = require('./errors/validation-error'); @@ -18,6 +18,8 @@ function createValidator() { isRpdeFeed, Rule, validate, + validateWithMeasures, + addMeasures, ValidationError, ValidationErrorCategory, ValidationErrorType, diff --git a/src/measures/exclusion-modes.js b/src/measures/exclusion-modes.js new file mode 100644 index 00000000..e2e55c5f --- /dev/null +++ b/src/measures/exclusion-modes.js @@ -0,0 +1,6 @@ +const ExclusionMode = Object.freeze({ + ALL: 'all', + ANY: 'any', +}); + +module.exports = ExclusionMode; diff --git a/src/measures/index.js b/src/measures/index.js new file mode 100644 index 00000000..4b67e91d --- /dev/null +++ b/src/measures/index.js @@ -0,0 +1,6 @@ +/* eslint-disable global-require */ + +module.exports = [ + require('./profiles/common'), + require('./profiles/accessibility'), +]; diff --git a/src/measures/profile-processor.js b/src/measures/profile-processor.js new file mode 100644 index 00000000..f5fc27f2 --- /dev/null +++ b/src/measures/profile-processor.js @@ -0,0 +1,50 @@ +// After validation is complete, and errors have been generated, the following set of profiles are applied to the errors array. +// The results of this are rendered to a separate "profileMeasures" array, which are available to: +// - The test suite's HTML output +// - The validator (as a tab or collapsable box on the right-hand side?), potentially linking to the errors themselves below (which would then be available in the visualiser) +// - The status page + +// Open questions: +// - Do we need to think about combining parent and child in the feed within the test suite for a more accurate assessment of e.g. the `url`? + +const ExclusionMode = require('./exclusion-modes'); +const profiles = require('.'); + +const ProfileProcessor = class { + static matchTree(targetFields, tree) { + const { name } = tree.slice(-1)[0]; + const { type } = tree.slice(-2)[0]; + for (const [targetType, targetField] of Object.entries(targetFields)) { + if (targetType === type && targetField.includes(name)) return true; + } + return false; + } + + static doesErrorMatchExclusion(error, exclusion) { + return exclusion.errorType.includes(error.type) + && ( + (exclusion.targetFields && this.matchTree(exclusion.targetFields, error.pathTree)) + || (exclusion.targetPaths + && exclusion.targetPaths.some((path) => path.test(error.path))) + ); + } + + static doErrorsMatchExclusionsInMeasure(errors, measure) { + const matchingExclusions = measure.exclusions.filter((exclusion) => errors.some((error) => this.doesErrorMatchExclusion(error, exclusion))); + return measure.exclusionMode === ExclusionMode.ALL ? matchingExclusions.length === measure.exclusions.length : matchingExclusions.length > 0; + } + + static calculateMeasures(errors) { + const results = {}; + for (const profile of profiles) { + const profileResult = {}; + for (const measure of profile.measures) { + profileResult[measure.name] = this.doErrorsMatchExclusionsInMeasure(errors, measure) ? 0 : 1; + } + results[profile.identifier] = profileResult; + } + return results; + } +}; + +module.exports = ProfileProcessor; diff --git a/src/measures/profiles/accessibility.js b/src/measures/profiles/accessibility.js new file mode 100644 index 00000000..c92405a6 --- /dev/null +++ b/src/measures/profiles/accessibility.js @@ -0,0 +1,21 @@ +const ValidationErrorType = require('../../errors/validation-error-type'); + +module.exports = { + name: 'Accessibility', + identifier: 'accessibility', + measures: [ + { + name: 'Has accessibilitySupport', + exclusions: [ // Logic: if no errors are present for any of the paths (paths are matched with endsWith) + { + errorType: [ + ValidationErrorType.MISSING_RECOMMENDED_FIELD, + ], + targetPaths: [ + /\.accessibilitySupport/, + ], + }, + ], + }, + ], +}; diff --git a/src/measures/profiles/common.js b/src/measures/profiles/common.js new file mode 100644 index 00000000..f860eb49 --- /dev/null +++ b/src/measures/profiles/common.js @@ -0,0 +1,174 @@ +const ValidationErrorType = require('../../errors/validation-error-type'); +const ExclusionMode = require('../exclusion-modes'); + +module.exports = { + name: 'Common use', + identifier: 'common', + measures: [ + { + name: 'Has a leader name', + exclusions: [ // Logic: if no errors are present for any of the paths (paths are matched with endsWith), for any exclusion + { + errorType: [ + ValidationErrorType.MISSING_REQUIRED_FIELD, + ], + targetPaths: [ + /\.leader\.name$/, + ], + }, + ], + }, + { + name: 'Has a name', + exclusions: [ // Logic: if no errors are present for any of the target fields, for any exclusion + { + errorType: [ + ValidationErrorType.MISSING_REQUIRED_FIELD, + ], + targetFields: { + Event: ['name'], + FacilityUse: ['name'], + IndividualFacilityUse: ['name'], + CourseInstance: ['name'], + EventSeries: ['name'], + HeadlineEvent: ['name'], + SessionSeries: ['name'], + Course: ['name'], + }, + }, + ], + }, + { + name: 'Has a description', + exclusions: [ // Logic: if no errors are present for any of the target fields, for any exclusion + { + errorType: [ + ValidationErrorType.MISSING_RECOMMENDED_FIELD, + ], + targetFields: { + Event: ['description'], + FacilityUse: ['description'], + IndividualFacilityUse: ['description'], + CourseInstance: ['description'], + EventSeries: ['description'], + HeadlineEvent: ['description'], + SessionSeries: ['description'], + Course: ['description'], + }, + }, + ], + }, + { + name: 'Has a postcode or lat/long', + exclusions: [ + { + errorType: [ + ValidationErrorType.MISSING_REQUIRED_FIELD, + ], + targetFields: { + Place: ['geo', 'address'], + }, + }, + ], + }, + { + name: 'Has a date in the future', + exclusions: [ + { + errorType: [ + ValidationErrorType.DATE_IN_THE_PAST, // TODO: Add this rule, outputs a warning for dates in the past + ], + }, + ], + }, + { + name: 'Activity List ID matches', + exclusions: [ + { + errorType: [ + ValidationErrorType.MISSING_REQUIRED_FIELD, + ], + targetFields: { + Event: ['activity'], + CourseInstance: ['activity'], + EventSeries: ['activity'], + HeadlineEvent: ['activity'], + SessionSeries: ['activity'], + Course: ['activity'], + }, + }, + { + errorType: [ + ValidationErrorType.ACTIVITY_NOT_IN_ACTIVITY_LIST, + ValidationErrorType.USE_OFFICIAL_ACTIVITY_LIST, + ], + }, + ], + }, + { + name: 'Session has name, description or matching activity', + exclusionMode: ExclusionMode.ALL, // ALL: all exclusions must be present to discount the item; ANY (default): Any exclusions discount the item if present + exclusions: [ + { + errorType: [ + ValidationErrorType.MISSING_REQUIRED_FIELD, + ], + targetFields: { + Event: ['name'], + FacilityUse: ['name'], + IndividualFacilityUse: ['name'], + CourseInstance: ['name'], + EventSeries: ['name'], + HeadlineEvent: ['name'], + SessionSeries: ['name'], + Course: ['name'], + }, + }, + { + errorType: [ + ValidationErrorType.MISSING_RECOMMENDED_FIELD, + ], + targetFields: { + Event: ['description'], + FacilityUse: ['description'], + IndividualFacilityUse: ['description'], + CourseInstance: ['description'], + EventSeries: ['description'], + HeadlineEvent: ['description'], + SessionSeries: ['description'], + Course: ['description'], + }, + }, + { + errorType: [ + ValidationErrorType.ACTIVITY_NOT_IN_ACTIVITY_LIST, + ValidationErrorType.USE_OFFICIAL_ACTIVITY_LIST, + ], + }, + ], + }, + { + name: 'Has has a unique URL (e.g. for booking)', + exclusions: [ // Logic: if no errors are present for any of the target fields, for any exclusion + // Note that the required field logic is applied before errors outputted, therefore the below applies all inheritance logic etc implicitly + { + errorType: [ + ValidationErrorType.MISSING_REQUIRED_FIELD, // TODO: Add a rule that checks that the url is unique within the set of URLs given on the page (?), of perhaps using a hashmap of the @id + ], + targetFields: { + Event: ['url'], + FacilityUse: ['url'], + Slot: ['url'], + IndividualFacilityUse: ['url'], + CourseInstance: ['url'], + EventSeries: ['url'], + HeadlineEvent: ['url'], + SessionSeries: ['url'], + ScheduledSession: ['url'], + Course: ['url'], + }, + }, + ], + }, + ], +}; diff --git a/src/rules/rule.js b/src/rules/rule.js index 1daceb30..f74fdee7 100644 --- a/src/rules/rule.js +++ b/src/rules/rule.js @@ -59,6 +59,8 @@ class Rule { const error = Object.assign( extra, { + path: extra.path.string, + pathTree: extra.path.tree, rule: this.meta.name, category: rule.category, type: rule.type, diff --git a/src/validate-spec.js b/src/validate-spec.js index 99d85bb3..6da063d2 100644 --- a/src/validate-spec.js +++ b/src/validate-spec.js @@ -1,5 +1,5 @@ const nock = require('nock'); -const { validate } = require('./validate'); +const { validate, validateWithMeasures } = require('./validate'); const ValidationErrorSeverity = require('./errors/validation-error-severity'); const ValidationErrorType = require('./errors/validation-error-type'); const DataModelHelper = require('./helpers/data-model'); @@ -690,4 +690,23 @@ describe('validate', () => { expect(result[0].severity).toBe(ValidationErrorSeverity.NOTICE); }); }); + + describe('validateWithMeasures()', () => { + it('should include', async () => { + const event = { ...validSessionSeries }; + + delete event.name; + + const { errors, profileMeasures } = await validateWithMeasures(event, options); + + expect(errors.length).toBe(1); + + expect(errors[0].type).toBe(ValidationErrorType.MISSING_REQUIRED_FIELD); + expect(errors[0].severity).toBe(ValidationErrorSeverity.FAILURE); + expect(errors[0].path).toBe('$.name'); + + expect(profileMeasures.profiles.common['Has a name']).toEqual(0); + expect(profileMeasures.profiles.common['Has a description']).toEqual(1); + }); + }); }); diff --git a/src/validate.js b/src/validate.js index b0228aa1..cba0db4d 100644 --- a/src/validate.js +++ b/src/validate.js @@ -5,6 +5,7 @@ const RawHelper = require('./helpers/raw'); const OptionsHelper = require('./helpers/options'); const PropertyHelper = require('./helpers/property'); const Rules = require('./rules'); +const ProfileProcessor = require('./measures/profile-processor'); class ApplyRules { static loadModel(modelName, version) { @@ -211,11 +212,44 @@ async function validate(value, options) { return errors.map((x) => x.data); } +async function validateWithMeasures(value, options) { + const errors = await validate(value, options); + const profileMeasures = { + profiles: ProfileProcessor.calculateMeasures(errors), + totalItemCount: 1, + }; + return { + errors, + profileMeasures, + }; +} + +async function addMeasures(existingMeasure, newMeasure) { + for (const [profile, measures] of Object.entries(newMeasure.profiles)) { + for (const [measure, count] of Object.entries(measures)) { + // eslint-disable-next-line no-param-reassign + if (!existingMeasure.profiles) existingMeasure.profiles = {}; + // eslint-disable-next-line no-param-reassign + if (!existingMeasure.profiles[profile]) existingMeasure.profiles[profile] = {}; + // eslint-disable-next-line no-param-reassign + if (!existingMeasure.profiles[profile][measure]) existingMeasure.profiles[profile][measure] = 0; + // eslint-disable-next-line no-param-reassign + existingMeasure.profiles[profile][measure] += count; + } + } + // eslint-disable-next-line no-param-reassign + if (!existingMeasure.totalItemCount) existingMeasure.totalItemCount = 0; + // eslint-disable-next-line no-param-reassign + existingMeasure.totalItemCount += newMeasure.totalItemCount; +} + function isRpdeFeed(data) { return RawHelper.isRpdeFeed(data); } module.exports = { validate, + validateWithMeasures, + addMeasures, isRpdeFeed, }; From 6e0d7ec227c37fb1f49a5bb35931d97e81cb9b2b Mon Sep 17 00:00:00 2001 From: nickevansuk <2616208+nickevansuk@users.noreply.github.com> Date: Thu, 27 Apr 2023 10:55:11 +0100 Subject: [PATCH 2/2] Add description --- src/measures/profiles/common.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/measures/profiles/common.js b/src/measures/profiles/common.js index f860eb49..5483a726 100644 --- a/src/measures/profiles/common.js +++ b/src/measures/profiles/common.js @@ -20,6 +20,7 @@ module.exports = { }, { name: 'Has a name', + description: 'The name of the opportunity is essential for a participant to understand what the activity', exclusions: [ // Logic: if no errors are present for any of the target fields, for any exclusion { errorType: [