diff --git a/osm_fieldwork/OdkCentral.py b/osm_fieldwork/OdkCentral.py index b75a492e..412af735 100755 --- a/osm_fieldwork/OdkCentral.py +++ b/osm_fieldwork/OdkCentral.py @@ -1043,7 +1043,6 @@ def createForm( return form_name new_form_name = json_data.get("xmlFormId") - log.warning(json_data) log.debug(f"Creating XForm on ODK server: ({new_form_name})") return new_form_name @@ -1115,7 +1114,7 @@ def publishForm( return result.status_code - def form_fields(self, projectId: int, xform: str): + def formFields(self, projectId: int, xform: str): """Retrieves the form fields for a xform from odk central. Args: @@ -1127,8 +1126,18 @@ def form_fields(self, projectId: int, xform: str): """ url = f"{self.base}projects/{projectId}/forms/{xform}/fields?odata=true" - result = self.session.get(url, verify=self.verify) - return result.json() + response = self.session.get(url, verify=self.verify) + + # TODO wrap this logic and put in every method requiring form name + if response.status_code != 200: + if response.status_code == 404: + msg = f"The ODK form you referenced does not exist yet: {xform}" + log.debug(msg) + raise requests.exceptions.HTTPError(msg) + log.debug(f"Failed to retrieve form fields. Status code: {response.status_code}") + response.raise_for_status() + + return response.json() def dump(self): """Dump internal data structures, for debugging purposes only.""" diff --git a/tests/conftest.py b/tests/conftest.py index 51d10aec..ed51118d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -20,6 +20,7 @@ import logging import sys import uuid +from pathlib import Path import pytest @@ -33,6 +34,8 @@ stream=sys.stdout, ) +testdata_dir = Path(__file__).parent / "testdata" + # from sqlalchemy import create_engine # from sqlalchemy.orm import sessionmaker @@ -134,9 +137,28 @@ def odk_form(project_details) -> tuple: return odk_id, form +@pytest.fixture(scope="function") +def odk_form_cleanup(odk_form): + """Get xform for project, with automatic cleanup after.""" + odk_id, xform = odk_form + test_xform = testdata_dir / "buildings.xml" + + # Create form + form_name = xform.createForm(odk_id, str(test_xform)) + assert form_name == "test_form" + + # Before yield is used in tests + yield odk_id, form_name, xform + # After yield is test cleanup + + # Delete form + success = xform.deleteForm(odk_id, "test_form") + assert success + + @pytest.fixture(scope="session", autouse=True) def cleanup(): - """Cleanup projects and forms after tests.""" + """Cleanup projects and forms after all tests (session).""" project = OdkProject( url="https://proxy", user="test@hotosm.org", diff --git a/tests/test_central.py b/tests/test_central.py index e7f3bfab..2dfcd34a 100644 --- a/tests/test_central.py +++ b/tests/test_central.py @@ -20,6 +20,8 @@ from io import BytesIO from pathlib import Path +import pytest +import requests import segno testdata_dir = Path(__file__).parent / "testdata" @@ -91,24 +93,14 @@ def test_create_form_delete(project, odk_form): assert len(project.listForms(odk_id)) == 0 -def test_create_form_and_publish(project, odk_form): +def test_create_form_and_publish(project, odk_form_cleanup): """Create form and publish.""" - odk_id, xform = odk_form - test_xform = testdata_dir / "buildings.xml" - - form_name = xform.createForm(odk_id, str(test_xform)) - assert form_name == "test_form" + odk_id, form_name, xform = odk_form_cleanup response_code = xform.publishForm(odk_id, form_name) assert response_code == 200 assert xform.published == True - success = xform.deleteForm(odk_id, form_name) - assert success - - forms = project.listForms(odk_id) - assert len(forms) == 0 - def test_create_form_and_publish_immediately(project, odk_form): """Create form and publish immediately.""" @@ -125,14 +117,12 @@ def test_create_form_and_publish_immediately(project, odk_form): assert len(project.listForms(odk_id)) == 0 -def test_create_form_draft(project, odk_form): +def test_create_form_draft(project, odk_form_cleanup): """Create form draft from existing form.""" - odk_id, xform = odk_form + odk_id, original_form_name, xform = odk_form_cleanup test_xform = testdata_dir / "buildings.xml" - # Create original form - original_form_name = xform.createForm(odk_id, str(test_xform)) - assert original_form_name == "test_form" + # Check original form is not draft assert xform.draft == False # Publish original form @@ -162,21 +152,10 @@ def test_create_form_draft(project, odk_form): assert len(project.listForms(odk_id)) == 1 - # Delete published draft form - success = xform.deleteForm(odk_id, draft_form_name) - assert success - - assert len(project.listForms(odk_id)) == 0 - -def test_upload_media_filepath(project, odk_form): +def test_upload_media_filepath(project, odk_form_cleanup): """Create form and upload media.""" - odk_id, xform = odk_form - test_xform = testdata_dir / "buildings.xml" - - # Create form - form_name = xform.createForm(odk_id, str(test_xform)) - assert form_name == "test_form" + odk_id, form_name, xform = odk_form_cleanup # Publish form first response_code = xform.publishForm(odk_id, form_name) @@ -191,12 +170,6 @@ def test_upload_media_filepath(project, odk_form): ) assert result.status_code == 200 - # Delete form - success = xform.deleteForm(odk_id, "test_form") - assert success - - assert len(project.listForms(odk_id)) == 0 - def test_upload_media_bytesio_publish(project, odk_form): """Create form and upload media.""" @@ -225,3 +198,29 @@ def test_upload_media_bytesio_publish(project, odk_form): assert success assert len(project.listForms(odk_id)) == 0 + + +def test_form_fields_no_form(odk_form): + """Attempt usage of form_fields before form exists.""" + odk_id, xform = odk_form + with pytest.raises(requests.exceptions.HTTPError): + xform.formFields(odk_id, "test_form") + + +def test_form_fields(odk_form_cleanup): + """Test form fields for created form.""" + odk_id, form_name, xform = odk_form_cleanup + + # Get form fields + form_fields = xform.formFields(odk_id, form_name) + assert len(form_fields) == 66 + + sorted_form_fields = sorted(form_fields, key=lambda x: x["name"]) + buildings_heritage = sorted_form_fields[30] + assert buildings_heritage == { + "path": "/all/buildings/heritage", + "name": "heritage", + "type": "string", + "binary": None, + "selectMultiple": None, + }