diff --git a/contentcuration/contentcuration/constants/completion_criteria.py b/contentcuration/contentcuration/constants/completion_criteria.py index a409547d2f..deee7fd62a 100644 --- a/contentcuration/contentcuration/constants/completion_criteria.py +++ b/contentcuration/contentcuration/constants/completion_criteria.py @@ -1,7 +1,23 @@ -import jsonschema from django.core.exceptions import ValidationError +from jsonschema import RefResolver +from jsonschema.validators import validator_for from le_utils.constants import completion_criteria from le_utils.constants import content_kinds +from le_utils.constants import mastery_criteria + + +def _build_validator(): + """ + Builds the validator, once, for the completion criteria schema and includes the external mastery criteria schema + :rtype: jsonschema.Draft7Validator|jsonschema.validators.Validator + """ + cls = validator_for(completion_criteria.SCHEMA) + validator = cls(completion_criteria.SCHEMA) + validator.resolver.store.update(RefResolver.from_schema(mastery_criteria.SCHEMA).store) + return validator + + +validator = _build_validator() def validate(data, kind=None): @@ -14,10 +30,24 @@ def validate(data, kind=None): if isinstance(data, (dict,)) and not data: return - try: - jsonschema.validate(instance=data, schema=completion_criteria.SCHEMA) - except jsonschema.ValidationError as e: - raise ValidationError("Completion criteria does not conform to schema") from e + error_descriptions = [] + # @see https://python-jsonschema.readthedocs.io/en/latest/errors/ + for error in validator.iter_errors(data): + if error.cause: + # documentation says this will only be set on FormatChecker errors + error_descriptions.append(error.cause) + elif error.absolute_path: + # if there's a path to a field, we can give a specific error + json_path = ".".join(error.absolute_path) + error_descriptions.append(ValidationError("{} {}".format(json_path, error.message))) + else: + # without a path, likely top-level validation error, e.g. `anyOf` conditions + error_descriptions.append(ValidationError("object doesn't satisfy '{}' conditions".format(error.validator))) + + if error_descriptions: + e = ValidationError("Completion criteria doesn't conform to schema") + e.error_list.extend(error_descriptions) + raise e model = data.get("model") if kind is None or model is None: diff --git a/contentcuration/contentcuration/frontend/shared/leUtils/CompletionCriteria.js b/contentcuration/contentcuration/frontend/shared/leUtils/CompletionCriteria.js index e0f806bb9a..a9146f5cef 100644 --- a/contentcuration/contentcuration/frontend/shared/leUtils/CompletionCriteria.js +++ b/contentcuration/contentcuration/frontend/shared/leUtils/CompletionCriteria.js @@ -1,4 +1,5 @@ import CompletionCriteriaModels, { SCHEMA } from 'kolibri-constants/CompletionCriteria'; +import { SCHEMA as MASTERY_SCHEMA } from 'kolibri-constants/MasteryCriteria'; import { ContentKindsNames } from 'shared/leUtils/ContentKinds'; import { compile } from 'shared/utils/jsonSchema'; import { ValidationErrors } from 'shared/constants'; @@ -6,7 +7,7 @@ import { ValidationErrors } from 'shared/constants'; /** * @type {Function|ValidateFunction} */ -const _validate = compile(SCHEMA); +const _validate = compile(SCHEMA, [MASTERY_SCHEMA]); /** * @param {Object} criteria diff --git a/contentcuration/contentcuration/frontend/shared/utils/jsonSchema.js b/contentcuration/contentcuration/frontend/shared/utils/jsonSchema.js index ccab840610..b5a65804bc 100644 --- a/contentcuration/contentcuration/frontend/shared/utils/jsonSchema.js +++ b/contentcuration/contentcuration/frontend/shared/utils/jsonSchema.js @@ -9,6 +9,21 @@ const ajv = new Ajv({ ], }); -export function compile(schema) { - return ajv.compile(schema); +/** + * Compiles the AJV instance with a JSON schema to get a validation function + * + * @param {Object} schema + * @param {Object[]} dependentSchemas + * @return {ValidateFunction} + */ +export function compile(schema, dependentSchemas = null) { + let instance = ajv; + // Add dependent schemas to local instance + if (dependentSchemas) { + instance = dependentSchemas.reduce( + (instance, dependentSchema) => instance.addSchema(dependentSchema), + instance + ); + } + return instance.compile(schema); } diff --git a/contentcuration/contentcuration/tests/test_completion_criteria.py b/contentcuration/contentcuration/tests/test_completion_criteria.py index 0d7e7c8fa6..6af5cdcfa6 100644 --- a/contentcuration/contentcuration/tests/test_completion_criteria.py +++ b/contentcuration/contentcuration/tests/test_completion_criteria.py @@ -2,6 +2,7 @@ from django.test import SimpleTestCase from le_utils.constants import completion_criteria from le_utils.constants import content_kinds +from le_utils.constants import mastery_criteria from contentcuration.constants.completion_criteria import validate @@ -13,12 +14,16 @@ def test_validate__success(self): def test_validate__success__empty(self): validate({}) - def test_validate__fail(self): - with self.assertRaises(ValidationError): + def test_validate__fail__model(self): + with self.assertRaisesRegex(ValidationError, "model 'does not exist' is not one of"): validate({"model": "does not exist"}) + def test_validate__fail__threshold(self): + with self.assertRaisesRegex(ValidationError, "object doesn't satisfy 'anyOf' conditions"): + validate({"model": completion_criteria.PAGES, "threshold": "not a number"}) + def test_validate__content_kind(self): - with self.assertRaises(ValidationError): - validate({"model": completion_criteria.PAGES}, kind=content_kinds.EXERCISE) - with self.assertRaises(ValidationError): - validate({"model": completion_criteria.MASTERY}, kind=content_kinds.DOCUMENT) + with self.assertRaisesRegex(ValidationError, "is invalid for content kind"): + validate({"model": completion_criteria.PAGES, "threshold": 1}, kind=content_kinds.EXERCISE) + with self.assertRaisesRegex(ValidationError, "is invalid for content kind"): + validate({"model": completion_criteria.MASTERY, "threshold": {"mastery_model": mastery_criteria.DO_ALL}}, kind=content_kinds.DOCUMENT) diff --git a/package.json b/package.json index 055ba5f3e9..775ceda9c3 100644 --- a/package.json +++ b/package.json @@ -82,7 +82,7 @@ "intl": "1.2.5", "jquery": "^2.2.4", "jspdf": "https://github.com/MrRio/jsPDF.git#b7a1d8239c596292ce86dafa77f05987bcfa2e6e", - "kolibri-constants": "^0.1.39", + "kolibri-constants": "^0.1.40", "kolibri-tools": "^0.14.5-dev.4", "lodash": "^4.17.21", "material-icons": "0.3.1", diff --git a/requirements.in b/requirements.in index fed4087eb2..010b719271 100644 --- a/requirements.in +++ b/requirements.in @@ -6,7 +6,7 @@ djangorestframework==3.12.4 psycopg2-binary==2.8.6 django-js-reverse==0.9.1 django-registration==3.1.2 -le-utils==0.1.39 +le-utils==0.1.40 gunicorn==19.6.0 django-postmark==0.1.6 jsonfield==3.1.0 diff --git a/requirements.txt b/requirements.txt index 7584c4f527..c099d38443 100644 --- a/requirements.txt +++ b/requirements.txt @@ -165,7 +165,7 @@ jsonschema==3.2.0 # via -r requirements.in kombu==4.6.11 # via celery -le-utils==0.1.39 +le-utils==0.1.40 # via -r requirements.in newrelic==6.2.0.156 # via -r requirements.in diff --git a/yarn.lock b/yarn.lock index 4ecf5871c8..6613de20da 100644 --- a/yarn.lock +++ b/yarn.lock @@ -12316,10 +12316,10 @@ knuth-shuffle-seeded@^1.0.6: dependencies: seed-random "~2.2.0" -kolibri-constants@^0.1.39: - version "0.1.39" - resolved "https://registry.yarnpkg.com/kolibri-constants/-/kolibri-constants-0.1.39.tgz#f91ec1cf19d5043a46b81580da19e1ccedaa39b8" - integrity sha512-6tqli9wb7N7cxQFFT4IEWFUUWTnU96+zVPJG+F2/AIwPFVSmEJdqVh/p+syfDUwqQe8eDxbdd4e+ZSC+cEcxuw== +kolibri-constants@^0.1.40: + version "0.1.40" + resolved "https://registry.yarnpkg.com/kolibri-constants/-/kolibri-constants-0.1.40.tgz#ea75e7ce9716a027638c4ac6db73867bc04d1858" + integrity sha512-264LtFuLoQ15GbZKxWnMcl+uDovzj75GWopnW40ixAVMERrmhSIGuSl1wID9IVOyT788xLQCPhcIqOP5LmDA9A== "kolibri-design-system@github:learningequality/kolibri-design-system#c2acc3fd19cee52ccd2a17fa8d2b10cddb2b3d63": version "1.3.0"