nsinnovations/erpnext
2
0
Fork 0
mirror of https://github.com/frappe/erpnext.git synced 2026-07-06 23:17:57 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(item): rework multiple variant dialog for large numeric ranges

Browse Source 
The 'Create Multiple Variants' dialog rendered one checkbox per attribute
value and read the numeric config from the variant attribute child row. This
broke in several ways:

- A template whose attribute was made numeric after being added kept
  numeric_values=0 on the child row, so the dialog treated it as non-numeric,
  queried the empty Item Attribute Value table, and showed no values.
- Enumerating a large range (e.g. 1-100000) into checkboxes froze the browser.

Rework the dialog:

- Read numeric_values / from_range / to_range / increment from the Item
  Attribute master, and guard increment > 0.
- Replace the checkbox-per-value list with one MultiSelectPills per attribute,
  with a search placeholder.
- Stop enumerating numeric ranges: preview the first few values and validate
  typed input against the range on demand, so huge ranges stay instant.
- Block variant creation with a modal error if any selected value or pending
  input is invalid (out of range, off-increment, or not a number), so garbage
  like '00A' can't reach creation.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit 99152b8300)
(cherry picked from commit 025d0cd7f3)
This commit is contained in:
Mihir Kandoi
2026-07-01 23:28:03 +05:30
committed by Mergify
parent 1b1ea7f2aa
commit 66912173bd
1 changed files with 170 additions and 97 deletions
Download Patch File Download Diff File Expand all files Collapse all files

267
erpnext/stock/doctype/item/item.js
View File

