Skip to content

feat: add EnterpriseEnrollmentViewProcessor pipeline step for CourseEnrollmentStarted filter#2553

Merged
kiram15 merged 4 commits into
masterfrom
pwnage101/ENT-11570
Jul 9, 2026
Merged

feat: add EnterpriseEnrollmentViewProcessor pipeline step for CourseEnrollmentStarted filter#2553
kiram15 merged 4 commits into
masterfrom
pwnage101/ENT-11570

Conversation

@pwnage101

@pwnage101 pwnage101 commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

Description

An EnterpriseEnrollmentViewProcessor pipeline step is defined in enterprise/filters/enrollment.py for the new openedx-filter CourseEnrollmentViewStarted (here).

The pipeline step checks whether the enrollment user is linked to an enterprise customer, and if so, calls the enterprise and consent API clients to post the enterprise course enrollment and provide consent.

Jira

ENT-11570

Testing instructions

Requires all three branches installed together (follow instructions here)

Use this branch, openedx-filters branch openedx/openedx-filters#363, and edx-platform branch kiram15/ENT-11570

Needs ENABLE_ENTERPRISE_INTEGRATION=True, an API key, an EnterpriseCustomer, and a learner linked to it via EnterpriseCustomerUser.

In the LMS shell, POST to the enrollment endpoint as the worker, patching the two API clients to assert they fire:

from unittest import mock
from django.conf import settings
from rest_framework.test import APIClient

EC_UUID = "<enterprise-uuid>"
CID = "<course-id>"
worker = APIClient(); worker.credentials(HTTP_X_EDX_API_KEY=settings.EDX_API_KEY)

with mock.patch("enterprise.filters.enrollment.EnterpriseApiServiceClient") as MEnt, \
     mock.patch("enterprise.filters.enrollment.ConsentApiServiceClient") as MCon:
    resp = worker.post("/api/enrollment/v1/enrollment",
        {"user": "<username>", "course_details": {"course_id": CID},
         "mode": "audit", "linked_enterprise_customer": EC_UUID}, format="json")
    print(resp.status_code)
    print(MEnt.return_value.post_enterprise_course_enrollment.call_args)
    print(MCon.return_value.provide_consent.call_args)

Expected: 200; the LMS log shows EnterpriseEnrollmentViewProcessor running: …; post_enterprise_course_enrollment is called with (username, course_id); provide_consent is called with username, course_id, and enterprise_customer_uuid.

I also personally tested these other scenarios

  • no linked_enterprise: 200, neither client called
  • no API-key permission: 200, neither client called
  • enterprise post fails: 400, consent not called, no enrollment created
  • consent fails: 400, enterprise called once, no enrollment created

Related:

@codecov

codecov Bot commented Apr 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.56098% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.68%. Comparing base (8ed796e) to head (4679983).

Files with missing lines Patch % Lines
enterprise/filters/enrollment.py 97.56% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2553      +/-   ##
==========================================
+ Coverage   86.65%   86.68%   +0.02%     
==========================================
  Files         257      258       +1     
  Lines       16954    16995      +41     
  Branches     1677     1680       +3     
==========================================
+ Hits        14692    14732      +40     
  Misses       1927     1927              
- Partials      335      336       +1     
Flag Coverage Δ
unittests 86.68% <97.56%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@kiram15 kiram15 force-pushed the pwnage101/ENT-11570 branch from a35d99d to ceabb63 Compare April 23, 2026 20:39
@kiram15 kiram15 requested a review from Copilot April 23, 2026 20:49

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds an EnterpriseEnrollmentPostProcessor pipeline step to post enterprise enrollment + consent on course enrollment events, with accompanying unit tests.

Changes:

  • Introduces enterprise.filters.enrollment.EnterpriseEnrollmentPostProcessor pipeline step.
  • Adds unit tests covering enterprise vs non-enterprise paths and exception logging behavior.
  • Adds minimal tests/filters package init.

Reviewed changes

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

