diff --git a/CHANGES.rst b/CHANGES.rst index a22864f1f9..f314d3146b 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -4,6 +4,7 @@ Changelog 2.7.0 (unreleased) ------------------ +- #3049 Fix sample template records stored in types the schema refuses - #3060 Fix empty link text in the Client share field - #3059 Fix Indian Rupee symbol displayed as raw CLDR choice pattern - #3053 Fix TypeError when saving a Method with empty Instruments diff --git a/src/senaite/core/content/sampletemplate.py b/src/senaite/core/content/sampletemplate.py index 94639de7de..85795a4999 100644 --- a/src/senaite/core/content/sampletemplate.py +++ b/src/senaite/core/content/sampletemplate.py @@ -32,6 +32,7 @@ from senaite.core.interfaces import ISampleTemplate from senaite.core.schema import UIDReferenceField from senaite.core.schema.fields import DataGridRow +from senaite.core.schema.uidreferencefield import to_uid_list from senaite.core.z3cform.widgets.datagrid import DataGridWidgetFactory from senaite.core.z3cform.widgets.listing.widget import ListingWidgetFactory from senaite.core.z3cform.widgets.uidreference import UIDReferenceWidgetFactory @@ -402,24 +403,11 @@ def setPartitions(self, value): value = [value] records = [] for v in value: - part_id = v.get("part_id", "") - container = v.get("container", "") - preservation = v.get("preservation", "") - sampletype = v.get("sampletype", "") - - # ensure UIDs for reference fields - if container: - container = api.get_uid(container) - if preservation: - preservation = api.get_uid(preservation) - if sampletype: - sampletype = api.get_uid(sampletype) - records.append({ - "part_id": part_id, - "container": container, - "preservation": preservation, - "sampletype": sampletype, + "part_id": api.safe_unicode(v.get("part_id") or ""), + "container": to_uid_list(v.get("container")), + "preservation": to_uid_list(v.get("preservation")), + "sampletype": to_uid_list(v.get("sampletype")), }) mutator = self.mutator("partitions") mutator(self, records) @@ -507,7 +495,7 @@ def setServices(self, value, keep_inactive=True): records.append({ "uid": uid, "hidden": hidden, - "part_id": part_id, + "part_id": api.safe_unicode(part_id or ""), }) if keep_inactive: diff --git a/src/senaite/core/profiles/default/metadata.xml b/src/senaite/core/profiles/default/metadata.xml index 30366279f7..dbdbf31e5f 100644 --- a/src/senaite/core/profiles/default/metadata.xml +++ b/src/senaite/core/profiles/default/metadata.xml @@ -1,6 +1,6 @@ - 2758 + 2759 profile-Products.ATContentTypes:base profile-senaite.impress:default diff --git a/src/senaite/core/schema/uidreferencefield.py b/src/senaite/core/schema/uidreferencefield.py index 82de3f99db..e5ffc0f446 100644 --- a/src/senaite/core/schema/uidreferencefield.py +++ b/src/senaite/core/schema/uidreferencefield.py @@ -113,6 +113,27 @@ def get_backref_storage(context): return annotation[BACKREFS_STORAGE] +def to_uid_list(value): + """Normalize a single reference to the list this field stores it in + + A UIDReferenceField derives from `zope.schema.List` and holds a + list even where it references a single object, so a sub-field of a + datagrid row is refused on validation when the bare UID is stored + instead. See https://github.com/senaite/senaite.core/pull/2630 + + :param value: object, UID or a list of either, may be empty + :type value: Any + :returns: list with the UID of the referenced object, or an empty + list + :rtype: list + """ + if isinstance(value, (list, tuple)): + value = value[0] if value else None + if not value: + return [] + return [api.get_uid(value)] + + @implementer(IUIDReferenceField) class UIDReferenceField(List, BaseField): """Stores UID references to other objects diff --git a/src/senaite/core/tests/doctests/SampleTemplate.rst b/src/senaite/core/tests/doctests/SampleTemplate.rst index 995bb8e306..a5fe5fd631 100644 --- a/src/senaite/core/tests/doctests/SampleTemplate.rst +++ b/src/senaite/core/tests/doctests/SampleTemplate.rst @@ -209,6 +209,24 @@ Test get/set methods: >>> len(template1.getPartitions()) 2 +The references come back as the UIDs they went in as, no matter which of +the two shapes they were given in: + + >>> part1, part2 = template1.getPartitions() + >>> part1["container"] == part2["container"] == api.get_uid(container) + True + >>> part1["preservation"] == api.get_uid(preservation) + True + >>> part1["sampletype"] == api.get_uid(sampletype) + True + +And the stored records satisfy the field, which they do not when a single +valued reference is stored as the bare UID instead of a list of one: + + >>> from senaite.core.content.sampletemplate import ISampleTemplateSchema + >>> field = ISampleTemplateSchema["partitions"] + >>> field.validate(field.get(template1)) + Auto Partition ^^^^^^^^^^^^^^ @@ -294,10 +312,12 @@ Check if a specific analysis service is configured as "hidden": >>> template1.isAnalysisServiceHidden(Au) True -Get the partition ID for a given service: +Get the partition ID for a given service. It comes back as the text the +record schema holds, while a service that is not assigned falls back to +the empty native string: >>> template1.getAnalysisServicePartitionID(Au) - 'part-2' + u'part-2' >>> template1.getAnalysisServicePartitionID(Zn) '' diff --git a/src/senaite/core/upgrade/v02_07_000.py b/src/senaite/core/upgrade/v02_07_000.py index 472ffdde52..639908a456 100644 --- a/src/senaite/core/upgrade/v02_07_000.py +++ b/src/senaite/core/upgrade/v02_07_000.py @@ -91,6 +91,10 @@ version = "2.7.0" # Remember version number in metadata.xml and setup.py profile = "profile-{0}:default".format(product) +# Single valued reference sub-fields of a partition record, +# which are stored as a list of one UID +PARTITION_REFERENCES = ("container", "preservation", "sampletype") + REMOVE_AT_TYPES = [ "ARReport", "AuditLog", @@ -3155,3 +3159,74 @@ def migrate_method_to_dx(src, destination): target.reindexObject() logger.info("Migrated Method from %s -> %s" % (src, target)) + + +def to_reference_list(value): + """Return a single valued partition reference as the list it is stored in + + :param value: the stored value, a bare UID or a list already + :type value: str, list or None + :returns: list with the UID, or an empty list + :rtype: list + """ + if isinstance(value, (list, tuple)): + return list(value) + return [value] if value else [] + + +def fix_partition_record(record): + """Return a partition record with its values in the stored types + + :param record: a single partition of a sample template + :type record: dict + :returns: the record with list references and a text partition id + :rtype: dict + """ + fixed = dict(record) + fixed["part_id"] = u(record.get("part_id") or "") + for name in PARTITION_REFERENCES: + fixed[name] = to_reference_list(record.get(name)) + return fixed + + +def fix_service_record(record): + """Return a service record with its partition id in the stored type + + :param record: a single service setting of a sample template + :type record: dict + :returns: the record with a text partition id + :rtype: dict + """ + fixed = dict(record) + fixed["part_id"] = u(record.get("part_id") or "") + return fixed + + +def fix_sampletemplate_records(tool): + """Store the records of sample templates in the types the schema holds + + `setPartitions` stored the single valued reference sub-fields of a + partition as the bare UID, where the record schema holds a list of one, + and both setters stored the partition id as a native string, where the + schema holds text. Every template that the 2.6.0 migration brought over + from Archetypes went through those two setters, and its partitions read + back the first character of a UID instead of the reference. + """ + logger.info("Fixing the records of sample templates ...") + brains = api.search({"portal_type": "SampleTemplate"}, SETUP_CATALOG) + total = len(brains) + for num, brain in enumerate(brains): + if num and num % 100 == 0: + logger.info("Fixing sample template records: %s/%s" + % (num, total)) + template = api.get_object(brain) + partitions = list(getattr(template, "partitions", None) or []) + services = list(getattr(template, "services", None) or []) + fixed_partitions = [fix_partition_record(r) for r in partitions] + fixed_services = [fix_service_record(r) for r in services] + if fixed_partitions != partitions: + template.partitions = fixed_partitions + if fixed_services != services: + template.services = fixed_services + + logger.info("Fixing the records of sample templates [DONE]") diff --git a/src/senaite/core/upgrade/v02_07_000.zcml b/src/senaite/core/upgrade/v02_07_000.zcml index dc231a6775..d921a7eb4e 100644 --- a/src/senaite/core/upgrade/v02_07_000.zcml +++ b/src/senaite/core/upgrade/v02_07_000.zcml @@ -2,6 +2,18 @@ xmlns="http://namespaces.zope.org/zope" xmlns:genericsetup="http://namespaces.zope.org/genericsetup"> + +