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

[Andy Doan]

On 03/08/2016 03:54 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>

> diff --git a/patchwork/migrations/0007_move_comment_content_to_patch_content.py b/patchwork/migrations/0007_move_comment_content_to_patch_content.py

> +from django.db import migrations, models

Hate to nitpick over migration warnings since they are usually machine 
generated, but "models" is an unused import for this module.

> +def copy_comment_field(apps, schema_editor):
> +    Comment = apps.get_model('patchwork', 'Comment')
> +    Patch = apps.get_model('patchwork', 'Patch')
> +
> +    for patch in Patch.objects.all():
> +        # this can only return one entry due to the unique_together
> +        # constraint
> +        comment = Comment.objects.get(patch=patch, msgid=patch.msgid)
> +        patch.content = comment.content
> +        patch.save()

This should work based on the normal flow of patchwork. However, I've 
found a place where instance can't complete this migration. We've added 
some hacks to track AOSP contributions[1]. We don't actually create a 
comment linked to the patch, we just use the "content" field to point to 
the AOSP change. I *think* if you changed this to 
Comment.objects.filter(....) and if count() == 1 it wouldn't break my 
instance. However, I'm not sure that's fair to ask you to add baggage to 
deal with me when its probably possible for me to add an internal 
migration to "fix" our AOSP stuff. Thought it would hurt to ask though.

1: http://patches.linaro.org/patch/7669/

-andy