From e540534e420af614f194066c82c6b35a746c6da9 Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Sun, 31 Mar 2019 16:42:41 +0530 Subject: [PATCH 1/2] fix: Pricing rule not working for POS --- .../doctype/pricing_rule/pricing_rule.py | 62 +++++------- .../accounts/doctype/pricing_rule/utils.py | 98 ++++++++++++------- .../doctype/purchase_order/purchase_order.js | 3 - erpnext/controllers/accounts_controller.py | 2 +- erpnext/public/js/controllers/transaction.js | 81 ++++++++------- .../page/point_of_sale/point_of_sale.js | 14 ++- 6 files changed, 148 insertions(+), 112 deletions(-) diff --git a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py index 1e11c4e163..8c1ddbb035 100644 --- a/erpnext/accounts/doctype/pricing_rule/pricing_rule.py +++ b/erpnext/accounts/doctype/pricing_rule/pricing_rule.py @@ -119,7 +119,7 @@ class PricingRule(Document): #-------------------------------------------------------------------------------- @frappe.whitelist() -def apply_pricing_rule(args): +def apply_pricing_rule(args, doc=None): """ args = { "items": [{"doctype": "", "name": "", "item_code": "", "brand": "", "item_group": ""}, ...], @@ -139,6 +139,7 @@ def apply_pricing_rule(args): "ignore_pricing_rule": "something" } """ + if isinstance(args, string_types): args = json.loads(args) @@ -161,10 +162,11 @@ def apply_pricing_rule(args): for item in item_list: args_copy = copy.deepcopy(args) args_copy.update(item) - data = get_pricing_rule_for_item(args_copy, item.get('price_list_rate')) + data = get_pricing_rule_for_item(args_copy, item.get('price_list_rate'), doc=doc) out.append(data) if set_serial_nos_based_on_fifo and not args.get('is_return'): - out.append(get_serial_no_for_item(args_copy)) + out[0].update(get_serial_no_for_item(args_copy)) + return out def get_serial_no_for_item(args): @@ -182,6 +184,12 @@ def get_serial_no_for_item(args): def get_pricing_rule_for_item(args, price_list_rate=0, doc=None): from erpnext.accounts.doctype.pricing_rule.utils import get_pricing_rules + if isinstance(doc, string_types): + doc = json.loads(doc) + + if doc: + doc = frappe.get_doc(doc) + if (args.get('is_free_item') or args.get("parenttype") == "Material Request"): return {} @@ -227,11 +235,11 @@ def get_pricing_rule_for_item(args, price_list_rate=0, doc=None): if pricing_rules: rules = [] - item_details.discount_percentage = 0 - item_details.discount_amount = 0 for pricing_rule in pricing_rules: if not pricing_rule or pricing_rule.get('suggestion'): continue + item_details.validate_applied_rule = pricing_rule.get("validate_applied_rule", 0) + rules.append(get_pricing_rule_details(args, pricing_rule)) if pricing_rule.mixed_conditions or pricing_rule.apply_rule_on_other: continue @@ -243,8 +251,8 @@ def get_pricing_rule_for_item(args, price_list_rate=0, doc=None): item_details.has_pricing_rule = 1 # if discount is applied on the rate and not on price list rate - if price_list_rate: - set_discount_amount(price_list_rate, item_details) + # if price_list_rate: + # set_discount_amount(price_list_rate, item_details) item_details.pricing_rules = ','.join([d.pricing_rule for d in rules]) @@ -264,7 +272,7 @@ def get_pricing_rule_details(args, pricing_rule): 'pricing_rule': pricing_rule.name, 'rate_or_discount': pricing_rule.rate_or_discount, 'margin_type': pricing_rule.margin_type, - 'item_code': pricing_rule.item_code, + 'item_code': pricing_rule.item_code or args.get("item_code"), 'child_docname': args.get('child_docname') }) @@ -296,6 +304,9 @@ def apply_price_discount_pricing_rule(pricing_rule, item_details, args): discount_field = "{0}_on_rate".format(field) item_details[discount_field].append(pricing_rule.get(field, 0)) else: + if field not in item_details: + item_details.setdefault(field, 0) + item_details[field] += (pricing_rule.get(field, 0) if pricing_rule else args.get(field, 0)) @@ -308,6 +319,7 @@ def set_discount_amount(rate, item_details): item_details.rate = rate def remove_pricing_rule_for_item(pricing_rules, item_details, item_code=None): + from erpnext.accounts.doctype.pricing_rule.utils import get_apply_on_and_items for d in pricing_rules.split(','): if not d: continue pricing_rule = frappe.get_doc('Pricing Rule', d) @@ -327,6 +339,11 @@ def remove_pricing_rule_for_item(pricing_rules, item_details, item_code=None): item_details.remove_free_item = (item_code if pricing_rule.get('same_item') else pricing_rule.get('free_item')) + if pricing_rule.get("mixed_conditions") or pricing_rule.get("apply_rule_on_other"): + apply_on, items = get_apply_on_and_items(pricing_rule, item_details) + item_details.apply_on = apply_on + item_details.applied_on_items = ','.join(items) + item_details.pricing_rules = '' return item_details @@ -368,35 +385,6 @@ def make_pricing_rule(doctype, docname): return doc -@frappe.whitelist() -def get_free_items(pricing_rules, item_row): - if isinstance(item_row, string_types): - item_row = json.loads(item_row) - - free_items = [] - pricing_rules = list(set(pricing_rules.split(','))) - - for d in pricing_rules: - pr_doc = frappe.get_doc('Pricing Rule', d) - if pr_doc.price_or_product_discount == 'Product': - item = (item_row.get('item_code') if pr_doc.same_item - else pr_doc.free_item) - if not item: return free_items - - doc = frappe.get_doc('Item', item) - - free_items.append({ - 'item_code': item, - 'item_name': doc.item_name, - 'description': doc.description, - 'qty': pr_doc.free_qty, - 'uom': pr_doc.free_item_uom, - 'rate': pr_doc.free_item_rate or 0, - 'is_free_item': 1 - }) - - return free_items - def get_item_uoms(doctype, txt, searchfield, start, page_len, filters): items = [filters.get('value')] if filters.get('apply_on') != 'Item Code': diff --git a/erpnext/accounts/doctype/pricing_rule/utils.py b/erpnext/accounts/doctype/pricing_rule/utils.py index 5382baa129..936f97cba9 100644 --- a/erpnext/accounts/doctype/pricing_rule/utils.py +++ b/erpnext/accounts/doctype/pricing_rule/utils.py @@ -4,10 +4,9 @@ # For license information, please see license.txt from __future__ import unicode_literals -import frappe -import json -import copy +import frappe, copy, json from frappe import throw, _ +from six import string_types from frappe.utils import flt, cint, get_datetime from erpnext.stock.doctype.warehouse.warehouse import get_child_warehouses from erpnext.stock.get_item_details import get_conversion_factor @@ -39,7 +38,9 @@ def get_pricing_rules(args, doc=None): if pricing_rule: rules.append(pricing_rule) else: - rules.append(filter_pricing_rules(args, pricing_rules, doc)) + pricing_rule = filter_pricing_rules(args, pricing_rules, doc) + if pricing_rule: + rules.append(pricing_rule) return rules @@ -175,7 +176,7 @@ def filter_pricing_rules(args, pricing_rules, doc=None): pr_doc = frappe.get_doc('Pricing Rule', pricing_rules[0].name) if pricing_rules[0].mixed_conditions and doc: - stock_qty, amount = get_qty_and_rate_for_mixed_conditions(doc, pr_doc) + stock_qty, amount = get_qty_and_rate_for_mixed_conditions(doc, pr_doc, args) elif pricing_rules[0].is_cumulative: items = [args.get(frappe.scrub(pr_doc.get('apply_on')))] @@ -314,7 +315,7 @@ def apply_internal_priority(pricing_rules, field_set, args): return filtered_rules or pricing_rules -def get_qty_and_rate_for_mixed_conditions(doc, pr_doc): +def get_qty_and_rate_for_mixed_conditions(doc, pr_doc, args): sum_qty, sum_amt = [0, 0] items = get_pricing_rule_items(pr_doc) or [] apply_on = frappe.scrub(pr_doc.get('apply_on')) @@ -324,8 +325,12 @@ def get_qty_and_rate_for_mixed_conditions(doc, pr_doc): if row.get(apply_on) not in items: continue if pr_doc.mixed_conditions: - sum_qty += row.stock_qty - sum_amt += row.amount + amt = args.get('qty') * args.get("price_list_rate") + if args.get("item_code") != row.get("item_code"): + amt = row.get('qty') * row.get("price_list_rate") + + sum_qty += row.get("stock_qty") or args.get("stock_qty") + sum_amt += amt if pr_doc.is_cumulative: data = get_qty_amount_data_for_cumulative(pr_doc, doc, items) @@ -340,8 +345,8 @@ def get_qty_and_rate_for_other_item(doc, pr_doc, pricing_rules): for d in get_pricing_rule_items(pr_doc): for row in doc.items: if d == row.get(frappe.scrub(pr_doc.apply_on)): - pricing_rules = filter_pricing_rules_for_qty_amount(row.stock_qty, - row.amount, pricing_rules, row) + pricing_rules = filter_pricing_rules_for_qty_amount(row.get("stock_qty"), + row.get("amount"), pricing_rules, row) if pricing_rules and pricing_rules[0]: return pricing_rules @@ -395,17 +400,15 @@ def get_qty_amount_data_for_cumulative(pr_doc, doc, items=[]): def validate_pricing_rules(doc): validate_pricing_rule_on_transactions(doc) - if not doc.pricing_rules: return - for d in doc.items: validate_pricing_rule_on_items(doc, d) doc.calculate_taxes_and_totals() -def validate_pricing_rule_on_items(doc, item_row): +def validate_pricing_rule_on_items(doc, item_row, do_not_validate = False): value = 0 - for pr_row in get_applied_pricing_rules(doc, item_row): - pr_doc = frappe.get_doc('Pricing Rule', pr_row.pricing_rule) + for pricing_rule in get_applied_pricing_rules(doc, item_row): + pr_doc = frappe.get_doc('Pricing Rule', pricing_rule) if pr_doc.get('apply_on') == 'Transaction': continue @@ -416,7 +419,7 @@ def validate_pricing_rule_on_items(doc, item_row): if not pr_doc.get(field): continue value += pr_doc.get(field) - apply_pricing_rule(doc, pr_doc, pr_row, item_row, value) + apply_pricing_rule(doc, pr_doc, item_row, value, do_not_validate) def validate_pricing_rule_on_transactions(doc): conditions = "apply_on = 'Transaction'" @@ -451,8 +454,8 @@ def validate_pricing_rule_on_transactions(doc): apply_pricing_rule_for_free_items(doc, d) def get_applied_pricing_rules(doc, item_row): - return [d for d in doc.pricing_rules - if d.child_docname == item_row.name] + return (item_row.get("pricing_rules").split(',') + if item_row.get("pricing_rules") else []) def apply_pricing_rule_for_free_items(doc, pricing_rule): if pricing_rule.get('free_item'): @@ -471,7 +474,37 @@ def apply_pricing_rule_for_free_items(doc, pricing_rule): doc.set_missing_values() -def apply_pricing_rule(doc, pr_doc, pr_row, item_row, value): +def apply_pricing_rule(doc, pr_doc, item_row, value, do_not_validate=False): + apply_on, items = get_apply_on_and_items(pr_doc, item_row) + + rule_applied = {} + + for item in doc.get("items"): + if not item.pricing_rules: + item.pricing_rules = item_row.pricing_rules + + if item.get(apply_on) in items: + for field in ['discount_percentage', 'discount_amount', 'rate']: + if not pr_doc.get(field): continue + + key = (item.name, item.pricing_rules) + if not pr_doc.validate_applied_rule: + rule_applied[key] = 1 + item.set(field, value) + elif item.get(field) < value: + if not do_not_validate and item.idx == item_row.idx: + rule_applied[key] = 0 + frappe.msgprint(_("Row {0}: user has not applied rule {1} on the item {2}") + .format(item.idx, pr_doc.title, item.item_code)) + + if rule_applied and doc.pricing_rules: + for d in doc.pricing_rules: + key = (d.child_docname, d.pricing_rule) + if key in rule_applied: + d.rule_applied = 1 + +def get_apply_on_and_items(pr_doc, item_row): + # for mixed or other items conditions apply_on = frappe.scrub(pr_doc.get('apply_on')) items = (get_pricing_rule_items(pr_doc) if pr_doc.mixed_conditions else [item_row.get(apply_on)]) @@ -480,23 +513,22 @@ def apply_pricing_rule(doc, pr_doc, pr_row, item_row, value): apply_on = frappe.scrub(pr_doc.apply_rule_on_other) items = [pr_doc.get(apply_on)] - rule_applied = 1 - if item_row.get(apply_on) in items: - for field in ['discount_percentage', 'discount_amount', 'rate']: - if not pr_doc.get(field): continue - - if not pr_doc.validate_applied_rule: - item_row.set(field, value) - elif item_row.get(field) < value: - rule_applied = 0 - frappe.msgprint(_("Row {0}: user has not applied rule {1} on the item {2}") - .format(item_row.idx, pr_doc.title, item_row.item_code)) - - pr_row.rule_applied = rule_applied + return apply_on, items def get_pricing_rule_items(pr_doc): apply_on = frappe.scrub(pr_doc.get('apply_on')) pricing_rule_apply_on = apply_on_table.get(pr_doc.get('apply_on')) - return [item.get(apply_on) for item in pr_doc.get(pricing_rule_apply_on)] or [] \ No newline at end of file + return [item.get(apply_on) for item in pr_doc.get(pricing_rule_apply_on)] or [] + +@frappe.whitelist() +def validate_pricing_rule_for_different_cond(doc): + if isinstance(doc, string_types): + doc = json.loads(doc) + + doc = frappe.get_doc(doc) + for d in doc.get("items"): + validate_pricing_rule_on_items(doc, d, True) + + return doc \ No newline at end of file diff --git a/erpnext/buying/doctype/purchase_order/purchase_order.js b/erpnext/buying/doctype/purchase_order/purchase_order.js index b2c1de57f2..cf4ec49a78 100644 --- a/erpnext/buying/doctype/purchase_order/purchase_order.js +++ b/erpnext/buying/doctype/purchase_order/purchase_order.js @@ -25,9 +25,6 @@ frappe.ui.form.on("Purchase Order", { frm.set_indicator_formatter('item_code', function(doc) { return (doc.qty<=doc.received_qty) ? "green" : "orange" }) - frm.set_indicator_formatter('pricing_rule', - function(doc) { return (doc.rule_applied) ? "green" : "red" }) - frm.set_query("blanket_order", "items", function() { return { filters: { diff --git a/erpnext/controllers/accounts_controller.py b/erpnext/controllers/accounts_controller.py index 602f9b80ba..27c31eb961 100644 --- a/erpnext/controllers/accounts_controller.py +++ b/erpnext/controllers/accounts_controller.py @@ -282,7 +282,7 @@ class AccountsController(TransactionBase): if self.doctype in ["Purchase Invoice", "Sales Invoice"] and item.meta.get_field('is_fixed_asset'): item.set('is_fixed_asset', ret.get('is_fixed_asset', 0)) - if ret.get("pricing_rules"): + if ret.get("pricing_rules") and not ret.get("validate_applied_rule", 0): # if user changed the discount percentage then set user's discount percentage ? item.set("pricing_rules", ret.get("pricing_rules")) item.set("discount_percentage", ret.get("discount_percentage")) diff --git a/erpnext/public/js/controllers/transaction.js b/erpnext/public/js/controllers/transaction.js index f64791325e..62b8155b14 100644 --- a/erpnext/public/js/controllers/transaction.js +++ b/erpnext/public/js/controllers/transaction.js @@ -158,6 +158,11 @@ erpnext.TransactionController = erpnext.taxes_and_totals.extend({ }; }); } + + if(frappe.meta.get_docfield(this.frm.doc.doctype, "pricing_rules")) { + this.frm.set_indicator_formatter('pricing_rule', + function(doc) { return (doc.rule_applied) ? "green" : "red" }); + } }, onload: function() { var me = this; @@ -422,6 +427,7 @@ erpnext.TransactionController = erpnext.taxes_and_totals.extend({ method: "erpnext.stock.get_item_details.get_item_details", child: item, args: { + doc: me.frm.doc, args: { item_code: item.item_code, barcode: item.barcode, @@ -456,7 +462,7 @@ erpnext.TransactionController = erpnext.taxes_and_totals.extend({ cost_center: item.cost_center, tax_category: me.frm.doc.tax_category, item_tax_template: item.item_tax_template, - child_docname: item.name + child_docname: item.name, } }, @@ -482,7 +488,7 @@ erpnext.TransactionController = erpnext.taxes_and_totals.extend({ } }, () => me.conversion_factor(doc, cdt, cdn, true), - () => me.update_free_items(item) + () => me.validate_pricing_rule(item) ]); } } @@ -1116,7 +1122,7 @@ erpnext.TransactionController = erpnext.taxes_and_totals.extend({ callback: function(r) { if (!r.exc && r.message) { r.message.forEach(row_item => { - me.update_free_items(row_item); + me.validate_pricing_rule(row_item); }); me._set_values_for_item_list(r.message); me.calculate_taxes_and_totals(); @@ -1139,14 +1145,12 @@ erpnext.TransactionController = erpnext.taxes_and_totals.extend({ return this.frm.call({ method: "erpnext.accounts.doctype.pricing_rule.pricing_rule.apply_pricing_rule", - args: { args: args }, + args: { args: args, doc: me.frm.doc }, callback: function(r) { if (!r.exc && r.message) { me._set_values_for_item_list(r.message); if(item) me.set_gross_profit(item); - if(calculate_taxes_and_totals) me.calculate_taxes_and_totals(); if(me.frm.doc.apply_discount_on) me.frm.trigger("apply_discount_on") - me.update_free_items(item); } } }); @@ -1200,7 +1204,6 @@ erpnext.TransactionController = erpnext.taxes_and_totals.extend({ "warehouse": d.warehouse, "serial_no": d.serial_no, "price_list_rate": d.price_list_rate, - "discount_percentage": d.discount_percentage || 0.0, "conversion_factor": d.conversion_factor || 1.0 }); @@ -1241,6 +1244,8 @@ erpnext.TransactionController = erpnext.taxes_and_totals.extend({ // if pricing rule set as blank from an existing value, apply price_list if(!me.frm.doc.ignore_pricing_rule && existing_pricing_rule && !d.pricing_rules) { me.apply_price_list(frappe.get_doc(d.doctype, d.name)); + } else { + me.validate_pricing_rule(frappe.get_doc(d.doctype, d.name)); } } @@ -1288,41 +1293,29 @@ erpnext.TransactionController = erpnext.taxes_and_totals.extend({ }); }, - update_free_items: function(item) { - var me = this; + validate_pricing_rule: function(item) { + let me = this; + const fields = ["discount_percentage", "discount_amount", "pricing_rules"]; if (item.pricing_rules) { frappe.call({ - method: "erpnext.accounts.doctype.pricing_rule.pricing_rule.get_free_items", + method: "erpnext.accounts.doctype.pricing_rule.utils.validate_pricing_rule_for_different_cond", args: { - pricing_rules: item.pricing_rules, - item_row: item + doc: me.frm.doc }, callback: function(r) { - let items = []; - let child = ''; - - me.frm.doc.items.map(d => { - items[d.item_code] = d; - }); - - if(r.message && r.message.length) { - r.message.forEach(d => { - // If free item is already exists - - if(d.item_code in items && - d.is_free_item && items[d.item_code].is_free_item) { - child = items[d.item_code]; - } else { - child = frappe.model.add_child(me.frm.doc, item.doctype, "items"); - } - - $.each(d, function(k, v) { - child[k] = v; + if (r.message) { + r.message.items.forEach(d => { + me.frm.doc.items.forEach(row => { + if(d.name == row.name) { + fields.forEach(f => { + row[f] = d[f]; + }); + } }); - - me.frm.script_manager.trigger("price_list_rate", child.doctype, child.name); }); + + me.trigger_price_list_rate(); } } }); @@ -1337,9 +1330,29 @@ erpnext.TransactionController = erpnext.taxes_and_totals.extend({ me.frm.doc.items = items; refresh_field('items'); + } else if(item.applied_on_items && item.apply_on) { + const applied_on_items = item.applied_on_items.split(','); + me.frm.doc.items.forEach(row => { + if(applied_on_items.includes(row[item.apply_on])) { + fields.forEach(f => { + row[f] = 0; + }); + } + }); + + me.trigger_price_list_rate(); } }, + trigger_price_list_rate: function() { + var me = this; + + this.frm.doc.items.forEach(child_row => { + me.frm.script_manager.trigger("price_list_rate", + child_row.doctype, child_row.name); + }) + }, + validate_company_and_party: function() { var me = this; var valid = true; diff --git a/erpnext/selling/page/point_of_sale/point_of_sale.js b/erpnext/selling/page/point_of_sale/point_of_sale.js index f5e21d14cd..b098fdefa4 100644 --- a/erpnext/selling/page/point_of_sale/point_of_sale.js +++ b/erpnext/selling/page/point_of_sale/point_of_sale.js @@ -233,9 +233,13 @@ erpnext.pos.PointOfSale = class PointOfSale { } else { this.update_item_in_frm(item, field, value) .then(() => { - // update cart - this.update_cart_data(item); - this.set_form_action(); + this.frm.doc.items.forEach(item_row => { + // update cart + frappe.run_serially([ + () => this.update_cart_data(item_row), + () => this.set_form_action() + ]); + }); }); } return; @@ -256,7 +260,9 @@ erpnext.pos.PointOfSale = class PointOfSale { .then(() => { this.frm.script_manager.trigger('qty', item.doctype, item.name) .then(() => { - this.update_cart_data(item); + this.frm.doc.items.forEach(item => { + this.update_cart_data(item); + }); }); }); }, From 70b996af73e0301cb59ee250d28c365ffa4958b7 Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Sun, 31 Mar 2019 20:33:15 +0530 Subject: [PATCH 2/2] fix: test cases --- erpnext/hr/doctype/salary_slip/salary_slip.py | 5 ++++- erpnext/public/js/controllers/transaction.js | 5 +++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/erpnext/hr/doctype/salary_slip/salary_slip.py b/erpnext/hr/doctype/salary_slip/salary_slip.py index 684f3483f0..eb7cb113fc 100644 --- a/erpnext/hr/doctype/salary_slip/salary_slip.py +++ b/erpnext/hr/doctype/salary_slip/salary_slip.py @@ -452,7 +452,10 @@ class SalarySlip(TransactionBase): self.set_loan_repayment() - self.net_pay = (flt(self.gross_pay) - (flt(self.total_deduction) + flt(self.total_loan_repayment))) * flt(self.payment_days / self.total_working_days) + self.net_pay = 0 + if self.total_working_days: + self.net_pay = (flt(self.gross_pay) - (flt(self.total_deduction) + flt(self.total_loan_repayment))) * flt(self.payment_days / self.total_working_days) + self.rounded_total = rounded(self.net_pay, self.precision("net_pay") if disable_rounded_total else 0) diff --git a/erpnext/public/js/controllers/transaction.js b/erpnext/public/js/controllers/transaction.js index 62b8155b14..fa1b6df557 100644 --- a/erpnext/public/js/controllers/transaction.js +++ b/erpnext/public/js/controllers/transaction.js @@ -160,8 +160,9 @@ erpnext.TransactionController = erpnext.taxes_and_totals.extend({ } if(frappe.meta.get_docfield(this.frm.doc.doctype, "pricing_rules")) { - this.frm.set_indicator_formatter('pricing_rule', - function(doc) { return (doc.rule_applied) ? "green" : "red" }); + this.frm.set_indicator_formatter('pricing_rule', function(doc) { + return (doc.rule_applied) ? "green" : "red"; + }); } }, onload: function() {