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

Add support for defining Entity properties within grouped sections #2644

Merged
merged 9 commits into from
Jul 16, 2024
Merged
Show file tree
Hide file tree
Changes from 8 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
2 changes: 1 addition & 1 deletion onadata/apps/api/tests/viewsets/test_attachment_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def setUp(self):

self._publish_xls_form_to_project()

@flaky(max_runs=3)
@flaky(max_runs=10)
def test_retrieve_view(self):
self._submit_transport_instance_w_attachment()

Expand Down
18 changes: 13 additions & 5 deletions onadata/apps/logger/models/registration_form.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,20 @@ def get_save_to(self, version: str | None = None) -> dict[str, str]:
xform_json = self.xform.json

result = {}
fields = xform_json.get("children", [])
entity_properties = filter(
lambda field: "bind" in field and "entities:saveto" in field["bind"], fields
)
children = xform_json.get("children", [])

def get_entity_property_fields(form_fields):
property_fields = []

for field in form_fields:
if "bind" in field and "entities:saveto" in field["bind"]:
property_fields.append(field)
elif field.get("children", []):
property_fields += get_entity_property_fields(field["children"])

return property_fields

for field in entity_properties:
for field in get_entity_property_fields(children):
alias = field["bind"]["entities:saveto"]
result[alias] = field["name"]

Expand Down
32 changes: 28 additions & 4 deletions onadata/apps/logger/tests/models/test_registration_form.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from django.db.utils import IntegrityError

from onadata.apps.main.tests.test_base import TestBase
from onadata.apps.logger.models import RegistrationForm, EntityList, XFormVersion
from onadata.apps.logger.models import RegistrationForm, EntityList, XForm, XFormVersion


class RegistrationFormTestCase(TestBase):
Expand Down Expand Up @@ -45,12 +45,12 @@ def test_creation(self, mock_now):

