From da73685f7172290151a279f8cf796628dbf6617e Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 10 Feb 2022 13:07:51 +0530 Subject: [PATCH 01/44] fix: Multiple fixes in Gross Profit report --- .../report/gross_profit/gross_profit.js | 10 +++-- .../report/gross_profit/gross_profit.py | 43 +++++++++++++------ 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/erpnext/accounts/report/gross_profit/gross_profit.js b/erpnext/accounts/report/gross_profit/gross_profit.js index 685f2d6176b..c8a9a228c61 100644 --- a/erpnext/accounts/report/gross_profit/gross_profit.js +++ b/erpnext/accounts/report/gross_profit/gross_profit.js @@ -8,20 +8,22 @@ frappe.query_reports["Gross Profit"] = { "label": __("Company"), "fieldtype": "Link", "options": "Company", - "reqd": 1, - "default": frappe.defaults.get_user_default("Company") + "default": frappe.defaults.get_user_default("Company"), + "reqd": 1 }, { "fieldname":"from_date", "label": __("From Date"), "fieldtype": "Date", - "default": frappe.defaults.get_user_default("year_start_date") + "default": frappe.defaults.get_user_default("year_start_date"), + "reqd": 1 }, { "fieldname":"to_date", "label": __("To Date"), "fieldtype": "Date", - "default": frappe.defaults.get_user_default("year_end_date") + "default": frappe.defaults.get_user_default("year_end_date"), + "reqd": 1 }, { "fieldname":"sales_invoice", diff --git a/erpnext/accounts/report/gross_profit/gross_profit.py b/erpnext/accounts/report/gross_profit/gross_profit.py index 84effc0f467..225b7c6426a 100644 --- a/erpnext/accounts/report/gross_profit/gross_profit.py +++ b/erpnext/accounts/report/gross_profit/gross_profit.py @@ -369,20 +369,37 @@ class GrossProfitGenerator(object): return self.average_buying_rate[item_code] def get_last_purchase_rate(self, item_code, row): - condition = '' - if row.project: - condition += " AND a.project=%s" % (frappe.db.escape(row.project)) - elif row.cost_center: - condition += " AND a.cost_center=%s" % (frappe.db.escape(row.cost_center)) - if self.filters.to_date: - condition += " AND modified='%s'" % (self.filters.to_date) + purchase_invoice = frappe.qb.DocType("Purchase Invoice") + purchase_invoice_item = frappe.qb.DocType("Purchase Invoice Item") - last_purchase_rate = frappe.db.sql(""" - select (a.base_rate / a.conversion_factor) - from `tabPurchase Invoice Item` a - where a.item_code = %s and a.docstatus=1 - {0} - order by a.modified desc limit 1""".format(condition), item_code) + query = (frappe.qb.from_(purchase_invoice_item) + .inner_join( + purchase_invoice + ).on( + purchase_invoice.name == purchase_invoice_item.parent + ).select( + purchase_invoice_item.base_rate / purchase_invoice_item.conversion_factor + ).where( + purchase_invoice.docstatus == 1 + ).where( + purchase_invoice.posting_date <= self.filters.to_date + ).where( + purchase_invoice_item.item_code == item_code + )) + + if row.project: + query.where( + purchase_invoice_item.item_code == row.project + ) + + if row.cost_center: + query.where( + purchase_invoice_item.cost_center == row.cost_center + ) + + query.orderby(purchase_invoice.posting_date, order=frappe.qb.desc) + query.limit(1) + last_purchase_rate = query.run() return flt(last_purchase_rate[0][0]) if last_purchase_rate else 0 From 2172ab2d37d8be0c43d1f885a40657d352d255b4 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Fri, 11 Feb 2022 14:48:39 +0530 Subject: [PATCH 02/44] fix: Update columns in new format --- .../report/gross_profit/gross_profit.json | 4 +- .../report/gross_profit/gross_profit.py | 80 ++++++------------- 2 files changed, 28 insertions(+), 56 deletions(-) diff --git a/erpnext/accounts/report/gross_profit/gross_profit.json b/erpnext/accounts/report/gross_profit/gross_profit.json index 76c560ad247..0730ffd77e5 100644 --- a/erpnext/accounts/report/gross_profit/gross_profit.json +++ b/erpnext/accounts/report/gross_profit/gross_profit.json @@ -1,5 +1,5 @@ { - "add_total_row": 0, + "add_total_row": 1, "columns": [], "creation": "2013-02-25 17:03:34", "disable_prepared_report": 0, @@ -9,7 +9,7 @@ "filters": [], "idx": 3, "is_standard": "Yes", - "modified": "2021-11-13 19:14:23.730198", + "modified": "2022-02-11 10:18:36.956558", "modified_by": "Administrator", "module": "Accounts", "name": "Gross Profit", diff --git a/erpnext/accounts/report/gross_profit/gross_profit.py b/erpnext/accounts/report/gross_profit/gross_profit.py index 225b7c6426a..c403b76f876 100644 --- a/erpnext/accounts/report/gross_profit/gross_profit.py +++ b/erpnext/accounts/report/gross_profit/gross_profit.py @@ -70,43 +70,42 @@ def get_data_when_grouped_by_invoice(columns, gross_profit_data, filters, group_ data.append(row) def get_data_when_not_grouped_by_invoice(gross_profit_data, filters, group_wise_columns, data): - for idx, src in enumerate(gross_profit_data.grouped_data): + for src in gross_profit_data.grouped_data: row = [] for col in group_wise_columns.get(scrub(filters.group_by)): row.append(src.get(col)) row.append(filters.currency) - if idx == len(gross_profit_data.grouped_data)-1: - row[0] = "Total" data.append(row) def get_columns(group_wise_columns, filters): columns = [] column_map = frappe._dict({ - "parent": _("Sales Invoice") + ":Link/Sales Invoice:120", - "invoice_or_item": _("Sales Invoice") + ":Link/Sales Invoice:120", - "posting_date": _("Posting Date") + ":Date:100", - "posting_time": _("Posting Time") + ":Data:100", - "item_code": _("Item Code") + ":Link/Item:100", - "item_name": _("Item Name") + ":Data:100", - "item_group": _("Item Group") + ":Link/Item Group:100", - "brand": _("Brand") + ":Link/Brand:100", - "description": _("Description") +":Data:100", - "warehouse": _("Warehouse") + ":Link/Warehouse:100", - "qty": _("Qty") + ":Float:80", - "base_rate": _("Avg. Selling Rate") + ":Currency/currency:100", - "buying_rate": _("Valuation Rate") + ":Currency/currency:100", - "base_amount": _("Selling Amount") + ":Currency/currency:100", - "buying_amount": _("Buying Amount") + ":Currency/currency:100", - "gross_profit": _("Gross Profit") + ":Currency/currency:100", - "gross_profit_percent": _("Gross Profit %") + ":Percent:100", - "project": _("Project") + ":Link/Project:100", - "sales_person": _("Sales person"), - "allocated_amount": _("Allocated Amount") + ":Currency/currency:100", - "customer": _("Customer") + ":Link/Customer:100", - "customer_group": _("Customer Group") + ":Link/Customer Group:100", - "territory": _("Territory") + ":Link/Territory:100" + "parent": {"label": _('Sales Invoice'), "fieldname": "parent_invoice", "fieldtype": "Link", "options": "Sales Invoice", "width": 120}, + "invoice_or_item": {"label": _('Sales Invoice'), "fieldtype": "Link", "options": "Sales Invoice", "width": 120}, + "posting_date": {"label": _('Posting Date'), "fieldname": "posting_date", "fieldtype": "Date", "width": 100}, + "posting_time": {"label": _('Posting Time'), "fieldname": "posting_time", "fieldtype": "Data", "width": 100}, + "item_code": {"label": _('Item Code'), "fieldname": "item_code", "fieldtype": "Link", "options": "Item", "width": 100}, + "item_name": {"label": _('Item Name'), "fieldname": "item_name", "fieldtype": "Data", "width": 100}, + "item_group": {"label": _('Item Group'), "fieldname": "item_group", "fieldtype": "Link", "options": "Item Group", "width": 100}, + "brand": {"label": _('Brand'), "fieldtype": "Link", "options": "Brand", "width": 100}, + "description": {"label": _('Description'), "fieldname": "description", "fieldtype": "Data", "width": 100}, + "warehouse": {"label": _('Warehouse'), "fieldname": "warehouse", "fieldtype": "Link", "options": "warehouse", "width": 100}, + "qty": {"label": _('Qty'), "fieldname": "qty", "fieldtype": "Float", "width": 80}, + "base_rate": {"label": _('Avg. Selling Rate'), "fieldname": "avg._selling_rate", "fieldtype": "Currency", "options": "currency", "width": 100}, + "buying_rate": {"label": _('Valuation Rate'), "fieldname": "valuation_rate", "fieldtype": "Currency", "options": "currency", "width": 100}, + "base_amount": {"label": _('Selling Amount'), "fieldname": "selling_amount", "fieldtype": "Currency", "options": "currency", "width": 100}, + "buying_amount": {"label": _('Buying Amount'), "fieldname": "buying_amount", "fieldtype": "Currency", "options": "currency", "width": 100}, + "gross_profit": {"label": _('Gross Profit'), "fieldname": "gross_profit", "fieldtype": "Currency", "options": "currency", "width": 100}, + "gross_profit_percent": {"label": _('Gross Profit Percent'), "fieldname": "gross_profit_%", + "fieldtype": "Percent", "width": 100}, + "project": {"label": _('Project'), "fieldname": "project", "fieldtype": "Link", "options": "Project", "width": 100}, + "sales_person": {"label": _('Sales Person'), "fieldname": "sales_person", "fieldtype": "Data","width": 100}, + "allocated_amount": {"label": _('Allocated Amount'), "fieldname": "allocated_amount", "fieldtype": "Currency", "options": "currency", "width": 100}, + "customer": {"label": _('Customer'), "fieldname": "customer", "fieldtype": "Link", "options": "Customer", "width": 100}, + "customer_group": {"label": _('Customer Group'), "fieldname": "customer_group", "fieldtype": "Link", "options": "customer", "width": 100}, + "territory": {"label": _('Territory'), "fieldname": "territory", "fieldtype": "Link", "options": "territory", "width": 100}, }) for col in group_wise_columns.get(scrub(filters.group_by)): @@ -223,16 +222,6 @@ class GrossProfitGenerator(object): self.get_average_rate_based_on_group_by() def get_average_rate_based_on_group_by(self): - # sum buying / selling totals for group - self.totals = frappe._dict( - qty=0, - base_amount=0, - buying_amount=0, - gross_profit=0, - gross_profit_percent=0, - base_rate=0, - buying_rate=0 - ) for key in list(self.grouped): if self.filters.get("group_by") != "Invoice": for i, row in enumerate(self.grouped[key]): @@ -244,7 +233,6 @@ class GrossProfitGenerator(object): new_row.base_amount += flt(row.base_amount, self.currency_precision) new_row = self.set_average_rate(new_row) self.grouped_data.append(new_row) - self.add_to_totals(new_row) else: for i, row in enumerate(self.grouped[key]): if row.indent == 1.0: @@ -258,17 +246,6 @@ class GrossProfitGenerator(object): if (flt(row.qty) or row.base_amount): row = self.set_average_rate(row) self.grouped_data.append(row) - self.add_to_totals(row) - - self.set_average_gross_profit(self.totals) - - if self.filters.get("group_by") == "Invoice": - self.totals.indent = 0.0 - self.totals.parent_invoice = "" - self.totals.invoice_or_item = "Total" - self.si_list.append(self.totals) - else: - self.grouped_data.append(self.totals) def is_not_invoice_row(self, row): return (self.filters.get("group_by") == "Invoice" and row.indent != 0.0) or self.filters.get("group_by") != "Invoice" @@ -284,11 +261,6 @@ class GrossProfitGenerator(object): new_row.gross_profit_percent = flt(((new_row.gross_profit / new_row.base_amount) * 100.0), self.currency_precision) \ if new_row.base_amount else 0 - def add_to_totals(self, new_row): - for key in self.totals: - if new_row.get(key): - self.totals[key] += new_row[key] - def get_returned_invoice_items(self): returned_invoices = frappe.db.sql(""" select @@ -389,7 +361,7 @@ class GrossProfitGenerator(object): if row.project: query.where( - purchase_invoice_item.item_code == row.project + purchase_invoice_item.project == row.project ) if row.cost_center: From 07bcbc6c7e10f977bc5a6ff8f5b48d91ec9b2b70 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Sat, 12 Feb 2022 19:05:03 +0530 Subject: [PATCH 03/44] fix: Remove unused param --- erpnext/accounts/report/gross_profit/gross_profit.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/report/gross_profit/gross_profit.py b/erpnext/accounts/report/gross_profit/gross_profit.py index c403b76f876..ebb929aaacb 100644 --- a/erpnext/accounts/report/gross_profit/gross_profit.py +++ b/erpnext/accounts/report/gross_profit/gross_profit.py @@ -172,7 +172,7 @@ class GrossProfitGenerator(object): buying_amount = 0 for row in reversed(self.si_list): - if self.skip_row(row, self.product_bundles): + if self.skip_row(row): continue row.base_amount = flt(row.base_net_amount, self.currency_precision) @@ -278,7 +278,7 @@ class GrossProfitGenerator(object): self.returned_invoices.setdefault(inv.return_against, frappe._dict())\ .setdefault(inv.item_code, []).append(inv) - def skip_row(self, row, product_bundles): + def skip_row(self, row): if self.filters.get("group_by") != "Invoice": if not row.get(scrub(self.filters.get("group_by", ""))): return True From 973f6b1bbd53594e5b2a51a1dcdf7d9e38dd46a8 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Mon, 14 Feb 2022 22:14:17 +0530 Subject: [PATCH 04/44] fix: Gross profit for credit notes --- erpnext/accounts/report/gross_profit/gross_profit.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/report/gross_profit/gross_profit.py b/erpnext/accounts/report/gross_profit/gross_profit.py index ebb929aaacb..b03bb9bb13f 100644 --- a/erpnext/accounts/report/gross_profit/gross_profit.py +++ b/erpnext/accounts/report/gross_profit/gross_profit.py @@ -282,8 +282,8 @@ class GrossProfitGenerator(object): if self.filters.get("group_by") != "Invoice": if not row.get(scrub(self.filters.get("group_by", ""))): return True - elif row.get("is_return") == 1: - return True + + return False def get_buying_amount_from_product_bundle(self, row, product_bundle): buying_amount = 0.0 From 555b1335f65cca4f77c28294e153002a39e114a4 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Thu, 17 Feb 2022 19:15:30 +0530 Subject: [PATCH 05/44] feat: Bank Reconciliation for loan documents --- .../bank_reconciliation_tool.py | 73 ++++++++++++++++++- .../loan_disbursement/loan_disbursement.json | 47 ++++++++++-- .../loan_disbursement/loan_disbursement.py | 12 +-- .../loan_repayment/loan_repayment.json | 52 ++++++++++++- .../doctype/loan_repayment/loan_repayment.py | 24 +++--- .../dialog_manager.js | 17 ++++- 6 files changed, 190 insertions(+), 35 deletions(-) diff --git a/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py b/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py index 4211bd0169d..26078d63298 100644 --- a/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py +++ b/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py @@ -275,6 +275,10 @@ def check_matching(bank_account, company, transaction, document_types): } matching_vouchers = [] + + matching_vouchers.extend(get_loan_vouchers(bank_account, transaction, + document_types, filters)) + for query in subquery: matching_vouchers.extend( frappe.db.sql(query, filters,) @@ -311,6 +315,74 @@ def get_queries(bank_account, company, transaction, document_types): return queries +def get_loan_vouchers(bank_account, transaction, document_types, filters): + vouchers = [] + amount_condition = True if "exact_match" in document_types else False + + if transaction.withdrawal > 0 and "loan_disbursement" in document_types: + vouchers.append(get_ld_matching_query(bank_account, amount_condition, filters)) + + if transaction.deposit > 0 and "loan_repayment" in document_types: + vouchers.append(get_lr_matching_query(bank_account, amount_condition, filters)) + +def get_ld_matching_query(bank_account, amount_condition, filters): + loan_disbursement = frappe.qb.DocType("Loan Disbursement") + query = frappe.qb.from_(loan_disbursement).select( + loan_disbursement.name, + loan_disbursement.disbursed_amount, + loan_disbursement.reference_number, + loan_disbursement.reference_date, + loan_disbursement.applicant_type, + loan_disbursement.disbursement_date + ).where( + loan_disbursement.docstatus == 1 + ).where( + loan_disbursement.clearance_date.isnull() + ).where( + loan_disbursement.disbursement_account == bank_account + ) + + if amount_condition: + query.where( + loan_disbursement.disbursed_amount == filters.get('amount') + ) + else: + query.where( + loan_disbursement.disbursed_amount <= filters.get('amount') + ) + + vouchers = query.run(as_dict=1) + return vouchers + +def get_lr_matching_query(bank_account, amount_condition, filters): + loan_repayment = frappe.qb.DocType("Loan Repayment") + query = frappe.qb.from_(loan_repayment).select( + loan_repayment.name, + loan_repayment.paid_amount, + loan_repayment.reference_number, + loan_repayment.reference_date, + loan_repayment.applicant_type, + loan_repayment.posting_date + ).where( + loan_repayment.docstatus == 1 + ).where( + loan_repayment.clearance_date.isnull() + ).where( + loan_repayment.disbursement_account == bank_account + ) + + if amount_condition: + query.where( + loan_repayment.paid_amount == filters.get('amount') + ) + else: + query.where( + loan_repayment.paid_amount <= filters.get('amount') + ) + + vouchers = query.run(as_dict=1) + return vouchers + def get_pe_matching_query(amount_condition, account_from_to, transaction): # get matching payment entries query if transaction.deposit > 0: @@ -348,7 +420,6 @@ def get_je_matching_query(amount_condition, transaction): # We have mapping at the bank level # So one bank could have both types of bank accounts like asset and liability # So cr_or_dr should be judged only on basis of withdrawal and deposit and not account type - company_account = frappe.get_value("Bank Account", transaction.bank_account, "account") cr_or_dr = "credit" if transaction.withdrawal > 0 else "debit" return f""" diff --git a/erpnext/loan_management/doctype/loan_disbursement/loan_disbursement.json b/erpnext/loan_management/doctype/loan_disbursement/loan_disbursement.json index 7811d56a758..50926d77268 100644 --- a/erpnext/loan_management/doctype/loan_disbursement/loan_disbursement.json +++ b/erpnext/loan_management/doctype/loan_disbursement/loan_disbursement.json @@ -14,11 +14,15 @@ "applicant", "section_break_7", "disbursement_date", + "clearance_date", "column_break_8", "disbursed_amount", "accounting_dimensions_section", "cost_center", - "customer_details_section", + "accounting_details", + "disbursement_account", + "column_break_16", + "loan_account", "bank_account", "disbursement_references_section", "reference_date", @@ -106,11 +110,6 @@ "fieldtype": "Section Break", "label": "Disbursement Details" }, - { - "fieldname": "customer_details_section", - "fieldtype": "Section Break", - "label": "Customer Details" - }, { "fetch_from": "against_loan.applicant_type", "fieldname": "applicant_type", @@ -149,15 +148,48 @@ "fieldname": "reference_number", "fieldtype": "Data", "label": "Reference Number" + }, + { + "fieldname": "clearance_date", + "fieldtype": "Date", + "label": "Clearance Date", + "no_copy": 1, + "read_only": 1 + }, + { + "fieldname": "accounting_details", + "fieldtype": "Section Break", + "label": "Accounting Details" + }, + { + "fetch_from": "against_loan.disbursement_account", + "fieldname": "disbursement_account", + "fieldtype": "Link", + "label": "Disbursement Account", + "options": "Account", + "read_only": 1 + }, + { + "fieldname": "column_break_16", + "fieldtype": "Column Break" + }, + { + "fetch_from": "against_loan.loan_account", + "fieldname": "loan_account", + "fieldtype": "Link", + "label": "Loan Account", + "options": "Account", + "read_only": 1 } ], "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], - "modified": "2021-04-19 18:09:32.175355", + "modified": "2022-02-17 18:23:44.157598", "modified_by": "Administrator", "module": "Loan Management", "name": "Loan Disbursement", + "naming_rule": "Expression (old style)", "owner": "Administrator", "permissions": [ { @@ -194,5 +226,6 @@ "quick_entry": 1, "sort_field": "modified", "sort_order": "DESC", + "states": [], "track_changes": 1 } \ No newline at end of file diff --git a/erpnext/loan_management/doctype/loan_disbursement/loan_disbursement.py b/erpnext/loan_management/doctype/loan_disbursement/loan_disbursement.py index df3aadfb18d..54a03b92b5e 100644 --- a/erpnext/loan_management/doctype/loan_disbursement/loan_disbursement.py +++ b/erpnext/loan_management/doctype/loan_disbursement/loan_disbursement.py @@ -42,9 +42,6 @@ class LoanDisbursement(AccountsController): if not self.posting_date: self.posting_date = self.disbursement_date or nowdate() - if not self.bank_account and self.applicant_type == "Customer": - self.bank_account = frappe.db.get_value("Customer", self.applicant, "default_bank_account") - def validate_disbursal_amount(self): possible_disbursal_amount = get_disbursal_amount(self.against_loan) @@ -117,12 +114,11 @@ class LoanDisbursement(AccountsController): def make_gl_entries(self, cancel=0, adv_adj=0): gle_map = [] - loan_details = frappe.get_doc("Loan", self.against_loan) gle_map.append( self.get_gl_dict({ - "account": loan_details.loan_account, - "against": loan_details.disbursement_account, + "account": self.loan_account, + "against": self.disbursement_account, "debit": self.disbursed_amount, "debit_in_account_currency": self.disbursed_amount, "against_voucher_type": "Loan", @@ -137,8 +133,8 @@ class LoanDisbursement(AccountsController): gle_map.append( self.get_gl_dict({ - "account": loan_details.disbursement_account, - "against": loan_details.loan_account, + "account": self.disbursement_account, + "against": self.loan_account, "credit": self.disbursed_amount, "credit_in_account_currency": self.disbursed_amount, "against_voucher_type": "Loan", diff --git a/erpnext/loan_management/doctype/loan_repayment/loan_repayment.json b/erpnext/loan_management/doctype/loan_repayment/loan_repayment.json index 93ef2170420..766602de866 100644 --- a/erpnext/loan_management/doctype/loan_repayment/loan_repayment.json +++ b/erpnext/loan_management/doctype/loan_repayment/loan_repayment.json @@ -1,7 +1,7 @@ { "actions": [], "autoname": "LM-REP-.####", - "creation": "2019-09-03 14:44:39.977266", + "creation": "2022-01-25 10:30:02.767941", "doctype": "DocType", "editable_grid": 1, "engine": "InnoDB", @@ -13,6 +13,7 @@ "column_break_3", "company", "posting_date", + "clearance_date", "rate_of_interest", "payroll_payable_account", "is_term_loan", @@ -37,7 +38,12 @@ "total_penalty_paid", "total_interest_paid", "repayment_details", - "amended_from" + "amended_from", + "accounting_details_section", + "repayment_account", + "penalty_income_account", + "column_break_36", + "loan_account" ], "fields": [ { @@ -260,12 +266,52 @@ "fieldname": "repay_from_salary", "fieldtype": "Check", "label": "Repay From Salary" + }, + { + "fieldname": "clearance_date", + "fieldtype": "Date", + "label": "Clearance Date", + "no_copy": 1, + "read_only": 1 + }, + { + "fieldname": "accounting_details_section", + "fieldtype": "Section Break", + "label": "Accounting Details" + }, + { + "fetch_from": "against_loan.payment_account", + "fieldname": "repayment_account", + "fieldtype": "Link", + "label": "Repayment Account", + "options": "Account", + "read_only": 1 + }, + { + "fieldname": "column_break_36", + "fieldtype": "Column Break" + }, + { + "fetch_from": "against_loan.loan_account", + "fieldname": "loan_account", + "fieldtype": "Link", + "label": "Loan Account", + "options": "Account", + "read_only": 1 + }, + { + "fetch_from": "against_loan.penalty_income_account", + "fieldname": "penalty_income_account", + "fieldtype": "Link", + "hidden": 1, + "label": "Penalty Income Account", + "options": "Account" } ], "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], - "modified": "2022-01-06 01:51:06.707782", + "modified": "2022-02-17 19:10:07.742298", "modified_by": "Administrator", "module": "Loan Management", "name": "Loan Repayment", diff --git a/erpnext/loan_management/doctype/loan_repayment/loan_repayment.py b/erpnext/loan_management/doctype/loan_repayment/loan_repayment.py index f3ed6112556..67c2b1ee14d 100644 --- a/erpnext/loan_management/doctype/loan_repayment/loan_repayment.py +++ b/erpnext/loan_management/doctype/loan_repayment/loan_repayment.py @@ -310,7 +310,6 @@ class LoanRepayment(AccountsController): def make_gl_entries(self, cancel=0, adv_adj=0): gle_map = [] - loan_details = frappe.get_doc("Loan", self.against_loan) if self.shortfall_amount and self.amount_paid > self.shortfall_amount: remarks = _("Shortfall Repayment of {0}.\nRepayment against Loan: {1}").format(self.shortfall_amount, @@ -323,13 +322,13 @@ class LoanRepayment(AccountsController): if self.repay_from_salary: payment_account = self.payroll_payable_account else: - payment_account = loan_details.payment_account + payment_account = self.payment_account if self.total_penalty_paid: gle_map.append( self.get_gl_dict({ - "account": loan_details.loan_account, - "against": loan_details.payment_account, + "account": self.loan_account, + "against": payment_account, "debit": self.total_penalty_paid, "debit_in_account_currency": self.total_penalty_paid, "against_voucher_type": "Loan", @@ -344,8 +343,8 @@ class LoanRepayment(AccountsController): gle_map.append( self.get_gl_dict({ - "account": loan_details.penalty_income_account, - "against": loan_details.loan_account, + "account": self.penalty_income_account, + "against": self.loan_account, "credit": self.total_penalty_paid, "credit_in_account_currency": self.total_penalty_paid, "against_voucher_type": "Loan", @@ -359,8 +358,7 @@ class LoanRepayment(AccountsController): gle_map.append( self.get_gl_dict({ "account": payment_account, - "against": loan_details.loan_account + ", " + loan_details.interest_income_account - + ", " + loan_details.penalty_income_account, + "against": self.loan_account + ", " + self.penalty_income_account, "debit": self.amount_paid, "debit_in_account_currency": self.amount_paid, "against_voucher_type": "Loan", @@ -368,16 +366,16 @@ class LoanRepayment(AccountsController): "remarks": remarks, "cost_center": self.cost_center, "posting_date": getdate(self.posting_date), - "party_type": loan_details.applicant_type if self.repay_from_salary else '', - "party": loan_details.applicant if self.repay_from_salary else '' + "party_type": self.applicant_type if self.repay_from_salary else '', + "party": self.applicant if self.repay_from_salary else '' }) ) gle_map.append( self.get_gl_dict({ - "account": loan_details.loan_account, - "party_type": loan_details.applicant_type, - "party": loan_details.applicant, + "account": self.loan_account, + "party_type": self.applicant_type, + "party": self.applicant, "against": payment_account, "credit": self.amount_paid, "credit_in_account_currency": self.amount_paid, diff --git a/erpnext/public/js/bank_reconciliation_tool/dialog_manager.js b/erpnext/public/js/bank_reconciliation_tool/dialog_manager.js index ca73393c546..214a1be1344 100644 --- a/erpnext/public/js/bank_reconciliation_tool/dialog_manager.js +++ b/erpnext/public/js/bank_reconciliation_tool/dialog_manager.js @@ -181,6 +181,12 @@ erpnext.accounts.bank_reconciliation.DialogManager = class DialogManager { fieldname: "journal_entry", onchange: () => this.update_options(), }, + { + fieldtype: "Check", + label: "Loan Repayment", + fieldname: "loan_repayment", + onchange: () => this.update_options(), + }, { fieldname: "column_break_5", fieldtype: "Column Break", @@ -191,13 +197,18 @@ erpnext.accounts.bank_reconciliation.DialogManager = class DialogManager { fieldname: "sales_invoice", onchange: () => this.update_options(), }, - { fieldtype: "Check", label: "Purchase Invoice", fieldname: "purchase_invoice", onchange: () => this.update_options(), }, + { + fieldtype: "Check", + label: "Show Only Exact Amount", + fieldname: "exact_match", + onchange: () => this.update_options(), + }, { fieldname: "column_break_5", fieldtype: "Column Break", @@ -210,8 +221,8 @@ erpnext.accounts.bank_reconciliation.DialogManager = class DialogManager { }, { fieldtype: "Check", - label: "Show Only Exact Amount", - fieldname: "exact_match", + label: "Loan Disbursement", + fieldname: "loan_disbursement", onchange: () => this.update_options(), }, { From a0bdcbd0cd551895af63955343f517051917c8eb Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Mon, 21 Feb 2022 11:44:00 +0530 Subject: [PATCH 06/44] fix: Add patch for account fields --- erpnext/patches.txt | 1 + .../v13_0/update_accounts_in_loan_docs.py | 37 +++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 erpnext/patches/v13_0/update_accounts_in_loan_docs.py diff --git a/erpnext/patches.txt b/erpnext/patches.txt index d104bc003c8..b24bf0a7e0a 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -352,3 +352,4 @@ erpnext.patches.v13_0.shopping_cart_to_ecommerce erpnext.patches.v13_0.update_disbursement_account erpnext.patches.v13_0.update_reserved_qty_closed_wo erpnext.patches.v14_0.delete_amazon_mws_doctype +erpnext.patches.v13_0.update_accounts_in_loan_docs diff --git a/erpnext/patches/v13_0/update_accounts_in_loan_docs.py b/erpnext/patches/v13_0/update_accounts_in_loan_docs.py new file mode 100644 index 00000000000..440f912be21 --- /dev/null +++ b/erpnext/patches/v13_0/update_accounts_in_loan_docs.py @@ -0,0 +1,37 @@ +import frappe + + +def execute(): + ld = frappe.qb.DocType("Loan Disbursement").as_("ld") + lr = frappe.qb.DocType("Loan Repayment").as_("lr") + loan = frappe.qb.DocType("Loan") + + frappe.qb.update( + ld + ).inner_join( + loan + ).on( + loan.name == ld.against_loan + ).set( + ld.disbursement_account, loan.disbursement_account + ).set( + ld.loan_account, loan.loan_account + ).where( + ld.docstatus < 2 + ).run() + + frappe.qb.update( + lr + ).inner_join( + loan + ).on( + loan.name == lr.against_loan + ).set( + lr.payment_account, loan.payment_account + ).set( + lr.loan_account, loan.loan_account + ).set( + lr.penalty_income_account, loan.penalty_income_account + ).where( + lr.docstatus < 2 + ).run() From 295cbb0ff22b04c705148d727d96f70b836fee93 Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Mon, 21 Feb 2022 11:45:23 +0530 Subject: [PATCH 07/44] fix: Update queries in Bank Reconciliation Tool --- .../bank_reconciliation_tool.py | 57 ++++++++++++++++--- .../bank_transaction/bank_transaction.py | 13 ++++- .../loan_repayment/loan_repayment.json | 6 +- 3 files changed, 63 insertions(+), 13 deletions(-) diff --git a/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py b/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py index 26078d63298..f3351ddcba4 100644 --- a/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py +++ b/erpnext/accounts/doctype/bank_reconciliation_tool/bank_reconciliation_tool.py @@ -7,6 +7,7 @@ import json import frappe from frappe import _ from frappe.model.document import Document +from frappe.query_builder.custom import ConstantColumn from frappe.utils import flt from erpnext import get_company_currency @@ -320,14 +321,34 @@ def get_loan_vouchers(bank_account, transaction, document_types, filters): amount_condition = True if "exact_match" in document_types else False if transaction.withdrawal > 0 and "loan_disbursement" in document_types: - vouchers.append(get_ld_matching_query(bank_account, amount_condition, filters)) + vouchers.extend(get_ld_matching_query(bank_account, amount_condition, filters)) if transaction.deposit > 0 and "loan_repayment" in document_types: - vouchers.append(get_lr_matching_query(bank_account, amount_condition, filters)) + vouchers.extend(get_lr_matching_query(bank_account, amount_condition, filters)) + + return vouchers def get_ld_matching_query(bank_account, amount_condition, filters): loan_disbursement = frappe.qb.DocType("Loan Disbursement") + matching_reference = loan_disbursement.reference_number == filters.get("reference_number") + matching_party = loan_disbursement.applicant_type == filters.get("party_type") and \ + loan_disbursement.applicant == filters.get("party") + + rank = ( + frappe.qb.terms.Case() + .when(matching_reference, 1) + .else_(0) + ) + + rank1 = ( + frappe.qb.terms.Case() + .when(matching_party, 1) + .else_(0) + ) + query = frappe.qb.from_(loan_disbursement).select( + rank + rank1 + 1, + ConstantColumn("Loan Disbursement").as_("doctype"), loan_disbursement.name, loan_disbursement.disbursed_amount, loan_disbursement.reference_number, @@ -351,14 +372,33 @@ def get_ld_matching_query(bank_account, amount_condition, filters): loan_disbursement.disbursed_amount <= filters.get('amount') ) - vouchers = query.run(as_dict=1) + vouchers = query.run(as_list=True) + return vouchers def get_lr_matching_query(bank_account, amount_condition, filters): loan_repayment = frappe.qb.DocType("Loan Repayment") + matching_reference = loan_repayment.reference_number == filters.get("reference_number") + matching_party = loan_repayment.applicant_type == filters.get("party_type") and \ + loan_repayment.applicant == filters.get("party") + + rank = ( + frappe.qb.terms.Case() + .when(matching_reference, 1) + .else_(0) + ) + + rank1 = ( + frappe.qb.terms.Case() + .when(matching_party, 1) + .else_(0) + ) + query = frappe.qb.from_(loan_repayment).select( + rank + rank1 + 1, + ConstantColumn("Loan Repayment").as_("doctype"), loan_repayment.name, - loan_repayment.paid_amount, + loan_repayment.amount_paid, loan_repayment.reference_number, loan_repayment.reference_date, loan_repayment.applicant_type, @@ -368,19 +408,20 @@ def get_lr_matching_query(bank_account, amount_condition, filters): ).where( loan_repayment.clearance_date.isnull() ).where( - loan_repayment.disbursement_account == bank_account + loan_repayment.payment_account == bank_account ) if amount_condition: query.where( - loan_repayment.paid_amount == filters.get('amount') + loan_repayment.amount_paid == filters.get('amount') ) else: query.where( - loan_repayment.paid_amount <= filters.get('amount') + loan_repayment.amount_paid <= filters.get('amount') ) - vouchers = query.run(as_dict=1) + vouchers = query.run() + return vouchers def get_pe_matching_query(amount_condition, account_from_to, transaction): diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py index 51e1d6e9a00..da944fa4cee 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py @@ -49,7 +49,8 @@ class BankTransaction(StatusUpdater): def clear_linked_payment_entries(self, for_cancel=False): for payment_entry in self.payment_entries: - if payment_entry.payment_document in ["Payment Entry", "Journal Entry", "Purchase Invoice", "Expense Claim"]: + if payment_entry.payment_document in ["Payment Entry", "Journal Entry", "Purchase Invoice", "Expense Claim", "Loan Repayment", + "Loan Disbursement"]: self.clear_simple_entry(payment_entry, for_cancel=for_cancel) elif payment_entry.payment_document == "Sales Invoice": @@ -104,6 +105,7 @@ def get_total_allocated_amount(payment_entry): bt.docstatus = 1""", (payment_entry.payment_document, payment_entry.payment_entry), as_dict=True) def get_paid_amount(payment_entry, currency, bank_account): + print(payment_entry.payment_document, "#@#@#@") if payment_entry.payment_document in ["Payment Entry", "Sales Invoice", "Purchase Invoice"]: paid_amount_field = "paid_amount" @@ -116,11 +118,18 @@ def get_paid_amount(payment_entry, currency, bank_account): payment_entry.payment_entry, paid_amount_field) elif payment_entry.payment_document == "Journal Entry": - return frappe.db.get_value('Journal Entry Account', {'parent': payment_entry.payment_entry, 'account': bank_account}, "sum(credit_in_account_currency)") + return frappe.db.get_value('Journal Entry Account', {'parent': payment_entry.payment_entry, 'account': bank_account}, + "sum(credit_in_account_currency)") elif payment_entry.payment_document == "Expense Claim": return frappe.db.get_value(payment_entry.payment_document, payment_entry.payment_entry, "total_amount_reimbursed") + elif payment_entry.payment_document == "Loan Disbursement": + return frappe.db.get_value(payment_entry.payment_document, payment_entry.payment_entry, "disbursed_amount") + + elif payment_entry.payment_document == "Loan Repayment": + return frappe.db.get_value(payment_entry.payment_document, payment_entry.payment_entry, "amount_paid") + else: frappe.throw("Please reconcile {0}: {1} manually".format(payment_entry.payment_document, payment_entry.payment_entry)) diff --git a/erpnext/loan_management/doctype/loan_repayment/loan_repayment.json b/erpnext/loan_management/doctype/loan_repayment/loan_repayment.json index 766602de866..480e010b49a 100644 --- a/erpnext/loan_management/doctype/loan_repayment/loan_repayment.json +++ b/erpnext/loan_management/doctype/loan_repayment/loan_repayment.json @@ -40,7 +40,7 @@ "repayment_details", "amended_from", "accounting_details_section", - "repayment_account", + "payment_account", "penalty_income_account", "column_break_36", "loan_account" @@ -281,7 +281,7 @@ }, { "fetch_from": "against_loan.payment_account", - "fieldname": "repayment_account", + "fieldname": "payment_account", "fieldtype": "Link", "label": "Repayment Account", "options": "Account", @@ -311,7 +311,7 @@ "index_web_pages_for_search": 1, "is_submittable": 1, "links": [], - "modified": "2022-02-17 19:10:07.742298", + "modified": "2022-02-18 19:10:07.742298", "modified_by": "Administrator", "module": "Loan Management", "name": "Loan Repayment", From 0b5e618e3ab206f7ae080f570a736a87fcbccf2d Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Mon, 21 Feb 2022 11:46:44 +0530 Subject: [PATCH 08/44] fix: Update bank reconciliation statement --- .../bank_reconciliation_statement.py | 105 ++++++++++++++++-- 1 file changed, 95 insertions(+), 10 deletions(-) diff --git a/erpnext/accounts/report/bank_reconciliation_statement/bank_reconciliation_statement.py b/erpnext/accounts/report/bank_reconciliation_statement/bank_reconciliation_statement.py index 6c401fb8f3b..b72d2669775 100644 --- a/erpnext/accounts/report/bank_reconciliation_statement/bank_reconciliation_statement.py +++ b/erpnext/accounts/report/bank_reconciliation_statement/bank_reconciliation_statement.py @@ -4,7 +4,12 @@ import frappe from frappe import _ -from frappe.utils import flt, getdate, nowdate +from frappe.query_builder.custom import ConstantColumn +from frappe.query_builder.functions import Sum +from frappe.utils import flt, getdate +from pypika import CustomFunction + +from erpnext.accounts.utils import get_balance_on def execute(filters=None): @@ -18,7 +23,6 @@ def execute(filters=None): data = get_entries(filters) - from erpnext.accounts.utils import get_balance_on balance_as_per_system = get_balance_on(filters["account"], filters["report_date"]) total_debit, total_credit = 0,0 @@ -118,7 +122,21 @@ def get_columns(): ] def get_entries(filters): - journal_entries = frappe.db.sql(""" + journal_entries = get_journal_entries(filters) + + payment_entries = get_payment_entries(filters) + + loan_entries = get_loan_entries(filters) + + pos_entries = [] + if filters.include_pos_transactions: + pos_entries = get_pos_entries(filters) + + return sorted(list(payment_entries)+list(journal_entries+list(pos_entries) + list(loan_entries)), + key=lambda k: getdate(k['posting_date'])) + +def get_journal_entries(filters): + return frappe.db.sql(""" select "Journal Entry" as payment_document, jv.posting_date, jv.name as payment_entry, jvd.debit_in_account_currency as debit, jvd.credit_in_account_currency as credit, jvd.against_account, @@ -130,7 +148,8 @@ def get_entries(filters): and ifnull(jv.clearance_date, '4000-01-01') > %(report_date)s and ifnull(jv.is_opening, 'No') = 'No'""", filters, as_dict=1) - payment_entries = frappe.db.sql(""" +def get_payment_entries(filters): + return frappe.db.sql(""" select "Payment Entry" as payment_document, name as payment_entry, reference_no, reference_date as ref_date, @@ -145,9 +164,8 @@ def get_entries(filters): and ifnull(clearance_date, '4000-01-01') > %(report_date)s """, filters, as_dict=1) - pos_entries = [] - if filters.include_pos_transactions: - pos_entries = frappe.db.sql(""" +def get_pos_entries(filters): + return frappe.db.sql(""" select "Sales Invoice Payment" as payment_document, sip.name as payment_entry, sip.amount as debit, si.posting_date, si.debit_to as against_account, sip.clearance_date, @@ -161,8 +179,42 @@ def get_entries(filters): si.posting_date ASC, si.name DESC """, filters, as_dict=1) - return sorted(list(payment_entries)+list(journal_entries+list(pos_entries)), - key=lambda k: k['posting_date'] or getdate(nowdate())) +def get_loan_entries(filters): + loan_docs = [] + for doctype in ["Loan Disbursement", "Loan Repayment"]: + loan_doc = frappe.qb.DocType(doctype) + ifnull = CustomFunction('IFNULL', ['value', 'default']) + + if doctype == "Loan Disbursement": + amount_field = (loan_doc.disbursed_amount).as_("credit") + posting_date = (loan_doc.disbursement_date).as_("posting_date") + account = loan_doc.disbursement_account + else: + amount_field = (loan_doc.amount_paid).as_("debit") + posting_date = (loan_doc.posting_date).as_("posting_date") + account = loan_doc.payment_account + + entries = frappe.qb.from_(loan_doc).select( + ConstantColumn(doctype).as_("payment_document"), + (loan_doc.name).as_("payment_entry"), + (loan_doc.reference_number).as_("reference_no"), + (loan_doc.reference_date).as_("ref_date"), + amount_field, + posting_date, + ).where( + loan_doc.docstatus == 1 + ).where( + account == filters.get('account') + ).where( + posting_date <= getdate(filters.get('report_date')) + ).where( + ifnull(loan_doc.clearance_date, '4000-01-01') > getdate(filters.get('report_date')) + ).run(as_dict=1) + + loan_docs.extend(entries) + + return loan_docs + def get_amounts_not_reflected_in_system(filters): je_amount = frappe.db.sql(""" @@ -182,7 +234,40 @@ def get_amounts_not_reflected_in_system(filters): pe_amount = flt(pe_amount[0][0]) if pe_amount else 0.0 - return je_amount + pe_amount + loan_amount = get_loan_amount(filters) + + return je_amount + pe_amount + loan_amount + +def get_loan_amount(filters): + total_amount = 0 + for doctype in ["Loan Disbursement", "Loan Repayment"]: + loan_doc = frappe.qb.DocType(doctype) + ifnull = CustomFunction('IFNULL', ['value', 'default']) + + if doctype == "Loan Disbursement": + amount_field = Sum(loan_doc.disbursed_amount) + posting_date = (loan_doc.disbursement_date).as_("posting_date") + account = loan_doc.disbursement_account + else: + amount_field = Sum(loan_doc.amount_paid) + posting_date = (loan_doc.posting_date).as_("posting_date") + account = loan_doc.payment_account + + amount = frappe.qb.from_(loan_doc).select( + amount_field + ).where( + loan_doc.docstatus == 1 + ).where( + account == filters.get('account') + ).where( + posting_date > getdate(filters.get('report_date')) + ).where( + ifnull(loan_doc.clearance_date, '4000-01-01') <= getdate(filters.get('report_date')) + ).run()[0][0] + + total_amount += flt(amount) + + return amount def get_balance_row(label, amount, account_currency): if amount > 0: From c5808543c83ea43f62784331fb7c513543e454f0 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Mon, 21 Feb 2022 12:41:08 +0530 Subject: [PATCH 09/44] fix(asset): no. of depreciation booked cannot be equal to total no. of depreciations --- erpnext/assets/doctype/asset/asset.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/erpnext/assets/doctype/asset/asset.py b/erpnext/assets/doctype/asset/asset.py index 6e87426ccbe..ea473fa7bb5 100644 --- a/erpnext/assets/doctype/asset/asset.py +++ b/erpnext/assets/doctype/asset/asset.py @@ -417,11 +417,12 @@ class Asset(AccountsController): def validate_asset_finance_books(self, row): if flt(row.expected_value_after_useful_life) >= flt(self.gross_purchase_amount): frappe.throw(_("Row {0}: Expected Value After Useful Life must be less than Gross Purchase Amount") - .format(row.idx)) + .format(row.idx), title=_("Invalid Schedule")) if not row.depreciation_start_date: if not self.available_for_use_date: - frappe.throw(_("Row {0}: Depreciation Start Date is required").format(row.idx)) + frappe.throw(_("Row {0}: Depreciation Start Date is required") + .format(row.idx), title=_("Invalid Schedule")) row.depreciation_start_date = get_last_day(self.available_for_use_date) if not self.is_existing_asset: @@ -439,8 +440,9 @@ class Asset(AccountsController): else: self.number_of_depreciations_booked = 0 - if cint(self.number_of_depreciations_booked) > cint(row.total_number_of_depreciations): - frappe.throw(_("Number of Depreciations Booked cannot be greater than Total Number of Depreciations")) + if flt(row.total_number_of_depreciations) <= cint(self.number_of_depreciations_booked): + frappe.throw(_("Row {0}: Total Number of Depreciations cannot be less than or equal to Number of Depreciations Booked") + .format(row.idx), title=_("Invalid Schedule")) if row.depreciation_start_date and getdate(row.depreciation_start_date) < getdate(self.purchase_date): frappe.throw(_("Depreciation Row {0}: Next Depreciation Date cannot be before Purchase Date") From 780694f6e2d686ca7d037556a52e097802814266 Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Mon, 21 Feb 2022 12:45:52 +0530 Subject: [PATCH 10/44] test: number_of_depr_booked = total_number_of_depr --- erpnext/assets/doctype/asset/test_asset.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/erpnext/assets/doctype/asset/test_asset.py b/erpnext/assets/doctype/asset/test_asset.py index c08dc21a8fe..ddbff89fc72 100644 --- a/erpnext/assets/doctype/asset/test_asset.py +++ b/erpnext/assets/doctype/asset/test_asset.py @@ -873,8 +873,9 @@ class TestDepreciationBasics(AssetSetup): self.assertRaises(frappe.ValidationError, asset.save) def test_number_of_depreciations(self): - """Tests if an error is raised when number_of_depreciations_booked > total_number_of_depreciations.""" + """Tests if an error is raised when number_of_depreciations_booked >= total_number_of_depreciations.""" + # number_of_depreciations_booked > total_number_of_depreciations asset = create_asset( item_code = "Macbook Pro", calculate_depreciation = 1, @@ -889,6 +890,21 @@ class TestDepreciationBasics(AssetSetup): self.assertRaises(frappe.ValidationError, asset.save) + # number_of_depreciations_booked = total_number_of_depreciations + asset_2 = create_asset( + item_code = "Macbook Pro", + calculate_depreciation = 1, + available_for_use_date = "2019-12-31", + total_number_of_depreciations = 5, + expected_value_after_useful_life = 10000, + depreciation_start_date = "2020-07-01", + opening_accumulated_depreciation = 10000, + number_of_depreciations_booked = 5, + do_not_save = 1 + ) + + self.assertRaises(frappe.ValidationError, asset_2.save) + def test_depreciation_start_date_is_before_purchase_date(self): asset = create_asset( item_code = "Macbook Pro", From a4c6cb9f12f0ff931909a15b657b62a4bc85a20b Mon Sep 17 00:00:00 2001 From: Deepesh Garg Date: Mon, 21 Feb 2022 17:08:25 +0530 Subject: [PATCH 11/44] fix: Remove print statements --- erpnext/accounts/doctype/bank_transaction/bank_transaction.py | 1 - 1 file changed, 1 deletion(-) diff --git a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py index da944fa4cee..a476cab55f7 100644 --- a/erpnext/accounts/doctype/bank_transaction/bank_transaction.py +++ b/erpnext/accounts/doctype/bank_transaction/bank_transaction.py @@ -105,7 +105,6 @@ def get_total_allocated_amount(payment_entry): bt.docstatus = 1""", (payment_entry.payment_document, payment_entry.payment_entry), as_dict=True) def get_paid_amount(payment_entry, currency, bank_account): - print(payment_entry.payment_document, "#@#@#@") if payment_entry.payment_document in ["Payment Entry", "Sales Invoice", "Purchase Invoice"]: paid_amount_field = "paid_amount" From 00e8565868e3bb8a1547abeedd2d158a9b7e5bf4 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 21 Feb 2022 17:41:23 +0530 Subject: [PATCH 12/44] fix: round off increments in numeric item variant --- erpnext/stock/doctype/item/item.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/stock/doctype/item/item.js b/erpnext/stock/doctype/item/item.js index dfc09181cab..ffea9c2d6e0 100644 --- a/erpnext/stock/doctype/item/item.js +++ b/erpnext/stock/doctype/item/item.js @@ -594,7 +594,7 @@ $.extend(erpnext.item, { const increment = r.message.increment; let values = []; - for(var i = from; i <= to; i += increment) { + for(var i = from; i <= to; i = flt(i + increment, 6)) { values.push(i); } attr_val_fields[d.attribute] = values; From f4af75f60b7bb594df4f9a6e6d0cb1ad949dfa33 Mon Sep 17 00:00:00 2001 From: 18alantom <2.alan.tom@gmail.com> Date: Tue, 15 Feb 2022 11:51:52 +0530 Subject: [PATCH 13/44] feat: batchwise valuation flag This is required to avoid breaking behaviour in valuation of old batches --- erpnext/patches.txt | 1 + .../patches/v14_0/update_batch_valuation_flag.py | 12 ++++++++++++ erpnext/stock/doctype/batch/batch.json | 16 +++++++++++++++- 3 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 erpnext/patches/v14_0/update_batch_valuation_flag.py diff --git a/erpnext/patches.txt b/erpnext/patches.txt index a93ceca437a..52c29b22b9e 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -353,3 +353,4 @@ erpnext.patches.v13_0.update_reserved_qty_closed_wo erpnext.patches.v13_0.update_exchange_rate_settings erpnext.patches.v14_0.delete_amazon_mws_doctype erpnext.patches.v13_0.set_work_order_qty_in_so_from_mr +erpnext.patches.v14_0.update_batch_valuation_flag diff --git a/erpnext/patches/v14_0/update_batch_valuation_flag.py b/erpnext/patches/v14_0/update_batch_valuation_flag.py new file mode 100644 index 00000000000..d9f08d8d97b --- /dev/null +++ b/erpnext/patches/v14_0/update_batch_valuation_flag.py @@ -0,0 +1,12 @@ +import frappe + + +def execute(): + """ + - Don't use batchwise valuation for existing batches. + - Only batches created after this patch shoule use it. + """ + frappe.db.sql(""" + UPDATE `tabBatch` + SET use_batchwise_valuation=0 + """) diff --git a/erpnext/stock/doctype/batch/batch.json b/erpnext/stock/doctype/batch/batch.json index fc4cf1dbdb8..0d28ea09190 100644 --- a/erpnext/stock/doctype/batch/batch.json +++ b/erpnext/stock/doctype/batch/batch.json @@ -9,6 +9,8 @@ "field_order": [ "sb_disabled", "disabled", + "column_break_24", + "use_batchwise_valuation", "sb_batch", "batch_id", "item", @@ -186,6 +188,18 @@ "fieldtype": "Float", "label": "Produced Qty", "read_only": 1 + }, + { + "fieldname": "column_break_24", + "fieldtype": "Column Break" + }, + { + "default": "1", + "fieldname": "use_batchwise_valuation", + "fieldtype": "Check", + "label": "Use Batch-wise Valuation", + "read_only": 1, + "set_only_once": 1 } ], "icon": "fa fa-archive", @@ -193,7 +207,7 @@ "image_field": "image", "links": [], "max_attachments": 5, - "modified": "2021-07-08 16:22:01.343105", + "modified": "2021-10-11 13:38:12.806976", "modified_by": "Administrator", "module": "Stock", "name": "Batch", From ce0514c8db17d59f2f84b3f6c263cd7e5877a049 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 15 Feb 2022 11:41:41 +0530 Subject: [PATCH 14/44] feat: batch wise valuation rates start with most used case: negative inventory isn't enabled - simple addition of qty and value when new batch qty is added - fetch outgoing rate from stock movement of specific batch --- erpnext/stock/doctype/batch/test_batch.py | 46 ++++++++++++++++++++ erpnext/stock/stock_ledger.py | 52 +++++++++++++++++++++++ 2 files changed, 98 insertions(+) diff --git a/erpnext/stock/doctype/batch/test_batch.py b/erpnext/stock/doctype/batch/test_batch.py index 0a663c2a188..e7d04db4547 100644 --- a/erpnext/stock/doctype/batch/test_batch.py +++ b/erpnext/stock/doctype/batch/test_batch.py @@ -7,6 +7,7 @@ from frappe.utils import cint, flt from erpnext.accounts.doctype.purchase_invoice.test_purchase_invoice import make_purchase_invoice from erpnext.stock.doctype.batch.batch import UnableToSelectBatchError, get_batch_no, get_batch_qty +from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry from erpnext.stock.get_item_details import get_item_details from erpnext.tests.utils import ERPNextTestCase @@ -300,6 +301,51 @@ class TestBatch(ERPNextTestCase): details = get_item_details(args) self.assertEqual(details.get('price_list_rate'), 400) + + def test_basic_batch_wise_valuation(self, batch_qty = 100): + item_code = "_TestBatchWiseVal" + warehouse = "_Test Warehouse - _TC" + self.make_batch_item(item_code) + + rates = [42, 420] + + batches = {} + for rate in rates: + se = make_stock_entry(item_code=item_code, qty=10, rate=rate, target=warehouse) + batches[se.items[0].batch_no] = rate + + LOW, HIGH = list(batches.keys()) + + # consume things out of order + consumption_plan = [ + (HIGH, 1), + (LOW, 2), + (HIGH, 2), + (HIGH, 4), + (LOW, 6), + ] + + stock_value = sum(rates) * 10 + qty_after_transaction = 20 + for batch, qty in consumption_plan: + # consume out of order + se = make_stock_entry(item_code=item_code, source=warehouse, qty=qty, batch_no=batch) + + sle = frappe.get_last_doc("Stock Ledger Entry", {"is_cancelled": 0, "voucher_no": se.name}) + + stock_value_difference = sle.actual_qty * batches[sle.batch_no] + self.assertAlmostEqual(sle.stock_value_difference, stock_value_difference) + + stock_value += stock_value_difference + self.assertAlmostEqual(sle.stock_value, stock_value) + + qty_after_transaction += sle.actual_qty + self.assertAlmostEqual(sle.qty_after_transaction, qty_after_transaction) + self.assertAlmostEqual(sle.valuation_rate, stock_value / qty_after_transaction) + + self.assertEqual(sle.stock_queue, []) # queues don't apply on batched items + + def create_batch(item_code, rate, create_item_price_for_batch): pi = make_purchase_invoice(company="_Test Company", warehouse= "Stores - _TC", cost_center = "Main - _TC", update_stock=1, diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index 00ca81f2b42..c33cc12c2f9 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -447,6 +447,8 @@ class update_entries_after(object): self.wh_data.qty_after_transaction = sle.qty_after_transaction self.wh_data.stock_value = flt(self.wh_data.qty_after_transaction) * flt(self.wh_data.valuation_rate) + elif sle.batch_no and frappe.db.get_value("Batch", sle.batch_no, "use_batchwise_valuation", cache=True): + self.update_batched_values(sle) else: if sle.voucher_type=="Stock Reconciliation" and not sle.batch_no: # assert @@ -481,6 +483,7 @@ class update_entries_after(object): if not self.args.get("sle_id"): self.update_outgoing_rate_on_transaction(sle) + def validate_negative_stock(self, sle): """ validate negative stock for entries current datetime onwards @@ -736,7 +739,22 @@ class update_entries_after(object): if not self.wh_data.stock_queue: self.wh_data.stock_queue.append([0, sle.incoming_rate or sle.outgoing_rate or self.wh_data.valuation_rate]) + def update_batched_values(self, sle): + incoming_rate = flt(sle.incoming_rate) + actual_qty = flt(sle.actual_qty) + self.wh_data.qty_after_transaction += actual_qty + + if actual_qty > 0: + stock_value_difference = incoming_rate * actual_qty + self.wh_data.stock_value += stock_value_difference + else: + outgoing_rate = _get_batch_outgoing_rate(item_code=sle.item_code, warehouse=sle.warehouse, batch_no=sle.batch_no, posting_date=sle.posting_date, posting_time=sle.posting_time, creation=sle.creation) + stock_value_difference = outgoing_rate * actual_qty + self.wh_data.stock_value += stock_value_difference + + if self.wh_data.qty_after_transaction: + self.wh_data.valuation_rate = self.wh_data.stock_value / self.wh_data.qty_after_transaction def check_if_allow_zero_valuation_rate(self, voucher_type, voucher_detail_no): ref_item_dt = "" @@ -897,6 +915,40 @@ def get_sle_by_voucher_detail_no(voucher_detail_no, excluded_sle=None): ['item_code', 'warehouse', 'posting_date', 'posting_time', 'timestamp(posting_date, posting_time) as timestamp'], as_dict=1) +def _get_batch_outgoing_rate(item_code, warehouse, batch_no, posting_date, posting_time, creation): + + batch_details = frappe.db.sql(""" + select sum(stock_value_difference) as batch_value, sum(actual_qty) as batch_qty + from `tabStock Ledger Entry` + where + item_code = %(item_code)s + and warehouse = %(warehouse)s + and batch_no = %(batch_no)s + and is_cancelled = 0 + and ( + timestamp(posting_date, posting_time) < timestamp(%(posting_date)s, %(posting_time)s) + or ( + timestamp(posting_date, posting_time) = timestamp(%(posting_date)s, %(posting_time)s) + and creation < %(creation)s + ) + ) + """, + { + "item_code": item_code, + "warehouse": warehouse, + "batch_no": batch_no, + "posting_date": posting_date, + "posting_time": posting_time, + "creation": creation, + }, + as_dict=True + ) + + if batch_details and batch_details[0].batch_qty: + return batch_details[0].batch_value / batch_details[0].batch_qty + + + def get_valuation_rate(item_code, warehouse, voucher_type, voucher_no, allow_zero_rate=False, currency=None, company=None, raise_error_if_no_rate=True): From 342d09a671c522031f73ba777950c70983cea31a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 14:28:51 +0530 Subject: [PATCH 15/44] feat: get_valuation_rate batch wise This function is used to show valuation rate on frontend and also as fallback in case values aren't available. Add "batch_no" param to get batch specific valuation rates. Co-Authored-By: Alan Tom <2.alan.tom@gmail.com> --- erpnext/controllers/buying_controller.py | 1 + .../controllers/sales_and_purchase_return.py | 1 + erpnext/controllers/selling_controller.py | 1 + erpnext/public/js/controllers/transaction.js | 1 + erpnext/stock/doctype/batch/test_batch.py | 39 +++++++++++++++++ .../stock/doctype/stock_entry/stock_entry.js | 3 ++ .../stock/doctype/stock_entry/stock_entry.py | 3 +- erpnext/stock/stock_ledger.py | 43 +++++++++++++------ erpnext/stock/utils.py | 2 +- 9 files changed, 79 insertions(+), 15 deletions(-) diff --git a/erpnext/controllers/buying_controller.py b/erpnext/controllers/buying_controller.py index a181af73133..b8315572004 100644 --- a/erpnext/controllers/buying_controller.py +++ b/erpnext/controllers/buying_controller.py @@ -249,6 +249,7 @@ class BuyingController(StockController, Subcontracting): "posting_time": self.get('posting_time'), "qty": -1 * flt(d.get('stock_qty')), "serial_no": d.get('serial_no'), + "batch_no": d.get("batch_no"), "company": self.company, "voucher_type": self.doctype, "voucher_no": self.name, diff --git a/erpnext/controllers/sales_and_purchase_return.py b/erpnext/controllers/sales_and_purchase_return.py index df3c5f10c1b..8c3aab442bb 100644 --- a/erpnext/controllers/sales_and_purchase_return.py +++ b/erpnext/controllers/sales_and_purchase_return.py @@ -420,6 +420,7 @@ def get_rate_for_return(voucher_type, voucher_no, item_code, return_against=None "posting_time": sle.get('posting_time'), "qty": sle.actual_qty, "serial_no": sle.get('serial_no'), + "batch_no": sle.get("batch_no"), "company": sle.company, "voucher_type": sle.voucher_type, "voucher_no": sle.voucher_no diff --git a/erpnext/controllers/selling_controller.py b/erpnext/controllers/selling_controller.py index 31b22093998..e918cde7c48 100644 --- a/erpnext/controllers/selling_controller.py +++ b/erpnext/controllers/selling_controller.py @@ -394,6 +394,7 @@ class SellingController(StockController): "posting_time": self.get('posting_time') or nowtime(), "qty": qty if cint(self.get("is_return")) else (-1 * qty), "serial_no": d.get('serial_no'), + "batch_no": d.get("batch_no"), "company": self.company, "voucher_type": self.doctype, "voucher_no": self.name, diff --git a/erpnext/public/js/controllers/transaction.js b/erpnext/public/js/controllers/transaction.js index 136e1edb6b9..933ced0bd70 100644 --- a/erpnext/public/js/controllers/transaction.js +++ b/erpnext/public/js/controllers/transaction.js @@ -719,6 +719,7 @@ erpnext.TransactionController = class TransactionController extends erpnext.taxe 'posting_time': posting_time, 'qty': item.qty * item.conversion_factor, 'serial_no': item.serial_no, + 'batch_no': item.batch_no, 'voucher_type': voucher_type, 'company': company, 'allow_zero_valuation_rate': item.allow_zero_valuation_rate diff --git a/erpnext/stock/doctype/batch/test_batch.py b/erpnext/stock/doctype/batch/test_batch.py index e7d04db4547..73a48b3f13e 100644 --- a/erpnext/stock/doctype/batch/test_batch.py +++ b/erpnext/stock/doctype/batch/test_batch.py @@ -8,7 +8,11 @@ from frappe.utils import cint, flt from erpnext.accounts.doctype.purchase_invoice.test_purchase_invoice import make_purchase_invoice from erpnext.stock.doctype.batch.batch import UnableToSelectBatchError, get_batch_no, get_batch_qty from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry +from erpnext.stock.doctype.stock_reconciliation.test_stock_reconciliation import ( + create_stock_reconciliation, +) from erpnext.stock.get_item_details import get_item_details +from erpnext.stock.stock_ledger import get_valuation_rate from erpnext.tests.utils import ERPNextTestCase @@ -345,6 +349,41 @@ class TestBatch(ERPNextTestCase): self.assertEqual(sle.stock_queue, []) # queues don't apply on batched items + def test_moving_batch_valuation_rates(self): + item_code = "_TestBatchWiseVal" + warehouse = "_Test Warehouse - _TC" + self.make_batch_item(item_code) + + def assertValuation(expected): + actual = get_valuation_rate(item_code, warehouse, "voucher_type", "voucher_no", batch_no=batch_no) + self.assertAlmostEqual(actual, expected) + + se = make_stock_entry(item_code=item_code, qty=100, rate=10, target=warehouse) + batch_no = se.items[0].batch_no + assertValuation(10) + + # consumption should never affect current valuation rate + make_stock_entry(item_code=item_code, qty=20, source=warehouse) + assertValuation(10) + + make_stock_entry(item_code=item_code, qty=30, source=warehouse) + assertValuation(10) + + # 50 * 10 = 500 current value, add more item with higher valuation + make_stock_entry(item_code=item_code, qty=50, rate=20, target=warehouse, batch_no=batch_no) + assertValuation(15) + + # consuming again shouldn't do anything + make_stock_entry(item_code=item_code, qty=20, source=warehouse) + assertValuation(15) + + # reset rate with stock reconiliation + create_stock_reconciliation(item_code=item_code, warehouse=warehouse, qty=10, rate=25, batch_no=batch_no) + assertValuation(25) + + make_stock_entry(item_code=item_code, qty=20, rate=20, target=warehouse, batch_no=batch_no) + assertValuation((20 * 20 + 10 * 25) / (10 + 20)) + def create_batch(item_code, rate, create_item_price_for_batch): pi = make_purchase_invoice(company="_Test Company", diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.js b/erpnext/stock/doctype/stock_entry/stock_entry.js index c4b8131305e..5c9da3a2052 100644 --- a/erpnext/stock/doctype/stock_entry/stock_entry.js +++ b/erpnext/stock/doctype/stock_entry/stock_entry.js @@ -425,6 +425,7 @@ frappe.ui.form.on('Stock Entry', { 'posting_time' : frm.doc.posting_time, 'warehouse' : cstr(item.s_warehouse) || cstr(item.t_warehouse), 'serial_no' : item.serial_no, + 'batch_no' : item.batch_no, 'company' : frm.doc.company, 'qty' : item.s_warehouse ? -1*flt(item.transfer_qty) : flt(item.transfer_qty), 'voucher_type' : frm.doc.doctype, @@ -457,6 +458,7 @@ frappe.ui.form.on('Stock Entry', { 'warehouse': cstr(child.s_warehouse) || cstr(child.t_warehouse), 'transfer_qty': child.transfer_qty, 'serial_no': child.serial_no, + 'batch_no': child.batch_no, 'qty': child.s_warehouse ? -1* child.transfer_qty : child.transfer_qty, 'posting_date': frm.doc.posting_date, 'posting_time': frm.doc.posting_time, @@ -680,6 +682,7 @@ frappe.ui.form.on('Stock Entry Detail', { 'warehouse' : cstr(d.s_warehouse) || cstr(d.t_warehouse), 'transfer_qty' : d.transfer_qty, 'serial_no' : d.serial_no, + 'batch_no' : d.batch_no, 'bom_no' : d.bom_no, 'expense_account' : d.expense_account, 'cost_center' : d.cost_center, diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.py b/erpnext/stock/doctype/stock_entry/stock_entry.py index 9ba007a186e..99cf4de5de7 100644 --- a/erpnext/stock/doctype/stock_entry/stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/stock_entry.py @@ -510,7 +510,7 @@ class StockEntry(StockController): d.basic_rate = get_valuation_rate(d.item_code, d.t_warehouse, self.doctype, self.name, d.allow_zero_valuation_rate, currency=erpnext.get_company_currency(self.company), company=self.company, - raise_error_if_no_rate=raise_error_if_no_rate) + raise_error_if_no_rate=raise_error_if_no_rate, batch_no=d.batch_no) d.basic_rate = flt(d.basic_rate, d.precision("basic_rate")) if d.is_process_loss: @@ -541,6 +541,7 @@ class StockEntry(StockController): "posting_time": self.posting_time, "qty": item.s_warehouse and -1*flt(item.transfer_qty) or flt(item.transfer_qty), "serial_no": item.serial_no, + "batch_no": item.batch_no, "voucher_type": self.doctype, "voucher_no": self.name, "company": self.company, diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index c33cc12c2f9..53bfed87229 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -634,7 +634,7 @@ class update_entries_after(object): if not allow_zero_rate: self.wh_data.valuation_rate = get_valuation_rate(sle.item_code, sle.warehouse, sle.voucher_type, sle.voucher_no, self.allow_zero_rate, - currency=erpnext.get_company_currency(sle.company), company=sle.company) + currency=erpnext.get_company_currency(sle.company), company=sle.company, batch_no=sle.batch_no) def get_incoming_value_for_serial_nos(self, sle, serial_nos): # get rate from serial nos within same company @@ -702,7 +702,7 @@ class update_entries_after(object): if not allow_zero_valuation_rate: self.wh_data.valuation_rate = get_valuation_rate(sle.item_code, sle.warehouse, sle.voucher_type, sle.voucher_no, self.allow_zero_rate, - currency=erpnext.get_company_currency(sle.company), company=sle.company) + currency=erpnext.get_company_currency(sle.company), company=sle.company, batch_no=sle.batch_no) def update_queue_values(self, sle): incoming_rate = flt(sle.incoming_rate) @@ -722,7 +722,7 @@ class update_entries_after(object): if not allow_zero_valuation_rate: return get_valuation_rate(sle.item_code, sle.warehouse, sle.voucher_type, sle.voucher_no, self.allow_zero_rate, - currency=erpnext.get_company_currency(sle.company), company=sle.company) + currency=erpnext.get_company_currency(sle.company), company=sle.company, batch_no=sle.batch_no) else: return 0.0 @@ -950,21 +950,38 @@ def _get_batch_outgoing_rate(item_code, warehouse, batch_no, posting_date, posti def get_valuation_rate(item_code, warehouse, voucher_type, voucher_no, - allow_zero_rate=False, currency=None, company=None, raise_error_if_no_rate=True): + allow_zero_rate=False, currency=None, company=None, raise_error_if_no_rate=True, batch_no=None): if not company: company = frappe.get_cached_value("Warehouse", warehouse, "company") + last_valuation_rate = None + + # Get moving average rate of a specific batch number + if warehouse and batch_no and frappe.db.get_value("Batch", batch_no, "use_batchwise_valuation"): + last_valuation_rate = frappe.db.sql(""" + select sum(stock_value_difference) / sum(actual_qty) + from `tabStock Ledger Entry` + where + item_code = %s + AND warehouse = %s + AND batch_no = %s + AND is_cancelled = 0 + AND NOT (voucher_no = %s AND voucher_type = %s) + """, + (item_code, warehouse, batch_no, voucher_no, voucher_type)) + # Get valuation rate from last sle for the same item and warehouse - last_valuation_rate = frappe.db.sql("""select valuation_rate - from `tabStock Ledger Entry` force index (item_warehouse) - where - item_code = %s - AND warehouse = %s - AND valuation_rate >= 0 - AND is_cancelled = 0 - AND NOT (voucher_no = %s AND voucher_type = %s) - order by posting_date desc, posting_time desc, name desc limit 1""", (item_code, warehouse, voucher_no, voucher_type)) + if not last_valuation_rate or last_valuation_rate[0][0] is None: + last_valuation_rate = frappe.db.sql("""select valuation_rate + from `tabStock Ledger Entry` force index (item_warehouse) + where + item_code = %s + AND warehouse = %s + AND valuation_rate >= 0 + AND is_cancelled = 0 + AND NOT (voucher_no = %s AND voucher_type = %s) + order by posting_date desc, posting_time desc, name desc limit 1""", (item_code, warehouse, voucher_no, voucher_type)) if not last_valuation_rate: # Get valuation rate from last sle for the item against any warehouse diff --git a/erpnext/stock/utils.py b/erpnext/stock/utils.py index 7263e39cc9f..3be252e5935 100644 --- a/erpnext/stock/utils.py +++ b/erpnext/stock/utils.py @@ -231,7 +231,7 @@ def get_incoming_rate(args, raise_error_if_no_rate=True): in_rate = get_valuation_rate(args.get('item_code'), args.get('warehouse'), args.get('voucher_type'), voucher_no, args.get('allow_zero_valuation'), currency=erpnext.get_company_currency(args.get('company')), company=args.get('company'), - raise_error_if_no_rate=raise_error_if_no_rate) + raise_error_if_no_rate=raise_error_if_no_rate, batch_no=args.get("batch_no")) return flt(in_rate) From ab926521bd0c9802666032cb3c32aa803655bde0 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 15:37:03 +0530 Subject: [PATCH 16/44] fix: correct incoming rate for batched items --- erpnext/stock/stock_ledger.py | 5 ++--- erpnext/stock/utils.py | 22 ++++++++++++++++++---- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index 53bfed87229..4748ad4e462 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -749,7 +749,7 @@ class update_entries_after(object): stock_value_difference = incoming_rate * actual_qty self.wh_data.stock_value += stock_value_difference else: - outgoing_rate = _get_batch_outgoing_rate(item_code=sle.item_code, warehouse=sle.warehouse, batch_no=sle.batch_no, posting_date=sle.posting_date, posting_time=sle.posting_time, creation=sle.creation) + outgoing_rate = get_batch_incoming_rate(item_code=sle.item_code, warehouse=sle.warehouse, batch_no=sle.batch_no, posting_date=sle.posting_date, posting_time=sle.posting_time, creation=sle.creation) stock_value_difference = outgoing_rate * actual_qty self.wh_data.stock_value += stock_value_difference @@ -915,7 +915,7 @@ def get_sle_by_voucher_detail_no(voucher_detail_no, excluded_sle=None): ['item_code', 'warehouse', 'posting_date', 'posting_time', 'timestamp(posting_date, posting_time) as timestamp'], as_dict=1) -def _get_batch_outgoing_rate(item_code, warehouse, batch_no, posting_date, posting_time, creation): +def get_batch_incoming_rate(item_code, warehouse, batch_no, posting_date, posting_time, creation=None): batch_details = frappe.db.sql(""" select sum(stock_value_difference) as batch_value, sum(actual_qty) as batch_qty @@ -948,7 +948,6 @@ def _get_batch_outgoing_rate(item_code, warehouse, batch_no, posting_date, posti return batch_details[0].batch_value / batch_details[0].batch_qty - def get_valuation_rate(item_code, warehouse, voucher_type, voucher_no, allow_zero_rate=False, currency=None, company=None, raise_error_if_no_rate=True, batch_no=None): diff --git a/erpnext/stock/utils.py b/erpnext/stock/utils.py index 3be252e5935..e2bd2f197d0 100644 --- a/erpnext/stock/utils.py +++ b/erpnext/stock/utils.py @@ -209,13 +209,28 @@ def _create_bin(item_code, warehouse): @frappe.whitelist() def get_incoming_rate(args, raise_error_if_no_rate=True): """Get Incoming Rate based on valuation method""" - from erpnext.stock.stock_ledger import get_previous_sle, get_valuation_rate + from erpnext.stock.stock_ledger import ( + get_batch_incoming_rate, + get_previous_sle, + get_valuation_rate, + ) if isinstance(args, str): args = json.loads(args) - in_rate = 0 + voucher_no = args.get('voucher_no') or args.get('name') + + in_rate = None if (args.get("serial_no") or "").strip(): in_rate = get_avg_purchase_rate(args.get("serial_no")) + elif args.get("batch_no") and \ + frappe.db.get_value("Batch", args.get("batch_no"), "use_batchwise_valuation", cache=True): + in_rate = get_batch_incoming_rate( + item_code=args.get('item_code'), + warehouse=args.get('warehouse'), + batch_no=args.get("batch_no"), + posting_date=args.get("posting_date"), + posting_time=args.get("posting_time"), + ) else: valuation_method = get_valuation_method(args.get("item_code")) previous_sle = get_previous_sle(args) @@ -226,8 +241,7 @@ def get_incoming_rate(args, raise_error_if_no_rate=True): elif valuation_method == 'Moving Average': in_rate = previous_sle.get('valuation_rate') or 0 - if not in_rate: - voucher_no = args.get('voucher_no') or args.get('name') + if in_rate is None: in_rate = get_valuation_rate(args.get('item_code'), args.get('warehouse'), args.get('voucher_type'), voucher_no, args.get('allow_zero_valuation'), currency=erpnext.get_company_currency(args.get('company')), company=args.get('company'), From 102fff24c886b49d08776307d513d68ffd56e918 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 15:51:04 +0530 Subject: [PATCH 17/44] refactor: convert query to QB and make creation optional --- erpnext/stock/doctype/batch/test_batch.py | 4 +- erpnext/stock/stock_ledger.py | 53 ++++++++++++----------- 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/erpnext/stock/doctype/batch/test_batch.py b/erpnext/stock/doctype/batch/test_batch.py index 73a48b3f13e..6495b56e929 100644 --- a/erpnext/stock/doctype/batch/test_batch.py +++ b/erpnext/stock/doctype/batch/test_batch.py @@ -1,6 +1,8 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: GNU General Public License v3. See license.txt +import json + import frappe from frappe.exceptions import ValidationError from frappe.utils import cint, flt @@ -347,7 +349,7 @@ class TestBatch(ERPNextTestCase): self.assertAlmostEqual(sle.qty_after_transaction, qty_after_transaction) self.assertAlmostEqual(sle.valuation_rate, stock_value / qty_after_transaction) - self.assertEqual(sle.stock_queue, []) # queues don't apply on batched items + self.assertEqual(json.loads(sle.stock_queue), []) # queues don't apply on batched items def test_moving_batch_valuation_rates(self): item_code = "_TestBatchWiseVal" diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index 4748ad4e462..cacec408ce2 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -8,7 +8,9 @@ from typing import Optional import frappe from frappe import _ from frappe.model.meta import get_field_precision +from frappe.query_builder.functions import Sum from frappe.utils import cint, cstr, flt, get_link_to_form, getdate, now, nowdate +from pypika import CustomFunction import erpnext from erpnext.stock.doctype.bin.bin import update_qty as update_bin_qty @@ -24,7 +26,6 @@ class NegativeStockError(frappe.ValidationError): pass class SerialNoExistsInFutureTransaction(frappe.ValidationError): pass -_exceptions = frappe.local('stockledger_exceptions') def make_sl_entries(sl_entries, allow_negative_stock=False, via_landed_cost_voucher=False): from erpnext.controllers.stock_controller import future_sle_exists @@ -917,32 +918,32 @@ def get_sle_by_voucher_detail_no(voucher_detail_no, excluded_sle=None): def get_batch_incoming_rate(item_code, warehouse, batch_no, posting_date, posting_time, creation=None): - batch_details = frappe.db.sql(""" - select sum(stock_value_difference) as batch_value, sum(actual_qty) as batch_qty - from `tabStock Ledger Entry` - where - item_code = %(item_code)s - and warehouse = %(warehouse)s - and batch_no = %(batch_no)s - and is_cancelled = 0 - and ( - timestamp(posting_date, posting_time) < timestamp(%(posting_date)s, %(posting_time)s) - or ( - timestamp(posting_date, posting_time) = timestamp(%(posting_date)s, %(posting_time)s) - and creation < %(creation)s - ) + Timestamp = CustomFunction('timestamp', ['date', 'time']) + + sle = frappe.qb.DocType("Stock Ledger Entry") + + timestamp_condition = (Timestamp(sle.posting_date, sle.posting_time) < Timestamp(posting_date, posting_time)) + if creation: + timestamp_condition |= ( + (Timestamp(sle.posting_date, sle.posting_time) == Timestamp(posting_date, posting_time)) + & (sle.creation < creation) ) - """, - { - "item_code": item_code, - "warehouse": warehouse, - "batch_no": batch_no, - "posting_date": posting_date, - "posting_time": posting_time, - "creation": creation, - }, - as_dict=True - ) + + batch_details = ( + frappe.qb + .from_(sle) + .select( + Sum(sle.stock_value_difference).as_("batch_value"), + Sum(sle.actual_qty).as_("batch_qty") + ) + .where( + (sle.item_code == item_code) + & (sle.warehouse == warehouse) + & (sle.batch_no == batch_no) + & (sle.is_cancelled == 0) + ) + .where(timestamp_condition) + ).run(as_dict=True) if batch_details and batch_details[0].batch_qty: return batch_details[0].batch_value / batch_details[0].batch_qty From d130233ffc79d085b61bc1b63956d18c03de7a88 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 16:14:15 +0530 Subject: [PATCH 18/44] test: fix expected test failures --- .../stock_reconciliation/test_stock_reconciliation.py | 11 ++++++----- erpnext/stock/stock_ledger.py | 1 + 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py b/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py index 86af0a0cf3b..2ffe127d9a5 100644 --- a/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py +++ b/erpnext/stock/doctype/stock_reconciliation/test_stock_reconciliation.py @@ -200,7 +200,6 @@ class TestStockReconciliation(ERPNextTestCase): def test_stock_reco_for_batch_item(self): to_delete_records = [] - to_delete_serial_nos = [] # Add new serial nos item_code = "Stock-Reco-batch-Item-1" @@ -208,20 +207,22 @@ class TestStockReconciliation(ERPNextTestCase): sr = create_stock_reconciliation(item_code=item_code, warehouse = warehouse, qty=5, rate=200, do_not_submit=1) - sr.save(ignore_permissions=True) + sr.save() sr.submit() - self.assertTrue(sr.items[0].batch_no) + batch_no = sr.items[0].batch_no + self.assertTrue(batch_no) to_delete_records.append(sr.name) sr1 = create_stock_reconciliation(item_code=item_code, - warehouse = warehouse, qty=6, rate=300, batch_no=sr.items[0].batch_no) + warehouse = warehouse, qty=6, rate=300, batch_no=batch_no) args = { "item_code": item_code, "warehouse": warehouse, "posting_date": nowdate(), "posting_time": nowtime(), + "batch_no": batch_no, } valuation_rate = get_incoming_rate(args) @@ -230,7 +231,7 @@ class TestStockReconciliation(ERPNextTestCase): sr2 = create_stock_reconciliation(item_code=item_code, - warehouse = warehouse, qty=0, rate=0, batch_no=sr.items[0].batch_no) + warehouse = warehouse, qty=0, rate=0, batch_no=batch_no) stock_value = get_stock_value_on(warehouse, nowdate(), item_code) self.assertEqual(stock_value, 0) diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index cacec408ce2..2dd26643f74 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -751,6 +751,7 @@ class update_entries_after(object): self.wh_data.stock_value += stock_value_difference else: outgoing_rate = get_batch_incoming_rate(item_code=sle.item_code, warehouse=sle.warehouse, batch_no=sle.batch_no, posting_date=sle.posting_date, posting_time=sle.posting_time, creation=sle.creation) + # TODO: negative stock handling stock_value_difference = outgoing_rate * actual_qty self.wh_data.stock_value += stock_value_difference From 312db429e4605d6d0ce47d1034662fdf0ec053b7 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 16:26:17 +0530 Subject: [PATCH 19/44] refactor: use qb for patching flag --- erpnext/patches/v14_0/update_batch_valuation_flag.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/erpnext/patches/v14_0/update_batch_valuation_flag.py b/erpnext/patches/v14_0/update_batch_valuation_flag.py index d9f08d8d97b..55c8c48aa21 100644 --- a/erpnext/patches/v14_0/update_batch_valuation_flag.py +++ b/erpnext/patches/v14_0/update_batch_valuation_flag.py @@ -6,7 +6,6 @@ def execute(): - Don't use batchwise valuation for existing batches. - Only batches created after this patch shoule use it. """ - frappe.db.sql(""" - UPDATE `tabBatch` - SET use_batchwise_valuation=0 - """) + + batch = frappe.qb.DocType("Batch") + frappe.qb.update(batch).set(batch.use_batchwise_valuation, 0).run() From 683ef8a60397b728bd18e1a3c3c317e2f155793c Mon Sep 17 00:00:00 2001 From: 18alantom <2.alan.tom@gmail.com> Date: Sat, 19 Feb 2022 16:19:30 +0530 Subject: [PATCH 20/44] test: more tests for batchwise valuation Co-Authored-By: Ankush Menat --- .../purchase_receipt/test_purchase_receipt.py | 1 + .../test_stock_ledger_entry.py | 278 ++++++++++++++++++ 2 files changed, 279 insertions(+) diff --git a/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py b/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py index 5ab7929a2a6..d481689c130 100644 --- a/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py +++ b/erpnext/stock/doctype/purchase_receipt/test_purchase_receipt.py @@ -1540,6 +1540,7 @@ def make_purchase_receipt(**args): "conversion_factor": args.conversion_factor or 1.0, "stock_qty": flt(qty) * (flt(args.conversion_factor) or 1.0), "serial_no": args.serial_no, + "batch_no": args.batch_no, "stock_uom": args.stock_uom or "_Test UOM", "uom": uom, "cost_center": args.cost_center or frappe.get_cached_value('Company', pr.company, 'cost_center'), diff --git a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py index a1030d54964..60fea9613a3 100644 --- a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py +++ b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py @@ -1,6 +1,10 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # See license.txt +import json +from operator import itemgetter +from uuid import uuid4 + import frappe from frappe.core.page.permission_manager.permission_manager import reset from frappe.utils import add_days, today @@ -349,6 +353,170 @@ class TestStockLedgerEntry(ERPNextTestCase): frappe.set_user("Administrator") user.remove_roles("Stock Manager") + def test_batchwise_item_valuation_moving_average(self): + suffix = get_unique_suffix() + item, warehouses, batches = setup_item_valuation_test( + valuation_method="Moving Average", suffix=suffix + ) + + # Incoming Entries for Stock Value check + pr_entry_list = [ + (item, warehouses[0], batches[0], 1, 100), + (item, warehouses[0], batches[1], 1, 50), + (item, warehouses[0], batches[0], 1, 150), + (item, warehouses[0], batches[1], 1, 100), + ] + prs = create_purchase_receipt_entries_for_batchwise_item_valuation_test(pr_entry_list) + sle_details = fetch_sle_details_for_doc_list(prs, ['stock_value']) + sv_list = [d['stock_value'] for d in sle_details] + expected_sv = [100, 150, 300, 400] + self.assertEqual(expected_sv, sv_list, "Incorrect 'Stock Value' values") + + # Outgoing Entries for Stock Value Difference check + dn_entry_list = [ + (item, warehouses[0], batches[1], 1, 200), + (item, warehouses[0], batches[0], 1, 200), + (item, warehouses[0], batches[1], 1, 200), + (item, warehouses[0], batches[0], 1, 200) + ] + dns = create_delivery_note_entries_for_batchwise_item_valuation_test(dn_entry_list) + sle_details = fetch_sle_details_for_doc_list(dns, ['stock_value_difference']) + svd_list = [-1 * d['stock_value_difference'] for d in sle_details] + expected_incoming_rates = expected_abs_svd = [75, 125, 75, 125] + + self.assertEqual(expected_abs_svd, svd_list, "Incorrect 'Stock Value Difference' values") + for dn, incoming_rate in zip(dns, expected_incoming_rates): + self.assertEqual( + dn.items[0].incoming_rate, incoming_rate, + "Incorrect 'Incoming Rate' values fetched for DN items" + ) + + + def assertSLEs(self, doc, expected_sles): + """ Compare sorted SLEs, useful for vouchers that create multiple SLEs for same line""" + sles = frappe.get_all("Stock Ledger Entry", fields=["*"], + filters={"voucher_no": doc.name, "voucher_type": doc.doctype, "is_cancelled":0}, + order_by="timestamp(posting_date, posting_time), creation") + + for exp_sle, act_sle in zip(expected_sles, sles): + for k, v in exp_sle.items(): + self.assertEqual(v, act_sle[k], msg=f"{k} doesn't match \n{exp_sle}\n{act_sle}") + + def test_batchwise_item_valuation_stock_reco(self): + suffix = get_unique_suffix() + item, warehouses, batches = setup_item_valuation_test( + valuation_method="FIFO", suffix=suffix + ) + state = { + "stock_value" : 0.0, + "qty": 0.0 + } + def update_invariants(exp_sles): + for sle in exp_sles: + state["stock_value"] += sle["stock_value_difference"] + state["qty"] += sle["actual_qty"] + sle["stock_value"] = state["stock_value"] + sle["qty_after_transaction"] = state["qty"] + + osr1 = create_stock_reconciliation(warehouse=warehouses[0], item_code=item, qty=10, rate=100, batch_no=batches[1]) + expected_sles = [ + {"actual_qty": 10, "stock_value_difference": 1000}, + ] + update_invariants(expected_sles) + self.assertSLEs(osr1, expected_sles) + + osr2 = create_stock_reconciliation(warehouse=warehouses[0], item_code=item, qty=13, rate=200, batch_no=batches[0]) + expected_sles = [ + {"actual_qty": 13, "stock_value_difference": 200*13}, + ] + update_invariants(expected_sles) + self.assertSLEs(osr2, expected_sles) + + sr1 = create_stock_reconciliation(warehouse=warehouses[0], item_code=item, qty=5, rate=50, batch_no=batches[1]) + + expected_sles = [ + {"actual_qty": -10, "stock_value_difference": -10 * 100}, + {"actual_qty": 5, "stock_value_difference": 250} + ] + update_invariants(expected_sles) + self.assertSLEs(sr1, expected_sles) + + sr2 = create_stock_reconciliation(warehouse=warehouses[0], item_code=item, qty=20, rate=75, batch_no=batches[0]) + expected_sles = [ + {"actual_qty": -13, "stock_value_difference": -13 * 200}, + {"actual_qty": 20, "stock_value_difference": 20 * 75} + ] + update_invariants(expected_sles) + self.assertSLEs(sr2, expected_sles) + + def test_legacy_item_valuation_stock_entry(self): + suffix = get_unique_suffix() + columns = [ + 'stock_value_difference', + 'stock_value', + 'actual_qty', + 'qty_after_transaction', + 'stock_queue', + ] + item, warehouses, batches = setup_item_valuation_test( + valuation_method="FIFO", suffix=suffix, use_batchwise_valuation=0 + ) + + def check_sle_details_against_expected(sle_details, expected_sle_details, detail, columns): + for i, (sle_vals, ex_sle_vals) in enumerate(zip(sle_details, expected_sle_details)): + for col, sle_val, ex_sle_val in zip(columns, sle_vals, ex_sle_vals): + if col == 'stock_queue': + sle_val = get_stock_value_from_q(sle_val) + ex_sle_val = get_stock_value_from_q(ex_sle_val) + self.assertEqual( + sle_val, ex_sle_val, + f"Incorrect {col} value on transaction #: {i} in {detail}" + ) + + # List used to defer assertions to prevent commits cause of error skipped rollback + details_list = [] + + + # Test Material Receipt Entries + se_entry_list_mr = [ + (item, None, warehouses[0], batches[0], 1, 50, "2021-01-21"), + (item, None, warehouses[0], batches[1], 1, 100, "2021-01-23"), + ] + ses = create_stock_entry_entries_for_batchwise_item_valuation_test( + se_entry_list_mr, "Material Receipt" + ) + sle_details = fetch_sle_details_for_doc_list(ses, columns=columns, as_dict=0) + expected_sle_details = [ + (50.0, 50.0, 1.0, 1.0, '[[1.0, 50.0]]'), + (100.0, 150.0, 1.0, 2.0, '[[1.0, 50.0], [1.0, 100.0]]'), + ] + details_list.append(( + sle_details, expected_sle_details, + "Material Receipt Entries", columns + )) + + + # Test Material Issue Entries + se_entry_list_mi = [ + (item, warehouses[0], None, batches[1], 1, None, "2021-01-29"), + ] + ses = create_stock_entry_entries_for_batchwise_item_valuation_test( + se_entry_list_mi, "Material Issue" + ) + sle_details = fetch_sle_details_for_doc_list(ses, columns=columns, as_dict=0) + expected_sle_details = [ + (-50.0, 100.0, -1.0, 1.0, '[[1, 100.0]]') + ] + details_list.append(( + sle_details, expected_sle_details, + "Material Issue Entries", columns + )) + + + # Run assertions + for details in details_list: + check_sle_details_against_expected(*details) + def create_repack_entry(**args): args = frappe._dict(args) @@ -412,3 +580,113 @@ def create_items(): make_item(d, properties=properties) return items + +def setup_item_valuation_test(valuation_method, suffix, use_batchwise_valuation=1, batches_list=['X', 'Y']): + from erpnext.stock.doctype.batch.batch import make_batch + from erpnext.stock.doctype.item.test_item import make_item + from erpnext.stock.doctype.warehouse.test_warehouse import create_warehouse + + item = make_item( + f"IV - Test Item {valuation_method} {suffix}", + dict(valuation_method=valuation_method, has_batch_no=1) + ) + warehouses = [create_warehouse(f"IV - Test Warehouse {i}") for i in ['J', 'K']] + batches = [f"IV - Test Batch {i} {valuation_method} {suffix}" for i in batches_list] + + for i, batch_id in enumerate(batches): + if not frappe.db.exists("Batch", batch_id): + ubw = use_batchwise_valuation + if isinstance(use_batchwise_valuation, (list, tuple)): + ubw = use_batchwise_valuation[i] + make_batch( + frappe._dict( + batch_id=batch_id, + item=item.item_code, + use_batchwise_valuation=ubw + ) + ) + + return item.item_code, warehouses, batches + +def create_purchase_receipt_entries_for_batchwise_item_valuation_test(pr_entry_list): + from erpnext.stock.doctype.purchase_receipt.test_purchase_receipt import make_purchase_receipt + prs = [] + + for item, warehouse, batch_no, qty, rate in pr_entry_list: + pr = make_purchase_receipt(item=item, warehouse=warehouse, qty=qty, rate=rate, batch_no=batch_no) + prs.append(pr) + + return prs + +def create_delivery_note_entries_for_batchwise_item_valuation_test(dn_entry_list): + from erpnext.selling.doctype.sales_order.sales_order import make_delivery_note + from erpnext.selling.doctype.sales_order.test_sales_order import make_sales_order + dns = [] + for item, warehouse, batch_no, qty, rate in dn_entry_list: + so = make_sales_order( + rate=rate, + qty=qty, + item=item, + warehouse=warehouse, + against_blanket_order=0 + ) + + dn = make_delivery_note(so.name) + dn.items[0].batch_no = batch_no + dn.insert() + dn.submit() + dns.append(dn) + return dns + +def fetch_sle_details_for_doc_list(doc_list, columns, as_dict=1): + return frappe.db.sql(f""" + SELECT { ', '.join(columns)} + FROM `tabStock Ledger Entry` + WHERE + voucher_no IN %(voucher_nos)s + and docstatus = 1 + ORDER BY timestamp(posting_date, posting_time) ASC, CREATION ASC + """, dict( + voucher_nos=[doc.name for doc in doc_list] + ), as_dict=as_dict) + +def get_stock_value_from_q(q): + return sum(r*q for r,q in json.loads(q)) + +def create_stock_entry_entries_for_batchwise_item_valuation_test(se_entry_list, purpose): + ses = [] + for item, source, target, batch, qty, rate, posting_date in se_entry_list: + args = dict( + item_code=item, + qty=qty, + company="_Test Company", + batch_no=batch, + posting_date=posting_date, + purpose=purpose + ) + + if purpose == "Material Receipt": + args.update( + dict(to_warehouse=target, rate=rate) + ) + + elif purpose == "Material Issue": + args.update( + dict(from_warehouse=source) + ) + + elif purpose == "Material Transfer": + args.update( + dict(from_warehouse=source, to_warehouse=target) + ) + + else: + raise ValueError(f"Invalid purpose: {purpose}") + ses.append(make_stock_entry(**args)) + + return ses + +def get_unique_suffix(): + # Used to isolate valuation sensitive + # tests to prevent future tests from failing. + return str(uuid4())[:8].upper() From 5718777a2b3018e07ea310e87e5a2ea26ff3eb1b Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 18:36:16 +0530 Subject: [PATCH 21/44] fix: consider batch_no when getting incoming rate --- erpnext/controllers/buying_controller.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/erpnext/controllers/buying_controller.py b/erpnext/controllers/buying_controller.py index b8315572004..b740476481f 100644 --- a/erpnext/controllers/buying_controller.py +++ b/erpnext/controllers/buying_controller.py @@ -279,7 +279,8 @@ class BuyingController(StockController, Subcontracting): "posting_date": self.posting_date, "posting_time": self.posting_time, "qty": -1 * d.consumed_qty, - "serial_no": d.serial_no + "serial_no": d.serial_no, + "batch_no": d.batch_no, }) if rate > 0: From 60b8bae85f00b6a6bf4a26c7604e28e0b075bb52 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 19:18:35 +0530 Subject: [PATCH 22/44] test: batch wise valuation for transfer and intermediate --- .../test_stock_ledger_entry.py | 99 ++++++++++++++++--- 1 file changed, 86 insertions(+), 13 deletions(-) diff --git a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py index 60fea9613a3..c298b5a0963 100644 --- a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py +++ b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py @@ -354,10 +354,7 @@ class TestStockLedgerEntry(ERPNextTestCase): user.remove_roles("Stock Manager") def test_batchwise_item_valuation_moving_average(self): - suffix = get_unique_suffix() - item, warehouses, batches = setup_item_valuation_test( - valuation_method="Moving Average", suffix=suffix - ) + item, warehouses, batches = setup_item_valuation_test(valuation_method="Moving Average") # Incoming Entries for Stock Value check pr_entry_list = [ @@ -403,10 +400,7 @@ class TestStockLedgerEntry(ERPNextTestCase): self.assertEqual(v, act_sle[k], msg=f"{k} doesn't match \n{exp_sle}\n{act_sle}") def test_batchwise_item_valuation_stock_reco(self): - suffix = get_unique_suffix() - item, warehouses, batches = setup_item_valuation_test( - valuation_method="FIFO", suffix=suffix - ) + item, warehouses, batches = setup_item_valuation_test() state = { "stock_value" : 0.0, "qty": 0.0 @@ -449,8 +443,86 @@ class TestStockLedgerEntry(ERPNextTestCase): update_invariants(expected_sles) self.assertSLEs(sr2, expected_sles) + def test_batch_wise_valuation_across_warehouse(self): + item_code, warehouses, batches = setup_item_valuation_test() + source = warehouses[0] + target = warehouses[1] + + unrelated_batch = make_stock_entry(item_code=item_code, target=source, batch_no=batches[1], + qty=5, rate=10) + self.assertSLEs(unrelated_batch, [ + {"actual_qty": 5, "stock_value_difference": 10 * 5}, + ]) + + reciept = make_stock_entry(item_code=item_code, target=source, batch_no=batches[0], qty=5, rate=10) + self.assertSLEs(reciept, [ + {"actual_qty": 5, "stock_value_difference": 10 * 5}, + ]) + + transfer = make_stock_entry(item_code=item_code, source=source, target=target, batch_no=batches[0], qty=5) + self.assertSLEs(transfer, [ + {"actual_qty": -5, "stock_value_difference": -10 * 5, "warehouse": source}, + {"actual_qty": 5, "stock_value_difference": 10 * 5, "warehouse": target} + ]) + + backdated_receipt = make_stock_entry(item_code=item_code, target=source, batch_no=batches[0], + qty=5, rate=20, posting_date=add_days(today(), -1)) + self.assertSLEs(backdated_receipt, [ + {"actual_qty": 5, "stock_value_difference": 20 * 5}, + ]) + + # check reposted average rate in *future* transfer + self.assertSLEs(transfer, [ + {"actual_qty": -5, "stock_value_difference": -15 * 5, "warehouse": source, "stock_value": 15 * 5 + 10 * 5}, + {"actual_qty": 5, "stock_value_difference": 15 * 5, "warehouse": target, "stock_value": 15 * 5} + ]) + + transfer_unrelated = make_stock_entry(item_code=item_code, source=source, + target=target, batch_no=batches[1], qty=5) + self.assertSLEs(transfer_unrelated, [ + {"actual_qty": -5, "stock_value_difference": -10 * 5, "warehouse": source, "stock_value": 15 * 5}, + {"actual_qty": 5, "stock_value_difference": 10 * 5, "warehouse": target, "stock_value": 15 * 5 + 10 * 5} + ]) + + def test_intermediate_average_batch_wise_valuation(self): + """ A batch has moving average up until posting time, + check if same is respected when backdated entry is inserted in middle""" + item_code, warehouses, batches = setup_item_valuation_test() + warehouse = warehouses[0] + + batch = batches[0] + + yesterday = make_stock_entry(item_code=item_code, target=warehouse, batch_no=batch, + qty=1, rate=10, posting_date=add_days(today(), -1)) + self.assertSLEs(yesterday, [ + {"actual_qty": 1, "stock_value_difference": 10}, + ]) + + tomorrow = make_stock_entry(item_code=item_code, target=warehouse, batch_no=batches[0], + qty=1, rate=30, posting_date=add_days(today(), 1)) + self.assertSLEs(tomorrow, [ + {"actual_qty": 1, "stock_value_difference": 30}, + ]) + + create_today = make_stock_entry(item_code=item_code, target=warehouse, batch_no=batches[0], + qty=1, rate=20) + self.assertSLEs(create_today, [ + {"actual_qty": 1, "stock_value_difference": 20}, + ]) + + consume_today = make_stock_entry(item_code=item_code, source=warehouse, batch_no=batches[0], + qty=1) + self.assertSLEs(consume_today, [ + {"actual_qty": -1, "stock_value_difference": -15}, + ]) + + consume_tomorrow = make_stock_entry(item_code=item_code, source=warehouse, batch_no=batches[0], + qty=2, posting_date=add_days(today(), 2)) + self.assertSLEs(consume_tomorrow, [ + {"stock_value_difference": -(30 + 15), "stock_value": 0, "qty_after_transaction": 0}, + ]) + def test_legacy_item_valuation_stock_entry(self): - suffix = get_unique_suffix() columns = [ 'stock_value_difference', 'stock_value', @@ -458,9 +530,7 @@ class TestStockLedgerEntry(ERPNextTestCase): 'qty_after_transaction', 'stock_queue', ] - item, warehouses, batches = setup_item_valuation_test( - valuation_method="FIFO", suffix=suffix, use_batchwise_valuation=0 - ) + item, warehouses, batches = setup_item_valuation_test(use_batchwise_valuation=0) def check_sle_details_against_expected(sle_details, expected_sle_details, detail, columns): for i, (sle_vals, ex_sle_vals) in enumerate(zip(sle_details, expected_sle_details)): @@ -581,11 +651,14 @@ def create_items(): return items -def setup_item_valuation_test(valuation_method, suffix, use_batchwise_valuation=1, batches_list=['X', 'Y']): +def setup_item_valuation_test(valuation_method="FIFO", suffix=None, use_batchwise_valuation=1, batches_list=['X', 'Y']): from erpnext.stock.doctype.batch.batch import make_batch from erpnext.stock.doctype.item.test_item import make_item from erpnext.stock.doctype.warehouse.test_warehouse import create_warehouse + if not suffix: + suffix = get_unique_suffix() + item = make_item( f"IV - Test Item {valuation_method} {suffix}", dict(valuation_method=valuation_method, has_batch_no=1) From c5bd34d2383982e99db825cef1b5ec8215ccabee Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 19:21:12 +0530 Subject: [PATCH 23/44] test: multi-batch stock entry --- .../doctype/stock_entry/test_stock_entry.py | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/erpnext/stock/doctype/stock_entry/test_stock_entry.py b/erpnext/stock/doctype/stock_entry/test_stock_entry.py index 306f2c3e69f..6c6513beff5 100644 --- a/erpnext/stock/doctype/stock_entry/test_stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/test_stock_entry.py @@ -1107,6 +1107,52 @@ class TestStockEntry(ERPNextTestCase): posting_date='2021-09-02', # backdated consumption of 2nd batch purpose='Material Issue') + def test_multi_batch_value_diff(self): + """ Test value difference on stock entry in case of multi-batch. + | Stock entry | batch | qty | rate | value diff on SE | + | --- | --- | --- | --- | --- | + | receipt | A | 1 | 10 | 30 | + | receipt | B | 1 | 20 | | + | issue | A | -1 | 10 | -30 (to assert after submit) | + | issue | B | -1 | 20 | | + """ + from erpnext.stock.doctype.batch.test_batch import TestBatch + + batch_nos = [] + + item_code = '_TestMultibatchFifo' + TestBatch.make_batch_item(item_code) + warehouse = '_Test Warehouse - _TC' + receipt = make_stock_entry( + item_code=item_code, + qty=1, + rate=10, + to_warehouse=warehouse, + purpose='Material Receipt', + do_not_save=True + ) + receipt.append("items", frappe.copy_doc(receipt.items[0], ignore_no_copy=False).update({"basic_rate": 20}) ) + receipt.save() + receipt.submit() + batch_nos.extend(row.batch_no for row in receipt.items) + self.assertEqual(receipt.value_difference, 30) + + issue = make_stock_entry( + item_code=item_code, + qty=1, + from_warehouse=warehouse, + purpose='Material Issue', + do_not_save=True + ) + issue.append("items", frappe.copy_doc(issue.items[0], ignore_no_copy=False)) + for row, batch_no in zip(issue.items, batch_nos): + row.batch_no = batch_no + issue.save() + issue.submit() + + issue.reload() # reload because reposting current voucher updates rate + self.assertEqual(issue.value_difference, -30) + def make_serialized_item(**args): args = frappe._dict(args) se = frappe.copy_doc(test_records[0]) From d7ca83ef0b42af42bca94e43c18c26cbf8e19ed3 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 19:35:33 +0530 Subject: [PATCH 24/44] refactor: code duplication for fallback rates --- erpnext/stock/stock_ledger.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index 2dd26643f74..9339b3ea233 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -633,9 +633,7 @@ class update_entries_after(object): if not self.wh_data.valuation_rate and sle.voucher_detail_no: allow_zero_rate = self.check_if_allow_zero_valuation_rate(sle.voucher_type, sle.voucher_detail_no) if not allow_zero_rate: - self.wh_data.valuation_rate = get_valuation_rate(sle.item_code, sle.warehouse, - sle.voucher_type, sle.voucher_no, self.allow_zero_rate, - currency=erpnext.get_company_currency(sle.company), company=sle.company, batch_no=sle.batch_no) + self.wh_data.valuation_rate = self.get_fallback_rate(sle) def get_incoming_value_for_serial_nos(self, sle, serial_nos): # get rate from serial nos within same company @@ -701,9 +699,7 @@ class update_entries_after(object): if not self.wh_data.valuation_rate and sle.voucher_detail_no: allow_zero_valuation_rate = self.check_if_allow_zero_valuation_rate(sle.voucher_type, sle.voucher_detail_no) if not allow_zero_valuation_rate: - self.wh_data.valuation_rate = get_valuation_rate(sle.item_code, sle.warehouse, - sle.voucher_type, sle.voucher_no, self.allow_zero_rate, - currency=erpnext.get_company_currency(sle.company), company=sle.company, batch_no=sle.batch_no) + self.wh_data.valuation_rate = self.get_fallback_rate(sle) def update_queue_values(self, sle): incoming_rate = flt(sle.incoming_rate) @@ -721,9 +717,7 @@ class update_entries_after(object): def rate_generator() -> float: allow_zero_valuation_rate = self.check_if_allow_zero_valuation_rate(sle.voucher_type, sle.voucher_detail_no) if not allow_zero_valuation_rate: - return get_valuation_rate(sle.item_code, sle.warehouse, - sle.voucher_type, sle.voucher_no, self.allow_zero_rate, - currency=erpnext.get_company_currency(sle.company), company=sle.company, batch_no=sle.batch_no) + return self.get_fallback_rate(sle) else: return 0.0 @@ -771,6 +765,13 @@ class update_entries_after(object): else: return 0 + def get_fallback_rate(self, sle) -> float: + """When exact incoming rate isn't available use any of other "average" rates as fallback. + This should only get used for negative stock.""" + return get_valuation_rate(sle.item_code, sle.warehouse, + sle.voucher_type, sle.voucher_no, self.allow_zero_rate, + currency=erpnext.get_company_currency(sle.company), company=sle.company, batch_no=sle.batch_no) + def get_sle_before_datetime(self, args): """get previous stock ledger entry before current time-bucket""" sle = get_stock_ledger_entries(args, "<", "desc", "limit 1", for_update=False) From aba7a7ce4e4dc1fb264023db0034df5e906b5571 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 19:36:28 +0530 Subject: [PATCH 25/44] fix: handle negative inventory inside a batch --- erpnext/stock/stock_ledger.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index 9339b3ea233..edbe7553298 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -742,13 +742,17 @@ class update_entries_after(object): if actual_qty > 0: stock_value_difference = incoming_rate * actual_qty - self.wh_data.stock_value += stock_value_difference else: outgoing_rate = get_batch_incoming_rate(item_code=sle.item_code, warehouse=sle.warehouse, batch_no=sle.batch_no, posting_date=sle.posting_date, posting_time=sle.posting_time, creation=sle.creation) - # TODO: negative stock handling + if outgoing_rate is None: + # This can *only* happen if qty available for the batch is zero. + # in such case fall back various other rates. + # future entries will correct the overall accounting as each + # batch individually uses moving average rates. + outgoing_rate = self.get_fallback_rate(sle) stock_value_difference = outgoing_rate * actual_qty - self.wh_data.stock_value += stock_value_difference + self.wh_data.stock_value += stock_value_difference if self.wh_data.qty_after_transaction: self.wh_data.valuation_rate = self.wh_data.stock_value / self.wh_data.qty_after_transaction From b534fee2c7220390ed749d9ee87759663558a019 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 20:58:36 +0530 Subject: [PATCH 26/44] refactor: use queue difference instead of actual values --- erpnext/stock/stock_ledger.py | 19 ++++++++++++------- erpnext/stock/tests/test_valuation.py | 12 ++++++------ erpnext/stock/valuation.py | 12 ++++++------ 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index edbe7553298..677266ee0cd 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -19,7 +19,7 @@ from erpnext.stock.utils import ( get_or_make_bin, get_valuation_method, ) -from erpnext.stock.valuation import FIFOValuation, LIFOValuation +from erpnext.stock.valuation import FIFOValuation, LIFOValuation, round_off_if_near_zero class NegativeStockError(frappe.ValidationError): pass @@ -465,7 +465,6 @@ class update_entries_after(object): self.wh_data.stock_value = flt(self.wh_data.qty_after_transaction) * flt(self.wh_data.valuation_rate) else: self.update_queue_values(sle) - self.wh_data.qty_after_transaction += flt(sle.actual_qty) # rounding as per precision self.wh_data.stock_value = flt(self.wh_data.stock_value, self.precision) @@ -706,11 +705,15 @@ class update_entries_after(object): actual_qty = flt(sle.actual_qty) outgoing_rate = flt(sle.outgoing_rate) + self.wh_data.qty_after_transaction = round_off_if_near_zero(self.wh_data.qty_after_transaction + actual_qty) + if self.valuation_method == "LIFO": stock_queue = LIFOValuation(self.wh_data.stock_queue) else: stock_queue = FIFOValuation(self.wh_data.stock_queue) + _prev_qty, prev_stock_value = stock_queue.get_total_stock_and_value() + if actual_qty > 0: stock_queue.add_stock(qty=actual_qty, rate=incoming_rate) else: @@ -723,17 +726,19 @@ class update_entries_after(object): stock_queue.remove_stock(qty=abs(actual_qty), outgoing_rate=outgoing_rate, rate_generator=rate_generator) - stock_qty, stock_value = stock_queue.get_total_stock_and_value() + _qty, stock_value = stock_queue.get_total_stock_and_value() + + stock_value_difference = stock_value - prev_stock_value self.wh_data.stock_queue = stock_queue.state - self.wh_data.stock_value = stock_value - if stock_qty: - self.wh_data.valuation_rate = stock_value / stock_qty - + self.wh_data.stock_value = round_off_if_near_zero(self.wh_data.stock_value + stock_value_difference) if not self.wh_data.stock_queue: self.wh_data.stock_queue.append([0, sle.incoming_rate or sle.outgoing_rate or self.wh_data.valuation_rate]) + if self.wh_data.qty_after_transaction: + self.wh_data.valuation_rate = self.wh_data.stock_value / self.wh_data.qty_after_transaction + def update_batched_values(self, sle): incoming_rate = flt(sle.incoming_rate) actual_qty = flt(sle.actual_qty) diff --git a/erpnext/stock/tests/test_valuation.py b/erpnext/stock/tests/test_valuation.py index 648d4406ca9..bdb768f1ade 100644 --- a/erpnext/stock/tests/test_valuation.py +++ b/erpnext/stock/tests/test_valuation.py @@ -7,7 +7,7 @@ from hypothesis import strategies as st from erpnext.stock.doctype.item.test_item import make_item from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry -from erpnext.stock.valuation import FIFOValuation, LIFOValuation, _round_off_if_near_zero +from erpnext.stock.valuation import FIFOValuation, LIFOValuation, round_off_if_near_zero from erpnext.tests.utils import ERPNextTestCase qty_gen = st.floats(min_value=-1e6, max_value=1e6) @@ -113,11 +113,11 @@ class TestFIFOValuation(unittest.TestCase): self.assertTotalQty(0) def test_rounding_off_near_zero(self): - self.assertEqual(_round_off_if_near_zero(0), 0) - self.assertEqual(_round_off_if_near_zero(1), 1) - self.assertEqual(_round_off_if_near_zero(-1), -1) - self.assertEqual(_round_off_if_near_zero(-1e-8), 0) - self.assertEqual(_round_off_if_near_zero(1e-8), 0) + self.assertEqual(round_off_if_near_zero(0), 0) + self.assertEqual(round_off_if_near_zero(1), 1) + self.assertEqual(round_off_if_near_zero(-1), -1) + self.assertEqual(round_off_if_near_zero(-1e-8), 0) + self.assertEqual(round_off_if_near_zero(1e-8), 0) def test_totals(self): self.queue.add_stock(1, 10) diff --git a/erpnext/stock/valuation.py b/erpnext/stock/valuation.py index ee9477ed74b..e2bd1ad4dfe 100644 --- a/erpnext/stock/valuation.py +++ b/erpnext/stock/valuation.py @@ -34,7 +34,7 @@ class BinWiseValuation(ABC): total_qty += flt(qty) total_value += flt(qty) * flt(rate) - return _round_off_if_near_zero(total_qty), _round_off_if_near_zero(total_value) + return round_off_if_near_zero(total_qty), round_off_if_near_zero(total_value) def __repr__(self): return str(self.state) @@ -136,7 +136,7 @@ class FIFOValuation(BinWiseValuation): fifo_bin = self.queue[index] if qty >= fifo_bin[QTY]: # consume current bin - qty = _round_off_if_near_zero(qty - fifo_bin[QTY]) + qty = round_off_if_near_zero(qty - fifo_bin[QTY]) to_consume = self.queue.pop(index) consumed_bins.append(list(to_consume)) @@ -148,7 +148,7 @@ class FIFOValuation(BinWiseValuation): break else: # qty found in current bin consume it and exit - fifo_bin[QTY] = _round_off_if_near_zero(fifo_bin[QTY] - qty) + fifo_bin[QTY] = round_off_if_near_zero(fifo_bin[QTY] - qty) consumed_bins.append([qty, fifo_bin[RATE]]) qty = 0 @@ -231,7 +231,7 @@ class LIFOValuation(BinWiseValuation): stock_bin = self.stack[index] if qty >= stock_bin[QTY]: # consume current bin - qty = _round_off_if_near_zero(qty - stock_bin[QTY]) + qty = round_off_if_near_zero(qty - stock_bin[QTY]) to_consume = self.stack.pop(index) consumed_bins.append(list(to_consume)) @@ -243,14 +243,14 @@ class LIFOValuation(BinWiseValuation): break else: # qty found in current bin consume it and exit - stock_bin[QTY] = _round_off_if_near_zero(stock_bin[QTY] - qty) + stock_bin[QTY] = round_off_if_near_zero(stock_bin[QTY] - qty) consumed_bins.append([qty, stock_bin[RATE]]) qty = 0 return consumed_bins -def _round_off_if_near_zero(number: float, precision: int = 7) -> float: +def round_off_if_near_zero(number: float, precision: int = 7) -> float: """Rounds off the number to zero only if number is close to zero for decimal specified in precision. Precision defaults to 7. """ From b1555fd477923a968a203c2fde68e754777a1e08 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 21:21:39 +0530 Subject: [PATCH 27/44] chore: batch flag and consumption rate in invariant report --- .../stock_ledger_invariant_check.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/erpnext/stock/report/stock_ledger_invariant_check/stock_ledger_invariant_check.py b/erpnext/stock/report/stock_ledger_invariant_check/stock_ledger_invariant_check.py index cb35bf75d10..7826d344225 100644 --- a/erpnext/stock/report/stock_ledger_invariant_check/stock_ledger_invariant_check.py +++ b/erpnext/stock/report/stock_ledger_invariant_check/stock_ledger_invariant_check.py @@ -60,6 +60,9 @@ def add_invariant_check_fields(sles): fifo_qty += qty fifo_value += qty * rate + if sle.actual_qty < 0: + sle.consumption_rate = sle.stock_value_difference / sle.actual_qty + balance_qty += sle.actual_qty balance_stock_value += sle.stock_value_difference if sle.voucher_type == "Stock Reconciliation" and not sle.batch_no: @@ -90,6 +93,9 @@ def add_invariant_check_fields(sles): sle.fifo_stock_diff = sle.fifo_stock_value - sles[idx - 1].fifo_stock_value sle.fifo_difference_diff = sle.fifo_stock_diff - sle.stock_value_difference + if sle.batch_no: + sle.use_batchwise_valuation = frappe.db.get_value("Batch", sle.batch_no, "use_batchwise_valuation", cache=True) + return sles @@ -134,6 +140,11 @@ def get_columns(): "label": "Batch", "options": "Batch", }, + { + "fieldname": "use_batchwise_valuation", + "fieldtype": "Check", + "label": "Batchwise Valuation", + }, { "fieldname": "actual_qty", "fieldtype": "Float", @@ -145,9 +156,9 @@ def get_columns(): "label": "Incoming Rate", }, { - "fieldname": "outgoing_rate", + "fieldname": "consumption_rate", "fieldtype": "Float", - "label": "Outgoing Rate", + "label": "Consumption Rate", }, { "fieldname": "qty_after_transaction", From 76b395d62ee5f9ffb96e3c3e4920fa6eebaec175 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 22:01:34 +0530 Subject: [PATCH 28/44] test: old/new mix batches valuation consumption --- .../test_stock_ledger_entry.py | 83 ++++++++++++++++++- 1 file changed, 81 insertions(+), 2 deletions(-) diff --git a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py index c298b5a0963..b0df45ffd40 100644 --- a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py +++ b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py @@ -397,7 +397,15 @@ class TestStockLedgerEntry(ERPNextTestCase): for exp_sle, act_sle in zip(expected_sles, sles): for k, v in exp_sle.items(): - self.assertEqual(v, act_sle[k], msg=f"{k} doesn't match \n{exp_sle}\n{act_sle}") + act_value = act_sle[k] + if k == "stock_queue": + act_value = json.loads(act_value) + if act_value and act_value[0][0] == 0: + # ignore empty fifo bins + continue + + self.assertEqual(v, act_value, msg=f"{k} doesn't match \n{exp_sle}\n{act_sle}") + def test_batchwise_item_valuation_stock_reco(self): item, warehouses, batches = setup_item_valuation_test() @@ -587,6 +595,77 @@ class TestStockLedgerEntry(ERPNextTestCase): for details in details_list: check_sle_details_against_expected(*details) + def test_mixed_valuation_batches(self): + item_code, warehouses, batches = setup_item_valuation_test(use_batchwise_valuation=0) + warehouse = warehouses[0] + + state = { + "qty": 0.0, + "stock_value": 0.0 + } + def update_invariants(exp_sles): + for sle in exp_sles: + state["stock_value"] += sle["stock_value_difference"] + state["qty"] += sle["actual_qty"] + sle["stock_value"] = state["stock_value"] + sle["qty_after_transaction"] = state["qty"] + return exp_sles + + old1 = make_stock_entry(item_code=item_code, target=warehouse, batch_no=batches[0], + qty=10, rate=10) + self.assertSLEs(old1, update_invariants([ + {"actual_qty": 10, "stock_value_difference": 10*10, "stock_queue": [[10, 10]]}, + ])) + old2 = make_stock_entry(item_code=item_code, target=warehouse, batch_no=batches[1], + qty=10, rate=20) + self.assertSLEs(old2, update_invariants([ + {"actual_qty": 10, "stock_value_difference": 10*20, "stock_queue": [[10, 10], [10, 20]]}, + ])) + old3 = make_stock_entry(item_code=item_code, target=warehouse, batch_no=batches[0], + qty=5, rate=15) + + self.assertSLEs(old3, update_invariants([ + {"actual_qty": 5, "stock_value_difference": 5*15, "stock_queue": [[10, 10], [10, 20], [5, 15]]}, + ])) + + new1 = make_stock_entry(item_code=item_code, target=warehouse, qty=10, rate=40) + batches.append(new1.items[0].batch_no) + # assert old queue remains + self.assertSLEs(new1, update_invariants([ + {"actual_qty": 10, "stock_value_difference": 10*40, "stock_queue": [[10, 10], [10, 20], [5, 15]]}, + ])) + + new2 = make_stock_entry(item_code=item_code, target=warehouse, qty=10, rate=42) + batches.append(new2.items[0].batch_no) + self.assertSLEs(new2, update_invariants([ + {"actual_qty": 10, "stock_value_difference": 10*42, "stock_queue": [[10, 10], [10, 20], [5, 15]]}, + ])) + + # consume old batch as per FIFO + consume_old1 = make_stock_entry(item_code=item_code, source=warehouse, qty=15, batch_no=batches[0]) + self.assertSLEs(consume_old1, update_invariants([ + {"actual_qty": -15, "stock_value_difference": -10*10 - 5*20, "stock_queue": [[5, 20], [5, 15]]}, + ])) + + # consume new batch as per batch + consume_new2 = make_stock_entry(item_code=item_code, source=warehouse, qty=10, batch_no=batches[-1]) + self.assertSLEs(consume_new2, update_invariants([ + {"actual_qty": -10, "stock_value_difference": -10*42, "stock_queue": [[5, 20], [5, 15]]}, + ])) + + # finish all old batches + consume_old2 = make_stock_entry(item_code=item_code, source=warehouse, qty=10, batch_no=batches[1]) + self.assertSLEs(consume_old2, update_invariants([ + {"actual_qty": -10, "stock_value_difference": -5*20 - 5*15, "stock_queue": []}, + ])) + + # finish all new batches + consume_new1 = make_stock_entry(item_code=item_code, source=warehouse, qty=10, batch_no=batches[-2]) + self.assertSLEs(consume_new1, update_invariants([ + {"actual_qty": -10, "stock_value_difference": -10*40, "stock_queue": []}, + ])) + + def create_repack_entry(**args): args = frappe._dict(args) @@ -661,7 +740,7 @@ def setup_item_valuation_test(valuation_method="FIFO", suffix=None, use_batchwis item = make_item( f"IV - Test Item {valuation_method} {suffix}", - dict(valuation_method=valuation_method, has_batch_no=1) + dict(valuation_method=valuation_method, has_batch_no=1, create_new_batch=1) ) warehouses = [create_warehouse(f"IV - Test Warehouse {i}") for i in ['J', 'K']] batches = [f"IV - Test Batch {i} {valuation_method} {suffix}" for i in batches_list] From 35483242b3864e09c635979afe7793aac7f12596 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sat, 19 Feb 2022 22:22:27 +0530 Subject: [PATCH 29/44] fix: extend round_off_if_near_zero fix to other methods --- erpnext/stock/stock_ledger.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index 677266ee0cd..de6c409d7cf 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -743,12 +743,14 @@ class update_entries_after(object): incoming_rate = flt(sle.incoming_rate) actual_qty = flt(sle.actual_qty) - self.wh_data.qty_after_transaction += actual_qty + self.wh_data.qty_after_transaction = round_off_if_near_zero(self.wh_data.qty_after_transaction + actual_qty) if actual_qty > 0: stock_value_difference = incoming_rate * actual_qty else: - outgoing_rate = get_batch_incoming_rate(item_code=sle.item_code, warehouse=sle.warehouse, batch_no=sle.batch_no, posting_date=sle.posting_date, posting_time=sle.posting_time, creation=sle.creation) + outgoing_rate = get_batch_incoming_rate(item_code=sle.item_code, + warehouse=sle.warehouse, batch_no=sle.batch_no, posting_date=sle.posting_date, + posting_time=sle.posting_time, creation=sle.creation) if outgoing_rate is None: # This can *only* happen if qty available for the batch is zero. # in such case fall back various other rates. @@ -757,7 +759,7 @@ class update_entries_after(object): outgoing_rate = self.get_fallback_rate(sle) stock_value_difference = outgoing_rate * actual_qty - self.wh_data.stock_value += stock_value_difference + self.wh_data.stock_value = round_off_if_near_zero(self.wh_data.stock_value + stock_value_difference) if self.wh_data.qty_after_transaction: self.wh_data.valuation_rate = self.wh_data.stock_value / self.wh_data.qty_after_transaction From 609d2fccad2a1b60a1e7ffd93f504f0e1329136d Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 20 Feb 2022 11:35:53 +0530 Subject: [PATCH 30/44] fix: reset stock value if no qty --- erpnext/stock/stock_ledger.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/erpnext/stock/stock_ledger.py b/erpnext/stock/stock_ledger.py index de6c409d7cf..1b90086440f 100644 --- a/erpnext/stock/stock_ledger.py +++ b/erpnext/stock/stock_ledger.py @@ -468,6 +468,8 @@ class update_entries_after(object): # rounding as per precision self.wh_data.stock_value = flt(self.wh_data.stock_value, self.precision) + if not self.wh_data.qty_after_transaction: + self.wh_data.stock_value = 0.0 stock_value_difference = self.wh_data.stock_value - self.wh_data.prev_stock_value self.wh_data.prev_stock_value = self.wh_data.stock_value From 6b0bc350636776fbec3edc254086462a7670649c Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 20 Feb 2022 12:05:58 +0530 Subject: [PATCH 31/44] test: mixed moving average items --- .../test_stock_ledger_entry.py | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py index b0df45ffd40..9e819dd658f 100644 --- a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py +++ b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py @@ -595,7 +595,7 @@ class TestStockLedgerEntry(ERPNextTestCase): for details in details_list: check_sle_details_against_expected(*details) - def test_mixed_valuation_batches(self): + def test_mixed_valuation_batches_fifo(self): item_code, warehouses, batches = setup_item_valuation_test(use_batchwise_valuation=0) warehouse = warehouses[0] @@ -665,6 +665,34 @@ class TestStockLedgerEntry(ERPNextTestCase): {"actual_qty": -10, "stock_value_difference": -10*40, "stock_queue": []}, ])) + def test_mixed_valuation_batches_moving_average(self): + item_code, warehouses, batches = setup_item_valuation_test(use_batchwise_valuation=0, valuation_method="Moving Average") + warehouse = warehouses[0] + + make_stock_entry(item_code=item_code, target=warehouse, batch_no=batches[0], + qty=10, rate=10) + make_stock_entry(item_code=item_code, target=warehouse, batch_no=batches[1], + qty=10, rate=20) + make_stock_entry(item_code=item_code, target=warehouse, batch_no=batches[0], + qty=5, rate=15) + + new1 = make_stock_entry(item_code=item_code, target=warehouse, qty=10, rate=40) + batches.append(new1.items[0].batch_no) + new2 = make_stock_entry(item_code=item_code, target=warehouse, qty=10, rate=42) + batches.append(new2.items[0].batch_no) + + # consume old batch as per FIFO + make_stock_entry(item_code=item_code, source=warehouse, qty=15, batch_no=batches[0]) + # consume new batch as per batch + make_stock_entry(item_code=item_code, source=warehouse, qty=10, batch_no=batches[-1]) + # finish all old batches + make_stock_entry(item_code=item_code, source=warehouse, qty=10, batch_no=batches[1]) + + # finish all new batches + consume_new1 = make_stock_entry(item_code=item_code, source=warehouse, qty=10, batch_no=batches[-2]) + self.assertSLEs(consume_new1, ([ + {"stock_value": 0}, + ])) def create_repack_entry(**args): From f38690f7037c75bb1c5a5d946d686b40392a111a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Sun, 20 Feb 2022 12:58:53 +0530 Subject: [PATCH 32/44] fix: check if Moving average item can use batchwise valuation --- erpnext/stock/doctype/batch/batch.py | 32 ++++++++++++++++++++++++++++ erpnext/stock/utils.py | 2 +- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/erpnext/stock/doctype/batch/batch.py b/erpnext/stock/doctype/batch/batch.py index 96751d6eae5..b5e56ad301b 100644 --- a/erpnext/stock/doctype/batch/batch.py +++ b/erpnext/stock/doctype/batch/batch.py @@ -6,6 +6,7 @@ import frappe from frappe import _ from frappe.model.document import Document from frappe.model.naming import make_autoname, revert_series_if_last +from frappe.query_builder.functions import Sum from frappe.utils import cint, flt, get_link_to_form from frappe.utils.data import add_days from frappe.utils.jinja import render_template @@ -110,11 +111,15 @@ class Batch(Document): def validate(self): self.item_has_batch_enabled() + self.set_batchwise_valuation() def item_has_batch_enabled(self): if frappe.db.get_value("Item", self.item, "has_batch_no") == 0: frappe.throw(_("The selected item cannot have Batch")) + def set_batchwise_valuation(self): + self.use_batchwise_valuation = int(can_use_batchwise_valuation(self.item)) + def before_save(self): has_expiry_date, shelf_life_in_days = frappe.db.get_value('Item', self.item, ['has_expiry_date', 'shelf_life_in_days']) if not self.expiry_date and has_expiry_date and shelf_life_in_days: @@ -338,3 +343,30 @@ def get_pos_reserved_batch_qty(filters): flt_reserved_batch_qty = flt(reserved_batch_qty[0][0]) return flt_reserved_batch_qty + +def can_use_batchwise_valuation(item_code: str) -> bool: + """ Check if item can use batchwise valuation. + + Note: Item with existing moving average batches can't use batchwise valuation + until they are exhausted. + """ + from erpnext.stock.stock_ledger import get_valuation_method + batch = frappe.qb.DocType("Batch") + + if get_valuation_method(item_code) != "Moving Average": + return True + + batch_qty = ( + frappe.qb + .from_(batch) + .select(Sum(batch.batch_qty)) + .where( + (batch.use_batchwise_valuation == 0) + & (batch.item == item_code) + ) + ).run() + + if batch_qty and batch_qty[0][0]: + return False + + return True diff --git a/erpnext/stock/utils.py b/erpnext/stock/utils.py index e2bd2f197d0..f85a04f9447 100644 --- a/erpnext/stock/utils.py +++ b/erpnext/stock/utils.py @@ -261,7 +261,7 @@ def get_valuation_method(item_code): """get valuation method from item or default""" 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" + val_method = frappe.db.get_value("Stock Settings", None, "valuation_method", cache=True) or "FIFO" return val_method def get_fifo_rate(previous_stock_queue, qty): From 75fb5616987066b83b69455b4eb59d1a715b280e Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 21 Feb 2022 11:08:57 +0530 Subject: [PATCH 33/44] test: force correct flag in test data --- .../doctype/stock_ledger_entry/test_stock_ledger_entry.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py index 9e819dd658f..c65ed2888ef 100644 --- a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py +++ b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py @@ -778,13 +778,15 @@ def setup_item_valuation_test(valuation_method="FIFO", suffix=None, use_batchwis ubw = use_batchwise_valuation if isinstance(use_batchwise_valuation, (list, tuple)): ubw = use_batchwise_valuation[i] - make_batch( - frappe._dict( + batch = frappe.get_doc(frappe._dict( + doctype="Batch", batch_id=batch_id, item=item.item_code, use_batchwise_valuation=ubw ) - ) + ).insert() + batch.use_batchwise_valuation = ubw + batch.db_update() return item.item_code, warehouses, batches From af9fa049c749c9f72f0b21a5960111cb6ec57c12 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 21 Feb 2022 12:28:19 +0530 Subject: [PATCH 34/44] fix: batchwise valuation can only be used by FIFO/LIFO --- erpnext/stock/doctype/batch/batch.py | 24 ++------------- .../test_stock_ledger_entry.py | 30 ------------------- 2 files changed, 2 insertions(+), 52 deletions(-) diff --git a/erpnext/stock/doctype/batch/batch.py b/erpnext/stock/doctype/batch/batch.py index b5e56ad301b..93e8d413677 100644 --- a/erpnext/stock/doctype/batch/batch.py +++ b/erpnext/stock/doctype/batch/batch.py @@ -6,7 +6,6 @@ import frappe from frappe import _ from frappe.model.document import Document from frappe.model.naming import make_autoname, revert_series_if_last -from frappe.query_builder.functions import Sum from frappe.utils import cint, flt, get_link_to_form from frappe.utils.data import add_days from frappe.utils.jinja import render_template @@ -347,26 +346,7 @@ def get_pos_reserved_batch_qty(filters): def can_use_batchwise_valuation(item_code: str) -> bool: """ Check if item can use batchwise valuation. - Note: Item with existing moving average batches can't use batchwise valuation - until they are exhausted. - """ + Note: Moving average valuation method can not use batch_wise_valuation.""" from erpnext.stock.stock_ledger import get_valuation_method - batch = frappe.qb.DocType("Batch") - if get_valuation_method(item_code) != "Moving Average": - return True - - batch_qty = ( - frappe.qb - .from_(batch) - .select(Sum(batch.batch_qty)) - .where( - (batch.use_batchwise_valuation == 0) - & (batch.item == item_code) - ) - ).run() - - if batch_qty and batch_qty[0][0]: - return False - - return True + return get_valuation_method(item_code) != "Moving Average" diff --git a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py index c65ed2888ef..0864ece995f 100644 --- a/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py +++ b/erpnext/stock/doctype/stock_ledger_entry/test_stock_ledger_entry.py @@ -665,36 +665,6 @@ class TestStockLedgerEntry(ERPNextTestCase): {"actual_qty": -10, "stock_value_difference": -10*40, "stock_queue": []}, ])) - def test_mixed_valuation_batches_moving_average(self): - item_code, warehouses, batches = setup_item_valuation_test(use_batchwise_valuation=0, valuation_method="Moving Average") - warehouse = warehouses[0] - - make_stock_entry(item_code=item_code, target=warehouse, batch_no=batches[0], - qty=10, rate=10) - make_stock_entry(item_code=item_code, target=warehouse, batch_no=batches[1], - qty=10, rate=20) - make_stock_entry(item_code=item_code, target=warehouse, batch_no=batches[0], - qty=5, rate=15) - - new1 = make_stock_entry(item_code=item_code, target=warehouse, qty=10, rate=40) - batches.append(new1.items[0].batch_no) - new2 = make_stock_entry(item_code=item_code, target=warehouse, qty=10, rate=42) - batches.append(new2.items[0].batch_no) - - # consume old batch as per FIFO - make_stock_entry(item_code=item_code, source=warehouse, qty=15, batch_no=batches[0]) - # consume new batch as per batch - make_stock_entry(item_code=item_code, source=warehouse, qty=10, batch_no=batches[-1]) - # finish all old batches - make_stock_entry(item_code=item_code, source=warehouse, qty=10, batch_no=batches[1]) - - # finish all new batches - consume_new1 = make_stock_entry(item_code=item_code, source=warehouse, qty=10, batch_no=batches[-2]) - self.assertSLEs(consume_new1, ([ - {"stock_value": 0}, - ])) - - def create_repack_entry(**args): args = frappe._dict(args) repack = frappe.new_doc("Stock Entry") From 9661058cc7daf9802e054f3fcd99c7852ff935a4 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 21 Feb 2022 18:16:10 +0530 Subject: [PATCH 35/44] fix: only set batchwise valuation flag if new batch --- erpnext/stock/doctype/batch/batch.json | 6 ++++-- erpnext/stock/doctype/batch/batch.py | 13 ++++--------- erpnext/stock/doctype/batch/test_batch.py | 20 ++++++++++++++++++++ 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/erpnext/stock/doctype/batch/batch.json b/erpnext/stock/doctype/batch/batch.json index 0d28ea09190..967c5729bf4 100644 --- a/erpnext/stock/doctype/batch/batch.json +++ b/erpnext/stock/doctype/batch/batch.json @@ -194,7 +194,7 @@ "fieldtype": "Column Break" }, { - "default": "1", + "default": "0", "fieldname": "use_batchwise_valuation", "fieldtype": "Check", "label": "Use Batch-wise Valuation", @@ -207,10 +207,11 @@ "image_field": "image", "links": [], "max_attachments": 5, - "modified": "2021-10-11 13:38:12.806976", + "modified": "2022-02-21 08:08:23.999236", "modified_by": "Administrator", "module": "Stock", "name": "Batch", + "naming_rule": "By fieldname", "owner": "Administrator", "permissions": [ { @@ -231,6 +232,7 @@ "quick_entry": 1, "sort_field": "modified", "sort_order": "DESC", + "states": [], "title_field": "batch_id", "track_changes": 1 } \ No newline at end of file diff --git a/erpnext/stock/doctype/batch/batch.py b/erpnext/stock/doctype/batch/batch.py index 93e8d413677..c9b4c147f1d 100644 --- a/erpnext/stock/doctype/batch/batch.py +++ b/erpnext/stock/doctype/batch/batch.py @@ -117,7 +117,10 @@ class Batch(Document): frappe.throw(_("The selected item cannot have Batch")) def set_batchwise_valuation(self): - self.use_batchwise_valuation = int(can_use_batchwise_valuation(self.item)) + from erpnext.stock.stock_ledger import get_valuation_method + + if self.is_new() and get_valuation_method(self.item) != "Moving Average": + self.use_batchwise_valuation = 1 def before_save(self): has_expiry_date, shelf_life_in_days = frappe.db.get_value('Item', self.item, ['has_expiry_date', 'shelf_life_in_days']) @@ -342,11 +345,3 @@ def get_pos_reserved_batch_qty(filters): flt_reserved_batch_qty = flt(reserved_batch_qty[0][0]) return flt_reserved_batch_qty - -def can_use_batchwise_valuation(item_code: str) -> bool: - """ Check if item can use batchwise valuation. - - Note: Moving average valuation method can not use batch_wise_valuation.""" - from erpnext.stock.stock_ledger import get_valuation_method - - return get_valuation_method(item_code) != "Moving Average" diff --git a/erpnext/stock/doctype/batch/test_batch.py b/erpnext/stock/doctype/batch/test_batch.py index 6495b56e929..baa03024af1 100644 --- a/erpnext/stock/doctype/batch/test_batch.py +++ b/erpnext/stock/doctype/batch/test_batch.py @@ -6,6 +6,7 @@ import json import frappe from frappe.exceptions import ValidationError from frappe.utils import cint, flt +from frappe.utils.data import add_to_date, getdate from erpnext.accounts.doctype.purchase_invoice.test_purchase_invoice import make_purchase_invoice from erpnext.stock.doctype.batch.batch import UnableToSelectBatchError, get_batch_no, get_batch_qty @@ -387,6 +388,25 @@ class TestBatch(ERPNextTestCase): assertValuation((20 * 20 + 10 * 25) / (10 + 20)) + def test_update_batch_properties(self): + item_code = "_TestBatchWiseVal" + self.make_batch_item(item_code) + + se = make_stock_entry(item_code=item_code, qty=100, rate=10, target="_Test Warehouse - _TC") + batch_no = se.items[0].batch_no + batch = frappe.get_doc("Batch", batch_no) + + expiry_date = add_to_date(batch.manufacturing_date, days=30) + + batch.expiry_date = expiry_date + batch.save() + + batch.reload() + + self.assertEqual(getdate(batch.expiry_date), getdate(expiry_date)) + + + def create_batch(item_code, rate, create_item_price_for_batch): pi = make_purchase_invoice(company="_Test Company", warehouse= "Stores - _TC", cost_center = "Main - _TC", update_stock=1, From e4c4dc402e75d3ec501095fa3e914553fcd07a4d Mon Sep 17 00:00:00 2001 From: Sagar Sharma Date: Mon, 21 Feb 2022 19:49:19 +0530 Subject: [PATCH 36/44] fix: JobCard TimeLog to_date (#29872) --- erpnext/manufacturing/doctype/job_card/job_card.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/manufacturing/doctype/job_card/job_card.py b/erpnext/manufacturing/doctype/job_card/job_card.py index 8d00019b7d6..9f4ace296e8 100644 --- a/erpnext/manufacturing/doctype/job_card/job_card.py +++ b/erpnext/manufacturing/doctype/job_card/job_card.py @@ -62,7 +62,7 @@ class JobCard(Document): if self.get('time_logs'): for d in self.get('time_logs'): - if get_datetime(d.from_time) > get_datetime(d.to_time): + if d.to_time and get_datetime(d.from_time) > get_datetime(d.to_time): frappe.throw(_("Row {0}: From time must be less than to time").format(d.idx)) data = self.get_overlap_for(d) From 87b59fc96c7bb37fcfbce097bd7c8184fce967ba Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Mon, 21 Feb 2022 22:53:29 +0530 Subject: [PATCH 37/44] fix(LMS): program enrollment does not give any feedback (#29922) --- erpnext/www/lms/macros/hero.html | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/erpnext/www/lms/macros/hero.html b/erpnext/www/lms/macros/hero.html index e72bfc8175b..95ba8f7df28 100644 --- a/erpnext/www/lms/macros/hero.html +++ b/erpnext/www/lms/macros/hero.html @@ -11,7 +11,7 @@ {% if frappe.session.user == 'Guest' %} {{_('Sign Up')}} {% elif not has_access %} - + {% endif %}

@@ -20,34 +20,35 @@ From 4738367d6407e9ffc22ba2c9ef1649573608be50 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Mon, 21 Feb 2022 22:54:46 +0530 Subject: [PATCH 38/44] fix: boarding task dates not set when activity begins on is set to 0 (#29921) --- .../employee_boarding_controller.py | 4 +-- .../test_employee_onboarding.py | 32 +++++++++++++------ .../doctype/salary_slip/test_salary_slip.py | 6 ++-- 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/erpnext/controllers/employee_boarding_controller.py b/erpnext/controllers/employee_boarding_controller.py index ae2c73758cb..dd02ce17487 100644 --- a/erpnext/controllers/employee_boarding_controller.py +++ b/erpnext/controllers/employee_boarding_controller.py @@ -104,11 +104,11 @@ class EmployeeBoardingController(Document): def get_task_dates(self, activity, holiday_list): start_date = end_date = None - if activity.begin_on: + if activity.begin_on is not None: start_date = add_days(self.boarding_begins_on, activity.begin_on) start_date = self.update_if_holiday(start_date, holiday_list) - if activity.duration: + if activity.duration is not None: end_date = add_days(self.boarding_begins_on, activity.begin_on + activity.duration) end_date = self.update_if_holiday(end_date, holiday_list) diff --git a/erpnext/hr/doctype/employee_onboarding/test_employee_onboarding.py b/erpnext/hr/doctype/employee_onboarding/test_employee_onboarding.py index 2d129c8acfc..0fb821ddb2b 100644 --- a/erpnext/hr/doctype/employee_onboarding/test_employee_onboarding.py +++ b/erpnext/hr/doctype/employee_onboarding/test_employee_onboarding.py @@ -4,7 +4,7 @@ import unittest import frappe -from frappe.utils import getdate +from frappe.utils import add_days, getdate from erpnext.hr.doctype.employee_onboarding.employee_onboarding import ( IncompleteTaskError, @@ -35,6 +35,15 @@ class TestEmployeeOnboarding(unittest.TestCase): # boarding status self.assertEqual(onboarding.boarding_status, 'Pending') + # start and end dates + start_date, end_date = frappe.db.get_value('Task', onboarding.activities[0].task, ['exp_start_date', 'exp_end_date']) + self.assertEqual(getdate(start_date), getdate(onboarding.boarding_begins_on)) + self.assertEqual(getdate(end_date), add_days(start_date, onboarding.activities[0].duration)) + + start_date, end_date = frappe.db.get_value('Task', onboarding.activities[1].task, ['exp_start_date', 'exp_end_date']) + self.assertEqual(getdate(start_date), add_days(onboarding.boarding_begins_on, onboarding.activities[0].duration)) + self.assertEqual(getdate(end_date), add_days(start_date, onboarding.activities[1].duration)) + # complete the task project = frappe.get_doc('Project', onboarding.project) for task in frappe.get_all('Task', dict(project=project.name)): @@ -57,10 +66,7 @@ class TestEmployeeOnboarding(unittest.TestCase): self.assertEqual(employee.employee_name, 'Test Researcher') def tearDown(self): - for entry in frappe.get_all('Employee Onboarding'): - doc = frappe.get_doc('Employee Onboarding', entry.name) - doc.cancel() - doc.delete() + frappe.db.rollback() def get_job_applicant(): @@ -87,23 +93,31 @@ def get_job_offer(applicant_name): def create_employee_onboarding(): applicant = get_job_applicant() job_offer = get_job_offer(applicant.name) - holiday_list = make_holiday_list() + + holiday_list = make_holiday_list('_Test Employee Boarding') + holiday_list = frappe.get_doc('Holiday List', holiday_list) + holiday_list.holidays = [] + holiday_list.save() onboarding = frappe.new_doc('Employee Onboarding') onboarding.job_applicant = applicant.name onboarding.job_offer = job_offer.name onboarding.date_of_joining = onboarding.boarding_begins_on = getdate() onboarding.company = '_Test Company' - onboarding.holiday_list = holiday_list + onboarding.holiday_list = holiday_list.name onboarding.designation = 'Researcher' onboarding.append('activities', { 'activity_name': 'Assign ID Card', 'role': 'HR User', - 'required_for_employee_creation': 1 + 'required_for_employee_creation': 1, + 'begin_on': 0, + 'duration': 1 }) onboarding.append('activities', { 'activity_name': 'Assign a laptop', - 'role': 'HR User' + 'role': 'HR User', + 'begin_on': 1, + 'duration': 1 }) onboarding.status = 'Pending' onboarding.insert() diff --git a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py index daa0f8952bc..6a5debf9984 100644 --- a/erpnext/payroll/doctype/salary_slip/test_salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/test_salary_slip.py @@ -1019,13 +1019,13 @@ def setup_test(): frappe.db.set_value('HR Settings', None, 'leave_status_notification_template', None) frappe.db.set_value('HR Settings', None, 'leave_approval_notification_template', None) -def make_holiday_list(): +def make_holiday_list(holiday_list_name=None): fiscal_year = get_fiscal_year(nowdate(), company=erpnext.get_default_company()) - holiday_list = frappe.db.exists("Holiday List", "Salary Slip Test Holiday List") + holiday_list = frappe.db.exists("Holiday List", holiday_list_name or "Salary Slip Test Holiday List") if not holiday_list: holiday_list = frappe.get_doc({ "doctype": "Holiday List", - "holiday_list_name": "Salary Slip Test Holiday List", + "holiday_list_name": holiday_list_name or "Salary Slip Test Holiday List", "from_date": fiscal_year[1], "to_date": fiscal_year[2], "weekly_off": "Sunday" From d011a3f82c5cf9c1dc4fe0561194d47cff6099d0 Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Tue, 22 Feb 2022 11:41:09 +0530 Subject: [PATCH 39/44] fix(Salary Slip): TypeError while clearing any amount field in components (#29931) --- erpnext/payroll/doctype/salary_slip/salary_slip.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/payroll/doctype/salary_slip/salary_slip.py b/erpnext/payroll/doctype/salary_slip/salary_slip.py index f727ff4378d..d2a39989a61 100644 --- a/erpnext/payroll/doctype/salary_slip/salary_slip.py +++ b/erpnext/payroll/doctype/salary_slip/salary_slip.py @@ -1268,7 +1268,7 @@ class SalarySlip(TransactionBase): for i, earning in enumerate(self.earnings): if earning.salary_component == salary_component: self.earnings[i].amount = wages_amount - self.gross_pay += self.earnings[i].amount + self.gross_pay += flt(self.earnings[i].amount, earning.precision("amount")) self.net_pay = flt(self.gross_pay) - flt(self.total_deduction) def compute_year_to_date(self): From 235fc127b3ecf943176ed9c208425f9bda100798 Mon Sep 17 00:00:00 2001 From: Marica Date: Tue, 22 Feb 2022 12:53:46 +0530 Subject: [PATCH 40/44] fix: Fetch conversion factor even if it already existed in row, on item change (#29917) * fix: Fetch conversion factor even if it already existed in row, on item change * fix: Retain manually changed conversion factor - If item code changes, reset conversion factor on client side - Keep API behavious consistent, if conversion factor is sent, same must come back - API should not ideally reset values in most cases --- erpnext/public/js/controllers/transaction.js | 1 + 1 file changed, 1 insertion(+) diff --git a/erpnext/public/js/controllers/transaction.js b/erpnext/public/js/controllers/transaction.js index 933ced0bd70..ae8c0c8c6d3 100644 --- a/erpnext/public/js/controllers/transaction.js +++ b/erpnext/public/js/controllers/transaction.js @@ -525,6 +525,7 @@ erpnext.TransactionController = class TransactionController extends erpnext.taxe item.weight_per_unit = 0; item.weight_uom = ''; + item.conversion_factor = 0; if(['Sales Invoice'].includes(this.frm.doc.doctype)) { update_stock = cint(me.frm.doc.update_stock); From 7f55226a5807645db4f93c8038f1cc03a6fc0ce6 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 22 Feb 2022 16:55:43 +0530 Subject: [PATCH 41/44] fix: remove customer field value when MR is not customer provided (#29938) --- .../stock/doctype/material_request/material_request.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/erpnext/stock/doctype/material_request/material_request.py b/erpnext/stock/doctype/material_request/material_request.py index b39328f85bf..51209acb275 100644 --- a/erpnext/stock/doctype/material_request/material_request.py +++ b/erpnext/stock/doctype/material_request/material_request.py @@ -56,14 +56,13 @@ class MaterialRequest(BuyingController): if actual_so_qty and (flt(so_items[so_no][item]) + already_indented > actual_so_qty): frappe.throw(_("Material Request of maximum {0} can be made for Item {1} against Sales Order {2}").format(actual_so_qty - already_indented, item, so_no)) - # Validate - # --------------------- def validate(self): super(MaterialRequest, self).validate() self.validate_schedule_date() self.check_for_on_hold_or_closed_status('Sales Order', 'sales_order') self.validate_uom_is_integer("uom", "qty") + self.validate_material_request_type() if not self.status: self.status = "Draft" @@ -83,6 +82,12 @@ class MaterialRequest(BuyingController): self.reset_default_field_value("set_warehouse", "items", "warehouse") self.reset_default_field_value("set_from_warehouse", "items", "from_warehouse") + def validate_material_request_type(self): + """ Validate fields in accordance with selected type """ + + if self.material_request_type != "Customer Provided": + self.customer = None + def set_title(self): '''Set title as comma separated list of items''' if not self.title: From 745f7bc5f0fd014dcc837c41e2058be91166e1b4 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 22 Feb 2022 17:03:11 +0530 Subject: [PATCH 42/44] docs: add human readable specifications for stock ledger (#29308) * docs: add human readable specifications for stock ledger * docs: reposting technical implementation notes --- erpnext/stock/spec/README.md | 103 ++++++++++++++++++++++++++++++++ erpnext/stock/spec/reposting.md | 38 ++++++++++++ 2 files changed, 141 insertions(+) create mode 100644 erpnext/stock/spec/README.md create mode 100644 erpnext/stock/spec/reposting.md diff --git a/erpnext/stock/spec/README.md b/erpnext/stock/spec/README.md new file mode 100644 index 00000000000..f5a3501fe47 --- /dev/null +++ b/erpnext/stock/spec/README.md @@ -0,0 +1,103 @@ +# Implementation notes for Stock Ledger + + +## Important files + +- `stock/stock_ledger.py` +- `controllers/stock_controller.py` +- `stock/valuation.py` + +## What is in an Stock Ledger Entry (SLE)? + +Stock Ledger Entry is a single row in the Stock Ledger. It signifies some +modification of stock for a particular Item in the specified warehouse. + +- `item_code`: item for which ledger entry is made +- `warehouse`: warehouse where inventory is affected +- `actual_qty`: change in qty +- `qty_after_transaction`: quantity available after the transaction is processed +- `incoming_rate`: rate at which inventory was received. +- `is_cancelled`: if 1 then stock ledger entry is cancelled and should not be used +for any business logic except for the code that handles cancellation. +- `posting_date` & `posting_time`: Specify the temporal ordering of stock ledger + entries. Ties are broken by `creation` timestamp. +- `voucher_type`: Many transaction can create SLE, e.g. Stock Entry, Purchase + Invoice +- `voucher_no`: `name` of the transaction that created SLE +- `voucher_detail_no`: `name` of the child table row from parent transaction + that created the SLE. +- `dependant_sle_voucher_detail_no`: cross-warehouse transfers need this + reference in order to update dependent warehouse rates in case of change in + rate. +- `recalculate_rate`: if this is checked in/out rates are recomputed on + transactions. +- `valuation_rate`: current average valuation rate. +- `stock_value`: current total stock value +- `stock_value_difference`: stock value difference made between last and current + entry. This value is booked in accounting ledger. +- `stock_queue`: if FIFO/LIFO is used this represents queue/stack maintained for + computing incoming rate for inventory getting consumed. +- `batch_no`: batch no for which stock entry is made; each stock entry can only + affect one batch number. +- `serial_no`: newline separated list of serial numbers that were added (if + actual_qty > 0) or else removed. Currently multiple serial nos can have single + SLE but this will likely change in future. + + +## Implementation of Stock Ledger + +Stock Ledger Entry affects stock of combinations of (item_code, warehouse) and +optionally batch no if specified. For simplicity, lets avoid batch no. for now. + + +Stock Ledger Entry table stores stock ledger for all combinations of item_code +and warehouse. So whenever any operations are to be performed on said +item-warehouse combination stock ledger is filtered and sorted by posting +datetime. A typical query that will give you individual ledger looks like this: + +```sql +select * +from `tabStock Ledger Entry` as sle +where + is_cancelled = 0 --- cancelled entries don't affect ledger + and item_code = 'item_code' and warehouse = 'warehouse_name' +order by timestamp(posting_date, posting_time), creation +``` + +New entry is just an update to the last entry which is found by looking at last +row in the filter ledger. + + +### Serial nos + +Serial numbers do not follow any valuation method configuration and they are +consumed at rate they were produced unless they are grouped in which case they +are consumed at weighted average rate. + + +### Batch Nos + +Batches are currently NOT consumed as per batch wise valuation rate, instead +global FIFO queue for the item is used for valuation rate. + + +## Creation process of SLEs + +- SLE creation is usually triggered by Stock Transactions using a method + conventionally named `update_stock_ledger()` This might not be defined for + stock transaction and could be specified somewhere in inheritance hierarchy of + controllers. +- This method produces SLE objects which are processed by `make_sl_entries` in + `stock_ledger.py` which commits the SLE to database. +- `update_entries_after` class is used to process ONLY the inserted SLE's queue + and valuation. +- The change in qty is propagated to future entries immediately. Valuation and + queue for future entries is processed in background using repost item + valuation. + + +## Accounting impact + +- Accounting impact for stock transaction is handled by `get_gl_entries()` + method on controllers. Each transaction has different business logic for + booking the accounting impact. diff --git a/erpnext/stock/spec/reposting.md b/erpnext/stock/spec/reposting.md new file mode 100644 index 00000000000..b0d59fe9bb1 --- /dev/null +++ b/erpnext/stock/spec/reposting.md @@ -0,0 +1,38 @@ +# Stock Reposting + +Stock "reposting" is process of re-processing Stock Ledger Entry and GL Entries +in event of backdated stock transaction. + +*Backdated stock transaction*: Any stock transaction for which some +item-warehouse combination has a future transactions. + +## Why is this required? +Stock Ledger is stateful, it maintains queue, qty at any +point in time. So if you do a backdated transaction all future values change, +queues need to be re-evaluated etc. Watch Nabin and Rohit's conference +presentation for explanation: https://www.youtube.com/watch?v=mw3WAnekGIM + +## How is this implemented? +Whenever backdated transaction is detected, instead of +fully processing it while submitting, the processing is queued using "Repost +Item Valuation" doctype. Every hour a scheduled job runs and processes this +queue (for up to maximum of 25 minutes) + + +## Queue implementation +- "Repost item valuation" (RIV) is automatically submitted from backdated transactions. (check stock_controller.py) +- Draft and cancelled RIV are ignored. +- Keep filter of "submitted" documents when doing anything with RIVs. +- The default status is "Queued". +- When background job runs, it picks the oldest pending reposts and changes the status to "In Progress" and when it finishes it +changes to "Completed" +- There are two more status: "Failed" when reposting failed and "Skipped" when reposting is deemed not necessary so it's skipped. +- technical detail: Entry point for whole process is "repost_entries" function in repost_item_valuation.py + + +## How to identify broken stock data: +There are 4 major reports for checking broken stock data: +- Incorrect balance qty after the transaction - to check if the running total of qty isn't correct. +- Incorrect stock value report - to check incorrect value books in accounts for stock transactions +- Incorrect serial no valuation -specific to serial nos +- Stock ledger invariant check - combined report for checking qty, running total, queue, balance value etc From 1682a26fe69b9b3fa64293e692e79a553b842ca2 Mon Sep 17 00:00:00 2001 From: Subin Tom Date: Tue, 22 Feb 2022 17:20:48 +0530 Subject: [PATCH 43/44] fix: Taxjar minor fixes --- .../taxjar_integration.py | 46 +++++++++++-------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/erpnext/erpnext_integrations/taxjar_integration.py b/erpnext/erpnext_integrations/taxjar_integration.py index a4e21579e32..14c86d56328 100644 --- a/erpnext/erpnext_integrations/taxjar_integration.py +++ b/erpnext/erpnext_integrations/taxjar_integration.py @@ -8,10 +8,6 @@ from frappe.utils import cint, flt from erpnext import get_default_company, get_region -TAX_ACCOUNT_HEAD = frappe.db.get_single_value("TaxJar Settings", "tax_account_head") -SHIP_ACCOUNT_HEAD = frappe.db.get_single_value("TaxJar Settings", "shipping_account_head") -TAXJAR_CREATE_TRANSACTIONS = frappe.db.get_single_value("TaxJar Settings", "taxjar_create_transactions") -TAXJAR_CALCULATE_TAX = frappe.db.get_single_value("TaxJar Settings", "taxjar_calculate_tax") SUPPORTED_COUNTRY_CODES = ["AT", "AU", "BE", "BG", "CA", "CY", "CZ", "DE", "DK", "EE", "ES", "FI", "FR", "GB", "GR", "HR", "HU", "IE", "IT", "LT", "LU", "LV", "MT", "NL", "PL", "PT", "RO", "SE", "SI", "SK", "US"] @@ -35,12 +31,14 @@ def get_client(): if api_key and api_url: client = taxjar.Client(api_key=api_key, api_url=api_url) client.set_api_config('headers', { - 'x-api-version': '2020-08-07' + 'x-api-version': '2022-01-24' }) return client def create_transaction(doc, method): + TAXJAR_CREATE_TRANSACTIONS = frappe.db.get_single_value("TaxJar Settings", "taxjar_create_transactions") + """Create an order transaction in TaxJar""" if not TAXJAR_CREATE_TRANSACTIONS: @@ -51,6 +49,7 @@ def create_transaction(doc, method): if not client: return + TAX_ACCOUNT_HEAD = frappe.db.get_single_value("TaxJar Settings", "tax_account_head") sales_tax = sum([tax.tax_amount for tax in doc.taxes if tax.account_head == TAX_ACCOUNT_HEAD]) if not sales_tax: @@ -79,6 +78,7 @@ def create_transaction(doc, method): def delete_transaction(doc, method): """Delete an existing TaxJar order transaction""" + TAXJAR_CREATE_TRANSACTIONS = frappe.db.get_single_value("TaxJar Settings", "taxjar_create_transactions") if not TAXJAR_CREATE_TRANSACTIONS: return @@ -92,6 +92,8 @@ def delete_transaction(doc, method): def get_tax_data(doc): + SHIP_ACCOUNT_HEAD = frappe.db.get_single_value("TaxJar Settings", "shipping_account_head") + from_address = get_company_address_details(doc) from_shipping_state = from_address.get("state") from_country_code = frappe.db.get_value("Country", from_address.country, "code") @@ -113,20 +115,20 @@ def get_tax_data(doc): to_shipping_state = get_state_code(to_address, 'Shipping') tax_dict = { - 'from_country': from_country_code, - 'from_zip': from_address.pincode, - 'from_state': from_shipping_state, - 'from_city': from_address.city, - 'from_street': from_address.address_line1, - 'to_country': to_country_code, - 'to_zip': to_address.pincode, - 'to_city': to_address.city, - 'to_street': to_address.address_line1, - 'to_state': to_shipping_state, - 'shipping': shipping, - 'amount': doc.net_total, - 'plugin': 'erpnext', - 'line_items': line_items + "from_country": from_country_code, + "from_zip": from_address.pincode, + "from_state": from_shipping_state, + "from_city": from_address.city, + "from_street": from_address.address_line1, + "to_country": to_country_code, + "to_zip": to_address.pincode, + "to_city": to_address.city, + "to_street": to_address.address_line1, + "to_state": to_shipping_state, + "shipping": shipping, + "amount": doc.net_total, + "plugin": "erpnext", + "line_items": line_items } return tax_dict @@ -156,6 +158,9 @@ def get_line_item_dict(item, docstatus): return tax_dict def set_sales_tax(doc, method): + TAX_ACCOUNT_HEAD = frappe.db.get_single_value("TaxJar Settings", "tax_account_head") + TAXJAR_CALCULATE_TAX = frappe.db.get_single_value("TaxJar Settings", "taxjar_calculate_tax") + if not TAXJAR_CALCULATE_TAX: return @@ -206,6 +211,7 @@ def set_sales_tax(doc, method): doc.run_method("calculate_taxes_and_totals") def check_for_nexus(doc, tax_dict): + TAX_ACCOUNT_HEAD = frappe.db.get_single_value("TaxJar Settings", "tax_account_head") if not frappe.db.get_value('TaxJar Nexus', {'region_code': tax_dict["to_state"]}): for item in doc.get("items"): item.tax_collectable = flt(0) @@ -218,6 +224,8 @@ def check_for_nexus(doc, tax_dict): def check_sales_tax_exemption(doc): # if the party is exempt from sales tax, then set all tax account heads to zero + TAX_ACCOUNT_HEAD = frappe.db.get_single_value("TaxJar Settings", "tax_account_head") + sales_tax_exempted = hasattr(doc, "exempt_from_sales_tax") and doc.exempt_from_sales_tax \ or frappe.db.has_column("Customer", "exempt_from_sales_tax") \ and frappe.db.get_value("Customer", doc.customer, "exempt_from_sales_tax") From 5d403449bdcbe514c33b8807b674fd23ba24d93a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 22 Feb 2022 19:24:49 +0530 Subject: [PATCH 44/44] test: move report tests to subttest (#29945) Basically failfast=False but for sub-tests --- erpnext/accounts/test/test_reports.py | 15 ++++++++------- erpnext/manufacturing/report/test_reports.py | 15 ++++++++------- erpnext/stock/report/test_reports.py | 15 ++++++++------- 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/erpnext/accounts/test/test_reports.py b/erpnext/accounts/test/test_reports.py index 78c109ab947..4ed966dcb9d 100644 --- a/erpnext/accounts/test/test_reports.py +++ b/erpnext/accounts/test/test_reports.py @@ -39,10 +39,11 @@ class TestReports(unittest.TestCase): def test_execute_all_accounts_reports(self): """Test that all script report in stock modules are executable with supported filters""" for report, filter in REPORT_FILTER_TEST_CASES: - execute_script_report( - report_name=report, - module="Accounts", - filters=filter, - default_filters=DEFAULT_FILTERS, - optional_filters=OPTIONAL_FILTERS if filter.get("_optional") else None, - ) + with self.subTest(report=report): + execute_script_report( + report_name=report, + module="Accounts", + filters=filter, + default_filters=DEFAULT_FILTERS, + optional_filters=OPTIONAL_FILTERS if filter.get("_optional") else None, + ) diff --git a/erpnext/manufacturing/report/test_reports.py b/erpnext/manufacturing/report/test_reports.py index 9f51ded6c77..e436fdca646 100644 --- a/erpnext/manufacturing/report/test_reports.py +++ b/erpnext/manufacturing/report/test_reports.py @@ -55,10 +55,11 @@ class TestManufacturingReports(unittest.TestCase): def test_execute_all_manufacturing_reports(self): """Test that all script report in manufacturing modules are executable with supported filters""" for report, filter in REPORT_FILTER_TEST_CASES: - execute_script_report( - report_name=report, - module="Manufacturing", - filters=filter, - default_filters=DEFAULT_FILTERS, - optional_filters=OPTIONAL_FILTERS if filter.get("_optional") else None, - ) + with self.subTest(report=report): + execute_script_report( + report_name=report, + module="Manufacturing", + filters=filter, + default_filters=DEFAULT_FILTERS, + optional_filters=OPTIONAL_FILTERS if filter.get("_optional") else None, + ) diff --git a/erpnext/stock/report/test_reports.py b/erpnext/stock/report/test_reports.py index 525af40b412..76c20798bfb 100644 --- a/erpnext/stock/report/test_reports.py +++ b/erpnext/stock/report/test_reports.py @@ -73,10 +73,11 @@ class TestReports(unittest.TestCase): def test_execute_all_stock_reports(self): """Test that all script report in stock modules are executable with supported filters""" for report, filter in REPORT_FILTER_TEST_CASES: - execute_script_report( - report_name=report, - module="Stock", - filters=filter, - default_filters=DEFAULT_FILTERS, - optional_filters=OPTIONAL_FILTERS if filter.get("_optional") else None, - ) + with self.subTest(report=report): + execute_script_report( + report_name=report, + module="Stock", + filters=filter, + default_filters=DEFAULT_FILTERS, + optional_filters=OPTIONAL_FILTERS if filter.get("_optional") else None, + )