From 4019815f25a7c7f16791081db589b9b71b2329ff Mon Sep 17 00:00:00 2001 From: melton-jason Date: Sun, 28 Jun 2026 22:19:42 -0500 Subject: [PATCH 01/66] feat: general backend pagination for delete blockers --- specifyweb/backend/delete_blockers/urls.py | 1 + specifyweb/backend/delete_blockers/views.py | 66 ++++++++++++++++++++- 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/specifyweb/backend/delete_blockers/urls.py b/specifyweb/backend/delete_blockers/urls.py index 9945728ca5d..d141bb9f968 100644 --- a/specifyweb/backend/delete_blockers/urls.py +++ b/specifyweb/backend/delete_blockers/urls.py @@ -3,5 +3,6 @@ from . import views urlpatterns = [ re_path(r'^delete_blockers/(?P\w+)/(?P\d+)/$', views.delete_blockers), + re_path(r'^old_delete_blockers/(?P\w+)/(?P\d+)/$', views.old_delete_blockers), ] diff --git a/specifyweb/backend/delete_blockers/views.py b/specifyweb/backend/delete_blockers/views.py index e562390aa76..ac17ff71b95 100644 --- a/specifyweb/backend/delete_blockers/views.py +++ b/specifyweb/backend/delete_blockers/views.py @@ -1,24 +1,32 @@ +from collections import defaultdict + from django import http from django.db import router, transaction -from django.db.models.deletion import Collector +from django.db.models.deletion import Collector, CASCADE, PROTECT from django.db.models import ForeignKey +from django.views.decorators.http import require_POST from specifyweb.middleware.general import require_http_methods from specifyweb.specify.api.crud import ( get_discipline_delete_guard_blockers, get_object_or_404, + get_model, prepare_discipline_for_delete, ) +from specifyweb.specify.models import protect_with_blockers from specifyweb.specify.api.serializers import toJson from specifyweb.specify.views import login_maybe_required @login_maybe_required @require_http_methods(['GET', 'HEAD']) -def delete_blockers(request, model, id): +def old_delete_blockers(request, model, id): """Returns a JSON list of fields on that point to related resources which prevent the resource of that model from being deleted. """ + # limit = request.GET["limit"] + # depth_limit = request.GET["depthLimit"] + obj = get_object_or_404(model, id=int(id)) using = router.db_for_write(obj.__class__, instance=obj) @@ -38,6 +46,60 @@ def delete_blockers(request, model, id): return http.HttpResponse(toJson(result), content_type='application/json') +@login_maybe_required +@require_http_methods(['GET']) +def delete_blockers(request, model, id): + limit = int(request.GET["limit"]) if "limit" in request.GET else 20 + offset = int(request.GET["offset"]) if "offset" in request.GET else 0 + obj = get_object_or_404(model, id=int(id)) + immediate, deferred = fetch_immediate_blockers(obj, limit=limit, offset=offset) + result = { + "results": immediate, + "next": deferred + } + return http.HttpResponse(toJson(result), content_type='application/json') + +def fetch_immediate_blockers(obj, limit=20, offset=0): + all_fields = obj._meta.get_fields(include_hidden=True) + all_relationships = filter( + # Check whether there are any concrete fields that SHOULD be included + # here, like some ToOne fields that acts as blockers + lambda field: field.is_relation and not field.concrete, + all_fields + ) + results = [] + next = [] + for relationship in all_relationships: + related_ids = _prepare_blockers(obj, relationship, limit=limit, offset=offset) + if len(related_ids) == 0: + continue + complete = limit == 0 or len(related_ids) < limit + payload = { + "table": relationship.related_model._meta.model_name, + "field": relationship.field.name, + "ids": list(related_ids), + "offset": offset, + "limit": limit, + "complete": complete + } + if relationship.on_delete is protect_with_blockers or relationship.on_delete is PROTECT: + results.append(payload) + elif relationship.on_delete is CASCADE: + next.append(payload) + return results, next + + +def _prepare_blockers(obj, relationship, limit=20, offset=0): + query_set = ( + relationship.related_model.objects + .filter( + **{relationship.field.name: obj.pk} + ).order_by("pk") + .values_list("pk", flat=True)) + if limit != 0: + query_set = query_set[offset: offset + limit] + return query_set + def _collect_delete_blockers(obj, using) -> list[dict]: collector = Collector(using=using) collector.delete_blockers = [] From f821721267e35c7482175eb43816ddd7ab9d3d75 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Tue, 28 Jul 2026 12:23:09 -0500 Subject: [PATCH 02/66] chore: remove whitespace in .env for consisitency --- .env | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.env b/.env index f9cfe7040ab..1daacb33da3 100644 --- a/.env +++ b/.env @@ -31,7 +31,7 @@ APP_USER_PASSWORD=specify_user # https://discourse.specifysoftware.org/t/allow-support-login-documentation/2838 ALLOW_SUPPORT_LOGIN=false # The amount of time in seconds each token is valid for -SUPPORT_LOGIN_TTL = 180 +SUPPORT_LOGIN_TTL=180 # Make sure to set the `SECRET_KEY` to a unique value SECRET_KEY=change_this_to_some_unique_random_string @@ -60,7 +60,7 @@ CELERY_RESULT_BACKEND=redis://redis/1 # timezone as the operating system. # If running in a Windows environment this must be set to the same as your # system time zone. -TIME_ZONE = America/Chicago +TIME_ZONE=America/Chicago # This variable controls the Specify 7 logging level. Possible values # are: From 383e28cf914124c0a12b8b05c364b331a1311385 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Mon, 3 Aug 2026 09:59:45 -0500 Subject: [PATCH 03/66] fix: incorrect lookup for role policy bulk create (cherry picked from commit b738b91ad650ccb00b561cbce27694c6921c022c) --- specifyweb/backend/permissions/initialize.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/specifyweb/backend/permissions/initialize.py b/specifyweb/backend/permissions/initialize.py index 0f410b82232..d4238c37799 100644 --- a/specifyweb/backend/permissions/initialize.py +++ b/specifyweb/backend/permissions/initialize.py @@ -2,6 +2,7 @@ import logging from collections import defaultdict +from typing import TypedDict from django.db import transaction, connection from django.db.models.functions import Lower @@ -388,8 +389,12 @@ def assign_users_to_roles_during_testing(apps=apps) -> None: } } +class DefaultRole(TypedDict): + description: str + policies: dict[str, tuple[str, ...]] + def _create_role_and_policies(role_model, role_policy_model, role_name: str, role_filters: dict = dict()): - resolved_role = LIBRARY_ROLES[role_name] + resolved_role: DefaultRole = LIBRARY_ROLES[role_name] role, is_new = role_model.objects.get_or_create( name=role_name, **role_filters, @@ -407,8 +412,7 @@ def _create_role_and_policies(role_model, role_policy_model, role_name: str, rol resource=resource, action=action ) - for policy in resolved_role["policies"] - for resource, actions in policy.items() + for resource, actions in resolved_role["policies"].items() for action in actions ] ) From 905537ba7b19a760c246d84a56a315c1b62913ad Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 9 Sep 2026 09:16:18 -0500 Subject: [PATCH 04/66] chore: add propper Jest links --- .../__tests__/__snapshots__/AppResourceDownload.test.tsx.snap | 2 +- .../__tests__/__snapshots__/AppResourceEditButton.test.tsx.snap | 2 +- .../__tests__/__snapshots__/AppResourceLoad.test.tsx.snap | 2 +- .../__tests__/__snapshots__/AppResourcesAside.test.tsx.snap | 2 +- .../__tests__/__snapshots__/AppResourcesFilters.test.tsx.snap | 2 +- .../__tests__/__snapshots__/AppResourcesTab.test.tsx.snap | 2 +- .../AppResources/__tests__/__snapshots__/Tabs.test.tsx.snap | 2 +- .../__snapshots__/buildAppResourceConformation.test.ts.snap | 2 +- .../__tests__/__snapshots__/useResourcesTree.test.ts.snap | 2 +- .../Atoms/__tests__/__snapshots__/Button.test.tsx.snap | 2 +- .../Atoms/__tests__/__snapshots__/DataEntry.test.ts.snap | 2 +- .../components/Atoms/__tests__/__snapshots__/Form.test.tsx.snap | 2 +- .../components/Atoms/__tests__/__snapshots__/Link.test.ts.snap | 2 +- .../Atoms/__tests__/__snapshots__/Submit.test.ts.snap | 2 +- .../Atoms/__tests__/__snapshots__/index.test.tsx.snap | 2 +- .../__tests__/__snapshots__/AttachmentCell.test.tsx.snap | 2 +- .../__tests__/__snapshots__/ChronoChart.test.tsx.snap | 2 +- .../__tests__/__snapshots__/UploadAttachment.test.tsx.snap | 2 +- .../Attachments/__tests__/__snapshots__/utils.test.ts.snap | 2 +- .../__tests__/__snapshots__/utils.test.ts.snap | 2 +- .../DataModel/__tests__/__snapshots__/specifyTable.test.ts.snap | 2 +- .../FieldFormatters/__tests__/__snapshots__/index.test.ts.snap | 2 +- .../FormEditor/__tests__/__snapshots__/createView.test.ts.snap | 2 +- .../__tests__/__snapshots__/fetchAllViews.test.tsx.snap | 2 +- .../FormEditor/__tests__/__snapshots__/index.test.ts.snap | 2 +- .../FormEditor/__tests__/__snapshots__/viewSpec.test.ts.snap | 2 +- .../FormMeta/__tests__/__snapshots__/CarryForward.test.ts.snap | 2 +- .../__tests__/__snapshots__/PartialDateUi.test.tsx.snap | 2 +- .../Formatters/__tests__/__snapshots__/formatters.test.ts.snap | 2 +- .../__tests__/__snapshots__/remotePrefs.test.ts.snap | 2 +- .../__tests__/__snapshots__/treeRanks.test.ts.snap | 2 +- .../Merging/__tests__/__snapshots__/autoMerge.test.ts.snap | 2 +- .../QueryBuilder/__tests__/__snapshots__/fromTree.test.ts.snap | 2 +- .../__tests__/__snapshots__/useTypeSearch.test.ts.snap | 2 +- .../WebLinks/__tests__/__snapshots__/index.test.ts.snap | 2 +- .../js_src/lib/utils/__tests__/__snapshots__/fonts.test.ts.snap | 2 +- 36 files changed, 36 insertions(+), 36 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourceDownload.test.tsx.snap b/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourceDownload.test.tsx.snap index 292c15a107e..46751898bc3 100644 --- a/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourceDownload.test.tsx.snap +++ b/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourceDownload.test.tsx.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`AppResourceDownload simple render 1`] = ` diff --git a/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourceEditButton.test.tsx.snap b/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourceEditButton.test.tsx.snap index 4ca187a9c14..501bcf85b47 100644 --- a/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourceEditButton.test.tsx.snap +++ b/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourceEditButton.test.tsx.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`AppResourceEditButton simple render 1`] = ` diff --git a/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourceLoad.test.tsx.snap b/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourceLoad.test.tsx.snap index 19648bbfc44..c1ef6e24b60 100644 --- a/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourceLoad.test.tsx.snap +++ b/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourceLoad.test.tsx.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`AppResourceLoad simple render 1`] = ` diff --git a/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourcesAside.test.tsx.snap b/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourcesAside.test.tsx.snap index 04acc08c687..ae2678bdc88 100644 --- a/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourcesAside.test.tsx.snap +++ b/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourcesAside.test.tsx.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`AppResourcesAside (expanded case) expanded case 1`] = ` diff --git a/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourcesFilters.test.tsx.snap b/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourcesFilters.test.tsx.snap index cb54620dd9f..c1e27f76c83 100644 --- a/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourcesFilters.test.tsx.snap +++ b/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourcesFilters.test.tsx.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`AppResourcesFilters simple render 1`] = ` diff --git a/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourcesTab.test.tsx.snap b/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourcesTab.test.tsx.snap index 2b59062a1df..ec02fe71b3a 100644 --- a/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourcesTab.test.tsx.snap +++ b/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourcesTab.test.tsx.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`AppResourcesTab dialog render 1`] = `
diff --git a/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/buildAppResourceConformation.test.ts.snap b/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/buildAppResourceConformation.test.ts.snap index e5f1b432813..cd1ef64c3e8 100644 --- a/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/buildAppResourceConformation.test.ts.snap +++ b/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/buildAppResourceConformation.test.ts.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`buildAppResourceConformation multi-level completely empty tree 1`] = ` [ diff --git a/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/useResourcesTree.test.ts.snap b/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/useResourcesTree.test.ts.snap index 22f837bf573..d0991deb267 100644 --- a/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/useResourcesTree.test.ts.snap +++ b/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/useResourcesTree.test.ts.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`useResourcesTree all appresource dir 1`] = ` [ diff --git a/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/Button.test.tsx.snap b/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/Button.test.tsx.snap index c0a3a8aed1d..e89d2dfdfec 100644 --- a/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/Button.test.tsx.snap +++ b/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/Button.test.tsx.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`Button.BorderedGray renders without errors 1`] = ` diff --git a/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/DataEntry.test.ts.snap b/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/DataEntry.test.ts.snap index 1d35add6612..2724a1b1932 100644 --- a/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/DataEntry.test.ts.snap +++ b/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/DataEntry.test.ts.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[` renders without errors 1`] = ` diff --git a/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/Form.test.tsx.snap b/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/Form.test.tsx.snap index f3058a02cbf..0f7e95583a1 100644 --- a/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/Form.test.tsx.snap +++ b/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/Form.test.tsx.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`
removes notSubmitted class name on submit 1`] = ` diff --git a/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/Link.test.ts.snap b/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/Link.test.ts.snap index b39a77c83d8..01cd1d18170 100644 --- a/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/Link.test.ts.snap +++ b/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/Link.test.ts.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`Link.BorderedGray renders without errors 1`] = ` diff --git a/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/Submit.test.ts.snap b/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/Submit.test.ts.snap index 4a36eb44633..38ac70e7b28 100644 --- a/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/Submit.test.ts.snap +++ b/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/Submit.test.ts.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`Submit.Danger renders without errors 1`] = ` diff --git a/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/index.test.tsx.snap b/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/index.test.tsx.snap index b8b75efd116..9aab22b6984 100644 --- a/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/index.test.tsx.snap +++ b/specifyweb/frontend/js_src/lib/components/Atoms/__tests__/__snapshots__/index.test.tsx.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`Can use as a controlled component 1`] = ` diff --git a/specifyweb/frontend/js_src/lib/components/Attachments/__tests__/__snapshots__/AttachmentCell.test.tsx.snap b/specifyweb/frontend/js_src/lib/components/Attachments/__tests__/__snapshots__/AttachmentCell.test.tsx.snap index 01111a2da05..39a988c47eb 100644 --- a/specifyweb/frontend/js_src/lib/components/Attachments/__tests__/__snapshots__/AttachmentCell.test.tsx.snap +++ b/specifyweb/frontend/js_src/lib/components/Attachments/__tests__/__snapshots__/AttachmentCell.test.tsx.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`AttachmentCell simple render 1`] = ` diff --git a/specifyweb/frontend/js_src/lib/components/Attachments/__tests__/__snapshots__/ChronoChart.test.tsx.snap b/specifyweb/frontend/js_src/lib/components/Attachments/__tests__/__snapshots__/ChronoChart.test.tsx.snap index a3c20bb31a1..c3e8ec312b6 100644 --- a/specifyweb/frontend/js_src/lib/components/Attachments/__tests__/__snapshots__/ChronoChart.test.tsx.snap +++ b/specifyweb/frontend/js_src/lib/components/Attachments/__tests__/__snapshots__/ChronoChart.test.tsx.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`ChronoChart dialog open and close 1`] = `
diff --git a/specifyweb/frontend/js_src/lib/components/Attachments/__tests__/__snapshots__/utils.test.ts.snap b/specifyweb/frontend/js_src/lib/components/Attachments/__tests__/__snapshots__/utils.test.ts.snap index a60a2eb0d03..206c6fdda2c 100644 --- a/specifyweb/frontend/js_src/lib/components/Attachments/__tests__/__snapshots__/utils.test.ts.snap +++ b/specifyweb/frontend/js_src/lib/components/Attachments/__tests__/__snapshots__/utils.test.ts.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`allTablesWithAttachments 1`] = ` [ diff --git a/specifyweb/frontend/js_src/lib/components/AttachmentsBulkImport/__tests__/__snapshots__/utils.test.ts.snap b/specifyweb/frontend/js_src/lib/components/AttachmentsBulkImport/__tests__/__snapshots__/utils.test.ts.snap index 6d12772ce10..2a4a271fe67 100644 --- a/specifyweb/frontend/js_src/lib/components/AttachmentsBulkImport/__tests__/__snapshots__/utils.test.ts.snap +++ b/specifyweb/frontend/js_src/lib/components/AttachmentsBulkImport/__tests__/__snapshots__/utils.test.ts.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`reconstruct deleting attachment spec 1`] = ` [ diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/__snapshots__/specifyTable.test.ts.snap b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/__snapshots__/specifyTable.test.ts.snap index fd73e1fe5c9..4425fd98366 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/__snapshots__/specifyTable.test.ts.snap +++ b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/__snapshots__/specifyTable.test.ts.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`fields are loaded 1`] = ` [ diff --git a/specifyweb/frontend/js_src/lib/components/FieldFormatters/__tests__/__snapshots__/index.test.ts.snap b/specifyweb/frontend/js_src/lib/components/FieldFormatters/__tests__/__snapshots__/index.test.ts.snap index c823d7dba47..13662ae86ec 100644 --- a/specifyweb/frontend/js_src/lib/components/FieldFormatters/__tests__/__snapshots__/index.test.ts.snap +++ b/specifyweb/frontend/js_src/lib/components/FieldFormatters/__tests__/__snapshots__/index.test.ts.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`field formatters are fetched and parsed correctly 1`] = ` { diff --git a/specifyweb/frontend/js_src/lib/components/FormEditor/__tests__/__snapshots__/createView.test.ts.snap b/specifyweb/frontend/js_src/lib/components/FormEditor/__tests__/__snapshots__/createView.test.ts.snap index 4ced007cfc9..d91863bbc5f 100644 --- a/specifyweb/frontend/js_src/lib/components/FormEditor/__tests__/__snapshots__/createView.test.ts.snap +++ b/specifyweb/frontend/js_src/lib/components/FormEditor/__tests__/__snapshots__/createView.test.ts.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`Tables with form tables computed correctly 1`] = ` [ diff --git a/specifyweb/frontend/js_src/lib/components/FormEditor/__tests__/__snapshots__/fetchAllViews.test.tsx.snap b/specifyweb/frontend/js_src/lib/components/FormEditor/__tests__/__snapshots__/fetchAllViews.test.tsx.snap index ab1ce534d9b..98208a49a69 100644 --- a/specifyweb/frontend/js_src/lib/components/FormEditor/__tests__/__snapshots__/fetchAllViews.test.tsx.snap +++ b/specifyweb/frontend/js_src/lib/components/FormEditor/__tests__/__snapshots__/fetchAllViews.test.tsx.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`fetchAllViews 1`] = ` { diff --git a/specifyweb/frontend/js_src/lib/components/FormEditor/__tests__/__snapshots__/index.test.ts.snap b/specifyweb/frontend/js_src/lib/components/FormEditor/__tests__/__snapshots__/index.test.ts.snap index 1e7ca28896a..2ebe6233144 100644 --- a/specifyweb/frontend/js_src/lib/components/FormEditor/__tests__/__snapshots__/index.test.ts.snap +++ b/specifyweb/frontend/js_src/lib/components/FormEditor/__tests__/__snapshots__/index.test.ts.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`Can edit a form definition 1`] = ` { diff --git a/specifyweb/frontend/js_src/lib/components/FormEditor/__tests__/__snapshots__/viewSpec.test.ts.snap b/specifyweb/frontend/js_src/lib/components/FormEditor/__tests__/__snapshots__/viewSpec.test.ts.snap index 02fdefd49aa..56159e20d5c 100644 --- a/specifyweb/frontend/js_src/lib/components/FormEditor/__tests__/__snapshots__/viewSpec.test.ts.snap +++ b/specifyweb/frontend/js_src/lib/components/FormEditor/__tests__/__snapshots__/viewSpec.test.ts.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`Can edit form definition 1`] = ` " diff --git a/specifyweb/frontend/js_src/lib/components/FormMeta/__tests__/__snapshots__/CarryForward.test.ts.snap b/specifyweb/frontend/js_src/lib/components/FormMeta/__tests__/__snapshots__/CarryForward.test.ts.snap index 5e4e6f21c14..ee37d431ac5 100644 --- a/specifyweb/frontend/js_src/lib/components/FormMeta/__tests__/__snapshots__/CarryForward.test.ts.snap +++ b/specifyweb/frontend/js_src/lib/components/FormMeta/__tests__/__snapshots__/CarryForward.test.ts.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`dependentFields 1`] = ` { diff --git a/specifyweb/frontend/js_src/lib/components/FormPlugins/__tests__/__snapshots__/PartialDateUi.test.tsx.snap b/specifyweb/frontend/js_src/lib/components/FormPlugins/__tests__/__snapshots__/PartialDateUi.test.tsx.snap index d2930538113..fb4094c85f1 100644 --- a/specifyweb/frontend/js_src/lib/components/FormPlugins/__tests__/__snapshots__/PartialDateUi.test.tsx.snap +++ b/specifyweb/frontend/js_src/lib/components/FormPlugins/__tests__/__snapshots__/PartialDateUi.test.tsx.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`PartialDateUi renders without errors 1`] = ` diff --git a/specifyweb/frontend/js_src/lib/components/Formatters/__tests__/__snapshots__/formatters.test.ts.snap b/specifyweb/frontend/js_src/lib/components/Formatters/__tests__/__snapshots__/formatters.test.ts.snap index 33999118207..dea1513e2c1 100644 --- a/specifyweb/frontend/js_src/lib/components/Formatters/__tests__/__snapshots__/formatters.test.ts.snap +++ b/specifyweb/frontend/js_src/lib/components/Formatters/__tests__/__snapshots__/formatters.test.ts.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`Formatters are fetched and parsed correctly 1`] = ` { diff --git a/specifyweb/frontend/js_src/lib/components/InitialContext/__tests__/__snapshots__/remotePrefs.test.ts.snap b/specifyweb/frontend/js_src/lib/components/InitialContext/__tests__/__snapshots__/remotePrefs.test.ts.snap index 8e56db1c7c4..a145bc62b30 100644 --- a/specifyweb/frontend/js_src/lib/components/InitialContext/__tests__/__snapshots__/remotePrefs.test.ts.snap +++ b/specifyweb/frontend/js_src/lib/components/InitialContext/__tests__/__snapshots__/remotePrefs.test.ts.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`fetches and parses remotePrefs correctly 1`] = ` { diff --git a/specifyweb/frontend/js_src/lib/components/InitialContext/__tests__/__snapshots__/treeRanks.test.ts.snap b/specifyweb/frontend/js_src/lib/components/InitialContext/__tests__/__snapshots__/treeRanks.test.ts.snap index def76761ae1..0088f4403af 100644 --- a/specifyweb/frontend/js_src/lib/components/InitialContext/__tests__/__snapshots__/treeRanks.test.ts.snap +++ b/specifyweb/frontend/js_src/lib/components/InitialContext/__tests__/__snapshots__/treeRanks.test.ts.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`Get tree definition for the Geography tree 1`] = ` [ diff --git a/specifyweb/frontend/js_src/lib/components/Merging/__tests__/__snapshots__/autoMerge.test.ts.snap b/specifyweb/frontend/js_src/lib/components/Merging/__tests__/__snapshots__/autoMerge.test.ts.snap index 6c6d6979017..f14891e48d1 100644 --- a/specifyweb/frontend/js_src/lib/components/Merging/__tests__/__snapshots__/autoMerge.test.ts.snap +++ b/specifyweb/frontend/js_src/lib/components/Merging/__tests__/__snapshots__/autoMerge.test.ts.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`autoMerge cautious 1`] = ` { diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/__snapshots__/fromTree.test.ts.snap b/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/__snapshots__/fromTree.test.ts.snap index ce5558cd8c2..04dc1e16698 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/__snapshots__/fromTree.test.ts.snap +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/__tests__/__snapshots__/fromTree.test.ts.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`queryFromTree 1`] = ` [ diff --git a/specifyweb/frontend/js_src/lib/components/QueryComboBox/__tests__/__snapshots__/useTypeSearch.test.ts.snap b/specifyweb/frontend/js_src/lib/components/QueryComboBox/__tests__/__snapshots__/useTypeSearch.test.ts.snap index 1aaa7712679..075e54a269e 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryComboBox/__tests__/__snapshots__/useTypeSearch.test.ts.snap +++ b/specifyweb/frontend/js_src/lib/components/QueryComboBox/__tests__/__snapshots__/useTypeSearch.test.ts.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`type searches are fetched and parsed correctly 1`] = ` [ diff --git a/specifyweb/frontend/js_src/lib/components/WebLinks/__tests__/__snapshots__/index.test.ts.snap b/specifyweb/frontend/js_src/lib/components/WebLinks/__tests__/__snapshots__/index.test.ts.snap index 75ae2c01278..4a044a71656 100644 --- a/specifyweb/frontend/js_src/lib/components/WebLinks/__tests__/__snapshots__/index.test.ts.snap +++ b/specifyweb/frontend/js_src/lib/components/WebLinks/__tests__/__snapshots__/index.test.ts.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`weblinks are fetched and parsed correctly 1`] = ` { diff --git a/specifyweb/frontend/js_src/lib/utils/__tests__/__snapshots__/fonts.test.ts.snap b/specifyweb/frontend/js_src/lib/utils/__tests__/__snapshots__/fonts.test.ts.snap index a0f85639adf..eb8cb86fba0 100644 --- a/specifyweb/frontend/js_src/lib/utils/__tests__/__snapshots__/fonts.test.ts.snap +++ b/specifyweb/frontend/js_src/lib/utils/__tests__/__snapshots__/fonts.test.ts.snap @@ -1,4 +1,4 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP +// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing exports[`Unavailable fonts are filterd out 1`] = ` [ From 3c6eea450a0d64acde19574d66f0bdd5ed4731c1 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 9 Sep 2026 11:47:17 -0500 Subject: [PATCH 05/66] feat: generalize QueryBuilder result hook --- .../{QueryBuilder => FormSliders}/hooks.tsx | 105 ++++++++++++------ .../lib/components/QueryBuilder/Results.tsx | 4 +- .../lib/components/SpecifyNetwork/Map.tsx | 4 +- specifyweb/frontend/js_src/lib/utils/utils.ts | 2 +- specifyweb/specify/api/crud.py | 3 - specifyweb/specify/models.py | 1 - 6 files changed, 76 insertions(+), 43 deletions(-) rename specifyweb/frontend/js_src/lib/components/{QueryBuilder => FormSliders}/hooks.tsx (58%) diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/hooks.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/hooks.tsx similarity index 58% rename from specifyweb/frontend/js_src/lib/components/QueryBuilder/hooks.tsx rename to specifyweb/frontend/js_src/lib/components/FormSliders/hooks.tsx index 5877a45cc6b..b5be6940a3b 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/hooks.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/hooks.tsx @@ -1,25 +1,70 @@ import React from 'react'; - import { useTriggerState } from '../../hooks/useTriggerState'; -import type { GetOrSet, IR, R, RA } from '../../utils/types'; +import { GetOrSet, R, RA } from '../../utils/types'; import { removeKey } from '../../utils/utils'; +import { DEFAULT_FETCH_LIMIT } from '../DataModel/collection'; import { raise, softFail } from '../Errors/Crash'; -import type { QueryResultRow, QueryResultsProps } from './Results'; -export function useFetchQueryResults({ +/** + * Provides a way to somewhat lazily paginate through an arbitary set of + * records. + * Pseudocode Example: + * ```ts + * // Say we have some record IDs that can be in some set of arbitary size, and + * // we want to handle pagination within that set. + * const initialIds = [1, 2, 3]; + * const totalCount = 200; + * // The "fetch" function fetches a page of records at some offset up to some + * // limit + * const fetchMoreIds = (offset: number) => fetch("url", {offset, limit: 20}); + * const { + * results: [results, setResults], + * onFetchMore: handleFetchMore, + * totalCount: [totalCount, setTotalCount], + * canFetchMore + * } = usePaginatedRecords({ + * initialData: initialIds, + * totalCount, + * fetchSize: 3, + * fetchResults: fetchMoreIds + * }); + * + * // We can call handleFetchMore to automatically grab the next fetchSize + * // records + * const fetchedNext = await handleFetchMore(); + * console.log(fetchedNext); // [4, 5, 6] + * // Now results will have the next fetchSize results + * console.log(results); // [1, 2, 3, 4, 5, 6] + * // We can pass in a specific index to fetch only from that index + * const fetchFromFar = await handleFetchMore(100); + * console.log(fetchFromFar); // [99, 100, 101] + * // Note that the results array can be sparse, with holes at the indexes + * // where results have not been fetched + * console.log(results); + * // [1, 2, 3, 4, 5, 6, , 99, 100, 101] + * ``` + */ +export function usePaginatedRecords< + PAGINATED_TYPE, + FETCH_ARGS extends RA, +>({ initialData, - fetchResults, totalCount: initialTotalCount, - fetchSize, -}: Pick< - QueryResultsProps, - 'fetchResults' | 'fetchSize' | 'initialData' | 'totalCount' ->): { - readonly results: GetOrSet | undefined>; - readonly fetchersRef: { - readonly current: IR | void>>; - }; - readonly onFetchMore: (index?: number) => Promise | void>; + fetchSize = DEFAULT_FETCH_LIMIT, + fetchResults, +}: { + readonly initialData: RA | undefined; + readonly totalCount: number | undefined; + readonly fetchSize?: number; + readonly fetchResults: + | ((offset: number, ...args: FETCH_ARGS) => Promise>) + | undefined; +}): { + readonly results: GetOrSet | undefined>; + readonly onFetchMore: ( + index?: number, + ...args: FETCH_ARGS + ) => Promise | void>; readonly totalCount: GetOrSet; readonly canFetchMore: boolean; } { @@ -30,34 +75,27 @@ export function useFetchQueryResults({ * hundreds of thousands of results. */ const getSetResults = useTriggerState< - RA | undefined + RA | undefined >(initialData); const [results, setResults] = getSetResults; const resultsRef = React.useRef(results); - const handleSetResults: GetOrSet< - RA | undefined - >[1] = React.useCallback( - (results) => { - const resolved = - typeof results === 'function' ? results(resultsRef.current) : results; - setResults(resolved); - resultsRef.current = resolved; - }, - [setResults] - ); // Queue for fetching - const fetchersRef = React.useRef | void>>>({}); + const fetchersRef = React.useRef | void>>>({}); const getSetTotalCount = useTriggerState(initialTotalCount); const [totalCount] = getSetTotalCount; + const canFetchMore = !Array.isArray(results) || totalCount === undefined || results.length < totalCount; const handleFetchMore = React.useCallback( - async (index?: number): Promise | void> => { + async ( + index?: number, + ...args: FETCH_ARGS + ): Promise | void> => { const currentResults = resultsRef.current; const canFetch = Array.isArray(currentResults); @@ -87,7 +125,7 @@ export function useFetchQueryResults({ : naiveFetchIndex; // Prevent concurrent fetching in different places - fetchersRef.current[fetchIndex] ??= fetchResults(fetchIndex) + fetchersRef.current[fetchIndex] ??= fetchResults(fetchIndex, ...args) .then(async (newResults) => { if ( process.env.NODE_ENV === 'development' && @@ -111,7 +149,7 @@ export function useFetchQueryResults({ combinedResults[fetchIndex] ??= undefined; combinedResults.splice(fetchIndex, newResults.length, ...newResults); - handleSetResults(combinedResults); + setResults(combinedResults); fetchersRef.current = removeKey( fetchersRef.current, @@ -119,7 +157,7 @@ export function useFetchQueryResults({ ); if (typeof index === 'number' && index >= combinedResults.length) - return handleFetchMore(index); + return handleFetchMore(index, ...args); return newResults; }) .catch(raise); @@ -130,8 +168,7 @@ export function useFetchQueryResults({ ); return { - fetchersRef, - results: [results, handleSetResults], + results: [results, setResults], onFetchMore: handleFetchMore, totalCount: getSetTotalCount, canFetchMore, diff --git a/specifyweb/frontend/js_src/lib/components/QueryBuilder/Results.tsx b/specifyweb/frontend/js_src/lib/components/QueryBuilder/Results.tsx index b9153b0697d..cf6eeb3c5e9 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryBuilder/Results.tsx +++ b/specifyweb/frontend/js_src/lib/components/QueryBuilder/Results.tsx @@ -13,6 +13,7 @@ import type { SpecifyResource } from '../DataModel/legacyTypes'; import { schema } from '../DataModel/schema'; import type { SpecifyTable } from '../DataModel/specifyTable'; import type { SpQuery } from '../DataModel/types'; +import { usePaginatedRecords } from '../FormSliders/hooks'; import { treeRanksPromise } from '../InitialContext/treeRanks'; import { RecordMergingLink } from '../Merging'; import { loadingGif } from '../Molecules'; @@ -31,7 +32,6 @@ import { CreateRecordSet } from './CreateRecordSet'; import type { QueryFieldSpec } from './fieldSpec'; import type { QueryField } from './helpers'; import { sortTypes } from './helpers'; -import { useFetchQueryResults } from './hooks'; import { QueryResultsTable } from './ResultsTable'; import { QueryToForms } from './ToForms'; import { QueryToMap } from './ToMap'; @@ -99,7 +99,7 @@ export function QueryResults(props: QueryResultsProps): JSX.Element { onFetchMore: handleFetchMore, totalCount: [totalCount, setTotalCount], canFetchMore, - } = useFetchQueryResults(props); + } = usePaginatedRecords(props); const canMergeTable = canMerge(table); diff --git a/specifyweb/frontend/js_src/lib/components/SpecifyNetwork/Map.tsx b/specifyweb/frontend/js_src/lib/components/SpecifyNetwork/Map.tsx index 5dc4454f599..4217eb75e6f 100644 --- a/specifyweb/frontend/js_src/lib/components/SpecifyNetwork/Map.tsx +++ b/specifyweb/frontend/js_src/lib/components/SpecifyNetwork/Map.tsx @@ -13,12 +13,12 @@ import type { SerializedResource } from '../DataModel/helperTypes'; import type { SpecifyResource } from '../DataModel/legacyTypes'; import { genericTables, getTableById, tables } from '../DataModel/tables'; import type { SpQuery, Tables } from '../DataModel/types'; +import { usePaginatedRecords } from '../FormSliders/hooks'; import type { LeafletInstance } from '../Leaflet/addOns'; import { LoadingScreen } from '../Molecules/Dialog'; import { queryFromTree } from '../QueryBuilder/fromTree'; import type { QueryField } from '../QueryBuilder/helpers'; import { parseQueryFields } from '../QueryBuilder/helpers'; -import { useFetchQueryResults } from '../QueryBuilder/hooks'; import type { QueryResultRow } from '../QueryBuilder/Results'; import { useQueryResultsWrapper } from '../QueryBuilder/ResultsWrapper'; import { @@ -144,7 +144,7 @@ function Map({ results: [results], canFetchMore, onFetchMore: handleFetchMore, - } = useFetchQueryResults(props); + } = usePaginatedRecords(props); const undefinedResult = results?.indexOf(undefined); const loadedResults = ( diff --git a/specifyweb/frontend/js_src/lib/utils/utils.ts b/specifyweb/frontend/js_src/lib/utils/utils.ts index 39f1f86830a..437d3a6d924 100644 --- a/specifyweb/frontend/js_src/lib/utils/utils.ts +++ b/specifyweb/frontend/js_src/lib/utils/utils.ts @@ -217,7 +217,7 @@ export const split = ( * Convert an array of [key,value] tuples to a RA<[key, RA]> * * @remarks - * KEY doesn't have to be a string. It can be of any time + * KEY doesn't have to be a string. It can be of any type */ export const group = ( entries: RA diff --git a/specifyweb/specify/api/crud.py b/specifyweb/specify/api/crud.py index a20cdc0abe2..086a44b5e15 100644 --- a/specifyweb/specify/api/crud.py +++ b/specifyweb/specify/api/crud.py @@ -1,6 +1,3 @@ - - - import logging from typing import Any, Dict from collections.abc import Callable diff --git a/specifyweb/specify/models.py b/specifyweb/specify/models.py index 075ff8c174f..c8da16cd41f 100644 --- a/specifyweb/specify/models.py +++ b/specifyweb/specify/models.py @@ -1,6 +1,5 @@ from functools import partialmethod from django.db import models -from django.db.models import Q, CheckConstraint from django.utils import timezone from specifyweb.backend.businessrules.exceptions import AbortSave from specifyweb.specify.models_utils.model_timestamp import save_auto_timestamp_field_with_override From 6357f669c362e1a5f6dbcf2e4e6fb1f3135f44b3 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 9 Sep 2026 12:01:55 -0500 Subject: [PATCH 06/66] chore: fix offset in hook comment --- .../frontend/js_src/lib/components/FormSliders/hooks.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/hooks.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/hooks.tsx index b5be6940a3b..e6516f0cd4c 100644 --- a/specifyweb/frontend/js_src/lib/components/FormSliders/hooks.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/hooks.tsx @@ -36,12 +36,12 @@ import { raise, softFail } from '../Errors/Crash'; * // Now results will have the next fetchSize results * console.log(results); // [1, 2, 3, 4, 5, 6] * // We can pass in a specific index to fetch only from that index - * const fetchFromFar = await handleFetchMore(100); - * console.log(fetchFromFar); // [99, 100, 101] + * const fetchFromFar = await handleFetchMore(99); + * console.log(fetchFromFar); // [100, 101, 102] * // Note that the results array can be sparse, with holes at the indexes * // where results have not been fetched * console.log(results); - * // [1, 2, 3, 4, 5, 6, , 99, 100, 101] + * // [1, 2, 3, 4, 5, 6, , 100, 101, 102] * ``` */ export function usePaginatedRecords< From 3eba926f329c83008eae14d12955bf8a26cca61d Mon Sep 17 00:00:00 2001 From: melton-jason Date: Fri, 2 Oct 2026 19:51:24 -0500 Subject: [PATCH 07/66] feat: keyset pagination and fast tree blockers --- specifyweb/backend/cache/redis/store.py | 4 +- specifyweb/backend/delete_blockers/filter.py | 179 ++++++++++++++++++ .../backend/delete_blockers/paginate.py | 107 +++++++++++ specifyweb/backend/delete_blockers/urls.py | 1 + specifyweb/backend/delete_blockers/utils.py | 106 +++++++++++ .../backend/delete_blockers/validators.py | 116 ++++++++++++ specifyweb/backend/delete_blockers/views.py | 91 ++++----- specifyweb/backend/table_rows/forms.py | 16 -- specifyweb/backend/trees/extras.py | 6 +- specifyweb/backend/workbench/upload/clone.py | 2 +- .../backend/workbench/upload/upload_table.py | 11 +- specifyweb/specify/api/crud.py | 2 + specifyweb/specify/models.py | 33 ++-- .../specify/models_utils/model_extras.py | 57 +----- .../specify/models_utils/tree_model_extras.py | 51 +++++ 15 files changed, 633 insertions(+), 149 deletions(-) create mode 100644 specifyweb/backend/delete_blockers/filter.py create mode 100644 specifyweb/backend/delete_blockers/paginate.py create mode 100644 specifyweb/backend/delete_blockers/utils.py create mode 100644 specifyweb/backend/delete_blockers/validators.py delete mode 100644 specifyweb/backend/table_rows/forms.py create mode 100644 specifyweb/specify/models_utils/tree_model_extras.py diff --git a/specifyweb/backend/cache/redis/store.py b/specifyweb/backend/cache/redis/store.py index 1e9576b2303..c5ef92ead37 100644 --- a/specifyweb/backend/cache/redis/store.py +++ b/specifyweb/backend/cache/redis/store.py @@ -10,11 +10,11 @@ def set_bytes(key: str | bytes, value: bytes, time_to_live=None, override_existi return _set_string(format_key(key), value, time_to_live=time_to_live, override_existing=override_existing, decode_responses=False) -def get_string(key: str | bytes, delete_key=False) -> str: +def get_string(key: str | bytes, delete_key=False) -> str | None: return _get_string(format_key(key), delete_key=delete_key, decode_responses=True) -def get_bytes(key: str | bytes, delete_key=False) -> bytes: +def get_bytes(key: str | bytes, delete_key=False) -> bytes | None: return _get_string(format_key(key), delete_key=delete_key, decode_responses=False) diff --git a/specifyweb/backend/delete_blockers/filter.py b/specifyweb/backend/delete_blockers/filter.py new file mode 100644 index 00000000000..01567e61c22 --- /dev/null +++ b/specifyweb/backend/delete_blockers/filter.py @@ -0,0 +1,179 @@ +from typing import TypedDict +from functools import reduce + +from django.db.models import Model + +from specifyweb.specify.api.crud import get_model +from specifyweb.specify.models_utils.load_datamodel import TableDoesNotExistError + +from specifyweb.backend.delete_blockers.validators import DELETE_BLOCKER_LIMIT, CleanedDeleteBlockerFilter, CleanedDeleteBlockerRequestForm +from specifyweb.backend.delete_blockers.utils import field_is_remote, relationship_blocks_deletion, relationship_cascades_delete, blocker_relationships_for_obj +from specifyweb.backend.delete_blockers.paginate import resolve_reference_paginator, ReferencePaginator + +class DeleteBlocker(TypedDict): + table: str + field: str + ids: list[int] + limit: int + complete: bool + +class DeleteBlockerCount(TypedDict): + table: str + field: str + count: int + +class DeleteBlockerCounts(TypedDict): + results: list[DeleteBlockerCount] + total_count: int + +class DeleteBlockerFilter: + def __init__(self, paginator: ReferencePaginator) -> None: + self.paginator = paginator + + @property + def obj(self): + return self.paginator.obj + + @property + def relationship(self): + return self.paginator.relationship + + @property + def table_name(self): + table_name = self.relationship.related_model._meta.model_name + if table_name is None: + raise ValueError(f"Unable to get related table name for {self.relationship}") + return table_name + + def cascades(self): + return relationship_cascades_delete(self.relationship) + + @classmethod + def from_json(cls, obj: Model, json: CleanedDeleteBlockerFilter): + table_name = json["table"] + model = get_model(table_name) + if model is None: + raise TableDoesNotExistError(f"Unable to find {table_name}") + field_name = json["field"] + field = model._meta.get_field(field_name) + if (not field_is_remote(field) + or not (relationship_blocks_deletion(field) or relationship_cascades_delete(field))): + raise ValueError(f"Can not use field {table_name}.{field_name} for delete blockers") + Paginator = resolve_reference_paginator(obj) + return cls( + paginator=Paginator( + obj=obj, + relationship=field, + limit=json["limit"], + anchor_id=json["anchor"], + backwards=json["backwards"] + ) + ) + + def count(self) -> None | DeleteBlockerCount: + # We're only collecting counts for direct blockers + if not relationship_blocks_deletion(self.relationship): + return None + + record_count = self.paginator.count() + return { + "table": self.table_name, + "field": self.relationship.field.name, + "count": record_count + } + + def fetch_page(self) -> tuple[bool, DeleteBlocker]: + fetched_ids = self.paginator.fetch_page() + complete = len(fetched_ids) < self.paginator.limit + cascades = self.cascades() + return cascades, { + "table": self.table_name, + "field": self.relationship.field.name, + "ids": fetched_ids, + "limit": self.paginator.limit, + "complete": complete + } + +def default_filters(obj: Model, limit=DELETE_BLOCKER_LIMIT, count_only=False) -> list[DeleteBlockerFilter]: + filters = [] + protect_rels, cascade_rels = blocker_relationships_for_obj(obj) + Paginator = resolve_reference_paginator(obj) + for rel in protect_rels: + filters.append( + DeleteBlockerFilter( + Paginator( + obj, + relationship=rel, + limit=limit + ) + ) + ) + # REFACTOR: clean this up a little + if not count_only: + for rel in cascade_rels: + filters.append( + DeleteBlockerFilter( + Paginator( + obj, + relationship=rel, + limit=limit + ) + ) + ) + return filters + +def merge_delete_blocker_counts(aggregated: DeleteBlockerCounts, current_count: DeleteBlockerCount | None) -> DeleteBlockerCounts: + if current_count is None or current_count["count"] == 0: + return aggregated + new_results = [*aggregated["results"], current_count] + new_count = aggregated["total_count"] + current_count["count"] + return { + "results": new_results, + "total_count": new_count + } + + +class DeleteBlockerFilters: + def __init__(self, + obj: Model, + limit=DELETE_BLOCKER_LIMIT, + filters: list[DeleteBlockerFilter] | None = None, + count_only = False + ) -> None: + self.obj = obj + if filters is None: + self.filters = default_filters(obj, limit, count_only) + else: + self.filters = filters + + @classmethod + def from_json(cls, obj: Model, json: CleanedDeleteBlockerRequestForm, count_only=False): + if json["filters"] is None: + filters = None + else: + filters = [DeleteBlockerFilter.from_json(obj=obj, json=blocker_filter) for blocker_filter in json["filters"]] + limit = json["limit"] + return cls(obj=obj, limit=limit, filters=filters, count_only=count_only) + + def count(self) -> DeleteBlockerCounts: + starting_value: DeleteBlockerCounts = { + "results": [], + "total_count": 0 + } + return reduce( + merge_delete_blocker_counts, + (blocker.count() for blocker in self.filters), + starting_value + ) + + def fetch(self) -> tuple[list[DeleteBlocker], list[DeleteBlocker]]: + blocked, deferred = [], [] + for blocker_filter in self.filters: + blocker_deferred, page = blocker_filter.fetch_page() + if len(page["ids"]) == 0: + continue + if blocker_deferred: + deferred.append(page) + else: + blocked.append(page) + return blocked, deferred diff --git a/specifyweb/backend/delete_blockers/paginate.py b/specifyweb/backend/delete_blockers/paginate.py new file mode 100644 index 00000000000..3d74d6d6f39 --- /dev/null +++ b/specifyweb/backend/delete_blockers/paginate.py @@ -0,0 +1,107 @@ +from django.db.models import Model, ForeignObjectRel, ForeignKey + +from specifyweb.backend.trees.extras import Tree +from specifyweb.backend.stored_queries.utils import log_sqlalchemy_query +from specifyweb.backend.delete_blockers.validators import DELETE_BLOCKER_LIMIT +from specifyweb.backend.delete_blockers.utils import relationship_cascades_delete, node_numbers_valid_for_tree + +class ReferencePaginator: + def __init__(self, + obj: Model, + relationship: ForeignObjectRel, + limit: int = DELETE_BLOCKER_LIMIT, + anchor_id: int | None = None, + backwards: bool = False): + self.obj = obj + self.relationship = relationship + self.limit = limit + self.anchor_id = anchor_id + self.backwards = backwards + + def count(self) -> int: + queryset = self._get_base_queryset() + return queryset.count() + + def fetch_page(self) -> list[int]: + queryset = self._get_page_queryset() + return list(queryset) + + def _get_page_queryset(self): + queryset = self._get_base_queryset() + + if self.anchor_id is not None: + direction = "lt" if self.backwards else "gt" + queryset = queryset.filter(**{f"pk__{direction}": self.anchor_id}) + + if self.backwards: + queryset = queryset.order_by("-pk") + else: + queryset = queryset.order_by("pk") + + queryset = queryset.values_list("pk", flat=True) + + if self.limit != 0: + queryset = queryset[:self.limit] + return queryset + + def _get_base_queryset(self): + other_side_model = self.relationship.related_model + return other_side_model.objects.filter( + **{self.relationship.field.name: self.obj.pk} + ) + +class TreeTableReferencePaginator(ReferencePaginator): + """ + This paginator should be used for objects that are Tree Tables + (Taxon, Geography, Storage, Lithostrat, etc.). + WARNING: This strictly assumes: + + 1. All NodeNumbers for the tree are valid and up-to-date + 2. The ON_DELETE behavior for the 'parent' FK is set to CASCADE + 3. The NodeNumber column on the tree is indexed + + Use the base ReferencePaginator if you can not guarantee all of the above + properties. + """ + def _get_base_queryset(self): + node_number = self.obj.nodenumber + highest_node_number = self.obj.highestchildnodenumber + other_side_model = self.relationship.related_model + # With the assumption that nodenumbers are up-to-date and correctly + # formed for the tree, we can use their properties to optimize fetching + # references. + # Particularly, we know the node and all of its descendants lie within + # the range >= NodeNumber and <= HighestChildNodeNumber. + # Essentially node numbers can "flatten" any subtree within the + # tree so we don't have to worry about expensive or complicated + # Parent self-joins to traverse the subtree structure + node_number_filter = { + f"{self.relationship.field.name}__nodenumber__range": (node_number, highest_node_number) + } + return other_side_model.objects.filter(**node_number_filter) + +def resolve_reference_paginator(obj: Model) -> type[ReferencePaginator]: + if isinstance(obj, Tree): + return _resolve_tree_paginator(obj) + return ReferencePaginator + +def _resolve_tree_paginator(obj: Tree) -> type[ReferencePaginator]: + # This is first because it is far cheaper than checking for NodeNumber validity + if not _tree_cascades_delete(obj.__class__): + return ReferencePaginator + definition_id = obj.definition_id + node_numbers_valid = node_numbers_valid_for_tree(obj._meta.db_table, definition_id) + if node_numbers_valid: + return TreeTableReferencePaginator + return ReferencePaginator + +def _tree_cascades_delete(obj: type[Tree]): + parent_field = obj._meta.get_field("parent") + if not isinstance(parent_field, ForeignKey): + return False + # The ON_DELETE behavior is on the other side of the FK + other_side = parent_field.remote_field + if not isinstance(other_side, ForeignObjectRel): + return False + + return relationship_cascades_delete(other_side) diff --git a/specifyweb/backend/delete_blockers/urls.py b/specifyweb/backend/delete_blockers/urls.py index d141bb9f968..da4d564d2ef 100644 --- a/specifyweb/backend/delete_blockers/urls.py +++ b/specifyweb/backend/delete_blockers/urls.py @@ -3,6 +3,7 @@ from . import views urlpatterns = [ re_path(r'^delete_blockers/(?P\w+)/(?P\d+)/$', views.delete_blockers), + re_path(r'^count/(?P\w+)/(?P\d+)/$', views.count_blockers), re_path(r'^old_delete_blockers/(?P\w+)/(?P\d+)/$', views.old_delete_blockers), ] diff --git a/specifyweb/backend/delete_blockers/utils.py b/specifyweb/backend/delete_blockers/utils.py new file mode 100644 index 00000000000..f532266b1c7 --- /dev/null +++ b/specifyweb/backend/delete_blockers/utils.py @@ -0,0 +1,106 @@ +from typing_extensions import TypeIs + +from django.db.models import Field, ForeignObjectRel +from django.db.models.deletion import PROTECT, CASCADE +from sqlalchemy import select, and_, or_, not_, union_all +from sqlalchemy.orm import aliased + +from specifyweb.specify.models_utils.model_extras import ModelWithTable +from specifyweb.specify.models import protect_with_blockers +from specifyweb.specify.api.crud import strict_get_model +from specifyweb.backend.stored_queries import models as sqlmodels + +def field_is_remote(field: Field | ForeignObjectRel) -> TypeIs[ForeignObjectRel]: + # TODO: Check whether there are any concrete fields that SHOULD be + # included here, like some ToOne fields that acts as blockers + return field.is_relation and not getattr(field, "concrete", True) + +def relationship_blocks_deletion(relationship: ForeignObjectRel): + return relationship.on_delete is protect_with_blockers or relationship.on_delete is PROTECT + +def relationship_cascades_delete(relationship: ForeignObjectRel): + return relationship.on_delete is CASCADE + +def blocker_relationships_for_obj(obj: ModelWithTable) -> tuple[list[ForeignObjectRel], list[ForeignObjectRel]]: + protect, cascade = [], [] + all_fields = obj._meta.get_fields(include_hidden=True) + remote_relationships = filter( + field_is_remote, + all_fields + ) + for relationship in remote_relationships: + if relationship_blocks_deletion(relationship): + protect.append(relationship) + elif relationship_cascades_delete(relationship): + cascade.append(relationship) + + return protect, cascade + +# FIXME: add redis cache here. This can move a lot of pages into and out of the +# buffer pool. Make sure to invalidate the cache on tree renumbering +# REFACTOR: If we need to speed this up even more, consider using the DB cursor +# directly and skip SQLAlchemy constructs (like validate_tree_numbering) +def node_numbers_valid_for_tree(tree_name: str, definition_id: int) -> bool: + tree_model = strict_get_model(tree_name) + tree_table = tree_model.specify_model + canonical_tree_name = tree_table.name + tree_node = sqlmodels.models_by_tableid[tree_table.tableId] + with sqlmodels.session_context() as session: + invalid_nodes_exist = _individual_nodes_invalid(tree_node, canonical_tree_name, definition_id) + nodes_in_acceptable_ranges = _node_numbers_within_parent_ranges(tree_node, canonical_tree_name, definition_id) + final_query = ( + union_all(invalid_nodes_exist, nodes_in_acceptable_ranges) + .limit(1) + ) + result = session.execute(final_query).scalar_one_or_none() + return result is None + +def _individual_nodes_invalid(tree_model, tree_name: str, definition_id: int): + tree_def = getattr(tree_model, tree_name + "TreeDefID") + invalid_nodes_exist_query = ( + select(1) + .select_from(tree_model) + .where( + and_( + tree_def == definition_id, + or_( + tree_model.nodeNumber.is_(None), + tree_model.highestChildNodeNumber.is_(None), + tree_model.highestChildNodeNumber < tree_model.nodeNumber + ) + ) + ) + .limit(1) + ) + return invalid_nodes_exist_query + +def _node_numbers_within_parent_ranges(tree_model, tree_name: str, definition_id: int): + # This is one of two generally efficient ways i've found to validate nodes + # are within their correct ranges. + # The other method is to build a derived table that groups tree nodes by + # parent and gets the MIN and MAX of all of the children's NodeNumbers. + # Then the tree table is joined on the derived table by ParentID and + # validates the MIN and MAX nodenumber across all children lie within the + # parent's expected range. + # That method uses significantly less page accesses (~10x on some DBs) than + # this direct Parent JOIN, but these accesses are usually not for distinct + # pages, so not a big deal as long as they're in the buffer pool. + # The other method does use some space in the buffer pool for filesort + # operations, so keeping with this approach for now. + tree_def = getattr(tree_model, tree_name + "TreeDefID") + parent = aliased(tree_model) + children_within_node_ranges = ( + select(1) + .select_from(tree_model) + .join(parent, tree_model.ParentID == parent._id) + .where( + tree_def == definition_id, + not_(tree_model.nodeNumber.between( + parent.nodeNumber, + parent.highestChildNodeNumber + ) + ) + ) + .limit(1) + ) + return children_within_node_ranges diff --git a/specifyweb/backend/delete_blockers/validators.py b/specifyweb/backend/delete_blockers/validators.py new file mode 100644 index 00000000000..ee7605d58f4 --- /dev/null +++ b/specifyweb/backend/delete_blockers/validators.py @@ -0,0 +1,116 @@ +from typing import cast, TypedDict, NotRequired, Unpack + +from django import forms +from django.core.exceptions import ValidationError + +DELETE_BLOCKER_LIMIT = 40 + +class DeleteBlockerFilterJSON(TypedDict): + table: str + field: str + # The direction of the fetch. If true then results will be returned in + # descending order starting at the anchor + backwards: NotRequired[bool] + # The ID that will be used to anchor and base the results from to achieve + # keysey/cursor pagination. + # The anchor is exclusive and will not be included in the results + # If the anchor is not provided, the default value is based on the + # direction of the request. If fetching records forwards, the anchor will be + # 0 + # If fetching records backwards, the anchor will be the last record + anchor: NotRequired[int | None] + limit: NotRequired[int] + +class CleanedDeleteBlockerFilter(TypedDict): + table: str + field: str + backwards: bool + anchor: int | None + limit: int + +class CleanedDeleteBlockerRequestForm(TypedDict): + limit: int + filters: None | list[CleanedDeleteBlockerFilter] + +class DeleteBlockerRequestForm(forms.Form): + limit = forms.IntegerField(required=False) + + # Whether the format the results based on the default table formatter for + # the resulting table + # format = forms.BooleanField(required=False) + + filters = forms.JSONField(required=False) + + def clean(self): + super().clean() + if self.cleaned_data.get('limit') is None: + self.cleaned_data["limit"] = DELETE_BLOCKER_LIMIT + + return self.cleaned_data + + # BUG: Currently a table + field can be defined more than once within the + # list of filters. The backend will run each filter independently. + # Might be desirable to error or arbitarily choose the last defined filter. + # I guess the current behavior could be considered a feature if you wanted + # to use different anchors, directions, limits, etc. at the same time? + def clean_filters(self): + cleaned_filters = self.cleaned_data.get('filters') + if cleaned_filters is None: + return cleaned_filters + if not isinstance(cleaned_filters, list): + raise ValidationError("filters must be a list") + new_filters: list[CleanedDeleteBlockerFilter] = [] + for filter_idx, filter in enumerate(cleaned_filters): + if not isinstance(filter, dict): + raise ValidationError( + "Filter at index '%(filter_idx)d' is not a dict", + params={"filter_idx":filter_idx} + ) + required_keys = {'table', 'field'} + if not required_keys.issubset(filter.keys()): + missing_keys = list(required_keys - filter.keys()) + raise ValidationError( + "Filter at index '%(filter_idx)d' missing required keys '%(missing_keys)s'", + params={"filter_idx": filter_idx, + "missing_keys": str(missing_keys)} + ) + if not all(isinstance(key, str) for key in required_keys): + formatted_keys = " or ".join(required_keys) + raise ValidationError( + "Filter at index '%(filter_idx)d' did not get str for '%(required_keys)s'", + params={ + "filter_idx": filter_idx, + "required_keys": formatted_keys, + } + ) + new_filter: CleanedDeleteBlockerFilter = { + "table": filter['table'], + "field": filter['field'], + "anchor": filter.get('anchor'), + "limit": filter.get('limit', DELETE_BLOCKER_LIMIT), + "backwards": filter.get('backwards', False) + } + expected_types = { + "table": str, + "field": str, + "backwards": bool, + "anchor": (int, None), + "limit": int + } + for key, types in expected_types.items(): + if not isinstance(new_filter[key], types): + formatted_types = ( + " or ".join(map(lambda cls: cls.__name__, types)) + if isinstance(types, tuple) + else str(types) + ) + raise ValidationError( + "Filter at index '%(filter_idx)d' did not get %(types)s for '%(key)s'", + params={ + "filter_idx": filter_idx, + "types": formatted_types, + "key": key + } + ) + new_filters.append(new_filter) + return new_filters diff --git a/specifyweb/backend/delete_blockers/views.py b/specifyweb/backend/delete_blockers/views.py index ac17ff71b95..961e255d158 100644 --- a/specifyweb/backend/delete_blockers/views.py +++ b/specifyweb/backend/delete_blockers/views.py @@ -1,22 +1,23 @@ -from collections import defaultdict +import json from django import http from django.db import router, transaction -from django.db.models.deletion import Collector, CASCADE, PROTECT +from django.db.models.deletion import Collector from django.db.models import ForeignKey -from django.views.decorators.http import require_POST from specifyweb.middleware.general import require_http_methods from specifyweb.specify.api.crud import ( get_discipline_delete_guard_blockers, get_object_or_404, - get_model, prepare_discipline_for_delete, ) -from specifyweb.specify.models import protect_with_blockers from specifyweb.specify.api.serializers import toJson from specifyweb.specify.views import login_maybe_required +from specifyweb.backend.delete_blockers.validators import DeleteBlockerRequestForm +from specifyweb.backend.delete_blockers.filter import DeleteBlockerFilters + + @login_maybe_required @require_http_methods(['GET', 'HEAD']) def old_delete_blockers(request, model, id): @@ -30,7 +31,7 @@ def old_delete_blockers(request, model, id): obj = get_object_or_404(model, id=int(id)) using = router.db_for_write(obj.__class__, instance=obj) - if obj._meta.model_name == 'discipline': # Special case for discipline + if obj._meta.model_name == 'discipline': # Special case for discipline guard_blockers = get_discipline_delete_guard_blockers(obj) if guard_blockers: result = guard_blockers @@ -46,59 +47,40 @@ def old_delete_blockers(request, model, id): return http.HttpResponse(toJson(result), content_type='application/json') + @login_maybe_required -@require_http_methods(['GET']) -def delete_blockers(request, model, id): - limit = int(request.GET["limit"]) if "limit" in request.GET else 20 - offset = int(request.GET["offset"]) if "offset" in request.GET else 0 +@require_http_methods(['GET', 'POST']) +def delete_blockers(request: http.HttpRequest, model: str, id: int): obj = get_object_or_404(model, id=int(id)) - immediate, deferred = fetch_immediate_blockers(obj, limit=limit, offset=offset) + + if request.method == "POST": + request_data = json.loads(request.body) + else: + request_data = request.GET + + + request_filters = DeleteBlockerRequestForm(request_data) + if not request_filters.is_valid(): + return http.HttpResponseBadRequest( + toJson(request_filters.errors), content_type='application/json' + ) + + blocker_fetcher = DeleteBlockerFilters.from_json(obj, json=request_filters.cleaned_data) + blocked, deferred = blocker_fetcher.fetch() result = { - "results": immediate, + "results": blocked, "next": deferred } - return http.HttpResponse(toJson(result), content_type='application/json') + return http.JsonResponse(result) -def fetch_immediate_blockers(obj, limit=20, offset=0): - all_fields = obj._meta.get_fields(include_hidden=True) - all_relationships = filter( - # Check whether there are any concrete fields that SHOULD be included - # here, like some ToOne fields that acts as blockers - lambda field: field.is_relation and not field.concrete, - all_fields - ) - results = [] - next = [] - for relationship in all_relationships: - related_ids = _prepare_blockers(obj, relationship, limit=limit, offset=offset) - if len(related_ids) == 0: - continue - complete = limit == 0 or len(related_ids) < limit - payload = { - "table": relationship.related_model._meta.model_name, - "field": relationship.field.name, - "ids": list(related_ids), - "offset": offset, - "limit": limit, - "complete": complete - } - if relationship.on_delete is protect_with_blockers or relationship.on_delete is PROTECT: - results.append(payload) - elif relationship.on_delete is CASCADE: - next.append(payload) - return results, next - - -def _prepare_blockers(obj, relationship, limit=20, offset=0): - query_set = ( - relationship.related_model.objects - .filter( - **{relationship.field.name: obj.pk} - ).order_by("pk") - .values_list("pk", flat=True)) - if limit != 0: - query_set = query_set[offset: offset + limit] - return query_set +@login_maybe_required +@require_http_methods(['GET', 'POST']) +def count_blockers(request: http.HttpRequest, model: str, id: int): + obj = get_object_or_404(model, id=int(id)) + + delete_block_fetcher = DeleteBlockerFilters(obj, count_only=True) + result = delete_block_fetcher.count() + return http.JsonResponse(result) def _collect_delete_blockers(obj, using) -> list[dict]: collector = Collector(using=using) @@ -110,6 +92,7 @@ def _collect_delete_blockers(obj, using) -> list[dict]: ] for field, sub_objs in collector.delete_blockers ]) + def _serialize_delete_blocker(field, sub_objs) -> dict: normalized = _normalize_many_to_many_blocker(field, sub_objs) if normalized is not None: @@ -121,6 +104,7 @@ def _serialize_delete_blocker(field, sub_objs) -> dict: 'ids': [sub_obj.id for sub_obj in sub_objs] } + def _normalize_many_to_many_blocker(field, sub_objs) -> dict | None: through_model = sub_objs[0].__class__ if hasattr(through_model, 'specify_model'): @@ -167,5 +151,6 @@ def _normalize_many_to_many_blocker(field, sub_objs) -> dict | None: 'ids': [getattr(sub_obj, other_field.attname) for sub_obj in sub_objs], } + def flatten(l): return [item for sublist in l for item in sublist] diff --git a/specifyweb/backend/table_rows/forms.py b/specifyweb/backend/table_rows/forms.py deleted file mode 100644 index 4814525940a..00000000000 --- a/specifyweb/backend/table_rows/forms.py +++ /dev/null @@ -1,16 +0,0 @@ -from django import forms - -from specifyweb.specify.api.validators import GetCollectionForm - -class RowsForm(GetCollectionForm): - fields = forms.CharField(required=True) # type: ignore - distinct = forms.CharField(required=False) - defaults = dict( - domainfilter=None, - limit=0, - offset=0, - orderby=None, - distinct=False, - fields=None, - filterchronostrat=False, - ) \ No newline at end of file diff --git a/specifyweb/backend/trees/extras.py b/specifyweb/backend/trees/extras.py index 99cccbd7a9e..56f52b8b1d4 100644 --- a/specifyweb/backend/trees/extras.py +++ b/specifyweb/backend/trees/extras.py @@ -12,10 +12,9 @@ from django.db import models, connection from django.db.models import F, Q, ProtectedError -from django.conf import settings from specifyweb.backend.businessrules.exceptions import TreeBusinessRuleException -import specifyweb.specify.models as spmodels +from specifyweb.specify.models_utils.model_extras import ModelWithTable from specifyweb.backend.workbench.upload.auditcodes import TREE_BULK_MOVE, TREE_MERGE, TREE_SYNONYMIZE, TREE_DESYNONYMIZE @@ -29,8 +28,9 @@ def validate_node_numbers(table, revalidate_after=True): if revalidate_after: validate_tree_numbering(table) -class Tree(models.Model): +class Tree(ModelWithTable): _requires_collection_user = True + class Meta: abstract = True diff --git a/specifyweb/backend/workbench/upload/clone.py b/specifyweb/backend/workbench/upload/clone.py index 3ba75db28e0..c33ecdeb5f8 100644 --- a/specifyweb/backend/workbench/upload/clone.py +++ b/specifyweb/backend/workbench/upload/clone.py @@ -11,7 +11,7 @@ from specifyweb.specify.utils.func import Func from specifyweb.specify.models_utils.load_datamodel import Table -from specifyweb.specify.models import ModelWithTable +from specifyweb.specify.models_utils.model_extras import ModelWithTable FIELDS_TO_NOT_CLONE: dict[str, list[str]] = json.load( open("specifyweb/frontend/js_src/lib/components/DataModel/uniqueFields.json") diff --git a/specifyweb/backend/workbench/upload/upload_table.py b/specifyweb/backend/workbench/upload/upload_table.py index 84cbffd80ab..3eec6a41130 100644 --- a/specifyweb/backend/workbench/upload/upload_table.py +++ b/specifyweb/backend/workbench/upload/upload_table.py @@ -6,6 +6,7 @@ from specifyweb.backend.businessrules.exceptions import BusinessRuleException from specifyweb.specify import models +from specifyweb.specify.models_utils.model_extras import ModelWithTable from specifyweb.specify.utils.func import Func from specifyweb.specify.utils.field_change_info import FieldChangeInfo from specifyweb.specify.utils.autonumbering import AutonumberingLockDispatcher @@ -359,7 +360,7 @@ def can_save(self) -> bool: return isinstance(self.current_id, int) @property - def django_model(self) -> models.ModelWithTable: + def django_model(self) -> ModelWithTable: return getattr(models, self.name.capitalize()) @property @@ -502,8 +503,8 @@ def save_row(self, force=False) -> UploadResult: else update_table.process_row_with_null() ) - def _get_reference(self, should_cache=True) -> models.ModelWithTable | None: - model: models.ModelWithTable = self.django_model + def _get_reference(self, should_cache=True) -> ModelWithTable | None: + model: ModelWithTable = self.django_model current_id = self.current_id if current_id is None: @@ -705,7 +706,7 @@ def _check_missing_required(self) -> ParseFailures | None: def _do_upload( self, - model: models.ModelWithTable, + model: ModelWithTable, to_one_results: dict[str, UploadResult], info: ReportInfo, ) -> UploadResult: @@ -779,7 +780,7 @@ def _do_upload( return UploadResult(record, to_one_results, to_many_results) def _handle_to_many( - self, update: bool, parent_id: int, model: models.ModelWithTable + self, update: bool, parent_id: int, model: ModelWithTable ): return { fieldname: _upload_to_manys( diff --git a/specifyweb/specify/api/crud.py b/specifyweb/specify/api/crud.py index 915f1c32c49..5db24395a90 100644 --- a/specifyweb/specify/api/crud.py +++ b/specifyweb/specify/api/crud.py @@ -430,6 +430,8 @@ def strict_get_model(name: str, apps = apps): for model in app.get_models(): if model._meta.model_name == name: return model + # REFACTOR: consider raising a Table/Model DoesNotExist Error here + # instead? raise e def get_model(name: str, apps=apps): diff --git a/specifyweb/specify/models.py b/specifyweb/specify/models.py index bb3a6a19954..ac8b99e4b41 100644 --- a/specifyweb/specify/models.py +++ b/specifyweb/specify/models.py @@ -3,8 +3,8 @@ from django.utils import timezone from specifyweb.backend.businessrules.exceptions import AbortSave from specifyweb.specify.models_utils.model_timestamp import save_auto_timestamp_field_with_override -from specifyweb.specify.models_utils import model_extras -from .datamodel import datamodel, Table +from specifyweb.specify.models_utils import model_extras, tree_model_extras +from .datamodel import datamodel import logging logger = logging.getLogger(__name__) @@ -24,11 +24,6 @@ def custom_save(self, *args, **kwargs): logger.error("Save operation aborted: %s", e) return -# TODO: Use this everywhere -class ModelWithTable(models.Model): - specify_model: Table - class Meta: - abstract = True # These Django model classes were generated by the specifyweb.specify.sp7_build_models.py script. # The original models in this file are based on the Specify 6.8.03 datamodel schema. @@ -3670,7 +3665,7 @@ class Meta: save = partialmethod(custom_save) -class Geography(model_extras.Geography): +class Geography(tree_model_extras.Geography): specify_model = datamodel.get_table_strict('geography') # ID Field @@ -3753,7 +3748,7 @@ class Meta: save = partialmethod(custom_save) -class Geographytreedefitem(model_extras.Geographytreedefitem): +class Geographytreedefitem(tree_model_extras.Geographytreedefitem): specify_model = datamodel.get_table_strict('geographytreedefitem') # ID Field @@ -3789,7 +3784,7 @@ class Meta: save = partialmethod(custom_save) -class Geologictimeperiod(model_extras.Geologictimeperiod): +class Geologictimeperiod(tree_model_extras.Geologictimeperiod): specify_model = datamodel.get_table_strict('geologictimeperiod') # aka. Chronostratigraphy # ID Field @@ -3867,7 +3862,7 @@ class Meta: save = partialmethod(custom_save) -class Geologictimeperiodtreedefitem(model_extras.Geologictimeperiodtreedefitem): +class Geologictimeperiodtreedefitem(tree_model_extras.Geologictimeperiodtreedefitem): specify_model = datamodel.get_table_strict('geologictimeperiodtreedefitem') # ID Field @@ -4302,7 +4297,7 @@ class Meta: save = partialmethod(custom_save) -class Lithostrat(model_extras.Lithostrat): +class Lithostrat(tree_model_extras.Lithostrat): specify_model = datamodel.get_table_strict('lithostrat') # ID Field @@ -4378,7 +4373,7 @@ class Meta: save = partialmethod(custom_save) -class Lithostrattreedefitem(model_extras.Lithostrattreedefitem): +class Lithostrattreedefitem(tree_model_extras.Lithostrattreedefitem): specify_model = datamodel.get_table_strict('lithostrattreedefitem') # ID Field @@ -6773,7 +6768,7 @@ class Meta: # save = partialmethod(custom_save) -class Storage(model_extras.Storage): +class Storage(tree_model_extras.Storage): specify_model = datamodel.get_table_strict('storage') # ID Field @@ -6874,7 +6869,7 @@ class Meta: save = partialmethod(custom_save) -class Storagetreedefitem(model_extras.Storagetreedefitem): +class Storagetreedefitem(tree_model_extras.Storagetreedefitem): specify_model = datamodel.get_table_strict('storagetreedefitem') # ID Field @@ -6910,7 +6905,7 @@ class Meta: save = partialmethod(custom_save) -class Taxon(model_extras.Taxon): +class Taxon(tree_model_extras.Taxon): specify_model = datamodel.get_table_strict('taxon') # ID Field @@ -7312,7 +7307,7 @@ class Meta: save = partialmethod(custom_save) -class Taxontreedefitem(model_extras.Taxontreedefitem): +class Taxontreedefitem(tree_model_extras.Taxontreedefitem): specify_model = datamodel.get_table_strict('taxontreedefitem') # ID Field @@ -8111,7 +8106,7 @@ class Meta: save = partialmethod(custom_save) -class Tectonicunittreedefitem(model_extras.Tectonicunittreedefitem): +class Tectonicunittreedefitem(tree_model_extras.Tectonicunittreedefitem): specify_model = datamodel.get_table_strict('tectonicUnittreedefitem') # ID Field @@ -8146,7 +8141,7 @@ class Meta: save = partialmethod(custom_save) -class Tectonicunit(model_extras.Tectonicunit): +class Tectonicunit(tree_model_extras.Tectonicunit): specify_model = datamodel.get_table_strict('tectonicunit') # ID Field diff --git a/specifyweb/specify/models_utils/model_extras.py b/specifyweb/specify/models_utils/model_extras.py index 70c14bb72ea..7e5cda4a79b 100644 --- a/specifyweb/specify/models_utils/model_extras.py +++ b/specifyweb/specify/models_utils/model_extras.py @@ -3,16 +3,21 @@ from django.db import models from django.contrib.auth.base_user import BaseUserManager from django.conf import settings -from django.utils import timezone +from specifyweb.specify.datamodel import Table from .model_timestamp import save_auto_timestamp_field_with_override -from specifyweb.backend.trees.extras import Tree, TreeRank if settings.AUTH_LDAP_SERVER_URI is not None: from .auth import ldap_extras logger = logging.getLogger(__name__) +# TODO: Use this everywhere +class ModelWithTable(models.Model): + specify_model: Table + class Meta: + abstract = True + class SpecifyUserManager(BaseUserManager): def create_user(self, name, password=None): raise NotImplementedError() @@ -228,51 +233,3 @@ def is_paleo_geo(self): class Meta: abstract = True - -class Taxon(Tree): - class Meta: - abstract = True - -class Storage(Tree): - class Meta: - abstract = True - -class Geography(Tree): - class Meta: - abstract = True - -class Geologictimeperiod(Tree): - class Meta: - abstract = True - -class Lithostrat(Tree): - class Meta: - abstract = True - -class Tectonicunit(Tree): - class Meta: - abstract = True - -class Geographytreedefitem(TreeRank): - class Meta: - abstract = True - -class Geologictimeperiodtreedefitem(TreeRank): - class Meta: - abstract = True - -class Lithostrattreedefitem(TreeRank): - class Meta: - abstract = True - -class Storagetreedefitem(TreeRank): - class Meta: - abstract = True - -class Taxontreedefitem(TreeRank): - class Meta: - abstract = True - -class Tectonicunittreedefitem(TreeRank): - class Meta: - abstract = True diff --git a/specifyweb/specify/models_utils/tree_model_extras.py b/specifyweb/specify/models_utils/tree_model_extras.py new file mode 100644 index 00000000000..22a24a70cc6 --- /dev/null +++ b/specifyweb/specify/models_utils/tree_model_extras.py @@ -0,0 +1,51 @@ +from specifyweb.backend.trees.extras import Tree, TreeRank + +# REFACTOR: remove these in favor of just using Tree and TreeRank on the actual +# model definitions +class Taxon(Tree): + class Meta: + abstract = True + +class Storage(Tree): + class Meta: + abstract = True + +class Geography(Tree): + class Meta: + abstract = True + +class Geologictimeperiod(Tree): + class Meta: + abstract = True + +class Lithostrat(Tree): + class Meta: + abstract = True + +class Tectonicunit(Tree): + class Meta: + abstract = True + +class Geographytreedefitem(TreeRank): + class Meta: + abstract = True + +class Geologictimeperiodtreedefitem(TreeRank): + class Meta: + abstract = True + +class Lithostrattreedefitem(TreeRank): + class Meta: + abstract = True + +class Storagetreedefitem(TreeRank): + class Meta: + abstract = True + +class Taxontreedefitem(TreeRank): + class Meta: + abstract = True + +class Tectonicunittreedefitem(TreeRank): + class Meta: + abstract = True \ No newline at end of file From b96ad9a0069454b74c22373f2fd8cc8d2e33879f Mon Sep 17 00:00:00 2001 From: melton-jason Date: Mon, 5 Oct 2026 10:27:49 -0500 Subject: [PATCH 08/66] refactor: unify redis usage throughout backend --- specifyweb/backend/cache/redis/__init__.py | 12 +- specifyweb/backend/cache/redis/connect.py | 104 ++++++++++++++---- specifyweb/backend/cache/redis/rqueue.py | 61 ---------- specifyweb/backend/cache/redis/store.py | 38 ------- specifyweb/backend/cache/redis/utils.py | 79 ------------- specifyweb/backend/delete_blockers/utils.py | 3 + specifyweb/backend/setup_tool/apps.py | 5 +- specifyweb/backend/setup_tool/redis.py | 6 +- specifyweb/backend/setup_tool/setup_tasks.py | 14 ++- .../backend/setup_tool/task_tracking.py | 16 ++- specifyweb/backend/trees/apps.py | 5 +- specifyweb/backend/trees/defaults.py | 17 +-- specifyweb/backend/trees/redis.py | 2 +- specifyweb/specify/auth/support_login.py | 8 +- .../migrations/0043_normalize_many_to_many.py | 23 ++-- 15 files changed, 143 insertions(+), 250 deletions(-) delete mode 100644 specifyweb/backend/cache/redis/rqueue.py delete mode 100644 specifyweb/backend/cache/redis/store.py delete mode 100644 specifyweb/backend/cache/redis/utils.py diff --git a/specifyweb/backend/cache/redis/__init__.py b/specifyweb/backend/cache/redis/__init__.py index a2642045fd8..1496d59c66c 100644 --- a/specifyweb/backend/cache/redis/__init__.py +++ b/specifyweb/backend/cache/redis/__init__.py @@ -1,11 +1 @@ -from .store import ( - set_bytes, - set_string, - get_bytes, - get_string, - add_to_set, - delete_key, - remove_from_set, - set_members, - redis_type -) \ No newline at end of file +from .connect import RedisConnection, RedisSet, RedisList, RedisString diff --git a/specifyweb/backend/cache/redis/connect.py b/specifyweb/backend/cache/redis/connect.py index 8c8f9d3862e..f0935967f94 100644 --- a/specifyweb/backend/cache/redis/connect.py +++ b/specifyweb/backend/cache/redis/connect.py @@ -1,6 +1,18 @@ +from typing import Literal + from redis import Redis from django.conf import settings +# https://redis.io/docs/latest/commands/type/ +Redis_Type = Literal["none", "string", "list", "set", "hash", "stream", "vectorset"] + +# REFACTOR: It might make more sense to move this to a setting on +# RedisConnection, rather than having it as a flag on each adapter +def _format_key(key: str): + db_name = getattr(settings, "DATABASE_NAME") + key_parts = ["specify", db_name, key] + return ":".join(key_parts) + class RedisConnection: def __init__(self, host=getattr(settings, "REDIS_HOST", None), @@ -21,9 +33,16 @@ def __init__(self, decode_responses=self.decode_responses ) - def delete(self, key: str): + def delete(self, key: str, format_key=True): + if format_key: + key = _format_key(key) return self.connection.delete(key) + def key_type(self, key: str, format_key=True) -> Redis_Type: + if format_key: + key = _format_key(key) + return self.connection.type(key) + class RedisDataType: def __init__(self, established: RedisConnection) -> None: @@ -33,36 +52,55 @@ def __init__(self, established: RedisConnection) -> None: def connection(self): return self._established.connection - def delete(self, key: str): - return self._established.delete(key) + def delete(self, key: str, format_key=True): + return self._established.delete(key, format_key=format_key) + + def key_type(self, key: str, format_key=True) -> Redis_Type: + return self._established.key_type(key, format_key=format_key) class RedisList(RedisDataType): """ See https://redis.io/docs/latest/develop/data-types/lists/ """ - def left_push(self, key: str, value) -> int: + def left_push(self, key: str, value, format_key=True) -> int: + if format_key: + key = _format_key(key) return self.connection.lpush(key, value) - def right_push(self, key: str, value) -> int: + def right_push(self, key: str, value, format_key=True) -> int: + if format_key: + key = _format_key(key) return self.connection.rpush(key, value) - def right_pop(self, key: str) -> str | bytes | None: + def right_pop(self, key: str, format_key=True) -> str | bytes | None: + if format_key: + key = _format_key(key) return self.connection.rpop(key) - def left_pop(self, key: str) -> str | bytes | None: + def left_pop(self, key: str, format_key=True) -> str | bytes | None: + if format_key: + key = _format_key(key) return self.connection.lpop(key) - def length(self, key: str) -> int: + def length(self, key: str, format_key=True) -> int: + if format_key: + key = _format_key(key) return self.connection.llen(key) - def range(self, key: str, start_index: int, end_index: int) -> list[str] | list[bytes]: + def range(self, key: str, start_index: int, end_index: int, format_key=True) -> list[str] | list[bytes]: + if format_key: + key = _format_key(key) return self.connection.lrange(key, start_index, end_index) - def trim(self, key: str, start_index: int, end_index: int) -> list[str] | list[bytes]: + def trim(self, key: str, start_index: int, end_index: int, format_key=True) -> list[str] | list[bytes]: + if format_key: + key = _format_key(key) return self.connection.ltrim(key, start_index, end_index) - def blocking_left_pop(self, key: str, timeout: int) -> str | bytes | None: + def blocking_left_pop(self, key: str, timeout: int, format_key=True) -> str | bytes | None: + if format_key: + key = _format_key(key) # This will block the thread until either an item is pushed into the # List or timeout seconds have passed # https://redis.io/docs/latest/commands/blpop/ @@ -76,44 +114,66 @@ class RedisSet(RedisDataType): """ See https://redis.io/docs/latest/develop/data-types/sets/ """ - def add(self, key: str, *values: str) -> int: + def add(self, key: str, *values: str, format_key=True) -> int: + if format_key: + key = _format_key(key) return self.connection.sadd(key, *values) - def is_member(self, key: str, value: str) -> bool: + def is_member(self, key: str, value: str, format_key=True) -> bool: + if format_key: + key = _format_key(key) is_member = int(self.connection.sismember(key, value)) return is_member == 1 - def remove(self, key: str, value: str): - return self.connection.srem(key, value) + def remove(self, key: str, *values: str, format_key=True): + if format_key: + key = _format_key(key) + return self.connection.srem(key, *values) - def size(self, key: str) -> int: + def size(self, key: str, format_key=True) -> int: + if format_key: + key = _format_key(key) return self.connection.scard(key) - def members(self, key: str) -> set[str]: + def members(self, key: str, format_key=True) -> set[str]: + if format_key: + key = _format_key(key) return self.connection.smembers(key) - def union(self, *keys: str) -> set[str]: + def union(self, *keys: str, format_keys=True) -> set[str]: + if format_keys: + keys = tuple((_format_key(key) for key in keys)) return self.connection.sunion(*keys) - def intersection(self, *keys: str) -> set[str]: + def intersection(self, *keys: str, format_keys=True) -> set[str]: + if format_keys: + keys = tuple((_format_key(key) for key in keys)) return self.connection.sinter(*keys) - def difference(self, *keys: str) -> set[str]: + def difference(self, *keys: str, format_keys=True) -> set[str]: + if format_keys: + keys = tuple((_format_key(key) for key in keys)) return self.connection.sdiff(*keys) +# REFACTOR: Make a dedicated RedisBytes class to make working with bytes more +# intuitive class RedisString(RedisDataType): """ See https://redis.io/docs/latest/develop/data-types/strings/ """ - def set(self, key, value, time_to_live=None, override_existing=True): + def set(self, key, value, time_to_live=None, override_existing=True, format_key=True): + if format_key: + key = _format_key(key) flags = { "ex": time_to_live, "nx": not override_existing } self.connection.set(key, value, **flags) - def get(self, key, delete_key=False) -> str | bytes | None: + def get(self, key, delete_key=False, format_key=True) -> str | bytes | None: + if format_key: + key = _format_key(key) if delete_key: return self.connection.getdel(key) return self.connection.get(key) diff --git a/specifyweb/backend/cache/redis/rqueue.py b/specifyweb/backend/cache/redis/rqueue.py deleted file mode 100644 index e7ab1bf1d11..00000000000 --- a/specifyweb/backend/cache/redis/rqueue.py +++ /dev/null @@ -1,61 +0,0 @@ -import json - -from typing import Callable, Generator, Iterable, cast - -from specifyweb.backend.cache.redis.connect import RedisConnection, RedisList - -type Serialized = str | bytes | bytearray - -type Serializer[T] = Callable[[T], str] -type Deserializer[T] = Callable[[Serialized], T] - -def default_serializer(obj) -> str: - return str(obj) - -def default_deserializer(serialized: Serialized): - return serialized - -class RedisQueue[T]: - def __init__(self, connection: RedisConnection, key: str, - serializer: Serializer[T] | None = None, - deserializer: Deserializer[T] | None = None): - self.connection = RedisList(connection) - self.key = key - self.serializer = serializer or cast(Serializer[T], default_serializer) - self.deserializer = deserializer or cast(Deserializer[T], default_deserializer) - - def key_name(self, *name_parts: str | None): - key_name = "_".join([self.key, *(part for part in name_parts if part is not None)]) - return key_name - - def push(self, *objs: T, sub_key: str | None = None) -> int: - key_name = self.key_name(sub_key) - return self.connection.right_push(key_name, *self._serialize_objs(*objs)) - - def pop(self, sub_key: str | None = None) -> T | None: - key_name = self.key_name(sub_key) - popped = self.connection.left_pop(key_name) - if popped is None: - return None - return self.deserializer(popped) - - def wait_and_pop(self, timeout: int = 0, sub_key: str | None = None) -> T: - key_name = self.key_name(sub_key) - popped = self.connection.blocking_left_pop(key_name, timeout) - if popped is None: - raise TimeoutError("No items in queue after timeout") - return self.deserializer(popped) - - def peek(self, sub_key: str | None = None) -> T | None: - key_name = self.key_name(sub_key) - top_value = self._deserialize_objs(*self.connection.range(key_name, 0, 0)) - if len(top_value) == 0: - return None - return top_value[0] - - def _serialize_objs(self, *objs: T) -> Generator[str, None, None]: - return (self.serializer(obj) for obj in objs) - - def _deserialize_objs(self, *serialized: Serialized): - return tuple(self.deserializer(obj) for obj in serialized) - diff --git a/specifyweb/backend/cache/redis/store.py b/specifyweb/backend/cache/redis/store.py deleted file mode 100644 index c5ef92ead37..00000000000 --- a/specifyweb/backend/cache/redis/store.py +++ /dev/null @@ -1,38 +0,0 @@ -from .utils import _set_string, _get_string, _delete_key, _add_to_set, _remove_from_set, _set_elements, _redis_type, format_key - -# REFACTOR: Replace these with RedisConnection adapters - -def set_string(key: str | bytes, value: str, time_to_live=None, override_existing=True): - return _set_string(format_key(key), value, time_to_live=time_to_live, override_existing=override_existing, decode_responses=True) - - -def set_bytes(key: str | bytes, value: bytes, time_to_live=None, override_existing=True): - return _set_string(format_key(key), value, time_to_live=time_to_live, override_existing=override_existing, decode_responses=False) - - -def get_string(key: str | bytes, delete_key=False) -> str | None: - return _get_string(format_key(key), delete_key=delete_key, decode_responses=True) - - -def get_bytes(key: str | bytes, delete_key=False) -> bytes | None: - return _get_string(format_key(key), delete_key=delete_key, decode_responses=False) - - -def add_to_set(key: str | bytes, *elements: str): - return _add_to_set(format_key(key), *elements) - - -def remove_from_set(key: str | bytes, *elements: str): - return _remove_from_set(format_key(key), *elements) - - -def set_members(key: str | bytes): - return _set_elements(format_key(key)) - - -def delete_key(key: str | bytes): - return _delete_key(format_key(key)) - - -def redis_type(key: str | bytes): - return _redis_type(format_key(key)) diff --git a/specifyweb/backend/cache/redis/utils.py b/specifyweb/backend/cache/redis/utils.py deleted file mode 100644 index 5682b559358..00000000000 --- a/specifyweb/backend/cache/redis/utils.py +++ /dev/null @@ -1,79 +0,0 @@ -from typing import overload, Literal - -from redis import Redis -from django.conf import settings - - -def redis_connection(decode_responses=True): - redis_host = getattr(settings, "REDIS_HOST", None) - redis_port = getattr(settings, "REDIS_PORT", None) - redis_db_index = getattr(settings, "REDIS_DB_INDEX", 0) - if None in (redis_host, redis_port, redis_db_index): - raise ValueError("Redis is not correctly configured", redis_host, redis_port) - return Redis(host=redis_host, port=redis_port, db=redis_db_index, decode_responses=decode_responses) - -@overload -def format_key(key: str) -> str: ... - -@overload -def format_key(key: bytes) -> bytes: ... - -def format_key(key: str | bytes) -> str | bytes: - """Formats key to avoid collisions when specify instances are sharing a cache. - Expected format: specify:{database}:app:name""" - if isinstance(key, bytes): - return key - db_name = getattr(settings, "DATABASE_NAME") - return key.format(database=db_name) - -def _delete_key(key: str): - host = redis_connection() - host.delete(key) - -def _set_string(key: str, value: str, time_to_live=None, override_existing=True, decode_responses=True): - host = redis_connection(decode_responses=decode_responses) - # See https://redis.readthedocs.io/en/stable/commands.html#redis.commands.core.CoreCommands.set - flags = { - "ex": time_to_live, - "nx": not override_existing - } - host.set(key, value, **flags) - - -@overload -def _get_string(key: str, delete_key: bool, decode_responses: True) -> str | None: ... - - -@overload -def _get_string(key: str, delete_key: bool, decode_responses: False) -> bytes | None: ... - - -def _get_string(key: str, delete_key: bool=False, decode_responses=True) -> str | bytes | None: - host = redis_connection(decode_responses=decode_responses) - if delete_key: - return host.getdel(key) - - return host.get(key) - -def _add_to_set(key: str, *elements: str): - if len(elements) <= 0: - return 0 - host = redis_connection(decode_responses=True) - return host.sadd(key, *elements) - -def _remove_from_set(key: str, *elements: str) -> int: - if len(elements) <= 0: - return 0 - host = redis_connection(decode_responses=True) - return host.srem(key, *elements) - -def _set_elements(key: str) -> set: - host = redis_connection(decode_responses=True) - return host.smembers(key) - -# https://redis.io/docs/latest/commands/type/ -Redis_Type = Literal["none", "string", "list", "set", "hash", "stream", "vectorset"] - -def _redis_type(key: str) -> Redis_Type: - host = redis_connection(decode_responses=True) - return host.type(key) \ No newline at end of file diff --git a/specifyweb/backend/delete_blockers/utils.py b/specifyweb/backend/delete_blockers/utils.py index f532266b1c7..9cc181bd50c 100644 --- a/specifyweb/backend/delete_blockers/utils.py +++ b/specifyweb/backend/delete_blockers/utils.py @@ -55,6 +55,9 @@ def node_numbers_valid_for_tree(tree_name: str, definition_id: int) -> bool: result = session.execute(final_query).scalar_one_or_none() return result is None +# REFACTOR: It might be better here to validate all NodeNumbers and +# HighestChildNodeNumbers are distinct here instead. +# e.g., COUNT(*) == COUNT(DISTINCT NodeNumber) == COUNT(DISTINCT HighestChildNodeNumber) def _individual_nodes_invalid(tree_model, tree_name: str, definition_id: int): tree_def = getattr(tree_model, tree_name + "TreeDefID") invalid_nodes_exist_query = ( diff --git a/specifyweb/backend/setup_tool/apps.py b/specifyweb/backend/setup_tool/apps.py index 728cb672e71..b8cad285d20 100644 --- a/specifyweb/backend/setup_tool/apps.py +++ b/specifyweb/backend/setup_tool/apps.py @@ -1,5 +1,5 @@ from django.apps import AppConfig -from specifyweb.backend.cache.redis import get_string +from specifyweb.backend.cache.redis.connect import RedisConnection, RedisString import logging logger = logging.getLogger(__name__) @@ -10,8 +10,9 @@ class SetupToolConfig(AppConfig): def ready(self): from specifyweb.backend.setup_tool.redis import ACTIVE_TASK_REDIS_KEY try: + redis_string = RedisString(RedisConnection()) # Clear potential setup task id leftover from a crash - task_id = get_string(ACTIVE_TASK_REDIS_KEY, delete_key=True) + task_id = redis_string.get(ACTIVE_TASK_REDIS_KEY, delete_key=True) if task_id is not None: logger.debug(f'Clearing last active setup task: {task_id}') except Exception: diff --git a/specifyweb/backend/setup_tool/redis.py b/specifyweb/backend/setup_tool/redis.py index df96acab946..52b177b17c8 100644 --- a/specifyweb/backend/setup_tool/redis.py +++ b/specifyweb/backend/setup_tool/redis.py @@ -1,10 +1,10 @@ # Keep track of the currently running setup task. There should only ever be one. # Also defined separately in setup_tool/apps.py -ACTIVE_TASK_REDIS_KEY = "specify:{database}:setup:active_task_id" +ACTIVE_TASK_REDIS_KEY = "setup:active_task_id" ACTIVE_TASK_TTL = 60*60*2 # setup should be less than 2 hours # Keep track of last error. -LAST_ERROR_REDIS_KEY = "specify:{database}:setup:last_error" +LAST_ERROR_REDIS_KEY = "setup:last_error" # Track async setup/config tasks by resource scope. -DISCIPLINE_TASKS_REDIS_KEY = "specify:{database}:setup:discipline:{discipline_id}:tasks" +DISCIPLINE_TASKS_REDIS_KEY = "setup:discipline:{discipline_id}:tasks" diff --git a/specifyweb/backend/setup_tool/setup_tasks.py b/specifyweb/backend/setup_tool/setup_tasks.py index ad8af24f07a..db243492162 100644 --- a/specifyweb/backend/setup_tool/setup_tasks.py +++ b/specifyweb/backend/setup_tool/setup_tasks.py @@ -11,7 +11,7 @@ from specifyweb.backend.setup_tool.tree_defaults import start_preload_default_tree from specifyweb.specify.models_utils.model_extras import PALEO_DISCIPLINES, GEOLOGY_DISCIPLINES from specifyweb.celery_tasks import is_worker_alive, MissingWorkerError -from specifyweb.backend.cache.redis import set_string, get_string +from specifyweb.backend.cache.redis.connect import RedisString, RedisConnection from specifyweb.backend.setup_tool.redis import ACTIVE_TASK_REDIS_KEY, ACTIVE_TASK_TTL, LAST_ERROR_REDIS_KEY from uuid import uuid4 @@ -33,13 +33,15 @@ def setup_database_background(data: dict) -> str: task = setup_database_task.apply_async(args, task_id=task_id) - set_string(ACTIVE_TASK_REDIS_KEY, task.id, time_to_live=ACTIVE_TASK_TTL) + redis_string = RedisString(RedisConnection()) + redis_string.set(ACTIVE_TASK_REDIS_KEY, task.id, time_to_live=ACTIVE_TASK_TTL) return task.id def get_active_setup_task() -> Tuple[Optional[AsyncResult], bool]: """Return the current setup task if it is active, and also if it is busy.""" - task_id = get_string(ACTIVE_TASK_REDIS_KEY) + redis_string = RedisString(RedisConnection()) + task_id = redis_string.get(ACTIVE_TASK_REDIS_KEY) if not task_id: return None, False @@ -183,13 +185,15 @@ def update_progress(): raise def get_last_setup_error() -> Optional[str]: - err = get_string(LAST_ERROR_REDIS_KEY) + redis_string = RedisString(RedisConnection()) + err = redis_string.get(LAST_ERROR_REDIS_KEY) if err == '': return None return err def set_last_setup_error(error_text: Optional[str]): - set_string(LAST_ERROR_REDIS_KEY, error_text or '', time_to_live=60*60*24) + redis_string = RedisString(RedisConnection()) + redis_string.set(LAST_ERROR_REDIS_KEY, error_text or '', time_to_live=60*60*24) def create_discipline_and_trees_task(data: dict): from specifyweb.specify.models import Discipline diff --git a/specifyweb/backend/setup_tool/task_tracking.py b/specifyweb/backend/setup_tool/task_tracking.py index 31a174622fb..ae18ba94354 100644 --- a/specifyweb/backend/setup_tool/task_tracking.py +++ b/specifyweb/backend/setup_tool/task_tracking.py @@ -1,7 +1,7 @@ from typing import Optional import logging -from specifyweb.backend.cache.redis import add_to_set, delete_key, remove_from_set, set_members +from specifyweb.backend.cache.redis.connect import RedisConnection, RedisSet from specifyweb.celery_tasks import CELERY_TASK_STATE, app from specifyweb.backend.setup_tool.redis import ( DISCIPLINE_TASKS_REDIS_KEY, @@ -32,13 +32,16 @@ def _discipline_tasks_key(discipline_id: int) -> str: return DISCIPLINE_TASKS_REDIS_KEY.replace("{discipline_id}", str(discipline_id)) def _remove_task_ids_and_delete_empty_key(key: str, *task_ids: str) -> None: - remove_from_set(key, *task_ids) - if len(set_members(key)) == 0: - delete_key(key) + redis_set = RedisSet(RedisConnection()) + redis_set.remove(key, *task_ids) + if redis_set.size(key) == 0: + redis_set.delete(key) def queue_discipline_background_task(discipline_id: int, task_id: str) -> None: try: - add_to_set(_discipline_tasks_key(discipline_id), task_id) + redis_set = RedisSet(RedisConnection()) + key = _discipline_tasks_key(discipline_id) + redis_set.add(key, task_id) except Exception: logger.warning( "Failed to track discipline task %s for discipline %s.", @@ -58,7 +61,8 @@ def finish_discipline_background_task(discipline_id: int, task_id: str) -> None: def _active_task_ids_from_redis_key(key: str) -> set[str]: try: - task_ids = set_members(key) + redis_set = RedisSet(RedisConnection()) + task_ids = redis_set.members(key) if not task_ids: return set() diff --git a/specifyweb/backend/trees/apps.py b/specifyweb/backend/trees/apps.py index a6dd26a5f4c..43c52c7f7c8 100644 --- a/specifyweb/backend/trees/apps.py +++ b/specifyweb/backend/trees/apps.py @@ -1,5 +1,5 @@ from django.apps import AppConfig -from specifyweb.backend.cache.redis import get_string +from specifyweb.backend.cache.redis.connect import RedisConnection, RedisString import logging logger = logging.getLogger(__name__) @@ -11,8 +11,9 @@ class TreesConfig(AppConfig): def ready(self): from specifyweb.backend.trees.redis import ACTIVE_DEFAULT_TREE_TASK_REDIS_KEY try: + redis_string = RedisString(RedisConnection()) # Clear potential leftover tree creation tracking information - data = get_string(ACTIVE_DEFAULT_TREE_TASK_REDIS_KEY, delete_key=True) + data = redis_string.get(ACTIVE_DEFAULT_TREE_TASK_REDIS_KEY, delete_key=True) if data is not None: logger.debug(f'Clearing last active default tree creation tasks: {data}') except Exception: diff --git a/specifyweb/backend/trees/defaults.py b/specifyweb/backend/trees/defaults.py index 1a137ce59d6..590fc549df2 100644 --- a/specifyweb/backend/trees/defaults.py +++ b/specifyweb/backend/trees/defaults.py @@ -9,7 +9,7 @@ from specifyweb.backend.trees.default_tree_files import stream_default_tree_csv from specifyweb.backend.trees.utils import get_models, TREE_ROOT_NODES from specifyweb.backend.trees.extras import renumber_tree, set_fullnames -from specifyweb.backend.cache.redis import add_to_set, remove_from_set, set_members +from specifyweb.backend.cache.redis.connect import RedisConnection, RedisSet from specifyweb.backend.trees.redis import ACTIVE_DEFAULT_TREE_TASK_REDIS_KEY import logging @@ -324,8 +324,6 @@ def add_default_tree_record(context: DefaultTreeContext, row: dict, tree_cfg: Tr if v: defaults[model_field] = v - rank_title = rank_mapping.get('title', rank_name.capitalize()) - # Get the rank by the rank id, or column name as a fallback. # Skip creating on this rank if it doesn't exist if rank_mapping['rank']: @@ -393,18 +391,21 @@ def add_default_tree_record(context: DefaultTreeContext, row: dict, tree_cfg: Tr def queue_create_default_tree_task(task_id): """Store queued (and active) default tree creation tasks so they can be reliably tracked later.""" - add_to_set(ACTIVE_DEFAULT_TREE_TASK_REDIS_KEY, task_id) - logger.debug(f"Queued task {task_id}. Current tasks: {set_members(ACTIVE_DEFAULT_TREE_TASK_REDIS_KEY)}") + redis_set = RedisSet(RedisConnection()) + redis_set.add(ACTIVE_DEFAULT_TREE_TASK_REDIS_KEY, task_id) + logger.debug(f"Queued task {task_id}. Current tasks: {redis_set.members(ACTIVE_DEFAULT_TREE_TASK_REDIS_KEY)}") def get_active_create_default_tree_tasks() -> list[str]: - tasks = set_members(ACTIVE_DEFAULT_TREE_TASK_REDIS_KEY) + redis_set = RedisSet(RedisConnection()) + tasks = redis_set.members(ACTIVE_DEFAULT_TREE_TASK_REDIS_KEY) logger.debug(f"Active tree creation tasks: {tasks}") return list(tasks) def finish_create_default_tree_task(task_id): """Clear a finished tree creation task from redis cache.""" - remove_from_set(ACTIVE_DEFAULT_TREE_TASK_REDIS_KEY, task_id) - logger.debug(f"Finished task {task_id}. Current tasks: {set_members(ACTIVE_DEFAULT_TREE_TASK_REDIS_KEY)}") + redis_set = RedisSet(RedisConnection()) + redis_set.remove(ACTIVE_DEFAULT_TREE_TASK_REDIS_KEY, task_id) + logger.debug(f"Finished task {task_id}. Current tasks: {redis_set.members(ACTIVE_DEFAULT_TREE_TASK_REDIS_KEY)}") @app.task(base=LogErrorsTask, bind=True) def create_default_tree_task(self, url: str, discipline_id: int, tree_type: str, specify_collection_id: Optional[int], diff --git a/specifyweb/backend/trees/redis.py b/specifyweb/backend/trees/redis.py index 1b9b023ec0b..0caae40a8b6 100644 --- a/specifyweb/backend/trees/redis.py +++ b/specifyweb/backend/trees/redis.py @@ -1,2 +1,2 @@ # Used to track active default tree creation tasks -ACTIVE_DEFAULT_TREE_TASK_REDIS_KEY = "specify:{database}:trees:active_tree_creation" \ No newline at end of file +ACTIVE_DEFAULT_TREE_TASK_REDIS_KEY = "trees:active_tree_creation" \ No newline at end of file diff --git a/specifyweb/specify/auth/support_login.py b/specifyweb/specify/auth/support_login.py index f6175d699e7..aa05bc93392 100644 --- a/specifyweb/specify/auth/support_login.py +++ b/specifyweb/specify/auth/support_login.py @@ -17,7 +17,7 @@ from specifyweb.specify.models import Specifyuser from specifyweb.backend.workbench.upload.auditlog import str_to_bytes -from specifyweb.backend.cache.redis import set_bytes, get_bytes +from specifyweb.backend.cache.redis.connect import RedisConnection, RedisString logger = logging.getLogger(__name__) @@ -91,7 +91,8 @@ def make_token(user, key: bytes): "exp": issue_time + TTL } token: str = jwt.encode(payload, signing_key, algorithm="HS256") - set_bytes(key, salt, time_to_live=TTL) + redis_bytes = RedisString(RedisConnection(decode_responses=False)) + redis_bytes.set(key, salt, time_to_live=TTL) return token @@ -146,9 +147,10 @@ def authenticate(self, request, token=None, key=None): if token is None or key is None: return None + redis_bytes = RedisString(RedisConnection(decode_responses=False)) # A key should always be "consumed" and deleted from Redis once used: # all suport login tokens are a one-time use - salt = get_bytes(key, delete_key=True) + salt = redis_bytes.get(key, delete_key=True) if salt is None: return None diff --git a/specifyweb/specify/migrations/0043_normalize_many_to_many.py b/specifyweb/specify/migrations/0043_normalize_many_to_many.py index 2939a9032df..0390b3368b4 100644 --- a/specifyweb/specify/migrations/0043_normalize_many_to_many.py +++ b/specifyweb/specify/migrations/0043_normalize_many_to_many.py @@ -5,7 +5,7 @@ from django.db import migrations, models import specifyweb.specify.models -from specifyweb.backend.cache.redis import delete_key, redis_type, add_to_set, set_members +from specifyweb.backend.cache.redis.connect import RedisConnection, RedisSet """ WARNING: Data loss may occur if the Redis container is stopped once this @@ -213,7 +213,7 @@ } def redis_table_key(table: str): - return "specify:{database}:migration:0043:" + table + return "migration:0043:" + table results = dict() @@ -264,18 +264,20 @@ def get_existing_records(connection, table: str, table_schema) -> tuple[str, ... def store_existing_records(connection): existing_tables = tables_exist( connection, *LEGACY_MANY_TO_MANY_JOIN_TABLES.keys()) + redis_set = RedisSet(RedisConnection()) for existing_table in existing_tables: schema = LEGACY_MANY_TO_MANY_JOIN_TABLES[existing_table] - if redis_type(redis_table_key(existing_table)) != "set": - delete_key(redis_table_key(existing_table)) + if redis_set.key_type(redis_table_key(existing_table)) != 'set': + redis_set.delete(redis_table_key(existing_table)) existing_records = get_existing_records( connection, existing_table, schema) - add_to_set(redis_table_key(existing_table), *existing_records) + redis_set.add(redis_table_key(existing_table), *existing_records) def migrate_old_records(apps): + redis_set = RedisSet(RedisConnection()) for table in LEGACY_MANY_TO_MANY_JOIN_TABLES.keys(): - raw_existing_records = set_members(redis_table_key(table)) + raw_existing_records = redis_set.members(redis_table_key(table)) many_to_many_migration_schema = LEGACY_MANY_TO_MANY_JOIN_TABLES[table] app_label, model_label = many_to_many_migration_schema["to"] Model = apps.get_model(app_label, model_label) @@ -286,8 +288,9 @@ def split_iterable(iterable, chunk_size=999): yield iterable[i:i+chunk_size] def migrate_to_legacy(connection): + redis_set = RedisSet(RedisConnection()) for table, table_schema in LEGACY_MANY_TO_MANY_JOIN_TABLES.items(): - raw_existing_records = tuple(set_members(redis_table_key(table))) + raw_existing_records = tuple(redis_set.members(redis_table_key(table))) if len(raw_existing_records) <= 0: continue columns = { @@ -315,14 +318,16 @@ def migrate_to_legacy(connection): def wrapped_migrate_to_legacy(apps, schema_editor): connection = schema_editor.connection + redis_connection = RedisConnection() migrate_to_legacy(connection) for table_name in LEGACY_MANY_TO_MANY_JOIN_TABLES.keys(): - delete_key(redis_table_key(table_name)) + redis_connection.delete(redis_table_key(table_name)) def wrapped_migrate_old_records(apps, schema_editor): migrate_old_records(apps) + redis_connection = RedisConnection() for table_name in LEGACY_MANY_TO_MANY_JOIN_TABLES.keys(): - delete_key(redis_table_key(table_name)) + redis_connection.delete(redis_table_key(table_name)) def wrapped_store_records(apps, schema_editor): From fdf2b47c46914c8608a283b5356c32ed14d9c0d9 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Mon, 5 Oct 2026 18:51:14 -0500 Subject: [PATCH 09/66] feat: cache nodenumber validation result --- specifyweb/backend/delete_blockers/utils.py | 68 +++++++++++++++++++-- specifyweb/backend/trees/extras.py | 2 +- 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/specifyweb/backend/delete_blockers/utils.py b/specifyweb/backend/delete_blockers/utils.py index 9cc181bd50c..d81f2874cf2 100644 --- a/specifyweb/backend/delete_blockers/utils.py +++ b/specifyweb/backend/delete_blockers/utils.py @@ -9,18 +9,23 @@ from specifyweb.specify.models import protect_with_blockers from specifyweb.specify.api.crud import strict_get_model from specifyweb.backend.stored_queries import models as sqlmodels +from specifyweb.backend.cache.redis.connect import RedisConnection, RedisString + def field_is_remote(field: Field | ForeignObjectRel) -> TypeIs[ForeignObjectRel]: # TODO: Check whether there are any concrete fields that SHOULD be # included here, like some ToOne fields that acts as blockers return field.is_relation and not getattr(field, "concrete", True) + def relationship_blocks_deletion(relationship: ForeignObjectRel): return relationship.on_delete is protect_with_blockers or relationship.on_delete is PROTECT + def relationship_cascades_delete(relationship: ForeignObjectRel): return relationship.on_delete is CASCADE + def blocker_relationships_for_obj(obj: ModelWithTable) -> tuple[list[ForeignObjectRel], list[ForeignObjectRel]]: protect, cascade = [], [] all_fields = obj._meta.get_fields(include_hidden=True) @@ -36,26 +41,76 @@ def blocker_relationships_for_obj(obj: ModelWithTable) -> tuple[list[ForeignObje return protect, cascade -# FIXME: add redis cache here. This can move a lot of pages into and out of the -# buffer pool. Make sure to invalidate the cache on tree renumbering # REFACTOR: If we need to speed this up even more, consider using the DB cursor # directly and skip SQLAlchemy constructs (like validate_tree_numbering) +# REFACTOR: Maybe we can try to renumber the tree before this check. +# That might need to be cached as well to prevent renumbering the tree for +# every request def node_numbers_valid_for_tree(tree_name: str, definition_id: int) -> bool: + cached_value = _cached_node_numbers_valid(tree_name, definition_id) + if cached_value is not None: + return cached_value + tree_model = strict_get_model(tree_name) tree_table = tree_model.specify_model canonical_tree_name = tree_table.name tree_node = sqlmodels.models_by_tableid[tree_table.tableId] with sqlmodels.session_context() as session: - invalid_nodes_exist = _individual_nodes_invalid(tree_node, canonical_tree_name, definition_id) - nodes_in_acceptable_ranges = _node_numbers_within_parent_ranges(tree_node, canonical_tree_name, definition_id) + invalid_nodes_exist = _individual_nodes_invalid( + tree_node, canonical_tree_name, definition_id + ) + nodes_in_acceptable_ranges = _node_numbers_within_parent_ranges( + tree_node, canonical_tree_name, definition_id + ) final_query = ( union_all(invalid_nodes_exist, nodes_in_acceptable_ranges) .limit(1) ) result = session.execute(final_query).scalar_one_or_none() - return result is None + tree_is_valid = result is None + _set_cached_node_numbers_valid(tree_name, definition_id, tree_is_valid) + return tree_is_valid + -# REFACTOR: It might be better here to validate all NodeNumbers and +def _node_number_cache_key(tree_name: str, definition_id: int) -> str: + return f"{tree_name}:{definition_id}:nodenumbers:valid" + + +# The node number validation queries can push a lot of pages into the InnoDB +# Buffer Pool, potentially evicting other hot entries which would need to be +# read from disk. +# This cache helps alleviate the problem by caching the result of the node +# number validation queries to give the buffer pool some breathing room between +# validations +def _cached_node_numbers_valid(tree_name: str, definition_id: int) -> bool | None: + redis_string = RedisString(RedisConnection()) + key = _node_number_cache_key(tree_name, definition_id) + cached_value = redis_string.get(key) + if cached_value is None: + return None + return cached_value == "1" + + +# REFACTOR: We can be smarter here and update the cache when the tree is +# renumbered. This would let us set a much longer TTL. +def _set_cached_node_numbers_valid(tree_name: str, definition_id: int, is_valid: bool, time_to_live: int | None = None): + redis_string = RedisString(RedisConnection()) + key = _node_number_cache_key(tree_name, definition_id) + cache_value = "1" if is_valid else "0" + if time_to_live is None: + # If the node numbers are already valid, it's very unlikely for them to + # become invalid. + # If the node numbers are invalid, then they won't become valid until + # the tree is renumbered (through a tree operation in Specify). + # However, running the validation queries on an invalid tree can be far + # more performant than on a valid tree. + ttl = 900 if is_valid else 600 + else: + ttl = time_to_live + redis_string.set(key, cache_value, time_to_live=ttl) + + +# REFACTOR: It might be better here to validate that all NodeNumbers and # HighestChildNodeNumbers are distinct here instead. # e.g., COUNT(*) == COUNT(DISTINCT NodeNumber) == COUNT(DISTINCT HighestChildNodeNumber) def _individual_nodes_invalid(tree_model, tree_name: str, definition_id: int): @@ -77,6 +132,7 @@ def _individual_nodes_invalid(tree_model, tree_name: str, definition_id: int): ) return invalid_nodes_exist_query + def _node_numbers_within_parent_ranges(tree_model, tree_name: str, definition_id: int): # This is one of two generally efficient ways i've found to validate nodes # are within their correct ranges. diff --git a/specifyweb/backend/trees/extras.py b/specifyweb/backend/trees/extras.py index 56f52b8b1d4..637cf7b11fb 100644 --- a/specifyweb/backend/trees/extras.py +++ b/specifyweb/backend/trees/extras.py @@ -19,7 +19,7 @@ from specifyweb.backend.workbench.upload.auditcodes import TREE_BULK_MOVE, TREE_MERGE, TREE_SYNONYMIZE, TREE_DESYNONYMIZE @contextmanager -def validate_node_numbers(table, revalidate_after=True): +def validate_node_numbers(table: str, revalidate_after=True): try: validate_tree_numbering(table) except AssertionError: From 5efbf796e8030e9b6edabd0ad907dca40f9930d9 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Mon, 5 Oct 2026 19:25:00 -0500 Subject: [PATCH 10/66] feat: exclude parent relationship from optimized tree blockers --- specifyweb/backend/delete_blockers/paginate.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/specifyweb/backend/delete_blockers/paginate.py b/specifyweb/backend/delete_blockers/paginate.py index 3d74d6d6f39..0f3db57b689 100644 --- a/specifyweb/backend/delete_blockers/paginate.py +++ b/specifyweb/backend/delete_blockers/paginate.py @@ -75,10 +75,23 @@ def _get_base_queryset(self): # Essentially node numbers can "flatten" any subtree within the # tree so we don't have to worry about expensive or complicated # Parent self-joins to traverse the subtree structure - node_number_filter = { + node_number_filters = { f"{self.relationship.field.name}__nodenumber__range": (node_number, highest_node_number) + # We shouldn't need a filter on definition, as NodeNumbers should + # be unique even across trees of the same type + # TEST: Make sure we have tests for the above assumption in case + # the behavior is ever changed } - return other_side_model.objects.filter(**node_number_filter) + return other_side_model.objects.filter(**node_number_filters) + + def fetch_page(self) -> list[int]: + # With the above node number optimization, we should be able to exclude + # pages for the parent relationship for delete blockers + # REFACTOR: This might not be the best place this for this. + # Consider moving this to DeleteBlockerFilter + if self.relationship.field is self.obj._meta.get_field("parent"): + return [] + return super().fetch_page() def resolve_reference_paginator(obj: Model) -> type[ReferencePaginator]: if isinstance(obj, Tree): From 3bcebf5911094cf2c7ea0fa7c915402b26d92ef0 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 7 Oct 2026 12:20:33 -0500 Subject: [PATCH 11/66] feat: include anchor and backwards options in response --- specifyweb/backend/delete_blockers/filter.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/specifyweb/backend/delete_blockers/filter.py b/specifyweb/backend/delete_blockers/filter.py index 01567e61c22..6b2a32cfb41 100644 --- a/specifyweb/backend/delete_blockers/filter.py +++ b/specifyweb/backend/delete_blockers/filter.py @@ -16,6 +16,8 @@ class DeleteBlocker(TypedDict): ids: list[int] limit: int complete: bool + anchor: int | None + backwards: bool class DeleteBlockerCount(TypedDict): table: str @@ -91,7 +93,9 @@ def fetch_page(self) -> tuple[bool, DeleteBlocker]: "field": self.relationship.field.name, "ids": fetched_ids, "limit": self.paginator.limit, - "complete": complete + "complete": complete, + "anchor": self.paginator.anchor_id, + "backwards": self.paginator.backwards } def default_filters(obj: Model, limit=DELETE_BLOCKER_LIMIT, count_only=False) -> list[DeleteBlockerFilter]: From c4539335436340911bf8a80bd424ed51e077492c Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 7 Oct 2026 12:21:12 -0500 Subject: [PATCH 12/66] feat: implement LRU cache on frontend --- .../frontend/js_src/lib/utils/lruCache.ts | 157 ++++++++++++++++++ 1 file changed, 157 insertions(+) create mode 100644 specifyweb/frontend/js_src/lib/utils/lruCache.ts diff --git a/specifyweb/frontend/js_src/lib/utils/lruCache.ts b/specifyweb/frontend/js_src/lib/utils/lruCache.ts new file mode 100644 index 00000000000..e37f15339db --- /dev/null +++ b/specifyweb/frontend/js_src/lib/utils/lruCache.ts @@ -0,0 +1,157 @@ +import { softError } from '../components/Errors/assert'; + +const MISSING = {}; + +// FEATURE: This can be fairly easily extended to support a TTL for each entry +// Or we can import some external library +// eslint-disable-next-line functional/no-class +export class LRUCache { + private readonly cache: Map; + // eslint-disable-next-line functional/prefer-readonly-type + private maxSize: number; + // eslint-disable-next-line functional/prefer-readonly-type + private size: number; + private readonly getItemSize: ((item: V) => number) | undefined; + + public constructor({ + maxSize, + getItemSize, + }: { + readonly maxSize: number; + readonly getItemSize?: (item: V) => number; + }) { + this.cache = new Map(); + this.maxSize = maxSize; + this.size = 0; + this.getItemSize = getItemSize; + } + + /** + * Retrieve an item from the cache, marking it as recenetly used if present. + * Returns undefined if the item is not present in the cache. + */ + public get(key: K): V | undefined { + const item = this._get(key, true); + if (item === MISSING) { + return undefined; + } + return item as V; + } + + /** + * Sets an item into the cache. Will remove old entries to make space for + * this if over maxSize. + * If the key already exists in the cache, this overwrites the entry and + * marks it recently used + */ + public set(key: K, value: V): void { + this.delete(key); + const itemSize = this.freeSpaceForItem(value); + this.size += itemSize; + this.cache.set(key, value); + } + + /** + * Deletes a key from the cache. Returns true if an entry at the key was + * deleted, and false if the key was not in the cache. + */ + public delete(key: K): boolean { + // We're going to be removing this item anyways, so no need to change its + // recency + const item = this._get(key, false); + if (item === MISSING) { + // This should always return false here, but just in case + return this.cache.delete(key); + } + const itemSize = this.getCanonicalItemSize(item as V); + this.size = Math.min(0, this.size - itemSize); + return this.cache.delete(key); + } + + /** + * Removes all entries from the cache + */ + public clear() { + this.cache.clear(); + this.size = 0; + } + + /** + * Similar to get, but does not mark the entry at the key as the most + * recently accessed + */ + public peek(key: K): V | undefined { + const item = this._get(key, false); + if (item === MISSING) { + return undefined; + } + return item as V; + } + + public isFull(): boolean { + return this.getSize() >= this.maxSize; + } + + /** + * Gets the size of the cache. + * This is not necessarily the number of entries in the cache if a custom + * getItemSize function was provided at cache initialization + */ + public getSize(): number { + return this.size; + } + + /** + * Sets the new max size for the LRU cache. + * Will be enforced in the next set call + */ + public setMaxSize(newSize: number) { + this.maxSize = newSize; + } + + private _get(key: K, updateRecency: boolean = true) { + if (!this.cache.has(key)) { + return MISSING; + } + const item = this.cache.get(key) as V; + if (updateRecency) { + this.cache.delete(key); + this.cache.set(key, item); + } + return item; + } + + private getCanonicalItemSize(item: V): number { + const itemSize = + this.getItemSize === undefined ? 1 : this.getItemSize(item); + return itemSize; + } + + private freeSpaceForItem(item: V): number { + const itemSize = this.getCanonicalItemSize(item); + + if (itemSize >= this.maxSize) { + softError('Item exceeded maximum size for LRU Cache: ', item); + this.clear(); + return itemSize; + } + + if (itemSize + this.getSize() <= this.maxSize) { + // The requested size can already fit into the cache, so do nothing + return itemSize; + } + + // The new item can not fit into the cache, so we keep removing least + // accessed objects until we have enough space for the new item + const cacheKeysIterator = this.cache.keys(); + while (this.getSize() + itemSize > this.maxSize) { + const nextKey = cacheKeysIterator.next().value; + // Stop if we've hit the end of all keys in the map + if (nextKey === undefined) break; + // Otherwise remove the oldest key + this.delete(nextKey); + } + + return itemSize; + } +} From 7387f3fa93a41fa4287e8a8826da21c19eff96b1 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 7 Oct 2026 12:21:39 -0500 Subject: [PATCH 13/66] feat: implement a named promise queue on frontend --- .../frontend/js_src/lib/utils/promiseQueue.ts | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 specifyweb/frontend/js_src/lib/utils/promiseQueue.ts diff --git a/specifyweb/frontend/js_src/lib/utils/promiseQueue.ts b/specifyweb/frontend/js_src/lib/utils/promiseQueue.ts new file mode 100644 index 00000000000..3d67b9be4de --- /dev/null +++ b/specifyweb/frontend/js_src/lib/utils/promiseQueue.ts @@ -0,0 +1,74 @@ +type PromiseGenerator = () => Promise; + +// eslint-disable-next-line functional/no-class +export class PromiseQueue { + private readonly maxConcurrentPromises: number; + private readonly queue: Map>; + private readonly activePromises: Map>; + + public constructor(maxConcurrentPromises: number = 5) { + this.queue = new Map(); + this.activePromises = new Map(); + this.maxConcurrentPromises = maxConcurrentPromises; + } + + /** + * Add the promise to the Queue. If the queue has capacity to run the + * promise, it starts executing immediately. + * Otherwise the promise is held in a queue and will be executed when + * available. + * + * Returns the promise if made active and false if the promise was held in + * the queue. + */ + public enqueue(key: K, promiseGen: PromiseGenerator): Promise | false { + if (this.activePromises.size >= this.maxConcurrentPromises) { + // Add the promise to the queue + this.queue.set(key, promiseGen); + return false; + } else { + // The promise can be handled directly + return this.activatePromise(key, promiseGen); + } + } + + /** + * Returns the promise if active, false if the promise is in the queue, and + * undefined if the promise is not active or in the queue + */ + public get(key: K): Promise | false | undefined { + const activePromise = this.activePromises.get(key); + if (activePromise !== undefined) { + return activePromise; + } + const queuedPromise = this.queue.get(key); + if (queuedPromise !== undefined) { + return false; + } + return undefined; + } + + private activatePromise(key: K, promiseGen: PromiseGenerator): Promise { + const activePromise = promiseGen(); + const wrapped = this.wrapPromise(key, activePromise); + this.activePromises.set(key, wrapped); + return wrapped; + } + + private wrapPromise(key: K, promise: Promise): Promise { + return promise.finally(() => { + this.activePromises.delete(key); + this.queueNext(); + }); + } + + private queueNext() { + const firstItem = this.queue.entries().next().value; + // If there's no items in the iterator, we don't need to do anything + if (firstItem === undefined) { + return; + } + const [key, promiseGen] = firstItem; + this.activatePromise(key, promiseGen); + } +} From 157a2cd745ee85b1a8bf8888b842bcc23a756432 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 7 Oct 2026 12:22:29 -0500 Subject: [PATCH 14/66] feat: implement delete blocker cache on frontend --- .../lib/components/DeleteBlockers/index.tsx | 47 ++++++++++++++++ .../components/DeleteBlockers/pageCache.ts | 56 +++++++++++++++++++ .../lib/components/DeleteBlockers/types.ts | 19 +++++++ 3 files changed, 122 insertions(+) create mode 100644 specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx create mode 100644 specifyweb/frontend/js_src/lib/components/DeleteBlockers/pageCache.ts create mode 100644 specifyweb/frontend/js_src/lib/components/DeleteBlockers/types.ts diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx new file mode 100644 index 00000000000..64b1a41c175 --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx @@ -0,0 +1,47 @@ +import React from 'react'; +import { Button } from '../Atoms/Button'; +import { SpecifyResource } from '../DataModel/legacyTypes'; +import { AnySchema } from '../DataModel/helperTypes'; + +export function NewDeleteBlockers({ + resource, +}: { + readonly resource: SpecifyResource; +}) { + // [page, setPage] = useDeleteBlockers + // const [currentRel, setRel] = + + return ( +
+ +
{/* Paginator goes here */}
+
+ ); +} + +function DeleteBlockersAside({ + resources, +}: { + readonly resources: SpecifyResource; +}): JSX.Element { + return ( + + ); +} + +function DeleteBlockersTable(parentResource, table, rels) { + // for rel in rels: rended button to set paginator +} + +function DeleteBlockersPaginator(parentResource, rel) {} diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/pageCache.ts b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/pageCache.ts new file mode 100644 index 00000000000..2ef6a261a2e --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/pageCache.ts @@ -0,0 +1,56 @@ +import { LRUCache } from '../../utils/lrucache'; +import { RA } from '../../utils/types'; +import { AnySchema } from '../DataModel/helperTypes'; +import { SpecifyResource } from '../DataModel/legacyTypes'; +import { softFail } from '../Errors/Crash'; +import { APIDeleteBlockerPage, BlockerPageCacheKey } from './types'; + +type DeleteBlockerLRUPage = RA; + +function blockerPageToCacheKey( + owner: SpecifyResource, + page: APIDeleteBlockerPage +): BlockerPageCacheKey { + const ownerTable = owner.specifyTable.name.toLowerCase(); + const ownerId = owner.id; + if (ownerId === undefined) { + softFail('Attempting to get DeleteBlockers for record without ID'); + } + const tableName = page.table; + const relationshipName = page.field; + const anchorId = page.anchor ?? 0; + const keyParts = [ownerTable, ownerId, tableName, relationshipName, anchorId]; + return keyParts.join('_'); +} + +// eslint-disable-next-line functional/no-class +export class DeleteBlockerLRU { + private readonly pageCache: LRUCache< + BlockerPageCacheKey, + DeleteBlockerLRUPage + >; + public constructor({ maxPages = 200 }: { readonly maxPages?: number }) { + this.pageCache = new LRUCache({ maxSize: maxPages }); + } + + public cachePages( + resource: SpecifyResource, + blockerPages: RA + ) { + blockerPages.forEach((page) => this.cachePage(resource, page)); + } + + private cachePage( + resource: SpecifyResource, + blockerPage: APIDeleteBlockerPage + ) { + const cacheKey = blockerPageToCacheKey(resource, blockerPage); + this.pageCache.set(cacheKey, blockerPage.ids); + } + + public getPage( + cacheKey: BlockerPageCacheKey + ): DeleteBlockerLRUPage | undefined { + return this.pageCache.get(cacheKey); + } +} diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/types.ts b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/types.ts new file mode 100644 index 00000000000..a2766bc8f8d --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/types.ts @@ -0,0 +1,19 @@ +import { RA } from '../../utils/types'; +import { Tables } from '../DataModel/types'; + +export type APIDeleteBlockerPage = { + readonly table: Lowercase; + readonly field: string; + readonly ids: RA; + readonly limit: number; + readonly complete: boolean; + readonly anchor: null | number; + readonly backwards: boolean; +}; + +export type APIDeleteBlockers = { + readonly results: RA; + readonly next: RA; +}; + +export type BlockerPageCacheKey = string; From 44f56b3a3608b1c6a8936bd2b4946d58e9f7a916 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 7 Oct 2026 12:30:31 -0500 Subject: [PATCH 15/66] feat: add updated delete blocker fetch functions --- .../DeleteBlockers/deleteBlockers.ts | 90 +++++++++++++++++++ .../components/DeleteBlockers/pageCache.ts | 2 +- 2 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 specifyweb/frontend/js_src/lib/components/DeleteBlockers/deleteBlockers.ts diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/deleteBlockers.ts b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/deleteBlockers.ts new file mode 100644 index 00000000000..f6a56f98ccf --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/deleteBlockers.ts @@ -0,0 +1,90 @@ +import { ajax } from '../../utils/ajax'; +import { Http } from '../../utils/ajax/definitions'; +import { RA } from '../../utils/types'; +import { group } from '../../utils/utils'; +import { Tables } from '../DataModel/types'; +import { formatUrl } from '../Router/queryString'; +import { APIDeleteBlockerPage, APIDeleteBlockers } from './types'; + +const DELETE_BLOCKER_LIMIT = 40; + +type APIDeleteBlockerCount = { + readonly table: Lowercase; + readonly field: string; + readonly count: number; +}; + +type APIDeleteBlockerCounts = { + readonly results: RA; + readonly total_count: number; +}; + +type DeleteBlockerFilter = { + readonly table: keyof Tables | Lowercase; + readonly field: string; + readonly anchor?: number; + readonly limit?: number; + readonly backwards?: boolean; +}; + +export function groupBlockers(blockers: RA): RA<{ + readonly table: Lowercase; + readonly blockers: RA; +}> { + return group(blockers.map((blocker) => [blocker.table, blocker])).map( + ([table, blockers]) => ({ table, blockers }) + ); +} + +export async function fetchInitialBlockers( + table: keyof Tables, + recordId: number, + limit: number = DELETE_BLOCKER_LIMIT, + expectFailure = false +): Promise { + return ajax( + formatUrl( + `/delete_blockers/delete_blockers/${table.toLowerCase()}/${recordId}/`, + { + limit, + } + ), + { + headers: { Accept: 'application/json' }, + expectedErrors: expectFailure ? [Http.NOT_FOUND] : [], + } + ).then(({ data }) => data); +} + +export async function filterDeleteBlockers( + table: keyof Tables, + recordId: number, + filters: RA +): Promise { + return ajax( + `/delete_blockers/delete_blockers/${table.toLowerCase()}/${recordId}/`, + { + method: 'POST', + headers: { Accept: 'application/json' }, + body: filters.map((filter) => ({ + ...filter, + table: filter.table.toLowerCase(), + field: filter.field.toLowerCase(), + })), + } + ).then(({ data }) => data); +} + +export async function fetchCounts( + table: keyof Tables, + recordId: number, + expectFailure = false +): Promise { + return ajax( + `/delete_blockers/count/${table.toLowerCase()}/${recordId}/`, + { + headers: { Accept: 'application/json' }, + expectedErrors: expectFailure ? [Http.NOT_FOUND] : [], + } + ).then(({ data }) => data); +} diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/pageCache.ts b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/pageCache.ts index 2ef6a261a2e..60cd7f15b84 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/pageCache.ts +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/pageCache.ts @@ -7,7 +7,7 @@ import { APIDeleteBlockerPage, BlockerPageCacheKey } from './types'; type DeleteBlockerLRUPage = RA; -function blockerPageToCacheKey( +export function blockerPageToCacheKey( owner: SpecifyResource, page: APIDeleteBlockerPage ): BlockerPageCacheKey { From e5d000c3a8573618d972c58afec9b54c9e5f5fa5 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 7 Oct 2026 12:31:04 -0500 Subject: [PATCH 16/66] feat: implement DeleteBlockerStore to manage blocker state --- .../lib/components/DeleteBlockers/store.ts | 235 ++++++++++++++++++ 1 file changed, 235 insertions(+) create mode 100644 specifyweb/frontend/js_src/lib/components/DeleteBlockers/store.ts diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/store.ts b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/store.ts new file mode 100644 index 00000000000..1678dabc4d2 --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/store.ts @@ -0,0 +1,235 @@ +import { PromiseQueue } from '../../utils/promiseQueue'; +import { filterArray, RA } from '../../utils/types'; +import { DEFAULT_FETCH_LIMIT } from '../DataModel/collection'; +import { AnySchema } from '../DataModel/helperTypes'; +import { SpecifyResource } from '../DataModel/legacyTypes'; +import { getTable } from '../DataModel/tables'; +import { Tables } from '../DataModel/types'; +import { softFail } from '../Errors/Crash'; +import { + fetchInitialBlockers, + filterDeleteBlockers, + groupBlockers, +} from './deleteBlockers'; +import { blockerPageToCacheKey, DeleteBlockerLRU } from './pageCache'; +import { + APIDeleteBlockerPage, + APIDeleteBlockers, + BlockerPageCacheKey, +} from './types'; + +function recordToBlockerCacheKey( + resource: SpecifyResource, + relationshipInfo?: { + readonly relatedTable: string; + readonly relationshipName: string; + }, + anchorId: number = 0 +): BlockerPageCacheKey { + const tableName = resource.specifyTable.name.toLowerCase(); + const recordId = resource.id; + if (recordId === undefined) { + softFail('Attempting to get DeleteBlockers for record without ID'); + } + const relationshipTable = relationshipInfo?.relatedTable.toLowerCase(); + const relationshipName = relationshipInfo?.relationshipName.toLowerCase(); + const keyParts = filterArray([ + tableName, + recordId, + relationshipTable, + relationshipName, + anchorId, + ]); + return keyParts.join('_'); +} + +type DeleteBlockerRecord = { + readonly table: Lowercase; + readonly count?: number; + readonly relationships: RA<{ + readonly name: string; + readonly count?: number; + }>; +}; + +type DeleteBlockerRecords = { + readonly resource: SpecifyResource; + readonly count?: number; + readonly records: RA; +}; + +// eslint-disable-next-line functional/no-class +export class DeleteBlockerStore { + private readonly pageCache: DeleteBlockerLRU; + private readonly countPromiseQueue: PromiseQueue; + private readonly pagePromiseQueue: PromiseQueue; + private readonly records: Map; + private readonly recordsPerPage: number; + + public constructor({ + maxPages = 400, + recordsPerPage = DEFAULT_FETCH_LIMIT, + }: { + readonly recordsPerPage?: number; + readonly maxPages?: number; + }) { + this.recordsPerPage = recordsPerPage; + this.pageCache = new DeleteBlockerLRU({ maxPages }); + // We only allow one promise to be resolving at a time for counts, and + // three for pages. + // This should still let the browser handle other requests while fetching + // blocker information for records with a lot of references + this.countPromiseQueue = new PromiseQueue(1); + this.pagePromiseQueue = new PromiseQueue(3); + this.records = new Map(); + } + + public async seedBlockers(resource: SpecifyResource) { + const queueKey = recordToBlockerCacheKey(resource); + const promiseInQueue = this.pagePromiseQueue.get(queueKey); + if (promiseInQueue === false || promiseInQueue !== undefined) { + return; + } + this.pagePromiseQueue.enqueue(queueKey, () => + this.handleMainBlockers(resource) + ); + } + + private async handleMainBlockers(resource: SpecifyResource) { + const blockerResponse = await fetchInitialBlockers( + resource.specifyTable.name, + resource.id, + this.recordsPerPage + ); + this.handleDeleteBlockerPage(resource, blockerResponse); + } + + private handleDeleteBlockerPage( + resource: SpecifyResource, + blockers: APIDeleteBlockers + ) { + const { results, next } = blockers; + this.pageCache.cachePages(resource, results); + this.records.set(this.resourceToStringIdentifier(resource), { + resource, + records: groupBlockers(results).map((group) => ({ + table: group.table, + relationships: group.blockers.map((blocker) => ({ + name: blocker.field, + })), + })), + }); + this.queueNextBlockers(resource, next); + } + + // FIXME: add count support + public async queueBlockerCounts(resource: SpecifyResource) { + const countKey = this.resourceToStringIdentifier(resource); + const alreadyQueued = this.countPromiseQueue.get(countKey); + if (alreadyQueued === false) { + return false; + } + if (alreadyQueued !== undefined) { + return undefined; + } + } + + private resourceToStringIdentifier(resource: SpecifyResource) { + const keyParts = [resource.specifyTable.name.toLowerCase(), resource.id]; + return keyParts.join('_'); + } + + private async queueNextBlockers( + resource: SpecifyResource, + nextBlockers: RA + ) { + nextBlockers.forEach((nextBlocker) => { + this.queueNextBlocker(resource, nextBlocker); + }); + } + + private pageIsFetching(key: BlockerPageCacheKey): boolean { + const isCurrentlyFetching = this.pagePromiseQueue.get(key); + return isCurrentlyFetching !== false && isCurrentlyFetching !== undefined; + } + + private pageIsCached(key: BlockerPageCacheKey): boolean { + const isCached = this.pageCache.getPage(key); + return isCached !== undefined; + } + + private async queueNextBlocker( + resource: SpecifyResource, + nextBlocker: APIDeleteBlockerPage + ) { + const cacheKey = blockerPageToCacheKey(resource, nextBlocker); + if (this.pageIsCached(cacheKey) || this.pageIsFetching(cacheKey)) { + return; + } + const table = getTable(nextBlocker.table); + if (table === undefined) { + softFail('Unknown table for Delete Blocker'); + return; + } + for (const id of nextBlocker.ids) { + const resource = new table.Resource({ id }, { noBusinessRules: true }); + this.seedBlockers(resource); + } + // We need to handle queuing up the rest of the next blockers + if (nextBlocker.complete === false) { + this.handleNextPage(resource, nextBlocker); + } + } + + public async filterBlockers( + resource: SpecifyResource, + relatedTable: keyof Tables | Lowercase, + relationshipName: string, + anchor: number | undefined = undefined + ) { + const cacheKey = recordToBlockerCacheKey(resource, { + relatedTable, + relationshipName, + }); + if (this.pageIsFetching(cacheKey)) { + return; + } + const fetchBlockers = () => + filterDeleteBlockers(resource.specifyTable.name, resource.id, [ + { + table: relatedTable, + field: relationshipName, + anchor, + limit: this.recordsPerPage, + }, + ]).then((blockers) => this.handleDeleteBlockerPage(resource, blockers)); + this.pagePromiseQueue.enqueue(cacheKey, fetchBlockers); + } + + private async handleNextPage( + resource: SpecifyResource, + blockerPage: APIDeleteBlockerPage + ) { + if (blockerPage.complete) { + // The page we're requesting the next page from is already complete. + // Skip the request + return; + } + const newAnchor = blockerPage.ids.at(-1); + + if (newAnchor === undefined) { + // A blocker page with no IDs should be considered complete + softFail('Empty blocker with no IDs not marked complete'); + return; + } + const newPage = { + ...blockerPage, + anchor: newAnchor, + }; + const cacheKey = blockerPageToCacheKey(resource, newPage); + if (this.pageIsFetching(cacheKey)) { + return; + } + this.filterBlockers(resource, newPage.table, newPage.field, newAnchor); + } +} From 4f431d0b78d29d423a82c806bccb578fbb75a756 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 7 Oct 2026 16:17:18 -0500 Subject: [PATCH 17/66] fix: handle foreign key relationships from POST requests --- specifyweb/backend/delete_blockers/filter.py | 24 ++++---- .../backend/delete_blockers/paginate.py | 61 ++++++++++++++++--- specifyweb/backend/delete_blockers/utils.py | 16 +++-- .../backend/delete_blockers/validators.py | 6 +- 4 files changed, 82 insertions(+), 25 deletions(-) diff --git a/specifyweb/backend/delete_blockers/filter.py b/specifyweb/backend/delete_blockers/filter.py index 6b2a32cfb41..a72188bd3a6 100644 --- a/specifyweb/backend/delete_blockers/filter.py +++ b/specifyweb/backend/delete_blockers/filter.py @@ -8,7 +8,7 @@ from specifyweb.backend.delete_blockers.validators import DELETE_BLOCKER_LIMIT, CleanedDeleteBlockerFilter, CleanedDeleteBlockerRequestForm from specifyweb.backend.delete_blockers.utils import field_is_remote, relationship_blocks_deletion, relationship_cascades_delete, blocker_relationships_for_obj -from specifyweb.backend.delete_blockers.paginate import resolve_reference_paginator, ReferencePaginator +from specifyweb.backend.delete_blockers.paginate import resolve_reference_paginator, ReferencePaginator, RemoteReferencePaginator class DeleteBlocker(TypedDict): table: str @@ -42,10 +42,11 @@ def relationship(self): @property def table_name(self): - table_name = self.relationship.related_model._meta.model_name - if table_name is None: - raise ValueError(f"Unable to get related table name for {self.relationship}") - return table_name + return self.paginator.table_name + + @property + def field_name(self): + return self.paginator.field_name def cascades(self): return relationship_cascades_delete(self.relationship) @@ -58,10 +59,7 @@ def from_json(cls, obj: Model, json: CleanedDeleteBlockerFilter): raise TableDoesNotExistError(f"Unable to find {table_name}") field_name = json["field"] field = model._meta.get_field(field_name) - if (not field_is_remote(field) - or not (relationship_blocks_deletion(field) or relationship_cascades_delete(field))): - raise ValueError(f"Can not use field {table_name}.{field_name} for delete blockers") - Paginator = resolve_reference_paginator(obj) + Paginator = resolve_reference_paginator(obj, field) return cls( paginator=Paginator( obj=obj, @@ -80,7 +78,7 @@ def count(self) -> None | DeleteBlockerCount: record_count = self.paginator.count() return { "table": self.table_name, - "field": self.relationship.field.name, + "field": self.field_name, "count": record_count } @@ -90,7 +88,7 @@ def fetch_page(self) -> tuple[bool, DeleteBlocker]: cascades = self.cascades() return cascades, { "table": self.table_name, - "field": self.relationship.field.name, + "field": self.field_name, "ids": fetched_ids, "limit": self.paginator.limit, "complete": complete, @@ -101,8 +99,9 @@ def fetch_page(self) -> tuple[bool, DeleteBlocker]: def default_filters(obj: Model, limit=DELETE_BLOCKER_LIMIT, count_only=False) -> list[DeleteBlockerFilter]: filters = [] protect_rels, cascade_rels = blocker_relationships_for_obj(obj) - Paginator = resolve_reference_paginator(obj) for rel in protect_rels: + # FIXME: optimize this, shouldn't need new reference for every relaitonship + Paginator = resolve_reference_paginator(obj, rel) filters.append( DeleteBlockerFilter( Paginator( @@ -115,6 +114,7 @@ def default_filters(obj: Model, limit=DELETE_BLOCKER_LIMIT, count_only=False) -> # REFACTOR: clean this up a little if not count_only: for rel in cascade_rels: + Paginator = resolve_reference_paginator(obj, rel) filters.append( DeleteBlockerFilter( Paginator( diff --git a/specifyweb/backend/delete_blockers/paginate.py b/specifyweb/backend/delete_blockers/paginate.py index 0f3db57b689..158903a1ffe 100644 --- a/specifyweb/backend/delete_blockers/paginate.py +++ b/specifyweb/backend/delete_blockers/paginate.py @@ -1,14 +1,14 @@ -from django.db.models import Model, ForeignObjectRel, ForeignKey +from django.db.models import Model, ForeignObjectRel, ForeignKey, QuerySet, Field from specifyweb.backend.trees.extras import Tree from specifyweb.backend.stored_queries.utils import log_sqlalchemy_query from specifyweb.backend.delete_blockers.validators import DELETE_BLOCKER_LIMIT -from specifyweb.backend.delete_blockers.utils import relationship_cascades_delete, node_numbers_valid_for_tree +from specifyweb.backend.delete_blockers.utils import relationship_cascades_delete, node_numbers_valid_for_tree, field_is_remote class ReferencePaginator: def __init__(self, obj: Model, - relationship: ForeignObjectRel, + relationship: ForeignObjectRel | ForeignKey, limit: int = DELETE_BLOCKER_LIMIT, anchor_id: int | None = None, backwards: bool = False): @@ -18,6 +18,17 @@ def __init__(self, self.anchor_id = anchor_id self.backwards = backwards + def _get_base_queryset(self) -> QuerySet: + raise NotImplementedError(f"_get_base_queryset needs implemented on {self.__class__}") + + @property + def table_name(self) -> str: + raise NotImplementedError(f"table_name needs implemented on {self.__class__}") + + @property + def field_name(self) -> str: + raise NotImplementedError(f"field_name needs implemented on {self.__class__}") + def count(self) -> int: queryset = self._get_base_queryset() return queryset.count() @@ -44,13 +55,26 @@ def _get_page_queryset(self): queryset = queryset[:self.limit] return queryset +class RemoteReferencePaginator(ReferencePaginator): + def __init__(self, obj: Model, relationship: ForeignObjectRel, limit: int = DELETE_BLOCKER_LIMIT, anchor_id: int | None = None, backwards: bool = False): + super().__init__(obj, relationship, limit, anchor_id, backwards) + self.relationship = relationship + def _get_base_queryset(self): other_side_model = self.relationship.related_model return other_side_model.objects.filter( - **{self.relationship.field.name: self.obj.pk} + **{self.field_name: self.obj.pk} ) -class TreeTableReferencePaginator(ReferencePaginator): + @property + def table_name(self) -> str: + return self.relationship.related_model._meta.db_table + + @property + def field_name(self) -> str: + return self.relationship.field.name + +class TreeTableReferencePaginator(RemoteReferencePaginator): """ This paginator should be used for objects that are Tree Tables (Taxon, Geography, Storage, Lithostrat, etc.). @@ -93,10 +117,33 @@ def fetch_page(self) -> list[int]: return [] return super().fetch_page() -def resolve_reference_paginator(obj: Model) -> type[ReferencePaginator]: +class ForeignKeyPaginator(ReferencePaginator): + def __init__(self, obj: Model, relationship: ForeignKey, limit: int = DELETE_BLOCKER_LIMIT, anchor_id: int | None = None, backwards: bool = False): + super().__init__(obj, relationship, limit, anchor_id, backwards) + self.relationship = relationship + + def _get_base_queryset(self) -> QuerySet: + model = self.relationship.model + return model.objects.filter( + **{self.field_name: self.obj.pk} + ) + + @property + def table_name(self) -> str: + return self.relationship.model._meta.db_table + + @property + def field_name(self) -> str: + return self.relationship.name + +def resolve_reference_paginator(obj: Model, field: Field | ForeignObjectRel) -> type[ReferencePaginator]: if isinstance(obj, Tree): return _resolve_tree_paginator(obj) - return ReferencePaginator + elif field_is_remote(field): + return RemoteReferencePaginator + elif isinstance(field, ForeignKey): + return ForeignKeyPaginator + raise TypeError(f"Not able to resolve paginator for field: {field}") def _resolve_tree_paginator(obj: Tree) -> type[ReferencePaginator]: # This is first because it is far cheaper than checking for NodeNumber validity diff --git a/specifyweb/backend/delete_blockers/utils.py b/specifyweb/backend/delete_blockers/utils.py index d81f2874cf2..d04f9d691a2 100644 --- a/specifyweb/backend/delete_blockers/utils.py +++ b/specifyweb/backend/delete_blockers/utils.py @@ -1,6 +1,6 @@ from typing_extensions import TypeIs -from django.db.models import Field, ForeignObjectRel +from django.db.models import Field, ForeignObjectRel, ForeignKey from django.db.models.deletion import PROTECT, CASCADE from sqlalchemy import select, and_, or_, not_, union_all from sqlalchemy.orm import aliased @@ -18,13 +18,19 @@ def field_is_remote(field: Field | ForeignObjectRel) -> TypeIs[ForeignObjectRel] return field.is_relation and not getattr(field, "concrete", True) -def relationship_blocks_deletion(relationship: ForeignObjectRel): - return relationship.on_delete is protect_with_blockers or relationship.on_delete is PROTECT +def relationship_blocks_deletion(relationship: ForeignObjectRel | ForeignKey): + remote = ensure_remote(relationship) + return remote.on_delete is protect_with_blockers or remote.on_delete is PROTECT -def relationship_cascades_delete(relationship: ForeignObjectRel): - return relationship.on_delete is CASCADE +def relationship_cascades_delete(relationship: ForeignObjectRel | ForeignKey): + remote = ensure_remote(relationship) + return remote.on_delete is CASCADE +def ensure_remote(field: ForeignObjectRel | ForeignKey) -> ForeignObjectRel: + if isinstance(field, ForeignKey): + return field.remote_field + return field def blocker_relationships_for_obj(obj: ModelWithTable) -> tuple[list[ForeignObjectRel], list[ForeignObjectRel]]: protect, cascade = [], [] diff --git a/specifyweb/backend/delete_blockers/validators.py b/specifyweb/backend/delete_blockers/validators.py index ee7605d58f4..82851039ffc 100644 --- a/specifyweb/backend/delete_blockers/validators.py +++ b/specifyweb/backend/delete_blockers/validators.py @@ -5,6 +5,7 @@ DELETE_BLOCKER_LIMIT = 40 + class DeleteBlockerFilterJSON(TypedDict): table: str field: str @@ -21,6 +22,7 @@ class DeleteBlockerFilterJSON(TypedDict): anchor: NotRequired[int | None] limit: NotRequired[int] + class CleanedDeleteBlockerFilter(TypedDict): table: str field: str @@ -28,10 +30,12 @@ class CleanedDeleteBlockerFilter(TypedDict): anchor: int | None limit: int + class CleanedDeleteBlockerRequestForm(TypedDict): limit: int filters: None | list[CleanedDeleteBlockerFilter] + class DeleteBlockerRequestForm(forms.Form): limit = forms.IntegerField(required=False) @@ -64,7 +68,7 @@ def clean_filters(self): if not isinstance(filter, dict): raise ValidationError( "Filter at index '%(filter_idx)d' is not a dict", - params={"filter_idx":filter_idx} + params={"filter_idx": filter_idx} ) required_keys = {'table', 'field'} if not required_keys.issubset(filter.keys()): From c959e02b15a31c640126fa737bef7e8e17bb1bab Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 7 Oct 2026 16:20:51 -0500 Subject: [PATCH 18/66] fix: make sure to clean up pending promise queue --- specifyweb/frontend/js_src/lib/utils/promiseQueue.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/specifyweb/frontend/js_src/lib/utils/promiseQueue.ts b/specifyweb/frontend/js_src/lib/utils/promiseQueue.ts index 3d67b9be4de..1d9b97bc594 100644 --- a/specifyweb/frontend/js_src/lib/utils/promiseQueue.ts +++ b/specifyweb/frontend/js_src/lib/utils/promiseQueue.ts @@ -69,6 +69,7 @@ export class PromiseQueue { return; } const [key, promiseGen] = firstItem; + this.queue.delete(key); this.activatePromise(key, promiseGen); } } From d0f58d4de7f0870aa763a9aede9e097ac85deaf9 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 7 Oct 2026 22:34:49 -0500 Subject: [PATCH 19/66] fix: improve indexing of blocker relationships --- .../lib/components/DeleteBlockers/Context.tsx | 71 +++ .../DeleteBlockers/deleteBlockers.ts | 14 +- .../components/DeleteBlockers/pageCache.ts | 39 +- .../lib/components/DeleteBlockers/state.ts | 443 ++++++++++++++++++ .../lib/components/DeleteBlockers/store.ts | 248 ++-------- .../frontend/js_src/lib/utils/lruCache.ts | 15 +- .../frontend/js_src/lib/utils/promiseQueue.ts | 7 + 7 files changed, 607 insertions(+), 230 deletions(-) create mode 100644 specifyweb/frontend/js_src/lib/components/DeleteBlockers/Context.tsx create mode 100644 specifyweb/frontend/js_src/lib/components/DeleteBlockers/state.ts diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/Context.tsx b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/Context.tsx new file mode 100644 index 00000000000..c6322669c27 --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/Context.tsx @@ -0,0 +1,71 @@ +import React from 'react'; +import { DeleteBlockerState, resourceToStringIdentifier } from './state'; +import { error } from '../Errors/assert'; +import { SpecifyResource } from '../DataModel/legacyTypes'; +import { AnySchema } from '../DataModel/helperTypes'; +import { DeleteBlockerStore } from './store'; + +const DeleteBlockerContext = React.createContext( + null +); + +export function DeleteBlockerProvider({ + blockerStoreParams, + children, +}: { + readonly blockerStoreParams?: ConstructorParameters< + typeof DeleteBlockerState + >[0]; + readonly children: JSX.Element; +}) { + const existingStore = React.useContext(DeleteBlockerContext); + if (existingStore !== null) { + return children; + } + const deleteBlockerStore = React.useRef( + new DeleteBlockerStore(blockerStoreParams) + ); + + const store = deleteBlockerStore.current; + React.useEffect(() => { + return () => { + if (store === null) { + return; + } + store.clearState(); + }; + }, [store]); + + return ( + + {children} + + ); +} + +function useDeleteBlockerStore(): DeleteBlockerStore { + const store = React.useContext(DeleteBlockerContext); + if (store === null) { + error('Attempting to access a delete blocker store that is not created'); + } + return store; +} + +export function useDeleteBlockersForResource( + resource: SpecifyResource +) { + const store = useDeleteBlockerStore(); + const blockerKey = resourceToStringIdentifier(resource); + + React.useEffect(() => { + void store.seedBlockers(resource); + }, [store, resource]); + + React.useSyncExternalStore(store.subscribe, () => + React.useMemo( + () => store.getNode(blockerKey)?.version ?? 0, + [store, blockerKey] + ) + ); + return store.getBlockerGraph(resource); +} diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/deleteBlockers.ts b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/deleteBlockers.ts index f6a56f98ccf..8c009e57b7a 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/deleteBlockers.ts +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/deleteBlockers.ts @@ -6,7 +6,7 @@ import { Tables } from '../DataModel/types'; import { formatUrl } from '../Router/queryString'; import { APIDeleteBlockerPage, APIDeleteBlockers } from './types'; -const DELETE_BLOCKER_LIMIT = 40; +export const DELETE_BLOCKER_LIMIT = 40; type APIDeleteBlockerCount = { readonly table: Lowercase; @@ -66,11 +66,13 @@ export async function filterDeleteBlockers( { method: 'POST', headers: { Accept: 'application/json' }, - body: filters.map((filter) => ({ - ...filter, - table: filter.table.toLowerCase(), - field: filter.field.toLowerCase(), - })), + body: { + filters: filters.map((filter) => ({ + ...filter, + table: filter.table.toLowerCase(), + field: filter.field.toLowerCase(), + })), + }, } ).then(({ data }) => data); } diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/pageCache.ts b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/pageCache.ts index 60cd7f15b84..8fa89b14c04 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/pageCache.ts +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/pageCache.ts @@ -1,11 +1,15 @@ -import { LRUCache } from '../../utils/lrucache'; +import { LRUCache } from '../../utils/lruCache'; import { RA } from '../../utils/types'; import { AnySchema } from '../DataModel/helperTypes'; import { SpecifyResource } from '../DataModel/legacyTypes'; +import { Tables } from '../DataModel/types'; import { softFail } from '../Errors/Crash'; import { APIDeleteBlockerPage, BlockerPageCacheKey } from './types'; -type DeleteBlockerLRUPage = RA; +export type DeleteBlockerLRUPage = { + readonly table: Lowercase; + readonly ids: RA; +}; export function blockerPageToCacheKey( owner: SpecifyResource, @@ -29,8 +33,17 @@ export class DeleteBlockerLRU { BlockerPageCacheKey, DeleteBlockerLRUPage >; - public constructor({ maxPages = 200 }: { readonly maxPages?: number }) { - this.pageCache = new LRUCache({ maxSize: maxPages }); + public constructor({ + maxPages = 200, + onEvict = undefined, + }: { + readonly maxPages?: number; + readonly onEvict?: ( + key: BlockerPageCacheKey, + page: DeleteBlockerLRUPage + ) => void; + }) { + this.pageCache = new LRUCache({ maxSize: maxPages, onEvict }); } public cachePages( @@ -40,12 +53,16 @@ export class DeleteBlockerLRU { blockerPages.forEach((page) => this.cachePage(resource, page)); } - private cachePage( + public cachePage( resource: SpecifyResource, blockerPage: APIDeleteBlockerPage ) { const cacheKey = blockerPageToCacheKey(resource, blockerPage); - this.pageCache.set(cacheKey, blockerPage.ids); + const cachedPage: DeleteBlockerLRUPage = { + ids: blockerPage.ids, + table: blockerPage.table, + }; + this.pageCache.set(cacheKey, cachedPage); } public getPage( @@ -53,4 +70,14 @@ export class DeleteBlockerLRU { ): DeleteBlockerLRUPage | undefined { return this.pageCache.get(cacheKey); } + + public peekPage( + cacheKey: BlockerPageCacheKey + ): DeleteBlockerLRUPage | undefined { + return this.pageCache.peek(cacheKey); + } + + public clear() { + return this.pageCache.clear(); + } } diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/state.ts b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/state.ts new file mode 100644 index 00000000000..0b9291d52ce --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/state.ts @@ -0,0 +1,443 @@ +import { PromiseQueue } from '../../utils/promiseQueue'; +import { filterArray, RA, WritableArray } from '../../utils/types'; +import { AnySchema } from '../DataModel/helperTypes'; +import { SpecifyResource } from '../DataModel/legacyTypes'; +import { getTable } from '../DataModel/tables'; +import { Tables } from '../DataModel/types'; +import { softFail } from '../Errors/Crash'; +import { + DELETE_BLOCKER_LIMIT, + fetchInitialBlockers, + filterDeleteBlockers, + groupBlockers, +} from './deleteBlockers'; +import { + blockerPageToCacheKey, + DeleteBlockerLRU, + DeleteBlockerLRUPage, +} from './pageCache'; +import { + APIDeleteBlockerPage, + APIDeleteBlockers, + BlockerPageCacheKey, +} from './types'; + +export type ResourceIdentifier = string; + +function recordToBlockerCacheKey( + resource: SpecifyResource, + relationshipInfo?: { + readonly relatedTable: string; + readonly relationshipName: string; + }, + anchorId: number = 0 +): BlockerPageCacheKey { + const tableName = resource.specifyTable.name.toLowerCase(); + const recordId = resource.id; + if (recordId === undefined) { + softFail('Attempting to get DeleteBlockers for record without ID'); + } + const relationshipTable = relationshipInfo?.relatedTable.toLowerCase(); + const relationshipName = relationshipInfo?.relationshipName.toLowerCase(); + const keyParts = filterArray([ + tableName, + recordId, + relationshipTable, + relationshipName, + anchorId, + ]); + return keyParts.join('_'); +} + +type BlockerRelationship = { + readonly key: BlockerPageCacheKey; + readonly table: Lowercase; + readonly field: string; + readonly count?: number; +}; + +export type BlockerNode = { + readonly key: ResourceIdentifier; + readonly resource: SpecifyResource; + readonly count?: number; + readonly relationshipMetaData: Map; + readonly cascadeChildren: Set; + version: number; +}; + +export function resourceToStringIdentifier( + resource: SpecifyResource +): ResourceIdentifier { + return makeBlockerKey(resource.specifyTable.name.toLowerCase(), resource.id); +} + +function makeBlockerKey(...components: RA) { + return components.join('_'); +} + +// eslint-disable-next-line functional/no-class +export class DeleteBlockerState { + private readonly pageCache: DeleteBlockerLRU; + private readonly countPromiseQueue: PromiseQueue; + private readonly pagePromiseQueue: PromiseQueue< + BlockerPageCacheKey, + APIDeleteBlockers + >; + private readonly nodes: Map; + private readonly cascadeParents: Map>; + private readonly recordLocations: Map< + ResourceIdentifier, + Set + >; + private readonly recordsPerPage: number; + private readonly onChange?: () => void; + + public constructor({ + maxPages = 400, + recordsPerPage = DELETE_BLOCKER_LIMIT, + onChange = undefined, + }: { + readonly recordsPerPage?: number; + readonly maxPages?: number; + readonly onChange?: () => void; + } = {}) { + this.recordsPerPage = recordsPerPage; + this.pageCache = new DeleteBlockerLRU({ + maxPages, + onEvict: (key, page) => this.unindexBlockerPage(key, page), + }); + // We only allow one promise to be resolving at a time for counts, and + // three for pages. + // This should still let the browser handle other requests while fetching + // blocker information for records with a lot of references + this.countPromiseQueue = new PromiseQueue(1); + this.pagePromiseQueue = new PromiseQueue(3); + this.nodes = new Map(); + this.cascadeParents = new Map(); + this.recordLocations = new Map(); + this.onChange = onChange; + } + + public getNode(nodeKey: ResourceIdentifier) { + return this.nodes.get(nodeKey); + } + + public destroy() { + this.countPromiseQueue.clear(); + this.pagePromiseQueue.clear(); + this.pageCache.clear(); + this.nodes.clear(); + this.recordLocations.clear(); + this.cascadeParents.clear(); + } + + private getOrCreateNode(resource: SpecifyResource): BlockerNode { + const resourceKey = resourceToStringIdentifier(resource); + // REFACTOR: We could replace the following with getOrInsert + const existingNode = this.nodes.get(resourceKey); + if (existingNode !== undefined) { + return existingNode; + } + const newNode: BlockerNode = { + key: resourceKey, + resource, + relationshipMetaData: new Map(), + cascadeChildren: new Set(), + version: 0, + }; + this.nodes.set(resourceKey, newNode); + return newNode; + } + + private getOrCreateRelationship( + node: BlockerNode, + table: Lowercase, + field: string + ): BlockerRelationship { + const relationshipKey = this.nodeRelationshipKey(node, table, field); + const existingRelationship = node.relationshipMetaData.get(relationshipKey); + + if (existingRelationship !== undefined) { + return existingRelationship; + } + + const newRelationship: BlockerRelationship = { + key: relationshipKey, + table, + field, + }; + node.relationshipMetaData.set(relationshipKey, newRelationship); + return newRelationship; + } + + private nodeRelationshipKey( + node: BlockerNode, + table: Lowercase, + field: string + ) { + return makeBlockerKey(node.key, table, field); + } + + public async seedBlockers(resource: SpecifyResource) { + const queueKey = recordToBlockerCacheKey(resource); + const promiseInQueue = this.pagePromiseQueue.get(queueKey); + if (promiseInQueue === false || promiseInQueue !== undefined) { + return promiseInQueue; + } + return this.pagePromiseQueue.enqueue(queueKey, () => + this.handleMainBlockers(resource) + ); + } + + private async handleMainBlockers(resource: SpecifyResource) { + return fetchInitialBlockers( + resource.specifyTable.name, + resource.id, + this.recordsPerPage + ).then((blockers) => { + return this.handleDeleteBlockerPage(resource, blockers); + }); + } + + private handleDeleteBlockerPage( + resource: SpecifyResource, + blockers: APIDeleteBlockers + ) { + const node = this.getOrCreateNode(resource); + this.applyBlockerPages(node, blockers.results); + this.updateAncestors(node.key); + this.queueNextBlockers(resource, blockers.next); + this.onChange?.(); + return blockers; + } + + private applyBlockerPages( + node: BlockerNode, + blockerPages: RA + ) { + blockerPages.forEach((page) => { + this.applyBlockerPage(node, page); + }); + } + + private applyBlockerPage( + node: BlockerNode, + blockerPage: APIDeleteBlockerPage + ) { + this.getOrCreateRelationship(node, blockerPage.table, blockerPage.field); + const blockerKey = blockerPageToCacheKey(node.resource, blockerPage); + const existingPage = this.pageCache.peekPage(blockerKey); + if (existingPage !== undefined) { + this.unindexBlockerPage(blockerKey, existingPage); + } + this.pageCache.cachePage(node.resource, blockerPage); + this.indexBlockerPage(node.resource, blockerPage); + } + + private indexBlockerPage( + resource: SpecifyResource, + blockerPage: APIDeleteBlockerPage + ) { + for (const id of blockerPage.ids) { + const resourceKey = makeBlockerKey(blockerPage.table.toLowerCase(), id); + this.recordLocations + .getOrInsert(resourceKey, new Set()) + .add(blockerPageToCacheKey(resource, blockerPage)); + } + } + + private unindexBlockerPage( + pageKey: BlockerPageCacheKey, + blockerPage: DeleteBlockerLRUPage + ) { + for (const id of blockerPage.ids) { + const resourceKey = makeBlockerKey(blockerPage.table.toLowerCase(), id); + const locations = this.recordLocations.get(resourceKey); + if (locations === undefined) { + continue; + } + locations.delete(pageKey); + if (locations.size <= 0) { + this.recordLocations.delete(resourceKey); + } + } + } + + // FIXME: add count support + public async queueBlockerCounts(resource: SpecifyResource) { + const countKey = resourceToStringIdentifier(resource); + const alreadyQueued = this.countPromiseQueue.get(countKey); + if (alreadyQueued === false) { + return false; + } + if (alreadyQueued !== undefined) { + return undefined; + } + } + + public getBlockerGraph( + resource: SpecifyResource + ): RA { + const resourceKey = resourceToStringIdentifier(resource); + const result: WritableArray = []; + + this.graphIterator(resourceKey, 'children', (node) => result.push(node)); + return result; + } + + private async queueNextBlockers( + resource: SpecifyResource, + nextBlockers: RA + ) { + nextBlockers.forEach((nextBlocker) => { + this.queueNextBlocker(resource, nextBlocker); + }); + } + + private pageIsFetching(key: BlockerPageCacheKey): boolean { + const isCurrentlyFetching = this.pagePromiseQueue.get(key); + return isCurrentlyFetching !== false && isCurrentlyFetching !== undefined; + } + + private pageIsCached(key: BlockerPageCacheKey): boolean { + const isCached = this.pageCache.peekPage(key); + return isCached !== undefined; + } + + private async queueNextBlocker( + resource: SpecifyResource, + nextBlocker: APIDeleteBlockerPage + ) { + const parentNode = this.getOrCreateNode(resource); + + const table = getTable(nextBlocker.table); + if (table === undefined) { + softFail('Unknown table for Delete Blocker'); + return; + } + for (const id of nextBlocker.ids) { + const childResource = new table.Resource( + { id }, + { noBusinessRules: true } + ); + const childNode = this.getOrCreateNode(childResource); + const marked = this.addCascadeRelationship(parentNode, childNode); + if (marked) { + this.updateAncestors(parentNode.key); + } + this.seedBlockers(childResource); + } + // We need to handle queuing up the rest of the next blockers + if (nextBlocker.complete === false) { + this.handleNextPage(resource, nextBlocker); + } + } + + private addCascadeRelationship( + parentNode: BlockerNode, + childNode: BlockerNode + ): boolean { + const hasChild = parentNode.cascadeChildren.has(childNode.key); + + if (hasChild) { + return false; + } + + parentNode.cascadeChildren.add(childNode.key); + + this.cascadeParents + .getOrInsert(childNode.key, new Set()) + .add(parentNode.key); + + return true; + } + + private updateNode(node: BlockerNode) { + node.version += 1; + } + + private graphIterator( + rootKey: ResourceIdentifier, + direction: 'children' | 'ancestors', + nodeFunction: (node: BlockerNode) => void + ) { + const visitedRecords = new Set(); + const stack: WritableArray = [rootKey]; + + while (stack.length > 0) { + const currentKey = stack.pop()!; + if (visitedRecords.has(currentKey)) { + continue; + } + visitedRecords.add(currentKey); + const node = this.nodes.get(currentKey); + if (node === undefined) { + continue; + } + nodeFunction(node); + const collection = + direction === 'ancestors' + ? this.cascadeParents.get(currentKey) + : node.cascadeChildren; + for (const nextKey of collection ?? []) { + stack.push(nextKey); + } + } + } + + private updateAncestors(changedKey: ResourceIdentifier) { + this.graphIterator(changedKey, 'ancestors', this.updateNode); + } + + public async filterBlockers( + resource: SpecifyResource, + relatedTable: keyof Tables | Lowercase, + relationshipName: string, + anchor: number | undefined = undefined + ) { + const cacheKey = recordToBlockerCacheKey(resource, { + relatedTable, + relationshipName, + }); + if (this.pageIsFetching(cacheKey) || this.pageIsCached(cacheKey)) { + return; + } + const fetchBlockers = () => + filterDeleteBlockers(resource.specifyTable.name, resource.id, [ + { + table: relatedTable, + field: relationshipName, + anchor, + limit: this.recordsPerPage, + }, + ]).then((blockers) => this.handleDeleteBlockerPage(resource, blockers)); + this.pagePromiseQueue.enqueue(cacheKey, fetchBlockers); + } + + private async handleNextPage( + resource: SpecifyResource, + blockerPage: APIDeleteBlockerPage + ) { + if (blockerPage.complete) { + // The page we're requesting the next page from is already complete. + // Skip the request + return; + } + const newAnchor = blockerPage.ids.at(-1); + + if (newAnchor === undefined) { + // A blocker page with no IDs should be considered complete + softFail('Empty blocker with no IDs not marked complete'); + return; + } + const newPage = { + ...blockerPage, + anchor: newAnchor, + }; + const cacheKey = blockerPageToCacheKey(resource, newPage); + if (this.pageIsFetching(cacheKey)) { + return; + } + this.filterBlockers(resource, newPage.table, newPage.field, newAnchor); + } +} diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/store.ts b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/store.ts index 1678dabc4d2..6096a8bc6c3 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/store.ts +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/store.ts @@ -1,235 +1,55 @@ -import { PromiseQueue } from '../../utils/promiseQueue'; -import { filterArray, RA } from '../../utils/types'; -import { DEFAULT_FETCH_LIMIT } from '../DataModel/collection'; +import { RA } from '../../utils/types'; import { AnySchema } from '../DataModel/helperTypes'; import { SpecifyResource } from '../DataModel/legacyTypes'; -import { getTable } from '../DataModel/tables'; -import { Tables } from '../DataModel/types'; -import { softFail } from '../Errors/Crash'; -import { - fetchInitialBlockers, - filterDeleteBlockers, - groupBlockers, -} from './deleteBlockers'; -import { blockerPageToCacheKey, DeleteBlockerLRU } from './pageCache'; -import { - APIDeleteBlockerPage, - APIDeleteBlockers, - BlockerPageCacheKey, -} from './types'; +import { BlockerNode, DeleteBlockerState, ResourceIdentifier } from './state'; -function recordToBlockerCacheKey( - resource: SpecifyResource, - relationshipInfo?: { - readonly relatedTable: string; - readonly relationshipName: string; - }, - anchorId: number = 0 -): BlockerPageCacheKey { - const tableName = resource.specifyTable.name.toLowerCase(); - const recordId = resource.id; - if (recordId === undefined) { - softFail('Attempting to get DeleteBlockers for record without ID'); - } - const relationshipTable = relationshipInfo?.relatedTable.toLowerCase(); - const relationshipName = relationshipInfo?.relationshipName.toLowerCase(); - const keyParts = filterArray([ - tableName, - recordId, - relationshipTable, - relationshipName, - anchorId, - ]); - return keyParts.join('_'); -} - -type DeleteBlockerRecord = { - readonly table: Lowercase; - readonly count?: number; - readonly relationships: RA<{ - readonly name: string; - readonly count?: number; - }>; -}; - -type DeleteBlockerRecords = { - readonly resource: SpecifyResource; - readonly count?: number; - readonly records: RA; -}; - -// eslint-disable-next-line functional/no-class export class DeleteBlockerStore { - private readonly pageCache: DeleteBlockerLRU; - private readonly countPromiseQueue: PromiseQueue; - private readonly pagePromiseQueue: PromiseQueue; - private readonly records: Map; - private readonly recordsPerPage: number; - - public constructor({ - maxPages = 400, - recordsPerPage = DEFAULT_FETCH_LIMIT, - }: { - readonly recordsPerPage?: number; - readonly maxPages?: number; - }) { - this.recordsPerPage = recordsPerPage; - this.pageCache = new DeleteBlockerLRU({ maxPages }); - // We only allow one promise to be resolving at a time for counts, and - // three for pages. - // This should still let the browser handle other requests while fetching - // blocker information for records with a lot of references - this.countPromiseQueue = new PromiseQueue(1); - this.pagePromiseQueue = new PromiseQueue(3); - this.records = new Map(); - } - - public async seedBlockers(resource: SpecifyResource) { - const queueKey = recordToBlockerCacheKey(resource); - const promiseInQueue = this.pagePromiseQueue.get(queueKey); - if (promiseInQueue === false || promiseInQueue !== undefined) { - return; - } - this.pagePromiseQueue.enqueue(queueKey, () => - this.handleMainBlockers(resource) - ); - } + private readonly state: DeleteBlockerState; - private async handleMainBlockers(resource: SpecifyResource) { - const blockerResponse = await fetchInitialBlockers( - resource.specifyTable.name, - resource.id, - this.recordsPerPage - ); - this.handleDeleteBlockerPage(resource, blockerResponse); - } + private readonly listeners = new Set<() => void>(); - private handleDeleteBlockerPage( - resource: SpecifyResource, - blockers: APIDeleteBlockers + public constructor( + blockerStateParams: ConstructorParameters< + typeof DeleteBlockerState + >[0] = undefined ) { - const { results, next } = blockers; - this.pageCache.cachePages(resource, results); - this.records.set(this.resourceToStringIdentifier(resource), { - resource, - records: groupBlockers(results).map((group) => ({ - table: group.table, - relationships: group.blockers.map((blocker) => ({ - name: blocker.field, - })), - })), - }); - this.queueNextBlockers(resource, next); - } - - // FIXME: add count support - public async queueBlockerCounts(resource: SpecifyResource) { - const countKey = this.resourceToStringIdentifier(resource); - const alreadyQueued = this.countPromiseQueue.get(countKey); - if (alreadyQueued === false) { - return false; - } - if (alreadyQueued !== undefined) { - return undefined; - } + const params: ConstructorParameters[0] = { + ...blockerStateParams, + onChange: () => { + blockerStateParams?.onChange?.(); + this.notify(); + }, + }; + this.state = new DeleteBlockerState(params); } - private resourceToStringIdentifier(resource: SpecifyResource) { - const keyParts = [resource.specifyTable.name.toLowerCase(), resource.id]; - return keyParts.join('_'); - } + public readonly subscribe = (listener: () => void): (() => void) => { + this.listeners.add(listener); - private async queueNextBlockers( - resource: SpecifyResource, - nextBlockers: RA - ) { - nextBlockers.forEach((nextBlocker) => { - this.queueNextBlocker(resource, nextBlocker); - }); - } + return () => { + this.listeners.delete(listener); + }; + }; - private pageIsFetching(key: BlockerPageCacheKey): boolean { - const isCurrentlyFetching = this.pagePromiseQueue.get(key); - return isCurrentlyFetching !== false && isCurrentlyFetching !== undefined; + public notify(): void { + this.listeners.forEach((listener) => listener()); } - private pageIsCached(key: BlockerPageCacheKey): boolean { - const isCached = this.pageCache.getPage(key); - return isCached !== undefined; + public async seedBlockers(resource: SpecifyResource) { + return this.state.seedBlockers(resource); } - private async queueNextBlocker( - resource: SpecifyResource, - nextBlocker: APIDeleteBlockerPage - ) { - const cacheKey = blockerPageToCacheKey(resource, nextBlocker); - if (this.pageIsCached(cacheKey) || this.pageIsFetching(cacheKey)) { - return; - } - const table = getTable(nextBlocker.table); - if (table === undefined) { - softFail('Unknown table for Delete Blocker'); - return; - } - for (const id of nextBlocker.ids) { - const resource = new table.Resource({ id }, { noBusinessRules: true }); - this.seedBlockers(resource); - } - // We need to handle queuing up the rest of the next blockers - if (nextBlocker.complete === false) { - this.handleNextPage(resource, nextBlocker); - } + public getBlockerGraph( + resource: SpecifyResource + ): RA { + return this.state.getBlockerGraph(resource); } - public async filterBlockers( - resource: SpecifyResource, - relatedTable: keyof Tables | Lowercase, - relationshipName: string, - anchor: number | undefined = undefined - ) { - const cacheKey = recordToBlockerCacheKey(resource, { - relatedTable, - relationshipName, - }); - if (this.pageIsFetching(cacheKey)) { - return; - } - const fetchBlockers = () => - filterDeleteBlockers(resource.specifyTable.name, resource.id, [ - { - table: relatedTable, - field: relationshipName, - anchor, - limit: this.recordsPerPage, - }, - ]).then((blockers) => this.handleDeleteBlockerPage(resource, blockers)); - this.pagePromiseQueue.enqueue(cacheKey, fetchBlockers); + public getNode(nodeKey: ResourceIdentifier) { + return this.state.getNode(nodeKey); } - private async handleNextPage( - resource: SpecifyResource, - blockerPage: APIDeleteBlockerPage - ) { - if (blockerPage.complete) { - // The page we're requesting the next page from is already complete. - // Skip the request - return; - } - const newAnchor = blockerPage.ids.at(-1); - - if (newAnchor === undefined) { - // A blocker page with no IDs should be considered complete - softFail('Empty blocker with no IDs not marked complete'); - return; - } - const newPage = { - ...blockerPage, - anchor: newAnchor, - }; - const cacheKey = blockerPageToCacheKey(resource, newPage); - if (this.pageIsFetching(cacheKey)) { - return; - } - this.filterBlockers(resource, newPage.table, newPage.field, newAnchor); + public clearState() { + this.state.destroy(); } } diff --git a/specifyweb/frontend/js_src/lib/utils/lruCache.ts b/specifyweb/frontend/js_src/lib/utils/lruCache.ts index e37f15339db..3bf1a87dfe8 100644 --- a/specifyweb/frontend/js_src/lib/utils/lruCache.ts +++ b/specifyweb/frontend/js_src/lib/utils/lruCache.ts @@ -12,18 +12,22 @@ export class LRUCache { // eslint-disable-next-line functional/prefer-readonly-type private size: number; private readonly getItemSize: ((item: V) => number) | undefined; + private readonly onEvict?: (key: K, item: V) => void; public constructor({ maxSize, - getItemSize, + getItemSize = undefined, + onEvict = undefined, }: { readonly maxSize: number; readonly getItemSize?: (item: V) => number; + readonly onEvict?: (key: K, item: V) => void; }) { this.cache = new Map(); this.maxSize = maxSize; this.size = 0; this.getItemSize = getItemSize; + this.onEvict = onEvict; } /** @@ -65,6 +69,7 @@ export class LRUCache { } const itemSize = this.getCanonicalItemSize(item as V); this.size = Math.min(0, this.size - itemSize); + // BUG: should we call onEvict here? return this.cache.delete(key); } @@ -143,13 +148,15 @@ export class LRUCache { // The new item can not fit into the cache, so we keep removing least // accessed objects until we have enough space for the new item - const cacheKeysIterator = this.cache.keys(); + const cacheIterator = this.cache.entries(); while (this.getSize() + itemSize > this.maxSize) { - const nextKey = cacheKeysIterator.next().value; + const nextEntry = cacheIterator.next().value; // Stop if we've hit the end of all keys in the map - if (nextKey === undefined) break; + if (nextEntry === undefined) break; // Otherwise remove the oldest key + const [nextKey, nextItem] = nextEntry; this.delete(nextKey); + this.onEvict?.(nextKey, nextItem); } return itemSize; diff --git a/specifyweb/frontend/js_src/lib/utils/promiseQueue.ts b/specifyweb/frontend/js_src/lib/utils/promiseQueue.ts index 1d9b97bc594..9144eba4a69 100644 --- a/specifyweb/frontend/js_src/lib/utils/promiseQueue.ts +++ b/specifyweb/frontend/js_src/lib/utils/promiseQueue.ts @@ -48,6 +48,13 @@ export class PromiseQueue { return undefined; } + /** + * Removes all promises from the queue + */ + public clear() { + this.queue.clear(); + } + private activatePromise(key: K, promiseGen: PromiseGenerator): Promise { const activePromise = promiseGen(); const wrapped = this.wrapPromise(key, activePromise); From 35c8e8019c83ea5394a62e01a91f09085ffbe948 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 8 Oct 2026 01:11:19 -0500 Subject: [PATCH 20/66] feat: nicer external usage for delete blockers --- .../lib/components/DeleteBlockers/Context.tsx | 28 ++++-- .../lib/components/DeleteBlockers/state.ts | 16 +-- .../lib/components/DeleteBlockers/store.ts | 99 ++++++++++++++++++- 3 files changed, 117 insertions(+), 26 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/Context.tsx b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/Context.tsx index c6322669c27..242b67ced5e 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/Context.tsx +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/Context.tsx @@ -3,7 +3,8 @@ import { DeleteBlockerState, resourceToStringIdentifier } from './state'; import { error } from '../Errors/assert'; import { SpecifyResource } from '../DataModel/legacyTypes'; import { AnySchema } from '../DataModel/helperTypes'; -import { DeleteBlockerStore } from './store'; +import { DeleteBlockerResource, DeleteBlockerStore } from './store'; +import { RA } from '../../utils/types'; const DeleteBlockerContext = React.createContext( null @@ -53,19 +54,28 @@ function useDeleteBlockerStore(): DeleteBlockerStore { export function useDeleteBlockersForResource( resource: SpecifyResource -) { +): RA | undefined { const store = useDeleteBlockerStore(); const blockerKey = resourceToStringIdentifier(resource); React.useEffect(() => { void store.seedBlockers(resource); - }, [store, resource]); + }, [store, blockerKey]); - React.useSyncExternalStore(store.subscribe, () => - React.useMemo( - () => store.getNode(blockerKey)?.version ?? 0, - [store, blockerKey] - ) + const nodeVersion = React.useSyncExternalStore( + store.subscribe, + () => store.getNode(blockerKey)?.version ?? 0 ); - return store.getBlockerGraph(resource); + + const records = React.useMemo( + () => store.getBlockerGraph(resource), + [store, nodeVersion, blockerKey] + ); + + return records; } + +export function useDeleteBlockerPaginator( + parentResource: SpecifyResource, + paginatorKey: string +) {} diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/state.ts b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/state.ts index 0b9291d52ce..ec5578e1ea9 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/state.ts +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/state.ts @@ -49,7 +49,7 @@ function recordToBlockerCacheKey( return keyParts.join('_'); } -type BlockerRelationship = { +export type BlockerRelationship = { readonly key: BlockerPageCacheKey; readonly table: Lowercase; readonly field: string; @@ -71,7 +71,7 @@ export function resourceToStringIdentifier( return makeBlockerKey(resource.specifyTable.name.toLowerCase(), resource.id); } -function makeBlockerKey(...components: RA) { +export function makeBlockerKey(...components: RA) { return components.join('_'); } @@ -275,16 +275,6 @@ export class DeleteBlockerState { } } - public getBlockerGraph( - resource: SpecifyResource - ): RA { - const resourceKey = resourceToStringIdentifier(resource); - const result: WritableArray = []; - - this.graphIterator(resourceKey, 'children', (node) => result.push(node)); - return result; - } - private async queueNextBlockers( resource: SpecifyResource, nextBlockers: RA @@ -356,7 +346,7 @@ export class DeleteBlockerState { node.version += 1; } - private graphIterator( + public graphIterator( rootKey: ResourceIdentifier, direction: 'children' | 'ancestors', nodeFunction: (node: BlockerNode) => void diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/store.ts b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/store.ts index 6096a8bc6c3..40a2c7792c0 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/store.ts +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/store.ts @@ -1,7 +1,40 @@ -import { RA } from '../../utils/types'; +import { RA, RR, Writable, WritableArray } from '../../utils/types'; import { AnySchema } from '../DataModel/helperTypes'; import { SpecifyResource } from '../DataModel/legacyTypes'; -import { BlockerNode, DeleteBlockerState, ResourceIdentifier } from './state'; +import { Tables } from '../DataModel/types'; +import { + BlockerNode, + DeleteBlockerState, + makeBlockerKey, + ResourceIdentifier, + resourceToStringIdentifier, +} from './state'; +import { BlockerPageCacheKey } from './types'; + +export type DeleteBlockerRelationship = { + readonly key: BlockerPageCacheKey; + readonly relationshipName: string; + readonly count?: number; +}; + +export type DeleteBlockerTable = { + readonly tableName: string; + readonly count?: number; + readonly relationships: RA; +}; + +export type DeleteBlockerResource = { + readonly key: ResourceIdentifier; + readonly resource: SpecifyResource; + readonly count?: number; + readonly tables: RA; +}; + +type MutableDeleteBlockerTable = Writable< + Omit +> & { + relationships: Map; +}; export class DeleteBlockerStore { private readonly state: DeleteBlockerState; @@ -41,8 +74,20 @@ export class DeleteBlockerStore { public getBlockerGraph( resource: SpecifyResource - ): RA { - return this.state.getBlockerGraph(resource); + ): RA | undefined { + const identifier = resourceToStringIdentifier(resource); + const node = this.getNode(identifier); + if (node === undefined) { + return undefined; + } + const results: WritableArray = []; + this.state.graphIterator(identifier, 'children', (node) => { + const blockerResource = this.blockerNodeToResource(node); + if (blockerResource !== undefined) { + results.push(blockerResource); + } + }); + return results.length <= 0 ? undefined : results; } public getNode(nodeKey: ResourceIdentifier) { @@ -52,4 +97,50 @@ export class DeleteBlockerStore { public clearState() { this.state.destroy(); } + + private blockerNodeToResource( + node: BlockerNode + ): DeleteBlockerResource | undefined { + // The node is still be initialized + if (node.relationshipMetaData.size === 0) { + return undefined; + } + const tables = new Map(); + let resourceCount = 0; + let hasUndefinedCounts = false; + + for (const relationship of node.relationshipMetaData.values()) { + const defaultTable: MutableDeleteBlockerTable = { + tableName: relationship.table, + relationships: new Map(), + }; + const table = tables.getOrInsert(relationship.table, defaultTable); + + const relationshipKey = makeBlockerKey( + relationship.table, + relationship.field + ); + table.relationships.set(relationshipKey, { + key: relationship.key, + relationshipName: relationship.field, + count: relationship.count, + }); + if (relationship.count !== undefined) { + table.count = table.count ?? 0 + relationship.count; + resourceCount += relationship.count; + } else { + hasUndefinedCounts = true; + } + } + return { + resource: node.resource, + key: node.key, + count: hasUndefinedCounts ? undefined : resourceCount, + tables: Array.from(tables.values(), (table) => ({ + tableName: table.tableName, + count: table.count, + relationships: Array.from(table.relationships.values()), + })), + }; + } } From 71f166d48482c51e30c3b154af05ee736d179b22 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 8 Oct 2026 01:12:53 -0500 Subject: [PATCH 21/66] feat: delete blocker aside --- .../lib/components/DeleteBlockers/index.tsx | 158 ++++++++++++++++-- 1 file changed, 141 insertions(+), 17 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx index 64b1a41c175..b801d4041b3 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx @@ -2,46 +2,170 @@ import React from 'react'; import { Button } from '../Atoms/Button'; import { SpecifyResource } from '../DataModel/legacyTypes'; import { AnySchema } from '../DataModel/helperTypes'; +import { useDeleteBlockersForResource } from './Context'; +import { resourceToStringIdentifier } from './state'; +import { RA } from '../../utils/types'; +import { format } from '../Formatters/formatters'; +import { useAsyncState } from '../../hooks/useAsyncState'; +import { commonText } from '../../localization/common'; +import { useBooleanState } from '../../hooks/useBooleanState'; +import { TableIcon } from '../Molecules/TableIcon'; +import { getTable, strictGetTable } from '../DataModel/tables'; +import { + DeleteBlockerResource, + DeleteBlockerTable, + DeleteBlockerRelationship, +} from './store'; export function NewDeleteBlockers({ resource, }: { readonly resource: SpecifyResource; }) { - // [page, setPage] = useDeleteBlockers - // const [currentRel, setRel] = + const records = useDeleteBlockersForResource(resource); - return ( + const [paginatorKey, setPaginatorKey] = React.useState(records?.at(0)?.key); + + return records === undefined ? null : (
- -
{/* Paginator goes here */}
+ +
+ {paginatorKey !== undefined && ( + + )} +
); } function DeleteBlockersAside({ - resources, + records, + onRelationshipActive: handleRelationshipActive, }: { - readonly resources: SpecifyResource; + readonly records: RA; + readonly onRelationshipActive: (paginatorKey: string) => void; }): JSX.Element { return ( ); } -function DeleteBlockersTable(parentResource, table, rels) { - // for rel in rels: rended button to set paginator +function BlockerResource({ + record, + onRelationshipActive: handleRelationshipActive, +}: { + readonly record: DeleteBlockerResource; + readonly onRelationshipActive: (paginatorKey: string) => void; +}): JSX.Element { + const [formatted] = useAsyncState( + React.useCallback(() => format(record.resource), [record.resource]), + false + ); + + const [isOpen, _, __, handleToggleOpen] = useBooleanState(false); + + return ( +
+ + + + {formatted === undefined ? commonText.loading() : formatted} + + + {isOpen && ( +
+ {record.tables.map((blockerTable) => ( + + ))} +
+ )} +
+ ); } -function DeleteBlockersPaginator(parentResource, rel) {} +function DeleteBlockersTable({ + blockerTable, + onRelationshipActive: handleRelationshipActive, +}: { + readonly blockerTable: DeleteBlockerTable; + readonly onRelationshipActive: (paginatorKey: string) => void; +}) { + const [tableName, tableLabel] = React.useMemo(() => { + const table = getTable(blockerTable.tableName); + const tableName = table?.name ?? blockerTable.tableName; + const tableLabel = table?.label ?? blockerTable.tableName; + return [tableName, tableLabel]; + }, [blockerTable]); + + const [isOpen, _, __, handleToggleOpen] = useBooleanState(false); + return ( + <> + + + {tableLabel} + + {isOpen && ( +
+ {blockerTable.relationships.map((blockerRel) => ( + + ))} +
+ )} + + ); +} + +function DeleteBlockersRelationship({ + tableName, + blockerRelationship, + onRelationshipActive: handleRelationshipActive, +}: { + readonly tableName: string; + readonly blockerRelationship: DeleteBlockerRelationship; + readonly onRelationshipActive: (paginatorKey: string) => void; +}) { + const relationship = React.useMemo(() => { + const table = strictGetTable(tableName); + const field = table.strictGetField(blockerRelationship.relationshipName); + return field; + }, [tableName, blockerRelationship]); + return ( + { + handleRelationshipActive(blockerRelationship.key); + }} + > + {relationship.label} + + ); +} + +function DeleteBlockersPaginator({ + parentResource, +}: { + readonly parentResource: SpecifyResource; +}): JSX.Element {} From 90cee0d3ce2c7c5c3302b389663ebb7b0eefc288 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 8 Oct 2026 07:13:39 -0500 Subject: [PATCH 22/66] feat: smarter blocker collection for TreeDef CASCADE relationship --- specifyweb/backend/delete_blockers/filter.py | 17 ++- .../backend/delete_blockers/paginate.py | 131 +++++++++++++----- specifyweb/backend/delete_blockers/utils.py | 12 +- 3 files changed, 114 insertions(+), 46 deletions(-) diff --git a/specifyweb/backend/delete_blockers/filter.py b/specifyweb/backend/delete_blockers/filter.py index a72188bd3a6..ca62de4263a 100644 --- a/specifyweb/backend/delete_blockers/filter.py +++ b/specifyweb/backend/delete_blockers/filter.py @@ -8,7 +8,8 @@ from specifyweb.backend.delete_blockers.validators import DELETE_BLOCKER_LIMIT, CleanedDeleteBlockerFilter, CleanedDeleteBlockerRequestForm from specifyweb.backend.delete_blockers.utils import field_is_remote, relationship_blocks_deletion, relationship_cascades_delete, blocker_relationships_for_obj -from specifyweb.backend.delete_blockers.paginate import resolve_reference_paginator, ReferencePaginator, RemoteReferencePaginator +from specifyweb.backend.delete_blockers.paginate import resolve_reference_paginator, ReferencePaginator + class DeleteBlocker(TypedDict): table: str @@ -19,15 +20,18 @@ class DeleteBlocker(TypedDict): anchor: int | None backwards: bool + class DeleteBlockerCount(TypedDict): table: str field: str count: int + class DeleteBlockerCounts(TypedDict): results: list[DeleteBlockerCount] total_count: int + class DeleteBlockerFilter: def __init__(self, paginator: ReferencePaginator) -> None: self.paginator = paginator @@ -96,6 +100,7 @@ def fetch_page(self) -> tuple[bool, DeleteBlocker]: "backwards": self.paginator.backwards } + def default_filters(obj: Model, limit=DELETE_BLOCKER_LIMIT, count_only=False) -> list[DeleteBlockerFilter]: filters = [] protect_rels, cascade_rels = blocker_relationships_for_obj(obj) @@ -126,6 +131,7 @@ def default_filters(obj: Model, limit=DELETE_BLOCKER_LIMIT, count_only=False) -> ) return filters + def merge_delete_blocker_counts(aggregated: DeleteBlockerCounts, current_count: DeleteBlockerCount | None) -> DeleteBlockerCounts: if current_count is None or current_count["count"] == 0: return aggregated @@ -135,15 +141,15 @@ def merge_delete_blocker_counts(aggregated: DeleteBlockerCounts, current_count: "results": new_results, "total_count": new_count } - + class DeleteBlockerFilters: def __init__(self, obj: Model, limit=DELETE_BLOCKER_LIMIT, filters: list[DeleteBlockerFilter] | None = None, - count_only = False - ) -> None: + count_only=False + ) -> None: self.obj = obj if filters is None: self.filters = default_filters(obj, limit, count_only) @@ -155,7 +161,8 @@ def from_json(cls, obj: Model, json: CleanedDeleteBlockerRequestForm, count_only if json["filters"] is None: filters = None else: - filters = [DeleteBlockerFilter.from_json(obj=obj, json=blocker_filter) for blocker_filter in json["filters"]] + filters = [DeleteBlockerFilter.from_json( + obj=obj, json=blocker_filter) for blocker_filter in json["filters"]] limit = json["limit"] return cls(obj=obj, limit=limit, filters=filters, count_only=count_only) diff --git a/specifyweb/backend/delete_blockers/paginate.py b/specifyweb/backend/delete_blockers/paginate.py index 158903a1ffe..f6cd278754b 100644 --- a/specifyweb/backend/delete_blockers/paginate.py +++ b/specifyweb/backend/delete_blockers/paginate.py @@ -1,13 +1,16 @@ -from django.db.models import Model, ForeignObjectRel, ForeignKey, QuerySet, Field +from django.db.models import Model, ForeignObjectRel, ForeignKey, QuerySet -from specifyweb.backend.trees.extras import Tree +from specifyweb.backend.trees.extras import Tree, TreeRank from specifyweb.backend.stored_queries.utils import log_sqlalchemy_query from specifyweb.backend.delete_blockers.validators import DELETE_BLOCKER_LIMIT from specifyweb.backend.delete_blockers.utils import relationship_cascades_delete, node_numbers_valid_for_tree, field_is_remote + class ReferencePaginator: def __init__(self, obj: Model, + # REFACTOR: We should be able to remove the ForeignObjectRel + # type relationship: ForeignObjectRel | ForeignKey, limit: int = DELETE_BLOCKER_LIMIT, anchor_id: int | None = None, @@ -19,15 +22,18 @@ def __init__(self, self.backwards = backwards def _get_base_queryset(self) -> QuerySet: - raise NotImplementedError(f"_get_base_queryset needs implemented on {self.__class__}") + raise NotImplementedError( + f"_get_base_queryset needs implemented on {self.__class__}") @property def table_name(self) -> str: - raise NotImplementedError(f"table_name needs implemented on {self.__class__}") + raise NotImplementedError( + f"table_name needs implemented on {self.__class__}") @property def field_name(self) -> str: - raise NotImplementedError(f"field_name needs implemented on {self.__class__}") + raise NotImplementedError( + f"field_name needs implemented on {self.__class__}") def count(self) -> int: queryset = self._get_base_queryset() @@ -55,26 +61,28 @@ def _get_page_queryset(self): queryset = queryset[:self.limit] return queryset -class RemoteReferencePaginator(ReferencePaginator): - def __init__(self, obj: Model, relationship: ForeignObjectRel, limit: int = DELETE_BLOCKER_LIMIT, anchor_id: int | None = None, backwards: bool = False): + +class ForeignKeyPaginator(ReferencePaginator): + def __init__(self, obj: Model, relationship: ForeignKey, limit: int = DELETE_BLOCKER_LIMIT, anchor_id: int | None = None, backwards: bool = False): super().__init__(obj, relationship, limit, anchor_id, backwards) self.relationship = relationship - def _get_base_queryset(self): - other_side_model = self.relationship.related_model - return other_side_model.objects.filter( + def _get_base_queryset(self) -> QuerySet: + model = self.relationship.model + return model.objects.filter( **{self.field_name: self.obj.pk} ) @property def table_name(self) -> str: - return self.relationship.related_model._meta.db_table + return self.relationship.model._meta.db_table @property def field_name(self) -> str: - return self.relationship.field.name + return self.relationship.name + -class TreeTableReferencePaginator(RemoteReferencePaginator): +class TreeTableReferencePaginator(ForeignKeyPaginator): """ This paginator should be used for objects that are Tree Tables (Taxon, Geography, Storage, Lithostrat, etc.). @@ -87,10 +95,11 @@ class TreeTableReferencePaginator(RemoteReferencePaginator): Use the base ReferencePaginator if you can not guarantee all of the above properties. """ + def _get_base_queryset(self): node_number = self.obj.nodenumber highest_node_number = self.obj.highestchildnodenumber - other_side_model = self.relationship.related_model + model = self.relationship.model # With the assumption that nodenumbers are up-to-date and correctly # formed for the tree, we can use their properties to optimize fetching # references. @@ -100,60 +109,106 @@ def _get_base_queryset(self): # tree so we don't have to worry about expensive or complicated # Parent self-joins to traverse the subtree structure node_number_filters = { - f"{self.relationship.field.name}__nodenumber__range": (node_number, highest_node_number) + f"{self.relationship.name}__nodenumber__range": (node_number, highest_node_number) # We shouldn't need a filter on definition, as NodeNumbers should # be unique even across trees of the same type # TEST: Make sure we have tests for the above assumption in case # the behavior is ever changed } - return other_side_model.objects.filter(**node_number_filters) + return model.objects.filter(**node_number_filters) def fetch_page(self) -> list[int]: # With the above node number optimization, we should be able to exclude # pages for the parent relationship for delete blockers # REFACTOR: This might not be the best place this for this. # Consider moving this to DeleteBlockerFilter - if self.relationship.field is self.obj._meta.get_field("parent"): + if self.relationship is self.obj._meta.get_field("parent"): return [] return super().fetch_page() -class ForeignKeyPaginator(ReferencePaginator): - def __init__(self, obj: Model, relationship: ForeignKey, limit: int = DELETE_BLOCKER_LIMIT, anchor_id: int | None = None, backwards: bool = False): - super().__init__(obj, relationship, limit, anchor_id, backwards) - self.relationship = relationship +class TreeNodeToDefinitionPaginator(ForeignKeyPaginator): def _get_base_queryset(self) -> QuerySet: model = self.relationship.model return model.objects.filter( - **{self.field_name: self.obj.pk} + **{self.field_name: self.obj.pk}, + parent__isnull=True ) - @property - def table_name(self) -> str: - return self.relationship.model._meta.db_table - @property - def field_name(self) -> str: - return self.relationship.name +class TreeRankToDefinitionPaginator(ForeignKeyPaginator): + def _get_base_queryset(self) -> QuerySet: + model = self.relationship.model + return model.objects.filter( + **{self.field_name: self.obj.pk}, + parent__isnull=True + ) -def resolve_reference_paginator(obj: Model, field: Field | ForeignObjectRel) -> type[ReferencePaginator]: - if isinstance(obj, Tree): - return _resolve_tree_paginator(obj) - elif field_is_remote(field): - return RemoteReferencePaginator - elif isinstance(field, ForeignKey): + +def _resolve_paginator_by_field(field: ForeignKey) -> type[ReferencePaginator] | None: + if isinstance(field, ForeignKey): return ForeignKeyPaginator + return None + + +def is_tree_definition(obj: Model): + db_table = obj._meta.db_table.lower() + return db_table.endswith("treedef") + + +def resolve_reference_paginator(obj: Model, field: ForeignKey) -> type[ReferencePaginator]: + Paginator = None + if isinstance(obj, Tree): + Paginator = _resolve_tree_node_paginator(obj, field) + elif is_tree_definition(obj): + Paginator = _resolve_tree_definition_paginator(obj, field) + + if Paginator is None: + Paginator = _resolve_paginator_by_field(field) + + if Paginator is not None: + return Paginator raise TypeError(f"Not able to resolve paginator for field: {field}") -def _resolve_tree_paginator(obj: Tree) -> type[ReferencePaginator]: + +def _resolve_tree_node_paginator(obj: Tree, field: ForeignKey) -> type[ReferencePaginator] | None: # This is first because it is far cheaper than checking for NodeNumber validity if not _tree_cascades_delete(obj.__class__): - return ReferencePaginator + return None definition_id = obj.definition_id - node_numbers_valid = node_numbers_valid_for_tree(obj._meta.db_table, definition_id) + node_numbers_valid = node_numbers_valid_for_tree( + obj._meta.db_table, definition_id) if node_numbers_valid: return TreeTableReferencePaginator - return ReferencePaginator + return None + + +def _resolve_tree_definition_paginator(obj: Model, field: ForeignKey) -> type[ReferencePaginator] | None: + if not isinstance(field, ForeignKey): + return None + + # These Paginators should only apply when the field CASCADEs delete + if not relationship_cascades_delete(field): + return None + + tree_name = obj._meta.db_table.lower().replace("treedef", "") + + field_name = field.name.lower() + + if field_name.lower() not in ("definition", "treedef"): + return None + + tree_is_valid = node_numbers_valid_for_tree(tree_name, obj.pk) + + if not tree_is_valid: + return None + + if issubclass(field.model, Tree) and field_name == "definition": + return TreeNodeToDefinitionPaginator + elif issubclass(field.model, TreeRank) and field_name == "treedef": + return TreeRankToDefinitionPaginator + return None + def _tree_cascades_delete(obj: type[Tree]): parent_field = obj._meta.get_field("parent") diff --git a/specifyweb/backend/delete_blockers/utils.py b/specifyweb/backend/delete_blockers/utils.py index d04f9d691a2..14eee02cd2c 100644 --- a/specifyweb/backend/delete_blockers/utils.py +++ b/specifyweb/backend/delete_blockers/utils.py @@ -27,12 +27,14 @@ def relationship_cascades_delete(relationship: ForeignObjectRel | ForeignKey): remote = ensure_remote(relationship) return remote.on_delete is CASCADE + def ensure_remote(field: ForeignObjectRel | ForeignKey) -> ForeignObjectRel: if isinstance(field, ForeignKey): return field.remote_field return field -def blocker_relationships_for_obj(obj: ModelWithTable) -> tuple[list[ForeignObjectRel], list[ForeignObjectRel]]: + +def blocker_relationships_for_obj(obj: ModelWithTable) -> tuple[list[ForeignKey], list[ForeignKey]]: protect, cascade = [], [] all_fields = obj._meta.get_fields(include_hidden=True) remote_relationships = filter( @@ -40,10 +42,12 @@ def blocker_relationships_for_obj(obj: ModelWithTable) -> tuple[list[ForeignObje all_fields ) for relationship in remote_relationships: + if not isinstance(relationship.field, ForeignKey): + continue if relationship_blocks_deletion(relationship): - protect.append(relationship) + protect.append(relationship.field) elif relationship_cascades_delete(relationship): - cascade.append(relationship) + cascade.append(relationship.field) return protect, cascade @@ -52,6 +56,8 @@ def blocker_relationships_for_obj(obj: ModelWithTable) -> tuple[list[ForeignObje # REFACTOR: Maybe we can try to renumber the tree before this check. # That might need to be cached as well to prevent renumbering the tree for # every request + + def node_numbers_valid_for_tree(tree_name: str, definition_id: int) -> bool: cached_value = _cached_node_numbers_valid(tree_name, definition_id) if cached_value is not None: From 13303a896714dc77d10b26c52f5618baa7c704e9 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 8 Oct 2026 07:16:45 -0500 Subject: [PATCH 23/66] chore: change placement of comment --- specifyweb/backend/delete_blockers/utils.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/specifyweb/backend/delete_blockers/utils.py b/specifyweb/backend/delete_blockers/utils.py index 14eee02cd2c..1e7740dc317 100644 --- a/specifyweb/backend/delete_blockers/utils.py +++ b/specifyweb/backend/delete_blockers/utils.py @@ -51,14 +51,13 @@ def blocker_relationships_for_obj(obj: ModelWithTable) -> tuple[list[ForeignKey] return protect, cascade -# REFACTOR: If we need to speed this up even more, consider using the DB cursor -# directly and skip SQLAlchemy constructs (like validate_tree_numbering) -# REFACTOR: Maybe we can try to renumber the tree before this check. -# That might need to be cached as well to prevent renumbering the tree for -# every request - def node_numbers_valid_for_tree(tree_name: str, definition_id: int) -> bool: + # REFACTOR: If we need to speed this up even more, consider using the DB cursor + # directly and skip SQLAlchemy constructs (like validate_tree_numbering) + # REFACTOR: Maybe we can try to renumber the tree before this check. + # That might need to be cached as well to prevent renumbering the tree for + # every request cached_value = _cached_node_numbers_valid(tree_name, definition_id) if cached_value is not None: return cached_value From 609e9c29b97f55e04df29c088cc6158a3f8a434b Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 8 Oct 2026 12:00:21 -0500 Subject: [PATCH 24/66] feat: implement pagination hook for DeleteBlockers --- .../lib/components/DeleteBlockers/Context.tsx | 42 +++- .../DeleteBlockers/deleteBlockers.ts | 2 +- .../lib/components/DeleteBlockers/index.tsx | 75 +++++-- .../components/DeleteBlockers/pageCache.ts | 7 +- .../lib/components/DeleteBlockers/state.ts | 65 ++++-- .../lib/components/DeleteBlockers/store.ts | 49 +++++ .../lib/components/FormSliders/LazySlider.tsx | 65 ++++++ .../FormSliders/RecordSelectorFromPage.tsx | 195 ++++++++++++++++++ 8 files changed, 460 insertions(+), 40 deletions(-) create mode 100644 specifyweb/frontend/js_src/lib/components/FormSliders/LazySlider.tsx create mode 100644 specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromPage.tsx diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/Context.tsx b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/Context.tsx index 242b67ced5e..6b2fa4d964f 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/Context.tsx +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/Context.tsx @@ -5,6 +5,8 @@ import { SpecifyResource } from '../DataModel/legacyTypes'; import { AnySchema } from '../DataModel/helperTypes'; import { DeleteBlockerResource, DeleteBlockerStore } from './store'; import { RA } from '../../utils/types'; +import { Relationship } from '../DataModel/specifyField'; +import { DeleteBlockerLRUPage } from './pageCache'; const DeleteBlockerContext = React.createContext( null @@ -75,7 +77,39 @@ export function useDeleteBlockersForResource( return records; } -export function useDeleteBlockerPaginator( - parentResource: SpecifyResource, - paginatorKey: string -) {} +export function useDeleteBlockerPages( + resource: SpecifyResource, + relationship: Relationship +) { + const store = useDeleteBlockerStore(); + + const [anchor, setAnchor] = React.useState(null); + + React.useEffect(() => { + store.filterBlockers(resource, relationship, anchor); + }, [store, resource, relationship]); + + async function handleNextPageFetch( + previousPage: DeleteBlockerLRUPage, + direction: 'next' | 'previous' | 'first' | 'last' + ) { + const backwards = direction === 'previous' || direction === 'last'; + const anchor = + direction === 'next' + ? previousPage.ids.at(-1) + : direction === 'previous' + ? previousPage.ids.at(0) + : null; + setAnchor(anchor ?? null); + store.filterBlockers(resource, relationship, anchor, backwards); + } + + const deleteBlockerPage = React.useSyncExternalStore(store.subscribe, () => + store.getCachedPage(resource, relationship, anchor) + ); + + return { + page: deleteBlockerPage, + onNextPageFetch: handleNextPageFetch, + }; +} diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/deleteBlockers.ts b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/deleteBlockers.ts index 8c009e57b7a..e69954fe913 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/deleteBlockers.ts +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/deleteBlockers.ts @@ -22,7 +22,7 @@ type APIDeleteBlockerCounts = { type DeleteBlockerFilter = { readonly table: keyof Tables | Lowercase; readonly field: string; - readonly anchor?: number; + readonly anchor?: number | null; readonly limit?: number; readonly backwards?: boolean; }; diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx index b801d4041b3..b2abd227378 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx @@ -2,8 +2,8 @@ import React from 'react'; import { Button } from '../Atoms/Button'; import { SpecifyResource } from '../DataModel/legacyTypes'; import { AnySchema } from '../DataModel/helperTypes'; -import { useDeleteBlockersForResource } from './Context'; -import { resourceToStringIdentifier } from './state'; +import { useDeleteBlockerPages, useDeleteBlockersForResource } from './Context'; +import { ResourceIdentifier, resourceToStringIdentifier } from './state'; import { RA } from '../../utils/types'; import { format } from '../Formatters/formatters'; import { useAsyncState } from '../../hooks/useAsyncState'; @@ -11,11 +11,15 @@ import { commonText } from '../../localization/common'; import { useBooleanState } from '../../hooks/useBooleanState'; import { TableIcon } from '../Molecules/TableIcon'; import { getTable, strictGetTable } from '../DataModel/tables'; +import { localized } from '../../utils/types'; import { DeleteBlockerResource, DeleteBlockerTable, DeleteBlockerRelationship, } from './store'; +import { RecordSelectorFromPage } from '../FormSliders/RecordSelectorFromPage'; +import { f } from '../../utils/functools'; +import { Relationship } from '../DataModel/specifyField'; export function NewDeleteBlockers({ resource, @@ -24,17 +28,31 @@ export function NewDeleteBlockers({ }) { const records = useDeleteBlockersForResource(resource); - const [paginatorKey, setPaginatorKey] = React.useState(records?.at(0)?.key); + const [paginatorKey, setPaginatorKey] = React.useState< + | undefined + | { + readonly resource: SpecifyResource; + readonly relationship: Relationship; + } + >(undefined); return records === undefined ? null : (
+ setPaginatorKey(() => ({ + resource, + relationship, + })) + } />
{paginatorKey !== undefined && ( - + )}
@@ -46,7 +64,10 @@ function DeleteBlockersAside({ onRelationshipActive: handleRelationshipActive, }: { readonly records: RA; - readonly onRelationshipActive: (paginatorKey: string) => void; + readonly onRelationshipActive: ( + resource: SpecifyResource, + relationship: Relationship + ) => void; }): JSX.Element { return (
@@ -108,7 +134,7 @@ function DeleteBlockersTable({ onRelationshipActive: handleRelationshipActive, }: { readonly blockerTable: DeleteBlockerTable; - readonly onRelationshipActive: (paginatorKey: string) => void; + readonly onRelationshipActive: (relationship: Relationship) => void; }) { const [tableName, tableLabel] = React.useMemo(() => { const table = getTable(blockerTable.tableName); @@ -122,7 +148,7 @@ function DeleteBlockersTable({ <> - {tableLabel} + {localized(tableLabel)} {isOpen && (
@@ -146,17 +172,19 @@ function DeleteBlockersRelationship({ }: { readonly tableName: string; readonly blockerRelationship: DeleteBlockerRelationship; - readonly onRelationshipActive: (paginatorKey: string) => void; + readonly onRelationshipActive: (relationship: Relationship) => void; }) { const relationship = React.useMemo(() => { const table = strictGetTable(tableName); - const field = table.strictGetField(blockerRelationship.relationshipName); + const field = table.strictGetRelationship( + blockerRelationship.relationshipName + ); return field; }, [tableName, blockerRelationship]); return ( { - handleRelationshipActive(blockerRelationship.key); + handleRelationshipActive(relationship); }} > {relationship.label} @@ -166,6 +194,25 @@ function DeleteBlockersRelationship({ function DeleteBlockersPaginator({ parentResource, + relationship, }: { readonly parentResource: SpecifyResource; -}): JSX.Element {} + readonly relationship: Relationship; +}): JSX.Element { + const { page, onNextPageFetch: handleNextPageFetch } = useDeleteBlockerPages( + parentResource, + relationship + ); + return ( + + ); +} diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/pageCache.ts b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/pageCache.ts index 8fa89b14c04..d6b698044ae 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/pageCache.ts +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/pageCache.ts @@ -9,6 +9,8 @@ import { APIDeleteBlockerPage, BlockerPageCacheKey } from './types'; export type DeleteBlockerLRUPage = { readonly table: Lowercase; readonly ids: RA; + readonly anchor: number | null; + readonly backwards: boolean; }; export function blockerPageToCacheKey( @@ -22,7 +24,7 @@ export function blockerPageToCacheKey( } const tableName = page.table; const relationshipName = page.field; - const anchorId = page.anchor ?? 0; + const anchorId = page.anchor; const keyParts = [ownerTable, ownerId, tableName, relationshipName, anchorId]; return keyParts.join('_'); } @@ -61,8 +63,11 @@ export class DeleteBlockerLRU { const cachedPage: DeleteBlockerLRUPage = { ids: blockerPage.ids, table: blockerPage.table, + anchor: blockerPage.anchor, + backwards: blockerPage.backwards, }; this.pageCache.set(cacheKey, cachedPage); + return cachedPage; } public getPage( diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/state.ts b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/state.ts index ec5578e1ea9..66a6240a55a 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/state.ts +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/state.ts @@ -24,13 +24,13 @@ import { export type ResourceIdentifier = string; -function recordToBlockerCacheKey( +export function recordToBlockerCacheKey( resource: SpecifyResource, relationshipInfo?: { readonly relatedTable: string; readonly relationshipName: string; }, - anchorId: number = 0 + anchorId: number | null = null ): BlockerPageCacheKey { const tableName = resource.specifyTable.name.toLowerCase(); const recordId = resource.id; @@ -81,7 +81,7 @@ export class DeleteBlockerState { private readonly countPromiseQueue: PromiseQueue; private readonly pagePromiseQueue: PromiseQueue< BlockerPageCacheKey, - APIDeleteBlockers + RA >; private readonly nodes: Map; private readonly cascadeParents: Map>; @@ -202,22 +202,20 @@ export class DeleteBlockerState { private handleDeleteBlockerPage( resource: SpecifyResource, blockers: APIDeleteBlockers - ) { + ): RA { const node = this.getOrCreateNode(resource); - this.applyBlockerPages(node, blockers.results); + const cachedPages = this.applyBlockerPages(node, blockers.results); this.updateAncestors(node.key); this.queueNextBlockers(resource, blockers.next); this.onChange?.(); - return blockers; + return cachedPages; } private applyBlockerPages( node: BlockerNode, blockerPages: RA - ) { - blockerPages.forEach((page) => { - this.applyBlockerPage(node, page); - }); + ): RA { + return blockerPages.map((page) => this.applyBlockerPage(node, page)); } private applyBlockerPage( @@ -230,8 +228,9 @@ export class DeleteBlockerState { if (existingPage !== undefined) { this.unindexBlockerPage(blockerKey, existingPage); } - this.pageCache.cachePage(node.resource, blockerPage); + const cachedPage = this.pageCache.cachePage(node.resource, blockerPage); this.indexBlockerPage(node.resource, blockerPage); + return cachedPage; } private indexBlockerPage( @@ -379,18 +378,30 @@ export class DeleteBlockerState { this.graphIterator(changedKey, 'ancestors', this.updateNode); } + public getBlockerPage(cacheKey: string) { + return this.pageCache.getPage(cacheKey); + } + public async filterBlockers( resource: SpecifyResource, relatedTable: keyof Tables | Lowercase, relationshipName: string, - anchor: number | undefined = undefined + anchor: number | null = null, + backwards: boolean = false ) { - const cacheKey = recordToBlockerCacheKey(resource, { - relatedTable, - relationshipName, - }); - if (this.pageIsFetching(cacheKey) || this.pageIsCached(cacheKey)) { - return; + const cacheKey = recordToBlockerCacheKey( + resource, + { + relatedTable, + relationshipName, + }, + anchor + ); + if (this.pageIsCached(cacheKey)) { + return this.getBlockerPage(cacheKey); + } + if (this.pageIsFetching(cacheKey)) { + return undefined; } const fetchBlockers = () => filterDeleteBlockers(resource.specifyTable.name, resource.id, [ @@ -398,10 +409,24 @@ export class DeleteBlockerState { table: relatedTable, field: relationshipName, anchor, + backwards, limit: this.recordsPerPage, }, - ]).then((blockers) => this.handleDeleteBlockerPage(resource, blockers)); - this.pagePromiseQueue.enqueue(cacheKey, fetchBlockers); + ]) + .then((blockers) => { + return backwards === true + ? { + ...blockers, + results: blockers.results.map((page) => ({ + ...page, + ids: [...page.ids].reverse(), + backwards: false, + })), + } + : blockers; + }) + .then((blockers) => this.handleDeleteBlockerPage(resource, blockers)); + return this.pagePromiseQueue.enqueue(cacheKey, fetchBlockers); } private async handleNextPage( diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/store.ts b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/store.ts index 40a2c7792c0..4f6adb38048 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/store.ts +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/store.ts @@ -1,11 +1,14 @@ import { RA, RR, Writable, WritableArray } from '../../utils/types'; import { AnySchema } from '../DataModel/helperTypes'; import { SpecifyResource } from '../DataModel/legacyTypes'; +import { Relationship } from '../DataModel/specifyField'; import { Tables } from '../DataModel/types'; +import { DeleteBlockerLRUPage } from './pageCache'; import { BlockerNode, DeleteBlockerState, makeBlockerKey, + recordToBlockerCacheKey, ResourceIdentifier, resourceToStringIdentifier, } from './state'; @@ -72,6 +75,52 @@ export class DeleteBlockerStore { return this.state.seedBlockers(resource); } + public fetchDeleteBlockerPage( + resource: SpecifyResource, + relationship: Relationship, + anchor: number | null = null, + backwards: boolean = false + ) { + return this.state.filterBlockers( + resource, + relationship.table.name, + relationship.name, + anchor, + backwards + ); + } + + public filterBlockers( + resource: SpecifyResource, + relationship: Relationship, + anchor: number | null = null, + backwards: boolean = false + ) { + return this.state.filterBlockers( + resource, + relationship.table.name, + relationship.name, + anchor, + backwards + ); + } + + public getCachedPage( + resource: SpecifyResource, + relationship: Relationship, + anchor: number | null = null + ): DeleteBlockerLRUPage | undefined { + const cacheKey = recordToBlockerCacheKey( + resource, + { + relatedTable: relationship.table.name, + relationshipName: relationship.name, + }, + anchor + ); + return this.state.getBlockerPage(cacheKey); + } + public getBlockerGraph( resource: SpecifyResource ): RA | undefined { diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/LazySlider.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/LazySlider.tsx new file mode 100644 index 00000000000..6e3bb4dc708 --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/LazySlider.tsx @@ -0,0 +1,65 @@ +import React from 'react'; +import { formsText } from '../../localization/forms'; +import { Button } from '../Atoms/Button'; +import { icons } from '../Atoms/Icons'; + +type DisabledProps = { + readonly first?: boolean; + readonly next?: boolean; + readonly previous?: boolean; + readonly last?: boolean; +}; + +export function LazySlider({ + buttonsDisabled = { + first: false, + next: false, + previous: false, + last: false, + }, + onChange: handleChange, +}: { + readonly buttonsDisabled?: DisabledProps; + readonly onChange: + | ((direction: 'first' | 'last' | 'next' | 'previous') => void) + | undefined; +}) { + return ( + + ); +} diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromPage.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromPage.tsx new file mode 100644 index 00000000000..35c15a277f1 --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromPage.tsx @@ -0,0 +1,195 @@ +import React from 'react'; +import { IR, RA } from '../../utils/types'; +import { ResourceView } from '../Forms/ResourceView'; +import { SpecifyTable } from '../DataModel/specifyTable'; +import { useLiveState } from '../../hooks/useLiveState'; +import { SpecifyResource } from '../DataModel/legacyTypes'; +import { AnySchema } from '../DataModel/helperTypes'; +import { LocalizedString } from 'typesafe-i18n'; +import { useTriggerState } from '../../hooks/useTriggerState'; +import { ReadOnlyContext } from '../Core/Contexts'; +import { hasTablePermission } from '../Permissions/helpers'; +import { LazySlider } from './LazySlider'; +import { DataEntry } from '../Atoms/DataEntry'; +import { commonText } from '../../localization/common'; + +type Page = { + readonly ids: RA; + readonly complete: boolean; +}; + +function usePagedRecordSelector({ + firstPage, + lastPage, + index, + records, + onSlide: handleSlide, + onDelete: handleDelete, +}: { + readonly firstPage: boolean; + readonly lastPage: boolean; + readonly index: number; + readonly records: RA> | undefined; + readonly onSlide: (direction: 'next' | 'previous' | 'first' | 'last') => void; + readonly onDelete: (index: number) => void; +}) { + const disableNextButtons = + records !== undefined && lastPage && index === records.length - 1; + + const disablePreviousButtons = firstPage && index === 0; + + return { + slider: ( + + ), + isLoading: records == undefined || records[index] === undefined, + resource: records?.[index], + onRemove: () => { + handleDelete(index); + handleSlide('previous'); + }, + }; +} + +export function RecordSelectorFromPage< + SCHEMA extends AnySchema, + PAGE_TYPE extends Page, +>({ + page: initialPage, + firstPage, + dialog, + headerButtons, + isDependent = false, + isLoading: isExternalLoading = false, + table, + title, + totalCount, + viewName, + onNextPageFetch: handleNextPageFetch, + onClose: handleClose, + onDelete: handleDelete, + onSaved: handleSaved, +}: { + readonly page: PAGE_TYPE | undefined; + readonly firstPage: boolean; + readonly dialog: false | 'modal' | 'nonModal'; + readonly headerButtons?: JSX.Element; + readonly isLoading?: boolean; + readonly isDependent?: boolean; + readonly table: SpecifyTable; + readonly title: LocalizedString | undefined; + readonly totalCount?: number; + readonly viewName?: string; + readonly onNextPageFetch: ( + previousPage: PAGE_TYPE, + direction: 'next' | 'previous' | 'first' | 'last' + ) => Promise; + readonly onClose: () => void; + readonly onDelete: (index: number) => void; + readonly onSaved: (resource: SpecifyResource) => void; +}): JSX.Element | null { + const [page, setPage] = useTriggerState(initialPage); + + const [records, setRecords] = useLiveState< + RA> | undefined + >( + React.useCallback( + () => page?.ids.map((id) => new table.Resource({ id })), + [page] + ) + ); + const [index, setIndex] = React.useState(0); + + const currentResource = records?.[index]; + + const isReadOnly = React.useContext(ReadOnlyContext); + + function handleSlide(direction: 'next' | 'previous' | 'first' | 'last') { + if (page === undefined) { + return; + } + if (direction === 'first') { + setIndex(0); + return handleNextPageFetch(page, 'first'); + } + if (direction === 'last') { + setIndex(page.ids.length - 1); + return handleNextPageFetch(page, 'last'); + } + + if (index === 0 && direction === 'previous') { + setIndex(page.ids.length - 1); + return handleNextPageFetch(page, 'previous'); + } + + if (index === records!.length - 1 && direction === 'next') { + setIndex(0); + return handleNextPageFetch(page, 'next'); + } + setIndex((oldIndex) => + direction === 'next' ? oldIndex + 1 : oldIndex - 1 + ); + return; + } + + const { + resource, + slider, + isLoading, + onRemove: handleRemove, + } = usePagedRecordSelector({ + index, + records, + firstPage, + lastPage: page?.complete ?? false, + onDelete: handleDelete, + onSlide: handleSlide, + }); + + return ( + <> + ( +
+
+ {headerButtons} + + handleRemove()} + /> + + {specifyNetworkBadge} +
+ {slider} +
+ )} + isDependent={isDependent} + isLoading={isLoading || isExternalLoading} + isSubForm={false} + resource={resource} + title={title} + viewName={viewName} + onClose={handleClose} + onDeleted={ + hasTablePermission(table.name, 'delete') ? handleRemove : undefined + } + onSaved={() => handleSaved(currentResource!)} + onAdd={undefined} + /> + + ); +} From 4292cf98cbf8622e3dcb9870a61bc3f11c144026 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 8 Oct 2026 19:04:57 -0500 Subject: [PATCH 25/66] fix: correct table for RecordSelector and better page navigation --- .../lib/components/DeleteBlockers/Context.tsx | 88 +++++++++++++++---- .../DeleteBlockers/deleteBlockers.ts | 26 +++--- .../lib/components/DeleteBlockers/index.tsx | 17 ++-- .../components/DeleteBlockers/pageCache.ts | 45 ++++++++-- .../lib/components/DeleteBlockers/state.ts | 81 +++++++---------- .../lib/components/DeleteBlockers/store.ts | 44 ++++------ .../lib/components/DeleteBlockers/types.ts | 19 ---- .../lib/components/FormSliders/LazySlider.tsx | 2 +- .../FormSliders/RecordSelectorFromPage.tsx | 50 ++++++----- 9 files changed, 213 insertions(+), 159 deletions(-) delete mode 100644 specifyweb/frontend/js_src/lib/components/DeleteBlockers/types.ts diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/Context.tsx b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/Context.tsx index 6b2fa4d964f..5a208d6154b 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/Context.tsx +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/Context.tsx @@ -7,6 +7,7 @@ import { DeleteBlockerResource, DeleteBlockerStore } from './store'; import { RA } from '../../utils/types'; import { Relationship } from '../DataModel/specifyField'; import { DeleteBlockerLRUPage } from './pageCache'; +import { PageMetaData } from '../FormSliders/RecordSelectorFromPage'; const DeleteBlockerContext = React.createContext( null @@ -80,36 +81,93 @@ export function useDeleteBlockersForResource( export function useDeleteBlockerPages( resource: SpecifyResource, relationship: Relationship -) { +): { + readonly page: DeleteBlockerLRUPage | undefined; + readonly pageMetaData: PageMetaData; + readonly pageSize: number; + readonly onNextPageFetch: ( + previous: DeleteBlockerLRUPage, + direction: 'next' | 'previous' | 'first' | 'last' + ) => Promise; +} { const store = useDeleteBlockerStore(); - const [anchor, setAnchor] = React.useState(null); + const [pageCursor, setPageCursor] = React.useState<{ + readonly anchor: number | null | 'last'; + readonly backwards: boolean; + }>({ + anchor: null, + backwards: false, + }); + + const deleteBlockerPage = React.useSyncExternalStore(store.subscribe, () => + store.getCachedPage(resource, relationship, pageCursor.anchor) + ); + // When the cursor changes, this useEffect tells the DeleteBlockerStore to + // fetch the page + // The useSyncExternalStore will update deleteBlockerPage when the promise + // resolves and the blockers are cached React.useEffect(() => { - store.filterBlockers(resource, relationship, anchor); - }, [store, resource, relationship]); + store.filterBlockers( + resource, + relationship, + pageCursor.anchor, + pageCursor.backwards + ); + }, [store, resource, relationship, pageCursor]); async function handleNextPageFetch( previousPage: DeleteBlockerLRUPage, direction: 'next' | 'previous' | 'first' | 'last' ) { const backwards = direction === 'previous' || direction === 'last'; - const anchor = - direction === 'next' - ? previousPage.ids.at(-1) - : direction === 'previous' - ? previousPage.ids.at(0) - : null; - setAnchor(anchor ?? null); - store.filterBlockers(resource, relationship, anchor, backwards); + + if (direction === 'last') { + setPageCursor(() => ({ + anchor: 'last', + backwards: true, + })); + return; + } + if (direction === 'first') { + setPageCursor(() => ({ + anchor: null, + backwards: false, + })); + return; + } + if (previousPage.ids.length === 0) { + setPageCursor(() => ({ + anchor: previousPage.anchor, + backwards, + })); + return; + } + setPageCursor(() => ({ + anchor: previousPage.ids.at(-1) ?? null, + backwards, + })); } - const deleteBlockerPage = React.useSyncExternalStore(store.subscribe, () => - store.getCachedPage(resource, relationship, anchor) - ); + const isFirstPage = + deleteBlockerPage?.anchor === null && !pageCursor.backwards; + + const isLastPage = + deleteBlockerPage?.complete === true || + (deleteBlockerPage?.anchor === null && pageCursor.backwards); return { page: deleteBlockerPage, + pageMetaData: + isFirstPage && isLastPage + ? 'single' + : isFirstPage + ? 'first' + : isLastPage + ? 'last' + : undefined, + pageSize: store.pageSize(), onNextPageFetch: handleNextPageFetch, }; } diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/deleteBlockers.ts b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/deleteBlockers.ts index e69954fe913..d907e917e94 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/deleteBlockers.ts +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/deleteBlockers.ts @@ -1,13 +1,26 @@ import { ajax } from '../../utils/ajax'; import { Http } from '../../utils/ajax/definitions'; import { RA } from '../../utils/types'; -import { group } from '../../utils/utils'; import { Tables } from '../DataModel/types'; import { formatUrl } from '../Router/queryString'; -import { APIDeleteBlockerPage, APIDeleteBlockers } from './types'; export const DELETE_BLOCKER_LIMIT = 40; +export type APIDeleteBlockerPage = { + readonly table: Lowercase; + readonly field: string; + readonly ids: RA; + readonly limit: number; + readonly complete: boolean; + readonly anchor: null | number; + readonly backwards: boolean; +}; + +export type APIDeleteBlockers = { + readonly results: RA; + readonly next: RA; +}; + type APIDeleteBlockerCount = { readonly table: Lowercase; readonly field: string; @@ -27,15 +40,6 @@ type DeleteBlockerFilter = { readonly backwards?: boolean; }; -export function groupBlockers(blockers: RA): RA<{ - readonly table: Lowercase; - readonly blockers: RA; -}> { - return group(blockers.map((blocker) => [blocker.table, blocker])).map( - ([table, blockers]) => ({ table, blockers }) - ); -} - export async function fetchInitialBlockers( table: keyof Tables, recordId: number, diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx index b2abd227378..e00e53898ed 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx @@ -3,7 +3,7 @@ import { Button } from '../Atoms/Button'; import { SpecifyResource } from '../DataModel/legacyTypes'; import { AnySchema } from '../DataModel/helperTypes'; import { useDeleteBlockerPages, useDeleteBlockersForResource } from './Context'; -import { ResourceIdentifier, resourceToStringIdentifier } from './state'; +import { resourceToStringIdentifier } from './state'; import { RA } from '../../utils/types'; import { format } from '../Formatters/formatters'; import { useAsyncState } from '../../hooks/useAsyncState'; @@ -199,15 +199,20 @@ function DeleteBlockersPaginator({ readonly parentResource: SpecifyResource; readonly relationship: Relationship; }): JSX.Element { - const { page, onNextPageFetch: handleNextPageFetch } = useDeleteBlockerPages( - parentResource, - relationship - ); + const { + page, + pageMetaData, + pageSize, + onNextPageFetch: handleNextPageFetch, + } = useDeleteBlockerPages(parentResource, relationship); + return ( ; readonly ids: RA; - readonly anchor: number | null; + readonly anchor: 'last' | number | null; readonly backwards: boolean; + readonly complete: boolean; }; -export function blockerPageToCacheKey( +export function pageAnchorToFilter(anchor: number | null | 'last') { + return anchor === undefined || anchor === 'last' ? null : anchor; +} + +export function makeBlockerKey(...components: RA) { + return filterArray(components).join('_'); +} + +export function buildPageCacheKey( owner: SpecifyResource, - page: APIDeleteBlockerPage -): BlockerPageCacheKey { + relatedTable?: string, + relationshipName?: string, + anchor: number | null | 'last' = null +) { const ownerTable = owner.specifyTable.name.toLowerCase(); const ownerId = owner.id; if (ownerId === undefined) { softFail('Attempting to get DeleteBlockers for record without ID'); } + const keyParts = [ + ownerTable, + ownerId, + relatedTable?.toLowerCase(), + relationshipName?.toLowerCase(), + anchor, + ]; + return makeBlockerKey(...keyParts); +} + +export function blockerPageToCacheKey( + owner: SpecifyResource, + page: APIDeleteBlockerPage +): BlockerPageCacheKey { const tableName = page.table; const relationshipName = page.field; + const backwards = page.backwards; const anchorId = page.anchor; - const keyParts = [ownerTable, ownerId, tableName, relationshipName, anchorId]; - return keyParts.join('_'); + const resolvedAnchor = anchorId === null && backwards ? 'last' : anchorId; + return buildPageCacheKey(owner, tableName, relationshipName, resolvedAnchor); } // eslint-disable-next-line functional/no-class @@ -65,6 +93,7 @@ export class DeleteBlockerLRU { table: blockerPage.table, anchor: blockerPage.anchor, backwards: blockerPage.backwards, + complete: blockerPage.complete, }; this.pageCache.set(cacheKey, cachedPage); return cachedPage; diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/state.ts b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/state.ts index 66a6240a55a..1dae09e7a86 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/state.ts +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/state.ts @@ -1,5 +1,5 @@ import { PromiseQueue } from '../../utils/promiseQueue'; -import { filterArray, RA, WritableArray } from '../../utils/types'; +import { RA, WritableArray } from '../../utils/types'; import { AnySchema } from '../DataModel/helperTypes'; import { SpecifyResource } from '../DataModel/legacyTypes'; import { getTable } from '../DataModel/tables'; @@ -9,46 +9,20 @@ import { DELETE_BLOCKER_LIMIT, fetchInitialBlockers, filterDeleteBlockers, - groupBlockers, } from './deleteBlockers'; import { + BlockerPageCacheKey, blockerPageToCacheKey, + buildPageCacheKey, DeleteBlockerLRU, DeleteBlockerLRUPage, + makeBlockerKey, + pageAnchorToFilter, } from './pageCache'; -import { - APIDeleteBlockerPage, - APIDeleteBlockers, - BlockerPageCacheKey, -} from './types'; +import { APIDeleteBlockerPage, APIDeleteBlockers } from './deleteBlockers'; export type ResourceIdentifier = string; -export function recordToBlockerCacheKey( - resource: SpecifyResource, - relationshipInfo?: { - readonly relatedTable: string; - readonly relationshipName: string; - }, - anchorId: number | null = null -): BlockerPageCacheKey { - const tableName = resource.specifyTable.name.toLowerCase(); - const recordId = resource.id; - if (recordId === undefined) { - softFail('Attempting to get DeleteBlockers for record without ID'); - } - const relationshipTable = relationshipInfo?.relatedTable.toLowerCase(); - const relationshipName = relationshipInfo?.relationshipName.toLowerCase(); - const keyParts = filterArray([ - tableName, - recordId, - relationshipTable, - relationshipName, - anchorId, - ]); - return keyParts.join('_'); -} - export type BlockerRelationship = { readonly key: BlockerPageCacheKey; readonly table: Lowercase; @@ -71,10 +45,6 @@ export function resourceToStringIdentifier( return makeBlockerKey(resource.specifyTable.name.toLowerCase(), resource.id); } -export function makeBlockerKey(...components: RA) { - return components.join('_'); -} - // eslint-disable-next-line functional/no-class export class DeleteBlockerState { private readonly pageCache: DeleteBlockerLRU; @@ -93,7 +63,7 @@ export class DeleteBlockerState { private readonly onChange?: () => void; public constructor({ - maxPages = 400, + maxPages = 200, recordsPerPage = DELETE_BLOCKER_LIMIT, onChange = undefined, }: { @@ -122,6 +92,10 @@ export class DeleteBlockerState { return this.nodes.get(nodeKey); } + public pageSize(): number { + return this.recordsPerPage; + } + public destroy() { this.countPromiseQueue.clear(); this.pagePromiseQueue.clear(); @@ -179,7 +153,7 @@ export class DeleteBlockerState { } public async seedBlockers(resource: SpecifyResource) { - const queueKey = recordToBlockerCacheKey(resource); + const queueKey = buildPageCacheKey(resource); const promiseInQueue = this.pagePromiseQueue.get(queueKey); if (promiseInQueue === false || promiseInQueue !== undefined) { return promiseInQueue; @@ -386,15 +360,13 @@ export class DeleteBlockerState { resource: SpecifyResource, relatedTable: keyof Tables | Lowercase, relationshipName: string, - anchor: number | null = null, + anchor: number | null | 'last' = null, backwards: boolean = false ) { - const cacheKey = recordToBlockerCacheKey( + const cacheKey = buildPageCacheKey( resource, - { - relatedTable, - relationshipName, - }, + relatedTable, + relationshipName, anchor ); if (this.pageIsCached(cacheKey)) { @@ -408,19 +380,32 @@ export class DeleteBlockerState { { table: relatedTable, field: relationshipName, - anchor, - backwards, + anchor: pageAnchorToFilter(anchor), + // This is needed to fetch the last page + // A null anchor depends on the direction of the request + backwards: anchor === 'last' ? true : backwards, limit: this.recordsPerPage, }, ]) .then((blockers) => { - return backwards === true + return anchor === 'last' || backwards === true ? { ...blockers, results: blockers.results.map((page) => ({ ...page, ids: [...page.ids].reverse(), - backwards: false, + // We normally treat backwards requests the same as forwards + // and transform them accordingly. + // However when the last page is requested then it's + // important to include indication of backwards as that's the + // only way to know the last page was requested. + // See blockerPageToCacheKey + // REFACTOR: Maybe we can define a modified type of the API + // response and pass those to the cache that allows anchor to + // be 'false'. + // That would decouple the anchor + backwards pairing to make + // working with cached pages easier + backwards: anchor === 'last', })), } : blockers; diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/store.ts b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/store.ts index 4f6adb38048..9fb7ff8d9e5 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/store.ts +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/store.ts @@ -1,18 +1,19 @@ -import { RA, RR, Writable, WritableArray } from '../../utils/types'; +import { RA, Writable, WritableArray } from '../../utils/types'; import { AnySchema } from '../DataModel/helperTypes'; import { SpecifyResource } from '../DataModel/legacyTypes'; import { Relationship } from '../DataModel/specifyField'; -import { Tables } from '../DataModel/types'; -import { DeleteBlockerLRUPage } from './pageCache'; +import { + BlockerPageCacheKey, + buildPageCacheKey, + DeleteBlockerLRUPage, + makeBlockerKey, +} from './pageCache'; import { BlockerNode, DeleteBlockerState, - makeBlockerKey, - recordToBlockerCacheKey, ResourceIdentifier, resourceToStringIdentifier, } from './state'; -import { BlockerPageCacheKey } from './types'; export type DeleteBlockerRelationship = { readonly key: BlockerPageCacheKey; @@ -71,29 +72,18 @@ export class DeleteBlockerStore { this.listeners.forEach((listener) => listener()); } - public async seedBlockers(resource: SpecifyResource) { - return this.state.seedBlockers(resource); + public pageSize(): number { + return this.state.pageSize(); } - public fetchDeleteBlockerPage( - resource: SpecifyResource, - relationship: Relationship, - anchor: number | null = null, - backwards: boolean = false - ) { - return this.state.filterBlockers( - resource, - relationship.table.name, - relationship.name, - anchor, - backwards - ); + public async seedBlockers(resource: SpecifyResource) { + return this.state.seedBlockers(resource); } public filterBlockers( resource: SpecifyResource, relationship: Relationship, - anchor: number | null = null, + anchor: number | null | 'last' = null, backwards: boolean = false ) { return this.state.filterBlockers( @@ -108,14 +98,12 @@ export class DeleteBlockerStore { public getCachedPage( resource: SpecifyResource, relationship: Relationship, - anchor: number | null = null + anchor: number | null | 'last' = null ): DeleteBlockerLRUPage | undefined { - const cacheKey = recordToBlockerCacheKey( + const cacheKey = buildPageCacheKey( resource, - { - relatedTable: relationship.table.name, - relationshipName: relationship.name, - }, + relationship.table.name, + relationship.name, anchor ); return this.state.getBlockerPage(cacheKey); diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/types.ts b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/types.ts deleted file mode 100644 index a2766bc8f8d..00000000000 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/types.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { RA } from '../../utils/types'; -import { Tables } from '../DataModel/types'; - -export type APIDeleteBlockerPage = { - readonly table: Lowercase; - readonly field: string; - readonly ids: RA; - readonly limit: number; - readonly complete: boolean; - readonly anchor: null | number; - readonly backwards: boolean; -}; - -export type APIDeleteBlockers = { - readonly results: RA; - readonly next: RA; -}; - -export type BlockerPageCacheKey = string; diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/LazySlider.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/LazySlider.tsx index 6e3bb4dc708..f3e623bfa06 100644 --- a/specifyweb/frontend/js_src/lib/components/FormSliders/LazySlider.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/LazySlider.tsx @@ -37,7 +37,7 @@ export function LazySlider({ handleChange?.('previous')} > diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromPage.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromPage.tsx index 35c15a277f1..a848afe9537 100644 --- a/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromPage.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromPage.tsx @@ -12,31 +12,35 @@ import { hasTablePermission } from '../Permissions/helpers'; import { LazySlider } from './LazySlider'; import { DataEntry } from '../Atoms/DataEntry'; import { commonText } from '../../localization/common'; +import { clamp } from '../../utils/utils'; type Page = { readonly ids: RA; readonly complete: boolean; }; +export type PageMetaData = 'single' | 'first' | 'last' | undefined; + function usePagedRecordSelector({ - firstPage, - lastPage, + pageMetaData, index, records, onSlide: handleSlide, onDelete: handleDelete, }: { - readonly firstPage: boolean; - readonly lastPage: boolean; + readonly pageMetaData: PageMetaData; readonly index: number; readonly records: RA> | undefined; readonly onSlide: (direction: 'next' | 'previous' | 'first' | 'last') => void; readonly onDelete: (index: number) => void; }) { const disableNextButtons = - records !== undefined && lastPage && index === records.length - 1; + records !== undefined && + index === records.length - 1 && + (pageMetaData === 'last' || pageMetaData === 'single'); - const disablePreviousButtons = firstPage && index === 0; + const disablePreviousButtons = + (pageMetaData === 'single' || pageMetaData === 'first') && index === 0; return { slider: ( @@ -64,7 +68,8 @@ export function RecordSelectorFromPage< PAGE_TYPE extends Page, >({ page: initialPage, - firstPage, + pageMetaData, + pageSize, dialog, headerButtons, isDependent = false, @@ -79,7 +84,8 @@ export function RecordSelectorFromPage< onSaved: handleSaved, }: { readonly page: PAGE_TYPE | undefined; - readonly firstPage: boolean; + readonly pageSize: number; + readonly pageMetaData: PageMetaData; readonly dialog: false | 'modal' | 'nonModal'; readonly headerButtons?: JSX.Element; readonly isLoading?: boolean; @@ -106,36 +112,35 @@ export function RecordSelectorFromPage< [page] ) ); - const [index, setIndex] = React.useState(0); + const [indexInPage, setIndexInPage] = React.useState(0); - const currentResource = records?.[index]; + const currentResource = records?.[indexInPage]; const isReadOnly = React.useContext(ReadOnlyContext); function handleSlide(direction: 'next' | 'previous' | 'first' | 'last') { - if (page === undefined) { + if (page === undefined || records === undefined) { return; } if (direction === 'first') { - setIndex(0); + setIndexInPage(0); return handleNextPageFetch(page, 'first'); } if (direction === 'last') { - setIndex(page.ids.length - 1); + setIndexInPage(pageSize - 1); return handleNextPageFetch(page, 'last'); } - - if (index === 0 && direction === 'previous') { - setIndex(page.ids.length - 1); + if (indexInPage === 0 && direction === 'previous') { + setIndexInPage(pageSize - 1); return handleNextPageFetch(page, 'previous'); } - if (index === records!.length - 1 && direction === 'next') { - setIndex(0); + if (indexInPage === records.length - 1 && direction === 'next') { + setIndexInPage(0); return handleNextPageFetch(page, 'next'); } - setIndex((oldIndex) => - direction === 'next' ? oldIndex + 1 : oldIndex - 1 + setIndexInPage((oldIndex) => + clamp(0, direction === 'next' ? oldIndex + 1 : oldIndex - 1, pageSize - 1) ); return; } @@ -146,10 +151,9 @@ export function RecordSelectorFromPage< isLoading, onRemove: handleRemove, } = usePagedRecordSelector({ - index, + index: indexInPage, + pageMetaData, records, - firstPage, - lastPage: page?.complete ?? false, onDelete: handleDelete, onSlide: handleSlide, }); From 0a95774655a2f244259c2ef0623e4d91841dcc4f Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 8 Oct 2026 19:05:46 -0500 Subject: [PATCH 26/66] fix: ensure NoneType is used when checking anchor type --- specifyweb/backend/delete_blockers/validators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specifyweb/backend/delete_blockers/validators.py b/specifyweb/backend/delete_blockers/validators.py index 82851039ffc..5bc5f99370a 100644 --- a/specifyweb/backend/delete_blockers/validators.py +++ b/specifyweb/backend/delete_blockers/validators.py @@ -98,7 +98,7 @@ def clean_filters(self): "table": str, "field": str, "backwards": bool, - "anchor": (int, None), + "anchor": (int, type(None)), "limit": int } for key, types in expected_types.items(): From bf114a87812a569b63d53aa85f2dcc7d902d03e9 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 8 Oct 2026 19:26:50 -0500 Subject: [PATCH 27/66] feat: define hook for slider buttons --- .../components/FormSliders/BasicSlider.tsx | 40 +++++++++ .../lib/components/FormSliders/LazySlider.tsx | 65 -------------- .../FormSliders/RecordSelectorFromPage.tsx | 4 +- .../lib/components/FormSliders/Slider.tsx | 67 +++++++-------- .../FormSliders/useSliderButtons.tsx | 84 +++++++++++++++++++ 5 files changed, 157 insertions(+), 103 deletions(-) create mode 100644 specifyweb/frontend/js_src/lib/components/FormSliders/BasicSlider.tsx delete mode 100644 specifyweb/frontend/js_src/lib/components/FormSliders/LazySlider.tsx create mode 100644 specifyweb/frontend/js_src/lib/components/FormSliders/useSliderButtons.tsx diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/BasicSlider.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/BasicSlider.tsx new file mode 100644 index 00000000000..631d78397a1 --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/BasicSlider.tsx @@ -0,0 +1,40 @@ +import React from 'react'; +import { useSliderButtons } from './useSliderButtons'; + +type DisabledProps = { + readonly first?: boolean; + readonly next?: boolean; + readonly previous?: boolean; + readonly last?: boolean; +}; + +export function BasicSlider({ + buttonsDisabled = { + first: false, + next: false, + previous: false, + last: false, + }, + onChange: handleChange, +}: { + readonly buttonsDisabled?: DisabledProps; + readonly onChange: + | ((direction: 'first' | 'last' | 'next' | 'previous') => void) + | undefined; +}) { + const { + navigateToFirst, + navigateToPrevious, + navigateToNext, + navigateToLast, + } = useSliderButtons({ buttonsDisabled, onChange: handleChange }); + + return ( + + ); +} diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/LazySlider.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/LazySlider.tsx deleted file mode 100644 index f3e623bfa06..00000000000 --- a/specifyweb/frontend/js_src/lib/components/FormSliders/LazySlider.tsx +++ /dev/null @@ -1,65 +0,0 @@ -import React from 'react'; -import { formsText } from '../../localization/forms'; -import { Button } from '../Atoms/Button'; -import { icons } from '../Atoms/Icons'; - -type DisabledProps = { - readonly first?: boolean; - readonly next?: boolean; - readonly previous?: boolean; - readonly last?: boolean; -}; - -export function LazySlider({ - buttonsDisabled = { - first: false, - next: false, - previous: false, - last: false, - }, - onChange: handleChange, -}: { - readonly buttonsDisabled?: DisabledProps; - readonly onChange: - | ((direction: 'first' | 'last' | 'next' | 'previous') => void) - | undefined; -}) { - return ( - - ); -} diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromPage.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromPage.tsx index a848afe9537..90f2c45e35d 100644 --- a/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromPage.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromPage.tsx @@ -9,7 +9,7 @@ import { LocalizedString } from 'typesafe-i18n'; import { useTriggerState } from '../../hooks/useTriggerState'; import { ReadOnlyContext } from '../Core/Contexts'; import { hasTablePermission } from '../Permissions/helpers'; -import { LazySlider } from './LazySlider'; +import { BasicSlider } from './BasicSlider'; import { DataEntry } from '../Atoms/DataEntry'; import { commonText } from '../../localization/common'; import { clamp } from '../../utils/utils'; @@ -44,7 +44,7 @@ function usePagedRecordSelector({ return { slider: ( - { + if (direction === 'first') handleChange?.(0); + if (direction === 'previous') handleChange?.(value - 1); + if (direction === 'next') handleChange?.(value + 1); + if (direction === 'last') handleChange?.(count - 1); + }, + [value, handleChange] + ); + + const { + navigateToFirst, + navigateToPrevious, + navigateToNext, + navigateToLast, + } = useSliderButtons({ + buttonsDisabled: { + first: value === 0, + previous: value === 0, + next: value + 1 === count, + last: value + 1 == count, + }, + onChange: handleSlide, + }); + return count > 0 ? ( ) : null; } diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/useSliderButtons.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/useSliderButtons.tsx new file mode 100644 index 00000000000..6bb5824afba --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/useSliderButtons.tsx @@ -0,0 +1,84 @@ +import React from 'react'; +import { formsText } from '../../localization/forms'; +import { Button } from '../Atoms/Button'; +import { icons } from '../Atoms/Icons'; + +type DisabledProps = { + readonly first?: boolean; + readonly next?: boolean; + readonly previous?: boolean; + readonly last?: boolean; +}; + +export type SlideDirection = 'first' | 'last' | 'next' | 'previous'; + +export function useSliderButtons({ + buttonsDisabled = { + first: false, + next: false, + previous: false, + last: false, + }, + onChange: handleChange, +}: { + readonly buttonsDisabled?: DisabledProps; + readonly onChange: ((direction: SlideDirection) => void) | undefined; +}): { + readonly navigateToFirst: JSX.Element; + readonly navigateToPrevious: JSX.Element; + readonly navigateToNext: JSX.Element; + readonly navigateToLast: JSX.Element; +} { + const navigateToFirst = ( + handleChange?.('first')} + > + {icons.chevronDoubleLeft} + + ); + + const navigateToPrevious = ( + handleChange?.('previous')} + > + {icons.chevronLeft} + + ); + + const navigateToNext = ( + handleChange?.('next')} + > + {icons.chevronRight} + + ); + + const navigateToLast = ( + handleChange?.('last')} + > + {icons.chevronDoubleRight} + + ); + + return { + navigateToFirst, + navigateToPrevious, + navigateToNext, + navigateToLast, + }; +} From 76d05e354d842d7af6e081418879548ac7cd8f98 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 8 Oct 2026 19:31:45 -0500 Subject: [PATCH 28/66] chore: extract DeleteBlocker hooks to own files --- .../lib/components/DeleteBlockers/Context.tsx | 119 +----------------- .../lib/components/DeleteBlockers/index.tsx | 3 +- .../DeleteBlockers/useDeleteBlockerPages.tsx | 101 +++++++++++++++ .../useDeleteBlockersForResource.tsx | 30 +++++ 4 files changed, 134 insertions(+), 119 deletions(-) create mode 100644 specifyweb/frontend/js_src/lib/components/DeleteBlockers/useDeleteBlockerPages.tsx create mode 100644 specifyweb/frontend/js_src/lib/components/DeleteBlockers/useDeleteBlockersForResource.tsx diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/Context.tsx b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/Context.tsx index 5a208d6154b..cccf51c2538 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/Context.tsx +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/Context.tsx @@ -47,127 +47,10 @@ export function DeleteBlockerProvider({ ); } -function useDeleteBlockerStore(): DeleteBlockerStore { +export function useDeleteBlockerStore(): DeleteBlockerStore { const store = React.useContext(DeleteBlockerContext); if (store === null) { error('Attempting to access a delete blocker store that is not created'); } return store; } - -export function useDeleteBlockersForResource( - resource: SpecifyResource -): RA | undefined { - const store = useDeleteBlockerStore(); - const blockerKey = resourceToStringIdentifier(resource); - - React.useEffect(() => { - void store.seedBlockers(resource); - }, [store, blockerKey]); - - const nodeVersion = React.useSyncExternalStore( - store.subscribe, - () => store.getNode(blockerKey)?.version ?? 0 - ); - - const records = React.useMemo( - () => store.getBlockerGraph(resource), - [store, nodeVersion, blockerKey] - ); - - return records; -} - -export function useDeleteBlockerPages( - resource: SpecifyResource, - relationship: Relationship -): { - readonly page: DeleteBlockerLRUPage | undefined; - readonly pageMetaData: PageMetaData; - readonly pageSize: number; - readonly onNextPageFetch: ( - previous: DeleteBlockerLRUPage, - direction: 'next' | 'previous' | 'first' | 'last' - ) => Promise; -} { - const store = useDeleteBlockerStore(); - - const [pageCursor, setPageCursor] = React.useState<{ - readonly anchor: number | null | 'last'; - readonly backwards: boolean; - }>({ - anchor: null, - backwards: false, - }); - - const deleteBlockerPage = React.useSyncExternalStore(store.subscribe, () => - store.getCachedPage(resource, relationship, pageCursor.anchor) - ); - - // When the cursor changes, this useEffect tells the DeleteBlockerStore to - // fetch the page - // The useSyncExternalStore will update deleteBlockerPage when the promise - // resolves and the blockers are cached - React.useEffect(() => { - store.filterBlockers( - resource, - relationship, - pageCursor.anchor, - pageCursor.backwards - ); - }, [store, resource, relationship, pageCursor]); - - async function handleNextPageFetch( - previousPage: DeleteBlockerLRUPage, - direction: 'next' | 'previous' | 'first' | 'last' - ) { - const backwards = direction === 'previous' || direction === 'last'; - - if (direction === 'last') { - setPageCursor(() => ({ - anchor: 'last', - backwards: true, - })); - return; - } - if (direction === 'first') { - setPageCursor(() => ({ - anchor: null, - backwards: false, - })); - return; - } - if (previousPage.ids.length === 0) { - setPageCursor(() => ({ - anchor: previousPage.anchor, - backwards, - })); - return; - } - setPageCursor(() => ({ - anchor: previousPage.ids.at(-1) ?? null, - backwards, - })); - } - - const isFirstPage = - deleteBlockerPage?.anchor === null && !pageCursor.backwards; - - const isLastPage = - deleteBlockerPage?.complete === true || - (deleteBlockerPage?.anchor === null && pageCursor.backwards); - - return { - page: deleteBlockerPage, - pageMetaData: - isFirstPage && isLastPage - ? 'single' - : isFirstPage - ? 'first' - : isLastPage - ? 'last' - : undefined, - pageSize: store.pageSize(), - onNextPageFetch: handleNextPageFetch, - }; -} diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx index e00e53898ed..92ba82b6f74 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx @@ -2,7 +2,6 @@ import React from 'react'; import { Button } from '../Atoms/Button'; import { SpecifyResource } from '../DataModel/legacyTypes'; import { AnySchema } from '../DataModel/helperTypes'; -import { useDeleteBlockerPages, useDeleteBlockersForResource } from './Context'; import { resourceToStringIdentifier } from './state'; import { RA } from '../../utils/types'; import { format } from '../Formatters/formatters'; @@ -20,6 +19,8 @@ import { import { RecordSelectorFromPage } from '../FormSliders/RecordSelectorFromPage'; import { f } from '../../utils/functools'; import { Relationship } from '../DataModel/specifyField'; +import { useDeleteBlockersForResource } from './useDeleteBlockersForResource'; +import { useDeleteBlockerPages } from './useDeleteBlockerPages'; export function NewDeleteBlockers({ resource, diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/useDeleteBlockerPages.tsx b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/useDeleteBlockerPages.tsx new file mode 100644 index 00000000000..08647a387d8 --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/useDeleteBlockerPages.tsx @@ -0,0 +1,101 @@ +import React from 'react'; +import { SpecifyResource } from '../DataModel/legacyTypes'; +import { AnySchema } from '../DataModel/helperTypes'; +import { Relationship } from '../DataModel/specifyField'; +import { DeleteBlockerLRUPage } from './pageCache'; +import { PageMetaData } from '../FormSliders/RecordSelectorFromPage'; +import { useDeleteBlockerStore } from './Context'; + +export function useDeleteBlockerPages( + resource: SpecifyResource, + relationship: Relationship +): { + readonly page: DeleteBlockerLRUPage | undefined; + readonly pageMetaData: PageMetaData; + readonly pageSize: number; + readonly onNextPageFetch: ( + previous: DeleteBlockerLRUPage, + direction: 'next' | 'previous' | 'first' | 'last' + ) => Promise; +} { + const store = useDeleteBlockerStore(); + + const [pageCursor, setPageCursor] = React.useState<{ + readonly anchor: number | null | 'last'; + readonly backwards: boolean; + }>({ + anchor: null, + backwards: false, + }); + + const deleteBlockerPage = React.useSyncExternalStore(store.subscribe, () => + store.getCachedPage(resource, relationship, pageCursor.anchor) + ); + + // When the cursor changes, this useEffect tells the DeleteBlockerStore to + // fetch the page + // The useSyncExternalStore will update deleteBlockerPage when the promise + // resolves and the blockers are cached + React.useEffect(() => { + store.filterBlockers( + resource, + relationship, + pageCursor.anchor, + pageCursor.backwards + ); + }, [store, resource, relationship, pageCursor]); + + async function handleNextPageFetch( + previousPage: DeleteBlockerLRUPage, + direction: 'next' | 'previous' | 'first' | 'last' + ) { + const backwards = direction === 'previous' || direction === 'last'; + + if (direction === 'last') { + setPageCursor(() => ({ + anchor: 'last', + backwards: true, + })); + return; + } + if (direction === 'first') { + setPageCursor(() => ({ + anchor: null, + backwards: false, + })); + return; + } + if (previousPage.ids.length === 0) { + setPageCursor(() => ({ + anchor: previousPage.anchor, + backwards, + })); + return; + } + setPageCursor(() => ({ + anchor: previousPage.ids.at(-1) ?? null, + backwards, + })); + } + + const isFirstPage = + deleteBlockerPage?.anchor === null && !pageCursor.backwards; + + const isLastPage = + deleteBlockerPage?.complete === true || + (deleteBlockerPage?.anchor === null && pageCursor.backwards); + + return { + page: deleteBlockerPage, + pageMetaData: + isFirstPage && isLastPage + ? 'single' + : isFirstPage + ? 'first' + : isLastPage + ? 'last' + : undefined, + pageSize: store.pageSize(), + onNextPageFetch: handleNextPageFetch, + }; +} diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/useDeleteBlockersForResource.tsx b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/useDeleteBlockersForResource.tsx new file mode 100644 index 00000000000..b47adb9b902 --- /dev/null +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/useDeleteBlockersForResource.tsx @@ -0,0 +1,30 @@ +import React from 'react'; +import { resourceToStringIdentifier } from './state'; +import { SpecifyResource } from '../DataModel/legacyTypes'; +import { AnySchema } from '../DataModel/helperTypes'; +import { DeleteBlockerResource } from './store'; +import { RA } from '../../utils/types'; +import { useDeleteBlockerStore } from './Context'; + +export function useDeleteBlockersForResource( + resource: SpecifyResource +): RA | undefined { + const store = useDeleteBlockerStore(); + const blockerKey = resourceToStringIdentifier(resource); + + React.useEffect(() => { + void store.seedBlockers(resource); + }, [store, blockerKey]); + + const nodeVersion = React.useSyncExternalStore( + store.subscribe, + () => store.getNode(blockerKey)?.version ?? 0 + ); + + const records = React.useMemo( + () => store.getBlockerGraph(resource), + [store, nodeVersion, blockerKey] + ); + + return records; +} From 89cca6a90e3a8b892e651c1cb14b21daa313db1b Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 8 Oct 2026 20:49:29 -0500 Subject: [PATCH 29/66] feat: show delete blocker counts on aside --- .../DeleteBlockers/deleteBlockers.ts | 6 +- .../lib/components/DeleteBlockers/index.tsx | 45 +++++++++--- .../lib/components/DeleteBlockers/state.ts | 72 +++++++++++++++++-- 3 files changed, 103 insertions(+), 20 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/deleteBlockers.ts b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/deleteBlockers.ts index d907e917e94..e6bd28608be 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/deleteBlockers.ts +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/deleteBlockers.ts @@ -21,13 +21,13 @@ export type APIDeleteBlockers = { readonly next: RA; }; -type APIDeleteBlockerCount = { +export type APIDeleteBlockerCount = { readonly table: Lowercase; readonly field: string; readonly count: number; }; -type APIDeleteBlockerCounts = { +export type APIDeleteBlockerCounts = { readonly results: RA; readonly total_count: number; }; @@ -81,7 +81,7 @@ export async function filterDeleteBlockers( ).then(({ data }) => data); } -export async function fetchCounts( +export async function fetchReferenceCounts( table: keyof Tables, recordId: number, expectFailure = false diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx index 92ba82b6f74..98eb0943166 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx @@ -48,14 +48,12 @@ export function NewDeleteBlockers({ })) } /> -
- {paginatorKey !== undefined && ( - - )} -
+ {paginatorKey !== undefined && ( + + )}
); } @@ -102,6 +100,9 @@ function BlockerResource({ const [isOpen, _, __, handleToggleOpen] = useBooleanState(false); + const resourceLabel = + formatted === undefined ? commonText.loading() : formatted; + return (
- {formatted === undefined ? commonText.loading() : formatted} + {record.count !== undefined + ? commonText.countLine({ + resource: resourceLabel, + count: record.count, + }) + : // FIXME: Localize this? + `${resourceLabel} (${commonText.loading()})`} {isOpen && ( @@ -149,7 +156,15 @@ function DeleteBlockersTable({ <> - {localized(tableLabel)} + + {blockerTable.count !== undefined + ? commonText.countLine({ + resource: localized(tableLabel), + count: blockerTable.count, + }) + : // FIXME: Localize this? + `${localized(tableLabel)} (${commonText.loading()})`} + {isOpen && (
@@ -188,7 +203,15 @@ function DeleteBlockersRelationship({ handleRelationshipActive(relationship); }} > - {relationship.label} + + {blockerRelationship.count !== undefined + ? commonText.countLine({ + resource: relationship.label, + count: blockerRelationship.count, + }) + : // FIXME: Localize this? + `${relationship.label} (${commonText.loading()})`} + ); } diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/state.ts b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/state.ts index 1dae09e7a86..7bf3e305264 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/state.ts +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/state.ts @@ -6,8 +6,11 @@ import { getTable } from '../DataModel/tables'; import { Tables } from '../DataModel/types'; import { softFail } from '../Errors/Crash'; import { + APIDeleteBlockerCount, + APIDeleteBlockerCounts, DELETE_BLOCKER_LIMIT, fetchInitialBlockers, + fetchReferenceCounts, filterDeleteBlockers, } from './deleteBlockers'; import { @@ -27,15 +30,18 @@ export type BlockerRelationship = { readonly key: BlockerPageCacheKey; readonly table: Lowercase; readonly field: string; - readonly count?: number; + // eslint-disable-next-line functional/prefer-readonly-type + count?: number; }; export type BlockerNode = { readonly key: ResourceIdentifier; readonly resource: SpecifyResource; - readonly count?: number; readonly relationshipMetaData: Map; readonly cascadeChildren: Set; + // eslint-disable-next-line functional/prefer-readonly-type + count?: number; + // eslint-disable-next-line functional/prefer-readonly-type version: number; }; @@ -108,7 +114,7 @@ export class DeleteBlockerState { private getOrCreateNode(resource: SpecifyResource): BlockerNode { const resourceKey = resourceToStringIdentifier(resource); // REFACTOR: We could replace the following with getOrInsert - const existingNode = this.nodes.get(resourceKey); + const existingNode = this.getNode(resourceKey); if (existingNode !== undefined) { return existingNode; } @@ -149,7 +155,7 @@ export class DeleteBlockerState { table: Lowercase, field: string ) { - return makeBlockerKey(node.key, table, field); + return makeBlockerKey(node.key, table.toLowerCase(), field.toLowerCase()); } public async seedBlockers(resource: SpecifyResource) { @@ -181,6 +187,7 @@ export class DeleteBlockerState { const cachedPages = this.applyBlockerPages(node, blockers.results); this.updateAncestors(node.key); this.queueNextBlockers(resource, blockers.next); + this.queueBlockerCounts(resource); this.onChange?.(); return cachedPages; } @@ -236,7 +243,6 @@ export class DeleteBlockerState { } } - // FIXME: add count support public async queueBlockerCounts(resource: SpecifyResource) { const countKey = resourceToStringIdentifier(resource); const alreadyQueued = this.countPromiseQueue.get(countKey); @@ -246,6 +252,60 @@ export class DeleteBlockerState { if (alreadyQueued !== undefined) { return undefined; } + + const fetchCounts = () => + fetchReferenceCounts(resource.specifyTable.name, resource.id).then( + (counts) => this.handleDeleteBlockerCounts(resource, counts) + ); + + return this.countPromiseQueue.enqueue(countKey, fetchCounts); + } + + private handleDeleteBlockerCounts( + resource: SpecifyResource, + counts: APIDeleteBlockerCounts + ) { + const cacheKey = resourceToStringIdentifier(resource); + const node = this.getNode(cacheKey); + if (node === undefined) { + // We're expecting the node to exist at this point, but maybe the + // resource was deleted between the time the count request was made and + // this is executing? + console.warn( + 'Trying to handle counts for record not in DeleteBlocker state', + { resource: resource } + ); + return; + } + node.count = counts.total_count; + node.relationshipMetaData.forEach((blockerRelationship) => { + blockerRelationship.count = 0; + }); + counts.results.forEach((relationshipCount) => + this.addCountsToRelationship(node, relationshipCount) + ); + this.updateNode(node); + this.onChange?.(); + } + + private addCountsToRelationship( + node: BlockerNode, + referenceCounts: APIDeleteBlockerCount + ) { + const { table, field, count } = referenceCounts; + const relationshipKey = this.nodeRelationshipKey(node, table, field); + const relationship = node.relationshipMetaData.get(relationshipKey); + if (relationship === undefined) { + // We're expecting this relationship to already exist on the node. + // Maybe all records in the relationship were deleted and it was removed? + console.warn('Trying to handle counts for missing relationship', { + resource: resourceToStringIdentifier(node.resource), + table, + field, + }); + return; + } + relationship.count = count; } private async queueNextBlockers( @@ -333,7 +393,7 @@ export class DeleteBlockerState { continue; } visitedRecords.add(currentKey); - const node = this.nodes.get(currentKey); + const node = this.getNode(currentKey); if (node === undefined) { continue; } From 50a331cd3d86ba4303a6323a2ffba2dce561f1a6 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 8 Oct 2026 22:42:12 -0500 Subject: [PATCH 30/66] feat: improve styling of DeleteBlockerAside --- .../lib/components/DeleteBlockers/index.tsx | 74 +++++++++++++------ .../lib/components/DeleteBlockers/state.ts | 6 +- .../lib/components/DeleteBlockers/store.ts | 18 +++++ .../DeleteBlockers/useReferenceCount.tsx | 18 +++++ .../FormSliders/RecordSelectorFromPage.tsx | 6 -- 5 files changed, 92 insertions(+), 30 deletions(-) create mode 100644 specifyweb/frontend/js_src/lib/components/DeleteBlockers/useReferenceCount.tsx diff --git a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx index 98eb0943166..1ce3b79495d 100644 --- a/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx +++ b/specifyweb/frontend/js_src/lib/components/DeleteBlockers/index.tsx @@ -21,6 +21,7 @@ import { f } from '../../utils/functools'; import { Relationship } from '../DataModel/specifyField'; import { useDeleteBlockersForResource } from './useDeleteBlockersForResource'; import { useDeleteBlockerPages } from './useDeleteBlockerPages'; +import { useReferenceCount } from './useReferenceCount'; export function NewDeleteBlockers({ resource, @@ -34,6 +35,7 @@ export function NewDeleteBlockers({ | { readonly resource: SpecifyResource; readonly relationship: Relationship; + readonly relationshipKey: string | undefined; } >(undefined); @@ -41,10 +43,12 @@ export function NewDeleteBlockers({
+ activeRelationshipKey={paginatorKey?.relationshipKey} + onRelationshipActive={(resource, relationship, relationshipKey) => setPaginatorKey(() => ({ resource, relationship, + relationshipKey, })) } /> @@ -60,22 +64,24 @@ export function NewDeleteBlockers({ function DeleteBlockersAside({ records, + activeRelationshipKey, onRelationshipActive: handleRelationshipActive, }: { readonly records: RA; + readonly activeRelationshipKey: string | undefined; readonly onRelationshipActive: ( resource: SpecifyResource, - relationship: Relationship + relationship: Relationship, + relationshipKey: string ) => void; }): JSX.Element { return ( -