feat: replace enterprise imports in learner_home/views.py with pluggable_override - #383
Merged
Merged
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
July 10, 2026 14:53
2445797 to
ceaf54b
Compare
pwnage101
reviewed
Jul 10, 2026
marlonkeating
force-pushed
the
mkeating/ENT-11571-ulmo
branch
2 times, most recently
from
July 20, 2026 18:28
119e0a0 to
9935474
Compare
marlonkeating
marked this pull request as ready for review
July 20, 2026 20:27
|
|
||
| import logging | ||
| from collections import OrderedDict | ||
| from typing import TYPE_CHECKING, Optional, TypedDict |
Comment on lines
+216
to
+219
| uuid: str | ||
| slug: str | ||
| auth_org_id: Optional[str] | ||
| enable_learner_portal: bool |
pwnage101
reviewed
Jul 21, 2026
marlonkeating
force-pushed
the
mkeating/ENT-11571-ulmo
branch
from
July 21, 2026 19:15
a357700 to
96ca085
Compare
pwnage101
reviewed
Jul 21, 2026
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
lms/djangoapps/learner_home/views.py:219
- EnterpriseDashboardSerializer explicitly supports missing
auth_org_id(see TestEnterpriseDashboardSerializer.test_no_auth_org_id), but this TypedDict currently requires theauth_org_idkey (even if its value is Optional). That makes the stated override contract stricter than the actual consumer and will cause type-checking friction for plugin authors who omit the key.
slug: str
auth_org_id: Optional[str]
enable_learner_portal: bool
pwnage101
requested changes
Jul 21, 2026
Comment on lines
+432
to
+437
| return { | ||
| "prev_fn": prev_fn, | ||
| "user": user, | ||
| "request": request, | ||
| "is_masquerading": is_masquerading, | ||
| } |
Comment on lines
+232
to
236
| Return the enterprise customer dict for the given user, or None. | ||
|
|
||
| This function can be overridden by an installed plugin via the | ||
| OVERRIDE_LEARNER_HOME_GET_ENTERPRISE_CUSTOMER setting. | ||
| """ |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
lms/djangoapps/learner_home/views.py:215
- EnterpriseDashboardSerializer (and its tests) support
auth_org_idbeing omitted entirely, but thisTypedDictcurrently makes all keys required. That can make the stated override contract stricter than the data actually accepted/produced (and stricter than existing enterprise-customer dicts), which reduces the usefulness of the type hint for override implementers.
class EnterpriseCustomerData(TypedDict):
pwnage101
approved these changes
Jul 22, 2026
pwnage101
left a comment
pwnage101
left a comment
Member
There was a problem hiding this comment.
LGTM! Please be careful to make sure what gets merged is one commit with a useful and accurate message. Ideally I'd like to have seen the commit message before approving
marlonkeating
force-pushed
the
mkeating/ENT-11571-ulmo
branch
from
July 22, 2026 15:19
3e82a18 to
0e8792e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sister PR to openedx#38107 as part of ENT-11571
Blocked on release of openedx/edx-enterprise#2554