fix: catching orphaned records without catalog uuids

enterprise/constants.py

@@ -204,6 +204,9 @@ def json_serialized_course_modes():
     503: 'The server is temporarily unavailable.',
 }
 
+IC_CREATE_ACTION = 'create'
+IC_UPDATE_ACTION = 'update'
+IC_DELETE_ACTION = 'delete'
 
 class FulfillmentTypes:
     LICENSE = 'license'

integrated_channels/integrated_channel/exporters/content_metadata.py

@@ -14,7 +14,12 @@
 from django.db.models import Q
 
 from enterprise.api_client.enterprise_catalog import EnterpriseCatalogApiClient
-from enterprise.constants import EXEC_ED_CONTENT_DESCRIPTION_TAG, EXEC_ED_COURSE_TYPE, TRANSMISSION_MARK_CREATE
+from enterprise.constants import (
+    EXEC_ED_CONTENT_DESCRIPTION_TAG,
+    EXEC_ED_COURSE_TYPE,
+    IC_DELETE_ACTION,
+    TRANSMISSION_MARK_CREATE,
+)
 from enterprise.utils import get_content_metadata_item_id
 from integrated_channels.integrated_channel.exporters import Exporter
 from integrated_channels.utils import generate_formatted_log, truncate_item_dicts
@@ -533,8 +538,23 @@ def export(self, **kwargs):
                         existing_record.channel_metadata = transformed_item
                         update_payload[key] = existing_record
 
-            for key, item in items_to_delete.items():
-                delete_payload[key] = item
+            if items_to_delete:
+                delete_metadata_items = { get_content_metadata_item_id(item): item for item in self.enterprise_catalog_api.get_content_metadata(
+                    self.enterprise_customer,
+                    [enterprise_customer_catalog],
+                    list(items_to_delete.keys()),
+                )}
+                for key, item in items_to_delete.items():
+                    delete_exec_ed_metadata = self._transform_exec_ed_content(
+                        delete_metadata_items[item.content_id]
+                    )
+                    transformed_delete_metadata = self._transform_item(
+                        delete_exec_ed_metadata,
+                        action=IC_DELETE_ACTION,
+                    )
+                    item.channel_metadata = transformed_delete_metadata
+                    item.save()
+                    delete_payload[key] = item
 
         # If we're not at the max payload count, we can check for orphaned content and shove it in the delete payload
         current_payload_count = len(create_payload) + len(update_payload) + len(delete_payload)
@@ -586,7 +606,7 @@ def _transform_exec_ed_content(self, content):
                 content['full_description'] = EXEC_ED_CONTENT_DESCRIPTION_TAG + description
         return content
 
-    def _transform_item(self, content_metadata_item):
+    def _transform_item(self, content_metadata_item, action=None):
         """
         Transform the provided content metadata item to the schema expected by the integrated channel.
         """
@@ -634,8 +654,17 @@ def _transform_item(self, content_metadata_item):
                 continue
             transformed_item[integrated_channel_schema_key] = transformed_value
 
+        if action == IC_DELETE_ACTION:
+            self._transform_item_for_delete(transformed_item)
+
         return transformed_item
 
+    def _transform_item_for_delete(self, content_metadata_item):
+        """
+        Transform the provided content metadata item to the schema expected by the integrated channel.
+        """
+        return content_metadata_item
+
     def update_content_transmissions_catalog_uuids(self):
         """
         Retrieve all content under the enterprise customer's catalog(s) and update all past transmission audits to have

integrated_channels/integrated_channel/models.py

@@ -251,12 +251,16 @@ def fetch_orphaned_content_audits(self):
             self.enterprise_customer.enterprise_customer_catalogs.all()
 
         customer_catalog_uuids = enterprise_customer_catalogs.values_list('uuid', flat=True)
+
+        non_existent_catalogs_filter = Q(enterprise_customer_catalog_uuid__in=customer_catalog_uuids)
+        null_catalogs_filter = Q(enterprise_customer_catalog_uuid__isnull=True)
+
         return ContentMetadataItemTransmission.objects.filter(
             integrated_channel_code=self.channel_code(),
             enterprise_customer=self.enterprise_customer,
             remote_deleted_at__isnull=True,
             remote_created_at__isnull=False,
-        ).exclude(enterprise_customer_catalog_uuid__in=customer_catalog_uuids)
+        ).filter(~non_existent_catalogs_filter | null_catalogs_filter)
 
     def update_content_synced_at(self, action_happened_at, was_successful):
         """

integrated_channels/integrated_channel/transmitters/content_metadata.py

@@ -136,7 +136,7 @@ def _transmit_action(self, content_metadata_item_map, client_method, action_name
             self.enterprise_configuration.channel_code()
         )
 
