From bcf5dcf180402c89cdd0736c18de11c3fe72f140 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 27 Apr 2020 11:30:51 +0200 Subject: [PATCH] Converted server handling actions into regular actions --- src/container/store.js | 2 +- src/servers/ServersDropdown.js | 3 - src/servers/reducers/servers.js | 28 +++++-- src/servers/services/provideServices.js | 10 +-- .../{server.test.js => servers.test.js} | 83 ++++++++++--------- 5 files changed, 70 insertions(+), 56 deletions(-) rename test/servers/reducers/{server.test.js => servers.test.js} (57%) diff --git a/src/container/store.js b/src/container/store.js index 6d0de762..41e9b9ce 100644 --- a/src/container/store.js +++ b/src/container/store.js @@ -8,7 +8,7 @@ const composeEnhancers = process.env.NODE_ENV !== 'production' && window.__REDUX : compose; const localStorageConfig = { - states: [ 'settings' ], + states: [ 'settings', 'servers' ], namespace: 'shlink', namespaceSeparator: '.', }; diff --git a/src/servers/ServersDropdown.js b/src/servers/ServersDropdown.js index f6092d60..6c036a69 100644 --- a/src/servers/ServersDropdown.js +++ b/src/servers/ServersDropdown.js @@ -8,7 +8,6 @@ const ServersDropdown = (serversExporter) => class ServersDropdown extends React static propTypes = { servers: PropTypes.object, selectedServer: serverType, - listServers: PropTypes.func, history: PropTypes.shape({ push: PropTypes.func, }), @@ -39,8 +38,6 @@ const ServersDropdown = (serversExporter) => class ServersDropdown extends React ); }; - componentDidMount = this.props.listServers; - render = () => ( Servers diff --git a/src/servers/reducers/servers.js b/src/servers/reducers/servers.js index 41027c69..09e18aa4 100644 --- a/src/servers/reducers/servers.js +++ b/src/servers/reducers/servers.js @@ -1,9 +1,14 @@ import { handleActions } from 'redux-actions'; -import { pipe, isEmpty, assoc, map, prop } from 'ramda'; +import { pipe, isEmpty, assoc, map, prop, 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'; +/* eslint-enable padding-line-between-statements */ const initialState = {}; @@ -11,6 +16,11 @@ 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] + ? state + : assoc(serverId, { ...state[serverId], ...serverData }, state), }, initialState); export const listServers = ({ listServers, createServers }, { get }) => () => async (dispatch) => { @@ -42,14 +52,16 @@ export const listServers = ({ listServers, createServers }, { get }) => () => as dispatch({ type: LIST_SERVERS, list: remoteList.reduce((map, server) => ({ ...map, [server.id]: server }), {}) }); }; -export const createServer = ({ createServer }, listServersAction) => pipe(createServer, listServersAction); +export const createServer = (server) => createServers([ server ]); -export const editServer = ({ editServer }, listServersAction) => pipe(editServer, listServersAction); +const serversListToMap = reduce((acc, server) => assoc(server.id, server, acc), {}); -export const deleteServer = ({ deleteServer }, listServersAction) => pipe(deleteServer, listServersAction); - -export const createServers = ({ createServers }, listServersAction) => pipe( +export const createServers = pipe( map(assocId), - createServers, - listServersAction + serversListToMap, + (newServers) => ({ type: CREATE_SERVERS, newServers }) ); + +export const editServer = (serverId, serverData) => ({ type: EDIT_SERVER, serverId, serverData }); + +export const deleteServer = ({ id }) => ({ type: DELETE_SERVER, serverId: id }); diff --git a/src/servers/services/provideServices.js b/src/servers/services/provideServices.js index fd3fcec7..ba8e1a88 100644 --- a/src/servers/services/provideServices.js +++ b/src/servers/services/provideServices.js @@ -23,7 +23,7 @@ const provideServices = (bottle, connect, withRouter) => { bottle.serviceFactory('ServersDropdown', ServersDropdown, 'ServersExporter'); bottle.decorator('ServersDropdown', withRouter); - bottle.decorator('ServersDropdown', connect([ 'servers', 'selectedServer' ], [ 'listServers' ])); + bottle.decorator('ServersDropdown', connect([ 'servers', 'selectedServer' ])); bottle.serviceFactory('DeleteServerModal', () => DeleteServerModal); bottle.decorator('DeleteServerModal', withRouter); @@ -48,10 +48,10 @@ const provideServices = (bottle, connect, withRouter) => { // Actions bottle.serviceFactory('selectServer', selectServer, 'ServersService', 'buildShlinkApiClient', 'loadMercureInfo'); - bottle.serviceFactory('createServer', createServer, 'ServersService', 'listServers'); - bottle.serviceFactory('createServers', createServers, 'ServersService', 'listServers'); - bottle.serviceFactory('deleteServer', deleteServer, 'ServersService', 'listServers'); - bottle.serviceFactory('editServer', editServer, 'ServersService', 'listServers'); + bottle.serviceFactory('createServer', () => createServer); + bottle.serviceFactory('createServers', () => createServers); + bottle.serviceFactory('deleteServer', () => deleteServer); + bottle.serviceFactory('editServer', () => editServer); bottle.serviceFactory('listServers', listServers, 'ServersService', 'axios'); bottle.serviceFactory('resetSelectedServer', () => resetSelectedServer); diff --git a/test/servers/reducers/server.test.js b/test/servers/reducers/servers.test.js similarity index 57% rename from test/servers/reducers/server.test.js rename to test/servers/reducers/servers.test.js index 2a68bbce..24710c54 100644 --- a/test/servers/reducers/server.test.js +++ b/test/servers/reducers/servers.test.js @@ -6,6 +6,9 @@ import reducer, { createServers, editServer, LIST_SERVERS, + EDIT_SERVER, + DELETE_SERVER, + CREATE_SERVERS, } from '../../../src/servers/reducers/servers'; describe('serverReducer', () => { @@ -27,10 +30,36 @@ describe('serverReducer', () => { 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, + { type: EDIT_SERVER, serverId: 'abc123', serverData: { foo: 'foo' } }, + )).toEqual({ + abc123: { id: 'abc123', foo: 'foo' }, + def456: { id: 'def456' }, + })); + + it('removes server when action is DELETE_SERVER', () => + expect(reducer(list, { type: DELETE_SERVER, serverId: 'abc123' })).toEqual({ + def456: { id: 'def456' }, + })); + + it('appends server when action is CREATE_SERVERS', () => + expect(reducer(list, { + type: CREATE_SERVERS, + newServers: { + ghi789: { id: 'ghi789' }, + }, + })).toEqual({ + abc123: { id: 'abc123' }, + def456: { id: 'def456' }, + ghi789: { id: 'ghi789' }, + })); }); describe('action creators', () => { - describe('listServers', () => { + xdescribe('listServers', () => { const axios = { get: jest.fn() }; const dispatch = jest.fn(); const NoListServersServiceMock = { ...ServersServiceMock, listServers: jest.fn(() => ({})) }; @@ -100,62 +129,38 @@ describe('serverReducer', () => { }); describe('createServer', () => { - it('adds new server and then fetches servers again', () => { + it('returns expected action', () => { const serverToCreate = { id: 'abc123' }; - const result = createServer(ServersServiceMock, () => expectedFetchServersResult)(serverToCreate); + const result = createServer(serverToCreate); - expect(result).toEqual(expectedFetchServersResult); - expect(ServersServiceMock.listServers).not.toHaveBeenCalled(); - expect(ServersServiceMock.createServer).toHaveBeenCalledTimes(1); - expect(ServersServiceMock.createServer).toHaveBeenCalledWith(serverToCreate); - expect(ServersServiceMock.editServer).not.toHaveBeenCalled(); - expect(ServersServiceMock.deleteServer).not.toHaveBeenCalled(); - expect(ServersServiceMock.createServers).not.toHaveBeenCalled(); + expect(result).toEqual(expect.objectContaining({ type: CREATE_SERVERS })); }); }); describe('editServer', () => { - it('edits existing server and then fetches servers again', () => { - const serverToEdit = { name: 'edited' }; - const result = editServer(ServersServiceMock, () => expectedFetchServersResult)('123', serverToEdit); + it('returns expected action', () => { + const serverData = { name: 'edited' }; + const result = editServer('123', serverData); - expect(result).toEqual(expectedFetchServersResult); - expect(ServersServiceMock.listServers).not.toHaveBeenCalled(); - expect(ServersServiceMock.createServer).not.toHaveBeenCalled(); - expect(ServersServiceMock.editServer).toHaveBeenCalledTimes(1); - expect(ServersServiceMock.editServer).toHaveBeenCalledWith('123', serverToEdit); - expect(ServersServiceMock.deleteServer).not.toHaveBeenCalled(); - expect(ServersServiceMock.createServers).not.toHaveBeenCalled(); + expect(result).toEqual({ type: EDIT_SERVER, serverId: '123', serverData }); }); }); describe('deleteServer', () => { - it('deletes a server and then fetches servers again', () => { + it('returns expected action', () => { const serverToDelete = { id: 'abc123' }; - const result = deleteServer(ServersServiceMock, () => expectedFetchServersResult)(serverToDelete); + const result = deleteServer(serverToDelete); - expect(result).toEqual(expectedFetchServersResult); - expect(ServersServiceMock.listServers).not.toHaveBeenCalled(); - expect(ServersServiceMock.createServer).not.toHaveBeenCalled(); - expect(ServersServiceMock.createServers).not.toHaveBeenCalled(); - expect(ServersServiceMock.editServer).not.toHaveBeenCalled(); - expect(ServersServiceMock.deleteServer).toHaveBeenCalledTimes(1); - expect(ServersServiceMock.deleteServer).toHaveBeenCalledWith(serverToDelete); + expect(result).toEqual({ type: DELETE_SERVER, serverId: 'abc123' }); }); }); describe('createServers', () => { - it('creates multiple servers and then fetches servers again', () => { - const serversToCreate = values(list); - const result = createServers(ServersServiceMock, () => expectedFetchServersResult)(serversToCreate); + it('returns expected action', () => { + const newServers = values(list); + const result = createServers(newServers); - expect(result).toEqual(expectedFetchServersResult); - expect(ServersServiceMock.listServers).not.toHaveBeenCalled(); - expect(ServersServiceMock.createServer).not.toHaveBeenCalled(); - expect(ServersServiceMock.editServer).not.toHaveBeenCalled(); - expect(ServersServiceMock.deleteServer).not.toHaveBeenCalled(); - expect(ServersServiceMock.createServers).toHaveBeenCalledTimes(1); - expect(ServersServiceMock.createServers).toHaveBeenCalledWith(serversToCreate); + expect(result).toEqual(expect.objectContaining({ type: CREATE_SERVERS })); }); }); });