-
Notifications
You must be signed in to change notification settings - Fork 123
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
[Work-in-Progress] Notify when use charge is above limit (#278) #282
Conversation
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.
It might be a good idea to test that if maximum_limit
exists, it is always greater than quota
since any usage including in quota
is "free"...
When maximum_limit
is less than quota
, we would trigger a 'limit crossed' before reaching the end of the free tier...
I am currently of the opinion that we might want to enforce quota < maxium_limit
and trigger a signal when reaching quota
and another signal when reaching maximum_limit
.
saas/models.py
Outdated
@@ -4629,6 +4630,11 @@ def record_use_charge(subscription, use_charge, quantity=1): | |||
if usage < use_charge.quota: | |||
amount = 0 | |||
descr += " (complimentary in plan)" | |||
if use_charge.maximum_limit and use_charge.maximum_limit > 0: |
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.
if use_charge.maximum_limit:
will return the same condition as if use_charge.maximum_limit and use_charge.maximum_limit > 0:
.
- Added validation check to ensure use_charge.maximum_limit is greater than use_charge.quota - Added separate signals to send upon reaching quota and reaching the maximum limit. Notes: - Can consider implementing a flag within the transaction description to indicate when the quota or maximum limit has been reached. This would allow us to filter Transactions for the same subscription and prevent sending repeat signals within the same billing period.
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.
Code is good as-is, yet best to check the constraint before writing to the database model instead of when loading the record back.
saas/models.py
Outdated
@@ -4621,6 +4621,10 @@ def get_sub_event_id(subscription, use_charge=None): | |||
|
|||
|
|||
def record_use_charge(subscription, use_charge, quantity=1): | |||
if (use_charge.maximum_limit and use_charge.quota |
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 check should be done in the form / serializer or in UseCharge.save
.
- "maximum_limit" check moved to serializer - Updated PlanDetailSerializer to support the "maximum_limit."
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.
OK.
maximum_limit = data.get('maximum_limit') | ||
quota = data.get('quota') | ||
|
||
if maximum_limit is not None and maximum_limit != 0: |
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.
equivalent to if maximum_limit:
in Python.
Added a maximum limit field on the UseCharge model, logic to send a signal in record_use_charge method, a signal in signals.py, and a migrations file.