diff --git a/common/djangoapps/util/model_utils.py b/common/djangoapps/util/model_utils.py index 66e9237498..9bcda3f3ef 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,13 @@ 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)) - emit_setting_changed_event(user, db_table, field_name, old_value, new_value) + clean_changed_fields[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 @@ -127,33 +131,35 @@ def truncate_fields(old_value, new_value): return {'old': serialized_old_value, 'new': serialized_new_value, 'truncated': truncated_values} -def emit_setting_changed_event(user, db_table, setting_name, old_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. - setting_name (str): the name of the setting being changed. - old_value (object): the value before the change. - new_value (object): the new value being saved. + changed_fields: all changed settings, with both their old and new values Returns: None """ - truncated_fields = truncate_fields(old_value, new_value) + for (setting_name, (old_value, new_value)) in changed_fields.items(): + truncated_fields = truncate_fields(old_value, new_value) - truncated_fields['setting'] = setting_name - truncated_fields['user_id'] = user.id - truncated_fields['table'] = db_table + truncated_fields['setting'] = setting_name + truncated_fields['user_id'] = user.id + truncated_fields['table'] = db_table - tracker.emit( - USER_SETTINGS_CHANGED_EVENT_NAME, - truncated_fields - ) + tracker.emit( + USER_SETTINGS_CHANGED_EVENT_NAME, + truncated_fields + ) + + # Announce field change + USER_FIELD_CHANGED.send(sender=None, user=user, table=db_table, setting=setting_name, + old_value=old_value, new_value=new_value) # Announce field change - USER_FIELD_CHANGED.send(sender=None, user=user, table=db_table, setting=setting_name, - old_value=old_value, new_value=new_value) + USER_FIELDS_CHANGED.send(sender=None, user=user, table=db_table, changed_fields=changed_fields) def _get_truncated_setting_value(value, max_length=None): diff --git a/lms/djangoapps/email_marketing/signals.py b/lms/djangoapps/email_marketing/signals.py index f430ccd6a2..122669be20 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 @@ -39,12 +42,15 @@ CHANGED_FIELDNAMES = ['username', 'is_active', 'name', 'gender', 'education', 'age', 'level_of_education', 'year_of_birth', 'country', LANGUAGE_KEY] +# TODO: Remove in AA-607 WAFFLE_NAMESPACE = 'sailthru' WAFFLE_SWITCHES = LegacyWaffleSwitchNamespace(name=WAFFLE_NAMESPACE) +# TODO: Remove in AA-607 SAILTHRU_AUDIT_PURCHASE_ENABLED = 'audit_purchase_enabled' +# TODO: Remove in AA-607 @receiver(SAILTHRU_AUDIT_PURCHASE) def update_sailthru(sender, user, mode, course_id, **kwargs): # pylint: disable=unused-argument """ @@ -61,6 +67,7 @@ def update_sailthru(sender, user, mode, course_id, **kwargs): # pylint: disable update_course_enrollment.delay(email, course_id, mode, site=_get_current_site()) +# TODO: Remove in AA-607 @receiver(CREATE_LOGON_COOKIE) def add_email_marketing_cookies(sender, response=None, user=None, **kwargs): # pylint: disable=unused-argument @@ -101,7 +108,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: @@ -128,6 +135,7 @@ def add_email_marketing_cookies(sender, response=None, user=None, return response +# TODO: Remove in AA-607 @receiver(REGISTER_USER) def email_marketing_register_user(sender, user, registration, **kwargs): # pylint: disable=unused-argument @@ -153,6 +161,7 @@ def email_marketing_register_user(sender, user, registration, site=_get_current_site(), new_user=True) +# TODO: Remove in AA-607 @receiver(USER_FIELD_CHANGED) def email_marketing_user_field_changed(sender, user=None, table=None, setting=None, old_value=None, new_value=None, @@ -221,6 +230,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/lms/djangoapps/email_marketing/tasks.py b/lms/djangoapps/email_marketing/tasks.py index c79e567795..9d89181796 100644 --- a/lms/djangoapps/email_marketing/tasks.py +++ b/lms/djangoapps/email_marketing/tasks.py @@ -21,6 +21,7 @@ log = logging.getLogger(__name__) SAILTHRU_LIST_CACHE_KEY = "email.marketing.cache" +# TODO: Remove in AA-607 @shared_task(bind=True) @set_code_owner_attribute def get_email_cookies_via_sailthru(self, user_email, post_parms): @@ -62,6 +63,7 @@ def get_email_cookies_via_sailthru(self, user_email, post_parms): return None +# TODO: Remove in AA-607 @shared_task(bind=True, default_retry_delay=3600, max_retries=24) @set_code_owner_attribute def update_user(self, sailthru_vars, email, site=None, new_user=False, activation=False): @@ -145,6 +147,7 @@ def is_default_site(site): return not site or site.get('id') == settings.SITE_ID +# TODO: Remove in AA-607 @shared_task(bind=True, default_retry_delay=3600, max_retries=24) @set_code_owner_attribute def update_user_email(self, new_email, old_email): @@ -200,6 +203,7 @@ def _create_email_user_param(sailthru_vars, sailthru_client, email, new_user, em return sailthru_user +# TODO: Remove in AA-607 def _get_or_create_user_list_for_site(sailthru_client, site=None, default_list_name=None): """ Get the user list name from cache if exists else create one and return the name, @@ -218,6 +222,7 @@ def _get_or_create_user_list_for_site(sailthru_client, site=None, default_list_n return list_name if sailthru_list else default_list_name +# TODO: Remove in AA-607 def _get_or_create_user_list(sailthru_client, list_name): """ Get list from sailthru and return if list_name exists else create a new one @@ -247,6 +252,7 @@ def _get_or_create_user_list(sailthru_client, list_name): return sailthru_list +# TODO: Remove in AA-607 def _get_list_from_email_marketing_provider(sailthru_client): """ Get sailthru list @@ -293,6 +299,7 @@ def _create_user_list(sailthru_client, list_name): return True +# TODO: Remove in AA-607 def _retryable_sailthru_error(error): """ Return True if error should be retried. @@ -306,6 +313,7 @@ def _retryable_sailthru_error(error): return code == 9 or code == 43 +# TODO: Remove in AA-607 @shared_task(bind=True) @set_code_owner_attribute def update_course_enrollment(self, email, course_key, mode, site=None): @@ -360,6 +368,7 @@ def build_course_url(course_key): course_key=six.text_type(course_key)) +# TODO: Remove in AA-607 def update_unenrolled_list(sailthru_client, email, course_url, unenroll): """Maintain a list of courses the user has unenrolled from in the Sailthru user record Arguments: @@ -414,12 +423,14 @@ def update_unenrolled_list(sailthru_client, email, course_url, unenroll): return False +# TODO: Remove in AA-607 def schedule_retry(self, config): """Schedule a retry""" raise self.retry(countdown=config.sailthru_retry_interval, max_retries=config.sailthru_max_retries) +# TODO: Remove in AA-607 def _get_course_content(course_id, course_url, sailthru_client, config): """Get course information using the Sailthru content api or from cache. If there is an error, just return with an empty response. @@ -451,6 +462,7 @@ def _get_course_content(course_id, course_url, sailthru_client, config): return response +# TODO: Remove in AA-607 def _build_purchase_item(course_id, course_url, cost_in_cents, mode, course_data): """Build and return Sailthru purchase item object""" @@ -478,6 +490,7 @@ def _build_purchase_item(course_id, course_url, cost_in_cents, mode, course_data return item +# TODO: Remove in AA-607 def _record_purchase(sailthru_client, email, item, options): """ Record a purchase in Sailthru diff --git a/lms/djangoapps/instructor/tests/test_tools.py b/lms/djangoapps/instructor/tests/test_tools.py index 5c653fae22..f6606a2716 100644 --- a/lms/djangoapps/instructor/tests/test_tools.py +++ b/lms/djangoapps/instructor/tests/test_tools.py @@ -333,13 +333,20 @@ class TestDataDumps(ModuleStoreTestCase): self.week1.display_name) assert ( report['header'] == ["Username", "Full Name", "Extended Due Date"]) - assert (report['data'] == [ - {"Username": self.user1.username, - "Full Name": self.user1.profile.name, - "Extended Due Date": "2013-12-25 00:00"}, - {"Username": self.user2.username, - "Full Name": self.user2.profile.name, - "Extended Due Date": "2013-12-25 00:00"}]) + self.assertCountEqual( + report['data'], + [ + { + "Username": self.user1.username, + "Full Name": self.user1.profile.name, + "Extended Due Date": "2013-12-25 00:00" + }, { + "Username": self.user2.username, + "Full Name": self.user2.profile.name, + "Extended Due Date": "2013-12-25 00:00" + } + ] + ) def test_dump_student_extensions(self): extended = datetime.datetime(2013, 12, 25, 0, 0, tzinfo=UTC) diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index 276f16e4f2..57f2929024 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_settings_changed_event from common.djangoapps.util.password_policy_validators import validate_password from openedx.core.djangoapps.user_api import accounts, errors, helpers @@ -270,12 +270,15 @@ def _update_preferences_if_needed(data, requesting_user, user): def _notify_language_proficiencies_update_if_needed(data, user, user_profile, old_language_proficiencies): if "language_proficiencies" in data: new_language_proficiencies = data["language_proficiencies"] - emit_setting_changed_event( + emit_settings_changed_event( user=user, db_table=user_profile.language_proficiencies.model._meta.db_table, - setting_name="language_proficiencies", - old_value=old_language_proficiencies, - new_value=new_language_proficiencies, + changed_fields={ + "language_proficiencies": ( + old_language_proficiencies, + new_language_proficiencies, + ) + } ) diff --git a/openedx/core/djangoapps/user_api/models.py b/openedx/core/djangoapps/user_api/models.py index 64f45cbdcd..ae402f10d4 100644 --- a/openedx/core/djangoapps/user_api/models.py +++ b/openedx/core/djangoapps/user_api/models.py @@ -29,7 +29,10 @@ 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_settings_changed_event, + get_changed_fields_dict, +) class RetirementStateError(Exception): @@ -118,9 +121,14 @@ def post_save_callback(sender, **kwargs): """ user_preference = kwargs["instance"] - emit_setting_changed_event( - 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, # pylint: disable=protected-access + user_preference.value + ) + } ) user_preference._old_value = None # pylint: disable=protected-access @@ -131,8 +139,10 @@ def post_delete_callback(sender, **kwargs): Event changes to user preferences. """ user_preference = kwargs["instance"] - 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) + } )