fix: catching orphaned records without catalog uuids

CHANGELOG.rst

@@ -15,6 +15,10 @@ Change Log
 
 Unreleased
 ----------
+[3.69.0]
+--------
+fix: content metadata exporter sanitizing content to delete
+
 [3.69.0]
 --------
 refactor: Replaced the deprecated `NullBooleanField` with `BooleanField(null=True)`

enterprise/__init__.py

@@ -2,6 +2,6 @@
 Your project description goes here.
 """
 
-__version__ = "3.69.0"
+__version__ = "3.69.1"
 
 default_app_config = "enterprise.apps.EnterpriseConfig"

enterprise/constants.py

@@ -204,6 +204,10 @@ 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,14 @@
 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_CREATE_ACTION,
+    IC_DELETE_ACTION,
+    IC_UPDATE_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
@@ -445,7 +452,19 @@ def _get_customer_config_orphaned_content(self, max_set_count, content_key=None)
         ordered_and_chunked_orphaned_content = orphaned_content.order_by('created')[:max_set_count]
         return ordered_and_chunked_orphaned_content
 
-    # pylint: disable=too-many-statements
+    def _sanitize_and_set_item_metadata(self, item, metadata, action):
+        """
+        Helper method to sanitize and set the metadata of an audit record according to
+        the provided action being performed on the item.
+        """
+        metadata_transformed_for_exec_ed = self._transform_exec_ed_content(metadata)
+        transformed_item = self._transform_item(metadata_transformed_for_exec_ed, action=action)
+
+        item.channel_metadata = transformed_item
+        item.content_title = transformed_item.get('title')
+        item.content_last_changed = transformed_item.get('content_last_modified')
+        item.save()
+
     def export(self, **kwargs):
         """
         Export transformed content metadata if there has been an update to the consumer's catalogs
@@ -454,7 +473,7 @@ def export(self, **kwargs):
             self.enterprise_customer.enterprise_customer_catalogs.all()
 
         # a maximum number of changes/payloads to export at once
-        # default to something huge to simplifly logic, the max system int size
+        # default to something huge to simplify logic, the max system int size
         max_payload_count = kwargs.get('max_payload_count', sys.maxsize)
 
         self._log_info(
@@ -492,48 +511,37 @@ def export(self, **kwargs):
             self._log_info(f'diff items_to_update: {items_to_update}')
             self._log_info(f'diff items_to_delete: {items_to_delete}')
 
-            # We only need to fetch content metadata if there are items to update or create
-            if items_to_create or items_to_update:
-                items_create_keys = list(items_to_create.keys())
-                items_update_keys = list(items_to_update.keys())
-                content_keys_filter = items_create_keys + items_update_keys
+            content_keys_filter = list(items_to_create.keys()) + list(items_to_update.keys()) + \
+                list(items_to_delete.keys())
+            if content_keys_filter:
                 content_metadata_items = self.enterprise_catalog_api.get_content_metadata(
                     self.enterprise_customer,
                     [enterprise_customer_catalog],
                     content_keys_filter,
                 )
-                for item in content_metadata_items:
-                    key = get_content_metadata_item_id(item)
-
-                    # Ensure executive education content is properly tagged before transforming the content to
-                    # the channel specific, expected form
-                    item = self._transform_exec_ed_content(item)
-
-                    # transform the content metadata into the channel specific format
-                    transformed_item = self._transform_item(item)
-                    if key in items_create_keys:
-                        existing_record = items_to_create.get(key)
-                        existing_record.channel_metadata = transformed_item
-                        existing_record.content_title = item.get('title')
-                        existing_record.content_last_changed = item.get('content_last_modified')
-                        # Sanity check
-                        existing_record.enterprise_customer_catalog_uuid = enterprise_customer_catalog.uuid
-                        existing_record.save()
-                        create_payload[key] = existing_record
-                    elif key in items_update_keys:
-                        existing_record = items_to_update.get(key)
-                        existing_record.content_title = item.get('title')
-                        # Sanity check
-                        existing_record.enterprise_customer_catalog_uuid = enterprise_customer_catalog.uuid
-                        existing_record.save()
-                        # Intentionally setting the channel_metadata and content_last_changed
-                        # fields post-save to clarify what data has been transmitted
-                        # and so that untransmitted courses will get picked up again in subsequent runs
-                        existing_record.content_last_changed = item.get('content_last_modified')
-                        existing_record.channel_metadata = transformed_item
-                        update_payload[key] = existing_record
+                key_to_content_metadata_mapping = {
+                    get_content_metadata_item_id(item): item for item in content_metadata_items
+                }
+            for key, item in items_to_create.items():
+                self._sanitize_and_set_item_metadata(item, key_to_content_metadata_mapping[key], IC_CREATE_ACTION)
+                # Sanity check
+                item.enterprise_customer_catalog_uuid = enterprise_customer_catalog.uuid
+                item.save()
+
+                create_payload[key] = item
+            for key, item in items_to_update.items():
+                self._sanitize_and_set_item_metadata(item, key_to_content_metadata_mapping[key], IC_UPDATE_ACTION)
+                # Sanity check
+                item.enterprise_customer_catalog_uuid = enterprise_customer_catalog.uuid
+                item.save()
 
+                update_payload[key] = item
             for key, item in items_to_delete.items():
+                self._sanitize_and_set_item_metadata(item, key_to_content_metadata_mapping[key], IC_DELETE_ACTION)
+                # Sanity check
+                item.enterprise_customer_catalog_uuid = enterprise_customer_catalog.uuid
+                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
@@ -586,7 +594,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):
         """
         Transform the provided content metadata item to the schema expected by the integrated channel.
         """
