From 5e9c8f028d55ab45d3fddc105e90806080523be1 Mon Sep 17 00:00:00 2001 From: Alexander J Sheehan Date: Mon, 16 Oct 2023 20:09:58 +0000 Subject: [PATCH 1/2] chore: orchestrator exception handling and submission refinements --- CHANGELOG.rst | 4 ++++ enterprise/__init__.py | 2 +- .../api/v1/views/enterprise_customer_sso_configuration.py | 2 +- enterprise/models.py | 7 ++++--- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 8c1d55d531..9576a20b7d 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -15,6 +15,10 @@ Change Log Unreleased ---------- +[4.6.6] +------- +chore: orchestrator exception handling and submission refinements + [4.6.5] ------- feat: Added logs for Degreed2 client diff --git a/enterprise/__init__.py b/enterprise/__init__.py index 1f15cde935..4a7b6943fd 100644 --- a/enterprise/__init__.py +++ b/enterprise/__init__.py @@ -2,4 +2,4 @@ Your project description goes here. """ -__version__ = "4.6.5" +__version__ = "4.6.6" diff --git a/enterprise/api/v1/views/enterprise_customer_sso_configuration.py b/enterprise/api/v1/views/enterprise_customer_sso_configuration.py index a93156a57c..252f7aed1f 100644 --- a/enterprise/api/v1/views/enterprise_customer_sso_configuration.py +++ b/enterprise/api/v1/views/enterprise_customer_sso_configuration.py @@ -322,7 +322,7 @@ def update(self, request, *args, **kwargs): with transaction.atomic(): sso_configuration_record.update(**request_data) sso_configuration_record.first().submit_for_configuration(updating_existing_record=True) - except (TypeError, FieldDoesNotExist, ValidationError) as e: + except (TypeError, FieldDoesNotExist, ValidationError, SsoOrchestratorClientError) as e: LOGGER.error(f'{CONFIG_UPDATE_ERROR}{e}') return Response({'error': f'{CONFIG_UPDATE_ERROR}{e}'}, status=HTTP_400_BAD_REQUEST) serializer = self.serializer_class(sso_configuration_record.first()) diff --git a/enterprise/models.py b/enterprise/models.py index c883dac36a..4cd6fba832 100644 --- a/enterprise/models.py +++ b/enterprise/models.py @@ -4082,7 +4082,8 @@ def submit_for_configuration(self, updating_existing_record=False): config_data = {} if self.identity_provider == self.SAP_SUCCESS_FACTORS: for field in self.sap_config_fields: - sap_data[utils.camelCase(field)] = getattr(self, field) + if field_value := getattr(self, field): + sap_data[utils.camelCase(field)] = field_value is_sap = True else: for field in self.base_saml_config_fields: @@ -4091,8 +4092,8 @@ def submit_for_configuration(self, updating_existing_record=False): config_data['enable'] = True else: config_data['enable'] = getattr(self, field) - else: - config_data[utils.camelCase(field)] = getattr(self, field) + elif field_value := getattr(self, field): + config_data[utils.camelCase(field)] = field_value EnterpriseSSOOrchestratorApiClient().configure_sso_orchestration_record( config_data=config_data, From 9933465694ca2d02f65cc5093097bec6dccfa609 Mon Sep 17 00:00:00 2001 From: Ehmad Saeed Date: Tue, 17 Oct 2023 01:42:01 +0500 Subject: [PATCH 2/2] feat: parse API Response body of SAPSF to log relevant courses only --- .../transmitters/content_metadata.py | 14 +++++++-- .../transmitters/content_metadata.py | 20 +++++++++++++ .../test_content_metadata.py | 29 +++++++++++++++++++ 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/integrated_channels/integrated_channel/transmitters/content_metadata.py b/integrated_channels/integrated_channel/transmitters/content_metadata.py index 297fe13e07..ece8d95057 100644 --- a/integrated_channels/integrated_channel/transmitters/content_metadata.py +++ b/integrated_channels/integrated_channel/transmitters/content_metadata.py @@ -125,6 +125,14 @@ def _serialize_items(self, channel_metadata_items): sort_keys=True ).encode('utf-8') + def _filter_api_response(self, response, content_id): # pylint: disable=unused-argument + """ + Filter the response from the integrated channel API client. + This can be overridden by subclasses to parse the response + expected by the integrated channel. + """ + return response + def _transmit_action(self, content_metadata_item_map, client_method, action_name): # pylint: disable=too-many-statements """ Do the work of calling the appropriate client method, saving the results, and updating @@ -219,9 +227,9 @@ def _transmit_action(self, content_metadata_item_map, client_method, action_name ) transmission.api_response_status_code = response_status_code was_successful = response_status_code < 300 - + api_content_response = self._filter_api_response(response_body, content_id) if transmission.api_record: - transmission.api_record.body = response_body + transmission.api_record.body = api_content_response transmission.api_record.status_code = response_status_code transmission.api_record.save() else: @@ -230,7 +238,7 @@ def _transmit_action(self, content_metadata_item_map, client_method, action_name 'ApiResponseRecord' ) transmission.api_record = ApiResponseRecord.objects.create( - body=response_body, status_code=response_status_code + body=api_content_response, status_code=response_status_code ) if action_name == 'create': transmission.remote_created_at = action_happened_at diff --git a/integrated_channels/sap_success_factors/transmitters/content_metadata.py b/integrated_channels/sap_success_factors/transmitters/content_metadata.py index e54f7e26b5..916a8e878b 100644 --- a/integrated_channels/sap_success_factors/transmitters/content_metadata.py +++ b/integrated_channels/sap_success_factors/transmitters/content_metadata.py @@ -1,9 +1,14 @@ """ Class for transmitting content metadata to SuccessFactors. """ +import json +import logging + from integrated_channels.integrated_channel.transmitters.content_metadata import ContentMetadataTransmitter from integrated_channels.sap_success_factors.client import SAPSuccessFactorsAPIClient +LOGGER = logging.getLogger(__name__) + class SapSuccessFactorsContentMetadataTransmitter(ContentMetadataTransmitter): """ @@ -19,6 +24,21 @@ def __init__(self, enterprise_configuration, client=SAPSuccessFactorsAPIClient): client=client ) + def _filter_api_response(self, response, content_id): + """ + Filter the response from SAPSF to only include the content + based on the content_id + """ + try: + parsed_response = json.loads(response) + parsed_response["ocnCourses"] = [item for item in parsed_response["ocnCourses"] + if item["courseID"] == content_id] + filtered_response = json.dumps(parsed_response) + return filtered_response + except Exception as exc: # pylint: disable=broad-except + LOGGER.error("Error filtering response from SAPSF: %s", exc) + return response + def transmit(self, create_payload, update_payload, delete_payload): """ Transmit method overriding base transmissions. Due to rate limiting on SAP 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 1f0ccf790e..4ae99c6abf 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 @@ -5,6 +5,7 @@ import unittest from datetime import datetime from unittest import mock +import json import responses from pytest import mark @@ -326,3 +327,31 @@ def test_transmit_api_usage_limit_disabled(self, create_content_metadata_mock): plugin_configuration_id=self.enterprise_config.id, integrated_channel_code=self.enterprise_config.channel_code(), ).count() == 2 + + @mock.patch('integrated_channels.sap_success_factors.transmitters.content_metadata.LOGGER') + def test_fiter_api_response_successful(self, logger_mock): + """ + Test that the api response is successfully filtered + """ + response = '{"ocnCourses": [{"courseID": "course:DemoX"}, {"courseID": "course:DemoX2"}]}' + content_id = 'course:DemoX' + + transmitter = SapSuccessFactorsContentMetadataTransmitter(self.enterprise_config) + filtered_response = transmitter._filter_api_response(response, content_id) + + assert json.loads(filtered_response) == {"ocnCourses": [{"courseID": "course:DemoX2"}]} + assert logger_mock.error.call_count == 0 + + @mock.patch('integrated_channels.sap_success_factors.transmitters.content_metadata.LOGGER') + def test_filter_api_response_exception(self, logger_mock): + """ + Test that the api response is not filtered if an exception occurs + """ + response = 'Invalid JSON response' + content_id = 'course:DemoX' + + transmitter = SapSuccessFactorsContentMetadataTransmitter(self.enterprise_config) + filtered_response = transmitter._filter_api_response(response, content_id) + + assert filtered_response == response + logger_mock.error.assert_called_once()