From 03a6c38f06034383b57593145f8c9e77c42d6370 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 26 Jul 2021 22:24:25 +0530 Subject: [PATCH 1/7] test: fix test due to rename change --- erpnext/selling/doctype/sales_order/test_sales_order.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/erpnext/selling/doctype/sales_order/test_sales_order.py b/erpnext/selling/doctype/sales_order/test_sales_order.py index 974648d6d4..1de1e00097 100644 --- a/erpnext/selling/doctype/sales_order/test_sales_order.py +++ b/erpnext/selling/doctype/sales_order/test_sales_order.py @@ -673,6 +673,8 @@ class TestSalesOrder(unittest.TestCase): so.cancel() + dn.load_from_db() + self.assertRaises(frappe.CancelledLinkError, dn.submit) def test_service_type_product_bundle(self): From 9152715f9049585e8d59967dd5c4fef15f04428c Mon Sep 17 00:00:00 2001 From: Ankush Date: Wed, 11 Aug 2021 11:17:50 +0530 Subject: [PATCH 2/7] perf: various minor perf fixes for ledger postings (#26775) * perf: only validate if voucher is journal entry * perf: optimize merge GLE - Order fields such that comparison will fail faster - Break out of loops if not matched * perf: don't try to match SLE if count mismatch * refactor: simplify initialize_previous_data * perf: use cache for fetching valuation_method These are set only once fields * refactor: simplify get_future_stock_vouchers * refactor: simplify get_voucherwise_gl_entries * perf: fetch only required fields for GL comparison `select *` fetches all fields, output of this function is only used for comparing. * perf: reorder conditions in PL cost center check * perf: reduce query while validating new gle * perf: use cache for validating warehouse props These properties don't change often, no need to query everytime. * perf: use cached stock settings to validate SLE * docs: update misleading docstring Co-authored-by: Marica --- erpnext/accounts/doctype/gl_entry/gl_entry.py | 22 ++++++----- erpnext/accounts/general_ledger.py | 25 ++++++++----- erpnext/accounts/utils.py | 37 +++++++++++++------ .../stock_ledger_entry/stock_ledger_entry.py | 4 +- erpnext/stock/stock_ledger.py | 12 +++--- erpnext/stock/utils.py | 8 ++-- 6 files changed, 65 insertions(+), 43 deletions(-) diff --git a/erpnext/accounts/doctype/gl_entry/gl_entry.py b/erpnext/accounts/doctype/gl_entry/gl_entry.py index 11465b711e..0844995f29 100644 --- a/erpnext/accounts/doctype/gl_entry/gl_entry.py +++ b/erpnext/accounts/doctype/gl_entry/gl_entry.py @@ -58,8 +58,8 @@ class GLEntry(Document): if not self.get(k): frappe.throw(_("{0} is required").format(_(self.meta.get_label(k)))) - account_type = frappe.get_cached_value("Account", self.account, "account_type") if not (self.party_type and self.party): + account_type = frappe.get_cached_value("Account", self.account, "account_type") if account_type == "Receivable": frappe.throw(_("{0} {1}: Customer is required against Receivable account {2}") .format(self.voucher_type, self.voucher_no, self.account)) @@ -73,15 +73,19 @@ class GLEntry(Document): .format(self.voucher_type, self.voucher_no, self.account)) def pl_must_have_cost_center(self): - if frappe.get_cached_value("Account", self.account, "report_type") == "Profit and Loss": - if not self.cost_center and self.voucher_type != 'Period Closing Voucher': - msg = _("{0} {1}: Cost Center is required for 'Profit and Loss' account {2}.").format( - self.voucher_type, self.voucher_no, self.account) - msg += " " - msg += _("Please set the cost center field in {0} or setup a default Cost Center for the Company.").format( - self.voucher_type) + """Validate that profit and loss type account GL entries have a cost center.""" - frappe.throw(msg, title=_("Missing Cost Center")) + if self.cost_center or self.voucher_type == 'Period Closing Voucher': + return + + if frappe.get_cached_value("Account", self.account, "report_type") == "Profit and Loss": + msg = _("{0} {1}: Cost Center is required for 'Profit and Loss' account {2}.").format( + self.voucher_type, self.voucher_no, self.account) + msg += " " + msg += _("Please set the cost center field in {0} or setup a default Cost Center for the Company.").format( + self.voucher_type) + + frappe.throw(msg, title=_("Missing Cost Center")) def validate_dimensions_for_pl_and_bs(self): account_type = frappe.db.get_value("Account", self.account, "report_type") diff --git a/erpnext/accounts/general_ledger.py b/erpnext/accounts/general_ledger.py index 25d2cf10bd..4c7c567b42 100644 --- a/erpnext/accounts/general_ledger.py +++ b/erpnext/accounts/general_ledger.py @@ -100,8 +100,8 @@ def merge_similar_entries(gl_map, precision=None): return merged_gl_map def check_if_in_list(gle, gl_map, dimensions=None): - account_head_fieldnames = ['party_type', 'party', 'against_voucher', 'against_voucher_type', - 'cost_center', 'project', 'voucher_detail_no'] + account_head_fieldnames = ['voucher_detail_no', 'party', 'against_voucher', + 'cost_center', 'against_voucher_type', 'party_type', 'project'] if dimensions: account_head_fieldnames = account_head_fieldnames + dimensions @@ -110,10 +110,12 @@ def check_if_in_list(gle, gl_map, dimensions=None): same_head = True if e.account != gle.account: same_head = False + continue for fieldname in account_head_fieldnames: if cstr(e.get(fieldname)) != cstr(gle.get(fieldname)): same_head = False + break if same_head: return e @@ -143,16 +145,19 @@ def make_entry(args, adv_adj, update_outstanding, from_repost=False): validate_expense_against_budget(args) def validate_cwip_accounts(gl_map): - cwip_enabled = any(cint(ac.enable_cwip_accounting) for ac in frappe.db.get_all("Asset Category","enable_cwip_accounting")) + """Validate that CWIP account are not used in Journal Entry""" + if gl_map and gl_map[0].voucher_type != "Journal Entry": + return - if cwip_enabled and gl_map[0].voucher_type == "Journal Entry": - cwip_accounts = [d[0] for d in frappe.db.sql("""select name from tabAccount - where account_type = 'Capital Work in Progress' and is_group=0""")] + cwip_enabled = any(cint(ac.enable_cwip_accounting) for ac in frappe.db.get_all("Asset Category", "enable_cwip_accounting")) + if cwip_enabled: + cwip_accounts = [d[0] for d in frappe.db.sql("""select name from tabAccount + where account_type = 'Capital Work in Progress' and is_group=0""")] - for entry in gl_map: - if entry.account in cwip_accounts: - frappe.throw( - _("Account: {0} is capital Work in progress and can not be updated by Journal Entry").format(entry.account)) + for entry in gl_map: + if entry.account in cwip_accounts: + frappe.throw( + _("Account: {0} is capital Work in progress and can not be updated by Journal Entry").format(entry.account)) def round_off_debit_credit(gl_map): precision = get_field_precision(frappe.get_meta("GL Entry").get_field("debit"), diff --git a/erpnext/accounts/utils.py b/erpnext/accounts/utils.py index 9272bc4fce..5b58e874fe 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -920,7 +920,6 @@ def repost_gle_for_stock_vouchers(stock_vouchers, posting_date, company=None, wa _delete_gl_entries(voucher_type, voucher_no) def get_future_stock_vouchers(posting_date, posting_time, for_warehouses=None, for_items=None, company=None): - future_stock_vouchers = [] values = [] condition = "" @@ -936,30 +935,46 @@ def get_future_stock_vouchers(posting_date, posting_time, for_warehouses=None, f condition += " and company = %s" values.append(company) - for d in frappe.db.sql("""select distinct sle.voucher_type, sle.voucher_no + future_stock_vouchers = frappe.db.sql("""select distinct sle.voucher_type, sle.voucher_no from `tabStock Ledger Entry` sle where timestamp(sle.posting_date, sle.posting_time) >= timestamp(%s, %s) and is_cancelled = 0 {condition} order by timestamp(sle.posting_date, sle.posting_time) asc, creation asc for update""".format(condition=condition), - tuple([posting_date, posting_time] + values), as_dict=True): - future_stock_vouchers.append([d.voucher_type, d.voucher_no]) + tuple([posting_date, posting_time] + values), as_dict=True) - return future_stock_vouchers + return [(d.voucher_type, d.voucher_no) for d in future_stock_vouchers] def get_voucherwise_gl_entries(future_stock_vouchers, posting_date): + """ Get voucherwise list of GL entries. + + Only fetches GLE fields required for comparing with new GLE. + Check compare_existing_and_expected_gle function below. + """ gl_entries = {} - if future_stock_vouchers: - for d in frappe.db.sql("""select * from `tabGL Entry` - where posting_date >= %s and voucher_no in (%s)""" % - ('%s', ', '.join(['%s']*len(future_stock_vouchers))), - tuple([posting_date] + [d[1] for d in future_stock_vouchers]), as_dict=1): - gl_entries.setdefault((d.voucher_type, d.voucher_no), []).append(d) + if not future_stock_vouchers: + return gl_entries + + voucher_nos = [d[1] for d in future_stock_vouchers] + + gles = frappe.db.sql(""" + select name, account, credit, debit, cost_center, project + from `tabGL Entry` + where + posting_date >= %s and voucher_no in (%s)""" % + ('%s', ', '.join(['%s'] * len(voucher_nos))), + tuple([posting_date] + voucher_nos), as_dict=1) + + for d in gles: + gl_entries.setdefault((d.voucher_type, d.voucher_no), []).append(d) return gl_entries def compare_existing_and_expected_gle(existing_gle, expected_gle, precision): + if len(existing_gle) != len(expected_gle): + return False + matched = True for entry in expected_gle: account_existed = False diff --git a/erpnext/stock/doctype/stock_ledger_entry/stock_ledger_entry.py b/erpnext/stock/doctype/stock_ledger_entry/stock_ledger_entry.py index b4f458388b..be1f00e37f 100644 --- a/erpnext/stock/doctype/stock_ledger_entry/stock_ledger_entry.py +++ b/erpnext/stock/doctype/stock_ledger_entry/stock_ledger_entry.py @@ -55,8 +55,8 @@ class StockLedgerEntry(Document): "sum(actual_qty)") or 0 frappe.db.set_value("Batch", self.batch_no, "batch_qty", batch_qty) - #check for item quantity available in stock def actual_amt_check(self): + """Validate that qty at warehouse for selected batch is >=0""" if self.batch_no and not self.get("allow_negative_stock"): batch_bal_after_transaction = flt(frappe.db.sql("""select sum(actual_qty) from `tabStock Ledger Entry` @@ -107,7 +107,7 @@ class StockLedgerEntry(Document): self.stock_uom = item_det.stock_uom def check_stock_frozen_date(self): - stock_settings = frappe.get_doc('Stock Settings', 'Stock Settings') + stock_settings = frappe.get_cached_doc('Stock Settings') if stock_settings.stock_frozen_upto: if (getdate(self.posting_date) <= getdate(stock_settings.stock_frozen_upto) diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index f990ce06be..eddd048c74 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -279,15 +279,13 @@ class update_entries_after(object): } """ - self.data.setdefault(args.warehouse, frappe._dict()) - warehouse_dict = self.data[args.warehouse] previous_sle = get_previous_sle_of_current_voucher(args) - warehouse_dict.previous_sle = previous_sle - for key in ("qty_after_transaction", "valuation_rate", "stock_value"): - setattr(warehouse_dict, key, flt(previous_sle.get(key))) - - warehouse_dict.update({ + self.data[args.warehouse] = frappe._dict({ + "previous_sle": previous_sle, + "qty_after_transaction": flt(previous_sle.qty_after_transaction), + "valuation_rate": flt(previous_sle.valuation_rate), + "stock_value": flt(previous_sle.stock_value), "prev_stock_value": previous_sle.stock_value or 0.0, "stock_queue": json.loads(previous_sle.stock_queue or "[]"), "stock_value_difference": 0.0 diff --git a/erpnext/stock/utils.py b/erpnext/stock/utils.py index b57b2aa6b8..9f6d0a8add 100644 --- a/erpnext/stock/utils.py +++ b/erpnext/stock/utils.py @@ -224,7 +224,7 @@ def get_avg_purchase_rate(serial_nos): def get_valuation_method(item_code): """get valuation method from item or default""" - val_method = frappe.db.get_value('Item', item_code, 'valuation_method') + val_method = frappe.db.get_value('Item', item_code, 'valuation_method', cache=True) if not val_method: val_method = frappe.db.get_value("Stock Settings", None, "valuation_method") or "FIFO" return val_method @@ -275,17 +275,17 @@ def get_valid_serial_nos(sr_nos, qty=0, item_code=''): return valid_serial_nos def validate_warehouse_company(warehouse, company): - warehouse_company = frappe.db.get_value("Warehouse", warehouse, "company") + warehouse_company = frappe.db.get_value("Warehouse", warehouse, "company", cache=True) if warehouse_company and warehouse_company != company: frappe.throw(_("Warehouse {0} does not belong to company {1}").format(warehouse, company), InvalidWarehouseCompany) def is_group_warehouse(warehouse): - if frappe.db.get_value("Warehouse", warehouse, "is_group"): + if frappe.db.get_value("Warehouse", warehouse, "is_group", cache=True): frappe.throw(_("Group node warehouse is not allowed to select for transactions")) def validate_disabled_warehouse(warehouse): - if frappe.db.get_value("Warehouse", warehouse, "disabled"): + if frappe.db.get_value("Warehouse", warehouse, "disabled", cache=True): frappe.throw(_("Disabled Warehouse {0} cannot be used for this transaction.").format(get_link_to_form('Warehouse', warehouse))) def update_included_uom_in_report(columns, result, include_uom, conversion_factors): From e5b02216933a71c06457920e5f3a58a06ffa5bbc Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 11 Aug 2021 13:02:49 +0530 Subject: [PATCH 3/7] test: fix attendance request tests - Use `frappe.db.get_value` instead of `get_doc` for asserting values - Get values after cancellation as reloading attendance doc breaks due to stale doc (primary key changed after cancel of attendance request) - rollback everything on tearDown --- .../test_attendance_request.py | 68 ++++++++++++++----- 1 file changed, 52 insertions(+), 16 deletions(-) diff --git a/erpnext/hr/doctype/attendance_request/test_attendance_request.py b/erpnext/hr/doctype/attendance_request/test_attendance_request.py index 3c42bd9fc3..0898476edf 100644 --- a/erpnext/hr/doctype/attendance_request/test_attendance_request.py +++ b/erpnext/hr/doctype/attendance_request/test_attendance_request.py @@ -15,6 +15,9 @@ class TestAttendanceRequest(unittest.TestCase): for doctype in ["Attendance Request", "Attendance"]: frappe.db.sql("delete from `tab{doctype}`".format(doctype=doctype)) + def tearDown(self): + frappe.db.rollback() + def test_on_duty_attendance_request(self): today = nowdate() employee = get_employee() @@ -26,15 +29,33 @@ class TestAttendanceRequest(unittest.TestCase): attendance_request.company = "_Test Company" attendance_request.insert() attendance_request.submit() - attendance = frappe.get_doc('Attendance', { - 'employee': employee.name, - 'attendance_date': date(date.today().year, 1, 1), - 'docstatus': 1 - }) - self.assertEqual(attendance.status, 'Present') + + attendance = frappe.db.get_value( + "Attendance", + filters={ + "attendance_request": attendance_request.name, + "attendance_date": date(date.today().year, 1, 1) + }, + fieldname=["status", "docstatus"], + as_dict=True + ) + self.assertEqual(attendance.status, "Present") + self.assertEqual(attendance.docstatus, 1) + + # cancelling attendance request cancels linked attendances attendance_request.cancel() - attendance.reload() - self.assertEqual(attendance.docstatus, 2) + + # cancellation alters docname + # fetch attendance value again to avoid stale docname + attendance_docstatus = frappe.db.get_value( + "Attendance", + filters={ + "attendance_request": attendance_request.name, + "attendance_date": date(date.today().year, 1, 1) + }, + fieldname="docstatus" + ) + self.assertEqual(attendance_docstatus, 2) def test_work_from_home_attendance_request(self): today = nowdate() @@ -47,15 +68,30 @@ class TestAttendanceRequest(unittest.TestCase): attendance_request.company = "_Test Company" attendance_request.insert() attendance_request.submit() - attendance = frappe.get_doc('Attendance', { - 'employee': employee.name, - 'attendance_date': date(date.today().year, 1, 1), - 'docstatus': 1 - }) - self.assertEqual(attendance.status, 'Work From Home') + + attendance_status = frappe.db.get_value( + "Attendance", + filters={ + "attendance_request": attendance_request.name, + "attendance_date": date(date.today().year, 1, 1) + }, + fieldname="status" + ) + self.assertEqual(attendance_status, 'Work From Home') + attendance_request.cancel() - attendance.reload() - self.assertEqual(attendance.docstatus, 2) + + # cancellation alters docname + # fetch attendance value again to avoid stale docname + attendance_docstatus = frappe.db.get_value( + "Attendance", + filters={ + "attendance_request": attendance_request.name, + "attendance_date": date(date.today().year, 1, 1) + }, + fieldname="docstatus" + ) + self.assertEqual(attendance_docstatus, 2) def get_employee(): return frappe.get_doc("Employee", "_T-Employee-00001") From bca30d6101f539b4f68c536e83a82c1cd29b1bb7 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 11 Aug 2021 13:29:44 +0530 Subject: [PATCH 4/7] test: fix Shift Request test - Use `get_value` instead of `get_doc` - Remove unnecessary loop, only one shift assignment is made against a shift request - Get value after cancel again. Get doc is not reliable since primary key changed after cancel --- .../test_attendance_request.py | 2 ++ .../shift_request/test_shift_request.py | 33 ++++++++++++------- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/erpnext/hr/doctype/attendance_request/test_attendance_request.py b/erpnext/hr/doctype/attendance_request/test_attendance_request.py index 0898476edf..9e668aa72f 100644 --- a/erpnext/hr/doctype/attendance_request/test_attendance_request.py +++ b/erpnext/hr/doctype/attendance_request/test_attendance_request.py @@ -19,6 +19,7 @@ class TestAttendanceRequest(unittest.TestCase): frappe.db.rollback() def test_on_duty_attendance_request(self): + "Test creation/updation of Attendace from Attendance Request, on duty." today = nowdate() employee = get_employee() attendance_request = frappe.new_doc("Attendance Request") @@ -58,6 +59,7 @@ class TestAttendanceRequest(unittest.TestCase): self.assertEqual(attendance_docstatus, 2) def test_work_from_home_attendance_request(self): + "Test creation/updation of Attendace from Attendance Request, work from home." today = nowdate() employee = get_employee() attendance_request = frappe.new_doc("Attendance Request") diff --git a/erpnext/hr/doctype/shift_request/test_shift_request.py b/erpnext/hr/doctype/shift_request/test_shift_request.py index 9c0d8e3198..3525540cdf 100644 --- a/erpnext/hr/doctype/shift_request/test_shift_request.py +++ b/erpnext/hr/doctype/shift_request/test_shift_request.py @@ -15,24 +15,35 @@ class TestShiftRequest(unittest.TestCase): for doctype in ["Shift Request", "Shift Assignment"]: frappe.db.sql("delete from `tab{doctype}`".format(doctype=doctype)) + def tearDown(self): + frappe.db.rollback() + def test_make_shift_request(self): + "Test creation/updation of Shift Assignment from Shift Request." department = frappe.get_value("Employee", "_T-Employee-00001", 'department') set_shift_approver(department) approver = frappe.db.sql("""select approver from `tabDepartment Approver` where parent= %s and parentfield = 'shift_request_approver'""", (department))[0][0] shift_request = make_shift_request(approver) - shift_assignments = frappe.db.sql(''' - SELECT shift_request, employee - FROM `tabShift Assignment` - WHERE shift_request = '{0}' - '''.format(shift_request.name), as_dict=1) - for d in shift_assignments: - employee = d.get('employee') - self.assertEqual(shift_request.employee, employee) - shift_request.cancel() - shift_assignment_doc = frappe.get_doc("Shift Assignment", {"shift_request": d.get('shift_request')}) - self.assertEqual(shift_assignment_doc.docstatus, 2) + # Only one shift assignment is created against a shift request + shift_assignment = frappe.db.get_value( + "Shift Assignment", + filters={"shift_request": shift_request.name}, + fieldname=["employee", "docstatus"], + as_dict=True + ) + self.assertEqual(shift_request.employee, shift_assignment.employee) + self.assertEqual(shift_assignment.docstatus, 1) + + shift_request.cancel() + + shift_assignment_docstatus = frappe.db.get_value( + "Shift Assignment", + filters={"shift_request": shift_request.name}, + fieldname="docstatus" + ) + self.assertEqual(shift_assignment_docstatus, 2) def test_shift_request_approver_perms(self): employee = frappe.get_doc("Employee", "_T-Employee-00001") From 8f1a3aef2e3f14e179f4ac91718e9f376b34198c Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 11 Aug 2021 15:17:06 +0530 Subject: [PATCH 5/7] test: fix POS Closing Entry Test - Separated into two tests, one checks if SI cancelling is blocked, the other checks PCE cancel impact - This is done because after cancel via assertRaises, damage done by cancel still exists or is partially comitted - Dont use this partially cancelled doc for any assertions further, end test at exception assertion - Use `get_value` to check SI docstatus, as its primary key changes after cancel --- .../test_pos_closing_entry.py | 43 +++++++++++++++++-- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/erpnext/accounts/doctype/pos_closing_entry/test_pos_closing_entry.py b/erpnext/accounts/doctype/pos_closing_entry/test_pos_closing_entry.py index b596c0cf25..0265f43476 100644 --- a/erpnext/accounts/doctype/pos_closing_entry/test_pos_closing_entry.py +++ b/erpnext/accounts/doctype/pos_closing_entry/test_pos_closing_entry.py @@ -19,6 +19,7 @@ class TestPOSClosingEntry(unittest.TestCase): def tearDown(self): frappe.set_user("Administrator") frappe.db.sql("delete from `tabPOS Profile`") + frappe.db.rollback() def test_pos_closing_entry(self): test_user, pos_profile = init_user_and_profile() @@ -50,7 +51,8 @@ class TestPOSClosingEntry(unittest.TestCase): self.assertEqual(pcv_doc.total_quantity, 2) self.assertEqual(pcv_doc.net_total, 6700) - def test_cancelling_of_pos_closing_entry(self): + def test_cancelling_of_consolidated_sales_invoice(self): + "Check if cancelling consolidated Sales Invoice with submitted POS Closing Entry is blocked." test_user, pos_profile = init_user_and_profile() opening_entry = create_opening_entry(pos_profile, test_user.name) @@ -83,13 +85,46 @@ class TestPOSClosingEntry(unittest.TestCase): si_doc = frappe.get_doc("Sales Invoice", pos_inv1.consolidated_invoice) self.assertRaises(frappe.ValidationError, si_doc.cancel) + def test_cancelling_of_pos_closing_entry(self): + "Check impact of cancelling POS Closing Entry." + test_user, pos_profile = init_user_and_profile() + opening_entry = create_opening_entry(pos_profile, test_user.name) + + pos_inv1 = create_pos_invoice(rate=3500, do_not_submit=1) + pos_inv1.append('payments', { + 'mode_of_payment': 'Cash', 'account': 'Cash - _TC', 'amount': 3500 + }) + pos_inv1.submit() + + pos_inv2 = create_pos_invoice(rate=3200, do_not_submit=1) + pos_inv2.append('payments', { + 'mode_of_payment': 'Cash', 'account': 'Cash - _TC', 'amount': 3200 + }) + pos_inv2.submit() + + pcv_doc = make_closing_entry_from_opening(opening_entry) + payment = pcv_doc.payment_reconciliation[0] + + for d in pcv_doc.payment_reconciliation: + if d.mode_of_payment == 'Cash': + d.closing_amount = 6700 + + pcv_doc.submit() + + pos_inv1.load_from_db() + si_name = pos_inv1.consolidated_invoice + pcv_doc.load_from_db() pcv_doc.cancel() - si_doc.load_from_db() pos_inv1.load_from_db() - self.assertEqual(si_doc.docstatus, 2) - self.assertEqual(pos_inv1.status, 'Paid') + # After POS Closing Entry cancel, SI doc gets cancelled, unlinked and renamed + # There's no reference doc to fetch SI with new cancelled name + # which is why we are hardcoding suffix + si_docstatus = frappe.db.get_value("Sales Invoice", si_name+"-CANC-0", "docstatus") + self.assertEqual(si_docstatus, 2) + self.assertEqual(pos_inv1.status, 'Paid') + self.assertIsNone(pos_inv1.consolidated_invoice) def init_user_and_profile(**args): user = 'test@example.com' From 867939fcae12c689c145c0d8610a469508f1b900 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 11 Aug 2021 16:40:07 +0530 Subject: [PATCH 6/7] test: fixed asset movement tests - set cwip account in company to avoid value missing - removed unused statement - removed trailing spaces --- .../test_pos_closing_entry.py | 1 - .../asset_movement/test_asset_movement.py | 19 ++++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/erpnext/accounts/doctype/pos_closing_entry/test_pos_closing_entry.py b/erpnext/accounts/doctype/pos_closing_entry/test_pos_closing_entry.py index 0265f43476..c585f310b1 100644 --- a/erpnext/accounts/doctype/pos_closing_entry/test_pos_closing_entry.py +++ b/erpnext/accounts/doctype/pos_closing_entry/test_pos_closing_entry.py @@ -103,7 +103,6 @@ class TestPOSClosingEntry(unittest.TestCase): pos_inv2.submit() pcv_doc = make_closing_entry_from_opening(opening_entry) - payment = pcv_doc.payment_reconciliation[0] for d in pcv_doc.payment_reconciliation: if d.mode_of_payment == 'Cash': diff --git a/erpnext/assets/doctype/asset_movement/test_asset_movement.py b/erpnext/assets/doctype/asset_movement/test_asset_movement.py index cddee5fa0f..985bca9d0d 100644 --- a/erpnext/assets/doctype/asset_movement/test_asset_movement.py +++ b/erpnext/assets/doctype/asset_movement/test_asset_movement.py @@ -15,6 +15,7 @@ from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import make_pu class TestAssetMovement(unittest.TestCase): def setUp(self): + frappe.db.set_value("Company", "_Test Company", "capital_work_in_progress_account", "CWIP Account - _TC") create_asset_data() make_location() @@ -45,12 +46,12 @@ class TestAssetMovement(unittest.TestCase): 'location_name': 'Test Location 2' }).insert() - movement1 = create_asset_movement(purpose = 'Transfer', company = asset.company, + movement1 = create_asset_movement(purpose = 'Transfer', company = asset.company, assets = [{ 'asset': asset.name , 'source_location': 'Test Location', 'target_location': 'Test Location 2'}], reference_doctype = 'Purchase Receipt', reference_name = pr.name) self.assertEqual(frappe.db.get_value("Asset", asset.name, "location"), "Test Location 2") - movement2 = create_asset_movement(purpose = 'Transfer', company = asset.company, + movement2 = create_asset_movement(purpose = 'Transfer', company = asset.company, assets = [{ 'asset': asset.name , 'source_location': 'Test Location 2', 'target_location': 'Test Location'}], reference_doctype = 'Purchase Receipt', reference_name = pr.name) self.assertEqual(frappe.db.get_value("Asset", asset.name, "location"), "Test Location") @@ -59,18 +60,18 @@ class TestAssetMovement(unittest.TestCase): self.assertEqual(frappe.db.get_value("Asset", asset.name, "location"), "Test Location") employee = make_employee("testassetmovemp@example.com", company="_Test Company") - movement3 = create_asset_movement(purpose = 'Issue', company = asset.company, + movement3 = create_asset_movement(purpose = 'Issue', company = asset.company, assets = [{ 'asset': asset.name , 'source_location': 'Test Location', 'to_employee': employee}], reference_doctype = 'Purchase Receipt', reference_name = pr.name) - + # after issuing asset should belong to an employee not at a location self.assertEqual(frappe.db.get_value("Asset", asset.name, "location"), None) self.assertEqual(frappe.db.get_value("Asset", asset.name, "custodian"), employee) - + def test_last_movement_cancellation(self): pr = make_purchase_receipt(item_code="Macbook Pro", qty=1, rate=100000.0, location="Test Location") - + asset_name = frappe.db.get_value("Asset", {"purchase_receipt": pr.name}, 'name') asset = frappe.get_doc('Asset', asset_name) asset.calculate_depreciation = 1 @@ -85,17 +86,17 @@ class TestAssetMovement(unittest.TestCase): }) if asset.docstatus == 0: asset.submit() - + if not frappe.db.exists("Location", "Test Location 2"): frappe.get_doc({ 'doctype': 'Location', 'location_name': 'Test Location 2' }).insert() - + movement = frappe.get_doc({'doctype': 'Asset Movement', 'reference_name': pr.name }) self.assertRaises(frappe.ValidationError, movement.cancel) - movement1 = create_asset_movement(purpose = 'Transfer', company = asset.company, + movement1 = create_asset_movement(purpose = 'Transfer', company = asset.company, assets = [{ 'asset': asset.name , 'source_location': 'Test Location', 'target_location': 'Test Location 2'}], reference_doctype = 'Purchase Receipt', reference_name = pr.name) self.assertEqual(frappe.db.get_value("Asset", asset.name, "location"), "Test Location 2") From 455d300fca044643eb37a78568e123a62a94ef38 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 11 Aug 2021 17:00:46 +0530 Subject: [PATCH 7/7] Revert "test: fix POS Closing Entry Test" This reverts commit 8f1a3aef2e3f14e179f4ac91718e9f376b34198c. --- .../test_pos_closing_entry.py | 42 ++----------------- .../asset_movement/test_asset_movement.py | 4 +- 2 files changed, 6 insertions(+), 40 deletions(-) diff --git a/erpnext/accounts/doctype/pos_closing_entry/test_pos_closing_entry.py b/erpnext/accounts/doctype/pos_closing_entry/test_pos_closing_entry.py index c585f310b1..b596c0cf25 100644 --- a/erpnext/accounts/doctype/pos_closing_entry/test_pos_closing_entry.py +++ b/erpnext/accounts/doctype/pos_closing_entry/test_pos_closing_entry.py @@ -19,7 +19,6 @@ class TestPOSClosingEntry(unittest.TestCase): def tearDown(self): frappe.set_user("Administrator") frappe.db.sql("delete from `tabPOS Profile`") - frappe.db.rollback() def test_pos_closing_entry(self): test_user, pos_profile = init_user_and_profile() @@ -51,8 +50,7 @@ class TestPOSClosingEntry(unittest.TestCase): self.assertEqual(pcv_doc.total_quantity, 2) self.assertEqual(pcv_doc.net_total, 6700) - def test_cancelling_of_consolidated_sales_invoice(self): - "Check if cancelling consolidated Sales Invoice with submitted POS Closing Entry is blocked." + def test_cancelling_of_pos_closing_entry(self): test_user, pos_profile = init_user_and_profile() opening_entry = create_opening_entry(pos_profile, test_user.name) @@ -85,45 +83,13 @@ class TestPOSClosingEntry(unittest.TestCase): si_doc = frappe.get_doc("Sales Invoice", pos_inv1.consolidated_invoice) self.assertRaises(frappe.ValidationError, si_doc.cancel) - def test_cancelling_of_pos_closing_entry(self): - "Check impact of cancelling POS Closing Entry." - test_user, pos_profile = init_user_and_profile() - opening_entry = create_opening_entry(pos_profile, test_user.name) - - pos_inv1 = create_pos_invoice(rate=3500, do_not_submit=1) - pos_inv1.append('payments', { - 'mode_of_payment': 'Cash', 'account': 'Cash - _TC', 'amount': 3500 - }) - pos_inv1.submit() - - pos_inv2 = create_pos_invoice(rate=3200, do_not_submit=1) - pos_inv2.append('payments', { - 'mode_of_payment': 'Cash', 'account': 'Cash - _TC', 'amount': 3200 - }) - pos_inv2.submit() - - pcv_doc = make_closing_entry_from_opening(opening_entry) - - for d in pcv_doc.payment_reconciliation: - if d.mode_of_payment == 'Cash': - d.closing_amount = 6700 - - pcv_doc.submit() - - pos_inv1.load_from_db() - si_name = pos_inv1.consolidated_invoice - pcv_doc.load_from_db() pcv_doc.cancel() + si_doc.load_from_db() pos_inv1.load_from_db() - - # After POS Closing Entry cancel, SI doc gets cancelled, unlinked and renamed - # There's no reference doc to fetch SI with new cancelled name - # which is why we are hardcoding suffix - si_docstatus = frappe.db.get_value("Sales Invoice", si_name+"-CANC-0", "docstatus") - self.assertEqual(si_docstatus, 2) + self.assertEqual(si_doc.docstatus, 2) self.assertEqual(pos_inv1.status, 'Paid') - self.assertIsNone(pos_inv1.consolidated_invoice) + def init_user_and_profile(**args): user = 'test@example.com' diff --git a/erpnext/assets/doctype/asset_movement/test_asset_movement.py b/erpnext/assets/doctype/asset_movement/test_asset_movement.py index 985bca9d0d..2b2d2b4400 100644 --- a/erpnext/assets/doctype/asset_movement/test_asset_movement.py +++ b/erpnext/assets/doctype/asset_movement/test_asset_movement.py @@ -51,7 +51,7 @@ class TestAssetMovement(unittest.TestCase): reference_doctype = 'Purchase Receipt', reference_name = pr.name) self.assertEqual(frappe.db.get_value("Asset", asset.name, "location"), "Test Location 2") - movement2 = create_asset_movement(purpose = 'Transfer', company = asset.company, + create_asset_movement(purpose = 'Transfer', company = asset.company, assets = [{ 'asset': asset.name , 'source_location': 'Test Location 2', 'target_location': 'Test Location'}], reference_doctype = 'Purchase Receipt', reference_name = pr.name) self.assertEqual(frappe.db.get_value("Asset", asset.name, "location"), "Test Location") @@ -60,7 +60,7 @@ class TestAssetMovement(unittest.TestCase): self.assertEqual(frappe.db.get_value("Asset", asset.name, "location"), "Test Location") employee = make_employee("testassetmovemp@example.com", company="_Test Company") - movement3 = create_asset_movement(purpose = 'Issue', company = asset.company, + create_asset_movement(purpose = 'Issue', company = asset.company, assets = [{ 'asset': asset.name , 'source_location': 'Test Location', 'to_employee': employee}], reference_doctype = 'Purchase Receipt', reference_name = pr.name)