mirror of
https://github.com/frappe/erpnext.git
synced 2026-08-11 13:41:47 +00:00
Merge pull request #57097 from aerele/fix/qi-reading-number-format
fix(stock): read quality inspection readings in the user's number format
This commit is contained in:
@@ -2,13 +2,15 @@
|
||||
# License: GNU General Public License v3. See license.txt
|
||||
|
||||
|
||||
from math import isfinite
|
||||
from typing import Any
|
||||
|
||||
import frappe
|
||||
from frappe import _
|
||||
from frappe.model.document import Document
|
||||
from frappe.model.mapper import get_mapped_doc
|
||||
from frappe.utils import cint, flt, get_link_to_form, get_number_format_info
|
||||
from frappe.utils import cint, flt, get_link_to_form
|
||||
from frappe.utils.number_format import NUMBER_FORMAT_MAP, NumberFormat
|
||||
|
||||
from erpnext.stock.doctype.quality_inspection_template.quality_inspection_template import (
|
||||
get_template_details,
|
||||
@@ -84,6 +86,7 @@ class QualityInspection(Document):
|
||||
reading.status = "Accepted"
|
||||
|
||||
if self.readings:
|
||||
self.validate_reading_number_format()
|
||||
self.inspect_and_set_status()
|
||||
|
||||
self.validate_inspection_required()
|
||||
@@ -281,6 +284,47 @@ class QualityInspection(Document):
|
||||
)
|
||||
break
|
||||
|
||||
def validate_reading_number_format(self):
|
||||
"""Reject newly entered readings that are not numbers in the user's format.
|
||||
|
||||
They would otherwise be misread rather than refused, silently rejecting an
|
||||
inspection whose readings are in fact within the acceptance range. Readings
|
||||
already stored are left alone, so a document entered by a user in one locale
|
||||
stays saveable and submittable by a user in another."""
|
||||
number_format = get_reading_number_format()
|
||||
decimal_str, comma_str = get_reading_separators(number_format)
|
||||
before_save = self.get_doc_before_save()
|
||||
|
||||
for reading in self.readings:
|
||||
if not cint(reading.numeric) or cint(reading.manual_inspection):
|
||||
continue
|
||||
|
||||
stored = before_save and before_save.get("readings", {"name": reading.name})
|
||||
stored = stored[0] if stored else None
|
||||
|
||||
for i in range(1, 11):
|
||||
field = "reading_" + str(i)
|
||||
value = reading.get(field)
|
||||
if value is None or not value.strip():
|
||||
continue
|
||||
|
||||
if stored and stored.get(field) == value:
|
||||
continue
|
||||
|
||||
if parse_reading(value, decimal_str, comma_str) is None:
|
||||
frappe.throw(
|
||||
_(
|
||||
"Row #{0}: Reading {1} {2} is not a valid number in the {3} number format. Use {4} as the decimal separator."
|
||||
).format(
|
||||
reading.idx,
|
||||
i,
|
||||
frappe.bold(value),
|
||||
frappe.bold(number_format.string),
|
||||
frappe.bold(decimal_str),
|
||||
),
|
||||
title=_("Invalid Reading"),
|
||||
)
|
||||
|
||||
def set_status_based_on_acceptance_values(self, reading):
|
||||
if not cint(reading.numeric):
|
||||
reading_value = reading.get("reading_value") or ""
|
||||
@@ -511,17 +555,61 @@ def make_quality_inspection(source_name: str, target_doc: str | dict | Document
|
||||
return doc
|
||||
|
||||
|
||||
def get_reading_number_format() -> NumberFormat:
|
||||
"""Number format the user enters readings in.
|
||||
|
||||
User defaults fall back to the global default, so this is the same format the
|
||||
user's desk formats numbers with."""
|
||||
number_format = frappe.defaults.get_user_default("number_format")
|
||||
if number_format not in NUMBER_FORMAT_MAP:
|
||||
number_format = "#,###.##"
|
||||
|
||||
return NumberFormat.from_string(number_format)
|
||||
|
||||
|
||||
def get_reading_separators(number_format: NumberFormat) -> tuple[str, str]:
|
||||
"""Decimal and thousands separator a reading may be written with.
|
||||
|
||||
A format with no decimal separator still has to accept decimal readings, so it
|
||||
falls back to a dot and gives up any grouping that would collide with it."""
|
||||
decimal_str = number_format.decimal_separator or "."
|
||||
comma_str = number_format.thousands_separator
|
||||
|
||||
return decimal_str, "" if comma_str == decimal_str else comma_str
|
||||
|
||||
|
||||
def parse_reading(value: str, decimal_str: str, comma_str: str) -> float | None:
|
||||
"""Reading as a float, or None when it is not a number in that format."""
|
||||
value = value.strip()
|
||||
integer_part = value.partition(decimal_str)[0]
|
||||
|
||||
if comma_str and comma_str in integer_part:
|
||||
groups = integer_part.split(comma_str)
|
||||
lead = groups[0][1:] if groups[0][:1] in ("+", "-") else groups[0]
|
||||
if not 1 <= len(lead) <= 3 or len(groups[-1]) != 3:
|
||||
return None
|
||||
|
||||
if any(len(group) not in (2, 3) for group in groups[1:-1]):
|
||||
return None
|
||||
|
||||
value = value.replace(comma_str, "")
|
||||
|
||||
if decimal_str != ".":
|
||||
value = value.replace(decimal_str, ".")
|
||||
|
||||
try:
|
||||
number = float(value)
|
||||
except ValueError:
|
||||
return None
|
||||
|
||||
return number if isfinite(number) else None
|
||||
|
||||
|
||||
def parse_float(num: str) -> float:
|
||||
"""Since reading_# fields are `Data` field they might contain number which
|
||||
is representation in user's prefered number format instead of machine
|
||||
readable format. This function converts them to machine readable format."""
|
||||
|
||||
number_format = frappe.db.get_default("number_format") or "#,###.##"
|
||||
decimal_str, comma_str, _number_format_precision = get_number_format_info(number_format)
|
||||
decimal_str, comma_str = get_reading_separators(get_reading_number_format())
|
||||
|
||||
if decimal_str == "," and comma_str == ".":
|
||||
num = num.replace(",", "#$")
|
||||
num = num.replace(".", ",")
|
||||
num = num.replace("#$", ".")
|
||||
|
||||
return flt(num)
|
||||
return flt(parse_reading(num, decimal_str, comma_str))
|
||||
|
||||
@@ -1,8 +1,11 @@
|
||||
# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors and Contributors
|
||||
# See license.txt
|
||||
|
||||
from contextlib import contextmanager
|
||||
|
||||
import frappe
|
||||
from frappe.utils import nowdate
|
||||
from frappe.utils.number_format import NumberFormat
|
||||
|
||||
from erpnext.controllers.stock_controller import (
|
||||
QualityInspectionNotSubmittedError,
|
||||
@@ -12,10 +15,29 @@ from erpnext.controllers.stock_controller import (
|
||||
)
|
||||
from erpnext.stock.doctype.delivery_note.test_delivery_note import create_delivery_note
|
||||
from erpnext.stock.doctype.item.test_item import create_item
|
||||
from erpnext.stock.doctype.quality_inspection.quality_inspection import (
|
||||
get_reading_separators,
|
||||
parse_reading,
|
||||
)
|
||||
from erpnext.stock.doctype.stock_entry.stock_entry_utils import make_stock_entry
|
||||
from erpnext.tests.utils import ERPNextTestSuite
|
||||
|
||||
|
||||
@contextmanager
|
||||
def user_number_format(number_format):
|
||||
"""Temporarily set the session user's own number format."""
|
||||
user = frappe.session.user
|
||||
previous = frappe.db.get_value("DefaultValue", {"parent": user, "defkey": "number_format"}, "defvalue")
|
||||
frappe.defaults.set_user_default("number_format", number_format)
|
||||
try:
|
||||
yield
|
||||
finally:
|
||||
if previous:
|
||||
frappe.defaults.set_user_default("number_format", previous)
|
||||
else:
|
||||
frappe.defaults.clear_user_default("number_format")
|
||||
|
||||
|
||||
class TestQualityInspection(ERPNextTestSuite):
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
@@ -108,7 +130,6 @@ class TestQualityInspection(ERPNextTestSuite):
|
||||
"acceptance_formula": "mean < 0.9",
|
||||
"reading_1": "0.5",
|
||||
"reading_2": "0.7",
|
||||
"reading_3": "random text", # check if random string input causes issues
|
||||
},
|
||||
{
|
||||
"specification": "Calcium Content", # non-numeric reading
|
||||
@@ -252,6 +273,208 @@ class TestQualityInspection(ERPNextTestSuite):
|
||||
qa.delete()
|
||||
dn.delete()
|
||||
|
||||
def test_non_numeric_reading(self):
|
||||
dn = create_delivery_note(item_code="_Test Item with QA", do_not_submit=True)
|
||||
create_quality_inspection_parameter("Density")
|
||||
|
||||
readings = [
|
||||
{"specification": "Density", "min_value": 1.15, "max_value": 1.20, "reading_1": "random text"}
|
||||
]
|
||||
qa = create_quality_inspection(
|
||||
reference_type="Delivery Note", reference_name=dn.name, readings=readings, do_not_save=True
|
||||
)
|
||||
|
||||
self.assertRaises(frappe.ValidationError, qa.save)
|
||||
|
||||
dn.delete()
|
||||
|
||||
def test_non_numeric_reading_in_formula_based_criteria(self):
|
||||
dn = create_delivery_note(item_code="_Test Item with QA", do_not_submit=True)
|
||||
create_quality_inspection_parameter("Density")
|
||||
|
||||
readings = [
|
||||
{
|
||||
"specification": "Density",
|
||||
"formula_based_criteria": 1,
|
||||
"acceptance_formula": "mean < 0.9",
|
||||
"reading_1": "0.5",
|
||||
"reading_2": "0.7",
|
||||
"reading_3": "random text",
|
||||
}
|
||||
]
|
||||
qa = create_quality_inspection(
|
||||
reference_type="Delivery Note", reference_name=dn.name, readings=readings, do_not_save=True
|
||||
)
|
||||
|
||||
self.assertRaises(frappe.ValidationError, qa.save)
|
||||
|
||||
dn.delete()
|
||||
|
||||
def test_manual_inspection_reading_is_not_number_checked(self):
|
||||
dn = create_delivery_note(item_code="_Test Item with QA", do_not_submit=True)
|
||||
create_quality_inspection_parameter("Density")
|
||||
|
||||
readings = [
|
||||
{
|
||||
"specification": "Density",
|
||||
"manual_inspection": 1,
|
||||
"status": "Accepted",
|
||||
"min_value": 1.15,
|
||||
"max_value": 1.20,
|
||||
"reading_1": "1.15 g/cm3",
|
||||
}
|
||||
]
|
||||
qa = create_quality_inspection(
|
||||
reference_type="Delivery Note", reference_name=dn.name, readings=readings, do_not_save=True
|
||||
)
|
||||
qa.save()
|
||||
|
||||
self.assertEqual(qa.readings[0].status, "Accepted")
|
||||
|
||||
qa.delete()
|
||||
dn.delete()
|
||||
|
||||
def test_reading_in_comma_decimal_number_format(self):
|
||||
dn = create_delivery_note(item_code="_Test Item with QA", do_not_submit=True)
|
||||
create_quality_inspection_parameter("Density")
|
||||
|
||||
readings = [{"specification": "Density", "min_value": 1.15, "max_value": 1.20, "reading_1": "1,15"}]
|
||||
with user_number_format("#.###,##"):
|
||||
qa = create_quality_inspection(
|
||||
reference_type="Delivery Note", reference_name=dn.name, readings=readings, do_not_save=True
|
||||
)
|
||||
qa.save()
|
||||
|
||||
self.assertEqual(qa.readings[0].status, "Accepted")
|
||||
self.assertEqual(qa.status, "Accepted")
|
||||
|
||||
qa.delete()
|
||||
dn.delete()
|
||||
|
||||
def test_reading_in_space_grouped_number_format(self):
|
||||
dn = create_delivery_note(item_code="_Test Item with QA", do_not_submit=True)
|
||||
create_quality_inspection_parameter("Density")
|
||||
|
||||
readings = [{"specification": "Density", "min_value": 1.15, "max_value": 1.20, "reading_1": "1,15"}]
|
||||
with user_number_format("# ###,##"):
|
||||
qa = create_quality_inspection(
|
||||
reference_type="Delivery Note", reference_name=dn.name, readings=readings, do_not_save=True
|
||||
)
|
||||
qa.save()
|
||||
|
||||
self.assertEqual(qa.readings[0].status, "Accepted")
|
||||
self.assertEqual(qa.status, "Accepted")
|
||||
|
||||
qa.delete()
|
||||
dn.delete()
|
||||
|
||||
def test_reading_in_wrong_decimal_number_format(self):
|
||||
dn = create_delivery_note(item_code="_Test Item with QA", do_not_submit=True)
|
||||
create_quality_inspection_parameter("Density")
|
||||
|
||||
readings = [{"specification": "Density", "min_value": 1.15, "max_value": 1.20, "reading_1": "1.15"}]
|
||||
with user_number_format("#.###,##"):
|
||||
qa = create_quality_inspection(
|
||||
reference_type="Delivery Note", reference_name=dn.name, readings=readings, do_not_save=True
|
||||
)
|
||||
|
||||
self.assertRaises(frappe.ValidationError, qa.save)
|
||||
|
||||
dn.delete()
|
||||
|
||||
def test_reading_with_comma_in_dot_decimal_number_format(self):
|
||||
dn = create_delivery_note(item_code="_Test Item with QA", do_not_submit=True)
|
||||
create_quality_inspection_parameter("Density")
|
||||
|
||||
readings = [{"specification": "Density", "min_value": 1.15, "max_value": 1.20, "reading_1": "1,15"}]
|
||||
with user_number_format("#,###.##"):
|
||||
qa = create_quality_inspection(
|
||||
reference_type="Delivery Note", reference_name=dn.name, readings=readings, do_not_save=True
|
||||
)
|
||||
|
||||
self.assertRaises(frappe.ValidationError, qa.save)
|
||||
|
||||
dn.delete()
|
||||
|
||||
@ERPNextTestSuite.change_settings("System Settings", {"number_format": "#,###.##"})
|
||||
def test_reading_number_format_prefers_the_user_over_the_system(self):
|
||||
dn = create_delivery_note(item_code="_Test Item with QA", do_not_submit=True)
|
||||
create_quality_inspection_parameter("Density")
|
||||
|
||||
readings = [{"specification": "Density", "min_value": 1.15, "max_value": 1.20, "reading_1": "1,15"}]
|
||||
with user_number_format("#.###,##"):
|
||||
qa = create_quality_inspection(
|
||||
reference_type="Delivery Note", reference_name=dn.name, readings=readings, do_not_save=True
|
||||
)
|
||||
qa.save()
|
||||
|
||||
self.assertEqual(qa.readings[0].status, "Accepted")
|
||||
|
||||
qa.delete()
|
||||
dn.delete()
|
||||
|
||||
def test_stored_reading_stays_submittable_in_another_number_format(self):
|
||||
dn = create_delivery_note(item_code="_Test Item with QA", do_not_submit=True)
|
||||
create_quality_inspection_parameter("Density")
|
||||
|
||||
readings = [{"specification": "Density", "min_value": 1.15, "max_value": 1.20, "reading_1": "1,15"}]
|
||||
with user_number_format("#.###,##"):
|
||||
qa = create_quality_inspection(
|
||||
reference_type="Delivery Note", reference_name=dn.name, readings=readings, do_not_save=True
|
||||
)
|
||||
qa.save()
|
||||
|
||||
with user_number_format("#,###.##"):
|
||||
qa.reload()
|
||||
qa.submit()
|
||||
|
||||
self.assertEqual(qa.docstatus, 1)
|
||||
|
||||
qa.cancel()
|
||||
qa.delete()
|
||||
dn.delete()
|
||||
|
||||
def test_parse_reading_in_every_number_format(self):
|
||||
accepted = [
|
||||
("#,###.##", "1.15", 1.15),
|
||||
("#,###.##", "1,234.56", 1234.56),
|
||||
("#,##,###.##", "12,34,567.89", 1234567.89),
|
||||
("#,###.###", "1,234.567", 1234.567),
|
||||
("#.###,##", "1,15", 1.15),
|
||||
("#.###,##", "1.234,56", 1234.56),
|
||||
("# ###,##", "1,15", 1.15),
|
||||
("# ###,##", "1.15", 1.15),
|
||||
("# ###,##", "1 234,56", 1234.56),
|
||||
("# ###.##", "1 234.56", 1234.56),
|
||||
("#'###.##", "1'234.56", 1234.56),
|
||||
("#, ###.##", "1, 234.56", 1234.56),
|
||||
("#.########", "1.15", 1.15),
|
||||
("#,###", "1.5", 1.5),
|
||||
("#,###", "1,500", 1500.0),
|
||||
("#.###", "1.5", 1.5),
|
||||
("#.###", "1.500", 1.5),
|
||||
("#,###.##", "-1,234.56", -1234.56),
|
||||
]
|
||||
refused = [
|
||||
("#,###.##", "1,15"),
|
||||
("#.###,##", "1.15"),
|
||||
("#,###.##", "--1.15"),
|
||||
("#,###.##", "1²"),
|
||||
("#,###.##", "nan"),
|
||||
("#,###.##", "random text"),
|
||||
("#,###", "1,50"),
|
||||
]
|
||||
|
||||
for number_format, value, expected in accepted:
|
||||
decimal_str, comma_str = get_reading_separators(NumberFormat.from_string(number_format))
|
||||
with self.subTest(number_format=number_format, value=value):
|
||||
self.assertEqual(parse_reading(value, decimal_str, comma_str), expected)
|
||||
|
||||
for number_format, value in refused:
|
||||
decimal_str, comma_str = get_reading_separators(NumberFormat.from_string(number_format))
|
||||
with self.subTest(number_format=number_format, value=value):
|
||||
self.assertIsNone(parse_reading(value, decimal_str, comma_str))
|
||||
|
||||
def test_delete_quality_inspection_linked_with_stock_entry(self):
|
||||
item_code = create_item("_Test Cicuular Dependecy Item with QA").name
|
||||
|
||||
|
||||
Reference in New Issue
Block a user