From 954d2d5384961715b1774e06de59f89f852f4183 Mon Sep 17 00:00:00 2001 From: Ismail Veruru Shaik Date: Tue, 26 May 2026 11:18:31 +0000 Subject: [PATCH 01/11] feat: Addressed comments for PR B --- .../api_client/enterprise_catalog_client.py | 57 +++++++++ .../tests/test_enterprise_catalog_client.py | 112 ++++++++++++++++++ 2 files changed, 169 insertions(+) diff --git a/enterprise_access/apps/api_client/enterprise_catalog_client.py b/enterprise_access/apps/api_client/enterprise_catalog_client.py index a976685e..ccdcabf3 100644 --- a/enterprise_access/apps/api_client/enterprise_catalog_client.py +++ b/enterprise_access/apps/api_client/enterprise_catalog_client.py @@ -24,6 +24,39 @@ def __init__(self): self.enterprise_catalog_endpoint = urljoin(self.api_base_url, 'enterprise-catalogs/') super().__init__() + def _fetch_all_pages(self, endpoint, params=None): + """Fetch and merge paginated enterprise-catalog responses into one payload.""" + merged_results = [] + next_url = endpoint + request_params = params + base_payload = None + + while next_url: + response = self.client.get(next_url, params=request_params) + response.raise_for_status() + payload = response.json() + + if not isinstance(payload, dict): + return payload + + if base_payload is None: + base_payload = payload.copy() + + page_results = payload.get('results') + if isinstance(page_results, list): + merged_results.extend(page_results) + + next_url = payload.get('next') + request_params = None + + if base_payload is None: + return {'count': 0, 'next': None, 'previous': None, 'results': []} + + base_payload['results'] = merged_results + base_payload['count'] = len(merged_results) + base_payload['next'] = None + return base_payload + @backoff.on_exception(wait_gen=backoff.expo, exception=autoretry_for_exceptions) def get_academy(self, academy_uuid): """ @@ -110,6 +143,30 @@ def get_content_metadata_count(self, catalog_uuid): response.raise_for_status() return response.json()['count'] + @backoff.on_exception(wait_gen=backoff.expo, exception=autoretry_for_exceptions) + def get_academies(self, academy_uuid: str | None = None) -> dict: + """ + Fetch academies for Essentials flows from enterprise-catalog. + + Returns: + dict: Paginated response shape with keys including count/next/previous/results. + If the endpoint paginates, all pages are merged into a single response payload. + """ + params = {'academy_uuid': str(academy_uuid)} if academy_uuid else None + return self._fetch_all_pages(self.academies_endpoint, params=params) + + @backoff.on_exception(wait_gen=backoff.expo, exception=autoretry_for_exceptions) + def get_catalogs(self, enterprise_customer_uuid: str | None = None) -> dict: + """ + Fetch enterprise catalogs, optionally scoped to an enterprise customer UUID. + + Returns: + dict: Paginated response shape with keys including count/next/previous/results. + If the endpoint paginates, all pages are merged into a single response payload. + """ + params = {'enterprise_customer': str(enterprise_customer_uuid)} if enterprise_customer_uuid else None + return self._fetch_all_pages(self.enterprise_catalog_endpoint, params=params) + def content_metadata(self, content_id): raise NotImplementedError('There is currently no v2 API implementation for this endpoint.') diff --git a/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py b/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py index 29ef6037..c0393723 100644 --- a/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py +++ b/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py @@ -123,6 +123,118 @@ def test_get_content_metadata_count(self, mock_oauth_client): f'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/{catalog_uuid}/get_content_metadata/', ) + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') + def test_get_academies(self, mock_oauth_client): + mock_response_json = {'count': 1, 'next': None, 'previous': None, 'results': [{'title': 'AI Academy'}]} + mock_oauth_client.return_value.get.return_value.json.return_value = mock_response_json + + client = EnterpriseCatalogApiClient() + fetched = client.get_academies() + + self.assertEqual(fetched, mock_response_json) + mock_oauth_client.return_value.get.assert_called_with( + 'http://enterprise-catalog.example.com/api/v2/academies/', + params=None, + ) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') + def test_get_academies_with_uuid(self, mock_oauth_client): + mock_response_json = {'count': 0, 'next': None, 'previous': None, 'results': []} + mock_oauth_client.return_value.get.return_value.json.return_value = mock_response_json + + academy_uuid = uuid4() + client = EnterpriseCatalogApiClient() + fetched = client.get_academies(academy_uuid=str(academy_uuid)) + + self.assertEqual(fetched, mock_response_json) + mock_oauth_client.return_value.get.assert_called_with( + 'http://enterprise-catalog.example.com/api/v2/academies/', + params={'academy_uuid': str(academy_uuid)}, + ) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') + def test_get_catalogs(self, mock_oauth_client): + mock_response_json = {'count': 1, 'next': None, 'previous': None, 'results': [{'uuid': str(uuid4())}]} + mock_oauth_client.return_value.get.return_value.json.return_value = mock_response_json + + client = EnterpriseCatalogApiClient() + fetched = client.get_catalogs() + + self.assertEqual(fetched, mock_response_json) + mock_oauth_client.return_value.get.assert_called_with( + 'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/', + params=None, + ) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') + def test_get_catalogs_with_enterprise_customer(self, mock_oauth_client): + mock_response_json = {'count': 1, 'next': None, 'previous': None, 'results': [{'uuid': str(uuid4())}]} + mock_oauth_client.return_value.get.return_value.json.return_value = mock_response_json + + customer_uuid = str(uuid4()) + client = EnterpriseCatalogApiClient() + fetched = client.get_catalogs(enterprise_customer_uuid=customer_uuid) + + self.assertEqual(fetched, mock_response_json) + mock_oauth_client.return_value.get.assert_called_with( + 'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/', + params={'enterprise_customer': customer_uuid}, + ) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') + def test_get_academies_merges_paginated_results(self, mock_oauth_client): + page_1 = { + 'count': 2, + 'next': 'http://enterprise-catalog.example.com/api/v2/academies/?page=2', + 'previous': None, + 'results': [{'title': 'AI Academy'}], + } + page_2 = { + 'count': 2, + 'next': None, + 'previous': 'http://enterprise-catalog.example.com/api/v2/academies/?page=1', + 'results': [{'title': 'Data Academy'}], + } + mock_oauth_client.return_value.get.side_effect = [ + mock.Mock(json=mock.Mock(return_value=page_1), raise_for_status=mock.Mock()), + mock.Mock(json=mock.Mock(return_value=page_2), raise_for_status=mock.Mock()), + ] + + client = EnterpriseCatalogApiClient() + fetched = client.get_academies() + + self.assertEqual(fetched['count'], 2) + self.assertEqual(len(fetched['results']), 2) + self.assertIsNone(fetched['next']) + self.assertEqual(mock_oauth_client.return_value.get.call_count, 2) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') + def test_get_catalogs_merges_paginated_results(self, mock_oauth_client): + page_1 = { + 'count': 2, + 'next': 'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/?page=2', + 'previous': None, + 'results': [{'uuid': str(uuid4())}], + } + page_2 = { + 'count': 2, + 'next': None, + 'previous': 'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/?page=1', + 'results': [{'uuid': str(uuid4())}], + } + mock_oauth_client.return_value.get.side_effect = [ + mock.Mock(json=mock.Mock(return_value=page_1), raise_for_status=mock.Mock()), + mock.Mock(json=mock.Mock(return_value=page_2), raise_for_status=mock.Mock()), + ] + + client = EnterpriseCatalogApiClient() + fetched = client.get_catalogs() + + self.assertEqual(fetched['count'], 2) + self.assertEqual(len(fetched['results']), 2) + self.assertIsNone(fetched['next']) + self.assertEqual(mock_oauth_client.return_value.get.call_count, 2) + @ddt.ddt class TestEnterpriseCatalogApiV1Client(TestCase): From aaf41d1910254a4b6577d7618f5af500b3fb8e2f Mon Sep 17 00:00:00 2001 From: Ismail Veruru Shaik Date: Mon, 22 Jun 2026 17:30:26 +0000 Subject: [PATCH 02/11] feat: catalog client implementation --- .../api_client/enterprise_catalog_client.py | 237 +++++++--- .../tests/test_enterprise_catalog_client.py | 447 +++++++++++++++++- 2 files changed, 629 insertions(+), 55 deletions(-) diff --git a/enterprise_access/apps/api_client/enterprise_catalog_client.py b/enterprise_access/apps/api_client/enterprise_catalog_client.py index ccdcabf3..225cc775 100644 --- a/enterprise_access/apps/api_client/enterprise_catalog_client.py +++ b/enterprise_access/apps/api_client/enterprise_catalog_client.py @@ -19,44 +19,10 @@ class EnterpriseCatalogApiClient(BaseOAuthClient): def __init__(self): self.api_base_url = urljoin(settings.ENTERPRISE_CATALOG_URL, f'api/{self.api_version}/') - # Academies are exposed on v1 of the enterprise-catalog API, not v2. - self.academies_endpoint = urljoin(settings.ENTERPRISE_CATALOG_URL, 'api/v1/academies/') + self.academies_endpoint = urljoin(self.api_base_url, 'academies/') self.enterprise_catalog_endpoint = urljoin(self.api_base_url, 'enterprise-catalogs/') super().__init__() - def _fetch_all_pages(self, endpoint, params=None): - """Fetch and merge paginated enterprise-catalog responses into one payload.""" - merged_results = [] - next_url = endpoint - request_params = params - base_payload = None - - while next_url: - response = self.client.get(next_url, params=request_params) - response.raise_for_status() - payload = response.json() - - if not isinstance(payload, dict): - return payload - - if base_payload is None: - base_payload = payload.copy() - - page_results = payload.get('results') - if isinstance(page_results, list): - merged_results.extend(page_results) - - next_url = payload.get('next') - request_params = None - - if base_payload is None: - return {'count': 0, 'next': None, 'previous': None, 'results': []} - - base_payload['results'] = merged_results - base_payload['count'] = len(merged_results) - base_payload['next'] = None - return base_payload - @backoff.on_exception(wait_gen=backoff.expo, exception=autoretry_for_exceptions) def get_academy(self, academy_uuid): """ @@ -73,6 +39,121 @@ def get_academy(self, academy_uuid): response.raise_for_status() return response.json() + @backoff.on_exception(wait_gen=backoff.expo, exception=autoretry_for_exceptions) + def get_academies(self, academy_uuid: str = None, is_active: bool | None = None) -> dict | list: + """ + Fetch a list of academies, optionally filtered by academy_uuid. + + Returns a paginated-style dict or a raw list depending on the upstream service. + If the response contains a `next` link, subsequent pages will be fetched and merged into a single + results list. + """ + # Defensive: if no endpoint configured, return empty paginated shape + if not self.academies_endpoint: + return {'count': 0, 'next': None, 'previous': None, 'results': []} + + params = {} + if academy_uuid: + params['academy_uuid'] = academy_uuid + if is_active is not None: + params['is_active'] = bool(is_active) + if not params: + params = None + response = self.client.get(self.academies_endpoint, params=params) + response.raise_for_status() + raw_payload = response.json() + + def normalize_page(payload): + """Normalize a response payload into a paginated dict. + + Behavior is intentionally conservative to match historical expectations: + - If the service returned a non-dict (e.g. a list), return it unchanged. + - If the service returned a dict without a list-valued `results`, treat + `results` as empty (do not coerce non-list into a single-item list). + - Preserve the incoming `count` value when present; only fall back to + computed lengths when absent. + """ + if payload is None: + return ({'count': 0, 'results': [], 'next': None, 'previous': None}, False) + # If upstream returned a raw list, preserve that shape + if isinstance(payload, list): + return payload, False + if isinstance(payload, dict): + results = payload.get('results') + # If results is missing or not a list, treat as empty list + if not isinstance(results, list): + results = [] + raw_count = payload.get('count', None) + # Try to coerce incoming count to an int; if that fails, treat + # it as absent so we fall back to the computed length. + try: + count = int(raw_count) if raw_count is not None else None + except (ValueError, TypeError): + count = None + explicit_count = count is not None + if count is None: + count = len(results) + return ({ + 'count': count, + 'results': results, + 'next': payload.get('next'), + 'previous': payload.get('previous'), + }, explicit_count) + # Fallback: for unexpected types, wrap as single-item paginated dict + return ({'count': 1, 'results': [payload], 'next': None, 'previous': None}, False) + + # If upstream returned a raw list, preserve that shape + if isinstance(raw_payload, list): + return raw_payload + + data, explicit = normalize_page(raw_payload) + + # Merge paginated results if necessary; be defensive about types + + next_url = data.get('next') + while next_url: + next_resp = self.client.get(next_url) + next_resp.raise_for_status() + next_payload = next_resp.json() + next_data, next_explicit = normalize_page(next_payload) + # Only extend if both are lists + if isinstance(data.get('results'), list) and isinstance(next_data.get('results'), list): + data['results'].extend(next_data.get('results', [])) + # update pagination markers + data['next'] = next_data.get('next') + data['previous'] = data.get('previous') or next_data.get('previous') + # If the upstream provided an explicit numeric count, preserve it. + # Otherwise recompute the total as the length of the merged results. + # If either page did not provide an explicit numeric count, recompute + explicit = explicit and next_explicit + if not explicit: + data['count'] = len(data.get('results', [])) + next_url = data.get('next') + return data + + @backoff.on_exception(wait_gen=backoff.expo, exception=autoretry_for_exceptions) + def associate_academy_with_catalog(self, academy_uuid, enterprise_catalog_uuid): + """ + Associate an academy with an enterprise catalog in enterprise-catalog. + + Arguments: + academy_uuid (str|UUID): UUID of the academy to update. + enterprise_catalog_uuid (str|UUID): UUID of the enterprise catalog to associate. + + Returns: + dict: Response payload, or an empty dict when the endpoint returns no body. + """ + endpoint = urljoin(self.academies_endpoint, f'{academy_uuid}/associate-catalog/') + response = self.client.post( + endpoint, + json={'enterprise_catalog_uuid': str(enterprise_catalog_uuid)}, + ) + response.raise_for_status() + try: + return response.json() + except ValueError: + return {} + @backoff.on_exception(wait_gen=backoff.expo, exception=autoretry_for_exceptions) def contains_content_items(self, catalog_uuid, content_ids): """ @@ -144,28 +225,76 @@ def get_content_metadata_count(self, catalog_uuid): return response.json()['count'] @backoff.on_exception(wait_gen=backoff.expo, exception=autoretry_for_exceptions) - def get_academies(self, academy_uuid: str | None = None) -> dict: + def get_catalogs(self, enterprise_customer_uuid: str = None) -> dict | list: """ - Fetch academies for Essentials flows from enterprise-catalog. + Fetch a list of enterprise catalogs, optionally filtered by enterprise_customer_uuid. - Returns: - dict: Paginated response shape with keys including count/next/previous/results. - If the endpoint paginates, all pages are merged into a single response payload. + Returns a paginated-style dict or a raw list depending on the upstream service. + If the response contains a `next` link, subsequent pages will be fetched and merged into a single + results list. """ - params = {'academy_uuid': str(academy_uuid)} if academy_uuid else None - return self._fetch_all_pages(self.academies_endpoint, params=params) - - @backoff.on_exception(wait_gen=backoff.expo, exception=autoretry_for_exceptions) - def get_catalogs(self, enterprise_customer_uuid: str | None = None) -> dict: - """ - Fetch enterprise catalogs, optionally scoped to an enterprise customer UUID. - - Returns: - dict: Paginated response shape with keys including count/next/previous/results. - If the endpoint paginates, all pages are merged into a single response payload. - """ - params = {'enterprise_customer': str(enterprise_customer_uuid)} if enterprise_customer_uuid else None - return self._fetch_all_pages(self.enterprise_catalog_endpoint, params=params) + params = {'enterprise_customer': enterprise_customer_uuid} if enterprise_customer_uuid else None + response = self.client.get(self.enterprise_catalog_endpoint, params=params) + response.raise_for_status() + raw_payload = response.json() + + def normalize_page(payload): + """Normalize a response payload into a paginated dict. + + This mirrors `get_academies()` defensive behavior so callers can + safely handle dict or list payloads from upstream services. + Returns a tuple of (paginated_dict, explicit_count_flag) when + payload is dict-like, or (list_payload, False) when payload is + a raw list. When payload is None, return an empty paginated shape + and False for explicit flag. + """ + if payload is None: + return ({'count': 0, 'results': [], 'next': None, 'previous': None}, False) + if isinstance(payload, list): + return payload, False + if isinstance(payload, dict): + results = payload.get('results') + if not isinstance(results, list): + results = [] + raw_count = payload.get('count', None) + try: + count = int(raw_count) if raw_count is not None else None + except (ValueError, TypeError): + count = None + explicit_count = count is not None + if count is None: + count = len(results) + return ({ + 'count': count, + 'results': results, + 'next': payload.get('next'), + 'previous': payload.get('previous'), + }, explicit_count) + return ({'count': 1, 'results': [payload], 'next': None, 'previous': None}, False) + + # Preserve raw list payloads + if isinstance(raw_payload, list): + return raw_payload + + data, explicit = normalize_page(raw_payload) + + # Merge paginated results if necessary; be defensive about types + next_url = data.get('next') + while next_url: + next_resp = self.client.get(next_url) + next_resp.raise_for_status() + next_payload = next_resp.json() + next_data, next_explicit = normalize_page(next_payload) + if isinstance(data.get('results'), list) and isinstance(next_data.get('results'), list): + data['results'].extend(next_data.get('results', [])) + data['next'] = next_data.get('next') + data['previous'] = data.get('previous') or next_data.get('previous') + explicit = explicit and next_explicit + if not explicit: + data['count'] = len(data.get('results', [])) + next_url = data.get('next') + + return data def content_metadata(self, content_id): raise NotImplementedError('There is currently no v2 API implementation for this endpoint.') diff --git a/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py b/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py index c0393723..43d94a3b 100644 --- a/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py +++ b/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py @@ -152,6 +152,24 @@ def test_get_academies_with_uuid(self, mock_oauth_client): params={'academy_uuid': str(academy_uuid)}, ) + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') + def test_get_academy_fetches_single_record_from_v2(self, mock_oauth_client): + """Ensure `get_academy` calls the v2 academies endpoint and returns the academy JSON.""" + academy_uuid = uuid4() + mock_resp = mock.Mock() + mock_resp.status_code = 200 + mock_resp.json.return_value = {'uuid': str(academy_uuid), 'title': 'Test Academy'} + mock_resp.raise_for_status = mock.Mock() + mock_oauth_client.return_value.get.return_value = mock_resp + + client = EnterpriseCatalogApiClient() + result = client.get_academy(academy_uuid) + + self.assertEqual(result, {'uuid': str(academy_uuid), 'title': 'Test Academy'}) + mock_oauth_client.return_value.get.assert_called_with( + f'http://enterprise-catalog.example.com/api/v2/academies/{academy_uuid}/' + ) + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') def test_get_catalogs(self, mock_oauth_client): mock_response_json = {'count': 1, 'next': None, 'previous': None, 'results': [{'uuid': str(uuid4())}]} @@ -181,6 +199,66 @@ def test_get_catalogs_with_enterprise_customer(self, mock_oauth_client): params={'enterprise_customer': customer_uuid}, ) + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) + def test_get_catalogs_handles_none_payload(self, mock_oauth_client): + mock_oauth_client.return_value.get.return_value = mock.Mock( + json=mock.Mock(return_value=None), + raise_for_status=mock.Mock(), + ) + + client = EnterpriseCatalogApiClient() + result = client.get_catalogs() + + self.assertEqual(result, {'count': 0, 'results': [], 'next': None, 'previous': None}) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) + def test_get_catalogs_preserves_raw_list_payload(self, mock_oauth_client): + payload = [{'uuid': str(uuid4())}, {'uuid': str(uuid4())}] + mock_oauth_client.return_value.get.return_value = mock.Mock( + json=mock.Mock(return_value=payload), + raise_for_status=mock.Mock(), + ) + + client = EnterpriseCatalogApiClient() + result = client.get_catalogs() + + self.assertEqual(result, payload) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) + def test_get_catalogs_ignores_non_list_results_and_invalid_count(self, mock_oauth_client): + payload = { + 'count': 'not-an-int', + 'next': None, + 'previous': None, + 'results': {'uuid': str(uuid4())}, + } + mock_oauth_client.return_value.get.return_value = mock.Mock( + json=mock.Mock(return_value=payload), + raise_for_status=mock.Mock(), + ) + + client = EnterpriseCatalogApiClient() + result = client.get_catalogs() + + self.assertEqual(result, {'count': 0, 'results': [], 'next': None, 'previous': None}) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) + def test_get_catalogs_wraps_unexpected_payload_type(self, mock_oauth_client): + mock_oauth_client.return_value.get.return_value = mock.Mock( + json=mock.Mock(return_value='catalog-value'), + raise_for_status=mock.Mock(), + ) + + client = EnterpriseCatalogApiClient() + result = client.get_catalogs() + + self.assertEqual(result, { + 'count': 1, + 'results': ['catalog-value'], + 'next': None, + 'previous': None, + }) + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') def test_get_academies_merges_paginated_results(self, mock_oauth_client): page_1 = { @@ -235,6 +313,347 @@ def test_get_catalogs_merges_paginated_results(self, mock_oauth_client): self.assertIsNone(fetched['next']) self.assertEqual(mock_oauth_client.return_value.get.call_count, 2) + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) + def test_associate_academy_with_catalog(self, mock_oauth_client): + academy_uuid = uuid4() + catalog_uuid = uuid4() + mock_post = mock_oauth_client.return_value.post + mock_post.return_value.json.return_value = {'detail': 'ok'} + + client = EnterpriseCatalogApiClient() + result = client.associate_academy_with_catalog(academy_uuid, catalog_uuid) + + self.assertEqual(result, {'detail': 'ok'}) + mock_post.assert_called_once_with( + f'http://enterprise-catalog.example.com/api/v2/academies/{academy_uuid}/associate-catalog/', + json={'enterprise_catalog_uuid': str(catalog_uuid)}, + ) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) + def test_associate_academy_with_catalog_empty_response_body(self, mock_oauth_client): + academy_uuid = uuid4() + catalog_uuid = uuid4() + mock_post = mock_oauth_client.return_value.post + mock_post.return_value.json.side_effect = ValueError() + + client = EnterpriseCatalogApiClient() + result = client.associate_academy_with_catalog(academy_uuid, catalog_uuid) + + self.assertEqual(result, {}) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) + def test_get_academies_returns_non_dict_payload(self, mock_oauth_client): + payload = ['not-a-dict'] + mock_oauth_client.return_value.get.return_value = mock.Mock( + json=mock.Mock(return_value=payload), + raise_for_status=mock.Mock(), + ) + + client = EnterpriseCatalogApiClient() + result = client.get_academies() + + self.assertEqual(result, payload) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) + def test_get_academies_with_empty_endpoint_returns_empty_payload(self, mock_oauth_client): + client = EnterpriseCatalogApiClient() + client.academies_endpoint = '' + + result = client.get_academies() + + self.assertEqual(result, {'count': 0, 'next': None, 'previous': None, 'results': []}) + mock_oauth_client.return_value.get.assert_not_called() + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) + def test_get_academies_ignores_non_list_results(self, mock_oauth_client): + payload = {'count': 1, 'next': None, 'previous': None, 'results': {'title': 'not-a-list'}} + mock_oauth_client.return_value.get.return_value = mock.Mock( + json=mock.Mock(return_value=payload), + raise_for_status=mock.Mock(), + ) + + client = EnterpriseCatalogApiClient() + result = client.get_academies() + + self.assertEqual(result['results'], []) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) + def test_get_academies_with_is_active_param(self, mock_oauth_client): + mock_oauth_client.return_value.get.return_value = mock.Mock( + json=mock.Mock(return_value={'count': 0, 'next': None, 'previous': None, 'results': []}), + raise_for_status=mock.Mock(), + ) + + client = EnterpriseCatalogApiClient() + result = client.get_academies(is_active=True) + + self.assertEqual(result['results'], []) + mock_oauth_client.return_value.get.assert_called_with( + 'http://enterprise-catalog.example.com/api/v2/academies/', + params={'is_active': True}, + ) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) + def test_get_academies_merges_paginated_results_missing_count(self, mock_oauth_client): + page_1 = { + 'count': None, + 'next': 'http://enterprise-catalog.example.com/api/v2/academies/?page=2', + 'previous': None, + 'results': [{'title': 'AI Academy'}], + } + page_2 = { + 'count': None, + 'next': None, + 'previous': 'http://enterprise-catalog.example.com/api/v2/academies/?page=1', + 'results': [{'title': 'Data Academy'}], + } + mock_oauth_client.return_value.get.side_effect = [ + mock.Mock(json=mock.Mock(return_value=page_1), raise_for_status=mock.Mock()), + mock.Mock(json=mock.Mock(return_value=page_2), raise_for_status=mock.Mock()), + ] + + client = EnterpriseCatalogApiClient() + fetched = client.get_academies() + + self.assertEqual(fetched['count'], 2) + self.assertEqual(len(fetched['results']), 2) + self.assertIsNone(fetched['next']) + self.assertEqual(mock_oauth_client.return_value.get.call_count, 2) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) + def test_get_academies_merges_paginated_results_non_int_count(self, mock_oauth_client): + page_1 = { + 'count': 'not-an-int', + 'next': 'http://enterprise-catalog.example.com/api/v2/academies/?page=2', + 'previous': None, + 'results': [{'title': 'AI Academy'}], + } + page_2 = { + 'count': 'also-not-int', + 'next': None, + 'previous': 'http://enterprise-catalog.example.com/api/v2/academies/?page=1', + 'results': [{'title': 'Data Academy'}], + } + mock_oauth_client.return_value.get.side_effect = [ + mock.Mock(json=mock.Mock(return_value=page_1), raise_for_status=mock.Mock()), + mock.Mock(json=mock.Mock(return_value=page_2), raise_for_status=mock.Mock()), + ] + + client = EnterpriseCatalogApiClient() + fetched = client.get_academies() + + # non-int counts should be handled gracefully and fallback to length + self.assertEqual(fetched['count'], 2) + self.assertEqual(len(fetched['results']), 2) + self.assertIsNone(fetched['next']) + self.assertEqual(mock_oauth_client.return_value.get.call_count, 2) + + def test_catalog_content_metadata_raises_for_empty_content_keys_with_traversal(self): + client = EnterpriseCatalogApiClient() + + with self.assertRaisesRegex(Exception, 'Cannot request all metadata for a catalog'): + client.catalog_content_metadata(uuid4(), content_keys=[], traverse_pagination=True) + + def test_content_metadata_not_implemented_for_v2_client(self): + client = EnterpriseCatalogApiClient() + + with self.assertRaises(NotImplementedError): + client.content_metadata('some-content-key') + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') + def test_get_catalogs_three_page_merge(self, mock_oauth_client): + page_1 = {'results': [{'uuid': '1'}], 'next': 'http://p2', 'previous': None, 'count': None} + page_2 = {'results': [{'uuid': '2'}], 'next': 'http://p3', 'previous': 'http://p1', 'count': None} + page_3 = {'results': [{'uuid': '3'}], 'next': None, 'previous': 'http://p2', 'count': None} + mock_oauth_client.return_value.get.side_effect = [ + mock.Mock(json=mock.Mock(return_value=page_1), raise_for_status=mock.Mock()), + mock.Mock(json=mock.Mock(return_value=page_2), raise_for_status=mock.Mock()), + mock.Mock(json=mock.Mock(return_value=page_3), raise_for_status=mock.Mock()), + ] + + client = EnterpriseCatalogApiClient() + fetched = client.get_catalogs() + + self.assertEqual(len(fetched['results']), 3) + self.assertIsNone(fetched['next']) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') + def test_catalog_content_metadata_traverse_false_allows_empty_keys(self, mock_oauth_client): + mock_oauth_client.return_value.get.return_value = mock.Mock( + json=mock.Mock(return_value={}), raise_for_status=mock.Mock() + ) + client = EnterpriseCatalogApiClient() + # Should not raise when traverse_pagination is False and content_keys empty + res = client.catalog_content_metadata('catalog', [], traverse_pagination=False) + self.assertEqual(res, {}) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') + def test_get_academies_wraps_unexpected_type(self, mock_oauth_client): + # Upstream returns an int (unexpected); client should wrap as paginated dict + mock_oauth_client.return_value.get.return_value = mock.Mock( + json=mock.Mock(return_value=123), raise_for_status=mock.Mock() + ) + client = EnterpriseCatalogApiClient() + res = client.get_academies() + self.assertIsInstance(res, dict) + self.assertEqual(res['count'], 1) + self.assertEqual(res['results'], [123]) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) + def test_associate_academy_with_catalog_with_string_uuids(self, mock_oauth_client): + # Ensure we post the expected JSON body and handle JSON response + mock_post = mock_oauth_client.return_value.post + mock_post.return_value.json.return_value = {'detail': 'ok'} + + client = EnterpriseCatalogApiClient() + result = client.associate_academy_with_catalog('academy-1', 'catalog-1') + + self.assertEqual(result, {'detail': 'ok'}) + mock_post.assert_called_once_with( + 'http://enterprise-catalog.example.com/api/v2/academies/academy-1/associate-catalog/', + json={'enterprise_catalog_uuid': 'catalog-1'}, + ) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') + def test_contains_content_items_true_value(self, mock_oauth_client): + mock_oauth_client.return_value.get.return_value = mock.Mock( + json=mock.Mock(return_value={'contains_content_items': True}), raise_for_status=mock.Mock() + ) + client = EnterpriseCatalogApiClient() + self.assertTrue(client.contains_content_items('catalog', ['x'])) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') + def test_get_content_metadata_count_raises_when_key_missing(self, mock_oauth_client): + mock_oauth_client.return_value.get.return_value = mock.Mock( + json=mock.Mock(return_value={}), raise_for_status=mock.Mock() + ) + client = EnterpriseCatalogApiClient() + with self.assertRaises(KeyError): + client.get_content_metadata_count('catalog') + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') + def test_get_catalogs_preserve_results_when_next_page_empty(self, mock_oauth_client): + page_1 = {'results': [{'uuid': '1'}], 'next': 'http://p2', 'previous': None, 'count': 2} + page_2 = {'results': [], 'next': None, 'previous': 'http://p1', 'count': 2} + mock_oauth_client.return_value.get.side_effect = [ + mock.Mock(json=mock.Mock(return_value=page_1), raise_for_status=mock.Mock()), + mock.Mock(json=mock.Mock(return_value=page_2), raise_for_status=mock.Mock()), + ] + + client = EnterpriseCatalogApiClient() + fetched = client.get_catalogs() + self.assertEqual(fetched['count'], 2) + self.assertEqual(len(fetched['results']), 1) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') + def test_get_academies_handles_malformed_next_page(self, mock_oauth_client): + # Next page returns a dict with non-list results; extension should be safe + page_1 = {'results': [{'id': 1}], 'next': 'http://p2', 'previous': None, 'count': None} + page_2 = {'results': {'bad': 'type'}, 'next': None, 'previous': 'http://p1', 'count': None} + mock_oauth_client.return_value.get.side_effect = [ + mock.Mock(json=mock.Mock(return_value=page_1), raise_for_status=mock.Mock()), + mock.Mock(json=mock.Mock(return_value=page_2), raise_for_status=mock.Mock()), + ] + + client = EnterpriseCatalogApiClient() + res = client.get_academies() + # page_2 results are ignored (not list), so merged length remains 1 + self.assertEqual(len(res['results']), 1) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') + def test_catalog_content_metadata_returns_json_payload(self, mock_oauth_client): + payload = {'next': None, 'results': [{'key': 'k'}], 'count': 1} + mock_oauth_client.return_value.get.return_value = mock.Mock( + json=mock.Mock(return_value=payload), raise_for_status=mock.Mock() + ) + client = EnterpriseCatalogApiClient() + self.assertEqual(client.catalog_content_metadata('cat', ['k']), payload) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') + def test_get_academies_preserve_raw_list(self, mock_oauth_client): + payload = [{'a': 1}, {'a': 2}] + mock_oauth_client.return_value.get.return_value = mock.Mock( + json=mock.Mock(return_value=payload), + raise_for_status=mock.Mock(), + ) + + client = EnterpriseCatalogApiClient() + result = client.get_academies() + self.assertEqual(result, payload) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) + def test_get_academies_handles_none_payload(self, mock_oauth_client): + """If upstream returns None, client should return an empty paginated dict.""" + mock_oauth_client.return_value.get.return_value = mock.Mock( + json=mock.Mock(return_value=None), + raise_for_status=mock.Mock(), + ) + + client = EnterpriseCatalogApiClient() + result = client.get_academies() + + self.assertEqual(result, {'count': 0, 'results': [], 'next': None, 'previous': None}) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) + def test_get_academies_merges_when_both_results_are_lists(self, mock_oauth_client): + """Ensure results are merged when both pages return list 'results'.""" + page_1 = { + 'count': None, + 'next': 'http://example.com/?page=2', + 'previous': None, + 'results': [{'id': 1}, {'id': 2}], + } + page_2 = { + 'count': None, + 'next': None, + 'previous': 'http://example.com/?page=1', + 'results': [{'id': 3}], + } + mock_oauth_client.return_value.get.side_effect = [ + mock.Mock(json=mock.Mock(return_value=page_1), raise_for_status=mock.Mock()), + mock.Mock(json=mock.Mock(return_value=page_2), raise_for_status=mock.Mock()), + ] + + client = EnterpriseCatalogApiClient() + res = client.get_academies() + + self.assertEqual(len(res['results']), 3) + self.assertIsNone(res['next']) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') + def test_get_academies_preserves_explicit_count_across_pages(self, mock_oauth_client): + # First page reports an explicit larger count; second page has fewer results + page_1 = { + 'results': [{'id': 1}], + 'count': 4, + 'next': 'http://next', + 'previous': None, + } + page_2 = { + 'results': [{'id': 2}, {'id': 3}], + 'count': 2, + 'next': None, + 'previous': None, + } + mock_oauth_client.return_value.get.side_effect = [ + mock.Mock(json=mock.Mock(return_value=page_1), raise_for_status=mock.Mock()), + mock.Mock(json=mock.Mock(return_value=page_2), raise_for_status=mock.Mock()), + ] + + client = EnterpriseCatalogApiClient() + res = client.get_academies() + self.assertEqual(res['count'], 4) + self.assertEqual(len(res['results']), 3) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') + def test_contains_content_items_missing_key_returns_false(self, mock_oauth_client): + mock_oauth_client.return_value.get.return_value = mock.Mock( + json=mock.Mock(return_value={}), + raise_for_status=mock.Mock(), + ) + client = EnterpriseCatalogApiClient() + self.assertFalse(client.contains_content_items('catalog', ['x'])) + @ddt.ddt class TestEnterpriseCatalogApiV1Client(TestCase): @@ -387,6 +806,31 @@ def test_secured_algolia_api_key(self, mock_crum_get_current_request, mock_send) # Assert the response is as expected self.assertEqual(result, expected_result) + @mock.patch('requests.Session.send') + @mock.patch('crum.get_current_request') + def test_secured_algolia_api_key_raises_on_http_error(self, mock_crum_get_current_request, mock_send): + """Ensure HTTP errors from the backend are propagated.""" + expected_url = ( + f'http://enterprise-catalog.example.com/api/v1' + f'/enterprise-customer/{self.mock_enterprise_customer_uuid}/secured-algolia-api-key/' + ) + request = self.factory.get(expected_url) + request.headers = { + "Authorization": 'test-auth', + self.request_id_key: 'test-request-id' + } + request.user = self.user + mock_crum_get_current_request.return_value = request + + mock_response = mock.Mock() + mock_response.status_code = 400 + mock_response.raise_for_status.side_effect = HTTPError('bad') + mock_send.return_value = mock_response + + client = EnterpriseCatalogUserV1ApiClient(request) + with self.assertRaises(HTTPError): + client.get_secured_algolia_api_key(enterprise_customer_uuid=self.mock_enterprise_customer_uuid) + class TestEnterpriseCatalogApiClientGetAcademy(TestCase): """Tests for EnterpriseCatalogApiClient.get_academy().""" @@ -402,7 +846,7 @@ def test_get_academy_success(self, mock_oauth_client): self.assertEqual(result, expected) mock_oauth_client.return_value.get.assert_called_with( - f'http://enterprise-catalog.example.com/api/v1/academies/{academy_uuid}/', + f'http://enterprise-catalog.example.com/api/v2/academies/{academy_uuid}/', ) @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') @@ -415,3 +859,4 @@ def test_get_academy_raises_on_error(self, mock_oauth_client): client = EnterpriseCatalogApiClient() with self.assertRaises(Exception): client.get_academy(uuid4()) + From 07740efd30b445552f6110910e149b99a2d95aa6 Mon Sep 17 00:00:00 2001 From: Ismail Veruru Shaik Date: Mon, 22 Jun 2026 18:46:35 +0000 Subject: [PATCH 03/11] feat: added test case for coverage --- .../tests/test_enterprise_catalog_client.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py b/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py index 43d94a3b..e5befa06 100644 --- a/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py +++ b/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py @@ -354,6 +354,23 @@ def test_get_academies_returns_non_dict_payload(self, mock_oauth_client): self.assertEqual(result, payload) + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) + def test_get_academies_returns_raw_list_response(self, mock_oauth_client): + payload = [ + {'uuid': 'academy-1'}, + {'uuid': 'academy-2'}, + ] + + mock_oauth_client.return_value.get.return_value = mock.Mock( + json=mock.Mock(return_value=payload), + raise_for_status=mock.Mock(), + ) + + client = EnterpriseCatalogApiClient() + result = client.get_academies() + + self.assertEqual(result, payload) + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) def test_get_academies_with_empty_endpoint_returns_empty_payload(self, mock_oauth_client): client = EnterpriseCatalogApiClient() @@ -859,4 +876,3 @@ def test_get_academy_raises_on_error(self, mock_oauth_client): client = EnterpriseCatalogApiClient() with self.assertRaises(Exception): client.get_academy(uuid4()) - From 4379308dc049ed81ec3042f9b2020323dac7cad5 Mon Sep 17 00:00:00 2001 From: Ismail Veruru Shaik Date: Tue, 23 Jun 2026 06:21:13 +0000 Subject: [PATCH 04/11] feat: updated version in tests --- .../api_client/enterprise_catalog_client.py | 6 +- .../tests/test_enterprise_catalog_client.py | 80 +++++++++++++------ 2 files changed, 59 insertions(+), 27 deletions(-) diff --git a/enterprise_access/apps/api_client/enterprise_catalog_client.py b/enterprise_access/apps/api_client/enterprise_catalog_client.py index 225cc775..9188c8dd 100644 --- a/enterprise_access/apps/api_client/enterprise_catalog_client.py +++ b/enterprise_access/apps/api_client/enterprise_catalog_client.py @@ -13,9 +13,9 @@ class EnterpriseCatalogApiClient(BaseOAuthClient): """ - V2 API client for calls to the enterprise catalog service. + v1 API client for calls to the enterprise catalog service. """ - api_version = 'v2' + api_version = 'v1' def __init__(self): self.api_base_url = urljoin(settings.ENTERPRISE_CATALOG_URL, f'api/{self.api_version}/') @@ -297,7 +297,7 @@ def normalize_page(payload): return data def content_metadata(self, content_id): - raise NotImplementedError('There is currently no v2 API implementation for this endpoint.') + raise NotImplementedError('There is currently no v1 API implementation for this endpoint.') class EnterpriseCatalogApiV1Client(EnterpriseCatalogApiClient): diff --git a/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py b/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py index e5befa06..2e750c25 100644 --- a/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py +++ b/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py @@ -45,7 +45,7 @@ def test_contains_content_items(self, mock_oauth_client, mock_json): assert contains_content_items mock_oauth_client.return_value.get.assert_called_with( - f'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/{ent_uuid}/contains_content_items/', + f'http://enterprise-catalog.example.com/api/v1/enterprise-catalogs/{ent_uuid}/contains_content_items/', params={'course_run_ids': ['AB+CD101']}, ) @@ -76,7 +76,7 @@ def test_catalog_content_metadata(self, mock_oauth_client): self.assertEqual(fetched_metadata['results'], mock_response_json['results']) mock_oauth_client.return_value.get.assert_called_with( - f'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/{customer_uuid}/get_content_metadata/', + f'http://enterprise-catalog.example.com/api/v1/enterprise-catalogs/{customer_uuid}/get_content_metadata/', params={ 'content_keys': content_keys, 'traverse_pagination': True, @@ -98,7 +98,7 @@ def test_catalog_content_metadata_raises_http_error(self, mock_oauth_client): client.catalog_content_metadata(customer_uuid, content_keys) mock_oauth_client.return_value.get.assert_called_with( - f'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/{customer_uuid}/get_content_metadata/', + f'http://enterprise-catalog.example.com/api/v1/enterprise-catalogs/{customer_uuid}/get_content_metadata/', params={ 'content_keys': content_keys, 'traverse_pagination': True, @@ -120,7 +120,7 @@ def test_get_content_metadata_count(self, mock_oauth_client): self.assertEqual(fetched_metadata, mock_response_json['count']) mock_oauth_client.return_value.get.assert_called_with( - f'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/{catalog_uuid}/get_content_metadata/', + f'http://enterprise-catalog.example.com/api/v1/enterprise-catalogs/{catalog_uuid}/get_content_metadata/', ) @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') @@ -133,7 +133,7 @@ def test_get_academies(self, mock_oauth_client): self.assertEqual(fetched, mock_response_json) mock_oauth_client.return_value.get.assert_called_with( - 'http://enterprise-catalog.example.com/api/v2/academies/', + 'http://enterprise-catalog.example.com/api/v1/academies/', params=None, ) @@ -148,13 +148,13 @@ def test_get_academies_with_uuid(self, mock_oauth_client): self.assertEqual(fetched, mock_response_json) mock_oauth_client.return_value.get.assert_called_with( - 'http://enterprise-catalog.example.com/api/v2/academies/', + 'http://enterprise-catalog.example.com/api/v1/academies/', params={'academy_uuid': str(academy_uuid)}, ) @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') - def test_get_academy_fetches_single_record_from_v2(self, mock_oauth_client): - """Ensure `get_academy` calls the v2 academies endpoint and returns the academy JSON.""" + def test_get_academy_fetches_single_record_from_v1(self, mock_oauth_client): + """Ensure `get_academy` calls the v1 academies endpoint and returns the academy JSON.""" academy_uuid = uuid4() mock_resp = mock.Mock() mock_resp.status_code = 200 @@ -167,7 +167,7 @@ def test_get_academy_fetches_single_record_from_v2(self, mock_oauth_client): self.assertEqual(result, {'uuid': str(academy_uuid), 'title': 'Test Academy'}) mock_oauth_client.return_value.get.assert_called_with( - f'http://enterprise-catalog.example.com/api/v2/academies/{academy_uuid}/' + f'http://enterprise-catalog.example.com/api/v1/academies/{academy_uuid}/' ) @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') @@ -180,7 +180,7 @@ def test_get_catalogs(self, mock_oauth_client): self.assertEqual(fetched, mock_response_json) mock_oauth_client.return_value.get.assert_called_with( - 'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/', + 'http://enterprise-catalog.example.com/api/v1/enterprise-catalogs/', params=None, ) @@ -195,7 +195,7 @@ def test_get_catalogs_with_enterprise_customer(self, mock_oauth_client): self.assertEqual(fetched, mock_response_json) mock_oauth_client.return_value.get.assert_called_with( - 'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/', + 'http://enterprise-catalog.example.com/api/v1/enterprise-catalogs/', params={'enterprise_customer': customer_uuid}, ) @@ -263,14 +263,14 @@ def test_get_catalogs_wraps_unexpected_payload_type(self, mock_oauth_client): def test_get_academies_merges_paginated_results(self, mock_oauth_client): page_1 = { 'count': 2, - 'next': 'http://enterprise-catalog.example.com/api/v2/academies/?page=2', + 'next': 'http://enterprise-catalog.example.com/api/v1/academies/?page=2', 'previous': None, 'results': [{'title': 'AI Academy'}], } page_2 = { 'count': 2, 'next': None, - 'previous': 'http://enterprise-catalog.example.com/api/v2/academies/?page=1', + 'previous': 'http://enterprise-catalog.example.com/api/v1/academies/?page=1', 'results': [{'title': 'Data Academy'}], } mock_oauth_client.return_value.get.side_effect = [ @@ -290,14 +290,14 @@ def test_get_academies_merges_paginated_results(self, mock_oauth_client): def test_get_catalogs_merges_paginated_results(self, mock_oauth_client): page_1 = { 'count': 2, - 'next': 'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/?page=2', + 'next': 'http://enterprise-catalog.example.com/api/v1/enterprise-catalogs/?page=2', 'previous': None, 'results': [{'uuid': str(uuid4())}], } page_2 = { 'count': 2, 'next': None, - 'previous': 'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/?page=1', + 'previous': 'http://enterprise-catalog.example.com/api/v1/enterprise-catalogs/?page=1', 'results': [{'uuid': str(uuid4())}], } mock_oauth_client.return_value.get.side_effect = [ @@ -325,7 +325,7 @@ def test_associate_academy_with_catalog(self, mock_oauth_client): self.assertEqual(result, {'detail': 'ok'}) mock_post.assert_called_once_with( - f'http://enterprise-catalog.example.com/api/v2/academies/{academy_uuid}/associate-catalog/', + f'http://enterprise-catalog.example.com/api/v1/academies/{academy_uuid}/associate-catalog/', json={'enterprise_catalog_uuid': str(catalog_uuid)}, ) @@ -371,6 +371,38 @@ def test_get_academies_returns_raw_list_response(self, mock_oauth_client): self.assertEqual(result, payload) + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) + def test_get_academies_does_not_merge_when_next_results_not_list(self, mock_oauth_client, ): + page_1 = { + 'count': 1, + 'next': 'page-2', + 'previous': None, + 'results': [{'uuid': 'academy-1'}], + } + + page_2 = { + 'count': 1, + 'next': None, + 'previous': 'page-1', + 'results': 'not-a-list', + } + + mock_oauth_client.return_value.get.side_effect = [ + mock.Mock( + json=mock.Mock(return_value=page_1), + raise_for_status=mock.Mock(), + ), + mock.Mock( + json=mock.Mock(return_value=page_2), + raise_for_status=mock.Mock(), + ), + ] + + client = EnterpriseCatalogApiClient() + result = client.get_academies() + + self.assertEqual(result['results'], [{'uuid': 'academy-1'}]) + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) def test_get_academies_with_empty_endpoint_returns_empty_payload(self, mock_oauth_client): client = EnterpriseCatalogApiClient() @@ -406,7 +438,7 @@ def test_get_academies_with_is_active_param(self, mock_oauth_client): self.assertEqual(result['results'], []) mock_oauth_client.return_value.get.assert_called_with( - 'http://enterprise-catalog.example.com/api/v2/academies/', + 'http://enterprise-catalog.example.com/api/v1/academies/', params={'is_active': True}, ) @@ -414,14 +446,14 @@ def test_get_academies_with_is_active_param(self, mock_oauth_client): def test_get_academies_merges_paginated_results_missing_count(self, mock_oauth_client): page_1 = { 'count': None, - 'next': 'http://enterprise-catalog.example.com/api/v2/academies/?page=2', + 'next': 'http://enterprise-catalog.example.com/api/v1/academies/?page=2', 'previous': None, 'results': [{'title': 'AI Academy'}], } page_2 = { 'count': None, 'next': None, - 'previous': 'http://enterprise-catalog.example.com/api/v2/academies/?page=1', + 'previous': 'http://enterprise-catalog.example.com/api/v1/academies/?page=1', 'results': [{'title': 'Data Academy'}], } mock_oauth_client.return_value.get.side_effect = [ @@ -441,14 +473,14 @@ def test_get_academies_merges_paginated_results_missing_count(self, mock_oauth_c def test_get_academies_merges_paginated_results_non_int_count(self, mock_oauth_client): page_1 = { 'count': 'not-an-int', - 'next': 'http://enterprise-catalog.example.com/api/v2/academies/?page=2', + 'next': 'http://enterprise-catalog.example.com/api/v1/academies/?page=2', 'previous': None, 'results': [{'title': 'AI Academy'}], } page_2 = { 'count': 'also-not-int', 'next': None, - 'previous': 'http://enterprise-catalog.example.com/api/v2/academies/?page=1', + 'previous': 'http://enterprise-catalog.example.com/api/v1/academies/?page=1', 'results': [{'title': 'Data Academy'}], } mock_oauth_client.return_value.get.side_effect = [ @@ -471,7 +503,7 @@ def test_catalog_content_metadata_raises_for_empty_content_keys_with_traversal(s with self.assertRaisesRegex(Exception, 'Cannot request all metadata for a catalog'): client.catalog_content_metadata(uuid4(), content_keys=[], traverse_pagination=True) - def test_content_metadata_not_implemented_for_v2_client(self): + def test_content_metadata_not_implemented_for_v1_client(self): client = EnterpriseCatalogApiClient() with self.assertRaises(NotImplementedError): @@ -527,7 +559,7 @@ def test_associate_academy_with_catalog_with_string_uuids(self, mock_oauth_clien self.assertEqual(result, {'detail': 'ok'}) mock_post.assert_called_once_with( - 'http://enterprise-catalog.example.com/api/v2/academies/academy-1/associate-catalog/', + 'http://enterprise-catalog.example.com/api/v1/academies/academy-1/associate-catalog/', json={'enterprise_catalog_uuid': 'catalog-1'}, ) @@ -863,7 +895,7 @@ def test_get_academy_success(self, mock_oauth_client): self.assertEqual(result, expected) mock_oauth_client.return_value.get.assert_called_with( - f'http://enterprise-catalog.example.com/api/v2/academies/{academy_uuid}/', + f'http://enterprise-catalog.example.com/api/v1/academies/{academy_uuid}/', ) @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') From 194d1dabe7c5efd6d3c6d42c0f15002f9d6bd845 Mon Sep 17 00:00:00 2001 From: Ismail Veruru Shaik Date: Tue, 23 Jun 2026 10:45:34 +0000 Subject: [PATCH 05/11] feat: added patch test --- .../api_client/enterprise_catalog_client.py | 9 ++--- .../tests/test_enterprise_catalog_client.py | 33 ++++++++++++++----- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/enterprise_access/apps/api_client/enterprise_catalog_client.py b/enterprise_access/apps/api_client/enterprise_catalog_client.py index 9188c8dd..fc18e6cc 100644 --- a/enterprise_access/apps/api_client/enterprise_catalog_client.py +++ b/enterprise_access/apps/api_client/enterprise_catalog_client.py @@ -13,13 +13,14 @@ class EnterpriseCatalogApiClient(BaseOAuthClient): """ - v1 API client for calls to the enterprise catalog service. + v2 API client for calls to the enterprise catalog service. """ - api_version = 'v1' + api_version = 'v2' def __init__(self): self.api_base_url = urljoin(settings.ENTERPRISE_CATALOG_URL, f'api/{self.api_version}/') - self.academies_endpoint = urljoin(self.api_base_url, 'academies/') + # Academies are exposed on v1 of the enterprise-catalog API, not v2 + self.academies_endpoint = urljoin(settings.ENTERPRISE_CATALOG_URL, 'api/v1/academies/') self.enterprise_catalog_endpoint = urljoin(self.api_base_url, 'enterprise-catalogs/') super().__init__() @@ -297,7 +298,7 @@ def normalize_page(payload): return data def content_metadata(self, content_id): - raise NotImplementedError('There is currently no v1 API implementation for this endpoint.') + raise NotImplementedError('There is currently no v2 API implementation for this endpoint.') class EnterpriseCatalogApiV1Client(EnterpriseCatalogApiClient): diff --git a/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py b/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py index 2e750c25..64838db4 100644 --- a/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py +++ b/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py @@ -45,7 +45,7 @@ def test_contains_content_items(self, mock_oauth_client, mock_json): assert contains_content_items mock_oauth_client.return_value.get.assert_called_with( - f'http://enterprise-catalog.example.com/api/v1/enterprise-catalogs/{ent_uuid}/contains_content_items/', + f'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/{ent_uuid}/contains_content_items/', params={'course_run_ids': ['AB+CD101']}, ) @@ -76,7 +76,7 @@ def test_catalog_content_metadata(self, mock_oauth_client): self.assertEqual(fetched_metadata['results'], mock_response_json['results']) mock_oauth_client.return_value.get.assert_called_with( - f'http://enterprise-catalog.example.com/api/v1/enterprise-catalogs/{customer_uuid}/get_content_metadata/', + f'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/{customer_uuid}/get_content_metadata/', params={ 'content_keys': content_keys, 'traverse_pagination': True, @@ -98,7 +98,7 @@ def test_catalog_content_metadata_raises_http_error(self, mock_oauth_client): client.catalog_content_metadata(customer_uuid, content_keys) mock_oauth_client.return_value.get.assert_called_with( - f'http://enterprise-catalog.example.com/api/v1/enterprise-catalogs/{customer_uuid}/get_content_metadata/', + f'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/{customer_uuid}/get_content_metadata/', params={ 'content_keys': content_keys, 'traverse_pagination': True, @@ -120,7 +120,7 @@ def test_get_content_metadata_count(self, mock_oauth_client): self.assertEqual(fetched_metadata, mock_response_json['count']) mock_oauth_client.return_value.get.assert_called_with( - f'http://enterprise-catalog.example.com/api/v1/enterprise-catalogs/{catalog_uuid}/get_content_metadata/', + f'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/{catalog_uuid}/get_content_metadata/', ) @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') @@ -180,7 +180,7 @@ def test_get_catalogs(self, mock_oauth_client): self.assertEqual(fetched, mock_response_json) mock_oauth_client.return_value.get.assert_called_with( - 'http://enterprise-catalog.example.com/api/v1/enterprise-catalogs/', + 'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/', params=None, ) @@ -195,7 +195,7 @@ def test_get_catalogs_with_enterprise_customer(self, mock_oauth_client): self.assertEqual(fetched, mock_response_json) mock_oauth_client.return_value.get.assert_called_with( - 'http://enterprise-catalog.example.com/api/v1/enterprise-catalogs/', + 'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/', params={'enterprise_customer': customer_uuid}, ) @@ -290,14 +290,14 @@ def test_get_academies_merges_paginated_results(self, mock_oauth_client): def test_get_catalogs_merges_paginated_results(self, mock_oauth_client): page_1 = { 'count': 2, - 'next': 'http://enterprise-catalog.example.com/api/v1/enterprise-catalogs/?page=2', + 'next': 'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/?page=2', 'previous': None, 'results': [{'uuid': str(uuid4())}], } page_2 = { 'count': 2, 'next': None, - 'previous': 'http://enterprise-catalog.example.com/api/v1/enterprise-catalogs/?page=1', + 'previous': 'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/?page=1', 'results': [{'uuid': str(uuid4())}], } mock_oauth_client.return_value.get.side_effect = [ @@ -371,6 +371,23 @@ def test_get_academies_returns_raw_list_response(self, mock_oauth_client): self.assertEqual(result, payload) + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) + def test_get_catalogs_returns_raw_list_response(self, mock_oauth_client): + payload = [ + {'uuid': 'catalog-1'}, + {'uuid': 'catalog-2'}, + ] + + mock_oauth_client.return_value.get.return_value = mock.Mock( + json=mock.Mock(return_value=payload), + raise_for_status=mock.Mock(), + ) + + client = EnterpriseCatalogApiClient() + result = client.get_catalogs() + + self.assertEqual(result, payload) + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) def test_get_academies_does_not_merge_when_next_results_not_list(self, mock_oauth_client, ): page_1 = { From 43dd14e2c46f34a1f4ab66f51e29612a1d3e5d47 Mon Sep 17 00:00:00 2001 From: Ismail Veruru Shaik Date: Wed, 24 Jun 2026 10:57:41 +0000 Subject: [PATCH 06/11] feat: addressed commets --- .../api_client/enterprise_catalog_client.py | 63 +---- .../tests/test_enterprise_catalog_client.py | 244 +++++------------- 2 files changed, 67 insertions(+), 240 deletions(-) diff --git a/enterprise_access/apps/api_client/enterprise_catalog_client.py b/enterprise_access/apps/api_client/enterprise_catalog_client.py index fc18e6cc..95696fc6 100644 --- a/enterprise_access/apps/api_client/enterprise_catalog_client.py +++ b/enterprise_access/apps/api_client/enterprise_catalog_client.py @@ -62,73 +62,20 @@ def get_academies(self, academy_uuid: str = None, is_active: bool | None = None) params = None response = self.client.get(self.academies_endpoint, params=params) response.raise_for_status() - raw_payload = response.json() - - def normalize_page(payload): - """Normalize a response payload into a paginated dict. - - Behavior is intentionally conservative to match historical expectations: - - If the service returned a non-dict (e.g. a list), return it unchanged. - - If the service returned a dict without a list-valued `results`, treat - `results` as empty (do not coerce non-list into a single-item list). - - Preserve the incoming `count` value when present; only fall back to - computed lengths when absent. - """ - if payload is None: - return ({'count': 0, 'results': [], 'next': None, 'previous': None}, False) - # If upstream returned a raw list, preserve that shape - if isinstance(payload, list): - return payload, False - if isinstance(payload, dict): - results = payload.get('results') - # If results is missing or not a list, treat as empty list - if not isinstance(results, list): - results = [] - raw_count = payload.get('count', None) - # Try to coerce incoming count to an int; if that fails, treat - # it as absent so we fall back to the computed length. - try: - count = int(raw_count) if raw_count is not None else None - except (ValueError, TypeError): - count = None - explicit_count = count is not None - if count is None: - count = len(results) - return ({ - 'count': count, - 'results': results, - 'next': payload.get('next'), - 'previous': payload.get('previous'), - }, explicit_count) - # Fallback: for unexpected types, wrap as single-item paginated dict - return ({'count': 1, 'results': [payload], 'next': None, 'previous': None}, False) - - # If upstream returned a raw list, preserve that shape - if isinstance(raw_payload, list): - return raw_payload - - data, explicit = normalize_page(raw_payload) - # Merge paginated results if necessary; be defensive about types + data = response.json() next_url = data.get('next') + while next_url: next_resp = self.client.get(next_url) next_resp.raise_for_status() - next_payload = next_resp.json() - next_data, next_explicit = normalize_page(next_payload) - # Only extend if both are lists - if isinstance(data.get('results'), list) and isinstance(next_data.get('results'), list): - data['results'].extend(next_data.get('results', [])) + + next_data = next_resp.json() + data['results'].extend(next_data.get('results', [])) # update pagination markers data['next'] = next_data.get('next') data['previous'] = data.get('previous') or next_data.get('previous') - # If the upstream provided an explicit numeric count, preserve it. - # Otherwise recompute the total as the length of the merged results. - # If either page did not provide an explicit numeric count, recompute - explicit = explicit and next_explicit - if not explicit: - data['count'] = len(data.get('results', [])) next_url = data.get('next') return data diff --git a/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py b/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py index 64838db4..0b737618 100644 --- a/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py +++ b/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py @@ -341,36 +341,6 @@ def test_associate_academy_with_catalog_empty_response_body(self, mock_oauth_cli self.assertEqual(result, {}) - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) - def test_get_academies_returns_non_dict_payload(self, mock_oauth_client): - payload = ['not-a-dict'] - mock_oauth_client.return_value.get.return_value = mock.Mock( - json=mock.Mock(return_value=payload), - raise_for_status=mock.Mock(), - ) - - client = EnterpriseCatalogApiClient() - result = client.get_academies() - - self.assertEqual(result, payload) - - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) - def test_get_academies_returns_raw_list_response(self, mock_oauth_client): - payload = [ - {'uuid': 'academy-1'}, - {'uuid': 'academy-2'}, - ] - - mock_oauth_client.return_value.get.return_value = mock.Mock( - json=mock.Mock(return_value=payload), - raise_for_status=mock.Mock(), - ) - - client = EnterpriseCatalogApiClient() - result = client.get_academies() - - self.assertEqual(result, payload) - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) def test_get_catalogs_returns_raw_list_response(self, mock_oauth_client): payload = [ @@ -388,38 +358,6 @@ def test_get_catalogs_returns_raw_list_response(self, mock_oauth_client): self.assertEqual(result, payload) - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) - def test_get_academies_does_not_merge_when_next_results_not_list(self, mock_oauth_client, ): - page_1 = { - 'count': 1, - 'next': 'page-2', - 'previous': None, - 'results': [{'uuid': 'academy-1'}], - } - - page_2 = { - 'count': 1, - 'next': None, - 'previous': 'page-1', - 'results': 'not-a-list', - } - - mock_oauth_client.return_value.get.side_effect = [ - mock.Mock( - json=mock.Mock(return_value=page_1), - raise_for_status=mock.Mock(), - ), - mock.Mock( - json=mock.Mock(return_value=page_2), - raise_for_status=mock.Mock(), - ), - ] - - client = EnterpriseCatalogApiClient() - result = client.get_academies() - - self.assertEqual(result['results'], [{'uuid': 'academy-1'}]) - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) def test_get_academies_with_empty_endpoint_returns_empty_payload(self, mock_oauth_client): client = EnterpriseCatalogApiClient() @@ -430,19 +368,6 @@ def test_get_academies_with_empty_endpoint_returns_empty_payload(self, mock_oaut self.assertEqual(result, {'count': 0, 'next': None, 'previous': None, 'results': []}) mock_oauth_client.return_value.get.assert_not_called() - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) - def test_get_academies_ignores_non_list_results(self, mock_oauth_client): - payload = {'count': 1, 'next': None, 'previous': None, 'results': {'title': 'not-a-list'}} - mock_oauth_client.return_value.get.return_value = mock.Mock( - json=mock.Mock(return_value=payload), - raise_for_status=mock.Mock(), - ) - - client = EnterpriseCatalogApiClient() - result = client.get_academies() - - self.assertEqual(result['results'], []) - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) def test_get_academies_with_is_active_param(self, mock_oauth_client): mock_oauth_client.return_value.get.return_value = mock.Mock( @@ -459,61 +384,6 @@ def test_get_academies_with_is_active_param(self, mock_oauth_client): params={'is_active': True}, ) - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) - def test_get_academies_merges_paginated_results_missing_count(self, mock_oauth_client): - page_1 = { - 'count': None, - 'next': 'http://enterprise-catalog.example.com/api/v1/academies/?page=2', - 'previous': None, - 'results': [{'title': 'AI Academy'}], - } - page_2 = { - 'count': None, - 'next': None, - 'previous': 'http://enterprise-catalog.example.com/api/v1/academies/?page=1', - 'results': [{'title': 'Data Academy'}], - } - mock_oauth_client.return_value.get.side_effect = [ - mock.Mock(json=mock.Mock(return_value=page_1), raise_for_status=mock.Mock()), - mock.Mock(json=mock.Mock(return_value=page_2), raise_for_status=mock.Mock()), - ] - - client = EnterpriseCatalogApiClient() - fetched = client.get_academies() - - self.assertEqual(fetched['count'], 2) - self.assertEqual(len(fetched['results']), 2) - self.assertIsNone(fetched['next']) - self.assertEqual(mock_oauth_client.return_value.get.call_count, 2) - - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) - def test_get_academies_merges_paginated_results_non_int_count(self, mock_oauth_client): - page_1 = { - 'count': 'not-an-int', - 'next': 'http://enterprise-catalog.example.com/api/v1/academies/?page=2', - 'previous': None, - 'results': [{'title': 'AI Academy'}], - } - page_2 = { - 'count': 'also-not-int', - 'next': None, - 'previous': 'http://enterprise-catalog.example.com/api/v1/academies/?page=1', - 'results': [{'title': 'Data Academy'}], - } - mock_oauth_client.return_value.get.side_effect = [ - mock.Mock(json=mock.Mock(return_value=page_1), raise_for_status=mock.Mock()), - mock.Mock(json=mock.Mock(return_value=page_2), raise_for_status=mock.Mock()), - ] - - client = EnterpriseCatalogApiClient() - fetched = client.get_academies() - - # non-int counts should be handled gracefully and fallback to length - self.assertEqual(fetched['count'], 2) - self.assertEqual(len(fetched['results']), 2) - self.assertIsNone(fetched['next']) - self.assertEqual(mock_oauth_client.return_value.get.call_count, 2) - def test_catalog_content_metadata_raises_for_empty_content_keys_with_traversal(self): client = EnterpriseCatalogApiClient() @@ -553,18 +423,6 @@ def test_catalog_content_metadata_traverse_false_allows_empty_keys(self, mock_oa res = client.catalog_content_metadata('catalog', [], traverse_pagination=False) self.assertEqual(res, {}) - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') - def test_get_academies_wraps_unexpected_type(self, mock_oauth_client): - # Upstream returns an int (unexpected); client should wrap as paginated dict - mock_oauth_client.return_value.get.return_value = mock.Mock( - json=mock.Mock(return_value=123), raise_for_status=mock.Mock() - ) - client = EnterpriseCatalogApiClient() - res = client.get_academies() - self.assertIsInstance(res, dict) - self.assertEqual(res['count'], 1) - self.assertEqual(res['results'], [123]) - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) def test_associate_academy_with_catalog_with_string_uuids(self, mock_oauth_client): # Ensure we post the expected JSON body and handle JSON response @@ -611,21 +469,6 @@ def test_get_catalogs_preserve_results_when_next_page_empty(self, mock_oauth_cli self.assertEqual(fetched['count'], 2) self.assertEqual(len(fetched['results']), 1) - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') - def test_get_academies_handles_malformed_next_page(self, mock_oauth_client): - # Next page returns a dict with non-list results; extension should be safe - page_1 = {'results': [{'id': 1}], 'next': 'http://p2', 'previous': None, 'count': None} - page_2 = {'results': {'bad': 'type'}, 'next': None, 'previous': 'http://p1', 'count': None} - mock_oauth_client.return_value.get.side_effect = [ - mock.Mock(json=mock.Mock(return_value=page_1), raise_for_status=mock.Mock()), - mock.Mock(json=mock.Mock(return_value=page_2), raise_for_status=mock.Mock()), - ] - - client = EnterpriseCatalogApiClient() - res = client.get_academies() - # page_2 results are ignored (not list), so merged length remains 1 - self.assertEqual(len(res['results']), 1) - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') def test_catalog_content_metadata_returns_json_payload(self, mock_oauth_client): payload = {'next': None, 'results': [{'key': 'k'}], 'count': 1} @@ -635,31 +478,6 @@ def test_catalog_content_metadata_returns_json_payload(self, mock_oauth_client): client = EnterpriseCatalogApiClient() self.assertEqual(client.catalog_content_metadata('cat', ['k']), payload) - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') - def test_get_academies_preserve_raw_list(self, mock_oauth_client): - payload = [{'a': 1}, {'a': 2}] - mock_oauth_client.return_value.get.return_value = mock.Mock( - json=mock.Mock(return_value=payload), - raise_for_status=mock.Mock(), - ) - - client = EnterpriseCatalogApiClient() - result = client.get_academies() - self.assertEqual(result, payload) - - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) - def test_get_academies_handles_none_payload(self, mock_oauth_client): - """If upstream returns None, client should return an empty paginated dict.""" - mock_oauth_client.return_value.get.return_value = mock.Mock( - json=mock.Mock(return_value=None), - raise_for_status=mock.Mock(), - ) - - client = EnterpriseCatalogApiClient() - result = client.get_academies() - - self.assertEqual(result, {'count': 0, 'results': [], 'next': None, 'previous': None}) - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) def test_get_academies_merges_when_both_results_are_lists(self, mock_oauth_client): """Ensure results are merged when both pages return list 'results'.""" @@ -686,6 +504,68 @@ def test_get_academies_merges_when_both_results_are_lists(self, mock_oauth_clien self.assertEqual(len(res['results']), 3) self.assertIsNone(res['next']) + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) + def test_get_academies_returns_paginated_response(self, mock_oauth_client): + mock_response = { + 'count': 1, + 'next': None, + 'previous': None, + 'results': [{'uuid': 'academy-1'}], + } + + mock_oauth_client.return_value.get.return_value = mock.Mock( + json=mock.Mock(return_value=mock_response), + raise_for_status=mock.Mock(), + ) + + client = EnterpriseCatalogApiClient() + result = client.get_academies() + + self.assertEqual(result, mock_response) + + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) + def test_get_academies_merges_paginated_results_(self, mock_oauth_client): + first_page = mock.Mock( + json=mock.Mock( + return_value={ + 'count': 2, + 'next': 'next-page', + 'previous': None, + 'results': [{'uuid': 'academy-1'}], + } + ), + raise_for_status=mock.Mock(), + ) + + second_page = mock.Mock( + json=mock.Mock( + return_value={ + 'count': 2, + 'next': None, + 'previous': None, + 'results': [{'uuid': 'academy-2'}], + } + ), + raise_for_status=mock.Mock(), + ) + + mock_oauth_client.return_value.get.side_effect = [ + first_page, + second_page, + ] + + client = EnterpriseCatalogApiClient() + result = client.get_academies() + + self.assertEqual(result['count'], 2) + self.assertEqual( + result['results'], + [ + {'uuid': 'academy-1'}, + {'uuid': 'academy-2'}, + ], + ) + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') def test_get_academies_preserves_explicit_count_across_pages(self, mock_oauth_client): # First page reports an explicit larger count; second page has fewer results From 76977f8f20a8faa89e0449b6ee00ad423f638f60 Mon Sep 17 00:00:00 2001 From: Ismail Veruru Shaik Date: Wed, 24 Jun 2026 17:49:04 +0000 Subject: [PATCH 07/11] feat: added new helper --- .../api_client/enterprise_catalog_client.py | 14 ++++----- enterprise_access/apps/api_client/utils.py | 31 +++++++++++++++++++ 2 files changed, 38 insertions(+), 7 deletions(-) create mode 100644 enterprise_access/apps/api_client/utils.py diff --git a/enterprise_access/apps/api_client/enterprise_catalog_client.py b/enterprise_access/apps/api_client/enterprise_catalog_client.py index 95696fc6..e5d68b00 100644 --- a/enterprise_access/apps/api_client/enterprise_catalog_client.py +++ b/enterprise_access/apps/api_client/enterprise_catalog_client.py @@ -9,6 +9,7 @@ from enterprise_access.apps.api_client.base_oauth import BaseOAuthClient from enterprise_access.apps.api_client.base_user import BaseUserApiClient from enterprise_access.apps.api_client.constants import autoretry_for_exceptions +from enterprise_access.apps.api_client.utils import get_paginated_payloads class EnterpriseCatalogApiClient(BaseOAuthClient): @@ -226,21 +227,20 @@ def normalize_page(payload): data, explicit = normalize_page(raw_payload) - # Merge paginated results if necessary; be defensive about types - next_url = data.get('next') - while next_url: - next_resp = self.client.get(next_url) - next_resp.raise_for_status() - next_payload = next_resp.json() + # Fetch and Merge paginated results using the shared pagination helper + for next_payload in get_paginated_payloads(self.client, data.get("next")): next_data, next_explicit = normalize_page(next_payload) + if isinstance(data.get('results'), list) and isinstance(next_data.get('results'), list): data['results'].extend(next_data.get('results', [])) + data['next'] = next_data.get('next') data['previous'] = data.get('previous') or next_data.get('previous') + explicit = explicit and next_explicit + if not explicit: data['count'] = len(data.get('results', [])) - next_url = data.get('next') return data diff --git a/enterprise_access/apps/api_client/utils.py b/enterprise_access/apps/api_client/utils.py new file mode 100644 index 00000000..d4438ee7 --- /dev/null +++ b/enterprise_access/apps/api_client/utils.py @@ -0,0 +1,31 @@ +""" +Utility helpers for the API client implementation. +""" + + +def get_paginated_payloads(client, next_url): + """ + Iterate through paginated API responses. + + Follows `next` links until all pages have been retrieved and yields + the JSON payload from each page response. + + Args: + client: HTTP client instance. + next_url (str): Initial pagination URL. + + Yields: + dict: Response payload for each page. + """ + payloads = [] + + while next_url: + response = client.get(next_url) + response.raise_for_status() + + payload = response.json() + payloads.append(payload) + + next_url = payload.get("next") + + return payloads From 15c0978af745a04efe421df254ab273ac064361b Mon Sep 17 00:00:00 2001 From: Ismail Veruru Shaik Date: Wed, 24 Jun 2026 20:15:09 +0000 Subject: [PATCH 08/11] feat: more review comments addressed --- .../api_client/enterprise_catalog_client.py | 80 ++++--------------- .../tests/test_enterprise_catalog_client.py | 79 +----------------- 2 files changed, 17 insertions(+), 142 deletions(-) diff --git a/enterprise_access/apps/api_client/enterprise_catalog_client.py b/enterprise_access/apps/api_client/enterprise_catalog_client.py index e5d68b00..0da6cd9c 100644 --- a/enterprise_access/apps/api_client/enterprise_catalog_client.py +++ b/enterprise_access/apps/api_client/enterprise_catalog_client.py @@ -1,6 +1,7 @@ """ API client for enterprise-catalog service. """ +import logging from urllib.parse import urljoin import backoff @@ -11,6 +12,8 @@ from enterprise_access.apps.api_client.constants import autoretry_for_exceptions from enterprise_access.apps.api_client.utils import get_paginated_payloads +logger = logging.getLogger(__name__) + class EnterpriseCatalogApiClient(BaseOAuthClient): """ @@ -59,8 +62,6 @@ def get_academies(self, academy_uuid: str = None, is_active: bool | None = None) params['academy_uuid'] = academy_uuid if is_active is not None: params['is_active'] = bool(is_active) - if not params: - params = None response = self.client.get(self.academies_endpoint, params=params) response.raise_for_status() @@ -68,16 +69,13 @@ def get_academies(self, academy_uuid: str = None, is_active: bool | None = None) next_url = data.get('next') - while next_url: - next_resp = self.client.get(next_url) - next_resp.raise_for_status() + for next_payload in get_paginated_payloads(self.client, next_url, ): + + next_data = next_payload - next_data = next_resp.json() - data['results'].extend(next_data.get('results', [])) - # update pagination markers - data['next'] = next_data.get('next') - data['previous'] = data.get('previous') or next_data.get('previous') - next_url = data.get('next') + data["results"].extend(next_data.get("results", [])) + data["next"] = next_data.get("next") + data["previous"] = data.get("previous") or next_data.get("previous") return data @backoff.on_exception(wait_gen=backoff.expo, exception=autoretry_for_exceptions) @@ -101,6 +99,9 @@ def associate_academy_with_catalog(self, academy_uuid, enterprise_catalog_uuid): try: return response.json() except ValueError: + logger.warning( + "Failed to parse JSON response from Enterprise Catalog API." + ) return {} @backoff.on_exception(wait_gen=backoff.expo, exception=autoretry_for_exceptions) @@ -185,62 +186,13 @@ def get_catalogs(self, enterprise_customer_uuid: str = None) -> dict | list: params = {'enterprise_customer': enterprise_customer_uuid} if enterprise_customer_uuid else None response = self.client.get(self.enterprise_catalog_endpoint, params=params) response.raise_for_status() - raw_payload = response.json() - - def normalize_page(payload): - """Normalize a response payload into a paginated dict. - - This mirrors `get_academies()` defensive behavior so callers can - safely handle dict or list payloads from upstream services. - Returns a tuple of (paginated_dict, explicit_count_flag) when - payload is dict-like, or (list_payload, False) when payload is - a raw list. When payload is None, return an empty paginated shape - and False for explicit flag. - """ - if payload is None: - return ({'count': 0, 'results': [], 'next': None, 'previous': None}, False) - if isinstance(payload, list): - return payload, False - if isinstance(payload, dict): - results = payload.get('results') - if not isinstance(results, list): - results = [] - raw_count = payload.get('count', None) - try: - count = int(raw_count) if raw_count is not None else None - except (ValueError, TypeError): - count = None - explicit_count = count is not None - if count is None: - count = len(results) - return ({ - 'count': count, - 'results': results, - 'next': payload.get('next'), - 'previous': payload.get('previous'), - }, explicit_count) - return ({'count': 1, 'results': [payload], 'next': None, 'previous': None}, False) - - # Preserve raw list payloads - if isinstance(raw_payload, list): - return raw_payload - - data, explicit = normalize_page(raw_payload) + data = response.json() # Fetch and Merge paginated results using the shared pagination helper for next_payload in get_paginated_payloads(self.client, data.get("next")): - next_data, next_explicit = normalize_page(next_payload) - - if isinstance(data.get('results'), list) and isinstance(next_data.get('results'), list): - data['results'].extend(next_data.get('results', [])) - - data['next'] = next_data.get('next') - data['previous'] = data.get('previous') or next_data.get('previous') - - explicit = explicit and next_explicit - - if not explicit: - data['count'] = len(data.get('results', [])) + data['results'].extend(next_payload.get('results', [])) + data['next'] = next_payload.get('next') + data['previous'] = data.get('previous') or next_payload.get('previous') return data diff --git a/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py b/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py index 0b737618..5e3fe2fc 100644 --- a/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py +++ b/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py @@ -134,7 +134,7 @@ def test_get_academies(self, mock_oauth_client): self.assertEqual(fetched, mock_response_json) mock_oauth_client.return_value.get.assert_called_with( 'http://enterprise-catalog.example.com/api/v1/academies/', - params=None, + params={}, ) @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') @@ -199,66 +199,6 @@ def test_get_catalogs_with_enterprise_customer(self, mock_oauth_client): params={'enterprise_customer': customer_uuid}, ) - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) - def test_get_catalogs_handles_none_payload(self, mock_oauth_client): - mock_oauth_client.return_value.get.return_value = mock.Mock( - json=mock.Mock(return_value=None), - raise_for_status=mock.Mock(), - ) - - client = EnterpriseCatalogApiClient() - result = client.get_catalogs() - - self.assertEqual(result, {'count': 0, 'results': [], 'next': None, 'previous': None}) - - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) - def test_get_catalogs_preserves_raw_list_payload(self, mock_oauth_client): - payload = [{'uuid': str(uuid4())}, {'uuid': str(uuid4())}] - mock_oauth_client.return_value.get.return_value = mock.Mock( - json=mock.Mock(return_value=payload), - raise_for_status=mock.Mock(), - ) - - client = EnterpriseCatalogApiClient() - result = client.get_catalogs() - - self.assertEqual(result, payload) - - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) - def test_get_catalogs_ignores_non_list_results_and_invalid_count(self, mock_oauth_client): - payload = { - 'count': 'not-an-int', - 'next': None, - 'previous': None, - 'results': {'uuid': str(uuid4())}, - } - mock_oauth_client.return_value.get.return_value = mock.Mock( - json=mock.Mock(return_value=payload), - raise_for_status=mock.Mock(), - ) - - client = EnterpriseCatalogApiClient() - result = client.get_catalogs() - - self.assertEqual(result, {'count': 0, 'results': [], 'next': None, 'previous': None}) - - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) - def test_get_catalogs_wraps_unexpected_payload_type(self, mock_oauth_client): - mock_oauth_client.return_value.get.return_value = mock.Mock( - json=mock.Mock(return_value='catalog-value'), - raise_for_status=mock.Mock(), - ) - - client = EnterpriseCatalogApiClient() - result = client.get_catalogs() - - self.assertEqual(result, { - 'count': 1, - 'results': ['catalog-value'], - 'next': None, - 'previous': None, - }) - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') def test_get_academies_merges_paginated_results(self, mock_oauth_client): page_1 = { @@ -341,23 +281,6 @@ def test_associate_academy_with_catalog_empty_response_body(self, mock_oauth_cli self.assertEqual(result, {}) - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) - def test_get_catalogs_returns_raw_list_response(self, mock_oauth_client): - payload = [ - {'uuid': 'catalog-1'}, - {'uuid': 'catalog-2'}, - ] - - mock_oauth_client.return_value.get.return_value = mock.Mock( - json=mock.Mock(return_value=payload), - raise_for_status=mock.Mock(), - ) - - client = EnterpriseCatalogApiClient() - result = client.get_catalogs() - - self.assertEqual(result, payload) - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) def test_get_academies_with_empty_endpoint_returns_empty_payload(self, mock_oauth_client): client = EnterpriseCatalogApiClient() From abaeffcf8ac34c612745bcabf5b6280b52c647aa Mon Sep 17 00:00:00 2001 From: Ismail Veruru Shaik Date: Wed, 24 Jun 2026 21:57:05 +0000 Subject: [PATCH 09/11] feat: updated helper and removed redundant function --- .../api_client/enterprise_catalog_client.py | 78 ++++--------------- .../tests/test_enterprise_catalog_client.py | 47 +---------- enterprise_access/apps/api_client/utils.py | 37 +++++---- 3 files changed, 39 insertions(+), 123 deletions(-) diff --git a/enterprise_access/apps/api_client/enterprise_catalog_client.py b/enterprise_access/apps/api_client/enterprise_catalog_client.py index 0da6cd9c..0d2441ef 100644 --- a/enterprise_access/apps/api_client/enterprise_catalog_client.py +++ b/enterprise_access/apps/api_client/enterprise_catalog_client.py @@ -10,7 +10,7 @@ from enterprise_access.apps.api_client.base_oauth import BaseOAuthClient from enterprise_access.apps.api_client.base_user import BaseUserApiClient from enterprise_access.apps.api_client.constants import autoretry_for_exceptions -from enterprise_access.apps.api_client.utils import get_paginated_payloads +from enterprise_access.apps.api_client.utils import fetch_all_results logger = logging.getLogger(__name__) @@ -47,11 +47,12 @@ def get_academy(self, academy_uuid): @backoff.on_exception(wait_gen=backoff.expo, exception=autoretry_for_exceptions) def get_academies(self, academy_uuid: str = None, is_active: bool | None = None) -> dict | list: """ - Fetch a list of academies, optionally filtered by academy_uuid. + Fetch a list of academies. - Returns a paginated-style dict or a raw list depending on the upstream service. - If the response contains a `next` link, subsequent pages will be fetched and merged into a single - results list. + Optionally filters results by academy UUID and active status. + + Returns: + dict: Paginated response containing academy results. """ # Defensive: if no endpoint configured, return empty paginated shape if not self.academies_endpoint: @@ -62,47 +63,7 @@ def get_academies(self, academy_uuid: str = None, is_active: bool | None = None) params['academy_uuid'] = academy_uuid if is_active is not None: params['is_active'] = bool(is_active) - response = self.client.get(self.academies_endpoint, params=params) - response.raise_for_status() - - data = response.json() - - next_url = data.get('next') - - for next_payload in get_paginated_payloads(self.client, next_url, ): - - next_data = next_payload - - data["results"].extend(next_data.get("results", [])) - data["next"] = next_data.get("next") - data["previous"] = data.get("previous") or next_data.get("previous") - return data - - @backoff.on_exception(wait_gen=backoff.expo, exception=autoretry_for_exceptions) - def associate_academy_with_catalog(self, academy_uuid, enterprise_catalog_uuid): - """ - Associate an academy with an enterprise catalog in enterprise-catalog. - - Arguments: - academy_uuid (str|UUID): UUID of the academy to update. - enterprise_catalog_uuid (str|UUID): UUID of the enterprise catalog to associate. - - Returns: - dict: Response payload, or an empty dict when the endpoint returns no body. - """ - endpoint = urljoin(self.academies_endpoint, f'{academy_uuid}/associate-catalog/') - response = self.client.post( - endpoint, - json={'enterprise_catalog_uuid': str(enterprise_catalog_uuid)}, - ) - response.raise_for_status() - try: - return response.json() - except ValueError: - logger.warning( - "Failed to parse JSON response from Enterprise Catalog API." - ) - return {} + return fetch_all_results(self.client, self.academies_endpoint, params=params) @backoff.on_exception(wait_gen=backoff.expo, exception=autoretry_for_exceptions) def contains_content_items(self, catalog_uuid, content_ids): @@ -177,24 +138,17 @@ def get_content_metadata_count(self, catalog_uuid): @backoff.on_exception(wait_gen=backoff.expo, exception=autoretry_for_exceptions) def get_catalogs(self, enterprise_customer_uuid: str = None) -> dict | list: """ - Fetch a list of enterprise catalogs, optionally filtered by enterprise_customer_uuid. + Fetch a list of enterprise catalogs. - Returns a paginated-style dict or a raw list depending on the upstream service. - If the response contains a `next` link, subsequent pages will be fetched and merged into a single - results list. - """ - params = {'enterprise_customer': enterprise_customer_uuid} if enterprise_customer_uuid else None - response = self.client.get(self.enterprise_catalog_endpoint, params=params) - response.raise_for_status() - data = response.json() - - # Fetch and Merge paginated results using the shared pagination helper - for next_payload in get_paginated_payloads(self.client, data.get("next")): - data['results'].extend(next_payload.get('results', [])) - data['next'] = next_payload.get('next') - data['previous'] = data.get('previous') or next_payload.get('previous') + Optionally filters results by enterprise_customer_uuid. - return data + Returns: + dict: Paginated response containing enterprise catalog results. + """ + params = {} + if enterprise_customer_uuid: + params['enterprise_customer'] = enterprise_customer_uuid + return fetch_all_results(self.client, self.enterprise_catalog_endpoint, params=params) def content_metadata(self, content_id): raise NotImplementedError('There is currently no v2 API implementation for this endpoint.') diff --git a/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py b/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py index 5e3fe2fc..3761d7fc 100644 --- a/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py +++ b/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py @@ -181,7 +181,7 @@ def test_get_catalogs(self, mock_oauth_client): self.assertEqual(fetched, mock_response_json) mock_oauth_client.return_value.get.assert_called_with( 'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/', - params=None, + params={}, ) @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') @@ -196,7 +196,7 @@ def test_get_catalogs_with_enterprise_customer(self, mock_oauth_client): self.assertEqual(fetched, mock_response_json) mock_oauth_client.return_value.get.assert_called_with( 'http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/', - params={'enterprise_customer': customer_uuid}, + params={"enterprise_customer": customer_uuid, } ) @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') @@ -253,34 +253,6 @@ def test_get_catalogs_merges_paginated_results(self, mock_oauth_client): self.assertIsNone(fetched['next']) self.assertEqual(mock_oauth_client.return_value.get.call_count, 2) - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) - def test_associate_academy_with_catalog(self, mock_oauth_client): - academy_uuid = uuid4() - catalog_uuid = uuid4() - mock_post = mock_oauth_client.return_value.post - mock_post.return_value.json.return_value = {'detail': 'ok'} - - client = EnterpriseCatalogApiClient() - result = client.associate_academy_with_catalog(academy_uuid, catalog_uuid) - - self.assertEqual(result, {'detail': 'ok'}) - mock_post.assert_called_once_with( - f'http://enterprise-catalog.example.com/api/v1/academies/{academy_uuid}/associate-catalog/', - json={'enterprise_catalog_uuid': str(catalog_uuid)}, - ) - - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) - def test_associate_academy_with_catalog_empty_response_body(self, mock_oauth_client): - academy_uuid = uuid4() - catalog_uuid = uuid4() - mock_post = mock_oauth_client.return_value.post - mock_post.return_value.json.side_effect = ValueError() - - client = EnterpriseCatalogApiClient() - result = client.associate_academy_with_catalog(academy_uuid, catalog_uuid) - - self.assertEqual(result, {}) - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) def test_get_academies_with_empty_endpoint_returns_empty_payload(self, mock_oauth_client): client = EnterpriseCatalogApiClient() @@ -346,21 +318,6 @@ def test_catalog_content_metadata_traverse_false_allows_empty_keys(self, mock_oa res = client.catalog_content_metadata('catalog', [], traverse_pagination=False) self.assertEqual(res, {}) - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient', autospec=True) - def test_associate_academy_with_catalog_with_string_uuids(self, mock_oauth_client): - # Ensure we post the expected JSON body and handle JSON response - mock_post = mock_oauth_client.return_value.post - mock_post.return_value.json.return_value = {'detail': 'ok'} - - client = EnterpriseCatalogApiClient() - result = client.associate_academy_with_catalog('academy-1', 'catalog-1') - - self.assertEqual(result, {'detail': 'ok'}) - mock_post.assert_called_once_with( - 'http://enterprise-catalog.example.com/api/v1/academies/academy-1/associate-catalog/', - json={'enterprise_catalog_uuid': 'catalog-1'}, - ) - @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') def test_contains_content_items_true_value(self, mock_oauth_client): mock_oauth_client.return_value.get.return_value = mock.Mock( diff --git a/enterprise_access/apps/api_client/utils.py b/enterprise_access/apps/api_client/utils.py index d4438ee7..2eaa8f06 100644 --- a/enterprise_access/apps/api_client/utils.py +++ b/enterprise_access/apps/api_client/utils.py @@ -3,29 +3,34 @@ """ -def get_paginated_payloads(client, next_url): +def fetch_all_results(client, url, params=None): """ - Iterate through paginated API responses. - - Follows `next` links until all pages have been retrieved and yields - the JSON payload from each page response. + Fetch all paginated results. Args: - client: HTTP client instance. - next_url (str): Initial pagination URL. + client: HTTP client. + url (str): Endpoint URL. + params (dict | None): Optional query parameters. - Yields: - dict: Response payload for each page. + Returns: + dict: Response payload with all pages merged. """ - payloads = [] + if params is None: + params = {} + + response = client.get(url, params=params) + response.raise_for_status() + + data = response.json() - while next_url: - response = client.get(next_url) + while data.get("next"): + response = client.get(data["next"]) response.raise_for_status() - payload = response.json() - payloads.append(payload) + page = response.json() - next_url = payload.get("next") + data["results"].extend(page.get("results", [])) + data["next"] = page.get("next") + data["previous"] = data.get("previous") or page.get("previous") - return payloads + return data From 3ec5733c56cd8bad5e9570bfac9eee06a0f5096d Mon Sep 17 00:00:00 2001 From: Ismail Veruru Shaik Date: Thu, 25 Jun 2026 11:30:12 +0000 Subject: [PATCH 10/11] feat: added test for coverage --- .../tests/test_enterprise_catalog_client.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py b/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py index 3761d7fc..8533b363 100644 --- a/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py +++ b/enterprise_access/apps/api_client/tests/test_enterprise_catalog_client.py @@ -19,6 +19,7 @@ EnterpriseCatalogUserV1ApiClient ) from enterprise_access.apps.api_client.tests.test_constants import DATE_FORMAT_ISO_8601 +from enterprise_access.apps.api_client.utils import fetch_all_results from enterprise_access.apps.core.tests.factories import UserFactory from enterprise_access.utils import _days_from_now @@ -184,6 +185,30 @@ def test_get_catalogs(self, mock_oauth_client): params={}, ) + @mock.patch("enterprise_access.apps.api_client.base_oauth.OAuthAPIClient") + def test_fetch_all_results_with_none_params(self, mock_oauth_client): + first_page = { + "count": 1, + "next": None, + "previous": None, + "results": [{"uuid": "123"}], + } + + mock_oauth_client.return_value.get.return_value.json.return_value = first_page + + result = fetch_all_results( + mock_oauth_client.return_value, + "http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/", + params=None, + ) + + self.assertEqual(result, first_page) + + mock_oauth_client.return_value.get.assert_called_once_with( + "http://enterprise-catalog.example.com/api/v2/enterprise-catalogs/", + params={}, + ) + @mock.patch('enterprise_access.apps.api_client.base_oauth.OAuthAPIClient') def test_get_catalogs_with_enterprise_customer(self, mock_oauth_client): mock_response_json = {'count': 1, 'next': None, 'previous': None, 'results': [{'uuid': str(uuid4())}]} From 8db556bd437296d62a73e804bd9d8e370c8cbbed Mon Sep 17 00:00:00 2001 From: Ismail Veruru Shaik Date: Thu, 25 Jun 2026 22:23:13 +0530 Subject: [PATCH 11/11] feat: addressed quality issues --- enterprise_access/apps/api_client/enterprise_catalog_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/enterprise_access/apps/api_client/enterprise_catalog_client.py b/enterprise_access/apps/api_client/enterprise_catalog_client.py index cf2f3862..6d6aca40 100644 --- a/enterprise_access/apps/api_client/enterprise_catalog_client.py +++ b/enterprise_access/apps/api_client/enterprise_catalog_client.py @@ -64,7 +64,7 @@ def get_academies(self, academy_uuid: str = None, is_active: bool | None = None) if is_active is not None: params['is_active'] = bool(is_active) return fetch_all_results(self.client, self.academies_endpoint, params=params) - + @backoff.on_exception(wait_gen=backoff.expo, exception=autoretry_for_exceptions) def associate_academy_with_catalog(self, academy_uuid, enterprise_catalog_uuid): """