From 91982d1e4f7012a6a28ed8116a65fdfd9b9f973f Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 24 Jan 2023 18:03:53 +0530 Subject: [PATCH 01/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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 b38fe240900da4e3ac64c7bb03ef3aecbcd09f0d Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Wed, 1 Mar 2023 11:06:46 +0530 Subject: [PATCH 14/16] 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 15/16] 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 16/16] 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 { %}