From e04b3aaf7a2cb0792436a7e5f868572cb35c39f4 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Tue, 18 Jan 2022 14:36:22 +0530 Subject: [PATCH 01/16] feat(Employee Advance): add 'Returned' and 'Partly Claimed and Returned' status --- .../employee_advance/employee_advance.json | 42 +++++++++++++++++-- .../employee_advance/employee_advance.py | 39 ++++++++++------- .../hr/doctype/expense_claim/expense_claim.js | 2 +- .../hr/doctype/expense_claim/expense_claim.py | 31 +++++++++----- 4 files changed, 85 insertions(+), 29 deletions(-) diff --git a/erpnext/hr/doctype/employee_advance/employee_advance.json b/erpnext/hr/doctype/employee_advance/employee_advance.json index 04754530c3..b0501830cc 100644 --- a/erpnext/hr/doctype/employee_advance/employee_advance.json +++ b/erpnext/hr/doctype/employee_advance/employee_advance.json @@ -2,7 +2,7 @@ "actions": [], "allow_import": 1, "autoname": "naming_series:", - "creation": "2017-10-09 14:26:29.612365", + "creation": "2022-01-17 18:36:51.450395", "doctype": "DocType", "editable_grid": 1, "engine": "InnoDB", @@ -121,7 +121,7 @@ "fieldtype": "Select", "label": "Status", "no_copy": 1, - "options": "Draft\nPaid\nUnpaid\nClaimed\nCancelled", + "options": "Draft\nPaid\nUnpaid\nClaimed\nReturned\nPartly Claimed and Returned\nCancelled", "read_only": 1 }, { @@ -200,7 +200,7 @@ ], "is_submittable": 1, "links": [], - "modified": "2021-09-11 18:38:38.617478", + "modified": "2022-01-17 19:33:52.345823", "modified_by": "Administrator", "module": "HR", "name": "Employee Advance", @@ -237,5 +237,41 @@ "search_fields": "employee,employee_name", "sort_field": "modified", "sort_order": "DESC", + "states": [ + { + "color": "Red", + "custom": 1, + "title": "Draft" + }, + { + "color": "Green", + "custom": 1, + "title": "Paid" + }, + { + "color": "Orange", + "custom": 1, + "title": "Unpaid" + }, + { + "color": "Blue", + "custom": 1, + "title": "Claimed" + }, + { + "color": "Gray", + "title": "Returned" + }, + { + "color": "Yellow", + "title": "Partly Claimed and Returned" + }, + { + "color": "Red", + "custom": 1, + "title": "Cancelled" + } + ], + "title_field": "employee_name", "track_changes": 1 } \ No newline at end of file diff --git a/erpnext/hr/doctype/employee_advance/employee_advance.py b/erpnext/hr/doctype/employee_advance/employee_advance.py index 7aac2b63ed..e17eb214a1 100644 --- a/erpnext/hr/doctype/employee_advance/employee_advance.py +++ b/erpnext/hr/doctype/employee_advance/employee_advance.py @@ -28,18 +28,31 @@ class EmployeeAdvance(Document): def on_cancel(self): self.ignore_linked_doctypes = ('GL Entry') - def set_status(self): + def set_status(self, update=False): + precision = self.precision("paid_amount") + total_amount = flt(flt(self.claimed_amount) + flt(self.return_amount), precision) + status = None + if self.docstatus == 0: - self.status = "Draft" - if self.docstatus == 1: - if self.claimed_amount and flt(self.claimed_amount) == flt(self.paid_amount): - self.status = "Claimed" - elif self.paid_amount and self.advance_amount == flt(self.paid_amount): - self.status = "Paid" + status = "Draft" + elif self.docstatus == 1: + if flt(self.claimed_amount) > 0 and flt(self.claimed_amount, precision) == flt(self.paid_amount, precision): + status = "Claimed" + elif flt(self.return_amount) > 0 and flt(self.return_amount, precision) == flt(self.paid_amount, precision): + status = "Returned" + elif flt(self.claimed_amount) > 0 and (flt(self.return_amount) > 0) and total_amount == flt(self.paid_amount, precision): + status = "Partly Claimed and Returned" + elif flt(self.paid_amount) > 0 and flt(self.advance_amount, precision) == flt(self.paid_amount, precision): + status = "Paid" else: - self.status = "Unpaid" + status = "Unpaid" elif self.docstatus == 2: - self.status = "Cancelled" + status = "Cancelled" + + if update: + self.db_set("status", status) + else: + self.status = status def set_total_advance_paid(self): gle = frappe.qb.DocType("GL Entry") @@ -85,9 +98,7 @@ class EmployeeAdvance(Document): self.db_set("paid_amount", paid_amount) self.db_set("return_amount", return_amount) - self.set_status() - frappe.db.set_value("Employee Advance", self.name , "status", self.status) - + self.set_status(update=True) def update_claimed_amount(self): claimed_amount = frappe.db.sql(""" @@ -103,8 +114,8 @@ class EmployeeAdvance(Document): frappe.db.set_value("Employee Advance", self.name, "claimed_amount", flt(claimed_amount)) self.reload() - self.set_status() - frappe.db.set_value("Employee Advance", self.name, "status", self.status) + self.set_status(update=True) + @frappe.whitelist() def get_pending_amount(employee, posting_date): diff --git a/erpnext/hr/doctype/expense_claim/expense_claim.js b/erpnext/hr/doctype/expense_claim/expense_claim.js index 047945787d..af80b63845 100644 --- a/erpnext/hr/doctype/expense_claim/expense_claim.js +++ b/erpnext/hr/doctype/expense_claim/expense_claim.js @@ -171,7 +171,7 @@ frappe.ui.form.on("Expense Claim", { ['docstatus', '=', 1], ['employee', '=', frm.doc.employee], ['paid_amount', '>', 0], - ['status', '!=', 'Claimed'] + ['status', 'not in', ['Claimed', 'Returned', 'Partly Claimed and Returned']] ] }; }); diff --git a/erpnext/hr/doctype/expense_claim/expense_claim.py b/erpnext/hr/doctype/expense_claim/expense_claim.py index 7e3898b7d5..2d2bb093ce 100644 --- a/erpnext/hr/doctype/expense_claim/expense_claim.py +++ b/erpnext/hr/doctype/expense_claim/expense_claim.py @@ -341,18 +341,27 @@ def get_expense_claim_account(expense_claim_type, company): @frappe.whitelist() def get_advances(employee, advance_id=None): - if not advance_id: - condition = 'docstatus=1 and employee={0} and paid_amount > 0 and paid_amount > claimed_amount + return_amount'.format(frappe.db.escape(employee)) - else: - condition = 'name={0}'.format(frappe.db.escape(advance_id)) + advance = frappe.qb.DocType("Employee Advance") - return frappe.db.sql(""" - select - name, posting_date, paid_amount, claimed_amount, advance_account - from - `tabEmployee Advance` - where {0} - """.format(condition), as_dict=1) + query = ( + frappe.qb.from_(advance) + .select( + advance.name, advance.posting_date, advance.paid_amount, + advance.claimed_amount, advance.advance_account + ) + ) + + if not advance_id: + query = query.where( + (advance.docstatus == 1) + & (advance.employee == employee) + & (advance.paid_amount > 0) + & (advance.status.notin(["Claimed", "Returned", "Partly Claimed and Returned"])) + ) + else: + query = query.where(advance.name == advance_id) + + return query.run(as_dict=True) @frappe.whitelist() From bf30932de0524c42ab3d9718e3d19a73e1cb2d39 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Tue, 18 Jan 2022 14:36:38 +0530 Subject: [PATCH 02/16] patch: Employee Advance return statuses --- erpnext/patches.txt | 3 ++- .../v14_0/update_employee_advance_status.py | 26 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 erpnext/patches/v14_0/update_employee_advance_status.py diff --git a/erpnext/patches.txt b/erpnext/patches.txt index fa62b7fc27..ed39c204f6 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -326,4 +326,5 @@ erpnext.patches.v13_0.agriculture_deprecation_warning erpnext.patches.v14_0.delete_agriculture_doctypes erpnext.patches.v13_0.update_exchange_rate_settings erpnext.patches.v14_0.rearrange_company_fields -erpnext.patches.v14_0.update_leave_notification_template \ No newline at end of file +erpnext.patches.v14_0.update_leave_notification_template +erpnext.patches.v14_0.update_employee_advance_status \ No newline at end of file diff --git a/erpnext/patches/v14_0/update_employee_advance_status.py b/erpnext/patches/v14_0/update_employee_advance_status.py new file mode 100644 index 0000000000..a20e35a9f6 --- /dev/null +++ b/erpnext/patches/v14_0/update_employee_advance_status.py @@ -0,0 +1,26 @@ +import frappe + + +def execute(): + frappe.reload_doc('hr', 'doctype', 'employee_advance') + + advance = frappe.qb.DocType('Employee Advance') + (frappe.qb + .update(advance) + .set(advance.status, 'Returned') + .where( + (advance.docstatus == 1) + & ((advance.return_amount) & (advance.paid_amount == advance.return_amount)) + & (advance.status == 'Paid') + ) + ).run() + + (frappe.qb + .update(advance) + .set(advance.status, 'Partly Claimed and Returned') + .where( + (advance.docstatus == 1) + & ((advance.claimed_amount & advance.return_amount) & (advance.paid_amount == (advance.return_amount + advance.claimed_amount))) + & (advance.status == 'Paid') + ) + ).run() \ No newline at end of file From 0843d4388569616803a562a6928d6f1204f0e733 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Tue, 18 Jan 2022 14:37:45 +0530 Subject: [PATCH 03/16] fix(Expense Claim): validate advances after setting totals --- erpnext/hr/doctype/expense_claim/expense_claim.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/hr/doctype/expense_claim/expense_claim.py b/erpnext/hr/doctype/expense_claim/expense_claim.py index 2d2bb093ce..5146a5be90 100644 --- a/erpnext/hr/doctype/expense_claim/expense_claim.py +++ b/erpnext/hr/doctype/expense_claim/expense_claim.py @@ -23,10 +23,10 @@ class ExpenseClaim(AccountsController): def validate(self): validate_active_employee(self.employee) - self.validate_advances() + set_employee_name(self) self.validate_sanctioned_amount() self.calculate_total_amount() - set_employee_name(self) + self.validate_advances() self.set_expense_account(validate=True) self.set_payable_account() self.set_cost_center() From 85be0d22d4300a40f46b7268f7c56d8e7facbd40 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Tue, 18 Jan 2022 14:38:16 +0530 Subject: [PATCH 04/16] fix: employee advance status update on return via additional salary --- erpnext/payroll/doctype/additional_salary/additional_salary.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/erpnext/payroll/doctype/additional_salary/additional_salary.py b/erpnext/payroll/doctype/additional_salary/additional_salary.py index bf8bd05fcc..d618568416 100644 --- a/erpnext/payroll/doctype/additional_salary/additional_salary.py +++ b/erpnext/payroll/doctype/additional_salary/additional_salary.py @@ -105,6 +105,8 @@ class AdditionalSalary(Document): return_amount += self.amount frappe.db.set_value("Employee Advance", self.ref_docname, "return_amount", return_amount) + advance = frappe.get_doc("Employee Advance", self.ref_docname) + advance.set_status(update=True) def update_employee_referral(self, cancel=False): if self.ref_doctype == "Employee Referral": From 17b1f5f256ff63d34b3c20b4792cd77ae75402e0 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Tue, 18 Jan 2022 18:35:25 +0530 Subject: [PATCH 05/16] test: employee advance status --- .../employee_advance/employee_advance.py | 6 +- .../employee_advance/test_employee_advance.py | 97 ++++++++++++++++++- 2 files changed, 100 insertions(+), 3 deletions(-) diff --git a/erpnext/hr/doctype/employee_advance/employee_advance.py b/erpnext/hr/doctype/employee_advance/employee_advance.py index e17eb214a1..f63bb86129 100644 --- a/erpnext/hr/doctype/employee_advance/employee_advance.py +++ b/erpnext/hr/doctype/employee_advance/employee_advance.py @@ -233,7 +233,8 @@ def make_return_entry(employee, company, employee_advance_name, return_amount, 'reference_name': employee_advance_name, 'party_type': 'Employee', 'party': employee, - 'is_advance': 'Yes' + 'is_advance': 'Yes', + 'cost_center': erpnext.get_default_cost_center(company) }) bank_amount = flt(return_amount) if bank_cash_account.account_currency==currency \ @@ -244,7 +245,8 @@ def make_return_entry(employee, company, employee_advance_name, return_amount, "debit_in_account_currency": bank_amount, "account_currency": bank_cash_account.account_currency, "account_type": bank_cash_account.account_type, - "exchange_rate": flt(exchange_rate) if bank_cash_account.account_currency == currency else 1 + "exchange_rate": flt(exchange_rate) if bank_cash_account.account_currency == currency else 1, + "cost_center": erpnext.get_default_cost_center(company) }) return je.as_dict() diff --git a/erpnext/hr/doctype/employee_advance/test_employee_advance.py b/erpnext/hr/doctype/employee_advance/test_employee_advance.py index 5f2e720eb4..5f3a66a04f 100644 --- a/erpnext/hr/doctype/employee_advance/test_employee_advance.py +++ b/erpnext/hr/doctype/employee_advance/test_employee_advance.py @@ -4,7 +4,7 @@ import unittest import frappe -from frappe.utils import nowdate +from frappe.utils import flt, nowdate import erpnext from erpnext.hr.doctype.employee.test_employee import make_employee @@ -12,6 +12,12 @@ from erpnext.hr.doctype.employee_advance.employee_advance import ( EmployeeAdvanceOverPayment, create_return_through_additional_salary, make_bank_entry, + make_return_entry, +) +from erpnext.hr.doctype.expense_claim.expense_claim import get_advances +from erpnext.hr.doctype.expense_claim.test_expense_claim import ( + get_payable_account, + make_expense_claim, ) from erpnext.payroll.doctype.salary_component.test_salary_component import create_salary_component from erpnext.payroll.doctype.salary_structure.test_salary_structure import make_salary_structure @@ -52,9 +58,75 @@ class TestEmployeeAdvance(unittest.TestCase): self.assertEqual(advance.paid_amount, 0) self.assertEqual(advance.status, "Unpaid") + def test_claimed_and_returned_status(self): + # Claimed Status check, full amount claimed + 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) + pe = make_payment_entry(advance) + pe.submit() + + claim = get_advances_for_claim(claim, advance.name) + claim.save() + claim.submit() + + advance.reload() + self.assertEqual(advance.claimed_amount, 1000) + self.assertEqual(advance.status, "Claimed") + + # cancel claim; status should be Paid + claim.cancel() + advance.reload() + self.assertEqual(advance.claimed_amount, 0) + self.assertEqual(advance.status, "Paid") + + # Partly Claimed and Returned status check + # 500 Claimed, 500 Returned + claim = make_expense_claim(payable_account, 500, 500, "_Test Company", "Travel Expenses - _TC", do_not_submit=True) + + advance = make_employee_advance(claim.employee) + pe = make_payment_entry(advance) + pe.submit() + + claim = get_advances_for_claim(claim, advance.name, amount=500) + claim.save() + claim.submit() + + advance.reload() + self.assertEqual(advance.claimed_amount, 500) + self.assertEqual(advance.status, "Paid") + + entry = make_return_entry( + employee=advance.employee, + company=advance.company, + employee_advance_name=advance.name, + return_amount=flt(advance.paid_amount - advance.claimed_amount), + advance_account=advance.advance_account, + mode_of_payment=advance.mode_of_payment, + currency=advance.currency, + exchange_rate=advance.exchange_rate + ) + + entry = frappe.get_doc(entry) + entry.insert() + entry.submit() + + advance.reload() + self.assertEqual(advance.return_amount, 500) + self.assertEqual(advance.status, "Partly Claimed and Returned") + + # Cancel return entry; status should change to Paid + entry.cancel() + advance.reload() + self.assertEqual(advance.return_amount, 0) + self.assertEqual(advance.status, "Paid") + def test_repay_unclaimed_amount_from_salary(self): employee_name = make_employee("_T@employe.advance") advance = make_employee_advance(employee_name, {"repay_unclaimed_amount_from_salary": 1}) + pe = make_payment_entry(advance) + pe.submit() args = {"type": "Deduction"} create_salary_component("Advance Salary - Deduction", **args) @@ -82,11 +154,13 @@ class TestEmployeeAdvance(unittest.TestCase): advance.reload() self.assertEqual(advance.return_amount, 1000) + self.assertEqual(advance.status, "Returned") # update advance return amount on additional salary cancellation additional_salary.cancel() advance.reload() self.assertEqual(advance.return_amount, 700) + self.assertEqual(advance.status, "Paid") def tearDown(self): frappe.db.rollback() @@ -118,3 +192,24 @@ def make_employee_advance(employee_name, args=None): doc.submit() return doc + + +def get_advances_for_claim(claim, advance_name, amount=None): + advances = get_advances(claim.employee, advance_name) + + for entry in advances: + if amount: + allocated_amount = amount + else: + allocated_amount = flt(entry.paid_amount) - flt(entry.claimed_amount) + + claim.append("advances", { + "employee_advance": entry.name, + "posting_date": entry.posting_date, + "advance_account": entry.advance_account, + "advance_paid": entry.paid_amount, + "unclaimed_amount": allocated_amount, + "allocated_amount": allocated_amount + }) + + return claim \ No newline at end of file From 6d72aa377f98f07f28312074b453a97e482998e8 Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 24 Feb 2022 15:20:18 +0530 Subject: [PATCH 06/16] feat: Combine Sub-assembly items in Production Plan - Combine subassembly rows if same Item, Warehouse, Inhouse/Outhouse Manu.g, BOM No. --- erpnext/manufacturing/doctype/bom/bom.py | 2 +- .../production_plan/production_plan.js | 9 +++- .../production_plan/production_plan.json | 11 ++++- .../production_plan/production_plan.py | 49 ++++++++++++++++--- 4 files changed, 61 insertions(+), 10 deletions(-) diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index d640f3fda7..37d2b9ff97 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -918,7 +918,7 @@ def validate_bom_no(item, bom_no): frappe.throw(_("BOM {0} does not belong to Item {1}").format(bom_no, item)) @frappe.whitelist() -def get_children(doctype, parent=None, is_root=False, **filters): +def get_children(parent=None, is_root=False, **filters): if not parent or parent=="BOM": frappe.msgprint(_('Please select a BOM')) return diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.js b/erpnext/manufacturing/doctype/production_plan/production_plan.js index 0babf875e7..c78ff51a7c 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.js +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.js @@ -232,7 +232,7 @@ frappe.ui.form.on('Production Plan', { }); }, combine_items: function (frm) { - frm.clear_table('prod_plan_references'); + frm.clear_table("prod_plan_references"); frappe.call({ method: "get_items", @@ -247,6 +247,13 @@ frappe.ui.form.on('Production Plan', { }); }, + combine_sub_items: (frm) => { + if (frm.doc.sub_assembly_items.length > 0) { + frm.clear_table("sub_assembly_items"); + frm.trigger("get_sub_assembly_items"); + } + }, + get_sub_assembly_items: function(frm) { frm.dirty(); diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.json b/erpnext/manufacturing/doctype/production_plan/production_plan.json index 56cf2b4f08..3bfb764ba5 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.json +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.json @@ -36,6 +36,7 @@ "prod_plan_references", "section_break_24", "get_sub_assembly_items", + "combine_sub_items", "sub_assembly_items", "material_request_planning", "include_non_stock_items", @@ -340,7 +341,6 @@ { "fieldname": "prod_plan_references", "fieldtype": "Table", - "hidden": 1, "label": "Production Plan Item Reference", "options": "Production Plan Item Reference" }, @@ -370,16 +370,23 @@ "fieldname": "to_delivery_date", "fieldtype": "Date", "label": "To Delivery Date" + }, + { + "default": "0", + "fieldname": "combine_sub_items", + "fieldtype": "Check", + "label": "Consolidate Sub Assembly Items" } ], "icon": "fa fa-calendar", "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], - "modified": "2021-09-06 18:35:59.642232", + "modified": "2022-02-23 17:16:10.629378", "modified_by": "Administrator", "module": "Manufacturing", "name": "Production Plan", + "naming_rule": "By \"Naming Series\" field", "owner": "Administrator", "permissions": [ { diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.py b/erpnext/manufacturing/doctype/production_plan/production_plan.py index 80003dab78..48cd753d75 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.py @@ -21,7 +21,8 @@ from frappe.utils import ( ) from frappe.utils.csvutils import build_csv_response -from erpnext.manufacturing.doctype.bom.bom import get_children, validate_bom_no +from erpnext.manufacturing.doctype.bom.bom import get_children as get_bom_children +from erpnext.manufacturing.doctype.bom.bom import validate_bom_no from erpnext.manufacturing.doctype.work_order.work_order import get_item_details from erpnext.setup.doctype.item_group.item_group import get_item_group_defaults @@ -570,17 +571,28 @@ class ProductionPlan(Document): @frappe.whitelist() def get_sub_assembly_items(self, manufacturing_type=None): + "Fetch sub assembly items and optionally combine them." self.sub_assembly_items = [] + sub_assembly_items_store = [] # temporary store to process all subassembly items + for row in self.po_items: bom_data = [] get_sub_assembly_items(row.bom_no, bom_data, row.planned_qty) self.set_sub_assembly_items_based_on_level(row, bom_data, manufacturing_type) + sub_assembly_items_store.extend(bom_data) - self.sub_assembly_items.sort(key= lambda d: d.bom_level, reverse=True) - for idx, row in enumerate(self.sub_assembly_items, start=1): - row.idx = idx + if self.combine_sub_items: + # Combine subassembly items + sub_assembly_items_store = self.combine_subassembly_items(sub_assembly_items_store) + + sub_assembly_items_store.sort(key= lambda d: d.bom_level, reverse=True) # sort by bom level + + for idx, row in enumerate(sub_assembly_items_store): + row.idx = idx + 1 + self.append("sub_assembly_items", row) def set_sub_assembly_items_based_on_level(self, row, bom_data, manufacturing_type=None): + "Modify bom_data, set additional details." for data in bom_data: data.qty = data.stock_qty data.production_plan_item = row.name @@ -589,7 +601,32 @@ class ProductionPlan(Document): data.type_of_manufacturing = manufacturing_type or ("Subcontract" if data.is_sub_contracted_item else "In House") - self.append("sub_assembly_items", data) + def combine_subassembly_items(self, sub_assembly_items_store): + "Aggregate if same: Item, Warehouse, Inhouse/Outhouse Manu.g, BOM No." + key_wise_data = {} + for row in sub_assembly_items_store: + key = ( + row.get("production_item"), row.get("fg_warehouse"), + row.get("bom_no"), row.get("type_of_manufacturing") + ) + if key not in key_wise_data: + # intialise (item, wh, bom no, man.g type) wise dict + key_wise_data[key] = row + continue + + existing_row = key_wise_data[key] + if existing_row: + # if row with same (item, wh, bom no, man.g type) key, merge + existing_row.qty += flt(row.qty) + existing_row.stock_qty += flt(row.stock_qty) + existing_row.bom_level = max(existing_row.bom_level, row.bom_level) + continue + else: + # add row with key + key_wise_data[key] = row + + sub_assembly_items_store = [key_wise_data[key] for key in key_wise_data] # unpack into single level list + return sub_assembly_items_store def all_items_completed(self): all_items_produced = all(flt(d.planned_qty) - flt(d.produced_qty) < 0.000001 @@ -1031,7 +1068,7 @@ def get_item_data(item_code): } def get_sub_assembly_items(bom_no, bom_data, to_produce_qty, indent=0): - data = get_children('BOM', parent = bom_no) + data = get_bom_children(parent=bom_no) for d in data: if d.expandable: parent_item_code = frappe.get_cached_value("BOM", bom_no, "item") From 292fe370dbb1d4c090c78f34bd70ddf95774fe5d Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 28 Feb 2022 16:35:20 +0530 Subject: [PATCH 07/16] test: combine sub assembly items and make Production Plan tests atomic --- .../production_plan/test_production_plan.py | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py index 2359815813..8e34e1bca3 100644 --- a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py @@ -20,7 +20,7 @@ from erpnext.tests.utils import ERPNextTestCase class TestProductionPlan(ERPNextTestCase): - def setUp(self): + def setUp(self) -> None: for item in ['Test Production Item 1', 'Subassembly Item 1', 'Raw Material Item 1', 'Raw Material Item 2']: create_item(item, valuation_rate=100) @@ -38,6 +38,9 @@ class TestProductionPlan(ERPNextTestCase): if not frappe.db.get_value('BOM', {'item': item}): make_bom(item = item, raw_materials = raw_materials) + def tearDown(self) -> None: + frappe.db.rollback() + def test_production_plan_mr_creation(self): "Test if MRs are created for unavailable raw materials." pln = create_production_plan(item_code='Test Production Item 1') @@ -258,6 +261,46 @@ class TestProductionPlan(ERPNextTestCase): pln.reload() pln.cancel() + def test_production_plan_combine_subassembly(self): + """ + Test combining Sub assembly items belonging to the same BOM in Prod Plan. + 1) Red-Car -> Wheel (sub assembly) > BOM-WHEEL-001 + 2) Green-Car -> Wheel (sub assembly) > BOM-WHEEL-001 + """ + from erpnext.manufacturing.doctype.bom.test_bom import create_nested_bom + + bom_tree_1 = { + "Red-Car": {"Wheel": {"Rubber": {}}} + } + bom_tree_2 = { + "Green-Car": {"Wheel": {"Rubber": {}}} + } + + parent_bom_1 = create_nested_bom(bom_tree_1, prefix="") + parent_bom_2 = create_nested_bom(bom_tree_2, prefix="") + + # make sure both boms use same subassembly bom + subassembly_bom = parent_bom_1.items[0].bom_no + frappe.db.set_value("BOM Item", parent_bom_2.items[0].name, "bom_no", subassembly_bom) + + plan = create_production_plan(item_code="Red-Car", use_multi_level_bom=1, do_not_save=True) + plan.append("po_items", { # Add Green-Car to Prod Plan + 'use_multi_level_bom': 1, + 'item_code': "Green-Car", + 'bom_no': frappe.db.get_value('Item', "Green-Car", 'default_bom'), + 'planned_qty': 1, + 'planned_start_date': now_datetime() + }) + plan.get_sub_assembly_items() + self.assertTrue(len(plan.sub_assembly_items), 2) + + plan.combine_sub_items = 1 + plan.get_sub_assembly_items() + + self.assertTrue(len(plan.sub_assembly_items), 1) # check if sub-assembly items merged + self.assertEqual(plan.sub_assembly_items[0].qty, 2.0) + self.assertEqual(plan.sub_assembly_items[0].stock_qty, 2.0) + def test_pp_to_mr_customer_provided(self): " Test Material Request from Production Plan for Customer Provided Item." create_item('CUST-0987', is_customer_provided_item = 1, customer = '_Test Customer', is_purchase_item = 0) From 25fa6344a6c3dc3af7ecea250d30a421f4016afc Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 1 Mar 2022 12:24:45 +0530 Subject: [PATCH 08/16] test: Adjust test as per teardown - make tests use multi level bom in WO, as BOM at setup is used - earlier, an intermediate BOM made in a test would override the BOM at setup --- .../doctype/production_plan/test_production_plan.py | 4 +++- erpnext/manufacturing/doctype/work_order/test_work_order.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py index 85ecec2f69..ca8cc7a47e 100644 --- a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py @@ -154,7 +154,7 @@ class TestProductionPlan(FrappeTestCase): use_multi_level_bom=0, ignore_existing_ordered_qty=0 ) - self.assertTrue(len(pln.mr_items), 0) + self.assertTrue(len(pln.mr_items)) sr1.cancel() sr2.cancel() @@ -575,6 +575,7 @@ class TestProductionPlan(FrappeTestCase): wip_warehouse='Work In Progress - _TC', fg_warehouse='Finished Goods - _TC', skip_transfer=1, + use_multi_level_bom=1, do_not_submit=True ) wo.production_plan = pln.name @@ -619,6 +620,7 @@ class TestProductionPlan(FrappeTestCase): wip_warehouse='Work In Progress - _TC', fg_warehouse='Finished Goods - _TC', skip_transfer=1, + use_multi_level_bom=1, do_not_submit=True ) wo.production_plan = pln.name diff --git a/erpnext/manufacturing/doctype/work_order/test_work_order.py b/erpnext/manufacturing/doctype/work_order/test_work_order.py index 549ec7b4a6..bc07d22e83 100644 --- a/erpnext/manufacturing/doctype/work_order/test_work_order.py +++ b/erpnext/manufacturing/doctype/work_order/test_work_order.py @@ -1040,7 +1040,7 @@ def make_wo_order_test_record(**args): wo_order.scrap_warehouse = args.fg_warehouse or "_Test Scrap Warehouse - _TC" wo_order.company = args.company or "_Test Company" wo_order.stock_uom = args.stock_uom or "_Test UOM" - wo_order.use_multi_level_bom=0 + wo_order.use_multi_level_bom= args.use_multi_level_bom or 0 wo_order.skip_transfer=args.skip_transfer or 0 wo_order.get_items_and_operations_from_bom() wo_order.sales_order = args.sales_order or None From ffbb2c831152686b9ec0b230b3c72b94edce0c6a Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 1 Mar 2022 12:57:09 +0530 Subject: [PATCH 09/16] fix: Assert False for test that is anticipating an empty table and remove second arg to assertTrue --- .../doctype/production_plan/test_production_plan.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py index ca8cc7a47e..59cc17a5ee 100644 --- a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py @@ -113,7 +113,7 @@ class TestProductionPlan(FrappeTestCase): item_code='Test Production Item 1', ignore_existing_ordered_qty=1 ) - self.assertTrue(len(pln.mr_items), 1) + self.assertTrue(len(pln.mr_items)) self.assertTrue(flt(pln.mr_items[0].quantity), 1.0) sr1.cancel() @@ -154,7 +154,7 @@ class TestProductionPlan(FrappeTestCase): use_multi_level_bom=0, ignore_existing_ordered_qty=0 ) - self.assertTrue(len(pln.mr_items)) + self.assertFalse(len(pln.mr_items)) sr1.cancel() sr2.cancel() From d22a1d440c96d5a476ce466074a264362a4c814a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 1 Mar 2022 13:06:40 +0530 Subject: [PATCH 10/16] fix: track changes on warehouse --- erpnext/stock/doctype/warehouse/warehouse.json | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/erpnext/stock/doctype/warehouse/warehouse.json b/erpnext/stock/doctype/warehouse/warehouse.json index 05076b51a3..c695d541bf 100644 --- a/erpnext/stock/doctype/warehouse/warehouse.json +++ b/erpnext/stock/doctype/warehouse/warehouse.json @@ -244,7 +244,7 @@ "idx": 1, "is_tree": 1, "links": [], - "modified": "2021-12-03 04:40:06.414630", + "modified": "2022-03-01 02:37:48.034944", "modified_by": "Administrator", "module": "Stock", "name": "Warehouse", @@ -301,5 +301,7 @@ "show_name_in_global_search": 1, "sort_field": "modified", "sort_order": "DESC", - "title_field": "warehouse_name" + "states": [], + "title_field": "warehouse_name", + "track_changes": 1 } \ No newline at end of file From ad2c64f3ff3cafa0abcee35639c9e5d4952bccdf Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Tue, 1 Mar 2022 13:32:34 +0530 Subject: [PATCH 11/16] fix: debit credit difference case with rounding adjustment --- .../sales_invoice/test_sales_invoice.py | 50 +++++++++++++++++++ erpnext/accounts/general_ledger.py | 2 +- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py index 941061f2a2..83caab43b4 100644 --- a/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/test_sales_invoice.py @@ -1595,6 +1595,56 @@ class TestSalesInvoice(unittest.TestCase): self.assertEqual(expected_values[gle.account][1], gle.debit) self.assertEqual(expected_values[gle.account][2], gle.credit) + def test_rounding_adjustment_3(self): + si = create_sales_invoice(do_not_save=True) + si.items = [] + for d in [(1122, 2), (1122.01, 1), (1122.01, 1)]: + si.append("items", { + "item_code": "_Test Item", + "gst_hsn_code": "999800", + "warehouse": "_Test Warehouse - _TC", + "qty": d[1], + "rate": d[0], + "income_account": "Sales - _TC", + "cost_center": "_Test Cost Center - _TC" + }) + for tax_account in ["_Test Account VAT - _TC", "_Test Account Service Tax - _TC"]: + si.append("taxes", { + "charge_type": "On Net Total", + "account_head": tax_account, + "description": tax_account, + "rate": 6, + "cost_center": "_Test Cost Center - _TC", + "included_in_print_rate": 1 + }) + si.save() + si.submit() + self.assertEqual(si.net_total, 4007.16) + self.assertEqual(si.grand_total, 4488.02) + self.assertEqual(si.total_taxes_and_charges, 480.86) + self.assertEqual(si.rounding_adjustment, -0.02) + + expected_values = dict((d[0], d) for d in [ + [si.debit_to, 4488.0, 0.0], + ["_Test Account Service Tax - _TC", 0.0, 240.43], + ["_Test Account VAT - _TC", 0.0, 240.43], + ["Sales - _TC", 0.0, 4007.15], + ["Round Off - _TC", 0.01, 0] + ]) + + gl_entries = frappe.db.sql("""select account, debit, credit + from `tabGL Entry` where voucher_type='Sales Invoice' and voucher_no=%s + order by account asc""", si.name, as_dict=1) + + debit_credit_diff = 0 + for gle in gl_entries: + self.assertEqual(expected_values[gle.account][0], gle.account) + self.assertEqual(expected_values[gle.account][1], gle.debit) + self.assertEqual(expected_values[gle.account][2], gle.credit) + debit_credit_diff += (gle.debit - gle.credit) + + self.assertEqual(debit_credit_diff, 0) + def test_sales_invoice_with_shipping_rule(self): from erpnext.accounts.doctype.shipping_rule.test_shipping_rule import create_shipping_rule diff --git a/erpnext/accounts/general_ledger.py b/erpnext/accounts/general_ledger.py index d24d56b4bb..0cd5e86a8c 100644 --- a/erpnext/accounts/general_ledger.py +++ b/erpnext/accounts/general_ledger.py @@ -274,7 +274,7 @@ def make_round_off_gle(gl_map, debit_credit_diff, precision): debit_credit_diff += flt(d.credit) round_off_account_exists = True - if round_off_account_exists and abs(debit_credit_diff) <= (1.0 / (10**precision)): + if round_off_account_exists and abs(debit_credit_diff) < (1.0 / (10**precision)): gl_map.remove(round_off_gle) return From 47fe87a72f40d1651951cecc97e9ae2ae0115fa4 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Tue, 1 Mar 2022 14:56:24 +0530 Subject: [PATCH 12/16] fix(test): flaky test_point_of_sale --- erpnext/tests/test_point_of_sale.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/tests/test_point_of_sale.py b/erpnext/tests/test_point_of_sale.py index 3299c8885f..38f2c16d93 100644 --- a/erpnext/tests/test_point_of_sale.py +++ b/erpnext/tests/test_point_of_sale.py @@ -25,7 +25,7 @@ class TestPointOfSale(unittest.TestCase): Test Stock and Service Item Search. """ - pos_profile = make_pos_profile() + pos_profile = make_pos_profile(name="Test POS Profile for Search") item1 = make_item("Test Search Stock Item", {"is_stock_item": 1}) make_stock_entry( item_code="Test Search Stock Item", From 1d1d586d4dd356cf84edaed8a51f934194219d11 Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 1 Mar 2022 15:42:34 +0530 Subject: [PATCH 13/16] test: Included sub assembly non-merging in test case --- .../doctype/production_plan/test_production_plan.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py index 59cc17a5ee..eeab788d5c 100644 --- a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py @@ -301,6 +301,11 @@ class TestProductionPlan(FrappeTestCase): self.assertEqual(plan.sub_assembly_items[0].qty, 2.0) self.assertEqual(plan.sub_assembly_items[0].stock_qty, 2.0) + # change warehouse in one row, sub-assemblies should not merge + plan.po_items[0].warehouse = "Finished Goods - _TC" + plan.get_sub_assembly_items() + self.assertTrue(len(plan.sub_assembly_items), 2) + def test_pp_to_mr_customer_provided(self): " Test Material Request from Production Plan for Customer Provided Item." create_item('CUST-0987', is_customer_provided_item = 1, customer = '_Test Customer', is_purchase_item = 0) From 65bb72703029a38fedc02f2ad8f03ebfadf0b223 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 1 Mar 2022 17:25:42 +0530 Subject: [PATCH 14/16] fix: dont validate empty category --- .../sales_taxes_and_charges_template.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/erpnext/accounts/doctype/sales_taxes_and_charges_template/sales_taxes_and_charges_template.py b/erpnext/accounts/doctype/sales_taxes_and_charges_template/sales_taxes_and_charges_template.py index 1d30934df9..8043a1b66f 100644 --- a/erpnext/accounts/doctype/sales_taxes_and_charges_template/sales_taxes_and_charges_template.py +++ b/erpnext/accounts/doctype/sales_taxes_and_charges_template/sales_taxes_and_charges_template.py @@ -55,5 +55,8 @@ def validate_disabled(doc): frappe.throw(_("Disabled template must not be default template")) def validate_for_tax_category(doc): + if not doc.tax_category: + return + if frappe.db.exists(doc.doctype, {"company": doc.company, "tax_category": doc.tax_category, "disabled": 0, "name": ["!=", doc.name]}): frappe.throw(_("A template with tax category {0} already exists. Only one template is allowed with each tax category").format(frappe.bold(doc.tax_category))) From 57f92df108ae6c33567a95be22a43015fe3178e5 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Tue, 1 Mar 2022 20:50:36 +0530 Subject: [PATCH 15/16] fix: flake8 issues --- erpnext/hr/doctype/expense_claim/expense_claim.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/erpnext/hr/doctype/expense_claim/expense_claim.py b/erpnext/hr/doctype/expense_claim/expense_claim.py index 5146a5be90..fe04efbbab 100644 --- a/erpnext/hr/doctype/expense_claim/expense_claim.py +++ b/erpnext/hr/doctype/expense_claim/expense_claim.py @@ -345,10 +345,10 @@ def get_advances(employee, advance_id=None): query = ( frappe.qb.from_(advance) - .select( - advance.name, advance.posting_date, advance.paid_amount, - advance.claimed_amount, advance.advance_account - ) + .select( + advance.name, advance.posting_date, advance.paid_amount, + advance.claimed_amount, advance.advance_account + ) ) if not advance_id: From 9988ec697f056b476ead3be1a371a82c6337be8e Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Tue, 1 Mar 2022 21:40:39 +0530 Subject: [PATCH 16/16] test: test advance filters in expense claim and cancelled status --- .../employee_advance/employee_advance.py | 1 + .../employee_advance/test_employee_advance.py | 38 +++++++++++++++++-- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/erpnext/hr/doctype/employee_advance/employee_advance.py b/erpnext/hr/doctype/employee_advance/employee_advance.py index f63bb86129..79d389d440 100644 --- a/erpnext/hr/doctype/employee_advance/employee_advance.py +++ b/erpnext/hr/doctype/employee_advance/employee_advance.py @@ -27,6 +27,7 @@ class EmployeeAdvance(Document): def on_cancel(self): self.ignore_linked_doctypes = ('GL Entry') + self.set_status(update=True) def set_status(self, update=False): precision = self.precision("paid_amount") diff --git a/erpnext/hr/doctype/employee_advance/test_employee_advance.py b/erpnext/hr/doctype/employee_advance/test_employee_advance.py index 5f3a66a04f..e3c1487ca2 100644 --- a/erpnext/hr/doctype/employee_advance/test_employee_advance.py +++ b/erpnext/hr/doctype/employee_advance/test_employee_advance.py @@ -24,6 +24,9 @@ from erpnext.payroll.doctype.salary_structure.test_salary_structure import make_ class TestEmployeeAdvance(unittest.TestCase): + def setUp(self): + frappe.db.delete("Employee Advance") + def test_paid_amount_and_status(self): employee_name = make_employee("_T@employe.advance") advance = make_employee_advance(employee_name) @@ -58,8 +61,12 @@ class TestEmployeeAdvance(unittest.TestCase): self.assertEqual(advance.paid_amount, 0) self.assertEqual(advance.status, "Unpaid") - def test_claimed_and_returned_status(self): - # Claimed Status check, full amount claimed + advance.cancel() + advance.reload() + self.assertEqual(advance.status, "Cancelled") + + def test_claimed_status(self): + # CLAIMED Status check, full amount claimed payable_account = get_payable_account("_Test Company") claim = make_expense_claim(payable_account, 1000, 1000, "_Test Company", "Travel Expenses - _TC", do_not_submit=True) @@ -75,13 +82,26 @@ class TestEmployeeAdvance(unittest.TestCase): self.assertEqual(advance.claimed_amount, 1000) self.assertEqual(advance.status, "Claimed") + # advance should not be shown in claims + advances = get_advances(claim.employee) + advances = [entry.name for entry in advances] + self.assertTrue(advance.name not in advances) + # cancel claim; status should be Paid claim.cancel() advance.reload() self.assertEqual(advance.claimed_amount, 0) self.assertEqual(advance.status, "Paid") - # Partly Claimed and Returned status check + def test_partly_claimed_and_returned_status(self): + 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) + pe = make_payment_entry(advance) + pe.submit() + + # PARTLY CLAIMED AND RETURNED status check # 500 Claimed, 500 Returned claim = make_expense_claim(payable_account, 500, 500, "_Test Company", "Travel Expenses - _TC", do_not_submit=True) @@ -116,12 +136,22 @@ class TestEmployeeAdvance(unittest.TestCase): self.assertEqual(advance.return_amount, 500) self.assertEqual(advance.status, "Partly Claimed and Returned") - # Cancel return entry; status should change to Paid + # advance should not be shown in claims + advances = get_advances(claim.employee) + advances = [entry.name for entry in advances] + self.assertTrue(advance.name not in advances) + + # Cancel return entry; status should change to PAID entry.cancel() advance.reload() self.assertEqual(advance.return_amount, 0) self.assertEqual(advance.status, "Paid") + # advance should be shown in claims + advances = get_advances(claim.employee) + advances = [entry.name for entry in advances] + self.assertTrue(advance.name in advances) + def test_repay_unclaimed_amount_from_salary(self): employee_name = make_employee("_T@employe.advance") advance = make_employee_advance(employee_name, {"repay_unclaimed_amount_from_salary": 1})