Commit 4797a74e by cahrens Committed by Andy Armstrong

Emit events for language proficiencies from the update_account_settings method.

parent 97c43ada
...@@ -1671,6 +1671,13 @@ class EntranceExamConfiguration(models.Model): ...@@ -1671,6 +1671,13 @@ class EntranceExamConfiguration(models.Model):
class LanguageProficiency(models.Model): class LanguageProficiency(models.Model):
""" """
Represents a user's language proficiency. Represents a user's language proficiency.
Note that we have not found a way to emit analytics change events by using signals directly on this
model or on UserProfile. Therefore if you are changing LanguageProficiency values, it is important
to go through the accounts API (AccountsView) defined in
/edx-platform/openedx/core/djangoapps/user_api/accounts/views.py or the AccountLegacyProfileSerializer
in /edx-platform/openedx/core/djangoapps/user_api/accounts/serializers.py so that the events are
emitted.
""" """
class Meta: class Meta:
unique_together = (('code', 'user_profile'),) unique_together = (('code', 'user_profile'),)
......
...@@ -46,14 +46,14 @@ class AccountSettingsTestMixin(EventsTestMixin, WebAppTest): ...@@ -46,14 +46,14 @@ class AccountSettingsTestMixin(EventsTestMixin, WebAppTest):
self.USER_SETTINGS_CHANGED_EVENT_NAME, self.start_time, self.user_id, num_times, setting=setting self.USER_SETTINGS_CHANGED_EVENT_NAME, self.start_time, self.user_id, num_times, setting=setting
) )
def verify_settings_changed_events(self, events): def verify_settings_changed_events(self, events, table=None):
""" """
Verify a particular set of account settings change events were fired. Verify a particular set of account settings change events were fired.
""" """
expected_referers = [self.ACCOUNT_SETTINGS_REFERER] * len(events) expected_referers = [self.ACCOUNT_SETTINGS_REFERER] * len(events)
for event in events: for event in events:
event[u'user_id'] = long(self.user_id) event[u'user_id'] = long(self.user_id)
event[u'table'] = u"auth_userprofile" event[u'table'] = u"auth_userprofile" if table is None else table
self.verify_events_of_type(self.USER_SETTINGS_CHANGED_EVENT_NAME, events, expected_referers=expected_referers) self.verify_events_of_type(self.USER_SETTINGS_CHANGED_EVENT_NAME, events, expected_referers=expected_referers)
...@@ -424,6 +424,20 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, WebAppTest): ...@@ -424,6 +424,20 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, WebAppTest):
[u'Pushto', u''], [u'Pushto', u''],
) )
self.verify_settings_changed_events(
[{
u"setting": u"language_proficiencies",
u"old": [],
u"new": [{u"code": u"ps"}],
},
{
u"setting": u"language_proficiencies",
u"old": [{u"code": u"ps"}],
u"new": [],
}],
table=u"student_languageproficiency"
)
def test_connected_accounts(self): def test_connected_accounts(self):
""" """
Test that fields for third party auth providers exist. Test that fields for third party auth providers exist.
......
...@@ -6,7 +6,7 @@ from django.core.exceptions import ObjectDoesNotExist ...@@ -6,7 +6,7 @@ from django.core.exceptions import ObjectDoesNotExist
from django.conf import settings from django.conf import settings
from django.core.validators import validate_email, validate_slug, ValidationError from django.core.validators import validate_email, validate_slug, ValidationError
from student.models import User, UserProfile, Registration from student.models import User, UserProfile, Registration, USER_SETTINGS_CHANGED_EVENT_NAME
from student import views as student_views from student import views as student_views
from ..errors import ( from ..errors import (
...@@ -24,6 +24,7 @@ from . import ( ...@@ -24,6 +24,7 @@ from . import (
USERNAME_MIN_LENGTH, USERNAME_MAX_LENGTH USERNAME_MIN_LENGTH, USERNAME_MAX_LENGTH
) )
from .serializers import AccountLegacyProfileSerializer, AccountUserSerializer from .serializers import AccountLegacyProfileSerializer, AccountUserSerializer
from eventtracking import tracker
@intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError]) @intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError])
...@@ -187,9 +188,28 @@ def update_account_settings(requesting_user, update, username=None): ...@@ -187,9 +188,28 @@ def update_account_settings(requesting_user, update, username=None):
try: try:
# If everything validated, go ahead and save the serializers. # If everything validated, go ahead and save the serializers.
# We have not found a way using signals to get the language proficiency changes (grouped by user).
# As a workaround, store old and new values here and emit them after save is complete.
if "language_proficiencies" in update:
old_language_proficiencies = legacy_profile_serializer.data["language_proficiencies"]
for serializer in user_serializer, legacy_profile_serializer: for serializer in user_serializer, legacy_profile_serializer:
serializer.save() serializer.save()
if "language_proficiencies" in update:
new_language_proficiencies = legacy_profile_serializer.data["language_proficiencies"]
tracker.emit(
USER_SETTINGS_CHANGED_EVENT_NAME,
{
"setting": "language_proficiencies",
"old": old_language_proficiencies,
"new": new_language_proficiencies,
"user_id": existing_user.id,
"table": existing_user_profile.language_proficiencies.model._meta.db_table,
}
)
# If the name was changed, store information about the change operation. This is outside of the # If the name was changed, store information about the change operation. This is outside of the
# serializer so that we can store who requested the change. # serializer so that we can store who requested the change.
if old_name: if old_name:
......
...@@ -15,7 +15,7 @@ from student.tests.factories import UserFactory ...@@ -15,7 +15,7 @@ from student.tests.factories import UserFactory
from django.conf import settings from django.conf import settings
from django.contrib.auth.models import User from django.contrib.auth.models import User
from django.core import mail from django.core import mail
from student.models import PendingEmailChange from student.models import PendingEmailChange, USER_SETTINGS_CHANGED_EVENT_NAME
from ...errors import ( from ...errors import (
UserNotFound, UserNotAuthorized, AccountUpdateError, AccountValidationError, UserNotFound, UserNotAuthorized, AccountUpdateError, AccountValidationError,
AccountUserAlreadyExists, AccountUsernameInvalid, AccountEmailInvalid, AccountPasswordInvalid, AccountRequestError AccountUserAlreadyExists, AccountUsernameInvalid, AccountEmailInvalid, AccountPasswordInvalid, AccountRequestError
...@@ -24,6 +24,7 @@ from ..api import ( ...@@ -24,6 +24,7 @@ from ..api import (
get_account_settings, update_account_settings, create_account, activate_account, request_password_change get_account_settings, update_account_settings, create_account, activate_account, request_password_change
) )
from .. import USERNAME_MAX_LENGTH, EMAIL_MAX_LENGTH, PASSWORD_MAX_LENGTH from .. import USERNAME_MAX_LENGTH, EMAIL_MAX_LENGTH, PASSWORD_MAX_LENGTH
from util.testing import EventTestMixin
def mock_render_to_string(template_name, context): def mock_render_to_string(template_name, context):
...@@ -32,7 +33,7 @@ def mock_render_to_string(template_name, context): ...@@ -32,7 +33,7 @@ def mock_render_to_string(template_name, context):
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS') @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS')
class TestAccountApi(TestCase): class TestAccountApi(EventTestMixin, TestCase):
""" """
These tests specifically cover the parts of the API methods that are not covered by test_views.py. These tests specifically cover the parts of the API methods that are not covered by test_views.py.
This includes the specific types of error raised, and default behavior when optional arguments This includes the specific types of error raised, and default behavior when optional arguments
...@@ -41,7 +42,7 @@ class TestAccountApi(TestCase): ...@@ -41,7 +42,7 @@ class TestAccountApi(TestCase):
password = "test" password = "test"
def setUp(self): def setUp(self):
super(TestAccountApi, self).setUp() super(TestAccountApi, self).setUp('openedx.core.djangoapps.user_api.accounts.api.tracker')
self.user = UserFactory.create(password=self.password) self.user = UserFactory.create(password=self.password)
self.different_user = UserFactory.create(password=self.password) self.different_user = UserFactory.create(password=self.password)
self.staff_user = UserFactory(is_staff=True, password=self.password) self.staff_user = UserFactory(is_staff=True, password=self.password)
...@@ -191,6 +192,31 @@ class TestAccountApi(TestCase): ...@@ -191,6 +192,31 @@ class TestAccountApi(TestCase):
pending_change = PendingEmailChange.objects.filter(user=self.user) pending_change = PendingEmailChange.objects.filter(user=self.user)
self.assertEqual(0, len(pending_change)) self.assertEqual(0, len(pending_change))
def test_language_proficiency_eventing(self):
"""
Test that eventing of language proficiencies, which happens update_account_settings method, behaves correctly.
"""
def verify_event_emitted(new_value, old_value):
update_account_settings(self.user, {"language_proficiencies": new_value})
self.assert_event_emitted(
USER_SETTINGS_CHANGED_EVENT_NAME, setting="language_proficiencies",
old=old_value, new=new_value, user_id=self.user.id, table="student_languageproficiency"
)
self.reset_tracker()
# First, test that no event is emitted if language_proficiencies is not included.
update_account_settings(self.user, {"year_of_birth": 900})
account_settings = get_account_settings(self.user)
self.assertEqual(900, account_settings["year_of_birth"])
self.assert_no_events_were_emitted()
# New change language_proficiencies and verify events are fired.
verify_event_emitted([{"code": "en"}], [])
verify_event_emitted([{"code": "en"}, {"code": "fr"}], [{"code": "en"}])
# Note that events are fired even if there has been no actual change.
verify_event_emitted([{"code": "en"}, {"code": "fr"}], [{"code": "en"}, {"code": "fr"}])
verify_event_emitted([], [{"code": "en"}, {"code": "fr"}])
@patch('openedx.core.djangoapps.user_api.accounts.image_helpers._PROFILE_IMAGE_SIZES', [50, 10]) @patch('openedx.core.djangoapps.user_api.accounts.image_helpers._PROFILE_IMAGE_SIZES', [50, 10])
@patch.dict( @patch.dict(
......
...@@ -56,7 +56,7 @@ def intercept_errors(api_error, ignore_errors=None): ...@@ -56,7 +56,7 @@ def intercept_errors(api_error, ignore_errors=None):
func_name=func.func_name, func_name=func.func_name,
args=args, args=args,
kwargs=kwargs, kwargs=kwargs,
exception=repr(ex) exception=ex.developer_message if hasattr(ex, 'developer_message') else repr(ex)
) )
LOGGER.warning(msg) LOGGER.warning(msg)
raise raise
...@@ -70,7 +70,7 @@ def intercept_errors(api_error, ignore_errors=None): ...@@ -70,7 +70,7 @@ def intercept_errors(api_error, ignore_errors=None):
func_name=func.func_name, func_name=func.func_name,
args=args, args=args,
kwargs=kwargs, kwargs=kwargs,
exception=repr(ex) exception=ex.developer_message if hasattr(ex, 'developer_message') else repr(ex)
) )
LOGGER.exception(msg) LOGGER.exception(msg)
raise api_error(msg) raise api_error(msg)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment