audit_log: Record RealmAuditLog in do_change_notification_settings.

Removed logging with log_event and used RealmAuditLog instead.
Added tests in test_audit_log for the same.
This commit is contained in:
arpit551
2020-07-13 03:11:53 +05:30
committed by Tim Abbott
parent 54df9290b9
commit 0d79b55b2e
7 changed files with 49 additions and 16 deletions

View File

@@ -3755,11 +3755,13 @@ def do_create_realm(string_id: str, name: str,
return realm
def do_change_notification_settings(user_profile: UserProfile, name: str,
value: Union[bool, int, str], log: bool=True) -> None:
value: Union[bool, int, str],
acting_user: Optional[UserProfile]=None) -> None:
"""Takes in a UserProfile object, the name of a global notification
preference to update, and the value to update to
"""
old_value = getattr(user_profile, name)
notification_setting_type = UserProfile.notification_setting_types[name]
assert isinstance(value, notification_setting_type), (
f'Cannot update {name}: {value} is not an instance of {notification_setting_type}')
@@ -3775,8 +3777,14 @@ def do_change_notification_settings(user_profile: UserProfile, name: str,
'user': user_profile.email,
'notification_name': name,
'setting': value}
if log:
log_event(event)
event_time = timezone_now()
RealmAuditLog.objects.create(
realm=user_profile.realm, event_type=RealmAuditLog.USER_NOTIFICATION_SETTINGS_CHANGED, event_time=event_time,
acting_user=acting_user, modified_user=user_profile, extra_data=ujson.dumps({
RealmAuditLog.OLD_VALUE: {'property': name, 'value': old_value},
RealmAuditLog.NEW_VALUE: {'property': name, 'value': value}
}))
send_event(user_profile.realm, event, [user_profile.id])
def do_change_enter_sends(user_profile: UserProfile, enter_sends: bool) -> None:

View File

@@ -23,7 +23,7 @@ class Command(ZulipBaseCommand):
for user_profile in user_profiles:
already_disabled_prefix = ""
if user_profile.enable_digest_emails:
do_change_notification_settings(user_profile, 'enable_digest_emails', False)
do_change_notification_settings(user_profile, 'enable_digest_emails', False, acting_user=None)
else:
already_disabled_prefix = "(already off) "
print(f"{already_disabled_prefix}{user_profile.full_name} <{user_profile.delivery_email}>")

View File

@@ -2715,6 +2715,7 @@ class AbstractRealmAuditLog(models.Model):
USER_DEFAULT_SENDING_STREAM_CHANGED = 129
USER_DEFAULT_REGISTER_STREAM_CHANGED = 130
USER_DEFAULT_ALL_PUBLIC_STREAMS_CHANGED = 131
USER_NOTIFICATION_SETTINGS_CHANGED = 132
REALM_DEACTIVATED = 201
REALM_REACTIVATED = 202

View File

@@ -1,5 +1,5 @@
from datetime import timedelta
from typing import Any, Dict
from typing import Any, Dict, Union
import ujson
from django.contrib.auth.password_validation import validate_password
@@ -16,6 +16,7 @@ from zerver.lib.actions import (
do_change_default_events_register_stream,
do_change_default_sending_stream,
do_change_icon_source,
do_change_notification_settings,
do_change_password,
do_change_subscription_property,
do_change_tos_version,
@@ -439,3 +440,25 @@ class TestRealmAuditLog(ZulipTestCase):
RealmAuditLog.NEW_VALUE: 'updated name'
})).count(), 1)
self.assertEqual(stream.name, 'updated name')
def test_change_notification_settings(self) -> None:
user = self.example_user('hamlet')
value: Union[bool, int, str]
for setting, v in user.notification_setting_types.items():
if setting == "notification_sound":
value = 'ding'
elif setting == "desktop_icon_count_display":
value = 3
else:
value = False
now = timezone_now()
old_value = getattr(user, setting)
do_change_notification_settings(user, setting, value, acting_user=user)
expected_extra_data = {RealmAuditLog.OLD_VALUE: {'property': setting, 'value': old_value},
RealmAuditLog.NEW_VALUE: {'property': setting, 'value': value}}
self.assertEqual(RealmAuditLog.objects.filter(
realm=user.realm, event_type=RealmAuditLog.USER_NOTIFICATION_SETTINGS_CHANGED,
event_time__gte=now, acting_user=user, modified_user=user,
extra_data=ujson.dumps(expected_extra_data)).count(), 1)
self.assertEqual(getattr(user, setting), value)

View File

@@ -1270,7 +1270,8 @@ class NormalActionsTest(BaseAction):
# These settings are tested in their own tests.
continue
do_change_notification_settings(self.user_profile, notification_setting, False)
do_change_notification_settings(self.user_profile, notification_setting, False,
acting_user=self.user_profile)
for setting_value in [True, False]:
events = self.verify_action(
@@ -1278,13 +1279,14 @@ class NormalActionsTest(BaseAction):
self.user_profile,
notification_setting,
setting_value,
log=False))
acting_user=self.user_profile))
check_update_global_notifications('events[0]', events[0], setting_value)
# Also test with notification_settings_null=True
events = self.verify_action(
lambda: do_change_notification_settings(
self.user_profile, notification_setting, setting_value, log=False),
self.user_profile, notification_setting, setting_value,
acting_user=self.user_profile),
notification_settings_null=True,
state_change_expected=False)
check_update_global_notifications('events[0]', events[0], setting_value)
@@ -1296,8 +1298,7 @@ class NormalActionsTest(BaseAction):
lambda: do_change_notification_settings(
self.user_profile,
notification_setting,
'ding',
log=False))
'ding'))
check_update_global_notifications('events[0]', events[0], 'ding')
def test_change_desktop_icon_count_display(self) -> None:
@@ -1308,7 +1309,7 @@ class NormalActionsTest(BaseAction):
self.user_profile,
notification_setting,
2,
log=False))
acting_user=self.user_profile))
check_update_global_notifications('events[0]', events[0], 2)
events = self.verify_action(
@@ -1316,7 +1317,7 @@ class NormalActionsTest(BaseAction):
self.user_profile,
notification_setting,
1,
log=False))
acting_user=self.user_profile))
check_update_global_notifications('events[0]', events[0], 1)
def test_realm_update_plan_type(self) -> None:

View File

@@ -26,16 +26,16 @@ def process_unsubscribe(request: HttpRequest, confirmation_key: str, subscriptio
# processor(user_profile).
def do_missedmessage_unsubscribe(user_profile: UserProfile) -> None:
do_change_notification_settings(user_profile, 'enable_offline_email_notifications', False)
do_change_notification_settings(user_profile, 'enable_offline_email_notifications', False, acting_user=user_profile)
def do_welcome_unsubscribe(user_profile: UserProfile) -> None:
clear_scheduled_emails([user_profile.id], ScheduledEmail.WELCOME)
def do_digest_unsubscribe(user_profile: UserProfile) -> None:
do_change_notification_settings(user_profile, 'enable_digest_emails', False)
do_change_notification_settings(user_profile, 'enable_digest_emails', False, acting_user=user_profile)
def do_login_unsubscribe(user_profile: UserProfile) -> None:
do_change_notification_settings(user_profile, 'enable_login_emails', False)
do_change_notification_settings(user_profile, 'enable_login_emails', False, acting_user=user_profile)
# The keys are part of the URL for the unsubscribe link and must be valid
# without encoding.

View File

@@ -229,7 +229,7 @@ def json_change_notify_settings(
for k, v in list(req_vars.items()):
if v is not None and getattr(user_profile, k) != v:
do_change_notification_settings(user_profile, k, v)
do_change_notification_settings(user_profile, k, v, acting_user=user_profile)
result[k] = v
return json_success(result)