Fix remove hardcoding ID to index of records

client/src/components/GridField/GridField.js

@@ -12,7 +12,7 @@ import GridFieldAction from './GridFieldAction';
 import FormConstants from 'components/Form/FormConstants';
 import * as actions from 'state/records/RecordsActions';
 
-const NotYetLoaded = {};
+const NotYetLoaded = [];
 
 /**
  * The component acts as a container for a grid field,
@@ -44,13 +44,12 @@ class GridField extends SilverStripeComponent {
   }
 
   render() {
-    // props.records is keyed by record identifiers
     if (this.props.records === NotYetLoaded) {
       // TODO Replace with better loading indicator
       return <div>{ i18n._t('CampaignAdmin.LOADING', 'Loading...') }</div>;
     }
 
-    if (!Object.getOwnPropertyNames(this.props.records).length) {
+    if (!this.props.records.length) {
       return <div>{ i18n._t('CampaignAdmin.NO_RECORDS', 'No campaigns created yet.') }</div>;
     }
 
@@ -60,8 +59,8 @@ class GridField extends SilverStripeComponent {
       <GridFieldHeaderCell key={`${column.name}`}>{column.name}</GridFieldHeaderCell>
     );
     const header = <GridFieldHeader>{headerCells.concat(actionPlaceholder)}</GridFieldHeader>;
-    const rows = Object.keys(this.props.records).map((key) =>
-      this.createRow(this.props.records[key])
+    const rows = this.props.records.map((record) =>
+      this.createRow(record)
     );
 
     return (
@@ -173,10 +172,10 @@ GridField.propTypes = {
 };
 
 function mapStateToProps(state, ownProps) {
-  const recordType = ownProps.data ? ownProps.data.recordType : null;
+  const recordType = ownProps.data && ownProps.data.recordType;
   return {
     config: state.config,
-    records: recordType && state.records[recordType] ? state.records[recordType] : NotYetLoaded,
+    records: (recordType && state.records[recordType]) ? state.records[recordType] : NotYetLoaded,
   };
 }
 

client/src/state/records/RecordsActionTypes.js

@@ -1,7 +1,4 @@
 export default {
-  CREATE_RECORD: 'CREATE_RECORD',
-  UPDATE_RECORD: 'UPDATE_RECORD',
-  DELETE_RECORD: 'DELETE_RECORD',
   FETCH_RECORDS_REQUEST: 'FETCH_RECORDS_REQUEST',
   FETCH_RECORDS_FAILURE: 'FETCH_RECORDS_FAILURE',
   FETCH_RECORDS_SUCCESS: 'FETCH_RECORDS_SUCCESS',

client/src/state/records/RecordsActions.js

@@ -1,5 +1,5 @@
 import ACTION_TYPES from './RecordsActionTypes';
-import backend from 'lib/Backend.js';
+import backend from 'lib/Backend';
 
 /**
  * Populate strings based on a whitelist.
@@ -8,21 +8,23 @@ import backend from 'lib/Backend.js';
  *
  * Example: populate("foo/bar/:id", {id: 123}) => "foo/bar/123"
  *
- * @param string str A template string with ":<name>" notation.
- * @param object Map of names to values
+ * @param {string} template A template string with ":<name>" notation.
+ * @param {object} params of names to values
  * @return string
  */
-function populate(str, params) {
-  const names = ['id'];
-  return names.reduce((acc, name) => acc.replace(`:${name}`, params[name]), str);
+function populate(template, params) {
+  return Object.keys(params)
+    .reduce((result, name) => (
+      result.replace(`:${name}`, params[name])
+    ), template);
 }
 
 /**
  * Retrieves all records
  *
- * @param string recordType Type of record (the "class name")
- * @param string method HTTP method
- * @param string url API endpoint
+ * @param {string} recordType Type of record (the "class name")
+ * @param {string} method HTTP method
+ * @param {string} url API endpoint
  */
 export function fetchRecords(recordType, method, url) {
   const payload = { recordType };
@@ -61,9 +63,9 @@ export function fetchRecords(recordType, method, url) {
 /**
  * Fetches a single record
  *
- * @param string recordType Type of record (the "class name")
- * @param string method HTTP method
- * @param string url API endpoint
+ * @param {string} recordType Type of record (the "class name")
+ * @param {string} method HTTP method
+ * @param {string} url API endpoint
  */
 export function fetchRecord(recordType, method, url) {
   const payload = { recordType };
@@ -105,7 +107,7 @@ export function fetchRecord(recordType, method, url) {
  * @param {number} id Database identifier
  * @param {string} method HTTP method
  * @param {string} url API endpoint
- * @param {object} Headers
+ * @param {object} headers
  */
 export function deleteRecord(recordType, id, method, url, headers = {}) {
   const payload = { recordType, id };

client/src/state/records/RecordsReducer.js

@@ -4,77 +4,59 @@ import ACTION_TYPES from './RecordsActionTypes';
 const initialState = {};
 
 function recordsReducer(state = initialState, action) {
-  let records = null;
-  let recordType = null;
-  let record = null;
-
   switch (action.type) {
-    case ACTION_TYPES.CREATE_RECORD:
-      return deepFreeze(Object.assign({}, state, {}));
-
-    case ACTION_TYPES.UPDATE_RECORD:
-      return deepFreeze(Object.assign({}, state, {}));
-
-    case ACTION_TYPES.DELETE_RECORD:
-      return deepFreeze(Object.assign({}, state, {}));
-
-    case ACTION_TYPES.FETCH_RECORDS_REQUEST:
-      return state;
-
-    case ACTION_TYPES.FETCH_RECORDS_FAILURE:
-      return state;
-
-    case ACTION_TYPES.FETCH_RECORDS_SUCCESS:
-      recordType = action.payload.recordType;
+    case ACTION_TYPES.FETCH_RECORDS_SUCCESS: {
+      const recordType = action.payload.recordType;
       if (!recordType) {
         throw new Error('Undefined record type');
       }
-      records = action.payload.data._embedded[recordType] || {};
-      records = records.reduce((prev, val) => Object.assign({}, prev, { [val.ID]: val }), {});
-      return deepFreeze(Object.assign({}, state, {
+      const records = action.payload.data._embedded[recordType] || [];
+      return deepFreeze({
+        ...state,
         [recordType]: records,
-      }));
-
-    case ACTION_TYPES.FETCH_RECORD_REQUEST:
-      return state;
-
-    case ACTION_TYPES.FETCH_RECORD_FAILURE:
-      return state;
-
-    case ACTION_TYPES.FETCH_RECORD_SUCCESS:
-      recordType = action.payload.recordType;
-      record = action.payload.data;
+      });
+    }
 
+    case ACTION_TYPES.FETCH_RECORD_SUCCESS: {
+      const recordType = action.payload.recordType;
+      const newRecord = action.payload.data;
       if (!recordType) {
         throw new Error('Undefined record type');
       }
-      return deepFreeze(Object.assign({}, state, {
-        [recordType]: Object.assign({}, state[recordType], { [record.ID]: record }),
-      }));
-
-    case ACTION_TYPES.DELETE_RECORD_REQUEST:
-      return state;
-
-    case ACTION_TYPES.DELETE_RECORD_FAILURE:
-      return state;
-
-    case ACTION_TYPES.DELETE_RECORD_SUCCESS:
-      recordType = action.payload.recordType;
-      records = state[recordType];
-      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;
-        }, {});
+      if (!newRecord) {
+        throw new Error('Undefined record data given');
+      }
+      const records = state[recordType] || [];
+      // Conditionally replace or append
+      if (records.find((next) => next.ID === newRecord.ID)) {
+        return deepFreeze({
+          ...state,
+          [recordType]: records.map((next) => (next.ID === newRecord.ID ? newRecord : next)),
+        });
+      }
+      return deepFreeze({
+        ...state,
+        [recordType]: [...records, newRecord],
+      });
+    }
+
+    case ACTION_TYPES.DELETE_RECORD_SUCCESS: {
+      const recordType = action.payload.recordType;
+      if (!recordType) {
+        throw new Error('Undefined record type');
+      }
+      const records = state[recordType]
+        .filter((record) => record.ID !== action.payload.id);
 
-      return deepFreeze(Object.assign({}, state, {
+      return deepFreeze({
+        ...state,
         [recordType]: records,
-      }));
+      });
+    }
 
-    default:
+    default: {
       return state;
+    }
   }
 }
 

client/src/state/records/tests/RecordsReducer-test.js

@@ -9,56 +9,67 @@ const ACTION_TYPES = require('../RecordsActionTypes').default;
 
 describe('recordsReducer', () => {
   describe('FETCH_RECORD_SUCCESS', () => {
-    it('adds a new record', () => {
-      const initialState = {
-        TypeA: {
-          11: { ID: 11 },
-        },
-        TypeB: {
-          11: { ID: 11 },
-        },
+    let initialState = null;
+    beforeEach(() => {
+      initialState = {
+        TypeA: [{ ID: 11, title: 'record a11' }, { ID: 12, title: 'record a12' }],
+        TypeB: [{ ID: 11, title: 'record b11' }],
       };
+    });
 
+    it('adds a new record', () => {
       const nextState = recordsReducer(initialState, {
         type: ACTION_TYPES.FETCH_RECORD_SUCCESS,
-        payload: { recordType: 'TypeA', data: { ID: 12 } },
+        payload: { recordType: 'TypeA', data: { ID: 13, title: 'record a13' } },
       });
+      expect(nextState.TypeA).toEqual([
+        { ID: 11, title: 'record a11' },
+        { ID: 12, title: 'record a12' },
+        { ID: 13, title: 'record a13' },
+      ]);
+      expect(nextState.TypeB).toEqual([
+        { ID: 11, title: 'record b11' },
+      ]);
+    });
 
-      expect(nextState.TypeA).toEqual({
-        11: { ID: 11 },
-        12: { ID: 12 },
-      });
-      expect(nextState.TypeB).toEqual({
-        11: { ID: 11 },
+    it('updates existing record', () => {
+      const nextState = recordsReducer(initialState, {
+        type: ACTION_TYPES.FETCH_RECORD_SUCCESS,
+        payload: { recordType: 'TypeA', data: { ID: 11, title: 'record a11 updated' } },
       });
+      expect(nextState.TypeA).toEqual([
+        { ID: 11, title: 'record a11 updated' },
+        { ID: 12, title: 'record a12' },
+      ]);
+      expect(nextState.TypeB).toEqual([
+        { ID: 11, title: 'record b11' },
+      ]);
     });
   });
 
   describe('DELETE_RECORD_SUCCESS', () => {
-    const initialState = {
-      TypeA: {
-        11: { ID: 11 },
-        12: { ID: 12 },
-      },
-      TypeB: {
-        11: { ID: 11 },
-        12: { ID: 12 },
-      },
-    };
+    let initialState = null;
+
+    beforeEach(() => {
+      initialState = {
+        TypeA: [{ ID: 11 }, { ID: 12 }],
+        TypeB: [{ ID: 11 }, { ID: 12 }],
+      };
+    });
 
     it('removes records from the declared type', () => {
       const nextState = recordsReducer(initialState, {
         type: ACTION_TYPES.DELETE_RECORD_SUCCESS,
         payload: { recordType: 'TypeA', id: 12 },
       });
 
-      expect(nextState.TypeA).toEqual({
-        11: { ID: 11 },
-      });
-      expect(nextState.TypeB).toEqual({
-        11: { ID: 11 },
-        12: { ID: 12 },
-      });
+      expect(nextState.TypeA).toEqual([
+        { ID: 11 },
+      ]);
+      expect(nextState.TypeB).toEqual([
+        { ID: 11 },
+        { ID: 12 },
+      ]);
     });
   });
 });