Moved remote servers loading to separated action

src/App.js

@@ -1,9 +1,23 @@
-import React from 'react';
+import React, { useEffect } from 'react';
+import PropTypes from 'prop-types';
 import { Route, Switch } from 'react-router-dom';
 import NotFound from './common/NotFound';
 import './App.scss';
 
-const App = (MainHeader, Home, MenuLayout, CreateServer, EditServer, Settings) => () => (
+const propTypes = {
+  fetchServers: PropTypes.func,
+  servers: PropTypes.object,
+};
+
+const App = (MainHeader, Home, MenuLayout, CreateServer, EditServer, Settings) => ({ fetchServers, servers }) => {
+  // On first load, try to fetch the remote servers if the list is empty
+  useEffect(() => {
+    if (Object.keys(servers).length === 0) {
+      fetchServers();
+    }
+  }, []);
+
+  return (
     <div className="container-fluid app-container">
       <MainHeader />
 
@@ -19,5 +33,8 @@ const App = (MainHeader, Home, MenuLayout, CreateServer, EditServer, Settings) =
       </div>
     </div>
   );
+};
+
+App.propTypes = propTypes;
 
 export default App;

src/container/index.js

@@ -29,6 +29,7 @@ const connect = (propsFromState, actionServiceNames = []) =>
   );
 
 bottle.serviceFactory('App', App, 'MainHeader', 'Home', 'MenuLayout', 'CreateServer', 'EditServer', 'Settings');
+bottle.decorator('App', connect([ 'servers' ], [ 'fetchServers' ]));
 
 provideCommonServices(bottle, connect, withRouter);
 provideShortUrlsServices(bottle, connect);

src/servers/reducers/remoteServers.js

@@ -0,0 +1,22 @@
+import { pipe, prop } from 'ramda';
+import { homepage } from '../../../package.json';
+import { createServers } from './servers';
+
+const responseToServersList = pipe(
+  prop('data'),
+  (value) => {
+    if (!Array.isArray(value)) {
+      throw new Error('Value is not an array');
+    }
+
+    return value;
+  },
+);
+
+export const fetchServers = ({ get }) => () => async (dispatch) => {
+  const remoteList = await get(`${homepage}/servers.json`)
+    .then(responseToServersList)
+    .catch(() => []);
+
+  dispatch(createServers(remoteList));
+};

src/servers/reducers/servers.js

@@ -1,10 +1,8 @@
 import { handleActions } from 'redux-actions';
-import { pipe, isEmpty, assoc, map, prop, reduce, dissoc } from 'ramda';
+import { pipe, assoc, map, reduce, dissoc } from 'ramda';
 import { v4 as uuid } from 'uuid';
-import { homepage } from '../../../package.json';
 
 /* eslint-disable padding-line-between-statements */
-export const LIST_SERVERS = 'shlink/servers/LIST_SERVERS';
 export const EDIT_SERVER = 'shlink/servers/EDIT_SERVER';
 export const DELETE_SERVER = 'shlink/servers/DELETE_SERVER';
 export const CREATE_SERVERS = 'shlink/servers/CREATE_SERVERS';
