From 47228f3a951c72505ce048ef4b66e6b096d85ba9 Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Sat, 2 May 2020 18:42:30 +0200 Subject: [PATCH] actions: Implement do_delete_user. To have a reasonable way of creating the dummy user without duplicating code, we need change create_user to have the optional force_id argument. --- zerver/lib/actions.py | 28 +++++++++++++++++ zerver/lib/create_user.py | 16 +++++++--- zerver/tests/test_users.py | 61 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 4 deletions(-) diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 8258e80031..b5ce2ba4ee 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -879,6 +879,34 @@ def do_scrub_realm(realm: Realm, acting_user: Optional[UserProfile]=None) -> Non acting_user=acting_user, event_type=RealmAuditLog.REALM_SCRUBBED) +def do_delete_user(user_profile: UserProfile) -> None: + if user_profile.realm.is_zephyr_mirror_realm: + raise AssertionError("Deleting zephyr mirror users is not supported") + + do_deactivate_user(user_profile) + + subscribed_huddle_recipient_ids = set( + Subscription.objects.filter(user_profile=user_profile, recipient__type=Recipient.HUDDLE) + .values_list('recipient_id', flat=True) + ) + user_id = user_profile.id + realm = user_profile.realm + personal_recipient = user_profile.recipient + + user_profile.delete() + # Recipient objects don't get deleted through CASCADE, so we need to handle + # the user's personal recipient manually. This will also delete all Messages pointing + # to this recipient (all private messages sent to the user). + personal_recipient.delete() + replacement_user = create_user(force_id=user_id, email=f"deleteduser{user_id}@{realm.uri}", + password=None, + realm=realm, + full_name=f"Deleted User {user_id}", + is_mirror_dummy=True) + subs_to_recreate = [Subscription(user_profile=replacement_user, recipient=recipient) + for recipient in Recipient.objects.filter(id__in=subscribed_huddle_recipient_ids)] + Subscription.objects.bulk_create(subs_to_recreate) + def do_deactivate_user(user_profile: UserProfile, acting_user: Optional[UserProfile]=None, _cascade: bool=True) -> None: diff --git a/zerver/lib/create_user.py b/zerver/lib/create_user.py index 85f2221f72..e573a204f4 100644 --- a/zerver/lib/create_user.py +++ b/zerver/lib/create_user.py @@ -72,10 +72,15 @@ def create_user_profile(realm: Realm, email: str, password: Optional[str], is_mirror_dummy: bool, tos_version: Optional[str], timezone: Optional[str], tutorial_status: str = UserProfile.TUTORIAL_WAITING, - enter_sends: bool = False) -> UserProfile: + enter_sends: bool = False, + force_id: Optional[int] = None) -> UserProfile: now = timezone_now() email = UserManager.normalize_email(email) + extra_kwargs = {} + if force_id is not None: + extra_kwargs['id'] = force_id + user_profile = UserProfile(is_staff=False, is_active=active, full_name=full_name, last_login=now, date_joined=now, realm=realm, @@ -87,7 +92,8 @@ def create_user_profile(realm: Realm, email: str, password: Optional[str], onboarding_steps=orjson.dumps([]).decode(), default_language=realm.default_language, twenty_four_hour_time=realm.default_twenty_four_hour_time, - delivery_email=email) + delivery_email=email, + **extra_kwargs) if bot_type or not active: password = None if user_profile.email_address_is_realm_public(): @@ -112,7 +118,8 @@ def create_user(email: str, default_sending_stream: Optional[Stream] = None, default_events_register_stream: Optional[Stream] = None, default_all_public_streams: Optional[bool] = None, - source_profile: Optional[UserProfile] = None) -> UserProfile: + source_profile: Optional[UserProfile] = None, + force_id: Optional[int] = None) -> UserProfile: user_profile = create_user_profile( realm, email, @@ -123,7 +130,8 @@ def create_user(email: str, bot_owner, is_mirror_dummy, tos_version, - timezone + timezone, + force_id=force_id ) user_profile.avatar_source = avatar_source user_profile.timezone = timezone diff --git a/zerver/tests/test_users.py b/zerver/tests/test_users.py index 617889b1a0..941bd49399 100644 --- a/zerver/tests/test_users.py +++ b/zerver/tests/test_users.py @@ -15,6 +15,7 @@ from zerver.lib.actions import ( do_change_user_role, do_create_user, do_deactivate_user, + do_delete_user, do_invite_users, do_reactivate_user, do_set_realm_property, @@ -50,6 +51,7 @@ from zerver.models import ( Recipient, ScheduledEmail, Stream, + Subscription, UserHotspot, UserProfile, check_valid_user_ids, @@ -1703,6 +1705,65 @@ class GetProfileTest(ZulipTestCase): avatar_url(hamlet), ) +class DeleteUserTest(ZulipTestCase): + def test_do_delete_user(self) -> None: + realm = get_realm("zulip") + cordelia = self.example_user('cordelia') + othello = self.example_user('othello') + hamlet = self.example_user('hamlet') + hamlet_personal_recipient = hamlet.recipient + hamlet_user_id = hamlet.id + + self.send_personal_message(cordelia, hamlet) + self.send_personal_message(hamlet, cordelia) + + personal_message_ids_to_hamlet = Message.objects.filter(recipient=hamlet_personal_recipient) \ + .values_list('id', flat=True) + self.assertTrue(len(personal_message_ids_to_hamlet) > 0) + self.assertTrue(Message.objects.filter(sender=hamlet).exists()) + + huddle_message_ids_from_cordelia = [ + self.send_huddle_message( + cordelia, + [hamlet, othello] + ) + for i in range(3) + ] + huddle_message_ids_from_hamlet = [ + self.send_huddle_message( + hamlet, + [cordelia, othello] + ) + for i in range(3) + ] + + huddle_with_hamlet_recipient_ids = list( + Subscription.objects.filter(user_profile=hamlet, recipient__type=Recipient.HUDDLE) + .values_list('recipient_id', flat=True) + ) + self.assertTrue(len(huddle_with_hamlet_recipient_ids) > 0) + + do_delete_user(hamlet) + + replacement_dummy_user = UserProfile.objects.get(id=hamlet_user_id, realm=realm) + + self.assertEqual(replacement_dummy_user.delivery_email, f"deleteduser{hamlet_user_id}@{realm.uri}") + self.assertEqual(replacement_dummy_user.is_mirror_dummy, True) + + self.assertEqual(Message.objects.filter(id__in=personal_message_ids_to_hamlet).count(), 0) + # Huddle messages from hamlet should have been deleted, but messages of other participants should + # be kept. + self.assertEqual(Message.objects.filter(id__in=huddle_message_ids_from_hamlet).count(), 0) + self.assertEqual(Message.objects.filter(id__in=huddle_message_ids_from_cordelia).count(), 3) + + self.assertEqual(Message.objects.filter(sender_id=hamlet_user_id).count(), 0) + + # Verify that the dummy user is subscribed to the deleted user's huddles, to keep huddle data + # in a correct state. + for recipient_id in huddle_with_hamlet_recipient_ids: + self.assertTrue(Subscription.objects.filter(user_profile=replacement_dummy_user, + recipient_id=recipient_id).exists()) + class FakeEmailDomainTest(ZulipTestCase): @override_settings(FAKE_EMAIL_DOMAIN="invaliddomain") def test_invalid_fake_email_domain(self) -> None: