From fdac36ec13a0da25e86152c5b204d0585f27d0cf Mon Sep 17 00:00:00 2001 From: Calen Pennington <cale@edx.org> Date: Tue, 12 Jan 2021 11:27:53 -0500 Subject: [PATCH] Call segment.identify with any changed values when user profile fields change --- common/djangoapps/util/model_utils.py | 20 ++++++++ lms/djangoapps/email_marketing/signals.py | 48 +++++++++++++++++-- .../core/djangoapps/user_api/accounts/api.py | 7 ++- openedx/core/djangoapps/user_api/models.py | 16 ++++++- 4 files changed, 86 insertions(+), 5 deletions(-) diff --git a/common/djangoapps/util/model_utils.py b/common/djangoapps/util/model_utils.py index 66e92374986..815503097cd 100644 --- a/common/djangoapps/util/model_utils.py +++ b/common/djangoapps/util/model_utils.py @@ -2,6 +2,7 @@ Utilities for django models. """ +from typing import Dict, Any, Tuple import six from django.conf import settings @@ -13,6 +14,7 @@ from eventtracking import tracker USER_SETTINGS_CHANGED_EVENT_NAME = u'edx.user.settings.changed' # Used to signal a field value change USER_FIELD_CHANGED = Signal(providing_args=["user", "table", "setting", "old_value", "new_value"]) +USER_FIELDS_CHANGED = Signal(providing_args=["user", "table", "changed_values"]) def get_changed_fields_dict(instance, model_class): @@ -88,11 +90,14 @@ def emit_field_changed_events(instance, user, db_table, excluded_fields=None, hi excluded_fields = excluded_fields or [] hidden_fields = hidden_fields or [] changed_fields = getattr(instance, '_changed_fields', {}) + clean_changed_fields = {} for field_name in changed_fields: if field_name not in excluded_fields: old_value = clean_field(field_name, changed_fields[field_name]) new_value = clean_field(field_name, getattr(instance, field_name)) + clean_changed_fields[field_name] = (old_value, new_value) emit_setting_changed_event(user, db_table, field_name, old_value, new_value) + emit_settings_changed_event(user, db_table, clean_changed_fields) # Remove the now inaccurate _changed_fields attribute. if hasattr(instance, '_changed_fields'): del instance._changed_fields @@ -156,6 +161,21 @@ def emit_setting_changed_event(user, db_table, setting_name, old_value, new_valu old_value=old_value, new_value=new_value) +def emit_settings_changed_event(user, db_table, changed_fields: Dict[str, Tuple[Any, Any]]): + """Emits an event for a change in a setting. + + Args: + user (User): the user that this setting is associated with. + db_table (str): the name of the table that we're modifying. + changed_fields: all changed settings, with both their old and new values + + Returns: + None + """ + # Announce field change + USER_FIELDS_CHANGED.send(sender=None, user=user, table=db_table, changed_fields=changed_fields) + + def _get_truncated_setting_value(value, max_length=None): """ Returns the truncated form of a setting value. diff --git a/lms/djangoapps/email_marketing/signals.py b/lms/djangoapps/email_marketing/signals.py index f430ccd6a26..34cefc5fa5c 100644 --- a/lms/djangoapps/email_marketing/signals.py +++ b/lms/djangoapps/email_marketing/signals.py @@ -6,9 +6,10 @@ This module contains signals needed for email integration import datetime import logging from random import randint +from typing import Dict, Any, Optional, Tuple import crum -from celery.exceptions import TimeoutError +from celery.exceptions import TimeoutError as CeleryTimeoutError from django.conf import settings from django.dispatch import receiver from sailthru.sailthru_error import SailthruClientError @@ -28,7 +29,9 @@ from openedx.core.djangoapps.user_authn.cookies import CREATE_LOGON_COOKIE from openedx.core.djangoapps.user_authn.views.register import REGISTER_USER from common.djangoapps.student.helpers import does_user_profile_exist from common.djangoapps.student.signals import SAILTHRU_AUDIT_PURCHASE -from common.djangoapps.util.model_utils import USER_FIELD_CHANGED +from common.djangoapps.student.models import UserProfile +from common.djangoapps.track import segment +from common.djangoapps.util.model_utils import USER_FIELD_CHANGED, USER_FIELDS_CHANGED from .models import EmailMarketingConfiguration @@ -101,7 +104,7 @@ def add_email_marketing_cookies(sender, response=None, user=None, cookie = sailthru_response.result _log_sailthru_api_call_time(time_before_call) - except TimeoutError as exc: + except CeleryTimeoutError as exc: log.error(u"Timeout error while attempting to obtain cookie from Sailthru: %s", text_type(exc)) return response except SailthruClientError as exc: @@ -221,6 +224,45 @@ def email_marketing_user_field_changed(sender, user=None, table=None, setting=No update_user_email.delay(user.email, old_value) +@receiver(USER_FIELDS_CHANGED) +def email_marketing_user_fields_changed( + sender, # pylint: disable=unused-argument + user=None, + table=None, + changed_fields: Optional[Dict[str, Tuple[Any, Any]]] = None, + **kwargs +): + """ + Update a collection of user profile fields + + Args: + sender: Not used + user: The user object for the user being changed + table: The name of the table being updated + changed_fields: A mapping from changed field name to old and new values. + kwargs: Not used + """ + + fields = {field: new_value for (field, (old_value, new_value)) in changed_fields.items()} + # This mirrors the logic in openedx/core/djangoapps/user_authn/views/register.py:_track_user_registration + if table == 'auth_userprofile': + if 'gender' in fields and fields['gender']: + fields['gender'] = dict(UserProfile.GENDER_CHOICES)[fields['gender']] + if 'country' in fields: + fields['country'] = str(fields['country']) + if 'level_of_education' in fields and fields['level_of_education']: + fields['level_of_education'] = dict(UserProfile.LEVEL_OF_EDUCATION_CHOICES)[fields['level_of_education']] + if 'year_of_birth' in fields: + fields['yearOfBirth'] = fields.pop('year_of_birth') + if 'mailing_address' in fields: + fields['address'] = fields.pop('mailing_address') + + segment.identify( + user.id, + fields + ) + + def _create_sailthru_user_vars(user, profile, registration=None): """ Create sailthru user create/update vars from user + profile. diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index 276f16e4f25..fd71021b3ea 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -23,7 +23,7 @@ from common.djangoapps.student.models import ( email_exists_or_retired, username_exists_or_retired ) -from common.djangoapps.util.model_utils import emit_setting_changed_event +from common.djangoapps.util.model_utils import emit_setting_changed_event, emit_settings_changed_event from common.djangoapps.util.password_policy_validators import validate_password from openedx.core.djangoapps.user_api import accounts, errors, helpers @@ -277,6 +277,11 @@ def _notify_language_proficiencies_update_if_needed(data, user, user_profile, ol old_value=old_language_proficiencies, new_value=new_language_proficiencies, ) + emit_settings_changed_event( + user=user, + db_table=user_profile.language_proficiencies.model._meta.db_table, + changed_fields={"language_proficiencies": (old_language_proficiencies, new_language_proficiencies)}, + ) def _update_extended_profile_if_needed(data, user_profile): diff --git a/openedx/core/djangoapps/user_api/models.py b/openedx/core/djangoapps/user_api/models.py index 64f45cbdcd7..a3347bf2c34 100644 --- a/openedx/core/djangoapps/user_api/models.py +++ b/openedx/core/djangoapps/user_api/models.py @@ -29,7 +29,11 @@ from common.djangoapps.student.models import ( get_retired_email_by_email, get_retired_username_by_username ) -from common.djangoapps.util.model_utils import emit_setting_changed_event, get_changed_fields_dict +from common.djangoapps.util.model_utils import ( + emit_setting_changed_event, + get_changed_fields_dict, + emit_settings_changed_event, +) class RetirementStateError(Exception): @@ -122,6 +126,11 @@ def post_save_callback(sender, **kwargs): user_preference.user, sender._meta.db_table, user_preference.key, user_preference._old_value, user_preference.value # pylint: disable=protected-access ) + emit_settings_changed_event( + user_preference.user, + sender._meta.db_table, + {user_preference.key: (user_preference._old_value, user_preference.value)} # pylint: disable=protected-access + ) user_preference._old_value = None # pylint: disable=protected-access @@ -134,6 +143,11 @@ def post_delete_callback(sender, **kwargs): emit_setting_changed_event( user_preference.user, sender._meta.db_table, user_preference.key, user_preference.value, None ) + emit_settings_changed_event( + user_preference.user, + sender._meta.db_table, + {user_preference.key: (user_preference.value, None)} + ) class UserCourseTag(models.Model): -- GitLab