diff mbox series

[5/5] models: Merge 'Patch' and 'Submission'

Message ID 20200304115457.32300-6-stephen@that.guru
State Superseded
Headers show
Series Remove 'Submission' model | expand

Commit Message

Stephen Finucane March 4, 2020, 11:54 a.m. UTC
Oh, the follies of youth. Time to undo the damage of 2.0.0, specifically
commit 86172ccc16, which split Patch into two separate models using
concrete inheritance. As noted previously, this introduced a large
number of unavoidable JOINs across large tables and the performance
impacts these introduce are blocking other features we want, such as
improved tagging functionality. To combine these two models, we must do
the following:

- Update any references to the 'Patch' model to point to the
  'Submission' model instead
- Move everything from 'Patch' to 'Submission', including both fields
  and options
- Delete the 'Patch' model
- Rename the 'Submission' model to 'Patch'

With this change, our model "hierarchy" goes from:

  Submission
    Patch
    PatchComment
  Cover
    CoverComment

To a nice, flat:

  Patch
    PatchComment
  Cover
    CoverComment

I expect this migration to be intensive, particularly for MySQL users
who will see their entire tables rewritten. Unfortunately I don't see
any way to resolve this in an easier manner.

Also note that we have to use two migrations and separate the moving of
fields from the renaming of models, in order to work around a bug in
Django 1.11. This bug has been fixed since Django 2.0 [1][2] but the fix
wasn't backported and Django 1.11 is only accepting security fixes at
this point in time.

[1] https://code.djangoproject.com/ticket/25530
[2] https://code.djangoproject.com/ticket/31337

Signed-off-by: Stephen Finucane <stephen@that.guru>
Cc: Daniel Axtens <dja@axtens.net>
Cc: Stewart Smith <stewart@linux.ibm.com>
---
TODO:
- Figure out if the indexes are correct (Stewart?). I've just included
  everything that's displayed on the '/list' page.
- If this isn't merged until 3.0, combine the migrations since we will
  no longer have to support Django 1.11.
---
 patchwork/api/comment.py                      |   4 +-
 patchwork/management/commands/dumparchive.py  |   2 +-
 .../0041_merge_patch_submission_a.py          | 281 ++++++++++++++++++
 .../0042_merge_patch_submission_b.py          |  17 ++
 patchwork/models.py                           |  94 +++---
 patchwork/parser.py                           |  16 +-
 patchwork/tests/test_mboxviews.py             |  26 +-
 patchwork/tests/utils.py                      |   5 +-
 patchwork/views/__init__.py                   |   2 +-
 patchwork/views/utils.py                      |   4 +-
 10 files changed, 376 insertions(+), 75 deletions(-)
 create mode 100644 patchwork/migrations/0041_merge_patch_submission_a.py
 create mode 100644 patchwork/migrations/0042_merge_patch_submission_b.py

Comments

Daniel Axtens March 16, 2020, 2:52 a.m. UTC | #1
Stephen Finucane <stephen@that.guru> writes:

> Oh, the follies of youth. Time to undo the damage of 2.0.0, specifically
> commit 86172ccc16, which split Patch into two separate models using
> concrete inheritance. As noted previously, this introduced a large
> number of unavoidable JOINs across large tables and the performance
> impacts these introduce are blocking other features we want, such as
> improved tagging functionality. To combine these two models, we must do
> the following:
>
> - Update any references to the 'Patch' model to point to the
>   'Submission' model instead
> - Move everything from 'Patch' to 'Submission', including both fields
>   and options
> - Delete the 'Patch' model
> - Rename the 'Submission' model to 'Patch'
>
> With this change, our model "hierarchy" goes from:
>
>   Submission
>     Patch
>     PatchComment
>   Cover
>     CoverComment
>
> To a nice, flat:
>
>   Patch
>     PatchComment
>   Cover
>     CoverComment
>
> I expect this migration to be intensive, particularly for MySQL users
> who will see their entire tables rewritten. Unfortunately I don't see
> any way to resolve this in an easier manner.
>
> Also note that we have to use two migrations and separate the moving of
> fields from the renaming of models, in order to work around a bug in
> Django 1.11. This bug has been fixed since Django 2.0 [1][2] but the fix
> wasn't backported and Django 1.11 is only accepting security fixes at
> this point in time.
>
> [1] https://code.djangoproject.com/ticket/25530
> [2] https://code.djangoproject.com/ticket/31337
>
> Signed-off-by: Stephen Finucane <stephen@that.guru>
> Cc: Daniel Axtens <dja@axtens.net>
> Cc: Stewart Smith <stewart@linux.ibm.com>
> ---
> TODO:
> - Figure out if the indexes are correct (Stewart?). I've just included
>   everything that's displayed on the '/list' page.

You might have more luck with stewart@flamingspork.com now that Stewart
has ascended to the cloud.

Regards,
Daniel

