diff --git a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py index 646aec13559..25d2d6c6cee 100644 --- a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py +++ b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py @@ -330,13 +330,17 @@ def get_pricing_rule_details(args, pricing_rule): def apply_price_discount_rule(pricing_rule, item_details, args): item_details.pricing_rule_for = pricing_rule.rate_or_discount - if ((pricing_rule.margin_type == 'Amount' and pricing_rule.currency == args.currency) - or (pricing_rule.margin_type == 'Percentage')): - item_details.margin_type = pricing_rule.margin_type - item_details.margin_rate_or_amount = pricing_rule.margin_rate_or_amount - else: - item_details.margin_type = None - item_details.margin_rate_or_amount = 0.0 + for apply_on in ['Percentage', 'Amount']: + if pricing_rule.margin_type != apply_on: + continue + + field = 'margin_rate_or_amount' + if field not in item_details: + item_details.setdefault(field, 0) + item_details.setdefault('margin_type', apply_on) + + item_details[field] += (pricing_rule.get(field, 0) + if pricing_rule else args.get(field, 0)) if pricing_rule.rate_or_discount == 'Rate': pricing_rule_rate = 0.0 diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.js b/erpnext/accounts/doctype/sales_invoice/sales_invoice.js index 18a791d38ce..91ef9a0f8cc 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.js +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.js @@ -825,45 +825,43 @@ frappe.ui.form.on('Sales Invoice', { method: "erpnext.accounts.doctype.sales_invoice.sales_invoice.create_invoice_discounting", frm: frm }); - } -}) + }, -frappe.ui.form.on('Sales Invoice Timesheet', { + calculate_timesheet_totals: function(frm) { + frm.set_value("total_billing_amount", + frm.doc.timesheets.reduce((a, b) => a + (b["billing_amount"] || 0.0), 0.0)); + frm.set_value("total_billing_hours", + frm.doc.timesheets.reduce((a, b) => a + (b["billing_hours"] || 0.0), 0.0)); + } +}); + +frappe.ui.form.on("Sales Invoice Timesheet", { time_sheet: function(frm, cdt, cdn){ var d = locals[cdt][cdn]; if(d.time_sheet) { frappe.call({ method: "erpnext.projects.doctype.timesheet.timesheet.get_timesheet_data", args: { - 'name': d.time_sheet, - 'project': frm.doc.project || null + "name": d.time_sheet, + "project": frm.doc.project || null }, - callback: function(r, rt) { + callback: function(r) { if(r.message){ - data = r.message; - frappe.model.set_value(cdt, cdn, "billing_hours", data.billing_hours); - frappe.model.set_value(cdt, cdn, "billing_amount", data.billing_amount); - frappe.model.set_value(cdt, cdn, "timesheet_detail", data.timesheet_detail); - calculate_total_billing_amount(frm) + frappe.model.set_value(cdt, cdn, "billing_hours", r.message.billing_hours); + frappe.model.set_value(cdt, cdn, "billing_amount", r.message.billing_amount); + frappe.model.set_value(cdt, cdn, "timesheet_detail", r.message.timesheet_detail); + frm.trigger("calculate_timesheet_totals"); } } - }) + }); } + }, + + timesheets_remove: function(frm, cdt, cdn) { + frm.trigger("calculate_timesheet_totals"); } -}) +}); -var calculate_total_billing_amount = function(frm) { - var doc = frm.doc; - - doc.total_billing_amount = 0.0 - if(doc.timesheets) { - $.each(doc.timesheets, function(index, data){ - doc.total_billing_amount += data.billing_amount - }) - } - - refresh_field('total_billing_amount') -} var select_loyalty_program = function(frm, loyalty_programs) { var dialog = new frappe.ui.Dialog({ diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.json b/erpnext/accounts/doctype/sales_invoice/sales_invoice.json index 205d535e188..ca387be6cca 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.json +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.json @@ -69,6 +69,7 @@ "time_sheet_list", "timesheets", "total_billing_amount", + "total_billing_hours", "section_break_30", "total_qty", "base_total", @@ -1564,12 +1565,20 @@ { "fieldname": "dimension_col_break", "fieldtype": "Column Break" + }, + { + "default": "0", + "fieldname": "total_billing_hours", + "fieldtype": "Float", + "label": "Total Billing Hours", + "print_hide": 1, + "read_only": 1 } ], "icon": "fa fa-file-text", "idx": 181, "is_submittable": 1, - "modified": "2020-07-01 12:41:29.484813", + "modified": "2021-07-26 14:01:34.605644", "modified_by": "Administrator", "module": "Accounts", "name": "Sales Invoice", diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py index 3e341ff0c6d..0176db7f14c 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py @@ -683,12 +683,11 @@ class SalesInvoice(SellingController): self.calculate_billing_amount_for_timesheet() def calculate_billing_amount_for_timesheet(self): - total_billing_amount = 0.0 - for data in self.timesheets: - if data.billing_amount: - total_billing_amount += data.billing_amount + def timesheet_sum(field): + return sum((ts.get(field) or 0.0) for ts in self.timesheets) - self.total_billing_amount = total_billing_amount + self.total_billing_amount = timesheet_sum("billing_amount") + self.total_billing_hours = timesheet_sum("billing_hours") def get_warehouse(self): user_pos_profile = frappe.db.sql("""select name, warehouse from `tabPOS Profile` diff --git a/erpnext/accounts/doctype/subscription/subscription.py b/erpnext/accounts/doctype/subscription/subscription.py index 1abb93464b0..01696686daa 100644 --- a/erpnext/accounts/doctype/subscription/subscription.py +++ b/erpnext/accounts/doctype/subscription/subscription.py @@ -288,6 +288,7 @@ class Subscription(Document): invoice.to_date = self.current_invoice_end invoice.flags.ignore_mandatory = True + invoice.set_missing_values() invoice.save() if self.submit_invoice: diff --git a/erpnext/accounts/report/gross_profit/gross_profit.json b/erpnext/accounts/report/gross_profit/gross_profit.json index cd6bac2d77d..5fff3fdba77 100644 --- a/erpnext/accounts/report/gross_profit/gross_profit.json +++ b/erpnext/accounts/report/gross_profit/gross_profit.json @@ -1,16 +1,20 @@ { - "add_total_row": 1, + "add_total_row": 0, + "columns": [], "creation": "2013-02-25 17:03:34", + "disable_prepared_report": 0, "disabled": 0, "docstatus": 0, "doctype": "Report", + "filters": [], "idx": 3, "is_standard": "Yes", - "modified": "2020-08-13 11:26:39.112352", + "modified": "2021-08-19 18:57:07.468202", "modified_by": "Administrator", "module": "Accounts", "name": "Gross Profit", "owner": "Administrator", + "prepared_report": 0, "ref_doctype": "Sales Invoice", "report_name": "Gross Profit", "report_type": "Script Report", diff --git a/erpnext/accounts/report/gross_profit/gross_profit.py b/erpnext/accounts/report/gross_profit/gross_profit.py index 4e22b05a81d..ef048bfc3bb 100644 --- a/erpnext/accounts/report/gross_profit/gross_profit.py +++ b/erpnext/accounts/report/gross_profit/gross_profit.py @@ -41,12 +41,14 @@ def execute(filters=None): columns = get_columns(group_wise_columns, filters) - for src in gross_profit_data.grouped_data: + for idx, src in enumerate(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] = frappe.bold("Total") data.append(row) return columns, data @@ -154,6 +156,15 @@ class GrossProfitGenerator(object): 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]): @@ -165,6 +176,7 @@ 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.parent in self.returned_invoices \ @@ -177,15 +189,25 @@ class GrossProfitGenerator(object): if 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) + self.grouped_data.append(self.totals) def set_average_rate(self, new_row): + self.set_average_gross_profit(new_row) + new_row.buying_rate = flt(new_row.buying_amount / new_row.qty, self.float_precision) if new_row.qty else 0 + new_row.base_rate = flt(new_row.base_amount / new_row.qty, self.float_precision) if new_row.qty else 0 + return new_row + + def set_average_gross_profit(self, new_row): new_row.gross_profit = flt(new_row.base_amount - new_row.buying_amount, self.currency_precision) 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 - new_row.buying_rate = flt(new_row.buying_amount / new_row.qty, self.float_precision) if new_row.qty else 0 - new_row.base_rate = flt(new_row.base_amount / new_row.qty, self.float_precision) if new_row.qty else 0 - return new_row + 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(""" diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 762b4dcdb71..0a711881e24 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -1290,6 +1290,11 @@ def update_child_qty_rate(parent_doctype, trans_items, parent_doctype_name, chil for d in data: new_child_flag = False + + if not d.get("item_code"): + # ignore empty rows + continue + if not d.get("docname"): new_child_flag = True check_doc_permissions(parent, 'create') diff --git a/erpnext/controllers/buying_controller.py b/erpnext/controllers/buying_controller.py index e5012f972b2..dfa8546da0f 100644 --- a/erpnext/controllers/buying_controller.py +++ b/erpnext/controllers/buying_controller.py @@ -983,11 +983,11 @@ def get_non_stock_items(purchase_order, fg_item_code): def set_serial_nos(raw_material, consumed_serial_nos, qty): consumed_serial_nos_list = [] - if isinstance(consumed_serial_nos, list): + if consumed_serial_nos and isinstance(consumed_serial_nos, list): for row in consumed_serial_nos: consumed_serial_nos_list.extend(get_serial_nos(row)) - else: - consumed_serial_nos_list = get_serial_nos(row) + elif consumed_serial_nos: + consumed_serial_nos_list = get_serial_nos(consumed_serial_nos) serial_nos = set(get_serial_nos(raw_material.serial_nos)) - set(consumed_serial_nos_list) diff --git a/erpnext/maintenance/doctype/maintenance_visit/maintenance_visit.js b/erpnext/maintenance/doctype/maintenance_visit/maintenance_visit.js index 2e2a9ce0401..fff46ad67e3 100644 --- a/erpnext/maintenance/doctype/maintenance_visit/maintenance_visit.js +++ b/erpnext/maintenance/doctype/maintenance_visit/maintenance_visit.js @@ -49,13 +49,17 @@ erpnext.maintenance.MaintenanceVisit = frappe.ui.form.Controller.extend({ if (this.frm.doc.docstatus===0) { this.frm.add_custom_button(__('Maintenance Schedule'), - function() { + function () { + if (!me.frm.doc.customer) { + frappe.msgprint(__('Please select Customer first')); + return; + } erpnext.utils.map_current_doc({ method: "erpnext.maintenance.doctype.maintenance_schedule.maintenance_schedule.make_maintenance_visit", source_doctype: "Maintenance Schedule", target: me.frm, setters: { - customer: me.frm.doc.customer || undefined, + customer: me.frm.doc.customer, }, get_query_filters: { docstatus: 1, @@ -80,13 +84,17 @@ erpnext.maintenance.MaintenanceVisit = frappe.ui.form.Controller.extend({ }) }, __("Get items from")); this.frm.add_custom_button(__('Sales Order'), - function() { + function () { + if (!me.frm.doc.customer) { + frappe.msgprint(__('Please select Customer first')); + return; + } erpnext.utils.map_current_doc({ method: "erpnext.selling.doctype.sales_order.sales_order.make_maintenance_visit", source_doctype: "Sales Order", target: me.frm, setters: { - customer: me.frm.doc.customer || undefined, + customer: me.frm.doc.customer, }, get_query_filters: { docstatus: 1, @@ -99,4 +107,4 @@ erpnext.maintenance.MaintenanceVisit = frappe.ui.form.Controller.extend({ }, }); -$.extend(cur_frm.cscript, new erpnext.maintenance.MaintenanceVisit({frm: cur_frm})); \ No newline at end of file +$.extend(cur_frm.cscript, new erpnext.maintenance.MaintenanceVisit({frm: cur_frm})); diff --git a/erpnext/manufacturing/doctype/bom/bom.py b/erpnext/manufacturing/doctype/bom/bom.py index 0bc6a70376b..0d324eb2070 100644 --- a/erpnext/manufacturing/doctype/bom/bom.py +++ b/erpnext/manufacturing/doctype/bom/bom.py @@ -414,25 +414,29 @@ class BOM(WebsiteGenerator): frappe.throw(_("Quantity required for Item {0} in row {1}").format(m.item_code, m.idx)) check_list.append(m) - def check_recursion(self, bom_list=[]): + def check_recursion(self, bom_list=None): """ Check whether recursion occurs in any bom""" + def _throw_error(bom_name): + frappe.throw(_("BOM recursion: {0} cannot be parent or child of {0}").format(bom_name)) + bom_list = self.traverse_tree() - bom_nos = frappe.get_all('BOM Item', fields=["bom_no"], - filters={'parent': ('in', bom_list), 'parenttype': 'BOM'}) + child_items = frappe.get_all('BOM Item', fields=["bom_no", "item_code"], + filters={'parent': ('in', bom_list), 'parenttype': 'BOM'}) or [] - raise_exception = False - if bom_nos and self.name in [d.bom_no for d in bom_nos]: - raise_exception = True + child_bom = {d.bom_no for d in child_items} + child_items_codes = {d.item_code for d in child_items} - if not raise_exception: - bom_nos = frappe.get_all('BOM Item', fields=["parent"], - filters={'bom_no': self.name, 'parenttype': 'BOM'}) + if self.name in child_bom: + _throw_error(self.name) - if self.name in [d.parent for d in bom_nos]: - raise_exception = True + if self.item in child_items_codes: + _throw_error(self.item) - if raise_exception: - frappe.throw(_("BOM recursion: {0} cannot be parent or child of {1}").format(self.name, self.name)) + bom_nos = frappe.get_all('BOM Item', fields=["parent"], + filters={'bom_no': self.name, 'parenttype': 'BOM'}) or [] + + if self.name in {d.parent for d in bom_nos}: + _throw_error(self.name) def update_cost_and_exploded_items(self, bom_list=[]): bom_list = self.traverse_tree(bom_list) diff --git a/erpnext/manufacturing/doctype/production_plan/production_plan.py b/erpnext/manufacturing/doctype/production_plan/production_plan.py index 1b2cd542a4f..e42bc2d5ffb 100644 --- a/erpnext/manufacturing/doctype/production_plan/production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/production_plan.py @@ -228,7 +228,7 @@ class ProductionPlan(Document): if self.total_produced_qty > 0: self.status = "In Process" - if self.total_produced_qty == self.total_planned_qty: + if self.total_produced_qty >= self.total_planned_qty: self.status = "Completed" if self.status != 'Completed': diff --git a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py index 2bf883809da..24cdede4433 100644 --- a/erpnext/manufacturing/doctype/production_plan/test_production_plan.py +++ b/erpnext/manufacturing/doctype/production_plan/test_production_plan.py @@ -198,6 +198,7 @@ class TestProductionPlan(unittest.TestCase): pln.cancel() frappe.delete_doc("Production Plan", pln.name) + def create_production_plan(**args): args = frappe._dict(args) diff --git a/erpnext/public/js/utils.js b/erpnext/public/js/utils.js index c827fca378f..18705a0d573 100755 --- a/erpnext/public/js/utils.js +++ b/erpnext/public/js/utils.js @@ -550,7 +550,7 @@ erpnext.utils.update_child_items = function(opts) { }, ], primary_action: function() { - const trans_items = this.get_values()["trans_items"]; + const trans_items = this.get_values()["trans_items"].filter((item) => !!item.item_code); frappe.call({ method: 'erpnext.controllers.accounts_controller.update_child_qty_rate', freeze: true, diff --git a/erpnext/selling/doctype/sales_order/sales_order.py b/erpnext/selling/doctype/sales_order/sales_order.py index 59df180ff8c..086737ca04a 100755 --- a/erpnext/selling/doctype/sales_order/sales_order.py +++ b/erpnext/selling/doctype/sales_order/sales_order.py @@ -714,8 +714,7 @@ def make_maintenance_schedule(source_name, target_doc=None): "doctype": "Maintenance Schedule Item", "field_map": { "parent": "sales_order" - }, - "add_if_empty": True + } } }, target_doc) @@ -741,8 +740,7 @@ def make_maintenance_visit(source_name, target_doc=None): "field_map": { "parent": "prevdoc_docname", "parenttype": "prevdoc_doctype" - }, - "add_if_empty": True + } } }, target_doc) diff --git a/erpnext/setup/doctype/item_group/item_group.py b/erpnext/setup/doctype/item_group/item_group.py index 43778404b60..433545a01f4 100644 --- a/erpnext/setup/doctype/item_group/item_group.py +++ b/erpnext/setup/doctype/item_group/item_group.py @@ -33,6 +33,7 @@ class ItemGroup(NestedSet, WebsiteGenerator): self.parent_item_group = _('All Item Groups') self.make_route() + self.validate_item_group_defaults() def on_update(self): NestedSet.on_update(self) @@ -88,6 +89,10 @@ class ItemGroup(NestedSet, WebsiteGenerator): def delete_child_item_groups_key(self): frappe.cache().hdel("child_item_groups", self.name) + def validate_item_group_defaults(self): + from erpnext.stock.doctype.item.item import validate_item_default_company_links + validate_item_default_company_links(self.item_group_defaults) + @frappe.whitelist(allow_guest=True) def get_product_list_for_group(product_group=None, start=0, limit=10, search=None): if product_group: diff --git a/erpnext/setup/doctype/item_group/test_records.json b/erpnext/setup/doctype/item_group/test_records.json index 71159643209..37e172de209 100644 --- a/erpnext/setup/doctype/item_group/test_records.json +++ b/erpnext/setup/doctype/item_group/test_records.json @@ -1,73 +1,74 @@ [ { - "doctype": "Item Group", - "is_group": 0, - "item_group_name": "_Test Item Group", + "doctype": "Item Group", + "is_group": 0, + "item_group_name": "_Test Item Group", "parent_item_group": "All Item Groups", "item_group_defaults": [{ "company": "_Test Company", "buying_cost_center": "_Test Cost Center 2 - _TC", - "selling_cost_center": "_Test Cost Center 2 - _TC" + "selling_cost_center": "_Test Cost Center 2 - _TC", + "default_warehouse": "_Test Warehouse - _TC" }] - }, + }, { - "doctype": "Item Group", - "is_group": 0, - "item_group_name": "_Test Item Group Desktops", + "doctype": "Item Group", + "is_group": 0, + "item_group_name": "_Test Item Group Desktops", "parent_item_group": "All Item Groups" - }, + }, { - "doctype": "Item Group", - "is_group": 1, - "item_group_name": "_Test Item Group A", + "doctype": "Item Group", + "is_group": 1, + "item_group_name": "_Test Item Group A", "parent_item_group": "All Item Groups" - }, + }, { - "doctype": "Item Group", - "is_group": 1, - "item_group_name": "_Test Item Group B", + "doctype": "Item Group", + "is_group": 1, + "item_group_name": "_Test Item Group B", "parent_item_group": "All Item Groups" - }, + }, { - "doctype": "Item Group", - "is_group": 1, - "item_group_name": "_Test Item Group B - 1", + "doctype": "Item Group", + "is_group": 1, + "item_group_name": "_Test Item Group B - 1", "parent_item_group": "_Test Item Group B" - }, + }, { - "doctype": "Item Group", - "is_group": 1, - "item_group_name": "_Test Item Group B - 2", + "doctype": "Item Group", + "is_group": 1, + "item_group_name": "_Test Item Group B - 2", "parent_item_group": "_Test Item Group B" - }, + }, { - "doctype": "Item Group", - "is_group": 0, - "item_group_name": "_Test Item Group B - 3", + "doctype": "Item Group", + "is_group": 0, + "item_group_name": "_Test Item Group B - 3", "parent_item_group": "_Test Item Group B" - }, + }, { - "doctype": "Item Group", - "is_group": 1, - "item_group_name": "_Test Item Group C", + "doctype": "Item Group", + "is_group": 1, + "item_group_name": "_Test Item Group C", "parent_item_group": "All Item Groups" - }, + }, { - "doctype": "Item Group", - "is_group": 1, - "item_group_name": "_Test Item Group C - 1", + "doctype": "Item Group", + "is_group": 1, + "item_group_name": "_Test Item Group C - 1", "parent_item_group": "_Test Item Group C" - }, + }, { - "doctype": "Item Group", - "is_group": 1, - "item_group_name": "_Test Item Group C - 2", + "doctype": "Item Group", + "is_group": 1, + "item_group_name": "_Test Item Group C - 2", "parent_item_group": "_Test Item Group C" - }, + }, { - "doctype": "Item Group", - "is_group": 1, - "item_group_name": "_Test Item Group D", + "doctype": "Item Group", + "is_group": 1, + "item_group_name": "_Test Item Group D", "parent_item_group": "All Item Groups" }, { @@ -104,4 +105,4 @@ } ] } -] \ No newline at end of file +] diff --git a/erpnext/stock/doctype/item/item.js b/erpnext/stock/doctype/item/item.js index b41aea8b880..7ab973ddbdd 100644 --- a/erpnext/stock/doctype/item/item.js +++ b/erpnext/stock/doctype/item/item.js @@ -85,7 +85,7 @@ frappe.ui.form.on("Item", { } if (frm.doc.variant_of) { frm.set_intro(__('This Item is a Variant of {0} (Template).', - [`${frm.doc.variant_of}`]), true); + [`${frm.doc.variant_of}`]), true); } if (frappe.defaults.get_default("item_naming_by")!="Naming Series" || frm.doc.variant_of) { diff --git a/erpnext/stock/doctype/item/item.py b/erpnext/stock/doctype/item/item.py index f231aac086b..bdaa63cd872 100644 --- a/erpnext/stock/doctype/item/item.py +++ b/erpnext/stock/doctype/item/item.py @@ -6,6 +6,7 @@ from __future__ import unicode_literals import itertools import json import erpnext + import frappe import copy from erpnext.controllers.item_variant import (ItemVariantExistsError, @@ -121,9 +122,9 @@ class Item(WebsiteGenerator): self.validate_fixed_asset() self.validate_retain_sample() self.validate_uom_conversion_factor() - self.validate_item_defaults() self.validate_customer_provided_part() self.update_defaults_from_item_group() + self.validate_item_defaults() self.validate_auto_reorder_enabled_in_stock_settings() self.cant_change() self.update_show_in_website() @@ -758,35 +759,39 @@ class Item(WebsiteGenerator): if len(companies) != len(self.item_defaults): frappe.throw(_("Cannot set multiple Item Defaults for a company.")) + validate_item_default_company_links(self.item_defaults) + + def update_defaults_from_item_group(self): """Get defaults from Item Group""" - if self.item_group and not self.item_defaults: - item_defaults = frappe.db.get_values("Item Default", {"parent": self.item_group}, - ['company', 'default_warehouse','default_price_list','buying_cost_center','default_supplier', - 'expense_account','selling_cost_center','income_account'], as_dict = 1) - if item_defaults: - for item in item_defaults: - self.append('item_defaults', { - 'company': item.company, - 'default_warehouse': item.default_warehouse, - 'default_price_list': item.default_price_list, - 'buying_cost_center': item.buying_cost_center, - 'default_supplier': item.default_supplier, - 'expense_account': item.expense_account, - 'selling_cost_center': item.selling_cost_center, - 'income_account': item.income_account - }) - else: - warehouse = '' - defaults = frappe.defaults.get_defaults() or {} + if self.item_defaults or not self.item_group: + return - # To check default warehouse is belong to the default company - if defaults.get("default_warehouse") and defaults.company and frappe.db.exists("Warehouse", - {'name': defaults.default_warehouse, 'company': defaults.company}): - self.append("item_defaults", { - "company": defaults.get("company"), - "default_warehouse": defaults.default_warehouse - }) + item_defaults = frappe.db.get_values("Item Default", {"parent": self.item_group}, + ['company', 'default_warehouse','default_price_list','buying_cost_center','default_supplier', + 'expense_account','selling_cost_center','income_account'], as_dict = 1) + if item_defaults: + for item in item_defaults: + self.append('item_defaults', { + 'company': item.company, + 'default_warehouse': item.default_warehouse, + 'default_price_list': item.default_price_list, + 'buying_cost_center': item.buying_cost_center, + 'default_supplier': item.default_supplier, + 'expense_account': item.expense_account, + 'selling_cost_center': item.selling_cost_center, + 'income_account': item.income_account + }) + else: + defaults = frappe.defaults.get_defaults() or {} + + # To check default warehouse is belong to the default company + if defaults.get("default_warehouse") and defaults.company and frappe.db.exists("Warehouse", + {'name': defaults.default_warehouse, 'company': defaults.company}): + self.append("item_defaults", { + "company": defaults.get("company"), + "default_warehouse": defaults.default_warehouse + }) def update_variants(self): if self.flags.dont_update_variants or \ @@ -1237,3 +1242,24 @@ def update_variants(variants, template, publish_progress=True): def on_doctype_update(): # since route is a Text column, it needs a length for indexing frappe.db.add_index("Item", ["route(500)"]) + +def validate_item_default_company_links(item_defaults): + for item_default in item_defaults: + for doctype, field in [ + ['Warehouse', 'default_warehouse'], + ['Cost Center', 'buying_cost_center'], + ['Cost Center', 'selling_cost_center'], + ['Account', 'expense_account'], + ['Account', 'income_account'] + ]: + if item_default.get(field): + company = frappe.db.get_value(doctype, item_default.get(field), 'company', cache=True) + if company and company != item_default.company: + frappe.throw(_("Row #{}: {} {} doesn't belong to Company {}. Please select valid {}.") + .format( + item_default.idx, + doctype, + frappe.bold(item_default.get(field)), + frappe.bold(item_default.company), + frappe.bold(frappe.unscrub(field)) + ), title=_("Invalid Item Defaults")) diff --git a/erpnext/stock/doctype/item/test_item.py b/erpnext/stock/doctype/item/test_item.py index da53d8d6b15..5074040d891 100644 --- a/erpnext/stock/doctype/item/test_item.py +++ b/erpnext/stock/doctype/item/test_item.py @@ -219,6 +219,23 @@ class TestItem(unittest.TestCase): for key, value in iteritems(purchase_item_check): self.assertEqual(value, purchase_item_details.get(key)) + def test_item_default_validations(self): + + with self.assertRaises(frappe.ValidationError) as ve: + make_item("Bad Item defaults", { + "item_group": "_Test Item Group", + "item_defaults": [{ + "company": "_Test Company 1", + "default_warehouse": "_Test Warehouse - _TC", + "expense_account": "Stock In Hand - _TC", + "buying_cost_center": "_Test Cost Center - _TC", + "selling_cost_center": "_Test Cost Center - _TC", + }] + }) + + self.assertTrue("belong to company" in str(ve.exception).lower(), + msg="Mismatching company entities in item defaults should not be allowed.") + def test_item_attribute_change_after_variant(self): frappe.delete_doc_if_exists("Item", "_Test Variant Item-L", force=1) diff --git a/erpnext/stock/get_item_details.py b/erpnext/stock/get_item_details.py index ae796758c75..a8742eef81e 100644 --- a/erpnext/stock/get_item_details.py +++ b/erpnext/stock/get_item_details.py @@ -72,9 +72,7 @@ def get_item_details(args, doc=None, for_validate=False, overwrite_warehouse=Tru update_party_blanket_order(args, out) - if not doc or cint(doc.get('is_return')) == 0: - # get price list rate only if the invoice is not a credit or debit note - get_price_list_rate(args, item, out) + get_price_list_rate(args, item, out) if args.customer and cint(args.is_pos): out.update(get_pos_profile_item_details(args.company, args, update_data=True)) diff --git a/erpnext/stock/report/stock_ageing/stock_ageing.py b/erpnext/stock/report/stock_ageing/stock_ageing.py index 4919f8b4c05..47345ce7f8d 100644 --- a/erpnext/stock/report/stock_ageing/stock_ageing.py +++ b/erpnext/stock/report/stock_ageing/stock_ageing.py @@ -175,9 +175,7 @@ def get_fifo_queue(filters, sle=None): fifo_queue.append([d.actual_qty, d.posting_date]) else: if serial_no_list: - for serial_no in fifo_queue: - if serial_no[0] in serial_no_list: - fifo_queue.remove(serial_no) + fifo_queue[:] = [serial_no for serial_no in fifo_queue if serial_no[0] not in serial_no_list] else: qty_to_pop = abs(d.actual_qty) while qty_to_pop: