Merge branch 'master' into MueezKhan/Removing-Unencrypted-User-Creden…

…tials-Data

CHANGELOG.rst

@@ -16,6 +16,21 @@ Change Log
 Unreleased
 ----------
 
+[4.9.3]
+--------
+
+fix: Remove SAP debug logs
+
+[4.9.2]
+--------
+
+refactor: learner data transmission audit record  to utilize the existing records (ENT-8005)
+
+[4.9.1]
+--------
+
+fix: enable printing of error messages from the SSO Orchestrator API (ENT-8169)
+
 [4.9.0]
 --------
 

enterprise/__init__.py

@@ -2,4 +2,4 @@
 Your project description goes here.
 """
 
-__version__ = "4.9.0"
+__version__ = "4.9.3"

enterprise/api_client/sso_orchestrator.py

@@ -108,7 +108,10 @@ def _post(self, url, data=None):
         response = self.session.post(url, json=data, auth=self._create_auth_header())
         if response.status_code >= 300:
             raise SsoOrchestratorClientError(
-                f"Failed to make SSO Orchestrator API request: {response.status_code}",
+                (
+                    f"Failed to make SSO Orchestrator API request: {response.status_code}\n"
+                    f"{response.content}"
+                ),
                 response=response,
             )
         return response.json()

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

@@ -397,11 +397,6 @@ def export(self, **kwargs):
         )
         enrollment_ids_to_export = [enrollment.id for enrollment in enrollments_permitted]
 
-        LOGGER.info(
-            f"[Debug-SAP]: course_run_id:{course_run_id}, channel_name: {channel_name}"
-            f"lms_user_for_filter:{lms_user_for_filter}, grade:{grade} "
-            f"enrollment_ids_to_export: {enrollment_ids_to_export}"
-        )
         for enterprise_enrollment in enrollments_permitted:
             lms_user_id = enterprise_enrollment.enterprise_customer_user.user_id
             user_email = enterprise_enrollment.enterprise_customer_user.user_email
@@ -460,18 +455,7 @@ def export(self, **kwargs):
                 course_completed=_is_course_completed,
                 grade_percent=grade_percent,
             )
-            LOGGER.info(
-                generate_formatted_log(
-                    channel_name,
-                    enterprise_customer_uuid,
-                    lms_user_id,
-                    course_id,
-                    f", [Debug-SAP]: _is_course_completed: {_is_course_completed}, progress_status:{progress_status}"
-                    f",completed_date_from_api: {completed_date_from_api}, grade_from_api:{grade_from_api}"
-                    f"is_passing_from_api: {is_passing_from_api}, grade_percent:{grade_percent}"
-                    f"passed_timestamp:{passed_timestamp}, records: {records}",
-                )
-            )
+
             if records:
                 # There are some cases where we won't receive a record from the above
                 # method; right now, that should only happen if we have an Enterprise-linked
@@ -489,12 +473,6 @@ def export(self, **kwargs):
 
                     yield record
 
-        LOGGER.info(generate_formatted_log(
-            channel_name, None, lms_user_for_filter, course_run_id,
-            f'[Debug-SAP]: export finished. Did not export records for EnterpriseCourseEnrollment objects: '
-            f' {enrollment_ids_to_export}.'
-        ))
-
     def _filter_out_pre_transmitted_enrollments(
             self,
             enrollments_to_process,
@@ -546,42 +524,15 @@ def get_enrollments_to_process(self, lms_user_for_filter, course_run_id, channel
             enterprise_customer_user__active=True,
         )
         if lms_user_for_filter and course_run_id:
-            LOGGER.info(
-                generate_formatted_log(
-                    channel_name,
-                    self.enterprise_customer.uuid,
-                    lms_user_for_filter,
-                    course_run_id,
-                    f"[Debug-SAP]: enrollments to process before filtering: {list(enrollment_queryset)}",
-                )
-            )
             enrollment_queryset = enrollment_queryset.filter(
                 course_id=course_run_id,
                 enterprise_customer_user__user_id=lms_user_for_filter.id,
             )
-            LOGGER.info(
-                generate_formatted_log(
-                    channel_name,
-                    self.enterprise_customer.uuid,
-                    lms_user_for_filter,
-                    course_run_id,
-                    f"[Debug-SAP]: enrollments to process after filtering: {list(enrollment_queryset)}",
-                )
-            )
             LOGGER.info(generate_formatted_log(
                 channel_name, self.enterprise_customer.uuid, lms_user_for_filter, course_run_id,
                 'get_enrollments_to_process run for single learner and course.'))
         enrollment_queryset = enrollment_queryset.order_by('course_id')
         # return resolved list instead of queryset
-        LOGGER.info(
-            generate_formatted_log(
-                channel_name,
-                self.enterprise_customer.uuid,
-                lms_user_for_filter,
-                course_run_id,
-                f"[Debug-SAP]: enrollments to process: {list(enrollment_queryset)}",
-            )
-        )
         return list(enrollment_queryset)
 
     def get_learner_assessment_data_records(
@@ -637,22 +588,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