ENT-1289 | Removing enterprise entitlement logic from codebase #590
Conversation
christopappas
requested review from
brittneyexline,
adamstankiewicz and
douglashall
There was a problem hiding this comment.
Looks like you got all of the entitlement-related code in edx-enterprise. Only question I have is whether we should also remove enterprise.HistoricalEnterpriseCustomerEntitlement: from the .annotation_safe_list.yml file. I don't see any other references to HistoricalEnterpriseCustomerEntitlement in edx-enterprise.
enterprise/api_client/ecommerce.py
Outdated
| @@ -43,25 +40,3 @@ def __init__(self, user): | |||
|
|
|||
| self.user = user | |||
| self.client = ecommerce_api_client(user) | |||
|
|
|||
| def get_course_final_price(self, mode, currency='$', enterprise_catalog_uuid=None): | |||
There was a problem hiding this comment.
although the comment implies this is specific to entitlements, i think it's lying. it seems like this is generally trying to get discounted prices from ecommerce, and this is still relevant to the enterprise enrollment workflow. i would keep this in.
enterprise/views.py
Outdated
| @@ -922,22 +921,6 @@ class CourseEnrollmentView(NonAtomicView): | |||
| 'self_paced': _('Self-Paced') | |||
| } | |||
|
|
|||
| def set_final_prices(self, modes, request): | |||
There was a problem hiding this comment.
again, i would keep all of this in.
|
@brittneyexline tagged you for rereview -- added back in the things you called out in your comments |
Updating tests and a few other spots I missed Adding back in some things I should not have taken out Bumping version and changelog
christopappas
force-pushed
the
cpappas/ENT-1289
branch
from
2fef26e to
c733cc0
Compare
…dx#590) Updating tests and a few other spots I missed Adding back in some things I should not have taken out Bumping version and changelog
This needs to be merged and deployed AFTER the corresponding ecommerce PR here: https://github.com/edx/ecommerce/pull/2612
Merge checklist:
requirements/*.txtfiles)base.inif needed in production but edx-platform doesn't install ittest-master.inif edx-platform pins it, with a matching versionmake upgrade && make requirementshave been run to regenerate requirementsmake statichas been run to update webpack bundling if any static content was updatedPost merge: