Commit 183d6881 by Greg Price

Add events for cohort modifications

This includes cohort creation/deletion and membership changes.
parent 606ad7ab
...@@ -6,16 +6,69 @@ forums, and to the cohort admin views. ...@@ -6,16 +6,69 @@ forums, and to the cohort admin views.
import logging import logging
import random import random
from django.db.models.signals import post_save, m2m_changed
from django.dispatch import receiver
from django.http import Http404 from django.http import Http404
from django.utils.translation import ugettext as _ from django.utils.translation import ugettext as _
from courseware import courses from courseware import courses
from eventtracking import tracker
from student.models import get_user_by_username_or_email from student.models import get_user_by_username_or_email
from .models import CourseUserGroup from .models import CourseUserGroup
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
@receiver(post_save, sender=CourseUserGroup)
def _cohort_added(sender, **kwargs):
"""Emits a tracking log event each time a cohort is created"""
instance = kwargs["instance"]
if kwargs["created"] and instance.group_type == CourseUserGroup.COHORT:
tracker.emit(
"edx.cohort.created",
{"cohort_id": instance.id, "cohort_name": instance.name}
)
@receiver(m2m_changed, sender=CourseUserGroup.users.through)
def _cohort_membership_changed(sender, **kwargs):
"""Emits a tracking log event each time cohort membership is modified"""
def get_event_iter(user_id_iter, cohort_iter):
return (
{"cohort_id": cohort.id, "cohort_name": cohort.name, "user_id": user_id}
for user_id in user_id_iter
for cohort in cohort_iter
)
action = kwargs["action"]
instance = kwargs["instance"]
pk_set = kwargs["pk_set"]
reverse = kwargs["reverse"]
if action == "post_add":
event_name = "edx.cohort.user_added"
elif action in ["post_remove", "pre_clear"]:
event_name = "edx.cohort.user_removed"
else:
return
if reverse:
user_id_iter = [instance.id]
if action == "pre_clear":
cohort_iter = instance.course_groups.filter(group_type=CourseUserGroup.COHORT)
else:
cohort_iter = CourseUserGroup.objects.filter(pk__in=pk_set, group_type=CourseUserGroup.COHORT)
else:
cohort_iter = [instance] if instance.group_type == CourseUserGroup.COHORT else []
if action == "pre_clear":
user_id_iter = (user.id for user in instance.users.all())
else:
user_id_iter = pk_set
for event in get_event_iter(user_id_iter, cohort_iter):
tracker.emit(event_name, event)
# A 'default cohort' is an auto-cohort that is automatically created for a course if no auto_cohort_groups have been # A 'default cohort' is an auto-cohort that is automatically created for a course if no auto_cohort_groups have been
# specified. It is intended to be used in a cohorted-course for users who have yet to be assigned to a cohort. # specified. It is intended to be used in a cohorted-course for users who have yet to be assigned to a cohort.
# Note 1: If an administrator chooses to configure a cohort with the same name, the said cohort will be used as # Note 1: If an administrator chooses to configure a cohort with the same name, the said cohort will be used as
...@@ -258,12 +311,16 @@ def add_cohort(course_key, name): ...@@ -258,12 +311,16 @@ def add_cohort(course_key, name):
except Http404: except Http404:
raise ValueError("Invalid course_key") raise ValueError("Invalid course_key")
return CourseUserGroup.objects.create( cohort = CourseUserGroup.objects.create(
course_id=course.id, course_id=course.id,
group_type=CourseUserGroup.COHORT, group_type=CourseUserGroup.COHORT,
name=name name=name
) )
tracker.emit(
"edx.cohort.creation_requested",
{"cohort_name": cohort.name, "cohort_id": cohort.id}
)
return cohort
def add_user_to_cohort(cohort, username_or_email): def add_user_to_cohort(cohort, username_or_email):
""" """
...@@ -281,7 +338,8 @@ def add_user_to_cohort(cohort, username_or_email): ...@@ -281,7 +338,8 @@ def add_user_to_cohort(cohort, username_or_email):
ValueError if user already present in this cohort. ValueError if user already present in this cohort.
""" """
user = get_user_by_username_or_email(username_or_email) user = get_user_by_username_or_email(username_or_email)
previous_cohort = None previous_cohort_name = None
previous_cohort_id = None
course_cohorts = CourseUserGroup.objects.filter( course_cohorts = CourseUserGroup.objects.filter(
course_id=cohort.course_id, course_id=cohort.course_id,
...@@ -295,8 +353,20 @@ def add_user_to_cohort(cohort, username_or_email): ...@@ -295,8 +353,20 @@ def add_user_to_cohort(cohort, username_or_email):
cohort_name=cohort.name cohort_name=cohort.name
)) ))
else: else:
previous_cohort = course_cohorts[0].name previous_cohort = course_cohorts[0]
course_cohorts[0].users.remove(user) previous_cohort.users.remove(user)
previous_cohort_name = previous_cohort.name
previous_cohort_id = previous_cohort.id
tracker.emit(
"edx.cohort.user_add_requested",
{
"user_id": user.id,
"cohort_id": cohort.id,
"cohort_name": cohort.name,
"previous_cohort_id": previous_cohort_id,
"previous_cohort_name": previous_cohort_name,
}
)
cohort.users.add(user) cohort.users.add(user)
return (user, previous_cohort) return (user, previous_cohort_name)
...@@ -4,6 +4,7 @@ Helper methods for testing cohorts. ...@@ -4,6 +4,7 @@ Helper methods for testing cohorts.
from factory import post_generation, Sequence from factory import post_generation, Sequence
from factory.django import DjangoModelFactory from factory.django import DjangoModelFactory
from course_groups.models import CourseUserGroup from course_groups.models import CourseUserGroup
from opaque_keys.edx.locations import SlashSeparatedCourseKey
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore import ModuleStoreEnum
...@@ -15,7 +16,7 @@ class CohortFactory(DjangoModelFactory): ...@@ -15,7 +16,7 @@ class CohortFactory(DjangoModelFactory):
FACTORY_FOR = CourseUserGroup FACTORY_FOR = CourseUserGroup
name = Sequence("cohort{}".format) name = Sequence("cohort{}".format)
course_id = "dummy_id" course_id = SlashSeparatedCourseKey("dummy", "dummy", "dummy")
group_type = CourseUserGroup.COHORT group_type = CourseUserGroup.COHORT
@post_generation @post_generation
......
...@@ -4,6 +4,7 @@ from django.conf import settings ...@@ -4,6 +4,7 @@ from django.conf import settings
from django.http import Http404 from django.http import Http404
from django.test.utils import override_settings from django.test.utils import override_settings
from mock import call, patch
from student.models import CourseEnrollment from student.models import CourseEnrollment
from student.tests.factories import UserFactory from student.tests.factories import UserFactory
...@@ -25,6 +26,103 @@ TEST_MAPPING = {'edX/toy/2012_Fall': 'xml'} ...@@ -25,6 +26,103 @@ TEST_MAPPING = {'edX/toy/2012_Fall': 'xml'}
TEST_DATA_MIXED_MODULESTORE = mixed_store_config(TEST_DATA_DIR, TEST_MAPPING) TEST_DATA_MIXED_MODULESTORE = mixed_store_config(TEST_DATA_DIR, TEST_MAPPING)
@patch("course_groups.cohorts.tracker")
class TestCohortSignals(django.test.TestCase):
def setUp(self):
self.course_key = SlashSeparatedCourseKey("dummy", "dummy", "dummy")
def test_cohort_added(self, mock_tracker):
# Add cohort
cohort = CourseUserGroup.objects.create(
name="TestCohort",
course_id=self.course_key,
group_type=CourseUserGroup.COHORT
)
mock_tracker.emit.assert_called_with(
"edx.cohort.created",
{"cohort_id": cohort.id, "cohort_name": cohort.name}
)
mock_tracker.reset_mock()
# Modify existing cohort
cohort.name = "NewName"
cohort.save()
self.assertFalse(mock_tracker.called)
# Add non-cohort group
CourseUserGroup.objects.create(
name="TestOtherGroupType",
course_id=self.course_key,
group_type="dummy"
)
self.assertFalse(mock_tracker.called)
def test_cohort_membership_changed(self, mock_tracker):
cohort_list = [CohortFactory() for _ in range(2)]
non_cohort = CourseUserGroup.objects.create(
name="dummy",
course_id=self.course_key,
group_type="dummy"
)
user_list = [UserFactory() for _ in range(2)]
mock_tracker.reset_mock()
def assert_events(event_name_suffix, user_list, cohort_list):
mock_tracker.emit.assert_has_calls([
call(
"edx.cohort.user_" + event_name_suffix,
{
"user_id": user.id,
"cohort_id": cohort.id,
"cohort_name": cohort.name,
}
)
for user in user_list for cohort in cohort_list
])
# Add users to cohort
cohort_list[0].users.add(*user_list)
assert_events("added", user_list, cohort_list[:1])
mock_tracker.reset_mock()
# Remove users from cohort
cohort_list[0].users.remove(*user_list)
assert_events("removed", user_list, cohort_list[:1])
mock_tracker.reset_mock()
# Clear users from cohort
cohort_list[0].users.add(*user_list)
cohort_list[0].users.clear()
assert_events("removed", user_list, cohort_list[:1])
mock_tracker.reset_mock()
# Clear users from non-cohort group
non_cohort.users.add(*user_list)
non_cohort.users.clear()
self.assertFalse(mock_tracker.emit.called)
# Add cohorts to user
user_list[0].course_groups.add(*cohort_list)
assert_events("added", user_list[:1], cohort_list)
mock_tracker.reset_mock()
# Remove cohorts from user
user_list[0].course_groups.remove(*cohort_list)
assert_events("removed", user_list[:1], cohort_list)
mock_tracker.reset_mock()
# Clear cohorts from user
user_list[0].course_groups.add(*cohort_list)
user_list[0].course_groups.clear()
assert_events("removed", user_list[:1], cohort_list)
mock_tracker.reset_mock()
# Clear non-cohort groups from user
user_list[0].course_groups.add(non_cohort)
user_list[0].course_groups.clear()
self.assertFalse(mock_tracker.emit.called)
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
class TestCohorts(django.test.TestCase): class TestCohorts(django.test.TestCase):
...@@ -354,13 +452,18 @@ class TestCohorts(django.test.TestCase): ...@@ -354,13 +452,18 @@ class TestCohorts(django.test.TestCase):
lambda: cohorts.get_cohort_by_id(course.id, cohort.id) lambda: cohorts.get_cohort_by_id(course.id, cohort.id)
) )
def test_add_cohort(self): @patch("course_groups.cohorts.tracker")
def test_add_cohort(self, mock_tracker):
""" """
Make sure cohorts.add_cohort() properly adds a cohort to a course and handles Make sure cohorts.add_cohort() properly adds a cohort to a course and handles
errors. errors.
""" """
course = modulestore().get_course(self.toy_course_key) course = modulestore().get_course(self.toy_course_key)
added_cohort = cohorts.add_cohort(course.id, "My Cohort") added_cohort = cohorts.add_cohort(course.id, "My Cohort")
mock_tracker.emit.assert_any_call(
"edx.cohort.creation_requested",
{"cohort_name": added_cohort.name, "cohort_id": added_cohort.id}
)
self.assertEqual(added_cohort.name, "My Cohort") self.assertEqual(added_cohort.name, "My Cohort")
self.assertRaises( self.assertRaises(
...@@ -372,7 +475,8 @@ class TestCohorts(django.test.TestCase): ...@@ -372,7 +475,8 @@ class TestCohorts(django.test.TestCase):
lambda: cohorts.add_cohort(SlashSeparatedCourseKey("course", "does_not", "exist"), "My Cohort") lambda: cohorts.add_cohort(SlashSeparatedCourseKey("course", "does_not", "exist"), "My Cohort")
) )
def test_add_user_to_cohort(self): @patch("course_groups.cohorts.tracker")
def test_add_user_to_cohort(self, mock_tracker):
""" """
Make sure cohorts.add_user_to_cohort() properly adds a user to a cohort and Make sure cohorts.add_user_to_cohort() properly adds a user to a cohort and
handles errors. handles errors.
...@@ -390,13 +494,32 @@ class TestCohorts(django.test.TestCase): ...@@ -390,13 +494,32 @@ class TestCohorts(django.test.TestCase):
cohorts.add_user_to_cohort(first_cohort, "Username"), cohorts.add_user_to_cohort(first_cohort, "Username"),
(course_user, None) (course_user, None)
) )
mock_tracker.emit.assert_any_call(
"edx.cohort.user_add_requested",
{
"user_id": course_user.id,
"cohort_id": first_cohort.id,
"cohort_name": first_cohort.name,
"previous_cohort_id": None,
"previous_cohort_name": None,
}
)
# Should get (user, previous_cohort_name) when moved from one cohort to # Should get (user, previous_cohort_name) when moved from one cohort to
# another # another
self.assertEqual( self.assertEqual(
cohorts.add_user_to_cohort(second_cohort, "Username"), cohorts.add_user_to_cohort(second_cohort, "Username"),
(course_user, "FirstCohort") (course_user, "FirstCohort")
) )
mock_tracker.emit.assert_any_call(
"edx.cohort.user_add_requested",
{
"user_id": course_user.id,
"cohort_id": second_cohort.id,
"cohort_name": second_cohort.name,
"previous_cohort_id": first_cohort.id,
"previous_cohort_name": first_cohort.name,
}
)
# Error cases # Error cases
# Should get ValueError if user already in cohort # Should get ValueError if user already in cohort
self.assertRaises( self.assertRaises(
......
...@@ -3,6 +3,10 @@ ...@@ -3,6 +3,10 @@
End-to-end tests related to the cohort management on the LMS Instructor Dashboard End-to-end tests related to the cohort management on the LMS Instructor Dashboard
""" """
from datetime import datetime
from pymongo import MongoClient
from bok_choy.promise import EmptyPromise from bok_choy.promise import EmptyPromise
from .helpers import CohortTestMixin from .helpers import CohortTestMixin
from ..helpers import UniqueCourseTest from ..helpers import UniqueCourseTest
...@@ -25,6 +29,8 @@ class CohortConfigurationTest(UniqueCourseTest, CohortTestMixin): ...@@ -25,6 +29,8 @@ class CohortConfigurationTest(UniqueCourseTest, CohortTestMixin):
""" """
super(CohortConfigurationTest, self).setUp() super(CohortConfigurationTest, self).setUp()
self.event_collection = MongoClient()["test"]["events"]
# create course with cohorts # create course with cohorts
self.manual_cohort_name = "ManualCohort1" self.manual_cohort_name = "ManualCohort1"
self.auto_cohort_name = "AutoCohort1" self.auto_cohort_name = "AutoCohort1"
...@@ -106,7 +112,9 @@ class CohortConfigurationTest(UniqueCourseTest, CohortTestMixin): ...@@ -106,7 +112,9 @@ class CohortConfigurationTest(UniqueCourseTest, CohortTestMixin):
And I get a notification that 2 users have been added to the cohort And I get a notification that 2 users have been added to the cohort
And I get a notification that 1 user was moved from the other cohort And I get a notification that 1 user was moved from the other cohort
And the user input field is empty And the user input field is empty
And appropriate events have been emitted
""" """
start_time = datetime.now()
self.membership_page.select_cohort(self.auto_cohort_name) self.membership_page.select_cohort(self.auto_cohort_name)
self.assertEqual(0, self.membership_page.get_selected_cohort_count()) self.assertEqual(0, self.membership_page.get_selected_cohort_count())
self.membership_page.add_students_to_selected_cohort([self.student_name, self.instructor_name]) self.membership_page.add_students_to_selected_cohort([self.student_name, self.instructor_name])
...@@ -119,6 +127,44 @@ class CohortConfigurationTest(UniqueCourseTest, CohortTestMixin): ...@@ -119,6 +127,44 @@ class CohortConfigurationTest(UniqueCourseTest, CohortTestMixin):
self.assertEqual("2 students have been added to this cohort group", confirmation_messages[0]) self.assertEqual("2 students have been added to this cohort group", confirmation_messages[0])
self.assertEqual("1 student was removed from " + self.manual_cohort_name, confirmation_messages[1]) self.assertEqual("1 student was removed from " + self.manual_cohort_name, confirmation_messages[1])
self.assertEqual("", self.membership_page.get_cohort_student_input_field_value()) self.assertEqual("", self.membership_page.get_cohort_student_input_field_value())
self.assertEqual(
self.event_collection.find({
"name": "edx.cohort.user_added",
"time": {"$gt": start_time},
"event.user_id": {"$in": [int(self.instructor_id), int(self.student_id)]},
"event.cohort_name": self.auto_cohort_name,
}).count(),
2
)
self.assertEqual(
self.event_collection.find({
"name": "edx.cohort.user_removed",
"time": {"$gt": start_time},
"event.user_id": int(self.student_id),
"event.cohort_name": self.manual_cohort_name,
}).count(),
1
)
self.assertEqual(
self.event_collection.find({
"name": "edx.cohort.user_add_requested",
"time": {"$gt": start_time},
"event.user_id": int(self.instructor_id),
"event.cohort_name": self.auto_cohort_name,
"event.previous_cohort_name": None,
}).count(),
1
)
self.assertEqual(
self.event_collection.find({
"name": "edx.cohort.user_add_requested",
"time": {"$gt": start_time},
"event.user_id": int(self.student_id),
"event.cohort_name": self.auto_cohort_name,
"event.previous_cohort_name": self.manual_cohort_name,
}).count(),
1
)
def test_add_students_to_cohort_failure(self): def test_add_students_to_cohort_failure(self):
""" """
...@@ -164,7 +210,9 @@ class CohortConfigurationTest(UniqueCourseTest, CohortTestMixin): ...@@ -164,7 +210,9 @@ class CohortConfigurationTest(UniqueCourseTest, CohortTestMixin):
Then the new cohort is displayed and has no users in it Then the new cohort is displayed and has no users in it
And when I add the user to the new cohort And when I add the user to the new cohort
Then the cohort has 1 user Then the cohort has 1 user
And appropriate events have been emitted
""" """
start_time = datetime.now()
new_cohort = str(uuid.uuid4().get_hex()[0:20]) new_cohort = str(uuid.uuid4().get_hex()[0:20])
self.assertFalse(new_cohort in self.membership_page.get_cohorts()) self.assertFalse(new_cohort in self.membership_page.get_cohorts())
self.membership_page.add_cohort(new_cohort) self.membership_page.add_cohort(new_cohort)
...@@ -178,3 +226,19 @@ class CohortConfigurationTest(UniqueCourseTest, CohortTestMixin): ...@@ -178,3 +226,19 @@ class CohortConfigurationTest(UniqueCourseTest, CohortTestMixin):
EmptyPromise( EmptyPromise(
lambda: 1 == self.membership_page.get_selected_cohort_count(), 'Waiting for student to be added' lambda: 1 == self.membership_page.get_selected_cohort_count(), 'Waiting for student to be added'
).fulfill() ).fulfill()
self.assertEqual(
self.event_collection.find({
"name": "edx.cohort.created",
"time": {"$gt": start_time},
"event.cohort_name": new_cohort,
}).count(),
1
)
self.assertEqual(
self.event_collection.find({
"name": "edx.cohort.creation_requested",
"time": {"$gt": start_time},
"event.cohort_name": new_cohort,
}).count(),
1
)
gprice@gprice-mbp.local.3450
\ No newline at end of file
...@@ -46,6 +46,15 @@ ...@@ -46,6 +46,15 @@
"port": 27017, "port": 27017,
"user": "edxapp" "user": "edxapp"
}, },
"EVENT_TRACKING_BACKENDS": {
"mongo": {
"ENGINE": "eventtracking.backends.mongodb.MongoBackend",
"OPTIONS": {
"database": "test",
"collection": "events"
}
}
},
"MODULESTORE": { "MODULESTORE": {
"default": { "default": {
"ENGINE": "xmodule.modulestore.mixed.MixedModuleStore", "ENGINE": "xmodule.modulestore.mixed.MixedModuleStore",
......
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