From 7b80d78dc5cccd028b1d389d2c6b1b3190ffcf88 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 21 Apr 2019 11:31:40 +0200 Subject: [PATCH] Removed duplicated code when building ShlinkApiClient --- src/common/MenuLayout.js | 4 +-- src/short-urls/helpers/ShortUrlsRowMenu.js | 4 +-- src/short-urls/reducers/shortUrlCreation.js | 5 ++-- src/short-urls/reducers/shortUrlDeletion.js | 3 +- src/short-urls/reducers/shortUrlTags.js | 5 ++-- src/short-urls/reducers/shortUrlsList.js | 3 +- src/tags/reducers/tagDelete.js | 5 ++-- src/tags/reducers/tagEdit.js | 5 ++-- src/tags/reducers/tagsList.js | 13 +++----- src/tags/services/provideServices.js | 8 +++-- src/utils/services/ShlinkApiClientBuilder.js | 17 ++++++++--- src/utils/utils.js | 2 ++ src/visits/reducers/shortUrlDetail.js | 5 ++-- src/visits/reducers/shortUrlVisits.js | 3 +- .../services/ShlinkApiClientBuilder.test.js | 30 ++++++++++++------- 15 files changed, 60 insertions(+), 52 deletions(-) diff --git a/src/common/MenuLayout.js b/src/common/MenuLayout.js index 6ec90603..f2edf47f 100644 --- a/src/common/MenuLayout.js +++ b/src/common/MenuLayout.js @@ -20,9 +20,7 @@ const MenuLayout = (TagsList, ShortUrls, AsideMenu, CreateShortUrl, ShortUrlVisi state = { showSideBar: false }; - // FIXME Shouldn't use componentWillMount, but this code has to be run before children components are rendered - /* eslint react/no-deprecated: "off" */ - componentWillMount() { + componentDidMount() { const { match, selectServer } = this.props; const { params: { serverId } } = match; diff --git a/src/short-urls/helpers/ShortUrlsRowMenu.js b/src/short-urls/helpers/ShortUrlsRowMenu.js index bebc5ee7..19c7fea0 100644 --- a/src/short-urls/helpers/ShortUrlsRowMenu.js +++ b/src/short-urls/helpers/ShortUrlsRowMenu.js @@ -35,7 +35,7 @@ const ShortUrlsRowMenu = (DeleteShortUrlModal, EditTagsModal) => class ShortUrls toggle = () => this.setState(({ isOpen }) => ({ isOpen: !isOpen })); render() { - const { onCopyToClipboard, shortUrl, selectedServer: { id } } = this.props; + const { onCopyToClipboard, shortUrl, selectedServer } = this.props; const completeShortUrl = shortUrl && shortUrl.shortUrl ? shortUrl.shortUrl : ''; const toggleModal = (prop) => () => this.setState((prevState) => ({ [prop]: !prevState[prop] })); const toggleQrCode = toggleModal('isQrModalOpen'); @@ -49,7 +49,7 @@ const ShortUrlsRowMenu = (DeleteShortUrlModal, EditTagsModal) => class ShortUrls    - +  Visit stats diff --git a/src/short-urls/reducers/shortUrlCreation.js b/src/short-urls/reducers/shortUrlCreation.js index 2cd4e9e3..0640f2c7 100644 --- a/src/short-urls/reducers/shortUrlCreation.js +++ b/src/short-urls/reducers/shortUrlCreation.js @@ -32,11 +32,10 @@ export default handleActions({ export const createShortUrl = (buildShlinkApiClient) => (data) => async (dispatch, getState) => { dispatch({ type: CREATE_SHORT_URL_START }); - const { selectedServer } = getState(); - const shlinkApiClient = buildShlinkApiClient(selectedServer); + const { createShortUrl } = await buildShlinkApiClient(getState); try { - const result = await shlinkApiClient.createShortUrl(data); + const result = await createShortUrl(data); dispatch({ type: CREATE_SHORT_URL, result }); } catch (e) { diff --git a/src/short-urls/reducers/shortUrlDeletion.js b/src/short-urls/reducers/shortUrlDeletion.js index 2c2a568b..62d63208 100644 --- a/src/short-urls/reducers/shortUrlDeletion.js +++ b/src/short-urls/reducers/shortUrlDeletion.js @@ -36,8 +36,7 @@ export default handleActions({ export const deleteShortUrl = (buildShlinkApiClient) => (shortCode) => async (dispatch, getState) => { dispatch({ type: DELETE_SHORT_URL_START }); - const { selectedServer } = getState(); - const { deleteShortUrl } = buildShlinkApiClient(selectedServer); + const { deleteShortUrl } = await buildShlinkApiClient(getState); try { await deleteShortUrl(shortCode); diff --git a/src/short-urls/reducers/shortUrlTags.js b/src/short-urls/reducers/shortUrlTags.js index 9c9f8149..3109f0f0 100644 --- a/src/short-urls/reducers/shortUrlTags.js +++ b/src/short-urls/reducers/shortUrlTags.js @@ -33,11 +33,10 @@ export default handleActions({ export const editShortUrlTags = (buildShlinkApiClient) => (shortCode, tags) => async (dispatch, getState) => { dispatch({ type: EDIT_SHORT_URL_TAGS_START }); - const { selectedServer } = getState(); - const shlinkApiClient = buildShlinkApiClient(selectedServer); + const { updateShortUrlTags } = await buildShlinkApiClient(getState); try { - const normalizedTags = await shlinkApiClient.updateShortUrlTags(shortCode, tags); + const normalizedTags = await updateShortUrlTags(shortCode, tags); dispatch({ tags: normalizedTags, shortCode, type: EDIT_SHORT_URL_TAGS }); } catch (e) { diff --git a/src/short-urls/reducers/shortUrlsList.js b/src/short-urls/reducers/shortUrlsList.js index e48c0492..c977f458 100644 --- a/src/short-urls/reducers/shortUrlsList.js +++ b/src/short-urls/reducers/shortUrlsList.js @@ -45,8 +45,7 @@ export default handleActions({ export const listShortUrls = (buildShlinkApiClient) => (params = {}) => async (dispatch, getState) => { dispatch({ type: LIST_SHORT_URLS_START }); - const { selectedServer = {} } = getState(); - const { listShortUrls } = buildShlinkApiClient(selectedServer); + const { listShortUrls } = await buildShlinkApiClient(getState); try { const shortUrls = await listShortUrls(params); diff --git a/src/tags/reducers/tagDelete.js b/src/tags/reducers/tagDelete.js index d8fd747a..0420da6d 100644 --- a/src/tags/reducers/tagDelete.js +++ b/src/tags/reducers/tagDelete.js @@ -27,11 +27,10 @@ export default handleActions({ export const deleteTag = (buildShlinkApiClient) => (tag) => async (dispatch, getState) => { dispatch({ type: DELETE_TAG_START }); - const { selectedServer } = getState(); - const shlinkApiClient = buildShlinkApiClient(selectedServer); + const { deleteTags } = await buildShlinkApiClient(getState); try { - await shlinkApiClient.deleteTags([ tag ]); + await deleteTags([ tag ]); dispatch({ type: DELETE_TAG }); } catch (e) { dispatch({ type: DELETE_TAG_ERROR }); diff --git a/src/tags/reducers/tagEdit.js b/src/tags/reducers/tagEdit.js index 2613169e..560397a8 100644 --- a/src/tags/reducers/tagEdit.js +++ b/src/tags/reducers/tagEdit.js @@ -32,11 +32,10 @@ export const editTag = (buildShlinkApiClient, colorGenerator) => (oldName, newNa ) => { dispatch({ type: EDIT_TAG_START }); - const { selectedServer } = getState(); - const shlinkApiClient = buildShlinkApiClient(selectedServer); + const { editTag } = await buildShlinkApiClient(getState); try { - await shlinkApiClient.editTag(oldName, newName); + await editTag(oldName, newName); colorGenerator.setColorForKey(newName, color); dispatch({ type: EDIT_TAG, oldName, newName }); } catch (e) { diff --git a/src/tags/reducers/tagsList.js b/src/tags/reducers/tagsList.js index 899be3e6..622389c1 100644 --- a/src/tags/reducers/tagsList.js +++ b/src/tags/reducers/tagsList.js @@ -1,6 +1,5 @@ import { handleActions } from 'redux-actions'; import { isEmpty, reject } from 'ramda'; -import { buildShlinkApiClientWithAxios as buildShlinkApiClient } from '../../utils/services/ShlinkApiClientBuilder'; import { TAG_DELETED } from './tagDelete'; import { TAG_EDITED } from './tagEdit'; @@ -41,8 +40,8 @@ export default handleActions({ }), }, initialState); -export const _listTags = (buildShlinkApiClient, force = false) => async (dispatch, getState) => { - const { tagsList, selectedServer } = getState(); +export const listTags = (buildShlinkApiClient, force = true) => () => async (dispatch, getState) => { + const { tagsList } = getState(); if (!force && (tagsList.loading || !isEmpty(tagsList.tags))) { return; @@ -51,8 +50,8 @@ export const _listTags = (buildShlinkApiClient, force = false) => async (dispatc dispatch({ type: LIST_TAGS_START }); try { - const shlinkApiClient = buildShlinkApiClient(selectedServer); - const tags = await shlinkApiClient.listTags(); + const { listTags } = await buildShlinkApiClient(getState); + const tags = await listTags(); dispatch({ tags, type: LIST_TAGS }); } catch (e) { @@ -60,10 +59,6 @@ export const _listTags = (buildShlinkApiClient, force = false) => async (dispatc } }; -export const listTags = () => _listTags(buildShlinkApiClient); - -export const forceListTags = () => _listTags(buildShlinkApiClient, true); - export const filterTags = (searchTerm) => ({ type: FILTER_TAGS, searchTerm, diff --git a/src/tags/services/provideServices.js b/src/tags/services/provideServices.js index ba7c7be9..e7e0c481 100644 --- a/src/tags/services/provideServices.js +++ b/src/tags/services/provideServices.js @@ -3,7 +3,7 @@ import TagCard from '../TagCard'; import DeleteTagConfirmModal from '../helpers/DeleteTagConfirmModal'; import EditTagModal from '../helpers/EditTagModal'; import TagsList from '../TagsList'; -import { filterTags, forceListTags, listTags } from '../reducers/tagsList'; +import { filterTags, listTags } from '../reducers/tagsList'; import { deleteTag, tagDeleted } from '../reducers/tagDelete'; import { editTag, tagEdited } from '../reducers/tagEdit'; @@ -24,9 +24,11 @@ const provideServices = (bottle, connect) => { bottle.decorator('TagsList', connect([ 'tagsList' ], [ 'forceListTags', 'filterTags' ])); // Actions + const listTagsActionFactory = (force) => ({ buildShlinkApiClient }) => listTags(buildShlinkApiClient, force); + + bottle.factory('listTags', listTagsActionFactory(false)); + bottle.factory('forceListTags', listTagsActionFactory(true)); bottle.serviceFactory('filterTags', () => filterTags); - bottle.serviceFactory('forceListTags', () => forceListTags); - bottle.serviceFactory('listTags', () => listTags); bottle.serviceFactory('tagDeleted', () => tagDeleted); bottle.serviceFactory('tagEdited', () => tagEdited); diff --git a/src/utils/services/ShlinkApiClientBuilder.js b/src/utils/services/ShlinkApiClientBuilder.js index 23b050c8..c239bbc6 100644 --- a/src/utils/services/ShlinkApiClientBuilder.js +++ b/src/utils/services/ShlinkApiClientBuilder.js @@ -1,9 +1,20 @@ -import * as axios from 'axios'; +import { wait } from '../utils'; import ShlinkApiClient from './ShlinkApiClient'; const apiClients = {}; -const buildShlinkApiClient = (axios) => ({ url, apiKey }) => { +const getSelectedServerFromState = async (getState) => { + const { selectedServer } = getState(); + + if (!selectedServer) { + return wait(250).then(() => getSelectedServerFromState(getState)); + } + + return selectedServer; +}; + +const buildShlinkApiClient = (axios) => async (getState) => { + const { url, apiKey } = await getSelectedServerFromState(getState); const clientKey = `${url}_${apiKey}`; if (!apiClients[clientKey]) { @@ -14,5 +25,3 @@ const buildShlinkApiClient = (axios) => ({ url, apiKey }) => { }; export default buildShlinkApiClient; - -export const buildShlinkApiClientWithAxios = buildShlinkApiClient(axios); diff --git a/src/utils/utils.js b/src/utils/utils.js index c727965c..5f416bca 100644 --- a/src/utils/utils.js +++ b/src/utils/utils.js @@ -51,3 +51,5 @@ export const useToggle = (initialValue = false) => { return [ flag, () => setFlag(!flag) ]; }; + +export const wait = (seconds) => new Promise((resolve) => setTimeout(resolve, seconds)); diff --git a/src/visits/reducers/shortUrlDetail.js b/src/visits/reducers/shortUrlDetail.js index cd06db65..893c12ac 100644 --- a/src/visits/reducers/shortUrlDetail.js +++ b/src/visits/reducers/shortUrlDetail.js @@ -29,11 +29,10 @@ export default handleActions({ export const getShortUrlDetail = (buildShlinkApiClient) => (shortCode) => async (dispatch, getState) => { dispatch({ type: GET_SHORT_URL_DETAIL_START }); - const { selectedServer } = getState(); - const shlinkApiClient = buildShlinkApiClient(selectedServer); + const { getShortUrl } = await buildShlinkApiClient(getState); try { - const shortUrl = await shlinkApiClient.getShortUrl(shortCode); + const shortUrl = await getShortUrl(shortCode); dispatch({ shortUrl, type: GET_SHORT_URL_DETAIL }); } catch (e) { diff --git a/src/visits/reducers/shortUrlVisits.js b/src/visits/reducers/shortUrlVisits.js index 16c1f14f..faefcc5b 100644 --- a/src/visits/reducers/shortUrlVisits.js +++ b/src/visits/reducers/shortUrlVisits.js @@ -52,8 +52,7 @@ export default handleActions({ export const getShortUrlVisits = (buildShlinkApiClient) => (shortCode, dates) => async (dispatch, getState) => { dispatch({ type: GET_SHORT_URL_VISITS_START }); - const { selectedServer } = getState(); - const { getShortUrlVisits } = buildShlinkApiClient(selectedServer); + const { getShortUrlVisits } = await buildShlinkApiClient(getState); const itemsPerPage = 5000; const isLastPage = ({ currentPage, pagesCount }) => currentPage >= pagesCount; diff --git a/test/utils/services/ShlinkApiClientBuilder.test.js b/test/utils/services/ShlinkApiClientBuilder.test.js index 9316087f..c6875ed0 100644 --- a/test/utils/services/ShlinkApiClientBuilder.test.js +++ b/test/utils/services/ShlinkApiClientBuilder.test.js @@ -1,23 +1,33 @@ import buildShlinkApiClient from '../../../src/utils/services/ShlinkApiClientBuilder'; describe('ShlinkApiClientBuilder', () => { - const builder = buildShlinkApiClient({}); + const createBuilder = () => { + const builder = buildShlinkApiClient({}); - it('creates new instances when provided params are different', () => { - const firstApiClient = builder({ url: 'foo', apiKey: 'bar' }); - const secondApiClient = builder({ url: 'bar', apiKey: 'bar' }); - const thirdApiClient = builder({ url: 'bar', apiKey: 'foo' }); + return (selectedServer) => builder(() => ({ selectedServer })); + }; + + it('creates new instances when provided params are different', async () => { + const builder = createBuilder(); + const [ firstApiClient, secondApiClient, thirdApiClient ] = await Promise.all([ + builder({ url: 'foo', apiKey: 'bar' }), + builder({ url: 'bar', apiKey: 'bar' }), + builder({ url: 'bar', apiKey: 'foo' }), + ]); expect(firstApiClient).not.toBe(secondApiClient); expect(firstApiClient).not.toBe(thirdApiClient); expect(secondApiClient).not.toBe(thirdApiClient); }); - it('returns existing instances when provided params are the same', () => { - const params = { url: 'foo', apiKey: 'bar' }; - const firstApiClient = builder(params); - const secondApiClient = builder(params); - const thirdApiClient = builder(params); + it('returns existing instances when provided params are the same', async () => { + const builder = createBuilder(); + const selectedServer = { url: 'foo', apiKey: 'bar' }; + const [ firstApiClient, secondApiClient, thirdApiClient ] = await Promise.all([ + builder(selectedServer), + builder(selectedServer), + builder(selectedServer), + ]); expect(firstApiClient).toBe(secondApiClient); expect(firstApiClient).toBe(thirdApiClient);