Merge pull request #1973 from openedx/asaeed/ENT-8005

[ENT-8005]: Refactor learner data transmission audit record

CHANGELOG.rst

@@ -16,6 +16,11 @@ Change Log
 Unreleased
 ----------
 
+[4.9.2]
+--------
+
+refactor: learner data transmission audit record  to utilize the existing records (ENT-8005)
+
 [4.9.1]
 --------
 

enterprise/__init__.py

@@ -2,4 +2,4 @@
 Your project description goes here.
 """
 
-__version__ = "4.9.1"
+__version__ = "4.9.2"

integrated_channels/blackboard/exporters/learner_data.py

@@ -50,9 +50,14 @@ def get_learner_data_records(
             'blackboard',
             'BlackboardLearnerDataTransmissionAudit'
         )
-
-        return [
-            BlackboardLearnerDataTransmissionAudit(
+        course_id = get_course_id_for_enrollment(enterprise_enrollment)
+        # We only want to send one record per enrollment and course, so we check if one exists first.
+        learner_transmission_record = BlackboardLearnerDataTransmissionAudit.objects.filter(
+            enterprise_course_enrollment_id=enterprise_enrollment.id,
+            course_id=course_id,
+        ).first()
+        if learner_transmission_record is None:
+            learner_transmission_record = BlackboardLearnerDataTransmissionAudit(
                 enterprise_course_enrollment_id=enterprise_enrollment.id,
                 blackboard_user_email=enterprise_customer_user.user_email,
                 user_email=enterprise_customer_user.user_email,
@@ -64,7 +69,7 @@ def get_learner_data_records(
                 enterprise_customer_uuid=enterprise_customer_user.enterprise_customer.uuid,
                 plugin_configuration_id=self.enterprise_configuration.id,
             )
-        ]
+        return [learner_transmission_record]
 
     def get_learner_assessment_data_records(
             self,

integrated_channels/canvas/exporters/learner_data.py

@@ -50,10 +50,14 @@ def get_learner_data_records(
             'canvas',
             'CanvasLearnerDataTransmissionAudit'
         )
-        # We return two records here, one with the course key and one with the course run id, to account for
-        # uncertainty about the type of content (course vs. course run) that was sent to the integrated channel.
-        return [
-            CanvasLearnerDataTransmissionAudit(
+        course_id = get_course_id_for_enrollment(enterprise_enrollment)
+        # We only want to send one record per enrollment and course, so we check if one exists first.
+        learner_transmission_record = CanvasLearnerDataTransmissionAudit.objects.filter(
+            enterprise_course_enrollment_id=enterprise_enrollment.id,
+            course_id=course_id,
+        ).first()
+        if learner_transmission_record is None:
+            learner_transmission_record = CanvasLearnerDataTransmissionAudit(
                 enterprise_course_enrollment_id=enterprise_enrollment.id,
                 canvas_user_email=enterprise_customer_user.user_email,
                 user_email=enterprise_customer_user.user_email,
@@ -64,8 +68,9 @@ def get_learner_data_records(
                 canvas_completed_timestamp=canvas_completed_timestamp,
                 enterprise_customer_uuid=enterprise_customer_user.enterprise_customer.uuid,
                 plugin_configuration_id=self.enterprise_configuration.id,
-            ),
-        ]
+            )
+        # We return one record here, with the course key, that was sent to the integrated channel.
+        return [learner_transmission_record]
 
     def get_learner_assessment_data_records(
             self,

integrated_channels/degreed2/exporters/learner_data.py

@@ -57,10 +57,14 @@ def get_learner_data_records(
                 'degreed2',
                 'Degreed2LearnerDataTransmissionAudit'
             )
-            # We return two records here, one with the course key and one with the course run id, to account for
-            # uncertainty about the type of content (course vs. course run) that was sent to the integrated channel.
-            return [
-                Degreed2LearnerDataTransmissionAudit(
+            course_id = get_course_id_for_enrollment(enterprise_enrollment)
+            # We only want to send one record per enrollment and course, so we check if one exists first.
+            learner_transmission_record = Degreed2LearnerDataTransmissionAudit.objects.filter(
+                enterprise_course_enrollment_id=enterprise_enrollment.id,
+                course_id=course_id,
+            ).first()
+            if learner_transmission_record is None:
+                learner_transmission_record = Degreed2LearnerDataTransmissionAudit(
                     enterprise_course_enrollment_id=enterprise_enrollment.id,
                     degreed_user_email=enterprise_enrollment.enterprise_customer_user.user_email,
                     user_email=enterprise_enrollment.enterprise_customer_user.user_email,
@@ -71,8 +75,9 @@ def get_learner_data_records(
                     grade=percent_grade,
                     enterprise_customer_uuid=enterprise_enrollment.enterprise_customer_user.enterprise_customer.uuid,
                     plugin_configuration_id=self.enterprise_configuration.id,
-                ),
-            ]
+                )
+            # We return one record here, with the course key, that was sent to the integrated channel.
+            return [learner_transmission_record]
         LOGGER.info(generate_formatted_log(
             self.enterprise_configuration.channel_code(),
             enterprise_enrollment.enterprise_customer_user.enterprise_customer.uuid,

integrated_channels/integrated_channel/exporters/learner_data.py

@@ -637,22 +637,27 @@ def get_learner_data_records(
         completed_timestamp = None
         if completed_date is not None:
             completed_timestamp = parse_datetime_to_epoch_millis(completed_date)
-        # We return two records here, one with the course key and one with the course run id, to account for
-        # uncertainty about the type of content (course vs. course run) that was sent to the integrated channel.
-        return [
-            TransmissionAudit(
+        course_id = get_course_id_for_enrollment(enterprise_enrollment)
+        # We only want to send one record per enrollment and course, so we check if one exists first.
+        learner_transmission_record = TransmissionAudit.objects.filter(
+            enterprise_course_enrollment_id=enterprise_enrollment.id,
+            course_id=course_id,
+        ).first()
+        if learner_transmission_record is None:
+            learner_transmission_record = TransmissionAudit(
                 plugin_configuration_id=self.enterprise_configuration.id,
                 enterprise_customer_uuid=self.enterprise_configuration.enterprise_customer.uuid,
                 enterprise_course_enrollment_id=enterprise_enrollment.id,
-                course_id=get_course_id_for_enrollment(enterprise_enrollment),
+                course_id=course_id,
                 course_completed=course_completed,
                 completed_timestamp=completed_timestamp,
                 grade=grade,
                 user_email=user_email,
                 content_title=content_title,
                 progress_status=progress_status,
-            ),
-        ]
+            )
+        # We return one record here, with the course key, that was sent to the integrated channel.
+        return [learner_transmission_record]
 
     def collect_certificate_data(self, enterprise_enrollment, channel_name):
         """

