Fix failing tests and formatting errors

onadata/apps/api/permissions.py

@@ -127,6 +127,7 @@ class DjangoObjectPermissionsAllowAnon(DjangoObjectPermissions):
     authenticated_users_only = False
 
 
+# pylint: disable=too-few-public-methods
 class ImportPermissions(DjangoObjectPermissions):
     """
     ImportPermissions - custom permissions check on Imports viewset.

onadata/apps/api/tests/viewsets/v2/test_imports_viewset.py

@@ -6,12 +6,11 @@
 from typing import IO, Any
 from unittest.mock import patch
 
-from django.contrib.auth.models import User
+from django.contrib.auth import get_user_model
 from django.conf import settings
 from django.test import override_settings
 from httmock import HTTMock
 
-from onadata.celeryapp import app
 from onadata.apps.api.viewsets.v2.imports_viewset import ImportsViewSet
 from onadata.apps.api.tests.viewsets.test_abstract_viewset import TestAbstractViewSet
 from onadata.apps.api.tests.mocked_data import enketo_mock
@@ -29,9 +28,12 @@ def fixtures_path(filepath: str) -> IO[Any]:
 
 
 class TestImportsViewSet(TestAbstractViewSet):
+    """
+    Test for ImportsViewSet
+    """
 
     def setUp(self):
-        super(TestImportsViewSet, self).setUp()
+        super().setUp()
         self.view = ImportsViewSet.as_view({
             "post": "create",
             "get": "retrieve",
@@ -53,9 +55,9 @@ def test_create_permissions(self):
                 "tutorial.xlsx",
             )
             self._publish_xls_form_to_project(xlsform_path=xls_path)
-            user = User.objects.create(username="joe",
-                                       email="[email protected]",
-                                       first_name="Joe")
+            user = get_user_model().objects.create(username="joe",
+                                                   email="[email protected]",
+                                                   first_name="Joe")
             _ = UserProfile.objects.create(user=user)
             extra = {"HTTP_AUTHORIZATION": f"Token {user.auth_token}"}
             csv_import = fixtures_path("good.csv")
@@ -102,7 +104,7 @@ def test_create_permissions(self):
             self.assertEqual(response.status_code, 202)
 
     @override_settings(DISABLE_ASYNCHRONOUS_IMPORTS=True)
-    def test_create_expected_synchronous_response(self):
+    def test_synchronous_response(self):
         """
         Tests that the `api/v2/imports/<xself.xform.pk>` route processes a request
         successfully when `DISABLE_ASYNCHRONOUS_IMPORTS` is set to `True`
@@ -130,7 +132,7 @@ def test_create_expected_synchronous_response(self):
             self.assertEqual(response.data.get("additions"), 9)
             self.assertEqual(response.data.get("updates"), 0)
 
-    def test_create_expected_async_response(self):
+    def test_expected_async_response(self):
         """
         Tests that the `api/v2/imports/<xself.xform.pk>` route processes a request
         successfully when `DISABLE_ASYNCHRONOUS_IMPORTS` is set to `False`
@@ -160,7 +162,7 @@ def test_create_expected_async_response(self):
             self.assertEqual(expected_fields, list(response.data.keys()))
 
     @patch("onadata.apps.api.viewsets.v2.imports_viewset.get_active_tasks")
-    def test_create_ongoing_overwrite_task(self, mocked_get_active_tasks):
+    def test_ongoing_overwrite_task(self, mocked_get_active_tasks):
         """
         Test that the `api/v2/imports/<xself.xform.pk>` route refuses to process request
         when an overwrite import task is ongoing
@@ -238,8 +240,9 @@ def test_create_request_validation(self):
             expected_response = {"error": "xls_file not an excel file"}
             self.assertEqual(expected_response, response.data)
 
-            post_data = {"csv_file": open(xls_path, "rb")}
-            request = self.factory.post(path, data=post_data, **self.extra)
+            with open(xls_path, "rb") as xls_file:
+                post_data = {"csv_file": xls_file}
+                request = self.factory.post(path, data=post_data, **self.extra)
             response = self.view(request, pk=self.xform.pk)
 
             self.assertEqual(response.status_code, 400)
@@ -263,9 +266,9 @@ def test_delete_permissions(self):
             )
             self._publish_xls_form_to_project(xlsform_path=xls_path)
             path = "/api/v2/imports/{self.xform.pk}?task_uuid=11"
-            user = User.objects.create(username="joe",
-                                       email="[email protected]",
-                                       first_name="Joe")
+            user = get_user_model().objects.create(username="joe",
+                                                   email="[email protected]",
+                                                   first_name="Joe")
             _ = UserProfile.objects.create(user=user)
             extra = {"HTTP_AUTHORIZATION": f"Token {user.auth_token}"}
 

onadata/apps/api/urls/v2_urls.py

