From 2c40bee93de0039e183cc294c11b3868413f2373 Mon Sep 17 00:00:00 2001 From: rethik Date: Mon, 6 Jan 2025 11:17:05 +0530 Subject: [PATCH 1/5] test: add unit test to validate company and department (cherry picked from commit 681ccbce64709fea6073e258a1b9c85ed3819012) --- hrms/hr/doctype/expense_claim/test_expense_claim.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/hrms/hr/doctype/expense_claim/test_expense_claim.py b/hrms/hr/doctype/expense_claim/test_expense_claim.py index 2d74922eae..8c12ad6afa 100644 --- a/hrms/hr/doctype/expense_claim/test_expense_claim.py +++ b/hrms/hr/doctype/expense_claim/test_expense_claim.py @@ -10,6 +10,7 @@ from erpnext.setup.doctype.employee.test_employee import make_employee from hrms.hr.doctype.expense_claim.expense_claim import ( + MismatchError, get_outstanding_amount_for_claim, make_bank_entry, make_expense_claim_for_delivery_trip, @@ -568,6 +569,13 @@ def test_repost(self): ) self.assertEqual(ledger_balance, expected_data) + def test_company_department_validation(self): + # validate company and department + expense_claim = frappe.new_doc("Expense Claim") + expense_claim.company = "_Test Company 3" + expense_claim.department = "Accounts - _TC2" + self.assertRaises(MismatchError, expense_claim.save) + def get_payable_account(company): return frappe.get_cached_value("Company", company, "default_payable_account") From 09d21d284e4f31e16633ff7e9093ba67166db3fb Mon Sep 17 00:00:00 2001 From: rethik Date: Mon, 6 Jan 2025 11:18:02 +0530 Subject: [PATCH 2/5] feat: add company filter for department (cherry picked from commit f3f45c49fb49052dc50fc737b4b81b15711e5286) --- hrms/hr/doctype/expense_claim/expense_claim.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/hrms/hr/doctype/expense_claim/expense_claim.js b/hrms/hr/doctype/expense_claim/expense_claim.js index 0f826807bd..32c02611f2 100644 --- a/hrms/hr/doctype/expense_claim/expense_claim.js +++ b/hrms/hr/doctype/expense_claim/expense_claim.js @@ -64,6 +64,14 @@ frappe.ui.form.on("Expense Claim", { query: "erpnext.controllers.queries.employee_query", }; }); + + frm.set_query("department", function () { + return { + filters: { + company: frm.doc.company, + }, + }; + }); }, onload: function (frm) { From bc2fdbde62b64f6ab94a1bf1cef8b9bcbd93953b Mon Sep 17 00:00:00 2001 From: rethik Date: Mon, 6 Jan 2025 11:19:16 +0530 Subject: [PATCH 3/5] fix: validate department based on company (cherry picked from commit 6033d8bfbccce44d14e31ecc08c9df323f531082) --- hrms/hr/doctype/expense_claim/expense_claim.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/hrms/hr/doctype/expense_claim/expense_claim.py b/hrms/hr/doctype/expense_claim/expense_claim.py index 04a02ba5d1..455410d445 100644 --- a/hrms/hr/doctype/expense_claim/expense_claim.py +++ b/hrms/hr/doctype/expense_claim/expense_claim.py @@ -29,6 +29,10 @@ class ExpenseApproverIdentityError(frappe.ValidationError): pass +class MismatchError(frappe.ValidationError): + pass + + class ExpenseClaim(AccountsController, PWANotificationsMixin): def onload(self): self.get("__onload").make_payment_via_journal_entry = frappe.db.get_single_value( @@ -47,6 +51,7 @@ def validate(self): self.set_expense_account(validate=True) self.calculate_taxes() self.set_status() + self.validate_company_and_department() if self.task and not self.project: self.project = frappe.db.get_value("Task", self.task, "project") @@ -83,6 +88,14 @@ def set_status(self, update=False): else: self.status = status + def validate_company_and_department(self): + if self.department: + if self.company != frappe.db.get_value("Department", self.department, "company"): + frappe.throw( + _("Department {0} does not belong to company: {1}").format(self.department, self.company), + exc=MismatchError, + ) + def on_update(self): share_doc_with_approver(self, self.expense_approver) self.publish_update() From a7202409d8d9cab2e030ce318c9c76de7763e2f7 Mon Sep 17 00:00:00 2001 From: rethik Date: Wed, 8 Jan 2025 19:53:33 +0530 Subject: [PATCH 4/5] refactor: combine nested if (cherry picked from commit ad09502f6ccddd0ed7393a11afdff5fa1d16eacb) --- hrms/hr/doctype/expense_claim/expense_claim.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/hrms/hr/doctype/expense_claim/expense_claim.py b/hrms/hr/doctype/expense_claim/expense_claim.py index 455410d445..e01bdcd6a8 100644 --- a/hrms/hr/doctype/expense_claim/expense_claim.py +++ b/hrms/hr/doctype/expense_claim/expense_claim.py @@ -89,12 +89,11 @@ def set_status(self, update=False): self.status = status def validate_company_and_department(self): - if self.department: - if self.company != frappe.db.get_value("Department", self.department, "company"): - frappe.throw( - _("Department {0} does not belong to company: {1}").format(self.department, self.company), - exc=MismatchError, - ) + if self.department and self.company != frappe.db.get_value("Department", self.department, "company"): + frappe.throw( + _("Department {0} does not belong to company: {1}").format(self.department, self.company), + exc=MismatchError, + ) def on_update(self): share_doc_with_approver(self, self.expense_approver) From 99065c5dd2d8386d84073a78ab2562741c83b816 Mon Sep 17 00:00:00 2001 From: rethik Date: Fri, 10 Jan 2025 15:53:51 +0530 Subject: [PATCH 5/5] fix: validate if company is set in department (cherry picked from commit e86ef6db04394f2015a4fd68bfecefcab3dc8677) --- hrms/hr/doctype/expense_claim/expense_claim.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/hrms/hr/doctype/expense_claim/expense_claim.py b/hrms/hr/doctype/expense_claim/expense_claim.py index e01bdcd6a8..0717eed569 100644 --- a/hrms/hr/doctype/expense_claim/expense_claim.py +++ b/hrms/hr/doctype/expense_claim/expense_claim.py @@ -89,11 +89,13 @@ def set_status(self, update=False): self.status = status def validate_company_and_department(self): - if self.department and self.company != frappe.db.get_value("Department", self.department, "company"): - frappe.throw( - _("Department {0} does not belong to company: {1}").format(self.department, self.company), - exc=MismatchError, - ) + if self.department: + company = frappe.db.get_value("Department", self.department, "company") + if company and self.company != company: + frappe.throw( + _("Department {0} does not belong to company: {1}").format(self.department, self.company), + exc=MismatchError, + ) def on_update(self): share_doc_with_approver(self, self.expense_approver)