From 701ac3edc8043ed8b6141aafacda4fecc116e80b Mon Sep 17 00:00:00 2001 From: iamkhanraheel Date: Thu, 7 May 2026 16:25:10 +0530 Subject: [PATCH 01/11] refactor(expense_claim): change advance allocation in claim from document-driven to ledger-driven using aple --- .../hr/doctype/expense_claim/expense_claim.py | 207 +++++++----------- 1 file changed, 74 insertions(+), 133 deletions(-) diff --git a/hrms/hr/doctype/expense_claim/expense_claim.py b/hrms/hr/doctype/expense_claim/expense_claim.py index 4dcb873269..a1cbe70ae1 100644 --- a/hrms/hr/doctype/expense_claim/expense_claim.py +++ b/hrms/hr/doctype/expense_claim/expense_claim.py @@ -19,7 +19,6 @@ from erpnext.accounts.utils import ( create_gain_loss_journal, unlink_ref_doc_from_payment_entries, - update_reference_in_payment_entry, ) from erpnext.controllers.accounts_controller import AccountsController @@ -206,8 +205,6 @@ def on_submit(self): update_reimbursed_amount(self) self.update_claimed_amount_in_employee_advance() self.create_exchange_gain_loss_je() - if not frappe.db.get_single_value("Accounts Settings", "make_payment_via_journal_entry"): - self.update_against_claim_in_pe() def on_update_after_submit(self): if self.check_if_fields_updated([], {"taxes": ("account_head",), "expenses": ()}): @@ -305,36 +302,30 @@ def get_gl_entries(self): ) ) - make_payment_via_je = frappe.db.get_single_value( - "Accounts Settings", "make_payment_via_journal_entry" - ) # gl entry against advance for data in self.advances: if data.allocated_amount: - gl_dict = { - "account": data.advance_account, - "credit": data.base_allocated_amount, - "credit_in_account_currency": data.allocated_amount, - "credit_in_transaction_currency": data.allocated_amount, - "against": ",".join([d.default_account for d in self.expenses]), - "party_type": "Employee", - "party": self.employee, - "voucher_type": self.doctype, - "voucher_no": self.name, - "advance_voucher_type": "Employee Advance", - "advance_voucher_no": data.employee_advance, - "transaction_exchange_rate": self.exchange_rate, - "cost_center": self.cost_center, - "project": self.project, - } - if not make_payment_via_je: - gl_dict.update( + gl_entry.append( + self.get_gl_dict( { - "against_voucher_type": "Payment Entry", - "against_voucher": data.payment_entry, - } + "account": data.advance_account, + "credit": data.base_allocated_amount, + "credit_in_account_currency": data.allocated_amount, + "credit_in_transaction_currency": data.allocated_amount, + "against": ",".join([d.default_account for d in self.expenses]), + "party_type": "Employee", + "party": self.employee, + "against_voucher_type": data.reference_type, + "against_voucher": data.reference_name, + "advance_voucher_type": data.reference_type, + "advance_voucher_no": data.reference_name, + "transaction_exchange_rate": self.exchange_rate, + "cost_center": self.cost_center, + "project": self.project, + }, + account_currency=self.currency, ) - gl_entry.append(self.get_gl_dict(gl_dict, account_currency=self.currency)) + ) self.add_tax_gl_entries(gl_entry) @@ -592,35 +583,6 @@ def set_expense_account(self, validate=False): "account" ] - def update_against_claim_in_pe(self): - reference_against_pe = [] - for advance in self.advances: - if flt(advance.allocated_amount) > 0: - args = frappe._dict( - { - "voucher_type": "Payment Entry", - "voucher_no": advance.payment_entry, - "against_voucher_type": self.doctype, - "against_voucher": self.name, - "voucher_detail_no": advance.payment_entry_reference, - "account": advance.advance_account, - "party_type": "Employee", - "party": self.employee, - "is_advance": "Yes", - "dr_or_cr": "credit_in_account_currency", - "unadjusted_amount": flt(advance.advance_paid), - "allocated_amount": flt(advance.allocated_amount), - "precision": advance.precision("advance_paid"), - "exchange_rate": advance.exchange_rate, - "difference_posting_date": advance.posting_date, - } - ) - reference_against_pe.append(args) - if reference_against_pe: - for pe_ref in reference_against_pe: - payment_entry = frappe.get_doc("Payment Entry", pe_ref.voucher_no) - update_reference_in_payment_entry(pe_ref, payment_entry, skip_ref_details_update_for_pe=True) - def update_reimbursed_amount(doc): total_amount_reimbursed = get_total_reimbursed_amount(doc) @@ -785,11 +747,7 @@ def get_advances(expense_claim: str | dict | Document, advance_id: str | None = advances = query.run(as_dict=True) - payment_via_journal_entry = frappe.db.get_single_value( - "Accounts Settings", "make_payment_via_journal_entry" - ) for advance in advances: - advance.update({"payment_via_journal_entry": payment_via_journal_entry}) get_expense_claim_advances(expense_claim_doc, advance) return expense_claim_doc.advances @@ -828,25 +786,61 @@ def get_expense_claim(employee_advance: str | dict, payment_via_journal_entry: s def get_expense_claim_advances(expense_claim, employee_advance): - return_amount = flt(employee_advance.return_amount) - if int(employee_advance.payment_via_journal_entry): - paid_amount = flt(employee_advance.paid_amount) - claimed_amount = flt(employee_advance.claimed_amount) - exchange_rate = frappe.db.get_value( - "Advance Payment Ledger Entry", - { - "voucher_type": "Journal Entry", - "against_voucher_type": "Employee Advance", - "against_voucher_no": employee_advance.name, - "delinked": False, - "amount": paid_amount, - }, + advance_payments = frappe.get_all( + "Advance Payment Ledger Entry", + filters={ + "company": expense_claim.company, + "against_voucher_type": "Employee Advance", + "against_voucher_no": employee_advance.name, + "event": "Submit", + "delinked": False, + }, + fields=[ + "voucher_type", + "voucher_no", + "amount", + "base_amount", "exchange_rate", + ], + ) + + if not advance_payments: + return + + advance_payments.sort(key=lambda x: x.get("voucher_no")) + advance_payment_voucher_nos = [payment["voucher_no"] for payment in advance_payments] + + claimed_payments = frappe.get_all( + "Advance Payment Ledger Entry", + filters={ + "company": expense_claim.company, + "event": "Adjustment", + "against_voucher_no": ["in", advance_payment_voucher_nos], + "delinked": False, + }, + fields=[ + "against_voucher_type", + "against_voucher_no", + "amount", + ], + ) + + adjustment_map = {} + for adjustment_entry in claimed_payments: + payment_reference = (adjustment_entry["against_voucher_type"], adjustment_entry["against_voucher_no"]) + adjustment_map[payment_reference] = adjustment_map.get(payment_reference, 0) + abs( + adjustment_entry["amount"] ) + + for advance in advance_payments: + paid_amount = flt(advance["amount"]) + claimed_amount = adjustment_map.get((advance["voucher_type"], advance["voucher_no"]), 0) + unclaimed_amount = paid_amount - claimed_amount + return_amount = flt(employee_advance.return_amount) allocated_amount = get_allocation_amount( - paid_amount=paid_amount, claimed_amount=claimed_amount, return_amount=return_amount + paid_amount=(paid_amount), claimed_amount=(claimed_amount), return_amount=(return_amount) ) - unclaimed_amount = paid_amount - claimed_amount + expense_claim.append( "advances", { @@ -854,68 +848,15 @@ def get_expense_claim_advances(expense_claim, employee_advance): "employee_advance": employee_advance.name, "posting_date": employee_advance.posting_date, "advance_paid": paid_amount, - "base_advance_paid": flt(employee_advance.base_paid_amount), + "base_advance_paid": flt(advance["base_amount"]), "unclaimed_amount": unclaimed_amount, "allocated_amount": allocated_amount, "return_amount": return_amount, - "exchange_rate": exchange_rate, + "exchange_rate": advance["exchange_rate"], + "reference_type": advance["voucher_type"], + "reference_name": advance["voucher_no"], }, ) - else: - pe = frappe.qb.DocType("Payment Entry") - pe_ref = frappe.qb.DocType("Payment Entry Reference") - payment_entries = ( - frappe.qb.from_(pe) - .inner_join(pe_ref) - .on(pe_ref.parent == pe.name) - .select( - (pe.name).as_("payment_entry"), - (pe.total_allocated_amount).as_("advance_paid"), - (pe.unallocated_amount), - (pe.base_total_allocated_amount).as_("base_advance_paid"), - (pe.target_exchange_rate).as_("exchange_rate"), - (pe_ref.name).as_("pe_ref_name"), - (pe_ref.outstanding_amount), - (pe_ref.allocated_amount).as_("pe_ref_allocated_amount"), - ) - .where( - (pe.docstatus == 1) - & (pe_ref.reference_doctype == "Employee Advance") - & (pe_ref.reference_name == employee_advance.name) - & (pe_ref.allocated_amount > 0) - ) - ).run(as_dict=True) - - for pe in payment_entries: - advance_paid = flt(pe.advance_paid) + flt(pe.unallocated_amount) - unclaimed_amount = flt(pe.advance_paid) - if flt(pe.pe_ref_allocated_amount): - unclaimed_amount = flt(pe.pe_ref_allocated_amount) + flt(pe.unallocated_amount) - allocated_amount = get_allocation_amount( - paid_amount=flt(pe.advance_paid), - claimed_amount=(flt(pe.advance_paid) - unclaimed_amount), - return_amount=(return_amount), - ) - - expense_claim.append( - "advances", - { - "advance_account": employee_advance.advance_account, - "employee_advance": employee_advance.name, - "posting_date": employee_advance.posting_date, - "advance_paid": advance_paid, - "base_advance_paid": advance_paid * pe.exchange_rate, - "unclaimed_amount": unclaimed_amount, - "allocated_amount": allocated_amount, - "return_amount": return_amount, - "exchange_rate": pe.exchange_rate, - "payment_entry": pe.payment_entry, - "payment_entry_reference": pe.pe_ref_name - if flt(pe.advance_paid) >= advance_paid - else None, - "purpose": employee_advance.purpose, - }, - ) def update_payment_for_expense_claim(doc, method=None): From d26bbde86532fb1506b5ca51ad1c43e767d8bb9e Mon Sep 17 00:00:00 2001 From: iamkhanraheel Date: Thu, 7 May 2026 17:11:15 +0530 Subject: [PATCH 02/11] refactor(expense_claim): add reference type and reference name field & remove payment entry field from expense claim advance child table --- .../expense_claim_advance.json | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/hrms/hr/doctype/expense_claim_advance/expense_claim_advance.json b/hrms/hr/doctype/expense_claim_advance/expense_claim_advance.json index a427e19e8f..9c5311cd08 100644 --- a/hrms/hr/doctype/expense_claim_advance/expense_claim_advance.json +++ b/hrms/hr/doctype/expense_claim_advance/expense_claim_advance.json @@ -7,7 +7,8 @@ "engine": "InnoDB", "field_order": [ "employee_advance", - "payment_entry", + "reference_type", + "reference_name", "posting_date", "advance_paid", "base_advance_paid", @@ -154,14 +155,6 @@ "options": "Company:company:default_currency", "read_only": 1 }, - { - "fieldname": "payment_entry", - "fieldtype": "Link", - "label": "Payment Entry", - "no_copy": 1, - "options": "Payment Entry", - "read_only": 1 - }, { "fieldname": "payment_entry_reference", "fieldtype": "Data", @@ -170,11 +163,27 @@ "no_copy": 1, "print_hide": 1, "read_only": 1 + }, + { + "fieldname": "reference_type", + "fieldtype": "Select", + "label": "Reference Type", + "no_copy": 1, + "options": "\nPayment Entry\nJournal Entry", + "read_only": 1 + }, + { + "fieldname": "reference_name", + "fieldtype": "Dynamic Link", + "label": "Reference Name", + "no_copy": 1, + "options": "reference_type", + "read_only": 1 } ], "istable": 1, "links": [], - "modified": "2025-11-12 19:48:47.708734", + "modified": "2026-05-07 17:10:55.068753", "modified_by": "Administrator", "module": "HR", "name": "Expense Claim Advance", From bb3a627fabec4ca8946bb80cfd2ddb53321eb7d4 Mon Sep 17 00:00:00 2001 From: iamkhanraheel Date: Thu, 7 May 2026 17:18:57 +0530 Subject: [PATCH 03/11] fix: update reference type & name --- hrms/hr/doctype/expense_claim/expense_claim.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hrms/hr/doctype/expense_claim/expense_claim.js b/hrms/hr/doctype/expense_claim/expense_claim.js index 01d4c50676..56896b06fc 100644 --- a/hrms/hr/doctype/expense_claim/expense_claim.js +++ b/hrms/hr/doctype/expense_claim/expense_claim.js @@ -486,8 +486,8 @@ frappe.ui.form.on("Expense Claim", { row.return_amount = flt(d.return_amount); row.allocated_amount = d.allocated_amount; row.exchange_rate = d.exchange_rate; - row.payment_entry = d.payment_entry; - row.payment_entry_reference = d.payment_entry_reference; + row.reference_type = d.reference_type; + row.reference_name = d.reference_name; }); refresh_field("advances"); } @@ -589,8 +589,8 @@ frappe.ui.form.on("Expense Claim Advance", { child.return_amount = flt(r.message[0].return_amount); child.allocated_amount = flt(r.message[0].allocated_amount); child.exchange_rate = r.message[0].exchange_rate; - child.payment_entry = r.message[0].payment_entry; - child.payment_entry_reference = r.message[0].payment_entry_reference; + child.reference_type = r.message[0].reference_type; + child.reference_name = r.message[0].reference_name; set_in_company_currency( frm, child, From 1f8a36a8d1ce60cf4712d7b8f3231f37fdb713f5 Mon Sep 17 00:00:00 2001 From: iamkhanraheel Date: Fri, 8 May 2026 15:54:36 +0530 Subject: [PATCH 04/11] refactor: remove journal entry creation from employee advance and expense claim payment flow via payment button --- .../employee_advance/employee_advance.js | 14 ++------------ .../employee_advance/employee_advance.py | 5 ----- .../employee_advance/test_employee_advance.py | 2 -- hrms/hr/doctype/expense_claim/expense_claim.js | 17 ++--------------- hrms/hr/doctype/expense_claim/expense_claim.py | 12 +----------- .../expense_claim_advance.json | 3 +-- .../expense_claim_advance.py | 3 ++- 7 files changed, 8 insertions(+), 48 deletions(-) diff --git a/hrms/hr/doctype/employee_advance/employee_advance.js b/hrms/hr/doctype/employee_advance/employee_advance.js index 04195d85f2..07408a1b67 100644 --- a/hrms/hr/doctype/employee_advance/employee_advance.js +++ b/hrms/hr/doctype/employee_advance/employee_advance.js @@ -32,12 +32,7 @@ frappe.ui.form.on("Employee Advance", { frm.doc.docstatus === 1 && flt(frm.doc.paid_amount) < flt(frm.doc.advance_amount) && frappe.model.can_create("Payment Entry") && - !( - (frm.doc.repay_unclaimed_amount_from_salary == 1 && frm.doc.paid_amount) || - (frm.doc.__onload && - frm.doc.__onload.make_payment_via_journal_entry == 1 && - frm.doc.paid_amount) - ) + !(frm.doc.repay_unclaimed_amount_from_salary == 1 && frm.doc.paid_amount) ) { frm.add_custom_button( __("Payment"), @@ -105,12 +100,8 @@ frappe.ui.form.on("Employee Advance", { }, make_payment_entry: function (frm) { - let method = "hrms.overrides.employee_payment_entry.get_payment_entry_for_employee"; - if (frm.doc.__onload && frm.doc.__onload.make_payment_via_journal_entry) { - method = "hrms.hr.doctype.employee_advance.employee_advance.make_bank_entry"; - } return frappe.call({ - method: method, + method: "hrms.overrides.employee_payment_entry.get_payment_entry_for_employee", args: { dt: frm.doc.doctype, dn: frm.doc.name, @@ -127,7 +118,6 @@ frappe.ui.form.on("Employee Advance", { method: "hrms.hr.doctype.expense_claim.expense_claim.get_expense_claim", args: { employee_advance: frm.doc.name, - payment_via_journal_entry: frm.doc.__onload.make_payment_via_journal_entry, }, callback: function (r) { const doclist = frappe.model.sync(r.message); diff --git a/hrms/hr/doctype/employee_advance/employee_advance.py b/hrms/hr/doctype/employee_advance/employee_advance.py index 421d9b8f9a..69e46448cf 100644 --- a/hrms/hr/doctype/employee_advance/employee_advance.py +++ b/hrms/hr/doctype/employee_advance/employee_advance.py @@ -51,11 +51,6 @@ class EmployeeAdvance(Document): ] # end: auto-generated types - def onload(self): - self.get("__onload").make_payment_via_journal_entry = frappe.db.get_single_value( - "Accounts Settings", "make_payment_via_journal_entry" - ) - def validate(self): validate_active_employee(self.employee) self.validate_advance_account_currency() diff --git a/hrms/hr/doctype/employee_advance/test_employee_advance.py b/hrms/hr/doctype/employee_advance/test_employee_advance.py index ed121dfea7..e9ab5ae98f 100644 --- a/hrms/hr/doctype/employee_advance/test_employee_advance.py +++ b/hrms/hr/doctype/employee_advance/test_employee_advance.py @@ -376,7 +376,6 @@ def test_status_on_discard(self): def make_journal_entry_for_advance(advance): - frappe.db.set_single_value("Accounts Settings", "make_payment_via_journal_entry", True) journal_entry = frappe.get_doc(make_bank_entry("Employee Advance", advance.name)) journal_entry.cheque_no = "123123" journal_entry.cheque_date = nowdate() @@ -386,7 +385,6 @@ def make_journal_entry_for_advance(advance): def make_payment_entry(advance, amount=None): - frappe.db.set_single_value("Accounts Settings", "make_payment_via_journal_entry", False) from hrms.overrides.employee_payment_entry import get_payment_entry_for_employee payment_entry = get_payment_entry_for_employee(advance.doctype, advance.name) diff --git a/hrms/hr/doctype/expense_claim/expense_claim.js b/hrms/hr/doctype/expense_claim/expense_claim.js index 56896b06fc..99e4a4b482 100644 --- a/hrms/hr/doctype/expense_claim/expense_claim.js +++ b/hrms/hr/doctype/expense_claim/expense_claim.js @@ -270,16 +270,7 @@ frappe.ui.form.on("Expense Claim", { } if (!frm.doc.__islocal && frm.doc.docstatus === 1) { - let entry_doctype, entry_reference_doctype, entry_reference_name; - if (frm.doc.__onload.make_payment_via_journal_entry) { - entry_doctype = "Journal Entry"; - entry_reference_doctype = "Journal Entry Account.reference_type"; - entry_reference_name = "Journal Entry.reference_name"; - } else { - entry_doctype = "Payment Entry"; - entry_reference_doctype = "Payment Entry Reference.reference_doctype"; - entry_reference_name = "Payment Entry Reference.reference_name"; - } + const entry_doctype = "Payment Entry"; if ( cint(frm.doc.total_amount_reimbursed) > 0 && @@ -366,12 +357,8 @@ frappe.ui.form.on("Expense Claim", { }); }, make_payment_entry: function (frm) { - let method = "hrms.overrides.employee_payment_entry.get_payment_entry_for_employee"; - if (frm.doc.__onload && frm.doc.__onload.make_payment_via_journal_entry) { - method = "hrms.hr.doctype.expense_claim.expense_claim.make_bank_entry"; - } return frappe.call({ - method: method, + method: "hrms.overrides.employee_payment_entry.get_payment_entry_for_employee", args: { dt: frm.doc.doctype, dn: frm.doc.name, diff --git a/hrms/hr/doctype/expense_claim/expense_claim.py b/hrms/hr/doctype/expense_claim/expense_claim.py index a1cbe70ae1..0042905923 100644 --- a/hrms/hr/doctype/expense_claim/expense_claim.py +++ b/hrms/hr/doctype/expense_claim/expense_claim.py @@ -94,9 +94,6 @@ class ExpenseClaim(AccountsController, PWANotificationsMixin): # end: auto-generated types def onload(self): - self.get("__onload").make_payment_via_journal_entry = frappe.db.get_single_value( - "Accounts Settings", "make_payment_via_journal_entry" - ) self.set_onload( "self_expense_approval_not_allowed", frappe.db.get_single_value("HR Settings", "prevent_self_expense_approval"), @@ -753,7 +750,7 @@ def get_advances(expense_claim: str | dict | Document, advance_id: str | None = @frappe.whitelist() -def get_expense_claim(employee_advance: str | dict, payment_via_journal_entry: str | int | bool) -> Document: +def get_expense_claim(employee_advance: str | dict) -> Document: if isinstance(employee_advance, str): employee_advance = frappe.get_doc("Employee Advance", employee_advance) @@ -774,13 +771,6 @@ def get_expense_claim(employee_advance: str | dict, payment_via_journal_entry: s ) expense_claim.cost_center = default_cost_center expense_claim.is_paid = 1 if flt(employee_advance.paid_amount) else 0 - - employee_advance.update( - { - "payment_via_journal_entry": payment_via_journal_entry, - } - ) - get_expense_claim_advances(expense_claim, employee_advance) return expense_claim diff --git a/hrms/hr/doctype/expense_claim_advance/expense_claim_advance.json b/hrms/hr/doctype/expense_claim_advance/expense_claim_advance.json index 9c5311cd08..3eb11a08a5 100644 --- a/hrms/hr/doctype/expense_claim_advance/expense_claim_advance.json +++ b/hrms/hr/doctype/expense_claim_advance/expense_claim_advance.json @@ -81,7 +81,6 @@ "oldfieldtype": "Currency", "options": "currency", "print_width": "120px", - "read_only": 1, "width": "120px" }, { @@ -183,7 +182,7 @@ ], "istable": 1, "links": [], - "modified": "2026-05-07 17:10:55.068753", + "modified": "2026-05-08 15:51:23.314675", "modified_by": "Administrator", "module": "HR", "name": "Expense Claim Advance", diff --git a/hrms/hr/doctype/expense_claim_advance/expense_claim_advance.py b/hrms/hr/doctype/expense_claim_advance/expense_claim_advance.py index d0f547b886..46701349cc 100644 --- a/hrms/hr/doctype/expense_claim_advance/expense_claim_advance.py +++ b/hrms/hr/doctype/expense_claim_advance/expense_claim_advance.py @@ -26,9 +26,10 @@ class ExpenseClaimAdvance(Document): parent: DF.Data parentfield: DF.Data parenttype: DF.Data - payment_entry: DF.Link | None payment_entry_reference: DF.Data | None posting_date: DF.Date | None + reference_name: DF.DynamicLink | None + reference_type: DF.Literal["", "Payment Entry", "Journal Entry"] return_amount: DF.Currency unclaimed_amount: DF.Currency # end: auto-generated types From f88b54fa5b48d5f621d95a0a0902801ad8283e59 Mon Sep 17 00:00:00 2001 From: iamkhanraheel Date: Fri, 8 May 2026 17:22:37 +0530 Subject: [PATCH 05/11] test(employee_advance): use payment entry instead of journal for the payment of advance --- .../employee_advance/employee_advance.py | 41 ----------- .../employee_advance/test_employee_advance.py | 73 +++++++++++++------ .../expense_claim/test_expense_claim.py | 16 ++-- .../payroll_entry/test_payroll_entry.py | 5 +- 4 files changed, 59 insertions(+), 76 deletions(-) diff --git a/hrms/hr/doctype/employee_advance/employee_advance.py b/hrms/hr/doctype/employee_advance/employee_advance.py index 69e46448cf..b382af2472 100644 --- a/hrms/hr/doctype/employee_advance/employee_advance.py +++ b/hrms/hr/doctype/employee_advance/employee_advance.py @@ -281,47 +281,6 @@ def check_linked_payment_entry(self): update_accounting_ledgers_after_reference_removal(self.doctype, self.name) -@frappe.whitelist() -def make_bank_entry(dt: str, dn: str) -> dict: - doc = frappe.get_doc(dt, dn) - payment_account = get_same_currency_bank_cash_account(doc.company, doc.currency, doc.mode_of_payment) - - je = frappe.new_doc("Journal Entry") - je.posting_date = nowdate() - je.voucher_type = "Bank Entry" - je.company = doc.company - je.remark = "Payment against Employee Advance: " + dn + "\n" + doc.purpose - je.multi_currency = 1 if doc.currency != erpnext.get_company_currency(doc.company) else 0 - - je.append( - "accounts", - { - "account": doc.advance_account, - "account_currency": doc.currency, - "debit_in_account_currency": flt(doc.advance_amount), - "reference_type": "Employee Advance", - "reference_name": doc.name, - "party_type": "Employee", - "cost_center": erpnext.get_default_cost_center(doc.company), - "party": doc.employee, - "is_advance": "Yes", - }, - ) - - je.append( - "accounts", - { - "account": payment_account.account or payment_account.name, - "cost_center": erpnext.get_default_cost_center(doc.company), - "credit_in_account_currency": flt(doc.advance_amount), - "account_currency": doc.currency, - "account_type": payment_account.account_type, - }, - ) - - return je.as_dict() - - @frappe.whitelist() def create_return_through_additional_salary(doc: str | dict | Document) -> Document: import json diff --git a/hrms/hr/doctype/employee_advance/test_employee_advance.py b/hrms/hr/doctype/employee_advance/test_employee_advance.py index e9ab5ae98f..b2a04145b2 100644 --- a/hrms/hr/doctype/employee_advance/test_employee_advance.py +++ b/hrms/hr/doctype/employee_advance/test_employee_advance.py @@ -11,7 +11,7 @@ from hrms.hr.doctype.employee_advance.employee_advance import ( EmployeeAdvanceOverPayment, create_return_through_additional_salary, - make_bank_entry, + get_same_currency_bank_cash_account, make_return_entry, ) from hrms.hr.doctype.expense_claim.expense_claim import get_advances @@ -35,7 +35,7 @@ def test_paid_amount_and_status(self): employee_name = make_employee("_T@employee.advance", "_Test Company") advance = make_employee_advance(employee_name) - journal_entry = make_journal_entry_for_advance(advance) + journal_entry = manual_journal_entry_for_advance(advance) journal_entry.submit() advance.reload() @@ -44,22 +44,19 @@ def test_paid_amount_and_status(self): self.assertEqual(advance.status, "Paid") # try making over payment - journal_entry1 = make_journal_entry_for_advance(advance) + journal_entry1 = manual_journal_entry_for_advance(advance) self.assertRaises(EmployeeAdvanceOverPayment, journal_entry1.submit) def test_paid_amount_on_pe_cancellation(self): employee_name = make_employee("_T@employee.advance", "_Test Company") advance = make_employee_advance(employee_name) - - journal_entry = make_journal_entry_for_advance(advance) - journal_entry.submit() - + payment_entry = make_payment_entry(advance) advance.reload() self.assertEqual(advance.paid_amount, 1000) self.assertEqual(advance.status, "Paid") - journal_entry.cancel() + payment_entry.cancel() advance.reload() self.assertEqual(advance.paid_amount, 0) @@ -77,8 +74,7 @@ def test_claimed_status(self): ) advance = make_employee_advance(claim.employee) - journal_entry = make_journal_entry_for_advance(advance) - journal_entry.submit() + make_payment_entry(advance) claim = get_advances_for_claim(claim, advance.name) claim.save() @@ -107,8 +103,7 @@ def test_partly_claimed_and_returned_status(self): ) advance = make_employee_advance(claim.employee) - journal_entry = make_journal_entry_for_advance(advance) - journal_entry.submit() + make_payment_entry(advance) # PARTLY CLAIMED AND RETURNED status check # 500 Claimed, 500 Returned @@ -117,8 +112,7 @@ def test_partly_claimed_and_returned_status(self): ) advance = make_employee_advance(claim.employee) - journal_entry = make_journal_entry_for_advance(advance) - journal_entry.submit() + make_payment_entry(advance) claim = get_advances_for_claim(claim, advance.name, amount=500) claim.save() @@ -165,8 +159,7 @@ def test_partly_claimed_and_returned_status(self): def test_repay_unclaimed_amount_from_salary(self): employee_name = make_employee("_T@employee.advance", "_Test Company") advance = make_employee_advance(employee_name, {"repay_unclaimed_amount_from_salary": 1}) - journal_entry = make_journal_entry_for_advance(advance) - journal_entry.submit() + make_payment_entry(advance) args = {"type": "Deduction"} create_salary_component("Advance Salary - Deduction", **args) @@ -229,8 +222,7 @@ def test_payment_entry_against_advance(self): def test_precision(self): employee_name = make_employee("_T@employee.advance", "_Test Company") advance = make_employee_advance(employee_name) - journal_entry = make_journal_entry_for_advance(advance) - journal_entry.submit() + make_payment_entry(advance) # PARTLY CLAIMED AND RETURNED payable_account = get_payable_account("_Test Company") @@ -375,13 +367,46 @@ def test_status_on_discard(self): self.assertEqual(advance.status, "Cancelled") -def make_journal_entry_for_advance(advance): - journal_entry = frappe.get_doc(make_bank_entry("Employee Advance", advance.name)) - journal_entry.cheque_no = "123123" - journal_entry.cheque_date = nowdate() - journal_entry.save() +def manual_journal_entry_for_advance(advance) -> dict: + doc = frappe.get_doc("Employee Advance", advance.name) + payment_account = get_same_currency_bank_cash_account(doc.company, doc.currency, doc.mode_of_payment) + + je = frappe.new_doc("Journal Entry") + je.posting_date = nowdate() + je.voucher_type = "Bank Entry" + je.company = doc.company + je.remark = "Payment against Employee Advance: " + advance.name + "\n" + doc.purpose + je.multi_currency = 1 if doc.currency != erpnext.get_company_currency(doc.company) else 0 + + je.append( + "accounts", + { + "account": doc.advance_account, + "account_currency": doc.currency, + "debit_in_account_currency": flt(doc.advance_amount), + "reference_type": "Employee Advance", + "reference_name": doc.name, + "party_type": "Employee", + "cost_center": erpnext.get_default_cost_center(doc.company), + "party": doc.employee, + "is_advance": "Yes", + }, + ) - return journal_entry + je.append( + "accounts", + { + "account": payment_account.account or payment_account.name, + "cost_center": erpnext.get_default_cost_center(doc.company), + "credit_in_account_currency": flt(doc.advance_amount), + "account_currency": doc.currency, + "account_type": payment_account.account_type, + }, + ) + je.cheque_no = "123123" + je.cheque_date = nowdate() + je.save() + return je def make_payment_entry(advance, amount=None): diff --git a/hrms/hr/doctype/expense_claim/test_expense_claim.py b/hrms/hr/doctype/expense_claim/test_expense_claim.py index f8ad34e6bc..de0db922e1 100644 --- a/hrms/hr/doctype/expense_claim/test_expense_claim.py +++ b/hrms/hr/doctype/expense_claim/test_expense_claim.py @@ -193,7 +193,7 @@ def test_expense_claim_against_fully_paid_advances(self): from hrms.hr.doctype.employee_advance.test_employee_advance import ( get_advances_for_claim, make_employee_advance, - make_journal_entry_for_advance, + manual_journal_entry_for_advance, ) frappe.db.delete("Employee Advance") @@ -204,7 +204,7 @@ def test_expense_claim_against_fully_paid_advances(self): ) advance = make_employee_advance(claim.employee) - pe = make_journal_entry_for_advance(advance) + pe = manual_journal_entry_for_advance(advance) pe.submit() # claim for already paid out advances @@ -219,7 +219,7 @@ def test_advance_amount_allocation_against_claim_with_taxes(self): from hrms.hr.doctype.employee_advance.test_employee_advance import ( get_advances_for_claim, make_employee_advance, - make_journal_entry_for_advance, + manual_journal_entry_for_advance, ) frappe.db.delete("Employee Advance") @@ -238,7 +238,7 @@ def test_advance_amount_allocation_against_claim_with_taxes(self): claim.save() advance = make_employee_advance(claim.employee) - pe = make_journal_entry_for_advance(advance) + pe = manual_journal_entry_for_advance(advance) pe.submit() # claim for already paid out advances @@ -253,7 +253,7 @@ def test_expense_claim_partially_paid_via_advance(self): from hrms.hr.doctype.employee_advance.test_employee_advance import ( get_advances_for_claim, make_employee_advance, - make_journal_entry_for_advance, + manual_journal_entry_for_advance, ) frappe.db.delete("Employee Advance") @@ -265,7 +265,7 @@ def test_expense_claim_partially_paid_via_advance(self): # link advance for partial amount advance = make_employee_advance(claim.employee, {"advance_amount": 500}) - pe = make_journal_entry_for_advance(advance) + pe = manual_journal_entry_for_advance(advance) pe.submit() claim = get_advances_for_claim(claim, advance.name) @@ -287,7 +287,7 @@ def test_expense_claim_with_deducted_returned_advance(self): create_return_through_additional_salary, get_advances_for_claim, make_employee_advance, - make_journal_entry_for_advance, + manual_journal_entry_for_advance, ) from hrms.hr.doctype.expense_claim.expense_claim import get_allocation_amount from hrms.payroll.doctype.salary_component.test_salary_component import create_salary_component @@ -296,7 +296,7 @@ def test_expense_claim_with_deducted_returned_advance(self): # create employee and employee advance employee_name = make_employee("_T@employee.advance", "_Test Company") advance = make_employee_advance(employee_name, {"repay_unclaimed_amount_from_salary": 1}) - journal_entry = make_journal_entry_for_advance(advance) + journal_entry = manual_journal_entry_for_advance(advance) journal_entry.submit() advance.reload() diff --git a/hrms/payroll/doctype/payroll_entry/test_payroll_entry.py b/hrms/payroll/doctype/payroll_entry/test_payroll_entry.py index bff769f42f..cd78f4df9e 100644 --- a/hrms/payroll/doctype/payroll_entry/test_payroll_entry.py +++ b/hrms/payroll/doctype/payroll_entry/test_payroll_entry.py @@ -15,7 +15,7 @@ ) from hrms.hr.doctype.employee_advance.test_employee_advance import ( make_employee_advance, - make_journal_entry_for_advance, + make_payment_entry, ) from hrms.payroll.doctype.payroll_entry.payroll_entry import ( PayrollEntry, @@ -574,8 +574,7 @@ def test_advance_deduction_in_accrual_journal_entry(self): # create employee advance advance = make_employee_advance(employee, {"repay_unclaimed_amount_from_salary": 1}) - journal_entry = make_journal_entry_for_advance(advance) - journal_entry.submit() + make_payment_entry(advance) advance.reload() # return advance through additional salary (deduction) From 3dedd35f9a1e2baa8bf49ba1d2f4cb500d815e50 Mon Sep 17 00:00:00 2001 From: iamkhanraheel Date: Fri, 8 May 2026 18:24:03 +0530 Subject: [PATCH 06/11] test(expense_claim): remove make_bank_entry func from expense claim and update func in test to create JE --- .../hr/doctype/expense_claim/expense_claim.py | 44 ------------------ .../expense_claim/test_expense_claim.py | 46 +++++++++++++++++-- 2 files changed, 41 insertions(+), 49 deletions(-) diff --git a/hrms/hr/doctype/expense_claim/expense_claim.py b/hrms/hr/doctype/expense_claim/expense_claim.py index 0042905923..42a80538f0 100644 --- a/hrms/hr/doctype/expense_claim/expense_claim.py +++ b/hrms/hr/doctype/expense_claim/expense_claim.py @@ -643,50 +643,6 @@ def get_outstanding_amount_for_claim(claim): return flt(outstanding_amt, precision) -@frappe.whitelist() -def make_bank_entry(dt: str, dn: str) -> dict: - from erpnext.accounts.doctype.journal_entry.journal_entry import get_default_bank_cash_account - - expense_claim = frappe.get_doc(dt, dn) - default_bank_cash_account = get_default_bank_cash_account(expense_claim.company, "Bank") - if not default_bank_cash_account: - default_bank_cash_account = get_default_bank_cash_account(expense_claim.company, "Cash") - - payable_amount = get_outstanding_amount_for_claim(expense_claim) - - je = frappe.new_doc("Journal Entry") - je.voucher_type = "Bank Entry" - je.company = expense_claim.company - je.remark = "Payment against Expense Claim: " + dn - - je.append( - "accounts", - { - "account": expense_claim.payable_account, - "debit_in_account_currency": payable_amount, - "reference_type": "Expense Claim", - "party_type": "Employee", - "party": expense_claim.employee, - "cost_center": erpnext.get_default_cost_center(expense_claim.company), - "reference_name": expense_claim.name, - }, - ) - - je.append( - "accounts", - { - "account": default_bank_cash_account.account, - "credit_in_account_currency": payable_amount, - "balance": default_bank_cash_account.balance, - "account_currency": default_bank_cash_account.account_currency, - "cost_center": erpnext.get_default_cost_center(expense_claim.company), - "account_type": default_bank_cash_account.account_type, - }, - ) - - return je.as_dict() - - @frappe.whitelist() def get_expense_claim_account_and_cost_center(expense_claim_type: str, company: str) -> dict: data = get_expense_claim_account(expense_claim_type, company) diff --git a/hrms/hr/doctype/expense_claim/test_expense_claim.py b/hrms/hr/doctype/expense_claim/test_expense_claim.py index de0db922e1..929e15c824 100644 --- a/hrms/hr/doctype/expense_claim/test_expense_claim.py +++ b/hrms/hr/doctype/expense_claim/test_expense_claim.py @@ -4,6 +4,7 @@ import frappe from frappe.utils import flt, nowdate, random_string, today +import erpnext from erpnext import get_company_currency from erpnext.accounts.doctype.account.test_account import create_account from erpnext.accounts.doctype.payment_entry.test_payment_entry import get_payment_entry @@ -13,7 +14,6 @@ from hrms.hr.doctype.expense_claim.expense_claim import ( MismatchError, get_outstanding_amount_for_claim, - make_bank_entry, make_expense_claim_for_delivery_trip, ) from hrms.tests.utils import HRMSTestSuite @@ -135,7 +135,7 @@ def test_expense_claim_status_as_payment_allocation_using_pr(self): employee1 = entry.party if not entry.party_type: - entry.credit += 200 + entry.credit = flt(entry.credit) + 200 entry.credit_in_account_currency += 200 je.append( @@ -1056,15 +1056,51 @@ def make_claim_payment_entry(expense_claim, amount): def make_journal_entry(expense_claim, do_not_submit=False): - je_dict = make_bank_entry("Expense Claim", expense_claim.name) - je = frappe.get_doc(je_dict) + from erpnext.accounts.doctype.journal_entry.journal_entry import get_default_bank_cash_account + + expense_claim = frappe.get_doc("Expense Claim", expense_claim.name) + default_bank_cash_account = get_default_bank_cash_account(expense_claim.company, "Bank") + if not default_bank_cash_account: + default_bank_cash_account = get_default_bank_cash_account(expense_claim.company, "Cash") + + payable_amount = get_outstanding_amount_for_claim(expense_claim) + + je = frappe.new_doc("Journal Entry") + je.voucher_type = "Bank Entry" + je.company = expense_claim.company + je.remark = "Payment against Expense Claim: " + expense_claim.name + + je.append( + "accounts", + { + "account": expense_claim.payable_account, + "debit_in_account_currency": payable_amount, + "reference_type": "Expense Claim", + "party_type": "Employee", + "party": expense_claim.employee, + "cost_center": erpnext.get_default_cost_center(expense_claim.company), + "reference_name": expense_claim.name, + }, + ) + + je.append( + "accounts", + { + "account": default_bank_cash_account.account, + "credit_in_account_currency": payable_amount, + "balance": default_bank_cash_account.balance, + "account_currency": default_bank_cash_account.account_currency, + "cost_center": erpnext.get_default_cost_center(expense_claim.company), + "account_type": default_bank_cash_account.account_type, + }, + ) + je.posting_date = nowdate() je.cheque_no = random_string(5) je.cheque_date = nowdate() if not do_not_submit: je.submit() - return je From 3bd91dd8d7da4f6ad012edf6ccebf5810c4eb5a6 Mon Sep 17 00:00:00 2001 From: iamkhanraheel Date: Sat, 9 May 2026 03:20:53 +0530 Subject: [PATCH 07/11] test(expense_claim): use payment entry instead of journal & test manual journal payments flow --- .../expense_claim/test_expense_claim.py | 84 ++++++++++++++++--- 1 file changed, 71 insertions(+), 13 deletions(-) diff --git a/hrms/hr/doctype/expense_claim/test_expense_claim.py b/hrms/hr/doctype/expense_claim/test_expense_claim.py index 929e15c824..04517fe2a8 100644 --- a/hrms/hr/doctype/expense_claim/test_expense_claim.py +++ b/hrms/hr/doctype/expense_claim/test_expense_claim.py @@ -7,7 +7,6 @@ import erpnext from erpnext import get_company_currency from erpnext.accounts.doctype.account.test_account import create_account -from erpnext.accounts.doctype.payment_entry.test_payment_entry import get_payment_entry from erpnext.setup.doctype.employee.test_employee import make_employee from erpnext.setup.utils import get_exchange_rate @@ -193,7 +192,7 @@ def test_expense_claim_against_fully_paid_advances(self): from hrms.hr.doctype.employee_advance.test_employee_advance import ( get_advances_for_claim, make_employee_advance, - manual_journal_entry_for_advance, + make_payment_entry, ) frappe.db.delete("Employee Advance") @@ -204,8 +203,7 @@ def test_expense_claim_against_fully_paid_advances(self): ) advance = make_employee_advance(claim.employee) - pe = manual_journal_entry_for_advance(advance) - pe.submit() + pe = make_payment_entry(advance) # claim for already paid out advances claim = get_advances_for_claim(claim, advance.name) @@ -214,12 +212,19 @@ def test_expense_claim_against_fully_paid_advances(self): self.assertEqual(claim.grand_total, 0) self.assertEqual(claim.status, "Paid") + advance_row = claim.advances[0] + self.assertEqual(advance_row.employee_advance, advance.name) + self.assertEqual(advance_row.reference_type, "Payment Entry") + self.assertEqual(advance_row.reference_name, pe.name) + self.assertEqual(advance_row.advance_paid, 1000) + self.assertEqual(advance_row.unclaimed_amount, 1000) + self.assertEqual(advance_row.allocated_amount, 1000) def test_advance_amount_allocation_against_claim_with_taxes(self): from hrms.hr.doctype.employee_advance.test_employee_advance import ( get_advances_for_claim, make_employee_advance, - manual_journal_entry_for_advance, + make_payment_entry, ) frappe.db.delete("Employee Advance") @@ -238,8 +243,7 @@ def test_advance_amount_allocation_against_claim_with_taxes(self): claim.save() advance = make_employee_advance(claim.employee) - pe = manual_journal_entry_for_advance(advance) - pe.submit() + make_payment_entry(advance) # claim for already paid out advances claim = get_advances_for_claim(claim, advance.name, 763) @@ -253,7 +257,7 @@ def test_expense_claim_partially_paid_via_advance(self): from hrms.hr.doctype.employee_advance.test_employee_advance import ( get_advances_for_claim, make_employee_advance, - manual_journal_entry_for_advance, + make_payment_entry, ) frappe.db.delete("Employee Advance") @@ -265,8 +269,7 @@ def test_expense_claim_partially_paid_via_advance(self): # link advance for partial amount advance = make_employee_advance(claim.employee, {"advance_amount": 500}) - pe = manual_journal_entry_for_advance(advance) - pe.submit() + make_payment_entry(advance) claim = get_advances_for_claim(claim, advance.name) claim.save() @@ -281,13 +284,18 @@ def test_expense_claim_partially_paid_via_advance(self): self.assertEqual(claim.total_amount_reimbursed, 500) self.assertEqual(claim.status, "Paid") + self.assertEqual(claim.total_claimed_amount, 1000) + advance_row = claim.advances[0] + self.assertEqual(advance_row.advance_paid, 500) + self.assertEqual(advance_row.unclaimed_amount, 500) + self.assertEqual(advance_row.allocated_amount, 500) def test_expense_claim_with_deducted_returned_advance(self): from hrms.hr.doctype.employee_advance.test_employee_advance import ( create_return_through_additional_salary, get_advances_for_claim, make_employee_advance, - manual_journal_entry_for_advance, + make_payment_entry, ) from hrms.hr.doctype.expense_claim.expense_claim import get_allocation_amount from hrms.payroll.doctype.salary_component.test_salary_component import create_salary_component @@ -296,8 +304,7 @@ def test_expense_claim_with_deducted_returned_advance(self): # create employee and employee advance employee_name = make_employee("_T@employee.advance", "_Test Company") advance = make_employee_advance(employee_name, {"repay_unclaimed_amount_from_salary": 1}) - journal_entry = manual_journal_entry_for_advance(advance) - journal_entry.submit() + make_payment_entry(advance) advance.reload() # set up salary components and structure @@ -339,6 +346,10 @@ def test_expense_claim_with_deducted_returned_advance(self): ), 600, ) + advance_row = claim.advances[0] + self.assertEqual(advance_row.advance_paid, 1000) + self.assertEqual(advance_row.return_amount, 400) + self.assertEqual(advance_row.allocated_amount, 200) def test_expense_claim_gl_entry(self): payable_account = get_payable_account(company_name) @@ -953,6 +964,53 @@ def test_status_on_discard(self): expense_claim.reload() self.assertEqual(expense_claim.status, "Cancelled") + def test_expense_claim_advance_payment_via_journal_entrry(self): + from hrms.hr.doctype.employee_advance.test_employee_advance import ( + get_advances_for_claim, + make_employee_advance, + manual_journal_entry_for_advance, + ) + + payable_account = get_payable_account("_Test Company") + claim = make_expense_claim( + payable_account, + 1000, + 1000, + "_Test Company", + "Travel Expenses - _TC", + do_not_submit=True, + ) + + advance = make_employee_advance(claim.employee) + je = manual_journal_entry_for_advance(advance) + je.submit() + advance.reload() + self.assertEqual(advance.status, "Paid") + + claim = get_advances_for_claim(claim, advance.name) + claim.save().submit() + claim.reload() + advance.reload() + + self.assertEqual(claim.status, "Paid") + self.assertEqual(advance.status, "Claimed") + self.assertEqual(len(claim.advances), 1) + + advance_row = claim.advances[0] + self.assertEqual(advance_row.employee_advance, advance.name) + self.assertEqual(advance_row.reference_type, "Journal Entry") + self.assertEqual(advance_row.reference_name, je.name) + self.assertEqual(advance_row.advance_paid, 1000) + self.assertEqual(advance_row.unclaimed_amount, 1000) + self.assertEqual(advance_row.allocated_amount, 1000) + + claim.cancel() + claim.reload() + advance.reload() + + self.assertEqual(advance.status, "Paid") + self.assertEqual(advance.claimed_amount, 0) + def get_payable_account(company): return frappe.get_cached_value("Company", company, "default_payable_account") From 4af4ad75a7d771999932c83a02e898e715fb6ee2 Mon Sep 17 00:00:00 2001 From: iamkhanraheel Date: Sat, 9 May 2026 16:51:59 +0530 Subject: [PATCH 08/11] patch: set reference fields in expense claim advance --- hrms/patches.txt | 1 + ..._reference_fields_in_expense_claim_advance.py | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 hrms/patches/v16_0/set_reference_fields_in_expense_claim_advance.py diff --git a/hrms/patches.txt b/hrms/patches.txt index d60bd5b418..4ee444654e 100644 --- a/hrms/patches.txt +++ b/hrms/patches.txt @@ -46,3 +46,4 @@ hrms.patches.v16_0.set_base_paid_amount_in_employee_advance hrms.patches.v16_0.set_currency_and_base_fields_in_expense_claim hrms.patches.v16_0.remove_ess_user_type_limit hrms.patches.v16_0.add_default_hr_role_permissions +hrms.patches.v16_0.set_reference_fields_in_expense_claim_advance diff --git a/hrms/patches/v16_0/set_reference_fields_in_expense_claim_advance.py b/hrms/patches/v16_0/set_reference_fields_in_expense_claim_advance.py new file mode 100644 index 0000000000..d2578df811 --- /dev/null +++ b/hrms/patches/v16_0/set_reference_fields_in_expense_claim_advance.py @@ -0,0 +1,16 @@ +import frappe +from frappe.query_builder.functions import IfNull + + +def execute(): + ExpenseClaimAdvance = frappe.qb.DocType("Expense Claim Advance") + + ( + frappe.qb.update(ExpenseClaimAdvance) + .set(ExpenseClaimAdvance.reference_name, ExpenseClaimAdvance.payment_entry) + .set(ExpenseClaimAdvance.reference_type, "Payment Entry") + .where( + (IfNull(ExpenseClaimAdvance.payment_entry, "") != "") + & (IfNull(ExpenseClaimAdvance.reference_name, "") == "") + ) + ).run() From 64ff857aa3ae4edb2e8c03062796f8c058301ccd Mon Sep 17 00:00:00 2001 From: iamkhanraheel Date: Sat, 9 May 2026 17:04:13 +0530 Subject: [PATCH 09/11] fix(expense_claim): sort advance payment in claim based on creation --- hrms/hr/doctype/expense_claim/expense_claim.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/hrms/hr/doctype/expense_claim/expense_claim.py b/hrms/hr/doctype/expense_claim/expense_claim.py index 42a80538f0..f1af75701e 100644 --- a/hrms/hr/doctype/expense_claim/expense_claim.py +++ b/hrms/hr/doctype/expense_claim/expense_claim.py @@ -741,19 +741,13 @@ def get_expense_claim_advances(expense_claim, employee_advance): "event": "Submit", "delinked": False, }, - fields=[ - "voucher_type", - "voucher_no", - "amount", - "base_amount", - "exchange_rate", - ], + fields=["voucher_type", "voucher_no", "amount", "base_amount", "exchange_rate", "creation"], ) if not advance_payments: return - advance_payments.sort(key=lambda x: x.get("voucher_no")) + advance_payments.sort(key=lambda x: x.get("creation")) advance_payment_voucher_nos = [payment["voucher_no"] for payment in advance_payments] claimed_payments = frappe.get_all( From ee3c1abf1c4ff829af4eff1317e63f095e2a84d8 Mon Sep 17 00:00:00 2001 From: iamkhanraheel Date: Sat, 9 May 2026 17:08:13 +0530 Subject: [PATCH 10/11] test(expense_claim): typo --- hrms/hr/doctype/expense_claim/test_expense_claim.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hrms/hr/doctype/expense_claim/test_expense_claim.py b/hrms/hr/doctype/expense_claim/test_expense_claim.py index 04517fe2a8..2d30aee3d3 100644 --- a/hrms/hr/doctype/expense_claim/test_expense_claim.py +++ b/hrms/hr/doctype/expense_claim/test_expense_claim.py @@ -964,7 +964,7 @@ def test_status_on_discard(self): expense_claim.reload() self.assertEqual(expense_claim.status, "Cancelled") - def test_expense_claim_advance_payment_via_journal_entrry(self): + def test_expense_claim_advance_payment_via_journal_entry(self): from hrms.hr.doctype.employee_advance.test_employee_advance import ( get_advances_for_claim, make_employee_advance, From 816a45182641912e145b67828c15b7947db7cefe Mon Sep 17 00:00:00 2001 From: Krishna Pramod Shirsath <91021227+krishna-254@users.noreply.github.com> Date: Mon, 11 May 2026 16:04:00 +0530 Subject: [PATCH 11/11] refactor: optimize database queries using query builder in payroll reports (#4488) * refactor: optimize database queries using query builder in payroll reports * refactor(provident-fund-deductions-report): orderby syntax use frappe qb * refactor(professional-tax-deductions): remove debug print statements --- .../professional_tax_deductions.py | 54 +++++++---- .../provident_fund_deductions.py | 95 +++++++++++-------- .../salary_payments_based_on_payment_mode.py | 51 +++++----- 3 files changed, 116 insertions(+), 84 deletions(-) diff --git a/hrms/payroll/report/professional_tax_deductions/professional_tax_deductions.py b/hrms/payroll/report/professional_tax_deductions/professional_tax_deductions.py index 50a2f251c4..26f0c8b85e 100644 --- a/hrms/payroll/report/professional_tax_deductions/professional_tax_deductions.py +++ b/hrms/payroll/report/professional_tax_deductions/professional_tax_deductions.py @@ -4,6 +4,7 @@ import frappe from frappe import _ +from frappe.query_builder import DocType from hrms.payroll.report.provident_fund_deductions.provident_fund_deductions import get_conditions @@ -37,31 +38,42 @@ def get_columns(filters): def get_data(filters): data = [] - - component_type_dict = frappe._dict( - frappe.db.sql( - """ select name, component_type from `tabSalary Component` - where component_type = 'Professional Tax' """ - ) + SalarySlip = DocType("Salary Slip") + SalaryDetail = DocType("Salary Detail") + SalaryComponent = DocType("Salary Component") + + component_type_list = ( + frappe.qb.from_(SalaryComponent) + .select(SalaryComponent.name, SalaryComponent.component_type) + .where(SalaryComponent.component_type == "Professional Tax") + .run(pluck="name") ) - if not len(component_type_dict): + if not len(component_type_list): return [] - conditions = get_conditions(filters) - - # nosemgrep: frappe-semgrep-rules.rules.frappe-using-db-sql - entry = frappe.db.sql( - """SELECT sal.employee, sal.employee_name, ded.salary_component, ded.amount - FROM `tabSalary Slip` sal, `tabSalary Detail` ded - WHERE sal.name = ded.parent - AND ded.parentfield = 'deductions' - AND ded.parenttype = 'Salary Slip' - AND sal.docstatus = 1 {} - AND ded.salary_component IN ({}) - """.format(conditions, ", ".join(["%s"] * len(component_type_dict))), - tuple(component_type_dict.keys()), - as_dict=1, + filter_clauses = get_conditions(filters) + base_where = SalarySlip.docstatus == 1 + for clause in filter_clauses: + base_where = base_where & clause + + entry = ( + frappe.qb.from_(SalarySlip) + .join(SalaryDetail) + .on(SalarySlip.name == SalaryDetail.parent) + .select( + SalarySlip.employee, + SalarySlip.employee_name, + SalaryDetail.salary_component, + SalaryDetail.amount, + ) + .where( + (SalaryDetail.parentfield == "deductions") + & (SalaryDetail.parenttype == "Salary Slip") + & (SalaryDetail.salary_component.isin(component_type_list)) + & base_where + ) + .run(as_dict=True) ) for d in entry: diff --git a/hrms/payroll/report/provident_fund_deductions/provident_fund_deductions.py b/hrms/payroll/report/provident_fund_deductions/provident_fund_deductions.py index ba1d46899f..021ca57b0b 100644 --- a/hrms/payroll/report/provident_fund_deductions/provident_fund_deductions.py +++ b/hrms/payroll/report/provident_fund_deductions/provident_fund_deductions.py @@ -4,6 +4,8 @@ import frappe from frappe import _ +from frappe.query_builder import DocType +from frappe.query_builder.functions import Extract from frappe.utils import getdate @@ -56,34 +58,31 @@ def get_columns(filters): def get_conditions(filters): - conditions = [""] + SalarySlip = DocType("Salary Slip") + filter_clauses = [] if filters.get("department"): - conditions.append("sal.department = '%s' " % (filters["department"])) - + filter_clauses.append(SalarySlip.department == filters["department"]) if filters.get("branch"): - conditions.append("sal.branch = '%s' " % (filters["branch"])) - + filter_clauses.append(SalarySlip.branch == filters["branch"]) if filters.get("company"): - conditions.append("sal.company = '%s' " % (filters["company"])) - + filter_clauses.append(SalarySlip.company == filters["company"]) if filters.get("month"): - conditions.append("month(sal.start_date) = '%s' " % (filters["month"])) - + filter_clauses.append(Extract("month", SalarySlip.start_date) == int(filters["month"])) if filters.get("year"): - conditions.append("year(start_date) = '%s' " % (filters["year"])) - + filter_clauses.append(Extract("year", SalarySlip.start_date) == int(filters["year"])) if filters.get("mode_of_payment"): - conditions.append("sal.mode_of_payment = '%s' " % (filters["mode_of_payment"])) + filter_clauses.append(SalarySlip.mode_of_payment == filters["mode_of_payment"]) - return " and ".join(conditions) + return filter_clauses def prepare_data(entry, component_type_dict): data_list = {} + Employee = DocType("Employee") employee_account_dict = frappe._dict( - frappe.db.sql(""" select name, provident_fund_account from `tabEmployee`""") + frappe.qb.from_(Employee).select(Employee.name, Employee.provident_fund_account).run() ) for d in entry: @@ -107,39 +106,48 @@ def prepare_data(entry, component_type_dict): def get_data(filters): data = [] + SalarySlip = DocType("Salary Slip") + SalaryDetail = DocType("Salary Detail") + SalaryComponent = DocType("Salary Component") - conditions = get_conditions(filters) - - salary_slips = frappe.db.sql( - """ select sal.name from `tabSalary Slip` sal - where docstatus = 1 %s - """ - % (conditions), - as_dict=1, - ) + filter_clauses = get_conditions(filters) + component_types = ["Provident Fund", "Additional Provident Fund", "Provident Fund Loan"] component_type_dict = frappe._dict( - frappe.db.sql( - """ select name, component_type from `tabSalary Component` - where component_type in ('Provident Fund', 'Additional Provident Fund', 'Provident Fund Loan')""" - ) + frappe.qb.from_(SalaryComponent) + .select(SalaryComponent.name, SalaryComponent.component_type) + .where(SalaryComponent.component_type.isin(component_types)) + .run() ) if not len(component_type_dict): return [] - # nosemgrep: frappe-semgrep-rules.rules.frappe-using-db-sql - entry = frappe.db.sql( - """ select sal.name, sal.employee, sal.employee_name, ded.salary_component, ded.amount - from `tabSalary Slip` sal, `tabSalary Detail` ded - where sal.name = ded.parent - and ded.parentfield = 'deductions' - and ded.parenttype = 'Salary Slip' - and sal.docstatus = 1 {} - and ded.salary_component in ({}) - """.format(conditions, ", ".join(["%s"] * len(component_type_dict.keys()))), - tuple(component_type_dict.keys()), - as_dict=1, + base_where = SalarySlip.docstatus == 1 + for clause in filter_clauses: + base_where = base_where & clause + + salary_slips = frappe.qb.from_(SalarySlip).select(SalarySlip.name).where(base_where).run(as_dict=True) + + comp_names = list(component_type_dict.keys()) + entry = ( + frappe.qb.from_(SalarySlip) + .join(SalaryDetail) + .on(SalarySlip.name == SalaryDetail.parent) + .select( + SalarySlip.name, + SalarySlip.employee, + SalarySlip.employee_name, + SalaryDetail.salary_component, + SalaryDetail.amount, + ) + .where( + (SalaryDetail.parentfield == "deductions") + & (SalaryDetail.parenttype == "Salary Slip") + & (SalaryDetail.salary_component.isin(comp_names)) + & base_where + ) + .run(as_dict=True) ) data_list = prepare_data(entry, component_type_dict) @@ -174,8 +182,13 @@ def get_data(filters): @frappe.whitelist() def get_years() -> str: - year_list = frappe.db.sql_list( - """select distinct YEAR(end_date) from `tabSalary Slip` ORDER BY YEAR(end_date) DESC""" + SalarySlip = DocType("Salary Slip") + year_list = ( + frappe.qb.from_(SalarySlip) + .select(Extract("year", SalarySlip.end_date).as_("year")) + .distinct() + .orderby(Extract("year", SalarySlip.end_date), order=frappe.qb.desc) + .run(pluck=True) ) if not year_list: year_list = [getdate().year] diff --git a/hrms/payroll/report/salary_payments_based_on_payment_mode/salary_payments_based_on_payment_mode.py b/hrms/payroll/report/salary_payments_based_on_payment_mode/salary_payments_based_on_payment_mode.py index 04a7cf66f9..5821120d1b 100644 --- a/hrms/payroll/report/salary_payments_based_on_payment_mode/salary_payments_based_on_payment_mode.py +++ b/hrms/payroll/report/salary_payments_based_on_payment_mode/salary_payments_based_on_payment_mode.py @@ -4,6 +4,7 @@ import frappe from frappe import _ +from frappe.query_builder import DocType import erpnext @@ -43,10 +44,13 @@ def get_columns(filters, mode_of_payments): def get_payment_modes(): - mode_of_payments = frappe.db.sql_list( - """ - select distinct mode_of_payment from `tabSalary Slip` where docstatus = 1 - """ + SalarySlip = DocType("Salary Slip") + mode_of_payments = ( + frappe.qb.from_(SalarySlip) + .select(SalarySlip.mode_of_payment) + .where(SalarySlip.docstatus == 1) + .distinct() + .run(pluck=True) ) return mode_of_payments @@ -67,28 +71,31 @@ def prepare_data(entry): def get_data(filters, mode_of_payments): data = [] - - conditions = get_conditions(filters) - - entry = frappe.db.sql( - """ - select branch, mode_of_payment, sum(net_pay) as net_pay, sum(gross_pay) as gross_pay - from `tabSalary Slip` sal - where docstatus = 1 %s - group by branch, mode_of_payment - """ - % (conditions), - as_dict=1, + SalarySlip = DocType("Salary Slip") + + filter_clauses = get_conditions(filters) + + base_where = SalarySlip.docstatus == 1 + for clause in filter_clauses: + base_where = base_where & clause + + entry = ( + frappe.qb.from_(SalarySlip) + .select( + SalarySlip.branch, + SalarySlip.mode_of_payment, + frappe.query_builder.functions.Sum(SalarySlip.net_pay).as_("net_pay"), + frappe.query_builder.functions.Sum(SalarySlip.gross_pay).as_("gross_pay"), + ) + .where(base_where) + .groupby(SalarySlip.branch, SalarySlip.mode_of_payment) + .run(as_dict=True) ) branch_wise_entries, gross_pay = prepare_data(entry) - branches = frappe.db.sql_list( - """ - select distinct branch from `tabSalary Slip` sal - where docstatus = 1 %s - """ - % (conditions) + branches = ( + frappe.qb.from_(SalarySlip).select(SalarySlip.branch).where(base_where).distinct().run(pluck=True) ) total_row = {"total": 0, "branch": "Total"}