Commit f17ab1ec by Gabe Mulley

Merge pull request #1533 from mulby/gabe/enrollment-event

emit enrollment events
parents a6c02ccc 5734cae1
...@@ -29,6 +29,12 @@ from django.forms import ModelForm, forms ...@@ -29,6 +29,12 @@ from django.forms import ModelForm, forms
from course_modes.models import CourseMode from course_modes.models import CourseMode
import comment_client as cc import comment_client as cc
from pytz import UTC from pytz import UTC
import crum
from track import contexts
from track.views import server_track
from eventtracking import tracker
unenroll_done = django.dispatch.Signal(providing_args=["course_enrollment"]) unenroll_done = django.dispatch.Signal(providing_args=["course_enrollment"])
...@@ -667,6 +673,11 @@ class PendingEmailChange(models.Model): ...@@ -667,6 +673,11 @@ class PendingEmailChange(models.Model):
activation_key = models.CharField(('activation key'), max_length=32, unique=True, db_index=True) activation_key = models.CharField(('activation key'), max_length=32, unique=True, db_index=True)
EVENT_NAME_ENROLLMENT_ACTIVATED = 'edx.course.enrollment.activated'
EVENT_NAME_ENROLLMENT_DEACTIVATED = 'edx.course.enrollment.deactivated'
class CourseEnrollment(models.Model): class CourseEnrollment(models.Model):
""" """
Represents a Student's Enrollment record for a single Course. You should Represents a Student's Enrollment record for a single Course. You should
...@@ -737,19 +748,55 @@ class CourseEnrollment(models.Model): ...@@ -737,19 +748,55 @@ class CourseEnrollment(models.Model):
if user.id is None: if user.id is None:
user.save() user.save()
enrollment, _ = CourseEnrollment.objects.get_or_create( enrollment, created = CourseEnrollment.objects.get_or_create(
user=user, user=user,
course_id=course_id, course_id=course_id,
) )
# In case we're reactivating a deactivated enrollment, or changing the
# enrollment mode. activation_changed = False
if enrollment.mode != mode or enrollment.is_active != is_active: if enrollment.is_active != is_active:
enrollment.mode = mode
enrollment.is_active = is_active enrollment.is_active = is_active
activation_changed = True
mode_changed = False
if enrollment.mode != mode:
enrollment.mode = mode
mode_changed = True
if activation_changed or mode_changed:
enrollment.save() enrollment.save()
if created:
if is_active:
enrollment.emit_event(EVENT_NAME_ENROLLMENT_ACTIVATED)
else:
if activation_changed:
if is_active:
enrollment.emit_event(EVENT_NAME_ENROLLMENT_ACTIVATED)
else:
enrollment.emit_event(EVENT_NAME_ENROLLMENT_DEACTIVATED)
return enrollment return enrollment
def emit_event(self, event_name):
"""
Emits an event to explicitly track course enrollment and unenrollment.
"""
try:
context = contexts.course_context_from_course_id(self.course_id)
data = {
'user_id': self.user.id,
'course_id': self.course_id,
'mode': self.mode,
}
with tracker.get_tracker().context(event_name, context):
server_track(crum.get_current_request(), event_name, data)
except: # pylint: disable=bare-except
if event_name and self.course_id:
log.exception('Unable to emit event %s for user %s and course %s', event_name, self.user.username, self.course_id)
@classmethod @classmethod
def enroll(cls, user, course_id, mode="honor"): def enroll(cls, user, course_id, mode="honor"):
""" """
...@@ -825,9 +872,13 @@ class CourseEnrollment(models.Model): ...@@ -825,9 +872,13 @@ class CourseEnrollment(models.Model):
""" """
try: try:
record = CourseEnrollment.objects.get(user=user, course_id=course_id) record = CourseEnrollment.objects.get(user=user, course_id=course_id)
record.is_active = False if record.is_active:
record.save() record.is_active = False
unenroll_done.send(sender=cls, course_enrollment=record) record.save()
record.emit_event(EVENT_NAME_ENROLLMENT_DEACTIVATED)
unenroll_done.send(sender=cls, course_enrollment=record)
except cls.DoesNotExist: except cls.DoesNotExist:
err_msg = u"Tried to unenroll student {} from {} but they were not enrolled" err_msg = u"Tried to unenroll student {} from {} but they were not enrolled"
log.error(err_msg.format(user, course_id)) log.error(err_msg.format(user, course_id))
...@@ -918,6 +969,7 @@ class CourseEnrollment(models.Model): ...@@ -918,6 +969,7 @@ class CourseEnrollment(models.Model):
if not self.is_active: if not self.is_active:
self.is_active = True self.is_active = True
self.save() self.save()
self.emit_event(EVENT_NAME_ENROLLMENT_ACTIVATED)
def deactivate(self): def deactivate(self):
"""Makes this `CourseEnrollment` record inactive. Saves immediately. An """Makes this `CourseEnrollment` record inactive. Saves immediately. An
...@@ -926,6 +978,7 @@ class CourseEnrollment(models.Model): ...@@ -926,6 +978,7 @@ class CourseEnrollment(models.Model):
if self.is_active: if self.is_active:
self.is_active = False self.is_active = False
self.save() self.save()
self.emit_event(EVENT_NAME_ENROLLMENT_DEACTIVATED)
def refundable(self): def refundable(self):
""" """
......
...@@ -25,7 +25,7 @@ from xmodule.modulestore.tests.factories import CourseFactory ...@@ -25,7 +25,7 @@ from xmodule.modulestore.tests.factories import CourseFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from courseware.tests.tests import TEST_DATA_MIXED_MODULESTORE from courseware.tests.tests import TEST_DATA_MIXED_MODULESTORE
from mock import Mock, patch from mock import Mock, patch, sentinel
from textwrap import dedent from textwrap import dedent
from student.models import unique_id_for_user, CourseEnrollment from student.models import unique_id_for_user, CourseEnrollment
...@@ -276,6 +276,16 @@ class DashboardTest(TestCase): ...@@ -276,6 +276,16 @@ class DashboardTest(TestCase):
class EnrollInCourseTest(TestCase): class EnrollInCourseTest(TestCase):
"""Tests enrolling and unenrolling in courses.""" """Tests enrolling and unenrolling in courses."""
def setUp(self):
patcher = patch('student.models.server_track')
self.mock_server_track = patcher.start()
self.addCleanup(patcher.stop)
crum_patcher = patch('student.models.crum.get_current_request')
self.mock_get_current_request = crum_patcher.start()
self.addCleanup(crum_patcher.stop)
self.mock_get_current_request.return_value = sentinel.request
def test_enrollment(self): def test_enrollment(self):
user = User.objects.create_user("joe", "joe@joe.com", "password") user = User.objects.create_user("joe", "joe@joe.com", "password")
course_id = "edX/Test101/2013" course_id = "edX/Test101/2013"
...@@ -289,24 +299,28 @@ class EnrollInCourseTest(TestCase): ...@@ -289,24 +299,28 @@ class EnrollInCourseTest(TestCase):
self.assertTrue(CourseEnrollment.is_enrolled(user, course_id)) self.assertTrue(CourseEnrollment.is_enrolled(user, course_id))
self.assertTrue(CourseEnrollment.is_enrolled_by_partial(user, self.assertTrue(CourseEnrollment.is_enrolled_by_partial(user,
course_id_partial)) course_id_partial))
self.assert_enrollment_event_was_emitted(user, course_id)
# Enrolling them again should be harmless # Enrolling them again should be harmless
CourseEnrollment.enroll(user, course_id) CourseEnrollment.enroll(user, course_id)
self.assertTrue(CourseEnrollment.is_enrolled(user, course_id)) self.assertTrue(CourseEnrollment.is_enrolled(user, course_id))
self.assertTrue(CourseEnrollment.is_enrolled_by_partial(user, self.assertTrue(CourseEnrollment.is_enrolled_by_partial(user,
course_id_partial)) course_id_partial))
self.assert_no_events_were_emitted()
# Now unenroll the user # Now unenroll the user
CourseEnrollment.unenroll(user, course_id) CourseEnrollment.unenroll(user, course_id)
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id)) self.assertFalse(CourseEnrollment.is_enrolled(user, course_id))
self.assertFalse(CourseEnrollment.is_enrolled_by_partial(user, self.assertFalse(CourseEnrollment.is_enrolled_by_partial(user,
course_id_partial)) course_id_partial))
self.assert_unenrollment_event_was_emitted(user, course_id)
# Unenrolling them again should also be harmless # Unenrolling them again should also be harmless
CourseEnrollment.unenroll(user, course_id) CourseEnrollment.unenroll(user, course_id)
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id)) self.assertFalse(CourseEnrollment.is_enrolled(user, course_id))
self.assertFalse(CourseEnrollment.is_enrolled_by_partial(user, self.assertFalse(CourseEnrollment.is_enrolled_by_partial(user,
course_id_partial)) course_id_partial))
self.assert_no_events_were_emitted()
# The enrollment record should still exist, just be inactive # The enrollment record should still exist, just be inactive
enrollment_record = CourseEnrollment.objects.get( enrollment_record = CourseEnrollment.objects.get(
...@@ -315,6 +329,37 @@ class EnrollInCourseTest(TestCase): ...@@ -315,6 +329,37 @@ class EnrollInCourseTest(TestCase):
) )
self.assertFalse(enrollment_record.is_active) self.assertFalse(enrollment_record.is_active)
def assert_no_events_were_emitted(self):
"""Ensures no events were emitted since the last event related assertion"""
self.assertFalse(self.mock_server_track.called)
self.mock_server_track.reset_mock()
def assert_enrollment_event_was_emitted(self, user, course_id):
"""Ensures an enrollment event was emitted since the last event related assertion"""
self.mock_server_track.assert_called_once_with(
sentinel.request,
'edx.course.enrollment.activated',
{
'course_id': course_id,
'user_id': user.pk,
'mode': 'honor'
}
)
self.mock_server_track.reset_mock()
def assert_unenrollment_event_was_emitted(self, user, course_id):
"""Ensures an unenrollment event was emitted since the last event related assertion"""
self.mock_server_track.assert_called_once_with(
sentinel.request,
'edx.course.enrollment.deactivated',
{
'course_id': course_id,
'user_id': user.pk,
'mode': 'honor'
}
)
self.mock_server_track.reset_mock()
def test_enrollment_non_existent_user(self): def test_enrollment_non_existent_user(self):
# Testing enrollment of newly unsaved user (i.e. no database entry) # Testing enrollment of newly unsaved user (i.e. no database entry)
user = User(username="rusty", email="rusty@fake.edx.org") user = User(username="rusty", email="rusty@fake.edx.org")
...@@ -324,11 +369,13 @@ class EnrollInCourseTest(TestCase): ...@@ -324,11 +369,13 @@ class EnrollInCourseTest(TestCase):
# Unenroll does nothing # Unenroll does nothing
CourseEnrollment.unenroll(user, course_id) CourseEnrollment.unenroll(user, course_id)
self.assert_no_events_were_emitted()
# Implicit save() happens on new User object when enrolling, so this # Implicit save() happens on new User object when enrolling, so this
# should still work # should still work
CourseEnrollment.enroll(user, course_id) CourseEnrollment.enroll(user, course_id)
self.assertTrue(CourseEnrollment.is_enrolled(user, course_id)) self.assertTrue(CourseEnrollment.is_enrolled(user, course_id))
self.assert_enrollment_event_was_emitted(user, course_id)
def test_enrollment_by_email(self): def test_enrollment_by_email(self):
user = User.objects.create(username="jack", email="jack@fake.edx.org") user = User.objects.create(username="jack", email="jack@fake.edx.org")
...@@ -336,11 +383,13 @@ class EnrollInCourseTest(TestCase): ...@@ -336,11 +383,13 @@ class EnrollInCourseTest(TestCase):
CourseEnrollment.enroll_by_email("jack@fake.edx.org", course_id) CourseEnrollment.enroll_by_email("jack@fake.edx.org", course_id)
self.assertTrue(CourseEnrollment.is_enrolled(user, course_id)) self.assertTrue(CourseEnrollment.is_enrolled(user, course_id))
self.assert_enrollment_event_was_emitted(user, course_id)
# This won't throw an exception, even though the user is not found # This won't throw an exception, even though the user is not found
self.assertIsNone( self.assertIsNone(
CourseEnrollment.enroll_by_email("not_jack@fake.edx.org", course_id) CourseEnrollment.enroll_by_email("not_jack@fake.edx.org", course_id)
) )
self.assert_no_events_were_emitted()
self.assertRaises( self.assertRaises(
User.DoesNotExist, User.DoesNotExist,
...@@ -349,17 +398,21 @@ class EnrollInCourseTest(TestCase): ...@@ -349,17 +398,21 @@ class EnrollInCourseTest(TestCase):
course_id, course_id,
ignore_errors=False ignore_errors=False
) )
self.assert_no_events_were_emitted()
# Now unenroll them by email # Now unenroll them by email
CourseEnrollment.unenroll_by_email("jack@fake.edx.org", course_id) CourseEnrollment.unenroll_by_email("jack@fake.edx.org", course_id)
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id)) self.assertFalse(CourseEnrollment.is_enrolled(user, course_id))
self.assert_unenrollment_event_was_emitted(user, course_id)
# Harmless second unenroll # Harmless second unenroll
CourseEnrollment.unenroll_by_email("jack@fake.edx.org", course_id) CourseEnrollment.unenroll_by_email("jack@fake.edx.org", course_id)
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id)) self.assertFalse(CourseEnrollment.is_enrolled(user, course_id))
self.assert_no_events_were_emitted()
# Unenroll on non-existent user shouldn't throw an error # Unenroll on non-existent user shouldn't throw an error
CourseEnrollment.unenroll_by_email("not_jack@fake.edx.org", course_id) CourseEnrollment.unenroll_by_email("not_jack@fake.edx.org", course_id)
self.assert_no_events_were_emitted()
def test_enrollment_multiple_classes(self): def test_enrollment_multiple_classes(self):
user = User(username="rusty", email="rusty@fake.edx.org") user = User(username="rusty", email="rusty@fake.edx.org")
...@@ -367,15 +420,19 @@ class EnrollInCourseTest(TestCase): ...@@ -367,15 +420,19 @@ class EnrollInCourseTest(TestCase):
course_id2 = "MITx/6.003z/2012" course_id2 = "MITx/6.003z/2012"
CourseEnrollment.enroll(user, course_id1) CourseEnrollment.enroll(user, course_id1)
self.assert_enrollment_event_was_emitted(user, course_id1)
CourseEnrollment.enroll(user, course_id2) CourseEnrollment.enroll(user, course_id2)
self.assert_enrollment_event_was_emitted(user, course_id2)
self.assertTrue(CourseEnrollment.is_enrolled(user, course_id1)) self.assertTrue(CourseEnrollment.is_enrolled(user, course_id1))
self.assertTrue(CourseEnrollment.is_enrolled(user, course_id2)) self.assertTrue(CourseEnrollment.is_enrolled(user, course_id2))
CourseEnrollment.unenroll(user, course_id1) CourseEnrollment.unenroll(user, course_id1)
self.assert_unenrollment_event_was_emitted(user, course_id1)
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id1)) self.assertFalse(CourseEnrollment.is_enrolled(user, course_id1))
self.assertTrue(CourseEnrollment.is_enrolled(user, course_id2)) self.assertTrue(CourseEnrollment.is_enrolled(user, course_id2))
CourseEnrollment.unenroll(user, course_id2) CourseEnrollment.unenroll(user, course_id2)
self.assert_unenrollment_event_was_emitted(user, course_id2)
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id1)) self.assertFalse(CourseEnrollment.is_enrolled(user, course_id1))
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id2)) self.assertFalse(CourseEnrollment.is_enrolled(user, course_id2))
...@@ -388,27 +445,33 @@ class EnrollInCourseTest(TestCase): ...@@ -388,27 +445,33 @@ class EnrollInCourseTest(TestCase):
# (calling CourseEnrollment.enroll() would have) # (calling CourseEnrollment.enroll() would have)
enrollment = CourseEnrollment.create_enrollment(user, course_id) enrollment = CourseEnrollment.create_enrollment(user, course_id)
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id)) self.assertFalse(CourseEnrollment.is_enrolled(user, course_id))
self.assert_no_events_were_emitted()
# Until you explicitly activate it # Until you explicitly activate it
enrollment.activate() enrollment.activate()
self.assertTrue(CourseEnrollment.is_enrolled(user, course_id)) self.assertTrue(CourseEnrollment.is_enrolled(user, course_id))
self.assert_enrollment_event_was_emitted(user, course_id)
# Activating something that's already active does nothing # Activating something that's already active does nothing
enrollment.activate() enrollment.activate()
self.assertTrue(CourseEnrollment.is_enrolled(user, course_id)) self.assertTrue(CourseEnrollment.is_enrolled(user, course_id))
self.assert_no_events_were_emitted()
# Now deactive # Now deactive
enrollment.deactivate() enrollment.deactivate()
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id)) self.assertFalse(CourseEnrollment.is_enrolled(user, course_id))
self.assert_unenrollment_event_was_emitted(user, course_id)
# Deactivating something that's already inactive does nothing # Deactivating something that's already inactive does nothing
enrollment.deactivate() enrollment.deactivate()
self.assertFalse(CourseEnrollment.is_enrolled(user, course_id)) self.assertFalse(CourseEnrollment.is_enrolled(user, course_id))
self.assert_no_events_were_emitted()
# A deactivated enrollment should be activated if enroll() is called # A deactivated enrollment should be activated if enroll() is called
# for that user/course_id combination # for that user/course_id combination
CourseEnrollment.enroll(user, course_id) CourseEnrollment.enroll(user, course_id)
self.assertTrue(CourseEnrollment.is_enrolled(user, course_id)) self.assertTrue(CourseEnrollment.is_enrolled(user, course_id))
self.assert_enrollment_event_was_emitted(user, course_id)
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
......
"""Generates common contexts""" """Generates common contexts"""
import re import re
import logging
from xmodule.course_module import CourseDescriptor
COURSE_REGEX = re.compile(r'^.*?/courses/(?P<course_id>(?P<org_id>[^/]+)/[^/]+/[^/]+)')
COURSE_REGEX = re.compile(r'^.*?/courses/(?P<course_id>[^/]+/[^/]+/[^/]+)')
log = logging.getLogger(__name__)
def course_context_from_url(url): def course_context_from_url(url):
""" """
Extracts the course_id from the given `url.` Extracts the course_id from the given `url` and passes it on to
`course_context_from_course_id()`.
"""
url = url or ''
match = COURSE_REGEX.match(url)
course_id = ''
if match:
course_id = match.group('course_id') or ''
return course_context_from_course_id(course_id)
def course_context_from_course_id(course_id):
"""
Creates a course context from a `course_id`.
Example Returned Context:: Example Returned Context::
...@@ -18,14 +37,21 @@ def course_context_from_url(url): ...@@ -18,14 +37,21 @@ def course_context_from_url(url):
} }
""" """
url = url or ''
course_id = course_id or ''
context = { context = {
'course_id': '', 'course_id': course_id,
'org_id': '' 'org_id': ''
} }
match = COURSE_REGEX.match(url) try:
if match: location = CourseDescriptor.id_to_location(course_id)
context.update(match.groupdict()) context['org_id'] = location.org
except ValueError:
log.warning(
'Unable to parse course_id "{course_id}"'.format(
course_id=course_id
),
exc_info=True
)
return context return context
...@@ -8,6 +8,7 @@ Feature: LMS.Verified certificates ...@@ -8,6 +8,7 @@ Feature: LMS.Verified certificates
Given I am logged in Given I am logged in
When I select the audit track When I select the audit track
Then I should see the course on my dashboard Then I should see the course on my dashboard
And a "edx.course.enrollment.activated" server event is emitted
Scenario: I can submit photos to verify my identity Scenario: I can submit photos to verify my identity
Given I am logged in Given I am logged in
...@@ -36,6 +37,7 @@ Feature: LMS.Verified certificates ...@@ -36,6 +37,7 @@ Feature: LMS.Verified certificates
Then I see the course on my dashboard Then I see the course on my dashboard
And I see that I am on the verified track And I see that I am on the verified track
And I do not see the upsell link on my dashboard And I do not see the upsell link on my dashboard
And a "edx.course.enrollment.activated" server event is emitted
# Not easily automated # Not easily automated
# Scenario: I can re-take photos # Scenario: I can re-take photos
...@@ -71,6 +73,7 @@ Feature: LMS.Verified certificates ...@@ -71,6 +73,7 @@ Feature: LMS.Verified certificates
And the course has an honor mode And the course has an honor mode
When I give a reason why I cannot pay When I give a reason why I cannot pay
Then I should see the course on my dashboard Then I should see the course on my dashboard
And a "edx.course.enrollment.activated" server event is emitted
Scenario: The upsell offer is on the dashboard if I am auditing. Scenario: The upsell offer is on the dashboard if I am auditing.
Given I am logged in Given I am logged in
...@@ -91,4 +94,5 @@ Feature: LMS.Verified certificates ...@@ -91,4 +94,5 @@ Feature: LMS.Verified certificates
And I navigate to my dashboard And I navigate to my dashboard
Then I see the course on my dashboard Then I see the course on my dashboard
And I see that I am on the verified track And I see that I am on the verified track
And a "edx.course.enrollment.activated" server event is emitted
...@@ -10,6 +10,7 @@ Feature: LMS.Register for a course ...@@ -10,6 +10,7 @@ Feature: LMS.Register for a course
And I visit the courses page And I visit the courses page
When I register for the course "6.002x" When I register for the course "6.002x"
Then I should see the course numbered "6.002x" in my dashboard Then I should see the course numbered "6.002x" in my dashboard
And a "edx.course.enrollment.activated" server event is emitted
Scenario: I can unregister for a course Scenario: I can unregister for a course
Given I am registered for the course "6.002x" Given I am registered for the course "6.002x"
...@@ -19,3 +20,4 @@ Feature: LMS.Register for a course ...@@ -19,3 +20,4 @@ Feature: LMS.Register for a course
Then I should be on the dashboard page Then I should be on the dashboard page
And I should see an empty dashboard message And I should see an empty dashboard message
And I should NOT see the course numbered "6.002x" in my dashboard And I should NOT see the course numbered "6.002x" in my dashboard
And a "edx.course.enrollment.deactivated" server event is emitted
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