[PATCH v2 2/3] models: Merge patch and first comment

[Finucane, Stephen]

On 15 Mar 16:01, Andy Doan wrote:
> On 03/13/2016 03:50 PM, Stephen Finucane wrote:
> >At the moment a patch is split into two model entries: a Patch and a
> >linked Comment. The original rationale for this was that a Patch is
> >really a sub-class of Comment. A comment is a record of the text
> >content of an incoming mail, while a patch is that, plus the patch
> >content too. Hence the separation and a one-to-one relationship when a
> >patch is present. However, this decision was made before Django added
> >support for model inheritance and is no longer necessary. This change
> >flatten the models in preparation for some email subclassing work. This
> >is achieved by copying over the non-duplicated fields from the Comment
> >to the linked Patch, then deleting the Comment.
> >
> >The migrations are broken into two steps: a schema migration and a data
> >migration, per the recommendations of the Django documentation [1]. SQL
> >migration scripts, where necessary, will need to be created manually as
> >there appears to be no way to do this in a way that is
> >RDBMS-independant [2][3].
> >
> >[1] https://docs.djangoproject.com/en/1.9/topics/migrations/#data-migrations
> >[2] https://stackoverflow.com/q/6856849/
> >[3] https://stackoverflow.com/q/224732/
> >
> >Signed-off-by: Stephen Finucane <stephen.finucane at intel.com>
> 
> Reviewed-by: Andy Doan <andy.doan at linaro.org>

Merged, with below comments taken into account.

> Not sure where or if its worth mentioning, but this migration is
> taking me around 30 minutes on my instance. So announcing a
> maintenance window is probably advisable.

Good point. I'll address this in a follow up patch, along with changes
to the CHANGELOG.

> >diff --git a/patchwork/migrations/0007_move_comment_content_to_patch_content.py b/patchwork/migrations/0007_move_comment_content_to_patch_content.py
> 
> >+def uncopy_comment_field(apps, schema_editor):
> >+    Comment = apps.get_model('patchwork', 'Comment')
> 
> Comment is an unused variable.

Done.

> >+def remove_duplicate_comments(apps, schema_editor):
> >+    Comment = apps.get_model('patchwork', 'Comment')
> >+    Patch = apps.get_model('patchwork', 'Patch')
> >+
> >+    for patch in Patch.objects.all():
> >+        comment = Comment.objects.get(patch=patch, msgid=patch.msgid)
> >+        comment.delete()
> 
> You need a try/except block here, eg:
> 
>         try:
>             # when available, this can only return one entry due to the
>             # unique_together constraint
>             Comment.objects.get(patch=patch, msgid=patch.msgid).delete()
>         except Comment.DoesNotExist:
>             # though there's no requirement to actually have a comment
>             continue

Done.

> I see no need for a v3 once you've fixed this.

Also added the 'diff' field to the 'defer' in 'view/__init__.py', which
should boost performance slightly.

Cheers,
Stephen