From 980736be7588bbcec47cdd139df881acf0dbde6e Mon Sep 17 00:00:00 2001 From: nospame Date: Tue, 10 Dec 2024 15:02:20 -0800 Subject: [PATCH 1/8] Remove _ensure_full_coverage --- corehq/apps/accounting/invoicing.py | 71 ----------------------------- 1 file changed, 71 deletions(-) diff --git a/corehq/apps/accounting/invoicing.py b/corehq/apps/accounting/invoicing.py index ca254cf0f624..52aecea41173 100644 --- a/corehq/apps/accounting/invoicing.py +++ b/corehq/apps/accounting/invoicing.py @@ -28,7 +28,6 @@ CreditLine, CustomerBillingRecord, CustomerInvoice, - DefaultProductPlan, DomainUserHistory, EntryPoint, FeatureType, @@ -84,7 +83,6 @@ def __init__(self, date_start, date_end, domain, recipients=None): def create_invoices(self): all_subscriptions = self._get_subscriptions() - self._ensure_full_coverage(all_subscriptions) chargeable_subscriptions = [sub for sub in all_subscriptions if sub.plan_version.plan.edition != SoftwarePlanEdition.PAUSED] for subscription in chargeable_subscriptions: @@ -111,47 +109,6 @@ def _get_subscriptions(self): ).order_by('date_start', 'date_end').all() return list(subscriptions) - @transaction.atomic - def _ensure_full_coverage(self, subscriptions): - plan_version = DefaultProductPlan.get_default_plan_version() - if not plan_version.feature_charges_exist_for_domain(self.domain): - return - - community_ranges = self._get_community_ranges(subscriptions) - if not community_ranges: - return - - # First check to make sure none of the existing subscriptions is set - # to do not invoice. Let's be on the safe side and not send a - # community invoice out, if that's the case. - do_not_invoice = any([s.do_not_invoice for s in subscriptions]) - - account = BillingAccount.get_or_create_account_by_domain( - self.domain.name, - created_by=self.__class__.__name__, - entry_point=EntryPoint.SELF_STARTED, - )[0] - if account.date_confirmed_extra_charges is None: - log_accounting_info( - "Did not generate invoice because date_confirmed_extra_charges " - "was null for domain %s" % self.domain.name - ) - do_not_invoice = True - - for start_date, end_date in community_ranges: - # create a new community subscription for each - # date range that the domain did not have a subscription - community_subscription = Subscription( - account=account, - plan_version=plan_version, - subscriber=self.subscriber, - date_start=start_date, - date_end=end_date, - do_not_invoice=do_not_invoice, - ) - community_subscription.save() - subscriptions.append(community_subscription) - def _create_invoice_for_subscription(self, subscription): def _get_invoice_start(sub, date_start): return max(sub.date_start, date_start) @@ -194,34 +151,6 @@ def _get_invoice_end(sub, date_end): return invoice - def _get_community_ranges(self, subscriptions): - community_ranges = [] - if len(subscriptions) == 0: - return [(self.date_start, self.date_end + datetime.timedelta(days=1))] - else: - prev_sub_end = self.date_end - for ind, sub in enumerate(subscriptions): - if ind == 0 and sub.date_start > self.date_start: - # the first subscription started AFTER the beginning - # of the invoicing period - community_ranges.append((self.date_start, sub.date_start)) - - if prev_sub_end < self.date_end and sub.date_start > prev_sub_end: - community_ranges.append((prev_sub_end, sub.date_start)) - prev_sub_end = sub.date_end - - if ( - ind == len(subscriptions) - 1 - and sub.date_end is not None - and sub.date_end <= self.date_end - ): - # the last subscription ended BEFORE the end of - # the invoicing period - community_ranges.append( - (sub.date_end, self.date_end + datetime.timedelta(days=1)) - ) - return community_ranges - def _generate_invoice(self, subscription, invoice_start, invoice_end): # use create_or_get when is_hidden_to_ops is False to utilize unique index on Invoice # so our test will make sure the unique index prevent race condition From 3bfdd704893ed2fddbfbee08672e033b4553576d Mon Sep 17 00:00:00 2001 From: nospame Date: Tue, 10 Dec 2024 15:07:16 -0800 Subject: [PATCH 2/8] Remove tests for _ensure_full_coverage --- .../accounting/tests/test_invoice_factory.py | 69 ------------------- 1 file changed, 69 deletions(-) diff --git a/corehq/apps/accounting/tests/test_invoice_factory.py b/corehq/apps/accounting/tests/test_invoice_factory.py index 8ad47c26078a..6a31f7149e4a 100644 --- a/corehq/apps/accounting/tests/test_invoice_factory.py +++ b/corehq/apps/accounting/tests/test_invoice_factory.py @@ -57,75 +57,6 @@ def test_feature_charges(self): self.assertFalse(self.community.feature_charges_exist_for_domain(domain_under_limits)) domain_under_limits.delete() - def test_incomplete_starting_coverage(self): - some_plan = generator.subscribable_plan_version() - subscription = Subscription.new_domain_subscription( - self.account, self.domain.name, some_plan, - date_start=self.invoice_start + datetime.timedelta(days=3) - ) - subscriptions = self.invoice_factory._get_subscriptions() - community_ranges = self.invoice_factory._get_community_ranges(subscriptions) - self.assertEqual(len(community_ranges), 1) - self.assertEqual(community_ranges[0][0], self.invoice_start) - self.assertEqual(community_ranges[0][1], subscription.date_start) - - def test_incomplete_ending_coverage(self): - some_plan = generator.subscribable_plan_version() - subscription = Subscription.new_domain_subscription( - self.account, self.domain.name, some_plan, - date_start=self.invoice_start, - date_end=self.invoice_end - datetime.timedelta(days=3) - ) - subscriptions = self.invoice_factory._get_subscriptions() - community_ranges = self.invoice_factory._get_community_ranges(subscriptions) - self.assertEqual(len(community_ranges), 1) - self.assertEqual(community_ranges[0][0], subscription.date_end) - self.assertEqual(community_ranges[0][1], - self.invoice_end + datetime.timedelta(days=1)) - - def test_patchy_coverage(self): - some_plan = generator.subscribable_plan_version() - middle_date = self.invoice_end - datetime.timedelta(days=15) - Subscription.new_domain_subscription( - self.account, self.domain.name, some_plan, - date_start=self.invoice_start + datetime.timedelta(days=1), - date_end=middle_date - ) - next_start = middle_date + datetime.timedelta(days=2) - next_end = next_start + datetime.timedelta(days=2) - Subscription.new_domain_subscription( - self.account, self.domain.name, some_plan, - date_start=next_start, - date_end=next_end, - ) - final_start = next_end + datetime.timedelta(days=2) - Subscription.new_domain_subscription( - self.account, self.domain.name, some_plan, - date_start=final_start, - date_end=self.invoice_end - datetime.timedelta(days=1), - ) - subscriptions = self.invoice_factory._get_subscriptions() - self.assertEqual(len(subscriptions), 3) - community_ranges = self.invoice_factory._get_community_ranges(subscriptions) - self.assertEqual(len(community_ranges), 4) - - def test_full_coverage(self): - some_plan = generator.subscribable_plan_version() - Subscription.new_domain_subscription( - self.account, self.domain.name, some_plan, - date_start=self.invoice_start, - date_end=self.invoice_end + datetime.timedelta(days=1), - ) - subscriptions = self.invoice_factory._get_subscriptions() - community_ranges = self.invoice_factory._get_community_ranges(subscriptions) - self.assertEqual(len(community_ranges), 0) - - def test_no_coverage(self): - subscriptions = self.invoice_factory._get_subscriptions() - self.assertEqual(len(subscriptions), 0) - community_ranges = self.invoice_factory._get_community_ranges(subscriptions) - self.assertEqual(community_ranges, [(self.invoice_start, self.invoice_end + datetime.timedelta(days=1))]) - def test_community_plan_generates_invoice(self): """ Ensure that Community plans can generate invoices. From c62e38b25a3508b12aeb8ca1ac1aa536c0d1d7e0 Mon Sep 17 00:00:00 2001 From: nospame Date: Tue, 10 Dec 2024 15:30:14 -0800 Subject: [PATCH 3/8] Update test_over_community_limit to assign subscription --- corehq/apps/accounting/tests/test_invoicing.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/corehq/apps/accounting/tests/test_invoicing.py b/corehq/apps/accounting/tests/test_invoicing.py index 83fd1f1162d1..93e7fa49cc5d 100644 --- a/corehq/apps/accounting/tests/test_invoicing.py +++ b/corehq/apps/accounting/tests/test_invoicing.py @@ -2,6 +2,8 @@ import random from decimal import Decimal +from dateutil.relativedelta import relativedelta + from django.conf import settings from django.core import mail from django.test import override_settings @@ -23,6 +25,7 @@ Invoice, SoftwarePlanEdition, Subscriber, + Subscription, SubscriptionType, ) from corehq.apps.accounting.tasks import calculate_users_in_all_domains @@ -417,7 +420,7 @@ def num_users(): def test_community_over_limit(self): """ - For a domain under community (no subscription) with users over the community limit, make sure that: + For a domain under community with users over the community limit, make sure that: - base_description is None - base_cost is 0.0 - unit_description is not None @@ -436,8 +439,14 @@ def test_community_over_limit(self): account.date_confirmed_extra_charges = today account.save() + community_plan = DefaultProductPlan.get_default_plan_version() + Subscription.new_domain_subscription( + account, domain.name, community_plan, + date_start=datetime.date(today.year, today.month, 1) - relativedelta(months=1), + ) + calculate_users_in_all_domains(datetime.date(today.year, today.month, 1)) - tasks.generate_invoices_based_on_date(datetime.date.today()) + tasks.generate_invoices_based_on_date(today) subscriber = Subscriber.objects.get(domain=domain.name) invoice = Invoice.objects.filter(subscription__subscriber=subscriber).get() user_line_item = invoice.lineitem_set.get_feature_by_type(FeatureType.USER).get() From e760209141e1db626f51a2000099b653b686f0e7 Mon Sep 17 00:00:00 2001 From: nospame Date: Tue, 10 Dec 2024 15:40:37 -0800 Subject: [PATCH 4/8] Remove test_community_invoice Redundant between test_subscription_invoice and TestDomainInvoiceFactory::test_community_plan_generates_invoice --- .../apps/accounting/tests/test_invoicing.py | 28 ------------------- 1 file changed, 28 deletions(-) diff --git a/corehq/apps/accounting/tests/test_invoicing.py b/corehq/apps/accounting/tests/test_invoicing.py index 93e7fa49cc5d..c76595eba715 100644 --- a/corehq/apps/accounting/tests/test_invoicing.py +++ b/corehq/apps/accounting/tests/test_invoicing.py @@ -145,34 +145,6 @@ def test_community_no_charges_no_invoice(self): self.assertRaises(Invoice.DoesNotExist, lambda: Invoice.objects.get(subscription__subscriber__domain=domain.name)) - def test_community_invoice(self): - """ - For an unsubscribed domain with any charges over the community limit for the month of invoicing, - make sure that an invoice is generated in addition to a subscription for that month to - the community plan. - """ - domain = generator.arbitrary_domain() - self.addCleanup(domain.delete) - generator.create_excess_community_users(domain) - account = BillingAccount.get_or_create_account_by_domain( - domain, created_by=self.dimagi_user)[0] - generator.arbitrary_contact_info(account, self.dimagi_user) - account.date_confirmed_extra_charges = datetime.date.today() - account.save() - today = datetime.date.today() - calculate_users_in_all_domains(datetime.date(today.year, today.month, 1)) - tasks.generate_invoices_based_on_date(today) - subscriber = Subscriber.objects.get(domain=domain.name) - invoices = Invoice.objects.filter(subscription__subscriber=subscriber) - self.assertEqual(invoices.count(), 1) - invoice = invoices.get() - self.assertEqual(invoice.subscription.subscriber.domain, domain.name) - self.assertEqual(invoice.subscription.date_start, invoice.date_start) - self.assertEqual( - invoice.subscription.date_end - datetime.timedelta(days=1), - invoice.date_end - ) - def test_date_due_not_set_small_invoice(self): """Date Due doesn't get set if the invoice is small""" invoice_date_small = utils.months_from_date(self.subscription.date_start, 1) From 893f6af7796d56bb4f3dc0213c63cd5c9df643c0 Mon Sep 17 00:00:00 2001 From: nospame Date: Tue, 10 Dec 2024 15:41:50 -0800 Subject: [PATCH 5/8] Rename to test_no_subscription_no_charges_no_invoice This should never happen in real life because all accounts are assigned a subscription, but is still a relevant test case to ensure that invoicing doesn't fail altogether --- corehq/apps/accounting/tests/test_invoicing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/corehq/apps/accounting/tests/test_invoicing.py b/corehq/apps/accounting/tests/test_invoicing.py index c76595eba715..a94de4411d60 100644 --- a/corehq/apps/accounting/tests/test_invoicing.py +++ b/corehq/apps/accounting/tests/test_invoicing.py @@ -134,7 +134,7 @@ def test_no_invoice_after_end(self): tasks.generate_invoices_based_on_date(invoice_date) self.assertEqual(self.subscription.invoice_set.count(), 0) - def test_community_no_charges_no_invoice(self): + def test_no_subscription_no_charges_no_invoice(self): """ No invoices should be generated for domains that are not on a subscription and do not have any per_excess charges on users or SMS messages From 2821c2bc10f9a43f3e1a12883e73662806d403e8 Mon Sep 17 00:00:00 2001 From: nospame Date: Tue, 10 Dec 2024 15:43:50 -0800 Subject: [PATCH 6/8] Remove add_months_to_date from test_invoicing --- corehq/apps/accounting/tests/test_invoicing.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/corehq/apps/accounting/tests/test_invoicing.py b/corehq/apps/accounting/tests/test_invoicing.py index a94de4411d60..ab65d508062c 100644 --- a/corehq/apps/accounting/tests/test_invoicing.py +++ b/corehq/apps/accounting/tests/test_invoicing.py @@ -8,8 +8,6 @@ from django.core import mail from django.test import override_settings -from dimagi.utils.dates import add_months_to_date - from corehq.apps.accounting import tasks, utils from corehq.apps.accounting.invoicing import DomainInvoiceFactory from corehq.apps.accounting.models import ( @@ -71,7 +69,7 @@ def setUpClass(cls): if cls.is_testing_web_user_feature: # make sure the subscription is still active when we count web users cls.subscription_is_active = True - cls.subscription_end_date = add_months_to_date(cls.subscription_start_date, cls.subscription_length) + cls.subscription_end_date = cls.subscription_start_date + relativedelta(months=cls.subscription_length) cls.subscription = generator.generate_domain_subscription( cls.account, cls.domain, From 72528eb9c08898c3301f1912acd86a5bcda5f664 Mon Sep 17 00:00:00 2001 From: nospame Date: Wed, 18 Dec 2024 16:06:32 -0800 Subject: [PATCH 7/8] Revert "Remove test_community_invoice" This reverts commit e760209141e1db626f51a2000099b653b686f0e7. --- .../apps/accounting/tests/test_invoicing.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/corehq/apps/accounting/tests/test_invoicing.py b/corehq/apps/accounting/tests/test_invoicing.py index ab65d508062c..00a8f16b20aa 100644 --- a/corehq/apps/accounting/tests/test_invoicing.py +++ b/corehq/apps/accounting/tests/test_invoicing.py @@ -143,6 +143,34 @@ def test_no_subscription_no_charges_no_invoice(self): self.assertRaises(Invoice.DoesNotExist, lambda: Invoice.objects.get(subscription__subscriber__domain=domain.name)) + def test_community_invoice(self): + """ + For an unsubscribed domain with any charges over the community limit for the month of invoicing, + make sure that an invoice is generated in addition to a subscription for that month to + the community plan. + """ + domain = generator.arbitrary_domain() + self.addCleanup(domain.delete) + generator.create_excess_community_users(domain) + account = BillingAccount.get_or_create_account_by_domain( + domain, created_by=self.dimagi_user)[0] + generator.arbitrary_contact_info(account, self.dimagi_user) + account.date_confirmed_extra_charges = datetime.date.today() + account.save() + today = datetime.date.today() + calculate_users_in_all_domains(datetime.date(today.year, today.month, 1)) + tasks.generate_invoices_based_on_date(today) + subscriber = Subscriber.objects.get(domain=domain.name) + invoices = Invoice.objects.filter(subscription__subscriber=subscriber) + self.assertEqual(invoices.count(), 1) + invoice = invoices.get() + self.assertEqual(invoice.subscription.subscriber.domain, domain.name) + self.assertEqual(invoice.subscription.date_start, invoice.date_start) + self.assertEqual( + invoice.subscription.date_end - datetime.timedelta(days=1), + invoice.date_end + ) + def test_date_due_not_set_small_invoice(self): """Date Due doesn't get set if the invoice is small""" invoice_date_small = utils.months_from_date(self.subscription.date_start, 1) From 54e762667ad052e8a5649ea2def45543c8b649db Mon Sep 17 00:00:00 2001 From: nospame Date: Wed, 18 Dec 2024 16:21:07 -0800 Subject: [PATCH 8/8] Update tests to use generator.generate_domain_subscription --- .../apps/accounting/tests/test_invoice_factory.py | 8 +++----- corehq/apps/accounting/tests/test_invoicing.py | 14 ++++++-------- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/corehq/apps/accounting/tests/test_invoice_factory.py b/corehq/apps/accounting/tests/test_invoice_factory.py index 6a31f7149e4a..80899d9847c1 100644 --- a/corehq/apps/accounting/tests/test_invoice_factory.py +++ b/corehq/apps/accounting/tests/test_invoice_factory.py @@ -61,11 +61,9 @@ def test_community_plan_generates_invoice(self): """ Ensure that Community plans can generate invoices. """ - community_plan = DefaultProductPlan.get_default_plan_version() - subscription = Subscription.new_domain_subscription( - self.account, self.domain.name, community_plan, - date_start=self.invoice_start, - date_end=self.invoice_end + datetime.timedelta(days=1), + subscription = generator.generate_domain_subscription( + self.account, self.domain, self.invoice_start, None, + plan_version=generator.subscribable_plan_version(edition=SoftwarePlanEdition.COMMUNITY) ) DomainUserHistory.objects.create( domain=self.domain.name, record_date=self.invoice_end, num_users=10) diff --git a/corehq/apps/accounting/tests/test_invoicing.py b/corehq/apps/accounting/tests/test_invoicing.py index 00a8f16b20aa..b138c3578c2c 100644 --- a/corehq/apps/accounting/tests/test_invoicing.py +++ b/corehq/apps/accounting/tests/test_invoicing.py @@ -145,9 +145,8 @@ def test_no_subscription_no_charges_no_invoice(self): def test_community_invoice(self): """ - For an unsubscribed domain with any charges over the community limit for the month of invoicing, - make sure that an invoice is generated in addition to a subscription for that month to - the community plan. + For community-subscribed domain with any charges over the community limit for the month of invoicing, + make sure that an invoice is generated. """ domain = generator.arbitrary_domain() self.addCleanup(domain.delete) @@ -155,6 +154,10 @@ def test_community_invoice(self): account = BillingAccount.get_or_create_account_by_domain( domain, created_by=self.dimagi_user)[0] generator.arbitrary_contact_info(account, self.dimagi_user) + generator.generate_domain_subscription( + account, domain, self.subscription_start_date, None, + plan_version=generator.subscribable_plan_version(edition=SoftwarePlanEdition.COMMUNITY) + ) account.date_confirmed_extra_charges = datetime.date.today() account.save() today = datetime.date.today() @@ -165,11 +168,6 @@ def test_community_invoice(self): self.assertEqual(invoices.count(), 1) invoice = invoices.get() self.assertEqual(invoice.subscription.subscriber.domain, domain.name) - self.assertEqual(invoice.subscription.date_start, invoice.date_start) - self.assertEqual( - invoice.subscription.date_end - datetime.timedelta(days=1), - invoice.date_end - ) def test_date_due_not_set_small_invoice(self): """Date Due doesn't get set if the invoice is small"""