feat: filter academies by actual content in enterprise catalog (ENT-11220) by sjasti-sonata-svg · Pull Request #73 · edx/enterprise-catalog

sjasti-sonata-svg · 2026-04-10T10:56:01Z

Context

As part of ENT-11220, Academies is being enabled by default for all Teams and Enterprise Subscription customers with search enabled.

Acceptance Criteria (ENT-11220):

If a customer does not have Academy courses in their subscription catalog, then they should not be able to see Academies.

This PR is mandatory to fulfill the above AC. Without it, the ticket is incomplete.

Why This Change Is Required

The Problem (without this change)

The current academies API (AcademiesReadOnlyViewSet.get_queryset) only checks if an academy's catalog is associated with the enterprise customer:

# Current code - only checks catalog association
if enterprise_associated_catalogs:
    user_accessible_academy_uuids.append(academy.uuid)  # ← always adds if associated

This means: if an academy's catalog is linked to an enterprise but the catalog does not actually contain the academy's courses, the learner still sees the academy tile. They click it and find zero or partial courses — a broken experience that violates the AC.

The Fix (this PR)

We now verify that the academy has actual course content in the enterprise's catalog before showing it:

# New code - checks catalog association AND content existence
if enterprise_associated_catalogs:
    academy_content = ContentMetadata.objects.filter(tags__academies=academy)
    catalog_query_ids = enterprise_associated_catalogs.values_list('catalog_query_id', flat=True)
    if academy_content.filter(catalog_queries__in=catalog_query_ids).exists():
        user_accessible_academy_uuids.append(academy.uuid)  # ← only adds if content exists

Data flow:

Academy → Tags → ContentMetadata (courses in the academy)
                        ↓
              catalog_queries (M2M)
                        ↓
              CatalogQuery ← EnterpriseCatalog (customer's catalog)
                        ↓
              If content overlap exists → show academy
              If no content overlap → hide academy

Files Changed

File	Change	Why
`views/academies.py` (line 12)	Added `ContentMetadata` import	Required for the content check query
`views/academies.py` (lines 49-55)	Added content existence check	Ensures academy has actual courses in the catalog before showing it
`tests/test_views.py` (lines 2072-2074)	Added `ContentMetadata` to test setup	Tests need content data to validate the new filtering logic

What Happens Without This PR

Scenario	Without this PR	With this PR
Academy catalog associated AND has courses	✅ Shows academy	✅ Shows academy
Academy catalog associated but NO courses in catalog	❌ Shows empty academy (AC violated)	✅ Hides academy (AC met)
No catalog association	✅ Hidden	✅ Hidden

Related PR

feat: enable academies by default for all enterprise customers with search (ENT-11220) openedx/edx-enterprise#2594 (model default True + data migration for existing customers)

Ticket

https://2u-internal.atlassian.net/browse/ENT-11220

Test plan

Verify academies API returns academies only when actual course content exists in the enterprise catalog
Verify academies API returns empty list when catalog is associated but has no matching course content
Verify all 4 existing academy tests pass (AcademiesViewSetTests)

Copilot

Pull request overview

Updates the Academies API so academies are only listed for an enterprise customer when there is matching content in that enterprise’s catalog query, preventing “empty” academies from appearing to learners.

Changes:

Add a content-existence filter in AcademiesReadOnlyViewSet.get_queryset() that checks ContentMetadata linked via academy tags and the enterprise’s catalog query.
Update AcademiesViewSetTests setup to create ContentMetadata wired to tags + catalog query to satisfy the new filter.
Refresh/pin a wide set of Python dependencies across environment requirement files.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
requirements/validation.txt	Dependency pin updates for validation tooling environment.
requirements/test.txt	Dependency pin updates for test environment.
requirements/quality.txt	Dependency pin updates for lint/quality environment.
requirements/production.txt	Dependency pin updates for production environment.
requirements/pip.txt	Regenerated pip requirements metadata/comments.
requirements/pip-tools.txt	Update build dependency used by pip-tools workflow.
requirements/doc.txt	Dependency pin updates for docs build environment.
requirements/django.txt	Bump pinned Django patch version.
requirements/dev.txt	Dependency pin updates for dev environment.
requirements/common_constraints.txt	Removes a previously-global pip constraint.
requirements/base.txt	Core dependency pin updates shared by other requirement sets.
enterprise_catalog/apps/api/v1/views/academies.py	Adds “actual content exists in enterprise catalog” gating for academy listing.
enterprise_catalog/apps/api/v1/tests/test_views.py	Adjusts academies view tests to create related content metadata for the new gating.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-10T11:02:22Z

+                            'catalog_query_id', flat=True,
+                        )
+                        if academy_content.filter(catalog_queries__in=catalog_query_ids).exists():
+                            user_accessible_academy_uuids.append(academy.uuid)


