refactor: Make acting_user a mandatory kwarg for do_deactivate_user.

This commit is contained in:
shanukun 2021-03-27 10:32:12 +05:30 committed by Tim Abbott
parent 8f3ae715c0
commit c95061e9b9
16 changed files with 39 additions and 39 deletions

View File

@ -1322,7 +1322,7 @@ class TestLoggingCountStats(AnalyticsTestCase):
"value__sum" "value__sum"
], ],
) )
do_deactivate_user(user) do_deactivate_user(user, acting_user=None)
self.assertEqual( self.assertEqual(
0, 0,
RealmCount.objects.filter(property=property, subgroup=False).aggregate(Sum("value"))[ RealmCount.objects.filter(property=property, subgroup=False).aggregate(Sum("value"))[
@ -1336,7 +1336,7 @@ class TestLoggingCountStats(AnalyticsTestCase):
"value__sum" "value__sum"
], ],
) )
do_deactivate_user(user) do_deactivate_user(user, acting_user=None)
self.assertEqual( self.assertEqual(
0, 0,
RealmCount.objects.filter(property=property, subgroup=False).aggregate(Sum("value"))[ RealmCount.objects.filter(property=property, subgroup=False).aggregate(Sum("value"))[
@ -1662,7 +1662,7 @@ class TestActiveUsersAudit(AnalyticsTestCase):
user4 = do_create_user( user4 = do_create_user(
"email4", "password", self.default_realm, "full_name", acting_user=None "email4", "password", self.default_realm, "full_name", acting_user=None
) )
do_deactivate_user(user2) do_deactivate_user(user2, acting_user=None)
do_activate_user(user3, acting_user=None) do_activate_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
@ -1785,7 +1785,7 @@ class TestRealmActiveHumans(AnalyticsTestCase):
time_zero = floor_to_day(timezone_now()) + self.DAY time_zero = floor_to_day(timezone_now()) + self.DAY
update_user_activity_interval(user1, time_zero) update_user_activity_interval(user1, time_zero)
update_user_activity_interval(user2, time_zero) update_user_activity_interval(user2, time_zero)
do_deactivate_user(user2) do_deactivate_user(user2, acting_user=None)
for property in [ for property in [
"active_users_audit:is_bot:day", "active_users_audit:is_bot:day",
"15day_actives::day", "15day_actives::day",

View File

@ -1531,7 +1531,7 @@ class StripeTest(StripeTestCase):
self.assertEqual(get_latest_seat_count(realm), initial_count + 1) self.assertEqual(get_latest_seat_count(realm), initial_count + 1)
# Test that inactive users aren't counted # Test that inactive users aren't counted
do_deactivate_user(user2) do_deactivate_user(user2, acting_user=None)
self.assertEqual(get_latest_seat_count(realm), initial_count) self.assertEqual(get_latest_seat_count(realm), initial_count)
# Test guests # Test guests
@ -2719,7 +2719,7 @@ class LicenseLedgerTest(StripeTestCase):
def test_user_changes(self) -> None: def test_user_changes(self) -> None:
self.local_upgrade(self.seat_count, True, CustomerPlan.ANNUAL, "token") self.local_upgrade(self.seat_count, True, CustomerPlan.ANNUAL, "token")
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) 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_user, but fine for this test
do_activate_user(user, acting_user=None) do_activate_user(user, acting_user=None)

View File

@ -1090,7 +1090,7 @@ def do_delete_user(user_profile: UserProfile) -> None:
if user_profile.realm.is_zephyr_mirror_realm: if user_profile.realm.is_zephyr_mirror_realm:
raise AssertionError("Deleting zephyr mirror users is not supported") raise AssertionError("Deleting zephyr mirror users is not supported")
do_deactivate_user(user_profile) do_deactivate_user(user_profile, acting_user=None)
subscribed_huddle_recipient_ids = set( subscribed_huddle_recipient_ids = set(
Subscription.objects.filter( Subscription.objects.filter(
@ -1122,7 +1122,7 @@ def do_delete_user(user_profile: UserProfile) -> None:
def do_deactivate_user( def do_deactivate_user(
user_profile: UserProfile, acting_user: Optional[UserProfile] = None, _cascade: bool = True user_profile: UserProfile, _cascade: bool = True, *, acting_user: Optional[UserProfile]
) -> None: ) -> None:
if not user_profile.is_active: if not user_profile.is_active:
return return
@ -1184,7 +1184,7 @@ def do_deactivate_user(
is_bot=True, is_active=True, bot_owner=user_profile is_bot=True, is_active=True, bot_owner=user_profile
) )
for profile in bot_profiles: for profile in bot_profiles:
do_deactivate_user(profile, acting_user=acting_user, _cascade=False) do_deactivate_user(profile, _cascade=False, acting_user=acting_user)
def do_deactivate_stream( def do_deactivate_stream(

View File

@ -45,5 +45,5 @@ class Command(ZulipBaseCommand):
if not options["for_real"]: if not options["for_real"]:
raise CommandError("This was a dry run. Pass -f to actually deactivate.") raise CommandError("This was a dry run. Pass -f to actually deactivate.")
do_deactivate_user(user_profile) do_deactivate_user(user_profile, acting_user=None)
print("Sessions deleted, user deactivated.") print("Sessions deleted, user deactivated.")

View File

@ -162,7 +162,7 @@ class AuthBackendTest(ZulipTestCase):
self.assertEqual(user_profile, result) self.assertEqual(user_profile, result)
# Verify auth fails with a deactivated user # Verify auth fails with a deactivated user
do_deactivate_user(user_profile) do_deactivate_user(user_profile, acting_user=None)
result = backend.authenticate(**good_kwargs) result = backend.authenticate(**good_kwargs)
if isinstance(backend, SocialAuthMixin): if isinstance(backend, SocialAuthMixin):
# Returns a redirect to login page with an error. # Returns a redirect to login page with an error.
@ -1067,7 +1067,7 @@ class SocialAuthBase(DesktopFlowTestingLib, ZulipTestCase):
def test_social_auth_deactivated_user(self) -> None: def test_social_auth_deactivated_user(self) -> None:
user_profile = self.example_user("hamlet") user_profile = self.example_user("hamlet")
do_deactivate_user(user_profile) do_deactivate_user(user_profile, acting_user=None)
account_data_dict = self.get_account_data_dict(email=self.email, name=self.name) account_data_dict = self.get_account_data_dict(email=self.email, name=self.name)
# We expect to go through the "choose email" screen here, # We expect to go through the "choose email" screen here,
# because there won't be an existing user account we can # because there won't be an existing user account we can
@ -3812,7 +3812,7 @@ class FetchAPIKeyTest(ZulipTestCase):
self.assert_json_success(result) self.assert_json_success(result)
def test_inactive_user(self) -> None: def test_inactive_user(self) -> None:
do_deactivate_user(self.user_profile) do_deactivate_user(self.user_profile, acting_user=None)
result = self.client_post( result = self.client_post(
"/api/v1/fetch_api_key", "/api/v1/fetch_api_key",
dict(username=self.email, password=initial_password(self.email)), dict(username=self.email, password=initial_password(self.email)),
@ -3853,7 +3853,7 @@ class DevFetchAPIKeyTest(ZulipTestCase):
self.assert_json_error_contains(result, "This user is not registered.", 403) self.assert_json_error_contains(result, "This user is not registered.", 403)
def test_inactive_user(self) -> None: def test_inactive_user(self) -> None:
do_deactivate_user(self.user_profile) do_deactivate_user(self.user_profile, acting_user=None)
result = self.client_post("/api/v1/dev_fetch_api_key", dict(username=self.email)) result = self.client_post("/api/v1/dev_fetch_api_key", dict(username=self.email))
self.assert_json_error_contains(result, "Your account has been disabled", 403) self.assert_json_error_contains(result, "Your account has been disabled", 403)
@ -5181,7 +5181,7 @@ class TestLDAP(ZulipLDAPTestCase):
@override_settings(AUTHENTICATION_BACKENDS=("zproject.backends.ZulipLDAPAuthBackend",)) @override_settings(AUTHENTICATION_BACKENDS=("zproject.backends.ZulipLDAPAuthBackend",))
def test_login_failure_due_to_deactivated_user(self) -> None: def test_login_failure_due_to_deactivated_user(self) -> None:
user_profile = self.example_user("hamlet") user_profile = self.example_user("hamlet")
do_deactivate_user(user_profile) do_deactivate_user(user_profile, acting_user=None)
with self.settings(LDAP_APPEND_DOMAIN="zulip.com"): with self.settings(LDAP_APPEND_DOMAIN="zulip.com"):
user_profile = self.backend.authenticate( user_profile = self.backend.authenticate(
request=mock.MagicMock(), request=mock.MagicMock(),
@ -5364,7 +5364,7 @@ class TestZulipLDAPUserPopulator(ZulipLDAPTestCase):
) )
def test_reactivate_user(self) -> None: def test_reactivate_user(self) -> None:
do_deactivate_user(self.example_user("hamlet")) do_deactivate_user(self.example_user("hamlet"), acting_user=None)
with self.settings( with self.settings(
AUTH_LDAP_USER_ATTR_MAP={"full_name": "cn", "userAccountControl": "userAccountControl"} AUTH_LDAP_USER_ATTR_MAP={"full_name": "cn", "userAccountControl": "userAccountControl"}
@ -5411,7 +5411,7 @@ class TestZulipLDAPUserPopulator(ZulipLDAPTestCase):
) )
self.assertFalse(result) self.assertFalse(result)
do_deactivate_user(othello) do_deactivate_user(othello, acting_user=None)
mock_logger = mock.MagicMock() mock_logger = mock.MagicMock()
result = sync_user_from_ldap(othello, mock_logger) result = sync_user_from_ldap(othello, mock_logger)
# In this case the logger shouldn't be used. # In this case the logger shouldn't be used.

View File

@ -777,7 +777,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
bot_email = "hambot-bot@zulip.testserver" bot_email = "hambot-bot@zulip.testserver"
bot_user = get_user(bot_email, bot_realm) bot_user = get_user(bot_email, bot_realm)
do_deactivate_user(bot_user) do_deactivate_user(bot_user, acting_user=None)
# A regular user cannot reactivate a generic bot # A regular user cannot reactivate a generic bot
self.assert_num_bots_equal(0) self.assert_num_bots_equal(0)
@ -963,7 +963,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
self.assert_num_bots_equal(1) self.assert_num_bots_equal(1)
target_user_profile = self.example_user("othello") target_user_profile = self.example_user("othello")
do_deactivate_user(target_user_profile) do_deactivate_user(target_user_profile, acting_user=None)
target_user_profile = self.example_user("othello") target_user_profile = self.example_user("othello")
self.assertFalse(target_user_profile.is_active) self.assertFalse(target_user_profile.is_active)
bot_info = { bot_info = {

View File

@ -1184,7 +1184,7 @@ class InactiveUserTest(ZulipTestCase):
""" """
user_profile = self.example_user("hamlet") user_profile = self.example_user("hamlet")
self.login_user(user_profile) self.login_user(user_profile)
do_deactivate_user(user_profile) do_deactivate_user(user_profile, acting_user=None)
result = self.client_post( result = self.client_post(
"/json/messages", "/json/messages",
@ -1249,7 +1249,7 @@ class InactiveUserTest(ZulipTestCase):
""" """
user_profile = self.example_user("hamlet") user_profile = self.example_user("hamlet")
do_deactivate_user(user_profile) do_deactivate_user(user_profile, acting_user=None)
result = self.login_with_return(self.example_email("hamlet")) result = self.login_with_return(self.example_email("hamlet"))
self.assert_in_response("Your account is no longer active.", result) self.assert_in_response("Your account is no longer active.", result)
@ -1278,7 +1278,7 @@ class InactiveUserTest(ZulipTestCase):
self.assertTrue(form.is_valid()) self.assertTrue(form.is_valid())
# Test a mirror-dummy deactivated user. # Test a mirror-dummy deactivated user.
do_deactivate_user(user_profile) do_deactivate_user(user_profile, acting_user=None)
user_profile.save() user_profile.save()
form = OurAuthenticationForm(request, payload) form = OurAuthenticationForm(request, payload)
@ -1301,7 +1301,7 @@ class InactiveUserTest(ZulipTestCase):
""" """
user_profile = self.example_user("hamlet") user_profile = self.example_user("hamlet")
do_deactivate_user(user_profile) do_deactivate_user(user_profile, acting_user=None)
api_key = get_api_key(user_profile) api_key = get_api_key(user_profile)
url = f"/api/v1/external/jira?api_key={api_key}&stream=jira_custom" url = f"/api/v1/external/jira?api_key={api_key}&stream=jira_custom"

View File

@ -1041,7 +1041,7 @@ class TestMissedMessageEmailMessages(ZulipTestCase):
mm_address = create_missed_message_address(user_profile, message) mm_address = create_missed_message_address(user_profile, message)
do_deactivate_user(user_profile) do_deactivate_user(user_profile, acting_user=None)
incoming_valid_message = EmailMessage() incoming_valid_message = EmailMessage()
incoming_valid_message.set_content("TestMissedMessageEmailMessages Body") incoming_valid_message.set_content("TestMissedMessageEmailMessages Body")

View File

@ -1480,14 +1480,14 @@ class NormalActionsTest(BaseAction):
def test_do_deactivate_user(self) -> None: def test_do_deactivate_user(self) -> None:
bot = self.create_bot("test") bot = self.create_bot("test")
action = lambda: do_deactivate_user(bot) action = lambda: do_deactivate_user(bot, acting_user=None)
events = self.verify_action(action, num_events=2) events = self.verify_action(action, num_events=2)
check_realm_user_remove("events[0]", events[0]) check_realm_user_remove("events[0]", events[0])
check_realm_bot_remove("events[1]", events[1]) check_realm_bot_remove("events[1]", events[1])
def test_do_reactivate_user(self) -> None: def test_do_reactivate_user(self) -> None:
bot = self.create_bot("test") bot = self.create_bot("test")
do_deactivate_user(bot) do_deactivate_user(bot, acting_user=None)
action = lambda: do_reactivate_user(bot, acting_user=None) action = lambda: do_reactivate_user(bot, acting_user=None)
events = self.verify_action(action, num_events=2) events = self.verify_action(action, num_events=2)
check_realm_bot_add("events[1]", events[1]) check_realm_bot_add("events[1]", events[1])

View File

@ -1697,7 +1697,7 @@ class GetOldMessagesTest(ZulipTestCase):
[self.example_user("iago"), self.example_user("aaron")], [self.example_user("iago"), self.example_user("aaron")],
) )
aaron = self.example_user("aaron") aaron = self.example_user("aaron")
do_deactivate_user(aaron) do_deactivate_user(aaron, acting_user=None)
self.assertFalse(aaron.is_active) self.assertFalse(aaron.is_active)
personals = [ personals = [

View File

@ -546,7 +546,7 @@ class MessagePOSTTest(ZulipTestCase):
""" """
othello = self.example_user("othello") othello = self.example_user("othello")
cordelia = self.example_user("cordelia") cordelia = self.example_user("cordelia")
do_deactivate_user(othello) do_deactivate_user(othello, acting_user=None)
self.login("hamlet") self.login("hamlet")
result = self.client_post( result = self.client_post(

View File

@ -468,7 +468,7 @@ class SingleUserPresenceTests(ZulipTestCase):
result = self.client_get(f"/json/users/{cordelia.id}/presence") result = self.client_get(f"/json/users/{cordelia.id}/presence")
self.assert_json_error(result, f"No presence data for {cordelia.id}") self.assert_json_error(result, f"No presence data for {cordelia.id}")
do_deactivate_user(self.example_user("cordelia")) do_deactivate_user(self.example_user("cordelia"), acting_user=None)
result = self.client_get("/json/users/cordelia@zulip.com/presence") result = self.client_get("/json/users/cordelia@zulip.com/presence")
self.assert_json_error(result, "No such user") self.assert_json_error(result, "No such user")

View File

@ -443,7 +443,7 @@ class PasswordResetTest(ZulipTestCase):
def test_password_reset_for_deactivated_user(self) -> None: def test_password_reset_for_deactivated_user(self) -> None:
user_profile = self.example_user("hamlet") user_profile = self.example_user("hamlet")
email = user_profile.delivery_email email = user_profile.delivery_email
do_deactivate_user(user_profile) do_deactivate_user(user_profile, acting_user=None)
# start the password reset process by supplying an email address # start the password reset process by supplying an email address
result = self.client_post("/accounts/password/reset/", {"email": email}) result = self.client_post("/accounts/password/reset/", {"email": email})
@ -654,7 +654,7 @@ class LoginTest(ZulipTestCase):
def test_login_deactivated_user(self) -> None: def test_login_deactivated_user(self) -> None:
user_profile = self.example_user("hamlet") user_profile = self.example_user("hamlet")
do_deactivate_user(user_profile) do_deactivate_user(user_profile, acting_user=None)
result = self.login_with_return(self.example_email("hamlet"), "xxx") result = self.login_with_return(self.example_email("hamlet"), "xxx")
self.assertEqual(result.status_code, 200) self.assertEqual(result.status_code, 200)
self.assert_in_response("Your account is no longer active.", result) self.assert_in_response("Your account is no longer active.", result)
@ -4768,7 +4768,7 @@ class TestFindMyTeam(ZulipTestCase):
self.assertEqual(len(outbox), 1) self.assertEqual(len(outbox), 1)
def test_find_team_deactivated_user(self) -> None: def test_find_team_deactivated_user(self) -> None:
do_deactivate_user(self.example_user("hamlet")) do_deactivate_user(self.example_user("hamlet"), acting_user=None)
data = {"emails": self.example_email("hamlet")} data = {"emails": self.example_email("hamlet")}
result = self.client_post("/accounts/find/", data) result = self.client_post("/accounts/find/", data)
self.assertEqual(result.status_code, 302) self.assertEqual(result.status_code, 302)

View File

@ -3922,7 +3922,7 @@ class SubscriptionAPITest(ZulipTestCase):
) )
self.common_subscribe_to_streams(self.test_user, "Verona", post_data) self.common_subscribe_to_streams(self.test_user, "Verona", post_data)
do_deactivate_user(target_profile) do_deactivate_user(target_profile, acting_user=None)
result = self.common_subscribe_to_streams( result = self.common_subscribe_to_streams(
self.test_user, "Denmark", post_data, allow_fail=True self.test_user, "Denmark", post_data, allow_fail=True
) )

View File

@ -405,7 +405,7 @@ class PermissionTest(ZulipTestCase):
# Can only access deactivated users if allow_deactivated is passed # Can only access deactivated users if allow_deactivated is passed
hamlet = self.example_user("hamlet") hamlet = self.example_user("hamlet")
do_deactivate_user(hamlet) do_deactivate_user(hamlet, acting_user=None)
with self.assertRaises(JsonableError): with self.assertRaises(JsonableError):
access_user_by_id(iago, hamlet.id, for_admin=False) access_user_by_id(iago, hamlet.id, for_admin=False)
with self.assertRaises(JsonableError): with self.assertRaises(JsonableError):
@ -1158,7 +1158,7 @@ class UserProfileTest(ZulipTestCase):
# We verify that get_accounts_for_email don't return deactivated users accounts # We verify that get_accounts_for_email don't return deactivated users accounts
user = self.example_user("hamlet") user = self.example_user("hamlet")
do_deactivate_user(user) do_deactivate_user(user, acting_user=None)
email = self.example_email("hamlet") email = self.example_email("hamlet")
accounts = get_accounts_for_email(email) accounts = get_accounts_for_email(email)
with self.assertRaises(AssertionError): with self.assertRaises(AssertionError):
@ -1326,7 +1326,7 @@ class UserProfileTest(ZulipTestCase):
class ActivateTest(ZulipTestCase): class ActivateTest(ZulipTestCase):
def test_basics(self) -> None: def test_basics(self) -> None:
user = self.example_user("hamlet") user = self.example_user("hamlet")
do_deactivate_user(user) do_deactivate_user(user, acting_user=None)
self.assertFalse(user.is_active) self.assertFalse(user.is_active)
do_reactivate_user(user, acting_user=None) do_reactivate_user(user, acting_user=None)
self.assertTrue(user.is_active) self.assertTrue(user.is_active)
@ -1407,7 +1407,7 @@ class ActivateTest(ZulipTestCase):
delay=datetime.timedelta(hours=1), delay=datetime.timedelta(hours=1),
) )
self.assertEqual(ScheduledEmail.objects.count(), 1) self.assertEqual(ScheduledEmail.objects.count(), 1)
do_deactivate_user(user) do_deactivate_user(user, acting_user=None)
self.assertEqual(ScheduledEmail.objects.count(), 0) self.assertEqual(ScheduledEmail.objects.count(), 0)
def test_send_future_email_with_multiple_recipients(self) -> None: def test_send_future_email_with_multiple_recipients(self) -> None:

View File

@ -903,7 +903,7 @@ class ZulipLDAPUserPopulator(ZulipLDAPAuthBackendBase):
"Deactivating user %s because they are disabled in LDAP.", "Deactivating user %s because they are disabled in LDAP.",
user.delivery_email, user.delivery_email,
) )
do_deactivate_user(user) do_deactivate_user(user, acting_user=None)
# Do an early return to avoid trying to sync additional data. # Do an early return to avoid trying to sync additional data.
return (user, built) return (user, built)
elif not user.is_active: elif not user.is_active:
@ -949,7 +949,7 @@ def sync_user_from_ldap(user_profile: UserProfile, logger: logging.Logger) -> bo
if settings.LDAP_DEACTIVATE_NON_MATCHING_USERS is None if settings.LDAP_DEACTIVATE_NON_MATCHING_USERS is None
else settings.LDAP_DEACTIVATE_NON_MATCHING_USERS else settings.LDAP_DEACTIVATE_NON_MATCHING_USERS
): ):
do_deactivate_user(user_profile) do_deactivate_user(user_profile, acting_user=None)
logger.info("Deactivated non-matching user: %s", user_profile.delivery_email) logger.info("Deactivated non-matching user: %s", user_profile.delivery_email)
return True return True
elif user_profile.is_active: elif user_profile.is_active: