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.
This commit is contained in:
Mateusz Mandera 2019-10-01 04:22:50 +02:00 committed by Tim Abbott
parent d66cbd2832
commit 4166c901ef
8 changed files with 22 additions and 21 deletions

View File

@ -5458,8 +5458,9 @@ def notify_user_update_custom_profile_data(user_profile: UserProfile,
event = dict(type="realm_user", op="update", person=payload) event = dict(type="realm_user", op="update", person=payload)
send_event(user_profile.realm, event, active_user_ids(user_profile.realm.id)) send_event(user_profile.realm, event, active_user_ids(user_profile.realm.id))
def do_update_user_custom_profile_data(user_profile: UserProfile, def do_update_user_custom_profile_data_if_changed(user_profile: UserProfile,
data: List[Dict[str, Union[int, str, List[int]]]]) -> None: data: List[Dict[str, Union[int, str, List[int]]]]
) -> None:
with transaction.atomic(): with transaction.atomic():
for field in data: for field in data:
field_value, created = CustomProfileFieldValue.objects.get_or_create( field_value, created = CustomProfileFieldValue.objects.get_or_create(

View File

@ -2915,7 +2915,7 @@ class TestZulipLDAPUserPopulator(ZulipLDAPTestCase):
with self.settings(AUTH_LDAP_USER_ATTR_MAP={'full_name': 'cn', with self.settings(AUTH_LDAP_USER_ATTR_MAP={'full_name': 'cn',
'custom_profile_field__birthday': 'birthDate', 'custom_profile_field__birthday': 'birthDate',
'custom_profile_field__phone_number': 'phoneNumber'}): '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')) self.perform_ldap_sync(self.example_user('hamlet'))
f.assert_called_once_with(*expected_call_args) f.assert_called_once_with(*expected_call_args)

View File

@ -3,7 +3,7 @@
from typing import Union, List, Dict, Any from typing import Union, List, Dict, Any
from zerver.lib.actions import try_add_realm_custom_profile_field, \ 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 try_reorder_realm_custom_profile_fields
from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_classes import ZulipTestCase
from zerver.lib.bugdown import convert as bugdown_convert from zerver.lib.bugdown import convert as bugdown_convert
@ -311,7 +311,7 @@ class DeleteCustomProfileFieldTest(CustomProfileFieldTestCase):
field = CustomProfileField.objects.get(name="Mentor", realm=realm) field = CustomProfileField.objects.get(name="Mentor", realm=realm)
data = [{'id': field.id, data = [{'id': field.id,
'value': [self.example_user("aaron").id]}] # type: List[Dict[str, Union[int, str, List[int]]]] '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) iago_value = CustomProfileFieldValue.objects.get(user_profile=iago, field=field)
converter = field.FIELD_CONVERTERS[field.field_type] converter = field.FIELD_CONVERTERS[field.field_type]
@ -333,7 +333,7 @@ class DeleteCustomProfileFieldTest(CustomProfileFieldTestCase):
realm = user_profile.realm realm = user_profile.realm
field = CustomProfileField.objects.get(name="Phone number", realm=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]]]] 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.assertTrue(self.custom_field_exists_in_realm(field.id))
self.assertEqual(user_profile.customprofilefieldvalue_set.count(), self.original_count) self.assertEqual(user_profile.customprofilefieldvalue_set.count(), self.original_count)
@ -588,7 +588,7 @@ class UpdateCustomProfileFieldTest(CustomProfileFieldTestCase):
"id": quote.id, "id": quote.id,
"value": "***beware*** of jealousy..." "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] iago_profile_quote = self.example_user("iago").profile_data[-1]
value = iago_profile_quote["value"] value = iago_profile_quote["value"]
@ -606,12 +606,12 @@ class UpdateCustomProfileFieldTest(CustomProfileFieldTestCase):
field = CustomProfileField.objects.get(name="Mentor", realm=realm) field = CustomProfileField.objects.get(name="Mentor", realm=realm)
data = [{'id': field.id, data = [{'id': field.id,
'value': [self.example_user("aaron").id]}] # type: List[Dict[str, Union[int, str, List[int]]]] '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: 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, # Attempting to "update" the field value, when it wouldn't actually change,
# if always_notify is disabled, shouldn't trigger notify. # 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() mock_notify.assert_not_called()
class ListCustomProfileFieldTest(CustomProfileFieldTestCase): class ListCustomProfileFieldTest(CustomProfileFieldTestCase):

View File

@ -102,7 +102,7 @@ from zerver.lib.actions import (
bulk_add_members_to_user_group, bulk_add_members_to_user_group,
remove_members_from_user_group, remove_members_from_user_group,
check_delete_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 ( from zerver.lib.events import (
apply_events, apply_events,
@ -1159,7 +1159,7 @@ class EventsRegisterTest(ZulipTestCase):
"id": field_id, "id": field_id,
"value": "New value", "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]) error = schema_checker_with_rendered_value('events[0]', events[0])
self.assert_on_error(error) self.assert_on_error(error)
@ -1170,7 +1170,7 @@ class EventsRegisterTest(ZulipTestCase):
"id": field_id, "id": field_id,
"value": [self.example_user("ZOE").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]) error = schema_checker_basic('events[0]', events[0])
self.assert_on_error(error) self.assert_on_error(error)

View File

@ -10,7 +10,7 @@ from zerver.lib.request import has_request_variables, REQ
from zerver.lib.actions import (try_add_realm_custom_profile_field, from zerver.lib.actions import (try_add_realm_custom_profile_field,
do_remove_realm_custom_profile_field, do_remove_realm_custom_profile_field,
try_update_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_reorder_realm_custom_profile_fields,
try_add_realm_default_custom_profile_field, try_add_realm_default_custom_profile_field,
check_remove_custom_profile_field_value) check_remove_custom_profile_field_value)
@ -176,6 +176,6 @@ def update_user_custom_profile_data(
check_dict([('id', check_int)])))) -> HttpResponse: check_dict([('id', check_int)])))) -> HttpResponse:
validate_user_custom_profile_data(user_profile.realm.id, data) 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 # We need to call this explicitly otherwise constraints are not check
return json_success() return json_success()

View File

@ -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, \ 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, \ 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_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.avatar import avatar_url, get_gravatar_url, get_avatar_field
from zerver.lib.bot_config import set_bot_config from zerver.lib.bot_config import set_bot_config
from zerver.lib.exceptions import CannotDeactivateLastUserError from zerver.lib.exceptions import CannotDeactivateLastUserError
@ -119,7 +119,7 @@ def update_user_backend(request: HttpRequest, user_profile: UserProfile, user_id
else: else:
clean_profile_data.append(entry) clean_profile_data.append(entry)
validate_user_custom_profile_data(target.realm.id, clean_profile_data) 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() return json_success()

View File

@ -14,7 +14,7 @@ from django.utils.timezone import now as timezone_now
from django.utils.timezone import timedelta as timezone_timedelta from django.utils.timezone import timedelta as timezone_timedelta
from zerver.lib.actions import STREAM_ASSIGNMENT_COLORS, check_add_realm_emoji, \ 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 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.bulk_create import bulk_create_streams, bulk_create_users
from zerver.lib.cache import cache_set from zerver.lib.cache import cache_set
@ -364,7 +364,7 @@ class Command(BaseCommand):
# Fill in values for Iago and Hamlet # Fill in values for Iago and Hamlet
hamlet = get_user("hamlet@zulip.com", zulip_realm) 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": phone_number.id, "value": "+1-234-567-8901"},
{"id": biography.id, "value": "Betrayer of Othello."}, {"id": biography.id, "value": "Betrayer of Othello."},
{"id": favorite_food.id, "value": "Apples"}, {"id": favorite_food.id, "value": "Apples"},
@ -374,7 +374,7 @@ class Command(BaseCommand):
{"id": mentor.id, "value": [hamlet.id]}, {"id": mentor.id, "value": [hamlet.id]},
{"id": github_profile.id, "value": 'zulip'}, {"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": phone_number.id, "value": "+0-11-23-456-7890"},
{ {
"id": biography.id, "id": biography.id,

View File

@ -38,7 +38,7 @@ from social_core.pipeline.partial import partial
from social_core.exceptions import AuthFailed, SocialAuthBaseException from social_core.exceptions import AuthFailed, SocialAuthBaseException
from zerver.lib.actions import do_create_user, do_reactivate_user, do_deactivate_user, \ 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 import is_avatar_new, avatar_url
from zerver.lib.avatar_hash import user_avatar_content_hash from zerver.lib.avatar_hash import user_avatar_content_hash
from zerver.lib.dev_ldap_directory import init_fakeldap from zerver.lib.dev_ldap_directory import init_fakeldap
@ -426,7 +426,7 @@ class ZulipLDAPAuthBackendBase(ZulipAuthMixin, LDAPBackend):
'id': field.id, 'id': field.id,
'value': value, '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, def get_or_build_user(self, username: str,
ldap_user: _LDAPUser) -> Tuple[UserProfile, bool]: ldap_user: _LDAPUser) -> Tuple[UserProfile, bool]: