Task unification todo task removal (#682)

* Add todo.step and todo.project data migration * Remove task from add_todolist_template * Remove task from IsAssociatedWithProject * Fix update_todos_from_todolist_template and tests * Fix auth * Remove task from worker_task_recent_todo_qas * Fix auth * Fix worker_task_recent_todo_qas * Fix auth * Remove unused code * Remove unused code * Fix todos/auth * Remove task from serializer * Fix tests * Remove unused code * Step should be slug * Add delete/destroy view * Remove task, pass step slug as one of args * Add helper * Fix api * Linting fix * Remove console.log * Update compiled js * Fix add_todolist_template usage * Fix tests * Fix tests after rebase * Fix tests * Simplify * Make it possible to get related todos via Task * Linting fixes * Fix tests * Clean-up and add comment * Add comments to migration * Rename reverse lookup name * Remove unused code * Clean tests * Rename module * Fix comment * Clean urls * Filter properly * Remove task__project and task__step from todo admin * Fix js formatting issue * Limit mmethods * Add missing method
b12io · Sep 17, 2020 · d8687a5 · d8687a5
1 parent 02a4491
commit d8687a5
Show file tree

Hide file tree

Showing 24 changed files with 378 additions and 208 deletions.
diff --git a/orchestra/admin.py b/orchestra/admin.py
@@ -180,9 +180,9 @@ class TodoAdmin(admin.ModelAdmin):
     list_display = ('id', 'created_at', 'task', 'title', 'completed')
     ordering = ('-created_at',)
     search_fields = (
-        'task__project__short_description', 'task__step__name',
+        'project__short_description', 'step__name',
         'title')
-    list_filter = ('task__project__workflow_version',)
+    list_filter = ('project__workflow_version',)
 
 
 @admin.register(TodoQA)

diff --git a/orchestra/migrations/0087_todo_task_data_migration.py b/orchestra/migrations/0087_todo_task_data_migration.py
@@ -0,0 +1,56 @@
+# Generated by Django 2.2.13 on 2020-09-04 14:49
+
+
+from django.db import migrations
+from django.db.models import OuterRef
+from django.db.models import Subquery
+
+
+def fill_project_and_step_fields(apps, schema_editor):
+    Step = apps.get_model('orchestra', 'Step')
+    Project = apps.get_model('orchestra', 'Project')
+    Todo = apps.get_model('orchestra', 'Todo')
+    # This is the outer query field the querysets in the subqueries need to refer to
+    o = OuterRef('id')
+    # Select a step which has a task which has a todo which has an identical ID
+    step_outerref_filter_qs = Step.objects.filter(tasks__todos__id=o)
+    # Select a project which has a task which has a todo which has an identical ID
+    project_outterref_filter_qs = Project.objects.filter(tasks__todos__id=o)
+    # We need id value only. These will go as subqueries
+    subq_step = step_outerref_filter_qs.values('id')[:1]
+    subq_proj = project_outterref_filter_qs.values('id')[:1]
+    # Update all todos
+    Todo.objects.update(project=Subquery(subq_proj), step=Subquery(subq_step))
+    """
+    The query above generates the following SQL-query:
+
+    UPDATE "orchestra_todo"
+    SET "project_id" = (
+          SELECT     u0."id"
+          FROM       "orchestra_project" U0
+          INNER JOIN "orchestra_task" U1
+          ON         (u0."id" = u1."project_id")
+          INNER JOIN "orchestra_todo" U2
+          ON         (u1."id" = u2."task_id")
+          WHERE      u2."id" = ("orchestra_todo"."id") limit 1),
+       "step_id" = (
+          SELECT     u0."id"
+          FROM       "orchestra_step" u0
+          INNER JOIN "orchestra_task" u1
+          ON         (u0."id" = u1."step_id")
+          INNER JOIN "orchestra_todo" u2
+          ON         (u1."id" = u2."task_id")
+          WHERE      u2."id" = ("orchestra_todo"."id") limit 1)
+    WHERE  "orchestra_todo"."is_deleted" = false;args=(false,)
+    """
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('orchestra', '0086_add_fields_to_todo'),
+    ]
+
+    operations = [
+        migrations.RunPython(fill_project_and_step_fields,  # manually-reviewed
+                             reverse_code=migrations.RunPython.noop),  # manually-reviewed
+    ]
diff --git a/orchestra/migrations/0088_change_related_name_todos_old.py b/orchestra/migrations/0088_change_related_name_todos_old.py
@@ -0,0 +1,19 @@
+# Generated by Django 2.2.13 on 2020-09-15 14:52
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('orchestra', '0087_todo_task_data_migration'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='todo',
+            name='task',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='todos_deprecated', to='orchestra.Task'),
+        ),
+    ]
diff --git a/orchestra/models/core/mixins.py b/orchestra/models/core/mixins.py
@@ -114,6 +114,12 @@ def __str__(self):
 
 class TaskMixin(object):
 
