From 618e737b0bf0e243736f439c14404be5024079d0 Mon Sep 17 00:00:00 2001 From: David Date: Thu, 5 Sep 2024 14:07:37 +0200 Subject: [PATCH 1/6] fix: add lp collection min to list --- .../loyalty_program_collection/loyalty_program_collection.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/loyalty_program_collection/loyalty_program_collection.json b/erpnext/accounts/doctype/loyalty_program_collection/loyalty_program_collection.json index f5fc26d7d30..3c1d73d120d 100644 --- a/erpnext/accounts/doctype/loyalty_program_collection/loyalty_program_collection.json +++ b/erpnext/accounts/doctype/loyalty_program_collection/loyalty_program_collection.json @@ -20,6 +20,7 @@ { "fieldname": "min_spent", "fieldtype": "Currency", + "in_list_view": 1, "label": "Minimum Total Spent" }, { @@ -37,7 +38,7 @@ ], "istable": 1, "links": [], - "modified": "2024-03-27 13:10:03.536071", + "modified": "2024-09-05 07:41:25.694041", "modified_by": "Administrator", "module": "Accounts", "name": "Loyalty Program Collection", From 81dc5872c4509be228ee6804c62bea87ed530aba Mon Sep 17 00:00:00 2001 From: David Date: Tue, 21 May 2024 07:50:25 +0200 Subject: [PATCH 2/6] fix: tiered loyalty program --- erpnext/accounts/doctype/loyalty_program/loyalty_program.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/erpnext/accounts/doctype/loyalty_program/loyalty_program.py b/erpnext/accounts/doctype/loyalty_program/loyalty_program.py index f3ad84bf6d3..9a2b1c113a6 100644 --- a/erpnext/accounts/doctype/loyalty_program/loyalty_program.py +++ b/erpnext/accounts/doctype/loyalty_program/loyalty_program.py @@ -86,10 +86,9 @@ def get_loyalty_program_details_with_points( tier_spent_level = sorted( [d.as_dict() for d in loyalty_program.collection_rules], key=lambda rule: rule.min_spent, - reverse=True, ) for i, d in enumerate(tier_spent_level): - if i == 0 or (lp_details.total_spent + current_transaction_amount) <= d.min_spent: + if i == 0 or (lp_details.total_spent + current_transaction_amount) >= d.min_spent: lp_details.tier_name = d.tier_name lp_details.collection_factor = d.collection_factor else: From c914ea4adc2a4d66c1091b4bb3579d03cf05164d Mon Sep 17 00:00:00 2001 From: David Date: Thu, 5 Sep 2024 12:32:27 +0200 Subject: [PATCH 3/6] test: add tests for loyalty tier selection --- .../loyalty_program/loyalty_program.py | 5 +- .../loyalty_program/test_loyalty_program.py | 65 +++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/doctype/loyalty_program/loyalty_program.py b/erpnext/accounts/doctype/loyalty_program/loyalty_program.py index 9a2b1c113a6..240866df8de 100644 --- a/erpnext/accounts/doctype/loyalty_program/loyalty_program.py +++ b/erpnext/accounts/doctype/loyalty_program/loyalty_program.py @@ -79,9 +79,10 @@ def get_loyalty_program_details_with_points( ): lp_details = get_loyalty_program_details(customer, loyalty_program, company=company, silent=silent) loyalty_program = frappe.get_doc("Loyalty Program", loyalty_program) - lp_details.update( - get_loyalty_details(customer, loyalty_program.name, expiry_date, company, include_expired_entry) + loyalty_details = get_loyalty_details( + customer, loyalty_program.name, expiry_date, company, include_expired_entry ) + lp_details.update(loyalty_details) tier_spent_level = sorted( [d.as_dict() for d in loyalty_program.collection_rules], diff --git a/erpnext/accounts/doctype/loyalty_program/test_loyalty_program.py b/erpnext/accounts/doctype/loyalty_program/test_loyalty_program.py index 4d21fb69806..a79109bcd2b 100644 --- a/erpnext/accounts/doctype/loyalty_program/test_loyalty_program.py +++ b/erpnext/accounts/doctype/loyalty_program/test_loyalty_program.py @@ -7,6 +7,7 @@ import frappe from frappe.utils import cint, flt, getdate, today from erpnext.accounts.doctype.loyalty_program.loyalty_program import ( + get_loyalty_details, get_loyalty_program_details_with_points, ) from erpnext.accounts.party import get_dashboard_info @@ -197,6 +198,70 @@ class TestLoyaltyProgram(unittest.TestCase): for d in company_wise_info: self.assertTrue(d.get("loyalty_points")) + @unittest.mock.patch("erpnext.accounts.doctype.loyalty_program.loyalty_program.get_loyalty_details") + def test_tier_selection(self, mock_get_loyalty_details): + # Create a new loyalty program with multiple tiers + loyalty_program = frappe.get_doc( + { + "doctype": "Loyalty Program", + "loyalty_program_name": "Test Tier Selection", + "auto_opt_in": 1, + "from_date": today(), + "loyalty_program_type": "Multiple Tier Program", + "conversion_factor": 1, + "expiry_duration": 10, + "company": "_Test Company", + "cost_center": "Main - _TC", + "expense_account": "Loyalty - _TC", + "collection_rules": [ + {"tier_name": "Gold", "collection_factor": 1000, "min_spent": 20000}, + {"tier_name": "Silver", "collection_factor": 1000, "min_spent": 10000}, + {"tier_name": "Bronze", "collection_factor": 1000, "min_spent": 5000}, + ], + } + ) + loyalty_program.insert() + + # Test cases with different total_spent and current_transaction_amount combinations + test_cases = [ + (0, 6000, "Bronze"), + (0, 15000, "Silver"), + (0, 25000, "Gold"), + (4000, 500, "Bronze"), + (8000, 3000, "Silver"), + (18000, 3000, "Gold"), + (22000, 5000, "Gold"), + ] + + for total_spent, current_transaction_amount, expected_tier in test_cases: + with self.subTest(total_spent=total_spent, current_transaction_amount=current_transaction_amount): + # Mock the get_loyalty_details function to update the total_spent + def side_effect(*args, **kwargs): + result = get_loyalty_details(*args, **kwargs) + result.update({"total_spent": total_spent}) + return result + + mock_get_loyalty_details.side_effect = side_effect + + lp_details = get_loyalty_program_details_with_points( + "Test Loyalty Customer", + loyalty_program=loyalty_program.name, + company="_Test Company", + current_transaction_amount=current_transaction_amount, + ) + + # Get the selected tier based on the current implementation + selected_tier = lp_details.tier_name + + self.assertEqual( + selected_tier, + expected_tier, + f"Expected tier {expected_tier} for total_spent {total_spent} and current_transaction_amount {current_transaction_amount}, but got {selected_tier}", + ) + + # Clean up + loyalty_program.delete() + def get_points_earned(self): def get_returned_amount(): From ad02e4112fa8bb3dd3b880f6dbd0d5ee4897f247 Mon Sep 17 00:00:00 2001 From: David Date: Thu, 5 Sep 2024 14:07:06 +0200 Subject: [PATCH 4/6] fix: loyalty test case -> validate against actual database state --- .../accounts/doctype/loyalty_program/test_loyalty_program.py | 3 ++- erpnext/accounts/doctype/sales_invoice/sales_invoice.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/doctype/loyalty_program/test_loyalty_program.py b/erpnext/accounts/doctype/loyalty_program/test_loyalty_program.py index a79109bcd2b..64adce9ec15 100644 --- a/erpnext/accounts/doctype/loyalty_program/test_loyalty_program.py +++ b/erpnext/accounts/doctype/loyalty_program/test_loyalty_program.py @@ -80,6 +80,7 @@ class TestLoyaltyProgram(unittest.TestCase): si_original = create_sales_invoice_record() si_original.insert() si_original.submit() + customer.reload() earned_points = get_points_earned(si_original) @@ -102,8 +103,8 @@ class TestLoyaltyProgram(unittest.TestCase): si_redeem.loyalty_points = earned_points si_redeem.insert() si_redeem.submit() + customer.reload() - customer = frappe.get_doc("Customer", {"customer_name": "Test Loyalty Customer"}) earned_after_redemption = get_points_earned(si_redeem) lpe_redeem = frappe.get_doc( diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py index 978c1ad021c..6e8f1b2bbf2 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py @@ -1776,7 +1776,8 @@ class SalesInvoice(SellingController): loyalty_program=self.loyalty_program, include_expired_entry=True, ) - frappe.db.set_value("Customer", self.customer, "loyalty_program_tier", lp_details.tier_name) + customer = frappe.get_doc("Customer", self.customer) + customer.db_set("loyalty_program_tier", lp_details.tier_name) def get_returned_amount(self): from frappe.query_builder.functions import Sum From 4bd26b845e0fcc727cff2b2228b349c6445b0edb Mon Sep 17 00:00:00 2001 From: David Date: Thu, 5 Sep 2024 14:29:06 +0200 Subject: [PATCH 5/6] fix: test ensures min spent respected --- .../accounts/doctype/loyalty_program/test_loyalty_program.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/erpnext/accounts/doctype/loyalty_program/test_loyalty_program.py b/erpnext/accounts/doctype/loyalty_program/test_loyalty_program.py index 64adce9ec15..af8b38cca91 100644 --- a/erpnext/accounts/doctype/loyalty_program/test_loyalty_program.py +++ b/erpnext/accounts/doctype/loyalty_program/test_loyalty_program.py @@ -39,6 +39,9 @@ class TestLoyaltyProgram(unittest.TestCase): ) self.assertEqual(si_original.get("loyalty_program"), customer.loyalty_program) + self.assertEqual( + lpe.get("loyalty_program_tier"), None + ) # 10000 does not surpass the first tier (11000) self.assertEqual(lpe.get("loyalty_program_tier"), customer.loyalty_program_tier) self.assertEqual(lpe.loyalty_points, earned_points) From efd8f1e978ddad5dd5011b43d5832ac4d041775b Mon Sep 17 00:00:00 2001 From: "David (aider)" Date: Thu, 5 Sep 2024 12:46:09 +0200 Subject: [PATCH 6/6] feat: Add validation to ensure lowest tier has min_spent of 0 in LoyaltyProgram --- .../doctype/loyalty_program/loyalty_program.py | 12 +++++++++++- .../doctype/loyalty_program/test_loyalty_program.py | 9 ++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/erpnext/accounts/doctype/loyalty_program/loyalty_program.py b/erpnext/accounts/doctype/loyalty_program/loyalty_program.py index 240866df8de..6bf08e04e52 100644 --- a/erpnext/accounts/doctype/loyalty_program/loyalty_program.py +++ b/erpnext/accounts/doctype/loyalty_program/loyalty_program.py @@ -36,7 +36,17 @@ class LoyaltyProgram(Document): to_date: DF.Date | None # end: auto-generated types - pass + def validate(self): + self.validate_lowest_tier() + + def validate_lowest_tier(self): + tiers = sorted(self.collection_rules, key=lambda x: x.min_spent) + if tiers and tiers[0].min_spent != 0: + frappe.throw( + _( + "The lowest tier must have a minimum spent amount of 0. Customers need to be part of a tier as soon as they are enrolled in the program." + ) + ) def get_loyalty_details( diff --git a/erpnext/accounts/doctype/loyalty_program/test_loyalty_program.py b/erpnext/accounts/doctype/loyalty_program/test_loyalty_program.py index af8b38cca91..322ece836f0 100644 --- a/erpnext/accounts/doctype/loyalty_program/test_loyalty_program.py +++ b/erpnext/accounts/doctype/loyalty_program/test_loyalty_program.py @@ -39,9 +39,7 @@ class TestLoyaltyProgram(unittest.TestCase): ) self.assertEqual(si_original.get("loyalty_program"), customer.loyalty_program) - self.assertEqual( - lpe.get("loyalty_program_tier"), None - ) # 10000 does not surpass the first tier (11000) + self.assertEqual(lpe.get("loyalty_program_tier"), "Bronce") # is always in the first tier self.assertEqual(lpe.get("loyalty_program_tier"), customer.loyalty_program_tier) self.assertEqual(lpe.loyalty_points, earned_points) @@ -220,7 +218,7 @@ class TestLoyaltyProgram(unittest.TestCase): "collection_rules": [ {"tier_name": "Gold", "collection_factor": 1000, "min_spent": 20000}, {"tier_name": "Silver", "collection_factor": 1000, "min_spent": 10000}, - {"tier_name": "Bronze", "collection_factor": 1000, "min_spent": 5000}, + {"tier_name": "Bronze", "collection_factor": 1000, "min_spent": 0}, ], } ) @@ -354,7 +352,7 @@ def create_records(): "company": "_Test Company", "cost_center": "Main - _TC", "expense_account": "Loyalty - _TC", - "collection_rules": [{"tier_name": "Silver", "collection_factor": 1000, "min_spent": 1000}], + "collection_rules": [{"tier_name": "Bronce", "collection_factor": 1000, "min_spent": 0}], } ).insert() @@ -385,6 +383,7 @@ def create_records(): "cost_center": "Main - _TC", "expense_account": "Loyalty - _TC", "collection_rules": [ + {"tier_name": "Bronze", "collection_factor": 1000, "min_spent": 0}, {"tier_name": "Silver", "collection_factor": 1000, "min_spent": 10000}, {"tier_name": "Gold", "collection_factor": 1000, "min_spent": 19000}, ],