fix: Only remove object(s) from record array if in collection #8318 (#…

…8323) * fix: Only remove from record array if in collection #8318 * test: Update tests to use hasMany relationship and add deprecatedTests * chore: Seperate tests for sync/async * update tests post-review Co-authored-by: Chris Thoburn <[email protected]>
emberjs · Dec 7, 2022 · 9b5ce4a · 9b5ce4a
1 parent 608f90c
commit 9b5ce4a
Show file tree

Hide file tree

Showing 2 changed files with 187 additions and 2 deletions.
diff --git a/packages/store/addon/-private/record-arrays/identifier-array.ts b/packages/store/addon/-private/record-arrays/identifier-array.ts
@@ -658,13 +658,21 @@ if (DEPRECATE_ARRAY_LIKE) {
 
   IdentifierArray.prototype.removeObject = function (obj: RecordInstance) {
     deprecateArrayLike(this.DEPRECATED_CLASS_NAME, 'removeObject', 'splice');
-    this.splice(this.indexOf(obj), 1);
+    const index = this.indexOf(obj);
+    if (index !== -1) {
+      this.splice(index, 1);
+    }
     return this;
   };
 
   IdentifierArray.prototype.removeObjects = function (objs: RecordInstance[]) {
     deprecateArrayLike(this.DEPRECATED_CLASS_NAME, 'removeObjects', 'splice');
-    objs.forEach((obj) => this.splice(this.indexOf(obj), 1));
+    objs.forEach((obj) => {
+      const index = this.indexOf(obj);
+      if (index !== -1) {
+        this.splice(index, 1);
+      }
+    });
     return this;
   };
 

diff --git a/tests/main/tests/integration/relationships/has-many-test.js b/tests/main/tests/integration/relationships/has-many-test.js
@@ -4069,4 +4069,181 @@ module('integration/relationships/has_many - Has-Many Relationships', function (
 
     assert.strictEqual(person.phoneNumbers.length, 1);
   });
+
+  deprecatedTest(
+    'a synchronous hasMany record array should only remove object(s) if found in collection',
+    {
+      id: 'ember-data:deprecate-array-like',
+      count: 3,
+      until: '5.0',
+    },
+    async function (assert) {
+      class Person extends Model {
+        @attr()
+        name;
+        @belongsTo('tag', { async: false, inverse: 'people' })
+        tag;
+      }
+
+      class Tag extends Model {
+        @hasMany('person', { async: false, inverse: 'tag' })
+        people;
+      }
+
+      this.owner.register('model:person', Person);
+      this.owner.register('model:tag', Tag);
+
+      const store = this.owner.lookup('service:store');
+      // eslint-disable-next-line no-unused-vars
+      const [tag, scumbagInRecordArray, _person2, scumbagNotInRecordArray] = store.push({
+        data: [
+          {
+            type: 'tag',
+            id: '1',
+            relationships: {
+              people: {
+                data: [
+                  { type: 'person', id: '1' },
+                  { type: 'person', id: '2' },
+                ],
+              },
+            },
+          },
+          {
+            type: 'person',
+            id: '1',
+            attributes: {
+              name: 'Scumbag Dale',
+            },
+          },
+          {
+            type: 'person',
+            id: '2',
+            attributes: {
+              name: 'Scumbag Tom',
+            },
+          },
+          {
+            type: 'person',
+            id: '3',
+            attributes: {
+              name: 'Scumbag Ross',
+            },
+          },
+        ],
+      });
+
+      let recordArray = tag.people;
+
+      recordArray.removeObject(scumbagNotInRecordArray);
+
+      assert.strictEqual(
+        recordArray.length,
+        2,
+        'Record array unchanged after attempting to remove object not found in collection'
+      );
+
+      recordArray.removeObject(scumbagInRecordArray);
+
+      let didRemoveObject = recordArray.length === 1 && !recordArray.includes(scumbagInRecordArray);
+      assert.true(didRemoveObject, 'Record array successfully removed expected object from collection');
+
+      recordArray.push(scumbagInRecordArray);
+
+      let scumbagsToRemove = [scumbagInRecordArray, scumbagNotInRecordArray];
+      recordArray.removeObjects(scumbagsToRemove);
+
+      didRemoveObject = recordArray.length === 1 && !recordArray.includes(scumbagInRecordArray);
+      assert.true(didRemoveObject, 'Record array only removes objects in list that are found in collection');
+    }
+  );
+
+  deprecatedTest(
+    'an asynchronous hasMany record array should only remove object(s) if found in collection',
+    {
+      id: 'ember-data:deprecate-promise-many-array-behaviors',
+      count: 6,
+      until: '5.0',
+    },
+    async function (assert) {
+      class Person extends Model {
+        @attr()
+        name;
+        @belongsTo('tag', { async: false, inverse: 'people' })
+        tag;
+      }
+
+      class Tag extends Model {
+        @hasMany('person', { async: true, inverse: 'tag' })
+        people;
+      }
+
+      const store = this.owner.lookup('service:store');
+      this.owner.register('model:person', Person);
+      this.owner.register('model:tag', Tag);
+
+      // eslint-disable-next-line no-unused-vars
+      const [tag, scumbagInRecordArray, _person2, scumbagNotInRecordArray] = store.push({
+        data: [
+          {
+            type: 'tag',
+            id: '1',
+            relationships: {
+              people: {
+                data: [
+                  { type: 'person', id: '1' },
+                  { type: 'person', id: '2' },
+                ],
+              },
+            },
+          },
+          {
+            type: 'person',
+            id: '1',
+            attributes: {
+              name: 'Scumbag Dale',
+            },
+          },
+          {
+            type: 'person',
+            id: '2',
+            attributes: {
+              name: 'Scumbag Tom',
+            },
+          },
+          {
+            type: 'person',
+            id: '3',
+            attributes: {
+              name: 'Scumbag Ross',
+            },
+          },
+        ],
+      });
+
+      let recordArray = tag.people;
+
+      recordArray.removeObject(scumbagNotInRecordArray);
+
+      assert.strictEqual(
+        recordArray.length,
+        2,
+        'Record array unchanged after attempting to remove object not found in collection'
+      );
+
+      recordArray.removeObject(scumbagInRecordArray);
+
+      let didRemoveObject = recordArray.length === 1 && !recordArray.includes(scumbagInRecordArray);
+      assert.true(didRemoveObject, 'Record array successfully removed expected object from collection');
+
+      recordArray.pushObject(scumbagInRecordArray);
+
+      let scumbagsToRemove = [scumbagInRecordArray, scumbagNotInRecordArray];
+      recordArray.removeObjects(scumbagsToRemove);
+
+      didRemoveObject = recordArray.length === 1 && !recordArray.includes(scumbagInRecordArray);
+      assert.true(didRemoveObject, 'Record array only removes objects in list that are found in collection');
+      assert.expectDeprecation({ id: 'ember-data:deprecate-array-like', count: 4 });
+    }
+  );
 });