diff --git a/tools/semgrep-py.yml b/tools/semgrep-py.yml index 851e8a2f9c..a83ac442f8 100644 --- a/tools/semgrep-py.yml +++ b/tools/semgrep-py.yml @@ -69,6 +69,7 @@ rules: - zerver/migrations/0260_missed_message_addresses_from_redis_to_db.py - zerver/migrations/0387_reupload_realmemoji_again.py - zerver/migrations/0443_userpresence_new_table_schema.py + - zerver/migrations/0493_rename_userhotspot_to_onboardingstep.py - pgroonga/migrations/0002_html_escape_subject.py - id: html-format diff --git a/zerver/actions/hotspots.py b/zerver/actions/hotspots.py index 28fbb4214d..35daff66c5 100644 --- a/zerver/actions/hotspots.py +++ b/zerver/actions/hotspots.py @@ -1,9 +1,9 @@ from zerver.lib.hotspots import get_next_hotspots -from zerver.models import UserHotspot, UserProfile +from zerver.models import OnboardingStep, UserProfile from zerver.tornado.django_api import send_event def do_mark_hotspot_as_read(user: UserProfile, hotspot: str) -> None: - UserHotspot.objects.get_or_create(user=user, hotspot=hotspot) + OnboardingStep.objects.get_or_create(user=user, onboarding_step=hotspot) event = dict(type="hotspots", hotspots=get_next_hotspots(user)) send_event(user.realm, event, [user.id]) diff --git a/zerver/lib/export.py b/zerver/lib/export.py index c9732b8396..e25edbb1e5 100644 --- a/zerver/lib/export.py +++ b/zerver/lib/export.py @@ -45,6 +45,7 @@ from zerver.models import ( Huddle, Message, MutedUser, + OnboardingStep, Reaction, Realm, RealmAuditLog, @@ -63,7 +64,6 @@ from zerver.models import ( UserActivityInterval, UserGroup, UserGroupMembership, - UserHotspot, UserMessage, UserPresence, UserProfile, @@ -138,6 +138,7 @@ ALL_ZULIP_TABLES = { "zerver_missedmessageemailaddress", "zerver_multiuseinvite", "zerver_multiuseinvite_streams", + "zerver_onboardingstep", "zerver_preregistrationrealm", "zerver_preregistrationuser", "zerver_preregistrationuser_streams", @@ -165,7 +166,6 @@ ALL_ZULIP_TABLES = { "zerver_useractivityinterval", "zerver_usergroup", "zerver_usergroupmembership", - "zerver_userhotspot", "zerver_usermessage", "zerver_userpresence", "zerver_userprofile", @@ -296,7 +296,7 @@ DATE_FIELDS: Dict[TableName, List[Field]] = { "zerver_stream": ["date_created"], "zerver_useractivityinterval": ["start", "end"], "zerver_useractivity": ["last_visit"], - "zerver_userhotspot": ["timestamp"], + "zerver_onboardingstep": ["timestamp"], "zerver_userpresence": ["last_active_time", "last_connected_time"], "zerver_userprofile": ["date_joined", "last_login", "last_reminder"], "zerver_userprofile_mirrordummy": ["date_joined", "last_login", "last_reminder"], @@ -939,6 +939,13 @@ def add_user_profile_child_configs(user_profile_config: Config) -> None: include_rows="user_profile_id__in", ) + Config( + table="zerver_onboardingstep", + model=OnboardingStep, + normal_parent=user_profile_config, + include_rows="user_id__in", + ) + Config( table="zerver_useractivity", model=UserActivity, @@ -953,13 +960,6 @@ def add_user_profile_child_configs(user_profile_config: Config) -> None: include_rows="user_profile_id__in", ) - Config( - table="zerver_userhotspot", - model=UserHotspot, - normal_parent=user_profile_config, - include_rows="user_id__in", - ) - Config( table="zerver_userpresence", model=UserPresence, diff --git a/zerver/lib/hotspots.py b/zerver/lib/hotspots.py index 7e324f35c6..6b104fba44 100644 --- a/zerver/lib/hotspots.py +++ b/zerver/lib/hotspots.py @@ -7,7 +7,7 @@ from django.conf import settings from django.utils.translation import gettext_lazy from django_stubs_ext import StrPromise -from zerver.models import UserHotspot, UserProfile +from zerver.models import OnboardingStep, UserProfile @dataclass @@ -87,7 +87,7 @@ def get_next_hotspots(user: UserProfile) -> List[Dict[str, Union[str, float, boo return [] seen_hotspots = frozenset( - UserHotspot.objects.filter(user=user).values_list("hotspot", flat=True) + OnboardingStep.objects.filter(user=user).values_list("onboarding_step", flat=True) ) hotspots = [hotspot.to_dict() for hotspot in NON_INTRO_HOTSPOTS] @@ -108,9 +108,11 @@ def get_next_hotspots(user: UserProfile) -> List[Dict[str, Union[str, float, boo def copy_hotspots(source_profile: UserProfile, target_profile: UserProfile) -> None: - for userhotspot in frozenset(UserHotspot.objects.filter(user=source_profile)): - UserHotspot.objects.create( - user=target_profile, hotspot=userhotspot.hotspot, timestamp=userhotspot.timestamp + for userhotspot in frozenset(OnboardingStep.objects.filter(user=source_profile)): + OnboardingStep.objects.create( + user=target_profile, + onboarding_step=userhotspot.onboarding_step, + timestamp=userhotspot.timestamp, ) target_profile.tutorial_status = source_profile.tutorial_status diff --git a/zerver/lib/import_realm.py b/zerver/lib/import_realm.py index d2898de736..d4158493df 100644 --- a/zerver/lib/import_realm.py +++ b/zerver/lib/import_realm.py @@ -51,6 +51,7 @@ from zerver.models import ( Huddle, Message, MutedUser, + OnboardingStep, Reaction, Realm, RealmAuditLog, @@ -70,7 +71,6 @@ from zerver.models import ( UserActivityInterval, UserGroup, UserGroupMembership, - UserHotspot, UserMessage, UserPresence, UserProfile, @@ -111,6 +111,7 @@ ID_MAP: Dict[str, Dict[int, int]] = { "recipient": {}, "subscription": {}, "defaultstream": {}, + "onboardingstep": {}, "reaction": {}, "realmauthenticationmethod": {}, "realmemoji": {}, @@ -128,7 +129,6 @@ ID_MAP: Dict[str, Dict[int, int]] = { "attachment": {}, "realmauditlog": {}, "recipient_to_huddle_map": {}, - "userhotspot": {}, "usertopic": {}, "muteduser": {}, "service": {}, @@ -1189,11 +1189,11 @@ def do_import_realm(import_dir: Path, subdomain: str, processes: int = 1) -> Rea update_model_ids(AlertWord, data, "alertword") bulk_import_model(data, AlertWord) - if "zerver_userhotspot" in data: - fix_datetime_fields(data, "zerver_userhotspot") - re_map_foreign_keys(data, "zerver_userhotspot", "user", related_table="user_profile") - update_model_ids(UserHotspot, data, "userhotspot") - bulk_import_model(data, UserHotspot) + if "zerver_onboardingstep" in data: + fix_datetime_fields(data, "zerver_onboardingstep") + re_map_foreign_keys(data, "zerver_onboardingstep", "user", related_table="user_profile") + update_model_ids(OnboardingStep, data, "onboardingstep") + bulk_import_model(data, OnboardingStep) if "zerver_usertopic" in data: fix_datetime_fields(data, "zerver_usertopic") diff --git a/zerver/lib/test_helpers.py b/zerver/lib/test_helpers.py index 1847fd9f93..e31576932d 100644 --- a/zerver/lib/test_helpers.py +++ b/zerver/lib/test_helpers.py @@ -607,6 +607,7 @@ def use_db_models( Huddle = apps.get_model("zerver", "Huddle") Message = apps.get_model("zerver", "Message") MultiuseInvite = apps.get_model("zerver", "MultiuseInvite") + OnboardingStep = apps.get_model("zerver", "OnboardingStep") PreregistrationUser = apps.get_model("zerver", "PreregistrationUser") PushDeviceToken = apps.get_model("zerver", "PushDeviceToken") Reaction = apps.get_model("zerver", "Reaction") @@ -628,7 +629,6 @@ def use_db_models( UserActivityInterval = apps.get_model("zerver", "UserActivityInterval") UserGroup = apps.get_model("zerver", "UserGroup") UserGroupMembership = apps.get_model("zerver", "UserGroupMembership") - UserHotspot = apps.get_model("zerver", "UserHotspot") UserMessage = apps.get_model("zerver", "UserMessage") UserPresence = apps.get_model("zerver", "UserPresence") UserProfile = apps.get_model("zerver", "UserProfile") @@ -652,6 +652,7 @@ def use_db_models( Message=Message, MultiuseInvite=MultiuseInvite, UserTopic=UserTopic, + OnboardingStep=OnboardingStep, PreregistrationUser=PreregistrationUser, PushDeviceToken=PushDeviceToken, Reaction=Reaction, @@ -670,7 +671,6 @@ def use_db_models( UserActivityInterval=UserActivityInterval, UserGroup=UserGroup, UserGroupMembership=UserGroupMembership, - UserHotspot=UserHotspot, UserMessage=UserMessage, UserPresence=UserPresence, UserProfile=UserProfile, diff --git a/zerver/migrations/0493_rename_userhotspot_to_onboardingstep.py b/zerver/migrations/0493_rename_userhotspot_to_onboardingstep.py new file mode 100644 index 0000000000..1d5d1b7a2a --- /dev/null +++ b/zerver/migrations/0493_rename_userhotspot_to_onboardingstep.py @@ -0,0 +1,37 @@ +# Generated by Django 4.2.7 on 2023-12-01 06:55 + +from django.db import migrations + +from zerver.lib.migrate import rename_indexes_constraints + + +class Migration(migrations.Migration): + dependencies = [ + ("zerver", "0492_realm_push_notifications_enabled_and_more"), + ] + + operations = [ + migrations.SeparateDatabaseAndState( + database_operations=[ + migrations.RenameField( + model_name="UserHotspot", old_name="hotspot", new_name="onboarding_step" + ), + migrations.RunPython( + rename_indexes_constraints( + "zerver_userhotspot", + "zerver_onboardingstep", + ), + reverse_code=rename_indexes_constraints( + "zerver_onboardingstep", + "zerver_userhotspot", + ), + ), + ], + state_operations=[ + migrations.RenameField( + model_name="UserHotspot", old_name="hotspot", new_name="onboarding_step" + ), + migrations.RenameModel(old_name="UserHotspot", new_name="OnboardingStep"), + ], + ), + ] diff --git a/zerver/models.py b/zerver/models.py index 9b3126bdc4..48cd1b95da 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -2086,7 +2086,7 @@ class UserProfile(AbstractBaseUser, PermissionsMixin, UserBaseSettings): # type avatar_hash = models.CharField(null=True, max_length=64) # TODO: TUTORIAL_STATUS was originally an optimization designed to - # allow us to skip querying the UserHotspot table when loading + # allow us to skip querying the OnboardingStep table when loading # /. This optimization is no longer effective, so it's possible we # should delete it. TUTORIAL_WAITING = "W" @@ -4951,13 +4951,13 @@ class RealmAuditLog(AbstractRealmAuditLog): ] -class UserHotspot(models.Model): +class OnboardingStep(models.Model): user = models.ForeignKey(UserProfile, on_delete=CASCADE) - hotspot = models.CharField(max_length=30) + onboarding_step = models.CharField(max_length=30) timestamp = models.DateTimeField(default=timezone_now) class Meta: - unique_together = ("user", "hotspot") + unique_together = ("user", "onboarding_step") def check_valid_user_ids(realm_id: int, val: object, allow_deactivated: bool = False) -> List[int]: diff --git a/zerver/tests/test_hotspots.py b/zerver/tests/test_hotspots.py index 05fd58bead..3c598da799 100644 --- a/zerver/tests/test_hotspots.py +++ b/zerver/tests/test_hotspots.py @@ -4,7 +4,7 @@ from zerver.actions.create_user import do_create_user from zerver.actions.hotspots import do_mark_hotspot_as_read from zerver.lib.hotspots import ALL_HOTSPOTS, INTRO_HOTSPOTS, NON_INTRO_HOTSPOTS, get_next_hotspots from zerver.lib.test_classes import ZulipTestCase -from zerver.models import UserHotspot, UserProfile, get_realm +from zerver.models import OnboardingStep, UserProfile, get_realm # Splitting this out, since I imagine this will eventually have most of the @@ -56,7 +56,9 @@ class TestHotspots(ZulipTestCase): user = self.example_user("hamlet") do_mark_hotspot_as_read(user, "intro_compose") self.assertEqual( - list(UserHotspot.objects.filter(user=user).values_list("hotspot", flat=True)), + list( + OnboardingStep.objects.filter(user=user).values_list("onboarding_step", flat=True) + ), ["intro_compose"], ) @@ -66,13 +68,17 @@ class TestHotspots(ZulipTestCase): result = self.client_post("/json/users/me/hotspots", {"hotspot": "intro_streams"}) self.assert_json_success(result) self.assertEqual( - list(UserHotspot.objects.filter(user=user).values_list("hotspot", flat=True)), + list( + OnboardingStep.objects.filter(user=user).values_list("onboarding_step", flat=True) + ), ["intro_streams"], ) result = self.client_post("/json/users/me/hotspots", {"hotspot": "invalid"}) self.assert_json_error(result, "Unknown hotspot: invalid") self.assertEqual( - list(UserHotspot.objects.filter(user=user).values_list("hotspot", flat=True)), + list( + OnboardingStep.objects.filter(user=user).values_list("onboarding_step", flat=True) + ), ["intro_streams"], ) diff --git a/zerver/tests/test_import_export.py b/zerver/tests/test_import_export.py index 2d769a80a3..cd57c09d2c 100644 --- a/zerver/tests/test_import_export.py +++ b/zerver/tests/test_import_export.py @@ -66,6 +66,7 @@ from zerver.models import ( Huddle, Message, MutedUser, + OnboardingStep, Reaction, Realm, RealmAuditLog, @@ -78,7 +79,6 @@ from zerver.models import ( SystemGroups, UserGroup, UserGroupMembership, - UserHotspot, UserMessage, UserPresence, UserProfile, @@ -798,9 +798,9 @@ class RealmImportExportTest(ExportFile): self.assertEqual(reaction.emoji_code, str(realm_emoji.id)) # data to test import of hotspots - UserHotspot.objects.create( + OnboardingStep.objects.create( user=sample_user, - hotspot="intro_streams", + onboarding_step="intro_streams", ) # data to test import of muted topic @@ -1187,8 +1187,8 @@ class RealmImportExportTest(ExportFile): @getter def get_user_hotspots(r: Realm) -> Set[str]: user_id = get_user_id(r, "King Hamlet") - hotspots = UserHotspot.objects.filter(user_id=user_id) - user_hotspots = {hotspot.hotspot for hotspot in hotspots} + hotspots = OnboardingStep.objects.filter(user_id=user_id) + user_hotspots = {hotspot.onboarding_step for hotspot in hotspots} return user_hotspots # test muted topics @@ -1939,12 +1939,12 @@ class SingleUserExportTest(ExportFile): (rec,) = records self.assertEqual(rec["value"], 42) - UserHotspot.objects.create(user=cordelia, hotspot="topics") - UserHotspot.objects.create(user=othello, hotspot="bogus") + OnboardingStep.objects.create(user=cordelia, onboarding_step="topics") + OnboardingStep.objects.create(user=othello, onboarding_step="bogus") @checker - def zerver_userhotspot(records: List[Record]) -> None: - self.assertEqual(records[-1]["hotspot"], "topics") + def zerver_onboardingstep(records: List[Record]) -> None: + self.assertEqual(records[-1]["onboarding_step"], "topics") """ The zerver_realmauditlog checker basically assumes that diff --git a/zerver/tests/test_migrations.py b/zerver/tests/test_migrations.py index 8f02025900..c7fa5c9f71 100644 --- a/zerver/tests/test_migrations.py +++ b/zerver/tests/test_migrations.py @@ -24,9 +24,9 @@ from zerver.lib.test_classes import MigrationsTestCase # "zerver_subscription" because it has pending trigger events -class UserMessageIndex(MigrationsTestCase): - migrate_from = "0485_alter_usermessage_flags_and_add_index" - migrate_to = "0486_clear_old_data_for_unused_usermessage_flags" +class RenameUserHotspot(MigrationsTestCase): + migrate_from = "0492_realm_push_notifications_enabled_and_more" + migrate_to = "0493_rename_userhotspot_to_onboardingstep" @override def setUp(self) -> None: @@ -35,42 +35,21 @@ class UserMessageIndex(MigrationsTestCase): @override def setUpBeforeMigration(self, apps: StateApps) -> None: - UserMessage = apps.get_model("zerver", "usermessage") + self.assertRaises(LookupError, lambda: apps.get_model("zerver", "onboardingstep")) - um_1 = UserMessage.objects.get(id=1) - um_1.flags.topic_wildcard_mentioned = True - um_1.flags.stream_wildcard_mentioned = True - um_1.flags.force_expand = True - um_1.save() + UserHotspot = apps.get_model("zerver", "userhotspot") - um_2 = UserMessage.objects.get(id=2) - um_2.flags.group_mentioned = True - um_2.flags.topic_wildcard_mentioned = True - um_2.flags.mentioned = True - um_2.flags.force_collapse = True - um_2.save() + expected_field_names = {"id", "hotspot", "timestamp", "user"} + fields_name = {field.name for field in UserHotspot._meta.get_fields()} - um_1 = UserMessage.objects.get(id=1) - um_2 = UserMessage.objects.get(id=2) + self.assertEqual(fields_name, expected_field_names) - self.assertTrue(um_1.flags.topic_wildcard_mentioned) - self.assertTrue(um_1.flags.stream_wildcard_mentioned) - self.assertTrue(um_1.flags.force_expand) - self.assertTrue(um_2.flags.group_mentioned) - self.assertTrue(um_2.flags.topic_wildcard_mentioned) - self.assertTrue(um_2.flags.mentioned) - self.assertTrue(um_2.flags.force_collapse) + def test_renamed_model_and_field(self) -> None: + self.assertRaises(LookupError, lambda: self.apps.get_model("zerver", "userhotspot")) - def test_clear_topic_wildcard_and_group_mentioned_flags(self) -> None: - UserMessage = self.apps.get_model("zerver", "usermessage") + OnboardingStep = self.apps.get_model("zerver", "onboardingstep") - um_1 = UserMessage.objects.get(id=1) - um_2 = UserMessage.objects.get(id=2) + expected_field_names = {"id", "onboarding_step", "timestamp", "user"} + fields_name = {field.name for field in OnboardingStep._meta.get_fields()} - self.assertFalse(um_1.flags.topic_wildcard_mentioned) - self.assertTrue(um_1.flags.stream_wildcard_mentioned) - self.assertFalse(um_1.flags.force_expand) - self.assertFalse(um_2.flags.group_mentioned) - self.assertFalse(um_2.flags.topic_wildcard_mentioned) - self.assertTrue(um_2.flags.mentioned) - self.assertFalse(um_2.flags.force_collapse) + self.assertEqual(fields_name, expected_field_names) diff --git a/zerver/tests/test_users.py b/zerver/tests/test_users.py index c74c58afbf..95e7643dd2 100644 --- a/zerver/tests/test_users.py +++ b/zerver/tests/test_users.py @@ -60,6 +60,7 @@ from zerver.models import ( CustomProfileField, InvalidFakeEmailDomainError, Message, + OnboardingStep, PreregistrationUser, RealmAuditLog, RealmDomain, @@ -70,7 +71,6 @@ from zerver.models import ( Subscription, SystemGroups, UserGroupMembership, - UserHotspot, UserProfile, UserTopic, check_valid_user_ids, @@ -1270,11 +1270,11 @@ class UserProfileTest(ZulipTestCase): with get_test_image_file("img.png") as image_file: upload_avatar_image(image_file, cordelia, cordelia) - UserHotspot.objects.filter(user=cordelia).delete() - UserHotspot.objects.filter(user=iago).delete() + OnboardingStep.objects.filter(user=cordelia).delete() + OnboardingStep.objects.filter(user=iago).delete() hotspots_completed = {"intro_streams", "intro_topics"} for hotspot in hotspots_completed: - UserHotspot.objects.create(user=cordelia, hotspot=hotspot) + OnboardingStep.objects.create(user=cordelia, onboarding_step=hotspot) # Check that we didn't send an realm_user update events to # users; this work is happening before the user account is @@ -1316,7 +1316,9 @@ class UserProfileTest(ZulipTestCase): self.assertEqual(cordelia.enter_sends, False) self.assertEqual(hamlet.enter_sends, True) - hotspots = set(UserHotspot.objects.filter(user=iago).values_list("hotspot", flat=True)) + hotspots = set( + OnboardingStep.objects.filter(user=iago).values_list("onboarding_step", flat=True) + ) self.assertEqual(hotspots, hotspots_completed) def test_copy_default_settings_from_realm_user_default(self) -> None: