From 30ab2dba6ed8e085bdd4d9e9085df9d59aac4a7c Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Wed, 15 Jul 2026 10:34:07 +0530 Subject: [PATCH 01/37] fix: set correct currency in supplier quotation net rate field (cherry picked from commit 27672851cdbc2fe8d5addb628a94b2215768ce11) --- .../supplier_quotation_item/supplier_quotation_item.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 a41638966f1..7570e24ef2d 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", From 5e2e15436d522a335221277bc3c8bef1e39b17d1 Mon Sep 17 00:00:00 2001 From: Poovitha Palanivelu Date: Tue, 14 Jul 2026 15:08:59 +0530 Subject: [PATCH 02/37] feat: add on hold status to project (cherry picked from commit 672fadaa78befee144cc81895698b7ae86226085) --- erpnext/projects/doctype/project/project.json | 4 ++-- erpnext/projects/doctype/project/project.py | 6 +++--- erpnext/projects/doctype/project/project_list.js | 2 ++ 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/erpnext/projects/doctype/project/project.json b/erpnext/projects/doctype/project/project.json index 24841972ea4..bf573c020db 100644 --- a/erpnext/projects/doctype/project/project.json +++ b/erpnext/projects/doctype/project/project.json @@ -83,7 +83,7 @@ "no_copy": 1, "oldfieldname": "status", "oldfieldtype": "Select", - "options": "Open\nCompleted\nCancelled", + "options": "Open\nOn hold\nCompleted\nCancelled", "search_index": 1 }, { @@ -464,7 +464,7 @@ "index_web_pages_for_search": 1, "links": [], "max_attachments": 4, - "modified": "2026-05-22 16:45:50.762759", + "modified": "2026-07-14 14:20:50.418911", "modified_by": "Administrator", "module": "Projects", "name": "Project", diff --git a/erpnext/projects/doctype/project/project.py b/erpnext/projects/doctype/project/project.py index a188015fb55..68c7e657cbc 100644 --- a/erpnext/projects/doctype/project/project.py +++ b/erpnext/projects/doctype/project/project.py @@ -61,7 +61,7 @@ class Project(Document): project_type: DF.Link | None sales_order: DF.Link | None second_email: DF.Time | None - status: DF.Literal["Open", "Completed", "Cancelled"] + status: DF.Literal["Open", "On hold", "Completed", "Cancelled"] subject: DF.Data | None to_time: DF.Time | None total_billable_amount: DF.Currency @@ -264,8 +264,8 @@ class Project(Document): pct_complete += row["progress"] * frappe.utils.safe_div(row["task_weight"], weight_sum) self.percent_complete = flt(flt(pct_complete), 2) - # don't update status if it is cancelled - if self.status == "Cancelled": + # don't update status if it is manually set to cancelled or on hold + if self.status in ("Cancelled", "On hold"): return self.status = "Completed" if self.percent_complete == 100 else "Open" diff --git a/erpnext/projects/doctype/project/project_list.js b/erpnext/projects/doctype/project/project_list.js index 1503b1ee5d3..28a774524d4 100644 --- a/erpnext/projects/doctype/project/project_list.js +++ b/erpnext/projects/doctype/project/project_list.js @@ -4,6 +4,8 @@ frappe.listview_settings["Project"] = { get_indicator: function (doc) { if (doc.status == "Open" && doc.percent_complete) { return [__("{0}%", [cint(doc.percent_complete)]), "orange", "percent_complete,>,0|status,=,Open"]; + } else if (doc.status == "On hold") { + return [__("On hold"), "blue", "status,=,On hold"]; } else { return [__(doc.status), frappe.utils.guess_colour(doc.status), "status,=," + doc.status]; } From 654890dce876ebcc4eecc3573fcc4ec20596f211 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 15 Jul 2026 07:18:42 +0000 Subject: [PATCH 03/37] Revert "chore: remove unused whitelisted method from project" (backport #56660) (#57177) Co-authored-by: Diptanil Saha --- erpnext/templates/pages/projects.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/erpnext/templates/pages/projects.py b/erpnext/templates/pages/projects.py index 8f153105e3e..46ad25ed6ed 100644 --- a/erpnext/templates/pages/projects.py +++ b/erpnext/templates/pages/projects.py @@ -64,6 +64,21 @@ def get_tasks(project, start=0, search=None, item_status=None): return list(filter(lambda x: not x.parent_task, tasks)) +@frappe.whitelist() +def get_task_html(project: str, start: int = 0, item_status: str | None = None): + return frappe.render_template( + "erpnext/templates/includes/projects/project_tasks.html", + { + "doc": { + "name": project, + "project_name": project, + "tasks": get_tasks(project, start, item_status=item_status), + } + }, + is_path=True, + ) + + def get_timesheets(project, start=0, search=None): filters = {"project": project} if search: @@ -89,6 +104,15 @@ def get_timesheets(project, start=0, search=None): return timesheets +@frappe.whitelist() +def get_timesheet_html(project: str, start: int = 0): + return frappe.render_template( + "erpnext/templates/includes/projects/project_timesheets.html", + {"doc": {"timesheets": get_timesheets(project, start)}}, + is_path=True, + ) + + def get_attachments(project): return frappe.get_all( "File", From 202f52271ca2f689c2836fc27ca30c9cd507958e Mon Sep 17 00:00:00 2001 From: ervishnucs Date: Fri, 10 Jul 2026 17:25:03 +0530 Subject: [PATCH 04/37] fix: apply user permissions via build_match_conditions --- .../sales_person_wise_transaction_summary.py | 151 ++++++++++-------- 1 file changed, 80 insertions(+), 71 deletions(-) diff --git a/erpnext/selling/report/sales_person_wise_transaction_summary/sales_person_wise_transaction_summary.py b/erpnext/selling/report/sales_person_wise_transaction_summary/sales_person_wise_transaction_summary.py index 405159215cd..fa81c47b16d 100644 --- a/erpnext/selling/report/sales_person_wise_transaction_summary/sales_person_wise_transaction_summary.py +++ b/erpnext/selling/report/sales_person_wise_transaction_summary/sales_person_wise_transaction_summary.py @@ -4,7 +4,9 @@ import frappe from frappe import _, msgprint, qb -from frappe.query_builder import Criterion +from frappe.desk.reportview import build_match_conditions +from frappe.query_builder import Case, Criterion +from pypika.terms import LiteralValue from erpnext import get_company_currency @@ -155,84 +157,94 @@ def get_columns(filters): def get_entries(filters): - date_field = filters["doc_type"] == "Sales Order" and "transaction_date" or "posting_date" - if filters["doc_type"] == "Sales Order": - qty_field = "delivered_qty" - else: - qty_field = "qty" - conditions, values = get_conditions(filters, date_field) + doc_type = filters["doc_type"] - entries = frappe.db.sql( - """ - SELECT - dt.name, dt.customer, dt.territory, dt.{} as posting_date, dt_item.item_code, - st.sales_person, st.allocated_percentage, dt_item.warehouse, - CASE - WHEN dt.status = "Closed" THEN dt_item.{} * dt_item.conversion_factor - ELSE dt_item.stock_qty - END as stock_qty, - CASE - WHEN dt.status = "Closed" THEN (dt_item.base_net_rate * dt_item.{} * dt_item.conversion_factor) - ELSE dt_item.base_net_amount - END as base_net_amount, - CASE - WHEN dt.status = "Closed" THEN ((dt_item.base_net_rate * dt_item.{} * dt_item.conversion_factor) * st.allocated_percentage/100) - ELSE dt_item.base_net_amount * st.allocated_percentage/100 - END as contribution_amt - FROM - `tab{}` dt, `tab{} Item` dt_item, `tabSales Team` st - WHERE - st.parent = dt.name and dt.name = dt_item.parent and st.parenttype = {} - and dt.docstatus = 1 {} order by st.sales_person, dt.name desc - """.format( - date_field, - qty_field, - qty_field, - qty_field, - filters["doc_type"], - filters["doc_type"], - "%s", - conditions, - ), - tuple([filters["doc_type"], *values]), - as_dict=1, + date_field = "transaction_date" if doc_type == "Sales Order" else "posting_date" + qty_field = "delivered_qty" if doc_type == "Sales Order" else "qty" + + dt = frappe.qb.DocType(doc_type) + dt_item = frappe.qb.DocType(f"{doc_type} Item") + st = frappe.qb.DocType("Sales Team") + + calc_qty = dt_item[qty_field] * dt_item.conversion_factor + calc_net_amount = dt_item.base_net_rate * calc_qty + + stock_qty_case = Case().when(dt.status == "Closed", calc_qty).else_(dt_item.stock_qty).as_("stock_qty") + + base_net_amount_case = ( + Case() + .when(dt.status == "Closed", calc_net_amount) + .else_(dt_item.base_net_amount) + .as_("base_net_amount") ) - return entries - - -def get_conditions(filters, date_field): - conditions = [""] - values = [] + contribution_amt_case = ( + Case() + .when(dt.status == "Closed", (calc_net_amount * st.allocated_percentage / 100)) + .else_(dt_item.base_net_amount * st.allocated_percentage / 100) + .as_("contribution_amt") + ) + # Only pass valid document-field filters to get_query; report-specific keys such as + # doc_type / sales_person / item_group are handled separately below. + doc_filters = {"docstatus": 1} for field in ["company", "customer", "territory"]: if filters.get(field): - conditions.append(f"dt.{field}=%s") - values.append(filters[field]) + doc_filters[field] = filters.get(field) + + if filters.get("from_date") and filters.get("to_date"): + doc_filters[date_field] = ["between", [filters.get("from_date"), filters.get("to_date")]] + elif filters.get("from_date"): + doc_filters[date_field] = [">=", filters.get("from_date")] + elif filters.get("to_date"): + doc_filters[date_field] = ["<=", filters.get("to_date")] + + query = ( + frappe.get_query(dt, filters=doc_filters) + .join(dt_item) + .on(dt.name == dt_item.parent) + .join(st) + .on(dt.name == st.parent) + .select( + dt.name, + dt.customer, + dt.territory, + dt[date_field].as_("posting_date"), + dt_item.item_code, + st.sales_person, + st.allocated_percentage, + dt_item.warehouse, + stock_qty_case, + base_net_amount_case, + contribution_amt_case, + ) + .where(st.parenttype == doc_type) + ) if filters.get("sales_person"): - lft, rgt = frappe.get_value("Sales Person", filters.get("sales_person"), ["lft", "rgt"]) - conditions.append( - f"exists(select name from `tabSales Person` where lft >= {lft} and rgt <= {rgt} and name=st.sales_person)" + lft, rgt = frappe.db.get_value("Sales Person", filters.get("sales_person"), ["lft", "rgt"]) + sp = frappe.qb.DocType("Sales Person") + query = query.where( + st.sales_person.isin(frappe.qb.from_(sp).select(sp.name).where((sp.lft >= lft) & (sp.rgt <= rgt))) ) - if filters.get("from_date"): - conditions.append(f"dt.{date_field}>=%s") - values.append(filters["from_date"]) + # only resolve items when an item_group/brand filter is set; otherwise get_items + # would return every item in the system and add a huge IN() clause on each run + if filters.get("item_group") or filters.get("brand"): + items = get_items(filters) + if not items: + # the item_group/brand filter matched nothing -> no rows + return [] + query = query.where(dt_item.item_code.isin([d[0] for d in items])) - if filters.get("to_date"): - conditions.append(f"dt.{date_field}<=%s") - values.append(filters["to_date"]) + query = query.orderby(st.sales_person).orderby(dt.name, order=frappe.qb.desc) - items = get_items(filters) - if items: - conditions.append("dt_item.item_code in (%s)" % ", ".join(["%s"] * len(items))) - values += items - else: - # return empty result, if no items are fetched after filtering on 'item group' and 'brand' - conditions.append("dt_item.item_code = Null") + # Apply user permissions (v15: ignore_permissions is not available) + match_conditions = build_match_conditions(doc_type) + if match_conditions: + query = query.where(LiteralValue(match_conditions)) - return " and ".join(conditions), values + return query.run(as_dict=True) def get_items(filters): @@ -259,8 +271,5 @@ def get_items(filters): def get_item_details(): - item_details = {} - for d in frappe.db.sql("""SELECT `name`, `item_group`, `brand` FROM `tabItem`""", as_dict=1): - item_details.setdefault(d.name, d) - - return item_details + items = frappe.get_all("Item", fields=["name", "item_group", "brand"]) + return {d.name: d for d in items} From 6b23b007a4e03de7f98609ed33c15f9a09bf244c Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 15 Jul 2026 14:34:44 +0530 Subject: [PATCH 05/37] fix: permission issue (backport #57112) (#57142) * fix: permission issue (#57112) (cherry picked from commit 1fd2faa68d0b4960d9e2e48ab782be9cc6b1b644) # Conflicts: # erpnext/controllers/stock_controller.py * chore: fix conflicts --------- Co-authored-by: rohitwaghchaure --- erpnext/controllers/stock_controller.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/erpnext/controllers/stock_controller.py b/erpnext/controllers/stock_controller.py index c48eb2bd620..b4218b85f0e 100644 --- a/erpnext/controllers/stock_controller.py +++ b/erpnext/controllers/stock_controller.py @@ -1513,9 +1513,10 @@ 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_doc(doctype, docname) + doc.check_permission("read") doc.run_method("before_gl_preview") gl_columns, gl_data = get_accounting_ledger_preview(doc, filters) @@ -1526,9 +1527,10 @@ 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_doc(doctype, docname) + doc.check_permission("read") doc.run_method("before_sl_preview") sl_columns, sl_data = get_stock_ledger_preview(doc, filters) From ff6c8bbb44dce4cebc08a4adc65eb941fb956583 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 15 Jul 2026 15:38:46 +0530 Subject: [PATCH 06/37] fix(project): improved access control for project users (backport #56675) (#57180) Co-authored-by: Diptanil Saha --- erpnext/patches.txt | 1 + .../v16_0/access_control_for_project_users.py | 33 +++++++++ erpnext/projects/doctype/project/project.json | 18 ++++- erpnext/projects/doctype/project/project.py | 30 ++++++++ .../projects/doctype/project/test_project.py | 55 ++++++++++++++ erpnext/templates/pages/projects.py | 22 +++--- erpnext/templates/pages/test_projects.py | 72 +++++++++++++++++++ 7 files changed, 219 insertions(+), 12 deletions(-) create mode 100644 erpnext/patches/v16_0/access_control_for_project_users.py create mode 100644 erpnext/templates/pages/test_projects.py diff --git a/erpnext/patches.txt b/erpnext/patches.txt index f3f216ac64c..7adc55d57c4 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -442,3 +442,4 @@ erpnext.patches.v15_0.backfill_sla_link_filters_on_custom_field erpnext.patches.v15_0.backfill_sla_link_filters_on_docfield erpnext.patches.v16_0.crm_settings_handle_allowed_users_for_frappe_crm erpnext.patches.v16_0.backfill_pick_list_transferred_qty +erpnext.patches.v16_0.access_control_for_project_users diff --git a/erpnext/patches/v16_0/access_control_for_project_users.py b/erpnext/patches/v16_0/access_control_for_project_users.py new file mode 100644 index 00000000000..3c8cf9fe857 --- /dev/null +++ b/erpnext/patches/v16_0/access_control_for_project_users.py @@ -0,0 +1,33 @@ +import frappe + + +def execute(): + Project = frappe.qb.DocType("Project") + ProjectUser = frappe.qb.DocType("Project User") + + query = ( + frappe.qb.from_(Project) + .join(ProjectUser) + .on(Project.name == ProjectUser.parent) + .select(Project.name, ProjectUser.user) + ) + + proj_users = query.run(as_dict=1) + + project_mapped_users = get_project_mapped_users(proj_users) + + for d in proj_users: + if d.user in project_mapped_users[d.name]: + continue + + frappe.share.add_docshare("Project", d.name, user=d.user) + + +def get_project_mapped_users(proj_users): + projects = set([d.name for d in proj_users]) + project_mapped_users = {} + + for d in projects: + project_mapped_users[d] = [d.user for d in frappe.share.get_users("Project", d)] + + return project_mapped_users diff --git a/erpnext/projects/doctype/project/project.json b/erpnext/projects/doctype/project/project.json index bf573c020db..f464fa31d82 100644 --- a/erpnext/projects/doctype/project/project.json +++ b/erpnext/projects/doctype/project/project.json @@ -205,13 +205,15 @@ "fieldname": "users", "fieldtype": "Table", "label": "Users", - "options": "Project User" + "options": "Project User", + "permlevel": 1 }, { "fieldname": "copied_from", "fieldtype": "Data", "hidden": 1, "label": "Copied From", + "permlevel": 1, "read_only": 1 }, { @@ -464,13 +466,25 @@ "index_web_pages_for_search": 1, "links": [], "max_attachments": 4, - "modified": "2026-07-14 14:20:50.418911", + "modified": "2026-07-14 14:32:11.328347", "modified_by": "Administrator", "module": "Projects", "name": "Project", "naming_rule": "By \"Naming Series\" field", "owner": "Administrator", "permissions": [ + { + "delete": 1, + "email": 1, + "export": 1, + "permlevel": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "Projects Manager", + "share": 1, + "write": 1 + }, { "create": 1, "delete": 1, diff --git a/erpnext/projects/doctype/project/project.py b/erpnext/projects/doctype/project/project.py index 68c7e657cbc..0259a64ef09 100644 --- a/erpnext/projects/doctype/project/project.py +++ b/erpnext/projects/doctype/project/project.py @@ -93,6 +93,7 @@ class Project(Document): def validate(self): if not self.is_new(): self.copy_from_template() # nosemgrep + self.control_access_for_project_users() self.send_welcome_email() self.update_costing() self.update_percent_complete() @@ -207,6 +208,7 @@ class Project(Document): self.copy_from_template() # nosemgrep if self.sales_order: frappe.db.set_value("Sales Order", self.sales_order, "project", self.name) + self.control_access_for_project_users() def on_trash(self): frappe.db.set_value("Sales Order", {"project": self.name}, "project", "") @@ -377,6 +379,34 @@ class Project(Document): ) user.welcome_email_sent = 1 + def control_access_for_project_users(self): + def revoke_access_for_project_users(removed_users): + users = set([d.user for d in frappe.share.get_users(self.doctype, self.name)]) + for user in removed_users: + if user not in users: + continue + + frappe.share.remove(self.doctype, self.name, user) + + def grant_access_for_project_users(new_users): + for user in new_users: + frappe.share.add_docshare(self.doctype, self.name, user=user) + + current_users = set([d.user for d in self.users]) + old_doc = self.get_doc_before_save() + + if not old_doc: + grant_access_for_project_users(current_users) + return + + previous_users = set([d.user for d in old_doc.users]) + + new_users = current_users - previous_users + removed_users = previous_users - current_users + + revoke_access_for_project_users(removed_users) + grant_access_for_project_users(new_users) + def get_timeline_data(doctype: str, name: str) -> dict[int, int]: """Return timeline for attendance""" diff --git a/erpnext/projects/doctype/project/test_project.py b/erpnext/projects/doctype/project/test_project.py index e5996c2da9d..edae80fd4d2 100644 --- a/erpnext/projects/doctype/project/test_project.py +++ b/erpnext/projects/doctype/project/test_project.py @@ -227,6 +227,61 @@ class TestProject(FrappeTestCase): project.save() self.assertEqual(project.status, "Completed") + def _create_portal_user(self, email): + """A user with no Project-related role, so read access can only come from + control_access_for_project_users() sharing the doc with them.""" + if not frappe.db.exists("User", email): + frappe.get_doc( + { + "doctype": "User", + "email": email, + "first_name": "Portal", + "send_welcome_email": 0, + } + ).insert(ignore_permissions=True) + return email + + def test_new_project_grants_access_to_its_users(self): + member = self._create_portal_user(f"new_proj_member_{frappe.generate_hash(length=6)}@example.com") + + project = frappe.get_doc( + doctype="Project", + project_name=f"_Test New Project Access {frappe.generate_hash(length=6)}", + status="Open", + company="_Test Company", + ) + project.append("users", {"user": member, "welcome_email_sent": 1}) + project.insert() # must not raise + + self.assertTrue(project.has_permission(user=member)) + shared_with = [d.user for d in frappe.share.get_users("Project", project.name)] + self.assertIn(member, shared_with) + + def test_adding_and_removing_project_user_updates_access(self): + stays = self._create_portal_user(f"stays_{frappe.generate_hash(length=6)}@example.com") + leaves = self._create_portal_user(f"leaves_{frappe.generate_hash(length=6)}@example.com") + + project = frappe.get_doc( + doctype="Project", + project_name=f"_Test Project User Membership {frappe.generate_hash(length=6)}", + status="Open", + company="_Test Company", + ) + project.append("users", {"user": stays, "welcome_email_sent": 1}) + project.insert() + self.assertTrue(project.has_permission(user=stays)) + + # adding a user on update (not insert) must also grant them access + project.append("users", {"user": leaves, "welcome_email_sent": 1}) + project.save() + self.assertTrue(project.has_permission(user=leaves)) + + # removing a user must revoke the share that was granted for membership + project.users = [d for d in project.users if d.user != leaves] + project.save() + self.assertFalse(project.has_permission(user=leaves)) + self.assertTrue(project.has_permission(user=stays)) + def get_project(name, template): project = frappe.get_doc( diff --git a/erpnext/templates/pages/projects.py b/erpnext/templates/pages/projects.py index 46ad25ed6ed..646e2085ace 100644 --- a/erpnext/templates/pages/projects.py +++ b/erpnext/templates/pages/projects.py @@ -6,21 +6,12 @@ import frappe def get_context(context): - project_user = frappe.db.get_value( - "Project User", - {"parent": frappe.form_dict.project, "user": frappe.session.user}, - ["user", "view_attachments", "hide_timesheets"], - as_dict=True, - ) - if frappe.session.user != "Administrator" and (not project_user or frappe.session.user == "Guest"): - raise frappe.PermissionError + project_user = validate_and_get_project_user(project=frappe.form_dict.project) context.no_cache = 1 context.show_sidebar = True project = frappe.get_doc("Project", frappe.form_dict.project) - project.has_permission("read") - project.tasks = get_tasks( project.name, start=0, item_status="open", search=frappe.form_dict.get("search") ) @@ -66,6 +57,7 @@ def get_tasks(project, start=0, search=None, item_status=None): @frappe.whitelist() def get_task_html(project: str, start: int = 0, item_status: str | None = None): + validate_and_get_project_user(project=project) return frappe.render_template( "erpnext/templates/includes/projects/project_tasks.html", { @@ -106,6 +98,7 @@ def get_timesheets(project, start=0, search=None): @frappe.whitelist() def get_timesheet_html(project: str, start: int = 0): + validate_and_get_project_user(project=project) return frappe.render_template( "erpnext/templates/includes/projects/project_timesheets.html", {"doc": {"timesheets": get_timesheets(project, start)}}, @@ -119,3 +112,12 @@ def get_attachments(project): filters={"attached_to_name": project, "attached_to_doctype": "Project", "is_private": 0}, fields=["file_name", "file_url", "file_size"], ) + + +def validate_and_get_project_user(project: str): + project_doc = frappe.get_doc("Project", project) + project_doc.check_permission() + + project_user = next((d for d in project_doc.users if d.user == frappe.session.user), None) + + return project_user diff --git a/erpnext/templates/pages/test_projects.py b/erpnext/templates/pages/test_projects.py new file mode 100644 index 00000000000..43b5046ea4f --- /dev/null +++ b/erpnext/templates/pages/test_projects.py @@ -0,0 +1,72 @@ +# Copyright (c) 2026, Frappe Technologies Pvt. Ltd. and Contributors +# License: GNU General Public License v3. See license.txt + +import frappe +from frappe.tests.utils import FrappeTestCase + +from erpnext.projects.doctype.project.test_project import make_project +from erpnext.templates.pages.projects import validate_and_get_project_user + + +class TestProjectsPage(FrappeTestCase): + """validate_and_get_project_user() gates the /projects portal page. It must raise + frappe.PermissionError for a user who can't read the Project, and otherwise return + that user's Project User row (or None if they're permitted but not listed as one -- + e.g. an internal Projects Manager browsing the portal).""" + + def _create_user(self, email): + if not frappe.db.exists("User", email): + frappe.get_doc( + { + "doctype": "User", + "email": email, + "first_name": "Portal", + "send_welcome_email": 0, + } + ).insert(ignore_permissions=True) + return email + + def test_raises_permission_error_for_user_without_access(self): + project = make_project({"project_name": f"_Test Portal Access {frappe.generate_hash(length=6)}"}) + outsider = self._create_user(f"outsider_{frappe.generate_hash(length=6)}@example.com") + + with self.set_user(outsider): + self.assertRaises(frappe.PermissionError, validate_and_get_project_user, project.name) + + def test_allows_user_listed_as_project_user_and_returns_their_row(self): + # Being a Project User shares the Project with that user (see + # Project.control_access_for_project_users), which is what lets them past + # check_permission() here. + member = self._create_user(f"member_{frappe.generate_hash(length=6)}@example.com") + + project = frappe.get_doc( + doctype="Project", + project_name=f"_Test Portal Access {frappe.generate_hash(length=6)}", + status="Open", + company="_Test Company", + ) + project.append( + "users", {"user": member, "view_attachments": 1, "hide_timesheets": 1, "welcome_email_sent": 1} + ) + project.insert() + + with self.set_user(member): + project_user = validate_and_get_project_user(project.name) + + self.assertIsNotNone(project_user) + self.assertEqual(project_user.user, member) + self.assertEqual(project_user.view_attachments, 1) + self.assertEqual(project_user.hide_timesheets, 1) + + def test_allows_internally_permitted_user_not_listed_as_project_user(self): + # The permission gate must be the real permission system (check_permission()), + # not "is this user in the Project's users child table" -- a Projects Manager + # can open any project's portal page without ever being added as its user. + project = make_project({"project_name": f"_Test Portal Access {frappe.generate_hash(length=6)}"}) + manager = self._create_user(f"manager_{frappe.generate_hash(length=6)}@example.com") + frappe.get_doc("User", manager).add_roles("Projects Manager") + + with self.set_user(manager): + project_user = validate_and_get_project_user(project.name) + + self.assertIsNone(project_user) From 77ec6447c3457b45460223e078d240ed096654cb Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Thu, 16 Jul 2026 14:04:37 +0530 Subject: [PATCH 07/37] fix: consider min order qty in the purchase/transfer flow of production plan (backport #57204) (#57209) * fix: consider min order qty in the purchase/transfer flow of production plan (backport #57204) The transfer flow ignored Consider Minimum Order Qty twice: the JS handler force-reset the checkbox before fetching items, and the purchase remainder left after allocating transfers from other warehouses was never raised to min_order_qty (the check runs on the total requirement before the split). Drop the JS reset and apply min order qty to the purchase remainder, in stock UOM before the purchase UOM conversion. * test: do not set ignore_existing_ordered_qty; v15 gates the transfer split on it being unset --- .../doctype/production_plan/production_plan.js | 2 -- .../doctype/production_plan/production_plan.py | 15 ++++++++++++--- .../production_plan/test_production_plan.py | 11 +++++++++++ 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.js b/erpnext/manufacturing/doctype/production_plan/production_plan.js index 09abcb6351e..347b0a2a333 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.js +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.js @@ -365,8 +365,6 @@ frappe.ui.form.on("Production Plan", { frappe.throw(__("Select the Warehouse")); } - frm.set_value("consider_minimum_order_qty", 0); - if (frm.doc.ignore_existing_ordered_qty) { frm.events.get_items_for_material_requests(frm); } else { diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.py b/erpnext/manufacturing/doctype/production_plan/production_plan.py index cb8f24fc9f1..52f301bad11 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.py @@ -1708,7 +1708,13 @@ def get_items_for_material_requests(doc, warehouses=None, get_parent_warehouse_d if (not ignore_existing_ordered_qty or get_parent_warehouse_data) and warehouses: new_mr_items = [] for item in mr_items: - get_materials_from_other_locations(item, warehouses, new_mr_items, company) + get_materials_from_other_locations( + item, + warehouses, + new_mr_items, + company, + consider_minimum_order_qty=doc.get("consider_minimum_order_qty"), + ) mr_items = new_mr_items @@ -1728,7 +1734,9 @@ def get_items_for_material_requests(doc, warehouses=None, get_parent_warehouse_d return mr_items -def get_materials_from_other_locations(item, warehouses, new_mr_items, company): +def get_materials_from_other_locations( + item, warehouses, new_mr_items, company, consider_minimum_order_qty=False +): from erpnext.stock.doctype.pick_list.pick_list import get_available_item_locations stock_uom, purchase_uom = frappe.db.get_value( @@ -1773,7 +1781,8 @@ def get_materials_from_other_locations(item, warehouses, new_mr_items, company): precision = frappe.get_precision("Material Request Plan Item", "quantity") if flt(required_qty, precision) > 0: - required_qty = required_qty + if consider_minimum_order_qty: + required_qty = max(required_qty, flt(item.get("min_order_qty"))) if frappe.db.get_value("UOM", purchase_uom, "must_be_whole_number"): required_qty = ceil(required_qty) diff --git a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py index e64a8c16b84..b1f1866a12c 100644 --- a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py @@ -1832,6 +1832,17 @@ class TestProductionPlan(FrappeTestCase): for d in mr_items: self.assertEqual(d.get("quantity"), 1000.0) + source_warehouse = create_warehouse("MOQ Source Warehouse", company="_Test Company") + make_stock_entry(item_code=rm_item, qty=7, rate=100, target=source_warehouse) + + mr_items = get_items_for_material_requests( + pln.as_dict(), warehouses=[{"warehouse": source_warehouse}] + ) + self.assertEqual(len(mr_items), 2) + items_by_type = {d.get("material_request_type"): d for d in mr_items} + self.assertEqual(items_by_type["Material Transfer"].get("quantity"), 7.0) + self.assertEqual(items_by_type["Purchase"].get("quantity"), 1000.0) + def test_fg_item_quantity(self): fg_item = make_item(properties={"is_stock_item": 1}).name rm_item = make_item(properties={"is_stock_item": 1}).name From 4f9ea989c45624886219a29b9c526aa7c0d66802 Mon Sep 17 00:00:00 2001 From: Afsal Syed Date: Thu, 16 Jul 2026 14:44:49 +0530 Subject: [PATCH 08/37] feat(stock): automatically link portal users to their associated contact profiles for customers and suppliers (cherry picked from commit 337a06dfb6d529e85fa6d6c29acf90024f7390f0) --- erpnext/buying/doctype/supplier/supplier.py | 6 +- .../controllers/website_list_for_contact.py | 62 +++++++++++++++++++ erpnext/selling/doctype/customer/customer.py | 7 ++- 3 files changed, 73 insertions(+), 2 deletions(-) diff --git a/erpnext/buying/doctype/supplier/supplier.py b/erpnext/buying/doctype/supplier/supplier.py index f0e85523ea3..0b463a36530 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 @@ -103,6 +106,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/controllers/website_list_for_contact.py b/erpnext/controllers/website_list_for_contact.py index a62fccc752c..f0b9eadd749 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/selling/doctype/customer/customer.py b/erpnext/selling/doctype/customer/customer.py index 8b5761f5930..4d21ff94d3e 100644 --- a/erpnext/selling/doctype/customer/customer.py +++ b/erpnext/selling/doctype/customer/customer.py @@ -23,7 +23,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 @@ -243,6 +246,8 @@ class Customer(TransactionBase): self.update_customer_groups() + link_portal_users_to_contacts(self) + def add_role_for_user(self): for portal_user in self.portal_users: add_role_for_portal_user(portal_user, "Customer") From b1e47eb97e8c4cc8631c0de47b7030d020fbd0e8 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 16 Jul 2026 11:21:28 +0000 Subject: [PATCH 09/37] refactor(dunning): converted `get_dunning_letter_text` to doc method and `restrict_globals` on `render_template` (backport #57205) (#57213) Co-authored-by: Diptanil Saha --- erpnext/accounts/doctype/dunning/dunning.js | 21 ++---- erpnext/accounts/doctype/dunning/dunning.py | 72 ++++++++++--------- .../doctype/sales_invoice/sales_invoice.py | 12 +--- 3 files changed, 46 insertions(+), 59 deletions(-) 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 0f5831a967d..c8e4adc5be4 100644 --- a/erpnext/accounts/doctype/dunning/dunning.py +++ b/erpnext/accounts/doctype/dunning/dunning.py @@ -156,6 +156,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 ( @@ -234,35 +274,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/sales_invoice/sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py index 97b2afd7751..53e35e7407f 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py @@ -2884,8 +2884,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) @@ -2894,14 +2892,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: From 314dd16aa39f1481290d78c80e831a824fc2f9aa Mon Sep 17 00:00:00 2001 From: Shllokkk Date: Thu, 16 Jul 2026 00:17:35 +0530 Subject: [PATCH 10/37] fix: strip account number when building account name in COA importer (cherry picked from commit cbe406ee2afd0711fb2ca0ab287cdfe14386ef21) --- .../chart_of_accounts_importer/chart_of_accounts_importer.py | 1 + 1 file changed, 1 insertion(+) 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 9c4f2f8fd49..eeaa5abfc6d 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 @@ -215,6 +215,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() From 1b6ff72f0f53056622e8ba9cfa5924f228ffa7a0 Mon Sep 17 00:00:00 2001 From: Afsal Syed Date: Thu, 16 Jul 2026 23:21:47 +0530 Subject: [PATCH 11/37] test(stock): add portal user contact link verification for customer and supplier test(stock): add portal user contact link verification for customer and supplier --- .../buying/doctype/supplier/test_supplier.py | 21 ++++++++++++++ .../selling/doctype/customer/test_customer.py | 28 +++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/erpnext/buying/doctype/supplier/test_supplier.py b/erpnext/buying/doctype/supplier/test_supplier.py index 04983721a64..719770a6853 100644 --- a/erpnext/buying/doctype/supplier/test_supplier.py +++ b/erpnext/buying/doctype/supplier/test_supplier.py @@ -208,3 +208,24 @@ class TestSupplierPortal(FrappeTestCase): _, 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/selling/doctype/customer/test_customer.py b/erpnext/selling/doctype/customer/test_customer.py index a8fd5ed76ca..bfd66d51d24 100644 --- a/erpnext/selling/doctype/customer/test_customer.py +++ b/erpnext/selling/doctype/customer/test_customer.py @@ -386,6 +386,34 @@ class TestCustomer(FrappeTestCase): self.assertEqual(middle, "Michael") self.assertEqual(last, "Doe") + 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 Customer User" + user.send_welcome_email = False + user.insert(ignore_permissions=True) + + contact = frappe.new_doc("Contact") + contact.first_name = "Test Portal Customer User" + contact.add_email(user_email, is_primary=1) + contact.links = [] + contact.insert(ignore_permissions=True) + + customer = frappe.get_doc( + { + "doctype": "Customer", + "customer_name": "Test Portal Contact Customer", + "customer_type": "Individual", + "customer_group": "_Test Customer Group", + } + ) + customer.append("portal_users", {"user": user.name}) + customer.insert() + + contact.reload() + self.assertTrue(contact.has_link("Customer", customer.name)) + def get_customer_dict(customer_name): return { From 941423067140779b8e1eb94411dbb9bea9c70927 Mon Sep 17 00:00:00 2001 From: Afsal Syed Date: Fri, 17 Jul 2026 00:21:48 +0530 Subject: [PATCH 12/37] chore: fix test case user context --- erpnext/buying/doctype/supplier/test_supplier.py | 4 ++-- erpnext/selling/doctype/customer/test_customer.py | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/erpnext/buying/doctype/supplier/test_supplier.py b/erpnext/buying/doctype/supplier/test_supplier.py index 719770a6853..e0a2a379ed8 100644 --- a/erpnext/buying/doctype/supplier/test_supplier.py +++ b/erpnext/buying/doctype/supplier/test_supplier.py @@ -204,8 +204,8 @@ class TestSupplierPortal(FrappeTestCase): supplier.append("portal_users", {"user": user}) supplier.save() - frappe.set_user(user) - _, suppliers = get_customers_suppliers("Purchase Order", user) + with self.set_user(user): + _, suppliers = get_customers_suppliers("Purchase Order", user) self.assertIn(supplier.name, suppliers) diff --git a/erpnext/selling/doctype/customer/test_customer.py b/erpnext/selling/doctype/customer/test_customer.py index bfd66d51d24..e6ed7acb508 100644 --- a/erpnext/selling/doctype/customer/test_customer.py +++ b/erpnext/selling/doctype/customer/test_customer.py @@ -29,7 +29,8 @@ class TestCustomer(FrappeTestCase): make_test_records("Item") def tearDown(self): - set_credit_limit("_Test Customer", "_Test Company", 0) + if frappe.db.exists("Customer", "_Test Customer"): + set_credit_limit("_Test Customer", "_Test Company", 0) def test_get_customer_group_details(self): doc = frappe.new_doc("Customer Group") From fd01171df5742e8357aae494e9177a59c99179f7 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Fri, 17 Jul 2026 13:34:30 +0530 Subject: [PATCH 13/37] fix: added missing validations for `Dunning Type` (backport #57224) (#57226) Co-authored-by: diptanilsaha --- .../doctype/dunning_type/dunning_type.py | 134 ++++++++++++ .../doctype/dunning_type/test_dunning_type.py | 198 +++++++++++++++++- 2 files changed, 328 insertions(+), 4 deletions(-) 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 67b72e4be75..bdec57ec142 100644 --- a/erpnext/accounts/doctype/dunning_type/test_dunning_type.py +++ b/erpnext/accounts/doctype/dunning_type/test_dunning_type.py @@ -1,9 +1,199 @@ # Copyright (c) 2020, Frappe Technologies Pvt. Ltd. and Contributors # See license.txt -# import frappe -import unittest +import frappe +from frappe.tests.utils import FrappeTestCase -class TestDunningType(unittest.TestCase): - pass +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(FrappeTestCase): + 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) From 5e539643ec807fd1fefba401acda46387957cbaf Mon Sep 17 00:00:00 2001 From: Diptanil Saha Date: Fri, 17 Jul 2026 13:35:04 +0530 Subject: [PATCH 14/37] chore: bump frappe dependency from 15.40.4 to 15.111.0 (#57229) --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index f5bc11693a4..b6702cb873b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,7 +29,7 @@ requires = ["flit_core >=3.4,<4"] build-backend = "flit_core.buildapi" [tool.bench.frappe-dependencies] -frappe = ">=15.40.4,<16.0.0" +frappe = ">=15.111.0,<16.0.0" [tool.ruff] line-length = 110 From 88443e4a97c6c0a40d85a9e6785577191296ee39 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Fri, 17 Jul 2026 08:25:47 +0000 Subject: [PATCH 15/37] fix: restrict jinja globals in process statement of accounts templates (backport #56458) (#57231) Co-authored-by: Shllokkk --- .../process_statement_of_accounts.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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 9f0680de3ee..e71041eea5b 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 @@ -97,9 +97,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.")) @@ -501,15 +501,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") From 96dc408484ee846c2a6bbb0ac545a47455545685 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Fri, 17 Jul 2026 16:52:13 +0530 Subject: [PATCH 16/37] fix: validate buying price list on material request and update item rates on change --- .../material_request/material_request.js | 9 +++-- .../material_request/material_request.py | 38 ++++++++++++++++++- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/erpnext/stock/doctype/material_request/material_request.js b/erpnext/stock/doctype/material_request/material_request.js index 7642133fe3e..1a868dc546a 100644 --- a/erpnext/stock/doctype/material_request/material_request.js +++ b/erpnext/stock/doctype/material_request/material_request.js @@ -92,7 +92,10 @@ frappe.ui.form.on("Material Request", { erpnext.accounts.dimensions.setup_dimension_filters(frm, frm.doctype); if (!frm.doc.buying_price_list) { - frm.doc.buying_price_list = frappe.defaults.get_default("buying_price_list"); + const buying_price_list = frappe.defaults.get_default("buying_price_list"); + if (frappe.has_permission("Price List", "read", buying_price_list)) { + frm.set_value("buying_price_list", buying_price_list); + } } }, @@ -271,9 +274,7 @@ frappe.ui.form.on("Material Request", { from_warehouse: item.from_warehouse, warehouse: item.warehouse, doctype: frm.doc.doctype, - buying_price_list: frm.doc.buying_price_list - ? frm.doc.buying_price_list - : frappe.defaults.get_default("buying_price_list"), + buying_price_list: frm.doc.buying_price_list, currency: frappe.defaults.get_default("Currency"), name: frm.doc.name, qty: item.qty || 1, diff --git a/erpnext/stock/doctype/material_request/material_request.py b/erpnext/stock/doctype/material_request/material_request.py index ffd37d91df4..ea4ccb20a36 100644 --- a/erpnext/stock/doctype/material_request/material_request.py +++ b/erpnext/stock/doctype/material_request/material_request.py @@ -19,6 +19,7 @@ from erpnext.buying.utils import check_on_hold_or_closed_status, validate_for_it from erpnext.controllers.buying_controller import BuyingController from erpnext.manufacturing.doctype.work_order.work_order import get_item_details from erpnext.stock.doctype.item.item import get_item_defaults +from erpnext.stock.get_item_details import get_price_list_rate_for from erpnext.stock.stock_balance import get_indented_qty, update_bin_qty form_grid_templates = {"items": "templates/form_grid/material_request_grid.html"} @@ -169,8 +170,43 @@ class MaterialRequest(BuyingController): self.reset_default_field_value("set_warehouse", "items", "warehouse") self.reset_default_field_value("set_from_warehouse", "items", "from_warehouse") + if self.buying_price_list and not frappe.get_value("Price List", self.buying_price_list, "buying"): + self.buying_price_list = None + if not self.buying_price_list: - self.buying_price_list = frappe.defaults.get_defaults().buying_price_list + buying_price_list = frappe.defaults.get_defaults().buying_price_list + if frappe.has_permission("Price List", "read", buying_price_list): + self.buying_price_list = buying_price_list + + def on_update(self): + if self.buying_price_list and self.has_value_changed("buying_price_list"): + self.update_item_rates() + + def update_item_rates(self): + price_not_uom_dependent = frappe.get_value( + "Price List", self.buying_price_list, "price_not_uom_dependent" + ) + for item in self.items: + rate = get_price_list_rate_for( + frappe._dict( + { + "price_list": self.buying_price_list, + "uom": item.uom, + "transaction_date": self.transaction_date, + "qty": item.qty, + "stock_uom": item.stock_uom, + "price_not_uom_dependent": price_not_uom_dependent, + } + ), + item.item_code, + ) + item.db_set({"rate": flt(rate), "amount": flt(flt(rate) * item.qty, item.precision("amount"))}) + frappe.msgprint( + _("Item rates have been updated based on the selected Buying Price List {0}").format( + self.buying_price_list + ), + alert=True, + ) def before_update_after_submit(self): self.validate_schedule_date() From 302cbbe5d864f72e4e1556b59b877fbb219739f4 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Fri, 17 Jul 2026 18:24:15 +0530 Subject: [PATCH 17/37] fix: pass ctx keys get_price_list_rate_for reads, skip rate update on insert update_item_rates passed price_not_uom_dependent, a key get_price_list_rate_for never reads, and omitted conversion_factor, so a stock-UOM price was never converted to the row UOM. The function's (historically misnamed) price_list_uom_dependant ctx key carries the Price List's price_not_uom_dependent value: truthy returns the found rate as-is, falsy multiplies by conversion_factor. Also guard on_update with is_new(): has_value_changed returns True when there is no doc_before_save, so every first save re-wrote item rates. --- erpnext/stock/doctype/material_request/material_request.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/erpnext/stock/doctype/material_request/material_request.py b/erpnext/stock/doctype/material_request/material_request.py index ea4ccb20a36..550c562da4a 100644 --- a/erpnext/stock/doctype/material_request/material_request.py +++ b/erpnext/stock/doctype/material_request/material_request.py @@ -179,7 +179,7 @@ class MaterialRequest(BuyingController): self.buying_price_list = buying_price_list def on_update(self): - if self.buying_price_list and self.has_value_changed("buying_price_list"): + if not self.is_new() and self.buying_price_list and self.has_value_changed("buying_price_list"): self.update_item_rates() def update_item_rates(self): @@ -195,7 +195,8 @@ class MaterialRequest(BuyingController): "transaction_date": self.transaction_date, "qty": item.qty, "stock_uom": item.stock_uom, - "price_not_uom_dependent": price_not_uom_dependent, + "conversion_factor": item.conversion_factor, + "price_list_uom_dependant": price_not_uom_dependent, } ), item.item_code, From 74e9718871dcd34f8abbccafc5f22e30d50b235e Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Fri, 17 Jul 2026 21:23:49 +0530 Subject: [PATCH 18/37] fix: dont overwrite rate with 0 if not found --- erpnext/stock/doctype/material_request/material_request.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/erpnext/stock/doctype/material_request/material_request.py b/erpnext/stock/doctype/material_request/material_request.py index 550c562da4a..7ba8dd27571 100644 --- a/erpnext/stock/doctype/material_request/material_request.py +++ b/erpnext/stock/doctype/material_request/material_request.py @@ -201,7 +201,9 @@ class MaterialRequest(BuyingController): ), item.item_code, ) - item.db_set({"rate": flt(rate), "amount": flt(flt(rate) * item.qty, item.precision("amount"))}) + if rate is not None: + item.db_set({"rate": flt(rate), "amount": flt(flt(rate) * item.qty, item.precision("amount"))}) + frappe.msgprint( _("Item rates have been updated based on the selected Buying Price List {0}").format( self.buying_price_list From 1fabf78dea0bc68ded4347bd783e5bb20235c649 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Fri, 17 Jul 2026 21:24:40 +0530 Subject: [PATCH 19/37] chore: remove unneccessary flt --- erpnext/stock/doctype/material_request/material_request.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/stock/doctype/material_request/material_request.py b/erpnext/stock/doctype/material_request/material_request.py index 7ba8dd27571..ceae67114f9 100644 --- a/erpnext/stock/doctype/material_request/material_request.py +++ b/erpnext/stock/doctype/material_request/material_request.py @@ -202,7 +202,7 @@ class MaterialRequest(BuyingController): item.item_code, ) if rate is not None: - item.db_set({"rate": flt(rate), "amount": flt(flt(rate) * item.qty, item.precision("amount"))}) + item.db_set({"rate": rate, "amount": flt(rate * item.qty, item.precision("amount"))}) frappe.msgprint( _("Item rates have been updated based on the selected Buying Price List {0}").format( From 6ffd75996893f358e59a9296764912e623545300 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Fri, 17 Jul 2026 22:08:15 +0530 Subject: [PATCH 20/37] fix: add fetch from in production plan material request child table (cherry picked from commit dfc2a411e1cf83225d522345b8f1210df3ad98ff) --- .../production_plan_material_request.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/erpnext/manufacturing/doctype/production_plan_material_request/production_plan_material_request.json b/erpnext/manufacturing/doctype/production_plan_material_request/production_plan_material_request.json index b5eb73ed061..d8005ea044e 100644 --- a/erpnext/manufacturing/doctype/production_plan_material_request/production_plan_material_request.json +++ b/erpnext/manufacturing/doctype/production_plan_material_request/production_plan_material_request.json @@ -82,6 +82,7 @@ "bold": 0, "collapsible": 0, "columns": 0, + "fetch_from": "material_request.transaction_date", "fieldname": "material_request_date", "fieldtype": "Date", "hidden": 0, @@ -122,7 +123,7 @@ "issingle": 0, "istable": 1, "max_attachments": 0, - "modified": "2017-10-29 12:31:57.986869", + "modified": "2026-07-17 22:06:35.428875", "modified_by": "Administrator", "module": "Manufacturing", "name": "Production Plan Material Request", From d6f797d0776bca628418e8cdf009858efb247f23 Mon Sep 17 00:00:00 2001 From: Mohd Haris Date: Mon, 20 Jul 2026 11:59:15 +0530 Subject: [PATCH 21/37] fix: use account currency in Bank Reconciliation Statement print The custom print template formatted debit/credit amounts with format_currency() without passing a currency, so it fell back to the company/system default currency (e.g. INR) instead of the selected bank account's currency. The on-screen report already formats correctly via the column's account_currency option. Pass each row's account_currency to format_currency() so the printed/PDF output matches the on-screen currency. Co-Authored-By: Claude Opus 4.8 --- .../bank_reconciliation_statement.html | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/erpnext/accounts/report/bank_reconciliation_statement/bank_reconciliation_statement.html b/erpnext/accounts/report/bank_reconciliation_statement/bank_reconciliation_statement.html index 6957ab41681..4596374cff6 100644 --- a/erpnext/accounts/report/bank_reconciliation_statement/bank_reconciliation_statement.html +++ b/erpnext/accounts/report/bank_reconciliation_statement/bank_reconciliation_statement.html @@ -28,16 +28,16 @@
{%= __("Clearance Date") %}: {%= frappe.datetime.str_to_user(data[i]["clearance_date"]) %} {% } %} - {%= format_currency(data[i]["debit"]) %} - {%= format_currency(data[i]["credit"]) %} + {%= format_currency(data[i]["debit"], data[i]["account_currency"]) %} + {%= format_currency(data[i]["credit"], data[i]["account_currency"]) %} {% } else { %} {%= data[i]["payment_entry"] %} - {%= format_currency(data[i]["debit"]) %} - {%= format_currency(data[i]["credit"]) %} + {%= format_currency(data[i]["debit"], data[i]["account_currency"]) %} + {%= format_currency(data[i]["credit"], data[i]["account_currency"]) %} {% } %} {% } %} From 21e5620e924563455237262a9df3bc2211672e47 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Mon, 20 Jul 2026 12:29:21 +0530 Subject: [PATCH 22/37] fix: project % complete field allowing modification when manual method (backport #57274) (#57275) Co-authored-by: nishkagosalia --- erpnext/projects/doctype/project/project.json | 4 +- erpnext/projects/doctype/project/project.py | 2 + .../projects/doctype/project/test_project.py | 55 +++++++++++++++++++ 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/erpnext/projects/doctype/project/project.json b/erpnext/projects/doctype/project/project.json index f464fa31d82..649aa888fbe 100644 --- a/erpnext/projects/doctype/project/project.json +++ b/erpnext/projects/doctype/project/project.json @@ -117,7 +117,7 @@ "fieldtype": "Percent", "label": "% Completed", "no_copy": 1, - "read_only": 1 + "read_only_depends_on": "eval:doc.percent_complete_method != 'Manual'" }, { "fieldname": "column_break_5", @@ -466,7 +466,7 @@ "index_web_pages_for_search": 1, "links": [], "max_attachments": 4, - "modified": "2026-07-14 14:32:11.328347", + "modified": "2026-07-21 11:23:22.000000", "modified_by": "Administrator", "module": "Projects", "name": "Project", diff --git a/erpnext/projects/doctype/project/project.py b/erpnext/projects/doctype/project/project.py index 0259a64ef09..80ef45c099d 100644 --- a/erpnext/projects/doctype/project/project.py +++ b/erpnext/projects/doctype/project/project.py @@ -224,6 +224,8 @@ class Project(Document): if self.percent_complete_method == "Manual": if self.status == "Completed": self.percent_complete = 100 + elif flt(self.percent_complete) < 0 or flt(self.percent_complete) > 100: + frappe.throw(_("% Complete must be between 0 and 100")) return total = frappe.db.count("Task", dict(project=self.name)) diff --git a/erpnext/projects/doctype/project/test_project.py b/erpnext/projects/doctype/project/test_project.py index edae80fd4d2..75e1eba9a16 100644 --- a/erpnext/projects/doctype/project/test_project.py +++ b/erpnext/projects/doctype/project/test_project.py @@ -227,6 +227,61 @@ class TestProject(FrappeTestCase): project.save() self.assertEqual(project.status, "Completed") + def _project_with_tasks(self, method, count): + name = f"_Test PercentComplete {frappe.generate_hash(length=8)}" + project = frappe.get_doc( + { + "doctype": "Project", + "project_name": name, + "status": "Open", + "percent_complete_method": method, + "company": "_Test Company", + "expected_start_date": nowdate(), + } + ).insert() + task_names = [] + for i in range(count): + task = frappe.get_doc( + { + "doctype": "Task", + "subject": f"{name} Task {i}", + "project": project.name, + "status": "Open", + "exp_start_date": nowdate(), + "exp_end_date": nowdate(), + } + ).insert() + task_names.append(task.name) + return project, task_names + + def test_percent_complete_manual(self): + project, tasks = self._project_with_tasks("Manual", 2) + + # manual value is preserved on save, even with linked tasks + project.percent_complete = 42 + project.save() + self.assertEqual(project.percent_complete, 42) + + # task updates do not overwrite the manual value + frappe.db.set_value("Task", tasks[0], "status", "Completed") + project.update_percent_complete() + self.assertEqual(project.percent_complete, 42) + + # out-of-range values are rejected + project.percent_complete = 150 + self.assertRaises(frappe.ValidationError, project.save) + project.reload() + + project.percent_complete = -10 + self.assertRaises(frappe.ValidationError, project.save) + project.reload() + + # Completed status forces 100 regardless of the manual value + project.percent_complete = 42 + project.status = "Completed" + project.save() + self.assertEqual(project.percent_complete, 100) + def _create_portal_user(self, email): """A user with no Project-related role, so read access can only come from control_access_for_project_users() sharing the doc with them.""" From f24e09cc9747fa726b58938abd25fd50cfe6355a Mon Sep 17 00:00:00 2001 From: Afsal Syed Date: Mon, 20 Jul 2026 13:08:37 +0530 Subject: [PATCH 23/37] fix: correct typo in allow_negative_stock parameter --- .../stock_and_account_value_comparison.py | 4 ++-- .../stock_ledger_invariant_check.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/erpnext/stock/report/stock_and_account_value_comparison/stock_and_account_value_comparison.py b/erpnext/stock/report/stock_and_account_value_comparison/stock_and_account_value_comparison.py index ec02318ce71..a62c4d8ead9 100644 --- a/erpnext/stock/report/stock_and_account_value_comparison/stock_and_account_value_comparison.py +++ b/erpnext/stock/report/stock_and_account_value_comparison/stock_and_account_value_comparison.py @@ -206,7 +206,7 @@ def create_reposting_entries(rows: str | list, company: str): "posting_date": sle.posting_date, "posting_time": sle.posting_time, "company": company, - "allow_nagative_stock": 1, + "allow_negative_stock": 1, } ).submit() @@ -247,7 +247,7 @@ def repost_based_on_transaction(rows, company=None, entries=None): "posting_date": row.get("posting_date"), "posting_time": row.get("posting_time"), "company": company, - "allow_nagative_stock": 1, + "allow_negative_stock": 1, "recalculate_valuation_rate": 1, } ).submit() diff --git a/erpnext/stock/report/stock_ledger_invariant_check/stock_ledger_invariant_check.py b/erpnext/stock/report/stock_ledger_invariant_check/stock_ledger_invariant_check.py index aef9fec6414..ffb024acfb1 100644 --- a/erpnext/stock/report/stock_ledger_invariant_check/stock_ledger_invariant_check.py +++ b/erpnext/stock/report/stock_ledger_invariant_check/stock_ledger_invariant_check.py @@ -325,7 +325,7 @@ def create_reposting_entries(rows, item_code=None, warehouse=None): "warehouse": warehouse or row.warehouse, "posting_date": row.posting_date, "posting_time": row.posting_time, - "allow_nagative_stock": 1, + "allow_negative_stock": 1, } ).submit() From 90009a4687f1d71fa9848c5bf841d1f2ad22c07a Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Mon, 20 Jul 2026 20:05:37 +0530 Subject: [PATCH 24/37] feat(stock): expose all Bin qty fields in Stock Summary and Stock Projected Qty Stock Summary's sort selector only offered 5 of Bin's 10 qty fields; add the rest (ordered, requested, planned, reserved for production plan, reserved stock) and extend get_data's or_filters so bins whose only nonzero qty is one of the new fields show up when sorted by it. Sort labels now mirror Bin field labels. Stock Projected Qty report had a column for every Bin qty field except reserved_stock; add it. (cherry picked from commit 59c0c15c2ed9a82369358856cca212d8ceb4b01f) --- erpnext/stock/dashboard/item_dashboard.py | 5 +++++ .../stock/page/stock_balance/stock_balance.js | 18 +++++++++++++----- .../stock_projected_qty/stock_projected_qty.py | 9 +++++++++ 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/erpnext/stock/dashboard/item_dashboard.py b/erpnext/stock/dashboard/item_dashboard.py index e5635bb4093..ac83e207d07 100644 --- a/erpnext/stock/dashboard/item_dashboard.py +++ b/erpnext/stock/dashboard/item_dashboard.py @@ -53,6 +53,11 @@ def get_data( "reserved_qty": ["!=", 0], "reserved_qty_for_production": ["!=", 0], "reserved_qty_for_sub_contract": ["!=", 0], + "reserved_qty_for_production_plan": ["!=", 0], + "reserved_stock": ["!=", 0], + "ordered_qty": ["!=", 0], + "indented_qty": ["!=", 0], + "planned_qty": ["!=", 0], "actual_qty": ["!=", 0], }, filters=filters, diff --git a/erpnext/stock/page/stock_balance/stock_balance.js b/erpnext/stock/page/stock_balance/stock_balance.js index a5fba9f98f3..531e335dfdb 100644 --- a/erpnext/stock/page/stock_balance/stock_balance.js +++ b/erpnext/stock/page/stock_balance/stock_balance.js @@ -48,11 +48,19 @@ frappe.pages["stock-balance"].on_page_load = function (wrapper) { sort_by: "projected_qty", sort_order: "asc", options: [ - { fieldname: "projected_qty", label: __("Projected qty") }, - { fieldname: "reserved_qty", label: __("Reserved for sale") }, - { fieldname: "reserved_qty_for_production", label: __("Reserved for manufacturing") }, - { fieldname: "reserved_qty_for_sub_contract", label: __("Reserved for sub contracting") }, - { fieldname: "actual_qty", label: __("Actual qty in stock") }, + { fieldname: "projected_qty", label: __("Projected Qty") }, + { fieldname: "reserved_qty", label: __("Reserved Qty") }, + { fieldname: "reserved_qty_for_production", label: __("Reserved Qty for Production") }, + { fieldname: "reserved_qty_for_sub_contract", label: __("Reserved Qty for Subcontract") }, + { + fieldname: "reserved_qty_for_production_plan", + label: __("Reserved Qty for Production Plan"), + }, + { fieldname: "reserved_stock", label: __("Reserved Stock") }, + { fieldname: "ordered_qty", label: __("Ordered Qty") }, + { fieldname: "indented_qty", label: __("Requested Qty") }, + { fieldname: "planned_qty", label: __("Planned Qty") }, + { fieldname: "actual_qty", label: __("Actual Qty") }, ], }, change: function (sort_by, sort_order) { diff --git a/erpnext/stock/report/stock_projected_qty/stock_projected_qty.py b/erpnext/stock/report/stock_projected_qty/stock_projected_qty.py index 3193ba3de51..3bb557d42c2 100644 --- a/erpnext/stock/report/stock_projected_qty/stock_projected_qty.py +++ b/erpnext/stock/report/stock_projected_qty/stock_projected_qty.py @@ -84,6 +84,7 @@ def execute(filters=None): bin.reserved_qty_for_production_plan, bin.reserved_qty_for_sub_contract, reserved_qty_for_pos, + bin.reserved_stock, bin.projected_qty, re_order_level, re_order_qty, @@ -200,6 +201,13 @@ def get_columns(): "width": 100, "convertible": "qty", }, + { + "label": _("Reserved Stock"), + "fieldname": "reserved_stock", + "fieldtype": "Float", + "width": 100, + "convertible": "qty", + }, { "label": _("Projected Qty"), "fieldname": "projected_qty", @@ -246,6 +254,7 @@ def get_bin_list(filters): bin.reserved_qty_for_production, bin.reserved_qty_for_sub_contract, bin.reserved_qty_for_production_plan, + bin.reserved_stock, bin.projected_qty, ) .orderby(bin.item_code, bin.warehouse) From 2cd531d099768addb1cf087d7c41b273705c09cd Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Mon, 20 Jul 2026 22:58:36 +0530 Subject: [PATCH 25/37] fix: block changing Stock account type when stock ledger entries exist (backport #57283) (#57284) fix: block changing Stock account type when stock ledger entries exist (#57283) (cherry picked from commit 4cdaa8dba672e5f031ed22a4dbe31719bb0e5c1c) Co-authored-by: rohitwaghchaure --- erpnext/accounts/doctype/account/account.py | 31 +++++++++++++++++++ .../accounts/doctype/account/test_account.py | 25 +++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/erpnext/accounts/doctype/account/account.py b/erpnext/accounts/doctype/account/account.py index 5dc862cc4cd..cdb16c89164 100644 --- a/erpnext/accounts/doctype/account/account.py +++ b/erpnext/accounts/doctype/account/account.py @@ -119,6 +119,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: @@ -207,6 +208,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 4cce4d2ee28..8106e1f7c5b 100644 --- a/erpnext/accounts/doctype/account/test_account.py +++ b/erpnext/accounts/doctype/account/test_account.py @@ -313,6 +313,31 @@ class TestAccount(unittest.TestCase): 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 From 319841d59ca71a9ea845c41a7c4c77a5b3da27fe Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 21 Jul 2026 03:23:32 +0530 Subject: [PATCH 26/37] refactor: rework appointment booking lifecycle and portal verification (backport #57270) (#57294) Co-authored-by: Claude Fable 5 Co-authored-by: Diptanil Saha --- .../crm/doctype/appointment/appointment.json | 46 +- .../crm/doctype/appointment/appointment.py | 529 +++++++++++------ .../doctype/appointment/test_appointment.py | 540 +++++++++++++++++- .../appointment_booking_settings.json | 117 +++- .../appointment_booking_settings.py | 71 ++- .../test_appointment_booking_settings.py | 124 +++- erpnext/hooks.py | 1 + .../emails/appointment_confirmed.html | 6 + .../templates/emails/confirm_appointment.html | 1 + erpnext/www/book_appointment/index.js | 4 +- erpnext/www/book_appointment/index.py | 33 +- .../www/book_appointment/verify/index.html | 2 +- erpnext/www/book_appointment/verify/index.py | 54 +- 13 files changed, 1279 insertions(+), 249 deletions(-) create mode 100644 erpnext/templates/emails/appointment_confirmed.html diff --git a/erpnext/crm/doctype/appointment/appointment.json b/erpnext/crm/doctype/appointment/appointment.json index c26b064c4c5..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,6 +82,7 @@ "fieldname": "customer_email", "fieldtype": "Data", "label": "Email", + "options": "Email", "reqd": 1 }, { @@ -99,14 +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": "2022-12-15 11:11:02.131986", + "modified": "2026-07-20 02:00:00.000000", "modified_by": "Administrator", "module": "CRM", "name": "Appointment", - "name_case": "UPPER CASE", + "naming_rule": "Expression (old style)", "owner": "Administrator", "permissions": [ { @@ -158,8 +193,9 @@ } ], "quick_entry": 1, - "sort_field": "modified", + "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 811cc784cc1..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,103 +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 not 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.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 not 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) - 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 @@ -139,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): @@ -226,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 178b9d2de53..10813811664 100644 --- a/erpnext/crm/doctype/appointment/test_appointment.py +++ b/erpnext/crm/doctype/appointment/test_appointment.py @@ -2,37 +2,175 @@ # See license.txt import datetime -import unittest +from unittest.mock import patch +from urllib.parse import parse_qs, urlparse import frappe +from frappe.tests.utils import FrappeTestCase +from frappe.utils import add_to_date, getdate, now_datetime, set_request +from frappe.utils.data import get_system_timezone, 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.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 -class TestAppointment(unittest.TestCase): - def setUpClass(): - frappe.db.delete("Lead", {"email_id": LEAD_EMAIL}) +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(FrappeTestCase): def setUp(self): + # sending an email commits the transaction (EmailQueue sets its status + # with commit=True), which would break the per-test rollback below + frappe.flags.mute_emails = 1 + 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 tearDown(self): + frappe.db.rollback() + frappe.clear_document_cache("Appointment Booking Settings", "Appointment Booking Settings") + frappe.flags.mute_emails = 0 + + 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=get_system_timezone(), + 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) @@ -40,3 +178,369 @@ class TestAppointment(unittest.TestCase): 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): + 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 436eb10c888..b45a0f1bbde 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,18 +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, "issingle": 1, "links": [], - "modified": "2022-12-15 11:10:13.517742", + "modified": "2026-07-20 00:11:18.996384", "modified_by": "Administrator", "module": "CRM", "name": "Appointment Booking Settings", @@ -137,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, @@ -144,4 +213,4 @@ "sort_order": "DESC", "states": [], "track_changes": 1 -} \ No newline at end of file +} 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 9997f97dcc8..57f78f6d300 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): @@ -26,33 +26,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) @@ -67,3 +77,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 bc68bbd86c6..cc698a40f5e 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,9 +1,125 @@ # Copyright (c) 2019, Frappe Technologies Pvt. Ltd. and Contributors # See license.txt -# import frappe -import unittest +import datetime + +import frappe +from frappe.tests.utils import FrappeTestCase +from frappe.utils import add_to_date, getdate + +from erpnext.setup.doctype.holiday_list.test_holiday_list import make_holiday_list -class TestAppointmentBookingSettings(unittest.TestCase): - pass +class TestAppointmentBookingSettings(FrappeTestCase): + 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 118f047f19c..78352735ec1 100644 --- a/erpnext/hooks.py +++ b/erpnext/hooks.py @@ -431,6 +431,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/templates/emails/appointment_confirmed.html b/erpnext/templates/emails/appointment_confirmed.html new file mode 100644 index 00000000000..12fa2232f58 --- /dev/null +++ b/erpnext/templates/emails/appointment_confirmed.html @@ -0,0 +1,6 @@ +

{{_("Dear")}} {{ full_name }},

+

{{_("Your email has been verified and your appointment has been confirmed for {0}").format(scheduled_time)}}.

+

{{_("We look forward to meeting you")}}.

+ +
+

{{_("This email was sent from {0}").format(site_url)}}

diff --git a/erpnext/templates/emails/confirm_appointment.html b/erpnext/templates/emails/confirm_appointment.html index 6c9b28bc136..ce6a9f88a99 100644 --- a/erpnext/templates/emails/confirm_appointment.html +++ b/erpnext/templates/emails/confirm_appointment.html @@ -1,6 +1,7 @@

{{_("Dear")}} {{ full_name }}{% if last_name %} {{ last_name}}{% endif %},

{{_("A new appointment has been created for you with {0}").format(site_url)}}.

{{_("Click on the link below to verify your email and confirm the appointment")}}.

+

{{_("This link is valid for {0} minutes").format(expiry_minutes)}}.

{{ _("Verify Email") }} diff --git a/erpnext/www/book_appointment/index.js b/erpnext/www/book_appointment/index.js index 0770d102046..0021e47fcf1 100644 --- a/erpnext/www/book_appointment/index.js +++ b/erpnext/www/book_appointment/index.js @@ -237,9 +237,9 @@ async function submit() { frappe.show_alert(__("Appointment Created Successfully")); } setTimeout(() => { - let redirect_url = "/"; + let redirect_url = "/book_appointment"; if (window.appointment_settings.success_redirect_url) { - redirect_url += window.appointment_settings.success_redirect_url; + redirect_url = `/${window.appointment_settings.success_redirect_url}`; } window.location.href = redirect_url; }, 5000); diff --git a/erpnext/www/book_appointment/index.py b/erpnext/www/book_appointment/index.py index 7cdf95c03b5..f00698d11a6 100644 --- a/erpnext/www/book_appointment/index.py +++ b/erpnext/www/book_appointment/index.py @@ -4,6 +4,7 @@ import json import frappe import pytz from frappe import _ +from frappe.rate_limiter import rate_limit from frappe.utils.data import get_system_timezone WEEKDAYS = ["Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday", "Sunday"] @@ -18,7 +19,7 @@ def get_context(context): def handle_appointment_booking_disabled(): - if not frappe.get_single_value("Appointment Booking Settings", "enable_scheduling"): + if not frappe.get_single_value("Appointment Booking Settings", "enable_appointment_portal"): frappe.redirect_to_message( _("Appointment Scheduling Disabled"), _("Appointment Scheduling has been disabled for this site"), @@ -66,6 +67,8 @@ def get_appointment_slots(date, timezone): ) holiday_list = frappe.get_doc("Holiday List", settings.holiday_list) timeslots = get_available_slots_between(query_start_time, query_end_time, settings) + # fetch the day's booked slots once instead of querying per timeslot + booked_times = get_booked_slot_times_for(timeslots, settings.appointment_duration) # Filter and convert timeslots converted_timeslots = [] @@ -76,7 +79,7 @@ def get_appointment_slots(date, timezone): converted_timeslots.append(dict(time=converted_timeslot, availability=False)) continue # Check availability - if check_availabilty(timeslot, settings) and converted_timeslot >= now: + if is_slot_available(timeslot, booked_times, settings) and converted_timeslot >= now: converted_timeslots.append(dict(time=converted_timeslot, availability=True)) else: converted_timeslots.append(dict(time=converted_timeslot, availability=False)) @@ -102,7 +105,8 @@ def get_available_slots_between(query_start_time, query_end_time, settings): return timeslots -@frappe.whitelist(allow_guest=True) +@frappe.whitelist(allow_guest=True, methods=["POST"]) +@rate_limit(limit=5, seconds=300) def create_appointment(date, time, tz, contact): handle_appointment_booking_disabled() format_string = "%Y-%m-%d %H:%M:%S" @@ -114,13 +118,13 @@ def create_appointment(date, time, tz, contact): # Create a appointment document from form appointment = frappe.new_doc("Appointment") appointment.scheduled_time = scheduled_time - contact = json.loads(contact) + contact = frappe.parse_json(contact) appointment.customer_name = contact.get("name", None) appointment.customer_phone_number = contact.get("number", None) appointment.customer_skype = contact.get("skype", None) appointment.customer_details = contact.get("notes", None) appointment.customer_email = contact.get("email", None) - appointment.status = "Open" + appointment.created_through_portal = 1 appointment.insert(ignore_permissions=True) return appointment @@ -150,8 +154,23 @@ def convert_to_system_timezone(guest_tz, datetimeobject): return datetimeobject -def check_availabilty(timeslot, settings): - return frappe.db.count("Appointment", {"scheduled_time": timeslot}) < settings.number_of_agents +def get_booked_slot_times_for(timeslots, appointment_duration): + if not timeslots: + return [] + + from erpnext.crm.doctype.appointment.appointment import get_booked_slot_times + + duration = datetime.timedelta(minutes=appointment_duration) + return get_booked_slot_times(min(timeslots) - duration, max(timeslots) + duration) + + +def is_slot_available(timeslot, booked_times, settings): + # mirror the server capacity check: count non-Closed appointments whose + # duration window overlaps this slot, without a per-slot query + duration = datetime.timedelta(minutes=settings.appointment_duration) + lower, upper = timeslot - duration, timeslot + duration + overlapping = sum(1 for booked in booked_times if lower < booked < upper) + return overlapping < settings.number_of_agents def _is_holiday(date, holiday_list): diff --git a/erpnext/www/book_appointment/verify/index.html b/erpnext/www/book_appointment/verify/index.html index 58c07e85ccc..8e8a1096e5e 100644 --- a/erpnext/www/book_appointment/verify/index.html +++ b/erpnext/www/book_appointment/verify/index.html @@ -12,7 +12,7 @@ {% else %}

- {{ _("Verification failed please check the link") }} + {{ message or _("Verification failed please check the link") }}
{% endif %} {% endblock%} diff --git a/erpnext/www/book_appointment/verify/index.py b/erpnext/www/book_appointment/verify/index.py index 3beb8667ae7..5b84a37aec7 100644 --- a/erpnext/www/book_appointment/verify/index.py +++ b/erpnext/www/book_appointment/verify/index.py @@ -1,20 +1,58 @@ import frappe -from frappe.utils.verified_command import verify_request +from frappe import _ +from frappe.utils import add_to_date, now_datetime +from frappe.utils.data import sha256_hash + +from erpnext.crm.doctype.appointment.appointment import get_verification_link_expiry def get_context(context): - if not verify_request(): + key = frappe.form_dict.get("key") + if not key: context.success = False return context - email = frappe.form_dict["email"] - appointment_name = frappe.form_dict["appointment"] + appointment_name = frappe.db.get_value("Appointment", {"verification_token": sha256_hash(key)}, "name") + if not appointment_name: + context.success = False + context.message = _("This verification link is invalid. Please book the appointment again.") + return context - if email and appointment_name: - appointment = frappe.get_doc("Appointment", appointment_name) - appointment.set_verified(email) + appointment = frappe.get_doc("Appointment", appointment_name) + + # report a settled status before expiry: a closed/verified appointment is + # more informative than a generic "expired" (and creation-based expiry would + # otherwise mask a sweeper-closed appointment) + if appointment.status == "Closed": + context.success = False + context.message = _("Appointment has been closed. Please book the appointment again.") + return context + + if appointment.status == "Open": context.success = True + context.message = _("Appointment is already verified.") return context - else: + + if now_datetime() > add_to_date(appointment.creation, minutes=get_verification_link_expiry()): context.success = False + context.message = _("Verification link has expired.") return context + + verify_appointment(appointment) + # GET requests are rolled back at the end of the request unless this flag is set + frappe.local.flags.commit = True + context.success = True + return context + + +def verify_appointment(appointment): + # the signed link is the authorization; materializing the appointment + # (agent assignment) needs system privileges the Guest visitor lacks + visitor = frappe.session.user + try: + frappe.set_user("Administrator") + appointment.email_verified = True + appointment.status = "Open" + appointment.save(ignore_permissions=True) + finally: + frappe.set_user(visitor) From 88abe119c31496e2bc288aedc6460130ff315d17 Mon Sep 17 00:00:00 2001 From: khushi8112 Date: Tue, 21 Jul 2026 11:34:40 +0530 Subject: [PATCH 27/37] fix: apply default accounting dimensions reliably on new documents Default accounting dimensions were applied from the `company` client trigger, which reads dimension data fetched asynchronously in `setup_dimension_filters`. When the trigger fired before that fetch returned, new documents were left without their default dimensions, and Sales Order never called `update_dimension` at all. - Apply defaults from the fetch callback in dimension_tree_filter.js so they no longer depend on `company`-trigger timing. This fixes the race for Sales Invoice, Purchase Invoice and Payment Entry, and populates defaults on new Sales Orders. - Add a `company()` override on SalesOrderController so the default is re-applied when the company changes, matching the sibling doctypes. Co-Authored-By: Claude Opus 4.8 --- erpnext/public/js/utils/dimension_tree_filter.js | 1 + erpnext/selling/doctype/sales_order/sales_order.js | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/erpnext/public/js/utils/dimension_tree_filter.js b/erpnext/public/js/utils/dimension_tree_filter.js index 68bf11de5d8..5b463066260 100644 --- a/erpnext/public/js/utils/dimension_tree_filter.js +++ b/erpnext/public/js/utils/dimension_tree_filter.js @@ -22,6 +22,7 @@ erpnext.accounts.dimensions = { }); me.default_dimensions = r.message[1]; me.setup_filters(frm, doctype); + me.update_dimension(frm, doctype); }, }); }, diff --git a/erpnext/selling/doctype/sales_order/sales_order.js b/erpnext/selling/doctype/sales_order/sales_order.js index edfcb0becfe..635a43c3113 100644 --- a/erpnext/selling/doctype/sales_order/sales_order.js +++ b/erpnext/selling/doctype/sales_order/sales_order.js @@ -589,6 +589,11 @@ erpnext.selling.SalesOrderController = class SalesOrderController extends erpnex super.onload(doc, dt, dn); } + company() { + super.company(); + erpnext.accounts.dimensions.update_dimension(this.frm, this.frm.doctype); + } + refresh(doc, dt, dn) { var me = this; super.refresh(); From d51f9076b5b008cb14d89e473dbe9271362a9247 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Tue, 21 Jul 2026 13:44:36 +0530 Subject: [PATCH 28/37] fix: rescale stock ageing FIFO slot values on stock reconciliation A reconciliation's stock_value_difference includes the revaluation of stock already in the FIFO queue, but the whole amount was attached to the qty-delta slot while older slots kept pre-revaluation values. A downward revaluation therefore produced negative bucket values in the Stock Ageing report, and repeated recos let the queue total drift away from Stock Balance. Re-derive every slot value as qty * valuation_rate after processing a reco SLE, since a reconciliation values the entire balance at its rate. Covers both single-SLE recos and the zero-out/re-add pair that flows through the transfer bucket. --- .../stock/report/stock_ageing/stock_ageing.py | 9 ++ .../report/stock_ageing/test_stock_ageing.py | 85 +++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/erpnext/stock/report/stock_ageing/stock_ageing.py b/erpnext/stock/report/stock_ageing/stock_ageing.py index e0106f7ddb0..f519527fc8b 100644 --- a/erpnext/stock/report/stock_ageing/stock_ageing.py +++ b/erpnext/stock/report/stock_ageing/stock_ageing.py @@ -370,6 +370,7 @@ class FIFOSlots: row, fifo_queue, transferred_item_key, serial_nos, batch_nos, from_end ) + self._revalue_stock_reconciliation_slots(row, fifo_queue) self._update_balances(row, key) self._trim_serial_fifo_queue(row, key, fifo_queue) @@ -393,6 +394,14 @@ class FIFOSlots: # Stock reconciliation stores the final balance; FIFO needs the movement delta. row.actual_qty = flt(row.qty_after_transaction) - flt(prev_balance_qty) + def _revalue_stock_reconciliation_slots(self, row: dict, fifo_queue: list) -> None: + if row.voucher_type != "Stock Reconciliation" or row.has_serial_no or row.has_batch_no: + return + + for slot in fifo_queue: + if is_qty_slot(slot): + slot[FIFO_VALUE_INDEX] = flt(slot[FIFO_QTY_INDEX] * flt(row.valuation_rate)) + def _get_serial_and_batch_nos( self, row: dict, bundle_wise_serial_nos: dict, bundle_wise_batch_nos: dict ) -> tuple[list, list]: diff --git a/erpnext/stock/report/stock_ageing/test_stock_ageing.py b/erpnext/stock/report/stock_ageing/test_stock_ageing.py index 2f74e1e3327..a9d0fe16ec5 100644 --- a/erpnext/stock/report/stock_ageing/test_stock_ageing.py +++ b/erpnext/stock/report/stock_ageing/test_stock_ageing.py @@ -383,6 +383,91 @@ class TestStockAgeing(FrappeTestCase): self.assertEqual(queue, [[60.0, "2025-11-30", 60.0], [30.0, "2026-01-31", 30.0]]) self.assertEqual(report_data[0][7:15], [30.0, 30.0, 0.0, 0.0, 60.0, 60.0, 0.0, 0.0]) + def test_stock_reco_revaluation_rescales_queue_values(self): + "Ledger (same wh): [+15 @ 100, reco reset >> 20 @ 50]" + sle = [ + frappe._dict( + name="Flask Item", + actual_qty=15, + qty_after_transaction=15, + stock_value_difference=1500, + valuation_rate=100, + warehouse="WH 1", + posting_date="2021-12-01", + voucher_type="Stock Entry", + voucher_no="001", + has_serial_no=False, + serial_no=None, + ), + frappe._dict( + name="Flask Item", + actual_qty=0, + qty_after_transaction=20, + stock_value_difference=(-500), + valuation_rate=50, + warehouse="WH 1", + posting_date="2021-12-02", + voucher_type="Stock Reconciliation", + voucher_no="002", + has_serial_no=False, + serial_no=None, + ), + ] + + slots = FIFOSlots(self.filters, sle).generate() + queue = slots["Flask Item"]["fifo_queue"] + + self.assertEqual(queue, [[15.0, "2021-12-01", 750.0], [5.0, "2021-12-02", 250.0]]) + + def test_stock_reco_with_split_out_and_in_sles_revalues_queue(self): + "Ledger (same wh): [+10 @ 100, reco out >> 0, reco in >> 12 @ 2]" + sle = [ + frappe._dict( + name="Flask Item", + actual_qty=10, + qty_after_transaction=10, + stock_value_difference=1000, + valuation_rate=100, + warehouse="WH 1", + posting_date="2021-12-01", + voucher_type="Stock Entry", + voucher_no="001", + has_serial_no=False, + serial_no=None, + ), + frappe._dict( + name="Flask Item", + actual_qty=(-10), + qty_after_transaction=0, + stock_value_difference=(-1000), + valuation_rate=100, + warehouse="WH 1", + posting_date="2021-12-02", + voucher_type="Stock Reconciliation", + voucher_no="002", + has_serial_no=False, + serial_no=None, + ), + frappe._dict( + name="Flask Item", + actual_qty=12, + qty_after_transaction=12, + stock_value_difference=24, + valuation_rate=2, + warehouse="WH 1", + posting_date="2021-12-02", + voucher_type="Stock Reconciliation", + voucher_no="002", + has_serial_no=False, + serial_no=None, + ), + ] + + slots = FIFOSlots(self.filters, sle).generate() + queue = slots["Flask Item"]["fifo_queue"] + + self.assertEqual(queue, [[10.0, "2021-12-01", 20.0], [2.0, "2021-12-02", 4.0]]) + def test_sequential_stock_reco_same_warehouse(self): """ Test back to back stock recos (same warehouse). From 72b3210cbb24e6ed1f6691463a17cb978cac50cd Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Tue, 21 Jul 2026 13:49:21 +0530 Subject: [PATCH 29/37] fix: rescale batch FIFO slot values on stock reconciliation Batch items take the batch-slot path, which mirrors the same value arithmetic: the reco's incoming entry dumps the revaluation remainder on one slot. Rescale each reconciled batch's slots at its post-reco rate (stock_value_difference / qty of the incoming bundle entry). --- .../stock/report/stock_ageing/stock_ageing.py | 21 ++++++-- .../report/stock_ageing/test_stock_ageing.py | 50 +++++++++++++++++++ 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/erpnext/stock/report/stock_ageing/stock_ageing.py b/erpnext/stock/report/stock_ageing/stock_ageing.py index f519527fc8b..5f035da69b8 100644 --- a/erpnext/stock/report/stock_ageing/stock_ageing.py +++ b/erpnext/stock/report/stock_ageing/stock_ageing.py @@ -370,7 +370,7 @@ class FIFOSlots: row, fifo_queue, transferred_item_key, serial_nos, batch_nos, from_end ) - self._revalue_stock_reconciliation_slots(row, fifo_queue) + self._revalue_stock_reconciliation_slots(row, fifo_queue, batch_nos) self._update_balances(row, key) self._trim_serial_fifo_queue(row, key, fifo_queue) @@ -394,14 +394,29 @@ class FIFOSlots: # Stock reconciliation stores the final balance; FIFO needs the movement delta. row.actual_qty = flt(row.qty_after_transaction) - flt(prev_balance_qty) - def _revalue_stock_reconciliation_slots(self, row: dict, fifo_queue: list) -> None: - if row.voucher_type != "Stock Reconciliation" or row.has_serial_no or row.has_batch_no: + def _revalue_stock_reconciliation_slots(self, row: dict, fifo_queue: list, batch_nos: list) -> None: + if row.voucher_type != "Stock Reconciliation" or row.has_serial_no: + return + + if row.has_batch_no: + if flt(row.actual_qty) > 0: + self._revalue_reconciled_batch_slots(fifo_queue, batch_nos) return for slot in fifo_queue: if is_qty_slot(slot): slot[FIFO_VALUE_INDEX] = flt(slot[FIFO_QTY_INDEX] * flt(row.valuation_rate)) + def _revalue_reconciled_batch_slots(self, fifo_queue: list, batch_nos: list) -> None: + for batch_no, _use_batchwise_valuation, qty, stock_value_difference in batch_nos: + if not flt(qty): + continue + + rate = flt(stock_value_difference) / flt(qty) + for slot in fifo_queue: + if is_batch_slot(slot) and slot[BATCH_SLOT_BATCH_INDEX] == batch_no: + slot[BATCH_SLOT_VALUE_INDEX] = flt(slot[BATCH_SLOT_QTY_INDEX] * rate) + def _get_serial_and_batch_nos( self, row: dict, bundle_wise_serial_nos: dict, bundle_wise_batch_nos: dict ) -> tuple[list, list]: diff --git a/erpnext/stock/report/stock_ageing/test_stock_ageing.py b/erpnext/stock/report/stock_ageing/test_stock_ageing.py index a9d0fe16ec5..b44003a170c 100644 --- a/erpnext/stock/report/stock_ageing/test_stock_ageing.py +++ b/erpnext/stock/report/stock_ageing/test_stock_ageing.py @@ -468,6 +468,56 @@ class TestStockAgeing(FrappeTestCase): self.assertEqual(queue, [[10.0, "2021-12-01", 20.0], [2.0, "2021-12-02", 4.0]]) + def test_batch_stock_reco_revaluation_rescales_slot_values(self): + "Ledger (same wh, batch B): [+10 @ 100, reco out >> 0, reco in >> 12 @ 2]" + from erpnext.stock.doctype.item.test_item import make_item + + item_code = make_item( + "Test Stock Ageing Batch Reco Revaluation", + {"is_stock_item": 1, "has_batch_no": 1, "valuation_method": "FIFO"}, + ).name + + batch_no = "SA-RECO-REVALUE-BATCH" + if not frappe.db.exists("Batch", batch_no): + frappe.get_doc({"doctype": "Batch", "batch_id": batch_no, "item": item_code}).insert( + ignore_permissions=True + ) + frappe.db.set_value("Batch", batch_no, "use_batchwise_valuation", 1) + + def make_sle(posting_date, voucher_type, voucher_no, actual_qty, qty_after, stock_value_difference): + return frappe._dict( + name=item_code, + actual_qty=actual_qty, + qty_after_transaction=qty_after, + stock_value_difference=stock_value_difference, + valuation_rate=abs(stock_value_difference / actual_qty), + warehouse="WH 1", + posting_date=posting_date, + voucher_type=voucher_type, + voucher_no=voucher_no, + has_serial_no=False, + has_batch_no=True, + serial_no=None, + batch_no=batch_no, + ) + + sle = [ + make_sle("2021-12-01", "Stock Entry", "001", 10, 10, 1000), + make_sle("2021-12-02", "Stock Reconciliation", "002", -10, 0, -1000), + make_sle("2021-12-02", "Stock Reconciliation", "002", 12, 12, 24), + ] + + slots = FIFOSlots(self.filters, sle).generate() + queue = slots[item_code]["fifo_queue"] + + self.assertEqual( + queue, + [ + [batch_no, 1, 10.0, "2021-12-01", 20.0], + [batch_no, 1, 2.0, "2021-12-02", 4.0], + ], + ) + def test_sequential_stock_reco_same_warehouse(self): """ Test back to back stock recos (same warehouse). From 2673029bd44ae9df396b9e577687a10ff8e63246 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Tue, 21 Jul 2026 14:09:28 +0530 Subject: [PATCH 30/37] fix: revalue batch reco slots only when the entry covers the full batch stock_value_difference / qty equals the new batch rate only when the reco entry carries the entire batch, as the split out/in reco SLEs and batches reconciled from zero do. Partial direct-batch_no entries mix a qty delta with existing stock, so their slots keep prior values. Plain items need no such guard: the valuation engine collapses the FIFO stack to qty_after * valuation_rate on every reconciliation, so rescaling remaining slots at the reco rate matches the ledger. Lock that with a test. --- .../stock/report/stock_ageing/stock_ageing.py | 13 ++- .../report/stock_ageing/test_stock_ageing.py | 104 +++++++++++++++++- 2 files changed, 113 insertions(+), 4 deletions(-) diff --git a/erpnext/stock/report/stock_ageing/stock_ageing.py b/erpnext/stock/report/stock_ageing/stock_ageing.py index 5f035da69b8..06c2e6882f6 100644 --- a/erpnext/stock/report/stock_ageing/stock_ageing.py +++ b/erpnext/stock/report/stock_ageing/stock_ageing.py @@ -412,10 +412,17 @@ class FIFOSlots: if not flt(qty): continue + slots = [ + slot + for slot in fifo_queue + if is_batch_slot(slot) and slot[BATCH_SLOT_BATCH_INDEX] == batch_no + ] + if flt(sum(flt(slot[BATCH_SLOT_QTY_INDEX]) for slot in slots) - flt(qty), 6): + continue + rate = flt(stock_value_difference) / flt(qty) - for slot in fifo_queue: - if is_batch_slot(slot) and slot[BATCH_SLOT_BATCH_INDEX] == batch_no: - slot[BATCH_SLOT_VALUE_INDEX] = flt(slot[BATCH_SLOT_QTY_INDEX] * rate) + for slot in slots: + slot[BATCH_SLOT_VALUE_INDEX] = flt(slot[BATCH_SLOT_QTY_INDEX] * rate) def _get_serial_and_batch_nos( self, row: dict, bundle_wise_serial_nos: dict, bundle_wise_batch_nos: dict diff --git a/erpnext/stock/report/stock_ageing/test_stock_ageing.py b/erpnext/stock/report/stock_ageing/test_stock_ageing.py index b44003a170c..fb488c47eff 100644 --- a/erpnext/stock/report/stock_ageing/test_stock_ageing.py +++ b/erpnext/stock/report/stock_ageing/test_stock_ageing.py @@ -468,6 +468,57 @@ class TestStockAgeing(FrappeTestCase): self.assertEqual(queue, [[10.0, "2021-12-01", 20.0], [2.0, "2021-12-02", 4.0]]) + def test_stock_reco_decrease_rescales_slots_at_reco_rate(self): + """Ledger (same wh): [+10 @ 100, +20 @ 250, reco reset >> 25 @ 220] + The valuation engine collapses the FIFO stack to qty_after * valuation_rate + on a reco, so remaining slot values follow the reco rate, not the lot rates.""" + sle = [ + frappe._dict( + name="Flask Item", + actual_qty=10, + qty_after_transaction=10, + stock_value_difference=1000, + valuation_rate=100, + warehouse="WH 1", + posting_date="2021-12-01", + voucher_type="Stock Entry", + voucher_no="001", + has_serial_no=False, + serial_no=None, + ), + frappe._dict( + name="Flask Item", + actual_qty=20, + qty_after_transaction=30, + stock_value_difference=5000, + valuation_rate=200, + warehouse="WH 1", + posting_date="2021-12-02", + voucher_type="Stock Entry", + voucher_no="002", + has_serial_no=False, + serial_no=None, + ), + frappe._dict( + name="Flask Item", + actual_qty=0, + qty_after_transaction=25, + stock_value_difference=(-500), + valuation_rate=220, + warehouse="WH 1", + posting_date="2021-12-03", + voucher_type="Stock Reconciliation", + voucher_no="003", + has_serial_no=False, + serial_no=None, + ), + ] + + slots = FIFOSlots(self.filters, sle).generate() + queue = slots["Flask Item"]["fifo_queue"] + + self.assertEqual(queue, [[5.0, "2021-12-01", 1100.0], [20.0, "2021-12-02", 4400.0]]) + def test_batch_stock_reco_revaluation_rescales_slot_values(self): "Ledger (same wh, batch B): [+10 @ 100, reco out >> 0, reco in >> 12 @ 2]" from erpnext.stock.doctype.item.test_item import make_item @@ -490,7 +541,7 @@ class TestStockAgeing(FrappeTestCase): actual_qty=actual_qty, qty_after_transaction=qty_after, stock_value_difference=stock_value_difference, - valuation_rate=abs(stock_value_difference / actual_qty), + valuation_rate=abs(stock_value_difference / actual_qty) if actual_qty else 0, warehouse="WH 1", posting_date=posting_date, voucher_type=voucher_type, @@ -518,6 +569,57 @@ class TestStockAgeing(FrappeTestCase): ], ) + def test_partial_batch_reco_keeps_existing_slot_values(self): + """Ledger (same wh, batch B): [+10 @ 100, single-SLE reco >> 12] + The reco entry qty (delta 2) does not cover the whole batch, so + stock_value_difference / qty is not the batch rate: skip the rescale.""" + from erpnext.stock.doctype.item.test_item import make_item + + item_code = make_item( + "Test Stock Ageing Partial Batch Reco", + {"is_stock_item": 1, "has_batch_no": 1, "valuation_method": "FIFO"}, + ).name + + batch_no = "SA-PARTIAL-RECO-BATCH" + if not frappe.db.exists("Batch", batch_no): + frappe.get_doc({"doctype": "Batch", "batch_id": batch_no, "item": item_code}).insert( + ignore_permissions=True + ) + frappe.db.set_value("Batch", batch_no, "use_batchwise_valuation", 1) + + def make_sle(posting_date, voucher_type, voucher_no, actual_qty, qty_after, stock_value_difference): + return frappe._dict( + name=item_code, + actual_qty=actual_qty, + qty_after_transaction=qty_after, + stock_value_difference=stock_value_difference, + valuation_rate=abs(stock_value_difference / actual_qty) if actual_qty else 0, + warehouse="WH 1", + posting_date=posting_date, + voucher_type=voucher_type, + voucher_no=voucher_no, + has_serial_no=False, + has_batch_no=True, + serial_no=None, + batch_no=batch_no, + ) + + sle = [ + make_sle("2021-12-01", "Stock Entry", "001", 10, 10, 1000), + make_sle("2021-12-02", "Stock Reconciliation", "002", 0, 12, -400), + ] + + slots = FIFOSlots(self.filters, sle).generate() + queue = slots[item_code]["fifo_queue"] + + self.assertEqual( + queue, + [ + [batch_no, 1, 10.0, "2021-12-01", 1000.0], + [batch_no, 1, 2.0, "2021-12-01", 400.0], + ], + ) + def test_sequential_stock_reco_same_warehouse(self): """ Test back to back stock recos (same warehouse). From 1679bdecdcaa99224353ebde4d033d3336a2da47 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Tue, 21 Jul 2026 14:18:19 +0530 Subject: [PATCH 31/37] fix: use system float precision for batch qty comparison --- erpnext/stock/report/stock_ageing/stock_ageing.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/erpnext/stock/report/stock_ageing/stock_ageing.py b/erpnext/stock/report/stock_ageing/stock_ageing.py index 06c2e6882f6..f3d480854fa 100644 --- a/erpnext/stock/report/stock_ageing/stock_ageing.py +++ b/erpnext/stock/report/stock_ageing/stock_ageing.py @@ -408,6 +408,7 @@ class FIFOSlots: slot[FIFO_VALUE_INDEX] = flt(slot[FIFO_QTY_INDEX] * flt(row.valuation_rate)) def _revalue_reconciled_batch_slots(self, fifo_queue: list, batch_nos: list) -> None: + precision = get_float_precision() for batch_no, _use_batchwise_valuation, qty, stock_value_difference in batch_nos: if not flt(qty): continue @@ -417,7 +418,7 @@ class FIFOSlots: for slot in fifo_queue if is_batch_slot(slot) and slot[BATCH_SLOT_BATCH_INDEX] == batch_no ] - if flt(sum(flt(slot[BATCH_SLOT_QTY_INDEX]) for slot in slots) - flt(qty), 6): + if flt(sum(flt(slot[BATCH_SLOT_QTY_INDEX]) for slot in slots) - flt(qty), precision): continue rate = flt(stock_value_difference) / flt(qty) From b9ff5be43e97ed482b18b8577ae3388aa4cb90f1 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Tue, 21 Jul 2026 14:34:08 +0530 Subject: [PATCH 32/37] fix: resolve float precision before streaming stock ledger entries get_single_value inside _revalue_reconciled_batch_slots runs while rows stream through the unbuffered cursor on MariaDB, killing the active iterator. Resolve it once in generate() with the other prefetches. --- erpnext/stock/report/stock_ageing/stock_ageing.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/stock/report/stock_ageing/stock_ageing.py b/erpnext/stock/report/stock_ageing/stock_ageing.py index f3d480854fa..e745a10ea7e 100644 --- a/erpnext/stock/report/stock_ageing/stock_ageing.py +++ b/erpnext/stock/report/stock_ageing/stock_ageing.py @@ -306,6 +306,7 @@ class FIFOSlots: # prepare single sle voucher detail lookup self.prepare_stock_reco_voucher_wise_count() + self.float_precision = get_float_precision() if stock_ledger_entries is None: # streaming path: nested queries invalidate the streaming cursor below, @@ -408,7 +409,6 @@ class FIFOSlots: slot[FIFO_VALUE_INDEX] = flt(slot[FIFO_QTY_INDEX] * flt(row.valuation_rate)) def _revalue_reconciled_batch_slots(self, fifo_queue: list, batch_nos: list) -> None: - precision = get_float_precision() for batch_no, _use_batchwise_valuation, qty, stock_value_difference in batch_nos: if not flt(qty): continue @@ -418,7 +418,7 @@ class FIFOSlots: for slot in fifo_queue if is_batch_slot(slot) and slot[BATCH_SLOT_BATCH_INDEX] == batch_no ] - if flt(sum(flt(slot[BATCH_SLOT_QTY_INDEX]) for slot in slots) - flt(qty), precision): + if flt(sum(flt(slot[BATCH_SLOT_QTY_INDEX]) for slot in slots) - flt(qty), self.float_precision): continue rate = flt(stock_value_difference) / flt(qty) From f0e24e2f53bb09554f2eda56601d99c57484e3d9 Mon Sep 17 00:00:00 2001 From: Shllokkk Date: Tue, 21 Jul 2026 15:04:42 +0530 Subject: [PATCH 33/37] fix: sync process loss percentage when fg qty changes (cherry picked from commit beeffee8f99023ff53eb3e54d1f5e248f03e053c) # Conflicts: # erpnext/stock/doctype/stock_entry/test_stock_entry.py --- .../stock/doctype/stock_entry/stock_entry.py | 2 +- .../doctype/stock_entry/test_stock_entry.py | 53 +++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.py b/erpnext/stock/doctype/stock_entry/stock_entry.py index ebfa7269912..d601dc093b8 100644 --- a/erpnext/stock/doctype/stock_entry/stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/stock_entry.py @@ -2648,7 +2648,7 @@ class StockEntry(StockController): self.process_loss_qty = flt( (flt(self.fg_completed_qty) * flt(self.process_loss_percentage)) / 100 ) - elif self.process_loss_qty and not self.process_loss_percentage: + elif self.process_loss_qty and self.fg_completed_qty: self.process_loss_percentage = flt( (flt(self.process_loss_qty) / flt(self.fg_completed_qty)) * 100 ) diff --git a/erpnext/stock/doctype/stock_entry/test_stock_entry.py b/erpnext/stock/doctype/stock_entry/test_stock_entry.py index ea231ff466c..29e92986563 100644 --- a/erpnext/stock/doctype/stock_entry/test_stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/test_stock_entry.py @@ -2730,7 +2730,60 @@ class TestStockEntry(FrappeTestCase): frappe.delete_doc("Document Naming Rule", qc_naming_rule.name) +<<<<<<< HEAD def make_serialized_item(**args): +======= + frappe.set_value("UOM", "Nos", "must_be_whole_number", 0) + + fg_item = make_item("FG Item", properties={"is_stock_item": 1}).name + rm_item = make_item("RM Item", properties={"is_stock_item": 1}).name + scrap_item = make_item("Scrap Item", properties={"is_stock_item": 1}).name + warehouse = "_Test Warehouse - _TC" + make_stock_entry(item_code=rm_item, target=warehouse, qty=5, rate=10, purpose="Material Receipt") + + bom_no = make_bom( + item=fg_item, raw_materials=[rm_item], scrap_items=[scrap_item], process_loss_percentage=10 + ).name + se = make_stock_entry(item_code=fg_item, qty=5, purpose="Manufacture", do_not_save=True) + se.from_bom = 1 + se.bom_no = bom_no + se.fg_completed_qty = 5 + se.from_warehouse = warehouse + se.to_warehouse = "_Test Warehouse 1 - _TC" + se.get_items() + se.save() + se.reload() + + self.assertEqual(se.items[1].qty, 4.5) + self.assertEqual(se.items[1].amount, 45) + self.assertEqual(se.items[2].qty, 4.5) + self.assertEqual(se.items[2].amount, 5) + + def test_process_loss_percentage_resyncs_from_qty(self): + # changing fg qty recomputes process_loss_qty + se = frappe.new_doc("Stock Entry") + se.purpose = "Manufacture" + se.fg_completed_qty = 200 + se.process_loss_qty = 100 + se.process_loss_percentage = 80 + + se.set_process_loss_qty() + + self.assertEqual(se.process_loss_percentage, 50) + + def test_process_loss_qty_derived_from_percentage_when_qty_blank(self): + se = frappe.new_doc("Stock Entry") + se.purpose = "Manufacture" + se.fg_completed_qty = 200 + se.process_loss_percentage = 25 + + se.set_process_loss_qty() + + self.assertEqual(se.process_loss_qty, 50) + + +def make_serialized_item(self, **args): +>>>>>>> beeffee8f9 (fix: sync process loss percentage when fg qty changes) args = frappe._dict(args) se = frappe.copy_doc(test_records[0]) From e7f0461b5757a218e42d4b3e49040bbf77b1a532 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Tue, 21 Jul 2026 16:45:02 +0530 Subject: [PATCH 34/37] fix: read serial and batch flags from Item in Stock Balance's SLE query Stock Balance builds stock ageing by passing its own SLE rows into FIFOSlots, whose gates read row.has_batch_no and row.has_serial_no. The query never selected has_batch_no at all, and read has_serial_no from the SLE column, which is unset on rows predating v15's bundle migration (no backfill patch exists). With the flags falsy, incoming batch/serial stock was queued as plain qty slots while outgoing was consumed batch/serial-wise, so the shapes never consume each other, and bundle-based rows resolved no batch details, letting the plain consume path overwrite batch slots in place (flt(batch_no) reads as qty 0). Besides wrong ageing figures, sorting the resulting queue crashed with TypeError: '<' not supported between 'int' and 'datetime.date'. Select both flags from Item, matching Stock Ageing's own query. Only this report is affected; develop dropped the sle_entries pass-in in #44489. --- erpnext/stock/report/stock_balance/stock_balance.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/erpnext/stock/report/stock_balance/stock_balance.py b/erpnext/stock/report/stock_balance/stock_balance.py index 2c25b2c5b2b..715b42c168c 100644 --- a/erpnext/stock/report/stock_balance/stock_balance.py +++ b/erpnext/stock/report/stock_balance/stock_balance.py @@ -384,8 +384,9 @@ class StockBalanceReport: sle.batch_no, sle.serial_no, sle.serial_and_batch_bundle, - sle.has_serial_no, sle.voucher_detail_no, + item_table.has_serial_no, + item_table.has_batch_no, item_table.item_group, item_table.stock_uom, item_table.item_name, From 81e865f6c78d522dda01d1fc7df1c5b30effc555 Mon Sep 17 00:00:00 2001 From: Shllokkk Date: Tue, 21 Jul 2026 17:03:41 +0530 Subject: [PATCH 35/37] fix: resolve merge conflicts --- .../doctype/stock_entry/test_stock_entry.py | 33 +------------------ 1 file changed, 1 insertion(+), 32 deletions(-) diff --git a/erpnext/stock/doctype/stock_entry/test_stock_entry.py b/erpnext/stock/doctype/stock_entry/test_stock_entry.py index 29e92986563..79a353ace30 100644 --- a/erpnext/stock/doctype/stock_entry/test_stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/test_stock_entry.py @@ -2729,36 +2729,6 @@ class TestStockEntry(FrappeTestCase): # delete naming rule frappe.delete_doc("Document Naming Rule", qc_naming_rule.name) - -<<<<<<< HEAD -def make_serialized_item(**args): -======= - frappe.set_value("UOM", "Nos", "must_be_whole_number", 0) - - fg_item = make_item("FG Item", properties={"is_stock_item": 1}).name - rm_item = make_item("RM Item", properties={"is_stock_item": 1}).name - scrap_item = make_item("Scrap Item", properties={"is_stock_item": 1}).name - warehouse = "_Test Warehouse - _TC" - make_stock_entry(item_code=rm_item, target=warehouse, qty=5, rate=10, purpose="Material Receipt") - - bom_no = make_bom( - item=fg_item, raw_materials=[rm_item], scrap_items=[scrap_item], process_loss_percentage=10 - ).name - se = make_stock_entry(item_code=fg_item, qty=5, purpose="Manufacture", do_not_save=True) - se.from_bom = 1 - se.bom_no = bom_no - se.fg_completed_qty = 5 - se.from_warehouse = warehouse - se.to_warehouse = "_Test Warehouse 1 - _TC" - se.get_items() - se.save() - se.reload() - - self.assertEqual(se.items[1].qty, 4.5) - self.assertEqual(se.items[1].amount, 45) - self.assertEqual(se.items[2].qty, 4.5) - self.assertEqual(se.items[2].amount, 5) - def test_process_loss_percentage_resyncs_from_qty(self): # changing fg qty recomputes process_loss_qty se = frappe.new_doc("Stock Entry") @@ -2782,8 +2752,7 @@ def make_serialized_item(**args): self.assertEqual(se.process_loss_qty, 50) -def make_serialized_item(self, **args): ->>>>>>> beeffee8f9 (fix: sync process loss percentage when fg qty changes) +def make_serialized_item(**args): args = frappe._dict(args) se = frappe.copy_doc(test_records[0]) From 0249e0dfbeacddb6d054563b92ccb69d2bafeae5 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 21 Jul 2026 20:41:06 +0000 Subject: [PATCH 36/37] chore: remove `apiclient` (backport #57339) (#57340) * chore: remove `apiclient` (#57339) (cherry picked from commit 6d31af3a523198edaec4563b1ad1eaa07c6a29c5) # Conflicts: # pyproject.toml * chore: resolve conflict * chore: bump python-youtube to 0.9.9 --------- Co-authored-by: Diptanil Saha --- erpnext/utilities/doctype/video_settings/video_settings.py | 4 ++-- pyproject.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/erpnext/utilities/doctype/video_settings/video_settings.py b/erpnext/utilities/doctype/video_settings/video_settings.py index 762a795a733..fb7da9ed754 100644 --- a/erpnext/utilities/doctype/video_settings/video_settings.py +++ b/erpnext/utilities/doctype/video_settings/video_settings.py @@ -3,9 +3,9 @@ import frappe -from apiclient.discovery import build from frappe import _ from frappe.model.document import Document +from pyyoutube import Api, PyYouTubeException class VideoSettings(Document): @@ -28,7 +28,7 @@ class VideoSettings(Document): def validate_youtube_api_key(self): if self.enable_youtube_tracking and self.api_key: try: - build("youtube", "v3", developerKey=self.api_key) + Api(api_key=self.api_key).get_i18n_languages(parts="snippet") except Exception: title = _("Failed to Authenticate the API key.") self.log_error("Failed to authenticate API key") diff --git a/pyproject.toml b/pyproject.toml index b6702cb873b..07b27ddec03 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,7 +18,7 @@ dependencies = [ # integration dependencies "googlemaps", "plaid-python~=7.2.1", - "python-youtube~=0.8.0", + "python-youtube~=0.9.9", # Not used directly - required by PyQRCode for PNG generation "pypng~=0.20220715.0", From 3efddfd2702db478672821465b436d026d4361e9 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 21 Jul 2026 21:40:17 +0000 Subject: [PATCH 37/37] fix(payments): ensure `payments` app installed on the site in `payment_app_import_guard` (backport #57342) (#57343) Co-authored-by: Diptanil Saha --- erpnext/utilities/__init__.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/erpnext/utilities/__init__.py b/erpnext/utilities/__init__.py index f01aa1312f6..162e07e2558 100644 --- a/erpnext/utilities/__init__.py +++ b/erpnext/utilities/__init__.py @@ -47,7 +47,11 @@ def payment_app_import_guard(): msg = _("payments app is not installed. Please install it from {} or {}").format( marketplace_link, github_link ) + + if "payments" not in frappe.get_installed_apps(): + frappe.throw(msg, title=_("Missing Payments App"), exc=frappe.AppNotInstalledError) + try: yield except ImportError: - frappe.throw(msg, title=_("Missing Payments App")) + frappe.throw(msg, title=_("Missing Payments App"), exc=frappe.AppNotInstalledError)