Merge pull request #8847 from edx/ziafazal/SOL-1021
ziafazal/SOL-1021: Create Analytics events for certificate actions
This commit is contained in:
@@ -33,6 +33,7 @@ from django.views.decorators.http import require_http_methods
|
||||
from contentstore.utils import reverse_course_url
|
||||
from edxmako.shortcuts import render_to_response
|
||||
from opaque_keys.edx.keys import CourseKey, AssetKey
|
||||
from eventtracking import tracker
|
||||
from student.auth import has_studio_write_access
|
||||
from util.db import generate_int_id, MYSQL_MAX_INT
|
||||
from util.json_request import JsonResponse
|
||||
@@ -247,6 +248,20 @@ class CertificateManager(object):
|
||||
store.update_item(course, request.user.id)
|
||||
break
|
||||
|
||||
@staticmethod
|
||||
def track_event(event_name, event_data):
|
||||
"""Track certificate configuration event.
|
||||
|
||||
Arguments:
|
||||
event_name (str): Name of the event to be logged.
|
||||
event_data (dict): A Dictionary containing event data
|
||||
Returns:
|
||||
None
|
||||
|
||||
"""
|
||||
event_name = '.'.join(['edx', 'certificate', 'configuration', event_name])
|
||||
tracker.emit(event_name, event_data)
|
||||
|
||||
|
||||
class Certificate(object):
|
||||
"""
|
||||
@@ -296,6 +311,10 @@ def certificate_activation_handler(request, course_key_string):
|
||||
break
|
||||
|
||||
store.update_item(course, request.user.id)
|
||||
cert_event_type = 'activated' if is_active else 'deactivated'
|
||||
CertificateManager.track_event(cert_event_type, {
|
||||
'course_id': unicode(course.id),
|
||||
})
|
||||
return HttpResponse(status=200)
|
||||
|
||||
|
||||
@@ -375,6 +394,10 @@ def certificates_list_handler(request, course_key_string):
|
||||
kwargs={'certificate_id': new_certificate.id} # pylint: disable=no-member
|
||||
)
|
||||
store.update_item(course, request.user.id)
|
||||
CertificateManager.track_event('created', {
|
||||
'course_id': unicode(course.id),
|
||||
'configuration_id': new_certificate.id
|
||||
})
|
||||
course = _get_course_and_check_access(course_key, request.user)
|
||||
return response
|
||||
else:
|
||||
@@ -414,12 +437,18 @@ def certificates_detail_handler(request, course_key_string, certificate_id):
|
||||
return JsonResponse({"error": err.message}, status=400)
|
||||
|
||||
serialized_certificate = CertificateManager.serialize_certificate(new_certificate)
|
||||
cert_event_type = 'created'
|
||||
if match_cert:
|
||||
cert_event_type = 'modified'
|
||||
certificates_list[match_index] = serialized_certificate
|
||||
else:
|
||||
certificates_list.append(serialized_certificate)
|
||||
|
||||
store.update_item(course, request.user.id)
|
||||
CertificateManager.track_event(cert_event_type, {
|
||||
'course_id': unicode(course.id),
|
||||
'configuration_id': serialized_certificate["id"]
|
||||
})
|
||||
return JsonResponse(serialized_certificate, status=201)
|
||||
|
||||
elif request.method == "DELETE":
|
||||
@@ -431,6 +460,10 @@ def certificates_detail_handler(request, course_key_string, certificate_id):
|
||||
course=course,
|
||||
certificate_id=certificate_id
|
||||
)
|
||||
CertificateManager.track_event('deleted', {
|
||||
'course_id': unicode(course.id),
|
||||
'configuration_id': certificate_id
|
||||
})
|
||||
return JsonResponse(status=204)
|
||||
|
||||
|
||||
|
||||
@@ -24,6 +24,7 @@ from student.tests.factories import UserFactory
|
||||
from contentstore.views.certificates import CertificateManager
|
||||
from django.test.utils import override_settings
|
||||
from contentstore.utils import get_lms_link_for_certificate_web_view
|
||||
from util.testing import EventTestMixin
|
||||
|
||||
FEATURES_WITH_CERTS_ENABLED = settings.FEATURES.copy()
|
||||
FEATURES_WITH_CERTS_ENABLED['CERTIFICATES_HTML_VIEW'] = True
|
||||
@@ -194,7 +195,7 @@ class CertificatesBaseTestCase(object):
|
||||
|
||||
# pylint: disable=no-member
|
||||
@override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED)
|
||||
class CertificatesListHandlerTestCase(CourseTestCase, CertificatesBaseTestCase, HelperMethods):
|
||||
class CertificatesListHandlerTestCase(EventTestMixin, CourseTestCase, CertificatesBaseTestCase, HelperMethods):
|
||||
"""
|
||||
Test cases for certificates_list_handler.
|
||||
"""
|
||||
@@ -202,7 +203,7 @@ class CertificatesListHandlerTestCase(CourseTestCase, CertificatesBaseTestCase,
|
||||
"""
|
||||
Set up CertificatesListHandlerTestCase.
|
||||
"""
|
||||
super(CertificatesListHandlerTestCase, self).setUp()
|
||||
super(CertificatesListHandlerTestCase, self).setUp('contentstore.views.certificates.tracker')
|
||||
|
||||
def _url(self):
|
||||
"""
|
||||
@@ -229,8 +230,13 @@ class CertificatesListHandlerTestCase(CourseTestCase, CertificatesBaseTestCase,
|
||||
self.assertEqual(response.status_code, 201)
|
||||
self.assertIn("Location", response)
|
||||
content = json.loads(response.content)
|
||||
self._remove_ids(content) # pylint: disable=unused-variable
|
||||
certificate_id = self._remove_ids(content) # pylint: disable=unused-variable
|
||||
self.assertEqual(content, expected)
|
||||
self.assert_event_emitted(
|
||||
'edx.certificate.configuration.created',
|
||||
course_id=unicode(self.course.id),
|
||||
configuration_id=certificate_id,
|
||||
)
|
||||
|
||||
def test_cannot_create_certificate_if_user_has_no_write_permissions(self):
|
||||
"""
|
||||
@@ -347,13 +353,19 @@ class CertificatesListHandlerTestCase(CourseTestCase, CertificatesBaseTestCase,
|
||||
|
||||
@ddt.ddt
|
||||
@override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED)
|
||||
class CertificatesDetailHandlerTestCase(CourseTestCase, CertificatesBaseTestCase, HelperMethods):
|
||||
class CertificatesDetailHandlerTestCase(EventTestMixin, CourseTestCase, CertificatesBaseTestCase, HelperMethods):
|
||||
"""
|
||||
Test cases for CertificatesDetailHandlerTestCase.
|
||||
"""
|
||||
|
||||
_id = 0
|
||||
|
||||
def setUp(self): # pylint: disable=arguments-differ
|
||||
"""
|
||||
Set up CertificatesDetailHandlerTestCase.
|
||||
"""
|
||||
super(CertificatesDetailHandlerTestCase, self).setUp('contentstore.views.certificates.tracker')
|
||||
|
||||
def _url(self, cid=-1):
|
||||
"""
|
||||
Return url for the handler.
|
||||
@@ -388,6 +400,11 @@ class CertificatesDetailHandlerTestCase(CourseTestCase, CertificatesBaseTestCase
|
||||
)
|
||||
content = json.loads(response.content)
|
||||
self.assertEqual(content, expected)
|
||||
self.assert_event_emitted(
|
||||
'edx.certificate.configuration.created',
|
||||
course_id=unicode(self.course.id),
|
||||
configuration_id=666,
|
||||
)
|
||||
|
||||
def test_can_edit_certificate(self):
|
||||
"""
|
||||
@@ -415,6 +432,11 @@ class CertificatesDetailHandlerTestCase(CourseTestCase, CertificatesBaseTestCase
|
||||
)
|
||||
content = json.loads(response.content)
|
||||
self.assertEqual(content, expected)
|
||||
self.assert_event_emitted(
|
||||
'edx.certificate.configuration.modified',
|
||||
course_id=unicode(self.course.id),
|
||||
configuration_id=1,
|
||||
)
|
||||
self.reload_course()
|
||||
|
||||
# Verify that certificate is properly updated in the course.
|
||||
@@ -440,6 +462,11 @@ class CertificatesDetailHandlerTestCase(CourseTestCase, CertificatesBaseTestCase
|
||||
HTTP_X_REQUESTED_WITH="XMLHttpRequest",
|
||||
)
|
||||
self.assertEqual(response.status_code, 204)
|
||||
self.assert_event_emitted(
|
||||
'edx.certificate.configuration.deleted',
|
||||
course_id=unicode(self.course.id),
|
||||
configuration_id='1',
|
||||
)
|
||||
self.reload_course()
|
||||
# Verify that certificates are properly updated in the course.
|
||||
certificates = self.course.certificates['certificates']
|
||||
@@ -553,6 +580,11 @@ class CertificatesDetailHandlerTestCase(CourseTestCase, CertificatesBaseTestCase
|
||||
course = self.store.get_course(self.course.id)
|
||||
certificates = course.certificates['certificates']
|
||||
self.assertEqual(certificates[0].get('is_active'), is_active)
|
||||
cert_event_type = 'activated' if is_active else 'deactivated'
|
||||
self.assert_event_emitted(
|
||||
'.'.join(['edx.certificate.configuration', cert_event_type]),
|
||||
course_id=unicode(self.course.id),
|
||||
)
|
||||
|
||||
@ddt.data(True, False)
|
||||
def test_certificate_activation_without_write_permissions(self, activate):
|
||||
|
||||
@@ -79,8 +79,18 @@ class CertificatesPage(CoursePage):
|
||||
We can't use confirm_prompt because its wait_for_notification is flaky when asynchronous operation
|
||||
completed very quickly.
|
||||
"""
|
||||
self.wait_for_element_visibility('.prompt', 'Prompt is visible')
|
||||
self.wait_for_element_visibility('.prompt .action-primary', 'Confirmation button is visible')
|
||||
EmptyPromise(
|
||||
lambda: self.q(css='.prompt').present,
|
||||
'Confirmation prompt is displayed'
|
||||
).fulfill()
|
||||
EmptyPromise(
|
||||
lambda: self.q(css='.prompt .action-primary').present,
|
||||
'Primary button is displayed'
|
||||
).fulfill()
|
||||
EmptyPromise(
|
||||
lambda: self.q(css='.prompt .action-primary').visible,
|
||||
'Primary button is visible'
|
||||
).fulfill()
|
||||
|
||||
def wait_for_first_certificate_button(self):
|
||||
"""
|
||||
@@ -108,17 +118,23 @@ class CertificatesPage(CoursePage):
|
||||
"""
|
||||
Clicks the 'Create your first certificate' button, which is only displayed at zero state
|
||||
"""
|
||||
self.wait_for_first_certificate_button()
|
||||
self.q(css=self.certficate_css + " .new-button").first.click()
|
||||
|
||||
def click_add_certificate_button(self):
|
||||
"""
|
||||
Clicks the 'Add new certificate' button, which is displayed when certificates already exist
|
||||
"""
|
||||
self.wait_for_add_certificate_button()
|
||||
self.q(css=self.certficate_css + " .action-add").first.click()
|
||||
|
||||
################
|
||||
# Workflows
|
||||
################
|
||||
def click_confirmation_prompt_primary_button(self):
|
||||
"""
|
||||
Clicks the main action presented by the prompt (such as 'Delete')
|
||||
"""
|
||||
self.wait_for_confirmation_prompt()
|
||||
self.q(css='a.button.action-primary').first.click()
|
||||
self.wait_for_ajax()
|
||||
|
||||
|
||||
class Certificate(object):
|
||||
@@ -240,13 +256,19 @@ class Certificate(object):
|
||||
"""
|
||||
Returns whether or not the certificate delete icon is present.
|
||||
"""
|
||||
return self.find_css('.actions .delete').present
|
||||
EmptyPromise(
|
||||
lambda: self.find_css('.actions .delete').present,
|
||||
'Certificate delete button is displayed'
|
||||
).fulfill()
|
||||
|
||||
def wait_for_hide_details_toggle(self):
|
||||
"""
|
||||
Certificate details are expanded.
|
||||
"""
|
||||
return self.find_css('a.detail-toggle.hide-details').present
|
||||
EmptyPromise(
|
||||
lambda: self.find_css('a.detail-toggle.hide-details').present,
|
||||
'Certificate details are expanded'
|
||||
).fulfill()
|
||||
|
||||
################
|
||||
# Click Actions
|
||||
@@ -290,20 +312,12 @@ class Certificate(object):
|
||||
"""
|
||||
self.find_css('a.detail-toggle').first.click()
|
||||
|
||||
################
|
||||
# Workflows
|
||||
################
|
||||
|
||||
def delete_certificate(self):
|
||||
def click_delete_certificate_button(self):
|
||||
"""
|
||||
Delete the certificate
|
||||
Remove the first (possibly the only) certificate from the set
|
||||
"""
|
||||
self.wait_for_certificate_delete_button()
|
||||
|
||||
self.find_css('.actions .delete').first.click()
|
||||
self.page.wait_for_confirmation_prompt()
|
||||
self.page.q(css='a.button.action-primary').first.click()
|
||||
self.page.q(css='a.button.action-primary').first.click()
|
||||
self.page.wait_for_ajax()
|
||||
|
||||
|
||||
|
||||
@@ -126,9 +126,12 @@ class CertificatesTest(StudioCourseTest):
|
||||
|
||||
self.assertEqual(len(self.certificates_page.certificates), 1)
|
||||
|
||||
# Delete certificate
|
||||
certificate.delete_certificate()
|
||||
# Delete the certificate we just created
|
||||
certificate.click_delete_certificate_button()
|
||||
self.certificates_page.click_confirmation_prompt_primary_button()
|
||||
self.certificates_page.wait_for_first_certificate_button()
|
||||
|
||||
# Reload the page and confirm there are no certificates
|
||||
self.certificates_page.visit()
|
||||
self.assertEqual(len(self.certificates_page.certificates), 0)
|
||||
|
||||
|
||||
@@ -154,7 +154,11 @@ def set_cert_generation_enabled(course_key, is_enabled):
|
||||
|
||||
"""
|
||||
CertificateGenerationCourseSetting.set_enabled_for_course(course_key, is_enabled)
|
||||
|
||||
cert_event_type = 'enabled' if is_enabled else 'disabled'
|
||||
event_name = '.'.join(['edx', 'certificate', 'generation', cert_event_type])
|
||||
tracker.emit(event_name, {
|
||||
'course_id': unicode(course_key),
|
||||
})
|
||||
if is_enabled:
|
||||
log.info(u"Enabled self-generated certificates for course '%s'.", unicode(course_key))
|
||||
else:
|
||||
|
||||
@@ -229,13 +229,13 @@ class GenerateUserCertificatesTest(EventTestMixin, ModuleStoreTestCase):
|
||||
|
||||
@attr('shard_1')
|
||||
@ddt.ddt
|
||||
class CertificateGenerationEnabledTest(TestCase):
|
||||
class CertificateGenerationEnabledTest(EventTestMixin, TestCase):
|
||||
"""Test enabling/disabling self-generated certificates for a course. """
|
||||
|
||||
COURSE_KEY = CourseLocator(org='test', course='test', run='test')
|
||||
|
||||
def setUp(self):
|
||||
super(CertificateGenerationEnabledTest, self).setUp()
|
||||
super(CertificateGenerationEnabledTest, self).setUp('certificates.api.tracker')
|
||||
|
||||
# Since model-based configuration is cached, we need
|
||||
# to clear the cache before each test.
|
||||
@@ -256,6 +256,12 @@ class CertificateGenerationEnabledTest(TestCase):
|
||||
|
||||
if is_course_enabled is not None:
|
||||
certs_api.set_cert_generation_enabled(self.COURSE_KEY, is_course_enabled)
|
||||
cert_event_type = 'enabled' if is_course_enabled else 'disabled'
|
||||
event_name = '.'.join(['edx', 'certificate', 'generation', cert_event_type])
|
||||
self.assert_event_emitted(
|
||||
event_name,
|
||||
course_id=unicode(self.COURSE_KEY),
|
||||
)
|
||||
|
||||
self._assert_enabled_for_course(self.COURSE_KEY, expect_enabled)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user