From 5fa9a7a1b4c445f40afd4359d9d487c3545154df Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Mon, 1 Apr 2019 00:40:38 +0530 Subject: [PATCH 1/2] feat: unlink advance payment entry on cancelation of order --- .../accounts_settings/accounts_settings.json | 61 +++++++++++++++++-- .../doctype/sales_invoice/sales_invoice.py | 6 +- erpnext/controllers/accounts_controller.py | 11 ++++ erpnext/controllers/buying_controller.py | 2 + .../selling/doctype/quotation/quotation.py | 2 + .../doctype/sales_order/sales_order.py | 2 + .../doctype/delivery_note/delivery_note.py | 4 +- 7 files changed, 78 insertions(+), 10 deletions(-) diff --git a/erpnext/accounts/doctype/accounts_settings/accounts_settings.json b/erpnext/accounts/doctype/accounts_settings/accounts_settings.json index 945d5d47f5..6f280c406b 100644 --- a/erpnext/accounts/doctype/accounts_settings/accounts_settings.json +++ b/erpnext/accounts/doctype/accounts_settings/accounts_settings.json @@ -23,6 +23,7 @@ "columns": 0, "default": "1", "description": "If enabled, the system will post accounting entries for inventory automatically.", + "fetch_if_empty": 0, "fieldname": "auto_accounting_for_stock", "fieldtype": "Check", "hidden": 1, @@ -55,6 +56,7 @@ "collapsible": 0, "columns": 0, "description": "Accounting entry frozen up to this date, nobody can do / modify entry except role specified below.", + "fetch_if_empty": 0, "fieldname": "acc_frozen_upto", "fieldtype": "Date", "hidden": 0, @@ -87,6 +89,7 @@ "collapsible": 0, "columns": 0, "description": "Users with this role are allowed to set frozen accounts and create / modify accounting entries against frozen accounts", + "fetch_if_empty": 0, "fieldname": "frozen_accounts_modifier", "fieldtype": "Link", "hidden": 0, @@ -121,6 +124,7 @@ "columns": 0, "default": "Billing Address", "description": "Address used to determine Tax Category in transactions.", + "fetch_if_empty": 0, "fieldname": "determine_address_tax_category_from", "fieldtype": "Select", "hidden": 0, @@ -154,6 +158,7 @@ "bold": 0, "collapsible": 0, "columns": 0, + "fetch_if_empty": 0, "fieldname": "column_break_4", "fieldtype": "Column Break", "hidden": 0, @@ -186,6 +191,7 @@ "collapsible": 0, "columns": 0, "description": "Role that is allowed to submit transactions that exceed credit limits set.", + "fetch_if_empty": 0, "fieldname": "credit_controller", "fieldtype": "Link", "hidden": 0, @@ -218,6 +224,7 @@ "bold": 0, "collapsible": 0, "columns": 0, + "fetch_if_empty": 0, "fieldname": "check_supplier_invoice_uniqueness", "fieldtype": "Check", "hidden": 0, @@ -250,6 +257,7 @@ "bold": 0, "collapsible": 0, "columns": 0, + "fetch_if_empty": 0, "fieldname": "make_payment_via_journal_entry", "fieldtype": "Check", "hidden": 0, @@ -283,6 +291,7 @@ "collapsible": 0, "columns": 0, "default": "1", + "fetch_if_empty": 0, "fieldname": "unlink_payment_on_cancellation_of_invoice", "fieldtype": "Check", "hidden": 0, @@ -308,6 +317,39 @@ "translatable": 0, "unique": 0 }, + { + "allow_bulk_edit": 0, + "allow_in_quick_entry": 0, + "allow_on_submit": 0, + "bold": 0, + "collapsible": 0, + "columns": 0, + "fetch_if_empty": 0, + "fieldname": "unlink_advance_payment_on_cancellation_of_order", + "fieldtype": "Check", + "hidden": 0, + "ignore_user_permissions": 0, + "ignore_xss_filter": 0, + "in_filter": 0, + "in_global_search": 0, + "in_list_view": 0, + "in_standard_filter": 0, + "label": "Unlink Advance Payment on Cancellation of Order", + "length": 0, + "no_copy": 0, + "permlevel": 0, + "precision": "", + "print_hide": 0, + "print_hide_if_no_value": 0, + "read_only": 0, + "remember_last_selected_value": 0, + "report_hide": 0, + "reqd": 0, + "search_index": 0, + "set_only_once": 0, + "translatable": 0, + "unique": 0 + }, { "allow_bulk_edit": 0, "allow_in_quick_entry": 0, @@ -316,6 +358,7 @@ "collapsible": 0, "columns": 0, "default": "1", + "fetch_if_empty": 0, "fieldname": "book_asset_depreciation_entry_automatically", "fieldtype": "Check", "hidden": 0, @@ -348,6 +391,7 @@ "bold": 0, "collapsible": 0, "columns": 0, + "fetch_if_empty": 0, "fieldname": "allow_cost_center_in_entry_of_bs_account", "fieldtype": "Check", "hidden": 0, @@ -381,6 +425,7 @@ "collapsible": 0, "columns": 0, "default": "1", + "fetch_if_empty": 0, "fieldname": "add_taxes_from_item_tax_template", "fieldtype": "Check", "hidden": 0, @@ -413,6 +458,7 @@ "bold": 0, "collapsible": 0, "columns": 0, + "fetch_if_empty": 0, "fieldname": "print_settings", "fieldtype": "Section Break", "hidden": 0, @@ -445,6 +491,7 @@ "bold": 0, "collapsible": 0, "columns": 0, + "fetch_if_empty": 0, "fieldname": "show_inclusive_tax_in_print", "fieldtype": "Check", "hidden": 0, @@ -477,6 +524,7 @@ "bold": 0, "collapsible": 0, "columns": 0, + "fetch_if_empty": 0, "fieldname": "column_break_12", "fieldtype": "Column Break", "hidden": 0, @@ -508,6 +556,7 @@ "bold": 0, "collapsible": 0, "columns": 0, + "fetch_if_empty": 0, "fieldname": "show_payment_schedule_in_print", "fieldtype": "Check", "hidden": 0, @@ -540,6 +589,7 @@ "bold": 0, "collapsible": 0, "columns": 0, + "fetch_if_empty": 0, "fieldname": "currency_exchange_section", "fieldtype": "Section Break", "hidden": 0, @@ -573,6 +623,7 @@ "collapsible": 0, "columns": 0, "default": "1", + "fetch_if_empty": 0, "fieldname": "allow_stale", "fieldtype": "Check", "hidden": 0, @@ -607,6 +658,7 @@ "columns": 0, "default": "1", "depends_on": "eval:doc.allow_stale==0", + "fetch_if_empty": 0, "fieldname": "stale_days", "fieldtype": "Int", "hidden": 0, @@ -639,6 +691,7 @@ "bold": 0, "collapsible": 0, "columns": 0, + "fetch_if_empty": 0, "fieldname": "report_settings_sb", "fieldtype": "Section Break", "hidden": 0, @@ -673,6 +726,7 @@ "columns": 0, "default": "0", "description": "Only select if you have setup Cash Flow Mapper documents", + "fetch_if_empty": 0, "fieldname": "use_custom_cash_flow", "fieldtype": "Check", "hidden": 0, @@ -700,17 +754,15 @@ } ], "has_web_view": 0, - "hide_heading": 0, "hide_toolbar": 0, "icon": "icon-cog", "idx": 1, - "image_view": 0, "in_create": 0, "is_submittable": 0, "issingle": 1, "istable": 0, "max_attachments": 0, - "modified": "2019-01-07 00:42:34.510150", + "modified": "2019-04-01 00:22:18.482570", "modified_by": "Administrator", "module": "Accounts", "name": "Accounts Settings", @@ -776,10 +828,9 @@ ], "quick_entry": 1, "read_only": 0, - "read_only_onload": 0, "show_name_in_global_search": 0, "sort_order": "ASC", "track_changes": 1, "track_seen": 0, "track_views": 0 -} +} \ No newline at end of file diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py index 09b1da48ee..94d0fc331b 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py @@ -207,11 +207,9 @@ class SalesInvoice(SellingController): def on_cancel(self): - self.check_close_sales_order("sales_order") + super(SalesInvoice, self).on_cancel() - from erpnext.accounts.utils import unlink_ref_doc_from_payment_entries - if frappe.db.get_single_value('Accounts Settings', 'unlink_payment_on_cancellation_of_invoice'): - unlink_ref_doc_from_payment_entries(self) + self.check_close_sales_order("sales_order") if self.is_return and not self.update_billed_amount_in_sales_order: # NOTE status updating bypassed for is_return diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 27c31eb961..28757c5cd6 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -552,6 +552,17 @@ class AccountsController(TransactionBase): from erpnext.accounts.utils import reconcile_against_document reconcile_against_document(lst) + def on_cancel(self): + from erpnext.accounts.utils import unlink_ref_doc_from_payment_entries + + if self.doctype in ["Sales Invoice", "Purchase Invoice"]: + if frappe.db.get_single_value('Accounts Settings', 'unlink_payment_on_cancellation_of_invoice'): + unlink_ref_doc_from_payment_entries(self) + + elif self.doctype in ["Sales Order", "Purchase Order"]: + if frappe.db.get_single_value('Accounts Settings', 'unlink_advance_payment_on_cancellation_of_order'): + unlink_ref_doc_from_payment_entries(self) + def validate_multiple_billing(self, ref_dt, item_ref_dn, based_on, parentfield): from erpnext.controllers.status_updater import get_tolerance_for item_tolerance = {} diff --git a/erpnext/controllers/buying_controller.py b/erpnext/controllers/buying_controller.py index a1dfffe611..2dd1e61a5d 100644 --- a/erpnext/controllers/buying_controller.py +++ b/erpnext/controllers/buying_controller.py @@ -531,6 +531,8 @@ class BuyingController(StockController): update_last_purchase_rate(self, is_submit = 1) def on_cancel(self): + super(BuyingController, self).on_cancel() + if self.get('is_return'): return diff --git a/erpnext/selling/doctype/quotation/quotation.py b/erpnext/selling/doctype/quotation/quotation.py index c85065cf12..ad151bef00 100644 --- a/erpnext/selling/doctype/quotation/quotation.py +++ b/erpnext/selling/doctype/quotation/quotation.py @@ -97,6 +97,8 @@ class Quotation(SellingController): self.update_lead() def on_cancel(self): + super(Quotation, self).on_cancel() + #update enquiry status self.set_status(update=True) self.update_opportunity() diff --git a/erpnext/selling/doctype/sales_order/sales_order.py b/erpnext/selling/doctype/sales_order/sales_order.py index 1ad4e9b864..7eab352fc0 100755 --- a/erpnext/selling/doctype/sales_order/sales_order.py +++ b/erpnext/selling/doctype/sales_order/sales_order.py @@ -183,6 +183,8 @@ class SalesOrder(SellingController): self.update_blanket_order() def on_cancel(self): + super(SalesOrder, self).on_cancel() + # Cannot cancel closed SO if self.status == 'Closed': frappe.throw(_("Closed order cannot be cancelled. Unclose to cancel.")) diff --git a/erpnext/stock/doctype/delivery_note/delivery_note.py b/erpnext/stock/doctype/delivery_note/delivery_note.py index 869b614f7f..0b61240035 100644 --- a/erpnext/stock/doctype/delivery_note/delivery_note.py +++ b/erpnext/stock/doctype/delivery_note/delivery_note.py @@ -177,7 +177,7 @@ class DeliveryNote(SellingController): frappe.msgprint(_("Note: Item {0} entered multiple times").format(d.item_code)) else: chk_dupl_itm.append(f) - #Customer Provided parts will have zero valuation rate + #Customer Provided parts will have zero valuation rate if frappe.db.get_value('Item', d.item_code, 'is_customer_provided_item'): d.allow_zero_valuation_rate = 1 @@ -223,6 +223,8 @@ class DeliveryNote(SellingController): self.make_gl_entries() def on_cancel(self): + super(DeliveryNote, self).on_cancel() + self.check_close_sales_order("against_sales_order") self.check_next_docstatus() From 9835c89e17287f7949ed2ba4d5f2ad811238c762 Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Mon, 1 Apr 2019 01:02:06 +0530 Subject: [PATCH 2/2] Added test cases --- .../accounts_settings/accounts_settings.json | 7 ++--- .../purchase_invoice/purchase_invoice.py | 4 --- .../purchase_order/test_purchase_order.py | 27 +++++++++++++++++++ erpnext/controllers/accounts_controller.py | 4 ++- .../doctype/sales_order/test_sales_order.py | 25 +++++++++++++++-- 5 files changed, 57 insertions(+), 10 deletions(-) diff --git a/erpnext/accounts/doctype/accounts_settings/accounts_settings.json b/erpnext/accounts/doctype/accounts_settings/accounts_settings.json index 6f280c406b..be5407b926 100644 --- a/erpnext/accounts/doctype/accounts_settings/accounts_settings.json +++ b/erpnext/accounts/doctype/accounts_settings/accounts_settings.json @@ -324,8 +324,9 @@ "bold": 0, "collapsible": 0, "columns": 0, + "default": "1", "fetch_if_empty": 0, - "fieldname": "unlink_advance_payment_on_cancellation_of_order", + "fieldname": "unlink_advance_payment_on_cancelation_of_order", "fieldtype": "Check", "hidden": 0, "ignore_user_permissions": 0, @@ -334,7 +335,7 @@ "in_global_search": 0, "in_list_view": 0, "in_standard_filter": 0, - "label": "Unlink Advance Payment on Cancellation of Order", + "label": "Unlink Advance Payment on Cancelation of Order", "length": 0, "no_copy": 0, "permlevel": 0, @@ -762,7 +763,7 @@ "issingle": 1, "istable": 0, "max_attachments": 0, - "modified": "2019-04-01 00:22:18.482570", + "modified": "2019-04-06 12:28:43.026250", "modified_by": "Administrator", "module": "Accounts", "name": "Accounts Settings", diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py index 450f2d0eb7..896eab8a4a 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py @@ -765,10 +765,6 @@ class PurchaseInvoice(BuyingController): self.update_status_updater_args() if not self.is_return: - from erpnext.accounts.utils import unlink_ref_doc_from_payment_entries - if frappe.db.get_single_value('Accounts Settings', 'unlink_payment_on_cancellation_of_invoice'): - unlink_ref_doc_from_payment_entries(self) - self.update_prevdoc_status() self.update_billing_status_for_zero_amount_refdoc("Purchase Order") self.update_billing_status_in_pr() diff --git a/erpnext/buying/doctype/purchase_order/test_purchase_order.py b/erpnext/buying/doctype/purchase_order/test_purchase_order.py index f1507364df..774156eda4 100644 --- a/erpnext/buying/doctype/purchase_order/test_purchase_order.py +++ b/erpnext/buying/doctype/purchase_order/test_purchase_order.py @@ -465,6 +465,33 @@ class TestPurchaseOrder(unittest.TestCase): self.assertEquals(se_items, supplied_items) update_backflush_based_on("BOM") + def test_advance_payment_entry_unlink_against_purchase_order(self): + from erpnext.accounts.doctype.payment_entry.test_payment_entry import get_payment_entry + frappe.db.set_value("Accounts Settings", "Accounts Settings", + "unlink_advance_payment_on_cancelation_of_order", 1) + + po_doc = create_purchase_order() + + pe = get_payment_entry("Purchase Order", po_doc.name, bank_account="_Test Bank - _TC") + pe.reference_no = "1" + pe.reference_date = nowdate() + pe.paid_from_account_currency = po_doc.currency + pe.paid_to_account_currency = po_doc.currency + pe.source_exchange_rate = 1 + pe.target_exchange_rate = 1 + pe.paid_amount = po_doc.grand_total + pe.save(ignore_permissions=True) + pe.submit() + + po_doc = frappe.get_doc('Purchase Order', po_doc.name) + po_doc.cancel() + + pe_doc = frappe.get_doc('Payment Entry', pe.name) + pe_doc.cancel() + + frappe.db.set_value("Accounts Settings", "Accounts Settings", + "unlink_advance_payment_on_cancelation_of_order", 0) + def make_subcontracted_item(item_code): from erpnext.manufacturing.doctype.production_plan.test_production_plan import make_bom diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 28757c5cd6..7bb71ec9b5 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -556,11 +556,13 @@ class AccountsController(TransactionBase): from erpnext.accounts.utils import unlink_ref_doc_from_payment_entries if self.doctype in ["Sales Invoice", "Purchase Invoice"]: + if self.is_return: return + if frappe.db.get_single_value('Accounts Settings', 'unlink_payment_on_cancellation_of_invoice'): unlink_ref_doc_from_payment_entries(self) elif self.doctype in ["Sales Order", "Purchase Order"]: - if frappe.db.get_single_value('Accounts Settings', 'unlink_advance_payment_on_cancellation_of_order'): + if frappe.db.get_single_value('Accounts Settings', 'unlink_advance_payment_on_cancelation_of_order'): unlink_ref_doc_from_payment_entries(self) def validate_multiple_billing(self, ref_dt, item_ref_dn, based_on, parentfield): diff --git a/erpnext/selling/doctype/sales_order/test_sales_order.py b/erpnext/selling/doctype/sales_order/test_sales_order.py index 766e4e6ed0..3a1383f6c8 100644 --- a/erpnext/selling/doctype/sales_order/test_sales_order.py +++ b/erpnext/selling/doctype/sales_order/test_sales_order.py @@ -2,7 +2,7 @@ # License: GNU General Public License v3. See license.txt from __future__ import unicode_literals import frappe -from frappe.utils import flt, add_days +from frappe.utils import flt, add_days, nowdate import frappe.permissions import unittest from erpnext.selling.doctype.sales_order.sales_order \ @@ -13,7 +13,6 @@ from erpnext.controllers.accounts_controller import update_child_qty_rate import json from erpnext.selling.doctype.sales_order.sales_order import make_raw_material_request - class TestSalesOrder(unittest.TestCase): def tearDown(self): frappe.set_user("Administrator") @@ -703,6 +702,28 @@ class TestSalesOrder(unittest.TestCase): se.cancel() self.assertFalse(frappe.db.exists("Serial No", {"sales_order": so.name})) + def test_advance_payment_entry_unlink_against_sales_order(self): + from erpnext.accounts.doctype.payment_entry.test_payment_entry import get_payment_entry + frappe.db.set_value("Accounts Settings", "Accounts Settings", + "unlink_advance_payment_on_cancelation_of_order", 0) + + so = make_sales_order() + + pe = get_payment_entry("Sales Order", so.name, bank_account="_Test Bank - _TC") + pe.reference_no = "1" + pe.reference_date = nowdate() + pe.paid_from_account_currency = so.currency + pe.paid_to_account_currency = so.currency + pe.source_exchange_rate = 1 + pe.target_exchange_rate = 1 + pe.paid_amount = so.grand_total + pe.save(ignore_permissions=True) + pe.submit() + + so_doc = frappe.get_doc('Sales Order', so.name) + + self.assertRaises(frappe.LinkExistsError, so_doc.cancel) + def test_request_for_raw_materials(self): from erpnext.stock.doctype.item.test_item import make_item item = make_item("_Test Finished Item", {"is_stock_item": 1,