Use keyed redux record store

It's the recommended approach in Redux docs, and more performant (key-based).
It'll also simplify a move to Immutable.js later on.

The PHP APIs still return unkeyed data, which is standard REST API behaviour,
but for Redux state we transform it to be keyed.

admin/javascript/src/components/grid-field/index.js

@@ -56,7 +56,8 @@ class GridField extends SilverStripeComponent {
     );
     const header = <GridFieldHeader>{headerCells.concat(actionPlaceholder)}</GridFieldHeader>;
 
-    const rows = records.map((record, i) => {
+    const rows = Object.keys(records).map((i) => {
+      const record = records[i];
       // Build cells
       const cells = columns.map((column, j) => {
         // Get value by dot notation
@@ -141,7 +142,7 @@ GridField.propTypes = {
 function mapStateToProps(state, ownProps) {
   const recordType = ownProps.data ? ownProps.data.recordType : null;
   return {
-    records: (state.records && recordType) ? state.records[recordType] : [],
+    records: (state.records && recordType) ? state.records[recordType] : {},
   };
 }
 

admin/javascript/src/sections/campaign-admin/list.js

@@ -218,9 +218,7 @@ function mapStateToProps(state, ownProps) {
   // Find record specific to this item
   let record = null;
   if (state.records && state.records.ChangeSet && ownProps.campaignId) {
-    record = state.records.ChangeSet.find(
+    record = state.records.ChangeSet[parseInt(ownProps.campaignId, 10)];
-      (nextRecord) => (nextRecord.ID === parseInt(ownProps.campaignId, 10))
-    );
   }
   return {
     record: record || [],

admin/javascript/src/state/records/reducer.js

@@ -7,7 +7,6 @@ function recordsReducer(state = initialState, action) {
   let records;
   let recordType;
   let record;
-  let recordIndex;
 
   switch (action.type) {
 
@@ -29,7 +28,8 @@ function recordsReducer(state = initialState, action) {
     case ACTION_TYPES.FETCH_RECORDS_SUCCESS:
       recordType = action.payload.recordType;
       // TODO Automatic pluralisation from recordType
-      records = action.payload.data._embedded[`${recordType}s`];
+      records = action.payload.data._embedded[`${recordType}s`] || [];
+      records = records.reduce((prev, val) => Object.assign({}, prev, { [val.id]: val }), {});
       return deepFreeze(Object.assign({}, state, {
         [recordType]: records,
       }));
@@ -43,18 +43,9 @@ function recordsReducer(state = initialState, action) {
     case ACTION_TYPES.FETCH_RECORD_SUCCESS:
       recordType = action.payload.recordType;
       record = action.payload.data;
-      records = state[recordType] ? state[recordType] : [];
-
-      // Update or insert
-      recordIndex = records.findIndex((nextRecord) => (nextRecord.ID === record.ID));
-      if (recordIndex > -1) {
-        records[recordIndex] = record;
-      } else {
-        records.push(record);
-      }
 
       return deepFreeze(Object.assign({}, state, {
-        [recordType]: records,
+        [recordType]: Object.assign({}, state[recordType], { [record.ID]: record }),
       }));
 
     case ACTION_TYPES.DELETE_RECORD_REQUEST:
@@ -65,8 +56,14 @@ function recordsReducer(state = initialState, action) {
 
     case ACTION_TYPES.DELETE_RECORD_SUCCESS:
       recordType = action.payload.recordType;
-      records = state[recordType]
+      records = state[recordType];
-        .filter(nextRecord => nextRecord.ID !== action.payload.id);
+      records = Object.keys(records)
+        .reduce((result, key) => {
+          if (parseInt(key, 10) !== parseInt(action.payload.id, 10)) {
+            return Object.assign({}, result, { [key]: records[key] });
+          }
+          return result;
+        }, {});
 
       return deepFreeze(Object.assign({}, state, {
         [recordType]: records,

admin/javascript/src/state/records/tests/reducer-test.js

@@ -8,29 +8,57 @@ const recordsReducer = require('../reducer').default;
 const ACTION_TYPES = require('../action-types').default;
 
 describe('recordsReducer', () => {
+  describe('FETCH_RECORD_SUCCESS', () => {
+    it('adds a new record', () => {
+      const initialState = {
+        TypeA: {
+          11: { ID: 11 },
+        },
+        TypeB: {
+          11: { ID: 11 },
+        },
+      };
+
+      const nextState = recordsReducer(initialState, {
+        type: ACTION_TYPES.FETCH_RECORD_SUCCESS,
+        payload: { recordType: 'TypeA', data: { ID: 12 } },
+      });
+
+      expect(nextState.TypeA).toEqual({
+        11: { ID: 11 },
+        12: { ID: 12 },
+      });
+      expect(nextState.TypeB).toEqual({
+        11: { ID: 11 },
+      });
+    });
+  });
+
   describe('DELETE_RECORD_SUCCESS', () => {
     const initialState = {
-      TypeA: [
+      TypeA: {
-        { ID: 1 },
+        11: { ID: 11 },
-        { ID: 2 },
+        12: { ID: 12 },
-      ],
+      },
-      TypeB: [
+      TypeB: {
-        { ID: 1 },
+        11: { ID: 11 },
-        { ID: 2 },
+        12: { ID: 12 },
-      ],
+      },
     };
 
     it('removes records from the declared type', () => {
       const nextState = recordsReducer(initialState, {
         type: ACTION_TYPES.DELETE_RECORD_SUCCESS,
-        payload: { recordType: 'TypeA', id: 2 },
+        payload: { recordType: 'TypeA', id: 12 },
       });
 
-      expect(nextState.TypeA.length).toBe(1);
+      expect(nextState.TypeA).toEqual({
-      expect(nextState.TypeA[0].ID).toBe(1);
+        11: { ID: 11 },
-      expect(nextState.TypeB.length).toBe(2);
+      });
-      expect(nextState.TypeB[0].ID).toBe(1);
+      expect(nextState.TypeB).toEqual({
-      expect(nextState.TypeB[1].ID).toBe(2);
+        11: { ID: 11 },
+        12: { ID: 12 },
+      });
     });
   });
 });