models: Split 'CoverLetter' from 'Submission'

We want to get rid of the split between 'Patch' and 'Submission' because
of the cost of using JOINs basically everywhere we use 'Patch'. Before
we do that, we need to move the other users of 'Submission' to other
models and other models that rely on these users sharing the common
'Submission' base. For the former, there is only one user,
'CoverLetter', while for the latter there is only the 'Comment' model.
As a result, we must do the following:

- Create a new 'Cover' model
- Create a new 'CoverComment' model
- Move everything from 'CoverLetter' to 'Cover' and all entries
  associated with a 'CoverLetter' from 'Comment' to 'CoverComment'
- Delete the 'CoverLetter' model
- Rename the 'Comment' model to 'PatchComment'

This means our model "hierarchy" goes from:

  Submission
    Patch
    CoverLetter
    Comment

To:

  Submission
    Patch
    PatchComment
  Cover
    CoverComment

A future change will flatten the 'Submission' and 'Patch' model.

Note that this actually highlighted a bug in Django, which has since
been reported upstream [1]. As noted there, the issue stems from MySQL's
refusal to remove an index from a foreign key when DB constraints are
used and the workaround is to remove the foreign key constraint before
altering the indexes and then re-add the constraint after.

[1] https://code.djangoproject.com/ticket/31335

Signed-off-by: Stephen Finucane <stephen@that.guru>

7 files changed, 65 insertions, 41 deletions

diff --git a/patchwork/tests/api/test_comment.py b/patchwork/tests/api/test_comment.py
index f48bfce..dfbf904 100644
--- a/patchwork/tests/api/test_comment.py
+++ b/patchwork/tests/api/test_comment.py
@@ -10,9 +10,10 @@ from django.urls import NoReverseMatch
 from django.urls import reverse
 
 from patchwork.tests.api import utils
-from patchwork.tests.utils import create_comment
 from patchwork.tests.utils import create_cover
+from patchwork.tests.utils import create_cover_comment
 from patchwork.tests.utils import create_patch
+from patchwork.tests.utils import create_patch_comment
 from patchwork.tests.utils import SAMPLE_CONTENT
 
 if settings.ENABLE_REST_API:
@@ -47,7 +48,7 @@ class TestCoverComments(utils.APITestCase):
     def test_list(self):
         """List cover letter comments."""
         cover = create_cover()
-        comment = create_comment(submission=cover)
+        comment = create_cover_comment(cover=cover)
 
         resp = self.client.get(self.api_url(cover))
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
@@ -57,7 +58,7 @@ class TestCoverComments(utils.APITestCase):
     def test_list_version_1_0(self):
         """List cover letter comments using API v1.0."""
         cover = create_cover()
-        create_comment(submission=cover)
+        create_cover_comment(cover=cover)
 
         # check we can't access comments using the old version of the API
         with self.assertRaises(NoReverseMatch):
@@ -98,7 +99,7 @@ class TestPatchComments(utils.APITestCase):
     def test_list(self):
         """List patch comments."""
         patch = create_patch()
-        comment = create_comment(submission=patch)
+        comment = create_patch_comment(patch=patch)
 
         resp = self.client.get(self.api_url(patch))
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
@@ -108,7 +109,7 @@ class TestPatchComments(utils.APITestCase):
     def test_list_version_1_0(self):
         """List patch comments using API v1.0."""
         patch = create_patch()
-        create_comment(submission=patch)
+        create_patch_comment(patch=patch)
 
         # check we can't access comments using the old version of the API
         with self.assertRaises(NoReverseMatch):
diff --git a/patchwork/tests/test_detail.py b/patchwork/tests/test_detail.py
index ddc2b93..393ebc7 100644
--- a/patchwork/tests/test_detail.py
+++ b/patchwork/tests/test_detail.py
@@ -6,9 +6,10 @@
 from django.test import TestCase
 from django.urls import reverse
 
-from patchwork.tests.utils import create_comment
 from patchwork.tests.utils import create_cover
+from patchwork.tests.utils import create_cover_comment
 from patchwork.tests.utils import create_patch
+from patchwork.tests.utils import create_patch_comment
 from patchwork.tests.utils import create_project
 
 
@@ -159,24 +160,32 @@ class PatchViewTest(TestCase):
 
 class CommentRedirectTest(TestCase):
 
