From 4166c901ef23e11e449fe33a38a0812f60f644a8 Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Tue, 1 Oct 2019 04:22:50 +0200 Subject: [PATCH] do_update_user_custom_profile_data: Rename to ..._if_changed. This adds clarity to the fact that the function no longer does anything if the field values haven't changed. --- zerver/lib/actions.py | 5 +++-- zerver/tests/test_auth_backends.py | 2 +- zerver/tests/test_custom_profile_data.py | 12 ++++++------ zerver/tests/test_events.py | 6 +++--- zerver/views/custom_profile_fields.py | 4 ++-- zerver/views/users.py | 4 ++-- zilencer/management/commands/populate_db.py | 6 +++--- zproject/backends.py | 4 ++-- 8 files changed, 22 insertions(+), 21 deletions(-) diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 04ec3ef25d..fff4031577 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -5458,8 +5458,9 @@ def notify_user_update_custom_profile_data(user_profile: UserProfile, event = dict(type="realm_user", op="update", person=payload) send_event(user_profile.realm, event, active_user_ids(user_profile.realm.id)) -def do_update_user_custom_profile_data(user_profile: UserProfile, - data: List[Dict[str, Union[int, str, List[int]]]]) -> None: +def do_update_user_custom_profile_data_if_changed(user_profile: UserProfile, + data: List[Dict[str, Union[int, str, List[int]]]] + ) -> None: with transaction.atomic(): for field in data: field_value, created = CustomProfileFieldValue.objects.get_or_create( diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index 75a26ce2f1..05375b58c1 100644 --- a/zerver/tests/test_auth_backends.py +++ b/zerver/tests/test_auth_backends.py @@ -2915,7 +2915,7 @@ class TestZulipLDAPUserPopulator(ZulipLDAPTestCase): with self.settings(AUTH_LDAP_USER_ATTR_MAP={'full_name': 'cn', 'custom_profile_field__birthday': 'birthDate', 'custom_profile_field__phone_number': 'phoneNumber'}): - with mock.patch('zproject.backends.do_update_user_custom_profile_data') as f: + with mock.patch('zproject.backends.do_update_user_custom_profile_data_if_changed') as f: self.perform_ldap_sync(self.example_user('hamlet')) f.assert_called_once_with(*expected_call_args) diff --git a/zerver/tests/test_custom_profile_data.py b/zerver/tests/test_custom_profile_data.py index 1c5097dd34..2a83af2c13 100644 --- a/zerver/tests/test_custom_profile_data.py +++ b/zerver/tests/test_custom_profile_data.py @@ -3,7 +3,7 @@ from typing import Union, List, Dict, Any from zerver.lib.actions import try_add_realm_custom_profile_field, \ - do_update_user_custom_profile_data, do_remove_realm_custom_profile_field, \ + do_update_user_custom_profile_data_if_changed, do_remove_realm_custom_profile_field, \ try_reorder_realm_custom_profile_fields from zerver.lib.test_classes import ZulipTestCase from zerver.lib.bugdown import convert as bugdown_convert @@ -311,7 +311,7 @@ class DeleteCustomProfileFieldTest(CustomProfileFieldTestCase): field = CustomProfileField.objects.get(name="Mentor", realm=realm) data = [{'id': field.id, 'value': [self.example_user("aaron").id]}] # type: List[Dict[str, Union[int, str, List[int]]]] - do_update_user_custom_profile_data(iago, data) + do_update_user_custom_profile_data_if_changed(iago, data) iago_value = CustomProfileFieldValue.objects.get(user_profile=iago, field=field) converter = field.FIELD_CONVERTERS[field.field_type] @@ -333,7 +333,7 @@ class DeleteCustomProfileFieldTest(CustomProfileFieldTestCase): realm = user_profile.realm field = CustomProfileField.objects.get(name="Phone number", realm=realm) data = [{'id': field.id, 'value': u'123456'}] # type: List[Dict[str, Union[int, str, List[int]]]] - do_update_user_custom_profile_data(user_profile, data) + do_update_user_custom_profile_data_if_changed(user_profile, data) self.assertTrue(self.custom_field_exists_in_realm(field.id)) self.assertEqual(user_profile.customprofilefieldvalue_set.count(), self.original_count) @@ -588,7 +588,7 @@ class UpdateCustomProfileFieldTest(CustomProfileFieldTestCase): "id": quote.id, "value": "***beware*** of jealousy..." } - do_update_user_custom_profile_data(iago, [update_dict]) + do_update_user_custom_profile_data_if_changed(iago, [update_dict]) iago_profile_quote = self.example_user("iago").profile_data[-1] value = iago_profile_quote["value"] @@ -606,12 +606,12 @@ class UpdateCustomProfileFieldTest(CustomProfileFieldTestCase): field = CustomProfileField.objects.get(name="Mentor", realm=realm) data = [{'id': field.id, 'value': [self.example_user("aaron").id]}] # type: List[Dict[str, Union[int, str, List[int]]]] - do_update_user_custom_profile_data(iago, data) + do_update_user_custom_profile_data_if_changed(iago, data) with mock.patch("zerver.lib.actions.notify_user_update_custom_profile_data") as mock_notify: # Attempting to "update" the field value, when it wouldn't actually change, # if always_notify is disabled, shouldn't trigger notify. - do_update_user_custom_profile_data(iago, data) + do_update_user_custom_profile_data_if_changed(iago, data) mock_notify.assert_not_called() class ListCustomProfileFieldTest(CustomProfileFieldTestCase): diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index a9facdefdc..bce0f3d1db 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -102,7 +102,7 @@ from zerver.lib.actions import ( bulk_add_members_to_user_group, remove_members_from_user_group, check_delete_user_group, - do_update_user_custom_profile_data, + do_update_user_custom_profile_data_if_changed, ) from zerver.lib.events import ( apply_events, @@ -1159,7 +1159,7 @@ class EventsRegisterTest(ZulipTestCase): "id": field_id, "value": "New value", } - events = self.do_test(lambda: do_update_user_custom_profile_data(self.user_profile, [field])) + events = self.do_test(lambda: do_update_user_custom_profile_data_if_changed(self.user_profile, [field])) error = schema_checker_with_rendered_value('events[0]', events[0]) self.assert_on_error(error) @@ -1170,7 +1170,7 @@ class EventsRegisterTest(ZulipTestCase): "id": field_id, "value": [self.example_user("ZOE").id], } - events = self.do_test(lambda: do_update_user_custom_profile_data(self.user_profile, [field])) + events = self.do_test(lambda: do_update_user_custom_profile_data_if_changed(self.user_profile, [field])) error = schema_checker_basic('events[0]', events[0]) self.assert_on_error(error) diff --git a/zerver/views/custom_profile_fields.py b/zerver/views/custom_profile_fields.py index c611e60d1b..fa416040ef 100644 --- a/zerver/views/custom_profile_fields.py +++ b/zerver/views/custom_profile_fields.py @@ -10,7 +10,7 @@ from zerver.lib.request import has_request_variables, REQ from zerver.lib.actions import (try_add_realm_custom_profile_field, do_remove_realm_custom_profile_field, try_update_realm_custom_profile_field, - do_update_user_custom_profile_data, + do_update_user_custom_profile_data_if_changed, try_reorder_realm_custom_profile_fields, try_add_realm_default_custom_profile_field, check_remove_custom_profile_field_value) @@ -176,6 +176,6 @@ def update_user_custom_profile_data( check_dict([('id', check_int)])))) -> HttpResponse: validate_user_custom_profile_data(user_profile.realm.id, data) - do_update_user_custom_profile_data(user_profile, data) + do_update_user_custom_profile_data_if_changed(user_profile, data) # We need to call this explicitly otherwise constraints are not check return json_success() diff --git a/zerver/views/users.py b/zerver/views/users.py index b4b780a6c3..cea09b8023 100644 --- a/zerver/views/users.py +++ b/zerver/views/users.py @@ -15,7 +15,7 @@ from zerver.lib.actions import do_change_avatar_fields, do_change_bot_owner, \ do_create_user, do_deactivate_user, do_reactivate_user, do_regenerate_api_key, \ check_change_full_name, notify_created_bot, do_update_outgoing_webhook_service, \ do_update_bot_config_data, check_change_bot_full_name, do_change_is_guest, \ - do_update_user_custom_profile_data, check_remove_custom_profile_field_value + do_update_user_custom_profile_data_if_changed, check_remove_custom_profile_field_value from zerver.lib.avatar import avatar_url, get_gravatar_url, get_avatar_field from zerver.lib.bot_config import set_bot_config from zerver.lib.exceptions import CannotDeactivateLastUserError @@ -119,7 +119,7 @@ def update_user_backend(request: HttpRequest, user_profile: UserProfile, user_id else: clean_profile_data.append(entry) validate_user_custom_profile_data(target.realm.id, clean_profile_data) - do_update_user_custom_profile_data(target, clean_profile_data) + do_update_user_custom_profile_data_if_changed(target, clean_profile_data) return json_success() diff --git a/zilencer/management/commands/populate_db.py b/zilencer/management/commands/populate_db.py index f6161a6034..466a0a881f 100644 --- a/zilencer/management/commands/populate_db.py +++ b/zilencer/management/commands/populate_db.py @@ -14,7 +14,7 @@ from django.utils.timezone import now as timezone_now from django.utils.timezone import timedelta as timezone_timedelta from zerver.lib.actions import STREAM_ASSIGNMENT_COLORS, check_add_realm_emoji, \ - do_change_is_admin, do_send_messages, do_update_user_custom_profile_data, \ + do_change_is_admin, do_send_messages, do_update_user_custom_profile_data_if_changed, \ try_add_realm_custom_profile_field, try_add_realm_default_custom_profile_field from zerver.lib.bulk_create import bulk_create_streams, bulk_create_users from zerver.lib.cache import cache_set @@ -364,7 +364,7 @@ class Command(BaseCommand): # Fill in values for Iago and Hamlet hamlet = get_user("hamlet@zulip.com", zulip_realm) - do_update_user_custom_profile_data(iago, [ + do_update_user_custom_profile_data_if_changed(iago, [ {"id": phone_number.id, "value": "+1-234-567-8901"}, {"id": biography.id, "value": "Betrayer of Othello."}, {"id": favorite_food.id, "value": "Apples"}, @@ -374,7 +374,7 @@ class Command(BaseCommand): {"id": mentor.id, "value": [hamlet.id]}, {"id": github_profile.id, "value": 'zulip'}, ]) - do_update_user_custom_profile_data(hamlet, [ + do_update_user_custom_profile_data_if_changed(hamlet, [ {"id": phone_number.id, "value": "+0-11-23-456-7890"}, { "id": biography.id, diff --git a/zproject/backends.py b/zproject/backends.py index ee5d76e1e5..f4191e823c 100644 --- a/zproject/backends.py +++ b/zproject/backends.py @@ -38,7 +38,7 @@ from social_core.pipeline.partial import partial from social_core.exceptions import AuthFailed, SocialAuthBaseException from zerver.lib.actions import do_create_user, do_reactivate_user, do_deactivate_user, \ - do_update_user_custom_profile_data, validate_email_for_realm + do_update_user_custom_profile_data_if_changed, validate_email_for_realm from zerver.lib.avatar import is_avatar_new, avatar_url from zerver.lib.avatar_hash import user_avatar_content_hash from zerver.lib.dev_ldap_directory import init_fakeldap @@ -426,7 +426,7 @@ class ZulipLDAPAuthBackendBase(ZulipAuthMixin, LDAPBackend): 'id': field.id, 'value': value, }) - do_update_user_custom_profile_data(user_profile, profile_data) + do_update_user_custom_profile_data_if_changed(user_profile, profile_data) def get_or_build_user(self, username: str, ldap_user: _LDAPUser) -> Tuple[UserProfile, bool]: