From 0b65abcdf54eceb0abbdf8e132ea52207c72b6e7 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Fri, 3 Jul 2020 13:03:33 +0000 Subject: [PATCH] pointer: Remove pointer from UserProfile. Most of the changes here are just that we no longer need to provide a value for pointer when we create UserProfile objects. --- corporate/tests/test_stripe.py | 12 +++++----- zerver/data_import/gitter.py | 1 - zerver/data_import/import_util.py | 2 -- zerver/data_import/slack.py | 1 - zerver/lib/create_user.py | 2 +- zerver/lib/import_realm.py | 24 ------------------- .../0294_remove_userprofile_pointer.py | 17 +++++++++++++ zerver/models.py | 3 --- zerver/tests/test_messages.py | 1 - 9 files changed, 24 insertions(+), 39 deletions(-) create mode 100644 zerver/migrations/0294_remove_userprofile_pointer.py diff --git a/corporate/tests/test_stripe.py b/corporate/tests/test_stripe.py index 9aec5de310..97cd594083 100644 --- a/corporate/tests/test_stripe.py +++ b/corporate/tests/test_stripe.py @@ -1154,8 +1154,8 @@ class StripeTest(StripeTestCase): def test_get_latest_seat_count(self) -> None: realm = get_realm("zulip") initial_count = get_latest_seat_count(realm) - user1 = UserProfile.objects.create(realm=realm, email='user1@zulip.com', pointer=-1) - user2 = UserProfile.objects.create(realm=realm, email='user2@zulip.com', pointer=-1) + user1 = UserProfile.objects.create(realm=realm, email='user1@zulip.com') + user2 = UserProfile.objects.create(realm=realm, email='user2@zulip.com') self.assertEqual(get_latest_seat_count(realm), initial_count + 2) # Test that bots aren't counted @@ -1169,17 +1169,17 @@ class StripeTest(StripeTestCase): # Test guests # Adding a guest to a realm with a lot of members shouldn't change anything - UserProfile.objects.create(realm=realm, email='user3@zulip.com', pointer=-1, role=UserProfile.ROLE_GUEST) + UserProfile.objects.create(realm=realm, email='user3@zulip.com', role=UserProfile.ROLE_GUEST) self.assertEqual(get_latest_seat_count(realm), initial_count) # Test 1 member and 5 guests realm = Realm.objects.create(string_id='second', name='second') - UserProfile.objects.create(realm=realm, email='member@second.com', pointer=-1) + UserProfile.objects.create(realm=realm, email='member@second.com') for i in range(5): UserProfile.objects.create(realm=realm, email=f'guest{i}@second.com', - pointer=-1, role=UserProfile.ROLE_GUEST) + role=UserProfile.ROLE_GUEST) self.assertEqual(get_latest_seat_count(realm), 1) # Test 1 member and 6 guests - UserProfile.objects.create(realm=realm, email='guest5@second.com', pointer=-1, + UserProfile.objects.create(realm=realm, email='guest5@second.com', role=UserProfile.ROLE_GUEST) self.assertEqual(get_latest_seat_count(realm), 2) diff --git a/zerver/data_import/gitter.py b/zerver/data_import/gitter.py index 60acfbec7d..c407985cf9 100644 --- a/zerver/data_import/gitter.py +++ b/zerver/data_import/gitter.py @@ -94,7 +94,6 @@ def build_userprofile(timestamp: Any, domain_name: str, email=email, delivery_email=email, avatar_source='U', - pointer=-1, date_joined=timestamp, last_login=timestamp) userprofile_dict = model_to_dict(userprofile) diff --git a/zerver/data_import/import_util.py b/zerver/data_import/import_util.py index ce368601bf..1352e48565 100644 --- a/zerver/data_import/import_util.py +++ b/zerver/data_import/import_util.py @@ -88,7 +88,6 @@ def build_user_profile(avatar_source: str, realm_id: int, short_name: str, timezone: Optional[str]) -> ZerverFieldsT: - pointer = -1 obj = UserProfile( avatar_source=avatar_source, date_joined=date_joined, @@ -99,7 +98,6 @@ def build_user_profile(avatar_source: str, is_mirror_dummy=is_mirror_dummy, is_active=is_active, role=role, - pointer=pointer, realm_id=realm_id, short_name=short_name, timezone=timezone, diff --git a/zerver/data_import/slack.py b/zerver/data_import/slack.py index 26190b5095..5cf2186f73 100755 --- a/zerver/data_import/slack.py +++ b/zerver/data_import/slack.py @@ -214,7 +214,6 @@ def users_to_zerver_userprofile(slack_data_dir: str, users: List[ZerverFieldsT], delivery_email=email, avatar_source='U', is_bot=user.get('is_bot', False), - pointer=-1, role=role, bot_type=1 if user.get('is_bot', False) else None, date_joined=timestamp, diff --git a/zerver/lib/create_user.py b/zerver/lib/create_user.py index 618b8e4362..457da0552d 100644 --- a/zerver/lib/create_user.py +++ b/zerver/lib/create_user.py @@ -79,7 +79,7 @@ def create_user_profile(realm: Realm, email: str, password: Optional[str], user_profile = UserProfile(is_staff=False, is_active=active, full_name=full_name, short_name=short_name, last_login=now, date_joined=now, realm=realm, - pointer=-1, is_bot=bool(bot_type), bot_type=bot_type, + is_bot=bool(bot_type), bot_type=bot_type, bot_owner=bot_owner, is_mirror_dummy=is_mirror_dummy, tos_version=tos_version, timezone=timezone, tutorial_status=tutorial_status, diff --git a/zerver/lib/import_realm.py b/zerver/lib/import_realm.py index 333a4626dc..c7c1768862 100644 --- a/zerver/lib/import_realm.py +++ b/zerver/lib/import_realm.py @@ -1045,30 +1045,6 @@ def do_import_realm(import_dir: Path, subdomain: str, processes: int=1) -> Realm update_model_ids(Reaction, data, 'reaction') bulk_import_model(data, Reaction) - for user_profile in UserProfile.objects.filter(is_bot=False, realm=realm): - # Since we now unconditionally renumbers message IDs, we need - # to reset the user's pointer to what will be a valid value. - # - # For zulip->zulip imports, we could do something clever, but - # it should always be safe to reset to first unread message. - # - # Longer-term, the plan is to eliminate pointer as a concept. - first_unread_message = UserMessage.objects.filter(user_profile=user_profile).extra( - where=[UserMessage.where_unread()], - ).order_by("message_id").first() - if first_unread_message is not None: - user_profile.pointer = first_unread_message.message_id - else: - last_message = UserMessage.objects.filter( - user_profile=user_profile).order_by("message_id").last() - if last_message is not None: - user_profile.pointer = last_message.message_id - else: - # -1 is the guard value for new user accounts with no messages. - user_profile.pointer = -1 - - user_profile.save(update_fields=["pointer"]) - # Similarly, we need to recalculate the first_message_id for stream objects. for stream in Stream.objects.filter(realm=realm): recipient = Recipient.objects.get(type=Recipient.STREAM, type_id=stream.id) diff --git a/zerver/migrations/0294_remove_userprofile_pointer.py b/zerver/migrations/0294_remove_userprofile_pointer.py new file mode 100644 index 0000000000..623444e6ef --- /dev/null +++ b/zerver/migrations/0294_remove_userprofile_pointer.py @@ -0,0 +1,17 @@ +# Generated by Django 2.2.13 on 2020-07-03 12:53 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('zerver', '0293_update_invite_as_dict_values'), + ] + + operations = [ + migrations.RemoveField( + model_name='userprofile', + name='pointer', + ), + ] diff --git a/zerver/models.py b/zerver/models.py index b6f0fc8927..e414f13c4e 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -900,9 +900,6 @@ class UserProfile(AbstractBaseUser, PermissionsMixin): tos_version: Optional[str] = models.CharField(null=True, max_length=10) api_key: str = models.CharField(max_length=API_KEY_LENGTH) - # pointer points to Message.id, NOT UserMessage.id. - pointer: int = models.IntegerField() - # Whether the user has access to server-level administrator pages, like /activity is_staff: bool = models.BooleanField(default=False) diff --git a/zerver/tests/test_messages.py b/zerver/tests/test_messages.py index 73ad25c595..6790f3e0a8 100644 --- a/zerver/tests/test_messages.py +++ b/zerver/tests/test_messages.py @@ -971,7 +971,6 @@ class StreamMessagesTest(ZulipTestCase): user = UserProfile.objects.create( realm=realm, email=email, - pointer=0, long_term_idle=long_term_idle, ) Subscription.objects.create(