From bbc47b387edcc55c60f3c8d2b71d78ba7102c847 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 26 Apr 2020 13:00:23 +0200 Subject: [PATCH 01/10] Created single reducer to handle settings --- src/reducers/index.js | 4 ++-- src/settings/RealTimeUpdates.js | 6 ++--- .../{realTimeUpdates.js => settings.js} | 15 +++++++----- src/settings/services/provideServices.js | 4 ++-- src/short-urls/ShortUrlsList.js | 6 ++--- src/short-urls/services/provideServices.js | 2 +- src/visits/ShortUrlVisits.js | 6 ++--- src/visits/services/provideServices.js | 2 +- ...alTimeUpdates.test.js => settings.test.js} | 23 ++++++++++--------- test/short-urls/ShortUrlsList.test.js | 2 ++ test/visits/ShortUrlVisits.test.js | 2 ++ 11 files changed, 40 insertions(+), 32 deletions(-) rename src/settings/reducers/{realTimeUpdates.js => settings.js} (68%) rename test/settings/reducers/{realTimeUpdates.test.js => settings.test.js} (60%) diff --git a/src/reducers/index.js b/src/reducers/index.js index 8e81e18a..feaf6d5d 100644 --- a/src/reducers/index.js +++ b/src/reducers/index.js @@ -14,7 +14,7 @@ import tagsListReducer from '../tags/reducers/tagsList'; import tagDeleteReducer from '../tags/reducers/tagDelete'; import tagEditReducer from '../tags/reducers/tagEdit'; import mercureInfoReducer from '../mercure/reducers/mercureInfo'; -import realTimeUpdatesReducer from '../settings/reducers/realTimeUpdates'; +import settingsReducer from '../settings/reducers/settings'; export default combineReducers({ servers: serversReducer, @@ -32,5 +32,5 @@ export default combineReducers({ tagDelete: tagDeleteReducer, tagEdit: tagEditReducer, mercureInfo: mercureInfoReducer, - realTimeUpdates: realTimeUpdatesReducer, + settings: settingsReducer, }); diff --git a/src/settings/RealTimeUpdates.js b/src/settings/RealTimeUpdates.js index eccd585f..1425a0ac 100644 --- a/src/settings/RealTimeUpdates.js +++ b/src/settings/RealTimeUpdates.js @@ -4,14 +4,14 @@ import PropTypes from 'prop-types'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { faInfoCircle } from '@fortawesome/free-solid-svg-icons'; import Checkbox from '../utils/Checkbox'; -import { RealTimeUpdatesType } from './reducers/realTimeUpdates'; +import { SettingsType } from './reducers/settings'; const propTypes = { - realTimeUpdates: RealTimeUpdatesType, + settings: SettingsType, setRealTimeUpdates: PropTypes.func, }; -const RealTimeUpdates = ({ realTimeUpdates, setRealTimeUpdates }) => ( +const RealTimeUpdates = ({ settings: { realTimeUpdates }, setRealTimeUpdates }) => ( Real-time updates diff --git a/src/settings/reducers/realTimeUpdates.js b/src/settings/reducers/settings.js similarity index 68% rename from src/settings/reducers/realTimeUpdates.js rename to src/settings/reducers/settings.js index c5cd726f..3160862b 100644 --- a/src/settings/reducers/realTimeUpdates.js +++ b/src/settings/reducers/settings.js @@ -3,16 +3,20 @@ import PropTypes from 'prop-types'; export const LOAD_REAL_TIME_UPDATES = 'shlink/realTimeUpdates/LOAD_REAL_TIME_UPDATES'; -export const RealTimeUpdatesType = PropTypes.shape({ - enabled: PropTypes.bool.isRequired, +export const SettingsType = PropTypes.shape({ + realTimeUpdates: PropTypes.shape({ + enabled: PropTypes.bool.isRequired, + }), }); const initialState = { - enabled: true, + realTimeUpdates: { + enabled: true, + }, }; export default handleActions({ - [LOAD_REAL_TIME_UPDATES]: (state, { enabled }) => ({ ...state, enabled }), + [LOAD_REAL_TIME_UPDATES]: (state, { realTimeUpdates }) => ({ ...state, realTimeUpdates }), }, initialState); export const setRealTimeUpdates = ({ updateSettings }, loadRealTimeUpdatesAction) => (enabled) => { @@ -23,10 +27,9 @@ export const setRealTimeUpdates = ({ updateSettings }, loadRealTimeUpdatesAction export const loadRealTimeUpdates = ({ loadSettings }) => () => { const { realTimeUpdates = {} } = loadSettings(); - const { enabled = true } = realTimeUpdates; return { type: LOAD_REAL_TIME_UPDATES, - enabled, + realTimeUpdates, }; }; diff --git a/src/settings/services/provideServices.js b/src/settings/services/provideServices.js index ef70b49f..cbc24a12 100644 --- a/src/settings/services/provideServices.js +++ b/src/settings/services/provideServices.js @@ -1,6 +1,6 @@ import RealTimeUpdates from '../RealTimeUpdates'; import Settings from '../Settings'; -import { loadRealTimeUpdates, setRealTimeUpdates } from '../reducers/realTimeUpdates'; +import { loadRealTimeUpdates, setRealTimeUpdates } from '../reducers/settings'; import SettingsService from './SettingsService'; const provideServices = (bottle, connect) => { @@ -8,7 +8,7 @@ const provideServices = (bottle, connect) => { bottle.serviceFactory('Settings', Settings, 'RealTimeUpdates'); bottle.serviceFactory('RealTimeUpdates', () => RealTimeUpdates); - bottle.decorator('RealTimeUpdates', connect([ 'realTimeUpdates' ], [ 'setRealTimeUpdates' ])); + bottle.decorator('RealTimeUpdates', connect([ 'settings' ], [ 'setRealTimeUpdates' ])); // Services bottle.service('SettingsService', SettingsService, 'Storage'); diff --git a/src/short-urls/ShortUrlsList.js b/src/short-urls/ShortUrlsList.js index 3b36d459..e0c2f30c 100644 --- a/src/short-urls/ShortUrlsList.js +++ b/src/short-urls/ShortUrlsList.js @@ -9,7 +9,7 @@ import SortingDropdown from '../utils/SortingDropdown'; import { determineOrderDir } from '../utils/utils'; import { MercureInfoType } from '../mercure/reducers/mercureInfo'; import { bindToMercureTopic } from '../mercure/helpers'; -import { RealTimeUpdatesType } from '../settings/reducers/realTimeUpdates'; +import { SettingsType } from '../settings/reducers/settings'; import { shortUrlType } from './reducers/shortUrlsList'; import { shortUrlsListParamsType } from './reducers/shortUrlsListParams'; import './ShortUrlsList.scss'; @@ -34,7 +34,7 @@ const propTypes = { createNewVisit: PropTypes.func, loadMercureInfo: PropTypes.func, mercureInfo: MercureInfoType, - realTimeUpdates: RealTimeUpdatesType, + settings: SettingsType, }; // FIXME Replace with typescript: (ShortUrlsRow component) @@ -52,7 +52,7 @@ const ShortUrlsList = (ShortUrlsRow) => { createNewVisit, loadMercureInfo, mercureInfo, - realTimeUpdates, + settings: { realTimeUpdates }, }) => { const { orderBy } = shortUrlsListParams; const [ order, setOrder ] = useState({ diff --git a/src/short-urls/services/provideServices.js b/src/short-urls/services/provideServices.js index 6a35ac3d..02a0d5fa 100644 --- a/src/short-urls/services/provideServices.js +++ b/src/short-urls/services/provideServices.js @@ -31,7 +31,7 @@ const provideServices = (bottle, connect) => { bottle.serviceFactory('ShortUrlsList', ShortUrlsList, 'ShortUrlsRow'); bottle.decorator('ShortUrlsList', connect( - [ 'selectedServer', 'shortUrlsListParams', 'mercureInfo', 'realTimeUpdates' ], + [ 'selectedServer', 'shortUrlsListParams', 'mercureInfo', 'settings' ], [ 'listShortUrls', 'resetShortUrlParams', 'createNewVisit', 'loadMercureInfo' ] )); diff --git a/src/visits/ShortUrlVisits.js b/src/visits/ShortUrlVisits.js index 6882e478..12f5deb3 100644 --- a/src/visits/ShortUrlVisits.js +++ b/src/visits/ShortUrlVisits.js @@ -12,7 +12,7 @@ import { formatDate } from '../utils/helpers/date'; import { useToggle } from '../utils/helpers/hooks'; import { MercureInfoType } from '../mercure/reducers/mercureInfo'; import { bindToMercureTopic } from '../mercure/helpers'; -import { RealTimeUpdatesType } from '../settings/reducers/realTimeUpdates'; +import { SettingsType } from '../settings/reducers/settings'; import SortableBarGraph from './SortableBarGraph'; import { shortUrlVisitsType } from './reducers/shortUrlVisits'; import VisitsHeader from './VisitsHeader'; @@ -39,7 +39,7 @@ const propTypes = { createNewVisit: PropTypes.func, loadMercureInfo: PropTypes.func, mercureInfo: MercureInfoType, - realTimeUpdates: RealTimeUpdatesType, + settings: SettingsType, }; const highlightedVisitsToStats = (highlightedVisits, prop) => highlightedVisits.reduce((acc, highlightedVisit) => { @@ -68,7 +68,7 @@ const ShortUrlVisits = ({ processStatsFromVisits, normalizeVisits }, OpenMapModa createNewVisit, loadMercureInfo, mercureInfo, - realTimeUpdates, + settings: { realTimeUpdates }, }) => { const [ startDate, setStartDate ] = useState(undefined); const [ endDate, setEndDate ] = useState(undefined); diff --git a/src/visits/services/provideServices.js b/src/visits/services/provideServices.js index f9f31b80..404bc4a1 100644 --- a/src/visits/services/provideServices.js +++ b/src/visits/services/provideServices.js @@ -11,7 +11,7 @@ const provideServices = (bottle, connect) => { bottle.serviceFactory('MapModal', () => MapModal); bottle.serviceFactory('ShortUrlVisits', ShortUrlVisits, 'VisitsParser', 'OpenMapModalBtn'); bottle.decorator('ShortUrlVisits', connect( - [ 'shortUrlVisits', 'shortUrlDetail', 'mercureInfo', 'realTimeUpdates' ], + [ 'shortUrlVisits', 'shortUrlDetail', 'mercureInfo', 'settings' ], [ 'getShortUrlVisits', 'getShortUrlDetail', 'cancelGetShortUrlVisits', 'createNewVisit', 'loadMercureInfo' ] )); diff --git a/test/settings/reducers/realTimeUpdates.test.js b/test/settings/reducers/settings.test.js similarity index 60% rename from test/settings/reducers/realTimeUpdates.test.js rename to test/settings/reducers/settings.test.js index cdd463e6..2eb7e5db 100644 --- a/test/settings/reducers/realTimeUpdates.test.js +++ b/test/settings/reducers/settings.test.js @@ -2,34 +2,35 @@ import reducer, { LOAD_REAL_TIME_UPDATES, loadRealTimeUpdates, setRealTimeUpdates, -} from '../../../src/settings/reducers/realTimeUpdates'; +} from '../../../src/settings/reducers/settings'; -describe('realTimeUpdatesReducer', () => { +describe('settingsReducer', () => { const SettingsServiceMock = { updateSettings: jest.fn(), loadSettings: jest.fn(), }; + const realTimeUpdates = { enabled: true }; afterEach(jest.clearAllMocks); describe('reducer', () => { it('returns realTimeUpdates when action is LOAD_REAL_TIME_UPDATES', () => { - expect(reducer({}, { type: LOAD_REAL_TIME_UPDATES, enabled: true })).toEqual({ enabled: true }); + expect(reducer({}, { type: LOAD_REAL_TIME_UPDATES, realTimeUpdates })).toEqual({ realTimeUpdates }); }); }); describe('loadRealTimeUpdates', () => { - it.each([ - [{}, true ], - [{ realTimeUpdates: {} }, true ], - [{ realTimeUpdates: { enabled: true } }, true ], - [{ realTimeUpdates: { enabled: false } }, false ], - ])('loads settings and returns LOAD_REAL_TIME_UPDATES action', (loadedSettings, expectedEnabled) => { - SettingsServiceMock.loadSettings.mockReturnValue(loadedSettings); + it.each([[ true ], [ false ]])('loads settings and returns LOAD_REAL_TIME_UPDATES action', (enabled) => { + const realTimeUpdates = { enabled }; + + SettingsServiceMock.loadSettings.mockReturnValue({ realTimeUpdates }); const result = loadRealTimeUpdates(SettingsServiceMock)(); - expect(result).toEqual({ type: LOAD_REAL_TIME_UPDATES, enabled: expectedEnabled }); + expect(result).toEqual({ + type: LOAD_REAL_TIME_UPDATES, + realTimeUpdates, + }); expect(SettingsServiceMock.loadSettings).toHaveBeenCalled(); }); }); diff --git a/test/short-urls/ShortUrlsList.test.js b/test/short-urls/ShortUrlsList.test.js index 050238b9..b66e9ac0 100644 --- a/test/short-urls/ShortUrlsList.test.js +++ b/test/short-urls/ShortUrlsList.test.js @@ -9,6 +9,7 @@ describe('', () => { const ShortUrlsRow = () => ''; const listShortUrlsMock = jest.fn(); const resetShortUrlParamsMock = jest.fn(); + const realTimeUpdates = { enabled: true }; const ShortUrlsList = shortUrlsListCreator(ShortUrlsRow); @@ -37,6 +38,7 @@ describe('', () => { ] } mercureInfo={{ loading: true }} + settings={{ realTimeUpdates }} /> ); }); diff --git a/test/visits/ShortUrlVisits.test.js b/test/visits/ShortUrlVisits.test.js index 0f221308..76cf0b28 100644 --- a/test/visits/ShortUrlVisits.test.js +++ b/test/visits/ShortUrlVisits.test.js @@ -21,6 +21,7 @@ describe('', () => { const history = { goBack: jest.fn(), }; + const realTimeUpdates = { enabled: true }; const createComponent = (shortUrlVisits) => { const ShortUrlVisits = createShortUrlVisits({ processStatsFromVisits, normalizeVisits: identity }, () => ''); @@ -36,6 +37,7 @@ describe('', () => { shortUrlDetail={{}} cancelGetShortUrlVisits={identity} matchMedia={() => ({ matches: false })} + settings={{ realTimeUpdates }} /> ); From 86bf1515d4d1eba412444002922101714dc22cff Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 26 Apr 2020 19:04:17 +0200 Subject: [PATCH 02/10] Added redux middleware to save parts of the store in the local storage transparently --- package-lock.json | 27 ++++++++++++++++ package.json | 1 + src/App.js | 36 +++++++++------------- src/container/index.js | 1 - src/container/store.js | 11 +++++-- src/settings/reducers/settings.js | 22 ++++--------- src/settings/services/provideServices.js | 5 ++- test/settings/reducers/settings.test.js | 39 +++--------------------- 8 files changed, 65 insertions(+), 77 deletions(-) diff --git a/package-lock.json b/package-lock.json index 1a22485b..05204cab 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1463,6 +1463,14 @@ "integrity": "sha512-shAmDyaQC4H92APFoIaVDHCx5bStIocgvbwQyxPRrbUY20V1EYTbSDchWbuwlMG3V17cprZhA6+78JfB+3DTPw==", "dev": true }, + "@shlinkio/redux-localstorage-simple": { + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/@shlinkio/redux-localstorage-simple/-/redux-localstorage-simple-2.2.0.tgz", + "integrity": "sha512-2/VggbehDAM1dOH7rT3Qjr/MTp7qQ6VeTM+Ez4JnMUPtU9OxgV9FQbKqduasLT4EZhlRUhxwBp7K6WO3gROQDA==", + "requires": { + "object-merge": "2.5.1" + } + }, "@stryker-mutator/api": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/@stryker-mutator/api/-/api-2.1.0.tgz", @@ -4246,6 +4254,11 @@ "shallow-clone": "^0.1.2" } }, + "clone-function": { + "version": "1.0.6", + "resolved": "https://registry.npmjs.org/clone-function/-/clone-function-1.0.6.tgz", + "integrity": "sha1-QoRxk3dQvKnEjsv7wW9uIy90oD0=" + }, "clone-regexp": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/clone-regexp/-/clone-regexp-1.0.1.tgz", @@ -11720,6 +11733,11 @@ "kind-of": "^3.0.3" } }, + "object-foreach": { + "version": "0.1.2", + "resolved": "https://registry.npmjs.org/object-foreach/-/object-foreach-0.1.2.tgz", + "integrity": "sha1-10IcW0DjtqPvV6xiQ2jSHY+NLew=" + }, "object-hash": { "version": "1.3.1", "resolved": "https://registry.npmjs.org/object-hash/-/object-hash-1.3.1.tgz", @@ -11744,6 +11762,15 @@ "integrity": "sha512-NuAESUOUMrlIXOfHKzD6bpPu3tYt3xvjNdRIQ+FeT0lNb4K8WR70CaDxhuNguS2XG+GjkyMwOzsN5ZktImfhLA==", "dev": true }, + "object-merge": { + "version": "2.5.1", + "resolved": "https://registry.npmjs.org/object-merge/-/object-merge-2.5.1.tgz", + "integrity": "sha1-B36JFc446nKUeIRIxd0znjTfQic=", + "requires": { + "clone-function": ">=1.0.1", + "object-foreach": ">=0.1.2" + } + }, "object-visit": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/object-visit/-/object-visit-1.0.1.tgz", diff --git a/package.json b/package.json index ab2033c1..2e2678a4 100644 --- a/package.json +++ b/package.json @@ -27,6 +27,7 @@ "@fortawesome/free-regular-svg-icons": "^5.11.2", "@fortawesome/free-solid-svg-icons": "^5.11.2", "@fortawesome/react-fontawesome": "^0.1.5", + "@shlinkio/redux-localstorage-simple": "^2.2.0", "array-filter": "^1.0.0", "array-map": "^0.0.0", "array-reduce": "^0.0.0", diff --git a/src/App.js b/src/App.js index fd6c859c..edb9df36 100644 --- a/src/App.js +++ b/src/App.js @@ -1,29 +1,23 @@ -import React, { useEffect } from 'react'; +import React from 'react'; import { Route, Switch } from 'react-router-dom'; import NotFound from './common/NotFound'; import './App.scss'; -const App = (MainHeader, Home, MenuLayout, CreateServer, EditServer, Settings) => ({ loadRealTimeUpdates }) => { - useEffect(() => { - loadRealTimeUpdates(); - }, []); +const App = (MainHeader, Home, MenuLayout, CreateServer, EditServer, Settings) => () => ( +
+ - return ( -
- - -
- - - - - - - - -
+
+ + + + + + + +
- ); -}; +
+); export default App; diff --git a/src/container/index.js b/src/container/index.js index 95383b13..3a8ac83a 100644 --- a/src/container/index.js +++ b/src/container/index.js @@ -29,7 +29,6 @@ const connect = (propsFromState, actionServiceNames = []) => ); bottle.serviceFactory('App', App, 'MainHeader', 'Home', 'MenuLayout', 'CreateServer', 'EditServer', 'Settings'); -bottle.decorator('App', connect(null, [ 'loadRealTimeUpdates' ])); provideCommonServices(bottle, connect, withRouter); provideShortUrlsServices(bottle, connect); diff --git a/src/container/store.js b/src/container/store.js index 0d5b2b2d..6d0de762 100644 --- a/src/container/store.js +++ b/src/container/store.js @@ -1,13 +1,20 @@ import ReduxThunk from 'redux-thunk'; import { applyMiddleware, compose, createStore } from 'redux'; +import { save, load } from '@shlinkio/redux-localstorage-simple'; import reducers from '../reducers'; const composeEnhancers = process.env.NODE_ENV !== 'production' && window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ ? window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ : compose; -const store = createStore(reducers, composeEnhancers( - applyMiddleware(ReduxThunk) +const localStorageConfig = { + states: [ 'settings' ], + namespace: 'shlink', + namespaceSeparator: '.', +}; + +const store = createStore(reducers, load(localStorageConfig), composeEnhancers( + applyMiddleware(save(localStorageConfig), ReduxThunk) )); export default store; diff --git a/src/settings/reducers/settings.js b/src/settings/reducers/settings.js index 3160862b..6771dbb5 100644 --- a/src/settings/reducers/settings.js +++ b/src/settings/reducers/settings.js @@ -1,7 +1,7 @@ import { handleActions } from 'redux-actions'; import PropTypes from 'prop-types'; -export const LOAD_REAL_TIME_UPDATES = 'shlink/realTimeUpdates/LOAD_REAL_TIME_UPDATES'; +export const SET_REAL_TIME_UPDATES = 'shlink/realTimeUpdates/SET_REAL_TIME_UPDATES'; export const SettingsType = PropTypes.shape({ realTimeUpdates: PropTypes.shape({ @@ -16,20 +16,10 @@ const initialState = { }; export default handleActions({ - [LOAD_REAL_TIME_UPDATES]: (state, { realTimeUpdates }) => ({ ...state, realTimeUpdates }), + [SET_REAL_TIME_UPDATES]: (state, { realTimeUpdates }) => ({ ...state, realTimeUpdates }), }, initialState); -export const setRealTimeUpdates = ({ updateSettings }, loadRealTimeUpdatesAction) => (enabled) => { - updateSettings({ realTimeUpdates: { enabled } }); - - return loadRealTimeUpdatesAction(); -}; - -export const loadRealTimeUpdates = ({ loadSettings }) => () => { - const { realTimeUpdates = {} } = loadSettings(); - - return { - type: LOAD_REAL_TIME_UPDATES, - realTimeUpdates, - }; -}; +export const setRealTimeUpdates = (enabled) => ({ + type: SET_REAL_TIME_UPDATES, + realTimeUpdates: { enabled }, +}); diff --git a/src/settings/services/provideServices.js b/src/settings/services/provideServices.js index cbc24a12..08ba77d3 100644 --- a/src/settings/services/provideServices.js +++ b/src/settings/services/provideServices.js @@ -1,6 +1,6 @@ import RealTimeUpdates from '../RealTimeUpdates'; import Settings from '../Settings'; -import { loadRealTimeUpdates, setRealTimeUpdates } from '../reducers/settings'; +import { setRealTimeUpdates } from '../reducers/settings'; import SettingsService from './SettingsService'; const provideServices = (bottle, connect) => { @@ -14,8 +14,7 @@ const provideServices = (bottle, connect) => { bottle.service('SettingsService', SettingsService, 'Storage'); // Actions - bottle.serviceFactory('setRealTimeUpdates', setRealTimeUpdates, 'SettingsService', 'loadRealTimeUpdates'); - bottle.serviceFactory('loadRealTimeUpdates', loadRealTimeUpdates, 'SettingsService'); + bottle.serviceFactory('setRealTimeUpdates', () => setRealTimeUpdates); }; export default provideServices; diff --git a/test/settings/reducers/settings.test.js b/test/settings/reducers/settings.test.js index 2eb7e5db..80f22c49 100644 --- a/test/settings/reducers/settings.test.js +++ b/test/settings/reducers/settings.test.js @@ -1,48 +1,19 @@ -import reducer, { - LOAD_REAL_TIME_UPDATES, - loadRealTimeUpdates, - setRealTimeUpdates, -} from '../../../src/settings/reducers/settings'; +import reducer, { SET_REAL_TIME_UPDATES, setRealTimeUpdates } from '../../../src/settings/reducers/settings'; describe('settingsReducer', () => { - const SettingsServiceMock = { - updateSettings: jest.fn(), - loadSettings: jest.fn(), - }; const realTimeUpdates = { enabled: true }; - afterEach(jest.clearAllMocks); - describe('reducer', () => { - it('returns realTimeUpdates when action is LOAD_REAL_TIME_UPDATES', () => { - expect(reducer({}, { type: LOAD_REAL_TIME_UPDATES, realTimeUpdates })).toEqual({ realTimeUpdates }); - }); - }); - - describe('loadRealTimeUpdates', () => { - it.each([[ true ], [ false ]])('loads settings and returns LOAD_REAL_TIME_UPDATES action', (enabled) => { - const realTimeUpdates = { enabled }; - - SettingsServiceMock.loadSettings.mockReturnValue({ realTimeUpdates }); - - const result = loadRealTimeUpdates(SettingsServiceMock)(); - - expect(result).toEqual({ - type: LOAD_REAL_TIME_UPDATES, - realTimeUpdates, - }); - expect(SettingsServiceMock.loadSettings).toHaveBeenCalled(); + it('returns realTimeUpdates when action is SET_REAL_TIME_UPDATES', () => { + expect(reducer({}, { type: SET_REAL_TIME_UPDATES, realTimeUpdates })).toEqual({ realTimeUpdates }); }); }); describe('setRealTimeUpdates', () => { it.each([[ true ], [ false ]])('updates settings with provided value and then loads updates again', (enabled) => { - const loadRealTimeUpdatesAction = jest.fn(); + const result = setRealTimeUpdates(enabled); - setRealTimeUpdates(SettingsServiceMock, loadRealTimeUpdatesAction)(enabled); - - expect(SettingsServiceMock.updateSettings).toHaveBeenCalledWith({ realTimeUpdates: { enabled } }); - expect(loadRealTimeUpdatesAction).toHaveBeenCalled(); + expect(result).toEqual({ type: SET_REAL_TIME_UPDATES, realTimeUpdates: { enabled } }); }); }); }); From 7dd6a3160992fdacfe07378894318e56905942f6 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 26 Apr 2020 19:07:47 +0200 Subject: [PATCH 03/10] Deleted SettingsService, as the task is not transparently handled by a redux middleware --- src/settings/services/SettingsService.js | 14 ------ src/settings/services/provideServices.js | 4 -- .../settings/services/SettingsService.test.js | 46 ------------------- 3 files changed, 64 deletions(-) delete mode 100644 src/settings/services/SettingsService.js delete mode 100644 test/settings/services/SettingsService.test.js diff --git a/src/settings/services/SettingsService.js b/src/settings/services/SettingsService.js deleted file mode 100644 index a3a39446..00000000 --- a/src/settings/services/SettingsService.js +++ /dev/null @@ -1,14 +0,0 @@ -const SETTINGS_STORAGE_KEY = 'settings'; - -export default class SettingsService { - constructor(storage) { - this.storage = storage; - } - - loadSettings = () => this.storage.get(SETTINGS_STORAGE_KEY) || {}; - - updateSettings = (settingsToUpdate) => this.storage.set(SETTINGS_STORAGE_KEY, { - ...this.loadSettings(), - ...settingsToUpdate, - }) -} diff --git a/src/settings/services/provideServices.js b/src/settings/services/provideServices.js index 08ba77d3..1c9e7863 100644 --- a/src/settings/services/provideServices.js +++ b/src/settings/services/provideServices.js @@ -1,7 +1,6 @@ import RealTimeUpdates from '../RealTimeUpdates'; import Settings from '../Settings'; import { setRealTimeUpdates } from '../reducers/settings'; -import SettingsService from './SettingsService'; const provideServices = (bottle, connect) => { // Components @@ -10,9 +9,6 @@ const provideServices = (bottle, connect) => { bottle.serviceFactory('RealTimeUpdates', () => RealTimeUpdates); bottle.decorator('RealTimeUpdates', connect([ 'settings' ], [ 'setRealTimeUpdates' ])); - // Services - bottle.service('SettingsService', SettingsService, 'Storage'); - // Actions bottle.serviceFactory('setRealTimeUpdates', () => setRealTimeUpdates); }; diff --git a/test/settings/services/SettingsService.test.js b/test/settings/services/SettingsService.test.js deleted file mode 100644 index 9e9419db..00000000 --- a/test/settings/services/SettingsService.test.js +++ /dev/null @@ -1,46 +0,0 @@ -import SettingsService from '../../../src/settings/services/SettingsService'; - -describe('SettingsService', () => { - const settings = { foo: 'bar' }; - const createService = (withSettings = true) => { - const storageMock = { - set: jest.fn(), - get: jest.fn(() => withSettings ? settings : undefined), - }; - const service = new SettingsService(storageMock); - - return [ service, storageMock ]; - }; - - afterEach(jest.resetAllMocks); - - describe('loadSettings', () => { - it.each([ - [ false, {}], - [ true, settings ], - ])('returns result if found in storage', (withSettings, expectedResult) => { - const [ service, storageMock ] = createService(withSettings); - - const result = service.loadSettings(); - - expect(result).toEqual(expectedResult); - expect(storageMock.get).toHaveBeenCalledTimes(1); - expect(storageMock.set).not.toHaveBeenCalled(); - }); - }); - - describe('updateSettings', () => { - it.each([ - [ false, { hi: 'goodbye' }, { hi: 'goodbye' }], - [ true, { hi: 'goodbye' }, { foo: 'bar', hi: 'goodbye' }], - [ true, { foo: 'goodbye' }, { foo: 'goodbye' }], - ])('appends provided data to existing settings', (withSettings, settingsToUpdate, expectedResult) => { - const [ service, storageMock ] = createService(withSettings); - - service.updateSettings(settingsToUpdate); - - expect(storageMock.get).toHaveBeenCalledTimes(1); - expect(storageMock.set).toHaveBeenCalledWith(expect.anything(), expectedResult); - }); - }); -}); From 277b5e43f81c13453e12f9a32ab6db3f57bf3f79 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 27 Apr 2020 10:49:55 +0200 Subject: [PATCH 04/10] Flatten model holding list of servers --- src/common/Home.js | 13 ++++++------- src/servers/ServersDropdown.js | 12 ++++-------- src/servers/helpers/ServerError.js | 4 ++-- src/servers/reducers/server.js | 12 ++---------- test/common/Home.test.js | 22 +++++++--------------- test/servers/ServersDropdown.test.js | 24 +++++------------------- test/servers/helpers/ServerError.test.js | 2 +- test/servers/reducers/server.test.js | 13 +++++-------- 8 files changed, 32 insertions(+), 70 deletions(-) diff --git a/src/common/Home.js b/src/common/Home.js index 7c7cf396..63fbc80a 100644 --- a/src/common/Home.js +++ b/src/common/Home.js @@ -10,9 +10,9 @@ const propTypes = { servers: PropTypes.object, }; -const Home = ({ resetSelectedServer, servers: { list, loading } }) => { - const servers = values(list); - const hasServers = !isEmpty(servers); +const Home = ({ resetSelectedServer, servers }) => { + const serversList = values(servers); + const hasServers = !isEmpty(serversList); useEffect(() => { resetSelectedServer(); @@ -21,10 +21,9 @@ const Home = ({ resetSelectedServer, servers: { list, loading } }) => { return (

Welcome to Shlink

- - {!loading && hasServers && Please, select a server.} - {!loading && !hasServers && Please, add a server.} - {loading && Trying to load servers...} + + {hasServers && Please, select a server.} + {!hasServers && Please, add a server.}
); diff --git a/src/servers/ServersDropdown.js b/src/servers/ServersDropdown.js index c66cb22c..f6092d60 100644 --- a/src/servers/ServersDropdown.js +++ b/src/servers/ServersDropdown.js @@ -15,22 +15,18 @@ const ServersDropdown = (serversExporter) => class ServersDropdown extends React }; renderServers = () => { - const { servers: { list, loading }, selectedServer } = this.props; - const servers = values(list); + const { servers, selectedServer } = this.props; + const serversList = values(servers); const { push } = this.props.history; const loadServer = (id) => push(`/server/${id}/list-short-urls/1`); - if (loading) { - return Trying to load servers...; - } - - if (isEmpty(servers)) { + if (isEmpty(serversList)) { return Add a server first...; } return ( - {servers.map(({ name, id }) => ( + {serversList.map(({ name, id }) => ( loadServer(id)}> {name} diff --git a/src/servers/helpers/ServerError.js b/src/servers/helpers/ServerError.js index 7389d394..14983987 100644 --- a/src/servers/helpers/ServerError.js +++ b/src/servers/helpers/ServerError.js @@ -13,7 +13,7 @@ const propTypes = { }; export const ServerError = (DeleteServerButton) => { - const ServerErrorComp = ({ type, servers: { list }, selectedServer }) => ( + const ServerErrorComp = ({ type, servers, selectedServer }) => (
@@ -27,7 +27,7 @@ export const ServerError = (DeleteServerButton) => {
- + These are the Shlink servers currently configured. Choose one of them or add a new one. diff --git a/src/servers/reducers/server.js b/src/servers/reducers/server.js index e665751c..d703ce11 100644 --- a/src/servers/reducers/server.js +++ b/src/servers/reducers/server.js @@ -3,25 +3,17 @@ import { pipe, isEmpty, assoc, map, prop } from 'ramda'; import { v4 as uuid } from 'uuid'; import { homepage } from '../../../package.json'; -/* eslint-disable padding-line-between-statements */ -export const FETCH_SERVERS_START = 'shlink/servers/FETCH_SERVERS_START'; export const FETCH_SERVERS = 'shlink/servers/FETCH_SERVERS'; -/* eslint-enable padding-line-between-statements */ -const initialState = { - list: {}, - loading: false, -}; +const initialState = {}; const assocId = (server) => assoc('id', server.id || uuid(), server); export default handleActions({ - [FETCH_SERVERS_START]: (state) => ({ ...state, loading: true }), - [FETCH_SERVERS]: (state, { list }) => ({ list, loading: false }), + [FETCH_SERVERS]: (state, { list }) => list, }, initialState); export const listServers = ({ listServers, createServers }, { get }) => () => async (dispatch) => { - dispatch({ type: FETCH_SERVERS_START }); const localList = listServers(); if (!isEmpty(localList)) { diff --git a/test/common/Home.test.js b/test/common/Home.test.js index fa6e7993..0af589b2 100644 --- a/test/common/Home.test.js +++ b/test/common/Home.test.js @@ -6,7 +6,7 @@ describe('', () => { let wrapped; const defaultProps = { resetSelectedServer: jest.fn(), - servers: { loading: false, list: {} }, + servers: {}, }; const createComponent = (props) => { const actualProps = { ...defaultProps, ...props }; @@ -24,20 +24,12 @@ describe('', () => { expect(wrapped.find('Link')).toHaveLength(1); }); - it('shows message when loading servers', () => { - const wrapped = createComponent({ servers: { loading: true } }); - const span = wrapped.find('span'); - - expect(span).toHaveLength(1); - expect(span.text()).toContain('Trying to load servers...'); - }); - - it('Asks to select a server when not loadign and servers exist', () => { - const list = [ - { name: 'foo', id: '1' }, - { name: 'bar', id: '2' }, - ]; - const wrapped = createComponent({ servers: { list } }); + it('asks to select a server when servers exist', () => { + const servers = { + 1: { name: 'foo', id: '1' }, + 2: { name: 'bar', id: '2' }, + }; + const wrapped = createComponent({ servers }); const span = wrapped.find('span'); expect(span).toHaveLength(1); diff --git a/test/servers/ServersDropdown.test.js b/test/servers/ServersDropdown.test.js index 2120e695..d90dbba2 100644 --- a/test/servers/ServersDropdown.test.js +++ b/test/servers/ServersDropdown.test.js @@ -8,12 +8,9 @@ describe('', () => { let wrapped; let ServersDropdown; const servers = { - list: { - '1a': { name: 'foo', id: 1 }, - '2b': { name: 'bar', id: 2 }, - '3c': { name: 'baz', id: 3 }, - }, - loading: false, + '1a': { name: 'foo', id: 1 }, + '2b': { name: 'bar', id: 2 }, + '3c': { name: 'baz', id: 3 }, }; const history = { push: jest.fn(), @@ -26,7 +23,7 @@ describe('', () => { afterEach(() => wrapped.unmount()); it('contains the list of servers, the divider and the export button', () => - expect(wrapped.find(DropdownItem)).toHaveLength(values(servers.list).length + 2)); + expect(wrapped.find(DropdownItem)).toHaveLength(values(servers).length + 2)); it('contains a toggle with proper title', () => expect(wrapped.find(DropdownToggle)).toHaveLength(1)); @@ -40,7 +37,7 @@ describe('', () => { it('shows a message when no servers exist yet', () => { wrapped = shallow( - + ); const item = wrapped.find(DropdownItem); @@ -48,15 +45,4 @@ describe('', () => { expect(item.prop('disabled')).toEqual(true); expect(item.find('i').text()).toEqual('Add a server first...'); }); - - it('shows a message when loading', () => { - wrapped = shallow( - - ); - const item = wrapped.find(DropdownItem); - - expect(item).toHaveLength(1); - expect(item.prop('disabled')).toEqual(true); - expect(item.find('i').text()).toEqual('Trying to load servers...'); - }); }); diff --git a/test/servers/helpers/ServerError.test.js b/test/servers/helpers/ServerError.test.js index 32e1871c..01811564 100644 --- a/test/servers/helpers/ServerError.test.js +++ b/test/servers/helpers/ServerError.test.js @@ -32,7 +32,7 @@ describe('', () => { ])('renders expected information for type "%s"', (type, textsToFind) => { wrapper = shallow( - + ); const wrapperText = wrapper.html(); diff --git a/test/servers/reducers/server.test.js b/test/servers/reducers/server.test.js index baa4e82f..73d6dd83 100644 --- a/test/servers/reducers/server.test.js +++ b/test/servers/reducers/server.test.js @@ -6,7 +6,6 @@ import reducer, { createServers, editServer, FETCH_SERVERS, - FETCH_SERVERS_START, } from '../../../src/servers/reducers/server'; describe('serverReducer', () => { @@ -27,7 +26,7 @@ describe('serverReducer', () => { describe('reducer', () => { it('returns servers when action is FETCH_SERVERS', () => - expect(reducer({}, { type: FETCH_SERVERS, list })).toEqual({ loading: false, list })); + expect(reducer({}, { type: FETCH_SERVERS, list })).toEqual(list)); }); describe('action creators', () => { @@ -39,9 +38,8 @@ describe('serverReducer', () => { it('fetches servers from local storage when found', async () => { await listServers(ServersServiceMock, axios)()(dispatch); - expect(dispatch).toHaveBeenCalledTimes(2); - expect(dispatch).toHaveBeenNthCalledWith(1, { type: FETCH_SERVERS_START }); - expect(dispatch).toHaveBeenNthCalledWith(2, expectedFetchServersResult); + expect(dispatch).toHaveBeenCalledTimes(1); + expect(dispatch).toHaveBeenNthCalledWith(1, expectedFetchServersResult); expect(ServersServiceMock.listServers).toHaveBeenCalledTimes(1); expect(ServersServiceMock.createServer).not.toHaveBeenCalled(); expect(ServersServiceMock.editServer).not.toHaveBeenCalled(); @@ -90,9 +88,8 @@ describe('serverReducer', () => { await listServers(NoListServersServiceMock, axios)()(dispatch); - expect(dispatch).toHaveBeenCalledTimes(2); - expect(dispatch).toHaveBeenNthCalledWith(1, { type: FETCH_SERVERS_START }); - expect(dispatch).toHaveBeenNthCalledWith(2, { type: FETCH_SERVERS, list: expectedList }); + expect(dispatch).toHaveBeenCalledTimes(1); + expect(dispatch).toHaveBeenNthCalledWith(1, { type: FETCH_SERVERS, list: expectedList }); expect(NoListServersServiceMock.listServers).toHaveBeenCalledTimes(1); expect(NoListServersServiceMock.createServer).not.toHaveBeenCalled(); expect(NoListServersServiceMock.editServer).not.toHaveBeenCalled(); From 8b2cbf7aea197e9c519ddcd206e797c221abfff5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 27 Apr 2020 10:52:19 +0200 Subject: [PATCH 05/10] Some minor refactorings --- src/reducers/index.js | 2 +- src/servers/reducers/{server.js => servers.js} | 8 ++++---- src/servers/services/provideServices.js | 2 +- test/servers/reducers/server.test.js | 12 ++++++------ 4 files changed, 12 insertions(+), 12 deletions(-) rename src/servers/reducers/{server.js => servers.js} (83%) diff --git a/src/reducers/index.js b/src/reducers/index.js index feaf6d5d..51b95d8c 100644 --- a/src/reducers/index.js +++ b/src/reducers/index.js @@ -1,5 +1,5 @@ import { combineReducers } from 'redux'; -import serversReducer from '../servers/reducers/server'; +import serversReducer from '../servers/reducers/servers'; import selectedServerReducer from '../servers/reducers/selectedServer'; import shortUrlsListReducer from '../short-urls/reducers/shortUrlsList'; import shortUrlsListParamsReducer from '../short-urls/reducers/shortUrlsListParams'; diff --git a/src/servers/reducers/server.js b/src/servers/reducers/servers.js similarity index 83% rename from src/servers/reducers/server.js rename to src/servers/reducers/servers.js index d703ce11..41027c69 100644 --- a/src/servers/reducers/server.js +++ b/src/servers/reducers/servers.js @@ -3,21 +3,21 @@ import { pipe, isEmpty, assoc, map, prop } from 'ramda'; import { v4 as uuid } from 'uuid'; import { homepage } from '../../../package.json'; -export const FETCH_SERVERS = 'shlink/servers/FETCH_SERVERS'; +export const LIST_SERVERS = 'shlink/servers/LIST_SERVERS'; const initialState = {}; const assocId = (server) => assoc('id', server.id || uuid(), server); export default handleActions({ - [FETCH_SERVERS]: (state, { list }) => list, + [LIST_SERVERS]: (state, { list }) => list, }, initialState); export const listServers = ({ listServers, createServers }, { get }) => () => async (dispatch) => { const localList = listServers(); if (!isEmpty(localList)) { - dispatch({ type: FETCH_SERVERS, list: localList }); + dispatch({ type: LIST_SERVERS, list: localList }); return; } @@ -39,7 +39,7 @@ export const listServers = ({ listServers, createServers }, { get }) => () => as .catch(() => []); createServers(remoteList); - dispatch({ type: FETCH_SERVERS, list: remoteList.reduce((map, server) => ({ ...map, [server.id]: server }), {}) }); + dispatch({ type: LIST_SERVERS, list: remoteList.reduce((map, server) => ({ ...map, [server.id]: server }), {}) }); }; export const createServer = ({ createServer }, listServersAction) => pipe(createServer, listServersAction); diff --git a/src/servers/services/provideServices.js b/src/servers/services/provideServices.js index ce325ffd..fd3fcec7 100644 --- a/src/servers/services/provideServices.js +++ b/src/servers/services/provideServices.js @@ -6,7 +6,7 @@ import DeleteServerButton from '../DeleteServerButton'; import { EditServer } from '../EditServer'; import ImportServersBtn from '../helpers/ImportServersBtn'; import { resetSelectedServer, selectServer } from '../reducers/selectedServer'; -import { createServer, createServers, deleteServer, editServer, listServers } from '../reducers/server'; +import { createServer, createServers, deleteServer, editServer, listServers } from '../reducers/servers'; import ForServerVersion from '../helpers/ForServerVersion'; import { ServerError } from '../helpers/ServerError'; import ServersImporter from './ServersImporter'; diff --git a/test/servers/reducers/server.test.js b/test/servers/reducers/server.test.js index 73d6dd83..2a68bbce 100644 --- a/test/servers/reducers/server.test.js +++ b/test/servers/reducers/server.test.js @@ -5,15 +5,15 @@ import reducer, { listServers, createServers, editServer, - FETCH_SERVERS, -} from '../../../src/servers/reducers/server'; + LIST_SERVERS, +} from '../../../src/servers/reducers/servers'; describe('serverReducer', () => { const list = { abc123: { id: 'abc123' }, def456: { id: 'def456' }, }; - const expectedFetchServersResult = { type: FETCH_SERVERS, list }; + const expectedFetchServersResult = { type: LIST_SERVERS, list }; const ServersServiceMock = { listServers: jest.fn(() => list), createServer: jest.fn(), @@ -25,8 +25,8 @@ describe('serverReducer', () => { afterEach(jest.clearAllMocks); describe('reducer', () => { - it('returns servers when action is FETCH_SERVERS', () => - expect(reducer({}, { type: FETCH_SERVERS, list })).toEqual(list)); + it('returns servers when action is LIST_SERVERS', () => + expect(reducer({}, { type: LIST_SERVERS, list })).toEqual(list)); }); describe('action creators', () => { @@ -89,7 +89,7 @@ describe('serverReducer', () => { await listServers(NoListServersServiceMock, axios)()(dispatch); expect(dispatch).toHaveBeenCalledTimes(1); - expect(dispatch).toHaveBeenNthCalledWith(1, { type: FETCH_SERVERS, list: expectedList }); + expect(dispatch).toHaveBeenNthCalledWith(1, { type: LIST_SERVERS, list: expectedList }); expect(NoListServersServiceMock.listServers).toHaveBeenCalledTimes(1); expect(NoListServersServiceMock.createServer).not.toHaveBeenCalled(); expect(NoListServersServiceMock.editServer).not.toHaveBeenCalled(); From bcf5dcf180402c89cdd0736c18de11c3fe72f140 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 27 Apr 2020 11:30:51 +0200 Subject: [PATCH 06/10] Converted server handling actions into regular actions --- src/container/store.js | 2 +- src/servers/ServersDropdown.js | 3 - src/servers/reducers/servers.js | 28 +++++-- src/servers/services/provideServices.js | 10 +-- .../{server.test.js => servers.test.js} | 83 ++++++++++--------- 5 files changed, 70 insertions(+), 56 deletions(-) rename test/servers/reducers/{server.test.js => servers.test.js} (57%) diff --git a/src/container/store.js b/src/container/store.js index 6d0de762..41e9b9ce 100644 --- a/src/container/store.js +++ b/src/container/store.js @@ -8,7 +8,7 @@ const composeEnhancers = process.env.NODE_ENV !== 'production' && window.__REDUX : compose; const localStorageConfig = { - states: [ 'settings' ], + states: [ 'settings', 'servers' ], namespace: 'shlink', namespaceSeparator: '.', }; diff --git a/src/servers/ServersDropdown.js b/src/servers/ServersDropdown.js index f6092d60..6c036a69 100644 --- a/src/servers/ServersDropdown.js +++ b/src/servers/ServersDropdown.js @@ -8,7 +8,6 @@ const ServersDropdown = (serversExporter) => class ServersDropdown extends React static propTypes = { servers: PropTypes.object, selectedServer: serverType, - listServers: PropTypes.func, history: PropTypes.shape({ push: PropTypes.func, }), @@ -39,8 +38,6 @@ const ServersDropdown = (serversExporter) => class ServersDropdown extends React ); }; - componentDidMount = this.props.listServers; - render = () => ( Servers diff --git a/src/servers/reducers/servers.js b/src/servers/reducers/servers.js index 41027c69..09e18aa4 100644 --- a/src/servers/reducers/servers.js +++ b/src/servers/reducers/servers.js @@ -1,9 +1,14 @@ import { handleActions } from 'redux-actions'; -import { pipe, isEmpty, assoc, map, prop } from 'ramda'; +import { pipe, isEmpty, assoc, map, prop, reduce, dissoc } from 'ramda'; import { v4 as uuid } from 'uuid'; import { homepage } from '../../../package.json'; +/* eslint-disable padding-line-between-statements */ export const LIST_SERVERS = 'shlink/servers/LIST_SERVERS'; +export const EDIT_SERVER = 'shlink/servers/EDIT_SERVER'; +export const DELETE_SERVER = 'shlink/servers/DELETE_SERVER'; +export const CREATE_SERVERS = 'shlink/servers/CREATE_SERVERS'; +/* eslint-enable padding-line-between-statements */ const initialState = {}; @@ -11,6 +16,11 @@ const assocId = (server) => assoc('id', server.id || uuid(), server); export default handleActions({ [LIST_SERVERS]: (state, { list }) => list, + [CREATE_SERVERS]: (state, { newServers }) => ({ ...state, ...newServers }), + [DELETE_SERVER]: (state, { serverId }) => dissoc(serverId, state), + [EDIT_SERVER]: (state, { serverId, serverData }) => !state[serverId] + ? state + : assoc(serverId, { ...state[serverId], ...serverData }, state), }, initialState); export const listServers = ({ listServers, createServers }, { get }) => () => async (dispatch) => { @@ -42,14 +52,16 @@ export const listServers = ({ listServers, createServers }, { get }) => () => as dispatch({ type: LIST_SERVERS, list: remoteList.reduce((map, server) => ({ ...map, [server.id]: server }), {}) }); }; -export const createServer = ({ createServer }, listServersAction) => pipe(createServer, listServersAction); +export const createServer = (server) => createServers([ server ]); -export const editServer = ({ editServer }, listServersAction) => pipe(editServer, listServersAction); +const serversListToMap = reduce((acc, server) => assoc(server.id, server, acc), {}); -export const deleteServer = ({ deleteServer }, listServersAction) => pipe(deleteServer, listServersAction); - -export const createServers = ({ createServers }, listServersAction) => pipe( +export const createServers = pipe( map(assocId), - createServers, - listServersAction + serversListToMap, + (newServers) => ({ type: CREATE_SERVERS, newServers }) ); + +export const editServer = (serverId, serverData) => ({ type: EDIT_SERVER, serverId, serverData }); + +export const deleteServer = ({ id }) => ({ type: DELETE_SERVER, serverId: id }); diff --git a/src/servers/services/provideServices.js b/src/servers/services/provideServices.js index fd3fcec7..ba8e1a88 100644 --- a/src/servers/services/provideServices.js +++ b/src/servers/services/provideServices.js @@ -23,7 +23,7 @@ const provideServices = (bottle, connect, withRouter) => { bottle.serviceFactory('ServersDropdown', ServersDropdown, 'ServersExporter'); bottle.decorator('ServersDropdown', withRouter); - bottle.decorator('ServersDropdown', connect([ 'servers', 'selectedServer' ], [ 'listServers' ])); + bottle.decorator('ServersDropdown', connect([ 'servers', 'selectedServer' ])); bottle.serviceFactory('DeleteServerModal', () => DeleteServerModal); bottle.decorator('DeleteServerModal', withRouter); @@ -48,10 +48,10 @@ const provideServices = (bottle, connect, withRouter) => { // Actions bottle.serviceFactory('selectServer', selectServer, 'ServersService', 'buildShlinkApiClient', 'loadMercureInfo'); - bottle.serviceFactory('createServer', createServer, 'ServersService', 'listServers'); - bottle.serviceFactory('createServers', createServers, 'ServersService', 'listServers'); - bottle.serviceFactory('deleteServer', deleteServer, 'ServersService', 'listServers'); - bottle.serviceFactory('editServer', editServer, 'ServersService', 'listServers'); + bottle.serviceFactory('createServer', () => createServer); + bottle.serviceFactory('createServers', () => createServers); + bottle.serviceFactory('deleteServer', () => deleteServer); + bottle.serviceFactory('editServer', () => editServer); bottle.serviceFactory('listServers', listServers, 'ServersService', 'axios'); bottle.serviceFactory('resetSelectedServer', () => resetSelectedServer); diff --git a/test/servers/reducers/server.test.js b/test/servers/reducers/servers.test.js similarity index 57% rename from test/servers/reducers/server.test.js rename to test/servers/reducers/servers.test.js index 2a68bbce..24710c54 100644 --- a/test/servers/reducers/server.test.js +++ b/test/servers/reducers/servers.test.js @@ -6,6 +6,9 @@ import reducer, { createServers, editServer, LIST_SERVERS, + EDIT_SERVER, + DELETE_SERVER, + CREATE_SERVERS, } from '../../../src/servers/reducers/servers'; describe('serverReducer', () => { @@ -27,10 +30,36 @@ describe('serverReducer', () => { describe('reducer', () => { it('returns servers when action is LIST_SERVERS', () => expect(reducer({}, { type: LIST_SERVERS, list })).toEqual(list)); + + it('returns edited server when action is EDIT_SERVER', () => + expect(reducer( + list, + { type: EDIT_SERVER, serverId: 'abc123', serverData: { foo: 'foo' } }, + )).toEqual({ + abc123: { id: 'abc123', foo: 'foo' }, + def456: { id: 'def456' }, + })); + + it('removes server when action is DELETE_SERVER', () => + expect(reducer(list, { type: DELETE_SERVER, serverId: 'abc123' })).toEqual({ + def456: { id: 'def456' }, + })); + + it('appends server when action is CREATE_SERVERS', () => + expect(reducer(list, { + type: CREATE_SERVERS, + newServers: { + ghi789: { id: 'ghi789' }, + }, + })).toEqual({ + abc123: { id: 'abc123' }, + def456: { id: 'def456' }, + ghi789: { id: 'ghi789' }, + })); }); describe('action creators', () => { - describe('listServers', () => { + xdescribe('listServers', () => { const axios = { get: jest.fn() }; const dispatch = jest.fn(); const NoListServersServiceMock = { ...ServersServiceMock, listServers: jest.fn(() => ({})) }; @@ -100,62 +129,38 @@ describe('serverReducer', () => { }); describe('createServer', () => { - it('adds new server and then fetches servers again', () => { + it('returns expected action', () => { const serverToCreate = { id: 'abc123' }; - const result = createServer(ServersServiceMock, () => expectedFetchServersResult)(serverToCreate); + const result = createServer(serverToCreate); - expect(result).toEqual(expectedFetchServersResult); - expect(ServersServiceMock.listServers).not.toHaveBeenCalled(); - expect(ServersServiceMock.createServer).toHaveBeenCalledTimes(1); - expect(ServersServiceMock.createServer).toHaveBeenCalledWith(serverToCreate); - expect(ServersServiceMock.editServer).not.toHaveBeenCalled(); - expect(ServersServiceMock.deleteServer).not.toHaveBeenCalled(); - expect(ServersServiceMock.createServers).not.toHaveBeenCalled(); + expect(result).toEqual(expect.objectContaining({ type: CREATE_SERVERS })); }); }); describe('editServer', () => { - it('edits existing server and then fetches servers again', () => { - const serverToEdit = { name: 'edited' }; - const result = editServer(ServersServiceMock, () => expectedFetchServersResult)('123', serverToEdit); + it('returns expected action', () => { + const serverData = { name: 'edited' }; + const result = editServer('123', serverData); - expect(result).toEqual(expectedFetchServersResult); - expect(ServersServiceMock.listServers).not.toHaveBeenCalled(); - expect(ServersServiceMock.createServer).not.toHaveBeenCalled(); - expect(ServersServiceMock.editServer).toHaveBeenCalledTimes(1); - expect(ServersServiceMock.editServer).toHaveBeenCalledWith('123', serverToEdit); - expect(ServersServiceMock.deleteServer).not.toHaveBeenCalled(); - expect(ServersServiceMock.createServers).not.toHaveBeenCalled(); + expect(result).toEqual({ type: EDIT_SERVER, serverId: '123', serverData }); }); }); describe('deleteServer', () => { - it('deletes a server and then fetches servers again', () => { + it('returns expected action', () => { const serverToDelete = { id: 'abc123' }; - const result = deleteServer(ServersServiceMock, () => expectedFetchServersResult)(serverToDelete); + const result = deleteServer(serverToDelete); - expect(result).toEqual(expectedFetchServersResult); - expect(ServersServiceMock.listServers).not.toHaveBeenCalled(); - expect(ServersServiceMock.createServer).not.toHaveBeenCalled(); - expect(ServersServiceMock.createServers).not.toHaveBeenCalled(); - expect(ServersServiceMock.editServer).not.toHaveBeenCalled(); - expect(ServersServiceMock.deleteServer).toHaveBeenCalledTimes(1); - expect(ServersServiceMock.deleteServer).toHaveBeenCalledWith(serverToDelete); + expect(result).toEqual({ type: DELETE_SERVER, serverId: 'abc123' }); }); }); describe('createServers', () => { - it('creates multiple servers and then fetches servers again', () => { - const serversToCreate = values(list); - const result = createServers(ServersServiceMock, () => expectedFetchServersResult)(serversToCreate); + it('returns expected action', () => { + const newServers = values(list); + const result = createServers(newServers); - expect(result).toEqual(expectedFetchServersResult); - expect(ServersServiceMock.listServers).not.toHaveBeenCalled(); - expect(ServersServiceMock.createServer).not.toHaveBeenCalled(); - expect(ServersServiceMock.editServer).not.toHaveBeenCalled(); - expect(ServersServiceMock.deleteServer).not.toHaveBeenCalled(); - expect(ServersServiceMock.createServers).toHaveBeenCalledTimes(1); - expect(ServersServiceMock.createServers).toHaveBeenCalledWith(serversToCreate); + expect(result).toEqual(expect.objectContaining({ type: CREATE_SERVERS })); }); }); }); From bdd7932e074cef0dab0ba10b30370cfb504111e4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 27 Apr 2020 12:30:17 +0200 Subject: [PATCH 07/10] Refactored ServersDropdown into functional component --- src/servers/ServersDropdown.js | 68 ++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 32 deletions(-) diff --git a/src/servers/ServersDropdown.js b/src/servers/ServersDropdown.js index 6c036a69..d997c403 100644 --- a/src/servers/ServersDropdown.js +++ b/src/servers/ServersDropdown.js @@ -4,46 +4,50 @@ import { DropdownItem, DropdownMenu, DropdownToggle, UncontrolledDropdown } from import PropTypes from 'prop-types'; import { serverType } from './prop-types'; -const ServersDropdown = (serversExporter) => class ServersDropdown extends React.Component { - static propTypes = { - servers: PropTypes.object, - selectedServer: serverType, - history: PropTypes.shape({ - push: PropTypes.func, - }), - }; +const propTypes = { + servers: PropTypes.object, + selectedServer: serverType, + history: PropTypes.shape({ + push: PropTypes.func, + }), +}; - renderServers = () => { - const { servers, selectedServer } = this.props; +const ServersDropdown = (serversExporter) => { + const ServersDropdownComp = ({ servers, selectedServer, history }) => { const serversList = values(servers); - const { push } = this.props.history; - const loadServer = (id) => push(`/server/${id}/list-short-urls/1`); + const loadServer = (id) => history.push(`/server/${id}/list-short-urls/1`); - if (isEmpty(serversList)) { - return Add a server first...; - } + const renderServers = () => { + if (isEmpty(serversList)) { + return Add a server first...; + } + + return ( + + {serversList.map(({ name, id }) => ( + loadServer(id)}> + {name} + + ))} + + serversExporter.exportServers()}> + Export servers + + + ); + }; return ( - - {serversList.map(({ name, id }) => ( - loadServer(id)}> - {name} - - ))} - - serversExporter.exportServers()}> - Export servers - - + + Servers + {renderServers()} + ); }; - render = () => ( - - Servers - {this.renderServers()} - - ); + ServersDropdownComp.propTypes = propTypes; + + return ServersDropdownComp; }; export default ServersDropdown; From b08c6748c721ccf76e1c547519cda29794d421e5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 27 Apr 2020 12:54:52 +0200 Subject: [PATCH 08/10] Moved remote servers loading to separated action --- src/App.js | 47 ++++++++---- src/container/index.js | 1 + src/servers/reducers/remoteServers.js | 22 ++++++ src/servers/reducers/servers.js | 34 +-------- src/servers/services/provideServices.js | 5 +- test/servers/reducers/remoteServers.test.js | 55 ++++++++++++++ test/servers/reducers/servers.test.js | 82 --------------------- 7 files changed, 114 insertions(+), 132 deletions(-) create mode 100644 src/servers/reducers/remoteServers.js create mode 100644 test/servers/reducers/remoteServers.test.js diff --git a/src/App.js b/src/App.js index edb9df36..5ed710e8 100644 --- a/src/App.js +++ b/src/App.js @@ -1,23 +1,40 @@ -import React from 'react'; +import React, { useEffect } from 'react'; +import PropTypes from 'prop-types'; import { Route, Switch } from 'react-router-dom'; import NotFound from './common/NotFound'; import './App.scss'; -const App = (MainHeader, Home, MenuLayout, CreateServer, EditServer, Settings) => () => ( -
- +const propTypes = { + fetchServers: PropTypes.func, + servers: PropTypes.object, +}; -
- - - - - - - - +const App = (MainHeader, Home, MenuLayout, CreateServer, EditServer, Settings) => ({ fetchServers, servers }) => { + // On first load, try to fetch the remote servers if the list is empty + useEffect(() => { + if (Object.keys(servers).length === 0) { + fetchServers(); + } + }, []); + + return ( +
+ + +
+ + + + + + + + +
-
-); + ); +}; + +App.propTypes = propTypes; export default App; diff --git a/src/container/index.js b/src/container/index.js index 3a8ac83a..51309435 100644 --- a/src/container/index.js +++ b/src/container/index.js @@ -29,6 +29,7 @@ const connect = (propsFromState, actionServiceNames = []) => ); bottle.serviceFactory('App', App, 'MainHeader', 'Home', 'MenuLayout', 'CreateServer', 'EditServer', 'Settings'); +bottle.decorator('App', connect([ 'servers' ], [ 'fetchServers' ])); provideCommonServices(bottle, connect, withRouter); provideShortUrlsServices(bottle, connect); diff --git a/src/servers/reducers/remoteServers.js b/src/servers/reducers/remoteServers.js new file mode 100644 index 00000000..675c36e7 --- /dev/null +++ b/src/servers/reducers/remoteServers.js @@ -0,0 +1,22 @@ +import { pipe, prop } from 'ramda'; +import { homepage } from '../../../package.json'; +import { createServers } from './servers'; + +const responseToServersList = pipe( + prop('data'), + (value) => { + if (!Array.isArray(value)) { + throw new Error('Value is not an array'); + } + + return value; + }, +); + +export const fetchServers = ({ get }) => () => async (dispatch) => { + const remoteList = await get(`${homepage}/servers.json`) + .then(responseToServersList) + .catch(() => []); + + dispatch(createServers(remoteList)); +}; diff --git a/src/servers/reducers/servers.js b/src/servers/reducers/servers.js index 09e18aa4..61c60ac4 100644 --- a/src/servers/reducers/servers.js +++ b/src/servers/reducers/servers.js @@ -1,10 +1,8 @@ import { handleActions } from 'redux-actions'; -import { pipe, isEmpty, assoc, map, prop, reduce, dissoc } from 'ramda'; +import { pipe, assoc, map, reduce, dissoc } from 'ramda'; import { v4 as uuid } from 'uuid'; -import { homepage } from '../../../package.json'; /* eslint-disable padding-line-between-statements */ -export const LIST_SERVERS = 'shlink/servers/LIST_SERVERS'; export const EDIT_SERVER = 'shlink/servers/EDIT_SERVER'; export const DELETE_SERVER = 'shlink/servers/DELETE_SERVER'; export const CREATE_SERVERS = 'shlink/servers/CREATE_SERVERS'; @@ -15,7 +13,6 @@ const initialState = {}; const assocId = (server) => assoc('id', server.id || uuid(), server); export default handleActions({ - [LIST_SERVERS]: (state, { list }) => list, [CREATE_SERVERS]: (state, { newServers }) => ({ ...state, ...newServers }), [DELETE_SERVER]: (state, { serverId }) => dissoc(serverId, state), [EDIT_SERVER]: (state, { serverId, serverData }) => !state[serverId] @@ -23,35 +20,6 @@ export default handleActions({ : assoc(serverId, { ...state[serverId], ...serverData }, state), }, initialState); -export const listServers = ({ listServers, createServers }, { get }) => () => async (dispatch) => { - const localList = listServers(); - - if (!isEmpty(localList)) { - dispatch({ type: LIST_SERVERS, list: localList }); - - return; - } - - // If local list is empty, try to fetch it remotely (making sure it's an array) and calculate IDs for every server - const getDataAsArrayWithIds = pipe( - prop('data'), - (value) => { - if (!Array.isArray(value)) { - throw new Error('Value is not an array'); - } - - return value; - }, - map(assocId), - ); - const remoteList = await get(`${homepage}/servers.json`) - .then(getDataAsArrayWithIds) - .catch(() => []); - - createServers(remoteList); - dispatch({ type: LIST_SERVERS, list: remoteList.reduce((map, server) => ({ ...map, [server.id]: server }), {}) }); -}; - export const createServer = (server) => createServers([ server ]); const serversListToMap = reduce((acc, server) => assoc(server.id, server, acc), {}); diff --git a/src/servers/services/provideServices.js b/src/servers/services/provideServices.js index ba8e1a88..9168bc52 100644 --- a/src/servers/services/provideServices.js +++ b/src/servers/services/provideServices.js @@ -6,7 +6,8 @@ import DeleteServerButton from '../DeleteServerButton'; import { EditServer } from '../EditServer'; import ImportServersBtn from '../helpers/ImportServersBtn'; import { resetSelectedServer, selectServer } from '../reducers/selectedServer'; -import { createServer, createServers, deleteServer, editServer, listServers } from '../reducers/servers'; +import { createServer, createServers, deleteServer, editServer } from '../reducers/servers'; +import { fetchServers } from '../reducers/remoteServers'; import ForServerVersion from '../helpers/ForServerVersion'; import { ServerError } from '../helpers/ServerError'; import ServersImporter from './ServersImporter'; @@ -52,7 +53,7 @@ const provideServices = (bottle, connect, withRouter) => { bottle.serviceFactory('createServers', () => createServers); bottle.serviceFactory('deleteServer', () => deleteServer); bottle.serviceFactory('editServer', () => editServer); - bottle.serviceFactory('listServers', listServers, 'ServersService', 'axios'); + bottle.serviceFactory('fetchServers', fetchServers, 'axios'); bottle.serviceFactory('resetSelectedServer', () => resetSelectedServer); }; diff --git a/test/servers/reducers/remoteServers.test.js b/test/servers/reducers/remoteServers.test.js new file mode 100644 index 00000000..ba4afc80 --- /dev/null +++ b/test/servers/reducers/remoteServers.test.js @@ -0,0 +1,55 @@ +import { fetchServers } from '../../../src/servers/reducers/remoteServers'; +import { CREATE_SERVERS } from '../../../src/servers/reducers/servers'; + +describe('remoteServersReducer', () => { + afterEach(jest.resetAllMocks); + + describe('fetchServers', () => { + const axios = { get: jest.fn() }; + const dispatch = jest.fn(); + + it.each([ + [ + Promise.resolve({ + data: [ + { + id: '111', + name: 'acel.me from servers.json', + url: 'https://acel.me', + apiKey: '07fb8a96-8059-4094-a24c-80a7d5e7e9b0', + }, + { + id: '222', + name: 'Local from servers.json', + url: 'http://localhost:8000', + apiKey: '7a531c75-134e-4d5c-86e0-a71b7167b57a', + }, + ], + }), + { + 111: { + id: '111', + name: 'acel.me from servers.json', + url: 'https://acel.me', + apiKey: '07fb8a96-8059-4094-a24c-80a7d5e7e9b0', + }, + 222: { + id: '222', + name: 'Local from servers.json', + url: 'http://localhost:8000', + apiKey: '7a531c75-134e-4d5c-86e0-a71b7167b57a', + }, + }, + ], + [ Promise.resolve(''), {}], + [ Promise.reject({}), {}], + ])('tries to fetch servers from remote', async (mockedValue, expectedList) => { + axios.get.mockReturnValue(mockedValue); + + await fetchServers(axios)()(dispatch); + + expect(dispatch).toHaveBeenCalledWith({ type: CREATE_SERVERS, newServers: expectedList }); + expect(axios.get).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/test/servers/reducers/servers.test.js b/test/servers/reducers/servers.test.js index 24710c54..319e18d2 100644 --- a/test/servers/reducers/servers.test.js +++ b/test/servers/reducers/servers.test.js @@ -2,10 +2,8 @@ import { values } from 'ramda'; import reducer, { createServer, deleteServer, - listServers, createServers, editServer, - LIST_SERVERS, EDIT_SERVER, DELETE_SERVER, CREATE_SERVERS, @@ -16,21 +14,10 @@ describe('serverReducer', () => { abc123: { id: 'abc123' }, def456: { id: 'def456' }, }; - const expectedFetchServersResult = { type: LIST_SERVERS, list }; - const ServersServiceMock = { - listServers: jest.fn(() => list), - createServer: jest.fn(), - editServer: jest.fn(), - deleteServer: jest.fn(), - createServers: jest.fn(), - }; afterEach(jest.clearAllMocks); describe('reducer', () => { - it('returns servers when action is LIST_SERVERS', () => - expect(reducer({}, { type: LIST_SERVERS, list })).toEqual(list)); - it('returns edited server when action is EDIT_SERVER', () => expect(reducer( list, @@ -59,75 +46,6 @@ describe('serverReducer', () => { }); describe('action creators', () => { - xdescribe('listServers', () => { - const axios = { get: jest.fn() }; - const dispatch = jest.fn(); - const NoListServersServiceMock = { ...ServersServiceMock, listServers: jest.fn(() => ({})) }; - - it('fetches servers from local storage when found', async () => { - await listServers(ServersServiceMock, axios)()(dispatch); - - expect(dispatch).toHaveBeenCalledTimes(1); - expect(dispatch).toHaveBeenNthCalledWith(1, expectedFetchServersResult); - expect(ServersServiceMock.listServers).toHaveBeenCalledTimes(1); - expect(ServersServiceMock.createServer).not.toHaveBeenCalled(); - expect(ServersServiceMock.editServer).not.toHaveBeenCalled(); - expect(ServersServiceMock.deleteServer).not.toHaveBeenCalled(); - expect(ServersServiceMock.createServers).not.toHaveBeenCalled(); - expect(axios.get).not.toHaveBeenCalled(); - }); - - it.each([ - [ - Promise.resolve({ - data: [ - { - id: '111', - name: 'acel.me from servers.json', - url: 'https://acel.me', - apiKey: '07fb8a96-8059-4094-a24c-80a7d5e7e9b0', - }, - { - id: '222', - name: 'Local from servers.json', - url: 'http://localhost:8000', - apiKey: '7a531c75-134e-4d5c-86e0-a71b7167b57a', - }, - ], - }), - { - 111: { - id: '111', - name: 'acel.me from servers.json', - url: 'https://acel.me', - apiKey: '07fb8a96-8059-4094-a24c-80a7d5e7e9b0', - }, - 222: { - id: '222', - name: 'Local from servers.json', - url: 'http://localhost:8000', - apiKey: '7a531c75-134e-4d5c-86e0-a71b7167b57a', - }, - }, - ], - [ Promise.resolve(''), {}], - [ Promise.reject({}), {}], - ])('tries to fetch servers from remote when not found locally', async (mockedValue, expectedList) => { - axios.get.mockReturnValue(mockedValue); - - await listServers(NoListServersServiceMock, axios)()(dispatch); - - expect(dispatch).toHaveBeenCalledTimes(1); - expect(dispatch).toHaveBeenNthCalledWith(1, { type: LIST_SERVERS, list: expectedList }); - expect(NoListServersServiceMock.listServers).toHaveBeenCalledTimes(1); - expect(NoListServersServiceMock.createServer).not.toHaveBeenCalled(); - expect(NoListServersServiceMock.editServer).not.toHaveBeenCalled(); - expect(NoListServersServiceMock.deleteServer).not.toHaveBeenCalled(); - expect(NoListServersServiceMock.createServers).toHaveBeenCalledTimes(1); - expect(axios.get).toHaveBeenCalledTimes(1); - }); - }); - describe('createServer', () => { it('returns expected action', () => { const serverToCreate = { id: 'abc123' }; From a7f941e8e4c7dd84b0f06eb4143bd2caacf4a7b0 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 27 Apr 2020 13:21:07 +0200 Subject: [PATCH 09/10] Deleted no-longer-needed ServersService --- src/servers/reducers/selectedServer.js | 9 +- src/servers/services/ServersExporter.js | 6 +- src/servers/services/ServersService.js | 38 ----- src/servers/services/provideServices.js | 6 +- test/servers/reducers/selectedServer.test.js | 27 ++-- test/servers/services/ServersExporter.test.js | 28 ++-- test/servers/services/ServersService.test.js | 130 ------------------ 7 files changed, 35 insertions(+), 209 deletions(-) delete mode 100644 src/servers/services/ServersService.js delete mode 100644 test/servers/services/ServersService.test.js diff --git a/src/servers/reducers/selectedServer.js b/src/servers/reducers/selectedServer.js index 64d94ae1..835df2a7 100644 --- a/src/servers/reducers/selectedServer.js +++ b/src/servers/reducers/selectedServer.js @@ -25,12 +25,15 @@ const getServerVersion = memoizeWith(identity, (serverId, health) => health().th export const resetSelectedServer = createAction(RESET_SELECTED_SERVER); -export const selectServer = ({ findServerById }, buildShlinkApiClient, loadMercureInfo) => (serverId) => async ( - dispatch +export const selectServer = (buildShlinkApiClient, loadMercureInfo) => (serverId) => async ( + dispatch, + getState ) => { dispatch(resetSelectedServer()); dispatch(resetShortUrlParams()); - const selectedServer = findServerById(serverId); + + const { servers } = getState(); + const selectedServer = servers[serverId]; if (!selectedServer) { dispatch({ diff --git a/src/servers/services/ServersExporter.js b/src/servers/services/ServersExporter.js index 9caf5ab9..01a33316 100644 --- a/src/servers/services/ServersExporter.js +++ b/src/servers/services/ServersExporter.js @@ -25,14 +25,14 @@ const saveCsv = (window, csv) => { }; export default class ServersExporter { - constructor(serversService, window, csvjson) { - this.serversService = serversService; + constructor(storage, window, csvjson) { + this.storage = storage; this.window = window; this.csvjson = csvjson; } exportServers = async () => { - const servers = values(this.serversService.listServers()).map(dissoc('id')); + const servers = values(this.storage.get('servers') || {}).map(dissoc('id')); try { const csv = this.csvjson.toCSV(servers, { diff --git a/src/servers/services/ServersService.js b/src/servers/services/ServersService.js deleted file mode 100644 index 9ec44afa..00000000 --- a/src/servers/services/ServersService.js +++ /dev/null @@ -1,38 +0,0 @@ -import { assoc, dissoc, reduce } from 'ramda'; - -const SERVERS_STORAGE_KEY = 'servers'; - -export default class ServersService { - constructor(storage) { - this.storage = storage; - } - - listServers = () => this.storage.get(SERVERS_STORAGE_KEY) || {}; - - findServerById = (serverId) => this.listServers()[serverId]; - - createServer = (server) => this.createServers([ server ]); - - createServers = (servers) => { - const allServers = reduce( - (serversObj, server) => assoc(server.id, server, serversObj), - this.listServers(), - servers - ); - - this.storage.set(SERVERS_STORAGE_KEY, allServers); - }; - - deleteServer = ({ id }) => - this.storage.set(SERVERS_STORAGE_KEY, dissoc(id, this.listServers())); - - editServer = (id, serverData) => { - const allServers = this.listServers(); - - if (!allServers[id]) { - return; - } - - this.storage.set(SERVERS_STORAGE_KEY, assoc(id, { ...allServers[id], ...serverData }, allServers)); - } -} diff --git a/src/servers/services/provideServices.js b/src/servers/services/provideServices.js index 9168bc52..a431d363 100644 --- a/src/servers/services/provideServices.js +++ b/src/servers/services/provideServices.js @@ -11,7 +11,6 @@ import { fetchServers } from '../reducers/remoteServers'; import ForServerVersion from '../helpers/ForServerVersion'; import { ServerError } from '../helpers/ServerError'; import ServersImporter from './ServersImporter'; -import ServersService from './ServersService'; import ServersExporter from './ServersExporter'; const provideServices = (bottle, connect, withRouter) => { @@ -44,11 +43,10 @@ const provideServices = (bottle, connect, withRouter) => { // Services bottle.constant('csvjson', csvjson); bottle.service('ServersImporter', ServersImporter, 'csvjson'); - bottle.service('ServersService', ServersService, 'Storage'); - bottle.service('ServersExporter', ServersExporter, 'ServersService', 'window', 'csvjson'); + bottle.service('ServersExporter', ServersExporter, 'Storage', 'window', 'csvjson'); // Actions - bottle.serviceFactory('selectServer', selectServer, 'ServersService', 'buildShlinkApiClient', 'loadMercureInfo'); + bottle.serviceFactory('selectServer', selectServer, 'buildShlinkApiClient', 'loadMercureInfo'); bottle.serviceFactory('createServer', () => createServer); bottle.serviceFactory('createServers', () => createServers); bottle.serviceFactory('deleteServer', () => deleteServer); diff --git a/test/servers/reducers/selectedServer.test.js b/test/servers/reducers/selectedServer.test.js index 2efd8291..9307841f 100644 --- a/test/servers/reducers/selectedServer.test.js +++ b/test/servers/reducers/selectedServer.test.js @@ -32,9 +32,7 @@ describe('selectedServerReducer', () => { id: 'abc123', }; const version = '1.19.0'; - const ServersServiceMock = { - findServerById: jest.fn(() => selectedServer), - }; + const createGetStateMock = (id) => jest.fn().mockReturnValue({ servers: { [id]: selectedServer } }); const apiClientMock = { health: jest.fn(), }; @@ -49,6 +47,8 @@ describe('selectedServerReducer', () => { [ 'latest', MAX_FALLBACK_VERSION, 'latest' ], [ '%invalid_semver%', MIN_FALLBACK_VERSION, '%invalid_semver%' ], ])('dispatches proper actions', async (serverVersion, expectedVersion, expectedPrintableVersion) => { + const id = uuid(); + const getState = createGetStateMock(id); const expectedSelectedServer = { ...selectedServer, version: expectedVersion, @@ -57,7 +57,7 @@ describe('selectedServerReducer', () => { apiClientMock.health.mockResolvedValue({ version: serverVersion }); - await selectServer(ServersServiceMock, buildApiClient, loadMercureInfo)(uuid())(dispatch); + await selectServer(buildApiClient, loadMercureInfo)(id)(dispatch, getState); expect(dispatch).toHaveBeenCalledTimes(4); expect(dispatch).toHaveBeenNthCalledWith(1, { type: RESET_SELECTED_SERVER }); @@ -67,18 +67,23 @@ describe('selectedServerReducer', () => { }); it('invokes dependencies', async () => { - await selectServer(ServersServiceMock, buildApiClient, loadMercureInfo)(uuid())(() => {}); + const id = uuid(); + const getState = createGetStateMock(id); - expect(ServersServiceMock.findServerById).toHaveBeenCalledTimes(1); + await selectServer(buildApiClient, loadMercureInfo)(id)(() => {}, getState); + + expect(getState).toHaveBeenCalledTimes(1); expect(buildApiClient).toHaveBeenCalledTimes(1); }); it('dispatches error when health endpoint fails', async () => { + const id = uuid(); + const getState = createGetStateMock(id); const expectedSelectedServer = { ...selectedServer, serverNotReachable: true }; apiClientMock.health.mockRejectedValue({}); - await selectServer(ServersServiceMock, buildApiClient, loadMercureInfo)(uuid())(dispatch); + await selectServer(buildApiClient, loadMercureInfo)(id)(dispatch, getState); expect(apiClientMock.health).toHaveBeenCalled(); expect(dispatch).toHaveBeenNthCalledWith(3, { type: SELECT_SERVER, selectedServer: expectedSelectedServer }); @@ -86,13 +91,13 @@ describe('selectedServerReducer', () => { }); it('dispatches error when server is not found', async () => { + const id = uuid(); + const getState = jest.fn(() => ({ servers: {} })); const expectedSelectedServer = { serverNotFound: true }; - ServersServiceMock.findServerById.mockReturnValue(undefined); + await selectServer(buildApiClient, loadMercureInfo)(id)(dispatch, getState); - await selectServer(ServersServiceMock, buildApiClient, loadMercureInfo)(uuid())(dispatch); - - expect(ServersServiceMock.findServerById).toHaveBeenCalled(); + expect(getState).toHaveBeenCalled(); expect(apiClientMock.health).not.toHaveBeenCalled(); expect(dispatch).toHaveBeenNthCalledWith(3, { type: SELECT_SERVER, selectedServer: expectedSelectedServer }); expect(loadMercureInfo).not.toHaveBeenCalled(); diff --git a/test/servers/services/ServersExporter.test.js b/test/servers/services/ServersExporter.test.js index 772df918..1dc2156e 100644 --- a/test/servers/services/ServersExporter.test.js +++ b/test/servers/services/ServersExporter.test.js @@ -18,8 +18,8 @@ describe('ServersExporter', () => { }, }, }); - const serversServiceMock = { - listServers: jest.fn(() => ({ + const storageMock = { + get: jest.fn(() => ({ abc123: { id: 'abc123', name: 'foo', @@ -48,19 +48,15 @@ describe('ServersExporter', () => { global.console = { error: jest.fn() }; global.Blob = class Blob {}; global.URL = { createObjectURL: () => '' }; - serversServiceMock.listServers.mockReset(); }); afterEach(() => { global.console = originalConsole; + jest.clearAllMocks(); }); it('logs an error if something fails', () => { const csvjsonMock = createCsvjsonMock(true); - const exporter = new ServersExporter( - serversServiceMock, - createWindowMock(), - csvjsonMock, - ); + const exporter = new ServersExporter(storageMock, createWindowMock(), csvjsonMock); exporter.exportServers(); @@ -70,30 +66,22 @@ describe('ServersExporter', () => { it('makes use of msSaveBlob API when available', () => { const windowMock = createWindowMock(); - const exporter = new ServersExporter( - serversServiceMock, - windowMock, - createCsvjsonMock(), - ); + const exporter = new ServersExporter(storageMock, windowMock, createCsvjsonMock()); exporter.exportServers(); - expect(serversServiceMock.listServers).toHaveBeenCalledTimes(1); + expect(storageMock.get).toHaveBeenCalledTimes(1); expect(windowMock.navigator.msSaveBlob).toHaveBeenCalledTimes(1); expect(windowMock.document.createElement).not.toHaveBeenCalled(); }); it('makes use of download link API when available', () => { const windowMock = createWindowMock(false); - const exporter = new ServersExporter( - serversServiceMock, - windowMock, - createCsvjsonMock(), - ); + const exporter = new ServersExporter(storageMock, windowMock, createCsvjsonMock()); exporter.exportServers(); - expect(serversServiceMock.listServers).toHaveBeenCalledTimes(1); + expect(storageMock.get).toHaveBeenCalledTimes(1); expect(windowMock.document.createElement).toHaveBeenCalledTimes(1); expect(windowMock.document.body.appendChild).toHaveBeenCalledTimes(1); expect(windowMock.document.body.removeChild).toHaveBeenCalledTimes(1); diff --git a/test/servers/services/ServersService.test.js b/test/servers/services/ServersService.test.js deleted file mode 100644 index 3c8b805f..00000000 --- a/test/servers/services/ServersService.test.js +++ /dev/null @@ -1,130 +0,0 @@ -import ServersService from '../../../src/servers/services/ServersService'; - -describe('ServersService', () => { - const servers = { - abc123: { id: 'abc123' }, - def456: { id: 'def456' }, - }; - const createService = (withServers = true) => { - const storageMock = { - set: jest.fn(), - get: jest.fn(() => withServers ? servers : undefined), - }; - const service = new ServersService(storageMock); - - return [ service, storageMock ]; - }; - - describe('listServers', () => { - it('returns an empty object when servers are not found in storage', () => { - const [ service, storageMock ] = createService(false); - - const result = service.listServers(); - - expect(result).toEqual({}); - expect(storageMock.get).toHaveBeenCalledTimes(1); - expect(storageMock.set).not.toHaveBeenCalled(); - }); - - it('returns value from storage when found', () => { - const [ service, storageMock ] = createService(); - - const result = service.listServers(); - - expect(result).toEqual(servers); - expect(storageMock.get).toHaveBeenCalledTimes(1); - expect(storageMock.set).not.toHaveBeenCalled(); - }); - }); - - describe('findServerById', () => { - it('returns undefined when requested server is not found', () => { - const [ service, storageMock ] = createService(); - - const result = service.findServerById('ghi789'); - - expect(result).toBeUndefined(); - expect(storageMock.get).toHaveBeenCalledTimes(1); - expect(storageMock.set).not.toHaveBeenCalled(); - }); - - it('returns server from list when found', () => { - const [ service, storageMock ] = createService(); - - const result = service.findServerById('abc123'); - - expect(result).toEqual({ id: 'abc123' }); - expect(storageMock.get).toHaveBeenCalledTimes(1); - expect(storageMock.set).not.toHaveBeenCalled(); - }); - }); - - describe('createServer', () => { - it('adds one server to the list', () => { - const [ service, storageMock ] = createService(); - - service.createServer({ id: 'ghi789' }); - - expect(storageMock.get).toHaveBeenCalledTimes(1); - expect(storageMock.set).toHaveBeenCalledTimes(1); - expect(storageMock.set).toHaveBeenCalledWith(expect.anything(), { - abc123: { id: 'abc123' }, - def456: { id: 'def456' }, - ghi789: { id: 'ghi789' }, - }); - }); - }); - - describe('createServers', () => { - it('adds multiple servers to the list', () => { - const [ service, storageMock ] = createService(); - - service.createServers([{ id: 'ghi789' }, { id: 'jkl123' }]); - - expect(storageMock.get).toHaveBeenCalledTimes(1); - expect(storageMock.set).toHaveBeenCalledTimes(1); - expect(storageMock.set).toHaveBeenCalledWith(expect.anything(), { - abc123: { id: 'abc123' }, - def456: { id: 'def456' }, - ghi789: { id: 'ghi789' }, - jkl123: { id: 'jkl123' }, - }); - }); - }); - - describe('deleteServer', () => { - it('removes one server from the list', () => { - const [ service, storageMock ] = createService(); - - service.deleteServer({ id: 'abc123' }); - - expect(storageMock.get).toHaveBeenCalledTimes(1); - expect(storageMock.set).toHaveBeenCalledTimes(1); - expect(storageMock.set).toHaveBeenCalledWith(expect.anything(), { - def456: { id: 'def456' }, - }); - }); - }); - - describe('editServer', () => { - it('dos nothing is provided server does not exist', () => { - const [ service, storageMock ] = createService(); - - service.editServer('notFound', {}); - - expect(storageMock.set).not.toHaveBeenCalled(); - }); - - it('updates the list with provided server data', () => { - const [ service, storageMock ] = createService(); - const serverData = { name: 'foo', apiKey: 'bar' }; - - service.editServer('abc123', serverData); - - expect(storageMock.set).toHaveBeenCalledWith(expect.anything(), { - abc123: { id: 'abc123', ...serverData }, - def456: { id: 'def456' }, - }); - }); - }); -}); From be2f86050f15e48b3bcfda52b54ee963988e04a9 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 27 Apr 2020 13:37:54 +0200 Subject: [PATCH 10/10] Updated changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cc73f47..0adcc7f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), #### Changed -* [#218](https://github.com/shlinkio/shlink-web-client/issues/218) Added back button to sections not displayed in left menu +* [#218](https://github.com/shlinkio/shlink-web-client/issues/218) Added back button to sections not displayed in left menu. +* [#255](https://github.com/shlinkio/shlink-web-client/issues/255) Improved how servers and settings are persisted in the local storage. #### Deprecated