Skip to content

Commit

Permalink
Merge pull request #1921 from openedx/asaeed/ENT-7715
Browse files Browse the repository at this point in the history
[ENT-7715] fix: filter the response only after a successful response status
  • Loading branch information
justEhmadSaeed authored Oct 24, 2023
2 parents b8dfb34 + 4064874 commit 2714365
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +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)
api_content_response = response_body
if was_successful:
api_content_response = self._filter_api_response(api_content_response, content_id)
if transmission.api_record:
transmission.api_record.body = api_content_response
transmission.api_record.status_code = response_status_code
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from integrated_channels.integrated_channel.transmitters.content_metadata import ContentMetadataTransmitter
from integrated_channels.sap_success_factors.client import SAPSuccessFactorsAPIClient
from integrated_channels.utils import log_exception

LOGGER = logging.getLogger(__name__)

Expand Down Expand Up @@ -36,7 +37,12 @@ def _filter_api_response(self, response, content_id):
filtered_response = json.dumps(parsed_response)
return filtered_response
except Exception as exc: # pylint: disable=broad-except
LOGGER.exception("Error filtering response from SAPSF for Course: %s, %s", content_id, exc)
log_exception(
self.enterprise_configuration,
f'Failed to filter the API response: '
f'{exc}',
content_id
)
return response

def transmit(self, create_payload, update_payload, delete_payload):
Expand Down
12 changes: 12 additions & 0 deletions integrated_channels/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,18 @@ def generate_formatted_log(
f'integrated_channel_plugin_configuration_id={plugin_configuration_id}, {message}'


def log_exception(enterprise_configuration, msg, course_or_course_run_key=None):
LOGGER.exception(
generate_formatted_log(
channel_name=enterprise_configuration.channel_code(),
enterprise_customer_uuid=enterprise_configuration.enterprise_customer.uuid,
course_or_course_run_key=course_or_course_run_key,
plugin_configuration_id=enterprise_configuration.id,
message=msg
)
)


def refresh_session_if_expired(
oauth_access_token_function,
session=None,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ def test_transmit_api_usage_limit_disabled(self, create_content_metadata_mock):
integrated_channel_code=self.enterprise_config.channel_code(),
).count() == 2

@mock.patch('integrated_channels.sap_success_factors.transmitters.content_metadata.LOGGER')
@mock.patch('integrated_channels.utils.LOGGER')
def test_filter_api_response_successful(self, logger_mock):
"""
Test that the api response is successfully filtered
Expand All @@ -343,7 +343,7 @@ def test_filter_api_response_successful(self, logger_mock):
assert json.loads(filtered_response) == {"ocnCourses": [{"courseID": "course:DemoX"}]}
assert logger_mock.exception.call_count == 0

@mock.patch('integrated_channels.sap_success_factors.transmitters.content_metadata.LOGGER')
@mock.patch('integrated_channels.utils.LOGGER')
def test_filter_api_response_exception(self, logger_mock):
"""
Test that the api response is not filtered if an exception occurs
Expand Down

0 comments on commit 2714365

Please sign in to comment.