Skip to content

Commit

Permalink
Revert "Fix #1638: Check for xlsform relevant clause syntax errors (#…
Browse files Browse the repository at this point in the history
…1641)"

This reverts commit dc0d717.
  • Loading branch information
oliverroick committed Aug 3, 2017
1 parent 60a42c9 commit e441ec7
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 176 deletions.
13 changes: 1 addition & 12 deletions cadasta/questionnaires/managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,6 @@
ATTRIBUTE_GROUPS = settings.ATTRIBUTE_GROUPS


def check_relevant_clause(relevant):
if not re.match(r"^\$\{\w+\}=('|\"|”)\w+('|\"|”)$", relevant):
raise InvalidQuestionnaire(
[_("Invalid relevant clause: {0}".format(relevant))])


def create_children(children, errors=[], project=None,
default_language='', kwargs={}):
if children:
Expand Down Expand Up @@ -89,9 +83,8 @@ def create_attrs_schema(project=None, dict=None, content_type=None,
if bind:
relevant = bind.get('relevant', None)
if relevant:
check_relevant_clause(relevant)
clauses = relevant.split('=')
selector = re.sub("('|\"|”)", '', clauses[1])
selector = re.sub("'", '', clauses[1])
selectors += (selector,)

try:
Expand Down Expand Up @@ -274,8 +267,6 @@ def create_from_dict(self, dict=None, question_group=None,
bind = dict.get('bind')
if bind:
relevant = bind.get('relevant', None)
if relevant:
check_relevant_clause(relevant)

instance.name = dict.get('name')
instance.label_xlat = dict.get('label', {})
Expand Down Expand Up @@ -310,8 +301,6 @@ def create_from_dict(self, errors=[], index=0, **kwargs):
bind = dict.get('bind')
if bind:
relevant = bind.get('relevant', None)
if relevant:
check_relevant_clause(relevant)
required = True if bind.get('required', 'no') == 'yes' else False

instance.type = type_dict[dict.get('type')]
Expand Down
17 changes: 6 additions & 11 deletions cadasta/questionnaires/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from .messages import MISSING_RELEVANT
from .exceptions import InvalidQuestionnaire
from .validators import validate_questionnaire
from .managers import fix_labels, check_relevant_clause
from .managers import fix_labels
from . import models


Expand Down Expand Up @@ -94,18 +94,15 @@ def to_representation(self, instance):
def create(self, validated_data):
initial_data = self.find_initial_data(validated_data['name'])
validated_data['label_xlat'] = initial_data['label']
relevant = initial_data.get('relevant', None)
if relevant:
check_relevant_clause(relevant)
question = models.Question.objects.create(
questionnaire_id=self.context['questionnaire_id'],
question_group_id=self.context.get('question_group_id'),
**validated_data)

option_serializer = QuestionOptionSerializer(
data=initial_data.get('options', []),
many=True,
context={'question_id': question.id})
data=initial_data.get('options', []),
many=True,
context={'question_id': question.id})

option_serializer.is_valid(raise_exception=True)
option_serializer.save()
Expand All @@ -120,7 +117,7 @@ class QuestionGroupSerializer(FindInitialMixin, serializers.ModelSerializer):

class Meta:
model = models.QuestionGroup
fields = ('id', 'name', 'label', 'type', 'questions',
fields = ('id', 'name', 'label', 'type', 'questions',
'question_groups', 'label_xlat', 'relevant', 'index', )
read_only_fields = ('id', 'questions', 'question_groups', )
write_only_fields = ('label_xlat', )
Expand All @@ -142,7 +139,6 @@ def get_question_groups(self, group):
def create(self, validated_data):
initial_data = self.find_initial_data(validated_data['name'])
validated_data['label_xlat'] = initial_data['label']

group = models.QuestionGroup.objects.create(
questionnaire_id=self.context['questionnaire_id'],
question_group_id=self.context.get('question_group_id'),
Expand All @@ -166,9 +162,8 @@ def create(self, validated_data):

relevant = initial_data.get('relevant', None)
if relevant:
check_relevant_clause(relevant)
clauses = relevant.split('=')
selector = re.sub("('|\"|”)", '', clauses[1])
selector = re.sub("'", '', clauses[1])
selectors += (selector,)

app_label = ATTRIBUTE_GROUPS[attr_group]['app_label']
Expand Down
80 changes: 2 additions & 78 deletions cadasta/questionnaires/tests/test_managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,10 @@

from . import factories
from .. import models
from ..managers import (create_children, create_options, santize_form,
check_relevant_clause)
from ..managers import create_children, create_options, santize_form
from ..messages import MISSING_RELEVANT


class RelevantSyntaxValidationTest(TestCase):
def test_relevant_syntax_valid(self):
assert check_relevant_clause("${party_type}='IN'") is None

def test_relevant_syntax_invalid(self):
relevant = "${party_type='IN'}"
with pytest.raises(InvalidQuestionnaire) as e:
check_relevant_clause(relevant)
assert str(e.value) == "Invalid relevant clause: ${party_type='IN'}"


class SanitizeFormTest(TestCase):
def test_santize_valid(self):
data = {
Expand Down Expand Up @@ -179,34 +167,6 @@ def test_create_children_with_repeat_group(self):
questionnaire=questionnaire,
question_group__isnull=False).count() == 2

def test_invalid_relevant_clause_in_children(self):
questionnaire = factories.QuestionnaireFactory.create()
children = [{
'label': 'This form showcases the different question',
'name': 'intro',
'type': 'note'
}, {
'label': 'Text question type',
'name': 'text_questions',
'type': 'group',
'children': [
{
'hint': 'Can be short or long but '
'always one line (type = '
'text)',
'label': 'Text',
'name': 'my_string',
'type': 'text',
'bind': {'relevant': '$geo_type="geoshape"'} # invalid
}
],
}]

with pytest.raises(InvalidQuestionnaire) as e:
create_children(children, kwargs={'questionnaire': questionnaire})

assert str(e.value) == 'Invalid relevant clause: $geo_type="geoshape"'


class CreateOptionsTest(TestCase):

Expand Down Expand Up @@ -344,7 +304,7 @@ def test_create_from_dict(self):
'label': 'Basic Select question types',
'name': 'select_questions',
'type': 'group',
'bind': {'relevant': "${party_type}=”IN”"}
'bind': {'relevant': "${party_type}='IN'"}
}
questionnaire = factories.QuestionnaireFactory.create()

Expand Down Expand Up @@ -383,21 +343,6 @@ def test_create_nested_group_from_dict(self):
assert model.question_groups.count() == 1
assert questionnaire.question_groups.count() == 2

def test_invalid_relevant_clause(self):
question_group_dict = {
'label': 'Basic Select question types',
'name': 'select_questions',
'type': 'group',
'bind': {'relevant': "$party_type='IN'"} # invalid
}
questionnaire = factories.QuestionnaireFactory.create()
with pytest.raises(InvalidQuestionnaire) as e:
models.QuestionGroup.objects.create_from_dict(
dict=question_group_dict,
questionnaire=questionnaire
)
assert str(e.value) == "Invalid relevant clause: $party_type='IN'"


class QuestionManagerTest(TestCase):

Expand Down Expand Up @@ -453,27 +398,6 @@ def test_create_from_dict_with_group(self):
assert model.name == question_dict['name']
assert model.type == 'IN'

def test_invalid_relevant_clause(self):
question_dict = {
'hint': 'For this field (type=integer)',
'label': 'Integer',
'name': 'my_int',
'type': 'integer',
'default': 'default val',
'hint': 'An informative hint',
'bind': {
'relevant': "$party_id='abc123'", # invalid
'required': 'yes'
}
}
questionnaire = factories.QuestionnaireFactory.create()
with pytest.raises(InvalidQuestionnaire) as e:
models.Question.objects.create_from_dict(
dict=question_dict,
questionnaire=questionnaire
)
assert str(e.value) == "Invalid relevant clause: $party_id='abc123'"


class MultilingualQuestionnaireTest(UserTestCase, FileStorageTestCase,
TestCase):
Expand Down
76 changes: 1 addition & 75 deletions cadasta/questionnaires/tests/test_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ def test_huge(self):
"default": None,
"hint": None,
"index": 1,
"relevant": None
"relevant": "${start}>='IN'"
},
{
"id": "rw7mt32858cu2w5urbf9z3a4",
Expand Down Expand Up @@ -593,9 +593,6 @@ def test_huge(self):
"label": "Label",
"type": 'group',
"index": 13,
"bind": {
"relevant": "$party_type='IN'"
},
"questions": [
{
"id": "8v5znbuyvtyinsdd96ytyrui",
Expand Down Expand Up @@ -962,37 +959,6 @@ def test_huge(self):
assert questionnaire.question_groups.count() == 7
assert Attribute.objects.count() == 13

def test_invalid_relevant_clause(self):
data = {
'title': 'yx8sqx6488wbc4yysnkrbnfq',
'id_string': 'yx8sqx6488wbc4yysnkrbnfq',
'default_language': 'en',
'questions': [{
'name': "start",
'label': 'Label',
'type': "ST",
'required': False,
'constraint': None,
'index': 0,
'relevant': "$party_type='IN'"
}, {
'name': "end",
'label': 'Label',
'type': "EN",
'index': 1
}]
}
project = ProjectFactory.create()

serializer = serializers.QuestionnaireSerializer(
data=data,
context={'project': project}
)
serializer.is_valid(raise_exception=True)
with pytest.raises(InvalidQuestionnaire) as e:
serializer.save()
assert str(e.value) == "Invalid relevant clause: $party_type='IN'"


class QuestionGroupSerializerTest(UserTestCase, TestCase):
def test_serialize(self):
Expand Down Expand Up @@ -1221,30 +1187,6 @@ def test_create_numeric_attribute_with_default_0(self):
assert questionnaire.question_groups.count() == 1
assert Attribute.objects.get(name='number').default == '0'

def test_question_group_with_invalid_relevant(self):
questionnaire = factories.QuestionnaireFactory.create()
data = [{
'label': 'A group',
'name': 'party_attributes_individual',
"relevant": "$party_type='IN'",
'questions': [{
'name': "start",
'label': 'Start',
'type': "TX",
'index': 0
}]
}]
serializer = serializers.QuestionGroupSerializer(
data=data,
many=True,
context={'questionnaire_id': questionnaire.id,
'project': questionnaire.project,
'default_language': 'en'})
serializer.is_valid(raise_exception=True)
with pytest.raises(InvalidQuestionnaire) as e:
serializer.save()
assert str(e.value) == "Invalid relevant clause: $party_type='IN'"


class QuestionSerializerTest(TestCase):
def test_serialize(self):
Expand Down Expand Up @@ -1384,22 +1326,6 @@ def test_bulk_create(self):
assert q.type == 'S1'
assert q.options.count() == 1

def test_create_question_with_invalid_relevant(self):
questionnaire = factories.QuestionnaireFactory.create()
data = {
'label': 'A question',
'name': 'question',
'type': 'TX',
'relevant': "$party_type='IN'"
}
serializer = serializers.QuestionSerializer(
data=data,
context={'questionnaire_id': questionnaire.id})
serializer.is_valid(raise_exception=True)
with pytest.raises(InvalidQuestionnaire) as e:
serializer.save()
assert str(e.value) == "Invalid relevant clause: $party_type='IN'"


class QuestionOptionSerializerTest(TestCase):
def test_serialize(self):
Expand Down

0 comments on commit e441ec7

Please sign in to comment.