Refactor sap system and database reducers (#1745)

* Refactor sap system and database reducers

* Destructure and rename signature variables

* Rename to instancesMatch

assets/js/state/databases.js

@@ -1,5 +1,5 @@
 import { createSlice } from '@reduxjs/toolkit';
-import { upsertInstances, maybeUpdateInstanceHealth } from './instances';
+import { instancesMatch, upsertInstances, updateInstance } from './instances';
 
 const initialState = {
   loading: false,
@@ -11,10 +11,10 @@ export const databasesListSlice = createSlice({
   name: 'databasesList',
   initialState,
   reducers: {
-    setDatabases: (state, { payload }) => {
-      state.databases = payload;
+    setDatabases: (state, { payload: databases }) => {
+      state.databases = databases;
 
-      state.databaseInstances = payload.flatMap(
+      state.databaseInstances = databases.flatMap(
         (database) => database.database_instances
       );
     },
@@ -24,14 +24,8 @@ export const databasesListSlice = createSlice({
     stopDatabasesLoading: (state) => {
       state.loading = false;
     },
-    appendDatabase: (state, action) => {
-      state.databases = [...state.databases, action.payload];
-    },
-    upsertDatabaseInstances: (state, action) => {
-      state.databaseInstances = upsertInstances(
-        state.databaseInstances,
-        action.payload
-      );
+    appendDatabase: (state, { payload: newDatabase }) => {
+      state.databases = [...state.databases, newDatabase];
     },
     removeDatabase: (state, { payload: { id } }) => {
       state.databases = state.databases.filter(
@@ -41,64 +35,60 @@ export const databasesListSlice = createSlice({
         (databaseInstance) => databaseInstance.sap_system_id !== id
       );
     },
-    removeDatabaseInstance: (
-      state,
-      { payload: { sap_system_id, host_id, instance_number } }
-    ) => {
-      state.databaseInstances = state.databaseInstances.filter(
-        (databaseInstance) =>
-          !(
-            databaseInstance.sap_system_id === sap_system_id &&
-            databaseInstance.host_id === host_id &&
-            databaseInstance.instance_number === instance_number
-          )
-      );
-    },
-    updateDatabaseHealth: (state, action) => {
+    updateDatabaseHealth: (state, { payload: { id, health } }) => {
       state.databases = state.databases.map((database) => {
-        if (database.id === action.payload.id) {
-          database.health = action.payload.health;
+        if (database.id === id) {
+          database.health = health;
         }
         return database;
       });
     },
-    updateDatabaseInstanceHealth: (state, action) => {
-      state.databaseInstances = state.databaseInstances.map((instance) =>
-        maybeUpdateInstanceHealth(action.payload, instance)
-      );
-    },
-    updateDatabaseInstanceSystemReplication: (state, action) => {
-      state.databaseInstances = state.databaseInstances.map((instance) => {
-        if (
-          action.payload.sap_system_id === instance.sap_system_id &&
-          action.payload.host_id === instance.host_id &&
-          action.payload.instance_number === instance.instance_number
-        ) {
-          instance.system_replication = action.payload.system_replication;
-          instance.system_replication_status =
-            action.payload.system_replication_status;
-        }
-        return instance;
-      });
-    },
-    addTagToDatabase: (state, action) => {
+    addTagToDatabase: (state, { payload: { id, tags } }) => {
       state.databases = state.databases.map((database) => {
-        if (database.id === action.payload.id) {
-          database.tags = [...database.tags, ...action.payload.tags];
+        if (database.id === id) {
+          database.tags = [...database.tags, ...tags];
         }
         return database;
       });
     },
-    removeTagFromDatabase: (state, action) => {
+    removeTagFromDatabase: (state, { payload: { id, tags } }) => {
       state.databases = state.databases.map((database) => {
-        if (database.id === action.payload.id) {
+        if (database.id === id) {
           database.tags = database.tags.filter(
-            (tag) => tag.value !== action.payload.tags[0].value
+            (tag) => tag.value !== tags[0].value
           );
         }
         return database;
       });
     },
+    upsertDatabaseInstances: (state, { payload: instances }) => {
+      state.databaseInstances = upsertInstances(
+        state.databaseInstances,
+        instances
+      );
+    },
+    removeDatabaseInstance: (state, { payload: instance }) => {
+      state.databaseInstances = state.databaseInstances.filter(
+        (databaseInstance) => !instancesMatch(databaseInstance, instance)
+      );
+    },
+    updateDatabaseInstanceHealth: (state, { payload: instance }) => {
+      state.databaseInstances = updateInstance(
+        state.databaseInstances,
+        instance,
+        { health: instance.health }
+      );
+    },
+    updateDatabaseInstanceSystemReplication: (state, { payload: instance }) => {
+      state.databaseInstances = updateInstance(
+        state.databaseInstances,
+        instance,
+        {
+          system_replication: instance.system_replication,
+          system_replication_status: instance.system_replication_status,
+        }
+      );
+    },
   },
 });
 

assets/js/state/databases.test.js

@@ -2,28 +2,15 @@ import databaseReducer, {
   removeDatabase,
   removeDatabaseInstance,
   upsertDatabaseInstances,
+  updateDatabaseInstanceHealth,
+  updateDatabaseInstanceSystemReplication,
 } from '@state/databases';
 import {
   databaseFactory,
   databaseInstanceFactory,
 } from '@lib/test-utils/factories/databases';
 
 describe('Databases reducer', () => {
-  it('should remove a database instance from state', () => {
-    const [instance1, instance2] = databaseInstanceFactory.buildList(2);
-    const initialState = {
-      databaseInstances: [instance1, instance2],
-    };
-
-    const action = removeDatabaseInstance(instance1);
-
-    const expectedState = {
-      databaseInstances: [instance2],
-    };
-
-    expect(databaseReducer(initialState, action)).toEqual(expectedState);
-  });
-
   it('should remove a database from state', () => {
     const [database1, database2] = databaseFactory.buildList(2);
     const database1DatabaseInstances = databaseInstanceFactory.buildList({
@@ -49,6 +36,21 @@ describe('Databases reducer', () => {
     expect(databaseReducer(initialState, action)).toEqual(expectedState);
   });
 
+  it('should remove a database instance from state', () => {
+    const [instance1, instance2] = databaseInstanceFactory.buildList(2);
+    const initialState = {
+      databaseInstances: [instance1, instance2],
+    };
+
+    const action = removeDatabaseInstance(instance1);
+
+    const expectedState = {
+      databaseInstances: [instance2],
+    };
+
+    expect(databaseReducer(initialState, action)).toEqual(expectedState);
+  });
+
   it('should upsert database instances', () => {
     const initialInstances = databaseInstanceFactory.buildList(2);
 
@@ -71,4 +73,57 @@ describe('Databases reducer', () => {
 
     expect(databaseReducer(initialState, action)).toEqual(expectedState);
   });
+
+  it('should update the health of a database instance', () => {
+    const instance = databaseInstanceFactory.build();
+    const newHealth = 'newHealth';
+
+    const initialState = {
+      databaseInstances: [instance],
+    };
+
+    const instanceToUpdate = {
+      sap_system_id: instance.sap_system_id,
+      instance_number: instance.instance_number,
+      host_id: instance.host_id,
+      health: newHealth,
+    };
+    const action = updateDatabaseInstanceHealth(instanceToUpdate);
+
+    const expectedState = {
+      databaseInstances: [{ ...instance, health: newHealth }],
+    };
+
+    expect(databaseReducer(initialState, action)).toEqual(expectedState);
+  });
+
+  it('should update the system replication data of a database instance', () => {
+    const instance = databaseInstanceFactory.build();
+    const newSystemReplication = 'newSR';
+    const newStatus = 'newStatus';
+
+    const initialState = {
+      databaseInstances: [instance],
+    };
+
+    const instanceToUpdate = {
+      ...instance,
+      system_replication: newSystemReplication,
+      system_replication_status: newStatus,
+    };
+
+    const action = updateDatabaseInstanceSystemReplication(instanceToUpdate);
+
+    const expectedState = {
+      databaseInstances: [
+        {
+          ...instance,
+          system_replication: newSystemReplication,
+          system_replication_status: newStatus,
+        },
+      ],
+    };
+
+    expect(databaseReducer(initialState, action)).toEqual(expectedState);
+  });
 });

assets/js/state/instances.js

@@ -1,21 +1,22 @@
-const payloadMatchesInstance = (payload, instance) =>
-  payload.sap_system_id === instance.sap_system_id &&
-  payload.host_id === instance.host_id &&
-  payload.instance_number === instance.instance_number;
+export const instancesMatch = (instance1, instance2) =>
+  instance1.sap_system_id === instance2.sap_system_id &&
+  instance1.host_id === instance2.host_id &&
+  instance1.instance_number === instance2.instance_number;
 
 const filterByInstances = (currentInstances, newInstances) =>
   currentInstances.filter((currentInstance) =>
     newInstances.every(
-      (newInstance) => !payloadMatchesInstance(newInstance, currentInstance)
+      (newInstance) => !instancesMatch(newInstance, currentInstance)
     )
   );
 
 export const upsertInstances = (currentInstances, newInstances) =>
   filterByInstances(currentInstances, newInstances).concat(newInstances);
 
-export const maybeUpdateInstanceHealth = (payload, instance) => {
-  if (payloadMatchesInstance(payload, instance)) {
-    instance.health = payload.health;
-  }
-  return instance;
-};
+export const updateInstance = (instances, instanceToUpdate, data) =>
+  instances.map((instance) => {
+    if (instancesMatch(instanceToUpdate, instance)) {
+      return { ...instance, ...data };
+    }
+    return instance;
+  });