Merge pull request #4828 from kobotoolbox/fix-xpath-keyerror

Fix 500 error when loading table view
kobotoolbox · Jan 30, 2024 · 45faa6c · 45faa6c
2 parents c9285ff + 54fd5d9
commit 45faa6c
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 6 deletions.
diff --git a/kpi/models/asset.py b/kpi/models/asset.py
@@ -549,12 +549,36 @@ def get_attachment_xpaths(self, deployed: bool = True) -> list:
         version = (
             self.latest_deployed_version if deployed else self.latest_version
         )
-        survey = version.to_formpack_schema()['content']['survey']
-        return [
-            q['$xpath']
-            for q in survey
-            if q['type'] in ATTACHMENT_QUESTION_TYPES
-        ]
+
+        if version:
+            content = version.to_formpack_schema()['content']
+        else:
+            content = self.content
+
+        survey = content['survey']
+
+        def _get_xpaths(survey_: dict) -> Optional[list]:
+            """
+            Returns an empty list if no questions that take attachments are
+            present. Returns `None` if XPath are missing from the survey
+            content
+            """
+            xpaths = []
+            for question in survey_:
+                if question['type'] not in ATTACHMENT_QUESTION_TYPES:
+                    continue
+                try:
+                    xpath = question['$xpath']
+                except KeyError:
+                    return None
+                xpaths.append(xpath)
+            return xpaths
+
+        if xpaths := _get_xpaths(survey):
+            return xpaths
+
+        self._insert_qpath(content)
+        return _get_xpaths(survey)
 
     def get_filters_for_partial_perm(
         self, user_id: int, perm: str = PERM_VIEW_SUBMISSIONS

diff --git a/kpi/tests/test_asset_content.py b/kpi/tests/test_asset_content.py
@@ -856,3 +856,22 @@ def test_populates_qpath_xpath_correctly():
     rs = asset.content['survey'][0:4]
     assert [rr['$qpath'] for rr in rs] == ['g1', 'g1-r1', 'g1-g2', 'g1-g2-r2']
     assert [rr['$xpath'] for rr in rs] == ['g1', 'g1/r1', 'g1/g2', 'g1/g2/r2']
+
+
+def test_return_xpaths_and_qpath_even_if_missing():
+    asset = Asset(content={
+        'survey': [
+            {'type': 'begin_group', 'name': 'g1'},
+            {'type': 'audio', 'name': 'r1', '$kuid': 'k1'},
+            {'type': 'begin_group', 'name': 'g2'},
+            {'type': 'image', 'name': 'r2', '$kuid': 'k2'},
+            {'type': 'end_group'},
+            {'type': 'end_group'},
+        ],
+    })
+    expected = ['g1/r1', 'g1/g2/r2']
+    # 'qpath' and 'xpath' are not injected until an Asset object is saved with `adjust_content=True`
+    # or `adjust_content_on_save()` is called directly.
+    # No matter what, `get_attachment_xpaths()` should be able to return
+    # attachment xpaths.
+    assert asset.get_attachment_xpaths(deployed=False) == expected