integrated_channels/moodle/exporters/learner_data.py

@@ -52,11 +52,14 @@ def get_learner_data_records(
         )
 
         percent_grade = kwargs.get('grade_percent', None)
-        # We return two records here, one with the course key and one with the course run id, to account for
-        # uncertainty about the type of content (course vs. course run) that was sent to the integrated channel.
-        # TODO: this shouldn't be necessary anymore and eventually phased out as part of tech debt
-        return [
-            MoodleLearnerDataTransmissionAudit(
+        course_id = get_course_id_for_enrollment(enterprise_enrollment)
+        # We only want to send one record per enrollment and course, so we check if one exists first.
+        learner_transmission_record = MoodleLearnerDataTransmissionAudit.objects.filter(
+            enterprise_course_enrollment_id=enterprise_enrollment.id,
+            course_id=course_id,
+        ).first()
+        if learner_transmission_record is None:
+            learner_transmission_record = MoodleLearnerDataTransmissionAudit(
                 enterprise_course_enrollment_id=enterprise_enrollment.id,
                 moodle_user_email=enterprise_customer_user.user_email,
                 user_email=enterprise_customer_user.user_email,
@@ -67,5 +70,7 @@ def get_learner_data_records(
                 moodle_completed_timestamp=moodle_completed_timestamp,
                 enterprise_customer_uuid=enterprise_customer_user.enterprise_customer.uuid,
                 plugin_configuration_id=self.enterprise_configuration.id,
-            ),
-        ]
+            )
+        # We return one record here, with the course key, that was sent to the integrated channel.
+        # TODO: this shouldn't be necessary anymore and eventually phased out as part of tech debt
+        return [learner_transmission_record]

integrated_channels/sap_success_factors/exporters/learner_data.py

@@ -58,8 +58,14 @@ def get_learner_data_records(
             total_hours = 0.0
             if course_run and self.enterprise_configuration.transmit_total_hours:
                 total_hours = course_run.get("estimated_hours", 0.0)
-            return [
-                SapSuccessFactorsLearnerDataTransmissionAudit(
+            course_id = get_course_id_for_enrollment(enterprise_enrollment)
+            # We only want to send one record per enrollment and course, so we check if one exists first.
+            learner_transmission_record = SapSuccessFactorsLearnerDataTransmissionAudit.objects.filter(
+                enterprise_course_enrollment_id=enterprise_enrollment.id,
+                course_id=course_id,
+            ).first()
+            if learner_transmission_record is None:
+                learner_transmission_record = SapSuccessFactorsLearnerDataTransmissionAudit(
                     enterprise_course_enrollment_id=enterprise_enrollment.id,
                     sapsf_user_id=sapsf_user_id,
                     user_email=enterprise_enrollment.enterprise_customer_user.user_email,
@@ -72,8 +78,8 @@ def get_learner_data_records(
                     credit_hours=total_hours,
                     enterprise_customer_uuid=self.enterprise_configuration.enterprise_customer.uuid,
                     plugin_configuration_id=self.enterprise_configuration.id
-                ),
-            ]
+                )
+            return [learner_transmission_record]
         LOGGER.info(
             generate_formatted_log(
                 self.enterprise_configuration.channel_code(),

tests/test_integrated_channels/test_blackboard/test_exporters/test_learner_data.py

@@ -0,0 +1,52 @@
+"""
+Tests for Blackboard learner data exporters.
+"""
+
+import unittest
+from unittest import mock
+
+from pytest import mark
+
+from integrated_channels.blackboard.exporters.learner_data import BlackboardLearnerExporter
+from test_utils import factories
+
+
+@mark.django_db
+class TestBlackboardLearnerDataExporter(unittest.TestCase):
+    """
+    Test BlackboardLearnerDataExporter
+    """
+
+    def setUp(self):
+        super().setUp()
+        self.enterprise_customer = factories.EnterpriseCustomerFactory()
+        self.enterprise_customer_user = factories.EnterpriseCustomerUserFactory(
+            enterprise_customer=self.enterprise_customer,
+        )
+        self.course_id = 'course-v1:edX+DemoX+DemoCourse'
+        self.course_key = 'edX+DemoX'
+        self.config = factories.BlackboardEnterpriseCustomerConfigurationFactory(
+            enterprise_customer=self.enterprise_customer,
+            blackboard_base_url='foobar',
+            client_id='client_id',
+            client_secret='client_secret',
+            refresh_token='token',
+        )
+
+    @mock.patch('enterprise.api_client.discovery.CourseCatalogApiServiceClient')
+    def test_retrieve_same_learner_data_record(self, mock_course_catalog_api):
+        """
+        If a learner data record already exists for the enrollment, it should be retrieved instead of created.
+        """
+        enterprise_course_enrollment = factories.EnterpriseCourseEnrollmentFactory(
+            course_id=self.course_id,
+            enterprise_customer_user=self.enterprise_customer_user,
+        )
+        mock_course_catalog_api.return_value.get_course_id.return_value = self.course_key
+        exporter = BlackboardLearnerExporter('fake-user', self.config)
+        learner_data_records_1 = exporter.get_learner_data_records(enterprise_course_enrollment)[0]
+        learner_data_records_1.save()
+        learner_data_records_2 = exporter.get_learner_data_records(enterprise_course_enrollment)[0]
+        learner_data_records_2.save()
+
+        assert learner_data_records_1.id == learner_data_records_2.id

tests/test_integrated_channels/test_canvas/test_exporters/test_learner_data.py

@@ -0,0 +1,51 @@
+"""
+Tests for Canvas learner data exporters.
+"""
+
+import unittest
+from unittest import mock
+
+from pytest import mark
+
+from integrated_channels.canvas.exporters.learner_data import CanvasLearnerExporter
+from test_utils import factories
+
+
+@mark.django_db
+class TestCanvasLearnerDataExporter(unittest.TestCase):
+    """
+    Test CanvasLearnerDataExporter
+    """
+
+    def setUp(self):
+        super().setUp()
+        self.user = factories.UserFactory(id=1, email='example@email.com')
+        self.enterprise_customer = factories.EnterpriseCustomerFactory()
+        self.enterprise_customer_user = factories.EnterpriseCustomerUserFactory(
+            user_id=self.user.id,
+            enterprise_customer=self.enterprise_customer,
+        )
+        self.course_id = 'course-v1:edX+DemoX+DemoCourse'
+        self.course_key = 'edX+DemoX'
+        self.config = factories.CanvasEnterpriseCustomerConfigurationFactory(
+            enterprise_customer=self.enterprise_customer,
+            canvas_base_url='foobar',
+        )
+
+    @mock.patch('enterprise.api_client.discovery.CourseCatalogApiServiceClient')
+    def test_retrieve_same_learner_data_record(self, mock_course_catalog_api):
+        """
+        If a learner data record already exists for the enrollment, it should be retrieved instead of created.
+        """
+        enterprise_course_enrollment = factories.EnterpriseCourseEnrollmentFactory(
+            course_id=self.course_id,
+            enterprise_customer_user=self.enterprise_customer_user,
+        )
+        mock_course_catalog_api.return_value.get_course_id.return_value = self.course_key
+        exporter = CanvasLearnerExporter('fake-user', self.config)
+        learner_data_records_1 = exporter.get_learner_data_records(enterprise_course_enrollment)[0]
+        learner_data_records_1.save()
+        learner_data_records_2 = exporter.get_learner_data_records(enterprise_course_enrollment)[0]
+        learner_data_records_2.save()
+
+        assert learner_data_records_1.id == learner_data_records_2.id

tests/test_integrated_channels/test_cornerstone/test_exporters/test_learner_data.py

@@ -117,6 +117,20 @@ def test_get_learner_data_record(self, completed_date):
             self.NOW if completed_date is not None else None
         )
 
+    @mock.patch('enterprise.api_client.discovery.CourseCatalogApiServiceClient')
+    def test_retrieve_same_learner_data_record(self, mock_course_catalog_api):
+        """
+        If a learner data record already exists for the enrollment, it should be retrieved instead of created.
+        """
+        mock_course_catalog_api.return_value.get_course_id.return_value = self.course_key
+        exporter = CornerstoneLearnerExporter('fake-user', self.config)
+        learner_data_records_1 = exporter.get_learner_data_records(self.enterprise_course_enrollment)[0]
+        learner_data_records_1.save()
+        learner_data_records_2 = exporter.get_learner_data_records(self.enterprise_course_enrollment)[0]
+        learner_data_records_2.save()
+
+        assert learner_data_records_1.id == learner_data_records_2.id
+
     def test_get_learner_data_record_not_exist(self):
         """
         If learner data is not already exist, nothing is returned.

tests/test_integrated_channels/test_degreed2/test_exporters/test_learner_data.py

@@ -91,6 +91,26 @@ def test_get_learner_data_record(self, completed_date, grade_percent):
             )
             assert learner_data_record.grade == (grade_percent * 100 if grade_percent else None)
 
+    def test_retrieve_same_learner_data_record(self):
+        """
+        If a learner data record already exists for the enrollment, it should be retrieved instead of created.
+        """
+        enterprise_course_enrollment = factories.EnterpriseCourseEnrollmentFactory(
+            enterprise_customer_user=self.enterprise_customer_user,
+            course_id=self.course_id,
+        )
+        exporter = Degreed2LearnerExporter('fake-user', self.config)
+        learner_data_records_1 = exporter.get_learner_data_records(
+            enterprise_course_enrollment,
+        )[0]
+        learner_data_records_1.save()
+        learner_data_records_2 = exporter.get_learner_data_records(
+            enterprise_course_enrollment,
+        )[0]
+        learner_data_records_2.save()
+
+        assert learner_data_records_1.id == learner_data_records_2.id
+
     def test_no_remote_id(self):
         """
         If the TPA API Client returns no remote user ID, nothing is returned.

tests/test_integrated_channels/test_integrated_channel/test_exporters/test_learner_data.py

@@ -131,6 +131,33 @@ def test_get_learner_data_record(self, completed_date, mock_course_catalog_api):
         assert learner_data_record.completed_timestamp == (self.NOW_TIMESTAMP if completed_date is not None else None)
         assert learner_data_record.grade == 'A+'
 
+    @mock.patch('enterprise.api_client.discovery.CourseCatalogApiServiceClient')
+    def test_retrieve_same_learner_data_record(self, mock_course_catalog_api):
+        """
+        If a learner data record already exists for the enrollment, it should be retrieved instead of created.
+        """
+        enterprise_course_enrollment = factories.EnterpriseCourseEnrollmentFactory(
+            enterprise_customer_user=self.enterprise_customer_user,
+            course_id=self.course_id,
+        )
+        mock_course_catalog_api.return_value.get_course_id.return_value = self.course_key
+        expected_course_completed = True
+        exporter = LearnerExporter('fake-user', self.config)
+        learner_data_records_1 = exporter.get_learner_data_records(
+            enterprise_course_enrollment,
+            course_completed=expected_course_completed,
+            progress_status='Passed'
+        )[0]
+        learner_data_records_1.save()
+        learner_data_records_2 = exporter.get_learner_data_records(
+            enterprise_course_enrollment,
+            course_completed=expected_course_completed,
+            progress_status='Passed'
+        )[0]
+        learner_data_records_2.save()
+
+        assert learner_data_records_1.id == learner_data_records_2.id
+
     def test_get_learner_subsection_data_records(self):
         """
         Test that the base learner subsection data exporter generates appropriate learner records from assessment grade