@@ -596,25 +604,21 @@ def _transform_item(self, content_metadata_item):
             # Look for transformer functions defined on subclasses.
             # Favor content type-specific functions.
             transformer = (
-                getattr(
-                    self,
-                    'transform_{content_type}_{edx_data_schema_key}'.format(
-                        content_type=content_metadata_type,
-                        edx_data_schema_key=edx_data_schema_key
-                    ),
-                    None
-                )
+                getattr(self, f'transform_{content_metadata_type}_{edx_data_schema_key}', None)
                 or
-                getattr(
-                    self,
-                    'transform_{edx_data_schema_key}'.format(
-                        edx_data_schema_key=edx_data_schema_key
-                    ),
-                    None
-                )
+                getattr(self, f'transform_{edx_data_schema_key}', None)
             )
+
+            transformer_for_action = (
+                getattr(self, f'transform_for_action_{content_metadata_type}_{edx_data_schema_key}', None)
+                or
+                getattr(self, f'transform_for_action_{edx_data_schema_key}', None)
+            )
+
             if transformer:
                 transformed_value = transformer(content_metadata_item)
+            elif transformer_for_action:
+                transformed_value = transformer_for_action(content_metadata_item, action)
             else:
                 # The concrete subclass does not define an override for the given field,
                 # so just use the data key to index the content metadata item dictionary.

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

@@ -6,6 +6,7 @@
 
 from django.utils.translation import gettext_lazy as _
 
