From 91982d1e4f7012a6a28ed8116a65fdfd9b9f973f Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 24 Jan 2023 18:03:53 +0530 Subject: [PATCH 01/23] feat: Filter out alternative item rows in taxes and totals for Quotation - Added a Quotation Item field `is_alternative_item` - Use filtered rows for taxes and totals computation --- erpnext/controllers/taxes_and_totals.py | 32 ++++++++++++------- .../quotation_item/quotation_item.json | 11 ++++++- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/erpnext/controllers/taxes_and_totals.py b/erpnext/controllers/taxes_and_totals.py index 8c403aa9bf..5815cfa7ac 100644 --- a/erpnext/controllers/taxes_and_totals.py +++ b/erpnext/controllers/taxes_and_totals.py @@ -24,11 +24,19 @@ class calculate_taxes_and_totals(object): def __init__(self, doc: Document): self.doc = doc frappe.flags.round_off_applicable_accounts = [] + + self._items = self.filter_rows() if self.doc.doctype == "Quotation" else self.doc.get("items") + get_round_off_applicable_accounts(self.doc.company, frappe.flags.round_off_applicable_accounts) self.calculate() + def filter_rows(self): + """Exclude rows, that do not fulfill the filter criteria, from totals computation.""" + items = list(filter(lambda item: not item.get("is_alternative_item"), self.doc.get("items"))) + return items + def calculate(self): - if not len(self.doc.get("items")): + if not len(self._items): return self.discount_amount_applied = False @@ -70,7 +78,7 @@ class calculate_taxes_and_totals(object): if hasattr(self.doc, "tax_withholding_net_total"): sum_net_amount = 0 sum_base_net_amount = 0 - for item in self.doc.get("items"): + for item in self._items: if hasattr(item, "apply_tds") and item.apply_tds: sum_net_amount += item.net_amount sum_base_net_amount += item.base_net_amount @@ -79,7 +87,7 @@ class calculate_taxes_and_totals(object): self.doc.base_tax_withholding_net_total = sum_base_net_amount def validate_item_tax_template(self): - for item in self.doc.get("items"): + for item in self._items: if item.item_code and item.get("item_tax_template"): item_doc = frappe.get_cached_doc("Item", item.item_code) args = { @@ -137,7 +145,7 @@ class calculate_taxes_and_totals(object): return if not self.discount_amount_applied: - for item in self.doc.get("items"): + for item in self._items: self.doc.round_floats_in(item) if item.discount_percentage == 100: @@ -236,7 +244,7 @@ class calculate_taxes_and_totals(object): if not any(cint(tax.included_in_print_rate) for tax in self.doc.get("taxes")): return - for item in self.doc.get("items"): + for item in self._items: item_tax_map = self._load_item_tax_rate(item.item_tax_rate) cumulated_tax_fraction = 0 total_inclusive_tax_amount_per_qty = 0 @@ -317,7 +325,7 @@ class calculate_taxes_and_totals(object): self.doc.total ) = self.doc.base_total = self.doc.net_total = self.doc.base_net_total = 0.0 - for item in self.doc.get("items"): + for item in self._items: self.doc.total += item.amount self.doc.total_qty += item.qty self.doc.base_total += item.base_amount @@ -354,7 +362,7 @@ class calculate_taxes_and_totals(object): ] ) - for n, item in enumerate(self.doc.get("items")): + for n, item in enumerate(self._items): item_tax_map = self._load_item_tax_rate(item.item_tax_rate) for i, tax in enumerate(self.doc.get("taxes")): # tax_amount represents the amount of tax for the current step @@ -363,7 +371,7 @@ class calculate_taxes_and_totals(object): # Adjust divisional loss to the last item if tax.charge_type == "Actual": actual_tax_dict[tax.idx] -= current_tax_amount - if n == len(self.doc.get("items")) - 1: + if n == len(self._items) - 1: current_tax_amount += actual_tax_dict[tax.idx] # accumulate tax amount into tax.tax_amount @@ -391,7 +399,7 @@ class calculate_taxes_and_totals(object): ) # set precision in the last item iteration - if n == len(self.doc.get("items")) - 1: + if n == len(self._items) - 1: self.round_off_totals(tax) self._set_in_company_currency(tax, ["tax_amount", "tax_amount_after_discount_amount"]) @@ -570,7 +578,7 @@ class calculate_taxes_and_totals(object): def calculate_total_net_weight(self): if self.doc.meta.get_field("total_net_weight"): self.doc.total_net_weight = 0.0 - for d in self.doc.items: + for d in self._items: if d.total_weight: self.doc.total_net_weight += d.total_weight @@ -630,7 +638,7 @@ class calculate_taxes_and_totals(object): if total_for_discount_amount: # calculate item amount after Discount Amount - for i, item in enumerate(self.doc.get("items")): + for i, item in enumerate(self._items): distributed_amount = ( flt(self.doc.discount_amount) * item.net_amount / total_for_discount_amount ) @@ -643,7 +651,7 @@ class calculate_taxes_and_totals(object): self.doc.apply_discount_on == "Net Total" or not taxes or total_for_discount_amount == self.doc.net_total - ) and i == len(self.doc.get("items")) - 1: + ) and i == len(self._items) - 1: discount_amount_loss = flt( self.doc.net_total - net_total - self.doc.discount_amount, self.doc.precision("net_total") ) diff --git a/erpnext/selling/doctype/quotation_item/quotation_item.json b/erpnext/selling/doctype/quotation_item/quotation_item.json index ca7dfd2337..eaa4d1d747 100644 --- a/erpnext/selling/doctype/quotation_item/quotation_item.json +++ b/erpnext/selling/doctype/quotation_item/quotation_item.json @@ -49,6 +49,7 @@ "pricing_rules", "stock_uom_rate", "is_free_item", + "is_alternative_item", "section_break_43", "valuation_rate", "column_break_45", @@ -643,12 +644,19 @@ "no_copy": 1, "options": "currency", "read_only": 1 + }, + { + "default": "0", + "fieldname": "is_alternative_item", + "fieldtype": "Check", + "label": "Is Alternative Item", + "print_hide": 1 } ], "idx": 1, "istable": 1, "links": [], - "modified": "2022-12-25 02:49:53.926625", + "modified": "2023-01-24 08:48:06.290335", "modified_by": "Administrator", "module": "Selling", "name": "Quotation Item", @@ -656,5 +664,6 @@ "permissions": [], "sort_field": "modified", "sort_order": "DESC", + "states": [], "track_changes": 1 } \ No newline at end of file From f19eadab9ab1a7defd74cf3b7337012e4e111ac8 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 25 Jan 2023 13:10:03 +0530 Subject: [PATCH 02/23] feat: Consider filtered items table in JS for totals computation - Set `_items` as filtered rows if quotation else the entire table. Set at entry point of JS API - Use `_items` instead of `items` to compute taxes and charges. Exclude alternative item rows --- .../public/js/controllers/taxes_and_totals.js | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/erpnext/public/js/controllers/taxes_and_totals.js b/erpnext/public/js/controllers/taxes_and_totals.js index 2ce0c7eb00..607b928d01 100644 --- a/erpnext/public/js/controllers/taxes_and_totals.js +++ b/erpnext/public/js/controllers/taxes_and_totals.js @@ -91,6 +91,9 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { } _calculate_taxes_and_totals() { + const is_quotation = this.frm.doc.doctype == "Quotation"; + this.frm.doc._items = is_quotation ? this.filtered_items() : this.frm.doc.items; + this.validate_conversion_rate(); this.calculate_item_values(); this.initialize_taxes(); @@ -122,7 +125,7 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { calculate_item_values() { var me = this; if (!this.discount_amount_applied) { - for (const item of this.frm.doc.items || []) { + for (const item of this.frm.doc._items || []) { frappe.model.round_floats_in(item); item.net_rate = item.rate; item.qty = item.qty === undefined ? (me.frm.doc.is_return ? -1 : 1) : item.qty; @@ -197,7 +200,7 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { }); if(has_inclusive_tax==false) return; - $.each(me.frm.doc["items"] || [], function(n, item) { + $.each(me.frm.doc._items || [], function(n, item) { var item_tax_map = me._load_item_tax_rate(item.item_tax_rate); var cumulated_tax_fraction = 0.0; var total_inclusive_tax_amount_per_qty = 0; @@ -268,7 +271,7 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { var me = this; this.frm.doc.total_qty = this.frm.doc.total = this.frm.doc.base_total = this.frm.doc.net_total = this.frm.doc.base_net_total = 0.0; - $.each(this.frm.doc["items"] || [], function(i, item) { + $.each(this.frm.doc._items || [], function(i, item) { me.frm.doc.total += item.amount; me.frm.doc.total_qty += item.qty; me.frm.doc.base_total += item.base_amount; @@ -321,7 +324,7 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { } }); - $.each(this.frm.doc["items"] || [], function(n, item) { + $.each(this.frm.doc._items || [], function(n, item) { var item_tax_map = me._load_item_tax_rate(item.item_tax_rate); $.each(me.frm.doc["taxes"] || [], function(i, tax) { // tax_amount represents the amount of tax for the current step @@ -330,7 +333,7 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { // Adjust divisional loss to the last item if (tax.charge_type == "Actual") { actual_tax_dict[tax.idx] -= current_tax_amount; - if (n == me.frm.doc["items"].length - 1) { + if (n == me.frm.doc._items.length - 1) { current_tax_amount += actual_tax_dict[tax.idx]; } } @@ -367,7 +370,7 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { } // set precision in the last item iteration - if (n == me.frm.doc["items"].length - 1) { + if (n == me.frm.doc._items.length - 1) { me.round_off_totals(tax); me.set_in_company_currency(tax, ["tax_amount", "tax_amount_after_discount_amount"]); @@ -590,10 +593,11 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { _cleanup() { this.frm.doc.base_in_words = this.frm.doc.in_words = ""; + let items = this.frm.doc._items; - if(this.frm.doc["items"] && this.frm.doc["items"].length) { - if(!frappe.meta.get_docfield(this.frm.doc["items"][0].doctype, "item_tax_amount", this.frm.doctype)) { - $.each(this.frm.doc["items"] || [], function(i, item) { + if(items && items.length) { + if(!frappe.meta.get_docfield(items[0].doctype, "item_tax_amount", this.frm.doctype)) { + $.each(items || [], function(i, item) { delete item["item_tax_amount"]; }); } @@ -646,7 +650,7 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { var net_total = 0; // calculate item amount after Discount Amount if (total_for_discount_amount) { - $.each(this.frm.doc["items"] || [], function(i, item) { + $.each(this.frm.doc._items || [], function(i, item) { distributed_amount = flt(me.frm.doc.discount_amount) * item.net_amount / total_for_discount_amount; item.net_amount = flt(item.net_amount - distributed_amount, precision("base_amount", item)); @@ -654,7 +658,7 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { // discount amount rounding loss adjustment if no taxes if ((!(me.frm.doc.taxes || []).length || total_for_discount_amount==me.frm.doc.net_total || (me.frm.doc.apply_discount_on == "Net Total")) - && i == (me.frm.doc.items || []).length - 1) { + && i == (me.frm.doc._items || []).length - 1) { var discount_amount_loss = flt(me.frm.doc.net_total - net_total - me.frm.doc.discount_amount, precision("net_total")); item.net_amount = flt(item.net_amount + discount_amount_loss, @@ -883,4 +887,8 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { } } + + filtered_items() { + return this.frm.doc.items.filter(item => !item["is_alternative_item"]); + } }; From cef7dfd0b48ba6ebd6dfb9eabb722c78e4493ccb Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 26 Jan 2023 14:36:25 +0530 Subject: [PATCH 03/23] feat: Dialog to select alternative item before creating Sales order - Users can leave the row blank in the dialog if original item is to be used - Else users can select an alternative item against an original item - In the document, users must check `Is Alternative Item` if needed and also specify which item it is an altenrative to since there are no documented mappings --- erpnext/controllers/taxes_and_totals.py | 2 +- .../public/js/controllers/taxes_and_totals.js | 2 +- .../selling/doctype/quotation/quotation.js | 79 ++++++++++++++++++- .../quotation_item/quotation_item.json | 18 ++++- 4 files changed, 94 insertions(+), 7 deletions(-) diff --git a/erpnext/controllers/taxes_and_totals.py b/erpnext/controllers/taxes_and_totals.py index 5815cfa7ac..1edd7bf85e 100644 --- a/erpnext/controllers/taxes_and_totals.py +++ b/erpnext/controllers/taxes_and_totals.py @@ -32,7 +32,7 @@ class calculate_taxes_and_totals(object): def filter_rows(self): """Exclude rows, that do not fulfill the filter criteria, from totals computation.""" - items = list(filter(lambda item: not item.get("is_alternative_item"), self.doc.get("items"))) + items = list(filter(lambda item: not item.get("is_alternative"), self.doc.get("items"))) return items def calculate(self): diff --git a/erpnext/public/js/controllers/taxes_and_totals.js b/erpnext/public/js/controllers/taxes_and_totals.js index 607b928d01..029d6c0c41 100644 --- a/erpnext/public/js/controllers/taxes_and_totals.js +++ b/erpnext/public/js/controllers/taxes_and_totals.js @@ -889,6 +889,6 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { } filtered_items() { - return this.frm.doc.items.filter(item => !item["is_alternative_item"]); + return this.frm.doc.items.filter(item => !item["is_alternative"]); } }; diff --git a/erpnext/selling/doctype/quotation/quotation.js b/erpnext/selling/doctype/quotation/quotation.js index 6b42e4daea..6f75673a8e 100644 --- a/erpnext/selling/doctype/quotation/quotation.js +++ b/erpnext/selling/doctype/quotation/quotation.js @@ -87,7 +87,7 @@ erpnext.selling.QuotationController = class QuotationController extends erpnext. if (doc.docstatus == 1 && !["Lost", "Ordered"].includes(doc.status)) { this.frm.add_custom_button( __("Sales Order"), - this.frm.cscript["Make Sales Order"], + () => this.make_sales_order(), __("Create") ); @@ -141,6 +141,20 @@ erpnext.selling.QuotationController = class QuotationController extends erpnext. } + make_sales_order() { + var me = this; + + let has_alternative_item = this.frm.doc.items.some((item) => item.is_alternative); + if (has_alternative_item) { + this.show_alternative_item_dialog(); + } else { + frappe.model.open_mapped_doc({ + method: "erpnext.selling.doctype.quotation.quotation.make_sales_order", + frm: me.frm + }); + } + } + set_dynamic_field_label(){ if (this.frm.doc.quotation_to == "Customer") { @@ -216,6 +230,69 @@ erpnext.selling.QuotationController = class QuotationController extends erpnext. } }) } + + show_alternative_item_dialog() { + // Create a `{original item: [alternate items]}` map + const item_alt_map = {}; + this.frm.doc.items.filter( + (item) => item.is_alternative + ).forEach((item) => + (item_alt_map[item.alternative_to] ??= []).push(item.item_code) + ) + + const fields = [{ + fieldtype:"Link", + fieldname:"original_item", + options: "Item", + label: __("Original Item"), + read_only: 1, + in_list_view: 1, + }, + { + fieldtype:"Link", + fieldname:"alternative_item", + options: "Item", + label: __("Alternative Item"), + in_list_view: 1, + get_query: (row, cdt, cdn) => { + return { + filters: { + "item_code": ["in", item_alt_map[row.original_item]] + } + } + }, + }]; + + this.data = Object.keys(item_alt_map).map((item) => { + return {"original_item": item} + }); + + const dialog = new frappe.ui.Dialog({ + title: __("Select Alternatives for Sales Order"), + fields: [ + { + fieldname: "alternative_items", + fieldtype: "Table", + label: "Items with Alternatives", + cannot_add_rows: true, + in_place_edit: true, + reqd: 1, + data: this.data, + description: __("Select an alternative to be used in the Sales Order or leave it blank to use the original item."), + get_data: () => { + return this.data; + }, + fields: fields + }, + ], + primary_action: function() { + this.hide(); + }, + primary_action_label: __('Continue') + }); + + dialog.show(); + } }; cur_frm.script_manager.make(erpnext.selling.QuotationController); diff --git a/erpnext/selling/doctype/quotation_item/quotation_item.json b/erpnext/selling/doctype/quotation_item/quotation_item.json index eaa4d1d747..f62a0997dc 100644 --- a/erpnext/selling/doctype/quotation_item/quotation_item.json +++ b/erpnext/selling/doctype/quotation_item/quotation_item.json @@ -49,7 +49,8 @@ "pricing_rules", "stock_uom_rate", "is_free_item", - "is_alternative_item", + "is_alternative", + "alternative_to", "section_break_43", "valuation_rate", "column_break_45", @@ -647,16 +648,25 @@ }, { "default": "0", - "fieldname": "is_alternative_item", + "fieldname": "is_alternative", "fieldtype": "Check", - "label": "Is Alternative Item", + "label": "Is Alternative", + "print_hide": 1 + }, + { + "depends_on": "is_alternative", + "fieldname": "alternative_to", + "fieldtype": "Link", + "label": "Alternative To", + "mandatory_depends_on": "is_alternative", + "options": "Item", "print_hide": 1 } ], "idx": 1, "istable": 1, "links": [], - "modified": "2023-01-24 08:48:06.290335", + "modified": "2023-01-26 07:32:02.768197", "modified_by": "Administrator", "module": "Selling", "name": "Quotation Item", From 94cacb60de00bda141537eb59d3d775004576a3d Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 30 Jan 2023 13:54:30 +0530 Subject: [PATCH 04/23] feat: Filter rows to be mapped on server side mapping function - Pass dialog selections to `make_sales_order` - Map either original item or its alternative depending on mapping - Only qty check for simple rows (without alternatives and not an alternative itself) --- .../selling/doctype/quotation/quotation.js | 20 +++++++------ .../selling/doctype/quotation/quotation.py | 29 ++++++++++++++++++- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/erpnext/selling/doctype/quotation/quotation.js b/erpnext/selling/doctype/quotation/quotation.js index 6f75673a8e..0ea424f1b4 100644 --- a/erpnext/selling/doctype/quotation/quotation.js +++ b/erpnext/selling/doctype/quotation/quotation.js @@ -233,7 +233,9 @@ erpnext.selling.QuotationController = class QuotationController extends erpnext. show_alternative_item_dialog() { // Create a `{original item: [alternate items]}` map - const item_alt_map = {}; + var me = this; + let item_alt_map = {}; + this.frm.doc.items.filter( (item) => item.is_alternative ).forEach((item) => @@ -286,7 +288,14 @@ erpnext.selling.QuotationController = class QuotationController extends erpnext. }, ], primary_action: function() { - this.hide(); + frappe.model.open_mapped_doc({ + method: "erpnext.selling.doctype.quotation.quotation.make_sales_order", + frm: me.frm, + args: { + mapping: dialog.get_value("alternative_items") + } + }); + dialog.hide(); }, primary_action_label: __('Continue') }); @@ -297,13 +306,6 @@ erpnext.selling.QuotationController = class QuotationController extends erpnext. cur_frm.script_manager.make(erpnext.selling.QuotationController); -cur_frm.cscript['Make Sales Order'] = function() { - frappe.model.open_mapped_doc({ - method: "erpnext.selling.doctype.quotation.quotation.make_sales_order", - frm: cur_frm - }) -} - frappe.ui.form.on("Quotation Item", "items_on_form_rendered", "packed_items_on_form_rendered", function(frm, cdt, cdn) { // enable tax_amount field if Actual }) diff --git a/erpnext/selling/doctype/quotation/quotation.py b/erpnext/selling/doctype/quotation/quotation.py index 6836d56647..d4ae66e53b 100644 --- a/erpnext/selling/doctype/quotation/quotation.py +++ b/erpnext/selling/doctype/quotation/quotation.py @@ -210,6 +210,10 @@ def _make_sales_order(source_name, target_doc=None, ignore_permissions=False): ) ) + alternative_map = { + x.get("original_item") : x.get("alternative_item") for x in frappe.flags.get("args", {}).get("mapping", []) + } + def set_missing_values(source, target): if customer: target.customer = customer.name @@ -233,6 +237,29 @@ def _make_sales_order(source_name, target_doc=None, ignore_permissions=False): target.blanket_order = obj.blanket_order target.blanket_order_rate = obj.blanket_order_rate + def can_map_row(item) -> bool: + """ + Row mapping from Quotation to Sales order: + 1. Simple row: Map if adequate qty + 2. Has Alternative Item: Map if no alternative was selected against original item and #1 + 3. Is Alternative Item: Map if alternative was selected against original item and #1 + """ + has_qty = item.qty > 0 + + has_alternative = item.item_code in alternative_map + is_alternative = item.is_alternative + + if not alternative_map or not (is_alternative or has_alternative): + # No alternative items in doc or current row is a simple item (without alternatives) + return has_qty + + if is_alternative: + is_selected = alternative_map.get(item.alternative_to) == item.item_code + else: + is_selected = alternative_map.get(item.item_code) is None + return is_selected and has_qty + + doclist = get_mapped_doc( "Quotation", source_name, @@ -242,7 +269,7 @@ def _make_sales_order(source_name, target_doc=None, ignore_permissions=False): "doctype": "Sales Order Item", "field_map": {"parent": "prevdoc_docname", "name": "quotation_item"}, "postprocess": update_item, - "condition": lambda doc: doc.qty > 0, + "condition": can_map_row, }, "Sales Taxes and Charges": {"doctype": "Sales Taxes and Charges", "add_if_empty": True}, "Sales Team": {"doctype": "Sales Team", "add_if_empty": True}, From fa9b327501a33850374f69f64dcf27ac5b2f2ae3 Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 30 Jan 2023 16:27:01 +0530 Subject: [PATCH 05/23] chore: Validate 'alternative_to' field values, must be a valid non-alterntaive item from table --- erpnext/selling/doctype/quotation/quotation.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/erpnext/selling/doctype/quotation/quotation.py b/erpnext/selling/doctype/quotation/quotation.py index d4ae66e53b..f5613bab15 100644 --- a/erpnext/selling/doctype/quotation/quotation.py +++ b/erpnext/selling/doctype/quotation/quotation.py @@ -28,6 +28,7 @@ class Quotation(SellingController): self.validate_valid_till() self.validate_shopping_cart_items() self.set_customer_name() + self.validate_alternative_items() if self.items: self.with_items = 1 @@ -99,6 +100,21 @@ class Quotation(SellingController): ) self.customer_name = company_name or lead_name + def validate_alternative_items(self): + items_with_alternatives = filter(lambda item: not item.is_alternative, self.get("items")) + items_with_alternatives = map(lambda item: item.item_code, items_with_alternatives) + + alternative_items = filter(lambda item: item.is_alternative, self.get("items")) + for row in alternative_items: + if row.alternative_to not in items_with_alternatives: + frappe.throw( + _("Row #{0}: {1} is not a valid non-alternative Item from the table").format( + row.idx, frappe.bold(row.alternative_to) + ), + title=_("Invalid Item"), + ) + + def update_opportunity(self, status): for opportunity in set(d.prevdoc_docname for d in self.get("items")): if opportunity: From ece6358e60f5c0bfae6129e3d9613de0d9a40402 Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 31 Jan 2023 17:13:05 +0530 Subject: [PATCH 06/23] fix: Iterate over list instead of map's output and formatting --- erpnext/selling/doctype/quotation/quotation.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/erpnext/selling/doctype/quotation/quotation.py b/erpnext/selling/doctype/quotation/quotation.py index f5613bab15..98d5c91179 100644 --- a/erpnext/selling/doctype/quotation/quotation.py +++ b/erpnext/selling/doctype/quotation/quotation.py @@ -102,7 +102,7 @@ class Quotation(SellingController): def validate_alternative_items(self): items_with_alternatives = filter(lambda item: not item.is_alternative, self.get("items")) - items_with_alternatives = map(lambda item: item.item_code, items_with_alternatives) + items_with_alternatives = list(map(lambda item: item.item_code, items_with_alternatives)) alternative_items = filter(lambda item: item.is_alternative, self.get("items")) for row in alternative_items: @@ -114,7 +114,6 @@ class Quotation(SellingController): title=_("Invalid Item"), ) - def update_opportunity(self, status): for opportunity in set(d.prevdoc_docname for d in self.get("items")): if opportunity: @@ -227,7 +226,8 @@ def _make_sales_order(source_name, target_doc=None, ignore_permissions=False): ) alternative_map = { - x.get("original_item") : x.get("alternative_item") for x in frappe.flags.get("args", {}).get("mapping", []) + x.get("original_item"): x.get("alternative_item") + for x in frappe.flags.get("args", {}).get("mapping", []) } def set_missing_values(source, target): @@ -253,7 +253,7 @@ def _make_sales_order(source_name, target_doc=None, ignore_permissions=False): target.blanket_order = obj.blanket_order target.blanket_order_rate = obj.blanket_order_rate - def can_map_row(item) -> bool: + def can_map_row(item) -> bool: """ Row mapping from Quotation to Sales order: 1. Simple row: Map if adequate qty @@ -275,7 +275,6 @@ def _make_sales_order(source_name, target_doc=None, ignore_permissions=False): is_selected = alternative_map.get(item.item_code) is None return is_selected and has_qty - doclist = get_mapped_doc( "Quotation", source_name, From b3fe7c6dad442c5000959dde8537bfcdf6b55390 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 1 Feb 2023 19:10:32 +0530 Subject: [PATCH 07/23] fix: Consider only ordered alternative/original item for Quotation status - The original and its alternatives make a set of items where one is chosen - While setting order status of Quotation, check if the chosen item from the set is fully ordered or not - Filter out unselected items from the set - Create a map containing the set of items and if they were ordered or not for ease of grouping - The simple items will work as it used to --- .../selling/doctype/quotation/quotation.py | 68 +++++++++++++++++-- 1 file changed, 62 insertions(+), 6 deletions(-) diff --git a/erpnext/selling/doctype/quotation/quotation.py b/erpnext/selling/doctype/quotation/quotation.py index 98d5c91179..6ef1458f49 100644 --- a/erpnext/selling/doctype/quotation/quotation.py +++ b/erpnext/selling/doctype/quotation/quotation.py @@ -61,6 +61,7 @@ class Quotation(SellingController): ) def get_ordered_status(self): + status = "Open" ordered_items = frappe._dict( frappe.db.get_all( "Sales Order Item", @@ -71,16 +72,35 @@ class Quotation(SellingController): ) ) - status = "Open" - if ordered_items: + if not ordered_items: + return status + + alternative_items = list(filter(lambda row: row.is_alternative, self.get("items"))) + self._items = self.get_valid_items(alternative_items) if alternative_items else self.get("items") + + if any(row.qty > ordered_items.get(row.item_code, 0.0) for row in self._items): + status = "Partially Ordered" + else: status = "Ordered" - for item in self.get("items"): - if item.qty > ordered_items.get(item.item_code, 0.0): - status = "Partially Ordered" - return status + def get_valid_items(self, alternative_items): + """ + Filters out unordered alternative items/original item from table. + """ + alternatives_map = self.get_alternative_item_map(alternative_items) + + def can_map(row) -> bool: + if row.is_alternative: + return alternatives_map[row.alternative_to][row.item_code] + elif row.item_code in alternatives_map: + return alternatives_map[row.item_code][row.item_code] + else: + return True + + return list(filter(can_map, self.get("items"))) + def is_fully_ordered(self): return self.get_ordered_status() == "Ordered" @@ -114,6 +134,42 @@ class Quotation(SellingController): title=_("Invalid Item"), ) + def get_alternative_item_map(self, alternative_items): + """ + Returns a map of alternatives & the original item with which one was selected by the Customer. + This is to identify sets of alternative-original items from the table. + Structure: + { + 'Original Item': {'Original Item': False, 'Alt-1': True, 'Alt-2': False} + } + """ + alternatives_map = {} + + def add_to_map(row): + in_sales_order = frappe.db.exists( + "Sales Order Item", {"quotation_item": row.name, "item_code": row.item_code} + ) + alternatives_map[row.alternative_to][row.item_code] = bool(in_sales_order) + + for row in alternative_items: + if not alternatives_map.get(row.alternative_to): + alternatives_map.setdefault(row.alternative_to, {}) + add_to_map(row) + + original_item_row = frappe._dict( + name=frappe.get_value( + "Quotation Item", {"item_code": row.alternative_to, "is_alternative": 0} + ), + item_code=row.alternative_to, + alternative_to=row.alternative_to, + ) + add_to_map(original_item_row) + continue + + add_to_map(row) + + return alternatives_map + def update_opportunity(self, status): for opportunity in set(d.prevdoc_docname for d in self.get("items")): if opportunity: From 03321f5f1396bb386b08d289db827962c9b6cbc3 Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 2 Feb 2023 17:28:03 +0530 Subject: [PATCH 08/23] chore: Code simplification - Map is not required, avoid filter multiple times, use single loop instead - Better variable name - Reduce LOC --- .../selling/doctype/quotation/quotation.js | 2 +- .../selling/doctype/quotation/quotation.py | 80 +++++++------------ 2 files changed, 31 insertions(+), 51 deletions(-) diff --git a/erpnext/selling/doctype/quotation/quotation.js b/erpnext/selling/doctype/quotation/quotation.js index 0ea424f1b4..183619e6f3 100644 --- a/erpnext/selling/doctype/quotation/quotation.js +++ b/erpnext/selling/doctype/quotation/quotation.js @@ -232,10 +232,10 @@ erpnext.selling.QuotationController = class QuotationController extends erpnext. } show_alternative_item_dialog() { - // Create a `{original item: [alternate items]}` map var me = this; let item_alt_map = {}; + // Create a `{original item: [alternate items]}` map this.frm.doc.items.filter( (item) => item.is_alternative ).forEach((item) => diff --git a/erpnext/selling/doctype/quotation/quotation.py b/erpnext/selling/doctype/quotation/quotation.py index 6ef1458f49..d7882c9eb4 100644 --- a/erpnext/selling/doctype/quotation/quotation.py +++ b/erpnext/selling/doctype/quotation/quotation.py @@ -75,8 +75,8 @@ class Quotation(SellingController): if not ordered_items: return status - alternative_items = list(filter(lambda row: row.is_alternative, self.get("items"))) - self._items = self.get_valid_items(alternative_items) if alternative_items else self.get("items") + has_alternatives = any(row.is_alternative for row in self.get("items")) + self._items = self.get_valid_items() if has_alternatives else self.get("items") if any(row.qty > ordered_items.get(row.item_code, 0.0) for row in self._items): status = "Partially Ordered" @@ -85,19 +85,26 @@ class Quotation(SellingController): return status - def get_valid_items(self, alternative_items): + def get_valid_items(self): """ - Filters out unordered alternative items/original item from table. + Filters out items in an alternatives set that were not ordered. """ - alternatives_map = self.get_alternative_item_map(alternative_items) + + def is_in_sales_order(row): + in_sales_order = bool( + frappe.db.exists( + "Sales Order Item", {"quotation_item": row.name, "item_code": row.item_code, "docstatus": 1} + ) + ) + return in_sales_order + + items_with_alternatives = self.get_items_having_alternatives() def can_map(row) -> bool: - if row.is_alternative: - return alternatives_map[row.alternative_to][row.item_code] - elif row.item_code in alternatives_map: - return alternatives_map[row.item_code][row.item_code] - else: - return True + if row.is_alternative or (row.item_code in items_with_alternatives): + return is_in_sales_order(row) + + return True return list(filter(can_map, self.get("items"))) @@ -121,12 +128,16 @@ class Quotation(SellingController): self.customer_name = company_name or lead_name def validate_alternative_items(self): - items_with_alternatives = filter(lambda item: not item.is_alternative, self.get("items")) - items_with_alternatives = list(map(lambda item: item.item_code, items_with_alternatives)) + if not any(row.is_alternative for row in self.get("items")): + return + + non_alternative_items = filter(lambda item: not item.is_alternative, self.get("items")) + non_alternative_items = list(map(lambda item: item.item_code, non_alternative_items)) alternative_items = filter(lambda item: item.is_alternative, self.get("items")) + for row in alternative_items: - if row.alternative_to not in items_with_alternatives: + if row.alternative_to not in non_alternative_items: frappe.throw( _("Row #{0}: {1} is not a valid non-alternative Item from the table").format( row.idx, frappe.bold(row.alternative_to) @@ -134,42 +145,6 @@ class Quotation(SellingController): title=_("Invalid Item"), ) - def get_alternative_item_map(self, alternative_items): - """ - Returns a map of alternatives & the original item with which one was selected by the Customer. - This is to identify sets of alternative-original items from the table. - Structure: - { - 'Original Item': {'Original Item': False, 'Alt-1': True, 'Alt-2': False} - } - """ - alternatives_map = {} - - def add_to_map(row): - in_sales_order = frappe.db.exists( - "Sales Order Item", {"quotation_item": row.name, "item_code": row.item_code} - ) - alternatives_map[row.alternative_to][row.item_code] = bool(in_sales_order) - - for row in alternative_items: - if not alternatives_map.get(row.alternative_to): - alternatives_map.setdefault(row.alternative_to, {}) - add_to_map(row) - - original_item_row = frappe._dict( - name=frappe.get_value( - "Quotation Item", {"item_code": row.alternative_to, "is_alternative": 0} - ), - item_code=row.alternative_to, - alternative_to=row.alternative_to, - ) - add_to_map(original_item_row) - continue - - add_to_map(row) - - return alternatives_map - def update_opportunity(self, status): for opportunity in set(d.prevdoc_docname for d in self.get("items")): if opportunity: @@ -247,6 +222,11 @@ class Quotation(SellingController): def on_recurring(self, reference_doc, auto_repeat_doc): self.valid_till = None + def get_items_having_alternatives(self): + alternative_items = filter(lambda item: item.is_alternative, self.get("items")) + items_with_alternatives = set((map(lambda item: item.alternative_to, alternative_items))) + return items_with_alternatives + def get_list_context(context=None): from erpnext.controllers.website_list_for_contact import get_list_context From db2076db693a54a8962588ba26a31682a1acc99f Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 6 Feb 2023 16:25:38 +0530 Subject: [PATCH 09/23] refactor: Order based alternative items mapping - Alternatives must be followed by a non-alternative item row - On submit, store non-alternative rows in hidden checkbox to avoid recomputation - Check for valid/mappable rows by row name - UI: Select from table rows.Add single row for original/alternative item in dialog - UI: Indicator for alternative items in dialog grid - UI: Indicator legend and description of table - DB: Added check field 'Has Alternative Item' not to be confused with 'Has Alternative' in Mfg --- .../selling/doctype/quotation/quotation.js | 93 ++++++++++++------- .../selling/doctype/quotation/quotation.py | 72 +++++++------- .../quotation_item/quotation_item.json | 18 ++-- 3 files changed, 102 insertions(+), 81 deletions(-) diff --git a/erpnext/selling/doctype/quotation/quotation.js b/erpnext/selling/doctype/quotation/quotation.js index 183619e6f3..a67f9b05cc 100644 --- a/erpnext/selling/doctype/quotation/quotation.js +++ b/erpnext/selling/doctype/quotation/quotation.js @@ -146,7 +146,7 @@ erpnext.selling.QuotationController = class QuotationController extends erpnext. let has_alternative_item = this.frm.doc.items.some((item) => item.is_alternative); if (has_alternative_item) { - this.show_alternative_item_dialog(); + this.show_alternative_items_dialog(); } else { frappe.model.open_mapped_doc({ method: "erpnext.selling.doctype.quotation.quotation.make_sales_order", @@ -231,60 +231,83 @@ erpnext.selling.QuotationController = class QuotationController extends erpnext. }) } - show_alternative_item_dialog() { + show_alternative_items_dialog() { var me = this; - let item_alt_map = {}; - // Create a `{original item: [alternate items]}` map - this.frm.doc.items.filter( - (item) => item.is_alternative - ).forEach((item) => - (item_alt_map[item.alternative_to] ??= []).push(item.item_code) - ) - - const fields = [{ - fieldtype:"Link", - fieldname:"original_item", - options: "Item", - label: __("Original Item"), + const table_fields = [ + { + fieldtype:"Data", + fieldname:"name", + label: __("Name"), read_only: 1, - in_list_view: 1, }, { fieldtype:"Link", - fieldname:"alternative_item", + fieldname:"item_code", options: "Item", - label: __("Alternative Item"), + label: __("Item Code"), + read_only: 1, in_list_view: 1, - get_query: (row, cdt, cdn) => { - return { - filters: { - "item_code": ["in", item_alt_map[row.original_item]] - } - } - }, + columns: 2, + formatter: (value, df, options, doc) => { + return doc.is_alternative ? `${value}` : value; + } + }, + { + fieldtype:"Data", + fieldname:"description", + label: __("Description"), + in_list_view: 1, + read_only: 1, + }, + { + fieldtype:"Currency", + fieldname:"amount", + label: __("Amount"), + options: "currency", + in_list_view: 1, + read_only: 1, + }, + { + fieldtype:"Check", + fieldname:"is_alternative", + label: __("Is Alternative"), + read_only: 1, }]; - this.data = Object.keys(item_alt_map).map((item) => { - return {"original_item": item} + + this.data = this.frm.doc.items.filter( + (item) => item.is_alternative || item.has_alternative_item + ).map((item) => { + return { + "name": item.name, + "item_code": item.item_code, + "description": item.description, + "amount": item.amount, + "is_alternative": item.is_alternative, + } }); const dialog = new frappe.ui.Dialog({ - title: __("Select Alternatives for Sales Order"), + title: __("Select Alternative Items for Sales Order"), fields: [ + { + fieldname: "info", + fieldtype: "HTML", + read_only: 1 + }, { fieldname: "alternative_items", fieldtype: "Table", - label: "Items with Alternatives", cannot_add_rows: true, in_place_edit: true, reqd: 1, data: this.data, - description: __("Select an alternative to be used in the Sales Order or leave it blank to use the original item."), + description: __("Select an item from each set to be used in the Sales Order."), get_data: () => { return this.data; }, - fields: fields + fields: table_fields }, ], primary_action: function() { @@ -292,7 +315,7 @@ erpnext.selling.QuotationController = class QuotationController extends erpnext. method: "erpnext.selling.doctype.quotation.quotation.make_sales_order", frm: me.frm, args: { - mapping: dialog.get_value("alternative_items") + selected_items: dialog.fields_dict.alternative_items.grid.get_selected_children() } }); dialog.hide(); @@ -300,6 +323,12 @@ erpnext.selling.QuotationController = class QuotationController extends erpnext. primary_action_label: __('Continue') }); + dialog.fields_dict.info.$wrapper.html( + `

+ + Alternative Items +

` + ) dialog.show(); } }; diff --git a/erpnext/selling/doctype/quotation/quotation.py b/erpnext/selling/doctype/quotation/quotation.py index d7882c9eb4..a4a5667f8e 100644 --- a/erpnext/selling/doctype/quotation/quotation.py +++ b/erpnext/selling/doctype/quotation/quotation.py @@ -28,7 +28,6 @@ class Quotation(SellingController): self.validate_valid_till() self.validate_shopping_cart_items() self.set_customer_name() - self.validate_alternative_items() if self.items: self.with_items = 1 @@ -36,6 +35,9 @@ class Quotation(SellingController): make_packing_list(self) + def before_submit(self): + self.set_has_alternative_item() + def validate_valid_till(self): if self.valid_till and getdate(self.valid_till) < getdate(self.transaction_date): frappe.throw(_("Valid till date cannot be before transaction date")) @@ -60,6 +62,16 @@ class Quotation(SellingController): title=_("Unpublished Item"), ) + def set_has_alternative_item(self): + """Mark 'Has Alternative Item' for rows.""" + if not any(row.is_alternative for row in self.get("items")): + return + + items_with_alternatives = self.get_rows_with_alternatives() + for row in self.get("items"): + if not row.is_alternative and row.name in items_with_alternatives: + row.has_alternative_item = 1 + def get_ordered_status(self): status = "Open" ordered_items = frappe._dict( @@ -98,10 +110,8 @@ class Quotation(SellingController): ) return in_sales_order - items_with_alternatives = self.get_items_having_alternatives() - def can_map(row) -> bool: - if row.is_alternative or (row.item_code in items_with_alternatives): + if row.is_alternative or row.has_alternative_item: return is_in_sales_order(row) return True @@ -127,24 +137,6 @@ class Quotation(SellingController): ) self.customer_name = company_name or lead_name - def validate_alternative_items(self): - if not any(row.is_alternative for row in self.get("items")): - return - - non_alternative_items = filter(lambda item: not item.is_alternative, self.get("items")) - non_alternative_items = list(map(lambda item: item.item_code, non_alternative_items)) - - alternative_items = filter(lambda item: item.is_alternative, self.get("items")) - - for row in alternative_items: - if row.alternative_to not in non_alternative_items: - frappe.throw( - _("Row #{0}: {1} is not a valid non-alternative Item from the table").format( - row.idx, frappe.bold(row.alternative_to) - ), - title=_("Invalid Item"), - ) - def update_opportunity(self, status): for opportunity in set(d.prevdoc_docname for d in self.get("items")): if opportunity: @@ -222,10 +214,21 @@ class Quotation(SellingController): def on_recurring(self, reference_doc, auto_repeat_doc): self.valid_till = None - def get_items_having_alternatives(self): - alternative_items = filter(lambda item: item.is_alternative, self.get("items")) - items_with_alternatives = set((map(lambda item: item.alternative_to, alternative_items))) - return items_with_alternatives + def get_rows_with_alternatives(self): + rows_with_alternatives = [] + table_length = len(self.get("items")) + + for idx, row in enumerate(self.get("items")): + if row.is_alternative: + continue + + if idx == (table_length - 1): + break + + if self.get("items")[idx + 1].is_alternative: + rows_with_alternatives.append(row.name) + + return rows_with_alternatives def get_list_context(context=None): @@ -261,10 +264,7 @@ def _make_sales_order(source_name, target_doc=None, ignore_permissions=False): ) ) - alternative_map = { - x.get("original_item"): x.get("alternative_item") - for x in frappe.flags.get("args", {}).get("mapping", []) - } + selected_rows = [x.get("name") for x in frappe.flags.get("args", {}).get("selected_items", [])] def set_missing_values(source, target): if customer: @@ -297,19 +297,11 @@ def _make_sales_order(source_name, target_doc=None, ignore_permissions=False): 3. Is Alternative Item: Map if alternative was selected against original item and #1 """ has_qty = item.qty > 0 - - has_alternative = item.item_code in alternative_map - is_alternative = item.is_alternative - - if not alternative_map or not (is_alternative or has_alternative): + if not (item.is_alternative or item.has_alternative_item): # No alternative items in doc or current row is a simple item (without alternatives) return has_qty - if is_alternative: - is_selected = alternative_map.get(item.alternative_to) == item.item_code - else: - is_selected = alternative_map.get(item.item_code) is None - return is_selected and has_qty + return (item.name in selected_rows) and has_qty doclist = get_mapped_doc( "Quotation", diff --git a/erpnext/selling/doctype/quotation_item/quotation_item.json b/erpnext/selling/doctype/quotation_item/quotation_item.json index f62a0997dc..f2aabc5240 100644 --- a/erpnext/selling/doctype/quotation_item/quotation_item.json +++ b/erpnext/selling/doctype/quotation_item/quotation_item.json @@ -50,7 +50,7 @@ "stock_uom_rate", "is_free_item", "is_alternative", - "alternative_to", + "has_alternative_item", "section_break_43", "valuation_rate", "column_break_45", @@ -654,19 +654,19 @@ "print_hide": 1 }, { - "depends_on": "is_alternative", - "fieldname": "alternative_to", - "fieldtype": "Link", - "label": "Alternative To", - "mandatory_depends_on": "is_alternative", - "options": "Item", - "print_hide": 1 + "default": "0", + "fieldname": "has_alternative_item", + "fieldtype": "Check", + "hidden": 1, + "label": "Has Alternative Item", + "print_hide": 1, + "read_only": 1 } ], "idx": 1, "istable": 1, "links": [], - "modified": "2023-01-26 07:32:02.768197", + "modified": "2023-02-06 11:00:07.042364", "modified_by": "Administrator", "module": "Selling", "name": "Quotation Item", From 74fab53e281b42c2eb3436ae3145d820108b1c13 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 15 Feb 2023 15:01:55 +0530 Subject: [PATCH 10/23] test: Alternative items in Quotation - Taxes and totals, mapping, back updation --- .../doctype/quotation/test_quotation.py | 133 ++++++++++++++++++ 1 file changed, 133 insertions(+) diff --git a/erpnext/selling/doctype/quotation/test_quotation.py b/erpnext/selling/doctype/quotation/test_quotation.py index cdf5f5d00c..67f6518657 100644 --- a/erpnext/selling/doctype/quotation/test_quotation.py +++ b/erpnext/selling/doctype/quotation/test_quotation.py @@ -457,6 +457,139 @@ class TestQuotation(FrappeTestCase): expected_index = id + 1 self.assertEqual(item.idx, expected_index) + def test_alternative_items_with_stock_items(self): + """ + Check if taxes & totals considers only non-alternative items with: + - One set of non-alternative & alternative items [first 3 rows] + - One simple stock item + """ + from erpnext.stock.doctype.item.test_item import make_item + + item_list = [] + stock_items = { + "_Test Simple Item 1": 100, + "_Test Alt 1": 120, + "_Test Alt 2": 110, + "_Test Simple Item 2": 200, + } + + for item, rate in stock_items.items(): + make_item(item, {"is_stock_item": 1}) + item_list.append( + { + "item_code": item, + "qty": 1, + "rate": rate, + "is_alternative": bool("Alt" in item), + } + ) + + quotation = make_quotation(item_list=item_list, do_not_submit=1) + quotation.append( + "taxes", + { + "account_head": "_Test Account VAT - _TC", + "charge_type": "On Net Total", + "cost_center": "_Test Cost Center - _TC", + "description": "VAT", + "doctype": "Sales Taxes and Charges", + "rate": 10, + }, + ) + quotation.submit() + + self.assertEqual(quotation.net_total, 300) + self.assertEqual(quotation.grand_total, 330) + + def test_alternative_items_with_service_items(self): + """ + Check if taxes & totals considers only non-alternative items with: + - One set of non-alternative & alternative service items [first 3 rows] + - One simple non-alternative service item + All having the same item code and unique item name/description due to + dynamic services + """ + from erpnext.stock.doctype.item.test_item import make_item + + item_list = [] + service_items = { + "Tiling with Standard Tiles": 100, + "Alt Tiling with Durable Tiles": 150, + "Alt Tiling with Premium Tiles": 180, + "False Ceiling with Material #234": 190, + } + + make_item("_Test Dynamic Service Item", {"is_stock_item": 0}) + + for name, rate in service_items.items(): + item_list.append( + { + "item_code": "_Test Dynamic Service Item", + "item_name": name, + "description": name, + "qty": 1, + "rate": rate, + "is_alternative": bool("Alt" in name), + } + ) + + quotation = make_quotation(item_list=item_list, do_not_submit=1) + quotation.append( + "taxes", + { + "account_head": "_Test Account VAT - _TC", + "charge_type": "On Net Total", + "cost_center": "_Test Cost Center - _TC", + "description": "VAT", + "doctype": "Sales Taxes and Charges", + "rate": 10, + }, + ) + quotation.submit() + + self.assertEqual(quotation.net_total, 290) + self.assertEqual(quotation.grand_total, 319) + + def test_alternative_items_sales_order_mapping_with_stock_items(self): + from erpnext.selling.doctype.quotation.quotation import make_sales_order + from erpnext.stock.doctype.item.test_item import make_item + + frappe.flags.args = frappe._dict() + item_list = [] + stock_items = { + "_Test Simple Item 1": 100, + "_Test Alt 1": 120, + "_Test Alt 2": 110, + "_Test Simple Item 2": 200, + } + + for item, rate in stock_items.items(): + make_item(item, {"is_stock_item": 1}) + item_list.append( + { + "item_code": item, + "qty": 1, + "rate": rate, + "is_alternative": bool("Alt" in item), + "warehouse": "_Test Warehouse - _TC", + } + ) + + quotation = make_quotation(item_list=item_list) + + frappe.flags.args.selected_items = [quotation.items[2]] + sales_order = make_sales_order(quotation.name) + sales_order.delivery_date = add_days(sales_order.transaction_date, 10) + sales_order.save() + + self.assertEqual(sales_order.items[0].item_code, "_Test Alt 2") + self.assertEqual(sales_order.items[1].item_code, "_Test Simple Item 2") + self.assertEqual(sales_order.net_total, 310) + + sales_order.submit() + quotation.reload() + self.assertEqual(quotation.status, "Ordered") + test_records = frappe.get_test_records("Quotation") From 3c96791d52f06b611ea14a2814a651af2ecd7649 Mon Sep 17 00:00:00 2001 From: Marica Date: Mon, 20 Feb 2023 12:04:14 +0530 Subject: [PATCH 11/23] fix: Use block variable Co-authored-by: Deepesh Garg --- erpnext/selling/doctype/quotation/quotation.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/selling/doctype/quotation/quotation.js b/erpnext/selling/doctype/quotation/quotation.js index f77dce8f1a..81ef44d53e 100644 --- a/erpnext/selling/doctype/quotation/quotation.js +++ b/erpnext/selling/doctype/quotation/quotation.js @@ -236,7 +236,7 @@ erpnext.selling.QuotationController = class QuotationController extends erpnext. } show_alternative_items_dialog() { - var me = this; + let me = this; const table_fields = [ { From 19456127cfde02bdf6873da5dff89ea519e5dddd Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 20 Feb 2023 20:52:14 +0530 Subject: [PATCH 12/23] fix: Handle `Get Items From` in Sales Order - Map all non alternatives from Quotation to SO if no selected items - Show disclaimer mentioning that Qtns with alternatives must be mapped to SO from the Qtn form --- erpnext/selling/doctype/quotation/quotation.py | 18 +++++++++++------- .../selling/doctype/sales_order/sales_order.js | 13 +++++++++++-- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/erpnext/selling/doctype/quotation/quotation.py b/erpnext/selling/doctype/quotation/quotation.py index 185f63c345..b5eddce89e 100644 --- a/erpnext/selling/doctype/quotation/quotation.py +++ b/erpnext/selling/doctype/quotation/quotation.py @@ -303,16 +303,20 @@ def _make_sales_order(source_name, target_doc=None, ignore_permissions=False): def can_map_row(item) -> bool: """ Row mapping from Quotation to Sales order: - 1. Simple row: Map if adequate qty - 2. Has Alternative Item: Map if no alternative was selected against original item and #1 - 3. Is Alternative Item: Map if alternative was selected against original item and #1 + 1. If no selections, map all non-alternative rows (that sum up to the grand total) + 2. If selections: Is Alternative Item/Has Alternative Item: Map if selected and adequate qty + 3. If selections: Simple row: Map if adequate qty """ has_qty = item.qty > 0 - if not (item.is_alternative or item.has_alternative_item): - # No alternative items in doc or current row is a simple item (without alternatives) - return has_qty - return (item.name in selected_rows) and has_qty + if not selected_rows: + return not item.is_alternative + + if selected_rows and (item.is_alternative or item.has_alternative_item): + return (item.name in selected_rows) and has_qty + + # Simple row + return has_qty doclist = get_mapped_doc( "Quotation", diff --git a/erpnext/selling/doctype/sales_order/sales_order.js b/erpnext/selling/doctype/sales_order/sales_order.js index fb64772479..a0a63f61d2 100644 --- a/erpnext/selling/doctype/sales_order/sales_order.js +++ b/erpnext/selling/doctype/sales_order/sales_order.js @@ -275,7 +275,7 @@ erpnext.selling.SalesOrderController = class SalesOrderController extends erpnex if (this.frm.doc.docstatus===0) { this.frm.add_custom_button(__('Quotation'), function() { - erpnext.utils.map_current_doc({ + let d = erpnext.utils.map_current_doc({ method: "erpnext.selling.doctype.quotation.quotation.make_sales_order", source_doctype: "Quotation", target: me.frm, @@ -293,7 +293,16 @@ erpnext.selling.SalesOrderController = class SalesOrderController extends erpnex docstatus: 1, status: ["!=", "Lost"] } - }) + }); + + setTimeout(() => { + d.$parent.append(` + + ${__("Note: Please create Sales Orders from individual Quotations to select from among Alternative Items.")} + + `); + }, 200); + }, __("Get Items From")); } From 6b789e2f0492c3e6932852507b746d1111412028 Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 20 Feb 2023 21:17:43 +0530 Subject: [PATCH 13/23] fix: Map only non alternative items from Quotation in Sales Invoice - Since there's no item selection, only Quotation selection :/ --- erpnext/selling/doctype/quotation/quotation.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/erpnext/selling/doctype/quotation/quotation.py b/erpnext/selling/doctype/quotation/quotation.py index b5eddce89e..fc66db20d2 100644 --- a/erpnext/selling/doctype/quotation/quotation.py +++ b/erpnext/selling/doctype/quotation/quotation.py @@ -396,7 +396,11 @@ def _make_sales_invoice(source_name, target_doc=None, ignore_permissions=False): source_name, { "Quotation": {"doctype": "Sales Invoice", "validation": {"docstatus": ["=", 1]}}, - "Quotation Item": {"doctype": "Sales Invoice Item", "postprocess": update_item}, + "Quotation Item": { + "doctype": "Sales Invoice Item", + "postprocess": update_item, + "condition": lambda row: not row.is_alternative, + }, "Sales Taxes and Charges": {"doctype": "Sales Taxes and Charges", "add_if_empty": True}, "Sales Team": {"doctype": "Sales Team", "add_if_empty": True}, }, From 49af5ba4349a03131e70c23a4580cb6c9c7d0c7d Mon Sep 17 00:00:00 2001 From: HarryPaulo Date: Thu, 23 Feb 2023 18:11:09 -0300 Subject: [PATCH 14/23] fix: Performance improvement when adding a new item --- erpnext/public/js/controllers/transaction.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/public/js/controllers/transaction.js b/erpnext/public/js/controllers/transaction.js index 09f2c5d5cb..2d2945473b 100644 --- a/erpnext/public/js/controllers/transaction.js +++ b/erpnext/public/js/controllers/transaction.js @@ -488,7 +488,7 @@ erpnext.TransactionController = class TransactionController extends erpnext.taxe () => { var d = locals[cdt][cdn]; me.add_taxes_from_item_tax_template(d.item_tax_rate); - if (d.free_item_data) { + if (d.free_item_data && d.free_item_data.length > 0) { me.apply_product_discount(d); } }, From eab775ef3222c2948fa643436612816f01cb52bb Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Wed, 22 Feb 2023 12:30:09 +0530 Subject: [PATCH 15/23] feat: adjust purchase receipt valuation rate as per purchase invoice rate --- .../purchase_invoice/purchase_invoice.py | 35 +++++++++++++- .../buying_settings/buying_settings.json | 11 ++++- erpnext/controllers/buying_controller.py | 5 +- .../purchase_receipt/purchase_receipt.py | 47 ++++++++++++++++++- .../purchase_receipt_item.json | 11 ++++- 5 files changed, 103 insertions(+), 6 deletions(-) diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py index 21addab240..28ed8b7da1 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py @@ -528,6 +528,32 @@ class PurchaseInvoice(BuyingController): self.update_advance_tax_references() self.process_common_party_accounting() + self.adjust_incoming_rate_of_purchase_receipt() + + def adjust_incoming_rate_of_purchase_receipt(self): + if ( + not frappe.db.get_single_value( + "Buying Settings", "adjust_incoming_rate_based_on_purchase_invoice_rate" + ) + and self.is_subcontracted + ): + return + + purchase_receipts = [] + for item in self.items: + if item.purchase_receipt and item.purchase_receipt not in purchase_receipts: + purchase_receipts.append(item.purchase_receipt) + + for purchase_receipt in purchase_receipts: + doc = frappe.get_doc("Purchase Receipt", purchase_receipt) + doc.docstatus = 2 + doc.update_stock_ledger(allow_negative_stock=True, via_landed_cost_voucher=True) + doc.make_gl_entries_on_cancel() + + doc.docstatus = 1 + doc.update_stock_ledger(allow_negative_stock=True, via_landed_cost_voucher=True) + doc.make_gl_entries() + doc.repost_future_sle_and_gle() def make_gl_entries(self, gl_entries=None, from_repost=False): if not gl_entries: @@ -1423,6 +1449,7 @@ class PurchaseInvoice(BuyingController): "Tax Withheld Vouchers", ) self.update_advance_tax_references(cancel=1) + self.adjust_incoming_rate_of_purchase_receipt() def update_project(self): project_list = [] @@ -1485,11 +1512,17 @@ class PurchaseInvoice(BuyingController): if po_details: updated_pr += update_billed_amount_based_on_po(po_details, update_modified) + adjust_incoming_rate = frappe.db.get_single_value( + "Buying Settings", "adjust_incoming_rate_based_on_purchase_invoice_rate" + ) + for pr in set(updated_pr): from erpnext.stock.doctype.purchase_receipt.purchase_receipt import update_billing_percentage pr_doc = frappe.get_doc("Purchase Receipt", pr) - update_billing_percentage(pr_doc, update_modified=update_modified) + update_billing_percentage( + pr_doc, update_modified=update_modified, adjust_incoming_rate=adjust_incoming_rate + ) def get_pr_details_billed_amt(self): # Get billed amount based on purchase receipt item reference (pr_detail) in purchase invoice diff --git a/erpnext/buying/doctype/buying_settings/buying_settings.json b/erpnext/buying/doctype/buying_settings/buying_settings.json index 652dcf0d43..7bbe89a063 100644 --- a/erpnext/buying/doctype/buying_settings/buying_settings.json +++ b/erpnext/buying/doctype/buying_settings/buying_settings.json @@ -18,6 +18,7 @@ "pr_required", "column_break_12", "maintain_same_rate", + "adjust_incoming_rate_based_on_purchase_invoice_rate", "allow_multiple_items", "bill_for_rejected_quantity_in_purchase_invoice", "disable_last_purchase_rate", @@ -147,6 +148,14 @@ "fieldname": "show_pay_button", "fieldtype": "Check", "label": "Show Pay Button in Purchase Order Portal" + }, + { + "default": "0", + "depends_on": "eval: !doc.maintain_same_rate", + "description": "Users can enable the checkbox If they want to adjust the incoming rate (set using purchase receipt) based on the purchase invoice rate.", + "fieldname": "adjust_incoming_rate_based_on_purchase_invoice_rate", + "fieldtype": "Check", + "label": "Adjust Incoming Rate Based on Purchase Invoice Rate" } ], "icon": "fa fa-cog", @@ -154,7 +163,7 @@ "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2023-02-15 14:42:10.200679", + "modified": "2023-02-20 14:25:58.544143", "modified_by": "Administrator", "module": "Buying", "name": "Buying Settings", diff --git a/erpnext/controllers/buying_controller.py b/erpnext/controllers/buying_controller.py index 4f7d9ad92e..603468d96f 100644 --- a/erpnext/controllers/buying_controller.py +++ b/erpnext/controllers/buying_controller.py @@ -265,7 +265,10 @@ class BuyingController(SubcontractingController): ) / qty_in_stock_uom else: item.valuation_rate = ( - item.base_net_amount + item.item_tax_amount + flt(item.landed_cost_voucher_amount) + item.base_net_amount + + item.item_tax_amount + + flt(item.landed_cost_voucher_amount) + + flt(item.get("adjust_incoming_rate")) ) / qty_in_stock_uom else: item.valuation_rate = 0.0 diff --git a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py index c8a4bd3d27..0f3224cffb 100644 --- a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py +++ b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py @@ -293,6 +293,7 @@ class PurchaseReceipt(BuyingController): get_purchase_document_details, ) + stock_rbnb = None if erpnext.is_perpetual_inventory_enabled(self.company): stock_rbnb = self.get_company_default("stock_received_but_not_billed") landed_cost_entries = get_item_account_wise_additional_cost(self.name) @@ -450,6 +451,21 @@ class PurchaseReceipt(BuyingController): item=d, ) + if d.adjust_incoming_rate and stock_rbnb: + account_currency = get_account_currency(stock_rbnb) + self.add_gl_entry( + gl_entries=gl_entries, + account=stock_rbnb, + cost_center=d.cost_center, + debit=0.0, + credit=flt(d.adjust_incoming_rate), + remarks=_("Adjustment based on Purchase Invoice rate"), + against_account=warehouse_account_name, + account_currency=account_currency, + project=d.project, + item=d, + ) + # sub-contracting warehouse if flt(d.rm_supp_cost) and warehouse_account.get(self.supplier_warehouse): self.add_gl_entry( @@ -470,6 +486,7 @@ class PurchaseReceipt(BuyingController): + flt(d.landed_cost_voucher_amount) + flt(d.rm_supp_cost) + flt(d.item_tax_amount) + + flt(d.adjust_incoming_rate) ) divisional_loss = flt( @@ -765,7 +782,7 @@ class PurchaseReceipt(BuyingController): updated_pr += update_billed_amount_based_on_po(po_details, update_modified) for pr in set(updated_pr): - pr_doc = self if (pr == self.name) else frappe.get_cached_doc("Purchase Receipt", pr) + pr_doc = self if (pr == self.name) else frappe.get_doc("Purchase Receipt", pr) update_billing_percentage(pr_doc, update_modified=update_modified) self.load_from_db() @@ -881,7 +898,7 @@ def get_billed_amount_against_po(po_items): return {d.po_detail: flt(d.billed_amt) for d in query} -def update_billing_percentage(pr_doc, update_modified=True): +def update_billing_percentage(pr_doc, update_modified=True, adjust_incoming_rate=False): # Reload as billed amount was set in db directly pr_doc.load_from_db() @@ -897,6 +914,12 @@ def update_billing_percentage(pr_doc, update_modified=True): total_amount += total_billable_amount total_billed_amount += flt(item.billed_amt) + if adjust_incoming_rate: + adjusted_amt = 0.0 + if item.billed_amt and item.amount: + adjusted_amt = flt(item.billed_amt) - flt(item.amount) + + item.db_set("adjust_incoming_rate", adjusted_amt, update_modified=False) percent_billed = round(100 * (total_billed_amount / (total_amount or 1)), 6) pr_doc.db_set("per_billed", percent_billed) @@ -906,6 +929,26 @@ def update_billing_percentage(pr_doc, update_modified=True): pr_doc.set_status(update=True) pr_doc.notify_update() + if adjust_incoming_rate: + adjust_incoming_rate_for_pr(pr_doc) + + +def adjust_incoming_rate_for_pr(doc): + doc.update_valuation_rate(reset_outgoing_rate=False) + + for item in doc.get("items"): + item.db_update() + + doc.docstatus = 2 + doc.update_stock_ledger(allow_negative_stock=True, via_landed_cost_voucher=True) + doc.make_gl_entries_on_cancel() + + # update stock & gl entries for submit state of PR + doc.docstatus = 1 + doc.update_stock_ledger(allow_negative_stock=True, via_landed_cost_voucher=True) + doc.make_gl_entries() + doc.repost_future_sle_and_gle() + def get_item_wise_returned_qty(pr_doc): items = [d.name for d in pr_doc.items] diff --git a/erpnext/stock/doctype/purchase_receipt_item/purchase_receipt_item.json b/erpnext/stock/doctype/purchase_receipt_item/purchase_receipt_item.json index 7a350b9e44..e12c583461 100644 --- a/erpnext/stock/doctype/purchase_receipt_item/purchase_receipt_item.json +++ b/erpnext/stock/doctype/purchase_receipt_item/purchase_receipt_item.json @@ -69,6 +69,7 @@ "item_tax_amount", "rm_supp_cost", "landed_cost_voucher_amount", + "adjust_incoming_rate", "billed_amt", "warehouse_and_reference", "warehouse", @@ -1007,12 +1008,20 @@ "fieldtype": "Check", "label": "Has Item Scanned", "read_only": 1 + }, + { + "fieldname": "adjust_incoming_rate", + "fieldtype": "Currency", + "label": "Adjust Incoming Rate (Purchase Invoice)", + "no_copy": 1, + "print_hide": 1, + "read_only": 1 } ], "idx": 1, "istable": 1, "links": [], - "modified": "2023-01-18 15:48:58.114923", + "modified": "2023-02-28 12:05:59.732266", "modified_by": "Administrator", "module": "Stock", "name": "Purchase Receipt Item", From 8e865537170251e8a788a73034345f12b90017d1 Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Tue, 28 Feb 2023 14:39:05 +0530 Subject: [PATCH 16/23] test: added test cases --- .../purchase_invoice/purchase_invoice.py | 27 ------ .../purchase_invoice/test_purchase_invoice.py | 88 +++++++++++++++++++ .../buying_settings/buying_settings.py | 7 ++ 3 files changed, 95 insertions(+), 27 deletions(-) diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py index 28ed8b7da1..140eaafba1 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py @@ -528,32 +528,6 @@ class PurchaseInvoice(BuyingController): self.update_advance_tax_references() self.process_common_party_accounting() - self.adjust_incoming_rate_of_purchase_receipt() - - def adjust_incoming_rate_of_purchase_receipt(self): - if ( - not frappe.db.get_single_value( - "Buying Settings", "adjust_incoming_rate_based_on_purchase_invoice_rate" - ) - and self.is_subcontracted - ): - return - - purchase_receipts = [] - for item in self.items: - if item.purchase_receipt and item.purchase_receipt not in purchase_receipts: - purchase_receipts.append(item.purchase_receipt) - - for purchase_receipt in purchase_receipts: - doc = frappe.get_doc("Purchase Receipt", purchase_receipt) - doc.docstatus = 2 - doc.update_stock_ledger(allow_negative_stock=True, via_landed_cost_voucher=True) - doc.make_gl_entries_on_cancel() - - doc.docstatus = 1 - doc.update_stock_ledger(allow_negative_stock=True, via_landed_cost_voucher=True) - doc.make_gl_entries() - doc.repost_future_sle_and_gle() def make_gl_entries(self, gl_entries=None, from_repost=False): if not gl_entries: @@ -1449,7 +1423,6 @@ class PurchaseInvoice(BuyingController): "Tax Withheld Vouchers", ) self.update_advance_tax_references(cancel=1) - self.adjust_incoming_rate_of_purchase_receipt() def update_project(self): project_list = [] diff --git a/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py b/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py index f901257ccf..d20eddfc96 100644 --- a/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py @@ -1523,6 +1523,94 @@ class TestPurchaseInvoice(unittest.TestCase, StockTestMixin): company.enable_provisional_accounting_for_non_stock_items = 0 company.save() + def test_adjust_incoming_rate(self): + frappe.db.set_single_value("Buying Settings", "maintain_same_rate", 0) + + frappe.db.set_single_value( + "Buying Settings", "adjust_incoming_rate_based_on_purchase_invoice_rate", 1 + ) + + # Increase the cost of the item + + pr = make_purchase_receipt(qty=1, rate=100) + + stock_value_difference = frappe.db.get_value( + "Stock Ledger Entry", + {"voucher_type": "Purchase Receipt", "voucher_no": pr.name}, + "stock_value_difference", + ) + self.assertEqual(stock_value_difference, 100) + + pi = create_purchase_invoice_from_receipt(pr.name) + for row in pi.items: + row.rate = 150 + + pi.save() + pi.submit() + + stock_value_difference = frappe.db.get_value( + "Stock Ledger Entry", + {"voucher_type": "Purchase Receipt", "voucher_no": pr.name}, + "stock_value_difference", + ) + self.assertEqual(stock_value_difference, 150) + + # Reduce the cost of the item + + pr = make_purchase_receipt(qty=1, rate=100) + + stock_value_difference = frappe.db.get_value( + "Stock Ledger Entry", + {"voucher_type": "Purchase Receipt", "voucher_no": pr.name}, + "stock_value_difference", + ) + self.assertEqual(stock_value_difference, 100) + + pi = create_purchase_invoice_from_receipt(pr.name) + for row in pi.items: + row.rate = 50 + + pi.save() + pi.submit() + + stock_value_difference = frappe.db.get_value( + "Stock Ledger Entry", + {"voucher_type": "Purchase Receipt", "voucher_no": pr.name}, + "stock_value_difference", + ) + self.assertEqual(stock_value_difference, 50) + + frappe.db.set_single_value( + "Buying Settings", "adjust_incoming_rate_based_on_purchase_invoice_rate", 0 + ) + + # Don't adjust incoming rate + + pr = make_purchase_receipt(qty=1, rate=100) + + stock_value_difference = frappe.db.get_value( + "Stock Ledger Entry", + {"voucher_type": "Purchase Receipt", "voucher_no": pr.name}, + "stock_value_difference", + ) + self.assertEqual(stock_value_difference, 100) + + pi = create_purchase_invoice_from_receipt(pr.name) + for row in pi.items: + row.rate = 50 + + pi.save() + pi.submit() + + stock_value_difference = frappe.db.get_value( + "Stock Ledger Entry", + {"voucher_type": "Purchase Receipt", "voucher_no": pr.name}, + "stock_value_difference", + ) + self.assertEqual(stock_value_difference, 100) + + frappe.db.set_single_value("Buying Settings", "maintain_same_rate", 1) + def test_item_less_defaults(self): pi = frappe.new_doc("Purchase Invoice") diff --git a/erpnext/buying/doctype/buying_settings/buying_settings.py b/erpnext/buying/doctype/buying_settings/buying_settings.py index be1ebdeb64..d14ffc90ad 100644 --- a/erpnext/buying/doctype/buying_settings/buying_settings.py +++ b/erpnext/buying/doctype/buying_settings/buying_settings.py @@ -21,3 +21,10 @@ class BuyingSettings(Document): self.get("supp_master_name") == "Naming Series", hide_name_field=False, ) + + def before_save(self): + self.check_maintain_same_rate() + + def check_maintain_same_rate(self): + if self.maintain_same_rate: + self.adjust_incoming_rate_based_on_purchase_invoice_rate = 0 From a8445da02aa48dfa2a44e426074c59f4f04e5356 Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Tue, 28 Feb 2023 15:47:45 +0530 Subject: [PATCH 17/23] fix: labels name --- .../accounts/doctype/purchase_invoice/purchase_invoice.py | 2 +- .../doctype/purchase_invoice/test_purchase_invoice.py | 4 ++-- .../buying/doctype/buying_settings/buying_settings.json | 8 ++++---- erpnext/buying/doctype/buying_settings/buying_settings.py | 2 +- erpnext/controllers/buying_controller.py | 2 +- .../stock/doctype/purchase_receipt/purchase_receipt.py | 8 ++++---- .../purchase_receipt_item/purchase_receipt_item.json | 8 ++++---- 7 files changed, 17 insertions(+), 17 deletions(-) diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py index 140eaafba1..b79af71bef 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py @@ -1486,7 +1486,7 @@ class PurchaseInvoice(BuyingController): updated_pr += update_billed_amount_based_on_po(po_details, update_modified) adjust_incoming_rate = frappe.db.get_single_value( - "Buying Settings", "adjust_incoming_rate_based_on_purchase_invoice_rate" + "Buying Settings", "set_landed_cost_based_on_purchase_invoice_rate" ) for pr in set(updated_pr): diff --git a/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py b/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py index d20eddfc96..a6d7df6971 100644 --- a/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/test_purchase_invoice.py @@ -1527,7 +1527,7 @@ class TestPurchaseInvoice(unittest.TestCase, StockTestMixin): frappe.db.set_single_value("Buying Settings", "maintain_same_rate", 0) frappe.db.set_single_value( - "Buying Settings", "adjust_incoming_rate_based_on_purchase_invoice_rate", 1 + "Buying Settings", "set_landed_cost_based_on_purchase_invoice_rate", 1 ) # Increase the cost of the item @@ -1581,7 +1581,7 @@ class TestPurchaseInvoice(unittest.TestCase, StockTestMixin): self.assertEqual(stock_value_difference, 50) frappe.db.set_single_value( - "Buying Settings", "adjust_incoming_rate_based_on_purchase_invoice_rate", 0 + "Buying Settings", "set_landed_cost_based_on_purchase_invoice_rate", 0 ) # Don't adjust incoming rate diff --git a/erpnext/buying/doctype/buying_settings/buying_settings.json b/erpnext/buying/doctype/buying_settings/buying_settings.json index 7bbe89a063..95857e4604 100644 --- a/erpnext/buying/doctype/buying_settings/buying_settings.json +++ b/erpnext/buying/doctype/buying_settings/buying_settings.json @@ -18,7 +18,7 @@ "pr_required", "column_break_12", "maintain_same_rate", - "adjust_incoming_rate_based_on_purchase_invoice_rate", + "set_landed_cost_based_on_purchase_invoice_rate", "allow_multiple_items", "bill_for_rejected_quantity_in_purchase_invoice", "disable_last_purchase_rate", @@ -153,9 +153,9 @@ "default": "0", "depends_on": "eval: !doc.maintain_same_rate", "description": "Users can enable the checkbox If they want to adjust the incoming rate (set using purchase receipt) based on the purchase invoice rate.", - "fieldname": "adjust_incoming_rate_based_on_purchase_invoice_rate", + "fieldname": "set_landed_cost_based_on_purchase_invoice_rate", "fieldtype": "Check", - "label": "Adjust Incoming Rate Based on Purchase Invoice Rate" + "label": "Set Landed Cost Based on Purchase Invoice Rate" } ], "icon": "fa fa-cog", @@ -163,7 +163,7 @@ "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2023-02-20 14:25:58.544143", + "modified": "2023-02-28 15:41:32.686805", "modified_by": "Administrator", "module": "Buying", "name": "Buying Settings", diff --git a/erpnext/buying/doctype/buying_settings/buying_settings.py b/erpnext/buying/doctype/buying_settings/buying_settings.py index d14ffc90ad..4680a889d3 100644 --- a/erpnext/buying/doctype/buying_settings/buying_settings.py +++ b/erpnext/buying/doctype/buying_settings/buying_settings.py @@ -27,4 +27,4 @@ class BuyingSettings(Document): def check_maintain_same_rate(self): if self.maintain_same_rate: - self.adjust_incoming_rate_based_on_purchase_invoice_rate = 0 + self.set_landed_cost_based_on_purchase_invoice_rate = 0 diff --git a/erpnext/controllers/buying_controller.py b/erpnext/controllers/buying_controller.py index 603468d96f..e15b61287e 100644 --- a/erpnext/controllers/buying_controller.py +++ b/erpnext/controllers/buying_controller.py @@ -268,7 +268,7 @@ class BuyingController(SubcontractingController): item.base_net_amount + item.item_tax_amount + flt(item.landed_cost_voucher_amount) - + flt(item.get("adjust_incoming_rate")) + + flt(item.get("rate_difference_with_purchase_invoice")) ) / qty_in_stock_uom else: item.valuation_rate = 0.0 diff --git a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py index 0f3224cffb..c1abd31bcc 100644 --- a/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py +++ b/erpnext/stock/doctype/purchase_receipt/purchase_receipt.py @@ -451,14 +451,14 @@ class PurchaseReceipt(BuyingController): item=d, ) - if d.adjust_incoming_rate and stock_rbnb: + if d.rate_difference_with_purchase_invoice and stock_rbnb: account_currency = get_account_currency(stock_rbnb) self.add_gl_entry( gl_entries=gl_entries, account=stock_rbnb, cost_center=d.cost_center, debit=0.0, - credit=flt(d.adjust_incoming_rate), + credit=flt(d.rate_difference_with_purchase_invoice), remarks=_("Adjustment based on Purchase Invoice rate"), against_account=warehouse_account_name, account_currency=account_currency, @@ -486,7 +486,7 @@ class PurchaseReceipt(BuyingController): + flt(d.landed_cost_voucher_amount) + flt(d.rm_supp_cost) + flt(d.item_tax_amount) - + flt(d.adjust_incoming_rate) + + flt(d.rate_difference_with_purchase_invoice) ) divisional_loss = flt( @@ -919,7 +919,7 @@ def update_billing_percentage(pr_doc, update_modified=True, adjust_incoming_rate if item.billed_amt and item.amount: adjusted_amt = flt(item.billed_amt) - flt(item.amount) - item.db_set("adjust_incoming_rate", adjusted_amt, update_modified=False) + item.db_set("rate_difference_with_purchase_invoice", adjusted_amt, update_modified=False) percent_billed = round(100 * (total_billed_amount / (total_amount or 1)), 6) pr_doc.db_set("per_billed", percent_billed) diff --git a/erpnext/stock/doctype/purchase_receipt_item/purchase_receipt_item.json b/erpnext/stock/doctype/purchase_receipt_item/purchase_receipt_item.json index e12c583461..cd320fdfcd 100644 --- a/erpnext/stock/doctype/purchase_receipt_item/purchase_receipt_item.json +++ b/erpnext/stock/doctype/purchase_receipt_item/purchase_receipt_item.json @@ -69,7 +69,7 @@ "item_tax_amount", "rm_supp_cost", "landed_cost_voucher_amount", - "adjust_incoming_rate", + "rate_difference_with_purchase_invoice", "billed_amt", "warehouse_and_reference", "warehouse", @@ -1010,9 +1010,9 @@ "read_only": 1 }, { - "fieldname": "adjust_incoming_rate", + "fieldname": "rate_difference_with_purchase_invoice", "fieldtype": "Currency", - "label": "Adjust Incoming Rate (Purchase Invoice)", + "label": "Rate Difference with Purchase Invoice", "no_copy": 1, "print_hide": 1, "read_only": 1 @@ -1021,7 +1021,7 @@ "idx": 1, "istable": 1, "links": [], - "modified": "2023-02-28 12:05:59.732266", + "modified": "2023-02-28 15:43:04.470104", "modified_by": "Administrator", "module": "Stock", "name": "Purchase Receipt Item", From 35c70f39fa28860b6c22dc3091eadb79c727a4de Mon Sep 17 00:00:00 2001 From: ruthra kumar Date: Tue, 28 Feb 2023 12:05:10 +0530 Subject: [PATCH 18/23] fix: pos return throwing amount greater than grand total --- erpnext/public/js/controllers/taxes_and_totals.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/erpnext/public/js/controllers/taxes_and_totals.js b/erpnext/public/js/controllers/taxes_and_totals.js index a87c3ec951..974b937fa2 100644 --- a/erpnext/public/js/controllers/taxes_and_totals.js +++ b/erpnext/public/js/controllers/taxes_and_totals.js @@ -131,8 +131,8 @@ erpnext.taxes_and_totals = class TaxesAndTotals extends erpnext.payments { item.net_amount = item.amount = flt(item.rate * item.qty, precision("amount", item)); } else { - let qty = item.qty || 1; - qty = me.frm.doc.is_return ? -1 * qty : qty; + // allow for '0' qty on Credit/Debit notes + let qty = item.qty || -1 item.net_amount = item.amount = flt(item.rate * qty, precision("amount", item)); } From dfddc4efc31c7853b395c64dc978cce2c37aa0db Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Tue, 28 Feb 2023 17:53:51 +0530 Subject: [PATCH 19/23] fix: default date in Subcontracting reports --- .../subcontracted_item_to_be_received.js | 4 ++-- .../subcontracted_raw_materials_to_be_transferred.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/erpnext/buying/report/subcontracted_item_to_be_received/subcontracted_item_to_be_received.js b/erpnext/buying/report/subcontracted_item_to_be_received/subcontracted_item_to_be_received.js index 6304a0908d..9db769d59b 100644 --- a/erpnext/buying/report/subcontracted_item_to_be_received/subcontracted_item_to_be_received.js +++ b/erpnext/buying/report/subcontracted_item_to_be_received/subcontracted_item_to_be_received.js @@ -22,14 +22,14 @@ frappe.query_reports["Subcontracted Item To Be Received"] = { fieldname:"from_date", label: __("From Date"), fieldtype: "Date", - default: frappe.datetime.add_months(frappe.datetime.month_start(), -1), + default: frappe.datetime.add_months(frappe.datetime.get_today(), -1), reqd: 1 }, { fieldname:"to_date", label: __("To Date"), fieldtype: "Date", - default: frappe.datetime.add_days(frappe.datetime.month_start(),-1), + default: frappe.datetime.get_today(), reqd: 1 }, ] diff --git a/erpnext/buying/report/subcontracted_raw_materials_to_be_transferred/subcontracted_raw_materials_to_be_transferred.js b/erpnext/buying/report/subcontracted_raw_materials_to_be_transferred/subcontracted_raw_materials_to_be_transferred.js index b6739fe663..7e5338f353 100644 --- a/erpnext/buying/report/subcontracted_raw_materials_to_be_transferred/subcontracted_raw_materials_to_be_transferred.js +++ b/erpnext/buying/report/subcontracted_raw_materials_to_be_transferred/subcontracted_raw_materials_to_be_transferred.js @@ -22,14 +22,14 @@ frappe.query_reports["Subcontracted Raw Materials To Be Transferred"] = { fieldname:"from_date", label: __("From Date"), fieldtype: "Date", - default: frappe.datetime.add_months(frappe.datetime.month_start(), -1), + default: frappe.datetime.add_months(frappe.datetime.get_today(), -1), reqd: 1 }, { fieldname:"to_date", label: __("To Date"), fieldtype: "Date", - default: frappe.datetime.add_days(frappe.datetime.month_start(),-1), + default: frappe.datetime.get_today(), reqd: 1 }, ] From b38fe240900da4e3ac64c7bb03ef3aecbcd09f0d Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Wed, 1 Mar 2023 11:06:46 +0530 Subject: [PATCH 20/23] fix: consumed qty validation for subcontracting receipt --- .../subcontracting_receipt.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/erpnext/subcontracting/doctype/subcontracting_receipt/subcontracting_receipt.py b/erpnext/subcontracting/doctype/subcontracting_receipt/subcontracting_receipt.py index f4fd4de169..95dbc83bf8 100644 --- a/erpnext/subcontracting/doctype/subcontracting_receipt/subcontracting_receipt.py +++ b/erpnext/subcontracting/doctype/subcontracting_receipt/subcontracting_receipt.py @@ -191,14 +191,17 @@ class SubcontractingReceipt(SubcontractingController): def validate_available_qty_for_consumption(self): for item in self.get("supplied_items"): + precision = item.precision("consumed_qty") if ( - item.available_qty_for_consumption and item.available_qty_for_consumption < item.consumed_qty + item.available_qty_for_consumption + and flt(item.available_qty_for_consumption, precision) - flt(item.consumed_qty, precision) < 0 ): - frappe.throw( - _( - "Row {0}: Consumed Qty must be less than or equal to Available Qty For Consumption in Consumed Items Table." - ).format(item.idx) - ) + msg = f"""Row {item.idx}: Consumed Qty {flt(item.consumed_qty, precision)} + must be less than or equal to Available Qty For Consumption + {flt(item.available_qty_for_consumption, precision)} + in Consumed Items Table.""" + + frappe.throw(_(msg)) def validate_items_qty(self): for item in self.items: From 46a722d51c7c4a6b210ce4553f76bf93958fcf21 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Wed, 1 Mar 2023 16:16:58 +0530 Subject: [PATCH 21/23] fix: Wrap unexpectedly long text in remark (cherry picked from commit ba66a6714c495f9d70dad79344342779c8011c6e) # Conflicts: # erpnext/accounts/report/general_ledger/general_ledger.html (cherry picked from commit b13bf1ebc517ccd9c5b6b079208ca3883e7418fd) --- .../report/general_ledger/general_ledger.html | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/erpnext/accounts/report/general_ledger/general_ledger.html b/erpnext/accounts/report/general_ledger/general_ledger.html index 475be92add..e91c05dfb8 100644 --- a/erpnext/accounts/report/general_ledger/general_ledger.html +++ b/erpnext/accounts/report/general_ledger/general_ledger.html @@ -38,8 +38,11 @@ {% if(data[i].posting_date) { %} {%= frappe.datetime.str_to_user(data[i].posting_date) %} {%= data[i].voucher_type %} -
{%= data[i].voucher_no %} - +
{%= data[i].voucher_no %} + + {% var longest_word = cstr(data[i].remarks).split(" ").reduce((longest, word) => word.length > longest.length ? word : longest, ""); %} + 45 %} class="overflow-wrap-anywhere" {% endif %}> + {% if(!(filters.party || filters.account)) { %} {%= data[i].party || data[i].account %}
@@ -49,11 +52,22 @@ {% if(data[i].bill_no) { %}
{%= __("Supplier Invoice No") %}: {%= data[i].bill_no %} {% } %} +<<<<<<< HEAD {%= format_currency(data[i].debit, filters.presentation_currency || data[i].account_currency) %} {%= format_currency(data[i].credit, filters.presentation_currency || data[i].account_currency) %} +======= + + + + {%= format_currency(data[i].debit, filters.presentation_currency) %} + + + {%= format_currency(data[i].credit, filters.presentation_currency) %} + +>>>>>>> ba66a6714c (fix: Wrap unexpectedly long text in remark) {% } else { %} From 6cd2ba5c663e6b9b89bf6938a158fe9a849f51cc Mon Sep 17 00:00:00 2001 From: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> Date: Wed, 1 Mar 2023 16:25:25 +0530 Subject: [PATCH 22/23] fix: Resolve conflicts (cherry picked from commit f6469d83981386a7bb57b41178055eae5cda30bf) --- .../accounts/report/general_ledger/general_ledger.html | 8 -------- 1 file changed, 8 deletions(-) diff --git a/erpnext/accounts/report/general_ledger/general_ledger.html b/erpnext/accounts/report/general_ledger/general_ledger.html index e91c05dfb8..2d5ca49765 100644 --- a/erpnext/accounts/report/general_ledger/general_ledger.html +++ b/erpnext/accounts/report/general_ledger/general_ledger.html @@ -52,13 +52,6 @@ {% if(data[i].bill_no) { %}
{%= __("Supplier Invoice No") %}: {%= data[i].bill_no %} {% } %} -<<<<<<< HEAD - - - {%= format_currency(data[i].debit, filters.presentation_currency || data[i].account_currency) %} - - {%= format_currency(data[i].credit, filters.presentation_currency || data[i].account_currency) %} -======= @@ -67,7 +60,6 @@ {%= format_currency(data[i].credit, filters.presentation_currency) %} ->>>>>>> ba66a6714c (fix: Wrap unexpectedly long text in remark) {% } else { %} From 330ae419be6767df1f3fe4c5f503d0496e527699 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 2 Mar 2023 11:59:18 +0530 Subject: [PATCH 23/23] fix!: Parse float as per number format in quality inspection (#34259) fix: Parse float as per number format in quality inspection --- .../quality_inspection/quality_inspection.py | 26 +++++++++++--- .../test_quality_inspection.py | 36 ++++++++++++++++++- 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/erpnext/stock/doctype/quality_inspection/quality_inspection.py b/erpnext/stock/doctype/quality_inspection/quality_inspection.py index 2a9f091bd0..9673c81e55 100644 --- a/erpnext/stock/doctype/quality_inspection/quality_inspection.py +++ b/erpnext/stock/doctype/quality_inspection/quality_inspection.py @@ -6,7 +6,7 @@ import frappe from frappe import _ from frappe.model.document import Document from frappe.model.mapper import get_mapped_doc -from frappe.utils import cint, cstr, flt +from frappe.utils import cint, cstr, flt, get_number_format_info from erpnext.stock.doctype.quality_inspection_template.quality_inspection_template import ( get_template_details, @@ -156,7 +156,9 @@ class QualityInspection(Document): for i in range(1, 11): reading_value = reading.get("reading_" + str(i)) if reading_value is not None and reading_value.strip(): - result = flt(reading.get("min_value")) <= flt(reading_value) <= flt(reading.get("max_value")) + result = ( + flt(reading.get("min_value")) <= parse_float(reading_value) <= flt(reading.get("max_value")) + ) if not result: return False return True @@ -196,7 +198,7 @@ class QualityInspection(Document): # numeric readings for i in range(1, 11): field = "reading_" + str(i) - data[field] = flt(reading.get(field)) + data[field] = parse_float(reading.get(field)) data["mean"] = self.calculate_mean(reading) return data @@ -210,7 +212,7 @@ class QualityInspection(Document): for i in range(1, 11): reading_value = reading.get("reading_" + str(i)) if reading_value is not None and reading_value.strip(): - readings_list.append(flt(reading_value)) + readings_list.append(parse_float(reading_value)) actual_mean = mean(readings_list) if readings_list else 0 return actual_mean @@ -324,3 +326,19 @@ def make_quality_inspection(source_name, target_doc=None): ) return doc + + +def parse_float(num: str) -> float: + """Since reading_# fields are `Data` field they might contain number which + is representation in user's prefered number format instead of machine + readable format. This function converts them to machine readable format.""" + + number_format = frappe.db.get_default("number_format") or "#,###.##" + decimal_str, comma_str, _number_format_precision = get_number_format_info(number_format) + + if decimal_str == "," and comma_str == ".": + num = num.replace(",", "#$") + num = num.replace(".", ",") + num = num.replace("#$", ".") + + return flt(num) diff --git a/erpnext/stock/doctype/quality_inspection/test_quality_inspection.py b/erpnext/stock/doctype/quality_inspection/test_quality_inspection.py index 4f19643ad5..9d2e139622 100644 --- a/erpnext/stock/doctype/quality_inspection/test_quality_inspection.py +++ b/erpnext/stock/doctype/quality_inspection/test_quality_inspection.py @@ -2,7 +2,7 @@ # See license.txt import frappe -from frappe.tests.utils import FrappeTestCase +from frappe.tests.utils import FrappeTestCase, change_settings from frappe.utils import nowdate from erpnext.controllers.stock_controller import ( @@ -216,6 +216,40 @@ class TestQualityInspection(FrappeTestCase): qa.save() self.assertEqual(qa.status, "Accepted") + @change_settings("System Settings", {"number_format": "#.###,##"}) + def test_diff_number_format(self): + self.assertEqual(frappe.db.get_default("number_format"), "#.###,##") # sanity check + + # Test QI based on acceptance values (Non formula) + dn = create_delivery_note(item_code="_Test Item with QA", do_not_submit=True) + readings = [ + { + "specification": "Iron Content", # numeric reading + "min_value": 60, + "max_value": 100, + "reading_1": "70,000", + }, + { + "specification": "Iron Content", # numeric reading + "min_value": 60, + "max_value": 100, + "reading_1": "1.100,00", + }, + ] + + qa = create_quality_inspection( + reference_type="Delivery Note", reference_name=dn.name, readings=readings, do_not_save=True + ) + + qa.save() + + # status must be auto set as per formula + self.assertEqual(qa.readings[0].status, "Accepted") + self.assertEqual(qa.readings[1].status, "Rejected") + + qa.delete() + dn.delete() + def create_quality_inspection(**args): args = frappe._dict(args)