Add academy association to Enterprise catalog client by vshaikismail-sonata · Pull Request #210 · edx/enterprise-access

vshaikismail-sonata · 2026-06-22T17:33:12Z

Description:
Add a description of your changes here.

Jira:
ENT-XXXX

Merge checklist:

./manage.py makemigrations has been run
- Note: This must be run if you modified any models.
  - It may or may not make a migration depending on exactly what you modified, but it should still be run.

Post merge:

Ensure that your changes went out to the stage instance
Deploy to prod instance

codecov · 2026-06-22T17:37:09Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.58%. Comparing base (0854543) to head (8db556b).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #210      +/-   ##
==========================================
+ Coverage   86.52%   86.58%   +0.05%     
==========================================
  Files         154      155       +1     
  Lines       12920    12953      +33     
  Branches     1233     1239       +6     
==========================================
+ Hits        11179    11215      +36     
+ Misses       1425     1423       -2     
+ Partials      316      315       -1

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

rthota-sonata-hue

Changes are requested.

wrongly commented.

rthota-sonata-hue

Please resolve the patch check before sharing to Alex. Looks Good for merge. Approved.

iloveagent57 · 2026-06-23T20:04:50Z

+        response.raise_for_status()
+        raw_payload = response.json()
+
+        def normalize_page(payload):


this feels overly-defensive for handling an API response of an endpoint that we own/maintain. Just have it expect the schema that the endpoint actually uses.

iloveagent57 · 2026-06-23T20:06:26Z

+        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


Instead of using this duplicated pattern, it would be better to extract the pattern used here:

enterprise-access/enterprise_access/apps/api_client/license_manager_client.py

Line 133 in 21ab523

def get_learner_subscription_licenses_for_admin(

into a shared helper/utility method, and then use that utility method inside this new client methods.

iloveagent57 · 2026-06-24T18:12:52Z

+        if not params:
+            params = None


why not just pass in empty params?

iloveagent57 · 2026-06-24T18:13:19Z

+        while next_url:
+            next_resp = self.client.get(next_url)
+            next_resp.raise_for_status()
+
+            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')
+        return data


Use the utility method you abstracted here.

iloveagent57 · 2026-06-24T18:13:57Z

+        try:
+            return response.json()
+        except ValueError:
+            return {}


we should at least log something here instead of silently swallowing the exception.

This comment also addressed but it s not showing outdated....!!!

iloveagent57 · 2026-06-24T18:15:43Z

+        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', []))


A lot of this seems like reusable stuff that should live inside of the utility function

iloveagent57 · 2026-06-24T18:17:51Z

+        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)


There's really no need to be this defensive - the endpoint we're calling should have a consistent and expected response schema.

iloveagent57 · 2026-06-24T20:40:49Z

+"""
+
+
+def get_paginated_payloads(client, next_url):


Let's push the accumulation into the helper too, that'll help simplify things quite a bit:

def fetch_all_results(client, url, params=None): response = client.get(url, params=params) response.raise_for_status() data = response.json() while data.get('next'): resp = client.get(data['next']) resp.raise_for_status() page = resp.json() data['results'].extend(page.get('results', [])) data['next'] = page.get('next') return data

Callers become:

def get_academies(self, academy_uuid=None, is_active=None): 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) return fetch_all_results(self.client, self.academies_endpoint, params=params or None) def get_catalogs(self, enterprise_customer_uuid=None): params = {'enterprise_customer': enterprise_customer_uuid} if enterprise_customer_uuid else None return fetch_all_results(self.client, self.enterprise_catalog_endpoint, params=params)

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

vshaikismail-sonata added 2 commits June 22, 2026 11:42

feat: Addressed comments for PR B

954d2d5

feat: catalog client implementation

aaf41d1

vshaikismail-sonata requested review from a team as code owners June 22, 2026 17:33

vshaikismail-sonata added 2 commits June 22, 2026 18:46

feat: added test case for coverage

07740ef

feat: updated version in tests

4379308

rthota-sonata-hue previously approved these changes Jun 23, 2026

View reviewed changes

rthota-sonata-hue self-requested a review June 23, 2026 07:52

feat: added patch test

194d1da

vshaikismail-sonata force-pushed the 11883-2 branch from d7af54e to 194d1da Compare June 23, 2026 14:25

rthota-sonata-hue approved these changes Jun 23, 2026

View reviewed changes

iloveagent57 requested changes Jun 23, 2026

View reviewed changes

feat: addressed commets

43dd14e

vshaikismail-sonata force-pushed the 11883-2 branch from 21ab523 to 43dd14e Compare June 24, 2026 11:03

vshaikismail-sonata added 2 commits June 24, 2026 17:49

feat: added new helper

76977f8

Merge branch 'main' into 11883-2

7673ec5

iloveagent57 requested changes Jun 24, 2026

View reviewed changes

feat: more review comments addressed

15c0978

iloveagent57 reviewed Jun 24, 2026

View reviewed changes

vshaikismail-sonata added 3 commits June 24, 2026 21:57

feat: updated helper and removed redundant function

abaeffc

feat: added test for coverage

3ec5733

Merge branch 'main' into 11883-2

7264d39

iloveagent57 approved these changes Jun 25, 2026

View reviewed changes

Merge branch 'main' into 11883-2

f1bf8a4

Copilot AI review requested due to automatic review settings June 25, 2026 14:03

Copilot started reviewing on behalf of vshaikismail-sonata June 25, 2026 14:04 View session

Copilot AI reviewed Jun 25, 2026

vshaikismail-sonata force-pushed the 11883-2 branch 3 times, most recently from f6639a3 to f1bf8a4 Compare June 25, 2026 15:30

Merge branch 'main' into 11883-2

9bdb6fe

vshaikismail-sonata force-pushed the 11883-2 branch from 55a83ee to 9bdb6fe Compare June 25, 2026 16:52

feat: addressed quality issues

8db556b

vshaikismail-sonata merged commit 1ac224a into main Jun 25, 2026
6 checks passed

vshaikismail-sonata deleted the 11883-2 branch June 25, 2026 16:58

Uh oh!

Conversation

vshaikismail-sonata commented Jun 22, 2026

Uh oh!

codecov Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

rthota-sonata-hue left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rthota-sonata-hue left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented Jun 22, 2026 •

edited

Loading

rthota-sonata-hue left a comment •

edited

Loading

rthota-sonata-hue left a comment •

edited

Loading