mirror of
https://github.com/frappe/erpnext.git
synced 2026-08-26 13:25:20 +00:00
fix: block disabled/frozen party on Opportunity and Request for Quotation (backport #57983) (#58035)
* fix: block disabled/frozen customers on Opportunity Opportunity inherits TransactionBase instead of AccountsController, so it never ran validate_party_frozen_disabled like Quotation, Sales Order and Sales Invoice do. A disabled Customer could be saved as an Opportunity's party and only get caught later at Quotation stage. Also fixes the party_name Link query on the client: it referenced erpnext.queries.customer, which was never defined, so disabled customers showed up in the picker. (cherry picked from commit90937ce6d9) * fix: block disabled/frozen suppliers on Request for Quotation Request for Quotation overrides validate() entirely and never calls super().validate(), so it never goes through AccountsController's party validation. Suppliers also sit in a child table, so the shared PartyValidator wouldn't have caught it anyway (it only checks a single top-level party field). A disabled or frozen Supplier could be added to an RFQ and the RFQ submitted without any warning. Also filters the suppliers grid's supplier Link field to disabled=0, matching the same client-side fix applied to Opportunity's party_name. (cherry picked from commit4bf65ffc1d) # Conflicts: # erpnext/buying/doctype/request_for_quotation/request_for_quotation.py # erpnext/buying/doctype/request_for_quotation/test_request_for_quotation.py * fix: scope Opportunity party validation to Customer only validate_party_frozen_disabled only enforces Customer/Supplier/Employee, so passing opportunity_from straight through silently no-op'd for Lead and Prospect. Made the Customer-only scope explicit instead of relying on that implicit fallthrough. Lead.disabled is not enforced anywhere else in the codebase (lead_query, the picker used for this same field, only filters status/docstatus), so deliberately not extending validation to Lead-sourced Opportunities. (cherry picked from commit8c0a945417) * refactor: move RFQ supplier disabled filter to link_filters Static filters with no doc-dependent values belong on the field definition, not in JS. Matches the existing pattern used for Warehouse/Item link_filters elsewhere (e.g. job_card_item.json, product_bundle_item.json). (cherry picked from commit6b35c51ff1) * fix: resolve backport conflicts for disabled/frozen party validation The automated backport left unresolved merge conflict markers committed in request_for_quotation.py and test_request_for_quotation.py. Dropped test_duplicate_supplier_rejected, test_rfq_blocked_for_supplier_with_prevent_rfqs and test_rfq_status_lifecycle from the conflict resolution, they don't exist on this branch and aren't part of this backport. --------- Co-authored-by: Jatin3128 <jatinsarna8@gmail.com>
This commit is contained in:
@@ -15,7 +15,11 @@ from frappe.utils import get_url
|
||||
from frappe.utils.print_format import download_pdf
|
||||
from frappe.utils.user import get_user_fullname
|
||||
|
||||
from erpnext.accounts.party import _get_party_details, get_party_account_currency
|
||||
from erpnext.accounts.party import (
|
||||
_get_party_details,
|
||||
get_party_account_currency,
|
||||
validate_party_frozen_disabled,
|
||||
)
|
||||
from erpnext.buying.utils import validate_for_items
|
||||
from erpnext.controllers.buying_controller import BuyingController
|
||||
from erpnext.stock.doctype.material_request.material_request import set_missing_values
|
||||
@@ -126,6 +130,8 @@ class RequestforQuotation(BuyingController):
|
||||
|
||||
def validate_supplier_list(self):
|
||||
for d in self.suppliers:
|
||||
validate_party_frozen_disabled(self.company, "Supplier", d.supplier)
|
||||
|
||||
prevent_rfqs = frappe.db.get_value("Supplier", d.supplier, "prevent_rfqs")
|
||||
if prevent_rfqs:
|
||||
standing = frappe.db.get_value("Supplier Scorecard", d.supplier, "status")
|
||||
|
||||
@@ -17,6 +17,7 @@ from erpnext.buying.doctype.request_for_quotation.request_for_quotation import (
|
||||
from erpnext.controllers.accounts_controller import InvalidQtyError
|
||||
from erpnext.crm.doctype.opportunity.opportunity import make_request_for_quotation as make_rfq
|
||||
from erpnext.crm.doctype.opportunity.test_opportunity import make_opportunity
|
||||
from erpnext.exceptions import PartyDisabled
|
||||
from erpnext.stock.doctype.item.test_item import make_item
|
||||
from erpnext.templates.pages.rfq import check_supplier_has_docname_access
|
||||
from erpnext.tests.utils import ERPNextTestSuite
|
||||
@@ -58,6 +59,17 @@ class TestRequestforQuotation(ERPNextTestSuite):
|
||||
self.assertEqual(rfq.get("suppliers")[0].quote_status, "Received")
|
||||
self.assertEqual(rfq.get("suppliers")[1].quote_status, "Pending")
|
||||
|
||||
def test_rfq_blocked_for_disabled_supplier(self):
|
||||
frappe.db.set_value("Supplier", "_Test Supplier", "disabled", 1)
|
||||
rfq = make_request_for_quotation(
|
||||
supplier_data=[{"supplier": "_Test Supplier", "supplier_name": "_Test Supplier"}],
|
||||
do_not_save=True,
|
||||
)
|
||||
self.assertRaises(PartyDisabled, rfq.save)
|
||||
|
||||
frappe.db.set_value("Supplier", "_Test Supplier", "disabled", 0)
|
||||
rfq.save()
|
||||
|
||||
def test_make_supplier_quotation(self):
|
||||
rfq = make_request_for_quotation()
|
||||
|
||||
|
||||
@@ -40,6 +40,7 @@
|
||||
"fieldtype": "Link",
|
||||
"in_list_view": 1,
|
||||
"label": "Supplier",
|
||||
"link_filters": "[[\"Supplier\",\"disabled\",\"=\",0]]",
|
||||
"options": "Supplier",
|
||||
"reqd": 1
|
||||
},
|
||||
|
||||
@@ -13,6 +13,7 @@ from frappe.query_builder import DocType, Interval
|
||||
from frappe.query_builder.functions import Now
|
||||
from frappe.utils import flt, get_fullname
|
||||
|
||||
from erpnext.accounts.party import validate_party_frozen_disabled
|
||||
from erpnext.crm.utils import (
|
||||
CRMNote,
|
||||
copy_comments,
|
||||
@@ -133,6 +134,7 @@ class Opportunity(TransactionBase, CRMNote):
|
||||
self.validate_item_details()
|
||||
self.validate_uom_is_integer("uom", "qty")
|
||||
self.validate_cust_name()
|
||||
self.validate_party()
|
||||
self.map_fields()
|
||||
self.validate_qty()
|
||||
self.set_exchange_rate()
|
||||
@@ -348,6 +350,10 @@ class Opportunity(TransactionBase, CRMNote):
|
||||
return False
|
||||
return True
|
||||
|
||||
def validate_party(self) -> None:
|
||||
if self.opportunity_from == "Customer":
|
||||
validate_party_frozen_disabled(self.company, "Customer", self.party_name)
|
||||
|
||||
def validate_cust_name(self):
|
||||
if self.party_name:
|
||||
if self.opportunity_from == "Customer":
|
||||
|
||||
@@ -9,6 +9,7 @@ from erpnext.crm.doctype.lead.lead import make_customer
|
||||
from erpnext.crm.doctype.lead.test_lead import make_lead
|
||||
from erpnext.crm.doctype.opportunity.opportunity import make_quotation
|
||||
from erpnext.crm.utils import get_linked_communication_list
|
||||
from erpnext.exceptions import PartyDisabled
|
||||
from erpnext.tests.utils import ERPNextTestSuite
|
||||
|
||||
|
||||
@@ -71,6 +72,23 @@ class TestOpportunity(ERPNextTestSuite):
|
||||
opportunity_doc = make_opportunity(with_items=1, rate=1100, qty=2)
|
||||
self.assertEqual(opportunity_doc.total, 2200)
|
||||
|
||||
def test_disabled_customer_not_allowed(self):
|
||||
frappe.db.set_value("Customer", "_Test Customer", "disabled", 1)
|
||||
|
||||
self.assertRaises(PartyDisabled, make_opportunity, with_items=0)
|
||||
|
||||
frappe.db.set_value("Customer", "_Test Customer", "disabled", 0)
|
||||
make_opportunity(with_items=0)
|
||||
|
||||
def test_disabled_lead_not_blocked(self):
|
||||
# Lead.disabled isn't enforced anywhere else (e.g. the Lead picker query only
|
||||
# excludes Converted leads), so it shouldn't block Opportunity creation either.
|
||||
lead_doc = make_lead()
|
||||
frappe.db.set_value("Lead", lead_doc.name, "disabled", 1)
|
||||
|
||||
opp_doc = make_opportunity(opportunity_from="Lead", lead=lead_doc.name)
|
||||
self.assertEqual(opp_doc.party_name, lead_doc.name)
|
||||
|
||||
def test_carry_forward_of_email_and_comments(self):
|
||||
frappe.db.set_single_value("CRM Settings", "carry_forward_communication_and_comments", 1)
|
||||
lead_doc = make_lead()
|
||||
|
||||
@@ -12,6 +12,10 @@ $.extend(erpnext.queries, {
|
||||
return { query: "erpnext.controllers.queries.lead_query" };
|
||||
},
|
||||
|
||||
customer: function () {
|
||||
return { filters: { disabled: 0 } };
|
||||
},
|
||||
|
||||
item: function (filters) {
|
||||
var args = { query: "erpnext.controllers.queries.item_query" };
|
||||
if (filters) args["filters"] = filters;
|
||||
|
||||
Reference in New Issue
Block a user