-
Notifications
You must be signed in to change notification settings - Fork 571
feat(usage): return the current billing period #8501
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
base: main
Are you sure you want to change the base?
Changes from all commits
71bc9fa
cabc9ef
2a19f18
6f9e941
b3e5927
4899df3
7f4c73e
a1fec6f
2f5bf0f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -1,8 +1,9 @@ | ||||||||
| import re | ||||||||
| from datetime import timedelta | ||||||||
| from datetime import datetime, timedelta | ||||||||
| from typing import Any | ||||||||
|
|
||||||||
| from common.core.utils import is_enterprise, is_saas | ||||||||
| from dateutil.relativedelta import relativedelta | ||||||||
| from django.conf import settings | ||||||||
| from django.core.cache import caches | ||||||||
| from django.core.validators import MaxValueValidator, MinValueValidator | ||||||||
|
|
@@ -321,6 +322,12 @@ def has_active_billing_periods(self) -> bool: | |||||||
| and self.organisation.subscription_information_cache.has_active_billing_periods() | ||||||||
| ) | ||||||||
|
|
||||||||
| @property | ||||||||
| def current_billing_period(self) -> tuple[datetime, datetime] | None: | ||||||||
| if not self.organisation.has_subscription_information_cache(): | ||||||||
| return None | ||||||||
| return self.organisation.subscription_information_cache.current_billing_period() | ||||||||
|
|
||||||||
| @property | ||||||||
| def is_free_plan(self) -> bool: | ||||||||
| return self.subscription_plan_family == SubscriptionPlanFamily.FREE | ||||||||
|
|
@@ -601,19 +608,35 @@ def _get_default_subscription_metadata_kwargs(self) -> dict[str, Any]: | |||||||
| } | ||||||||
|
|
||||||||
| def has_active_billing_periods(self) -> bool: | ||||||||
| """Whether the organisation is inside a billing term.""" | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The name of this function is sufficient, we don't need the docstring.
Suggested change
|
||||||||
| return self.current_billing_period() is not None | ||||||||
|
|
||||||||
| def current_billing_period(self) -> tuple[datetime, datetime] | None: | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, as above, let's use the |
||||||||
| """ | ||||||||
| Returns True if current date is within the billing term. | ||||||||
| If either start or end date is None, returns False. | ||||||||
| Returns the monthly allowance window, or None outside a billing term. | ||||||||
| A term can run longer than a month, so the window opens at the most | ||||||||
| recent monthly anniversary of its start. | ||||||||
| """ | ||||||||
| starts_at, ends_at = ( | ||||||||
| self.current_billing_term_starts_at, | ||||||||
| self.current_billing_term_ends_at, | ||||||||
| ) | ||||||||
|
|
||||||||
| starts_at = self.current_billing_term_starts_at | ||||||||
| ends_at = self.current_billing_term_ends_at | ||||||||
| if starts_at is None or ends_at is None: | ||||||||
| return False | ||||||||
| return None | ||||||||
|
|
||||||||
| # One reading, so a clock crossing the term end mid-method cannot open | ||||||||
| # a window past it. The end is exclusive: at that instant the term is | ||||||||
| # over and the next one has not been written yet. | ||||||||
|
Comment on lines
+625
to
+627
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| now = timezone.now() | ||||||||
| if not starts_at <= now < ends_at: | ||||||||
| return None | ||||||||
|
|
||||||||
| return starts_at <= timezone.now() <= ends_at | ||||||||
| elapsed = relativedelta(now, starts_at) | ||||||||
| months = elapsed.years * 12 + elapsed.months | ||||||||
| # Both ends count from the term start. Counting the second from the | ||||||||
| # first loses the original day when a month is too short for it. | ||||||||
|
Comment on lines
+634
to
+635
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| return ( | ||||||||
| starts_at + relativedelta(months=months), | ||||||||
| starts_at + relativedelta(months=months + 1), | ||||||||
| ) | ||||||||
|
|
||||||||
|
|
||||||||
| class OrganisationAPIUsageNotification(models.Model): | ||||||||
|
|
||||||||
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.
Can we make this a NamedTuple?
Something like:
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'm also not sure this should be a property.
self.organisation.subscription_information_cache.current_billing_period()relies ontimezone.now(), hence it is dynamic which for me feels wrong for a property.