-    def _test_redirect(self, submission, submission_url):
-        comment_id = create_comment(submission=submission).id
+    def test_patch_redirect(self):
+        patch = create_patch()
+        comment_id = create_patch_comment(patch=patch).id
 
         requested_url = reverse('comment-redirect',
                                 kwargs={'comment_id': comment_id})
         redirect_url = '%s#%d' % (
-            reverse(submission_url,
-                    kwargs={'project_id': submission.project.linkname,
-                            'msgid': submission.url_msgid}),
+            reverse('patch-detail',
+                    kwargs={'project_id': patch.project.linkname,
+                            'msgid': patch.url_msgid}),
             comment_id)
 
         response = self.client.get(requested_url)
         self.assertRedirects(response, redirect_url)
 
-    def test_patch_redirect(self):
-        patch = create_patch()
-        self._test_redirect(patch, 'patch-detail')
-
     def test_cover_redirect(self):
         cover = create_cover()
-        self._test_redirect(cover, 'cover-detail')
+        comment_id = create_cover_comment(cover=cover).id
+
+        requested_url = reverse('comment-redirect',
+                                kwargs={'comment_id': comment_id})
+        redirect_url = '%s#%d' % (
+            reverse('cover-detail',
+                    kwargs={'project_id': cover.project.linkname,
+                            'msgid': cover.url_msgid}),
+            comment_id)
+
+        response = self.client.get(requested_url)
+        self.assertRedirects(response, redirect_url)
diff --git a/patchwork/tests/test_mboxviews.py b/patchwork/tests/test_mboxviews.py
index 1e7bfb0..a7b0186 100644
--- a/patchwork/tests/test_mboxviews.py
+++ b/patchwork/tests/test_mboxviews.py
@@ -13,8 +13,8 @@ import email
 from django.test import TestCase
 from django.urls import reverse
 
-from patchwork.tests.utils import create_comment
 from patchwork.tests.utils import create_patch
+from patchwork.tests.utils import create_patch_comment
 from patchwork.tests.utils import create_project
 from patchwork.tests.utils import create_person
 from patchwork.tests.utils import create_series
@@ -34,8 +34,8 @@ class MboxPatchResponseTest(TestCase):
             project=self.project,
             submitter=self.person,
             content='comment 1 text\nAcked-by: 1\n')