-        # If we're deleting, fetch all orphaned, uneresolved content transmissions
+        # If we're deleting, fetch all orphaned, unresolved content transmissions
         is_delete_action = action_name == 'delete'
         if is_delete_action:
             OrphanedContentTransmissions = apps.get_model(

integrated_channels/sap_success_factors/exporters/content_metadata.py

@@ -46,6 +46,10 @@ class SapSuccessFactorsContentMetadataExporter(ContentMetadataExporter):
         'price': 'price',
     }
 
+    def _transform_item_for_delete(self, content_metadata_item):
+        content_metadata_item['status'] = 'INACTIVE'
+        return content_metadata_item
+
     def transform_provider_id(self, content_metadata_item):  # pylint: disable=unused-argument
         """
         Return the provider ID from the integrated channel configuration.

tests/test_integrated_channels/test_integrated_channel/test_exporters/test_content_metadata.py

@@ -16,6 +16,7 @@
 from enterprise.constants import EXEC_ED_COURSE_TYPE
 from enterprise.utils import get_content_metadata_item_id
 from integrated_channels.integrated_channel.exporters.content_metadata import ContentMetadataExporter
+from integrated_channels.sap_success_factors.exporters.content_metadata import SapSuccessFactorsContentMetadataExporter
 from integrated_channels.integrated_channel.models import ContentMetadataItemTransmission
 from test_utils import FAKE_UUIDS, factories
 from test_utils.factories import ContentMetadataItemTransmissionFactory
@@ -58,11 +59,123 @@ def setUp(self):
         }
         super().setUp()
 
+    @mock.patch('enterprise.api_client.enterprise_catalog.EnterpriseCatalogApiClient.get_content_metadata')
     @mock.patch('enterprise.api_client.enterprise_catalog.EnterpriseCatalogApiClient.get_catalog_diff')
-    def test_exporter_get_catalog_diff_works_with_orphaned_content(
-        self,
+    def test_exporter_transforms_metadata_of_items_to_be_deleted(
+        self, 
         mock_get_catalog_diff,
+        mock_get_content_metadata,
     ):
+        """
+        Test that the exporter properly transforms the metadata of items that are to be deleted.
+        """
+        sap_config = factories.SAPSuccessFactorsEnterpriseCustomerConfiguration(
+            enterprise_customer=self.enterprise_customer_catalog.enterprise_customer,
+        )
+
+        # Existing audit metadata, needs to be updated/transformed.
+        # According to the SAP channel, upon deletion we need to set each content metadata blob's status to `INACTIVE`.
+        # Additionally, according to the mocked catalog metadata, the transformed mappings of the record need ot be
+        # updated. We will assert that both of these transformations are applied by the exporter.
+        channel_metadata = {
+            "courseID": "NYIF+BOPC.4x",
+            "providerID": "EDX",
+            # This status should be transformed to `INACTIVE` by the exporter
+            "status": "ACTIVE",
+            "title": [{
+                    "locale": "English",
+                    "value": "Brokerage Operations Professional Certificate Examination"
+            }],
+            "content": [
+                {
+                    "providerID": "EDX",
+                    "contentTitle": "Brokerage Operations Professional Certificate Examination",
+                    "contentID": "NYIF+BOPC.4x",
+                    "launchType": 3,
+                    "mobileEnabled": True,
+                }
+            ],
+            "schedule": [
+                {
+                    "startDate": "",
+                    "endDate": "",
+                    "active": False,
+                    "duration": "0 days"
+                }
+            ],
+            "price": [
+                {
+                    "currency": "USD",
+                    "value": 0.0
+                }
+            ]
+        }
+        content_id = "NYIF+BOPC.4x"
+        mock_metadata = {
+            "aggregation_key": "course:NYIF+BOPC.4x",
+            "content_type": "course",
+            "full_description": "ayylmao",
+            "key": content_id,
+            "title": "Brokerage Operations Professional Certificate Examination",
+            "course_runs": [
+                {
+                    "key": "course-v1:NYIF+BOPC.4x+3T2017",
+                    "uuid": "69b38e2f-e041-4634-b537-fc8d8e12c87e",
+                    "title": "Brokerage Operations Professional Certificate Examination",
+                    "short_description": "ayylmao",
+                    "availability": "Archived",
+                    "pacing_type": "self_paced",
+                    "seats": [
+                        {
+                            "type": "professional",
+                            "price": "500.00",
+                            "currency": "USD",
+                            "upgrade_deadline": None,
+                            "upgrade_deadline_override": None,
+                            "credit_provider": None,
+                            "credit_hours": None,
+                            "sku": "0C1BF31",
+                            "bulk_sku": "668527E"
+                        }
+                    ],
+                    "start": "2017-09-07T00:00:00Z",
+                    # A none value here will cause the sanitization to remove the schedule blob
+                    "end": None,
+                }
+            ],
+            "uuid": "bbbf059e-b9fb-4ad7-a53e-4c6f85f47f4e",
+            "end_date": "2019-09-07T00:00:00Z",
+            "course_ends": "Future",
+            "entitlements": [],
+            "modified": "2023-07-10T04:29:38.934204Z",
+            "additional_information": None,
+            "course_run_keys": [
+                "course-v1:NYIF+BOPC.4x+3T2017",
+                "course-v1:NYIF+BOPC.4x+1T2017"
+            ],
+            "enrollment_url": "https://foobar.com"
+        }
+        # Create the transmission audit with the out of date channel metadata
+        transmission_audit = factories.ContentMetadataItemTransmissionFactory(
+            content_id=content_id,
+            enterprise_customer=self.config.enterprise_customer,
+            plugin_configuration_id=sap_config.id,
+            integrated_channel_code=sap_config.channel_code(),
+            channel_metadata=channel_metadata,
+            remote_created_at=datetime.datetime.utcnow(),
+            enterprise_customer_catalog_uuid=self.enterprise_customer_catalog.uuid,
+        )
+        # Mock the catalog service to return the metadata of the content to be deleted
+        mock_get_content_metadata.return_value = [mock_metadata]
+        # Mock the catalog service to return the content to be deleted in the delete payload
+        mock_get_catalog_diff.return_value = ([], [{'content_key': transmission_audit.content_id}], [])
+        exporter = SapSuccessFactorsContentMetadataExporter('fake-user', sap_config)
+        _, _, delete_payload = exporter.export()
+        assert delete_payload[transmission_audit.content_id].channel_metadata.get('schedule') == []
+        assert delete_payload[transmission_audit.content_id].channel_metadata.get('status') == 'INACTIVE'
+
+    @mock.patch('enterprise.api_client.enterprise_catalog.EnterpriseCatalogApiClient.get_catalog_diff')
+    def test_exporter_get_catalog_diff_works_with_orphaned_content(self, mock_get_catalog_diff):
         """
         Test that the exporter _get_catalog_diff function properly marks orphaned content that is requested to be
         created by another, linked catalog as resolved.
