ziafazal/SOL-1021: Capture additional certificate configuration events

* Changed certificate_id to configuration_id
* Fix for acceptance test faiiure
* Add waits to bok choy tests

fix for flaky test

cms/djangoapps/contentstore/views/certificates.py

@@ -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)
 
 

cms/djangoapps/contentstore/views/tests/test_certificates.py

@@ -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):

common/test/acceptance/pages/studio/settings_certificates.py

@@ -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()
 
 

common/test/acceptance/tests/studio/test_studio_settings_certificates.py

@@ -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)
 

lms/djangoapps/certificates/api.py

@@ -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:

lms/djangoapps/certificates/tests/test_api.py

@@ -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)