From 9fc81cbcb2b9026b07678d7ccd801fed1e5212df Mon Sep 17 00:00:00 2001 From: Alexander J Sheehan Date: Fri, 7 Jul 2023 16:19:32 +0000 Subject: [PATCH] fix: catching orphaned records without catalog uuids --- CHANGELOG.rst | 4 + enterprise/__init__.py | 2 +- enterprise/constants.py | 4 + .../exporters/content_metadata.py | 112 +++++++------- .../integrated_channel/models.py | 6 +- .../transmitters/content_metadata.py | 2 +- .../exporters/content_metadata.py | 5 +- .../transmitters/content_metadata.py | 10 -- test_utils/fake_catalog_api.py | 7 +- .../test_cornerstone/test_views.py | 21 ++- .../test_exporters/test_content_metadata.py | 141 +++++++++++++++++- .../test_exporters/test_content_metadata.py | 51 +++++++ .../test_content_metadata.py | 1 - tests/test_management.py | 9 ++ 14 files changed, 289 insertions(+), 86 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index c5c0f55238..ead49237b9 100644 --- a/CHANGELOG.rst +++ b/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)` diff --git a/enterprise/__init__.py b/enterprise/__init__.py index a05ed9eb57..9c2708d202 100644 --- a/enterprise/__init__.py +++ b/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" diff --git a/enterprise/constants.py b/enterprise/constants.py index 9415fbd1c9..eeb57a4dea 100644 --- a/enterprise/constants.py +++ b/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' diff --git a/integrated_channels/integrated_channel/exporters/content_metadata.py b/integrated_channels/integrated_channel/exporters/content_metadata.py index d156c74caa..2337994da3 100644 --- a/integrated_channels/integrated_channel/exporters/content_metadata.py +++ b/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. diff --git a/integrated_channels/integrated_channel/models.py b/integrated_channels/integrated_channel/models.py index 753ed24ec8..1cfe9db1fa 100644 --- a/integrated_channels/integrated_channel/models.py +++ b/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): """ diff --git a/integrated_channels/integrated_channel/transmitters/content_metadata.py b/integrated_channels/integrated_channel/transmitters/content_metadata.py index 6f3bca3701..49bb9a5d01 100644 --- a/integrated_channels/integrated_channel/transmitters/content_metadata.py +++ b/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( diff --git a/integrated_channels/sap_success_factors/exporters/content_metadata.py b/integrated_channels/sap_success_factors/exporters/content_metadata.py index 6c1b8e9e75..ba9897073e 100644 --- a/integrated_channels/sap_success_factors/exporters/content_metadata.py +++ b/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' diff --git a/integrated_channels/sap_success_factors/transmitters/content_metadata.py b/integrated_channels/sap_success_factors/transmitters/content_metadata.py index b675a4993d..6383d78e5d 100644 --- a/integrated_channels/sap_success_factors/transmitters/content_metadata.py +++ b/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 diff --git a/test_utils/fake_catalog_api.py b/test_utils/fake_catalog_api.py index d20aa22390..0778a13003 100644 --- a/test_utils/fake_catalog_api.py +++ b/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']} ], [], [] diff --git a/tests/test_integrated_channels/test_cornerstone/test_views.py b/tests/test_integrated_channels/test_cornerstone/test_views.py index 6ceb3abd60..b39d55cb32 100644 --- a/tests/test_integrated_channels/test_cornerstone/test_views.py +++ b/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() diff --git a/tests/test_integrated_channels/test_integrated_channel/test_exporters/test_content_metadata.py b/tests/test_integrated_channels/test_integrated_channel/test_exporters/test_content_metadata.py index 74aa398c89..315cef0570 100644 --- a/tests/test_integrated_channels/test_integrated_channel/test_exporters/test_content_metadata.py +++ b/tests/test_integrated_channels/test_integrated_channel/test_exporters/test_content_metadata.py @@ -17,6 +17,7 @@ from enterprise.utils import get_content_metadata_item_id from integrated_channels.integrated_channel.exporters.content_metadata import ContentMetadataExporter from integrated_channels.integrated_channel.models import ContentMetadataItemTransmission +from integrated_channels.sap_success_factors.exporters.content_metadata import SapSuccessFactorsContentMetadataExporter from test_utils import FAKE_UUIDS, factories from test_utils.factories import ContentMetadataItemTransmissionFactory from test_utils.fake_catalog_api import ( @@ -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( + 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. @@ -149,6 +262,14 @@ def test_content_exporter_create_export(self, mock_get_catalog_diff, mock_get_co """ ``ContentMetadataExporter``'s ``export`` produces a JSON dump of the course data. """ + ContentMetadataExporter.DATA_TRANSFORM_MAPPING = { + 'contentId': 'key', + 'title': 'title', + 'description': 'description', + 'imageUrl': 'image', + 'url': 'enrollment_url', + 'language': 'content_language' + } mock_get_content_metadata.return_value = get_fake_content_metadata() mock_get_catalog_diff.return_value = get_fake_catalog_diff_create() exporter = ContentMetadataExporter('fake-user', self.config) @@ -160,8 +281,7 @@ def test_content_exporter_create_export(self, mock_get_catalog_diff, mock_get_co assert mock_get_content_metadata.get(FAKE_COURSE_RUN['key']) for key in create_payload: - assert key in ['edX+DemoX', 'course-v1:edX+DemoX+Demo_Course', FAKE_UUIDS[3]] - + assert key in [FAKE_COURSE['key'], FAKE_COURSE_RUN.get('key'), FAKE_UUIDS[3]] cmit = ContentMetadataItemTransmission.objects.get(content_id=FAKE_COURSE['key']) assert cmit.content_title is not None @@ -244,13 +364,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 +448,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 +475,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') @@ -417,9 +541,10 @@ def test_content_exporter_bad_data_transform_mapping(self, mock_api_client): """ ``ContentMetadataExporter``'s ``export`` raises an exception when DATA_TRANSFORM_MAPPING is invalid. """ - content_id = 'course:DemoX' + fake_content_metadata = get_fake_content_metadata() + content_id = fake_content_metadata[0].get('key') - mock_api_client.return_value.get_content_metadata.return_value = get_fake_content_metadata() + mock_api_client.return_value.get_content_metadata.return_value = fake_content_metadata mock_create_items = [{'content_key': content_id}] mock_delete_items = {} mock_matched_items = [] diff --git a/tests/test_integrated_channels/test_sap_success_factors/test_exporters/test_content_metadata.py b/tests/test_integrated_channels/test_sap_success_factors/test_exporters/test_content_metadata.py index ed2e4681eb..5740252d75 100644 --- a/tests/test_integrated_channels/test_sap_success_factors/test_exporters/test_content_metadata.py +++ b/tests/test_integrated_channels/test_sap_success_factors/test_exporters/test_content_metadata.py @@ -8,6 +8,7 @@ import responses from pytest import mark +from enterprise.constants import IC_DELETE_ACTION from enterprise.utils import parse_lms_api_datetime from integrated_channels.sap_success_factors.exporters.content_metadata import SapSuccessFactorsContentMetadataExporter from integrated_channels.utils import parse_datetime_to_epoch_millis @@ -30,6 +31,56 @@ def setUp(self): super().setUp() + def test_exporter_delete_transform(self): + exporter = SapSuccessFactorsContentMetadataExporter('fake-user', self.config) + # pylint: disable=protected-access + transformed_delete_metadata = exporter._transform_item({ + "aggregation_key": "course:NYIF+BOPC.4x", + "content_type": "course", + "full_description": "ayylmao", + "key": 'ayylmao', + "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" + }, action=IC_DELETE_ACTION) + assert transformed_delete_metadata['status'] == 'INACTIVE' + assert transformed_delete_metadata['schedule'] == [] + @ddt.data( ( # advertised course run has price, use it diff --git a/tests/test_integrated_channels/test_sap_success_factors/test_transmitters/test_content_metadata.py b/tests/test_integrated_channels/test_sap_success_factors/test_transmitters/test_content_metadata.py index fc1aa1df04..be6f3e65d6 100644 --- a/tests/test_integrated_channels/test_sap_success_factors/test_transmitters/test_content_metadata.py +++ b/tests/test_integrated_channels/test_sap_success_factors/test_transmitters/test_content_metadata.py @@ -251,7 +251,6 @@ def test_transmit_content_metadata_updates_records( content_id=content_id_2, ).first() assert item_deleted.remote_deleted_at - assert item_deleted.channel_metadata.get('status') == 'INACTIVE' item_created = ContentMetadataItemTransmission.objects.filter( enterprise_customer_catalog_uuid=self.enterprise_customer_catalog.uuid, content_id=content_id_3, diff --git a/tests/test_management.py b/tests/test_management.py index ec6ad6b365..9e30713324 100644 --- a/tests/test_management.py +++ b/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