diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 794c6ef7b9f203b4b44e422c48b5b17e6e82d586..6387f282e15352932ecc03d3b1f5f240c8e11125 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -29,6 +29,12 @@ from django.forms import ModelForm, forms from course_modes.models import CourseMode import comment_client as cc 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"]) @@ -667,6 +673,11 @@ class PendingEmailChange(models.Model): 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): """ Represents a Student's Enrollment record for a single Course. You should @@ -737,19 +748,55 @@ class CourseEnrollment(models.Model): if user.id is None: user.save() - enrollment, _ = CourseEnrollment.objects.get_or_create( + enrollment, created = CourseEnrollment.objects.get_or_create( user=user, course_id=course_id, ) - # In case we're reactivating a deactivated enrollment, or changing the - # enrollment mode. - if enrollment.mode != mode or enrollment.is_active != is_active: - enrollment.mode = mode + + activation_changed = False + if 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() + 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 + 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 def enroll(cls, user, course_id, mode="honor"): """ @@ -825,9 +872,13 @@ class CourseEnrollment(models.Model): """ try: record = CourseEnrollment.objects.get(user=user, course_id=course_id) - record.is_active = False - record.save() - unenroll_done.send(sender=cls, course_enrollment=record) + if record.is_active: + record.is_active = False + record.save() + + record.emit_event(EVENT_NAME_ENROLLMENT_DEACTIVATED) + unenroll_done.send(sender=cls, course_enrollment=record) + except cls.DoesNotExist: err_msg = u"Tried to unenroll student {} from {} but they were not enrolled" log.error(err_msg.format(user, course_id)) @@ -918,6 +969,7 @@ class CourseEnrollment(models.Model): if not self.is_active: self.is_active = True self.save() + self.emit_event(EVENT_NAME_ENROLLMENT_ACTIVATED) def deactivate(self): """Makes this `CourseEnrollment` record inactive. Saves immediately. An @@ -926,6 +978,7 @@ class CourseEnrollment(models.Model): if self.is_active: self.is_active = False self.save() + self.emit_event(EVENT_NAME_ENROLLMENT_DEACTIVATED) def refundable(self): """ diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index 41a95ff13ad0b420d1a7e9c75104496ea5ae0256..634996fd55cf16ff5b12c2e22dce25e3d176f5bf 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -25,7 +25,7 @@ from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase 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 student.models import unique_id_for_user, CourseEnrollment @@ -276,6 +276,16 @@ class DashboardTest(TestCase): class EnrollInCourseTest(TestCase): """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): user = User.objects.create_user("joe", "joe@joe.com", "password") course_id = "edX/Test101/2013" @@ -289,24 +299,28 @@ class EnrollInCourseTest(TestCase): self.assertTrue(CourseEnrollment.is_enrolled(user, course_id)) self.assertTrue(CourseEnrollment.is_enrolled_by_partial(user, course_id_partial)) + self.assert_enrollment_event_was_emitted(user, course_id) # Enrolling them again should be harmless CourseEnrollment.enroll(user, course_id) self.assertTrue(CourseEnrollment.is_enrolled(user, course_id)) self.assertTrue(CourseEnrollment.is_enrolled_by_partial(user, course_id_partial)) + self.assert_no_events_were_emitted() # Now unenroll the user CourseEnrollment.unenroll(user, course_id) self.assertFalse(CourseEnrollment.is_enrolled(user, course_id)) self.assertFalse(CourseEnrollment.is_enrolled_by_partial(user, course_id_partial)) + self.assert_unenrollment_event_was_emitted(user, course_id) # Unenrolling them again should also be harmless CourseEnrollment.unenroll(user, course_id) self.assertFalse(CourseEnrollment.is_enrolled(user, course_id)) self.assertFalse(CourseEnrollment.is_enrolled_by_partial(user, course_id_partial)) + self.assert_no_events_were_emitted() # The enrollment record should still exist, just be inactive enrollment_record = CourseEnrollment.objects.get( @@ -315,6 +329,37 @@ class EnrollInCourseTest(TestCase): ) 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): # Testing enrollment of newly unsaved user (i.e. no database entry) user = User(username="rusty", email="rusty@fake.edx.org") @@ -324,11 +369,13 @@ class EnrollInCourseTest(TestCase): # Unenroll does nothing CourseEnrollment.unenroll(user, course_id) + self.assert_no_events_were_emitted() # Implicit save() happens on new User object when enrolling, so this # should still work CourseEnrollment.enroll(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): user = User.objects.create(username="jack", email="jack@fake.edx.org") @@ -336,11 +383,13 @@ class EnrollInCourseTest(TestCase): CourseEnrollment.enroll_by_email("jack@fake.edx.org", 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 self.assertIsNone( CourseEnrollment.enroll_by_email("not_jack@fake.edx.org", course_id) ) + self.assert_no_events_were_emitted() self.assertRaises( User.DoesNotExist, @@ -349,17 +398,21 @@ class EnrollInCourseTest(TestCase): course_id, ignore_errors=False ) + self.assert_no_events_were_emitted() # Now unenroll them by email CourseEnrollment.unenroll_by_email("jack@fake.edx.org", course_id) self.assertFalse(CourseEnrollment.is_enrolled(user, course_id)) + self.assert_unenrollment_event_was_emitted(user, course_id) # Harmless second unenroll CourseEnrollment.unenroll_by_email("jack@fake.edx.org", 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 CourseEnrollment.unenroll_by_email("not_jack@fake.edx.org", course_id) + self.assert_no_events_were_emitted() def test_enrollment_multiple_classes(self): user = User(username="rusty", email="rusty@fake.edx.org") @@ -367,15 +420,19 @@ class EnrollInCourseTest(TestCase): course_id2 = "MITx/6.003z/2012" CourseEnrollment.enroll(user, course_id1) + self.assert_enrollment_event_was_emitted(user, course_id1) 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_id2)) CourseEnrollment.unenroll(user, course_id1) + self.assert_unenrollment_event_was_emitted(user, course_id1) self.assertFalse(CourseEnrollment.is_enrolled(user, course_id1)) self.assertTrue(CourseEnrollment.is_enrolled(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_id2)) @@ -388,27 +445,33 @@ class EnrollInCourseTest(TestCase): # (calling CourseEnrollment.enroll() would have) enrollment = CourseEnrollment.create_enrollment(user, course_id) self.assertFalse(CourseEnrollment.is_enrolled(user, course_id)) + self.assert_no_events_were_emitted() # Until you explicitly activate it enrollment.activate() 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 enrollment.activate() self.assertTrue(CourseEnrollment.is_enrolled(user, course_id)) + self.assert_no_events_were_emitted() # Now deactive enrollment.deactivate() 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 enrollment.deactivate() self.assertFalse(CourseEnrollment.is_enrolled(user, course_id)) + self.assert_no_events_were_emitted() # A deactivated enrollment should be activated if enroll() is called # for that user/course_id combination CourseEnrollment.enroll(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) diff --git a/common/djangoapps/track/contexts.py b/common/djangoapps/track/contexts.py index d87d2bbf3be4f7ba043a5a57d391b9689bc00217..90a14736b4df263b5ac23d0f42661b7f83c65fc2 100644 --- a/common/djangoapps/track/contexts.py +++ b/common/djangoapps/track/contexts.py @@ -1,14 +1,33 @@ """Generates common contexts""" 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): """ - 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:: @@ -18,14 +37,21 @@ def course_context_from_url(url): } """ - url = url or '' + course_id = course_id or '' context = { - 'course_id': '', + 'course_id': course_id, 'org_id': '' } - match = COURSE_REGEX.match(url) - if match: - context.update(match.groupdict()) + try: + location = CourseDescriptor.id_to_location(course_id) + 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 diff --git a/lms/djangoapps/courseware/features/certificates.feature b/lms/djangoapps/courseware/features/certificates.feature index 16eca8aabd2a477e8e0512dac63ec9cedda78704..4a4ceb44b103be62eb8a88dc7bae5227717e7e51 100644 --- a/lms/djangoapps/courseware/features/certificates.feature +++ b/lms/djangoapps/courseware/features/certificates.feature @@ -8,6 +8,7 @@ Feature: LMS.Verified certificates Given I am logged in When I select the audit track 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 Given I am logged in @@ -36,6 +37,7 @@ Feature: LMS.Verified certificates Then I see the course on my dashboard And I see that I am on the verified track And I do not see the upsell link on my dashboard + And a "edx.course.enrollment.activated" server event is emitted # Not easily automated # Scenario: I can re-take photos @@ -71,6 +73,7 @@ Feature: LMS.Verified certificates And the course has an honor mode When I give a reason why I cannot pay 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. Given I am logged in @@ -91,4 +94,5 @@ Feature: LMS.Verified certificates And I navigate to my dashboard Then I see the course on my dashboard And I see that I am on the verified track + And a "edx.course.enrollment.activated" server event is emitted diff --git a/lms/djangoapps/courseware/features/registration.feature b/lms/djangoapps/courseware/features/registration.feature index c779462db35564996295ec49959275db2dd5c020..807c3d8cc511c4c03683627201a92ec247b2a0d4 100644 --- a/lms/djangoapps/courseware/features/registration.feature +++ b/lms/djangoapps/courseware/features/registration.feature @@ -10,6 +10,7 @@ Feature: LMS.Register for a course And I visit the courses page When I register for the course "6.002x" 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 Given I am registered for the course "6.002x" @@ -19,3 +20,4 @@ Feature: LMS.Register for a course Then I should be on the dashboard page And I should see an empty dashboard message And I should NOT see the course numbered "6.002x" in my dashboard + And a "edx.course.enrollment.deactivated" server event is emitted