-        create_comment(
-            submission=patch,
+        create_patch_comment(
+            patch=patch,
             submitter=self.person,
             content='comment 2 text\nAcked-by: 2\n')
         response = self.client.get(
@@ -48,8 +48,8 @@ class MboxPatchResponseTest(TestCase):
             project=self.project,
             submitter=self.person,
             content='patch text\n')
-        create_comment(
-            submission=patch,
+        create_patch_comment(
+            patch=patch,
             submitter=self.person,
             content=u'comment\nAcked-by:\u00A0 foo')
         response = self.client.get(
@@ -71,8 +71,8 @@ class MboxPatchSplitResponseTest(TestCase):
             submitter=self.person,
             diff='',
             content='comment 1 text\nAcked-by: 1\n---\nupdate\n')
-        self.comment = create_comment(
-            submission=self.patch,
+        self.comment = create_patch_comment(
+            patch=self.patch,
             submitter=self.person,
             content='comment 2 text\nAcked-by: 2\n')
 
diff --git a/patchwork/tests/test_parser.py b/patchwork/tests/test_parser.py
index 07c2b97..99e27f1 100644
--- a/patchwork/tests/test_parser.py
+++ b/patchwork/tests/test_parser.py
@@ -17,11 +17,11 @@ from django.test import TransactionTestCase
 from django.db.transaction import atomic
 from django.db import connection
 
-from patchwork.models import Comment
+from patchwork.models import Cover
 from patchwork.models import Patch
+from patchwork.models import PatchComment
 from patchwork.models import Person
 from patchwork.models import State
-from patchwork.models import CoverLetter
 from patchwork.parser import clean_subject
 from patchwork.parser import get_or_create_author
 from patchwork.parser import find_patch_content as find_content
@@ -551,7 +551,7 @@ class MultipleProjectPatchCommentTest(MultipleProjectPatchTest):
             patch = Patch.objects.filter(project=project)[0]
             # we should see the reply comment only
             self.assertEqual(
-                Comment.objects.filter(submission=patch).count(), 1)
+                PatchComment.objects.filter(patch=patch).count(), 1)
 
 
 class ListIdHeaderTest(TestCase):
@@ -1157,7 +1157,7 @@ class DuplicateMailTest(TestCase):
         self._test_duplicate_mail(m2)
 
         self.assertEqual(Patch.objects.count(), 1)
-        self.assertEqual(Comment.objects.count(), 1)
+        self.assertEqual(PatchComment.objects.count(), 1)
 
     def test_duplicate_coverletter(self):
         m = create_email('test', listid=self.listid, msgid='1@example.com')
@@ -1166,4 +1166,4 @@ class DuplicateMailTest(TestCase):
 
         self._test_duplicate_mail(m)
 
-        self.assertEqual(CoverLetter.objects.count(), 1)
+        self.assertEqual(Cover.objects.count(), 1)
diff --git a/patchwork/tests/test_series.py b/patchwork/tests/test_series.py
index 2e86e47..e68ee88 100644
--- a/patchwork/tests/test_series.py
+++ b/patchwork/tests/test_series.py
@@ -35,7 +35,7 @@ class _BaseTestCase(TestCase):
         mbox = mailbox.mbox(os.path.join(TEST_SERIES_DIR, name), create=False)
         for msg in mbox:
             obj = parser.parse_mail(msg, project.listid)
-            if type(obj) == models.CoverLetter:
+            if type(obj) == models.Cover:
                 results[0].append(obj)
             elif type(obj) == models.Patch:
                 results[1].append(obj)
diff --git a/patchwork/tests/test_tags.py b/patchwork/tests/test_tags.py
index 97afba0..6c13687 100644
--- a/patchwork/tests/test_tags.py
+++ b/patchwork/tests/test_tags.py
@@ -9,8 +9,8 @@ from django.test import TransactionTestCase
 from patchwork.models import Patch
 from patchwork.models import PatchTag
 from patchwork.models import Tag
-from patchwork.tests.utils import create_comment
 from patchwork.tests.utils import create_patch
+from patchwork.tests.utils import create_patch_comment
 
 
 class ExtractTagsTest(TestCase):
@@ -107,8 +107,8 @@ class PatchTagsTest(TransactionTestCase):
         return '%s-by: Test Tagger <tagger@example.com>\n' % tags[tagtype]
 
     def create_tag_comment(self, patch, tagtype=None):
-        comment = create_comment(
-            submission=patch,
+        comment = create_patch_comment(
+            patch=patch,
             content=self.create_tag(tagtype))
         return comment
 
diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py
index 1a7baa2..83bd66a 100644
--- a/patchwork/tests/utils.py
+++ b/patchwork/tests/utils.py
@@ -13,9 +13,10 @@ from django.contrib.auth.models import User
 
 from patchwork.models import Bundle
 from patchwork.models import Check
-from patchwork.models import Comment
-from patchwork.models import CoverLetter
+from patchwork.models import Cover
+from patchwork.models import CoverComment
 from patchwork.models import Patch
+from patchwork.models import PatchComment
 from patchwork.models import PatchRelation
 from patchwork.models import Person
 from patchwork.models import Project
@@ -203,8 +204,8 @@ def create_patch(**kwargs):
 
 
 def create_cover(**kwargs):
-    """Create 'CoverLetter' object."""
-    num = CoverLetter.objects.count()
+    """Create 'Cover' object."""
+    num = Cover.objects.count()
 
     # NOTE(stephenfin): Despite first appearances, passing 'series' to the
     # 'create' function doesn't actually cause the relationship to be created.
@@ -232,7 +233,7 @@ def create_cover(**kwargs):
     }
     values.update(kwargs)
 
-    cover = CoverLetter.objects.create(**values)
+    cover = Cover.objects.create(**values)
 
     if series:
         series.add_cover_letter(cover)
@@ -240,17 +241,30 @@ def create_cover(**kwargs):
     return cover
 
 
-def create_comment(**kwargs):
-    """Create 'Comment' object."""
+def create_cover_comment(**kwargs):
+    """Create 'CoverComment' object."""
     values = {
         'submitter': create_person() if 'submitter' not in kwargs else None,
-        'submission': create_patch() if 'submission' not in kwargs else None,
+        'cover': create_cover() if 'cover' not in kwargs else None,
         'msgid': make_msgid(),
         'content': SAMPLE_CONTENT,
     }
     values.update(kwargs)
 
-    return Comment.objects.create(**values)
+    return CoverComment.objects.create(**values)
+
+
+def create_patch_comment(**kwargs):
+    """Create 'PatchComment' object."""
+    values = {
+        'submitter': create_person() if 'submitter' not in kwargs else None,
+        'patch': create_patch() if 'patch' not in kwargs else None,
+        'msgid': make_msgid(),
+        'content': SAMPLE_CONTENT,
+    }
+    values.update(kwargs)
+
+    return PatchComment.objects.create(**values)
 
 
 def create_check(**kwargs):