@@ -244,13 +357,12 @@ def test_content_exporter_delete_export(self, mock_get_catalog_diff, mock_get_co
         mock_delete_items = [{'content_key': FAKE_COURSE_RUN['key']}]
         mock_matched_items = []
         mock_get_catalog_diff.return_value = mock_create_items, mock_delete_items, mock_matched_items
+        mock_get_content_metadata.return_value = [FAKE_COURSE_RUN]
         exporter = ContentMetadataExporter('fake-user', self.config)
         create_payload, update_payload, delete_payload = exporter.export()
         assert not create_payload
         assert not update_payload
         assert delete_payload.get(FAKE_COURSE_RUN['key']) == past_transmission
-        # Sanity check
-        mock_get_content_metadata.assert_not_called()
 
     @mock.patch('enterprise.api_client.enterprise_catalog.EnterpriseCatalogApiClient.get_content_metadata')
     @mock.patch('enterprise.api_client.enterprise_catalog.EnterpriseCatalogApiClient.get_catalog_diff')
@@ -329,6 +441,9 @@ def test_content_exporter_truncation_bug_export(self, mock_api_client):
             [FAKE_COURSE_RUN['key']]
         )
 
+        mock_api_client.return_value.get_catalog_diff.reset_mock()
+        mock_api_client.return_value.get_content_metadata.reset_mock()
+
         past_transmission_to_update.content_last_changed = datetime.datetime.now()
         past_transmission_to_update.save()
 
@@ -353,7 +468,9 @@ def test_content_exporter_truncation_bug_export(self, mock_api_client):
         assert not update_payload
         assert not create_payload
         assert delete_payload.get(FAKE_COURSE_RUN2['key']) == past_transmission_to_delete
-        assert mock_api_client.return_value.get_catalog_diff.call_count == 2
+        # get_catalog_diff is called once per customer catalog
+        assert mock_api_client.return_value.get_catalog_diff.call_count == 1
+        # get_content_metadata is called once for the single item to delete
         assert mock_api_client.return_value.get_content_metadata.call_count == 1
 
     @mock.patch('integrated_channels.integrated_channel.exporters.content_metadata.EnterpriseCatalogApiClient')

tests/test_management.py

@@ -1932,6 +1932,15 @@ def test_normal_run(self):
         orphaned_content = OrphanedContentTransmissions.objects.first()
         assert orphaned_content.content_id == self.orphaned_content.content_id
 
+    @override_settings(ALLOW_ORPHANED_CONTENT_REMOVAL=True)
+    def test_orphaned_content_without_catalog_uuids(self):
+        self.orphaned_content.enterprise_customer_catalog_uuid = None
+        self.orphaned_content.save()
+        assert not OrphanedContentTransmissions.objects.all()
+        call_command('mark_orphaned_content_metadata_audits')
+        num_orphaned_content = OrphanedContentTransmissions.objects.count()
+        assert num_orphaned_content == 1
+
 
 @mark.django_db
 @ddt.ddt