feat: replace enterprise imports in learner_home/views.py with pluggable_override#383
feat: replace enterprise imports in learner_home/views.py with pluggable_override#383marlonkeating wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to decouple learner_home from the built-in Enterprise support code by removing direct Enterprise imports in lms/djangoapps/learner_home/views.py and introducing a pluggable_override hook for Enterprise customer lookup. It also bumps the edx-enterprise dependency version across the platform requirements/constraints to align with the related Enterprise-side changes.
Changes:
- Bump
edx-enterprisefrom8.3.0to8.5.0in constraints and compiled requirements sets. - Remove
openedx.features.enterprise_support.apiimports from Learner Home and add apluggable_override-decoratedget_enterprise_customerhook.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| requirements/edx/testing.txt | Bumps pinned edx-enterprise version for test requirements. |
| requirements/edx/doc.txt | Bumps pinned edx-enterprise version for doc requirements. |
| requirements/edx/development.txt | Bumps pinned edx-enterprise version for dev requirements. |
| requirements/edx/base.txt | Bumps pinned edx-enterprise version in base requirements. |
| requirements/constraints.txt | Bumps globally constrained edx-enterprise version. |
| lms/djangoapps/learner_home/views.py | Removes Enterprise support imports and introduces a pluggable override for Enterprise customer retrieval. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return learner_data[0]["enterprise_customer"] if learner_data else None | ||
| else: | ||
| return enterprise_customer_from_session_or_learner_data(request) | ||
| return None |
marlonkeating
force-pushed
the
mkeating/ENT-11571-ulmo
branch
from
2445797 to
ceaf54b
Compare
|
|
||
| @pluggable_override('OVERRIDE_LEARNER_HOME_GET_ENTERPRISE_CUSTOMER') | ||
| @function_trace("get_enterprise_customer") | ||
| def get_enterprise_customer(user, request, is_masquerading): |
There was a problem hiding this comment.
Now that the actual business logic is disjoint from this repo, we need to be as clear as possible about the contract. This function returns an object whose keys are hard-coded downstream, so at the very least those keys should be documented here at the plugin interface.
In the past, we'd just document the keys in the docstring, but I think nowadays I'd explicitly type the dict:
class EnterpriseCustomerData(TypedDict):
"""Contract for the OVERRIDE_LEARNER_HOME_GET_ENTERPRISE_CUSTOMER return value."""
name: str
uuid: str
slug: str
auth_org_id: Optional[str]
enable_learner_portal: boolThen update the function signature:
def get_enterprise_customer(
user: "User",
request: Request,
is_masquerading: bool,
) -> Optional[EnterpriseCustomerData]:
…ble_override
Remove enterprise_customer_from_session_or_learner_data and get_enterprise_learner_data_from_db
imports. Replace get_enterprise_customer body with a default return None, decorated with
@pluggable_override('OVERRIDE_LEARNER_HOME_GET_ENTERPRISE_CUSTOMER').
ENT-11571
marlonkeating
force-pushed
the
mkeating/ENT-11571-ulmo
branch
from
b201542 to
119e0a0
Compare
Sister PR to openedx#38107 as part of ENT-11571
Blocked on release of openedx/edx-enterprise#2554