actions: Rename do_activate_user to do_activate_mirror_dummy_user.

This function had a confusing name, which could result in someone
using it unintentionally when they meant do_reactivate_user.

We also add docstrings for both functions.
This commit is contained in:
Tim Abbott 2021-07-08 17:27:06 -07:00
parent 093ca7a574
commit c804ab27d5
5 changed files with 28 additions and 14 deletions

View File

@ -33,7 +33,7 @@ from analytics.models import (
installation_epoch, installation_epoch,
) )
from zerver.lib.actions import ( from zerver.lib.actions import (
do_activate_user, do_activate_mirror_dummy_user,
do_create_realm, do_create_realm,
do_create_user, do_create_user,
do_deactivate_user, do_deactivate_user,
@ -1329,7 +1329,7 @@ class TestLoggingCountStats(AnalyticsTestCase):
"value__sum" "value__sum"
], ],
) )
do_activate_user(user, acting_user=None) do_activate_mirror_dummy_user(user, acting_user=None)
self.assertEqual( self.assertEqual(
1, 1,
RealmCount.objects.filter(property=property, subgroup=False).aggregate(Sum("value"))[ RealmCount.objects.filter(property=property, subgroup=False).aggregate(Sum("value"))[
@ -1663,7 +1663,7 @@ class TestActiveUsersAudit(AnalyticsTestCase):
"email4", "password", self.default_realm, "full_name", acting_user=None "email4", "password", self.default_realm, "full_name", acting_user=None
) )
do_deactivate_user(user2, acting_user=None) do_deactivate_user(user2, acting_user=None)
do_activate_user(user3, acting_user=None) do_activate_mirror_dummy_user(user3, acting_user=None)
do_reactivate_user(user4, acting_user=None) do_reactivate_user(user4, acting_user=None)
end_time = floor_to_day(timezone_now()) + self.DAY end_time = floor_to_day(timezone_now()) + self.DAY
do_fill_count_stat_at_hour(self.stat, end_time) do_fill_count_stat_at_hour(self.stat, end_time)

View File

@ -78,7 +78,7 @@ from corporate.models import (
get_customer_by_realm, get_customer_by_realm,
) )
from zerver.lib.actions import ( from zerver.lib.actions import (
do_activate_user, do_activate_mirror_dummy_user,
do_create_realm, do_create_realm,
do_create_user, do_create_user,
do_deactivate_realm, do_deactivate_realm,
@ -3366,8 +3366,8 @@ class LicenseLedgerTest(StripeTestCase):
user = do_create_user("email", "password", get_realm("zulip"), "name", acting_user=None) user = do_create_user("email", "password", get_realm("zulip"), "name", acting_user=None)
do_deactivate_user(user, acting_user=None) do_deactivate_user(user, acting_user=None)
do_reactivate_user(user, acting_user=None) do_reactivate_user(user, acting_user=None)
# Not a proper use of do_activate_user, but fine for this test # Not a proper use of do_activate_mirror_dummy_user, but fine for this test
do_activate_user(user, acting_user=None) do_activate_mirror_dummy_user(user, acting_user=None)
ledger_entries = list( ledger_entries = list(
LicenseLedger.objects.values_list( LicenseLedger.objects.values_list(
"is_renewal", "licenses", "licenses_at_next_renewal" "is_renewal", "licenses", "licenses_at_next_renewal"

View File

@ -707,7 +707,22 @@ def do_create_user(
return user_profile return user_profile
def do_activate_user(user_profile: UserProfile, *, acting_user: Optional[UserProfile]) -> None: def do_activate_mirror_dummy_user(
user_profile: UserProfile, *, acting_user: Optional[UserProfile]
) -> None:
"""Called to have a user "take over" a "mirror dummy" user
(i.e. is_mirror_dummy=True) account when they sign up with the
same email address.
Essentially, the result should be as though we had created the
UserProfile just now with do_create_user, except that the mirror
dummy user may appear as the recipient or sender of messages from
before their account was fully created.
TODO: This function likely has bugs resulting from this being a
parallel code path to do_create_user; e.g. it likely does not
handle preferences or default streams properly.
"""
with transaction.atomic(): with transaction.atomic():
change_user_is_active(user_profile, True) change_user_is_active(user_profile, True)
user_profile.is_mirror_dummy = False user_profile.is_mirror_dummy = False
@ -744,8 +759,7 @@ def do_activate_user(user_profile: UserProfile, *, acting_user: Optional[UserPro
def do_reactivate_user(user_profile: UserProfile, *, acting_user: Optional[UserProfile]) -> None: def do_reactivate_user(user_profile: UserProfile, *, acting_user: Optional[UserProfile]) -> None:
# Unlike do_activate_user, this is meant for re-activating existing users, """Reactivate a user that had previously been deactivated"""
# so it doesn't reset their password, etc.
with transaction.atomic(): with transaction.atomic():
change_user_is_active(user_profile, True) change_user_is_active(user_profile, True)

View File

@ -9,7 +9,7 @@ from analytics.models import StreamCount
from zerver.lib.actions import ( from zerver.lib.actions import (
bulk_add_subscriptions, bulk_add_subscriptions,
bulk_remove_subscriptions, bulk_remove_subscriptions,
do_activate_user, do_activate_mirror_dummy_user,
do_change_avatar_fields, do_change_avatar_fields,
do_change_bot_owner, do_change_bot_owner,
do_change_default_all_public_streams, do_change_default_all_public_streams,
@ -70,7 +70,7 @@ class TestRealmAuditLog(ZulipTestCase):
now = timezone_now() now = timezone_now()
user = do_create_user("email", "password", realm, "full_name", acting_user=None) user = do_create_user("email", "password", realm, "full_name", acting_user=None)
do_deactivate_user(user, acting_user=user) do_deactivate_user(user, acting_user=user)
do_activate_user(user, acting_user=user) do_activate_mirror_dummy_user(user, acting_user=user)
do_deactivate_user(user, acting_user=user) do_deactivate_user(user, acting_user=user)
do_reactivate_user(user, acting_user=user) do_reactivate_user(user, acting_user=user)
self.assertEqual(RealmAuditLog.objects.filter(event_time__gte=now).count(), 6) self.assertEqual(RealmAuditLog.objects.filter(event_time__gte=now).count(), 6)

View File

@ -36,7 +36,7 @@ from zerver.forms import (
) )
from zerver.lib.actions import ( from zerver.lib.actions import (
bulk_add_subscriptions, bulk_add_subscriptions,
do_activate_user, do_activate_mirror_dummy_user,
do_change_full_name, do_change_full_name,
do_change_password, do_change_password,
do_create_realm, do_create_realm,
@ -406,11 +406,11 @@ def accounts_register(request: HttpRequest) -> HttpResponse:
if existing_user_profile is not None and existing_user_profile.is_mirror_dummy: if existing_user_profile is not None and existing_user_profile.is_mirror_dummy:
user_profile = existing_user_profile user_profile = existing_user_profile
do_activate_user(user_profile, acting_user=user_profile) do_activate_mirror_dummy_user(user_profile, acting_user=user_profile)
do_change_password(user_profile, password) do_change_password(user_profile, password)
do_change_full_name(user_profile, full_name, user_profile) do_change_full_name(user_profile, full_name, user_profile)
do_set_user_display_setting(user_profile, "timezone", timezone) do_set_user_display_setting(user_profile, "timezone", timezone)
# TODO: When we clean up the `do_activate_user` code path, # TODO: When we clean up the `do_activate_mirror_dummy_user` code path,
# make it respect invited_as_admin / is_realm_admin. # make it respect invited_as_admin / is_realm_admin.
if user_profile is None: if user_profile is None: