From 157ee3b6719ccbd594414ec2dcc0d8add7479c1b Mon Sep 17 00:00:00 2001 From: Michal Torma Date: Thu, 15 Oct 2026 21:10:48 +0200 Subject: [PATCH 1/3] Fix(SystemConfig): scope catalog format pick list to target discipline When creating or editing a Collection, Catalog Num Format Name lists UIFormatters for that collection's discipline (plus sibling collections, common, and backstop) instead of the login session's formatters. Fixes #8678. Co-authored-by: Cursor --- specifyweb/backend/context/app_resource.py | 96 ++++++++++++++++ .../context/tests/test_app_resource.py | 75 ++++++++++++- specifyweb/backend/context/urls.py | 1 + specifyweb/backend/context/views.py | 65 ++++++++++- .../lib/components/FormFields/ComboBox.tsx | 3 + .../PickLists/CatalogNumberFormatPickList.tsx | 104 ++++++++++++++++++ 6 files changed, 342 insertions(+), 2 deletions(-) create mode 100644 specifyweb/frontend/js_src/lib/components/PickLists/CatalogNumberFormatPickList.tsx diff --git a/specifyweb/backend/context/app_resource.py b/specifyweb/backend/context/app_resource.py index 53be974152a..5b2c28029eb 100644 --- a/specifyweb/backend/context/app_resource.py +++ b/specifyweb/backend/context/app_resource.py @@ -347,3 +347,99 @@ def merge_formatters(custom_xml, default_xml): # Convert the merged XML tree back to a string return ElementTree.tostring(custom_root, encoding='utf-8').decode('utf-8') + + +def merge_ui_formatters_by_name(*xml_documents): + """Merge UIFormatters XML documents by format ``name`` (first wins). + + Used when listing catalog-number formats for a target discipline so that + collection/discipline/common/backstop definitions combine without the + login-collection session scope (#8678). + """ + merged_root = None + seen_names = set() + for xml in xml_documents: + if not xml: + continue + root = ElementTree.fromstring(xml) + if merged_root is None: + merged_root = ElementTree.Element(root.tag, root.attrib) + for format_elem in root.findall('format'): + name = format_elem.get('name') + if not name or name in seen_names: + continue + merged_root.append(format_elem) + seen_names.add(name) + if merged_root is None: + return None + return ElementTree.tostring(merged_root, encoding='utf-8').decode('utf-8') + + +def _uiformatters_xml_from_dirs(dirs): + resources = Spappresourcedata.objects.filter( + spappresource__name='UIFormatters', + spappresource__spappresourcedir__in=dirs, + ) + return [resource.data for resource in resources if resource.data] + + +def get_catalog_number_format_names_for_discipline(discipline): + """Catalog-number UIFormatter names visible for creating a collection under *discipline*. + + Includes formats from: + - collections already in that discipline, + - the discipline itself, + - common/global, + - backstop defaults. + + Does **not** use the logged-in collection's unrelated UIFormatters. + """ + from specifyweb.specify.models import Collection + + xml_documents = [] + + # Sibling collection-level UIFormatters (most specific within the discipline). + for collection in Collection.objects.filter(discipline=discipline): + dirs = Spappresourcedir.objects.filter( + ispersonal=False, + usertype__isnull=True, + collection=collection, + discipline=discipline, + ) + xml_documents.extend(_uiformatters_xml_from_dirs(dirs)) + + # Discipline-level. + discipline_dirs = Spappresourcedir.objects.filter( + ispersonal=False, + usertype__isnull=True, + collection__isnull=True, + discipline=discipline, + ) + xml_documents.extend(_uiformatters_xml_from_dirs(discipline_dirs)) + + # Common / global. + common_dirs = Spappresourcedir.objects.filter( + ispersonal=False, + usertype__isnull=True, + collection__isnull=True, + discipline__isnull=True, + ) + xml_documents.extend(_uiformatters_xml_from_dirs(common_dirs)) + + # Backstop filesystem defaults. + backstop = load_resource_at_level(None, None, 'Backstop', 'UIFormatters') + if backstop is not None: + xml_documents.append(backstop[0]) + + merged = merge_ui_formatters_by_name(*xml_documents) + if not merged: + return [] + + names = [] + for format_elem in ElementTree.fromstring(merged).findall('format'): + if format_elem.get('fieldname') != 'catalogNumber': + continue + name = format_elem.get('name') + if name: + names.append(name) + return names diff --git a/specifyweb/backend/context/tests/test_app_resource.py b/specifyweb/backend/context/tests/test_app_resource.py index 900576db564..3059785d0db 100644 --- a/specifyweb/backend/context/tests/test_app_resource.py +++ b/specifyweb/backend/context/tests/test_app_resource.py @@ -1,13 +1,33 @@ +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_catalog_number_format_names_for_discipline, + get_data_view_queries_resource, + merge_ui_formatters_by_name, +) FOUND_RESOURCE = ('"Value"', "text/xml", 4) +DISCIPLINE_UI_FORMATTERS = """ + + + + +""" + +BACKSTOP_UI_FORMATTERS = """ + + + + +""" + class TestAppResource(ApiTests): @@ -90,3 +110,56 @@ 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_first_wins(self): + merged = merge_ui_formatters_by_name( + DISCIPLINE_UI_FORMATTERS, BACKSTOP_UI_FORMATTERS + ) + root = ElementTree.fromstring(merged) + names = [fmt.get('name') for fmt in root.findall('format')] + self.assertEqual( + names, ['BirdsCatalog', 'GridRefMGRS', 'CatalogNumber', 'CatalogNumberNumeric'] + ) + + @patch("specifyweb.backend.context.app_resource.load_resource_at_level") + @patch("specifyweb.backend.context.app_resource._uiformatters_xml_from_dirs") + def test_catalog_number_format_names_for_discipline( + self, xml_from_dirs: Mock, load_from_filesystem: Mock + ): + xml_from_dirs.return_value = [DISCIPLINE_UI_FORMATTERS] + load_from_filesystem.return_value = ( + BACKSTOP_UI_FORMATTERS, + 'text/xml', + None, + ) + + names = get_catalog_number_format_names_for_discipline(self.discipline) + self.assertEqual( + names, + ['BirdsCatalog', 'CatalogNumber', 'CatalogNumberNumeric'], + ) + self.assertNotIn('GridRefMGRS', names) + + def test_catalog_number_format_names_endpoint(self): + c = Client() + c.force_login(self.specifyuser) + + with patch( + 'specifyweb.backend.context.views.get_catalog_number_format_names_for_discipline', + return_value=['CatalogNumber', 'BirdsCatalog'], + ): + response = c.get( + f'/context/catalog_number_format_names.json?discipline={self.discipline.id}' + ) + + self._assertStatusCodeEqual(response, 200) + self.assertEqual( + response.json(), + {'names': ['CatalogNumber', 'BirdsCatalog']}, + ) + + def test_catalog_number_format_names_endpoint_requires_discipline(self): + c = Client() + c.force_login(self.specifyuser) + response = c.get('/context/catalog_number_format_names.json') + self._assertStatusCodeEqual(response, 400) diff --git a/specifyweb/backend/context/urls.py b/specifyweb/backend/context/urls.py index 844fcb0e7fd..bafdb9ddf3f 100644 --- a/specifyweb/backend/context/urls.py +++ b/specifyweb/backend/context/urls.py @@ -31,6 +31,7 @@ path('schema_localization.json', views.schema_localization), path('schema_localization_import.json', views.schema_localization_import), path('app.resource', views.app_resource), + path('catalog_number_format_names.json', views.catalog_number_format_names), path('available_related_searches.json', views.available_related_searches), path('remoteprefs.properties', views.remote_prefs), diff --git a/specifyweb/backend/context/views.py b/specifyweb/backend/context/views.py index 53e3a98c03f..3527f513fcf 100644 --- a/specifyweb/backend/context/views.py +++ b/specifyweb/backend/context/views.py @@ -38,7 +38,11 @@ from specifyweb.specify.api.serializers import uri_for_model from specifyweb.specify.utils.specify_jar import specify_jar from specifyweb.specify.views import login_maybe_required, openapi -from .app_resource import get_app_resource, FORM_RESOURCE_EXCLUDED_LST +from .app_resource import ( + FORM_RESOURCE_EXCLUDED_LST, + get_app_resource, + get_catalog_number_format_names_for_discipline, +) from .schema_localization import get_schema_languages, get_schema_localization from .viewsets import get_views from specifyweb.backend.setup_tool.api import ( @@ -437,6 +441,65 @@ def app_resource(request): return response +@openapi(schema={ + "get": { + "parameters": [ + { + "name": "discipline", + "in": "query", + "required": True, + "schema": {"type": "integer"}, + "description": "Discipline id for which catalog-number format names should be listed.", + } + ], + "responses": { + "200": { + "description": "Catalog-number UIFormatter names for the discipline (+ global/backstop).", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "names": { + "type": "array", + "items": {"type": "string"}, + } + }, + "required": ["names"], + } + } + }, + }, + "400": {"description": "Missing or invalid discipline id."}, + "404": {"description": "Discipline not found."}, + }, + } +}) +@login_maybe_required +@require_http_methods(['GET', 'HEAD']) +def catalog_number_format_names(request): + """List catalog-number format names relevant to a target discipline. + + Used by System Configuration when creating/editing a Collection so the + Catalog Num Format Name pick list is not limited to the login collection's + UIFormatters (#8678). + """ + raw = request.GET.get('discipline') + try: + discipline_id = int(raw) + except (TypeError, ValueError): + return HttpResponseBadRequest('discipline query parameter is required') + + try: + discipline = Discipline.objects.get(id=discipline_id) + except Discipline.DoesNotExist: + raise Http404() + + return JsonResponse({ + 'names': get_catalog_number_format_names_for_discipline(discipline), + }) + + @login_maybe_required @require_http_methods(['GET', 'HEAD']) @cache_control(max_age=86400, private=True) diff --git a/specifyweb/frontend/js_src/lib/components/FormFields/ComboBox.tsx b/specifyweb/frontend/js_src/lib/components/FormFields/ComboBox.tsx index 0379e69f261..8ac3e9db8de 100644 --- a/specifyweb/frontend/js_src/lib/components/FormFields/ComboBox.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormFields/ComboBox.tsx @@ -15,6 +15,7 @@ import type { SpecifyResource } from '../DataModel/legacyTypes'; import type { LiteralField, Relationship } from '../DataModel/specifyField'; import type { PickList, Tables } from '../DataModel/types'; import { PickListComboBox } from '../PickLists'; +import { CatalogNumberFormatPickList } from '../PickLists/CatalogNumberFormatPickList'; import { PickListTypes } from '../PickLists/definitions'; import { fetchPickList, getPickListItems } from '../PickLists/fetch'; import { FieldsPickList } from '../PickLists/FieldsPickList'; @@ -42,6 +43,8 @@ export const specialPickLists = { _fieldsPickList: FieldsPickList, _formattersPickList: FormattersPickList, _tableNamePickList: TablesPickList, + // Overrides session-scoped front-end pick list with discipline-scoped names (#8678). + _CatalogNumberUIFormatters: CatalogNumberFormatPickList, } as const; export const specialPickListMapping: RR< diff --git a/specifyweb/frontend/js_src/lib/components/PickLists/CatalogNumberFormatPickList.tsx b/specifyweb/frontend/js_src/lib/components/PickLists/CatalogNumberFormatPickList.tsx new file mode 100644 index 00000000000..5b39c83bee8 --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/PickLists/CatalogNumberFormatPickList.tsx @@ -0,0 +1,104 @@ +/** + * Catalog Num Format Name pick list scoped to a Collection's target discipline + * (+ global/backstop), not the login collection's UIFormatters session. + * + * @see https://github.com/specify/specify7/issues/8678 + */ + +import React from 'react'; + +import { useAsyncState } from '../../hooks/useAsyncState'; +import { ajax } from '../../utils/ajax'; +import type { RA } from '../../utils/types'; +import { idFromUrl, resourceOn } from '../DataModel/resource'; +import { tables } from '../DataModel/tables'; +import { raise } from '../Errors/Crash'; +import { getUiFormatters } from '../FieldFormatters'; +import type { + DefaultComboBoxProps, + PickListItemSimple, +} from '../FormFields/ComboBox'; +import { PickListComboBox } from './index'; + +const catalogNumberField = + tables.CollectionObject.strictGetLiteralField('catalogNumber'); + +const sessionCatalogNumberFormatItems = (): RA => + Object.entries(getUiFormatters()) + .filter(([_, formatter]) => formatter.field === catalogNumberField) + .map(([formatterName]) => ({ + value: formatterName, + title: formatterName, + })); + +async function fetchCatalogNumberFormatItems( + disciplineId: number | undefined +): Promise> { + if (typeof disciplineId !== 'number') { + return sessionCatalogNumberFormatItems(); + } + + const { data } = await ajax<{ readonly names: RA }>( + `/context/catalog_number_format_names.json?discipline=${disciplineId}`, + { + headers: { Accept: 'application/json' }, + } + ); + + return data.names.map((name) => ({ value: name, title: name })); +} + +function disciplineIdFromResource( + resource: DefaultComboBoxProps['resource'] +): number | undefined { + if (resource === undefined) return undefined; + if (resource.specifyTable.name === 'Collection') { + const discipline = resource.get('discipline'); + return typeof discipline === 'string' ? idFromUrl(discipline) : undefined; + } + return undefined; +} + +export function CatalogNumberFormatPickList( + props: DefaultComboBoxProps +): JSX.Element { + const fetchItems = React.useCallback( + async () => fetchCatalogNumberFormatItems(disciplineIdFromResource(props.resource)), + [props.resource] + ); + + const [items, setItems] = useAsyncState>( + fetchItems, + false + ); + + React.useEffect( + () => + props.resource === undefined + ? undefined + : resourceOn( + props.resource, + 'change:discipline', + (): void => { + fetchItems() + .then(setItems) + .catch((error) => { + setItems(undefined); + raise(error); + }); + }, + true + ), + [props.resource, fetchItems, setItems] + ); + + return ( + + ); +} From 2f4e868e61898a8932ca3f88000b3c2d658cd429 Mon Sep 17 00:00:00 2001 From: Michal Torma Date: Thu, 15 Oct 2026 22:52:02 +0200 Subject: [PATCH 2/3] Refactor(CatalogNumberFormatPickList): encapsulate catalog number field retrieval within sessionCatalogNumberFormatItems function Moved the catalog number field retrieval into the sessionCatalogNumberFormatItems function for improved readability and maintainability. --- .../PickLists/CatalogNumberFormatPickList.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/PickLists/CatalogNumberFormatPickList.tsx b/specifyweb/frontend/js_src/lib/components/PickLists/CatalogNumberFormatPickList.tsx index 5b39c83bee8..c9c106cdad2 100644 --- a/specifyweb/frontend/js_src/lib/components/PickLists/CatalogNumberFormatPickList.tsx +++ b/specifyweb/frontend/js_src/lib/components/PickLists/CatalogNumberFormatPickList.tsx @@ -20,16 +20,16 @@ import type { } from '../FormFields/ComboBox'; import { PickListComboBox } from './index'; -const catalogNumberField = - tables.CollectionObject.strictGetLiteralField('catalogNumber'); - -const sessionCatalogNumberFormatItems = (): RA => - Object.entries(getUiFormatters()) +const sessionCatalogNumberFormatItems = (): RA => { + const catalogNumberField = + tables.CollectionObject.strictGetLiteralField('catalogNumber'); + return Object.entries(getUiFormatters()) .filter(([_, formatter]) => formatter.field === catalogNumberField) .map(([formatterName]) => ({ value: formatterName, title: formatterName, })); +}; async function fetchCatalogNumberFormatItems( disciplineId: number | undefined From be452e0b973e415f70e24f1edf1e9d9aa2bcf124 Mon Sep 17 00:00:00 2001 From: Michal Torma Date: Fri, 16 Oct 2026 11:44:26 +0200 Subject: [PATCH 3/3] Enhance catalog number UIFormatters handling in app_resource Added functions to filter and append catalogNumber UIFormatters from XML documents, ensuring that non-catalog formats do not block catalogNumber formats with the same name. Updated the get_catalog_number_format_names_for_discipline function to utilize these new methods for improved clarity and efficiency. Added unit tests to verify the filtering behavior. Co-authored-by: Cursor --- specifyweb/backend/context/app_resource.py | 86 +++++++++---- .../context/tests/test_app_resource.py | 48 ++++++- .../PickLists/CatalogNumberFormatPickList.tsx | 119 +++++++++++------- 3 files changed, 179 insertions(+), 74 deletions(-) diff --git a/specifyweb/backend/context/app_resource.py b/specifyweb/backend/context/app_resource.py index 5b2c28029eb..ae344be8052 100644 --- a/specifyweb/backend/context/app_resource.py +++ b/specifyweb/backend/context/app_resource.py @@ -383,63 +383,105 @@ def _uiformatters_xml_from_dirs(dirs): return [resource.data for resource in resources if resource.data] +def _catalog_number_uiformatters_only(xml): + """Return a UIFormatters document containing only catalogNumber formats. + + Filtering before name-dedupe avoids a non-catalog ```` + blocking a later catalogNumber format that reuses the same name. + """ + root = ElementTree.fromstring(xml) + filtered = ElementTree.Element(root.tag, root.attrib) + for format_elem in root.findall('format'): + if format_elem.get('fieldname') != 'catalogNumber': + continue + if not format_elem.get('name'): + continue + filtered.append(format_elem) + if len(filtered) == 0: + return None + return ElementTree.tostring(filtered, encoding='utf-8').decode('utf-8') + + +def _append_catalog_number_uiformatters(xml_documents, xml): + filtered = _catalog_number_uiformatters_only(xml) if xml else None + if filtered: + xml_documents.append(filtered) + + +def _append_fs_catalog_number_uiformatters(xml_documents, collection, level): + """Append catalogNumber UIFormatters from the filesystem at *level*, if present.""" + resource = load_resource_at_level(collection, None, level, 'UIFormatters') + if resource is not None: + _append_catalog_number_uiformatters(xml_documents, resource[0]) + + def get_catalog_number_format_names_for_discipline(discipline): """Catalog-number UIFormatter names visible for creating a collection under *discipline*. - Includes formats from: - - collections already in that discipline, - - the discipline itself, - - common/global, - - backstop defaults. + Includes formats from (most specific first; first name wins): + - collections already in that discipline (DB), + - the discipline itself (DB, then filesystem), + - common/global (DB, then filesystem), + - backstop defaults (filesystem). Does **not** use the logged-in collection's unrelated UIFormatters. + Only ``fieldname="catalogNumber"`` formats are considered before name dedupe. """ + from types import SimpleNamespace + from specifyweb.specify.models import Collection xml_documents = [] + sibling_collections = list(Collection.objects.filter(discipline=discipline)) + # Sibling collection-level UIFormatters (most specific within the discipline). - for collection in Collection.objects.filter(discipline=discipline): + for collection in sibling_collections: dirs = Spappresourcedir.objects.filter( ispersonal=False, usertype__isnull=True, collection=collection, discipline=discipline, ) - xml_documents.extend(_uiformatters_xml_from_dirs(dirs)) + for data in _uiformatters_xml_from_dirs(dirs): + _append_catalog_number_uiformatters(xml_documents, data) - # Discipline-level. + # Discipline-level (DB, then filesystem). discipline_dirs = Spappresourcedir.objects.filter( ispersonal=False, usertype__isnull=True, collection__isnull=True, discipline=discipline, ) - xml_documents.extend(_uiformatters_xml_from_dirs(discipline_dirs)) + for data in _uiformatters_xml_from_dirs(discipline_dirs): + _append_catalog_number_uiformatters(xml_documents, data) + # load_resource_at_level needs a collection for discipline.type → FS path. + # When creating the first collection, synthesize a minimal stand-in. + fs_collection = sibling_collections[0] if sibling_collections else SimpleNamespace( + discipline=discipline + ) + _append_fs_catalog_number_uiformatters(xml_documents, fs_collection, 'Discipline') - # Common / global. + # Common / global (DB, then filesystem). common_dirs = Spappresourcedir.objects.filter( ispersonal=False, usertype__isnull=True, collection__isnull=True, discipline__isnull=True, ) - xml_documents.extend(_uiformatters_xml_from_dirs(common_dirs)) + for data in _uiformatters_xml_from_dirs(common_dirs): + _append_catalog_number_uiformatters(xml_documents, data) + _append_fs_catalog_number_uiformatters(xml_documents, None, 'Common') # Backstop filesystem defaults. - backstop = load_resource_at_level(None, None, 'Backstop', 'UIFormatters') - if backstop is not None: - xml_documents.append(backstop[0]) + _append_fs_catalog_number_uiformatters(xml_documents, None, 'Backstop') merged = merge_ui_formatters_by_name(*xml_documents) if not merged: return [] - names = [] - for format_elem in ElementTree.fromstring(merged).findall('format'): - if format_elem.get('fieldname') != 'catalogNumber': - continue - name = format_elem.get('name') - if name: - names.append(name) - return names + return [ + format_elem.get('name') + for format_elem in ElementTree.fromstring(merged).findall('format') + if format_elem.get('name') + ] diff --git a/specifyweb/backend/context/tests/test_app_resource.py b/specifyweb/backend/context/tests/test_app_resource.py index 3059785d0db..d83a98e9234 100644 --- a/specifyweb/backend/context/tests/test_app_resource.py +++ b/specifyweb/backend/context/tests/test_app_resource.py @@ -6,6 +6,7 @@ from unittest.mock import Mock, patch from specifyweb.backend.context.app_resource import ( + _catalog_number_uiformatters_only, get_catalog_number_format_names_for_discipline, get_data_view_queries_resource, merge_ui_formatters_by_name, @@ -121,16 +122,50 @@ def test_merge_ui_formatters_by_name_first_wins(self): names, ['BirdsCatalog', 'GridRefMGRS', 'CatalogNumber', 'CatalogNumberNumeric'] ) + def test_catalog_number_filter_before_name_dedupe(self): + """A non-catalog format must not block a same-named catalogNumber format.""" + non_catalog_first = """ + + + +""" + catalog_later = """ + + + +""" + # Unfiltered merge would keep only the Locality format. + unfiltered = merge_ui_formatters_by_name(non_catalog_first, catalog_later) + self.assertEqual( + [ + (f.get('name'), f.get('fieldname')) + for f in ElementTree.fromstring(unfiltered).findall('format') + ], + [('SharedName', 'text3')], + ) + + filtered = merge_ui_formatters_by_name( + _catalog_number_uiformatters_only(non_catalog_first), + _catalog_number_uiformatters_only(catalog_later), + ) + self.assertEqual( + [ + (f.get('name'), f.get('fieldname')) + for f in ElementTree.fromstring(filtered).findall('format') + ], + [('SharedName', 'catalogNumber')], + ) + @patch("specifyweb.backend.context.app_resource.load_resource_at_level") @patch("specifyweb.backend.context.app_resource._uiformatters_xml_from_dirs") def test_catalog_number_format_names_for_discipline( self, xml_from_dirs: Mock, load_from_filesystem: Mock ): xml_from_dirs.return_value = [DISCIPLINE_UI_FORMATTERS] - load_from_filesystem.return_value = ( - BACKSTOP_UI_FORMATTERS, - 'text/xml', - None, + load_from_filesystem.side_effect = lambda _collection, _user, level, _name: ( + (BACKSTOP_UI_FORMATTERS, 'text/xml', None) + if level == 'Backstop' + else None ) names = get_catalog_number_format_names_for_discipline(self.discipline) @@ -139,6 +174,11 @@ def test_catalog_number_format_names_for_discipline( ['BirdsCatalog', 'CatalogNumber', 'CatalogNumberNumeric'], ) self.assertNotIn('GridRefMGRS', names) + # Discipline + Common + Backstop filesystem levels are consulted. + self.assertEqual( + [call.args[2] for call in load_from_filesystem.call_args_list], + ['Discipline', 'Common', 'Backstop'], + ) def test_catalog_number_format_names_endpoint(self): c = Client() diff --git a/specifyweb/frontend/js_src/lib/components/PickLists/CatalogNumberFormatPickList.tsx b/specifyweb/frontend/js_src/lib/components/PickLists/CatalogNumberFormatPickList.tsx index c9c106cdad2..d630a502db6 100644 --- a/specifyweb/frontend/js_src/lib/components/PickLists/CatalogNumberFormatPickList.tsx +++ b/specifyweb/frontend/js_src/lib/components/PickLists/CatalogNumberFormatPickList.tsx @@ -7,7 +7,6 @@ import React from 'react'; -import { useAsyncState } from '../../hooks/useAsyncState'; import { ajax } from '../../utils/ajax'; import type { RA } from '../../utils/types'; import { idFromUrl, resourceOn } from '../DataModel/resource'; @@ -31,66 +30,90 @@ const sessionCatalogNumberFormatItems = (): RA => { })); }; -async function fetchCatalogNumberFormatItems( - disciplineId: number | undefined -): Promise> { - if (typeof disciplineId !== 'number') { - return sessionCatalogNumberFormatItems(); +/** + * Stable key for the formats we intend to show. Used to ignore stale responses + * when discipline changes while a request is in flight. + */ +function catalogNumberFormatRequestKey( + resource: DefaultComboBoxProps['resource'] +): string { + if (resource?.specifyTable.name === 'Collection') { + const discipline = resource.get('discipline'); + return typeof discipline === 'string' + ? `collection:${discipline}` + : 'collection:none'; } - - const { data } = await ajax<{ readonly names: RA }>( - `/context/catalog_number_format_names.json?discipline=${disciplineId}`, - { - headers: { Accept: 'application/json' }, - } - ); - - return data.names.map((name) => ({ value: name, title: name })); + return 'session'; } -function disciplineIdFromResource( +async function fetchCatalogNumberFormatItems( resource: DefaultComboBoxProps['resource'] -): number | undefined { - if (resource === undefined) return undefined; - if (resource.specifyTable.name === 'Collection') { +): Promise> { + // Collection: only show formats for its target discipline (never login session). + if (resource?.specifyTable.name === 'Collection') { const discipline = resource.get('discipline'); - return typeof discipline === 'string' ? idFromUrl(discipline) : undefined; + const disciplineId = + typeof discipline === 'string' ? idFromUrl(discipline) : undefined; + if (typeof disciplineId !== 'number') { + return []; + } + + const { data } = await ajax<{ readonly names: RA }>( + `/context/catalog_number_format_names.json?discipline=${disciplineId}`, + { + headers: { Accept: 'application/json' }, + } + ); + + return data.names.map((name) => ({ value: name, title: name })); } - return undefined; + + // Other tables (e.g. CollectionObjectType) keep session UIFormatters. + return sessionCatalogNumberFormatItems(); } export function CatalogNumberFormatPickList( props: DefaultComboBoxProps ): JSX.Element { - const fetchItems = React.useCallback( - async () => fetchCatalogNumberFormatItems(disciplineIdFromResource(props.resource)), - [props.resource] - ); + const [items, setItems] = React.useState< + RA | undefined + >(undefined); + const requestIdRef = React.useRef(0); - const [items, setItems] = useAsyncState>( - fetchItems, - false - ); + const loadItems = React.useCallback(() => { + const resource = props.resource; + const requestId = ++requestIdRef.current; + const expectedKey = catalogNumberFormatRequestKey(resource); - React.useEffect( - () => - props.resource === undefined - ? undefined - : resourceOn( - props.resource, - 'change:discipline', - (): void => { - fetchItems() - .then(setItems) - .catch((error) => { - setItems(undefined); - raise(error); - }); - }, - true - ), - [props.resource, fetchItems, setItems] - ); + void fetchCatalogNumberFormatItems(resource) + .then((nextItems) => { + if (requestId !== requestIdRef.current) return; + if (catalogNumberFormatRequestKey(resource) !== expectedKey) return; + setItems(nextItems); + }) + .catch((error: unknown) => { + if (requestId !== requestIdRef.current) return; + setItems(undefined); + raise(error); + }); + }, [props.resource]); + + React.useEffect(() => { + setItems(undefined); + loadItems(); + + if (props.resource === undefined) return undefined; + + return resourceOn( + props.resource, + 'change:discipline', + (): void => { + setItems(undefined); + loadItems(); + }, + false + ); + }, [props.resource, loadItems]); return (