user_message: Rename unused flags and create an index.

This commit renames the two unused and historical bits of the
'fields' bitfield of the 'UserMessage' and 'ArchivedUserMessage'
tables.

* 'summarize_in_home' to 'topic_wildcard_mentioned'
* 'summarize_in_stream' to 'group_mentioned'

The 'group_mentioned' flag doesn't affect the feature,
but completing the work here helps to save future migration
and indexing efforts on the UserMessage table, as we plan to
use this flag in the future for group mentions.

The unused bits may have old data; we'll clear that in
a separate commit.

It creates the 'zerver_usermessage_any_mentioned_message_id'
index concurrently.
This commit is contained in:
Prakhar Pratyush
2023-10-19 13:47:09 +05:30
committed by Tim Abbott
parent 5a44a6624b
commit c37871ac3a
4 changed files with 106 additions and 79 deletions

View File

@@ -2277,7 +2277,7 @@ one or more new messages.
return body return body
class MigrationsTestCase(ZulipTestCase): # nocoverage class MigrationsTestCase(ZulipTransactionTestCase): # nocoverage
""" """
Test class for database migrations inspired by this blog post: Test class for database migrations inspired by this blog post:
https://www.caktusgroup.com/blog/2016/02/02/writing-unit-tests-django-migrations/ https://www.caktusgroup.com/blog/2016/02/02/writing-unit-tests-django-migrations/
@@ -2295,6 +2295,7 @@ class MigrationsTestCase(ZulipTestCase): # nocoverage
@override @override
def setUp(self) -> None: def setUp(self) -> None:
super().setUp()
assert ( assert (
self.migrate_from and self.migrate_to self.migrate_from and self.migrate_to
), f"TestCase '{type(self).__name__}' must define migrate_from and migrate_to properties" ), f"TestCase '{type(self).__name__}' must define migrate_from and migrate_to properties"

View File

@@ -0,0 +1,69 @@
# Generated by Django 4.2.6 on 2023-10-19 06:37
import bitfield.models
from django.contrib.postgres.operations import AddIndexConcurrently
from django.db import migrations, models
class Migration(migrations.Migration):
atomic = False
dependencies = [
("zerver", "0484_preregistrationrealm_default_language"),
]
operations = [
migrations.AlterField(
model_name="archivedusermessage",
name="flags",
field=bitfield.models.BitField(
[
"read",
"starred",
"collapsed",
"mentioned",
"wildcard_mentioned",
"topic_wildcard_mentioned",
"group_mentioned",
"force_expand",
"force_collapse",
"has_alert_word",
"historical",
"is_private",
"active_mobile_push_notification",
],
default=0,
),
),
migrations.AlterField(
model_name="usermessage",
name="flags",
field=bitfield.models.BitField(
[
"read",
"starred",
"collapsed",
"mentioned",
"wildcard_mentioned",
"topic_wildcard_mentioned",
"group_mentioned",
"force_expand",
"force_collapse",
"has_alert_word",
"historical",
"is_private",
"active_mobile_push_notification",
],
default=0,
),
),
AddIndexConcurrently(
model_name="usermessage",
index=models.Index(
"user_profile",
"message",
condition=models.Q(("flags__andnz", 120)),
name="zerver_usermessage_any_mentioned_message_id",
),
),
]

View File

@@ -3476,9 +3476,9 @@ class AbstractUserMessage(models.Model):
"collapsed", "collapsed",
"mentioned", "mentioned",
"wildcard_mentioned", "wildcard_mentioned",
# These next 4 flags are from features that have since been removed. "topic_wildcard_mentioned",
"summarize_in_home", "group_mentioned",
"summarize_in_stream", # These next 2 flags are from features that have since been removed.
"force_expand", "force_expand",
"force_collapse", "force_collapse",
# Whether the message contains any of the user's alert words. # Whether the message contains any of the user's alert words.
@@ -3506,12 +3506,12 @@ class AbstractUserMessage(models.Model):
"has_alert_word", "has_alert_word",
"mentioned", "mentioned",
"wildcard_mentioned", "wildcard_mentioned",
"topic_wildcard_mentioned",
"group_mentioned",
"historical", "historical",
# Unused flags can't be edited. # Unused flags can't be edited.
"force_expand", "force_expand",
"force_collapse", "force_collapse",
"summarize_in_home",
"summarize_in_stream",
} }
flags: BitHandler = BitField(flags=ALL_FLAGS, default=0) flags: BitHandler = BitField(flags=ALL_FLAGS, default=0)
@@ -3607,6 +3607,17 @@ class UserMessage(AbstractUserMessage):
| Q(flags__andnz=AbstractUserMessage.flags.wildcard_mentioned.mask), | Q(flags__andnz=AbstractUserMessage.flags.wildcard_mentioned.mask),
name="zerver_usermessage_wildcard_mentioned_message_id", name="zerver_usermessage_wildcard_mentioned_message_id",
), ),
models.Index(
"user_profile",
"message",
condition=Q(
flags__andnz=AbstractUserMessage.flags.mentioned.mask
| AbstractUserMessage.flags.wildcard_mentioned.mask
| AbstractUserMessage.flags.topic_wildcard_mentioned.mask
| AbstractUserMessage.flags.group_mentioned.mask
),
name="zerver_usermessage_any_mentioned_message_id",
),
models.Index( models.Index(
"user_profile", "user_profile",
"message", "message",

View File

@@ -4,10 +4,7 @@
# You can also read # You can also read
# https://www.caktusgroup.com/blog/2016/02/02/writing-unit-tests-django-migrations/ # https://www.caktusgroup.com/blog/2016/02/02/writing-unit-tests-django-migrations/
# to get a tutorial on the framework that inspired this feature. # to get a tutorial on the framework that inspired this feature.
from datetime import datetime, timezone from django.db import connections
from unittest import skip
import orjson
from django.db.migrations.state import StateApps from django.db.migrations.state import StateApps
from typing_extensions import override from typing_extensions import override
@@ -30,77 +27,26 @@ from zerver.lib.test_helpers import use_db_models
# been tested for a migration being merged. # been tested for a migration being merged.
@skip("Cannot be run because there is a non-atomic migration that has been merged after it") class UserMessageIndex(MigrationsTestCase):
class ScheduledEmailData(MigrationsTestCase): migrate_from = "0484_preregistrationrealm_default_language"
migrate_from = "0467_rename_extradata_realmauditlog_extra_data_json" migrate_to = "0485_alter_usermessage_flags_and_add_index"
migrate_to = "0468_rename_followup_day_email_templates"
def index_exists(self, index_name: str) -> bool:
table_name = "zerver_usermessage"
connection = connections["default"]
with connection.cursor() as cursor:
# We use parameterized query to prevent SQL injection vulnerabilities
query = "SELECT indexname FROM pg_indexes WHERE tablename = %s AND indexname = %s"
cursor.execute(query, [table_name, index_name])
return cursor.fetchone() is not None
@use_db_models @use_db_models
@override @override
def setUpBeforeMigration(self, apps: StateApps) -> None: def setUpBeforeMigration(self, apps: StateApps) -> None:
iago = self.example_user("iago") self.assertTrue(self.index_exists("zerver_usermessage_wildcard_mentioned_message_id"))
ScheduledEmail = apps.get_model("zerver", "ScheduledEmail") self.assertFalse(self.index_exists("zerver_usermessage_any_mentioned_message_id"))
send_date = datetime(2025, 1, 1, 1, 0, 0, 0, tzinfo=timezone.utc)
templates = [ def test_new_index_created_old_index_not_removed(self) -> None:
["zerver/emails/followup_day1", "a", True, 10], self.assertTrue(self.index_exists("zerver_usermessage_wildcard_mentioned_message_id"))
["zerver/emails/followup_day2", "b", False, 20], self.assertTrue(self.index_exists("zerver_usermessage_any_mentioned_message_id"))
["zerver/emails/onboarding_zulip_guide", "c", True, 30],
]
for template in templates:
email_fields = {
"template_prefix": template[0],
"string_context": template[1],
"boolean_context": template[2],
"integer_context": template[3],
}
email = ScheduledEmail.objects.create(
type=1,
realm=iago.realm,
scheduled_timestamp=send_date,
data=orjson.dumps(email_fields).decode(),
)
email.users.add(iago.id)
def test_updated_email_templates(self) -> None:
ScheduledEmail = self.apps.get_model("zerver", "ScheduledEmail")
send_date = datetime(2025, 1, 1, 1, 0, 0, 0, tzinfo=timezone.utc)
old_templates = [
"zerver/emails/followup_day1",
"zerver/emails/followup_day2",
]
current_templates = [
"zerver/emails/account_registered",
"zerver/emails/onboarding_zulip_guide",
"zerver/emails/onboarding_zulip_topics",
]
email_data = [
["zerver/emails/account_registered", "a", True, 10],
["zerver/emails/onboarding_zulip_topics", "b", False, 20],
["zerver/emails/onboarding_zulip_guide", "c", True, 30],
]
scheduled_emails = ScheduledEmail.objects.all()
self.assert_length(scheduled_emails, 3)
checked_emails = []
for email in scheduled_emails:
self.assertEqual(email.type, 1)
self.assertEqual(email.scheduled_timestamp, send_date)
updated_data = orjson.loads(email.data)
template_prefix = updated_data["template_prefix"]
self.assertFalse(template_prefix in old_templates)
for data in email_data:
if template_prefix == data[0]:
self.assertEqual(updated_data["string_context"], data[1])
self.assertEqual(updated_data["boolean_context"], data[2])
self.assertEqual(updated_data["integer_context"], data[3])
checked_emails.append(template_prefix)
self.assertEqual(current_templates, sorted(checked_emails))