Merge pull request #6018 from specify/issue-5508

Add better support for one-to-one relationships (COG/COJO Data Entry fixes)

specifyweb/frontend/js_src/lib/components/DataModel/__tests__/resourceApi.test.ts

@@ -1,7 +1,7 @@
 import { overrideAjax } from '../../../tests/ajax';
 import { requireContext } from '../../../tests/helpers';
 import type { RA } from '../../../utils/types';
-import { replaceItem } from '../../../utils/utils';
+import { removeKey, replaceItem } from '../../../utils/utils';
 import { addMissingFields } from '../addMissingFields';
 import type { SerializedRecord } from '../helperTypes';
 import { getResourceApiUrl } from '../resource';
@@ -181,14 +181,20 @@ describe('rgetCollection', () => {
     expect(agents.models).toHaveLength(0);
   });
 
-  test('repeated calls for independent return different object', async () => {
+  test('repeated calls for independent merge different objects', async () => {
     const resource = new tables.CollectionObject.Resource({
       id: collectionObjectId,
     });
+    const testCEText = 'someOtherText';
+
     const firstCollectingEvent = await resource.rgetPromise('collectingEvent');
+    firstCollectingEvent?.set('text1', testCEText);
     const secondCollectingEvent = await resource.rgetPromise('collectingEvent');
-    expect(firstCollectingEvent?.toJSON()).toEqual(collectingEventResponse);
-    expect(firstCollectingEvent).not.toBe(secondCollectingEvent);
+    expect(testCEText).not.toBe(collectingEventText);
+    expect(secondCollectingEvent?.get('text1')).toBe(testCEText);
+    expect(
+      removeKey(firstCollectingEvent?.toJSON() ?? {}, 'text1')
+    ).toStrictEqual(removeKey(secondCollectingEvent?.toJSON() ?? {}, 'text1'));
   });
 
   test('call for independent refetches related', async () => {

specifyweb/frontend/js_src/lib/components/DataModel/collectionApi.ts

@@ -200,14 +200,16 @@ export const IndependentCollection = LazyCollection.extend({
     this.on(
       'change',
       function (resource: SpecifyResource<AnySchema>) {
-        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');
+        if (resource.isBeingInitialized()) return;
+        if (
+          relationshipIsToMany(this.field) ||
+          this.field.type === 'one-to-one'
+        ) {
+          const otherSideName = this.field.getReverse().name;
+          this.related.set(otherSideName, resource);
         }
+        this.updated[resource.cid] = resource;
+        this.trigger('saverequired');
       },
       this
     );

specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts

@@ -4,6 +4,7 @@ import _ from 'underscore';
 
 import { hijackBackboneAjax } from '../../utils/ajax/backboneAjax';
 import { Http } from '../../utils/ajax/definitions';
+import type { RA } from '../../utils/types';
 import { removeKey } from '../../utils/utils';
 import { assert } from '../Errors/assert';
 import { softFail } from '../Errors/Crash';
@@ -46,10 +47,6 @@ function eventHandlerForToOne(related, field) {
         this.set(field.name, related.url());
         return;
       }
-      case 'changing': {
-        this.trigger.apply(this, args);
-        return;
-      }
     }
 
     // Pass change:field events up the tree, updating fields with dot notation
@@ -65,10 +62,6 @@ function eventHandlerForToMany(related, field) {
   return function (event) {
     const args = _.toArray(arguments);
     switch (event) {
-      case 'changing': {
-        this.trigger.apply(this, args);
-        break;
-      }
       case 'saverequired': {
         this.handleChanged();
         break;
@@ -182,6 +175,7 @@ export const ResourceBase = Backbone.Model.extend({
           if (exemptFields.includes(fieldName)) return;
           const field = self.specifyTable.getField(fieldName);
           switch (field.type) {
+            case 'one-to-one':
             case 'many-to-one': {
               /*
                * Many-to-one wouldn't ordinarily be dependent, but
@@ -510,6 +504,7 @@ export const ResourceBase = Backbone.Model.extend({
          */
         return undefined;
       }
+      case 'one-to-one':
       case 'many-to-one': {
         if (!value) {
           /*
@@ -547,12 +542,6 @@ export const ResourceBase = Backbone.Model.extend({
         this.trigger('change', this);
         return undefined;
       }
-      /*
-       * Needed for taxonTreeDef on discipline because field.isVirtual equals false
-       */
-      case 'one-to-one': {
-        return value;
-      }
     }
     if (!field.isVirtual)
       softFail('Unhandled setting of relationship field', {
@@ -617,12 +606,12 @@ export const ResourceBase = Backbone.Model.extend({
     };
     return this.getRelated(fieldName, options);
   },
-  async getRelated(fieldName, options) {
+  async getRelated(fieldName: RA<string> | string, options) {
     options ||= {
       prePop: false,
       noBusinessRules: false,
     };
-    const path = _(fieldName).isArray()
+    const path = Array.isArray(fieldName)
       ? fieldName
       : fieldName.split(backboneFieldSeparator);
 
@@ -689,26 +678,31 @@ export const ResourceBase = Backbone.Model.extend({
         // A foreign key field.
         if (!value) return value; // No related object
 
-        // Is the related resource a cached dependent?
-        let toOne = this.dependentResources[fieldName];
+        // Is the related resource cached?
+        let toOne =
+          this.dependentResources[fieldName] ??
+          this.independentResources[fieldName];
 
         if (!toOne) {
-          if (typeof value === 'string')
+          if (typeof value === 'string') {
             toOne = resourceFromUrl(value, {
               noBusinessRules: options.noBusinessRules,
             });
-          else if (field.isDependent() && typeof value === 'object') {
+            if (toOne === undefined) softFail('expected URI, got', value);
+          } else if (typeof value === 'object') {
             toOne = new field.relatedTable.Resource({ ...value });
-          } else _(value).isString() || softFail('expected URI, got', value);
+          }
 
           if (field.isDependent()) {
             console.warn('expected dependent resource to be in cache');
             this.storeDependent(field, toOne);
-          } else {
-            // Always store and refetch independent related resources
-            this.storeIndependent(field, toOne);
           }
         }
+
+        // Always store and refetch independent related resources
+        if (!field.isDependent()) {
+          this.storeIndependent(field, toOne);
+        }
         // If we want a field within the related resource then recur
         return path.length > 1 ? toOne.rget(_.tail(path)) : toOne;
       }
@@ -728,7 +722,7 @@ export const ResourceBase = Backbone.Model.extend({
          */
 
         // Is it already cached?
-        if (!_.isUndefined(this.dependentResources[fieldName])) {
+        if (this.dependentResources[fieldName] !== undefined) {
           value = this.dependentResources[fieldName];
           if (value == null) return null;
           // Recur if we need to traverse more
@@ -902,7 +896,7 @@ export const ResourceBase = Backbone.Model.extend({
     });
 
     // Check added to avoid infinite loop in following forEach for collectionRelationship see https://github.com/specify/specify7/issues/6025
-    if (self.specifyTable.name === 'CollectionRelationship') return json
+    if (self.specifyTable.name === 'CollectionRelationship') return json;
 
     Object.entries(self.independentResources).forEach(
       ([fieldName, related]) => {