When contextualizing a form, remove unneeded elements in conditions

brunobord · brunobord · commit 842f8d43eeb4 · 2018-03-06T09:15:25.000+01:00
* Remove source fields and target fields if they don't exist in the context of the form
* Remove conditions when the source fields (in `tests`) are not in the contextualized form
* Remove conditions if their target fields (`fields_ids`) is empty
* After the Proof of Concept, we've refactored the contextualize_conditions to make it faster
diff --git a/demo/tests/fixtures/conditions-contextualization.json b/demo/tests/fixtures/conditions-contextualization.json
@@ -0,0 +1,67 @@
+{
+  "fields": [
+    {
+      "defaults": [],
+      "description": "Yes/No",
+      "disabled": false,
+      "placeholder": null,
+      "required": false,
+      "slug": "to-check",
+      "type_id": "checkbox",
+      "validations": [],
+      "accesses": [
+        { "access_id": "TEST_ROLE", "level": "EDITABLE" },
+        { "access_id": "TEST_ROLE2", "level": "EDITABLE" }
+      ]
+    },
+    {
+      "defaults": [],
+      "description": null,
+      "disabled": false,
+      "label": "second field",
+      "placeholder": null,
+      "required": false,
+      "slug": "to-display",
+      "type_id": "paragraph",
+      "validations": [],
+      "accesses": [
+        { "access_id": "TEST_ROLE2", "level": "EDITABLE" },
+        { "access_id": "TEST_ROLE", "level": "EDITABLE" }
+      ]
+    },
+    {
+      "defaults": [],
+      "description": null,
+      "disabled": false,
+      "label": "Always displayed field",
+      "placeholder": null,
+      "required": false,
+      "slug": "always-displayed",
+      "type_id": "paragraph",
+      "validations": [],
+      "accesses": [
+        { "access_id": "TEST_ROLE2", "level": "EDITABLE" },
+        { "access_id": "TEST_ROLE", "level": "EDITABLE" }
+      ]
+    }
+  ],
+  "id": 1,
+  "label": "Some label here",
+  "description": "Description",
+  "conditions": [
+    {
+      "action": "display_iff",
+      "fields_ids": [
+        "to-display"
+      ],
+      "name": "Rule",
+      "tests": [
+        {
+          "field_id": "to-check",
+          "operator": "eq",
+          "values": [ true ]
+        }
+      ]
+    }
+  ]
+}
diff --git a/demo/tests/test_conditions.py b/demo/tests/test_conditions.py
@@ -4,6 +4,7 @@
 
 import os
 import json
+import copy
 
 from django.test import TestCase
 
@@ -224,23 +225,6 @@ def test_condition_field_doesnt_exist(self):
         else:
             self.assertTrue(True)
 
-    def test_filter_conditions_by_fields_ids(self):
-        first_schema = contextualize(self.conditions_schema, 'TEST_ROLE')
-        conditions = first_schema['conditions']
-        self.assertEqual(len(conditions), 1)
-        condition = conditions[0]
-
-        self.assertEqual(condition['fields_ids'], ['first-field'])
-
-        second_schema = contextualize(self.conditions_schema, 'TEST_ROLE2')
-        conditions = second_schema['conditions']
-        self.assertEqual(len(conditions), 1)
-        condition = conditions[0]
-        self.assertEqual(
-            sorted(condition['fields_ids']),
-            ['first-field', 'second-field']
-        )
-
 
 class ConditionFromSchemaTestCase(ConditionTestCase):
 
@@ -297,3 +281,143 @@ def test_serializer(self):
         self.assertTrue(serializer.is_valid(), serializer.errors)
         instance = serializer.save()
         self.assertEqual(instance.conditions, self.payload['conditions'])
