From c1f7272174b6a230521d7f3c9625962205e20dda Mon Sep 17 00:00:00 2001 From: Justin Hynes Date: Wed, 30 Jun 2021 09:23:46 -0400 Subject: [PATCH 1/2] feat!: Remove certificate generation segment event [MICROBA-1298] * Remove a Segment certificate event that was, historically, only emit from the courseware. The Segment event that was previously (only) issued in the courseware doesn't seem to be important or referenced anywhere we could see. Since this event only tracked certificate generation attempts from one (of multiple) paths to initiate certificate generation in v1, I didn't think the historical event data captured is complete. We approached our data teams and I determined this event was not of high importance. For this reason, I am suggesting we just stop using this event and remove the code responsible for sending it. We will continue to track certificate creation/generation using the existing `edx.certificate.created` event. --- lms/djangoapps/certificates/generation.py | 2 -- .../certificates/tests/test_generation.py | 22 ------------------- lms/djangoapps/certificates/utils.py | 18 --------------- lms/djangoapps/courseware/views/views.py | 18 --------------- 4 files changed, 60 deletions(-) diff --git a/lms/djangoapps/certificates/generation.py b/lms/djangoapps/certificates/generation.py index cd9892620d..eeed571b82 100644 --- a/lms/djangoapps/certificates/generation.py +++ b/lms/djangoapps/certificates/generation.py @@ -18,7 +18,6 @@ from lms.djangoapps.certificates.models import GeneratedCertificate from lms.djangoapps.certificates.queue import XQueueCertInterface from lms.djangoapps.certificates.utils import ( emit_certificate_event, - emit_segment_event, has_html_certificates_enabled ) from lms.djangoapps.grades.api import CourseGradeFactory @@ -55,7 +54,6 @@ def generate_course_certificate(user, course_key, status, generation_mode): 'generation_mode': generation_mode } emit_certificate_event(event_name='created', user=user, course_id=course_key, event_data=event_data) - emit_segment_event(user_id=user.id, course_id=course_key) elif CertificateStatuses.unverified == cert.status: cert.mark_unverified(source='certificate_generation') diff --git a/lms/djangoapps/certificates/tests/test_generation.py b/lms/djangoapps/certificates/tests/test_generation.py index 68bdaeb4c8..a2f55d91a3 100644 --- a/lms/djangoapps/certificates/tests/test_generation.py +++ b/lms/djangoapps/certificates/tests/test_generation.py @@ -2,7 +2,6 @@ Tests for certificate generation """ import logging -from unittest.mock import patch from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory from common.djangoapps.util.testing import EventTestMixin @@ -59,27 +58,6 @@ class CertificateTests(EventTestMixin, ModuleStoreTestCase): generation_mode=self.gen_mode ) - def test_segment_event(self): - """ - Test that a segment event is created - """ - analytics_patcher = patch('lms.djangoapps.certificates.utils.segment') - mock_tracker = analytics_patcher.start() - self.addCleanup(analytics_patcher.stop) - - generated_cert = generate_course_certificate(self.u, self.key, CertificateStatuses.downloadable, self.gen_mode) - assert generated_cert.status, CertificateStatuses.downloadable - - mock_tracker.track.assert_called_once_with( - self.u.id, - 'edx.bi.user.certificate.generate', - { - 'category': 'certificates', - 'label': str(self.key) - }, - ) - mock_tracker.reset_mock() - def test_generation_existing(self): """ Test certificate generation when a certificate already exists diff --git a/lms/djangoapps/certificates/utils.py b/lms/djangoapps/certificates/utils.py index 853a7554c6..93d9d4c0ba 100644 --- a/lms/djangoapps/certificates/utils.py +++ b/lms/djangoapps/certificates/utils.py @@ -9,7 +9,6 @@ from django.urls import reverse from eventtracking import tracker from opaque_keys.edx.keys import CourseKey -from common.djangoapps.track import segment from lms.djangoapps.certificates.models import GeneratedCertificate from openedx.core.djangoapps.content.course_overviews.api import get_course_overview @@ -62,23 +61,6 @@ def emit_certificate_event(event_name, user, course_id, course_overview=None, ev tracker.emit(event_name, event_data) -def emit_segment_event(user_id, course_id): - """ - Track a successful certificate generation event in segment. - - Arguments: - user_id (str): The ID of the user associated with the certificate. - course_id (CourseKey): Identifier for the course. - Returns: - None - """ - event_name = 'edx.bi.user.certificate.generate' - segment.track(user_id, event_name, { - 'category': 'certificates', - 'label': str(course_id) - }) - - def get_certificate_url(user_id=None, course_id=None, uuid=None, user_certificate=None): """ Returns the certificate URL diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index f37d3c13fb..6856fcae7d 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -124,7 +124,6 @@ from openedx.features.course_experience.waffle import ENABLE_COURSE_ABOUT_SIDEBA from openedx.features.course_experience.waffle import waffle as course_experience_waffle from openedx.features.enterprise_support.api import data_sharing_consent_required from common.djangoapps.student.models import CourseEnrollment, UserTestGroup -from common.djangoapps.track import segment from common.djangoapps.util.cache import cache, cache_if_anonymous from common.djangoapps.util.db import outer_atomic from common.djangoapps.util.milestones_helpers import get_prerequisite_courses_display @@ -1649,27 +1648,10 @@ def generate_user_cert(request, course_id): # with a management command. From the user's perspective, # it will appear that the certificate task was submitted successfully. certs_api.generate_user_certificates(student, course.id, generation_mode='self') - _track_successful_certificate_generation(student.id, course.id) return HttpResponse() -def _track_successful_certificate_generation(user_id, course_id): - """ - Track a successful certificate generation event. - Arguments: - user_id (str): The ID of the user generating the certificate. - course_id (CourseKey): Identifier for the course. - Returns: - None - """ - event_name = 'edx.bi.user.certificate.generate' - segment.track(user_id, event_name, { - 'category': 'certificates', - 'label': str(course_id) - }) - - def enclosing_sequence_for_gating_checks(block): """ Return the first ancestor of this block that is a SequenceDescriptor. From 4cd15acaf9ca69a6afa636381a12b211bb18790f Mon Sep 17 00:00:00 2001 From: Justin Hynes Date: Wed, 30 Jun 2021 10:21:10 -0400 Subject: [PATCH 2/2] Fix failing unit test in courseware --- lms/djangoapps/courseware/tests/test_views.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 7ba4e3ef5c..12bff0384a 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -2281,24 +2281,16 @@ class GenerateUserCertTests(ModuleStoreTestCase): ) @patch('lms.djangoapps.courseware.views.views.is_course_passed', return_value=True) - @override_settings(CERT_QUEUE='certificates', LMS_SEGMENT_KEY="foobar") + @override_settings(CERT_QUEUE='certificates') def test_user_with_passing_grade(self, mock_is_course_passed): # lint-amnesty, pylint: disable=unused-argument # If user has above passing grading then json will return cert generating message and # status valid code - # mocking xqueue and Segment analytics - - analytics_patcher = patch('lms.djangoapps.courseware.views.views.segment') - mock_tracker = analytics_patcher.start() - self.addCleanup(analytics_patcher.stop) - with patch('capa.xqueue_interface.XQueueInterface.send_to_queue') as mock_send_to_queue: mock_send_to_queue.return_value = (0, "Successfully queued") resp = self.client.post(self.url) assert resp.status_code == 200 - mock_tracker.reset_mock() - def test_user_with_passing_existing_generating_cert(self): # If user has passing grade but also has existing generating cert # then json will return cert generating message with bad request code @@ -2316,7 +2308,7 @@ class GenerateUserCertTests(ModuleStoreTestCase): resp = self.client.post(self.url) self.assertContains(resp, "Certificate is being created.", status_code=HttpResponseBadRequest.status_code) - @override_settings(CERT_QUEUE='certificates', LMS_SEGMENT_KEY="foobar") + @override_settings(CERT_QUEUE='certificates') def test_user_with_passing_existing_downloadable_cert(self): # If user has already downloadable certificate # then json will return cert generating message with bad request code