@@ -11,6 +11,9 @@
 
 
 class DetailedPostRouter(routers.DefaultRouter):
+    """
+    Custom router
+    """
     routes = [
         # List route.
         routers.Route(
@@ -52,6 +55,7 @@ class DetailedPostRouter(routers.DefaultRouter):
         ),
     ]
 
+    # pylint: disable=redefined-outer-name
     def extend(self, router):
         """
         Extends the routers routes with the routes from another router

onadata/apps/api/viewsets/v2/imports_viewset.py

@@ -1,4 +1,6 @@
-import json
+"""
+Imports v2 viewset
+"""
 import os
 
 from django.conf import settings
@@ -7,7 +9,6 @@
 from rest_framework import viewsets, status
 from rest_framework.response import Response
 from rest_framework.exceptions import ParseError, ErrorDetail
-from rest_framework.decorators import action
 
 from onadata.celeryapp import app
 from onadata.apps.api.tools import get_baseviewset_class
@@ -28,28 +29,40 @@
 
 
 def terminate_import_task(task_uuid: str, xform_pk: int) -> bool:
+    """Terminate an import task given a uuid and xform_pk"""
     task_details = app.control.inspect().query_task(task_uuid)
     if task_details:
         task_details = list(task_details.values())
     if task_details and task_uuid in task_details[0]:
         task_details = task_details[0][task_uuid]
 
-    if task_details and task_details[1]["args"][1] == xform_pk:
+    if task_details and task_details[0] == 'active' and \
+        task_details[1]["args"] and task_details[1]["args"][1] == xform_pk:
         app.control.terminate(task_uuid)
         return True
     return False
 
 
+# pylint: disable=too-many-ancestors
 class ImportsViewSet(AuthenticateHeaderMixin, ETagsMixin, CacheControlMixin,
                      viewsets.ViewSet):
+    """
+    Implements api/v2/imports endpoints
+    """
     permission_classes = [ImportPermissions]
     queryset = XForm.objects.filter(deleted_at__isnull=True)
     task_names = ["onadata.libs.utils.csv_import.submit_csv_async"]
 
     def get_queryset(self):
+        """
+        Retreive all XForms that have not been deleted
+        """
         return XForm.objects.filter(deleted_at__isnull=True)
 
-    def get_object(self, pk: int):
+    def get_object(self, pk: int):  # pylint: disable=invalid-name
+        """
+        Retreive XForm with specified pk
+        """
         queryset = self.get_queryset()
         obj = get_object_or_404(queryset, pk=pk)
         self.check_object_permissions(self.request, obj)
@@ -61,19 +74,22 @@ def _get_active_tasks(self, xform: XForm) -> str:
         """
         return get_active_tasks(self.task_names, xform)
 
+    # pylint: disable=invalid-name
     def create(self, request, pk: int = None) -> Response:
         """
-        Starts a new Import task for a given form; The route processes imports asynchronously
-        unless the `DISABLE_ASYNCHRONOUS_IMPORTS` setting is set to false.
+        Starts a new Import task for a given form; The route processes imports
+        asynchronously unless the `DISABLE_ASYNCHRONOUS_IMPORTS` setting
+        is set to false.
 
         Curl example:
 
         $ curl -X POST "http://<domain>/api/v2/imports/<xform_id>"
 
         Supported Query Parameters:
 
-        - [Optional] overwrite: bool = Whether the server should permanently delete the data currently available on
-          the form then reimport the data using the csv_file/xls_file sent with the request.
+        - [Optional] overwrite: bool = Whether the server should permanently
+          delete the data currently available on the form then reimport the
+          data using the csv_file/xls_file sent with the request.
 
         Required Request Arguements:
 
@@ -82,10 +98,15 @@ def create(self, request, pk: int = None) -> Response:
 
         Possible Response status codes:
 
-        - 202 Accepted: Server has successfully accepted your request for data import and has queued the task
-        - 200 Ok: Server has successfully imported your data to the form; Only returned when asynchronous imports are disabled
-        - 400 Bad Request: Request has been refused due to incorrect/missing csv_file or xls_file file
-        - 403 Forbidden: The request was valid but the server refused to process it. An explanation on why it was refused can be found in the JSON Response
+        - 202 Accepted: Server has successfully accepted your request for data
+          import and has queued the task
+        - 200 Ok: Server has successfully imported your data to the form;
+          Only returned when asynchronous imports are disabled
+        - 400 Bad Request: Request has been refused due to incorrect/missing
+          csv_file or xls_file file
+        - 403 Forbidden: The request was valid but the server refused to
+          process it. An explanation on why it was refused can be found
+          in the JSON Response
         - 401 Unauthorized: The request has been refused due to missing authentication
         """
         xform = self.get_object(pk)
@@ -116,17 +137,14 @@ def create(self, request, pk: int = None) -> Response:
                     resp.update({
                         "detail":
                         ErrorDetail(
-                            f"An ongoing overwrite request with the ID {task_id} is being processed"
+                            "An ongoing overwrite request with the ID " +
+                            f"{task_id} is being processed"
                         )
                     })
                     status_code = status.HTTP_403_FORBIDDEN
                     break
 
             if not status_code == status.HTTP_403_FORBIDDEN:
-                try:
-                    csv_size = csv_file.size
-                except AttributeError:
-                    csv_size = csv_file.__sizeof__()
                 csv_file.seek(0)
 
                 if getattr(settings, "DISABLE_ASYNCHRONOUS_IMPORTS", False):
@@ -149,6 +167,7 @@ def create(self, request, pk: int = None) -> Response:
                         status=status_code,
                         content_type="application/json")
 
+    # pylint: disable=redefined-builtin, unused-argument
     def retrieve(self, request, pk: int = None, format=None) -> Response:
         """Returns csv import async tasks that belong to this form"""
         xform = self.get_object(pk)
@@ -170,7 +189,8 @@ def destroy(self, request, pk: int = None) -> Response:
         Possible Response status codes:
 
         - 204 No Content: Request was successfully processed. Task was terminated.
-        - 400 Bad Request: Request was rejected either due to a missing `task_uuid` query parameter or because the `task_uuid` does not exist for the XForm
+        - 400 Bad Request: Request was rejected either due to a missing `task_uuid`
+            query parameter or because the `task_uuid` does not exist for the XForm
         """
         xform = self.get_object(pk)
         task_uuid = request.query_params.get("task_uuid")

onadata/apps/api/viewsets/xform_viewset.py

@@ -97,7 +97,6 @@
     get_form_url,
 )
 from onadata.settings.common import CSV_EXTENSION, XLS_EXTENSIONS
-from onadata.libs.utils.async_status import get_active_tasks
 
 ENKETO_AUTH_COOKIE = getattr(settings, "ENKETO_AUTH_COOKIE", "__enketo")
 ENKETO_META_UID_COOKIE = getattr(

onadata/libs/tests/utils/test_async_status.py

@@ -10,6 +10,7 @@
 from onadata.libs.utils import async_status
 from onadata.apps.logger.models.xform import XForm
 
+
 class TestAsyncStatus(TestBase):
 
     def test_celery_state_to_status(self):
@@ -50,7 +51,7 @@ def test_get_active_tasks(self):
             async_status.get_active_tasks(
                 ['onadata.libs.utils.csv_import.submit_csv_async'], xform
             ),
-            '[]',
+            [],
         )
         inspect = MagicMock()
         inspect.active = MagicMock(
@@ -67,12 +68,10 @@ def test_get_active_tasks(self):
         )
         app.control.inspect = MagicMock(return_value=inspect)
 
-        self.assertEqual(
-            async_status.get_active_tasks(
-                ['onadata.libs.utils.csv_import.submit_csv_async'], xform
-            ),
-            '[{"job_uuid": "11", "time_start"'
-            + ": \""
-            + datetime.fromtimestamp(time_start).strftime("%Y-%m-%dT%H:%M:%S")
-            + '", "file": "/home/ona/import.csv", "overwrite": true}]',
-        )
+        self.assertEqual(async_status.get_active_tasks(
+            ['onadata.libs.utils.csv_import.submit_csv_async'],
+            xform),
+            [{'job_uuid': '11',
+              'time_start': datetime.fromtimestamp(time_start).strftime(
+                  "%Y-%m-%dT%H:%M:%S"),
+              "file": "/home/ona/import.csv", "overwrite": True}])

onadata/libs/utils/async_status.py

@@ -1,7 +1,6 @@
 """
 Utilities for celery asyncronous tasks
 """
-import json
 from datetime import datetime
 
 from typing import List
@@ -47,6 +46,17 @@ def async_status(status, error=None):
     return status
 
 
+def format_task(task):
+    """Format a celery task element"""
+    return {"job_uuid": gettext(task["id"]),
+            "time_start": datetime.fromtimestamp(task["time_start"]).strftime(
+        "%Y-%m-%dT%H:%M:%S"
+    ),
+        "file": gettext(task["args"][2]),
+        "overwrite": task["args"][3],
+    }
+
+
 def get_active_tasks(task_names: List[str], xform: XForm):
     """Get active celery tasks"""
     inspect = app.control.inspect()
@@ -61,16 +71,4 @@ def get_active_tasks(task_names: List[str], xform: XForm):
             )
         )
 
-    return list(
-            map(
-                lambda i: {
-                    "job_uuid": gettext(i["id"]),
-                    "time_start": datetime.fromtimestamp(i["time_start"]).strftime(
-                        "%Y-%m-%dT%H:%M:%S"
-                    ),
-                    "file": gettext(i["args"][2]),
-                    "overwrite": i["args"][3],
-                },
-                data,
-            )
-        )
+    return list(map(format_task, data))