Add model change event tracking to UserProfile.
This commit is contained in:
committed by
Andy Armstrong
parent
8ff9fd84e4
commit
3f20a6f304
@@ -28,7 +28,7 @@ from django.contrib.auth.hashers import make_password
|
||||
from django.contrib.auth.signals import user_logged_in, user_logged_out
|
||||
from django.db import models, IntegrityError
|
||||
from django.db.models import Count
|
||||
from django.db.models.signals import pre_save
|
||||
from django.db.models.signals import pre_save, post_save
|
||||
from django.dispatch import receiver, Signal
|
||||
from django.core.exceptions import ObjectDoesNotExist
|
||||
from django.utils.translation import ugettext_noop
|
||||
@@ -37,6 +37,7 @@ from config_models.models import ConfigurationModel
|
||||
from track import contexts
|
||||
from eventtracking import tracker
|
||||
from importlib import import_module
|
||||
from model_utils import FieldTracker
|
||||
|
||||
from opaque_keys.edx.locations import SlashSeparatedCourseKey
|
||||
|
||||
@@ -57,6 +58,7 @@ UNENROLL_DONE = Signal(providing_args=["course_enrollment", "skip_refund"])
|
||||
log = logging.getLogger(__name__)
|
||||
AUDIT_LOG = logging.getLogger("audit")
|
||||
SessionStore = import_module(settings.SESSION_ENGINE).SessionStore # pylint: disable=invalid-name
|
||||
USER_SETTINGS_CHANGED_EVENT_NAME = 'edx.user.settings.changed'
|
||||
|
||||
|
||||
class AnonymousUserId(models.Model):
|
||||
@@ -253,6 +255,9 @@ class UserProfile(models.Model):
|
||||
bio = models.CharField(blank=True, null=True, max_length=3000, db_index=False)
|
||||
profile_image_uploaded_at = models.DateTimeField(null=True)
|
||||
|
||||
# Use FieldTracker to track changes to model instances.
|
||||
tracker = FieldTracker()
|
||||
|
||||
@property
|
||||
def has_profile_image(self):
|
||||
"""
|
||||
@@ -332,6 +337,50 @@ def user_profile_pre_save_callback(sender, **kwargs):
|
||||
user_profile.profile_image_uploaded_at = None
|
||||
|
||||
|
||||
@receiver(post_save, sender=UserProfile)
|
||||
def user_profile_post_save_callback(sender, **kwargs):
|
||||
"""
|
||||
Emit analytics events after saving the UserProfile.
|
||||
"""
|
||||
user_profile = kwargs['instance']
|
||||
# pylint: disable=protected-access
|
||||
emit_field_changed_events(
|
||||
user_profile, user_profile.user, USER_SETTINGS_CHANGED_EVENT_NAME, sender._meta.db_table, ['meta']
|
||||
)
|
||||
|
||||
|
||||
def emit_field_changed_events(instance, user, event_name, db_table, excluded_fields=None):
|
||||
""" For the given model instance, save the fields that have changed since the last save.
|
||||
|
||||
Requires that the Model uses 'model_utils.FieldTracker' and has assigned it to 'tracker'.
|
||||
|
||||
Args:
|
||||
instance (Model instance): the model instance that is being saved
|
||||
user (User): the user that this instance 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
|
||||
excluded_fields (list): a list of field names for which events should
|
||||
not be emitted
|
||||
|
||||
Returns:
|
||||
None
|
||||
"""
|
||||
excluded_fields = excluded_fields or []
|
||||
changed_fields = instance.tracker.changed()
|
||||
for field in changed_fields:
|
||||
if field not in excluded_fields:
|
||||
tracker.emit(
|
||||
event_name,
|
||||
{
|
||||
"setting": field,
|
||||
'old': changed_fields[field],
|
||||
'new': getattr(instance, field),
|
||||
"user_id": user.id,
|
||||
"table": db_table
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
class UserSignupSource(models.Model):
|
||||
"""
|
||||
This table contains information about users registering
|
||||
|
||||
48
common/djangoapps/student/tests/test_events.py
Normal file
48
common/djangoapps/student/tests/test_events.py
Normal file
@@ -0,0 +1,48 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
"""
|
||||
Test that various events are fired for models in the student app.
|
||||
"""
|
||||
from django.test import TestCase
|
||||
|
||||
from student.tests.factories import UserFactory
|
||||
from student.tests.tests import UserProfileEventTestMixin
|
||||
|
||||
|
||||
class TestUserProfileEvents(UserProfileEventTestMixin, TestCase):
|
||||
"""
|
||||
Test that we emit field change events when UserProfile models are changed.
|
||||
"""
|
||||
def setUp(self):
|
||||
super(TestUserProfileEvents, self).setUp()
|
||||
self.user = UserFactory.create()
|
||||
self.profile = self.user.profile
|
||||
self.reset_tracker()
|
||||
|
||||
def test_change_one_field(self):
|
||||
"""
|
||||
Verify that we emit an event when a single field changes on the user
|
||||
profile.
|
||||
"""
|
||||
self.profile.year_of_birth = 1900
|
||||
self.profile.save()
|
||||
self.assert_profile_event_emitted(setting='year_of_birth', old=None, new=self.profile.year_of_birth)
|
||||
|
||||
def test_change_many_fields(self):
|
||||
"""
|
||||
Verify that we emit one event per field when many fields change on the
|
||||
user profile in one transaction.
|
||||
"""
|
||||
self.profile.gender = u'o'
|
||||
self.profile.bio = 'test bio'
|
||||
self.profile.save()
|
||||
self.assert_profile_event_emitted(setting='bio', old=None, new=self.profile.bio)
|
||||
self.assert_profile_event_emitted(setting='gender', old=u'm', new=u'o')
|
||||
|
||||
def test_unicode(self):
|
||||
"""
|
||||
Verify that the events we emit can handle unicode characters.
|
||||
"""
|
||||
old_name = self.profile.name
|
||||
self.profile.name = u'Dånîél'
|
||||
self.profile.save()
|
||||
self.assert_profile_event_emitted(setting='name', old=old_name, new=self.profile.name)
|
||||
@@ -22,11 +22,12 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey
|
||||
|
||||
from student.models import (
|
||||
anonymous_id_for_user, user_by_anonymous_id, CourseEnrollment, unique_id_for_user,
|
||||
LinkedInAddToProfileConfiguration
|
||||
LinkedInAddToProfileConfiguration, USER_SETTINGS_CHANGED_EVENT_NAME
|
||||
)
|
||||
from student.views import (process_survey_link, _cert_info,
|
||||
change_enrollment, complete_course_mode_info)
|
||||
from student.tests.factories import UserFactory, CourseModeFactory
|
||||
from util.testing import EventTestMixin
|
||||
from xmodule.modulestore.tests.factories import CourseFactory
|
||||
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
|
||||
|
||||
@@ -485,19 +486,31 @@ class DashboardTest(ModuleStoreTestCase):
|
||||
self.assertContains(response, expected_url)
|
||||
|
||||
|
||||
class EnrollmentEventTestMixin(object):
|
||||
""" Mixin with assertions for validating enrollment events. """
|
||||
|
||||
class UserProfileEventTestMixin(EventTestMixin):
|
||||
"""
|
||||
Mixin for verifying that UserProfile events were emitted during a test.
|
||||
"""
|
||||
def setUp(self):
|
||||
super(EnrollmentEventTestMixin, self).setUp()
|
||||
patcher = patch('student.models.tracker')
|
||||
self.mock_tracker = patcher.start()
|
||||
self.addCleanup(patcher.stop)
|
||||
super(UserProfileEventTestMixin, self).setUp('student.models.tracker')
|
||||
|
||||
def assert_no_events_were_emitted(self):
|
||||
"""Ensures no events were emitted since the last event related assertion"""
|
||||
self.assertFalse(self.mock_tracker.emit.called) # pylint: disable=maybe-no-member
|
||||
self.mock_tracker.reset_mock()
|
||||
def assert_profile_event_emitted(self, **kwargs):
|
||||
"""
|
||||
Helper method to assert that we emit the expected user settings events.
|
||||
|
||||
Expected settings are passed in via `kwargs`.
|
||||
"""
|
||||
self.assert_event_emitted(
|
||||
USER_SETTINGS_CHANGED_EVENT_NAME,
|
||||
table='auth_userprofile',
|
||||
user_id=self.user.id,
|
||||
**kwargs
|
||||
)
|
||||
|
||||
|
||||
class EnrollmentEventTestMixin(EventTestMixin):
|
||||
""" Mixin with assertions for validating enrollment events. """
|
||||
def setUp(self):
|
||||
super(EnrollmentEventTestMixin, self).setUp('student.models.tracker')
|
||||
|
||||
def assert_enrollment_mode_change_event_was_emitted(self, user, course_key, mode):
|
||||
"""Ensures an enrollment mode change event was emitted"""
|
||||
|
||||
@@ -9,6 +9,7 @@ from bok_choy.web_app_test import WebAppTest
|
||||
from ...pages.lms.account_settings import AccountSettingsPage
|
||||
from ...pages.lms.auto_auth import AutoAuthPage
|
||||
from ...pages.lms.dashboard import DashboardPage
|
||||
from ..helpers import EventsTestMixin
|
||||
|
||||
from ..helpers import EventsTestMixin
|
||||
|
||||
@@ -206,6 +207,8 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, WebAppTest):
|
||||
[u'another name', self.USERNAME],
|
||||
)
|
||||
|
||||
self.assert_event_emitted_num_times('edx.user.settings.changed', self.start_time, self.user_id, 2)
|
||||
|
||||
def test_email_field(self):
|
||||
"""
|
||||
Test behaviour of "Email" field.
|
||||
@@ -287,6 +290,7 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, WebAppTest):
|
||||
u'',
|
||||
[u'Bachelor\'s degree', u''],
|
||||
)
|
||||
self.assert_event_emitted_num_times('edx.user.settings.changed', self.start_time, self.user_id, 2)
|
||||
|
||||
def test_gender_field(self):
|
||||
"""
|
||||
@@ -298,11 +302,13 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, WebAppTest):
|
||||
u'',
|
||||
[u'Female', u''],
|
||||
)
|
||||
self.assert_event_emitted_num_times('edx.user.settings.changed', self.start_time, self.user_id, 2)
|
||||
|
||||
def test_year_of_birth_field(self):
|
||||
"""
|
||||
Test behaviour of "Year of Birth" field.
|
||||
"""
|
||||
# Note that when we clear the year_of_birth here we're firing an event.
|
||||
self.assertEqual(self.account_settings_page.value_for_dropdown_field('year_of_birth', ''), '')
|
||||
self._test_dropdown_field(
|
||||
u'year_of_birth',
|
||||
@@ -310,6 +316,7 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, WebAppTest):
|
||||
u'',
|
||||
[u'1980', u''],
|
||||
)
|
||||
self.assert_event_emitted_num_times('edx.user.settings.changed', self.start_time, self.user_id, 3)
|
||||
|
||||
def test_country_field(self):
|
||||
"""
|
||||
|
||||
@@ -77,6 +77,9 @@ class OrdersViewTests(EnrollmentEventTestMixin, EcommerceApiTestMixin, ModuleSto
|
||||
sku=uuid4().hex.decode('ascii')
|
||||
)
|
||||
|
||||
# Ignore events fired from UserFactory creation
|
||||
self.reset_tracker()
|
||||
|
||||
def test_login_required(self):
|
||||
"""
|
||||
The view should return HTTP 403 status if the user is not logged in.
|
||||
|
||||
@@ -14,6 +14,7 @@ from PIL import Image
|
||||
from rest_framework.test import APITestCase, APIClient
|
||||
|
||||
from student.tests.factories import UserFactory
|
||||
from student.tests.tests import UserProfileEventTestMixin
|
||||
|
||||
from ...user_api.accounts.image_helpers import (
|
||||
set_has_profile_image,
|
||||
@@ -28,7 +29,7 @@ TEST_PASSWORD = "test"
|
||||
TEST_UPLOAD_DT = datetime.datetime(2002, 1, 9, 15, 43, 01, tzinfo=UTC)
|
||||
|
||||
|
||||
class ProfileImageEndpointTestCase(APITestCase):
|
||||
class ProfileImageEndpointTestCase(UserProfileEventTestMixin, APITestCase):
|
||||
"""
|
||||
Base class / shared infrastructure for tests of profile_image "upload" and
|
||||
"remove" endpoints.
|
||||
@@ -50,6 +51,10 @@ class ProfileImageEndpointTestCase(APITestCase):
|
||||
# assume user.profile.has_profile_image is False by default
|
||||
self.assertFalse(self.user.profile.has_profile_image)
|
||||
|
||||
# Reset the mock event tracker so that we're not considering the
|
||||
# initial profile creation events.
|
||||
self.reset_tracker()
|
||||
|
||||
def tearDown(self):
|
||||
super(ProfileImageEndpointTestCase, self).tearDown()
|
||||
for name in get_profile_image_names(self.user.username).values():
|
||||
@@ -104,6 +109,15 @@ class ProfileImageUploadTestCase(ProfileImageEndpointTestCase):
|
||||
"""
|
||||
_view_name = "profile_image_upload"
|
||||
|
||||
def check_upload_event_emitted(self):
|
||||
"""
|
||||
Make sure we emit a UserProfile event corresponding to the
|
||||
profile_image_uploaded_at field changing.
|
||||
"""
|
||||
self.assert_profile_event_emitted(
|
||||
setting='profile_image_uploaded_at', old=None, new=TEST_UPLOAD_DT
|
||||
)
|
||||
|
||||
def test_unsupported_methods(self, mock_log):
|
||||
"""
|
||||
Test that GET, PUT, PATCH, and DELETE are not supported.
|
||||
@@ -113,6 +127,7 @@ class ProfileImageUploadTestCase(ProfileImageEndpointTestCase):
|
||||
self.assertEqual(405, self.client.patch(self.url).status_code)
|
||||
self.assertEqual(405, self.client.delete(self.url).status_code)
|
||||
self.assertFalse(mock_log.info.called)
|
||||
self.assert_no_events_were_emitted()
|
||||
|
||||
def test_anonymous_access(self, mock_log):
|
||||
"""
|
||||
@@ -122,6 +137,7 @@ class ProfileImageUploadTestCase(ProfileImageEndpointTestCase):
|
||||
response = anonymous_client.post(self.url)
|
||||
self.assertEqual(401, response.status_code)
|
||||
self.assertFalse(mock_log.info.called)
|
||||
self.assert_no_events_were_emitted()
|
||||
|
||||
@patch('openedx.core.djangoapps.profile_images.views._make_upload_dt', return_value=TEST_UPLOAD_DT)
|
||||
def test_upload_self(self, mock_make_image_version, mock_log): # pylint: disable=unused-argument
|
||||
@@ -137,12 +153,15 @@ class ProfileImageUploadTestCase(ProfileImageEndpointTestCase):
|
||||
LOG_MESSAGE_CREATE,
|
||||
{'image_names': get_profile_image_names(self.user.username).values(), 'user_id': self.user.id}
|
||||
)
|
||||
self.check_upload_event_emitted()
|
||||
|
||||
def test_upload_other(self, mock_log):
|
||||
"""
|
||||
Test that an authenticated user cannot POST to another user's upload endpoint.
|
||||
"""
|
||||
different_user = UserFactory.create(password=TEST_PASSWORD)
|
||||
# Ignore UserProfileFactory creation events.
|
||||
self.reset_tracker()
|
||||
different_client = APIClient()
|
||||
different_client.login(username=different_user.username, password=TEST_PASSWORD)
|
||||
with make_image_file() as image_file:
|
||||
@@ -151,12 +170,15 @@ class ProfileImageUploadTestCase(ProfileImageEndpointTestCase):
|
||||
self.check_images(False)
|
||||
self.check_has_profile_image(False)
|
||||
self.assertFalse(mock_log.info.called)
|
||||
self.assert_no_events_were_emitted()
|
||||
|
||||
def test_upload_staff(self, mock_log):
|
||||
"""
|
||||
Test that an authenticated staff cannot POST to another user's upload endpoint.
|
||||
"""
|
||||
staff_user = UserFactory(is_staff=True, password=TEST_PASSWORD)
|
||||
# Ignore UserProfileFactory creation events.
|
||||
self.reset_tracker()
|
||||
staff_client = APIClient()
|
||||
staff_client.login(username=staff_user.username, password=TEST_PASSWORD)
|
||||
with make_image_file() as image_file:
|
||||
@@ -165,6 +187,7 @@ class ProfileImageUploadTestCase(ProfileImageEndpointTestCase):
|
||||
self.check_images(False)
|
||||
self.check_has_profile_image(False)
|
||||
self.assertFalse(mock_log.info.called)
|
||||
self.assert_no_events_were_emitted()
|
||||
|
||||
def test_upload_missing_file(self, mock_log):
|
||||
"""
|
||||
@@ -179,6 +202,7 @@ class ProfileImageUploadTestCase(ProfileImageEndpointTestCase):
|
||||
self.check_images(False)
|
||||
self.check_has_profile_image(False)
|
||||
self.assertFalse(mock_log.info.called)
|
||||
self.assert_no_events_were_emitted()
|
||||
|
||||
def test_upload_not_a_file(self, mock_log):
|
||||
"""
|
||||
@@ -194,6 +218,7 @@ class ProfileImageUploadTestCase(ProfileImageEndpointTestCase):
|
||||
self.check_images(False)
|
||||
self.check_has_profile_image(False)
|
||||
self.assertFalse(mock_log.info.called)
|
||||
self.assert_no_events_were_emitted()
|
||||
|
||||
def test_upload_validation(self, mock_log):
|
||||
"""
|
||||
@@ -214,6 +239,7 @@ class ProfileImageUploadTestCase(ProfileImageEndpointTestCase):
|
||||
self.check_images(False)
|
||||
self.check_has_profile_image(False)
|
||||
self.assertFalse(mock_log.info.called)
|
||||
self.assert_no_events_were_emitted()
|
||||
|
||||
@patch('PIL.Image.open')
|
||||
def test_upload_failure(self, image_open, mock_log):
|
||||
@@ -228,6 +254,7 @@ class ProfileImageUploadTestCase(ProfileImageEndpointTestCase):
|
||||
self.check_images(False)
|
||||
self.check_has_profile_image(False)
|
||||
self.assertFalse(mock_log.info.called)
|
||||
self.assert_no_events_were_emitted()
|
||||
|
||||
|
||||
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Profile Image API is only supported in LMS')
|
||||
@@ -244,6 +271,17 @@ class ProfileImageRemoveTestCase(ProfileImageEndpointTestCase):
|
||||
create_profile_images(image_file, get_profile_image_names(self.user.username))
|
||||
self.check_images()
|
||||
set_has_profile_image(self.user.username, True, TEST_UPLOAD_DT)
|
||||
# Ignore previous event
|
||||
self.reset_tracker()
|
||||
|
||||
def check_remove_event_emitted(self):
|
||||
"""
|
||||
Make sure we emit a UserProfile event corresponding to the
|
||||
profile_image_uploaded_at field changing.
|
||||
"""
|
||||
self.assert_profile_event_emitted(
|
||||
setting='profile_image_uploaded_at', old=TEST_UPLOAD_DT, new=None
|
||||
)
|
||||
|
||||
def test_unsupported_methods(self, mock_log):
|
||||
"""
|
||||
@@ -254,6 +292,7 @@ class ProfileImageRemoveTestCase(ProfileImageEndpointTestCase):
|
||||
self.assertEqual(405, self.client.patch(self.url).status_code)
|
||||
self.assertEqual(405, self.client.delete(self.url).status_code)
|
||||
self.assertFalse(mock_log.info.called)
|
||||
self.assert_no_events_were_emitted()
|
||||
|
||||
def test_anonymous_access(self, mock_log):
|
||||
"""
|
||||
@@ -264,6 +303,7 @@ class ProfileImageRemoveTestCase(ProfileImageEndpointTestCase):
|
||||
response = request(self.url)
|
||||
self.assertEqual(401, response.status_code)
|
||||
self.assertFalse(mock_log.info.called)
|
||||
self.assert_no_events_were_emitted()
|
||||
|
||||
def test_remove_self(self, mock_log):
|
||||
"""
|
||||
@@ -278,6 +318,7 @@ class ProfileImageRemoveTestCase(ProfileImageEndpointTestCase):
|
||||
LOG_MESSAGE_DELETE,
|
||||
{'image_names': get_profile_image_names(self.user.username).values(), 'user_id': self.user.id}
|
||||
)
|
||||
self.check_remove_event_emitted()
|
||||
|
||||
def test_remove_other(self, mock_log):
|
||||
"""
|
||||
@@ -285,6 +326,8 @@ class ProfileImageRemoveTestCase(ProfileImageEndpointTestCase):
|
||||
profile images.
|
||||
"""
|
||||
different_user = UserFactory.create(password=TEST_PASSWORD)
|
||||
# Ignore UserProfileFactory creation events.
|
||||
self.reset_tracker()
|
||||
different_client = APIClient()
|
||||
different_client.login(username=different_user.username, password=TEST_PASSWORD)
|
||||
response = different_client.post(self.url)
|
||||
@@ -292,6 +335,7 @@ class ProfileImageRemoveTestCase(ProfileImageEndpointTestCase):
|
||||
self.check_images(True) # thumbnails should remain intact.
|
||||
self.check_has_profile_image(True)
|
||||
self.assertFalse(mock_log.info.called)
|
||||
self.assert_no_events_were_emitted()
|
||||
|
||||
def test_remove_staff(self, mock_log):
|
||||
"""
|
||||
@@ -309,6 +353,7 @@ class ProfileImageRemoveTestCase(ProfileImageEndpointTestCase):
|
||||
LOG_MESSAGE_DELETE,
|
||||
{'image_names': get_profile_image_names(self.user.username).values(), 'user_id': self.user.id}
|
||||
)
|
||||
self.check_remove_event_emitted()
|
||||
|
||||
@patch('student.models.UserProfile.save')
|
||||
def test_remove_failure(self, user_profile_save, mock_log):
|
||||
@@ -322,3 +367,4 @@ class ProfileImageRemoveTestCase(ProfileImageEndpointTestCase):
|
||||
self.check_images(True) # thumbnails should remain intact.
|
||||
self.check_has_profile_image(True)
|
||||
self.assertFalse(mock_log.info.called)
|
||||
self.assert_no_events_were_emitted()
|
||||
|
||||
Reference in New Issue
Block a user