Repository navigation
Conversation
d4b7d11 to
2c0bf73
Compare
brobro10000
left a comment
brobro10000
left a comment
There was a problem hiding this comment.
Reviewed with Claude Code (pr-review-hil, cross-checked against companion PRs openedx#2702 and openedx-platform#39166). Deletion is complete and verified — no dangling imports, pin bump is consistent with openedx#2702's release, and the two "deletion with no counterpart" cases (tests/factories.py, tests/test_logout.py) are both confirmed benign. Draft/CI-red status is expected and tracked separately (blocked on edx-enterprise==9.0.0 publishing, not a code issue). Two non-blocking items inline below — approving since nothing here blocks the move itself.
| injects its own URLs via ``url_config``, at which point ``enterprise.urls`` | ||
| moves there too. | ||
| """ | ||
| return django_apps.is_installed('enterprise') and settings.FEATURES.get('ENABLE_ENTERPRISE_INTEGRATION', False) |
There was a problem hiding this comment.
settings.FEATURES.get('ENABLE_ENTERPRISE_INTEGRATION', False) — confirmed this is byte-for-byte what the deleted enterprise_support/api.py already had, so no regression here. But ENABLE_ENTERPRISE_INTEGRATION is declared as a top-level setting in this repo (common.py:606, devstack.py:423), never inside FEATURES — so .get() always falls back to False unless some deployment's YAML config injects it into FEATURES directly. openedx-platform's equivalent already uses getattr(settings, 'ENABLE_ENTERPRISE_INTEGRATION', False), which matches its own settings the same way. Since this migration is already touching the function, worth confirming with whoever owns prod config whether this needs aligning — not asking you to guess at it blind. Non-blocking.
…enterprise Every hookable caller of openedx/features/enterprise_support/ has already been migrated behind filters, signals, and pluggable overrides, and the package itself has been moved into edx-enterprise as enterprise.platform_support. Drop the platform copy. - Delete openedx/features/enterprise_support/ in full. - Remove EnterpriseSupportConfig from INSTALLED_APPS in lms/envs/common.py. - Replace lms/urls.py's enterprise_support import with a module-private _enterprise_enabled(), keeping both halves of the original check, including release-ulmo's FEATURES['ENABLE_ENTERPRISE_INTEGRATION'] key. The conditional block and both of its includes (enterprise.urls and channel_integrations.urls) are unchanged; the helper is documented as temporary, to be removed once enterprise-integrated-channels becomes a plugin that injects its own URLs. - Delete the vestigial test_update_success_for_enterprise from the user_api accounts API tests. Ticket ENT-11510 moved readonly-field enforcement behind AccountSettingsReadOnlyFieldsRequested, and the filter test immediately below it already covers the real contract. - Relocate the package's EnterpriseLogoutTests to openedx/core/djangoapps/user_authn/views/tests/test_logout.py. It covers LogoutView._is_enterprise_target and the logout.html branch keyed on it, both of which stay in the LMS, and no other test in the repo covers them -- so deleting the package wholesale would drop their only coverage. The original's enterprise API mocks, customer fixtures and learner factory are not carried over: _is_enterprise_target is a pure regex over the redirect path and consults nothing in the enterprise app. - Drop the openedx.features.enterprise_support.utils import-linter contract from setup.cfg. - Bump the edx-enterprise pin to 9.0.0, the release that ships enterprise.platform_support. - Refresh the stale enterprise_support reference in ADR 0010. ENT-11576
2c0bf73 to
f5d3a68
Compare
2 participants
Every hookable caller of openedx/features/enterprise_support/ has already been migrated behind filters, signals, and pluggable overrides, and the package itself has been moved into edx-enterprise as enterprise.platform_support. Drop the platform copy.
ENT-11576