Merge pull request #287 from peopledoc/allow-wrong-fields-ids

Do not raise an exception when conditional field is not visible

Author: alexdashkov

CHANGELOG.rst

@@ -5,7 +5,7 @@ ChangeLog
 master (unreleased)
 ===================
 
-Nothing here yet.
+- Allow wrong field ids in conditions
 
 Release 1.1.0 (2017-12-04)
 ==========================

demo/tests/fixtures/wrong-conditions.json

@@ -0,0 +1,47 @@
+{
+  "conditions": [
+    {
+      "action": "display_iff",
+      "fields_ids": [
+        "third-field",
+        "fourth-field"
+      ],
+      "name": "Rule",
+      "tests": [
+        {
+          "field_id": "test-field",
+          "operator": "eq",
+          "values": [
+            true
+          ]
+        }
+      ]
+    }
+  ],
+  "description": "Description",
+  "fields": [
+    {
+      "defaults": [],
+      "description": "first field",
+      "disabled": false,
+      "placeholder": null,
+      "required": false,
+      "slug": "first-field",
+      "type_id": "help_text",
+      "validations": []
+    },
+    {
+      "defaults": [],
+      "description": null,
+      "disabled": false,
+      "label": "second field",
+      "placeholder": null,
+      "required": false,
+      "slug": "file",
+      "type_id": "file",
+      "validations": []
+    }
+  ],
+  "id": 1,
+  "label": "Some label here"
+}

demo/tests/test_conditions.py

@@ -2,8 +2,12 @@
 
 from __future__ import unicode_literals
 
+import os
+import json
+
 from django.test import TestCase
 
+
 from formidable import constants
 from formidable.forms import (
     FormidableForm, fields, get_dynamic_form_class,
@@ -13,6 +17,8 @@
     ContextFormSerializer, FormidableSerializer
 )
 
+TESTS_DIR = os.path.dirname(os.path.abspath(__file__))
+
 
 class ConditionTestCase(TestCase):
 
@@ -187,6 +193,32 @@ def test_readonly_field_not_displayed(self):
         self.assertTrue(form.is_valid(), form.errors)
         self.assertEqual(form.cleaned_data, {'checkbox': True})
 
+    def test_condition_field_doesnt_exist(self):
+        """
+        This test checks situation when the conditional fields don't exists
+        in the list of fields.
+        it shows that we don't raise an Exception anymore.
+
+        You could get this situation when you specified the fields access
+        that current user doesn't have an access to the conditional field.
+
+        In the fixture 'wrong-conditions.json' you see this situation.
+        Conditions are configured for the 'test-field' but current user
+        doesn't have rights to read or write to it. So can't see this field
+        in the 'fields' section.
+        """
+        schema = json.load(open(
+            os.path.join(
+                TESTS_DIR, 'fixtures', 'wrong-conditions.json'
+            )
+        ))
+        try:
+            get_dynamic_form_class_from_schema(schema)
+        except KeyError:
+            self.fail("Doesn't have to raise an exception here ")
+        else:
+            self.assertTrue(True)
+
 
 class ConditionFromSchemaTestCase(ConditionTestCase):
 

formidable/forms/conditions.py

@@ -39,8 +39,12 @@ def gen_from_schema(self, fields, conditions_schema):
 
             # cast values to field's type
             def convert_values(field_id, values):
-                field = fields[field_id]
-                return [field.to_python(value) for value in values]
+                try:
+                    field = fields[field_id]
+                except KeyError:
+                    return []
+                else:
+                    return [field.to_python(value) for value in values]
 
             tests = [ConditionTest(test['field_id'],
                                    test['operator'],

tox.ini

@@ -16,6 +16,7 @@ deps =
     django18: Django>=1.8,<1.9
     django18: djangorestframework<3.7
     django19: Django>=1.9,<1.10
+    django19: djangorestframework<3.7
     django110: Django>=1.10,<1.11
     ; Requirements from demo project
     -rdemo/requirements-demo.pip