@@ -777,62 +777,135 @@ $.extend(erpnext.item, {
function make_fields_from_attribute_values(attr_dict) {
let fields = [];
let att_key = frm.doc.attributes.map((idx) => idx.attribute);
att_key.forEach((name, i) => {
let attributes = frm.doc.attributes.filter((row) => !row.disabled);
attributes.forEach((row, i) => {
let name = row.attribute;
if (i % 3 === 0) {
fields.push({ fieldtype: "Section Break" });
}
fields.push({ fieldtype: "Column Break", label: name });
fields.push({ fieldtype: "Column Break" });
fields.push({
fieldtype: "Data",
placeholder: "Search",
fieldname: `search_${frappe.scrub(name)}`,
onchange: function (e) {
let value = e.target.value;
let result = attr_dict[name].filter((attr_value) =>
attr_value.toString().toLowerCase().includes(value.toLowerCase())
);
attr_dict[name].forEach((attr_value) => {
if (result.includes(attr_value)) {
me.multiple_variant_dialog.set_df_property(attr_value, "hidden", 0);
} else {
me.multiple_variant_dialog.set_df_property(attr_value, "hidden", 1);
}
});
},
});
attr_dict[name].forEach((value) => {
fields.push({
fieldtype: "Check",
label: value,
fieldname: value,
default: 0,
onchange: function () {
let selected_attributes = get_selected_attributes();
let lengths = Object.keys(selected_attributes).map((key) => {
return selected_attributes[key].length;
});
if (!lengths.length) {
me.multiple_variant_dialog.get_primary_btn().html(__("Create Variants"));
me.multiple_variant_dialog.disable_primary_action();
} else {
let no_of_combinations = lengths.reduce((a, b) => a * b, 1);
let msg;
if (no_of_combinations === 1) {
msg = __("Make {0} Variant", [no_of_combinations]);
} else {
msg = __("Make {0} Variants", [no_of_combinations]);
}
me.multiple_variant_dialog.get_primary_btn().html(msg);
me.multiple_variant_dialog.enable_primary_action();
}
},
});
fieldtype: "MultiSelectPills",
label: name,
fieldname: frappe.scrub(name),
placeholder: __("Search values..."),
get_data: (txt) => get_attribute_suggestions(attr_dict[name], txt),
onchange: update_primary_action,
});
});
return fields;
}
function get_attribute_suggestions(spec, txt) {
if (!spec) return [];
return Array.isArray(spec) ? filter_list(spec, txt) : numeric_suggestions(spec, txt);
}
// Cap matches so a long value list never hands everything to Awesomplete,
// which would freeze the browser.
function filter_list(values, txt) {
txt = (txt || "").toLowerCase();
let matches = [];
for (let value of values) {
if (!txt || value.toLowerCase().includes(txt)) {
matches.push(value);
if (matches.length >= 50) break;
}
}
return matches;
}
// Numeric ranges aren't enumerated. With no input, preview the first few
// values; once the user types, accept it only if it lies on the increment
// within [from, to]. Both paths are cheap even for huge ranges.
function numeric_suggestions(range, txt) {
let { from_range: from, to_range: to, increment } = range;
if (!(increment > 0) || from > to) return [];
txt = (txt || "").trim();
if (!txt) {
let preview = [];
for (
let value = from;
value <= to && preview.length < 50;
value = flt(value + increment, 6)
) {
preview.push(String(value));
}
return preview;
}
return is_valid_attribute_value(range, txt) ? [String(flt(txt, 6))] : [];
}
function is_valid_attribute_value(spec, value) {
if (!spec || !value) return false;
if (Array.isArray(spec)) return spec.includes(value);
let { from_range: from, to_range: to, increment } = spec;
if (!(increment > 0)) return false;
// Reject anything that isn't cleanly a number ("abc", "5000xyz", "");
// flt would coerce these to 0 and wrongly accept them.
let text = String(value).trim();
let num = Number(text);
if (text === "" || !Number.isFinite(num)) return false;
if (num < from || num > to) return false;
let steps = (num - from) / increment;
return Math.abs(Math.round(steps) - steps) <= 1e-6;
}
// Block variant creation if ANY value is invalid. Checks both the committed
// pills and any text still sitting in the input box (typed but not selected),
// so garbage like "00A" can never slip through to creation.
function validate_selected_attributes() {
let invalid = [];
frm.doc.attributes.forEach((row) => {
if (row.disabled) return;
let field = me.multiple_variant_dialog.get_field(frappe.scrub(row.attribute));
if (!field) return;
let spec = attr_val_fields[row.attribute];
let values = (field.get_value() || []).slice();
let pending = (field.$input?.val() || "").trim();
if (pending) values.push(pending);
let bad = [...new Set(values.filter((v) => !is_valid_attribute_value(spec, v)))];
if (bad.length) {
invalid.push(`<b>${frappe.utils.escape_html(row.attribute)}</b>: ${bad.join(", ")}`);
}
});
if (invalid.length) {
frappe.throw({
title: __("Invalid Attribute Values"),
message:
__("Please remove the following invalid values before creating variants:") +
"<br><br>" +
invalid.join("<br>"),
indicator: "red",
});
}
}
function update_primary_action() {
let selected_attributes = get_selected_attributes();
let counts = Object.keys(selected_attributes).map((key) => selected_attributes[key].length);
if (!counts.length) {
me.multiple_variant_dialog.get_primary_btn().html(__("Create Variants"));
me.multiple_variant_dialog.disable_primary_action();
} else {
let no_of_combinations = counts.reduce((a, b) => a * b, 1);
let msg =
no_of_combinations === 1
? __("Make {0} Variant", [no_of_combinations])
: __("Make {0} Variants", [no_of_combinations]);
me.multiple_variant_dialog.get_primary_btn().html(msg);
me.multiple_variant_dialog.enable_primary_action();
}
}
function make_and_show_dialog(fields) {
me.multiple_variant_dialog = new frappe.ui.Dialog({
title: __("Select Attribute Values"),
@@ -858,6 +931,8 @@ $.extend(erpnext.item, {
});
me.multiple_variant_dialog.set_primary_action(__("Create Variants"), () => {
validate_selected_attributes();
let selected_attributes = get_selected_attributes();
let use_template_image = me.multiple_variant_dialog.get_value("use_template_image");
@@ -885,72 +960,70 @@ $.extend(erpnext.item, {
});
});
$($(me.multiple_variant_dialog.$wrapper.find(".form-column")).find(".frappe-control")).css(
"margin-bottom",
"0px"
);
me.multiple_variant_dialog.disable_primary_action();
me.multiple_variant_dialog.clear();
me.multiple_variant_dialog.show();
me.multiple_variant_dialog.$wrapper
.find("div[data-fieldname^='search_']")
.find(".clearfix")
.hide();
}
function get_selected_attributes() {
let selected_attributes = {};
me.multiple_variant_dialog.$wrapper.find(".form-column").each((i, col) => {
if (i === 0) return;
let attribute_name = $(col).find(".column-label").html().trim();
selected_attributes[attribute_name] = [];
let checked_opts = $(col).find(".checkbox input");
checked_opts.each((i, opt) => {
if ($(opt).is(":checked")) {
selected_attributes[attribute_name].push($(opt).attr("data-fieldname"));
}
});
if (!selected_attributes[attribute_name].length) {
delete selected_attributes[attribute_name];
frm.doc.attributes.forEach((row) => {
if (row.disabled) return;
let values = me.multiple_variant_dialog.get_value(frappe.scrub(row.attribute));
if (values && values.length) {
selected_attributes[row.attribute] = values;
}
});
return selected_attributes;
}
frm.doc.attributes.forEach(function (d) {
if (!d.disabled) {
let p = new Promise((resolve) => {
if (!d.numeric_values) {
frappe
.call({
method: "frappe.client.get_list",
args: {
doctype: "Item Attribute Value",
filters: [["parent", "=", d.attribute]],
fields: ["attribute_value"],
limit_page_length: 0,
parent: "Item Attribute",
order_by: "idx",
},
})
.then((r) => {
if (r.message) {
attr_val_fields[d.attribute] = r.message.map(function (d) {
return d.attribute_value;
// Read the numeric configuration from the Item Attribute master
// instead of the variant attribute row, which may be stale or
// blank if the attribute was made numeric after it was added here.
frappe.db
.get_value("Item Attribute", d.attribute, [
"numeric_values",
"from_range",
"to_range",
"increment",
])
.then((res) => {
let attr = res.message || {};
if (!attr.numeric_values) {
frappe
.call({
method: "frappe.client.get_list",
args: {
doctype: "Item Attribute Value",
filters: [["parent", "=", d.attribute]],
fields: ["attribute_value"],
limit_page_length: 0,
parent: "Item Attribute",
order_by: "idx",
},
})
.then((r) => {
attr_val_fields[d.attribute] = (r.message || []).map(
(row) => row.attribute_value
);
resolve();
});
resolve();
}
});
} else {
let values = [];
for (var i = d.from_range; i <= d.to_range; i = flt(i + d.increment, 6)) {
values.push(i);
}
attr_val_fields[d.attribute] = values;
resolve();
}
} else {
// Store the range instead of enumerating it; a large range
// (e.g. 1-100000) is slow to build and to search. Values are
// validated against the range on demand while typing.
attr_val_fields[d.attribute] = {
from_range: flt(attr.from_range),
to_range: flt(attr.to_range),
increment: flt(attr.increment),
};
resolve();
}
});
});
promises.push(p);