Fix(AppResources): merge UIFormatters with backstop defaults

specifyweb/backend/context/app_resource.py

@@ -68,42 +68,87 @@ def get_app_resource(collection, user, resource_name, additional_default=False):
     if resource_name == 'DataViewQueries':
         return get_data_view_queries_resource(collection, user)
 
-    # Handling for DataObjFormatters to support fallback to defaults
-    if resource_name == 'DataObjFormatters':
+    # UIFormatters + additional_default: return Backstop only (same contract as
+    # TypeSearches with additionaldefault=true). Do not merge custom resources.
+    if resource_name == 'UIFormatters' and additional_default:
+        return load_resource_at_level(collection, user, 'Backstop', resource_name)
+
+    # Handling for DataObjFormatters / UIFormatters to support fallback to defaults.
+    # Custom resources at a more-specific level would otherwise fully replace
+    # backstop definitions (see #6243, #8676).
+    if resource_name in ('DataObjFormatters', 'UIFormatters'):
         custom_formatter = None
-        default_formatter = None
-        
-        # Try to search for the the custom formatter first
+        custom_level = None
+
+        # Try to search for the custom formatter first
         for level in DIR_LEVELS:
             # First look in the database.
             from_db = get_app_resource_from_db(collection, user, level, resource_name)
             if from_db is not None:
                 custom_formatter = from_db
+                custom_level = level
                 break
 
             # If resource was not found, look on the filesystem.
             from_fs = load_resource_at_level(collection, user, level, resource_name)
             if from_fs is not None:
                 custom_formatter = from_fs
+                custom_level = level
                 break
-        
+
         # If custom formatter not found, try to load the default formatter from the backstop level
         default_formatter = load_resource_at_level(collection, user, 'Backstop', resource_name)
-
-        # If both custom and default formatters are found, merge them
+
+        if resource_name == 'UIFormatters':
+            # Merge by format name: many UIFormatters share the same class
+            # (e.g. several CatalogNumber* formats on CollectionObject).
+            # Order: most-specific custom → Common → Backstop so overlapping
+            # names keep the most-specific definition (#8676).
+            if custom_formatter is not None:
+                merged_xml = custom_formatter[0]
+
+                # When custom is above Common, also pull Common definitions.
+                if custom_level not in ('Common', 'Backstop'):
+                    common_formatter = get_app_resource_from_db(
+                        collection, user, 'Common', resource_name
+                    )
+                    if common_formatter is None:
+                        common_formatter = load_resource_at_level(
+                            collection, user, 'Common', resource_name
+                        )
+                    if common_formatter is not None:
+                        merged_xml = merge_ui_formatters(
+                            merged_xml, common_formatter[0]
+                        )
+
+                if default_formatter is not None:
+                    merged_xml = merge_ui_formatters(
+                        merged_xml, default_formatter[0]
+                    )
+
+                return merged_xml, custom_formatter[1], custom_formatter[2]
+
+            if default_formatter is not None:
+                return default_formatter
+
+            return None
+
+        # DataObjFormatters: merge by class with Backstop only.
         if custom_formatter is not None and default_formatter is not None:
-            merged_formatter = merge_formatters(custom_formatter[0], default_formatter[0])
+            merged_formatter = merge_formatters(
+                custom_formatter[0], default_formatter[0]
+            )
             return merged_formatter, custom_formatter[1], custom_formatter[2]
-        
+
         # Return the custom formatter if it exists, otherwise return the default
         if custom_formatter is not None:
             return custom_formatter
-        
+
         if default_formatter is not None:
             return default_formatter
-        
+
         return None
-    
+
     if not additional_default:
         # Traverse the hierarchy.
         for level in DIR_LEVELS:
@@ -323,15 +368,15 @@ def get_app_resource_dirs_for_level(collection, user, level):
     return Spappresourcedir.objects.filter(**filters)
 
 def merge_formatters(custom_xml, default_xml):
-    """Merge custom and default XML formatters.
-    This function takes two XML strings, one for custom formatters and one for default formatters,
-    and merges them by adding any formatters from the default XML that are not already present in
-    the custom XML. It returns the merged XML as a string.
+    """Merge custom and default XML DataObjFormatters by ``class``.
+
+    Adds any formatters from the default XML whose ``class`` is not already
+    present in the custom XML. Returns the merged XML as a string.
     """
-    
+
     custom_root = ElementTree.fromstring(custom_xml)
     default_root = ElementTree.fromstring(default_xml)
-    
+
     # Create a set of existing classes in the custom XML to avoid duplicates
     custom_classes = set()
     for format_elem in custom_root.findall('format'):
@@ -344,6 +389,33 @@ def merge_formatters(custom_xml, default_xml):
         class_attr = format_elem.get('class')
         if class_attr and class_attr not in custom_classes:
             custom_root.append(format_elem)
-    
+
     # Convert the merged XML tree back to a string
     return ElementTree.tostring(custom_root, encoding='utf-8').decode('utf-8')
+
+
+def merge_ui_formatters(custom_xml, default_xml):
+    """Merge custom and default UIFormatters by format ``name``.
+
+    Unlike DataObjFormatters (merged by ``class``), several UIFormatters can
+    share the same datamodel class (e.g. multiple catalog-number formats on
+    CollectionObject). Custom entries with the same name win; missing names
+    are filled from the default/backstop resource.
+    """
+
+    custom_root = ElementTree.fromstring(custom_xml)
+    default_root = ElementTree.fromstring(default_xml)
+
+    custom_names = {
+        format_elem.get('name')
+        for format_elem in custom_root.findall('format')
+        if format_elem.get('name')
+    }
+
+    for format_elem in default_root.findall('format'):
+        name = format_elem.get('name')
+        if name and name not in custom_names:
+            custom_root.append(format_elem)
+            custom_names.add(name)
+
+    return ElementTree.tostring(custom_root, encoding='utf-8').decode('utf-8')

specifyweb/backend/context/tests/test_app_resource.py

@@ -1,13 +1,62 @@
+from xml.etree import ElementTree
+
 from django.test import Client
 
 from specifyweb.specify.tests.test_api import ApiTests
 from unittest.mock import Mock, patch
 
-from specifyweb.backend.context.app_resource import get_data_view_queries_resource
+from specifyweb.backend.context.app_resource import (
+    get_app_resource,
+    get_data_view_queries_resource,
+    merge_ui_formatters,
+)
 
 
 FOUND_RESOURCE = ('<resource><text>"Value"</text></resource>', "text/xml", 4)
 
+CUSTOM_UI_FORMATTERS = """<?xml version="1.0" encoding="UTF-8"?>
+<formats>
+  <format system="false" name="GridRefMGRS" class="edu.ku.brc.specify.datamodel.Locality" fieldname="text3" default="false">
+    <field type="regex" value="[A-Z]+" pattern="AA" />
+  </format>
+  <format system="false" name="CatalogNumber" class="edu.ku.brc.specify.datamodel.CollectionObject" fieldname="catalogNumber" default="true">
+    <field type="numeric" size="6" inc="true"/>
+  </format>
+</formats>
+"""
+
+COMMON_UI_FORMATTERS = """<?xml version="1.0" encoding="UTF-8"?>
+<formats>
+  <format system="false" name="AccessionNumber" class="edu.ku.brc.specify.datamodel.Accession" fieldname="accessionNumber" default="true">
+    <field type="year" size="4" value="YEAR"/>
+    <field type="separator" value="-"/>
+    <field type="alphanumeric" size="2" value="AA"/>
+    <field type="separator" value="-"/>
+    <field type="numeric" size="3" inc="true"/>
+  </format>
+  <format system="false" name="CatalogNumber" class="edu.ku.brc.specify.datamodel.CollectionObject" fieldname="catalogNumber" default="true">
+    <field type="regex" value="COMMON" pattern="COMMON" />
+  </format>
+</formats>
+"""
+
+BACKSTOP_UI_FORMATTERS = """<?xml version="1.0" encoding="UTF-8"?>
+<formats>
+  <format system="false" name="CatalogNumber" class="edu.ku.brc.specify.datamodel.CollectionObject" fieldname="catalogNumber" default="true">
+    <field type="year" size="4" value="YEAR"/>
+  </format>
+  <format system="false" name="AccessionNumber" class="edu.ku.brc.specify.datamodel.Accession" fieldname="accessionNumber" default="true">
+    <field type="regex" value="BACKSTOP" pattern="BACKSTOP" />
+  </format>
+  <format system="true" name="CatalogNumberNumeric" class="edu.ku.brc.specify.datamodel.CollectionObject" fieldname="catalogNumber" default="false">
+    <external>edu.ku.brc.specify.ui.CatalogNumberUIFieldFormatter</external>
+  </format>
+  <format system="true" name="CatalogNumberString" class="edu.ku.brc.specify.datamodel.CollectionObject" fieldname="catalogNumber" default="false">
+    <external>edu.ku.brc.specify.ui.CatalogNumberStringUIFieldFormatter</external>
+  </format>
+</formats>
+"""
+
 
 class TestAppResource(ApiTests):
 
@@ -90,3 +139,142 @@ def test_not_found_quiet(self, get_app_resource: Mock):
 
         response = c.get("/context/app.resource?name='simple'&quiet='true'")
         self._assertStatusCodeEqual(response, 204)
+
+    def test_merge_ui_formatters_by_name(self):
+        merged = merge_ui_formatters(CUSTOM_UI_FORMATTERS, BACKSTOP_UI_FORMATTERS)
+        root = ElementTree.fromstring(merged)
+        names = [fmt.get('name') for fmt in root.findall('format')]
+
+        self.assertEqual(
+            names,
+            [
+                'GridRefMGRS',
+                'CatalogNumber',
+                'AccessionNumber',
+                'CatalogNumberNumeric',
+                'CatalogNumberString',
+            ],
+        )
+        # Custom CatalogNumber wins over backstop (keeps numeric part, not year).
+        catalog_number = root.find("./format[@name='CatalogNumber']")
+        self.assertIsNotNone(catalog_number)
+        self.assertIsNotNone(catalog_number.find("./field[@type='numeric']"))
+        self.assertIsNone(catalog_number.find("./field[@type='year']"))
+
+    @patch("specifyweb.backend.context.app_resource.load_resource_at_level")
+    @patch("specifyweb.backend.context.app_resource.get_app_resource_from_db")
+    def test_uiformatters_merge_custom_with_backstop(
+        self, get_from_db: Mock, load_from_filesystem: Mock
+    ):
+        get_from_db.side_effect = lambda _collection, _user, level, _name: (
+            (CUSTOM_UI_FORMATTERS, 'text/xml', 23) if level == 'Collection' else None
+        )
+        load_from_filesystem.side_effect = lambda _collection, _user, level, _name: (
+            (BACKSTOP_UI_FORMATTERS, 'text/xml', None) if level == 'Backstop' else None
+        )
+
+        result = get_app_resource(self.collection, self.specifyuser, 'UIFormatters')
+
+        self.assertIsNotNone(result)
+        data, mimetype, resource_id = result
+        self.assertEqual(mimetype, 'text/xml')
+        self.assertEqual(resource_id, 23)
+
+        names = {
+            fmt.get('name')
+            for fmt in ElementTree.fromstring(data).findall('format')
+        }
+        self.assertEqual(
+            names,
+            {
+                'GridRefMGRS',
+                'CatalogNumber',
+                'AccessionNumber',
+                'CatalogNumberNumeric',
+                'CatalogNumberString',
+            },
+        )
+
+    @patch("specifyweb.backend.context.app_resource.load_resource_at_level")
+    @patch("specifyweb.backend.context.app_resource.get_app_resource_from_db")
+    def test_uiformatters_merge_common_before_backstop(
+        self, get_from_db: Mock, load_from_filesystem: Mock
+    ):
+        get_from_db.side_effect = lambda _collection, _user, level, _name: {
+            'Collection': (CUSTOM_UI_FORMATTERS, 'text/xml', 23),
+            'Common': (COMMON_UI_FORMATTERS, 'text/xml', 11),
+        }.get(level)
+        load_from_filesystem.side_effect = lambda _collection, _user, level, _name: (
+            (BACKSTOP_UI_FORMATTERS, 'text/xml', None) if level == 'Backstop' else None
+        )
+
+        result = get_app_resource(self.collection, self.specifyuser, 'UIFormatters')
+
+        self.assertIsNotNone(result)
+        data, mimetype, resource_id = result
+        self.assertEqual(mimetype, 'text/xml')
+        self.assertEqual(resource_id, 23)
+
+        root = ElementTree.fromstring(data)
+        names = [fmt.get('name') for fmt in root.findall('format')]
+        self.assertEqual(
+            names,
+            [
+                'GridRefMGRS',
+                'CatalogNumber',
+                'AccessionNumber',
+                'CatalogNumberNumeric',
+                'CatalogNumberString',
+            ],
+        )
+        # Collection CatalogNumber wins over Common and Backstop.
+        catalog_number = root.find("./format[@name='CatalogNumber']")
+        self.assertIsNotNone(catalog_number.find("./field[@type='numeric']"))
+        self.assertIsNone(catalog_number.find("./field[@type='regex']"))
+        self.assertIsNone(catalog_number.find("./field[@type='year']"))
+        # AccessionNumber is only in Common/Backstop: Common wins over Backstop.
+        accession_number = root.find("./format[@name='AccessionNumber']")
+        self.assertIsNotNone(accession_number.find("./field[@type='year']"))
+        self.assertIsNone(accession_number.find("./field[@type='regex']"))
+
+    @patch("specifyweb.backend.context.app_resource.load_resource_at_level")
+    @patch("specifyweb.backend.context.app_resource.get_app_resource_from_db")
+    def test_uiformatters_additional_default_skips_custom(
+        self, get_from_db: Mock, load_from_filesystem: Mock
+    ):
+        get_from_db.return_value = (CUSTOM_UI_FORMATTERS, 'text/xml', 23)
+        load_from_filesystem.return_value = (
+            BACKSTOP_UI_FORMATTERS,
+            'text/xml',
+            None,
+        )
+
+        result = get_app_resource(
+            self.collection,
+            self.specifyuser,
+            'UIFormatters',
+            additional_default=True,
+        )
+
+        self.assertIsNotNone(result)
+        data, mimetype, resource_id = result
+        self.assertEqual(mimetype, 'text/xml')
+        self.assertIsNone(resource_id)
+        names = {
+            fmt.get('name')
+            for fmt in ElementTree.fromstring(data).findall('format')
+        }
+        self.assertEqual(
+            names,
+            {
+                'CatalogNumber',
+                'AccessionNumber',
+                'CatalogNumberNumeric',
+                'CatalogNumberString',
+            },
+        )
+        self.assertNotIn('GridRefMGRS', names)
+        get_from_db.assert_not_called()
+        load_from_filesystem.assert_called_once_with(
+            self.collection, self.specifyuser, 'Backstop', 'UIFormatters'
+        )