From ec7c962fa97b8b05db6581a924002b9e4df5d18a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrian=20Or=C5=82=C3=B3w?= <adrian.orlow@fishbrain.com> Date: Thu, 26 Oct 2023 13:36:25 +0200 Subject: [PATCH] feat: rewrite map middleware with use of RTK util --- src/redux/map/map.thunks.test.ts | 8 +- .../map/middleware/checkIfActionValid.test.ts | 121 -------------- .../map/middleware/checkIfIsActionValid.ts | 19 --- .../checkIfIsMapUpdateActionValid.test.ts | 68 ++++++++ .../checkIfIsMapUpdateActionValid.ts | 11 ++ .../map/middleware/getModelIdFromAction.ts | 10 ++ src/redux/map/middleware/getUpdatedModel.ts | 11 +- .../map/middleware/map.middleware.test.ts | 156 +++++++++++------- src/redux/map/middleware/map.middleware.ts | 51 +++--- src/redux/store.ts | 19 ++- .../testing/getReduxWrapperWithStore.tsx | 3 +- 11 files changed, 237 insertions(+), 240 deletions(-) delete mode 100644 src/redux/map/middleware/checkIfActionValid.test.ts delete mode 100644 src/redux/map/middleware/checkIfIsActionValid.ts create mode 100644 src/redux/map/middleware/checkIfIsMapUpdateActionValid.test.ts create mode 100644 src/redux/map/middleware/checkIfIsMapUpdateActionValid.ts create mode 100644 src/redux/map/middleware/getModelIdFromAction.ts diff --git a/src/redux/map/map.thunks.test.ts b/src/redux/map/map.thunks.test.ts index e6eb1523..b14e744d 100644 --- a/src/redux/map/map.thunks.test.ts +++ b/src/redux/map/map.thunks.test.ts @@ -9,7 +9,7 @@ import { apiPath } from '../apiPath'; import { backgroundsDataSelector } from '../backgrounds/background.selectors'; import { modelsDataSelector } from '../models/models.selectors'; import { overlaysDataSelector } from '../overlays/overlays.selectors'; -import { StoreType } from '../store'; +import { AppDispatch, StoreType } from '../store'; import { initMapData } from './map.thunks'; import { InitMapDataActionPayload } from './map.types'; @@ -32,7 +32,8 @@ describe('map thunks', () => { .reply(HttpStatusCode.Ok, backgroundsFixture); store = getReduxWrapperWithStore().store; - payload = (await store.dispatch(initMapData())).payload as InitMapDataActionPayload; + const dispatch = store.dispatch as AppDispatch; + payload = (await dispatch(initMapData())).payload as InitMapDataActionPayload; }); it('should fetch backgrounds data in store', async () => { @@ -74,7 +75,8 @@ describe('map thunks', () => { .reply(HttpStatusCode.Ok, []); store = getReduxWrapperWithStore().store; - payload = (await store.dispatch(initMapData())).payload as InitMapDataActionPayload; + const dispatch = store.dispatch as AppDispatch; + payload = (await dispatch(initMapData())).payload as InitMapDataActionPayload; }); it('should return empty payload', () => { diff --git a/src/redux/map/middleware/checkIfActionValid.test.ts b/src/redux/map/middleware/checkIfActionValid.test.ts deleted file mode 100644 index ed62dfc9..00000000 --- a/src/redux/map/middleware/checkIfActionValid.test.ts +++ /dev/null @@ -1,121 +0,0 @@ -import { RootState } from '@/redux/store'; -import { Loading } from '@/types/loadingState'; -import { MAP_DATA_INITIAL_STATE, MIDDLEWARE_ALLOWED_ACTIONS } from '../map.constants'; -import { checkIfIsActionValid } from './checkIfIsActionValid'; - -const state: Pick<RootState, 'map'> = { - map: { - data: { - ...MAP_DATA_INITIAL_STATE, - modelId: 2137, - }, - loading: 'idle' as Loading, - error: { name: '', message: '' }, - }, -}; - -describe('checkIfIsActionValid - util', () => { - describe('when action type is allowed', () => { - describe('when action payload has model id equal to current', () => { - const modelId = 2137; - - it.each(MIDDLEWARE_ALLOWED_ACTIONS)('should return false', actionType => { - expect( - checkIfIsActionValid( - { - type: actionType, - payload: { - modelId, - }, - }, - state as RootState, - ), - ).toBe(false); - }); - }); - - describe('when action payload has model id different than current', () => { - const modelId = 997; - - it.each(MIDDLEWARE_ALLOWED_ACTIONS)('should return true', actionType => { - expect( - checkIfIsActionValid( - { - type: actionType, - payload: { - modelId, - }, - }, - state as RootState, - ), - ).toBe(true); - }); - }); - - describe('when action payload has NOT model id', () => { - it.each(MIDDLEWARE_ALLOWED_ACTIONS)('should return true', actionType => { - expect( - checkIfIsActionValid( - { - type: actionType, - payload: {}, - }, - state as RootState, - ), - ).toBe(true); - }); - }); - }); - - describe('when action type is NOT allowed', () => { - describe('when action payload has model id equal to current', () => { - const modelId = 2137; - - it('should return false', () => { - expect( - checkIfIsActionValid( - { - type: 'some/other/action', - payload: { - modelId, - }, - }, - state as RootState, - ), - ).toBe(false); - }); - }); - - describe('when action payload has model id different than current', () => { - const modelId = 997; - - it('should return false', () => { - expect( - checkIfIsActionValid( - { - type: 'some/other/action', - payload: { - modelId, - }, - }, - state as RootState, - ), - ).toBe(false); - }); - }); - - describe('when action payload has NOT model id', () => { - it('should return false', () => { - expect( - checkIfIsActionValid( - { - type: 'some/other/action', - payload: {}, - }, - state as RootState, - ), - ).toBe(false); - }); - }); - }); -}); diff --git a/src/redux/map/middleware/checkIfIsActionValid.ts b/src/redux/map/middleware/checkIfIsActionValid.ts deleted file mode 100644 index 38596143..00000000 --- a/src/redux/map/middleware/checkIfIsActionValid.ts +++ /dev/null @@ -1,19 +0,0 @@ -import type { RootState } from '@/redux/store'; -import { MIDDLEWARE_ALLOWED_ACTIONS } from '../map.constants'; -import { MiddlewareAllowedAction } from '../map.types'; - -export const checkIfIsActionValid = ( - action: MiddlewareAllowedAction, - state: RootState, -): boolean => { - const isAllowedAction = MIDDLEWARE_ALLOWED_ACTIONS.some(allowedAction => - action.type.includes(allowedAction), - ); - - const { modelId: currentModelId } = state.map.data; - const payload = action?.payload || {}; - const payloadModelId = 'modelId' in payload ? payload.modelId : null; - const isModelIdTheSame = currentModelId === payloadModelId; - - return isAllowedAction && !isModelIdTheSame; -}; diff --git a/src/redux/map/middleware/checkIfIsMapUpdateActionValid.test.ts b/src/redux/map/middleware/checkIfIsMapUpdateActionValid.test.ts new file mode 100644 index 00000000..aa889249 --- /dev/null +++ b/src/redux/map/middleware/checkIfIsMapUpdateActionValid.test.ts @@ -0,0 +1,68 @@ +import { RootState } from '@/redux/store'; +import { Loading } from '@/types/loadingState'; +import { MAP_DATA_INITIAL_STATE, MIDDLEWARE_ALLOWED_ACTIONS } from '../map.constants'; +import { SetMapDataAction } from '../map.types'; +import { checkIfIsMapUpdateActionValid } from './checkIfIsMapUpdateActionValid'; + +const state: Pick<RootState, 'map'> = { + map: { + data: { + ...MAP_DATA_INITIAL_STATE, + modelId: 2137, + }, + loading: 'idle' as Loading, + error: { name: '', message: '' }, + }, +}; + +describe('checkIfIsActionValid - util', () => { + describe('when action payload has model id equal to current', () => { + const modelId = 2137; + + it.each(MIDDLEWARE_ALLOWED_ACTIONS)('should return false', actionType => { + expect( + checkIfIsMapUpdateActionValid( + { + type: actionType, + payload: { + modelId, + }, + } as SetMapDataAction, + state as RootState, + ), + ).toBe(false); + }); + }); + + describe('when action payload has model id different than current', () => { + const modelId = 997; + + it.each(MIDDLEWARE_ALLOWED_ACTIONS)('should return true', actionType => { + expect( + checkIfIsMapUpdateActionValid( + { + type: actionType, + payload: { + modelId, + }, + } as SetMapDataAction, + state as RootState, + ), + ).toBe(true); + }); + }); + + describe('when action payload has NOT model id', () => { + it.each(MIDDLEWARE_ALLOWED_ACTIONS)('should return true', actionType => { + expect( + checkIfIsMapUpdateActionValid( + { + type: actionType, + payload: {}, + } as SetMapDataAction, + state as RootState, + ), + ).toBe(true); + }); + }); +}); diff --git a/src/redux/map/middleware/checkIfIsMapUpdateActionValid.ts b/src/redux/map/middleware/checkIfIsMapUpdateActionValid.ts new file mode 100644 index 00000000..19379e96 --- /dev/null +++ b/src/redux/map/middleware/checkIfIsMapUpdateActionValid.ts @@ -0,0 +1,11 @@ +import type { RootState } from '@/redux/store'; +import { Action } from '@reduxjs/toolkit'; +import { getModelIdFromAction } from './getModelIdFromAction'; + +export const checkIfIsMapUpdateActionValid = (action: Action, state: RootState): boolean => { + const { modelId: currentModelId } = state.map.data; + const payloadModelId = getModelIdFromAction(action); + const isModelIdTheSame = currentModelId === payloadModelId; + + return !isModelIdTheSame; +}; diff --git a/src/redux/map/middleware/getModelIdFromAction.ts b/src/redux/map/middleware/getModelIdFromAction.ts new file mode 100644 index 00000000..c7e786ae --- /dev/null +++ b/src/redux/map/middleware/getModelIdFromAction.ts @@ -0,0 +1,10 @@ +import { Action } from '@reduxjs/toolkit'; + +export const getModelIdFromAction = (action: Action): number | null => { + try { + const payload = 'payload' in action ? (action.payload as object) : {}; + return 'modelId' in payload ? (payload.modelId as number) : null; + } catch { + return null; + } +}; diff --git a/src/redux/map/middleware/getUpdatedModel.ts b/src/redux/map/middleware/getUpdatedModel.ts index 60c26a24..4302c552 100644 --- a/src/redux/map/middleware/getUpdatedModel.ts +++ b/src/redux/map/middleware/getUpdatedModel.ts @@ -1,15 +1,12 @@ import { modelsDataSelector } from '@/redux/models/models.selectors'; import type { RootState } from '@/redux/store'; import { MapModel } from '@/types/models'; -import { MiddlewareAllowedAction } from '../map.types'; +import { Action } from '@reduxjs/toolkit'; +import { getModelIdFromAction } from './getModelIdFromAction'; -export const getUpdatedModel = ( - action: MiddlewareAllowedAction, - state: RootState, -): MapModel | undefined => { +export const getUpdatedModel = (action: Action, state: RootState): MapModel | undefined => { const models = modelsDataSelector(state); - const payload = action?.payload || {}; - const payloadModelId = 'modelId' in payload ? payload.modelId : null; + const payloadModelId = getModelIdFromAction(action); return models.find(model => model.idObject === payloadModelId); }; diff --git a/src/redux/map/middleware/map.middleware.test.ts b/src/redux/map/middleware/map.middleware.test.ts index 7698a04f..c821eeda 100644 --- a/src/redux/map/middleware/map.middleware.test.ts +++ b/src/redux/map/middleware/map.middleware.test.ts @@ -1,16 +1,18 @@ /* eslint-disable no-magic-numbers */ import { modelsFixture } from '@/models/fixtures/modelsFixture'; -import type { RootState } from '@/redux/store'; import { Loading } from '@/types/loadingState'; +import { getReduxWrapperWithStore } from '@/utils/testing/getReduxWrapperWithStore'; +import { Action } from '@reduxjs/toolkit'; import { MAP_DATA_INITIAL_STATE, MIDDLEWARE_ALLOWED_ACTIONS } from '../map.constants'; -import * as checkIfIsActionValid from './checkIfIsActionValid'; +import * as setMapData from '../map.slice'; +import * as checkIfIsMapUpdateActionValid from './checkIfIsMapUpdateActionValid'; import * as getUpdatedModel from './getUpdatedModel'; -import { mapMiddleware } from './map.middleware'; +import { mapDataMiddlewareListener } from './map.middleware'; -jest.mock('./checkIfIsActionValid', () => { +jest.mock('./checkIfIsMapUpdateActionValid', () => { return { __esModule: true, - ...jest.requireActual('./checkIfIsActionValid'), + ...jest.requireActual('./checkIfIsMapUpdateActionValid'), }; }); @@ -21,85 +23,111 @@ jest.mock('./getUpdatedModel', () => { }; }); +jest.mock('./getUpdatedModel', () => { + return { + __esModule: true, + ...jest.requireActual('./getUpdatedModel'), + }; +}); + +jest.mock('../map.slice', () => { + return { + __esModule: true, + ...jest.requireActual('../map.slice'), + }; +}); + const defaultSliceState = { data: [], loading: 'idle' as Loading, error: { name: '', message: '' }, }; -const checkIfIsActionValidSpy = jest.spyOn(checkIfIsActionValid, 'checkIfIsActionValid'); +const checkIfIsMapUpdateActionValidSpy = jest.spyOn( + checkIfIsMapUpdateActionValid, + 'checkIfIsMapUpdateActionValid', +); const getUpdatedModelSpy = jest.spyOn(getUpdatedModel, 'getUpdatedModel'); +const setMapDataSpy = jest.spyOn(setMapData, 'setMapData'); + +const { store } = getReduxWrapperWithStore({ + map: { + ...defaultSliceState, + data: { + ...MAP_DATA_INITIAL_STATE, + modelId: modelsFixture[0].idObject, + }, + }, + models: { + ...defaultSliceState, + data: modelsFixture, + }, +}); + +const doDispatch = jest.fn(); +const doGetState = store.getState; + +const handler = async (action: Action): Promise<void> => + mapDataMiddlewareListener(action, { + dispatch: doDispatch, + getOriginalState: doGetState, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any); describe('map middleware', () => { - describe('on action', () => { - const doDispatch = jest.fn(); - const doGetState = jest.fn().mockImplementation( - (): Pick<RootState, 'models' | 'map'> => ({ - map: { - ...defaultSliceState, - data: MAP_DATA_INITIAL_STATE, - }, - models: { - ...defaultSliceState, - data: modelsFixture, - }, - }), - ); - - const handler = mapMiddleware({ - dispatch: doDispatch, - getState: doGetState, + describe('on listen', () => { + afterEach(() => { + jest.clearAllMocks(); }); - describe('when action is valid', () => { - const next = jest.fn(); - - describe('and when model is valid', () => { - it.each(MIDDLEWARE_ALLOWED_ACTIONS)('should run next action', actionType => { - const model = modelsFixture[0]; - const action = { - payload: { - modelId: model.idObject, - }, - type: actionType, - }; - - handler(next)(action); - expect(checkIfIsActionValidSpy).toHaveLastReturnedWith(true); - expect(getUpdatedModelSpy).toHaveLastReturnedWith(model); - expect(next).toBeCalled(); - }); + describe('when model is valid and different than current', () => { + it.each(MIDDLEWARE_ALLOWED_ACTIONS)('should dispatch setMapData', async actionType => { + const model = modelsFixture[1]; + + const action = { + payload: { + modelId: model.idObject, + }, + type: actionType, + }; + + await handler(action); + expect(checkIfIsMapUpdateActionValidSpy).toHaveLastReturnedWith(true); + expect(getUpdatedModelSpy).toHaveLastReturnedWith(model); + expect(setMapDataSpy).toBeCalled(); }); + }); + + describe('when model is valid and identical with current', () => { + it.each(MIDDLEWARE_ALLOWED_ACTIONS)('should NOT dispatch setMapData', async actionType => { + const model = modelsFixture[0]; + const action = { + payload: { + modelId: model.idObject, + }, + type: actionType, + }; - describe('and when model is NOT valid', () => { - it.each(MIDDLEWARE_ALLOWED_ACTIONS)('should run next action', actionType => { - const action = { - payload: { - modelId: null, - }, - type: actionType, - }; - - handler(next)(action); - expect(checkIfIsActionValidSpy).toHaveLastReturnedWith(true); - expect(getUpdatedModelSpy).toHaveLastReturnedWith(undefined); - expect(next).toBeCalled(); - }); + await handler(action); + expect(checkIfIsMapUpdateActionValidSpy).toHaveLastReturnedWith(false); + expect(getUpdatedModelSpy).toHaveLastReturnedWith(model); + expect(setMapDataSpy).not.toBeCalled(); }); }); - describe('when action is NOT valid', () => { - it('should run next action', () => { - const next = jest.fn(); + describe('when model is NOT valid', () => { + it.each(MIDDLEWARE_ALLOWED_ACTIONS)('should NOT dispatch setMapData', async actionType => { const action = { - payload: {}, - type: 'some/other/action', + payload: { + modelId: null, + }, + type: actionType, }; - handler(next)(action); - expect(checkIfIsActionValidSpy).toHaveLastReturnedWith(false); + await handler(action); + expect(checkIfIsMapUpdateActionValidSpy).toHaveLastReturnedWith(true); expect(getUpdatedModelSpy).toHaveLastReturnedWith(undefined); - expect(next).toBeCalled(); + expect(setMapDataSpy).not.toBeCalled(); }); }); }); diff --git a/src/redux/map/middleware/map.middleware.ts b/src/redux/map/middleware/map.middleware.ts index 93b9e3ff..0221d9f6 100644 --- a/src/redux/map/middleware/map.middleware.ts +++ b/src/redux/map/middleware/map.middleware.ts @@ -1,28 +1,37 @@ +import type { AppListenerEffectAPI, AppStartListening } from '@/redux/store'; import { getUpdatedMapData } from '@/utils/map/getUpdatedMapData'; -import { Middleware, MiddlewareAPI } from '@reduxjs/toolkit'; -import type { AppDispatch, RootState } from '../../store'; +import { Action, createListenerMiddleware } from '@reduxjs/toolkit'; import { setMapData } from '../map.slice'; -import { MiddlewareAllowedAction } from '../map.types'; -import { checkIfIsActionValid } from './checkIfIsActionValid'; +import { initMapData } from '../map.thunks'; +import { checkIfIsMapUpdateActionValid } from './checkIfIsMapUpdateActionValid'; import { getUpdatedModel } from './getUpdatedModel'; -/* prettier-ignore */ -export const mapMiddleware: Middleware = - ({ getState, dispatch }: MiddlewareAPI<AppDispatch, RootState>) => - (next: AppDispatch) => - // eslint-disable-next-line consistent-return - (action: MiddlewareAllowedAction) => { - const state = getState(); - const isActionValid = checkIfIsActionValid(action, state); - const updatedModel = getUpdatedModel(action, state); - const returnValue = next(action); +export const mapListenerMiddleware = createListenerMiddleware(); - if (!isActionValid || !updatedModel) { - return returnValue; - } +const startListening = mapListenerMiddleware.startListening as AppStartListening; - const updatedMapData = getUpdatedMapData({ model: updatedModel }); - dispatch(setMapData(updatedMapData)); +export const mapDataMiddlewareListener = async ( + action: Action, + { getOriginalState, dispatch }: AppListenerEffectAPI, +): Promise<void> => { + const state = getOriginalState(); + const updatedModel = getUpdatedModel(action, state); + const isActionValid = checkIfIsMapUpdateActionValid(action, state); - return returnValue; - }; + if (!updatedModel || !isActionValid) { + return; + } + + const updatedMapData = getUpdatedMapData({ model: updatedModel }); + dispatch(setMapData(updatedMapData)); +}; + +startListening({ + actionCreator: initMapData.fulfilled, + effect: mapDataMiddlewareListener, +}); + +startListening({ + type: 'map/setMapData', + effect: mapDataMiddlewareListener, +}); diff --git a/src/redux/store.ts b/src/redux/store.ts index aaecc167..d0d3e673 100644 --- a/src/redux/store.ts +++ b/src/redux/store.ts @@ -9,8 +9,14 @@ import modelsReducer from '@/redux/models/models.slice'; import overlaysReducer from '@/redux/overlays/overlays.slice'; import projectReducer from '@/redux/project/project.slice'; import searchReducer from '@/redux/search/search.slice'; -import { applyMiddleware, configureStore } from '@reduxjs/toolkit'; -import { mapMiddleware } from './map/middleware/map.middleware'; +import { + AnyAction, + ListenerEffectAPI, + ThunkDispatch, + TypedStartListening, + configureStore, +} from '@reduxjs/toolkit'; +import { mapListenerMiddleware } from './map/middleware/map.middleware'; export const reducers = { search: searchReducer, @@ -26,14 +32,19 @@ export const reducers = { models: modelsReducer, }; +export const middlewares = [mapListenerMiddleware.middleware]; + export const store = configureStore({ reducer: reducers, devTools: true, - enhancers: [applyMiddleware(mapMiddleware)], + middleware: getDefaultMiddleware => getDefaultMiddleware().prepend(...middlewares), }); export type StoreType = typeof store; // Infer the `RootState` and `AppDispatch` types from the store itself export type RootState = ReturnType<typeof store.getState>; // Inferred type: {posts: PostsState, comments: CommentsState, users: UsersState} -export type AppDispatch = typeof store.dispatch; +export type TypedDispatch<T> = ThunkDispatch<T, unknown, AnyAction>; +export type AppDispatch = TypedDispatch<RootState>; +export type AppStartListening = TypedStartListening<RootState, AppDispatch>; +export type AppListenerEffectAPI = ListenerEffectAPI<RootState, AppDispatch>; diff --git a/src/utils/testing/getReduxWrapperWithStore.tsx b/src/utils/testing/getReduxWrapperWithStore.tsx index 604f81c4..5c799a25 100644 --- a/src/utils/testing/getReduxWrapperWithStore.tsx +++ b/src/utils/testing/getReduxWrapperWithStore.tsx @@ -1,4 +1,4 @@ -import { RootState, StoreType, reducers } from '@/redux/store'; +import { RootState, StoreType, middlewares, reducers } from '@/redux/store'; import { configureStore } from '@reduxjs/toolkit'; import { Provider } from 'react-redux'; @@ -19,6 +19,7 @@ export const getReduxWrapperWithStore: GetReduxWrapperUsingSliceReducer = ( const testStore = configureStore({ reducer: reducers, preloadedState, + middleware: getDefaultMiddleware => getDefaultMiddleware().prepend(...middlewares), }); const Wrapper = ({ children }: WrapperProps): JSX.Element => ( -- GitLab