@@ -15,7 +13,6 @@ const initialState = {};
 const assocId = (server) => assoc('id', server.id || uuid(), server);
 
 export default handleActions({
-  [LIST_SERVERS]: (state, { list }) => list,
   [CREATE_SERVERS]: (state, { newServers }) => ({ ...state, ...newServers }),
   [DELETE_SERVER]: (state, { serverId }) => dissoc(serverId, state),
   [EDIT_SERVER]: (state, { serverId, serverData }) => !state[serverId]
@@ -23,35 +20,6 @@ export default handleActions({
     : assoc(serverId, { ...state[serverId], ...serverData }, state),
 }, initialState);
 
-export const listServers = ({ listServers, createServers }, { get }) => () => async (dispatch) => {
-  const localList = listServers();
-
-  if (!isEmpty(localList)) {
-    dispatch({ type: LIST_SERVERS, list: localList });
-
-    return;
-  }
-
-  // If local list is empty, try to fetch it remotely (making sure it's an array) and calculate IDs for every server
-  const getDataAsArrayWithIds = pipe(
-    prop('data'),
-    (value) => {
-      if (!Array.isArray(value)) {
-        throw new Error('Value is not an array');
-      }
-
-      return value;
-    },
-    map(assocId),
-  );
-  const remoteList = await get(`${homepage}/servers.json`)
-    .then(getDataAsArrayWithIds)
-    .catch(() => []);
-
-  createServers(remoteList);
-  dispatch({ type: LIST_SERVERS, list: remoteList.reduce((map, server) => ({ ...map, [server.id]: server }), {}) });
-};
-
 export const createServer = (server) => createServers([ server ]);
 
 const serversListToMap = reduce((acc, server) => assoc(server.id, server, acc), {});

src/servers/services/provideServices.js

@@ -6,7 +6,8 @@ import DeleteServerButton from '../DeleteServerButton';
 import { EditServer } from '../EditServer';
 import ImportServersBtn from '../helpers/ImportServersBtn';
 import { resetSelectedServer, selectServer } from '../reducers/selectedServer';
-import { createServer, createServers, deleteServer, editServer, listServers } from '../reducers/servers';
+import { createServer, createServers, deleteServer, editServer } from '../reducers/servers';
+import { fetchServers } from '../reducers/remoteServers';
 import ForServerVersion from '../helpers/ForServerVersion';
 import { ServerError } from '../helpers/ServerError';
 import ServersImporter from './ServersImporter';
@@ -52,7 +53,7 @@ const provideServices = (bottle, connect, withRouter) => {
   bottle.serviceFactory('createServers', () => createServers);
   bottle.serviceFactory('deleteServer', () => deleteServer);
   bottle.serviceFactory('editServer', () => editServer);
-  bottle.serviceFactory('listServers', listServers, 'ServersService', 'axios');
+  bottle.serviceFactory('fetchServers', fetchServers, 'axios');
 
   bottle.serviceFactory('resetSelectedServer', () => resetSelectedServer);
 };

test/servers/reducers/remoteServers.test.js

@@ -0,0 +1,55 @@
+import { fetchServers } from '../../../src/servers/reducers/remoteServers';
+import { CREATE_SERVERS } from '../../../src/servers/reducers/servers';
+
+describe('remoteServersReducer', () => {
+  afterEach(jest.resetAllMocks);
+
+  describe('fetchServers', () => {
+    const axios = { get: jest.fn() };
+    const dispatch = jest.fn();
+
+    it.each([
+      [
+        Promise.resolve({
+          data: [
+            {
+              id: '111',
+              name: 'acel.me from servers.json',
+              url: 'https://acel.me',
+              apiKey: '07fb8a96-8059-4094-a24c-80a7d5e7e9b0',
+            },
+            {
+              id: '222',
+              name: 'Local from servers.json',
+              url: 'http://localhost:8000',
+              apiKey: '7a531c75-134e-4d5c-86e0-a71b7167b57a',
+            },
+          ],
+        }),
+        {
+          111: {
+            id: '111',
+            name: 'acel.me from servers.json',
+            url: 'https://acel.me',
+            apiKey: '07fb8a96-8059-4094-a24c-80a7d5e7e9b0',
+          },
+          222: {
+            id: '222',
+            name: 'Local from servers.json',
+            url: 'http://localhost:8000',
+            apiKey: '7a531c75-134e-4d5c-86e0-a71b7167b57a',
+          },
+        },
+      ],
+      [ Promise.resolve('<html></html>'), {}],
+      [ Promise.reject({}), {}],
+    ])('tries to fetch servers from remote', async (mockedValue, expectedList) => {
+      axios.get.mockReturnValue(mockedValue);
+
+      await fetchServers(axios)()(dispatch);
+
+      expect(dispatch).toHaveBeenCalledWith({ type: CREATE_SERVERS, newServers: expectedList });
+      expect(axios.get).toHaveBeenCalledTimes(1);
+    });
+  });
+});

test/servers/reducers/servers.test.js

@@ -2,10 +2,8 @@ import { values } from 'ramda';
 import reducer, {
   createServer,
   deleteServer,
-  listServers,
   createServers,
   editServer,
-  LIST_SERVERS,
   EDIT_SERVER,
   DELETE_SERVER,
   CREATE_SERVERS,
@@ -16,21 +14,10 @@ describe('serverReducer', () => {
     abc123: { id: 'abc123' },
     def456: { id: 'def456' },
   };
-  const expectedFetchServersResult = { type: LIST_SERVERS, list };
-  const ServersServiceMock = {
-    listServers: jest.fn(() => list),
-    createServer: jest.fn(),
-    editServer: jest.fn(),
-    deleteServer: jest.fn(),
-    createServers: jest.fn(),
-  };
 
   afterEach(jest.clearAllMocks);
 
   describe('reducer', () => {
-    it('returns servers when action is LIST_SERVERS', () =>
-      expect(reducer({}, { type: LIST_SERVERS, list })).toEqual(list));
-
     it('returns edited server when action is EDIT_SERVER', () =>
       expect(reducer(
         list,
@@ -59,75 +46,6 @@ describe('serverReducer', () => {
   });
 
   describe('action creators', () => {
-    xdescribe('listServers', () => {
-      const axios = { get: jest.fn() };
-      const dispatch = jest.fn();
-      const NoListServersServiceMock = { ...ServersServiceMock, listServers: jest.fn(() => ({})) };
-
-      it('fetches servers from local storage when found', async () => {
-        await listServers(ServersServiceMock, axios)()(dispatch);
-
-        expect(dispatch).toHaveBeenCalledTimes(1);
-        expect(dispatch).toHaveBeenNthCalledWith(1, expectedFetchServersResult);
-        expect(ServersServiceMock.listServers).toHaveBeenCalledTimes(1);
-        expect(ServersServiceMock.createServer).not.toHaveBeenCalled();
-        expect(ServersServiceMock.editServer).not.toHaveBeenCalled();
-        expect(ServersServiceMock.deleteServer).not.toHaveBeenCalled();
-        expect(ServersServiceMock.createServers).not.toHaveBeenCalled();
-        expect(axios.get).not.toHaveBeenCalled();
-      });
-
-      it.each([
-        [
-          Promise.resolve({
-            data: [
-              {
-                id: '111',
-                name: 'acel.me from servers.json',
-                url: 'https://acel.me',
-                apiKey: '07fb8a96-8059-4094-a24c-80a7d5e7e9b0',
-              },
-              {
-                id: '222',
-                name: 'Local from servers.json',
-                url: 'http://localhost:8000',
-                apiKey: '7a531c75-134e-4d5c-86e0-a71b7167b57a',
-              },
-            ],
-          }),
-          {
-            111: {
-              id: '111',
-              name: 'acel.me from servers.json',
-              url: 'https://acel.me',
-              apiKey: '07fb8a96-8059-4094-a24c-80a7d5e7e9b0',
-            },
-            222: {
-              id: '222',
-              name: 'Local from servers.json',
-              url: 'http://localhost:8000',
-              apiKey: '7a531c75-134e-4d5c-86e0-a71b7167b57a',
-            },
-          },
-        ],
-        [ Promise.resolve('<html></html>'), {}],
-        [ Promise.reject({}), {}],
-      ])('tries to fetch servers from remote when not found locally', async (mockedValue, expectedList) => {
-        axios.get.mockReturnValue(mockedValue);
-
-        await listServers(NoListServersServiceMock, axios)()(dispatch);
-
-        expect(dispatch).toHaveBeenCalledTimes(1);
-        expect(dispatch).toHaveBeenNthCalledWith(1, { type: LIST_SERVERS, list: expectedList });
-        expect(NoListServersServiceMock.listServers).toHaveBeenCalledTimes(1);
-        expect(NoListServersServiceMock.createServer).not.toHaveBeenCalled();
-        expect(NoListServersServiceMock.editServer).not.toHaveBeenCalled();
-        expect(NoListServersServiceMock.deleteServer).not.toHaveBeenCalled();
-        expect(NoListServersServiceMock.createServers).toHaveBeenCalledTimes(1);
-        expect(axios.get).toHaveBeenCalledTimes(1);
-      });
-    });
-
     describe('createServer', () => {
       it('returns expected action', () => {
         const serverToCreate = { id: 'abc123' };