The current implementation does two DB hits per academy (truthiness evaluation of enterprise_associated_catalogs plus the ContentMetadata...exists() check), resulting in an N+1 query pattern that will scale poorly as the number of academies grows. Consider rewriting this to use a single ORM query (joins/subquery) that directly returns academies with (1) an associated EnterpriseCatalog for the given enterprise_uuid and (2) at least one related ContentMetadata matching those catalogs’ catalog_query IDs, using .exists() where appropriate instead of relying on queryset truthiness.

The number of academies is small in practice, so the N+1 pattern has negligible impact here. We can optimize this in a follow-up if needed.

Copilot · 2026-04-10T11:02:22Z

+                        # Verify the academy has actual content in the enterprise catalog,
+                        # not just a catalog association.
+                        academy_content = ContentMetadata.objects.filter(tags__academies=academy)
+                        catalog_query_ids = enterprise_associated_catalogs.values_list(
+                            'catalog_query_id', flat=True,
+                        )
+                        if academy_content.filter(catalog_queries__in=catalog_query_ids).exists():
+                            user_accessible_academy_uuids.append(academy.uuid)


The new “content exists” check does not constrain ContentMetadata to course records (e.g., content_type=COURSE). As written, a single program/pathway (or even a course run) tagged to the academy and present in the catalog query could cause the academy to appear, which may not satisfy the ticket’s requirement of “Academy courses in the subscription catalog.” Please narrow the filter to the intended content types (and update the test setup accordingly).

Good catch. Updated to filter by content_type=COURSE to match the AC requirement of "Academy courses in the subscription catalog."

Copilot · 2026-04-10T11:02:22Z

+        # Create content metadata linked to academy tags and catalog query
+        # so that the academy passes the content existence check.
+        self.content_metadata = ContentMetadataFactory()
+        self.content_metadata.catalog_queries.add(self.enterprise_catalog_query)
+        self.tag1.content_metadata.add(self.content_metadata)


The tests were updated to create content that satisfies the new check, but they still don’t cover the key negative case from the acceptance criteria: an academy is associated to an enterprise catalog but has no matching course content in that catalog query and therefore should be excluded. Add an explicit test asserting the list endpoint returns an empty set (or excludes that academy) in this scenario. Also, ContentMetadataFactory() cycles content types starting with COURSE_RUN; to avoid accidental pass/fail depending on future filtering, create course metadata explicitly (e.g., content_type=COURSE) for this setup.

Added a negative test case (test_list_excludes_academy_without_course_content) that verifies an academy with catalog association but no matching course content is excluded. Also updated the existing setup to explicitly use content_type=COURSE.

Copilot · 2026-04-10T11:02:23Z

@@ -30,13 +30,13 @@ backoff==1.10.0
    # via analytics-python
 billiard==4.2.4
    # via celery
-celery==5.6.0
+celery==5.6.2
    # via
    #   -c requirements/constraints.txt
    #   -r requirements/base.in
    #   django-celery-results
    #   edx-celeryutils
-certifi==2025.11.12
+certifi==2026.1.4


This PR includes a broad dependency refresh across multiple requirements files (e.g., Django/celery/pylint/etc. bumps), but the PR description focuses solely on academy filtering behavior. Please either (a) update the PR description/test plan to explicitly call out the dependency upgrade rationale/validation, or (b) split the dependency changes into a separate PR to keep review scope and rollback risk manageable.

We have not updated any requirements files in this PR. The dependency changes are from a separate commit on the base branch (commit 4cd5bf0 - "chore: Upgrade Python requirements") that was merged independently. This PR only touches views/academies.py and tests/test_views.py.

codecov · 2026-04-10T11:14:51Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.29%. Comparing base (3f600ae) to head (5dfb8f9).

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #73      +/-   ##
==========================================
+ Coverage   85.28%   85.29%   +0.01%     
==========================================
  Files         101      101              
  Lines        5613     5617       +4     
  Branches      670      670              
==========================================
+ Hits         4787     4791       +4     
  Misses        699      699              
  Partials      127      127

☔ View full report in Codecov by Sentry.
📢 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.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-12T06:43:45Z

@@ -54,7 +56,17 @@ def get_queryset(self):
                        enterprise_uuid=enterprise_customer
                    )
                    if enterprise_associated_catalogs:
-                        user_accessible_academy_uuids.append(academy.uuid)
+                        # Verify the academy has actual content in the enterprise catalog,
+                        # not just a catalog association.
+                        academy_content = ContentMetadata.objects.filter(
+                            tags__academies=academy,
+                            content_type=COURSE,
+                        )
+                        catalog_query_ids = enterprise_associated_catalogs.values_list(
+                            'catalog_query_id', flat=True,
+                        )
+                        if academy_content.filter(catalog_queries__in=catalog_query_ids).exists():


enterprise_associated_catalogs is being evaluated for truthiness, which triggers a DB query, and then you immediately run additional queries (values_list + exists()). You can drop the if enterprise_associated_catalogs: guard and rely on the final .exists() check (an empty catalog_query_ids subquery will naturally yield no matches), which avoids one query per academy without changing behavior.

Fixed. Removed the if enterprise_associated_catalogs: guard and consolidated into a single .exists() query.

Copilot · 2026-04-12T06:43:45Z

+        # Create an academy with a catalog association but no content metadata
+        academy_no_content = AcademyFactory()
+        tag_no_content = TagFactory(title='empty-tag')


TagFactory automatically creates and attaches ContentMetadata records in its post_generation hook, so tag_no_content (and therefore academy_no_content) actually has course content metadata. This makes the inline comment/docstring inaccurate and makes the test less explicit about what it’s validating. Consider creating the Tag directly (or clearing tag_no_content.content_metadata) so the test deterministically represents an academy with a catalog association but no course content.

Suggested change

# Create an academy with a catalog association but no content metadata

academy_no_content = AcademyFactory()

tag_no_content = TagFactory(title='empty-tag')

# Create an academy with a catalog association but explicitly no content metadata.

academy_no_content = AcademyFactory()

tag_no_content = TagFactory(title='empty-tag')

tag_no_content.content_metadata.clear()

Good catch. Added tag_no_content.content_metadata.clear() so the test deterministically represents an academy with no course content.

…1220) Verify that an academy has actual course content in the enterprise's catalog before showing it. Previously, only catalog association was checked, which could show empty academies to learners. - Filter ContentMetadata by content_type=COURSE to match AC - Add negative test for academy exclusion when no course content exists

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

iloveagent57 · 2026-04-21T15:37:41Z

+                    # Verify the academy has actual course content in the enterprise catalog.
+                    catalog_query_ids = enterprise_associated_catalogs.values_list(
+                        'catalog_query_id', flat=True,
+                    )
+                    has_content = ContentMetadata.objects.filter(
+                        tags__academies=academy,
+                        content_type=COURSE,
+                        catalog_queries__in=catalog_query_ids,
+                    ).exists()
+                    if has_content:


I don't understand why this change is needed at all - if the given customer has a catalog that's associated with the academy, then that academy should be surfaced to the customer's users. Can you describe how to re-create the issue that this pull request is attempting to fix?

From a quick chat with copilot:

The scenario the PR describes — "catalog is associated but has no matching course content" — sounds like a data integrity problem, not something the academies API should be papering over at query time.

A few specific concerns:
The catalog association is the contract. An EnterpriseCatalog links an enterprise to a CatalogQuery. If the academy is tied to that catalog, the expectation is that the catalog's query is set up to include the academy's courses. If it doesn't, that's a misconfiguration of the catalog or the academy — not something we should silently hide.

Performance cost in the hot path. The new code runs a ContentMetadata query with joins across tags, academies, and catalog_queries for every academy, on every list request. That's a meaningful N+1 situation in what's currently a simple filter.

It masks bad data. If an academy is associated with a catalog that doesn't actually contain relevant courses, we probably want to know about that — it likely means something went wrong in the catalog setup or content sync. Silently hiding the academy makes that harder to diagnose.

iloveagent57 · 2026-05-12T14:16:16Z

@sjasti-sonata-svg is this PR still needed? If not, we can close it. Thanks!

Copilot AI review requested due to automatic review settings April 10, 2026 10:56

sjasti-sonata-svg requested review from a team as code owners April 10, 2026 10:56

Copilot started reviewing on behalf of sjasti-sonata-svg April 10, 2026 10:56 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

sjasti-sonata-svg force-pushed the sjasti/ENT-11220-academies-content-filter branch from 9071d22 to 9006671 Compare April 10, 2026 11:12

Copilot AI review requested due to automatic review settings April 12, 2026 06:39

Copilot started reviewing on behalf of sjasti-sonata-svg April 12, 2026 06:40 View session

Copilot AI reviewed Apr 12, 2026

View reviewed changes

sjasti-sonata-svg force-pushed the sjasti/ENT-11220-academies-content-filter branch from 00dd12f to 7a0937c Compare April 12, 2026 06:46

sjasti-sonata-svg force-pushed the sjasti/ENT-11220-academies-content-filter branch from 7a0937c to 5dfb8f9 Compare April 12, 2026 06:55

Copilot AI review requested due to automatic review settings April 12, 2026 06:55

Copilot started reviewing on behalf of sjasti-sonata-svg April 12, 2026 06:56 View session

Copilot AI reviewed Apr 12, 2026

View reviewed changes

sjasti-sonata-svg mentioned this pull request Apr 15, 2026

feat: add migration for enable_academies default value change (ENT-11220) openedx/edx-enterprise#2597

Merged

3 tasks

iloveagent57 self-assigned this Apr 21, 2026

iloveagent57 requested changes Apr 21, 2026

View reviewed changes

Conversation

sjasti-sonata-svg commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Why This Change Is Required

The Problem (without this change)

The Fix (this PR)

Files Changed

What Happens Without This PR

Related PR

Ticket

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 12, 2026

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

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iloveagent57 commented May 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sjasti-sonata-svg commented Apr 10, 2026 •

edited

Loading

codecov Bot commented Apr 10, 2026 •

edited

Loading