From c7d0192755befccfa9e9fbbb8557825008f7de21 Mon Sep 17 00:00:00 2001 From: arpit551 Date: Fri, 19 Jun 2020 15:37:18 +0530 Subject: [PATCH] reaction: Fix missing unique constraint on Reactions model. This fixes a missing unique constraint on the Reactions data model state when using multiple aliases for an emoji code. As with any missing unique constraints, we first need to apply a migration that eliminates violations of the rule; in this case, deleting the duplicates is correct. Added unique constraint for "user_profile", "message", "reaction_type", "emoji_code". Fixes #15347. --- .../0287_clear_duplicate_reactions.py | 35 +++++++++++++++++++ .../0288_reaction_unique_on_emoji_code.py | 21 +++++++++++ zerver/models.py | 3 +- 3 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 zerver/migrations/0287_clear_duplicate_reactions.py create mode 100644 zerver/migrations/0288_reaction_unique_on_emoji_code.py diff --git a/zerver/migrations/0287_clear_duplicate_reactions.py b/zerver/migrations/0287_clear_duplicate_reactions.py new file mode 100644 index 0000000000..d15371b71f --- /dev/null +++ b/zerver/migrations/0287_clear_duplicate_reactions.py @@ -0,0 +1,35 @@ +from django.db import migrations +from django.db.backends.postgresql.schema import DatabaseSchemaEditor +from django.db.migrations.state import StateApps +from django.db.models import Count + + +def clear_duplicate_reactions(apps: StateApps, schema_editor: DatabaseSchemaEditor) -> None: + """Zulip's data model for reactions has enforced via code, + nontransactionally, that they can only react with one emoji_code + for a given reaction_type. This fixes any that were stored in the + database via a race; the next migration will add the appropriate + database-level unique constraint. + """ + Reaction = apps.get_model('zerver', 'Reaction') + + duplicate_reactions = Reaction.objects.all().values( + "user_profile_id", "message_id", "reaction_type", "emoji_code").annotate( + Count('id')).filter(id__count__gt=1) + for duplicate_reaction in duplicate_reactions: + duplicate_reaction.pop('id__count') + to_cleanup = Reaction.objects.filter(**duplicate_reaction)[1:] + for reaction in to_cleanup: + reaction.delete() + +class Migration(migrations.Migration): + + dependencies = [ + ('zerver', '0286_merge_0260_0285'), + ] + + operations = [ + migrations.RunPython(clear_duplicate_reactions, + reverse_code=migrations.RunPython.noop, + elidable=True), + ] diff --git a/zerver/migrations/0288_reaction_unique_on_emoji_code.py b/zerver/migrations/0288_reaction_unique_on_emoji_code.py new file mode 100644 index 0000000000..d47b92f1f5 --- /dev/null +++ b/zerver/migrations/0288_reaction_unique_on_emoji_code.py @@ -0,0 +1,21 @@ +# Generated by Django 2.2.13 on 2020-06-19 08:16 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('zerver', '0287_clear_duplicate_reactions'), + ] + + operations = [ + migrations.AlterUniqueTogether( + name='archivedreaction', + unique_together={('user_profile', 'message', 'emoji_name'), ('user_profile', 'message', 'reaction_type', 'emoji_code')}, + ), + migrations.AlterUniqueTogether( + name='reaction', + unique_together={('user_profile', 'message', 'emoji_name'), ('user_profile', 'message', 'reaction_type', 'emoji_code')}, + ), + ] diff --git a/zerver/models.py b/zerver/models.py index a944aa0d4c..f7204660bc 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -1893,7 +1893,8 @@ class AbstractReaction(models.Model): class Meta: abstract = True - unique_together = ("user_profile", "message", "emoji_name") + unique_together = (("user_profile", "message", "emoji_name"), + ("user_profile", "message", "reaction_type", "emoji_code")) class Reaction(AbstractReaction): message: Message = models.ForeignKey(Message, on_delete=CASCADE)