def test_get_save_to(self):
"""Method `get_save_to` works correctly"""
form = RegistrationForm.objects.create(
registration_form = RegistrationForm.objects.create(
entity_list=self.entity_list,
xform=self.xform,
)
self.assertEqual(
form.get_save_to(),
registration_form.get_save_to(),
{
"geometry": "location",
"species": "species",
Expand Down Expand Up @@ -132,13 +132,37 @@ def test_get_save_to(self):
json=json.dumps(x_version_json),
)
self.assertEqual(
form.get_save_to("x"),
registration_form.get_save_to("x"),
{
"location": "location",
"species": "species",
"circumference": "circumference",
},
)
# Properties within grouped sections
group_md = """
| survey |
| | type | name | label | save_to |
| | begin group | tree | Tree | |
| | geopoint | location | Location | geometry|
| | text | species | Species | species |
| | end group | | | |
| settings| | | | |
| | form_title | form_id | instance_name| version |
| | Group | group | ${species} | 2022110901|
| entities| list_name | label | | |
| | trees | ${species}| | |
"""
self._publish_markdown(group_md, self.user, self.project, id_string="group")
xform = XForm.objects.get(id_string="group")
registration_form = RegistrationForm.objects.get(
xform=xform, entity_list=self.entity_list
)

self.assertEqual(
registration_form.get_save_to(),
{"geometry": "location", "species": "species"},
)

def test_entity_list_xform_unique(self):
"""No duplicates allowed for existing entity_list and xform"""
Expand Down
76 changes: 71 additions & 5 deletions onadata/libs/tests/utils/test_logger_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,14 @@
from defusedxml.ElementTree import ParseError

from onadata.apps.logger.import_tools import django_file
from onadata.apps.logger.models import Instance, Entity, RegistrationForm, SurveyType
from onadata.apps.logger.models import (
Instance,
Entity,
EntityList,
RegistrationForm,
SurveyType,
XForm,
)
from onadata.apps.logger.xform_instance_parser import AttachmentNameError
from onadata.apps.main.tests.test_base import TestBase
from onadata.libs.test_utils.pyxform_test_case import PyxformTestCase
Expand Down Expand Up @@ -651,8 +658,8 @@ def test_handle_parse_error(self):
self.assertContains(ret[0].content.decode(), "Improperly formatted XML.")


class CreateEntityTestCase(TestBase):
"""Tests for method `create_entity`"""
class CreateEntityFromInstanceTestCase(TestBase):
"""Tests for method `create_entity_from_instance`"""

def setUp(self):
super().setUp()
Expand All @@ -675,17 +682,18 @@ def setUp(self):
"</meta>"
"</data>"
)
survey_type = SurveyType.objects.create(slug="slug-foo")
self.survey_type = SurveyType.objects.create(slug="slug-foo")
instance = Instance(
xform=self.xform,
xml=self.xml,
version=self.xform.version,
survey_type=survey_type,
survey_type=self.survey_type,
)
# We use bulk_create to avoid calling create_entity signal
Instance.objects.bulk_create([instance])
self.instance = Instance.objects.first()
self.registration_form = RegistrationForm.objects.first()
self.entity_list = EntityList.objects.get(name="trees")

def test_entity_created(self):
"""Entity is created successfully"""
Expand Down Expand Up @@ -721,3 +729,61 @@ def test_entity_created(self):
self.assertEqual(entity_history.json, expected_json)
self.assertEqual(entity_history.form_version, self.xform.version)
self.assertEqual(entity_history.created_by, self.instance.user)

def test_grouped_section(self):
"""Entity properties within grouped section"""
group_md = """
| survey |
| | type | name | label | save_to |
| | begin group | tree | Tree | |
| | geopoint | location | Location | geometry|
| | text | species | Species | species |
| | end group | | | |
| settings| | | | |
| | form_title | form_id | instance_name| version |
| | Group | group | ${species} | 2022110901|
| entities| list_name | label | | |
| | trees | ${species}| | |
"""
self._publish_markdown(group_md, self.user, self.project, id_string="group")
xform = XForm.objects.get(id_string="group")
xml = (
'<?xml version="1.0" encoding="UTF-8"?>'
'<data xmlns:jr="http://openrosa.org/javarosa" xmlns:orx='
'"http://openrosa.org/xforms" id="group" version="2022110901">'
"<formhub><uuid>9833e23e6c6147298e0ae2d691dc1e6f</uuid></formhub>"
"<tree>"
"<location>-1.286905 36.772845 0 0</location>"
"<species>purpleheart</species>"
"</tree>"
"<meta>"
"<instanceID>uuid:b817c598-a215-4fa9-ba78-a7c738bd1f91</instanceID>"
"<instanceName>purpleheart</instanceName>"
'<entity create="1" dataset="trees" id="47e335da-46ce-4151-9898-7ed1d54778c6">'
"<label>purpleheart</label>"
"</entity>"
"</meta>"
"</data>"
)
instance = Instance(
xform=xform,
xml=xml,
version=xform.version,
survey_type=self.survey_type,
)
# We use bulk_create to avoid calling create_entity signal
Instance.objects.bulk_create([instance])
instance = Instance.objects.order_by("pk").last()
registration_form = RegistrationForm.objects.get(
xform=xform, entity_list=self.entity_list
)
create_entity_from_instance(instance, registration_form)
entity = Entity.objects.first()
expected_json = {
"geometry": "-1.286905 36.772845 0 0",
"species": "purpleheart",
"label": "purpleheart",
}

self.assertEqual(Entity.objects.count(), 1)
self.assertCountEqual(entity.json, expected_json)
50 changes: 21 additions & 29 deletions onadata/libs/utils/logger_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -994,46 +994,38 @@ def get_entity_json_from_instance(
# Field names with an alias defined
property_fields = list(mapped_properties.values())

def convert_to_alias(field_name: str) -> str:
"""Convert field name to it's alias"""
alias_field_name = field_name
# We split along / to take care of group questions
parts = field_name.split("/")
# Replace field parts with alias
for part in parts:
if part in property_fields:
for alias, field in mapped_properties.items():
if field == part:
alias_field_name = alias_field_name.replace(field, alias)
break

return alias_field_name
def get_field_alias(field_name: str) -> str:
"""Get the alias (save_to value) of a form field"""
for alias, field in mapped_properties.items():
if field == field_name:
return alias

return field_name

def parse_instance_json(data: dict[str, Any]) -> None:
"""Parse the original json, replacing field names with their alias

The data keys are modified in place
"""
for field_name in list(data):
if isinstance(data[field_name], list):
# Handle repeat question
for child_data in data[field_name]:
parse_instance_json(child_data)
temp = data[field_name]
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we rename temp var to be more descriptive?

del data[field_name]

else:
if field_name in property_fields:
alias_field_name = convert_to_alias(field_name)
if field_name.startswith("formhub"):
continue

if field_name.startswith("meta"):
if field_name == "meta/entity/label":
data["label"] = temp

if alias_field_name != field_name:
data[alias_field_name] = data[field_name]
del data[field_name]
continue

elif field_name == "meta/entity/label":
data["label"] = data["meta/entity/label"]
del data["meta/entity/label"]
# We extract field names within grouped sections
ungrouped_field_name = field_name.split("/")[-1]

else:
del data[field_name]
if ungrouped_field_name in property_fields:
field_alias = get_field_alias(ungrouped_field_name)
data[field_alias] = temp

parse_instance_json(instance_json)

Expand Down