Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Questionnaire API #938

Merged
merged 6 commits into from
Dec 6, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions cadasta/config/permissions/data-collector.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@
"effect": "allow",
"action": ["tenure_rel.*", "tenure_rel.resources.*"],
"object": ["tenure_rel/$organization/$project/*"]
},
{
"effect": "allow",
"action": ["questionnaire.view"],
"object": ["project/$organization/$project"]
}
]
}
6 changes: 5 additions & 1 deletion cadasta/config/permissions/superuser.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,11 @@
"action": ["resource.*"],
"object": ["project/*/*", "resource/*/*/*"]
},

{
"effect": "allow",
"action": ["questionnaire.*"],
"object": ["project/*/*"]
},
{
"effect": "allow",
"action": ["spatial.*"],
Expand Down
21 changes: 13 additions & 8 deletions cadasta/config/settings/default.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,23 +318,28 @@
ATTRIBUTE_GROUPS = {
'location_attributes': {
'app_label': 'spatial',
'model': 'spatialunit'
},
'location_relationship_attributes': {
'app_label': 'spatial',
'model': 'spatialrelationship'
'model': 'spatialunit',
'label': 'Location'
},
'party_attributes': {
'app_label': 'party',
'model': 'party'
'model': 'party',
'label': 'Party'
},
'location_relationship_attributes': {
'app_label': 'spatial',
'model': 'spatialrelationship',
'label': 'Spatial relationship'
},
'party_relationship_attributes': {
'app_label': 'party',
'model': 'partyrelationship'
'model': 'partyrelationship',
'label': 'Party relationship'
},
'tenure_relationship_attributes': {
'app_label': 'party',
'model': 'tenurerelationship'
'model': 'tenurerelationship',
'label': 'Tenure Relationship'
}
}

Expand Down
2 changes: 1 addition & 1 deletion cadasta/core/views/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def eval_json(response_data):
for e in response_data[key]:
try:
errors.append(json.loads(e))
except ValueError:
except (ValueError, TypeError):
errors.append(e)
response_data[key] = errors

Expand Down
6 changes: 6 additions & 0 deletions cadasta/organization/models.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from django.utils.functional import cached_property
from django.core.urlresolvers import reverse
from django.conf import settings
from django.db import models
Expand Down Expand Up @@ -267,6 +268,11 @@ def ui_detail_url(self):
},
)

@cached_property
def has_records(self):
check_records = ['parties', 'tenure_relationships', 'spatial_units']
return any([getattr(self, r).exists() for r in check_records])

def save(self, *args, **kwargs):
if ((self.country is None or self.country == '') and
self.extent is not None):
Expand Down
9 changes: 9 additions & 0 deletions cadasta/organization/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from core.tests.utils.cases import UserTestCase
from accounts.tests.factories import UserFactory
from geography import load as load_countries
from spatial.tests.factories import SpatialUnitFactory
from .factories import OrganizationFactory, ProjectFactory
from ..models import OrganizationRole, ProjectRole

Expand Down Expand Up @@ -163,6 +164,14 @@ def test_ui_detail_url(self):
org=project.organization.slug,
prj=project.slug))

def test_has_records(self):
project = ProjectFactory.create()
assert project.has_records is False

project = ProjectFactory.create()
SpatialUnitFactory.create(project=project)
assert project.has_records is True


class ProjectRoleTest(UserTestCase, TestCase):
def setUp(self):
Expand Down
25 changes: 24 additions & 1 deletion cadasta/organization/tests/test_views_default_projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -934,7 +934,7 @@ class ProjectEditDetailsTest(ViewTestCase, UserTestCase,
}

def setup_models(self):
self.project = ProjectFactory.create()
self.project = ProjectFactory.create(current_questionnaire='abc')

