Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggest a fix for ordering access policies so assignments are prioritized. #416

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions enterprise_access/apps/subsidy_access_policy/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ class SegmentEvents:

# Configure the priority of each policy type here. When given multiple redeemable policies to select for redemption,
# the policy resolution engine will select policies with the lowest priority number.
CREDIT_POLICY_TYPE_PRIORITY = 1
SUBSCRIPTION_POLICY_TYPE_PRIORITY = 2
# BB - Some padding between the larger policy type grouping...
CREDIT_POLICY_TYPE_PRIORITY = 10
SUBSCRIPTION_POLICY_TYPE_PRIORITY = 20


class PolicyTypes:
Expand Down
31 changes: 28 additions & 3 deletions enterprise_access/apps/subsidy_access_policy/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -931,14 +931,15 @@ def __str__(self):
class CreditPolicyMixin:
"""
Mixin class for credit type policies.
BB - Something in the docs about how to use this? Potentially remove?
"""

@property
def priority(self):
return CREDIT_POLICY_TYPE_PRIORITY


class PerLearnerEnrollmentCreditAccessPolicy(CreditPolicyMixin, SubsidyAccessPolicy):
class PerLearnerEnrollmentCreditAccessPolicy(SubsidyAccessPolicy):
"""
Policy that limits the number of enrollments transactions for a learner in a subsidy.

Expand Down Expand Up @@ -1007,8 +1008,16 @@ def remaining_balance_per_user(self, lms_user_id):
existing_transaction_count = len(self.transactions_for_learner(lms_user_id)['transactions'])
return self.per_learner_enrollment_limit - existing_transaction_count

@property
def priority(self):
"""
Ensure that Per learner enrollment policies are evaluated before spend cap but after assignment
based policy types.
"""
return CREDIT_POLICY_TYPE_PRIORITY + 1

class PerLearnerSpendCreditAccessPolicy(CreditPolicyMixin, SubsidyAccessPolicy):

class PerLearnerSpendCreditAccessPolicy(SubsidyAccessPolicy):
"""
Policy that limits the amount of learner spend for enrollment transactions.

Expand Down Expand Up @@ -1089,8 +1098,15 @@ def remaining_balance_per_user(self, lms_user_id=None):
positive_spent_amount = spent_amount * -1
return self.per_learner_spend_limit - positive_spent_amount

@property
def priority(self):
"""
Ensure that Per leaner spend policies are evaluated last, after other similar policy types.
"""
return CREDIT_POLICY_TYPE_PRIORITY + 2


class AssignedLearnerCreditAccessPolicy(CreditPolicyMixin, SubsidyAccessPolicy):
class AssignedLearnerCreditAccessPolicy(SubsidyAccessPolicy):
"""
Policy based on LearnerContentAssignments, backed by a learner credit type of subsidy.

Expand Down Expand Up @@ -1427,6 +1443,7 @@ def allocate(self, learner_emails, content_key, content_price_cents):
content_price_cents,
)

<<<<<<< Updated upstream

class PolicyGroupAssociation(TimeStampedModel):
"""
Expand Down Expand Up @@ -1455,3 +1472,11 @@ class Meta:
null=False,
help_text='The uuid that uniquely identifies the associated group.',
)
=======
@property
def priority(self):
"""
Ensure that assignment policies are evaluated before other similar policy types
"""
return CREDIT_POLICY_TYPE_PRIORITY + 0
>>>>>>> Stashed changes
Loading