File Description
enterprise/filters/enrollment.py New pipeline step that posts enterprise enrollment and consent, logging failures.
enterprise/filters/init.py Initializes enterprise.filters package.
tests/filters/test_enrollment.py New tests validating API client calls and exception logging.
tests/filters/init.py Initializes tests.filters package.

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

Comment thread tests/filters/test_enrollment.py Outdated
Comment thread tests/filters/test_enrollment.py Outdated
Comment thread enterprise/filters/enrollment.py Outdated

@pwnage101 pwnage101 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I'm just going to submit this review but before you implement fixes I'd like to meet with you synchronously because there's a lot to unpack and we should discuss an alternate path.

It looks like the CourseEnrollmentStarted filter you were planning to use (possibly adopted from my draft) is installed at the model layer, not the view layer containing the enterprise logic we are trying to replace.

Keep in mind enterprise enrollment creation was added nearly 10 years ago to the view layer, and meanwhile the model layer enroll() method is called from management commands, other views, entitlements, program enrollments, auto-auth, tests, etc. (in all over 80 call sites), so THERE BE DRAGONS if we change this.

Comment thread enterprise/filters/enrollment.py
Comment thread enterprise/filters/enrollment.py Outdated
Comment thread tests/filters/test_enrollment.py Outdated
Comment thread enterprise/api_client/xpert_ai.py Outdated
@kiram15 kiram15 force-pushed the pwnage101/ENT-11570 branch 5 times, most recently from e92d54e to c93b4e3 Compare June 7, 2026 23:18
@kiram15 kiram15 changed the title feat: add EnterpriseEnrollmentPostProcessor pipeline step for CourseEnrollmentStarted filter feat: add EnterpriseEnrollmentViewProcessor pipeline step for CourseEnrollmentStarted filter Jun 8, 2026
Comment thread enterprise/filters/enrollment.py Outdated
assert additions[FILTER_A]['pipeline'] == [STEP_X]


class TestEnterpriseFiltersConfig(unittest.TestCase):

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind this test being added, but curious why you decided to add it? Seems unrelated.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not directly related to the work, more related to the general filter initiative as an smoke test

Comment thread enterprise/filters/enrollment.py Outdated
Comment thread enterprise/filters/enrollment.py Outdated
@kiram15 kiram15 force-pushed the pwnage101/ENT-11570 branch from b6fd649 to d85c7e6 Compare June 23, 2026 21:53
@kiram15 kiram15 force-pushed the pwnage101/ENT-11570 branch from d85c7e6 to 04865fd Compare June 23, 2026 22:22
Comment thread enterprise/filters/enrollment.py Outdated
Comment thread tests/filters/test_enrollment.py Outdated
Comment thread tests/filters/test_enrollment.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Comment thread enterprise/filters/enrollment.py
@kiram15 kiram15 force-pushed the pwnage101/ENT-11570 branch from b886f42 to 0c22832 Compare June 26, 2026 20:15
@kiram15 kiram15 force-pushed the pwnage101/ENT-11570 branch from 02f1f3d to b5ce226 Compare July 8, 2026 21:13

@pwnage101 pwnage101 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just remove the dead code and merge.

Comment thread tests/filters/test_enrollment.py Outdated
Comment thread tests/filters/test_enrollment.py Outdated
Comment thread tests/filters/test_enrollment.py Outdated
@kiram15 kiram15 force-pushed the pwnage101/ENT-11570 branch from b5ce226 to 5d5017f Compare July 9, 2026 20:30
@kiram15 kiram15 force-pushed the pwnage101/ENT-11570 branch from 5d5017f to 4679983 Compare July 9, 2026 20:32
@kiram15 kiram15 enabled auto-merge (squash) July 9, 2026 20:52
@kiram15 kiram15 merged commit a045f9c into master Jul 9, 2026
11 checks passed
@kiram15 kiram15 deleted the pwnage101/ENT-11570 branch July 9, 2026 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants