mirror of
https://github.com/frappe/erpnext.git
synced 2026-06-05 21:29:11 +00:00
fix: prevent leakage of party-derived fields in cross doctype transactions (#55336)
Co-authored-by: Nabin Hait <nabinhait@gmail.com>
This commit is contained in:
@@ -29,7 +29,12 @@ from erpnext.accounts.doctype.repost_accounting_ledger.repost_accounting_ledger
|
|||||||
)
|
)
|
||||||
from erpnext.accounts.doctype.tax_withholding_entry.tax_withholding_entry import SalesTaxWithholding
|
from erpnext.accounts.doctype.tax_withholding_entry.tax_withholding_entry import SalesTaxWithholding
|
||||||
from erpnext.accounts.general_ledger import get_round_off_account_and_cost_center
|
from erpnext.accounts.general_ledger import get_round_off_account_and_cost_center
|
||||||
from erpnext.accounts.party import _get_party_details, get_due_date, get_party_account
|
from erpnext.accounts.party import (
|
||||||
|
CROSS_PARTY_FIELD_NO_MAP,
|
||||||
|
_get_party_details,
|
||||||
|
get_due_date,
|
||||||
|
get_party_account,
|
||||||
|
)
|
||||||
from erpnext.accounts.utils import (
|
from erpnext.accounts.utils import (
|
||||||
get_account_currency,
|
get_account_currency,
|
||||||
update_voucher_outstanding,
|
update_voucher_outstanding,
|
||||||
@@ -2883,7 +2888,7 @@ def make_inter_company_transaction(doctype, source_name, target_doc=None):
|
|||||||
"doctype": target_doctype,
|
"doctype": target_doctype,
|
||||||
"postprocess": update_details,
|
"postprocess": update_details,
|
||||||
"set_target_warehouse": "set_from_warehouse",
|
"set_target_warehouse": "set_from_warehouse",
|
||||||
"field_no_map": ["taxes_and_charges", "set_warehouse", "shipping_address", "cost_center"],
|
"field_no_map": [*CROSS_PARTY_FIELD_NO_MAP, "set_warehouse", "cost_center"],
|
||||||
},
|
},
|
||||||
doctype + " Item": item_field_map,
|
doctype + " Item": item_field_map,
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -2918,6 +2918,34 @@ class TestSalesInvoice(ERPNextTestSuite):
|
|||||||
self.assertEqual(target_doc.company, "_Test Company 1")
|
self.assertEqual(target_doc.company, "_Test Company 1")
|
||||||
self.assertEqual(target_doc.supplier, "_Test Internal Supplier")
|
self.assertEqual(target_doc.supplier, "_Test Internal Supplier")
|
||||||
|
|
||||||
|
def test_inter_company_transaction_does_not_inherit_party_fields(self):
|
||||||
|
"""
|
||||||
|
Party-derived fields on SI (from Customer) must not leak into the mapped PI.
|
||||||
|
"""
|
||||||
|
si = create_sales_invoice(
|
||||||
|
company="Wind Power LLC",
|
||||||
|
customer="_Test Internal Customer",
|
||||||
|
debit_to="Debtors - WP",
|
||||||
|
warehouse="Stores - WP",
|
||||||
|
income_account="Sales - WP",
|
||||||
|
expense_account="Cost of Goods Sold - WP",
|
||||||
|
cost_center="Main - WP",
|
||||||
|
currency="USD",
|
||||||
|
do_not_save=1,
|
||||||
|
)
|
||||||
|
si.selling_price_list = "_Test Price List Rest of the World"
|
||||||
|
si.tax_category = "_Test Tax Category 1"
|
||||||
|
si.language = "ar"
|
||||||
|
si.payment_terms_template = "_Test Payment Term Template"
|
||||||
|
si.submit()
|
||||||
|
|
||||||
|
pi = make_inter_company_transaction("Sales Invoice", si.name)
|
||||||
|
|
||||||
|
supplier = frappe.get_doc("Supplier", "_Test Internal Supplier")
|
||||||
|
self.assertEqual(pi.tax_category or None, supplier.tax_category or None)
|
||||||
|
self.assertEqual(pi.language or None, supplier.language or None)
|
||||||
|
self.assertEqual(pi.payment_terms_template or None, supplier.payment_terms or None)
|
||||||
|
|
||||||
def test_inter_company_transaction_without_default_warehouse(self):
|
def test_inter_company_transaction_without_default_warehouse(self):
|
||||||
"Check mapping (expense account) of inter company SI to PI in absence of default warehouse."
|
"Check mapping (expense account) of inter company SI to PI in absence of default warehouse."
|
||||||
# setup
|
# setup
|
||||||
|
|||||||
@@ -49,6 +49,25 @@ SALES_TRANSACTION_TYPES = {
|
|||||||
}
|
}
|
||||||
TRANSACTION_TYPES = PURCHASE_TRANSACTION_TYPES | SALES_TRANSACTION_TYPES
|
TRANSACTION_TYPES = PURCHASE_TRANSACTION_TYPES | SALES_TRANSACTION_TYPES
|
||||||
|
|
||||||
|
# Party-derived fields that must NOT be auto-copied by `get_mapped_doc` when the
|
||||||
|
# source and target documents belong to different parties (e.g. Sales Order →
|
||||||
|
# Purchase Order or inter-company Sales Invoice → Purchase Invoice).
|
||||||
|
CROSS_PARTY_FIELD_NO_MAP = [
|
||||||
|
"tax_category",
|
||||||
|
"tax_id",
|
||||||
|
"tax_withholding_category",
|
||||||
|
"taxes_and_charges",
|
||||||
|
"address_display",
|
||||||
|
"contact_display",
|
||||||
|
"contact_mobile",
|
||||||
|
"contact_email",
|
||||||
|
"contact_person",
|
||||||
|
"shipping_address",
|
||||||
|
"dispatch_address",
|
||||||
|
"payment_terms_template",
|
||||||
|
"language",
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
class DuplicatePartyAccountError(frappe.ValidationError):
|
class DuplicatePartyAccountError(frappe.ValidationError):
|
||||||
pass
|
pass
|
||||||
|
|||||||
@@ -21,7 +21,7 @@ from erpnext.accounts.doctype.sales_invoice.sales_invoice import (
|
|||||||
update_linked_doc,
|
update_linked_doc,
|
||||||
validate_inter_company_party,
|
validate_inter_company_party,
|
||||||
)
|
)
|
||||||
from erpnext.accounts.party import get_party_account
|
from erpnext.accounts.party import CROSS_PARTY_FIELD_NO_MAP, get_party_account
|
||||||
from erpnext.controllers.selling_controller import SellingController
|
from erpnext.controllers.selling_controller import SellingController
|
||||||
from erpnext.manufacturing.doctype.blanket_order.blanket_order import (
|
from erpnext.manufacturing.doctype.blanket_order.blanket_order import (
|
||||||
validate_against_blanket_order,
|
validate_against_blanket_order,
|
||||||
@@ -1743,7 +1743,6 @@ def make_purchase_order(
|
|||||||
target.shipping_rule = ""
|
target.shipping_rule = ""
|
||||||
target.tc_name = ""
|
target.tc_name = ""
|
||||||
target.terms = ""
|
target.terms = ""
|
||||||
target.payment_terms_template = ""
|
|
||||||
target.payment_schedule = []
|
target.payment_schedule = []
|
||||||
|
|
||||||
default_price_list = frappe.get_value("Supplier", supplier, "default_price_list")
|
default_price_list = frappe.get_value("Supplier", supplier, "default_price_list")
|
||||||
@@ -1810,16 +1809,7 @@ def make_purchase_order(
|
|||||||
{
|
{
|
||||||
"Sales Order": {
|
"Sales Order": {
|
||||||
"doctype": "Purchase Order",
|
"doctype": "Purchase Order",
|
||||||
"field_no_map": [
|
"field_no_map": [*CROSS_PARTY_FIELD_NO_MAP],
|
||||||
"address_display",
|
|
||||||
"contact_display",
|
|
||||||
"contact_mobile",
|
|
||||||
"contact_email",
|
|
||||||
"contact_person",
|
|
||||||
"taxes_and_charges",
|
|
||||||
"shipping_address",
|
|
||||||
"dispatch_address",
|
|
||||||
],
|
|
||||||
"validation": {"docstatus": ["=", 1]},
|
"validation": {"docstatus": ["=", 1]},
|
||||||
},
|
},
|
||||||
"Sales Order Item": {
|
"Sales Order Item": {
|
||||||
|
|||||||
@@ -25,6 +25,7 @@ from erpnext.selling.doctype.sales_order.sales_order import (
|
|||||||
make_delivery_note,
|
make_delivery_note,
|
||||||
make_material_request,
|
make_material_request,
|
||||||
make_production_plan,
|
make_production_plan,
|
||||||
|
make_purchase_order,
|
||||||
make_raw_material_request,
|
make_raw_material_request,
|
||||||
make_sales_invoice,
|
make_sales_invoice,
|
||||||
make_work_orders,
|
make_work_orders,
|
||||||
@@ -1163,9 +1164,6 @@ class TestSalesOrder(ERPNextTestSuite):
|
|||||||
|
|
||||||
def test_drop_shipping(self):
|
def test_drop_shipping(self):
|
||||||
from erpnext.buying.doctype.purchase_order.purchase_order import update_status
|
from erpnext.buying.doctype.purchase_order.purchase_order import update_status
|
||||||
from erpnext.selling.doctype.sales_order.sales_order import (
|
|
||||||
make_purchase_order,
|
|
||||||
)
|
|
||||||
from erpnext.selling.doctype.sales_order.sales_order import update_status as so_update_status
|
from erpnext.selling.doctype.sales_order.sales_order import update_status as so_update_status
|
||||||
|
|
||||||
# make items
|
# make items
|
||||||
@@ -1259,9 +1257,6 @@ class TestSalesOrder(ERPNextTestSuite):
|
|||||||
so.cancel()
|
so.cancel()
|
||||||
|
|
||||||
def test_drop_shipping_partial_order(self):
|
def test_drop_shipping_partial_order(self):
|
||||||
from erpnext.selling.doctype.sales_order.sales_order import (
|
|
||||||
make_purchase_order,
|
|
||||||
)
|
|
||||||
from erpnext.selling.doctype.sales_order.sales_order import update_status as so_update_status
|
from erpnext.selling.doctype.sales_order.sales_order import update_status as so_update_status
|
||||||
|
|
||||||
# make items
|
# make items
|
||||||
@@ -1319,10 +1314,6 @@ class TestSalesOrder(ERPNextTestSuite):
|
|||||||
|
|
||||||
def test_drop_shipping_full_for_default_suppliers(self):
|
def test_drop_shipping_full_for_default_suppliers(self):
|
||||||
"""Test if multiple POs are generated in one go against different default suppliers."""
|
"""Test if multiple POs are generated in one go against different default suppliers."""
|
||||||
from erpnext.selling.doctype.sales_order.sales_order import (
|
|
||||||
make_purchase_order,
|
|
||||||
)
|
|
||||||
|
|
||||||
if not frappe.db.exists("Item", "_Test Item for Drop Shipping 1"):
|
if not frappe.db.exists("Item", "_Test Item for Drop Shipping 1"):
|
||||||
make_item("_Test Item for Drop Shipping 1", {"is_stock_item": 1, "delivered_by_supplier": 1})
|
make_item("_Test Item for Drop Shipping 1", {"is_stock_item": 1, "delivered_by_supplier": 1})
|
||||||
|
|
||||||
@@ -1363,8 +1354,6 @@ class TestSalesOrder(ERPNextTestSuite):
|
|||||||
Tests if the the Product Bundles in the Items table of Sales Orders are replaced with
|
Tests if the the Product Bundles in the Items table of Sales Orders are replaced with
|
||||||
their child items(from the Packed Items table) on creating a Purchase Order from it.
|
their child items(from the Packed Items table) on creating a Purchase Order from it.
|
||||||
"""
|
"""
|
||||||
from erpnext.selling.doctype.sales_order.sales_order import make_purchase_order
|
|
||||||
|
|
||||||
product_bundle = make_item("_Test Product Bundle", {"is_stock_item": 0})
|
product_bundle = make_item("_Test Product Bundle", {"is_stock_item": 0})
|
||||||
make_item("_Test Bundle Item 1", {"is_stock_item": 1})
|
make_item("_Test Bundle Item 1", {"is_stock_item": 1})
|
||||||
make_item("_Test Bundle Item 2", {"is_stock_item": 1})
|
make_item("_Test Bundle Item 2", {"is_stock_item": 1})
|
||||||
@@ -1393,8 +1382,6 @@ class TestSalesOrder(ERPNextTestSuite):
|
|||||||
"""
|
"""
|
||||||
Tests if the packed item's `ordered_qty` is updated with the quantity of the Purchase Order
|
Tests if the packed item's `ordered_qty` is updated with the quantity of the Purchase Order
|
||||||
"""
|
"""
|
||||||
from erpnext.selling.doctype.sales_order.sales_order import make_purchase_order
|
|
||||||
|
|
||||||
product_bundle = make_item("_Test Product Bundle", {"is_stock_item": 0})
|
product_bundle = make_item("_Test Product Bundle", {"is_stock_item": 0})
|
||||||
make_item("_Test Bundle Item 1", {"is_stock_item": 1})
|
make_item("_Test Bundle Item 1", {"is_stock_item": 1})
|
||||||
make_item("_Test Bundle Item 2", {"is_stock_item": 1})
|
make_item("_Test Bundle Item 2", {"is_stock_item": 1})
|
||||||
@@ -2664,8 +2651,6 @@ class TestSalesOrder(ERPNextTestSuite):
|
|||||||
self.assertEqual(so.status, "To Deliver and Bill")
|
self.assertEqual(so.status, "To Deliver and Bill")
|
||||||
|
|
||||||
def test_item_tax_transfer_from_sales_to_purchase(self):
|
def test_item_tax_transfer_from_sales_to_purchase(self):
|
||||||
from erpnext.selling.doctype.sales_order.sales_order import make_purchase_order
|
|
||||||
|
|
||||||
item_tax = frappe.new_doc("Item Tax Template")
|
item_tax = frappe.new_doc("Item Tax Template")
|
||||||
item_tax.title = "Test Item Tax Template"
|
item_tax.title = "Test Item Tax Template"
|
||||||
item_tax.company = "_Test Company"
|
item_tax.company = "_Test Company"
|
||||||
@@ -2695,6 +2680,33 @@ class TestSalesOrder(ERPNextTestSuite):
|
|||||||
po.submit()
|
po.submit()
|
||||||
self.assertEqual(po.taxes[0].tax_amount, 2)
|
self.assertEqual(po.taxes[0].tax_amount, 2)
|
||||||
|
|
||||||
|
def test_make_purchase_order_does_not_inherit_party_fields(self):
|
||||||
|
"""
|
||||||
|
Customer-derived fields must not leak from a drop-ship SO into the PO.
|
||||||
|
"""
|
||||||
|
so_items = [
|
||||||
|
{
|
||||||
|
"item_code": "_Test Item",
|
||||||
|
"warehouse": "",
|
||||||
|
"qty": 1,
|
||||||
|
"rate": 100,
|
||||||
|
"delivered_by_supplier": 1,
|
||||||
|
"supplier": "_Test Supplier",
|
||||||
|
}
|
||||||
|
]
|
||||||
|
so = make_sales_order(item_list=so_items, do_not_submit=True)
|
||||||
|
so.tax_category = "_Test Tax Category 1"
|
||||||
|
so.language = "ar"
|
||||||
|
so.payment_terms_template = "_Test Payment Term Template"
|
||||||
|
so.submit()
|
||||||
|
|
||||||
|
po = make_purchase_order(so.name, selected_items=so_items)[0]
|
||||||
|
|
||||||
|
supplier = frappe.get_doc("Supplier", "_Test Supplier")
|
||||||
|
self.assertEqual(po.tax_category or None, supplier.tax_category or None)
|
||||||
|
self.assertEqual(po.language or None, supplier.language or None)
|
||||||
|
self.assertEqual(po.payment_terms_template or None, supplier.payment_terms or None)
|
||||||
|
|
||||||
def test_pending_quantity_after_update_item_during_invoice_creation(self):
|
def test_pending_quantity_after_update_item_during_invoice_creation(self):
|
||||||
so = make_sales_order(qty=30, rate=100)
|
so = make_sales_order(qty=30, rate=100)
|
||||||
|
|
||||||
|
|||||||
@@ -16,7 +16,7 @@ from frappe.query_builder import DocType
|
|||||||
from frappe.query_builder.functions import Abs, Sum
|
from frappe.query_builder.functions import Abs, Sum
|
||||||
from frappe.utils import cint, flt
|
from frappe.utils import cint, flt
|
||||||
|
|
||||||
from erpnext.accounts.party import get_due_date
|
from erpnext.accounts.party import CROSS_PARTY_FIELD_NO_MAP, get_due_date
|
||||||
from erpnext.controllers.accounts_controller import get_taxes_and_charges, merge_taxes
|
from erpnext.controllers.accounts_controller import get_taxes_and_charges, merge_taxes
|
||||||
from erpnext.controllers.selling_controller import SellingController
|
from erpnext.controllers.selling_controller import SellingController
|
||||||
from erpnext.stock.doctype.packed_item.packed_item import make_packing_list
|
from erpnext.stock.doctype.packed_item.packed_item import make_packing_list
|
||||||
@@ -1390,8 +1390,7 @@ def make_inter_company_transaction(doctype, source_name, target_doc=None):
|
|||||||
doctype: {
|
doctype: {
|
||||||
"doctype": target_doctype,
|
"doctype": target_doctype,
|
||||||
"postprocess": update_details,
|
"postprocess": update_details,
|
||||||
"field_no_map": ["taxes_and_charges", "set_warehouse"],
|
"field_no_map": [*CROSS_PARTY_FIELD_NO_MAP, "set_warehouse"],
|
||||||
"field_map": {"shipping_address_name": "shipping_address"},
|
|
||||||
},
|
},
|
||||||
doctype + " Item": {
|
doctype + " Item": {
|
||||||
"doctype": target_doctype + " Item",
|
"doctype": target_doctype + " Item",
|
||||||
|
|||||||
@@ -1051,6 +1051,40 @@ class TestPurchaseReceipt(ERPNextTestSuite):
|
|||||||
|
|
||||||
pr.cancel()
|
pr.cancel()
|
||||||
|
|
||||||
|
def test_inter_company_purchase_receipt_does_not_inherit_party_fields(self):
|
||||||
|
"""
|
||||||
|
Party-derived fields on DN (from Customer) must not leak into the mapped PR.
|
||||||
|
"""
|
||||||
|
from erpnext.stock.doctype.delivery_note.delivery_note import make_inter_company_purchase_receipt
|
||||||
|
from erpnext.stock.doctype.delivery_note.test_delivery_note import create_delivery_note
|
||||||
|
|
||||||
|
prepare_data_for_internal_transfer()
|
||||||
|
|
||||||
|
customer = "_Test Internal Customer 2"
|
||||||
|
company = "_Test Company with perpetual inventory"
|
||||||
|
|
||||||
|
dn = create_delivery_note(
|
||||||
|
company=company,
|
||||||
|
customer=customer,
|
||||||
|
cost_center="Main - TCP1",
|
||||||
|
expense_account="Cost of Goods Sold - TCP1",
|
||||||
|
qty=1,
|
||||||
|
rate=100,
|
||||||
|
warehouse="Stores - TCP1",
|
||||||
|
target_warehouse="Work In Progress - TCP1",
|
||||||
|
do_not_submit=True,
|
||||||
|
)
|
||||||
|
# Stamp customer-side party fields onto the DN
|
||||||
|
dn.tax_category = "_Test Tax Category 2"
|
||||||
|
dn.language = "ar"
|
||||||
|
dn.submit()
|
||||||
|
|
||||||
|
pr = make_inter_company_purchase_receipt(dn.name)
|
||||||
|
|
||||||
|
supplier = frappe.get_doc("Supplier", "_Test Internal Supplier 2")
|
||||||
|
self.assertEqual(pr.tax_category or None, supplier.tax_category or None)
|
||||||
|
self.assertEqual(pr.language or None, supplier.language or None)
|
||||||
|
|
||||||
def test_lcv_for_internal_transfer(self):
|
def test_lcv_for_internal_transfer(self):
|
||||||
from erpnext.stock.doctype.delivery_note.delivery_note import make_inter_company_purchase_receipt
|
from erpnext.stock.doctype.delivery_note.delivery_note import make_inter_company_purchase_receipt
|
||||||
from erpnext.stock.doctype.delivery_note.test_delivery_note import create_delivery_note
|
from erpnext.stock.doctype.delivery_note.test_delivery_note import create_delivery_note
|
||||||
|
|||||||
Reference in New Issue
Block a user