From d588d8d9effe45bb8d9a9f88359090d7d2aac4a3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 5 Nov 2022 13:01:00 +0100 Subject: [PATCH 01/15] Migrated create visit action to use payload --- src/short-urls/reducers/shortUrlsList.ts | 4 +-- src/tags/reducers/tagsList.ts | 4 +-- src/visits/reducers/domainVisits.ts | 4 +-- src/visits/reducers/nonOrphanVisits.ts | 4 +-- src/visits/reducers/orphanVisits.ts | 4 +-- src/visits/reducers/shortUrlVisits.ts | 4 +-- src/visits/reducers/tagVisits.ts | 4 +-- src/visits/reducers/visitCreation.ts | 8 +++--- src/visits/reducers/visitsOverview.ts | 4 +-- .../short-urls/reducers/shortUrlsList.test.ts | 2 +- test/visits/reducers/domainVisits.test.ts | 2 +- test/visits/reducers/nonOrphanVisits.test.ts | 8 +++--- test/visits/reducers/orphanVisits.test.ts | 8 +++--- test/visits/reducers/shortUrlVisits.test.ts | 8 +++--- test/visits/reducers/tagVisits.test.ts | 2 +- test/visits/reducers/visitCreation.test.ts | 7 +++-- test/visits/reducers/visitsOverview.test.ts | 28 ++++++++++--------- 17 files changed, 54 insertions(+), 51 deletions(-) diff --git a/src/short-urls/reducers/shortUrlsList.ts b/src/short-urls/reducers/shortUrlsList.ts index fdf2e4c0..7137a41d 100644 --- a/src/short-urls/reducers/shortUrlsList.ts +++ b/src/short-urls/reducers/shortUrlsList.ts @@ -56,13 +56,13 @@ export default buildReducer({ state, )), ), - [CREATE_VISITS]: (state, { createdVisits }) => assocPath( + [CREATE_VISITS]: (state, { payload }) => assocPath( ['shortUrls', 'data'], state.shortUrls?.data?.map( (currentShortUrl) => { // Find the last of the new visit for this short URL, and pick the amount of visits from it const lastVisit = last( - createdVisits.filter( + payload.createdVisits.filter( ({ shortUrl }) => shortUrl && shortUrlMatches(currentShortUrl, shortUrl.shortCode, shortUrl.domain), ), ); diff --git a/src/tags/reducers/tagsList.ts b/src/tags/reducers/tagsList.ts index cf7a7ac6..a31c060d 100644 --- a/src/tags/reducers/tagsList.ts +++ b/src/tags/reducers/tagsList.ts @@ -99,9 +99,9 @@ export default buildReducer({ ...state, filteredTags: state.tags.filter((tag) => tag.toLowerCase().match(searchTerm.toLowerCase())), }), - [CREATE_VISITS]: (state, { createdVisits }) => ({ + [CREATE_VISITS]: (state, { payload }) => ({ ...state, - stats: increaseVisitsForTags(calculateVisitsPerTag(createdVisits), state.stats), + stats: increaseVisitsForTags(calculateVisitsPerTag(payload.createdVisits), state.stats), }), [CREATE_SHORT_URL]: ({ tags: stateTags, ...rest }, { result }) => ({ ...rest, diff --git a/src/visits/reducers/domainVisits.ts b/src/visits/reducers/domainVisits.ts index 17ee4f63..72f83300 100644 --- a/src/visits/reducers/domainVisits.ts +++ b/src/visits/reducers/domainVisits.ts @@ -56,10 +56,10 @@ export default buildReducer({ [GET_DOMAIN_VISITS_CANCEL]: (state) => ({ ...state, cancelLoad: true }), [GET_DOMAIN_VISITS_PROGRESS_CHANGED]: (state, { progress }) => ({ ...state, progress }), [GET_DOMAIN_VISITS_FALLBACK_TO_INTERVAL]: (state, { fallbackInterval }) => ({ ...state, fallbackInterval }), - [CREATE_VISITS]: (state, { createdVisits }) => { + [CREATE_VISITS]: (state, { payload }) => { const { domain, visits, query = {} } = state; const { startDate, endDate } = query; - const newVisits = createdVisits + const newVisits = payload.createdVisits .filter(({ shortUrl, visit }) => shortUrl && domainMatches(shortUrl, domain) && isBetween(visit.date, startDate, endDate)) .map(({ visit }) => visit); diff --git a/src/visits/reducers/nonOrphanVisits.ts b/src/visits/reducers/nonOrphanVisits.ts index 05133800..84e2555f 100644 --- a/src/visits/reducers/nonOrphanVisits.ts +++ b/src/visits/reducers/nonOrphanVisits.ts @@ -52,10 +52,10 @@ export default buildReducer({ [GET_NON_ORPHAN_VISITS_CANCEL]: (state) => ({ ...state, cancelLoad: true }), [GET_NON_ORPHAN_VISITS_PROGRESS_CHANGED]: (state, { progress }) => ({ ...state, progress }), [GET_NON_ORPHAN_VISITS_FALLBACK_TO_INTERVAL]: (state, { fallbackInterval }) => ({ ...state, fallbackInterval }), - [CREATE_VISITS]: (state, { createdVisits }) => { + [CREATE_VISITS]: (state, { payload }) => { const { visits, query = {} } = state; const { startDate, endDate } = query; - const newVisits = createdVisits + const newVisits = payload.createdVisits .filter(({ visit }) => isBetween(visit.date, startDate, endDate)) .map(({ visit }) => visit); diff --git a/src/visits/reducers/orphanVisits.ts b/src/visits/reducers/orphanVisits.ts index 8b6aee65..27023458 100644 --- a/src/visits/reducers/orphanVisits.ts +++ b/src/visits/reducers/orphanVisits.ts @@ -55,10 +55,10 @@ export default buildReducer({ [GET_ORPHAN_VISITS_CANCEL]: (state) => ({ ...state, cancelLoad: true }), [GET_ORPHAN_VISITS_PROGRESS_CHANGED]: (state, { progress }) => ({ ...state, progress }), [GET_ORPHAN_VISITS_FALLBACK_TO_INTERVAL]: (state, { fallbackInterval }) => ({ ...state, fallbackInterval }), - [CREATE_VISITS]: (state, { createdVisits }) => { + [CREATE_VISITS]: (state, { payload }) => { const { visits, query = {} } = state; const { startDate, endDate } = query; - const newVisits = createdVisits + const newVisits = payload.createdVisits .filter(({ visit, shortUrl }) => !shortUrl && isBetween(visit.date, startDate, endDate)) .map(({ visit }) => visit); diff --git a/src/visits/reducers/shortUrlVisits.ts b/src/visits/reducers/shortUrlVisits.ts index 00327c46..2bfe505e 100644 --- a/src/visits/reducers/shortUrlVisits.ts +++ b/src/visits/reducers/shortUrlVisits.ts @@ -60,10 +60,10 @@ export default buildReducer({ [GET_SHORT_URL_VISITS_CANCEL]: (state) => ({ ...state, cancelLoad: true }), [GET_SHORT_URL_VISITS_PROGRESS_CHANGED]: (state, { progress }) => ({ ...state, progress }), [GET_SHORT_URL_VISITS_FALLBACK_TO_INTERVAL]: (state, { fallbackInterval }) => ({ ...state, fallbackInterval }), - [CREATE_VISITS]: (state, { createdVisits }) => { + [CREATE_VISITS]: (state, { payload }) => { const { shortCode, domain, visits, query = {} } = state; const { startDate, endDate } = query; - const newVisits = createdVisits + const newVisits = payload.createdVisits .filter( ({ shortUrl, visit }) => shortUrl && shortUrlMatches(shortUrl, shortCode, domain) && isBetween(visit.date, startDate, endDate), diff --git a/src/visits/reducers/tagVisits.ts b/src/visits/reducers/tagVisits.ts index f2fe6b85..07372ea1 100644 --- a/src/visits/reducers/tagVisits.ts +++ b/src/visits/reducers/tagVisits.ts @@ -53,10 +53,10 @@ export default buildReducer({ [GET_TAG_VISITS_CANCEL]: (state) => ({ ...state, cancelLoad: true }), [GET_TAG_VISITS_PROGRESS_CHANGED]: (state, { progress }) => ({ ...state, progress }), [GET_TAG_VISITS_FALLBACK_TO_INTERVAL]: (state, { fallbackInterval }) => ({ ...state, fallbackInterval }), - [CREATE_VISITS]: (state, { createdVisits }) => { + [CREATE_VISITS]: (state, { payload }) => { const { tag, visits, query = {} } = state; const { startDate, endDate } = query; - const newVisits = createdVisits + const newVisits = payload.createdVisits .filter(({ shortUrl, visit }) => shortUrl?.tags.includes(tag) && isBetween(visit.date, startDate, endDate)) .map(({ visit }) => visit); diff --git a/src/visits/reducers/visitCreation.ts b/src/visits/reducers/visitCreation.ts index e2335fc4..9400c02f 100644 --- a/src/visits/reducers/visitCreation.ts +++ b/src/visits/reducers/visitCreation.ts @@ -1,13 +1,13 @@ -import { Action } from 'redux'; +import { PayloadAction } from '@reduxjs/toolkit'; import { CreateVisit } from '../types'; export const CREATE_VISITS = 'shlink/visitCreation/CREATE_VISITS'; -export interface CreateVisitsAction extends Action { +export type CreateVisitsAction = PayloadAction<{ createdVisits: CreateVisit[]; -} +}>; export const createNewVisits = (createdVisits: CreateVisit[]): CreateVisitsAction => ({ type: CREATE_VISITS, - createdVisits, + payload: { createdVisits }, }); diff --git a/src/visits/reducers/visitsOverview.ts b/src/visits/reducers/visitsOverview.ts index ec5d8fec..a9a0ff9a 100644 --- a/src/visits/reducers/visitsOverview.ts +++ b/src/visits/reducers/visitsOverview.ts @@ -30,8 +30,8 @@ export default buildReducer ({ ...initialState, loading: true }), [GET_OVERVIEW_ERROR]: () => ({ ...initialState, error: true }), [GET_OVERVIEW]: (_, { visitsCount, orphanVisitsCount }) => ({ ...initialState, visitsCount, orphanVisitsCount }), - [CREATE_VISITS]: ({ visitsCount, orphanVisitsCount = 0, ...rest }, { createdVisits }) => { - const { regularVisits, orphanVisits } = groupNewVisitsByType(createdVisits); + [CREATE_VISITS]: ({ visitsCount, orphanVisitsCount = 0, ...rest }, { payload }) => { + const { regularVisits, orphanVisits } = groupNewVisitsByType(payload.createdVisits); return { ...rest, diff --git a/test/short-urls/reducers/shortUrlsList.test.ts b/test/short-urls/reducers/shortUrlsList.test.ts index fbff5de0..a4648121 100644 --- a/test/short-urls/reducers/shortUrlsList.test.ts +++ b/test/short-urls/reducers/shortUrlsList.test.ts @@ -85,7 +85,7 @@ describe('shortUrlsListReducer', () => { error: false, }; - expect(reducer(state, { type: CREATE_VISITS, createdVisits } as any)).toEqual({ + expect(reducer(state, { type: CREATE_VISITS, payload: { createdVisits } } as any)).toEqual({ shortUrls: { data: [ { shortCode, domain: 'example.com', visitsCount: 5 }, diff --git a/test/visits/reducers/domainVisits.test.ts b/test/visits/reducers/domainVisits.test.ts index 8c9bfccd..f3d8d7d8 100644 --- a/test/visits/reducers/domainVisits.test.ts +++ b/test/visits/reducers/domainVisits.test.ts @@ -135,7 +135,7 @@ describe('domainVisitsReducer', () => { const { visits } = reducer(prevState, { type: CREATE_VISITS, - createdVisits: [{ shortUrl, visit: { date: formatIsoDate(now) ?? undefined } }], + payload: { createdVisits: [{ shortUrl, visit: { date: formatIsoDate(now) ?? undefined } }] }, } as any); expect(visits).toHaveLength(expectedVisits); diff --git a/test/visits/reducers/nonOrphanVisits.test.ts b/test/visits/reducers/nonOrphanVisits.test.ts index 7e6c2911..f4c2bc40 100644 --- a/test/visits/reducers/nonOrphanVisits.test.ts +++ b/test/visits/reducers/nonOrphanVisits.test.ts @@ -105,10 +105,10 @@ describe('nonOrphanVisitsReducer', () => { const prevState = buildState({ ...state, visits: visitsMocks }); const visit = Mock.of({ date: formatIsoDate(now) ?? undefined }); - const { visits } = reducer( - prevState, - { type: CREATE_VISITS, createdVisits: [{ visit }, { visit }] } as any, - ); + const { visits } = reducer(prevState, { + type: CREATE_VISITS, + payload: { createdVisits: [{ visit }, { visit }] }, + } as any); expect(visits).toHaveLength(expectedVisits); }); diff --git a/test/visits/reducers/orphanVisits.test.ts b/test/visits/reducers/orphanVisits.test.ts index d57eff5c..5325ac77 100644 --- a/test/visits/reducers/orphanVisits.test.ts +++ b/test/visits/reducers/orphanVisits.test.ts @@ -105,10 +105,10 @@ describe('orphanVisitsReducer', () => { const prevState = buildState({ ...state, visits: visitsMocks }); const visit = Mock.of({ date: formatIsoDate(now) ?? undefined }); - const { visits } = reducer( - prevState, - { type: CREATE_VISITS, createdVisits: [{ visit }, { visit }] } as any, - ); + const { visits } = reducer(prevState, { + type: CREATE_VISITS, + payload: { createdVisits: [{ visit }, { visit }] }, + } as any); expect(visits).toHaveLength(expectedVisits); }); diff --git a/test/visits/reducers/shortUrlVisits.test.ts b/test/visits/reducers/shortUrlVisits.test.ts index 3b7cdbb8..d396edb4 100644 --- a/test/visits/reducers/shortUrlVisits.test.ts +++ b/test/visits/reducers/shortUrlVisits.test.ts @@ -126,10 +126,10 @@ describe('shortUrlVisitsReducer', () => { visits: visitsMocks, }); - const { visits } = reducer( - prevState, - { type: CREATE_VISITS, createdVisits: [{ shortUrl, visit: { date: formatIsoDate(now) ?? undefined } }] } as any, - ); + const { visits } = reducer(prevState, { + type: CREATE_VISITS, + payload: { createdVisits: [{ shortUrl, visit: { date: formatIsoDate(now) ?? undefined } }] }, + } as any); expect(visits).toHaveLength(expectedVisits); }); diff --git a/test/visits/reducers/tagVisits.test.ts b/test/visits/reducers/tagVisits.test.ts index 5a217d24..d5fd0375 100644 --- a/test/visits/reducers/tagVisits.test.ts +++ b/test/visits/reducers/tagVisits.test.ts @@ -128,7 +128,7 @@ describe('tagVisitsReducer', () => { const { visits } = reducer(prevState, { type: CREATE_VISITS, - createdVisits: [{ shortUrl, visit: { date: formatIsoDate(now) ?? undefined } }], + payload: { createdVisits: [{ shortUrl, visit: { date: formatIsoDate(now) ?? undefined } }] }, } as any); expect(visits).toHaveLength(expectedVisits); diff --git a/test/visits/reducers/visitCreation.test.ts b/test/visits/reducers/visitCreation.test.ts index c14f8b2e..eba1d4a7 100644 --- a/test/visits/reducers/visitCreation.test.ts +++ b/test/visits/reducers/visitCreation.test.ts @@ -9,9 +9,10 @@ describe('visitCreationReducer', () => { const visit = Mock.all(); it('just returns the action with proper type', () => { - expect(createNewVisits([{ shortUrl, visit }])).toEqual( - { type: CREATE_VISITS, createdVisits: [{ shortUrl, visit }] }, - ); + expect(createNewVisits([{ shortUrl, visit }])).toEqual({ + type: CREATE_VISITS, + payload: { createdVisits: [{ shortUrl, visit }] }, + }); }); }); }); diff --git a/test/visits/reducers/visitsOverview.test.ts b/test/visits/reducers/visitsOverview.test.ts index 98ced535..8405c43a 100644 --- a/test/visits/reducers/visitsOverview.test.ts +++ b/test/visits/reducers/visitsOverview.test.ts @@ -52,19 +52,21 @@ describe('visitsOverviewReducer', () => { state({ visitsCount: 100, orphanVisitsCount: providedOrphanVisitsCount }), { type: CREATE_VISITS, - createdVisits: [ - Mock.of({ visit: Mock.all() }), - Mock.of({ visit: Mock.all() }), - Mock.of({ - visit: Mock.of({ visitedUrl: '' }), - }), - Mock.of({ - visit: Mock.of({ visitedUrl: '' }), - }), - Mock.of({ - visit: Mock.of({ visitedUrl: '' }), - }), - ], + payload: { + createdVisits: [ + Mock.of({ visit: Mock.all() }), + Mock.of({ visit: Mock.all() }), + Mock.of({ + visit: Mock.of({ visitedUrl: '' }), + }), + Mock.of({ + visit: Mock.of({ visitedUrl: '' }), + }), + Mock.of({ + visit: Mock.of({ visitedUrl: '' }), + }), + ], + }, } as unknown as GetVisitsOverviewAction & CreateVisitsAction, ); From 7c61033bdf8f111bcedc08df6556d4e67195e3ef Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 5 Nov 2022 13:05:44 +0100 Subject: [PATCH 02/15] Migrated createNewVisits action creator to RTK --- src/visits/reducers/visitCreation.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/visits/reducers/visitCreation.ts b/src/visits/reducers/visitCreation.ts index 9400c02f..a14a339a 100644 --- a/src/visits/reducers/visitCreation.ts +++ b/src/visits/reducers/visitCreation.ts @@ -1,4 +1,4 @@ -import { PayloadAction } from '@reduxjs/toolkit'; +import { createAction, PayloadAction } from '@reduxjs/toolkit'; import { CreateVisit } from '../types'; export const CREATE_VISITS = 'shlink/visitCreation/CREATE_VISITS'; @@ -7,7 +7,7 @@ export type CreateVisitsAction = PayloadAction<{ createdVisits: CreateVisit[]; }>; -export const createNewVisits = (createdVisits: CreateVisit[]): CreateVisitsAction => ({ - type: CREATE_VISITS, - payload: { createdVisits }, -}); +export const createNewVisits = createAction( + CREATE_VISITS, + (createdVisits: CreateVisit[]) => ({ payload: { createdVisits } }), +); From 50823003b456d44b61b0bd286cb79196b9780809 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 6 Nov 2022 09:40:23 +0100 Subject: [PATCH 03/15] Migrated settings reducer to RTK --- src/reducers/index.ts | 2 +- src/settings/reducers/settings.ts | 74 +++++++++++-------------- test/settings/reducers/settings.test.ts | 43 ++++++++++---- 3 files changed, 65 insertions(+), 54 deletions(-) diff --git a/src/reducers/index.ts b/src/reducers/index.ts index c88fb12e..61bd0139 100644 --- a/src/reducers/index.ts +++ b/src/reducers/index.ts @@ -15,7 +15,7 @@ import shortUrlDetailReducer from '../short-urls/reducers/shortUrlDetail'; import tagsListReducer from '../tags/reducers/tagsList'; import tagDeleteReducer from '../tags/reducers/tagDelete'; import tagEditReducer from '../tags/reducers/tagEdit'; -import settingsReducer from '../settings/reducers/settings'; +import { settingsReducer } from '../settings/reducers/settings'; import visitsOverviewReducer from '../visits/reducers/visitsOverview'; import { appUpdatesReducer } from '../app/reducers/appUpdates'; import { sidebarReducer } from '../common/reducers/sidebar'; diff --git a/src/settings/reducers/settings.ts b/src/settings/reducers/settings.ts index 8a106682..bfc4e2aa 100644 --- a/src/settings/reducers/settings.ts +++ b/src/settings/reducers/settings.ts @@ -1,14 +1,10 @@ -import { Action } from 'redux'; -import { dissoc, mergeDeepRight } from 'ramda'; -import { buildReducer } from '../../utils/helpers/redux'; -import { RecursivePartial } from '../../utils/utils'; +import { createSlice, PayloadAction, PrepareAction } from '@reduxjs/toolkit'; +import { mergeDeepRight } from 'ramda'; import { Theme } from '../../utils/theme'; import { DateInterval } from '../../utils/dates/types'; import { TagsOrder } from '../../tags/data/TagsListChildrenProps'; import { ShortUrlsOrder } from '../../short-urls/data'; -export const SET_SETTINGS = 'shlink/realTimeUpdates/SET_SETTINGS'; - export const DEFAULT_SHORT_URLS_ORDERING: ShortUrlsOrder = { field: 'dateCreated', dir: 'DESC', @@ -78,45 +74,37 @@ const initialState: Settings = { }, }; -type SettingsAction = Action & Settings; +type SettingsAction = PayloadAction; +type SettingsPrepareAction = PrepareAction; -type PartialSettingsAction = Action & RecursivePartial; +const commonReducer = (state: Settings, { payload }: SettingsAction) => mergeDeepRight(state, payload); +const toReducer = (prepare: SettingsPrepareAction) => ({ reducer: commonReducer, prepare }); +const toPreparedAction: SettingsPrepareAction = (payload: Settings) => ({ payload }); -export default buildReducer({ - [SET_SETTINGS]: (state, action) => mergeDeepRight(state, dissoc('type', action)), -}, initialState); - -export const toggleRealTimeUpdates = (enabled: boolean): PartialSettingsAction => ({ - type: SET_SETTINGS, - realTimeUpdates: { enabled }, +const { reducer, actions } = createSlice({ + name: 'settingsReducer', + initialState, + reducers: { + toggleRealTimeUpdates: toReducer((enabled: boolean) => toPreparedAction({ realTimeUpdates: { enabled } })), + setRealTimeUpdatesInterval: toReducer((interval: number) => toPreparedAction({ realTimeUpdates: { interval } })), + setShortUrlCreationSettings: toReducer( + (shortUrlCreation: ShortUrlCreationSettings) => toPreparedAction({ shortUrlCreation }), + ), + setShortUrlsListSettings: toReducer((shortUrlsList: ShortUrlsListSettings) => toPreparedAction({ shortUrlsList })), + setUiSettings: toReducer((ui: UiSettings) => toPreparedAction({ ui })), + setVisitsSettings: toReducer((visits: VisitsSettings) => toPreparedAction({ visits })), + setTagsSettings: toReducer((tags: TagsSettings) => toPreparedAction({ tags })), + }, }); -export const setRealTimeUpdatesInterval = (interval: number): PartialSettingsAction => ({ - type: SET_SETTINGS, - realTimeUpdates: { interval }, -}); +export const { + toggleRealTimeUpdates, + setRealTimeUpdatesInterval, + setShortUrlCreationSettings, + setShortUrlsListSettings, + setUiSettings, + setVisitsSettings, + setTagsSettings, +} = actions; -export const setShortUrlCreationSettings = (settings: ShortUrlCreationSettings): PartialSettingsAction => ({ - type: SET_SETTINGS, - shortUrlCreation: settings, -}); - -export const setShortUrlsListSettings = (settings: ShortUrlsListSettings): PartialSettingsAction => ({ - type: SET_SETTINGS, - shortUrlsList: settings, -}); - -export const setUiSettings = (settings: UiSettings): PartialSettingsAction => ({ - type: SET_SETTINGS, - ui: settings, -}); - -export const setVisitsSettings = (settings: VisitsSettings): PartialSettingsAction => ({ - type: SET_SETTINGS, - visits: settings, -}); - -export const setTagsSettings = (settings: TagsSettings): PartialSettingsAction => ({ - type: SET_SETTINGS, - tags: settings, -}); +export const settingsReducer = reducer; diff --git a/test/settings/reducers/settings.test.ts b/test/settings/reducers/settings.test.ts index 427eec21..50d31ac1 100644 --- a/test/settings/reducers/settings.test.ts +++ b/test/settings/reducers/settings.test.ts @@ -1,6 +1,6 @@ -import reducer, { - SET_SETTINGS, +import { DEFAULT_SHORT_URLS_ORDERING, + settingsReducer, toggleRealTimeUpdates, setRealTimeUpdatesInterval, setShortUrlCreationSettings, @@ -20,7 +20,9 @@ describe('settingsReducer', () => { describe('reducer', () => { it('returns realTimeUpdates when action is SET_SETTINGS', () => { - expect(reducer(undefined, { type: SET_SETTINGS, realTimeUpdates })).toEqual(settings); + expect( + settingsReducer(undefined, { type: toggleRealTimeUpdates.toString(), payload: { realTimeUpdates } }), + ).toEqual(settings); }); }); @@ -28,7 +30,10 @@ describe('settingsReducer', () => { it.each([[true], [false]])('updates settings with provided value and then loads updates again', (enabled) => { const result = toggleRealTimeUpdates(enabled); - expect(result).toEqual({ type: SET_SETTINGS, realTimeUpdates: { enabled } }); + expect(result).toEqual({ + type: toggleRealTimeUpdates.toString(), + payload: { realTimeUpdates: { enabled } }, + }); }); }); @@ -36,7 +41,10 @@ describe('settingsReducer', () => { it.each([[0], [1], [2], [10]])('updates settings with provided value and then loads updates again', (interval) => { const result = setRealTimeUpdatesInterval(interval); - expect(result).toEqual({ type: SET_SETTINGS, realTimeUpdates: { interval } }); + expect(result).toEqual({ + type: setRealTimeUpdatesInterval.toString(), + payload: { realTimeUpdates: { interval } }, + }); }); }); @@ -44,7 +52,10 @@ describe('settingsReducer', () => { it('creates action to set shortUrlCreation settings', () => { const result = setShortUrlCreationSettings({ validateUrls: true }); - expect(result).toEqual({ type: SET_SETTINGS, shortUrlCreation: { validateUrls: true } }); + expect(result).toEqual({ + type: setShortUrlCreationSettings.toString(), + payload: { shortUrlCreation: { validateUrls: true } }, + }); }); }); @@ -52,7 +63,10 @@ describe('settingsReducer', () => { it('creates action to set ui settings', () => { const result = setUiSettings({ theme: 'dark' }); - expect(result).toEqual({ type: SET_SETTINGS, ui: { theme: 'dark' } }); + expect(result).toEqual({ + type: setUiSettings.toString(), + payload: { ui: { theme: 'dark' } }, + }); }); }); @@ -60,7 +74,10 @@ describe('settingsReducer', () => { it('creates action to set visits settings', () => { const result = setVisitsSettings({ defaultInterval: 'last180Days' }); - expect(result).toEqual({ type: SET_SETTINGS, visits: { defaultInterval: 'last180Days' } }); + expect(result).toEqual({ + type: setVisitsSettings.toString(), + payload: { visits: { defaultInterval: 'last180Days' } }, + }); }); }); @@ -68,7 +85,10 @@ describe('settingsReducer', () => { it('creates action to set tags settings', () => { const result = setTagsSettings({ defaultMode: 'list' }); - expect(result).toEqual({ type: SET_SETTINGS, tags: { defaultMode: 'list' } }); + expect(result).toEqual({ + type: setTagsSettings.toString(), + payload: { tags: { defaultMode: 'list' } }, + }); }); }); @@ -76,7 +96,10 @@ describe('settingsReducer', () => { it('creates action to set short URLs list settings', () => { const result = setShortUrlsListSettings({ defaultOrdering: DEFAULT_SHORT_URLS_ORDERING }); - expect(result).toEqual({ type: SET_SETTINGS, shortUrlsList: { defaultOrdering: DEFAULT_SHORT_URLS_ORDERING } }); + expect(result).toEqual({ + type: setShortUrlsListSettings.toString(), + payload: { shortUrlsList: { defaultOrdering: DEFAULT_SHORT_URLS_ORDERING } }, + }); }); }); }); From a316366ae9c93b73a1ab7be69561d46155a27821 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 6 Nov 2022 10:11:44 +0100 Subject: [PATCH 04/15] Migrated shortUrlCreation reducer to RTK --- src/reducers/index.ts | 3 +- src/short-urls/reducers/shortUrlCreation.ts | 61 ++++++++-------- src/short-urls/reducers/shortUrlsList.ts | 6 +- src/short-urls/services/provideServices.ts | 11 ++- src/tags/reducers/tagsList.ts | 4 +- .../reducers/shortUrlCreation.test.ts | 72 +++++++++---------- .../short-urls/reducers/shortUrlsList.test.ts | 2 +- test/tags/reducers/tagsList.test.ts | 4 +- 8 files changed, 80 insertions(+), 83 deletions(-) diff --git a/src/reducers/index.ts b/src/reducers/index.ts index 61bd0139..885033ce 100644 --- a/src/reducers/index.ts +++ b/src/reducers/index.ts @@ -3,7 +3,6 @@ import { combineReducers } from 'redux'; import { serversReducer } from '../servers/reducers/servers'; import selectedServerReducer from '../servers/reducers/selectedServer'; import shortUrlsListReducer from '../short-urls/reducers/shortUrlsList'; -import shortUrlCreationReducer from '../short-urls/reducers/shortUrlCreation'; import shortUrlDeletionReducer from '../short-urls/reducers/shortUrlDeletion'; import shortUrlEditionReducer from '../short-urls/reducers/shortUrlEdition'; import shortUrlVisitsReducer from '../visits/reducers/shortUrlVisits'; @@ -25,7 +24,7 @@ export default (container: IContainer) => combineReducers({ servers: serversReducer, selectedServer: selectedServerReducer, shortUrlsList: shortUrlsListReducer, - shortUrlCreationResult: shortUrlCreationReducer, + shortUrlCreationResult: container.shortUrlCreationReducer, shortUrlDeletion: shortUrlDeletionReducer, shortUrlEdition: shortUrlEditionReducer, shortUrlVisits: shortUrlVisitsReducer, diff --git a/src/short-urls/reducers/shortUrlCreation.ts b/src/short-urls/reducers/shortUrlCreation.ts index af7c1146..7bdc91f8 100644 --- a/src/short-urls/reducers/shortUrlCreation.ts +++ b/src/short-urls/reducers/shortUrlCreation.ts @@ -1,16 +1,11 @@ -import { Action, Dispatch } from 'redux'; -import { GetState } from '../../container/types'; +import { createSlice, PayloadAction } from '@reduxjs/toolkit'; import { ShortUrl, ShortUrlData } from '../data'; -import { buildReducer, buildActionCreator } from '../../utils/helpers/redux'; +import { createAsyncThunk } from '../../utils/helpers/redux'; import { ShlinkApiClientBuilder } from '../../api/services/ShlinkApiClientBuilder'; import { parseApiError } from '../../api/utils'; -import { ApiErrorAction } from '../../api/types/actions'; import { ProblemDetailsError } from '../../api/types/errors'; -export const CREATE_SHORT_URL_START = 'shlink/createShortUrl/CREATE_SHORT_URL_START'; -export const CREATE_SHORT_URL_ERROR = 'shlink/createShortUrl/CREATE_SHORT_URL_ERROR'; export const CREATE_SHORT_URL = 'shlink/createShortUrl/CREATE_SHORT_URL'; -export const RESET_CREATE_SHORT_URL = 'shlink/createShortUrl/RESET_CREATE_SHORT_URL'; export interface ShortUrlCreation { result: ShortUrl | null; @@ -19,9 +14,7 @@ export interface ShortUrlCreation { errorData?: ProblemDetailsError; } -export interface CreateShortUrlAction extends Action { - result: ShortUrl; -} +export type CreateShortUrlAction = PayloadAction; const initialState: ShortUrlCreation = { result: null, @@ -29,29 +22,33 @@ const initialState: ShortUrlCreation = { error: false, }; -export default buildReducer({ - [CREATE_SHORT_URL_START]: (state) => ({ ...state, saving: true, error: false }), - [CREATE_SHORT_URL_ERROR]: (state, { errorData }) => ({ ...state, saving: false, error: true, errorData }), - [CREATE_SHORT_URL]: (_, { result }) => ({ result, saving: false, error: false }), - [RESET_CREATE_SHORT_URL]: () => initialState, -}, initialState); +export const shortUrlCreationReducerCreator = (buildShlinkApiClient: ShlinkApiClientBuilder) => { + const createShortUrl = createAsyncThunk(CREATE_SHORT_URL, (data: ShortUrlData, { getState }): Promise => { + const { createShortUrl: shlinkCreateShortUrl } = buildShlinkApiClient(getState); + return shlinkCreateShortUrl(data); + }); -export const createShortUrl = (buildShlinkApiClient: ShlinkApiClientBuilder) => (data: ShortUrlData) => async ( - dispatch: Dispatch, - getState: GetState, -) => { - dispatch({ type: CREATE_SHORT_URL_START }); - const { createShortUrl: shlinkCreateShortUrl } = buildShlinkApiClient(getState); + const { reducer, actions } = createSlice({ + name: 'shortUrlCreationReducer', + initialState, + reducers: { + resetCreateShortUrl: () => initialState, + }, + extraReducers: (builder) => { + builder.addCase(createShortUrl.pending, (state) => ({ ...state, saving: true, error: false })); + builder.addCase( + createShortUrl.rejected, + (state, { error }) => ({ ...state, saving: false, error: true, errorData: parseApiError(error) }), + ); + builder.addCase(createShortUrl.fulfilled, (_, { payload: result }) => ({ result, saving: false, error: false })); + }, + }); - try { - const result = await shlinkCreateShortUrl(data); + const { resetCreateShortUrl } = actions; - dispatch({ type: CREATE_SHORT_URL, result }); - } catch (e: any) { - dispatch({ type: CREATE_SHORT_URL_ERROR, errorData: parseApiError(e) }); - - throw e; - } + return { + reducer, + createShortUrl, + resetCreateShortUrl, + }; }; - -export const resetCreateShortUrl = buildActionCreator(RESET_CREATE_SHORT_URL); diff --git a/src/short-urls/reducers/shortUrlsList.ts b/src/short-urls/reducers/shortUrlsList.ts index 7137a41d..7f826ad7 100644 --- a/src/short-urls/reducers/shortUrlsList.ts +++ b/src/short-urls/reducers/shortUrlsList.ts @@ -74,13 +74,13 @@ export default buildReducer({ ), state, ), - [CREATE_SHORT_URL]: pipe( + [`${CREATE_SHORT_URL}/fulfilled`]: pipe( // TODO Do not hardcode action type here // The only place where the list and the creation form coexist is the overview page. // There we can assume we are displaying page 1, and therefore, we can safely prepend the new short URL. // We can also remove the items above the amount that is displayed there. - (state: ShortUrlsList, { result }: CreateShortUrlAction) => (!state.shortUrls ? state : assocPath( + (state: ShortUrlsList, { payload }: CreateShortUrlAction) => (!state.shortUrls ? state : assocPath( ['shortUrls', 'data'], - [result, ...state.shortUrls.data.slice(0, ITEMS_IN_OVERVIEW_PAGE - 1)], + [payload, ...state.shortUrls.data.slice(0, ITEMS_IN_OVERVIEW_PAGE - 1)], state, )), (state: ShortUrlsList) => (!state.shortUrls ? state : assocPath( diff --git a/src/short-urls/services/provideServices.ts b/src/short-urls/services/provideServices.ts index a19ca9b6..d357e0fb 100644 --- a/src/short-urls/services/provideServices.ts +++ b/src/short-urls/services/provideServices.ts @@ -1,4 +1,5 @@ import Bottle from 'bottlejs'; +import { prop } from 'ramda'; import { ShortUrlsFilteringBar } from '../ShortUrlsFilteringBar'; import { ShortUrlsList } from '../ShortUrlsList'; import { ShortUrlsRow } from '../helpers/ShortUrlsRow'; @@ -7,7 +8,7 @@ import { CreateShortUrl } from '../CreateShortUrl'; import { DeleteShortUrlModal } from '../helpers/DeleteShortUrlModal'; import { CreateShortUrlResult } from '../helpers/CreateShortUrlResult'; import { listShortUrls } from '../reducers/shortUrlsList'; -import { createShortUrl, resetCreateShortUrl } from '../reducers/shortUrlCreation'; +import { shortUrlCreationReducerCreator } from '../reducers/shortUrlCreation'; import { deleteShortUrl, resetDeleteShortUrl } from '../reducers/shortUrlDeletion'; import { editShortUrl } from '../reducers/shortUrlEdition'; import { ConnectDecorator } from '../../container/types'; @@ -55,11 +56,15 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator) => { bottle.serviceFactory('ExportShortUrlsBtn', ExportShortUrlsBtn, 'buildShlinkApiClient', 'ReportExporter'); bottle.decorator('ExportShortUrlsBtn', connect(['selectedServer'])); + // Reducers + bottle.serviceFactory('shortUrlCreationReducerCreator', shortUrlCreationReducerCreator, 'buildShlinkApiClient'); + bottle.serviceFactory('shortUrlCreationReducer', prop('reducer'), 'shortUrlCreationReducerCreator'); + // Actions bottle.serviceFactory('listShortUrls', listShortUrls, 'buildShlinkApiClient'); - bottle.serviceFactory('createShortUrl', createShortUrl, 'buildShlinkApiClient'); - bottle.serviceFactory('resetCreateShortUrl', () => resetCreateShortUrl); + bottle.serviceFactory('createShortUrl', prop('createShortUrl'), 'shortUrlCreationReducerCreator'); + bottle.serviceFactory('resetCreateShortUrl', prop('resetCreateShortUrl'), 'shortUrlCreationReducerCreator'); bottle.serviceFactory('deleteShortUrl', deleteShortUrl, 'buildShlinkApiClient'); bottle.serviceFactory('resetDeleteShortUrl', () => resetDeleteShortUrl); diff --git a/src/tags/reducers/tagsList.ts b/src/tags/reducers/tagsList.ts index a31c060d..485b55f4 100644 --- a/src/tags/reducers/tagsList.ts +++ b/src/tags/reducers/tagsList.ts @@ -103,9 +103,9 @@ export default buildReducer({ ...state, stats: increaseVisitsForTags(calculateVisitsPerTag(payload.createdVisits), state.stats), }), - [CREATE_SHORT_URL]: ({ tags: stateTags, ...rest }, { result }) => ({ + [`${CREATE_SHORT_URL}/fulfilled`]: ({ tags: stateTags, ...rest }, { payload }) => ({ // TODO Do not hardcode action type here ...rest, - tags: stateTags.concat(result.tags.filter((tag) => !stateTags.includes(tag))), // More performant than [ ...new Set(...) ] + tags: stateTags.concat(payload.tags.filter((tag) => !stateTags.includes(tag))), // More performant than [ ...new Set(...) ] }), }, initialState); diff --git a/test/short-urls/reducers/shortUrlCreation.test.ts b/test/short-urls/reducers/shortUrlCreation.test.ts index 25128bc4..94b5f02b 100644 --- a/test/short-urls/reducers/shortUrlCreation.test.ts +++ b/test/short-urls/reducers/shortUrlCreation.test.ts @@ -1,19 +1,19 @@ import { Mock } from 'ts-mockery'; -import reducer, { - CREATE_SHORT_URL_START, - CREATE_SHORT_URL_ERROR, - CREATE_SHORT_URL, - RESET_CREATE_SHORT_URL, - createShortUrl, - resetCreateShortUrl, +import { CreateShortUrlAction, + shortUrlCreationReducerCreator, } from '../../../src/short-urls/reducers/shortUrlCreation'; import { ShortUrl } from '../../../src/short-urls/data'; import { ShlinkApiClient } from '../../../src/api/services/ShlinkApiClient'; import { ShlinkState } from '../../../src/container/types'; describe('shortUrlCreationReducer', () => { - const shortUrl = Mock.all(); + const shortUrl = Mock.of(); + const createShortUrlCall = jest.fn(); + const buildShlinkApiClient = () => Mock.of({ createShortUrl: createShortUrlCall }); + const { reducer, createShortUrl, resetCreateShortUrl } = shortUrlCreationReducerCreator(buildShlinkApiClient); + + afterEach(jest.resetAllMocks); describe('reducer', () => { const action = (type: string, args: Partial = {}) => Mock.of( @@ -21,7 +21,7 @@ describe('shortUrlCreationReducer', () => { ); it('returns loading on CREATE_SHORT_URL_START', () => { - expect(reducer(undefined, action(CREATE_SHORT_URL_START))).toEqual({ + expect(reducer(undefined, action(createShortUrl.pending.toString()))).toEqual({ result: null, saving: true, error: false, @@ -29,7 +29,7 @@ describe('shortUrlCreationReducer', () => { }); it('returns error on CREATE_SHORT_URL_ERROR', () => { - expect(reducer(undefined, action(CREATE_SHORT_URL_ERROR))).toEqual({ + expect(reducer(undefined, action(createShortUrl.rejected.toString()))).toEqual({ result: null, saving: false, error: true, @@ -37,7 +37,7 @@ describe('shortUrlCreationReducer', () => { }); it('returns result on CREATE_SHORT_URL', () => { - expect(reducer(undefined, action(CREATE_SHORT_URL, { result: shortUrl }))).toEqual({ + expect(reducer(undefined, action(createShortUrl.fulfilled.toString(), { payload: shortUrl }))).toEqual({ result: shortUrl, saving: false, error: false, @@ -45,7 +45,7 @@ describe('shortUrlCreationReducer', () => { }); it('returns default state on RESET_CREATE_SHORT_URL', () => { - expect(reducer(undefined, action(RESET_CREATE_SHORT_URL))).toEqual({ + expect(reducer(undefined, action(resetCreateShortUrl.toString()))).toEqual({ result: null, saving: false, error: false, @@ -54,47 +54,43 @@ describe('shortUrlCreationReducer', () => { }); describe('resetCreateShortUrl', () => { - it('returns proper action', () => expect(resetCreateShortUrl()).toEqual({ type: RESET_CREATE_SHORT_URL })); + it('returns proper action', () => expect(resetCreateShortUrl()).toEqual({ type: resetCreateShortUrl.toString() })); }); describe('createShortUrl', () => { - const createApiClientMock = (result: Promise) => Mock.of({ - createShortUrl: jest.fn().mockReturnValue(result), - }); const dispatch = jest.fn(); const getState = () => Mock.all(); - afterEach(jest.resetAllMocks); - it('calls API on success', async () => { - const apiClientMock = createApiClientMock(Promise.resolve(shortUrl)); - const dispatchable = createShortUrl(() => apiClientMock)({ longUrl: 'foo' }); + createShortUrlCall.mockResolvedValue(shortUrl); + await createShortUrl({ longUrl: 'foo' })(dispatch, getState, {}); - await dispatchable(dispatch, getState); - - expect(apiClientMock.createShortUrl).toHaveBeenCalledTimes(1); + expect(createShortUrlCall).toHaveBeenCalledTimes(1); expect(dispatch).toHaveBeenCalledTimes(2); - expect(dispatch).toHaveBeenNthCalledWith(1, { type: CREATE_SHORT_URL_START }); - expect(dispatch).toHaveBeenNthCalledWith(2, { type: CREATE_SHORT_URL, result: shortUrl }); + expect(dispatch).toHaveBeenNthCalledWith(1, expect.objectContaining({ + type: createShortUrl.pending.toString(), + })); + expect(dispatch).toHaveBeenNthCalledWith(2, expect.objectContaining({ + type: createShortUrl.fulfilled.toString(), + payload: shortUrl, + })); }); it('throws on error', async () => { - const error = 'Error'; - const apiClientMock = createApiClientMock(Promise.reject(error)); - const dispatchable = createShortUrl(() => apiClientMock)({ longUrl: 'foo' }); + const error = new Error('Error message'); + createShortUrlCall.mockRejectedValue(error); - expect.assertions(5); + await createShortUrl({ longUrl: 'foo' })(dispatch, getState, {}); - try { - await dispatchable(dispatch, getState); - } catch (e) { - expect(e).toEqual(error); - } - - expect(apiClientMock.createShortUrl).toHaveBeenCalledTimes(1); + expect(createShortUrlCall).toHaveBeenCalledTimes(1); expect(dispatch).toHaveBeenCalledTimes(2); - expect(dispatch).toHaveBeenNthCalledWith(1, { type: CREATE_SHORT_URL_START }); - expect(dispatch).toHaveBeenNthCalledWith(2, { type: CREATE_SHORT_URL_ERROR }); + expect(dispatch).toHaveBeenNthCalledWith(1, expect.objectContaining({ + type: createShortUrl.pending.toString(), + })); + expect(dispatch).toHaveBeenNthCalledWith(2, expect.objectContaining({ + type: createShortUrl.rejected.toString(), + error: expect.objectContaining({ message: 'Error message' }), + })); }); }); }); diff --git a/test/short-urls/reducers/shortUrlsList.test.ts b/test/short-urls/reducers/shortUrlsList.test.ts index a4648121..54349fb5 100644 --- a/test/short-urls/reducers/shortUrlsList.test.ts +++ b/test/short-urls/reducers/shortUrlsList.test.ts @@ -142,7 +142,7 @@ describe('shortUrlsListReducer', () => { error: false, }; - expect(reducer(state, { type: CREATE_SHORT_URL, result: newShortUrl } as any)).toEqual({ + expect(reducer(state, { type: `${CREATE_SHORT_URL}/fulfilled`, payload: newShortUrl } as any)).toEqual({ shortUrls: { data: expectedData, pagination: { totalItems: 16 }, diff --git a/test/tags/reducers/tagsList.test.ts b/test/tags/reducers/tagsList.test.ts index 767cdc3d..277d3c28 100644 --- a/test/tags/reducers/tagsList.test.ts +++ b/test/tags/reducers/tagsList.test.ts @@ -83,9 +83,9 @@ describe('tagsListReducer', () => { [['new', 'tag'], ['foo', 'bar', 'baz', 'foo2', 'fo', 'new', 'tag']], ])('appends new short URL\'s tags to the list of tags on CREATE_SHORT_URL', (shortUrlTags, expectedTags) => { const tags = ['foo', 'bar', 'baz', 'foo2', 'fo']; - const result = Mock.of({ tags: shortUrlTags }); + const payload = Mock.of({ tags: shortUrlTags }); - expect(reducer(state({ tags }), { type: CREATE_SHORT_URL, result } as any)).toEqual({ + expect(reducer(state({ tags }), { type: `${CREATE_SHORT_URL}/fulfilled`, payload } as any)).toEqual({ tags: expectedTags, }); }); From bf84e4a2edec166195d20703b99b43d38da77b1e Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 6 Nov 2022 11:53:23 +0100 Subject: [PATCH 05/15] Migrated editShortUrl payload action --- src/api/types/actions.ts | 1 + src/short-urls/reducers/shortUrlEdition.ts | 13 ++++++------- src/short-urls/reducers/shortUrlsList.ts | 2 +- src/utils/helpers/redux.ts | 2 ++ test/short-urls/reducers/shortUrlEdition.test.ts | 4 ++-- test/short-urls/reducers/shortUrlsList.test.ts | 2 +- 6 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/api/types/actions.ts b/src/api/types/actions.ts index b60b4c9c..2c8f6d38 100644 --- a/src/api/types/actions.ts +++ b/src/api/types/actions.ts @@ -1,6 +1,7 @@ import { Action } from 'redux'; import { ProblemDetailsError } from './errors'; +/** @deprecated */ export interface ApiErrorAction extends Action { errorData?: ProblemDetailsError; } diff --git a/src/short-urls/reducers/shortUrlEdition.ts b/src/short-urls/reducers/shortUrlEdition.ts index 72bf437d..64d374a5 100644 --- a/src/short-urls/reducers/shortUrlEdition.ts +++ b/src/short-urls/reducers/shortUrlEdition.ts @@ -1,4 +1,5 @@ -import { Action, Dispatch } from 'redux'; +import { PayloadAction } from '@reduxjs/toolkit'; +import { Dispatch } from 'redux'; import { buildReducer } from '../../utils/helpers/redux'; import { GetState } from '../../container/types'; import { OptionalString } from '../../utils/utils'; @@ -19,9 +20,7 @@ export interface ShortUrlEdition { errorData?: ProblemDetailsError; } -export interface ShortUrlEditedAction extends Action { - shortUrl: ShortUrl; -} +export type ShortUrlEditedAction = PayloadAction; const initialState: ShortUrlEdition = { saving: false, @@ -31,7 +30,7 @@ const initialState: ShortUrlEdition = { export default buildReducer({ [EDIT_SHORT_URL_START]: (state) => ({ ...state, saving: true, error: false }), [EDIT_SHORT_URL_ERROR]: (state, { errorData }) => ({ ...state, saving: false, error: true, errorData }), - [SHORT_URL_EDITED]: (_, { shortUrl }) => ({ shortUrl, saving: false, error: false }), + [SHORT_URL_EDITED]: (_, { payload: shortUrl }) => ({ shortUrl, saving: false, error: false }), }, initialState); export const editShortUrl = (buildShlinkApiClient: ShlinkApiClientBuilder) => ( @@ -44,9 +43,9 @@ export const editShortUrl = (buildShlinkApiClient: ShlinkApiClientBuilder) => ( const { updateShortUrl } = buildShlinkApiClient(getState); try { - const shortUrl = await updateShortUrl(shortCode, domain, data as any); // FIXME parse dates; + const payload = await updateShortUrl(shortCode, domain, data as any); // FIXME parse dates; - dispatch({ shortUrl, type: SHORT_URL_EDITED }); + dispatch({ payload, type: SHORT_URL_EDITED }); } catch (e: any) { dispatch({ type: EDIT_SHORT_URL_ERROR, errorData: parseApiError(e) }); diff --git a/src/short-urls/reducers/shortUrlsList.ts b/src/short-urls/reducers/shortUrlsList.ts index 7f826ad7..136bd164 100644 --- a/src/short-urls/reducers/shortUrlsList.ts +++ b/src/short-urls/reducers/shortUrlsList.ts @@ -89,7 +89,7 @@ export default buildReducer({ state, )), ), - [SHORT_URL_EDITED]: (state, { shortUrl: editedShortUrl }) => (!state.shortUrls ? state : assocPath( + [SHORT_URL_EDITED]: (state, { payload: editedShortUrl }) => (!state.shortUrls ? state : assocPath( ['shortUrls', 'data'], state.shortUrls.data.map((shortUrl) => { const { shortCode, domain } = editedShortUrl; diff --git a/src/utils/helpers/redux.ts b/src/utils/helpers/redux.ts index f8962883..8ae09e05 100644 --- a/src/utils/helpers/redux.ts +++ b/src/utils/helpers/redux.ts @@ -5,6 +5,7 @@ import { ShlinkState } from '../../container/types'; type ActionHandler = (currentState: State, action: AT) => State; type ActionHandlerMap = Record>; +/** @deprecated */ export const buildReducer = (map: ActionHandlerMap, initialState: State) => ( state: State | undefined, action: AT, @@ -16,6 +17,7 @@ export const buildReducer = (map: ActionHandlerMap(type: T) => (): Action => ({ type }); export const createAsyncThunk = ( diff --git a/test/short-urls/reducers/shortUrlEdition.test.ts b/test/short-urls/reducers/shortUrlEdition.test.ts index 313d7556..da67e589 100644 --- a/test/short-urls/reducers/shortUrlEdition.test.ts +++ b/test/short-urls/reducers/shortUrlEdition.test.ts @@ -31,7 +31,7 @@ describe('shortUrlEditionReducer', () => { }); it('returns provided tags and shortCode on SHORT_URL_EDITED', () => { - expect(reducer(undefined, { type: SHORT_URL_EDITED, shortUrl })).toEqual({ + expect(reducer(undefined, { type: SHORT_URL_EDITED, payload: shortUrl })).toEqual({ shortUrl, saving: false, error: false, @@ -55,7 +55,7 @@ describe('shortUrlEditionReducer', () => { expect(updateShortUrl).toHaveBeenCalledWith(shortCode, domain, { longUrl }); expect(dispatch).toHaveBeenCalledTimes(2); expect(dispatch).toHaveBeenNthCalledWith(1, { type: EDIT_SHORT_URL_START }); - expect(dispatch).toHaveBeenNthCalledWith(2, { type: SHORT_URL_EDITED, shortUrl }); + expect(dispatch).toHaveBeenNthCalledWith(2, { type: SHORT_URL_EDITED, payload: shortUrl }); }); it('dispatches error on failure', async () => { diff --git a/test/short-urls/reducers/shortUrlsList.test.ts b/test/short-urls/reducers/shortUrlsList.test.ts index 54349fb5..bd2dbb93 100644 --- a/test/short-urls/reducers/shortUrlsList.test.ts +++ b/test/short-urls/reducers/shortUrlsList.test.ts @@ -181,7 +181,7 @@ describe('shortUrlsListReducer', () => { error: false, }; - const result = reducer(state, { type: SHORT_URL_EDITED, shortUrl: editedShortUrl } as any); + const result = reducer(state, { type: SHORT_URL_EDITED, payload: editedShortUrl } as any); expect(result.shortUrls?.data).toEqual(expectedList); }); From 77cbb8ebc4ed35a06c219fdc8f43e5b54c65dcec Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 6 Nov 2022 11:59:39 +0100 Subject: [PATCH 06/15] Refactored editShortUrl action to require just one param --- src/short-urls/EditShortUrl.tsx | 7 +++---- src/short-urls/reducers/shortUrlEdition.ts | 10 +++++++--- test/short-urls/reducers/shortUrlEdition.test.ts | 4 ++-- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/short-urls/EditShortUrl.tsx b/src/short-urls/EditShortUrl.tsx index 3c243a25..8445bfc7 100644 --- a/src/short-urls/EditShortUrl.tsx +++ b/src/short-urls/EditShortUrl.tsx @@ -14,8 +14,7 @@ import { ShlinkApiError } from '../api/ShlinkApiError'; import { useGoBack, useToggle } from '../utils/helpers/hooks'; import { ShortUrlFormProps } from './ShortUrlForm'; import { ShortUrlDetail } from './reducers/shortUrlDetail'; -import { EditShortUrlData } from './data'; -import { ShortUrlEdition } from './reducers/shortUrlEdition'; +import { EditShortUrl as EditShortUrlInfo, ShortUrlEdition } from './reducers/shortUrlEdition'; import { shortUrlDataFromShortUrl, urlDecodeShortCode } from './helpers'; interface EditShortUrlConnectProps { @@ -24,7 +23,7 @@ interface EditShortUrlConnectProps { shortUrlDetail: ShortUrlDetail; shortUrlEdition: ShortUrlEdition; getShortUrlDetail: (shortCode: string, domain: OptionalString) => void; - editShortUrl: (shortUrl: string, domain: OptionalString, data: EditShortUrlData) => Promise; + editShortUrl: (editShortUrl: EditShortUrlInfo) => Promise; } export const EditShortUrl = (ShortUrlForm: FC) => ({ @@ -89,7 +88,7 @@ export const EditShortUrl = (ShortUrlForm: FC) => ({ } isNotSuccessful(); - editShortUrl(shortUrl.shortCode, shortUrl.domain, shortUrlData) + editShortUrl({ ...shortUrl, data: shortUrlData }) .then(isSuccessful) .catch(isNotSuccessful); }} diff --git a/src/short-urls/reducers/shortUrlEdition.ts b/src/short-urls/reducers/shortUrlEdition.ts index 64d374a5..e446c33e 100644 --- a/src/short-urls/reducers/shortUrlEdition.ts +++ b/src/short-urls/reducers/shortUrlEdition.ts @@ -20,6 +20,12 @@ export interface ShortUrlEdition { errorData?: ProblemDetailsError; } +export interface EditShortUrl { + shortCode: string; + domain?: OptionalString; + data: EditShortUrlData; +} + export type ShortUrlEditedAction = PayloadAction; const initialState: ShortUrlEdition = { @@ -34,9 +40,7 @@ export default buildReducer ( - shortCode: string, - domain: OptionalString, - data: EditShortUrlData, + { shortCode, domain, data }: EditShortUrl, ) => async (dispatch: Dispatch, getState: GetState) => { dispatch({ type: EDIT_SHORT_URL_START }); diff --git a/test/short-urls/reducers/shortUrlEdition.test.ts b/test/short-urls/reducers/shortUrlEdition.test.ts index da67e589..792b33ab 100644 --- a/test/short-urls/reducers/shortUrlEdition.test.ts +++ b/test/short-urls/reducers/shortUrlEdition.test.ts @@ -48,7 +48,7 @@ describe('shortUrlEditionReducer', () => { afterEach(jest.clearAllMocks); it.each([[undefined], [null], ['example.com']])('dispatches short URL on success', async (domain) => { - await editShortUrl(buildShlinkApiClient)(shortCode, domain, { longUrl })(dispatch, createGetState()); + await editShortUrl(buildShlinkApiClient)({ shortCode, domain, data: { longUrl } })(dispatch, createGetState()); expect(buildShlinkApiClient).toHaveBeenCalledTimes(1); expect(updateShortUrl).toHaveBeenCalledTimes(1); @@ -64,7 +64,7 @@ describe('shortUrlEditionReducer', () => { updateShortUrl.mockRejectedValue(error); try { - await editShortUrl(buildShlinkApiClient)(shortCode, undefined, { longUrl })(dispatch, createGetState()); + await editShortUrl(buildShlinkApiClient)({ shortCode, data: { longUrl } })(dispatch, createGetState()); } catch (e) { expect(e).toBe(error); } From 2a268de2cb37eaa134839020d3ffc5178c728f67 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 6 Nov 2022 12:32:55 +0100 Subject: [PATCH 07/15] Migrated editShortUrl reducer to RTK --- src/reducers/index.ts | 3 +- src/short-urls/EditShortUrl.tsx | 16 ++---- src/short-urls/reducers/shortUrlEdition.ts | 56 ++++++++++--------- src/short-urls/reducers/shortUrlsList.ts | 2 +- src/short-urls/services/provideServices.ts | 7 ++- test/short-urls/EditShortUrl.test.tsx | 9 ++- .../reducers/shortUrlEdition.test.ts | 45 +++++++-------- .../short-urls/reducers/shortUrlsList.test.ts | 2 +- 8 files changed, 74 insertions(+), 66 deletions(-) diff --git a/src/reducers/index.ts b/src/reducers/index.ts index 885033ce..707fe5ca 100644 --- a/src/reducers/index.ts +++ b/src/reducers/index.ts @@ -4,7 +4,6 @@ import { serversReducer } from '../servers/reducers/servers'; import selectedServerReducer from '../servers/reducers/selectedServer'; import shortUrlsListReducer from '../short-urls/reducers/shortUrlsList'; import shortUrlDeletionReducer from '../short-urls/reducers/shortUrlDeletion'; -import shortUrlEditionReducer from '../short-urls/reducers/shortUrlEdition'; import shortUrlVisitsReducer from '../visits/reducers/shortUrlVisits'; import tagVisitsReducer from '../visits/reducers/tagVisits'; import domainVisitsReducer from '../visits/reducers/domainVisits'; @@ -26,7 +25,7 @@ export default (container: IContainer) => combineReducers({ shortUrlsList: shortUrlsListReducer, shortUrlCreationResult: container.shortUrlCreationReducer, shortUrlDeletion: shortUrlDeletionReducer, - shortUrlEdition: shortUrlEditionReducer, + shortUrlEdition: container.shortUrlEditionReducer, shortUrlVisits: shortUrlVisitsReducer, tagVisits: tagVisitsReducer, domainVisits: domainVisitsReducer, diff --git a/src/short-urls/EditShortUrl.tsx b/src/short-urls/EditShortUrl.tsx index 8445bfc7..58687d7e 100644 --- a/src/short-urls/EditShortUrl.tsx +++ b/src/short-urls/EditShortUrl.tsx @@ -11,7 +11,7 @@ import { parseQuery } from '../utils/helpers/query'; import { Message } from '../utils/Message'; import { Result } from '../utils/Result'; import { ShlinkApiError } from '../api/ShlinkApiError'; -import { useGoBack, useToggle } from '../utils/helpers/hooks'; +import { useGoBack } from '../utils/helpers/hooks'; import { ShortUrlFormProps } from './ShortUrlForm'; import { ShortUrlDetail } from './reducers/shortUrlDetail'; import { EditShortUrl as EditShortUrlInfo, ShortUrlEdition } from './reducers/shortUrlEdition'; @@ -23,7 +23,7 @@ interface EditShortUrlConnectProps { shortUrlDetail: ShortUrlDetail; shortUrlEdition: ShortUrlEdition; getShortUrlDetail: (shortCode: string, domain: OptionalString) => void; - editShortUrl: (editShortUrl: EditShortUrlInfo) => Promise; + editShortUrl: (editShortUrl: EditShortUrlInfo) => void; } export const EditShortUrl = (ShortUrlForm: FC) => ({ @@ -38,13 +38,12 @@ export const EditShortUrl = (ShortUrlForm: FC) => ({ const params = useParams<{ shortCode: string }>(); const goBack = useGoBack(); const { loading, error, errorData, shortUrl } = shortUrlDetail; - const { saving, error: savingError, errorData: savingErrorData } = shortUrlEdition; + const { saving, saved, error: savingError, errorData: savingErrorData } = shortUrlEdition; const { domain } = parseQuery<{ domain?: string }>(search); const initialState = useMemo( () => shortUrlDataFromShortUrl(shortUrl, shortUrlCreationSettings), [shortUrl, shortUrlCreationSettings], ); - const [savingSucceeded,, isSuccessful, isNotSuccessful] = useToggle(); useEffect(() => { params.shortCode && getShortUrlDetail(urlDecodeShortCode(params.shortCode), domain); @@ -87,18 +86,15 @@ export const EditShortUrl = (ShortUrlForm: FC) => ({ return; } - isNotSuccessful(); - editShortUrl({ ...shortUrl, data: shortUrlData }) - .then(isSuccessful) - .catch(isNotSuccessful); + editShortUrl({ ...shortUrl, data: shortUrlData }); }} /> - {savingError && ( + {saved && savingError && ( )} - {savingSucceeded && Short URL properly edited.} + {saved && !savingError && Short URL properly edited.} ); }; diff --git a/src/short-urls/reducers/shortUrlEdition.ts b/src/short-urls/reducers/shortUrlEdition.ts index e446c33e..462515ca 100644 --- a/src/short-urls/reducers/shortUrlEdition.ts +++ b/src/short-urls/reducers/shortUrlEdition.ts @@ -1,22 +1,18 @@ -import { PayloadAction } from '@reduxjs/toolkit'; -import { Dispatch } from 'redux'; -import { buildReducer } from '../../utils/helpers/redux'; -import { GetState } from '../../container/types'; +import { createSlice, PayloadAction } from '@reduxjs/toolkit'; +import { createAsyncThunk } from '../../utils/helpers/redux'; import { OptionalString } from '../../utils/utils'; import { EditShortUrlData, ShortUrl } from '../data'; import { ShlinkApiClientBuilder } from '../../api/services/ShlinkApiClientBuilder'; import { parseApiError } from '../../api/utils'; -import { ApiErrorAction } from '../../api/types/actions'; import { ProblemDetailsError } from '../../api/types/errors'; -export const EDIT_SHORT_URL_START = 'shlink/shortUrlEdition/EDIT_SHORT_URL_START'; -export const EDIT_SHORT_URL_ERROR = 'shlink/shortUrlEdition/EDIT_SHORT_URL_ERROR'; export const SHORT_URL_EDITED = 'shlink/shortUrlEdition/SHORT_URL_EDITED'; export interface ShortUrlEdition { shortUrl?: ShortUrl; saving: boolean; error: boolean; + saved: boolean; errorData?: ProblemDetailsError; } @@ -30,29 +26,35 @@ export type ShortUrlEditedAction = PayloadAction; const initialState: ShortUrlEdition = { saving: false, + saved: false, error: false, }; -export default buildReducer({ - [EDIT_SHORT_URL_START]: (state) => ({ ...state, saving: true, error: false }), - [EDIT_SHORT_URL_ERROR]: (state, { errorData }) => ({ ...state, saving: false, error: true, errorData }), - [SHORT_URL_EDITED]: (_, { payload: shortUrl }) => ({ shortUrl, saving: false, error: false }), -}, initialState); +export const shortUrlEditionReducerCreator = (buildShlinkApiClient: ShlinkApiClientBuilder) => { + const editShortUrl = createAsyncThunk( + SHORT_URL_EDITED, + ({ shortCode, domain, data }: EditShortUrl, { getState }): Promise => { + const { updateShortUrl } = buildShlinkApiClient(getState); + return updateShortUrl(shortCode, domain, data as any); // FIXME parse dates + }, + ); -export const editShortUrl = (buildShlinkApiClient: ShlinkApiClientBuilder) => ( - { shortCode, domain, data }: EditShortUrl, -) => async (dispatch: Dispatch, getState: GetState) => { - dispatch({ type: EDIT_SHORT_URL_START }); + const { reducer } = createSlice({ + name: 'shortUrlEditionReducer', + initialState, + reducers: {}, + extraReducers: (builder) => { + builder.addCase(editShortUrl.pending, (state) => ({ ...state, saving: true, error: false, saved: false })); + builder.addCase( + editShortUrl.rejected, + (state, { error }) => ({ ...state, saving: false, error: true, saved: false, errorData: parseApiError(error) }), + ); + builder.addCase( + editShortUrl.fulfilled, + (_, { payload: shortUrl }) => ({ shortUrl, saving: false, error: false, saved: true }), + ); + }, + }); - const { updateShortUrl } = buildShlinkApiClient(getState); - - try { - const payload = await updateShortUrl(shortCode, domain, data as any); // FIXME parse dates; - - dispatch({ payload, type: SHORT_URL_EDITED }); - } catch (e: any) { - dispatch({ type: EDIT_SHORT_URL_ERROR, errorData: parseApiError(e) }); - - throw e; - } + return { reducer, editShortUrl }; }; diff --git a/src/short-urls/reducers/shortUrlsList.ts b/src/short-urls/reducers/shortUrlsList.ts index 136bd164..e2246524 100644 --- a/src/short-urls/reducers/shortUrlsList.ts +++ b/src/short-urls/reducers/shortUrlsList.ts @@ -89,7 +89,7 @@ export default buildReducer({ state, )), ), - [SHORT_URL_EDITED]: (state, { payload: editedShortUrl }) => (!state.shortUrls ? state : assocPath( + [`${SHORT_URL_EDITED}/fulfilled`]: (state, { payload: editedShortUrl }) => (!state.shortUrls ? state : assocPath( ['shortUrls', 'data'], state.shortUrls.data.map((shortUrl) => { const { shortCode, domain } = editedShortUrl; diff --git a/src/short-urls/services/provideServices.ts b/src/short-urls/services/provideServices.ts index d357e0fb..aab487c4 100644 --- a/src/short-urls/services/provideServices.ts +++ b/src/short-urls/services/provideServices.ts @@ -10,7 +10,7 @@ import { CreateShortUrlResult } from '../helpers/CreateShortUrlResult'; import { listShortUrls } from '../reducers/shortUrlsList'; import { shortUrlCreationReducerCreator } from '../reducers/shortUrlCreation'; import { deleteShortUrl, resetDeleteShortUrl } from '../reducers/shortUrlDeletion'; -import { editShortUrl } from '../reducers/shortUrlEdition'; +import { shortUrlEditionReducerCreator } from '../reducers/shortUrlEdition'; import { ConnectDecorator } from '../../container/types'; import { ShortUrlsTable } from '../ShortUrlsTable'; import { QrCodeModal } from '../helpers/QrCodeModal'; @@ -60,6 +60,9 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator) => { bottle.serviceFactory('shortUrlCreationReducerCreator', shortUrlCreationReducerCreator, 'buildShlinkApiClient'); bottle.serviceFactory('shortUrlCreationReducer', prop('reducer'), 'shortUrlCreationReducerCreator'); + bottle.serviceFactory('shortUrlEditionReducerCreator', shortUrlEditionReducerCreator, 'buildShlinkApiClient'); + bottle.serviceFactory('shortUrlEditionReducer', prop('reducer'), 'shortUrlEditionReducerCreator'); + // Actions bottle.serviceFactory('listShortUrls', listShortUrls, 'buildShlinkApiClient'); @@ -71,7 +74,7 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator) => { bottle.serviceFactory('getShortUrlDetail', getShortUrlDetail, 'buildShlinkApiClient'); - bottle.serviceFactory('editShortUrl', editShortUrl, 'buildShlinkApiClient'); + bottle.serviceFactory('editShortUrl', prop('editShortUrl'), 'shortUrlEditionReducerCreator'); }; export default provideServices; diff --git a/test/short-urls/EditShortUrl.test.tsx b/test/short-urls/EditShortUrl.test.tsx index e97152fd..8af59969 100644 --- a/test/short-urls/EditShortUrl.test.tsx +++ b/test/short-urls/EditShortUrl.test.tsx @@ -46,9 +46,16 @@ describe('', () => { }); it('shows error when saving data has failed', () => { - setUp({}, { error: true }); + setUp({}, { error: true, saved: true }); expect(screen.getByText('An error occurred while updating short URL :(')).toBeInTheDocument(); expect(screen.getByText('ShortUrlForm')).toBeInTheDocument(); }); + + it('shows message when saving data succeeds', () => { + setUp({}, { error: false, saved: true }); + + expect(screen.getByText('Short URL properly edited.')).toBeInTheDocument(); + expect(screen.getByText('ShortUrlForm')).toBeInTheDocument(); + }); }); diff --git a/test/short-urls/reducers/shortUrlEdition.test.ts b/test/short-urls/reducers/shortUrlEdition.test.ts index 792b33ab..7b0ecc43 100644 --- a/test/short-urls/reducers/shortUrlEdition.test.ts +++ b/test/short-urls/reducers/shortUrlEdition.test.ts @@ -1,11 +1,5 @@ import { Mock } from 'ts-mockery'; -import reducer, { - EDIT_SHORT_URL_START, - EDIT_SHORT_URL_ERROR, - SHORT_URL_EDITED, - editShortUrl, - ShortUrlEditedAction, -} from '../../../src/short-urls/reducers/shortUrlEdition'; +import { ShortUrlEditedAction, shortUrlEditionReducerCreator } from '../../../src/short-urls/reducers/shortUrlEdition'; import { ShlinkState } from '../../../src/container/types'; import { ShortUrl } from '../../../src/short-urls/data'; import { SelectedServer } from '../../../src/servers/data'; @@ -14,48 +8,59 @@ describe('shortUrlEditionReducer', () => { const longUrl = 'https://shlink.io'; const shortCode = 'abc123'; const shortUrl = Mock.of({ longUrl, shortCode }); + const updateShortUrl = jest.fn().mockResolvedValue(shortUrl); + const buildShlinkApiClient = jest.fn().mockReturnValue({ updateShortUrl }); + const { reducer, editShortUrl } = shortUrlEditionReducerCreator(buildShlinkApiClient); + + afterEach(jest.clearAllMocks); describe('reducer', () => { it('returns loading on EDIT_SHORT_URL_START', () => { - expect(reducer(undefined, Mock.of({ type: EDIT_SHORT_URL_START }))).toEqual({ + expect(reducer(undefined, Mock.of({ type: editShortUrl.pending.toString() }))).toEqual({ saving: true, + saved: false, error: false, }); }); it('returns error on EDIT_SHORT_URL_ERROR', () => { - expect(reducer(undefined, Mock.of({ type: EDIT_SHORT_URL_ERROR }))).toEqual({ + expect(reducer(undefined, Mock.of({ type: editShortUrl.rejected.toString() }))).toEqual({ saving: false, + saved: false, error: true, }); }); it('returns provided tags and shortCode on SHORT_URL_EDITED', () => { - expect(reducer(undefined, { type: SHORT_URL_EDITED, payload: shortUrl })).toEqual({ + expect(reducer(undefined, { type: editShortUrl.fulfilled.toString(), payload: shortUrl })).toEqual({ shortUrl, saving: false, + saved: true, error: false, }); }); }); describe('editShortUrl', () => { - const updateShortUrl = jest.fn().mockResolvedValue(shortUrl); - const buildShlinkApiClient = jest.fn().mockReturnValue({ updateShortUrl }); const dispatch = jest.fn(); const createGetState = (selectedServer: SelectedServer = null) => () => Mock.of({ selectedServer }); afterEach(jest.clearAllMocks); it.each([[undefined], [null], ['example.com']])('dispatches short URL on success', async (domain) => { - await editShortUrl(buildShlinkApiClient)({ shortCode, domain, data: { longUrl } })(dispatch, createGetState()); + await editShortUrl({ shortCode, domain, data: { longUrl } })(dispatch, createGetState(), {}); expect(buildShlinkApiClient).toHaveBeenCalledTimes(1); expect(updateShortUrl).toHaveBeenCalledTimes(1); expect(updateShortUrl).toHaveBeenCalledWith(shortCode, domain, { longUrl }); expect(dispatch).toHaveBeenCalledTimes(2); - expect(dispatch).toHaveBeenNthCalledWith(1, { type: EDIT_SHORT_URL_START }); - expect(dispatch).toHaveBeenNthCalledWith(2, { type: SHORT_URL_EDITED, payload: shortUrl }); + expect(dispatch).toHaveBeenNthCalledWith(1, expect.objectContaining({ + type: editShortUrl.pending.toString(), + })); + expect(dispatch).toHaveBeenNthCalledWith(2, expect.objectContaining({ + type: editShortUrl.fulfilled.toString(), + payload: shortUrl, + })); }); it('dispatches error on failure', async () => { @@ -63,18 +68,14 @@ describe('shortUrlEditionReducer', () => { updateShortUrl.mockRejectedValue(error); - try { - await editShortUrl(buildShlinkApiClient)({ shortCode, data: { longUrl } })(dispatch, createGetState()); - } catch (e) { - expect(e).toBe(error); - } + await editShortUrl({ shortCode, data: { longUrl } })(dispatch, createGetState(), {}); expect(buildShlinkApiClient).toHaveBeenCalledTimes(1); expect(updateShortUrl).toHaveBeenCalledTimes(1); expect(updateShortUrl).toHaveBeenCalledWith(shortCode, undefined, { longUrl }); expect(dispatch).toHaveBeenCalledTimes(2); - expect(dispatch).toHaveBeenNthCalledWith(1, { type: EDIT_SHORT_URL_START }); - expect(dispatch).toHaveBeenNthCalledWith(2, { type: EDIT_SHORT_URL_ERROR }); + expect(dispatch).toHaveBeenNthCalledWith(1, expect.objectContaining({ type: editShortUrl.pending.toString() })); + expect(dispatch).toHaveBeenNthCalledWith(2, expect.objectContaining({ type: editShortUrl.rejected.toString() })); }); }); }); diff --git a/test/short-urls/reducers/shortUrlsList.test.ts b/test/short-urls/reducers/shortUrlsList.test.ts index bd2dbb93..38c4a24d 100644 --- a/test/short-urls/reducers/shortUrlsList.test.ts +++ b/test/short-urls/reducers/shortUrlsList.test.ts @@ -181,7 +181,7 @@ describe('shortUrlsListReducer', () => { error: false, }; - const result = reducer(state, { type: SHORT_URL_EDITED, payload: editedShortUrl } as any); + const result = reducer(state, { type: `${SHORT_URL_EDITED}/fulfilled`, payload: editedShortUrl } as any); expect(result.shortUrls?.data).toEqual(expectedList); }); From d468fb1efeebe6d6593478276c12f5e3b9d649b3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 6 Nov 2022 12:46:29 +0100 Subject: [PATCH 08/15] Migrated deleteShortUrl action creator to use PayloadAction and have a single param --- .../helpers/DeleteShortUrlModal.tsx | 14 +++++++------- src/short-urls/reducers/shortUrlDeletion.ts | 19 +++++++++++++------ src/short-urls/reducers/shortUrlsList.ts | 7 +++++-- .../reducers/shortUrlDeletion.test.ts | 14 ++++++++++---- .../short-urls/reducers/shortUrlsList.test.ts | 2 +- 5 files changed, 36 insertions(+), 20 deletions(-) diff --git a/src/short-urls/helpers/DeleteShortUrlModal.tsx b/src/short-urls/helpers/DeleteShortUrlModal.tsx index 1bf7579f..4704d666 100644 --- a/src/short-urls/helpers/DeleteShortUrlModal.tsx +++ b/src/short-urls/helpers/DeleteShortUrlModal.tsx @@ -1,16 +1,16 @@ import { useEffect, useState } from 'react'; import { Modal, ModalBody, ModalFooter, ModalHeader } from 'reactstrap'; import { identity, pipe } from 'ramda'; -import { ShortUrlDeletion } from '../reducers/shortUrlDeletion'; +import { DeleteShortUrl, ShortUrlDeletion } from '../reducers/shortUrlDeletion'; import { ShortUrlModalProps } from '../data'; -import { handleEventPreventingDefault, OptionalString } from '../../utils/utils'; +import { handleEventPreventingDefault } from '../../utils/utils'; import { Result } from '../../utils/Result'; import { isInvalidDeletionError } from '../../api/utils'; import { ShlinkApiError } from '../../api/ShlinkApiError'; interface DeleteShortUrlModalConnectProps extends ShortUrlModalProps { shortUrlDeletion: ShortUrlDeletion; - deleteShortUrl: (shortCode: string, domain: OptionalString) => Promise; + deleteShortUrl: (shortUrl: DeleteShortUrl) => Promise; resetDeleteShortUrl: () => void; } @@ -21,12 +21,12 @@ export const DeleteShortUrlModal = ( useEffect(() => resetDeleteShortUrl, []); - const { error, errorData } = shortUrlDeletion; + const { loading, error, errorData } = shortUrlDeletion; const close = pipe(resetDeleteShortUrl, toggle); const handleDeleteUrl = handleEventPreventingDefault(() => { const { shortCode, domain } = shortUrl; - deleteShortUrl(shortCode, domain) + deleteShortUrl({ shortCode, domain }) .then(toggle) .catch(identity); }); @@ -61,9 +61,9 @@ export const DeleteShortUrlModal = ( diff --git a/src/short-urls/reducers/shortUrlDeletion.ts b/src/short-urls/reducers/shortUrlDeletion.ts index 52edeed3..2e9cecc5 100644 --- a/src/short-urls/reducers/shortUrlDeletion.ts +++ b/src/short-urls/reducers/shortUrlDeletion.ts @@ -1,4 +1,5 @@ -import { Action, Dispatch } from 'redux'; +import { PayloadAction } from '@reduxjs/toolkit'; +import { Dispatch } from 'redux'; import { buildActionCreator, buildReducer } from '../../utils/helpers/redux'; import { GetState } from '../../container/types'; import { ShlinkApiClientBuilder } from '../../api/services/ShlinkApiClientBuilder'; @@ -18,11 +19,13 @@ export interface ShortUrlDeletion { errorData?: ProblemDetailsError; } -export interface DeleteShortUrlAction extends Action { +export interface DeleteShortUrl { shortCode: string; domain?: string | null; } +export type DeleteShortUrlAction = PayloadAction; + const initialState: ShortUrlDeletion = { shortCode: '', loading: false, @@ -32,20 +35,24 @@ const initialState: ShortUrlDeletion = { export default buildReducer({ [DELETE_SHORT_URL_START]: (state) => ({ ...state, loading: true, error: false }), [DELETE_SHORT_URL_ERROR]: (state, { errorData }) => ({ ...state, errorData, loading: false, error: true }), - [SHORT_URL_DELETED]: (state, { shortCode }) => ({ ...state, shortCode, loading: false, error: false }), + [SHORT_URL_DELETED]: (state, { payload }) => ( + { ...state, shortCode: payload.shortCode, loading: false, error: false } + ), [RESET_DELETE_SHORT_URL]: () => initialState, }, initialState); export const deleteShortUrl = (buildShlinkApiClient: ShlinkApiClientBuilder) => ( - shortCode: string, - domain?: string | null, + { shortCode, domain }: DeleteShortUrl, ) => async (dispatch: Dispatch, getState: GetState) => { dispatch({ type: DELETE_SHORT_URL_START }); const { deleteShortUrl: shlinkDeleteShortUrl } = buildShlinkApiClient(getState); try { await shlinkDeleteShortUrl(shortCode, domain); - dispatch({ type: SHORT_URL_DELETED, shortCode, domain }); + dispatch({ + type: SHORT_URL_DELETED, + payload: { shortCode, domain }, + }); } catch (e: any) { dispatch({ type: DELETE_SHORT_URL_ERROR, errorData: parseApiError(e) }); diff --git a/src/short-urls/reducers/shortUrlsList.ts b/src/short-urls/reducers/shortUrlsList.ts index e2246524..317cc523 100644 --- a/src/short-urls/reducers/shortUrlsList.ts +++ b/src/short-urls/reducers/shortUrlsList.ts @@ -44,10 +44,12 @@ export default buildReducer({ [LIST_SHORT_URLS_START]: (state) => ({ ...state, loading: true, error: false }), [LIST_SHORT_URLS_ERROR]: () => ({ loading: false, error: true }), [LIST_SHORT_URLS]: (_, { shortUrls }) => ({ loading: false, error: false, shortUrls }), + // [`${SHORT_URL_DELETED}/fulfilled`]: pipe( // TODO Do not hardcode action type here [SHORT_URL_DELETED]: pipe( - (state: ShortUrlsList, { shortCode, domain }: DeleteShortUrlAction) => (!state.shortUrls ? state : assocPath( + (state: ShortUrlsList, { payload }: DeleteShortUrlAction) => (!state.shortUrls ? state : assocPath( ['shortUrls', 'data'], - reject((shortUrl) => shortUrlMatches(shortUrl, shortCode, domain), state.shortUrls.data), + reject((shortUrl) => + shortUrlMatches(shortUrl, payload.shortCode, payload.domain), state.shortUrls.data), state, )), (state) => (!state.shortUrls ? state : assocPath( @@ -89,6 +91,7 @@ export default buildReducer({ state, )), ), + // TODO Do not hardcode action type here [`${SHORT_URL_EDITED}/fulfilled`]: (state, { payload: editedShortUrl }) => (!state.shortUrls ? state : assocPath( ['shortUrls', 'data'], state.shortUrls.data.map((shortUrl) => { diff --git a/test/short-urls/reducers/shortUrlDeletion.test.ts b/test/short-urls/reducers/shortUrlDeletion.test.ts index 95500ee5..9346cc45 100644 --- a/test/short-urls/reducers/shortUrlDeletion.test.ts +++ b/test/short-urls/reducers/shortUrlDeletion.test.ts @@ -27,7 +27,10 @@ describe('shortUrlDeletionReducer', () => { })); it('returns shortCode on SHORT_URL_DELETED', () => - expect(reducer(undefined, { type: SHORT_URL_DELETED, shortCode: 'foo' } as any)).toEqual({ + expect(reducer(undefined, { + type: SHORT_URL_DELETED, + payload: { shortCode: 'foo' }, + } as any)).toEqual({ shortCode: 'foo', loading: false, error: false, @@ -67,11 +70,14 @@ describe('shortUrlDeletionReducer', () => { }); const shortCode = 'abc123'; - await deleteShortUrl(() => apiClientMock)(shortCode, domain)(dispatch, getState); + await deleteShortUrl(() => apiClientMock)({ shortCode, domain })(dispatch, getState); expect(dispatch).toHaveBeenCalledTimes(2); expect(dispatch).toHaveBeenNthCalledWith(1, { type: DELETE_SHORT_URL_START }); - expect(dispatch).toHaveBeenNthCalledWith(2, { type: SHORT_URL_DELETED, shortCode, domain }); + expect(dispatch).toHaveBeenNthCalledWith(2, { + type: SHORT_URL_DELETED, + payload: { shortCode, domain }, + }); expect(apiClientMock.deleteShortUrl).toHaveBeenCalledTimes(1); expect(apiClientMock.deleteShortUrl).toHaveBeenCalledWith(shortCode, domain); @@ -86,7 +92,7 @@ describe('shortUrlDeletionReducer', () => { const shortCode = 'abc123'; try { - await deleteShortUrl(() => apiClientMock)(shortCode)(dispatch, getState); + await deleteShortUrl(() => apiClientMock)({ shortCode })(dispatch, getState); } catch (e) { expect(e).toEqual(error); } diff --git a/test/short-urls/reducers/shortUrlsList.test.ts b/test/short-urls/reducers/shortUrlsList.test.ts index 38c4a24d..fd62a6a4 100644 --- a/test/short-urls/reducers/shortUrlsList.test.ts +++ b/test/short-urls/reducers/shortUrlsList.test.ts @@ -52,7 +52,7 @@ describe('shortUrlsListReducer', () => { error: false, }; - expect(reducer(state, { type: SHORT_URL_DELETED, shortCode } as any)).toEqual({ + expect(reducer(state, { type: SHORT_URL_DELETED, payload: { shortCode } } as any)).toEqual({ shortUrls: { data: [{ shortCode, domain: 'example.com' }, { shortCode: 'foo' }], pagination: { totalItems: 9 }, From 830071278e59dc5cdd1b0dc6d64fbf32e41e191b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 6 Nov 2022 13:06:55 +0100 Subject: [PATCH 09/15] Migrated shortUrlDeletion reducer to RTK --- src/reducers/index.ts | 3 +- src/short-urls/reducers/shortUrlDeletion.ts | 64 ++++++++-------- src/short-urls/reducers/shortUrlsList.ts | 3 +- src/short-urls/services/provideServices.ts | 9 ++- .../reducers/shortUrlDeletion.test.ts | 74 +++++++++---------- .../short-urls/reducers/shortUrlsList.test.ts | 2 +- 6 files changed, 73 insertions(+), 82 deletions(-) diff --git a/src/reducers/index.ts b/src/reducers/index.ts index 707fe5ca..fc93de93 100644 --- a/src/reducers/index.ts +++ b/src/reducers/index.ts @@ -3,7 +3,6 @@ import { combineReducers } from 'redux'; import { serversReducer } from '../servers/reducers/servers'; import selectedServerReducer from '../servers/reducers/selectedServer'; import shortUrlsListReducer from '../short-urls/reducers/shortUrlsList'; -import shortUrlDeletionReducer from '../short-urls/reducers/shortUrlDeletion'; import shortUrlVisitsReducer from '../visits/reducers/shortUrlVisits'; import tagVisitsReducer from '../visits/reducers/tagVisits'; import domainVisitsReducer from '../visits/reducers/domainVisits'; @@ -24,7 +23,7 @@ export default (container: IContainer) => combineReducers({ selectedServer: selectedServerReducer, shortUrlsList: shortUrlsListReducer, shortUrlCreationResult: container.shortUrlCreationReducer, - shortUrlDeletion: shortUrlDeletionReducer, + shortUrlDeletion: container.shortUrlDeletionReducer, shortUrlEdition: container.shortUrlEditionReducer, shortUrlVisits: shortUrlVisitsReducer, tagVisits: tagVisitsReducer, diff --git a/src/short-urls/reducers/shortUrlDeletion.ts b/src/short-urls/reducers/shortUrlDeletion.ts index 2e9cecc5..87fceed9 100644 --- a/src/short-urls/reducers/shortUrlDeletion.ts +++ b/src/short-urls/reducers/shortUrlDeletion.ts @@ -1,20 +1,15 @@ -import { PayloadAction } from '@reduxjs/toolkit'; -import { Dispatch } from 'redux'; -import { buildActionCreator, buildReducer } from '../../utils/helpers/redux'; -import { GetState } from '../../container/types'; +import { createSlice, PayloadAction } from '@reduxjs/toolkit'; +import { createAsyncThunk } from '../../utils/helpers/redux'; import { ShlinkApiClientBuilder } from '../../api/services/ShlinkApiClientBuilder'; import { parseApiError } from '../../api/utils'; -import { ApiErrorAction } from '../../api/types/actions'; import { ProblemDetailsError } from '../../api/types/errors'; -export const DELETE_SHORT_URL_START = 'shlink/deleteShortUrl/DELETE_SHORT_URL_START'; -export const DELETE_SHORT_URL_ERROR = 'shlink/deleteShortUrl/DELETE_SHORT_URL_ERROR'; export const SHORT_URL_DELETED = 'shlink/deleteShortUrl/SHORT_URL_DELETED'; -export const RESET_DELETE_SHORT_URL = 'shlink/deleteShortUrl/RESET_DELETE_SHORT_URL'; export interface ShortUrlDeletion { shortCode: string; loading: boolean; + deleted: boolean; error: boolean; errorData?: ProblemDetailsError; } @@ -29,35 +24,38 @@ export type DeleteShortUrlAction = PayloadAction; const initialState: ShortUrlDeletion = { shortCode: '', loading: false, + deleted: false, error: false, }; -export default buildReducer({ - [DELETE_SHORT_URL_START]: (state) => ({ ...state, loading: true, error: false }), - [DELETE_SHORT_URL_ERROR]: (state, { errorData }) => ({ ...state, errorData, loading: false, error: true }), - [SHORT_URL_DELETED]: (state, { payload }) => ( - { ...state, shortCode: payload.shortCode, loading: false, error: false } - ), - [RESET_DELETE_SHORT_URL]: () => initialState, -}, initialState); +export const shortUrlDeletionReducerCreator = (buildShlinkApiClient: ShlinkApiClientBuilder) => { + const deleteShortUrl = createAsyncThunk( + SHORT_URL_DELETED, + async ({ shortCode, domain }: DeleteShortUrl, { getState }): Promise => { + const { deleteShortUrl: shlinkDeleteShortUrl } = buildShlinkApiClient(getState); + await shlinkDeleteShortUrl(shortCode, domain); + return { shortCode, domain }; + }, + ); -export const deleteShortUrl = (buildShlinkApiClient: ShlinkApiClientBuilder) => ( - { shortCode, domain }: DeleteShortUrl, -) => async (dispatch: Dispatch, getState: GetState) => { - dispatch({ type: DELETE_SHORT_URL_START }); - const { deleteShortUrl: shlinkDeleteShortUrl } = buildShlinkApiClient(getState); + const { actions, reducer } = createSlice({ + name: 'shortUrlDeletion', + initialState, + reducers: { + resetDeleteShortUrl: () => initialState, + }, + extraReducers: (builder) => { + builder.addCase(deleteShortUrl.pending, (state) => ({ ...state, loading: true, error: false, deleted: false })); + builder.addCase(deleteShortUrl.rejected, (state, { error }) => ( + { ...state, errorData: parseApiError(error), loading: false, error: true, deleted: false } + )); + builder.addCase(deleteShortUrl.fulfilled, (state, { payload }) => ( + { ...state, shortCode: payload.shortCode, loading: false, error: false, deleted: true } + )); + }, + }); - try { - await shlinkDeleteShortUrl(shortCode, domain); - dispatch({ - type: SHORT_URL_DELETED, - payload: { shortCode, domain }, - }); - } catch (e: any) { - dispatch({ type: DELETE_SHORT_URL_ERROR, errorData: parseApiError(e) }); + const { resetDeleteShortUrl } = actions; - throw e; - } + return { reducer, deleteShortUrl, resetDeleteShortUrl }; }; - -export const resetDeleteShortUrl = buildActionCreator(RESET_DELETE_SHORT_URL); diff --git a/src/short-urls/reducers/shortUrlsList.ts b/src/short-urls/reducers/shortUrlsList.ts index 317cc523..54b2b4cc 100644 --- a/src/short-urls/reducers/shortUrlsList.ts +++ b/src/short-urls/reducers/shortUrlsList.ts @@ -44,8 +44,7 @@ export default buildReducer({ [LIST_SHORT_URLS_START]: (state) => ({ ...state, loading: true, error: false }), [LIST_SHORT_URLS_ERROR]: () => ({ loading: false, error: true }), [LIST_SHORT_URLS]: (_, { shortUrls }) => ({ loading: false, error: false, shortUrls }), - // [`${SHORT_URL_DELETED}/fulfilled`]: pipe( // TODO Do not hardcode action type here - [SHORT_URL_DELETED]: pipe( + [`${SHORT_URL_DELETED}/fulfilled`]: pipe( // TODO Do not hardcode action type here (state: ShortUrlsList, { payload }: DeleteShortUrlAction) => (!state.shortUrls ? state : assocPath( ['shortUrls', 'data'], reject((shortUrl) => diff --git a/src/short-urls/services/provideServices.ts b/src/short-urls/services/provideServices.ts index aab487c4..25845ea2 100644 --- a/src/short-urls/services/provideServices.ts +++ b/src/short-urls/services/provideServices.ts @@ -9,7 +9,7 @@ import { DeleteShortUrlModal } from '../helpers/DeleteShortUrlModal'; import { CreateShortUrlResult } from '../helpers/CreateShortUrlResult'; import { listShortUrls } from '../reducers/shortUrlsList'; import { shortUrlCreationReducerCreator } from '../reducers/shortUrlCreation'; -import { deleteShortUrl, resetDeleteShortUrl } from '../reducers/shortUrlDeletion'; +import { shortUrlDeletionReducerCreator } from '../reducers/shortUrlDeletion'; import { shortUrlEditionReducerCreator } from '../reducers/shortUrlEdition'; import { ConnectDecorator } from '../../container/types'; import { ShortUrlsTable } from '../ShortUrlsTable'; @@ -63,14 +63,17 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator) => { bottle.serviceFactory('shortUrlEditionReducerCreator', shortUrlEditionReducerCreator, 'buildShlinkApiClient'); bottle.serviceFactory('shortUrlEditionReducer', prop('reducer'), 'shortUrlEditionReducerCreator'); + bottle.serviceFactory('shortUrlDeletionReducerCreator', shortUrlDeletionReducerCreator, 'buildShlinkApiClient'); + bottle.serviceFactory('shortUrlDeletionReducer', prop('reducer'), 'shortUrlDeletionReducerCreator'); + // Actions bottle.serviceFactory('listShortUrls', listShortUrls, 'buildShlinkApiClient'); bottle.serviceFactory('createShortUrl', prop('createShortUrl'), 'shortUrlCreationReducerCreator'); bottle.serviceFactory('resetCreateShortUrl', prop('resetCreateShortUrl'), 'shortUrlCreationReducerCreator'); - bottle.serviceFactory('deleteShortUrl', deleteShortUrl, 'buildShlinkApiClient'); - bottle.serviceFactory('resetDeleteShortUrl', () => resetDeleteShortUrl); + bottle.serviceFactory('deleteShortUrl', prop('deleteShortUrl'), 'shortUrlDeletionReducerCreator'); + bottle.serviceFactory('resetDeleteShortUrl', prop('resetDeleteShortUrl'), 'shortUrlDeletionReducerCreator'); bottle.serviceFactory('getShortUrlDetail', getShortUrlDetail, 'buildShlinkApiClient'); diff --git a/test/short-urls/reducers/shortUrlDeletion.test.ts b/test/short-urls/reducers/shortUrlDeletion.test.ts index 9346cc45..6ec4b661 100644 --- a/test/short-urls/reducers/shortUrlDeletion.test.ts +++ b/test/short-urls/reducers/shortUrlDeletion.test.ts @@ -1,48 +1,52 @@ import { Mock } from 'ts-mockery'; -import reducer, { - DELETE_SHORT_URL_ERROR, - DELETE_SHORT_URL_START, - RESET_DELETE_SHORT_URL, - SHORT_URL_DELETED, - resetDeleteShortUrl, - deleteShortUrl, -} from '../../../src/short-urls/reducers/shortUrlDeletion'; +import { shortUrlDeletionReducerCreator } from '../../../src/short-urls/reducers/shortUrlDeletion'; import { ShlinkApiClient } from '../../../src/api/services/ShlinkApiClient'; import { ProblemDetailsError } from '../../../src/api/types/errors'; describe('shortUrlDeletionReducer', () => { + const deleteShortUrlCall = jest.fn(); + const buildShlinkApiClient = () => Mock.of({ deleteShortUrl: deleteShortUrlCall }); + const { reducer, resetDeleteShortUrl, deleteShortUrl } = shortUrlDeletionReducerCreator(buildShlinkApiClient); + + beforeEach(jest.clearAllMocks); + describe('reducer', () => { it('returns loading on DELETE_SHORT_URL_START', () => - expect(reducer(undefined, { type: DELETE_SHORT_URL_START } as any)).toEqual({ + expect(reducer(undefined, { type: deleteShortUrl.pending.toString() } as any)).toEqual({ shortCode: '', loading: true, error: false, + deleted: false, })); it('returns default on RESET_DELETE_SHORT_URL', () => - expect(reducer(undefined, { type: RESET_DELETE_SHORT_URL } as any)).toEqual({ + expect(reducer(undefined, { type: resetDeleteShortUrl.toString() } as any)).toEqual({ shortCode: '', loading: false, error: false, + deleted: false, })); it('returns shortCode on SHORT_URL_DELETED', () => expect(reducer(undefined, { - type: SHORT_URL_DELETED, + type: deleteShortUrl.fulfilled.toString(), payload: { shortCode: 'foo' }, } as any)).toEqual({ shortCode: 'foo', loading: false, error: false, + deleted: true, })); it('returns errorData on DELETE_SHORT_URL_ERROR', () => { const errorData = Mock.of({ type: 'bar' }); + const error = { response: { data: errorData } }; - expect(reducer(undefined, { type: DELETE_SHORT_URL_ERROR, errorData } as any)).toEqual({ + expect(reducer(undefined, { type: deleteShortUrl.rejected.toString(), error } as any)).toEqual({ shortCode: '', loading: false, error: true, + deleted: false, errorData, }); }); @@ -50,59 +54,47 @@ describe('shortUrlDeletionReducer', () => { describe('resetDeleteShortUrl', () => { it('returns expected action', () => - expect(resetDeleteShortUrl()).toEqual({ type: RESET_DELETE_SHORT_URL })); + expect(resetDeleteShortUrl()).toEqual({ type: resetDeleteShortUrl.toString() })); }); describe('deleteShortUrl', () => { const dispatch = jest.fn(); const getState = jest.fn().mockReturnValue({ selectedServer: {} }); - afterEach(() => { - dispatch.mockReset(); - getState.mockClear(); - }); - it.each( [[undefined], [null], ['example.com']], )('dispatches proper actions if API client request succeeds', async (domain) => { - const apiClientMock = Mock.of({ - deleteShortUrl: jest.fn(() => ''), - }); const shortCode = 'abc123'; - await deleteShortUrl(() => apiClientMock)({ shortCode, domain })(dispatch, getState); + await deleteShortUrl({ shortCode, domain })(dispatch, getState, {}); expect(dispatch).toHaveBeenCalledTimes(2); - expect(dispatch).toHaveBeenNthCalledWith(1, { type: DELETE_SHORT_URL_START }); - expect(dispatch).toHaveBeenNthCalledWith(2, { - type: SHORT_URL_DELETED, + expect(dispatch).toHaveBeenNthCalledWith(1, expect.objectContaining({ type: deleteShortUrl.pending.toString() })); + expect(dispatch).toHaveBeenNthCalledWith(2, expect.objectContaining({ + type: deleteShortUrl.fulfilled.toString(), payload: { shortCode, domain }, - }); + })); - expect(apiClientMock.deleteShortUrl).toHaveBeenCalledTimes(1); - expect(apiClientMock.deleteShortUrl).toHaveBeenCalledWith(shortCode, domain); + expect(deleteShortUrlCall).toHaveBeenCalledTimes(1); + expect(deleteShortUrlCall).toHaveBeenCalledWith(shortCode, domain); }); it('dispatches proper actions if API client request fails', async () => { const data = { foo: 'bar' }; - const error = { response: { data } }; - const apiClientMock = Mock.of({ - deleteShortUrl: jest.fn(async () => Promise.reject(error)), - }); const shortCode = 'abc123'; - try { - await deleteShortUrl(() => apiClientMock)({ shortCode })(dispatch, getState); - } catch (e) { - expect(e).toEqual(error); - } + deleteShortUrlCall.mockRejectedValue({ response: { data } }); + + await deleteShortUrl({ shortCode })(dispatch, getState, {}); expect(dispatch).toHaveBeenCalledTimes(2); - expect(dispatch).toHaveBeenNthCalledWith(1, { type: DELETE_SHORT_URL_START }); - expect(dispatch).toHaveBeenNthCalledWith(2, { type: DELETE_SHORT_URL_ERROR, errorData: data }); + expect(dispatch).toHaveBeenNthCalledWith(1, expect.objectContaining({ type: deleteShortUrl.pending.toString() })); + expect(dispatch).toHaveBeenNthCalledWith(2, expect.objectContaining({ + type: deleteShortUrl.rejected.toString(), + })); - expect(apiClientMock.deleteShortUrl).toHaveBeenCalledTimes(1); - expect(apiClientMock.deleteShortUrl).toHaveBeenCalledWith(shortCode, undefined); + expect(deleteShortUrlCall).toHaveBeenCalledTimes(1); + expect(deleteShortUrlCall).toHaveBeenCalledWith(shortCode, undefined); }); }); }); diff --git a/test/short-urls/reducers/shortUrlsList.test.ts b/test/short-urls/reducers/shortUrlsList.test.ts index fd62a6a4..05b500ea 100644 --- a/test/short-urls/reducers/shortUrlsList.test.ts +++ b/test/short-urls/reducers/shortUrlsList.test.ts @@ -52,7 +52,7 @@ describe('shortUrlsListReducer', () => { error: false, }; - expect(reducer(state, { type: SHORT_URL_DELETED, payload: { shortCode } } as any)).toEqual({ + expect(reducer(state, { type: `${SHORT_URL_DELETED}/fulfilled`, payload: { shortCode } } as any)).toEqual({ shortUrls: { data: [{ shortCode, domain: 'example.com' }, { shortCode: 'foo' }], pagination: { totalItems: 9 }, From 0f552ae6f42a39813e1d973273fb263fa59fb070 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 6 Nov 2022 13:21:46 +0100 Subject: [PATCH 10/15] Removed unnecessary castings to any --- test/short-urls/reducers/shortUrlDeletion.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/short-urls/reducers/shortUrlDeletion.test.ts b/test/short-urls/reducers/shortUrlDeletion.test.ts index 6ec4b661..fde38ff8 100644 --- a/test/short-urls/reducers/shortUrlDeletion.test.ts +++ b/test/short-urls/reducers/shortUrlDeletion.test.ts @@ -12,7 +12,7 @@ describe('shortUrlDeletionReducer', () => { describe('reducer', () => { it('returns loading on DELETE_SHORT_URL_START', () => - expect(reducer(undefined, { type: deleteShortUrl.pending.toString() } as any)).toEqual({ + expect(reducer(undefined, { type: deleteShortUrl.pending.toString() })).toEqual({ shortCode: '', loading: true, error: false, @@ -20,7 +20,7 @@ describe('shortUrlDeletionReducer', () => { })); it('returns default on RESET_DELETE_SHORT_URL', () => - expect(reducer(undefined, { type: resetDeleteShortUrl.toString() } as any)).toEqual({ + expect(reducer(undefined, { type: resetDeleteShortUrl.toString() })).toEqual({ shortCode: '', loading: false, error: false, @@ -31,7 +31,7 @@ describe('shortUrlDeletionReducer', () => { expect(reducer(undefined, { type: deleteShortUrl.fulfilled.toString(), payload: { shortCode: 'foo' }, - } as any)).toEqual({ + })).toEqual({ shortCode: 'foo', loading: false, error: false, @@ -42,7 +42,7 @@ describe('shortUrlDeletionReducer', () => { const errorData = Mock.of({ type: 'bar' }); const error = { response: { data: errorData } }; - expect(reducer(undefined, { type: deleteShortUrl.rejected.toString(), error } as any)).toEqual({ + expect(reducer(undefined, { type: deleteShortUrl.rejected.toString(), error })).toEqual({ shortCode: '', loading: false, error: true, From cf4143e4e225c5a67c93fc9a209079bb83e30e7a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 6 Nov 2022 18:48:47 +0100 Subject: [PATCH 11/15] Removed unnecesarry promise references in delete short URL modal --- src/short-urls/helpers/DeleteShortUrlModal.tsx | 12 +++--------- src/short-urls/helpers/ShortUrlsRowMenu.tsx | 12 ++++++------ test/short-urls/helpers/DeleteShortUrlModal.test.tsx | 2 +- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/short-urls/helpers/DeleteShortUrlModal.tsx b/src/short-urls/helpers/DeleteShortUrlModal.tsx index 4704d666..06caab5e 100644 --- a/src/short-urls/helpers/DeleteShortUrlModal.tsx +++ b/src/short-urls/helpers/DeleteShortUrlModal.tsx @@ -1,6 +1,6 @@ import { useEffect, useState } from 'react'; import { Modal, ModalBody, ModalFooter, ModalHeader } from 'reactstrap'; -import { identity, pipe } from 'ramda'; +import { pipe } from 'ramda'; import { DeleteShortUrl, ShortUrlDeletion } from '../reducers/shortUrlDeletion'; import { ShortUrlModalProps } from '../data'; import { handleEventPreventingDefault } from '../../utils/utils'; @@ -10,7 +10,7 @@ import { ShlinkApiError } from '../../api/ShlinkApiError'; interface DeleteShortUrlModalConnectProps extends ShortUrlModalProps { shortUrlDeletion: ShortUrlDeletion; - deleteShortUrl: (shortUrl: DeleteShortUrl) => Promise; + deleteShortUrl: (shortUrl: DeleteShortUrl) => void; resetDeleteShortUrl: () => void; } @@ -23,13 +23,7 @@ export const DeleteShortUrlModal = ( const { loading, error, errorData } = shortUrlDeletion; const close = pipe(resetDeleteShortUrl, toggle); - const handleDeleteUrl = handleEventPreventingDefault(() => { - const { shortCode, domain } = shortUrl; - - deleteShortUrl({ shortCode, domain }) - .then(toggle) - .catch(identity); - }); + const handleDeleteUrl = handleEventPreventingDefault(() => deleteShortUrl(shortUrl)); return ( diff --git a/src/short-urls/helpers/ShortUrlsRowMenu.tsx b/src/short-urls/helpers/ShortUrlsRowMenu.tsx index 877233f3..a4563a52 100644 --- a/src/short-urls/helpers/ShortUrlsRowMenu.tsx +++ b/src/short-urls/helpers/ShortUrlsRowMenu.tsx @@ -24,8 +24,8 @@ export const ShortUrlsRowMenu = ( QrCodeModal: ShortUrlModal, ) => ({ shortUrl, selectedServer }: ShortUrlsRowMenuProps) => { const [isOpen, toggle] = useToggle(); - const [isQrModalOpen, toggleQrCode] = useToggle(); - const [isDeleteModalOpen, toggleDelete] = useToggle(); + const [isQrModalOpen,, openQrCodeModal, closeQrCodeModal] = useToggle(); + const [isDeleteModalOpen,, openDeleteModal, closeDeleteModal] = useToggle(); return ( @@ -37,17 +37,17 @@ export const ShortUrlsRowMenu = ( Edit short URL - + QR code - + - + Delete short URL - + ); }; diff --git a/test/short-urls/helpers/DeleteShortUrlModal.test.tsx b/test/short-urls/helpers/DeleteShortUrlModal.test.tsx index a386b198..282c4738 100644 --- a/test/short-urls/helpers/DeleteShortUrlModal.test.tsx +++ b/test/short-urls/helpers/DeleteShortUrlModal.test.tsx @@ -12,7 +12,7 @@ describe('', () => { shortCode: 'abc123', longUrl: 'https://long-domain.com/foo/bar', }); - const deleteShortUrl = jest.fn(async () => Promise.resolve()); + const deleteShortUrl = jest.fn(); const setUp = (shortUrlDeletion: Partial) => renderWithEvents( Date: Sun, 6 Nov 2022 19:06:39 +0100 Subject: [PATCH 12/15] Updated getShortUrlDetail action to use payload action --- src/short-urls/reducers/shortUrlDetail.ts | 13 ++++++------- test/short-urls/reducers/shortUrlDetail.test.ts | 6 +++--- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/short-urls/reducers/shortUrlDetail.ts b/src/short-urls/reducers/shortUrlDetail.ts index 81ab192c..19c2d888 100644 --- a/src/short-urls/reducers/shortUrlDetail.ts +++ b/src/short-urls/reducers/shortUrlDetail.ts @@ -1,4 +1,5 @@ -import { Action, Dispatch } from 'redux'; +import { PayloadAction } from '@reduxjs/toolkit'; +import { Dispatch } from 'redux'; import { ShortUrl } from '../data'; import { buildReducer } from '../../utils/helpers/redux'; import { ShlinkApiClientBuilder } from '../../api/services/ShlinkApiClientBuilder'; @@ -20,9 +21,7 @@ export interface ShortUrlDetail { errorData?: ProblemDetailsError; } -export interface ShortUrlDetailAction extends Action { - shortUrl: ShortUrl; -} +export type ShortUrlDetailAction = PayloadAction; const initialState: ShortUrlDetail = { loading: false, @@ -32,7 +31,7 @@ const initialState: ShortUrlDetail = { export default buildReducer({ [GET_SHORT_URL_DETAIL_START]: () => ({ loading: true, error: false }), [GET_SHORT_URL_DETAIL_ERROR]: (_, { errorData }) => ({ loading: false, error: true, errorData }), - [GET_SHORT_URL_DETAIL]: (_, { shortUrl }) => ({ shortUrl, ...initialState }), + [GET_SHORT_URL_DETAIL]: (_, { payload: shortUrl }) => ({ shortUrl, ...initialState }), }, initialState); export const getShortUrlDetail = (buildShlinkApiClient: ShlinkApiClientBuilder) => ( @@ -43,11 +42,11 @@ export const getShortUrlDetail = (buildShlinkApiClient: ShlinkApiClientBuilder) try { const { shortUrlsList } = getState(); - const shortUrl = shortUrlsList?.shortUrls?.data.find( + const payload = shortUrlsList?.shortUrls?.data.find( (url) => shortUrlMatches(url, shortCode, domain), ) ?? await buildShlinkApiClient(getState).getShortUrl(shortCode, domain); - dispatch({ shortUrl, type: GET_SHORT_URL_DETAIL }); + dispatch({ payload, type: GET_SHORT_URL_DETAIL }); } catch (e: any) { dispatch({ type: GET_SHORT_URL_DETAIL_ERROR, errorData: parseApiError(e) }); } diff --git a/test/short-urls/reducers/shortUrlDetail.test.ts b/test/short-urls/reducers/shortUrlDetail.test.ts index 2a6b9df7..5e2237c0 100644 --- a/test/short-urls/reducers/shortUrlDetail.test.ts +++ b/test/short-urls/reducers/shortUrlDetail.test.ts @@ -34,7 +34,7 @@ describe('shortUrlDetailReducer', () => { it('return short URL on GET_SHORT_URL_DETAIL', () => { const actionShortUrl = Mock.of({ longUrl: 'foo', shortCode: 'bar' }); - const state = reducer({ loading: true, error: false }, { type: GET_SHORT_URL_DETAIL, shortUrl: actionShortUrl }); + const state = reducer({ loading: true, error: false }, { type: GET_SHORT_URL_DETAIL, payload: actionShortUrl }); const { loading, error, shortUrl } = state; expect(loading).toEqual(false); @@ -84,7 +84,7 @@ describe('shortUrlDetailReducer', () => { expect(dispatchMock).toHaveBeenCalledTimes(2); expect(dispatchMock).toHaveBeenNthCalledWith(1, { type: GET_SHORT_URL_DETAIL_START }); - expect(dispatchMock).toHaveBeenNthCalledWith(2, { type: GET_SHORT_URL_DETAIL, shortUrl: resolvedShortUrl }); + expect(dispatchMock).toHaveBeenNthCalledWith(2, { type: GET_SHORT_URL_DETAIL, payload: resolvedShortUrl }); expect(ShlinkApiClient.getShortUrl).toHaveBeenCalledTimes(1); }); @@ -103,7 +103,7 @@ describe('shortUrlDetailReducer', () => { expect(dispatchMock).toHaveBeenCalledTimes(2); expect(dispatchMock).toHaveBeenNthCalledWith(1, { type: GET_SHORT_URL_DETAIL_START }); - expect(dispatchMock).toHaveBeenNthCalledWith(2, { type: GET_SHORT_URL_DETAIL, shortUrl: foundShortUrl }); + expect(dispatchMock).toHaveBeenNthCalledWith(2, { type: GET_SHORT_URL_DETAIL, payload: foundShortUrl }); expect(ShlinkApiClient.getShortUrl).not.toHaveBeenCalled(); }); }); From ea199dbf8f33e24060a5c7a713f4638efb4bf2dd Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 6 Nov 2022 19:12:41 +0100 Subject: [PATCH 13/15] Removed redundant types --- src/short-urls/data/index.ts | 10 +++++----- src/short-urls/helpers/DeleteShortUrlModal.tsx | 6 +++--- src/short-urls/reducers/shortUrlDeletion.ts | 10 +++------- src/short-urls/reducers/shortUrlEdition.ts | 7 ++----- 4 files changed, 13 insertions(+), 20 deletions(-) diff --git a/src/short-urls/data/index.ts b/src/short-urls/data/index.ts index 3c17a812..88f5b22b 100644 --- a/src/short-urls/data/index.ts +++ b/src/short-urls/data/index.ts @@ -21,6 +21,11 @@ export interface ShortUrlData extends EditShortUrlData { findIfExists?: boolean; } +export interface ShortUrlIdentifier { + shortCode: string; + domain?: OptionalString; +} + export interface ShortUrl { shortCode: string; shortUrl: string; @@ -47,11 +52,6 @@ export interface ShortUrlModalProps { toggle: () => void; } -export interface ShortUrlIdentifier { - shortCode: string; - domain: OptionalString; -} - export const SHORT_URLS_ORDERABLE_FIELDS = { dateCreated: 'Created at', shortCode: 'Short URL', diff --git a/src/short-urls/helpers/DeleteShortUrlModal.tsx b/src/short-urls/helpers/DeleteShortUrlModal.tsx index 06caab5e..d240279f 100644 --- a/src/short-urls/helpers/DeleteShortUrlModal.tsx +++ b/src/short-urls/helpers/DeleteShortUrlModal.tsx @@ -1,8 +1,8 @@ import { useEffect, useState } from 'react'; import { Modal, ModalBody, ModalFooter, ModalHeader } from 'reactstrap'; import { pipe } from 'ramda'; -import { DeleteShortUrl, ShortUrlDeletion } from '../reducers/shortUrlDeletion'; -import { ShortUrlModalProps } from '../data'; +import { ShortUrlDeletion } from '../reducers/shortUrlDeletion'; +import { ShortUrlIdentifier, ShortUrlModalProps } from '../data'; import { handleEventPreventingDefault } from '../../utils/utils'; import { Result } from '../../utils/Result'; import { isInvalidDeletionError } from '../../api/utils'; @@ -10,7 +10,7 @@ import { ShlinkApiError } from '../../api/ShlinkApiError'; interface DeleteShortUrlModalConnectProps extends ShortUrlModalProps { shortUrlDeletion: ShortUrlDeletion; - deleteShortUrl: (shortUrl: DeleteShortUrl) => void; + deleteShortUrl: (shortUrl: ShortUrlIdentifier) => void; resetDeleteShortUrl: () => void; } diff --git a/src/short-urls/reducers/shortUrlDeletion.ts b/src/short-urls/reducers/shortUrlDeletion.ts index 87fceed9..9af657b8 100644 --- a/src/short-urls/reducers/shortUrlDeletion.ts +++ b/src/short-urls/reducers/shortUrlDeletion.ts @@ -3,6 +3,7 @@ import { createAsyncThunk } from '../../utils/helpers/redux'; import { ShlinkApiClientBuilder } from '../../api/services/ShlinkApiClientBuilder'; import { parseApiError } from '../../api/utils'; import { ProblemDetailsError } from '../../api/types/errors'; +import { ShortUrlIdentifier } from '../data'; export const SHORT_URL_DELETED = 'shlink/deleteShortUrl/SHORT_URL_DELETED'; @@ -14,12 +15,7 @@ export interface ShortUrlDeletion { errorData?: ProblemDetailsError; } -export interface DeleteShortUrl { - shortCode: string; - domain?: string | null; -} - -export type DeleteShortUrlAction = PayloadAction; +export type DeleteShortUrlAction = PayloadAction; const initialState: ShortUrlDeletion = { shortCode: '', @@ -31,7 +27,7 @@ const initialState: ShortUrlDeletion = { export const shortUrlDeletionReducerCreator = (buildShlinkApiClient: ShlinkApiClientBuilder) => { const deleteShortUrl = createAsyncThunk( SHORT_URL_DELETED, - async ({ shortCode, domain }: DeleteShortUrl, { getState }): Promise => { + async ({ shortCode, domain }: ShortUrlIdentifier, { getState }): Promise => { const { deleteShortUrl: shlinkDeleteShortUrl } = buildShlinkApiClient(getState); await shlinkDeleteShortUrl(shortCode, domain); return { shortCode, domain }; diff --git a/src/short-urls/reducers/shortUrlEdition.ts b/src/short-urls/reducers/shortUrlEdition.ts index 462515ca..267d9b23 100644 --- a/src/short-urls/reducers/shortUrlEdition.ts +++ b/src/short-urls/reducers/shortUrlEdition.ts @@ -1,7 +1,6 @@ import { createSlice, PayloadAction } from '@reduxjs/toolkit'; import { createAsyncThunk } from '../../utils/helpers/redux'; -import { OptionalString } from '../../utils/utils'; -import { EditShortUrlData, ShortUrl } from '../data'; +import { EditShortUrlData, ShortUrl, ShortUrlIdentifier } from '../data'; import { ShlinkApiClientBuilder } from '../../api/services/ShlinkApiClientBuilder'; import { parseApiError } from '../../api/utils'; import { ProblemDetailsError } from '../../api/types/errors'; @@ -16,9 +15,7 @@ export interface ShortUrlEdition { errorData?: ProblemDetailsError; } -export interface EditShortUrl { - shortCode: string; - domain?: OptionalString; +export interface EditShortUrl extends ShortUrlIdentifier { data: EditShortUrlData; } From f93bb88d35b6942b2465bc0a254c6d56679066f8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 6 Nov 2022 19:16:51 +0100 Subject: [PATCH 14/15] Refactored getShortUrlDetail action to have a single DTO param --- src/short-urls/EditShortUrl.tsx | 6 +++--- src/short-urls/reducers/shortUrlDetail.ts | 6 ++---- src/visits/ShortUrlVisits.tsx | 5 +++-- test/short-urls/reducers/shortUrlDetail.test.ts | 6 +++--- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/short-urls/EditShortUrl.tsx b/src/short-urls/EditShortUrl.tsx index 58687d7e..db8c4d43 100644 --- a/src/short-urls/EditShortUrl.tsx +++ b/src/short-urls/EditShortUrl.tsx @@ -6,7 +6,7 @@ import { ExternalLink } from 'react-external-link'; import { useLocation, useParams } from 'react-router-dom'; import { SelectedServer } from '../servers/data'; import { Settings } from '../settings/reducers/settings'; -import { OptionalString } from '../utils/utils'; +import { ShortUrlIdentifier } from './data'; import { parseQuery } from '../utils/helpers/query'; import { Message } from '../utils/Message'; import { Result } from '../utils/Result'; @@ -22,7 +22,7 @@ interface EditShortUrlConnectProps { selectedServer: SelectedServer; shortUrlDetail: ShortUrlDetail; shortUrlEdition: ShortUrlEdition; - getShortUrlDetail: (shortCode: string, domain: OptionalString) => void; + getShortUrlDetail: (shortUrl: ShortUrlIdentifier) => void; editShortUrl: (editShortUrl: EditShortUrlInfo) => void; } @@ -46,7 +46,7 @@ export const EditShortUrl = (ShortUrlForm: FC) => ({ ); useEffect(() => { - params.shortCode && getShortUrlDetail(urlDecodeShortCode(params.shortCode), domain); + params.shortCode && getShortUrlDetail({ shortCode: urlDecodeShortCode(params.shortCode), domain }); }, []); if (loading) { diff --git a/src/short-urls/reducers/shortUrlDetail.ts b/src/short-urls/reducers/shortUrlDetail.ts index 19c2d888..a703d867 100644 --- a/src/short-urls/reducers/shortUrlDetail.ts +++ b/src/short-urls/reducers/shortUrlDetail.ts @@ -1,9 +1,8 @@ import { PayloadAction } from '@reduxjs/toolkit'; import { Dispatch } from 'redux'; -import { ShortUrl } from '../data'; +import { ShortUrl, ShortUrlIdentifier } from '../data'; import { buildReducer } from '../../utils/helpers/redux'; import { ShlinkApiClientBuilder } from '../../api/services/ShlinkApiClientBuilder'; -import { OptionalString } from '../../utils/utils'; import { GetState } from '../../container/types'; import { shortUrlMatches } from '../helpers'; import { parseApiError } from '../../api/utils'; @@ -35,8 +34,7 @@ export default buildReducer ( - shortCode: string, - domain: OptionalString, + { shortCode, domain }: ShortUrlIdentifier, ) => async (dispatch: Dispatch, getState: GetState) => { dispatch({ type: GET_SHORT_URL_DETAIL_START }); diff --git a/src/visits/ShortUrlVisits.tsx b/src/visits/ShortUrlVisits.tsx index 99bed7d3..5ef1b005 100644 --- a/src/visits/ShortUrlVisits.tsx +++ b/src/visits/ShortUrlVisits.tsx @@ -14,11 +14,12 @@ import { NormalizedVisit, VisitsParams } from './types'; import { CommonVisitsProps } from './types/CommonVisitsProps'; import { toApiParams } from './types/helpers'; import { urlDecodeShortCode } from '../short-urls/helpers'; +import { ShortUrlIdentifier } from '../short-urls/data'; export interface ShortUrlVisitsProps extends CommonVisitsProps { getShortUrlVisits: (shortCode: string, query?: ShlinkVisitsParams, doIntervalFallback?: boolean) => void; shortUrlVisits: ShortUrlVisitsState; - getShortUrlDetail: Function; + getShortUrlDetail: (shortUrl: ShortUrlIdentifier) => void; shortUrlDetail: ShortUrlDetail; cancelGetShortUrlVisits: () => void; } @@ -44,7 +45,7 @@ export const ShortUrlVisits = ({ exportVisits }: ReportExporter) => boundToMercu ); useEffect(() => { - getShortUrlDetail(urlDecodeShortCode(shortCode), domain); + getShortUrlDetail({ shortCode: urlDecodeShortCode(shortCode), domain }); }, []); return ( diff --git a/test/short-urls/reducers/shortUrlDetail.test.ts b/test/short-urls/reducers/shortUrlDetail.test.ts index 5e2237c0..e520822e 100644 --- a/test/short-urls/reducers/shortUrlDetail.test.ts +++ b/test/short-urls/reducers/shortUrlDetail.test.ts @@ -53,7 +53,7 @@ describe('shortUrlDetailReducer', () => { it('dispatches start and error when promise is rejected', async () => { const ShlinkApiClient = buildApiClientMock(Promise.reject({})); - await getShortUrlDetail(() => ShlinkApiClient)('abc123', '')(dispatchMock, buildGetState()); + await getShortUrlDetail(() => ShlinkApiClient)({ shortCode: 'abc123', domain: '' })(dispatchMock, buildGetState()); expect(dispatchMock).toHaveBeenCalledTimes(2); expect(dispatchMock).toHaveBeenNthCalledWith(1, { type: GET_SHORT_URL_DETAIL_START }); @@ -80,7 +80,7 @@ describe('shortUrlDetailReducer', () => { const resolvedShortUrl = Mock.of({ longUrl: 'foo', shortCode: 'abc123' }); const ShlinkApiClient = buildApiClientMock(Promise.resolve(resolvedShortUrl)); - await getShortUrlDetail(() => ShlinkApiClient)('abc123', '')(dispatchMock, buildGetState(shortUrlsList)); + await getShortUrlDetail(() => ShlinkApiClient)({ shortCode: 'abc123', domain: '' })(dispatchMock, buildGetState(shortUrlsList)); expect(dispatchMock).toHaveBeenCalledTimes(2); expect(dispatchMock).toHaveBeenNthCalledWith(1, { type: GET_SHORT_URL_DETAIL_START }); @@ -92,7 +92,7 @@ describe('shortUrlDetailReducer', () => { const foundShortUrl = Mock.of({ longUrl: 'foo', shortCode: 'abc123' }); const ShlinkApiClient = buildApiClientMock(Promise.resolve(Mock.all())); - await getShortUrlDetail(() => ShlinkApiClient)(foundShortUrl.shortCode, foundShortUrl.domain)( + await getShortUrlDetail(() => ShlinkApiClient)(foundShortUrl)( dispatchMock, buildGetState(Mock.of({ shortUrls: { From f803941fe4da1c5a4cf47c7ac39bc67da7ea10f5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 6 Nov 2022 19:32:02 +0100 Subject: [PATCH 15/15] Migrated short URL detail reducer to RTK --- src/reducers/index.ts | 3 +- src/short-urls/reducers/shortUrlDetail.ts | 51 +++++++------- src/short-urls/services/provideServices.ts | 9 ++- .../reducers/shortUrlDetail.test.ts | 69 +++++++++++-------- 4 files changed, 73 insertions(+), 59 deletions(-) diff --git a/src/reducers/index.ts b/src/reducers/index.ts index fc93de93..9e473c05 100644 --- a/src/reducers/index.ts +++ b/src/reducers/index.ts @@ -8,7 +8,6 @@ import tagVisitsReducer from '../visits/reducers/tagVisits'; import domainVisitsReducer from '../visits/reducers/domainVisits'; import orphanVisitsReducer from '../visits/reducers/orphanVisits'; import nonOrphanVisitsReducer from '../visits/reducers/nonOrphanVisits'; -import shortUrlDetailReducer from '../short-urls/reducers/shortUrlDetail'; import tagsListReducer from '../tags/reducers/tagsList'; import tagDeleteReducer from '../tags/reducers/tagDelete'; import tagEditReducer from '../tags/reducers/tagEdit'; @@ -25,12 +24,12 @@ export default (container: IContainer) => combineReducers({ shortUrlCreationResult: container.shortUrlCreationReducer, shortUrlDeletion: container.shortUrlDeletionReducer, shortUrlEdition: container.shortUrlEditionReducer, + shortUrlDetail: container.shortUrlDetailReducer, shortUrlVisits: shortUrlVisitsReducer, tagVisits: tagVisitsReducer, domainVisits: domainVisitsReducer, orphanVisits: orphanVisitsReducer, nonOrphanVisits: nonOrphanVisitsReducer, - shortUrlDetail: shortUrlDetailReducer, tagsList: tagsListReducer, tagDelete: tagDeleteReducer, tagEdit: tagEditReducer, diff --git a/src/short-urls/reducers/shortUrlDetail.ts b/src/short-urls/reducers/shortUrlDetail.ts index a703d867..b23591c6 100644 --- a/src/short-urls/reducers/shortUrlDetail.ts +++ b/src/short-urls/reducers/shortUrlDetail.ts @@ -1,17 +1,12 @@ -import { PayloadAction } from '@reduxjs/toolkit'; -import { Dispatch } from 'redux'; +import { createSlice, PayloadAction } from '@reduxjs/toolkit'; import { ShortUrl, ShortUrlIdentifier } from '../data'; -import { buildReducer } from '../../utils/helpers/redux'; +import { createAsyncThunk } from '../../utils/helpers/redux'; import { ShlinkApiClientBuilder } from '../../api/services/ShlinkApiClientBuilder'; -import { GetState } from '../../container/types'; import { shortUrlMatches } from '../helpers'; import { parseApiError } from '../../api/utils'; -import { ApiErrorAction } from '../../api/types/actions'; import { ProblemDetailsError } from '../../api/types/errors'; -export const GET_SHORT_URL_DETAIL_START = 'shlink/shortUrlDetail/GET_SHORT_URL_DETAIL_START'; -export const GET_SHORT_URL_DETAIL_ERROR = 'shlink/shortUrlDetail/GET_SHORT_URL_DETAIL_ERROR'; -export const GET_SHORT_URL_DETAIL = 'shlink/shortUrlDetail/GET_SHORT_URL_DETAIL'; +const GET_SHORT_URL_DETAIL = 'shlink/shortUrlDetail/GET_SHORT_URL_DETAIL'; export interface ShortUrlDetail { shortUrl?: ShortUrl; @@ -27,25 +22,29 @@ const initialState: ShortUrlDetail = { error: false, }; -export default buildReducer({ - [GET_SHORT_URL_DETAIL_START]: () => ({ loading: true, error: false }), - [GET_SHORT_URL_DETAIL_ERROR]: (_, { errorData }) => ({ loading: false, error: true, errorData }), - [GET_SHORT_URL_DETAIL]: (_, { payload: shortUrl }) => ({ shortUrl, ...initialState }), -}, initialState); +export const shortUrlDetailReducerCreator = (buildShlinkApiClient: ShlinkApiClientBuilder) => { + const getShortUrlDetail = createAsyncThunk( + GET_SHORT_URL_DETAIL, + async ({ shortCode, domain }: ShortUrlIdentifier, { getState }): Promise => { + const { shortUrlsList } = getState(); + const alreadyLoaded = shortUrlsList?.shortUrls?.data.find((url) => shortUrlMatches(url, shortCode, domain)); -export const getShortUrlDetail = (buildShlinkApiClient: ShlinkApiClientBuilder) => ( - { shortCode, domain }: ShortUrlIdentifier, -) => async (dispatch: Dispatch, getState: GetState) => { - dispatch({ type: GET_SHORT_URL_DETAIL_START }); + return alreadyLoaded ?? await buildShlinkApiClient(getState).getShortUrl(shortCode, domain); + }, + ); - try { - const { shortUrlsList } = getState(); - const payload = shortUrlsList?.shortUrls?.data.find( - (url) => shortUrlMatches(url, shortCode, domain), - ) ?? await buildShlinkApiClient(getState).getShortUrl(shortCode, domain); + const { reducer } = createSlice({ + name: 'shortUrlDetailReducer', + initialState, + reducers: {}, + extraReducers: (builder) => { + builder.addCase(getShortUrlDetail.pending, () => ({ loading: true, error: false })); + builder.addCase(getShortUrlDetail.rejected, (_, { error }) => ( + { loading: false, error: true, errorData: parseApiError(error) } + )); + builder.addCase(getShortUrlDetail.fulfilled, (_, { payload: shortUrl }) => ({ ...initialState, shortUrl })); + }, + }); - dispatch({ payload, type: GET_SHORT_URL_DETAIL }); - } catch (e: any) { - dispatch({ type: GET_SHORT_URL_DETAIL_ERROR, errorData: parseApiError(e) }); - } + return { reducer, getShortUrlDetail }; }; diff --git a/src/short-urls/services/provideServices.ts b/src/short-urls/services/provideServices.ts index 25845ea2..d2b865a8 100644 --- a/src/short-urls/services/provideServices.ts +++ b/src/short-urls/services/provideServices.ts @@ -11,12 +11,12 @@ import { listShortUrls } from '../reducers/shortUrlsList'; import { shortUrlCreationReducerCreator } from '../reducers/shortUrlCreation'; import { shortUrlDeletionReducerCreator } from '../reducers/shortUrlDeletion'; import { shortUrlEditionReducerCreator } from '../reducers/shortUrlEdition'; +import { shortUrlDetailReducerCreator } from '../reducers/shortUrlDetail'; import { ConnectDecorator } from '../../container/types'; import { ShortUrlsTable } from '../ShortUrlsTable'; -import { QrCodeModal } from '../helpers/QrCodeModal'; import { ShortUrlForm } from '../ShortUrlForm'; import { EditShortUrl } from '../EditShortUrl'; -import { getShortUrlDetail } from '../reducers/shortUrlDetail'; +import { QrCodeModal } from '../helpers/QrCodeModal'; import { ExportShortUrlsBtn } from '../helpers/ExportShortUrlsBtn'; const provideServices = (bottle: Bottle, connect: ConnectDecorator) => { @@ -66,6 +66,9 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator) => { bottle.serviceFactory('shortUrlDeletionReducerCreator', shortUrlDeletionReducerCreator, 'buildShlinkApiClient'); bottle.serviceFactory('shortUrlDeletionReducer', prop('reducer'), 'shortUrlDeletionReducerCreator'); + bottle.serviceFactory('shortUrlDetailReducerCreator', shortUrlDetailReducerCreator, 'buildShlinkApiClient'); + bottle.serviceFactory('shortUrlDetailReducer', prop('reducer'), 'shortUrlDetailReducerCreator'); + // Actions bottle.serviceFactory('listShortUrls', listShortUrls, 'buildShlinkApiClient'); @@ -75,7 +78,7 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator) => { bottle.serviceFactory('deleteShortUrl', prop('deleteShortUrl'), 'shortUrlDeletionReducerCreator'); bottle.serviceFactory('resetDeleteShortUrl', prop('resetDeleteShortUrl'), 'shortUrlDeletionReducerCreator'); - bottle.serviceFactory('getShortUrlDetail', getShortUrlDetail, 'buildShlinkApiClient'); + bottle.serviceFactory('getShortUrlDetail', prop('getShortUrlDetail'), 'shortUrlDetailReducerCreator'); bottle.serviceFactory('editShortUrl', prop('editShortUrl'), 'shortUrlEditionReducerCreator'); }; diff --git a/test/short-urls/reducers/shortUrlDetail.test.ts b/test/short-urls/reducers/shortUrlDetail.test.ts index e520822e..cb745724 100644 --- a/test/short-urls/reducers/shortUrlDetail.test.ts +++ b/test/short-urls/reducers/shortUrlDetail.test.ts @@ -1,31 +1,29 @@ import { Mock } from 'ts-mockery'; -import reducer, { - getShortUrlDetail, - GET_SHORT_URL_DETAIL_START, - GET_SHORT_URL_DETAIL_ERROR, - GET_SHORT_URL_DETAIL, - ShortUrlDetailAction, -} from '../../../src/short-urls/reducers/shortUrlDetail'; +import { ShortUrlDetailAction, shortUrlDetailReducerCreator } from '../../../src/short-urls/reducers/shortUrlDetail'; import { ShortUrl } from '../../../src/short-urls/data'; import { ShlinkApiClient } from '../../../src/api/services/ShlinkApiClient'; import { ShlinkState } from '../../../src/container/types'; import { ShortUrlsList } from '../../../src/short-urls/reducers/shortUrlsList'; describe('shortUrlDetailReducer', () => { + const getShortUrlCall = jest.fn(); + const buildShlinkApiClient = () => Mock.of({ getShortUrl: getShortUrlCall }); + const { reducer, getShortUrlDetail } = shortUrlDetailReducerCreator(buildShlinkApiClient); + beforeEach(jest.clearAllMocks); describe('reducer', () => { const action = (type: string) => Mock.of({ type }); it('returns loading on GET_SHORT_URL_DETAIL_START', () => { - const state = reducer({ loading: false, error: false }, action(GET_SHORT_URL_DETAIL_START)); + const state = reducer({ loading: false, error: false }, action(getShortUrlDetail.pending.toString())); const { loading } = state; expect(loading).toEqual(true); }); it('stops loading and returns error on GET_SHORT_URL_DETAIL_ERROR', () => { - const state = reducer({ loading: true, error: false }, action(GET_SHORT_URL_DETAIL_ERROR)); + const state = reducer({ loading: true, error: false }, action(getShortUrlDetail.rejected.toString())); const { loading, error } = state; expect(loading).toEqual(false); @@ -34,7 +32,10 @@ describe('shortUrlDetailReducer', () => { it('return short URL on GET_SHORT_URL_DETAIL', () => { const actionShortUrl = Mock.of({ longUrl: 'foo', shortCode: 'bar' }); - const state = reducer({ loading: true, error: false }, { type: GET_SHORT_URL_DETAIL, payload: actionShortUrl }); + const state = reducer( + { loading: true, error: false }, + { type: getShortUrlDetail.fulfilled.toString(), payload: actionShortUrl }, + ); const { loading, error, shortUrl } = state; expect(loading).toEqual(false); @@ -44,21 +45,22 @@ describe('shortUrlDetailReducer', () => { }); describe('getShortUrlDetail', () => { - const buildApiClientMock = (returned: Promise) => Mock.of({ - getShortUrl: jest.fn(async () => returned), - }); const dispatchMock = jest.fn(); const buildGetState = (shortUrlsList?: ShortUrlsList) => () => Mock.of({ shortUrlsList }); it('dispatches start and error when promise is rejected', async () => { - const ShlinkApiClient = buildApiClientMock(Promise.reject({})); + getShortUrlCall.mockRejectedValue({}); - await getShortUrlDetail(() => ShlinkApiClient)({ shortCode: 'abc123', domain: '' })(dispatchMock, buildGetState()); + await getShortUrlDetail({ shortCode: 'abc123', domain: '' })(dispatchMock, buildGetState(), {}); expect(dispatchMock).toHaveBeenCalledTimes(2); - expect(dispatchMock).toHaveBeenNthCalledWith(1, { type: GET_SHORT_URL_DETAIL_START }); - expect(dispatchMock).toHaveBeenNthCalledWith(2, { type: GET_SHORT_URL_DETAIL_ERROR }); - expect(ShlinkApiClient.getShortUrl).toHaveBeenCalledTimes(1); + expect(dispatchMock).toHaveBeenNthCalledWith(1, expect.objectContaining({ + type: getShortUrlDetail.pending.toString(), + })); + expect(dispatchMock).toHaveBeenNthCalledWith(2, expect.objectContaining({ + type: getShortUrlDetail.rejected.toString(), + })); + expect(getShortUrlCall).toHaveBeenCalledTimes(1); }); it.each([ @@ -78,33 +80,44 @@ describe('shortUrlDetailReducer', () => { ], ])('performs API call when short URL is not found in local state', async (shortUrlsList?: ShortUrlsList) => { const resolvedShortUrl = Mock.of({ longUrl: 'foo', shortCode: 'abc123' }); - const ShlinkApiClient = buildApiClientMock(Promise.resolve(resolvedShortUrl)); + getShortUrlCall.mockResolvedValue(resolvedShortUrl); - await getShortUrlDetail(() => ShlinkApiClient)({ shortCode: 'abc123', domain: '' })(dispatchMock, buildGetState(shortUrlsList)); + await getShortUrlDetail({ shortCode: 'abc123', domain: '' })(dispatchMock, buildGetState(shortUrlsList), {}); expect(dispatchMock).toHaveBeenCalledTimes(2); - expect(dispatchMock).toHaveBeenNthCalledWith(1, { type: GET_SHORT_URL_DETAIL_START }); - expect(dispatchMock).toHaveBeenNthCalledWith(2, { type: GET_SHORT_URL_DETAIL, payload: resolvedShortUrl }); - expect(ShlinkApiClient.getShortUrl).toHaveBeenCalledTimes(1); + expect(dispatchMock).toHaveBeenNthCalledWith(1, expect.objectContaining({ + type: getShortUrlDetail.pending.toString(), + })); + expect(dispatchMock).toHaveBeenNthCalledWith(2, expect.objectContaining({ + type: getShortUrlDetail.fulfilled.toString(), + payload: resolvedShortUrl, + })); + expect(getShortUrlCall).toHaveBeenCalledTimes(1); }); it('avoids API calls when short URL is found in local state', async () => { const foundShortUrl = Mock.of({ longUrl: 'foo', shortCode: 'abc123' }); - const ShlinkApiClient = buildApiClientMock(Promise.resolve(Mock.all())); + getShortUrlCall.mockResolvedValue(Mock.all()); - await getShortUrlDetail(() => ShlinkApiClient)(foundShortUrl)( + await getShortUrlDetail(foundShortUrl)( dispatchMock, buildGetState(Mock.of({ shortUrls: { data: [foundShortUrl], }, })), + {}, ); expect(dispatchMock).toHaveBeenCalledTimes(2); - expect(dispatchMock).toHaveBeenNthCalledWith(1, { type: GET_SHORT_URL_DETAIL_START }); - expect(dispatchMock).toHaveBeenNthCalledWith(2, { type: GET_SHORT_URL_DETAIL, payload: foundShortUrl }); - expect(ShlinkApiClient.getShortUrl).not.toHaveBeenCalled(); + expect(dispatchMock).toHaveBeenNthCalledWith(1, expect.objectContaining({ + type: getShortUrlDetail.pending.toString(), + })); + expect(dispatchMock).toHaveBeenNthCalledWith(2, expect.objectContaining({ + type: getShortUrlDetail.fulfilled.toString(), + payload: foundShortUrl, + })); + expect(getShortUrlCall).not.toHaveBeenCalled(); }); }); });