> - If this isn't merged until 3.0, combine the migrations since we will
>   no longer have to support Django 1.11.
> ---
>  patchwork/api/comment.py                      |   4 +-
>  patchwork/management/commands/dumparchive.py  |   2 +-
>  .../0041_merge_patch_submission_a.py          | 281 ++++++++++++++++++
>  .../0042_merge_patch_submission_b.py          |  17 ++
>  patchwork/models.py                           |  94 +++---
>  patchwork/parser.py                           |  16 +-
>  patchwork/tests/test_mboxviews.py             |  26 +-
>  patchwork/tests/utils.py                      |   5 +-
>  patchwork/views/__init__.py                   |   2 +-
>  patchwork/views/utils.py                      |   4 +-
>  10 files changed, 376 insertions(+), 75 deletions(-)
>  create mode 100644 patchwork/migrations/0041_merge_patch_submission_a.py
>  create mode 100644 patchwork/migrations/0042_merge_patch_submission_b.py
>
> diff --git a/patchwork/api/comment.py b/patchwork/api/comment.py
> index 3802dab9..43b26c61 100644
> --- a/patchwork/api/comment.py
> +++ b/patchwork/api/comment.py
> @@ -14,7 +14,7 @@ from patchwork.api.base import PatchworkPermission
>  from patchwork.api.embedded import PersonSerializer
>  from patchwork.models import Cover
>  from patchwork.models import CoverComment
> -from patchwork.models import Submission
> +from patchwork.models import Patch
>  from patchwork.models import PatchComment
>  
>  
> @@ -105,7 +105,7 @@ class PatchCommentList(ListAPIView):
>      lookup_url_kwarg = 'pk'
>  
>      def get_queryset(self):
> -        if not Submission.objects.filter(pk=self.kwargs['pk']).exists():
> +        if not Patch.objects.filter(pk=self.kwargs['pk']).exists():
>              raise Http404
>  
>          return PatchComment.objects.filter(
> diff --git a/patchwork/management/commands/dumparchive.py b/patchwork/management/commands/dumparchive.py
> index 9ee80c8b..e9445eab 100644
> --- a/patchwork/management/commands/dumparchive.py
> +++ b/patchwork/management/commands/dumparchive.py
> @@ -58,7 +58,7 @@ class Command(BaseCommand):
>                      i + 1, len(projects), project.linkname))
>  
>                  with tempfile.NamedTemporaryFile(delete=False) as mbox:
> -                    patches = Patch.objects.filter(patch_project=project)
> +                    patches = Patch.objects.filter(project=project)
>                      count = patches.count()
>                      for j, patch in enumerate(patches):
>                          if not (j % 10):
> diff --git a/patchwork/migrations/0041_merge_patch_submission_a.py b/patchwork/migrations/0041_merge_patch_submission_a.py
> new file mode 100644
> index 00000000..80b8dc2f
> --- /dev/null
> +++ b/patchwork/migrations/0041_merge_patch_submission_a.py
> @@ -0,0 +1,281 @@
> +# -*- coding: utf-8 -*-
> +from __future__ import unicode_literals
> +
> +from django.conf import settings
> +from django.db import connection, migrations, models
> +import django.db.models.deletion
> +
> +import patchwork.fields
> +
> +
> +def migrate_data(apps, schema_editor):
> +    if connection.vendor == 'postgresql':
> +        schema_editor.execute(
> +            """
> +            UPDATE patchwork_submission
> +              SET archived = patchwork_patch.archived2,
> +                  commit_ref = patchwork_patch.commit_ref2,
> +                  delegate_id = patchwork_patch.delegate2_id,
> +                  diff = patchwork_patch.diff2,
> +                  hash = patchwork_patch.hash2,
> +                  number = patchwork_patch.number2,
> +                  pull_url = patchwork_patch.pull_url2,
> +                  series_id = patchwork_patch.series2_id,
> +                  state_id = patchwork_patch.state2_id,
> +            FROM patchwork_patch
> +              WHERE patchwork_submission.id = patchwork_patch.submission_ptr_id
> +            """
> +        )
> +    elif connection.vendor == 'mysql':
> +        schema_editor.execute(
> +            """
> +            UPDATE patchwork_submission, patchwork_patch
> +              SET patchwork_submission.archived = patchwork_patch.archived2,
> +                  patchwork_submission.commit_ref = patchwork_patch.commit_ref2,
> +                  patchwork_submission.delegate_id = patchwork_patch.delegate2_id,
> +                  patchwork_submission.diff = patchwork_patch.diff2,
> +                  patchwork_submission.hash = patchwork_patch.hash2,
> +                  patchwork_submission.number = patchwork_patch.number2,
> +                  patchwork_submission.pull_url = patchwork_patch.pull_url2,
> +                  patchwork_submission.series_id = patchwork_patch.series2_id,
> +                  patchwork_submission.state_id = patchwork_patch.state2_id
> +            WHERE patchwork_submission.id = patchwork_patch.submission_ptr_id
> +            """  # noqa
> +        )
> +    else:
> +        raise Exception('DB not supported')
> +
> +
> +class Migration(migrations.Migration):
> +
> +    dependencies = [
> +        ('patchwork', '0040_add_cover_model'),
> +    ]
> +
> +    operations = [
> +        # move the 'PatchTag' model to point to 'Submission'
> +
> +        migrations.RemoveField(model_name='patch', name='tags',),
> +        migrations.AddField(
> +            model_name='submission',
> +            name='tags',
> +            field=models.ManyToManyField(
> +                through='patchwork.PatchTag', to='patchwork.Tag'
> +            ),
> +        ),
> +        migrations.AlterField(
> +            model_name='patchtag',
> +            name='patch',
> +            field=models.ForeignKey(
> +                on_delete=django.db.models.deletion.CASCADE,
> +                to='patchwork.Submission',
> +            ),
> +        ),
> +
> +        # do the same for any other field that references 'Patch'
> +
> +        migrations.AlterField(
> +            model_name='bundle',
> +            name='patches',
> +            field=models.ManyToManyField(
> +                through='patchwork.BundlePatch', to='patchwork.Submission'
> +            ),
> +        ),
> +        migrations.AlterField(
> +            model_name='bundlepatch',
> +            name='patch',
> +            field=models.ForeignKey(
> +                on_delete=django.db.models.deletion.CASCADE,
> +                to='patchwork.Submission',
> +            ),
> +        ),
> +        migrations.AlterField(
> +            model_name='check',
> +            name='patch',
> +            field=models.ForeignKey(
> +                on_delete=django.db.models.deletion.CASCADE,
> +                to='patchwork.Submission',
> +            ),
> +        ),
> +        migrations.AlterField(
> +            model_name='event',
> +            name='patch',
> +            field=models.ForeignKey(
> +                blank=True,
> +                help_text=b'The patch that this event was created for.',
> +                null=True,
> +                on_delete=django.db.models.deletion.CASCADE,
> +                related_name='+',
> +                to='patchwork.Submission',
> +            ),
> +        ),
> +        migrations.AlterField(
> +            model_name='patchchangenotification',
> +            name='patch',
> +            field=models.OneToOneField(
> +                on_delete=django.db.models.deletion.CASCADE,
> +                primary_key=True,
> +                serialize=False,
> +                to='patchwork.Submission',
> +            ),
> +        ),
> +
> +        # rename all the fields on 'Patch' so we don't have duplicates when we
> +        # add them to 'Submission'
> +
> +        migrations.RemoveIndex(
> +            model_name='patch', name='patch_list_covering_idx',
> +        ),
> +        migrations.AlterUniqueTogether(name='patch', unique_together=set([]),),
> +        migrations.RenameField(
> +            model_name='patch', old_name='archived', new_name='archived2',
> +        ),
> +        migrations.RenameField(
> +            model_name='patch', old_name='commit_ref', new_name='commit_ref2',
> +        ),
> +        migrations.RenameField(
> +            model_name='patch', old_name='delegate', new_name='delegate2',
> +        ),
> +        migrations.RenameField(
> +            model_name='patch', old_name='diff', new_name='diff2',
> +        ),
> +        migrations.RenameField(
> +            model_name='patch', old_name='hash', new_name='hash2',
> +        ),
> +        migrations.RenameField(
> +            model_name='patch', old_name='number', new_name='number2',
> +        ),
> +        migrations.RenameField(
> +            model_name='patch', old_name='pull_url', new_name='pull_url2',
> +        ),
> +        migrations.RenameField(
> +            model_name='patch', old_name='series', new_name='series2',
> +        ),
> +        migrations.RenameField(
> +            model_name='patch', old_name='state', new_name='state2',
> +        ),
> +
> +        # add the fields found on 'Patch' to 'Submission'
> +
> +        migrations.AddField(
> +            model_name='submission',
> +            name='archived',
> +            field=models.BooleanField(default=False),
> +        ),
> +        migrations.AddField(
> +            model_name='submission',
> +            name='commit_ref',
> +            field=models.CharField(blank=True, max_length=255, null=True),
> +        ),
> +        migrations.AddField(
> +            model_name='submission',
> +            name='delegate',
> +            field=models.ForeignKey(
> +                blank=True,
> +                null=True,
> +                on_delete=django.db.models.deletion.CASCADE,
> +                to=settings.AUTH_USER_MODEL,
> +            ),
> +        ),
> +        migrations.AddField(
> +            model_name='submission',
> +            name='diff',
> +            field=models.TextField(blank=True, null=True),
> +        ),
> +        migrations.AddField(
> +            model_name='submission',
> +            name='hash',
> +            field=patchwork.fields.HashField(
> +                blank=True, max_length=40, null=True
> +            ),
> +        ),
> +        migrations.AddField(
> +            model_name='submission',
> +            name='number',
> +            field=models.PositiveSmallIntegerField(
> +                default=None,
> +                help_text=b'The number assigned to this patch in the series',
> +                null=True,
> +            ),
> +        ),
> +        migrations.AddField(
> +            model_name='submission',
> +            name='pull_url',
> +            field=models.CharField(blank=True, max_length=255, null=True),
> +        ),
> +        migrations.AddField(
> +            model_name='submission',
> +            name='series',
> +            field=models.ForeignKey(
> +                blank=True,
> +                null=True,
> +                on_delete=django.db.models.deletion.CASCADE,
> +                related_name='patches',
> +                related_query_name='patch',
> +                to='patchwork.Series',
> +            ),
> +        ),
> +        migrations.AddField(
> +            model_name='submission',
> +            name='state',
> +            field=models.ForeignKey(
> +                null=True,
> +                on_delete=django.db.models.deletion.CASCADE,
> +                to='patchwork.State',
> +            ),
> +        ),
> +
> +        # copy the data from 'Patch' to 'Submission'
> +
> +        migrations.RunPython(migrate_data, None, atomic=False),
> +
> +        # configure metadata for the 'Submission' model
> +
> +        migrations.AlterModelOptions(
> +            name='submission',
> +            options={
> +                'base_manager_name': 'objects',
> +                'ordering': ['date'],
> +                'verbose_name_plural': 'Patches',
> +            },
> +        ),
> +        migrations.AlterUniqueTogether(
> +            name='submission',
> +            unique_together=set([('series', 'number'), ('msgid', 'project')]),
> +        ),
> +        migrations.RemoveIndex(
> +            model_name='submission', name='submission_covering_idx',
> +        ),
> +        migrations.AddIndex(
> +            model_name='submission',
> +            index=models.Index(
> +                fields=[
> +                    b'archived',
> +                    b'state',
> +                    b'delegate',
> +                    b'date',
> +                    b'project',
> +                    b'submitter',
> +                    b'name',
> +                ],
> +                name=b'patch_covering_idx',
> +            ),
> +        ),
> +
> +        # remove the foreign key fields from the 'Patch' model
> +
> +        migrations.RemoveField(model_name='patch', name='delegate2',),
> +        migrations.RemoveField(model_name='patch', name='patch_project',),
> +        migrations.RemoveField(model_name='patch', name='series2',),
> +        migrations.RemoveField(model_name='patch', name='state2',),
> +        migrations.RemoveField(model_name='patch', name='submission_ptr',),
> +
> +        # drop the 'Patch' model and rename 'Submission' to 'Patch'; this is
> +        # done in a seperate migration to work around bug #31337 for anyone
> +        # still using Django 1.11
> +        #
> +        # https://code.djangoproject.com/ticket/3133
> +
> +        # migrations.DeleteModel(name='Patch',),
> +        # migrations.RenameModel(old_name='Submission', new_name='Patch',),
> +    ]
> diff --git a/patchwork/migrations/0042_merge_patch_submission_b.py b/patchwork/migrations/0042_merge_patch_submission_b.py
> new file mode 100644
> index 00000000..0051ce87
> --- /dev/null
> +++ b/patchwork/migrations/0042_merge_patch_submission_b.py
> @@ -0,0 +1,17 @@
> +# -*- coding: utf-8 -*-
> +
> +from __future__ import unicode_literals
> +
> +from django.db import migrations
> +
> +
> +class Migration(migrations.Migration):
> +
> +    dependencies = [
> +        ('patchwork', '0041_merge_patch_submission_a'),
> +    ]
> +
> +    operations = [
> +        migrations.DeleteModel(name='Patch',),
> +        migrations.RenameModel(old_name='Submission', new_name='Patch',),
> +    ]
> diff --git a/patchwork/models.py b/patchwork/models.py
> index 37cbce32..c7b95de4 100644
> --- a/patchwork/models.py
> +++ b/patchwork/models.py
> @@ -169,7 +169,7 @@ class UserProfile(models.Model):
>      @property
>      def contributor_projects(self):
>          submitters = Person.objects.filter(user=self.user)
> -        return Project.objects.filter(id__in=Submission.objects.filter(
> +        return Project.objects.filter(id__in=Patch.objects.filter(
>              submitter__in=submitters).values('project_id').query)
>  
>      @property
> @@ -292,8 +292,7 @@ class PatchQuerySet(models.query.QuerySet):
>              select[tag.attr_name] = (
>                  "coalesce("
>                  "(SELECT count FROM patchwork_patchtag"
> -                " WHERE patchwork_patchtag.patch_id="
> -                "patchwork_patch.submission_ptr_id"
> +                " WHERE patchwork_patchtag.patch_id=patchwork_patch.id"
>                  " AND patchwork_patchtag.tag_id=%s), 0)")
>              select_params.append(tag.id)
>  
> @@ -424,24 +423,8 @@ class Cover(FilenameMixin, EmailMixin, SubmissionMixin):
>          ]
>  
>  
> -class Submission(SubmissionMixin, FilenameMixin, EmailMixin):
> -
> -    class Meta:
> -        ordering = ['date']
> -        unique_together = [('msgid', 'project')]
> -        indexes = [
> -            # This is a covering index for the /list/ query
> -            # Like what we have for Patch, but used for displaying what we want
> -            # rather than for working out the count (of course, this all
> -            # depends on the SQL optimiser of your db engine)
> -            models.Index(fields=['date', 'project', 'submitter', 'name'],
> -                         name='submission_covering_idx'),
> -        ]
> -
> -
>  @python_2_unicode_compatible
> -class Patch(Submission):
> -    # patch metadata
> +class Patch(FilenameMixin, EmailMixin, SubmissionMixin):
>  
>      diff = models.TextField(null=True, blank=True)
>      commit_ref = models.CharField(max_length=255, null=True, blank=True)
> @@ -450,24 +433,31 @@ class Patch(Submission):
>  
>      # patchwork metadata
>  
> -    delegate = models.ForeignKey(User, blank=True, null=True,
> -                                 on_delete=models.CASCADE)
> +    delegate = models.ForeignKey(
> +        User,
> +        blank=True,
> +        null=True,
> +        on_delete=models.CASCADE,
> +    )
>      state = models.ForeignKey(State, null=True, on_delete=models.CASCADE)
>      archived = models.BooleanField(default=False)
>      hash = HashField(null=True, blank=True)
>  
> -    # duplicate project from submission in subclass so we can count the
> -    # patches in a project without needing to do a JOIN.
> -    patch_project = models.ForeignKey(Project, on_delete=models.CASCADE)
> -
>      # series metadata
>  
>      series = models.ForeignKey(
> -        'Series', null=True, blank=True, on_delete=models.CASCADE,
> -        related_name='patches', related_query_name='patch')
> +        'Series',
> +        null=True,
> +        blank=True,
> +        on_delete=models.CASCADE,
> +        related_name='patches',
> +        related_query_name='patch',
> +    )
>      number = models.PositiveSmallIntegerField(
> -        default=None, null=True,
> -        help_text='The number assigned to this patch in the series')
> +        default=None,
> +        null=True,
> +        help_text='The number assigned to this patch in the series',
> +    )
>  
>      objects = PatchManager()
>  
> @@ -632,14 +622,23 @@ class Patch(Submission):
>  
>      class Meta:
>          verbose_name_plural = 'Patches'
> +        ordering = ['date']
>          base_manager_name = 'objects'
> -        unique_together = [('series', 'number')]
> -
> +        unique_together = [('msgid', 'project'), ('series', 'number')]
>          indexes = [
>              # This is a covering index for the /list/ query
> -            models.Index(fields=['archived', 'patch_project', 'state',
> -                                 'delegate'],
> -                         name='patch_list_covering_idx'),
> +            models.Index(
> +                fields=[
> +                    'archived',
> +                    'state',
> +                    'delegate',
> +                    'date',
> +                    'project',
> +                    'submitter',
> +                    'name',
> +                ],
> +                name='patch_covering_idx',
> +            ),
>          ]
>  
>  
> @@ -678,7 +677,7 @@ class PatchComment(EmailMixin, models.Model):
>      # parent
>  
>      patch = models.ForeignKey(
> -        Submission,
> +        Patch,
>          related_name='comments',
>          related_query_name='comment',
>          on_delete=models.CASCADE,
> @@ -698,15 +697,11 @@ class PatchComment(EmailMixin, models.Model):
>  
>      def save(self, *args, **kwargs):
>          super(PatchComment, self).save(*args, **kwargs)
> -        # TODO(stephenfin): Update this once patch is flattened
> -        if hasattr(self.patch, 'patch'):
> -            self.patch.patch.refresh_tag_counts()
> +        self.patch.refresh_tag_counts()
>  
>      def delete(self, *args, **kwargs):
>          super(PatchComment, self).delete(*args, **kwargs)
> -        # TODO(stephenfin): Update this once patch is flattened
> -        if hasattr(self.patch, 'patch'):
> -            self.patch.patch.refresh_tag_counts()
> +        self.patch.refresh_tag_counts()
>  
>      def is_editable(self, user):
>          return False
> @@ -749,10 +744,10 @@ class Series(FilenameMixin, models.Model):
>  
>      @staticmethod
>      def _format_name(obj):
> -        # The parser ensure 'Submission.name' will always take the form
> -        # 'subject' or '[prefix_a,prefix_b,...] subject'. There will never be
> -        # multiple prefixes (text inside brackets), thus, we don't need to
> -        # account for multiple prefixes here.
> +        # The parser ensure 'Cover.name' will always take the form 'subject' or
> +        # '[prefix_a,prefix_b,...] subject'. There will never be multiple
> +        # prefixes (text inside brackets), thus, we don't need to account for
> +        # multiple prefixes here.
>          prefix_re = re.compile(r'^\[([^\]]*)\]\s*(.*)$')
>          match = prefix_re.match(obj.name)
>          if match:
> @@ -1118,7 +1113,10 @@ class EmailOptout(models.Model):
>  
>  
>  class PatchChangeNotification(models.Model):
> -    patch = models.OneToOneField(Patch, primary_key=True,
> -                                 on_delete=models.CASCADE)
> +    patch = models.OneToOneField(
> +        Patch,
> +        primary_key=True,
> +        on_delete=models.CASCADE,
> +    )
>      last_modified = models.DateTimeField(default=datetime.datetime.utcnow)
>      orig_state = models.ForeignKey(State, on_delete=models.CASCADE)
> diff --git a/patchwork/parser.py b/patchwork/parser.py
> index cd82f3be..69cfb63a 100644
> --- a/patchwork/parser.py
> +++ b/patchwork/parser.py
> @@ -30,7 +30,6 @@ from patchwork.models import Project
>  from patchwork.models import Series
>  from patchwork.models import SeriesReference
>  from patchwork.models import State
> -from patchwork.models import Submission
>  
>  
>  _hunk_re = re.compile(r'^\@\@ -\d+(?:,(\d+))? \+\d+(?:,(\d+))? \@\@')
> @@ -647,14 +646,14 @@ def find_comment_content(mail):
>      return None, commentbuf
>  
>  
> -def find_submission_for_comment(project, refs):
> +def find_patch_for_comment(project, refs):
>      for ref in refs:
>          ref = ref[:255]
>          # first, check for a direct reply
>          try:
> -            submission = Submission.objects.get(project=project, msgid=ref)
> -            return submission
> -        except Submission.DoesNotExist:
> +            patch = Patch.objects.get(project=project, msgid=ref)
> +            return patch
> +        except Patch.DoesNotExist:
>              pass
>  
>          # see if we have comments that refer to a patch
> @@ -1099,7 +1098,6 @@ def parse_mail(mail, list_id=None):
>              patch = Patch.objects.create(
>                  msgid=msgid,
>                  project=project,
> -                patch_project=project,
>                  name=name[:255],
>                  date=date,
>                  headers=headers,
> @@ -1273,13 +1271,13 @@ def parse_mail(mail, list_id=None):
>      # comments
>  
>      # we only save comments if we have the parent email
> -    submission = find_submission_for_comment(project, refs)
> -    if submission:
> +    patch = find_patch_for_comment(project, refs)
> +    if patch:
>          author = get_or_create_author(mail, project)
>  
>          try:
>              comment = PatchComment.objects.create(
> -                patch=submission,
> +                patch=patch,
>                  msgid=msgid,
>                  date=date,
>                  headers=headers,
> diff --git a/patchwork/tests/test_mboxviews.py b/patchwork/tests/test_mboxviews.py
> index a7b0186a..1535c5cb 100644
> --- a/patchwork/tests/test_mboxviews.py
> +++ b/patchwork/tests/test_mboxviews.py
> @@ -268,9 +268,12 @@ class MboxSeriesDependencies(TestCase):
>      def test_patch_with_wildcard_series(self):
>          _, patch_a, patch_b = self._create_patches()
>  
> -        response = self.client.get('%s?series=*' % reverse(
> -            'patch-mbox', args=[patch_b.patch.project.linkname,
> -                                patch_b.patch.url_msgid]))
> +        response = self.client.get(
> +            '%s?series=*' % reverse(
> +                'patch-mbox',
> +                args=[patch_b.project.linkname, patch_b.url_msgid],
> +            ),
> +        )
>  
>          self.assertContains(response, patch_a.content)
>          self.assertContains(response, patch_b.content)
> @@ -279,9 +282,12 @@ class MboxSeriesDependencies(TestCase):
>          series, patch_a, patch_b = self._create_patches()
>  
>          response = self.client.get('%s?series=%d' % (
> -            reverse('patch-mbox', args=[patch_b.patch.project.linkname,
> -                                        patch_b.patch.url_msgid]),
> -            series.id))
> +            reverse(
> +                'patch-mbox',
> +                args=[patch_b.project.linkname, patch_b.url_msgid],
> +            ),
> +            series.id,
> +        ))
>  
>          self.assertContains(response, patch_a.content)
>          self.assertContains(response, patch_b.content)
> @@ -291,8 +297,12 @@ class MboxSeriesDependencies(TestCase):
>  
>          for value in ('foo', str(series.id + 1)):
>              response = self.client.get('%s?series=%s' % (
> -                reverse('patch-mbox', args=[patch_b.patch.project.linkname,
> -                                            patch_b.patch.url_msgid]), value))
> +                reverse(
> +                    'patch-mbox',
> +                    args=[patch_b.project.linkname, patch_b.url_msgid]
> +                ),
> +                value,
> +            ))
>  
>              self.assertEqual(response.status_code, 404)
>  
> diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py
> index aeeb14d7..b24a992e 100644
> --- a/patchwork/tests/utils.py
> +++ b/patchwork/tests/utils.py
> @@ -190,9 +190,6 @@ def create_patch(**kwargs):
>      }
>      values.update(kwargs)
>  
> -    if 'patch_project' not in values:
> -        values['patch_project'] = values['project']
> -
>      patch = Patch.objects.create(**values)
>  
>      if series:
> @@ -311,7 +308,7 @@ def create_series_reference(**kwargs):
>  
>  
>  def _create_submissions(create_func, count=1, **kwargs):
> -    """Create 'count' Submission-based objects.
> +    """Create 'count' SubmissionMixin-based objects.
>  
>      Args:
>          count (int): Number of patches to create
> diff --git a/patchwork/views/__init__.py b/patchwork/views/__init__.py
> index ad17a070..3efe90cd 100644
> --- a/patchwork/views/__init__.py
> +++ b/patchwork/views/__init__.py
> @@ -257,7 +257,7 @@ def generic_list(request, project, view, view_args=None, filter_settings=None,
>              context['filters'].set_status(filterclass, setting)
>  
>      if patches is None:
> -        patches = Patch.objects.filter(patch_project=project)
> +        patches = Patch.objects.filter(project=project)
>  
>      # annotate with tag counts
>      patches = patches.with_tag_counts(project)
> diff --git a/patchwork/views/utils.py b/patchwork/views/utils.py
> index 875edf45..b301fba9 100644
> --- a/patchwork/views/utils.py
> +++ b/patchwork/views/utils.py
> @@ -183,8 +183,8 @@ def series_to_mbox(series):
>      """
>      mbox = []
>  
> -    for dep in series.patches.all().order_by('number'):
> -        mbox.append(patch_to_mbox(dep.patch))
> +    for patch in series.patches.all().order_by('number'):
> +        mbox.append(patch_to_mbox(patch))
>  
>      return '\n'.join(mbox)
>  
> -- 
> 2.24.1
diff mbox series

Patch

diff --git a/patchwork/api/comment.py b/patchwork/api/comment.py
index 3802dab9..43b26c61 100644
--- a/patchwork/api/comment.py
+++ b/patchwork/api/comment.py
@@ -14,7 +14,7 @@  from patchwork.api.base import PatchworkPermission
 from patchwork.api.embedded import PersonSerializer
 from patchwork.models import Cover
 from patchwork.models import CoverComment
-from patchwork.models import Submission
+from patchwork.models import Patch
 from patchwork.models import PatchComment
 
 
@@ -105,7 +105,7 @@  class PatchCommentList(ListAPIView):
     lookup_url_kwarg = 'pk'
 
     def get_queryset(self):
-        if not Submission.objects.filter(pk=self.kwargs['pk']).exists():
+        if not Patch.objects.filter(pk=self.kwargs['pk']).exists():
             raise Http404
 
         return PatchComment.objects.filter(
diff --git a/patchwork/management/commands/dumparchive.py b/patchwork/management/commands/dumparchive.py
index 9ee80c8b..e9445eab 100644
--- a/patchwork/management/commands/dumparchive.py
+++ b/patchwork/management/commands/dumparchive.py
@@ -58,7 +58,7 @@  class Command(BaseCommand):
                     i + 1, len(projects), project.linkname))
 
                 with tempfile.NamedTemporaryFile(delete=False) as mbox:
-                    patches = Patch.objects.filter(patch_project=project)
+                    patches = Patch.objects.filter(project=project)
                     count = patches.count()
                     for j, patch in enumerate(patches):
                         if not (j % 10):
diff --git a/patchwork/migrations/0041_merge_patch_submission_a.py b/patchwork/migrations/0041_merge_patch_submission_a.py
new file mode 100644
index 00000000..80b8dc2f
--- /dev/null
+++ b/patchwork/migrations/0041_merge_patch_submission_a.py
@@ -0,0 +1,281 @@ 
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.conf import settings
+from django.db import connection, migrations, models
+import django.db.models.deletion
+
+import patchwork.fields
+
+
+def migrate_data(apps, schema_editor):
+    if connection.vendor == 'postgresql':
+        schema_editor.execute(
+            """
+            UPDATE patchwork_submission
+              SET archived = patchwork_patch.archived2,
+                  commit_ref = patchwork_patch.commit_ref2,
+                  delegate_id = patchwork_patch.delegate2_id,
+                  diff = patchwork_patch.diff2,
+                  hash = patchwork_patch.hash2,
+                  number = patchwork_patch.number2,
+                  pull_url = patchwork_patch.pull_url2,
+                  series_id = patchwork_patch.series2_id,
+                  state_id = patchwork_patch.state2_id,
+            FROM patchwork_patch
+              WHERE patchwork_submission.id = patchwork_patch.submission_ptr_id
+            """
+        )
+    elif connection.vendor == 'mysql':
+        schema_editor.execute(
+            """
+            UPDATE patchwork_submission, patchwork_patch
+              SET patchwork_submission.archived = patchwork_patch.archived2,
+                  patchwork_submission.commit_ref = patchwork_patch.commit_ref2,
+                  patchwork_submission.delegate_id = patchwork_patch.delegate2_id,
+                  patchwork_submission.diff = patchwork_patch.diff2,
+                  patchwork_submission.hash = patchwork_patch.hash2,
+                  patchwork_submission.number = patchwork_patch.number2,
+                  patchwork_submission.pull_url = patchwork_patch.pull_url2,
+                  patchwork_submission.series_id = patchwork_patch.series2_id,
+                  patchwork_submission.state_id = patchwork_patch.state2_id
+            WHERE patchwork_submission.id = patchwork_patch.submission_ptr_id
+            """  # noqa
+        )
+    else:
+        raise Exception('DB not supported')
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('patchwork', '0040_add_cover_model'),
+    ]
+
+    operations = [
+        # move the 'PatchTag' model to point to 'Submission'
+
+        migrations.RemoveField(model_name='patch', name='tags',),
+        migrations.AddField(
+            model_name='submission',
+            name='tags',
+            field=models.ManyToManyField(
+                through='patchwork.PatchTag', to='patchwork.Tag'
+            ),
+        ),
+        migrations.AlterField(
+            model_name='patchtag',
+            name='patch',
+            field=models.ForeignKey(
+                on_delete=django.db.models.deletion.CASCADE,
+                to='patchwork.Submission',
+            ),
+        ),
+
+        # do the same for any other field that references 'Patch'
+
+        migrations.AlterField(
+            model_name='bundle',
+            name='patches',
+            field=models.ManyToManyField(
+                through='patchwork.BundlePatch', to='patchwork.Submission'
+            ),
+        ),
+        migrations.AlterField(
+            model_name='bundlepatch',
+            name='patch',
+            field=models.ForeignKey(
+                on_delete=django.db.models.deletion.CASCADE,
+                to='patchwork.Submission',
+            ),
+        ),
+        migrations.AlterField(
+            model_name='check',
+            name='patch',
+            field=models.ForeignKey(
+                on_delete=django.db.models.deletion.CASCADE,
+                to='patchwork.Submission',
+            ),
+        ),
+        migrations.AlterField(
+            model_name='event',
+            name='patch',
+            field=models.ForeignKey(
+                blank=True,
+                help_text=b'The patch that this event was created for.',
+                null=True,
+                on_delete=django.db.models.deletion.CASCADE,
+                related_name='+',
+                to='patchwork.Submission',
+            ),
+        ),
+        migrations.AlterField(
+            model_name='patchchangenotification',
+            name='patch',
+            field=models.OneToOneField(
+                on_delete=django.db.models.deletion.CASCADE,
+                primary_key=True,
+                serialize=False,
+                to='patchwork.Submission',
+            ),
+        ),
+
+        # rename all the fields on 'Patch' so we don't have duplicates when we
+        # add them to 'Submission'
+
+        migrations.RemoveIndex(
+            model_name='patch', name='patch_list_covering_idx',
+        ),
+        migrations.AlterUniqueTogether(name='patch', unique_together=set([]),),
+        migrations.RenameField(
+            model_name='patch', old_name='archived', new_name='archived2',
+        ),
+        migrations.RenameField(
+            model_name='patch', old_name='commit_ref', new_name='commit_ref2',
+        ),
+        migrations.RenameField(
+            model_name='patch', old_name='delegate', new_name='delegate2',
+        ),
+        migrations.RenameField(
+            model_name='patch', old_name='diff', new_name='diff2',
+        ),
+        migrations.RenameField(
+            model_name='patch', old_name='hash', new_name='hash2',
+        ),
+        migrations.RenameField(
+            model_name='patch', old_name='number', new_name='number2',
+        ),
+        migrations.RenameField(
+            model_name='patch', old_name='pull_url', new_name='pull_url2',
+        ),
+        migrations.RenameField(
+            model_name='patch', old_name='series', new_name='series2',
+        ),
+        migrations.RenameField(
+            model_name='patch', old_name='state', new_name='state2',
+        ),
+
+        # add the fields found on 'Patch' to 'Submission'
+
+        migrations.AddField(
+            model_name='submission',
+            name='archived',
+            field=models.BooleanField(default=False),
+        ),
+        migrations.AddField(
+            model_name='submission',
+            name='commit_ref',
+            field=models.CharField(blank=True, max_length=255, null=True),
+        ),
+        migrations.AddField(
+            model_name='submission',
+            name='delegate',
+            field=models.ForeignKey(
+                blank=True,
+                null=True,
+                on_delete=django.db.models.deletion.CASCADE,
+                to=settings.AUTH_USER_MODEL,
+            ),
+        ),
+        migrations.AddField(
+            model_name='submission',
+            name='diff',
+            field=models.TextField(blank=True, null=True),
+        ),
+        migrations.AddField(
+            model_name='submission',
+            name='hash',
+            field=patchwork.fields.HashField(
+                blank=True, max_length=40, null=True
+            ),
+        ),
+        migrations.AddField(
+            model_name='submission',
+            name='number',
+            field=models.PositiveSmallIntegerField(
+                default=None,
+                help_text=b'The number assigned to this patch in the series',
+                null=True,
+            ),
+        ),
+        migrations.AddField(
+            model_name='submission',
+            name='pull_url',
+            field=models.CharField(blank=True, max_length=255, null=True),
+        ),
+        migrations.AddField(
+            model_name='submission',
+            name='series',
+            field=models.ForeignKey(
+                blank=True,
+                null=True,
+                on_delete=django.db.models.deletion.CASCADE,
+                related_name='patches',
+                related_query_name='patch',
+                to='patchwork.Series',
+            ),
+        ),
+        migrations.AddField(
+            model_name='submission',
+            name='state',
+            field=models.ForeignKey(
+                null=True,
+                on_delete=django.db.models.deletion.CASCADE,
+                to='patchwork.State',
+            ),
+        ),
+
+        # copy the data from 'Patch' to 'Submission'
+
+        migrations.RunPython(migrate_data, None, atomic=False),
+
+        # configure metadata for the 'Submission' model
+
+        migrations.AlterModelOptions(
+            name='submission',
+            options={
+                'base_manager_name': 'objects',
+                'ordering': ['date'],
+                'verbose_name_plural': 'Patches',
+            },
+        ),
+        migrations.AlterUniqueTogether(
+            name='submission',
+            unique_together=set([('series', 'number'), ('msgid', 'project')]),
+        ),
+        migrations.RemoveIndex(
+            model_name='submission', name='submission_covering_idx',
+        ),
+        migrations.AddIndex(
+            model_name='submission',
+            index=models.Index(
+                fields=[
+                    b'archived',
+                    b'state',
+                    b'delegate',
+                    b'date',
+                    b'project',
+                    b'submitter',
+                    b'name',
+                ],
+                name=b'patch_covering_idx',
+            ),
+        ),
+
+        # remove the foreign key fields from the 'Patch' model
+
+        migrations.RemoveField(model_name='patch', name='delegate2',),
+        migrations.RemoveField(model_name='patch', name='patch_project',),
+        migrations.RemoveField(model_name='patch', name='series2',),
+        migrations.RemoveField(model_name='patch', name='state2',),
+        migrations.RemoveField(model_name='patch', name='submission_ptr',),
+
+        # drop the 'Patch' model and rename 'Submission' to 'Patch'; this is
+        # done in a seperate migration to work around bug #31337 for anyone
+        # still using Django 1.11
+        #
+        # https://code.djangoproject.com/ticket/3133
+
+        # migrations.DeleteModel(name='Patch',),
+        # migrations.RenameModel(old_name='Submission', new_name='Patch',),
+    ]
diff --git a/patchwork/migrations/0042_merge_patch_submission_b.py b/patchwork/migrations/0042_merge_patch_submission_b.py
new file mode 100644
index 00000000..0051ce87
--- /dev/null
+++ b/patchwork/migrations/0042_merge_patch_submission_b.py
@@ -0,0 +1,17 @@ 
+# -*- coding: utf-8 -*-
+
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('patchwork', '0041_merge_patch_submission_a'),
+    ]
+
+    operations = [
+        migrations.DeleteModel(name='Patch',),
+        migrations.RenameModel(old_name='Submission', new_name='Patch',),
+    ]
diff --git a/patchwork/models.py b/patchwork/models.py
index 37cbce32..c7b95de4 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -169,7 +169,7 @@  class UserProfile(models.Model):
     @property
     def contributor_projects(self):
         submitters = Person.objects.filter(user=self.user)
-        return Project.objects.filter(id__in=Submission.objects.filter(
+        return Project.objects.filter(id__in=Patch.objects.filter(
             submitter__in=submitters).values('project_id').query)
 
     @property
@@ -292,8 +292,7 @@  class PatchQuerySet(models.query.QuerySet):
             select[tag.attr_name] = (
                 "coalesce("
                 "(SELECT count FROM patchwork_patchtag"
-                " WHERE patchwork_patchtag.patch_id="
-                "patchwork_patch.submission_ptr_id"
+                " WHERE patchwork_patchtag.patch_id=patchwork_patch.id"
                 " AND patchwork_patchtag.tag_id=%s), 0)")
             select_params.append(tag.id)
 
@@ -424,24 +423,8 @@  class Cover(FilenameMixin, EmailMixin, SubmissionMixin):
         ]
 
 
-class Submission(SubmissionMixin, FilenameMixin, EmailMixin):
-
-    class Meta:
-        ordering = ['date']
-        unique_together = [('msgid', 'project')]
-        indexes = [
-            # This is a covering index for the /list/ query
-            # Like what we have for Patch, but used for displaying what we want
-            # rather than for working out the count (of course, this all
-            # depends on the SQL optimiser of your db engine)
-            models.Index(fields=['date', 'project', 'submitter', 'name'],
-                         name='submission_covering_idx'),
-        ]
-
-
 @python_2_unicode_compatible
-class Patch(Submission):
-    # patch metadata
+class Patch(FilenameMixin, EmailMixin, SubmissionMixin):
 
     diff = models.TextField(null=True, blank=True)
     commit_ref = models.CharField(max_length=255, null=True, blank=True)
@@ -450,24 +433,31 @@  class Patch(Submission):
 
     # patchwork metadata
 
-    delegate = models.ForeignKey(User, blank=True, null=True,
-                                 on_delete=models.CASCADE)
+    delegate = models.ForeignKey(
+        User,
+        blank=True,
+        null=True,
+        on_delete=models.CASCADE,
+    )
     state = models.ForeignKey(State, null=True, on_delete=models.CASCADE)
     archived = models.BooleanField(default=False)
     hash = HashField(null=True, blank=True)
 
-    # duplicate project from submission in subclass so we can count the
-    # patches in a project without needing to do a JOIN.
-    patch_project = models.ForeignKey(Project, on_delete=models.CASCADE)
-
     # series metadata
 
     series = models.ForeignKey(
-        'Series', null=True, blank=True, on_delete=models.CASCADE,
-        related_name='patches', related_query_name='patch')
+        'Series',
+        null=True,
+        blank=True,
+        on_delete=models.CASCADE,
+        related_name='patches',
+        related_query_name='patch',
+    )
     number = models.PositiveSmallIntegerField(
-        default=None, null=True,
-        help_text='The number assigned to this patch in the series')
+        default=None,
+        null=True,
+        help_text='The number assigned to this patch in the series',
+    )
 
     objects = PatchManager()
 
@@ -632,14 +622,23 @@  class Patch(Submission):
 
     class Meta:
         verbose_name_plural = 'Patches'
+        ordering = ['date']
         base_manager_name = 'objects'
-        unique_together = [('series', 'number')]
-
+        unique_together = [('msgid', 'project'), ('series', 'number')]
         indexes = [
             # This is a covering index for the /list/ query
-            models.Index(fields=['archived', 'patch_project', 'state',
-                                 'delegate'],
-                         name='patch_list_covering_idx'),
+            models.Index(
+                fields=[
+                    'archived',
+                    'state',
+                    'delegate',
+                    'date',
+                    'project',
+                    'submitter',
+                    'name',
+                ],
+                name='patch_covering_idx',
+            ),
         ]
 
 
@@ -678,7 +677,7 @@  class PatchComment(EmailMixin, models.Model):
     # parent
 
     patch = models.ForeignKey(
-        Submission,
+        Patch,
         related_name='comments',
         related_query_name='comment',
         on_delete=models.CASCADE,
@@ -698,15 +697,11 @@  class PatchComment(EmailMixin, models.Model):
 
     def save(self, *args, **kwargs):
         super(PatchComment, self).save(*args, **kwargs)
-        # TODO(stephenfin): Update this once patch is flattened
-        if hasattr(self.patch, 'patch'):
-            self.patch.patch.refresh_tag_counts()
+        self.patch.refresh_tag_counts()
 
     def delete(self, *args, **kwargs):
         super(PatchComment, self).delete(*args, **kwargs)
-        # TODO(stephenfin): Update this once patch is flattened
-        if hasattr(self.patch, 'patch'):
-            self.patch.patch.refresh_tag_counts()
+        self.patch.refresh_tag_counts()
 
     def is_editable(self, user):
         return False
@@ -749,10 +744,10 @@  class Series(FilenameMixin, models.Model):
 
     @staticmethod
     def _format_name(obj):
-        # The parser ensure 'Submission.name' will always take the form
-        # 'subject' or '[prefix_a,prefix_b,...] subject'. There will never be
-        # multiple prefixes (text inside brackets), thus, we don't need to
-        # account for multiple prefixes here.
+        # The parser ensure 'Cover.name' will always take the form 'subject' or
+        # '[prefix_a,prefix_b,...] subject'. There will never be multiple
+        # prefixes (text inside brackets), thus, we don't need to account for
+        # multiple prefixes here.
         prefix_re = re.compile(r'^\[([^\]]*)\]\s*(.*)$')
         match = prefix_re.match(obj.name)
         if match:
@@ -1118,7 +1113,10 @@  class EmailOptout(models.Model):
 
 
 class PatchChangeNotification(models.Model):
-    patch = models.OneToOneField(Patch, primary_key=True,
-                                 on_delete=models.CASCADE)
+    patch = models.OneToOneField(
+        Patch,
+        primary_key=True,
+        on_delete=models.CASCADE,
+    )
     last_modified = models.DateTimeField(default=datetime.datetime.utcnow)
     orig_state = models.ForeignKey(State, on_delete=models.CASCADE)
diff --git a/patchwork/parser.py b/patchwork/parser.py
index cd82f3be..69cfb63a 100644
--- a/patchwork/parser.py
+++ b/patchwork/parser.py
@@ -30,7 +30,6 @@  from patchwork.models import Project
 from patchwork.models import Series
 from patchwork.models import SeriesReference
 from patchwork.models import State
-from patchwork.models import Submission
 
 
 _hunk_re = re.compile(r'^\@\@ -\d+(?:,(\d+))? \+\d+(?:,(\d+))? \@\@')
@@ -647,14 +646,14 @@  def find_comment_content(mail):
     return None, commentbuf
 
 
-def find_submission_for_comment(project, refs):
+def find_patch_for_comment(project, refs):
     for ref in refs:
         ref = ref[:255]
         # first, check for a direct reply
         try:
-            submission = Submission.objects.get(project=project, msgid=ref)
-            return submission
-        except Submission.DoesNotExist:
+            patch = Patch.objects.get(project=project, msgid=ref)
+            return patch
+        except Patch.DoesNotExist:
             pass
 
         # see if we have comments that refer to a patch
@@ -1099,7 +1098,6 @@  def parse_mail(mail, list_id=None):
             patch = Patch.objects.create(
                 msgid=msgid,
                 project=project,
-                patch_project=project,
                 name=name[:255],
                 date=date,
                 headers=headers,
@@ -1273,13 +1271,13 @@  def parse_mail(mail, list_id=None):
     # comments
 
     # we only save comments if we have the parent email
-    submission = find_submission_for_comment(project, refs)
-    if submission:
+    patch = find_patch_for_comment(project, refs)
+    if patch:
         author = get_or_create_author(mail, project)
 
         try:
             comment = PatchComment.objects.create(
-                patch=submission,
+                patch=patch,
                 msgid=msgid,
                 date=date,
                 headers=headers,
diff --git a/patchwork/tests/test_mboxviews.py b/patchwork/tests/test_mboxviews.py
index a7b0186a..1535c5cb 100644
--- a/patchwork/tests/test_mboxviews.py
+++ b/patchwork/tests/test_mboxviews.py
@@ -268,9 +268,12 @@  class MboxSeriesDependencies(TestCase):
     def test_patch_with_wildcard_series(self):
         _, patch_a, patch_b = self._create_patches()
 
-        response = self.client.get('%s?series=*' % reverse(
-            'patch-mbox', args=[patch_b.patch.project.linkname,
-                                patch_b.patch.url_msgid]))
+        response = self.client.get(
+            '%s?series=*' % reverse(
+                'patch-mbox',
+                args=[patch_b.project.linkname, patch_b.url_msgid],
+            ),
+        )
 
         self.assertContains(response, patch_a.content)
         self.assertContains(response, patch_b.content)
@@ -279,9 +282,12 @@  class MboxSeriesDependencies(TestCase):
         series, patch_a, patch_b = self._create_patches()
 
         response = self.client.get('%s?series=%d' % (
-            reverse('patch-mbox', args=[patch_b.patch.project.linkname,
-                                        patch_b.patch.url_msgid]),
-            series.id))
+            reverse(
+                'patch-mbox',
+                args=[patch_b.project.linkname, patch_b.url_msgid],
+            ),
+            series.id,
+        ))
 
         self.assertContains(response, patch_a.content)
         self.assertContains(response, patch_b.content)
@@ -291,8 +297,12 @@  class MboxSeriesDependencies(TestCase):
 
         for value in ('foo', str(series.id + 1)):
             response = self.client.get('%s?series=%s' % (
-                reverse('patch-mbox', args=[patch_b.patch.project.linkname,
-                                            patch_b.patch.url_msgid]), value))
+                reverse(
+                    'patch-mbox',
+                    args=[patch_b.project.linkname, patch_b.url_msgid]
+                ),
+                value,
+            ))
 
             self.assertEqual(response.status_code, 404)
 
diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py
index aeeb14d7..b24a992e 100644
--- a/patchwork/tests/utils.py
+++ b/patchwork/tests/utils.py
@@ -190,9 +190,6 @@  def create_patch(**kwargs):
     }
     values.update(kwargs)
 
-    if 'patch_project' not in values:
-        values['patch_project'] = values['project']
-
     patch = Patch.objects.create(**values)
 
     if series:
@@ -311,7 +308,7 @@  def create_series_reference(**kwargs):
 
 
 def _create_submissions(create_func, count=1, **kwargs):
-    """Create 'count' Submission-based objects.
+    """Create 'count' SubmissionMixin-based objects.
 
     Args:
         count (int): Number of patches to create
diff --git a/patchwork/views/__init__.py b/patchwork/views/__init__.py
index ad17a070..3efe90cd 100644
--- a/patchwork/views/__init__.py
+++ b/patchwork/views/__init__.py
@@ -257,7 +257,7 @@  def generic_list(request, project, view, view_args=None, filter_settings=None,
             context['filters'].set_status(filterclass, setting)
 
     if patches is None:
-        patches = Patch.objects.filter(patch_project=project)
+        patches = Patch.objects.filter(project=project)
 
     # annotate with tag counts
     patches = patches.with_tag_counts(project)
diff --git a/patchwork/views/utils.py b/patchwork/views/utils.py
index 875edf45..b301fba9 100644
--- a/patchwork/views/utils.py
+++ b/patchwork/views/utils.py
@@ -183,8 +183,8 @@  def series_to_mbox(series):
     """
     mbox = []
 
-    for dep in series.patches.all().order_by('number'):
-        mbox.append(patch_to_mbox(dep.patch))
+    for patch in series.patches.all().order_by('number'):
+        mbox.append(patch_to_mbox(patch))
 
     return '\n'.join(mbox)