From 2e0e24d87b72f0485712df6bee438273847a2cbd Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 10 Nov 2022 20:44:58 +0100 Subject: [PATCH 1/5] Migrated fetchServers to RTK --- src/servers/reducers/remoteServers.ts | 15 +++++++++------ test/servers/reducers/remoteServers.test.ts | 13 ++++++++++--- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/servers/reducers/remoteServers.ts b/src/servers/reducers/remoteServers.ts index 52e1bc9f..8f5083a8 100644 --- a/src/servers/reducers/remoteServers.ts +++ b/src/servers/reducers/remoteServers.ts @@ -1,18 +1,21 @@ import { pipe, prop } from 'ramda'; import { AxiosInstance } from 'axios'; -import { Dispatch } from 'redux'; import pack from '../../../package.json'; import { hasServerData, ServerData } from '../data'; import { createServers } from './servers'; +import { createAsyncThunk } from '../../utils/helpers/redux'; const responseToServersList = pipe( prop('data'), (data: any): ServerData[] => (Array.isArray(data) ? data.filter(hasServerData) : []), ); -export const fetchServers = ({ get }: AxiosInstance) => () => async (dispatch: Dispatch) => { - const resp = await get(`${pack.homepage}/servers.json`); - const remoteList = responseToServersList(resp); +export const fetchServers = ({ get }: AxiosInstance) => createAsyncThunk( + 'shlink/remoteServers/fetchServers', + async (_: void, { dispatch }): Promise => { + const resp = await get(`${pack.homepage}/servers.json`); + const result = responseToServersList(resp); - dispatch(createServers(remoteList)); -}; + dispatch(createServers(result)); + }, +); diff --git a/test/servers/reducers/remoteServers.test.ts b/test/servers/reducers/remoteServers.test.ts index 708e2905..41e19238 100644 --- a/test/servers/reducers/remoteServers.test.ts +++ b/test/servers/reducers/remoteServers.test.ts @@ -7,9 +7,9 @@ describe('remoteServersReducer', () => { afterEach(jest.clearAllMocks); describe('fetchServers', () => { + const dispatch = jest.fn(); const get = jest.fn(); const axios = Mock.of({ get }); - const dispatch = jest.fn(); it.each([ [ @@ -84,10 +84,17 @@ describe('remoteServersReducer', () => { [{}, {}], ])('tries to fetch servers from remote', async (mockedValue, expectedNewServers) => { get.mockResolvedValue(mockedValue); + const doFetchServers = fetchServers(axios); - await fetchServers(axios)()(dispatch); + await doFetchServers()(dispatch, jest.fn(), {}); - expect(dispatch).toHaveBeenCalledWith({ type: createServers.toString(), payload: expectedNewServers }); + expect(dispatch).toHaveBeenNthCalledWith(1, expect.objectContaining({ + type: doFetchServers.pending.toString(), + })); + expect(dispatch).toHaveBeenNthCalledWith(2, { type: createServers.toString(), payload: expectedNewServers }); + expect(dispatch).toHaveBeenNthCalledWith(3, expect.objectContaining({ + type: doFetchServers.fulfilled.toString(), + })); expect(get).toHaveBeenCalledTimes(1); }); }); From 6221f9ed059a512fe355dec18c6dcb741763d3f7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 10 Nov 2022 22:44:25 +0100 Subject: [PATCH 2/5] Migrated selectServer action to RTK and moved loadMercureInfo to an action listener --- src/container/store.ts | 2 +- src/reducers/index.ts | 2 +- src/servers/reducers/selectedServer.ts | 65 ++++++++++---------- src/servers/services/provideServices.ts | 5 +- src/visits/reducers/visitCreation.ts | 4 +- test/servers/reducers/selectedServer.test.ts | 61 +++++++++--------- 6 files changed, 71 insertions(+), 68 deletions(-) diff --git a/src/container/store.ts b/src/container/store.ts index 5f214f59..7af11755 100644 --- a/src/container/store.ts +++ b/src/container/store.ts @@ -20,5 +20,5 @@ export const setUpStore = (container: IContainer) => configureStore({ preloadedState, middleware: (defaultMiddlewaresIncludingReduxThunk) => defaultMiddlewaresIncludingReduxThunk( { immutableCheck: false, serializableCheck: false }, // State is too big for these - ).concat(save(localStorageConfig)), + ).prepend(container.selectServerListener.middleware).concat(save(localStorageConfig)), }); diff --git a/src/reducers/index.ts b/src/reducers/index.ts index 66ff189c..6d0d71fd 100644 --- a/src/reducers/index.ts +++ b/src/reducers/index.ts @@ -1,5 +1,5 @@ import { IContainer } from 'bottlejs'; -import { combineReducers } from 'redux'; +import { combineReducers } from '@reduxjs/toolkit'; import { serversReducer } from '../servers/reducers/servers'; import selectedServerReducer from '../servers/reducers/selectedServer'; import shortUrlVisitsReducer from '../visits/reducers/shortUrlVisits'; diff --git a/src/servers/reducers/selectedServer.ts b/src/servers/reducers/selectedServer.ts index 78c0d69d..f6ef4500 100644 --- a/src/servers/reducers/selectedServer.ts +++ b/src/servers/reducers/selectedServer.ts @@ -1,15 +1,13 @@ -import { PayloadAction } from '@reduxjs/toolkit'; +import { createAction, createListenerMiddleware, PayloadAction } from '@reduxjs/toolkit'; import { identity, memoizeWith, pipe } from 'ramda'; -import { Action, Dispatch } from 'redux'; import { versionToPrintable, versionToSemVer as toSemVer } from '../../utils/helpers/version'; -import { SelectedServer } from '../data'; -import { GetState } from '../../container/types'; +import { isReachableServer, SelectedServer } from '../data'; import { ShlinkHealth } from '../../api/types'; -import { buildActionCreator, buildReducer } from '../../utils/helpers/redux'; +import { buildReducer, createAsyncThunk } from '../../utils/helpers/redux'; import { ShlinkApiClientBuilder } from '../../api/services/ShlinkApiClientBuilder'; -export const SELECT_SERVER = 'shlink/selectedServer/SELECT_SERVER'; -export const RESET_SELECTED_SERVER = 'shlink/selectedServer/RESET_SELECTED_SERVER'; +export const SELECT_SERVER = 'shlink/selectedServer/selectServer'; +export const RESET_SELECTED_SERVER = 'shlink/selectedServer/resetSelectedServer'; export const MIN_FALLBACK_VERSION = '1.0.0'; export const MAX_FALLBACK_VERSION = '999.999.999'; @@ -35,50 +33,49 @@ const initialState: SelectedServer = null; export default buildReducer({ [RESET_SELECTED_SERVER]: () => initialState, [SELECT_SERVER]: (_, { payload }) => payload, + [`${SELECT_SERVER}/fulfilled`]: (_, { payload }) => payload, }, initialState); -export const resetSelectedServer = buildActionCreator(RESET_SELECTED_SERVER); +export const resetSelectedServer = createAction(RESET_SELECTED_SERVER); export const selectServer = ( buildShlinkApiClient: ShlinkApiClientBuilder, - loadMercureInfo: () => Action, -) => ( - serverId: string, -) => async ( - dispatch: Dispatch, - getState: GetState, -) => { +) => createAsyncThunk(SELECT_SERVER, async (serverId: string, { dispatch, getState }): Promise => { dispatch(resetSelectedServer()); const { servers } = getState(); const selectedServer = servers[serverId]; if (!selectedServer) { - dispatch({ - type: SELECT_SERVER, - payload: { serverNotFound: true }, - }); - - return; + return { serverNotFound: true }; } try { const { health } = buildShlinkApiClient(selectedServer); const { version, printableVersion } = await getServerVersion(serverId, health); - dispatch({ - type: SELECT_SERVER, - payload: { - ...selectedServer, - version, - printableVersion, - }, - }); - dispatch(loadMercureInfo()); + return { + ...selectedServer, + version, + printableVersion, + }; } catch (e) { - dispatch({ - type: SELECT_SERVER, - payload: { ...selectedServer, serverNotReachable: true }, - }); + return { ...selectedServer, serverNotReachable: true }; } +}); + +export const selectServerListener = ( + selectServerThunk: ReturnType, + loadMercureInfo: () => PayloadAction, // TODO Consider setting actual type, if relevant +) => { + const listener = createListenerMiddleware(); + + listener.startListening({ + actionCreator: selectServerThunk.fulfilled, + effect: ({ payload }, { dispatch }) => { + isReachableServer(payload) && dispatch(loadMercureInfo()); + }, + }); + + return listener; }; diff --git a/src/servers/services/provideServices.ts b/src/servers/services/provideServices.ts index f6e03f88..e9acf638 100644 --- a/src/servers/services/provideServices.ts +++ b/src/servers/services/provideServices.ts @@ -5,7 +5,7 @@ import { DeleteServerModal } from '../DeleteServerModal'; import { DeleteServerButton } from '../DeleteServerButton'; import { EditServer } from '../EditServer'; import { ImportServersBtn } from '../helpers/ImportServersBtn'; -import { resetSelectedServer, selectServer } from '../reducers/selectedServer'; +import { resetSelectedServer, selectServer, selectServerListener } from '../reducers/selectedServer'; import { createServers, deleteServer, editServer, setAutoConnect } from '../reducers/servers'; import { fetchServers } from '../reducers/remoteServers'; import { ServerError } from '../helpers/ServerError'; @@ -77,6 +77,9 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator) => { bottle.serviceFactory('fetchServers', fetchServers, 'axios'); bottle.serviceFactory('resetSelectedServer', () => resetSelectedServer); + + // Reducers + bottle.serviceFactory('selectServerListener', selectServerListener, 'selectServer', 'loadMercureInfo'); }; export default provideServices; diff --git a/src/visits/reducers/visitCreation.ts b/src/visits/reducers/visitCreation.ts index 3f7f9137..eb037bd7 100644 --- a/src/visits/reducers/visitCreation.ts +++ b/src/visits/reducers/visitCreation.ts @@ -1,13 +1,11 @@ import { createAction, PayloadAction } from '@reduxjs/toolkit'; import { CreateVisit } from '../types'; -const CREATE_VISITS = 'shlink/visitCreation/CREATE_VISITS'; - export type CreateVisitsAction = PayloadAction<{ createdVisits: CreateVisit[]; }>; export const createNewVisits = createAction( - CREATE_VISITS, + 'shlink/visitCreation/createNewVisits', (createdVisits: CreateVisit[]) => ({ payload: { createdVisits } }), ); diff --git a/test/servers/reducers/selectedServer.test.ts b/test/servers/reducers/selectedServer.test.ts index f04a4eed..313ab639 100644 --- a/test/servers/reducers/selectedServer.test.ts +++ b/test/servers/reducers/selectedServer.test.ts @@ -1,31 +1,36 @@ import { v4 as uuid } from 'uuid'; import { Mock } from 'ts-mockery'; import reducer, { - selectServer, + selectServer as selectServerCreator, resetSelectedServer, - RESET_SELECTED_SERVER, - SELECT_SERVER, MAX_FALLBACK_VERSION, MIN_FALLBACK_VERSION, } from '../../../src/servers/reducers/selectedServer'; import { ShlinkState } from '../../../src/container/types'; import { NonReachableServer, NotFoundServer, RegularServer } from '../../../src/servers/data'; +import { ShlinkApiClient } from '../../../src/api/services/ShlinkApiClient'; describe('selectedServerReducer', () => { + const health = jest.fn(); + const buildApiClient = jest.fn().mockReturnValue(Mock.of({ health })); + const selectServer = selectServerCreator(buildApiClient); + + afterEach(jest.clearAllMocks); + describe('reducer', () => { it('returns default when action is RESET_SELECTED_SERVER', () => - expect(reducer(null, { type: RESET_SELECTED_SERVER, payload: null })).toBeNull()); + expect(reducer(null, { type: resetSelectedServer.toString(), payload: null })).toBeNull()); it('returns selected server when action is SELECT_SERVER', () => { const payload = Mock.of({ id: 'abc123' }); - expect(reducer(null, { type: SELECT_SERVER, payload })).toEqual(payload); + expect(reducer(null, { type: selectServer.fulfilled.toString(), payload })).toEqual(payload); }); }); describe('resetSelectedServer', () => { it('returns proper action', () => { - expect(resetSelectedServer()).toEqual({ type: RESET_SELECTED_SERVER }); + expect(resetSelectedServer()).toEqual({ type: resetSelectedServer.toString() }); }); }); @@ -35,14 +40,7 @@ describe('selectedServerReducer', () => { }; const version = '1.19.0'; const createGetStateMock = (id: string) => jest.fn().mockReturnValue({ servers: { [id]: selectedServer } }); - const apiClientMock = { - health: jest.fn(), - }; - const buildApiClient = jest.fn().mockReturnValue(apiClientMock); const dispatch = jest.fn(); - const loadMercureInfo = jest.fn(); - - afterEach(jest.clearAllMocks); it.each([ [version, version, `v${version}`], @@ -57,21 +55,24 @@ describe('selectedServerReducer', () => { printableVersion: expectedPrintableVersion, }; - apiClientMock.health.mockResolvedValue({ version: serverVersion }); + health.mockResolvedValue({ version: serverVersion }); - await selectServer(buildApiClient, loadMercureInfo)(id)(dispatch, getState); + await selectServer(id)(dispatch, getState, {}); expect(dispatch).toHaveBeenCalledTimes(3); - expect(dispatch).toHaveBeenNthCalledWith(1, { type: RESET_SELECTED_SERVER }); - expect(dispatch).toHaveBeenNthCalledWith(2, { type: SELECT_SERVER, payload: expectedSelectedServer }); - expect(loadMercureInfo).toHaveBeenCalledTimes(1); + expect(dispatch).toHaveBeenNthCalledWith(1, expect.objectContaining({ type: selectServer.pending.toString() })); + expect(dispatch).toHaveBeenNthCalledWith(2, expect.objectContaining({ type: resetSelectedServer.toString() })); + expect(dispatch).toHaveBeenNthCalledWith(3, expect.objectContaining({ + type: selectServer.fulfilled.toString(), + payload: expectedSelectedServer, + })); }); it('invokes dependencies', async () => { const id = uuid(); const getState = createGetStateMock(id); - await selectServer(buildApiClient, loadMercureInfo)(id)(jest.fn(), getState); + await selectServer(id)(jest.fn(), getState, {}); expect(getState).toHaveBeenCalledTimes(1); expect(buildApiClient).toHaveBeenCalledTimes(1); @@ -82,13 +83,15 @@ describe('selectedServerReducer', () => { const getState = createGetStateMock(id); const expectedSelectedServer = Mock.of({ ...selectedServer, serverNotReachable: true }); - apiClientMock.health.mockRejectedValue({}); + health.mockRejectedValue({}); - await selectServer(buildApiClient, loadMercureInfo)(id)(dispatch, getState); + await selectServer(id)(dispatch, getState, {}); - expect(apiClientMock.health).toHaveBeenCalled(); - expect(dispatch).toHaveBeenNthCalledWith(2, { type: SELECT_SERVER, payload: expectedSelectedServer }); - expect(loadMercureInfo).not.toHaveBeenCalled(); + expect(health).toHaveBeenCalled(); + expect(dispatch).toHaveBeenNthCalledWith(3, expect.objectContaining({ + type: selectServer.fulfilled.toString(), + payload: expectedSelectedServer, + })); }); it('dispatches error when server is not found', async () => { @@ -96,12 +99,14 @@ describe('selectedServerReducer', () => { const getState = jest.fn(() => Mock.of({ servers: {} })); const expectedSelectedServer: NotFoundServer = { serverNotFound: true }; - await selectServer(buildApiClient, loadMercureInfo)(id)(dispatch, getState); + await selectServer(id)(dispatch, getState, {}); expect(getState).toHaveBeenCalled(); - expect(apiClientMock.health).not.toHaveBeenCalled(); - expect(dispatch).toHaveBeenNthCalledWith(2, { type: SELECT_SERVER, payload: expectedSelectedServer }); - expect(loadMercureInfo).not.toHaveBeenCalled(); + expect(health).not.toHaveBeenCalled(); + expect(dispatch).toHaveBeenNthCalledWith(3, expect.objectContaining({ + type: selectServer.fulfilled.toString(), + payload: expectedSelectedServer, + })); }); }); }); From d44fe945d806fcb9bbab58a025ced1ea55b84cf6 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 11 Nov 2022 19:31:05 +0100 Subject: [PATCH 3/5] Migrated selectedServer reducer to RTK --- src/container/store.ts | 7 +- src/reducers/index.ts | 3 +- src/servers/reducers/selectedServer.ts | 72 +++++++++++--------- src/servers/services/provideServices.ts | 10 ++- test/servers/reducers/selectedServer.test.ts | 4 +- 5 files changed, 55 insertions(+), 41 deletions(-) diff --git a/src/container/store.ts b/src/container/store.ts index 7af11755..cf48b3c2 100644 --- a/src/container/store.ts +++ b/src/container/store.ts @@ -18,7 +18,8 @@ export const setUpStore = (container: IContainer) => configureStore({ devTools: !isProduction, reducer: reducer(container), preloadedState, - middleware: (defaultMiddlewaresIncludingReduxThunk) => defaultMiddlewaresIncludingReduxThunk( - { immutableCheck: false, serializableCheck: false }, // State is too big for these - ).prepend(container.selectServerListener.middleware).concat(save(localStorageConfig)), + middleware: (defaultMiddlewaresIncludingReduxThunk) => + defaultMiddlewaresIncludingReduxThunk({ immutableCheck: false, serializableCheck: false })// State is too big for these + .prepend(container.selectServerListener.middleware) + .concat(save(localStorageConfig)), }); diff --git a/src/reducers/index.ts b/src/reducers/index.ts index 6d0d71fd..553933c1 100644 --- a/src/reducers/index.ts +++ b/src/reducers/index.ts @@ -1,7 +1,6 @@ import { IContainer } from 'bottlejs'; import { combineReducers } from '@reduxjs/toolkit'; import { serversReducer } from '../servers/reducers/servers'; -import selectedServerReducer from '../servers/reducers/selectedServer'; import shortUrlVisitsReducer from '../visits/reducers/shortUrlVisits'; import tagVisitsReducer from '../visits/reducers/tagVisits'; import domainVisitsReducer from '../visits/reducers/domainVisits'; @@ -15,7 +14,7 @@ import { ShlinkState } from '../container/types'; export default (container: IContainer) => combineReducers({ servers: serversReducer, - selectedServer: selectedServerReducer, + selectedServer: container.selectedServerReducer, shortUrlsList: container.shortUrlsListReducer, shortUrlCreation: container.shortUrlCreationReducer, shortUrlDeletion: container.shortUrlDeletionReducer, diff --git a/src/servers/reducers/selectedServer.ts b/src/servers/reducers/selectedServer.ts index f6ef4500..3415a15c 100644 --- a/src/servers/reducers/selectedServer.ts +++ b/src/servers/reducers/selectedServer.ts @@ -1,20 +1,17 @@ -import { createAction, createListenerMiddleware, PayloadAction } from '@reduxjs/toolkit'; +import { createAction, createListenerMiddleware, createSlice, PayloadAction } from '@reduxjs/toolkit'; import { identity, memoizeWith, pipe } from 'ramda'; import { versionToPrintable, versionToSemVer as toSemVer } from '../../utils/helpers/version'; import { isReachableServer, SelectedServer } from '../data'; import { ShlinkHealth } from '../../api/types'; -import { buildReducer, createAsyncThunk } from '../../utils/helpers/redux'; +import { createAsyncThunk } from '../../utils/helpers/redux'; import { ShlinkApiClientBuilder } from '../../api/services/ShlinkApiClientBuilder'; -export const SELECT_SERVER = 'shlink/selectedServer/selectServer'; -export const RESET_SELECTED_SERVER = 'shlink/selectedServer/resetSelectedServer'; +const REDUCER_PREFIX = 'shlink/selectedServer'; export const MIN_FALLBACK_VERSION = '1.0.0'; export const MAX_FALLBACK_VERSION = '999.999.999'; export const LATEST_VERSION_CONSTRAINT = 'latest'; -export type SelectServerAction = PayloadAction; - const versionToSemVer = pipe( (version: string) => (version === LATEST_VERSION_CONSTRAINT ? MAX_FALLBACK_VERSION : version), toSemVer(MIN_FALLBACK_VERSION), @@ -30,42 +27,39 @@ const getServerVersion = memoizeWith( const initialState: SelectedServer = null; -export default buildReducer({ - [RESET_SELECTED_SERVER]: () => initialState, - [SELECT_SERVER]: (_, { payload }) => payload, - [`${SELECT_SERVER}/fulfilled`]: (_, { payload }) => payload, -}, initialState); +export const resetSelectedServer = createAction(`${REDUCER_PREFIX}/resetSelectedServer`); -export const resetSelectedServer = createAction(RESET_SELECTED_SERVER); +export const selectServer = (buildShlinkApiClient: ShlinkApiClientBuilder) => createAsyncThunk( + `${REDUCER_PREFIX}/selectServer`, + async (serverId: string, { dispatch, getState }): Promise => { + dispatch(resetSelectedServer()); -export const selectServer = ( - buildShlinkApiClient: ShlinkApiClientBuilder, -) => createAsyncThunk(SELECT_SERVER, async (serverId: string, { dispatch, getState }): Promise => { - dispatch(resetSelectedServer()); + const { servers } = getState(); + const selectedServer = servers[serverId]; - const { servers } = getState(); - const selectedServer = servers[serverId]; + if (!selectedServer) { + return { serverNotFound: true }; + } - if (!selectedServer) { - return { serverNotFound: true }; - } + try { + const { health } = buildShlinkApiClient(selectedServer); + const { version, printableVersion } = await getServerVersion(serverId, health); - try { - const { health } = buildShlinkApiClient(selectedServer); - const { version, printableVersion } = await getServerVersion(serverId, health); + return { + ...selectedServer, + version, + printableVersion, + }; + } catch (e) { + return { ...selectedServer, serverNotReachable: true }; + } + }, +); - return { - ...selectedServer, - version, - printableVersion, - }; - } catch (e) { - return { ...selectedServer, serverNotReachable: true }; - } -}); +type SelectServerThunk = ReturnType; export const selectServerListener = ( - selectServerThunk: ReturnType, + selectServerThunk: SelectServerThunk, loadMercureInfo: () => PayloadAction, // TODO Consider setting actual type, if relevant ) => { const listener = createListenerMiddleware(); @@ -79,3 +73,13 @@ export const selectServerListener = ( return listener; }; + +export const selectedServerReducerCreator = (selectServerThunk: SelectServerThunk) => createSlice({ + name: REDUCER_PREFIX, + initialState, + reducers: {}, + extraReducers: (builder) => { + builder.addCase(resetSelectedServer, () => initialState); + builder.addCase(selectServerThunk.fulfilled, (_, { payload }) => payload as any); + }, +}); diff --git a/src/servers/services/provideServices.ts b/src/servers/services/provideServices.ts index e9acf638..842f931a 100644 --- a/src/servers/services/provideServices.ts +++ b/src/servers/services/provideServices.ts @@ -1,3 +1,4 @@ +import { prop } from 'ramda'; import Bottle from 'bottlejs'; import { CreateServer } from '../CreateServer'; import { ServersDropdown } from '../ServersDropdown'; @@ -5,7 +6,12 @@ import { DeleteServerModal } from '../DeleteServerModal'; import { DeleteServerButton } from '../DeleteServerButton'; import { EditServer } from '../EditServer'; import { ImportServersBtn } from '../helpers/ImportServersBtn'; -import { resetSelectedServer, selectServer, selectServerListener } from '../reducers/selectedServer'; +import { + resetSelectedServer, + selectedServerReducerCreator, + selectServer, + selectServerListener, +} from '../reducers/selectedServer'; import { createServers, deleteServer, editServer, setAutoConnect } from '../reducers/servers'; import { fetchServers } from '../reducers/remoteServers'; import { ServerError } from '../helpers/ServerError'; @@ -80,6 +86,8 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator) => { // Reducers bottle.serviceFactory('selectServerListener', selectServerListener, 'selectServer', 'loadMercureInfo'); + bottle.serviceFactory('selectedServerReducerCreator', selectedServerReducerCreator, 'selectServer'); + bottle.serviceFactory('selectedServerReducer', prop('reducer'), 'selectedServerReducerCreator'); }; export default provideServices; diff --git a/test/servers/reducers/selectedServer.test.ts b/test/servers/reducers/selectedServer.test.ts index 313ab639..37fbbefc 100644 --- a/test/servers/reducers/selectedServer.test.ts +++ b/test/servers/reducers/selectedServer.test.ts @@ -1,8 +1,9 @@ import { v4 as uuid } from 'uuid'; import { Mock } from 'ts-mockery'; -import reducer, { +import { selectServer as selectServerCreator, resetSelectedServer, + selectedServerReducerCreator, MAX_FALLBACK_VERSION, MIN_FALLBACK_VERSION, } from '../../../src/servers/reducers/selectedServer'; @@ -14,6 +15,7 @@ describe('selectedServerReducer', () => { const health = jest.fn(); const buildApiClient = jest.fn().mockReturnValue(Mock.of({ health })); const selectServer = selectServerCreator(buildApiClient); + const { reducer } = selectedServerReducerCreator(selectServer); afterEach(jest.clearAllMocks); From 002d2ba8e68b316523a9cfa09f3710d1937a1a29 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 11 Nov 2022 20:01:45 +0100 Subject: [PATCH 4/5] Added tests for selectedServerReducerCreator --- test/servers/reducers/selectedServer.test.ts | 35 ++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/test/servers/reducers/selectedServer.test.ts b/test/servers/reducers/selectedServer.test.ts index 37fbbefc..a8db0c95 100644 --- a/test/servers/reducers/selectedServer.test.ts +++ b/test/servers/reducers/selectedServer.test.ts @@ -4,14 +4,16 @@ import { selectServer as selectServerCreator, resetSelectedServer, selectedServerReducerCreator, + selectServerListener, MAX_FALLBACK_VERSION, MIN_FALLBACK_VERSION, } from '../../../src/servers/reducers/selectedServer'; import { ShlinkState } from '../../../src/container/types'; -import { NonReachableServer, NotFoundServer, RegularServer } from '../../../src/servers/data'; +import { NonReachableServer, NotFoundServer, ReachableServer, RegularServer } from '../../../src/servers/data'; import { ShlinkApiClient } from '../../../src/api/services/ShlinkApiClient'; describe('selectedServerReducer', () => { + const dispatch = jest.fn(); const health = jest.fn(); const buildApiClient = jest.fn().mockReturnValue(Mock.of({ health })); const selectServer = selectServerCreator(buildApiClient); @@ -42,7 +44,6 @@ describe('selectedServerReducer', () => { }; const version = '1.19.0'; const createGetStateMock = (id: string) => jest.fn().mockReturnValue({ servers: { [id]: selectedServer } }); - const dispatch = jest.fn(); it.each([ [version, version, `v${version}`], @@ -111,4 +112,34 @@ describe('selectedServerReducer', () => { })); }); }); + + describe('selectServerListener', () => { + const getState = jest.fn(() => ({})); + const loadMercureInfo = jest.fn(); + const { middleware } = selectServerListener(selectServer, loadMercureInfo); + + it.each([ + [Mock.of({ version: '1.2.3' }), 1], + [Mock.of({ serverNotFound: true }), 0], + [Mock.of({ serverNotReachable: true }), 0], + ])('dispatches loadMercureInfo when provided server is reachable', (payload, expectedCalls) => { + middleware({ dispatch, getState })(jest.fn())({ + payload, + type: selectServer.fulfilled.toString(), + }); + + expect(dispatch).toHaveBeenCalledTimes(expectedCalls); + expect(loadMercureInfo).toHaveBeenCalledTimes(expectedCalls); + }); + + it('does not dispatch loadMercureInfo when action is not of the proper type', () => { + middleware({ dispatch, getState })(jest.fn())({ + payload: Mock.of({ version: '1.2.3' }), + type: 'something_else', + }); + + expect(dispatch).not.toHaveBeenCalled(); + expect(loadMercureInfo).not.toHaveBeenCalled(); + }); + }); }); From 5095a2c59e5730ef7e82f8e8907e883f5a2a2101 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 11 Nov 2022 20:23:19 +0100 Subject: [PATCH 5/5] Migrated visitsOverview reducer to RTK --- src/reducers/index.ts | 3 +- src/visits/reducers/visitsOverview.ts | 66 ++++++++++----------- src/visits/services/provideServices.ts | 7 ++- test/visits/reducers/visitsOverview.test.ts | 65 ++++++++++++-------- 4 files changed, 78 insertions(+), 63 deletions(-) diff --git a/src/reducers/index.ts b/src/reducers/index.ts index 553933c1..ae1a6ba0 100644 --- a/src/reducers/index.ts +++ b/src/reducers/index.ts @@ -7,7 +7,6 @@ import domainVisitsReducer from '../visits/reducers/domainVisits'; import orphanVisitsReducer from '../visits/reducers/orphanVisits'; import nonOrphanVisitsReducer from '../visits/reducers/nonOrphanVisits'; import { settingsReducer } from '../settings/reducers/settings'; -import visitsOverviewReducer from '../visits/reducers/visitsOverview'; import { appUpdatesReducer } from '../app/reducers/appUpdates'; import { sidebarReducer } from '../common/reducers/sidebar'; import { ShlinkState } from '../container/types'; @@ -31,7 +30,7 @@ export default (container: IContainer) => combineReducers({ mercureInfo: container.mercureInfoReducer, settings: settingsReducer, domainsList: container.domainsListReducer, - visitsOverview: visitsOverviewReducer, + visitsOverview: container.visitsOverviewReducer, appUpdated: appUpdatesReducer, sidebar: sidebarReducer, }); diff --git a/src/visits/reducers/visitsOverview.ts b/src/visits/reducers/visitsOverview.ts index 824d7f5f..105bfd2b 100644 --- a/src/visits/reducers/visitsOverview.ts +++ b/src/visits/reducers/visitsOverview.ts @@ -1,14 +1,11 @@ -import { Action, Dispatch } from 'redux'; +import { createSlice, PayloadAction } from '@reduxjs/toolkit'; import { ShlinkVisitsOverview } from '../../api/types'; import { ShlinkApiClientBuilder } from '../../api/services/ShlinkApiClientBuilder'; -import { GetState } from '../../container/types'; -import { buildReducer } from '../../utils/helpers/redux'; +import { createAsyncThunk } from '../../utils/helpers/redux'; import { groupNewVisitsByType } from '../types/helpers'; -import { createNewVisits, CreateVisitsAction } from './visitCreation'; +import { createNewVisits } from './visitCreation'; -export const GET_OVERVIEW_START = 'shlink/visitsOverview/GET_OVERVIEW_START'; -export const GET_OVERVIEW_ERROR = 'shlink/visitsOverview/GET_OVERVIEW_ERROR'; -export const GET_OVERVIEW = 'shlink/visitsOverview/GET_OVERVIEW'; +const REDUCER_PREFIX = 'shlink/visitsOverview'; export interface VisitsOverview { visitsCount: number; @@ -17,7 +14,7 @@ export interface VisitsOverview { error: boolean; } -export type GetVisitsOverviewAction = ShlinkVisitsOverview & Action; +export type GetVisitsOverviewAction = PayloadAction; const initialState: VisitsOverview = { visitsCount: 0, @@ -26,33 +23,30 @@ const initialState: VisitsOverview = { error: false, }; -export default buildReducer({ - [GET_OVERVIEW_START]: () => ({ ...initialState, loading: true }), - [GET_OVERVIEW_ERROR]: () => ({ ...initialState, error: true }), - [GET_OVERVIEW]: (_, { visitsCount, orphanVisitsCount }) => ({ ...initialState, visitsCount, orphanVisitsCount }), - [createNewVisits.toString()]: ({ visitsCount, orphanVisitsCount = 0, ...rest }, { payload }) => { - const { regularVisits, orphanVisits } = groupNewVisitsByType(payload.createdVisits); +export const loadVisitsOverview = (buildShlinkApiClient: ShlinkApiClientBuilder) => createAsyncThunk( + `${REDUCER_PREFIX}/loadVisitsOverview`, + (_: void, { getState }): Promise => buildShlinkApiClient(getState).getVisitsOverview(), +); - return { - ...rest, - visitsCount: visitsCount + regularVisits.length, - orphanVisitsCount: orphanVisitsCount + orphanVisits.length, - }; +export const visitsOverviewReducerCreator = ( + loadVisitsOverviewThunk: ReturnType, +) => createSlice({ + name: REDUCER_PREFIX, + initialState, + reducers: {}, + extraReducers: (builder) => { + builder.addCase(loadVisitsOverviewThunk.pending, () => ({ ...initialState, loading: true })); + builder.addCase(loadVisitsOverviewThunk.rejected, () => ({ ...initialState, error: true })); + builder.addCase(loadVisitsOverviewThunk.fulfilled, (_, { payload }) => ({ ...initialState, ...payload })); + + builder.addCase(createNewVisits, ({ visitsCount, orphanVisitsCount = 0, ...rest }, { payload }) => { + const { createdVisits } = payload; + const { regularVisits, orphanVisits } = groupNewVisitsByType(createdVisits); + return { + ...rest, + visitsCount: visitsCount + regularVisits.length, + orphanVisitsCount: orphanVisitsCount + orphanVisits.length, + }; + }); }, -}, initialState); - -export const loadVisitsOverview = (buildShlinkApiClient: ShlinkApiClientBuilder) => () => async ( - dispatch: Dispatch, - getState: GetState, -) => { - dispatch({ type: GET_OVERVIEW_START }); - - try { - const { getVisitsOverview } = buildShlinkApiClient(getState); - const result = await getVisitsOverview(); - - dispatch({ type: GET_OVERVIEW, ...result }); - } catch (e) { - dispatch({ type: GET_OVERVIEW_ERROR }); - } -}; +}); diff --git a/src/visits/services/provideServices.ts b/src/visits/services/provideServices.ts index a6b0d931..28684da3 100644 --- a/src/visits/services/provideServices.ts +++ b/src/visits/services/provideServices.ts @@ -1,4 +1,5 @@ import Bottle from 'bottlejs'; +import { prop } from 'ramda'; import { MapModal } from '../helpers/MapModal'; import { createNewVisits } from '../reducers/visitCreation'; import { ShortUrlVisits } from '../ShortUrlVisits'; @@ -11,7 +12,7 @@ import { cancelGetDomainVisits, getDomainVisits } from '../reducers/domainVisits import { cancelGetOrphanVisits, getOrphanVisits } from '../reducers/orphanVisits'; import { cancelGetNonOrphanVisits, getNonOrphanVisits } from '../reducers/nonOrphanVisits'; import { ConnectDecorator } from '../../container/types'; -import { loadVisitsOverview } from '../reducers/visitsOverview'; +import { loadVisitsOverview, visitsOverviewReducerCreator } from '../reducers/visitsOverview'; import * as visitsParser from './VisitsParser'; import { DomainVisits } from '../DomainVisits'; @@ -70,6 +71,10 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator) => { bottle.serviceFactory('createNewVisits', () => createNewVisits); bottle.serviceFactory('loadVisitsOverview', loadVisitsOverview, 'buildShlinkApiClient'); + + // Reducers + bottle.serviceFactory('visitsOverviewReducerCreator', visitsOverviewReducerCreator, 'loadVisitsOverview'); + bottle.serviceFactory('visitsOverviewReducer', prop('reducer'), 'visitsOverviewReducerCreator'); }; export default provideServices; diff --git a/test/visits/reducers/visitsOverview.test.ts b/test/visits/reducers/visitsOverview.test.ts index e93a6565..15ce1cf8 100644 --- a/test/visits/reducers/visitsOverview.test.ts +++ b/test/visits/reducers/visitsOverview.test.ts @@ -1,11 +1,9 @@ import { Mock } from 'ts-mockery'; -import reducer, { - GET_OVERVIEW_START, - GET_OVERVIEW_ERROR, - GET_OVERVIEW, +import { GetVisitsOverviewAction, VisitsOverview, - loadVisitsOverview, + loadVisitsOverview as loadVisitsOverviewCreator, + visitsOverviewReducerCreator, } from '../../../src/visits/reducers/visitsOverview'; import { createNewVisits, CreateVisitsAction } from '../../../src/visits/reducers/visitCreation'; import { ShlinkApiClient } from '../../../src/api/services/ShlinkApiClient'; @@ -14,29 +12,42 @@ import { ShlinkState } from '../../../src/container/types'; import { CreateVisit, OrphanVisit, Visit } from '../../../src/visits/types'; describe('visitsOverviewReducer', () => { + const getVisitsOverview = jest.fn(); + const buildApiClientMock = () => Mock.of({ getVisitsOverview }); + const loadVisitsOverview = loadVisitsOverviewCreator(buildApiClientMock); + const { reducer } = visitsOverviewReducerCreator(loadVisitsOverview); + + beforeEach(jest.clearAllMocks); + describe('reducer', () => { const action = (type: string) => Mock.of({ type }) as GetVisitsOverviewAction & CreateVisitsAction; const state = (payload: Partial = {}) => Mock.of(payload); it('returns loading on GET_OVERVIEW_START', () => { - const { loading } = reducer(state({ loading: false, error: false }), action(GET_OVERVIEW_START)); + const { loading } = reducer( + state({ loading: false, error: false }), + action(loadVisitsOverview.pending.toString()), + ); expect(loading).toEqual(true); }); it('stops loading and returns error on GET_OVERVIEW_ERROR', () => { - const { loading, error } = reducer(state({ loading: true, error: false }), action(GET_OVERVIEW_ERROR)); + const { loading, error } = reducer( + state({ loading: true, error: false }), + action(loadVisitsOverview.rejected.toString()), + ); expect(loading).toEqual(false); expect(error).toEqual(true); }); it('return visits overview on GET_OVERVIEW', () => { - const { loading, error, visitsCount } = reducer( - state({ loading: true, error: false }), - { type: GET_OVERVIEW, visitsCount: 100 } as unknown as GetVisitsOverviewAction & CreateVisitsAction, - ); + const { loading, error, visitsCount } = reducer(state({ loading: true, error: false }), { + type: loadVisitsOverview.fulfilled.toString(), + payload: { visitsCount: 100 }, + }); expect(loading).toEqual(false); expect(error).toEqual(false); @@ -76,35 +87,41 @@ describe('visitsOverviewReducer', () => { }); describe('loadVisitsOverview', () => { - const buildApiClientMock = (returned: Promise) => Mock.of({ - getVisitsOverview: jest.fn(async () => returned), - }); const dispatchMock = jest.fn(); const getState = () => Mock.of(); beforeEach(() => dispatchMock.mockReset()); it('dispatches start and error when promise is rejected', async () => { - const ShlinkApiClient = buildApiClientMock(Promise.reject()); + getVisitsOverview.mockRejectedValue(undefined); - await loadVisitsOverview(() => ShlinkApiClient)()(dispatchMock, getState); + await loadVisitsOverview()(dispatchMock, getState, {}); expect(dispatchMock).toHaveBeenCalledTimes(2); - expect(dispatchMock).toHaveBeenNthCalledWith(1, { type: GET_OVERVIEW_START }); - expect(dispatchMock).toHaveBeenNthCalledWith(2, { type: GET_OVERVIEW_ERROR }); - expect(ShlinkApiClient.getVisitsOverview).toHaveBeenCalledTimes(1); + expect(dispatchMock).toHaveBeenNthCalledWith(1, expect.objectContaining({ + type: loadVisitsOverview.pending.toString(), + })); + expect(dispatchMock).toHaveBeenNthCalledWith(2, expect.objectContaining({ + type: loadVisitsOverview.rejected.toString(), + })); + expect(getVisitsOverview).toHaveBeenCalledTimes(1); }); it('dispatches start and success when promise is resolved', async () => { const resolvedOverview = Mock.of({ visitsCount: 50 }); - const shlinkApiClient = buildApiClientMock(Promise.resolve(resolvedOverview)); + getVisitsOverview.mockResolvedValue(resolvedOverview); - await loadVisitsOverview(() => shlinkApiClient)()(dispatchMock, getState); + await loadVisitsOverview()(dispatchMock, getState, {}); expect(dispatchMock).toHaveBeenCalledTimes(2); - expect(dispatchMock).toHaveBeenNthCalledWith(1, { type: GET_OVERVIEW_START }); - expect(dispatchMock).toHaveBeenNthCalledWith(2, { type: GET_OVERVIEW, visitsCount: 50 }); - expect(shlinkApiClient.getVisitsOverview).toHaveBeenCalledTimes(1); + expect(dispatchMock).toHaveBeenNthCalledWith(1, expect.objectContaining({ + type: loadVisitsOverview.pending.toString(), + })); + expect(dispatchMock).toHaveBeenNthCalledWith(2, expect.objectContaining({ + type: loadVisitsOverview.fulfilled.toString(), + payload: { visitsCount: 50 }, + })); + expect(getVisitsOverview).toHaveBeenCalledTimes(1); }); }); });