Fix #1853 - updateMany on sorted entity adapter

packages/toolkit/src/entities/sorted_state_adapter.ts

@@ -71,32 +71,40 @@ export function createSortedStateAdapter<T>(
     return updateManyMutably([update], state)
   }
 
-  // eslint-disable-next-line @typescript-eslint/prefer-readonly-parameter-types
-  function takeUpdatedModel(models: T[], update: Update<T>, state: R): boolean {
-    if (!(update.id in state.entities)) {
-      return false
-    }
-
-    const original = state.entities[update.id]
-    const updated = Object.assign({}, original, update.changes)
-    const newKey = selectIdValue(updated, selectId)
-
-    delete state.entities[update.id]
-
-    models.push(updated)
-
-    return newKey !== update.id
-  }
-
   function updateManyMutably(
     updates: ReadonlyArray<Update<T>>,
     state: R
   ): void {
-    const models: T[] = []
+    const changedKeys: EntityId[] = []
+
+    const updatesPerEntity: { [id: string]: Update<T> } = {}
+
+    updates.forEach((update) => {
+      if (update.id in state.entities) {
+        updatesPerEntity[update.id] = {
+          id: update.id,
+          changes: {
+            ...(updatesPerEntity[update.id]
+              ? updatesPerEntity[update.id].changes
+              : null),
+            ...update.changes,
+          },
+        }
+      }
+      const newId = selectId(update.changes as T)
+      if (newId !== undefined && update.id !== newId) {
+        changedKeys.push(update.id)
+      }
+    })
 
-    updates.forEach((update) => takeUpdatedModel(models, update, state))
+    updates = Object.values(updatesPerEntity)
 
-    if (models.length !== 0) {
+    if (updates.length > 0) {
+      const models = updates.map(
+        (update) =>
+          Object.assign({}, state.entities[update.id], update.changes) as T
+      )
+      changedKeys.forEach((key) => delete state.entities[key])
       merge(models, state)
     }
   }

packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts

@@ -30,9 +30,7 @@ describe('Sorted State Adapter', () => {
   beforeEach(() => {
     adapter = createEntityAdapter({
       selectId: (book: BookModel) => book.id,
-      sortComparer: (a, b) => {
-        return a.title.localeCompare(b.title)
-      },
+      sortComparer: (a, b) => a.title.localeCompare(b.title),
     })
 
     state = { ids: [], entities: {} }
@@ -652,12 +650,20 @@ describe('Sorted State Adapter', () => {
     test('updateMany', () => {
       const firstChange = { title: 'First Change' }
       const secondChange = { title: 'Second Change' }
-      const withMany = adapter.setAll(state, [TheGreatGatsby, AClockworkOrange])
+      const thirdChange = { title: 'Third Change' }
+      const fourthChange = { author: 'Fourth Change' }
+      const withMany = adapter.setAll(state, [
+        TheGreatGatsby,
+        AClockworkOrange,
+        TheHobbit,
+      ])
 
       const result = createNextState(withMany, (draft) => {
         adapter.updateMany(draft, [
-          { id: TheGreatGatsby.id, changes: firstChange },
-          { id: AClockworkOrange.id, changes: secondChange },
+          { id: TheHobbit.id, changes: firstChange },
+          { id: TheGreatGatsby.id, changes: secondChange },
+          { id: AClockworkOrange.id, changes: thirdChange },
+          { id: TheHobbit.id, changes: fourthChange },
         ])
       })
 
@@ -666,14 +672,20 @@ describe('Sorted State Adapter', () => {
           "entities": Object {
             "aco": Object {
               "id": "aco",
-              "title": "Second Change",
+              "title": "Third Change",
             },
             "tgg": Object {
               "id": "tgg",
+              "title": "Second Change",
+            },
+            "th": Object {
+              "author": "Fourth Change",
+              "id": "th",
               "title": "First Change",
             },
           },
           "ids": Array [
+            "th",
             "tgg",
             "aco",
           ],

packages/toolkit/src/entities/unsorted_state_adapter.ts

@@ -98,26 +98,6 @@ export function createUnsortedStateAdapter<T>(
     })
   }
 
-  function takeNewKey(
-    keys: { [id: string]: EntityId },
-    update: Update<T>,
-    state: R
-  ): boolean {
-    const original = state.entities[update.id]
-    const updated: T = Object.assign({}, original, update.changes)
-    const newKey = selectIdValue(updated, selectId)
-    const hasNewKey = newKey !== update.id
-
-    if (hasNewKey) {
-      keys[update.id] = newKey
-      delete state.entities[update.id]
-    }
-
-    state.entities[newKey] = updated
-
-    return hasNewKey
-  }
-
   function updateOneMutably(update: Update<T>, state: R): void {
     return updateManyMutably([update], state)
   }
@@ -127,7 +107,6 @@ export function createUnsortedStateAdapter<T>(
     state: R
   ): void {
     const newKeys: { [id: string]: EntityId } = {}
-
     const updatesPerEntity: { [id: string]: Update<T> } = {}
 
     updates.forEach((update) => {
@@ -146,16 +125,26 @@ export function createUnsortedStateAdapter<T>(
           },
         }
       }
+      const newId = selectId(update.changes as T)
+      if (newId !== undefined && update.id !== newId) {
+        newKeys[update.id] = newId;
+      }
     })
 
     updates = Object.values(updatesPerEntity)
 
     const didMutateEntities = updates.length > 0
 
     if (didMutateEntities) {
-      const didMutateIds =
-        updates.filter((update) => takeNewKey(newKeys, update, state)).length >
-        0
+      const changedKeys = Object.keys(newKeys);
+      const didMutateIds = changedKeys.length > 0;
+
+      updates.forEach((update) => {
+        const newEntity = Object.assign({}, state.entities[update.id], update.changes)
+        state.entities[newKeys[update.id] || update.id] = newEntity
+      })
+
+      changedKeys.forEach(key => delete state.entities[key])
 
       if (didMutateIds) {
         state.ids = state.ids.map((id) => newKeys[id] || id)