+    @property
+    def todos(self):
+        from orchestra.models import Todo
+        return Todo.objects.filter(project=self.project,
+                                   step=self.step)
+
     def is_worker_assigned(self, worker):
         """
         Check if specified worker is assigned to the given task.

diff --git a/orchestra/models/core/models.py b/orchestra/models/core/models.py
@@ -666,7 +666,8 @@ class Status(ChoicesEnum):
         DECLINED = 'declined'
 
     task = models.ForeignKey(Task, null=True, blank=True,
-                             related_name='todos', on_delete=models.SET_NULL)
+                             related_name='todos_deprecated',
+                             on_delete=models.SET_NULL)
     project = models.ForeignKey(Project, null=True, blank=True,
                                 related_name='todos', on_delete=models.CASCADE)
     step = models.ForeignKey(Step, null=True, blank=True,

diff --git a/orchestra/project_api/tests/test_project_api.py b/orchestra/project_api/tests/test_project_api.py
@@ -29,6 +29,7 @@
 from orchestra.tests.helpers.fixtures import StepFactory
 from orchestra.tests.helpers.fixtures import ProjectFactory
 from orchestra.tests.helpers.fixtures import TodoFactory
+from orchestra.tests.helpers.fixtures import WorkflowVersionFactory
 from orchestra.tests.helpers.google_apps import mock_create_drive_service
 from orchestra.utils.load_json import load_encoded_json
 from orchestra.utils.task_lifecycle import get_new_task_assignment
@@ -477,8 +478,12 @@ def setUp(self):
         self.request_client = APIClient(enforce_csrf_checks=True)
         self.request_client.force_authenticate(user=SignedUser())
         setup_models(self)
-        self.project = ProjectFactory()
-        self.step = StepFactory(slug='step-slug')
+        self.workflow_version = WorkflowVersionFactory()
+        self.step = StepFactory(
+            slug='step-slug',
+            workflow_version=self.workflow_version)
+        self.project = ProjectFactory(
+            workflow_version=self.workflow_version)
         self.list_url = reverse('orchestra:api:todo-api-list')
         self.todo = TodoFactory(project=self.project)
         self.todo_with_step = TodoFactory(project=self.project, step=self.step)
@@ -487,7 +492,7 @@ def test_permissions(self):
         data = {
             'title': 'Testing title 1',
             'project': self.project.id,
-            'step': self.step.id
+            'step': self.step.slug
         }
         request_client = APIClient(enforce_csrf_checks=True)
         resp = request_client.post(
@@ -516,7 +521,7 @@ def test_create(self, mock_notify):
         data = {
             'title': 'Testing create action',
             'project': self.project.id,
-            'step': self.step.id
+            'step': self.step.slug
         }
         resp = self.request_client.post(
             self.list_url, data=json.dumps(data),
@@ -536,7 +541,7 @@ def test_bulk_create(self):
             {
                 'title': 'Testing title {}'.format(x),
                 'project': self.project.id,
-                'step': self.step.id
+                'step': self.step.slug
             } for x in range(10)
         ]
         resp = self.request_client.post(
@@ -568,13 +573,13 @@ def test_get_list_of_todos_with_filters(self):
         resp = self.request_client.get(url_with_step_filter)
         self.assertEqual(resp.status_code, 200)
         self.assertEqual(len(resp.json()), 1)
-        self.assertEqual(resp.json()[0]['step'], self.todo_with_step.step.id)
+        self.assertEqual(resp.json()[0]['step'], self.todo_with_step.step.slug)
 
         url_with_filters = '{}?project={}&step__slug={}'.format(
             self.list_url, self.project.id, self.todo_with_step.step.slug)
         resp = self.request_client.get(url_with_filters)
         self.assertEqual(resp.status_code, 200)
-        self.assertEqual(resp.json()[0]['step'], self.todo_with_step.step.id)
+        self.assertEqual(resp.json()[0]['step'], self.todo_with_step.step.slug)
 
     @patch('orchestra.todos.views.notify_single_todo_update')
     def test_update_functionality(self, mock_notify):
@@ -607,13 +612,13 @@ def test_partial_update_functionality(self, mock_notify):
             detail_url,
             data=json.dumps({
                 'title': expected_title,
-                'step': self.step.id,
+                'step': self.step.slug,
                 'project': self.project.id
             }),
             content_type='application/json')
         self.assertEqual(resp.status_code, 200)
         self.assertEqual(resp.json()['title'], expected_title)
-        self.assertEqual(resp.json()['step'], self.step.id)
+        self.assertEqual(resp.json()['step'], self.step.slug)
         self.assertEqual(resp.json()['project'], self.project.id)
 
         resp = self.request_client.patch(
@@ -677,8 +682,8 @@ def test_bulk_update(self):
             self.list_url, data=json.dumps(updated),
             content_type='application/json')
         self.assertEqual(resp.status_code, 400)
-        self.assertEqual(resp.json()[0]['project'],
-                         ['project should be supplied.'])
+        self.assertEqual(resp.json()[0]['step'],
+                         ['This field may not be null.'])
 
     def test_bulk_partial_update(self):
         todo1 = TodoFactory(
@@ -693,7 +698,7 @@ def test_bulk_partial_update(self):
         todos_with_updated_titles = [{
             'id': x.id,
             'title': 'Updated title {}'.format(x.id),
-            'step': x.step.id,
+            'step': x.step.slug,
             'project': x.project.id
         } for x in [todo1, todo3, todo2]]
         resp = self.request_client.patch(