From 024c28acf752c790e70fd9bb5f28c54b4c789635 Mon Sep 17 00:00:00 2001 From: tundebabzy Date: Sat, 3 Mar 2018 10:32:02 +0100 Subject: [PATCH] fix proration logic to reduce possibility of rounding errors --- .../doctype/subscriptions/subscriptions.py | 24 ++++++++------ .../subscriptions/test_subscriptions.py | 32 ++++++++++++------- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/erpnext/accounts/doctype/subscriptions/subscriptions.py b/erpnext/accounts/doctype/subscriptions/subscriptions.py index 7796daa8e3..15e4611149 100644 --- a/erpnext/accounts/doctype/subscriptions/subscriptions.py +++ b/erpnext/accounts/doctype/subscriptions/subscriptions.py @@ -7,7 +7,7 @@ from __future__ import unicode_literals import frappe from frappe import _ from frappe.model.document import Document -from frappe.utils.data import nowdate, getdate, cint, add_days, date_diff, get_last_day, add_to_date +from frappe.utils.data import nowdate, getdate, cint, add_days, date_diff, get_last_day, add_to_date, flt class Subscriptions(Document): @@ -304,7 +304,7 @@ class Subscriptions(Document): ) elif plan_items: - prorate_factor = get_prorata_factor(self.current_invoice_end, self.current_invoice_start) + prorate_factor = self.get_prorata_factor(self.current_invoice_end, self.current_invoice_start) item_names = frappe.db.sql( 'select item as item_code, cost * %s as rate from `tabSubscription Plan` where name in %s', @@ -420,6 +420,18 @@ class Subscriptions(Document): else: frappe.throw(_('You cannot restart a Subscription that is not cancelled.')) + def get_prorata_factor(self, period_end, period_start): + diff = flt(date_diff(nowdate(), period_start) + 1) + plan_days = flt(date_diff(period_end, period_start) + 1) + prorate_factor = diff / plan_days + + return prorate_factor + + def get_precision(self): + invoice = self.get_current_invoice() + if invoice: + return invoice.precision('grand_total') + def process_all(): """ @@ -456,14 +468,6 @@ def process(data): frappe.db.commit() -def get_prorata_factor(period_end, period_start): - diff = date_diff(nowdate(), period_start) + 1 - plan_days = date_diff(period_end, period_start) + 1 - prorate_factor = diff/plan_days - - return prorate_factor - - @frappe.whitelist() def cancel_subscription(name): """ diff --git a/erpnext/accounts/doctype/subscriptions/test_subscriptions.py b/erpnext/accounts/doctype/subscriptions/test_subscriptions.py index 4452e631fb..fb92d8626f 100644 --- a/erpnext/accounts/doctype/subscriptions/test_subscriptions.py +++ b/erpnext/accounts/doctype/subscriptions/test_subscriptions.py @@ -6,8 +6,7 @@ from __future__ import unicode_literals import unittest import frappe -from erpnext.accounts.doctype.subscriptions.subscriptions import get_prorata_factor -from frappe.utils.data import nowdate, add_days, add_to_date, add_months, date_diff +from frappe.utils.data import nowdate, add_days, add_to_date, add_months, date_diff, flt def create_plan(): @@ -275,6 +274,11 @@ class TestSubscriptions(unittest.TestCase): subscription.delete() def test_subscription_cancellation_invoices(self): + settings = frappe.get_single('Subscription Settings') + to_prorate = settings.prorate + settings.prorate = 1 + settings.save() + subscription = frappe.new_doc('Subscriptions') subscription.subscriber = '_Test Customer' subscription.append('plans', {'plan': '_Test Plan Name'}) @@ -287,18 +291,22 @@ class TestSubscriptions(unittest.TestCase): self.assertEqual(len(subscription.invoices), 1) invoice = subscription.get_current_invoice() - diff = date_diff(nowdate(), subscription.current_invoice_start) + 1 - plan_days = date_diff(subscription.current_invoice_end, subscription.current_invoice_start) + 1 - prorate_factor = diff/plan_days + diff = flt(date_diff(nowdate(), subscription.current_invoice_start) + 1) + plan_days = flt(date_diff(subscription.current_invoice_end, subscription.current_invoice_start) + 1) + prorate_factor = flt(diff/plan_days) self.assertEqual( - get_prorata_factor(subscription.current_invoice_end, subscription.current_invoice_start), - prorate_factor + flt( + subscription.get_prorata_factor(subscription.current_invoice_end, subscription.current_invoice_start), + 2), + flt(prorate_factor, 2) ) - self.assertEqual(invoice.grand_total, prorate_factor * 900) + self.assertEqual(flt(invoice.grand_total, 2), flt(prorate_factor * 900, 2)) self.assertEqual(subscription.status, 'Canceled') subscription.delete() + settings.prorate = to_prorate + settings.save() def test_subscription_cancellation_invoices_with_prorata_false(self): settings = frappe.get_single('Subscription Settings') @@ -333,11 +341,11 @@ class TestSubscriptions(unittest.TestCase): subscription.cancel_subscription() invoice = subscription.get_current_invoice() - diff = date_diff(nowdate(), subscription.current_invoice_start) + 1 - plan_days = date_diff(subscription.current_invoice_end, subscription.current_invoice_start) + 1 - prorate_factor = diff/plan_days + diff = flt(date_diff(nowdate(), subscription.current_invoice_start) + 1) + plan_days = flt(date_diff(subscription.current_invoice_end, subscription.current_invoice_start) + 1) + prorate_factor = flt(diff / plan_days) - self.assertEqual(invoice.grand_total, prorate_factor * 900) + self.assertEqual(flt(invoice.grand_total, 2), flt(prorate_factor * 900, 2)) settings.prorate = to_prorate settings.save()