From 8b8ae1664ec204bbdf1b690158b8d4440591e37a Mon Sep 17 00:00:00 2001 From: Max Patiiuk Date: Wed, 8 Mar 2023 14:44:09 -0600 Subject: [PATCH 01/66] Allow independent subviews Fixes #114 --- specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx index 5c3d7504ef4..e700d3f771a 100644 --- a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx +++ b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx @@ -231,7 +231,7 @@ export function SubView({ formType={formType} mode={ !isAttachmentMisconfigured && - relationship.isDependent() && + (relationship.isDependent() || isButton) && initialMode !== 'view' ? 'edit' : 'view' From a9a0affe07468aa53d484b18c6b03eade68138ef Mon Sep 17 00:00:00 2001 From: Max Patiiuk Date: Wed, 8 Mar 2023 15:49:24 -0600 Subject: [PATCH 02/66] Turn independent sub views into buttons Fixes #3127 --- .../frontend/js_src/lib/components/Forms/SubView.tsx | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx index e700d3f771a..313942188ed 100644 --- a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx +++ b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx @@ -41,7 +41,7 @@ export function SubView({ mode: initialMode, parentFormType, formType: initialFormType, - isButton, + isButton: rawIsButton, viewName = relationship.relatedModel.view, icon = relationship.relatedModel.name, sortField: initialSortField, @@ -187,6 +187,9 @@ export function SubView({ [relationship, formType, sortField, setFormType, setSortField] ); + // See https://github.com/specify/specify7/issues/3127 + const isButton = + rawIsButton || (initialMode === 'edit' && !relationship.isDependent()); const [isOpen, _, handleClose, handleToggle] = useBooleanState(!isButton); const [isAttachmentConfigured] = usePromise(attachmentSettingsPromise, true); @@ -231,6 +234,11 @@ export function SubView({ formType={formType} mode={ !isAttachmentMisconfigured && + /* + * Only button subview's can display independent resources + * See https://github.com/specify/specify7/pull/3125#issue-1615911079 + * for reasons why. + */ (relationship.isDependent() || isButton) && initialMode !== 'view' ? 'edit' From 8db292583820d2948d0eb8f264d29d9e8851fc89 Mon Sep 17 00:00:00 2001 From: Jason Melton <64045831+melton-jason@users.noreply.github.com> Date: Fri, 24 Nov 2023 20:09:17 +0000 Subject: [PATCH 03/66] Lint code with ESLint and Prettier Triggered by dfaa5d09831607c3d4dea10b9ae2aaf15e95a298 on branch refs/heads/issue-114 --- .../lib/components/Statistics/Categories.tsx | 8 +++++--- .../js_src/lib/components/Statistics/hooks.tsx | 14 ++++++++------ 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/Statistics/Categories.tsx b/specifyweb/frontend/js_src/lib/components/Statistics/Categories.tsx index 771e7e109d8..dda226fa192 100644 --- a/specifyweb/frontend/js_src/lib/components/Statistics/Categories.tsx +++ b/specifyweb/frontend/js_src/lib/components/Statistics/Categories.tsx @@ -53,9 +53,11 @@ function ItemOverride({ const noAccessTables: RA = React.useMemo( () => filterArray([ - // Dummy value to get the tables involved in the backend queries. Need this - // to show no permission when backend query fails due to permission denied - // The backend tables could be stored separately to avoid this + /* + * Dummy value to get the tables involved in the backend queries. Need this + * to show no permission when backend query fails due to permission denied + * The backend tables could be stored separately to avoid this + */ backEndSpecResolve?.querySpec?.(backEndSpecResolve.responseKey), dynamicSpecResolve?.dynamicQuerySpec, ]) diff --git a/specifyweb/frontend/js_src/lib/components/Statistics/hooks.tsx b/specifyweb/frontend/js_src/lib/components/Statistics/hooks.tsx index 47236406e16..d219dadebd7 100644 --- a/specifyweb/frontend/js_src/lib/components/Statistics/hooks.tsx +++ b/specifyweb/frontend/js_src/lib/components/Statistics/hooks.tsx @@ -261,13 +261,13 @@ export function resolveStatsSpec( const pathToValue = item.pathToValue ?? statSpecItem.spec.pathToValue; return { type: 'BackEndStat', - pathToValue: pathToValue, + pathToValue, fetchUrl: statUrl, formatter: statSpecItem.spec.formatterGenerator(formatterSpec), querySpec: - pathToValue !== undefined - ? statSpecItem.spec.querySpec?.(pathToValue.toString()) - : undefined, + pathToValue === undefined + ? undefined + : statSpecItem.spec.querySpec?.(pathToValue.toString()), }; } if ( @@ -300,9 +300,11 @@ export function useResolvedStatSpec( const statsSpecRef = React.useRef>(undefined); if (rawStatsSpec !== undefined) { - if (rawStatsSpec.pathToValue !== undefined) + if (rawStatsSpec.pathToValue === undefined) { + statsSpecRef.current = undefined; + } else { statsSpecRef.current ??= rawStatsSpec; - else statsSpecRef.current = undefined; + } } return statsSpecRef.current ?? rawStatsSpec; From a373d00f7bc222a6ff3f08bbe29c50b799caaf3b Mon Sep 17 00:00:00 2001 From: Caroline D <108160931+CarolineDenis@users.noreply.github.com> Date: Wed, 17 Jan 2024 18:32:46 +0000 Subject: [PATCH 04/66] Lint code with ESLint and Prettier Triggered by 1359149a06fb2bca266f47e7a54db6379f7d8d79 on branch refs/heads/issue-114 --- .../js_src/lib/components/DataModel/businessRuleDefs.ts | 4 ++-- .../frontend/js_src/lib/components/DataModel/resource.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/businessRuleDefs.ts b/specifyweb/frontend/js_src/lib/components/DataModel/businessRuleDefs.ts index 391941aaffe..fe1a1452421 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/businessRuleDefs.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/businessRuleDefs.ts @@ -1,5 +1,5 @@ -import { BusinessRuleResult } from './businessRules'; -import { AnySchema, TableFields } from './helperTypes'; +import type { BusinessRuleResult } from './businessRules'; +import type { AnySchema, TableFields } from './helperTypes'; import { checkPrepAvailability, getTotalLoaned, diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/resource.ts b/specifyweb/frontend/js_src/lib/components/DataModel/resource.ts index 1197d2de2f8..e88ab595406 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/resource.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/resource.ts @@ -10,6 +10,7 @@ import { userPreferences } from '../Preferences/userPreferences'; import { formatUrl } from '../Router/queryString'; import { relationshipIsToMany } from '../WbPlanView/mappingHelpers'; import { addMissingFields } from './addMissingFields'; +import { getFieldsFromPath } from './businessRules'; import { serializeResource } from './helpers'; import type { AnySchema, @@ -21,7 +22,6 @@ import { getModel, schema } from './schema'; import type { SpecifyModel } from './specifyModel'; import type { Tables } from './types'; import { getUniquenessRules } from './uniquenessRules'; -import { getFieldsFromPath } from './businessRules'; // FEATURE: use this everywhere export const resourceEvents = eventListener<{ From 74502def697b5098e2cc1ee0c73aa5960ec6d0e6 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 26 Jun 2024 10:45:04 -0500 Subject: [PATCH 05/66] Allow modifying remote side of indedpenent resources via API --- specifyweb/specify/api.py | 41 +++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/specifyweb/specify/api.py b/specifyweb/specify/api.py index 4dfd4d4b88a..ae8972bb37e 100644 --- a/specifyweb/specify/api.py +++ b/specifyweb/specify/api.py @@ -601,17 +601,16 @@ def handle_to_many(collection, agent, obj, data: Dict[str, Any]) -> None: for field_name, val in list(data.items()): field = obj._meta.get_field(field_name) if not field.is_relation or (field.many_to_one or field.one_to_one): continue # Skip *-to-one fields. - - if isinstance(val, list): - assert isinstance(obj, models.Recordset) or obj.specify_model.get_field(field_name).dependent, \ - "got inline data for non dependent field %s in %s: %r" % (field_name, obj, val) - else: - # The field contains something other than nested data. - # Probably the URI of the collection of objects. - assert not obj.specify_model.get_field(field_name).dependent, \ - "didn't get inline data for dependent field %s in %s: %r" % (field_name, obj, val) - continue - + is_dependent = is_dependent_field(obj, field_name) + + if not isinstance(val, list): + if is_dependent: + raise AssertionError("didn't get inline data for dependent field %s in %s: %r" % (field_name, obj, val)) + else: + # The field contains something other than nested data. + # Probably the URI of the collection + continue + rel_model = field.related_model ids = [] # Ids not in this list will be deleted at the end. for rel_data in val: @@ -621,19 +620,27 @@ def handle_to_many(collection, agent, obj, data: Dict[str, Any]) -> None: rel_obj = update_obj(collection, agent, rel_model, rel_data['id'], rel_data['version'], rel_data, - parent_obj=obj) + parent_obj=obj if is_dependent_field(obj, field_name) else None) + else: # Create a new related object. rel_obj = create_obj(collection, agent, rel_model, rel_data, parent_obj=obj) + + if not is_dependent and not (isinstance(obj, models.Recordset) and field_name == 'recordsetitems'): + getattr(obj, field_name).add(rel_obj) ids.append(rel_obj.id) # Record the id as one to keep. # Delete related objects not in the ids list. # TODO: Check versions for optimistic locking. - to_delete = getattr(obj, field_name).exclude(id__in=ids) - for rel_obj in to_delete: - check_table_permissions(collection, agent, rel_obj, "delete") - auditlog.remove(rel_obj, agent, obj) - to_delete.delete() + to_remove = getattr(obj, field_name).exclude(id__in=ids).select_for_update() + if is_dependent or (isinstance(obj, models.Recordset) and field_name == 'recordsetitems'): + for rel_obj in to_remove: + check_table_permissions(collection, agent, rel_obj, "delete") + auditlog.remove(rel_obj, agent, obj) + + to_remove.delete() + else: + getattr(obj, field_name).remove(*list(to_remove)) @transaction.atomic def delete_resource(collection, agent, name, id, version) -> None: From a55de9ed95dc1c1a556def5c7ba42125a8dece62 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 26 Jun 2024 10:49:55 -0500 Subject: [PATCH 06/66] Add tests for inline independent resources --- specifyweb/permissions/permissions.py | 9 +-- specifyweb/specify/tests/test_api.py | 85 ++++++++++++++++++++++++++- 2 files changed, 88 insertions(+), 6 deletions(-) diff --git a/specifyweb/permissions/permissions.py b/specifyweb/permissions/permissions.py index 1fa6f90ec86..b01fdb38852 100644 --- a/specifyweb/permissions/permissions.py +++ b/specifyweb/permissions/permissions.py @@ -1,11 +1,10 @@ -from typing import Any, Callable, Tuple, List, Dict, Union, Iterable, Optional, NamedTuple +from typing import Any, Callable, Literal, List, Dict, Union, Iterable, Optional, NamedTuple import logging logger = logging.getLogger(__name__) from django.db import connection from django.db.models import Model -from django.core.exceptions import ObjectDoesNotExist from specifyweb.specify.models import Agent from specifyweb.specify.datamodel import Table @@ -172,7 +171,9 @@ def query(collectionid: Optional[int], userid: int, resource: str, action: str) ) -def check_table_permissions(collection, actor, obj, action: str) -> None: +TABLE_ACTION = Literal["read", "create", "update", "delete"] + +def check_table_permissions(collection, actor, obj, action: TABLE_ACTION) -> None: if isinstance(obj, Table): name = obj.name.lower() else: @@ -186,7 +187,7 @@ def check_field_permissions(collection, actor, obj, fields: Iterable[str], actio table = obj.specify_model.name.lower() enforce(collection, actor, [f'/field/{table}/{field}' for field in fields], action) -def table_permissions_checker(collection, actor, action: str) -> Callable[[Any], None]: +def table_permissions_checker(collection, actor, action: TABLE_ACTION) -> Callable[[Any], None]: def checker(obj) -> None: check_table_permissions(collection, actor, obj, action) return checker diff --git a/specifyweb/specify/tests/test_api.py b/specifyweb/specify/tests/test_api.py index b7568983f42..01a9558b198 100644 --- a/specifyweb/specify/tests/test_api.py +++ b/specifyweb/specify/tests/test_api.py @@ -518,7 +518,9 @@ def test_update_object_with_more_inlines(self): even_dets = [d for d in data['determinations'] if d['number1'] % 2 == 0] for d in even_dets: data['determinations'].remove(d) - data['collectionobjectattribute'] = {'text1': 'look! an attribute'} + text1_data = 'look! an attribute' + + data['collectionobjectattribute'] = {'text1': text1_data} api.update_obj(self.collection, self.agent, 'collectionobject', data['id'], data['version'], data) @@ -528,9 +530,88 @@ def test_update_object_with_more_inlines(self): for d in obj.determinations.all(): self.assertFalse(d.number1 % 2 == 0) - self.assertEqual(obj.collectionobjectattribute.text1, 'look! an attribute') + self.assertEqual(obj.collectionobjectattribute.text1, text1_data) + + def test_independent_set_inline(self): + accession_data = { + 'accessionnumber': "a", + 'division': api.uri_for_model('division', self.division.id), + 'collectionobjects': [ + api.obj_to_data(self.collectionobjects[0]) + ] + } + + accession = api.create_obj(self.collection, self.agent, 'Accession', accession_data) + self.collectionobjects[0].refresh_from_db() + self.assertEqual(accession, self.collectionobjects[0].accession) + + def test_indepenent_removing_from_inline(self): + accession = models.Accession.objects.create( + accessionnumber="a", + version="0", + division=self.division + ) + + accession.collectionobjects.set(self.collectionobjects) + + collection_objects_to_set = [self.collectionobjects[0], self.collectionobjects[3]] + + accession_data = { + 'accessionnumber': "a", + 'division': api.uri_for_model('division', self.division.id), + 'collectionobjects': [ + api.obj_to_data(collection_object) for collection_object in collection_objects_to_set + ] + } + accession = api.update_obj(self.collection, self.agent, 'Accession', accession.id, accession.version, accession_data) + + self.assertEqual(list(accession.collectionobjects.all()), collection_objects_to_set) + + # ensure the other CollectionObjects have not been deleted + self.assertEqual(len(models.Collectionobject.objects.all()), len(self.collectionobjects)) + + def test_updating_independent_resource(self): + co_to_modify = api.obj_to_data(self.collectionobjects[2]) + co_to_modify.update({ + 'integer1': 10, + 'determinations': [ + { + 'iscurrent': True, + 'collectionmemberid': self.collection.id, + 'collectionobject': api.uri_for_model('Collectionobject', self.collectionobjects[2].id) + } + ] + }) + accession_data = { + 'accessionnumber': "a", + 'division': api.uri_for_model('division', self.division.id), + 'collectionobjects': [ + co_to_modify + ] + } + + accession = api.create_obj(self.collection, self.agent, 'Accession', accession_data) + self.collectionobjects[2].refresh_from_db() + self.assertEqual(self.collectionobjects[2].integer1, 10) + self.assertEqual(len(self.collectionobjects[2].determinations.all()), 1) + + def test_independent_creating_from_remoteside(self): + new_catalognumber = f'num-{len(self.collectionobjects)}' + accession_data = { + 'accessionnumber': "a", + 'division': api.uri_for_model('division', self.division.id), + 'collectionobjects': [ + { + 'catalognumber': new_catalognumber, + 'collection': api.uri_for_model('Collection', self.collection.id) + } + ] + } + accession = api.create_obj(self.collection, self.agent, 'Accession', accession_data) + self.assertTrue(models.Collectionobject.objects.filter(catalognumber=new_catalognumber).exists()) + # version control on inlined resources should be tested From d970ca175f9f6f45330c74822134248e0ef7597e Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 27 Jun 2024 13:44:49 -0500 Subject: [PATCH 07/66] Write test for reassigning to-one relationship from to-many side --- specifyweb/specify/tests/test_api.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/specifyweb/specify/tests/test_api.py b/specifyweb/specify/tests/test_api.py index 01a9558b198..d1a6064b629 100644 --- a/specifyweb/specify/tests/test_api.py +++ b/specifyweb/specify/tests/test_api.py @@ -611,6 +611,26 @@ def test_independent_creating_from_remoteside(self): accession = api.create_obj(self.collection, self.agent, 'Accession', accession_data) self.assertTrue(models.Collectionobject.objects.filter(catalognumber=new_catalognumber).exists()) + + def test_reassigning_independent(self): + acc1 = models.Accession.objects.create( + accessionnumber="a", + division = self.division + ) + + self.collectionobjects[0].accession = acc1 + self.collectionobjects[0].save() + + accession_data = { + 'accessionnumber': "b", + 'division': api.uri_for_model('division', self.division.id), + 'collectionobjects': [ + api.obj_to_data(self.collectionobjects[0]) + ] + } + acc2 = api.create_obj(self.collection, self.agent, 'Accession', accession_data) + self.collectionobjects[0].refresh_from_db() + self.assertEqual(self.collectionobjects[0].accession, acc2) # version control on inlined resources should be tested From efd44b94be9ce1c7a1dd14f75d9f8c4654561207 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 3 Jul 2024 10:53:03 -0500 Subject: [PATCH 08/66] Modify frontend orm to handle indpendent to-many relationships --- .../lib/components/DataModel/collectionApi.ts | 5 +- .../lib/components/DataModel/resourceApi.ts | 186 +++++++++++++----- .../lib/components/DataModel/specifyTable.ts | 2 +- 3 files changed, 143 insertions(+), 50 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts index f74ebf47166..15a29079a72 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts @@ -88,9 +88,10 @@ export const DependentCollection = Base.extend({ export const LazyCollection = Base.extend({ __name__: 'LazyCollectionBase', _neverFetched: true, - constructor(options = {}) { + constructor(options = {}, records = []) { this.table = this.model; - Base.call(this, null, options); + assert(_.isArray(records)); + Base.call(this, records, options); this.filters = options.filters || {}; this.domainfilter = Boolean(options.domainfilter) && diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts index 67b1c07982b..2356ee4ab84 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts @@ -4,12 +4,16 @@ import _ from 'underscore'; import { hijackBackboneAjax } from '../../utils/ajax/backboneAjax'; import { Http } from '../../utils/ajax/definitions'; +import { filterArray, ValueOf } from '../../utils/types'; import { removeKey } from '../../utils/utils'; import { assert } from '../Errors/assert'; import { softFail } from '../Errors/Crash'; +import { relationshipIsToMany } from '../WbPlanView/mappingHelpers'; import { Backbone } from './backbone'; import { attachBusinessRules } from './businessRules'; import { backboneFieldSeparator } from './helpers'; +import { AnySchema, SerializedResource, SerializedRecord } from './helperTypes'; +import { SpecifyResource } from './legacyTypes'; import { getFieldsToNotClone, getResourceApiUrl, @@ -18,7 +22,12 @@ import { resourceFromUrl, } from './resource'; import { initializeResource } from './scoping'; -import { specialFields } from './serializers'; +import { serializeResource, specialFields } from './serializers'; +import { LiteralField, Relationship } from './specifyField'; +import { Collection, SpecifyTable } from './specifyTable'; +import { Tables } from './types'; +import { IR } from '../../utils/types'; +import { tables } from './tables'; // REFACTOR: remove @ts-nocheck @@ -76,7 +85,19 @@ function eventHandlerForToMany(_related, field) { } // Always returns a resource -const maybeMakeResource = (value, relatedTable) => +const maybeMakeResource = < + TABLE extends SpecifyTable, + TABLE_SCHEMA extends Tables[TABLE['name']] +>( + value: + | SpecifyResource + | Partial< + | SerializedResource + | SerializedRecord + | { readonly id?: number } + >, + relatedTable: TABLE +): SpecifyResource => value instanceof ResourceBase ? value : new relatedTable.Resource(value, { parse: true }); @@ -100,6 +121,7 @@ export const ResourceBase = Backbone.Model.extend({ constructor() { this.specifyTable = this.constructor.specifyTable; this.dependentResources = {}; // References to related objects referred to by field in this resource + this.independentResources = {}; Reflect.apply(Backbone.Model, this, arguments); // TEST: check if this is necessary }, initialize(attributes, options) { @@ -211,7 +233,10 @@ export const ResourceBase = Backbone.Model.extend({ // Case insensitive return Backbone.Model.prototype.get.call(this, attribute.toLowerCase()); }, - storeDependent(field, related) { + storeDependent( + field: Relationship, + related: Collection | SpecifyResource | null + ): void { assert(field.isDependent()); const setter = field.type === 'one-to-many' @@ -219,7 +244,7 @@ export const ResourceBase = Backbone.Model.extend({ : '_setDependentToOne'; this[setter](field, related); }, - _setDependentToOne(field, related) { + _setDependentToOne(field: Relationship, related) { const oldRelated = this.dependentResources[field.name.toLowerCase()]; if (!related) { if (oldRelated) { @@ -255,7 +280,7 @@ export const ResourceBase = Backbone.Model.extend({ } } }, - _setDependentToMany(field, toMany) { + _setDependentToMany(field: Relationship, toMany: Collection) { const oldToMany = this.dependentResources[field.name.toLowerCase()]; oldToMany && oldToMany.off('all', null, this); @@ -263,6 +288,14 @@ export const ResourceBase = Backbone.Model.extend({ this.dependentResources[field.name.toLowerCase()] = toMany; toMany.on('all', eventHandlerForToMany(toMany, field), this); }, + storeIndependentToMany(field: Relationship, toMany: Collection) { + assert(!field.isDependent() && relationshipIsToMany(field)); + const oldToMany = this.independentResources[field.name.toLowerCase()]; + if (oldToMany !== undefined) oldToMany.off('all', null, this); + + this.independentResources[field.name.toLowerCase()] = toMany; + toMany.on('all', eventHandlerForToMany(toMany, field), this); + }, // Separate name to simplify typing bulkSet(attributes, options) { return this.set(attributes, options); @@ -383,7 +416,7 @@ export const ResourceBase = Backbone.Model.extend({ }, _handleInlineDataOrResource(value, fieldName) { // BUG: check type of value - const field = this.specifyTable.getField(fieldName); + const field: Relationship = this.specifyTable.getField(fieldName); const relatedTable = field.relatedTable; // BUG: don't do anything for virtual fields @@ -399,15 +432,23 @@ export const ResourceBase = Backbone.Model.extend({ ); this.storeDependent(field, collection); } else { - console.warn( - 'got unexpected inline data for independent collection field', - { collection: this, field, value } + const collection = new relatedTable.ToOneCollection( + collectionOptions, + value ); + this.storeIndependentToMany(field, collection); } // Because the foreign key is on the other side this.trigger(`change:${fieldName}`, this); this.trigger('change', this); + + /** + * These are serialized and added to the JSON before being sent to the + * server and are not in the resource's attributes + * + * https://backbonejs.org/#Sync + */ return undefined; } case 'many-to-one': { @@ -533,9 +574,13 @@ export const ResourceBase = Backbone.Model.extend({ return value; }); }, - async _rget(path, options) { + async _rget( + path: RA, + options: OPTIONS + ) { let fieldName = path[0].toLowerCase(); - const field = this.specifyTable.getField(fieldName); + const field: LiteralField | Relationship | undefined = + this.specifyTable.getField(fieldName); field && (fieldName = field.name.toLowerCase()); // In case fieldName is an alias let value = this.get(fieldName); field || @@ -588,41 +633,9 @@ export const ResourceBase = Backbone.Model.extend({ throw "can't traverse into a collection using dot notation"; } - // Is the collection cached? - let toMany = this.dependentResources[fieldName]; - if (!toMany) { - const collectionOptions = { - field: field.getReverse(), - related: this, - }; - - if (!field.isDependent()) { - return new related.ToOneCollection(collectionOptions); - } - - if (this.isNew()) { - toMany = new related.DependentCollection(collectionOptions, []); - this.storeDependent(field, toMany); - return toMany; - } else { - console.warn('expected dependent resource to be in cache'); - const temporaryCollection = new related.ToOneCollection( - collectionOptions - ); - return temporaryCollection - .fetch({ limit: 0 }) - .then( - () => - new related.DependentCollection( - collectionOptions, - temporaryCollection.tables - ) - ) - .then((toMany) => { - _this.storeDependent(field, toMany); - }); - } - } + return field.isDependent() + ? this.getDependentToMany(field) + : this.getIndependentToMany(field); } case 'zero-to-one': { /* @@ -664,7 +677,77 @@ export const ResourceBase = Backbone.Model.extend({ } } }, - save({ + async getDependentToMany( + field: Relationship + ): Promise> { + assert(field.isDependent()); + + const fieldName = field.name.toLowerCase(); + const relatedTable = field.relatedTable; + + let toMany = this.dependentResources[fieldName]; + + if (toMany) return toMany; + + const collectionOptions = { + field: field.getReverse(), + related: this, + }; + + if (this.isNew()) { + toMany = new relatedTable.DependentCollection(collectionOptions, []); + this.storeDependent(field, toMany); + return toMany; + } else { + console.warn('expected dependent resource to be in cache'); + const temporaryCollection = new relatedTable.ToOneCollection( + collectionOptions + ); + return temporaryCollection + .fetch({ limit: 0 }) + .then( + () => + new relatedTable.DependentCollection( + collectionOptions, + temporaryCollection.tables + ) + ) + .then((toMany) => { + _this.storeDependent(field, toMany); + }); + } + }, + async getIndependentToMany( + field: Relationship + ): Promise> { + assert(!field.isDependent()); + + const fieldName = field.name.toLowerCase(); + const relatedTable = field.relatedTable; + + let toMany: Collection = this.independentResources[fieldName]; + + if (toMany) return toMany; + + const collectionOptions = { + field: field.getReverse(), + related: this, + }; + + if (this.isNew()) { + toMany = new relatedTable.ToOneCollection(collectionOptions); + this.storeIndependentToMany(field, toMany); + return toMany; + } + + return new relatedTable.ToOneCollection(collectionOptions) + .fetch({ limit: 0 }) + .then((collection) => { + this.storeIndependentToMany(field, collection); + return collection; + }); + }, + async save({ onSaveConflict: handleSaveConflict, errorOnAlreadySaving = true, } = {}) { @@ -730,6 +813,15 @@ export const ResourceBase = Backbone.Model.extend({ json[fieldName] = related ? related.toJSON() : null; } }); + + Object.entries(self.independentResources).forEach( + ([fieldName, related]) => { + const field = self.specifyTable.getField(fieldName); + if (field.type === 'one-to-many' && related) { + json[fieldName] = related.toJSON(); + } + } + ); if (typeof this.get('resource_uri') !== 'string') json._tableName = this.specifyTable.name; return json; diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts b/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts index 8ce8576e7a3..36c7b1de01e 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts @@ -73,7 +73,7 @@ type CollectionConstructor = new ( >; readonly domainfilter?: boolean; }, - tables?: RA> + initalResources?: RA> ) => UnFetchedCollection; export type UnFetchedCollection = { From f2a439791d1648394b8f5941aaccea0b72cc1f0b Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 25 Jul 2024 10:28:03 -0500 Subject: [PATCH 09/66] Add Independent Collection to Collection API --- .../lib/components/DataModel/collectionApi.ts | 74 ++++++++++++++++++- .../lib/components/DataModel/resourceApi.ts | 6 +- .../lib/components/DataModel/specifyTable.ts | 8 ++ 3 files changed, 82 insertions(+), 6 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts index 15a29079a72..02692b7a657 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts @@ -85,13 +85,81 @@ export const DependentCollection = Base.extend({ create: notSupported, }); +export const IndependentCollection = Base.extend({ + __name__: 'IndependentCollectionBase', + constructor(options, records = []) { + this.table = this.model; + assert(_.isArray(records)); + Base.call(this, records, options); + this.filters = options.filters || {}; + this.domainfilter = + Boolean(options.domainfilter) && + this.model?.specifyTable.getScopingRelationship() !== undefined; + }, + initialize(_tables, options) { + this.on( + 'add remove', + function () { + /* + * Warning: changing a collection record does not trigger a + * change event in the parent (though it probably should) + */ + this.trigger('saverequired'); + }, + this + ); + + setupToOne(this, options); + }, + url() { + return `/api/specify/${this.model.specifyTable.name.toLowerCase()}/`; + }, + parse(resp) { + let objects; + if (resp.meta) { + objects = resp.objects; + } else { + console.warn("expected 'meta' in response"); + objects = resp; + } + + return objects; + }, + async fetch(options) { + if (this.related.isNew()) { + return this; + } + + this.filters[this.field.name.toLowerCase()] = this.related.id; + if (this._fetch) return this._fetch; + + options ||= {}; + + options.update = true; + options.remove = false; + options.silent = true; + assert(options.at == null); + + options.data = + options.data || + _.extend({ domainfilter: this.domainfilter }, this.filters); + options.data.offset = this.length; + + _(options).has('limit') && (options.data.limit = options.limit); + this._fetch = Backbone.Collection.prototype.fetch.call(this, options); + return this._fetch.then(() => { + this._fetch = null; + return this; + }); + }, +}); + export const LazyCollection = Base.extend({ __name__: 'LazyCollectionBase', _neverFetched: true, - constructor(options = {}, records = []) { + constructor(options = {}) { this.table = this.model; - assert(_.isArray(records)); - Base.call(this, records, options); + Base.call(this, null, options); this.filters = options.filters || {}; this.domainfilter = Boolean(options.domainfilter) && diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts index 3e75eeecbee..a7e346afa90 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts @@ -434,7 +434,7 @@ export const ResourceBase = Backbone.Model.extend({ ); this.storeDependent(field, collection); } else { - const collection = new relatedTable.ToOneCollection( + const collection = new relatedTable.IndependentCollection( collectionOptions, value ); @@ -737,12 +737,12 @@ export const ResourceBase = Backbone.Model.extend({ }; if (this.isNew()) { - toMany = new relatedTable.ToOneCollection(collectionOptions); + toMany = new relatedTable.IndependentCollection(collectionOptions); this.storeIndependentToMany(field, toMany); return toMany; } - return new relatedTable.ToOneCollection(collectionOptions) + return new relatedTable.IndependentCollection(collectionOptions) .fetch({ limit: 0 }) .then((collection) => { this.storeIndependentToMany(field, collection); diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts b/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts index 36c7b1de01e..b8dc99894ef 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts @@ -16,6 +16,7 @@ import { parentTableRelationship } from '../Forms/parentTables'; import { relationshipIsToMany } from '../WbPlanView/mappingHelpers'; import { DependentCollection, + IndependentCollection, LazyCollection, ToOneCollection, } from './collectionApi'; @@ -177,6 +178,8 @@ export class SpecifyTable { */ public readonly DependentCollection: CollectionConstructor; + public readonly IndependentCollection: CollectionConstructor; + /** * A Backbone collection for loading a collection of items of this type as a * backwards -to-one collection of some other resource. @@ -235,6 +238,11 @@ export class SpecifyTable { model: this.Resource, }); + this.IndependentCollection = IndependentCollection.extend({ + __name__: `${this.name}IndependentCollection`, + model: this.Resource, + }); + this.ToOneCollection = ToOneCollection.extend({ __name__: `${this.name}ToOneCollection`, model: this.Resource, From e63365cae95c9fbcaa48bd440dfb89fd2da78132 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 25 Jul 2024 21:01:38 -0500 Subject: [PATCH 10/66] Recompute resource delete blockers on save --- .../js_src/lib/components/Forms/DeleteButton.tsx | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/specifyweb/frontend/js_src/lib/components/Forms/DeleteButton.tsx b/specifyweb/frontend/js_src/lib/components/Forms/DeleteButton.tsx index 3d0a7c8bf4f..f6706b4e45f 100644 --- a/specifyweb/frontend/js_src/lib/components/Forms/DeleteButton.tsx +++ b/specifyweb/frontend/js_src/lib/components/Forms/DeleteButton.tsx @@ -17,6 +17,7 @@ import { icons } from '../Atoms/Icons'; import { LoadingContext } from '../Core/Contexts'; import type { AnySchema } from '../DataModel/helperTypes'; import type { SpecifyResource } from '../DataModel/legacyTypes'; +import { resourceOn } from '../DataModel/resource'; import { serializeResource } from '../DataModel/serializers'; import type { Relationship } from '../DataModel/specifyField'; import { strictGetTable } from '../DataModel/tables'; @@ -71,6 +72,19 @@ export function DeleteButton({ false ); + React.useEffect( + () => + deferred + ? undefined + : resourceOn( + resource, + 'saved', + () => void fetchBlockers(resource).then(setBlockers), + false + ), + [resource, deferred] + ); + const [isOpen, handleOpen, handleClose] = useBooleanState(); const loading = React.useContext(LoadingContext); From 5b41b023796fe0827d4ede53c64ef740eeca0f58 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 25 Jul 2024 21:42:36 -0500 Subject: [PATCH 11/66] Don't default independent subviews as buttons --- .../frontend/js_src/lib/components/Forms/SubView.tsx | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx index e0fdb77cf1b..2c675d7ef66 100644 --- a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx +++ b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx @@ -41,7 +41,7 @@ export function SubView({ parentResource, parentFormType, formType: initialFormType, - isButton: rawIsButton, + isButton, viewName = relationship.relatedTable.view, icon = relationship.relatedTable.name, sortField: initialSortField, @@ -190,8 +190,6 @@ export function SubView({ const isReadOnly = React.useContext(ReadOnlyContext); - // See https://github.com/specify/specify7/issues/3127 - const isButton = rawIsButton || (!isReadOnly && !relationship.isDependent()); const [isOpen, _, handleClose, handleToggle] = useBooleanState(!isButton); const [isAttachmentConfigured] = usePromise(attachmentSettingsPromise, true); @@ -238,11 +236,7 @@ export function SubView({ )} {typeof collection === 'object' && isOpen ? ( Date: Thu, 25 Jul 2024 21:52:57 -0500 Subject: [PATCH 12/66] Propagate save blockers on independent resources to related record This is not likely the behavior we want, so this is something to be discussed. See related #3127 --- .../lib/components/DataModel/legacyTypes.ts | 4 ++ .../lib/components/DataModel/saveBlockers.tsx | 41 +++++++++---------- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/legacyTypes.ts b/specifyweb/frontend/js_src/lib/components/DataModel/legacyTypes.ts index bf0d9ee2289..46b65d3b3df 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/legacyTypes.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/legacyTypes.ts @@ -156,6 +156,10 @@ export type SpecifyResource = { ): SpecifyResource; // Not type safe bulkSet(value: IR): SpecifyResource; + //Unsafe + readonly independentResources: IR< + Collection | SpecifyResource | null | undefined + >; // Unsafe. Use getDependentResource instead whenever possible readonly dependentResources: IR< Collection | SpecifyResource | null | undefined diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/saveBlockers.tsx b/specifyweb/frontend/js_src/lib/components/DataModel/saveBlockers.tsx index 117f0e085e0..f8e982124f3 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/saveBlockers.tsx +++ b/specifyweb/frontend/js_src/lib/components/DataModel/saveBlockers.tsx @@ -200,28 +200,25 @@ const getAllBlockers = ( resources: [resource], })) ?? []), ...filterArray( - Object.entries(resource.dependentResources).flatMap( - ([fieldName, collectionOrResource]) => - (filterBlockers !== undefined && - fieldName.toLowerCase() !== filterBlockers?.name.toLowerCase()) || - collectionOrResource === undefined || - collectionOrResource === null - ? undefined - : (collectionOrResource instanceof ResourceBase - ? getAllBlockers( - collectionOrResource as SpecifyResource - ) - : (collectionOrResource as Collection).models.flatMap( - f.unary(getAllBlockers) - ) - ).map(({ field, resources, message }) => ({ - field: [ - resource.specifyTable.strictGetField(fieldName), - ...field, - ], - resources: [...resources, resource], - message, - })) + Object.entries({ + ...resource.dependentResources, + ...resource.independentResources, + }).flatMap(([fieldName, collectionOrResource]) => + (filterBlockers !== undefined && + fieldName.toLowerCase() !== filterBlockers?.name.toLowerCase()) || + collectionOrResource === undefined || + collectionOrResource === null + ? undefined + : (collectionOrResource instanceof ResourceBase + ? getAllBlockers(collectionOrResource as SpecifyResource) + : (collectionOrResource as Collection).models.flatMap( + f.unary(getAllBlockers) + ) + ).map(({ field, resources, message }) => ({ + field: [resource.specifyTable.strictGetField(fieldName), ...field], + resources: [...resources, resource], + message, + })) ) ), ]; From 59d1ff86a7c958bf4e52b8c7560f5c2d7fb3ce7e Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 22 Aug 2024 09:45:24 -0500 Subject: [PATCH 13/66] Add CO -> isMemberOfCOG virtual field to frontend --- .../js_src/lib/components/DataModel/schemaExtras.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/schemaExtras.ts b/specifyweb/frontend/js_src/lib/components/DataModel/schemaExtras.ts index 6fbde1459ac..c073c863c11 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/schemaExtras.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/schemaExtras.ts @@ -83,6 +83,14 @@ export const schemaExtras: { indexed: false, unique: false, }), + new LiteralField(table, { + name: 'isMemberOfCOG', + required: false, + readOnly: true, + type: 'java.lang.Boolean', + indexed: false, + unique: false, + }), ], (): void => { const collection = getField(table, 'collection'); From 4abacf0e4a904431768ef83564727198c72b4e9f Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 22 Aug 2024 09:46:59 -0500 Subject: [PATCH 14/66] Improve date backendfilters on frontend --- .../js_src/lib/components/DataModel/helpers.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/helpers.ts b/specifyweb/frontend/js_src/lib/components/DataModel/helpers.ts index 8ccdfdb534b..3e04d2b4c96 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/helpers.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/helpers.ts @@ -1,5 +1,6 @@ import { f } from '../../utils/functools'; import type { RA, ValueOf } from '../../utils/types'; +import { caseInsensitiveHash } from '../../utils/utils'; import { isTreeResource } from '../InitialContext/treeRanks'; import { relationshipIsToMany } from '../WbPlanView/mappingHelpers'; import type { @@ -40,7 +41,7 @@ const weekDayMap = { Thursday: 5, Friday: 6, Saturday: 7, -}; +} as const; /** * Use this to construct a query using a lookup for Django. @@ -99,7 +100,7 @@ export const backendFilter = (field: string) => ({ [[field, 'day'].join(djangoLookupSeparator)]: value, }), monthEquals: (value: number) => ({ - [[field, 'lte'].join(djangoLookupSeparator)]: value, + [[field, 'month'].join(djangoLookupSeparator)]: value, }), yearEquals: (value: number) => ({ [[field, 'year'].join(djangoLookupSeparator)]: value, @@ -107,8 +108,13 @@ export const backendFilter = (field: string) => ({ weekEquals: (value: number) => ({ [[field, 'week'].join(djangoLookupSeparator)]: value, }), - weekDayEquals: (value: keyof typeof weekDayMap) => ({ - [[field, 'week_day'].join(djangoLookupSeparator)]: weekDayMap[value], + weekDayEquals: ( + value: ValueOf | keyof typeof weekDayMap + ) => ({ + [[field, 'week_day'].join(djangoLookupSeparator)]: + typeof value === 'number' + ? value + : caseInsensitiveHash(weekDayMap, value), }), }); From dc570f65ebd9c79ec34937dde49987ec15189edf Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 22 Aug 2024 09:50:33 -0500 Subject: [PATCH 15/66] Allow creating new records in Independent ToMany Collections --- .../lib/components/DataModel/collectionApi.ts | 68 +++++++++++++----- .../lib/components/DataModel/resourceApi.ts | 51 ++++++------- .../lib/components/DataModel/saveBlockers.tsx | 7 +- .../lib/components/FormCells/FormTable.tsx | 71 ++++++++----------- .../FormSliders/IntegratedRecordSelector.tsx | 19 ++++- .../components/FormSliders/RecordSelector.tsx | 61 +++++++++------- .../RecordSelectorFromCollection.tsx | 5 +- .../js_src/lib/components/Forms/SubView.tsx | 11 ++- .../lib/components/SearchDialog/index.tsx | 50 +++++++++++-- .../WorkBench/useDisambiguationDialog.tsx | 3 +- specifyweb/specify/api.py | 31 ++++++-- specifyweb/specify/tests/test_api.py | 52 +++++++++++++- 12 files changed, 293 insertions(+), 136 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts index 02692b7a657..34371d71228 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts @@ -1,9 +1,13 @@ // @ts-nocheck import _ from 'underscore'; +import { removeKey } from '../../utils/utils'; import { assert } from '../Errors/assert'; +import { formatUrl } from '../Router/queryString'; import { Backbone } from './backbone'; +import { AnySchema } from './helperTypes'; +import { SpecifyResource } from './legacyTypes'; // REFACTOR: remove @ts-nocheck @@ -14,6 +18,10 @@ const Base = Backbone.Collection.extend({ }, }); +export const isRelationshipCollection = (value: unknown): boolean => + value instanceof DependentCollection || + value instanceof IndependentCollection; + function notSupported() { throw new Error('method is not supported'); } @@ -95,15 +103,25 @@ export const IndependentCollection = Base.extend({ this.domainfilter = Boolean(options.domainfilter) && this.model?.specifyTable.getScopingRelationship() !== undefined; + + this.changedResources = Object.fromEntries( + records.map((record) => [record.cid, false]) + ); }, initialize(_tables, options) { + this.on( + 'change', + function (resource: SpecifyResource) { + if (!resource.isBeingInitialized()) + this.changedResources[resource.cid] = true; + this.trigger('saverequired'); + }, + this + ); this.on( 'add remove', - function () { - /* - * Warning: changing a collection record does not trigger a - * change event in the parent (though it probably should) - */ + function (resource: SpecifyResource) { + this.changedResources[resource.cid] = false; this.trigger('saverequired'); }, this @@ -115,15 +133,14 @@ export const IndependentCollection = Base.extend({ return `/api/specify/${this.model.specifyTable.name.toLowerCase()}/`; }, parse(resp) { - let objects; + let records; if (resp.meta) { - objects = resp.objects; + records = resp.objects; } else { console.warn("expected 'meta' in response"); - objects = resp; + records = resp; } - - return objects; + return records; }, async fetch(options) { if (this.related.isNew()) { @@ -133,25 +150,38 @@ export const IndependentCollection = Base.extend({ this.filters[this.field.name.toLowerCase()] = this.related.id; if (this._fetch) return this._fetch; - options ||= {}; + options = { ...(options ?? {}), silent: true }; - options.update = true; - options.remove = false; - options.silent = true; assert(options.at == null); - options.data = - options.data || - _.extend({ domainfilter: this.domainfilter }, this.filters); - options.data.offset = this.length; + options.data = options.data || { + ...this.filters, + domainfilter: this.domainfilter, + limit: options.limit, + }; - _(options).has('limit') && (options.data.limit = options.limit); this._fetch = Backbone.Collection.prototype.fetch.call(this, options); return this._fetch.then(() => { this._fetch = null; return this; }); }, + isComplete() { + return true; + }, + toApiJSON(options) { + const self = this; + const resources = + Object.keys(self.changedResources).length === 0 + ? formatUrl(this.url(), this.filters) + : this.map(function (resource) { + return self.changedResources[resource.cid] === true + ? resource.toJSON(options) + : resource.url(); + }); + this.changedResources = []; + return resources; + }, }); export const LazyCollection = Base.extend({ diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts index 3d69ca9827e..24eee2f73dc 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts @@ -4,16 +4,20 @@ import _ from 'underscore'; import { hijackBackboneAjax } from '../../utils/ajax/backboneAjax'; import { Http } from '../../utils/ajax/definitions'; -import { filterArray, ValueOf } from '../../utils/types'; import { removeKey } from '../../utils/utils'; import { assert } from '../Errors/assert'; import { softFail } from '../Errors/Crash'; import { relationshipIsToMany } from '../WbPlanView/mappingHelpers'; import { Backbone } from './backbone'; import { attachBusinessRules } from './businessRules'; +import { isRelationshipCollection } from './collectionApi'; import { backboneFieldSeparator } from './helpers'; -import { AnySchema, SerializedResource, SerializedRecord } from './helperTypes'; -import { SpecifyResource } from './legacyTypes'; +import type { + AnySchema, + SerializedRecord, + SerializedResource, +} from './helperTypes'; +import type { SpecifyResource } from './legacyTypes'; import { getFieldsToNotClone, getResourceApiUrl, @@ -22,12 +26,10 @@ import { resourceFromUrl, } from './resource'; import { initializeResource } from './scoping'; -import { serializeResource, specialFields } from './serializers'; -import { LiteralField, Relationship } from './specifyField'; -import { Collection, SpecifyTable } from './specifyTable'; -import { Tables } from './types'; -import { IR } from '../../utils/types'; -import { tables } from './tables'; +import { specialFields } from './serializers'; +import type { LiteralField, Relationship } from './specifyField'; +import type { Collection, SpecifyTable } from './specifyTable'; +import type { Tables } from './types'; // REFACTOR: remove @ts-nocheck @@ -577,6 +579,11 @@ export const ResourceBase = Backbone.Model.extend({ if (!value) return value; // Ok if the related resource doesn't exist else if (typeof value.fetchIfNotPopulated === 'function') return value.fetchIfNotPopulated(); + /* + * Relationship Collections have already been fetched through _rget. + * This is needed to prevent refetching the collection with the default + * limit of 20 + */ else if (isRelationshipCollection(value)) return value; else if (typeof value.fetch === 'function') return value.fetch(); } return value; @@ -733,27 +740,23 @@ export const ResourceBase = Backbone.Model.extend({ const fieldName = field.name.toLowerCase(); const relatedTable = field.relatedTable; - let toMany: Collection = this.independentResources[fieldName]; - - if (toMany) return toMany; + const existingToMany: Collection | undefined = + this.independentResources[fieldName]; const collectionOptions = { field: field.getReverse(), related: this, }; - if (this.isNew()) { - toMany = new relatedTable.IndependentCollection(collectionOptions); - this.storeIndependentToMany(field, toMany); - return toMany; - } + const collection = + existingToMany === undefined + ? new relatedTable.IndependentCollection(collectionOptions) + : existingToMany; - return new relatedTable.IndependentCollection(collectionOptions) - .fetch({ limit: 0 }) - .then((collection) => { - this.storeIndependentToMany(field, collection); - return collection; - }); + return collection.fetch({ limit: 0 }).then((collection) => { + this.storeIndependentToMany(field, collection); + return collection; + }); }, async save({ onSaveConflict: handleSaveConflict, @@ -826,7 +829,7 @@ export const ResourceBase = Backbone.Model.extend({ ([fieldName, related]) => { const field = self.specifyTable.getField(fieldName); if (field.type === 'one-to-many' && related) { - json[fieldName] = related.toJSON(); + json[fieldName] = related.toApiJSON(); } } ); diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/saveBlockers.tsx b/specifyweb/frontend/js_src/lib/components/DataModel/saveBlockers.tsx index f8e982124f3..28501c12dc2 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/saveBlockers.tsx +++ b/specifyweb/frontend/js_src/lib/components/DataModel/saveBlockers.tsx @@ -9,6 +9,7 @@ import { eventListener } from '../../utils/events'; import { f } from '../../utils/functools'; import type { GetOrSet, RA } from '../../utils/types'; import { filterArray } from '../../utils/types'; +import type { SET } from '../../utils/utils'; import { removeItem } from '../../utils/utils'; import { softError } from '../Errors/assert'; import { softFail } from '../Errors/Crash'; @@ -108,10 +109,10 @@ export function useSaveBlockers( ]; } -export function setSaveBlockers( - resource: SpecifyResource, +export function setSaveBlockers( + resource: SpecifyResource, field: LiteralField | Relationship, - errors: Parameters>[1]>[0], + errors: Parameters>[typeof SET]>[0], blockerKey: string ): void { const resolvedErrors = diff --git a/specifyweb/frontend/js_src/lib/components/FormCells/FormTable.tsx b/specifyweb/frontend/js_src/lib/components/FormCells/FormTable.tsx index e58d70ba633..acc11b08d04 100644 --- a/specifyweb/frontend/js_src/lib/components/FormCells/FormTable.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormCells/FormTable.tsx @@ -1,6 +1,5 @@ import React from 'react'; import type { LocalizedString } from 'typesafe-i18n'; -import type { State } from 'typesafe-reducer'; import { useId } from '../../hooks/useId'; import { useInfiniteScroll } from '../../hooks/useInfiniteScroll'; @@ -31,7 +30,7 @@ import type { SortConfig } from '../Molecules/Sorting'; import { SortIndicator } from '../Molecules/Sorting'; import { hasTablePermission } from '../Permissions/helpers'; import { userPreferences } from '../Preferences/userPreferences'; -import { SearchDialog } from '../SearchDialog'; +import { SearchDialog, useSearchDialog } from '../SearchDialog'; import { AttachmentPluginSkeleton } from '../SkeletonLoaders/AttachmentPlugin'; import { relationshipIsToMany } from '../WbPlanView/mappingHelpers'; import { FormCell } from './index'; @@ -173,9 +172,6 @@ export function FormTable({ const [isExpanded, setExpandedRecords] = React.useState< IR >({}); - const [state, setState] = React.useState< - State<'MainState'> | State<'SearchState'> - >({ type: 'MainState' }); const [flexibleColumnWidth] = userPreferences.use( 'form', 'definition', @@ -198,6 +194,14 @@ export function FormTable({ const [maxHeight] = userPreferences.use('form', 'formTable', 'maxHeight'); + const { searchDialog, showSearchDialog } = useSearchDialog({ + forceCollection: undefined, + extraFilters: undefined, + table: relationship.relatedTable, + multiple: !isToOne, + onSelected: handleAddResources, + }); + const children = collapsedViewDefinition === undefined ? ( commonText.loading() @@ -387,7 +391,9 @@ export function FormTable({ )}
- {displayViewButton && isExpanded[resource.cid] === true ? ( + {displayViewButton && + isExpanded[resource.cid] === true && + !resource.isNew() ? ( ({
); - const addButton = + const addButtons = typeof handleAddResources === 'function' && mode !== 'view' && - !disableAdding && - hasTablePermission( - relationship.relatedTable.name, - isDependent ? 'create' : 'read' - ) ? ( - { - const resource = new relationship.relatedTable.Resource(); - handleAddResources([resource]); - } - : (): void => - setState({ - type: 'SearchState', - }) - } - /> + !disableAdding ? ( + <> + {!isDependent && + hasTablePermission(relationship.relatedTable.name, 'read') ? ( + + ) : undefined} + {hasTablePermission(relationship.relatedTable.name, 'create') ? ( + { + const resource = new relationship.relatedTable.Resource(); + handleAddResources([resource]); + }} + /> + ) : undefined} + ) : undefined; return dialog === false ? ( {preHeaderButtons} {header} - {addButton} + {addButtons} {children} - {state.type === 'SearchState' && - typeof handleAddResources === 'function' ? ( - setState({ type: 'MainState' })} - onSelected={handleAddResources} - /> - ) : undefined} + {searchDialog} ) : ( diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx index 8e594d2a0a6..120c896eba6 100644 --- a/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx @@ -120,7 +120,7 @@ export function IntegratedRecordSelector({ collection={collection} defaultIndex={isToOne ? 0 : index} relationship={relationship} - onAdd={(resources) => { + onAdd={(resources): void => { if (isInteraction) { setInteractionResource(resources[0]); handleOpenDialog(); @@ -145,6 +145,7 @@ export function IntegratedRecordSelector({ resource, onAdd: handleAdd, onRemove: handleRemove, + showSearchDialog, isLoading, }): JSX.Element => ( <> @@ -178,9 +179,23 @@ export function IntegratedRecordSelector({ !isDependent && dialog === false ? resource : undefined } /> + {!isDependent && + hasTablePermission( + relationship.relatedTable.name, + 'read' + ) && + typeof handleAdd === 'function' ? ( + 0) + } + onClick={showSearchDialog} + /> + ) : undefined} {hasTablePermission( relationship.relatedTable.name, - isDependent ? 'create' : 'read' + 'create' ) && typeof handleAdd === 'function' ? ( = { @@ -56,6 +56,7 @@ export type RecordSelectorState = { readonly onRemove: | ((source: 'deleteButton' | 'minusButton') => void) | undefined; + readonly showSearchDialog: () => void; // True while fetching new record readonly isLoading: boolean; }; @@ -82,9 +83,33 @@ export function useRecordSelector({ [index] ); - const [state, setState] = React.useState< - State<'AddBySearch'> | State<'Main'> - >({ type: 'Main' }); + const isToOne = !relationshipIsToMany(field) || field?.type === 'zero-to-one'; + + const handleResourcesSelected = React.useMemo( + () => + typeof handleAdded === 'function' + ? (resources: RA>): void => { + if (field?.isDependent() ?? true) + f.maybe(field?.otherSideName, (fieldName) => + f.maybe(relatedResource?.url(), (url) => + resources.forEach((resource) => { + resource.set(fieldName, url as never); + }) + ) + ); + handleAdded(resources); + } + : undefined, + [handleAdded, relatedResource, field] + ); + + const { searchDialog, showSearchDialog } = useSearchDialog({ + extraFilters: undefined, + forceCollection: undefined, + multiple: !isToOne, + table, + onSelected: handleResourcesSelected, + }); return { slider: ( @@ -94,7 +119,7 @@ export function useRecordSelector({ onChange={ handleSlide === undefined ? undefined - : (index) => handleSlide?.(index, false) + : (index): void => handleSlide?.(index, false) } /> ), @@ -103,26 +128,7 @@ export function useRecordSelector({ isLoading: records[index] === undefined && totalCount !== 0, // While new resource is loading, display previous resource resource: records[index] ?? records[lastIndexRef.current], - dialogs: - state.type === 'AddBySearch' && typeof handleAdded === 'function' ? ( - setState({ type: 'Main' })} - onSelected={(resources): void => { - f.maybe(field?.otherSideName, (fieldName) => - f.maybe(relatedResource?.url(), (url) => - resources.forEach((resource) => - resource.set(fieldName, url as never) - ) - ) - ); - handleAdded(resources); - }} - /> - ) : null, + dialogs: searchDialog, onAdd: typeof handleAdded === 'function' ? (resources: RA>): void => { @@ -134,7 +140,7 @@ export function useRecordSelector({ ) resource.set(field.otherSideName, relatedResource.url() as any); handleAdded([resource]); - } else setState({ type: 'AddBySearch' }); + } else showSearchDialog(); } : undefined, onRemove: @@ -156,5 +162,6 @@ export function useRecordSelector({ ) : undefined : undefined, + showSearchDialog, }; } diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx index 76485597dfd..7c72ed933e5 100644 --- a/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx @@ -63,7 +63,7 @@ export function RecordSelectorFromCollection({ () => resourceOn( collection, - 'add remove destroy', + 'add remove destroy sync', (): void => setRecords(getRecords), true ), @@ -94,8 +94,9 @@ export function RecordSelectorFromCollection({ ...rest, index, table: collection.table.specifyTable, + field: relationship, records, - relatedResource: isDependent ? collection.related : undefined, + relatedResource: isLazy ? undefined : collection.related, totalCount: collection._totalCount ?? records.length, onAdd: (rawResources): void => { const resources = isToOne ? rawResources.slice(0, 1) : rawResources; diff --git a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx index 2c675d7ef66..51f181452e2 100644 --- a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx +++ b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx @@ -4,7 +4,7 @@ import { usePromise } from '../../hooks/useAsyncState'; import { useBooleanState } from '../../hooks/useBooleanState'; import { useTriggerState } from '../../hooks/useTriggerState'; import { commonText } from '../../localization/common'; -import { overwriteReadOnly } from '../../utils/types'; +import { overwriteReadOnly, RA } from '../../utils/types'; import { sortFunction } from '../../utils/utils'; import { Button } from '../Atoms/Button'; import { attachmentSettingsPromise } from '../Attachments/attachments'; @@ -118,10 +118,9 @@ export function SubView({ related: parentResource, field: reverse, }) - : new relationship.relatedTable.LazyCollection({ - filters: { - [reverse.name]: parentResource.id, - }, + : new relationship.relatedTable.IndependentCollection({ + related: parentResource, + field: reverse, }) ) as Collection; if (relationship.isDependent() && parentResource.isNew()) @@ -154,7 +153,7 @@ export function SubView({ () => resourceOn( parentResource, - `change:${relationship.name}`, + `change:${relationship.name} saved`, (): void => { versionRef.current += 1; const localVersionRef = versionRef.current; diff --git a/specifyweb/frontend/js_src/lib/components/SearchDialog/index.tsx b/specifyweb/frontend/js_src/lib/components/SearchDialog/index.tsx index 96da3cb751f..bbb48929f9e 100644 --- a/specifyweb/frontend/js_src/lib/components/SearchDialog/index.tsx +++ b/specifyweb/frontend/js_src/lib/components/SearchDialog/index.tsx @@ -1,5 +1,6 @@ import React from 'react'; import type { LocalizedString } from 'typesafe-i18n'; +import type { State } from 'typesafe-reducer'; import { useBooleanState } from '../../hooks/useBooleanState'; import { useId } from '../../hooks/useId'; @@ -49,10 +50,7 @@ const viewNameExceptions: Partial> = { GeologicTimePeriod: 'ChronosStratSearch', }; -/** - * Display a resource search dialog - */ -export function SearchDialog(props: { +type SearchDialogProps = { readonly forceCollection: number | undefined; readonly extraFilters: RA> | undefined; readonly table: SpecifyTable; @@ -61,7 +59,14 @@ export function SearchDialog(props: { readonly searchView?: string; readonly onSelected: (resources: RA>) => void; readonly onlyUseQueryBuilder?: boolean; -}): JSX.Element | null { +}; + +/** + * Display a resource search dialog + */ +export function SearchDialog( + props: SearchDialogProps +): JSX.Element | null { const [alwaysUseQueryBuilder] = userPreferences.use( 'form', 'queryComboBox', @@ -84,6 +89,41 @@ export function SearchDialog(props: { ); } +/** + * Displays a SearchDialog whenever `showSearchDialog` is invoked + */ +export function useSearchDialog({ + onSelected: handleSelected, + onClose: handleClosed, + ...rest +}: Omit, 'onClose' | 'onSelected'> & + Partial, 'onClose' | 'onSelected'>>): { + readonly searchDialog: JSX.Element | null; + readonly showSearchDialog: () => void; +} { + const [state, setState] = React.useState | State<'Search'>>({ + type: 'Main', + }); + + return { + searchDialog: + state.type === 'Search' && typeof handleSelected === 'function' ? ( + { + handleClosed?.(); + setState({ type: 'Main' }); + }} + onSelected={handleSelected} + /> + ) : null, + showSearchDialog: () => + typeof handleSelected === 'function' + ? setState({ type: 'Search' }) + : undefined, + }; +} + const filterResults = ( results: RA>, extraFilters: RA> diff --git a/specifyweb/frontend/js_src/lib/components/WorkBench/useDisambiguationDialog.tsx b/specifyweb/frontend/js_src/lib/components/WorkBench/useDisambiguationDialog.tsx index 6272bb7090b..04478d71858 100644 --- a/specifyweb/frontend/js_src/lib/components/WorkBench/useDisambiguationDialog.tsx +++ b/specifyweb/frontend/js_src/lib/components/WorkBench/useDisambiguationDialog.tsx @@ -6,6 +6,7 @@ import { commonText } from '../../localization/common'; import { wbText } from '../../localization/workbench'; import { type RA } from '../../utils/types'; import { LoadingContext } from '../Core/Contexts'; +import { backendFilter } from '../DataModel/helpers'; import type { AnySchema } from '../DataModel/helperTypes'; import type { Collection } from '../DataModel/specifyTable'; import { strictGetTable } from '../DataModel/tables'; @@ -76,7 +77,7 @@ export function useDisambiguationDialog({ ); const table = strictGetTable(tableName); const resources = new table.LazyCollection({ - filters: { id__in: matches.ids.join(',') }, + filters: backendFilter('id').isIn(matches.ids), }) as Collection; loading( diff --git a/specifyweb/specify/api.py b/specifyweb/specify/api.py index 527bd2637d1..1fd30ce3c74 100644 --- a/specifyweb/specify/api.py +++ b/specifyweb/specify/api.py @@ -594,9 +594,7 @@ def handle_fk_fields(collection, agent, obj, data: Dict[str, Any]) -> Tuple[List elif isinstance(val, str): # The related object is given by a URI reference. assert not dependent, "didn't get inline data for dependent field %s in %s: %r" % (field_name, obj, val) - fk_model, fk_id = parse_uri(val) - assert fk_model == field.related_model.__name__.lower() - assert fk_id is not None + fk_model, fk_id = strict_uri_to_model(val, field.related_model.__name__) setattr(obj, field_name, get_object_or_404(fk_model, id=fk_id)) new_related_id = fk_id @@ -653,8 +651,27 @@ def handle_to_many(collection, agent, obj, data: Dict[str, Any]) -> None: continue rel_model = field.related_model - ids = [] # Ids not in this list will be deleted at the end. + ids = [] # Ids not in this list will be deleted (if dependent) or removed from obj (if independent) at the end. + + ids_to_fetch = [] + cached_objs = dict() + fk_model = None + if not is_dependent: + for rel_data in val: + if not isinstance(rel_data, str): continue + fk_model, fk_id = strict_uri_to_model(rel_data, rel_model.__name__) + ids_to_fetch.append(fk_id) + + if fk_model is not None: + cached_objs = {item.id: obj_to_data(item) for item in get_model(fk_model).objects.filter(id__in=ids_to_fetch)} + for rel_data in val: + if isinstance(rel_data, str): + assert not is_dependent, "expected object for dependent field %s in %s: %s" % (field_name, obj, rel_data) + fk_model, fk_id = strict_uri_to_model(rel_data, rel_model.__name__) + rel_data = cached_objs[fk_id] + + #FIXME: only update related objects when they change rel_data[field.field.name] = obj if 'id' in rel_data: # Update an existing related object. @@ -788,6 +805,12 @@ def parse_uri(uri: str) -> Tuple[str, str]: groups = match.groups() return groups[0], groups[2] +def strict_uri_to_model(uri: str, model: str) -> Tuple[str, int]: + uri_model, uri_id = parse_uri(uri) + assert model.lower() == uri_model.lower() + assert uri_id is not None + return uri_model, int(uri_id) + def obj_to_data(obj) -> Dict[str, Any]: "Wrapper for backwards compat w/ other modules that use this function." # TODO: Such functions should be audited for whether they should apply diff --git a/specifyweb/specify/tests/test_api.py b/specifyweb/specify/tests/test_api.py index b96b9094df3..473d31ab12e 100644 --- a/specifyweb/specify/tests/test_api.py +++ b/specifyweb/specify/tests/test_api.py @@ -499,13 +499,16 @@ def test_independent_set_inline(self): 'accessionnumber': "a", 'division': api.uri_for_model('division', self.division.id), 'collectionobjects': [ - api.obj_to_data(self.collectionobjects[0]) + api.obj_to_data(self.collectionobjects[0]), + api.uri_for_model('collectionobject', self.collectionobjects[1].id) ] } accession = api.create_obj(self.collection, self.agent, 'Accession', accession_data) self.collectionobjects[0].refresh_from_db() + self.collectionobjects[1].refresh_from_db() self.assertEqual(accession, self.collectionobjects[0].accession) + self.assertEqual(accession, self.collectionobjects[1].accession) def test_indepenent_removing_from_inline(self): accession = models.Accession.objects.create( @@ -516,13 +519,17 @@ def test_indepenent_removing_from_inline(self): accession.collectionobjects.set(self.collectionobjects) + self.assertEqual(accession, self.collectionobjects[0].accession) + collection_objects_to_set = [self.collectionobjects[0], self.collectionobjects[3]] accession_data = { 'accessionnumber': "a", 'division': api.uri_for_model('division', self.division.id), 'collectionobjects': [ - api.obj_to_data(collection_object) for collection_object in collection_objects_to_set + api.obj_to_data(collection_object) if index % 2 == 0 + else api.uri_for_model('collectionobject', collection_object.id) + for index, collection_object in enumerate(collection_objects_to_set) ] } accession = api.update_obj(self.collection, self.agent, 'Accession', accession.id, accession.version, accession_data) @@ -553,6 +560,8 @@ def test_updating_independent_resource(self): ] } + self.assertEqual(self.collectionobjects[2].integer1, None) + self.assertEqual(list(self.collectionobjects[2].determinations.all()), []) accession = api.create_obj(self.collection, self.agent, 'Accession', accession_data) self.collectionobjects[2].refresh_from_db() self.assertEqual(self.collectionobjects[2].integer1, 10) @@ -582,17 +591,54 @@ def test_reassigning_independent(self): self.collectionobjects[0].accession = acc1 self.collectionobjects[0].save() + self.collectionobjects[1].accession = acc1 + self.collectionobjects[1].save() accession_data = { 'accessionnumber': "b", 'division': api.uri_for_model('division', self.division.id), 'collectionobjects': [ - api.obj_to_data(self.collectionobjects[0]) + api.obj_to_data(self.collectionobjects[0]), + api.uri_for_model('collectionobject', self.collectionobjects[1].id) ] } acc2 = api.create_obj(self.collection, self.agent, 'Accession', accession_data) self.collectionobjects[0].refresh_from_db() + self.collectionobjects[1].refresh_from_db() self.assertEqual(self.collectionobjects[0].accession, acc2) + self.assertEqual(self.collectionobjects[1].accession, acc2) + + def inline_error_handling(self): + collection_object_data = { + 'id': self.collectionobjects[0].id, + 'catalognumber': self.collectionobjects[0].catalognumber, + 'collection': api.uri_for_model('Collection', self.collection.id), + 'determinations': f'/api/specify/determination/?collectionobject={self.collectionobjects[0].id}' + } + + with self.assertRaises(AssertionError): + api.update_obj(self.collection, self.agent, + 'Collectionobject', self.collectionobjects[0].id, + self.collectionobjects[0].version, collection_object_data) + + new_determination = models.Determination.objects.create( + collectionobject=self.collectionobjects[1] + ) + + other_collection_object_data = { + 'id': self.collectionobjects[0].id, + 'catalognumber': self.collectionobjects[0].catalognumber, + 'collection': api.uri_for_model('Collection', self.collection.id), + 'determinations': [ + api.uri_for_model('determination', new_determination.id) + ] + } + + with self.assertRaises(AssertionError): + api.update_obj(self.collection, self.agent, + 'Collectionobject', self.collectionobjects[0].id, + self.collectionobjects[0].version, other_collection_object_data) + # version control on inlined resources should be tested From 08544c3d38ee24107430a5ef38f7e68300a406ef Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 22 Aug 2024 12:11:18 -0500 Subject: [PATCH 16/66] Allow creating new resources from independent to-one side --- .../lib/components/DataModel/resourceApi.ts | 77 ++++++++++++++++--- specifyweb/specify/api.py | 13 ++-- specifyweb/specify/tests/test_api.py | 49 ++++++++++-- 3 files changed, 115 insertions(+), 24 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts index 799639f91dc..c3d23b82b8a 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts @@ -300,10 +300,61 @@ export const ResourceBase = Backbone.Model.extend({ this.dependentResources[field.name.toLowerCase()] = toMany; toMany.on('all', eventHandlerForToMany(toMany, field), this); }, - storeIndependentToMany(field: Relationship, toMany: Collection) { - assert(!field.isDependent() && relationshipIsToMany(field)); - const oldToMany = this.independentResources[field.name.toLowerCase()]; - if (oldToMany !== undefined) oldToMany.off('all', null, this); + storeIndependent( + field: Relationship, + related: Collection | SpecifyResource | null + ) { + assert(!field.isDependent()); + + if (field.type === 'one-to-many') + this._storeIndependentToMany(field, related); + else this._storeIndependentToOne(field, related); + }, + _storeIndependentToOne( + field: Relationship, + related: SpecifyResource | null + ) { + const oldRelated = this.independentResources[field.name.toLowerCase()]; + if (!related) { + if (oldRelated) { + oldRelated.off('all', null, this); + this.trigger('saverequired'); + } + this.independentResources[field.name.toLowerCase()] = null; + return; + } + related.toApiJSON = (options) => + related.isNew() || related.needsSaved + ? related.toJSON(options) + : related.url(); + + if (oldRelated && oldRelated.cid === related.cid) return; + + oldRelated && oldRelated.off('all', null, this); + + related.on('all', eventHandlerForToOne(related, field), this); + + switch (field.type) { + case 'one-to-one': + case 'many-to-one': { + this.independentResources[field.name.toLowerCase()] = related; + break; + } + case 'zero-to-one': { + this.independentResources[field.name.toLowerCase()] = related; + related.set(field.otherSideName, this.url()); + break; + } + default: { + throw new Error( + `setDependentToOne: unhandled field type: ${field.type}` + ); + } + } + }, + _storeIndependentToMany(field: Relationship, toMany: Collection) { + const oldIndependent = this.independentResources[field.name.toLowerCase()]; + if (oldIndependent !== undefined) oldIndependent.off('all', null, this); this.independentResources[field.name.toLowerCase()] = toMany; toMany.on('all', eventHandlerForToMany(toMany, field), this); @@ -428,7 +479,7 @@ export const ResourceBase = Backbone.Model.extend({ }, _handleInlineDataOrResource(value, fieldName) { // BUG: check type of value - const field: Relationship = this.specifyTable.getField(fieldName); + const field: Relationship = this.specifyTable.strictGetField(fieldName); const relatedTable = field.relatedTable; // BUG: don't do anything for virtual fields @@ -448,7 +499,7 @@ export const ResourceBase = Backbone.Model.extend({ collectionOptions, value ); - this.storeIndependentToMany(field, collection); + this.storeIndependent(field, collection); } // Because the foreign key is on the other side @@ -469,13 +520,14 @@ export const ResourceBase = Backbone.Model.extend({ * BUG: tighten up this check. * The FK is null, or not a URI or inlined resource at any rate */ - field.isDependent() && this.storeDependent(field, null); + if (field.isDependent()) this.storeDependent(field, null); + else this.storeIndependent(field, null); return value; } const toOne = maybeMakeResource(value, relatedTable); - - field.isDependent() && this.storeDependent(field, toOne); + if (field.isDependent()) this.storeDependent(field, toOne); + else this.storeIndependent(field, toOne); this.trigger(`change:${fieldName}`, this); this.trigger('change', this); return toOne.url(); @@ -646,6 +698,8 @@ export const ResourceBase = Backbone.Model.extend({ if (field.isDependent()) { console.warn('expected dependent resource to be in cache'); this.storeDependent(field, toOne); + } else { + this.storeIndependent(field, toOne); } } // If we want a field within the related resource then recur @@ -762,7 +816,7 @@ export const ResourceBase = Backbone.Model.extend({ : existingToMany; return collection.fetch({ limit: 0 }).then((collection) => { - this.storeIndependentToMany(field, collection); + this.storeIndependent(field, collection); return collection; }); }, @@ -835,8 +889,7 @@ export const ResourceBase = Backbone.Model.extend({ Object.entries(self.independentResources).forEach( ([fieldName, related]) => { - const field = self.specifyTable.getField(fieldName); - if (field.type === 'one-to-many' && related) { + if (related) { json[fieldName] = related.toApiJSON(); } } diff --git a/specifyweb/specify/api.py b/specifyweb/specify/api.py index 1fd30ce3c74..632a2fea366 100644 --- a/specifyweb/specify/api.py +++ b/specifyweb/specify/api.py @@ -600,20 +600,19 @@ def handle_fk_fields(collection, agent, obj, data: Dict[str, Any]) -> Tuple[List elif hasattr(val, 'items'): # i.e. it's a dict of some sort # The related object is represented by a nested dict of data. - assert dependent, "got inline data for non dependent field %s in %s: %r" % (field_name, obj, val) rel_model = field.related_model if 'id' in val: # The related object is an existing resource with an id. - # This should never happen. + # This should never happen for dependent resources. rel_obj = update_obj(collection, agent, rel_model, val['id'], val['version'], val, - parent_obj=obj) + parent_obj=obj if dependent else None) else: # The related object is to be created. rel_obj = create_obj(collection, agent, rel_model, val, - parent_obj=obj) + parent_obj=obj if dependent else None) setattr(obj, field_name, rel_obj) if dependent and old_related and old_related.id != rel_obj.id: @@ -678,11 +677,11 @@ def handle_to_many(collection, agent, obj, data: Dict[str, Any]) -> None: rel_obj = update_obj(collection, agent, rel_model, rel_data['id'], rel_data['version'], rel_data, - parent_obj=obj if is_dependent_field(obj, field_name) else None) + parent_obj=obj if is_dependent else None) else: # Create a new related object. - rel_obj = create_obj(collection, agent, rel_model, rel_data, parent_obj=obj) + rel_obj = create_obj(collection, agent, rel_model, rel_data, parent_obj=obj if is_dependent else None) if not is_dependent and not (isinstance(obj, models.Recordset) and field_name == 'recordsetitems'): getattr(obj, field_name).add(rel_obj) @@ -807,7 +806,7 @@ def parse_uri(uri: str) -> Tuple[str, str]: def strict_uri_to_model(uri: str, model: str) -> Tuple[str, int]: uri_model, uri_id = parse_uri(uri) - assert model.lower() == uri_model.lower() + assert model.lower() == uri_model.lower(), f"{model} does not match model in uri: {uri_model}" assert uri_id is not None return uri_model, int(uri_id) diff --git a/specifyweb/specify/tests/test_api.py b/specifyweb/specify/tests/test_api.py index 1c184203fba..be6fa4a632c 100644 --- a/specifyweb/specify/tests/test_api.py +++ b/specifyweb/specify/tests/test_api.py @@ -494,7 +494,7 @@ def test_update_object_with_more_inlines(self): self.assertEqual(obj.collectionobjectattribute.text1, text1_data) - def test_independent_set_inline(self): + def test_independent_to_many_set_inline(self): accession_data = { 'accessionnumber': "a", 'division': api.uri_for_model('division', self.division.id), @@ -510,7 +510,19 @@ def test_independent_set_inline(self): self.assertEqual(accession, self.collectionobjects[0].accession) self.assertEqual(accession, self.collectionobjects[1].accession) - def test_indepenent_removing_from_inline(self): + def test_independent_to_one_set_inline(self): + collection_object_data = { + 'collection': api.uri_for_model('collection', self.collection.id), + 'accession': { + 'accessionnumber': "a", + 'division': api.uri_for_model('division', self.division.id), + } + } + + created_co = api.create_obj(self.collection, self.agent, 'Collectionobject', collection_object_data) + self.assertIsNotNone(created_co.accession) + + def test_indepenent_to_many_removing_from_inline(self): accession = models.Accession.objects.create( accessionnumber="a", version="0", @@ -539,7 +551,7 @@ def test_indepenent_removing_from_inline(self): # ensure the other CollectionObjects have not been deleted self.assertEqual(len(models.Collectionobject.objects.all()), len(self.collectionobjects)) - def test_updating_independent_resource(self): + def test_updating_independent_to_many_resource(self): co_to_modify = api.obj_to_data(self.collectionobjects[2]) co_to_modify.update({ 'integer1': 10, @@ -567,7 +579,34 @@ def test_updating_independent_resource(self): self.assertEqual(self.collectionobjects[2].integer1, 10) self.assertEqual(len(self.collectionobjects[2].determinations.all()), 1) - def test_independent_creating_from_remoteside(self): + def test_updating_independent_to_one_resource(self): + accession_data = { + 'accessionnumber': "a", + 'division': api.uri_for_model('division', self.division.id) + } + accession = api.create_obj(self.collection, self.agent, 'Accession', accession_data) + + accession_text = 'someText' + accession_data.update({ + 'id': accession.id, + 'accessionnumber': "a1", + 'text1': accession_text, + 'version': accession.version + }) + + collection_object_data = { + 'collection': api.uri_for_model('collection', self.collection.id), + 'accession': accession_data + } + + self.assertEqual(accession.text1, None) + self.assertEqual(accession.accessionnumber, 'a') + created_co = api.create_obj(self.collection, self.agent, 'Collectionobject', collection_object_data) + accession.refresh_from_db() + self.assertEqual(accession.text1, accession_text) + self.assertEqual(accession.accessionnumber, 'a1') + + def test_independent_to_many_creating_from_remoteside(self): new_catalognumber = f'num-{len(self.collectionobjects)}' accession_data = { 'accessionnumber': "a", @@ -583,7 +622,7 @@ def test_independent_creating_from_remoteside(self): accession = api.create_obj(self.collection, self.agent, 'Accession', accession_data) self.assertTrue(models.Collectionobject.objects.filter(catalognumber=new_catalognumber).exists()) - def test_reassigning_independent(self): + def test_reassigning_independent_to_many(self): acc1 = models.Accession.objects.create( accessionnumber="a", division = self.division From 70c33453f65692812d7b7f969eddf9cd0ad5b8ba Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 22 Aug 2024 12:31:15 -0500 Subject: [PATCH 17/66] Remove toApiJSON method declaration on independent to-one resources --- .../js_src/lib/components/DataModel/resourceApi.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts index c3d23b82b8a..bbce67c7e0a 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts @@ -323,10 +323,6 @@ export const ResourceBase = Backbone.Model.extend({ this.independentResources[field.name.toLowerCase()] = null; return; } - related.toApiJSON = (options) => - related.isNew() || related.needsSaved - ? related.toJSON(options) - : related.url(); if (oldRelated && oldRelated.cid === related.cid) return; @@ -890,7 +886,11 @@ export const ResourceBase = Backbone.Model.extend({ Object.entries(self.independentResources).forEach( ([fieldName, related]) => { if (related) { - json[fieldName] = related.toApiJSON(); + json[fieldName] = isRelationshipCollection(related) + ? related.toApiJSON() + : related.isNew() || related.needsSaved + ? related.toJSON(options) + : related.url(); } } ); From 02e262ec14ebd4b33c707a501715f9a2570ba787 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 22 Aug 2024 19:53:52 -0500 Subject: [PATCH 18/66] Remove changedResources on IndependentCollection --- .../lib/components/DataModel/collectionApi.ts | 36 +++++-------------- .../js_src/lib/components/Forms/SubView.tsx | 2 +- 2 files changed, 9 insertions(+), 29 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts index 34371d71228..82939ab4596 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts @@ -1,13 +1,12 @@ // @ts-nocheck import _ from 'underscore'; -import { removeKey } from '../../utils/utils'; import { assert } from '../Errors/assert'; import { formatUrl } from '../Router/queryString'; import { Backbone } from './backbone'; -import { AnySchema } from './helperTypes'; -import { SpecifyResource } from './legacyTypes'; +import type { AnySchema } from './helperTypes'; +import type { SpecifyResource } from './legacyTypes'; // REFACTOR: remove @ts-nocheck @@ -103,25 +102,11 @@ export const IndependentCollection = Base.extend({ this.domainfilter = Boolean(options.domainfilter) && this.model?.specifyTable.getScopingRelationship() !== undefined; - - this.changedResources = Object.fromEntries( - records.map((record) => [record.cid, false]) - ); }, initialize(_tables, options) { this.on( - 'change', + 'change add remove', function (resource: SpecifyResource) { - if (!resource.isBeingInitialized()) - this.changedResources[resource.cid] = true; - this.trigger('saverequired'); - }, - this - ); - this.on( - 'add remove', - function (resource: SpecifyResource) { - this.changedResources[resource.cid] = false; this.trigger('saverequired'); }, this @@ -171,16 +156,11 @@ export const IndependentCollection = Base.extend({ }, toApiJSON(options) { const self = this; - const resources = - Object.keys(self.changedResources).length === 0 - ? formatUrl(this.url(), this.filters) - : this.map(function (resource) { - return self.changedResources[resource.cid] === true - ? resource.toJSON(options) - : resource.url(); - }); - this.changedResources = []; - return resources; + + return this.map(function (resource: SpecifyResource) { + const formatAsObject = resource.needsSaved || resource.isNew(); + return formatAsObject ? resource.toJSON(options) : resource.url(); + }); }, }); diff --git a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx index 51f181452e2..ec62321d046 100644 --- a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx +++ b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx @@ -4,7 +4,7 @@ import { usePromise } from '../../hooks/useAsyncState'; import { useBooleanState } from '../../hooks/useBooleanState'; import { useTriggerState } from '../../hooks/useTriggerState'; import { commonText } from '../../localization/common'; -import { overwriteReadOnly, RA } from '../../utils/types'; +import { overwriteReadOnly } from '../../utils/types'; import { sortFunction } from '../../utils/utils'; import { Button } from '../Atoms/Button'; import { attachmentSettingsPromise } from '../Attachments/attachments'; From a2d3574d3d9fd0bee7cc417c156c1b85f066bf82 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 22 Aug 2024 20:17:08 -0500 Subject: [PATCH 19/66] Display loading indicator while fetching collection --- .../frontend/js_src/lib/components/Forms/SubView.tsx | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx index ec62321d046..43f526d3a52 100644 --- a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx +++ b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx @@ -7,6 +7,7 @@ import { commonText } from '../../localization/common'; import { overwriteReadOnly } from '../../utils/types'; import { sortFunction } from '../../utils/utils'; import { Button } from '../Atoms/Button'; +import { DataEntry } from '../Atoms/DataEntry'; import { attachmentSettingsPromise } from '../Attachments/attachments'; import { attachmentRelatedTables } from '../Attachments/utils'; import { ReadOnlyContext } from '../Core/Contexts'; @@ -265,7 +266,16 @@ export function SubView({ } /> - ) : undefined} + ) : isAttachmentMisconfigured ? undefined : ( + + + + {relationship.label} + + + {commonText.loading()} + + )} ); } From cf89d5be54694930fc07f778b7380d55f0b4d6fe Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 22 Aug 2024 21:09:17 -0500 Subject: [PATCH 20/66] Use ResourceView dialog when adding independent resource --- .../FormSliders/IntegratedRecordSelector.tsx | 41 ++++++++++++++++++- .../lib/components/QueryComboBox/index.tsx | 2 +- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx index 120c896eba6..b2bf7fc3b5b 100644 --- a/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx @@ -1,4 +1,5 @@ import React from 'react'; +import { State } from 'typesafe-reducer'; import { useSearchParameter } from '../../hooks/navigation'; import { useBooleanState } from '../../hooks/useBooleanState'; @@ -71,6 +72,14 @@ export function IntegratedRecordSelector({ const [isCollapsed, _handleCollapsed, handleExpand, handleToggle] = useBooleanState(defaultCollapsed); + const [state, setState] = React.useState< + | State< + 'AddResourceState', + { readonly resource: SpecifyResource } + > + | State<'MainState'> + >({ type: 'MainState' }); + const blockers = useAllSaveBlockers(collection.related, relationship); const hasBlockers = blockers.length > 0; React.useEffect(() => { @@ -203,10 +212,22 @@ export function IntegratedRecordSelector({ (isToOne && collection.models.length > 0) } onClick={(): void => { - focusFirstField(); const resource = new collection.table.specifyTable.Resource(); - handleAdd([resource]); + + if (isDependent) { + focusFirstField(); + handleAdd([resource]); + return; + } + + if (state.type === 'AddResourceState') + setState({ type: 'MainState' }); + else + setState({ + type: 'AddResourceState', + resource, + }); }} /> ) : undefined} @@ -279,6 +300,22 @@ export function IntegratedRecordSelector({ /> ) : null} {dialogs} + {state.type === 'AddResourceState' && + typeof handleAdd === 'function' ? ( + setState({ type: 'MainState' })} + onDeleted={undefined} + onSaved={(): void => { + handleAdd([state.resource]); + setState({ type: 'MainState' }); + }} + /> + ) : null} )} diff --git a/specifyweb/frontend/js_src/lib/components/QueryComboBox/index.tsx b/specifyweb/frontend/js_src/lib/components/QueryComboBox/index.tsx index 98605a30a2c..da1e7ed286c 100644 --- a/specifyweb/frontend/js_src/lib/components/QueryComboBox/index.tsx +++ b/specifyweb/frontend/js_src/lib/components/QueryComboBox/index.tsx @@ -508,7 +508,7 @@ export function QueryComboBox({ } /> )} - {hasViewButton && hasTablePermission(relatedTable.name, 'create') + {hasViewButton && hasTablePermission(relatedTable.name, 'read') ? viewButton : undefined} From ad5a8eff58f14fdab29f32ac1cf95efe4a7e9e17 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 22 Aug 2024 21:18:22 -0500 Subject: [PATCH 21/66] Only show loading indicators for non-button subviews --- specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx index 43f526d3a52..55942d9c4c1 100644 --- a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx +++ b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx @@ -266,7 +266,7 @@ export function SubView({ } /> - ) : isAttachmentMisconfigured ? undefined : ( + ) : isButton ? undefined : ( From d555321a0627ce943f20c572e1b69c622fae4b90 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 22 Aug 2024 21:22:51 -0500 Subject: [PATCH 22/66] Only show ResourceView dialog when specified viewname differs from table view --- .../lib/components/FormSliders/IntegratedRecordSelector.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx index b2bf7fc3b5b..2d81ddd1c16 100644 --- a/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx @@ -215,7 +215,10 @@ export function IntegratedRecordSelector({ const resource = new collection.table.specifyTable.Resource(); - if (isDependent) { + if ( + isDependent || + viewName === relationship.relatedTable.view + ) { focusFirstField(); handleAdd([resource]); return; From 2664cf1384d6d8ab5151f26a868bb4a86bb45e80 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 22 Aug 2024 21:41:51 -0500 Subject: [PATCH 23/66] Use aria-pressed on add button when ResourceView is open --- .../lib/components/FormSliders/IntegratedRecordSelector.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx index 2d81ddd1c16..66d6037ff93 100644 --- a/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx @@ -207,6 +207,7 @@ export function IntegratedRecordSelector({ 'create' ) && typeof handleAdd === 'function' ? ( 0) From 14e2ab3b1bf017bf3dcb2c15bd39ad285ca56380 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 22 Aug 2024 22:21:42 -0500 Subject: [PATCH 24/66] Refactor getDependentToMany --- .../lib/components/DataModel/resourceApi.ts | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts index bbce67c7e0a..3ebfe5aa75f 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts @@ -755,40 +755,40 @@ export const ResourceBase = Backbone.Model.extend({ ): Promise> { assert(field.isDependent()); + const self = this; const fieldName = field.name.toLowerCase(); const relatedTable = field.relatedTable; - let toMany = this.dependentResources[fieldName]; - - if (toMany) return toMany; + const existingToMany: Collection | undefined = + this.dependentResources[fieldName]; const collectionOptions = { field: field.getReverse(), related: this, }; - if (this.isNew()) { - toMany = new relatedTable.DependentCollection(collectionOptions, []); - this.storeDependent(field, toMany); - return toMany; - } else { + if (!this.isNew() && existingToMany === undefined) console.warn('expected dependent resource to be in cache'); - const temporaryCollection = new relatedTable.ToOneCollection( - collectionOptions - ); - return temporaryCollection - .fetch({ limit: 0 }) - .then( - () => - new relatedTable.DependentCollection( - collectionOptions, - temporaryCollection.tables - ) - ) - .then((toMany) => { - _this.storeDependent(field, toMany); - }); - } + + const collection = + existingToMany !== undefined + ? existingToMany + : this.isNew() + ? new relatedTable.DependentCollection(collectionOptions, []) + : await new relatedTable.ToOneCollection(collectionOptions) + .fetch({ limit: 0 }) + .then( + (collection) => + new relatedTable.DependentCollection( + collectionOptions, + collection.models + ) + ); + + return collection.fetch({ limit: 0 }).then((collection) => { + self.storeDependent(field, collection); + return collection; + }); }, async getIndependentToMany( field: Relationship From d47585db14762d48cff850ea103319da3e1617e2 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Tue, 27 Aug 2024 13:40:07 -0500 Subject: [PATCH 25/66] Fix bug when fetching independent collection when related is initializing --- .../lib/components/DataModel/collectionApi.ts | 8 ++++---- .../lib/components/DataModel/resourceApi.ts | 18 +++++++++--------- specifyweb/specify/api.py | 19 ++++++++++--------- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts index 82939ab4596..556161ca286 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts @@ -128,7 +128,7 @@ export const IndependentCollection = Base.extend({ return records; }, async fetch(options) { - if (this.related.isNew()) { + if (this.related.isNew() || this.related.isBeingInitialized()) { return this; } @@ -144,11 +144,11 @@ export const IndependentCollection = Base.extend({ domainfilter: this.domainfilter, limit: options.limit, }; - + const self = this; this._fetch = Backbone.Collection.prototype.fetch.call(this, options); return this._fetch.then(() => { - this._fetch = null; - return this; + self._fetch = null; + return self; }); }, isComplete() { diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts index 3ebfe5aa75f..2219ab516db 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts @@ -7,7 +7,6 @@ import { Http } from '../../utils/ajax/definitions'; import { removeKey } from '../../utils/utils'; import { assert } from '../Errors/assert'; import { softFail } from '../Errors/Crash'; -import { relationshipIsToMany } from '../WbPlanView/mappingHelpers'; import { Backbone } from './backbone'; import { attachBusinessRules } from './businessRules'; import { isRelationshipCollection } from './collectionApi'; @@ -112,7 +111,7 @@ export const ResourceBase = Backbone.Model.extend({ _save: null, // Stores reference to the ajax deferred while the resource is being saved /** - * Returns true if the resource is being fetched and saved from Backbone + * Returns true if the resource is being fetched or saved from Backbone * More specifically, returns true while this resource holds a reference * to Backbone's save() and fetch() in _save and _fetch */ @@ -811,9 +810,9 @@ export const ResourceBase = Backbone.Model.extend({ ? new relatedTable.IndependentCollection(collectionOptions) : existingToMany; - return collection.fetch({ limit: 0 }).then((collection) => { - this.storeIndependent(field, collection); - return collection; + return collection.fetch({ limit: 0 }).then((fetchedCollection) => { + this.storeIndependent(field, fetchedCollection); + return fetchedCollection; }); }, async save({ @@ -872,14 +871,15 @@ export const ResourceBase = Backbone.Model.extend({ }, toJSON() { const self = this; - const json = Backbone.Model.prototype.toJSON.apply(self, arguments); + const options = arguments; + const json = Backbone.Model.prototype.toJSON.apply(self, options); _.each(self.dependentResources, (related, fieldName) => { const field = self.specifyTable.getField(fieldName); if (field.type === 'zero-to-one') { - json[fieldName] = related ? [related.toJSON()] : []; + json[fieldName] = related ? [related.toJSON(options)] : []; } else { - json[fieldName] = related ? related.toJSON() : null; + json[fieldName] = related ? related.toJSON(options) : null; } }); @@ -887,7 +887,7 @@ export const ResourceBase = Backbone.Model.extend({ ([fieldName, related]) => { if (related) { json[fieldName] = isRelationshipCollection(related) - ? related.toApiJSON() + ? related.toApiJSON(options) : related.isNew() || related.needsSaved ? related.toJSON(options) : related.url(); diff --git a/specifyweb/specify/api.py b/specifyweb/specify/api.py index 632a2fea366..2c90c15fc03 100644 --- a/specifyweb/specify/api.py +++ b/specifyweb/specify/api.py @@ -672,16 +672,8 @@ def handle_to_many(collection, agent, obj, data: Dict[str, Any]) -> None: #FIXME: only update related objects when they change rel_data[field.field.name] = obj - if 'id' in rel_data: - # Update an existing related object. - rel_obj = update_obj(collection, agent, - rel_model, rel_data['id'], - rel_data['version'], rel_data, - parent_obj=obj if is_dependent else None) - else: - # Create a new related object. - rel_obj = create_obj(collection, agent, rel_model, rel_data, parent_obj=obj if is_dependent else None) + rel_obj = update_or_create_resource(collection, agent, rel_model, rel_data, parent_obj=obj if is_dependent else None) if not is_dependent and not (isinstance(obj, models.Recordset) and field_name == 'recordsetitems'): getattr(obj, field_name).add(rel_obj) @@ -699,6 +691,15 @@ def handle_to_many(collection, agent, obj, data: Dict[str, Any]) -> None: else: getattr(obj, field_name).remove(*list(to_remove)) +def update_or_create_resource(collection, agent, model, data, parent_obj): + if 'id' in data: + return update_obj(collection, agent, + model, data['id'], + data['version'], data, + parent_obj=parent_obj) + else: + return create_obj(collection, agent, model, data, parent_obj=parent_obj) + @transaction.atomic def delete_resource(collection, agent, name, id, version) -> None: """Delete the resource with 'id' and model named 'name' with optimistic From 5c9f22d40c4ae9172ee48639c07a8aaa7d50eac8 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Tue, 27 Aug 2024 15:31:16 -0500 Subject: [PATCH 26/66] Only update related version when changed --- specifyweb/specify/api.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/specifyweb/specify/api.py b/specifyweb/specify/api.py index 2c90c15fc03..59e9a37f00d 100644 --- a/specifyweb/specify/api.py +++ b/specifyweb/specify/api.py @@ -669,8 +669,10 @@ def handle_to_many(collection, agent, obj, data: Dict[str, Any]) -> None: assert not is_dependent, "expected object for dependent field %s in %s: %s" % (field_name, obj, rel_data) fk_model, fk_id = strict_uri_to_model(rel_data, rel_model.__name__) rel_data = cached_objs[fk_id] + if rel_data[field.field.name] == uri_for_model(obj.__class__, obj.id): + ids.append(rel_data["id"]) + continue - #FIXME: only update related objects when they change rel_data[field.field.name] = obj rel_obj = update_or_create_resource(collection, agent, rel_model, rel_data, parent_obj=obj if is_dependent else None) From 3a6b9b5a7419cdf40e442e30459499d33c89a566 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 28 Aug 2024 22:11:13 -0500 Subject: [PATCH 27/66] Fix test not being automatically ran --- specifyweb/specify/tests/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specifyweb/specify/tests/test_api.py b/specifyweb/specify/tests/test_api.py index be6fa4a632c..2646bbd1d66 100644 --- a/specifyweb/specify/tests/test_api.py +++ b/specifyweb/specify/tests/test_api.py @@ -647,7 +647,7 @@ def test_reassigning_independent_to_many(self): self.assertEqual(self.collectionobjects[0].accession, acc2) self.assertEqual(self.collectionobjects[1].accession, acc2) - def inline_error_handling(self): + def test_inline_error_handling(self): collection_object_data = { 'id': self.collectionobjects[0].id, 'catalognumber': self.collectionobjects[0].catalognumber, From 2e46526f4a8dbac57d9f8ca2682bfe34174d6be4 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Mon, 2 Sep 2024 09:36:40 -0500 Subject: [PATCH 28/66] Show helpful error when rendering to-many as querycbx --- .../lib/components/FormEditor/viewSpec.ts | 30 +++++++++++++++++-- .../js_src/lib/components/FormParse/fields.ts | 10 +++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/FormEditor/viewSpec.ts b/specifyweb/frontend/js_src/lib/components/FormEditor/viewSpec.ts index 55d488e32af..772e53d6d3d 100644 --- a/specifyweb/frontend/js_src/lib/components/FormEditor/viewSpec.ts +++ b/specifyweb/frontend/js_src/lib/components/FormEditor/viewSpec.ts @@ -17,6 +17,7 @@ import { syncers } from '../Syncer/syncers'; import type { SimpleXmlNode } from '../Syncer/xmlToJson'; import { createSimpleXmlNode } from '../Syncer/xmlToJson'; import { createXmlSpec } from '../Syncer/xmlUtils'; +import { relationshipIsToMany } from '../WbPlanView/mappingHelpers'; /* eslint-disable @typescript-eslint/no-magic-numbers */ // eslint-disable-next-line @typescript-eslint/explicit-function-return-type @@ -799,8 +800,32 @@ const textAreaSpec = ( ), }); -const queryComboBoxSpec = f.store(() => +const queryComboBoxSpec = ( + _spec: SpecToJson>, + { + table, + }: { + readonly table: SpecifyTable | undefined; + } +) => createXmlSpec({ + field: pipe( + rawFieldSpec(table).field, + syncer( + ({ parsed, ...rest }) => { + if ( + parsed?.some( + (field) => field.isRelationship && relationshipIsToMany(field) + ) + ) + console.error( + 'Unable to render a to-many relationship as a querycbx. Use a Subview instead' + ); + return { parsed, ...rest }; + }, + (value) => value + ) + ), // Customize view name dialogViewName: syncers.xmlAttribute('initialize displayDlg', 'skip'), searchDialogViewName: syncers.xmlAttribute('initialize searchDlg', 'skip'), @@ -836,8 +861,7 @@ const queryComboBoxSpec = f.store(() => syncers.maybe(syncers.toBoolean), syncers.default(true) ), - }) -); + }); const checkBoxSpec = f.store(() => createXmlSpec({ diff --git a/specifyweb/frontend/js_src/lib/components/FormParse/fields.ts b/specifyweb/frontend/js_src/lib/components/FormParse/fields.ts index 1f06902f7f8..cc723f79eda 100644 --- a/specifyweb/frontend/js_src/lib/components/FormParse/fields.ts +++ b/specifyweb/frontend/js_src/lib/components/FormParse/fields.ts @@ -24,6 +24,7 @@ import { getBooleanAttribute, getParsedAttribute, } from '../Syncer/xmlUtils'; +import { relationshipIsToMany } from '../WbPlanView/mappingHelpers'; import type { PluginDefinition } from './plugins'; import { parseUiPlugin } from './plugins'; @@ -222,6 +223,15 @@ const processFieldType: { if (fields === undefined) { console.error('Trying to render a query combobox without a field name'); return { type: 'Blank' }; + } else if ( + fields.some( + (field) => field.isRelationship && relationshipIsToMany(field) + ) + ) { + console.error( + 'Unable to render a to-many relationship as a querycbx. Use a Subview instead' + ); + return { type: 'Blank' }; } else if (fields.at(-1)?.isRelationship === true) { return { type: 'QueryComboBox', From 5921a397bb85a0b45c918f6e52cdae43eac4c0b4 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Mon, 2 Sep 2024 10:21:41 -0500 Subject: [PATCH 29/66] Hide relationship when needed to avoid cyclical rendering --- .../js_src/lib/components/Forms/SubView.tsx | 170 ++++++++++-------- 1 file changed, 100 insertions(+), 70 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx index 55942d9c4c1..d8356bf81d1 100644 --- a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx +++ b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx @@ -4,6 +4,7 @@ import { usePromise } from '../../hooks/useAsyncState'; import { useBooleanState } from '../../hooks/useBooleanState'; import { useTriggerState } from '../../hooks/useTriggerState'; import { commonText } from '../../localization/common'; +import type { RA } from '../../utils/types'; import { overwriteReadOnly } from '../../utils/types'; import { sortFunction } from '../../utils/utils'; import { Button } from '../Atoms/Button'; @@ -23,18 +24,26 @@ import { IntegratedRecordSelector } from '../FormSliders/IntegratedRecordSelecto import { TableIcon } from '../Molecules/TableIcon'; import { relationshipIsToMany } from '../WbPlanView/mappingHelpers'; -export const SubViewContext = React.createContext< +type SubViewContextType = | { readonly relationship: Relationship | undefined; readonly formType: FormType; readonly sortField: SubViewSortField | undefined; + /** + * Don't render a relationship if it is already being rendered in a + * parent subview. + * Avoids infinite cycles in rendering forms + */ + readonly parentContext: RA | undefined; readonly handleChangeFormType: (formType: FormType) => void; readonly handleChangeSortField: ( sortField: SubViewSortField | undefined ) => void; } - | undefined ->(undefined); + | undefined; + +export const SubViewContext = + React.createContext(undefined); SubViewContext.displayName = 'SubViewContext'; export function SubView({ @@ -175,17 +184,31 @@ export function SubView({ ), [parentResource, relationship, fetchCollection] ); + const subviewContext = React.useContext(SubViewContext); const [formType, setFormType] = useTriggerState(initialFormType); - const contextValue = React.useMemo( + const parentContext = React.useMemo( + () => subviewContext?.parentContext ?? [], + [subviewContext?.parentContext] + ); + + const contextValue = React.useMemo( () => ({ relationship, formType, sortField, + parentContext: [...parentContext, relationship], handleChangeFormType: setFormType, handleChangeSortField: setSortField, }), - [relationship, formType, sortField, setFormType, setSortField] + [ + relationship, + formType, + sortField, + parentContext, + setFormType, + setSortField, + ] ); const isReadOnly = React.useContext(ReadOnlyContext); @@ -203,11 +226,13 @@ export function SubView({ return ( - {isButton && ( - + {isButton && ( + 0 @@ -215,66 +240,71 @@ export function SubView({ : '' } ${isOpen ? '!bg-brand-300 dark:!bg-brand-500' : ''}`} - title={relationship.label} - onClick={handleToggle} - > - { - /* - * Attachment table icons have lots of vertical white space, making - * them look overly small on the forms. - * See https://github.com/specify/specify7/issues/1259 - * Thus, have to introduce some inconsistency here - */ - parentFormType === 'form' && ( - - ) - } - - {collection?.models.length ?? commonText.loading()} - - - )} - {typeof collection === 'object' && isOpen ? ( - - - void parentResource.set( - relationship.name, - resource as never - ) - } - onClose={handleClose} - onDelete={ - relationshipIsToMany(relationship) && - relationship.type !== 'zero-to-one' - ? undefined - : (): void => - void parentResource.set(relationship.name, null as never) - } - /> - - ) : isButton ? undefined : ( - - - - {relationship.label} - - - {commonText.loading()} - + title={relationship.label} + onClick={handleToggle} + > + { + /* + * Attachment table icons have lots of vertical white space, making + * them look overly small on the forms. + * See https://github.com/specify/specify7/issues/1259 + * Thus, have to introduce some inconsistency here + */ + parentFormType === 'form' && ( + + ) + } + + {collection?.models.length ?? commonText.loading()} + + + )} + {typeof collection === 'object' && isOpen ? ( + + + void parentResource.set( + relationship.name, + resource as never + ) + } + onClose={handleClose} + onDelete={ + relationshipIsToMany(relationship) && + relationship.type !== 'zero-to-one' + ? undefined + : (): void => + void parentResource.set( + relationship.name, + null as never + ) + } + /> + + ) : isButton ? undefined : ( + + + + {relationship.label} + + + {commonText.loading()} + + )} + )} ); From 137a2779904d407a4fe094c5710f631099f9c405 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Mon, 2 Sep 2024 10:49:28 -0500 Subject: [PATCH 30/66] Resolve type errors --- .../js_src/lib/components/DataModel/resourceApi.ts | 10 +++------- .../js_src/lib/components/DataModel/saveBlockers.tsx | 4 ++-- .../js_src/lib/components/DataModel/specifyTable.ts | 2 +- .../js_src/lib/components/FormCells/FormTable.tsx | 2 +- 4 files changed, 7 insertions(+), 11 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts index 2219ab516db..45a39644a57 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts @@ -87,16 +87,12 @@ function eventHandlerForToMany(_related, field) { // Always returns a resource const maybeMakeResource = < - TABLE extends SpecifyTable, + TABLE extends SpecifyTable, TABLE_SCHEMA extends Tables[TABLE['name']] >( value: - | SpecifyResource - | Partial< - | SerializedResource - | SerializedRecord - | { readonly id?: number } - >, + | Partial | SerializedResource> + | SpecifyResource, relatedTable: TABLE ): SpecifyResource => value instanceof ResourceBase diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/saveBlockers.tsx b/specifyweb/frontend/js_src/lib/components/DataModel/saveBlockers.tsx index 28501c12dc2..ed6ce30ac50 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/saveBlockers.tsx +++ b/specifyweb/frontend/js_src/lib/components/DataModel/saveBlockers.tsx @@ -109,8 +109,8 @@ export function useSaveBlockers( ]; } -export function setSaveBlockers( - resource: SpecifyResource, +export function setSaveBlockers( + resource: SpecifyResource, field: LiteralField | Relationship, errors: Parameters>[typeof SET]>[0], blockerKey: string diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts b/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts index b8dc99894ef..43bcfdd833d 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts @@ -74,7 +74,7 @@ type CollectionConstructor = new ( >; readonly domainfilter?: boolean; }, - initalResources?: RA> + initalResources?: RA> ) => UnFetchedCollection; export type UnFetchedCollection = { diff --git a/specifyweb/frontend/js_src/lib/components/FormCells/FormTable.tsx b/specifyweb/frontend/js_src/lib/components/FormCells/FormTable.tsx index acc11b08d04..d6173ee3495 100644 --- a/specifyweb/frontend/js_src/lib/components/FormCells/FormTable.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormCells/FormTable.tsx @@ -30,7 +30,7 @@ import type { SortConfig } from '../Molecules/Sorting'; import { SortIndicator } from '../Molecules/Sorting'; import { hasTablePermission } from '../Permissions/helpers'; import { userPreferences } from '../Preferences/userPreferences'; -import { SearchDialog, useSearchDialog } from '../SearchDialog'; +import { useSearchDialog } from '../SearchDialog'; import { AttachmentPluginSkeleton } from '../SkeletonLoaders/AttachmentPlugin'; import { relationshipIsToMany } from '../WbPlanView/mappingHelpers'; import { FormCell } from './index'; From 014f87169d4dcdda4a57d533d0ffe355f41233c1 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Mon, 2 Sep 2024 11:14:42 -0500 Subject: [PATCH 31/66] Resolve frontend tests --- .../__snapshots__/specifyTable.test.ts.snap | 104 ++++++++++++++++++ .../DataModel/__tests__/resource.test.ts | 2 + .../DataModel/__tests__/resourceApi.test.ts | 2 +- .../DataModel/__tests__/specifyTable.test.ts | 101 +---------------- .../Formatters/__tests__/formatters.test.ts | 2 +- .../components/Syncer/__tests__/index.test.ts | 4 +- .../tests/ajax/static/api/specify_trees.json | 2 - 7 files changed, 111 insertions(+), 106 deletions(-) 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 38b0269a18c..a45d476e10c 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 @@ -26,6 +26,7 @@ exports[`fields are loaded 1`] = ` "[literalField CollectionObject.text2]", "[literalField CollectionObject.inventoryDate]", "[literalField CollectionObject.inventoryDatePrecision]", + "[literalField CollectionObject.isMemberOfCOG]", "[literalField CollectionObject.modifier]", "[literalField CollectionObject.name]", "[literalField CollectionObject.notifications]", @@ -101,6 +102,108 @@ exports[`fields are loaded 1`] = ` ] `; +exports[`indexed fields are loaded 1`] = ` +{ + "accession": "[relationship CollectionObject.accession]", + "actualTotalCountAmt": "[literalField CollectionObject.actualTotalCountAmt]", + "agent1": "[relationship CollectionObject.agent1]", + "altCatalogNumber": "[literalField CollectionObject.altCatalogNumber]", + "appraisal": "[relationship CollectionObject.appraisal]", + "availability": "[literalField CollectionObject.availability]", + "catalogNumber": "[literalField CollectionObject.catalogNumber]", + "catalogedDate": "[literalField CollectionObject.catalogedDate]", + "catalogedDatePrecision": "[literalField CollectionObject.catalogedDatePrecision]", + "catalogedDateVerbatim": "[literalField CollectionObject.catalogedDateVerbatim]", + "cataloger": "[relationship CollectionObject.cataloger]", + "cojo": "[relationship CollectionObject.cojo]", + "collectingEvent": "[relationship CollectionObject.collectingEvent]", + "collection": "[relationship CollectionObject.collection]", + "collectionMemberId": "[literalField CollectionObject.collectionMemberId]", + "collectionObjectAttachments": "[relationship CollectionObject.collectionObjectAttachments]", + "collectionObjectAttribute": "[relationship CollectionObject.collectionObjectAttribute]", + "collectionObjectAttrs": "[relationship CollectionObject.collectionObjectAttrs]", + "collectionObjectCitations": "[relationship CollectionObject.collectionObjectCitations]", + "collectionObjectProperties": "[relationship CollectionObject.collectionObjectProperties]", + "collectionObjectType": "[relationship CollectionObject.collectionObjectType]", + "conservDescriptions": "[relationship CollectionObject.conservDescriptions]", + "container": "[relationship CollectionObject.container]", + "containerOwner": "[relationship CollectionObject.containerOwner]", + "countAmt": "[literalField CollectionObject.countAmt]", + "createdByAgent": "[relationship CollectionObject.createdByAgent]", + "currentDetermination": "[relationship CollectionObject.currentDetermination]", + "date1": "[literalField CollectionObject.date1]", + "date1Precision": "[literalField CollectionObject.date1Precision]", + "deaccessioned": "[literalField CollectionObject.deaccessioned]", + "description": "[literalField CollectionObject.description]", + "determinations": "[relationship CollectionObject.determinations]", + "dnaSequences": "[relationship CollectionObject.dnaSequences]", + "embargoAuthority": "[relationship CollectionObject.embargoAuthority]", + "embargoReason": "[literalField CollectionObject.embargoReason]", + "embargoReleaseDate": "[literalField CollectionObject.embargoReleaseDate]", + "embargoReleaseDatePrecision": "[literalField CollectionObject.embargoReleaseDatePrecision]", + "embargoStartDate": "[literalField CollectionObject.embargoStartDate]", + "embargoStartDatePrecision": "[literalField CollectionObject.embargoStartDatePrecision]", + "exsiccataItems": "[relationship CollectionObject.exsiccataItems]", + "fieldNotebookPage": "[relationship CollectionObject.fieldNotebookPage]", + "fieldNumber": "[literalField CollectionObject.fieldNumber]", + "guid": "[literalField CollectionObject.guid]", + "integer1": "[literalField CollectionObject.integer1]", + "integer2": "[literalField CollectionObject.integer2]", + "inventorizedBy": "[relationship CollectionObject.inventorizedBy]", + "inventoryDate": "[literalField CollectionObject.inventoryDate]", + "inventoryDatePrecision": "[literalField CollectionObject.inventoryDatePrecision]", + "isMemberOfCOG": "[literalField CollectionObject.isMemberOfCOG]", + "leftSideRels": "[relationship CollectionObject.leftSideRels]", + "modifiedByAgent": "[relationship CollectionObject.modifiedByAgent]", + "modifier": "[literalField CollectionObject.modifier]", + "name": "[literalField CollectionObject.name]", + "notifications": "[literalField CollectionObject.notifications]", + "number1": "[literalField CollectionObject.number1]", + "number2": "[literalField CollectionObject.number2]", + "numberOfDuplicates": "[literalField CollectionObject.numberOfDuplicates]", + "objectCondition": "[literalField CollectionObject.objectCondition]", + "ocr": "[literalField CollectionObject.ocr]", + "otherIdentifiers": "[relationship CollectionObject.otherIdentifiers]", + "paleoContext": "[relationship CollectionObject.paleoContext]", + "preparations": "[relationship CollectionObject.preparations]", + "projectNumber": "[literalField CollectionObject.projectNumber]", + "projects": "[relationship CollectionObject.projects]", + "remarks": "[literalField CollectionObject.remarks]", + "reservedInteger3": "[literalField CollectionObject.reservedInteger3]", + "reservedInteger4": "[literalField CollectionObject.reservedInteger4]", + "reservedText": "[literalField CollectionObject.reservedText]", + "reservedText2": "[literalField CollectionObject.reservedText2]", + "reservedText3": "[literalField CollectionObject.reservedText3]", + "restrictions": "[literalField CollectionObject.restrictions]", + "rightSideRels": "[relationship CollectionObject.rightSideRels]", + "sgrStatus": "[literalField CollectionObject.sgrStatus]", + "text1": "[literalField CollectionObject.text1]", + "text2": "[literalField CollectionObject.text2]", + "text3": "[literalField CollectionObject.text3]", + "text4": "[literalField CollectionObject.text4]", + "text5": "[literalField CollectionObject.text5]", + "text6": "[literalField CollectionObject.text6]", + "text7": "[literalField CollectionObject.text7]", + "text8": "[literalField CollectionObject.text8]", + "timestampCreated": "[literalField CollectionObject.timestampCreated]", + "timestampModified": "[literalField CollectionObject.timestampModified]", + "totalCountAmt": "[literalField CollectionObject.totalCountAmt]", + "totalValue": "[literalField CollectionObject.totalValue]", + "treatmentEvents": "[relationship CollectionObject.treatmentEvents]", + "uniqueIdentifier": "[literalField CollectionObject.uniqueIdentifier]", + "version": "[literalField CollectionObject.version]", + "visibility": "[literalField CollectionObject.visibility]", + "visibilitySetBy": "[relationship CollectionObject.visibilitySetBy]", + "voucherRelationships": "[relationship CollectionObject.voucherRelationships]", + "yesNo1": "[literalField CollectionObject.yesNo1]", + "yesNo2": "[literalField CollectionObject.yesNo2]", + "yesNo3": "[literalField CollectionObject.yesNo3]", + "yesNo4": "[literalField CollectionObject.yesNo4]", + "yesNo5": "[literalField CollectionObject.yesNo5]", + "yesNo6": "[literalField CollectionObject.yesNo6]", +} +`; + exports[`literal fields are loaded 1`] = ` [ "[literalField CollectionObject.actualTotalCountAmt]", @@ -127,6 +230,7 @@ exports[`literal fields are loaded 1`] = ` "[literalField CollectionObject.text2]", "[literalField CollectionObject.inventoryDate]", "[literalField CollectionObject.inventoryDatePrecision]", + "[literalField CollectionObject.isMemberOfCOG]", "[literalField CollectionObject.modifier]", "[literalField CollectionObject.name]", "[literalField CollectionObject.notifications]", diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resource.test.ts b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resource.test.ts index beec6f4d451..7f4de6b9bad 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resource.test.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resource.test.ts @@ -294,6 +294,7 @@ test('getFieldsToNotClone', () => { 'catalogNumber', 'timestampModified', 'guid', + 'isMemberOfCOG', 'timestampCreated', 'totalCountAmt', 'uniqueIdentifier', @@ -307,6 +308,7 @@ test('getFieldsToNotClone', () => { 'catalogNumber', 'timestampModified', 'guid', + 'isMemberOfCOG', 'text1', 'timestampCreated', 'totalCountAmt', diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts index 57c4118a250..62f09a5fac7 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts @@ -147,7 +147,7 @@ const accessionsResponse = [ ]; overrideAjax( - '/api/specify/accession/?domainfilter=false&addressofrecord=42&offset=0', + '/api/specify/accession/?addressofrecord=42&domainfilter=false&limit=0', { meta: { total_count: 2 }, objects: accessionsResponse, diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/specifyTable.test.ts b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/specifyTable.test.ts index 8da5ba64de8..07b923db338 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/specifyTable.test.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/specifyTable.test.ts @@ -364,103 +364,4 @@ test('tableScoping', () => ).toMatchSnapshot()); test('indexed fields are loaded', () => - expect(tables.CollectionObject.field).toMatchInlineSnapshot(` - { - "accession": "[relationship CollectionObject.accession]", - "actualTotalCountAmt": "[literalField CollectionObject.actualTotalCountAmt]", - "agent1": "[relationship CollectionObject.agent1]", - "altCatalogNumber": "[literalField CollectionObject.altCatalogNumber]", - "appraisal": "[relationship CollectionObject.appraisal]", - "availability": "[literalField CollectionObject.availability]", - "catalogNumber": "[literalField CollectionObject.catalogNumber]", - "catalogedDate": "[literalField CollectionObject.catalogedDate]", - "catalogedDatePrecision": "[literalField CollectionObject.catalogedDatePrecision]", - "catalogedDateVerbatim": "[literalField CollectionObject.catalogedDateVerbatim]", - "cataloger": "[relationship CollectionObject.cataloger]", - "cojo": "[relationship CollectionObject.cojo]", - "collectingEvent": "[relationship CollectionObject.collectingEvent]", - "collection": "[relationship CollectionObject.collection]", - "collectionMemberId": "[literalField CollectionObject.collectionMemberId]", - "collectionObjectAttachments": "[relationship CollectionObject.collectionObjectAttachments]", - "collectionObjectAttribute": "[relationship CollectionObject.collectionObjectAttribute]", - "collectionObjectAttrs": "[relationship CollectionObject.collectionObjectAttrs]", - "collectionObjectCitations": "[relationship CollectionObject.collectionObjectCitations]", - "collectionObjectProperties": "[relationship CollectionObject.collectionObjectProperties]", - "collectionObjectType": "[relationship CollectionObject.collectionObjectType]", - "conservDescriptions": "[relationship CollectionObject.conservDescriptions]", - "container": "[relationship CollectionObject.container]", - "containerOwner": "[relationship CollectionObject.containerOwner]", - "countAmt": "[literalField CollectionObject.countAmt]", - "createdByAgent": "[relationship CollectionObject.createdByAgent]", - "currentDetermination": "[relationship CollectionObject.currentDetermination]", - "date1": "[literalField CollectionObject.date1]", - "date1Precision": "[literalField CollectionObject.date1Precision]", - "deaccessioned": "[literalField CollectionObject.deaccessioned]", - "description": "[literalField CollectionObject.description]", - "determinations": "[relationship CollectionObject.determinations]", - "dnaSequences": "[relationship CollectionObject.dnaSequences]", - "embargoAuthority": "[relationship CollectionObject.embargoAuthority]", - "embargoReason": "[literalField CollectionObject.embargoReason]", - "embargoReleaseDate": "[literalField CollectionObject.embargoReleaseDate]", - "embargoReleaseDatePrecision": "[literalField CollectionObject.embargoReleaseDatePrecision]", - "embargoStartDate": "[literalField CollectionObject.embargoStartDate]", - "embargoStartDatePrecision": "[literalField CollectionObject.embargoStartDatePrecision]", - "exsiccataItems": "[relationship CollectionObject.exsiccataItems]", - "fieldNotebookPage": "[relationship CollectionObject.fieldNotebookPage]", - "fieldNumber": "[literalField CollectionObject.fieldNumber]", - "guid": "[literalField CollectionObject.guid]", - "integer1": "[literalField CollectionObject.integer1]", - "integer2": "[literalField CollectionObject.integer2]", - "inventorizedBy": "[relationship CollectionObject.inventorizedBy]", - "inventoryDate": "[literalField CollectionObject.inventoryDate]", - "inventoryDatePrecision": "[literalField CollectionObject.inventoryDatePrecision]", - "leftSideRels": "[relationship CollectionObject.leftSideRels]", - "modifiedByAgent": "[relationship CollectionObject.modifiedByAgent]", - "modifier": "[literalField CollectionObject.modifier]", - "name": "[literalField CollectionObject.name]", - "notifications": "[literalField CollectionObject.notifications]", - "number1": "[literalField CollectionObject.number1]", - "number2": "[literalField CollectionObject.number2]", - "numberOfDuplicates": "[literalField CollectionObject.numberOfDuplicates]", - "objectCondition": "[literalField CollectionObject.objectCondition]", - "ocr": "[literalField CollectionObject.ocr]", - "otherIdentifiers": "[relationship CollectionObject.otherIdentifiers]", - "paleoContext": "[relationship CollectionObject.paleoContext]", - "preparations": "[relationship CollectionObject.preparations]", - "projectNumber": "[literalField CollectionObject.projectNumber]", - "projects": "[relationship CollectionObject.projects]", - "remarks": "[literalField CollectionObject.remarks]", - "reservedInteger3": "[literalField CollectionObject.reservedInteger3]", - "reservedInteger4": "[literalField CollectionObject.reservedInteger4]", - "reservedText": "[literalField CollectionObject.reservedText]", - "reservedText2": "[literalField CollectionObject.reservedText2]", - "reservedText3": "[literalField CollectionObject.reservedText3]", - "restrictions": "[literalField CollectionObject.restrictions]", - "rightSideRels": "[relationship CollectionObject.rightSideRels]", - "sgrStatus": "[literalField CollectionObject.sgrStatus]", - "text1": "[literalField CollectionObject.text1]", - "text2": "[literalField CollectionObject.text2]", - "text3": "[literalField CollectionObject.text3]", - "text4": "[literalField CollectionObject.text4]", - "text5": "[literalField CollectionObject.text5]", - "text6": "[literalField CollectionObject.text6]", - "text7": "[literalField CollectionObject.text7]", - "text8": "[literalField CollectionObject.text8]", - "timestampCreated": "[literalField CollectionObject.timestampCreated]", - "timestampModified": "[literalField CollectionObject.timestampModified]", - "totalCountAmt": "[literalField CollectionObject.totalCountAmt]", - "totalValue": "[literalField CollectionObject.totalValue]", - "treatmentEvents": "[relationship CollectionObject.treatmentEvents]", - "uniqueIdentifier": "[literalField CollectionObject.uniqueIdentifier]", - "version": "[literalField CollectionObject.version]", - "visibility": "[literalField CollectionObject.visibility]", - "visibilitySetBy": "[relationship CollectionObject.visibilitySetBy]", - "voucherRelationships": "[relationship CollectionObject.voucherRelationships]", - "yesNo1": "[literalField CollectionObject.yesNo1]", - "yesNo2": "[literalField CollectionObject.yesNo2]", - "yesNo3": "[literalField CollectionObject.yesNo3]", - "yesNo4": "[literalField CollectionObject.yesNo4]", - "yesNo5": "[literalField CollectionObject.yesNo5]", - "yesNo6": "[literalField CollectionObject.yesNo6]", - } - `)); + expect(tables.CollectionObject.field).toMatchSnapshot()); diff --git a/specifyweb/frontend/js_src/lib/components/Formatters/__tests__/formatters.test.ts b/specifyweb/frontend/js_src/lib/components/Formatters/__tests__/formatters.test.ts index ee14ccd2fc3..9b2f8fbd7df 100644 --- a/specifyweb/frontend/js_src/lib/components/Formatters/__tests__/formatters.test.ts +++ b/specifyweb/frontend/js_src/lib/components/Formatters/__tests__/formatters.test.ts @@ -120,7 +120,7 @@ const taxonCitation = { referenceWork: getResourceApiUrl('ReferenceWork', referenceWorkId), }; overrideAjax( - '/api/specify/taxoncitation/?domainfilter=false&referencework=1&offset=0', + '/api/specify/taxoncitation/?referencework=1&domainfilter=false&limit=0', { meta: { total_count: 1, diff --git a/specifyweb/frontend/js_src/lib/components/Syncer/__tests__/index.test.ts b/specifyweb/frontend/js_src/lib/components/Syncer/__tests__/index.test.ts index 89b362c28c0..bb7abf22a9c 100644 --- a/specifyweb/frontend/js_src/lib/components/Syncer/__tests__/index.test.ts +++ b/specifyweb/frontend/js_src/lib/components/Syncer/__tests__/index.test.ts @@ -94,8 +94,8 @@ test('Editing Data Object Formatter', () => { - - + + " `); diff --git a/specifyweb/frontend/js_src/lib/tests/ajax/static/api/specify_trees.json b/specifyweb/frontend/js_src/lib/tests/ajax/static/api/specify_trees.json index 52fcc38131d..4a255c74ace 100644 --- a/specifyweb/frontend/js_src/lib/tests/ajax/static/api/specify_trees.json +++ b/specifyweb/frontend/js_src/lib/tests/ajax/static/api/specify_trees.json @@ -538,7 +538,6 @@ "version": 8, "createdbyagent": null, "modifiedbyagent": "/api/specify/agent/1514/", - "institutions": "/api/specify/institution/?storagetreedef=1", "treeentries": "/api/specify/storage/?definition=1", "treedefitems": [ { @@ -736,7 +735,6 @@ "version": 3, "createdbyagent": null, "modifiedbyagent": "/api/specify/agent/1514/", - "disciplines": "/api/specify/discipline/?geographytreedef=1", "treeentries": "/api/specify/geography/?definition=1", "treedefitems": [ { From 629b92c38fcd5d1db3c78243b56bf29bf2eb1ee0 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Mon, 2 Sep 2024 11:23:29 -0500 Subject: [PATCH 32/66] Check independent update permission when removing from relationship --- specifyweb/specify/api.py | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/specifyweb/specify/api.py b/specifyweb/specify/api.py index 59e9a37f00d..939fb9949c5 100644 --- a/specifyweb/specify/api.py +++ b/specifyweb/specify/api.py @@ -601,18 +601,8 @@ def handle_fk_fields(collection, agent, obj, data: Dict[str, Any]) -> Tuple[List elif hasattr(val, 'items'): # i.e. it's a dict of some sort # The related object is represented by a nested dict of data. rel_model = field.related_model - if 'id' in val: - # The related object is an existing resource with an id. - # This should never happen for dependent resources. - rel_obj = update_obj(collection, agent, - rel_model, val['id'], - val['version'], val, - parent_obj=obj if dependent else None) - else: - # The related object is to be created. - rel_obj = create_obj(collection, agent, - rel_model, val, - parent_obj=obj if dependent else None) + + rel_obj = update_or_create_resource(collection, agent, rel_model, val, obj if dependent else None) setattr(obj, field_name, rel_obj) if dependent and old_related and old_related.id != rel_obj.id: @@ -677,8 +667,6 @@ def handle_to_many(collection, agent, obj, data: Dict[str, Any]) -> None: rel_obj = update_or_create_resource(collection, agent, rel_model, rel_data, parent_obj=obj if is_dependent else None) - if not is_dependent and not (isinstance(obj, models.Recordset) and field_name == 'recordsetitems'): - getattr(obj, field_name).add(rel_obj) ids.append(rel_obj.id) # Record the id as one to keep. # Delete related objects not in the ids list. @@ -691,6 +679,8 @@ def handle_to_many(collection, agent, obj, data: Dict[str, Any]) -> None: to_remove.delete() else: + for rel_obj in to_remove: + check_table_permissions(collection, agent, to_remove[0], 'update') getattr(obj, field_name).remove(*list(to_remove)) def update_or_create_resource(collection, agent, model, data, parent_obj): From 6ab0e34301b61f39ecd11cd8341502a99aa90e98 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Tue, 3 Sep 2024 13:24:37 -0500 Subject: [PATCH 33/66] Don't render SubView if reverse relationship doesn't exist --- .../js_src/lib/components/FormEditor/viewSpec.ts | 8 ++++++++ .../frontend/js_src/lib/components/Forms/SubView.tsx | 10 ++++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/FormEditor/viewSpec.ts b/specifyweb/frontend/js_src/lib/components/FormEditor/viewSpec.ts index 772e53d6d3d..9a4a6ac76e9 100644 --- a/specifyweb/frontend/js_src/lib/components/FormEditor/viewSpec.ts +++ b/specifyweb/frontend/js_src/lib/components/FormEditor/viewSpec.ts @@ -369,6 +369,14 @@ const subViewSpec = ( console.error('SubView can only be used to display a relationship'); return undefined; } + if (field !== undefined && field.getReverse() === undefined) { + console.error( + `No reverse relationship exists${ + relationshipIsToMany(field) ? '' : '. Use a querycbx instead' + }` + ); + return undefined; + } if (field?.type === 'many-to-many') { // ResourceApi does not support .rget() on a many-to-many console.warn('Many-to-many relationships are not supported'); diff --git a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx index d8356bf81d1..ff0a60eb9c6 100644 --- a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx +++ b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx @@ -70,8 +70,9 @@ export function SubView({ const [sortField, setSortField] = useTriggerState(initialSortField); const fetchCollection = React.useCallback( + // If false is returned, then the Subview should not be rendered async function fetchCollection(): Promise< - Collection | undefined + Collection | false | undefined > { if ( relationshipIsToMany(relationship) && @@ -120,7 +121,7 @@ export function SubView({ `reverse relationship does not exist` ) ); - return undefined; + return false; } const collection = ( relationship.isDependent() @@ -156,7 +157,7 @@ export function SubView({ ); const [collection, setCollection] = React.useState< - Collection | undefined + Collection | false | undefined >(undefined); const versionRef = React.useRef(0); React.useEffect( @@ -226,7 +227,8 @@ export function SubView({ return ( - {parentContext.includes(relationship) ? undefined : ( + {parentContext.includes(relationship) || + collection === false ? undefined : ( <> {isButton && ( Date: Wed, 4 Sep 2024 10:21:31 -0500 Subject: [PATCH 34/66] Insert removal of independent to-manys into auditlog --- specifyweb/specify/api.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/specifyweb/specify/api.py b/specifyweb/specify/api.py index 939fb9949c5..73a27897dcc 100644 --- a/specifyweb/specify/api.py +++ b/specifyweb/specify/api.py @@ -29,6 +29,7 @@ from .uiformatters import AutonumberOverflowException from .filter_by_col import filter_by_collection from .auditlog import auditlog +from .datamodel import datamodel from .calculated_fields import calculate_extra_fields ReadPermChecker = Callable[[Any], None] @@ -680,7 +681,12 @@ def handle_to_many(collection, agent, obj, data: Dict[str, Any]) -> None: to_remove.delete() else: for rel_obj in to_remove: - check_table_permissions(collection, agent, to_remove[0], 'update') + check_table_permissions(collection, agent, rel_obj, 'update') + related_field = datamodel.reverse_relationship(obj.specify_model.get_field_strict(field_name)) + if related_field is not None: + # REFACTOR: use fld_change_info + field_change_info: FieldChangeInfo = {"field_name": related_field.name, "old_value": obj.id, "new_value": None} + auditlog.update(rel_obj, agent, None, [field_change_info]) getattr(obj, field_name).remove(*list(to_remove)) def update_or_create_resource(collection, agent, model, data, parent_obj): From 067aaa6ae643ebe7b189b1cd7cd5711997ce4446 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 4 Sep 2024 16:46:23 -0500 Subject: [PATCH 35/66] Resolve backend tests --- specifyweb/specify/tests/test_api.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/specifyweb/specify/tests/test_api.py b/specifyweb/specify/tests/test_api.py index 2646bbd1d66..ebb1d4dbdf1 100644 --- a/specifyweb/specify/tests/test_api.py +++ b/specifyweb/specify/tests/test_api.py @@ -665,8 +665,8 @@ def test_inline_error_handling(self): ) other_collection_object_data = { - 'id': self.collectionobjects[0].id, - 'catalognumber': self.collectionobjects[0].catalognumber, + 'id': self.collectionobjects[1].id, + 'catalognumber': self.collectionobjects[1].catalognumber, 'collection': api.uri_for_model('Collection', self.collection.id), 'determinations': [ api.uri_for_model('determination', new_determination.id) @@ -675,8 +675,8 @@ def test_inline_error_handling(self): with self.assertRaises(AssertionError): api.update_obj(self.collection, self.agent, - 'Collectionobject', self.collectionobjects[0].id, - self.collectionobjects[0].version, other_collection_object_data) + 'Collectionobject', self.collectionobjects[1].id, + self.collectionobjects[1].version, other_collection_object_data) # version control on inlined resources should be tested From f10e41062107053d082c5a0daa818b54d722ea53 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 4 Sep 2024 17:10:16 -0500 Subject: [PATCH 36/66] Resolve frontend tests --- .../lib/components/DataModel/scoping.ts | 36 +++++++++---------- .../lib/components/DataModel/specifyTable.ts | 1 + 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/scoping.ts b/specifyweb/frontend/js_src/lib/components/DataModel/scoping.ts index 20279182832..304640ff3ec 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/scoping.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/scoping.ts @@ -1,6 +1,5 @@ import type { RA } from '../../utils/types'; import { takeBetween } from '../../utils/utils'; -import { raise } from '../Errors/Crash'; import { getCollectionPref } from '../InitialContext/remotePrefs'; import { getTablePermissions } from '../Permissions'; import { hasTablePermission } from '../Permissions/helpers'; @@ -10,11 +9,11 @@ import type { AnySchema } from './helperTypes'; import type { SpecifyResource } from './legacyTypes'; import { getResourceApiUrl, idFromUrl } from './resource'; import { schema } from './schema'; +import { serializeResource } from './serializers'; import type { Relationship } from './specifyField'; import type { SpecifyTable } from './specifyTable'; import { strictGetTable, tables } from './tables'; -import type { CollectionObject } from './types'; -import type { Tables } from './types'; +import type { CollectionObject, Tables } from './types'; /** * Some tasks to do after a new resource is created @@ -51,27 +50,26 @@ export function initializeResource(resource: SpecifyResource): void { getCollectionPref('CO_CREATE_PREP', schema.domainLevelIds.collection) && hasTablePermission('Preparation', 'create') && resource.createdBy !== 'clone' - ) - collectionObject - .rgetCollection('preparations') - .then((preparations) => { - if (preparations.models.length === 0) - preparations.add(new tables.Preparation.Resource()); - }) - .catch(raise); + ) { + const preps = collectionObject.getDependentResource('preparations') ?? []; + if (preps.length === 0) + collectionObject.set('preparations', [ + serializeResource(new tables.Preparation.Resource()), + ]); + } if ( getCollectionPref('CO_CREATE_DET', schema.domainLevelIds.collection) && hasTablePermission('Determination', 'create') && resource.createdBy !== 'clone' - ) - collectionObject - .rgetCollection('determinations') - .then((determinations) => { - if (determinations.models.length === 0) - determinations.add(new tables.Determination.Resource()); - }) - .catch(raise); + ) { + const determinations = + collectionObject.getDependentResource('determinations') ?? []; + if (determinations.length === 0) + collectionObject.set('determinations', [ + serializeResource(new tables.Determination.Resource()), + ]); + } } export function getDomainResource< diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts b/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts index 43bcfdd833d..1eede1b49e0 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts @@ -88,6 +88,7 @@ export type Collection = { readonly related?: SpecifyResource; readonly _totalCount?: number; readonly models: RA>; + readonly length: number; readonly table: { readonly specifyTable: SpecifyTable; }; From a6a233df0c71db31de2899c45c9db06e688d2632 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Mon, 9 Sep 2024 08:46:48 -0500 Subject: [PATCH 37/66] Make independent subview lazy --- .../lib/components/DataModel/collectionApi.ts | 176 ++++++++++-------- .../lib/components/DataModel/resourceApi.ts | 2 +- .../FormSliders/IntegratedRecordSelector.tsx | 12 +- .../RecordSelectorFromCollection.tsx | 5 +- .../js_src/lib/components/Forms/SubView.tsx | 6 +- specifyweb/specify/api.py | 103 ++++++++-- specifyweb/specify/tests/test_api.py | 57 +++--- 7 files changed, 236 insertions(+), 125 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts index 556161ca286..0276a11010a 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts @@ -2,8 +2,8 @@ import _ from 'underscore'; +import { removeKey } from '../../utils/utils'; import { assert } from '../Errors/assert'; -import { formatUrl } from '../Router/queryString'; import { Backbone } from './backbone'; import type { AnySchema } from './helperTypes'; import type { SpecifyResource } from './legacyTypes'; @@ -92,78 +92,6 @@ export const DependentCollection = Base.extend({ create: notSupported, }); -export const IndependentCollection = Base.extend({ - __name__: 'IndependentCollectionBase', - constructor(options, records = []) { - this.table = this.model; - assert(_.isArray(records)); - Base.call(this, records, options); - this.filters = options.filters || {}; - this.domainfilter = - Boolean(options.domainfilter) && - this.model?.specifyTable.getScopingRelationship() !== undefined; - }, - initialize(_tables, options) { - this.on( - 'change add remove', - function (resource: SpecifyResource) { - this.trigger('saverequired'); - }, - this - ); - - setupToOne(this, options); - }, - url() { - return `/api/specify/${this.model.specifyTable.name.toLowerCase()}/`; - }, - parse(resp) { - let records; - if (resp.meta) { - records = resp.objects; - } else { - console.warn("expected 'meta' in response"); - records = resp; - } - return records; - }, - async fetch(options) { - if (this.related.isNew() || this.related.isBeingInitialized()) { - return this; - } - - this.filters[this.field.name.toLowerCase()] = this.related.id; - if (this._fetch) return this._fetch; - - options = { ...(options ?? {}), silent: true }; - - assert(options.at == null); - - options.data = options.data || { - ...this.filters, - domainfilter: this.domainfilter, - limit: options.limit, - }; - const self = this; - this._fetch = Backbone.Collection.prototype.fetch.call(this, options); - return this._fetch.then(() => { - self._fetch = null; - return self; - }); - }, - isComplete() { - return true; - }, - toApiJSON(options) { - const self = this; - - return this.map(function (resource: SpecifyResource) { - const formatAsObject = resource.needsSaved || resource.isNew(); - return formatAsObject ? resource.toJSON(options) : resource.url(); - }); - }, -}); - export const LazyCollection = Base.extend({ __name__: 'LazyCollectionBase', _neverFetched: true, @@ -213,7 +141,7 @@ export const LazyCollection = Base.extend({ options.data = options.data || _.extend({ domainfilter: this.domainfilter }, this.filters); - options.data.offset = this.length; + options.data.offset = options.offset || this.length; _(options).has('limit') && (options.data.limit = options.limit); this._fetch = Backbone.Collection.prototype.fetch.call(this, options); @@ -233,6 +161,106 @@ export const LazyCollection = Base.extend({ }, }); +export const IndependentCollection = LazyCollection.extend({ + __name__: 'IndependentCollectionBase', + constructor(options, records = []) { + this.table = this.model; + assert(_.isArray(records)); + Base.call(this, records, options); + this.filters = options.filters || {}; + this.domainfilter = + Boolean(options.domainfilter) && + this.model?.specifyTable.getScopingRelationship() !== undefined; + + this.removed = new Set(); + this.updated = {}; + }, + initialize(_tables, options) { + this.on( + 'change', + function (resource: SpecifyResource) { + if (!resource.isBeingInitialized()) { + this.updated[resource.cid] = resource; + this.trigger('saverequired'); + } + }, + this + ); + + this.on( + 'add', + function (resource: SpecifyResource) { + if (!resource.isNew()) { + (this.removed as Set).delete(resource.url()); + this.updated[resource.cid] = resource.url(); + } else { + this.updated[resource.cid] = resource; + } + this._totalCount += 1; + this.trigger('saverequired'); + }, + this + ); + + this.on( + 'remove', + function (resource: SpecifyResource) { + if (!resource.isNew()) { + (this.removed as Set).add(resource.url()); + } + this.updated = removeKey(this.updated, resource.cid); + this._totalCount -= 1; + this.trigger('saverequired'); + }, + this + ); + + this.listenTo(options.related, 'saved', function () { + this.updated = {}; + this.removed = new Set(); + }); + + setupToOne(this, options); + }, + parse(resp) { + const self = this; + const records = Reflect.apply( + LazyCollection.prototype.parse, + this, + arguments + ); + + this._totalCount -= (this.removed as Set).size; + + return records.filter( + ({ resource_uri }) => !(this.removed as Set).has(resource_uri) + ); + }, + async fetch(options) { + if (this.related.isBeingInitialized()) { + return this; + } + this.filters[this.field.name.toLowerCase()] = this.related.id; + + const offset = + this.length === 0 && this.removed.size > 0 + ? this.removed.size + : this.length; + + options = { ...(options ?? {}), silent: true, offset }; + + return Reflect.apply(LazyCollection.prototype.fetch, this, [options]); + }, + toApiJSON(options) { + const self = this; + + return { + update: Object.values(this.updated), + remove: Array.from(self.removed), + }; + }, +}); + export const ToOneCollection = LazyCollection.extend({ __name__: 'LazyToOneCollectionBase', initialize(_models, options) { diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts index 45a39644a57..f45fc1340e9 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts @@ -806,7 +806,7 @@ export const ResourceBase = Backbone.Model.extend({ ? new relatedTable.IndependentCollection(collectionOptions) : existingToMany; - return collection.fetch({ limit: 0 }).then((fetchedCollection) => { + return collection.fetch().then((fetchedCollection) => { this.storeIndependent(field, fetchedCollection); return fetchedCollection; }); diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx index 66d6037ff93..36e62af0487 100644 --- a/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx @@ -1,5 +1,5 @@ import React from 'react'; -import { State } from 'typesafe-reducer'; +import type { State } from 'typesafe-reducer'; import { useSearchParameter } from '../../hooks/navigation'; import { useBooleanState } from '../../hooks/useBooleanState'; @@ -75,7 +75,12 @@ export function IntegratedRecordSelector({ const [state, setState] = React.useState< | State< 'AddResourceState', - { readonly resource: SpecifyResource } + { + readonly resource: SpecifyResource; + readonly handleAdd: ( + resources: RA> + ) => void; + } > | State<'MainState'> >({ type: 'MainState' }); @@ -231,6 +236,7 @@ export function IntegratedRecordSelector({ setState({ type: 'AddResourceState', resource, + handleAdd, }); }} /> @@ -315,7 +321,7 @@ export function IntegratedRecordSelector({ onClose={(): void => setState({ type: 'MainState' })} onDeleted={undefined} onSaved={(): void => { - handleAdd([state.resource]); + state.handleAdd([state.resource]); setState({ type: 'MainState' }); }} /> diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx index 7c72ed933e5..5d0f0440ec4 100644 --- a/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx @@ -5,6 +5,7 @@ import type { RA } from '../../utils/types'; import { defined } from '../../utils/types'; import { DependentCollection, + isRelationshipCollection, LazyCollection, } from '../DataModel/collectionApi'; import type { AnySchema } from '../DataModel/helperTypes'; @@ -96,7 +97,9 @@ export function RecordSelectorFromCollection({ table: collection.table.specifyTable, field: relationship, records, - relatedResource: isLazy ? undefined : collection.related, + relatedResource: isRelationshipCollection(collection) + ? collection.related + : undefined, totalCount: collection._totalCount ?? records.length, onAdd: (rawResources): void => { const resources = isToOne ? rawResources.slice(0, 1) : rawResources; diff --git a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx index ff0a60eb9c6..22279228fd6 100644 --- a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx +++ b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx @@ -263,7 +263,11 @@ export function SubView({ )} {typeof collection === 'object' && isOpen ? ( None: for field_name, val in list(data.items()): field = obj._meta.get_field(field_name) if not field.is_relation or (field.many_to_one or field.one_to_one): continue # Skip *-to-one fields. - is_dependent = is_dependent_field(obj, field_name) - - if not isinstance(val, list): - if is_dependent: - raise AssertionError("didn't get inline data for dependent field %s in %s: %r" % (field_name, obj, val)) - else: - # The field contains something other than nested data. - # Probably the URI of the collection - continue - + dependent = is_dependent_field(obj, field_name) + + if isinstance(val, list): + assert dependent or (isinstance(obj, models.Recordset) and field_name == 'recordsetitems'), \ + "got inline data for non dependent field %s in %s: %r" % (field_name, obj, val) + elif hasattr(val, "items"): + assert not dependent, "got inline dictionary data for dependent field %s in %s: %r" % (field_name, obj, val) + else: + # The field contains something other than nested data. + # Probably the URI of the collection + continue + + if dependent or (isinstance(obj, models.Recordset) and field_name == 'recordsetitems'): + _handle_dependent_to_many(collection, agent, obj, field, val) + else: + _handle_independent_to_many(collection, agent, obj, field, val) + + return + rel_model = field.related_model ids = [] # Ids not in this list will be deleted (if dependent) or removed from obj (if independent) at the end. - + ids_to_fetch = [] cached_objs = dict() fk_model = None @@ -689,6 +698,74 @@ def handle_to_many(collection, agent, obj, data: Dict[str, Any]) -> None: auditlog.update(rel_obj, agent, None, [field_change_info]) getattr(obj, field_name).remove(*list(to_remove)) +def _handle_dependent_to_many(collection, agent, obj, field, value): + if not isinstance(value, list): + assert isinstance(value, list), "didn't get inline data for dependent field %s in %s: %r" % (field.name, obj, value) + + rel_model = field.related_model + ids = [] # Ids not in this list will be deleted (if dependent) or removed from obj (if independent) at the end. + + for rel_data in value: + rel_data[field.field.name] = obj + + rel_obj = update_or_create_resource(collection, agent, rel_model, rel_data, parent_obj=obj) + + ids.append(rel_obj.id) # Record the id as one to keep. + + # Delete related objects not in the ids list. + # TODO: Check versions for optimistic locking. + to_remove = getattr(obj, field.name).exclude(id__in=ids).select_for_update() + for rel_obj in to_remove: + check_table_permissions(collection, agent, rel_obj, "delete") + auditlog.remove(rel_obj, agent, obj) + + to_remove.delete() + +class IndependentInline(TypedDict): + update: List[Union[str, Dict[str, Any]]] + remove: List[str] + +def _handle_independent_to_many(collection, agent, obj, field, value: IndependentInline): + logger.warning("Updating independent collections via the API is experimental and the structure may be changed in the future") + + rel_model = field.related_model + + to_update = value.get('update', []) + to_remove = value.get('remove', []) + + ids_to_fetch = [] + cached_objs = dict() + fk_model = None + + # Fetch the related records which are provided as strings + for rel_data in [*to_update, *to_remove]: + if not isinstance(rel_data, str): continue + fk_model, fk_id = strict_uri_to_model(rel_data, rel_model.__name__) + ids_to_fetch.append(fk_id) + + if fk_model is not None: + cached_objs = {item.id: obj_to_data(item) for item in get_model(fk_model).objects.filter(id__in=ids_to_fetch).select_for_update()} + + for rel_data in to_update: + if isinstance(rel_data, str): + fk_model, fk_id = strict_uri_to_model(rel_data, rel_model.__name__) + rel_data = cached_objs[fk_id] + if rel_data[field.field.name] == uri_for_model(obj.__class__, obj.id): + continue + + rel_data[field.field.name] = obj + update_or_create_resource(collection, agent, rel_model, rel_data, None) + + if len(to_remove) > 0: + check_table_permissions(collection, agent, rel_model, 'update') + related_field = datamodel.reverse_relationship(obj.specify_model.get_field_strict(field.name)) + assert related_field is not None, f"no reverse relationship for {obj.__class__.__name__}.{field.field.name}" + for rel_obj in to_remove: + fk_model, fk_id = strict_uri_to_model(rel_obj, rel_model.__name__) + rel_data = cached_objs[fk_id] + rel_data[related_field.name] = None + update_obj(collection, agent, rel_model, rel_data["id"], rel_data["version"], rel_data) + def update_or_create_resource(collection, agent, model, data, parent_obj): if 'id' in data: return update_obj(collection, agent, diff --git a/specifyweb/specify/tests/test_api.py b/specifyweb/specify/tests/test_api.py index ebb1d4dbdf1..55e6ef677a9 100644 --- a/specifyweb/specify/tests/test_api.py +++ b/specifyweb/specify/tests/test_api.py @@ -498,10 +498,12 @@ def test_independent_to_many_set_inline(self): accession_data = { 'accessionnumber': "a", 'division': api.uri_for_model('division', self.division.id), - 'collectionobjects': [ - api.obj_to_data(self.collectionobjects[0]), - api.uri_for_model('collectionobject', self.collectionobjects[1].id) - ] + 'collectionobjects': { + "update": [ + api.obj_to_data(self.collectionobjects[0]), + api.uri_for_model('collectionobject', self.collectionobjects[1].id) + ] + } } accession = api.create_obj(self.collection, self.agent, 'Accession', accession_data) @@ -538,11 +540,13 @@ def test_indepenent_to_many_removing_from_inline(self): accession_data = { 'accessionnumber': "a", 'division': api.uri_for_model('division', self.division.id), - 'collectionobjects': [ - api.obj_to_data(collection_object) if index % 2 == 0 - else api.uri_for_model('collectionobject', collection_object.id) - for index, collection_object in enumerate(collection_objects_to_set) - ] + 'collectionobjects': { + "remove": [ + api.uri_for_model('collectionobject', collection_object.id) + for index, collection_object in enumerate(collection_objects_to_set) + if index % 2 == 0 + ] + } } accession = api.update_obj(self.collection, self.agent, 'Accession', accession.id, accession.version, accession_data) @@ -567,9 +571,11 @@ def test_updating_independent_to_many_resource(self): accession_data = { 'accessionnumber': "a", 'division': api.uri_for_model('division', self.division.id), - 'collectionobjects': [ + 'collectionobjects': { + "update": [ co_to_modify - ] + ] + } } self.assertEqual(self.collectionobjects[2].integer1, None) @@ -611,12 +617,14 @@ def test_independent_to_many_creating_from_remoteside(self): accession_data = { 'accessionnumber': "a", 'division': api.uri_for_model('division', self.division.id), - 'collectionobjects': [ + 'collectionobjects': { + "update": [ { 'catalognumber': new_catalognumber, 'collection': api.uri_for_model('Collection', self.collection.id) } - ] + ] + } } accession = api.create_obj(self.collection, self.agent, 'Accession', accession_data) @@ -636,10 +644,12 @@ def test_reassigning_independent_to_many(self): accession_data = { 'accessionnumber': "b", 'division': api.uri_for_model('division', self.division.id), - 'collectionobjects': [ + 'collectionobjects': { + "update": [ api.obj_to_data(self.collectionobjects[0]), api.uri_for_model('collectionobject', self.collectionobjects[1].id) - ] + ] + } } acc2 = api.create_obj(self.collection, self.agent, 'Accession', accession_data) self.collectionobjects[0].refresh_from_db() @@ -659,24 +669,7 @@ def test_inline_error_handling(self): api.update_obj(self.collection, self.agent, 'Collectionobject', self.collectionobjects[0].id, self.collectionobjects[0].version, collection_object_data) - - new_determination = models.Determination.objects.create( - collectionobject=self.collectionobjects[1] - ) - - other_collection_object_data = { - 'id': self.collectionobjects[1].id, - 'catalognumber': self.collectionobjects[1].catalognumber, - 'collection': api.uri_for_model('Collection', self.collection.id), - 'determinations': [ - api.uri_for_model('determination', new_determination.id) - ] - } - with self.assertRaises(AssertionError): - api.update_obj(self.collection, self.agent, - 'Collectionobject', self.collectionobjects[1].id, - self.collectionobjects[1].version, other_collection_object_data) # version control on inlined resources should be tested From 434047bed8ed721febc1264adeb86616b10c33a4 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Mon, 9 Sep 2024 09:07:36 -0500 Subject: [PATCH 38/66] Cleanup handle_to_many code --- specifyweb/specify/api.py | 51 --------------------------------------- 1 file changed, 51 deletions(-) diff --git a/specifyweb/specify/api.py b/specifyweb/specify/api.py index 4e8764843d3..4e3498a2ffc 100644 --- a/specifyweb/specify/api.py +++ b/specifyweb/specify/api.py @@ -647,57 +647,6 @@ def handle_to_many(collection, agent, obj, data: Dict[str, Any]) -> None: else: _handle_independent_to_many(collection, agent, obj, field, val) - return - - rel_model = field.related_model - ids = [] # Ids not in this list will be deleted (if dependent) or removed from obj (if independent) at the end. - - ids_to_fetch = [] - cached_objs = dict() - fk_model = None - if not is_dependent: - for rel_data in val: - if not isinstance(rel_data, str): continue - fk_model, fk_id = strict_uri_to_model(rel_data, rel_model.__name__) - ids_to_fetch.append(fk_id) - - if fk_model is not None: - cached_objs = {item.id: obj_to_data(item) for item in get_model(fk_model).objects.filter(id__in=ids_to_fetch)} - - for rel_data in val: - if isinstance(rel_data, str): - assert not is_dependent, "expected object for dependent field %s in %s: %s" % (field_name, obj, rel_data) - fk_model, fk_id = strict_uri_to_model(rel_data, rel_model.__name__) - rel_data = cached_objs[fk_id] - if rel_data[field.field.name] == uri_for_model(obj.__class__, obj.id): - ids.append(rel_data["id"]) - continue - - rel_data[field.field.name] = obj - - rel_obj = update_or_create_resource(collection, agent, rel_model, rel_data, parent_obj=obj if is_dependent else None) - - ids.append(rel_obj.id) # Record the id as one to keep. - - # Delete related objects not in the ids list. - # TODO: Check versions for optimistic locking. - to_remove = getattr(obj, field_name).exclude(id__in=ids).select_for_update() - if is_dependent or (isinstance(obj, models.Recordset) and field_name == 'recordsetitems'): - for rel_obj in to_remove: - check_table_permissions(collection, agent, rel_obj, "delete") - auditlog.remove(rel_obj, agent, obj) - - to_remove.delete() - else: - for rel_obj in to_remove: - check_table_permissions(collection, agent, rel_obj, 'update') - related_field = datamodel.reverse_relationship(obj.specify_model.get_field_strict(field_name)) - if related_field is not None: - # REFACTOR: use fld_change_info - field_change_info: FieldChangeInfo = {"field_name": related_field.name, "old_value": obj.id, "new_value": None} - auditlog.update(rel_obj, agent, None, [field_change_info]) - getattr(obj, field_name).remove(*list(to_remove)) - def _handle_dependent_to_many(collection, agent, obj, field, value): if not isinstance(value, list): assert isinstance(value, list), "didn't get inline data for dependent field %s in %s: %r" % (field.name, obj, value) From febbc16bce65eb9b6a8d5379ec27c9f0b9bcbcf1 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 11 Sep 2024 09:20:19 -0500 Subject: [PATCH 39/66] Cleanup API for to-many Collections --- specifyweb/specify/api.py | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/specifyweb/specify/api.py b/specifyweb/specify/api.py index 4e3498a2ffc..2d46539f575 100644 --- a/specifyweb/specify/api.py +++ b/specifyweb/specify/api.py @@ -651,24 +651,24 @@ def _handle_dependent_to_many(collection, agent, obj, field, value): if not isinstance(value, list): assert isinstance(value, list), "didn't get inline data for dependent field %s in %s: %r" % (field.name, obj, value) - rel_model = field.related_model - ids = [] # Ids not in this list will be deleted (if dependent) or removed from obj (if independent) at the end. + rel_model = field.related_model + ids = [] # Ids not in this list will be deleted (if dependent) or removed from obj (if independent) at the end. - for rel_data in value: - rel_data[field.field.name] = obj + for rel_data in value: + rel_data[field.field.name] = obj - rel_obj = update_or_create_resource(collection, agent, rel_model, rel_data, parent_obj=obj) + rel_obj = update_or_create_resource(collection, agent, rel_model, rel_data, parent_obj=obj) - ids.append(rel_obj.id) # Record the id as one to keep. + ids.append(rel_obj.id) # Record the id as one to keep. - # Delete related objects not in the ids list. - # TODO: Check versions for optimistic locking. - to_remove = getattr(obj, field.name).exclude(id__in=ids).select_for_update() - for rel_obj in to_remove: - check_table_permissions(collection, agent, rel_obj, "delete") - auditlog.remove(rel_obj, agent, obj) - - to_remove.delete() + # Delete related objects not in the ids list. + # TODO: Check versions for optimistic locking. + to_remove = getattr(obj, field.name).exclude(id__in=ids).select_for_update() + for rel_obj in to_remove: + check_table_permissions(collection, agent, rel_obj, "delete") + auditlog.remove(rel_obj, agent, obj) + + to_remove.delete() class IndependentInline(TypedDict): update: List[Union[str, Dict[str, Any]]] @@ -686,15 +686,17 @@ def _handle_independent_to_many(collection, agent, obj, field, value: Independen cached_objs = dict() fk_model = None + to_fetch = [*to_update, *to_remove] + # Fetch the related records which are provided as strings - for rel_data in [*to_update, *to_remove]: + for rel_data in to_fetch: if not isinstance(rel_data, str): continue fk_model, fk_id = strict_uri_to_model(rel_data, rel_model.__name__) ids_to_fetch.append(fk_id) if fk_model is not None: cached_objs = {item.id: obj_to_data(item) for item in get_model(fk_model).objects.filter(id__in=ids_to_fetch).select_for_update()} - + for rel_data in to_update: if isinstance(rel_data, str): fk_model, fk_id = strict_uri_to_model(rel_data, rel_model.__name__) @@ -706,12 +708,13 @@ def _handle_independent_to_many(collection, agent, obj, field, value: Independen update_or_create_resource(collection, agent, rel_model, rel_data, None) if len(to_remove) > 0: - check_table_permissions(collection, agent, rel_model, 'update') + assert obj.pk is not None, f"Unable to remove {obj.__class__.__name__}.{field.field.name} resources from new {obj.__class__.__name__}" related_field = datamodel.reverse_relationship(obj.specify_model.get_field_strict(field.name)) assert related_field is not None, f"no reverse relationship for {obj.__class__.__name__}.{field.field.name}" for rel_obj in to_remove: fk_model, fk_id = strict_uri_to_model(rel_obj, rel_model.__name__) rel_data = cached_objs[fk_id] + assert rel_data[related_field.name] == uri_for_model(obj.__class__, obj.pk), f"Related {related_field.relatedModelName} does not belong to {obj.__class__.__name__}.{field.field.name}: {rel_obj}" rel_data[related_field.name] = None update_obj(collection, agent, rel_model, rel_data["id"], rel_data["version"], rel_data) From 794fdee44594aa63ea823a3b4ca3cbeb5cdb8103 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 11 Sep 2024 09:28:12 -0500 Subject: [PATCH 40/66] Don't automatically fetch Independent ToOne Collections when modified --- .../components/FormSliders/RecordSelectorFromCollection.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx index 5d0f0440ec4..2268b1377d3 100644 --- a/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx @@ -5,6 +5,7 @@ import type { RA } from '../../utils/types'; import { defined } from '../../utils/types'; import { DependentCollection, + IndependentCollection, isRelationshipCollection, LazyCollection, } from '../DataModel/collectionApi'; @@ -80,6 +81,7 @@ export function RecordSelectorFromCollection({ * don't need to fetch all records in between) */ if ( + !isToOne && isLazy && collection.related?.isNew() !== true && !collection.isComplete() && @@ -89,7 +91,7 @@ export function RecordSelectorFromCollection({ .fetch() .then(() => setRecords(getRecords)) .catch(raise); - }, [collection, isLazy, getRecords, index, records.length]); + }, [collection, isLazy, getRecords, index, records.length, isToOne]); const state = useRecordSelector({ ...rest, From 872f6019217d6c2a5f7c2a580fc414ae9ade3a08 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 12 Sep 2024 14:37:02 -0500 Subject: [PATCH 41/66] Initialize totalCount variable for lazy collections --- .../js_src/lib/components/DataModel/collectionApi.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts index 0276a11010a..2b177574c84 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts @@ -98,6 +98,8 @@ export const LazyCollection = Base.extend({ constructor(options = {}) { this.table = this.model; Base.call(this, null, options); + this._neverFetched = true; + this._totalCount = undefined; this.filters = options.filters || {}; this.domainfilter = Boolean(options.domainfilter) && @@ -107,7 +109,7 @@ export const LazyCollection = Base.extend({ return `/api/specify/${this.model.specifyTable.name.toLowerCase()}/`; }, isComplete() { - return this.length === this._totalCount; + return this._neverFetched && this.length === this._totalCount; }, parse(resp) { let objects; @@ -172,6 +174,8 @@ export const IndependentCollection = LazyCollection.extend({ Boolean(options.domainfilter) && this.model?.specifyTable.getScopingRelationship() !== undefined; + this._totalCount = records.length; + this.removed = new Set(); this.updated = {}; }, @@ -232,9 +236,7 @@ export const IndependentCollection = LazyCollection.extend({ this._totalCount -= (this.removed as Set).size; - return records.filter( - ({ resource_uri }) => !(this.removed as Set).has(resource_uri) - ); + return records; }, async fetch(options) { if (this.related.isBeingInitialized()) { From 868e76a460a3fd9f7d9da2db1d477d320a751deb Mon Sep 17 00:00:00 2001 From: Caroline D <108160931+CarolineDenis@users.noreply.github.com> Date: Fri, 13 Sep 2024 06:59:48 -0700 Subject: [PATCH 42/66] Remove import + chnage url test --- .../lib/components/DataModel/__tests__/resourceApi.test.ts | 2 +- .../lib/components/FormSliders/RecordSelectorFromCollection.tsx | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts index 62f09a5fac7..57c4118a250 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts @@ -147,7 +147,7 @@ const accessionsResponse = [ ]; overrideAjax( - '/api/specify/accession/?addressofrecord=42&domainfilter=false&limit=0', + '/api/specify/accession/?domainfilter=false&addressofrecord=42&offset=0', { meta: { total_count: 2 }, objects: accessionsResponse, diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx index 2268b1377d3..2bc987ee9cb 100644 --- a/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx @@ -5,7 +5,6 @@ import type { RA } from '../../utils/types'; import { defined } from '../../utils/types'; import { DependentCollection, - IndependentCollection, isRelationshipCollection, LazyCollection, } from '../DataModel/collectionApi'; From 22d057f5bb30366e6c4a57f6c67f1df1e7db7848 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Mon, 16 Sep 2024 10:46:05 -0500 Subject: [PATCH 43/66] Indep-to-many: use Django fieldnames over datamodel field names --- specifyweb/specify/api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/specifyweb/specify/api.py b/specifyweb/specify/api.py index 2d46539f575..56dd8e72cc2 100644 --- a/specifyweb/specify/api.py +++ b/specifyweb/specify/api.py @@ -714,8 +714,8 @@ def _handle_independent_to_many(collection, agent, obj, field, value: Independen for rel_obj in to_remove: fk_model, fk_id = strict_uri_to_model(rel_obj, rel_model.__name__) rel_data = cached_objs[fk_id] - assert rel_data[related_field.name] == uri_for_model(obj.__class__, obj.pk), f"Related {related_field.relatedModelName} does not belong to {obj.__class__.__name__}.{field.field.name}: {rel_obj}" - rel_data[related_field.name] = None + assert rel_data[field.field.name] == uri_for_model(obj.__class__, obj.pk), f"Related {related_field.relatedModelName} does not belong to {obj.__class__.__name__}.{field.field.name}: {rel_obj}" + rel_data[field.field.name] = None update_obj(collection, agent, rel_model, rel_data["id"], rel_data["version"], rel_data) def update_or_create_resource(collection, agent, model, data, parent_obj): From 9ce312e56779ebc64c9070efe5da7a94b5ae0028 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 18 Sep 2024 06:41:25 -0500 Subject: [PATCH 44/66] Define useCollection hook, reset collection on parent save, fix formtable subviews --- .../lib/components/Attachments/index.tsx | 4 +- .../lib/components/DataModel/collection.ts | 5 +- .../lib/components/DataModel/collectionApi.ts | 35 ++- .../lib/components/DataModel/resourceApi.ts | 6 +- .../lib/components/DataModel/specifyTable.ts | 5 +- .../lib/components/FormCells/FormTable.tsx | 6 +- .../FormCells/FormTableCollection.tsx | 8 +- .../FormSliders/IntegratedRecordSelector.tsx | 5 + .../RecordSelectorFromCollection.tsx | 14 +- .../js_src/lib/components/Forms/SubView.tsx | 133 ++--------- .../js_src/lib/hooks/useCollection.tsx | 220 ++++++++++++------ .../lib/hooks/useSerializedCollection.tsx | 89 +++++++ 12 files changed, 316 insertions(+), 214 deletions(-) create mode 100644 specifyweb/frontend/js_src/lib/hooks/useSerializedCollection.tsx diff --git a/specifyweb/frontend/js_src/lib/components/Attachments/index.tsx b/specifyweb/frontend/js_src/lib/components/Attachments/index.tsx index baef46664ac..8090f1ee5d2 100644 --- a/specifyweb/frontend/js_src/lib/components/Attachments/index.tsx +++ b/specifyweb/frontend/js_src/lib/components/Attachments/index.tsx @@ -7,7 +7,7 @@ import { useNavigate } from 'react-router-dom'; import { useAsyncState, usePromise } from '../../hooks/useAsyncState'; import { useCachedState } from '../../hooks/useCachedState'; -import { useCollection } from '../../hooks/useCollection'; +import { useSerializedCollection } from '../../hooks/useSerializedCollection'; import { attachmentsText } from '../../localization/attachments'; import { commonText } from '../../localization/common'; import { schemaText } from '../../localization/schema'; @@ -125,7 +125,7 @@ function Attachments({ 'scale' ); - const [collection, setCollection, fetchMore] = useCollection( + const [collection, setCollection, fetchMore] = useSerializedCollection( React.useCallback( async (offset) => fetchCollection( diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collection.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collection.ts index f6a2e2d3e4f..e7c7d56b1cf 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collection.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collection.ts @@ -23,9 +23,10 @@ export type CollectionFetchFilters = Partial< number > > & { - readonly limit: number; + readonly limit?: number; + readonly reset?: boolean; readonly offset?: number; - readonly domainFilter: boolean; + readonly domainFilter?: boolean; readonly orderBy?: | keyof CommonFields | keyof SCHEMA['fields'] diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts index 2b177574c84..db9daab6164 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts @@ -6,6 +6,7 @@ import { removeKey } from '../../utils/utils'; import { assert } from '../Errors/assert'; import { Backbone } from './backbone'; import type { AnySchema } from './helperTypes'; +import { DEFAULT_FETCH_LIMIT } from './collection'; import type { SpecifyResource } from './legacyTypes'; // REFACTOR: remove @ts-nocheck @@ -135,15 +136,15 @@ export const LazyCollection = Base.extend({ options ||= {}; - options.update = true; - options.remove = false; + options.update ??= true; + options.remove ??= false; options.silent = true; assert(options.at == null); options.data = options.data || _.extend({ domainfilter: this.domainfilter }, this.filters); - options.data.offset = options.offset || this.length; + options.data.offset = options.offset ?? this.length; _(options).has('limit') && (options.data.limit = options.limit); this._fetch = Backbone.Collection.prototype.fetch.call(this, options); @@ -157,6 +158,9 @@ export const LazyCollection = Base.extend({ ? this.fetch() : this; }, + getFetchOffset() { + return this.length; + }, getTotalCount() { if (_.isNumber(this._totalCount)) return Promise.resolve(this._totalCount); return this.fetchIfNotPopulated().then((_this) => _this._totalCount); @@ -236,22 +240,29 @@ export const IndependentCollection = LazyCollection.extend({ this._totalCount -= (this.removed as Set).size; - return records; + return records.filter( + ({ resource_uri }) => !(this.removed as Set).has(resource_uri) + ); }, async fetch(options) { - if (this.related.isBeingInitialized()) { + // If the related is being fetched, don't try and fetch the collection + if (this.related._fetch !== null) { return this; } this.filters[this.field.name.toLowerCase()] = this.related.id; - const offset = - this.length === 0 && this.removed.size > 0 - ? this.removed.size - : this.length; - - options = { ...(options ?? {}), silent: true, offset }; + const newOptions = { + ...options, + update: options?.reset !== true, + offset: options?.offset ?? this.getFetchOffset(), + }; - return Reflect.apply(LazyCollection.prototype.fetch, this, [options]); + return Reflect.apply(LazyCollection.prototype.fetch, this, [newOptions]); + }, + getFetchOffset() { + return this.length === 0 && this.removed.size > 0 + ? this.removed.size + : Math.floor(this.length / DEFAULT_FETCH_LIMIT) * DEFAULT_FETCH_LIMIT; }, toApiJSON(options) { const self = this; diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts index f45fc1340e9..ac59de6eee5 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts @@ -680,12 +680,16 @@ export const ResourceBase = Backbone.Model.extend({ if (!value) return value; // No related object // Is the related resource cached? - let toOne = this.dependentResources[fieldName]; + let toOne = + this.dependentResources[fieldName] ?? + this.independentResources[fieldName]; + if (!toOne) { _(value).isString() || softFail('expected URI, got', value); toOne = resourceFromUrl(value, { noBusinessRules: options.noBusinessRules, }); + if (field.isDependent()) { console.warn('expected dependent resource to be in cache'); this.storeDependent(field, toOne); diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts b/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts index 1eede1b49e0..f18a0c98693 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts @@ -14,6 +14,7 @@ import { error } from '../Errors/assert'; import { attachmentView } from '../FormParse/webOnlyViews'; import { parentTableRelationship } from '../Forms/parentTables'; import { relationshipIsToMany } from '../WbPlanView/mappingHelpers'; +import { CollectionFetchFilters } from './collection'; import { DependentCollection, IndependentCollection, @@ -106,7 +107,9 @@ export type Collection = { toJSON>(): RA; add(resource: RA> | SpecifyResource): void; remove(resource: SpecifyResource): void; - fetch(filter?: { readonly limit: number }): Promise>; + fetch( + filters?: CollectionFetchFilters + ): Promise>; trigger(eventName: string): void; on(eventName: string, callback: (...args: RA) => void): void; once(eventName: string, callback: (...args: RA) => void): void; diff --git a/specifyweb/frontend/js_src/lib/components/FormCells/FormTable.tsx b/specifyweb/frontend/js_src/lib/components/FormCells/FormTable.tsx index d6173ee3495..03281b94406 100644 --- a/specifyweb/frontend/js_src/lib/components/FormCells/FormTable.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormCells/FormTable.tsx @@ -208,7 +208,10 @@ export function FormTable({ ) : resources.length === 0 ? (

{formsText.noData()}

) : ( -
+
({ maxHeight: `${maxHeight}px`, }} viewDefinition={collapsedViewDefinition} - onScroll={handleScroll} >
[0], @@ -21,13 +22,14 @@ export function FormTableCollection({ readonly onDelete: | ((resource: SpecifyResource, index: number) => void) | undefined; + readonly onFetch?: () => void; }): JSX.Element | null { const [records, setRecords] = React.useState(Array.from(collection.models)); React.useEffect( () => resourceOn( collection, - 'add remove sort', + 'add remove sort sync', () => setRecords(Array.from(collection.models)), true ), @@ -35,9 +37,9 @@ export function FormTableCollection({ ); const handleFetchMore = React.useCallback(async () => { - await collection.fetch(); + handleFetch?.() ?? collection.fetch(); setRecords(Array.from(collection.models)); - }, [collection]); + }, [collection, handleFetch]); const isDependent = collection instanceof DependentCollection; const relationship = collection.field?.getReverse(); diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx index 36e62af0487..3c541dc10c3 100644 --- a/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx @@ -9,6 +9,7 @@ import type { RA } from '../../utils/types'; import { Button } from '../Atoms/Button'; import { DataEntry } from '../Atoms/DataEntry'; import { ReadOnlyContext } from '../Core/Contexts'; +import type { CollectionFetchFilters } from '../DataModel/collection'; import { DependentCollection } from '../DataModel/collectionApi'; import type { AnyInteractionPreparation, @@ -43,6 +44,7 @@ export function IntegratedRecordSelector({ onClose: handleClose, onAdd: handleAdd, onDelete: handleDelete, + onFetch: handleFetch, isCollapsed: defaultCollapsed, ...rest }: Omit< @@ -54,6 +56,7 @@ export function IntegratedRecordSelector({ readonly viewName?: string; readonly urlParameter?: string; readonly onClose: () => void; + readonly onFetch?: (filters?: CollectionFetchFilters) => void; readonly sortField: SubViewSortField | undefined; }): JSX.Element { const containerRef = React.useRef(null); @@ -147,6 +150,7 @@ export function IntegratedRecordSelector({ if (isCollapsed) handleExpand(); handleDelete?.(...args); }} + onFetch={handleFetch} onSlide={(index): void => { handleExpand(); if (typeof urlParameter === 'string') setIndex(index.toString()); @@ -307,6 +311,7 @@ export function IntegratedRecordSelector({ if (isCollapsed) handleExpand(); handleDelete?.(index, 'minusButton'); }} + onFetch={handleFetch} /> ) : null} {dialogs} diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx index 2bc987ee9cb..0561b75bc6b 100644 --- a/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx @@ -3,6 +3,7 @@ import React from 'react'; import { useTriggerState } from '../../hooks/useTriggerState'; import type { RA } from '../../utils/types'; import { defined } from '../../utils/types'; +import type { CollectionFetchFilters } from '../DataModel/collection'; import { DependentCollection, isRelationshipCollection, @@ -13,7 +14,6 @@ import type { SpecifyResource } from '../DataModel/legacyTypes'; import { resourceOn } from '../DataModel/resource'; import type { Relationship } from '../DataModel/specifyField'; import type { Collection } from '../DataModel/specifyTable'; -import { raise } from '../Errors/Crash'; import { relationshipIsToMany } from '../WbPlanView/mappingHelpers'; import type { RecordSelectorProps, @@ -27,6 +27,7 @@ export function RecordSelectorFromCollection({ onAdd: handleAdd, onDelete: handleDelete, onSlide: handleSlide, + onFetch: handleFetch, children, defaultIndex = 0, ...rest @@ -45,6 +46,7 @@ export function RecordSelectorFromCollection({ readonly relationship: Relationship; readonly defaultIndex?: number; readonly children: (state: RecordSelectorState) => JSX.Element; + readonly onFetch?: (filters?: CollectionFetchFilters) => void; }): JSX.Element | null { const getRecords = React.useCallback( (): RA | undefined> => @@ -80,17 +82,17 @@ export function RecordSelectorFromCollection({ * don't need to fetch all records in between) */ if ( + typeof handleFetch === 'function' && !isToOne && isLazy && collection.related?.isNew() !== true && !collection.isComplete() && collection.models[index] === undefined ) - collection - .fetch() - .then(() => setRecords(getRecords)) - .catch(raise); - }, [collection, isLazy, getRecords, index, records.length, isToOne]); + handleFetch({ + offset: collection.getFetchOffset(), + }); + }, [collection, isLazy, index, records.length, isToOne, handleFetch]); const state = useRecordSelector({ ...rest, diff --git a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx index 22279228fd6..a893969763b 100644 --- a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx +++ b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx @@ -2,10 +2,10 @@ import React from 'react'; import { usePromise } from '../../hooks/useAsyncState'; import { useBooleanState } from '../../hooks/useBooleanState'; +import { useCollection } from '../../hooks/useCollection'; import { useTriggerState } from '../../hooks/useTriggerState'; import { commonText } from '../../localization/common'; import type { RA } from '../../utils/types'; -import { overwriteReadOnly } from '../../utils/types'; import { sortFunction } from '../../utils/utils'; import { Button } from '../Atoms/Button'; import { DataEntry } from '../Atoms/DataEntry'; @@ -16,8 +16,6 @@ import type { AnySchema } from '../DataModel/helperTypes'; import type { SpecifyResource } from '../DataModel/legacyTypes'; import { resourceOn } from '../DataModel/resource'; import type { Relationship } from '../DataModel/specifyField'; -import type { Collection } from '../DataModel/specifyTable'; -import { raise, softFail } from '../Errors/Crash'; import type { FormType } from '../FormParse'; import type { SubViewSortField } from '../FormParse/cells'; import { IntegratedRecordSelector } from '../FormSliders/IntegratedRecordSelector'; @@ -69,122 +67,36 @@ export function SubView({ }): JSX.Element { const [sortField, setSortField] = useTriggerState(initialSortField); - const fetchCollection = React.useCallback( - // If false is returned, then the Subview should not be rendered - async function fetchCollection(): Promise< - Collection | false | undefined - > { - if ( - relationshipIsToMany(relationship) && - relationship.type !== 'zero-to-one' - ) - return parentResource - .rgetCollection(relationship.name) - .then((collection) => { - // TEST: check if this can ever happen - if (collection === null || collection === undefined) - return new relationship.relatedTable.DependentCollection({ - related: parentResource, - field: relationship.getReverse(), - }) as Collection; - if (sortField === undefined) return collection; - // BUG: this does not look into related tables - const field = sortField.fieldNames[0]; - // Overwriting the tables on the collection - overwriteReadOnly( - collection, - 'models', - Array.from(collection.models).sort( - sortFunction( - (resource) => resource.get(field), - sortField.direction === 'desc' - ) - ) - ); - return collection; - }); - else { - /** - * If relationship is -to-one, create a collection for the related - * resource. This allows to reuse most of the code from the -to-many - * relationships. RecordSelector handles collections with -to-one - * related field by removing the "+" button after first record is added - * and not rendering record count or record slider. - */ - const resource = await parentResource.rgetPromise(relationship.name); - const reverse = relationship.getReverse(); - if (reverse === undefined) { - softFail( - new Error( - `Can't render a SubView for ` + - `${relationship.table.name}.${relationship.name} because ` + - `reverse relationship does not exist` - ) - ); - return false; - } - const collection = ( - relationship.isDependent() - ? new relationship.relatedTable.DependentCollection({ - related: parentResource, - field: reverse, - }) - : new relationship.relatedTable.IndependentCollection({ - related: parentResource, - field: reverse, - }) - ) as Collection; - if (relationship.isDependent() && parentResource.isNew()) - // Prevent fetching related for newly created parent - overwriteReadOnly(collection, '_totalCount', 0); - - if (typeof resource === 'object' && resource !== null) - collection.add(resource); - overwriteReadOnly( - collection, - 'related', - collection.related ?? parentResource - ); - overwriteReadOnly( - collection, - 'field', - collection.field ?? relationship.getReverse() - ); - return collection; - } - }, - [parentResource, relationship, sortField] - ); + const [collection, _setCollection, handleFetch] = useCollection({ + parentResource, + relationship, + collectionSortFunction: + sortField === undefined + ? undefined + : sortFunction( + (resource) => resource.get(sortField.fieldNames[0]), + sortField.direction === 'desc' + ), + }); - const [collection, setCollection] = React.useState< - Collection | false | undefined - >(undefined); - const versionRef = React.useRef(0); React.useEffect( () => resourceOn( parentResource, - `change:${relationship.name} saved`, + 'saved', (): void => { - versionRef.current += 1; - const localVersionRef = versionRef.current; - fetchCollection() - .then((collection) => - /* - * If value changed since begun fetching, don't update the - * collection to prevent a race condition. - * REFACTOR: simplify this - */ - versionRef.current === localVersionRef - ? setCollection(collection) - : undefined - ) - .catch(raise); + if (!relationship.isDependent()) { + handleFetch({ + offset: 0, + reset: true, + }); + } }, - true + false ), - [parentResource, relationship, fetchCollection] + [parentResource, relationship, handleFetch] ); + const subviewContext = React.useContext(SubViewContext); const [formType, setFormType] = useTriggerState(initialFormType); @@ -298,6 +210,7 @@ export function SubView({ null as never ) } + onFetch={handleFetch} /> ) : isButton ? undefined : ( diff --git a/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx b/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx index 1e492a3f446..ebd6040a8d5 100644 --- a/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx +++ b/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx @@ -1,89 +1,159 @@ import React from 'react'; -import type { SerializedCollection } from '../components/DataModel/collection'; +import type { CollectionFetchFilters } from '../components/DataModel/collection'; import type { AnySchema } from '../components/DataModel/helperTypes'; -import { f } from '../utils/functools'; +import type { SpecifyResource } from '../components/DataModel/legacyTypes'; +import type { Relationship } from '../components/DataModel/specifyField'; +import type { Collection } from '../components/DataModel/specifyTable'; +import { raise } from '../components/Errors/Crash'; +import { relationshipIsToMany } from '../components/WbPlanView/mappingHelpers'; import type { GetOrSet } from '../utils/types'; -import { defined } from '../utils/types'; +import { overwriteReadOnly } from '../utils/types'; +import type { sortFunction } from '../utils/utils'; import { useAsyncState } from './useAsyncState'; -/** - * A hook for fetching a collection of resources in a paginated way - */ -export function useCollection( - fetch: (offset: number) => Promise> -): readonly [ - SerializedCollection | undefined, - GetOrSet | undefined>[1], - () => Promise +type UseCollectionProps = { + readonly parentResource: SpecifyResource; + readonly relationship: Relationship; + readonly collectionSortFunction?: ReturnType< + typeof sortFunction< + SpecifyResource, + ReturnType['get']> + > + >; +}; + +export function useCollection({ + parentResource, + relationship, + collectionSortFunction, +}: UseCollectionProps): readonly [ + ...GetOrSet | false | undefined>, + (filters?: CollectionFetchFilters) => void ] { - const fetchRef = React.useRef< - Promise | undefined> | undefined - >(undefined); + const [collection, setCollection] = useAsyncState< + Collection | false | undefined + >( + React.useCallback( + async () => + relationshipIsToMany(relationship) && + relationship.type !== 'zero-to-one' + ? fetchToManyCollection({ + parentResource, + relationship, + collectionSortFunction, + }) + : fetchToOneCollection({ + parentResource, + relationship, + collectionSortFunction, + }), + [collectionSortFunction, parentResource, relationship] + ), + false + ); - const callback = React.useCallback(async () => { - if (typeof fetchRef.current === 'object') - return fetchRef.current.then(f.undefined); - if ( - collectionRef.current !== undefined && - collectionRef.current?.records.length === - collectionRef.current?.totalCount - ) - return undefined; - fetchRef.current = fetch(collectionRef.current?.records.length ?? 0).then( - (data) => { - fetchRef.current = undefined; - return data; - } - ); - return fetchRef.current; - }, [fetch]); + const versionRef = React.useRef(0); - const currentCallback = React.useRef(f.void); + const handleFetch = React.useCallback( + (filters?: CollectionFetchFilters): void => { + if (typeof collection !== 'object') return undefined; - const [collection, setCollection] = useAsyncState( - React.useCallback(async () => { - currentCallback.current = callback; - fetchRef.current = undefined; - collectionRef.current = undefined; - return callback(); - }, [callback]), - false - ); - const collectionRef = React.useRef< - SerializedCollection | undefined - >(); - collectionRef.current = collection; + versionRef.current += 1; + const localVersionRef = versionRef.current; - const fetchMore = React.useCallback( - async () => - /* - * Ignore calls to fetchMore before collection is fetched for the first - * time - */ - currentCallback.current === callback - ? typeof fetchRef.current === 'object' - ? callback().then(f.undefined) - : callback().then((result) => - result !== undefined && - result.records.length > 0 && - // If the fetch function changed while fetching, discard the results - currentCallback.current === callback - ? setCollection((collection) => ({ - records: [ - ...defined( - collection, - 'Try to fetch more before collection is fetch.' - ).records, - ...result.records, - ], - totalCount: defined(collection).totalCount, - })) - : undefined - ) - : undefined, - [callback, collection] + collection + .fetch(filters) + .then((collection) => { + if (collection === undefined) return undefined; + /* + * If the collection is already being fetched, don't update it + * to prevent a race condition. + * REFACTOR: simplify this + */ + versionRef.current === localVersionRef + ? setCollection(collection) + : undefined; + }) + .catch(raise); + }, + [collection, setCollection] ); + return [collection, setCollection, handleFetch]; +} - return [collection, setCollection, fetchMore] as const; +const fetchToManyCollection = async ({ + parentResource, + relationship, + collectionSortFunction, +}: UseCollectionProps): Promise | undefined> => + parentResource.rgetCollection(relationship.name).then((collection) => { + // TEST: check if this can ever happen + if (collection === null || collection === undefined) + return new relationship.relatedTable.DependentCollection({ + related: parentResource, + field: relationship.getReverse(), + }) as Collection; + if (collectionSortFunction === undefined) return collection; + + // Overwriting the models on the collection + overwriteReadOnly( + collection, + 'models', + Array.from(collection.models).sort(collectionSortFunction) + ); + return collection; + }); + +async function fetchToOneCollection({ + parentResource, + relationship, + collectionSortFunction, +}: UseCollectionProps): Promise< + Collection | false | undefined +> { + /** + * If relationship is -to-one, create a collection for the related + * resource. This allows to reuse most of the code from the -to-many + * relationships. RecordSelector handles collections with -to-one + * related field by removing the "+" button after first record is added + * and not rendering record count or record slider. + */ + const resource = await parentResource.rgetPromise(relationship.name); + const reverse = relationship.getReverse(); + if (reverse === undefined) return false; + const collection = ( + relationship.isDependent() + ? new relationship.relatedTable.DependentCollection({ + related: parentResource, + field: reverse, + }) + : new relationship.relatedTable.IndependentCollection({ + related: parentResource, + field: reverse, + }) + ) as Collection; + if (relationship.isDependent() && parentResource.isNew()) + // Prevent fetching related for newly created parent + overwriteReadOnly(collection, '_totalCount', 0); + + if (typeof resource === 'object' && resource !== null) + collection.add(resource); + overwriteReadOnly( + collection, + 'related', + collection.related ?? parentResource + ); + overwriteReadOnly( + collection, + 'field', + collection.field ?? relationship.getReverse() + ); + if (collectionSortFunction !== undefined) + overwriteReadOnly( + collection, + 'models', + Array.from(collection.models).sort(collectionSortFunction) + ); + return collection; } diff --git a/specifyweb/frontend/js_src/lib/hooks/useSerializedCollection.tsx b/specifyweb/frontend/js_src/lib/hooks/useSerializedCollection.tsx new file mode 100644 index 00000000000..1cc18d7c584 --- /dev/null +++ b/specifyweb/frontend/js_src/lib/hooks/useSerializedCollection.tsx @@ -0,0 +1,89 @@ +import React from 'react'; + +import type { SerializedCollection } from '../components/DataModel/collection'; +import type { AnySchema } from '../components/DataModel/helperTypes'; +import { f } from '../utils/functools'; +import type { GetOrSet } from '../utils/types'; +import { defined } from '../utils/types'; +import { useAsyncState } from './useAsyncState'; + +/** + * A hook for fetching a collection of resources in a paginated way + */ +export function useSerializedCollection( + fetch: (offset: number) => Promise> +): readonly [ + SerializedCollection | undefined, + GetOrSet | undefined>[1], + () => Promise +] { + const fetchRef = React.useRef< + Promise | undefined> | undefined + >(undefined); + + const callback = React.useCallback(async () => { + if (typeof fetchRef.current === 'object') + return fetchRef.current.then(f.undefined); + if ( + collectionRef.current !== undefined && + collectionRef.current?.records.length === + collectionRef.current?.totalCount + ) + return undefined; + fetchRef.current = fetch(collectionRef.current?.records.length ?? 0).then( + (data) => { + fetchRef.current = undefined; + return data; + } + ); + return fetchRef.current; + }, [fetch]); + + const currentCallback = React.useRef(f.void); + + const [collection, setCollection] = useAsyncState( + React.useCallback(async () => { + currentCallback.current = callback; + fetchRef.current = undefined; + collectionRef.current = undefined; + return callback(); + }, [callback]), + false + ); + const collectionRef = React.useRef< + SerializedCollection | undefined + >(); + collectionRef.current = collection; + + const fetchMore = React.useCallback( + async () => + /* + * Ignore calls to fetchMore before collection is fetched for the first + * time + */ + currentCallback.current === callback + ? typeof fetchRef.current === 'object' + ? callback().then(f.undefined) + : callback().then((result) => + result !== undefined && + result.records.length > 0 && + // If the fetch function changed while fetching, discard the results + currentCallback.current === callback + ? setCollection((collection) => ({ + records: [ + ...defined( + collection, + 'Try to fetch more before collection is fetch.' + ).records, + ...result.records, + ], + totalCount: defined(collection).totalCount, + })) + : undefined + ) + : undefined, + [callback, collection] + ); + + return [collection, setCollection, fetchMore] as const; +} From 134c65fde55192925d264b920d5c012f286a60ec Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 18 Sep 2024 06:55:58 -0500 Subject: [PATCH 45/66] Resolve TS errors --- .../js_src/lib/components/DataModel/collectionApi.ts | 3 +++ .../js_src/lib/components/DataModel/specifyTable.ts | 1 + specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx | 3 ++- specifyweb/frontend/js_src/lib/hooks/useCollection.tsx | 6 +++--- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts index db9daab6164..14d5d680f07 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts @@ -88,6 +88,9 @@ export const DependentCollection = Base.extend({ isComplete() { return true; }, + getFetchOffset() { + return 0; + }, fetch: fakeFetch, sync: notSupported, create: notSupported, diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts b/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts index f18a0c98693..4995661d1a6 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts @@ -102,6 +102,7 @@ export type Collection = { /* eslint-disable @typescript-eslint/method-signature-style */ isComplete(): boolean; getTotalCount(): Promise; + getFetchOffset(): number; indexOf(resource: SpecifyResource): number; // eslint-disable-next-line @typescript-eslint/naming-convention toJSON>(): RA; diff --git a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx index a893969763b..5c04c60475c 100644 --- a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx +++ b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx @@ -12,6 +12,7 @@ import { DataEntry } from '../Atoms/DataEntry'; import { attachmentSettingsPromise } from '../Attachments/attachments'; import { attachmentRelatedTables } from '../Attachments/utils'; import { ReadOnlyContext } from '../Core/Contexts'; +import type { CollectionFetchFilters } from '../DataModel/collection'; import type { AnySchema } from '../DataModel/helperTypes'; import type { SpecifyResource } from '../DataModel/legacyTypes'; import { resourceOn } from '../DataModel/resource'; @@ -89,7 +90,7 @@ export function SubView({ handleFetch({ offset: 0, reset: true, - }); + } as CollectionFetchFilters); } }, false diff --git a/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx b/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx index ebd6040a8d5..f9fd04703a6 100644 --- a/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx +++ b/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx @@ -63,7 +63,7 @@ export function useCollection({ const localVersionRef = versionRef.current; collection - .fetch(filters) + .fetch(filters as CollectionFetchFilters) .then((collection) => { if (collection === undefined) return undefined; /* @@ -71,8 +71,8 @@ export function useCollection({ * to prevent a race condition. * REFACTOR: simplify this */ - versionRef.current === localVersionRef - ? setCollection(collection) + return versionRef.current === localVersionRef + ? void setCollection(collection) : undefined; }) .catch(raise); From dd61235de05fecf4e5694833474a07e2e7b1e78c Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 18 Sep 2024 07:21:11 -0500 Subject: [PATCH 46/66] Resolve backend tests, update URI for frontend format test --- .../lib/components/DataModel/collectionApi.ts | 2 +- specifyweb/specify/tests/test_api.py | 24 ++++--------------- 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts index 14d5d680f07..01f3dd18884 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts @@ -113,7 +113,7 @@ export const LazyCollection = Base.extend({ return `/api/specify/${this.model.specifyTable.name.toLowerCase()}/`; }, isComplete() { - return this._neverFetched && this.length === this._totalCount; + return !this._neverFetched && this.length === this._totalCount; }, parse(resp) { let objects; diff --git a/specifyweb/specify/tests/test_api.py b/specifyweb/specify/tests/test_api.py index 55e6ef677a9..9480581cff1 100644 --- a/specifyweb/specify/tests/test_api.py +++ b/specifyweb/specify/tests/test_api.py @@ -535,7 +535,9 @@ def test_indepenent_to_many_removing_from_inline(self): self.assertEqual(accession, self.collectionobjects[0].accession) - collection_objects_to_set = [self.collectionobjects[0], self.collectionobjects[3]] + collection_objects_to_remove = [self.collectionobjects[0], self.collectionobjects[3]] + + cos_to_keep = [collection_object for collection_object in self.collectionobjects if not collection_object in collection_objects_to_remove] accession_data = { 'accessionnumber': "a", @@ -543,14 +545,13 @@ def test_indepenent_to_many_removing_from_inline(self): 'collectionobjects': { "remove": [ api.uri_for_model('collectionobject', collection_object.id) - for index, collection_object in enumerate(collection_objects_to_set) - if index % 2 == 0 + for collection_object in collection_objects_to_remove ] } } accession = api.update_obj(self.collection, self.agent, 'Accession', accession.id, accession.version, accession_data) - self.assertEqual(list(accession.collectionobjects.all()), collection_objects_to_set) + self.assertEqual(list(accession.collectionobjects.all()), cos_to_keep) # ensure the other CollectionObjects have not been deleted self.assertEqual(len(models.Collectionobject.objects.all()), len(self.collectionobjects)) @@ -657,21 +658,6 @@ def test_reassigning_independent_to_many(self): self.assertEqual(self.collectionobjects[0].accession, acc2) self.assertEqual(self.collectionobjects[1].accession, acc2) - def test_inline_error_handling(self): - collection_object_data = { - 'id': self.collectionobjects[0].id, - 'catalognumber': self.collectionobjects[0].catalognumber, - 'collection': api.uri_for_model('Collection', self.collection.id), - 'determinations': f'/api/specify/determination/?collectionobject={self.collectionobjects[0].id}' - } - - with self.assertRaises(AssertionError): - api.update_obj(self.collection, self.agent, - 'Collectionobject', self.collectionobjects[0].id, - self.collectionobjects[0].version, collection_object_data) - - - # version control on inlined resources should be tested From d512e333358f649de9c67bfd1b7b4efa3a1c87b8 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 18 Sep 2024 08:03:10 -0500 Subject: [PATCH 47/66] Change ordering for setting of _neverFetched --- .../js_src/lib/components/DataModel/collectionApi.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts index 01f3dd18884..870bc7cfdf0 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts @@ -102,7 +102,6 @@ export const LazyCollection = Base.extend({ constructor(options = {}) { this.table = this.model; Base.call(this, null, options); - this._neverFetched = true; this._totalCount = undefined; this.filters = options.filters || {}; this.domainfilter = @@ -129,11 +128,11 @@ export const LazyCollection = Base.extend({ return objects; }, async fetch(options) { - this._neverFetched = false; - if (this._fetch) return this._fetch; else if (this.isComplete() || this.related?.isNew()) return this; + this._neverFetched = false; + if (this.isComplete()) console.error('fetching for already filled collection'); @@ -249,9 +248,8 @@ export const IndependentCollection = LazyCollection.extend({ }, async fetch(options) { // If the related is being fetched, don't try and fetch the collection - if (this.related._fetch !== null) { - return this; - } + if (this.related._fetch !== null) return this; + this.filters[this.field.name.toLowerCase()] = this.related.id; const newOptions = { From 473ea5c41f91cb304fed729133d227afa5ddd2d6 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 18 Sep 2024 09:16:46 -0500 Subject: [PATCH 48/66] Improve orderby resolution for to-many subviews --- .../lib/components/DataModel/collectionApi.ts | 1 + .../js_src/lib/components/Forms/SubView.tsx | 9 +--- .../js_src/lib/hooks/useCollection.tsx | 46 ++++++++++++------- 3 files changed, 31 insertions(+), 25 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts index 870bc7cfdf0..2d68b97eada 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts @@ -147,6 +147,7 @@ export const LazyCollection = Base.extend({ options.data || _.extend({ domainfilter: this.domainfilter }, this.filters); options.data.offset = options.offset ?? this.length; + options.data.orderby = options.orderby; _(options).has('limit') && (options.data.limit = options.limit); this._fetch = Backbone.Collection.prototype.fetch.call(this, options); diff --git a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx index 5c04c60475c..54b674005e4 100644 --- a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx +++ b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx @@ -6,7 +6,6 @@ import { useCollection } from '../../hooks/useCollection'; import { useTriggerState } from '../../hooks/useTriggerState'; import { commonText } from '../../localization/common'; import type { RA } from '../../utils/types'; -import { sortFunction } from '../../utils/utils'; import { Button } from '../Atoms/Button'; import { DataEntry } from '../Atoms/DataEntry'; import { attachmentSettingsPromise } from '../Attachments/attachments'; @@ -71,13 +70,7 @@ export function SubView({ const [collection, _setCollection, handleFetch] = useCollection({ parentResource, relationship, - collectionSortFunction: - sortField === undefined - ? undefined - : sortFunction( - (resource) => resource.get(sortField.fieldNames[0]), - sortField.direction === 'desc' - ), + sortBy: sortField, }); React.useEffect( diff --git a/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx b/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx index f9fd04703a6..0e41415580f 100644 --- a/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx +++ b/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx @@ -6,27 +6,23 @@ import type { SpecifyResource } from '../components/DataModel/legacyTypes'; import type { Relationship } from '../components/DataModel/specifyField'; import type { Collection } from '../components/DataModel/specifyTable'; import { raise } from '../components/Errors/Crash'; +import type { SubViewSortField } from '../components/FormParse/cells'; import { relationshipIsToMany } from '../components/WbPlanView/mappingHelpers'; import type { GetOrSet } from '../utils/types'; import { overwriteReadOnly } from '../utils/types'; -import type { sortFunction } from '../utils/utils'; +import { sortFunction } from '../utils/utils'; import { useAsyncState } from './useAsyncState'; type UseCollectionProps = { readonly parentResource: SpecifyResource; readonly relationship: Relationship; - readonly collectionSortFunction?: ReturnType< - typeof sortFunction< - SpecifyResource, - ReturnType['get']> - > - >; + readonly sortBy?: SubViewSortField; }; export function useCollection({ parentResource, relationship, - collectionSortFunction, + sortBy, }: UseCollectionProps): readonly [ ...GetOrSet | false | undefined>, (filters?: CollectionFetchFilters) => void @@ -41,14 +37,14 @@ export function useCollection({ ? fetchToManyCollection({ parentResource, relationship, - collectionSortFunction, + sortBy, }) : fetchToOneCollection({ parentResource, relationship, - collectionSortFunction, + sortBy, }), - [collectionSortFunction, parentResource, relationship] + [sortBy, parentResource, relationship] ), false ); @@ -85,7 +81,7 @@ export function useCollection({ const fetchToManyCollection = async ({ parentResource, relationship, - collectionSortFunction, + sortBy, }: UseCollectionProps): Promise | undefined> => parentResource.rgetCollection(relationship.name).then((collection) => { // TEST: check if this can ever happen @@ -94,13 +90,21 @@ const fetchToManyCollection = async ({ related: parentResource, field: relationship.getReverse(), }) as Collection; - if (collectionSortFunction === undefined) return collection; + if (sortBy === undefined) return collection; + + // BUG: this does not look into related tables + const field = sortBy.fieldNames[0]; // Overwriting the models on the collection overwriteReadOnly( collection, 'models', - Array.from(collection.models).sort(collectionSortFunction) + Array.from(collection.models).sort( + sortFunction( + (resource) => resource.get(field), + sortBy.direction === 'desc' + ) + ) ); return collection; }); @@ -108,7 +112,7 @@ const fetchToManyCollection = async ({ async function fetchToOneCollection({ parentResource, relationship, - collectionSortFunction, + sortBy, }: UseCollectionProps): Promise< Collection | false | undefined > { @@ -149,11 +153,19 @@ async function fetchToOneCollection({ 'field', collection.field ?? relationship.getReverse() ); - if (collectionSortFunction !== undefined) + if (sortBy !== undefined) { + // BUG: this does not look into related tables + const field = sortBy.fieldNames[0]; overwriteReadOnly( collection, 'models', - Array.from(collection.models).sort(collectionSortFunction) + Array.from(collection.models).sort( + sortFunction( + (resource) => resource.get(field), + sortBy.direction === 'desc' + ) + ) ); + } return collection; } From cf84978f7b31ecd612c60360eead14b594262b6b Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 18 Sep 2024 09:37:54 -0500 Subject: [PATCH 49/66] Resolving failing frontend test --- .../frontend/js_src/lib/components/DataModel/collectionApi.ts | 4 ++-- .../lib/components/Formatters/__tests__/formatters.test.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts index 2d68b97eada..aaf3a8b9828 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts @@ -131,11 +131,11 @@ export const LazyCollection = Base.extend({ if (this._fetch) return this._fetch; else if (this.isComplete() || this.related?.isNew()) return this; - this._neverFetched = false; - if (this.isComplete()) console.error('fetching for already filled collection'); + this._neverFetched = false; + options ||= {}; options.update ??= true; diff --git a/specifyweb/frontend/js_src/lib/components/Formatters/__tests__/formatters.test.ts b/specifyweb/frontend/js_src/lib/components/Formatters/__tests__/formatters.test.ts index 9b2f8fbd7df..ee14ccd2fc3 100644 --- a/specifyweb/frontend/js_src/lib/components/Formatters/__tests__/formatters.test.ts +++ b/specifyweb/frontend/js_src/lib/components/Formatters/__tests__/formatters.test.ts @@ -120,7 +120,7 @@ const taxonCitation = { referenceWork: getResourceApiUrl('ReferenceWork', referenceWorkId), }; overrideAjax( - '/api/specify/taxoncitation/?referencework=1&domainfilter=false&limit=0', + '/api/specify/taxoncitation/?domainfilter=false&referencework=1&offset=0', { meta: { total_count: 1, From 33515c4c719c9e08c85b27c007390b4f613ff1d9 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Tue, 24 Sep 2024 14:53:37 -0500 Subject: [PATCH 50/66] Prevent storing independent toMany if fetch not successful --- .../js_src/lib/components/DataModel/resourceApi.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts index ac59de6eee5..528dc184a2d 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts @@ -338,7 +338,7 @@ export const ResourceBase = Backbone.Model.extend({ } default: { throw new Error( - `setDependentToOne: unhandled field type: ${field.type}` + `storeIndependentToOne: unhandled field type: ${field.type} for ${this.specifyTable.name}.${field.name}` ); } } @@ -810,9 +810,11 @@ export const ResourceBase = Backbone.Model.extend({ ? new relatedTable.IndependentCollection(collectionOptions) : existingToMany; - return collection.fetch().then((fetchedCollection) => { - this.storeIndependent(field, fetchedCollection); - return fetchedCollection; + return collection.fetch({ + // Only store the collection if fetch is successful (doesn't return undefined) + success: (collection) => { + this.storeIndependent(field, collection); + }, }); }, async save({ From be03bcd80fa2a8196919262a11237636894ae6c7 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Fri, 27 Sep 2024 10:36:43 -0500 Subject: [PATCH 51/66] Set independent toOne on resource change --- .../lib/components/DataModel/collection.ts | 2 ++ .../lib/components/DataModel/collectionApi.ts | 12 ++++++++-- .../lib/components/DataModel/resourceApi.ts | 3 ++- .../js_src/lib/hooks/useCollection.tsx | 24 +++++++++---------- 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collection.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collection.ts index e7c7d56b1cf..0c5b516245d 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collection.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collection.ts @@ -10,6 +10,7 @@ import type { } from './helperTypes'; import { parseResourceUrl } from './resource'; import { serializeResource } from './serializers'; +import { Collection } from './specifyTable'; import { genericTables, tables } from './tables'; import type { Tables } from './types'; @@ -32,6 +33,7 @@ export type CollectionFetchFilters = Partial< | keyof SCHEMA['fields'] | `-${string & keyof CommonFields}` | `-${string & keyof SCHEMA['fields']}`; + readonly success?: (collection: Collection) => void; }; export const DEFAULT_FETCH_LIMIT = 20; diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts index aaf3a8b9828..b21230c03e1 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts @@ -8,6 +8,7 @@ import { Backbone } from './backbone'; import type { AnySchema } from './helperTypes'; import { DEFAULT_FETCH_LIMIT } from './collection'; import type { SpecifyResource } from './legacyTypes'; +import { relationshipIsToMany } from '../WbPlanView/mappingHelpers'; // REFACTOR: remove @ts-nocheck @@ -187,10 +188,16 @@ export const IndependentCollection = LazyCollection.extend({ this.updated = {}; }, initialize(_tables, options) { + setupToOne(this, options); + this.on( 'change', function (resource: SpecifyResource) { if (!resource.isBeingInitialized()) { + if (relationshipIsToMany(this.field)) { + const otherSideName = this.field.getReverse().name; + this.related.set(otherSideName, resource); + } this.updated[resource.cid] = resource; this.trigger('saverequired'); } @@ -230,8 +237,9 @@ export const IndependentCollection = LazyCollection.extend({ this.updated = {}; this.removed = new Set(); }); - - setupToOne(this, options); + }, + isComplete() { + return false; }, parse(resp) { const self = this; diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts index 528dc184a2d..584adc1cad3 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts @@ -7,6 +7,7 @@ import { Http } from '../../utils/ajax/definitions'; import { removeKey } from '../../utils/utils'; import { assert } from '../Errors/assert'; import { softFail } from '../Errors/Crash'; +import { relationshipIsToMany } from '../WbPlanView/mappingHelpers'; import { Backbone } from './backbone'; import { attachBusinessRules } from './businessRules'; import { isRelationshipCollection } from './collectionApi'; @@ -301,7 +302,7 @@ export const ResourceBase = Backbone.Model.extend({ ) { assert(!field.isDependent()); - if (field.type === 'one-to-many') + if (relationshipIsToMany(field)) this._storeIndependentToMany(field, related); else this._storeIndependentToOne(field, related); }, diff --git a/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx b/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx index 0e41415580f..638d4ad55df 100644 --- a/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx +++ b/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx @@ -59,18 +59,18 @@ export function useCollection({ const localVersionRef = versionRef.current; collection - .fetch(filters as CollectionFetchFilters) - .then((collection) => { - if (collection === undefined) return undefined; - /* - * If the collection is already being fetched, don't update it - * to prevent a race condition. - * REFACTOR: simplify this - */ - return versionRef.current === localVersionRef - ? void setCollection(collection) - : undefined; - }) + .fetch({ + ...filters, + success: (collection) => { + /* + * If the collection is already being fetched, don't update it + * to prevent a race condition. + * REFACTOR: simplify this + */ + if (versionRef.current === localVersionRef) + setCollection(collection); + }, + } as CollectionFetchFilters) .catch(raise); }, [collection, setCollection] From 7bcab20eb807a627fc345c944192c4bee64d2a50 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Mon, 30 Sep 2024 10:11:18 -0500 Subject: [PATCH 52/66] Fetch independnet toOne when rgetPromise called --- .../DataModel/__tests__/resourceApi.test.ts | 28 ++++++++++++++++--- .../lib/components/DataModel/resourceApi.ts | 3 +- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts index 57c4118a250..e56274cdeeb 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts @@ -21,7 +21,11 @@ const collectionObjectUrl = getResourceApiUrl( ); const accessionId = 11; const accessionUrl = getResourceApiUrl('Accession', accessionId); -const collectingEventUrl = getResourceApiUrl('CollectingEvent', 8868); +const collectingEventId = 8868; +const collectingEventUrl = getResourceApiUrl( + 'CollectingEvent', + collectingEventId +); const determinationUrl = getResourceApiUrl('Determination', 123); const determinationsResponse: RA>> = [ @@ -64,9 +68,12 @@ const accessionResponse = { }; overrideAjax(accessionUrl, accessionResponse); +const collectingEventText = 'testCollectingEvent'; + const collectingEventResponse = { resource_uri: collectingEventUrl, - id: 8868, + text1: collectingEventText, + id: collectingEventId, }; overrideAjax(collectingEventUrl, collectingEventResponse); @@ -172,14 +179,27 @@ describe('rgetCollection', () => { expect(agents.models).toHaveLength(0); }); - test('repeated calls for independent return different object', async () => { + test('repeated calls for independent return same object', async () => { const resource = new tables.CollectionObject.Resource({ id: collectionObjectId, }); const firstCollectingEvent = await resource.rgetPromise('collectingEvent'); const secondCollectingEvent = await resource.rgetPromise('collectingEvent'); expect(firstCollectingEvent?.toJSON()).toEqual(collectingEventResponse); - expect(firstCollectingEvent).not.toBe(secondCollectingEvent); + expect(firstCollectingEvent).toBe(secondCollectingEvent); + }); + + test('call for independent refetches related', async () => { + const resource = new tables.CollectionObject.Resource({ + id: collectionObjectId, + }); + const newCollectingEvent = new tables.CollectingEvent.Resource({ + id: collectingEventId, + text1: 'someOtherText', + }); + resource.set('collectingEvent', newCollectingEvent); + const firstCollectingEvent = await resource.rgetPromise('collectingEvent'); + expect(firstCollectingEvent?.get('text1')).toEqual(collectingEventText); }); test('repeated calls for dependent return same object', async () => { diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts index 584adc1cad3..1bac466e73e 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts @@ -695,7 +695,8 @@ export const ResourceBase = Backbone.Model.extend({ console.warn('expected dependent resource to be in cache'); this.storeDependent(field, toOne); } else { - this.storeIndependent(field, toOne); + const fetchedToOne = toOne.isNew() ? toOne : await toOne.fetch(); + this.storeIndependent(field, fetchedToOne); } } // If we want a field within the related resource then recur From bab5e1d853f5d457f4eec48743bb19938d8b7684 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 3 Oct 2024 15:19:50 -0500 Subject: [PATCH 53/66] Separate isComplete logic for Lazy/Relationship fetch calls --- .../DataModel/__tests__/businessRules.test.ts | 5 ++ .../lib/components/DataModel/collectionApi.ts | 60 ++++++++++--------- .../lib/components/DataModel/resourceApi.ts | 2 +- .../RecordSelectorFromCollection.tsx | 1 - 4 files changed, 37 insertions(+), 31 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/businessRules.test.ts b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/businessRules.test.ts index a3a04754ab0..48f44c23246 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/businessRules.test.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/businessRules.test.ts @@ -262,6 +262,11 @@ describe('uniqueness rules', () => { ]); }); + overrideAjax(getResourceApiUrl('Agent', 1), { + id: 1, + resource_uri: getResourceApiUrl('Agent', 1), + }); + test('rule with local collection', async () => { const accessionId = 1; const accession = new tables.Accession.Resource({ diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts index b21230c03e1..0df0b6c1984 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts @@ -31,6 +31,33 @@ async function fakeFetch() { return this; } +async function lazyFetch(options) { + assert(this instanceof LazyCollection); + if (this._fetch) return this._fetch; + if (this.related?.isNew()) return this; + + this._neverFetched = false; + + options ||= {}; + + options.update ??= true; + options.remove ??= false; + options.silent = true; + assert(options.at == null); + + options.data = + options.data || _.extend({ domainfilter: this.domainfilter }, this.filters); + options.data.offset = options.offset ?? this.length; + options.data.orderby = options.orderby; + + _(options).has('limit') && (options.data.limit = options.limit); + this._fetch = Backbone.Collection.prototype.fetch.call(this, options); + return this._fetch.then(() => { + this._fetch = null; + return this; + }); +} + function setupToOne(collection, options) { collection.field = options.field; collection.related = options.related; @@ -129,33 +156,11 @@ export const LazyCollection = Base.extend({ return objects; }, async fetch(options) { - if (this._fetch) return this._fetch; - else if (this.isComplete() || this.related?.isNew()) return this; - - if (this.isComplete()) + if (this.isComplete()) { console.error('fetching for already filled collection'); - - this._neverFetched = false; - - options ||= {}; - - options.update ??= true; - options.remove ??= false; - options.silent = true; - assert(options.at == null); - - options.data = - options.data || - _.extend({ domainfilter: this.domainfilter }, this.filters); - options.data.offset = options.offset ?? this.length; - options.data.orderby = options.orderby; - - _(options).has('limit') && (options.data.limit = options.limit); - this._fetch = Backbone.Collection.prototype.fetch.call(this, options); - return this._fetch.then(() => { - this._fetch = null; return this; - }); + } + return lazyFetch.call(this, options); }, async fetchIfNotPopulated() { return this._neverFetched && this.related?.isNew() !== true @@ -238,9 +243,6 @@ export const IndependentCollection = LazyCollection.extend({ this.removed = new Set(); }); }, - isComplete() { - return false; - }, parse(resp) { const self = this; const records = Reflect.apply( @@ -267,7 +269,7 @@ export const IndependentCollection = LazyCollection.extend({ offset: options?.offset ?? this.getFetchOffset(), }; - return Reflect.apply(LazyCollection.prototype.fetch, this, [newOptions]); + return lazyFetch.call(this, newOptions); }, getFetchOffset() { return this.length === 0 && this.removed.size > 0 diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts index 1bac466e73e..6cdc5d48303 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts @@ -695,7 +695,7 @@ export const ResourceBase = Backbone.Model.extend({ console.warn('expected dependent resource to be in cache'); this.storeDependent(field, toOne); } else { - const fetchedToOne = toOne.isNew() ? toOne : await toOne.fetch(); + const fetchedToOne = toOne.isNew() ? toOne : toOne; this.storeIndependent(field, fetchedToOne); } } diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx index 0561b75bc6b..05fcc9b6acd 100644 --- a/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx @@ -86,7 +86,6 @@ export function RecordSelectorFromCollection({ !isToOne && isLazy && collection.related?.isNew() !== true && - !collection.isComplete() && collection.models[index] === undefined ) handleFetch({ From d1cd4df5c2d9747bd1ef1ff70c0eee9ebd39e2a4 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 3 Oct 2024 20:24:40 +0000 Subject: [PATCH 54/66] Lint code with ESLint and Prettier Triggered by bab5e1d853f5d457f4eec48743bb19938d8b7684 on branch refs/heads/issue-114-backend --- .../lib/components/DataModel/collectionApi.ts | 19 +++++++------ .../lib/components/DataModel/resourceApi.ts | 28 +++++++++---------- 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts index 0df0b6c1984..7ba93d804f4 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts @@ -4,11 +4,11 @@ import _ from 'underscore'; import { removeKey } from '../../utils/utils'; import { assert } from '../Errors/assert'; +import { relationshipIsToMany } from '../WbPlanView/mappingHelpers'; import { Backbone } from './backbone'; -import type { AnySchema } from './helperTypes'; import { DEFAULT_FETCH_LIMIT } from './collection'; +import type { AnySchema } from './helperTypes'; import type { SpecifyResource } from './legacyTypes'; -import { relationshipIsToMany } from '../WbPlanView/mappingHelpers'; // REFACTOR: remove @ts-nocheck @@ -213,11 +213,11 @@ export const IndependentCollection = LazyCollection.extend({ this.on( 'add', function (resource: SpecifyResource) { - if (!resource.isNew()) { - (this.removed as Set).delete(resource.url()); - this.updated[resource.cid] = resource.url(); - } else { + if (resource.isNew()) { this.updated[resource.cid] = resource; + } else { + (this.removed as ReadonlySet).delete(resource.url()); + this.updated[resource.cid] = resource.url(); } this._totalCount += 1; this.trigger('saverequired'); @@ -229,7 +229,7 @@ export const IndependentCollection = LazyCollection.extend({ 'remove', function (resource: SpecifyResource) { if (!resource.isNew()) { - (this.removed as Set).add(resource.url()); + (this.removed as ReadonlySet).add(resource.url()); } this.updated = removeKey(this.updated, resource.cid); this._totalCount -= 1; @@ -251,10 +251,11 @@ export const IndependentCollection = LazyCollection.extend({ arguments ); - this._totalCount -= (this.removed as Set).size; + this._totalCount -= (this.removed as ReadonlySet).size; return records.filter( - ({ resource_uri }) => !(this.removed as Set).has(resource_uri) + ({ resource_uri }) => + !(this.removed as ReadonlySet).has(resource_uri) ); }, async fetch(options) { diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts index 6cdc5d48303..f115ebfa3db 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts @@ -641,7 +641,7 @@ export const ResourceBase = Backbone.Model.extend({ return value; }); }, - async _rget( + async _rget( path: RA, options: OPTIONS ) { @@ -772,19 +772,19 @@ export const ResourceBase = Backbone.Model.extend({ console.warn('expected dependent resource to be in cache'); const collection = - existingToMany !== undefined - ? existingToMany - : this.isNew() - ? new relatedTable.DependentCollection(collectionOptions, []) - : await new relatedTable.ToOneCollection(collectionOptions) - .fetch({ limit: 0 }) - .then( - (collection) => - new relatedTable.DependentCollection( - collectionOptions, - collection.models - ) - ); + existingToMany === undefined + ? this.isNew() + ? new relatedTable.DependentCollection(collectionOptions, []) + : await new relatedTable.ToOneCollection(collectionOptions) + .fetch({ limit: 0 }) + .then( + (collection) => + new relatedTable.DependentCollection( + collectionOptions, + collection.models + ) + ) + : existingToMany; return collection.fetch({ limit: 0 }).then((collection) => { self.storeDependent(field, collection); From 91b8031ec2cb2248a18bd5c9768f186fc5e93999 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Sat, 5 Oct 2024 20:50:20 -0500 Subject: [PATCH 55/66] Write frontend tests for Independent Collections --- .../DataModel/__tests__/collectionApi.test.ts | 416 +++++++++++++++++- .../lib/components/DataModel/collection.ts | 2 +- .../lib/components/DataModel/collectionApi.ts | 12 +- .../lib/components/DataModel/specifyTable.ts | 12 +- 4 files changed, 413 insertions(+), 29 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/collectionApi.test.ts b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/collectionApi.test.ts index db1747601f7..c77a618646f 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/collectionApi.test.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/collectionApi.test.ts @@ -1,30 +1,33 @@ import { overrideAjax } from '../../../tests/ajax'; import { requireContext } from '../../../tests/helpers'; import { overwriteReadOnly } from '../../../utils/types'; +import type { CollectionFetchFilters } from '../collection'; +import { DEFAULT_FETCH_LIMIT } from '../collection'; +import type { AnySchema } from '../helperTypes'; import { getResourceApiUrl } from '../resource'; import type { Collection } from '../specifyTable'; import { tables } from '../tables'; -import type { Accession, Agent } from '../types'; +import type { Accession, Agent, CollectionObject } from '../types'; requireContext(); -const secondAccessionUrl = getResourceApiUrl('Accession', 12); -const accessionId = 11; -const accessionUrl = getResourceApiUrl('Accession', accessionId); -const accessionNumber = '2011-IC-116'; -const accessionsResponse = [ - { - resource_uri: accessionUrl, - id: 11, - accessionnumber: accessionNumber, - }, - { - resource_uri: secondAccessionUrl, - id: 12, - }, -]; - describe('LazyCollection', () => { + const secondAccessionUrl = getResourceApiUrl('Accession', 12); + const accessionId = 11; + const accessionUrl = getResourceApiUrl('Accession', accessionId); + const accessionNumber = '2011-IC-116'; + const accessionsResponse = [ + { + resource_uri: accessionUrl, + id: 11, + accessionnumber: accessionNumber, + }, + { + resource_uri: secondAccessionUrl, + id: 12, + }, + ]; + overrideAjax( '/api/specify/accession/?domainfilter=false&addressofrecord=4&offset=0', { @@ -71,3 +74,382 @@ describe('LazyCollection', () => { expect(collection.toJSON()).toEqual(accessionsResponse); }); }); + +describe('Independent Collection', () => { + const collectionObjectsResponse = Array.from({ length: 41 }, (_, index) => ({ + id: index + 1, + resource_uri: getResourceApiUrl('CollectionObject', index + 1), + })); + + overrideAjax( + '/api/specify/collectionobject/?domainfilter=false&accession=1&offset=0', + { + objects: collectionObjectsResponse.slice(0, DEFAULT_FETCH_LIMIT), + meta: { + limit: DEFAULT_FETCH_LIMIT, + total_count: collectionObjectsResponse.length, + }, + } + ); + + overrideAjax( + `/api/specify/collectionobject/?domainfilter=false&accession=1&offset=${DEFAULT_FETCH_LIMIT}`, + { + objects: collectionObjectsResponse.slice( + DEFAULT_FETCH_LIMIT, + DEFAULT_FETCH_LIMIT * 2 + ), + meta: { + limit: DEFAULT_FETCH_LIMIT, + total_count: collectionObjectsResponse.length, + }, + } + ); + + overrideAjax( + `/api/specify/collectionobject/?domainfilter=false&accession=1&offset=${ + DEFAULT_FETCH_LIMIT * 2 + }`, + { + objects: collectionObjectsResponse.slice(DEFAULT_FETCH_LIMIT * 2), + meta: { + limit: DEFAULT_FETCH_LIMIT, + total_count: collectionObjectsResponse.length, + }, + } + ); + + overrideAjax( + '/api/specify/collectionobject/?domainfilter=false&accession=1&offset=20&limit=0', + { + objects: collectionObjectsResponse.slice(DEFAULT_FETCH_LIMIT), + meta: { + limit: 0, + total_count: collectionObjectsResponse.length, + }, + } + ); + + test('lazily fetched', async () => { + const accession = new tables.Accession.Resource({ + id: 1, + }); + + const rawCollection = new tables.CollectionObject.IndependentCollection({ + related: accession, + field: tables.CollectionObject.strictGetRelationship('accession'), + }); + + const collection = await rawCollection.fetch(); + expect(collection._totalCount).toBe(collectionObjectsResponse.length); + expect(collection.length).toBe(DEFAULT_FETCH_LIMIT); + expect(collection.models.map(({ id }) => id)).toStrictEqual( + collectionObjectsResponse + .slice(0, DEFAULT_FETCH_LIMIT) + .map(({ id }) => id) + ); + + await collection.fetch(); + expect(collection.length).toBe(DEFAULT_FETCH_LIMIT * 2); + expect( + collection.models + .slice(DEFAULT_FETCH_LIMIT, DEFAULT_FETCH_LIMIT * 2) + .map(({ id }) => id) + ).toStrictEqual( + collectionObjectsResponse + .slice(DEFAULT_FETCH_LIMIT, DEFAULT_FETCH_LIMIT * 2) + .map(({ id }) => id) + ); + + await collection.fetch(); + expect(collection.length).toBe(collection._totalCount); + }); + + test('specified offset', async () => { + const accession = new tables.Accession.Resource({ + id: 1, + }); + + const rawCollection = new tables.CollectionObject.IndependentCollection({ + related: accession, + field: tables.CollectionObject.strictGetRelationship('accession'), + }); + + const collection = await rawCollection.fetch({ + offset: DEFAULT_FETCH_LIMIT, + }); + expect(collection.length).toBe(DEFAULT_FETCH_LIMIT); + expect(collection.models.map(({ id }) => id)).toStrictEqual( + collectionObjectsResponse + .slice(DEFAULT_FETCH_LIMIT, DEFAULT_FETCH_LIMIT * 2) + .map(({ id }) => id) + ); + }); + + test('reset', async () => { + const accession = new tables.Accession.Resource({ + id: 1, + }); + + const rawCollection = new tables.CollectionObject.IndependentCollection({ + related: accession, + field: tables.CollectionObject.strictGetRelationship('accession'), + }); + + const collection = await rawCollection.fetch({ + offset: DEFAULT_FETCH_LIMIT, + limit: 0, + }); + expect(collection.length).toBe( + collectionObjectsResponse.length - DEFAULT_FETCH_LIMIT + ); + expect(collection.models.map(({ id }) => id)).toStrictEqual( + collectionObjectsResponse.slice(DEFAULT_FETCH_LIMIT).map(({ id }) => id) + ); + await collection.fetch({ + reset: true, + offset: 0, + } as CollectionFetchFilters); + expect(collection.length).toBe(DEFAULT_FETCH_LIMIT); + expect(collection.models.map(({ id }) => id)).toStrictEqual( + collectionObjectsResponse + .slice(0, DEFAULT_FETCH_LIMIT) + .map(({ id }) => id) + ); + }); + + test('removed objects not refetched', async () => { + const accession = new tables.Accession.Resource({ + id: 1, + }); + + const rawCollection = new tables.CollectionObject.IndependentCollection({ + related: accession, + field: tables.CollectionObject.strictGetRelationship('accession'), + }); + + const collection = await rawCollection.fetch(); + const collectionObjectsToRemove = collection.models + .slice(0, 5) + .map((collectionObject) => ({ ...collectionObject })); + collectionObjectsToRemove.forEach((collectionObject) => + collection.remove(collectionObject) + ); + await collection.fetch({ offset: 0 }); + expect(collection.models.map(({ id }) => id)).toStrictEqual( + collectionObjectsResponse + .slice(5, DEFAULT_FETCH_LIMIT) + .map(({ id }) => id) + ); + }); + + test('offset adjusted when all models removed', async () => { + const accession = new tables.Accession.Resource({ + id: 1, + }); + + const rawCollection = new tables.CollectionObject.IndependentCollection({ + related: accession, + field: tables.CollectionObject.strictGetRelationship('accession'), + }); + + const collection = await rawCollection.fetch(); + const collectionObjectsToRemove = collection.models.map( + (collectionObject) => ({ ...collectionObject }) + ); + collectionObjectsToRemove.forEach((collectionObject) => + collection.remove(collectionObject) + ); + expect(collection.getFetchOffset()).toBe(DEFAULT_FETCH_LIMIT); + await collection.fetch(); + expect(collection.models.map(({ id }) => id)).toStrictEqual( + collectionObjectsResponse + .slice(DEFAULT_FETCH_LIMIT, DEFAULT_FETCH_LIMIT * 2) + .map(({ id }) => id) + ); + }); + + test('on resource change event', async () => { + const accession = new tables.Accession.Resource({ + id: 1, + }); + + const rawCollection = new tables.CollectionObject.IndependentCollection({ + related: accession, + field: tables.CollectionObject.strictGetRelationship('accession'), + }); + + const collection = await rawCollection.fetch(); + + expect(collection._totalCount).toBe(collectionObjectsResponse.length); + + collection.models[0].set('text1', 'someValue'); + expect( + Object.values(collection.updated ?? {}).map((resource) => + typeof resource === 'string' ? resource : resource.toJSON() + ) + ).toStrictEqual([ + { + id: 1, + resource_uri: '/api/specify/collectionobject/1/', + text1: 'someValue', + }, + ]); + }); + + overrideAjax('/api/specify/accession/1/', { + id: 1, + resource_uri: getResourceApiUrl('Accession', 1), + }); + + overrideAjax('/api/specify/collectionobject/1/', { + id: 1, + resource_uri: getResourceApiUrl('CollectionObject', 1), + }); + + test('on change toOne', async () => { + const collectionObject = new tables.CollectionObject.Resource({ id: 1 }); + + const collection = new tables.Accession.IndependentCollection({ + related: collectionObject, + field: tables.Accession.strictGetRelationship('collectionObjects'), + }) as Collection; + + const rawAccession = new tables.Accession.Resource({ id: 1 }); + const accession = await rawAccession.fetch(); + + expect(collectionObject.get('accession')).toBeUndefined(); + collection.add(accession); + expect(collection.updated?.[accession.cid]).toBe( + getResourceApiUrl('Accession', 1) + ); + accession.set('accessionNumber', '2011-IC-116'); + expect(collection.updated?.[accession.cid]).toBe(accession); + expect(collectionObject.get('accession')).toBe( + getResourceApiUrl('Accession', 1) + ); + }); + + test('on add event', async () => { + const accession = new tables.Accession.Resource({ + id: 1, + }); + + const rawCollection = new tables.CollectionObject.IndependentCollection({ + related: accession, + field: tables.CollectionObject.strictGetRelationship('accession'), + }); + + const collection = await rawCollection.fetch(); + + const newCollectionObjects = [ + new tables.CollectionObject.Resource(), + new tables.CollectionObject.Resource({ id: 100 }), + ]; + collection.add(newCollectionObjects); + expect(collection._totalCount).toBe( + collectionObjectsResponse.length + newCollectionObjects.length + ); + expect(Object.keys(collection.updated ?? {})).toStrictEqual( + newCollectionObjects.map(({ cid }) => cid) + ); + newCollectionObjects.forEach((collectionObject) => { + const updatedEntry = collection.updated?.[collectionObject.cid]; + expect(updatedEntry).toBe( + collectionObject.isNew() ? collectionObject : collectionObject.url() + ); + }); + }); + test('on remove event', async () => { + const accession = new tables.Accession.Resource({ + id: 1, + }); + + const rawCollection = new tables.CollectionObject.IndependentCollection({ + related: accession, + field: tables.CollectionObject.strictGetRelationship('accession'), + }); + + const collection = await rawCollection.fetch(); + + const collectionObjectsToRemove = collection.models.slice(0, 3); + collectionObjectsToRemove.forEach((collectionObject) => + collection.remove(collectionObject) + ); + expect(collection._totalCount).toBe( + collectionObjectsResponse.length - collectionObjectsToRemove.length + ); + expect(Array.from(collection.removed ?? [])).toStrictEqual( + collectionObjectsToRemove.map((resource) => resource.get('resource_uri')) + ); + }); + test('removed and updated modify eachother', () => { + const accession = new tables.Accession.Resource({ + id: 1, + }); + + const collection = new tables.CollectionObject.IndependentCollection({ + related: accession, + field: tables.CollectionObject.strictGetRelationship('accession'), + }) as Collection; + const collectionObject = new tables.CollectionObject.Resource({ id: 1 }); + collection.add(collectionObject); + expect(collection.updated).toStrictEqual({ + [collectionObject.cid]: collectionObject.url(), + }); + collection.remove(collectionObject); + expect(collection.removed).toStrictEqual(new Set([collectionObject.url()])); + expect(collection.updated).toStrictEqual({}); + collection.add(collectionObject); + expect(collection.updated).toStrictEqual({ + [collectionObject.cid]: collectionObject.url(), + }); + expect(collection.removed).toStrictEqual(new Set()); + }); + + overrideAjax('/api/specify/collectionobject/200/', { + id: 200, + resource_uri: getResourceApiUrl('CollectionObject', 200), + }); + + test('toApiJSON', async () => { + const accession = new tables.Accession.Resource({ + id: 1, + }); + + const rawCollection = new tables.CollectionObject.IndependentCollection({ + related: accession, + field: tables.CollectionObject.strictGetRelationship('accession'), + }); + const collection = await rawCollection.fetch(); + expect(collection.toApiJSON()).toStrictEqual({ + update: [], + remove: [], + }); + const collectionObjectsToRemove = collection.models + .slice(1, 4) + .map((collectionObject) => collectionObject); + + collectionObjectsToRemove.forEach((collectionObject) => { + collection.remove(collectionObject); + }); + + const collectionObjectsToAdd = [ + new tables.CollectionObject.Resource({ id: 200 }), + new tables.CollectionObject.Resource({ text1: 'someValue' }), + ]; + collection.add(collectionObjectsToAdd); + collection.models[0].set('catalogNumber', '000000001'); + + expect(collection.toApiJSON()).toStrictEqual({ + remove: collectionObjectsToRemove.map((collectionObject) => + collectionObject.get('resource_uri') + ), + update: [ + '/api/specify/collectionobject/200/', + collection.models.at(-1), + collection.models[0], + ], + }); + }); +}); diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collection.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collection.ts index 0c5b516245d..06010a7fbe3 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collection.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collection.ts @@ -10,7 +10,7 @@ import type { } from './helperTypes'; import { parseResourceUrl } from './resource'; import { serializeResource } from './serializers'; -import { Collection } from './specifyTable'; +import type { Collection } from './specifyTable'; import { genericTables, tables } from './tables'; import type { Tables } from './types'; diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts index 7ba93d804f4..df7270973ce 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts @@ -178,17 +178,15 @@ export const LazyCollection = Base.extend({ export const IndependentCollection = LazyCollection.extend({ __name__: 'IndependentCollectionBase', - constructor(options, records = []) { + constructor(options) { this.table = this.model; - assert(_.isArray(records)); - Base.call(this, records, options); + Base.call(this, null, options); this.filters = options.filters || {}; this.domainfilter = Boolean(options.domainfilter) && this.model?.specifyTable.getScopingRelationship() !== undefined; - this._totalCount = records.length; - + this._totalCount = 0; this.removed = new Set(); this.updated = {}; }, @@ -278,11 +276,9 @@ export const IndependentCollection = LazyCollection.extend({ : Math.floor(this.length / DEFAULT_FETCH_LIMIT) * DEFAULT_FETCH_LIMIT; }, toApiJSON(options) { - const self = this; - return { update: Object.values(this.updated), - remove: Array.from(self.removed), + remove: Array.from(this.removed), }; }, }); diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts b/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts index 4995661d1a6..1869be51db3 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts @@ -79,9 +79,9 @@ type CollectionConstructor = new ( ) => UnFetchedCollection; export type UnFetchedCollection = { - readonly fetch: (filter?: { - readonly limit: number; - }) => Promise>; + readonly fetch: ( + filter?: CollectionFetchFilters + ) => Promise>; }; export type Collection = { @@ -93,6 +93,8 @@ export type Collection = { readonly table: { readonly specifyTable: SpecifyTable; }; + readonly updated?: IR | string>; + readonly removed?: ReadonlySet; readonly constructor: CollectionConstructor; /* * Shorthand method signature is used to prevent @@ -103,6 +105,10 @@ export type Collection = { isComplete(): boolean; getTotalCount(): Promise; getFetchOffset(): number; + toApiJSON(): { + readonly update: RA | string>; + readonly remove: RA; + }; indexOf(resource: SpecifyResource): number; // eslint-disable-next-line @typescript-eslint/naming-convention toJSON>(): RA; From dc01fc41bef6006c6439b13e7ea6815b8e6c433e Mon Sep 17 00:00:00 2001 From: melton-jason Date: Sun, 6 Oct 2024 01:54:15 +0000 Subject: [PATCH 56/66] Lint code with ESLint and Prettier Triggered by 91b8031ec2cb2248a18bd5c9768f186fc5e93999 on branch refs/heads/issue-114-backend --- .../DataModel/__tests__/collectionApi.test.ts | 12 ++++++------ .../js_src/lib/components/DataModel/specifyTable.ts | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/collectionApi.test.ts b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/collectionApi.test.ts index c77a618646f..42735c60009 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/collectionApi.test.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/collectionApi.test.ts @@ -142,7 +142,7 @@ describe('Independent Collection', () => { const collection = await rawCollection.fetch(); expect(collection._totalCount).toBe(collectionObjectsResponse.length); - expect(collection.length).toBe(DEFAULT_FETCH_LIMIT); + expect(collection).toHaveLength(DEFAULT_FETCH_LIMIT); expect(collection.models.map(({ id }) => id)).toStrictEqual( collectionObjectsResponse .slice(0, DEFAULT_FETCH_LIMIT) @@ -150,7 +150,7 @@ describe('Independent Collection', () => { ); await collection.fetch(); - expect(collection.length).toBe(DEFAULT_FETCH_LIMIT * 2); + expect(collection).toHaveLength(DEFAULT_FETCH_LIMIT * 2); expect( collection.models .slice(DEFAULT_FETCH_LIMIT, DEFAULT_FETCH_LIMIT * 2) @@ -162,7 +162,7 @@ describe('Independent Collection', () => { ); await collection.fetch(); - expect(collection.length).toBe(collection._totalCount); + expect(collection).toHaveLength(collection._totalCount); }); test('specified offset', async () => { @@ -178,7 +178,7 @@ describe('Independent Collection', () => { const collection = await rawCollection.fetch({ offset: DEFAULT_FETCH_LIMIT, }); - expect(collection.length).toBe(DEFAULT_FETCH_LIMIT); + expect(collection).toHaveLength(DEFAULT_FETCH_LIMIT); expect(collection.models.map(({ id }) => id)).toStrictEqual( collectionObjectsResponse .slice(DEFAULT_FETCH_LIMIT, DEFAULT_FETCH_LIMIT * 2) @@ -200,7 +200,7 @@ describe('Independent Collection', () => { offset: DEFAULT_FETCH_LIMIT, limit: 0, }); - expect(collection.length).toBe( + expect(collection).toHaveLength( collectionObjectsResponse.length - DEFAULT_FETCH_LIMIT ); expect(collection.models.map(({ id }) => id)).toStrictEqual( @@ -210,7 +210,7 @@ describe('Independent Collection', () => { reset: true, offset: 0, } as CollectionFetchFilters); - expect(collection.length).toBe(DEFAULT_FETCH_LIMIT); + expect(collection).toHaveLength(DEFAULT_FETCH_LIMIT); expect(collection.models.map(({ id }) => id)).toStrictEqual( collectionObjectsResponse .slice(0, DEFAULT_FETCH_LIMIT) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts b/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts index 1869be51db3..e7abde2361e 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/specifyTable.ts @@ -14,7 +14,7 @@ import { error } from '../Errors/assert'; import { attachmentView } from '../FormParse/webOnlyViews'; import { parentTableRelationship } from '../Forms/parentTables'; import { relationshipIsToMany } from '../WbPlanView/mappingHelpers'; -import { CollectionFetchFilters } from './collection'; +import type { CollectionFetchFilters } from './collection'; import { DependentCollection, IndependentCollection, From 5a9fe94928adf697180547f52235ea0eb4040b37 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Tue, 8 Oct 2024 14:42:17 -0500 Subject: [PATCH 57/66] Always respect options to collection fetch calls --- .../lib/components/DataModel/collectionApi.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts index df7270973ce..7effed034b9 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts @@ -27,14 +27,19 @@ function notSupported() { throw new Error('method is not supported'); } -async function fakeFetch() { +async function fakeFetch(rawOptions) { + const options = { + ...rawOptions, + }; + if (typeof options.success === 'function') + options.success.call(options.context, this, undefined, options); return this; } async function lazyFetch(options) { assert(this instanceof LazyCollection); if (this._fetch) return this._fetch; - if (this.related?.isNew()) return this; + if (this.related?.isNew()) return fakeFetch.call(this, options); this._neverFetched = false; @@ -119,7 +124,9 @@ export const DependentCollection = Base.extend({ getFetchOffset() { return 0; }, - fetch: fakeFetch, + async fetch(options) { + return fakeFetch.call(this, options); + }, sync: notSupported, create: notSupported, }); @@ -258,7 +265,7 @@ export const IndependentCollection = LazyCollection.extend({ }, async fetch(options) { // If the related is being fetched, don't try and fetch the collection - if (this.related._fetch !== null) return this; + if (this.related._fetch !== null) return fakeFetch.call(this, options); this.filters[this.field.name.toLowerCase()] = this.related.id; From d3821701a4c039df7edb57376603654bab9bb5af Mon Sep 17 00:00:00 2001 From: melton-jason Date: Tue, 8 Oct 2024 14:59:32 -0500 Subject: [PATCH 58/66] Write test for collection fetch options --- .../DataModel/__tests__/collectionApi.test.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/collectionApi.test.ts b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/collectionApi.test.ts index 42735c60009..1c88f40f0a7 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/collectionApi.test.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/collectionApi.test.ts @@ -407,6 +407,24 @@ describe('Independent Collection', () => { expect(collection.removed).toStrictEqual(new Set()); }); + test('success options respected', async () => { + const accession = new tables.Accession.Resource(); + + expect(accession.isNew()).toBe(true); + + const collection = new tables.CollectionObject.IndependentCollection({ + related: accession, + field: tables.CollectionObject.strictGetRelationship('accession'), + }) as Collection; + + await collection.fetch({ + success: (collection) => { + collection.add(new tables.CollectionObject.Resource()); + }, + } as CollectionFetchFilters); + expect(collection.models).toHaveLength(1); + }); + overrideAjax('/api/specify/collectionobject/200/', { id: 200, resource_uri: getResourceApiUrl('CollectionObject', 200), From ad2a9d53fa1283b4e38fa308313a6ed1704fd757 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Tue, 8 Oct 2024 15:05:40 -0500 Subject: [PATCH 59/66] Fix type error in test --- .../lib/components/DataModel/__tests__/collectionApi.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/collectionApi.test.ts b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/collectionApi.test.ts index 1c88f40f0a7..3ea9b454cea 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/collectionApi.test.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/collectionApi.test.ts @@ -162,7 +162,8 @@ describe('Independent Collection', () => { ); await collection.fetch(); - expect(collection).toHaveLength(collection._totalCount); + // eslint-disable-next-line jest/prefer-to-have-length + expect(collection.length).toBe(collection._totalCount); }); test('specified offset', async () => { From fb3dc170c1b94cac890d09215750edd9ee0b16eb Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 17 Oct 2024 23:38:07 -0500 Subject: [PATCH 60/66] Propagate change from toMany relationship collections to related --- .../DataModel/__tests__/collectionApi.test.ts | 1 + .../DataModel/__tests__/resourceApi.test.ts | 85 +++++++++++++++++++ .../lib/components/DataModel/businessRules.ts | 1 + .../lib/components/DataModel/collectionApi.ts | 9 +- .../lib/components/DataModel/resourceApi.ts | 6 +- 5 files changed, 92 insertions(+), 10 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/collectionApi.test.ts b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/collectionApi.test.ts index 3ea9b454cea..7caab498f0d 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/collectionApi.test.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/collectionApi.test.ts @@ -293,6 +293,7 @@ describe('Independent Collection', () => { { id: 1, resource_uri: '/api/specify/collectionobject/1/', + collectionobjecttype: '/api/specify/collectionobjecttype/1/', text1: 'someValue', }, ]); diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts index ab6f0c8d895..de9ab69cf03 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts @@ -2,6 +2,7 @@ import { overrideAjax } from '../../../tests/ajax'; import { requireContext } from '../../../tests/helpers'; import type { RA } from '../../../utils/types'; import { replaceItem } from '../../../utils/utils'; +import { addMissingFields } from '../addMissingFields'; import type { SerializedRecord } from '../helperTypes'; import { getResourceApiUrl } from '../resource'; import { tables } from '../tables'; @@ -219,6 +220,90 @@ describe('rgetCollection', () => { // TEST: add dependent and independent tests for all relationship types (and zero-to-one) }); +describe('eventHandlerForToMany', () => { + test('saverequired', () => { + const resource = new tables.CollectionObject.Resource( + addMissingFields('CollectionObject', { + preparations: [ + { + id: 1, + _tableName: 'Preparation', + }, + ], + }) + ); + const testFunction = jest.fn(); + resource.on('saverequired', testFunction); + expect(testFunction).toHaveBeenCalledTimes(0); + expect(resource.needsSaved).toBe(false); + resource + .getDependentResource('preparations') + ?.models[0].set('text1', 'helloWorld'); + + expect(resource.needsSaved).toBe(true); + expect(testFunction).toHaveBeenCalledTimes(1); + }); + test('changing collection propagates to related', () => { + const resource = new tables.CollectionObject.Resource( + addMissingFields('CollectionObject', { + preparations: [ + { + id: 1, + _tableName: 'Preparation', + }, + ], + }) + ); + const onResourceChange = jest.fn(); + const onPrepChange = jest.fn(); + const onPrepAdd = jest.fn(); + const onPrepRemoval = jest.fn(); + resource.on('change', onResourceChange); + resource.on('change:preparations', onPrepChange); + resource.on('add:preparations', onPrepAdd); + resource.on('remove:preparations', onPrepRemoval); + + resource + .getDependentResource('preparations') + ?.models[0].set('text1', 'helloWorld', { silent: false }); + expect(onResourceChange).toHaveBeenCalledWith( + resource, + resource.getDependentResource('preparations') + ); + expect(onPrepChange).toHaveBeenCalledWith( + resource.getDependentResource('preparations')?.models[0], + { silent: false } + ); + const newPrep = new tables.Preparation.Resource({ + barCode: 'test', + }); + resource.getDependentResource('preparations')?.add(newPrep); + expect(onPrepAdd).toHaveBeenCalledWith( + newPrep, + resource.getDependentResource('preparations'), + {} + ); + resource.getDependentResource('preparations')?.remove(newPrep); + expect(onPrepRemoval).toHaveBeenCalledWith( + newPrep, + resource.getDependentResource('preparations'), + { index: 1 } + ); + + expect(onResourceChange).toHaveBeenCalledTimes(3); + + resource.set('determinations', [ + addMissingFields('Determination', { + taxon: getResourceApiUrl('Taxon', 1), + }), + ]); + expect(onResourceChange).toHaveBeenCalledTimes(4); + expect(onPrepChange).toHaveBeenCalledTimes(1); + expect(onPrepAdd).toHaveBeenCalledTimes(1); + expect(onPrepRemoval).toHaveBeenCalledTimes(1); + }); +}); + describe('needsSaved', () => { test('changing field makes needsSaved true', () => { const resource = new tables.CollectionObject.Resource({ diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/businessRules.ts b/specifyweb/frontend/js_src/lib/components/DataModel/businessRules.ts index 01c162c429e..1d674cda2a0 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/businessRules.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/businessRules.ts @@ -56,6 +56,7 @@ export class BusinessRuleManager { if (isTreeResource(this.resource as SpecifyResource)) initializeTreeRecord(this.resource as SpecifyResource); + // REFACTOR: use the 'changed' event over 'change' this.resource.on('change', this.changed, this); this.resource.on('add', this.added, this); this.resource.on('remove', this.removed, this); diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts index 7effed034b9..b5e227d0f7d 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts @@ -85,20 +85,15 @@ export const DependentCollection = Base.extend({ Base.call(this, records, options); }, initialize(_tables, options) { + setupToOne(this, options); this.on( 'add remove', function () { - /* - * Warning: changing a collection record does not trigger a - * change event in the parent (though it probably should) - */ this.trigger('saverequired'); }, this ); - setupToOne(this, options); - /* * If the id of the related resource changes, we go through and update * all the objects that point to it with the new pointer. @@ -243,7 +238,7 @@ export const IndependentCollection = LazyCollection.extend({ this ); - this.listenTo(options.related, 'saved', function () { + this.listenTo(this.related, 'saved', function () { this.updated = {}; this.removed = new Set(); }); diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts index f115ebfa3db..abd64f319cf 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts @@ -40,7 +40,6 @@ function eventHandlerForToOne(related, field) { switch (event) { case 'saverequired': { this.handleChanged(); - this.trigger.apply(this, args); return; } case 'change:id': { @@ -62,7 +61,7 @@ function eventHandlerForToOne(related, field) { }; } -function eventHandlerForToMany(_related, field) { +function eventHandlerForToMany(related, field) { return function (event) { const args = _.toArray(arguments); switch (event) { @@ -72,14 +71,15 @@ function eventHandlerForToMany(_related, field) { } case 'saverequired': { this.handleChanged(); - this.trigger.apply(this, args); break; } + case 'change': case 'add': case 'remove': { // Annotate add and remove events with the field in which they occurred args[0] = `${event}:${field.name.toLowerCase()}`; this.trigger.apply(this, args); + this.trigger.apply(this, ['change', this, related]); break; } } From 49d8307bc8e4873af029cdbd1c9275ce9851b3c1 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Fri, 18 Oct 2024 04:42:23 +0000 Subject: [PATCH 61/66] Lint code with ESLint and Prettier Triggered by fb3dc170c1b94cac890d09215750edd9ee0b16eb on branch refs/heads/issue-114-backend --- .../frontend/js_src/lib/components/DataModel/resourceApi.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts index abd64f319cf..afa52be294c 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts @@ -79,7 +79,7 @@ function eventHandlerForToMany(related, field) { // Annotate add and remove events with the field in which they occurred args[0] = `${event}:${field.name.toLowerCase()}`; this.trigger.apply(this, args); - this.trigger.apply(this, ['change', this, related]); + Reflect.apply(this.trigger, this, ['change', this, related]); break; } } From 44aa226669b2541fe5e86e59ddf414c0c9e4d14f Mon Sep 17 00:00:00 2001 From: Jason Melton <64045831+melton-jason@users.noreply.github.com> Date: Tue, 22 Oct 2024 04:54:47 +0000 Subject: [PATCH 62/66] Lint code with ESLint and Prettier Triggered by 7c458477ccd86190d9bb173ac040467be19fee0c on branch refs/heads/issue-114-backend --- .../InitialContext/__tests__/treeRanks.test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/InitialContext/__tests__/treeRanks.test.ts b/specifyweb/frontend/js_src/lib/components/InitialContext/__tests__/treeRanks.test.ts index 0d778d27776..1775ae0cb61 100644 --- a/specifyweb/frontend/js_src/lib/components/InitialContext/__tests__/treeRanks.test.ts +++ b/specifyweb/frontend/js_src/lib/components/InitialContext/__tests__/treeRanks.test.ts @@ -69,11 +69,11 @@ test('getTreeScope', () => expect( Object.fromEntries(testingTrees.map((tree) => [tree, getTreeScope(tree)])) ).toMatchInlineSnapshot(` - { - "Geography": "discipline", - "GeologicTimePeriod": "discipline", - "LithoStrat": "discipline", - "Storage": "institution", - "Taxon": "discipline", - } - `)); + { + "Geography": "discipline", + "GeologicTimePeriod": "discipline", + "LithoStrat": "discipline", + "Storage": "institution", + "Taxon": "discipline", + } + `)); From 075614d47f7352396f7bfab3550d7d5054e85d9f Mon Sep 17 00:00:00 2001 From: melton-jason Date: Mon, 21 Oct 2024 23:58:55 -0500 Subject: [PATCH 63/66] Improve collection fetching for Grid-based Subviews --- .../lib/components/DataModel/collectionApi.ts | 9 +++++---- .../FormCells/FormTableCollection.tsx | 11 ++++++++--- .../FormSliders/IntegratedRecordSelector.tsx | 6 ++++-- .../frontend/js_src/lib/hooks/useCollection.tsx | 17 ++++++++++++----- 4 files changed, 29 insertions(+), 14 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts index b5e227d0f7d..e05b3312bba 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts @@ -38,6 +38,7 @@ async function fakeFetch(rawOptions) { async function lazyFetch(options) { assert(this instanceof LazyCollection); + const self = this; if (this._fetch) return this._fetch; if (this.related?.isNew()) return fakeFetch.call(this, options); @@ -58,8 +59,8 @@ async function lazyFetch(options) { _(options).has('limit') && (options.data.limit = options.limit); this._fetch = Backbone.Collection.prototype.fetch.call(this, options); return this._fetch.then(() => { - this._fetch = null; - return this; + self._fetch = null; + return self; }); } @@ -216,7 +217,7 @@ export const IndependentCollection = LazyCollection.extend({ if (resource.isNew()) { this.updated[resource.cid] = resource; } else { - (this.removed as ReadonlySet).delete(resource.url()); + this.removed.delete(resource.url()); this.updated[resource.cid] = resource.url(); } this._totalCount += 1; @@ -229,7 +230,7 @@ export const IndependentCollection = LazyCollection.extend({ 'remove', function (resource: SpecifyResource) { if (!resource.isNew()) { - (this.removed as ReadonlySet).add(resource.url()); + this.removed.add(resource.url()); } this.updated = removeKey(this.updated, resource.cid); this._totalCount -= 1; diff --git a/specifyweb/frontend/js_src/lib/components/FormCells/FormTableCollection.tsx b/specifyweb/frontend/js_src/lib/components/FormCells/FormTableCollection.tsx index 7602b17ac3b..5471c55eb86 100644 --- a/specifyweb/frontend/js_src/lib/components/FormCells/FormTableCollection.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormCells/FormTableCollection.tsx @@ -1,5 +1,6 @@ import React from 'react'; +import type { CollectionFetchFilters } from '../DataModel/collection'; import { DependentCollection } from '../DataModel/collectionApi'; import type { AnySchema } from '../DataModel/helperTypes'; import type { SpecifyResource } from '../DataModel/legacyTypes'; @@ -12,7 +13,7 @@ export function FormTableCollection({ collection, onAdd: handleAdd, onDelete: handleDelete, - onFetch: handleFetch, + onFetchMore: handleFetch, ...props }: Omit< Parameters[0], @@ -22,7 +23,9 @@ export function FormTableCollection({ readonly onDelete: | ((resource: SpecifyResource, index: number) => void) | undefined; - readonly onFetch?: () => void; + readonly onFetchMore?: ( + filters?: CollectionFetchFilters + ) => Promise | undefined>; }): JSX.Element | null { const [records, setRecords] = React.useState(Array.from(collection.models)); React.useEffect( @@ -37,7 +40,9 @@ export function FormTableCollection({ ); const handleFetchMore = React.useCallback(async () => { - handleFetch?.() ?? collection.fetch(); + await (typeof handleFetch === 'function' + ? handleFetch() + : collection.fetch()); setRecords(Array.from(collection.models)); }, [collection, handleFetch]); diff --git a/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx b/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx index 3c541dc10c3..bb87436e140 100644 --- a/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx +++ b/specifyweb/frontend/js_src/lib/components/FormSliders/IntegratedRecordSelector.tsx @@ -56,7 +56,9 @@ export function IntegratedRecordSelector({ readonly viewName?: string; readonly urlParameter?: string; readonly onClose: () => void; - readonly onFetch?: (filters?: CollectionFetchFilters) => void; + readonly onFetch?: ( + filters?: CollectionFetchFilters + ) => Promise | undefined>; readonly sortField: SubViewSortField | undefined; }): JSX.Element { const containerRef = React.useRef(null); @@ -311,7 +313,7 @@ export function IntegratedRecordSelector({ if (isCollapsed) handleExpand(); handleDelete?.(index, 'minusButton'); }} - onFetch={handleFetch} + onFetchMore={handleFetch} /> ) : null} {dialogs} diff --git a/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx b/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx index 638d4ad55df..53f1b900ee1 100644 --- a/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx +++ b/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx @@ -8,7 +8,7 @@ import type { Collection } from '../components/DataModel/specifyTable'; import { raise } from '../components/Errors/Crash'; import type { SubViewSortField } from '../components/FormParse/cells'; import { relationshipIsToMany } from '../components/WbPlanView/mappingHelpers'; -import type { GetOrSet } from '../utils/types'; +import type { GetOrSet, RA } from '../utils/types'; import { overwriteReadOnly } from '../utils/types'; import { sortFunction } from '../utils/utils'; import { useAsyncState } from './useAsyncState'; @@ -25,7 +25,9 @@ export function useCollection({ sortBy, }: UseCollectionProps): readonly [ ...GetOrSet | false | undefined>, - (filters?: CollectionFetchFilters) => void + ( + filters?: CollectionFetchFilters + ) => Promise | undefined> ] { const [collection, setCollection] = useAsyncState< Collection | false | undefined @@ -52,13 +54,15 @@ export function useCollection({ const versionRef = React.useRef(0); const handleFetch = React.useCallback( - (filters?: CollectionFetchFilters): void => { + async ( + filters?: CollectionFetchFilters + ): Promise | undefined> => { if (typeof collection !== 'object') return undefined; versionRef.current += 1; const localVersionRef = versionRef.current; - collection + return collection .fetch({ ...filters, success: (collection) => { @@ -71,7 +75,10 @@ export function useCollection({ setCollection(collection); }, } as CollectionFetchFilters) - .catch(raise); + .catch((error: Error, ...args: RA) => { + raise(error, args); + return undefined; + }); }, [collection, setCollection] ); From c6c9d8c38e625efada67fb14159e7eb20cd40588 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 23 Oct 2024 12:00:08 -0500 Subject: [PATCH 64/66] Allow passing filters to resource.rgetCollection --- .../lib/components/DataModel/legacyTypes.ts | 4 +- .../lib/components/DataModel/resourceApi.ts | 27 +++-- .../js_src/lib/components/Forms/SubView.tsx | 10 +- .../js_src/lib/hooks/useCollection.tsx | 106 ++++++++---------- 4 files changed, 72 insertions(+), 75 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/legacyTypes.ts b/specifyweb/frontend/js_src/lib/components/DataModel/legacyTypes.ts index 46b65d3b3df..5386c463198 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/legacyTypes.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/legacyTypes.ts @@ -4,6 +4,7 @@ import type { IR, RA } from '../../utils/types'; import type { BusinessRuleManager } from './businessRules'; +import { CollectionFetchFilters } from './collection'; import type { AnySchema, CommonFields, @@ -113,7 +114,8 @@ export type SpecifyResource = { VALUE extends (SCHEMA['toManyDependent'] & SCHEMA['toManyIndependent'])[FIELD_NAME] >( - fieldName: FIELD_NAME + fieldName: FIELD_NAME, + filters?: CollectionFetchFilters ): Promise>; set< FIELD_NAME extends diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts index afa52be294c..a3a6f106d25 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts @@ -10,6 +10,7 @@ import { softFail } from '../Errors/Crash'; import { relationshipIsToMany } from '../WbPlanView/mappingHelpers'; import { Backbone } from './backbone'; import { attachBusinessRules } from './businessRules'; +import { CollectionFetchFilters } from './collection'; import { isRelationshipCollection } from './collectionApi'; import { backboneFieldSeparator } from './helpers'; import type { @@ -606,8 +607,12 @@ export const ResourceBase = Backbone.Model.extend({ ); }, // Duplicate definition for purposes of better typing: - async rgetCollection(fieldName) { - return this.getRelated(fieldName, { prePop: true }); + async rgetCollection(fieldName, rawOptions) { + const options = { + ...rawOptions, + prePop: true, + }; + return this.getRelated(fieldName, options); }, async getRelated(fieldName, options) { options ||= { @@ -695,8 +700,7 @@ export const ResourceBase = Backbone.Model.extend({ console.warn('expected dependent resource to be in cache'); this.storeDependent(field, toOne); } else { - const fetchedToOne = toOne.isNew() ? toOne : toOne; - this.storeIndependent(field, fetchedToOne); + this.storeIndependent(field, toOne); } } // If we want a field within the related resource then recur @@ -708,8 +712,8 @@ export const ResourceBase = Backbone.Model.extend({ } return field.isDependent() - ? this.getDependentToMany(field) - : this.getIndependentToMany(field); + ? this.getDependentToMany(field, options) + : this.getIndependentToMany(field, options); } case 'zero-to-one': { /* @@ -752,7 +756,8 @@ export const ResourceBase = Backbone.Model.extend({ } }, async getDependentToMany( - field: Relationship + field: Relationship, + filters ): Promise> { assert(field.isDependent()); @@ -776,7 +781,7 @@ export const ResourceBase = Backbone.Model.extend({ ? this.isNew() ? new relatedTable.DependentCollection(collectionOptions, []) : await new relatedTable.ToOneCollection(collectionOptions) - .fetch({ limit: 0 }) + .fetch({ ...filters, limit: 0 }) .then( (collection) => new relatedTable.DependentCollection( @@ -786,13 +791,14 @@ export const ResourceBase = Backbone.Model.extend({ ) : existingToMany; - return collection.fetch({ limit: 0 }).then((collection) => { + return collection.fetch({ ...filters, limit: 0 }).then((collection) => { self.storeDependent(field, collection); return collection; }); }, async getIndependentToMany( - field: Relationship + field: Relationship, + filters ): Promise> { assert(!field.isDependent()); @@ -813,6 +819,7 @@ export const ResourceBase = Backbone.Model.extend({ : existingToMany; return collection.fetch({ + ...filters, // Only store the collection if fetch is successful (doesn't return undefined) success: (collection) => { this.storeIndependent(field, collection); diff --git a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx index 54b674005e4..67de74bc85c 100644 --- a/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx +++ b/specifyweb/frontend/js_src/lib/components/Forms/SubView.tsx @@ -79,12 +79,10 @@ export function SubView({ parentResource, 'saved', (): void => { - if (!relationship.isDependent()) { - handleFetch({ - offset: 0, - reset: true, - } as CollectionFetchFilters); - } + handleFetch({ + offset: 0, + reset: true, + } as CollectionFetchFilters); }, false ), diff --git a/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx b/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx index 53f1b900ee1..0d9e28e6bb4 100644 --- a/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx +++ b/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx @@ -5,10 +5,9 @@ import type { AnySchema } from '../components/DataModel/helperTypes'; import type { SpecifyResource } from '../components/DataModel/legacyTypes'; import type { Relationship } from '../components/DataModel/specifyField'; import type { Collection } from '../components/DataModel/specifyTable'; -import { raise } from '../components/Errors/Crash'; import type { SubViewSortField } from '../components/FormParse/cells'; import { relationshipIsToMany } from '../components/WbPlanView/mappingHelpers'; -import type { GetOrSet, RA } from '../utils/types'; +import type { GetOrSet } from '../utils/types'; import { overwriteReadOnly } from '../utils/types'; import { sortFunction } from '../utils/utils'; import { useAsyncState } from './useAsyncState'; @@ -17,6 +16,7 @@ type UseCollectionProps = { readonly parentResource: SpecifyResource; readonly relationship: Relationship; readonly sortBy?: SubViewSortField; + readonly filters?: CollectionFetchFilters; }; export function useCollection({ @@ -44,7 +44,6 @@ export function useCollection({ : fetchToOneCollection({ parentResource, relationship, - sortBy, }), [sortBy, parentResource, relationship] ), @@ -62,25 +61,28 @@ export function useCollection({ versionRef.current += 1; const localVersionRef = versionRef.current; - return collection - .fetch({ - ...filters, - success: (collection) => { - /* - * If the collection is already being fetched, don't update it - * to prevent a race condition. - * REFACTOR: simplify this - */ - if (versionRef.current === localVersionRef) - setCollection(collection); - }, - } as CollectionFetchFilters) - .catch((error: Error, ...args: RA) => { - raise(error, args); - return undefined; - }); + const fetchCollection = + relationshipIsToMany(relationship) && + relationship.type !== 'zero-to-one' + ? fetchToManyCollection({ + parentResource, + relationship, + sortBy, + filters, + }) + : fetchToOneCollection({ parentResource, relationship }); + + return fetchCollection.then((collection) => { + if ( + typeof collection === 'object' && + versionRef.current === localVersionRef + ) { + setCollection(collection); + } + return collection === false ? undefined : collection; + }); }, - [collection, setCollection] + [collection, parentResource, relationship, setCollection, sortBy] ); return [collection, setCollection, handleFetch]; } @@ -89,37 +91,39 @@ const fetchToManyCollection = async ({ parentResource, relationship, sortBy, + filters, }: UseCollectionProps): Promise | undefined> => - parentResource.rgetCollection(relationship.name).then((collection) => { - // TEST: check if this can ever happen - if (collection === null || collection === undefined) - return new relationship.relatedTable.DependentCollection({ - related: parentResource, - field: relationship.getReverse(), - }) as Collection; - if (sortBy === undefined) return collection; + parentResource + .rgetCollection(relationship.name, filters) + .then((collection) => { + // TEST: check if this can ever happen + if (collection === null || collection === undefined) + return new relationship.relatedTable.DependentCollection({ + related: parentResource, + field: relationship.getReverse(), + }) as Collection; + if (sortBy === undefined) return collection; - // BUG: this does not look into related tables - const field = sortBy.fieldNames[0]; + // BUG: this does not look into related tables + const field = sortBy.fieldNames[0]; - // Overwriting the models on the collection - overwriteReadOnly( - collection, - 'models', - Array.from(collection.models).sort( - sortFunction( - (resource) => resource.get(field), - sortBy.direction === 'desc' + // Overwriting the models on the collection + overwriteReadOnly( + collection, + 'models', + Array.from(collection.models).sort( + sortFunction( + (resource) => resource.get(field), + sortBy.direction === 'desc' + ) ) - ) - ); - return collection; - }); + ); + return collection; + }); async function fetchToOneCollection({ parentResource, relationship, - sortBy, }: UseCollectionProps): Promise< Collection | false | undefined > { @@ -160,19 +164,5 @@ async function fetchToOneCollection({ 'field', collection.field ?? relationship.getReverse() ); - if (sortBy !== undefined) { - // BUG: this does not look into related tables - const field = sortBy.fieldNames[0]; - overwriteReadOnly( - collection, - 'models', - Array.from(collection.models).sort( - sortFunction( - (resource) => resource.get(field), - sortBy.direction === 'desc' - ) - ) - ); - } return collection; } From 81b8751fd7c90a3680205d374876c9bb8510723d Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 23 Oct 2024 17:04:05 +0000 Subject: [PATCH 65/66] Lint code with ESLint and Prettier Triggered by c6c9d8c38e625efada67fb14159e7eb20cd40588 on branch refs/heads/issue-114-backend --- .../frontend/js_src/lib/components/DataModel/legacyTypes.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/DataModel/legacyTypes.ts b/specifyweb/frontend/js_src/lib/components/DataModel/legacyTypes.ts index 5386c463198..13bb62f3826 100644 --- a/specifyweb/frontend/js_src/lib/components/DataModel/legacyTypes.ts +++ b/specifyweb/frontend/js_src/lib/components/DataModel/legacyTypes.ts @@ -4,7 +4,7 @@ import type { IR, RA } from '../../utils/types'; import type { BusinessRuleManager } from './businessRules'; -import { CollectionFetchFilters } from './collection'; +import type { CollectionFetchFilters } from './collection'; import type { AnySchema, CommonFields, @@ -158,7 +158,7 @@ export type SpecifyResource = { ): SpecifyResource; // Not type safe bulkSet(value: IR): SpecifyResource; - //Unsafe + // Unsafe readonly independentResources: IR< Collection | SpecifyResource | null | undefined >; From f53e48d62f469eb598c984b5877a22ba517af6f4 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 23 Oct 2024 12:43:52 -0500 Subject: [PATCH 66/66] Update comment in fetchToOneCollection --- specifyweb/frontend/js_src/lib/hooks/useCollection.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx b/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx index 0d9e28e6bb4..b9b95b00772 100644 --- a/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx +++ b/specifyweb/frontend/js_src/lib/hooks/useCollection.tsx @@ -129,10 +129,8 @@ async function fetchToOneCollection({ > { /** * If relationship is -to-one, create a collection for the related - * resource. This allows to reuse most of the code from the -to-many - * relationships. RecordSelector handles collections with -to-one - * related field by removing the "+" button after first record is added - * and not rendering record count or record slider. + * resource. This allows to reuse most of the code from -to-many + * relationships in components like Subview and RecordSelectorFromCollection */ const resource = await parentResource.rgetPromise(relationship.name); const reverse = relationship.getReverse();