Removed duplicated code when binding to mercure by checking if enabled first

This commit is contained in:
Alejandro Celaya 2020-06-06 09:24:05 +02:00
parent 05e3e87653
commit c46d5187c1
15 changed files with 47 additions and 53 deletions

View file

@ -1,10 +1,9 @@
import { EventSourcePolyfill as EventSource } from 'event-source-polyfill'; import { EventSourcePolyfill as EventSource } from 'event-source-polyfill';
export const bindToMercureTopic = (mercureInfo, realTimeUpdates, topic, onMessage, onTokenExpired) => () => { export const bindToMercureTopic = (mercureInfo, topic, onMessage, onTokenExpired) => () => {
const { enabled } = realTimeUpdates;
const { mercureHubUrl, token, loading, error } = mercureInfo; const { mercureHubUrl, token, loading, error } = mercureInfo;
if (!enabled || loading || error) { if (loading || error) {
return undefined; return undefined;
} }

View file

@ -29,8 +29,16 @@ export default handleActions({
export const loadMercureInfo = (buildShlinkApiClient) => () => async (dispatch, getState) => { export const loadMercureInfo = (buildShlinkApiClient) => () => async (dispatch, getState) => {
dispatch({ type: GET_MERCURE_INFO_START }); dispatch({ type: GET_MERCURE_INFO_START });
const { settings } = getState();
const { mercureInfo } = buildShlinkApiClient(getState); const { mercureInfo } = buildShlinkApiClient(getState);
if (!settings.realTimeUpdates.enabled) {
dispatch({ type: GET_MERCURE_INFO_ERROR });
return;
}
try { try {
const result = await mercureInfo(); const result = await mercureInfo();

View file

@ -9,7 +9,6 @@ import SortingDropdown from '../utils/SortingDropdown';
import { determineOrderDir } from '../utils/utils'; import { determineOrderDir } from '../utils/utils';
import { MercureInfoType } from '../mercure/reducers/mercureInfo'; import { MercureInfoType } from '../mercure/reducers/mercureInfo';
import { bindToMercureTopic } from '../mercure/helpers'; import { bindToMercureTopic } from '../mercure/helpers';
import { SettingsType } from '../settings/reducers/settings';
import { shortUrlType } from './reducers/shortUrlsList'; import { shortUrlType } from './reducers/shortUrlsList';
import { shortUrlsListParamsType } from './reducers/shortUrlsListParams'; import { shortUrlsListParamsType } from './reducers/shortUrlsListParams';
import './ShortUrlsList.scss'; import './ShortUrlsList.scss';
@ -34,7 +33,6 @@ const propTypes = {
createNewVisit: PropTypes.func, createNewVisit: PropTypes.func,
loadMercureInfo: PropTypes.func, loadMercureInfo: PropTypes.func,
mercureInfo: MercureInfoType, mercureInfo: MercureInfoType,
settings: SettingsType,
}; };
// FIXME Replace with typescript: (ShortUrlsRow component) // FIXME Replace with typescript: (ShortUrlsRow component)
@ -52,7 +50,6 @@ const ShortUrlsList = (ShortUrlsRow) => {
createNewVisit, createNewVisit,
loadMercureInfo, loadMercureInfo,
mercureInfo, mercureInfo,
settings: { realTimeUpdates },
}) => { }) => {
const { orderBy } = shortUrlsListParams; const { orderBy } = shortUrlsListParams;
const [ order, setOrder ] = useState({ const [ order, setOrder ] = useState({
@ -120,7 +117,7 @@ const ShortUrlsList = (ShortUrlsRow) => {
return resetShortUrlParams; return resetShortUrlParams;
}, []); }, []);
useEffect( useEffect(
bindToMercureTopic(mercureInfo, realTimeUpdates, 'https://shlink.io/new-visit', createNewVisit, loadMercureInfo), bindToMercureTopic(mercureInfo, 'https://shlink.io/new-visit', createNewVisit, loadMercureInfo),
[ mercureInfo ] [ mercureInfo ]
); );

View file

@ -31,7 +31,7 @@ const provideServices = (bottle, connect) => {
bottle.serviceFactory('ShortUrlsList', ShortUrlsList, 'ShortUrlsRow'); bottle.serviceFactory('ShortUrlsList', ShortUrlsList, 'ShortUrlsRow');
bottle.decorator('ShortUrlsList', connect( bottle.decorator('ShortUrlsList', connect(
[ 'selectedServer', 'shortUrlsListParams', 'mercureInfo', 'settings' ], [ 'selectedServer', 'shortUrlsListParams', 'mercureInfo' ],
[ 'listShortUrls', 'resetShortUrlParams', 'createNewVisit', 'loadMercureInfo' ] [ 'listShortUrls', 'resetShortUrlParams', 'createNewVisit', 'loadMercureInfo' ]
)); ));

View file

@ -5,7 +5,6 @@ import Message from '../utils/Message';
import SearchField from '../utils/SearchField'; import SearchField from '../utils/SearchField';
import { serverType } from '../servers/prop-types'; import { serverType } from '../servers/prop-types';
import { MercureInfoType } from '../mercure/reducers/mercureInfo'; import { MercureInfoType } from '../mercure/reducers/mercureInfo';
import { SettingsType } from '../settings/reducers/settings';
import { bindToMercureTopic } from '../mercure/helpers'; import { bindToMercureTopic } from '../mercure/helpers';
import { TagsListType } from './reducers/tagsList'; import { TagsListType } from './reducers/tagsList';
@ -20,21 +19,19 @@ const propTypes = {
createNewVisit: PropTypes.func, createNewVisit: PropTypes.func,
loadMercureInfo: PropTypes.func, loadMercureInfo: PropTypes.func,
mercureInfo: MercureInfoType, mercureInfo: MercureInfoType,
settings: SettingsType,
}; };
const TagsList = (TagCard) => { const TagsList = (TagCard) => {
const TagListComp = ( const TagListComp = (
{ filterTags, forceListTags, tagsList, selectedServer, createNewVisit, loadMercureInfo, mercureInfo, settings } { filterTags, forceListTags, tagsList, selectedServer, createNewVisit, loadMercureInfo, mercureInfo }
) => { ) => {
const { realTimeUpdates } = settings;
const [ displayedTag, setDisplayedTag ] = useState(); const [ displayedTag, setDisplayedTag ] = useState();
useEffect(() => { useEffect(() => {
forceListTags(); forceListTags();
}, []); }, []);
useEffect( useEffect(
bindToMercureTopic(mercureInfo, realTimeUpdates, 'https://shlink.io/new-visit', createNewVisit, loadMercureInfo), bindToMercureTopic(mercureInfo, 'https://shlink.io/new-visit', createNewVisit, loadMercureInfo),
[ mercureInfo ] [ mercureInfo ]
); );

View file

@ -29,7 +29,7 @@ const provideServices = (bottle, connect) => {
bottle.serviceFactory('TagsList', TagsList, 'TagCard'); bottle.serviceFactory('TagsList', TagsList, 'TagCard');
bottle.decorator('TagsList', connect( bottle.decorator('TagsList', connect(
[ 'tagsList', 'selectedServer', 'mercureInfo', 'settings' ], [ 'tagsList', 'selectedServer', 'mercureInfo' ],
[ 'forceListTags', 'filterTags', 'createNewVisit', 'loadMercureInfo' ] [ 'forceListTags', 'filterTags', 'createNewVisit', 'loadMercureInfo' ]
)); ));

View file

@ -3,7 +3,6 @@ import PropTypes from 'prop-types';
import qs from 'qs'; import qs from 'qs';
import { MercureInfoType } from '../mercure/reducers/mercureInfo'; import { MercureInfoType } from '../mercure/reducers/mercureInfo';
import { bindToMercureTopic } from '../mercure/helpers'; import { bindToMercureTopic } from '../mercure/helpers';
import { SettingsType } from '../settings/reducers/settings';
import { shortUrlVisitsType } from './reducers/shortUrlVisits'; import { shortUrlVisitsType } from './reducers/shortUrlVisits';
import ShortUrlVisitsHeader from './ShortUrlVisitsHeader'; import ShortUrlVisitsHeader from './ShortUrlVisitsHeader';
import { shortUrlDetailType } from './reducers/shortUrlDetail'; import { shortUrlDetailType } from './reducers/shortUrlDetail';
@ -26,7 +25,6 @@ const propTypes = {
createNewVisit: PropTypes.func, createNewVisit: PropTypes.func,
loadMercureInfo: PropTypes.func, loadMercureInfo: PropTypes.func,
mercureInfo: MercureInfoType, mercureInfo: MercureInfoType,
settings: SettingsType,
}; };
const ShortUrlVisits = (VisitsStats) => { const ShortUrlVisits = (VisitsStats) => {
@ -42,7 +40,6 @@ const ShortUrlVisits = (VisitsStats) => {
createNewVisit, createNewVisit,
loadMercureInfo, loadMercureInfo,
mercureInfo, mercureInfo,
settings: { realTimeUpdates },
}) => { }) => {
const { params } = match; const { params } = match;
const { shortCode } = params; const { shortCode } = params;
@ -55,13 +52,7 @@ const ShortUrlVisits = (VisitsStats) => {
getShortUrlDetail(shortCode, domain); getShortUrlDetail(shortCode, domain);
}, []); }, []);
useEffect( useEffect(
bindToMercureTopic( bindToMercureTopic(mercureInfo, `https://shlink.io/new-visit/${shortCode}`, createNewVisit, loadMercureInfo),
mercureInfo,
realTimeUpdates,
`https://shlink.io/new-visit/${shortCode}`,
createNewVisit,
loadMercureInfo
),
[ mercureInfo ], [ mercureInfo ],
); );

View file

@ -1,7 +1,6 @@
import React, { useEffect } from 'react'; import React, { useEffect } from 'react';
import PropTypes from 'prop-types'; import PropTypes from 'prop-types';
import { MercureInfoType } from '../mercure/reducers/mercureInfo'; import { MercureInfoType } from '../mercure/reducers/mercureInfo';
import { SettingsType } from '../settings/reducers/settings';
import { bindToMercureTopic } from '../mercure/helpers'; import { bindToMercureTopic } from '../mercure/helpers';
import { TagVisitsType } from './reducers/tagVisits'; import { TagVisitsType } from './reducers/tagVisits';
import TagVisitsHeader from './TagVisitsHeader'; import TagVisitsHeader from './TagVisitsHeader';
@ -19,7 +18,6 @@ const propTypes = {
createNewVisit: PropTypes.func, createNewVisit: PropTypes.func,
loadMercureInfo: PropTypes.func, loadMercureInfo: PropTypes.func,
mercureInfo: MercureInfoType, mercureInfo: MercureInfoType,
settings: SettingsType,
}; };
const TagVisits = (VisitsStats, colorGenerator) => { const TagVisits = (VisitsStats, colorGenerator) => {
@ -32,20 +30,13 @@ const TagVisits = (VisitsStats, colorGenerator) => {
createNewVisit, createNewVisit,
loadMercureInfo, loadMercureInfo,
mercureInfo, mercureInfo,
settings: { realTimeUpdates },
}) => { }) => {
const { params } = match; const { params } = match;
const { tag } = params; const { tag } = params;
const loadVisits = (dates) => getTagVisits(tag, dates); const loadVisits = (dates) => getTagVisits(tag, dates);
useEffect( useEffect(
bindToMercureTopic( bindToMercureTopic(mercureInfo, 'https://shlink.io/new-visit', createNewVisit, loadMercureInfo),
mercureInfo,
realTimeUpdates,
'https://shlink.io/new-visit',
createNewVisit,
loadMercureInfo
),
[ mercureInfo ], [ mercureInfo ],
); );

View file

@ -16,12 +16,12 @@ const provideServices = (bottle, connect) => {
bottle.serviceFactory('VisitsStats', VisitsStats, 'VisitsParser', 'OpenMapModalBtn'); bottle.serviceFactory('VisitsStats', VisitsStats, 'VisitsParser', 'OpenMapModalBtn');
bottle.serviceFactory('ShortUrlVisits', ShortUrlVisits, 'VisitsStats'); bottle.serviceFactory('ShortUrlVisits', ShortUrlVisits, 'VisitsStats');
bottle.decorator('ShortUrlVisits', connect( bottle.decorator('ShortUrlVisits', connect(
[ 'shortUrlVisits', 'shortUrlDetail', 'mercureInfo', 'settings' ], [ 'shortUrlVisits', 'shortUrlDetail', 'mercureInfo' ],
[ 'getShortUrlVisits', 'getShortUrlDetail', 'cancelGetShortUrlVisits', 'createNewVisit', 'loadMercureInfo' ] [ 'getShortUrlVisits', 'getShortUrlDetail', 'cancelGetShortUrlVisits', 'createNewVisit', 'loadMercureInfo' ]
)); ));
bottle.serviceFactory('TagVisits', TagVisits, 'VisitsStats', 'ColorGenerator'); bottle.serviceFactory('TagVisits', TagVisits, 'VisitsStats', 'ColorGenerator');
bottle.decorator('TagVisits', connect( bottle.decorator('TagVisits', connect(
[ 'tagVisits', 'mercureInfo', 'settings' ], [ 'tagVisits', 'mercureInfo' ],
[ 'getTagVisits', 'cancelGetTagVisits', 'createNewVisit', 'loadMercureInfo' ] [ 'getTagVisits', 'cancelGetTagVisits', 'createNewVisit', 'loadMercureInfo' ]
)); ));

View file

@ -11,12 +11,11 @@ describe('helpers', () => {
const onTokenExpired = jest.fn(); const onTokenExpired = jest.fn();
it.each([ it.each([
[{ loading: true, error: false }, { enabled: true }], [{ loading: true, error: false }],
[{ loading: false, error: true }, { enabled: true }], [{ loading: false, error: true }],
[{ loading: true, error: true }, { enabled: true }], [{ loading: true, error: true }],
[{ loading: false, error: false }, { enabled: false }], ])('does not bind an EventSource when loading or error', (mercureInfo) => {
])('does not bind an EventSource when disabled, loading or error', (mercureInfo, realTimeUpdates) => { bindToMercureTopic(mercureInfo)();
bindToMercureTopic(mercureInfo, realTimeUpdates)();
expect(EventSource).not.toHaveBeenCalled(); expect(EventSource).not.toHaveBeenCalled();
expect(onMessage).not.toHaveBeenCalled(); expect(onMessage).not.toHaveBeenCalled();
@ -36,7 +35,7 @@ describe('helpers', () => {
error: false, error: false,
mercureHubUrl, mercureHubUrl,
token, token,
}, { enabled: true }, topic, onMessage, onTokenExpired)(); }, topic, onMessage, onTokenExpired)();
expect(EventSource).toHaveBeenCalledWith(hubUrl, { expect(EventSource).toHaveBeenCalledWith(hubUrl, {
headers: { headers: {

View file

@ -40,14 +40,31 @@ describe('mercureInfoReducer', () => {
mercureInfo: jest.fn(() => result), mercureInfo: jest.fn(() => result),
}); });
const dispatch = jest.fn(); const dispatch = jest.fn();
const getState = () => ({}); const createGetStateMock = (enabled) => jest.fn(() => ({
settings: {
realTimeUpdates: { enabled },
},
}));
afterEach(jest.resetAllMocks); afterEach(jest.resetAllMocks);
it('dispatches error when real time updates are disabled', async () => {
const apiClientMock = createApiClientMock(Promise.resolve(mercureInfo));
const getState = createGetStateMock(false);
await loadMercureInfo(() => apiClientMock)()(dispatch, getState);
expect(apiClientMock.mercureInfo).not.toHaveBeenCalled();
expect(dispatch).toHaveBeenCalledTimes(2);
expect(dispatch).toHaveBeenNthCalledWith(1, { type: GET_MERCURE_INFO_START });
expect(dispatch).toHaveBeenNthCalledWith(2, { type: GET_MERCURE_INFO_ERROR });
});
it('calls API on success', async () => { it('calls API on success', async () => {
const apiClientMock = createApiClientMock(Promise.resolve(mercureInfo)); const apiClientMock = createApiClientMock(Promise.resolve(mercureInfo));
const getState = createGetStateMock(true);
await loadMercureInfo(() => apiClientMock)()(dispatch, getState()); await loadMercureInfo(() => apiClientMock)()(dispatch, getState);
expect(apiClientMock.mercureInfo).toHaveBeenCalledTimes(1); expect(apiClientMock.mercureInfo).toHaveBeenCalledTimes(1);
expect(dispatch).toHaveBeenCalledTimes(2); expect(dispatch).toHaveBeenCalledTimes(2);
@ -58,8 +75,9 @@ describe('mercureInfoReducer', () => {
it('throws error on failure', async () => { it('throws error on failure', async () => {
const error = 'Error'; const error = 'Error';
const apiClientMock = createApiClientMock(Promise.reject(error)); const apiClientMock = createApiClientMock(Promise.reject(error));
const getState = createGetStateMock(true);
await loadMercureInfo(() => apiClientMock)()(dispatch, getState()); await loadMercureInfo(() => apiClientMock)()(dispatch, getState);
expect(apiClientMock.mercureInfo).toHaveBeenCalledTimes(1); expect(apiClientMock.mercureInfo).toHaveBeenCalledTimes(1);
expect(dispatch).toHaveBeenCalledTimes(2); expect(dispatch).toHaveBeenCalledTimes(2);

View file

@ -9,7 +9,6 @@ describe('<ShortUrlsList />', () => {
const ShortUrlsRow = () => ''; const ShortUrlsRow = () => '';
const listShortUrlsMock = jest.fn(); const listShortUrlsMock = jest.fn();
const resetShortUrlParamsMock = jest.fn(); const resetShortUrlParamsMock = jest.fn();
const realTimeUpdates = { enabled: true };
const ShortUrlsList = shortUrlsListCreator(ShortUrlsRow); const ShortUrlsList = shortUrlsListCreator(ShortUrlsRow);
@ -38,7 +37,6 @@ describe('<ShortUrlsList />', () => {
] ]
} }
mercureInfo={{ loading: true }} mercureInfo={{ loading: true }}
settings={{ realTimeUpdates }}
/> />
); );
}); });

View file

@ -15,7 +15,7 @@ describe('<TagsList />', () => {
const TagsList = createTagsList(TagCard); const TagsList = createTagsList(TagCard);
wrapper = shallow( wrapper = shallow(
<TagsList forceListTags={identity} filterTags={filterTags} match={{ params }} tagsList={tagsList} settings={{}} /> <TagsList forceListTags={identity} filterTags={filterTags} match={{ params }} tagsList={tagsList} />
); );
return wrapper; return wrapper;

View file

@ -14,7 +14,6 @@ describe('<ShortUrlVisits />', () => {
const history = { const history = {
goBack: jest.fn(), goBack: jest.fn(),
}; };
const realTimeUpdates = { enabled: true };
const VisitsStats = jest.fn(); const VisitsStats = jest.fn();
beforeEach(() => { beforeEach(() => {
@ -31,7 +30,6 @@ describe('<ShortUrlVisits />', () => {
shortUrlDetail={{}} shortUrlDetail={{}}
cancelGetShortUrlVisits={identity} cancelGetShortUrlVisits={identity}
matchMedia={() => ({ matches: false })} matchMedia={() => ({ matches: false })}
settings={{ realTimeUpdates }}
/> />
); );
}); });

View file

@ -13,7 +13,6 @@ describe('<TagVisits />', () => {
const history = { const history = {
goBack: jest.fn(), goBack: jest.fn(),
}; };
const realTimeUpdates = { enabled: true };
const VisitsStats = jest.fn(); const VisitsStats = jest.fn();
beforeEach(() => { beforeEach(() => {
@ -26,7 +25,6 @@ describe('<TagVisits />', () => {
history={history} history={history}
tagVisits={{ loading: true, visits: [] }} tagVisits={{ loading: true, visits: [] }}
cancelGetTagVisits={identity} cancelGetTagVisits={identity}
settings={{ realTimeUpdates }}
/> />
); );
}); });