From af323ebe259b4d4e5769731d447dfc96d54ed699 Mon Sep 17 00:00:00 2001 From: Marc Date: Fri, 26 Sep 2014 10:38:50 +0000 Subject: [PATCH] Enough of fucking billing tests0 --- orchestra/apps/orders/models.py | 10 +++- orchestra/apps/resources/models.py | 1 + orchestra/apps/services/actions.py | 14 +++++ orchestra/apps/services/admin.py | 6 +- orchestra/apps/services/handlers.py | 7 +-- orchestra/apps/services/models.py | 7 +++ orchestra/apps/services/rating.py | 24 +++++++- orchestra/apps/services/settings.py | 3 + .../tests/functional_tests/test_domain.py | 5 +- .../tests/functional_tests/test_ftp.py | 3 - .../tests/functional_tests/test_job.py | 7 +-- .../tests/functional_tests/test_mailbox.py | 1 - .../tests/functional_tests/test_plan.py | 34 +++++++---- .../tests/functional_tests/test_traffic.py | 21 +++++-- orchestra/apps/services/tests/test_handler.py | 58 +++++++++++++------ 15 files changed, 150 insertions(+), 51 deletions(-) create mode 100644 orchestra/apps/services/actions.py diff --git a/orchestra/apps/orders/models.py b/orchestra/apps/orders/models.py index be69d778..651d5126 100644 --- a/orchestra/apps/orders/models.py +++ b/orchestra/apps/orders/models.py @@ -100,9 +100,13 @@ class Order(models.Model): return str(self.service) @classmethod - def update_orders(cls, instance): - Service = get_model(*settings.ORDERS_SERVICE_MODEL.split('.')) - for service in Service.get_services(instance): + def update_orders(cls, instance, service=None): + if service is None: + Service = get_model(*settings.ORDERS_SERVICE_MODEL.split('.')) + services = Service.get_services(instance) + else: + services = [service] + for service in services: orders = Order.objects.by_object(instance, service=service).active() if service.handler.matches(instance): if not orders: diff --git a/orchestra/apps/resources/models.py b/orchestra/apps/resources/models.py index 02d5a047..108e0b92 100644 --- a/orchestra/apps/resources/models.py +++ b/orchestra/apps/resources/models.py @@ -101,6 +101,7 @@ class Resource(models.Model): task.crontab = self.crontab task.save() if created: + # This only work on tests because of multiprocessing used on real deployments create_resource_relation() def delete(self, *args, **kwargs): diff --git a/orchestra/apps/services/actions.py b/orchestra/apps/services/actions.py new file mode 100644 index 00000000..0b84330f --- /dev/null +++ b/orchestra/apps/services/actions.py @@ -0,0 +1,14 @@ +from django.contrib import messages +from django.db import transaction +from django.utils.translation import ugettext_lazy as _ + + +@transaction.atomic +def update_orders(modeladmin, request, queryset): + for service in queryset: + service.update_orders() + modeladmin.log_change(request, transaction, 'Update orders') + msg = _("Orders for %s selected services have been updated.") % queryset.count() + modeladmin.message_user(request, msg) +update_orders.url_name = 'update-orders' +update_orders.verbose_name = _("Update orders") diff --git a/orchestra/apps/services/admin.py b/orchestra/apps/services/admin.py index 3582fd03..555995b2 100644 --- a/orchestra/apps/services/admin.py +++ b/orchestra/apps/services/admin.py @@ -4,10 +4,12 @@ from django.core.urlresolvers import reverse from django.utils import timezone from django.utils.translation import ugettext_lazy as _ +from orchestra.admin import ChangeViewActionsMixin from orchestra.admin.filters import UsedContentTypeFilter from orchestra.apps.accounts.admin import AccountAdminMixin from orchestra.core import services +from .actions import update_orders from .models import Plan, ContractedPlan, Rate, Service @@ -27,7 +29,7 @@ class ContractedPlanAdmin(AccountAdminMixin, admin.ModelAdmin): list_filter = ('plan__name',) -class ServiceAdmin(admin.ModelAdmin): +class ServiceAdmin(ChangeViewActionsMixin, admin.ModelAdmin): list_display = ( 'description', 'content_type', 'handler_type', 'num_orders', 'is_active' ) @@ -49,6 +51,8 @@ class ServiceAdmin(admin.ModelAdmin): }), ) inlines = [RateInline] + actions = [update_orders] + change_view_actions = actions def formfield_for_dbfield(self, db_field, **kwargs): """ Improve performance of account field and filter by account """ diff --git a/orchestra/apps/services/handlers.py b/orchestra/apps/services/handlers.py index 37197f91..3434efce 100644 --- a/orchestra/apps/services/handlers.py +++ b/orchestra/apps/services/handlers.py @@ -18,6 +18,8 @@ class ServiceHandler(plugins.Plugin): """ Separates all the logic of billing handling from the model allowing to better customize its behaviout + + Relax and enjoy the journey. """ model = None @@ -213,7 +215,6 @@ class ServiceHandler(plugins.Plugin): compensations, used_compensations = helpers.compensate(interval, compensations) order._compensations = used_compensations for comp in used_compensations: - # TODO get min right comp.order.new_billed_until = min(comp.order.billed_until, comp.ini, getattr(comp.order, 'new_billed_until', datetime.date.max)) if options.get('commit', True): @@ -337,7 +338,6 @@ class ServiceHandler(plugins.Plugin): # In most cases: # ini >= registered_date, end < registered_date # boundary lookup and exclude cancelled and billed - # TODO service.payment_style == self.POSTPAY no discounts no shit on_cancel orders_ = [] bp = None ini = datetime.date.max @@ -359,7 +359,7 @@ class ServiceHandler(plugins.Plugin): # Compensation related_orders = account.orders.filter(service=self.service) - if self.on_cancel == self.COMPENSATE: + if self.payment_style == self.PREPAY and self.on_cancel == self.COMPENSATE: # Get orders pending for compensation givers = list(related_orders.givers(ini, end)) givers.sort(cmp=helpers.cmp_billed_until_or_registered_on) @@ -381,7 +381,6 @@ class ServiceHandler(plugins.Plugin): # Periodic billing with no pricing period lines = self.bill_concurrent_orders(account, porders, rates, ini, end) else: - # TODO compensation in this case? # Periodic and one-time billing with pricing period lines = self.bill_registered_or_renew_events(account, porders, rates) else: diff --git a/orchestra/apps/services/models.py b/orchestra/apps/services/models.py index 17b70f7f..80e72eb7 100644 --- a/orchestra/apps/services/models.py +++ b/orchestra/apps/services/models.py @@ -3,6 +3,7 @@ import sys from django.db import models from django.db.models import F, Q +from django.db.models.loading import get_model from django.db.models.signals import pre_delete, post_delete, post_save from django.dispatch import receiver from django.contrib.contenttypes import generic @@ -324,6 +325,12 @@ class Service(models.Model): @property def rate_method(self): return self.RATE_METHODS[self.rate_algorithm] + + def update_orders(self): + order_model = get_model(settings.SERVICES_ORDER_MODEL) + related_model = self.content_type.model_class() + for instance in related_model.objects.all(): + order_model.update_orders(instance, service=self) accounts.register(ContractedPlan) diff --git a/orchestra/apps/services/rating.py b/orchestra/apps/services/rating.py index 20a3792f..369e5a57 100644 --- a/orchestra/apps/services/rating.py +++ b/orchestra/apps/services/rating.py @@ -42,11 +42,32 @@ def _compute(rates, metric): return value, steps +def _prepend_missing(rates): + """ + Support for incomplete rates + When first rate (quantity=5, price=10) defaults to nominal_price + """ + if rates: + first = rates[0] + if first.quantity == 0: + first.quantity = 1 + elif first.quantity > 1: + if not isinstance(rates, list): + rates = list(rates) + service = first.service + rate_class = type(first) + rates.insert(0, + rate_class(service=service, plan=first.plan, quantity=1, price=service.nominal_price) + ) + return rates + + def step_price(rates, metric): # Step price group = [] minimal = (sys.maxint, []) for plan, rates in rates.group_by('plan').iteritems(): + rates = _prepend_missing(rates) value, steps = _compute(rates, metric) if plan.is_combinable: group.append(steps) @@ -102,7 +123,8 @@ def match_price(rates, metric): candidates = [] selected = False prev = None - for rate in rates.distinct(): + rates = _prepend_missing(rates.distinct()) + for rate in rates: if prev: if prev.plan != rate.plan: if not selected and prev.quantity <= metric: diff --git a/orchestra/apps/services/settings.py b/orchestra/apps/services/settings.py index 06f49423..db6d5100 100644 --- a/orchestra/apps/services/settings.py +++ b/orchestra/apps/services/settings.py @@ -12,3 +12,6 @@ SERVICES_SERVICE_DEFAUL_TAX = getattr(settings, 'ORDERS_SERVICE_DFAULT_TAX', 0) SERVICES_SERVICE_ANUAL_BILLING_MONTH = getattr(settings, 'SERVICES_SERVICE_ANUAL_BILLING_MONTH', 4) + + +SERVICES_ORDER_MODEL = getattr(settings, 'SERVICES_ORDER_MODEL', 'orders.Order') diff --git a/orchestra/apps/services/tests/functional_tests/test_domain.py b/orchestra/apps/services/tests/functional_tests/test_domain.py index ba760383..27fcaa08 100644 --- a/orchestra/apps/services/tests/functional_tests/test_domain.py +++ b/orchestra/apps/services/tests/functional_tests/test_domain.py @@ -36,8 +36,9 @@ class DomainBillingTest(BaseBillingTest): if not account: account = self.create_account() domain_name = '%s.es' % random_ascii(10) - domain_service, __ = MiscService.objects.get_or_create(name='domain .es', description='Domain .ES') - return Miscellaneous.objects.create(service=domain_service, description=domain_name, account=account) + domain_service, __ = MiscService.objects.get_or_create(name='domain .es', + description='Domain .ES') + return account.miscellaneous.create(service=domain_service, description=domain_name) def test_domain(self): service = self.create_domain_service() diff --git a/orchestra/apps/services/tests/functional_tests/test_ftp.py b/orchestra/apps/services/tests/functional_tests/test_ftp.py index 229492b7..3567fb36 100644 --- a/orchestra/apps/services/tests/functional_tests/test_ftp.py +++ b/orchestra/apps/services/tests/functional_tests/test_ftp.py @@ -98,6 +98,3 @@ class FTPBillingTest(BaseBillingTest): order = account.orders.order_by('-id').first() self.assertEqual(first_bp, order.billed_until) self.assertEqual(decimal.Decimal(0), bills[0].get_total()) - - def test_ftp_account_with_rates(self): - pass diff --git a/orchestra/apps/services/tests/functional_tests/test_job.py b/orchestra/apps/services/tests/functional_tests/test_job.py index a54071fe..6d29b5e1 100644 --- a/orchestra/apps/services/tests/functional_tests/test_job.py +++ b/orchestra/apps/services/tests/functional_tests/test_job.py @@ -34,8 +34,9 @@ class JobBillingTest(BaseBillingTest): if not account: account = self.create_account() description = 'Random Job %s' % random_ascii(10) - service, __ = MiscService.objects.get_or_create(name='job', description=description, has_amount=True) - return Miscellaneous.objects.create(service=service, description=description, account=account, amount=amount) + service, __ = MiscService.objects.get_or_create(name='job', description=description, + has_amount=True) + return account.miscellaneous.create(service=service, description=description, amount=amount) def test_job(self): service = self.create_job_service() @@ -48,5 +49,3 @@ class JobBillingTest(BaseBillingTest): job = self.create_job(100, account=account) bill = account.orders.bill(new_open=True)[0] self.assertEqual(100*15, bill.get_total()) - - diff --git a/orchestra/apps/services/tests/functional_tests/test_mailbox.py b/orchestra/apps/services/tests/functional_tests/test_mailbox.py index db5ee338..141fb3a7 100644 --- a/orchestra/apps/services/tests/functional_tests/test_mailbox.py +++ b/orchestra/apps/services/tests/functional_tests/test_mailbox.py @@ -156,4 +156,3 @@ class MailboxBillingTest(BaseBillingTest): with freeze_time(now+relativedelta(months=6)): bills = service.orders.bill(new_open=True, **options) self.assertEqual([], bills) - diff --git a/orchestra/apps/services/tests/functional_tests/test_plan.py b/orchestra/apps/services/tests/functional_tests/test_plan.py index 79cac8cc..c0576030 100644 --- a/orchestra/apps/services/tests/functional_tests/test_plan.py +++ b/orchestra/apps/services/tests/functional_tests/test_plan.py @@ -1,8 +1,6 @@ from django.contrib.contenttypes.models import ContentType -from orchestra.apps.miscellaneous.models import MiscService, Miscellaneous - -from ...models import Service, Plan +from ...models import Service, Plan, ContractedPlan from . import BaseBillingTest @@ -11,8 +9,8 @@ class PlanBillingTest(BaseBillingTest): def create_plan_service(self): service = Service.objects.create( description="Association membership fee", - content_type=ContentType.objects.get_for_model(Miscellaneous), - match="account.is_active and account.type == 'ASSOCIATION'", + content_type=ContentType.objects.get_for_model(ContractedPlan), + match="contractedplan.plan.name == 'association_fee'", billing_period=Service.ANUAL, billing_point=Service.FIXED_DATE, is_fee=True, @@ -26,12 +24,28 @@ class PlanBillingTest(BaseBillingTest): ) return service - def create_plan(self): + def create_plan(self, account=None): if not account: account = self.create_account() - domain_name = '%s.es' % random_ascii(10) - domain_service, __ = MiscService.objects.get_or_create(name='domain .es', description='Domain .ES') - return Miscellaneous.objects.create(service=domain_service, description=domain_name, account=account) + plan, __ = Plan.objects.get_or_create(name='association_fee') + return plan.contracts.create(account=account) + + def test_update_orders(self): + account = self.create_account() + account1 = self.create_account() + self.create_plan(account=account) + self.create_plan(account=account1) + service = self.create_plan_service() + self.assertEqual(0, service.orders.count()) + service.update_orders() + self.assertEqual(2, service.orders.count()) def test_plan(self): - pass + account = self.create_account() + service = self.create_plan_service() + self.create_plan(account=account) + bill = account.orders.bill().pop() + self.assertEqual(bill.FEE, bill.type) + + +# TODO test price with multiple plans diff --git a/orchestra/apps/services/tests/functional_tests/test_traffic.py b/orchestra/apps/services/tests/functional_tests/test_traffic.py index b6dd17e2..d19f7dee 100644 --- a/orchestra/apps/services/tests/functional_tests/test_traffic.py +++ b/orchestra/apps/services/tests/functional_tests/test_traffic.py @@ -54,7 +54,8 @@ class BaseTrafficBillingTest(BaseBillingTest): def report_traffic(self, account, value): ct = ContentType.objects.get_for_model(Account) object_id = account.pk - MonitorData.objects.create(monitor='FTPTraffic', content_object=account.user, value=value, date=timezone.now()) + MonitorData.objects.create(monitor='FTPTraffic', content_object=account.user, + value=value, date=timezone.now()) data = ResourceData.get_or_create(account, self.resource) data.update() @@ -85,11 +86,20 @@ class TrafficBillingTest(BaseTrafficBillingTest): resource = self.create_traffic_resource() account1 = self.create_account() account2 = self.create_account() - # TODO + self.report_traffic(account1, 10**10) + self.report_traffic(account2, 10**10*5) + with freeze_time(timezone.now()+relativedelta(months=1)): + bill1 = account1.orders.bill().pop() + bill2 = account2.orders.bill().pop() + self.assertNotEqual(bill1.get_total(), bill2.get_total()) class TrafficPrepayBillingTest(BaseTrafficBillingTest): - METRIC = "max((account.resources.traffic.used or 0) - getattr(account.miscellaneous.filter(is_active=True, service__name='traffic prepay').last(), 'amount', 0), 0)" + METRIC = ("max(" + "(account.resources.traffic.used or 0) - " + "getattr(account.miscellaneous.filter(is_active=True, service__name='traffic prepay').last(), 'amount', 0)" + ", 0)" + ) def create_prepay_service(self): service = Service.objects.create( @@ -114,8 +124,9 @@ class TrafficPrepayBillingTest(BaseTrafficBillingTest): if not account: account = self.create_account() name = 'traffic prepay' - service, __ = MiscService.objects.get_or_create(name='traffic prepay', description='Traffic prepay', has_amount=True) - return Miscellaneous.objects.create(service=service, description=name, account=account, amount=amount) + service, __ = MiscService.objects.get_or_create(name='traffic prepay', + description='Traffic prepay', has_amount=True) + return account.miscellaneous.create(service=service, description=name, amount=amount) def test_traffic_prepay(self): service = self.create_traffic_service() diff --git a/orchestra/apps/services/tests/test_handler.py b/orchestra/apps/services/tests/test_handler.py index 827351a6..ea657f74 100644 --- a/orchestra/apps/services/tests/test_handler.py +++ b/orchestra/apps/services/tests/test_handler.py @@ -313,6 +313,47 @@ class HandlerTests(BaseTestCase): self.assertEqual(decimal.Decimal('9.00'), results[0].price) self.assertEqual(30, results[0].quantity) + def test_incomplete_rates(self): + service = self.create_ftp_service() + account = self.create_account() + superplan = Plan.objects.create( + name='SUPER', allow_multiple=False, is_combinable=True) + service.rates.create(plan=superplan, quantity=4, price=9) + service.rates.create(plan=superplan, quantity=10, price=1) + account.plans.create(plan=superplan) + results = service.get_rates(account, cache=False) + results = service.rate_method(results, 30) + rates = [ + {'price': decimal.Decimal('10.00'), 'quantity': 3}, + {'price': decimal.Decimal('9.00'), 'quantity': 6}, + {'price': decimal.Decimal('1.00'), 'quantity': 21} + ] + for rate, result in zip(rates, results): + self.assertEqual(rate['price'], result.price) + self.assertEqual(rate['quantity'], result.quantity) + + def test_zero_rates(self): + service = self.create_ftp_service() + account = self.create_account() + superplan = Plan.objects.create( + name='SUPER', allow_multiple=False, is_combinable=True) + service.rates.create(plan=superplan, quantity=0, price=0) + service.rates.create(plan=superplan, quantity=3, price=10) + service.rates.create(plan=superplan, quantity=4, price=9) + service.rates.create(plan=superplan, quantity=10, price=1) + account.plans.create(plan=superplan) + results = service.get_rates(account, cache=False) + results = service.rate_method(results, 30) + rates = [ + {'price': decimal.Decimal('0.00'), 'quantity': 2}, + {'price': decimal.Decimal('10.00'), 'quantity': 1}, + {'price': decimal.Decimal('9.00'), 'quantity': 6}, + {'price': decimal.Decimal('1.00'), 'quantity': 21} + ] + for rate, result in zip(rates, results): + self.assertEqual(rate['price'], result.price) + self.assertEqual(rate['quantity'], result.quantity) + def test_rates_allow_multiple(self): service = self.create_ftp_service() account = self.create_account() @@ -352,20 +393,3 @@ class HandlerTests(BaseTestCase): for rate, result in zip(rates, results): self.assertEqual(rate['price'], result.price) self.assertEqual(rate['quantity'], result.quantity) - - def test_generate_bill_lines_with_compensation(self): - service = self.create_ftp_service() - account = self.create_account() - now = timezone.now().date() - order = Order( - cancelled_on=now, - billed_until=now+relativedelta.relativedelta(years=2) - ) - order1 = Order() - orders = [order, order1] - lines = service.handler.generate_bill_lines(orders, account, commit=False) - print lines - print len(lines) - # TODO - - # TODO test incomplete rate 1 -> nominal_price 10 -> rate