+
+
+class ConditionContextualizationTest(TestCase):
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.conditions_schema = json.load(open(
+            os.path.join(
+                TESTS_DIR, 'fixtures', 'conditions-contextualization.json'
+            )
+        ))
+
+    def test_condition_normal_context(self):
+        schema = copy.deepcopy(self.conditions_schema)
+
+        first_schema = contextualize(schema, 'TEST_ROLE')
+        conditions = first_schema['conditions']
+        self.assertEqual(len(conditions), 1)
+
+        second_schema = contextualize(schema, 'TEST_ROLE2')
+        conditions = second_schema['conditions']
+        self.assertEqual(len(conditions), 1)
+
+    def test_condition_to_display_only_for_role2(self):
+        schema = copy.deepcopy(self.conditions_schema)
+
+        # The field "to-display" will only be visible for TEST_ROLE2
+        # As a consequence, the conditions for this role will filter it out.
+        to_display = schema['fields'][1]
+        to_display['accesses'] = [
+            {"access_id": "TEST_ROLE", "level": "HIDDEN"},
+            {"access_id": "TEST_ROLE2", "level": "EDITABLE"}
+        ]
+        # Contextualized form for the first role
+        first_schema = contextualize(schema, 'TEST_ROLE')
+        conditions = first_schema['conditions']
+        self.assertEqual(len(conditions), 0)
+
+        # Contextualized form for the second role
+        second_schema = contextualize(schema, 'TEST_ROLE2')
+        conditions = second_schema['conditions']
+        # Conditions are still here
+        self.assertEqual(len(conditions), 1)
+        condition = conditions[0]
+        self.assertEqual(condition['fields_ids'], ['to-display'])
+
+        # Correct tests
+        self.assertEqual(len(condition['tests']), 1)
+        tests = condition['tests'][0]
+        self.assertEqual(tests, {
+            "field_id": "to-check",
+            "operator": "eq",
+            "values": [True]
+        })
+
+    def test_condition_to_check_only_for_role2(self):
+        schema = copy.deepcopy(self.conditions_schema)
+
+        # The field "to-check" will only be visible for TEST_ROLE2
+        # As a consequence, the conditions for this role will filter it out.
+        to_check = schema['fields'][0]
+        to_check['accesses'] = [
+            {"access_id": "TEST_ROLE", "level": "HIDDEN"},
+            {"access_id": "TEST_ROLE2", "level": "EDITABLE"}
+        ]
+        first_schema = contextualize(schema, 'TEST_ROLE')
+        conditions = first_schema['conditions']
+        # Currently, it means that the conditions are still there, and they
+        # are configured as if the field was still there.
+        self.assertEqual(len(conditions), 0)
+
+        # Contextualized form for the second role
+        second_schema = contextualize(schema, 'TEST_ROLE2')
+        conditions = second_schema['conditions']
+        self.assertEqual(len(conditions), 1)
+        condition = conditions[0]
+        self.assertEqual(condition['fields_ids'], ['to-display'])
+
+        # Correct tests (Which is normal)
+        self.assertEqual(len(condition['tests']), 1)
+        tests = condition['tests'][0]
+        self.assertEqual(tests, {
+            "field_id": "to-check",
+            "operator": "eq",
+            "values": [True]
+        })
+
+    def test_condition_to_display_only_for_role2_plus_one_more_field(self):
+        schema = copy.deepcopy(self.conditions_schema)
+
+        # Add a field to the conditions
+        # The condition should be visible in the ROLE and ROLE2 Contextualized
+        # forms, because you still have one field to display.
+        schema['conditions'][0]['fields_ids'].append('always-displayed')
+
+        # The field "to-display" will only be visible for TEST_ROLE2
+        # As a consequence, the conditions for this role will filter it out.
+        to_display = schema['fields'][1]
+        to_display['accesses'] = [
+            {"access_id": "TEST_ROLE", "level": "HIDDEN"},
+            {"access_id": "TEST_ROLE2", "level": "EDITABLE"}
+        ]
+        # Contextualized form for the first role
+        first_schema = contextualize(schema, 'TEST_ROLE')
+        conditions = first_schema['conditions']
+        self.assertEqual(len(conditions), 1)
+        condition = conditions[0]
+        # Currently, it means that conditions **targeting** this field will
+        # see it removed from their "fields_ids" property
+        # Here, the fields_ids is empty, it looks a bit wrong.
+        self.assertEqual(condition['fields_ids'], ['always-displayed'])
+
+        # Correct tests
+        self.assertEqual(len(condition['tests']), 1)
+        tests = condition['tests'][0]
+        self.assertEqual(tests, {
+            "field_id": "to-check",
+            "operator": "eq",
+            "values": [True]
+        })
+
+        # Contextualized form for the second role
+        second_schema = contextualize(schema, 'TEST_ROLE2')
+        conditions = second_schema['conditions']
+        # Conditions are still here
+        self.assertEqual(len(conditions), 1)
+        condition = conditions[0]
+        self.assertEqual(
+            sorted(condition['fields_ids']),
+            sorted(['to-display', 'always-displayed'])
+        )
+
+        # Correct tests
+        self.assertEqual(len(condition['tests']), 1)
+        tests = condition['tests'][0]
+        self.assertEqual(tests, {
+            "field_id": "to-check",
+            "operator": "eq",
+            "values": [True]
+        })
diff --git a/formidable/serializers/forms.py b/formidable/serializers/forms.py
@@ -174,19 +174,36 @@ def contextualize_fields(fields, role):
         yield field
 
 
-def contextualize(form, role):
+def contextualize_conditions(form):
     """
-    Transform a FormidableJSON into a ContextFormJSON for a given role.
-
+    Extract conditions and filter them using the fields that exist in the form.
     """
-    form = copy.deepcopy(form)
-    form['fields'] = list(contextualize_fields(form['fields'], role))
-
     # filter conditions by fields ids for current role
     field_ids = {field['slug'] for field in form['fields']}
-    for condition in form.get('conditions', []):
+    conditions = form.get('conditions', [])
+
+    for condition in conditions:
+        # Build the tests based on the existing fields in the form
+        condition['tests'] = [
+            t for t in condition['tests']
+            if t['field_id'] in field_ids
+        ]
+        # Build the fields_ids based on the existing fields in the form.
         condition['fields_ids'] = list(
             set(condition.get('fields_ids', [])) & field_ids
         )
+        # 1. If the condition "tests" is empty, remove it
+        # 2. if the condition "fields_ids" is empty, remove it
+        if condition['tests'] and condition['fields_ids']:
+            yield condition
+
 
+def contextualize(form, role):
+    """
+    Transform a FormidableJSON into a ContextFormJSON for a given role.
+
+    """
+    form = copy.deepcopy(form)
+    form['fields'] = list(contextualize_fields(form['fields'], role))
+    form['conditions'] = list(contextualize_conditions(form))
     return form