def setup_url_kwargs(self):
return {
Expand All @@ -954,6 +954,17 @@ def test_get_with_authorized_user(self):
response = self.request(user=user)
assert response.status_code == 200
assert response.content == self.expected_content
assert 'Select the questionnaire' in self.expected_content

def test_get_with_blocked_questionnaire_upload(self):
user = UserFactory.create()
assign_policies(user)
SpatialUnitFactory.create(project=self.project)

response = self.request(user=user)
assert response.status_code == 200
assert response.content == self.expected_content
assert 'Select the questionnaire' not in self.expected_content

def test_get_with_authorized_user_include_questionnaire(self):
questionnaire = QuestionnaireFactory.create(project=self.project)
Expand Down Expand Up @@ -1000,6 +1011,18 @@ def test_post_with_authorized_user(self):
assert self.project.name == self.post_data['name']
assert self.project.description == self.post_data['description']

def test_post_with_blocked_questionnaire_upload(self):
SpatialUnitFactory.create(project=self.project)
user = UserFactory.create()
assign_policies(user)
response = self.request(user=user, method='POST')

assert response.status_code == 200
self.project.refresh_from_db()
assert self.project.name != self.post_data['name']
assert self.project.description != self.post_data['description']
assert self.project.current_questionnaire == 'abc'

def test_post_invalid_form(self):
question = self.get_form('xls-form-invalid')
user = UserFactory.create()
Expand Down
17 changes: 10 additions & 7 deletions cadasta/organization/views/default.py
Original file line number Diff line number Diff line change
Expand Up @@ -651,13 +651,16 @@ def get_initial(self):
return initial

def post(self, *args, **kwargs):
try:
return super().post(*args, **kwargs)
except InvalidXLSForm as e:
form = self.get_form()
for err in e.errors:
form.add_error('questionnaire', err)
return self.form_invalid(form)
if self.get_project().has_records:
return self.get(*args, **kwargs)
else:
try:
return super().post(*args, **kwargs)
except InvalidXLSForm as e:
form = self.get_form()
for err in e.errors:
form.add_error('questionnaire', err)
return self.form_invalid(form)


class ProjectEditPermissions(ProjectEdit, generic.UpdateView):
Expand Down
93 changes: 33 additions & 60 deletions cadasta/questionnaires/managers.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
import hashlib
import itertools
import os
import re
from datetime import datetime

from lxml import etree
from xml.dom.minidom import Element

from django.apps import apps
from django.conf import settings
from django.contrib.contenttypes.models import ContentType
Expand All @@ -25,11 +19,8 @@
def create_children(children, errors=[], project=None,
default_language='', kwargs={}):
if children:
for c in children:
if c.get('type') == 'repeat':
create_children(c['children'], errors, project,
default_language, kwargs)
elif c.get('type') == 'group':
for c, idx in zip(children, itertools.count()):
if c.get('type') in ['group', 'repeat']:
model_name = 'QuestionGroup'

# parse attribute group
Expand All @@ -49,9 +40,11 @@ def create_children(children, errors=[], project=None,
else:
model_name = 'Question'

if c.get('type') != 'repeat':
model = apps.get_model('questionnaires', model_name)
model.objects.create_from_dict(dict=c, errors=errors, **kwargs)
model = apps.get_model('questionnaires', model_name)
model.objects.create_from_dict(dict=c,
index=idx,
errors=errors,
**kwargs)


def create_options(options, question, errors=[]):
Expand Down Expand Up @@ -215,26 +208,10 @@ def create_from_form(self, xls_form=None, original_file=None,
instance.filename = json.get('name')
instance.title = json.get('title')
instance.id_string = json.get('id_string')
instance.version = int(
datetime.utcnow().strftime('%Y%m%d%H%M%S%f')[:-4]
)
instance.md5_hash = self.get_hash(
instance.filename, instance.id_string, instance.version
)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines 209 - 211 are not required here, they've been moved to the render method in the XFormRenderer.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I left it in there on purpose because this is the part that throws exceptions that are shown to the user as error messages. If we remove it here, we will have to rewrite error handling for XLSForms. I'm not sure if it is worth doing it, since we are thinking about replacing XLSForms.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok.. sounds good.

survey = create_survey_element_from_dict(json)
xml_form = survey.xml()
fix_languages(xml_form)
xml_form = xml_form.toxml()
# insert version attr into the xform instance root node
xml = self.insert_version_attribute(
xml_form, instance.filename, instance.version
)
name = os.path.join(instance.xml_form.field.upload_to,
os.path.basename(instance.filename))
url = instance.xml_form.storage.save(
'{}.xml'.format(name), xml)
instance.xml_form = url

instance.save()

Expand All @@ -258,38 +235,31 @@ def create_from_form(self, xls_form=None, original_file=None,
except PyXFormError as e:
raise InvalidXLSForm([str(e)])

def get_hash(self, filename, id_string, version):
string = str(filename) + str(id_string) + str(version)
return hashlib.md5(string.encode()).hexdigest()

def insert_version_attribute(self, xform, root_node, version):
ns = {'xf': 'http://www.w3.org/2002/xforms'}
root = etree.fromstring(xform)
inst = root.find(
'.//xf:instance/xf:{root_node}'.format(
root_node=root_node
), namespaces=ns
)
inst.set('version', str(version))
xml = etree.tostring(
root, method='xml', encoding='utf-8', pretty_print=True
)
return xml


class QuestionGroupManager(models.Manager):

def create_from_dict(self, dict=None, questionnaire=None, errors=[]):
instance = self.model(questionnaire=questionnaire)
def create_from_dict(self, dict=None, question_group=None,
questionnaire=None, errors=[], index=0):
instance = self.model(questionnaire=questionnaire,
question_group=question_group)

relevant = None
bind = dict.get('bind')
if bind:
relevant = bind.get('relevant', None)

instance.name = dict.get('name')
instance.label_xlat = dict.get('label', {})
instance.type = dict.get('type')
instance.relevant = relevant
instance.index = index
instance.save()

create_children(
children=dict.get('children'),
errors=errors,
project=questionnaire.project,
default_language=questionnaire.default_language,
kwargs={
'questionnaire': questionnaire,
'question_group': instance
Expand All @@ -301,24 +271,27 @@ def create_from_dict(self, dict=None, questionnaire=None, errors=[]):

class QuestionManager(models.Manager):

def create_from_dict(self, errors=[], **kwargs):
def create_from_dict(self, errors=[], index=0, **kwargs):
dict = kwargs.pop('dict')
instance = self.model(**kwargs)
type_dict = {name: code for code, name in instance.TYPE_CHOICES}

instance.type = type_dict[dict.get('type')]

# try:
# instance.type = type_dict[dict.get('type')]
# except KeyError as e:
# errors.append(
# _('{type} is not an accepted question type').format(type=e)
# )
relevant = None
required = False
bind = dict.get('bind')
if bind:
relevant = bind.get('relevant', None)
required = True if bind.get('required', 'no') == 'yes' else False

instance.type = type_dict[dict.get('type')]
instance.name = dict.get('name')
instance.label_xlat = dict.get('label', {})
instance.required = dict.get('required', False)
instance.required = required
instance.constraint = dict.get('constraint')
instance.default = dict.get('default', None)
instance.hint = dict.get('hint', None)
instance.relevant = relevant
instance.index = index
instance.save()

if instance.has_options:
Expand Down
Loading