+from enterprise.constants import IC_DELETE_ACTION
 from enterprise.utils import (
     get_advertised_course_run,
     get_closest_course_run,
@@ -52,12 +53,12 @@ def transform_provider_id(self, content_metadata_item):  # pylint: disable=unuse
         """
         return self.enterprise_configuration.provider_id
 
-    def transform_status(self, content_metadata_item):
+    def transform_for_action_status(self, _content_metadata_item, action):
         """
         Return the status of the content item.
         """
         # lets not overwrite something we've already tried to set INACTIVE
-        if content_metadata_item.get('status') == 'INACTIVE':
+        if action == IC_DELETE_ACTION:
             return 'INACTIVE'
         else:
             return 'ACTIVE'

integrated_channels/sap_success_factors/transmitters/content_metadata.py

@@ -19,16 +19,6 @@ def __init__(self, enterprise_configuration, client=SAPSuccessFactorsAPIClient):
             client=client
         )
 
-    def _transmit_action(self, content_metadata_item_map, client_method, action_name):
-        """
-        Set status to INACTIVE for items that should be deleted.
-        """
-        if action_name == 'delete':
-            for _, item in content_metadata_item_map.items():
-                item.channel_metadata['status'] = 'INACTIVE'
-                item.save()
-        return super()._transmit_action(content_metadata_item_map, client_method, action_name)
-
     def _prepare_items_for_transmission(self, channel_metadata_items):
         return {
             'ocnCourses': channel_metadata_items

test_utils/fake_catalog_api.py

@@ -1544,11 +1544,10 @@ def get_fake_catalog_diff_create_w_program():
     Returns a fake response from the EnterpriseCatalogApiClient.get_catalog_diff where two courses and a program are to
     be created
     """
-    # items_to_create, items_to_delete, matched_items
     return [
-        {'content_key': FAKE_COURSE_RUN_TO_CREATE['key']},
-        {'content_key': FAKE_COURSE_TO_CREATE['key']},
-        {'content_key': FAKE_SEARCH_ALL_PROGRAM_RESULT_1_TO_CREATE['uuid']}
+        {'content_key': FAKE_COURSE_RUN['key']},
+        {'content_key': FAKE_COURSE['key']},
+        {'content_key': FAKE_SEARCH_ALL_PROGRAM_RESULT_1['uuid']}
     ], [], []
 
 

tests/test_integrated_channels/test_cornerstone/test_views.py

@@ -14,6 +14,7 @@
 from django.conf import settings
 from django.utils.http import http_date
 
+from enterprise.constants import IC_CREATE_ACTION
 from enterprise.utils import get_enterprise_worker_user
 from integrated_channels.integrated_channel.models import ContentMetadataItemTransmission
 from test_utils import APITest, factories
@@ -85,7 +86,10 @@ def test_course_list_with_skip_key_if_none_false(self):
             )
             worker_user = get_enterprise_worker_user()
             exporter = self.config.get_content_metadata_exporter(worker_user)
-            transformed_item = exporter._transform_item(content_item.channel_metadata)  # pylint: disable=protected-access
+            transformed_item = exporter._transform_item(  # pylint: disable=protected-access
+                content_item.channel_metadata,
+                action=IC_CREATE_ACTION,
+            )
             content_item.channel_metadata = transformed_item
             content_item.save()
 
@@ -126,7 +130,10 @@ def test_course_list(self):
             )
             worker_user = get_enterprise_worker_user()
             exporter = self.config.get_content_metadata_exporter(worker_user)
-            transformed_item = exporter._transform_item(content_item.channel_metadata)  # pylint: disable=protected-access
+            transformed_item = exporter._transform_item(  # pylint: disable=protected-access
+                content_item.channel_metadata,
+                action=IC_CREATE_ACTION,
+            )
             content_item.channel_metadata = transformed_item
             content_item.save()
 
@@ -173,7 +180,10 @@ def test_course_list_last_modified(self):
             )
             worker_user = get_enterprise_worker_user()
             exporter = self.config.get_content_metadata_exporter(worker_user)
-            transformed_item = exporter._transform_item(content_item.channel_metadata)  # pylint: disable=protected-access
+            transformed_item = exporter._transform_item(  # pylint: disable=protected-access
+                content_item.channel_metadata,
+                action=IC_CREATE_ACTION,
+            )
             content_item.channel_metadata = transformed_item
             content_item.save()
 
@@ -211,7 +221,10 @@ def test_course_list_restricted_count(self):
             )
             worker_user = get_enterprise_worker_user()
             exporter = self.config.get_content_metadata_exporter(worker_user)
-            transformed_item = exporter._transform_item(content_item.channel_metadata)  # pylint: disable=protected-access
+            transformed_item = exporter._transform_item(  # pylint: disable=protected-access
+                content_item.channel_metadata,
+                action=IC_CREATE_ACTION,
+            )
             content_item.channel_metadata = transformed_item
             content_item.save()