fix: rename form_fields --> formFields and handle all http errors (#230)

* fix: rename form_fields --> formFields, catch http errors

* test: update central tests with cleanup + formFields test

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."""

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="[email protected]",

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,
+    }