From b08c6748c721ccf76e1c547519cda29794d421e5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 27 Apr 2020 12:54:52 +0200 Subject: [PATCH] Moved remote servers loading to separated action --- src/App.js | 47 ++++++++---- src/container/index.js | 1 + src/servers/reducers/remoteServers.js | 22 ++++++ src/servers/reducers/servers.js | 34 +-------- src/servers/services/provideServices.js | 5 +- test/servers/reducers/remoteServers.test.js | 55 ++++++++++++++ test/servers/reducers/servers.test.js | 82 --------------------- 7 files changed, 114 insertions(+), 132 deletions(-) create mode 100644 src/servers/reducers/remoteServers.js create mode 100644 test/servers/reducers/remoteServers.test.js diff --git a/src/App.js b/src/App.js index edb9df36..5ed710e8 100644 --- a/src/App.js +++ b/src/App.js @@ -1,23 +1,40 @@ -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 ( +
+ + +
+ + + + + + + + +
-
-); + ); +}; + +App.propTypes = propTypes; export default App; diff --git a/src/container/index.js b/src/container/index.js index 3a8ac83a..51309435 100644 --- a/src/container/index.js +++ b/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); diff --git a/src/servers/reducers/remoteServers.js b/src/servers/reducers/remoteServers.js new file mode 100644 index 00000000..675c36e7 --- /dev/null +++ b/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)); +}; diff --git a/src/servers/reducers/servers.js b/src/servers/reducers/servers.js index 09e18aa4..61c60ac4 100644 --- a/src/servers/reducers/servers.js +++ b/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), {}); diff --git a/src/servers/services/provideServices.js b/src/servers/services/provideServices.js index ba8e1a88..9168bc52 100644 --- a/src/servers/services/provideServices.js +++ b/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); }; diff --git a/test/servers/reducers/remoteServers.test.js b/test/servers/reducers/remoteServers.test.js new file mode 100644 index 00000000..ba4afc80 --- /dev/null +++ b/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(''), {}], + [ 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); + }); + }); +}); diff --git a/test/servers/reducers/servers.test.js b/test/servers/reducers/servers.test.js index 24710c54..319e18d2 100644 --- a/test/servers/reducers/servers.test.js +++ b/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(''), {}], - [ 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' };