Skip to content

E
edx-platform
Commit 7742735f by Andy Armstrong
Options

Event on user preference updates 

TNL-1851
parent 1f3d2c24
Showing with 360 additions and 99 deletions
common/djangoapps/util/model_utils.py
......@@ -3,6 +3,7 @@ Utilities for django models.
"""
from eventtracking import tracker
from django.conf import settings
from django.core.exceptions import ObjectDoesNotExist
from django.db.models.fields.related import RelatedField
......@@ -46,9 +47,7 @@ def get_changed_fields_dict(instance, model_class):
def emit_field_changed_events(instance, user, event_name, db_table, excluded_fields=None, hidden_fields=None):
"""
For the given model instance, emit a setting changed event the fields that
have changed since the last save.
"""Emits an event for each field that has changed.
Note that this function expects that a `_changed_fields` dict has been set
as an attribute on `instance` (see `get_changed_fields_dict`.
......@@ -87,16 +86,61 @@ def emit_field_changed_events(instance, user, event_name, db_table, excluded_fie
changed_fields = getattr(instance, '_changed_fields', {})
for field_name in changed_fields:
if field_name not in excluded_fields:
tracker.emit(
event_name,
{
"setting": field_name,
'old': clean_field(field_name, changed_fields[field_name]),
'new': clean_field(field_name, getattr(instance, field_name)),
"user_id": user.id,
"table": db_table
}
)
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, event_name, db_table, field_name, old_value, new_value)
# Remove the now inaccurate _changed_fields attribute.
if getattr(instance, '_changed_fields', None):
if hasattr(instance, '_changed_fields'):
del instance._changed_fields
def emit_setting_changed_event(user, event_name, db_table, setting_name, old_value, new_value):
"""Emits an event for a change in a setting.
Args:
user (User): the user that this setting is associated with.
event_name (str): the name of the event to be emitted.
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.
Returns:
None
"""
# Compute the maximum value length so that two copies can fit into the maximum event size
# in addition to all the other fields recorded.
max_value_length = settings.TRACK_MAX_EVENT / 4
serialized_old_value, old_was_truncated = _get_truncated_setting_value(old_value, max_length=max_value_length)
serialized_new_value, new_was_truncated = _get_truncated_setting_value(new_value, max_length=max_value_length)
truncated_values = []
if old_was_truncated:
truncated_values.append("old")
if new_was_truncated:
truncated_values.append("new")
tracker.emit(
event_name,
{
"setting": setting_name,
"old": serialized_old_value,
"new": serialized_new_value,
"truncated": truncated_values,
"user_id": user.id,
"table": db_table,
}
)
def _get_truncated_setting_value(value, max_length=None):
"""
Returns the truncated form of a setting value.
Returns:
truncated_value (object): the possibly truncated version of the value.
was_truncated (bool): returns true if the serialized value was truncated.
"""
if isinstance(value, basestring) and max_length is not None and len(value) > max_length:
return value[0:max_length], True
else:
return value, False
common/test/acceptance/pages/lms/learner_profile.py
......@@ -201,11 +201,12 @@ class LearnerProfilePage(FieldsMixin, PageObject):
self.wait_for_field('image')
return self.q(css='.u-field-upload-button').visible
def upload_file(self, filename):
def upload_file(self, filename, wait_for_upload_button=True):
"""
Helper method to upload an image file.
"""
self.wait_for_element_visibility('.u-field-upload-button', "upload button is visible")
if wait_for_upload_button:
self.wait_for_element_visibility('.u-field-upload-button', "upload button is visible")
file_path = InstructorDashboardPage.get_asset_path(filename)
# make the elements visible.
......
common/test/acceptance/tests/lms/test_account_settings.py
......@@ -47,8 +47,10 @@ class AccountSettingsTestMixin(EventsTestMixin, WebAppTest):
"""
expected_referers = [self.ACCOUNT_SETTINGS_REFERER] * len(events)
for event in events:
event[u'user_id'] = long(user_id)
event[u'table'] = u"auth_userprofile" if table is None else table
event[u"user_id"] = long(user_id)
event[u"table"] = u"auth_userprofile" if table is None else table
event[u"truncated"] = []
self.verify_events_of_type(
username, self.USER_SETTINGS_CHANGED_EVENT_NAME, events,
expected_referers=expected_referers
......@@ -233,16 +235,18 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, WebAppTest):
self.verify_settings_changed_events(
self.username, self.user_id,
[{
u"setting": u"name",
u"old": self.username,
u"new": u"another name",
},
{
u"setting": u"name",
u"old": u'another name',
u"new": self.username,
}]
[
{
u"setting": u"name",
u"old": self.username,
u"new": u"another name",
},
{
u"setting": u"name",
u"old": u'another name',
u"new": self.username,
}
]
)
def test_email_field(self):
......@@ -339,16 +343,18 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, WebAppTest):
)
self.verify_settings_changed_events(
self.username, self.user_id,
[{
u"setting": u"level_of_education",
u"old": None,
u"new": u'b',
},
{
u"setting": u"level_of_education",
u"old": u'b',
u"new": None,
}]
[
{
u"setting": u"level_of_education",
u"old": None,
u"new": u'b',
},
{
u"setting": u"level_of_education",
u"old": u'b',
u"new": None,
}
]
)
def test_gender_field(self):
......@@ -363,16 +369,18 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, WebAppTest):
)
self.verify_settings_changed_events(
self.username, self.user_id,
[{
u"setting": u"gender",
u"old": None,
u"new": u'f',
},
{
u"setting": u"gender",
u"old": u'f',
u"new": None,
}]
[
{
u"setting": u"gender",
u"old": None,
u"new": u'f',
},
{
u"setting": u"gender",
u"old": u'f',
u"new": None,
}
]
)
def test_year_of_birth_field(self):
......@@ -390,16 +398,18 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, WebAppTest):
)
self.verify_settings_changed_events(
self.username, self.user_id,
[{
u"setting": u"year_of_birth",
u"old": None,
u"new": 1980,
},
{
u"setting": u"year_of_birth",
u"old": 1980,
u"new": None,
}]
[
{
u"setting": u"year_of_birth",
u"old": None,
u"new": 1980L,
},
{
u"setting": u"year_of_birth",
u"old": 1980L,
u"new": None,
}
]
)
def test_country_field(self):
......@@ -441,16 +451,19 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, WebAppTest):
)
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": [],
}],
self.username, self.user_id,
[
{
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"
)
......
common/test/acceptance/tests/lms/test_learner_profile.py
......@@ -19,8 +19,8 @@ class LearnerProfileTestMixin(EventsTestMixin):
Mixin with helper methods for testing learner profile pages.
"""
PRIVACY_PUBLIC = 'all_users'
PRIVACY_PRIVATE = 'private'
PRIVACY_PUBLIC = u'all_users'
PRIVACY_PRIVATE = u'private'
PUBLIC_PROFILE_FIELDS = ['username', 'country', 'language_proficiencies', 'bio']
PRIVATE_PROFILE_FIELDS = ['username']
......@@ -84,6 +84,27 @@ class LearnerProfileTestMixin(EventsTestMixin):
str(birth_year)
)
def verify_profile_page_is_public(self, profile_page, is_editable=True):
"""
Verify that the profile page is currently public.
:return:
"""
self.assertEqual(profile_page.visible_fields, self.PUBLIC_PROFILE_FIELDS)
if is_editable:
self.assertTrue(profile_page.privacy_field_visible)
self.assertEqual(profile_page.editable_fields, self.PUBLIC_PROFILE_EDITABLE_FIELDS)
else:
self.assertEqual(profile_page.editable_fields, [])
def verify_profile_page_is_private(self, profile_page, is_editable=True):
"""
Verify that the profile page is currently private.
:return:
"""
if is_editable:
self.assertTrue(profile_page.privacy_field_visible)
self.assertEqual(profile_page.visible_fields, self.PRIVATE_PROFILE_FIELDS)
def verify_profile_page_view_event(self, requesting_username, profile_user_id, visibility=None):
"""
Verifies that the correct view event was captured for the profile page.
......@@ -108,6 +129,24 @@ class LearnerProfileTestMixin(EventsTestMixin):
self.USER_SETTINGS_CHANGED_EVENT_NAME, self.start_time, profile_user_id, num_times, setting=setting
)
def verify_user_preference_changed_event(self, username, user_id, setting, old_value=None, new_value=None):
"""
Verifies that the correct user preference changed event was recorded.
"""
self.verify_events_of_type(
username,
u"edx.user.settings.changed",
[{
u"user_id": long(user_id),
u"table": u"user_api_userpreference",
u"setting": unicode(setting),
u"old": old_value,
u"new": new_value,
u"truncated": [],
}],
expected_referers=["/u/{username}".format(username=username)],
)
class OwnLearnerProfilePageTest(LearnerProfileTestMixin, WebAppTest):
"""
......@@ -144,6 +183,56 @@ class OwnLearnerProfilePageTest(LearnerProfileTestMixin, WebAppTest):
self.assertTrue(profile_page.profile_has_default_image)
self.assertTrue(profile_page.profile_has_image_with_public_access())
def test_make_profile_public(self):
"""
Scenario: Verify that the user can change their privacy.
Given that I am a registered user
And I visit my private profile page
And I set the profile visibility to public
Then a user preference changed event should be recorded
When I reload the page
Then the profile visibility should be shown as public
"""
username, user_id = self.log_in_as_unique_user()
profile_page = self.visit_profile_page(username, privacy=self.PRIVACY_PRIVATE)
profile_page.privacy = self.PRIVACY_PUBLIC
self.verify_user_preference_changed_event(
username, user_id, "account_privacy",
old_value=None, # Note: no old value as the default preference is private
new_value=self.PRIVACY_PUBLIC,
)
# Reload the page and verify that the profile is now public
self.browser.refresh()
profile_page.wait_for_page()
self.verify_profile_page_is_public(profile_page)
def test_make_profile_private(self):
"""
Scenario: Verify that the user can change their privacy.
Given that I am a registered user
And I visit my public profile page
And I set the profile visibility to private
Then a user preference changed event should be recorded
When I reload the page
Then the profile visibility should be shown as private
"""
username, user_id = self.log_in_as_unique_user()
profile_page = self.visit_profile_page(username, privacy=self.PRIVACY_PUBLIC)
profile_page.privacy = self.PRIVACY_PRIVATE
self.verify_user_preference_changed_event(
username, user_id, "account_privacy",
old_value=self.PRIVACY_PUBLIC,
new_value=self.PRIVACY_PRIVATE,
)
# Reload the page and verify that the profile is now private
self.browser.refresh()
profile_page.wait_for_page()
self.verify_profile_page_is_private(profile_page)
def test_dashboard_learner_profile_link(self):
"""
Scenario: Verify that my profile link is present on dashboard page and we can navigate to correct page.
......@@ -177,10 +266,7 @@ class OwnLearnerProfilePageTest(LearnerProfileTestMixin, WebAppTest):
"""
username, user_id = self.log_in_as_unique_user()
profile_page = self.visit_profile_page(username, privacy=self.PRIVACY_PRIVATE)
self.assertTrue(profile_page.privacy_field_visible)
self.assertEqual(profile_page.visible_fields, self.PRIVATE_PROFILE_FIELDS)
self.verify_profile_page_is_private(profile_page)
self.verify_profile_page_view_event(username, user_id, visibility=self.PRIVACY_PRIVATE)
def test_fields_on_my_public_profile(self):
......@@ -197,12 +283,7 @@ class OwnLearnerProfilePageTest(LearnerProfileTestMixin, WebAppTest):
"""
username, user_id = self.log_in_as_unique_user()
profile_page = self.visit_profile_page(username, privacy=self.PRIVACY_PUBLIC)
self.assertTrue(profile_page.privacy_field_visible)
self.assertEqual(profile_page.visible_fields, self.PUBLIC_PROFILE_FIELDS)
self.assertEqual(profile_page.editable_fields, self.PUBLIC_PROFILE_EDITABLE_FIELDS)
self.verify_profile_page_is_public(profile_page)
self.verify_profile_page_view_event(username, user_id, visibility=self.PRIVACY_PUBLIC)
def _test_dropdown_field(self, profile_page, field_id, new_value, displayed_value, mode):
......@@ -510,7 +591,7 @@ class OwnLearnerProfilePageTest(LearnerProfileTestMixin, WebAppTest):
profile_page.visit()
self.assertTrue(profile_page.profile_has_default_image)
self.assert_event_emitted_num_times(user_id, 'profile_image_uploaded_at', 1)
self.assert_event_emitted_num_times(user_id, 'profile_image_uploaded_at', 2)
def test_user_cannot_remove_default_image(self):
"""
......@@ -539,8 +620,10 @@ class OwnLearnerProfilePageTest(LearnerProfileTestMixin, WebAppTest):
"""
username, user_id = self.log_in_as_unique_user()
profile_page = self.visit_profile_page(username, privacy=self.PRIVACY_PUBLIC)
self.assert_default_image_has_public_access(profile_page)
profile_page.upload_file(filename='image.jpg')
profile_page.upload_file(filename='image.jpg')
self.assertTrue(profile_page.image_upload_success)
profile_page.upload_file(filename='image.jpg', wait_for_upload_button=False)
self.assert_event_emitted_num_times(user_id, 'profile_image_uploaded_at', 2)
......@@ -560,9 +643,7 @@ class DifferentUserLearnerProfilePageTest(LearnerProfileTestMixin, WebAppTest):
different_username, different_user_id = self._initialize_different_user(privacy=self.PRIVACY_PRIVATE)
username, __ = self.log_in_as_unique_user()
profile_page = self.visit_profile_page(different_username)
self.assertFalse(profile_page.privacy_field_visible)
self.assertEqual(profile_page.visible_fields, self.PRIVATE_PROFILE_FIELDS)
self.verify_profile_page_is_private(profile_page, is_editable=False)
self.verify_profile_page_view_event(username, different_user_id, visibility=self.PRIVACY_PRIVATE)
def test_different_user_under_age(self):
......@@ -601,9 +682,7 @@ class DifferentUserLearnerProfilePageTest(LearnerProfileTestMixin, WebAppTest):
username, __ = self.log_in_as_unique_user()
profile_page = self.visit_profile_page(different_username)
profile_page.wait_for_public_fields()
self.assertFalse(profile_page.privacy_field_visible)
self.assertEqual(profile_page.visible_fields, self.PUBLIC_PROFILE_FIELDS)
self.assertEqual(profile_page.editable_fields, [])
self.verify_profile_page_is_public(profile_page, is_editable=False)
self.verify_profile_page_view_event(username, different_user_id, visibility=self.PRIVACY_PUBLIC)
def _initialize_different_user(self, privacy=None, birth_year=None):
......
openedx/core/djangoapps/user_api/accounts/api.py
......@@ -8,6 +8,7 @@ from django.core.validators import validate_email, validate_slug, ValidationErro
from student.models import User, UserProfile, Registration, USER_SETTINGS_CHANGED_EVENT_NAME
from student import views as student_views
from util.model_utils import emit_setting_changed_event
from ..errors import (
AccountUpdateError, AccountValidationError, AccountUsernameInvalid, AccountPasswordInvalid,
......@@ -24,7 +25,6 @@ from . import (
USERNAME_MIN_LENGTH, USERNAME_MAX_LENGTH
)
from .serializers import AccountLegacyProfileSerializer, AccountUserSerializer
from eventtracking import tracker
@intercept_errors(UserAPIInternalError, ignore_errors=[UserAPIRequestError])
......@@ -199,15 +199,13 @@ def update_account_settings(requesting_user, update, username=None):
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,
}
emit_setting_changed_event(
user=existing_user,
event_name=USER_SETTINGS_CHANGED_EVENT_NAME,
db_table=existing_user_profile.language_proficiencies.model._meta.db_table,
setting_name="language_proficiencies",
old_value=old_language_proficiencies,
new_value=new_language_proficiencies,
)
# If the name was changed, store information about the change operation. This is outside of the
......
openedx/core/djangoapps/user_api/models.py
from django.contrib.auth.models import User
from django.core.validators import RegexValidator
from django.db import models
from django.db.models.signals import pre_delete, post_delete, pre_save, post_save
from django.dispatch import receiver
from model_utils.models import TimeStampedModel
from student.models import USER_SETTINGS_CHANGED_EVENT_NAME
from util.model_utils import get_changed_fields_dict, emit_setting_changed_event
from xmodule_django.models import CourseKeyField
# Currently, the "student" app is responsible for
......@@ -47,6 +51,39 @@ class UserPreference(models.Model):
return None
@receiver(pre_save, sender=UserPreference)
def pre_save_callback(sender, **kwargs):
"""
Event changes to user preferences.
"""
user_preference = kwargs["instance"]
user_preference._old_value = get_changed_fields_dict(user_preference, sender).get("value", None)
@receiver(post_save, sender=UserPreference)
def post_save_callback(sender, **kwargs):
"""
Event changes to user preferences.
"""
user_preference = kwargs["instance"]
emit_setting_changed_event(
user_preference.user, USER_SETTINGS_CHANGED_EVENT_NAME, sender._meta.db_table,
user_preference.key, user_preference._old_value, user_preference.value
)
user_preference._old_value = None
@receiver(post_delete, sender=UserPreference)
def post_delete_callback(sender, **kwargs):
"""
Event changes to user preferences.
"""
user_preference = kwargs["instance"]
emit_setting_changed_event(
user_preference.user, USER_SETTINGS_CHANGED_EVENT_NAME, sender._meta.db_table,
user_preference.key, user_preference.value, None
)
class UserCourseTag(models.Model):
"""
Per-course user tags, to be used by various things that want to store tags about
......
openedx/core/djangoapps/user_api/tests/test_models.py
import json
from django.db import IntegrityError
from django.test import TestCase
from student.models import USER_SETTINGS_CHANGED_EVENT_NAME
from student.tests.factories import UserFactory
from util.testing import EventTestMixin
from xmodule.modulestore.tests.factories import CourseFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from student.tests.factories import UserFactory
from ..tests.factories import UserPreferenceFactory, UserCourseTagFactory, UserOrgTagFactory
from ..models import UserPreference
from ..preferences.api import set_user_preference
USER_PREFERENCE_TABLE_NAME = "user_api_userpreference"
class UserPreferenceModelTest(ModuleStoreTestCase):
def test_duplicate_user_key(self):
......@@ -83,3 +91,84 @@ class UserPreferenceModelTest(ModuleStoreTestCase):
# get preference for key that doesn't exist for user
pref = UserPreference.get_value(user, 'testkey_none')
self.assertIsNone(pref)
class TestUserPreferenceEvents(EventTestMixin, TestCase):
"""
Mixin for verifying that user preference events are fired correctly.
"""
def setUp(self):
super(TestUserPreferenceEvents, self).setUp('util.model_utils.tracker')
self.user = UserFactory.create()
self.TEST_KEY = "test key"
self.TEST_VALUE = "test value"
self.user_preference = UserPreference.objects.create(user=self.user, key=self.TEST_KEY, value=self.TEST_VALUE)
self.reset_tracker()
def test_create_user_preference(self):
"""
Verify that we emit an event when a user preference is created.
"""
UserPreference.objects.create(user=self.user, key="new key", value="new value")
self.assert_user_preference_event_emitted(
key="new key", old=None, new="new value"
)
def test_update_user_preference(self):
"""
Verify that we emit an event when a user preference is updated.
"""
self.user_preference.value = "new value"
self.user_preference.save()
self.assert_user_preference_event_emitted(
key=self.TEST_KEY, old=self.TEST_VALUE, new="new value"
)
def test_delete_user_preference(self):
"""
Verify that we emit an event when a user preference is deleted.
"""
self.user_preference.delete()
self.assert_user_preference_event_emitted(
key=self.TEST_KEY, old=self.TEST_VALUE, new=None
)
def assert_user_preference_event_emitted(self, key, old, new, truncated=[]):
"""
Helper method to assert that we emit the expected user preference events.
Expected settings are passed in via `kwargs`.
"""
self.assert_event_emitted(
USER_SETTINGS_CHANGED_EVENT_NAME,
table=USER_PREFERENCE_TABLE_NAME,
user_id=self.user.id,
setting=key,
old=old,
new=new,
truncated=truncated,
)
def test_truncated_user_preference_event(self):
"""
Verify that we truncate the preference value if it is too long.
"""
MAX_STRING_LENGTH = 12500
OVERSIZE_STRING_LENGTH = MAX_STRING_LENGTH + 10
self.user_preference.value = "z" * OVERSIZE_STRING_LENGTH
self.user_preference.save()
self.assert_user_preference_event_emitted(
key=self.TEST_KEY,
old=self.TEST_VALUE,
new="z" * MAX_STRING_LENGTH,
truncated=["new"],
)
self.user_preference.value = "x" * OVERSIZE_STRING_LENGTH
self.user_preference.save()
self.assert_user_preference_event_emitted(
key=self.TEST_KEY,
old="z" * MAX_STRING_LENGTH,
new="x" * MAX_STRING_LENGTH,
truncated=["old", "new"],
)
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