Merge pull request #3318 from bjester/completion-criteria

Support adding completion criteria and validating its structure

Author: rtibbles

contentcuration/contentcuration/constants/completion_criteria.py

@@ -0,0 +1,34 @@
+import jsonschema
+from django.core.exceptions import ValidationError
+from le_utils.constants import completion_criteria
+from le_utils.constants import content_kinds
+
+
+def validate(data, kind=None):
+    """
+    :param data: Dictionary of data to validate
+    :param kind: A str of the node content kind
+    :raises: ValidationError: When invalid
+    """
+    # empty dicts are okay
+    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
+
+    model = data.get("model")
+    if kind is None or model is None:
+        return
+
+    # validate that content kind is allowed for the completion criteria model
+    if (model == completion_criteria.PAGES and kind != content_kinds.DOCUMENT) or (
+        model == completion_criteria.MASTERY and kind != content_kinds.EXERCISE
+    ):
+        raise ValidationError(
+            "Completion criteria model '{}' is invalid for content kind '{}'".format(
+                model, kind
+            )
+        )

contentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue

@@ -317,6 +317,7 @@
   import Checkbox from 'shared/views/form/Checkbox';
   import { ContentKindsNames } from 'shared/leUtils/ContentKinds';
   import { NEW_OBJECT, FeatureFlagKeys, ContentModalities } from 'shared/constants';
+  import { validate as validateCompletionCriteria } from 'shared/leUtils/CompletionCriteria';
 
   // Define an object to act as the place holder for non unique values.
   const nonUniqueValue = {};
@@ -497,6 +498,23 @@
           this.updateExtraFields({ options });
         },
       },
+      // TODO remove eslint disable when `completionCriteria` is utilized
+      /* eslint-disable-next-line kolibri/vue-no-unused-properties */
+      completionCriteria: {
+        get() {
+          const options = this.getExtraFieldsValueFromNodes('options') || {};
+          return options.completion_criteria || {};
+        },
+        set(completion_criteria) {
+          // TODO Remove validation if unnecessary after implementing `completionCriteria`
+          if (validateCompletionCriteria(completion_criteria)) {
+            const options = { completion_criteria };
+            this.updateExtraFields({ options });
+          } else {
+            console.warn('Invalid completion criteria', [...validateCompletionCriteria.errors]);
+          }
+        },
+      },
 
       /* COMPUTED PROPS */
       disableAuthEdits() {

contentcuration/contentcuration/frontend/shared/constants.js

@@ -152,6 +152,7 @@ export const ValidationErrors = {
   QUESTION_REQUIRED: 'QUESTION_REQUIRED',
   INVALID_NUMBER_OF_CORRECT_ANSWERS: 'INVALID_NUMBER_OF_CORRECT_ANSWERS',
   NO_VALID_PRIMARY_FILES: 'NO_VALID_PRIMARY_FILES',
+  INVALID_COMPLETION_CRITERIA_MODEL: 'INVALID_COMPLETION_CRITERIA_MODEL',
   ...fileErrors,
 };
 

contentcuration/contentcuration/frontend/shared/leUtils/CompletionCriteria.js

@@ -0,0 +1,37 @@
+import CompletionCriteriaModels, { SCHEMA } from 'kolibri-constants/CompletionCriteria';
+import { ContentKindsNames } from 'shared/leUtils/ContentKinds';
+import { compile } from 'shared/utils/jsonSchema';
+import { ValidationErrors } from 'shared/constants';
+
+/**
+ * @type {Function<boolean>|ValidateFunction}
+ */
+const _validate = compile(SCHEMA);
+
+/**
+ * @param {Object} criteria
+ * @param {String} contentKind
+ * @return {boolean}
+ */
+export function validate(criteria, contentKind = null) {
+  // Mimic ajv validation for simplicity
+  validate.errors = [];
+
+  if (!_validate(criteria)) {
+    validate.errors = _validate.errors;
+    return false;
+  }
+
+  // Validate certain models only apply to certain content kinds
+  if (
+    (criteria.model === CompletionCriteriaModels.PAGES &&
+      contentKind !== ContentKindsNames.DOCUMENT) ||
+    (criteria.model === CompletionCriteriaModels.MASTERY &&
+      contentKind !== ContentKindsNames.EXERCISE)
+  ) {
+    validate.errors = [ValidationErrors.INVALID_COMPLETION_CRITERIA_MODEL];
+    return false;
+  }
+
+  return true;
+}

contentcuration/contentcuration/frontend/shared/utils/jsonSchema.js

@@ -0,0 +1,14 @@
+import Ajv from 'ajv';
+
+const ajv = new Ajv({
+  keywords: [
+    {
+      keyword: '$exportConstants',
+      schemaType: 'string',
+    },
+  ],
+});
+
+export function compile(schema) {
+  return ajv.compile(schema);
+}

contentcuration/contentcuration/frontend/shared/utils/validation.js

@@ -1,8 +1,10 @@
+import get from 'lodash/get';
 import { AssessmentItemTypes, ValidationErrors } from '../constants';
 import translator from '../translator';
 import Licenses from 'shared/leUtils/Licenses';
 import { MasteryModelsNames } from 'shared/leUtils/MasteryModels';
 import { ContentKindsNames } from 'shared/leUtils/ContentKinds';
+import { validate as validateCompletionCriteria } from 'shared/leUtils/CompletionCriteria';
 
 /**
  * Topic and resource
@@ -29,6 +31,10 @@ import { ContentKindsNames } from 'shared/leUtils/ContentKinds';
  * It must have at least one question
  * A question must have right answers
  *
+ * Non-topics
+ * ----------
+ * Completion criteria is validated to ensure it conforms to the schema
+ *   and is valid for the content kind
  */
 export function isNodeComplete({ nodeDetails, assessmentItems, files }) {
   if (!nodeDetails) {
@@ -56,6 +62,12 @@ export function isNodeComplete({ nodeDetails, assessmentItems, files }) {
       return false;
     }
   }
+  if (nodeDetails.kind !== ContentKindsNames.TOPIC) {
+    const completionCriteria = get(nodeDetails, 'extra_fields.options.completion_criteria');
+    if (completionCriteria && !validateCompletionCriteria(completionCriteria, nodeDetails.kind)) {
+      return false;
+    }
+  }
   if (nodeDetails.kind === ContentKindsNames.EXERCISE) {
     if (!assessmentItems.length) {
       return false;

contentcuration/contentcuration/frontend/shared/utils/validation.spec.js

@@ -1,5 +1,5 @@
 import each from 'jest-each';
-
+import CompletionCriteriaModels from 'kolibri-constants/CompletionCriteria';
 import { AssessmentItemTypes, ValidationErrors } from '../constants';
 import {
   translateValidator,
@@ -349,6 +349,12 @@ describe('channelEdit utils', () => {
           license: { id: 8 },
           extra_fields: {
             mastery_model: MasteryModelsNames.DO_ALL,
+            options: {
+              completion_criteria: {
+                model: CompletionCriteriaModels.TIME,
+                threshold: 10,
+              },
+            },
           },
         };
         assessmentItems = [
@@ -400,11 +406,43 @@ describe('channelEdit utils', () => {
         ).toBe(false);
       });
 
+      it('returns false if completion_criteria is invalid', () => {
+        const details = {
+          ...nodeDetails,
+          extra_fields: {
+            ...nodeDetails.extra_fields,
+            options: {
+              completion_criteria: {
+                // pages model not allowed for exercise
+                model: CompletionCriteriaModels.PAGES,
+                threshold: 12,
+              },
+            },
+          },
+        };
+        expect(isNodeComplete({ nodeDetails: details, assessmentItems })).toBe(false);
+      });
+
       it(`
         returns true if node details are valid,
         there is at least one assessment items,
         and all assessment items are valid`, () => {
-        expect(isNodeComplete({ nodeDetails, assessmentItems })).toBe(true);
+        const details = {
+          ...nodeDetails,
+          extra_fields: {
+            ...nodeDetails.extra_fields,
+            options: {
+              completion_criteria: {
+                // mastery model only allowed for exercise
+                model: CompletionCriteriaModels.MASTERY,
+                threshold: {
+                  mastery_model: 'do_all',
+                },
+              },
+            },
+          },
+        };
+        expect(isNodeComplete({ nodeDetails: details, assessmentItems })).toBe(true);
       });
     });
 
@@ -419,6 +457,14 @@ describe('channelEdit utils', () => {
           title: 'A node',
           license: { id: 8 },
           kind,
+          extra_fields: {
+            options: {
+              completion_criteria: {
+                model: CompletionCriteriaModels.TIME,
+                threshold: 10,
+              },
+            },
+          },
         };
         files = [
           {
@@ -456,6 +502,39 @@ describe('channelEdit utils', () => {
         expect(isNodeComplete({ nodeDetails, files: [...files, invalidFile] })).toBe(false);
       });
 
+      it('returns false if completion_criteria is invalid', () => {
+        const details = {
+          ...nodeDetails,
+          extra_fields: {
+            options: {
+              completion_criteria: {
+                model: 'pages',
+                threshold: -1,
+              },
+            },
+          },
+        };
+        expect(isNodeComplete({ nodeDetails: details, files })).toBe(false);
+      });
+
+      it('returns false if completion_criteria is invalid for kind', () => {
+        const details = {
+          ...nodeDetails,
+          extra_fields: {
+            options: {
+              completion_criteria: {
+                // mastery model only allowed for exercise
+                model: CompletionCriteriaModels.MASTERY,
+                threshold: {
+                  mastery_model: 'do_all',
+                },
+              },
+            },
+          },
+        };
+        expect(isNodeComplete({ nodeDetails: details, files })).toBe(false);
+      });
+
       it('returns true if node details and all files are valid', () => {
         expect(isNodeComplete({ nodeDetails, files })).toBe(true);
       });

contentcuration/contentcuration/migrations/0134_alter_contentkind_kind.py

@@ -0,0 +1,34 @@
+# Generated by Django 3.2.5 on 2022-02-23 18:42
+from django.db import migrations
+from django.db import models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("contentcuration", "0133_auto_20220124_2149"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="contentkind",
+            name="kind",
+            field=models.CharField(
+                choices=[
+                    ("topic", "Topic"),
+                    ("video", "Video"),
+                    ("audio", "Audio"),
+                    ("exercise", "Exercise"),
+                    ("document", "Document"),
+                    ("html5", "HTML5 App"),
+                    ("slideshow", "Slideshow"),
+                    ("h5p", "H5P"),
+                    ("zim", "Zim"),
+                    ("quiz", "Quiz"),
+                ],
+                max_length=200,
+                primary_key=True,
+                serialize=False,
+            ),
+        ),
+    ]

contentcuration/contentcuration/tests/test_completion_criteria.py

@@ -0,0 +1,24 @@
+from django.core.exceptions import ValidationError
+from django.test import SimpleTestCase
+from le_utils.constants import completion_criteria
+from le_utils.constants import content_kinds
+
+from contentcuration.constants.completion_criteria import validate
+
+
+class CompletionCriteriaTestCase(SimpleTestCase):
+    def test_validate__success(self):
+        validate({"model": completion_criteria.REFERENCE, "learner_managed": True})
+
+    def test_validate__success__empty(self):
+        validate({})
+
+    def test_validate__fail(self):
+        with self.assertRaises(ValidationError):
+            validate({"model": "does not exist"})
+
+    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)

contentcuration/contentcuration/tests/test_serializers.py

@@ -1,6 +1,7 @@
 from __future__ import absolute_import
 
 from django.db.models.query import QuerySet
+from django.test.testcases import SimpleTestCase
 
 from .base import BaseAPITestCase
 from contentcuration.models import Channel
@@ -9,6 +10,7 @@
 from contentcuration.viewsets.channel import ChannelSerializer as BaseChannelSerializer
 from contentcuration.viewsets.common import ContentDefaultsSerializer
 from contentcuration.viewsets.contentnode import ContentNodeSerializer
+from contentcuration.viewsets.contentnode import ExtraFieldsOptionsSerializer
 
 
 def ensure_no_querysets_in_serializer(object):
@@ -21,6 +23,27 @@ def ensure_no_querysets_in_serializer(object):
         )
 
 
+class ExtraFieldsOptionsSerializerTestCase(SimpleTestCase):
+    def setUp(self):
+        super(ExtraFieldsOptionsSerializerTestCase, self).setUp()
+        self.data = dict(modality="QUIZ")
+
+    @property
+    def serializer(self):
+        return ExtraFieldsOptionsSerializer(data=self.data)
+
+    def test_no_completion_criteria(self):
+        self.assertTrue(self.serializer.is_valid())
+
+    def test_completion_criteria__valid(self):
+        self.data.update(completion_criteria={"model": "time", "threshold": 10, "learner_managed": True})
+        self.assertTrue(self.serializer.is_valid())
+
+    def test_completion_criteria__invalid(self):
+        self.data.update(completion_criteria={"model": "time", "threshold": "test"})
+        self.assertFalse(self.serializer.is_valid())
+
+
 class ContentNodeSerializerTestCase(BaseAPITestCase):
     def test_repr_doesnt_evaluate_querysets(self):
         node_ids = [