Merge pull request #282 from acelaya-forks/feature/mercure-dup-code

Feature/mercure dup code

CHANGELOG.md

@@ -12,7 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 #### Changed
 
-* *Nothing*
+* [#254](https://github.com/shlinkio/shlink-web-client/issues/254) Reduced duplication on code to handle mercure topics binding.
 
 #### Deprecated
 

src/mercure/helpers/index.js

@@ -1,10 +1,10 @@
+import { useEffect } from 'react';
 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;
 
-  if (!enabled || loading || error) {
+  if (loading || error) {
     return undefined;
   }
 
@@ -22,3 +22,7 @@ export const bindToMercureTopic = (mercureInfo, realTimeUpdates, topic, onMessag
 
   return () => es.close();
 };
+
+export const useMercureTopicBinding = (mercureInfo, topic, onMessage, onTokenExpired) => {
+  useEffect(bindToMercureTopic(mercureInfo, topic, onMessage, onTokenExpired), [ mercureInfo ]);
+};

src/mercure/reducers/mercureInfo.js

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

src/short-urls/ShortUrlsList.js

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

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', 'settings' ],
+    [ 'selectedServer', 'shortUrlsListParams', 'mercureInfo' ],
     [ 'listShortUrls', 'resetShortUrlParams', 'createNewVisit', 'loadMercureInfo' ]
   ));
 

src/tags/TagsList.js

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

src/tags/services/provideServices.js

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

src/visits/ShortUrlVisits.js

@@ -2,8 +2,7 @@ import React, { useEffect } from 'react';
 import PropTypes from 'prop-types';
 import qs from 'qs';
 import { MercureInfoType } from '../mercure/reducers/mercureInfo';
-import { bindToMercureTopic } from '../mercure/helpers';
+import { useMercureTopicBinding } from '../mercure/helpers';
-import { SettingsType } from '../settings/reducers/settings';
 import { shortUrlVisitsType } from './reducers/shortUrlVisits';
 import ShortUrlVisitsHeader from './ShortUrlVisitsHeader';
 import { shortUrlDetailType } from './reducers/shortUrlDetail';
@@ -26,7 +25,6 @@ const propTypes = {
   createNewVisit: PropTypes.func,
   loadMercureInfo: PropTypes.func,
   mercureInfo: MercureInfoType,
-  settings: SettingsType,
 };
 
 const ShortUrlVisits = (VisitsStats) => {
@@ -42,7 +40,6 @@ const ShortUrlVisits = (VisitsStats) => {
     createNewVisit,
     loadMercureInfo,
     mercureInfo,
-    settings: { realTimeUpdates },
   }) => {
     const { params } = match;
     const { shortCode } = params;
@@ -54,16 +51,7 @@ const ShortUrlVisits = (VisitsStats) => {
     useEffect(() => {
       getShortUrlDetail(shortCode, domain);
     }, []);
-    useEffect(
+    useMercureTopicBinding(mercureInfo, `https://shlink.io/new-visit/${shortCode}`, createNewVisit, loadMercureInfo);
-      bindToMercureTopic(
-        mercureInfo,
-        realTimeUpdates,
-        `https://shlink.io/new-visit/${shortCode}`,
-        createNewVisit,
-        loadMercureInfo
-      ),
-      [ mercureInfo ],
-    );
 
     return (
       <VisitsStats getVisits={loadVisits} cancelGetVisits={cancelGetShortUrlVisits} visitsInfo={shortUrlVisits}>

src/visits/TagVisits.js

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

src/visits/services/provideServices.js

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

test/mercure/helpers/index.test.js

@@ -11,12 +11,11 @@ describe('helpers', () => {
     const onTokenExpired = jest.fn();
 
     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(onMessage).not.toHaveBeenCalled();
@@ -36,7 +35,7 @@ describe('helpers', () => {
         error: false,
         mercureHubUrl,
         token,
-      }, { enabled: true }, topic, onMessage, onTokenExpired)();
+      }, topic, onMessage, onTokenExpired)();
 
       expect(EventSource).toHaveBeenCalledWith(hubUrl, {
         headers: {

test/mercure/reducers/mercureInfo.test.js

@@ -40,14 +40,31 @@ describe('mercureInfoReducer', () => {
       mercureInfo: jest.fn(() => result),
     });
     const dispatch = jest.fn();
-    const getState = () => ({});
+    const createGetStateMock = (enabled) => jest.fn(() => ({
+      settings: {
+        realTimeUpdates: { enabled },
+      },
+    }));
 
     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 () => {
       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(dispatch).toHaveBeenCalledTimes(2);
@@ -58,8 +75,9 @@ describe('mercureInfoReducer', () => {
     it('throws error on failure', async () => {
       const error = '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(dispatch).toHaveBeenCalledTimes(2);

test/short-urls/ShortUrlsList.test.js

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

test/tags/TagsList.test.js

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

test/visits/ShortUrlVisits.test.js

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

test/visits/TagVisits.test.js

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