feat: add DiscountEligibilityStep pipeline step#2545
Conversation
Adds enterprise/filters/discounts.py with a DiscountEligibilityStep PipelineStep implementation that marks enterprise learners as ineligible for LMS-controlled course discounts, using the new DiscountEligibilityCheckRequested openedx-filter. ENT-11564 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pwnage101
left a comment
pwnage101
left a comment
There was a problem hiding this comment.
Notes for implementer (FYI @marlonkeating ).
Also don't forget to update the version of openedx-filters, and bump the edx-enterprise version in this PR.
| is_enterprise_learner = None | ||
|
|
||
|
|
||
| class DiscountEligibilityStep(PipelineStep): |
There was a problem hiding this comment.
Rename DiscountEligibilityStep to include "Enterprise" in the class name (e.g. DiscountEligibilityEnterpriseStep), mirroring the convention established by AccountSettingsEnterpriseReadOnlyFieldsStep in #2544. Pipeline step names show up directly in OPEN_EDX_FILTERS_CONFIG, and operators inspecting the configured pipeline need to be able to tell at a glance which step is owned by the enterprise plugin vs. other contributors.
| is_enterprise_learner = None | ||
|
|
||
|
|
||
| class DiscountEligibilityStep(PipelineStep): |
There was a problem hiding this comment.
Missing companion change in enterprise/settings/common.py: this PR adds the pipeline step but never wires it into ENTERPRISE_FILTERS_CONFIG, so even with the plugin loaded the step will never run. Every new enterprise pipeline step must be registered in the plugin's settings file in the same PR that adds it. Add an entry like:
"org.openedx.learning.discount.eligibility.check.requested.v1": {
"fail_silently": False,
"pipeline": ["enterprise.filters.discounts.<RenamedStep>"],
},| ``False`` so the calling code skips the discount. | ||
| """ | ||
|
|
||
| def run_filter(self, user, course_key, is_eligible): # pylint: disable=arguments-differ |
There was a problem hiding this comment.
Add type hints to run_filter. The docstring already names User, the course key, and bool — lift those into the signature:
def run_filter(self, user: User, course_key: CourseKey, is_eligible: bool) -> dict: # pylint: disable=arguments-differSame note was raised on the openedx-filters side in #335 ("docstring below already names User and CourseKey — lift those into the annotations"); applying it symmetrically here keeps the kwargs contract explicit and lets pyright/mypy catch step-vs-filter signature drift.
| dict: updated pipeline data. ``is_eligible`` is ``False`` when the user is | ||
| linked to an enterprise; otherwise the original ``is_eligible`` value is | ||
| preserved. | ||
| """ |
There was a problem hiding this comment.
Also, the first line of this pipeline step should be to log that we are starting this filter pipeline step. See similar comment from a different ticket: #2543 (comment)
1 participant
Adds enterprise/filters/discounts.py with a DiscountEligibilityStep PipelineStep implementation that marks enterprise learners as ineligible for LMS-controlled course discounts, using the new DiscountEligibilityCheckRequested openedx-filter.
ENT-11564
Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Blocked by:
Blocks: