Compare commits

..

5 Commits

Author SHA1 Message Date
Frappe PR Bot
d1d3b241ae chore(release): Bumped to Version 16.26.2
## [16.26.2](https://github.com/frappe/erpnext/compare/v16.26.1...v16.26.2) (2026-07-03)

### Bug Fixes

* **item-attribute:** clear attribute values when marking numeric ([1b1ea7f](1b1ea7f2aa))
* **item:** error on uncommitted input and escape values in variant dialog ([56d065b](56d065b919))
* **item:** rework multiple variant dialog for large numeric ranges ([6691217](66912173bd))
2026-07-03 10:29:44 +00:00
Mihir Kandoi
9cdaa738f1 Merge pull request #56848 from frappe/mergify/bp/version-16/pr-56742
fix(item): rework multiple variant dialog for large numeric ranges (backport #56741) (backport #56742)
2026-07-03 15:58:17 +05:30
Mihir Kandoi
56d065b919 fix(item): error on uncommitted input and escape values in variant dialog
Address review feedback:
- A typed-but-not-selected value passed validation yet was dropped by
  get_selected_attributes (reads committed pills only). Treat any pending
  input as an error so it is never silently omitted from creation.
- Escape pill / pending values before interpolating them into the HTML
  error message.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit d4da9a3d7d)
(cherry picked from commit 04c834d6a9)
2026-07-03 10:25:48 +00:00
Mihir Kandoi
66912173bd fix(item): rework multiple variant dialog for large numeric ranges
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)
2026-07-03 10:25:48 +00:00
Mihir Kandoi
1b1ea7f2aa fix(item-attribute): clear attribute values when marking numeric
Marking an attribute numeric hides the Item Attribute Values grid but leaves
its rows in the doc, whose mandatory Attribute Value / Abbreviation block the
save client-side before the server can clear them. Clear the table on the
client too so the save goes through.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
(cherry picked from commit 4afbd4d3d9)
(cherry picked from commit 374b340e73)
2026-07-03 10:25:47 +00:00
3 changed files with 187 additions and 99 deletions

View File

@@ -6,7 +6,7 @@ import frappe
from frappe.model.document import Document
from frappe.utils.user import is_website_user
__version__ = "16.26.1"
__version__ = "16.26.2"
def get_default_company(user=None):

View File

@@ -777,62 +777,141 @@ $.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 anything is wrong: an invalid committed pill, or
// text typed but not added as a pill (which get_selected_attributes would
// otherwise drop silently). The user must fix each before creation proceeds.
function validate_selected_attributes() {
let errors = [];
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 attribute = frappe.utils.escape_html(row.attribute);
let spec = attr_val_fields[row.attribute];
let invalid = [
...new Set((field.get_value() || []).filter((v) => !is_valid_attribute_value(spec, v))),
];
if (invalid.length) {
let values = invalid.map(frappe.utils.escape_html).join(", ");
errors.push(__("{0}: remove invalid value(s) {1}", [attribute, values]));
}
let pending = (field.$input?.val() || "").trim();
if (pending) {
let value = frappe.utils.escape_html(pending);
errors.push(
__("{0}: select the typed value {1} from the list or clear it", [attribute, value])
);
}
});
if (errors.length) {
frappe.throw({
title: __("Invalid Attribute Values"),
message: errors.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 +937,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 +966,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);

View File

@@ -1,4 +1,13 @@
// Copyright (c) 2019, Frappe Technologies Pvt. Ltd. and contributors
// For license information, please see license.txt
frappe.ui.form.on("Item Attribute", {});
frappe.ui.form.on("Item Attribute", {
numeric_values(frm) {
// Numeric attributes have no discrete values; drop the rows so their
// mandatory Attribute Value / Abbreviation don't block the save.
if (frm.doc.numeric_values) {
frm.clear_table("item_attribute_values");
frm.refresh_field("item_attribute_values");
}
},
});