diff --git a/erpnext/accounts/doctype/account/account.py b/erpnext/accounts/doctype/account/account.py index 2fb3c7875cf..a5855c41093 100644 --- a/erpnext/accounts/doctype/account/account.py +++ b/erpnext/accounts/doctype/account/account.py @@ -120,6 +120,7 @@ class Account(NestedSet): self.validate_account_currency() self.validate_root_company_and_sync_account_to_children() self.validate_receivable_payable_account_type() + self.validate_stock_account_type_change() def validate_parent_child_account_type(self): if self.parent_account: @@ -208,6 +209,36 @@ class Account(NestedSet): frappe.msgprint(msg) self.add_comment("Comment", msg) + def validate_stock_account_type_change(self): + doc_before_save = self.get_doc_before_save() + if not (doc_before_save and doc_before_save.account_type == "Stock"): + return + + if self.account_type == "Stock": + return + + if self.stock_ledger_entry_exists(): + frappe.throw( + _( + "The account type of {0} cannot be changed from {1} because stock ledger entries exist against it." + ).format(frappe.bold(self.name), frappe.bold(_("Stock"))) + ) + + def stock_ledger_entry_exists(self): + from erpnext.stock import get_warehouse_account_map + + warehouse_account = get_warehouse_account_map(self.company) + warehouses = [wh for wh, details in warehouse_account.items() if details.account == self.name] + if not warehouses: + return False + + return bool( + frappe.db.count( + "Stock Ledger Entry", + filters={"warehouse": ("in", warehouses), "is_cancelled": 0}, + ) + ) + def validate_root_details(self): doc_before_save = self.get_doc_before_save() diff --git a/erpnext/accounts/doctype/account/test_account.py b/erpnext/accounts/doctype/account/test_account.py index f840ac86207..dace7d34613 100644 --- a/erpnext/accounts/doctype/account/test_account.py +++ b/erpnext/accounts/doctype/account/test_account.py @@ -307,6 +307,31 @@ class TestAccount(ERPNextTestSuite): acc.account_currency = "USD" self.assertRaises(frappe.ValidationError, acc.save) + def test_stock_account_type_change_with_ledger_entries(self): + from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry + + company = "_Test Company with perpetual inventory" + warehouse = "Stores - TCP1" + stock_account = get_warehouse_account(frappe.get_doc("Warehouse", warehouse)) + + make_stock_entry( + item_code="_Test Item", + target=warehouse, + company=company, + qty=5, + basic_rate=100, + ) + + account = frappe.get_doc("Account", stock_account) + self.assertEqual(account.account_type, "Stock") + + account.account_type = "" + self.assertRaises(frappe.ValidationError, account.save) + + account.reload() + account.account_name = f"{account.account_name} Updated" + account.save() # non-type change stays allowed + def test_account_balance(self): from erpnext.accounts.utils import get_balance_on diff --git a/erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.py b/erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.py index b25d7f962c0..6a0958c7bb2 100644 --- a/erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.py +++ b/erpnext/accounts/doctype/chart_of_accounts_importer/chart_of_accounts_importer.py @@ -218,6 +218,7 @@ def build_forest(data): for row in data: account_name, parent_account, account_number, parent_account_number = row[0:4] if account_number: + account_number = cstr(account_number).strip() account_name = f"{account_number} - {account_name}" if parent_account_number: parent_account_number = cstr(parent_account_number).strip() diff --git a/erpnext/accounts/doctype/dunning/dunning.js b/erpnext/accounts/doctype/dunning/dunning.js index e9d091f2e85..c9955c7e359 100644 --- a/erpnext/accounts/doctype/dunning/dunning.js +++ b/erpnext/accounts/doctype/dunning/dunning.js @@ -169,23 +169,10 @@ frappe.ui.form.on("Dunning", { }, get_dunning_letter_text: function (frm) { if (frm.doc.dunning_type) { - frappe.call({ - method: "erpnext.accounts.doctype.dunning.dunning.get_dunning_letter_text", - args: { - dunning_type: frm.doc.dunning_type, - language: frm.doc.language, - doc: frm.doc, - }, - callback: function (r) { - if (r.message) { - frm.set_value("body_text", r.message.body_text); - frm.set_value("closing_text", r.message.closing_text); - frm.set_value("language", r.message.language); - } else { - frm.set_value("body_text", ""); - frm.set_value("closing_text", ""); - } - }, + frm.call("get_dunning_letter_text").then((r) => { + if (!r.exc) { + frm.refresh_fields(); + } }); } }, diff --git a/erpnext/accounts/doctype/dunning/dunning.py b/erpnext/accounts/doctype/dunning/dunning.py index f64e957400b..70cdb99ae1d 100644 --- a/erpnext/accounts/doctype/dunning/dunning.py +++ b/erpnext/accounts/doctype/dunning/dunning.py @@ -163,6 +163,46 @@ class Dunning(AccountsController): "Serial and Batch Bundle", ] + @frappe.whitelist() + def get_dunning_letter_text(self): + DOCTYPE = "Dunning Letter Text" + FIELDS = ["body_text", "closing_text", "language"] + + if not self.dunning_type: + return + + filters = {"parent": self.dunning_type, "is_default_language": 1} + + if self.language: + filters.pop("is_default_language") + filters["language"] = self.language + + letter_text = frappe.db.get_value(DOCTYPE, filters, FIELDS, as_dict=True) + + if not letter_text: + msg = ( + _("Dunning Letter for Dunning Type {0} in language '{1}' not found.").format( + frappe.bold(self.dunning_type), frappe.bold(self.language) + ) + if self.language + else _("Dunning Letter for Dunning Type {0} not found.").format( + frappe.bold(self.dunning_type) + ) + ) + frappe.msgprint(msg, alert=True, indicator="yellow") + + self.body_text = ( + frappe.render_template(letter_text.body_text, self.as_dict(), restrict_globals=True) + if letter_text + else None + ) + self.closing_text = ( + frappe.render_template(letter_text.closing_text, self.as_dict(), restrict_globals=True) + if letter_text + else None + ) + self.language = letter_text.language if letter_text else self.language + def update_linked_dunnings(doc, previous_outstanding_amount): if ( @@ -241,35 +281,3 @@ def get_linked_dunnings_as_per_state(sales_invoice, state): & (overdue_payment.sales_invoice == sales_invoice) ) ).run(as_dict=True) - - -@frappe.whitelist() -def get_dunning_letter_text(dunning_type: str, doc: str | dict, language: str | None = None) -> dict: - DOCTYPE = "Dunning Letter Text" - FIELDS = ["body_text", "closing_text", "language"] - - if isinstance(doc, str): - doc = json.loads(doc) - - if not language: - language = doc.get("language") - - letter_text = None - if language: - letter_text = frappe.db.get_value( - DOCTYPE, {"parent": dunning_type, "language": language}, FIELDS, as_dict=1 - ) - - if not letter_text: - letter_text = frappe.db.get_value( - DOCTYPE, {"parent": dunning_type, "is_default_language": 1}, FIELDS, as_dict=1 - ) - - if not letter_text: - return {} - - return { - "body_text": frappe.render_template(letter_text.body_text, doc), - "closing_text": frappe.render_template(letter_text.closing_text, doc), - "language": letter_text.language, - } diff --git a/erpnext/accounts/doctype/dunning_type/dunning_type.py b/erpnext/accounts/doctype/dunning_type/dunning_type.py index 77f2e004e3d..f267ee5b9a1 100644 --- a/erpnext/accounts/doctype/dunning_type/dunning_type.py +++ b/erpnext/accounts/doctype/dunning_type/dunning_type.py @@ -3,7 +3,10 @@ import frappe +from frappe import _ from frappe.model.document import Document +from frappe.utils import comma_and +from frappe.utils.jinja import validate_template class DunningType(Document): @@ -30,3 +33,134 @@ class DunningType(Document): def autoname(self): company_abbr = frappe.get_value("Company", self.company, "abbr") self.name = f"{self.dunning_type} - {company_abbr}" + + def validate(self): + self.validate_dunning_letter_text() + self.validate_income_account() + self.validate_cost_center() + self.set_default_dunning_type() + + def validate_dunning_letter_text(self): + self.validate_languages() + self.validate_is_default_language() + self.validate_dunning_letter_text_templates() + + def validate_income_account(self): + if not self.income_account: + return + + account = frappe.get_cached_doc("Account", self.income_account) + + msg = [] + if account.company != self.company: + msg.append( + _( + "{0} doesn't belong to Company {1}. Please select an Income Account that belongs to Company {1}." + ).format(frappe.bold(self.income_account), frappe.bold(self.company)) + ) + + if account.disabled: + msg.append( + _("{0} is disabled. Please select a valid Income Account.").format( + frappe.bold(self.income_account) + ) + ) + + if account.root_type != "Income": + msg.append( + _("{0} is not an Income Account. Please select a valid Income Account.").format( + frappe.bold(self.income_account) + ) + ) + + if account.is_group: + msg.append( + _("{0} is a group account. Please select a non-group Income Account.").format( + frappe.bold(self.income_account) + ) + ) + + if msg: + frappe.msgprint( + msg, + title=_("Income Account Validation Error"), + as_list=True, + raise_exception=frappe.ValidationError, + ) + + def validate_cost_center(self): + if not self.cost_center: + return + + cost_center = frappe.get_cached_doc("Cost Center", self.cost_center) + + msg = [] + if cost_center.company != self.company: + msg.append( + _( + "{0} doesn't belong to Company {1}. Please select a Cost Center that belongs to Company {1}." + ).format(frappe.bold(self.cost_center), frappe.bold(self.company)) + ) + + if cost_center.disabled: + msg.append( + _("{0} is disabled. Please select an enabled Cost Center.").format( + frappe.bold(self.cost_center) + ) + ) + + if cost_center.is_group: + msg.append( + _("{0} is a group Cost Center. Please select a non-group Cost Center.").format( + frappe.bold(self.cost_center) + ) + ) + + if msg: + frappe.msgprint( + msg, + title=_("Cost Center Validation Error"), + as_list=True, + raise_exception=frappe.ValidationError, + ) + + def validate_languages(self): + languages = [d.language for d in self.dunning_letter_text] + + if len(languages) == len(set(languages)): + return + + frappe.throw(_("Duplicate languages found on Dunning Letter Text. Keep only one of them.")) + + def validate_is_default_language(self): + is_default_language_list = [ + d.language for d in self.dunning_letter_text if d.is_default_language == 1 + ] + + if len(is_default_language_list) <= 1: + return + + frappe.throw( + _("{0} languages are marked as default languages. Please select only one of them.").format( + comma_and(is_default_language_list, add_quotes=True) + ) + ) + + def validate_dunning_letter_text_templates(self): + for d in self.dunning_letter_text: + if d.body_text: + validate_template(d.body_text, restrict_globals=True) + + if d.closing_text: + validate_template(d.closing_text, restrict_globals=True) + + def set_default_dunning_type(self): + if self.is_default != 1: + return + + frappe.db.set_value( + "Dunning Type", + {"company": self.company, "is_default": 1, "name": ["!=", self.name]}, + "is_default", + 0, + ) diff --git a/erpnext/accounts/doctype/dunning_type/test_dunning_type.py b/erpnext/accounts/doctype/dunning_type/test_dunning_type.py index 4cf60c86600..94c30fe089b 100644 --- a/erpnext/accounts/doctype/dunning_type/test_dunning_type.py +++ b/erpnext/accounts/doctype/dunning_type/test_dunning_type.py @@ -1,10 +1,200 @@ # Copyright (c) 2020, Frappe Technologies Pvt. Ltd. and Contributors # See license.txt -# import frappe -import unittest + +import frappe from erpnext.tests.utils import ERPNextTestSuite +def make_dunning_type(dunning_type, company="_Test Company", **kwargs): + doc = frappe.new_doc("Dunning Type") + doc.dunning_type = dunning_type + doc.company = company + doc.dunning_fee = kwargs.get("dunning_fee", 100) + doc.rate_of_interest = kwargs.get("rate_of_interest", 5) + doc.is_default = kwargs.get("is_default", 0) + + if "income_account" in kwargs: + doc.income_account = kwargs["income_account"] + elif kwargs.get("income_account") is not False: + doc.income_account = "Sales - _TC" if company == "_Test Company" else "Sales - _TC1" + + if "cost_center" in kwargs: + doc.cost_center = kwargs["cost_center"] + elif kwargs.get("cost_center") is not False: + doc.cost_center = "Main - _TC" if company == "_Test Company" else "Main - _TC1" + + for row in kwargs.get("dunning_letter_text", [{"language": "en", "body_text": "Test body"}]): + doc.append("dunning_letter_text", row) + + return doc + + class TestDunningType(ERPNextTestSuite): - pass + def test_income_account_must_belong_to_company(self): + doc = make_dunning_type("_Test Dunning Wrong Company Account", income_account="Sales - _TC1") + self.assertRaisesRegex(frappe.ValidationError, "doesn't belong to Company", doc.insert) + + def test_income_account_must_not_be_disabled(self): + disabled_account = frappe.get_doc( + { + "doctype": "Account", + "account_name": "_Test Disabled Income Account", + "parent_account": "Direct Income - _TC", + "company": "_Test Company", + "account_type": "Income Account", + "disabled": 1, + } + ).insert() + + doc = make_dunning_type("_Test Dunning Disabled Account", income_account=disabled_account.name) + self.assertRaisesRegex(frappe.ValidationError, "is disabled", doc.insert) + + def test_income_account_must_be_income_type(self): + doc = make_dunning_type("_Test Dunning Non Income Account", income_account="Debtors - _TC") + self.assertRaisesRegex(frappe.ValidationError, "is not an Income Account", doc.insert) + + def test_income_account_must_not_be_group(self): + doc = make_dunning_type("_Test Dunning Group Account", income_account="Income - _TC") + self.assertRaisesRegex(frappe.ValidationError, "is a group account", doc.insert) + + def test_income_account_is_optional(self): + doc = make_dunning_type("_Test Dunning No Income Account", income_account=False) + doc.insert() + self.assertFalse(doc.income_account) + + def test_valid_income_account_passes(self): + doc = make_dunning_type("_Test Dunning Valid Income Account", income_account="Sales - _TC") + doc.insert() + self.assertEqual(doc.income_account, "Sales - _TC") + + def test_cost_center_must_belong_to_company(self): + doc = make_dunning_type("_Test Dunning Wrong Company CC", cost_center="Main - _TC1") + self.assertRaisesRegex(frappe.ValidationError, "doesn't belong to Company", doc.insert) + + def test_cost_center_must_not_be_disabled(self): + disabled_cc = frappe.get_doc( + { + "doctype": "Cost Center", + "cost_center_name": "_Test Disabled Cost Center", + "parent_cost_center": "_Test Company - _TC", + "company": "_Test Company", + "disabled": 1, + } + ).insert() + + doc = make_dunning_type("_Test Dunning Disabled CC", cost_center=disabled_cc.name) + self.assertRaisesRegex(frappe.ValidationError, "is disabled", doc.insert) + + def test_cost_center_must_not_be_group(self): + doc = make_dunning_type("_Test Dunning Group CC", cost_center="_Test Company - _TC") + self.assertRaisesRegex(frappe.ValidationError, "is a group Cost Center", doc.insert) + + def test_cost_center_is_optional(self): + doc = make_dunning_type("_Test Dunning No CC", cost_center=False) + doc.insert() + self.assertFalse(doc.cost_center) + + def test_valid_cost_center_passes(self): + doc = make_dunning_type("_Test Dunning Valid CC", cost_center="Main - _TC") + doc.insert() + self.assertEqual(doc.cost_center, "Main - _TC") + + def test_duplicate_languages_not_allowed(self): + doc = make_dunning_type( + "_Test Dunning Duplicate Language", + dunning_letter_text=[ + {"language": "en", "body_text": "Body one"}, + {"language": "en", "body_text": "Body two"}, + ], + ) + self.assertRaisesRegex(frappe.ValidationError, "Duplicate languages found", doc.insert) + + def test_unique_languages_allowed(self): + doc = make_dunning_type( + "_Test Dunning Unique Languages", + dunning_letter_text=[ + {"language": "en", "body_text": "Body one"}, + {"language": "de", "body_text": "Body two"}, + ], + ) + doc.insert() + self.assertEqual(len(doc.dunning_letter_text), 2) + + def test_only_one_default_language_allowed(self): + doc = make_dunning_type( + "_Test Dunning Multiple Default Language", + dunning_letter_text=[ + {"language": "en", "body_text": "Body one", "is_default_language": 1}, + {"language": "de", "body_text": "Body two", "is_default_language": 1}, + ], + ) + self.assertRaisesRegex( + frappe.ValidationError, "languages are marked as default languages", doc.insert + ) + + def test_single_default_language_allowed(self): + doc = make_dunning_type( + "_Test Dunning Single Default Language", + dunning_letter_text=[ + {"language": "en", "body_text": "Body one", "is_default_language": 1}, + {"language": "de", "body_text": "Body two", "is_default_language": 0}, + ], + ) + doc.insert() + self.assertEqual(doc.dunning_letter_text[0].is_default_language, 1) + + def test_invalid_jinja_template_in_body_text_raises(self): + doc = make_dunning_type( + "_Test Dunning Invalid Body Template", + dunning_letter_text=[{"language": "en", "body_text": "{{ unclosed"}], + ) + self.assertRaisesRegex(frappe.ValidationError, "Syntax error in template", doc.insert) + + def test_invalid_jinja_template_in_closing_text_raises(self): + doc = make_dunning_type( + "_Test Dunning Invalid Closing Template", + dunning_letter_text=[ + {"language": "en", "body_text": "Valid body", "closing_text": "{{ unclosed"} + ], + ) + self.assertRaisesRegex(frappe.ValidationError, "Syntax error in template", doc.insert) + + def test_valid_jinja_template_passes(self): + doc = make_dunning_type( + "_Test Dunning Valid Template", + dunning_letter_text=[ + { + "language": "en", + "body_text": "Outstanding amount is {{ outstanding_amount }}", + "closing_text": "Regards, {{ company }}", + } + ], + ) + doc.insert() + self.assertTrue(doc.name) + + def test_set_default_dunning_type_unsets_previous_default(self): + first = make_dunning_type("_Test Dunning Default One", is_default=1) + first.insert() + self.assertEqual(frappe.db.get_value("Dunning Type", first.name, "is_default"), 1) + + second = make_dunning_type("_Test Dunning Default Two", is_default=1) + second.insert() + + self.assertEqual(frappe.db.get_value("Dunning Type", first.name, "is_default"), 0) + self.assertEqual(frappe.db.get_value("Dunning Type", second.name, "is_default"), 1) + + def test_set_default_dunning_type_scoped_per_company(self): + company_1 = make_dunning_type("_Test Dunning Default Co1", is_default=1) + company_1.insert() + + company_2 = make_dunning_type( + "_Test Dunning Default Co2", + company="_Test Company 1", + is_default=1, + ) + company_2.insert() + + self.assertEqual(frappe.db.get_value("Dunning Type", company_1.name, "is_default"), 1) + self.assertEqual(frappe.db.get_value("Dunning Type", company_2.name, "is_default"), 1) diff --git a/erpnext/accounts/doctype/payment_reference/payment_reference.json b/erpnext/accounts/doctype/payment_reference/payment_reference.json index a1adb181d35..4e1e0ac22e3 100644 --- a/erpnext/accounts/doctype/payment_reference/payment_reference.json +++ b/erpnext/accounts/doctype/payment_reference/payment_reference.json @@ -14,7 +14,8 @@ "section_break_mjlv", "due_date", "column_break_qghl", - "amount" + "amount", + "currency" ], "fields": [ { @@ -55,8 +56,18 @@ "fieldtype": "Currency", "in_list_view": 1, "label": "Amount", + "options": "currency", "precision": "2" }, + { + "fieldname": "currency", + "fieldtype": "Link", + "hidden": 1, + "label": "Currency", + "options": "Currency", + "print_hide": 1, + "read_only": 1 + }, { "fieldname": "column_break_lnjp", "fieldtype": "Column Break" @@ -74,7 +85,7 @@ "index_web_pages_for_search": 1, "istable": 1, "links": [], - "modified": "2026-01-19 02:21:36.455830", + "modified": "2026-07-11 00:00:00.000000", "modified_by": "Administrator", "module": "Accounts", "name": "Payment Reference", diff --git a/erpnext/accounts/doctype/payment_request/payment_request.py b/erpnext/accounts/doctype/payment_request/payment_request.py index 4e12deb5097..70b28141cf6 100644 --- a/erpnext/accounts/doctype/payment_request/payment_request.py +++ b/erpnext/accounts/doctype/payment_request/payment_request.py @@ -784,6 +784,7 @@ def set_payment_references(payment_schedules): "description": row.get("description"), "due_date": row.get("due_date"), "amount": row.get("payment_amount"), + "currency": row.get("currency"), } ) diff --git a/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.py b/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.py index e861747803e..f1a7351508e 100644 --- a/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.py +++ b/erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.py @@ -99,9 +99,9 @@ class ProcessStatementOfAccounts(Document): if not self.pdf_name: self.pdf_name = "{{ customer.customer_name }}" - validate_template(self.subject) - validate_template(self.body) - validate_template(self.pdf_name) + validate_template(self.subject, restrict_globals=True) + validate_template(self.body, restrict_globals=True) + validate_template(self.pdf_name, restrict_globals=True) if not self.customers: frappe.throw(_("Customers not selected.")) @@ -527,15 +527,15 @@ def send_emails(document_name, from_scheduler=False, posting_date=None): if report: for customer, report_pdf in report.items(): context = get_context(customer, doc) - filename = frappe.render_template(doc.pdf_name, context) + filename = frappe.render_template(doc.pdf_name, context, restrict_globals=True) attachments = [{"fname": filename + ".pdf", "fcontent": report_pdf}] recipients, cc = get_recipients_and_cc(customer, doc) if not recipients: continue - subject = frappe.render_template(doc.subject, context) - message = frappe.render_template(doc.body, context) + subject = frappe.render_template(doc.subject, context, restrict_globals=True) + message = frappe.render_template(doc.body, context, restrict_globals=True) if doc.sender: sender_email = frappe.db.get_value("Email Account", doc.sender, "email_id") diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py index 0fc0303edb4..051c7d87519 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py @@ -3156,8 +3156,6 @@ def create_dunning(source_name, target_doc=None, ignore_permissions=False): from frappe.model.mapper import get_mapped_doc def postprocess_dunning(source, target): - from erpnext.accounts.doctype.dunning.dunning import get_dunning_letter_text - dunning_type = frappe.db.exists("Dunning Type", {"is_default": 1, "company": source.company}) if dunning_type: dunning_type = frappe.get_doc("Dunning Type", dunning_type) @@ -3166,14 +3164,8 @@ def create_dunning(source_name, target_doc=None, ignore_permissions=False): target.dunning_fee = dunning_type.dunning_fee target.income_account = dunning_type.income_account target.cost_center = dunning_type.cost_center - letter_text = get_dunning_letter_text( - dunning_type=dunning_type.name, doc=target.as_dict(), language=source.language - ) - - if letter_text: - target.body_text = letter_text.get("body_text") - target.closing_text = letter_text.get("closing_text") - target.language = letter_text.get("language") + target.language = source.language + target.get_dunning_letter_text() # update outstanding from doc if source.payment_schedule and len(source.payment_schedule) == 1: diff --git a/erpnext/buying/doctype/purchase_order_item/purchase_order_item.json b/erpnext/buying/doctype/purchase_order_item/purchase_order_item.json index daeb262ba7c..ebe58d1a484 100644 --- a/erpnext/buying/doctype/purchase_order_item/purchase_order_item.json +++ b/erpnext/buying/doctype/purchase_order_item/purchase_order_item.json @@ -923,8 +923,10 @@ "fieldname": "job_card", "fieldtype": "Link", "label": "Job Card", + "no_copy": 1, "options": "Job Card", - "search_index": 1 + "print_hide": 1, + "read_only": 1 }, { "fieldname": "distributed_discount_amount", @@ -951,7 +953,7 @@ "index_web_pages_for_search": 1, "istable": 1, "links": [], - "modified": "2026-05-14 12:16:16.192936", + "modified": "2026-07-15 10:30:04.600510", "modified_by": "Administrator", "module": "Buying", "name": "Purchase Order Item", diff --git a/erpnext/buying/doctype/supplier/supplier.py b/erpnext/buying/doctype/supplier/supplier.py index b7732856e99..ad594d2d72e 100644 --- a/erpnext/buying/doctype/supplier/supplier.py +++ b/erpnext/buying/doctype/supplier/supplier.py @@ -16,7 +16,10 @@ from erpnext.accounts.party import ( validate_party_accounts, validate_party_currency_before_merging, ) -from erpnext.controllers.website_list_for_contact import add_role_for_portal_user +from erpnext.controllers.website_list_for_contact import ( + add_role_for_portal_user, + link_portal_users_to_contacts, +) from erpnext.utilities.transaction_base import TransactionBase @@ -109,6 +112,7 @@ class Supplier(TransactionBase): def on_update(self): self.create_primary_contact() self.create_primary_address() + link_portal_users_to_contacts(self) def add_role_for_user(self): for portal_user in self.portal_users: diff --git a/erpnext/buying/doctype/supplier/test_supplier.py b/erpnext/buying/doctype/supplier/test_supplier.py index ecdf85a3f89..9659075fd54 100644 --- a/erpnext/buying/doctype/supplier/test_supplier.py +++ b/erpnext/buying/doctype/supplier/test_supplier.py @@ -203,3 +203,24 @@ class TestSupplierPortal(ERPNextTestSuite): _, suppliers = get_customers_suppliers("Purchase Order", user) self.assertIn(supplier.name, suppliers) + + def test_portal_user_contact_link(self): + user_email = frappe.generate_hash() + "@example.com" + user = frappe.new_doc("User") + user.email = user_email + user.first_name = "Test Portal Contact User" + user.send_welcome_email = False + user.insert(ignore_permissions=True) + + contact = frappe.new_doc("Contact") + contact.first_name = "Test Portal Contact User" + contact.add_email(user_email, is_primary=1) + contact.links = [] + contact.insert(ignore_permissions=True) + + supplier = create_supplier() + supplier.append("portal_users", {"user": user.name}) + supplier.save() + + contact.reload() + self.assertTrue(contact.has_link("Supplier", supplier.name)) diff --git a/erpnext/buying/doctype/supplier_quotation_item/supplier_quotation_item.json b/erpnext/buying/doctype/supplier_quotation_item/supplier_quotation_item.json index c131439463f..31efaa6690b 100644 --- a/erpnext/buying/doctype/supplier_quotation_item/supplier_quotation_item.json +++ b/erpnext/buying/doctype/supplier_quotation_item/supplier_quotation_item.json @@ -307,6 +307,7 @@ "fieldname": "net_rate", "fieldtype": "Currency", "label": "Net Rate", + "options": "currency", "print_hide": 1, "read_only": 1 }, @@ -613,7 +614,7 @@ "index_web_pages_for_search": 1, "istable": 1, "links": [], - "modified": "2025-06-17 12:05:52.441645", + "modified": "2026-07-15 10:33:24.855979", "modified_by": "Administrator", "module": "Buying", "name": "Supplier Quotation Item", diff --git a/erpnext/controllers/stock_controller.py b/erpnext/controllers/stock_controller.py index bd94076a2af..62384f3a85a 100644 --- a/erpnext/controllers/stock_controller.py +++ b/erpnext/controllers/stock_controller.py @@ -1347,66 +1347,63 @@ class StockController(AccountsController): if not batches: return - field_mapper = { - "Sales Invoice": [["Sales Order", "sales_order"]], - "Delivery Note": [["Sales Order", "against_sales_order"]], - "Stock Entry": [ - ["Work Order", "work_order"], - ["Subcontracting Inward Order", "subcontracting_inward_order"], - ], + reference_fields = { + "Sales Invoice": ["sales_order"], + "Delivery Note": ["against_sales_order"], + "Stock Entry": ["work_order", "subcontracting_inward_order"], }.get(self.doctype) - qty_field = { - "Sales Invoice": "qty", - "Delivery Note": "qty", - "Stock Entry": "fg_completed_qty", - }.get(self.doctype) - - reserved_batches_data = self.get_reserved_batches(batches) items = self.items if self.doctype == "Stock Entry": items = [self] - for item in items: - for field in field_mapper: - if not item.get(field[1]): - continue + own_vouchers = {item.get(field) for item in items for field in reference_fields if item.get(field)} - value = item.get(field[1]) - for row in reserved_batches_data: - if self.doctype in ["Sales Invoice", "Delivery Note"] and row.item_code != item.get( - "item_code" - ): - continue + outstanding_qty = defaultdict(float) + reservations = defaultdict(list) + for row in self.get_reserved_batches(batches): + if row.voucher_no in own_vouchers: + continue - if row.voucher_no == value: - continue + key = (row.batch_no, row.warehouse) + outstanding = flt(row.qty) - flt(row.delivered_qty) + outstanding_qty[key] += outstanding + if outstanding > 0: + reservations[key].append(row) - batch_qty = get_batch_qty( - row.batch_no, - row.warehouse, - posting_date=self.posting_date, - posting_time=self.posting_time, - consider_negative_batches=True, - ) + for (batch_no, warehouse), reserved_qty in outstanding_qty.items(): + if flt(reserved_qty, 6) <= 0: + continue - if item.get(qty_field) < batch_qty: - continue + batch_qty = get_batch_qty( + batch_no, + warehouse, + posting_date=self.posting_date, + posting_time=self.posting_time, + consider_negative_batches=True, + ) - frappe.throw( - _( - "The batch {0} is already reserved in {1} {2}. So, cannot proceed with the {3} {4}, which is created against the {5} {6}." - ).format( - frappe.bold(row.batch_no), - frappe.bold(row.voucher_type), - frappe.bold(row.voucher_no), - frappe.bold(self.doctype), - frappe.bold(self.name), - frappe.bold(field[0]), - frappe.bold(value), - ), - title=_("Reserved Batch Conflict"), - ) + if flt(batch_qty, 6) >= flt(reserved_qty, 6): + continue + + vouchers = ", ".join( + f"{frappe.bold(voucher_type)} {frappe.bold(voucher_no)}" + for voucher_type, voucher_no in dict.fromkeys( + (row.voucher_type, row.voucher_no) for row in reservations[(batch_no, warehouse)] + ) + ) + frappe.throw( + _( + "The batch {0} is reserved for {1} in the warehouse {2} and the remaining quantity is not enough to cover the reservations. So, cannot proceed with the {3} {4}." + ).format( + frappe.bold(batch_no), + vouchers, + frappe.bold(warehouse), + frappe.bold(self.doctype), + frappe.bold(self.name), + ), + title=_("Reserved Batch Conflict"), + ) def get_reserved_batches(self, batches): doctype = frappe.qb.DocType("Stock Reservation Entry") @@ -1418,9 +1415,10 @@ class StockController(AccountsController): .on(doctype.name == child_doc.parent) .select( child_doc.batch_no, + child_doc.qty, + child_doc.delivered_qty, doctype.voucher_type, doctype.voucher_no, - doctype.item_code, doctype.warehouse, ) .where((doctype.docstatus == 1) & (child_doc.batch_no.isin(batches))) @@ -2057,7 +2055,7 @@ class StockController(AccountsController): @frappe.whitelist() -def show_accounting_ledger_preview(company, doctype, docname): +def show_accounting_ledger_preview(company: str, doctype: str, docname: str): filters = frappe._dict(company=company, include_dimensions=1) doc = frappe.get_lazy_doc(doctype, docname) doc.check_permission("read") @@ -2071,7 +2069,7 @@ def show_accounting_ledger_preview(company, doctype, docname): @frappe.whitelist() -def show_stock_ledger_preview(company, doctype, docname): +def show_stock_ledger_preview(company: str, doctype: str, docname: str): filters = frappe._dict(company=company) doc = frappe.get_lazy_doc(doctype, docname) doc.check_permission("read") diff --git a/erpnext/controllers/website_list_for_contact.py b/erpnext/controllers/website_list_for_contact.py index 88eb325b47a..652c5429990 100644 --- a/erpnext/controllers/website_list_for_contact.py +++ b/erpnext/controllers/website_list_for_contact.py @@ -7,6 +7,8 @@ import json import frappe from frappe import _ from frappe.modules.utils import get_module_app +from frappe.query_builder import Criterion +from frappe.query_builder.functions import Lower from frappe.utils import cint, flt, has_common from frappe.utils.user import is_website_user @@ -306,3 +308,63 @@ def add_role_for_portal_user(portal_user, role): user_doc.add_roles(role) frappe.msgprint(_("Added {1} Role to User {0}.").format(frappe.bold(user_doc.name), role), alert=True) + + +def link_portal_users_to_contacts(doc): + """When portal users are added to Supplier/Customer, link them to the Contact profile.""" + # a User's name is its (lowercased) email, so portal_users are already the emails + portal_users = {p.user for p in doc.get("portal_users") or [] if p.user} + if not portal_users: + return + + before = doc.get_doc_before_save() + if before: + previous_users = {p.user for p in before.get("portal_users") or [] if p.user} + if portal_users == previous_users: + return + + portal_users = list(portal_users) + + contact = frappe.qb.DocType("Contact") + contact_email = frappe.qb.DocType("Contact Email") + + query = ( + frappe.qb.from_(contact) + .left_join(contact_email) + .on(contact_email.parent == contact.name) + .select(contact.name) + .distinct() + ) + + conditions = [ + contact.user.isin(portal_users), + Lower(contact.email_id).isin(portal_users), + Lower(contact_email.email_id).isin(portal_users), + ] + + query = query.where(Criterion.any(conditions)) + contacts = query.run(pluck=True) + + if not contacts: + return + + dynamic_link = frappe.qb.DocType("Dynamic Link") + existing_links = ( + frappe.qb.from_(dynamic_link) + .select(dynamic_link.parent) + .where( + (dynamic_link.parenttype == "Contact") + & (dynamic_link.parent.isin(contacts)) + & (dynamic_link.link_doctype == doc.doctype) + & (dynamic_link.link_name == doc.name) + ) + .run(pluck=True) + ) + + contacts_to_link = [name for name in contacts if name not in existing_links] + + for name in contacts_to_link: + contact_doc = frappe.get_doc("Contact", name) + if not contact_doc.has_link(doc.doctype, doc.name): + contact_doc.append("links", {"link_doctype": doc.doctype, "link_name": doc.name}) + contact_doc.save(ignore_permissions=True) diff --git a/erpnext/crm/doctype/appointment/appointment.json b/erpnext/crm/doctype/appointment/appointment.json index c600eb088c3..b7a92dba6d1 100644 --- a/erpnext/crm/doctype/appointment/appointment.json +++ b/erpnext/crm/doctype/appointment/appointment.json @@ -7,7 +7,11 @@ "engine": "InnoDB", "field_order": [ "scheduled_time", + "column_break_xaox", "status", + "created_through_portal", + "email_verified", + "verification_token", "customer_details_section", "customer_name", "customer_phone_number", @@ -54,7 +58,8 @@ "fieldtype": "Datetime", "in_list_view": 1, "label": "Scheduled Time", - "reqd": 1 + "reqd": 1, + "search_index": 1 }, { "fieldname": "status", @@ -77,8 +82,8 @@ "fieldname": "customer_email", "fieldtype": "Data", "label": "Email", - "reqd": 1, - "options": "Email" + "options": "Email", + "reqd": 1 }, { "fieldname": "linked_docs_section", @@ -100,13 +105,43 @@ "fieldtype": "Dynamic Link", "label": "Party", "options": "appointment_with" + }, + { + "default": "0", + "fieldname": "created_through_portal", + "fieldtype": "Check", + "label": "Created through Portal", + "read_only": 1, + "set_only_once": 1 + }, + { + "fieldname": "column_break_xaox", + "fieldtype": "Column Break" + }, + { + "default": "0", + "depends_on": "eval:doc.created_through_portal === 1;", + "fieldname": "email_verified", + "fieldtype": "Check", + "label": "Email Verified", + "read_only": 1 + }, + { + "fieldname": "verification_token", + "fieldtype": "Data", + "label": "Verification Token", + "hidden": 1, + "read_only": 1, + "no_copy": 1, + "search_index": 1 } ], "links": [], - "modified": "2026-06-06 13:05:59.300573", + "modified": "2026-07-20 02:00:00.000000", "modified_by": "Administrator", "module": "CRM", "name": "Appointment", + "naming_rule": "Expression (old style)", "owner": "Administrator", "permissions": [ { @@ -158,8 +193,9 @@ } ], "quick_entry": 1, + "row_format": "Dynamic", "sort_field": "creation", "sort_order": "DESC", "states": [], "track_changes": 1 -} \ No newline at end of file +} diff --git a/erpnext/crm/doctype/appointment/appointment.py b/erpnext/crm/doctype/appointment/appointment.py index 0f7c52688a3..da91a73f105 100644 --- a/erpnext/crm/doctype/appointment/appointment.py +++ b/erpnext/crm/doctype/appointment/appointment.py @@ -3,14 +3,20 @@ from collections import Counter +from datetime import timedelta +from urllib.parse import urlencode import frappe from frappe import _ from frappe.desk.form.assign_to import add as add_assignment from frappe.model.document import Document from frappe.share import add_docshare -from frappe.utils import get_url, getdate, now -from frappe.utils.verified_command import get_signed_params +from frappe.utils import add_to_date, cint, date_diff, get_datetime, get_url, getdate, now, now_datetime +from frappe.utils.data import sha256_hash + +from erpnext.setup.doctype.holiday_list.holiday_list import is_holiday + +WEEKDAYS = ["Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday", "Sunday"] class Appointment(Document): @@ -24,104 +30,227 @@ class Appointment(Document): appointment_with: DF.Link | None calendar_event: DF.Link | None + created_through_portal: DF.Check customer_details: DF.LongText | None customer_email: DF.Data customer_name: DF.Data customer_phone_number: DF.Data | None customer_skype: DF.Data | None + email_verified: DF.Check party: DF.DynamicLink | None scheduled_time: DF.Datetime status: DF.Literal["Open", "Unverified", "Closed"] + verification_token: DF.Data | None # end: auto-generated types - def find_lead_by_email(self): - lead_list = frappe.get_list( - "Lead", filters={"email_id": self.customer_email}, ignore_permissions=True - ) - if lead_list: - return lead_list[0].name - return None + def validate(self): + self.validate_status_update() + if not self.has_value_changed("scheduled_time"): + return - def find_customer_by_email(self): - customer_list = frappe.get_list( - "Customer", filters={"email_id": self.customer_email}, ignore_permissions=True + self.validate_backdated_booking() + + if is_appointment_scheduling_enabled(): + self.validate_advanced_booking() + self.validate_holiday() + self.validate_slot_timing() + + self.validate_available_time_slot() + + def validate_status_update(self): + if not self.has_value_changed("status"): + return + + if not self.created_through_portal: + if self.status == "Unverified": + frappe.throw(_("Appointments created manually cannot have 'Unverified' status.")) + return + + if self.status == "Unverified" and self.email_verified: + frappe.throw(_("A verified appointment cannot be moved back to 'Unverified' status.")) + + if self.status == "Open" and not self.email_verified: + frappe.throw( + _("An appointment booked through the portal can only be opened via email verification.") + ) + + def validate_backdated_booking(self): + if get_datetime(self.scheduled_time) < now_datetime(): + frappe.throw(_("Appointment cannot be scheduled for a past time.")) + + def validate_advanced_booking(self): + advance_booking_days = cint(get_booking_settings().advance_booking_days) + + if advance_booking_days and date_diff(self.scheduled_time, now_datetime()) > advance_booking_days: + frappe.throw( + _("Appointment can only be scheduled up to {0} day(s) in advance.").format( + advance_booking_days + ) + ) + + def validate_holiday(self): + holiday_list = get_booking_settings().holiday_list + + if not holiday_list: + frappe.throw(_("Please add a valid Holiday List on Appointment Booking Settings.")) + + if is_holiday(holiday_list, getdate(self.scheduled_time)): + frappe.throw(_("Appointment cannot be scheduled on a holiday.")) + + def validate_slot_timing(self): + settings = get_booking_settings() + if not settings.availability_of_slots: + frappe.throw(_("No availability of slots are found. Please add on Appointment Booking Settings.")) + + scheduled_time = get_datetime(self.scheduled_time) + day_of_week = WEEKDAYS[scheduled_time.weekday()] + slot_start = timedelta( + hours=scheduled_time.hour, minutes=scheduled_time.minute, seconds=scheduled_time.second ) - if customer_list: - return customer_list[0].name - return None + slot_end = slot_start + timedelta(minutes=cint(settings.appointment_duration)) + + for slot in settings.availability_of_slots: + if slot.day_of_week == day_of_week and slot.from_time <= slot_start and slot_end <= slot.to_time: + return + + frappe.throw(_("Appointment must be scheduled within the available slot timings.")) + + def validate_available_time_slot(self): + settings = get_booking_settings() + if not cint(settings.number_of_agents): + return + + # the locking read serializes concurrent bookings for the same window, + # so two simultaneous requests cannot both pass the capacity check + booked = count_overlapping_appointments( + self.scheduled_time, + cint(settings.appointment_duration), + exclude_appointment=self.name, + for_update=True, + ) + + if booked >= cint(settings.number_of_agents): + frappe.throw(_("Time slot is not available")) def before_insert(self): - number_of_appointments_in_same_slot = frappe.db.count( - "Appointment", filters={"scheduled_time": self.scheduled_time} - ) - number_of_agents = frappe.db.get_single_value("Appointment Booking Settings", "number_of_agents") - if number_of_agents != 0: - if number_of_appointments_in_same_slot >= number_of_agents: - frappe.throw(_("Time slot is not available")) - # Link lead - if not self.party: - lead = self.find_lead_by_email() - customer = self.find_customer_by_email() - if customer: - self.appointment_with = "Customer" - self.party = customer - else: - self.appointment_with = "Lead" - self.party = lead + # Set status to "Unverified" for new Appointments. + if self.created_through_portal: + self.status = "Unverified" + return + + self.link_customer_lead() def after_insert(self): - if self.party: - # Create Calendar event + if not self.created_through_portal and self.party: self.auto_assign() self.create_calendar_event() - else: - # Set status to unverified - self.db_set("status", "Unverified") - # Send email to confirm - self.send_confirmation_email() + return + + # Send email to confirm + self.send_confirmation_email() + + def on_update(self): + # capture transitions before nested saves during materialization + # refresh the before-save snapshot + status_changed = self.has_value_changed("status") + email_just_verified = bool( + self.created_through_portal and self.email_verified + ) and self.has_value_changed("email_verified") + + self.link_auto_assign_and_create_calendar_event() + + if email_just_verified: + self.send_appointment_confirmed_email() + + if status_changed: + self.update_event_and_assignments_status() + + def on_trash(self): + # the Event only references the party, not the appointment, + # so it must be cleaned up explicitly + if not self.calendar_event: + return + + event = self.calendar_event + self.db_set("calendar_event", None, update_modified=False) + frappe.delete_doc("Event", event, ignore_permissions=True) def send_confirmation_email(self): - verify_url = self._get_verify_url() - template = "confirm_appointment" - args = { - "link": verify_url, - "site_url": frappe.utils.get_url(), - "full_name": self.customer_name, - } + self.send_email_to_customer( + template="confirm_appointment", + subject=_("Appointment Confirmation"), + args={"link": self._get_verify_url(), "expiry_minutes": get_verification_link_expiry()}, + ) + frappe.msgprint(_("Please check your email to confirm the appointment.")) + + def send_appointment_confirmed_email(self): + self.send_email_to_customer( + template="appointment_confirmed", + subject=_("Appointment Confirmed"), + args={"scheduled_time": frappe.utils.format_datetime(self.scheduled_time)}, + reference_doctype="Appointment", + reference_name=self.name, + ) + + def send_email_to_customer(self, template, subject, args, **kwargs): frappe.sendmail( recipients=[self.customer_email], template=template, - args=args, - subject=_("Appointment Confirmation"), + args={"full_name": self.customer_name, "site_url": frappe.utils.get_url(), **args}, + subject=subject, + **kwargs, ) - if frappe.session.user == "Guest": - frappe.msgprint(_("Please check your email to confirm the appointment")) - else: - frappe.msgprint( - _("Appointment was created. But no lead was found. Please check the email to confirm") - ) - def on_change(self): - # Sync Calendar - if not self.calendar_event: + def link_auto_assign_and_create_calendar_event(self): + if self.is_new() or (self.created_through_portal and not self.email_verified): return + + if not self.calendar_event: + # first materialization: link the party, assign an agent, create the event + self.link_customer_lead() + self.auto_assign() + self.create_calendar_event() + + self.sync_calendar_event() + + def sync_calendar_event(self): + if not self.calendar_event or not self.has_value_changed("scheduled_time"): + return + cal_event = frappe.get_doc("Event", self.calendar_event) cal_event.starts_on = self.scheduled_time cal_event.save(ignore_permissions=True) - def set_verified(self, email): - if email != self.customer_email: - frappe.throw(_("Email verification failed.")) - # Create new lead + def update_event_and_assignments_status(self): + """Close or reopen the calendar event and assignments along with the appointment.""" + if self.status == "Unverified": + return + + is_closed = self.status == "Closed" + new_status = "Closed" if is_closed else "Open" + + if self.calendar_event: + frappe.db.set_value("Event", self.calendar_event, "status", new_status) + + # only move ToDos between Open and Closed - never touch Cancelled ones + todo_filters = { + "reference_type": "Appointment", + "reference_name": self.name, + "status": "Open" if is_closed else "Closed", + } + frappe.db.set_value("ToDo", todo_filters, "status", new_status) + + def link_customer_lead(self): + if not self.party: + customer = self.find_party_by_email("Customer") + self.appointment_with = "Customer" if customer else "Lead" + self.party = customer or self.find_party_by_email("Lead") + self.create_lead_and_link() - # Remove unverified status - self.status = "Open" - # Create calender event - self.auto_assign() - self.create_calendar_event() - self.save(ignore_permissions=True) - if not frappe.in_test: - frappe.db.commit() + + def find_party_by_email(self, doctype): + party = frappe.get_all(doctype, filters={"email_id": self.customer_email}, limit=1, pluck="name") + return party[0] if party else None def create_lead_and_link(self): # Return if already linked @@ -140,86 +269,39 @@ class Appointment(Document): if self.customer_details: lead.append( "notes", - { - "note": self.customer_details, - "added_by": frappe.session.user, - "added_on": now(), - }, + {"note": self.customer_details, "added_by": frappe.session.user, "added_on": now()}, ) - lead.insert(ignore_permissions=True) - - # Link lead - self.party = lead.name + self.party = lead.insert(ignore_permissions=True).name def auto_assign(self): - existing_assignee = self.get_assignee_from_latest_opportunity() - if existing_assignee: - # If the latest opportunity is assigned to someone - # Assign the appointment to the same - self.assign_agent(existing_assignee) - return if self._assign: return - available_agents = _get_agents_sorted_by_asc_workload(getdate(self.scheduled_time)) - for agent in available_agents: - if _check_agent_availability(agent, self.scheduled_time): - self.assign_agent(agent[0]) - break + + if existing_assignee := self.get_assignee_from_latest_opportunity(): + # assign to whoever handles the party's latest opportunity + self.assign_agent(existing_assignee) + return + + busy_agents = get_busy_agents(self.scheduled_time) + for agent in _get_agents_sorted_by_asc_workload(getdate(self.scheduled_time)): + if agent not in busy_agents: + self.assign_agent(agent) + break def get_assignee_from_latest_opportunity(self): - if not self.party: + if not self.party or not frappe.db.exists("Lead", self.party): return None - if not frappe.db.exists("Lead", self.party): - return None - opporutnities = frappe.get_list( + + opportunities = frappe.get_all( "Opportunity", - filters={ - "party_name": self.party, - }, - ignore_permissions=True, + filters={"party_name": self.party}, + fields=["_assign"], order_by="creation desc", + limit=1, ) - if not opporutnities: - return None - latest_opportunity = frappe.get_doc("Opportunity", opporutnities[0].name) - assignee = latest_opportunity._assign - if not assignee: - return None - assignee = frappe.parse_json(assignee)[0] - return assignee - - def create_calendar_event(self): - if self.calendar_event: - return - appointment_event = frappe.get_doc( - { - "doctype": "Event", - "subject": " ".join(["Appointment with", self.customer_name]), - "starts_on": self.scheduled_time, - "status": "Open", - "type": "Public", - "send_reminder": frappe.db.get_single_value( - "Appointment Booking Settings", "email_reminders" - ), - "event_participants": [ - dict(reference_doctype=self.appointment_with, reference_docname=self.party) - ], - } - ) - employee = _get_employee_from_user(self._assign) - if employee: - appointment_event.append( - "event_participants", dict(reference_doctype="Employee", reference_docname=employee.name) - ) - appointment_event.insert(ignore_permissions=True) - self.calendar_event = appointment_event.name - self.save(ignore_permissions=True) - - def _get_verify_url(self): - verify_route = "/book_appointment/verify" - params = {"email": self.customer_email, "appointment": self.name} - return get_url(verify_route + "?" + get_signed_params(params)) + assignees = opportunities and frappe.parse_json(opportunities[0]._assign or "[]") + return assignees[0] if assignees else None def assign_agent(self, agent): if not frappe.has_permission(doc=self, user=agent): @@ -227,45 +309,157 @@ class Appointment(Document): add_assignment({"doctype": self.doctype, "name": self.name, "assign_to": [agent]}) + def create_calendar_event(self): + if self.calendar_event: + return + + event = frappe.get_doc( + { + "doctype": "Event", + "subject": f"Appointment with {self.customer_name}", + "starts_on": self.scheduled_time, + "status": "Open", + "type": "Public", + "send_reminder": cint(get_booking_settings().email_reminders), + "event_participants": self.get_event_participants(), + } + ).insert(ignore_permissions=True) + + self.calendar_event = event.name + self.save(ignore_permissions=True) + + def get_event_participants(self): + participants = [dict(reference_doctype=self.appointment_with, reference_docname=self.party)] + + if employee := _get_employee_from_user(self._assign): + participants.append(dict(reference_doctype="Employee", reference_docname=employee.name)) + + return participants + + def _get_verify_url(self): + key = self.generate_verification_key() + return get_url("/book_appointment/verify?" + urlencode({"key": key})) + + def generate_verification_key(self): + # store only the hash; the raw key lives solely in the emailed link + key = frappe.generate_hash() + self.db_set("verification_token", sha256_hash(key), update_modified=False) + return key + + +def get_booking_settings(): + return frappe.get_cached_doc("Appointment Booking Settings") + + +def is_appointment_scheduling_enabled(): + return bool(cint(get_booking_settings().enable_scheduling)) + + +def get_verification_link_expiry(): + """Verification link expiry window in minutes.""" + return cint(get_booking_settings().verification_link_expiry_duration) + + +def count_overlapping_appointments( + scheduled_time, appointment_duration, exclude_appointment=None, for_update=False +): + """Count non-Closed appointments whose duration window overlaps `scheduled_time`. + With `for_update`, the range stays locked until commit, serializing concurrent bookings.""" + # select the rows (not COUNT) so `for_update` stays valid: PostgreSQL + # rejects `FOR UPDATE` combined with an aggregate function + appointment = frappe.qb.DocType("Appointment") + query = ( + frappe.qb.from_(appointment) + .select(appointment.name) + .where(appointment.scheduled_time > add_to_date(scheduled_time, minutes=-appointment_duration)) + .where(appointment.scheduled_time < add_to_date(scheduled_time, minutes=appointment_duration)) + .where(appointment.status != "Closed") + ) + + if exclude_appointment: + query = query.where(appointment.name != exclude_appointment) + + if for_update: + query = query.for_update() + + return len(query.run()) + + +def handle_expired_unverified_appointments(): + """Close or delete Unverified appointments whose verification link has expired.""" + expiry = get_verification_link_expiry() + if not expiry: + return + + cutoff = add_to_date(now_datetime(), minutes=-expiry) + filters = {"status": "Unverified", "creation": ("<", cutoff)} + action = get_booking_settings().action_for_expired_unverified_appointments or "Mark as Closed" + + if action == "Mark as Closed": + frappe.db.set_value("Appointment", filters, "status", "Closed") + elif action == "Delete Permanently": + for name in frappe.get_all("Appointment", filters=filters, pluck="name"): + frappe.delete_doc("Appointment", name, ignore_permissions=True) + def _get_agents_sorted_by_asc_workload(date): - appointments = frappe.get_all("Appointment", fields="*") - agent_list = _get_agent_list_as_strings() - if not appointments: - return agent_list - appointment_counter = Counter(agent_list) - for appointment in appointments: - assign_data = appointment._assign - if isinstance(assign_data, str): - assign_data = assign_data.strip() - if not assign_data: - continue - assigned_to = frappe.parse_json(assign_data) - if assigned_to and (assigned_to[0] in agent_list) and getdate(appointment.scheduled_time) == date: - appointment_counter[assigned_to[0]] += 1 - sorted_agent_list = appointment_counter.most_common() - sorted_agent_list.reverse() - return sorted_agent_list + # count only the given day's assignments; scheduled_time is indexed so the + # date range is resolved in SQL instead of scanning every appointment ever + workload = Counter(agent.user for agent in get_booking_settings().agent_list) + assigns = frappe.get_all( + "Appointment", + filters=[ + ["_assign", "is", "set"], + ["scheduled_time", ">=", getdate(date)], + ["scheduled_time", "<", add_to_date(getdate(date), days=1)], + ], + pluck="_assign", + ) + + for assign in assigns: + assignees = frappe.parse_json((assign or "").strip() or "[]") + if assignees and assignees[0] in workload: + workload[assignees[0]] += 1 + + return [agent for agent, _workload in reversed(workload.most_common())] -def _get_agent_list_as_strings(): - agent_list_as_strings = [] - agent_list = frappe.get_doc("Appointment Booking Settings").agent_list - for agent in agent_list: - agent_list_as_strings.append(agent.user) - return agent_list_as_strings +def get_busy_agents(scheduled_time): + """Agents already assigned to a non-Closed appointment overlapping `scheduled_time`.""" + duration = _get_appointment_duration() + assigns = frappe.get_all( + "Appointment", + filters=[ + ["scheduled_time", ">", add_to_date(scheduled_time, minutes=-duration)], + ["scheduled_time", "<", add_to_date(scheduled_time, minutes=duration)], + ["status", "!=", "Closed"], + ], + pluck="_assign", + ) + return {assignee for assign in assigns for assignee in frappe.parse_json(assign or "[]")} def _check_agent_availability(agent_email, scheduled_time): - appointemnts_at_scheduled_time = frappe.get_all("Appointment", filters={"scheduled_time": scheduled_time}) - for appointment in appointemnts_at_scheduled_time: - if appointment._assign == agent_email: - return False - return True + return agent_email not in get_busy_agents(scheduled_time) + + +def get_booked_slot_times(from_time, to_time): + """scheduled_times of non-Closed appointments within (from_time, to_time), for slot availability.""" + return frappe.get_all( + "Appointment", + filters=[ + ["scheduled_time", ">", from_time], + ["scheduled_time", "<", to_time], + ["status", "!=", "Closed"], + ], + pluck="scheduled_time", + ) + + +def _get_appointment_duration(): + return cint(get_booking_settings().appointment_duration) def _get_employee_from_user(user): employee_docname = frappe.db.get_value("Employee", {"user_id": user}) - if employee_docname: - return frappe.get_doc("Employee", employee_docname) - return None + return frappe.get_doc("Employee", employee_docname) if employee_docname else None diff --git a/erpnext/crm/doctype/appointment/test_appointment.py b/erpnext/crm/doctype/appointment/test_appointment.py index 24974ecf472..80c0ced648e 100644 --- a/erpnext/crm/doctype/appointment/test_appointment.py +++ b/erpnext/crm/doctype/appointment/test_appointment.py @@ -1,37 +1,167 @@ # Copyright (c) 2019, Frappe Technologies Pvt. Ltd. and Contributors # See license.txt import datetime -import unittest +from unittest.mock import patch +from urllib.parse import parse_qs, urlparse import frappe +from frappe.utils import add_to_date, getdate, now_datetime, set_request +from frappe.utils.data import sha256_hash +from erpnext.crm.doctype.appointment.appointment import ( + Appointment, + _check_agent_availability, + handle_expired_unverified_appointments, +) +from erpnext.setup.doctype.holiday_list.test_holiday_list import make_holiday_list from erpnext.tests.utils import ERPNextTestSuite +from erpnext.www.book_appointment.index import create_appointment, get_appointment_slots +from erpnext.www.book_appointment.verify import index as verify_index LEAD_EMAIL = "test_appointment_lead@example.com" +VERIFICATION_EXPIRY_MINUTES = 30 +ALL_WEEKDAYS = ["Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday", "Sunday"] -def create_test_appointment(): - test_appointment = frappe.get_doc( - { - "doctype": "Appointment", - "status": "Open", - "customer_name": "Test Lead", - "customer_phone_number": "666", - "customer_skype": "test", - "customer_email": LEAD_EMAIL, - "scheduled_time": datetime.datetime.now(), - "customer_details": "Hello, Friend!", - } - ) +def create_test_appointment(**kwargs): + args = { + "doctype": "Appointment", + "status": "Open", + "customer_name": "Test Lead", + "customer_phone_number": "666", + "customer_skype": "test", + "customer_email": LEAD_EMAIL, + "scheduled_time": add_to_date(now_datetime(), hours=2), + "customer_details": "Hello, Friend!", + } + args.update(kwargs) + test_appointment = frappe.get_doc(args) test_appointment.insert() return test_appointment +def create_lead(email, name="Existing Lead"): + frappe.db.delete("Lead", {"email_id": email}) + return frappe.get_doc({"doctype": "Lead", "lead_name": name, "email_id": email}).insert( + ignore_permissions=True + ) + + +def set_booking_setting(field, value): + frappe.db.set_single_value("Appointment Booking Settings", field, value) + + +def slot_on(days_from_now, hour, minute=0): + day = datetime.date.today() + datetime.timedelta(days=days_from_now) + return datetime.datetime.combine(day, datetime.time(hour, minute)) + + +def backdate_creation(appointment_name, minutes): + frappe.db.set_value( + "Appointment", + appointment_name, + "creation", + add_to_date(now_datetime(), minutes=-minutes), + update_modified=False, + ) + + +def get_status(appointment_name): + return frappe.db.get_value("Appointment", appointment_name, "status") + + +def get_assignees(appointment_name): + return frappe.parse_json(frappe.db.get_value("Appointment", appointment_name, "_assign") or "[]") + + +def get_todo_statuses(appointment_name): + return frappe.get_all( + "ToDo", + filters={"reference_type": "Appointment", "reference_name": appointment_name}, + pluck="status", + ) + + +def parse_verify_url(verify_url): + parsed = urlparse(verify_url) + return parsed, {key: value[0] for key, value in parse_qs(parsed.query).items()} + + class TestAppointment(ERPNextTestSuite): def setUp(self): + set_booking_setting("verification_link_expiry_duration", VERIFICATION_EXPIRY_MINUTES) frappe.db.delete("Lead", {"email_id": LEAD_EMAIL}) self.test_appointment = create_test_appointment() - self.test_appointment.set_verified(self.test_appointment.customer_email) + + def _configure_booking_settings(self, holiday_dates=None, agents=None): + holiday_list = make_holiday_list( + "_Test Appointment Holiday List", + from_date=getdate(), + to_date=add_to_date(getdate(), days=60), + holiday_dates=holiday_dates or [], + ) + + settings = frappe.get_doc("Appointment Booking Settings") + settings.enable_scheduling = 1 + settings.enable_appointment_portal = 1 + settings.appointment_duration = 30 + settings.advance_booking_days = 30 + settings.verification_link_expiry_duration = VERIFICATION_EXPIRY_MINUTES + settings.holiday_list = holiday_list.name + settings.set("agent_list", []) + for agent in agents or ["Administrator"]: + settings.append("agent_list", {"user": agent}) + settings.set("availability_of_slots", []) + for day in ALL_WEEKDAYS: + settings.append( + "availability_of_slots", {"day_of_week": day, "from_time": "09:00:00", "to_time": "17:00:00"} + ) + settings.save() + + def _create_portal_appointment(self, email, days_from_now=7, time="10:00:00"): + """Book as Guest. The verification email is mocked and kept on + ``self._verification_email_mock`` for assertions.""" + if not getattr(self, "_booking_settings_configured", False): + self._configure_booking_settings() + self._booking_settings_configured = True + + with self.set_user("Guest"), patch.object(Appointment, "send_confirmation_email") as mock_send: + appointment = create_appointment( + date=str(datetime.date.today() + datetime.timedelta(days=days_from_now)), + time=time, + tz="UTC", + contact={"name": "Portal Visitor", "email": email, "number": "123", "skype": "", "notes": ""}, + ) + self._verification_email_mock = mock_send + return appointment + + def _request_verification(self, appointment, verify_url=None): + """Simulate the GET request made by clicking the emailed verification link. + + The confirmation email sent on successful verification is mocked and kept + on ``self._confirmed_email_mock`` for assertions. + """ + parsed, params = parse_verify_url(verify_url or appointment._get_verify_url()) + + old_request = getattr(frappe.local, "request", None) + old_form_dict = frappe.local.form_dict + old_user = frappe.session.user + try: + # the real link is clicked by an anonymous visitor; set_user resets + # form_dict, so switch the user before populating the request + frappe.set_user("Guest") + set_request(method="GET", path=f"{parsed.path}?{parsed.query}") + frappe.local.form_dict = frappe._dict(params) + context = frappe._dict() + with patch.object(Appointment, "send_appointment_confirmed_email") as mock_confirmed: + verify_index.get_context(context) + self._confirmed_email_mock = mock_confirmed + return context + finally: + frappe.set_user(old_user) + frappe.local.request = old_request + frappe.local.form_dict = old_form_dict + frappe.local.flags.commit = False def test_calendar_event_created(self): cal_event = frappe.get_doc("Event", self.test_appointment.calendar_event) @@ -39,3 +169,371 @@ class TestAppointment(ERPNextTestSuite): def test_lead_linked(self): self.assertTrue(self.test_appointment.party) + + def test_desk_created_appointment_skips_email_verification(self): + """Appointments created from the desk (created_through_portal unset) must be + linked and confirmed immediately - no verification email should be sent.""" + with patch.object(Appointment, "send_confirmation_email") as mock_send: + appointment = create_test_appointment(customer_email="another_desk_lead@example.com") + + mock_send.assert_not_called() + self.assertEqual(appointment.status, "Open") + self.assertTrue(appointment.party) + frappe.db.delete("Lead", {"email_id": "another_desk_lead@example.com"}) + + def test_portal_booking_stays_unverified_for_existing_lead(self): + """A portal booking whose email matches an existing Lead/Customer must NOT + be auto-linked - it must stay Unverified until the email is confirmed.""" + create_lead("existing_lead@example.com") + appointment = self._create_portal_appointment("existing_lead@example.com", days_from_now=5) + + self._verification_email_mock.assert_called_once() + self.assertTrue(appointment.created_through_portal) + self.assertEqual(appointment.status, "Unverified") + self.assertFalse(appointment.email_verified) + self.assertFalse(appointment.party) + + def test_verify_url_uses_opaque_token(self): + appointment = self._create_portal_appointment("portal_visitor@example.com") + parsed, params = parse_verify_url(appointment._get_verify_url()) + + # the link carries only an opaque key - no email, name or signed params + self.assertEqual(set(params), {"key"}) + self.assertNotIn("email", parsed.query) + # only the hash of that key is stored on the appointment + stored = frappe.db.get_value("Appointment", appointment.name, "verification_token") + self.assertEqual(stored, sha256_hash(params["key"])) + + def test_email_verification_within_expiry_window(self): + # Link used within the validity window - verification succeeds and the + # appointment gets linked, assigned and added to the calendar + on_time = self._create_portal_appointment("portal_visitor_on_time@example.com") + context = self._request_verification(on_time) + + self.assertTrue(context.success) + self._confirmed_email_mock.assert_called_once() + on_time.reload() + self.assertEqual(on_time.status, "Open") + self.assertTrue(on_time.email_verified) + self.assertTrue(on_time.party) + self.assertTrue(on_time.calendar_event) + + # Link used after the validity window - verification fails + late = self._create_portal_appointment("portal_visitor_late@example.com", days_from_now=10) + after_expiry = add_to_date(now_datetime(), minutes=VERIFICATION_EXPIRY_MINUTES + 1) + with patch.object(verify_index, "now_datetime", return_value=after_expiry): + context = self._request_verification(late) + + self.assertFalse(context.success) + self._confirmed_email_mock.assert_not_called() + late.reload() + self.assertEqual(late.status, "Unverified") + self.assertFalse(late.email_verified) + self.assertFalse(late.party) + + def test_verification_link_reused_after_success(self): + appointment = self._create_portal_appointment("portal_visitor_twice@example.com") + verify_url = appointment._get_verify_url() + + context = self._request_verification(appointment, verify_url=verify_url) + self.assertTrue(context.success) + self._confirmed_email_mock.assert_called_once() + + # re-clicking the link is idempotent and does not send another email + context = self._request_verification(appointment, verify_url=verify_url) + self.assertTrue(context.success) + self.assertIn("already verified", context.message) + self._confirmed_email_mock.assert_not_called() + + def test_verification_link_for_deleted_appointment(self): + """A verification link can outlive its appointment - clicking it must + render a friendly message, not crash.""" + appointment = self._create_portal_appointment("portal_visitor_gone@example.com") + verify_url = appointment._get_verify_url() + frappe.delete_doc("Appointment", appointment.name, ignore_permissions=True) + + context = self._request_verification(appointment, verify_url=verify_url) + + self.assertFalse(context.success) + self.assertIn("book the appointment again", context.message) + + def test_reschedule_syncs_calendar_event(self): + new_time = add_to_date(self.test_appointment.scheduled_time, hours=1) + self.test_appointment.scheduled_time = new_time + self.test_appointment.save() + + starts_on = frappe.db.get_value("Event", self.test_appointment.calendar_event, "starts_on") + self.assertEqual(starts_on, new_time) + + def test_portal_endpoint_disabled(self): + self._configure_booking_settings() + set_booking_setting("enable_appointment_portal", 0) + + with self.set_user("Guest"), self.assertRaises(frappe.Redirect): + create_appointment( + date=str(datetime.date.today() + datetime.timedelta(days=3)), + time="10:00:00", + tz="UTC", + contact={ + "name": "Blocked", + "email": "blocked@example.com", + "number": "1", + "skype": "", + "notes": "", + }, + ) + + def test_booked_slot_unavailable_on_portal(self): + from frappe.utils.data import get_system_timezone + + self._configure_booking_settings() + tz = get_system_timezone() + day = datetime.date.today() + datetime.timedelta(days=2) + + def get_availability(): + with self.set_user("Guest"): + slots = get_appointment_slots(str(day), tz) + return {slot["time"].strftime("%H:%M"): slot["availability"] for slot in slots} + + booked = create_test_appointment( + customer_email="slot_taken@example.com", scheduled_time=slot_on(2, 10) + ) + + availability = get_availability() + self.assertFalse(availability["10:00"]) + self.assertTrue(availability["13:00"]) + + # closing the appointment frees its slot on the portal + booked.status = "Closed" + booked.save() + self.assertTrue(get_availability()["10:00"]) + + # an off-grid desk appointment blocks every portal slot it overlaps + create_test_appointment(customer_email="off_grid@example.com", scheduled_time=slot_on(2, 13, 15)) + availability = get_availability() + self.assertFalse(availability["13:00"]) + self.assertFalse(availability["13:30"]) + self.assertTrue(availability["14:00"]) + + def test_expired_unverified_appointments_are_closed(self): + stale = self._create_portal_appointment("portal_visitor_stale@example.com", days_from_now=8) + fresh = self._create_portal_appointment("portal_visitor_fresh@example.com", days_from_now=9) + verify_url = stale._get_verify_url() + + backdate_creation(stale.name, VERIFICATION_EXPIRY_MINUTES + 15) + set_booking_setting("action_for_expired_unverified_appointments", "Mark as Closed") + + handle_expired_unverified_appointments() + + self.assertEqual(get_status(stale.name), "Closed") + self.assertEqual(get_status(fresh.name), "Unverified") + # Open appointments are never touched, regardless of age + self.assertEqual(get_status(self.test_appointment.name), "Open") + + # clicking the link of a closed appointment renders a friendly message + context = self._request_verification(stale, verify_url=verify_url) + self.assertFalse(context.success) + self.assertIn("closed", context.message) + + def test_expired_unverified_appointments_are_deleted(self): + stale = self._create_portal_appointment("portal_visitor_purged@example.com", days_from_now=8) + fresh = self._create_portal_appointment("portal_visitor_kept@example.com", days_from_now=9) + + backdate_creation(stale.name, VERIFICATION_EXPIRY_MINUTES + 15) + set_booking_setting("action_for_expired_unverified_appointments", "Delete Permanently") + + handle_expired_unverified_appointments() + + self.assertFalse(frappe.db.exists("Appointment", stale.name)) + self.assertTrue(frappe.db.exists("Appointment", fresh.name)) + self.assertTrue(frappe.db.exists("Appointment", self.test_appointment.name)) + + def test_cleanup_skipped_when_expiry_not_configured(self): + appointment = self._create_portal_appointment("portal_visitor_no_expiry@example.com") + backdate_creation(appointment.name, 5) + set_booking_setting("verification_link_expiry_duration", 0) + + handle_expired_unverified_appointments() + + self.assertEqual(get_status(appointment.name), "Unverified") + + def test_status_transition_rules(self): + # desk appointments can never be Unverified + with self.assertRaises(frappe.ValidationError): + create_test_appointment(customer_email="desk_unverified@example.com", status="Unverified") + + # portal appointments cannot be opened manually before verification + unverified = self._create_portal_appointment("manual_open@example.com") + unverified.status = "Open" + with self.assertRaises(frappe.ValidationError): + unverified.save(ignore_permissions=True) + + # verified appointments cannot be reverted to Unverified + verified = self._create_portal_appointment("revert_unverified@example.com", days_from_now=8) + self._request_verification(verified) + verified.reload() + verified.status = "Unverified" + with self.assertRaises(frappe.ValidationError): + verified.save(ignore_permissions=True) + + # both desk and verified portal appointments can be closed and reopened + for appointment in (self.test_appointment, verified): + appointment.reload() + appointment.status = "Closed" + appointment.save(ignore_permissions=True) + appointment.status = "Open" + appointment.save(ignore_permissions=True) + self.assertEqual(appointment.status, "Open") + + def test_agent_auto_assignment(self): + agent_email = "appointment_agent@example.com" + if not frappe.db.exists("User", agent_email): + frappe.get_doc( + {"doctype": "User", "email": agent_email, "first_name": "Appointment Agent"} + ).insert(ignore_permissions=True) + + self._configure_booking_settings(agents=["Administrator", agent_email]) + first = create_test_appointment( + customer_email="assigned_one@example.com", scheduled_time=slot_on(2, 11) + ) + second = create_test_appointment( + customer_email="assigned_two@example.com", scheduled_time=slot_on(2, 11) + ) + + # both appointments in the same slot get an agent, and never the same one + self.assertTrue(get_assignees(first.name)) + self.assertTrue(get_assignees(second.name)) + self.assertNotEqual(get_assignees(first.name), get_assignees(second.name)) + + # closing an assigned appointment closes its ToDo without re-assigning + first.reload() + first.status = "Closed" + first.save() + self.assertTrue(get_todo_statuses(first.name)) + self.assertTrue(all(status == "Closed" for status in get_todo_statuses(first.name))) + + # reopening brings the ToDos back + first.status = "Open" + first.save() + self.assertTrue(all(status == "Open" for status in get_todo_statuses(first.name))) + + def test_agent_busy_for_the_whole_appointment_duration(self): + self._configure_booking_settings() + slot = slot_on(3, 11) + appointment = create_test_appointment(customer_email="busy_agent@example.com", scheduled_time=slot) + assignee = get_assignees(appointment.name)[0] + + # busy anywhere inside the 30-minute appointment window, free right after it + self.assertFalse(_check_agent_availability(assignee, slot)) + self.assertFalse(_check_agent_availability(assignee, slot + datetime.timedelta(minutes=15))) + self.assertTrue(_check_agent_availability(assignee, slot + datetime.timedelta(minutes=30))) + + def test_closed_appointment_closes_calendar_event(self): + self.test_appointment.status = "Closed" + self.test_appointment.save() + event_status = frappe.db.get_value("Event", self.test_appointment.calendar_event, "status") + self.assertEqual(event_status, "Closed") + + # reopening the appointment reopens the calendar event + self.test_appointment.status = "Open" + self.test_appointment.save() + event_status = frappe.db.get_value("Event", self.test_appointment.calendar_event, "status") + self.assertEqual(event_status, "Open") + + def test_deleting_appointment_deletes_calendar_event(self): + event = self.test_appointment.calendar_event + self.assertTrue(frappe.db.exists("Event", event)) + + frappe.delete_doc("Appointment", self.test_appointment.name) + + self.assertFalse(frappe.db.exists("Event", event)) + + def test_backdated_appointment_is_rejected(self): + with self.assertRaises(frappe.ValidationError): + create_test_appointment( + customer_email="backdated@example.com", + scheduled_time=add_to_date(now_datetime(), hours=-1), + ) + + def test_booking_beyond_advance_window_is_rejected(self): + self._configure_booking_settings() + set_booking_setting("advance_booking_days", 7) + + # within the advance booking window - allowed + within = create_test_appointment( + customer_email="advance_within@example.com", scheduled_time=slot_on(5, 10) + ) + self.assertTrue(frappe.db.exists("Appointment", within.name)) + + # beyond the advance booking window - rejected + with self.assertRaises(frappe.ValidationError): + create_test_appointment( + customer_email="advance_beyond@example.com", scheduled_time=slot_on(8, 10) + ) + + def test_appointment_on_holiday_is_rejected(self): + holiday = add_to_date(getdate(), days=3) + self._configure_booking_settings( + holiday_dates=[{"holiday_date": holiday, "description": "Test Holiday"}] + ) + + with self.assertRaises(frappe.ValidationError): + create_test_appointment(customer_email="on_holiday@example.com", scheduled_time=slot_on(3, 10)) + + # the day after the holiday is bookable + after_holiday = create_test_appointment( + customer_email="after_holiday@example.com", scheduled_time=slot_on(4, 10) + ) + self.assertTrue(frappe.db.exists("Appointment", after_holiday.name)) + + def test_appointment_outside_slot_timing_is_rejected(self): + self._configure_booking_settings() + + # before the slot opens + with self.assertRaises(frappe.ValidationError): + create_test_appointment(customer_email="before_opening@example.com", scheduled_time=slot_on(2, 8)) + + # starts within the slot but would end after it closes + with self.assertRaises(frappe.ValidationError): + create_test_appointment( + customer_email="past_closing@example.com", scheduled_time=slot_on(2, 16, 45) + ) + + # within the slot timings + within = create_test_appointment( + customer_email="within_slot@example.com", scheduled_time=slot_on(2, 10) + ) + self.assertTrue(frappe.db.exists("Appointment", within.name)) + + def test_overlapping_time_slot_capacity(self): + set_booking_setting("number_of_agents", 1) + set_booking_setting("appointment_duration", 30) + + slot = slot_on(1, 10) + first = create_test_appointment(customer_email="slot_first@example.com", scheduled_time=slot) + + # a booking starting inside the first appointment's duration is rejected + with self.assertRaises(frappe.ValidationError): + create_test_appointment( + customer_email="slot_overlap@example.com", + scheduled_time=slot + datetime.timedelta(minutes=15), + ) + + # rescheduling must not count the appointment's own booked slot + first.scheduled_time = slot + datetime.timedelta(minutes=10) + first.save() + + # a booking starting exactly when the rescheduled one ends is allowed + adjacent = create_test_appointment( + customer_email="slot_adjacent@example.com", + scheduled_time=slot + datetime.timedelta(minutes=40), + ) + self.assertTrue(frappe.db.exists("Appointment", adjacent.name)) + + # a closed (cancelled) appointment frees its slot + first.status = "Closed" + first.save() + after_cancellation = create_test_appointment( + customer_email="after_cancellation@example.com", scheduled_time=slot + ) + self.assertTrue(frappe.db.exists("Appointment", after_cancellation.name)) diff --git a/erpnext/crm/doctype/appointment_booking_settings/appointment_booking_settings.json b/erpnext/crm/doctype/appointment_booking_settings/appointment_booking_settings.json index b79e974e301..8557dcf8791 100644 --- a/erpnext/crm/doctype/appointment_booking_settings/appointment_booking_settings.json +++ b/erpnext/crm/doctype/appointment_booking_settings/appointment_booking_settings.json @@ -1,48 +1,56 @@ { "actions": [], + "allow_bulk_edit": 1, "creation": "2019-08-27 10:56:48.309824", "doctype": "DocType", "editable_grid": 1, "engine": "InnoDB", "field_order": [ - "enable_scheduling", - "agent_detail_section", - "availability_of_slots", - "number_of_agents", - "agent_list", - "holiday_list", "appointment_details_section", "appointment_duration", "email_reminders", + "column_break_ehiq", + "agent_list", + "number_of_agents", + "agent_detail_section", + "enable_scheduling", + "availability_of_slots", + "section_break_bkln", + "column_break_alwa", "advance_booking_days", + "column_break_bspp", + "holiday_list", "success_details", - "success_redirect_url" + "enable_appointment_portal", + "verification_link_expiry_duration", + "column_break_fovk", + "success_redirect_url", + "action_for_expired_unverified_appointments" ], "fields": [ { + "depends_on": "eval:doc.enable_scheduling === 1;", "fieldname": "availability_of_slots", "fieldtype": "Table", "label": "Availability Of Slots", - "options": "Appointment Booking Slots", - "reqd": 1 + "mandatory_depends_on": "eval:doc.enable_scheduling === 1;", + "options": "Appointment Booking Slots" }, { - "default": "1", "fieldname": "number_of_agents", "fieldtype": "Int", - "hidden": 1, "in_list_view": 1, "label": "Number of Concurrent Appointments", - "read_only": 1, - "reqd": 1 + "read_only": 1 }, { + "depends_on": "eval:doc.enable_scheduling === 1;", "fieldname": "holiday_list", "fieldtype": "Link", "in_list_view": 1, "label": "Holiday List", - "options": "Holiday List", - "reqd": 1 + "mandatory_depends_on": "eval:doc.enable_scheduling === 1;", + "options": "Holiday List" }, { "default": "60", @@ -60,29 +68,31 @@ }, { "default": "7", + "depends_on": "eval:doc.enable_scheduling === 1;", "fieldname": "advance_booking_days", "fieldtype": "Int", "label": "Number of days appointments can be booked in advance", - "reqd": 1 + "mandatory_depends_on": "eval:doc.enable_scheduling === 1;" }, { "fieldname": "agent_list", "fieldtype": "Table MultiSelect", "label": "Agents", - "options": "Assignment Rule User", - "reqd": 1 + "mandatory_depends_on": "eval:doc.enable_scheduling === 1;", + "options": "Assignment Rule User" }, { "default": "0", "fieldname": "enable_scheduling", "fieldtype": "Check", "label": "Enable Appointment Scheduling", - "reqd": 1 + "mandatory_depends_on": "eval:doc.enable_appointment_portal === 1;" }, { "fieldname": "agent_detail_section", "fieldtype": "Section Break", - "label": "Agent Details" + "hide_border": 1, + "label": "Appointment Scheduling" }, { "fieldname": "appointment_details_section", @@ -92,20 +102,68 @@ { "fieldname": "success_details", "fieldtype": "Section Break", - "label": "Success Settings" + "label": "Appointment Booking Portal Settings" }, { "description": "Leave blank for home.\nThis is relative to site URL, for example \"about\" will redirect to \"https://yoursitename.com/about\"", "fieldname": "success_redirect_url", "fieldtype": "Data", - "label": "Success Redirect URL" + "label": "Success Redirect URL", + "permlevel": 1 + }, + { + "default": "30", + "depends_on": "eval: doc.enable_scheduling === 1;", + "description": "In Minutes (min: 15 mins, max: 60 mins)", + "fieldname": "verification_link_expiry_duration", + "fieldtype": "Int", + "label": "Verification Link Expiry Duration", + "mandatory_depends_on": "eval:doc.enable_appointment_portal === 1;", + "max_value": 60.0, + "min_value": 15.0, + "non_negative": 1, + "permlevel": 1 + }, + { + "fieldname": "column_break_ehiq", + "fieldtype": "Column Break" + }, + { + "default": "0", + "fieldname": "enable_appointment_portal", + "fieldtype": "Check", + "label": "Enable Appointment Booking Through Portal", + "permlevel": 1 + }, + { + "fieldname": "column_break_fovk", + "fieldtype": "Column Break" + }, + { + "default": "Mark as Closed", + "fieldname": "action_for_expired_unverified_appointments", + "fieldtype": "Select", + "label": "Action for Expired Unverified Appointments", + "options": "Mark as Closed\nDelete Permanently", + "permlevel": 1 + }, + { + "fieldname": "section_break_bkln", + "fieldtype": "Section Break" + }, + { + "fieldname": "column_break_alwa", + "fieldtype": "Column Break" + }, + { + "fieldname": "column_break_bspp", + "fieldtype": "Column Break" } ], "grid_page_length": 50, - "hide_toolbar": 0, "issingle": 1, "links": [], - "modified": "2026-03-16 13:28:21.198138", + "modified": "2026-07-20 00:11:18.996384", "modified_by": "Administrator", "module": "CRM", "name": "Appointment Booking Settings", @@ -139,6 +197,15 @@ "role": "Sales Manager", "share": 1, "write": 1 + }, + { + "email": 1, + "permlevel": 1, + "print": 1, + "read": 1, + "role": "System Manager", + "share": 1, + "write": 1 } ], "quick_entry": 1, diff --git a/erpnext/crm/doctype/appointment_booking_settings/appointment_booking_settings.py b/erpnext/crm/doctype/appointment_booking_settings/appointment_booking_settings.py index 9ef01283c31..67aab6fe8c9 100644 --- a/erpnext/crm/doctype/appointment_booking_settings/appointment_booking_settings.py +++ b/erpnext/crm/doctype/appointment_booking_settings/appointment_booking_settings.py @@ -3,11 +3,11 @@ import datetime -import typing import frappe from frappe import _ from frappe.model.document import Document +from frappe.utils import getdate class AppointmentBookingSettings(Document): @@ -24,33 +24,43 @@ class AppointmentBookingSettings(Document): AppointmentBookingSlots, ) + action_for_expired_unverified_appointments: DF.Literal["Mark as Closed", "Delete Permanently"] advance_booking_days: DF.Int agent_list: DF.TableMultiSelect[AssignmentRuleUser] appointment_duration: DF.Int availability_of_slots: DF.Table[AppointmentBookingSlots] email_reminders: DF.Check + enable_appointment_portal: DF.Check enable_scheduling: DF.Check - holiday_list: DF.Link + holiday_list: DF.Link | None number_of_agents: DF.Int success_redirect_url: DF.Data | None + verification_link_expiry_duration: DF.Int # end: auto-generated types - agent_list: typing.ClassVar[list] = [] # Hack - min_date = "01/01/1970 " - format_string = "%d/%m/%Y %H:%M:%S" - def validate(self): - self.validate_availability_of_slots() - - def save(self): self.number_of_agents = len(self.agent_list) - super().save() + self.validate_appointment_scheduling() + self.validate_portal_booking() + + def validate_appointment_scheduling(self): + if not self.enable_scheduling: + return + + self.validate_availability_of_slots() + self.validate_holiday_list() + self.validate_advance_booking_days() def validate_availability_of_slots(self): + if not self.availability_of_slots: + frappe.throw( + _("Please fill up the Availability of Slots table to enable Appointment Scheduling.") + ) + + format_string = "%Y-%m-%d %H:%M:%S" for record in self.availability_of_slots: - from_time = datetime.datetime.strptime(self.min_date + record.from_time, self.format_string) - to_time = datetime.datetime.strptime(self.min_date + record.to_time, self.format_string) - to_time - from_time + from_time = datetime.datetime.strptime(f"1970-01-01 {record.from_time}", format_string) + to_time = datetime.datetime.strptime(f"1970-01-01 {record.to_time}", format_string) self.validate_from_and_to_time(from_time, to_time, record) self.duration_is_divisible(from_time, to_time) @@ -65,3 +75,38 @@ class AppointmentBookingSettings(Document): timedelta = to_time - from_time if timedelta.total_seconds() % (self.appointment_duration * 60): frappe.throw(_("The difference between from time and To Time must be a multiple of Appointment")) + + def validate_holiday_list(self): + if not self.holiday_list: + frappe.throw(_("Please select a Holiday List to enable Appointment Scheduling.")) + + hl_from_date, hl_to_date = frappe.get_cached_value( + "Holiday List", self.holiday_list, ["from_date", "to_date"] + ) + now = getdate() + + if not (now >= hl_from_date and now <= hl_to_date): + frappe.throw(_("Holiday List - {0} is not valid for current date.").format(self.holiday_list)) + + def validate_advance_booking_days(self): + if not self.advance_booking_days: + frappe.throw(_("Advance Booking Days is mandatory for Appointment Scheduling.")) + + def validate_portal_booking(self): + if not self.enable_appointment_portal: + return + + if not self.enable_scheduling: + frappe.throw( + _("Appointment Scheduling needs to be enabled for Appointment Booking through portal.") + ) + + self.validate_link_expiry_duration() + + def validate_link_expiry_duration(self): + if ( + not self.verification_link_expiry_duration + or self.verification_link_expiry_duration > 60 + or self.verification_link_expiry_duration < 15 + ): + frappe.throw(_("'Verification Link Expiry Duration' must be between 15 to 60 minutes.")) diff --git a/erpnext/crm/doctype/appointment_booking_settings/test_appointment_booking_settings.py b/erpnext/crm/doctype/appointment_booking_settings/test_appointment_booking_settings.py index 96d86a224ed..ae121ab6883 100644 --- a/erpnext/crm/doctype/appointment_booking_settings/test_appointment_booking_settings.py +++ b/erpnext/crm/doctype/appointment_booking_settings/test_appointment_booking_settings.py @@ -1,10 +1,125 @@ # Copyright (c) 2019, Frappe Technologies Pvt. Ltd. and Contributors # See license.txt -# import frappe -import unittest +import datetime + +import frappe +from frappe.utils import add_to_date, getdate + +from erpnext.setup.doctype.holiday_list.test_holiday_list import make_holiday_list from erpnext.tests.utils import ERPNextTestSuite class TestAppointmentBookingSettings(ERPNextTestSuite): - pass + def assert_invalid(self, settings): + with self.assertRaises(frappe.ValidationError): + settings.save() + + def make_settings(self, appointment_duration=30): + doc = frappe.new_doc("Appointment Booking Settings") + doc.appointment_duration = appointment_duration + return doc + + def dt(self, hms): + # the controller parses times against a fixed epoch date + return datetime.datetime.strptime("1970-01-01 " + hms, "%Y-%m-%d %H:%M:%S") + + def get_valid_scheduling_settings(self): + holiday_list = make_holiday_list( + "_Test Booking Settings Holiday List", + from_date=getdate(), + to_date=add_to_date(getdate(), days=30), + holiday_dates=[], + ) + + settings = frappe.get_doc("Appointment Booking Settings") + settings.enable_scheduling = 1 + settings.appointment_duration = 30 + settings.advance_booking_days = 7 + settings.verification_link_expiry_duration = 30 + settings.holiday_list = holiday_list.name + settings.set("agent_list", []) + settings.append("agent_list", {"user": "Administrator"}) + settings.set("availability_of_slots", []) + settings.append( + "availability_of_slots", + {"day_of_week": "Monday", "from_time": "09:00:00", "to_time": "17:00:00"}, + ) + return settings + + def test_from_time_must_precede_to_time(self): + doc = self.make_settings() + record = frappe._dict(day_of_week="Monday") + self.assertRaises( + frappe.ValidationError, + doc.validate_from_and_to_time, + self.dt("18:00:00"), + self.dt("09:00:00"), + record, + ) + doc.validate_from_and_to_time(self.dt("09:00:00"), self.dt("18:00:00"), record) # valid order + + def test_slot_length_must_be_a_multiple_of_the_duration(self): + doc = self.make_settings(appointment_duration=30) + # 60 minutes is two 30-minute appointments -> fine + doc.duration_is_divisible(self.dt("09:00:00"), self.dt("10:00:00")) + # 45 minutes leaves a partial appointment -> rejected + self.assertRaises( + frappe.ValidationError, doc.duration_is_divisible, self.dt("09:00:00"), self.dt("09:45:00") + ) + + def test_scheduling_requires_slots(self): + settings = self.get_valid_scheduling_settings() + settings.set("availability_of_slots", []) + + self.assert_invalid(settings) + + def test_validate_checks_every_slot(self): + settings = self.get_valid_scheduling_settings() + settings.append( + "availability_of_slots", + {"day_of_week": "Tuesday", "from_time": "09:00:00", "to_time": "09:45:00"}, + ) + + self.assert_invalid(settings) + + def test_scheduling_requires_holiday_list_covering_today(self): + settings = self.get_valid_scheduling_settings() + settings.holiday_list = None + self.assert_invalid(settings) + + expired_list = make_holiday_list( + "_Test Booking Settings Expired Holiday List", + from_date=add_to_date(getdate(), days=-60), + to_date=add_to_date(getdate(), days=-30), + holiday_dates=[], + ) + settings.holiday_list = expired_list.name + self.assert_invalid(settings) + + def test_scheduling_requires_advance_booking_days(self): + settings = self.get_valid_scheduling_settings() + settings.advance_booking_days = 0 + + self.assert_invalid(settings) + + def test_portal_requires_scheduling(self): + settings = frappe.get_doc("Appointment Booking Settings") + settings.enable_scheduling = 0 + settings.enable_appointment_portal = 1 + + self.assert_invalid(settings) + + def test_portal_expiry_duration_bounds(self): + settings = self.get_valid_scheduling_settings() + settings.enable_appointment_portal = 1 + settings.verification_link_expiry_duration = 5 + + self.assert_invalid(settings) + + def test_number_of_agents_derived_from_agent_list(self): + settings = self.get_valid_scheduling_settings() + settings.number_of_agents = 99 + settings.save() + + self.assertEqual(frappe.db.get_single_value("Appointment Booking Settings", "number_of_agents"), 1) diff --git a/erpnext/hooks.py b/erpnext/hooks.py index d614d8b6356..4ce4e8047f6 100644 --- a/erpnext/hooks.py +++ b/erpnext/hooks.py @@ -433,8 +433,6 @@ scheduler_events = { "cron": { "0/15 * * * *": [ "erpnext.manufacturing.doctype.bom_update_log.bom_update_log.resume_bom_cost_update_jobs", - ], - "0/30 * * * *": [ "erpnext.stock.doctype.repost_item_valuation.repost_item_valuation.run_parallel_reposting", ], # Hourly but offset by 30 minutes @@ -449,6 +447,7 @@ scheduler_events = { ], "hourly_long": [], "hourly_maintenance": [ + "erpnext.crm.doctype.appointment.appointment.handle_expired_unverified_appointments", "erpnext.stock.doctype.repost_item_valuation.repost_item_valuation.repost_entries", "erpnext.utilities.bulk_transaction.retry", "erpnext.projects.doctype.project.project.collect_project_status", diff --git a/erpnext/locale/ar.po b/erpnext/locale/ar.po index 19fd9891776..de2b6e19eea 100644 --- a/erpnext/locale/ar.po +++ b/erpnext/locale/ar.po @@ -1,28 +1,36 @@ - msgid "" msgstr "" -"Project-Id-Version: frappe\n" +"Project-Id-Version: frappe\n" "Report-Msgid-Bugs-To: hello@frappe.io\n" -"POT-Creation-Date: 2026-07-05 10:19+0000\n" -"PO-Revision-Date: 2026-07-06 11:32+0000\n" +"POT-Creation-Date: 2026-07-12 10:05+0000\n" +"PO-Revision-Date: 2026-07-16 13:10\n" "Last-Translator: hello@frappe.io\n" -"Language: ar_SA\n" "Language-Team: Arabic\n" -"Plural-Forms: nplurals=6; plural=(n==0 ? 0 : n==1 ? 1 : n==2 ? 2 : n%100>=3 && n%100<=10 ? 3 : n%100>=11 && n%100<=99 ? 4 : 5);\n" "MIME-Version: 1.0\n" -"Content-Type: text/plain; charset=utf-8\n" +"Content-Type: text/plain; charset=UTF-8\n" "Content-Transfer-Encoding: 8bit\n" "Generated-By: Babel 2.16.0\n" +"Plural-Forms: nplurals=6; plural=(n==0 ? 0 : n==1 ? 1 : n==2 ? 2 : n%100>=3 && n%100<=10 ? 3 : n%100>=11 && n%100<=99 ? 4 : 5);\n" +"X-Crowdin-Project: frappe\n" +"X-Crowdin-Project-ID: 639578\n" +"X-Crowdin-Language: ar\n" +"X-Crowdin-File: /[frappe.erpnext] version-16-hotfix/erpnext/locale/main.pot\n" +"X-Crowdin-File-ID: 169\n" +"Language: ar_SA\n" -#: erpnext/stock/doctype/serial_and_batch_bundle/serial_and_batch_bundle.py:1641 -msgid "" -"\n" +#: erpnext/stock/doctype/serial_and_batch_bundle/serial_and_batch_bundle.py:1642 +msgid "\n" "\t\t\tThe Batch {0} of an item {1} has negative stock in the warehouse {2}{3}.\n" "\t\t\tPlease add a stock quantity of {4} to proceed with this entry.\n" "\t\t\tIf it is not possible to make an adjustment entry, please enable 'Allow Negative Stock for Batch' in the batch {0} or in the Stock Settings to proceed.\n" "\t\t\tHowever, enabling this setting may lead to negative stock in the system.\n" "\t\t\tSo please ensure the stock levels are adjusted as soon as possible to maintain the correct valuation rate." -msgstr "" +msgstr "\n" +"\t\t\tتحتوي الدفعة {0} من الصنف {1} على مخزون سالب في المستودع {2}{3}.\n" +"\t\t\tيرجى إضافة كمية مخزون قدرها {4} للمتابعة.\n" +"\t\t\tإذا تعذر إجراء تعديل، يرجى تفعيل خيار \"السماح بالمخزون السالب للدفعة\" في الدفعة {0} أو في إعدادات المخزون للمتابعة.\n" +"\t\t\tمع ذلك، قد يؤدي تفعيل هذا الخيار إلى وجود مخزون سالب في النظام.\n" +"\t\t\tلذا يرجى التأكد من تعديل مستويات المخزون في أسرع وقت ممكن للحفاظ على معدل التقييم الصحيح." #. Label of the column_break_32 (Column Break) field in DocType 'Email Digest' #: erpnext/setup/doctype/email_digest/email_digest.json @@ -160,7 +168,7 @@ msgstr "" msgid "% Delivered" msgstr "% تسليم" -#: erpnext/manufacturing/doctype/bom/bom.js:1022 +#: erpnext/manufacturing/doctype/bom/bom.js:1026 #, python-format msgid "% Finished Item Quantity" msgstr "% كمية المنتج النهائي" @@ -305,11 +313,11 @@ msgstr "\"لهُ رقم تسلسل\" لا يمكن ان يكون \"نعم\" ل #: erpnext/stock/doctype/quality_inspection/quality_inspection.py:147 msgid "'Inspection Required before Delivery' has disabled for the item {0}, no need to create the QI" -msgstr "" +msgstr "تم تعطيل خيار \"الفحص مطلوب قبل التسليم\" للعنصر {0}، ولا حاجة لإنشاء QI" #: erpnext/stock/doctype/quality_inspection/quality_inspection.py:138 msgid "'Inspection Required before Purchase' has disabled for the item {0}, no need to create the QI" -msgstr "" +msgstr "تم تعطيل 'الفحص مطلوب قبل الشراء' للعنصر {0}، لا حاجة لإنشاء QI" #: erpnext/stock/report/stock_ledger/stock_ledger.py:685 #: erpnext/stock/report/stock_ledger/stock_ledger.py:726 @@ -630,8 +638,7 @@ msgstr "" #. Accounts' #: erpnext/accounts/doctype/process_statement_of_accounts/process_statement_of_accounts.json #, python-format -msgid "" -"
\n" +msgid "
\n" "

Note

\n" "