Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: move item_code reactivity to server-side #44293

Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

ruthra-kumar
Copy link
Member

@ruthra-kumar ruthra-kumar commented Nov 22, 2024

Context

Selecting an Item is the most common action in Sales / Purchase cycle. Whenever an Item is selected, system fetches the basic details like rate, discounts, stock qty, warehouse and sales account as such and auto-fills these details in it's respective fields. These can simply be called as Reactivity.

All of this is achieved by a JS trigger on item_code located in transaction.js

item_code(doc, cdt, cdn) {
var me = this;
var item = frappe.get_doc(cdt, cdn);
var update_stock = 0, show_batch_dialog = 0;
item.weight_per_unit = 0;
item.weight_uom = '';
item.conversion_factor = 0;
if(['Sales Invoice', 'Purchase Invoice'].includes(this.frm.doc.doctype)) {
update_stock = cint(me.frm.doc.update_stock);
show_batch_dialog = update_stock;
} else if((this.frm.doc.doctype === 'Purchase Receipt') ||
this.frm.doc.doctype === 'Delivery Note') {
show_batch_dialog = 1;
}
if (show_batch_dialog && item.use_serial_batch_fields === 1) {
show_batch_dialog = 0;
}
item.barcode = null;
if(item.item_code || item.serial_no) {
if(!this.validate_company_and_party()) {
this.frm.fields_dict["items"].grid.grid_rows[item.idx - 1].remove();
} else {
item.pricing_rules = ''
return this.frm.call({
method: "erpnext.stock.get_item_details.get_item_details",
child: item,
args: {
doc: me.frm.doc,
ctx: {
item_code: item.item_code,
barcode: item.barcode,
serial_no: item.serial_no,
batch_no: item.batch_no,
set_warehouse: me.frm.doc.set_warehouse,
warehouse: item.warehouse,
customer: me.frm.doc.customer || me.frm.doc.party_name,
quotation_to: me.frm.doc.quotation_to,
supplier: me.frm.doc.supplier,
currency: me.frm.doc.currency,
is_internal_supplier: me.frm.doc.is_internal_supplier,
is_internal_customer: me.frm.doc.is_internal_customer,
update_stock: update_stock,
conversion_rate: me.frm.doc.conversion_rate,
price_list: me.frm.doc.selling_price_list || me.frm.doc.buying_price_list,
price_list_currency: me.frm.doc.price_list_currency,
plc_conversion_rate: me.frm.doc.plc_conversion_rate,
company: me.frm.doc.company,
order_type: me.frm.doc.order_type,
is_pos: cint(me.frm.doc.is_pos),
is_return: cint(me.frm.doc.is_return),
is_subcontracted: me.frm.doc.is_subcontracted,
ignore_pricing_rule: me.frm.doc.ignore_pricing_rule,
doctype: me.frm.doc.doctype,
name: me.frm.doc.name,
project: item.project || me.frm.doc.project,
qty: item.qty || 1,
net_rate: item.rate,
base_net_rate: item.base_net_rate,
stock_qty: item.stock_qty,
conversion_factor: item.conversion_factor,
weight_per_unit: item.weight_per_unit,
uom: item.uom,
weight_uom: item.weight_uom,
manufacturer: item.manufacturer,
stock_uom: item.stock_uom,
pos_profile: cint(me.frm.doc.is_pos) ? me.frm.doc.pos_profile : '',
cost_center: item.cost_center,
tax_category: me.frm.doc.tax_category,
item_tax_template: item.item_tax_template,
child_doctype: item.doctype,
child_docname: item.name,
is_old_subcontracting_flow: me.frm.doc.is_old_subcontracting_flow,
}
},
callback: function(r) {
if(!r.exc) {
frappe.run_serially([
() => {
if (item.docstatus === 0
&& frappe.meta.has_field(item.doctype, "use_serial_batch_fields")
&& !item.use_serial_batch_fields
&& cint(frappe.user_defaults?.use_serial_batch_fields) === 1
) {
item["use_serial_batch_fields"] = 1;
}
},
() => {
var d = locals[cdt][cdn];
me.add_taxes_from_item_tax_template(d.item_tax_rate);
if (d.free_item_data && d.free_item_data.length > 0) {
me.apply_product_discount(d);
}
},
() => {
// for internal customer instead of pricing rule directly apply valuation rate on item
if ((me.frm.doc.is_internal_customer || me.frm.doc.is_internal_supplier) && me.frm.doc.represents_company === me.frm.doc.company) {
me.get_incoming_rate(item, me.frm.posting_date, me.frm.posting_time,
me.frm.doc.doctype, me.frm.doc.company);
} else {
me.frm.script_manager.trigger("price_list_rate", cdt, cdn);
}
},
() => {
if (me.frm.doc.is_internal_customer || me.frm.doc.is_internal_supplier) {
me.calculate_taxes_and_totals();
}
},
() => me.toggle_conversion_factor(item),
() => {
if (show_batch_dialog && !frappe.flags.trigger_from_barcode_scanner)
return frappe.db.get_value("Item", item.item_code, ["has_batch_no", "has_serial_no"])
.then((r) => {
if (r.message &&
(r.message.has_batch_no || r.message.has_serial_no)) {
frappe.flags.hide_serial_batch_dialog = false;
} else {
show_batch_dialog = false;
}
});
},
() => {
// check if batch serial selector is disabled or not
if (show_batch_dialog && !frappe.flags.hide_serial_batch_dialog)
return frappe.db.get_single_value('Stock Settings', 'disable_serial_no_and_batch_selector')
.then((value) => {
if (value) {
frappe.flags.hide_serial_batch_dialog = true;
}
});
},
() => {
if(show_batch_dialog && !frappe.flags.hide_serial_batch_dialog && !frappe.flags.dialog_set) {
var d = locals[cdt][cdn];
$.each(r.message, function(k, v) {
if(!d[k]) d[k] = v;
});
if (d.has_batch_no && d.has_serial_no) {
d.batch_no = undefined;
}
frappe.flags.dialog_set = true;
erpnext.show_serial_batch_selector(me.frm, d, (item) => {
me.frm.script_manager.trigger('qty', item.doctype, item.name);
if (!me.frm.doc.set_warehouse)
me.frm.script_manager.trigger('warehouse', item.doctype, item.name);
me.apply_price_list(item, true);
}, undefined, !frappe.flags.hide_serial_batch_dialog);
} else {
frappe.flags.dialog_set = false;
}
},
() => me.conversion_factor(doc, cdt, cdn, true),
() => me.remove_pricing_rule(item),
() => {
if (item.apply_rule_on_other_items) {
let key = item.name;
me.apply_rule_on_other_items({key: item});
}
},
() => {
var company_currency = me.get_company_currency();
me.update_item_grid_labels(company_currency);
}
]);
}
}
});
}
}
}

But, due to a lack of UI test cases, this Reactivity is prone to regression, especially the ones related to discounts and free items. Check #42195 for more broken cases on Pricing Rules.

Long-Term Stability and Maintenance

To prevent frequent regression, moving most of the Reactivity to the server-side code makes sense, as it allows for server-side tests and a single source to maintain.

This PR is the first in that direction. It moves all of the UI Reactivity associated with Item selection to service-side.

@ruthra-kumar ruthra-kumar self-assigned this Nov 22, 2024
@github-actions github-actions bot added the needs-tests This PR needs automated unit-tests. label Nov 22, 2024
Handle internal parties
conversion factor and applying pricing list rate
@ruthra-kumar ruthra-kumar changed the title refactor: moving trigger to server refactor: move item_code reactivity to server-side Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Changes dont-merge needs-tests This PR needs automated unit-tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant