-
Notifications
You must be signed in to change notification settings - Fork 9
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
feat: Adds additional error reasons to SubsidyAccessPolicy #223
Conversation
if not content_metadata: | ||
return (False, REASON_CONTENT_NOT_IN_CATALOG, existing_transactions) | ||
|
||
if not subsidy_can_redeem_payload.get('can_redeem', False): | ||
return (False, REASON_NOT_ENOUGH_VALUE_IN_SUBSIDY, existing_transactions) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic swapped to avoid additional call into the subsidies can_redeem
. Check if the content_metadata
exist in the catalog before checking on a subsidy level.
578f52c
to
3603243
Compare
return (False, REASON_SUBSIDY_EXPIRED, []) | ||
|
||
# inactive policy with spend remaining (could have redeemed) | ||
if not self.active and not self.will_exceed_spend_limit(content_key, content_metadata=content_metadata): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This and the changes on like 149 are a little sus i think. I synced with @brobro10000 on slack with some suggestions. He's going to take another look at the logic in here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going to take another look at it after our discussion yesterday 👍🏽
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this makes the logic around the error handling easier to read. Nice work.
Adds more reasons and an additional error message for learner credit in the
SubsidyAccessPolicy
model for thecan_redeem
endpoint.Additional logic needed within the enterprise-subsidy repository to specify if a subsidy is not active by checking
isActive
within the subsidycan_redeem
endpointPending Tests