Refactored export task API

exporter/tasks.py

@@ -4,6 +4,8 @@
 
 from __future__ import unicode_literals
 
+from django.core.files.storage import default_storage
+
 from lore.celery import async
 from exporter.api import export_resources_to_tarball
 
@@ -19,7 +21,12 @@ def export_resources(learning_resources, username):
         username (unicode): Name of user
     Returns:
         (unicode, bool):
-            First item is newly created temp directory with files inside of it.
+            First item is URL of tarball using django-storage.
             Second item is True if a static asset collision was detected.
     """
-    return export_resources_to_tarball(learning_resources, username)
+    name, collision = export_resources_to_tarball(learning_resources, username)
+    return {
+        "name": name,
+        "url": default_storage.url(name),
+        "collision": collision
+    }

exporter/tests/test_export.py

@@ -175,8 +175,10 @@ def test_export_task(self):
         """Test exporting resources task."""
         resources = LearningResource.objects.all()
 
-        path, collision = export_resources.delay(
+        result = export_resources.delay(
             resources, self.user.username).get()
+        path = result['name']
+        collision = result['collision']
         tempdir = mkdtemp()
 
         self.assertTrue(collision)

rest/serializers.py

@@ -21,6 +21,7 @@
     SerializerMethodField,
     IntegerField,
     FloatField,
+    DictField,
 )
 
 from rest.util import LambdaDefault, RequiredBooleanField
@@ -307,6 +308,13 @@ class LearningResourceExportTaskSerializer(Serializer):
     """Serializer for export tasks for LearningResource."""
     id = CharField()
     status = CharField()
+    result = DictField()
+
+
+class ImportTaskSerializer(Serializer):
+    """Serializer for import tasks."""
+    id = CharField()
+    status = CharField()
     url = CharField()
 
 

rest/tests/test_export.py

@@ -48,22 +48,24 @@ def test_create_new_task(self):
             self.repo.slug)['results'][0]
         self.assertEqual(task_id, result['id'])
         self.assertEqual("success", result['status'])
-        self.assertTrue(result['url'].startswith(
+        self.assertTrue(result['result']['url'].startswith(
             "/media/resource_exports/test_exports.tar"))
+        # webGLDemo.css shows up twice
+        self.assertTrue(result['result']['collision'])
 
         with self.settings(
             DEFAULT_FILE_STORAGE='storages.backends.s3boto.S3BotoStorage'
         ):
             # change the default file storage to S3
             reload_module(ui.urls)
             # the view is not available any more
-            resp = self.client.get(result['url'])
+            resp = self.client.get(result['result']['url'])
             self.assertEqual(resp.status_code, HTTP_404_NOT_FOUND)
 
         # Update for change in file storage.
         reload_module(ui.urls)
 
-        resp = self.client.get(result['url'])
+        resp = self.client.get(result['result']['url'])
         self.assertEqual(HTTP_200_OK, resp.status_code)
 
         tempdir = mkdtemp()

rest/views.py

@@ -7,7 +7,6 @@
 from django.http.response import Http404
 from django.contrib.auth.models import User
 from django.core.exceptions import PermissionDenied
-from django.core.files.storage import default_storage
 from django.db import transaction
 from rest_framework import status
 from rest_framework.exceptions import ValidationError
@@ -741,7 +740,28 @@ def perform_destroy(self, instance):
             raise Http404
 
 
-def create_task_result_dict(task):
+def create_initial_task_dict(task):
+    """
+    Create initial task data about a newly created Celery task.
+
+    Args:
+        task (Task): A Celery task.
+    Returns:
+        dict: Initial data about task.
+    """
+
+    result = None
+    if task.successful():
+        result = task.get()
+
+    return {
+        "id": task.id,
+        "initial_state": task.state,
+        "result": result
+    }
+
+
+def create_task_result_dict(initial_data):
     """
     Convert initial data we put in session to dict for REST API.
     This will use the id to look up current data about task to return
@@ -752,36 +772,40 @@ def create_task_result_dict(task):
     Returns:
         dict: Current data about task.
     """
-    initial_state = task['initial_state']
-    task_id = task['id']
+    initial_state = initial_data['initial_state']
+    task_id = initial_data['id']
 
     state = "processing"
-    url = ""
-    collision = False
+    result = None
     # initial_state is a workaround for EagerResult used in testing.
     # In production initial_state should usually be pending.
     if initial_state == SUCCESS:
         state = "success"
-        url = task['url']
+        try:
+            result = initial_data['result']
+        except KeyError:
+            # Backwards compatibility.
+            result = {
+                "url": initial_data.get('url', ''),
+                "collision": initial_data.get('collision', False)
+            }
     elif initial_state in (FAILURE, REVOKED):
         state = "failure"
     else:
-        result = AsyncResult(task_id)
+        async_result = AsyncResult(task_id)
 
-        if result.successful():
+        if async_result.successful():
             state = "success"
-        elif result.failed():
+        elif async_result.failed():
             state = "failure"
 
-        if result.successful():
-            name, collision = result.get()
-            url = default_storage.url(name)
+        if async_result.successful():
+            result = async_result.get()
 
     return {
         "id": task_id,
         "status": state,
-        "url": url,
-        "collision": collision
+        "result": result
     }
 
 
@@ -845,20 +869,9 @@ def post(self, request, *args, **kwargs):
         result = export_resources.delay(
             learning_resources, self.request.user.username)
 
-        if result.successful():
-            name, collision = result.get()
-            url = default_storage.url(name)
-        else:
-            collision = False
-            url = ""
-
         # Put new task in session.
-        self.request.session[EXPORT_TASK_KEY][repo_slug][result.id] = {
-            "id": result.id,
-            "initial_state": result.state,
-            "url": url,
-            "collision": collision
-        }
+        self.request.session[EXPORT_TASK_KEY][repo_slug][
+            result.id] = create_initial_task_dict(result)
         self.request.session.modified = True
 
         return Response(

ui/jstests/test_lr_exports.jsx

@@ -58,8 +58,10 @@ define(['QUnit', 'jquery', 'lr_exports', 'react',
             {
               "id": "task1",
               "status": "successful",
-              "url": "/media/resource_exports/export_task1.tar.gz",
-              "collision": false
+              "result": {
+                "url": "/media/resource_exports/export_task1.tar.gz",
+                "collision": false
+              }
             }
           ]
         }
@@ -77,8 +79,7 @@ define(['QUnit', 'jquery', 'lr_exports', 'react',
         responseText: {
           "id": "task2",
           "status": "processing",
-          "url": "",
-          "collision": false
+          "result": null
         }
       });
       TestUtils.initMockjax({
@@ -153,8 +154,7 @@ define(['QUnit', 'jquery', 'lr_exports', 'react',
             responseText: {
               "id": "task2",
               "status": "processing",
-              "url": "",
-              "collision": false
+              "result": null
             }
           });
 
@@ -177,8 +177,10 @@ define(['QUnit', 'jquery', 'lr_exports', 'react',
               responseText: {
                 "id": "task2",
                 "status": "success",
-                "url": "/media/resource_exports/export_task2.tar.gz",
-                "collision": false
+                "result": {
+                  "url": "/media/resource_exports/export_task2.tar.gz",
+                  "collision": false
+                }
               }
             });
 
@@ -547,8 +549,7 @@ define(['QUnit', 'jquery', 'lr_exports', 'react',
             responseText: {
               "id": "task2",
               "status": "processing",
-              "url": "",
-              "collision": false
+              "result": null
             }
           });
 
@@ -572,8 +573,7 @@ define(['QUnit', 'jquery', 'lr_exports', 'react',
               responseText: {
                 "id": "task2",
                 "status": "failure",
-                "url": "",
-                "collision": false
+                "result": null
               }
             });
 
@@ -661,8 +661,7 @@ define(['QUnit', 'jquery', 'lr_exports', 'react',
             responseText: {
               "id": "task2",
               "status": "processing",
-              "url": "",
-              "collision": false
+              "result": null
             }
           });
 
@@ -774,8 +773,10 @@ define(['QUnit', 'jquery', 'lr_exports', 'react',
             responseText: {
               "id": "task2",
               "status": "success",
-              "url": "/media/resource_exports/export_task2.tar.gz",
-              "collision": true
+              "result": {
+                "url": "/media/resource_exports/export_task2.tar.gz",
+                "collision": true
+              }
             }
           });
           TestUtils.replaceMockjax({

ui/static/ui/js/lr_exports.jsx

@@ -206,8 +206,8 @@ define("lr_exports",
             });
 
             thiz.setState({
-              url: result.url,
-              collision: result.collision
+              url: result.result.url,
+              collision: result.result.collision
             });
           } else if (result.status === "processing") {
             setTimeout(function() {