Add tests & cleanup

Signed-off-by: Kipchirchir Sigei <[email protected]>

onadata/apps/api/tests/viewsets/test_xform_viewset.py

@@ -32,6 +32,7 @@
 from flaky import flaky
 from httmock import HTTMock
 from mock import Mock, patch
+from onadata.libs.utils.api_export_tools import get_existing_file_format
 from rest_framework import status
 from rest_framework.viewsets import ModelViewSet
 
@@ -863,6 +864,39 @@ def test_form_format(self):
             )
             self.assertEqual(response_xml, expected_doc.toxml())
 
+    def test_existing_form_format(self):
+        with HTTMock(enketo_mock):
+            self._publish_xls_form_to_project()
+            view = XFormViewSet.as_view({"get": "form"})
+            formid = self.xform.pk
+            request = self.factory.get("/", **self.extra)
+            # get existing form format
+            exsting_format = get_existing_file_format(self.xform.xls, 'xls')
+
+            # XLSX format
+            response = view(request, pk=formid, format="xlsx")
+            self.assertEqual(response.status_code, 200)
+            self.assertNotEqual(response.get("Cache-Control"), None)
+
+            # test correct content disposition
+            # ensure it still maintains the existing form extension
+            self.assertEqual(
+                response.get("Content-Disposition"),
+                "attachment; filename=" + self.xform.id_string + "." + exsting_format,
+            )
+
+            # XLS format
+            response = view(request, pk=formid, format="xls")
+            self.assertEqual(response.status_code, 200)
+            self.assertNotEqual(response.get("Cache-Control"), None)
+
+            # test correct content disposition
+            # ensure it still maintains the existing form extension
+            self.assertEqual(
+                response.get("Content-Disposition"),
+                "attachment; filename=" + self.xform.id_string + "." + exsting_format,
+            )
+
     def test_form_tags(self):
         with HTTMock(enketo_mock):
             self._publish_xls_form_to_project()

onadata/apps/api/tools.py

@@ -24,6 +24,7 @@
 from guardian.shortcuts import get_perms, get_perms_for_model, remove_perm
 from kombu.exceptions import OperationalError
 from multidb.pinning import use_master
+from onadata.settings.common import XLS_EXTENSIONS
 from registration.models import RegistrationProfile
 from rest_framework import exceptions
 from six import iteritems

onadata/apps/api/viewsets/xform_viewset.py

@@ -77,6 +77,7 @@
 from onadata.libs.utils.api_export_tools import (
     custom_response_handler,
     get_async_response,
+    get_existing_file_format,
     process_async_export,
     response_for_format,
 )
@@ -448,11 +449,7 @@ def form(self, request, **kwargs):
         response = response_for_format(form, format=form_format)
 
         # add backward compatibility for existing .xls forms
-        if form_format in XLS_EXTENSIONS:
-            data = response.data
-            existing_file_format = data.name.split(".")[-1]
-            if existing_file_format == 'xls':
-                form_format = existing_file_format
+        form_format = get_existing_file_format(form.xls, form_format)
         filename = form.id_string + "." + form_format
         response["Content-Disposition"] = "attachment; filename=" + filename
 

onadata/libs/tests/utils/test_api_export_tools.py

@@ -19,7 +19,7 @@
 from onadata.apps.viewer.models.export import Export, ExportConnectionError
 from onadata.libs.exceptions import ServiceUnavailable
 from onadata.libs.utils.api_export_tools import (
-    get_async_response, process_async_export,
+    get_async_response, get_existing_file_format, process_async_export,
     response_for_format,
     get_metadata_format,
     _get_google_credential
@@ -223,6 +223,19 @@ def test_get_metadata_format(self):
         fmt = get_metadata_format(data_value)
         self.assertEqual(fmt, "csv")
 
+    def test_get_existing_file_format(self):
+        """
+        Test existing form download format/ext.
+        """
+        self._publish_xlsx_file()
+        xform = XForm.objects.filter().last()
+        fmt = get_existing_file_format(xform.xls, 'xlsx')
+        self.assertEqual("xlsx", fmt)
+        # ensure it picks existing file extension regardless
+        # of format passed in request params
+        fmt = get_existing_file_format(xform.xls, 'xls')
+        self.assertEqual("xlsx", fmt)
+
     # pylint: disable=invalid-name
     @mock.patch(
         'onadata.libs.utils.api_export_tools.viewer_task.create_async_export')

onadata/libs/utils/api_export_tools.py

@@ -20,6 +20,7 @@
 from celery.backends.rpc import BacklogLimitExceeded
 from celery.result import AsyncResult
 from kombu.exceptions import OperationalError
+from onadata.settings.common import XLS_EXTENSIONS
 from rest_framework import exceptions, status
 from rest_framework.response import Response
 from rest_framework.reverse import reverse
@@ -610,6 +611,16 @@ def response_for_format(data, format=None):
     return Response(formatted_data)
 
 
+def get_existing_file_format(data, format):
+    """
+    Util function to extract the existing form extension
+    """
+    if format in XLS_EXTENSIONS:
+        existing_file_format = data.name.split(".")[-1]
+        return existing_file_format
+    return format
+
+
 def generate_google_web_flow(request):
     """
     Returns a OAuth2WebServerFlow object from the request redirect_uri.