Fix sample template records stored in types the schema refuses

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

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:

src/senaite/core/profiles/default/metadata.xml

@@ -1,6 +1,6 @@
 <?xml version="1.0"?>
 <metadata>
-  <version>2758</version>
+  <version>2759</version>
   <dependencies>
     <dependency>profile-Products.ATContentTypes:base</dependency>
     <dependency>profile-senaite.impress:default</dependency>

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

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)
     ''

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]")

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">
 
+  <genericsetup:upgradeStep
+      title="SENAITE.CORE 2.7.0: Fix the records of sample templates"
+      description="Store the single valued references of a partition as the
+                   list of one UID the record schema holds, and the partition
+                   id of both partitions and services as text. Templates that
+                   the 2.6.0 migration brought over from Archetypes read their
+                   partition references back as a single character."
+      source="2758"
+      destination="2759"
+      handler=".v02_07_000.fix_sampletemplate_records"
+      profile="senaite.core:default"/>
+
   <genericsetup:upgradeStep
       title="SENAITE.CORE 2.7.0: Drop ordering annotations from Instruments"
       description="Remove the plone.folder.ordered.order PersistentList and